Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Container inlining issue #1103

Open
l-you opened this issue Mar 7, 2023 · 4 comments
Open

Container inlining issue #1103

l-you opened this issue Mar 7, 2023 · 4 comments

Comments

@l-you
Copy link

l-you commented Mar 7, 2023

Consider following example

use App\GraphQL\Service\RequestService;
use Overblog\GraphQLBundle\Annotation\Mutation;
use Overblog\GraphQLBundle\Annotation\Provider;
use Overblog\GraphQLBundle\Annotation\Query;
use Overblog\GraphQLBundle\Definition\Resolver\QueryInterface;

#[Provider]
class TestProvider implements QueryInterface
{
    public function __construct(private readonly RequestService $requestService)
    {
    }

    #[Query(name: 'ssss')]
    public function getSomething(int $amount, bool $yes = true): string
    {
        return 'Hello world!'.$amount.' '.($yes ? '+' : '-').$this->requestService->getCurrentClientIp();
    }

    #[Mutation(name: 'ssm')]
    public function doSom(int $amount, bool $yes = true): string
    {
        return 'Hello mut!'.$amount.' '.($yes ? '+' : '-');
    }
}
overblog_graphql:
  security:
    enable_introspection: true
  definitions:
    schema:
      query: RootQuery
      mutation: RootMutation
      # the name of extra types that can not be detected
      # by graphql-php during static schema analysis.
      # These types names should be explicitly declare here
      types: [ ]
    mappings:
      types:
        - type: attribute
          dir: "%kernel.project_dir%/src/GraphQL"
          suffix: ~

Issue is I'm getting error that says The "App\GraphQL\TestProvider" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.

Resolver in RootQueryType in cache directory looks like following:

'resolve' => function ($value, $args, $context, $info) use ($services) {
        return $services->get('container')->get("App\\GraphQL\\TestProvider")->getSomething(...$services->get('container')->get('overblog_graphql.arguments_transformer')->getArguments(["amount" => "Int!", "yes" => "Boolean"], $args, $info));
    },

RootQuery looks like this.

declare(strict_types=1);

namespace App\GraphQL\Types;

use Overblog\GraphQLBundle\Annotation\Type;

#[Type]
final class RootQuery
{
    public function __construct()
    {
    }
}

Any suggestion how to make it work?

@l-you l-you changed the title No example of auto wiring in attributed classes Container inlining issue Mar 7, 2023
@Vincz
Copy link
Collaborator

Vincz commented Mar 19, 2023

Hi @rusted-love
Sorry for the late reply. The problem is that App\GraphQL\TestProvider must be a public registered service in the container.
So, in your services.yaml file, you should have something like this:

App\GraphQL\TestProvider:
    public: true

Also, your providers don't need to extend QueryInterface.

@l-you
Copy link
Author

l-you commented Mar 19, 2023

But why library does not provide some kind if compiled container?
With Symfony router I do not need to make services public.

@Vincz
Copy link
Collaborator

Vincz commented Mar 19, 2023

I guess we could. I'll look into it when I'll have a bit of time.

@mathroc
Copy link
Contributor

mathroc commented Dec 29, 2023

Hi! I had a quick look at this, in a very simple playground project, I have this generated type:

<?php

namespace GraphQL\AutoGeneratedDefinitions;

use GraphQL\Type\Definition\ObjectType;
use GraphQL\Type\Definition\Type;
use Overblog\GraphQLBundle\Definition\ConfigProcessor;
use Overblog\GraphQLBundle\Definition\GraphQLServices;
use Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface;
use Overblog\GraphQLBundle\Definition\Type\GeneratedTypeInterface;

/**
 * THIS FILE WAS GENERATED AND SHOULD NOT BE EDITED MANUALLY.
 */
final class QueryType extends ObjectType implements GeneratedTypeInterface, AliasedInterface
{
    public const NAME = 'Query';
    
    public function __construct(ConfigProcessor $configProcessor, GraphQLServices $services)
    {
        $config = [
            'name' => self::NAME,
            'fields' => fn() => [
                'hello' => [
                    'type' => Type::nonNull(Type::string()),
                    'resolve' => function ($value, $args, $context, $info) use ($services) {
                        return $services->get('container')->get("App\\Query\\Hello")->hello(...$services->get('container')->get('overblog_graphql.arguments_transformer')->getArguments(["name" => "String"], $args, $info));
                    },
                    'args' => [
                        [
                            'name' => 'name',
                            'type' => Type::string(),
                            'defaultValue' => 'World',
                        ],
                    ],
                ],
            ],
        ];
        
        parent::__construct($configProcessor->process($config));
    }
    
    /**
     * {@inheritdoc}
     */
    public static function getAliases(): array
    {
        return [self::NAME];
    }
}

I think the quickest way to solve this issue would be to register the GraphQL services into the GraphQLServices locator, so we can go from $services->get('container')->get("App\\Query\\Hello") to $services->get("App\\Query\\Hello")

But maybe we can go one step further and generate something like this instead:

<?php

namespace GraphQL\AutoGeneratedDefinitions;

use GraphQL\Type\Definition\ObjectType;
use GraphQL\Type\Definition\Type;
use Overblog\GraphQLBundle\Definition\ConfigProcessor;
use Overblog\GraphQLBundle\Definition\GraphQLServices;
use Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface;
use Overblog\GraphQLBundle\Definition\Type\GeneratedTypeInterface;

/**
 * THIS FILE WAS GENERATED AND SHOULD NOT BE EDITED MANUALLY.
 */
final class QueryType extends ObjectType implements GeneratedTypeInterface, AliasedInterface
{
    public const NAME = 'Query';
    
    public function __construct(
        ConfigProcessor $configProcessor, 
        App\Query\Hello $service0,
        ArgumentsTransformer $argumentTransformer,
    ) {
        $config = [
            'name' => self::NAME,
            'fields' => fn() => [
                'hello' => [
                    'type' => Type::nonNull(Type::string()),
                    'resolve' => static fn ($value, $args, $context, $info)
                        => $service0->hello(...$argumentTransformer->getArguments(["name" => "String"], $args, $info)),
                    'args' => [
                        [
                            'name' => 'name',
                            'type' => Type::string(),
                            'defaultValue' => 'World',
                        ],
                    ],
                ],
            ],
        ];
        
        parent::__construct($configProcessor->process($config));
    }
    
    /**
     * {@inheritdoc}
     */
    public static function getAliases(): array
    {
        return [self::NAME];
    }
}

It should let the SymfonyContainer do more optimization at compile time and improve performances a bit.

wdyt?

Depending on how much work this represents, I can take a look at it in the next weeks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants