auto-guessing & injecting resolver args

[akomm]

Q	A
Bug report?	no
Feature request?	yes
BC Break report?	no
RFC?	yes

Preamble

In following, there is a distinction between argument and parameter: Parameters are meant to be part of the signature definition and arguments are the values you pass when you call said method/function.

This is a proposal to change the way field resolve is handled in generated code. Initially I propose to make it BC with an additional option in type definition config. It might have potential to be a generalized solution later.

The change should be configuration-agnostic (yaml, annotation, ...). The purpose of the change is to enable features (or lay the path for some of them), like for example:

1. Removal of the need to pass resovler arguments via expression
   • auto-injection of resolver arguments (not to confuse with field arguments)
   • auto-hydrate field args
   • automated resolver id resolution based on contextual data
2. being able to alter contextual data before resolve and alter the result after resolve
3. being able to audit resolve(s) with contextual data

I see lot potential in this change, but I want to make it initially as small and simple as possible. Most things can later be discussed and added as features incrementally. Therefore the main focus is on:

1. Removal of the need to pass resovler arguments via expression
   • auto-injection of resolver arguments (not to confuse with field arguments)

Later it is possible to make this event based to enable basically any sort of behavior modification, like possible with the symfony kernel events (at least a good fraction of them). But it does not have to be events, its just one of many possible options. This however is not part of this RFC.

Flow of field resolution

1. FieldResolver delegation

Instead of calling resolver(s) directly and returning the result, there should be a generic FieldResolver service, which gets the resolution delegated:

Generated code, field resolver callback:

'resolve' => function ($value, $args, $context, ResolveInfo $info) use ($globalVariable) {
    return $globalVariable->get(FieldResolver::class)->resolve('DoResolver', $value, $args, $context, $info);
}

In order to generate the above code, a new field definition option resolver is required:

Mutation:
  type: 'object'
  config:
    fields:
      do:
        type: 'String!'
        resolver: 'DoResolver' # resolveR

note: If this option not set, the FieldResolver code is not generated/used.

The resolver option expects a resolver id which is then substituted into the generated code. In the above example the resolver id would be the string DoResolver.

When resolver and resolve option are both set at the same time, an exception should be thrown (compile time - config extension check).

This should also work with annotation, because the change is on type generation level, so at this point we only care about the processed config (php array), no matter the source/format. The processors of the different config formats however need to pass this option in some way, to utilize the feature.

2. FieldResolver::resolve - get the resolver

Field resolver fetches the actual resolver (via one of the resolver collection services ResolverResolver or MutationResolver).

When resolver is not found, fail with Unresolvable exception. Example:

$resolver = $resolvers->getSolution($resolverAlias);
$resolverOptions = $resolvers->getSolutionOptions($resolverAlias);
$resolverMethod = $resolverOptions['method'] ?? null;
$resolverCallable = [$resolver, $resolverMethod]; // will be called in last step
$resolverArgs = []; // will be populated in next steps

if (!$resolver) {
    throw new UnresolvableException(sprintf('Resolver "%s" not found', $resolverAlias));
}

3. FieldResolver::resolve - get resolver parameter metadata

Metadata needs to be collected for all resolver parameters. For the initial solution only the following information is required:

ParameterMetadata

• Param name: string
• Param type: string ("int", "string", "Fully\Qualified\Class\Name", ...)
• Param is primitive: boolean (e. G. type "int" = true, type "Fully\Qualified\ClassName" = false)

It might be useful to later add more, when we add features outside the scope of this RFC.

When metadata resolution fails, fail with Unresolvable exception.

It is important, that the ParameterMetadata is only computed once per resolver.

4. FieldResolver::resolve - resolve the resolver arguments

Using contextual data (info, args, ...), passed to the FieldResolver and the retrieved parameter metadata, the resolver arguments needs to be resolved.

// @var ParameterMetadata $resolverParam
foreach ($resolverParams as $resolverParam) {
    // $parentValue, $fieldArgs, ... can be wrapped in a type, e. G. `ContextualData` ?
    $resolverArgs[] = resolveArg($resolverParam, $parentValue, $fieldArgs, $info);
}

There might be different strategies how to resolve resolver arguments. It is possible, that two different strategies could resolve an argument, but the value is different. For example same type, but two different instances. Therefore there needs to be a strategy how to resolve ambiguities. I propose to solve this ambiguities by a precedence concept. We have a set of strategies, which is ordered (priority). First strategy to resolve wins. The user defines, which strategy comes first, which next and last. User can add or remove strategy. Using a variable set of strategies and ordering, plus strategies being able to have options, a logical solution can be created, that allows predictable argument resolution.

Initially however, I propose to make a fixed set of checks, representing strategies and later discuss how the mechanics for configuring it can work (for example tagged services).

Example how the above resolveArg method might look with a fixed solution, all those checks being later extracted as strategy services:

private function resolveArg(
    ParameterMetadata $resolverParam,
    $parentValue,
    ArgumentInterface $fieldArgs,
    ResolveInfo $info
) {
    if (ResolveInfo::class === $resolverParam->type()) {
        return $info;
    }
    if (is_a($resolverParam->type(), ArgumentInterface::class, true)) {
        return $fieldArgs;
    }
    if (is_object($parentValue) && !$resolverParam->isPrimitive() && is_a($parentValue, $resolverParam->type())) {
        return $parentValue;
    }
    if ('parentValue' === $resolverParam->name()) {
        return $parentValue;
    }

    $fieldArg = $fieldArgs[$resolverParam->name()] ?? null;

    if (null !== $fieldArg) {
        // this strategy could also hydrate later, currently not part of RFC, just so you get an idea
        // if (!$resolverParam->isPrimitive() && is_array($fieldArg)) { 
        //     $fieldArg = hydrate(...);
        // }
        return $fieldArg;
    }
    
    // given we inject $globalVariable (later, with extraction of strategy services not needed)
    if (!$resolverParameter->isPrimitive() && $globalVariable->get('container')->has($resolverParameter->getType()) {
        return $globalVariable->get('container')->has($resolverParameter->getType());
    }

    return null;
}

When no strategy could resolve a value, the argument is resolved as null. When parameter is nullable, the argument passes, otherwise a type error (\Throwable) is thrown. The nullable behavior mimics the that of DI using symfony/container. When you have optional dependency and its nullable, in case it can not be resolved, your service is constructed with the dependency being null.

5. FieldResolver::resolve call the resolver with resolved arguments

Now the actual resolver call must happen:

return \call_user_func_array($resolverCallable, $resolverArgs);

This is the place, where later you could add events (or single service user can define, if events are not desired), before resolver call and after: Example:

$events->trigger('before_resolve', $eventWithContextEtc);
$result = \call_user_func_array($resolverCallable, $resolverArgs);
$events->trigger('after_resolve', $eventWithResultEtc);
return $result;

Potentially, the whole arg resolve logic described above could go into the 'before_rsolve' event, the $event object provinding an API to alter resolver $resolverArgs. This is however not intended in first step, as you can see this is another matter to discuss. But I think its important to highlight the posibilities it opens later.

addition 1 Illustrating the possibility to also inject services in 4. FieldResolver::resolve - resolve the resolver arguments addition 2 Added caching requirement to 3. FieldResolver::resolve - get resolver parameter metadata addition 3 FieldResolver code generation conditions clarified.

[akomm]

@murtukov I think on top of this some of your solutions could build? You had some really good ideas, but I think some base work is simply missing before it? :) For example with the hydrate field args strategy, we could enable adding a HydratorInterface and you could use a strict one with type configuration, and I could use a soft one ;P

[murtukov]

@akomm What concerns me about this implementation is the fact, that all these calculations happen in runtime by every request to every resolver. This would hit the performance. Do you have a solution to this?

[akomm]

@murtukov You can't write code without calculation. The checks are fast. The only possible bottleneck is ParameterMetadata and that only when you have a field with resolver, which is in a listOf context with many items. In this case ParameterMetadata can be cached, so that for every resolver the code is run only once. Even reflection is fast, if not done hundreds and thousands of times.

In case you use it on a mutation field, its totally meaningless microseconds difference per request (run only once).

Of performance issue we can speak here only in query context.

[murtukov]

You can't write code without calculation.

I can write no code at all and thus avoid any unnecessary calculations.

Of performance issue we can speak here only in query context.

Query is a most frequent type of requests.

---

The bundle works without any problems now and you suggest to add a feature, which would slow down all queries and for what reason? Just so that users don't type a couple of variables in the configs? If I create a GraphQL type I am probably not going to change them in future, so why do I need these unnecessary calculations if I dont change configs? It's totally meaningless. Doesn't matter how fast these calculations are, it's definitely a wrong direction to develop this bundle.

This feature can only be accepted, if we implement a caching system to cache the results of those calculations.

[murtukov]

On all other points (except HydratorInterface) I agree with you. I like the possibility to use hooks.

[akomm]

You can't write code without calculation.

I can write no code at all and thus avoid any unnecessary calculations.

I think you missed the point. If you want to write fast code, don't use symfony or any library. Because all those calculation done there... oh boy... are unnecessary by your definition. They cover all sort of possible cases and "calculate" even though in specific context you never have all those cases. You could safe thousands of unnecessary calculations writing the code fit for the exact case yourself.

Of performance issue we can speak here only in query context.

Query is a most frequent type of requests.

I think I pointed out the solution, so whats the point ripping the note & answer from the answer and quoting the note by itself?

The bundle works without any problems now and you suggest to add a feature, which would slow down all queries and for what reason? Just so that users don't type a couple of variables in the configs?

Most of framework and library related features are there to reduce the typing and repeating of configuration and code. In this case it reduces redundancy between your expression and your resolver API. Also it allows with hydration to make your resolver correctly typed without writing the logic into the expression and without having only partially typed solutions like HydratedArguments discussed previously, in which the inner fields are not properly typed. For me its important part of having clean and proper written code. And this feature in my opinion would greatly increase the code quality and ease to understand it in team. You can focus on actual implementation and not navigation through multiple files to make 1 change.

Also: It does not slow down all queries. How you came to this conclusion? If you do not have resolver it does not even change anything.

If I create a GraphQL type I am probably not going to change them in future, so why do I need these unnecessary calculations if I dont change configs? It's totally meaningless.

But you do not reflect how everyone else works or how often types are changed or refactored.

This feature can only be accepted, if we implement a caching system to cache results of those calculations.

Didn't I actually answer this already? Metadata cache :) Also do make sure its clear: the code is not run for any feeld that does not have resolver option set.

[akomm]

@Vincz with annotation, you currently call resolver directly via serv, so it will not be found via resolver collections. I assume you do not want to have to register your resolver via tags explicitly (which would then work out of the box with this).

However I think it is solvable issue - I can also try fetch the resolver from the container in FieldResolver, if you pass the FQCN down as the new resolver option.

[murtukov]

@akomm

This feature can only be accepted, if we implement a caching system to cache results of those calculations.

Didn't I actually answer this already? Metadata cache :)

Editing your message, while I am typing, doesn't mean answering. You added the part about caching later.

---

I think you missed the point. If you want to write fast code, don't use symfony or any library.

I use Symfony, because it makes things I NEED efficient. Your original proposal didn't mention anything about caching and you presented code examples with an unefficient code.

Most of framework and library related features are there to reduce the typing and repeating of configuration and code. In this case it reduces redundancy between your expression and your resolver API. Also it allows with hydration to make your resolver correctly typed without writing the logic into the expression and without having only partially typed solutions like HydratedArguments discussed previously, in which the inner fields are not properly typed. For me its important part of having clean and proper written code. And this feature in my opinion would greatly increase the code quality and ease to understand it in team. You can focus on actual implementation and not navigation through multiple files to make 1 change.

I don't even know why you are explaining me the purpose of frameworks and libraries. Did I say I don't want this feature? No. Did I say it's a useless feature? No. I only told you, that you propose an unefficient solution and I only want to add a caching (which you mentioned later by editing your comment).

Now I simple example: Suppose I've developed a project and published it. I have thousands of visitors every day, who make hundreds of requests. All requests would trigger arguments calculations, which are obviously unnecessary, since I don't chage the code for months. This is an unefficient code.

If we both agree that caching is needed, then there is nothing to discuss.

[akomm]

Editing your message, while I am typing, doesn't mean answering. You added the part about caching later.

That is not true. Simply check version history of the comment, and you find the first thing was ParameterMetadata cache...

I use Symfony, because it makes things I NEED efficient.

What do you mean by makes things I NEED efficient.

Your original proposal didn't mention anything about caching and you presented code examples with an unefficient code.

I did not mentioned it. You asked for solution, I answered "caching". Where is the problem? You still came with "inefficient" and "pointless", hence I again replied. I see not where the problem is.

I also assumed its clear, that the code is not the implementation, otherwise I would have made directly a PR?

I don't even know why you are explaining me the purpose of frameworks and libraries. Did I say I don't want this feature? No. Did I say it's a useless feature? No. I only told you, that you propose an unefficient solution and I only want to add a caching (which you mentioned later by editing your comment).

Again, that is not true with the editing... Otherwise I would have answered differently when I added it later... Why do you not check before you write this? Its dead simple to find out...

Also you basically said the feature is pointless, so yes, you did it .... O.o One example:

The bundle works without any problems now and you suggest to add a feature, which would slow down all queries and for what reason? Just so that users don't type a couple of variables in the configs?

Hence I argued my viewpoint, why I disagree?

We should focus on the topic, so your point was correct. The ParameterMetadata need to be computed only once per resolver. Was a good point of you.

[murtukov]

@akomm I am not going to continue this pointless discussion.

[akomm]

@akomm What concerns me about this implementation is the fact, that all these calculations happen in runtime by every request to every resolver.

I answere regarding the metadata cache already, but now I also realize that the first is also incorrect. Because the code is, as explained in the RFC itself only generated (and therefore run) when you use the new resolver option on your field, quote:

In order to generate the above code, a new field definition option resolver is required

Maybe I need to clarify this in the RFC better. Thanks!

[zimzat]

I, personally, like the idea of having dependency injection at the resolver level. I think it makes a lot of sense to minimize the amount of "load everything!" that would otherwise happen. Handling it there also means you don't have to hard-wire it in annotations, and makes it available to a Resolver Map implementation.

I'm trying to kickstart a personal project and was hoping this Bundle would help glue everything together, but I'm having a lot of trouble understanding how to work with dependency injection and the Resolver Map.

At my current job the application is using webonyx/graphql-php directly but it has taken some work to build up the resolver, file upload, batching, etc support. That project is using Symfony's Service Subscriber and public method Reflection to determine all of the services to inject, while the resolver has a method that looks very similar to @akomm's resolveArg method for anything not using the default resolver. It's been working great for us there since it works similarly to how Symfony's Controller Action injection works (I think of each resolver as the equivalent of a Controller Action, and expect to have the same level of 'Just In Time' Dependency Injection available).

What would we need to do to explore moving forward with this functionality?