-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Description
Hi everyone!
Recently got interested in @ParamConverter
s 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 ArgumentValueResolverInterface
addsconfigure(OptionsResolver $resolver)
method. - A new subscriber
ControllerArgumentOptionsListener
listens to theKernelEvents::CONTROLLER
event and looks for@Arg
mappings 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.options
are 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
HttpKernel
component 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
ArgumentConfigLoaderInterface
intoArgumentMetadataFactory
and produce config awareArgumentMetadata
instances. -
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
@ParamConverters
with no need to requiresensio/framework-extra-bundle
.