Improve GraphQLServices by making it a ServiceLocator

Author: mcg-web

src/Definition/GraphQLServices.php

@@ -6,67 +6,22 @@
 
 use GraphQL\Type\Definition\ResolveInfo;
 use GraphQL\Type\Definition\Type;
-use LogicException;
-use Overblog\GraphQLBundle\Resolver\MutationResolver;
-use Overblog\GraphQLBundle\Resolver\QueryResolver;
-use Overblog\GraphQLBundle\Resolver\TypeResolver;
 use Overblog\GraphQLBundle\Validator\InputValidator;
+use Symfony\Component\DependencyInjection\ServiceLocator;
 
 /**
  * Container for special services to be passed to all generated types.
  */
-final class GraphQLServices
+final class GraphQLServices extends ServiceLocator
 {
-    private array $services;
-    private TypeResolver $types;
-    private QueryResolver $queryResolver;
-    private MutationResolver $mutationResolver;
-
-    public function __construct(
-        TypeResolver $typeResolver,
-        QueryResolver $queryResolver,
-        MutationResolver $mutationResolver,
-        array $services = []
-    ) {
-        $this->types = $typeResolver;
-        $this->queryResolver = $queryResolver;
-        $this->mutationResolver = $mutationResolver;
-        $this->services = $services;
-    }
-
-    /**
-     * @return mixed
-     */
-    public function get(string $name)
-    {
-        if (!isset($this->services[$name])) {
-            throw new LogicException(sprintf('GraphQL service "%s" could not be located. You should define it.', $name));
-        }
-
-        return $this->services[$name];
-    }
-
-    /**
-     * Get all GraphQL services.
-     */
-    public function getAll(): array
-    {
-        return $this->services;
-    }
-
-    public function has(string $name): bool
-    {
-        return isset($this->services[$name]);
-    }
-
     /**
      * @param mixed ...$args
      *
      * @return mixed
      */
     public function query(string $alias, ...$args)
     {
-        return $this->queryResolver->resolve([$alias, $args]);
+        return $this->get('queryResolver')->resolve([$alias, $args]);
     }
 
     /**
@@ -76,12 +31,12 @@ public function query(string $alias, ...$args)
      */
     public function mutation(string $alias, ...$args)
     {
-        return $this->mutationResolver->resolve([$alias, $args]);
+        return $this->get('mutationResolver')->resolve([$alias, $args]);
     }
 
     public function getType(string $typeName): ?Type
     {
-        return $this->types->resolve($typeName);
+        return $this->get('typeResolver')->resolve($typeName);
     }
 
     /**
@@ -92,7 +47,7 @@ public function getType(string $typeName): ?Type
      */
     public function createInputValidator($value, ArgumentInterface $args, $context, ResolveInfo $info): InputValidator
     {
-        return $this->services['input_validator_factory']->create(
+        return $this->get('input_validator_factory')->create(
             new ResolverArgs($value, $args, $context, $info)
         );
     }

src/DependencyInjection/Compiler/GraphQLServicesPass.php

@@ -32,7 +32,7 @@ public function process(ContainerBuilder $container): void
             $taggedServices = array_merge($taggedServices, $deprecatedTaggedServices);
         }
 
-        $serviceContainer = ['container' => new Reference('service_container')];
+        $locateableServices = [];
         $expressionLanguageDefinition = $container->findDefinition('overblog_graphql.expression_language');
 
         foreach ($taggedServices as $id => $tags) {
@@ -42,9 +42,9 @@ public function process(ContainerBuilder $container): void
                         sprintf('Service "%s" tagged "overblog_graphql.service" should have a valid "alias" attribute.', $id)
                     );
                 }
-                $serviceContainer[$attributes['alias']] = new Reference($id);
+                $locateableServices[$attributes['alias']] = new Reference($id);
 
-                $isPublic = isset($attributes['public']) ? (bool) $attributes['public'] : true;
+                $isPublic = !isset($attributes['public']) || $attributes['public'];
                 if ($isPublic) {
                     $expressionLanguageDefinition->addMethodCall(
                         'addGlobalName',
@@ -56,6 +56,8 @@ public function process(ContainerBuilder $container): void
                 }
             }
         }
-        $container->findDefinition(GraphQLServices::class)->addArgument($serviceContainer);
+        $locateableServices['container'] = new Reference('service_container');
+
+        $container->findDefinition(GraphQLServices::class)->addArgument($locateableServices);
     }
 }

src/ExpressionLanguage/ExpressionLanguage.php

@@ -27,6 +27,11 @@ public function addGlobalName(string $index, string $name): void
         $this->globalNames[$index] = $name;
     }
 
+    public function getGlobalNames(): array
+    {
+        return array_values($this->globalNames);
+    }
+
     /**
      * @param string|Expression $expression
      * @param array             $names
@@ -50,6 +55,22 @@ public function compile($expression, $names = [])
      * @throws SyntaxError
      */
     public static function expressionContainsVar(string $name, $expression): bool
+    {
+        foreach (static::extractExpressionVarNames($expression) as $varName) {
+            if ($name === $varName) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * @param string|Expression $expression - expression to search in (haystack)
+     *
+     * @throws SyntaxError
+     */
+    public static function extractExpressionVarNames($expression): iterable
     {
         if ($expression instanceof Expression) {
             $expression = $expression->__toString();
@@ -60,22 +81,30 @@ public static function expressionContainsVar(string $name, $expression): bool
         /** @var string $expression */
         $stream = (new Lexer())->tokenize($expression);
         $current = &$stream->current;
+        $isProperty = false;
+        $varNames = [];
 
         while (!$stream->isEOF()) {
-            if ($name === $current->value && Token::NAME_TYPE === $current->type) {
-                // Also check that it's not a function's name
-                $stream->next();
-                if ('(' !== $current->value) {
-                    $contained = true;
-                    break;
+            if ('.' === $current->value) {
+                $isProperty = true;
+            } elseif (Token::NAME_TYPE === $current->type) {
+                if (!$isProperty) {
+                    $name = $current->value;
+                    // Also check that it's not a function's name
+                    $stream->next();
+                    if ('(' !== $current->value) {
+                        $varNames[] = $name;
+                    }
+                    continue;
+                } else {
+                    $isProperty = false;
                 }
-                continue;
             }
 
             $stream->next();
         }
 
-        return $contained ?? false;
+        return $varNames;
     }
 
     /**

src/Resources/config/services.yaml

@@ -10,10 +10,7 @@ services:
     Overblog\GraphQLBundle\Resolver\FieldResolver: ~
 
     Overblog\GraphQLBundle\Definition\GraphQLServices:
-        arguments:
-            - '@Overblog\GraphQLBundle\Resolver\TypeResolver'
-            - '@Overblog\GraphQLBundle\Resolver\QueryResolver'
-            - '@Overblog\GraphQLBundle\Resolver\MutationResolver'
+        tags: ['container.service_locator']
 
     Overblog\GraphQLBundle\Request\Executor:
         arguments:
@@ -48,7 +45,12 @@ services:
             - "%overblog_graphql_types.classes_map%"
 
     Overblog\GraphQLBundle\Resolver\QueryResolver:
+        tags:
+            - { name: overblog_graphql.service, alias: queryResolver }
+
     Overblog\GraphQLBundle\Resolver\MutationResolver:
+        tags:
+            - { name: overblog_graphql.service, alias: mutationResolver }
 
     Overblog\GraphQLBundle\Resolver\AccessResolver:
         arguments:

src/Validator/Constraints/ExpressionValidator.php

@@ -5,9 +5,9 @@
 namespace Overblog\GraphQLBundle\Validator\Constraints;
 
 use Overblog\GraphQLBundle\Definition\GraphQLServices;
+use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionLanguage;
 use Overblog\GraphQLBundle\Generator\TypeGenerator;
 use Overblog\GraphQLBundle\Validator\ValidationNode;
-use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
 use Symfony\Component\HttpKernel\Kernel;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\Constraints\Expression;
@@ -23,10 +23,10 @@ public function __construct(ExpressionLanguage $expressionLanguage, GraphQLServi
     {
         $this->expressionLanguage = $expressionLanguage;
         $this->graphQLServices = $graphQLServices;
-        if (Kernel::VERSION_ID >= 40400) {  // @phpstan-ignore-line
-            parent::__construct($expressionLanguage);
-        } else {                            // @phpstan-ignore-line
-            parent::{'__construct'}(null, $expressionLanguage);
+        if (Kernel::VERSION_ID >= 40400) {
+            parent::__construct($expressionLanguage); // @phpstan-ignore-line
+        } else {
+            parent::__construct(null, $expressionLanguage); // @phpstan-ignore-line
         }
     }
 
@@ -47,21 +47,40 @@ public function validate($value, Constraint $constraint): void
 
         $variables['this'] = $object;
 
-        // Make all tagged GraphQL services available in the expression constraint
-        $variables = array_merge($variables, $this->graphQLServices->getAll());
-
         if ($object instanceof ValidationNode) {
             $variables['parentValue'] = $object->getResolverArg('value');
             $variables['context'] = $object->getResolverArg('context');
             $variables['args'] = $object->getResolverArg('args');
             $variables['info'] = $object->getResolverArg('info');
         }
 
+        // Make all tagged GraphQL public services available in the expression constraint
+        $this->addGlobalVariables($constraint->expression, $variables);
+
         if (!$this->expressionLanguage->evaluate($constraint->expression, $variables)) {
             $this->context->buildViolation($constraint->message)
                           ->setParameter('{{ value }}', $this->formatValue($value, self::OBJECT_TO_STRING))
                           ->setCode(Expression::EXPRESSION_FAILED_ERROR)
                           ->addViolation();
         }
     }
+
+    /**
+     * @param string|\Symfony\Component\ExpressionLanguage\Expression $expression
+     */
+    private function addGlobalVariables($expression, array &$variables): void
+    {
+        $globalVariables = $this->expressionLanguage->getGlobalNames();
+        foreach (ExpressionLanguage::extractExpressionVarNames($expression) as $extractExpressionVarName) {
+            if (
+                isset($variables[$extractExpressionVarName])
+                || !$this->graphQLServices->has($extractExpressionVarName)
+                || !in_array($extractExpressionVarName, $globalVariables)
+            ) {
+                continue;
+            }
+
+            $variables[$extractExpressionVarName] = $this->graphQLServices->get($extractExpressionVarName);
+        }
+    }
 }

tests/Definition/GraphQLServicesTest.php

@@ -4,32 +4,55 @@
 
 namespace Overblog\GraphQLBundle\Tests\ExpressionLanguage;
 
-use Generator;
 use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionLanguage;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\ExpressionLanguage\Expression;
 
 class ExpressionLanguageTest extends TestCase
 {
     /**
-     * @test
-     * @dataProvider expressionProvider
+     * @dataProvider expressionContainsVarProvider
      *
      * @param Expression|string $expression
      */
-    public function expressionContainsVar($expression, bool $expectedResult): void
+    public function testExpressionContainsVar($expression, bool $expectedResult): void
     {
         $result = ExpressionLanguage::expressionContainsVar('validator', $expression);
 
-        $this->assertEquals($result, $expectedResult);
+        $this->assertSame($expectedResult, $result);
     }
 
-    public function expressionProvider(): Generator
+    /**
+     * @dataProvider extractExpressionVarNamesProvider
+     *
+     * @param Expression|string $expression
+     */
+    public function testExtractExpressionVarNames($expression, array $expectedResult): void
+    {
+        $result = ExpressionLanguage::extractExpressionVarNames($expression);
+
+        $this->assertSame($expectedResult, $result);
+    }
+
+    public function expressionContainsVarProvider(): iterable
     {
         yield ["@=test('default', 15.6, validator)", true];
         yield ["@=validator('default', 15.6)", false];
         yield ["validator('default', validator(), 15.6)", false];
         yield [new Expression("validator('default', validator(), 15.6)"), false];
         yield [new Expression('validator'), true];
+        yield ['toto.validator', false];
+        yield ['toto . validator', false];
+        yield ['toto.test && validator', true];
+        yield ['toto . test && validator', true];
+    }
+
+    public function extractExpressionVarNamesProvider(): iterable
+    {
+        yield ['@=a + b - c', ['a', 'b', 'c']];
+        yield ['f()', []];
+        yield ['a.c + b', ['a', 'b']];
+        yield ['(a.c) + b - d', ['a', 'b', 'd']];
+        yield['a && b && c', ['a', 'b', 'c']];
     }
 }