diff --git a/execution/engine/execution_engine_test.go b/execution/engine/execution_engine_test.go index a96f934d27..ef99785b6c 100644 --- a/execution/engine/execution_engine_test.go +++ b/execution/engine/execution_engine_test.go @@ -843,7 +843,7 @@ func TestExecutionEngine_Execute(t *testing.T) { }, )) - t.Run("execute simple hero operation with propagating to subgraphs fetch reasons", runWithoutError( + t.Run("operation on interface, subgraph expects fetch reasons for all implementing types", runWithoutError( ExecutionEngineTestCase{ schema: graphql.StarwarsSchema(t), operation: graphql.LoadStarWarsQuery(starwars.FileSimpleHeroQuery, nil), @@ -854,7 +854,7 @@ 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":"Query","field":"hero","by_user":true}]}}`, + 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"}}}`, sendStatusCode: 200, }), @@ -862,15 +862,215 @@ func TestExecutionEngine_Execute(t *testing.T) { &plan.DataSourceMetadata{ RootNodes: []plan.TypeField{ { - TypeName: "Query", - FieldNames: []string{"hero"}, - FetchReasonFields: []string{"hero"}, + TypeName: "Query", + FieldNames: []string{"hero"}, + }, + { + TypeName: "Human", + FieldNames: []string{"name", "height", "friends"}, + }, + { + TypeName: "Droid", + FieldNames: []string{"name", "primaryFunctions", "friends"}, + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Character", + FieldNames: []string{"name", "friends"}, + // An interface field implicitly marks all the implementing types. + FetchReasonFields: []string{"name"}, + }, + }, + }, + mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "POST", + }, + SchemaConfiguration: mustSchemaConfig( + t, + nil, + string(graphql.StarwarsSchema(t).RawSchema()), + ), + }), + ), + }, + fields: []plan.FieldConfiguration{}, + expectedResponse: `{"data":{"hero":{"name":"Luke Skywalker"}}}`, + }, + withFetchReasons(), + )) + + t.Run("operation on interface, subgraph expects fetch reasons for one implementing type", runWithoutError( + ExecutionEngineTestCase{ + schema: graphql.StarwarsSchema(t), + operation: graphql.LoadStarWarsQuery(starwars.FileSimpleHeroQuery, nil), + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, + "id", + mustFactory(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"}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"hero"}, + }, + { + TypeName: "Human", + FieldNames: []string{"name", "height", "friends"}, + }, + { + TypeName: "Droid", + FieldNames: []string{"name", "primaryFunctions", "friends"}, + // Only for this field propagate the fetch reasons, + // even if a user has asked for the interface in the query. + FetchReasonFields: []string{"name"}, + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Character", + FieldNames: []string{"name", "friends"}, + }, + }, + }, + mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "POST", + }, + SchemaConfiguration: mustSchemaConfig( + t, + nil, + string(graphql.StarwarsSchema(t).RawSchema()), + ), + }), + ), + }, + fields: []plan.FieldConfiguration{}, + expectedResponse: `{"data":{"hero":{"name":"Droid Number 6"}}}`, + }, + withFetchReasons(), + )) + + t.Run("operation on interface, interface and object marked, subgraph expects fetch reasons for one implementing type", runWithoutError( + ExecutionEngineTestCase{ + schema: graphql.StarwarsSchema(t), + operation: graphql.LoadStarWarsQuery(starwars.FileSimpleHeroQuery, nil), + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, + "id", + mustFactory(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"}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"hero"}, + }, + { + TypeName: "Human", + FieldNames: []string{"name", "height", "friends"}, + }, + { + TypeName: "Droid", + FieldNames: []string{"name", "primaryFunctions", "friends"}, + FetchReasonFields: []string{"name"}, // implementing is marked }, }, ChildNodes: []plan.TypeField{ { TypeName: "Character", - FieldNames: []string{"name"}, + FieldNames: []string{"name", "friends"}, + FetchReasonFields: []string{"name"}, // interface is marked + }, + }, + }, + mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "POST", + }, + SchemaConfiguration: mustSchemaConfig( + t, + nil, + string(graphql.StarwarsSchema(t).RawSchema()), + ), + }), + ), + }, + fields: []plan.FieldConfiguration{}, + expectedResponse: `{"data":{"hero":{"name":"Droid Number 6"}}}`, + }, + withFetchReasons(), + )) + + t.Run("operation on fragment, subgraph expects fetch reasons for one implementing type", runWithoutError( + ExecutionEngineTestCase{ + schema: graphql.StarwarsSchema(t), + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + Query: `query { + hero { + ...humanFields + } + } + fragment humanFields on Human { + name + height + }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, + "id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: `{"query":"{hero {__typename ... on Human {name height}}}","extensions":{"fetch_reasons":[{"typename":"Human","field":"name","by_user":true}]}}`, + sendResponseBody: `{"data":{"hero":{"__typename": "Human", "name":"Luke Skywalker", "height": "1.99"}}}`, + sendStatusCode: 200, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"hero"}, + }, + { + TypeName: "Human", + FieldNames: []string{"name", "height", "friends"}, + }, + { + TypeName: "Droid", + FieldNames: []string{"name", "primaryFunctions", "friends"}, + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Character", + FieldNames: []string{"name", "friends"}, + // Interface is marked, and we should propagate reasons if + // a user has asked for the fragment on concrete type + // that is not marked for fetch reasons. FetchReasonFields: []string{"name"}, }, }, @@ -889,6 +1089,77 @@ func TestExecutionEngine_Execute(t *testing.T) { ), }, fields: []plan.FieldConfiguration{}, + expectedResponse: `{"data":{"hero":{"name":"Luke Skywalker","height":"1.99"}}}`, + }, + withFetchReasons(), + )) + t.Run("operation on fragment, subgraph expects fetch reasons for interface and implementing types without dupes", runWithoutError( + ExecutionEngineTestCase{ + schema: graphql.StarwarsSchema(t), + operation: func(t *testing.T) graphql.Request { + return graphql.Request{ + Query: `query { + hero { + ...humanFields + name + } + } + fragment humanFields on Human { + name + }`, + } + }, + dataSources: []plan.DataSource{ + mustGraphqlDataSourceConfiguration(t, + "id", + mustFactory(t, + testNetHttpClient(t, roundTripperTestCase{ + expectedHost: "example.com", + expectedPath: "/", + expectedBody: `{"query":"{hero {__typename ... on Human {name} 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, + }), + ), + &plan.DataSourceMetadata{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"hero"}, + }, + { + TypeName: "Human", + FieldNames: []string{"name", "height", "friends"}, + FetchReasonFields: []string{"name"}, // implementing is marked + }, + { + TypeName: "Droid", + FieldNames: []string{"name", "primaryFunctions", "friends"}, + FetchReasonFields: []string{"name"}, // implementing is marked + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Character", + FieldNames: []string{"name", "friends"}, + FetchReasonFields: []string{"name"}, // interface is marked + }, + }, + }, + mustConfiguration(t, graphql_datasource.ConfigurationInput{ + Fetch: &graphql_datasource.FetchConfiguration{ + URL: "https://example.com/", + Method: "POST", + }, + SchemaConfiguration: mustSchemaConfig( + t, + nil, + string(graphql.StarwarsSchema(t).RawSchema()), + ), + }), + ), + }, + fields: []plan.FieldConfiguration{}, expectedResponse: `{"data":{"hero":{"name":"Luke Skywalker"}}}`, }, withFetchReasons(), diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index 33ad874822..ef8d094757 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -1348,18 +1348,7 @@ func (v *Visitor) configureFetch(internal *objectFetchConfiguration, external re if len(singleFetch.Info.FetchReasons) == 0 { return singleFetch } - - // Propagate fetch reasons required by the data source. - dsConfig := v.planners[internal.fetchID].DataSourceConfiguration() - lookup := dsConfig.RequireFetchReasons() - propagated := make([]resolve.FetchReason, 0, len(lookup)) - for _, fr := range singleFetch.Info.FetchReasons { - field := FieldCoordinate{fr.TypeName, fr.FieldName} - if _, ok := lookup[field]; ok { - propagated = append(propagated, fr) - } - } - singleFetch.Info.PropagatedFetchReasons = propagated + singleFetch.Info.PropagatedFetchReasons = v.getPropagatedReasons(internal.fetchID, singleFetch.Info.FetchReasons) return singleFetch } @@ -1429,6 +1418,7 @@ func (v *Visitor) buildFetchReasons(fetchID int) []resolve.FetchReason { } reasons := make([]resolve.FetchReason, 0, len(fields)) + // index maps field coordinates to the position in the reason slice index := make(map[FieldCoordinate]int, len(fields)) for _, fieldRef := range fields { @@ -1512,15 +1502,17 @@ func (v *Visitor) buildFetchReasons(fetchID int) []resolve.FetchReason { } } - slices.SortFunc(reasons, func(a, b resolve.FetchReason) int { - return cmp.Or( - cmp.Compare(a.TypeName, b.TypeName), - cmp.Compare(a.FieldName, b.FieldName), - ) - }) + slices.SortFunc(reasons, cmpFetchReasons) return reasons } +func cmpFetchReasons(a, b resolve.FetchReason) int { + return cmp.Or( + cmp.Compare(a.TypeName, b.TypeName), + cmp.Compare(a.FieldName, b.FieldName), + ) +} + // fieldDefinitionRef returns the definition reference of a field in a given type or ast.InvalidRef if not found. func (v *Visitor) fieldDefinitionRef(typeName string, fieldName string) int { node, ok := v.Definition.NodeByNameStr(typeName) @@ -1533,3 +1525,102 @@ func (v *Visitor) fieldDefinitionRef(typeName string, fieldName string) int { } return defRef } + +// getPropagatedReasons collects fetch reasons required by the data source. Only fields +// marked by a special directive are used for propagation (returned by RequireFetchReasons). +// +// Additionally, interfaces were taken care of. When an interface field is marked, +// and a user requests that field in an operation, then fetch reasons for this field will be +// extended with fetch reasons for all the implementing types. +// In general, a marked interface field leads to all implementation's fields being used for propagation. +// +// This method returns deduplicated and sorted results. +func (v *Visitor) getPropagatedReasons(fetchID int, fetchReasons []resolve.FetchReason) []resolve.FetchReason { + dsConfig := v.planners[fetchID].DataSourceConfiguration() + // We should propagate fetch reasons for the coordinates in the lookup map. + lookup := dsConfig.RequireFetchReasons() + propagated := make([]resolve.FetchReason, 0, len(lookup)) + // index maps field coordinates to the position in the propagated slice + index := make(map[FieldCoordinate]int, len(lookup)) + + // appendOrMerge deduplicates and merges fetch reasons with the same + // (TypeName, FieldName) coordinate. + // This is necessary because: + // 1. When both interface and implementing type fields are in fetchReasons, we can add the same + // implementing type field twice (once from the interface, once from the implementing type itself). + // 2. Different entries might have different ByUser, BySubgraphs values that + // need to be merged (similar to buildFetchReasons). + appendOrMerge := func(key FieldCoordinate, reason resolve.FetchReason) { + if i, ok := index[key]; ok { + propagated[i].ByUser = propagated[i].ByUser || reason.ByUser + if len(reason.BySubgraphs) > 0 { + propagated[i].BySubgraphs = append(propagated[i].BySubgraphs, reason.BySubgraphs...) + slices.Sort(propagated[i].BySubgraphs) + propagated[i].BySubgraphs = slices.Compact(propagated[i].BySubgraphs) + propagated[i].IsKey = propagated[i].IsKey || reason.IsKey + propagated[i].IsRequires = propagated[i].IsRequires || reason.IsRequires + } + } else { + propagated = append(propagated, reason) + index[key] = len(propagated) - 1 + } + } + + for _, reason := range fetchReasons { + field := FieldCoordinate{reason.TypeName, reason.FieldName} + _, fieldInLookup := lookup[field] + if fieldInLookup { + appendOrMerge(field, reason) + } + + typeNode, exists := v.Definition.NodeByNameStr(reason.TypeName) + if !exists { + continue + } + + // Special case when the field belongs to an object, and this field is not in the lookup. + // If this object implements an interface that has a corresponding field in the lookup, + // then propagate it. + if typeNode.Kind == ast.NodeKindObjectTypeDefinition && !fieldInLookup { + objectDef := v.Definition.ObjectTypeDefinitions[typeNode.Ref] + for _, interfaceTypeRef := range objectDef.ImplementsInterfaces.Refs { + interfaceTypeName := v.Definition.ResolveTypeNameString(interfaceTypeRef) + interfaceField := FieldCoordinate{interfaceTypeName, reason.FieldName} + if _, ok := lookup[interfaceField]; ok { + appendOrMerge(field, reason) + break + } + } + continue + } + + // Special case when the field belongs to an interface type. + if typeNode.Kind != ast.NodeKindInterfaceTypeDefinition { + continue + } + implementingTypeNames, ok := v.Definition.InterfaceTypeDefinitionImplementedByObjectWithNames(typeNode.Ref) + if !ok { + continue + } + for _, implementingTypeName := range implementingTypeNames { + implementingField := FieldCoordinate{implementingTypeName, reason.FieldName} + _, implementingInLookup := lookup[implementingField] + // 1st case: interface field in the lookup; + // all the implementing fields should be propagated. + // + // 2nd case: interface field is not in the lookup, but the implementing field is; + // only the implementing fields found in the lookup should be propagated. + if fieldInLookup || implementingInLookup { + reasonClone := reason + reasonClone.TypeName = implementingTypeName + if len(reasonClone.BySubgraphs) > 0 { + reasonClone.BySubgraphs = slices.Clone(reasonClone.BySubgraphs) + } + appendOrMerge(implementingField, reasonClone) + } + } + } + + slices.SortFunc(propagated, cmpFetchReasons) + return propagated +} diff --git a/v2/pkg/engine/resolve/fetch.go b/v2/pkg/engine/resolve/fetch.go index d7148c7ada..deeea25a41 100644 --- a/v2/pkg/engine/resolve/fetch.go +++ b/v2/pkg/engine/resolve/fetch.go @@ -359,7 +359,7 @@ type FetchDependencyOrigin struct { IsRequires bool `json:"isRequires"` } -// FetchReason explains who requested a specific (typeName, fieldName) combination. +// FetchReason explains who requested a specific (TypeName, FieldName) coordinate. // A field can be requested by the user and/or by one or more subgraphs, with optional reasons. type FetchReason struct { TypeName string `json:"typename"`