From 3e07b7b0d1b3eec6622b51af83e9328f66c392c0 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 29 Apr 2021 10:37:42 +0200 Subject: [PATCH] Keep original accessibility option for services tagged as resolver allow resolver solution to be private service --- .../Compiler/TaggedServiceMappingPass.php | 5 +- src/Resolver/AbstractResolver.php | 65 ++----------------- src/Resolver/FluentResolverInterface.php | 10 +-- .../ResolverTaggedServiceMappingPassTest.php | 1 + tests/Resolver/AbstractProxyResolverTest.php | 2 +- tests/Resolver/TypeResolverTest.php | 19 +----- 6 files changed, 15 insertions(+), 87 deletions(-) diff --git a/src/DependencyInjection/Compiler/TaggedServiceMappingPass.php b/src/DependencyInjection/Compiler/TaggedServiceMappingPass.php index a99a4e1cf..89e7c84d8 100644 --- a/src/DependencyInjection/Compiler/TaggedServiceMappingPass.php +++ b/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] ); } } diff --git a/src/Resolver/AbstractResolver.php b/src/Resolver/AbstractResolver.php index 1ac53e8c7..b0deb7a54 100644 --- a/src/Resolver/AbstractResolver.php +++ b/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; - } } diff --git a/src/Resolver/FluentResolverInterface.php b/src/Resolver/FluentResolverInterface.php index ad73a6a30..d3021dda5 100644 --- a/src/Resolver/FluentResolverInterface.php +++ b/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 diff --git a/tests/DependencyInjection/Compiler/ResolverTaggedServiceMappingPassTest.php b/tests/DependencyInjection/Compiler/ResolverTaggedServiceMappingPassTest.php index 3d0988672..de5a55c26 100644 --- a/tests/DependencyInjection/Compiler/ResolverTaggedServiceMappingPassTest.php +++ b/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', ]); diff --git a/tests/Resolver/AbstractProxyResolverTest.php b/tests/Resolver/AbstractProxyResolverTest.php index ddc3eaf5b..5a268edc2 100644 --- a/tests/Resolver/AbstractProxyResolverTest.php +++ b/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'], ]; } diff --git a/tests/Resolver/TypeResolverTest.php b/tests/Resolver/TypeResolverTest.php index 396c55b9d..9d8560724 100644 --- a/tests/Resolver/TypeResolverTest.php +++ b/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');