diff --git a/v2/pkg/engine/postprocess/deduplicate_single_fetches.go b/v2/pkg/engine/postprocess/deduplicate_single_fetches.go index 9fb8c1c3df..c7864fc32c 100644 --- a/v2/pkg/engine/postprocess/deduplicate_single_fetches.go +++ b/v2/pkg/engine/postprocess/deduplicate_single_fetches.go @@ -6,6 +6,9 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" ) +// deduplicateSingleFetches is a post-processing step that removes duplicate single fetches +// from the initial fetch tree. It merges their fetch paths and updates dependencies accordingly. +// NOTE: initial tree structure should be flat and contain a single root item with all fetches as children. type deduplicateSingleFetches struct { disable bool } @@ -16,16 +19,52 @@ func (d *deduplicateSingleFetches) ProcessFetchTree(root *resolve.FetchTreeNode) } for i := range root.ChildNodes { for j := i + 1; j < len(root.ChildNodes); j++ { - if root.ChildNodes[i].Item.Equals(root.ChildNodes[j].Item) { + if root.ChildNodes[i].Item.EqualSingleFetch(root.ChildNodes[j].Item) { root.ChildNodes[i].Item.FetchPath = d.mergeFetchPath(root.ChildNodes[i].Item.FetchPath, root.ChildNodes[j].Item.FetchPath) + newId := root.ChildNodes[i].Item.Fetch.Dependencies().FetchID + oldId := root.ChildNodes[j].Item.Fetch.Dependencies().FetchID + root.ChildNodes = append(root.ChildNodes[:j], root.ChildNodes[j+1:]...) j-- + + // when we merge duplicated fetches, we need to update the dependencies of the other fetches + // because they might depend on the fetch that we are removing + d.replaceDependsOnFetchId(root, oldId, newId) + } + } + } +} + +// replaceDependsOnFetchId replaces all occurrences of oldId with newId in the dependencies of the fetch tree. +func (d *deduplicateSingleFetches) replaceDependsOnFetchId(root *resolve.FetchTreeNode, oldId, newId int) { + for i := range root.ChildNodes { + replaced := false + for j := range root.ChildNodes[i].Item.Fetch.Dependencies().DependsOnFetchIDs { + if root.ChildNodes[i].Item.Fetch.Dependencies().DependsOnFetchIDs[j] == oldId { + root.ChildNodes[i].Item.Fetch.Dependencies().DependsOnFetchIDs[j] = newId + replaced = true + } + } + + if !replaced { + continue + } + + for j := range root.ChildNodes[i].Item.Fetch.DependenciesCoordinates() { + for k := range root.ChildNodes[i].Item.Fetch.DependenciesCoordinates()[j].DependsOn { + if root.ChildNodes[i].Item.Fetch.DependenciesCoordinates()[j].DependsOn[k].FetchID == oldId { + root.ChildNodes[i].Item.Fetch.DependenciesCoordinates()[j].DependsOn[k].FetchID = newId + } } } } } +// mergeFetchPath merges the fetch paths of two single fetches. +// The goal of this method is to merge typename conditions. +// When fetches originate from different parent fragments - +// they will have different typenames, but the same path in response. func (d *deduplicateSingleFetches) mergeFetchPath(left, right []resolve.FetchItemPathElement) []resolve.FetchItemPathElement { for i := range left { left[i].TypeNames = d.mergeTypeNames(left[i].TypeNames, right[i].TypeNames) diff --git a/v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go b/v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go index 88cf2be895..abaafada36 100644 --- a/v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go +++ b/v2/pkg/engine/postprocess/deduplicate_single_fetches_test.go @@ -71,6 +71,202 @@ func TestDeduplicateSingleFetches_ProcessFetchTree(t *testing.T) { assert.Equal(t, output, input) }) + t.Run("same path, same input, different fetch ids - should update dependencies with merged fetch ids", func(t *testing.T) { + input := &resolve.FetchTreeNode{ + ChildNodes: []*resolve.FetchTreeNode{ + { + Kind: resolve.FetchTreeNodeKindSingle, + Item: &resolve.FetchItem{ + FetchPath: []resolve.FetchItemPathElement{{Kind: resolve.FetchItemPathElementKindObject, Path: []string{"root"}}}, + Fetch: &resolve.SingleFetch{ + FetchDependencies: resolve.FetchDependencies{ + FetchID: 0, + DependsOnFetchIDs: []int{}, + }, + FetchConfiguration: resolve.FetchConfiguration{Input: "rootQuery"}, + }, + }, + }, + { + Kind: resolve.FetchTreeNodeKindSingle, + Item: &resolve.FetchItem{ + FetchPath: []resolve.FetchItemPathElement{{Kind: resolve.FetchItemPathElementKindObject, Path: []string{"root.a"}}}, + Fetch: &resolve.SingleFetch{ + FetchDependencies: resolve.FetchDependencies{ + FetchID: 1, + DependsOnFetchIDs: []int{0}, + }, + FetchConfiguration: resolve.FetchConfiguration{ + Input: "a", + CoordinateDependencies: []resolve.FetchDependency{ + { + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + }, + }, + }, + }, + }, + }, + }, + }, + { + Kind: resolve.FetchTreeNodeKindSingle, + Item: &resolve.FetchItem{ + FetchPath: []resolve.FetchItemPathElement{{Kind: resolve.FetchItemPathElementKindObject, Path: []string{"root.a"}}}, + Fetch: &resolve.SingleFetch{ + FetchDependencies: resolve.FetchDependencies{ + FetchID: 2, + DependsOnFetchIDs: []int{0}, + }, + FetchConfiguration: resolve.FetchConfiguration{ + Input: "a", + CoordinateDependencies: []resolve.FetchDependency{ + { + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + }, + }, + }, + }, + }, + }, + }, + }, + { + Kind: resolve.FetchTreeNodeKindSingle, + Item: &resolve.FetchItem{ + FetchPath: []resolve.FetchItemPathElement{{Kind: resolve.FetchItemPathElementKindObject, Path: []string{"root.a.b"}}}, + Fetch: &resolve.SingleFetch{ + FetchDependencies: resolve.FetchDependencies{ + FetchID: 4, + DependsOnFetchIDs: []int{0, 2}, + }, + FetchConfiguration: resolve.FetchConfiguration{ + Input: "b", + CoordinateDependencies: []resolve.FetchDependency{ + { + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + }, + { + FetchID: 2, + }, + }, + }, + }, + }, + }, + }, + }, + { + Kind: resolve.FetchTreeNodeKindSingle, + Item: &resolve.FetchItem{ + FetchPath: []resolve.FetchItemPathElement{{Kind: resolve.FetchItemPathElementKindObject, Path: []string{"root.a.b"}}}, + Fetch: &resolve.SingleFetch{ + FetchDependencies: resolve.FetchDependencies{ + FetchID: 3, + DependsOnFetchIDs: []int{0, 1}, + }, + FetchConfiguration: resolve.FetchConfiguration{ + Input: "b", + CoordinateDependencies: []resolve.FetchDependency{ + { + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + }, + { + FetchID: 1, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + output := &resolve.FetchTreeNode{ + ChildNodes: []*resolve.FetchTreeNode{ + { + Kind: resolve.FetchTreeNodeKindSingle, + Item: &resolve.FetchItem{ + FetchPath: []resolve.FetchItemPathElement{{Kind: resolve.FetchItemPathElementKindObject, Path: []string{"root"}}}, + Fetch: &resolve.SingleFetch{ + FetchDependencies: resolve.FetchDependencies{ + FetchID: 0, + DependsOnFetchIDs: []int{}, + }, + FetchConfiguration: resolve.FetchConfiguration{Input: "rootQuery"}, + }, + }, + }, + { + Kind: resolve.FetchTreeNodeKindSingle, + Item: &resolve.FetchItem{ + FetchPath: []resolve.FetchItemPathElement{{Kind: resolve.FetchItemPathElementKindObject, Path: []string{"root.a"}}}, + Fetch: &resolve.SingleFetch{ + FetchDependencies: resolve.FetchDependencies{ + FetchID: 1, + DependsOnFetchIDs: []int{0}, + }, + FetchConfiguration: resolve.FetchConfiguration{ + Input: "a", + CoordinateDependencies: []resolve.FetchDependency{ + { + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + }, + }, + }, + }, + }, + }, + }, + }, + { + Kind: resolve.FetchTreeNodeKindSingle, + Item: &resolve.FetchItem{ + FetchPath: []resolve.FetchItemPathElement{{Kind: resolve.FetchItemPathElementKindObject, Path: []string{"root.a.b"}}}, + Fetch: &resolve.SingleFetch{ + FetchDependencies: resolve.FetchDependencies{ + FetchID: 4, + DependsOnFetchIDs: []int{0, 1}, + }, + FetchConfiguration: resolve.FetchConfiguration{ + Input: "b", + CoordinateDependencies: []resolve.FetchDependency{ + { + DependsOn: []resolve.FetchDependencyOrigin{ + { + FetchID: 0, + }, + { + FetchID: 1, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + dedup := &deduplicateSingleFetches{} + dedup.ProcessFetchTree(input) + + assert.Equal(t, output, input) + }) + t.Run("different path, same input", func(t *testing.T) { input := &resolve.FetchTreeNode{ ChildNodes: []*resolve.FetchTreeNode{ diff --git a/v2/pkg/engine/postprocess/postprocess.go b/v2/pkg/engine/postprocess/postprocess.go index a66a0105f1..c542caad03 100644 --- a/v2/pkg/engine/postprocess/postprocess.go +++ b/v2/pkg/engine/postprocess/postprocess.go @@ -134,7 +134,10 @@ func (p *Processor) Process(pre plan.Plan) plan.Plan { for i := range p.processResponseTree { p.processResponseTree[i].Process(t.Response.Data) } + // initialize the fetch tree p.createFetchTree(t.Response) + // NOTE: deduplication relies on the fact that the fetch tree + // have flat structure of child fetches p.dedupe.ProcessFetchTree(t.Response.Fetches) p.resolveInputTemplates.ProcessFetchTree(t.Response.Fetches) for i := range p.processFetchTree { @@ -156,6 +159,8 @@ func (p *Processor) Process(pre plan.Plan) plan.Plan { return pre } +// createFetchTree creates an inital fetch tree from the raw fetches in the GraphQL response. +// The initial fetch tree is a node of sequence fetch kind, with a flat list of fetches as children. func (p *Processor) createFetchTree(res *resolve.GraphQLResponse) { if p.disableExtractFetches { return diff --git a/v2/pkg/engine/resolve/fetch.go b/v2/pkg/engine/resolve/fetch.go index 9979256c66..2bf7b8a3f8 100644 --- a/v2/pkg/engine/resolve/fetch.go +++ b/v2/pkg/engine/resolve/fetch.go @@ -19,8 +19,9 @@ const ( type Fetch interface { FetchKind() FetchKind - Dependencies() FetchDependencies + Dependencies() *FetchDependencies DataSourceInfo() DataSourceInfo + DependenciesCoordinates() []FetchDependency } type FetchItem struct { @@ -42,7 +43,8 @@ func FetchItemWithPath(fetch Fetch, responsePath string, path ...FetchItemPathEl return item } -func (f *FetchItem) Equals(other *FetchItem) bool { +// EqualSingleFetch compares two FetchItem for equality, both items should be of kind FetchKindSingle +func (f *FetchItem) EqualSingleFetch(other *FetchItem) bool { if len(f.FetchPath) != len(other.FetchPath) { return false } @@ -93,8 +95,12 @@ type SingleFetch struct { Info *FetchInfo } -func (s *SingleFetch) Dependencies() FetchDependencies { - return s.FetchDependencies +func (s *SingleFetch) Dependencies() *FetchDependencies { + return &s.FetchDependencies +} + +func (s *SingleFetch) DependenciesCoordinates() []FetchDependency { + return s.FetchConfiguration.CoordinateDependencies } func (s *SingleFetch) DataSourceInfo() DataSourceInfo { @@ -164,8 +170,12 @@ type BatchEntityFetch struct { CoordinateDependencies []FetchDependency } -func (b *BatchEntityFetch) Dependencies() FetchDependencies { - return b.FetchDependencies +func (b *BatchEntityFetch) Dependencies() *FetchDependencies { + return &b.FetchDependencies +} + +func (b *BatchEntityFetch) DependenciesCoordinates() []FetchDependency { + return b.CoordinateDependencies } func (b *BatchEntityFetch) DataSourceInfo() DataSourceInfo { @@ -207,8 +217,12 @@ type EntityFetch struct { Info *FetchInfo } -func (e *EntityFetch) Dependencies() FetchDependencies { - return e.FetchDependencies +func (e *EntityFetch) Dependencies() *FetchDependencies { + return &e.FetchDependencies +} + +func (e *EntityFetch) DependenciesCoordinates() []FetchDependency { + return e.CoordinateDependencies } func (e *EntityFetch) DataSourceInfo() DataSourceInfo { @@ -238,8 +252,12 @@ type ParallelListItemFetch struct { Trace *DataSourceLoadTrace } -func (p *ParallelListItemFetch) Dependencies() FetchDependencies { - return p.Fetch.FetchDependencies +func (p *ParallelListItemFetch) Dependencies() *FetchDependencies { + return &p.Fetch.FetchDependencies +} + +func (p *ParallelListItemFetch) DependenciesCoordinates() []FetchDependency { + return p.Fetch.FetchConfiguration.CoordinateDependencies } func (_ *ParallelListItemFetch) FetchKind() FetchKind { @@ -303,6 +321,8 @@ func (fc *FetchConfiguration) Equals(other *FetchConfiguration) bool { } // Note: we do not compare datasources, as they will always be a different instance + // Note: we do not compare CoordinateDependencies, as they contain more detailed + // dependencies information that is already present in the FetchDependencies on the fetch itself if fc.RequiresParallelListItemFetch != other.RequiresParallelListItemFetch { return false @@ -319,23 +339,7 @@ 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 }