Make the resolver maps registrable using service tags and handle per-schema priority

ste93cry · ste93cry · commit 5bbdf221d437 · 2019-11-29T09:39:01.000+01:00
diff --git a/UPGRADE-0.13.md b/UPGRADE-0.13.md
@@ -5,6 +5,7 @@ UPGRADE FROM 0.12 to 0.13
 
 - [Rename default_field config](#rename-default_field-config)
 - [Improve default field resolver](#improve-default-field-resolver)
+- [Use service tags to register resolver maps](#use-service-tags-to-register-resolver-maps)
 
 ### Rename default_field config
 
@@ -50,3 +51,27 @@ MyType:
 ```
 
 [see default Field Resolver per type for more details](https://webonyx.github.io/graphql-php/data-fetching/#default-field-resolver-per-type)
+
+### Use service tags to register resolver maps
+
+The resolver maps used to be configured using the `overblog_graphql.definitions.schema.resolver_maps`
+option. This has been deprecated in favour of using service tags to register them.
+
+```diff
+# config/graphql.yaml
+overblog_graphql:
+    definitions:
+        schema:
+             # ...
+-            resolver_maps:
+-                - 'App\GraphQL\MyResolverMap'
+```
+
+```diff
+# services/graphql.yaml
+services:
+-    App\GraphQL\MyResolverMap: ~
++    App\GraphQL\MyResolverMap:
++        tags:
++            - { name: overblog_graphql.resolver_map, schema: default }
+```
diff --git a/docs/definitions/quick-start.md b/docs/definitions/quick-start.md
@@ -30,16 +30,6 @@ in files `config/graphql/types/*.graphql`
 
 4. Define schema Resolvers ([more details](resolver-map.md))
 
-```yaml
-# config/packages/graphql.yaml
-overblog_graphql:
-    definitions:
-        schema:
-            # ...
-            resolver_maps:
-                - App\Resolver\MyResolverMap
-```
-
 ```php
 <?php
 
diff --git a/docs/definitions/resolver-map.md b/docs/definitions/resolver-map.md
@@ -118,19 +118,21 @@ class MyResolverMap extends ResolverMap
 }
 ```
 
-Declare resolverMap to current schema
+Each resolver map must be tagged with the `overblog_graphql.resolver_map` tag
+that defines at which priority it should run for the given schema. The priority
+is an optional attribute and it has a default value of 0. The higher the number,
+the earlier the resolver map is executed.
 
 ```yaml
-overblog_graphql:
-    definitions:
-        schema:
-            # ...
-            # resolver maps services IDs
-            resolver_maps:
-                - App\Resolver\MyResolverMap
-
+# config/services.yaml
 services:
-    App\Resolver\MyResolverMap: ~
+    App\Resolver\MyResolverMap1:
+        tags:
+            - { name: overblog_graphql.resolver_map, schema: default }
+    
+    App\Resolver\MyResolverMap2:
+        tags:
+            - { name: overblog_graphql.resolver_map, schema: default, priority: 10 }
 ```
 
 **Notes:**
diff --git a/src/DependencyInjection/Compiler/ResolverMapTaggedServiceMappingPass.php b/src/DependencyInjection/Compiler/ResolverMapTaggedServiceMappingPass.php
@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Overblog\GraphQLBundle\DependencyInjection\Compiler;
+
+use Overblog\GraphQLBundle\EventListener\TypeDecoratorListener;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+use Symfony\Component\DependencyInjection\Reference;
+
+final class ResolverMapTaggedServiceMappingPass implements CompilerPassInterface
+{
+    private const SERVICE_TAG = 'overblog_graphql.resolver_map';
+
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container): void
+    {
+        $resolverMapsSortedBySchema = [];
+        $resolverMapsBySchemas = $container->getParameter('overblog_graphql.resolver_maps');
+        $typeDecoratorListenerDefinition = $container->getDefinition(TypeDecoratorListener::class);
+
+        foreach ($container->findTaggedServiceIds(self::SERVICE_TAG, true) as $serviceId => $tags) {
+            foreach ($tags as $tag) {
+                if (!isset($tag['schema'])) {
+                    throw new RuntimeException(\sprintf('The "schema" attribute on the "overblog_graphql.resolver_map" tag of the "%s" service is required.', $serviceId));
+                }
+
+                if (!isset($resolverMapsBySchemas[$tag['schema']])) {
+                    throw new RuntimeException(\sprintf('Service "%s" is invalid: schema "%s" specified on the tag "%s" does not exist (known ones are: "%s").', $serviceId, $tag['schema'], self::SERVICE_TAG, \implode('", "', \array_keys($resolverMapsBySchemas))));
+                }
+
+                $resolverMapsBySchemas[$tag['schema']][$serviceId] = $tag['priority'] ?? ($resolverMapsBySchemas[$tag['schema']][$serviceId] ?? 0);
+            }
+        }
+
+        foreach ($resolverMapsBySchemas as $schema => $resolverMaps) {
+            foreach ($resolverMaps as $resolverMap => $priority) {
+                $resolverMapsSortedBySchema[$schema][$priority][] = $resolverMap;
+            }
+        }
+
+        foreach ($resolverMapsSortedBySchema as $schema => $resolverMaps) {
+            \krsort($resolverMaps);
+
+            $resolverMaps = \array_merge(...$resolverMaps);
+
+            foreach ($resolverMaps as $index => $resolverMap) {
+                $resolverMaps[$index] = new Reference($resolverMap);
+            }
+
+            $typeDecoratorListenerDefinition->addMethodCall('addSchemaResolverMaps', [
+                $schema,
+                $resolverMaps,
+            ]);
+        }
+
+        $container->getParameterBag()->remove('overblog_graphql.resolver_maps');
+    }
+}
diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -210,6 +210,7 @@ private function definitionsSchemaSection()
                     ->arrayNode('resolver_maps')
                         ->defaultValue([])
                         ->prototype('scalar')->end()
+                        ->setDeprecated('The "%path%.%node%" configuration is deprecated since version 0.13 and will be removed in 0.14. Add the "overblog_graphql.resolver_map" tag to the services instead.')
                     ->end()
                     ->arrayNode('types')
                         ->defaultValue([])
diff --git a/src/DependencyInjection/OverblogGraphQLExtension.php b/src/DependencyInjection/OverblogGraphQLExtension.php
@@ -21,7 +21,6 @@
 use Overblog\GraphQLBundle\EventListener\DebugListener;
 use Overblog\GraphQLBundle\EventListener\ErrorHandlerListener;
 use Overblog\GraphQLBundle\EventListener\ErrorLoggerListener;
-use Overblog\GraphQLBundle\EventListener\TypeDecoratorListener;
 use Overblog\GraphQLBundle\Request\Executor;
 use Overblog\GraphQLBundle\Validator\ValidatorFactory;
 use Symfony\Component\Config\FileLocator;
@@ -89,8 +88,10 @@ private function registerForAutoconfiguration(ContainerBuilder $container): void
     {
         $container->registerForAutoconfiguration(MutationInterface::class)
             ->addTag('overblog_graphql.mutation');
+
         $container->registerForAutoconfiguration(ResolverInterface::class)
             ->addTag('overblog_graphql.resolver');
+
         $container->registerForAutoconfiguration(Type::class)
             ->addTag('overblog_graphql.type');
     }
@@ -235,41 +236,36 @@ private function setSchemaBuilderArguments(array $config, ContainerBuilder $cont
 
     private function setSchemaArguments(array $config, ContainerBuilder $container): void
     {
-        if (isset($config['definitions']['schema'])) {
-            $executorDefinition = $container->getDefinition(Executor::class);
-            $typeDecoratorListenerDefinition = $container->getDefinition(TypeDecoratorListener::class);
-
-            foreach ($config['definitions']['schema'] as $schemaName => $schemaConfig) {
-                // builder
-                $schemaBuilderID = \sprintf('%s.schema_builder_%s', $this->getAlias(), $schemaName);
-                $definition = $container->register($schemaBuilderID, \Closure::class);
-                $definition->setFactory([new Reference('overblog_graphql.schema_builder'), 'getBuilder']);
-                $definition->setArguments([
-                    $schemaName,
-                    $schemaConfig['query'],
-                    $schemaConfig['mutation'],
-                    $schemaConfig['subscription'],
-                    $schemaConfig['types'],
-                ]);
-                // schema
-                $schemaID = \sprintf('%s.schema_%s', $this->getAlias(), $schemaName);
-                $definition = $container->register($schemaID, Schema::class);
-                $definition->setFactory([new Reference($schemaBuilderID), 'call']);
-
-                if (!empty($schemaConfig['resolver_maps'])) {
-                    $typeDecoratorListenerDefinition->addMethodCall(
-                        'addSchemaResolverMaps',
-                        [
-                            $schemaName,
-                            \array_map(function ($id) {
-                                return new Reference($id);
-                            }, $schemaConfig['resolver_maps']),
-                        ]
-                    );
-                }
-                $executorDefinition->addMethodCall('addSchemaBuilder', [$schemaName, new Reference($schemaBuilderID)]);
-            }
+        if (!isset($config['definitions']['schema'])) {
+            return;
         }
+
+        $executorDefinition = $container->getDefinition(Executor::class);
+        $resolverMapsBySchema = [];
+
+        foreach ($config['definitions']['schema'] as $schemaName => $schemaConfig) {
+            // builder
+            $schemaBuilderID = \sprintf('%s.schema_builder_%s', $this->getAlias(), $schemaName);
+            $definition = $container->register($schemaBuilderID, \Closure::class);
+            $definition->setFactory([new Reference('overblog_graphql.schema_builder'), 'getBuilder']);
+            $definition->setArguments([
+                $schemaName,
+                $schemaConfig['query'],
+                $schemaConfig['mutation'],
+                $schemaConfig['subscription'],
+                $schemaConfig['types'],
+            ]);
+            // schema
+            $schemaID = \sprintf('%s.schema_%s', $this->getAlias(), $schemaName);
+            $definition = $container->register($schemaID, Schema::class);
+            $definition->setFactory([new Reference($schemaBuilderID), 'call']);
+
+            $executorDefinition->addMethodCall('addSchemaBuilder', [$schemaName, new Reference($schemaBuilderID)]);
+
+            $resolverMapsBySchema[$schemaName] = \array_fill_keys($schemaConfig['resolver_maps'], 0);
+        }
+
+        $container->setParameter(\sprintf('%s.resolver_maps', $this->getAlias()), $resolverMapsBySchema);
     }
 
     private function setServicesAliases(array $config, ContainerBuilder $container): void
diff --git a/src/OverblogGraphQLBundle.php b/src/OverblogGraphQLBundle.php
@@ -10,6 +10,7 @@
 use Overblog\GraphQLBundle\DependencyInjection\Compiler\ExpressionFunctionPass;
 use Overblog\GraphQLBundle\DependencyInjection\Compiler\GlobalVariablesPass;
 use Overblog\GraphQLBundle\DependencyInjection\Compiler\MutationTaggedServiceMappingTaggedPass;
+use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverMapTaggedServiceMappingPass;
 use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverMethodAliasesPass;
 use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverTaggedServiceMappingPass;
 use Overblog\GraphQLBundle\DependencyInjection\Compiler\TypeGeneratorPass;
@@ -43,6 +44,7 @@ public function build(ContainerBuilder $container): void
         $container->addCompilerPass(new ExpressionFunctionPass());
         $container->addCompilerPass(new ResolverMethodAliasesPass());
         $container->addCompilerPass(new AliasedPass());
+        $container->addCompilerPass(new ResolverMapTaggedServiceMappingPass());
         $container->addCompilerPass(new TypeGeneratorPass(), PassConfig::TYPE_BEFORE_REMOVING);
         $container->addCompilerPass(new TypeTaggedServiceMappingPass(), PassConfig::TYPE_BEFORE_REMOVING);
         $container->addCompilerPass(new ResolverTaggedServiceMappingPass(), PassConfig::TYPE_BEFORE_REMOVING);
diff --git a/tests/DependencyInjection/Compiler/ResolverMapTaggedServiceMappingPassTest.php b/tests/DependencyInjection/Compiler/ResolverMapTaggedServiceMappingPassTest.php
@@ -0,0 +1,126 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Overblog\GraphQLBundle\Tests\DependencyInjection\Compiler;
+
+use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverMapTaggedServiceMappingPass;
+use Overblog\GraphQLBundle\EventListener\TypeDecoratorListener;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+use Symfony\Component\DependencyInjection\Reference;
+
+final class ResolverMapTaggedServiceMappingPassTest extends TestCase
+{
+    public function testProcess(): void
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('overblog_graphql.resolver_maps', [
+            'foo' => [],
+            'bar' => [],
+        ]);
+
+        $container->register(TypeDecoratorListener::class);
+        $container->register('App\\GraphQl\\Resolver\\ResolverMap1')
+            ->addTag('overblog_graphql.resolver_map', ['schema' => 'foo']);
+
+        $container->register('App\\GraphQl\\Resolver\\ResolverMap2')
+            ->addTag('overblog_graphql.resolver_map', ['schema' => 'foo', 'priority' => 10])
+            ->addTag('overblog_graphql.resolver_map', ['schema' => 'bar', 'priority' => -10]);
+
+        $container->register('App\\GraphQl\\Resolver\\ResolverMap3')
+            ->addTag('overblog_graphql.resolver_map', ['schema' => 'foo', 'priority' => -10])
+            ->addTag('overblog_graphql.resolver_map', ['schema' => 'bar', 'priority' => 10]);
+
+        $container->register('App\\GraphQl\\Resolver\\ResolverMap4')
+            ->addTag('overblog_graphql.resolver_map', ['schema' => 'bar']);
+
+        (new ResolverMapTaggedServiceMappingPass())->process($container);
+
+        $typeDecoratorListenerDefinition = $container->getDefinition(TypeDecoratorListener::class);
+
+        $methodCalls = $typeDecoratorListenerDefinition->getMethodCalls();
+
+        $this->assertCount(2, $methodCalls);
+
+        $this->assertSame('addSchemaResolverMaps', $methodCalls[0][0]);
+        $this->assertSame('foo', $methodCalls[0][1][0]);
+        $this->assertEquals([
+            new Reference('App\\GraphQl\\Resolver\\ResolverMap2'),
+            new Reference('App\\GraphQl\\Resolver\\ResolverMap1'),
+            new Reference('App\\GraphQl\\Resolver\\ResolverMap3'),
+        ], $methodCalls[0][1][1]);
+
+        $this->assertSame('addSchemaResolverMaps', $methodCalls[1][0]);
+        $this->assertSame('bar', $methodCalls[1][1][0]);
+        $this->assertEquals([
+            new Reference('App\\GraphQl\\Resolver\\ResolverMap3'),
+            new Reference('App\\GraphQl\\Resolver\\ResolverMap4'),
+            new Reference('App\\GraphQl\\Resolver\\ResolverMap2'),
+        ], $methodCalls[1][1][1]);
+    }
+
+    public function testProcessThrowsIfSchemaAttributeIsNotDefinedOnTag(): void
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('overblog_graphql.resolver_maps', []);
+
+        $container->register(TypeDecoratorListener::class);
+        $container->register('App\\GraphQl\\Resolver\\ResolverMap')
+            ->addTag('overblog_graphql.resolver_map');
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('The "schema" attribute on the "overblog_graphql.resolver_map" tag of the "App\\GraphQl\\Resolver\\ResolverMap" service is required.');
+
+        (new ResolverMapTaggedServiceMappingPass())->process($container);
+    }
+
+    public function testProcessWithResolverMapBothTaggedAndInConfigDoesNotAddItTwice(): void
+    {
+        $container = new ContainerBuilder();
+        $container->setParameter('overblog_graphql.resolver_maps', [
+            'foo' => [
+                'App\\GraphQl\\Resolver\\ResolverMap1' => -10,
+                'App\\GraphQl\\Resolver\\ResolverMap2' => 0,
+            ],
+        ]);
+
+        $container->register(TypeDecoratorListener::class);
+        $container->register('App\\GraphQl\\Resolver\\ResolverMap1')
+            ->addTag('overblog_graphql.resolver_map', ['schema' => 'foo', 'priority' => 10]);
+
+        $container->register('App\\GraphQl\\Resolver\\ResolverMap2');
+
+        (new ResolverMapTaggedServiceMappingPass())->process($container);
+
+        $typeDecoratorListenerDefinition = $container->getDefinition(TypeDecoratorListener::class);
+
+        $methodCalls = $typeDecoratorListenerDefinition->getMethodCalls();
+
+        $this->assertCount(1, $methodCalls);
+        $this->assertSame('foo', $methodCalls[0][1][0]);
+        $this->assertEquals([
+            new Reference('App\\GraphQl\\Resolver\\ResolverMap1'),
+            new Reference('App\\GraphQl\\Resolver\\ResolverMap2'),
+        ], $methodCalls[0][1][1]);
+    }
+
+    public function testProcessThrowsIfTagReferencesUnknownSchema(): void
+    {
+        $container = new ContainerBuilder();
+        $container->register(TypeDecoratorListener::class);
+        $container->setParameter('overblog_graphql.resolver_maps', [
+            'foo' => [],
+            'bar' => [],
+        ]);
+
+        $container->register('App\\GraphQl\\Resolver\\ResolverMap')
+            ->addTag('overblog_graphql.resolver_map', ['schema' => 'baz']);
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('Service "App\\GraphQl\\Resolver\\ResolverMap" is invalid: schema "baz" specified on the tag "overblog_graphql.resolver_map" does not exist (known ones are: "foo", "bar").');
+
+        (new ResolverMapTaggedServiceMappingPass())->process($container);
+    }
+}
diff --git a/tests/Functional/App/config/schemaLanguage/config.yml b/tests/Functional/App/config/schemaLanguage/config.yml
