Skip to content

[RFC][HttpKernel] Configurable argument value resolvers #29692

@vudaltsov

Description

@vudaltsov

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 ArgumentValueResolverInterface adds configure(OptionsResolver $resolver) method.
  • A new subscriber ControllerArgumentOptionsListener listens to the KernelEvents::CONTROLLER event and looks for @Arg mappings for the current controller callable. Then it iterates over the ArgumentValueOptionsInterface[] instances to find a resolver based on supports(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

  1. 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;
    }
  2. 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 the ArgumentMetadata:

    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;
        }
    }
  3. 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 into ArgumentMetadataFactory and produce config aware ArgumentMetadata instances.

  4. 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();
        }
    }
  5. 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 require sensio/framework-extra-bundle.

Ready to discuss!
ping @fabpot, @iltar.

Metadata

Metadata

Assignees

No one assigned

    Labels

    HttpKernelRFCRFC = Request For Comments (proposals about features that you want to be discussed)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions