bug #297 Fix FluentResolver when tagging two services with same class…

… name 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.
overblog · Mar 5, 2018 · cab67b3 · cab67b3
2 parents e7e7ee7 + 04e48ae
commit cab67b3
Show file tree

Hide file tree

Showing 27 changed files with 96 additions and 166 deletions.
diff --git 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,53 +66,41 @@ 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);
         }
     }
 
     /**
      * @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/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;

diff --git 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
@@ -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
@@ -3,20 +3,18 @@
 namespace Overblog\GraphQLBundle\Relay\Node;
 
 use Overblog\GraphQLBundle\Definition\Builder\MappingInterface;
-use Overblog\GraphQLBundle\GraphQL\Relay\Node\GlobalIdFieldResolver;
 
 final class GlobalIdFieldDefinition implements MappingInterface
 {
     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
@@ -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,15 +14,14 @@ 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',
             'type' => $nodeInterfaceType,
             '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
@@ -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/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
@@ -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
@@ -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
@@ -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`)
+    * 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, 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)
 
@@ -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
@@ -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, 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
@@ -21,10 +21,7 @@ public function __construct(ResolverResolver $resolver)
                         'message' => ['type' => Type::string()],
                     ],
                     'resolve' => function ($root, $args) use ($resolver) {
-                        return $resolver->resolve([
-                            EchoResolver::class,
-                            [$args['message']],
-                        ]);
+                        return $resolver->resolve([EchoResolver::class, [$args['message']]]);
                     },
                 ],
             ],

diff --git 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
@@ -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
@@ -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
@@ -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" }