Merge pull request #837 from mcg-web/unchange-service-accessible

Keep original accessibility option for services tagged as resolver

Author: mcg-web

src/DependencyInjection/Compiler/TaggedServiceMappingPass.php

@@ -5,6 +5,7 @@
 namespace Overblog\GraphQLBundle\DependencyInjection\Compiler;
 
 use InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerAwareInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -63,13 +64,11 @@ public function process(ContainerBuilder $container): void
             $serviceID = $attributes['id'];
 
             $solutionDefinition = $container->findDefinition($serviceID);
-            // make solution service public to improve lazy loading
-            $solutionDefinition->setPublic(true);
             $this->autowireSolutionImplementingContainerAwareInterface($solutionDefinition, empty($attributes['generated']));
 
             $resolverDefinition->addMethodCall(
                 'addSolution',
-                [$solutionID, [[new Reference('service_container'), 'get'], [$serviceID]], $aliases, $attributes]
+                [$solutionID, new ServiceClosureArgument(new Reference($serviceID)), $aliases, $attributes]
             );
         }
     }

src/Resolver/AbstractResolver.php

@@ -4,13 +4,7 @@
 
 namespace Overblog\GraphQLBundle\Resolver;
 
-use GraphQL\Type\Definition\Type;
 use function array_keys;
-use function call_user_func_array;
-use function get_class;
-use function is_array;
-use function is_callable;
-use function sprintf;
 
 abstract class AbstractResolver implements FluentResolverInterface
 {
@@ -19,23 +13,12 @@ abstract class AbstractResolver implements FluentResolverInterface
     private array $solutionOptions = [];
     private array $fullyLoadedSolutions = [];
 
-    public function addSolution(string $id, $solutionOrFactory, array $aliases = [], array $options = []): self
+    public function addSolution(string $id, callable $factory, array $aliases = [], array $options = []): self
     {
         $this->fullyLoadedSolutions[$id] = false;
         $this->addAliases($id, $aliases);
 
-        $this->solutions[$id] = function () use ($id, $solutionOrFactory) {
-            $solution = $solutionOrFactory;
-            if (self::isSolutionFactory($solutionOrFactory)) {
-                if (!isset($solutionOrFactory[1])) {
-                    $solutionOrFactory[1] = [];
-                }
-                $solution = call_user_func_array(...$solutionOrFactory);
-            }
-            $this->checkSolution($id, $solution);
-
-            return $solution;
-        };
+        $this->solutions[$id] = $factory;
         $this->solutionOptions[$id] = $options;
 
         return $this;
@@ -78,7 +61,7 @@ public function getSolutionOptions(string $id)
     {
         $id = $this->resolveAlias($id);
 
-        return isset($this->solutionOptions[$id]) ? $this->solutionOptions[$id] : [];
+        return $this->solutionOptions[$id] ?? [];
     }
 
     /**
@@ -117,17 +100,9 @@ private function addAliases(string $id, array $aliases): void
         }
     }
 
-    /**
-     * @param object|array $solutionOrFactory
-     */
-    private static function isSolutionFactory($solutionOrFactory): bool
-    {
-        return is_array($solutionOrFactory) && isset($solutionOrFactory[0]) && is_callable($solutionOrFactory[0]);
-    }
-
     private function resolveAlias(string $alias): string
     {
-        return isset($this->aliases[$alias]) ? $this->aliases[$alias] : $alias;
+        return $this->aliases[$alias] ?? $alias;
     }
 
     /**
@@ -141,36 +116,4 @@ private function loadSolutions(): array
 
         return $this->solutions;
     }
-
-    /**
-     * @param mixed $solution
-     */
-    protected function supportsSolution($solution): bool
-    {
-        $supportedClass = $this->supportedSolutionClass();
-
-        return  null === $supportedClass || $solution instanceof $supportedClass;
-    }
-
-    /**
-     * @param mixed $solution
-     */
-    protected function checkSolution(string $id, $solution): void
-    {
-        if (!$this->supportsSolution($solution)) {
-            throw new UnsupportedResolverException(
-                sprintf('Resolver "%s" must be "%s" "%s" given.', $id, $this->supportedSolutionClass(), get_class($solution))
-            );
-        }
-    }
-
-    /**
-     * default return null to accept mixed type.
-     *
-     * @return string|null supported class name
-     */
-    protected function supportedSolutionClass(): ?string
-    {
-        return null;
-    }
 }

src/Resolver/FluentResolverInterface.php

@@ -16,14 +16,14 @@ public function resolve($input);
     /**
      * Add a solution to resolver.
      *
-     * @param string      $id                the solution identifier
-     * @param array|mixed $solutionOrFactory the solution itself or array with a factory and it arguments if needed [$factory] or [$factory, $factoryArgs]
-     * @param string[]    $aliases           the solution aliases
-     * @param array       $options           extra options
+     * @param string   $id      the solution identifier
+     * @param callable $factory the solution factory
+     * @param string[] $aliases the solution aliases
+     * @param array    $options extra options
      *
      * @return $this
      */
-    public function addSolution(string $id, $solutionOrFactory, array $aliases = [], array $options = []): self;
+    public function addSolution(string $id, callable $factory, array $aliases = [], array $options = []): self;
 
     /**
      * @return mixed

tests/DependencyInjection/Compiler/ResolverTaggedServiceMappingPassTest.php

@@ -40,6 +40,7 @@ public function testCompilationWorksPassConfigDirective(): void
     {
         $testResolver = new Definition(ResolverTestService::class);
         $testResolver
+            ->setPublic(true)
             ->addTag('overblog_graphql.query', [
                 'alias' => 'test_resolver', 'method' => 'doSomethingWithContainer',
             ]);

tests/Resolver/AbstractProxyResolverTest.php

@@ -12,7 +12,7 @@ abstract class AbstractProxyResolverTest extends AbstractResolverTest
     protected function getResolverSolutionsMapping(): array
     {
         return [
-            'Toto' => ['factory' => [[$this, 'createToto'], []], 'aliases' => ['foo', 'bar', 'baz'], 'method' => 'resolve'],
+            'Toto' => ['factory' => [$this, 'createToto'], 'aliases' => ['foo', 'bar', 'baz'], 'method' => 'resolve'],
         ];
     }
 

tests/Resolver/TypeResolverTest.php

@@ -5,12 +5,8 @@
 namespace Overblog\GraphQLBundle\Tests\Resolver;
 
 use GraphQL\Type\Definition\ObjectType;
-use GraphQL\Type\Definition\Type;
 use Overblog\GraphQLBundle\Resolver\TypeResolver;
 use Overblog\GraphQLBundle\Resolver\UnresolvableException;
-use Overblog\GraphQLBundle\Resolver\UnsupportedResolverException;
-use stdClass;
-use function sprintf;
 
 class TypeResolverTest extends AbstractResolverTest
 {
@@ -22,8 +18,8 @@ protected function createResolver(): TypeResolver
     protected function getResolverSolutionsMapping(): array
     {
         return [
-            'Toto' => ['factory' => [[$this, 'createObjectType'], [['name' => 'Toto']]], 'aliases' => ['foo']],
-            'Tata' => ['factory' => [[$this, 'createObjectType'], [['name' => 'Tata']]], 'aliases' => ['bar']],
+            'Toto' => ['factory' => fn () => $this->createObjectType(['name' => 'Toto']), 'aliases' => ['foo']],
+            'Tata' => ['factory' => fn () => $this->createObjectType(['name' => 'Tata']), 'aliases' => ['bar']],
         ];
     }
 
@@ -32,17 +28,6 @@ public function createObjectType(array $config): ObjectType
         return new ObjectType($config);
     }
 
-    public function testAddNotSupportedSolution(): void
-    {
-        $this->expectException(UnsupportedResolverException::class);
-        $this->expectExceptionMessage(sprintf(
-            'Resolver "not-supported" must be "%s" "stdClass" given.',
-            Type::class
-        ));
-        $this->resolver->addSolution('not-supported', new stdClass());
-        $this->resolver->getSolution('not-supported');
-    }
-
     public function testResolveKnownType(): void
     {
         $type = $this->resolver->resolve('Toto');