From 9fc5e9c75484d91551325f6e12cacb81da6d47a5 Mon Sep 17 00:00:00 2001 From: Vincent Date: Sun, 12 Jul 2020 17:58:34 +0200 Subject: [PATCH 1/5] Add schema name as Executor argument & fix default schema - Add schemaName in the ExecutorArgumentsEvent - Make the GraphQL collector retrieve schema from ExecutorArgument instead of routing - Consider default schema as the one with name 'default' or the first one defined - Apply same logic to Annotation Parser - Update doc --- docs/annotations/annotations-reference.md | 4 ++-- docs/definitions/schema.md | 4 ++++ src/Config/Parser/AnnotationParser.php | 5 +++-- src/DataCollector/GraphQLCollector.php | 7 ++++++- src/Event/ExecutorArgumentsEvent.php | 13 +++++++++++++ src/Request/Executor.php | 16 ++++++++++------ src/Resources/views/profiler/graphql.html.twig | 2 +- tests/DataCollector/GraphQLCollectorTest.php | 7 +++---- 8 files changed, 42 insertions(+), 16 deletions(-) diff --git a/docs/annotations/annotations-reference.md b/docs/annotations/annotations-reference.md index fcc0b1675..882a6e5ae 100644 --- a/docs/annotations/annotations-reference.md +++ b/docs/annotations/annotations-reference.md @@ -386,7 +386,7 @@ The class exposing the mutation(s) must be declared as a [service](https://symfo Optional attributes: -- **targetType** : The GraphQL type to attach the field to. It must be a mutation. (by default, it'll be the root Mutation type of the default schema). +- **targetType** : The GraphQL type to attach the field to. It must be a mutation. (by default, it'll be the root Mutation type of the default schema. see [Default Schema](../definitions/schema.md#default-schema)). Example: @@ -436,7 +436,7 @@ The class exposing the query(ies) must be declared as a [service](https://symfon Optional attributes: -- **targetType** : The GraphQL type to attach the field to (by default, it'll be the root Query type of the default schema). +- **targetType** : The GraphQL type to attach the field to (by default, it'll be the root Query type of the default schema. see [Default Schema](../definitions/schema.md#default-schema)). Example: diff --git a/docs/definitions/schema.md b/docs/definitions/schema.md index 09b282911..55cc6a99d 100644 --- a/docs/definitions/schema.md +++ b/docs/definitions/schema.md @@ -111,4 +111,8 @@ overblog_graphql: | batch request | `/graphql/bar/batch` | | GraphiQL* | `/graphiql/bar` | +### Default schema + +The schema considered as the default is the one with the name `default` if it exists, otherwise, it will be the first one defined. + \* `/graphiql` depends on [OverblogGraphiQLBundle](https://github.com/overblog/GraphiQLBundle) diff --git a/src/Config/Parser/AnnotationParser.php b/src/Config/Parser/AnnotationParser.php index e2543e050..d20bc7fee 100644 --- a/src/Config/Parser/AnnotationParser.php +++ b/src/Config/Parser/AnnotationParser.php @@ -253,19 +253,20 @@ private static function typeAnnotationToGQLConfiguration( ): array { $isMutation = $isDefault = $isRoot = false; if (isset($configs['definitions']['schema'])) { + $defaultSchemaName = isset($configs['definitions']['schema']['default']) ? 'default' : array_key_first($configs['definitions']['schema']); foreach ($configs['definitions']['schema'] as $schemaName => $schema) { $schemaQuery = $schema['query'] ?? null; $schemaMutation = $schema['mutation'] ?? null; if ($schemaQuery && $gqlName === $schemaQuery) { $isRoot = true; - if ('default' == $schemaName) { + if ($defaultSchemaName == $schemaName) { $isDefault = true; } } elseif ($schemaMutation && $gqlName === $schemaMutation) { $isMutation = true; $isRoot = true; - if ('default' == $schemaName) { + if ($defaultSchemaName == $schemaName) { $isDefault = true; } } diff --git a/src/DataCollector/GraphQLCollector.php b/src/DataCollector/GraphQLCollector.php index 1597d210f..f6121559c 100644 --- a/src/DataCollector/GraphQLCollector.php +++ b/src/DataCollector/GraphQLCollector.php @@ -28,7 +28,11 @@ public function collect(Request $request, Response $response, Throwable $excepti { $error = false; $count = 0; + $schema = false; foreach ($this->batches as $batch) { + if (!$schema) { + $schema = $batch['schema']; + } if (isset($batch['error'])) { $error = true; } @@ -36,7 +40,7 @@ public function collect(Request $request, Response $response, Throwable $excepti } $this->data = [ - 'schema' => $request->attributes->get('_route_params')['schemaName'] ?? 'default', + 'schema' => $schema, 'batches' => $this->batches, 'count' => $count, 'error' => $error, @@ -105,6 +109,7 @@ public function onPostExecutor(ExecutorResultEvent $event): void $result = $event->getResult()->toArray(); $batch = [ + 'schema' => $executorArgument->getSchemaName(), 'queryString' => $queryString, 'queryTime' => $queryTime, 'variables' => $this->cloneVar($variables), diff --git a/src/Event/ExecutorArgumentsEvent.php b/src/Event/ExecutorArgumentsEvent.php index 95edbf3bc..c527c8099 100644 --- a/src/Event/ExecutorArgumentsEvent.php +++ b/src/Event/ExecutorArgumentsEvent.php @@ -11,6 +11,7 @@ final class ExecutorArgumentsEvent extends Event { + private string $schemaName; private ExtensibleSchema $schema; private string $requestString; private ArrayObject $contextValue; @@ -27,6 +28,7 @@ final class ExecutorArgumentsEvent extends Event * @return static */ public static function create( + string $schemaName, ExtensibleSchema $schema, string $requestString, ArrayObject $contextValue, @@ -35,6 +37,7 @@ public static function create( string $operationName = null ): self { $instance = new static(); + $instance->setSchemaName($schemaName); $instance->setSchema($schema); $instance->setRequestString($requestString); $instance->setContextValue($contextValue); @@ -46,6 +49,11 @@ public static function create( return $instance; } + public function setSchemaName(string $schemaName): void + { + $this->schemaName = $schemaName; + } + public function setOperationName(?string $operationName): void { $this->operationName = $operationName; @@ -84,6 +92,11 @@ public function setStartTime(float $startTime): void $this->startTime = $startTime; } + public function getSchemaName(): string + { + return $this->schemaName; + } + public function getSchema(): ExtensibleSchema { return $this->schema; diff --git a/src/Request/Executor.php b/src/Request/Executor.php index 79562a550..cde74c5f3 100644 --- a/src/Request/Executor.php +++ b/src/Request/Executor.php @@ -84,14 +84,13 @@ public function getSchema(string $name = null): Schema } if (null === $name) { - // TODO(mcg-web): Replace by array_key_first PHP 7 >= 7.3.0. - foreach ($this->schemas as $name => $schema) { - break; - } + $name = isset($this->schemas['default']) ? 'default' : array_key_first($this->schemas); } + if (!isset($this->schemas[$name])) { throw new NotFoundHttpException(sprintf('Could not found "%s" schema.', $name)); } + $schema = $this->schemas[$name]; if (is_callable($schema)) { $schema = $schema(); @@ -137,8 +136,12 @@ public function execute(?string $schemaName, array $request, $rootValue = null): { $this->useExperimentalExecutor ? GraphQL::useExperimentalExecutor() : GraphQL::useReferenceExecutor(); + $schema = $this->getSchema($schemaName); + $schemaName = array_search($schema, $this->schemas); + $executorArgumentsEvent = $this->preExecute( - $this->getSchema($schemaName), + $schemaName, + $schema, $request[ParserInterface::PARAM_QUERY] ?? null, new ArrayObject(), $rootValue, @@ -168,6 +171,7 @@ public function execute(?string $schemaName, array $request, $rootValue = null): * @param mixed $rootValue */ private function preExecute( + string $schemaName, Schema $schema, string $requestString, ArrayObject $contextValue, @@ -182,7 +186,7 @@ private function preExecute( // @phpstan-ignore-next-line (only for Symfony 4.4) $object = $this->dispatcher->dispatch( /** @var ExtensibleSchema $schema */ - ExecutorArgumentsEvent::create($schema, $requestString, $contextValue, $rootValue, $variableValue, $operationName), + ExecutorArgumentsEvent::create($schemaName, $schema, $requestString, $contextValue, $rootValue, $variableValue, $operationName), Events::PRE_EXECUTOR ); diff --git a/src/Resources/views/profiler/graphql.html.twig b/src/Resources/views/profiler/graphql.html.twig index 3496091ad..bfbfda37c 100644 --- a/src/Resources/views/profiler/graphql.html.twig +++ b/src/Resources/views/profiler/graphql.html.twig @@ -114,7 +114,7 @@ {{ result.status_code|default('n/a') }} {{ result.time|date }} - {% if schemas|length > 0 %} + {% if schemas|length > 0 and graphql.schema %} schema: {{ graphql.schema }} {% endif %} {{ result.token }} diff --git a/tests/DataCollector/GraphQLCollectorTest.php b/tests/DataCollector/GraphQLCollectorTest.php index fd91149ed..75d3f8fe6 100644 --- a/tests/DataCollector/GraphQLCollectorTest.php +++ b/tests/DataCollector/GraphQLCollectorTest.php @@ -22,21 +22,20 @@ public function testCollect(): void $collector = new GraphQLCollector(); $request = new Request(); - $request->attributes->set('_route_params', ['schemaName' => 'myschema']); $collector->onPostExecutor(new ExecutorResultEvent( new ExecutionResult(['res' => 'ok', 'error' => 'my error']), - ExecutorArgumentsEvent::create(new ExtensibleSchema([]), 'invalid', new ArrayObject()) + ExecutorArgumentsEvent::create('test_schema', new ExtensibleSchema([]), 'invalid', new ArrayObject()) )); $collector->onPostExecutor(new ExecutorResultEvent( new ExecutionResult(['res' => 'ok', 'error' => 'my error']), - ExecutorArgumentsEvent::create(new ExtensibleSchema([]), 'query{ myalias: test{field1, field2} }', new ArrayObject(), null, ['variable1' => 'v1']) + ExecutorArgumentsEvent::create('test_schema', new ExtensibleSchema([]), 'query{ myalias: test{field1, field2} }', new ArrayObject(), null, ['variable1' => 'v1']) )); $collector->collect($request, new Response()); - $this->assertEquals($collector->getSchema(), 'myschema'); + $this->assertEquals($collector->getSchema(), 'test_schema'); $this->assertEquals($collector->getName(), 'graphql'); $this->assertEquals($collector->getCount(), 1); $this->assertTrue($collector->getError()); From c560c4785e7be91685846c44f8c7db3125eae8a5 Mon Sep 17 00:00:00 2001 From: Vincent Date: Sun, 12 Jul 2020 18:32:55 +0200 Subject: [PATCH 2/5] Fix phpstan Not sure why /** @var ExtensibleSchema $schema */ ExecutorArgumentsEvent::create($schemaName, $schema, $requestString, $contextValue, $rootValue, $variableValue, $operationName), was not accepted anymore to type hint $schema --- src/Request/Executor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Request/Executor.php b/src/Request/Executor.php index cde74c5f3..ae056df0f 100644 --- a/src/Request/Executor.php +++ b/src/Request/Executor.php @@ -14,7 +14,6 @@ use GraphQL\Validator\Rules\DisableIntrospection; use GraphQL\Validator\Rules\QueryComplexity; use GraphQL\Validator\Rules\QueryDepth; -use Overblog\GraphQLBundle\Definition\Type\ExtensibleSchema; use Overblog\GraphQLBundle\Event\Events; use Overblog\GraphQLBundle\Event\ExecutorArgumentsEvent; use Overblog\GraphQLBundle\Event\ExecutorContextEvent; @@ -137,6 +136,7 @@ public function execute(?string $schemaName, array $request, $rootValue = null): $this->useExperimentalExecutor ? GraphQL::useExperimentalExecutor() : GraphQL::useReferenceExecutor(); $schema = $this->getSchema($schemaName); + /** @var string $schemaName */ $schemaName = array_search($schema, $this->schemas); $executorArgumentsEvent = $this->preExecute( @@ -185,7 +185,7 @@ private function preExecute( /** @var ExecutorArgumentsEvent $object */ // @phpstan-ignore-next-line (only for Symfony 4.4) $object = $this->dispatcher->dispatch( - /** @var ExtensibleSchema $schema */ + // @phpstan-ignore-next-line ExecutorArgumentsEvent::create($schemaName, $schema, $requestString, $contextValue, $rootValue, $variableValue, $operationName), Events::PRE_EXECUTOR ); From 5837e200034cc1e98ce5ba8fcce0195d2b578fcb Mon Sep 17 00:00:00 2001 From: Vincent Date: Sun, 12 Jul 2020 20:08:23 +0200 Subject: [PATCH 3/5] Fix GraphQL typehint --- src/DataCollector/GraphQLCollector.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/DataCollector/GraphQLCollector.php b/src/DataCollector/GraphQLCollector.php index f6121559c..a675f1114 100644 --- a/src/DataCollector/GraphQLCollector.php +++ b/src/DataCollector/GraphQLCollector.php @@ -65,8 +65,10 @@ public function getCount(): int /** * Return the targeted schema. + * + * @return string|false */ - public function getSchema(): string + public function getSchema() { return $this->data['schema'] ?? 'default'; } From 2823faed6fa12fc5c18cffaa639e98ee5fc94340 Mon Sep 17 00:00:00 2001 From: Vincent Date: Mon, 13 Jul 2020 16:43:03 +0200 Subject: [PATCH 4/5] Fix review --- src/Config/Parser/AnnotationParser.php | 8 ++++---- src/DataCollector/GraphQLCollector.php | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Config/Parser/AnnotationParser.php b/src/Config/Parser/AnnotationParser.php index d20bc7fee..21bfb0ec1 100644 --- a/src/Config/Parser/AnnotationParser.php +++ b/src/Config/Parser/AnnotationParser.php @@ -258,15 +258,15 @@ private static function typeAnnotationToGQLConfiguration( $schemaQuery = $schema['query'] ?? null; $schemaMutation = $schema['mutation'] ?? null; - if ($schemaQuery && $gqlName === $schemaQuery) { + if ($gqlName === $schemaQuery) { $isRoot = true; - if ($defaultSchemaName == $schemaName) { + if ($defaultSchemaName === $schemaName) { $isDefault = true; } - } elseif ($schemaMutation && $gqlName === $schemaMutation) { + } elseif ($gqlName === $schemaMutation) { $isMutation = true; $isRoot = true; - if ($defaultSchemaName == $schemaName) { + if ($defaultSchemaName === $schemaName) { $isDefault = true; } } diff --git a/src/DataCollector/GraphQLCollector.php b/src/DataCollector/GraphQLCollector.php index a675f1114..d8b999a61 100644 --- a/src/DataCollector/GraphQLCollector.php +++ b/src/DataCollector/GraphQLCollector.php @@ -28,7 +28,7 @@ public function collect(Request $request, Response $response, Throwable $excepti { $error = false; $count = 0; - $schema = false; + $schema = null; foreach ($this->batches as $batch) { if (!$schema) { $schema = $batch['schema']; @@ -68,9 +68,9 @@ public function getCount(): int * * @return string|false */ - public function getSchema() + public function getSchema(): ?string { - return $this->data['schema'] ?? 'default'; + return $this->data['schema']; } /** From 34ae23e90f4d91e5acd59374a7875b546b3a7a7d Mon Sep 17 00:00:00 2001 From: Vincent Date: Mon, 13 Jul 2020 16:59:02 +0200 Subject: [PATCH 5/5] Remove useless docblock --- src/DataCollector/GraphQLCollector.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/DataCollector/GraphQLCollector.php b/src/DataCollector/GraphQLCollector.php index d8b999a61..8fd8fa29e 100644 --- a/src/DataCollector/GraphQLCollector.php +++ b/src/DataCollector/GraphQLCollector.php @@ -65,8 +65,6 @@ public function getCount(): int /** * Return the targeted schema. - * - * @return string|false */ public function getSchema(): ?string {