From 2ae164e56618ea3781ef7bc9e7882a8538aa2eed Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Mon, 20 Nov 2017 07:53:48 +0100 Subject: [PATCH 1/7] 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 | 2 +- .../OverblogGraphQLTypesExtension.php | 19 +- 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 ++++++++ 13 files changed, 525 insertions(+), 27 deletions(-) create mode 100644 Config/Parser/GraphQLParser.php 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 diff --git a/Config/Parser/GraphQLParser.php b/Config/Parser/GraphQLParser.php new file mode 100644 index 000000000..9be68ea4f --- /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::$instance) { + self::$instance = new static(); + } + try { + $ast = Parser::parse($content); + + /** @var Node $typeDef */ + foreach ($ast->definitions as $typeDef) { + $typeConfig = self::$instance->typeDefinitionToConfig($typeDef); + $typesConfig[$typeDef->name->value] = $typeConfig; + } + } catch (SyntaxError $e) { + throw new InvalidArgumentException(sprintf('An error occurred while parsing the file "%s".', $file), $e->getCode(), $e); + } + + 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 4cb70f409..d1e496554 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -109,7 +109,7 @@ public function getConfigTreeBuilder() }) ->end() ->children() - ->enumNode('type')->values(['yaml', 'xml'])->defaultNull()->end() + ->enumNode('type')->values(array_keys(OverblogGraphQLTypesExtension::SUPPORTED_TYPES_EXTENSIONS))->defaultNull()->end() ->scalarNode('dir')->defaultNull()->end() ->scalarNode('suffix')->defaultValue(OverblogGraphQLTypesExtension::DEFAULT_TYPES_SUFFIX)->end() ->end() diff --git a/DependencyInjection/OverblogGraphQLTypesExtension.php b/DependencyInjection/OverblogGraphQLTypesExtension.php index 874e4ac10..1927f7e92 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' => [ @@ -67,9 +74,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; } @@ -162,11 +167,11 @@ private function detectFilesByType(ContainerBuilder $container, $path, $type, $s $finder = new Finder(); - $types = null === $type ? self::$configTypes : [$type]; + $types = null === $type ? array_keys(self::SUPPORTED_TYPES_EXTENSIONS) : [$type]; foreach ($types as $type) { 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; } 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], + ], + ], + ], +]; From 381fab80745359eecf40167899550fdd2cb7af41 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Tue, 21 Nov 2017 11:57:56 +0100 Subject: [PATCH 2/7] Refactor Executor using Events --- Command/GraphQLDumpSchemaCommand.php | 2 +- Config/Parser/GraphQLParser.php | 22 ++-- Controller/GraphController.php | 9 +- DependencyInjection/Configuration.php | 6 + Event/Events.php | 2 + Event/ExecutorContextEvent.php | 18 +-- Event/ExecutorEvent.php | 85 ++++++++++++ Event/ExecutorResultEvent.php | 37 ++++++ EventListener/RequestFilesListener.php | 1 - Request/Executor.php | 173 ++++++++++++++++++------- Tests/Functional/TestCase.php | 2 +- Tests/Request/ExecutorTest.php | 21 ++- composer.json | 1 + 13 files changed, 291 insertions(+), 88 deletions(-) create mode 100644 Event/ExecutorEvent.php create mode 100644 Event/ExecutorResultEvent.php diff --git a/Command/GraphQLDumpSchemaCommand.php b/Command/GraphQLDumpSchemaCommand.php index 9bb40d7fe..a40c37f49 100644 --- a/Command/GraphQLDumpSchemaCommand.php +++ b/Command/GraphQLDumpSchemaCommand.php @@ -94,7 +94,7 @@ private function createFile(InputInterface $input) $modern = $this->useModernJsonFormat($input); $result = $this->getRequestExecutor() - ->execute($request, [], $schemaName) + ->execute($schemaName, $request) ->toArray(); $content = json_encode($modern ? $result : $result['data'], \JSON_PRETTY_PRINT); diff --git a/Config/Parser/GraphQLParser.php b/Config/Parser/GraphQLParser.php index 9be68ea4f..1e92e4267 100644 --- a/Config/Parser/GraphQLParser.php +++ b/Config/Parser/GraphQLParser.php @@ -2,7 +2,6 @@ namespace Overblog\GraphQLBundle\Config\Parser; -use GraphQL\Error\SyntaxError; use GraphQL\Language\AST\FieldDefinitionNode; use GraphQL\Language\AST\InputValueDefinitionNode; use GraphQL\Language\AST\Node; @@ -16,7 +15,8 @@ class GraphQLParser implements ParserInterface { - private static $instance; + /** @var self */ + private static $parser; const DEFINITION_TYPE_MAPPING = [ NodeKind::OBJECT_TYPE_DEFINITION => 'object', @@ -39,21 +39,21 @@ public static function parse(\SplFileInfo $file, ContainerBuilder $container) if (empty($content)) { return []; } - if (!self::$instance) { - self::$instance = new static(); + if (!self::$parser) { + self::$parser = new static(); } try { $ast = Parser::parse($content); - - /** @var Node $typeDef */ - foreach ($ast->definitions as $typeDef) { - $typeConfig = self::$instance->typeDefinitionToConfig($typeDef); - $typesConfig[$typeDef->name->value] = $typeConfig; - } - } catch (SyntaxError $e) { + } 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; } diff --git a/Controller/GraphController.php b/Controller/GraphController.php index 5022e2bdd..6dacdd5fa 100644 --- a/Controller/GraphController.php +++ b/Controller/GraphController.php @@ -95,12 +95,7 @@ private function processBatchQuery(Request $request, $schemaName = null) foreach ($queries as $query) { $payloadResult = $this->requestExecutor->execute( - [ - 'query' => $query['query'], - 'variables' => $query['variables'], - ], - [], - $schemaName + $schemaName, ['query' => $query['query'], 'variables' => $query['variables']] ); $payloads[] = $apolloBatching ? $payloadResult->toArray() : ['id' => $query['id'], 'payload' => $payloadResult->toArray()]; } @@ -112,6 +107,6 @@ private function processNormalQuery(Request $request, $schemaName = null) { $params = $this->requestParser->parse($request); - return $this->requestExecutor->execute($params, [], $schemaName)->toArray(); + return $this->requestExecutor->execute($schemaName, $params)->toArray(); } } diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index d1e496554..62a8dc9e6 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -6,6 +6,7 @@ use GraphQL\Validator\Rules\QueryDepth; use Overblog\GraphQLBundle\Error\ErrorHandler; use Overblog\GraphQLBundle\Resolver\Resolver; +use Symfony\Component\Config\Definition\Builder\NodeParentInterface; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; @@ -184,6 +185,8 @@ public function getConfigTreeBuilder() /** * @param string $name + * + * @return NodeParentInterface */ private function addBuilderSection($name) { @@ -220,6 +223,9 @@ private function addBuilderSection($name) /** * @param string $name + * @param bool $disabledValue + * + * @return NodeParentInterface */ private function addSecurityQuerySection($name, $disabledValue) { diff --git a/Event/Events.php b/Event/Events.php index 02f137226..102ff3fde 100644 --- a/Event/Events.php +++ b/Event/Events.php @@ -5,4 +5,6 @@ final class Events { const EXECUTOR_CONTEXT = 'graphql.executor.context'; + const PRE_EXECUTOR = 'graphql.pre_executor'; + const POST_EXECUTOR = 'graphql.post_executor'; } diff --git a/Event/ExecutorContextEvent.php b/Event/ExecutorContextEvent.php index eb800f720..53e871a6d 100644 --- a/Event/ExecutorContextEvent.php +++ b/Event/ExecutorContextEvent.php @@ -6,26 +6,22 @@ class ExecutorContextEvent extends Event { - private $executorContext = []; + /** @var \ArrayObject */ + private $executorContext; - public function __construct(array $executorContext) + /** + * @param \ArrayObject $executorContext + */ + public function __construct(\ArrayObject $executorContext) { $this->executorContext = $executorContext; } /** - * @return array + * @return \ArrayObject */ public function getExecutorContext() { return $this->executorContext; } - - /** - * @param array $executionContext - */ - public function setExecutorContext(array $executionContext) - { - $this->executorContext = $executionContext; - } } diff --git a/Event/ExecutorEvent.php b/Event/ExecutorEvent.php new file mode 100644 index 000000000..30165c28c --- /dev/null +++ b/Event/ExecutorEvent.php @@ -0,0 +1,85 @@ +schema = $schema; + $this->requestString = $requestString; + $this->rootValue = $rootValue; + $this->contextValue = $contextValue; + $this->variableValue = $variableValue; + $this->operationName = $operationName; + } + + /** + * @return Schema + */ + public function getSchema() + { + return $this->schema; + } + + /** + * @return string + */ + public function getRequestString() + { + return $this->requestString; + } + + /** + * @return \ArrayObject + */ + public function getRootValue() + { + return $this->rootValue; + } + + /** + * @return \ArrayObject + */ + public function getContextValue() + { + return $this->contextValue; + } + + /** + * @return array|null + */ + public function getVariableValue() + { + return $this->variableValue; + } + + /** + * @return null|string + */ + public function getOperationName() + { + return $this->operationName; + } +} diff --git a/Event/ExecutorResultEvent.php b/Event/ExecutorResultEvent.php new file mode 100644 index 000000000..acf408d2b --- /dev/null +++ b/Event/ExecutorResultEvent.php @@ -0,0 +1,37 @@ +result = $result; + $this->contextValue = $contextValue; + } + + /** + * @return ExecutionResult + */ + public function getResult() + { + return $this->result; + } + + /** + * @return \ArrayObject + */ + public function getContextValue() + { + return $this->contextValue; + } +} diff --git a/EventListener/RequestFilesListener.php b/EventListener/RequestFilesListener.php index d01a193ba..b3036abca 100644 --- a/EventListener/RequestFilesListener.php +++ b/EventListener/RequestFilesListener.php @@ -26,6 +26,5 @@ public function onExecutorContextEvent(ExecutorContextEvent $event) $context = $event->getExecutorContext(); $context['request_files'] = $request->files; - $event->setExecutorContext($context); } } diff --git a/Request/Executor.php b/Request/Executor.php index a3e04183a..36087bd00 100644 --- a/Request/Executor.php +++ b/Request/Executor.php @@ -11,6 +11,8 @@ use Overblog\GraphQLBundle\Error\ErrorHandler; use Overblog\GraphQLBundle\Event\Events; use Overblog\GraphQLBundle\Event\ExecutorContextEvent; +use Overblog\GraphQLBundle\Event\ExecutorEvent; +use Overblog\GraphQLBundle\Event\ExecutorResultEvent; use Overblog\GraphQLBundle\Executor\ExecutorInterface; use Overblog\GraphQLBundle\Executor\Promise\PromiseAdapterInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -46,7 +48,7 @@ class Executor public function __construct( ExecutorInterface $executor, - EventDispatcherInterface $dispatcher = null, + EventDispatcherInterface $dispatcher, $throwException = false, ErrorHandler $errorHandler = null, $hasDebugInfo = false, @@ -78,6 +80,9 @@ public function setPromiseAdapter(PromiseAdapter $promiseAdapter = null) /** * @param string $name + * @param Schema $schema + * + * @return $this */ public function addSchema($name, Schema $schema) { @@ -86,6 +91,29 @@ public function addSchema($name, Schema $schema) return $this; } + /** + * @param string|null $name + * + * @return Schema + */ + public function getSchema($name = null) + { + if (empty($this->schemas)) { + throw new \RuntimeException('At least one schema should be declare.'); + } + + if (null === $name) { + $schema = array_values($this->schemas)[0]; + } else { + if (!isset($this->schemas[$name])) { + throw new NotFoundHttpException(sprintf('Could not found "%s" schema.', $name)); + } + $schema = $this->schemas[$name]; + } + + return $schema; + } + public function enabledDebugInfo() { $this->hasDebugInfo = true; @@ -131,57 +159,92 @@ public function setThrowException($throwException) return $this; } - public function execute(array $data, array $context = [], $schemaName = null) + /** + * @param null|string $schemaName + * @param array $config + * @param null|array|\ArrayObject|object $rootValue + * @param null|array|\ArrayObject|object $contextValue + * + * @return ExecutionResult + */ + public function execute($schemaName, array $config, $rootValue = null, $contextValue = null) { - if (null !== $this->dispatcher) { - $event = new ExecutorContextEvent($context); - $this->dispatcher->dispatch(Events::EXECUTOR_CONTEXT, $event); - $context = $event->getExecutorContext(); - } - - if ($this->promiseAdapter) { - if (!$this->promiseAdapter instanceof PromiseAdapterInterface && !is_callable([$this->promiseAdapter, 'wait'])) { - throw new \RuntimeException( - sprintf( - 'PromiseAdapter should be an object instantiating "%s" or "%s" with a "wait" method.', - PromiseAdapterInterface::class, - PromiseAdapter::class - ) - ); - } - } - - $schema = $this->getSchema($schemaName); + $executorEvent = $this->preExecute( + $this->getSchema($schemaName), + isset($config[ParserInterface::PARAM_QUERY]) ? $config[ParserInterface::PARAM_QUERY] : null, + self::createArrayObject($rootValue), + self::createArrayObject($contextValue), + $config[ParserInterface::PARAM_VARIABLES], + isset($config[ParserInterface::PARAM_OPERATION_NAME]) ? $config[ParserInterface::PARAM_OPERATION_NAME] : null + ); $startTime = microtime(true); $startMemoryUsage = memory_get_usage(true); + $result = $this->executor->execute( + $executorEvent->getSchema(), + $executorEvent->getRequestString(), + $executorEvent->getRootValue(), + $executorEvent->getContextValue(), + $executorEvent->getVariableValue(), + $executorEvent->getOperationName() + ); + + $result = $this->postExecute($executorEvent->getContextValue(), $result, $startTime, $startMemoryUsage); + + return $result; + } + + /** + * @param Schema $schema + * @param string $requestString + * @param \ArrayObject $rootValue + * @param \ArrayObject $contextValue + * @param array|null $variableValue + * @param string|null $operationName + * + * @return ExecutorEvent + */ + private function preExecute(Schema $schema, $requestString, \ArrayObject $rootValue, \ArrayObject $contextValue, array $variableValue = null, $operationName = null) + { + $this->checkPromiseAdapter(); + $this->executor->setPromiseAdapter($this->promiseAdapter); // this is needed when not using only generated types if ($this->defaultFieldResolver) { $this->executor->setDefaultFieldResolver($this->defaultFieldResolver); } + $this->dispatcher->dispatch(Events::EXECUTOR_CONTEXT, new ExecutorContextEvent($contextValue)); - $result = $this->executor->execute( - $schema, - isset($data[ParserInterface::PARAM_QUERY]) ? $data[ParserInterface::PARAM_QUERY] : null, - $context, - $context, - $data[ParserInterface::PARAM_VARIABLES], - isset($data[ParserInterface::PARAM_OPERATION_NAME]) ? $data[ParserInterface::PARAM_OPERATION_NAME] : null + return $this->dispatcher->dispatch( + Events::PRE_EXECUTOR, + new ExecutorEvent($schema, $requestString, $rootValue, $contextValue, $variableValue, $operationName) ); + } + /** + * @param \ArrayObject $contextValue + * @param ExecutionResult $result + * @param int $startTime + * @param int $startMemoryUsage + * + * @return ExecutionResult + */ + private function postExecute(\ArrayObject $contextValue, ExecutionResult $result, $startTime, $startMemoryUsage) + { if ($this->promiseAdapter) { $result = $this->promiseAdapter->wait($result); } - if (!is_object($result) || !$result instanceof ExecutionResult) { - throw new \RuntimeException( - sprintf('Execution result should be an object instantiating "%s".', ExecutionResult::class) - ); - } + $this->checkExecutionResult($result); + $result = $this->prepareResult($result, $startTime, $startMemoryUsage); - return $this->prepareResult($result, $startTime, $startMemoryUsage); + $event = $this->dispatcher->dispatch( + Events::POST_EXECUTOR, + new ExecutorResultEvent($result, $contextValue) + ); + + return $event->getResult(); } /** @@ -207,26 +270,38 @@ private function prepareResult($result, $startTime, $startMemoryUsage) return $result; } - /** - * @param string|null $name - * - * @return Schema - */ - public function getSchema($name = null) + private function checkPromiseAdapter() { - if (empty($this->schemas)) { - throw new \RuntimeException('At least one schema should be declare.'); + if ($this->promiseAdapter && !$this->promiseAdapter instanceof PromiseAdapterInterface && !is_callable([$this->promiseAdapter, 'wait'])) { + throw new \RuntimeException( + sprintf( + 'PromiseAdapter should be an object instantiating "%s" or "%s" with a "wait" method.', + PromiseAdapterInterface::class, + PromiseAdapter::class + ) + ); } + } - if (null === $name) { - $schema = array_values($this->schemas)[0]; + private function checkExecutionResult($result) + { + if (!is_object($result) || !$result instanceof ExecutionResult) { + throw new \RuntimeException( + sprintf('Execution result should be an object instantiating "%s".', ExecutionResult::class) + ); + } + } + + private static function createArrayObject($data) + { + if ($data instanceof \ArrayObject) { + $object = $data; + } elseif (is_array($data) || is_object($data)) { + $object = new \ArrayObject($data); } else { - if (!isset($this->schemas[$name])) { - throw new NotFoundHttpException(sprintf('Could not found "%s" schema.', $name)); - } - $schema = $this->schemas[$name]; + $object = new \ArrayObject(); } - return $schema; + return $object; } } diff --git a/Tests/Functional/TestCase.php b/Tests/Functional/TestCase.php index 4076cec2e..311c1ccb6 100644 --- a/Tests/Functional/TestCase.php +++ b/Tests/Functional/TestCase.php @@ -67,7 +67,7 @@ protected static function executeGraphQLRequest($query, $rootValue = [], $throwE $req = static::getContainer()->get('overblog_graphql.request_parser')->parse($request); $executor = static::getContainer()->get('overblog_graphql.request_executor'); $executor->setThrowException($throwException); - $res = $executor->execute($req, $rootValue); + $res = $executor->execute(null, $req, $rootValue); return $res->toArray(); } diff --git a/Tests/Request/ExecutorTest.php b/Tests/Request/ExecutorTest.php index fff81684d..494a85cba 100644 --- a/Tests/Request/ExecutorTest.php +++ b/Tests/Request/ExecutorTest.php @@ -9,17 +9,24 @@ use Overblog\GraphQLBundle\Executor\Executor; use Overblog\GraphQLBundle\Request\Executor as RequestExecutor; use PHPUnit\Framework\TestCase; +use Symfony\Component\EventDispatcher\EventDispatcher; class ExecutorTest extends TestCase { /** @var RequestExecutor */ private $executor; + /** @var EventDispatcher|\PHPUnit_Framework_MockObject_MockObject */ + private $dispatcher; + private $request = ['query' => 'query debug{ myField }', 'variables' => [], 'operationName' => null]; public function setUp() { - $this->executor = new RequestExecutor(new Executor()); + $this->dispatcher = $this->getMockBuilder(EventDispatcher::class)->setMethods(['dispatch'])->getMock(); + $this->dispatcher->expects($this->any())->method('dispatch')->willReturnArgument(1); + + $this->executor = new RequestExecutor(new Executor(), $this->dispatcher); $queryType = new ObjectType([ 'name' => 'Query', 'fields' => [ @@ -41,7 +48,7 @@ public function setUp() public function testInvalidExecutorReturnNotObject() { $this->executor->setExecutor($this->createExecutorExecuteMock(false)); - $this->executor->execute($this->request); + $this->executor->execute(null, $this->request); } /** @@ -51,7 +58,7 @@ public function testInvalidExecutorReturnNotObject() public function testInvalidExecutorReturnInvalidObject() { $this->executor->setExecutor($this->createExecutorExecuteMock(new \stdClass())); - $this->executor->execute($this->request); + $this->executor->execute(null, $this->request); } /** @@ -61,17 +68,17 @@ public function testInvalidExecutorReturnInvalidObject() public function testInvalidExecutorAdapterPromise() { $this->executor->setPromiseAdapter(new ReactPromiseAdapter()); - $this->executor->execute($this->request); + $this->executor->execute(null, $this->request); } public function testDisabledDebugInfo() { - $this->assertArrayNotHasKey('debug', $this->executor->disabledDebugInfo()->execute($this->request)->extensions); + $this->assertArrayNotHasKey('debug', $this->executor->disabledDebugInfo()->execute(null, $this->request)->extensions); } public function testEnabledDebugInfo() { - $result = $this->executor->enabledDebugInfo()->execute($this->request); + $result = $this->executor->enabledDebugInfo()->execute(null, $this->request); $this->assertArrayHasKey('debug', $result->extensions); $this->assertArrayHasKey('executionTime', $result->extensions['debug']); @@ -84,7 +91,7 @@ public function testEnabledDebugInfo() */ public function testGetSchemaNoSchemaFound() { - (new RequestExecutor(new Executor()))->getSchema('fake'); + (new RequestExecutor(new Executor(), $this->dispatcher))->getSchema('fake'); } private function createExecutorExecuteMock($returnValue) diff --git a/composer.json b/composer.json index 2bd8e4180..0b594caa9 100644 --- a/composer.json +++ b/composer.json @@ -35,6 +35,7 @@ "symfony/cache": "^3.1 || ^4.0", "symfony/config": "^3.1 || ^4.0", "symfony/dependency-injection": "^3.1 || ^4.0", + "symfony/event-dispatcher": "^3.1 || ^4.0", "symfony/expression-language": "^3.1 || ^4.0", "symfony/framework-bundle": "^3.1 || ^4.0", "symfony/options-resolver": "^3.1 || ^4.0", From 0df9f2d7c7dd2add612e15093f3ab8d4069166d4 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Wed, 22 Nov 2017 11:30:24 +0100 Subject: [PATCH 3/7] Move Executor debug to dedicate EventListener --- .../OverblogGraphQLExtension.php | 15 ++++-- EventListener/DebugListener.php | 28 ++++++++++ Request/Executor.php | 51 +++---------------- Resources/config/services.yml | 1 - Tests/Functional/App/config/debug/config.yml | 7 +++ .../EventListener/DebugListenerTest.php | 26 ++++++++++ Tests/Functional/TestCase.php | 13 +++-- Tests/Request/ExecutorTest.php | 14 ----- 8 files changed, 90 insertions(+), 65 deletions(-) create mode 100644 EventListener/DebugListener.php create mode 100644 Tests/Functional/App/config/debug/config.yml create mode 100644 Tests/Functional/EventListener/DebugListenerTest.php diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index 5704573ba..0036dcb7c 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -5,7 +5,9 @@ use GraphQL\Type\Schema; use Overblog\GraphQLBundle\CacheWarmer\CompileCacheWarmer; use Overblog\GraphQLBundle\Config\Processor\BuilderProcessor; +use Overblog\GraphQLBundle\Event\Events; use Overblog\GraphQLBundle\EventListener\ClassLoaderListener; +use Overblog\GraphQLBundle\EventListener\DebugListener; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -35,7 +37,7 @@ public function load(array $configs, ContainerBuilder $container) $this->setErrorHandlerArguments($config, $container); $this->setSecurity($config, $container); $this->setConfigBuilders($config, $container); - $this->setShowDebug($config, $container); + $this->setDebugListener($config, $container); $this->setDefinitionParameters($config, $container); $this->setClassLoaderListener($config, $container); $this->setCompilerCacheWarmer($config, $container); @@ -117,9 +119,16 @@ private function setExpressionLanguageDefaultParser(ContainerBuilder $container) $container->setDefinition($this->getAlias().'.cache_expression_language_parser.default', $definition); } - private function setShowDebug(array $config, ContainerBuilder $container) + private function setDebugListener(array $config, ContainerBuilder $container) { - $container->getDefinition($this->getAlias().'.request_executor')->replaceArgument(4, $config['definitions']['show_debug_info']); + if ($config['definitions']['show_debug_info']) { + $definition = $container->setDefinition( + DebugListener::class, + new Definition(DebugListener::class) + ); + $definition->addTag('kernel.event_listener', ['event' => Events::PRE_EXECUTOR, 'method' => 'onPreExecutor']); + $definition->addTag('kernel.event_listener', ['event' => Events::POST_EXECUTOR, 'method' => 'onPostExecutor']); + } } private function setConfigBuilders(array $config, ContainerBuilder $container) diff --git a/EventListener/DebugListener.php b/EventListener/DebugListener.php new file mode 100644 index 000000000..7fc0157b4 --- /dev/null +++ b/EventListener/DebugListener.php @@ -0,0 +1,28 @@ +startTime = microtime(true); + $this->startMemoryUsage = memory_get_usage(true); + } + + public function onPostExecutor(ExecutorResultEvent $executorResultEvent) + { + $executorResultEvent->getResult()->extensions['debug'] = [ + 'executionTime' => sprintf('%d ms', round(microtime(true) - $this->startTime, 3) * 1000), + 'memoryUsage' => sprintf('%.2F MiB', (memory_get_usage(true) - $this->startMemoryUsage) / 1024 / 1024), + ]; + } +} diff --git a/Request/Executor.php b/Request/Executor.php index 36087bd00..73292b6da 100644 --- a/Request/Executor.php +++ b/Request/Executor.php @@ -3,6 +3,7 @@ namespace Overblog\GraphQLBundle\Request; use GraphQL\Executor\ExecutionResult; +use GraphQL\Executor\Promise\Promise; use GraphQL\Executor\Promise\PromiseAdapter; use GraphQL\Type\Schema; use GraphQL\Validator\DocumentValidator; @@ -34,9 +35,6 @@ class Executor /** @var ErrorHandler|null */ private $errorHandler; - /** @var bool */ - private $hasDebugInfo; - /** @var ExecutorInterface */ private $executor; @@ -51,7 +49,6 @@ public function __construct( EventDispatcherInterface $dispatcher, $throwException = false, ErrorHandler $errorHandler = null, - $hasDebugInfo = false, PromiseAdapter $promiseAdapter = null, callable $defaultFieldResolver = null ) { @@ -59,7 +56,6 @@ public function __construct( $this->dispatcher = $dispatcher; $this->throwException = (bool) $throwException; $this->errorHandler = $errorHandler; - $hasDebugInfo ? $this->enabledDebugInfo() : $this->disabledDebugInfo(); $this->promiseAdapter = $promiseAdapter; $this->defaultFieldResolver = $defaultFieldResolver; } @@ -114,25 +110,6 @@ public function getSchema($name = null) return $schema; } - public function enabledDebugInfo() - { - $this->hasDebugInfo = true; - - return $this; - } - - public function disabledDebugInfo() - { - $this->hasDebugInfo = false; - - return $this; - } - - public function hasDebugInfo() - { - return $this->hasDebugInfo; - } - public function setMaxQueryDepth($maxQueryDepth) { /** @var QueryDepth $queryDepth */ @@ -178,9 +155,6 @@ public function execute($schemaName, array $config, $rootValue = null, $contextV isset($config[ParserInterface::PARAM_OPERATION_NAME]) ? $config[ParserInterface::PARAM_OPERATION_NAME] : null ); - $startTime = microtime(true); - $startMemoryUsage = memory_get_usage(true); - $result = $this->executor->execute( $executorEvent->getSchema(), $executorEvent->getRequestString(), @@ -190,7 +164,7 @@ public function execute($schemaName, array $config, $rootValue = null, $contextV $executorEvent->getOperationName() ); - $result = $this->postExecute($executorEvent->getContextValue(), $result, $startTime, $startMemoryUsage); + $result = $this->postExecute($executorEvent->getContextValue(), $result); return $result; } @@ -223,21 +197,19 @@ private function preExecute(Schema $schema, $requestString, \ArrayObject $rootVa } /** - * @param \ArrayObject $contextValue - * @param ExecutionResult $result - * @param int $startTime - * @param int $startMemoryUsage + * @param \ArrayObject $contextValue + * @param ExecutionResult|Promise $result * * @return ExecutionResult */ - private function postExecute(\ArrayObject $contextValue, ExecutionResult $result, $startTime, $startMemoryUsage) + private function postExecute(\ArrayObject $contextValue, $result) { if ($this->promiseAdapter) { $result = $this->promiseAdapter->wait($result); } $this->checkExecutionResult($result); - $result = $this->prepareResult($result, $startTime, $startMemoryUsage); + $result = $this->prepareResult($result); $event = $this->dispatcher->dispatch( Events::POST_EXECUTOR, @@ -249,20 +221,11 @@ private function postExecute(\ArrayObject $contextValue, ExecutionResult $result /** * @param ExecutionResult $result - * @param int $startTime - * @param int $startMemoryUsage * * @return ExecutionResult */ - private function prepareResult($result, $startTime, $startMemoryUsage) + private function prepareResult($result) { - if ($this->hasDebugInfo()) { - $result->extensions['debug'] = [ - 'executionTime' => sprintf('%d ms', round(microtime(true) - $startTime, 3) * 1000), - 'memoryUsage' => sprintf('%.2F MiB', (memory_get_usage(true) - $startMemoryUsage) / 1024 / 1024), - ]; - } - if (null !== $this->errorHandler) { $this->errorHandler->handleErrors($result, $this->throwException); } diff --git a/Resources/config/services.yml b/Resources/config/services.yml index b0d6a640d..b90a8a8a4 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -23,7 +23,6 @@ services: - "@event_dispatcher" - "%kernel.debug%" - "@overblog_graphql.error_handler" - - false - "@overblog_graphql.promise_adapter" - "%overblog_graphql.default_resolver%" calls: diff --git a/Tests/Functional/App/config/debug/config.yml b/Tests/Functional/App/config/debug/config.yml new file mode 100644 index 000000000..dd7932985 --- /dev/null +++ b/Tests/Functional/App/config/debug/config.yml @@ -0,0 +1,7 @@ +imports: + - { resource: ../connection/config.yml } + +overblog_graphql: + definitions: + class_namespace: "Overblog\\GraphQLBundle\\Debug\\__DEFINITIONS__" + show_debug_info: true diff --git a/Tests/Functional/EventListener/DebugListenerTest.php b/Tests/Functional/EventListener/DebugListenerTest.php new file mode 100644 index 000000000..55a3796f3 --- /dev/null +++ b/Tests/Functional/EventListener/DebugListenerTest.php @@ -0,0 +1,26 @@ + 'connection']); + $response = $this->sendRequest($client, Introspection::getIntrospectionQuery(), true); + $this->assertArrayNotHasKey('extensions', $response); + } + + public function testEnabledDebugInfo() + { + $client = static::createClient(['test_case' => 'debug']); + $response = $this->sendRequest($client, Introspection::getIntrospectionQuery(), true); + $this->assertArrayHasKey('extensions', $response); + $this->assertArrayHasKey('debug', $response['extensions']); + $this->assertArrayHasKey('executionTime', $response['extensions']['debug']); + $this->assertArrayHasKey('memoryUsage', $response['extensions']['debug']); + } +} diff --git a/Tests/Functional/TestCase.php b/Tests/Functional/TestCase.php index 311c1ccb6..5f3e685a7 100644 --- a/Tests/Functional/TestCase.php +++ b/Tests/Functional/TestCase.php @@ -3,6 +3,7 @@ namespace Overblog\GraphQLBundle\Tests\Functional; use Overblog\GraphQLBundle\Tests\Functional\App\TestKernel; +use Symfony\Bundle\FrameworkBundle\Client; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\HttpFoundation\Request; @@ -119,12 +120,18 @@ protected static function createClientAuthenticated($username, $testCase, $passw protected static function assertResponse($query, array $expected, $username, $testCase, $password = self::DEFAULT_PASSWORD) { $client = self::createClientAuthenticated($username, $testCase, $password); - $client->request('GET', '/', ['query' => $query]); - - $result = $client->getResponse()->getContent(); + $result = self::sendRequest($client, $query); static::assertEquals($expected, json_decode($result, true), $result); return $client; } + + protected static function sendRequest(Client $client, $query, $isDecoded = false) + { + $client->request('GET', '/', ['query' => $query]); + $result = $client->getResponse()->getContent(); + + return $isDecoded ? json_decode($result, true) : $result; + } } diff --git a/Tests/Request/ExecutorTest.php b/Tests/Request/ExecutorTest.php index 494a85cba..8ad903f0b 100644 --- a/Tests/Request/ExecutorTest.php +++ b/Tests/Request/ExecutorTest.php @@ -71,20 +71,6 @@ public function testInvalidExecutorAdapterPromise() $this->executor->execute(null, $this->request); } - public function testDisabledDebugInfo() - { - $this->assertArrayNotHasKey('debug', $this->executor->disabledDebugInfo()->execute(null, $this->request)->extensions); - } - - public function testEnabledDebugInfo() - { - $result = $this->executor->enabledDebugInfo()->execute(null, $this->request); - - $this->assertArrayHasKey('debug', $result->extensions); - $this->assertArrayHasKey('executionTime', $result->extensions['debug']); - $this->assertArrayHasKey('memoryUsage', $result->extensions['debug']); - } - /** * @expectedException \RuntimeException * @expectedExceptionMessage At least one schema should be declare. From bbf2364d1dd19745bfd97ea470706428b79b9f18 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Wed, 22 Nov 2017 13:49:04 +0100 Subject: [PATCH 4/7] Refactor and add tests --- .../OverblogGraphQLTypesExtension.php | 2 +- Error/ErrorHandler.php | 2 +- Event/ExecutorResultEvent.php | 14 +--- EventListener/ErrorHandlerListener.php | 32 +++++++++ Request/Executor.php | 68 +++++-------------- Resources/config/services.yml | 15 ++-- Tests/Functional/App/config/config.yml | 3 + Tests/Functional/App/config/global/config.yml | 5 +- .../mapping/graphql/global.types.graphql | 14 ++++ .../global/mapping/yaml/decorators.types.yml | 23 +++++++ .../mapping/{ => yaml}/global.types.yml | 31 --------- Tests/Functional/TestCase.php | 5 +- 12 files changed, 108 insertions(+), 106 deletions(-) create mode 100644 EventListener/ErrorHandlerListener.php create mode 100644 Tests/Functional/App/config/global/mapping/graphql/global.types.graphql create mode 100644 Tests/Functional/App/config/global/mapping/yaml/decorators.types.yml rename Tests/Functional/App/config/global/mapping/{ => yaml}/global.types.yml (59%) diff --git a/DependencyInjection/OverblogGraphQLTypesExtension.php b/DependencyInjection/OverblogGraphQLTypesExtension.php index 1927f7e92..16a2189db 100644 --- a/DependencyInjection/OverblogGraphQLTypesExtension.php +++ b/DependencyInjection/OverblogGraphQLTypesExtension.php @@ -102,7 +102,7 @@ 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]; } diff --git a/Error/ErrorHandler.php b/Error/ErrorHandler.php index 6cc21ac30..a4f2144f2 100644 --- a/Error/ErrorHandler.php +++ b/Error/ErrorHandler.php @@ -156,7 +156,7 @@ public function logException($exception, $errorLevel = LogLevel::ERROR) public function handleErrors(ExecutionResult $executionResult, $throwRawException = false) { - $executionResult->setErrorFormatter(sprintf('\%s::formatError', GraphQLError::class)); + $executionResult->setErrorFormatter([GraphQLError::class, 'formatError']); $exceptions = $this->treatExceptions($executionResult->errors, $throwRawException); $executionResult->errors = $exceptions['errors']; if (!empty($exceptions['extensions']['warnings'])) { diff --git a/Event/ExecutorResultEvent.php b/Event/ExecutorResultEvent.php index acf408d2b..1e27d5400 100644 --- a/Event/ExecutorResultEvent.php +++ b/Event/ExecutorResultEvent.php @@ -10,13 +10,9 @@ class ExecutorResultEvent extends Event /** @var ExecutionResult */ private $result; - /** @var \ArrayObject */ - private $contextValue; - - public function __construct(ExecutionResult $result, \ArrayObject $contextValue) + public function __construct(ExecutionResult $result) { $this->result = $result; - $this->contextValue = $contextValue; } /** @@ -26,12 +22,4 @@ public function getResult() { return $this->result; } - - /** - * @return \ArrayObject - */ - public function getContextValue() - { - return $this->contextValue; - } } diff --git a/EventListener/ErrorHandlerListener.php b/EventListener/ErrorHandlerListener.php new file mode 100644 index 000000000..e5b52a0cd --- /dev/null +++ b/EventListener/ErrorHandlerListener.php @@ -0,0 +1,32 @@ +errorHandler = $errorHandler; + $this->throwException = $throwException; + } + + public function setThrowException($throwException) + { + $this->throwException = $throwException; + } + + public function onPostExecutor(ExecutorResultEvent $executorResultEvent) + { + $result = $executorResultEvent->getResult(); + $this->errorHandler->handleErrors($result, $this->throwException); + } +} diff --git a/Request/Executor.php b/Request/Executor.php index 73292b6da..9f06c96c6 100644 --- a/Request/Executor.php +++ b/Request/Executor.php @@ -9,7 +9,6 @@ use GraphQL\Validator\DocumentValidator; use GraphQL\Validator\Rules\QueryComplexity; use GraphQL\Validator\Rules\QueryDepth; -use Overblog\GraphQLBundle\Error\ErrorHandler; use Overblog\GraphQLBundle\Event\Events; use Overblog\GraphQLBundle\Event\ExecutorContextEvent; use Overblog\GraphQLBundle\Event\ExecutorEvent; @@ -29,12 +28,6 @@ class Executor /** @var EventDispatcherInterface|null */ private $dispatcher; - /** @var bool */ - private $throwException; - - /** @var ErrorHandler|null */ - private $errorHandler; - /** @var ExecutorInterface */ private $executor; @@ -47,15 +40,11 @@ class Executor public function __construct( ExecutorInterface $executor, EventDispatcherInterface $dispatcher, - $throwException = false, - ErrorHandler $errorHandler = null, PromiseAdapter $promiseAdapter = null, callable $defaultFieldResolver = null ) { $this->executor = $executor; $this->dispatcher = $dispatcher; - $this->throwException = (bool) $throwException; - $this->errorHandler = $errorHandler; $this->promiseAdapter = $promiseAdapter; $this->defaultFieldResolver = $defaultFieldResolver; } @@ -124,35 +113,23 @@ public function setMaxQueryComplexity($maxQueryComplexity) $queryComplexity->setMaxQueryComplexity($maxQueryComplexity); } - /** - * @param bool $throwException - * - * @return $this - */ - public function setThrowException($throwException) - { - $this->throwException = (bool) $throwException; - - return $this; - } - /** * @param null|string $schemaName - * @param array $config + * @param array $request * @param null|array|\ArrayObject|object $rootValue * @param null|array|\ArrayObject|object $contextValue * * @return ExecutionResult */ - public function execute($schemaName, array $config, $rootValue = null, $contextValue = null) + public function execute($schemaName, array $request, $rootValue = null, $contextValue = null) { $executorEvent = $this->preExecute( $this->getSchema($schemaName), - isset($config[ParserInterface::PARAM_QUERY]) ? $config[ParserInterface::PARAM_QUERY] : null, + isset($request[ParserInterface::PARAM_QUERY]) ? $request[ParserInterface::PARAM_QUERY] : null, self::createArrayObject($rootValue), self::createArrayObject($contextValue), - $config[ParserInterface::PARAM_VARIABLES], - isset($config[ParserInterface::PARAM_OPERATION_NAME]) ? $config[ParserInterface::PARAM_OPERATION_NAME] : null + $request[ParserInterface::PARAM_VARIABLES], + isset($request[ParserInterface::PARAM_OPERATION_NAME]) ? $request[ParserInterface::PARAM_OPERATION_NAME] : null ); $result = $this->executor->execute( @@ -164,7 +141,7 @@ public function execute($schemaName, array $config, $rootValue = null, $contextV $executorEvent->getOperationName() ); - $result = $this->postExecute($executorEvent->getContextValue(), $result); + $result = $this->postExecute($result); return $result; } @@ -179,8 +156,13 @@ public function execute($schemaName, array $config, $rootValue = null, $contextV * * @return ExecutorEvent */ - private function preExecute(Schema $schema, $requestString, \ArrayObject $rootValue, \ArrayObject $contextValue, array $variableValue = null, $operationName = null) - { + private function preExecute( + Schema $schema, $requestString, + \ArrayObject $rootValue, + \ArrayObject $contextValue, + array $variableValue = null, + $operationName = null + ) { $this->checkPromiseAdapter(); $this->executor->setPromiseAdapter($this->promiseAdapter); @@ -197,42 +179,26 @@ private function preExecute(Schema $schema, $requestString, \ArrayObject $rootVa } /** - * @param \ArrayObject $contextValue * @param ExecutionResult|Promise $result * * @return ExecutionResult */ - private function postExecute(\ArrayObject $contextValue, $result) + private function postExecute($result) { if ($this->promiseAdapter) { $result = $this->promiseAdapter->wait($result); } $this->checkExecutionResult($result); - $result = $this->prepareResult($result); $event = $this->dispatcher->dispatch( Events::POST_EXECUTOR, - new ExecutorResultEvent($result, $contextValue) + new ExecutorResultEvent($result) ); return $event->getResult(); } - /** - * @param ExecutionResult $result - * - * @return ExecutionResult - */ - private function prepareResult($result) - { - if (null !== $this->errorHandler) { - $this->errorHandler->handleErrors($result, $this->throwException); - } - - return $result; - } - private function checkPromiseAdapter() { if ($this->promiseAdapter && !$this->promiseAdapter instanceof PromiseAdapterInterface && !is_callable([$this->promiseAdapter, 'wait'])) { @@ -257,9 +223,7 @@ private function checkExecutionResult($result) private static function createArrayObject($data) { - if ($data instanceof \ArrayObject) { - $object = $data; - } elseif (is_array($data) || is_object($data)) { + if (is_array($data) || is_object($data)) { $object = new \ArrayObject($data); } else { $object = new \ArrayObject(); diff --git a/Resources/config/services.yml b/Resources/config/services.yml index b90a8a8a4..a0804aee0 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -11,6 +11,15 @@ services: - [] - false + Overblog\GraphQLBundle\EventListener\ErrorHandlerListener: + class: Overblog\GraphQLBundle\EventListener\ErrorHandlerListener + public: false + arguments: + - "@overblog_graphql.error_handler" + - "%kernel.debug%" + tags: + - { name: kernel.event_listener, event: graphql.post_executor, method: onPostExecutor } + overblog_graphql.executor.default: class: Overblog\GraphQLBundle\Executor\Executor public: false @@ -21,8 +30,6 @@ services: arguments: - "@overblog_graphql.executor" - "@event_dispatcher" - - "%kernel.debug%" - - "@overblog_graphql.error_handler" - "@overblog_graphql.promise_adapter" - "%overblog_graphql.default_resolver%" calls: @@ -103,9 +110,9 @@ services: - ["addImplement", ["Overblog\\GraphQLBundle\\Definition\\Type\\GeneratedTypeInterface"]] - ["setExpressionLanguage", ["@overblog_graphql.expression_language"]] - overblog_graphql.event_listener.request_file_listener: + Overblog\GraphQLBundle\EventListener\RequestFilesListener: class: Overblog\GraphQLBundle\EventListener\RequestFilesListener - public: true + public: false arguments: - "@request_stack" tags: diff --git a/Tests/Functional/App/config/config.yml b/Tests/Functional/App/config/config.yml index d3d1ad89a..932e6083c 100644 --- a/Tests/Functional/App/config/config.yml +++ b/Tests/Functional/App/config/config.yml @@ -22,3 +22,6 @@ services: #disable twig error pages twig.exception_listener: class: stdClass + error_handler_listener: + public: true + alias: Overblog\GraphQLBundle\EventListener\ErrorHandlerListener diff --git a/Tests/Functional/App/config/global/config.yml b/Tests/Functional/App/config/global/config.yml index d2b0671da..c7f13be1b 100644 --- a/Tests/Functional/App/config/global/config.yml +++ b/Tests/Functional/App/config/global/config.yml @@ -19,4 +19,7 @@ overblog_graphql: types: - type: yaml - dir: "%kernel.root_dir%/config/global/mapping" + dir: "%kernel.root_dir%/config/global/mapping/yaml" + - + type: graphql + dir: "%kernel.root_dir%/config/global/mapping/graphql" diff --git a/Tests/Functional/App/config/global/mapping/graphql/global.types.graphql b/Tests/Functional/App/config/global/mapping/graphql/global.types.graphql new file mode 100644 index 000000000..c1e5464d4 --- /dev/null +++ b/Tests/Functional/App/config/global/mapping/graphql/global.types.graphql @@ -0,0 +1,14 @@ +type Photo implements NodeInterface { + # The ID of an object + id: ID! + width: Int +} + +type Post implements NodeInterface { + # The ID of an object + id: ID! + text: String + status: Status +} + +union PhotoAndPost = Photo | Post diff --git a/Tests/Functional/App/config/global/mapping/yaml/decorators.types.yml b/Tests/Functional/App/config/global/mapping/yaml/decorators.types.yml new file mode 100644 index 000000000..2f53ab1dd --- /dev/null +++ b/Tests/Functional/App/config/global/mapping/yaml/decorators.types.yml @@ -0,0 +1,23 @@ +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)' diff --git a/Tests/Functional/App/config/global/mapping/global.types.yml b/Tests/Functional/App/config/global/mapping/yaml/global.types.yml similarity index 59% rename from Tests/Functional/App/config/global/mapping/global.types.yml rename to Tests/Functional/App/config/global/mapping/yaml/global.types.yml index 372b34ec1..c5a82c048 100644 --- a/Tests/Functional/App/config/global/mapping/global.types.yml +++ b/Tests/Functional/App/config/global/mapping/yaml/global.types.yml @@ -25,37 +25,6 @@ User: 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: diff --git a/Tests/Functional/TestCase.php b/Tests/Functional/TestCase.php index 5f3e685a7..a8d28c5fa 100644 --- a/Tests/Functional/TestCase.php +++ b/Tests/Functional/TestCase.php @@ -66,9 +66,8 @@ protected static function executeGraphQLRequest($query, $rootValue = [], $throwE $request->query->set('query', $query); $req = static::getContainer()->get('overblog_graphql.request_parser')->parse($request); - $executor = static::getContainer()->get('overblog_graphql.request_executor'); - $executor->setThrowException($throwException); - $res = $executor->execute(null, $req, $rootValue); + static::getContainer()->get('error_handler_listener')->setThrowException($throwException); + $res = static::getContainer()->get('overblog_graphql.request_executor')->execute(null, $req, $rootValue); return $res->toArray(); } From f9c1d34eae99263a7890d535b4cf6e94b8f965a0 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Wed, 29 Nov 2017 09:16:23 +0100 Subject: [PATCH 5/7] Allow multiple types for config files --- DependencyInjection/Configuration.php | 84 +++++++++++-------- .../OverblogGraphQLTypesExtension.php | 31 +++---- .../doc/definitions/type-system/index.md | 5 +- .../OverblogGraphQLTypesExtensionTest.php | 2 +- Tests/Functional/App/config/global/config.yml | 7 +- .../mapping/{yaml => }/decorators.types.yml | 7 ++ .../{graphql => }/global.types.graphql | 13 ++- .../mapping/{yaml => }/global.types.yml | 22 +---- 8 files changed, 97 insertions(+), 74 deletions(-) rename Tests/Functional/App/config/global/mapping/{yaml => }/decorators.types.yml (82%) rename Tests/Functional/App/config/global/mapping/{graphql => }/global.types.graphql (63%) rename Tests/Functional/App/config/global/mapping/{yaml => }/global.types.yml (73%) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 62a8dc9e6..480dca660 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -84,40 +84,7 @@ public function getConfigTreeBuilder() ->end() ->end() ->end() - ->arrayNode('mappings') - ->children() - ->arrayNode('auto_discover') - ->treatFalseLike(['bundles' => false, 'root_dir' => false]) - ->treatTrueLike(['bundles' => true, 'root_dir' => true]) - ->treatNullLike(['bundles' => true, 'root_dir' => true]) - ->addDefaultsIfNotSet() - ->children() - ->booleanNode('bundles')->defaultTrue()->end() - ->booleanNode('root_dir')->defaultTrue()->end() - ->end() - ->end() - ->arrayNode('types') - ->prototype('array') - ->addDefaultsIfNotSet() - ->beforeNormalization() - ->ifTrue(function ($v) { - return isset($v['type']) && 'yml' === $v['type']; - }) - ->then(function ($v) { - $v['type'] = 'yaml'; - - return $v; - }) - ->end() - ->children() - ->enumNode('type')->values(array_keys(OverblogGraphQLTypesExtension::SUPPORTED_TYPES_EXTENSIONS))->defaultNull()->end() - ->scalarNode('dir')->defaultNull()->end() - ->scalarNode('suffix')->defaultValue(OverblogGraphQLTypesExtension::DEFAULT_TYPES_SUFFIX)->end() - ->end() - ->end() - ->end() - ->end() - ->end() + ->append($this->addMappingSection()) ->booleanNode('map_exceptions_to_parent')->defaultFalse()->end() ->arrayNode('exceptions') ->addDefaultsIfNotSet() @@ -183,6 +150,55 @@ public function getConfigTreeBuilder() return $treeBuilder; } + private function addMappingSection() + { + $builder = new TreeBuilder(); + $node = $builder->root('mappings'); + $node + ->children() + ->arrayNode('auto_discover') + ->treatFalseLike(['bundles' => false, 'root_dir' => false]) + ->treatTrueLike(['bundles' => true, 'root_dir' => true]) + ->treatNullLike(['bundles' => true, 'root_dir' => true]) + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('bundles')->defaultTrue()->end() + ->booleanNode('root_dir')->defaultTrue()->end() + ->end() + ->end() + ->arrayNode('types') + ->prototype('array') + ->addDefaultsIfNotSet() + ->beforeNormalization() + ->ifTrue(function ($v) { + return isset($v['type']) && is_string($v['type']); + }) + ->then(function ($v) { + if ('yml' === $v['type']) { + $v['types'] = ['yaml']; + } else { + $v['types'] = [$v['type']]; + } + unset($v['type']); + + return $v; + }) + ->end() + ->children() + ->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() + ; + + return $node; + } + /** * @param string $name * diff --git a/DependencyInjection/OverblogGraphQLTypesExtension.php b/DependencyInjection/OverblogGraphQLTypesExtension.php index 16a2189db..3c46a047e 100644 --- a/DependencyInjection/OverblogGraphQLTypesExtension.php +++ b/DependencyInjection/OverblogGraphQLTypesExtension.php @@ -55,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); @@ -104,7 +105,7 @@ private function mappingConfig(array $config, ContainerBuilder $container) // 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); @@ -113,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'], ]; } @@ -127,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; }, @@ -150,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; @@ -165,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 ? array_keys(self::SUPPORTED_TYPES_EXTENSIONS) : [$type]; + $types = $stopOnFirstTypeMatching ? array_keys(self::SUPPORTED_TYPES_EXTENSIONS) : $types; + $files = []; foreach ($types as $type) { + $finder = Finder::create(); try { $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/Resources/doc/definitions/type-system/index.md b/Resources/doc/definitions/type-system/index.md index 103266cbe..32e3a72bf 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 in the same dir + dir: "%kernel.root_dir%/.../mapping" ``` 2. **The PHP way** diff --git a/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php b/Tests/DependencyInjection/OverblogGraphQLTypesExtensionTest.php index 9665f7c51..c3af46f21 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 c7f13be1b..0dcfc19b5 100644 --- a/Tests/Functional/App/config/global/config.yml +++ b/Tests/Functional/App/config/global/config.yml @@ -18,8 +18,5 @@ overblog_graphql: mappings: types: - - type: yaml - dir: "%kernel.root_dir%/config/global/mapping/yaml" - - - type: graphql - dir: "%kernel.root_dir%/config/global/mapping/graphql" + types: [yaml, graphql] + dir: "%kernel.root_dir%/config/global/mapping" diff --git a/Tests/Functional/App/config/global/mapping/yaml/decorators.types.yml b/Tests/Functional/App/config/global/mapping/decorators.types.yml similarity index 82% rename from Tests/Functional/App/config/global/mapping/yaml/decorators.types.yml rename to Tests/Functional/App/config/global/mapping/decorators.types.yml index 2f53ab1dd..b39dbce6d 100644 --- a/Tests/Functional/App/config/global/mapping/yaml/decorators.types.yml +++ b/Tests/Functional/App/config/global/mapping/decorators.types.yml @@ -21,3 +21,10 @@ PhotoAndPostDecorator: 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/graphql/global.types.graphql b/Tests/Functional/App/config/global/mapping/global.types.graphql similarity index 63% rename from Tests/Functional/App/config/global/mapping/graphql/global.types.graphql rename to Tests/Functional/App/config/global/mapping/global.types.graphql index c1e5464d4..46bf335de 100644 --- a/Tests/Functional/App/config/global/mapping/graphql/global.types.graphql +++ b/Tests/Functional/App/config/global/mapping/global.types.graphql @@ -1,9 +1,16 @@ + 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! @@ -11,4 +18,8 @@ type Post implements NodeInterface { status: Status } -union PhotoAndPost = Photo | Post +type User implements NodeInterface { + # The ID of an object + id: ID! + name: String +} diff --git a/Tests/Functional/App/config/global/mapping/yaml/global.types.yml b/Tests/Functional/App/config/global/mapping/global.types.yml similarity index 73% rename from Tests/Functional/App/config/global/mapping/yaml/global.types.yml rename to Tests/Functional/App/config/global/mapping/global.types.yml index c5a82c048..f12808c31 100644 --- a/Tests/Functional/App/config/global/mapping/yaml/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,20 +11,10 @@ Query: type: '[NodeInterface]' resolve: '@=service("overblog_graphql.test.resolver.global").resolveAllObjects()' -User: - type: object - config: - fields: - id: "Relay::GlobalId" - name: - type: String - interfaces: [NodeInterface] - -PhotoInput: - type: input-object +NodeInterface: + type: relay-node config: - fields: - width: { type: Int, defaultValue: 100 } + resolveType: '@=service("overblog_graphql.test.resolver.global").typeResolver(value)' Status: type: enum @@ -40,3 +25,4 @@ Status: STANDBY: value: 3 description: Waiting for validation + From 604971ae1232a0c9a1840809e13a96590609e0bb Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 30 Nov 2017 08:49:39 +0100 Subject: [PATCH 6/7] Add documentation --- README.md | 1 + .../definitions/graphql-schema-language.md | 70 +++++++++++++++++++ Resources/doc/definitions/index.md | 2 + .../doc/definitions/type-system/index.md | 2 +- 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 Resources/doc/definitions/graphql-schema-language.md diff --git a/README.md b/README.md index 6b149e9ab..d0d5fe26d 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 32e3a72bf..b03f7b7f4 100644 --- a/Resources/doc/definitions/type-system/index.md +++ b/Resources/doc/definitions/type-system/index.md @@ -32,7 +32,7 @@ Types can be define 3 different ways: 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 in the same dir + types: [yaml, graphql] # to include different types from the same dir dir: "%kernel.root_dir%/.../mapping" ``` From a596585caf43e6246e9676683c47c61425942c50 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 30 Nov 2017 09:01:29 +0100 Subject: [PATCH 7/7] Fix Listeners visibility --- DependencyInjection/OverblogGraphQLExtension.php | 1 + Resources/config/services.yml | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index 0036dcb7c..bda34d9b6 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -89,6 +89,7 @@ private function setClassLoaderListener(array $config, ContainerBuilder $contain $this->getAlias().'.event_listener.classloader_listener', new Definition(ClassLoaderListener::class) ); + $definition->setPublic(true); $definition->setArguments([new Reference($this->getAlias().'.cache_compiler')]); $definition->addTag('kernel.event_listener', ['event' => 'kernel.request', 'method' => 'load', 'priority' => 255]); $definition->addTag('kernel.event_listener', ['event' => 'console.command', 'method' => 'load', 'priority' => 255]); diff --git a/Resources/config/services.yml b/Resources/config/services.yml index a0804aee0..69b882a9a 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -13,7 +13,7 @@ services: Overblog\GraphQLBundle\EventListener\ErrorHandlerListener: class: Overblog\GraphQLBundle\EventListener\ErrorHandlerListener - public: false + public: true arguments: - "@overblog_graphql.error_handler" - "%kernel.debug%" @@ -112,7 +112,7 @@ services: Overblog\GraphQLBundle\EventListener\RequestFilesListener: class: Overblog\GraphQLBundle\EventListener\RequestFilesListener - public: false + public: true arguments: - "@request_stack" tags: