From 1636667f747f86e8b62d4060b493ad98242309c8 Mon Sep 17 00:00:00 2001 From: starptech Date: Tue, 24 Sep 2024 06:04:18 +0200 Subject: [PATCH 1/4] fix: simplify error string --- v2/pkg/engine/resolve/errors.go | 68 +++++++++------------------ v2/pkg/engine/resolve/errors_test.go | 69 ---------------------------- 2 files changed, 21 insertions(+), 116 deletions(-) diff --git a/v2/pkg/engine/resolve/errors.go b/v2/pkg/engine/resolve/errors.go index d8ba2d2f9..93b43f8c0 100644 --- a/v2/pkg/engine/resolve/errors.go +++ b/v2/pkg/engine/resolve/errors.go @@ -3,7 +3,7 @@ package resolve import ( "bytes" "fmt" - "strings" + "slices" ) type GraphQLError struct { @@ -41,14 +41,30 @@ func (e *SubgraphError) AppendDownstreamError(error *GraphQLError) { e.DownstreamErrors = append(e.DownstreamErrors, error) } -func (e *SubgraphError) Error() string { +func (e *SubgraphError) Codes() []string { + codes := make([]string, 0, len(e.DownstreamErrors)) + + for _, downstreamError := range e.DownstreamErrors { + if downstreamError.Extensions != nil { + if ok := downstreamError.Extensions["code"]; ok != nil { + if code, ok := downstreamError.Extensions["code"].(string); ok && !slices.Contains(codes, code) { + codes = append(codes, code) + } + } + } + } + return codes +} + +// Error returns the high-level error without downstream errors. For more details, call Summary(). +func (e *SubgraphError) Error() string { var bf bytes.Buffer - if e.DataSourceInfo.Name == "" { - fmt.Fprintf(&bf, "Failed to fetch Subgraph at Path: '%s'", e.Path) - } else { + if e.DataSourceInfo.Name != "" && e.Path != "" { fmt.Fprintf(&bf, "Failed to fetch from Subgraph '%s' at Path: '%s'", e.DataSourceInfo.Name, e.Path) + } else { + fmt.Fprintf(&bf, "Failed to fetch from Subgraph '%s'", e.DataSourceInfo.Name) } if e.Reason != "" { @@ -57,48 +73,6 @@ func (e *SubgraphError) Error() string { fmt.Fprintf(&bf, ".") } - if len(e.DownstreamErrors) > 0 { - - fmt.Fprintf(&bf, "\n") - fmt.Fprintf(&bf, "Downstream errors:\n") - - for i, downstreamError := range e.DownstreamErrors { - extensionCodeErrorString := "" - if downstreamError.Extensions != nil { - if ok := downstreamError.Extensions["code"]; ok != nil { - if code, ok := downstreamError.Extensions["code"].(string); ok { - extensionCodeErrorString = code - } - } - } - - if len(downstreamError.Path) > 0 { - builder := strings.Builder{} - for i := range downstreamError.Path { - switch t := downstreamError.Path[i].(type) { - case string: - builder.WriteString(t) - case int: - builder.WriteString(fmt.Sprintf("%d", t)) - } - if i < len(downstreamError.Path)-1 { - builder.WriteRune('.') - } - } - path := builder.String() - fmt.Fprintf(&bf, "%d. Subgraph error at Path '%s', Message: %s", i+1, path, downstreamError.Message) - } else { - fmt.Fprintf(&bf, "%d. Subgraph error with Message: %s", i+1, downstreamError.Message) - } - - if extensionCodeErrorString != "" { - fmt.Fprintf(&bf, ", Extension Code: %s.", extensionCodeErrorString) - } - - fmt.Fprintf(&bf, "\n") - } - } - return bf.String() } diff --git a/v2/pkg/engine/resolve/errors_test.go b/v2/pkg/engine/resolve/errors_test.go index 56d392761..3f9daf55d 100644 --- a/v2/pkg/engine/resolve/errors_test.go +++ b/v2/pkg/engine/resolve/errors_test.go @@ -36,75 +36,6 @@ func TestSubgraphError(t *testing.T) { require.Equal(t, len(e.DownstreamErrors), 0) require.EqualError(t, e, "Failed to fetch from Subgraph 'subgraphName' at Path: 'path', Reason: reason.") }) - - t.Run("With downstream errors", func(t *testing.T) { - e := NewSubgraphError(DataSourceInfo{ - Name: "subgraphName", - ID: "subgraphID", - }, "path", "reason", 500) - - require.Equal(t, e.DataSourceInfo.Name, "subgraphName") - require.Equal(t, e.Path, "path") - require.Equal(t, e.Reason, "reason") - require.Equal(t, e.ResponseCode, 500) - - e.AppendDownstreamError(&GraphQLError{ - Message: "errorMessage", - Path: []any{"path"}, - Extensions: map[string]interface{}{ - "code": "code", - }, - }) - - require.Equal(t, len(e.DownstreamErrors), 1) - require.EqualError(t, e, "Failed to fetch from Subgraph 'subgraphName' at Path: 'path', Reason: reason.\nDownstream errors:\n1. Subgraph error at Path 'path', Message: errorMessage, Extension Code: code.\n") - }) - - t.Run("With multi segment downstream errors", func(t *testing.T) { - e := NewSubgraphError(DataSourceInfo{ - Name: "subgraphName", - ID: "subgraphID", - }, "path", "reason", 500) - - require.Equal(t, e.DataSourceInfo.Name, "subgraphName") - require.Equal(t, e.Path, "path") - require.Equal(t, e.Reason, "reason") - require.Equal(t, e.ResponseCode, 500) - - e.AppendDownstreamError(&GraphQLError{ - Message: "errorMessage", - Path: []any{"path", "to", "success"}, - Extensions: map[string]interface{}{ - "code": "code", - }, - }) - - require.Equal(t, len(e.DownstreamErrors), 1) - require.EqualError(t, e, "Failed to fetch from Subgraph 'subgraphName' at Path: 'path', Reason: reason.\nDownstream errors:\n1. Subgraph error at Path 'path.to.success', Message: errorMessage, Extension Code: code.\n") - }) - - t.Run("With mixed multi segment downstream errors", func(t *testing.T) { - e := NewSubgraphError(DataSourceInfo{ - Name: "subgraphName", - ID: "subgraphID", - }, "path", "reason", 500) - - require.Equal(t, e.DataSourceInfo.Name, "subgraphName") - require.Equal(t, e.Path, "path") - require.Equal(t, e.Reason, "reason") - require.Equal(t, e.ResponseCode, 500) - - e.AppendDownstreamError(&GraphQLError{ - Message: "errorMessage", - Path: []any{"path", 1, "to", "success"}, - Extensions: map[string]interface{}{ - "code": "code", - }, - }) - - require.Equal(t, len(e.DownstreamErrors), 1) - require.EqualError(t, e, "Failed to fetch from Subgraph 'subgraphName' at Path: 'path', Reason: reason.\nDownstream errors:\n1. Subgraph error at Path 'path.1.to.success', Message: errorMessage, Extension Code: code.\n") - }) } func TestRateLimitError(t *testing.T) { From e60c9976fda804e251033d3cdcf7b8182d478ea8 Mon Sep 17 00:00:00 2001 From: starptech Date: Fri, 27 Sep 2024 09:59:20 +0200 Subject: [PATCH 2/4] fix: collect DS info and ensure corrected errors are passed --- v2/pkg/engine/postprocess/postprocess.go | 30 ++ v2/pkg/engine/postprocess/postprocess_test.go | 277 +++++++++++++++++- v2/pkg/engine/resolve/loader.go | 49 ++-- v2/pkg/engine/resolve/loader_hooks_test.go | 4 +- v2/pkg/engine/resolve/response.go | 1 + v2/pkg/internal/unique/unique.go | 13 + 6 files changed, 353 insertions(+), 21 deletions(-) create mode 100644 v2/pkg/internal/unique/unique.go diff --git a/v2/pkg/engine/postprocess/postprocess.go b/v2/pkg/engine/postprocess/postprocess.go index e1a95a6dc..9d4079126 100644 --- a/v2/pkg/engine/postprocess/postprocess.go +++ b/v2/pkg/engine/postprocess/postprocess.go @@ -3,6 +3,7 @@ package postprocess import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" + "github.com/wundergraph/graphql-go-tools/v2/pkg/internal/unique" ) type ResponseTreeProcessor interface { @@ -16,6 +17,7 @@ type FetchTreeProcessor interface { type Processor struct { disableExtractFetches bool + collectDataSourceInfo bool resolveInputTemplates *resolveInputTemplates dedupe *deduplicateSingleFetches processResponseTree []ResponseTreeProcessor @@ -30,6 +32,7 @@ type processorOptions struct { disableExtractFetches bool disableCreateParallelNodes bool disableAddMissingNestedDependencies bool + collectDataSourceInfo bool } type ProcessorOption func(*processorOptions) @@ -59,6 +62,12 @@ func DisableResolveInputTemplates() ProcessorOption { } } +func CollectDataSourceInfo() ProcessorOption { + return func(o *processorOptions) { + o.collectDataSourceInfo = true + } +} + func DisableExtractFetches() ProcessorOption { return func(o *processorOptions) { o.disableExtractFetches = true @@ -83,6 +92,7 @@ func NewProcessor(options ...ProcessorOption) *Processor { o(opts) } return &Processor{ + collectDataSourceInfo: opts.collectDataSourceInfo, disableExtractFetches: opts.disableExtractFetches, resolveInputTemplates: &resolveInputTemplates{ disable: opts.disableResolveInputTemplates, @@ -121,6 +131,9 @@ func (p *Processor) Process(pre plan.Plan) plan.Plan { p.processResponseTree[i].Process(t.Response.Data) } p.createFetchTree(t.Response) + if p.collectDataSourceInfo { + t.Response.DataSources = collectDataSourceInfos(t.Response.Fetches) + } p.dedupe.ProcessFetchTree(t.Response.Fetches) p.resolveInputTemplates.ProcessFetchTree(t.Response.Fetches) for i := range p.processFetchTree { @@ -131,6 +144,9 @@ func (p *Processor) Process(pre plan.Plan) plan.Plan { p.processResponseTree[i].ProcessSubscription(t.Response.Response.Data) } p.createFetchTree(t.Response.Response) + if p.collectDataSourceInfo { + t.Response.Response.DataSources = collectDataSourceInfos(t.Response.Response.Fetches) + } p.dedupe.ProcessFetchTree(t.Response.Response.Fetches) p.resolveInputTemplates.ProcessFetchTree(t.Response.Response.Fetches) p.resolveInputTemplates.ProcessTrigger(&t.Response.Trigger) @@ -161,3 +177,17 @@ func (p *Processor) createFetchTree(res *resolve.GraphQLResponse) { ChildNodes: children, } } + +// collectDataSourceInfos returns the list of involved data sources of the operation +func collectDataSourceInfos(node *resolve.FetchTreeNode) (list []resolve.DataSourceInfo) { + + if node.Item != nil && node.Item.Fetch != nil { + list = append(list, node.Item.Fetch.DataSourceInfo()) + } + + for _, childNode := range node.ChildNodes { + list = append(list, collectDataSourceInfos(childNode)...) + } + + return unique.SliceElements(list) +} diff --git a/v2/pkg/engine/postprocess/postprocess_test.go b/v2/pkg/engine/postprocess/postprocess_test.go index 2d7a64b6d..41f04ff31 100644 --- a/v2/pkg/engine/postprocess/postprocess_test.go +++ b/v2/pkg/engine/postprocess/postprocess_test.go @@ -2,6 +2,7 @@ package postprocess import ( "fmt" + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "reflect" "testing" @@ -334,7 +335,281 @@ func TestProcess_ExtractFetches(t *testing.T) { }, } - processor := NewProcessor(DisableDeduplicateSingleFetches(), DisableCreateConcreteSingleFetchTypes(), DisableMergeFields(), DisableCreateParallelNodes(), DisableAddMissingNestedDependencies()) + processor := NewProcessor( + DisableDeduplicateSingleFetches(), + DisableCreateConcreteSingleFetchTypes(), + DisableMergeFields(), + DisableCreateParallelNodes(), + DisableAddMissingNestedDependencies(), + ) + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + actual := processor.Process(c.pre) + + if !assert.Equal(t, c.expected, actual) { + formatterConfig := map[reflect.Type]interface{}{ + reflect.TypeOf([]byte{}): func(b []byte) string { return fmt.Sprintf(`"%s"`, string(b)) }, + } + + prettyCfg := &pretty.Config{ + Diffable: true, + IncludeUnexported: false, + Formatter: formatterConfig, + } + + if diff := prettyCfg.Compare(c.expected, actual); diff != "" { + t.Errorf("Plan does not match(-want +got)\n%s", diff) + } + } + }) + } +} + +func TestProcess_ExtractServiceNames(t *testing.T) { + type TestCase struct { + name string + pre plan.Plan + expected plan.Plan + } + + cases := []TestCase{ + { + name: "Collect all service names", + pre: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("field1"), + Value: &resolve.String{ + Path: []string{"field1"}, + }, + }, + }, + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "user-service", + DataSourceName: "user-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "user", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 1}, + }, + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "product-service", + DataSourceName: "product-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "product", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 2}, + }, + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "review-service", + DataSourceName: "review-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "review", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 3}}, + }, + }, + }, + }, + expected: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + DataSources: []resolve.DataSourceInfo{ + {ID: "user-service", Name: "user-service"}, + {ID: "product-service", Name: "product-service"}, + {ID: "review-service", Name: "review-service"}, + }, + Data: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("field1"), + Value: &resolve.String{ + Path: []string{"field1"}, + }, + }, + }, + }, + Fetches: resolve.Sequence( + resolve.Single(&resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "user-service", + DataSourceName: "user-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "user", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 1}}, + ), + resolve.Single( + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "product-service", + DataSourceName: "product-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "product", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 2}, + }, + ), + resolve.Single( + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "review-service", + DataSourceName: "review-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "review", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 3}, + }, + ), + ), + }, + }, + }, + { + name: "Deduplicate the same service names", + pre: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("field1"), + Value: &resolve.String{ + Path: []string{"field1"}, + }, + }, + }, + Fetches: []resolve.Fetch{ + + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "product-service", + DataSourceName: "product-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "product", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 2}, + }, + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "product-service", + DataSourceName: "product-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "products", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 2}, + }, + }, + }, + }, + }, + expected: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + DataSources: []resolve.DataSourceInfo{ + {ID: "product-service", Name: "product-service"}, + }, + Data: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("field1"), + Value: &resolve.String{ + Path: []string{"field1"}, + }, + }, + }, + }, + Fetches: resolve.Sequence( + resolve.Single( + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "product-service", + DataSourceName: "product-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "product", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 2}, + }, + ), + resolve.Single( + &resolve.SingleFetch{ + Info: &resolve.FetchInfo{ + DataSourceID: "product-service", + DataSourceName: "product-service", + OperationType: ast.OperationTypeQuery, + RootFields: []resolve.GraphCoordinate{ + { + TypeName: "Query", + FieldName: "products", + }, + }, + }, + FetchDependencies: resolve.FetchDependencies{FetchID: 2}, + }, + ), + ), + }, + }, + }, + } + + processor := NewProcessor( + DisableDeduplicateSingleFetches(), + DisableCreateConcreteSingleFetchTypes(), + DisableMergeFields(), + DisableCreateParallelNodes(), + DisableAddMissingNestedDependencies(), + CollectDataSourceInfo(), + ) for _, c := range cases { t.Run(c.name, func(t *testing.T) { diff --git a/v2/pkg/engine/resolve/loader.go b/v2/pkg/engine/resolve/loader.go index 7e662ba89..9c402eeae 100644 --- a/v2/pkg/engine/resolve/loader.go +++ b/v2/pkg/engine/resolve/loader.go @@ -652,29 +652,28 @@ func (l *Loader) renderErrorsInvalidInput(fetchItem *FetchItem, out *bytes.Buffe return nil } -func (l *Loader) mergeErrors(res *result, fetchItem *FetchItem, value *astjson.Value, values []*astjson.Value) error { - // Serialize subgraph errors from the response - // and append them to the subgraph downstream errors - if len(values) > 0 { - // print them into the buffer to be able to parse them - errorsJSON := value.MarshalTo(nil) - graphqlErrors := make([]GraphQLError, 0, len(values)) - err := json.Unmarshal(errorsJSON, &graphqlErrors) - if err != nil { - return errors.WithStack(err) - } +func (l *Loader) appendSubgraphError(res *result, fetchItem *FetchItem, value *astjson.Value, values []*astjson.Value) error { + // print them into the buffer to be able to parse them + errorsJSON := value.MarshalTo(nil) + graphqlErrors := make([]GraphQLError, 0, len(values)) + err := json.Unmarshal(errorsJSON, &graphqlErrors) + if err != nil { + return errors.WithStack(err) + } - subgraphError := NewSubgraphError(res.ds, fetchItem.ResponsePath, failedToFetchNoReason, res.statusCode) + subgraphError := NewSubgraphError(res.ds, fetchItem.ResponsePath, failedToFetchNoReason, res.statusCode) - for _, gqlError := range graphqlErrors { - gErr := gqlError - subgraphError.AppendDownstreamError(&gErr) - } + for _, gqlError := range graphqlErrors { + gErr := gqlError + subgraphError.AppendDownstreamError(&gErr) + } - l.ctx.appendSubgraphError(goerrors.Join(res.err, subgraphError)) + l.ctx.appendSubgraphError(goerrors.Join(res.err, subgraphError)) - } + return nil +} +func (l *Loader) mergeErrors(res *result, fetchItem *FetchItem, value *astjson.Value, values []*astjson.Value) error { l.optionallyOmitErrorLocations(values) l.optionallyRewriteErrorPaths(fetchItem, values) l.optionallyAllowCustomExtensionProperties(values) @@ -688,11 +687,25 @@ func (l *Loader) mergeErrors(res *result, fetchItem *FetchItem, value *astjson.V // Allow to delete extensions entirely l.optionallyOmitErrorExtensions(values) + if len(values) > 0 { + // Append the subgraph errors to the response payload + if err := l.appendSubgraphError(res, fetchItem, value, values); err != nil { + return err + } + } + // If the error propagation mode is pass-through, we append the errors to the root array astjson.MergeValues(l.resolvable.errors, value) return nil } + if len(values) > 0 { + // Append the subgraph errors to the response payload + if err := l.appendSubgraphError(res, fetchItem, value, values); err != nil { + return err + } + } + // Wrap mode (default) errorObject := astjson.MustParse(l.renderSubgraphBaseError(res.ds, fetchItem.ResponsePath, failedToFetchNoReason)) diff --git a/v2/pkg/engine/resolve/loader_hooks_test.go b/v2/pkg/engine/resolve/loader_hooks_test.go index 459dc9dcf..a72ab5d74 100644 --- a/v2/pkg/engine/resolve/loader_hooks_test.go +++ b/v2/pkg/engine/resolve/loader_hooks_test.go @@ -376,9 +376,9 @@ func TestLoaderHooks_FetchPipeline(t *testing.T) { assert.Equal(t, 0, subgraphError.ResponseCode) assert.Len(t, subgraphError.DownstreamErrors, 2) assert.Equal(t, "errorMessage", subgraphError.DownstreamErrors[0].Message) - assert.Equal(t, "GRAPHQL_VALIDATION_FAILED", subgraphError.DownstreamErrors[0].Extensions["code"]) + assert.Empty(t, subgraphError.DownstreamErrors[0].Extensions["code"]) assert.Equal(t, "errorMessage2", subgraphError.DownstreamErrors[1].Message) - assert.Equal(t, "BAD_USER_INPUT", subgraphError.DownstreamErrors[1].Extensions["code"]) + assert.Empty(t, subgraphError.DownstreamErrors[1].Extensions["code"]) assert.NotNil(t, resolveCtx.SubgraphErrors()) } diff --git a/v2/pkg/engine/resolve/response.go b/v2/pkg/engine/resolve/response.go index 1eee085df..51812b38a 100644 --- a/v2/pkg/engine/resolve/response.go +++ b/v2/pkg/engine/resolve/response.go @@ -26,6 +26,7 @@ type GraphQLResponse struct { RenameTypeNames []RenameTypeName Info *GraphQLResponseInfo Fetches *FetchTreeNode + DataSources []DataSourceInfo } type GraphQLResponseInfo struct { diff --git a/v2/pkg/internal/unique/unique.go b/v2/pkg/internal/unique/unique.go new file mode 100644 index 000000000..82481fed4 --- /dev/null +++ b/v2/pkg/internal/unique/unique.go @@ -0,0 +1,13 @@ +package unique + +func SliceElements[T comparable](inputSlice []T) []T { + uniqueSlice := make([]T, 0, len(inputSlice)) + seen := make(map[T]bool, len(inputSlice)) + for _, element := range inputSlice { + if !seen[element] { + uniqueSlice = append(uniqueSlice, element) + seen[element] = true + } + } + return uniqueSlice +} From 8fca107704f8f9c7dea1e72ef8a4e35f612a4db4 Mon Sep 17 00:00:00 2001 From: starptech Date: Fri, 27 Sep 2024 11:37:48 +0200 Subject: [PATCH 3/4] fix: address feedback --- v2/pkg/engine/postprocess/postprocess.go | 5 ++--- v2/pkg/engine/postprocess/postprocess_test.go | 1 - v2/pkg/internal/unique/unique.go | 13 ------------- 3 files changed, 2 insertions(+), 17 deletions(-) delete mode 100644 v2/pkg/internal/unique/unique.go diff --git a/v2/pkg/engine/postprocess/postprocess.go b/v2/pkg/engine/postprocess/postprocess.go index 9d4079126..3af7ec31e 100644 --- a/v2/pkg/engine/postprocess/postprocess.go +++ b/v2/pkg/engine/postprocess/postprocess.go @@ -3,7 +3,7 @@ package postprocess import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" - "github.com/wundergraph/graphql-go-tools/v2/pkg/internal/unique" + "slices" ) type ResponseTreeProcessor interface { @@ -180,7 +180,6 @@ func (p *Processor) createFetchTree(res *resolve.GraphQLResponse) { // collectDataSourceInfos returns the list of involved data sources of the operation func collectDataSourceInfos(node *resolve.FetchTreeNode) (list []resolve.DataSourceInfo) { - if node.Item != nil && node.Item.Fetch != nil { list = append(list, node.Item.Fetch.DataSourceInfo()) } @@ -189,5 +188,5 @@ func collectDataSourceInfos(node *resolve.FetchTreeNode) (list []resolve.DataSou list = append(list, collectDataSourceInfos(childNode)...) } - return unique.SliceElements(list) + return slices.Compact(list) } diff --git a/v2/pkg/engine/postprocess/postprocess_test.go b/v2/pkg/engine/postprocess/postprocess_test.go index 41f04ff31..e9d357f11 100644 --- a/v2/pkg/engine/postprocess/postprocess_test.go +++ b/v2/pkg/engine/postprocess/postprocess_test.go @@ -515,7 +515,6 @@ func TestProcess_ExtractServiceNames(t *testing.T) { }, }, Fetches: []resolve.Fetch{ - &resolve.SingleFetch{ Info: &resolve.FetchInfo{ DataSourceID: "product-service", diff --git a/v2/pkg/internal/unique/unique.go b/v2/pkg/internal/unique/unique.go deleted file mode 100644 index 82481fed4..000000000 --- a/v2/pkg/internal/unique/unique.go +++ /dev/null @@ -1,13 +0,0 @@ -package unique - -func SliceElements[T comparable](inputSlice []T) []T { - uniqueSlice := make([]T, 0, len(inputSlice)) - seen := make(map[T]bool, len(inputSlice)) - for _, element := range inputSlice { - if !seen[element] { - uniqueSlice = append(uniqueSlice, element) - seen[element] = true - } - } - return uniqueSlice -} From 2490aa37508819ef871f806b82c45ac985d465bb Mon Sep 17 00:00:00 2001 From: starptech Date: Fri, 27 Sep 2024 11:56:58 +0200 Subject: [PATCH 4/4] fix: address feedback --- v2/pkg/engine/postprocess/postprocess.go | 28 ++++++++---------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/v2/pkg/engine/postprocess/postprocess.go b/v2/pkg/engine/postprocess/postprocess.go index 3af7ec31e..4eb979425 100644 --- a/v2/pkg/engine/postprocess/postprocess.go +++ b/v2/pkg/engine/postprocess/postprocess.go @@ -131,9 +131,6 @@ func (p *Processor) Process(pre plan.Plan) plan.Plan { p.processResponseTree[i].Process(t.Response.Data) } p.createFetchTree(t.Response) - if p.collectDataSourceInfo { - t.Response.DataSources = collectDataSourceInfos(t.Response.Fetches) - } p.dedupe.ProcessFetchTree(t.Response.Fetches) p.resolveInputTemplates.ProcessFetchTree(t.Response.Fetches) for i := range p.processFetchTree { @@ -144,9 +141,6 @@ func (p *Processor) Process(pre plan.Plan) plan.Plan { p.processResponseTree[i].ProcessSubscription(t.Response.Response.Data) } p.createFetchTree(t.Response.Response) - if p.collectDataSourceInfo { - t.Response.Response.DataSources = collectDataSourceInfos(t.Response.Response.Fetches) - } p.dedupe.ProcessFetchTree(t.Response.Response.Fetches) p.resolveInputTemplates.ProcessFetchTree(t.Response.Response.Fetches) p.resolveInputTemplates.ProcessTrigger(&t.Response.Trigger) @@ -166,6 +160,15 @@ func (p *Processor) createFetchTree(res *resolve.GraphQLResponse) { } fetches := ex.extractFetches(res) children := make([]*resolve.FetchTreeNode, len(fetches)) + + if p.collectDataSourceInfo { + var list = make([]resolve.DataSourceInfo, 0, len(fetches)) + for _, fetch := range fetches { + list = append(list, fetch.Fetch.DataSourceInfo()) + } + res.DataSources = slices.Compact(list) + } + for i := range fetches { children[i] = &resolve.FetchTreeNode{ Kind: resolve.FetchTreeNodeKindSingle, @@ -177,16 +180,3 @@ func (p *Processor) createFetchTree(res *resolve.GraphQLResponse) { ChildNodes: children, } } - -// collectDataSourceInfos returns the list of involved data sources of the operation -func collectDataSourceInfos(node *resolve.FetchTreeNode) (list []resolve.DataSourceInfo) { - if node.Item != nil && node.Item.Fetch != nil { - list = append(list, node.Item.Fetch.DataSourceInfo()) - } - - for _, childNode := range node.ChildNodes { - list = append(list, collectDataSourceInfos(childNode)...) - } - - return slices.Compact(list) -}