From ed0a3dc4ec27937c350d8fd4925d578ece42bc03 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 1 Mar 2018 08:01:49 +0100 Subject: [PATCH 1/3] Fix FluentResolver when tagging two services with same class This bug was introduce because we was relying on class name to create FluentResolver id. We now relying on the service id as it is unique. --- Command/DebugCommand.php | 26 ++-------- .../Compiler/TaggedServiceMappingPass.php | 5 +- Relay/Mutation/MutationFieldDefinition.php | 4 +- Relay/Node/GlobalIdFieldDefinition.php | 4 +- Relay/Node/NodeFieldDefinition.php | 4 +- .../PluralIdentifyingRootFieldDefinition.php | 5 +- Resources/doc/definitions/resolver.md | 11 ++-- .../doc/definitions/type-system/index.md | 4 +- .../App/GraphQL/HelloWord/Type/QueryType.php | 3 +- .../App/config/autoMapping/config.yml | 4 +- Tests/Functional/App/config/node/config.yml | 2 +- .../App/config/node/mapping/node_type.yml | 2 +- Tests/Functional/App/config/plural/config.yml | 2 +- .../App/config/plural/mapping/Query.types.xml | 2 +- Tests/Functional/Command/DebugCommandTest.php | 20 ++------ .../Command/fixtures/debug/debug-all.txt | 50 ------------------- .../Command/fixtures/debug/debug-mutation.txt | 12 ++--- .../Command/fixtures/debug/debug-resolver.txt | 16 +++--- .../Command/fixtures/debug/debug-type.txt | 10 ++-- Tests/Relay/Node/NodeFieldDefinitionTest.php | 3 +- ...uralIdentifyingRootFieldDefinitionTest.php | 3 +- UPGRADE-0.11.md | 22 ++++++++ 22 files changed, 72 insertions(+), 142 deletions(-) delete mode 100644 Tests/Functional/Command/fixtures/debug/debug-all.txt diff --git a/Command/DebugCommand.php b/Command/DebugCommand.php index 2e84016ba..d379a3bd5 100644 --- a/Command/DebugCommand.php +++ b/Command/DebugCommand.php @@ -53,12 +53,6 @@ protected function configure() InputOption::VALUE_IS_ARRAY | InputOption::VALUE_OPTIONAL, sprintf('filter by a category (%s).', implode(', ', self::$categories)) ) - ->addOption( - 'with-service-id', - null, - InputOption::VALUE_NONE, - 'also display service id' - ) ->setDescription('Display current GraphQL services (types, resolvers and mutations)'); } @@ -72,20 +66,16 @@ protected function execute(InputInterface $input, OutputInterface $output) } $categories = empty($categoriesOption) ? self::$categories : $categoriesOption; - $withServiceId = $input->getOption('with-service-id'); $io = new SymfonyStyle($input, $output); $tableHeaders = ['solution id', 'aliases']; - if ($withServiceId) { - $tableHeaders[] = 'service id'; - } foreach ($categories as $category) { $io->title(sprintf('GraphQL %ss Services', ucfirst($category))); /** @var FluentResolverInterface $resolver */ $resolver = $this->{$category.'Resolver'}; - $this->renderTable($resolver, $tableHeaders, $io, $withServiceId); + $this->renderTable($resolver, $tableHeaders, $io); } } @@ -93,32 +83,24 @@ protected function execute(InputInterface $input, OutputInterface $output) * @param FluentResolverInterface $resolver * @param array $tableHeaders * @param SymfonyStyle $io - * @param bool $withServiceId */ - private function renderTable(FluentResolverInterface $resolver, array $tableHeaders, SymfonyStyle $io, $withServiceId) + private function renderTable(FluentResolverInterface $resolver, array $tableHeaders, SymfonyStyle $io) { $tableRows = []; $solutionIDs = array_keys($resolver->getSolutions()); sort($solutionIDs); foreach ($solutionIDs as $solutionID) { $aliases = $resolver->getSolutionAliases($solutionID); - $options = $resolver->getSolutionOptions($solutionID); - $tableRows[$solutionID] = [$solutionID, self::serializeAliases($aliases, $options)]; - if ($withServiceId) { - $tableRows[$solutionID][] = $options['id']; - } + $tableRows[$solutionID] = [$solutionID, self::serializeAliases($aliases)]; } ksort($tableRows); $io->table($tableHeaders, $tableRows); $io->write("\n\n"); } - private static function serializeAliases(array $aliases, array $options) + private static function serializeAliases(array $aliases) { ksort($aliases); - $aliases = array_map(function ($alias) use ($options) { - return $alias.(isset($options['method']) ? ' (method: '.$options['method'].')' : ''); - }, $aliases); return implode("\n", $aliases); } diff --git a/DependencyInjection/Compiler/TaggedServiceMappingPass.php b/DependencyInjection/Compiler/TaggedServiceMappingPass.php index f2fd5e3c4..3f88e25a2 100644 --- a/DependencyInjection/Compiler/TaggedServiceMappingPass.php +++ b/DependencyInjection/Compiler/TaggedServiceMappingPass.php @@ -18,14 +18,13 @@ private function getTaggedServiceMapping(ContainerBuilder $container, $tagName) $isType = TypeTaggedServiceMappingPass::TAG_NAME === $tagName; foreach ($taggedServices as $id => $tags) { - $className = $container->findDefinition($id)->getClass(); foreach ($tags as $attributes) { $this->checkRequirements($id, $attributes); $attributes = self::resolveAttributes($attributes, $id, !$isType); - $solutionID = $className; + $solutionID = $id; if (!$isType && '__invoke' !== $attributes['method']) { - $solutionID = sprintf('%s::%s', $className, $attributes['method']); + $solutionID = sprintf('%s::%s', $id, $attributes['method']); } if (!isset($serviceMapping[$solutionID])) { diff --git a/Relay/Mutation/MutationFieldDefinition.php b/Relay/Mutation/MutationFieldDefinition.php index 0ac4bf192..d01f83e6b 100644 --- a/Relay/Mutation/MutationFieldDefinition.php +++ b/Relay/Mutation/MutationFieldDefinition.php @@ -3,7 +3,6 @@ namespace Overblog\GraphQLBundle\Relay\Mutation; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; -use Overblog\GraphQLBundle\GraphQL\Relay\Mutation\MutationFieldResolver; final class MutationFieldDefinition implements MappingInterface { @@ -16,14 +15,13 @@ public function toMappingDefinition(array $config) $mutateAndGetPayload = $this->cleanMutateAndGetPayload($config['mutateAndGetPayload']); $payloadType = isset($config['payloadType']) && is_string($config['payloadType']) ? $config['payloadType'] : null; $inputType = isset($config['inputType']) && is_string($config['inputType']) ? $config['inputType'].'!' : null; - $resolver = addslashes(MutationFieldResolver::class); return [ 'type' => $payloadType, 'args' => [ 'input' => ['type' => $inputType], ], - 'resolve' => "@=resolver('$resolver', [args, context, info, mutateAndGetPayloadCallback($mutateAndGetPayload)])", + 'resolve' => "@=resolver('relay_mutation_field', [args, context, info, mutateAndGetPayloadCallback($mutateAndGetPayload)])", ]; } diff --git a/Relay/Node/GlobalIdFieldDefinition.php b/Relay/Node/GlobalIdFieldDefinition.php index fe5ee5470..95d2bb5bc 100644 --- a/Relay/Node/GlobalIdFieldDefinition.php +++ b/Relay/Node/GlobalIdFieldDefinition.php @@ -3,7 +3,6 @@ namespace Overblog\GraphQLBundle\Relay\Node; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; -use Overblog\GraphQLBundle\GraphQL\Relay\Node\GlobalIdFieldResolver; final class GlobalIdFieldDefinition implements MappingInterface { @@ -11,12 +10,11 @@ public function toMappingDefinition(array $config) { $typeName = isset($config['typeName']) && is_string($config['typeName']) ? var_export($config['typeName'], true) : 'null'; $idFetcher = isset($config['idFetcher']) && is_string($config['idFetcher']) ? $this->cleanIdFetcher($config['idFetcher']) : 'null'; - $resolver = addslashes(GlobalIdFieldResolver::class); return [ 'description' => 'The ID of an object', 'type' => 'ID!', - 'resolve' => "@=resolver('$resolver', [value, info, $idFetcher, $typeName])", + 'resolve' => "@=resolver('relay_globalid_field', [value, info, $idFetcher, $typeName])", ]; } diff --git a/Relay/Node/NodeFieldDefinition.php b/Relay/Node/NodeFieldDefinition.php index cc17d195a..029bd2ef4 100644 --- a/Relay/Node/NodeFieldDefinition.php +++ b/Relay/Node/NodeFieldDefinition.php @@ -3,7 +3,6 @@ namespace Overblog\GraphQLBundle\Relay\Node; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; -use Overblog\GraphQLBundle\GraphQL\Relay\Node\NodeFieldResolver; final class NodeFieldDefinition implements MappingInterface { @@ -15,7 +14,6 @@ public function toMappingDefinition(array $config) $idFetcher = $this->cleanIdFetcher($config['idFetcher']); $nodeInterfaceType = isset($config['nodeInterfaceType']) && is_string($config['nodeInterfaceType']) ? $config['nodeInterfaceType'] : null; - $resolver = addslashes(NodeFieldResolver::class); return [ 'description' => 'Fetches an object given its ID', @@ -23,7 +21,7 @@ public function toMappingDefinition(array $config) 'args' => [ 'id' => ['type' => 'ID!', 'description' => 'The ID of an object'], ], - 'resolve' => "@=resolver('$resolver', [args, context, info, idFetcherCallback($idFetcher)])", + 'resolve' => "@=resolver('relay_node_field', [args, context, info, idFetcherCallback($idFetcher)])", ]; } diff --git a/Relay/Node/PluralIdentifyingRootFieldDefinition.php b/Relay/Node/PluralIdentifyingRootFieldDefinition.php index 292bd76f3..3a666a0d7 100644 --- a/Relay/Node/PluralIdentifyingRootFieldDefinition.php +++ b/Relay/Node/PluralIdentifyingRootFieldDefinition.php @@ -3,7 +3,6 @@ namespace Overblog\GraphQLBundle\Relay\Node; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; -use Overblog\GraphQLBundle\GraphQL\Relay\Node\PluralIdentifyingRootFieldResolver; final class PluralIdentifyingRootFieldDefinition implements MappingInterface { @@ -26,14 +25,12 @@ public function toMappingDefinition(array $config) } $argName = $config['argName']; - $resolver = addslashes(PluralIdentifyingRootFieldResolver::class); return [ 'type' => "[${config['outputType']}]", 'args' => [$argName => ['type' => "[${config['inputType']}!]!"]], 'resolve' => sprintf( - "@=resolver('%s', [args['$argName'], context, info, resolveSingleInputCallback(%s)])", - $resolver, + "@=resolver('relay_plural_identifying_field', [args['$argName'], context, info, resolveSingleInputCallback(%s)])", $this->cleanResolveSingleInput($config['resolveSingleInput']) ), ]; diff --git a/Resources/doc/definitions/resolver.md b/Resources/doc/definitions/resolver.md index 70a0b6113..a852c96d4 100644 --- a/Resources/doc/definitions/resolver.md +++ b/Resources/doc/definitions/resolver.md @@ -15,8 +15,9 @@ Resolvers can be define 2 different ways or `Overblog\GraphQLBundle\Definition\Resolver\MutationInterface`) in `src/*Bundle/GraphQL` or `app/GraphQL` and they will be auto discovered. Auto map classes method are accessible by: - * the class method name (example: `AppBunble\GraphQL\CustomResolver::myMethod`) - * the FQCN for callable classes (example: `AppBunble\GraphQL\InvokeResolver` for a resolver implementing the `__invoke` method) + * double colon (::) to separate service id (class name) and the method names + (example: `AppBunble\GraphQL\CustomResolver::myMethod` or `appbunble\graphql\customresolver::myMethod` for Symfony < 3.3) + * for callable classes you can use the service id (example: `AppBunble\GraphQL\InvokeResolver` for a resolver implementing the `__invoke` method) you can also alias a type by implementing `Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface` which returns a map of method/alias. The service created will autowire the `__construct` and `Symfony\Component\DependencyInjection\ContainerAwareInterface::setContainer` methods. @@ -133,7 +134,7 @@ Resolvers can be define 2 different ways ``` **Note:** - * When using FQCN in yaml definition, backslash must be correctly quotes, + * When using service id as FQCN in yaml definition, backslash must be correctly quotes, here an example `'@=resolver("App\\GraphQL\\Resolver\\Greetings", [args['name']])'`. * You can also see the more straight forward way using [resolver map](resolver-map.md) @@ -151,7 +152,7 @@ Resolvers can be define 2 different ways #class: App\GraphQL\Resolver\Greetings tags: - { name: overblog_graphql.resolver, method: sayHello, alias: say_hello } # add alias say_hello - - { name: overblog_graphql.resolver, method: sayHello } # add method full qualified name + - { name: overblog_graphql.resolver, method: sayHello } # add service id "App\GraphQL\Resolver\Greetings" ``` `SayHello` resolver can be access by using `App\GraphQL\Resolver\Greetings::sayHello` or @@ -168,7 +169,7 @@ Resolvers can be define 2 different ways - { name: overblog_graphql.resolver } ``` - This way resolver can be accessed with FQCN `App\GraphQL\Resolver\Greetings`. + This way resolver can be accessed with service id `App\GraphQL\Resolver\Greetings`. for mutation: diff --git a/Resources/doc/definitions/type-system/index.md b/Resources/doc/definitions/type-system/index.md index 08cd8a7da..05ba9551d 100644 --- a/Resources/doc/definitions/type-system/index.md +++ b/Resources/doc/definitions/type-system/index.md @@ -40,7 +40,7 @@ Types can be define 3 different ways: You can also declare PHP types (any subclass of `GraphQL\Type\Definition\Type`) in `src/*Bundle/GraphQL` or `app/GraphQL` - they will be auto discover (thanks to auto mapping). Auto map classes are accessible by FQCN + they will be auto discover (thanks to auto mapping). Auto map classes are accessible by service id (example: `AppBunble\GraphQL\Type\DateTimeType`), you can also alias a type by implementing `Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface` that returns an array of aliases. @@ -102,7 +102,7 @@ Types can be define 3 different ways: **Note:** * Types are lazy loaded so when using Symfony DI `autoconfigure` or this bundle auto mapping, the only access to type is FQCN (or aliases if implements the aliases interface). - * When using FQCN in yaml definition, backslash must be correctly quotes, + * When using service id as FQCN in yaml definition, backslash must be correctly quotes, 3. **The service way** diff --git a/Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php b/Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php index 72224176d..b82e78d76 100644 --- a/Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php +++ b/Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php @@ -7,6 +7,7 @@ use Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface; use Overblog\GraphQLBundle\Resolver\ResolverResolver; use Overblog\GraphQLBundle\Tests\Functional\App\IsolatedResolver\EchoResolver; +use Symfony\Component\HttpKernel\Kernel; final class QueryType extends ObjectType implements AliasedInterface { @@ -22,7 +23,7 @@ public function __construct(ResolverResolver $resolver) ], 'resolve' => function ($root, $args) use ($resolver) { return $resolver->resolve([ - EchoResolver::class, + version_compare(Kernel::VERSION, '3.3.0') < 0 ? strtolower(EchoResolver::class) : EchoResolver::class, [$args['message']], ]); }, diff --git a/Tests/Functional/App/config/autoMapping/config.yml b/Tests/Functional/App/config/autoMapping/config.yml index 776add1d1..832656bfa 100644 --- a/Tests/Functional/App/config/autoMapping/config.yml +++ b/Tests/Functional/App/config/autoMapping/config.yml @@ -9,5 +9,5 @@ overblog_graphql: enabled: true directories: ["%kernel.root_dir%/IsolatedResolver"] schema: - query: "Overblog\\GraphQLBundle\\Tests\\Functional\\App\\GraphQL\\HelloWord\\Type\\QueryType" - mutation: "Overblog\\GraphQLBundle\\Tests\\Functional\\App\\GraphQL\\HelloWord\\Type\\MutationType" + query: "Query" + mutation: "Calc" diff --git a/Tests/Functional/App/config/node/config.yml b/Tests/Functional/App/config/node/config.yml index f4c1ac422..677da302d 100644 --- a/Tests/Functional/App/config/node/config.yml +++ b/Tests/Functional/App/config/node/config.yml @@ -2,7 +2,7 @@ imports: - { resource: ../config.yml } services: - overblog_graphql.test.resolver.node: + node_resolver: class: Overblog\GraphQLBundle\Tests\Functional\App\Resolver\NodeResolver arguments: - "@overblog_graphql.type_resolver" diff --git a/Tests/Functional/App/config/node/mapping/node_type.yml b/Tests/Functional/App/config/node/mapping/node_type.yml index e5726aebd..fa5aee5ce 100644 --- a/Tests/Functional/App/config/node/mapping/node_type.yml +++ b/Tests/Functional/App/config/node/mapping/node_type.yml @@ -1,4 +1,4 @@ Node: type: relay-node config: - resolveType: '@=resolver("Overblog\\GraphQLBundle\\Tests\\Functional\\App\\Resolver\\NodeResolver::typeResolver", [value])' + resolveType: '@=resolver("node_resolver::typeResolver", [value])' diff --git a/Tests/Functional/App/config/plural/config.yml b/Tests/Functional/App/config/plural/config.yml index b27b065ef..dbc937039 100644 --- a/Tests/Functional/App/config/plural/config.yml +++ b/Tests/Functional/App/config/plural/config.yml @@ -2,7 +2,7 @@ imports: - { resource: ../config.yml } services: - overblog_graphql.test.resolver.plural: + plural_resolver: class: Overblog\GraphQLBundle\Tests\Functional\App\Resolver\PluralResolver tags: - { name: "overblog_graphql.resolver" } diff --git a/Tests/Functional/App/config/plural/mapping/Query.types.xml b/Tests/Functional/App/config/plural/mapping/Query.types.xml index 42e488ecf..99598ed88 100644 --- a/Tests/Functional/App/config/plural/mapping/Query.types.xml +++ b/Tests/Functional/App/config/plural/mapping/Query.types.xml @@ -15,7 +15,7 @@ Map from a username to the user String User - @=resolver("Overblog\\GraphQLBundle\\Tests\\Functional\\App\\Resolver\\PluralResolver", [value, info]) + @=resolver("plural_resolver", [value, info]) diff --git a/Tests/Functional/Command/DebugCommandTest.php b/Tests/Functional/Command/DebugCommandTest.php index 796b4dca6..81fe0cc90 100644 --- a/Tests/Functional/Command/DebugCommandTest.php +++ b/Tests/Functional/Command/DebugCommandTest.php @@ -26,10 +26,7 @@ public function setUp() $this->command = static::$kernel->getContainer()->get('overblog_graphql.command.debug'); $this->commandTester = new CommandTester($this->command); - $categories = DebugCommand::getCategories(); - $categories[] = 'all'; - - foreach ($categories as $category) { + foreach (DebugCommand::getCategories() as $category) { $content = file_get_contents( sprintf( __DIR__.'/fixtures/debug/debug-%s.txt', @@ -37,7 +34,7 @@ public function setUp() ) ); - $this->logs[$category] = 'all' === $category ? $content : trim($content); + $this->logs[$category] = trim($content); } } @@ -59,18 +56,7 @@ public function testProcess(array $categories) $expected .= $this->logs[$category]." \n\n\n\n"; } - $this->assertEquals($expected, $this->commandTester->getDisplay()); - } - - public function testProcessWithServiceId() - { - if (version_compare(Kernel::VERSION, '3.3.0') < 0) { - $this->markTestSkipped('Test only for Symfony >= 3.3.0.'); - } - - $this->commandTester->execute(['--with-service-id' => null]); - $this->assertEquals(0, $this->commandTester->getStatusCode()); - $this->assertEquals($this->logs['all'], $this->commandTester->getDisplay()); + $this->assertContains($expected, $this->commandTester->getDisplay(), '', version_compare(Kernel::VERSION, '3.3.0') < 0); } /** diff --git a/Tests/Functional/Command/fixtures/debug/debug-all.txt b/Tests/Functional/Command/fixtures/debug/debug-all.txt deleted file mode 100644 index 3e6f08d5c..000000000 --- a/Tests/Functional/Command/fixtures/debug/debug-all.txt +++ /dev/null @@ -1,50 +0,0 @@ - -GraphQL Types Services -====================== - - ------------------------------------------------------------------------------------------ -------------------------------------- ------------------------------------------------------------------------------------------ - solution id aliases service id - ------------------------------------------------------------------------------------------ -------------------------------------- ------------------------------------------------------------------------------------------ - GraphQL\Type\Definition\BooleanType Boolean overblog_graphql.definition.boolean_type - GraphQL\Type\Definition\FloatType Float overblog_graphql.definition.float_type - GraphQL\Type\Definition\IDType ID overblog_graphql.definition.id_type - GraphQL\Type\Definition\IntType Int overblog_graphql.definition.int_type - GraphQL\Type\Definition\StringType String overblog_graphql.definition.string_type - Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\PageInfoType PageInfo Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\PageInfoType - Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\RootMutationType RootMutation Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\RootMutationType - Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationInputType simpleMutationInput Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationInputType - Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationPayloadType simpleMutationPayload Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationPayloadType - Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationWithThunkFieldsInputType simpleMutationWithThunkFieldsInput Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationWithThunkFieldsInputType - Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationWithThunkFieldsPayloadType simpleMutationWithThunkFieldsPayload Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationWithThunkFieldsPayloadType - Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simplePromiseMutationInputType simplePromiseMutationInput Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simplePromiseMutationInputType - Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simplePromiseMutationPayloadType simplePromiseMutationPayload Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simplePromiseMutationPayloadType - ------------------------------------------------------------------------------------------ -------------------------------------- ------------------------------------------------------------------------------------------ - - - -GraphQL Mutations Services -========================== - - ---------------------------------------------------------------------------------------------------- ---------------------------------------------------- --------------------------------------------------------- - solution id aliases service id - ---------------------------------------------------------------------------------------------------- ---------------------------------------------------- --------------------------------------------------------- - Overblog\GraphQLBundle\Tests\Functional\App\Mutation\SimpleMutationWithThunkFieldsMutation::mutate simple_mutation_with_thunk_fields (method: mutate) overblog_graphql.test.simple_mutation_with_thunk_fields - Overblog\GraphQLBundle\Tests\Functional\App\Mutation\SimplePromiseMutation::mutate simple_promise_mutation (method: mutate) overblog_graphql.test.simple_promise_mutation - ---------------------------------------------------------------------------------------------------- ---------------------------------------------------- --------------------------------------------------------- - - - -GraphQL Resolvers Services -========================== - - ------------------------------------------------------------------------------ --------------------------------------------------- ------------------------------------------------------------------------------ - solution id aliases service id - ------------------------------------------------------------------------------ --------------------------------------------------- ------------------------------------------------------------------------------ - Overblog\GraphQLBundle\GraphQL\Relay\Mutation\MutationFieldResolver relay_mutation_field (method: __invoke) Overblog\GraphQLBundle\GraphQL\Relay\Mutation\MutationFieldResolver - Overblog\GraphQLBundle\GraphQL\Relay\Node\GlobalIdFieldResolver relay_globalid_field (method: __invoke) Overblog\GraphQLBundle\GraphQL\Relay\Node\GlobalIdFieldResolver - Overblog\GraphQLBundle\GraphQL\Relay\Node\NodeFieldResolver relay_node_field (method: __invoke) Overblog\GraphQLBundle\GraphQL\Relay\Node\NodeFieldResolver - Overblog\GraphQLBundle\GraphQL\Relay\Node\PluralIdentifyingRootFieldResolver relay_plural_identifying_field (method: __invoke) Overblog\GraphQLBundle\GraphQL\Relay\Node\PluralIdentifyingRootFieldResolver - ------------------------------------------------------------------------------ --------------------------------------------------- ------------------------------------------------------------------------------ - - - diff --git a/Tests/Functional/Command/fixtures/debug/debug-mutation.txt b/Tests/Functional/Command/fixtures/debug/debug-mutation.txt index a56827429..23ed6ecd5 100644 --- a/Tests/Functional/Command/fixtures/debug/debug-mutation.txt +++ b/Tests/Functional/Command/fixtures/debug/debug-mutation.txt @@ -2,12 +2,12 @@ GraphQL Mutations Services ========================== - ---------------------------------------------------------------------------------------------------- ---------------------------------------------------- - solution id aliases - ---------------------------------------------------------------------------------------------------- ---------------------------------------------------- - Overblog\GraphQLBundle\Tests\Functional\App\Mutation\SimpleMutationWithThunkFieldsMutation::mutate simple_mutation_with_thunk_fields (method: mutate) - Overblog\GraphQLBundle\Tests\Functional\App\Mutation\SimplePromiseMutation::mutate simple_promise_mutation (method: mutate) - ---------------------------------------------------------------------------------------------------- ---------------------------------------------------- + ----------------------------------------------------------------- ----------------------------------- + solution id aliases + ----------------------------------------------------------------- ----------------------------------- + overblog_graphql.test.simple_mutation_with_thunk_fields::mutate simple_mutation_with_thunk_fields + overblog_graphql.test.simple_promise_mutation::mutate simple_promise_mutation + ----------------------------------------------------------------- ----------------------------------- diff --git a/Tests/Functional/Command/fixtures/debug/debug-resolver.txt b/Tests/Functional/Command/fixtures/debug/debug-resolver.txt index a85e8a0e6..419f694ff 100644 --- a/Tests/Functional/Command/fixtures/debug/debug-resolver.txt +++ b/Tests/Functional/Command/fixtures/debug/debug-resolver.txt @@ -2,14 +2,14 @@ GraphQL Resolvers Services ========================== - ------------------------------------------------------------------------------ --------------------------------------------------- - solution id aliases - ------------------------------------------------------------------------------ --------------------------------------------------- - Overblog\GraphQLBundle\GraphQL\Relay\Mutation\MutationFieldResolver relay_mutation_field (method: __invoke) - Overblog\GraphQLBundle\GraphQL\Relay\Node\GlobalIdFieldResolver relay_globalid_field (method: __invoke) - Overblog\GraphQLBundle\GraphQL\Relay\Node\NodeFieldResolver relay_node_field (method: __invoke) - Overblog\GraphQLBundle\GraphQL\Relay\Node\PluralIdentifyingRootFieldResolver relay_plural_identifying_field (method: __invoke) - ------------------------------------------------------------------------------ --------------------------------------------------- + ------------------------------------------------------------------------------ -------------------------------- + solution id aliases + ------------------------------------------------------------------------------ -------------------------------- + Overblog\GraphQLBundle\GraphQL\Relay\Mutation\MutationFieldResolver relay_mutation_field + Overblog\GraphQLBundle\GraphQL\Relay\Node\GlobalIdFieldResolver relay_globalid_field + Overblog\GraphQLBundle\GraphQL\Relay\Node\NodeFieldResolver relay_node_field + Overblog\GraphQLBundle\GraphQL\Relay\Node\PluralIdentifyingRootFieldResolver relay_plural_identifying_field + ------------------------------------------------------------------------------ -------------------------------- diff --git a/Tests/Functional/Command/fixtures/debug/debug-type.txt b/Tests/Functional/Command/fixtures/debug/debug-type.txt index 2b57369bd..0b189e0cf 100644 --- a/Tests/Functional/Command/fixtures/debug/debug-type.txt +++ b/Tests/Functional/Command/fixtures/debug/debug-type.txt @@ -5,11 +5,6 @@ GraphQL Types Services ------------------------------------------------------------------------------------------ -------------------------------------- solution id aliases ------------------------------------------------------------------------------------------ -------------------------------------- - GraphQL\Type\Definition\BooleanType Boolean - GraphQL\Type\Definition\FloatType Float - GraphQL\Type\Definition\IDType ID - GraphQL\Type\Definition\IntType Int - GraphQL\Type\Definition\StringType String Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\PageInfoType PageInfo Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\RootMutationType RootMutation Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationInputType simpleMutationInput @@ -18,6 +13,11 @@ GraphQL Types Services Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simpleMutationWithThunkFieldsPayloadType simpleMutationWithThunkFieldsPayload Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simplePromiseMutationInputType simplePromiseMutationInput Overblog\GraphQLBundle\Mutation\__DEFINITIONS__\simplePromiseMutationPayloadType simplePromiseMutationPayload + overblog_graphql.definition.boolean_type Boolean + overblog_graphql.definition.float_type Float + overblog_graphql.definition.id_type ID + overblog_graphql.definition.int_type Int + overblog_graphql.definition.string_type String ------------------------------------------------------------------------------------------ -------------------------------------- diff --git a/Tests/Relay/Node/NodeFieldDefinitionTest.php b/Tests/Relay/Node/NodeFieldDefinitionTest.php index 7d6a4683d..790c7bcf4 100644 --- a/Tests/Relay/Node/NodeFieldDefinitionTest.php +++ b/Tests/Relay/Node/NodeFieldDefinitionTest.php @@ -2,7 +2,6 @@ namespace Overblog\GraphQLBundle\Tests\Relay\Node; -use Overblog\GraphQLBundle\GraphQL\Relay\Node\NodeFieldResolver; use Overblog\GraphQLBundle\Relay\Node\NodeFieldDefinition; use PHPUnit\Framework\TestCase; @@ -53,7 +52,7 @@ public function testValidConfig($idFetcher, $idFetcherCallbackArg, $nodeInterfac 'description' => 'Fetches an object given its ID', 'type' => $nodeInterfaceType, 'args' => ['id' => ['type' => 'ID!', 'description' => 'The ID of an object']], - 'resolve' => '@=resolver(\''.addslashes(NodeFieldResolver::class).'\', [args, context, info, idFetcherCallback('.$idFetcherCallbackArg.')])', + 'resolve' => '@=resolver(\'relay_node_field\', [args, context, info, idFetcherCallback('.$idFetcherCallbackArg.')])', ]; $this->assertEquals($expected, $this->definition->toMappingDefinition($config)); diff --git a/Tests/Relay/Node/PluralIdentifyingRootFieldDefinitionTest.php b/Tests/Relay/Node/PluralIdentifyingRootFieldDefinitionTest.php index ea1da4dd3..d5c1b738a 100644 --- a/Tests/Relay/Node/PluralIdentifyingRootFieldDefinitionTest.php +++ b/Tests/Relay/Node/PluralIdentifyingRootFieldDefinitionTest.php @@ -2,7 +2,6 @@ namespace Overblog\GraphQLBundle\Tests\Relay\Node; -use Overblog\GraphQLBundle\GraphQL\Relay\Node\PluralIdentifyingRootFieldResolver; use Overblog\GraphQLBundle\Relay\Node\PluralIdentifyingRootFieldDefinition; use PHPUnit\Framework\TestCase; @@ -97,7 +96,7 @@ public function testValidConfig($resolveSingleInput, $expectedResolveSingleInput $expected = [ 'type' => '[User]', 'args' => ['username' => ['type' => '[UserInput!]!']], - 'resolve' => '@=resolver(\''.addslashes(PluralIdentifyingRootFieldResolver::class).'\', [args[\'username\'], context, info, resolveSingleInputCallback('.$expectedResolveSingleInputCallbackArg.')])', + 'resolve' => '@=resolver(\'relay_plural_identifying_field\', [args[\'username\'], context, info, resolveSingleInputCallback('.$expectedResolveSingleInputCallbackArg.')])', ]; $this->assertEquals($expected, $this->definition->toMappingDefinition($config)); diff --git a/UPGRADE-0.11.md b/UPGRADE-0.11.md index 449bd75d5..de0aa3f00 100644 --- a/UPGRADE-0.11.md +++ b/UPGRADE-0.11.md @@ -10,6 +10,7 @@ UPGRADE FROM 0.10 to 0.11 - [Type autoMapping and Symfony DI autoconfigure](#type-automapping-and-symfony-di-autoconfigure) - [Events](#events) - [Explicitly declare non detected types](#explicitly-declare-non-detected-types) +- [Change fluent resolvers id](#change-fluent-resolvers-id) ### GraphiQL @@ -154,6 +155,7 @@ UPGRADE FROM 0.10 to 0.11 } ``` **Before 0.11**: DateTimeType could be accessed by FQCN `App\GraphQL\Type\DateTimeType` and the real `DateTimeType`. + **Since 0.11**: Only FQCN `App\GraphQL\Type\DateTimeType` is accessible here how this can be done in 0.11: @@ -252,3 +254,23 @@ UPGRADE FROM 0.10 to 0.11 `context` is of type `ArrayObject` and `rootValue` has no typeHint (default: `null`) so `$context !== $info->rootValue` and `$context !== $value` in root query resolver. Uploaded files is no more accessible under `$info->rootValue['request_files']` out of the box. + +### Change fluent resolvers id + + The use of class name as prefix of fluent resolver id remove the possibility to use same class as 2 different services. + see this [issue for more detail](https://github.com/overblog/GraphQLBundle/issues/296) + That the reason from 0.11 we using service id as prefix (like in Symfony 4.1)... + + Example: + ```yaml + services: + app.resolver.greetings: + class: App\GraphQL\Resolver\Greetings + tags: + - { name: overblog_graphql.resolver, method: __invoke, alias: say_hello } + - { name: overblog_graphql.resolver } + ``` + + **Before 0.11**: `'@=resolver("App\\GraphQL\\Resolver\\Greetings", [args['name']])'` + + **Since 0.11**: `'@=resolver("app.resolver.greetings", [args['name']])'` From 7c887f802f2c99f91120b72491241cab71949f4c Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 1 Mar 2018 08:05:54 +0100 Subject: [PATCH 2/3] Use https --- Config/TypeDefinition.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Config/TypeDefinition.php b/Config/TypeDefinition.php index 6a977fd19..2add640f0 100644 --- a/Config/TypeDefinition.php +++ b/Config/TypeDefinition.php @@ -37,7 +37,7 @@ protected function nameSection() ->ifTrue(function ($name) { return !preg_match('/^[_a-z][_0-9a-z]*$/i', $name); }) - ->thenInvalid('Invalid type name "%s". (see http://facebook.github.io/graphql/October2016/#Name)') + ->thenInvalid('Invalid type name "%s". (see https://facebook.github.io/graphql/October2016/#Name)') ->end(); return $node; From 04e48ae9948031fb0905468d02007af6c2e41c6a Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Mon, 5 Mar 2018 10:10:02 +0100 Subject: [PATCH 3/3] Fix review feedback --- Resolver/AbstractResolver.php | 20 ++++++++++++++++++- Resolver/TypeResolver.php | 18 ++++------------- Resources/config/services.yml | 2 -- Resources/doc/definitions/resolver.md | 6 +++--- .../doc/definitions/type-system/index.md | 2 +- .../App/GraphQL/HelloWord/Type/QueryType.php | 6 +----- UPGRADE-0.11.md | 4 ++-- composer.json | 4 +--- 8 files changed, 31 insertions(+), 31 deletions(-) diff --git a/Resolver/AbstractResolver.php b/Resolver/AbstractResolver.php index 1e11281e7..0442cebcf 100644 --- a/Resolver/AbstractResolver.php +++ b/Resolver/AbstractResolver.php @@ -2,6 +2,8 @@ namespace Overblog\GraphQLBundle\Resolver; +use Symfony\Component\HttpKernel\Kernel; + abstract class AbstractResolver implements FluentResolverInterface { /** @var array */ @@ -15,8 +17,17 @@ abstract class AbstractResolver implements FluentResolverInterface /** @var array */ private $fullyLoadedSolutions = []; + /** @var bool */ + private $ignoreCase = true; + + public function __construct() + { + $this->ignoreCase = version_compare(Kernel::VERSION, '3.3.0') < 0; + } + public function addSolution($id, $solutionOrFactory, array $aliases = [], array $options = []) { + $id = $this->cleanIdOrAlias($id); $this->fullyLoadedSolutions[$id] = false; $this->addAliases($id, $aliases); @@ -105,7 +116,7 @@ private function loadSolution($id) private function addAliases($id, $aliases) { foreach ($aliases as $alias) { - $this->aliases[$alias] = $id; + $this->aliases[$this->cleanIdOrAlias($alias)] = $id; } } @@ -116,9 +127,16 @@ private static function isSolutionFactory($solutionOrFactory) private function resolveAlias($alias) { + $alias = $this->cleanIdOrAlias($alias); + return isset($this->aliases[$alias]) ? $this->aliases[$alias] : $alias; } + private function cleanIdOrAlias($idOrAlias) + { + return $this->ignoreCase ? strtolower($idOrAlias) : $idOrAlias; + } + /** * @return mixed[] */ diff --git a/Resolver/TypeResolver.php b/Resolver/TypeResolver.php index a85957b0a..6ecaa5ae9 100644 --- a/Resolver/TypeResolver.php +++ b/Resolver/TypeResolver.php @@ -3,18 +3,10 @@ namespace Overblog\GraphQLBundle\Resolver; use GraphQL\Type\Definition\Type; -use Psr\Cache\CacheItemPoolInterface; -use Symfony\Component\Cache\Adapter\ArrayAdapter; class TypeResolver extends AbstractResolver { - /** @var CacheItemPoolInterface */ - private $cacheAdapter; - - public function __construct(CacheItemPoolInterface $cacheAdapter = null) - { - $this->cacheAdapter = null !== $cacheAdapter ? $cacheAdapter : new ArrayAdapter(0, false); - } + private $cache = []; /** * @param string $alias @@ -26,15 +18,13 @@ public function resolve($alias) if (null === $alias) { return; } - $item = $this->cacheAdapter->getItem(md5($alias)); - if (!$item->isHit()) { + if (!isset($this->cache[$alias])) { $type = $this->string2Type($alias); - $item->set($type); - $this->cacheAdapter->save($item); + $this->cache[$alias] = $type; } - return $item->get(); + return $this->cache[$alias]; } private function string2Type($alias) diff --git a/Resources/config/services.yml b/Resources/config/services.yml index cb1902ecf..e8e1d307f 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -38,8 +38,6 @@ services: overblog_graphql.type_resolver: class: Overblog\GraphQLBundle\Resolver\TypeResolver public: true - arguments: - - tags: - { name: overblog_graphql.global_variable, alias: typeResolver } diff --git a/Resources/doc/definitions/resolver.md b/Resources/doc/definitions/resolver.md index a852c96d4..d926ebde4 100644 --- a/Resources/doc/definitions/resolver.md +++ b/Resources/doc/definitions/resolver.md @@ -15,8 +15,8 @@ Resolvers can be define 2 different ways or `Overblog\GraphQLBundle\Definition\Resolver\MutationInterface`) in `src/*Bundle/GraphQL` or `app/GraphQL` and they will be auto discovered. Auto map classes method are accessible by: - * double colon (::) to separate service id (class name) and the method names - (example: `AppBunble\GraphQL\CustomResolver::myMethod` or `appbunble\graphql\customresolver::myMethod` for Symfony < 3.3) + * double-colon (::) to separate service id (class name) and the method names + (example: `AppBunble\GraphQL\CustomResolver::myMethod`) * for callable classes you can use the service id (example: `AppBunble\GraphQL\InvokeResolver` for a resolver implementing the `__invoke` method) you can also alias a type by implementing `Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface` which returns a map of method/alias. The service created will autowire the `__construct` @@ -134,7 +134,7 @@ Resolvers can be define 2 different ways ``` **Note:** - * When using service id as FQCN in yaml definition, backslash must be correctly quotes, + * When using service id as FQCN in yaml definition, backslashes must be correctly escaped, here an example `'@=resolver("App\\GraphQL\\Resolver\\Greetings", [args['name']])'`. * You can also see the more straight forward way using [resolver map](resolver-map.md) diff --git a/Resources/doc/definitions/type-system/index.md b/Resources/doc/definitions/type-system/index.md index 05ba9551d..c6bf02263 100644 --- a/Resources/doc/definitions/type-system/index.md +++ b/Resources/doc/definitions/type-system/index.md @@ -102,7 +102,7 @@ Types can be define 3 different ways: **Note:** * Types are lazy loaded so when using Symfony DI `autoconfigure` or this bundle auto mapping, the only access to type is FQCN (or aliases if implements the aliases interface). - * When using service id as FQCN in yaml definition, backslash must be correctly quotes, + * When using service id as FQCN in yaml definition, backslashes must be correctly escaped, 3. **The service way** diff --git a/Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php b/Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php index b82e78d76..26e8008d7 100644 --- a/Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php +++ b/Tests/Functional/App/GraphQL/HelloWord/Type/QueryType.php @@ -7,7 +7,6 @@ use Overblog\GraphQLBundle\Definition\Resolver\AliasedInterface; use Overblog\GraphQLBundle\Resolver\ResolverResolver; use Overblog\GraphQLBundle\Tests\Functional\App\IsolatedResolver\EchoResolver; -use Symfony\Component\HttpKernel\Kernel; final class QueryType extends ObjectType implements AliasedInterface { @@ -22,10 +21,7 @@ public function __construct(ResolverResolver $resolver) 'message' => ['type' => Type::string()], ], 'resolve' => function ($root, $args) use ($resolver) { - return $resolver->resolve([ - version_compare(Kernel::VERSION, '3.3.0') < 0 ? strtolower(EchoResolver::class) : EchoResolver::class, - [$args['message']], - ]); + return $resolver->resolve([EchoResolver::class, [$args['message']]]); }, ], ], diff --git a/UPGRADE-0.11.md b/UPGRADE-0.11.md index de0aa3f00..5a6797406 100644 --- a/UPGRADE-0.11.md +++ b/UPGRADE-0.11.md @@ -258,8 +258,8 @@ UPGRADE FROM 0.10 to 0.11 ### Change fluent resolvers id The use of class name as prefix of fluent resolver id remove the possibility to use same class as 2 different services. - see this [issue for more detail](https://github.com/overblog/GraphQLBundle/issues/296) - That the reason from 0.11 we using service id as prefix (like in Symfony 4.1)... + See issue [#296](https://github.com/overblog/GraphQLBundle/issues/296) for more detail + That's the reason why starting v0.11 we are using service id as prefix (like in Symfony 4.1)... Example: ```yaml diff --git a/composer.json b/composer.json index dbc97cdd5..ec8825850 100644 --- a/composer.json +++ b/composer.json @@ -30,9 +30,8 @@ }, "require": { "php": ">=5.6", - "doctrine/doctrine-cache-bundle": "^1.2", "overblog/graphql-php-generator": "^0.7.0", - "symfony/cache": "^3.1 || ^4.0", + "psr/log": "^1.0", "symfony/config": "^3.1 || ^4.0", "symfony/dependency-injection": "^3.1 || ^4.0", "symfony/event-dispatcher": "^3.1 || ^4.0", @@ -49,7 +48,6 @@ }, "require-dev": { "phpunit/phpunit": "^5.7.26 || ^6.0", - "psr/log": "^1.0", "react/promise": "^2.5", "sensio/framework-extra-bundle": "^3.0", "symfony/asset": "^3.1 || ^4.0",