From c20f02e98fdaf1ee6920530ce82ffd27194a87aa Mon Sep 17 00:00:00 2001 From: David Omid Date: Fri, 20 Mar 2026 13:24:27 +0000 Subject: [PATCH 1/4] fix: check interface implementation in potentiallySameObject for nullability relaxation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When relax_subgraph_operation_field_selection_merging_nullability is enabled, field selection merging should allow nullability differences between types that cannot overlap at runtime. The potentiallySameObject function previously treated any interface+object pair as potentially overlapping, which prevented nullability relaxation even when the object type does not implement the interface. Split the single interface case into three: - both interfaces → conservatively assume overlap - interface + object → overlap only if the object implements the interface - two objects → overlap only if same type name --- .../operation_rule_field_selection_merging.go | 12 +- .../operation_validation_test.go | 158 ++++++++++++------ 2 files changed, 115 insertions(+), 55 deletions(-) diff --git a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go index 04dad67ecc..3ecd0d6d58 100644 --- a/v2/pkg/astvalidation/operation_rule_field_selection_merging.go +++ b/v2/pkg/astvalidation/operation_rule_field_selection_merging.go @@ -217,14 +217,20 @@ func (f *fieldSelectionMergingVisitor) EnterField(ref int) { // to the same runtime object. This determines whether field merging must enforce // strict type equality (including nullability) or may relax it. // -// - If either type is an interface, returns true (conservative: any concrete -// type might implement that interface). +// - If one type is an interface and the other is an object, returns true only +// when the object type implements the interface (otherwise they cannot overlap). +// - If both types are interfaces, returns true (conservative: some concrete +// type might implement both). // - Two object types overlap only when they share the same name. // - All other combinations return false. func (f *fieldSelectionMergingVisitor) potentiallySameObject(left, right ast.Node) bool { switch { - case left.Kind == ast.NodeKindInterfaceTypeDefinition || right.Kind == ast.NodeKindInterfaceTypeDefinition: + case left.Kind == ast.NodeKindInterfaceTypeDefinition && right.Kind == ast.NodeKindInterfaceTypeDefinition: return true + case left.Kind == ast.NodeKindInterfaceTypeDefinition && right.Kind == ast.NodeKindObjectTypeDefinition: + return f.definition.NodeImplementsInterfaceNode(right, left) + case left.Kind == ast.NodeKindObjectTypeDefinition && right.Kind == ast.NodeKindInterfaceTypeDefinition: + return f.definition.NodeImplementsInterfaceNode(left, right) case left.Kind == ast.NodeKindObjectTypeDefinition && right.Kind == ast.NodeKindObjectTypeDefinition: return bytes.Equal(f.definition.ObjectTypeDefinitionNameBytes(left.Ref), f.definition.ObjectTypeDefinitionNameBytes(right.Ref)) default: diff --git a/v2/pkg/astvalidation/operation_validation_test.go b/v2/pkg/astvalidation/operation_validation_test.go index 74783bae26..b1daed7ff6 100644 --- a/v2/pkg/astvalidation/operation_validation_test.go +++ b/v2/pkg/astvalidation/operation_validation_test.go @@ -173,7 +173,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`operation name must be unique: getName`)) }) t.Run("94", func(t *testing.T) { - run(t, ` + run(t, ` query dogOperation { dog { name @@ -245,7 +245,7 @@ func TestExecutionValidation(t *testing.T) { SubscriptionSingleRootField(), Valid) }) t.Run("97 variant", func(t *testing.T) { - run(t, ` + run(t, ` query sub { foo bar @@ -253,7 +253,7 @@ func TestExecutionValidation(t *testing.T) { SubscriptionSingleRootField(), Valid) }) t.Run("97 variant", func(t *testing.T) { - run(t, ` + run(t, ` subscription sub { ... { foo } ... { bar } @@ -274,7 +274,7 @@ func TestExecutionValidation(t *testing.T) { SubscriptionSingleRootField(), Valid) }) t.Run("99", func(t *testing.T) { - run(t, ` + run(t, ` subscription sub { newMessage { body @@ -286,7 +286,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`subscription: sub must only have one root selection`)) }) t.Run("100", func(t *testing.T) { - run(t, ` + run(t, ` subscription sub { ...multipleSubscriptions } @@ -1115,19 +1115,33 @@ func TestExecutionValidation(t *testing.T) { }`, FieldSelectionMerging(), Invalid, withValidationErrors(`differing types 'Profile!' and 'Profile' for objectName 'profile'`)) }) - t.Run("disallows differing return type nullability when interface could overlap", func(t *testing.T) { - runWithDefinition(t, boxDefinition, ` + t.Run("allows differing scalar nullability on interface vs non implementing object type with relaxation", func(t *testing.T) { + runWithDefinition(t, interfaceNonImplementorDefinition, ` { - someBox { - ... on NonNullStringBox1 { - scalar - } - ... on StringBox { - scalar - } + item { + ... on Printable { title } + ... on Podcast { title } + } + }`, FieldSelectionMerging(true), Valid) + }) + t.Run("rejects differing scalar nullability on interface vs non implementing object type without relaxation", func(t *testing.T) { + runWithDefinition(t, interfaceNonImplementorDefinition, ` + { + item { + ... on Printable { title } + ... on Podcast { title } } }`, FieldSelectionMerging(), Invalid, - withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) + withValidationErrors(`fields 'title' conflict because they return conflicting types 'String!' and 'String'`)) + }) + t.Run("allows matching types on interface vs implementing object type with relaxation", func(t *testing.T) { + runWithDefinition(t, interfaceNonImplementorDefinition, ` + { + item { + ... on Printable { title } + ... on Book { title } + } + }`, FieldSelectionMerging(true), Valid) }) t.Run("same wrapped scalar return types", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` @@ -1200,7 +1214,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("107 variant", func(t *testing.T) { - run(t, ` + run(t, ` query mergeIdenticalFields { dog { name @@ -1225,7 +1239,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { name: nickname @@ -1265,7 +1279,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extra { string } @@ -1275,7 +1289,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extra { string } @@ -1306,7 +1320,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing types '[DogExtra]' and '[DogExtra]!' for objectName 'extras'`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { x: extras { string } @@ -1317,7 +1331,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing types '[DogExtra]' and '[DogExtra]!' for objectName 'x'`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { string,string2: string } @@ -1327,7 +1341,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { string,string2: string } @@ -1337,7 +1351,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { string,string2: string2 } @@ -1348,7 +1362,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'string2' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { ... { string },string2: string } @@ -1368,7 +1382,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'string' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { ...frag, ...frag } @@ -1397,7 +1411,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'string1' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { ...frag } @@ -1518,7 +1532,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("109 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: 1) doesKnowCommand(dogCommand: 0) @@ -1595,7 +1609,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: {foo: "bar"}) doesKnowCommand(dogCommand: {foo: "bar"}) @@ -1603,7 +1617,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("109 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: {foo: "bar"}) doesKnowCommand(dogCommand: {bar: "bar"}) @@ -1661,7 +1675,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("111", func(t *testing.T) { - run(t, ` + run(t, ` fragment safeDifferingFields on Pet { ... on Dog { volume: barkVolume @@ -1694,7 +1708,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`fields 'someValue' conflict because they return conflicting types 'String!' and 'Int'`)) }) t.Run("112 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment conflictingDifferingResponses on Pet { ... on Dog { extra { @@ -1974,7 +1988,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Invalid, withExpectNormalizationError()) }) t.Run("112 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingDifferingResponses { catOrDog { ...catDogFrag @@ -2036,7 +2050,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("112 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment conflictingDifferingResponses on Pet { ...dogFrag ... on Cat { @@ -2097,7 +2111,7 @@ func TestExecutionValidation(t *testing.T) { extra { ... on CatExtra { value: bool } ... on DogExtra { value: bool } - } + } }`, FieldSelectionMerging(), Invalid, withValidationErrors(`fields 'value' conflict because they return conflicting types 'Boolean' and 'Int'`)) @@ -2150,7 +2164,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelections(), Invalid, withExpectNormalizationError()) }) t.Run("116", func(t *testing.T) { - run(t, ` + run(t, ` query directQueryOnObjectWithoutSubFields { human }`, @@ -2182,7 +2196,7 @@ func TestExecutionValidation(t *testing.T) { t.Run("5.4 Arguments", func(t *testing.T) { t.Run("5.4.1 Argument Names", func(t *testing.T) { t.Run("117", func(t *testing.T) { - run(t, ` + run(t, ` fragment argOnRequiredArg on Dog { doesKnowCommand(dogCommand: SIT) } @@ -2216,7 +2230,7 @@ func TestExecutionValidation(t *testing.T) { KnownArguments(), Valid) }) t.Run("117 variant", func(t *testing.T) { - run(t, ` + run(t, ` query argOnRequiredArg($dogCommand: DogCommand = SIT) { dog { doesKnowCommand(dogCommand: $dogCommand) @@ -2261,7 +2275,7 @@ func TestExecutionValidation(t *testing.T) { Values(), Valid) }) t.Run("117 variant", func(t *testing.T) { - run(t, ` + run(t, ` query argOnRequiredArg($booleanArg: Boolean!) { dog { ...argOnOptional @@ -2273,7 +2287,7 @@ func TestExecutionValidation(t *testing.T) { Values(), Valid) }) t.Run("117 variant", func(t *testing.T) { - run(t, ` + run(t, ` query argOnRequiredArg($booleanArg: Boolean) { dog { ...argOnOptional @@ -2337,7 +2351,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`Variable "$intArg" of type "Integer" used in position expecting type "Boolean".`)) }) t.Run("118", func(t *testing.T) { - run(t, ` + run(t, ` { dog { ...invalidArgName} } @@ -2348,7 +2362,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`Unknown argument "command" on field "Dog.doesKnowCommand"`)) }) t.Run("118 variant", func(t *testing.T) { - run(t, ` + run(t, ` { dog { ...invalidArgName} } @@ -2380,7 +2394,7 @@ func TestExecutionValidation(t *testing.T) { }) t.Run("undefined arg", func(t *testing.T) { run(t, ` { - dog(name: "Goofy"){ + dog(name: "Goofy"){ name } }`, @@ -2561,7 +2575,7 @@ func TestExecutionValidation(t *testing.T) { Fragments(), Valid) }) t.Run("127", func(t *testing.T) { - run(t, ` + run(t, ` { dog { ...fragmentOne @@ -2604,13 +2618,13 @@ func TestExecutionValidation(t *testing.T) { }`, Fragments(), Valid) }) t.Run("129", func(t *testing.T) { - run(t, ` + run(t, ` fragment notOnExistingType on NotInSchema { name }`, Fragments(), Invalid, withExpectNormalizationError()) }) t.Run("129", func(t *testing.T) { - run(t, ` + run(t, ` fragment inlineNotExistingType on Dog { ... on NotInSchema { name @@ -3052,7 +3066,7 @@ func TestExecutionValidation(t *testing.T) { ` { titles { - ... on Name { + ... on Name { ... on A { name } @@ -4561,7 +4575,7 @@ func TestValidationEdgeCases(t *testing.T) { __typename status message - } + } } }`, false, )) @@ -4571,7 +4585,7 @@ func TestValidationEdgeCases(t *testing.T) { scalar _Any scalar String union _Entity = User - + extend type Query { _entities(representations: [_Any!]!): [_Entity]! } @@ -4586,8 +4600,8 @@ func TestValidationEdgeCases(t *testing.T) { ` query($representations: [_Any!]!) { _entities(representations: $representations) { - ... on User { - name + ... on User { + name } } }`, true, @@ -5734,6 +5748,46 @@ schema { query: Query }` +// interfaceNonImplementorDefinition models a union whose members include both +// types that implement an interface (Printable) and a type (Podcast) that does +// NOT implement it. This lets us test nullability relaxation when one inline +// fragment targets an interface and the other targets a non implementing object +// type. +const interfaceNonImplementorDefinition = ` +scalar String +scalar Int +scalar ID + +interface Printable { + title: String! + name: String! +} + +type Book implements Printable { + title: String! + name: String! +} + +type Magazine implements Printable { + title: String! + name: String! +} + +type Podcast { + title: String + name: String! +} + +union SearchResult = Book | Magazine | Podcast + +type Query { + item: SearchResult +} + +schema { + query: Query +}` + const countriesDefinition = `directive @cacheControl(maxAge: Int, scope: CacheControlScope) on FIELD_DEFINITION | OBJECT | INTERFACE scalar String @@ -6261,7 +6315,7 @@ input UpdateUserInput { } """ The @cache directive caches the response server side and sets cache control headers according to the configuration. -With this setting you can reduce the load on your backend systems for operations that get hit a lot while data doesn't change that frequently. +With this setting you can reduce the load on your backend systems for operations that get hit a lot while data doesn't change that frequently. """ directive @cache( """maxAge defines the maximum time in seconds a response will be understood 'fresh', defaults to 300 (5 minutes)""" @@ -6269,7 +6323,7 @@ directive @cache( """ vary defines the headers to append to the cache key In addition to all possible headers you can also select a custom claim for authenticated requests - Examples: 'jwt.sub', 'jwt.team' to vary the cache key based on 'sub' or 'team' fields on the jwt. + Examples: 'jwt.sub', 'jwt.team' to vary the cache key based on 'sub' or 'team' fields on the jwt. """ vary: [String]! = [] ) on QUERY From 7a19b95e2f09647f9829fc4df8f34b44f557cad0 Mon Sep 17 00:00:00 2001 From: David Omid Date: Fri, 20 Mar 2026 17:30:03 +0000 Subject: [PATCH 2/4] Improved tests and simplified test schema --- .../operation_validation_test.go | 158 ++++++------------ 1 file changed, 52 insertions(+), 106 deletions(-) diff --git a/v2/pkg/astvalidation/operation_validation_test.go b/v2/pkg/astvalidation/operation_validation_test.go index b1daed7ff6..74783bae26 100644 --- a/v2/pkg/astvalidation/operation_validation_test.go +++ b/v2/pkg/astvalidation/operation_validation_test.go @@ -173,7 +173,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`operation name must be unique: getName`)) }) t.Run("94", func(t *testing.T) { - run(t, ` + run(t, ` query dogOperation { dog { name @@ -245,7 +245,7 @@ func TestExecutionValidation(t *testing.T) { SubscriptionSingleRootField(), Valid) }) t.Run("97 variant", func(t *testing.T) { - run(t, ` + run(t, ` query sub { foo bar @@ -253,7 +253,7 @@ func TestExecutionValidation(t *testing.T) { SubscriptionSingleRootField(), Valid) }) t.Run("97 variant", func(t *testing.T) { - run(t, ` + run(t, ` subscription sub { ... { foo } ... { bar } @@ -274,7 +274,7 @@ func TestExecutionValidation(t *testing.T) { SubscriptionSingleRootField(), Valid) }) t.Run("99", func(t *testing.T) { - run(t, ` + run(t, ` subscription sub { newMessage { body @@ -286,7 +286,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`subscription: sub must only have one root selection`)) }) t.Run("100", func(t *testing.T) { - run(t, ` + run(t, ` subscription sub { ...multipleSubscriptions } @@ -1115,33 +1115,19 @@ func TestExecutionValidation(t *testing.T) { }`, FieldSelectionMerging(), Invalid, withValidationErrors(`differing types 'Profile!' and 'Profile' for objectName 'profile'`)) }) - t.Run("allows differing scalar nullability on interface vs non implementing object type with relaxation", func(t *testing.T) { - runWithDefinition(t, interfaceNonImplementorDefinition, ` - { - item { - ... on Printable { title } - ... on Podcast { title } - } - }`, FieldSelectionMerging(true), Valid) - }) - t.Run("rejects differing scalar nullability on interface vs non implementing object type without relaxation", func(t *testing.T) { - runWithDefinition(t, interfaceNonImplementorDefinition, ` + t.Run("disallows differing return type nullability when interface could overlap", func(t *testing.T) { + runWithDefinition(t, boxDefinition, ` { - item { - ... on Printable { title } - ... on Podcast { title } + someBox { + ... on NonNullStringBox1 { + scalar + } + ... on StringBox { + scalar + } } }`, FieldSelectionMerging(), Invalid, - withValidationErrors(`fields 'title' conflict because they return conflicting types 'String!' and 'String'`)) - }) - t.Run("allows matching types on interface vs implementing object type with relaxation", func(t *testing.T) { - runWithDefinition(t, interfaceNonImplementorDefinition, ` - { - item { - ... on Printable { title } - ... on Book { title } - } - }`, FieldSelectionMerging(true), Valid) + withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) }) t.Run("same wrapped scalar return types", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` @@ -1214,7 +1200,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("107 variant", func(t *testing.T) { - run(t, ` + run(t, ` query mergeIdenticalFields { dog { name @@ -1239,7 +1225,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'name' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { name: nickname @@ -1279,7 +1265,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extra { string } @@ -1289,7 +1275,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extra { string } @@ -1320,7 +1306,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing types '[DogExtra]' and '[DogExtra]!' for objectName 'extras'`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { x: extras { string } @@ -1331,7 +1317,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing types '[DogExtra]' and '[DogExtra]!' for objectName 'x'`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { string,string2: string } @@ -1341,7 +1327,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { string,string2: string } @@ -1351,7 +1337,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { string,string2: string2 } @@ -1362,7 +1348,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'string2' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { ... { string },string2: string } @@ -1382,7 +1368,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'string' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { ...frag, ...frag } @@ -1411,7 +1397,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'string1' on (potentially) same type`)) }) t.Run("108 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingBecauseAlias { dog { extras { ...frag } @@ -1532,7 +1518,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("109 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: 1) doesKnowCommand(dogCommand: 0) @@ -1609,7 +1595,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("109 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: {foo: "bar"}) doesKnowCommand(dogCommand: {foo: "bar"}) @@ -1617,7 +1603,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("109 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment mergeIdenticalFieldsWithIdenticalValues on Dog { doesKnowCommand(dogCommand: {foo: "bar"}) doesKnowCommand(dogCommand: {bar: "bar"}) @@ -1675,7 +1661,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`differing fields for objectName 'doesKnowCommand' on (potentially) same type`)) }) t.Run("111", func(t *testing.T) { - run(t, ` + run(t, ` fragment safeDifferingFields on Pet { ... on Dog { volume: barkVolume @@ -1708,7 +1694,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`fields 'someValue' conflict because they return conflicting types 'String!' and 'Int'`)) }) t.Run("112 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment conflictingDifferingResponses on Pet { ... on Dog { extra { @@ -1988,7 +1974,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Invalid, withExpectNormalizationError()) }) t.Run("112 variant", func(t *testing.T) { - run(t, ` + run(t, ` query conflictingDifferingResponses { catOrDog { ...catDogFrag @@ -2050,7 +2036,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelectionMerging(), Valid) }) t.Run("112 variant", func(t *testing.T) { - run(t, ` + run(t, ` fragment conflictingDifferingResponses on Pet { ...dogFrag ... on Cat { @@ -2111,7 +2097,7 @@ func TestExecutionValidation(t *testing.T) { extra { ... on CatExtra { value: bool } ... on DogExtra { value: bool } - } + } }`, FieldSelectionMerging(), Invalid, withValidationErrors(`fields 'value' conflict because they return conflicting types 'Boolean' and 'Int'`)) @@ -2164,7 +2150,7 @@ func TestExecutionValidation(t *testing.T) { FieldSelections(), Invalid, withExpectNormalizationError()) }) t.Run("116", func(t *testing.T) { - run(t, ` + run(t, ` query directQueryOnObjectWithoutSubFields { human }`, @@ -2196,7 +2182,7 @@ func TestExecutionValidation(t *testing.T) { t.Run("5.4 Arguments", func(t *testing.T) { t.Run("5.4.1 Argument Names", func(t *testing.T) { t.Run("117", func(t *testing.T) { - run(t, ` + run(t, ` fragment argOnRequiredArg on Dog { doesKnowCommand(dogCommand: SIT) } @@ -2230,7 +2216,7 @@ func TestExecutionValidation(t *testing.T) { KnownArguments(), Valid) }) t.Run("117 variant", func(t *testing.T) { - run(t, ` + run(t, ` query argOnRequiredArg($dogCommand: DogCommand = SIT) { dog { doesKnowCommand(dogCommand: $dogCommand) @@ -2275,7 +2261,7 @@ func TestExecutionValidation(t *testing.T) { Values(), Valid) }) t.Run("117 variant", func(t *testing.T) { - run(t, ` + run(t, ` query argOnRequiredArg($booleanArg: Boolean!) { dog { ...argOnOptional @@ -2287,7 +2273,7 @@ func TestExecutionValidation(t *testing.T) { Values(), Valid) }) t.Run("117 variant", func(t *testing.T) { - run(t, ` + run(t, ` query argOnRequiredArg($booleanArg: Boolean) { dog { ...argOnOptional @@ -2351,7 +2337,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`Variable "$intArg" of type "Integer" used in position expecting type "Boolean".`)) }) t.Run("118", func(t *testing.T) { - run(t, ` + run(t, ` { dog { ...invalidArgName} } @@ -2362,7 +2348,7 @@ func TestExecutionValidation(t *testing.T) { withValidationErrors(`Unknown argument "command" on field "Dog.doesKnowCommand"`)) }) t.Run("118 variant", func(t *testing.T) { - run(t, ` + run(t, ` { dog { ...invalidArgName} } @@ -2394,7 +2380,7 @@ func TestExecutionValidation(t *testing.T) { }) t.Run("undefined arg", func(t *testing.T) { run(t, ` { - dog(name: "Goofy"){ + dog(name: "Goofy"){ name } }`, @@ -2575,7 +2561,7 @@ func TestExecutionValidation(t *testing.T) { Fragments(), Valid) }) t.Run("127", func(t *testing.T) { - run(t, ` + run(t, ` { dog { ...fragmentOne @@ -2618,13 +2604,13 @@ func TestExecutionValidation(t *testing.T) { }`, Fragments(), Valid) }) t.Run("129", func(t *testing.T) { - run(t, ` + run(t, ` fragment notOnExistingType on NotInSchema { name }`, Fragments(), Invalid, withExpectNormalizationError()) }) t.Run("129", func(t *testing.T) { - run(t, ` + run(t, ` fragment inlineNotExistingType on Dog { ... on NotInSchema { name @@ -3066,7 +3052,7 @@ func TestExecutionValidation(t *testing.T) { ` { titles { - ... on Name { + ... on Name { ... on A { name } @@ -4575,7 +4561,7 @@ func TestValidationEdgeCases(t *testing.T) { __typename status message - } + } } }`, false, )) @@ -4585,7 +4571,7 @@ func TestValidationEdgeCases(t *testing.T) { scalar _Any scalar String union _Entity = User - + extend type Query { _entities(representations: [_Any!]!): [_Entity]! } @@ -4600,8 +4586,8 @@ func TestValidationEdgeCases(t *testing.T) { ` query($representations: [_Any!]!) { _entities(representations: $representations) { - ... on User { - name + ... on User { + name } } }`, true, @@ -5748,46 +5734,6 @@ schema { query: Query }` -// interfaceNonImplementorDefinition models a union whose members include both -// types that implement an interface (Printable) and a type (Podcast) that does -// NOT implement it. This lets us test nullability relaxation when one inline -// fragment targets an interface and the other targets a non implementing object -// type. -const interfaceNonImplementorDefinition = ` -scalar String -scalar Int -scalar ID - -interface Printable { - title: String! - name: String! -} - -type Book implements Printable { - title: String! - name: String! -} - -type Magazine implements Printable { - title: String! - name: String! -} - -type Podcast { - title: String - name: String! -} - -union SearchResult = Book | Magazine | Podcast - -type Query { - item: SearchResult -} - -schema { - query: Query -}` - const countriesDefinition = `directive @cacheControl(maxAge: Int, scope: CacheControlScope) on FIELD_DEFINITION | OBJECT | INTERFACE scalar String @@ -6315,7 +6261,7 @@ input UpdateUserInput { } """ The @cache directive caches the response server side and sets cache control headers according to the configuration. -With this setting you can reduce the load on your backend systems for operations that get hit a lot while data doesn't change that frequently. +With this setting you can reduce the load on your backend systems for operations that get hit a lot while data doesn't change that frequently. """ directive @cache( """maxAge defines the maximum time in seconds a response will be understood 'fresh', defaults to 300 (5 minutes)""" @@ -6323,7 +6269,7 @@ directive @cache( """ vary defines the headers to append to the cache key In addition to all possible headers you can also select a custom claim for authenticated requests - Examples: 'jwt.sub', 'jwt.team' to vary the cache key based on 'sub' or 'team' fields on the jwt. + Examples: 'jwt.sub', 'jwt.team' to vary the cache key based on 'sub' or 'team' fields on the jwt. """ vary: [String]! = [] ) on QUERY From 300bbd8307bb318eb624d59ee3ebd8ce943da55b Mon Sep 17 00:00:00 2001 From: David Omid Date: Fri, 20 Mar 2026 17:41:58 +0000 Subject: [PATCH 3/4] Improved tests --- .../operation_validation_test.go | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/v2/pkg/astvalidation/operation_validation_test.go b/v2/pkg/astvalidation/operation_validation_test.go index 74783bae26..6bee376c19 100644 --- a/v2/pkg/astvalidation/operation_validation_test.go +++ b/v2/pkg/astvalidation/operation_validation_test.go @@ -1129,6 +1129,35 @@ func TestExecutionValidation(t *testing.T) { }`, FieldSelectionMerging(), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) }) + t.Run("allows differing scalar nullability on interface vs non implementing object type with relaxation", func(t *testing.T) { + runWithDefinition(t, interfaceNonImplementorDefinition, ` + { + item { + ... on Printable { title } + ... on Podcast { title } + } + }`, FieldSelectionMerging(true), Valid) + }) + t.Run("rejects differing scalar nullability on interface vs non implementing object type without relaxation", func(t *testing.T) { + runWithDefinition(t, interfaceNonImplementorDefinition, ` + { + item { + ... on Printable { title } + ... on Podcast { title } + } + }`, FieldSelectionMerging(), Invalid, + withValidationErrors(`fields 'title' conflict because they return conflicting types 'String!' and 'String'`)) + }) + t.Run("rejects differing nullability on interface vs implementing object type even with relaxation", func(t *testing.T) { + runWithDefinition(t, interfaceNonImplementorDefinition, ` + { + item { + ... on Printable { name } + ... on Book { name } + } + }`, FieldSelectionMerging(true), Invalid, + withValidationErrors(`fields 'name' conflict because they return conflicting types 'String' and 'String!'`)) + }) t.Run("same wrapped scalar return types", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` { @@ -5734,6 +5763,40 @@ schema { query: Query }` +// interfaceNonImplementorDefinition models a union whose members include both +// a type that implements an interface (Printable) and a type (Podcast) that does +// NOT implement it. This lets us test nullability relaxation when one inline +// fragment targets an interface and the other targets a non implementing object +// type. +const interfaceNonImplementorDefinition = ` +scalar String +scalar Int +scalar ID + +interface Printable { + title: String! + name: String +} + +type Book implements Printable { + title: String! + name: String! +} + +type Podcast { + title: String +} + +union SearchResult = Book | Podcast + +type Query { + item: SearchResult +} + +schema { + query: Query +}` + const countriesDefinition = `directive @cacheControl(maxAge: Int, scope: CacheControlScope) on FIELD_DEFINITION | OBJECT | INTERFACE scalar String From 650abfe0d6ae242012cae1fd8d27724e46d187f7 Mon Sep 17 00:00:00 2001 From: David Omid Date: Fri, 20 Mar 2026 17:45:59 +0000 Subject: [PATCH 4/4] Removed test schema, simplified tests --- .../operation_validation_test.go | 74 +++++-------------- 1 file changed, 19 insertions(+), 55 deletions(-) diff --git a/v2/pkg/astvalidation/operation_validation_test.go b/v2/pkg/astvalidation/operation_validation_test.go index 6bee376c19..bdf401ee7e 100644 --- a/v2/pkg/astvalidation/operation_validation_test.go +++ b/v2/pkg/astvalidation/operation_validation_test.go @@ -1129,34 +1129,32 @@ func TestExecutionValidation(t *testing.T) { }`, FieldSelectionMerging(), Invalid, withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String!' and 'String'`)) }) - t.Run("allows differing scalar nullability on interface vs non implementing object type with relaxation", func(t *testing.T) { - runWithDefinition(t, interfaceNonImplementorDefinition, ` + t.Run("allows differing return type nullability on interface vs non implementing type with relaxation", func(t *testing.T) { + runWithDefinition(t, boxDefinition, ` { - item { - ... on Printable { title } - ... on Podcast { title } + someBox { + ... on NonNullStringBox1 { + scalar + } + ... on StringBox { + scalar + } } }`, FieldSelectionMerging(true), Valid) }) - t.Run("rejects differing scalar nullability on interface vs non implementing object type without relaxation", func(t *testing.T) { - runWithDefinition(t, interfaceNonImplementorDefinition, ` - { - item { - ... on Printable { title } - ... on Podcast { title } - } - }`, FieldSelectionMerging(), Invalid, - withValidationErrors(`fields 'title' conflict because they return conflicting types 'String!' and 'String'`)) - }) - t.Run("rejects differing nullability on interface vs implementing object type even with relaxation", func(t *testing.T) { - runWithDefinition(t, interfaceNonImplementorDefinition, ` + t.Run("rejects differing return type nullability on interface vs implementing type even with relaxation", func(t *testing.T) { + runWithDefinition(t, boxDefinition, ` { - item { - ... on Printable { name } - ... on Book { name } + someBox { + ... on SomeBox { + scalar + } + ... on NonNullStringBox1Impl { + scalar + } } }`, FieldSelectionMerging(true), Invalid, - withValidationErrors(`fields 'name' conflict because they return conflicting types 'String' and 'String!'`)) + withValidationErrors(`fields 'scalar' conflict because they return conflicting types 'String' and 'String!'`)) }) t.Run("same wrapped scalar return types", func(t *testing.T) { runWithDefinition(t, boxDefinition, ` @@ -5763,40 +5761,6 @@ schema { query: Query }` -// interfaceNonImplementorDefinition models a union whose members include both -// a type that implements an interface (Printable) and a type (Podcast) that does -// NOT implement it. This lets us test nullability relaxation when one inline -// fragment targets an interface and the other targets a non implementing object -// type. -const interfaceNonImplementorDefinition = ` -scalar String -scalar Int -scalar ID - -interface Printable { - title: String! - name: String -} - -type Book implements Printable { - title: String! - name: String! -} - -type Podcast { - title: String -} - -union SearchResult = Book | Podcast - -type Query { - item: SearchResult -} - -schema { - query: Query -}` - const countriesDefinition = `directive @cacheControl(maxAge: Int, scope: CacheControlScope) on FIELD_DEFINITION | OBJECT | INTERFACE scalar String