From ef22fd26aaa3ef3e29210b4b1a69b40878776c38 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Tue, 8 Jul 2025 11:56:24 +0200 Subject: [PATCH 1/4] feat: implement resolving fetch dependencies --- .../engine/federation_integration_test.go | 10 ++ execution/engine/graphql_client_test.go | 12 ++ .../queries/recursive_fragment.graphql | 12 ++ .../graphql_datasource_federation_test.go | 157 +++++++++++++++++- .../datasourcetesting/datasourcetesting.go | 20 ++- v2/pkg/engine/plan/configuration.go | 3 +- v2/pkg/engine/plan/node_selection_builder.go | 11 ++ v2/pkg/engine/plan/node_selection_visitor.go | 16 ++ v2/pkg/engine/plan/planner.go | 2 + v2/pkg/engine/plan/visitor.go | 96 +++++++++++ v2/pkg/engine/resolve/fetch.go | 31 ++++ 11 files changed, 360 insertions(+), 10 deletions(-) create mode 100644 execution/federationtesting/testdata/queries/recursive_fragment.graphql diff --git a/execution/engine/federation_integration_test.go b/execution/engine/federation_integration_test.go index 20d55d8474..baaa60ff50 100644 --- a/execution/engine/federation_integration_test.go +++ b/execution/engine/federation_integration_test.go @@ -354,6 +354,16 @@ func TestFederationIntegrationTest(t *testing.T) { assert.Equal(t, compact(expected), string(resp)) }) + t.Run("recursive fragment", func(t *testing.T) { + setup := federationtesting.NewFederationSetup(addGateway(false)) + t.Cleanup(setup.Close) + gqlClient := NewGraphqlClient(http.DefaultClient) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + resp := gqlClient.QueryStatusCode(ctx, setup.GatewayServer.URL, testQueryPath("queries/recursive_fragment.graphql"), nil, http.StatusInternalServerError, t) + assert.Len(t, resp, 0) + }) + t.Run("Union response type with interface fragments", func(t *testing.T) { setup := federationtesting.NewFederationSetup(addGateway(false)) t.Cleanup(setup.Close) diff --git a/execution/engine/graphql_client_test.go b/execution/engine/graphql_client_test.go index ac1abf4098..0bf88e6103 100644 --- a/execution/engine/graphql_client_test.go +++ b/execution/engine/graphql_client_test.go @@ -73,6 +73,18 @@ func (g *GraphqlClient) Query(ctx context.Context, addr, queryFilePath string, v return responseBodyBytes } +func (g *GraphqlClient) QueryStatusCode(ctx context.Context, addr, queryFilePath string, variables queryVariables, expectedStatusCode int, t *testing.T) []byte { + reqBody := loadQuery(t, queryFilePath, variables) + req, err := http.NewRequest(http.MethodPost, addr, bytes.NewBuffer(reqBody)) + require.NoError(t, err) + req = req.WithContext(ctx) + resp, err := g.httpClient.Do(req) + require.NoError(t, err) + responseBodyBytes, err := io.ReadAll(resp.Body) + require.NoError(t, err) + return responseBodyBytes +} + func (g *GraphqlClient) Subscription(ctx context.Context, addr, queryFilePath string, variables queryVariables, t *testing.T) chan []byte { messageCh := make(chan []byte) diff --git a/execution/federationtesting/testdata/queries/recursive_fragment.graphql b/execution/federationtesting/testdata/queries/recursive_fragment.graphql new file mode 100644 index 0000000000..f5bf3e2d2e --- /dev/null +++ b/execution/federationtesting/testdata/queries/recursive_fragment.graphql @@ -0,0 +1,12 @@ +query RecursiveFragmentQuery { + ...FragmentA +} + +fragment FragmentA on Query { + ...FragmentB +} + +fragment FragmentB on Query { + ...FragmentA +} + 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 65df7c6479..d9f5386667 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 @@ -3097,8 +3097,44 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://address-enricher.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Address {__typename country city}}}","variables":{"representations":[$$0$$]}}}`, - DataSource: &Source{}, + Input: `{"method":"POST","url":"http://address-enricher.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Address {__typename country city}}}","variables":{"representations":[$$0$$]}}}`, + DataSource: &Source{}, + Dependencies: []resolve.FetchDependency{ + { + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "country", + }, + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + Subgraph: "user.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "id", + }, + IsKey: true, + }, + }, + }, + { + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "city", + }, + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + Subgraph: "user.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "id", + }, + IsKey: true, + }, + }, + }, + }, PostProcessing: SingleEntityPostProcessingConfiguration, RequiresEntityFetch: true, Variables: []resolve.Variable{ @@ -3138,6 +3174,60 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSource: &Source{}, PostProcessing: SingleEntityPostProcessingConfiguration, RequiresEntityFetch: true, + Dependencies: []resolve.FetchDependency{ + { + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "line3", + }, + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + Subgraph: "user.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "id", + }, + IsKey: true, + }, + }, + }, + { + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "zip", + }, + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 1, + Subgraph: "address-enricher.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "country", + }, + IsRequires: true, + }, + { + FetchID: 1, + Subgraph: "address-enricher.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "city", + }, + IsRequires: true, + }, + { + FetchID: 0, + Subgraph: "user.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "id", + }, + IsKey: true, + }, + }, + }, + }, Variables: []resolve.Variable{ &resolve.ResolvableObjectVariable{ Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ @@ -3185,9 +3275,65 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Address {__typename fullAddress}}}","variables":{"representations":[$$0$$]}}}`, - DataSource: &Source{}, - PostProcessing: SingleEntityPostProcessingConfiguration, + Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Address {__typename fullAddress}}}","variables":{"representations":[$$0$$]}}}`, + DataSource: &Source{}, + PostProcessing: SingleEntityPostProcessingConfiguration, + Dependencies: []resolve.FetchDependency{ + { + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "fullAddress", + }, + IsUserRequested: true, + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + Subgraph: "user.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "line1", + }, + IsRequires: true, + }, + { + FetchID: 0, + Subgraph: "user.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "line2", + }, + IsRequires: true, + }, + { + FetchID: 2, + Subgraph: "address.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "line3", + }, + IsRequires: true, + }, + { + FetchID: 2, + Subgraph: "address.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "zip", + }, + IsRequires: true, + }, + { + FetchID: 0, + Subgraph: "user.service", + Coordinate: resolve.GraphCoordinate{ + TypeName: "Address", + FieldName: "id", + }, + IsKey: true, + }, + }, + }, + }, RequiresEntityFetch: true, Variables: []resolve.Variable{ &resolve.ResolvableObjectVariable{ @@ -3328,6 +3474,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, WithDefaultPostProcessor(), + WithFieldDependencies(), ) }) diff --git a/v2/pkg/engine/datasourcetesting/datasourcetesting.go b/v2/pkg/engine/datasourcetesting/datasourcetesting.go index fc26cdf103..594b394ead 100644 --- a/v2/pkg/engine/datasourcetesting/datasourcetesting.go +++ b/v2/pkg/engine/datasourcetesting/datasourcetesting.go @@ -28,10 +28,11 @@ import ( ) type testOptions struct { - postProcessors []*postprocess.Processor - skipReason string - withFieldInfo bool - withPrintPlan bool + postProcessors []*postprocess.Processor + skipReason string + withFieldInfo bool + withPrintPlan bool + withFieldDependencies bool } func WithPostProcessors(postProcessors ...*postprocess.Processor) func(*testOptions) { @@ -67,6 +68,12 @@ func WithPrintPlan() func(*testOptions) { } } +func WithFieldDependencies() func(*testOptions) { + return func(o *testOptions) { + o.withFieldDependencies = true + } +} + func RunWithPermutations(t *testing.T, definition, operation, operationName string, expectedPlan plan.Plan, config plan.Configuration, options ...func(*testOptions)) { t.Helper() @@ -125,6 +132,7 @@ func RunTestWithVariables(definition, operation, operationName, variables string // by default, we don't want to have field info in the tests because it's too verbose config.DisableIncludeInfo = true + config.DisableIncludeFieldDependencies = true opts := &testOptions{} for _, o := range options { @@ -135,6 +143,10 @@ func RunTestWithVariables(definition, operation, operationName, variables string config.DisableIncludeInfo = false } + if opts.withFieldDependencies { + config.DisableIncludeFieldDependencies = false + } + if opts.skipReason != "" { t.Skip(opts.skipReason) } diff --git a/v2/pkg/engine/plan/configuration.go b/v2/pkg/engine/plan/configuration.go index a9a38eaccf..c17c869c7a 100644 --- a/v2/pkg/engine/plan/configuration.go +++ b/v2/pkg/engine/plan/configuration.go @@ -27,7 +27,8 @@ type Configuration struct { MinifySubgraphOperations bool - DisableIncludeInfo bool + DisableIncludeInfo bool + DisableIncludeFieldDependencies bool } type DebugConfiguration struct { diff --git a/v2/pkg/engine/plan/node_selection_builder.go b/v2/pkg/engine/plan/node_selection_builder.go index 04686adcef..552af2ed15 100644 --- a/v2/pkg/engine/plan/node_selection_builder.go +++ b/v2/pkg/engine/plan/node_selection_builder.go @@ -19,12 +19,21 @@ type NodeSelectionBuilder struct { nodeSelectionsVisitor *nodeSelectionVisitor } +type fieldDependencyKind int + +const ( + fieldDependencyKindKey fieldDependencyKind = iota + fieldDependencyKindRequires +) + type NodeSelectionResult struct { dataSources []DataSource // data sources configurations, which used by the current operation nodeSuggestions *NodeSuggestions // nodeSuggestions holds information about suggested data sources for each field fieldDependsOn map[fieldIndexKey][]int // fieldDependsOn is a map[fieldIndexKey][]fieldRef - holds list of field refs which are required by a field ref, e.g. field should be planned only after required fields were planned fieldRequirementsConfigs map[fieldIndexKey][]FederationFieldConfiguration // fieldRequirementsConfigs is a map[fieldIndexKey]FederationFieldConfiguration - holds a list of required configuratuibs for a field ref to later built representation variables skipFieldsRefs []int // skipFieldsRefs holds required field refs added by planner and should not be added to user response + fieldRefDependsOn map[int][]int + fieldDependencyKind map[fieldDependencyKey]fieldDependencyKind } func NewNodeSelectionBuilder(config *Configuration) *NodeSelectionBuilder { @@ -170,6 +179,8 @@ func (p *NodeSelectionBuilder) SelectNodes(operation, definition *ast.Document, fieldDependsOn: p.nodeSelectionsVisitor.fieldDependsOn, fieldRequirementsConfigs: p.nodeSelectionsVisitor.fieldRequirementsConfigs, skipFieldsRefs: p.nodeSelectionsVisitor.skipFieldsRefs, + fieldRefDependsOn: p.nodeSelectionsVisitor.fieldRefDependsOn, + fieldDependencyKind: p.nodeSelectionsVisitor.fieldDependencyKind, } } diff --git a/v2/pkg/engine/plan/node_selection_visitor.go b/v2/pkg/engine/plan/node_selection_visitor.go index c12490a4da..b87d273833 100644 --- a/v2/pkg/engine/plan/node_selection_visitor.go +++ b/v2/pkg/engine/plan/node_selection_visitor.go @@ -38,6 +38,8 @@ type nodeSelectionVisitor struct { fieldRefDependsOn map[int][]int // fieldRefDependsOn is a map[fieldRef][]fieldRef - holds list of field refs which are required by a field ref, it is a second index without datasource hash fieldRequirementsConfigs map[fieldIndexKey][]FederationFieldConfiguration // fieldRequirementsConfigs is a map[fieldIndexKey]FederationFieldConfiguration - holds a list of required configuratuibs for a field ref to later built representation variables fieldLandedTo map[int]DSHash // fieldLandedTo is a map[fieldRef]DSHash - holds a datasource hash where field was landed to + fieldEnclosingTypeNames map[int]string + fieldDependencyKind map[fieldDependencyKey]fieldDependencyKind secondaryRun bool // secondaryRun is a flag to indicate that we're running the nodeSelectionVisitor not the first time hasNewFields bool // hasNewFields is used to determine if we need to run the planner again. It will be true in case required fields were added @@ -47,6 +49,10 @@ type nodeSelectionVisitor struct { rewrittenFieldRefs []int } +type fieldDependencyKey struct { + field, dependsOn int +} + func (c *nodeSelectionVisitor) shouldRevisit() bool { return c.hasNewFields || c.hasUnresolvedFields } @@ -142,6 +148,7 @@ func (c *nodeSelectionVisitor) EnterDocument(operation, definition *ast.Document c.visitedFieldsKeyChecks = make(map[fieldIndexKey]struct{}) c.pendingKeyRequirements = make(map[int]pendingKeyRequirements) c.pendingFieldRequirements = make(map[int]pendingFieldRequirements) + c.fieldDependencyKind = make(map[fieldDependencyKey]fieldDependencyKind) c.fieldDependsOn = make(map[fieldIndexKey][]int) c.fieldRefDependsOn = make(map[int][]int) @@ -502,6 +509,9 @@ func (c *nodeSelectionVisitor) addFieldRequirementsToOperation(selectionSetRef i RemappedPaths: addFieldsResult.remappedPaths, } c.fieldRequirementsConfigs[fieldKey] = append(c.fieldRequirementsConfigs[fieldKey], fieldConfiguration) + for _, requiredFieldRef := range addFieldsResult.requiredFieldRefs { + c.fieldDependencyKind[fieldDependencyKey{field: requestedByFieldRef, dependsOn: requiredFieldRef}] = fieldDependencyKindRequires + } } c.hasNewFields = true @@ -590,6 +600,9 @@ func (c *nodeSelectionVisitor) addKeyRequirementsToOperation(selectionSetRef int TypeName: previousJump.TypeName, SelectionSet: previousJump.SelectionSet, }) + for _, requiredFieldRef := range currentFieldRefs { + c.fieldDependencyKind[fieldDependencyKey{field: requestedByFieldRef, dependsOn: requiredFieldRef}] = fieldDependencyKindKey + } } } currentFieldRefs = addFieldsResult.requiredFieldRefs @@ -610,6 +623,9 @@ func (c *nodeSelectionVisitor) addKeyRequirementsToOperation(selectionSetRef int TypeName: jump.TypeName, SelectionSet: jump.SelectionSet, }) + for _, requiredFieldRef := range addFieldsResult.requiredFieldRefs { + c.fieldDependencyKind[fieldDependencyKey{field: requestedByFieldRef, dependsOn: requiredFieldRef}] = fieldDependencyKindKey + } } } diff --git a/v2/pkg/engine/plan/planner.go b/v2/pkg/engine/plan/planner.go index 6ba2056e23..265fefcd64 100644 --- a/v2/pkg/engine/plan/planner.go +++ b/v2/pkg/engine/plan/planner.go @@ -149,6 +149,8 @@ func (p *Planner) Plan(operation, definition *ast.Document, operationName string p.planningVisitor.planners = plannersConfigurations p.planningVisitor.Config = p.config p.planningVisitor.skipFieldsRefs = selectionsConfig.skipFieldsRefs + p.planningVisitor.fieldRefDependsOnFieldRefs = selectionsConfig.fieldRefDependsOn + p.planningVisitor.fieldDependencyKind = selectionsConfig.fieldDependencyKind p.planningWalker.ResetVisitors() p.planningWalker.SetVisitorFilter(p.planningVisitor) diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index f1a89c8f3b..4129c99755 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -43,6 +43,8 @@ type Visitor struct { currentField *resolve.Field planners []PlannerConfiguration skipFieldsRefs []int + fieldRefDependsOnFieldRefs map[int][]int + fieldDependencyKind map[fieldDependencyKey]fieldDependencyKind fieldConfigs map[int]*FieldConfiguration exportedVariables map[string]struct{} skipIncludeOnFragments map[int]skipIncludeInfo @@ -50,6 +52,15 @@ type Visitor struct { includeQueryPlans bool indirectInterfaceFields map[int]indirectInterfaceField pathCache map[astvisitor.VisitorKind]map[int]string + + // plannerFields tells which fields are planned on which planners + // map plannerID -> []fieldRef + plannerFields map[int][]int + // fieldPlanners tells which planners a field was planned on + // map fieldRef -> []plannerID + fieldPlanners map[int][]int + // fieldEnclosingTypeNames stores the enclosing type names for each field ref + fieldEnclosingTypeNames map[int]string } type indirectInterfaceField struct { @@ -191,6 +202,24 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any } } + if !v.Config.DisableIncludeFieldDependencies && kind == astvisitor.LeaveField { + // we don't need to do this twice, so we only do it on leave + + // store which fields are planned on which planners + if v.plannerFields[visitorID] == nil { + v.plannerFields[visitorID] = []int{ref} + } else { + v.plannerFields[visitorID] = append(v.plannerFields[visitorID], ref) + } + + // store which planners a field was planned on + if v.fieldPlanners[ref] == nil { + v.fieldPlanners[ref] = []int{visitorID} + } else { + v.fieldPlanners[ref] = append(v.fieldPlanners[ref], visitorID) + } + } + return shouldWalkFieldsOnPath case astvisitor.EnterInlineFragment, astvisitor.LeaveInlineFragment: // we allow visiting inline fragments only if particular planner has path for the fragment @@ -308,6 +337,9 @@ func (v *Visitor) LeaveSelectionSet(ref int) { func (v *Visitor) EnterField(ref int) { v.debugOnEnterNode(ast.NodeKindField, ref) + if !v.Config.DisableIncludeFieldDependencies { + v.fieldEnclosingTypeNames[ref] = strings.Clone(v.Walker.EnclosingTypeDefinition.NameString(v.Definition)) + } // check if we have to skip the field in the response // it means it was requested by the planner not the user if v.skipField(ref) { @@ -1026,6 +1058,9 @@ func (v *Visitor) EnterDocument(operation, definition *ast.Document) { v.skipIncludeOnFragments = map[int]skipIncludeInfo{} v.indirectInterfaceFields = map[int]indirectInterfaceField{} v.pathCache = map[astvisitor.VisitorKind]map[int]string{} + v.plannerFields = map[int][]int{} + v.fieldPlanners = map[int][]int{} + v.fieldEnclosingTypeNames = map[int]string{} } func (v *Visitor) LeaveDocument(_, _ *ast.Document) { @@ -1291,5 +1326,66 @@ func (v *Visitor) configureFetch(internal *objectFetchConfiguration, external re } } + if !v.Config.DisableIncludeFieldDependencies { + singleFetch.FetchConfiguration.Dependencies = v.resolveFetchDependencies(internal.fetchID) + } + return singleFetch } + +func (v *Visitor) resolveFetchDependencies(fetchID int) []resolve.FetchDependency { + fields, ok := v.plannerFields[fetchID] + if !ok { + return nil + } + dependencies := make([]resolve.FetchDependency, 0, len(fields)) + for _, fieldRef := range fields { + // skipField returns true if the field is a dependency and should not be included in the response + // consequently, if we don't skip the field, the client requested it in the query + userRequestedField := !v.skipField(fieldRef) + deps, ok := v.fieldRefDependsOnFieldRefs[fieldRef] + if !ok { + continue + } + dependency := resolve.FetchDependency{ + Coordinate: resolve.GraphCoordinate{ + FieldName: strings.Clone(v.Operation.FieldNameString(fieldRef)), + TypeName: v.fieldEnclosingTypeNames[fieldRef], + }, + IsUserRequested: userRequestedField, + } + for _, depFieldRef := range deps { + depFieldPlanners, ok := v.fieldPlanners[depFieldRef] + if !ok { + continue + } + for _, fetchID := range depFieldPlanners { // planner index == fetchID + ofc := v.planners[fetchID].ObjectFetchConfiguration() + if ofc == nil { + continue + } + origin := resolve.FetchDependencyOrigin{ + FetchID: fetchID, + Subgraph: ofc.sourceName, + Coordinate: resolve.GraphCoordinate{ + FieldName: strings.Clone(v.Operation.FieldNameString(depFieldRef)), + TypeName: v.fieldEnclosingTypeNames[depFieldRef], + }, + } + dependencyKind, ok := v.fieldDependencyKind[fieldDependencyKey{field: fieldRef, dependsOn: depFieldRef}] + if !ok { + continue + } + switch dependencyKind { + case fieldDependencyKindKey: + origin.IsKey = true + case fieldDependencyKindRequires: + origin.IsRequires = true + } + dependency.DependsOn = append(dependency.DependsOn, origin) + } + } + dependencies = append(dependencies, dependency) + } + return dependencies +} diff --git a/v2/pkg/engine/resolve/fetch.go b/v2/pkg/engine/resolve/fetch.go index 5a0efbebc2..3ef9a4759f 100644 --- a/v2/pkg/engine/resolve/fetch.go +++ b/v2/pkg/engine/resolve/fetch.go @@ -284,6 +284,10 @@ type FetchConfiguration struct { // Returning null in this case tells the batch implementation to skip this item SetTemplateOutputToNullOnVariableNull bool QueryPlan *QueryPlan + // Dependencies contain a list of GraphCoordinates (typeName+fieldName) and which fields from other fetches they depend on + // This information is useful to understand why a fetch depends on other fetches, + // and how multiple dependencies lead to a chain of fetches + Dependencies []FetchDependency } func (fc *FetchConfiguration) Equals(other *FetchConfiguration) bool { @@ -317,6 +321,33 @@ func (fc *FetchConfiguration) Equals(other *FetchConfiguration) bool { return true } +// FetchDependency explains how a GraphCoordinate depends on other GraphCoordinates from other fetches +type FetchDependency struct { + // Coordinate is the type+field which depends on one or more FetchDependencyOrigin + Coordinate GraphCoordinate + // IsUserRequested is true if the field was requested by the user/client + // If false, this indicates that the Coordinate is a dependency for another fetch + IsUserRequested bool + // DependsOn are the FetchDependencyOrigins the Coordinate depends on + DependsOn []FetchDependencyOrigin +} + +// FetchDependencyOrigin defines a GraphCoordinate on a FetchID that another Coordinate depends on +// In addition, it contains information on the Subgraph providing the field, +// and if the Coordinate is a @key or a @requires field dependency +type FetchDependencyOrigin struct { + // FetchID is the fetch id providing the Coordinate + FetchID int + // Subgraph is the subgraph providing the Coordinate + Subgraph string + // Coordinate is the GraphCoordinate that another Coordinate depends on + Coordinate GraphCoordinate + // IsKey is true if the Coordinate is a @key dependency + IsKey bool + // IsRequires is true if the Coordinate is a @requires dependency + IsRequires bool +} + type FetchInfo struct { DataSourceID string DataSourceName string From 50d16cd21d621b9f9904d6644d8c6293395de7fa Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Tue, 8 Jul 2025 12:50:33 +0200 Subject: [PATCH 2/4] chore: cleanup --- v2/pkg/engine/plan/visitor.go | 2 +- .../create_concrete_single_fetch_types.go | 10 +-- v2/pkg/engine/resolve/fetch.go | 64 ++++++++++++------- v2/pkg/engine/resolve/fetchtree.go | 20 +++--- 4 files changed, 60 insertions(+), 36 deletions(-) diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index 4129c99755..239e1666e9 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -1327,7 +1327,7 @@ func (v *Visitor) configureFetch(internal *objectFetchConfiguration, external re } if !v.Config.DisableIncludeFieldDependencies { - singleFetch.FetchConfiguration.Dependencies = v.resolveFetchDependencies(internal.fetchID) + singleFetch.FetchConfiguration.CoordinateDependencies = v.resolveFetchDependencies(internal.fetchID) } return singleFetch diff --git a/v2/pkg/engine/postprocess/create_concrete_single_fetch_types.go b/v2/pkg/engine/postprocess/create_concrete_single_fetch_types.go index f5d0b2ae26..c8a0a17e15 100644 --- a/v2/pkg/engine/postprocess/create_concrete_single_fetch_types.go +++ b/v2/pkg/engine/postprocess/create_concrete_single_fetch_types.go @@ -75,8 +75,9 @@ func (d *createConcreteSingleFetchTypes) createEntityBatchFetch(fetch *resolve.S } return &resolve.BatchEntityFetch{ - FetchDependencies: fetch.FetchDependencies, - Info: fetch.Info, + FetchDependencies: fetch.FetchDependencies, + CoordinateDependencies: fetch.CoordinateDependencies, + Info: fetch.Info, Input: resolve.BatchInput{ Header: resolve.InputTemplate{ Segments: fetch.InputTemplate.Segments[:representationsVariableIndex], @@ -120,8 +121,9 @@ func (d *createConcreteSingleFetchTypes) createEntityFetch(fetch *resolve.Single } return &resolve.EntityFetch{ - FetchDependencies: fetch.FetchDependencies, - Info: fetch.Info, + FetchDependencies: fetch.FetchDependencies, + CoordinateDependencies: fetch.CoordinateDependencies, + Info: fetch.Info, Input: resolve.EntityInput{ Header: resolve.InputTemplate{ Segments: fetch.InputTemplate.Segments[:representationsVariableIndex], diff --git a/v2/pkg/engine/resolve/fetch.go b/v2/pkg/engine/resolve/fetch.go index 3ef9a4759f..9979256c66 100644 --- a/v2/pkg/engine/resolve/fetch.go +++ b/v2/pkg/engine/resolve/fetch.go @@ -155,12 +155,13 @@ func (_ *SingleFetch) FetchKind() FetchKind { // representations variable will contain multiple items according to amount of entities matching this query type BatchEntityFetch struct { FetchDependencies - Input BatchInput - DataSource DataSource - PostProcessing PostProcessingConfiguration - DataSourceIdentifier []byte - Trace *DataSourceLoadTrace - Info *FetchInfo + Input BatchInput + DataSource DataSource + PostProcessing PostProcessingConfiguration + DataSourceIdentifier []byte + Trace *DataSourceLoadTrace + Info *FetchInfo + CoordinateDependencies []FetchDependency } func (b *BatchEntityFetch) Dependencies() FetchDependencies { @@ -197,12 +198,13 @@ func (_ *BatchEntityFetch) FetchKind() FetchKind { // representations variable will contain single item type EntityFetch struct { FetchDependencies - Input EntityInput - DataSource DataSource - PostProcessing PostProcessingConfiguration - DataSourceIdentifier []byte - Trace *DataSourceLoadTrace - Info *FetchInfo + CoordinateDependencies []FetchDependency + Input EntityInput + DataSource DataSource + PostProcessing PostProcessingConfiguration + DataSourceIdentifier []byte + Trace *DataSourceLoadTrace + Info *FetchInfo } func (e *EntityFetch) Dependencies() FetchDependencies { @@ -284,10 +286,10 @@ type FetchConfiguration struct { // Returning null in this case tells the batch implementation to skip this item SetTemplateOutputToNullOnVariableNull bool QueryPlan *QueryPlan - // Dependencies contain a list of GraphCoordinates (typeName+fieldName) and which fields from other fetches they depend on + // CoordinateDependencies contain a list of GraphCoordinates (typeName+fieldName) and which fields from other fetches they depend on // This information is useful to understand why a fetch depends on other fetches, // and how multiple dependencies lead to a chain of fetches - Dependencies []FetchDependency + CoordinateDependencies []FetchDependency } func (fc *FetchConfiguration) Equals(other *FetchConfiguration) bool { @@ -317,19 +319,35 @@ func (fc *FetchConfiguration) Equals(other *FetchConfiguration) bool { if fc.SetTemplateOutputToNullOnVariableNull != other.SetTemplateOutputToNullOnVariableNull { return false } - + if !slices.EqualFunc(fc.CoordinateDependencies, other.CoordinateDependencies, func(a, b FetchDependency) bool { + if a.Coordinate != b.Coordinate { + return false + } + if a.IsUserRequested != b.IsUserRequested { + return false + } + return slices.EqualFunc(a.DependsOn, b.DependsOn, func(x, y FetchDependencyOrigin) bool { + return x.FetchID == y.FetchID && + x.Subgraph == y.Subgraph && + x.Coordinate == y.Coordinate && + x.IsKey == y.IsKey && + x.IsRequires == y.IsRequires + }) + }) { + return false + } return true } // FetchDependency explains how a GraphCoordinate depends on other GraphCoordinates from other fetches type FetchDependency struct { // Coordinate is the type+field which depends on one or more FetchDependencyOrigin - Coordinate GraphCoordinate + Coordinate GraphCoordinate `json:"coordinate"` // IsUserRequested is true if the field was requested by the user/client // If false, this indicates that the Coordinate is a dependency for another fetch - IsUserRequested bool + IsUserRequested bool `json:"isUserRequested"` // DependsOn are the FetchDependencyOrigins the Coordinate depends on - DependsOn []FetchDependencyOrigin + DependsOn []FetchDependencyOrigin `json:"dependsOn"` } // FetchDependencyOrigin defines a GraphCoordinate on a FetchID that another Coordinate depends on @@ -337,15 +355,15 @@ type FetchDependency struct { // and if the Coordinate is a @key or a @requires field dependency type FetchDependencyOrigin struct { // FetchID is the fetch id providing the Coordinate - FetchID int + FetchID int `json:"fetchId"` // Subgraph is the subgraph providing the Coordinate - Subgraph string + Subgraph string `json:"subgraph"` // Coordinate is the GraphCoordinate that another Coordinate depends on - Coordinate GraphCoordinate + Coordinate GraphCoordinate `json:"coordinate"` // IsKey is true if the Coordinate is a @key dependency - IsKey bool + IsKey bool `json:"isKey"` // IsRequires is true if the Coordinate is a @requires dependency - IsRequires bool + IsRequires bool `json:"isRequires"` } type FetchInfo struct { diff --git a/v2/pkg/engine/resolve/fetchtree.go b/v2/pkg/engine/resolve/fetchtree.go index cdea7029f4..a1633723da 100644 --- a/v2/pkg/engine/resolve/fetchtree.go +++ b/v2/pkg/engine/resolve/fetchtree.go @@ -162,14 +162,15 @@ type FetchTreeQueryPlanNode struct { } type FetchTreeQueryPlan struct { - Kind string `json:"kind"` - Path string `json:"path,omitempty"` - SubgraphName string `json:"subgraphName"` - SubgraphID string `json:"subgraphId"` - FetchID int `json:"fetchId"` - DependsOnFetchIDs []int `json:"dependsOnFetchIds,omitempty"` - Representations []Representation `json:"representations,omitempty"` - Query string `json:"query,omitempty"` + Kind string `json:"kind"` + Path string `json:"path,omitempty"` + SubgraphName string `json:"subgraphName"` + SubgraphID string `json:"subgraphId"` + FetchID int `json:"fetchId"` + DependsOnFetchIDs []int `json:"dependsOnFetchIds,omitempty"` + Representations []Representation `json:"representations,omitempty"` + Query string `json:"query,omitempty"` + Dependencies []FetchDependency `json:"dependencies"` } func (n *FetchTreeNode) QueryPlan() *FetchTreeQueryPlanNode { @@ -215,6 +216,7 @@ func (n *FetchTreeNode) queryPlan() *FetchTreeQueryPlanNode { SubgraphName: f.Info.DataSourceName, SubgraphID: f.Info.DataSourceID, Path: n.Item.ResponsePath, + Dependencies: f.FetchConfiguration.CoordinateDependencies, } if f.Info.QueryPlan != nil { @@ -229,6 +231,7 @@ func (n *FetchTreeNode) queryPlan() *FetchTreeQueryPlanNode { SubgraphName: f.Info.DataSourceName, SubgraphID: f.Info.DataSourceID, Path: n.Item.ResponsePath, + Dependencies: f.CoordinateDependencies, } if f.Info.QueryPlan != nil { @@ -243,6 +246,7 @@ func (n *FetchTreeNode) queryPlan() *FetchTreeQueryPlanNode { SubgraphName: f.Info.DataSourceName, SubgraphID: f.Info.DataSourceID, Path: n.Item.ResponsePath, + Dependencies: f.CoordinateDependencies, } if f.Info.QueryPlan != nil { From 98772019ef13b330f75d822e511f90ea0d8ebd1b Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Tue, 8 Jul 2025 12:55:14 +0200 Subject: [PATCH 3/4] chore: cleanup --- .../graphql_datasource_federation_test.go | 6 +++--- v2/pkg/engine/resolve/fetchtree.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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 d9f5386667..b97bb5c808 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 @@ -3099,7 +3099,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://address-enricher.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Address {__typename country city}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, - Dependencies: []resolve.FetchDependency{ + CoordinateDependencies: []resolve.FetchDependency{ { Coordinate: resolve.GraphCoordinate{ TypeName: "Address", @@ -3174,7 +3174,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSource: &Source{}, PostProcessing: SingleEntityPostProcessingConfiguration, RequiresEntityFetch: true, - Dependencies: []resolve.FetchDependency{ + CoordinateDependencies: []resolve.FetchDependency{ { Coordinate: resolve.GraphCoordinate{ TypeName: "Address", @@ -3278,7 +3278,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Address {__typename fullAddress}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, PostProcessing: SingleEntityPostProcessingConfiguration, - Dependencies: []resolve.FetchDependency{ + CoordinateDependencies: []resolve.FetchDependency{ { Coordinate: resolve.GraphCoordinate{ TypeName: "Address", diff --git a/v2/pkg/engine/resolve/fetchtree.go b/v2/pkg/engine/resolve/fetchtree.go index a1633723da..3fcf2d4393 100644 --- a/v2/pkg/engine/resolve/fetchtree.go +++ b/v2/pkg/engine/resolve/fetchtree.go @@ -170,7 +170,7 @@ type FetchTreeQueryPlan struct { DependsOnFetchIDs []int `json:"dependsOnFetchIds,omitempty"` Representations []Representation `json:"representations,omitempty"` Query string `json:"query,omitempty"` - Dependencies []FetchDependency `json:"dependencies"` + Dependencies []FetchDependency `json:"dependencies,omitempty"` } func (n *FetchTreeNode) QueryPlan() *FetchTreeQueryPlanNode { From 76763f0e1cf6bacc8191a7ac036093b0253760f5 Mon Sep 17 00:00:00 2001 From: Jens Neuse Date: Tue, 8 Jul 2025 16:52:06 +0200 Subject: [PATCH 4/4] chore: cleanup --- v2/pkg/engine/plan/node_selection_visitor.go | 1 - 1 file changed, 1 deletion(-) diff --git a/v2/pkg/engine/plan/node_selection_visitor.go b/v2/pkg/engine/plan/node_selection_visitor.go index b87d273833..95c4f01aea 100644 --- a/v2/pkg/engine/plan/node_selection_visitor.go +++ b/v2/pkg/engine/plan/node_selection_visitor.go @@ -38,7 +38,6 @@ type nodeSelectionVisitor struct { fieldRefDependsOn map[int][]int // fieldRefDependsOn is a map[fieldRef][]fieldRef - holds list of field refs which are required by a field ref, it is a second index without datasource hash fieldRequirementsConfigs map[fieldIndexKey][]FederationFieldConfiguration // fieldRequirementsConfigs is a map[fieldIndexKey]FederationFieldConfiguration - holds a list of required configuratuibs for a field ref to later built representation variables fieldLandedTo map[int]DSHash // fieldLandedTo is a map[fieldRef]DSHash - holds a datasource hash where field was landed to - fieldEnclosingTypeNames map[int]string fieldDependencyKind map[fieldDependencyKey]fieldDependencyKind secondaryRun bool // secondaryRun is a flag to indicate that we're running the nodeSelectionVisitor not the first time