-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Description
Hi everyone!
Recently got interested in @ParamConverters as a convenient extension point in Symfony. Some time ago in sensiolabs/SensioFrameworkExtraBundle#436 @fabpot proposed an idea on how to implement configurable argument resolvers. Let me explain the idea:
- A single annotation
@Arg(string $name, array options = [])is proposed as a replacement for@ParamConverter. - A new interface
ArgumentValueOptionsInterface extends ArgumentValueResolverInterfaceaddsconfigure(OptionsResolver $resolver)method. - A new subscriber
ControllerArgumentOptionsListenerlistens to theKernelEvents::CONTROLLERevent and looks for@Argmappings for the current controller callable. Then it iterates over theArgumentValueOptionsInterface[]instances to find a resolver based onsupports(Request $request, ArgumentMetadata $argument). Once the resolver is found,@Arg.optionsare resolved and saved to$request->attributes->set($name.'_options', $resolvedOptions). - Later in
SomeArgumentValueResolver::resolve(Request $request, ArgumentMetadata $argument)we receive resolved options by calling$request->attributes->get($name.'_options', [])and use them to yield the value.
While I think that this is a nice solution (especially when looking from the perspective of the FrameworkExtraBundle), it has one important drawback. @ParamConverter has a possibility to specify converter explicitly by leveraging the converter argument. It allows to resolve arguments which by themselves do not provide enough information to find a resolver. A great example is @ParamConverter("name", converter="fos_rest.request_body") from FOSRestBundle. This converter allows to deserialize $request->getContent() into a DTO based on the argument type hint. Unlike SomeEntityClass which we can check with ManagerRegistry::getManagerForClass(), SomeDTOClass does not give any hint that it should be processed by a RequestBodyParamConverter. So, the @Arg concept without a possibility to be explicit about the resolver seems to be not suitable for this case.
I have a different idea. It think, it fits the HttpKernel much better than the bundle, that's why I am creating an issue here.
Btw, the idea to move @ParamConverters configuration to the core was already discussed in sensiolabs/SensioFrameworkExtraBundle#436 (comment).
Design
-
Let's agree that once the argument is explicitly configured (for example by an annotation), we already know what resolver it should be processed by. It's like with validator constraints. So I propose to create the following interface:
interface ArgumentConfigInterface { /** * Returns the FQCN of the argument value resolver that resolves the argument with this config. */ public function resolvedBy(): string; }
-
I think that within the
HttpKernelcomponent this config might be a part of argument's metadata. Later it will allow us to cache everything together easily. Thus let's add another property to theArgumentMetadata:namespace Symfony\Component\HttpKernel\ControllerMetadata; class ArgumentMetadata { // ... private $config; public function __construct( string $name, ?string $type, bool $isVariadic, bool $hasDefaultValue, $defaultValue, bool $isNullable = false, ?ArgumentConfigInterface $config = null ) { // ... $this->config = $config; } // ... public function getConfig(): ?ArgumentConfigInterface { return $this->config; } }
-
Should we stick to the annotation mapping? No, let's create an extension point!
interface ArgumentConfigLoaderInterface { public function getArgumentConfig(callable $controller, string $name): ?ArgumentConfigInterface; } final class AnnotationArgumentConfigLoader implements ArgumentConfigLoaderInterface { public function __construct(Doctrine\Common\Annotations\Reader $reader) { // ... } }
Now we can inject
ArgumentConfigLoaderInterfaceintoArgumentMetadataFactoryand produce config awareArgumentMetadatainstances. -
Next, let's adjust
ArgumentResolver. Previously we retrieved converters only by iteration, now we want to get them by FQCN as well.namespace Symfony\Component\HttpKernel\Controller; final class ArgumentResolver implements ArgumentResolverInterface { // ... private $argumentValueResolversByClass; public function __construct( // ... ?ContainerInterface $argumentValueResolversByClass = null ) { // ... $this->argumentValueResolversByClass = $argumentValueResolversByClass ?: self::getDefaultArgumentValueResolversByClass(); } // ... private function getResolver(Request $request, ArgumentMetadata $metadata): ArgumentValueResolverInterface { if (null !== $config = $metadata->getConfig()) { return $this->argumentValueResolversByClass->get($config->resolvedBy()); } foreach ($this->argumentValueResolvers as $resolver) { if ($resolver->supports($request, $metadata)) { return $resolver; } } throw new \RuntimeException(); } }
-
I think we should also cache all argument metadata. That can be easily done with a decorator
CachingArgumentMetadataFactory implements ArgumentMetadataFactoryInterface.
After all these non-breaking BC steps we'll be able to create configs like @Entity, @BodyConverter, @GetParam with ease.
Benefits
- Custom configs for argument value resolvers.
- Faster argument value resolver resolution for configured arguments.
- One point to cache everything.
- Out-of-the-box improved
@ParamConverterswith no need to requiresensio/framework-extra-bundle.