From 38d6b4b60394842750111711df9d6a97e0d03a49 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 23 Apr 2021 10:48:31 +0200 Subject: [PATCH 1/2] Fix overly eager validation of repeatable directive usage --- src/Type/SchemaValidationContext.php | 21 ++++--- tests/Type/ValidationTest.php | 86 ++++++++++++++++++---------- 2 files changed, 70 insertions(+), 37 deletions(-) diff --git a/src/Type/SchemaValidationContext.php b/src/Type/SchemaValidationContext.php index 7c5704393..2df40d111 100644 --- a/src/Type/SchemaValidationContext.php +++ b/src/Type/SchemaValidationContext.php @@ -372,8 +372,9 @@ public function validateTypes() : void */ private function validateDirectivesAtLocation($directives, string $location) { - $directivesNamed = []; - $schema = $this->schema; + /** @var array> $potentiallyDuplicateDirectives */ + $potentiallyDuplicateDirectives = []; + $schema = $this->schema; foreach ($directives as $directive) { $directiveName = $directive->name->value; @@ -386,6 +387,7 @@ private function validateDirectivesAtLocation($directives, string $location) ); continue; } + $includes = Utils::some( $schemaDirective->locations, static function ($schemaLocation) use ($location) : bool { @@ -402,17 +404,22 @@ static function ($schemaLocation) use ($location) : bool { ); } - $existingNodes = $directivesNamed[$directiveName] ?? []; - $existingNodes[] = $directive; - $directivesNamed[$directiveName] = $existingNodes; + if ($schemaDirective->isRepeatable) { + continue; + } + + $existingNodes = $potentiallyDuplicateDirectives[$directiveName] ?? []; + $existingNodes[] = $directive; + $potentiallyDuplicateDirectives[$directiveName] = $existingNodes; } - foreach ($directivesNamed as $directiveName => $directiveList) { + + foreach ($potentiallyDuplicateDirectives as $directiveName => $directiveList) { if (count($directiveList) <= 1) { continue; } $this->reportError( - sprintf('Directive @%s used twice at the same location.', $directiveName), + sprintf('Non-repeatable directive @%s used more than once at the same location.', $directiveName), $directiveList ); } diff --git a/tests/Type/ValidationTest.php b/tests/Type/ValidationTest.php index 4290c4526..0049f33f2 100644 --- a/tests/Type/ValidationTest.php +++ b/tests/Type/ValidationTest.php @@ -3078,19 +3078,19 @@ public function testRejectsASchemaWithSameSchemaDirectiveUsedTwice() type Query implements SomeInterface @object @object { test(arg: SomeInput @argument_definition @argument_definition): String } - + interface SomeInterface @interface @interface { test: String @field_definition @field_definition } - + union SomeUnion @union @union = Query - + scalar SomeScalar @scalar @scalar - + enum SomeEnum @enum @enum { SOME_VALUE @enum_value @enum_value } - + input SomeInput @input_object @input_object { some_input_field: String @input_field_definition @input_field_definition } @@ -3099,43 +3099,68 @@ enum SomeEnum @enum @enum { $schema->validate(), [ [ - 'message' => 'Directive @schema used twice at the same location.', + 'message' => 'Non-repeatable directive @schema used more than once at the same location.', 'locations' => [[ 'line' => 14, 'column' => 18 ], [ 'line' => 14, 'column' => 26 ]], - ],[ - 'message' => 'Directive @argument_definition used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @argument_definition used more than once at the same location.', 'locations' => [[ 'line' => 19, 'column' => 33 ], [ 'line' => 19, 'column' => 54 ]], - ],[ - 'message' => 'Directive @object used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @object used more than once at the same location.', 'locations' => [[ 'line' => 18, 'column' => 47 ], [ 'line' => 18, 'column' => 55 ]], - ],[ - 'message' => 'Directive @field_definition used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @field_definition used more than once at the same location.', 'locations' => [[ 'line' => 23, 'column' => 26 ], [ 'line' => 23, 'column' => 44 ]], - ],[ - 'message' => 'Directive @interface used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @interface used more than once at the same location.', 'locations' => [[ 'line' => 22, 'column' => 35 ], [ 'line' => 22, 'column' => 46 ]], - ],[ - 'message' => 'Directive @input_field_definition used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @input_field_definition used more than once at the same location.', 'locations' => [[ 'line' => 35, 'column' => 38 ], [ 'line' => 35, 'column' => 62 ]], - ],[ - 'message' => 'Directive @input_object used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @input_object used more than once at the same location.', 'locations' => [[ 'line' => 34, 'column' => 27 ], [ 'line' => 34, 'column' => 41 ]], - ],[ - 'message' => 'Directive @union used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @union used more than once at the same location.', 'locations' => [[ 'line' => 26, 'column' => 27 ], [ 'line' => 26, 'column' => 34 ]], - ],[ - 'message' => 'Directive @scalar used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @scalar used more than once at the same location.', 'locations' => [[ 'line' => 28, 'column' => 29 ], [ 'line' => 28, 'column' => 37 ]], - ],[ - 'message' => 'Directive @enum_value used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @enum_value used more than once at the same location.', 'locations' => [[ 'line' => 31, 'column' => 24 ], [ 'line' => 31, 'column' => 36 ]], - ],[ - 'message' => 'Directive @enum used twice at the same location.', + ], + [ + 'message' => 'Non-repeatable directive @enum used more than once at the same location.', 'locations' => [[ 'line' => 30, 'column' => 25 ], [ 'line' => 30, 'column' => 31 ]], ], ] ); } + public function testAllowsRepeatableDirectivesMultipleTimesAtTheSameLocation() : void + { + $schema = BuildSchema::build(' + directive @repeatable repeatable on OBJECT + + type Query @repeatable @repeatable { + foo: ID + } + ', null, ['assumeValid' => true]); + $this->assertMatchesValidationMessage( + $schema->validate(), + [] + ); + } + /** * @see it('rejects a Schema with directive used again in extension') */ @@ -3157,10 +3182,11 @@ public function testRejectsASchemaWithSameDefinitionDirectiveUsedTwice() $this->assertMatchesValidationMessage( $extendedSchema->validate(), - [[ - 'message' => 'Directive @testA used twice at the same location.', - 'locations' => [[ 'line' => 4, 'column' => 22 ], [ 'line' => 2, 'column' => 29 ]], - ], + [ + [ + 'message' => 'Non-repeatable directive @testA used more than once at the same location.', + 'locations' => [[ 'line' => 4, 'column' => 22 ], [ 'line' => 2, 'column' => 29 ]], + ], ] ); } From 2632960adf31418b834487c419009e36aa84d37d Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 23 Apr 2021 10:50:12 +0200 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ba165eb6..ae4c7a388 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ #### Unreleased +#### 14.6.2 + +Fix: +- Fix overly eager validation of repeatable directive usage + #### 14.6.1 Fix: