From ff6fb142f1051c933968f759d0166a2d9cae50eb Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Wed, 20 Dec 2017 11:28:17 +0100 Subject: [PATCH 01/14] Add GraphQL schema language configuration --- Config/Parser/GraphQLParser.php | 265 ++++++++++++++++++ Config/Parser/ParserInterface.php | 5 +- Config/Parser/XmlParser.php | 11 +- Config/Parser/YamlParser.php | 12 +- DependencyInjection/Configuration.php | 17 +- .../OverblogGraphQLTypesExtension.php | 50 ++-- README.md | 1 + .../definitions/graphql-schema-language.md | 70 +++++ Resources/doc/definitions/index.md | 2 + .../doc/definitions/type-system/index.md | 5 +- Tests/Config/Parser/GraphQLParserTest.php | 68 +++++ .../Parser/fixtures/graphql/empty.graphql | 0 .../Parser/fixtures/graphql/invalid.graphql | 1 + .../not-supported-scalar-definition.graphql | 1 + .../not-supported-schema-definition.graphql | 4 + .../Parser/fixtures/graphql/schema.graphql | 52 ++++ .../Config/Parser/fixtures/graphql/schema.php | 112 ++++++++ .../OverblogGraphQLTypesExtensionTest.php | 2 +- Tests/Functional/App/config/global/config.yml | 2 +- .../global/mapping/decorators.types.yml | 30 ++ .../global/mapping/global.types.graphql | 25 ++ .../config/global/mapping/global.types.yml | 50 +--- 22 files changed, 688 insertions(+), 97 deletions(-) create mode 100644 Config/Parser/GraphQLParser.php create mode 100644 Resources/doc/definitions/graphql-schema-language.md create mode 100644 Tests/Config/Parser/GraphQLParserTest.php create mode 100644 Tests/Config/Parser/fixtures/graphql/empty.graphql create mode 100644 Tests/Config/Parser/fixtures/graphql/invalid.graphql create mode 100644 Tests/Config/Parser/fixtures/graphql/not-supported-scalar-definition.graphql create mode 100644 Tests/Config/Parser/fixtures/graphql/not-supported-schema-definition.graphql create mode 100644 Tests/Config/Parser/fixtures/graphql/schema.graphql create mode 100644 Tests/Config/Parser/fixtures/graphql/schema.php create mode 100644 Tests/Functional/App/config/global/mapping/decorators.types.yml create mode 100644 Tests/Functional/App/config/global/mapping/global.types.graphql diff --git a/Config/Parser/GraphQLParser.php b/Config/Parser/GraphQLParser.php new file mode 100644 index 000000000..1e92e4267 --- /dev/null +++ b/Config/Parser/GraphQLParser.php @@ -0,0 +1,265 @@ + 'object', + NodeKind::INTERFACE_TYPE_DEFINITION => 'interface', + NodeKind::ENUM_TYPE_DEFINITION => 'enum', + NodeKind::UNION_TYPE_DEFINITION => 'union', + NodeKind::INPUT_OBJECT_TYPE_DEFINITION => 'input-object', + ]; + + /** + * {@inheritdoc} + */ + public static function parse(\SplFileInfo $file, ContainerBuilder $container) + { + $container->addResource(new FileResource($file->getRealPath())); + $content = trim(file_get_contents($file->getPathname())); + $typesConfig = []; + + // allow empty files + if (empty($content)) { + return []; + } + if (!self::$parser) { + self::$parser = new static(); + } + try { + $ast = Parser::parse($content); + } catch (\Exception $e) { + throw new InvalidArgumentException(sprintf('An error occurred while parsing the file "%s".', $file), $e->getCode(), $e); + } + + /** @var Node $typeDef */ + foreach ($ast->definitions as $typeDef) { + $typeConfig = self::$parser->typeDefinitionToConfig($typeDef); + $typesConfig[$typeDef->name->value] = $typeConfig; + } + + return $typesConfig; + } + + protected function typeDefinitionToConfig(Node $typeDef) + { + switch ($typeDef->kind) { + case NodeKind::OBJECT_TYPE_DEFINITION: + case NodeKind::INTERFACE_TYPE_DEFINITION: + case NodeKind::INPUT_OBJECT_TYPE_DEFINITION: + case NodeKind::ENUM_TYPE_DEFINITION: + case NodeKind::UNION_TYPE_DEFINITION: + $config = []; + $this->addTypeFields($typeDef, $config); + $this->addDescription($typeDef, $config); + $this->addInterfaces($typeDef, $config); + $this->addTypes($typeDef, $config); + $this->addValues($typeDef, $config); + + return [ + 'type' => self::DEFINITION_TYPE_MAPPING[$typeDef->kind], + 'config' => $config, + ]; + + default: + throw new InvalidArgumentException( + sprintf( + '%s definition is not supported right now.', + preg_replace('@Definition$@', '', $typeDef->kind) + ) + ); + } + } + + /** + * @param Node $typeDef + * @param array $config + */ + private function addTypeFields(Node $typeDef, array &$config) + { + if (!empty($typeDef->fields)) { + $fields = []; + /** @var FieldDefinitionNode|InputValueDefinitionNode $fieldDef */ + foreach ($typeDef->fields as $fieldDef) { + $fieldName = $fieldDef->name->value; + $fields[$fieldName] = []; + $this->addType($fieldDef, $fields[$fieldName]); + $this->addDescription($fieldDef, $fields[$fieldName]); + $this->addDefaultValue($fieldDef, $fields[$fieldName]); + $this->addFieldArguments($fieldDef, $fields[$fieldName]); + } + $config['fields'] = $fields; + } + } + + /** + * @param Node $fieldDef + * @param array $fieldConf + */ + private function addFieldArguments(Node $fieldDef, array &$fieldConf) + { + if (!empty($fieldDef->arguments)) { + $arguments = []; + /** @var InputValueDefinitionNode $definition */ + foreach ($fieldDef->arguments as $definition) { + $name = $definition->name->value; + $arguments[$name] = []; + $this->addType($definition, $arguments[$name]); + $this->addDescription($definition, $arguments[$name]); + $this->addDefaultValue($definition, $arguments[$name]); + } + $fieldConf['arguments'] = $arguments; + } + } + + /** + * @param Node $typeDef + * @param array $config + */ + private function addInterfaces(Node $typeDef, array &$config) + { + if (!empty($typeDef->interfaces)) { + $interfaces = []; + foreach ($typeDef->interfaces as $interface) { + $interfaces[] = $this->astTypeNodeToString($interface); + } + $config['interfaces'] = $interfaces; + } + } + + /** + * @param Node $typeDef + * @param array $config + */ + private function addTypes(Node $typeDef, array &$config) + { + if (!empty($typeDef->types)) { + $types = []; + foreach ($typeDef->types as $type) { + $types[] = $this->astTypeNodeToString($type); + } + $config['types'] = $types; + } + } + + /** + * @param Node $typeDef + * @param array $config + */ + private function addValues(Node $typeDef, array &$config) + { + if (!empty($typeDef->values)) { + $values = []; + foreach ($typeDef->values as $value) { + $values[] = ['value' => $value->name->value]; + } + $config['values'] = $values; + } + } + + /** + * @param Node $definition + * @param array $config + */ + private function addType(Node $definition, array &$config) + { + if (!empty($definition->type)) { + $config['type'] = $this->astTypeNodeToString($definition->type); + } + } + + /** + * @param Node $definition + * @param array $config + */ + private function addDescription(Node $definition, array &$config) + { + if ( + !empty($definition->description) + && $description = $this->cleanAstDescription($definition->description) + ) { + $config['description'] = $description; + } + } + + /** + * @param InputValueDefinitionNode|FieldDefinitionNode $definition + * @param array $config + */ + private function addDefaultValue($definition, array &$config) + { + if (!empty($definition->defaultValue)) { + $config['defaultValue'] = $this->astValueNodeToConfig($definition->defaultValue); + } + } + + private function astTypeNodeToString(TypeNode $typeNode) + { + $type = ''; + switch ($typeNode->kind) { + case NodeKind::NAMED_TYPE: + $type = $typeNode->name->value; + break; + + case NodeKind::NON_NULL_TYPE: + $type = $this->astTypeNodeToString($typeNode->type).'!'; + break; + + case NodeKind::LIST_TYPE: + $type = '['.$this->astTypeNodeToString($typeNode->type).']'; + break; + } + + return $type; + } + + private function astValueNodeToConfig(ValueNode $valueNode) + { + $config = null; + switch ($valueNode->kind) { + case NodeKind::INT: + case NodeKind::FLOAT: + case NodeKind::STRING: + case NodeKind::BOOLEAN: + case NodeKind::ENUM: + $config = $valueNode->value; + break; + + case NodeKind::LST: + $config = []; + foreach ($valueNode->values as $node) { + $config[] = $this->astValueNodeToConfig($node); + } + break; + + case NodeKind::NULL: + $config = null; + break; + } + + return $config; + } + + private function cleanAstDescription($description) + { + $description = trim($description); + + return empty($description) ? null : $description; + } +} diff --git a/Config/Parser/ParserInterface.php b/Config/Parser/ParserInterface.php index 218844799..586798812 100644 --- a/Config/Parser/ParserInterface.php +++ b/Config/Parser/ParserInterface.php @@ -3,15 +3,14 @@ namespace Overblog\GraphQLBundle\Config\Parser; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\Finder\SplFileInfo; interface ParserInterface { /** - * @param SplFileInfo $file + * @param \SplFileInfo $file * @param ContainerBuilder $container * * @return array */ - public static function parse(SplFileInfo $file, ContainerBuilder $container); + public static function parse(\SplFileInfo $file, ContainerBuilder $container); } diff --git a/Config/Parser/XmlParser.php b/Config/Parser/XmlParser.php index ab8de9fbf..aedef89bc 100644 --- a/Config/Parser/XmlParser.php +++ b/Config/Parser/XmlParser.php @@ -6,20 +6,16 @@ use Symfony\Component\Config\Util\XmlUtils; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; -use Symfony\Component\Finder\SplFileInfo; class XmlParser implements ParserInterface { /** - * @param SplFileInfo $file - * @param ContainerBuilder $container - * - * @return array + * {@inheritdoc} */ - public static function parse(SplFileInfo $file, ContainerBuilder $container) + public static function parse(\SplFileInfo $file, ContainerBuilder $container) { $typesConfig = []; - + $container->addResource(new FileResource($file->getRealPath())); try { $xml = XmlUtils::loadFile($file->getRealPath()); foreach ($xml->documentElement->childNodes as $node) { @@ -31,7 +27,6 @@ public static function parse(SplFileInfo $file, ContainerBuilder $container) $typesConfig = array_merge($typesConfig, $values); } } - $container->addResource(new FileResource($file->getRealPath())); } catch (\InvalidArgumentException $e) { throw new InvalidArgumentException(sprintf('Unable to parse file "%s".', $file), $e->getCode(), $e); } diff --git a/Config/Parser/YamlParser.php b/Config/Parser/YamlParser.php index 3580ac1bb..7b4f73dcd 100644 --- a/Config/Parser/YamlParser.php +++ b/Config/Parser/YamlParser.php @@ -5,7 +5,6 @@ use Symfony\Component\Config\Resource\FileResource; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; -use Symfony\Component\Finder\SplFileInfo; use Symfony\Component\Yaml\Exception\ParseException; use Symfony\Component\Yaml\Parser; @@ -15,20 +14,17 @@ class YamlParser implements ParserInterface private static $yamlParser; /** - * @param SplFileInfo $file - * @param ContainerBuilder $container - * - * @return array + * {@inheritdoc} */ - public static function parse(SplFileInfo $file, ContainerBuilder $container) + public static function parse(\SplFileInfo $file, ContainerBuilder $container) { if (null === self::$yamlParser) { self::$yamlParser = new Parser(); } + $container->addResource(new FileResource($file->getRealPath())); try { - $typesConfig = self::$yamlParser->parse($file->getContents()); - $container->addResource(new FileResource($file->getRealPath())); + $typesConfig = self::$yamlParser->parse(file_get_contents($file->getPathname())); } catch (ParseException $e) { throw new InvalidArgumentException(sprintf('The file "%s" does not contain valid YAML.', $file), 0, $e); } diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 25bee13e7..3c1bd2f42 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -94,7 +94,6 @@ private function errorsHandlerSection() ->arrayNode('errors') ->treatNullLike([]) ->prototype('scalar')->end() - ->end() ->end() ->end() ->end(); @@ -229,7 +228,6 @@ private function definitionsAutoMappingSection() private function definitionsMappingsSection() { $builder = new TreeBuilder(); - /** @var ArrayNodeDefinition $node */ $node = $builder->root('mappings'); $node ->children() @@ -248,23 +246,30 @@ private function definitionsMappingsSection() ->addDefaultsIfNotSet() ->beforeNormalization() ->ifTrue(function ($v) { - return isset($v['type']) && 'yml' === $v['type']; + return isset($v['type']) && is_string($v['type']); }) ->then(function ($v) { - $v['type'] = 'yaml'; + if ('yml' === $v['type']) { + $v['types'] = ['yaml']; + } else { + $v['types'] = [$v['type']]; + } + unset($v['type']); return $v; }) ->end() ->children() - ->enumNode('type')->values(['yaml', 'xml'])->defaultNull()->end() + ->arrayNode('types') + ->prototype('enum')->values(array_keys(OverblogGraphQLTypesExtension::SUPPORTED_TYPES_EXTENSIONS))->isRequired()->end() + ->end() ->scalarNode('dir')->defaultNull()->end() ->scalarNode('suffix')->defaultValue(OverblogGraphQLTypesExtension::DEFAULT_TYPES_SUFFIX)->end() ->end() ->end() ->end() ->end() - ->end(); + ; return $node; } diff --git a/DependencyInjection/OverblogGraphQLTypesExtension.php b/DependencyInjection/OverblogGraphQLTypesExtension.php index 874e4ac10..3c46a047e 100644 --- a/DependencyInjection/OverblogGraphQLTypesExtension.php +++ b/DependencyInjection/OverblogGraphQLTypesExtension.php @@ -2,6 +2,9 @@ namespace Overblog\GraphQLBundle\DependencyInjection; +use Overblog\GraphQLBundle\Config\Parser\GraphQLParser; +use Overblog\GraphQLBundle\Config\Parser\XmlParser; +use Overblog\GraphQLBundle\Config\Parser\YamlParser; use Overblog\GraphQLBundle\OverblogGraphQLBundle; use Symfony\Component\Config\Definition\Exception\ForbiddenOverwriteException; use Symfony\Component\Config\Resource\FileResource; @@ -12,9 +15,13 @@ class OverblogGraphQLTypesExtension extends Extension { - private static $configTypes = ['yaml', 'xml']; + const SUPPORTED_TYPES_EXTENSIONS = ['yaml' => '{yaml,yml}', 'xml' => 'xml', 'graphql' => '{graphql,graphqls}']; - private static $typeExtensions = ['yaml' => '{yaml,yml}', 'xml' => 'xml']; + const PARSERS = [ + 'yaml' => YamlParser::class, + 'xml' => XmlParser::class, + 'graphql' => GraphQLParser::class, + ]; private static $defaultDefaultConfig = [ 'definitions' => [ @@ -48,6 +55,7 @@ public function containerPrependExtensionConfig(array $config, ContainerBuilder $typesMappings = $this->mappingConfig($config, $container); // reset treated files $this->treatedFiles = []; + $typesMappings = call_user_func_array('array_merge', $typesMappings); // treats mappings foreach ($typesMappings as $params) { $this->prependExtensionConfigFromFiles($params['type'], $params['files'], $container); @@ -67,9 +75,7 @@ private function prependExtensionConfigFromFiles($type, $files, ContainerBuilder continue; } - $parserClass = sprintf('Overblog\\GraphQLBundle\\Config\\Parser\\%sParser', ucfirst($type)); - - $typeConfig = call_user_func($parserClass.'::parse', $file, $container); + $typeConfig = call_user_func(self::PARSERS[$type].'::parse', $file, $container); $container->prependExtensionConfig($this->getAlias(), $typeConfig); $this->treatedFiles[$file->getRealPath()] = true; } @@ -97,9 +103,9 @@ private function mappingConfig(array $config, ContainerBuilder $container) $mappingConfig = $config['definitions']['mappings']; $typesMappings = $mappingConfig['types']; - // app only config files (yml or xml) + // app only config files (yml or xml or graphql) if ($mappingConfig['auto_discover']['root_dir'] && $container->hasParameter('kernel.root_dir')) { - $typesMappings[] = ['dir' => $container->getParameter('kernel.root_dir').'/config/graphql', 'type' => null]; + $typesMappings[] = ['dir' => $container->getParameter('kernel.root_dir').'/config/graphql', 'types' => null]; } if ($mappingConfig['auto_discover']['bundles']) { $mappingFromBundles = $this->mappingFromBundles($container); @@ -108,7 +114,7 @@ private function mappingConfig(array $config, ContainerBuilder $container) // enabled only for this bundle $typesMappings[] = [ 'dir' => $this->bundleDir(OverblogGraphQLBundle::class).'/Resources/config/graphql', - 'type' => 'yaml', + 'types' => ['yaml'], ]; } @@ -122,12 +128,9 @@ private function detectFilesFromTypesMappings(array $typesMappings, ContainerBui { return array_filter(array_map( function (array $typeMapping) use ($container) { - $params = $this->detectFilesByType( - $container, - $typeMapping['dir'], - $typeMapping['type'], - isset($typeMapping['suffix']) ? $typeMapping['suffix'] : '' - ); + $suffix = isset($typeMapping['suffix']) ? $typeMapping['suffix'] : ''; + $types = isset($typeMapping['types']) ? $typeMapping['types'] : null; + $params = $this->detectFilesByTypes($container, $typeMapping['dir'], $suffix, $types); return $params; }, @@ -145,13 +148,13 @@ private function mappingFromBundles(ContainerBuilder $container) $bundleDir = $this->bundleDir($class); // only config files (yml or xml) - $typesMappings[] = ['dir' => $bundleDir.'/Resources/config/graphql', 'type' => null]; + $typesMappings[] = ['dir' => $bundleDir.'/Resources/config/graphql', 'types' => null]; } return $typesMappings; } - private function detectFilesByType(ContainerBuilder $container, $path, $type, $suffix) + private function detectFilesByTypes(ContainerBuilder $container, $path, $suffix, array $types = null) { // add the closest existing directory as a resource $resource = $path; @@ -160,25 +163,30 @@ private function detectFilesByType(ContainerBuilder $container, $path, $type, $s } $container->addResource(new FileResource($resource)); - $finder = new Finder(); + $stopOnFirstTypeMatching = empty($types); - $types = null === $type ? self::$configTypes : [$type]; + $types = $stopOnFirstTypeMatching ? array_keys(self::SUPPORTED_TYPES_EXTENSIONS) : $types; + $files = []; foreach ($types as $type) { + $finder = Finder::create(); try { - $finder->files()->in($path)->name('*'.$suffix.'.'.self::$typeExtensions[$type]); + $finder->files()->in($path)->name(sprintf('*%s.%s', $suffix, self::SUPPORTED_TYPES_EXTENSIONS[$type])); } catch (\InvalidArgumentException $e) { continue; } if ($finder->count() > 0) { - return [ + $files[] = [ 'type' => $type, 'files' => $finder, ]; + if ($stopOnFirstTypeMatching) { + break; + } } } - return; + return $files; } private function bundleDir($bundleClass) diff --git a/README.md b/README.md index 7e29b42f4..7f56ae50d 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ Documentation - [Lists](Resources/doc/definitions/type-system/lists.md) - [Non-Null](Resources/doc/definitions/type-system/non-null.md) - [Type Inheritance](Resources/doc/definitions/type-inheritance.md) + - [GraphQL schema language](Resources/doc/definitions/graphql-schema-language.md) - [Schema](Resources/doc/definitions/schema.md) - [Resolver](Resources/doc/definitions/resolver.md) - [Solving N+1 problem](Resources/doc/definitions/solving-n-plus-1-problem.md) diff --git a/Resources/doc/definitions/graphql-schema-language.md b/Resources/doc/definitions/graphql-schema-language.md new file mode 100644 index 000000000..aacb76bf4 --- /dev/null +++ b/Resources/doc/definitions/graphql-schema-language.md @@ -0,0 +1,70 @@ +GraphQL schema language +======================= + +This section we show how to define schema types using GraphQL schema language. +If you want to learn more about it, you can see +the [official documentation](http://graphql.org/learn/schema/) +or this [cheat sheet](https://github.com/sogko/graphql-shorthand-notation-cheat-sheet). + +Here is an example: + +```graphql +# config/graphql/schema.types.graphql + +type Query { + bar: Bar! +} + +interface Foo { + # Description of my is_foo field + is_foo: Boolean +} +type Bar implements Foo { + is_foo: Boolean + is_bar: Boolean +} +``` + +What about resolvers? To define resolvers for needed fields +you must use decorators with `hiers` [inheritance feature](type-inheritance.md). + +Here is how this can be done: + +- First of all, enable both config files format (yaml or xml with graphql) + + ```yaml + overblog_graphql: + definitions: + mappings: + types: + types: + - + types: [graphql, yaml] + dir: "%kernel.project_dir%/config/graphql/types" + ``` + +- Now you can write the decorators: + + ```yaml + # config/graphql/resolvers.types.yaml + + QueryDecorator: + decorator: true + hiers: Query + config: + fields: + bar: { resolve: "@=..." } + + FooDecorator: + decorator: true + hiers: Foo + config: + resolveType: "@=..." + ``` + +**Notes:** + +- This feature is experimental and could be improve or change in future releases +- Only type definition is allowed right now (excepted scalar type) using this shorthand syntax +- In general only required resolvers are the those managing fields on Root (query or mutation) +- Decorators is not limited to resolvers diff --git a/Resources/doc/definitions/index.md b/Resources/doc/definitions/index.md index 2bb1a2a21..dbc9e3450 100644 --- a/Resources/doc/definitions/index.md +++ b/Resources/doc/definitions/index.md @@ -3,11 +3,13 @@ Definitions * [Type System](type-system/index.md) * [Type Inheritance](type-inheritance.md) +* [GraphQL schema language](graphql-schema-language.md) * [Schema](schema.md) Go further ---------- +* [Solving N+1 problem](solving-n-plus-1-problem.md) * [Resolver](resolver.md) * [Mutation](mutation.md) * [Relay](relay/index.md) diff --git a/Resources/doc/definitions/type-system/index.md b/Resources/doc/definitions/type-system/index.md index 103266cbe..b03f7b7f4 100644 --- a/Resources/doc/definitions/type-system/index.md +++ b/Resources/doc/definitions/type-system/index.md @@ -28,9 +28,12 @@ Types can be define 3 different ways: # auto_discover: false # to disable bundles and root dir auto discover types: - - type: yaml # or xml or null + type: yaml # or xml or graphql null dir: "%kernel.root_dir%/.../mapping" # sub directories are also searched # suffix: .types # use to change default file suffix + - + types: [yaml, graphql] # to include different types from the same dir + dir: "%kernel.root_dir%/.../mapping" ``` 2. **The PHP way** diff --git a/Tests/Config/Parser/GraphQLParserTest.php b/Tests/Config/Parser/GraphQLParserTest.php new file mode 100644 index 000000000..3ed1e1d3b --- /dev/null +++ b/Tests/Config/Parser/GraphQLParserTest.php @@ -0,0 +1,68 @@ +containerBuilder = $this->getMockBuilder(ContainerBuilder::class)->setMethods(['addResource'])->getMock(); + } + + public function testParse() + { + $fileName = __DIR__.'/fixtures/graphql/schema.graphql'; + $expected = include __DIR__.'/fixtures/graphql/schema.php'; + + $this->assertContainerAddFileToRessources($fileName); + $config = GraphQLParser::parse(new \SplFileInfo($fileName), $this->containerBuilder); + $this->assertEquals($expected, $config); + } + + public function testParseEmptyFile() + { + $fileName = __DIR__.'/fixtures/graphql/empty.graphql'; + + $this->assertContainerAddFileToRessources($fileName); + + $config = GraphQLParser::parse(new \SplFileInfo($fileName), $this->containerBuilder); + $this->assertEquals([], $config); + } + + public function testParseInvalidFile() + { + $fileName = __DIR__.'/fixtures/graphql/invalid.graphql'; + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('An error occurred while parsing the file "%s"', $fileName)); + GraphQLParser::parse(new \SplFileInfo($fileName), $this->containerBuilder); + } + + public function testParseNotSupportedSchemaDefinition() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Schema definition is not supported right now.'); + GraphQLParser::parse(new \SplFileInfo(__DIR__.'/fixtures/graphql/not-supported-schema-definition.graphql'), $this->containerBuilder); + } + + public function testParseNotSupportedScalarDefinition() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('ScalarType definition is not supported right now.'); + GraphQLParser::parse(new \SplFileInfo(__DIR__.'/fixtures/graphql/not-supported-scalar-definition.graphql'), $this->containerBuilder); + } + + private function assertContainerAddFileToRessources($fileName) + { + $this->containerBuilder->expects($this->once()) + ->method('addResource') + ->with($fileName); + } +} diff --git a/Tests/Config/Parser/fixtures/graphql/empty.graphql b/Tests/Config/Parser/fixtures/graphql/empty.graphql new file mode 100644 index 000000000..e69de29bb diff --git a/Tests/Config/Parser/fixtures/graphql/invalid.graphql b/Tests/Config/Parser/fixtures/graphql/invalid.graphql new file mode 100644 index 000000000..c713dcb0c --- /dev/null +++ b/Tests/Config/Parser/fixtures/graphql/invalid.graphql @@ -0,0 +1 @@ +Query {} diff --git a/Tests/Config/Parser/fixtures/graphql/not-supported-scalar-definition.graphql b/Tests/Config/Parser/fixtures/graphql/not-supported-scalar-definition.graphql new file mode 100644 index 000000000..76dfa4c05 --- /dev/null +++ b/Tests/Config/Parser/fixtures/graphql/not-supported-scalar-definition.graphql @@ -0,0 +1 @@ +scalar Url diff --git a/Tests/Config/Parser/fixtures/graphql/not-supported-schema-definition.graphql b/Tests/Config/Parser/fixtures/graphql/not-supported-schema-definition.graphql new file mode 100644 index 000000000..c3c82a744 --- /dev/null +++ b/Tests/Config/Parser/fixtures/graphql/not-supported-schema-definition.graphql @@ -0,0 +1,4 @@ +schema { + query: Query + mutation: Mutation +} diff --git a/Tests/Config/Parser/fixtures/graphql/schema.graphql b/Tests/Config/Parser/fixtures/graphql/schema.graphql new file mode 100644 index 000000000..cfb345afd --- /dev/null +++ b/Tests/Config/Parser/fixtures/graphql/schema.graphql @@ -0,0 +1,52 @@ +# Root Query +type Query { + hero( + # Episode list to use to filter + episodes: [Episode!]! = [NEWHOPE, EMPIRE] + ): Character + # search for a droid + droid(id: ID!): Droid +} + +type Starship { + id: ID! + name: String! + length(unit: LengthUnit = METER): Float +} + +enum Episode { + NEWHOPE + EMPIRE + JEDI +} + +interface Character { + id: ID! + name: String! + friends: [Character] + appearsIn: [Episode]! +} + +type Human implements Character { + id: ID! + name: String! + friends: [Character] + appearsIn: [Episode]! + starships: [Starship] + totalCredits: Int +} + +type Droid implements Character { + id: ID! + name: String! + friends: [Character] + appearsIn: [Episode]! + primaryFunction: String +} + +union SearchResult = Human | Droid | Starship + +input ReviewInput { + stars: Int! = 5 + commentary: String = null +} diff --git a/Tests/Config/Parser/fixtures/graphql/schema.php b/Tests/Config/Parser/fixtures/graphql/schema.php new file mode 100644 index 000000000..c8f745745 --- /dev/null +++ b/Tests/Config/Parser/fixtures/graphql/schema.php @@ -0,0 +1,112 @@ + [ + 'type' => 'object', + 'config' => [ + 'description' => 'Root Query', + 'fields' => [ + 'hero' => [ + 'type' => 'Character', + 'arguments' => [ + 'episodes' => [ + 'type' => '[Episode!]!', + 'description' => 'Episode list to use to filter', + 'defaultValue' => ['NEWHOPE', 'EMPIRE'], + ], + ], + ], + 'droid' => [ + 'type' => 'Droid', + 'description' => 'search for a droid', + 'arguments' => [ + 'id' => [ + 'type' => 'ID!', + ], + ], + ], + ], + ], + ], + 'Starship' => [ + 'type' => 'object', + 'config' => [ + 'fields' => [ + 'id' => ['type' => 'ID!'], + 'name' => ['type' => 'String!'], + 'length' => [ + 'type' => 'Float', + 'arguments' => [ + 'unit' => [ + 'type' => 'LengthUnit', + 'defaultValue' => 'METER', + ], + ], + ], + ], + ], + ], + 'Episode' => [ + 'type' => 'enum', + 'config' => [ + 'values' => [ + ['value' => 'NEWHOPE'], + ['value' => 'EMPIRE'], + ['value' => 'JEDI'], + ], + ], + ], + 'Character' => [ + 'type' => 'interface', + 'config' => [ + 'fields' => [ + 'id' => ['type' => 'ID!'], + 'name' => ['type' => 'String!'], + 'friends' => ['type' => '[Character]'], + 'appearsIn' => ['type' => '[Episode]!'], + ], + ], + ], + 'Human' => [ + 'type' => 'object', + 'config' => [ + 'fields' => [ + 'id' => ['type' => 'ID!'], + 'name' => ['type' => 'String!'], + 'friends' => ['type' => '[Character]'], + 'appearsIn' => ['type' => '[Episode]!'], + 'starships' => ['type' => '[Starship]'], + 'totalCredits' => ['type' => 'Int'], + ], + 'interfaces' => ['Character'], + ], + ], + 'Droid' => [ + 'type' => 'object', + 'config' => [ + 'fields' => [ + 'id' => ['type' => 'ID!'], + 'name' => ['type' => 'String!'], + 'friends' => ['type' => '[Character]'], + 'appearsIn' => ['type' => '[Episode]!'], + 'primaryFunction' => ['type' => 'String'], + ], + 'interfaces' => ['Character'], + ], + ], + 'SearchResult' => [ + 'type' => 'union', + 'config' => [ + 'types' => ['Human', 'Droid', 'Starship'], + ], + ], + 'ReviewInput' => [ + 'type' => 'input-object', + 'config' => [ + 'fields' => [ + 'stars' => ['type' => 'Int!', 'defaultValue' => 5], + 'commentary' => ['type' => 'String', 'defaultValue' => null], + ], + ], + ], +]; diff --git a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php index 59d260168..f1ea2583e 100644 --- a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php +++ b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php @@ -256,7 +256,7 @@ private function getMappingConfig($type) 'mappings' => [ 'types' => [ [ - 'type' => $type, + 'types' => [$type], 'dir' => __DIR__.'/mapping/'.$type, ], ], diff --git a/Tests/Functional/App/config/global/config.yml b/Tests/Functional/App/config/global/config.yml index d2b0671da..0dcfc19b5 100644 --- a/Tests/Functional/App/config/global/config.yml +++ b/Tests/Functional/App/config/global/config.yml @@ -18,5 +18,5 @@ overblog_graphql: mappings: types: - - type: yaml + types: [yaml, graphql] dir: "%kernel.root_dir%/config/global/mapping" diff --git a/Tests/Functional/App/config/global/mapping/decorators.types.yml b/Tests/Functional/App/config/global/mapping/decorators.types.yml new file mode 100644 index 000000000..b39dbce6d --- /dev/null +++ b/Tests/Functional/App/config/global/mapping/decorators.types.yml @@ -0,0 +1,30 @@ +PhotoDecorator: + decorator: true + heirs: [Photo] + config: + fields: + id: + builder: "Relay::GlobalId" + builderConfig: + typeName: Photo + idFetcher: '@=value["photoId"]' + +PostDecorator: + decorator: true + heirs: [Post] + config: + fields: + id: "Relay::GlobalId" + +PhotoAndPostDecorator: + decorator: true + heirs: PhotoAndPost + config: + resolveType: '@=service("overblog_graphql.test.resolver.global").typeResolver(value)' + +UserDecorator: + decorator: true + heirs: [User] + config: + fields: + id: "Relay::GlobalId" diff --git a/Tests/Functional/App/config/global/mapping/global.types.graphql b/Tests/Functional/App/config/global/mapping/global.types.graphql new file mode 100644 index 000000000..46bf335de --- /dev/null +++ b/Tests/Functional/App/config/global/mapping/global.types.graphql @@ -0,0 +1,25 @@ + +type Photo implements NodeInterface { + # The ID of an object + id: ID! + width: Int +} + +union PhotoAndPost = Photo | Post + +input PhotoInput { + width: Int = 100 +} + +type Post implements NodeInterface { + # The ID of an object + id: ID! + text: String + status: Status +} + +type User implements NodeInterface { + # The ID of an object + id: ID! + name: String +} diff --git a/Tests/Functional/App/config/global/mapping/global.types.yml b/Tests/Functional/App/config/global/mapping/global.types.yml index 372b34ec1..94d79d1b3 100644 --- a/Tests/Functional/App/config/global/mapping/global.types.yml +++ b/Tests/Functional/App/config/global/mapping/global.types.yml @@ -1,8 +1,3 @@ -NodeInterface: - type: relay-node - config: - resolveType: '@=service("overblog_graphql.test.resolver.global").typeResolver(value)' - Query: type: object config: @@ -16,52 +11,11 @@ Query: type: '[NodeInterface]' resolve: '@=service("overblog_graphql.test.resolver.global").resolveAllObjects()' -User: - type: object - config: - fields: - id: "Relay::GlobalId" - name: - type: String - interfaces: [NodeInterface] - -Photo: - type: object - config: - fields: - id: - builder: "Relay::GlobalId" - builderConfig: - typeName: Photo - idFetcher: '@=value["photoId"]' - width: - type: Int - interfaces: [NodeInterface] - -Post: - type: object - config: - fields: - id: - builder: "Relay::GlobalId" - text: - type: String - status: - type: Status - interfaces: [NodeInterface] - -PhotoAndPost: - type: union +NodeInterface: + type: relay-node config: - types: [Photo, Post] resolveType: '@=service("overblog_graphql.test.resolver.global").typeResolver(value)' -PhotoInput: - type: input-object - config: - fields: - width: { type: Int, defaultValue: 100 } - Status: type: enum config: From 1c122f4b61d6dd764c7ff42406c987fe5b7cddbe Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sat, 6 Jan 2018 18:47:19 +0100 Subject: [PATCH 02/14] Introduce ConfigProcessor This helps to make changes on type config on type instantiation --- Definition/ConfigProcessor.php | 58 ++++++++++++++++++ .../ConfigProcessorInterface.php | 15 +++++ Definition/ConfigProcessor/InjectServices.php | 45 ++++++++++++++ .../PublicFieldsFilterConfigProcessor.php | 53 +++++++++++++++++ Definition/LazyConfig.php | 59 +++++++++++++++++++ .../Compiler/ConfigTypesPass.php | 3 +- .../DefinitionConfigProcessorCompilerPass.php | 30 ++++++++++ .../OverblogGraphQLExtension.php | 15 +++-- Generator/TypeGenerator.php | 10 ---- OverblogGraphQLBundle.php | 2 + .../config/definition_config_processors.yml | 14 +++++ Resources/config/services.yml | 7 ++- Resources/skeleton/TypeSystem.php.skeleton | 36 +++-------- 13 files changed, 300 insertions(+), 47 deletions(-) create mode 100644 Definition/ConfigProcessor.php create mode 100644 Definition/ConfigProcessor/ConfigProcessorInterface.php create mode 100644 Definition/ConfigProcessor/InjectServices.php create mode 100644 Definition/ConfigProcessor/PublicFieldsFilterConfigProcessor.php create mode 100644 Definition/LazyConfig.php create mode 100644 DependencyInjection/Compiler/DefinitionConfigProcessorCompilerPass.php create mode 100644 Resources/config/definition_config_processors.yml diff --git a/Definition/ConfigProcessor.php b/Definition/ConfigProcessor.php new file mode 100644 index 000000000..22707e450 --- /dev/null +++ b/Definition/ConfigProcessor.php @@ -0,0 +1,58 @@ +register($configProcessor, $priority); + } + + public function register(ConfigProcessorInterface $configProcessor, $priority = 0) + { + if ($this->isProcessed) { + throw new \LogicException('Registering config processor after calling process() is not supported.'); + } + $this->processors[] = ['processor' => $configProcessor, 'priority' => $priority]; + } + + public function process(LazyConfig $lazyConfig) + { + $this->initialize(); + foreach ($this->processors as $processor) { + $lazyConfig = $processor->process($lazyConfig); + } + + return $lazyConfig; + } + + private function initialize() + { + if (!$this->isProcessed) { + // order processors by DESC priority + usort($this->processors, function ($processorA, $processorB) { + if ($processorA['priority'] === $processorB['priority']) { + return 0; + } + + return ($processorA['priority'] < $processorB['priority']) ? 1 : -1; + }); + + $this->processors = array_column($this->processors, 'processor'); + $this->isProcessed = true; + } + } +} diff --git a/Definition/ConfigProcessor/ConfigProcessorInterface.php b/Definition/ConfigProcessor/ConfigProcessorInterface.php new file mode 100644 index 000000000..b2a498505 --- /dev/null +++ b/Definition/ConfigProcessor/ConfigProcessorInterface.php @@ -0,0 +1,15 @@ +container = $container; + } + + /** + * {@inheritdoc} + */ + public function process(LazyConfig $lazyConfig) + { + $vars = $lazyConfig->getVars(); + $container = $this->container; + $user = null; + $request = null; + $token = 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(); + } + } + $vars['token'] = $token; + $vars['container'] = $container; + $vars['user'] = $user; + $vars['request'] = $request; + + return $lazyConfig; + } +} diff --git a/Definition/ConfigProcessor/PublicFieldsFilterConfigProcessor.php b/Definition/ConfigProcessor/PublicFieldsFilterConfigProcessor.php new file mode 100644 index 000000000..c1d835a8c --- /dev/null +++ b/Definition/ConfigProcessor/PublicFieldsFilterConfigProcessor.php @@ -0,0 +1,53 @@ +getLoader(); + + $lazyConfig->setLoader(function (...$args) use ($configLoader) { + $config = $configLoader(...$args); + + if (isset($config['fields']) && is_callable($config['fields'])) { + $config['fields'] = function () use ($config) { + $fields = $config['fields'](); + + return static::filter($fields); + }; + } + + return $config; + }); + + return $lazyConfig; + } +} diff --git a/Definition/LazyConfig.php b/Definition/LazyConfig.php new file mode 100644 index 000000000..fdf687c44 --- /dev/null +++ b/Definition/LazyConfig.php @@ -0,0 +1,59 @@ +loader = $loader; + $this->vars = new \ArrayObject($vars); + } + + public static function create(\Closure $loader, array $vars = []) + { + return new self($loader, $vars); + } + + /** + * @return array + */ + public function load() + { + $loader = $this->loader; + + return $loader($this->vars->getArrayCopy()); + } + + /** + * @return \Closure + */ + public function getLoader() + { + return $this->loader; + } + + public function setLoader(\Closure $loader) + { + $this->loader = $loader; + } + + /** + * @return \ArrayObject + */ + public function getVars() + { + return $this->vars; + } + + public function setVars(\ArrayObject $vars) + { + $this->vars = $vars; + } +} diff --git a/DependencyInjection/Compiler/ConfigTypesPass.php b/DependencyInjection/Compiler/ConfigTypesPass.php index d4c1697ce..61541364a 100644 --- a/DependencyInjection/Compiler/ConfigTypesPass.php +++ b/DependencyInjection/Compiler/ConfigTypesPass.php @@ -2,6 +2,7 @@ namespace Overblog\GraphQLBundle\DependencyInjection\Compiler; +use Overblog\GraphQLBundle\Definition\ConfigProcessor; use Overblog\GraphQLBundle\Generator\TypeGenerator; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -25,7 +26,7 @@ private function setTypeServiceDefinition(ContainerBuilder $container, $class, a { $definition = $container->setDefinition($class, new Definition($class)); $definition->setPublic(false); - $definition->setArguments([new Reference('service_container')]); + $definition->setArguments([new Reference(ConfigProcessor::class)]); foreach ($aliases as $alias) { $definition->addTag(TypeTaggedServiceMappingPass::TAG_NAME, ['alias' => $alias, 'generated' => true]); } diff --git a/DependencyInjection/Compiler/DefinitionConfigProcessorCompilerPass.php b/DependencyInjection/Compiler/DefinitionConfigProcessorCompilerPass.php new file mode 100644 index 000000000..0d7a95e19 --- /dev/null +++ b/DependencyInjection/Compiler/DefinitionConfigProcessorCompilerPass.php @@ -0,0 +1,30 @@ +findDefinition(ConfigProcessor::class); + $taggedServices = $container->findTaggedServiceIds('overblog_graphql.definition_config_processor'); + + foreach ($taggedServices as $id => $tags) { + $definition->addMethodCall( + 'addConfigProcessor', + [ + new Reference($id), + isset($tags[0]['priority']) ? $tags[0]['priority'] : 0, + ] + ); + } + } +} diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index 5c2ac3fb9..32cc4eea5 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -29,11 +29,7 @@ class OverblogGraphQLExtension extends Extension implements PrependExtensionInte { public function load(array $configs, ContainerBuilder $container) { - $loader = new YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); - $loader->load('services.yml'); - $loader->load('graphql_types.yml'); - $loader->load('expression_language_functions.yml'); - + $this->loadConfigFiles($container); $config = $this->treatConfigs($configs, $container); $this->setBatchingMethod($config, $container); @@ -76,6 +72,15 @@ public function getConfiguration(array $config, ContainerBuilder $container) ); } + private function loadConfigFiles(ContainerBuilder $container) + { + $loader = new YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); + $loader->load('services.yml'); + $loader->load('graphql_types.yml'); + $loader->load('expression_language_functions.yml'); + $loader->load('definition_config_processors.yml'); + } + private function setCompilerCacheWarmer(array $config, ContainerBuilder $container) { if ($config['definitions']['auto_compile']) { diff --git a/Generator/TypeGenerator.php b/Generator/TypeGenerator.php index 66ef1204b..f6c77252b 100644 --- a/Generator/TypeGenerator.php +++ b/Generator/TypeGenerator.php @@ -75,16 +75,6 @@ protected function generateClassDocBlock(array $value) EOF; } - protected function generateOutputFields(array $config) - { - $outputFieldsCode = sprintf( - 'self::applyPublicFilters(%s)', - $this->processFromArray($config['fields'], 'OutputField') - ); - - return sprintf(static::$closureTemplate, '', $outputFieldsCode); - } - protected function generateClosureUseStatements(array $config) { return 'use ('.static::USE_FOR_CLOSURES.') '; diff --git a/OverblogGraphQLBundle.php b/OverblogGraphQLBundle.php index 8dc6124bd..44f7584a8 100644 --- a/OverblogGraphQLBundle.php +++ b/OverblogGraphQLBundle.php @@ -6,6 +6,7 @@ use Overblog\GraphQLBundle\DependencyInjection\Compiler\AutoMappingPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\AutowiringTypesPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ConfigTypesPass; +use Overblog\GraphQLBundle\DependencyInjection\Compiler\DefinitionConfigProcessorCompilerPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ExpressionFunctionPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\MutationTaggedServiceMappingTaggedPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverTaggedServiceMappingPass; @@ -28,6 +29,7 @@ public function build(ContainerBuilder $container) //ConfigTypesPass and AutoMappingPass must be before TypeTaggedServiceMappingPass $container->addCompilerPass(new ExpressionFunctionPass()); + $container->addCompilerPass(new DefinitionConfigProcessorCompilerPass()); $container->addCompilerPass(new AutoMappingPass()); $container->addCompilerPass(new ConfigTypesPass(), PassConfig::TYPE_BEFORE_REMOVING); $container->addCompilerPass(new AliasedPass()); diff --git a/Resources/config/definition_config_processors.yml b/Resources/config/definition_config_processors.yml new file mode 100644 index 000000000..036840cda --- /dev/null +++ b/Resources/config/definition_config_processors.yml @@ -0,0 +1,14 @@ +services: + Overblog\GraphQLBundle\Definition\ConfigProcessor\PublicFieldsFilterConfigProcessor: + class: Overblog\GraphQLBundle\Definition\ConfigProcessor\PublicFieldsFilterConfigProcessor + public: false + tags: + - { name: overblog_graphql.definition_config_processor, priority: 2048 } + + Overblog\GraphQLBundle\Definition\ConfigProcessor\InjectServices: + class: Overblog\GraphQLBundle\Definition\ConfigProcessor\InjectServices + public: false + arguments: + - '@service_container' + tags: + - { name: overblog_graphql.definition_config_processor } diff --git a/Resources/config/services.yml b/Resources/config/services.yml index cec626894..f67fe048f 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -86,8 +86,7 @@ services: - "%overblog_graphql_types.config%" - "%overblog_graphql.use_classloader_listener%" calls: - - ["addUseStatement", ["Symfony\\Component\\DependencyInjection\\ContainerInterface"]] - - ["addUseStatement", ["Symfony\\Component\\Security\\Core\\Authentication\\Token\\TokenInterface"]] + - ["addUseStatement", ["Overblog\\GraphQLBundle\\Definition\\ConfigProcessor"]] - ["addImplement", ["Overblog\\GraphQLBundle\\Definition\\Type\\GeneratedTypeInterface"]] - ["setExpressionLanguage", ["@overblog_graphql.expression_language"]] @@ -146,3 +145,7 @@ services: - "@overblog_graphql.cache_compiler" tags: - { name: console.command } + + Overblog\GraphQLBundle\Definition\ConfigProcessor: + class: Overblog\GraphQLBundle\Definition\ConfigProcessor + public: false diff --git a/Resources/skeleton/TypeSystem.php.skeleton b/Resources/skeleton/TypeSystem.php.skeleton index 9c12d23a2..60cc5ccde 100644 --- a/Resources/skeleton/TypeSystem.php.skeleton +++ b/Resources/skeleton/TypeSystem.php.skeleton @@ -5,36 +5,14 @@ class extends { -public function __construct(ContainerInterface $container) +public function __construct(ConfigProcessor $configProcessor) { -$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(); -} -} +$configLoader = function(array $vars) { +extract($vars); -parent::__construct(); -} - -private static function applyPublicFilters($fields) -{ -$filtered = []; -foreach ($fields as $fieldName => $field) { -$isPublic = isset($field['public']) ? $field['public'] : true; -if (is_callable($isPublic)) { -$isPublic = call_user_func($isPublic, $fieldName); -} -if ($isPublic) { -$filtered[$fieldName] = $field; -} -} -return $filtered; +return ; +}; +$config = $configProcessor->process(\Overblog\GraphQLBundle\Definition\LazyConfig::create($configLoader))->load(); +parent::__construct($config); } } From da9a54cffa0d28d5eda892dca00e5ae3eb516240 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sun, 7 Jan 2018 11:45:12 +0100 Subject: [PATCH 03/14] Add AclConfigProcessor --- .../ConfigProcessor/AclConfigProcessor.php | 61 +++++++++++++++++++ ....php => InjectServicesConfigProcessor.php} | 2 +- .../PublicFieldsFilterConfigProcessor.php | 14 +---- Definition/LazyConfig.php | 21 +++++-- Generator/TypeGenerator.php | 61 +++++-------------- .../config/definition_config_processors.yml | 12 +++- .../skeleton/OutputFieldConfig.php.skeleton | 2 + 7 files changed, 106 insertions(+), 67 deletions(-) create mode 100644 Definition/ConfigProcessor/AclConfigProcessor.php rename Definition/ConfigProcessor/{InjectServices.php => InjectServicesConfigProcessor.php} (93%) diff --git a/Definition/ConfigProcessor/AclConfigProcessor.php b/Definition/ConfigProcessor/AclConfigProcessor.php new file mode 100644 index 000000000..9235eb2f2 --- /dev/null +++ b/Definition/ConfigProcessor/AclConfigProcessor.php @@ -0,0 +1,61 @@ +accessResolver = $accessResolver; + } + + public static function acl(array $fields, AccessResolver $accessResolver) + { + $deniedAccess = static function () { + throw new UserWarning('Access denied to this field.'); + }; + foreach ($fields as &$field) { + if (isset($field['access']) && true !== $field['access']) { + $accessChecker = $field['access']; + if (false === $accessChecker) { + $field['resolve'] = $deniedAccess; + } elseif (is_callable($accessChecker) && isset($field['resolve'])) { // todo manage when resolver is not set + $field['resolve'] = static function ($value, $args, $context, ResolveInfo $info) use ($field, $accessChecker, $accessResolver) { + $resolverCallback = $field['resolve']; + $isMutation = 'mutation' === $info->operation->operation && $info->parentType === $info->schema->getMutationType(); + + return $accessResolver->resolve($accessChecker, $resolverCallback, [$value, new Argument($args), $context, $info], $isMutation); + }; + } + } + } + + return $fields; + } + + public function process(LazyConfig $lazyConfig) + { + $lazyConfig->addPostLoader(function ($config) { + if (isset($config['fields']) && is_callable($config['fields'])) { + $config['fields'] = function () use ($config) { + $fields = $config['fields'](); + + return static::acl($fields, $this->accessResolver); + }; + } + + return $config; + }); + + return $lazyConfig; + } +} diff --git a/Definition/ConfigProcessor/InjectServices.php b/Definition/ConfigProcessor/InjectServicesConfigProcessor.php similarity index 93% rename from Definition/ConfigProcessor/InjectServices.php rename to Definition/ConfigProcessor/InjectServicesConfigProcessor.php index dd8996ed7..227cfefd6 100644 --- a/Definition/ConfigProcessor/InjectServices.php +++ b/Definition/ConfigProcessor/InjectServicesConfigProcessor.php @@ -6,7 +6,7 @@ use Psr\Container\ContainerInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -final class InjectServices implements ConfigProcessorInterface +final class InjectServicesConfigProcessor implements ConfigProcessorInterface { /** @var ContainerInterface */ private $container; diff --git a/Definition/ConfigProcessor/PublicFieldsFilterConfigProcessor.php b/Definition/ConfigProcessor/PublicFieldsFilterConfigProcessor.php index c1d835a8c..9fdcc9d70 100644 --- a/Definition/ConfigProcessor/PublicFieldsFilterConfigProcessor.php +++ b/Definition/ConfigProcessor/PublicFieldsFilterConfigProcessor.php @@ -13,12 +13,8 @@ public static function filter(array $fields) function ($field, $fieldName) { $exposed = true; - if (isset($field['public'])) { - if (is_callable($field['public'])) { - $exposed = call_user_func($field['public'], $fieldName); - } else { - $exposed = (bool) $field['public']; - } + if (isset($field['public']) && is_callable($field['public'])) { + $exposed = (bool) call_user_func($field['public'], $fieldName); } return $exposed; @@ -32,11 +28,7 @@ function ($field, $fieldName) { */ public function process(LazyConfig $lazyConfig) { - $configLoader = $lazyConfig->getLoader(); - - $lazyConfig->setLoader(function (...$args) use ($configLoader) { - $config = $configLoader(...$args); - + $lazyConfig->addPostLoader(function ($config) { if (isset($config['fields']) && is_callable($config['fields'])) { $config['fields'] = function () use ($config) { $fields = $config['fields'](); diff --git a/Definition/LazyConfig.php b/Definition/LazyConfig.php index fdf687c44..28b685423 100644 --- a/Definition/LazyConfig.php +++ b/Definition/LazyConfig.php @@ -10,6 +10,11 @@ final class LazyConfig /** @var */ private $vars; + /** + * @var callable + */ + private $onPostLoad = []; + private function __construct(\Closure $loader, array $vars = []) { $this->loader = $loader; @@ -27,8 +32,17 @@ public static function create(\Closure $loader, array $vars = []) public function load() { $loader = $this->loader; + $config = $loader($this->vars->getArrayCopy()); + foreach ($this->onPostLoad as $postLoader) { + $config = $postLoader($config); + } - return $loader($this->vars->getArrayCopy()); + return $config; + } + + public function addPostLoader(callable $postLoader) + { + $this->onPostLoad[] = $postLoader; } /** @@ -51,9 +65,4 @@ public function getVars() { return $this->vars; } - - public function setVars(\ArrayObject $vars) - { - $this->vars = $vars; - } } diff --git a/Generator/TypeGenerator.php b/Generator/TypeGenerator.php index f6c77252b..301d9d966 100644 --- a/Generator/TypeGenerator.php +++ b/Generator/TypeGenerator.php @@ -3,10 +3,8 @@ namespace Overblog\GraphQLBundle\Generator; use Composer\Autoload\ClassLoader; -use GraphQL\Type\Definition\ResolveInfo; use Overblog\GraphQLBundle\Config\Processor; use Overblog\GraphQLBundle\Definition\Argument; -use Overblog\GraphQLBundle\Error\UserWarning; use Overblog\GraphQLGenerator\Generator\TypeGenerator as BaseTypeGenerator; use Symfony\Component\Filesystem\Filesystem; @@ -93,10 +91,6 @@ protected function generatePublic(array $value) $publicCallback = $this->callableCallbackFromArrayValue($value, 'public', '$typeName, $fieldName'); - if ('null' === $publicCallback) { - return $publicCallback; - } - $code = <<<'CODE' function ($fieldName) { $publicCallback = %s; @@ -109,54 +103,27 @@ function ($fieldName) { return $code; } + protected function generateAccess(array $value) + { + if (!$this->arrayKeyExistsAndIsNotNull($value, 'access')) { + return 'null'; + } + + if (is_bool($value['access'])) { + return $this->varExport($value['access']); + } + + return $this->callableCallbackFromArrayValue($value, 'access', '$value, $args, $context, \\GraphQL\\Type\\Definition\\ResolveInfo $info, $object'); + } + protected function generateResolve(array $value) { - $accessIsSet = $this->arrayKeyExistsAndIsNotNull($value, 'access'); $fieldOptions = $value; if (!$this->arrayKeyExistsAndIsNotNull($fieldOptions, 'resolve')) { $fieldOptions['resolve'] = $this->defaultResolver; } - $resolveCallback = parent::generateResolve($fieldOptions); - $resolveCallback = ltrim($this->prefixCodeWithSpaces($resolveCallback)); - if (!$accessIsSet || true === $fieldOptions['access']) { // access granted to this field - if ('null' === $resolveCallback) { - return $resolveCallback; - } - - $argumentClass = $this->shortenClassName(Argument::class); - $resolveInfoClass = $this->shortenClassName(ResolveInfo::class); - - $code = <<<'CODE' -function ($value, $args, $context, %s $info) { -$resolverCallback = %s; -return call_user_func_array($resolverCallback, [$value, new %s($args), $context, $info]); -} -CODE; - - return sprintf($code, $resolveInfoClass, $resolveCallback, $argumentClass); - } elseif ($accessIsSet && false === $fieldOptions['access']) { // access deny to this field - $exceptionClass = $this->shortenClassName(UserWarning::class); - - return sprintf('function () { throw new %s(\'Access denied to this field.\'); }', $exceptionClass); - } else { // wrap resolver with access - $accessChecker = $this->callableCallbackFromArrayValue($fieldOptions, 'access', '$value, $args, $context, '.ResolveInfo::class.' $info, $object'); - $resolveInfoClass = $this->shortenClassName(ResolveInfo::class); - $argumentClass = $this->shortenClassName(Argument::class); - - $code = <<<'CODE' -function ($value, $args, $context, %s $info) { -$resolverCallback = %s; -$accessChecker = %s; -$isMutation = $info instanceof ResolveInfo && 'mutation' === $info->operation->operation && $info->parentType === $info->schema->getMutationType(); -return $container->get('overblog_graphql.access_resolver')->resolve($accessChecker, $resolverCallback, [$value, new %s($args), $context, $info], $isMutation); -} -CODE; - - $code = sprintf($code, $resolveInfoClass, $resolveCallback, $accessChecker, $argumentClass); - - return $code; - } + return $resolveCallback = parent::generateResolve($fieldOptions); } /** diff --git a/Resources/config/definition_config_processors.yml b/Resources/config/definition_config_processors.yml index 036840cda..a23ad65b4 100644 --- a/Resources/config/definition_config_processors.yml +++ b/Resources/config/definition_config_processors.yml @@ -5,10 +5,18 @@ services: tags: - { name: overblog_graphql.definition_config_processor, priority: 2048 } - Overblog\GraphQLBundle\Definition\ConfigProcessor\InjectServices: - class: Overblog\GraphQLBundle\Definition\ConfigProcessor\InjectServices + Overblog\GraphQLBundle\Definition\ConfigProcessor\InjectServicesConfigProcessor: + class: Overblog\GraphQLBundle\Definition\ConfigProcessor\InjectServicesConfigProcessor public: false arguments: - '@service_container' + tags: + - { name: overblog_graphql.definition_config_processor, priority: 1024 } + + Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor: + class: Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor + arguments: + - '@overblog_graphql.access_resolver' + public: false tags: - { name: overblog_graphql.definition_config_processor } diff --git a/Resources/skeleton/OutputFieldConfig.php.skeleton b/Resources/skeleton/OutputFieldConfig.php.skeleton index 7dec5111e..1dcee2bb8 100644 --- a/Resources/skeleton/OutputFieldConfig.php.skeleton +++ b/Resources/skeleton/OutputFieldConfig.php.skeleton @@ -5,5 +5,7 @@ 'description' => , 'deprecationReason' => , 'complexity' => , +# public and access are custom options managed only by the bundle 'public' => , +'access' => , ], From d1aa0fb3e4bfd8d60ba491e3648ce18b7d0fb359 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sun, 7 Jan 2018 13:25:13 +0100 Subject: [PATCH 04/14] Improve test coverage --- Definition/ConfigProcessor.php | 8 +-- Definition/LazyConfig.php | 13 ----- .../ConfigProcessor/NullConfigProcessor.php | 17 ++++++ Tests/Definition/ConfigProcessorTest.php | 57 +++++++++++++++++++ 4 files changed, 78 insertions(+), 17 deletions(-) create mode 100644 Tests/Definition/ConfigProcessor/NullConfigProcessor.php create mode 100644 Tests/Definition/ConfigProcessorTest.php diff --git a/Definition/ConfigProcessor.php b/Definition/ConfigProcessor.php index 22707e450..781b4750a 100644 --- a/Definition/ConfigProcessor.php +++ b/Definition/ConfigProcessor.php @@ -14,7 +14,7 @@ final class ConfigProcessor implements ConfigProcessorInterface /** * @var bool */ - private $isProcessed = false; + private $isInitialized = false; public function addConfigProcessor(ConfigProcessorInterface $configProcessor, $priority = 0) { @@ -23,7 +23,7 @@ public function addConfigProcessor(ConfigProcessorInterface $configProcessor, $p public function register(ConfigProcessorInterface $configProcessor, $priority = 0) { - if ($this->isProcessed) { + if ($this->isInitialized) { throw new \LogicException('Registering config processor after calling process() is not supported.'); } $this->processors[] = ['processor' => $configProcessor, 'priority' => $priority]; @@ -41,7 +41,7 @@ public function process(LazyConfig $lazyConfig) private function initialize() { - if (!$this->isProcessed) { + if (!$this->isInitialized) { // order processors by DESC priority usort($this->processors, function ($processorA, $processorB) { if ($processorA['priority'] === $processorB['priority']) { @@ -52,7 +52,7 @@ private function initialize() }); $this->processors = array_column($this->processors, 'processor'); - $this->isProcessed = true; + $this->isInitialized = true; } } } diff --git a/Definition/LazyConfig.php b/Definition/LazyConfig.php index 28b685423..384a28878 100644 --- a/Definition/LazyConfig.php +++ b/Definition/LazyConfig.php @@ -45,19 +45,6 @@ public function addPostLoader(callable $postLoader) $this->onPostLoad[] = $postLoader; } - /** - * @return \Closure - */ - public function getLoader() - { - return $this->loader; - } - - public function setLoader(\Closure $loader) - { - $this->loader = $loader; - } - /** * @return \ArrayObject */ diff --git a/Tests/Definition/ConfigProcessor/NullConfigProcessor.php b/Tests/Definition/ConfigProcessor/NullConfigProcessor.php new file mode 100644 index 000000000..7035a473c --- /dev/null +++ b/Tests/Definition/ConfigProcessor/NullConfigProcessor.php @@ -0,0 +1,17 @@ +addConfigProcessor(new NullConfigProcessor()); + + $configProcessor->process(LazyConfig::create(function () { + return []; + })); + + $configProcessor->addConfigProcessor(new NullConfigProcessor()); + } + + public function testOrderByPriorityDesc() + { + $configProcessor = new ConfigProcessor(); + + $configProcessor->addConfigProcessor($nullConfigProcessor1 = new NullConfigProcessor()); + $configProcessor->addConfigProcessor($nullConfigProcessor2 = new NullConfigProcessor(), 4); + $configProcessor->addConfigProcessor($nullConfigProcessor3 = new NullConfigProcessor(), 256); + $configProcessor->addConfigProcessor($nullConfigProcessor4 = new NullConfigProcessor()); + $configProcessor->addConfigProcessor($nullConfigProcessor5 = new NullConfigProcessor(), 512); + + $configProcessor->process(LazyConfig::create(function () { + return []; + })); + + $getProcessors = \Closure::bind( + function () { + return $this->processors; + }, + $configProcessor, + get_class($configProcessor) + ); + + $processors = $getProcessors(); + + $this->assertSame( + $processors, + [$nullConfigProcessor5, $nullConfigProcessor3, $nullConfigProcessor2, $nullConfigProcessor1, $nullConfigProcessor4] + ); + } +} From ad8930ccba5208aabd5684b49855d3b6aaa282b7 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sun, 7 Jan 2018 16:56:37 +0100 Subject: [PATCH 05/14] Use native resolveField --- Config/ObjectTypeDefinition.php | 28 --------------- Definition/ConfigProcessor.php | 12 +++++-- .../ConfigProcessor/AclConfigProcessor.php | 36 +++++++++++++++---- Generator/TypeGenerator.php | 14 -------- .../config/definition_config_processors.yml | 1 + Resources/config/services.yml | 1 - Tests/Definition/ConfigProcessorTest.php | 6 ++-- 7 files changed, 43 insertions(+), 55 deletions(-) diff --git a/Config/ObjectTypeDefinition.php b/Config/ObjectTypeDefinition.php index c57aafc5c..0c1bc7b0f 100644 --- a/Config/ObjectTypeDefinition.php +++ b/Config/ObjectTypeDefinition.php @@ -32,7 +32,6 @@ public function getDefinition() $this->treatFieldsDefaultAccess($node); $this->treatFieldsDefaultPublic($node); - $this->treatResolveField($node); return $node; } @@ -86,31 +85,4 @@ private function treatFieldsDefaultPublic(ArrayNodeDefinition $node) }) ->end(); } - - /** - * resolveField is set as fields default resolver if not set - * then remove resolveField to keep "access" feature - * TODO(mcg-web) : get a cleaner way to use resolveField combine with "access" feature. - * - * @param ArrayNodeDefinition $node - */ - private function treatResolveField(ArrayNodeDefinition $node) - { - $node->validate() - ->ifTrue(function ($v) { - return array_key_exists('resolveField', $v) && null !== $v['resolveField']; - }) - ->then(function ($v) { - $resolveField = $v['resolveField']; - unset($v['resolveField']); - foreach ($v['fields'] as &$field) { - if (empty($field['resolve'])) { - $field['resolve'] = $resolveField; - } - } - - return $v; - }) - ->end(); - } } diff --git a/Definition/ConfigProcessor.php b/Definition/ConfigProcessor.php index 781b4750a..d5489636b 100644 --- a/Definition/ConfigProcessor.php +++ b/Definition/ConfigProcessor.php @@ -9,6 +9,11 @@ final class ConfigProcessor implements ConfigProcessorInterface /** * @var ConfigProcessorInterface[] */ + private $orderedProcessors; + + /** + * @var array + */ private $processors; /** @@ -32,7 +37,7 @@ public function register(ConfigProcessorInterface $configProcessor, $priority = public function process(LazyConfig $lazyConfig) { $this->initialize(); - foreach ($this->processors as $processor) { + foreach ($this->orderedProcessors as $processor) { $lazyConfig = $processor->process($lazyConfig); } @@ -43,7 +48,8 @@ private function initialize() { if (!$this->isInitialized) { // order processors by DESC priority - usort($this->processors, function ($processorA, $processorB) { + $processors = $this->processors; + usort($processors, function ($processorA, $processorB) { if ($processorA['priority'] === $processorB['priority']) { return 0; } @@ -51,7 +57,7 @@ private function initialize() return ($processorA['priority'] < $processorB['priority']) ? 1 : -1; }); - $this->processors = array_column($this->processors, 'processor'); + $this->orderedProcessors = array_column($processors, 'processor'); $this->isInitialized = true; } } diff --git a/Definition/ConfigProcessor/AclConfigProcessor.php b/Definition/ConfigProcessor/AclConfigProcessor.php index 9235eb2f2..2e9302bcf 100644 --- a/Definition/ConfigProcessor/AclConfigProcessor.php +++ b/Definition/ConfigProcessor/AclConfigProcessor.php @@ -13,12 +13,16 @@ final class AclConfigProcessor implements ConfigProcessorInterface /** @var AccessResolver */ private $accessResolver; - public function __construct(AccessResolver $accessResolver) + /** @var callable */ + private $defaultResolver; + + public function __construct(AccessResolver $accessResolver, callable $defaultResolver) { $this->accessResolver = $accessResolver; + $this->defaultResolver = $defaultResolver; } - public static function acl(array $fields, AccessResolver $accessResolver) + public static function acl(array $fields, AccessResolver $accessResolver, callable $defaultResolver) { $deniedAccess = static function () { throw new UserWarning('Access denied to this field.'); @@ -28,9 +32,9 @@ public static function acl(array $fields, AccessResolver $accessResolver) $accessChecker = $field['access']; if (false === $accessChecker) { $field['resolve'] = $deniedAccess; - } elseif (is_callable($accessChecker) && isset($field['resolve'])) { // todo manage when resolver is not set - $field['resolve'] = static function ($value, $args, $context, ResolveInfo $info) use ($field, $accessChecker, $accessResolver) { - $resolverCallback = $field['resolve']; + } elseif (is_callable($accessChecker)) { + $field['resolve'] = function ($value, $args, $context, ResolveInfo $info) use ($field, $accessChecker, $accessResolver, $defaultResolver) { + $resolverCallback = self::findFieldResolver($field, $info, $defaultResolver); $isMutation = 'mutation' === $info->operation->operation && $info->parentType === $info->schema->getMutationType(); return $accessResolver->resolve($accessChecker, $resolverCallback, [$value, new Argument($args), $context, $info], $isMutation); @@ -49,7 +53,7 @@ public function process(LazyConfig $lazyConfig) $config['fields'] = function () use ($config) { $fields = $config['fields'](); - return static::acl($fields, $this->accessResolver); + return static::acl($fields, $this->accessResolver, $this->defaultResolver); }; } @@ -58,4 +62,24 @@ public function process(LazyConfig $lazyConfig) return $lazyConfig; } + + /** + * @param array $field + * @param ResolveInfo $info + * @param callable $defaultResolver + * + * @return callable + */ + private static function findFieldResolver(array $field, ResolveInfo $info, callable $defaultResolver) + { + if (isset($field['resolve'])) { + $resolver = $field['resolve']; + } elseif (isset($info->parentType->config['resolveField'])) { + $resolver = $info->parentType->config['resolveField']; + } else { + $resolver = $defaultResolver; + } + + return $resolver; + } } diff --git a/Generator/TypeGenerator.php b/Generator/TypeGenerator.php index 301d9d966..93de4f8f8 100644 --- a/Generator/TypeGenerator.php +++ b/Generator/TypeGenerator.php @@ -16,8 +16,6 @@ class TypeGenerator extends BaseTypeGenerator private $cacheDir; - private $defaultResolver; - private $configProcessor; private $configs; @@ -30,13 +28,11 @@ public function __construct( $classNamespace, array $skeletonDirs, $cacheDir, - callable $defaultResolver, array $configs, $useClassMap = true, callable $configProcessor = null) { $this->setCacheDir($cacheDir); - $this->defaultResolver = $defaultResolver; $this->configProcessor = null === $configProcessor ? static::DEFAULT_CONFIG_PROCESSOR : $configProcessor; $this->configs = $configs; $this->useClassMap = $useClassMap; @@ -116,16 +112,6 @@ protected function generateAccess(array $value) return $this->callableCallbackFromArrayValue($value, 'access', '$value, $args, $context, \\GraphQL\\Type\\Definition\\ResolveInfo $info, $object'); } - protected function generateResolve(array $value) - { - $fieldOptions = $value; - if (!$this->arrayKeyExistsAndIsNotNull($fieldOptions, 'resolve')) { - $fieldOptions['resolve'] = $this->defaultResolver; - } - - return $resolveCallback = parent::generateResolve($fieldOptions); - } - /** * @param array $value * diff --git a/Resources/config/definition_config_processors.yml b/Resources/config/definition_config_processors.yml index a23ad65b4..0aacbfa53 100644 --- a/Resources/config/definition_config_processors.yml +++ b/Resources/config/definition_config_processors.yml @@ -17,6 +17,7 @@ services: class: Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor arguments: - '@overblog_graphql.access_resolver' + - "%overblog_graphql.default_resolver%" public: false tags: - { name: overblog_graphql.definition_config_processor } diff --git a/Resources/config/services.yml b/Resources/config/services.yml index f67fe048f..8adc249b4 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -82,7 +82,6 @@ services: - "%overblog_graphql.class_namespace%" - ["%overblog_graphql.resources_dir%/skeleton"] - "%overblog_graphql.cache_dir%" - - "%overblog_graphql.default_resolver%" - "%overblog_graphql_types.config%" - "%overblog_graphql.use_classloader_listener%" calls: diff --git a/Tests/Definition/ConfigProcessorTest.php b/Tests/Definition/ConfigProcessorTest.php index b7899c543..b23b56c71 100644 --- a/Tests/Definition/ConfigProcessorTest.php +++ b/Tests/Definition/ConfigProcessorTest.php @@ -39,15 +39,15 @@ public function testOrderByPriorityDesc() return []; })); - $getProcessors = \Closure::bind( + $getOrderedProcessors = \Closure::bind( function () { - return $this->processors; + return $this->orderedProcessors; }, $configProcessor, get_class($configProcessor) ); - $processors = $getProcessors(); + $processors = $getOrderedProcessors(); $this->assertSame( $processors, From 645b2d19c7f9238f5462c86ec2df76ccaf5e095f Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sun, 7 Jan 2018 22:24:12 +0100 Subject: [PATCH 06/14] Introduce ResolverMap --- Config/InterfaceTypeDefinition.php | 5 ++- Config/ObjectTypeDefinition.php | 4 ++- Config/TypeDefinition.php | 40 +++++++++++++++++++++++ Config/TypeWithOutputFieldsDefinition.php | 2 +- Config/UnionTypeDefinition.php | 3 ++ 5 files changed, 51 insertions(+), 3 deletions(-) diff --git a/Config/InterfaceTypeDefinition.php b/Config/InterfaceTypeDefinition.php index 07556383c..5a3c14889 100644 --- a/Config/InterfaceTypeDefinition.php +++ b/Config/InterfaceTypeDefinition.php @@ -14,11 +14,14 @@ public function getDefinition() $node ->children() ->append($this->nameSection()) - ->append($this->outputFieldsSelection('fields')) + ->append($this->outputFieldsSelection()) ->append($this->resolveTypeSection()) + ->append($this->resolverMapSection()) ->append($this->descriptionSection()) ->end(); + $this->validateResolverMap($node); + return $node; } } diff --git a/Config/ObjectTypeDefinition.php b/Config/ObjectTypeDefinition.php index 0c1bc7b0f..c8427d198 100644 --- a/Config/ObjectTypeDefinition.php +++ b/Config/ObjectTypeDefinition.php @@ -15,13 +15,14 @@ public function getDefinition() $node ->children() ->append($this->nameSection()) - ->append($this->outputFieldsSelection('fields')) + ->append($this->outputFieldsSelection()) ->append($this->descriptionSection()) ->arrayNode('interfaces') ->prototype('scalar')->info('One of internal or custom interface types.')->end() ->end() ->variableNode('isTypeOf')->end() ->variableNode('resolveField')->end() + ->append($this->resolverMapSection()) ->variableNode('fieldsDefaultAccess') ->info('Default access control to fields (expression language can be use here)') ->end() @@ -32,6 +33,7 @@ public function getDefinition() $this->treatFieldsDefaultAccess($node); $this->treatFieldsDefaultPublic($node); + $this->validateResolverMap($node); return $node; } diff --git a/Config/TypeDefinition.php b/Config/TypeDefinition.php index 4a8a5e097..18f235978 100644 --- a/Config/TypeDefinition.php +++ b/Config/TypeDefinition.php @@ -2,6 +2,7 @@ namespace Overblog\GraphQLBundle\Config; +use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; abstract class TypeDefinition @@ -28,6 +29,45 @@ protected function resolveTypeSection() return $node; } + protected function resolverMapSection() + { + $builder = new TreeBuilder(); + $node = $builder->root('resolverMap', 'scalar'); + + return $node; + } + + protected function validateResolverMap(NodeDefinition $node) + { + $node->validate() + ->ifTrue(function ($config) { + if (isset($config['resolverMap'])) { + foreach ($config['fields'] as $field) { + if (isset($field['resolve'])) { + return true; + } + } + + return isset($config['resolveField']); + } + + return false; + }) + ->then(function () { + throw new \InvalidArgumentException('ResolverMap should not be combine with resolveField or fields.*.resolve.'); + }); + + $node->validate() + ->ifTrue(function ($config) { + return isset($config['resolverMap']) && isset($config['resolveType']); + }) + ->then(function () { + throw new \InvalidArgumentException('ResolverMap should not be combine with resolveType.'); + }); + + return $node; + } + protected function nameSection() { $builder = new TreeBuilder(); diff --git a/Config/TypeWithOutputFieldsDefinition.php b/Config/TypeWithOutputFieldsDefinition.php index 355d486f5..e81232d54 100644 --- a/Config/TypeWithOutputFieldsDefinition.php +++ b/Config/TypeWithOutputFieldsDefinition.php @@ -12,7 +12,7 @@ abstract class TypeWithOutputFieldsDefinition extends TypeDefinition * * @return ArrayNodeDefinition|\Symfony\Component\Config\Definition\Builder\NodeDefinition */ - protected function outputFieldsSelection($name) + protected function outputFieldsSelection($name = 'fields') { $builder = new TreeBuilder(); $node = $builder->root($name); diff --git a/Config/UnionTypeDefinition.php b/Config/UnionTypeDefinition.php index a5ddf1410..47a965168 100644 --- a/Config/UnionTypeDefinition.php +++ b/Config/UnionTypeDefinition.php @@ -22,9 +22,12 @@ public function getDefinition() ->requiresAtLeastOneElement() ->end() ->append($this->resolveTypeSection()) + ->append($this->resolverMapSection()) ->append($this->descriptionSection()) ->end(); + $this->validateResolverMap($node); + return $node; } } From ed506fef4cbd45726791d8a6cc10a08cd674a14f Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Fri, 12 Jan 2018 17:20:34 +0100 Subject: [PATCH 07/14] Improve variables injection --- .../InjectServicesConfigProcessor.php | 45 ------------------- .../VariablesInjectorConfigProcessor.php | 37 +++++++++++++++ ....php => DefinitionConfigProcessorPass.php} | 4 +- .../Compiler/ExpressionFunctionPass.php | 2 +- .../Compiler/TaggedServiceMappingPass.php | 2 +- .../VariablesInjectorConfigProcessorPass.php | 27 +++++++++++ .../DependencyInjection/Parameter.php | 2 +- .../DependencyInjection/Service.php | 2 +- .../ExpressionFunction/GraphQL/Mutation.php | 2 +- .../ExpressionFunction/GraphQL/Resolver.php | 2 +- .../ExpressionFunction/Security/GetUser.php | 18 ++++++++ .../Security/HasAnyPermission.php | 2 +- .../Security/HasAnyRole.php | 2 +- .../Security/HasPermission.php | 2 +- .../ExpressionFunction/Security/HasRole.php | 2 +- .../Security/IsAnonymous.php | 2 +- .../Security/IsAuthenticated.php | 2 +- .../Security/IsFullyAuthenticated.php | 2 +- .../Security/IsRememberMe.php | 2 +- ExpressionLanguage/ExpressionLanguage.php | 20 +++++---- Generator/TypeGenerator.php | 6 +-- OverblogGraphQLBundle.php | 6 ++- .../config/definition_config_processors.yml | 8 ---- .../config/expression_language_functions.yml | 6 +++ Resources/config/services.yml | 2 - .../doc/definitions/expression-language.md | 3 +- Resources/skeleton/TypeSystem.php.skeleton | 2 - .../DependencyInjection/ParameterTest.php | 5 +-- .../DependencyInjection/ServiceTest.php | 5 +-- Tests/ExpressionLanguage/TestCase.php | 11 ++--- .../config/access/mapping/access.types.yml | 4 +- UPGRADE-0.11.md | 14 +++++- 32 files changed, 148 insertions(+), 103 deletions(-) delete mode 100644 Definition/ConfigProcessor/InjectServicesConfigProcessor.php create mode 100644 Definition/ConfigProcessor/VariablesInjectorConfigProcessor.php rename DependencyInjection/Compiler/{DefinitionConfigProcessorCompilerPass.php => DefinitionConfigProcessorPass.php} (87%) create mode 100644 DependencyInjection/Compiler/VariablesInjectorConfigProcessorPass.php create mode 100644 ExpressionLanguage/ExpressionFunction/Security/GetUser.php diff --git a/Definition/ConfigProcessor/InjectServicesConfigProcessor.php b/Definition/ConfigProcessor/InjectServicesConfigProcessor.php deleted file mode 100644 index 227cfefd6..000000000 --- a/Definition/ConfigProcessor/InjectServicesConfigProcessor.php +++ /dev/null @@ -1,45 +0,0 @@ -container = $container; - } - - /** - * {@inheritdoc} - */ - public function process(LazyConfig $lazyConfig) - { - $vars = $lazyConfig->getVars(); - $container = $this->container; - $user = null; - $request = null; - $token = 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(); - } - } - $vars['token'] = $token; - $vars['container'] = $container; - $vars['user'] = $user; - $vars['request'] = $request; - - return $lazyConfig; - } -} diff --git a/Definition/ConfigProcessor/VariablesInjectorConfigProcessor.php b/Definition/ConfigProcessor/VariablesInjectorConfigProcessor.php new file mode 100644 index 000000000..425aaef0b --- /dev/null +++ b/Definition/ConfigProcessor/VariablesInjectorConfigProcessor.php @@ -0,0 +1,37 @@ +expressionLanguage = $expressionLanguage; + } + + public function addVariable($name, $value = null) + { + $this->variables[$name] = $value; + $this->expressionLanguage->addGlobalName(sprintf('vars[\'%s\']', $name), $name); + } + + /** + * {@inheritdoc} + */ + public function process(LazyConfig $lazyConfig) + { + $vars = $lazyConfig->getVars(); + foreach ($this->variables as $name => $value) { + $vars[$name] = $value; + } + + return $lazyConfig; + } +} diff --git a/DependencyInjection/Compiler/DefinitionConfigProcessorCompilerPass.php b/DependencyInjection/Compiler/DefinitionConfigProcessorPass.php similarity index 87% rename from DependencyInjection/Compiler/DefinitionConfigProcessorCompilerPass.php rename to DependencyInjection/Compiler/DefinitionConfigProcessorPass.php index 0d7a95e19..c22f45756 100644 --- a/DependencyInjection/Compiler/DefinitionConfigProcessorCompilerPass.php +++ b/DependencyInjection/Compiler/DefinitionConfigProcessorPass.php @@ -7,7 +7,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; -final class DefinitionConfigProcessorCompilerPass implements CompilerPassInterface +final class DefinitionConfigProcessorPass implements CompilerPassInterface { /** * {@inheritdoc} @@ -15,7 +15,7 @@ final class DefinitionConfigProcessorCompilerPass implements CompilerPassInterfa public function process(ContainerBuilder $container) { $definition = $container->findDefinition(ConfigProcessor::class); - $taggedServices = $container->findTaggedServiceIds('overblog_graphql.definition_config_processor'); + $taggedServices = $container->findTaggedServiceIds('overblog_graphql.definition_config_processor', true); foreach ($taggedServices as $id => $tags) { $definition->addMethodCall( diff --git a/DependencyInjection/Compiler/ExpressionFunctionPass.php b/DependencyInjection/Compiler/ExpressionFunctionPass.php index 99d8c5879..0dcc3910d 100644 --- a/DependencyInjection/Compiler/ExpressionFunctionPass.php +++ b/DependencyInjection/Compiler/ExpressionFunctionPass.php @@ -14,7 +14,7 @@ final class ExpressionFunctionPass implements CompilerPassInterface public function process(ContainerBuilder $container) { $definition = $container->findDefinition('overblog_graphql.expression_language'); - $taggedServices = $container->findTaggedServiceIds('overblog_graphql.expression_function'); + $taggedServices = $container->findTaggedServiceIds('overblog_graphql.expression_function', true); foreach ($taggedServices as $id => $tags) { $definition->addMethodCall('addFunction', [new Reference($id)]); diff --git a/DependencyInjection/Compiler/TaggedServiceMappingPass.php b/DependencyInjection/Compiler/TaggedServiceMappingPass.php index 31dcea4f0..ee47a440b 100644 --- a/DependencyInjection/Compiler/TaggedServiceMappingPass.php +++ b/DependencyInjection/Compiler/TaggedServiceMappingPass.php @@ -13,7 +13,7 @@ private function getTaggedServiceMapping(ContainerBuilder $container, $tagName) { $serviceMapping = []; - $taggedServices = $container->findTaggedServiceIds($tagName); + $taggedServices = $container->findTaggedServiceIds($tagName, true); $isType = TypeTaggedServiceMappingPass::TAG_NAME === $tagName; foreach ($taggedServices as $id => $tags) { diff --git a/DependencyInjection/Compiler/VariablesInjectorConfigProcessorPass.php b/DependencyInjection/Compiler/VariablesInjectorConfigProcessorPass.php new file mode 100644 index 000000000..c7dddacd8 --- /dev/null +++ b/DependencyInjection/Compiler/VariablesInjectorConfigProcessorPass.php @@ -0,0 +1,27 @@ +register(VariablesInjectorConfigProcessor::class, VariablesInjectorConfigProcessor::class) + ->addTag('overblog_graphql.definition_config_processor', ['priority' => 1024]) + ->setArguments([new Reference('overblog_graphql.expression_language')]) + ; + + foreach (['container' => 'service_container', 'token' => 'security.token_storage', 'request' => 'request_stack'] as $name => $id) { + $definition->addMethodCall('addVariable', [$name, new Reference($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)]); + } + } +} diff --git a/ExpressionLanguage/ExpressionFunction/DependencyInjection/Parameter.php b/ExpressionLanguage/ExpressionFunction/DependencyInjection/Parameter.php index 4a4a96f66..9c82e4a22 100644 --- a/ExpressionLanguage/ExpressionFunction/DependencyInjection/Parameter.php +++ b/ExpressionLanguage/ExpressionFunction/DependencyInjection/Parameter.php @@ -11,7 +11,7 @@ public function __construct($name = 'parameter') parent::__construct( $name, function ($value) { - return sprintf('$container->getParameter(%s)', $value); + return sprintf('$vars[\'container\']->getParameter(%s)', $value); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/DependencyInjection/Service.php b/ExpressionLanguage/ExpressionFunction/DependencyInjection/Service.php index a49bbac98..ff1ff6f1b 100644 --- a/ExpressionLanguage/ExpressionFunction/DependencyInjection/Service.php +++ b/ExpressionLanguage/ExpressionFunction/DependencyInjection/Service.php @@ -11,7 +11,7 @@ public function __construct($name = 'service') parent::__construct( $name, function ($value) { - return sprintf('$container->get(%s)', $value); + return sprintf('$vars[\'container\']->get(%s)', $value); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/GraphQL/Mutation.php b/ExpressionLanguage/ExpressionFunction/GraphQL/Mutation.php index f824ac7fa..4cb7db4b3 100644 --- a/ExpressionLanguage/ExpressionFunction/GraphQL/Mutation.php +++ b/ExpressionLanguage/ExpressionFunction/GraphQL/Mutation.php @@ -11,7 +11,7 @@ public function __construct($name = 'mutation') parent::__construct( $name, function ($alias, $args = '[]') { - return sprintf('$container->get(\'overblog_graphql.mutation_resolver\')->resolve([%s, %s])', $alias, $args); + return sprintf('$vars[\'container\']->get(\'overblog_graphql.mutation_resolver\')->resolve([%s, %s])', $alias, $args); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/GraphQL/Resolver.php b/ExpressionLanguage/ExpressionFunction/GraphQL/Resolver.php index 52a2bfed7..c42d19e3b 100644 --- a/ExpressionLanguage/ExpressionFunction/GraphQL/Resolver.php +++ b/ExpressionLanguage/ExpressionFunction/GraphQL/Resolver.php @@ -11,7 +11,7 @@ public function __construct($name = 'resolver') parent::__construct( $name, function ($alias, $args = '[]') { - return sprintf('$container->get(\'overblog_graphql.resolver_resolver\')->resolve([%s, %s])', $alias, $args); + return sprintf('$vars[\'container\']->get(\'overblog_graphql.resolver_resolver\')->resolve([%s, %s])', $alias, $args); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/GetUser.php b/ExpressionLanguage/ExpressionFunction/Security/GetUser.php new file mode 100644 index 000000000..68aac9e92 --- /dev/null +++ b/ExpressionLanguage/ExpressionFunction/Security/GetUser.php @@ -0,0 +1,18 @@ +getUser() : null'; + } + ); + } +} diff --git a/ExpressionLanguage/ExpressionFunction/Security/HasAnyPermission.php b/ExpressionLanguage/ExpressionFunction/Security/HasAnyPermission.php index e811ca859..0d3eaf576 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/HasAnyPermission.php +++ b/ExpressionLanguage/ExpressionFunction/Security/HasAnyPermission.php @@ -11,7 +11,7 @@ public function __construct($name = 'hasAnyPermission') parent::__construct( $name, function ($object, $permissions) { - $code = sprintf('array_reduce(%s, function ($isGranted, $permission) use ($container, $object) { return $isGranted || $container->get(\'security.authorization_checker\')->isGranted($permission, %s); }, false)', $permissions, $object); + $code = sprintf('array_reduce(%s, function ($isGranted, $permission) use ($vars, $object) { return $isGranted || $vars[\'container\']->get(\'security.authorization_checker\')->isGranted($permission, %s); }, false)', $permissions, $object); return $code; } diff --git a/ExpressionLanguage/ExpressionFunction/Security/HasAnyRole.php b/ExpressionLanguage/ExpressionFunction/Security/HasAnyRole.php index 18845c7cb..edc098efe 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/HasAnyRole.php +++ b/ExpressionLanguage/ExpressionFunction/Security/HasAnyRole.php @@ -11,7 +11,7 @@ public function __construct($name = 'hasAnyRole') parent::__construct( $name, function ($roles) { - $code = sprintf('array_reduce(%s, function ($isGranted, $role) use ($container) { return $isGranted || $container->get(\'security.authorization_checker\')->isGranted($role); }, false)', $roles); + $code = sprintf('array_reduce(%s, function ($isGranted, $role) use ($vars) { return $isGranted || $vars[\'container\']->get(\'security.authorization_checker\')->isGranted($role); }, false)', $roles); return $code; } diff --git a/ExpressionLanguage/ExpressionFunction/Security/HasPermission.php b/ExpressionLanguage/ExpressionFunction/Security/HasPermission.php index d01a6bd1c..a8ff7e12e 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/HasPermission.php +++ b/ExpressionLanguage/ExpressionFunction/Security/HasPermission.php @@ -11,7 +11,7 @@ public function __construct($name = 'hasPermission') parent::__construct( $name, function ($object, $permission) { - $code = sprintf('$container->get(\'security.authorization_checker\')->isGranted(%s, %s)', $permission, $object); + $code = sprintf('$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(%s, %s)', $permission, $object); return $code; } diff --git a/ExpressionLanguage/ExpressionFunction/Security/HasRole.php b/ExpressionLanguage/ExpressionFunction/Security/HasRole.php index b04ea52b1..0de55e69b 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/HasRole.php +++ b/ExpressionLanguage/ExpressionFunction/Security/HasRole.php @@ -11,7 +11,7 @@ public function __construct($name = 'hasRole') parent::__construct( $name, function ($role) { - return sprintf('$container->get(\'security.authorization_checker\')->isGranted(%s)', $role); + return sprintf('$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(%s)', $role); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/IsAnonymous.php b/ExpressionLanguage/ExpressionFunction/Security/IsAnonymous.php index a4874110f..75097b62a 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/IsAnonymous.php +++ b/ExpressionLanguage/ExpressionFunction/Security/IsAnonymous.php @@ -11,7 +11,7 @@ public function __construct($name = 'isAnonymous') parent::__construct( $name, function () { - return '$container->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_ANONYMOUSLY\')'; + return '$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_ANONYMOUSLY\')'; } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/IsAuthenticated.php b/ExpressionLanguage/ExpressionFunction/Security/IsAuthenticated.php index d222e7949..a6eb8d5c8 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/IsAuthenticated.php +++ b/ExpressionLanguage/ExpressionFunction/Security/IsAuthenticated.php @@ -11,7 +11,7 @@ public function __construct($name = 'isAuthenticated') parent::__construct( $name, function () { - return '$container->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_REMEMBERED\') || $container->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_FULLY\')'; + return '$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_REMEMBERED\') || $vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_FULLY\')'; } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/IsFullyAuthenticated.php b/ExpressionLanguage/ExpressionFunction/Security/IsFullyAuthenticated.php index 2fb09b4c0..2bced1f32 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/IsFullyAuthenticated.php +++ b/ExpressionLanguage/ExpressionFunction/Security/IsFullyAuthenticated.php @@ -11,7 +11,7 @@ public function __construct($name = 'isFullyAuthenticated') parent::__construct( $name, function () { - return '$container->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_FULLY\')'; + return '$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_FULLY\')'; } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/IsRememberMe.php b/ExpressionLanguage/ExpressionFunction/Security/IsRememberMe.php index ff6fad79a..924cbe9d8 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/IsRememberMe.php +++ b/ExpressionLanguage/ExpressionFunction/Security/IsRememberMe.php @@ -11,7 +11,7 @@ public function __construct($name = 'isRememberMe') parent::__construct( $name, function () { - return '$container->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_REMEMBERED\')'; + return '$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_REMEMBERED\')'; } ); } diff --git a/ExpressionLanguage/ExpressionLanguage.php b/ExpressionLanguage/ExpressionLanguage.php index 26ef21019..dd6c1fd4c 100644 --- a/ExpressionLanguage/ExpressionLanguage.php +++ b/ExpressionLanguage/ExpressionLanguage.php @@ -2,21 +2,23 @@ namespace Overblog\GraphQLBundle\ExpressionLanguage; -use Symfony\Component\DependencyInjection\ContainerAwareTrait; use Symfony\Component\ExpressionLanguage\ExpressionLanguage as BaseExpressionLanguage; class ExpressionLanguage extends BaseExpressionLanguage { - use ContainerAwareTrait; + private $globalNames = []; - public function compile($expression, $names = []) + /** + * @param $index + * @param $name + */ + public function addGlobalName($index, $name) { - $names[] = 'container'; - $names[] = 'request'; - $names[] = 'security.token_storage'; - $names[] = 'token'; - $names[] = 'user'; + $this->globalNames[$index] = $name; + } - return parent::compile($expression, $names); + public function compile($expression, $names = []) + { + return parent::compile($expression, array_merge($names, $this->globalNames)); } } diff --git a/Generator/TypeGenerator.php b/Generator/TypeGenerator.php index 93de4f8f8..0622785c5 100644 --- a/Generator/TypeGenerator.php +++ b/Generator/TypeGenerator.php @@ -10,7 +10,7 @@ class TypeGenerator extends BaseTypeGenerator { - const USE_FOR_CLOSURES = '$container, $request, $user, $token'; + const USE_FOR_CLOSURES = '$vars'; const DEFAULT_CONFIG_PROCESSOR = [Processor::class, 'process']; @@ -71,12 +71,12 @@ protected function generateClassDocBlock(array $value) protected function generateClosureUseStatements(array $config) { - return 'use ('.static::USE_FOR_CLOSURES.') '; + return sprintf('use (%s) ', static::USE_FOR_CLOSURES); } protected function resolveTypeCode($alias) { - return sprintf('$container->get(\'%s\')->resolve(%s)', 'overblog_graphql.type_resolver', var_export($alias, true)); + return sprintf('$vars[\'container\']->get(\'%s\')->resolve(%s)', 'overblog_graphql.type_resolver', var_export($alias, true)); } protected function generatePublic(array $value) diff --git a/OverblogGraphQLBundle.php b/OverblogGraphQLBundle.php index 44f7584a8..c9d9ecb78 100644 --- a/OverblogGraphQLBundle.php +++ b/OverblogGraphQLBundle.php @@ -6,11 +6,12 @@ use Overblog\GraphQLBundle\DependencyInjection\Compiler\AutoMappingPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\AutowiringTypesPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ConfigTypesPass; -use Overblog\GraphQLBundle\DependencyInjection\Compiler\DefinitionConfigProcessorCompilerPass; +use Overblog\GraphQLBundle\DependencyInjection\Compiler\DefinitionConfigProcessorPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ExpressionFunctionPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\MutationTaggedServiceMappingTaggedPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverTaggedServiceMappingPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\TypeTaggedServiceMappingPass; +use Overblog\GraphQLBundle\DependencyInjection\Compiler\VariablesInjectorConfigProcessorPass; use Overblog\GraphQLBundle\DependencyInjection\OverblogGraphQLExtension; use Overblog\GraphQLBundle\DependencyInjection\OverblogGraphQLTypesExtension; use Symfony\Component\DependencyInjection\Compiler\PassConfig; @@ -29,7 +30,8 @@ public function build(ContainerBuilder $container) //ConfigTypesPass and AutoMappingPass must be before TypeTaggedServiceMappingPass $container->addCompilerPass(new ExpressionFunctionPass()); - $container->addCompilerPass(new DefinitionConfigProcessorCompilerPass()); + $container->addCompilerPass(new VariablesInjectorConfigProcessorPass()); + $container->addCompilerPass(new DefinitionConfigProcessorPass()); $container->addCompilerPass(new AutoMappingPass()); $container->addCompilerPass(new ConfigTypesPass(), PassConfig::TYPE_BEFORE_REMOVING); $container->addCompilerPass(new AliasedPass()); diff --git a/Resources/config/definition_config_processors.yml b/Resources/config/definition_config_processors.yml index 0aacbfa53..db7b0abee 100644 --- a/Resources/config/definition_config_processors.yml +++ b/Resources/config/definition_config_processors.yml @@ -5,14 +5,6 @@ services: tags: - { name: overblog_graphql.definition_config_processor, priority: 2048 } - Overblog\GraphQLBundle\Definition\ConfigProcessor\InjectServicesConfigProcessor: - class: Overblog\GraphQLBundle\Definition\ConfigProcessor\InjectServicesConfigProcessor - public: false - arguments: - - '@service_container' - tags: - - { name: overblog_graphql.definition_config_processor, priority: 1024 } - Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor: class: Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor arguments: diff --git a/Resources/config/expression_language_functions.yml b/Resources/config/expression_language_functions.yml index aaf827941..335f98c7e 100644 --- a/Resources/config/expression_language_functions.yml +++ b/Resources/config/expression_language_functions.yml @@ -1,5 +1,11 @@ services: # Authorization + Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\Security\GetUser: + class: Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\Security\GetUser + public: false + tags: + - { name: overblog_graphql.expression_function } + Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\Security\HasAnyPermission: class: Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\Security\HasAnyPermission public: false diff --git a/Resources/config/services.yml b/Resources/config/services.yml index 8adc249b4..7a1f88a79 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -72,8 +72,6 @@ services: public: false arguments: - '@overblog_graphql.cache_expression_language_parser' - calls: - - ["setContainer", ["@service_container"]] overblog_graphql.cache_compiler: class: Overblog\GraphQLBundle\Generator\TypeGenerator diff --git a/Resources/doc/definitions/expression-language.md b/Resources/doc/definitions/expression-language.md index 31ef93eaf..31a6a622a 100644 --- a/Resources/doc/definitions/expression-language.md +++ b/Resources/doc/definitions/expression-language.md @@ -23,6 +23,8 @@ boolean **isFullyAuthenticated**() | Checks whether the token is fully authentic boolean **isAuthenticated**() | Checks whether the token is not anonymous. | @=isAuthenticated() | boolean **hasPermission**(mixed $var, string $permission) | Checks whether the token has the given permission for the given object (requires the ACL system). | @=hasPermission(object, 'OWNER') | boolean **hasAnyPermission**(mixed $var, array $permissions) | Checks whether the token has any of the given permissions for the given object | @=hasAnyPermission(object, ['OWNER', 'ADMIN']) | +User **getUser**() | Returns the user which is currently in the security token storage. User can be null. | @=getUser() | + **Variables description:** @@ -31,7 +33,6 @@ Expression | Description | Scope **container** | DI container | global **request** | Refers to the current request. | Request **token** | Refers to the token which is currently in the security token storage. Token can be null. | Token -**user** | Refers to the user which is currently in the security token storage. User can be null. | Valid Token **object** | Refers to the value of the field for which access is being requested. For array `object` will be each item of the array. For Relay connection `object` will be the node of each connection edges. | only available for `config.fields.*.access` with query operation or mutation payload type. **value** | Resolver value | only available in resolve context **args** | Resolver args array | only available in resolve context diff --git a/Resources/skeleton/TypeSystem.php.skeleton b/Resources/skeleton/TypeSystem.php.skeleton index 60cc5ccde..69c71dda9 100644 --- a/Resources/skeleton/TypeSystem.php.skeleton +++ b/Resources/skeleton/TypeSystem.php.skeleton @@ -8,8 +8,6 @@ public function __construct(ConfigProcessor $configProcessor) { $configLoader = function(array $vars) { -extract($vars); - return ; }; $config = $configProcessor->process(\Overblog\GraphQLBundle\Definition\LazyConfig::create($configLoader))->load(); diff --git a/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ParameterTest.php b/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ParameterTest.php index fa8e7fa9d..70b0512e8 100644 --- a/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ParameterTest.php +++ b/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ParameterTest.php @@ -18,9 +18,8 @@ protected function getFunctions() */ public function testParameter($name) { - $container = $this->getDIContainerMock([], ['test' => 5]); - $this->expressionLanguage->setContainer($container); - $this->assertEquals(5, eval('return '.$this->expressionLanguage->compile($name.'("test")').';')); + $vars['container'] = $this->getDIContainerMock([], ['test' => 5]); + $this->assertSame(5, eval('return '.$this->expressionLanguage->compile($name.'("test")').';')); } public function getNames() diff --git a/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ServiceTest.php b/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ServiceTest.php index 36a6e9e90..05b752f65 100644 --- a/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ServiceTest.php +++ b/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ServiceTest.php @@ -19,9 +19,8 @@ protected function getFunctions() public function testService($name) { $object = new \stdClass(); - $container = $this->getDIContainerMock(['toto' => $object]); - $this->expressionLanguage->setContainer($container); - $this->assertEquals($object, eval('return '.$this->expressionLanguage->compile($name.'("toto")').';')); + $vars['container'] = $this->getDIContainerMock(['toto' => $object]); + $this->assertSame($object, eval('return '.$this->expressionLanguage->compile($name.'("toto")').';')); } public function getNames() diff --git a/Tests/ExpressionLanguage/TestCase.php b/Tests/ExpressionLanguage/TestCase.php index cca2c84c6..f9323848b 100644 --- a/Tests/ExpressionLanguage/TestCase.php +++ b/Tests/ExpressionLanguage/TestCase.php @@ -18,8 +18,6 @@ abstract class TestCase extends BaseTestCase public function setUp() { $this->expressionLanguage = new ExpressionLanguage(); - $container = $this->getDIContainerMock(); - $this->expressionLanguage->setContainer($container); foreach ($this->getFunctions() as $function) { $this->expressionLanguage->addFunction($function); } @@ -30,12 +28,11 @@ public function setUp() */ abstract protected function getFunctions(); - protected function assertExpressionCompile($expression, $with, array $expressionValues = [], $expects = null, $return = true, $assertMethod = 'assertTrue') + protected function assertExpressionCompile($expression, $with, array $vars = [], $expects = null, $return = true, $assertMethod = 'assertTrue') { - $expressionValues['container'] = $this->getDIContainerMock(['security.authorization_checker' => $this->getAuthorizationCheckerIsGrantedWithExpectation($with, $expects, $return)]); - extract($expressionValues); - - $code = $this->expressionLanguage->compile($expression, array_keys($expressionValues)); + extract($vars); + $code = $this->expressionLanguage->compile($expression, array_keys($vars)); + $vars['container'] = $this->getDIContainerMock(['security.authorization_checker' => $this->getAuthorizationCheckerIsGrantedWithExpectation($with, $expects, $return)]); $this->$assertMethod(eval('return '.$code.';')); } diff --git a/Tests/Functional/App/config/access/mapping/access.types.yml b/Tests/Functional/App/config/access/mapping/access.types.yml index 7836bbf68..0f549ae1b 100644 --- a/Tests/Functional/App/config/access/mapping/access.types.yml +++ b/Tests/Functional/App/config/access/mapping/access.types.yml @@ -6,7 +6,9 @@ RootQuery: user: type: User resolve: '@=resolver("query")' - + current_user: + type: User + resolve: '@=getUser()' Mutation: type: object config: diff --git a/UPGRADE-0.11.md b/UPGRADE-0.11.md index 9103d5a8c..99b75ffdd 100644 --- a/UPGRADE-0.11.md +++ b/UPGRADE-0.11.md @@ -4,8 +4,9 @@ UPGRADE FROM 0.10 to 0.11 # Table of Contents - [GraphiQL](#graphiql) -- [Errors Handler](#errors-handler) +- [Errors handler](#errors-handler) - [Promise adapter interface](#promise-adapter-interface) +- [Expression language](#expression-language) ### GraphiQL @@ -84,3 +85,14 @@ UPGRADE FROM 0.10 to 0.11 - public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null); + public function setPromiseAdapter(PromiseAdapter $promiseAdapter); ``` + +### Expression language + + * **user** expression variable has been replaced by **getUser** expression function + + Upgrading: + - Replace `user` by `getUser` in you schema configuration + ```diff + - resolve: '@=user' + + resolve: '@=getUser()' + ``` From df80a9b46f034ba553b1ed4416a85b3bb6c241ed Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sat, 13 Jan 2018 16:42:48 +0100 Subject: [PATCH 08/14] Refactor GlobalVariables --- ...GlobalVariablesInjectorConfigProcessor.php | 39 +++++++++ .../VariablesInjectorConfigProcessor.php | 37 --------- Definition/GlobalVariables.php | 38 +++++++++ Definition/LazyConfig.php | 18 ++-- .../DefinitionConfigProcessorPass.php | 16 ++-- .../Compiler/GlobalVariablesInjectorPass.php | 39 +++++++++ .../VariablesInjectorConfigProcessorPass.php | 27 ------ .../DependencyInjection/Parameter.php | 2 +- .../DependencyInjection/Service.php | 2 +- .../ExpressionFunction/GraphQL/Mutation.php | 2 +- .../ExpressionFunction/GraphQL/Resolver.php | 2 +- .../ExpressionFunction/Security/GetUser.php | 2 +- .../Security/HasAnyPermission.php | 3 +- .../Security/HasAnyRole.php | 3 +- .../Security/HasPermission.php | 2 +- .../ExpressionFunction/Security/HasRole.php | 2 +- .../ExpressionFunction/Security/Helper.php | 37 +++++++++ .../Security/IsAnonymous.php | 2 +- .../Security/IsAuthenticated.php | 2 +- .../Security/IsFullyAuthenticated.php | 2 +- .../Security/IsRememberMe.php | 2 +- Generator/TypeGenerator.php | 4 +- OverblogGraphQLBundle.php | 4 +- .../config/definition_config_processors.yml | 10 +++ Resources/config/services.yml | 11 ++- .../doc/definitions/expression-language.md | 4 +- Resources/skeleton/TypeSystem.php.skeleton | 4 +- Tests/DIContainerMockTrait.php | 7 +- .../DependencyInjection/ParameterTest.php | 4 +- .../DependencyInjection/ServiceTest.php | 4 +- .../Security/GetUserTest.php | 82 +++++++++++++++++++ Tests/ExpressionLanguage/TestCase.php | 10 ++- .../config/access/mapping/access.types.yml | 4 +- UPGRADE-0.11.md | 37 ++++++++- 34 files changed, 348 insertions(+), 116 deletions(-) create mode 100644 Definition/ConfigProcessor/GlobalVariablesInjectorConfigProcessor.php delete mode 100644 Definition/ConfigProcessor/VariablesInjectorConfigProcessor.php create mode 100644 Definition/GlobalVariables.php create mode 100644 DependencyInjection/Compiler/GlobalVariablesInjectorPass.php delete mode 100644 DependencyInjection/Compiler/VariablesInjectorConfigProcessorPass.php create mode 100644 ExpressionLanguage/ExpressionFunction/Security/Helper.php create mode 100644 Tests/ExpressionLanguage/ExpressionFunction/Security/GetUserTest.php diff --git a/Definition/ConfigProcessor/GlobalVariablesInjectorConfigProcessor.php b/Definition/ConfigProcessor/GlobalVariablesInjectorConfigProcessor.php new file mode 100644 index 000000000..fb09db8dd --- /dev/null +++ b/Definition/ConfigProcessor/GlobalVariablesInjectorConfigProcessor.php @@ -0,0 +1,39 @@ +expressionLanguage = $expressionLanguage; + } + + public function addGlobalVariable($name, $globalVariable, $isPublic = true) + { + $this->globalVariables[$name] = $globalVariable; + if ($isPublic) { + $this->expressionLanguage->addGlobalName(sprintf('globalVariables->get(\'%s\')', $name), $name); + } + } + + /** + * {@inheritdoc} + */ + public function process(LazyConfig $lazyConfig) + { + $globalVariables = $lazyConfig->getGlobalVariables(); + foreach ($this->globalVariables as $name => $variable) { + $globalVariables[$name] = $variable; + } + + return $lazyConfig; + } +} diff --git a/Definition/ConfigProcessor/VariablesInjectorConfigProcessor.php b/Definition/ConfigProcessor/VariablesInjectorConfigProcessor.php deleted file mode 100644 index 425aaef0b..000000000 --- a/Definition/ConfigProcessor/VariablesInjectorConfigProcessor.php +++ /dev/null @@ -1,37 +0,0 @@ -expressionLanguage = $expressionLanguage; - } - - public function addVariable($name, $value = null) - { - $this->variables[$name] = $value; - $this->expressionLanguage->addGlobalName(sprintf('vars[\'%s\']', $name), $name); - } - - /** - * {@inheritdoc} - */ - public function process(LazyConfig $lazyConfig) - { - $vars = $lazyConfig->getVars(); - foreach ($this->variables as $name => $value) { - $vars[$name] = $value; - } - - return $lazyConfig; - } -} diff --git a/Definition/GlobalVariables.php b/Definition/GlobalVariables.php new file mode 100644 index 000000000..994e29fc0 --- /dev/null +++ b/Definition/GlobalVariables.php @@ -0,0 +1,38 @@ +services = $services; + } + + /** + * @param string $name + * + * @return mixed + */ + public function get($name) + { + if (!isset($this->services[$name])) { + throw new \LogicException(sprintf('Global variable %s could not be located. You should define it.', json_encode($name))); + } + + return $this->services[$name]; + } + + /** + * @param string $name + * + * @return bool + */ + public function has($name) + { + return isset($this->services[$name]); + } +} diff --git a/Definition/LazyConfig.php b/Definition/LazyConfig.php index 384a28878..3e02df9e6 100644 --- a/Definition/LazyConfig.php +++ b/Definition/LazyConfig.php @@ -7,23 +7,23 @@ final class LazyConfig /** @var \Closure */ private $loader; - /** @var */ - private $vars; + /** @var \ArrayObject */ + private $globalVariables; /** * @var callable */ private $onPostLoad = []; - private function __construct(\Closure $loader, array $vars = []) + private function __construct(\Closure $loader, array $globalVariables = []) { $this->loader = $loader; - $this->vars = new \ArrayObject($vars); + $this->globalVariables = new \ArrayObject($globalVariables); } - public static function create(\Closure $loader, array $vars = []) + public static function create(\Closure $loader, array $globalVariables = []) { - return new self($loader, $vars); + return new self($loader, $globalVariables); } /** @@ -32,7 +32,7 @@ public static function create(\Closure $loader, array $vars = []) public function load() { $loader = $this->loader; - $config = $loader($this->vars->getArrayCopy()); + $config = $loader(new GlobalVariables($this->globalVariables->getArrayCopy())); foreach ($this->onPostLoad as $postLoader) { $config = $postLoader($config); } @@ -48,8 +48,8 @@ public function addPostLoader(callable $postLoader) /** * @return \ArrayObject */ - public function getVars() + public function getGlobalVariables() { - return $this->vars; + return $this->globalVariables; } } diff --git a/DependencyInjection/Compiler/DefinitionConfigProcessorPass.php b/DependencyInjection/Compiler/DefinitionConfigProcessorPass.php index c22f45756..24a5c5219 100644 --- a/DependencyInjection/Compiler/DefinitionConfigProcessorPass.php +++ b/DependencyInjection/Compiler/DefinitionConfigProcessorPass.php @@ -18,13 +18,15 @@ public function process(ContainerBuilder $container) $taggedServices = $container->findTaggedServiceIds('overblog_graphql.definition_config_processor', true); foreach ($taggedServices as $id => $tags) { - $definition->addMethodCall( - 'addConfigProcessor', - [ - new Reference($id), - isset($tags[0]['priority']) ? $tags[0]['priority'] : 0, - ] - ); + foreach ($tags as $attributes) { + $definition->addMethodCall( + 'addConfigProcessor', + [ + new Reference($id), + isset($attributes['priority']) ? $attributes['priority'] : 0, + ] + ); + } } } } diff --git a/DependencyInjection/Compiler/GlobalVariablesInjectorPass.php b/DependencyInjection/Compiler/GlobalVariablesInjectorPass.php new file mode 100644 index 000000000..75f0ab3f7 --- /dev/null +++ b/DependencyInjection/Compiler/GlobalVariablesInjectorPass.php @@ -0,0 +1,39 @@ +findDefinition(GlobalVariablesInjectorConfigProcessor::class); + $taggedServices = $container->findTaggedServiceIds('overblog_graphql.global_variable', true); + + foreach ($taggedServices as $id => $tags) { + foreach ($tags as $attributes) { + if (!isset($attributes['alias']) || !is_string($attributes['alias'])) { + throw new \InvalidArgumentException( + sprintf('Service "%s" tagged "overblog_graphql.global_variable" should have a valid "alias" attribute.', $id) + ); + } + + $definition->addMethodCall( + 'addGlobalVariable', + [ + $attributes['alias'], + new Reference($id), + isset($attributes['public']) ? (bool) $attributes['public'] : true, + ] + ); + } + } + } +} diff --git a/DependencyInjection/Compiler/VariablesInjectorConfigProcessorPass.php b/DependencyInjection/Compiler/VariablesInjectorConfigProcessorPass.php deleted file mode 100644 index c7dddacd8..000000000 --- a/DependencyInjection/Compiler/VariablesInjectorConfigProcessorPass.php +++ /dev/null @@ -1,27 +0,0 @@ -register(VariablesInjectorConfigProcessor::class, VariablesInjectorConfigProcessor::class) - ->addTag('overblog_graphql.definition_config_processor', ['priority' => 1024]) - ->setArguments([new Reference('overblog_graphql.expression_language')]) - ; - - foreach (['container' => 'service_container', 'token' => 'security.token_storage', 'request' => 'request_stack'] as $name => $id) { - $definition->addMethodCall('addVariable', [$name, new Reference($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)]); - } - } -} diff --git a/ExpressionLanguage/ExpressionFunction/DependencyInjection/Parameter.php b/ExpressionLanguage/ExpressionFunction/DependencyInjection/Parameter.php index 9c82e4a22..8a3bae7b4 100644 --- a/ExpressionLanguage/ExpressionFunction/DependencyInjection/Parameter.php +++ b/ExpressionLanguage/ExpressionFunction/DependencyInjection/Parameter.php @@ -11,7 +11,7 @@ public function __construct($name = 'parameter') parent::__construct( $name, function ($value) { - return sprintf('$vars[\'container\']->getParameter(%s)', $value); + return sprintf('$globalVariable->get(\'container\')->getParameter(%s)', $value); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/DependencyInjection/Service.php b/ExpressionLanguage/ExpressionFunction/DependencyInjection/Service.php index ff1ff6f1b..19aaa9d34 100644 --- a/ExpressionLanguage/ExpressionFunction/DependencyInjection/Service.php +++ b/ExpressionLanguage/ExpressionFunction/DependencyInjection/Service.php @@ -11,7 +11,7 @@ public function __construct($name = 'service') parent::__construct( $name, function ($value) { - return sprintf('$vars[\'container\']->get(%s)', $value); + return sprintf('$globalVariable->get(\'container\')->get(%s)', $value); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/GraphQL/Mutation.php b/ExpressionLanguage/ExpressionFunction/GraphQL/Mutation.php index 4cb7db4b3..d8c02e3fb 100644 --- a/ExpressionLanguage/ExpressionFunction/GraphQL/Mutation.php +++ b/ExpressionLanguage/ExpressionFunction/GraphQL/Mutation.php @@ -11,7 +11,7 @@ public function __construct($name = 'mutation') parent::__construct( $name, function ($alias, $args = '[]') { - return sprintf('$vars[\'container\']->get(\'overblog_graphql.mutation_resolver\')->resolve([%s, %s])', $alias, $args); + return sprintf('$globalVariable->get(\'mutationResolver\')->resolve([%s, %s])', $alias, $args); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/GraphQL/Resolver.php b/ExpressionLanguage/ExpressionFunction/GraphQL/Resolver.php index c42d19e3b..bd3d53c55 100644 --- a/ExpressionLanguage/ExpressionFunction/GraphQL/Resolver.php +++ b/ExpressionLanguage/ExpressionFunction/GraphQL/Resolver.php @@ -11,7 +11,7 @@ public function __construct($name = 'resolver') parent::__construct( $name, function ($alias, $args = '[]') { - return sprintf('$vars[\'container\']->get(\'overblog_graphql.resolver_resolver\')->resolve([%s, %s])', $alias, $args); + return sprintf('$globalVariable->get(\'resolverResolver\')->resolve([%s, %s])', $alias, $args); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/GetUser.php b/ExpressionLanguage/ExpressionFunction/Security/GetUser.php index 68aac9e92..4c791e701 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/GetUser.php +++ b/ExpressionLanguage/ExpressionFunction/Security/GetUser.php @@ -11,7 +11,7 @@ public function __construct() parent::__construct( 'getUser', function () { - return 'null !== $vars[\'token\'] ? $vars[\'token\']->getUser() : null'; + return sprintf('\%s::getUser($globalVariable)', Helper::class); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/HasAnyPermission.php b/ExpressionLanguage/ExpressionFunction/Security/HasAnyPermission.php index 0d3eaf576..66c457130 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/HasAnyPermission.php +++ b/ExpressionLanguage/ExpressionFunction/Security/HasAnyPermission.php @@ -3,6 +3,7 @@ namespace Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\Security; use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction; +use Overblog\GraphQLBundle\Generator\TypeGenerator; final class HasAnyPermission extends ExpressionFunction { @@ -11,7 +12,7 @@ public function __construct($name = 'hasAnyPermission') parent::__construct( $name, function ($object, $permissions) { - $code = sprintf('array_reduce(%s, function ($isGranted, $permission) use ($vars, $object) { return $isGranted || $vars[\'container\']->get(\'security.authorization_checker\')->isGranted($permission, %s); }, false)', $permissions, $object); + $code = sprintf('array_reduce(%s, function ($isGranted, $permission) use (%s, $object) { return $isGranted || $globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted($permission, %s); }, false)', $permissions, TypeGenerator::USE_FOR_CLOSURES, $object); return $code; } diff --git a/ExpressionLanguage/ExpressionFunction/Security/HasAnyRole.php b/ExpressionLanguage/ExpressionFunction/Security/HasAnyRole.php index edc098efe..ec57861d4 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/HasAnyRole.php +++ b/ExpressionLanguage/ExpressionFunction/Security/HasAnyRole.php @@ -3,6 +3,7 @@ namespace Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\Security; use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction; +use Overblog\GraphQLBundle\Generator\TypeGenerator; final class HasAnyRole extends ExpressionFunction { @@ -11,7 +12,7 @@ public function __construct($name = 'hasAnyRole') parent::__construct( $name, function ($roles) { - $code = sprintf('array_reduce(%s, function ($isGranted, $role) use ($vars) { return $isGranted || $vars[\'container\']->get(\'security.authorization_checker\')->isGranted($role); }, false)', $roles); + $code = sprintf('array_reduce(%s, function ($isGranted, $role) use (%s) { return $isGranted || $globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted($role); }, false)', $roles, TypeGenerator::USE_FOR_CLOSURES); return $code; } diff --git a/ExpressionLanguage/ExpressionFunction/Security/HasPermission.php b/ExpressionLanguage/ExpressionFunction/Security/HasPermission.php index a8ff7e12e..b8819e7e4 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/HasPermission.php +++ b/ExpressionLanguage/ExpressionFunction/Security/HasPermission.php @@ -11,7 +11,7 @@ public function __construct($name = 'hasPermission') parent::__construct( $name, function ($object, $permission) { - $code = sprintf('$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(%s, %s)', $permission, $object); + $code = sprintf('$globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted(%s, %s)', $permission, $object); return $code; } diff --git a/ExpressionLanguage/ExpressionFunction/Security/HasRole.php b/ExpressionLanguage/ExpressionFunction/Security/HasRole.php index 0de55e69b..303c032e1 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/HasRole.php +++ b/ExpressionLanguage/ExpressionFunction/Security/HasRole.php @@ -11,7 +11,7 @@ public function __construct($name = 'hasRole') parent::__construct( $name, function ($role) { - return sprintf('$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(%s)', $role); + return sprintf('$globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted(%s)', $role); } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/Helper.php b/ExpressionLanguage/ExpressionFunction/Security/Helper.php new file mode 100644 index 000000000..74853e2fc --- /dev/null +++ b/ExpressionLanguage/ExpressionFunction/Security/Helper.php @@ -0,0 +1,37 @@ +get('container')->has('security.token_storage')) { + return; + } + + return $globalVariable->get('container')->get('security.token_storage')->getToken(); + } + + public static function getUser(GlobalVariables $globalVariable) + { + if (!$token = self::getToken($globalVariable)) { + return; + } + + $user = $token->getUser(); + if (!is_object($user)) { + return; + } + + return $user; + } +} diff --git a/ExpressionLanguage/ExpressionFunction/Security/IsAnonymous.php b/ExpressionLanguage/ExpressionFunction/Security/IsAnonymous.php index 75097b62a..401285e3e 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/IsAnonymous.php +++ b/ExpressionLanguage/ExpressionFunction/Security/IsAnonymous.php @@ -11,7 +11,7 @@ public function __construct($name = 'isAnonymous') parent::__construct( $name, function () { - return '$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_ANONYMOUSLY\')'; + return '$globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_ANONYMOUSLY\')'; } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/IsAuthenticated.php b/ExpressionLanguage/ExpressionFunction/Security/IsAuthenticated.php index a6eb8d5c8..c47384e89 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/IsAuthenticated.php +++ b/ExpressionLanguage/ExpressionFunction/Security/IsAuthenticated.php @@ -11,7 +11,7 @@ public function __construct($name = 'isAuthenticated') parent::__construct( $name, function () { - return '$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_REMEMBERED\') || $vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_FULLY\')'; + return '$globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_REMEMBERED\') || $globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_FULLY\')'; } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/IsFullyAuthenticated.php b/ExpressionLanguage/ExpressionFunction/Security/IsFullyAuthenticated.php index 2bced1f32..d6615c318 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/IsFullyAuthenticated.php +++ b/ExpressionLanguage/ExpressionFunction/Security/IsFullyAuthenticated.php @@ -11,7 +11,7 @@ public function __construct($name = 'isFullyAuthenticated') parent::__construct( $name, function () { - return '$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_FULLY\')'; + return '$globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_FULLY\')'; } ); } diff --git a/ExpressionLanguage/ExpressionFunction/Security/IsRememberMe.php b/ExpressionLanguage/ExpressionFunction/Security/IsRememberMe.php index 924cbe9d8..693b46a18 100644 --- a/ExpressionLanguage/ExpressionFunction/Security/IsRememberMe.php +++ b/ExpressionLanguage/ExpressionFunction/Security/IsRememberMe.php @@ -11,7 +11,7 @@ public function __construct($name = 'isRememberMe') parent::__construct( $name, function () { - return '$vars[\'container\']->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_REMEMBERED\')'; + return '$globalVariable->get(\'container\')->get(\'security.authorization_checker\')->isGranted(\'IS_AUTHENTICATED_REMEMBERED\')'; } ); } diff --git a/Generator/TypeGenerator.php b/Generator/TypeGenerator.php index 0622785c5..1b011bb1a 100644 --- a/Generator/TypeGenerator.php +++ b/Generator/TypeGenerator.php @@ -10,7 +10,7 @@ class TypeGenerator extends BaseTypeGenerator { - const USE_FOR_CLOSURES = '$vars'; + const USE_FOR_CLOSURES = '$globalVariable'; const DEFAULT_CONFIG_PROCESSOR = [Processor::class, 'process']; @@ -76,7 +76,7 @@ protected function generateClosureUseStatements(array $config) protected function resolveTypeCode($alias) { - return sprintf('$vars[\'container\']->get(\'%s\')->resolve(%s)', 'overblog_graphql.type_resolver', var_export($alias, true)); + return sprintf('$globalVariable->get(\'typeResolver\')->resolve(%s)', var_export($alias, true)); } protected function generatePublic(array $value) diff --git a/OverblogGraphQLBundle.php b/OverblogGraphQLBundle.php index c9d9ecb78..1e428e1fa 100644 --- a/OverblogGraphQLBundle.php +++ b/OverblogGraphQLBundle.php @@ -8,10 +8,10 @@ use Overblog\GraphQLBundle\DependencyInjection\Compiler\ConfigTypesPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\DefinitionConfigProcessorPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ExpressionFunctionPass; +use Overblog\GraphQLBundle\DependencyInjection\Compiler\GlobalVariablesInjectorPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\MutationTaggedServiceMappingTaggedPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverTaggedServiceMappingPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\TypeTaggedServiceMappingPass; -use Overblog\GraphQLBundle\DependencyInjection\Compiler\VariablesInjectorConfigProcessorPass; use Overblog\GraphQLBundle\DependencyInjection\OverblogGraphQLExtension; use Overblog\GraphQLBundle\DependencyInjection\OverblogGraphQLTypesExtension; use Symfony\Component\DependencyInjection\Compiler\PassConfig; @@ -29,8 +29,8 @@ public function build(ContainerBuilder $container) parent::build($container); //ConfigTypesPass and AutoMappingPass must be before TypeTaggedServiceMappingPass + $container->addCompilerPass(new GlobalVariablesInjectorPass()); $container->addCompilerPass(new ExpressionFunctionPass()); - $container->addCompilerPass(new VariablesInjectorConfigProcessorPass()); $container->addCompilerPass(new DefinitionConfigProcessorPass()); $container->addCompilerPass(new AutoMappingPass()); $container->addCompilerPass(new ConfigTypesPass(), PassConfig::TYPE_BEFORE_REMOVING); diff --git a/Resources/config/definition_config_processors.yml b/Resources/config/definition_config_processors.yml index db7b0abee..4021ab9b0 100644 --- a/Resources/config/definition_config_processors.yml +++ b/Resources/config/definition_config_processors.yml @@ -5,6 +5,16 @@ services: tags: - { name: overblog_graphql.definition_config_processor, priority: 2048 } + Overblog\GraphQLBundle\Definition\ConfigProcessor\GlobalVariablesInjectorConfigProcessor: + class: Overblog\GraphQLBundle\Definition\ConfigProcessor\GlobalVariablesInjectorConfigProcessor + arguments: + - '@overblog_graphql.expression_language' + calls: + - ['addGlobalVariable', ['container', '@service_container', false]] + public: false + tags: + - { name: overblog_graphql.definition_config_processor, priority: 1024 } + Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor: class: Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor arguments: diff --git a/Resources/config/services.yml b/Resources/config/services.yml index 7a1f88a79..8618a0c67 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -40,6 +40,8 @@ services: public: true arguments: - + tags: + - { name: overblog_graphql.global_variable, alias: typeResolver } Overblog\GraphQLBundle\Resolver\TypeResolver: alias: overblog_graphql.type_resolver @@ -47,6 +49,8 @@ services: overblog_graphql.resolver_resolver: class: Overblog\GraphQLBundle\Resolver\ResolverResolver public: true + tags: + - { name: overblog_graphql.global_variable, alias: resolverResolver, public: false } Overblog\GraphQLBundle\Resolver\ResolverResolver: alias: overblog_graphql.resolver_resolver @@ -54,6 +58,8 @@ services: overblog_graphql.mutation_resolver: class: Overblog\GraphQLBundle\Resolver\MutationResolver public: true + tags: + - { name: overblog_graphql.global_variable, alias: mutationResolver, public: false } Overblog\GraphQLBundle\Resolver\MutationResolver: alias: overblog_graphql.mutation_resolver @@ -83,8 +89,11 @@ services: - "%overblog_graphql_types.config%" - "%overblog_graphql.use_classloader_listener%" calls: - - ["addUseStatement", ["Overblog\\GraphQLBundle\\Definition\\ConfigProcessor"]] + - ['addUseStatement', ['Overblog\GraphQLBundle\Definition\ConfigProcessor']] + - ['addUseStatement', ['Overblog\GraphQLBundle\Definition\LazyConfig']] + - ['addUseStatement', ['Overblog\GraphQLBundle\Definition\GlobalVariables']] - ["addImplement", ["Overblog\\GraphQLBundle\\Definition\\Type\\GeneratedTypeInterface"]] + - ["setExpressionLanguage", ["@overblog_graphql.expression_language"]] Overblog\GraphQLBundle\EventListener\RequestFilesListener: diff --git a/Resources/doc/definitions/expression-language.md b/Resources/doc/definitions/expression-language.md index 31a6a622a..f1c918779 100644 --- a/Resources/doc/definitions/expression-language.md +++ b/Resources/doc/definitions/expression-language.md @@ -30,9 +30,7 @@ User **getUser**() | Returns the user which is currently in the security token s Expression | Description | Scope ---------- | ----------- | -------- -**container** | DI container | global -**request** | Refers to the current request. | Request -**token** | Refers to the token which is currently in the security token storage. Token can be null. | Token +**typeResolver** | the type resolver | global **object** | Refers to the value of the field for which access is being requested. For array `object` will be each item of the array. For Relay connection `object` will be the node of each connection edges. | only available for `config.fields.*.access` with query operation or mutation payload type. **value** | Resolver value | only available in resolve context **args** | Resolver args array | only available in resolve context diff --git a/Resources/skeleton/TypeSystem.php.skeleton b/Resources/skeleton/TypeSystem.php.skeleton index 69c71dda9..1927b7edc 100644 --- a/Resources/skeleton/TypeSystem.php.skeleton +++ b/Resources/skeleton/TypeSystem.php.skeleton @@ -7,10 +7,10 @@ public function __construct(ConfigProcessor $configProcessor) { -$configLoader = function(array $vars) { +$configLoader = function(GlobalVariables $globalVariable) { return ; }; -$config = $configProcessor->process(\Overblog\GraphQLBundle\Definition\LazyConfig::create($configLoader))->load(); +$config = $configProcessor->process(LazyConfig::create($configLoader))->load(); parent::__construct($config); } } diff --git a/Tests/DIContainerMockTrait.php b/Tests/DIContainerMockTrait.php index 29cd334db..89408daa1 100644 --- a/Tests/DIContainerMockTrait.php +++ b/Tests/DIContainerMockTrait.php @@ -21,12 +21,11 @@ protected function getDIContainerMock(array $services = [], array $parameters = ->getMock(); $getMethod = $container->expects($this->any())->method('get'); + $hasMethod = $container->expects($this->any())->method('has'); foreach ($services as $id => $service) { - $getMethod - ->with($id) - ->willReturn($service) - ; + $getMethod->with($id)->willReturn($service); + $hasMethod->with($id)->willReturn(true); } $getParameterMethod = $container->expects($this->any())->method('getParameter'); diff --git a/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ParameterTest.php b/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ParameterTest.php index 70b0512e8..6cc94c8e4 100644 --- a/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ParameterTest.php +++ b/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ParameterTest.php @@ -2,6 +2,7 @@ namespace Overblog\GraphQLBundle\Tests\ExpressionLanguage\ExpressionFunction\DependencyInjection; +use Overblog\GraphQLBundle\Definition\GlobalVariables; use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\DependencyInjection\Parameter; use Overblog\GraphQLBundle\Tests\ExpressionLanguage\TestCase; @@ -18,7 +19,8 @@ protected function getFunctions() */ public function testParameter($name) { - $vars['container'] = $this->getDIContainerMock([], ['test' => 5]); + $globalVariable = new GlobalVariables(['container' => $this->getDIContainerMock([], ['test' => 5])]); + $globalVariable->get('container'); $this->assertSame(5, eval('return '.$this->expressionLanguage->compile($name.'("test")').';')); } diff --git a/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ServiceTest.php b/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ServiceTest.php index 05b752f65..20af961a9 100644 --- a/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ServiceTest.php +++ b/Tests/ExpressionLanguage/ExpressionFunction/DependencyInjection/ServiceTest.php @@ -2,6 +2,7 @@ namespace Overblog\GraphQLBundle\Tests\ExpressionLanguage\ExpressionFunction\DependencyInjection; +use Overblog\GraphQLBundle\Definition\GlobalVariables; use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionFunction\DependencyInjection\Service; use Overblog\GraphQLBundle\Tests\ExpressionLanguage\TestCase; @@ -19,7 +20,8 @@ protected function getFunctions() public function testService($name) { $object = new \stdClass(); - $vars['container'] = $this->getDIContainerMock(['toto' => $object]); + $globalVariable = new GlobalVariables(['container' => $this->getDIContainerMock(['toto' => $object])]); + $globalVariable->get('container'); $this->assertSame($object, eval('return '.$this->expressionLanguage->compile($name.'("toto")').';')); } diff --git a/Tests/ExpressionLanguage/ExpressionFunction/Security/GetUserTest.php b/Tests/ExpressionLanguage/ExpressionFunction/Security/GetUserTest.php new file mode 100644 index 000000000..8dbc40fe7 --- /dev/null +++ b/Tests/ExpressionLanguage/ExpressionFunction/Security/GetUserTest.php @@ -0,0 +1,82 @@ + $this->getDIContainerMock()]); + $globalVariable->has('container'); + $this->assertNull(eval($this->getCompileCode())); + } + + public function testGetUserNoToken() + { + $tokenStorage = $this->getMockBuilder(TokenStorageInterface::class)->getMock(); + $globalVariable = new GlobalVariables(['container' => $this->getDIContainerMock(['security.token_storage' => $tokenStorage])]); + $globalVariable->get('container'); + + $this->getDIContainerMock(['security.token_storage' => $tokenStorage]); + $this->assertNull(eval($this->getCompileCode())); + } + + /** + * @dataProvider getUserProvider + * + * @param $user + * @param $expectedUser + */ + public function testGetUser($user, $expectedUser) + { + $tokenStorage = $this->getMockBuilder(TokenStorageInterface::class)->getMock(); + $token = $this->getMockBuilder(TokenInterface::class)->getMock(); + $globalVariable = new GlobalVariables(['container' => $this->getDIContainerMock(['security.token_storage' => $tokenStorage])]); + $globalVariable->get('container'); + + $token + ->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + $tokenStorage + ->expects($this->once()) + ->method('getToken') + ->will($this->returnValue($token)); + + $this->assertSame($expectedUser, eval($this->getCompileCode())); + } + + public function getUserProvider() + { + $user = $this->getMockBuilder(UserInterface::class)->getMock(); + $std = new \stdClass(); + $token = $this->getMockBuilder(TokenInterface::class)->getMock(); + + return [ + [$user, $user], + [$std, $std], + [$token, $token], + ['Anon.', null], + [null, null], + [10, null], + [true, null], + ]; + } + + private function getCompileCode() + { + return 'return '.$this->expressionLanguage->compile('getUser()').';'; + } +} diff --git a/Tests/ExpressionLanguage/TestCase.php b/Tests/ExpressionLanguage/TestCase.php index f9323848b..24b691e7e 100644 --- a/Tests/ExpressionLanguage/TestCase.php +++ b/Tests/ExpressionLanguage/TestCase.php @@ -2,6 +2,7 @@ namespace Overblog\GraphQLBundle\Tests\ExpressionLanguage; +use Overblog\GraphQLBundle\Definition\GlobalVariables; use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionLanguage; use Overblog\GraphQLBundle\Tests\DIContainerMockTrait; use PHPUnit\Framework\TestCase as BaseTestCase; @@ -30,9 +31,14 @@ abstract protected function getFunctions(); protected function assertExpressionCompile($expression, $with, array $vars = [], $expects = null, $return = true, $assertMethod = 'assertTrue') { - extract($vars); $code = $this->expressionLanguage->compile($expression, array_keys($vars)); - $vars['container'] = $this->getDIContainerMock(['security.authorization_checker' => $this->getAuthorizationCheckerIsGrantedWithExpectation($with, $expects, $return)]); + $globalVariable = new GlobalVariables([ + 'container' => $this->getDIContainerMock( + ['security.authorization_checker' => $this->getAuthorizationCheckerIsGrantedWithExpectation($with, $expects, $return)] + ), + ]); + $globalVariable->get('container'); + extract($vars); $this->$assertMethod(eval('return '.$code.';')); } diff --git a/Tests/Functional/App/config/access/mapping/access.types.yml b/Tests/Functional/App/config/access/mapping/access.types.yml index 0f549ae1b..7836bbf68 100644 --- a/Tests/Functional/App/config/access/mapping/access.types.yml +++ b/Tests/Functional/App/config/access/mapping/access.types.yml @@ -6,9 +6,7 @@ RootQuery: user: type: User resolve: '@=resolver("query")' - current_user: - type: User - resolve: '@=getUser()' + Mutation: type: object config: diff --git a/UPGRADE-0.11.md b/UPGRADE-0.11.md index 99b75ffdd..5599b1a56 100644 --- a/UPGRADE-0.11.md +++ b/UPGRADE-0.11.md @@ -89,10 +89,43 @@ UPGRADE FROM 0.10 to 0.11 ### Expression language * **user** expression variable has been replaced by **getUser** expression function + * **container**, **request** and **token** expression variables has been removed. + `service` or `serv` expression function should be used instead. - Upgrading: - - Replace `user` by `getUser` in you schema configuration + Upgrading your schema configuration: + - Replace `user` by `getUser()`: ```diff - resolve: '@=user' + resolve: '@=getUser()' ``` + + or + + ```diff + - resolve: '@=resolver('foo', [user])' + + resolve: '@=resolver('foo', [getUser()])' + ``` + - Replace `token` by `serv('security.token_storage')` + ```diff + - resolve: '@=token' + + resolve: '@=serv('security.token_storage')' + ``` + + or + + ```diff + - resolve: '@=resolver('foo', [token])' + + resolve: '@=resolver('foo', [serv('security.token_storage')])' + ``` + - Replace `request` by `serv('request_stack')` + ```diff + - resolve: '@=request' + + resolve: '@=serv('request_stack')' + ``` + + or + + ```diff + - resolve: '@=resolver('foo', [request])' + + resolve: '@=resolver('foo', [serv('request_stack')])' + ``` From 268f653f9b1eaea513a13a664ee63cf1fb2bd926 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sun, 14 Jan 2018 11:54:42 +0100 Subject: [PATCH 09/14] Rename common resolver interface This will be less confusing with "Definition/Type/ResolverInterface" --- Command/DebugCommand.php | 6 +++--- Definition/Builder/SchemaBuilder.php | 6 +++--- Resolver/AbstractResolver.php | 2 +- .../{ResolverInterface.php => FluentResolverInterface.php} | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) rename Resolver/{ResolverInterface.php => FluentResolverInterface.php} (90%) diff --git a/Command/DebugCommand.php b/Command/DebugCommand.php index dad6d837b..3c059ff95 100644 --- a/Command/DebugCommand.php +++ b/Command/DebugCommand.php @@ -2,8 +2,8 @@ namespace Overblog\GraphQLBundle\Command; +use Overblog\GraphQLBundle\Resolver\FluentResolverInterface; use Overblog\GraphQLBundle\Resolver\MutationResolver; -use Overblog\GraphQLBundle\Resolver\ResolverInterface; use Overblog\GraphQLBundle\Resolver\ResolverResolver; use Overblog\GraphQLBundle\Resolver\TypeResolver; use Symfony\Component\Console\Command\Command; @@ -72,7 +72,7 @@ protected function execute(InputInterface $input, OutputInterface $output) foreach ($categories as $category) { $io->title(sprintf('GraphQL %ss Services', ucfirst($category))); - /** @var ResolverInterface $resolver */ + /** @var FluentResolverInterface $resolver */ $resolver = $this->{$category.'Resolver'}; $solutions = $this->retrieveSolutions($resolver); @@ -91,7 +91,7 @@ private function renderTable(array $tableHeaders, array $solutions, SymfonyStyle $io->write("\n\n"); } - private function retrieveSolutions(ResolverInterface $resolver) + private function retrieveSolutions(FluentResolverInterface $resolver) { $data = []; foreach ($resolver->getSolutions() as $alias => $solution) { diff --git a/Definition/Builder/SchemaBuilder.php b/Definition/Builder/SchemaBuilder.php index 7aee9d25a..0e819603a 100644 --- a/Definition/Builder/SchemaBuilder.php +++ b/Definition/Builder/SchemaBuilder.php @@ -3,17 +3,17 @@ namespace Overblog\GraphQLBundle\Definition\Builder; use GraphQL\Type\Schema; -use Overblog\GraphQLBundle\Resolver\ResolverInterface; +use Overblog\GraphQLBundle\Resolver\FluentResolverInterface; class SchemaBuilder { - /** @var ResolverInterface */ + /** @var FluentResolverInterface */ private $typeResolver; /** @var bool */ private $enableValidation; - public function __construct(ResolverInterface $typeResolver, $enableValidation = false) + public function __construct(FluentResolverInterface $typeResolver, $enableValidation = false) { $this->typeResolver = $typeResolver; $this->enableValidation = $enableValidation; diff --git a/Resolver/AbstractResolver.php b/Resolver/AbstractResolver.php index f78c594a0..0f02652c2 100644 --- a/Resolver/AbstractResolver.php +++ b/Resolver/AbstractResolver.php @@ -2,7 +2,7 @@ namespace Overblog\GraphQLBundle\Resolver; -abstract class AbstractResolver implements ResolverInterface +abstract class AbstractResolver implements FluentResolverInterface { /** @var array */ private $solutions = []; diff --git a/Resolver/ResolverInterface.php b/Resolver/FluentResolverInterface.php similarity index 90% rename from Resolver/ResolverInterface.php rename to Resolver/FluentResolverInterface.php index 02c542b01..958576ecb 100644 --- a/Resolver/ResolverInterface.php +++ b/Resolver/FluentResolverInterface.php @@ -2,7 +2,7 @@ namespace Overblog\GraphQLBundle\Resolver; -interface ResolverInterface +interface FluentResolverInterface { public function resolve($input); From 6c4cd812de31a2747f6a8c3a8a65ec73c17ef332 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 25 Jan 2018 07:22:56 +0100 Subject: [PATCH 10/14] Introduce SchemaDecorator and improve ResolverMap --- Config/InterfaceTypeDefinition.php | 3 - Config/ObjectTypeDefinition.php | 2 - Config/Parser/GraphQLParser.php | 26 +- Config/TypeDefinition.php | 40 --- Config/UnionTypeDefinition.php | 3 - Definition/Builder/SchemaBuilder.php | 49 ++- ...GlobalVariablesInjectorConfigProcessor.php | 39 --- Definition/GlobalVariables.php | 2 +- Definition/LazyConfig.php | 18 +- Definition/Type/SchemaDecorator.php | 179 ++++++++++ .../Compiler/ConfigTypesPass.php | 3 +- .../Compiler/GlobalVariablesInjectorPass.php | 39 --- .../Compiler/GlobalVariablesPass.php | 44 +++ DependencyInjection/Configuration.php | 4 + .../OverblogGraphQLExtension.php | 11 +- OverblogGraphQLBundle.php | 6 +- Resolver/ResolverMap.php | 96 ++++++ Resolver/ResolverMapInterface.php | 49 +++ Resolver/ResolverMaps.php | 69 ++++ Resolver/TypeResolver.php | 3 - .../config/definition_config_processors.yml | 10 - Resources/config/services.yml | 11 + .../definitions/graphql-schema-language.md | 62 ++-- Resources/doc/definitions/resolver-map.md | 147 ++++++++ Resources/doc/definitions/resolver.md | 25 +- Resources/skeleton/TypeSystem.php.skeleton | 4 +- Tests/Config/Parser/GraphQLParserTest.php | 14 +- .../not-supported-scalar-definition.graphql | 1 - .../Parser/fixtures/graphql/schema.graphql | 2 + .../Config/Parser/fixtures/graphql/schema.php | 20 +- Tests/Definition/GlobalVariablesTest.php | 16 + Tests/Definition/Type/SchemaDecoratorTest.php | 321 ++++++++++++++++++ .../Compiler/GlobalVariablesPassTest.php | 48 +++ Tests/Functional/App/Resolver/Characters.php | 128 +++++++ .../SchemaLanguageMutationResolverMap.php | 17 + .../SchemaLanguageQueryResolverMap.php | 81 +++++ Tests/Functional/App/config/global/config.yml | 2 +- .../global/mapping/decorators.types.yml | 30 -- .../global/mapping/global.types.graphql | 25 -- .../config/global/mapping/global.types.yml | 50 ++- .../App/config/schemaLanguage/config.yml | 26 ++ .../schemaLanguage/mapping/mutation.graphql | 3 + .../schemaLanguage/mapping/query.graphqls | 33 ++ .../SchemaLanguage/SchemaLanguageTest.php | 177 ++++++++++ Tests/Functional/TestCase.php | 8 +- Tests/Resolver/ResolverMapTest.php | 133 ++++++++ Tests/Resolver/ResolverMapsTest.php | 47 +++ Tests/Resolver/ResolverTest.php | 12 + Tests/Resolver/TypeResolverTest.php | 12 + 49 files changed, 1844 insertions(+), 306 deletions(-) delete mode 100644 Definition/ConfigProcessor/GlobalVariablesInjectorConfigProcessor.php create mode 100644 Definition/Type/SchemaDecorator.php delete mode 100644 DependencyInjection/Compiler/GlobalVariablesInjectorPass.php create mode 100644 DependencyInjection/Compiler/GlobalVariablesPass.php create mode 100644 Resolver/ResolverMap.php create mode 100644 Resolver/ResolverMapInterface.php create mode 100644 Resolver/ResolverMaps.php create mode 100644 Resources/doc/definitions/resolver-map.md delete mode 100644 Tests/Config/Parser/fixtures/graphql/not-supported-scalar-definition.graphql create mode 100644 Tests/Definition/GlobalVariablesTest.php create mode 100644 Tests/Definition/Type/SchemaDecoratorTest.php create mode 100644 Tests/DependencyInjection/Compiler/GlobalVariablesPassTest.php create mode 100644 Tests/Functional/App/Resolver/Characters.php create mode 100644 Tests/Functional/App/Resolver/SchemaLanguageMutationResolverMap.php create mode 100644 Tests/Functional/App/Resolver/SchemaLanguageQueryResolverMap.php delete mode 100644 Tests/Functional/App/config/global/mapping/decorators.types.yml delete mode 100644 Tests/Functional/App/config/global/mapping/global.types.graphql create mode 100644 Tests/Functional/App/config/schemaLanguage/config.yml create mode 100644 Tests/Functional/App/config/schemaLanguage/mapping/mutation.graphql create mode 100644 Tests/Functional/App/config/schemaLanguage/mapping/query.graphqls create mode 100644 Tests/Functional/SchemaLanguage/SchemaLanguageTest.php create mode 100644 Tests/Resolver/ResolverMapTest.php create mode 100644 Tests/Resolver/ResolverMapsTest.php diff --git a/Config/InterfaceTypeDefinition.php b/Config/InterfaceTypeDefinition.php index 5a3c14889..ba95dc90c 100644 --- a/Config/InterfaceTypeDefinition.php +++ b/Config/InterfaceTypeDefinition.php @@ -16,12 +16,9 @@ public function getDefinition() ->append($this->nameSection()) ->append($this->outputFieldsSelection()) ->append($this->resolveTypeSection()) - ->append($this->resolverMapSection()) ->append($this->descriptionSection()) ->end(); - $this->validateResolverMap($node); - return $node; } } diff --git a/Config/ObjectTypeDefinition.php b/Config/ObjectTypeDefinition.php index c8427d198..19391685b 100644 --- a/Config/ObjectTypeDefinition.php +++ b/Config/ObjectTypeDefinition.php @@ -22,7 +22,6 @@ public function getDefinition() ->end() ->variableNode('isTypeOf')->end() ->variableNode('resolveField')->end() - ->append($this->resolverMapSection()) ->variableNode('fieldsDefaultAccess') ->info('Default access control to fields (expression language can be use here)') ->end() @@ -33,7 +32,6 @@ public function getDefinition() $this->treatFieldsDefaultAccess($node); $this->treatFieldsDefaultPublic($node); - $this->validateResolverMap($node); return $node; } diff --git a/Config/Parser/GraphQLParser.php b/Config/Parser/GraphQLParser.php index 1e92e4267..324bbee0c 100644 --- a/Config/Parser/GraphQLParser.php +++ b/Config/Parser/GraphQLParser.php @@ -24,6 +24,7 @@ class GraphQLParser implements ParserInterface NodeKind::ENUM_TYPE_DEFINITION => 'enum', NodeKind::UNION_TYPE_DEFINITION => 'union', NodeKind::INPUT_OBJECT_TYPE_DEFINITION => 'input-object', + NodeKind::SCALAR_TYPE_DEFINITION => 'custom-scalar', ]; /** @@ -57,6 +58,11 @@ public static function parse(\SplFileInfo $file, ContainerBuilder $container) return $typesConfig; } + public static function mustOverrideConfig() + { + throw new \RuntimeException('Config entry must be override with ResolverMap to be used.'); + } + protected function typeDefinitionToConfig(Node $typeDef) { switch ($typeDef->kind) { @@ -77,6 +83,21 @@ protected function typeDefinitionToConfig(Node $typeDef) 'config' => $config, ]; + case NodeKind::SCALAR_TYPE_DEFINITION: + $mustOverride = [__CLASS__, 'mustOverrideConfig']; + $config = [ + 'serialize' => $mustOverride, + 'parseValue' => $mustOverride, + 'parseLiteral' => $mustOverride, + ]; + $this->addDescription($typeDef, $config); + + return [ + 'type' => self::DEFINITION_TYPE_MAPPING[$typeDef->kind], + 'config' => $config, + ]; + break; + default: throw new InvalidArgumentException( sprintf( @@ -124,7 +145,7 @@ private function addFieldArguments(Node $fieldDef, array &$fieldConf) $this->addDescription($definition, $arguments[$name]); $this->addDefaultValue($definition, $arguments[$name]); } - $fieldConf['arguments'] = $arguments; + $fieldConf['args'] = $arguments; } } @@ -167,7 +188,8 @@ private function addValues(Node $typeDef, array &$config) if (!empty($typeDef->values)) { $values = []; foreach ($typeDef->values as $value) { - $values[] = ['value' => $value->name->value]; + $values[$value->name->value] = ['value' => $value->name->value]; + $this->addDescription($value, $values[$value->name->value]); } $config['values'] = $values; } diff --git a/Config/TypeDefinition.php b/Config/TypeDefinition.php index 18f235978..4a8a5e097 100644 --- a/Config/TypeDefinition.php +++ b/Config/TypeDefinition.php @@ -2,7 +2,6 @@ namespace Overblog\GraphQLBundle\Config; -use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; abstract class TypeDefinition @@ -29,45 +28,6 @@ protected function resolveTypeSection() return $node; } - protected function resolverMapSection() - { - $builder = new TreeBuilder(); - $node = $builder->root('resolverMap', 'scalar'); - - return $node; - } - - protected function validateResolverMap(NodeDefinition $node) - { - $node->validate() - ->ifTrue(function ($config) { - if (isset($config['resolverMap'])) { - foreach ($config['fields'] as $field) { - if (isset($field['resolve'])) { - return true; - } - } - - return isset($config['resolveField']); - } - - return false; - }) - ->then(function () { - throw new \InvalidArgumentException('ResolverMap should not be combine with resolveField or fields.*.resolve.'); - }); - - $node->validate() - ->ifTrue(function ($config) { - return isset($config['resolverMap']) && isset($config['resolveType']); - }) - ->then(function () { - throw new \InvalidArgumentException('ResolverMap should not be combine with resolveType.'); - }); - - return $node; - } - protected function nameSection() { $builder = new TreeBuilder(); diff --git a/Config/UnionTypeDefinition.php b/Config/UnionTypeDefinition.php index 47a965168..a5ddf1410 100644 --- a/Config/UnionTypeDefinition.php +++ b/Config/UnionTypeDefinition.php @@ -22,12 +22,9 @@ public function getDefinition() ->requiresAtLeastOneElement() ->end() ->append($this->resolveTypeSection()) - ->append($this->resolverMapSection()) ->append($this->descriptionSection()) ->end(); - $this->validateResolverMap($node); - return $node; } } diff --git a/Definition/Builder/SchemaBuilder.php b/Definition/Builder/SchemaBuilder.php index 0e819603a..e81377aba 100644 --- a/Definition/Builder/SchemaBuilder.php +++ b/Definition/Builder/SchemaBuilder.php @@ -2,47 +2,68 @@ namespace Overblog\GraphQLBundle\Definition\Builder; +use GraphQL\Type\Definition\Type; use GraphQL\Type\Schema; -use Overblog\GraphQLBundle\Resolver\FluentResolverInterface; +use Overblog\GraphQLBundle\Definition\Type\SchemaDecorator; +use Overblog\GraphQLBundle\Resolver\ResolverMapInterface; +use Overblog\GraphQLBundle\Resolver\ResolverMaps; +use Overblog\GraphQLBundle\Resolver\TypeResolver; class SchemaBuilder { - /** @var FluentResolverInterface */ + /** @var TypeResolver */ private $typeResolver; + /** @var SchemaDecorator */ + private $decorator; + /** @var bool */ private $enableValidation; - public function __construct(FluentResolverInterface $typeResolver, $enableValidation = false) + public function __construct(TypeResolver $typeResolver, SchemaDecorator $decorator, $enableValidation = false) { $this->typeResolver = $typeResolver; + $this->decorator = $decorator; $this->enableValidation = $enableValidation; } /** - * @param null|string $queryAlias - * @param null|string $mutationAlias - * @param null|string $subscriptionAlias + * @param null|string $queryAlias + * @param null|string $mutationAlias + * @param null|string $subscriptionAlias + * @param ResolverMapInterface[] $resolverMaps * * @return Schema */ - public function create($queryAlias = null, $mutationAlias = null, $subscriptionAlias = null) + public function create($queryAlias = null, $mutationAlias = null, $subscriptionAlias = null, array $resolverMaps = []) { $query = $this->typeResolver->resolve($queryAlias); $mutation = $this->typeResolver->resolve($mutationAlias); $subscription = $this->typeResolver->resolve($subscriptionAlias); - $schema = new Schema([ - 'query' => $query, - 'mutation' => $mutation, - 'subscription' => $subscription, - 'typeLoader' => [$this->typeResolver, 'resolve'], - 'types' => [$this->typeResolver, 'getSolutions'], - ]); + $schema = new Schema($this->buildSchemaArguments($query, $mutation, $subscription)); + reset($resolverMaps); + $this->decorator->decorate($schema, 1 === count($resolverMaps) ? current($resolverMaps) : new ResolverMaps($resolverMaps)); + if ($this->enableValidation) { $schema->assertValid(); } return $schema; } + + private function buildSchemaArguments(Type $query = null, Type $mutation = null, Type $subscription = null) + { + return [ + 'query' => $query, + 'mutation' => $mutation, + 'subscription' => $subscription, + 'typeLoader' => function ($name) { + return $this->typeResolver->resolve($name); + }, + 'types' => function () { + return $this->typeResolver->getSolutions(); + }, + ]; + } } diff --git a/Definition/ConfigProcessor/GlobalVariablesInjectorConfigProcessor.php b/Definition/ConfigProcessor/GlobalVariablesInjectorConfigProcessor.php deleted file mode 100644 index fb09db8dd..000000000 --- a/Definition/ConfigProcessor/GlobalVariablesInjectorConfigProcessor.php +++ /dev/null @@ -1,39 +0,0 @@ -expressionLanguage = $expressionLanguage; - } - - public function addGlobalVariable($name, $globalVariable, $isPublic = true) - { - $this->globalVariables[$name] = $globalVariable; - if ($isPublic) { - $this->expressionLanguage->addGlobalName(sprintf('globalVariables->get(\'%s\')', $name), $name); - } - } - - /** - * {@inheritdoc} - */ - public function process(LazyConfig $lazyConfig) - { - $globalVariables = $lazyConfig->getGlobalVariables(); - foreach ($this->globalVariables as $name => $variable) { - $globalVariables[$name] = $variable; - } - - return $lazyConfig; - } -} diff --git a/Definition/GlobalVariables.php b/Definition/GlobalVariables.php index 994e29fc0..5efe5950d 100644 --- a/Definition/GlobalVariables.php +++ b/Definition/GlobalVariables.php @@ -7,7 +7,7 @@ final class GlobalVariables /** @var array */ private $services; - public function __construct(array $services) + public function __construct(array $services = []) { $this->services = $services; } diff --git a/Definition/LazyConfig.php b/Definition/LazyConfig.php index 3e02df9e6..24e84c33e 100644 --- a/Definition/LazyConfig.php +++ b/Definition/LazyConfig.php @@ -7,7 +7,7 @@ final class LazyConfig /** @var \Closure */ private $loader; - /** @var \ArrayObject */ + /** @var GlobalVariables */ private $globalVariables; /** @@ -15,13 +15,13 @@ final class LazyConfig */ private $onPostLoad = []; - private function __construct(\Closure $loader, array $globalVariables = []) + private function __construct(\Closure $loader, GlobalVariables $globalVariables = null) { $this->loader = $loader; - $this->globalVariables = new \ArrayObject($globalVariables); + $this->globalVariables = $globalVariables ?: new GlobalVariables(); } - public static function create(\Closure $loader, array $globalVariables = []) + public static function create(\Closure $loader, GlobalVariables $globalVariables = null) { return new self($loader, $globalVariables); } @@ -32,7 +32,7 @@ public static function create(\Closure $loader, array $globalVariables = []) public function load() { $loader = $this->loader; - $config = $loader(new GlobalVariables($this->globalVariables->getArrayCopy())); + $config = $loader($this->globalVariables); foreach ($this->onPostLoad as $postLoader) { $config = $postLoader($config); } @@ -44,12 +44,4 @@ public function addPostLoader(callable $postLoader) { $this->onPostLoad[] = $postLoader; } - - /** - * @return \ArrayObject - */ - public function getGlobalVariables() - { - return $this->globalVariables; - } } diff --git a/Definition/Type/SchemaDecorator.php b/Definition/Type/SchemaDecorator.php new file mode 100644 index 000000000..66233d389 --- /dev/null +++ b/Definition/Type/SchemaDecorator.php @@ -0,0 +1,179 @@ +covered() as $typeName) { + $type = $schema->getType($typeName); + + if ($type instanceof ObjectType) { + $this->decorateObjectType($type, $resolverMap); + } elseif ($type instanceof InterfaceType || $type instanceof UnionType) { + $this->decorateInterfaceOrUnionType($type, $resolverMap); + } elseif ($type instanceof EnumType) { + $this->decorateEnumType($type, $resolverMap); + } elseif ($type instanceof CustomScalarType) { + $this->decorateCustomScalarType($type, $resolverMap); + } else { + $covered = $resolverMap->covered($type->name); + if (!empty($covered)) { + throw new \InvalidArgumentException( + sprintf( + '"%s".{"%s"} defined in resolverMap, but type is not managed by SchemaDecorator.', + $type->name, + implode('", "', $covered) + ) + ); + } + } + } + } + + private function decorateObjectType(ObjectType $type, ResolverMapInterface $resolverMap) + { + $fieldsResolved = []; + foreach ($resolverMap->covered($type->name) as $fieldName) { + if (ResolverMapInterface::IS_TYPEOF === $fieldName) { + $this->configTypeMapping($type, $resolverMap, ResolverMapInterface::IS_TYPEOF); + } elseif (ResolverMapInterface::RESOLVE_FIELD === $fieldName) { + $resolveFieldFn = self::wrapResolver($resolverMap->resolve($type->name, ResolverMapInterface::RESOLVE_FIELD)); + $type->config[substr(ResolverMapInterface::RESOLVE_FIELD, 2)] = $resolveFieldFn; + $type->resolveFieldFn = $resolveFieldFn; + } else { + $fieldsResolved[] = $fieldName; + } + } + $this->decorateObjectTypeFields($type, $resolverMap, $fieldsResolved); + } + + /** + * @param InterfaceType|UnionType $type + * @param ResolverMapInterface $resolverMap + */ + private function decorateInterfaceOrUnionType($type, ResolverMapInterface $resolverMap) + { + $this->configTypeMapping($type, $resolverMap, ResolverMapInterface::RESOLVE_TYPE); + $covered = $resolverMap->covered($type->name); + if (!empty($covered)) { + $unknownFields = array_diff($covered, [ResolverMapInterface::RESOLVE_TYPE]); + if (!empty($unknownFields)) { + throw new \InvalidArgumentException( + sprintf( + '"%s".{"%s"} defined in resolverMap, but only "%s" is allowed.', + $type->name, + implode('", "', $unknownFields), + ResolverMapInterface::RESOLVE_TYPE + ) + ); + } + } + } + + private function decorateCustomScalarType(CustomScalarType $type, ResolverMapInterface $resolverMap) + { + static $allowedFields = [ResolverMapInterface::SERIALIZE, ResolverMapInterface::PARSE_VALUE, ResolverMapInterface::PARSE_LITERAL]; + + foreach ($allowedFields as $fieldName) { + $this->configTypeMapping($type, $resolverMap, $fieldName); + } + + $unknownFields = array_diff($resolverMap->covered($type->name), $allowedFields); + if (!empty($unknownFields)) { + throw new \InvalidArgumentException( + sprintf( + '"%s".{"%s"} defined in resolverMap, but only "%s" is allowed.', + $type->name, + implode('", "', $unknownFields), + implode('", "', $allowedFields) + ) + ); + } + } + + private function decorateEnumType(EnumType $type, ResolverMapInterface $resolverMaps) + { + $fieldNames = []; + foreach ($type->config['values'] as $key => &$value) { + $fieldName = isset($value['name']) ? $value['name'] : $key; + if ($resolverMaps->isResolvable($type->name, $fieldName)) { + $value['value'] = $resolverMaps->resolve($type->name, $fieldName); + } + $fieldNames[] = $fieldName; + } + $unknownFields = array_diff($resolverMaps->covered($type->name), $fieldNames); + if (!empty($unknownFields)) { + throw new \InvalidArgumentException( + sprintf( + '"%s".{"%s"} defined in resolverMap, was defined in resolvers, but enum is not in schema.', + $type->name, + implode('", "', $unknownFields) + ) + ); + } + } + + private function decorateObjectTypeFields(ObjectType $type, ResolverMapInterface $resolverMap, array $fieldsResolved) + { + $fields = $type->config['fields']; + + $decoratedFields = function () use ($fields, $type, $resolverMap, $fieldsResolved) { + if (is_callable($fields)) { + $fields = $fields(); + } + + $fieldNames = []; + foreach ($fields as $key => &$field) { + $fieldName = isset($field['name']) ? $field['name'] : $key; + + if ($resolverMap->isResolvable($type->name, $fieldName)) { + $field['resolve'] = self::wrapResolver($resolverMap->resolve($type->name, $fieldName)); + } + + $fieldNames[] = $fieldName; + } + + $unknownFields = array_diff($fieldsResolved, $fieldNames); + if (!empty($unknownFields)) { + throw new \InvalidArgumentException( + sprintf('"%s".{"%s"} defined in resolverMap, but not in schema.', $type->name, implode('", "', $unknownFields)) + ); + } + + return $fields; + }; + + $type->config['fields'] = is_callable($fields) ? $decoratedFields : $decoratedFields(); + } + + private function configTypeMapping(Type $type, ResolverMapInterface $resolverMap, $fieldName) + { + if ($resolverMap->isResolvable($type->name, $fieldName)) { + $type->config[substr($fieldName, 2)] = $resolverMap->resolve($type->name, $fieldName); + } + } + + private static function wrapResolver(callable $resolver) + { + return static function () use ($resolver) { + $args = func_get_args(); + if (count($args) > 1) { + $args[1] = new Argument($args[1]); + } + + return call_user_func_array($resolver, $args); + }; + } +} diff --git a/DependencyInjection/Compiler/ConfigTypesPass.php b/DependencyInjection/Compiler/ConfigTypesPass.php index 61541364a..6f0b620ed 100644 --- a/DependencyInjection/Compiler/ConfigTypesPass.php +++ b/DependencyInjection/Compiler/ConfigTypesPass.php @@ -3,6 +3,7 @@ namespace Overblog\GraphQLBundle\DependencyInjection\Compiler; use Overblog\GraphQLBundle\Definition\ConfigProcessor; +use Overblog\GraphQLBundle\Definition\GlobalVariables; use Overblog\GraphQLBundle\Generator\TypeGenerator; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -26,7 +27,7 @@ private function setTypeServiceDefinition(ContainerBuilder $container, $class, a { $definition = $container->setDefinition($class, new Definition($class)); $definition->setPublic(false); - $definition->setArguments([new Reference(ConfigProcessor::class)]); + $definition->setArguments([new Reference(ConfigProcessor::class), new Reference(GlobalVariables::class)]); foreach ($aliases as $alias) { $definition->addTag(TypeTaggedServiceMappingPass::TAG_NAME, ['alias' => $alias, 'generated' => true]); } diff --git a/DependencyInjection/Compiler/GlobalVariablesInjectorPass.php b/DependencyInjection/Compiler/GlobalVariablesInjectorPass.php deleted file mode 100644 index 75f0ab3f7..000000000 --- a/DependencyInjection/Compiler/GlobalVariablesInjectorPass.php +++ /dev/null @@ -1,39 +0,0 @@ -findDefinition(GlobalVariablesInjectorConfigProcessor::class); - $taggedServices = $container->findTaggedServiceIds('overblog_graphql.global_variable', true); - - foreach ($taggedServices as $id => $tags) { - foreach ($tags as $attributes) { - if (!isset($attributes['alias']) || !is_string($attributes['alias'])) { - throw new \InvalidArgumentException( - sprintf('Service "%s" tagged "overblog_graphql.global_variable" should have a valid "alias" attribute.', $id) - ); - } - - $definition->addMethodCall( - 'addGlobalVariable', - [ - $attributes['alias'], - new Reference($id), - isset($attributes['public']) ? (bool) $attributes['public'] : true, - ] - ); - } - } - } -} diff --git a/DependencyInjection/Compiler/GlobalVariablesPass.php b/DependencyInjection/Compiler/GlobalVariablesPass.php new file mode 100644 index 000000000..a0969d974 --- /dev/null +++ b/DependencyInjection/Compiler/GlobalVariablesPass.php @@ -0,0 +1,44 @@ +findTaggedServiceIds('overblog_graphql.global_variable', true); + $globalVariables = ['container' => new Reference('service_container')]; + $expressionLanguageDefinition = $container->findDefinition('overblog_graphql.expression_language'); + + foreach ($taggedServices as $id => $tags) { + foreach ($tags as $attributes) { + if (empty($attributes['alias']) || !is_string($attributes['alias'])) { + throw new \InvalidArgumentException( + sprintf('Service "%s" tagged "overblog_graphql.global_variable" should have a valid "alias" attribute.', $id) + ); + } + $globalVariables[$attributes['alias']] = new Reference($id); + + $isPublic = isset($attributes['public']) ? (bool) $attributes['public'] : true; + if ($isPublic) { + $expressionLanguageDefinition->addMethodCall( + 'addGlobalName', + [ + sprintf('globalVariables->get(\'%s\')', $attributes['alias']), + $attributes['alias'], + ] + ); + } + } + } + $container->findDefinition(GlobalVariables::class)->setArguments([$globalVariables]); + } +} diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 3c1bd2f42..847281600 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -196,6 +196,10 @@ private function definitionsSchemaSection() ->scalarNode('query')->defaultNull()->end() ->scalarNode('mutation')->defaultNull()->end() ->scalarNode('subscription')->defaultNull()->end() + ->arrayNode('resolver_maps') + ->defaultValue([]) + ->prototype('scalar')->end() + ->end() ->end() ->end() ->end(); diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index 32cc4eea5..822614f28 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -216,7 +216,7 @@ private function setErrorHandler(array $config, ContainerBuilder $container) private function setSchemaBuilderArguments(array $config, ContainerBuilder $container) { $container->getDefinition($this->getAlias().'.schema_builder') - ->replaceArgument(1, $config['definitions']['config_validation']); + ->replaceArgument(2, $config['definitions']['config_validation']); } private function setSchemaArguments(array $config, ContainerBuilder $container) @@ -228,7 +228,14 @@ private function setSchemaArguments(array $config, ContainerBuilder $container) $schemaID = sprintf('%s.schema_%s', $this->getAlias(), $schemaName); $definition = new Definition(Schema::class); $definition->setFactory([new Reference('overblog_graphql.schema_builder'), 'create']); - $definition->setArguments([$schemaConfig['query'], $schemaConfig['mutation'], $schemaConfig['subscription']]); + $definition->setArguments([ + $schemaConfig['query'], + $schemaConfig['mutation'], + $schemaConfig['subscription'], + array_map(function ($id) { + return new Reference($id); + }, $schemaConfig['resolver_maps']), + ]); $definition->setPublic(false); $container->setDefinition($schemaID, $definition); diff --git a/OverblogGraphQLBundle.php b/OverblogGraphQLBundle.php index 1e428e1fa..964d897c8 100644 --- a/OverblogGraphQLBundle.php +++ b/OverblogGraphQLBundle.php @@ -8,7 +8,7 @@ use Overblog\GraphQLBundle\DependencyInjection\Compiler\ConfigTypesPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\DefinitionConfigProcessorPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ExpressionFunctionPass; -use Overblog\GraphQLBundle\DependencyInjection\Compiler\GlobalVariablesInjectorPass; +use Overblog\GraphQLBundle\DependencyInjection\Compiler\GlobalVariablesPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\MutationTaggedServiceMappingTaggedPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\ResolverTaggedServiceMappingPass; use Overblog\GraphQLBundle\DependencyInjection\Compiler\TypeTaggedServiceMappingPass; @@ -29,14 +29,14 @@ public function build(ContainerBuilder $container) parent::build($container); //ConfigTypesPass and AutoMappingPass must be before TypeTaggedServiceMappingPass - $container->addCompilerPass(new GlobalVariablesInjectorPass()); + $container->addCompilerPass(new GlobalVariablesPass()); $container->addCompilerPass(new ExpressionFunctionPass()); $container->addCompilerPass(new DefinitionConfigProcessorPass()); $container->addCompilerPass(new AutoMappingPass()); - $container->addCompilerPass(new ConfigTypesPass(), PassConfig::TYPE_BEFORE_REMOVING); $container->addCompilerPass(new AliasedPass()); $container->addCompilerPass(new AutowiringTypesPass()); + $container->addCompilerPass(new ConfigTypesPass(), PassConfig::TYPE_BEFORE_REMOVING); $container->addCompilerPass(new TypeTaggedServiceMappingPass(), PassConfig::TYPE_BEFORE_REMOVING); $container->addCompilerPass(new ResolverTaggedServiceMappingPass(), PassConfig::TYPE_BEFORE_REMOVING); $container->addCompilerPass(new MutationTaggedServiceMappingTaggedPass(), PassConfig::TYPE_BEFORE_REMOVING); diff --git a/Resolver/ResolverMap.php b/Resolver/ResolverMap.php new file mode 100644 index 000000000..af1d67938 --- /dev/null +++ b/Resolver/ResolverMap.php @@ -0,0 +1,96 @@ +isMapLoaded) { + $this->checkMap($map = $this->map()); + $this->loadedMap = $map; + $this->isMapLoaded = true; + } + + return $this->loadedMap; + } + + /** + * {@inheritdoc} + */ + public function resolve($typeName, $fieldName) + { + $loadedMap = $this->getLoadedMap(); + + if (!$this->isResolvable($typeName, $fieldName)) { + throw new UnresolvableException(sprintf('Field "%s.%s" could not be resolved.', $typeName, $fieldName)); + } + + return $loadedMap[$typeName][$fieldName]; + } + + /** + * {@inheritdoc} + */ + public function isResolvable($typeName, $fieldName) + { + $key = $typeName.'.'.$fieldName; + if (!isset($this->memorized[$key])) { + $loadedMap = $this->getLoadedMap(); + $this->memorized[$key] = isset($loadedMap[$typeName]) && array_key_exists($fieldName, $loadedMap[$typeName]); + } + + return $this->memorized[$key]; + } + + /** + * {@inheritdoc} + */ + public function covered($typeName = null) + { + $loadedMap = $this->getLoadedMap(); + $covered = []; + $resolvers = []; + if (null === $typeName) { + $resolvers = $loadedMap; + } elseif (isset($loadedMap[$typeName])) { + $resolvers = $loadedMap[$typeName]; + } + + foreach ($resolvers as $key => $value) { + $covered[] = $key; + } + + return $covered; + } + + private function checkMap($map) + { + if (!is_array($map) && !($map instanceof \ArrayAccess && $map instanceof \Traversable)) { + throw new \RuntimeException(sprintf( + '%s::map() should return an array or an instance of \ArrayAccess and \Traversable but got "%s".', + get_class($this), + is_object($map) ? get_class($map) : gettype($map) + )); + } + } +} diff --git a/Resolver/ResolverMapInterface.php b/Resolver/ResolverMapInterface.php new file mode 100644 index 000000000..303fc967d --- /dev/null +++ b/Resolver/ResolverMapInterface.php @@ -0,0 +1,49 @@ +resolverMaps = $resolverMaps; + } + + /** + * {@inheritdoc} + */ + public function resolve($typeName, $fieldName) + { + foreach ($this->resolverMaps as $resolverMap) { + if ($resolverMap->isResolvable($typeName, $fieldName)) { + return $resolverMap->resolve($typeName, $fieldName); + } + } + throw new UnresolvableException(sprintf('Field "%s.%s" could not be resolved.', $typeName, $fieldName)); + } + + /** + * {@inheritdoc} + */ + public function isResolvable($typeName, $fieldName) + { + foreach ($this->resolverMaps as $resolverMap) { + if ($resolverMap->isResolvable($typeName, $fieldName)) { + return true; + } + } + + return false; + } + + /** + * {@inheritdoc} + */ + public function covered($typeName = null) + { + $covered = []; + foreach ($this->resolverMaps as $resolverMap) { + $covered = array_merge($covered, $resolverMap->covered($typeName)); + } + $covered = array_unique($covered); + + return $covered; + } + + private static function checkResolverMaps(array $resolverMaps) + { + foreach ($resolverMaps as $resolverMap) { + if (!$resolverMap instanceof ResolverMapInterface) { + throw new \InvalidArgumentException(sprintf( + 'ResolverMap should be instance of "%s" but got "%s".', + ResolverMapInterface::class, + is_object($resolverMap) ? get_class($resolverMap) : gettype($resolverMap) + )); + } + } + } +} diff --git a/Resolver/TypeResolver.php b/Resolver/TypeResolver.php index a53aba891..649808044 100644 --- a/Resolver/TypeResolver.php +++ b/Resolver/TypeResolver.php @@ -37,9 +37,6 @@ public function resolve($alias) return $item->get(); } - /** - * @param string $alias - */ private function string2Type($alias) { if (false !== ($type = $this->wrapTypeIfNeeded($alias))) { diff --git a/Resources/config/definition_config_processors.yml b/Resources/config/definition_config_processors.yml index 4021ab9b0..db7b0abee 100644 --- a/Resources/config/definition_config_processors.yml +++ b/Resources/config/definition_config_processors.yml @@ -5,16 +5,6 @@ services: tags: - { name: overblog_graphql.definition_config_processor, priority: 2048 } - Overblog\GraphQLBundle\Definition\ConfigProcessor\GlobalVariablesInjectorConfigProcessor: - class: Overblog\GraphQLBundle\Definition\ConfigProcessor\GlobalVariablesInjectorConfigProcessor - arguments: - - '@overblog_graphql.expression_language' - calls: - - ['addGlobalVariable', ['container', '@service_container', false]] - public: false - tags: - - { name: overblog_graphql.definition_config_processor, priority: 1024 } - Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor: class: Overblog\GraphQLBundle\Definition\ConfigProcessor\AclConfigProcessor arguments: diff --git a/Resources/config/services.yml b/Resources/config/services.yml index 8618a0c67..a17535dac 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -33,8 +33,13 @@ services: public: false arguments: - "@overblog_graphql.type_resolver" + - '@Overblog\GraphQLBundle\Definition\Type\SchemaDecorator' - false + Overblog\GraphQLBundle\Definition\Type\SchemaDecorator: + class: Overblog\GraphQLBundle\Definition\Type\SchemaDecorator + public: false + overblog_graphql.type_resolver: class: Overblog\GraphQLBundle\Resolver\TypeResolver public: true @@ -155,3 +160,9 @@ services: Overblog\GraphQLBundle\Definition\ConfigProcessor: class: Overblog\GraphQLBundle\Definition\ConfigProcessor public: false + + Overblog\GraphQLBundle\Definition\GlobalVariables: + class: Overblog\GraphQLBundle\Definition\GlobalVariables + public: false + arguments: + - [] diff --git a/Resources/doc/definitions/graphql-schema-language.md b/Resources/doc/definitions/graphql-schema-language.md index aacb76bf4..6d36c48ca 100644 --- a/Resources/doc/definitions/graphql-schema-language.md +++ b/Resources/doc/definitions/graphql-schema-language.md @@ -6,65 +6,41 @@ If you want to learn more about it, you can see the [official documentation](http://graphql.org/learn/schema/) or this [cheat sheet](https://github.com/sogko/graphql-shorthand-notation-cheat-sheet). -Here is an example: +#### Usage ```graphql # config/graphql/schema.types.graphql type Query { - bar: Bar! + bar: Foo! + baz(id: ID!): Baz } +scalar Baz + interface Foo { # Description of my is_foo field is_foo: Boolean } type Bar implements Foo { is_foo: Boolean - is_bar: Boolean + user: User! } -``` - -What about resolvers? To define resolvers for needed fields -you must use decorators with `hiers` [inheritance feature](type-inheritance.md). - -Here is how this can be done: - -- First of all, enable both config files format (yaml or xml with graphql) - - ```yaml - overblog_graphql: - definitions: - mappings: - types: - types: - - - types: [graphql, yaml] - dir: "%kernel.project_dir%/config/graphql/types" - ``` -- Now you can write the decorators: - - ```yaml - # config/graphql/resolvers.types.yaml - - QueryDecorator: - decorator: true - hiers: Query - config: - fields: - bar: { resolve: "@=..." } +enum User { + TATA + TITI + TOTO +} +``` - FooDecorator: - decorator: true - hiers: Foo - config: - resolveType: "@=..." - ``` +When using this shorthand syntax, you define your field resolvers (and some more configuration) separately +from the schema. Since the schema already describes all of the fields, arguments, and result types, the only +thing left is a collection of callable that are called to actually execute these fields. +This can be done using [resolver-map](resolver-map.md). **Notes:** - - This feature is experimental and could be improve or change in future releases -- Only type definition is allowed right now (excepted scalar type) using this shorthand syntax -- In general only required resolvers are the those managing fields on Root (query or mutation) -- Decorators is not limited to resolvers +- Only type definition is allowed right now using this shorthand syntax +- The definition of schema root query or/and mutation should still be done in +[main configuration file](schema.md). diff --git a/Resources/doc/definitions/resolver-map.md b/Resources/doc/definitions/resolver-map.md new file mode 100644 index 000000000..8a46964a7 --- /dev/null +++ b/Resources/doc/definitions/resolver-map.md @@ -0,0 +1,147 @@ +Resolver map +============ + +In order to respond to queries, a schema needs to have resolve functions for all fields. +Resolve functions cannot be included in the GraphQL schema language, so they must be added separately. +This collection of functions is called the "resolver map". + +Specification +-------------- + +A `resolverMap` is simple an object implementing `Overblog\GraphQLBundle\Resolver\ResolverMapInterface` +you can also just extend the concrete class `Overblog\GraphQLBundle\Resolver\ResolverMap` +and override `map` method and return an `array` or any `ArrayAccess` and `Traversable` implementation. + +### Resolving from a resolverMap + +* The `Overblog\GraphQLBundle\Resolver\ResolverMapInterface` exposes three methods: + `resolve`, `isResolvable` and `covered`. + It also exposes constants representing some specials config fields. +* `resolve` takes two mandatory parameters: the type name and the config field name to resolve, + which MUST be strings. `resolve` can return anything (a mixed value), + or throw a `Overblog\GraphQLBundle\UnresolvableException` if the resolver for type name and config field name + is not known to the resolverMap. +* `isResolvable` takes two parameters: the type name and the config field name to resolve, + which MUST be strings. + `isResolvable` MUST return true if the resolver for type name and config field name is known to + the resolverMap and false if it is not. If `isResolvable($typeName, $fieldName)` returns false, + `resolve($typeName, $fieldName)` MUST throw a `Overblog\GraphQLBundle\UnresolvableException`. +* `covered` takes unique optional parameter: the type name to resolve, + which MUST be strings. + `covered` MUST return an array of the names of the types covered if `$typeName` + equal to null or return the type fields covered. + If `covered($typeName)` returns an empty array or/and the fieldName is not present in array, + `resolve($typeName, $fieldName)` MUST throw a `Overblog\GraphQLBundle\UnresolvableException`. +* constants (specials config fields): + * [Union](type-system/union.md) and [Interface](type-system/interface.md) types + - `Overblog\GraphQLBundle\Resolver\ResolverMapInterface::RESOLVE_TYPE` equivalent to `resolveType`. + * [Object](type-system/object.md) type + - `Overblog\GraphQLBundle\Resolver\ResolverMapInterface::RESOLVE_FIELD` equivalent to `resolveField`. + - `Overblog\GraphQLBundle\Resolver\ResolverMapInterface::IS_TYPE_OF` equivalent to `isTypeOf`. + * [Custom scalar](type-system/scalars.md#custom-scalar) type + - `Overblog\GraphQLBundle\Resolver\ResolverMapInterface::SERIALIZE` equivalent to `serialize` + - `Overblog\GraphQLBundle\Resolver\ResolverMapInterface::PARSE_VALUE` equivalent to `parseValue` + - `Overblog\GraphQLBundle\Resolver\ResolverMapInterface::PARSE_LITERAL` equivalent to `parseLiteral` + +Usage +----- + +The following is an example of a valid resolverMap object +that can be be used for [GraphQL schema language](graphql-schema-language.md#usage) : + +```php + [ + self::RESOLVE_FIELD => function ($value, Argument $args, \ArrayObject $context, ResolveInfo $info) { + if ('baz' === $info->fieldName) { + $id = (int) $args['id']; + + return findBaz('baz', $id); + } + + return null; + }, + 'bar' => [Bar::class, 'getBar'], + ], + 'Foo' => [ + self::RESOLVE_TYPE => function ($value) { + return isset($value['user']) ? 'Bar' : null; + }, + ], + // enum internal values + 'User' => [ + 'TATA' => 1, + 'TITI' => 2, + 'TOTO' => 3, + ], + // custom scalar + 'Baz' => [ + self::SERIALIZE => function ($value) { + return sprintf('%s Formatted Baz', $value); + }, + self::PARSE_VALUE => function ($value) { + if (!is_string($value)) { + throw new Error(sprintf('Cannot represent following value as a valid Baz: %s.', Utils::printSafeJson($value))); + } + + return str_replace(' Formatted Baz', '', $value); + }, + self::PARSE_LITERAL => function ($valueNode) { + if (!$valueNode instanceof StringValueNode) { + throw new Error('Query error: Can only parse strings got: '.$valueNode->kind, [$valueNode]); + } + + return str_replace(' Formatted Baz', '', $valueNode->value); + }, + ], + ]; + } +} +``` + +Declare resolverMap to current schema + +```yaml +overblog_graphql: + definitions: + schema: + # ... + # resolver maps services IDs + resolver_maps: + - App\Resolver\MyResolverMap + +services: + App\Resolver\MyResolverMap: ~ +``` + +**Notes:** +- ResolverMap will override **all matching entries** when decorating types. +- ResolverMap does not supports `access` and `query complexity` right now. +- Many resolver map can be set for the same schema. + In this case the first resolverMap in list where `isResolvable` + returns `true` will be use. +- You don’t need to specify resolvers for every type in your schema. + If you don’t specify a resolver, GraphQL falls back to a default one, + which does the following. + +Credits +------- + +This feature was inspired by [Apollo GraphQL tools](https://www.apollographql.com/docs/graphql-tools/resolvers.html). + +Next step [solving N+1 problem](solving-n-plus-1-problem.md) diff --git a/Resources/doc/definitions/resolver.md b/Resources/doc/definitions/resolver.md index 1f4e1c9bf..e17c42835 100644 --- a/Resources/doc/definitions/resolver.md +++ b/Resources/doc/definitions/resolver.md @@ -100,7 +100,7 @@ Resolvers can be define 2 different ways `addition` mutation can be access by using `App\GraphQL\Mutation\CalcMutation::addition` or `add` alias. - You can also define custom dirs using the config: + You can also define custom dirs using the config (Symfony <3.3): ```yaml overblog_graphql: definitions: @@ -122,22 +122,27 @@ Resolvers can be define 2 different ways Here an example of how this can be done with DI `autoconfigure`: ```yaml - App\Mutation\: - resource: '../src/Mutation' - tags: ['overblog_graphql.mutation'] + services: + _instanceof: + GraphQL\Type\Definition\Type: + tags: ['overblog_graphql.type'] + + App\Mutation\: + resource: '../src/Mutation' + tags: ['overblog_graphql.mutation'] - App\Resolver\: - resource: '../src/Resolver' - tags: ['overblog_graphql.resolver'] + App\Resolver\: + resource: '../src/Resolver' + tags: ['overblog_graphql.resolver'] - App\Type\: - resource: '../src/Type' - tags: ['overblog_graphql.type'] + App\Type\: + resource: '../src/Type' ``` **Note:** * When using FQCN in yaml definition, backslash must be correctly quotes, here an example `'@=resolver("App\\GraphQL\\Resolver\\Greetings", [args['name']])'`. + * You can also see the more straight forward way using [resolver map](resolver-map.md) 2. **The service way** diff --git a/Resources/skeleton/TypeSystem.php.skeleton b/Resources/skeleton/TypeSystem.php.skeleton index 1927b7edc..e6c0f6f0e 100644 --- a/Resources/skeleton/TypeSystem.php.skeleton +++ b/Resources/skeleton/TypeSystem.php.skeleton @@ -5,12 +5,12 @@ class extends { -public function __construct(ConfigProcessor $configProcessor) +public function __construct(ConfigProcessor $configProcessor, GlobalVariables $globalVariables = null) { $configLoader = function(GlobalVariables $globalVariable) { return ; }; -$config = $configProcessor->process(LazyConfig::create($configLoader))->load(); +$config = $configProcessor->process(LazyConfig::create($configLoader, $globalVariables))->load(); parent::__construct($config); } } diff --git a/Tests/Config/Parser/GraphQLParserTest.php b/Tests/Config/Parser/GraphQLParserTest.php index 3ed1e1d3b..ab7ee7454 100644 --- a/Tests/Config/Parser/GraphQLParserTest.php +++ b/Tests/Config/Parser/GraphQLParserTest.php @@ -22,7 +22,7 @@ public function testParse() $fileName = __DIR__.'/fixtures/graphql/schema.graphql'; $expected = include __DIR__.'/fixtures/graphql/schema.php'; - $this->assertContainerAddFileToRessources($fileName); + $this->assertContainerAddFileToResources($fileName); $config = GraphQLParser::parse(new \SplFileInfo($fileName), $this->containerBuilder); $this->assertEquals($expected, $config); } @@ -31,7 +31,7 @@ public function testParseEmptyFile() { $fileName = __DIR__.'/fixtures/graphql/empty.graphql'; - $this->assertContainerAddFileToRessources($fileName); + $this->assertContainerAddFileToResources($fileName); $config = GraphQLParser::parse(new \SplFileInfo($fileName), $this->containerBuilder); $this->assertEquals([], $config); @@ -52,14 +52,14 @@ public function testParseNotSupportedSchemaDefinition() GraphQLParser::parse(new \SplFileInfo(__DIR__.'/fixtures/graphql/not-supported-schema-definition.graphql'), $this->containerBuilder); } - public function testParseNotSupportedScalarDefinition() + public function testCustomScalarTypeDefaultFieldValue() { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('ScalarType definition is not supported right now.'); - GraphQLParser::parse(new \SplFileInfo(__DIR__.'/fixtures/graphql/not-supported-scalar-definition.graphql'), $this->containerBuilder); + $this->expectException(\Exception::class); + $this->expectExceptionMessage('Config entry must be override with ResolverMap to be used.'); + GraphQLParser::mustOverrideConfig(); } - private function assertContainerAddFileToRessources($fileName) + private function assertContainerAddFileToResources($fileName) { $this->containerBuilder->expects($this->once()) ->method('addResource') diff --git a/Tests/Config/Parser/fixtures/graphql/not-supported-scalar-definition.graphql b/Tests/Config/Parser/fixtures/graphql/not-supported-scalar-definition.graphql deleted file mode 100644 index 76dfa4c05..000000000 --- a/Tests/Config/Parser/fixtures/graphql/not-supported-scalar-definition.graphql +++ /dev/null @@ -1 +0,0 @@ -scalar Url diff --git a/Tests/Config/Parser/fixtures/graphql/schema.graphql b/Tests/Config/Parser/fixtures/graphql/schema.graphql index cfb345afd..3fd725f2d 100644 --- a/Tests/Config/Parser/fixtures/graphql/schema.graphql +++ b/Tests/Config/Parser/fixtures/graphql/schema.graphql @@ -50,3 +50,5 @@ input ReviewInput { stars: Int! = 5 commentary: String = null } + +scalar Year diff --git a/Tests/Config/Parser/fixtures/graphql/schema.php b/Tests/Config/Parser/fixtures/graphql/schema.php index c8f745745..b9957c171 100644 --- a/Tests/Config/Parser/fixtures/graphql/schema.php +++ b/Tests/Config/Parser/fixtures/graphql/schema.php @@ -8,7 +8,7 @@ 'fields' => [ 'hero' => [ 'type' => 'Character', - 'arguments' => [ + 'args' => [ 'episodes' => [ 'type' => '[Episode!]!', 'description' => 'Episode list to use to filter', @@ -19,7 +19,7 @@ 'droid' => [ 'type' => 'Droid', 'description' => 'search for a droid', - 'arguments' => [ + 'args' => [ 'id' => [ 'type' => 'ID!', ], @@ -36,7 +36,7 @@ 'name' => ['type' => 'String!'], 'length' => [ 'type' => 'Float', - 'arguments' => [ + 'args' => [ 'unit' => [ 'type' => 'LengthUnit', 'defaultValue' => 'METER', @@ -50,9 +50,9 @@ 'type' => 'enum', 'config' => [ 'values' => [ - ['value' => 'NEWHOPE'], - ['value' => 'EMPIRE'], - ['value' => 'JEDI'], + 'NEWHOPE' => ['value' => 'NEWHOPE'], + 'EMPIRE' => ['value' => 'EMPIRE'], + 'JEDI' => ['value' => 'JEDI'], ], ], ], @@ -109,4 +109,12 @@ ], ], ], + 'Year' => [ + 'type' => 'custom-scalar', + 'config' => [ + 'serialize' => [\Overblog\GraphQLBundle\Config\Parser\GraphQLParser::class, 'mustOverrideConfig'], + 'parseValue' => [\Overblog\GraphQLBundle\Config\Parser\GraphQLParser::class, 'mustOverrideConfig'], + 'parseLiteral' => [\Overblog\GraphQLBundle\Config\Parser\GraphQLParser::class, 'mustOverrideConfig'], + ], + ], ]; diff --git a/Tests/Definition/GlobalVariablesTest.php b/Tests/Definition/GlobalVariablesTest.php new file mode 100644 index 000000000..f64a6e604 --- /dev/null +++ b/Tests/Definition/GlobalVariablesTest.php @@ -0,0 +1,16 @@ +expectException(\LogicException::class); + $this->expectExceptionMessage('Global variable "unknown" could not be located. You should define it.'); + (new GlobalVariables())->get('unknown'); + } +} diff --git a/Tests/Definition/Type/SchemaDecoratorTest.php b/Tests/Definition/Type/SchemaDecoratorTest.php new file mode 100644 index 000000000..2fd791ac1 --- /dev/null +++ b/Tests/Definition/Type/SchemaDecoratorTest.php @@ -0,0 +1,321 @@ +config[$fieldName]; + }; + } + $expected = static function () { + }; + $realFieldName = substr($fieldName, 2); + + $this->decorate( + [$typeWithSpecialField->name => $typeWithSpecialField], + [$typeWithSpecialField->name => [$fieldName => $expected]] + ); + + $actual = $fieldValueRetriever($typeWithSpecialField, $realFieldName); + + if ($strict) { + $this->assertSame($expected, $actual); + } else { + $this->assertNotNull($actual); + $this->assertInstanceOf(\Closure::class, $actual); + } + } + + public function testObjectTypeFieldDecoration() + { + $objectType = new ObjectType([ + 'name' => 'Foo', + 'fields' => function () { + return [ + 'bar' => ['type' => Type::string()], + 'baz' => ['type' => Type::string()], + 'toto' => ['type' => Type::boolean(), 'resolve' => null], + ]; + }, + ]); + $barResolver = static function () { + return 'bar'; + }; + $bazResolver = static function () { + return 'baz'; + }; + + $this->decorate( + [$objectType->name => $objectType], + [$objectType->name => ['bar' => $barResolver, 'baz' => $bazResolver]] + ); + $fields = $objectType->config['fields'](); + + foreach (['bar', 'baz'] as $fieldName) { + $this->assertInstanceOf(\Closure::class, $fields[$fieldName]['resolve']); + $this->assertSame($fieldName, $fields[$fieldName]['resolve']()); + } + + $this->assertNull($fields['toto']['resolve']); + + return $objectType; + } + + public function testWrappedResolver() + { + $objectType = new ObjectType([ + 'name' => 'Foo', + 'fields' => function () { + return [ + 'bar' => ['type' => Type::string()], + ]; + }, + ]); + + $this->decorate( + [$objectType->name => $objectType], + [ + $objectType->name => [ + 'bar' => function ($value, $args) { + return $args; + }, + ], + ] + ); + $expected = ['foo' => 'baz']; + $resolveFn = $objectType->getField('bar')->resolveFn; + /** @var Argument $args */ + $args = $resolveFn(null, $expected); + $this->assertInstanceOf(Argument::class, $args); + $this->assertSame($expected, $args->getRawArguments()); + } + + public function testEnumTypeValuesDecoration() + { + $enumType = new EnumType([ + 'name' => 'Foo', + 'values' => [ + 'BAR' => ['name' => 'BAR', 'value' => 'BAR'], + 'BAZ' => ['name' => 'BAZ', 'value' => 'BAZ'], + 'TOTO' => ['name' => 'TOTO', 'value' => 'TOTO'], + ], + ]); + + $this->decorate( + [$enumType->name => $enumType], + [$enumType->name => ['BAR' => 1, 'BAZ' => 2]] + ); + + $this->assertSame( + [ + 'BAR' => ['name' => 'BAR', 'value' => 1], + 'BAZ' => ['name' => 'BAZ', 'value' => 2], + 'TOTO' => ['name' => 'TOTO', 'value' => 'TOTO'], + ], + $enumType->config['values'] + ); + } + + public function testEnumTypeUnknownField() + { + $enumType = new EnumType([ + 'name' => 'Foo', + 'values' => [ + 'BAR' => ['name' => 'BAR', 'value' => 'BAR'], + ], + ]); + $this->assertDecorateException( + [$enumType->name => $enumType], + [$enumType->name => ['BAZ' => 1]], + \InvalidArgumentException::class, + '"Foo".{"BAZ"} defined in resolverMap, was defined in resolvers, but enum is not in schema.' + ); + } + + public function testUnionTypeUnknownField() + { + $unionType = new UnionType(['name' => 'Foo']); + $this->assertDecorateException( + [$unionType->name => $unionType], + [ + $unionType->name => [ + 'baz' => function () { + }, + ], + ], + \InvalidArgumentException::class, + '"Foo".{"baz"} defined in resolverMap, but only "__resolveType" is allowed.' + ); + } + + public function testInterfaceTypeUnknownField() + { + $interfaceType = new InterfaceType(['name' => 'Foo']); + $this->assertDecorateException( + [$interfaceType->name => $interfaceType], + [ + $interfaceType->name => [ + 'baz' => function () { + }, + ], + ], + \InvalidArgumentException::class, + '"Foo".{"baz"} defined in resolverMap, but only "__resolveType" is allowed.' + ); + } + + public function testCustomScalarTypeUnknownField() + { + $customScalarType = new CustomScalarType(['name' => 'Foo']); + $this->assertDecorateException( + [$customScalarType->name => $customScalarType], + [ + $customScalarType->name => [ + 'baz' => function () { + }, + ], + ], + \InvalidArgumentException::class, + '"Foo".{"baz"} defined in resolverMap, but only "__serialize", "__parseValue", "__parseLiteral" is allowed.' + ); + } + + public function testObjectTypeUnknownField() + { + $objectType = new ObjectType([ + 'name' => 'Foo', + 'fields' => [ + 'bar' => ['type' => Type::string()], + ], + ]); + $this->assertDecorateException( + [$objectType->name => $objectType], + [ + $objectType->name => [ + 'baz' => function () { + }, + ], + ], + \InvalidArgumentException::class, + '"Foo".{"baz"} defined in resolverMap, but not in schema.' + ); + } + + public function testUnSupportedTypeDefineInResolverMapShouldThrowAnException() + { + $this->assertDecorateException( + ['myType' => new InputObjectType(['name' => 'myType'])], + [ + 'myType' => [ + 'foo' => null, + 'bar' => null, + ], + ], + \InvalidArgumentException::class, + '"myType".{"foo", "bar"} defined in resolverMap, but type is not managed by SchemaDecorator.' + ); + } + + public function specialTypeFieldProvider() + { + $objectWithResolveField = new ObjectType(['name' => 'Bar', 'fields' => [], 'resolveField' => null]); + + return [ + // isTypeOf + [ResolverMapInterface::IS_TYPEOF, new ObjectType(['name' => 'Foo', 'fields' => [], 'isTypeOf' => null])], + // resolveField + [ + ResolverMapInterface::RESOLVE_FIELD, + $objectWithResolveField, + function (ObjectType $type) { + return $type->resolveFieldFn; + }, + false, + ], + [ResolverMapInterface::RESOLVE_FIELD, $objectWithResolveField, null, false], + // resolveType + [ResolverMapInterface::RESOLVE_TYPE, new UnionType(['name' => 'Baz', 'resolveType' => null])], + [ResolverMapInterface::RESOLVE_TYPE, new InterfaceType(['name' => 'Baz', 'resolveType' => null])], + // custom scalar + [ResolverMapInterface::SERIALIZE, new CustomScalarType(['name' => 'Custom', 'serialize' => null])], + [ResolverMapInterface::PARSE_VALUE, new CustomScalarType(['name' => 'Custom', 'parseValue' => null])], + [ResolverMapInterface::PARSE_LITERAL, new CustomScalarType(['name' => 'Custom', 'parseLiteral' => null])], + ]; + } + + private function assertDecorateException(array $types, array $map, $exception = null, $exceptionMessage = null) + { + if ($exception) { + $this->expectException($exception); + } + if ($exceptionMessage) { + $this->expectExceptionMessage($exceptionMessage); + } + + $this->decorate($types, $map); + } + + private function decorate(array $types, array $map) + { + (new SchemaDecorator())->decorate($this->createSchemaMock($types), $this->createResolverMapMock($map)); + } + + /** + * @param array $types + * + * @return \PHPUnit\Framework\MockObject\MockObject|Schema + */ + private function createSchemaMock(array $types = []) + { + $schema = $this->getMockBuilder(Schema::class)->disableOriginalConstructor()->setMethods(['getType'])->getMock(); + + $schema->expects($this->any())->method('getType')->willReturnCallback(function ($name) use ($types) { + if (!isset($types[$name])) { + throw new \Exception(sprintf('Type "%s" not found.', $name)); + } + + return $types[$name]; + }); + + return $schema; + } + + /** + * @param array $map + * + * @return \PHPUnit\Framework\MockObject\MockObject|ResolverMap + */ + private function createResolverMapMock(array $map = []) + { + $resolverMap = $this->getMockBuilder(ResolverMap::class)->setMethods(['map'])->getMock(); + $resolverMap->expects($this->any())->method('map')->willReturn($map); + + return $resolverMap; + } +} diff --git a/Tests/DependencyInjection/Compiler/GlobalVariablesPassTest.php b/Tests/DependencyInjection/Compiler/GlobalVariablesPassTest.php new file mode 100644 index 000000000..1311afb59 --- /dev/null +++ b/Tests/DependencyInjection/Compiler/GlobalVariablesPassTest.php @@ -0,0 +1,48 @@ +getMockBuilder(ContainerBuilder::class) + ->setMethods(['findTaggedServiceIds', 'findDefinition']) + ->getMock(); + $container->expects($this->once()) + ->method('findTaggedServiceIds') + ->willReturn([ + 'my-id' => [ + ['alias' => $invalidAlias], + ], + ]); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Service "my-id" tagged "overblog_graphql.global_variable" should have a valid "alias" attribute.'); + + (new GlobalVariablesPass())->process($container); + } + + public function invalidAliasProvider() + { + return [ + [null], + [new \stdClass()], + [[]], + [true], + [false], + [''], + ]; + } +} diff --git a/Tests/Functional/App/Resolver/Characters.php b/Tests/Functional/App/Resolver/Characters.php new file mode 100644 index 000000000..4df35291b --- /dev/null +++ b/Tests/Functional/App/Resolver/Characters.php @@ -0,0 +1,128 @@ + [ + 'id' => 1, + 'name' => 'Jon Snow', + 'direwolf' => 7, + 'status' => 1, + 'type' => self::TYPE_HUMAN, + 'dateOfBirth' => 281, + ], + 2 => [ + 'id' => 2, + 'name' => 'Arya', + 'direwolf' => 8, + 'status' => 1, + 'type' => self::TYPE_HUMAN, + 'dateOfBirth' => 287, + ], + 3 => [ + 'id' => 3, + 'name' => 'Bran', + 'direwolf' => 9, + 'status' => 1, + 'type' => self::TYPE_HUMAN, + 'dateOfBirth' => 288, + ], + 4 => [ + 'id' => 4, + 'name' => 'Rickon', + 'direwolf' => 10, + 'status' => 0, + 'type' => self::TYPE_HUMAN, + 'dateOfBirth' => 292, + ], + 5 => [ + 'id' => 5, + 'name' => 'Robb', + 'direwolf' => 11, + 'status' => 0, + 'type' => self::TYPE_HUMAN, + 'dateOfBirth' => 281, + ], + 6 => [ + 'id' => 6, + 'name' => 'Sansa', + 'direwolf' => 12, + 'status' => 1, + 'type' => self::TYPE_HUMAN, + 'dateOfBirth' => 285, + ], + 7 => [ + 'id' => 7, + 'name' => 'Ghost', + 'status' => 1, + 'type' => self::TYPE_DIREWOLF, + ], + 8 => [ + 'id' => 8, + 'name' => 'Nymeria', + 'status' => 1, + 'type' => self::TYPE_DIREWOLF, + ], + 9 => [ + 'id' => 9, + 'name' => 'Summer', + 'status' => 0, + 'type' => self::TYPE_DIREWOLF, + ], + 10 => [ + 'id' => 10, + 'name' => 'Shaggydog', + 'status' => 0, + 'type' => self::TYPE_DIREWOLF, + ], + 11 => [ + 'id' => 11, + 'name' => 'Grey Wind', + 'status' => 0, + 'type' => self::TYPE_DIREWOLF, + ], + 12 => [ + 'id' => 12, + 'name' => 'Lady', + 'status' => 0, + 'type' => self::TYPE_DIREWOLF, + ], + ]; + + public static function getCharacters() + { + return self::$characters; + } + + public static function getHumans() + { + $humans = self::findByType(self::TYPE_HUMAN); + + return $humans; + } + + public static function getDirewolves() + { + return self::findByType(self::TYPE_DIREWOLF); + } + + public static function resurrectZigZag() + { + $zigZag = self::$characters[4]; + $zigZag['status'] = 1; + + return $zigZag; + } + + private static function findByType($type) + { + return array_filter(self::$characters, function ($character) use ($type) { + return $type === $character['type']; + }); + } +} diff --git a/Tests/Functional/App/Resolver/SchemaLanguageMutationResolverMap.php b/Tests/Functional/App/Resolver/SchemaLanguageMutationResolverMap.php new file mode 100644 index 000000000..ad5985a99 --- /dev/null +++ b/Tests/Functional/App/Resolver/SchemaLanguageMutationResolverMap.php @@ -0,0 +1,17 @@ + [ + 'resurrectZigZag' => [Characters::class, 'resurrectZigZag'], + ], + ]; + } +} diff --git a/Tests/Functional/App/Resolver/SchemaLanguageQueryResolverMap.php b/Tests/Functional/App/Resolver/SchemaLanguageQueryResolverMap.php new file mode 100644 index 000000000..6378c696c --- /dev/null +++ b/Tests/Functional/App/Resolver/SchemaLanguageQueryResolverMap.php @@ -0,0 +1,81 @@ + [ + self::RESOLVE_FIELD => function ($value, Argument $args, \ArrayObject $context, ResolveInfo $info) { + if ('character' === $info->fieldName) { + $characters = Characters::getCharacters(); + $id = (int) $args['id']; + if (isset($characters[$id])) { + return $characters[$id]; + } + } + + return null; + }, + 'findHumansByDateOfBirth' => function ($value, Argument $args) { + $years = $args['years']; + + return array_filter(Characters::getHumans(), function ($human) use ($years) { + return in_array($human['dateOfBirth'], $years); + }); + }, + 'humans' => [Characters::class, 'getHumans'], + 'direwolves' => [Characters::class, 'getDirewolves'], + ], + 'Character' => [ + self::RESOLVE_TYPE => function ($value) { + return Characters::TYPE_HUMAN === $value['type'] ? 'Human' : 'Direwolf'; + }, + ], + 'Human' => [ + 'direwolf' => function ($value) { + $direwolves = Characters::getDirewolves(); + if (isset($direwolves[$value['direwolf']])) { + return $direwolves[$value['direwolf']]; + } else { + return null; + } + }, + ], + // enum internal values + 'Status' => [ + 'ALIVE' => 1, + 'DECEASED' => 0, + ], + // custom scalar + 'Year' => [ + self::SERIALIZE => function ($value) { + return sprintf('%s AC', $value); + }, + self::PARSE_VALUE => function ($value) { + if (!is_string($value)) { + throw new Error(sprintf('Cannot represent following value as a valid year: %s.', Utils::printSafeJson($value))); + } + + return (int) str_replace(' AC', '', $value); + }, + self::PARSE_LITERAL => function ($valueNode) { + if (!$valueNode instanceof StringValueNode) { + throw new Error('Query error: Can only parse strings got: '.$valueNode->kind, [$valueNode]); + } + + return (int) str_replace(' AC', '', $valueNode->value); + }, + ], + ]; + } +} diff --git a/Tests/Functional/App/config/global/config.yml b/Tests/Functional/App/config/global/config.yml index 0dcfc19b5..d2b0671da 100644 --- a/Tests/Functional/App/config/global/config.yml +++ b/Tests/Functional/App/config/global/config.yml @@ -18,5 +18,5 @@ overblog_graphql: mappings: types: - - types: [yaml, graphql] + type: yaml dir: "%kernel.root_dir%/config/global/mapping" diff --git a/Tests/Functional/App/config/global/mapping/decorators.types.yml b/Tests/Functional/App/config/global/mapping/decorators.types.yml deleted file mode 100644 index b39dbce6d..000000000 --- a/Tests/Functional/App/config/global/mapping/decorators.types.yml +++ /dev/null @@ -1,30 +0,0 @@ -PhotoDecorator: - decorator: true - heirs: [Photo] - config: - fields: - id: - builder: "Relay::GlobalId" - builderConfig: - typeName: Photo - idFetcher: '@=value["photoId"]' - -PostDecorator: - decorator: true - heirs: [Post] - config: - fields: - id: "Relay::GlobalId" - -PhotoAndPostDecorator: - decorator: true - heirs: PhotoAndPost - config: - resolveType: '@=service("overblog_graphql.test.resolver.global").typeResolver(value)' - -UserDecorator: - decorator: true - heirs: [User] - config: - fields: - id: "Relay::GlobalId" diff --git a/Tests/Functional/App/config/global/mapping/global.types.graphql b/Tests/Functional/App/config/global/mapping/global.types.graphql deleted file mode 100644 index 46bf335de..000000000 --- a/Tests/Functional/App/config/global/mapping/global.types.graphql +++ /dev/null @@ -1,25 +0,0 @@ - -type Photo implements NodeInterface { - # The ID of an object - id: ID! - width: Int -} - -union PhotoAndPost = Photo | Post - -input PhotoInput { - width: Int = 100 -} - -type Post implements NodeInterface { - # The ID of an object - id: ID! - text: String - status: Status -} - -type User implements NodeInterface { - # The ID of an object - id: ID! - name: String -} diff --git a/Tests/Functional/App/config/global/mapping/global.types.yml b/Tests/Functional/App/config/global/mapping/global.types.yml index 94d79d1b3..372b34ec1 100644 --- a/Tests/Functional/App/config/global/mapping/global.types.yml +++ b/Tests/Functional/App/config/global/mapping/global.types.yml @@ -1,3 +1,8 @@ +NodeInterface: + type: relay-node + config: + resolveType: '@=service("overblog_graphql.test.resolver.global").typeResolver(value)' + Query: type: object config: @@ -11,11 +16,52 @@ Query: type: '[NodeInterface]' resolve: '@=service("overblog_graphql.test.resolver.global").resolveAllObjects()' -NodeInterface: - type: relay-node +User: + type: object + config: + fields: + id: "Relay::GlobalId" + name: + type: String + interfaces: [NodeInterface] + +Photo: + type: object config: + fields: + id: + builder: "Relay::GlobalId" + builderConfig: + typeName: Photo + idFetcher: '@=value["photoId"]' + width: + type: Int + interfaces: [NodeInterface] + +Post: + type: object + config: + fields: + id: + builder: "Relay::GlobalId" + text: + type: String + status: + type: Status + interfaces: [NodeInterface] + +PhotoAndPost: + type: union + config: + types: [Photo, Post] resolveType: '@=service("overblog_graphql.test.resolver.global").typeResolver(value)' +PhotoInput: + type: input-object + config: + fields: + width: { type: Int, defaultValue: 100 } + Status: type: enum config: diff --git a/Tests/Functional/App/config/schemaLanguage/config.yml b/Tests/Functional/App/config/schemaLanguage/config.yml new file mode 100644 index 000000000..33ebecee9 --- /dev/null +++ b/Tests/Functional/App/config/schemaLanguage/config.yml @@ -0,0 +1,26 @@ +imports: + - { resource: ../config.yml } + +overblog_graphql: + definitions: + class_namespace: "Overblog\\GraphQLBundle\\SchemaLanguage\\__DEFINITIONS__" + schema: + query: Query + mutation: Mutation + resolver_maps: + - Overblog\GraphQLBundle\Tests\Functional\App\Resolver\SchemaLanguageQueryResolverMap + - Overblog\GraphQLBundle\Tests\Functional\App\Resolver\SchemaLanguageMutationResolverMap + mappings: + types: + - + type: graphql + dir: "%kernel.root_dir%/config/schemaLanguage/mapping" + suffix: ~ + +services: + Overblog\GraphQLBundle\Tests\Functional\App\Resolver\SchemaLanguageQueryResolverMap: + class: Overblog\GraphQLBundle\Tests\Functional\App\Resolver\SchemaLanguageQueryResolverMap + public: false + Overblog\GraphQLBundle\Tests\Functional\App\Resolver\SchemaLanguageMutationResolverMap: + class: Overblog\GraphQLBundle\Tests\Functional\App\Resolver\SchemaLanguageMutationResolverMap + public: false diff --git a/Tests/Functional/App/config/schemaLanguage/mapping/mutation.graphql b/Tests/Functional/App/config/schemaLanguage/mapping/mutation.graphql new file mode 100644 index 000000000..a3d48a150 --- /dev/null +++ b/Tests/Functional/App/config/schemaLanguage/mapping/mutation.graphql @@ -0,0 +1,3 @@ +type Mutation { + resurrectZigZag: Human! +} diff --git a/Tests/Functional/App/config/schemaLanguage/mapping/query.graphqls b/Tests/Functional/App/config/schemaLanguage/mapping/query.graphqls new file mode 100644 index 000000000..9df351fa1 --- /dev/null +++ b/Tests/Functional/App/config/schemaLanguage/mapping/query.graphqls @@ -0,0 +1,33 @@ +type Query { + character(id: ID!): Character + findHumansByDateOfBirth(years: [Year!]!): [Human!]! + humans: [Human!]! + direwolves: [Direwolf!]! +} + +scalar Year + +interface Character { + id: ID! + name: String! + status: Status! +} + +type Human implements Character { + id: ID! + name: String! + direwolf: Direwolf! + status: Status! + dateOfBirth: Year! +} + +type Direwolf implements Character { + id: ID! + name: String! + status: Status! +} + +enum Status { + ALIVE + DECEASED +} diff --git a/Tests/Functional/SchemaLanguage/SchemaLanguageTest.php b/Tests/Functional/SchemaLanguage/SchemaLanguageTest.php new file mode 100644 index 000000000..c00500327 --- /dev/null +++ b/Tests/Functional/SchemaLanguage/SchemaLanguageTest.php @@ -0,0 +1,177 @@ + [ + 'humans' => [ + [ + 'id' => 1, + 'name' => 'Jon Snow', + 'direwolf' => ['id' => 7, 'name' => 'Ghost'], + ], + [ + 'id' => 2, + 'name' => 'Arya', + 'direwolf' => ['id' => 8, 'name' => 'Nymeria'], + ], + [ + 'id' => 3, + 'name' => 'Bran', + 'direwolf' => ['id' => 9, 'name' => 'Summer'], + ], + [ + 'id' => 4, + 'name' => 'Rickon', + 'direwolf' => ['id' => 10, 'name' => 'Shaggydog'], + ], + [ + 'id' => 5, + 'name' => 'Robb', + 'direwolf' => ['id' => 11, 'name' => 'Grey Wind'], + ], + [ + 'id' => 6, + 'name' => 'Sansa', + 'direwolf' => ['id' => 12, 'name' => 'Lady'], + ], + ], + ], + ]; + + $this->assertResponse($query, $expected, static::ANONYMOUS_USER, 'schemaLanguage'); + } + + public function testQueryDirewolves() + { + $query = <<<'QUERY' +{ direwolves {name status} } +QUERY; + + $expected = [ + 'data' => [ + 'direwolves' => [ + ['name' => 'Ghost', 'status' => 'ALIVE'], + ['name' => 'Nymeria', 'status' => 'ALIVE'], + ['name' => 'Summer', 'status' => 'DECEASED'], + ['name' => 'Shaggydog', 'status' => 'DECEASED'], + ['name' => 'Grey Wind', 'status' => 'DECEASED'], + ['name' => 'Lady', 'status' => 'DECEASED'], + ], + ], + ]; + + $this->assertResponse($query, $expected, static::ANONYMOUS_USER, 'schemaLanguage'); + } + + public function testQueryACharacter() + { + $query = <<<'QUERY' +{ + character(id: 1) { + name + ...on Human { + dateOfBirth + } + } +} +QUERY; + + $expected = [ + 'data' => [ + 'character' => [ + 'name' => 'Jon Snow', + 'dateOfBirth' => '281 AC', + ], + ], + ]; + + $this->assertResponse($query, $expected, static::ANONYMOUS_USER, 'schemaLanguage'); + } + + public function testQueryHumanByDateOfBirth() + { + $query = <<<'QUERY' +{ + findHumansByDateOfBirth(years: ["281 AC", "288 AC"]) { + name + dateOfBirth + } +} +QUERY; + + $expected = [ + 'data' => [ + 'findHumansByDateOfBirth' => [ + [ + 'name' => 'Jon Snow', + 'dateOfBirth' => '281 AC', + ], + [ + 'name' => 'Bran', + 'dateOfBirth' => '288 AC', + ], + [ + 'name' => 'Robb', + 'dateOfBirth' => '281 AC', + ], + ], + ], + ]; + + $this->assertResponse($query, $expected, static::ANONYMOUS_USER, 'schemaLanguage'); + } + + public function testQueryHumanByDateOfBirthUsingVariables() + { + $query = <<<'QUERY' +query ($years: [Year!]!) { + findHumansByDateOfBirth(years: $years) { + name + dateOfBirth + } +} +QUERY; + + $expected = [ + 'data' => [ + 'findHumansByDateOfBirth' => [ + [ + 'name' => 'Bran', + 'dateOfBirth' => '288 AC', + ], + ], + ], + ]; + + $this->assertResponse($query, $expected, static::ANONYMOUS_USER, 'schemaLanguage', null, ['years' => ['288 AC']]); + } + + public function testMutation() + { + $query = <<<'QUERY' +mutation { resurrectZigZag {name status} } +QUERY; + + $expected = [ + 'data' => [ + 'resurrectZigZag' => [ + 'name' => 'Rickon', + 'status' => 'ALIVE', + ], + ], + ]; + + $this->assertResponse($query, $expected, static::ANONYMOUS_USER, 'schemaLanguage'); + } +} diff --git a/Tests/Functional/TestCase.php b/Tests/Functional/TestCase.php index 183f95034..09ca54e80 100644 --- a/Tests/Functional/TestCase.php +++ b/Tests/Functional/TestCase.php @@ -115,19 +115,19 @@ protected static function createClientAuthenticated($username, $testCase, $passw return $client; } - protected static function assertResponse($query, array $expected, $username, $testCase, $password = self::DEFAULT_PASSWORD) + protected static function assertResponse($query, array $expected, $username, $testCase, $password = self::DEFAULT_PASSWORD, array $variables = null) { $client = self::createClientAuthenticated($username, $testCase, $password); - $result = self::sendRequest($client, $query); + $result = self::sendRequest($client, $query, false, $variables); static::assertEquals($expected, json_decode($result, true), $result); return $client; } - protected static function sendRequest(Client $client, $query, $isDecoded = false) + protected static function sendRequest(Client $client, $query, $isDecoded = false, array $variables = null) { - $client->request('GET', '/', ['query' => $query]); + $client->request('GET', '/', ['query' => $query, 'variables' => json_encode($variables)]); $result = $client->getResponse()->getContent(); return $isDecoded ? json_decode($result, true) : $result; diff --git a/Tests/Resolver/ResolverMapTest.php b/Tests/Resolver/ResolverMapTest.php new file mode 100644 index 000000000..7ed0de874 --- /dev/null +++ b/Tests/Resolver/ResolverMapTest.php @@ -0,0 +1,133 @@ +createResolverMapMock($map); + $resolver = $resolverMap->resolve($typeName, $fieldName); + $this->assertSame($expectedResolver, $resolver); + } + + public function testMapMustBeOverride() + { + /** @var ResolverMap|\PHPUnit_Framework_MockObject_MockObject $resolverMap */ + $resolverMap = $this->getMockBuilder(ResolverMap::class)->setMethods(null)->getMock(); + $this->expectException(\LogicException::class); + $this->expectExceptionMessage(sprintf( + 'You must override the %s::map() method.', + get_class($resolverMap) + )); + + $resolverMap->resolve('Foo', 'bar'); + } + + /** + * @dataProvider invalidMapDataProvider + * + * @param mixed $invalidMap + * @param string $invalidType + */ + public function testInvalidMap($invalidMap, $invalidType) + { + $resolverMap = $this->createResolverMapMock($invalidMap); + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage(sprintf( + '%s::map() should return an array or an instance of \ArrayAccess and \Traversable but got "%s".', + get_class($resolverMap), + $invalidType + )); + $resolverMap->resolve('Foo', 'bar'); + } + + public function testUnresolvable() + { + $resolverMap = $this->createResolverMapMock([ + 'Query' => [ + ResolverMap::RESOLVE_FIELD => function () { + }, + ], + ]); + $this->expectException(UnresolvableException::class); + $this->expectExceptionMessage('Field "Foo.bar" could not be resolved.'); + $resolverMap->resolve('Foo', 'bar'); + } + + public function invalidMapDataProvider() + { + return [ + [null, 'NULL'], + [false, 'boolean'], + [true, 'boolean'], + ['baz', 'string'], + [new \stdClass(), 'stdClass'], + ]; + } + + public function validMapDataProvider() + { + $arrayMap = $this->map(); + $objectMap = new \ArrayObject($arrayMap); + + $validMap = []; + + foreach ([$arrayMap, $objectMap] as $map) { + $validMap = array_merge($validMap, [ + [$map, 'Query', ResolverMap::RESOLVE_FIELD, $map['Query'][ResolverMap::RESOLVE_FIELD]], + [$map, 'Query', 'foo', $map['Query']['foo']], + [$map, 'Query', 'bar', $map['Query']['bar']], + [$map, 'Query', 'baz', null], + [$map, 'FooInterface', ResolverMap::RESOLVE_TYPE, $map['FooInterface'][ResolverMap::RESOLVE_TYPE]], + ]); + } + + return $validMap; + } + + /** + * @param mixed $map + * + * @return ResolverMap|\PHPUnit_Framework_MockObject_MockObject + */ + private function createResolverMapMock($map) + { + /** @var ResolverMap|\PHPUnit_Framework_MockObject_MockObject $resolverMap */ + $resolverMap = $this->getMockBuilder(ResolverMap::class)->setMethods(['map'])->getMock(); + $resolverMap->method('map')->willReturn($map); + + return $resolverMap; + } + + private function map() + { + return [ + 'Query' => [ + ResolverMap::RESOLVE_FIELD => function () { + }, + 'foo' => function () { + }, + 'bar' => function () { + }, + 'baz' => null, + ], + 'FooInterface' => [ + ResolverMap::RESOLVE_TYPE => function () { + }, + ], + ]; + } +} diff --git a/Tests/Resolver/ResolverMapsTest.php b/Tests/Resolver/ResolverMapsTest.php new file mode 100644 index 000000000..e730a2682 --- /dev/null +++ b/Tests/Resolver/ResolverMapsTest.php @@ -0,0 +1,47 @@ +expectException(UnresolvableException::class); + $this->expectExceptionMessage('Field "Foo.bar" could not be resolved.'); + $resolverMaps->resolve('Foo', 'bar'); + } + + /** + * @dataProvider invalidResolverMapDataProvider + * + * @param array $resolverMaps + * @param string $type + */ + public function testInvalidResolverMap(array $resolverMaps, $type) + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf( + 'ResolverMap should be instance of "%s" but got "%s".', + ResolverMapInterface::class, + $type + )); + new ResolverMaps($resolverMaps); + } + + public function invalidResolverMapDataProvider() + { + return [ + [[null], 'NULL'], + [[false], 'boolean'], + [[true], 'boolean'], + [['baz'], 'string'], + [[new \stdClass()], 'stdClass'], + ]; + } +} diff --git a/Tests/Resolver/ResolverTest.php b/Tests/Resolver/ResolverTest.php index 3b3c1f18a..c6a92456e 100644 --- a/Tests/Resolver/ResolverTest.php +++ b/Tests/Resolver/ResolverTest.php @@ -22,6 +22,18 @@ public function testDefaultResolveFn($fieldName, $source, $expected) $this->assertEquals($expected, Resolver::defaultResolveFn($source, [], [], $info)); } + public function testSetObjectOrArrayValue() + { + $object = new \stdClass(); + $object->foo = null; + Resolver::setObjectOrArrayValue($object, 'foo', 'bar'); + $this->assertSame($object->foo, 'bar'); + + $data = ['foo' => null]; + Resolver::setObjectOrArrayValue($data, 'foo', 'bar'); + $this->assertSame($data['foo'], 'bar'); + } + public function resolverProvider() { $object = new Toto(); diff --git a/Tests/Resolver/TypeResolverTest.php b/Tests/Resolver/TypeResolverTest.php index 4cc8914f8..ac1e6d07b 100644 --- a/Tests/Resolver/TypeResolverTest.php +++ b/Tests/Resolver/TypeResolverTest.php @@ -27,6 +27,18 @@ public function createObjectType(array $config) return new ObjectType($config); } + /** + * @expectedException \RuntimeException + * @expectedExceptionMessage Type class for alias "type" could not be load. If you are using your own classLoader verify the path and the namespace please. + */ + public function testErrorLoadingType() + { + $this->resolver->addSolution('type', function () { + throw new \Exception('Could not load type.'); + }); + $this->resolver->resolve('type'); + } + /** * @expectedException \Overblog\GraphQLBundle\Resolver\UnsupportedResolverException * @expectedExceptionMessage Resolver "not-supported" must be "GraphQL\Type\Definition\Type" "stdClass" given. From c0a0f6aac97bca8c12112603b294274f93fbe773 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 25 Jan 2018 08:04:47 +0100 Subject: [PATCH 11/14] Add quick start documentation --- README.md | 1 + Resources/doc/definitions/quick-start.md | 68 ++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 Resources/doc/definitions/quick-start.md diff --git a/README.md b/README.md index 7f56ae50d..7eee33fcc 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ It also supports batching using libs like [ReactRelayNetworkLayer](https://githu Documentation ------------- +- [Quick start](Resources/doc/definitions/quick-start.md) - [Installation](Resources/doc/index.md) - [Definitions](Resources/doc/definitions/index.md) - [Type System](Resources/doc/definitions/type-system/index.md) diff --git a/Resources/doc/definitions/quick-start.md b/Resources/doc/definitions/quick-start.md new file mode 100644 index 000000000..85ec7e572 --- /dev/null +++ b/Resources/doc/definitions/quick-start.md @@ -0,0 +1,68 @@ +Quick start +=========== + +1. Install the bundle ([more details](../index.md)) + +```bash +composer require overblog/graphql-bundle +``` + +2. Configure the bundle to accept `graphql` format ([more details](graphql-schema-language.md)) + +```diff +# config/packages/graphql.yaml +overblog_graphql: + definitions: + schema: + query: Query + mappings: + auto_discover: false + types: + - +- type: yaml ++ type: graphql + dir: "%kernel.project_dir%/config/graphql/types" + suffix: ~ +``` + +3. Define schema using [GraphQL schema language](http://graphql.org/learn/schema/) +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 + Date: Thu, 25 Jan 2018 09:54:18 +0100 Subject: [PATCH 12/14] Fix travis php 5.6 build --- Tests/Definition/ConfigProcessorTest.php | 2 +- composer.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Definition/ConfigProcessorTest.php b/Tests/Definition/ConfigProcessorTest.php index b23b56c71..45ff0db4f 100644 --- a/Tests/Definition/ConfigProcessorTest.php +++ b/Tests/Definition/ConfigProcessorTest.php @@ -29,7 +29,7 @@ public function testOrderByPriorityDesc() { $configProcessor = new ConfigProcessor(); - $configProcessor->addConfigProcessor($nullConfigProcessor1 = new NullConfigProcessor()); + $configProcessor->addConfigProcessor($nullConfigProcessor1 = new NullConfigProcessor(), 2); $configProcessor->addConfigProcessor($nullConfigProcessor2 = new NullConfigProcessor(), 4); $configProcessor->addConfigProcessor($nullConfigProcessor3 = new NullConfigProcessor(), 256); $configProcessor->addConfigProcessor($nullConfigProcessor4 = new NullConfigProcessor()); diff --git a/composer.json b/composer.json index 7fb4d336d..dbc97cdd5 100644 --- a/composer.json +++ b/composer.json @@ -48,7 +48,7 @@ "react/promise": "To use ReactPHP promise adapter" }, "require-dev": { - "phpunit/phpunit": "^5.5 || ^6.0", + "phpunit/phpunit": "^5.7.26 || ^6.0", "psr/log": "^1.0", "react/promise": "^2.5", "sensio/framework-extra-bundle": "^3.0", From 9c53a7a4b2778613f740344a82b513629fa39f50 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 25 Jan 2018 10:39:36 +0100 Subject: [PATCH 13/14] Ignore scrutinizer "Tests" dir and improve GraphQLParser --- .scrutinizer.yml | 4 + Config/Parser/GraphQLParser.php | 134 ++++++++++++++++++-------------- Definition/LazyConfig.php | 2 +- Resolver/ResolverMap.php | 2 +- 4 files changed, 80 insertions(+), 62 deletions(-) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 8f5365c52..809eb0894 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -5,3 +5,7 @@ build: tests: override: - true + +filter: + excluded_paths: + - "Tests/" diff --git a/Config/Parser/GraphQLParser.php b/Config/Parser/GraphQLParser.php index 324bbee0c..9e50bacb3 100644 --- a/Config/Parser/GraphQLParser.php +++ b/Config/Parser/GraphQLParser.php @@ -2,8 +2,10 @@ namespace Overblog\GraphQLBundle\Config\Parser; +use GraphQL\Language\AST\DefinitionNode; use GraphQL\Language\AST\FieldDefinitionNode; use GraphQL\Language\AST\InputValueDefinitionNode; +use GraphQL\Language\AST\NameNode; use GraphQL\Language\AST\Node; use GraphQL\Language\AST\NodeKind; use GraphQL\Language\AST\TypeNode; @@ -49,10 +51,13 @@ public static function parse(\SplFileInfo $file, ContainerBuilder $container) throw new InvalidArgumentException(sprintf('An error occurred while parsing the file "%s".', $file), $e->getCode(), $e); } - /** @var Node $typeDef */ foreach ($ast->definitions as $typeDef) { - $typeConfig = self::$parser->typeDefinitionToConfig($typeDef); - $typesConfig[$typeDef->name->value] = $typeConfig; + if (isset($typeDef->name) && $typeDef->name instanceof NameNode) { + $typeConfig = self::$parser->typeDefinitionToConfig($typeDef); + $typesConfig[$typeDef->name->value] = $typeConfig; + } else { + self::throwUnsupportedDefinitionNode($typeDef); + } } return $typesConfig; @@ -63,56 +68,66 @@ public static function mustOverrideConfig() throw new \RuntimeException('Config entry must be override with ResolverMap to be used.'); } - protected function typeDefinitionToConfig(Node $typeDef) + protected function typeDefinitionToConfig(DefinitionNode $typeDef) { - switch ($typeDef->kind) { - case NodeKind::OBJECT_TYPE_DEFINITION: - case NodeKind::INTERFACE_TYPE_DEFINITION: - case NodeKind::INPUT_OBJECT_TYPE_DEFINITION: - case NodeKind::ENUM_TYPE_DEFINITION: - case NodeKind::UNION_TYPE_DEFINITION: - $config = []; - $this->addTypeFields($typeDef, $config); - $this->addDescription($typeDef, $config); - $this->addInterfaces($typeDef, $config); - $this->addTypes($typeDef, $config); - $this->addValues($typeDef, $config); - - return [ - 'type' => self::DEFINITION_TYPE_MAPPING[$typeDef->kind], - 'config' => $config, - ]; - - case NodeKind::SCALAR_TYPE_DEFINITION: - $mustOverride = [__CLASS__, 'mustOverrideConfig']; - $config = [ - 'serialize' => $mustOverride, - 'parseValue' => $mustOverride, - 'parseLiteral' => $mustOverride, - ]; - $this->addDescription($typeDef, $config); - - return [ - 'type' => self::DEFINITION_TYPE_MAPPING[$typeDef->kind], - 'config' => $config, - ]; - break; - - default: - throw new InvalidArgumentException( - sprintf( - '%s definition is not supported right now.', - preg_replace('@Definition$@', '', $typeDef->kind) - ) - ); + if (isset($typeDef->kind)) { + switch ($typeDef->kind) { + case NodeKind::OBJECT_TYPE_DEFINITION: + case NodeKind::INTERFACE_TYPE_DEFINITION: + case NodeKind::INPUT_OBJECT_TYPE_DEFINITION: + case NodeKind::ENUM_TYPE_DEFINITION: + case NodeKind::UNION_TYPE_DEFINITION: + $config = []; + $this->addTypeFields($typeDef, $config); + $this->addDescription($typeDef, $config); + $this->addInterfaces($typeDef, $config); + $this->addTypes($typeDef, $config); + $this->addValues($typeDef, $config); + + return [ + 'type' => self::DEFINITION_TYPE_MAPPING[$typeDef->kind], + 'config' => $config, + ]; + + case NodeKind::SCALAR_TYPE_DEFINITION: + $mustOverride = [__CLASS__, 'mustOverrideConfig']; + $config = [ + 'serialize' => $mustOverride, + 'parseValue' => $mustOverride, + 'parseLiteral' => $mustOverride, + ]; + $this->addDescription($typeDef, $config); + + return [ + 'type' => self::DEFINITION_TYPE_MAPPING[$typeDef->kind], + 'config' => $config, + ]; + break; + + default: + self::throwUnsupportedDefinitionNode($typeDef); + } } + + self::throwUnsupportedDefinitionNode($typeDef); + } + + private static function throwUnsupportedDefinitionNode(DefinitionNode $typeDef) + { + $path = explode('\\', get_class($typeDef)); + throw new InvalidArgumentException( + sprintf( + '%s definition is not supported right now.', + preg_replace('@DefinitionNode$@', '', array_pop($path)) + ) + ); } /** - * @param Node $typeDef - * @param array $config + * @param DefinitionNode $typeDef + * @param array $config */ - private function addTypeFields(Node $typeDef, array &$config) + private function addTypeFields(DefinitionNode $typeDef, array &$config) { if (!empty($typeDef->fields)) { $fields = []; @@ -131,13 +146,12 @@ private function addTypeFields(Node $typeDef, array &$config) /** * @param Node $fieldDef - * @param array $fieldConf + * @param array $fieldConf */ private function addFieldArguments(Node $fieldDef, array &$fieldConf) { if (!empty($fieldDef->arguments)) { $arguments = []; - /** @var InputValueDefinitionNode $definition */ foreach ($fieldDef->arguments as $definition) { $name = $definition->name->value; $arguments[$name] = []; @@ -150,10 +164,10 @@ private function addFieldArguments(Node $fieldDef, array &$fieldConf) } /** - * @param Node $typeDef - * @param array $config + * @param DefinitionNode $typeDef + * @param array $config */ - private function addInterfaces(Node $typeDef, array &$config) + private function addInterfaces(DefinitionNode $typeDef, array &$config) { if (!empty($typeDef->interfaces)) { $interfaces = []; @@ -165,10 +179,10 @@ private function addInterfaces(Node $typeDef, array &$config) } /** - * @param Node $typeDef - * @param array $config + * @param DefinitionNode $typeDef + * @param array $config */ - private function addTypes(Node $typeDef, array &$config) + private function addTypes(DefinitionNode $typeDef, array &$config) { if (!empty($typeDef->types)) { $types = []; @@ -180,10 +194,10 @@ private function addTypes(Node $typeDef, array &$config) } /** - * @param Node $typeDef + * @param DefinitionNode $typeDef * @param array $config */ - private function addValues(Node $typeDef, array &$config) + private function addValues(DefinitionNode $typeDef, array &$config) { if (!empty($typeDef->values)) { $values = []; @@ -208,7 +222,7 @@ private function addType(Node $definition, array &$config) /** * @param Node $definition - * @param array $config + * @param array $config */ private function addDescription(Node $definition, array &$config) { @@ -221,10 +235,10 @@ private function addDescription(Node $definition, array &$config) } /** - * @param InputValueDefinitionNode|FieldDefinitionNode $definition + * @param Node $definition * @param array $config */ - private function addDefaultValue($definition, array &$config) + private function addDefaultValue(Node $definition, array &$config) { if (!empty($definition->defaultValue)) { $config['defaultValue'] = $this->astValueNodeToConfig($definition->defaultValue); diff --git a/Definition/LazyConfig.php b/Definition/LazyConfig.php index 24e84c33e..fa6ed89b9 100644 --- a/Definition/LazyConfig.php +++ b/Definition/LazyConfig.php @@ -11,7 +11,7 @@ final class LazyConfig private $globalVariables; /** - * @var callable + * @var callable[] */ private $onPostLoad = []; diff --git a/Resolver/ResolverMap.php b/Resolver/ResolverMap.php index af1d67938..a374cbf4f 100644 --- a/Resolver/ResolverMap.php +++ b/Resolver/ResolverMap.php @@ -4,7 +4,7 @@ class ResolverMap implements ResolverMapInterface { - /** @var callable[] */ + /** @var array[] */ private $loadedMap; /** @var bool */ From 9437757da9a6e2ae61eb619c9ffa851914c2505b Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 25 Jan 2018 10:51:58 +0100 Subject: [PATCH 14/14] Give love to PHP CS --- Config/Parser/GraphQLParser.php | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Config/Parser/GraphQLParser.php b/Config/Parser/GraphQLParser.php index 9e50bacb3..b9f6d2dd9 100644 --- a/Config/Parser/GraphQLParser.php +++ b/Config/Parser/GraphQLParser.php @@ -124,8 +124,8 @@ private static function throwUnsupportedDefinitionNode(DefinitionNode $typeDef) } /** - * @param DefinitionNode $typeDef - * @param array $config + * @param DefinitionNode $typeDef + * @param array $config */ private function addTypeFields(DefinitionNode $typeDef, array &$config) { @@ -146,7 +146,7 @@ private function addTypeFields(DefinitionNode $typeDef, array &$config) /** * @param Node $fieldDef - * @param array $fieldConf + * @param array $fieldConf */ private function addFieldArguments(Node $fieldDef, array &$fieldConf) { @@ -164,8 +164,8 @@ private function addFieldArguments(Node $fieldDef, array &$fieldConf) } /** - * @param DefinitionNode $typeDef - * @param array $config + * @param DefinitionNode $typeDef + * @param array $config */ private function addInterfaces(DefinitionNode $typeDef, array &$config) { @@ -179,8 +179,8 @@ private function addInterfaces(DefinitionNode $typeDef, array &$config) } /** - * @param DefinitionNode $typeDef - * @param array $config + * @param DefinitionNode $typeDef + * @param array $config */ private function addTypes(DefinitionNode $typeDef, array &$config) { @@ -194,8 +194,8 @@ private function addTypes(DefinitionNode $typeDef, array &$config) } /** - * @param DefinitionNode $typeDef - * @param array $config + * @param DefinitionNode $typeDef + * @param array $config */ private function addValues(DefinitionNode $typeDef, array &$config) { @@ -222,7 +222,7 @@ private function addType(Node $definition, array &$config) /** * @param Node $definition - * @param array $config + * @param array $config */ private function addDescription(Node $definition, array &$config) { @@ -235,8 +235,8 @@ private function addDescription(Node $definition, array &$config) } /** - * @param Node $definition - * @param array $config + * @param Node $definition + * @param array $config */ private function addDefaultValue(Node $definition, array &$config) {