From bee1eb3565fa81d4ee5aa070a0a08e4352f8e9e2 Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 12:18:58 +0100 Subject: [PATCH 1/9] Initial experimentation in LazyTypeResolution strategy --- Definition/Builder/SchemaBuilder.php | 38 +++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/Definition/Builder/SchemaBuilder.php b/Definition/Builder/SchemaBuilder.php index cbf4c7234..d47b18abd 100644 --- a/Definition/Builder/SchemaBuilder.php +++ b/Definition/Builder/SchemaBuilder.php @@ -13,6 +13,7 @@ use GraphQL\Schema; use GraphQL\Type\Definition\Config; +use GraphQL\Type\LazyResolution; use Overblog\GraphQLBundle\Resolver\ResolverInterface; class SchemaBuilder @@ -46,11 +47,42 @@ public function create($queryAlias = null, $mutationAlias = null, $subscriptionA $mutation = $this->typeResolver->resolve($mutationAlias); $subscription = $this->typeResolver->resolve($subscriptionAlias); - return new Schema([ + $config = [ 'query' => $query, 'mutation' => $mutation, 'subscription' => $subscription, - 'types' => $this->typeResolver->getSolutions(), - ]); + 'types' => $this->typeResolver->getSolutions() + ]; + + $descriptorFile = __DIR__.'/../../../../../var/cache/dev/overblog/graph-bundle/schema-descriptor.php'; + if (file_exists($descriptorFile)) { + $descriptor = include $descriptorFile; + + $config['typeResolution'] = new LazyResolution($descriptor, function($typeName) { + $classNames = [ + "Overblog\\GraphQLBundle\\__DEFINITIONS__\\{$typeName}Type", + "GraphQL\\Type\\Definition\\{$typeName}Type" + ]; + + foreach ($classNames as $className) { + if (class_exists($className)) { + return new $className; + } + } + + return null; + }); + } + + $schema = new Schema($config); + + if (! file_exists($descriptorFile)) { + file_put_contents( + $descriptorFile, + "getDescriptor(), true) . ';' + ); + } + + return $schema; } } From 244e6bea134ace9e2d1b1be3a1d9a9cd4c60779a Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 12:20:12 +0100 Subject: [PATCH 2/9] use overblog TypeResolver instead of manual callback --- Definition/Builder/SchemaBuilder.php | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/Definition/Builder/SchemaBuilder.php b/Definition/Builder/SchemaBuilder.php index d47b18abd..115427529 100644 --- a/Definition/Builder/SchemaBuilder.php +++ b/Definition/Builder/SchemaBuilder.php @@ -58,20 +58,7 @@ public function create($queryAlias = null, $mutationAlias = null, $subscriptionA if (file_exists($descriptorFile)) { $descriptor = include $descriptorFile; - $config['typeResolution'] = new LazyResolution($descriptor, function($typeName) { - $classNames = [ - "Overblog\\GraphQLBundle\\__DEFINITIONS__\\{$typeName}Type", - "GraphQL\\Type\\Definition\\{$typeName}Type" - ]; - - foreach ($classNames as $className) { - if (class_exists($className)) { - return new $className; - } - } - - return null; - }); + $config['typeResolution'] = new LazyResolution($descriptor, [$this->typeResolver, 'resolve']); } $schema = new Schema($config); From e7664c7fdd433a02ce0ca21493200682c1886538 Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 12:40:05 +0100 Subject: [PATCH 3/9] make the TypeResolver lazy --- .../Compiler/TaggedServiceMappingPass.php | 10 +------ Resolver/TypeResolver.php | 29 ++++++++++++++----- Resources/config/services.yml | 1 + 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/DependencyInjection/Compiler/TaggedServiceMappingPass.php b/DependencyInjection/Compiler/TaggedServiceMappingPass.php index f0db04fc0..c5c7dd3ef 100644 --- a/DependencyInjection/Compiler/TaggedServiceMappingPass.php +++ b/DependencyInjection/Compiler/TaggedServiceMappingPass.php @@ -40,15 +40,7 @@ public function process(ContainerBuilder $container) $resolverDefinition = $container->findDefinition($this->getResolverServiceID()); foreach ($mapping as $name => $options) { - $cleanOptions = $options; - $solutionID = $options['id']; - - $definition = $container->findDefinition($solutionID); - if (is_subclass_of($definition->getClass(), 'Symfony\Component\DependencyInjection\ContainerAwareInterface')) { - $solutionDefinition = $container->findDefinition($options['id']); - $solutionDefinition->addMethodCall('setContainer', [new Reference('service_container')]); - } - $resolverDefinition->addMethodCall('addSolution', [$name, new Reference($solutionID), $cleanOptions]); + $resolverDefinition->addMethodCall('addSolution', [$name, null, $options]); } } diff --git a/Resolver/TypeResolver.php b/Resolver/TypeResolver.php index f2890251a..1af0752e7 100644 --- a/Resolver/TypeResolver.php +++ b/Resolver/TypeResolver.php @@ -14,6 +14,7 @@ use GraphQL\Type\Definition\Type; use Overblog\GraphQLBundle\Resolver\Cache\ArrayCache; use Overblog\GraphQLBundle\Resolver\Cache\CacheInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; class TypeResolver extends AbstractResolver { @@ -22,9 +23,23 @@ class TypeResolver extends AbstractResolver */ private $cache; - public function __construct(CacheInterface $cache = null) - { + /** + * @var ContainerInterface + */ + private $container; + + /** + * LazyTypeResolver constructor. + * + * @param CacheInterface|null $cache + * @param ContainerInterface $container + */ + public function __construct( + CacheInterface $cache = null, + ContainerInterface $container + ) { $this->cache = null !== $cache ? $cache : new ArrayCache(); + $this->container = $container; } /** @@ -65,6 +80,11 @@ private function baseType($alias) return $type; } + $typeOptions = $this->getSolutionOptions($alias); + if ($typeOptions and $this->container->has($typeOptions['id'])) { + return $this->container->get($typeOptions['id']); + } + throw new UnresolvableException( sprintf('Unknown type with alias "%s" (verified service tag)', $alias) ); @@ -99,9 +119,4 @@ private function hasNeedListOfWrapper($alias) return false; } - - protected function supportedSolutionClass() - { - return 'GraphQL\\Type\\Definition\\Type'; - } } diff --git a/Resources/config/services.yml b/Resources/config/services.yml index ed11bd8f1..948a4a44c 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -45,6 +45,7 @@ services: class: Overblog\GraphQLBundle\Resolver\TypeResolver arguments: - + - "@service_container" overblog_graphql.resolver_resolver: class: Overblog\GraphQLBundle\Resolver\ResolverResolver From cad4155648c84496bbe4f2863e5d8129184ba9d4 Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 13:16:09 +0100 Subject: [PATCH 4/9] Load all types in SchemaBuilder when trying to build descriptor --- Definition/Builder/SchemaBuilder.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Definition/Builder/SchemaBuilder.php b/Definition/Builder/SchemaBuilder.php index 115427529..426232e98 100644 --- a/Definition/Builder/SchemaBuilder.php +++ b/Definition/Builder/SchemaBuilder.php @@ -51,7 +51,6 @@ public function create($queryAlias = null, $mutationAlias = null, $subscriptionA 'query' => $query, 'mutation' => $mutation, 'subscription' => $subscription, - 'types' => $this->typeResolver->getSolutions() ]; $descriptorFile = __DIR__.'/../../../../../var/cache/dev/overblog/graph-bundle/schema-descriptor.php'; @@ -59,6 +58,15 @@ public function create($queryAlias = null, $mutationAlias = null, $subscriptionA $descriptor = include $descriptorFile; $config['typeResolution'] = new LazyResolution($descriptor, [$this->typeResolver, 'resolve']); + } else { + $solutions = $this->typeResolver->getSolutions(); + $config['types'] = array_map(function($type, $typeName) { + if (! $type) { + return $this->typeResolver->resolve($typeName); + } + + return $type; + }, $solutions, array_keys($solutions)); } $schema = new Schema($config); From 7daa4c5eff07d7410f7a2f785177d81290cab05b Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 13:26:25 +0100 Subject: [PATCH 5/9] lazy load resolvers in overblog to avoid having special cases in TaggedServiceMappingPass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I see now that in fact there are many things that relu on this… - TypeResolver - ResolverResolver - MutationResolver - AccessResolver --- Resolver/ResolverResolver.php | 31 +++++++++++++++++++++++++++++++ Resources/config/services.yml | 2 ++ 2 files changed, 33 insertions(+) diff --git a/Resolver/ResolverResolver.php b/Resolver/ResolverResolver.php index 8e21d8275..0d0b969dc 100644 --- a/Resolver/ResolverResolver.php +++ b/Resolver/ResolverResolver.php @@ -11,8 +11,39 @@ namespace Overblog\GraphQLBundle\Resolver; +use Symfony\Component\DependencyInjection\ContainerInterface; + class ResolverResolver extends AbstractProxyResolver { + /** + * @var ContainerInterface + */ + private $container; + + /** + * ResolverResolver constructor. + * + * @param ContainerInterface $container + */ + public function __construct(ContainerInterface $container) + { + $this->container = $container; + } + + public function getSolution($name) + { + $solution = parent::getSolution($name); + + if (! $solution) { + $typeOptions = $this->getSolutionOptions($name); + if ($typeOptions and $this->container->has($typeOptions['id'])) { + $solution = $this->container->get($typeOptions['id']); + } + } + + return $solution; + } + protected function unresolvableMessage($alias) { return sprintf('Unknown resolver with alias "%s" (verified service tag)', $alias); diff --git a/Resources/config/services.yml b/Resources/config/services.yml index 948a4a44c..cddcf9204 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -49,6 +49,8 @@ services: overblog_graphql.resolver_resolver: class: Overblog\GraphQLBundle\Resolver\ResolverResolver + arguments: + - "@service_container" overblog_graphql.mutation_resolver: class: Overblog\GraphQLBundle\Resolver\MutationResolver From 0ea72509c306940fca74f3895b3cbc8d9347a148 Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 13:57:01 +0100 Subject: [PATCH 6/9] remove current request and user passing from every compiled type --- Generator/TypeGenerator.php | 2 +- Resources/config/services.yml | 1 - Resources/skeleton/TypeSystem.php.skeleton | 12 ------------ 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/Generator/TypeGenerator.php b/Generator/TypeGenerator.php index 64d4be590..aa02e34ef 100644 --- a/Generator/TypeGenerator.php +++ b/Generator/TypeGenerator.php @@ -17,7 +17,7 @@ class TypeGenerator extends BaseTypeGenerator { - const USE_FOR_CLOSURES = '$container, $request, $user, $token'; + const USE_FOR_CLOSURES = '$container'; private $cacheDir; diff --git a/Resources/config/services.yml b/Resources/config/services.yml index cddcf9204..b6f67add4 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -82,7 +82,6 @@ services: - "%overblog_graphql.default_resolver%" calls: - ["addUseStatement", ["Symfony\\Component\\DependencyInjection\\ContainerInterface"]] - - ["addUseStatement", ["Symfony\\Component\\Security\\Core\\Authentication\\Token\\TokenInterface"]] - ["setExpressionLanguage", ["@overblog_graphql.expression_language"]] overblog_graphql.event_listener.classloader_listener: diff --git a/Resources/skeleton/TypeSystem.php.skeleton b/Resources/skeleton/TypeSystem.php.skeleton index 6e80f9dc3..844795581 100644 --- a/Resources/skeleton/TypeSystem.php.skeleton +++ b/Resources/skeleton/TypeSystem.php.skeleton @@ -6,18 +6,6 @@ { public function __construct(ContainerInterface $container) { -$request = null; -$token = null; -$user = null; -if ($container->has('request_stack')) { -$request = $container->get('request_stack')->getCurrentRequest(); -} -if ($container->has('security.token_storage')) { -$token = $container->get('security.token_storage')->getToken(); -if ($token instanceof TokenInterface) { -$user = $token->getUser(); -} -} parent::__construct(); } From 4c027228bc158fb3b40b1bcff2dda297abe1a2b7 Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 15:46:01 +0100 Subject: [PATCH 7/9] add Schema descriptor to TypePass --- Definition/Builder/SchemaBuilder.php | 51 +++++++++------------- DependencyInjection/Compiler/TypesPass.php | 25 +++++++++++ Resources/config/services.yml | 1 + 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/Definition/Builder/SchemaBuilder.php b/Definition/Builder/SchemaBuilder.php index 426232e98..6ef069268 100644 --- a/Definition/Builder/SchemaBuilder.php +++ b/Definition/Builder/SchemaBuilder.php @@ -23,12 +23,29 @@ class SchemaBuilder */ private $typeResolver; + /** + * @var array + */ + private $schemaDescriptor; + /** @var bool */ private $enableValidation; - public function __construct(ResolverInterface $typeResolver, $enableValidation = false) + /** + * SchemaBuilder constructor. + * + * @param ResolverInterface $typeResolver + * @param array $schemaDescriptor + * @param bool $enableValidation + */ + public function __construct( + ResolverInterface $typeResolver, + array $schemaDescriptor, + $enableValidation = false + ) { $this->typeResolver = $typeResolver; + $this->schemaDescriptor = $schemaDescriptor; $this->enableValidation = $enableValidation; } @@ -47,37 +64,11 @@ public function create($queryAlias = null, $mutationAlias = null, $subscriptionA $mutation = $this->typeResolver->resolve($mutationAlias); $subscription = $this->typeResolver->resolve($subscriptionAlias); - $config = [ + return new Schema([ 'query' => $query, 'mutation' => $mutation, 'subscription' => $subscription, - ]; - - $descriptorFile = __DIR__.'/../../../../../var/cache/dev/overblog/graph-bundle/schema-descriptor.php'; - if (file_exists($descriptorFile)) { - $descriptor = include $descriptorFile; - - $config['typeResolution'] = new LazyResolution($descriptor, [$this->typeResolver, 'resolve']); - } else { - $solutions = $this->typeResolver->getSolutions(); - $config['types'] = array_map(function($type, $typeName) { - if (! $type) { - return $this->typeResolver->resolve($typeName); - } - - return $type; - }, $solutions, array_keys($solutions)); - } - - $schema = new Schema($config); - - if (! file_exists($descriptorFile)) { - file_put_contents( - $descriptorFile, - "getDescriptor(), true) . ';' - ); - } - - return $schema; + 'typeResolution' => new LazyResolution($this->schemaDescriptor, [$this->typeResolver, 'resolve']) + ]); } } diff --git a/DependencyInjection/Compiler/TypesPass.php b/DependencyInjection/Compiler/TypesPass.php index c56d3eee7..c53f331dd 100644 --- a/DependencyInjection/Compiler/TypesPass.php +++ b/DependencyInjection/Compiler/TypesPass.php @@ -35,6 +35,31 @@ public function process(ContainerBuilder $container) ->addTag('overblog_graphql.type', ['alias' => $name]) ; } + + $types = []; + $possibleTypes = []; + + foreach ($config as $typeConf) { + $typeName = $typeConf['config']['name']; + $types[$typeName] = 1; + if ($typeConf['type'] != 'object') { + continue; + } + + $ifaces = $typeConf['config']['interfaces'] ?? []; + foreach ($ifaces as $iface) { + if (!array_key_exists($iface, $possibleTypes)) { + $possibleTypes[$iface] = []; + } + $possibleTypes[$iface][$typeName] = 1; + } + } + + $container->getDefinition('overblog_graphql.schema_builder')->replaceArgument(1, [ + 'typeMap' => $types, + 'possibleTypeMap' => $possibleTypes, + 'version' => '1.0' + ]); } private function processConfig(array $configs) diff --git a/Resources/config/services.yml b/Resources/config/services.yml index b6f67add4..410aa4363 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -39,6 +39,7 @@ services: public: false arguments: - "@overblog_graphql.type_resolver" + - "%overblog_graphql.definition.lazy_types_descriptor%" - false overblog_graphql.type_resolver: From 6a9f6dfc8f4c72e9aee630bbb109a2c7ef86c2c1 Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 16:24:26 +0100 Subject: [PATCH 8/9] try to fix introspection using lazy resolution of types Sadly further layers of the stack requires modification but this is fine for now. --- DependencyInjection/Compiler/TypesPass.php | 11 ++++++++++- Resolver/TypeResolver.php | 6 ++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/DependencyInjection/Compiler/TypesPass.php b/DependencyInjection/Compiler/TypesPass.php index c53f331dd..30cd9b64d 100644 --- a/DependencyInjection/Compiler/TypesPass.php +++ b/DependencyInjection/Compiler/TypesPass.php @@ -36,7 +36,16 @@ public function process(ContainerBuilder $container) ; } - $types = []; + $types = [ + '__Schema' => 1, + '__Type' => 1, + '__TypeKind' => 1, + '__Field' => 1, + '__InputValue' => 1, + '__EnumValue' => 1, + '__Directive' => 1, + '__DirectiveLocation' => 1 + ]; $possibleTypes = []; foreach ($config as $typeConf) { diff --git a/Resolver/TypeResolver.php b/Resolver/TypeResolver.php index 1af0752e7..1b39ec120 100644 --- a/Resolver/TypeResolver.php +++ b/Resolver/TypeResolver.php @@ -12,6 +12,7 @@ namespace Overblog\GraphQLBundle\Resolver; use GraphQL\Type\Definition\Type; +use GraphQL\Type\Introspection; use Overblog\GraphQLBundle\Resolver\Cache\ArrayCache; use Overblog\GraphQLBundle\Resolver\Cache\CacheInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -49,6 +50,11 @@ public function __construct( */ public function resolve($alias) { + if (strpos($alias, '__') === 0) { + $staticName = '_'.lcfirst(substr($alias, 2)); + return Introspection::$staticName(); + } + if (null === $alias) { return; } From 5882faa51e90d2058602bc542b93c19ff2588513 Mon Sep 17 00:00:00 2001 From: Andreas Heiberg Date: Tue, 27 Jun 2017 17:21:08 +0100 Subject: [PATCH 9/9] support unions in TypePass descripter --- DependencyInjection/Compiler/TypesPass.php | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/DependencyInjection/Compiler/TypesPass.php b/DependencyInjection/Compiler/TypesPass.php index 30cd9b64d..81602b5a7 100644 --- a/DependencyInjection/Compiler/TypesPass.php +++ b/DependencyInjection/Compiler/TypesPass.php @@ -51,16 +51,20 @@ public function process(ContainerBuilder $container) foreach ($config as $typeConf) { $typeName = $typeConf['config']['name']; $types[$typeName] = 1; - if ($typeConf['type'] != 'object') { - continue; + if ($typeConf['type'] == 'object') { + $ifaces = $typeConf['config']['interfaces'] ?? []; + foreach ($ifaces as $iface) { + if (!array_key_exists($iface, $possibleTypes)) { + $possibleTypes[$iface] = []; + } + $possibleTypes[$iface][$typeName] = 1; + } } - - $ifaces = $typeConf['config']['interfaces'] ?? []; - foreach ($ifaces as $iface) { - if (!array_key_exists($iface, $possibleTypes)) { - $possibleTypes[$iface] = []; + if ($typeConf['type'] == 'union') { + $possibleTypes[$typeName] = []; + foreach ($typeConf['config']['types'] as $unionMember) { + $possibleTypes[$typeName][$unionMember] = 1; } - $possibleTypes[$iface][$typeName] = 1; } }