From 632a0977fcfe7e9ffefc621605a0d1c0fa55d9fa Mon Sep 17 00:00:00 2001 From: endigma Date: Tue, 14 Jul 2026 13:37:40 +0100 Subject: [PATCH 1/9] test: abstract type validation --- .../engine/abstract_type_validation_test.go | 261 ++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 execution/engine/abstract_type_validation_test.go diff --git a/execution/engine/abstract_type_validation_test.go b/execution/engine/abstract_type_validation_test.go new file mode 100644 index 0000000000..1a3cc3185b --- /dev/null +++ b/execution/engine/abstract_type_validation_test.go @@ -0,0 +1,261 @@ +package engine + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/wundergraph/graphql-go-tools/execution/graphql" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasource/graphql_datasource" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" +) + +func TestAbstractTypeValidation(t *testing.T) { + t.Parallel() + + schema, err := graphql.NewSchemaFromString(abstractTypeValidationSDL) + require.NoError(t, err) + + tests := []struct { + name string + fieldName string + selection string + returnedTypeName string + serviceSDL string + query string // overrides the generated single-field query when set + responseBody string // overrides the generated subgraph response when set + options []executionTestOptions + expectedResponse string + }{ + { + name: "interface accepts an implementation", + fieldName: "nullableInterface", + selection: "__typename id", + returnedTypeName: "AccessibleNode", + expectedResponse: `{"data":{"nullableInterface":{"__typename":"AccessibleNode","id":"1"}}}`, + }, + { + name: "interface rejects an unknown implementation", + fieldName: "nullableInterface", + selection: "__typename id", + returnedTypeName: "UnexpectedNode", + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned invalid value 'UnexpectedNode' for __typename field.","path":["nullableInterface"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableInterface":null}}`, + }, + { + name: "interface redacts an inaccessible implementation", + fieldName: "nullableInterface", + selection: "__typename id", + returnedTypeName: "InaccessibleNode", + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["nullableInterface"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableInterface":null}}`, + }, + { + name: "non-null interface propagates null for an invalid implementation", + fieldName: "interface", + selection: "__typename id", + returnedTypeName: "UnexpectedNode", + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned invalid value 'UnexpectedNode' for __typename field.","path":["interface"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":null}`, + }, + { + name: "non-null interface redacts an inaccessible implementation and propagates null", + fieldName: "interface", + selection: "__typename id", + returnedTypeName: "InaccessibleNode", + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["interface"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":null}`, + }, + { + name: "union accepts a member", + fieldName: "nullableUnion", + selection: "__typename ... on AccessibleNode { id }", + returnedTypeName: "AccessibleNode", + expectedResponse: `{"data":{"nullableUnion":{"__typename":"AccessibleNode","id":"1"}}}`, + }, + { + name: "union rejects an unknown member", + fieldName: "nullableUnion", + selection: "__typename ... on AccessibleNode { id }", + returnedTypeName: "UnexpectedNode", + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned invalid value 'UnexpectedNode' for __typename field.","path":["nullableUnion"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableUnion":null}}`, + }, + { + name: "union rejects a runtime type that is not a contract member", + fieldName: "nullableUnion", + selection: "__typename ... on AccessibleNode { id }", + returnedTypeName: "RemovedNode", + serviceSDL: abstractTypeValidationSubgraphSDL, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned invalid value 'RemovedNode' for __typename field.","path":["nullableUnion"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableUnion":null}}`, + }, + { + name: "union redacts an inaccessible member", + fieldName: "nullableUnion", + selection: "__typename ... on AccessibleNode { id }", + returnedTypeName: "InaccessibleNode", + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["nullableUnion"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableUnion":null}}`, + }, + { + name: "non-null union propagates null for an invalid member", + fieldName: "union", + selection: "__typename ... on AccessibleNode { id }", + returnedTypeName: "UnexpectedNode", + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned invalid value 'UnexpectedNode' for __typename field.","path":["union"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":null}`, + }, + { + name: "non-null union redacts an inaccessible member and propagates null", + fieldName: "union", + selection: "__typename ... on AccessibleNode { id }", + returnedTypeName: "InaccessibleNode", + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["union"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":null}`, + }, + { + name: "list reports the index of a rejected unknown element", + fieldName: "interfaces", + selection: "__typename id", + responseBody: `{"data":{"interfaces":[{"__typename":"AccessibleNode","id":"1"},{"__typename":"UnexpectedNode","id":"2"}]}}`, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned invalid value 'UnexpectedNode' for __typename field.","path":["interfaces",1],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"interfaces":[{"__typename":"AccessibleNode","id":"1"},null]}}`, + }, + { + name: "list reports the index of a redacted inaccessible element", + fieldName: "interfaces", + selection: "__typename id", + responseBody: `{"data":{"interfaces":[{"__typename":"AccessibleNode","id":"1"},{"__typename":"InaccessibleNode","id":"2"}]}}`, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["interfaces",1],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"interfaces":[{"__typename":"AccessibleNode","id":"1"},null]}}`, + }, + { + name: "list with non-null elements propagates null for an inaccessible element", + fieldName: "requiredInterfaces", + selection: "__typename id", + responseBody: `{"data":{"requiredInterfaces":[{"__typename":"InaccessibleNode","id":"1"}]}}`, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["requiredInterfaces",0],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"requiredInterfaces":null}}`, + }, + { + name: "abstract fields merged from fragment selections stay validated", + fieldName: "nullableInterface", + query: `query { nullableInterface { __typename ... on AccessibleNode { friend { __typename id } } ... on SecondNode { friend { __typename id } } } }`, + responseBody: `{"data":{"nullableInterface":{"__typename":"AccessibleNode","friend":{"__typename":"InaccessibleNode","id":"1"}}}}`, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["nullableInterface","friend"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableInterface":{"__typename":"AccessibleNode","friend":null}}}`, + }, + { + name: "union rejects an unknown member with value completion", + fieldName: "nullableUnion", + selection: "__typename ... on AccessibleNode { id }", + returnedTypeName: "UnexpectedNode", + options: []executionTestOptions{withValueCompletion()}, + expectedResponse: `{"data":{"nullableUnion":null},"extensions":{"valueCompletion":[{"message":"Invalid __typename found for object at field Query.nullableUnion.","path":["nullableUnion"],"extensions":{"code":"INVALID_GRAPHQL"}}]}}`, + }, + { + name: "union redacts an inaccessible member with value completion", + fieldName: "nullableUnion", + selection: "__typename ... on AccessibleNode { id }", + returnedTypeName: "InaccessibleNode", + options: []executionTestOptions{withValueCompletion()}, + expectedResponse: `{"data":{"nullableUnion":null},"extensions":{"valueCompletion":[{"message":"Invalid __typename found for object at field Query.nullableUnion.","path":["nullableUnion"],"extensions":{"code":"INVALID_GRAPHQL"}}]}}`, + }, + } + + for _, tt := range tests { + serviceSDL := tt.serviceSDL + if serviceSDL == "" { + serviceSDL = abstractTypeValidationSDL + } + query := tt.query + if query == "" { + query = "query { " + tt.fieldName + " { " + tt.selection + " } }" + } + responseBody := tt.responseBody + if responseBody == "" { + responseBody = `{"data":{"` + tt.fieldName + `":{"__typename":"` + tt.returnedTypeName + `","id":"1"}}}` + } + + t.Run(tt.name, runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + Query: query, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfigurationWithName( + t, + "abstract-types", + "AbstractTypes", + mustFactory(t, testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + sendResponseBody: responseBody, + sendStatusCode: 200, + })), + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"interface", "nullableInterface", "union", "nullableUnion", "interfaces", "requiredInterfaces"}, + }, + }, + ChildNodes: []plan.TypeField{ + {TypeName: "Node", FieldNames: []string{"id"}}, + {TypeName: "AccessibleNode", FieldNames: []string{"id", "friend"}}, + {TypeName: "SecondNode", FieldNames: []string{"id", "friend"}}, + {TypeName: "InaccessibleNode", FieldNames: []string{"id"}}, + }, + }, + mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "GET", + }, + SchemaConfiguration: mustSchemaConfig( + t, + &graphql_datasource.FederationConfiguration{ + Enabled: true, + ServiceSDL: serviceSDL, + }, + serviceSDL, + ), + }), + ), + }, + expectedResponse: tt.expectedResponse, + }, + tt.options..., + )) + } +} + +const abstractTypeValidationSDL = ` + type Query { + interface: Node! + nullableInterface: Node + union: Result! + nullableUnion: Result + interfaces: [Node] + requiredInterfaces: [Node!] + } + + interface Node { + id: ID! + } + + type AccessibleNode implements Node { + id: ID! + friend: Node + } + + type SecondNode implements Node { + id: ID! + friend: Node + } + + type InaccessibleNode implements Node @inaccessible { + id: ID! + } + + union Result = AccessibleNode | InaccessibleNode +` + +const abstractTypeValidationSubgraphSDL = abstractTypeValidationSDL + ` + type RemovedNode implements Node { + id: ID! + } + + extend union Result = RemovedNode +` From c9906c1f463bc1e871aa8f15e08e108e5d849c47 Mon Sep 17 00:00:00 2001 From: endigma Date: Tue, 14 Jul 2026 16:18:05 +0100 Subject: [PATCH 2/9] feat(resolve): validate inaccessible types returned for abstract fields --- v2/pkg/engine/plan/visitor.go | 15 ++++++++++++--- v2/pkg/engine/resolve/node_object.go | 4 ++++ v2/pkg/engine/resolve/resolvable.go | 3 +++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index b43025d1da..ad8bcc6c24 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -814,13 +814,17 @@ func (v *Visitor) resolveFieldValue(fieldRef, typeRef int, nullable bool, path [ case ast.NodeKindInterfaceTypeDefinition: objectTypesImplementingInterface, _ := v.Definition.InterfaceTypeDefinitionImplementedByObjectWithNames(typeDefinitionNode.Ref) for _, implementingTypeName := range objectTypesImplementingInterface { - // exlude inaccessible types from possible types + // inaccessible types are recorded separately so the resolver can + // reject them without leaking the typename in the error if v.isInaccesibleType(implementingTypeName) { + if object.InaccessibleTypes == nil { + object.InaccessibleTypes = map[string]struct{}{} + } + object.InaccessibleTypes[implementingTypeName] = struct{}{} continue } object.PossibleTypes[implementingTypeName] = struct{}{} - } if slices.Contains(v.Config.EntityInterfaceNames, typeName) { @@ -830,8 +834,13 @@ func (v *Visitor) resolveFieldValue(fieldRef, typeRef int, nullable bool, path [ case ast.NodeKindUnionTypeDefinition: if unionMembers, ok := v.Definition.UnionTypeDefinitionMemberTypeNames(typeDefinitionNode.Ref); ok { for _, unionMember := range unionMembers { - // exlude inaccessible types from possible types + // inaccessible types are recorded separately so the resolver can + // reject them without leaking the typename in the error if v.isInaccesibleType(unionMember) { + if object.InaccessibleTypes == nil { + object.InaccessibleTypes = map[string]struct{}{} + } + object.InaccessibleTypes[unionMember] = struct{}{} continue } object.PossibleTypes[unionMember] = struct{}{} diff --git a/v2/pkg/engine/resolve/node_object.go b/v2/pkg/engine/resolve/node_object.go index 37790ed4f6..b4018a2e7b 100644 --- a/v2/pkg/engine/resolve/node_object.go +++ b/v2/pkg/engine/resolve/node_object.go @@ -13,6 +13,10 @@ type Object struct { PossibleTypes map[string]struct{} `json:"-"` SourceName string `json:"-"` TypeName string `json:"-"` + + // InaccessibleTypes are members/implementers of the abstract type marked + // @inaccessible; excluded from PossibleTypes. Nil when none. + InaccessibleTypes map[string]struct{} `json:"-"` } func (o *Object) Copy() Node { diff --git a/v2/pkg/engine/resolve/resolvable.go b/v2/pkg/engine/resolve/resolvable.go index 7f4d5a1a75..529c33cd49 100644 --- a/v2/pkg/engine/resolve/resolvable.go +++ b/v2/pkg/engine/resolve/resolvable.go @@ -1322,6 +1322,9 @@ func (r *Resolvable) walkObject(obj *Object, parent *astjson.Value) (hasError bo // during pre-walk we need to add an error when the typename do not match a possible type if r.options.ApolloCompatibilityValueCompletionInExtensions { r.addValueCompletion(fmt.Sprintf("Invalid __typename found for object at %s.", r.pathLastElementDescription(obj.TypeName)), errorcodes.InvalidGraphql) + } else if _, inaccessible := obj.InaccessibleTypes[string(typeName)]; inaccessible { + // the type is a member of the abstract type but @inaccessible so the error must not leak its name + r.addErrorWithCode(fmt.Sprintf("Subgraph '%s' returned an invalid value for __typename field.", obj.SourceName), errorcodes.InvalidGraphql) } else { r.addErrorWithCode(fmt.Sprintf("Subgraph '%s' returned invalid value '%s' for __typename field.", obj.SourceName, string(typeName)), errorcodes.InvalidGraphql) } From 6ccece63767ded25abd365f6eefe5c2657907081 Mon Sep 17 00:00:00 2001 From: endigma Date: Tue, 14 Jul 2026 16:59:58 +0100 Subject: [PATCH 3/9] test(engine): add test coverage for abstract validation when __typename is missing --- .../engine/abstract_type_validation_test.go | 135 +++++++++++------- 1 file changed, 86 insertions(+), 49 deletions(-) diff --git a/execution/engine/abstract_type_validation_test.go b/execution/engine/abstract_type_validation_test.go index 1a3cc3185b..910dd32521 100644 --- a/execution/engine/abstract_type_validation_test.go +++ b/execution/engine/abstract_type_validation_test.go @@ -24,6 +24,7 @@ func TestAbstractTypeValidation(t *testing.T) { serviceSDL string query string // overrides the generated single-field query when set responseBody string // overrides the generated subgraph response when set + expectedBody string // asserts the exact subgraph request body when set options []executionTestOptions expectedResponse string }{ @@ -133,6 +134,36 @@ func TestAbstractTypeValidation(t *testing.T) { responseBody: `{"data":{"nullableInterface":{"__typename":"AccessibleNode","friend":{"__typename":"InaccessibleNode","id":"1"}}}}`, expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["nullableInterface","friend"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableInterface":{"__typename":"AccessibleNode","friend":null}}}`, }, + { + // the subgraph request must ask for the runtime type even when the + // client selection does not force it, so validation always has a + // typename to check + name: "interface requests the runtime type from the subgraph", + fieldName: "nullableInterface", + selection: "id", + expectedBody: `{"query":"{nullableInterface {__typename id}}"}`, + responseBody: `{"data":{"nullableInterface":{"__typename":"AccessibleNode","id":"1"}}}`, + expectedResponse: `{"data":{"nullableInterface":{"id":"1"}}}`, + }, + { + // control for the case below: identical inaccessible data, but the + // subgraph self-reports the typename, so redaction fires even though + // the client never selected __typename + name: "interface redacts an inaccessible implementation when only the subgraph returns the typename", + fieldName: "nullableInterface", + selection: "id", + responseBody: `{"data":{"nullableInterface":{"__typename":"InaccessibleNode","id":"classified-secret-42"}}}`, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["nullableInterface"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableInterface":null}}`, + }, + { + // identical response minus the __typename key: validation must not + // depend on the subgraph volunteering the typename + name: "interface redacts an inaccessible implementation when the subgraph omits the typename", + fieldName: "nullableInterface", + selection: "id", + responseBody: `{"data":{"nullableInterface":{"id":"classified-secret-42"}}}`, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["nullableInterface"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableInterface":null}}`, + }, { name: "union rejects an unknown member with value completion", fieldName: "nullableUnion", @@ -165,59 +196,65 @@ func TestAbstractTypeValidation(t *testing.T) { responseBody = `{"data":{"` + tt.fieldName + `":{"__typename":"` + tt.returnedTypeName + `","id":"1"}}}` } - t.Run(tt.name, runWithoutError( - ExecutionEngineTestCase{ - schema: schema, - operation: func(t *testing.T) graphql.Request { - return graphql.Request{ - Query: query, - } - }, - dataSources: []plan.DataSource{ - mustGraphqlDataSourceConfigurationWithName( - t, - "abstract-types", - "AbstractTypes", - mustFactory(t, testNetHttpClient(t, roundTripperTestCase{ - expectedHost: "example.com", - expectedPath: "/", - sendResponseBody: responseBody, - sendStatusCode: 200, - })), - &plan.DataSourceMetadata{ - RootNodes: []plan.TypeField{ - { - TypeName: "Query", - FieldNames: []string{"interface", "nullableInterface", "union", "nullableUnion", "interfaces", "requiredInterfaces"}, + // the test case is built inside the subtest so the round tripper + // captures the subtest's t: require failures from the request-body + // assertion must fail the row, not Goexit the parent's goroutine + t.Run(tt.name, func(t *testing.T) { + runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + Query: query, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfigurationWithName( + t, + "abstract-types", + "AbstractTypes", + mustFactory(t, testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: tt.expectedBody, + sendResponseBody: responseBody, + sendStatusCode: 200, + })), + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"interface", "nullableInterface", "union", "nullableUnion", "interfaces", "requiredInterfaces"}, + }, + }, + ChildNodes: []plan.TypeField{ + {TypeName: "Node", FieldNames: []string{"id"}}, + {TypeName: "AccessibleNode", FieldNames: []string{"id", "friend"}}, + {TypeName: "SecondNode", FieldNames: []string{"id", "friend"}}, + {TypeName: "InaccessibleNode", FieldNames: []string{"id"}}, }, }, - ChildNodes: []plan.TypeField{ - {TypeName: "Node", FieldNames: []string{"id"}}, - {TypeName: "AccessibleNode", FieldNames: []string{"id", "friend"}}, - {TypeName: "SecondNode", FieldNames: []string{"id", "friend"}}, - {TypeName: "InaccessibleNode", FieldNames: []string{"id"}}, - }, - }, - mustConfiguration(t, graphql_datasource.ConfigurationInput{ - Fetch: &graphql_datasource.FetchConfiguration{ - URL: "https://example.com/", - Method: "GET", - }, - SchemaConfiguration: mustSchemaConfig( - t, - &graphql_datasource.FederationConfiguration{ - Enabled: true, - ServiceSDL: serviceSDL, + mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "GET", }, - serviceSDL, - ), - }), - ), + SchemaConfiguration: mustSchemaConfig( + t, + &graphql_datasource.FederationConfiguration{ + Enabled: true, + ServiceSDL: serviceSDL, + }, + serviceSDL, + ), + }), + ), + }, + expectedResponse: tt.expectedResponse, }, - expectedResponse: tt.expectedResponse, - }, - tt.options..., - )) + tt.options..., + )(t) + }) } } From 8d4e5d68468598c291d68366c54e98f22b27ed9d Mon Sep 17 00:00:00 2001 From: endigma Date: Tue, 14 Jul 2026 17:53:26 +0100 Subject: [PATCH 4/9] feat(engine): always request and enforce __typename for abstract values --- .../engine/execution_engine_cost_test.go | 32 ++++++++++------- .../engine/execution_engine_helpers_test.go | 2 +- execution/engine/execution_engine_test.go | 26 +++++++------- .../complex_nesting_query_with_art.json | 10 ++++-- .../graphql_datasource/graphql_datasource.go | 19 ++++------- ...urce_federation_interface_provides_test.go | 2 +- .../graphql_datasource_federation_test.go | 2 +- .../graphql_datasource_test.go | 34 +++++++++---------- v2/pkg/engine/resolve/resolvable.go | 16 +++++++++ 9 files changed, 81 insertions(+), 62 deletions(-) diff --git a/execution/engine/execution_engine_cost_test.go b/execution/engine/execution_engine_cost_test.go index c14f8f66f8..aa56fc93d9 100644 --- a/execution/engine/execution_engine_cost_test.go +++ b/execution/engine/execution_engine_cost_test.go @@ -296,7 +296,7 @@ func TestExecutionEngine_Cost(t *testing.T) { // When the subgraph resolves a single (non-list) abstract field and does NOT return __typename, // we must still record one occurrence for that field's path, falling back to the declared // abstract type name in actual costs. - t.Run("single abstract field without __typename takes into account implementing types", runWithoutError( + t.Run("single abstract field without __typename is rejected and bills nothing", runWithoutError( ExecutionEngineTestCase{ schema: graphql.StarwarsSchema(t), operation: func(t *testing.T) graphql.Request { @@ -325,7 +325,8 @@ func TestExecutionEngine_Cost(t *testing.T) { ), }, expectedEstimatedCost: intPtr(13), // Query.hero(13) - expectedActualCost: intPtr(13), // Query.hero(13) + // the abstract hero value is rejected and nulled, so nothing is billed + expectedActualCost: intPtr(0), }, computeCosts(), )) @@ -778,7 +779,8 @@ func TestExecutionEngine_Cost(t *testing.T) { customConfig, ), }, - expectedResponse: `{"data":{"hero":{"name":"Luke","friends":[{"name":"Leia"}]}}}`, + // friends items carry no __typename, so they are rejected and nulled + expectedResponse: `{"errors":[{"message":"Subgraph 'id' returned an invalid value for __typename field.","path":["hero","friends",0],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"hero":{"name":"Luke","friends":[null]}}}`, // Cost calculation: // Query.hero: 2 // Character.name: max(Human.name=3, Droid.name=5) = 5 @@ -787,8 +789,9 @@ func TestExecutionEngine_Cost(t *testing.T) { // name: max(Human.name=3, Droid.name=5) = 5 expectedEstimatedCost: intPtr(55), // 2 + 1*(5 + 6*(3 + 1*5)) // hero returned __typename Human, so its name is billed at Human.name (3). - // friends items carry no __typename, so their type weight and name keep the max (3, 5). - expectedActualCost: intPtr(13), // 2 + 1*(3 + 1*(3 + 1*5)) + // The rejected friends element is nulled: its max type weight is still + // counted for the returned element, but no field weights are billed. + expectedActualCost: intPtr(8), // 2 + 1*(3 + 1*3) }, computeCosts(), )) @@ -1313,7 +1316,8 @@ func TestExecutionEngine_Cost(t *testing.T) { customConfig, ), }, - expectedResponse: `{"data":{"hero":{"name":"Luke","friends":[{"name":"Leia"}]}}}`, + // friends items carry no __typename, so they are rejected and nulled + expectedResponse: `{"errors":[{"message":"Subgraph 'id' returned an invalid value for __typename field.","path":["hero","friends",0],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"hero":{"name":"Luke","friends":[null]}}}`, expectedEstimatedCost: intPtr(20), // 2 + 1*(0 + 6*(3 + 1*0)) expectedActualCost: intPtr(5), // 2 + 1*(0 + 1*(3 + 1*0)) }, @@ -7473,7 +7477,7 @@ func TestExecutionEngine_Cost(t *testing.T) { computeCosts(), )) - t.Run("without typenames keeps max weight", runWithoutError( + t.Run("without typenames rejects the values and bills nothing for them", runWithoutError( ExecutionEngineTestCase{ schema: schema, operation: func(t *testing.T) graphql.Request { @@ -7498,13 +7502,15 @@ func TestExecutionEngine_Cost(t *testing.T) { ), }, fields: []plan.FieldConfiguration{}, - expectedResponse: `{"data":{"items":[` + - `{"hero":{"name":"Luke"}},` + - `{"hero":{"name":"Han"}},` + - `{"hero":{"name":"R2D2"}}]}}`, + // Abstract values without a __typename are rejected and nulled, + // so the actual cost bills nothing for them. + expectedResponse: `{"errors":[` + + `{"message":"Subgraph 'id' returned an invalid value for __typename field.","path":["items",0,"hero"],"extensions":{"code":"INVALID_GRAPHQL"}},` + + `{"message":"Subgraph 'id' returned an invalid value for __typename field.","path":["items",1,"hero"],"extensions":{"code":"INVALID_GRAPHQL"}},` + + `{"message":"Subgraph 'id' returned an invalid value for __typename field.","path":["items",2,"hero"],"extensions":{"code":"INVALID_GRAPHQL"}}],` + + `"data":{"items":[{"hero":null},{"hero":null},{"hero":null}]}}`, expectedEstimatedCost: intPtr(170), // 10 * (0 + (0 + max(7, 17))) - // Subgraph returned no __typename for hero: no per-type info, keep the max. - expectedActualCost: intPtr(51), // 3 * 17 + expectedActualCost: intPtr(0), }, computeCosts(), )) diff --git a/execution/engine/execution_engine_helpers_test.go b/execution/engine/execution_engine_helpers_test.go index bcfd7c761b..7e062d8854 100644 --- a/execution/engine/execution_engine_helpers_test.go +++ b/execution/engine/execution_engine_helpers_test.go @@ -110,7 +110,7 @@ func createTestRoundTripper(t *testing.T, testCase roundTripperTestCase) testRou receivedBodyBytes, err = io.ReadAll(req.Body) require.NoError(t, err) } - require.Equal(t, testCase.expectedBody, string(receivedBodyBytes), "roundTripperTestCase received unexpected body") + assert.Equal(t, testCase.expectedBody, string(receivedBodyBytes), "roundTripperTestCase received unexpected body") } body := bytes.NewBuffer([]byte(testCase.sendResponseBody)) diff --git a/execution/engine/execution_engine_test.go b/execution/engine/execution_engine_test.go index 42b3536d5b..28e378143c 100644 --- a/execution/engine/execution_engine_test.go +++ b/execution/engine/execution_engine_test.go @@ -805,7 +805,7 @@ func TestExecutionEngine_Execute(t *testing.T) { expectedHost: "example.com", expectedPath: "/", expectedBody: "", - sendResponseBody: `{"data":{"hero":{"name":"Luke Skywalker"}}}`, + sendResponseBody: `{"data":{"hero":{"__typename":"Human","name":"Luke Skywalker"}}}`, sendStatusCode: 200, }), ), @@ -910,7 +910,7 @@ func TestExecutionEngine_Execute(t *testing.T) { expectedHost: "example.com", expectedPath: "/", expectedBody: "", - sendResponseBody: `{"data":{"hero":{"name":"Luke Skywalker"}}}`, + sendResponseBody: `{"data":{"hero":{"__typename":"Human","name":"Luke Skywalker"}}}`, sendStatusCode: 200, }), ), @@ -957,8 +957,8 @@ func TestExecutionEngine_Execute(t *testing.T) { testNetHttpClient(t, roundTripperTestCase{ expectedHost: "example.com", expectedPath: "/", - expectedBody: `{"query":"{hero {name}}","extensions":{"fetch_reasons":[{"typename":"Character","field":"name","by_user":true},{"typename":"Droid","field":"name","by_user":true},{"typename":"Human","field":"name","by_user":true}]}}`, - sendResponseBody: `{"data":{"hero":{"name":"Luke Skywalker"}}}`, + expectedBody: `{"query":"{hero {__typename name}}","extensions":{"fetch_reasons":[{"typename":"Character","field":"name","by_user":true},{"typename":"Droid","field":"name","by_user":true},{"typename":"Human","field":"name","by_user":true}]}}`, + sendResponseBody: `{"data":{"hero":{"__typename":"Human","name":"Luke Skywalker"}}}`, sendStatusCode: 200, }), ), @@ -1016,8 +1016,8 @@ func TestExecutionEngine_Execute(t *testing.T) { testNetHttpClient(t, roundTripperTestCase{ expectedHost: "example.com", expectedPath: "/", - expectedBody: `{"query":"{hero {name}}","extensions":{"fetch_reasons":[{"typename":"Droid","field":"name","by_user":true}]}}`, - sendResponseBody: `{"data":{"hero":{"name":"Droid Number 6"}}}`, + expectedBody: `{"query":"{hero {__typename name}}","extensions":{"fetch_reasons":[{"typename":"Droid","field":"name","by_user":true}]}}`, + sendResponseBody: `{"data":{"hero":{"__typename":"Droid","name":"Droid Number 6"}}}`, sendStatusCode: 200, }), ), @@ -1076,8 +1076,8 @@ func TestExecutionEngine_Execute(t *testing.T) { testNetHttpClient(t, roundTripperTestCase{ expectedHost: "example.com", expectedPath: "/", - expectedBody: `{"query":"{hero {name}}","extensions":{"fetch_reasons":[{"typename":"Character","field":"name","by_user":true},{"typename":"Droid","field":"name","by_user":true},{"typename":"Human","field":"name","by_user":true}]}}`, - sendResponseBody: `{"data":{"hero":{"name":"Droid Number 6"}}}`, + expectedBody: `{"query":"{hero {__typename name}}","extensions":{"fetch_reasons":[{"typename":"Character","field":"name","by_user":true},{"typename":"Droid","field":"name","by_user":true},{"typename":"Human","field":"name","by_user":true}]}}`, + sendResponseBody: `{"data":{"hero":{"__typename":"Droid","name":"Droid Number 6"}}}`, sendStatusCode: 200, }), ), @@ -1280,7 +1280,7 @@ func TestExecutionEngine_Execute(t *testing.T) { expectedHost: "example.com", expectedPath: "/", expectedBody: "", - sendResponseBody: `{"data":{"hero":{"name":"Luke Skywalker"}}, "errors": []}`, + sendResponseBody: `{"data":{"hero":{"__typename":"Human","name":"Luke Skywalker"}}, "errors": []}`, sendStatusCode: 200, }), ), @@ -1332,7 +1332,7 @@ func TestExecutionEngine_Execute(t *testing.T) { expectedHost: "example.com", expectedPath: "/", expectedBody: "", - sendResponseBody: `{"data":{"hero":{"name":"Luke Skywalker"}}}`, + sendResponseBody: `{"data":{"hero":{"__typename":"Human","name":"Luke Skywalker"}}}`, sendStatusCode: 200, }), ), @@ -2305,7 +2305,7 @@ func TestExecutionEngine_Execute(t *testing.T) { testNetHttpClient(t, roundTripperTestCase{ expectedHost: "example.com", expectedPath: "/", - expectedBody: `{"query":"{codeType {code __typename ... on Country {name}}}"}`, + expectedBody: `{"query":"{codeType {__typename code ... on Country {name}}}"}`, sendResponseBody: `{"data":{"codeType":{"__typename":"Country","code":"de","name":"Germany"}}}`, sendStatusCode: 200, }), @@ -2436,7 +2436,7 @@ func TestExecutionEngine_Execute(t *testing.T) { expectedHost: "example.com", expectedPath: "/", expectedBody: "", - sendResponseBody: `{"data":{"searchResults":[{"name":"Luke Skywalker"},{"length":13.37}]}}`, + sendResponseBody: `{"data":{"searchResults":[{"__typename":"Human","name":"Luke Skywalker"},{"__typename":"Starship","length":13.37}]}}`, sendStatusCode: 200, }), ), @@ -2484,7 +2484,7 @@ func TestExecutionEngine_Execute(t *testing.T) { ), }, fields: []plan.FieldConfiguration{}, - expectedResponse: `{"data":{"searchResults":[{},{}]}}`, + expectedResponse: `{"data":{"searchResults":[{"name":"Luke Skywalker"},{"length":13.37}]}}`, }, )) diff --git a/execution/engine/testdata/complex_nesting_query_with_art.json b/execution/engine/testdata/complex_nesting_query_with_art.json index d0b79acab3..1b9f261e2a 100644 --- a/execution/engine/testdata/complex_nesting_query_with_art.json +++ b/execution/engine/testdata/complex_nesting_query_with_art.json @@ -96,7 +96,7 @@ "raw_input_data": {}, "input": { "body": { - "query": "{me {id username history {__typename ... on Purchase {wallet {currency}} ... on Sale {location product {upc __typename}}} __typename}}" + "query": "{me {id username history {__typename ... on Purchase {wallet {__typename currency}} ... on Sale {location product {upc __typename}}} __typename}}" }, "header": {}, "method": "POST", @@ -111,6 +111,7 @@ { "__typename": "Purchase", "wallet": { + "__typename": "WalletType1", "currency": "USD" } }, @@ -125,6 +126,7 @@ { "__typename": "Purchase", "wallet": { + "__typename": "WalletType2", "currency": "USD" } } @@ -155,13 +157,13 @@ "status": "200 OK", "headers": { "Content-Length": [ - "277" + "331" ], "Content-Type": [ "application/json" ] }, - "body_size": 277 + "body_size": 331 } } } @@ -387,6 +389,7 @@ { "__typename": "Purchase", "wallet": { + "__typename": "WalletType1", "currency": "USD" } }, @@ -401,6 +404,7 @@ { "__typename": "Purchase", "wallet": { + "__typename": "WalletType2", "currency": "USD" } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index d8229f3262..f3cdebda67 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -578,19 +578,12 @@ func (p *Planner[T]) EnterSelectionSet(ref int) { p.addRepresentationsQuery() } - if p.visitor.Walker.EnclosingTypeDefinition.Kind != ast.NodeKindInterfaceTypeDefinition { - return - } - - // handle adding typename for the InterfaceObject - // In case we are inside selection set which returns an interface object - // we need to add __typename field to the selection set to get an initial typename value - typeName := p.visitor.Walker.EnclosingTypeDefinition.NameString(p.visitor.Definition) - for _, interfaceObjectCfg := range p.dataSourceConfig.FederationConfiguration().InterfaceObjects { - if interfaceObjectCfg.InterfaceTypeName == typeName { - p.addTypenameToSelectionSet(set.Ref) - return - } + // abstract values are validated against the contract by their runtime type, + // so the upstream must always report it. This also covers the InterfaceObject + // case, which needs __typename for an initial typename value. + switch p.visitor.Walker.EnclosingTypeDefinition.Kind { + case ast.NodeKindInterfaceTypeDefinition, ast.NodeKindUnionTypeDefinition: + p.addTypenameToSelectionSet(set.Ref) } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_interface_provides_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_interface_provides_test.go index 71a4f34ab4..0e40863850 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_interface_provides_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_interface_provides_test.go @@ -44,7 +44,7 @@ func interfaceProvidesPlan() *plan.SynchronousResponsePlan { Response: &resolve.GraphQLResponse{ Fetches: resolve.Sequence(resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://localhost:4250/provides-on-interface/b","body":{"query":"{media {__typename ... on Book {id animals {id name}}}}"}}`, + Input: `{"method":"POST","url":"http://localhost:4250/provides-on-interface/b","body":{"query":"{media {__typename ... on Book {id animals {__typename id name}}}}"}}`, DataSource: &Source{}, PostProcessing: DefaultPostProcessingConfiguration, }, diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go index a9948b9dcf..c34dfef99d 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go @@ -11328,7 +11328,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetches: resolve.Sequence( resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://first.service","body":{"query":"{account {some {__typename id}}}"}}`, + Input: `{"method":"POST","url":"http://first.service","body":{"query":"{account {__typename some {__typename id}}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, DataSource: &Source{}, }, diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go index bde0f49289..9cd62943c0 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go @@ -188,7 +188,7 @@ func TestGraphQLDataSource(t *testing.T) { resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","header":{"Authorization":["$$1$$"],"Invalid-Template":["{{ request.headers.Authorization }}"]},"body":{"query":"query($id: ID!){droid(id: $id){name aliased: name friends {name} primaryFunction} hero {name} stringList nestedStringList}","variables":{"id":$$0$$}}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","header":{"Authorization":["$$1$$"],"Invalid-Template":["{{ request.headers.Authorization }}"]},"body":{"query":"query($id: ID!){droid(id: $id){name aliased: name friends {__typename name} primaryFunction} hero {__typename name} stringList nestedStringList}","variables":{"id":$$0$$}}}`, Variables: resolve.NewVariables( &resolve.ContextVariable{ Path: []string{"id"}, @@ -383,7 +383,7 @@ func TestGraphQLDataSource(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","header":{"Authorization":["$$1$$"],"Invalid-Template":["{{ request.headers.Authorization }}"]},"body":{"query":"query($id: ID!){droid(id: $id){name aliased: name friends {name} primaryFunction} hero {name} stringList nestedStringList}","variables":{"id":$$0$$}}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","header":{"Authorization":["$$1$$"],"Invalid-Template":["{{ request.headers.Authorization }}"]},"body":{"query":"query($id: ID!){droid(id: $id){name aliased: name friends {__typename name} primaryFunction} hero {__typename name} stringList nestedStringList}","variables":{"id":$$0$$}}}`, Variables: resolve.NewVariables( &resolve.ContextVariable{ Path: []string{"id"}, @@ -696,7 +696,7 @@ func TestGraphQLDataSource(t *testing.T) { resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {id displayName}}"}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {__typename id displayName}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), @@ -917,7 +917,7 @@ func TestGraphQLDataSource(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {id}}"}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {__typename id}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, }, })), @@ -994,7 +994,7 @@ func TestGraphQLDataSource(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {id displayName}}"}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {__typename id displayName}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, }, })), @@ -1077,7 +1077,7 @@ func TestGraphQLDataSource(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {id displayName}}"}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {__typename id displayName}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, }, })), @@ -1159,7 +1159,7 @@ func TestGraphQLDataSource(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {id}}"}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {__typename id}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, }, })), @@ -1239,7 +1239,7 @@ func TestGraphQLDataSource(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {id displayName __typename ... on RegisteredUser {hasVerifiedEmail}}}"}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"{user {__typename id displayName ... on RegisteredUser {hasVerifiedEmail}}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, }, })), @@ -1434,7 +1434,7 @@ func TestGraphQLDataSource(t *testing.T) { resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"query($heroId: ID!){droid(id: $heroId){name} hero {id}}","variables":{"heroId":$$0$$}}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"query($heroId: ID!){droid(id: $heroId){name} hero {__typename id}}","variables":{"heroId":$$0$$}}}`, Variables: resolve.NewVariables( &resolve.ContextVariable{ Path: []string{"heroId"}, @@ -1567,7 +1567,7 @@ func TestGraphQLDataSource(t *testing.T) { resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","header":{"Authorization":["$$2$$"],"Invalid-Template":["{{ request.headers.Authorization }}"]},"body":{"query":"query($id: ID!, $heroName: String!){droid(id: $id){name aliased: name friends {name} primaryFunction} hero {name} search(name: $heroName){__typename ... on Droid {primaryFunction}} stringList nestedStringList}","variables":{"heroName":$$1$$,"id":$$0$$}}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","header":{"Authorization":["$$2$$"],"Invalid-Template":["{{ request.headers.Authorization }}"]},"body":{"query":"query($id: ID!, $heroName: String!){droid(id: $id){name aliased: name friends {__typename name} primaryFunction} hero {__typename name} search(name: $heroName){__typename ... on Droid {primaryFunction}} stringList nestedStringList}","variables":{"heroName":$$1$$,"id":$$0$$}}}`, Variables: resolve.NewVariables( &resolve.ContextVariable{ Path: []string{"id"}, @@ -2442,7 +2442,7 @@ func TestGraphQLDataSource(t *testing.T) { Fetches: resolve.Sequence( resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"query($birthdate: Date!){heroByBirthdate(birthdate: $birthdate){name}}","variables":{"birthdate":$$0$$}}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","body":{"query":"query($birthdate: Date!){heroByBirthdate(birthdate: $birthdate){__typename name}}","variables":{"birthdate":$$0$$}}}`, DataSource: &Source{}, Variables: resolve.NewVariables( &resolve.ContextVariable{ @@ -4568,7 +4568,7 @@ func TestGraphQLDataSource(t *testing.T) { FetchID: 1, }, FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://product.service","body":{"query":"query($b: String!){vehicle(id: $b){description}}","variables":{"b":$$0$$}}}`, + Input: `{"method":"POST","url":"http://product.service","body":{"query":"query($b: String!){vehicle(id: $b){__typename description}}","variables":{"b":$$0$$}}}`, DataSource: &Source{}, Variables: resolve.NewVariables( &resolve.ContextVariable{ @@ -4775,7 +4775,7 @@ func TestGraphQLDataSource(t *testing.T) { }, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityFetch: true, - Input: `{"method":"POST","url":"http://product.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on User {__typename vehicle {id description price __typename}}}}","variables":{"representations":[$$0$$]}}}`, + Input: `{"method":"POST","url":"http://product.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on User {__typename vehicle {__typename id description price}}}}","variables":{"representations":[$$0$$]}}}`, Variables: []resolve.Variable{ &resolve.ResolvableObjectVariable{ Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ @@ -5160,7 +5160,7 @@ func TestGraphQLDataSource(t *testing.T) { DependsOnFetchIDs: []int{0}, }, FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://product.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on User {__typename vehicle {id description price __typename}}}}","variables":{"representations":[$$0$$]}}}`, + Input: `{"method":"POST","url":"http://product.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on User {__typename vehicle {__typename id description price}}}}","variables":{"representations":[$$0$$]}}}`, Variables: []resolve.Variable{ &resolve.ResolvableObjectVariable{ Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ @@ -6357,7 +6357,7 @@ func TestGraphQLDataSource(t *testing.T) { Fetches: resolve.Sequence( resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://user.service","body":{"query":"{self {id __typename ... on User {uid: id username __typename id}}}"}}`, + Input: `{"method":"POST","url":"http://user.service","body":{"query":"{self {__typename id ... on User {uid: id username __typename id}}}"}}`, DataSource: &Source{}, PostProcessing: DefaultPostProcessingConfiguration, }, @@ -6794,7 +6794,7 @@ func TestGraphQLDataSource(t *testing.T) { DependsOnFetchIDs: []int{0}, }, FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://pet.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on User {__typename pets {name __typename ... on Cat {catField details {age}} ... on Dog {dogField species} details {hasOwner}}}}}","variables":{"representations":[$$0$$]}}}`, + Input: `{"method":"POST","url":"http://pet.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on User {__typename pets {__typename name ... on Cat {catField details {age}} ... on Dog {dogField species} details {hasOwner}}}}}","variables":{"representations":[$$0$$]}}}`, Variables: resolve.NewVariables( &resolve.ResolvableObjectVariable{ Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ @@ -7922,7 +7922,7 @@ func TestGraphQLDataSource(t *testing.T) { resolve.Single(&resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ DataSource: &Source{}, - Input: `{"method":"POST","url":"https://swapi.com/graphql","header":{"Authorization":["$$2$$"],"Invalid-Template":["{{ request.headers.Authorization }}"]},"body":{"query":"query($droidId: ID!, $reviewId: ReviewID!){droid(id: $droidId){name aliased: name friends {name} primaryFunction} review(id: $reviewId){stars}}","variables":{"reviewId":$$1$$,"droidId":$$0$$}}}`, + Input: `{"method":"POST","url":"https://swapi.com/graphql","header":{"Authorization":["$$2$$"],"Invalid-Template":["{{ request.headers.Authorization }}"]},"body":{"query":"query($droidId: ID!, $reviewId: ReviewID!){droid(id: $droidId){name aliased: name friends {__typename name} primaryFunction} review(id: $reviewId){stars}}","variables":{"reviewId":$$1$$,"droidId":$$0$$}}}`, Variables: resolve.NewVariables( &resolve.ContextVariable{ Path: []string{"droidId"}, diff --git a/v2/pkg/engine/resolve/resolvable.go b/v2/pkg/engine/resolve/resolvable.go index 529c33cd49..e10981ae62 100644 --- a/v2/pkg/engine/resolve/resolvable.go +++ b/v2/pkg/engine/resolve/resolvable.go @@ -1314,6 +1314,22 @@ func (r *Resolvable) walkObject(obj *Object, parent *astjson.Value) (hasError bo } typeName := value.GetStringBytes("__typename") + if typeName == nil && obj.isAbstract() { + // an abstract value without a runtime type cannot be validated against + // the contract, so it must be rejected + if !r.render() { + if r.options.ApolloCompatibilityValueCompletionInExtensions { + r.addValueCompletion(fmt.Sprintf("Invalid __typename found for object at %s.", r.pathLastElementDescription(obj.TypeName)), errorcodes.InvalidGraphql) + } else { + r.addErrorWithCode(fmt.Sprintf("Subgraph '%s' returned an invalid value for __typename field.", obj.SourceName), errorcodes.InvalidGraphql) + } + if !obj.Nullable { + return r.err() + } + return false + } + return r.walkNull() + } if typeName != nil && len(obj.PossibleTypes) > 0 { // when we have a typename field present in a json object, we need to check if the type is valid From 989839d4e5bcf7429a760df81997bffd58f88bd9 Mon Sep 17 00:00:00 2001 From: endigma Date: Wed, 15 Jul 2026 18:54:01 +0100 Subject: [PATCH 5/9] test: use runWithoutError directly despite issues --- .../engine/abstract_type_validation_test.go | 102 +++++++++--------- 1 file changed, 50 insertions(+), 52 deletions(-) diff --git a/execution/engine/abstract_type_validation_test.go b/execution/engine/abstract_type_validation_test.go index 910dd32521..9d5cf21b1b 100644 --- a/execution/engine/abstract_type_validation_test.go +++ b/execution/engine/abstract_type_validation_test.go @@ -199,62 +199,60 @@ func TestAbstractTypeValidation(t *testing.T) { // the test case is built inside the subtest so the round tripper // captures the subtest's t: require failures from the request-body // assertion must fail the row, not Goexit the parent's goroutine - t.Run(tt.name, func(t *testing.T) { - runWithoutError( - ExecutionEngineTestCase{ - schema: schema, - operation: func(t *testing.T) graphql.Request { - return graphql.Request{ - Query: query, - } - }, - dataSources: []plan.DataSource{ - mustGraphqlDataSourceConfigurationWithName( - t, - "abstract-types", - "AbstractTypes", - mustFactory(t, testNetHttpClient(t, roundTripperTestCase{ - expectedHost: "example.com", - expectedPath: "/", - expectedBody: tt.expectedBody, - sendResponseBody: responseBody, - sendStatusCode: 200, - })), - &plan.DataSourceMetadata{ - RootNodes: []plan.TypeField{ - { - TypeName: "Query", - FieldNames: []string{"interface", "nullableInterface", "union", "nullableUnion", "interfaces", "requiredInterfaces"}, - }, - }, - ChildNodes: []plan.TypeField{ - {TypeName: "Node", FieldNames: []string{"id"}}, - {TypeName: "AccessibleNode", FieldNames: []string{"id", "friend"}}, - {TypeName: "SecondNode", FieldNames: []string{"id", "friend"}}, - {TypeName: "InaccessibleNode", FieldNames: []string{"id"}}, + t.Run(tt.name, runWithoutError( + ExecutionEngineTestCase{ + schema: schema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + Query: query, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfigurationWithName( + t, + "abstract-types", + "AbstractTypes", + mustFactory(t, testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: tt.expectedBody, + sendResponseBody: responseBody, + sendStatusCode: 200, + })), + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"interface", "nullableInterface", "union", "nullableUnion", "interfaces", "requiredInterfaces"}, }, }, - mustConfiguration(t, graphql_datasource.ConfigurationInput{ - Fetch: &graphql_datasource.FetchConfiguration{ - URL: "https://example.com/", - Method: "GET", + ChildNodes: []plan.TypeField{ + {TypeName: "Node", FieldNames: []string{"id"}}, + {TypeName: "AccessibleNode", FieldNames: []string{"id", "friend"}}, + {TypeName: "SecondNode", FieldNames: []string{"id", "friend"}}, + {TypeName: "InaccessibleNode", FieldNames: []string{"id"}}, + }, + }, + mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "GET", + }, + SchemaConfiguration: mustSchemaConfig( + t, + &graphql_datasource.FederationConfiguration{ + Enabled: true, + ServiceSDL: serviceSDL, }, - SchemaConfiguration: mustSchemaConfig( - t, - &graphql_datasource.FederationConfiguration{ - Enabled: true, - ServiceSDL: serviceSDL, - }, - serviceSDL, - ), - }), - ), - }, - expectedResponse: tt.expectedResponse, + serviceSDL, + ), + }), + ), }, - tt.options..., - )(t) - }) + expectedResponse: tt.expectedResponse, + }, + tt.options..., + )) } } From c8b4bceef4093daa24c7719bb7959ca9ef06c1f1 Mon Sep 17 00:00:00 2001 From: endigma Date: Wed, 15 Jul 2026 19:36:01 +0100 Subject: [PATCH 6/9] test: add suggested tcs --- .../engine/abstract_type_validation_test.go | 88 ++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/execution/engine/abstract_type_validation_test.go b/execution/engine/abstract_type_validation_test.go index 9d5cf21b1b..b37714afd3 100644 --- a/execution/engine/abstract_type_validation_test.go +++ b/execution/engine/abstract_type_validation_test.go @@ -21,12 +21,14 @@ func TestAbstractTypeValidation(t *testing.T) { fieldName string selection string returnedTypeName string + clientSDL string // overrides the shared client schema when set serviceSDL string query string // overrides the generated single-field query when set responseBody string // overrides the generated subgraph response when set expectedBody string // asserts the exact subgraph request body when set options []executionTestOptions expectedResponse string + expectedError string // expects Execute to fail before resolving when set }{ { name: "interface accepts an implementation", @@ -164,6 +166,46 @@ func TestAbstractTypeValidation(t *testing.T) { responseBody: `{"data":{"nullableInterface":{"id":"classified-secret-42"}}}`, expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["nullableInterface"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableInterface":null}}`, }, + { + // unions have no shared fields, so the injected __typename is the + // only way to match fragments and validate the runtime type + name: "union requests the runtime type from the subgraph", + fieldName: "nullableUnion", + selection: "... on AccessibleNode { id }", + expectedBody: `{"query":"{nullableUnion {__typename ... on AccessibleNode {id}}}"}`, + responseBody: `{"data":{"nullableUnion":{"__typename":"AccessibleNode","id":"1"}}}`, + expectedResponse: `{"data":{"nullableUnion":{"id":"1"}}}`, + }, + { + // the suggested "data returns ObjectBs among the As" case: the client + // only fragments on the accessible member, never selects __typename, + // and the subgraph returns an inaccessible member + name: "union redacts an inaccessible member when the client omits the typename", + fieldName: "nullableUnion", + selection: "... on AccessibleNode { id }", + responseBody: `{"data":{"nullableUnion":{"__typename":"InaccessibleNode","id":"classified-secret-42"}}}`, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["nullableUnion"],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"nullableUnion":null}}`, + }, + { + // list elements are validated purely on the injected typename when + // the client selection is answerable from the interface alone + name: "list redacts an inaccessible element when the client omits the typename", + fieldName: "interfaces", + selection: "id", + expectedBody: `{"query":"{interfaces {__typename id}}"}`, + responseBody: `{"data":{"interfaces":[{"__typename":"AccessibleNode","id":"1"},{"__typename":"InaccessibleNode","id":"2"}]}}`, + expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["interfaces",1],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"interfaces":[{"id":"1"},null]}}`, + }, + { + // in a contract deployment the client-facing schema no longer contains + // inaccessible types at all, so naming one in a fragment must fail + // validation before any subgraph is contacted + name: "fragment on an inaccessible type is rejected before execution", + fieldName: "nullableUnion", + clientSDL: abstractTypeValidationContractSDL, + query: `query { nullableUnion { ... on AccessibleNode { id } ... on InaccessibleNode { id } } }`, + expectedError: `Unknown type "InaccessibleNode"., locations: [], path: [query,nullableUnion,$1InaccessibleNode]`, + }, { name: "union rejects an unknown member with value completion", fieldName: "nullableUnion", @@ -183,6 +225,12 @@ func TestAbstractTypeValidation(t *testing.T) { } for _, tt := range tests { + clientSchema := schema + if tt.clientSDL != "" { + var err error + clientSchema, err = graphql.NewSchemaFromString(tt.clientSDL) + require.NoError(t, err) + } serviceSDL := tt.serviceSDL if serviceSDL == "" { serviceSDL = abstractTypeValidationSDL @@ -196,12 +244,19 @@ func TestAbstractTypeValidation(t *testing.T) { responseBody = `{"data":{"` + tt.fieldName + `":{"__typename":"` + tt.returnedTypeName + `","id":"1"}}}` } + runner := func(testCase ExecutionEngineTestCase, options ...executionTestOptions) func(t *testing.T) { + if tt.expectedError != "" { + return runWithAndCompareError(testCase, tt.expectedError, options...) + } + return runWithoutError(testCase, options...) + } + // the test case is built inside the subtest so the round tripper // captures the subtest's t: require failures from the request-body // assertion must fail the row, not Goexit the parent's goroutine - t.Run(tt.name, runWithoutError( + t.Run(tt.name, runner( ExecutionEngineTestCase{ - schema: schema, + schema: clientSchema, operation: func(t *testing.T) graphql.Request { return graphql.Request{ Query: query, @@ -287,6 +342,35 @@ const abstractTypeValidationSDL = ` union Result = AccessibleNode | InaccessibleNode ` +// abstractTypeValidationSDL as a client would see it in a contract deployment: +// the inaccessible type is removed entirely rather than marked with a directive +const abstractTypeValidationContractSDL = ` + type Query { + interface: Node! + nullableInterface: Node + union: Result! + nullableUnion: Result + interfaces: [Node] + requiredInterfaces: [Node!] + } + + interface Node { + id: ID! + } + + type AccessibleNode implements Node { + id: ID! + friend: Node + } + + type SecondNode implements Node { + id: ID! + friend: Node + } + + union Result = AccessibleNode +` + const abstractTypeValidationSubgraphSDL = abstractTypeValidationSDL + ` type RemovedNode implements Node { id: ID! From cc310b4e1ceedb2055f21ed4304dbf28091723ea Mon Sep 17 00:00:00 2001 From: endigma Date: Thu, 16 Jul 2026 15:33:23 +0100 Subject: [PATCH 7/9] test: un-parallel the runExecutionTest helper causes issues, refactor to fix is probably not worth it --- execution/engine/execution_engine_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/execution/engine/execution_engine_test.go b/execution/engine/execution_engine_test.go index 28e378143c..d4df1d1830 100644 --- a/execution/engine/execution_engine_test.go +++ b/execution/engine/execution_engine_test.go @@ -63,7 +63,6 @@ func mustFactory(t testing.TB, httpClient *http.Client) plan.PlannerFactory[grap func runExecutionTest(testCase ExecutionEngineTestCase, withError bool, expectedErrorMessage string, options ...executionTestOptions) func(t *testing.T) { return func(t *testing.T) { - t.Parallel() t.Helper() if testCase.skipReason != "" { From 37b5eb1323f684a053ac9147d6bf42ddad2c9f3f Mon Sep 17 00:00:00 2001 From: endigma Date: Thu, 16 Jul 2026 15:33:23 +0100 Subject: [PATCH 8/9] test: simplify, fix parallel issue --- .../engine/abstract_type_validation_test.go | 181 ++++++++---------- 1 file changed, 83 insertions(+), 98 deletions(-) diff --git a/execution/engine/abstract_type_validation_test.go b/execution/engine/abstract_type_validation_test.go index b37714afd3..9fb104f702 100644 --- a/execution/engine/abstract_type_validation_test.go +++ b/execution/engine/abstract_type_validation_test.go @@ -21,14 +21,12 @@ func TestAbstractTypeValidation(t *testing.T) { fieldName string selection string returnedTypeName string - clientSDL string // overrides the shared client schema when set serviceSDL string query string // overrides the generated single-field query when set responseBody string // overrides the generated subgraph response when set expectedBody string // asserts the exact subgraph request body when set options []executionTestOptions expectedResponse string - expectedError string // expects Execute to fail before resolving when set }{ { name: "interface accepts an implementation", @@ -196,16 +194,6 @@ func TestAbstractTypeValidation(t *testing.T) { responseBody: `{"data":{"interfaces":[{"__typename":"AccessibleNode","id":"1"},{"__typename":"InaccessibleNode","id":"2"}]}}`, expectedResponse: `{"errors":[{"message":"Subgraph 'AbstractTypes' returned an invalid value for __typename field.","path":["interfaces",1],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"interfaces":[{"id":"1"},null]}}`, }, - { - // in a contract deployment the client-facing schema no longer contains - // inaccessible types at all, so naming one in a fragment must fail - // validation before any subgraph is contacted - name: "fragment on an inaccessible type is rejected before execution", - fieldName: "nullableUnion", - clientSDL: abstractTypeValidationContractSDL, - query: `query { nullableUnion { ... on AccessibleNode { id } ... on InaccessibleNode { id } } }`, - expectedError: `Unknown type "InaccessibleNode"., locations: [], path: [query,nullableUnion,$1InaccessibleNode]`, - }, { name: "union rejects an unknown member with value completion", fieldName: "nullableUnion", @@ -225,12 +213,6 @@ func TestAbstractTypeValidation(t *testing.T) { } for _, tt := range tests { - clientSchema := schema - if tt.clientSDL != "" { - var err error - clientSchema, err = graphql.NewSchemaFromString(tt.clientSDL) - require.NoError(t, err) - } serviceSDL := tt.serviceSDL if serviceSDL == "" { serviceSDL = abstractTypeValidationSDL @@ -244,74 +226,98 @@ func TestAbstractTypeValidation(t *testing.T) { responseBody = `{"data":{"` + tt.fieldName + `":{"__typename":"` + tt.returnedTypeName + `","id":"1"}}}` } - runner := func(testCase ExecutionEngineTestCase, options ...executionTestOptions) func(t *testing.T) { - if tt.expectedError != "" { - return runWithAndCompareError(testCase, tt.expectedError, options...) - } - return runWithoutError(testCase, options...) - } + t.Run(tt.name, func(t *testing.T) { + t.Parallel() - // the test case is built inside the subtest so the round tripper - // captures the subtest's t: require failures from the request-body - // assertion must fail the row, not Goexit the parent's goroutine - t.Run(tt.name, runner( - ExecutionEngineTestCase{ - schema: clientSchema, + tc := ExecutionEngineTestCase{ + schema: schema, operation: func(t *testing.T) graphql.Request { return graphql.Request{ Query: query, } }, dataSources: []plan.DataSource{ - mustGraphqlDataSourceConfigurationWithName( - t, - "abstract-types", - "AbstractTypes", - mustFactory(t, testNetHttpClient(t, roundTripperTestCase{ - expectedHost: "example.com", - expectedPath: "/", - expectedBody: tt.expectedBody, - sendResponseBody: responseBody, - sendStatusCode: 200, - })), - &plan.DataSourceMetadata{ - RootNodes: []plan.TypeField{ - { - TypeName: "Query", - FieldNames: []string{"interface", "nullableInterface", "union", "nullableUnion", "interfaces", "requiredInterfaces"}, - }, - }, - ChildNodes: []plan.TypeField{ - {TypeName: "Node", FieldNames: []string{"id"}}, - {TypeName: "AccessibleNode", FieldNames: []string{"id", "friend"}}, - {TypeName: "SecondNode", FieldNames: []string{"id", "friend"}}, - {TypeName: "InaccessibleNode", FieldNames: []string{"id"}}, - }, - }, - mustConfiguration(t, graphql_datasource.ConfigurationInput{ - Fetch: &graphql_datasource.FetchConfiguration{ - URL: "https://example.com/", - Method: "GET", - }, - SchemaConfiguration: mustSchemaConfig( - t, - &graphql_datasource.FederationConfiguration{ - Enabled: true, - ServiceSDL: serviceSDL, - }, - serviceSDL, - ), - }), - ), + abstractTypeValidationDataSource(t, serviceSDL, tt.expectedBody, responseBody), }, expectedResponse: tt.expectedResponse, - }, - tt.options..., - )) + } + + runWithoutError(tc, tt.options...)(t) + }) } + + // in a contract deployment the client-facing schema no longer contains + // inaccessible types at all, so naming one in a fragment must fail + // validation before any subgraph is contacted; the subgraph SDL still + // defines the type, proving the operation is checked against the contract + // schema rather than the subgraph schema + t.Run("fragment on an inaccessible type is rejected before execution", func(t *testing.T) { + t.Parallel() + + contractSchema, err := graphql.NewSchemaFromString(abstractTypeValidationContractSDL) + require.NoError(t, err) + + runWithAndCompareError( + ExecutionEngineTestCase{ + schema: contractSchema, + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + Query: `query { nullableUnion { ... on AccessibleNode { id } ... on InaccessibleNode { id } } }`, + } + }, + dataSources: []plan.DataSource{ + abstractTypeValidationDataSource(t, abstractTypeValidationSDL, "", ""), + }, + }, + `Unknown type "InaccessibleNode"., locations: [], path: [query,nullableUnion,$1InaccessibleNode]`, + )(t) + }) +} + +func abstractTypeValidationDataSource(t *testing.T, serviceSDL, expectedBody, responseBody string) plan.DataSource { + return mustGraphqlDataSourceConfigurationWithName( + t, + "abstract-types", + "AbstractTypes", + mustFactory(t, testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: expectedBody, + sendResponseBody: responseBody, + sendStatusCode: 200, + })), + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"interface", "nullableInterface", "union", "nullableUnion", "interfaces", "requiredInterfaces"}, + }, + }, + ChildNodes: []plan.TypeField{ + {TypeName: "Node", FieldNames: []string{"id"}}, + {TypeName: "AccessibleNode", FieldNames: []string{"id", "friend"}}, + {TypeName: "SecondNode", FieldNames: []string{"id", "friend"}}, + {TypeName: "InaccessibleNode", FieldNames: []string{"id"}}, + }, + }, + mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "GET", + }, + SchemaConfiguration: mustSchemaConfig( + t, + &graphql_datasource.FederationConfiguration{ + Enabled: true, + ServiceSDL: serviceSDL, + }, + serviceSDL, + ), + }), + ) } -const abstractTypeValidationSDL = ` +const abstractTypeValidationBaseSDL = ` type Query { interface: Node! nullableInterface: Node @@ -334,7 +340,9 @@ const abstractTypeValidationSDL = ` id: ID! friend: Node } +` +const abstractTypeValidationSDL = abstractTypeValidationBaseSDL + ` type InaccessibleNode implements Node @inaccessible { id: ID! } @@ -344,30 +352,7 @@ const abstractTypeValidationSDL = ` // abstractTypeValidationSDL as a client would see it in a contract deployment: // the inaccessible type is removed entirely rather than marked with a directive -const abstractTypeValidationContractSDL = ` - type Query { - interface: Node! - nullableInterface: Node - union: Result! - nullableUnion: Result - interfaces: [Node] - requiredInterfaces: [Node!] - } - - interface Node { - id: ID! - } - - type AccessibleNode implements Node { - id: ID! - friend: Node - } - - type SecondNode implements Node { - id: ID! - friend: Node - } - +const abstractTypeValidationContractSDL = abstractTypeValidationBaseSDL + ` union Result = AccessibleNode ` From 9fd2746035b9ed43ace4e07243611d9f54a536bd Mon Sep 17 00:00:00 2001 From: endigma Date: Fri, 17 Jul 2026 11:32:55 +0100 Subject: [PATCH 9/9] chore: review fixes --- .../engine/execution_engine_cost_test.go | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/execution/engine/execution_engine_cost_test.go b/execution/engine/execution_engine_cost_test.go index aa56fc93d9..86e376c25c 100644 --- a/execution/engine/execution_engine_cost_test.go +++ b/execution/engine/execution_engine_cost_test.go @@ -64,7 +64,8 @@ func TestExecutionEngine_Cost(t *testing.T) { Weights: map[plan.FieldCoordinate]*plan.FieldCost{ {TypeName: "Droid", FieldName: "name"}: {HasWeight: true, Weight: 17}, }, - }}, + }, + }, customConfig, ), }, @@ -120,7 +121,8 @@ func TestExecutionEngine_Cost(t *testing.T) { }, {TypeName: "Droid", FieldName: "name"}: {HasWeight: true, Weight: 17}, }, - }}, + }, + }, customConfig, ), }, @@ -181,7 +183,8 @@ func TestExecutionEngine_Cost(t *testing.T) { Types: map[string]int{ "Droid": -1, // Negative type weight }, - }}, + }, + }, customConfig, ), }, @@ -292,10 +295,6 @@ func TestExecutionEngine_Cost(t *testing.T) { computeCosts(), )) - // Regression test for the abstract field without __typename bug recordObjectTypeStats). - // When the subgraph resolves a single (non-list) abstract field and does NOT return __typename, - // we must still record one occurrence for that field's path, falling back to the declared - // abstract type name in actual costs. t.Run("single abstract field without __typename is rejected and bills nothing", runWithoutError( ExecutionEngineTestCase{ schema: graphql.StarwarsSchema(t), @@ -1046,7 +1045,8 @@ func TestExecutionEngine_Cost(t *testing.T) { Weights: map[plan.FieldCoordinate]*plan.FieldCost{ {TypeName: "Droid", FieldName: "primaryFunction"}: {HasWeight: true, Weight: 17}, }, - }}, + }, + }, customConfig, ), }, @@ -1319,7 +1319,7 @@ func TestExecutionEngine_Cost(t *testing.T) { // friends items carry no __typename, so they are rejected and nulled expectedResponse: `{"errors":[{"message":"Subgraph 'id' returned an invalid value for __typename field.","path":["hero","friends",0],"extensions":{"code":"INVALID_GRAPHQL"}}],"data":{"hero":{"name":"Luke","friends":[null]}}}`, expectedEstimatedCost: intPtr(20), // 2 + 1*(0 + 6*(3 + 1*0)) - expectedActualCost: intPtr(5), // 2 + 1*(0 + 1*(3 + 1*0)) + expectedActualCost: intPtr(5), // 2 + 1*(0 + 1*3) }, computeCosts(), costsIgnoreImplementingTypeWeights(), @@ -1380,7 +1380,6 @@ func TestExecutionEngine_Cost(t *testing.T) { costsIgnoreImplementingTypeWeights(), )) }) - }) t.Run("union types", func(t *testing.T) { @@ -2515,7 +2514,6 @@ func TestExecutionEngine_Cost(t *testing.T) { computeCosts(), )) }) - }) t.Run("nested lists with compounding multipliers", func(t *testing.T) { @@ -5275,7 +5273,6 @@ func TestExecutionEngine_Cost(t *testing.T) { computeCosts(), )) }) - }) t.Run("validate requireOneSlicingArgument on concrete types", func(t *testing.T) { @@ -6172,7 +6169,6 @@ func TestExecutionEngine_Cost(t *testing.T) { "external: field 'Paginated.items' requires exactly one slicing argument, but 2 were provided, locations: [], path: [search,items]", computeCosts(), )) - }) t.Run("input object cost", func(t *testing.T) { @@ -7514,7 +7510,6 @@ func TestExecutionEngine_Cost(t *testing.T) { }, computeCosts(), )) - }) t.Run("fragment fields sharing a response path under an abstract list", func(t *testing.T) {