{"action":"created","issue":{"url":"https://api.github.com/repos/symfony/symfony/issues/18308","repository_url":"https://api.github.com/repos/symfony/symfony","labels_url":"https://api.github.com/repos/symfony/symfony/issues/18308/labels{/name}","comments_url":"https://api.github.com/repos/symfony/symfony/issues/18308/comments","events_url":"https://api.github.com/repos/symfony/symfony/issues/18308/events","html_url":"https://github.com/symfony/symfony/pull/18308","id":143472558,"number":18308,"title":"Added an ArgumentResolver with clean extension point","user":{"login":"iltar","id":1754678,"avatar_url":"https://avatars.githubusercontent.com/u/1754678?v=3","gravatar_id":"","url":"https://api.github.com/users/iltar","html_url":"https://github.com/iltar","followers_url":"https://api.github.com/users/iltar/followers","following_url":"https://api.github.com/users/iltar/following{/other_user}","gists_url":"https://api.github.com/users/iltar/gists{/gist_id}","starred_url":"https://api.github.com/users/iltar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/iltar/subscriptions","organizations_url":"https://api.github.com/users/iltar/orgs","repos_url":"https://api.github.com/users/iltar/repos","events_url":"https://api.github.com/users/iltar/events{/privacy}","received_events_url":"https://api.github.com/users/iltar/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/symfony/symfony/labels/Status:%20Needs%20Review","name":"Status: Needs Review","color":"ededed"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":9,"created_at":"2016-03-25T10:25:01Z","updated_at":"2016-03-28T09:40:15Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/symfony/symfony/pulls/18308","html_url":"https://github.com/symfony/symfony/pull/18308","diff_url":"https://github.com/symfony/symfony/pull/18308.diff","patch_url":"https://github.com/symfony/symfony/pull/18308.patch"},"body":"| Q             | A\r\n| ------------- | ---\r\n| Branch?       | master\r\n| Bug fix?      | no\r\n| New feature?  | yes\r\n| BC breaks?    | no\r\n| Deprecations? | yes\r\n| Tests pass?   | yes\r\n| Fixed tickets | #17933 (pre-work), #1547, #10710\r\n| License       | MIT\r\n| Doc PR        | ~\r\n\r\n**This PR is a follow up for and blocked by: #18187**, relates to #11457 by @WouterJ. When reviewing, please take the last commit: [Added an ArgumentResolver with clean extension point](https://github.com/symfony/symfony/pull/18308/commits/4c092b31f91d6071918c90acf15d0035a2a6d12d)\r\n\r\nThis PR provides:\r\n- The ability to tag your own `ArgumentValueResolverInterface`. This means that you can effectively expand on the argument resolving in the `HttpKernel` without having to implement your own `ArgumentResolver`.\r\n\r\n- The possibility to cache away argument metadata via a new `ArgumentMetadataFactory` which simply fetches the data from the cache, effectively omitting 1 reflection call per request. *Not implemented in this PR, but possible once this is merged.*\r\n\r\n- The possibility to add a PSR-7 adapter to resolve the correct request, avoids the paramconverters\r\n- The possibility to add a value resolver to fetch stuff from $request->query\r\n- Drupal could simplify [their argument resolving](https://github.com/drupal/drupal/blob/8.1.x/core/lib/Drupal/Core/Controller/ControllerResolver.php) by a lot\r\n- etc.\r\n\r\nThe aim for this PR is to provide a 100% BC variant to add argument resolving in a clean way, this is shown by the 2 tests: `LegacyArgumentResolverTest` and `ArgumentResolverTest`. \r\n\r\n/cc @dawehner @larowlan if you have time, can you check the impact for Drupal? I think this should be a very simple change which should make it more maintainable. "},"comment":{"url":"https://api.github.com/repos/symfony/symfony/issues/comments/202322812","html_url":"https://github.com/symfony/symfony/pull/18308#issuecomment-202322812","issue_url":"https://api.github.com/repos/symfony/symfony/issues/18308","id":202322812,"user":{"login":"iltar","id":1754678,"avatar_url":"https://avatars.githubusercontent.com/u/1754678?v=3","gravatar_id":"","url":"https://api.github.com/users/iltar","html_url":"https://github.com/iltar","followers_url":"https://api.github.com/users/iltar/followers","following_url":"https://api.github.com/users/iltar/following{/other_user}","gists_url":"https://api.github.com/users/iltar/gists{/gist_id}","starred_url":"https://api.github.com/users/iltar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/iltar/subscriptions","organizations_url":"https://api.github.com/users/iltar/orgs","repos_url":"https://api.github.com/users/iltar/repos","events_url":"https://api.github.com/users/iltar/events{/privacy}","received_events_url":"https://api.github.com/users/iltar/received_events","type":"User","site_admin":false},"created_at":"2016-03-28T09:40:15Z","updated_at":"2016-03-28T09:40:15Z","body":"@fabpot I'll hook it up somewhere this week and check the performance. Ideally I'd also implement a cache for the benchmarks (because that's what this is all pre-work for).\r\n\r\nI agree about the complexity, that's why I want to keep this as simple as possible. Here's a list of the classes/interfaces and what they are designed for:\r\n - `ArgumentResolver(Interface)`; This is from the previous PR, making it easier to implement your own variant (this PR). This also allows the `LegacyArgumentResolver` (currently in `ControllerResolver`) to be injected.\r\n - `ArgumentValueResolverInterface`; This is what can actually be used on an application level for action arguments, think about: [ParamConverter to inject the current user #327](https://github.com/sensiolabs/SensioFrameworkExtraBundle/pull/327). Parameter converters are nice, but slow(er than when this is cached) due to their reflection. This involves less magic to implement and a clear trace for developers where it hooks in. It also avoids injecting objects in the Request object, scoping them more effectively.\r\n - `ArgumentMetadataInterface`; As I have commented before, this is a nice way to use the following bullet point to return a cached variant if generating classes.\r\n - `ArgumentMetadataFactoryInterface`; This allows multiple implementations to accommodate a cache layer. My idea is to decorate the real-time resolver, when the cached version does not exist, I can fall back to the real-time and still provide the same functionality. I think this interface should be marked with `@internal` but I'm not 100% sure, in case someone wants to provide their own layer (like never resolving due to performance?).\r\n\r\nRegarding the cache implementation I want to open a new PR so it can focus on that specific subject as I'm inexperienced with caching."}}