overblog / GraphQLBundle

This bundle provides tools to build a complete GraphQL API server in your Symfony App.
MIT License
782 stars 221 forks source link

Add the ability to inject the ResolveInfo in methods args #514

Open Vincz opened 5 years ago

Vincz commented 5 years ago
Q A
Bug report? no
Feature request? yes
BC Break report? no

It would be great to be able to get the ResolveInfo in methods with auto-guessed arguments. For example:

/**
 * @GQL\Query
 */
public function query(ResolveInfo $info, string $fu = 'bar'): string
{
....
}

Wondering also if we should allow services injection. But it would mean that the args could be either special (like ResolveInfo), a service or regular GraphQL argument. Maybe would it be too much?

mcg-web commented 5 years ago

This is the way to go also for regular resolver, allowing the injection base on the type declaration or the args name. But this a long run so maybe we should start by accepting the graphql resolver base arguments ( value, info, context, rootValue).

Vincz commented 5 years ago

I'm not sure what you mean by regular resolver. When the arguments list is explicit, we can already use the value, info, args, content with the expression language to inject whatever we want, no? Even services with the serv expression. With annotations, it makes sense as, it's from PHP to GraphQL (we generate the GraphQL from the PHP), but without, we generate GraphQL first and we are targeting PHP callable as resolver. Maybe we could find another way to declare resolvers with auto-guessed arguments but without annotation, I think it would be pointless as sometimes PHP is not enough to generate the GraphQL (like when we want to accept a multiple argument, we cannot detect it based on PHP argument as it's not possible to type-hint a PHP argument as string[] for example).

For the auto-guessed arguments with annotations, I made it work by checking the class of the argument for the ResolveInfo class, if I detect it, I inject in argument place the resolve info object. But I'm not sure about the others arguments as the only way would be based on the variable name. Not sure how we could limit collision with real GraphQL params (the GraphQL arguments names and PHP variables have the exact same form /[_A-Za-z][_0-9A-Za-z]*/). Any idea about this ?

mcg-web commented 5 years ago

Hi Vincz sorry for the late reply I'm working on a service that will ease the injection of contextual resolver args. The new service will allow getting current resolver args (value, args, context, info) without having to inject each of them. Maybe this new feature could ease this implementation ?

murtukov commented 5 years ago

@mcg-web Could you provide an example of the config and how it would be used in a resolver?

Sh1d0w commented 5 years ago

Yeah, I am also struggling with this. Auto guessing does not work and I don't know how exactly to explicitly specify the parameters, as it is not very well described in the docs. Tried with resolve="@=call(.....)" but had some other errors also it seems very verbose.

An example how to do it properly until auto guessing is implemented will be great.

murtukov commented 5 years ago

@Sh1d0w you should omit the @ symbol. It's stated in the documentation

Sh1d0w commented 5 years ago

@murtukov Sorry if I have misread the documentation.

I was referring to the examples given here and here.

However I am still struggling to inject ResolveInfo via explicit declarations in annotations, can you help me make this work:

   /**
     * @GQL\Query(
     *     type="Website",
     *     name="website",
     *     args={
     *          @GQL\Arg(name="hostname", type="String!", description="The website hostname.")
     *     },
     *     resolve="call(website, [args['hostname'], info])"
     * )
     */
     public function website(string $hostname, ResolveInfo $info): Website

The above throws an exception:

Attempted to load class \"QueryType\" from namespace \"Overblog\\GraphQLBundle\\__DEFINITIONS__\".\nDid you forget a \"use\" statement for another namespace?

If I remove ResolveInfo argument and the resolve entry in annotations everything works. But I need to access the ResolveInfo data.

Vincz commented 5 years ago

@Sh1d0w It should work. It looks like a cache problem. Can you clear and warmup it?

murtukov commented 5 years ago

@Vincz @mcg-web As i mentioned in another issue (in this message), we could add the ability to auto guess the resolver arguments.

Here is a yaml example:

Mutation:
    type: object
    config:
        fields:
            createPost:
                type: Post
                resolve: App\GraphQL\Mutation\PostMutation::create
                args:
# ...

Basically we only point to a method. Now you might be asking yourself this questions:

All other arguments could be distinguished by the class/interface name ($args, $info, $context).

It's also possible to autowire services directly into resolver by type hints. It can be done relatively easy. When I was working on the validation feature I've created a methods in the Generator class to inject any extra code into resolvers. We can use it for service autowiring and for all future features.

I don't know how it would be done with annotations. I need first to read all documentation about it and get familiar with its logic.

akomm commented 5 years ago

@mcg-web since the graphql-php library does not have any hook/event to do it (at least I did not find), I assume you are going to do it with a bundle resolver-wrapper, that calls the service and the args of the user resolver which are resolved via reflection? If so, I would like to know because I am working on something similar, I mentioned in Reconsider ArgumentsTransformer post, maybe we can exchange the ideas. Or do you do something entirely different?

akomm commented 5 years ago

I'm not sure what you mean by regular resolver. When the arguments list is explicit, we can already use the value, info, args, content with the expression language to inject whatever we want, no? I think the idea is, that you do need less or no expression at all, which is probably preferred. Expression is actually only an escape hatch due to difficulties making an abstract solution that matches all needs.

Even services with the serv expression. With annotations, it makes sense as, it's from PHP to GraphQL (we generate the GraphQL from the PHP), but without, we generate GraphQL first and we are targeting PHP callable as resolver.

I think whether the one way or the other way is better depends on the situation.

  1. If you have a class, that acts as a resolver with the main purpose of being a part of your (graphql) API implementation, it is better, if you could have the config inferred from the class or at least provided with it (like annotations do), instead of pointing to it from the config.
  2. If you have a graphql type which is resolved from a value of your app's domain, you (probably) do not want to have graphql configured on something in your app's domain, instead configure your API pointing to the domain and also defining the "how to map to it" logic there, without touching the domain.

Maybe we could find another way to declare resolvers with auto-guessed arguments but without annotation, I think it would be pointless as sometimes PHP is not enough to generate the GraphQL (like when we want to accept a multiple argument, we cannot detect it based on PHP argument as it's not possible to type-hint a PHP argument as string[] for example).

I thought of exact this. There are a few different way doing this, not only via name, but for example also 1. via ordering, 2. interfaces that provide hints of ordering and source (value, info, input). Each of them with advantages and disavantages. But the current solution has also disadvantages, its hard to find a perfect solution. We should rather ask will it be better? In my opinion if this is optional, it will. For example you can work with named method and if you have the rare chance to have name collision you can pick one of the alternatives for this one or two resolver. Then in majority it works out of the box and for edge cases you give the system a hint. For the auto-guessed arguments with annotations, I made it work by checking the class of the argument for the ResolveInfo class, if I detect it, I inject in argument place the resolve info object. But I'm not sure about the others arguments as the only way would be based on the variable name. Not sure how we could limit collision with real GraphQL params (the GraphQL arguments names and PHP variables have the exact same form /[_A-Za-z][_0-9A-Za-z]*/). Any idea about this ?

For non-Arguments parameters on the resolve its one of solution. You could do similar with the value, however in both cases a weird edge would be if you for whatever reason have same php type for value or resolveinfo as for some input argument. While in regrards to input type it is impossible from graphql type side, as input-object and object are distinct, but we speak of php types. I can not imagine this to be desired by anyone, but even if, its solvable via the alternative solutions I mentioned.

akomm commented 5 years ago

Example solution for name collission:

interface ResolverParameterMapInterface 
{
    const VALUE = 'value';
    const INPUT = 'input';
    const INFO = 'info';

    public static function getResolverParameterMap(): array;
}

Example solution for name collission:

class MyResolver implements ResolverParameterMapInterface
{
    public function fieldA($value, $info, $filters, $options)
    {
    }

    public function fieldB($value, $filters)
    {
    }

    public static function getResolverParameterMap(): array
    {
        return [
            // this is only needed in edge-cases
            // provides order and source, rest fetched via arg-name in resolver signature
            // name collision only possible between VALUE|INFO & INPUT, but not between INPUT's
            'fieldA' => [self::VALUE, self::INFO, self::INPUT, self::INPUT],
            'fieldB' => [self::VALUE, self::INPUT],
        ];
    }
}

The solution for incompatibility in the byte-ranges valid for names in graphql vs. php could be the same as this, just as a name map getResolverParameterNameMap or same interface, with syntax enhancement source => fieldName, e. G. self::VALUE => 'resolver param name'. And as said, it should be only needed in edge-cases.

murtukov commented 5 years ago

@mcg-web since the graphql-php library does not have any hook/event to do it (at least I did not find), I assume you are going to do it with a bundle resolver-wrapper, that calls the service and the args of the user resolver which are resolved via reflection? If so, I would like to know because I am working on something similar, I mentioned in Reconsider ArgumentsTransformer post, maybe we can exchange the ideas. Or do you do something entirely different?

Hi @akomm,

I didn't read all new messages yet, but I want to say one thing immediately:

No, webonyx doesn't provide any hooks, so right now the only place to get ResolverInfo $info (or any other resolver argument) is resolver closures in type definition classes. For example:

// ...
'resolve' => function ($value, $args, $context, ResolveInfo $info) use ($globalVariable) {
    // here you can add any extra code, for example get a service:
    $manager = $globalVariable->get('container')->get('App\Manager\User');

    // ... or analyse the target resolver's type-hints with ReflectionFunction

    return $globalVariable->get(/* resolver name */)->resolve([/* resolver arguments */]);
},
// ...

I have already mentioned in this PR, that I use the method generateExtraCode, to inject an extra code into resolver closure (validator in that case).

So if you are going to put some extra code, keep in mind that method.

murtukov commented 5 years ago

@akomm Btw I am currently working on the Hydrator. I will open an issue to share my views on its implementation (some points are changed since our last discussion)

akomm commented 5 years ago

@akomm Btw I am currently working on the Hydrator. I will open an issue to share my views on its implementation (some points are changed since our last discussion)

Also mentioned it before your PR in other issue here. For me its a sign, that its a common issue. Maybe worth a shot doing a PR on the lib so you can provide a callback to the executor/facade they provide, so bundles like this can then call such a callback to trigger an event.

murtukov commented 5 years ago

@akomm I am not familiar with the webonyx well enough, to open a PR for that lib. What about you?

akomm commented 5 years ago

@murtukov this is what I am talking about. I just have to many construction zones. So I can not tell when I can do it. Before I got time to get my hands on the field(s) builder type emitting addition, it took very long just as an example.

akomm commented 5 years ago

Not sure how we could limit collision with real GraphQL params (the GraphQL arguments names and PHP variables have the exact same form /[_A-Za-z][_0-9A-Za-z]*/). Any idea about this ?

It happens to be I once had to figure this out when I wrote some PR for Zend\Config

PHP allows almost any name, including null-bytes. In special semantic context, like you normally use in code its however limited to: [a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]* source.

I verified the validity of this statement. Also note it does not treat namespaces, but we do not care about namespaces here. GraphQL's spec for names is a subset of PHP's spec, so mapping from GraphQL to PHP should be safe. PHP adds some ranges outside ASCII on top of it, so the other way around is not safe.

Makapashev commented 2 years ago

Here is my solution. This way you can add ResolveInfo as an argument to query function.

<?php

declare(strict_types=1);

namespace App\Resolver;

use App\Application\UseCase\GoodByIdHandler;
use GraphQL\Type\Definition\ResolveInfo;
use Overblog\GraphQLBundle\Annotation as GQL;
use Overblog\GraphQLBundle\Definition\Resolver\QueryInterface;

#[GQL\Provider]
final class GoodResolver implements QueryInterface
{
    private GoodByIdHandler $handler;

    public function __construct(GoodByIdHandler $handler)
    {
        $this->handler = $handler;
    }

    #[GQL\Query(name: "getGoodById", type: 'Good', resolve: "query('App\\\Resolver\\\GoodResolver::getGoodById', args[\"id\"], info)")]
    #[GQL\Arg(name: 'id', type: 'Int!')]
    #[GQL\Description('Получение информации для товара по Id')]
    public function getGoodById(int $id, ResolveInfo $info)
    {
        $requestedFields = $info->getFieldSelection();
        return $this->handler->handle($id, $requestedFields);
    }
}