Skip to content

feat: improve abstract field validation #1588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
endigma merged 9 commits into from
Jul 17, 2026
+474 −80
Merged

feat: improve abstract field validation #1588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
632a097
test: abstract type validation
endigma Jul 14, 2026
c9906c1
feat(resolve): validate inaccessible types returned for abstract fields
endigma Jul 14, 2026
6ccece6
test(engine): add test coverage for abstract validation when __typena…
endigma Jul 14, 2026
8d4e5d6
feat(engine): always request and enforce __typename for abstract values
endigma Jul 14, 2026
989839d
test: use runWithoutError directly despite issues
endigma Jul 15, 2026
c8b4bce
test: add suggested tcs
endigma Jul 15, 2026
cc310b4
test: un-parallel the runExecutionTest helper
endigma Jul 16, 2026
37b5eb1
test: simplify, fix parallel issue
endigma Jul 16, 2026
9fd2746
chore: review fixes
endigma Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
365 changes: 365 additions & 0 deletions execution/engine/abstract_type_validation_test.go
View file
Open in desktop

Large diffs are not rendered by default.

55 changes: 28 additions & 27 deletions execution/engine/execution_engine_cost_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
},
Expand Down Expand Up @@ -120,7 +121,8 @@ func TestExecutionEngine_Cost(t *testing.T) {
},
{TypeName: "Droid", FieldName: "name"}: {HasWeight: true, Weight: 17},
},
}},
},
},
customConfig,
),
},
Expand Down Expand Up @@ -181,7 +183,8 @@ func TestExecutionEngine_Cost(t *testing.T) {
Types: map[string]int{
"Droid": -1, // Negative type weight
},
}},
},
},
customConfig,
),
},
Expand Down Expand Up @@ -292,11 +295,7 @@ 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 takes into account implementing types", runWithoutError(
Comment thread
endigma marked this conversation as resolved.
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 {
Expand Down Expand Up @@ -325,7 +324,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(),
))
Expand Down Expand Up @@ -778,7 +778,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
Expand All @@ -787,8 +788,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(),
))
Expand Down Expand Up @@ -1043,7 +1045,8 @@ func TestExecutionEngine_Cost(t *testing.T) {
Weights: map[plan.FieldCoordinate]*plan.FieldCost{
{TypeName: "Droid", FieldName: "primaryFunction"}: {HasWeight: true, Weight: 17},
},
}},
},
},
customConfig,
),
},
Expand Down Expand Up @@ -1313,9 +1316,10 @@ 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))
expectedActualCost: intPtr(5), // 2 + 1*(0 + 1*3)
},
computeCosts(),
costsIgnoreImplementingTypeWeights(),
Expand Down Expand Up @@ -1376,7 +1380,6 @@ func TestExecutionEngine_Cost(t *testing.T) {
costsIgnoreImplementingTypeWeights(),
))
})

})

t.Run("union types", func(t *testing.T) {
Expand Down Expand Up @@ -2511,7 +2514,6 @@ func TestExecutionEngine_Cost(t *testing.T) {
computeCosts(),
))
})

})

t.Run("nested lists with compounding multipliers", func(t *testing.T) {
Expand Down Expand Up @@ -5271,7 +5273,6 @@ func TestExecutionEngine_Cost(t *testing.T) {
computeCosts(),
))
})

})

t.Run("validate requireOneSlicingArgument on concrete types", func(t *testing.T) {
Expand Down Expand Up @@ -6168,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) {
Expand Down Expand Up @@ -7473,7 +7473,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 {
Expand All @@ -7498,17 +7498,18 @@ 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(),
))

})

t.Run("fragment fields sharing a response path under an abstract list", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion execution/engine/execution_engine_helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Comment thread
endigma marked this conversation as resolved.
}

body := bytes.NewBuffer([]byte(testCase.sendResponseBody))
Expand Down
27 changes: 13 additions & 14 deletions execution/engine/execution_engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Comment thread
endigma marked this conversation as resolved.
t.Helper()

if testCase.skipReason != "" {
Expand Down Expand Up @@ -805,7 +804,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,
}),
),
Expand Down Expand Up @@ -910,7 +909,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,
}),
),
Expand Down Expand Up @@ -957,8 +956,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,
}),
),
Expand Down Expand Up @@ -1016,8 +1015,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,
}),
),
Expand Down Expand Up @@ -1076,8 +1075,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,
}),
),
Expand Down Expand Up @@ -1280,7 +1279,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,
}),
),
Expand Down Expand Up @@ -1332,7 +1331,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,
}),
),
Expand Down Expand Up @@ -2305,7 +2304,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,
}),
Expand Down Expand Up @@ -2436,7 +2435,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,
}),
),
Expand Down Expand Up @@ -2484,7 +2483,7 @@ func TestExecutionEngine_Execute(t *testing.T) {
),
},
fields: []plan.FieldConfiguration{},
expectedResponse: `{"data":{"searchResults":[{},{}]}}`,
expectedResponse: `{"data":{"searchResults":[{"name":"Luke Skywalker"},{"length":13.37}]}}`,
},
))

Expand Down
10 changes: 7 additions & 3 deletions execution/engine/testdata/complex_nesting_query_with_art.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -111,6 +111,7 @@
{
"__typename": "Purchase",
"wallet": {
"__typename": "WalletType1",
"currency": "USD"
}
},
Expand All @@ -125,6 +126,7 @@
{
"__typename": "Purchase",
"wallet": {
"__typename": "WalletType2",
"currency": "USD"
}
}
Expand Down Expand Up @@ -155,13 +157,13 @@
"status": "200 OK",
"headers": {
"Content-Length": [
"277"
"331"
],
"Content-Type": [
"application/json"
]
},
"body_size": 277
"body_size": 331
}
}
}
Expand Down Expand Up @@ -387,6 +389,7 @@
{
"__typename": "Purchase",
"wallet": {
"__typename": "WalletType1",
"currency": "USD"
}
},
Expand All @@ -401,6 +404,7 @@
{
"__typename": "Purchase",
"wallet": {
"__typename": "WalletType2",
"currency": "USD"
}
}
Expand Down
19 changes: 6 additions & 13 deletions v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...ne/datasource/graphql_datasource/graphql_datasource_federation_interface_provides_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
2 changes: 1 addition & 1 deletion v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
},
Expand Down
Loading
Loading