From 496b3b90dd6019c2ea99ad43580e547686a7f0c7 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Thu, 28 Dec 2023 20:17:12 +0200 Subject: [PATCH 01/27] add different key jumps test --- .../graphql_datasource_federation_test.go | 386 ++++++++++++++++++ 1 file changed, 386 insertions(+) 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 1383575db..7a25af989 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 @@ -2913,4 +2913,390 @@ func TestGraphQLDataSourceFederation(t *testing.T) { )) }) }) + + t.Run("different entity keys jumps", func(t *testing.T) { + definition := ` + type User { + id: ID! + uuid: ID! + title: String! + name: String! + address: Address! + } + + type Address { + country: String! + } + + type Query { + user: User + } + ` + + firstSubgraphSDL := ` + type User @key(fields: "id") { + id: ID! + } + + type Query { + user: User + } + ` + + firstDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Query", + FieldNames: []string{"user"}, + }, + { + TypeName: "User", + FieldNames: []string{"id"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://first.service", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: firstSubgraphSDL, + }, + UpstreamSchema: firstSubgraphSDL, + }), + Factory: federationFactory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "User", + SelectionSet: "id", + }, + }, + }, + } + + secondSubgraphSDL := ` + type User @key(fields: "id") @key(fields: "uuid") { + id: ID! + uuid: ID! + name: String! + } + ` + secondDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "User", + FieldNames: []string{"id", "uuid", "name"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://second.service", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: secondSubgraphSDL, + }, + UpstreamSchema: secondSubgraphSDL, + }), + Factory: federationFactory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "User", + SelectionSet: "id", + }, + { + TypeName: "User", + SelectionSet: "uuid", + }, + }, + }, + } + + thirdSubgraphSDL := ` + type User @key(fields: "uuid") { + uuid: ID! + title: String! + } + ` + thirdDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "User", + FieldNames: []string{"uuid", "title"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://third.service", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: thirdSubgraphSDL, + }, + UpstreamSchema: thirdSubgraphSDL, + }), + Factory: federationFactory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "User", + SelectionSet: "uuid", + }, + }, + }, + } + + fourthSubgraphSDL := ` + type User @key(fields: "id") { + id: ID! + address: Address! + } + + type Address { + country: String! + } + ` + fourthDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "User", + FieldNames: []string{"id", "address"}, + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Address", + FieldNames: []string{"country"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://fourth.service", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: fourthSubgraphSDL, + }, + UpstreamSchema: fourthSubgraphSDL, + }), + Factory: federationFactory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "User", + SelectionSet: "id", + }, + }, + }, + } + + dataSources := []plan.DataSourceConfiguration{ + firstDatasourceConfiguration, + secondDatasourceConfiguration, + thirdDatasourceConfiguration, + fourthDatasourceConfiguration, + } + + planConfiguration := plan.Configuration{ + DataSources: ShuffleDS(dataSources), + DisableResolveFieldPositions: true, + Debug: plan.DebugConfiguration{ + PrintOperationTransformations: true, + PrintQueryPlans: true, + PrintPlanningPaths: true, + PrintNodeSuggestions: true, + + // DatasourceVisitor: true, + }, + } + + t.Run("run", RunTest( + definition, + ` + query User { + user { + id + name + title + address { + country + } + } + }`, + "User", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://first.service","body":{"query":"{user {id __typename}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + DataSource: &Source{}, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + Fields: []*resolve.Field{ + { + Name: []byte("user"), + Value: &resolve.Object{ + Path: []string{"user"}, + Nullable: false, + Fields: []*resolve.Field{ + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + }, + { + Name: []byte("name"), + Value: &resolve.String{ + Path: []string{"name"}, + }, + }, + { + Name: []byte("title"), + Value: &resolve.String{ + Path: []string{"title"}, + }, + }, + { + Name: []byte("address"), + Value: &resolve.Object{ + Path: []string{"address"}, + Fields: []*resolve.Field{ + { + Name: []byte("country"), + Value: &resolve.String{ + Path: []string{"country"}, + }, + }, + }, + }, + }, + }, + Fetch: &resolve.ParallelFetch{ + Fetches: []resolve.Fetch{ + &resolve.SerialFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{ + SerialID: 2, + FetchConfiguration: resolve.FetchConfiguration{ + RequiresEntityBatchFetch: false, + RequiresEntityFetch: true, + Input: `{"method":"POST","url":"http://second.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {name uuid}}}","variables":{"representations":[$$0$$]}}}`, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + }, + }), + }, + }, + PostProcessing: SingleEntityPostProcessingConfiguration, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + &resolve.SingleFetch{ + SerialID: 3, + FetchConfiguration: resolve.FetchConfiguration{ + RequiresEntityBatchFetch: false, + RequiresEntityFetch: true, + Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {title}}}","variables":{"representations":[$$0$$]}}}`, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("uuid"), + Value: &resolve.String{ + Path: []string{"uuid"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + }, + }), + }, + }, + PostProcessing: SingleEntityPostProcessingConfiguration, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + &resolve.SingleFetch{ + SerialID: 3, + FetchConfiguration: resolve.FetchConfiguration{ + RequiresEntityBatchFetch: false, + RequiresEntityFetch: true, + Input: `{"method":"POST","url":"http://fourth.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {address {country}}}}","variables":{"representations":[$$0$$]}}}`, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + }, + }), + }, + }, + PostProcessing: SingleEntityPostProcessingConfiguration, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + planConfiguration, + )) + }) } From 3670dbda8c65883aa92a740e2b30cd02420a178b Mon Sep 17 00:00:00 2001 From: spetrunin Date: Mon, 1 Jan 2024 15:53:55 +0200 Subject: [PATCH 02/27] setup fetch dependencies remove requires serial fetch --- .../graphql_datasource/graphql_datasource.go | 16 -- .../graphql_datasource_federation_test.go | 62 ++++--- .../introspection_datasource/planner_test.go | 4 +- v2/pkg/engine/plan/configuration_visitor.go | 158 ++++++++++++------ v2/pkg/engine/plan/planner.go | 1 - v2/pkg/engine/plan/planner_configuration.go | 7 +- v2/pkg/engine/plan/required_fields_visitor.go | 26 ++- v2/pkg/engine/plan/visitor.go | 39 ++--- v2/pkg/engine/postprocess/datasourceinput.go | 2 +- .../postprocess/datasourceinput_test.go | 12 +- v2/pkg/engine/resolve/fetch.go | 17 +- 11 files changed, 191 insertions(+), 153 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index b82be9f9e..abd32f8dd 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -407,7 +407,6 @@ func (p *Planner) ConfigureFetch() resolve.FetchConfiguration { }, Variables: p.variables, DisallowSingleFlight: p.disallowSingleFlight, - RequiresSerialFetch: p.requiresSerialFetch(), RequiresEntityFetch: p.requiresEntityFetch(), RequiresEntityBatchFetch: p.requiresEntityBatchFetch(), PostProcessing: postProcessing, @@ -420,21 +419,6 @@ func (p *Planner) shouldSelectSingleEntity() bool { p.dataSourcePlannerConfig.PathType == plan.PlannerPathObject } -func (p *Planner) requiresSerialFetch() bool { - if !p.dataSourcePlannerConfig.HasRequiredFields() { - return false - } - - for _, fieldConfig := range p.dataSourcePlannerConfig.RequiredFields { - if fieldConfig.FieldName != "" { - // if there field name in field config representation variables includes fields from @requires directive - return true - } - } - - return false -} - func (p *Planner) requiresEntityFetch() bool { return p.dataSourcePlannerConfig.HasRequiredFields() && p.dataSourcePlannerConfig.PathType == plan.PlannerPathObject } 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 7a25af989..2b00c8b29 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 @@ -372,7 +372,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://user.service","body":{"query":"{user {account {__typename id info {a b}}}}"}}`, @@ -416,7 +416,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Account {name shippingInfo {zip}}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -505,7 +505,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://user.service","body":{"query":"{user {account {__typename id info {a b}}}}"}}`, @@ -592,7 +592,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, Info: &resolve.FetchInfo{ DataSourceID: "account.service", }, @@ -682,7 +682,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input, @@ -719,10 +719,9 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetch: &resolve.SerialFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ - RequiresSerialFetch: true, Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Address {fullAddress}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, PostProcessing: SingleEntityPostProcessingConfiguration, @@ -782,10 +781,9 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, &resolve.SingleFetch{ - SerialID: 2, + FetchID: 2, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ - RequiresSerialFetch: true, Input: `{"method":"POST","url":"http://address.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Address {line3(test: "BOOM") zip}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, PostProcessing: SingleEntityPostProcessingConfiguration, @@ -831,7 +829,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, &resolve.SingleFetch{ - SerialID: 3, + FetchID: 3, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://address-enricher.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Address {country city}}}","variables":{"representations":[$$0$$]}}}`, @@ -930,7 +928,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://user.service","body":{"query":"{user {oldAccount {name shippingInfo {zip}}}}"}}`, @@ -1010,7 +1008,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://user.service","body":{"query":"{user {account {__typename id info {a b}} oldAccount {name shippingInfo {zip}}}}"}}`, @@ -1054,7 +1052,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Account {name shippingInfo {zip}}}}","variables":{"representations":[$$0$$]}}}`, @@ -1436,7 +1434,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://second.service","body":{"query":"{me {details {forename surname}}}"}}`, @@ -1488,7 +1486,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input1, @@ -1531,7 +1529,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input2, @@ -1633,7 +1631,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input1, @@ -1664,7 +1662,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input2, @@ -1764,7 +1762,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input1, @@ -1801,7 +1799,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input2, @@ -1924,7 +1922,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input1, @@ -1975,7 +1973,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetch: &resolve.ParallelFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://second.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {details {surname}}}}","variables":{"representations":[$$0$$]}}}`, @@ -2009,7 +2007,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, &resolve.SingleFetch{ - SerialID: 2, + FetchID: 2, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {details {age}}}}","variables":{"representations":[$$0$$]}}}`, @@ -2276,7 +2274,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, @@ -2370,7 +2368,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, @@ -2699,7 +2697,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetch: &resolve.ParallelFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://second.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {name} ... on Admin {adminName}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -2747,7 +2745,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), }, &resolve.SingleFetch{ - SerialID: 2, + FetchID: 2, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Moderator {subject}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -2833,7 +2831,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetch: &resolve.ParallelFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Moderator {title}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -2867,7 +2865,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), }, &resolve.SingleFetch{ - SerialID: 2, + FetchID: 2, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://second.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {title}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -3182,7 +3180,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { &resolve.SerialFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - SerialID: 2, + FetchID: 2, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, @@ -3217,7 +3215,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), }, &resolve.SingleFetch{ - SerialID: 3, + FetchID: 3, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, @@ -3254,7 +3252,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, &resolve.SingleFetch{ - SerialID: 3, + FetchID: 3, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, diff --git a/v2/pkg/engine/datasource/introspection_datasource/planner_test.go b/v2/pkg/engine/datasource/introspection_datasource/planner_test.go index 7a07986a7..1630614e3 100644 --- a/v2/pkg/engine/datasource/introspection_datasource/planner_test.go +++ b/v2/pkg/engine/datasource/introspection_datasource/planner_test.go @@ -376,7 +376,7 @@ func TestIntrospectionDataSourcePlanning(t *testing.T) { Fetch: &resolve.ParallelFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, DataSourceIdentifier: dataSourceIdentifier, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"request_type":3,"on_type_name":"$$0$$","include_deprecated":$$1$$}`, @@ -397,7 +397,7 @@ func TestIntrospectionDataSourcePlanning(t *testing.T) { }, }, &resolve.SingleFetch{ - SerialID: 2, + FetchID: 2, DataSourceIdentifier: dataSourceIdentifier, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"request_type":4,"on_type_name":"$$0$$","include_deprecated":$$1$$}`, diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index 89ff2cd1c..f3b56af97 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -22,25 +22,31 @@ type configurationVisitor struct { dataSources []DataSourceConfiguration planners []*plannerConfiguration - fetches []objectFetchConfiguration nodeSuggestions NodeSuggestions // nodeSuggestions holds information about suggested data sources for each field - currentFetchID int // currentFetchID is used to generate serial fetch IDs for each fetch parentTypeNodes []ast.Node // parentTypeNodes is a stack of parent type nodes - used to determine if the parent is abstract arrayFields []arrayField // arrayFields is a stack of array fields - used to plan nested queries selectionSetRefs []int // selectionSetRefs is a stack of selection set refs - used to add a required fields - skipFieldsRefs []int // skipFieldsRefs holds required field refs which should not be added to user response + skipFieldsRefs []int // skipFieldsRefs holds required field refs added by planner and should not be added to user response missingPathTracker map[string]missingPath // missingPathTracker is a map of paths which will be added on secondary runs addedPathTracker []pathConfiguration // addedPathTracker is a list of paths which were added - pendingRequiredFields map[int][]string // pendingRequiredFields is a map[selectionSetRef][]RequiredFieldsSelectionSet - handledRequires map[int]struct{} // handledRequires is a map[FieldRef] of already processed fields which has @requires directive - visitedFields map[int]struct{} // visitedFields is a map[FieldRef] of already processed fields which we check for abstract type, e.g. union or interface + pendingRequiredFields map[int][]fieldsRequiredByPlanner // pendingRequiredFields is a map[selectionSetRef][]fieldsRequiredByPlanner + handledRequires map[int]struct{} // handledRequires is a map[FieldRef] of already processed fields which has @requires directive + visitedFields map[int]struct{} // visitedFields is a map[FieldRef] of already processed fields which we check for abstract type, e.g. union or interface + requiredFieldsMapping map[int][]int // requiredFieldsMapping is a map[FieldRef][]plannerIdx holds dependencies between fields and planners secondaryRun bool // secondaryRun is a flag to indicate that we're running the planner 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 } +// fieldsRequiredByPlanner is a mapping between planner id which requested required fields +// and a list of fields which should be added +type fieldsRequiredByPlanner struct { + plannerIds []int + fieldSelections string +} + type arrayField struct { fieldRef int fieldPath string @@ -63,8 +69,9 @@ type objectFetchConfiguration struct { isSubscription bool fieldRef int fieldDefinitionRef int - fetchID int sourceID string + fetchID int + dependsOnFetchIDs []int } func (c *configurationVisitor) currentSelectionSet() int { @@ -235,18 +242,13 @@ func (c *configurationVisitor) EnterDocument(operation, definition *ast.Document } c.operation, c.definition = operation, definition - c.currentFetchID = -1 c.parentTypeNodes = c.parentTypeNodes[:0] if c.planners == nil { c.planners = make([]*plannerConfiguration, 0, 8) } else { c.planners = c.planners[:0] } - if c.fetches == nil { - c.fetches = []objectFetchConfiguration{} - } else { - c.fetches = c.fetches[:0] - } + if c.skipFieldsRefs == nil { c.skipFieldsRefs = make([]int, 0, 8) } else { @@ -256,7 +258,8 @@ func (c *configurationVisitor) EnterDocument(operation, definition *ast.Document c.missingPathTracker = make(map[string]missingPath) c.addedPathTracker = make([]pathConfiguration, 0, 8) - c.pendingRequiredFields = make(map[int][]string) + c.pendingRequiredFields = make(map[int][]fieldsRequiredByPlanner) + c.requiredFieldsMapping = make(map[int][]int) c.handledRequires = make(map[int]struct{}) c.visitedFields = make(map[int]struct{}) } @@ -354,8 +357,9 @@ func (c *configurationVisitor) EnterField(ref int) { } if planned { - c.handleRequirements(plannerIdx, parentPath, currentPath, typeName, fieldName, ref) + c.handleRequirements(plannerIdx, parentPath, typeName, fieldName, ref) c.rewriteSelectionSetOfFieldWithInterfaceType(ref, plannerIdx) + c.addPlannerDependencies(ref, plannerIdx) return } @@ -363,7 +367,27 @@ func (c *configurationVisitor) EnterField(ref int) { c.handleMissingPath(typeName, fieldName, currentPath) } -func (c *configurationVisitor) handleRequirements(plannerIdx int, parentPath string, currentPath string, typeName, fieldName string, fieldRef int) { +func (c *configurationVisitor) addPlannerDependencies(fieldRef int, plannerIdx int) { + plannerIds, mappingExists := c.requiredFieldsMapping[fieldRef] + if !mappingExists { + return + } + + for _, notifyPlannerIdx := range plannerIds { + notified := false + for _, existingPlannerId := range c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs { + if existingPlannerId == plannerIdx { + notified = true + break + } + } + if !notified { + c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs = append(c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs, plannerIdx) + } + } +} + +func (c *configurationVisitor) handleRequirements(plannerIdx int, parentPath string, typeName, fieldName string, fieldRef int) { plannerConfig := c.planners[plannerIdx] dsHash := plannerConfig.dataSourceConfiguration.Hash() @@ -374,7 +398,7 @@ func (c *configurationVisitor) handleRequirements(plannerIdx int, parentPath str } // add required fields for field and type (@requires) - c.handleFieldRequiredByRequires(plannerConfig, currentPath, typeName, fieldName, fieldRef) + c.handleFieldRequiredByRequires(plannerIdx, plannerConfig, typeName, fieldName, fieldRef) } func (c *configurationVisitor) handleProvidesSuggestions(ref int, typeName, fieldName, currentPath string) { @@ -578,7 +602,6 @@ func (c *configurationVisitor) addNewPlanner(ref int, typeName, fieldName, curre return -1, false } - fetchID := c.nextFetchID() planner := config.Factory.Planner(c.ctx) isParentAbstract := c.isParentTypeNodeAbstractType() @@ -646,28 +669,33 @@ func (c *configurationVisitor) addNewPlanner(ref int, typeName, fieldName, curre plannerPath = precedingFragmentPath } - plannerConfig := &plannerConfiguration{ - dataSourceConfiguration: *config, - parentPath: plannerPath, - planner: planner, - paths: paths, - parentPathType: c.plannerPathType(plannerPath), - } - fieldDefinition, ok := c.walker.FieldDefinition(ref) if !ok { return -1, false } - c.planners = append(c.planners, plannerConfig) - c.fetches = append(c.fetches, objectFetchConfiguration{ + // fetch id is an index of the current planner + fetchID := len(c.planners) + + fetchConfiguration := objectFetchConfiguration{ planner: planner, isSubscription: isSubscription, fieldRef: ref, fieldDefinitionRef: fieldDefinition, fetchID: fetchID, sourceID: config.ID, - }) + } + + plannerConfig := &plannerConfiguration{ + dataSourceConfiguration: *config, + objectFetchConfiguration: fetchConfiguration, + parentPath: plannerPath, + planner: planner, + paths: paths, + parentPathType: c.plannerPathType(plannerPath), + } + + c.planners = append(c.planners, plannerConfig) for _, pathConfiguration := range paths { c.saveAddedPath(pathConfiguration) @@ -822,12 +850,7 @@ func (c *configurationVisitor) isSubscription(root int, path string) bool { return strings.Count(path, ".") == 1 } -func (c *configurationVisitor) nextFetchID() int { - c.currentFetchID++ - return c.currentFetchID -} - -func (c *configurationVisitor) handleFieldRequiredByRequires(config *plannerConfiguration, currentPath string, typeName, fieldName string, fieldRef int) { +func (c *configurationVisitor) handleFieldRequiredByRequires(plannerIdx int, config *plannerConfiguration, typeName, fieldName string, fieldRef int) { if _, ok := c.handledRequires[fieldRef]; ok { return } @@ -835,7 +858,7 @@ func (c *configurationVisitor) handleFieldRequiredByRequires(config *plannerConf requiredFieldsForTypeAndField := config.dataSourceConfiguration.RequiredFieldsByRequires(typeName, fieldName) for _, requiredFieldsConfiguration := range requiredFieldsForTypeAndField { - c.planAddingRequiredFields(requiredFieldsConfiguration) + c.planAddingRequiredFields(plannerIdx, requiredFieldsConfiguration) var added bool config.requiredFields, added = appendRequiredFieldsConfigurationIfNotPresent(config.requiredFields, requiredFieldsConfiguration) if added { @@ -869,7 +892,7 @@ func (c *configurationVisitor) planKeyRequiredFields(plannerIdx int, parentPath } for _, possibleRequiredFieldConfig := range possibleRequiredFields { if c.planners[i].dataSourceConfiguration.HasKeyRequirement(typeName, possibleRequiredFieldConfig.SelectionSet) { - c.planAddingRequiredFields(possibleRequiredFieldConfig) + c.planAddingRequiredFields(plannerIdx, possibleRequiredFieldConfig) return possibleRequiredFieldConfig, true } } @@ -878,26 +901,50 @@ func (c *configurationVisitor) planKeyRequiredFields(plannerIdx int, parentPath return FederationFieldConfiguration{}, false } -func (c *configurationVisitor) planAddingRequiredFields(fieldConfiguration FederationFieldConfiguration) { +func (c *configurationVisitor) planAddingRequiredFields(plannerIdx int, fieldConfiguration FederationFieldConfiguration) { currentSelectionSet := c.currentSelectionSet() - selectionSets, hasSelectionSet := c.pendingRequiredFields[currentSelectionSet] - if !hasSelectionSet { - selectionSets = make([]string, 0, 2) - c.pendingRequiredFields[currentSelectionSet] = append(selectionSets, fieldConfiguration.SelectionSet) + requirements, hasRequirements := c.pendingRequiredFields[currentSelectionSet] + // no requirement was added for the current selection set + if !hasRequirements { + requirements = make([]fieldsRequiredByPlanner, 0, 2) + c.pendingRequiredFields[currentSelectionSet] = append(requirements, fieldsRequiredByPlanner{ + plannerIds: []int{plannerIdx}, + fieldSelections: fieldConfiguration.SelectionSet, + }) return } - exists := false - for _, existingSelectionSet := range selectionSets { - if existingSelectionSet == fieldConfiguration.SelectionSet { - exists = true + requirementExists := false + for i := range requirements { + if requirements[i].fieldSelections == fieldConfiguration.SelectionSet { + requirementExists = true + + plannerIdExists := false + for _, plannerId := range requirements[i].plannerIds { + if plannerId == plannerIdx { + plannerIdExists = true + break + } + } + // when we have already added the same requirements for the current selection set + // but not for such planner id + if !plannerIdExists { + requirements[i].plannerIds = append(requirements[i].plannerIds, plannerIdx) + } break } } - if !exists { - c.pendingRequiredFields[currentSelectionSet] = append(selectionSets, fieldConfiguration.SelectionSet) + + // when no such requirement was added for the current selection set + if !requirementExists { + requirements = append(requirements, fieldsRequiredByPlanner{ + plannerIds: []int{plannerIdx}, + fieldSelections: fieldConfiguration.SelectionSet, + }) } + + c.pendingRequiredFields[currentSelectionSet] = requirements } func (c *configurationVisitor) processPendingRequiredFields(selectionSetRef int) { @@ -906,14 +953,14 @@ func (c *configurationVisitor) processPendingRequiredFields(selectionSetRef int) return } - for _, requiredFields := range configs { - c.addRequiredFields(selectionSetRef, requiredFields) + for _, requiredFieldsCfg := range configs { + c.addRequiredFields(selectionSetRef, requiredFieldsCfg.plannerIds, requiredFieldsCfg.fieldSelections) } delete(c.pendingRequiredFields, selectionSetRef) } -func (c *configurationVisitor) addRequiredFields(selectionSetRef int, requiredFields string) { +func (c *configurationVisitor) addRequiredFields(selectionSetRef int, plannerIds []int, requiredFields string) { typeName := c.walker.EnclosingTypeDefinition.NameString(c.definition) key, report := RequiredFieldsFragment(typeName, requiredFields, true) if report.HasErrors() { @@ -928,14 +975,19 @@ func (c *configurationVisitor) addRequiredFields(selectionSetRef int, requiredFi definition: c.definition, report: report, operationSelectionSet: selectionSetRef, - skipFieldRefs: &c.skipFieldsRefs, parentPath: parentPath, } - addRequiredFields(input) + skipFieldRefs, requiredFieldRefs := addRequiredFields(input) if report.HasErrors() { c.walker.StopWithInternalErr(fmt.Errorf("failed to add required fields for %s", typeName)) } + + c.skipFieldsRefs = append(c.skipFieldsRefs, skipFieldRefs...) + + for _, fieldRef := range requiredFieldRefs { + c.requiredFieldsMapping[fieldRef] = append(c.requiredFieldsMapping[fieldRef], plannerIds...) + } } func (c *configurationVisitor) rewriteSelectionSetOfFieldWithInterfaceType(fieldRef int, plannerIdx int) { diff --git a/v2/pkg/engine/plan/planner.go b/v2/pkg/engine/plan/planner.go index 524e04393..f261c2ff8 100644 --- a/v2/pkg/engine/plan/planner.go +++ b/v2/pkg/engine/plan/planner.go @@ -98,7 +98,6 @@ func (p *Planner) Plan(operation, definition *ast.Document, operationName string p.planningVisitor.planners = p.configurationVisitor.planners p.planningVisitor.Config = p.config - p.planningVisitor.fetchConfigurations = p.configurationVisitor.fetches p.planningVisitor.skipFieldsRefs = p.configurationVisitor.skipFieldsRefs p.planningVisitor.allowFieldMerge = !hasConditionalSkipInclude diff --git a/v2/pkg/engine/plan/planner_configuration.go b/v2/pkg/engine/plan/planner_configuration.go index 40e362e6d..cbb0d12aa 100644 --- a/v2/pkg/engine/plan/planner_configuration.go +++ b/v2/pkg/engine/plan/planner_configuration.go @@ -11,9 +11,10 @@ type plannerConfiguration struct { parentPath string parentPathType PlannerPathType - planner DataSourcePlanner - paths []pathConfiguration - dataSourceConfiguration DataSourceConfiguration + planner DataSourcePlanner + paths []pathConfiguration + dataSourceConfiguration DataSourceConfiguration + objectFetchConfiguration objectFetchConfiguration requiredFields FederationFieldConfigurations providedFields NodeSuggestions diff --git a/v2/pkg/engine/plan/required_fields_visitor.go b/v2/pkg/engine/plan/required_fields_visitor.go index 6afa92f0b..f9b948934 100644 --- a/v2/pkg/engine/plan/required_fields_visitor.go +++ b/v2/pkg/engine/plan/required_fields_visitor.go @@ -29,32 +29,37 @@ type addRequiredFieldsInput struct { key, operation, definition *ast.Document report *operationreport.Report operationSelectionSet int - skipFieldRefs *[]int parentPath string } -func addRequiredFields(input *addRequiredFieldsInput) { +func addRequiredFields(input *addRequiredFieldsInput) (skipFieldRefs []int, requiredFieldRefs []int) { walker := astvisitor.NewWalker(48) importer := &astimport.Importer{} visitor := &requiredFieldsVisitor{ - Walker: &walker, - input: input, - importer: importer, + Walker: &walker, + input: input, + importer: importer, + skipFieldRefs: make([]int, 0, 2), + requiredFieldRefs: make([]int, 0, 2), } walker.RegisterEnterDocumentVisitor(visitor) walker.RegisterFieldVisitor(visitor) walker.RegisterEnterSelectionSetVisitor(visitor) walker.Walk(input.key, input.definition, input.report) + + return visitor.skipFieldRefs, visitor.requiredFieldRefs } type requiredFieldsVisitor struct { *astvisitor.Walker - OperationNodes []ast.Node - input *addRequiredFieldsInput - importer *astimport.Importer + OperationNodes []ast.Node + input *addRequiredFieldsInput + importer *astimport.Importer + skipFieldRefs []int + requiredFieldRefs []int } func (v *requiredFieldsVisitor) EnterDocument(_, _ *ast.Document) { @@ -92,6 +97,8 @@ func (v *requiredFieldsVisitor) EnterField(ref int) { operationHasField, operationFieldRef := v.input.operation.SelectionSetHasFieldSelectionWithExactName(selectionSetRef, fieldName) if operationHasField { + v.requiredFieldRefs = append(v.requiredFieldRefs, operationFieldRef) + // do not add required field if the field is already present in the operation with the same name // but add an operation node from operation if the field has selections if !v.input.operation.FieldHasSelections(operationFieldRef) { @@ -135,7 +142,8 @@ func (v *requiredFieldsVisitor) addRequiredField(keyRef int, fieldName ast.ByteS } v.input.operation.AddSelection(selectionSet, selection) - *v.input.skipFieldRefs = append(*v.input.skipFieldRefs, addedField.Ref) + v.skipFieldRefs = append(v.skipFieldRefs, addedField.Ref) + v.requiredFieldRefs = append(v.requiredFieldRefs, addedField.Ref) return addedField } diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index fe8119673..ed4b6418e 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -34,7 +34,6 @@ type Visitor struct { currentFields []objectFields currentField *resolve.Field planners []*plannerConfiguration - fetchConfigurations []objectFetchConfiguration skipFieldsRefs []int fieldConfigs map[int]*FieldConfiguration exportedVariables map[string]struct{} @@ -434,15 +433,15 @@ func (v *Visitor) resolveFieldInfo(ref, typeRef int, onTypeNames [][]byte) *reso } func (v *Visitor) linkFetchConfiguration(fieldRef int) { - for i := range v.fetchConfigurations { - if fieldRef == v.fetchConfigurations[i].fieldRef { - if v.fetchConfigurations[i].isSubscription { + for i := range v.planners { + if fieldRef == v.planners[i].objectFetchConfiguration.fieldRef { + if v.planners[i].objectFetchConfiguration.isSubscription { plan, ok := v.plan.(*SubscriptionResponsePlan) if ok { - v.fetchConfigurations[i].trigger = &plan.Response.Trigger + v.planners[i].objectFetchConfiguration.trigger = &plan.Response.Trigger } } else { - v.fetchConfigurations[i].object = v.objects[len(v.objects)-1] + v.planners[i].objectFetchConfiguration.object = v.objects[len(v.objects)-1] } } } @@ -841,11 +840,11 @@ func (v *Visitor) EnterDocument(operation, definition *ast.Document) { } func (v *Visitor) LeaveDocument(_, _ *ast.Document) { - for _, config := range v.fetchConfigurations { - if config.isSubscription { - v.configureSubscription(config) + for i := range v.planners { + if v.planners[i].objectFetchConfiguration.isSubscription { + v.configureSubscription(v.planners[i].objectFetchConfiguration) } else { - v.configureObjectFetch(config) + v.configureObjectFetch(v.planners[i].objectFetchConfiguration) } } } @@ -1093,20 +1092,11 @@ func (v *Visitor) configureObjectFetch(config objectFetchConfiguration) { switch existing := config.object.Fetch.(type) { case *resolve.SingleFetch: copyOfExisting := *existing - if copyOfExisting.RequiresSerialFetch { - serial := &resolve.SerialFetch{ - Fetches: []resolve.Fetch{©OfExisting, fetch}, - } - config.object.Fetch = serial - } else { - parallel := &resolve.ParallelFetch{ - Fetches: []resolve.Fetch{©OfExisting, fetch}, - } - config.object.Fetch = parallel + multi := &resolve.MultiFetch{ + Fetches: []resolve.Fetch{©OfExisting, fetch}, } - case *resolve.ParallelFetch: - existing.Fetches = append(existing.Fetches, fetch) - case *resolve.SerialFetch: + config.object.Fetch = multi + case *resolve.MultiFetch: existing.Fetches = append(existing.Fetches, fetch) } } @@ -1117,7 +1107,8 @@ func (v *Visitor) configureFetch(internal objectFetchConfiguration, external res singleFetch := &resolve.SingleFetch{ FetchConfiguration: external, - SerialID: internal.fetchID, + FetchID: internal.fetchID, + DependsOnFetchIDs: internal.dependsOnFetchIDs, DataSourceIdentifier: []byte(dataSourceType), } diff --git a/v2/pkg/engine/postprocess/datasourceinput.go b/v2/pkg/engine/postprocess/datasourceinput.go index 8b0129041..e5eabf297 100644 --- a/v2/pkg/engine/postprocess/datasourceinput.go +++ b/v2/pkg/engine/postprocess/datasourceinput.go @@ -50,7 +50,7 @@ func (d *ProcessDataSource) traverseFetch(fetch resolve.Fetch) { slices.SortFunc(f.Fetches, func(a, b resolve.Fetch) int { // serial fetch always has a single fetch as child // sort descending by serial id - return b.(*resolve.SingleFetch).SerialID - a.(*resolve.SingleFetch).SerialID + return b.(*resolve.SingleFetch).FetchID - a.(*resolve.SingleFetch).FetchID }) for i := range f.Fetches { d.traverseFetch(f.Fetches[i]) diff --git a/v2/pkg/engine/postprocess/datasourceinput_test.go b/v2/pkg/engine/postprocess/datasourceinput_test.go index 0d6400b3d..e2b9e062c 100644 --- a/v2/pkg/engine/postprocess/datasourceinput_test.go +++ b/v2/pkg/engine/postprocess/datasourceinput_test.go @@ -362,9 +362,9 @@ func TestDataSourceInput_ProcessSerialFetch(t *testing.T) { Data: &resolve.Object{ Fetch: &resolve.SerialFetch{ Fetches: []resolve.Fetch{ - &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `a`}, SerialID: 0}, - &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `b`}, SerialID: 2}, - &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `c`}, SerialID: 5}, + &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `a`}, FetchID: 0}, + &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `b`}, FetchID: 2}, + &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `c`}, FetchID: 5}, }, }, }, @@ -377,7 +377,7 @@ func TestDataSourceInput_ProcessSerialFetch(t *testing.T) { Fetch: &resolve.SerialFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - SerialID: 5, + FetchID: 5, InputTemplate: resolve.InputTemplate{ Segments: []resolve.TemplateSegment{ { @@ -388,7 +388,7 @@ func TestDataSourceInput_ProcessSerialFetch(t *testing.T) { }, }, &resolve.SingleFetch{ - SerialID: 2, + FetchID: 2, InputTemplate: resolve.InputTemplate{ Segments: []resolve.TemplateSegment{ { @@ -399,7 +399,7 @@ func TestDataSourceInput_ProcessSerialFetch(t *testing.T) { }, }, &resolve.SingleFetch{ - SerialID: 0, + FetchID: 0, InputTemplate: resolve.InputTemplate{ Segments: []resolve.TemplateSegment{ { diff --git a/v2/pkg/engine/resolve/fetch.go b/v2/pkg/engine/resolve/fetch.go index 289b5018f..6190a8a8b 100644 --- a/v2/pkg/engine/resolve/fetch.go +++ b/v2/pkg/engine/resolve/fetch.go @@ -13,17 +13,25 @@ const ( FetchKindParallelListItem FetchKindEntity FetchKindEntityBatch + FetchKindMulti ) type Fetch interface { FetchKind() FetchKind } -type Fetches []Fetch +type MultiFetch struct { + Fetches []Fetch +} + +func (_ *MultiFetch) FetchKind() FetchKind { + return FetchKindMulti +} type SingleFetch struct { FetchConfiguration - SerialID int + FetchID int + DependsOnFetchIDs []int InputTemplate InputTemplate DataSourceIdentifier []byte Trace *DataSourceLoadTrace @@ -151,10 +159,7 @@ type FetchConfiguration struct { // By default SingleFlight for fetches is disabled and needs to be enabled on the Resolver first // If the resolver allows SingleFlight it's up to each individual DataSource Planner to decide whether an Operation // should be allowed to use SingleFlight - DisallowSingleFlight bool - // RequiresSerialFetch is used to indicate that the single fetches should be executed serially - // When we have multiple fetches attached to the object - after post-processing of a plan we will get SerialFetch instead of ParallelFetch - RequiresSerialFetch bool + DisallowSingleFlight bool // TODO: remove - unused // RequiresParallelListItemFetch is used to indicate that the single fetches should be executed without batching // When we have multiple fetches attached to the object - after post-processing of a plan we will get ParallelListItemFetch instead of ParallelFetch RequiresParallelListItemFetch bool From 840826a005ae1fa02ca503e3250259368d10c3a8 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Mon, 1 Jan 2024 21:08:56 +0200 Subject: [PATCH 03/27] remove disallowSingleFlight --- .../graphql_datasource/graphql_datasource.go | 4 --- .../graphql_datasource_test.go | 26 +++++++------------ .../staticdatasource/static_datasource.go | 5 ++-- .../static_datasource_test.go | 5 ++-- v2/pkg/engine/postprocess/datasourcefetch.go | 10 +++---- v2/pkg/engine/resolve/fetch.go | 7 ----- 6 files changed, 17 insertions(+), 40 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index abd32f8dd..5e9dd84f4 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -60,7 +60,6 @@ type Planner struct { nodes []ast.Node variables resolve.Variables lastFieldEnclosingTypeName string - disallowSingleFlight bool fetchClient *http.Client subscriptionClient GraphQLSubscriptionClient rootTypeName string // rootTypeName - holds name of top level type @@ -406,7 +405,6 @@ func (p *Planner) ConfigureFetch() resolve.FetchConfiguration { httpClient: p.fetchClient, }, Variables: p.variables, - DisallowSingleFlight: p.disallowSingleFlight, RequiresEntityFetch: p.requiresEntityFetch(), RequiresEntityBatchFetch: p.requiresEntityBatchFetch(), PostProcessing: postProcessing, @@ -487,7 +485,6 @@ func (p *Planner) EnterOperationDefinition(ref int) { definition := p.upstreamOperation.AddOperationDefinitionToRootNodes(ast.OperationDefinition{ OperationType: operationType, }) - p.disallowSingleFlight = operationType == ast.OperationTypeMutation p.nodes = append(p.nodes, definition) } @@ -761,7 +758,6 @@ func (p *Planner) EnterDocument(_, _ *ast.Document) { p.parentTypeNodes = p.parentTypeNodes[:0] p.upstreamVariables = nil p.variables = p.variables[:0] - p.disallowSingleFlight = false p.hasFederationRoot = false p.extractEntities = false diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go index e9d11181d..db74ba64e 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go @@ -2712,8 +2712,7 @@ func TestGraphQLDataSource(t *testing.T) { Renderer: resolve.NewJSONVariableRendererWithValidation(`{"type":["string"]}`), }, ), - DisallowSingleFlight: true, - PostProcessing: DefaultPostProcessingConfiguration, + PostProcessing: DefaultPostProcessingConfiguration, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), }, @@ -2814,8 +2813,7 @@ func TestGraphQLDataSource(t *testing.T) { Renderer: resolve.NewJSONVariableRendererWithValidation(`{"type":["string","null"]}`), }, ), - DisallowSingleFlight: false, - PostProcessing: DefaultPostProcessingConfiguration, + PostProcessing: DefaultPostProcessingConfiguration, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), }, @@ -2919,8 +2917,7 @@ func TestGraphQLDataSource(t *testing.T) { Renderer: resolve.NewJSONVariableRendererWithValidation(`{"type":["string","integer"]}`), }, ), - DisallowSingleFlight: false, - PostProcessing: DefaultPostProcessingConfiguration, + PostProcessing: DefaultPostProcessingConfiguration, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), }, @@ -3042,8 +3039,7 @@ func TestGraphQLDataSource(t *testing.T) { Renderer: resolve.NewJSONVariableRendererWithValidation(`{"type":["string","integer"]}`), }, ), - DisallowSingleFlight: false, - PostProcessing: DefaultPostProcessingConfiguration, + PostProcessing: DefaultPostProcessingConfiguration, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), }, @@ -3538,8 +3534,7 @@ func TestGraphQLDataSource(t *testing.T) { Renderer: resolve.NewJSONVariableRendererWithValidation(`{"type":["string"]}`), }, ), - DisallowSingleFlight: true, - PostProcessing: DefaultPostProcessingConfiguration, + PostProcessing: DefaultPostProcessingConfiguration, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), }, @@ -3681,8 +3676,7 @@ func TestGraphQLDataSource(t *testing.T) { Renderer: resolve.NewJSONVariableRendererWithValidation(`{"type":["string","null"]}`), }, ), - DisallowSingleFlight: true, - PostProcessing: DefaultPostProcessingConfiguration, + PostProcessing: DefaultPostProcessingConfiguration, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), }, @@ -3807,8 +3801,7 @@ func TestGraphQLDataSource(t *testing.T) { Renderer: resolve.NewJSONVariableRendererWithValidation(`{"type":["boolean"]}`), }, ), - DisallowSingleFlight: true, - PostProcessing: DefaultPostProcessingConfiguration, + PostProcessing: DefaultPostProcessingConfiguration, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), }, @@ -3945,9 +3938,8 @@ func TestGraphQLDataSource(t *testing.T) { Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://api.com","body":{"query":"mutation($name: String!, $personal: Boolean!){namespaceCreate(input: {name: $name,personal: $personal}){__typename}}","variables":{"personal":$$1$$,"name":$$0$$}}}`, - DataSource: &Source{}, - DisallowSingleFlight: true, + Input: `{"method":"POST","url":"http://api.com","body":{"query":"mutation($name: String!, $personal: Boolean!){namespaceCreate(input: {name: $name,personal: $personal}){__typename}}","variables":{"personal":$$1$$,"name":$$0$$}}}`, + DataSource: &Source{}, Variables: resolve.NewVariables( &resolve.ContextVariable{ Path: []string{"name"}, diff --git a/v2/pkg/engine/datasource/staticdatasource/static_datasource.go b/v2/pkg/engine/datasource/staticdatasource/static_datasource.go index c46a89ee2..b29664a96 100644 --- a/v2/pkg/engine/datasource/staticdatasource/static_datasource.go +++ b/v2/pkg/engine/datasource/staticdatasource/static_datasource.go @@ -51,9 +51,8 @@ func (p *Planner) Register(_ *plan.Visitor, configuration plan.DataSourceConfigu func (p *Planner) ConfigureFetch() resolve.FetchConfiguration { return resolve.FetchConfiguration{ - Input: p.config.Data, - DataSource: Source{}, - DisallowSingleFlight: true, + Input: p.config.Data, + DataSource: Source{}, } } diff --git a/v2/pkg/engine/datasource/staticdatasource/static_datasource_test.go b/v2/pkg/engine/datasource/staticdatasource/static_datasource_test.go index 48ca37d4e..b109f4c37 100644 --- a/v2/pkg/engine/datasource/staticdatasource/static_datasource_test.go +++ b/v2/pkg/engine/datasource/staticdatasource/static_datasource_test.go @@ -29,9 +29,8 @@ func TestStaticDataSourcePlanning(t *testing.T) { Fetch: &resolve.SingleFetch{ DataSourceIdentifier: []byte("staticdatasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ - Input: "world", - DataSource: Source{}, - DisallowSingleFlight: true, + Input: "world", + DataSource: Source{}, }, }, }, diff --git a/v2/pkg/engine/postprocess/datasourcefetch.go b/v2/pkg/engine/postprocess/datasourcefetch.go index 04d30150f..309e9d8c9 100644 --- a/v2/pkg/engine/postprocess/datasourcefetch.go +++ b/v2/pkg/engine/postprocess/datasourcefetch.go @@ -112,9 +112,8 @@ func (d *DataSourceFetch) createEntityBatchFetch(fetch *resolve.SingleFetch) res SetTemplateOutputToNullOnVariableNull: fetch.InputTemplate.SetTemplateOutputToNullOnVariableNull, }, }, - DataSource: fetch.DataSource, - PostProcessing: fetch.PostProcessing, - DisallowSingleFlight: fetch.DisallowSingleFlight, + DataSource: fetch.DataSource, + PostProcessing: fetch.PostProcessing, } } @@ -145,8 +144,7 @@ func (d *DataSourceFetch) createEntityFetch(fetch *resolve.SingleFetch) resolve. SetTemplateOutputToNullOnVariableNull: fetch.InputTemplate.SetTemplateOutputToNullOnVariableNull, }, }, - DataSource: fetch.DataSource, - PostProcessing: fetch.PostProcessing, - DisallowSingleFlight: fetch.DisallowSingleFlight, + DataSource: fetch.DataSource, + PostProcessing: fetch.PostProcessing, } } diff --git a/v2/pkg/engine/resolve/fetch.go b/v2/pkg/engine/resolve/fetch.go index 6190a8a8b..b315acae1 100644 --- a/v2/pkg/engine/resolve/fetch.go +++ b/v2/pkg/engine/resolve/fetch.go @@ -93,7 +93,6 @@ type BatchEntityFetch struct { DataSource DataSource PostProcessing PostProcessingConfiguration DataSourceIdentifier []byte - DisallowSingleFlight bool Trace *DataSourceLoadTrace Info *FetchInfo } @@ -122,7 +121,6 @@ type EntityFetch struct { DataSource DataSource PostProcessing PostProcessingConfiguration DataSourceIdentifier []byte - DisallowSingleFlight bool Trace *DataSourceLoadTrace Info *FetchInfo } @@ -155,11 +153,6 @@ type FetchConfiguration struct { Input string Variables Variables DataSource DataSource - // DisallowSingleFlight is used for write operations like mutations, POST, DELETE etc. to disable singleFlight - // By default SingleFlight for fetches is disabled and needs to be enabled on the Resolver first - // If the resolver allows SingleFlight it's up to each individual DataSource Planner to decide whether an Operation - // should be allowed to use SingleFlight - DisallowSingleFlight bool // TODO: remove - unused // RequiresParallelListItemFetch is used to indicate that the single fetches should be executed without batching // When we have multiple fetches attached to the object - after post-processing of a plan we will get ParallelListItemFetch instead of ParallelFetch RequiresParallelListItemFetch bool From 7757e1740f4e7636c91c171f17a2b0fec80a45e5 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Mon, 1 Jan 2024 21:43:21 +0200 Subject: [PATCH 04/27] cleanup postprocess add draft create multi fetch types postprocessor --- ... => create_concrete_single_fetch_types.go} | 18 +++---- .../postprocess/create_multi_fetch_types.go | 50 +++++++++++++++++++ v2/pkg/engine/postprocess/postprocess.go | 5 +- ...rceinput.go => resolve_input_templates.go} | 32 ++++-------- ...est.go => resolve_input_templates_test.go} | 4 +- 5 files changed, 75 insertions(+), 34 deletions(-) rename v2/pkg/engine/postprocess/{datasourcefetch.go => create_concrete_single_fetch_types.go} (83%) create mode 100644 v2/pkg/engine/postprocess/create_multi_fetch_types.go rename v2/pkg/engine/postprocess/{datasourceinput.go => resolve_input_templates.go} (65%) rename v2/pkg/engine/postprocess/{datasourceinput_test.go => resolve_input_templates_test.go} (99%) diff --git a/v2/pkg/engine/postprocess/datasourcefetch.go b/v2/pkg/engine/postprocess/create_concrete_single_fetch_types.go similarity index 83% rename from v2/pkg/engine/postprocess/datasourcefetch.go rename to v2/pkg/engine/postprocess/create_concrete_single_fetch_types.go index 309e9d8c9..0e68d60ba 100644 --- a/v2/pkg/engine/postprocess/datasourcefetch.go +++ b/v2/pkg/engine/postprocess/create_concrete_single_fetch_types.go @@ -5,10 +5,10 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" ) -// DataSourceFetch is a postprocessor that transforms fetches into more concrete fetch types -type DataSourceFetch struct{} +// CreateConcreteSingleFetchTypes is a postprocessor that transforms fetches into more concrete fetch types +type CreateConcreteSingleFetchTypes struct{} -func (d *DataSourceFetch) Process(pre plan.Plan) plan.Plan { +func (d *CreateConcreteSingleFetchTypes) Process(pre plan.Plan) plan.Plan { switch t := pre.(type) { case *plan.SynchronousResponsePlan: d.traverseNode(t.Response.Data) @@ -18,7 +18,7 @@ func (d *DataSourceFetch) Process(pre plan.Plan) plan.Plan { return pre } -func (d *DataSourceFetch) traverseNode(node resolve.Node) { +func (d *CreateConcreteSingleFetchTypes) traverseNode(node resolve.Node) { switch n := node.(type) { case *resolve.Object: n.Fetch = d.traverseFetch(n.Fetch) @@ -30,7 +30,7 @@ func (d *DataSourceFetch) traverseNode(node resolve.Node) { } } -func (d *DataSourceFetch) traverseFetch(fetch resolve.Fetch) resolve.Fetch { +func (d *CreateConcreteSingleFetchTypes) traverseFetch(fetch resolve.Fetch) resolve.Fetch { if fetch == nil { return nil } @@ -54,7 +54,7 @@ func (d *DataSourceFetch) traverseFetch(fetch resolve.Fetch) resolve.Fetch { return fetch } -func (d *DataSourceFetch) traverseSingleFetch(fetch *resolve.SingleFetch) resolve.Fetch { +func (d *CreateConcreteSingleFetchTypes) traverseSingleFetch(fetch *resolve.SingleFetch) resolve.Fetch { switch { case fetch.RequiresEntityBatchFetch: return d.createEntityBatchFetch(fetch) @@ -67,13 +67,13 @@ func (d *DataSourceFetch) traverseSingleFetch(fetch *resolve.SingleFetch) resolv } } -func (d *DataSourceFetch) createParallelListItemFetch(fetch *resolve.SingleFetch) resolve.Fetch { +func (d *CreateConcreteSingleFetchTypes) createParallelListItemFetch(fetch *resolve.SingleFetch) resolve.Fetch { return &resolve.ParallelListItemFetch{ Fetch: fetch, } } -func (d *DataSourceFetch) createEntityBatchFetch(fetch *resolve.SingleFetch) resolve.Fetch { +func (d *CreateConcreteSingleFetchTypes) createEntityBatchFetch(fetch *resolve.SingleFetch) resolve.Fetch { representationsVariableIndex := -1 for i, segment := range fetch.InputTemplate.Segments { if segment.SegmentType == resolve.VariableSegmentType && @@ -117,7 +117,7 @@ func (d *DataSourceFetch) createEntityBatchFetch(fetch *resolve.SingleFetch) res } } -func (d *DataSourceFetch) createEntityFetch(fetch *resolve.SingleFetch) resolve.Fetch { +func (d *CreateConcreteSingleFetchTypes) createEntityFetch(fetch *resolve.SingleFetch) resolve.Fetch { representationsVariableIndex := -1 for i, segment := range fetch.InputTemplate.Segments { if segment.SegmentType == resolve.VariableSegmentType && diff --git a/v2/pkg/engine/postprocess/create_multi_fetch_types.go b/v2/pkg/engine/postprocess/create_multi_fetch_types.go new file mode 100644 index 000000000..7fbc3ed0c --- /dev/null +++ b/v2/pkg/engine/postprocess/create_multi_fetch_types.go @@ -0,0 +1,50 @@ +package postprocess + +import ( + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" +) + +// CreateMultiFetchTypes is a postprocessor that transforms multi fetches into more concrete fetch types +type CreateMultiFetchTypes struct{} + +func (d *CreateMultiFetchTypes) Process(pre plan.Plan) plan.Plan { + switch t := pre.(type) { + case *plan.SynchronousResponsePlan: + d.traverseNode(t.Response.Data) + case *plan.SubscriptionResponsePlan: + d.traverseNode(t.Response.Response.Data) + } + return pre +} + +func (d *CreateMultiFetchTypes) traverseNode(node resolve.Node) { + switch n := node.(type) { + case *resolve.Object: + n.Fetch = d.traverseFetch(n.Fetch) + for i := range n.Fields { + d.traverseNode(n.Fields[i].Value) + } + case *resolve.Array: + d.traverseNode(n.Item) + } +} + +func (d *CreateMultiFetchTypes) traverseFetch(fetch resolve.Fetch) resolve.Fetch { + if fetch == nil { + return nil + } + switch f := fetch.(type) { + case *resolve.SingleFetch: + return f + case *resolve.MultiFetch: + return d.processMultiFetch(f) + } + + return fetch +} + +func (d *CreateMultiFetchTypes) processMultiFetch(fetch *resolve.MultiFetch) resolve.Fetch { + + return fetch +} diff --git a/v2/pkg/engine/postprocess/postprocess.go b/v2/pkg/engine/postprocess/postprocess.go index 923fdfd9a..62a4a86fc 100644 --- a/v2/pkg/engine/postprocess/postprocess.go +++ b/v2/pkg/engine/postprocess/postprocess.go @@ -15,8 +15,9 @@ type Processor struct { func DefaultProcessor() *Processor { return &Processor{ []PostProcessor{ - &ProcessDataSource{}, - &DataSourceFetch{}, + &ResolveInputTemplates{}, + &CreateMultiFetchTypes{}, + &CreateConcreteSingleFetchTypes{}, }, } } diff --git a/v2/pkg/engine/postprocess/datasourceinput.go b/v2/pkg/engine/postprocess/resolve_input_templates.go similarity index 65% rename from v2/pkg/engine/postprocess/datasourceinput.go rename to v2/pkg/engine/postprocess/resolve_input_templates.go index e5eabf297..7160ee955 100644 --- a/v2/pkg/engine/postprocess/datasourceinput.go +++ b/v2/pkg/engine/postprocess/resolve_input_templates.go @@ -1,7 +1,6 @@ package postprocess import ( - "slices" "strconv" "strings" @@ -9,10 +8,10 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" ) -// ProcessDataSource is a postprocessor that processes input template and sort fetches of serial fetch -type ProcessDataSource struct{} +// ResolveInputTemplates is a postprocessor that resolves input template +type ResolveInputTemplates struct{} -func (d *ProcessDataSource) Process(pre plan.Plan) plan.Plan { +func (d *ResolveInputTemplates) Process(pre plan.Plan) plan.Plan { switch t := pre.(type) { case *plan.SynchronousResponsePlan: d.traverseNode(t.Response.Data) @@ -23,7 +22,7 @@ func (d *ProcessDataSource) Process(pre plan.Plan) plan.Plan { return pre } -func (d *ProcessDataSource) traverseNode(node resolve.Node) { +func (d *ResolveInputTemplates) traverseNode(node resolve.Node) { switch n := node.(type) { case *resolve.Object: d.traverseFetch(n.Fetch) @@ -35,40 +34,31 @@ func (d *ProcessDataSource) traverseNode(node resolve.Node) { } } -func (d *ProcessDataSource) traverseFetch(fetch resolve.Fetch) { +func (d *ResolveInputTemplates) traverseFetch(fetch resolve.Fetch) { if fetch == nil { return } switch f := fetch.(type) { case *resolve.SingleFetch: d.traverseSingleFetch(f) - case *resolve.ParallelFetch: - for i := range f.Fetches { - d.traverseFetch(f.Fetches[i]) - } - case *resolve.SerialFetch: - slices.SortFunc(f.Fetches, func(a, b resolve.Fetch) int { - // serial fetch always has a single fetch as child - // sort descending by serial id - return b.(*resolve.SingleFetch).FetchID - a.(*resolve.SingleFetch).FetchID - }) + case *resolve.MultiFetch: for i := range f.Fetches { d.traverseFetch(f.Fetches[i]) } default: // at this point, we should not have any other types of fetches - // as from planner we could get only SingleFetch, ParallelFetch and SerialFetch - // other types of fetches are created only during postprocessing via DataSourceFetch postprocessor + // as from planner we could get only SingleFetch and MultiFetch + // other types of fetches are created only during postprocessing via CreateConcreteSingleFetchTypes postprocessor } } -func (d *ProcessDataSource) traverseTrigger(trigger *resolve.GraphQLSubscriptionTrigger) { +func (d *ResolveInputTemplates) traverseTrigger(trigger *resolve.GraphQLSubscriptionTrigger) { d.resolveInputTemplate(trigger.Variables, string(trigger.Input), &trigger.InputTemplate) trigger.Input = nil trigger.Variables = nil } -func (d *ProcessDataSource) traverseSingleFetch(fetch *resolve.SingleFetch) { +func (d *ResolveInputTemplates) traverseSingleFetch(fetch *resolve.SingleFetch) { d.resolveInputTemplate(fetch.Variables, fetch.Input, &fetch.InputTemplate) fetch.Input = "" fetch.Variables = nil @@ -76,7 +66,7 @@ func (d *ProcessDataSource) traverseSingleFetch(fetch *resolve.SingleFetch) { fetch.SetTemplateOutputToNullOnVariableNull = false } -func (d *ProcessDataSource) resolveInputTemplate(variables resolve.Variables, input string, template *resolve.InputTemplate) { +func (d *ResolveInputTemplates) resolveInputTemplate(variables resolve.Variables, input string, template *resolve.InputTemplate) { if input == "" { return diff --git a/v2/pkg/engine/postprocess/datasourceinput_test.go b/v2/pkg/engine/postprocess/resolve_input_templates_test.go similarity index 99% rename from v2/pkg/engine/postprocess/datasourceinput_test.go rename to v2/pkg/engine/postprocess/resolve_input_templates_test.go index e2b9e062c..1c17fd2aa 100644 --- a/v2/pkg/engine/postprocess/datasourceinput_test.go +++ b/v2/pkg/engine/postprocess/resolve_input_templates_test.go @@ -342,7 +342,7 @@ func TestDataSourceInput_Process(t *testing.T) { }, } - processor := &ProcessDataSource{} + processor := &ResolveInputTemplates{} actual := processor.Process(pre) if !assert.Equal(t, expected, actual) { @@ -415,7 +415,7 @@ func TestDataSourceInput_ProcessSerialFetch(t *testing.T) { }, } - processor := &ProcessDataSource{} + processor := &ResolveInputTemplates{} actual := processor.Process(pre) if !assert.Equal(t, expected, actual) { From 275aba7c840e382e81ceb3d4aea553637e00e094 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Tue, 2 Jan 2024 16:10:13 +0200 Subject: [PATCH 05/27] implement multi fetch posprocessing --- v2/pkg/engine/plan/visitor.go | 10 +- .../postprocess/create_multi_fetch_types.go | 73 +++++++++- .../create_multi_fetch_types_test.go | 134 ++++++++++++++++++ .../resolve_input_templates_test.go | 73 ---------- v2/pkg/engine/resolve/fetch.go | 2 +- 5 files changed, 211 insertions(+), 81 deletions(-) create mode 100644 v2/pkg/engine/postprocess/create_multi_fetch_types_test.go diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index ed4b6418e..c46f97b73 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -1080,20 +1080,18 @@ func (v *Visitor) configureObjectFetch(config objectFetchConfiguration) { } fetchConfig := config.planner.ConfigureFetch() fetch := v.configureFetch(config, fetchConfig) + v.resolveInputTemplates(config, &fetch.Input, &fetch.Variables) - switch f := fetch.(type) { - case *resolve.SingleFetch: - v.resolveInputTemplates(config, &f.Input, &f.Variables) - } if config.object.Fetch == nil { config.object.Fetch = fetch return } + switch existing := config.object.Fetch.(type) { case *resolve.SingleFetch: copyOfExisting := *existing multi := &resolve.MultiFetch{ - Fetches: []resolve.Fetch{©OfExisting, fetch}, + Fetches: []*resolve.SingleFetch{©OfExisting, fetch}, } config.object.Fetch = multi case *resolve.MultiFetch: @@ -1101,7 +1099,7 @@ func (v *Visitor) configureObjectFetch(config objectFetchConfiguration) { } } -func (v *Visitor) configureFetch(internal objectFetchConfiguration, external resolve.FetchConfiguration) resolve.Fetch { +func (v *Visitor) configureFetch(internal objectFetchConfiguration, external resolve.FetchConfiguration) *resolve.SingleFetch { dataSourceType := reflect.TypeOf(external.DataSource).String() dataSourceType = strings.TrimPrefix(dataSourceType, "*") diff --git a/v2/pkg/engine/postprocess/create_multi_fetch_types.go b/v2/pkg/engine/postprocess/create_multi_fetch_types.go index 7fbc3ed0c..4ad873aa2 100644 --- a/v2/pkg/engine/postprocess/create_multi_fetch_types.go +++ b/v2/pkg/engine/postprocess/create_multi_fetch_types.go @@ -1,6 +1,8 @@ package postprocess import ( + "slices" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" ) @@ -45,6 +47,75 @@ func (d *CreateMultiFetchTypes) traverseFetch(fetch resolve.Fetch) resolve.Fetch } func (d *CreateMultiFetchTypes) processMultiFetch(fetch *resolve.MultiFetch) resolve.Fetch { + currentFetches := fetch.Fetches + dependsOn := make([]int, 0, len(fetch.Fetches)) - return fetch + for _, f := range fetch.Fetches { + dependsOn = append(dependsOn, f.DependsOnFetchIDs...) + } + + seenParentFetches := make(map[int]struct{}, len(fetch.Fetches)) + for _, parentID := range dependsOn { + if slices.ContainsFunc(currentFetches, func(f *resolve.SingleFetch) bool { + return parentID == f.FetchID + }) { + continue + } + + seenParentFetches[parentID] = struct{}{} + } + + layers := make([][]resolve.Fetch, 0, len(fetch.Fetches)) + + for len(currentFetches) > 0 { + currentLayer := make([]resolve.Fetch, 0, 2) + currentLayerFetchIds := make([]int, 0, 2) + + for _, fetch := range currentFetches { + shouldAdd := true + for _, parentID := range fetch.DependsOnFetchIDs { + if _, ok := seenParentFetches[parentID]; !ok { + shouldAdd = false + } + } + + if shouldAdd { + currentLayerFetchIds = append(currentLayerFetchIds, fetch.FetchID) + currentLayer = append(currentLayer, fetch) + } + } + + layers = append(layers, currentLayer) + + for _, fetchID := range currentLayerFetchIds { + seenParentFetches[fetchID] = struct{}{} + } + + currentFetches = slices.DeleteFunc(currentFetches, func(f *resolve.SingleFetch) bool { + return slices.Contains(currentLayerFetchIds, f.FetchID) + }) + + } + + if len(layers) == 1 { + return &resolve.ParallelFetch{ + Fetches: layers[0], + } + } + + fetches := make([]resolve.Fetch, 0, len(layers)) + for _, layer := range layers { + if len(layer) == 1 { + fetches = append(fetches, layer[0]) + continue + } + + fetches = append(fetches, &resolve.ParallelFetch{ + Fetches: layer, + }) + } + + return &resolve.SerialFetch{ + Fetches: fetches, + } } diff --git a/v2/pkg/engine/postprocess/create_multi_fetch_types_test.go b/v2/pkg/engine/postprocess/create_multi_fetch_types_test.go new file mode 100644 index 000000000..f154c22a2 --- /dev/null +++ b/v2/pkg/engine/postprocess/create_multi_fetch_types_test.go @@ -0,0 +1,134 @@ +package postprocess + +import ( + "encoding/json" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" +) + +func TestCreateMultiFetchTypes_Process(t *testing.T) { + type TestCase struct { + name string + pre plan.Plan + expected plan.Plan + } + + cases := []TestCase{ + { + name: "simple serial fetch", + pre: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.MultiFetch{ + Fetches: []*resolve.SingleFetch{ + {FetchConfiguration: resolve.FetchConfiguration{Input: `c`}, FetchID: 5, DependsOnFetchIDs: []int{2}}, + {FetchConfiguration: resolve.FetchConfiguration{Input: `a`}, FetchID: 1, DependsOnFetchIDs: []int{0}}, + {FetchConfiguration: resolve.FetchConfiguration{Input: `b`}, FetchID: 2, DependsOnFetchIDs: []int{1}}, + }, + }, + }, + }, + }, + expected: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SerialFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{ + FetchID: 1, + DependsOnFetchIDs: []int{0}, + FetchConfiguration: resolve.FetchConfiguration{Input: `a`}, + }, + &resolve.SingleFetch{ + FetchID: 2, + DependsOnFetchIDs: []int{1}, + FetchConfiguration: resolve.FetchConfiguration{Input: `b`}, + }, + &resolve.SingleFetch{ + FetchID: 5, + DependsOnFetchIDs: []int{2}, + FetchConfiguration: resolve.FetchConfiguration{Input: `c`}, + }, + }, + }, + }, + }, + }, + }, + { + /* + parent ID_0 DEPTH: 0 + / | \ \ + ID_1 ID_2 \ ID6 DEPTH: 1 + \ / \ / + ID_4 ID_3 DEPTH: 2 + \ + ID_5 DEPTH: 3 + */ + name: "complex dependency tree with a single parent fetch", + pre: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.MultiFetch{ + Fetches: []*resolve.SingleFetch{ + {FetchID: 1, DependsOnFetchIDs: []int{0}}, + {FetchID: 2, DependsOnFetchIDs: []int{0}}, + {FetchID: 3, DependsOnFetchIDs: []int{0, 6}}, + {FetchID: 4, DependsOnFetchIDs: []int{1, 2}}, + {FetchID: 5, DependsOnFetchIDs: []int{3}}, + {FetchID: 6, DependsOnFetchIDs: []int{0}}, + }, + }, + }, + }, + }, + expected: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SerialFetch{ + Fetches: []resolve.Fetch{ + &resolve.ParallelFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{FetchID: 1, DependsOnFetchIDs: []int{0}}, + &resolve.SingleFetch{FetchID: 2, DependsOnFetchIDs: []int{0}}, + &resolve.SingleFetch{FetchID: 6, DependsOnFetchIDs: []int{0}}, + }, + }, + &resolve.ParallelFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{FetchID: 3, DependsOnFetchIDs: []int{0, 6}}, + &resolve.SingleFetch{FetchID: 4, DependsOnFetchIDs: []int{1, 2}}, + }, + }, + &resolve.SingleFetch{FetchID: 5, DependsOnFetchIDs: []int{3}}, + }, + }, + }, + }, + }, + }, + } + + processor := &CreateMultiFetchTypes{} + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + actual := processor.Process(c.pre) + + if !assert.Equal(t, c.expected, actual) { + actualBytes, _ := json.MarshalIndent(actual, "", " ") + expectedBytes, _ := json.MarshalIndent(c.expected, "", " ") + + if string(expectedBytes) != string(actualBytes) { + assert.Equal(t, string(expectedBytes), string(actualBytes)) + t.Error(cmp.Diff(string(expectedBytes), string(actualBytes))) + } + } + }) + } +} diff --git a/v2/pkg/engine/postprocess/resolve_input_templates_test.go b/v2/pkg/engine/postprocess/resolve_input_templates_test.go index 1c17fd2aa..8dc362771 100644 --- a/v2/pkg/engine/postprocess/resolve_input_templates_test.go +++ b/v2/pkg/engine/postprocess/resolve_input_templates_test.go @@ -355,76 +355,3 @@ func TestDataSourceInput_Process(t *testing.T) { } } } - -func TestDataSourceInput_ProcessSerialFetch(t *testing.T) { - pre := &plan.SynchronousResponsePlan{ - Response: &resolve.GraphQLResponse{ - Data: &resolve.Object{ - Fetch: &resolve.SerialFetch{ - Fetches: []resolve.Fetch{ - &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `a`}, FetchID: 0}, - &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `b`}, FetchID: 2}, - &resolve.SingleFetch{FetchConfiguration: resolve.FetchConfiguration{Input: `c`}, FetchID: 5}, - }, - }, - }, - }, - } - - expected := &plan.SynchronousResponsePlan{ - Response: &resolve.GraphQLResponse{ - Data: &resolve.Object{ - Fetch: &resolve.SerialFetch{ - Fetches: []resolve.Fetch{ - &resolve.SingleFetch{ - FetchID: 5, - InputTemplate: resolve.InputTemplate{ - Segments: []resolve.TemplateSegment{ - { - Data: []byte(`c`), - SegmentType: resolve.StaticSegmentType, - }, - }, - }, - }, - &resolve.SingleFetch{ - FetchID: 2, - InputTemplate: resolve.InputTemplate{ - Segments: []resolve.TemplateSegment{ - { - Data: []byte(`b`), - SegmentType: resolve.StaticSegmentType, - }, - }, - }, - }, - &resolve.SingleFetch{ - FetchID: 0, - InputTemplate: resolve.InputTemplate{ - Segments: []resolve.TemplateSegment{ - { - Data: []byte(`a`), - SegmentType: resolve.StaticSegmentType, - }, - }, - }, - }, - }, - }, - }, - }, - } - - processor := &ResolveInputTemplates{} - actual := processor.Process(pre) - - if !assert.Equal(t, expected, actual) { - actualBytes, _ := json.MarshalIndent(actual, "", " ") - expectedBytes, _ := json.MarshalIndent(expected, "", " ") - - if string(expectedBytes) != string(actualBytes) { - assert.Equal(t, string(expectedBytes), string(actualBytes)) - t.Error(cmp.Diff(string(expectedBytes), string(actualBytes))) - } - } -} diff --git a/v2/pkg/engine/resolve/fetch.go b/v2/pkg/engine/resolve/fetch.go index b315acae1..6f145a8c8 100644 --- a/v2/pkg/engine/resolve/fetch.go +++ b/v2/pkg/engine/resolve/fetch.go @@ -21,7 +21,7 @@ type Fetch interface { } type MultiFetch struct { - Fetches []Fetch + Fetches []*SingleFetch } func (_ *MultiFetch) FetchKind() FetchKind { From 25275b1471e2a40c09ecda35650b6ad1696f202d Mon Sep 17 00:00:00 2001 From: spetrunin Date: Tue, 2 Jan 2024 18:29:03 +0200 Subject: [PATCH 06/27] add more comments and test cases --- .../postprocess/create_multi_fetch_types.go | 10 ++ .../create_multi_fetch_types_test.go | 132 +++++++++++++++--- 2 files changed, 124 insertions(+), 18 deletions(-) diff --git a/v2/pkg/engine/postprocess/create_multi_fetch_types.go b/v2/pkg/engine/postprocess/create_multi_fetch_types.go index 4ad873aa2..1659a51ed 100644 --- a/v2/pkg/engine/postprocess/create_multi_fetch_types.go +++ b/v2/pkg/engine/postprocess/create_multi_fetch_types.go @@ -54,6 +54,8 @@ func (d *CreateMultiFetchTypes) processMultiFetch(fetch *resolve.MultiFetch) res dependsOn = append(dependsOn, f.DependsOnFetchIDs...) } + // at the beginning we collect all dependencies of the current fetches, which not in the list of current fetches + // that will be parent fetches from lower depth seenParentFetches := make(map[int]struct{}, len(fetch.Fetches)) for _, parentID := range dependsOn { if slices.ContainsFunc(currentFetches, func(f *resolve.SingleFetch) bool { @@ -67,8 +69,15 @@ func (d *CreateMultiFetchTypes) processMultiFetch(fetch *resolve.MultiFetch) res layers := make([][]resolve.Fetch, 0, len(fetch.Fetches)) + // Here we create execution layers, layers will be fetched serially, + // but each layer items could be fetched in parallel. + // We iterate over items and look for fetches which depends on already seen fetches, + // such fetches added to the current layer, and removed from current fetches, + // we also mark them as seen after current layer is created + for len(currentFetches) > 0 { currentLayer := make([]resolve.Fetch, 0, 2) + // as currentLayer is a slice of interfaces resolve.Fetch, we store fetch ids separately currentLayerFetchIds := make([]int, 0, 2) for _, fetch := range currentFetches { @@ -76,6 +85,7 @@ func (d *CreateMultiFetchTypes) processMultiFetch(fetch *resolve.MultiFetch) res for _, parentID := range fetch.DependsOnFetchIDs { if _, ok := seenParentFetches[parentID]; !ok { shouldAdd = false + break } } diff --git a/v2/pkg/engine/postprocess/create_multi_fetch_types_test.go b/v2/pkg/engine/postprocess/create_multi_fetch_types_test.go index f154c22a2..d5d9dc407 100644 --- a/v2/pkg/engine/postprocess/create_multi_fetch_types_test.go +++ b/v2/pkg/engine/postprocess/create_multi_fetch_types_test.go @@ -19,6 +19,62 @@ func TestCreateMultiFetchTypes_Process(t *testing.T) { } cases := []TestCase{ + { + name: "parallel fetch without parent dependencies", + pre: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.MultiFetch{ + Fetches: []*resolve.SingleFetch{ + {FetchID: 1}, + {FetchID: 2}, + {FetchID: 3}, + }, + }, + }, + }, + }, + expected: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.ParallelFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{FetchID: 1}, + &resolve.SingleFetch{FetchID: 2}, + &resolve.SingleFetch{FetchID: 3}, + }, + }, + }, + }, + }, + }, + { + name: "serial fetch with the same level dependency", + pre: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.MultiFetch{ + Fetches: []*resolve.SingleFetch{ + {FetchID: 1, DependsOnFetchIDs: []int{3}}, + {FetchID: 3}, + }, + }, + }, + }, + }, + expected: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SerialFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{FetchID: 3}, + &resolve.SingleFetch{FetchID: 1, DependsOnFetchIDs: []int{3}}, + }, + }, + }, + }, + }, + }, { name: "simple serial fetch", pre: &plan.SynchronousResponsePlan{ @@ -26,9 +82,9 @@ func TestCreateMultiFetchTypes_Process(t *testing.T) { Data: &resolve.Object{ Fetch: &resolve.MultiFetch{ Fetches: []*resolve.SingleFetch{ - {FetchConfiguration: resolve.FetchConfiguration{Input: `c`}, FetchID: 5, DependsOnFetchIDs: []int{2}}, - {FetchConfiguration: resolve.FetchConfiguration{Input: `a`}, FetchID: 1, DependsOnFetchIDs: []int{0}}, - {FetchConfiguration: resolve.FetchConfiguration{Input: `b`}, FetchID: 2, DependsOnFetchIDs: []int{1}}, + {FetchID: 5, DependsOnFetchIDs: []int{2}}, + {FetchID: 1, DependsOnFetchIDs: []int{0}}, + {FetchID: 2, DependsOnFetchIDs: []int{1}}, }, }, }, @@ -39,21 +95,9 @@ func TestCreateMultiFetchTypes_Process(t *testing.T) { Data: &resolve.Object{ Fetch: &resolve.SerialFetch{ Fetches: []resolve.Fetch{ - &resolve.SingleFetch{ - FetchID: 1, - DependsOnFetchIDs: []int{0}, - FetchConfiguration: resolve.FetchConfiguration{Input: `a`}, - }, - &resolve.SingleFetch{ - FetchID: 2, - DependsOnFetchIDs: []int{1}, - FetchConfiguration: resolve.FetchConfiguration{Input: `b`}, - }, - &resolve.SingleFetch{ - FetchID: 5, - DependsOnFetchIDs: []int{2}, - FetchConfiguration: resolve.FetchConfiguration{Input: `c`}, - }, + &resolve.SingleFetch{FetchID: 1, DependsOnFetchIDs: []int{0}}, + &resolve.SingleFetch{FetchID: 2, DependsOnFetchIDs: []int{1}}, + &resolve.SingleFetch{FetchID: 5, DependsOnFetchIDs: []int{2}}, }, }, }, @@ -112,6 +156,58 @@ func TestCreateMultiFetchTypes_Process(t *testing.T) { }, }, }, + { + /* + ID_0 ID_4 DEPTH: 0 + / | \ \ + ID_1 ID_2 \ ID5 DEPTH: 1 + \ / \ / + ID_3 ID_6 DEPTH: 2 + \ + ID_7 DEPTH: 3 + */ + name: "complex dependency tree with 2 parent fetches", + pre: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.MultiFetch{ + Fetches: []*resolve.SingleFetch{ + {FetchID: 1, DependsOnFetchIDs: []int{0}}, + {FetchID: 2, DependsOnFetchIDs: []int{0}}, + {FetchID: 3, DependsOnFetchIDs: []int{1, 2}}, + {FetchID: 5, DependsOnFetchIDs: []int{4}}, + {FetchID: 6, DependsOnFetchIDs: []int{4, 5}}, + {FetchID: 7, DependsOnFetchIDs: []int{6}}, + }, + }, + }, + }, + }, + expected: &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SerialFetch{ + Fetches: []resolve.Fetch{ + &resolve.ParallelFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{FetchID: 1, DependsOnFetchIDs: []int{0}}, + &resolve.SingleFetch{FetchID: 2, DependsOnFetchIDs: []int{0}}, + &resolve.SingleFetch{FetchID: 5, DependsOnFetchIDs: []int{4}}, + }, + }, + &resolve.ParallelFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{FetchID: 3, DependsOnFetchIDs: []int{1, 2}}, + &resolve.SingleFetch{FetchID: 6, DependsOnFetchIDs: []int{4, 5}}, + }, + }, + &resolve.SingleFetch{FetchID: 7, DependsOnFetchIDs: []int{6}}, + }, + }, + }, + }, + }, + }, } processor := &CreateMultiFetchTypes{} From ae565762468a3340f01193c1ca03557ef8aa38cd Mon Sep 17 00:00:00 2001 From: spetrunin Date: Tue, 2 Jan 2024 21:43:42 +0200 Subject: [PATCH 07/27] fix tests --- .../graphql_datasource_federation_test.go | 141 +++++++++++------- .../graphql_datasource_test.go | 18 +-- .../introspection_datasource/planner_test.go | 2 +- .../datasourcetesting/datasourcetesting.go | 33 +++- 4 files changed, 114 insertions(+), 80 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 2b00c8b29..8194ac8a6 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 @@ -416,7 +416,8 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - FetchID: 1, + FetchID: 1, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Account {name shippingInfo {zip}}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -592,7 +593,8 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - FetchID: 1, + FetchID: 1, + DependsOnFetchIDs: []int{0}, Info: &resolve.FetchInfo{ DataSourceID: "account.service", }, @@ -719,10 +721,11 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetch: &resolve.SerialFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - FetchID: 1, + FetchID: 3, + DependsOnFetchIDs: []int{0}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Address {fullAddress}}}","variables":{"representations":[$$0$$]}}}`, + Input: `{"method":"POST","url":"http://address-enricher.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Address {country city}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, PostProcessing: SingleEntityPostProcessingConfiguration, RequiresEntityFetch: true, @@ -745,34 +748,6 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, OnTypeNames: [][]byte{[]byte("Address")}, }, - { - Name: []byte("line1"), - Value: &resolve.String{ - Path: []string{"line1"}, - }, - OnTypeNames: [][]byte{[]byte("Address")}, - }, - { - Name: []byte("line2"), - Value: &resolve.String{ - Path: []string{"line2"}, - }, - OnTypeNames: [][]byte{[]byte("Address")}, - }, - { - Name: []byte("line3"), - Value: &resolve.String{ - Path: []string{"line3"}, - }, - OnTypeNames: [][]byte{[]byte("Address")}, - }, - { - Name: []byte("zip"), - Value: &resolve.String{ - Path: []string{"zip"}, - }, - OnTypeNames: [][]byte{[]byte("Address")}, - }, }, }), }, @@ -782,6 +757,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, &resolve.SingleFetch{ FetchID: 2, + DependsOnFetchIDs: []int{0, 3}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://address.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Address {line3(test: "BOOM") zip}}}","variables":{"representations":[$$0$$]}}}`, @@ -829,10 +805,11 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, &resolve.SingleFetch{ - FetchID: 3, + FetchID: 1, + DependsOnFetchIDs: []int{0, 2}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://address-enricher.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Address {country city}}}","variables":{"representations":[$$0$$]}}}`, + Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Address {fullAddress}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, PostProcessing: SingleEntityPostProcessingConfiguration, RequiresEntityFetch: true, @@ -855,6 +832,34 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, OnTypeNames: [][]byte{[]byte("Address")}, }, + { + Name: []byte("line1"), + Value: &resolve.String{ + Path: []string{"line1"}, + }, + OnTypeNames: [][]byte{[]byte("Address")}, + }, + { + Name: []byte("line2"), + Value: &resolve.String{ + Path: []string{"line2"}, + }, + OnTypeNames: [][]byte{[]byte("Address")}, + }, + { + Name: []byte("line3"), + Value: &resolve.String{ + Path: []string{"line3"}, + }, + OnTypeNames: [][]byte{[]byte("Address")}, + }, + { + Name: []byte("zip"), + Value: &resolve.String{ + Path: []string{"zip"}, + }, + OnTypeNames: [][]byte{[]byte("Address")}, + }, }, }), }, @@ -904,6 +909,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, }, + WithMultiFetchPostProcessor(), )) }) }) @@ -1053,6 +1059,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, Fetch: &resolve.SingleFetch{ FetchID: 1, + DependsOnFetchIDs: []int{0}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://account.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Account {name shippingInfo {zip}}}}","variables":{"representations":[$$0$$]}}}`, @@ -1530,6 +1537,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, Fetch: &resolve.SingleFetch{ FetchID: 1, + DependsOnFetchIDs: []int{0}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input2, @@ -1663,6 +1671,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, Fetch: &resolve.SingleFetch{ FetchID: 1, + DependsOnFetchIDs: []int{0}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input2, @@ -1800,6 +1809,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, Fetch: &resolve.SingleFetch{ FetchID: 1, + DependsOnFetchIDs: []int{0}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: input2, @@ -1974,6 +1984,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetches: []resolve.Fetch{ &resolve.SingleFetch{ FetchID: 1, + DependsOnFetchIDs: []int{0}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://second.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {details {surname}}}}","variables":{"representations":[$$0$$]}}}`, @@ -2008,6 +2019,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, &resolve.SingleFetch{ FetchID: 2, + DependsOnFetchIDs: []int{0}, DataSourceIdentifier: []byte("graphql_datasource.Source"), FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {details {age}}}}","variables":{"representations":[$$0$$]}}}`, @@ -2067,6 +2079,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { "basic", expectedPlan(`{"method":"POST","url":"http://first.service","body":{"query":"{me {details {forename middlename} __typename id}}"}}`), planConfiguration, + WithMultiFetchPostProcessor(), )) }) }) @@ -2274,7 +2287,8 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - FetchID: 1, + FetchID: 1, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, @@ -2368,7 +2382,8 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - FetchID: 1, + FetchID: 1, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, @@ -2697,7 +2712,8 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetch: &resolve.ParallelFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - FetchID: 1, + FetchID: 1, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://second.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {name} ... on Admin {adminName}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -2745,7 +2761,8 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), }, &resolve.SingleFetch{ - FetchID: 2, + FetchID: 2, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Moderator {subject}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -2788,6 +2805,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, planConfiguration, + WithMultiFetchPostProcessor(), )) t.Run("interface query on array - with interface selection expanded to inline fragments", RunTest( @@ -2831,7 +2849,8 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fetch: &resolve.ParallelFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - FetchID: 1, + FetchID: 1, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Moderator {title}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -2865,7 +2884,8 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), }, &resolve.SingleFetch{ - FetchID: 2, + FetchID: 2, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://second.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {title}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, @@ -2908,11 +2928,14 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, planConfiguration, + WithMultiFetchPostProcessor(), )) }) }) t.Run("different entity keys jumps", func(t *testing.T) { + t.Skip("problem with node suggestions") + definition := ` type User { id: ID! @@ -2927,7 +2950,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { } type Query { - user: User + user: User! } ` @@ -3101,10 +3124,10 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSources: ShuffleDS(dataSources), DisableResolveFieldPositions: true, Debug: plan.DebugConfiguration{ - PrintOperationTransformations: true, - PrintQueryPlans: true, - PrintPlanningPaths: true, - PrintNodeSuggestions: true, + // PrintOperationTransformations: true, + PrintQueryPlans: true, + PrintPlanningPaths: true, + PrintNodeSuggestions: true, // DatasourceVisitor: true, }, @@ -3144,7 +3167,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { Fields: []*resolve.Field{ { Name: []byte("id"), - Value: &resolve.String{ + Value: &resolve.Scalar{ Path: []string{"id"}, }, }, @@ -3175,12 +3198,13 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, }, - Fetch: &resolve.ParallelFetch{ + Fetch: &resolve.SerialFetch{ Fetches: []resolve.Fetch{ - &resolve.SerialFetch{ + &resolve.ParallelFetch{ Fetches: []resolve.Fetch{ &resolve.SingleFetch{ - FetchID: 2, + FetchID: 1, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, @@ -3215,11 +3239,12 @@ func TestGraphQLDataSourceFederation(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), }, &resolve.SingleFetch{ - FetchID: 3, + FetchID: 3, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, - Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {title}}}","variables":{"representations":[$$0$$]}}}`, + Input: `{"method":"POST","url":"http://fourth.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {address {country}}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, SetTemplateOutputToNullOnVariableNull: true, Variables: []resolve.Variable{ @@ -3235,9 +3260,9 @@ func TestGraphQLDataSourceFederation(t *testing.T) { OnTypeNames: [][]byte{[]byte("User")}, }, { - Name: []byte("uuid"), + Name: []byte("id"), Value: &resolve.String{ - Path: []string{"uuid"}, + Path: []string{"id"}, }, OnTypeNames: [][]byte{[]byte("User")}, }, @@ -3252,11 +3277,12 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, &resolve.SingleFetch{ - FetchID: 3, + FetchID: 2, + DependsOnFetchIDs: []int{0, 1}, FetchConfiguration: resolve.FetchConfiguration{ RequiresEntityBatchFetch: false, RequiresEntityFetch: true, - Input: `{"method":"POST","url":"http://fourth.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {address {country}}}}","variables":{"representations":[$$0$$]}}}`, + Input: `{"method":"POST","url":"http://third.service","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on User {title}}}","variables":{"representations":[$$0$$]}}}`, DataSource: &Source{}, SetTemplateOutputToNullOnVariableNull: true, Variables: []resolve.Variable{ @@ -3272,9 +3298,9 @@ func TestGraphQLDataSourceFederation(t *testing.T) { OnTypeNames: [][]byte{[]byte("User")}, }, { - Name: []byte("id"), + Name: []byte("uuid"), Value: &resolve.String{ - Path: []string{"id"}, + Path: []string{"uuid"}, }, OnTypeNames: [][]byte{[]byte("User")}, }, @@ -3295,6 +3321,7 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }, }, planConfiguration, + WithMultiFetchPostProcessor(), )) }) } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go index db74ba64e..b6ae012c7 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go @@ -8294,25 +8294,17 @@ func TestSubscription_GTWS_SubProtocol(t *testing.T) { }) } -type runTestOnTestDefinitionOptions func(planConfig *plan.Configuration, extraChecks *[]CheckFunc) +type runTestOnTestDefinitionOptions func(planConfig *plan.Configuration) func testWithFactory(factory *Factory) runTestOnTestDefinitionOptions { - return func(planConfig *plan.Configuration, extraChecks *[]CheckFunc) { + return func(planConfig *plan.Configuration) { for _, ds := range planConfig.DataSources { ds.Factory = factory } } } -// nolint:deadcode,unused -func testWithExtraChecks(extraChecks ...CheckFunc) runTestOnTestDefinitionOptions { - return func(planConfig *plan.Configuration, availableChecks *[]CheckFunc) { - *availableChecks = append(*availableChecks, extraChecks...) - } -} - func runTestOnTestDefinition(operation, operationName string, expectedPlan plan.Plan, options ...runTestOnTestDefinitionOptions) func(t *testing.T) { - extraChecks := make([]CheckFunc, 0) config := plan.Configuration{ DataSources: []plan.DataSourceConfiguration{ { @@ -8409,11 +8401,7 @@ func runTestOnTestDefinition(operation, operationName string, expectedPlan plan. DisableResolveFieldPositions: true, } - for _, opt := range options { - opt(&config, &extraChecks) - } - - return RunTest(testDefinition, operation, operationName, expectedPlan, config, extraChecks...) + return RunTest(testDefinition, operation, operationName, expectedPlan, config) } func TestSource_Load(t *testing.T) { diff --git a/v2/pkg/engine/datasource/introspection_datasource/planner_test.go b/v2/pkg/engine/datasource/introspection_datasource/planner_test.go index 1630614e3..1cf2b2284 100644 --- a/v2/pkg/engine/datasource/introspection_datasource/planner_test.go +++ b/v2/pkg/engine/datasource/introspection_datasource/planner_test.go @@ -116,7 +116,7 @@ func TestIntrospectionDataSourcePlanning(t *testing.T) { Fields: cfgFactory.BuildFieldConfigurations(), } - datasourcetesting.RunTest(schema, introspectionQuery, "", expectedPlan, planConfiguration)(t) + datasourcetesting.RunTest(schema, introspectionQuery, "", expectedPlan, planConfiguration, datasourcetesting.WithMultiFetchPostProcessor())(t) } } diff --git a/v2/pkg/engine/datasourcetesting/datasourcetesting.go b/v2/pkg/engine/datasourcetesting/datasourcetesting.go index f8d6793aa..fa5970cd7 100644 --- a/v2/pkg/engine/datasourcetesting/datasourcetesting.go +++ b/v2/pkg/engine/datasourcetesting/datasourcetesting.go @@ -11,21 +11,38 @@ import ( "github.com/stretchr/testify/assert" "github.com/wundergraph/graphql-go-tools/v2/internal/pkg/unsafeparser" - "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization" "github.com/wundergraph/graphql-go-tools/v2/pkg/astprinter" "github.com/wundergraph/graphql-go-tools/v2/pkg/asttransform" "github.com/wundergraph/graphql-go-tools/v2/pkg/astvalidation" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/postprocess" "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" ) -type CheckFunc func(t *testing.T, op ast.Document, actualPlan plan.Plan) +type testOptions struct { + postProcessors []postprocess.PostProcessor +} + +func WithPostProcessors(postProcessors ...postprocess.PostProcessor) func(*testOptions) { + return func(o *testOptions) { + o.postProcessors = postProcessors + } +} -func RunTest(definition, operation, operationName string, expectedPlan plan.Plan, config plan.Configuration, extraChecks ...CheckFunc) func(t *testing.T) { +func WithMultiFetchPostProcessor() func(*testOptions) { + return WithPostProcessors(&postprocess.CreateMultiFetchTypes{}) +} + +func RunTest(definition, operation, operationName string, expectedPlan plan.Plan, config plan.Configuration, options ...func(*testOptions)) func(t *testing.T) { return func(t *testing.T) { t.Helper() + opts := &testOptions{} + for _, o := range options { + o(opts) + } + def := unsafeparser.ParseGraphqlDocumentString(definition) op := unsafeparser.ParseGraphqlDocumentString(operation) err := asttransform.MergeDefinitionWithBaseSchema(&def) @@ -53,6 +70,12 @@ func RunTest(definition, operation, operationName string, expectedPlan plan.Plan t.Fatal(report.Error()) } + if opts.postProcessors != nil { + for _, pp := range opts.postProcessors { + actualPlan = pp.Process(actualPlan) + } + } + actualBytes, _ := json.MarshalIndent(actualPlan, "", " ") expectedBytes, _ := json.MarshalIndent(expectedPlan, "", " ") @@ -60,10 +83,6 @@ func RunTest(definition, operation, operationName string, expectedPlan plan.Plan assert.Equal(t, expectedPlan, actualPlan) t.Error(cmp.Diff(string(expectedBytes), string(actualBytes))) } - - for _, extraCheck := range extraChecks { - extraCheck(t, op, actualPlan) - } } } From 09153badc87a44e981579f73714ee95580de6e1c Mon Sep 17 00:00:00 2001 From: spetrunin Date: Wed, 3 Jan 2024 18:03:45 +0200 Subject: [PATCH 08/27] fix selection of a proper datasource for the given key --- .../graphql_datasource_federation_test.go | 10 --- v2/pkg/engine/plan/configuration_visitor.go | 85 ++++++++++++------- .../engine/plan/datasource_filter_visitor.go | 56 ++++++++---- v2/pkg/engine/plan/planner.go | 2 +- .../postprocess/create_multi_fetch_types.go | 5 +- 5 files changed, 99 insertions(+), 59 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 8194ac8a6..282448a83 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 @@ -2934,8 +2934,6 @@ func TestGraphQLDataSourceFederation(t *testing.T) { }) t.Run("different entity keys jumps", func(t *testing.T) { - t.Skip("problem with node suggestions") - definition := ` type User { id: ID! @@ -3123,14 +3121,6 @@ func TestGraphQLDataSourceFederation(t *testing.T) { planConfiguration := plan.Configuration{ DataSources: ShuffleDS(dataSources), DisableResolveFieldPositions: true, - Debug: plan.DebugConfiguration{ - // PrintOperationTransformations: true, - PrintQueryPlans: true, - PrintPlanningPaths: true, - PrintNodeSuggestions: true, - - // DatasourceVisitor: true, - }, } t.Run("run", RunTest( diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index f3b56af97..8bdb34b53 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -23,7 +23,9 @@ type configurationVisitor struct { dataSources []DataSourceConfiguration planners []*plannerConfiguration - nodeSuggestions NodeSuggestions // nodeSuggestions holds information about suggested data sources for each field + nodeSuggestions NodeSuggestions // nodeSuggestions holds information about suggested data sources for each field + nodeSuggestionHints []NodeSuggestionHint // NodeSuggestionHints holds information about suggested data sources for key fields + parentTypeNodes []ast.Node // parentTypeNodes is a stack of parent type nodes - used to determine if the parent is abstract arrayFields []arrayField // arrayFields is a stack of array fields - used to plan nested queries selectionSetRefs []int // selectionSetRefs is a stack of selection set refs - used to add a required fields @@ -31,10 +33,10 @@ type configurationVisitor struct { missingPathTracker map[string]missingPath // missingPathTracker is a map of paths which will be added on secondary runs addedPathTracker []pathConfiguration // addedPathTracker is a list of paths which were added - pendingRequiredFields map[int][]fieldsRequiredByPlanner // pendingRequiredFields is a map[selectionSetRef][]fieldsRequiredByPlanner - handledRequires map[int]struct{} // handledRequires is a map[FieldRef] of already processed fields which has @requires directive - visitedFields map[int]struct{} // visitedFields is a map[FieldRef] of already processed fields which we check for abstract type, e.g. union or interface - requiredFieldsMapping map[int][]int // requiredFieldsMapping is a map[FieldRef][]plannerIdx holds dependencies between fields and planners + pendingRequiredFields map[int][]fieldsRequiredByPlanner // pendingRequiredFields is a map[selectionSetRef][]fieldsRequiredByPlanner + handledRequires map[int]struct{} // handledRequires is a map[FieldRef] of already processed fields which has @requires directive + visitedFields map[int]struct{} // visitedFields is a map[FieldRef] of already processed fields which we check for abstract type, e.g. union or interface + fieldDependenciesForPlanners map[int][]int // fieldDependenciesForPlanners is a map[FieldRef][]plannerIdx holds dependencies between fields and planners secondaryRun bool // secondaryRun is a flag to indicate that we're running the planner 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 @@ -43,8 +45,9 @@ type configurationVisitor struct { // fieldsRequiredByPlanner is a mapping between planner id which requested required fields // and a list of fields which should be added type fieldsRequiredByPlanner struct { - plannerIds []int - fieldSelections string + fieldSelections string + requestedByPlannerIDs []int + providedByPlannerID int } type arrayField struct { @@ -259,7 +262,7 @@ func (c *configurationVisitor) EnterDocument(operation, definition *ast.Document c.addedPathTracker = make([]pathConfiguration, 0, 8) c.pendingRequiredFields = make(map[int][]fieldsRequiredByPlanner) - c.requiredFieldsMapping = make(map[int][]int) + c.fieldDependenciesForPlanners = make(map[int][]int) c.handledRequires = make(map[int]struct{}) c.visitedFields = make(map[int]struct{}) } @@ -367,8 +370,8 @@ func (c *configurationVisitor) EnterField(ref int) { c.handleMissingPath(typeName, fieldName, currentPath) } -func (c *configurationVisitor) addPlannerDependencies(fieldRef int, plannerIdx int) { - plannerIds, mappingExists := c.requiredFieldsMapping[fieldRef] +func (c *configurationVisitor) addPlannerDependencies(fieldRef int, currentPlannerIdx int) { + plannerIds, mappingExists := c.fieldDependenciesForPlanners[fieldRef] if !mappingExists { return } @@ -376,13 +379,17 @@ func (c *configurationVisitor) addPlannerDependencies(fieldRef int, plannerIdx i for _, notifyPlannerIdx := range plannerIds { notified := false for _, existingPlannerId := range c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs { - if existingPlannerId == plannerIdx { + if existingPlannerId == currentPlannerIdx { notified = true break } } if !notified { - c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs = append(c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs, plannerIdx) + if notifyPlannerIdx == currentPlannerIdx { + c.walker.StopWithInternalErr(fmt.Errorf("wrong fetch dependencies planner %d depends on itself", notifyPlannerIdx)) + } + + c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs = append(c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs, currentPlannerIdx) } } } @@ -394,7 +401,7 @@ func (c *configurationVisitor) handleRequirements(plannerIdx int, parentPath str parentDSHash, ok := c.addedPathDSHash(parentPath) if ok && dsHash != parentDSHash { // add required fields for type (@key) - c.handleFieldsRequiredByKey(plannerIdx, plannerConfig, parentPath, typeName) + c.handleFieldsRequiredByKey(plannerIdx, plannerConfig, typeName) } // add required fields for field and type (@requires) @@ -858,7 +865,7 @@ func (c *configurationVisitor) handleFieldRequiredByRequires(plannerIdx int, con requiredFieldsForTypeAndField := config.dataSourceConfiguration.RequiredFieldsByRequires(typeName, fieldName) for _, requiredFieldsConfiguration := range requiredFieldsForTypeAndField { - c.planAddingRequiredFields(plannerIdx, requiredFieldsConfiguration) + c.planAddingRequiredFields(plannerIdx, -1, requiredFieldsConfiguration) var added bool config.requiredFields, added = appendRequiredFieldsConfigurationIfNotPresent(config.requiredFields, requiredFieldsConfiguration) if added { @@ -867,10 +874,10 @@ func (c *configurationVisitor) handleFieldRequiredByRequires(plannerIdx int, con } } -func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config *plannerConfiguration, parentPath string, typeName string) { +func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config *plannerConfiguration, typeName string) { requiredFieldsForType := config.dataSourceConfiguration.RequiredFieldsByKey(typeName) if len(requiredFieldsForType) > 0 { - requiredFieldsConfiguration, planned := c.planKeyRequiredFields(plannerIdx, parentPath, typeName, requiredFieldsForType) + requiredFieldsConfiguration, planned := c.planKeyRequiredFields(plannerIdx, typeName, requiredFieldsForType) if planned { var added bool config.requiredFields, added = appendRequiredFieldsConfigurationIfNotPresent(config.requiredFields, requiredFieldsConfiguration) @@ -881,27 +888,29 @@ func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config } } -func (c *configurationVisitor) planKeyRequiredFields(plannerIdx int, parentPath string, typeName string, possibleRequiredFields []FederationFieldConfiguration) (config FederationFieldConfiguration, planned bool) { +func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, typeName string, possibleRequiredFields []FederationFieldConfiguration) (config FederationFieldConfiguration, planned bool) { if len(possibleRequiredFields) == 0 { return } for i := range c.planners { - if i == plannerIdx { + if i == currentPlannerIdx { continue // skip current planner } for _, possibleRequiredFieldConfig := range possibleRequiredFields { if c.planners[i].dataSourceConfiguration.HasKeyRequirement(typeName, possibleRequiredFieldConfig.SelectionSet) { - c.planAddingRequiredFields(plannerIdx, possibleRequiredFieldConfig) + c.planAddingRequiredFields(currentPlannerIdx, i, possibleRequiredFieldConfig) return possibleRequiredFieldConfig, true } } } + // TODO: probably this should be an internal error - in case we haven't found any planner for the required fields + return FederationFieldConfiguration{}, false } -func (c *configurationVisitor) planAddingRequiredFields(plannerIdx int, fieldConfiguration FederationFieldConfiguration) { +func (c *configurationVisitor) planAddingRequiredFields(currentPlannerIdx int, providedByPlannerIdx int, fieldConfiguration FederationFieldConfiguration) { currentSelectionSet := c.currentSelectionSet() requirements, hasRequirements := c.pendingRequiredFields[currentSelectionSet] @@ -909,8 +918,8 @@ func (c *configurationVisitor) planAddingRequiredFields(plannerIdx int, fieldCon if !hasRequirements { requirements = make([]fieldsRequiredByPlanner, 0, 2) c.pendingRequiredFields[currentSelectionSet] = append(requirements, fieldsRequiredByPlanner{ - plannerIds: []int{plannerIdx}, - fieldSelections: fieldConfiguration.SelectionSet, + requestedByPlannerIDs: []int{currentPlannerIdx}, + fieldSelections: fieldConfiguration.SelectionSet, }) return } @@ -921,16 +930,17 @@ func (c *configurationVisitor) planAddingRequiredFields(plannerIdx int, fieldCon requirementExists = true plannerIdExists := false - for _, plannerId := range requirements[i].plannerIds { - if plannerId == plannerIdx { + for _, plannerId := range requirements[i].requestedByPlannerIDs { + if plannerId == currentPlannerIdx { plannerIdExists = true break } } + // when we have already added the same requirements for the current selection set // but not for such planner id if !plannerIdExists { - requirements[i].plannerIds = append(requirements[i].plannerIds, plannerIdx) + requirements[i].requestedByPlannerIDs = append(requirements[i].requestedByPlannerIDs, currentPlannerIdx) } break } @@ -939,8 +949,9 @@ func (c *configurationVisitor) planAddingRequiredFields(plannerIdx int, fieldCon // when no such requirement was added for the current selection set if !requirementExists { requirements = append(requirements, fieldsRequiredByPlanner{ - plannerIds: []int{plannerIdx}, - fieldSelections: fieldConfiguration.SelectionSet, + requestedByPlannerIDs: []int{currentPlannerIdx}, + providedByPlannerID: providedByPlannerIdx, + fieldSelections: fieldConfiguration.SelectionSet, }) } @@ -954,15 +965,15 @@ func (c *configurationVisitor) processPendingRequiredFields(selectionSetRef int) } for _, requiredFieldsCfg := range configs { - c.addRequiredFields(selectionSetRef, requiredFieldsCfg.plannerIds, requiredFieldsCfg.fieldSelections) + c.addRequiredFields(selectionSetRef, requiredFieldsCfg) } delete(c.pendingRequiredFields, selectionSetRef) } -func (c *configurationVisitor) addRequiredFields(selectionSetRef int, plannerIds []int, requiredFields string) { +func (c *configurationVisitor) addRequiredFields(selectionSetRef int, requiredFieldsCfg fieldsRequiredByPlanner) { typeName := c.walker.EnclosingTypeDefinition.NameString(c.definition) - key, report := RequiredFieldsFragment(typeName, requiredFields, true) + key, report := RequiredFieldsFragment(typeName, requiredFieldsCfg.fieldSelections, true) if report.HasErrors() { c.walker.StopWithInternalErr(fmt.Errorf("failed to parse required fields for %s", typeName)) } @@ -986,10 +997,22 @@ func (c *configurationVisitor) addRequiredFields(selectionSetRef int, plannerIds c.skipFieldsRefs = append(c.skipFieldsRefs, skipFieldRefs...) for _, fieldRef := range requiredFieldRefs { - c.requiredFieldsMapping[fieldRef] = append(c.requiredFieldsMapping[fieldRef], plannerIds...) + c.fieldDependenciesForPlanners[fieldRef] = append(c.fieldDependenciesForPlanners[fieldRef], requiredFieldsCfg.requestedByPlannerIDs...) + if requiredFieldsCfg.providedByPlannerID != -1 { + c.addNodeSuggestionHint(fieldRef, requiredFieldsCfg.providedByPlannerID) + } } } +func (c *configurationVisitor) addNodeSuggestionHint(fieldRef int, plannerIdx int) { + dsHash := c.planners[plannerIdx].dataSourceConfiguration.Hash() + + c.nodeSuggestionHints = append(c.nodeSuggestionHints, NodeSuggestionHint{ + fieldRef: fieldRef, + dsHash: dsHash, + }) +} + func (c *configurationVisitor) rewriteSelectionSetOfFieldWithInterfaceType(fieldRef int, plannerIdx int) { if _, ok := c.visitedFields[fieldRef]; ok { return diff --git a/v2/pkg/engine/plan/datasource_filter_visitor.go b/v2/pkg/engine/plan/datasource_filter_visitor.go index 9c8235c40..1a243c78e 100644 --- a/v2/pkg/engine/plan/datasource_filter_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_visitor.go @@ -29,8 +29,8 @@ func NewDataSourceFilter(operation, definition *ast.Document, report *operationr } } -func (f *DataSourceFilter) FilterDataSources(dataSources []DataSourceConfiguration, existingNodes NodeSuggestions) (used []DataSourceConfiguration, suggestions NodeSuggestions) { - suggestions = f.findBestDataSourceSet(dataSources, existingNodes) +func (f *DataSourceFilter) FilterDataSources(dataSources []DataSourceConfiguration, existingNodes NodeSuggestions, hints ...NodeSuggestionHint) (used []DataSourceConfiguration, suggestions NodeSuggestions) { + suggestions = f.findBestDataSourceSet(dataSources, existingNodes, hints...) if f.report.HasErrors() { return } @@ -49,8 +49,8 @@ func (f *DataSourceFilter) FilterDataSources(dataSources []DataSourceConfigurati return used, suggestions } -func (f *DataSourceFilter) findBestDataSourceSet(dataSources []DataSourceConfiguration, existingNodes NodeSuggestions) NodeSuggestions { - f.nodes = f.collectNodes(dataSources, existingNodes) +func (f *DataSourceFilter) findBestDataSourceSet(dataSources []DataSourceConfiguration, existingNodes NodeSuggestions, hints ...NodeSuggestionHint) NodeSuggestions { + f.nodes = f.collectNodes(dataSources, existingNodes, hints...) if f.report.HasErrors() { return nil } @@ -79,7 +79,7 @@ func (f *DataSourceFilter) printNodes(msg string) { } } -func (f *DataSourceFilter) collectNodes(dataSources []DataSourceConfiguration, existingNodes NodeSuggestions) (nodes NodeSuggestions) { +func (f *DataSourceFilter) collectNodes(dataSources []DataSourceConfiguration, existingNodes NodeSuggestions, hints ...NodeSuggestionHint) (nodes NodeSuggestions) { secondaryRun := existingNodes != nil walker := astvisitor.NewWalker(32) @@ -88,8 +88,9 @@ func (f *DataSourceFilter) collectNodes(dataSources []DataSourceConfiguration, e definition: f.definition, walker: &walker, dataSources: dataSources, - nodes: existingNodes, secondaryRun: secondaryRun, + nodes: existingNodes, + hints: hints, } walker.RegisterEnterDocumentVisitor(visitor) walker.RegisterEnterFieldVisitor(visitor) @@ -141,6 +142,11 @@ func (n *NodeSuggestion) String() string { n.DataSourceHash, n.Path, n.TypeName, n.FieldName, n.IsRootNode, n.selected, n.selectionReasons) } +type NodeSuggestionHint struct { + fieldRef int + dsHash DSHash +} + type NodeSuggestions []NodeSuggestion func appendSuggestionWithPresenceCheck(nodes NodeSuggestions, node NodeSuggestion) NodeSuggestions { @@ -334,13 +340,14 @@ func (f *nodesResolvableVisitor) EnterField(ref int) { } type collectNodesVisitor struct { - operation *ast.Document - definition *ast.Document - walker *astvisitor.Walker - - dataSources []DataSourceConfiguration - nodes NodeSuggestions + operation *ast.Document + definition *ast.Document + walker *astvisitor.Walker secondaryRun bool + + dataSources []DataSourceConfiguration + nodes NodeSuggestions + hints []NodeSuggestionHint } func (f *collectNodesVisitor) EnterDocument(_, _ *ast.Document) { @@ -370,7 +377,19 @@ func (f *collectNodesVisitor) EnterField(ref int) { currentPath := parentPath + "." + fieldAliasOrName + var dsHashHint *DSHash + for _, hint := range f.hints { + if hint.fieldRef == ref { + dsHashHint = &hint.dsHash + break + } + } + for _, v := range f.dataSources { + if dsHashHint != nil && v.Hash() != *dsHashHint { + continue + } + hasRootNode := v.HasRootNode(typeName, fieldName) || (isTypeName && v.HasRootNodeWithTypename(typeName)) hasChildNode := v.HasChildNode(typeName, fieldName) || (isTypeName && v.HasChildNodeWithTypename(typeName)) @@ -386,6 +405,10 @@ func (f *collectNodesVisitor) EnterField(ref int) { parentPathWithoutFragment: parentPathWithoutFragment, } + if dsHashHint != nil { + node.selectWithReason(ReasonKeyRequirementProvidedByPlanner) + } + if f.secondaryRun { f.nodes = appendSuggestionWithPresenceCheck(f.nodes, node) } else { @@ -411,12 +434,13 @@ const ( ReasonStage1SameSourceLeafChild = "stage1: same source leaf child of uniq node" ReasonStage1SameSourceLeafSibling = "stage1: same source leaf sibling of uniq node" - ReasonStage2SameSourceNodeOfSelectedParent = "stage2: node on the same source as selected parent" - ReasonStage2SameSourceDuplicateNodeOfSelectedParent = "stage2: duplicate node on the same source as selected parent" - ReasonStage2SameSourceNodeOfSelectedChild = "stage2: node on the same source as selected child" - ReasonStage2SameSourceNodeOfSelectedSibling = "stage2: node on the same source as selected sibling" + ReasonStage2SameSourceNodeOfSelectedParent = "stage2: node on the same source as selected parent" + ReasonStage2SameSourceNodeOfSelectedChild = "stage2: node on the same source as selected child" + ReasonStage2SameSourceNodeOfSelectedSibling = "stage2: node on the same source as selected sibling" ReasonStage3SelectAvailableNode = "stage3: select first available node" + + ReasonKeyRequirementProvidedByPlanner = "provided by planner as required by @key" ) // selectUniqNodes - selects nodes (e.g. fields) which are unique to a single datasource diff --git a/v2/pkg/engine/plan/planner.go b/v2/pkg/engine/plan/planner.go index f261c2ff8..a2a15a326 100644 --- a/v2/pkg/engine/plan/planner.go +++ b/v2/pkg/engine/plan/planner.go @@ -197,7 +197,7 @@ func (p *Planner) findPlanningPaths(operation, definition *ast.Document, report if p.configurationVisitor.hasNewFields { // update suggestions for the new required fields p.configurationVisitor.dataSources, p.configurationVisitor.nodeSuggestions = - dsFilter.FilterDataSources(p.config.DataSources, p.configurationVisitor.nodeSuggestions) + dsFilter.FilterDataSources(p.config.DataSources, p.configurationVisitor.nodeSuggestions, p.configurationVisitor.nodeSuggestionHints...) if report.HasErrors() { return } diff --git a/v2/pkg/engine/postprocess/create_multi_fetch_types.go b/v2/pkg/engine/postprocess/create_multi_fetch_types.go index 1659a51ed..ccf1bd037 100644 --- a/v2/pkg/engine/postprocess/create_multi_fetch_types.go +++ b/v2/pkg/engine/postprocess/create_multi_fetch_types.go @@ -101,10 +101,13 @@ func (d *CreateMultiFetchTypes) processMultiFetch(fetch *resolve.MultiFetch) res seenParentFetches[fetchID] = struct{}{} } + if len(currentLayer) == 0 { + panic("not able to setup fetch execution order - wrong execution plan") + } + currentFetches = slices.DeleteFunc(currentFetches, func(f *resolve.SingleFetch) bool { return slices.Contains(currentLayerFetchIds, f.FetchID) }) - } if len(layers) == 1 { From 953217375db55e8854c877dc97e1017bada3c00d Mon Sep 17 00:00:00 2001 From: spetrunin Date: Thu, 14 Dec 2023 21:00:32 +0200 Subject: [PATCH 09/27] add draft tests --- ...ource_federation_entity_interfaces_test.go | 310 ++++++++++++++++++ v2/pkg/engine/plan/federation_metadata.go | 7 +- v2/pkg/federation/schema.go | 1 + 3 files changed, 315 insertions(+), 3 deletions(-) create mode 100644 v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go new file mode 100644 index 000000000..d1c0db59e --- /dev/null +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go @@ -0,0 +1,310 @@ +package graphql_datasource + +import ( + "testing" + + . "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasourcetesting" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" +) + +func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { + federationFactory := &Factory{} + + definition := ` + interface Account { + id: ID! + title: String! + locations: [Location!] + age: Int! + } + + type Location { + country: String! + } + + type Admin implements Account { + id: ID! + title: String! + locations: [Location!] + age: Int! + } + + type Moderator implements Account { + id: ID! + title: String! + locations: [Location!] + age: Int! + } + + type User implements Account { + id: ID! + title: String! + locations: [Location!] + age: Int! + } + + union Accounts = Admin | Moderator | User + + type Query { + allAccountsInterface: [Account] + allAccountsUnion: [Accounts] + user(id: ID!): User + admin(id: ID!): Admin + accountLocations: [Account!]! + }` + + firstSubgraphSDL := ` + interface Account @key(fields: "id") { + id: ID! + title: String! + } + + type Admin implements Account @key(fields: "id"){ + id: ID! + title: String! @external + } + + type Moderator implements Account @key(fields: "id"){ + id: ID! + title: String! + } + + type User implements Account @key(fields: "id"){ + id: ID! + title: String! + } + + union Accounts = Admin | Moderator | User + + type Query { + allAccountsInterface: [Account] + allAccountsUnion: [Accounts] + user(id: ID!): User + admin(id: ID!): Admin + }` + + firstDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Account", + FieldNames: []string{"id", "title"}, + }, + { + TypeName: "Admin", + FieldNames: []string{"id"}, + }, + { + TypeName: "Moderator", + FieldNames: []string{"id", "title"}, + }, + { + TypeName: "User", + FieldNames: []string{"id", "title"}, + }, + { + TypeName: "Query", + FieldNames: []string{"allAccountsInterface", "allAccountsUnion", "user", "admin"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://first.service", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: firstSubgraphSDL, + }, + UpstreamSchema: firstSubgraphSDL, + }), + Factory: federationFactory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "Account", + SelectionSet: "id", + }, + { + TypeName: "Admin", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + { + TypeName: "Moderator", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + { + TypeName: "User", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + }, + }, + } + + secondSubgraphSDL := ` + type Account @key(fields: "id") @interfaceObject { + id: ID! + locations: [Location!] + } + + type Location { + country: String! + } + + type Query { + accountLocations: [Account!]! + }` + + secondDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Account", + FieldNames: []string{"id", "locations"}, + }, + { + TypeName: "Query", + FieldNames: []string{"accountLocations"}, + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Location", + FieldNames: []string{"country"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://second.service", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: secondSubgraphSDL, + }, + UpstreamSchema: secondSubgraphSDL, + }), + Factory: federationFactory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "Account", + SelectionSet: "id", + }, + }, + }, + } + + thirdSubgraphSDL := ` + type Admin @key(fields: "id"){ + id: ID! + title: String! + }` + + thirdDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Admin", + FieldNames: []string{"id", "title"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://third.service", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: thirdSubgraphSDL, + }, + UpstreamSchema: thirdSubgraphSDL, + }), + Factory: federationFactory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "Admin", + SelectionSet: "id", + }, + }, + }, + } + + fourthSubgraphSDL := ` + type Account @key(fields: "id") @interfaceObject { + id: ID! + age: Int! + }` + + fourthDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Account", + FieldNames: []string{"id", "age"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://fourth.service", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: fourthSubgraphSDL, + }, + UpstreamSchema: fourthSubgraphSDL, + }), + Factory: federationFactory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "Account", + SelectionSet: "id", + }, + }, + }, + } + + dataSources := []plan.DataSourceConfiguration{ + firstDatasourceConfiguration, + secondDatasourceConfiguration, + thirdDatasourceConfiguration, + fourthDatasourceConfiguration, + } + + planConfiguration := plan.Configuration{ + DataSources: ShuffleDS(dataSources), + DisableResolveFieldPositions: true, + Debug: plan.DebugConfiguration{ + PrintQueryPlans: true, + }, + } + + t.Run("query 1 - ", func(t *testing.T) { + + t.Run("run", RunTest( + definition, + ` + query { + } + `, + "Accounts", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://first.service","body":{"query":"{account {__typename ... on User {__typename id} ... on Admin {__typename id}}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + DataSource: &Source{}, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + Fields: []*resolve.Field{}, + }, + }, + }, + planConfiguration, + )) + + }) + +} diff --git a/v2/pkg/engine/plan/federation_metadata.go b/v2/pkg/engine/plan/federation_metadata.go index 28833684b..d8beea1bc 100644 --- a/v2/pkg/engine/plan/federation_metadata.go +++ b/v2/pkg/engine/plan/federation_metadata.go @@ -7,9 +7,10 @@ type FederationMetaData struct { } type FederationFieldConfiguration struct { - TypeName string - FieldName string - SelectionSet string + TypeName string + FieldName string + SelectionSet string + InterfaceNames []string } type FederationFieldConfigurations []FederationFieldConfiguration diff --git a/v2/pkg/federation/schema.go b/v2/pkg/federation/schema.go index e6f540e8a..84f11eff6 100644 --- a/v2/pkg/federation/schema.go +++ b/v2/pkg/federation/schema.go @@ -187,4 +187,5 @@ directive @requires(fields: _FieldSet!) on FIELD_DEFINITION directive @provides(fields: _FieldSet!) on FIELD_DEFINITION directive @key(fields: _FieldSet!) on OBJECT | INTERFACE directive @extends on OBJECT | INTERFACE +directive @interfaceObject on OBJECT ` From 80f389863c77c2c1374cdf366e5cebd6f548324b Mon Sep 17 00:00:00 2001 From: spetrunin Date: Fri, 15 Dec 2023 20:39:22 +0200 Subject: [PATCH 10/27] move field selection normalization after merging inline fragments --- v2/pkg/astnormalization/astnormalization.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index e22f8af0c..38f219f3c 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -224,9 +224,8 @@ func (o *OperationNormalizer) setupOperationWalkers() { other := astvisitor.NewWalker(48) removeSelfAliasing(&other) inlineSelectionsFromInlineFragments(&other) - mergeFieldSelections(&other) o.operationWalkers = append(o.operationWalkers, walkerStage{ - name: "removeSelfAliasing, inlineSelectionsFromInlineFragments, mergeFieldSelections", + name: "removeSelfAliasing, inlineSelectionsFromInlineFragments", walker: &other, }) @@ -238,6 +237,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { }) cleanup := astvisitor.NewWalker(48) + mergeFieldSelections(&cleanup) deduplicateFields(&cleanup) if o.options.removeFragmentDefinitions { removeFragmentDefinitions(&cleanup) @@ -246,7 +246,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { deleteUnusedVariables(&cleanup) } o.operationWalkers = append(o.operationWalkers, walkerStage{ - name: "deduplicateFields, removeFragmentDefinitions, deleteUnusedVariables", + name: "mergeFieldSelections, deduplicateFields, removeFragmentDefinitions, deleteUnusedVariables", walker: &cleanup, }) From 7bf677d6446800a7e0e2f4bc8997d65753419a24 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Fri, 15 Dec 2023 20:40:23 +0200 Subject: [PATCH 11/27] add draft configurations for entity interfaces add rename types to datasource config --- .../graphql_datasource/graphql_datasource.go | 7 +- ...ource_federation_entity_interfaces_test.go | 112 ++++++++++++++++-- .../engine/plan/datasource_configuration.go | 1 + 3 files changed, 109 insertions(+), 11 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 5e9dd84f4..401a75671 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -924,7 +924,12 @@ func (p *Planner) addOnTypeInlineFragment() { selectionSet := p.upstreamOperation.AddSelectionSet() p.addTypenameToSelectionSet(p.nodes[len(p.nodes)-1].Ref) + onTypeName := p.visitor.Config.Types.RenameTypeNameOnMatchBytes([]byte(p.lastFieldEnclosingTypeName)) + + // rename type name in case it is required by entity interface + onTypeName = p.dataSourceConfig.RenameTypes.RenameTypeNameOnMatchBytes([]byte(p.lastFieldEnclosingTypeName)) + typeRef := p.upstreamOperation.AddNamedType(onTypeName) inlineFragment := p.upstreamOperation.AddInlineFragment(ast.InlineFragment{ HasSelections: true, @@ -1429,7 +1434,7 @@ func (p *Planner) printOperation() []byte { return nil } - p.printQueryPlan(p.upstreamOperation) + p.printQueryPlan(operation) buf.Reset() diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go index d1c0db59e..5357058dc 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go @@ -86,10 +86,6 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { firstDatasourceConfiguration := plan.DataSourceConfiguration{ RootNodes: []plan.TypeField{ - { - TypeName: "Account", - FieldNames: []string{"id", "title"}, - }, { TypeName: "Admin", FieldNames: []string{"id"}, @@ -107,6 +103,12 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { FieldNames: []string{"allAccountsInterface", "allAccountsUnion", "user", "admin"}, }, }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Account", + FieldNames: []string{"id", "title"}, + }, + }, Custom: ConfigJson(Configuration{ Fetch: FetchConfiguration{ URL: "http://first.service", @@ -163,6 +165,18 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { TypeName: "Account", FieldNames: []string{"id", "locations"}, }, + { + TypeName: "User", + FieldNames: []string{"id", "locations"}, + }, + { + TypeName: "Moderator", + FieldNames: []string{"id", "locations"}, + }, + { + TypeName: "Admin", + FieldNames: []string{"id", "locations"}, + }, { TypeName: "Query", FieldNames: []string{"accountLocations"}, @@ -191,6 +205,35 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { TypeName: "Account", SelectionSet: "id", }, + { + TypeName: "Admin", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + { + TypeName: "Moderator", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + { + TypeName: "User", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + }, + }, + RenameTypes: plan.TypeConfigurations{ + { + TypeName: "Admin", + RenameTo: "Account", + }, + { + TypeName: "Moderator", + RenameTo: "Account", + }, + { + TypeName: "User", + RenameTo: "Account", }, }, } @@ -241,6 +284,18 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { TypeName: "Account", FieldNames: []string{"id", "age"}, }, + { + TypeName: "User", + FieldNames: []string{"id", "age"}, + }, + { + TypeName: "Moderator", + FieldNames: []string{"id", "age"}, + }, + { + TypeName: "Admin", + FieldNames: []string{"id", "age"}, + }, }, Custom: ConfigJson(Configuration{ Fetch: FetchConfiguration{ @@ -259,6 +314,35 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { TypeName: "Account", SelectionSet: "id", }, + { + TypeName: "Admin", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + { + TypeName: "Moderator", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + { + TypeName: "User", + InterfaceNames: []string{"Account"}, + SelectionSet: "id", + }, + }, + }, + RenameTypes: plan.TypeConfigurations{ + { + TypeName: "Admin", + RenameTo: "Account", + }, + { + TypeName: "Moderator", + RenameTo: "Account", + }, + { + TypeName: "User", + RenameTo: "Account", }, }, } @@ -274,25 +358,33 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { DataSources: ShuffleDS(dataSources), DisableResolveFieldPositions: true, Debug: plan.DebugConfiguration{ - PrintQueryPlans: true, + PrintOperationTransformations: true, + PrintQueryPlans: true, + PrintPlanningPaths: true, + PrintNodeSuggestions: true, }, } - t.Run("query 1 - ", func(t *testing.T) { + t.Run("query 1 - Interface to interface object", func(t *testing.T) { t.Run("run", RunTest( definition, ` - query { + query _1_InterfaceToInterfaceObject { + allAccountsInterface { + id + locations { + country + } } - `, - "Accounts", + }`, + "_1_InterfaceToInterfaceObject", &plan.SynchronousResponsePlan{ Response: &resolve.GraphQLResponse{ Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://first.service","body":{"query":"{account {__typename ... on User {__typename id} ... on Admin {__typename id}}}"}}`, + Input: `{"method":"POST","url":"http://first.service","body":{"query":""}}`, PostProcessing: DefaultPostProcessingConfiguration, DataSource: &Source{}, }, diff --git a/v2/pkg/engine/plan/datasource_configuration.go b/v2/pkg/engine/plan/datasource_configuration.go index b6d09ea3e..654a902dd 100644 --- a/v2/pkg/engine/plan/datasource_configuration.go +++ b/v2/pkg/engine/plan/datasource_configuration.go @@ -38,6 +38,7 @@ type DataSourceConfiguration struct { Custom json.RawMessage FederationMetaData FederationMetaData + RenameTypes TypeConfigurations hash DSHash } From e2619519f15960a643794c57b853593767009c4d Mon Sep 17 00:00:00 2001 From: spetrunin Date: Tue, 19 Dec 2023 18:43:19 +0200 Subject: [PATCH 12/27] move entity interfaces engine config to separate file --- .../entity_interfaces_engine_config.go | 360 ++++++++++++++++++ ...ource_federation_entity_interfaces_test.go | 355 +---------------- 2 files changed, 362 insertions(+), 353 deletions(-) create mode 100644 v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go diff --git a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go new file mode 100644 index 000000000..17ace1679 --- /dev/null +++ b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go @@ -0,0 +1,360 @@ +package graphql_datasource + +import ( + "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" +) + +const EntityInterfacesDefinition = ` + interface Account { + id: ID! + title: String! + locations: [Location!] + age: Int! + } + + type Location { + country: String! + } + + type Admin implements Account { + id: ID! + title: String! + locations: [Location!] + age: Int! + } + + type Moderator implements Account { + id: ID! + title: String! + locations: [Location!] + age: Int! + } + + type User implements Account { + id: ID! + title: String! + locations: [Location!] + age: Int! + } + + union Accounts = Admin | Moderator | User + + type Query { + allAccountsInterface: [Account] + allAccountsUnion: [Accounts] + user(id: ID!): User + admin(id: ID!): Admin + accountLocations: [Account!]! + }` + +func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Configuration { + firstSubgraphSDL := ` + interface Account @key(fields: "id") { + id: ID! + title: String! + } + + type Admin implements Account @key(fields: "id"){ + id: ID! + title: String! @external + } + + type Moderator implements Account @key(fields: "id"){ + id: ID! + title: String! + } + + type User implements Account @key(fields: "id"){ + id: ID! + title: String! + } + + union Accounts = Admin | Moderator | User + + type Query { + allAccountsInterface: [Account] + allAccountsUnion: [Accounts] + user(id: ID!): User + admin(id: ID!): Admin + }` + + firstDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Admin", + FieldNames: []string{"id"}, + }, + { + TypeName: "Moderator", + FieldNames: []string{"id", "title"}, + }, + { + TypeName: "User", + FieldNames: []string{"id", "title"}, + }, + { + TypeName: "Query", + FieldNames: []string{"allAccountsInterface", "allAccountsUnion", "user", "admin"}, + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Account", + FieldNames: []string{"id", "title"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://localhost:4001/graphql", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: firstSubgraphSDL, + }, + UpstreamSchema: firstSubgraphSDL, + }), + Factory: factory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "Account", + SelectionSet: "id", + }, + { + TypeName: "Admin", + SelectionSet: "id", + }, + { + TypeName: "Moderator", + SelectionSet: "id", + }, + { + TypeName: "User", + SelectionSet: "id", + }, + }, + }, + } + + secondSubgraphSDL := ` + type Account @key(fields: "id") @interfaceObject { + id: ID! + locations: [Location!] + } + + type Location { + country: String! + } + + type Query { + accountLocations: [Account!]! + }` + + secondDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Account", + FieldNames: []string{"id", "locations"}, + }, + { + TypeName: "User", + FieldNames: []string{"id", "locations"}, + }, + { + TypeName: "Moderator", + FieldNames: []string{"id", "locations"}, + }, + { + TypeName: "Admin", + FieldNames: []string{"id", "locations"}, + }, + { + TypeName: "Query", + FieldNames: []string{"accountLocations"}, + }, + }, + ChildNodes: []plan.TypeField{ + { + TypeName: "Location", + FieldNames: []string{"country"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://localhost:4002/graphql", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: secondSubgraphSDL, + }, + UpstreamSchema: secondSubgraphSDL, + }), + Factory: factory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "Account", + SelectionSet: "id", + }, + { + TypeName: "Admin", + InterfaceObjectTypeName: "Account", + SelectionSet: "id", + }, + { + TypeName: "Moderator", + InterfaceObjectTypeName: "Account", + SelectionSet: "id", + }, + { + TypeName: "User", + InterfaceObjectTypeName: "Account", + SelectionSet: "id", + }, + }, + }, + RenameTypes: plan.TypeConfigurations{ + { + TypeName: "Admin", + RenameTo: "Account", + }, + { + TypeName: "Moderator", + RenameTo: "Account", + }, + { + TypeName: "User", + RenameTo: "Account", + }, + }, + } + + thirdSubgraphSDL := ` + type Admin @key(fields: "id"){ + id: ID! + title: String! + }` + + thirdDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Admin", + FieldNames: []string{"id", "title"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://localhost:4003/graphql", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: thirdSubgraphSDL, + }, + UpstreamSchema: thirdSubgraphSDL, + }), + Factory: factory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "Admin", + SelectionSet: "id", + }, + }, + }, + } + + fourthSubgraphSDL := ` + type Account @key(fields: "id") @interfaceObject { + id: ID! + age: Int! + }` + + fourthDatasourceConfiguration := plan.DataSourceConfiguration{ + RootNodes: []plan.TypeField{ + { + TypeName: "Account", + FieldNames: []string{"id", "age"}, + }, + { + TypeName: "User", + FieldNames: []string{"id", "age"}, + }, + { + TypeName: "Moderator", + FieldNames: []string{"id", "age"}, + }, + { + TypeName: "Admin", + FieldNames: []string{"id", "age"}, + }, + }, + Custom: ConfigJson(Configuration{ + Fetch: FetchConfiguration{ + URL: "http://localhost:4004/graphql", + }, + Federation: FederationConfiguration{ + Enabled: true, + ServiceSDL: fourthSubgraphSDL, + }, + UpstreamSchema: fourthSubgraphSDL, + }), + Factory: factory, + FederationMetaData: plan.FederationMetaData{ + Keys: plan.FederationFieldConfigurations{ + { + TypeName: "Account", + SelectionSet: "id", + }, + { + TypeName: "Admin", + InterfaceObjectTypeName: "Account", + SelectionSet: "id", + }, + { + TypeName: "Moderator", + InterfaceObjectTypeName: "Account", + SelectionSet: "id", + }, + { + TypeName: "User", + InterfaceObjectTypeName: "Account", + SelectionSet: "id", + }, + }, + }, + RenameTypes: plan.TypeConfigurations{ + { + TypeName: "Admin", + RenameTo: "Account", + }, + { + TypeName: "Moderator", + RenameTo: "Account", + }, + { + TypeName: "User", + RenameTo: "Account", + }, + }, + } + + dataSources := []plan.DataSourceConfiguration{ + firstDatasourceConfiguration, + secondDatasourceConfiguration, + thirdDatasourceConfiguration, + fourthDatasourceConfiguration, + } + + planConfiguration := plan.Configuration{ + DataSources: dataSources, + DisableResolveFieldPositions: true, + Debug: plan.DebugConfiguration{ + PrintOperationTransformations: true, + PrintQueryPlans: true, + PrintPlanningPaths: true, + PrintNodeSuggestions: true, + }, + } + + return &planConfiguration +} diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go index 5357058dc..abdd61b7f 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go @@ -11,359 +11,8 @@ import ( func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { federationFactory := &Factory{} - definition := ` - interface Account { - id: ID! - title: String! - locations: [Location!] - age: Int! - } - - type Location { - country: String! - } - - type Admin implements Account { - id: ID! - title: String! - locations: [Location!] - age: Int! - } - - type Moderator implements Account { - id: ID! - title: String! - locations: [Location!] - age: Int! - } - - type User implements Account { - id: ID! - title: String! - locations: [Location!] - age: Int! - } - - union Accounts = Admin | Moderator | User - - type Query { - allAccountsInterface: [Account] - allAccountsUnion: [Accounts] - user(id: ID!): User - admin(id: ID!): Admin - accountLocations: [Account!]! - }` - - firstSubgraphSDL := ` - interface Account @key(fields: "id") { - id: ID! - title: String! - } - - type Admin implements Account @key(fields: "id"){ - id: ID! - title: String! @external - } - - type Moderator implements Account @key(fields: "id"){ - id: ID! - title: String! - } - - type User implements Account @key(fields: "id"){ - id: ID! - title: String! - } - - union Accounts = Admin | Moderator | User - - type Query { - allAccountsInterface: [Account] - allAccountsUnion: [Accounts] - user(id: ID!): User - admin(id: ID!): Admin - }` - - firstDatasourceConfiguration := plan.DataSourceConfiguration{ - RootNodes: []plan.TypeField{ - { - TypeName: "Admin", - FieldNames: []string{"id"}, - }, - { - TypeName: "Moderator", - FieldNames: []string{"id", "title"}, - }, - { - TypeName: "User", - FieldNames: []string{"id", "title"}, - }, - { - TypeName: "Query", - FieldNames: []string{"allAccountsInterface", "allAccountsUnion", "user", "admin"}, - }, - }, - ChildNodes: []plan.TypeField{ - { - TypeName: "Account", - FieldNames: []string{"id", "title"}, - }, - }, - Custom: ConfigJson(Configuration{ - Fetch: FetchConfiguration{ - URL: "http://first.service", - }, - Federation: FederationConfiguration{ - Enabled: true, - ServiceSDL: firstSubgraphSDL, - }, - UpstreamSchema: firstSubgraphSDL, - }), - Factory: federationFactory, - FederationMetaData: plan.FederationMetaData{ - Keys: plan.FederationFieldConfigurations{ - { - TypeName: "Account", - SelectionSet: "id", - }, - { - TypeName: "Admin", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - { - TypeName: "Moderator", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - { - TypeName: "User", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - }, - }, - } - - secondSubgraphSDL := ` - type Account @key(fields: "id") @interfaceObject { - id: ID! - locations: [Location!] - } - - type Location { - country: String! - } - - type Query { - accountLocations: [Account!]! - }` - - secondDatasourceConfiguration := plan.DataSourceConfiguration{ - RootNodes: []plan.TypeField{ - { - TypeName: "Account", - FieldNames: []string{"id", "locations"}, - }, - { - TypeName: "User", - FieldNames: []string{"id", "locations"}, - }, - { - TypeName: "Moderator", - FieldNames: []string{"id", "locations"}, - }, - { - TypeName: "Admin", - FieldNames: []string{"id", "locations"}, - }, - { - TypeName: "Query", - FieldNames: []string{"accountLocations"}, - }, - }, - ChildNodes: []plan.TypeField{ - { - TypeName: "Location", - FieldNames: []string{"country"}, - }, - }, - Custom: ConfigJson(Configuration{ - Fetch: FetchConfiguration{ - URL: "http://second.service", - }, - Federation: FederationConfiguration{ - Enabled: true, - ServiceSDL: secondSubgraphSDL, - }, - UpstreamSchema: secondSubgraphSDL, - }), - Factory: federationFactory, - FederationMetaData: plan.FederationMetaData{ - Keys: plan.FederationFieldConfigurations{ - { - TypeName: "Account", - SelectionSet: "id", - }, - { - TypeName: "Admin", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - { - TypeName: "Moderator", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - { - TypeName: "User", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - }, - }, - RenameTypes: plan.TypeConfigurations{ - { - TypeName: "Admin", - RenameTo: "Account", - }, - { - TypeName: "Moderator", - RenameTo: "Account", - }, - { - TypeName: "User", - RenameTo: "Account", - }, - }, - } - - thirdSubgraphSDL := ` - type Admin @key(fields: "id"){ - id: ID! - title: String! - }` - - thirdDatasourceConfiguration := plan.DataSourceConfiguration{ - RootNodes: []plan.TypeField{ - { - TypeName: "Admin", - FieldNames: []string{"id", "title"}, - }, - }, - Custom: ConfigJson(Configuration{ - Fetch: FetchConfiguration{ - URL: "http://third.service", - }, - Federation: FederationConfiguration{ - Enabled: true, - ServiceSDL: thirdSubgraphSDL, - }, - UpstreamSchema: thirdSubgraphSDL, - }), - Factory: federationFactory, - FederationMetaData: plan.FederationMetaData{ - Keys: plan.FederationFieldConfigurations{ - { - TypeName: "Admin", - SelectionSet: "id", - }, - }, - }, - } - - fourthSubgraphSDL := ` - type Account @key(fields: "id") @interfaceObject { - id: ID! - age: Int! - }` - - fourthDatasourceConfiguration := plan.DataSourceConfiguration{ - RootNodes: []plan.TypeField{ - { - TypeName: "Account", - FieldNames: []string{"id", "age"}, - }, - { - TypeName: "User", - FieldNames: []string{"id", "age"}, - }, - { - TypeName: "Moderator", - FieldNames: []string{"id", "age"}, - }, - { - TypeName: "Admin", - FieldNames: []string{"id", "age"}, - }, - }, - Custom: ConfigJson(Configuration{ - Fetch: FetchConfiguration{ - URL: "http://fourth.service", - }, - Federation: FederationConfiguration{ - Enabled: true, - ServiceSDL: fourthSubgraphSDL, - }, - UpstreamSchema: fourthSubgraphSDL, - }), - Factory: federationFactory, - FederationMetaData: plan.FederationMetaData{ - Keys: plan.FederationFieldConfigurations{ - { - TypeName: "Account", - SelectionSet: "id", - }, - { - TypeName: "Admin", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - { - TypeName: "Moderator", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - { - TypeName: "User", - InterfaceNames: []string{"Account"}, - SelectionSet: "id", - }, - }, - }, - RenameTypes: plan.TypeConfigurations{ - { - TypeName: "Admin", - RenameTo: "Account", - }, - { - TypeName: "Moderator", - RenameTo: "Account", - }, - { - TypeName: "User", - RenameTo: "Account", - }, - }, - } - - dataSources := []plan.DataSourceConfiguration{ - firstDatasourceConfiguration, - secondDatasourceConfiguration, - thirdDatasourceConfiguration, - fourthDatasourceConfiguration, - } - - planConfiguration := plan.Configuration{ - DataSources: ShuffleDS(dataSources), - DisableResolveFieldPositions: true, - Debug: plan.DebugConfiguration{ - PrintOperationTransformations: true, - PrintQueryPlans: true, - PrintPlanningPaths: true, - PrintNodeSuggestions: true, - }, - } + definition := EntityInterfacesDefinition + planConfiguration := *EntityInterfacesPlanConfiguration(federationFactory) t.Run("query 1 - Interface to interface object", func(t *testing.T) { From 9e5eaf26d6d93292e0d83210e6684f4f88cc6a4e Mon Sep 17 00:00:00 2001 From: spetrunin Date: Tue, 19 Dec 2023 18:46:06 +0200 Subject: [PATCH 13/27] add static string scalar add renaming concrete type to interface type on entity fragments add replacing __typename with a static type name for entity interface names --- .../graphql_datasource/graphql_datasource.go | 11 +++++--- .../representation_variable.go | 25 +++++++++++++------ v2/pkg/engine/plan/configuration.go | 10 ++++++++ v2/pkg/engine/plan/federation_metadata.go | 8 +++--- v2/pkg/engine/resolve/node.go | 1 + v2/pkg/engine/resolve/node_scalar.go | 14 +++++++++++ v2/pkg/engine/resolve/simple_resolver.go | 9 +++++++ 7 files changed, 64 insertions(+), 14 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 401a75671..881d83654 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -923,12 +923,16 @@ func (p *Planner) addOnTypeInlineFragment() { } selectionSet := p.upstreamOperation.AddSelectionSet() - p.addTypenameToSelectionSet(p.nodes[len(p.nodes)-1].Ref) - onTypeName := p.visitor.Config.Types.RenameTypeNameOnMatchBytes([]byte(p.lastFieldEnclosingTypeName)) // rename type name in case it is required by entity interface - onTypeName = p.dataSourceConfig.RenameTypes.RenameTypeNameOnMatchBytes([]byte(p.lastFieldEnclosingTypeName)) + var renamed bool + onTypeName, renamed = p.dataSourceConfig.RenameTypes.RenameTypeNameOnMatchBytesWithResult([]byte(p.lastFieldEnclosingTypeName)) + + // we should not request a typename of interface object + if !renamed { + p.addTypenameToSelectionSet(p.nodes[len(p.nodes)-1].Ref) + } typeRef := p.upstreamOperation.AddNamedType(onTypeName) inlineFragment := p.upstreamOperation.AddInlineFragment(ast.InlineFragment{ @@ -1434,6 +1438,7 @@ func (p *Planner) printOperation() []byte { return nil } + // p.printQueryPlan(p.upstreamOperation) // uncomment to print upstream operation before normalization p.printQueryPlan(operation) buf.Reset() diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go index 424514896..6e7af32d5 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go @@ -24,10 +24,11 @@ func buildRepresentationVariableNode(cfg plan.FederationFieldConfiguration, defi walker := astvisitor.NewWalker(48) visitor := &representationVariableVisitor{ - typeName: cfg.TypeName, - addOnType: true, - addTypeName: true, - Walker: &walker, + typeName: cfg.TypeName, + interfaceObjectTypeName: cfg.InterfaceObjectTypeName, + addOnType: true, + addTypeName: true, + Walker: &walker, } walker.RegisterEnterDocumentVisitor(visitor) walker.RegisterFieldVisitor(visitor) @@ -90,7 +91,9 @@ type representationVariableVisitor struct { currentFields []objectFields rootObject *resolve.Object - typeName string + typeName string + interfaceObjectTypeName string + addOnType bool addTypeName bool } @@ -103,9 +106,17 @@ func (v *representationVariableVisitor) EnterDocument(key, definition *ast.Docum if v.addTypeName { typeNameField := &resolve.Field{ Name: []byte("__typename"), - Value: &resolve.String{ + } + + if v.interfaceObjectTypeName != "" { + typeNameField.Value = &resolve.StaticString{ + Path: []string{"__typename"}, + Value: v.interfaceObjectTypeName, + } + } else { + typeNameField.Value = &resolve.String{ Path: []string{"__typename"}, - }, + } } if v.addOnType { diff --git a/v2/pkg/engine/plan/configuration.go b/v2/pkg/engine/plan/configuration.go index 1de493dcf..0b62587c9 100644 --- a/v2/pkg/engine/plan/configuration.go +++ b/v2/pkg/engine/plan/configuration.go @@ -56,6 +56,16 @@ func (t *TypeConfigurations) RenameTypeNameOnMatchBytes(typeName []byte) []byte return typeName } +func (t *TypeConfigurations) RenameTypeNameOnMatchBytesWithResult(typeName []byte) (newTypeName []byte, renamed bool) { + str := string(typeName) + for i := range *t { + if (*t)[i].TypeName == str { + return []byte((*t)[i].RenameTo), true + } + } + return typeName, false +} + type TypeConfiguration struct { TypeName string // RenameTo modifies the TypeName diff --git a/v2/pkg/engine/plan/federation_metadata.go b/v2/pkg/engine/plan/federation_metadata.go index d8beea1bc..deca2e231 100644 --- a/v2/pkg/engine/plan/federation_metadata.go +++ b/v2/pkg/engine/plan/federation_metadata.go @@ -7,10 +7,10 @@ type FederationMetaData struct { } type FederationFieldConfiguration struct { - TypeName string - FieldName string - SelectionSet string - InterfaceNames []string + TypeName string + FieldName string + SelectionSet string + InterfaceObjectTypeName string } type FederationFieldConfigurations []FederationFieldConfiguration diff --git a/v2/pkg/engine/resolve/node.go b/v2/pkg/engine/resolve/node.go index abcfb63c2..f4a904fb5 100644 --- a/v2/pkg/engine/resolve/node.go +++ b/v2/pkg/engine/resolve/node.go @@ -13,6 +13,7 @@ const ( NodeKindBigInt NodeKindCustom NodeKindScalar + NodeKindStaticString ) type Node interface { diff --git a/v2/pkg/engine/resolve/node_scalar.go b/v2/pkg/engine/resolve/node_scalar.go index b391a48c1..ac465a9fe 100644 --- a/v2/pkg/engine/resolve/node_scalar.go +++ b/v2/pkg/engine/resolve/node_scalar.go @@ -20,6 +20,7 @@ type String struct { Export *FieldExport `json:"export,omitempty"` UnescapeResponseJson bool `json:"unescape_response_json,omitempty"` IsTypeName bool `json:"is_type_name,omitempty"` + StaticValue string `json:"static_value,omitempty"` } func (_ *String) NodeKind() NodeKind { @@ -30,6 +31,19 @@ func (s *String) NodePath() []string { return s.Path } +type StaticString struct { + Path []string + Value string +} + +func (_ *StaticString) NodeKind() NodeKind { + return NodeKindStaticString +} + +func (s *StaticString) NodePath() []string { + return s.Path +} + type Boolean struct { Path []string Nullable bool diff --git a/v2/pkg/engine/resolve/simple_resolver.go b/v2/pkg/engine/resolve/simple_resolver.go index f647b963e..1371e9125 100644 --- a/v2/pkg/engine/resolve/simple_resolver.go +++ b/v2/pkg/engine/resolve/simple_resolver.go @@ -29,6 +29,8 @@ func (r *SimpleResolver) resolveNode(node Node, data []byte, buf *fastbuffer.Fas return case *String: return r.resolveString(n, data, buf) + case *StaticString: + return r.resolveStaticString(n, data, buf) case *Boolean: return r.resolveBoolean(n, data, buf) case *Integer: @@ -291,6 +293,13 @@ func (r *SimpleResolver) resolveString(str *String, data []byte, stringBuf *fast return nil } +func (r *SimpleResolver) resolveStaticString(str *StaticString, data []byte, stringBuf *fastbuffer.FastBuffer) error { + stringBuf.WriteBytes(quote) + stringBuf.WriteBytes([]byte(str.Value)) + stringBuf.WriteBytes(quote) + return nil +} + func (r *SimpleResolver) resolveEmptyArray(b *fastbuffer.FastBuffer) { b.WriteBytes(lBrack) b.WriteBytes(rBrack) From ff68075848d02517a43eb131a11a166d3c6cb2ed Mon Sep 17 00:00:00 2001 From: spetrunin Date: Tue, 19 Dec 2023 19:38:50 +0200 Subject: [PATCH 14/27] implement entity interface test #1 --- ...ource_federation_entity_interfaces_test.go | 107 +++++++++++++++++- 1 file changed, 105 insertions(+), 2 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go index abdd61b7f..0474e2b23 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go @@ -33,13 +33,116 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://first.service","body":{"query":""}}`, + Input: `{"method":"POST","url":"http://localhost:4001/graphql","body":{"query":"{allAccountsInterface {__typename ... on Admin {id __typename} ... on Moderator {id __typename} ... on User {id __typename}}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, DataSource: &Source{}, }, DataSourceIdentifier: []byte("graphql_datasource.Source"), }, - Fields: []*resolve.Field{}, + Fields: []*resolve.Field{ + { + Name: []byte("allAccountsInterface"), + Value: &resolve.Array{ + Path: []string{"allAccountsInterface"}, + Nullable: true, + Item: &resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("id"), + Value: &resolve.Scalar{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + { + Name: []byte("locations"), + Value: &resolve.Array{ + Path: []string{"locations"}, + Nullable: true, + Item: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("country"), + Value: &resolve.String{ + Path: []string{"country"}, + }, + }, + }, + }, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + }, + Fetch: &resolve.SingleFetch{ + SerialID: 1, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Account {locations {country}}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("Moderator")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + }, + }, }, }, }, From 71b64d954dd37d6dd200b36054646e489e2d79f3 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Thu, 28 Dec 2023 14:08:44 +0200 Subject: [PATCH 15/27] add entity interface configs to federation metadata fix initial entity interface planning --- .../entity_interfaces_engine_config.go | 78 +++--- .../graphql_datasource/graphql_datasource.go | 50 +++- ...ource_federation_entity_interfaces_test.go | 263 +++++++++++++++++- .../representation_variable.go | 43 ++- .../representation_variable_test.go | 61 +++- .../datasourcetesting/datasourcetesting.go | 3 + .../plan/abstract_selection_rewriter.go | 54 ++-- .../abstract_selection_rewriter_helpers.go | 22 +- v2/pkg/engine/plan/configuration_visitor.go | 151 ++++++++-- .../engine/plan/datasource_configuration.go | 1 - .../engine/plan/datasource_filter_visitor.go | 13 + v2/pkg/engine/plan/federation_metadata.go | 20 +- v2/pkg/engine/resolve/node_scalar.go | 1 - .../fixtures/federated_schema.golden | 1 + 14 files changed, 625 insertions(+), 136 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go index 17ace1679..1a9321957 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go +++ b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go @@ -115,6 +115,12 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config }), Factory: factory, FederationMetaData: plan.FederationMetaData{ + EntityInterfaces: []plan.InterfaceObjectConfiguration{ + { + InterfaceName: "Account", + ImplementingTypeNames: []string{"Admin", "Moderator", "User"}, + }, + }, Keys: plan.FederationFieldConfigurations{ { TypeName: "Account", @@ -191,42 +197,31 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config }), Factory: factory, FederationMetaData: plan.FederationMetaData{ + InterfaceObjects: []plan.InterfaceObjectConfiguration{ + { + InterfaceName: "Account", + ImplementingTypeNames: []string{"Admin", "Moderator", "User"}, + }, + }, Keys: plan.FederationFieldConfigurations{ { TypeName: "Account", SelectionSet: "id", }, { - TypeName: "Admin", - InterfaceObjectTypeName: "Account", - SelectionSet: "id", + TypeName: "Admin", + SelectionSet: "id", }, { - TypeName: "Moderator", - InterfaceObjectTypeName: "Account", - SelectionSet: "id", + TypeName: "Moderator", + SelectionSet: "id", }, { - TypeName: "User", - InterfaceObjectTypeName: "Account", - SelectionSet: "id", + TypeName: "User", + SelectionSet: "id", }, }, }, - RenameTypes: plan.TypeConfigurations{ - { - TypeName: "Admin", - RenameTo: "Account", - }, - { - TypeName: "Moderator", - RenameTo: "Account", - }, - { - TypeName: "User", - RenameTo: "Account", - }, - }, } thirdSubgraphSDL := ` @@ -300,42 +295,31 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config }), Factory: factory, FederationMetaData: plan.FederationMetaData{ + InterfaceObjects: []plan.InterfaceObjectConfiguration{ + { + InterfaceName: "Account", + ImplementingTypeNames: []string{"Admin", "Moderator", "User"}, + }, + }, Keys: plan.FederationFieldConfigurations{ { TypeName: "Account", SelectionSet: "id", }, { - TypeName: "Admin", - InterfaceObjectTypeName: "Account", - SelectionSet: "id", + TypeName: "Admin", + SelectionSet: "id", }, { - TypeName: "Moderator", - InterfaceObjectTypeName: "Account", - SelectionSet: "id", + TypeName: "Moderator", + SelectionSet: "id", }, { - TypeName: "User", - InterfaceObjectTypeName: "Account", - SelectionSet: "id", + TypeName: "User", + SelectionSet: "id", }, }, }, - RenameTypes: plan.TypeConfigurations{ - { - TypeName: "Admin", - RenameTo: "Account", - }, - { - TypeName: "Moderator", - RenameTo: "Account", - }, - { - TypeName: "User", - RenameTo: "Account", - }, - }, } dataSources := []plan.DataSourceConfiguration{ @@ -353,6 +337,8 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config PrintQueryPlans: true, PrintPlanningPaths: true, PrintNodeSuggestions: true, + + // DatasourceVisitor: true, }, } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 881d83654..662fd0db3 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -596,13 +596,24 @@ func (p *Planner) EnterInlineFragment(ref int) { // we need to add __typename field to selection set if hasTypeCondition && !IsOfTheSameType { typeCondition := p.visitor.Operation.InlineFragmentTypeConditionName(ref) - fragmentTypeRef := p.upstreamOperation.AddNamedType(p.visitor.Config.Types.RenameTypeNameOnMatchBytes(typeCondition)) + + onTypeName := p.visitor.Config.Types.RenameTypeNameOnMatchBytes(typeCondition) + + // rename type name in case it is required by entity interface + shouldRenameInterfaceObjectType, newName := p.interfaceObjectTypeShouldBeRenamed(string(onTypeName)) + if shouldRenameInterfaceObjectType { + onTypeName = []byte(newName) + } + + fragmentTypeRef := p.upstreamOperation.AddNamedType(onTypeName) p.upstreamOperation.InlineFragments[inlineFragmentRef].TypeCondition.Type = fragmentTypeRef - // add __typename field to selection set which contains typeCondition - // so that the resolver can distinguish between the response types - p.addTypenameToSelectionSet(currentSelectionSet) + if !shouldRenameInterfaceObjectType { + // add __typename field to selection set which contains typeCondition + // so that the resolver can distinguish between the response types + p.addTypenameToSelectionSet(currentSelectionSet) + } } selection := ast.Selection{ @@ -809,7 +820,7 @@ func (p *Planner) addRepresentationsVariable() { func (p *Planner) buildRepresentationsVariable() resolve.Variable { objects := make([]*resolve.Object, 0, len(p.dataSourcePlannerConfig.RequiredFields)) for _, cfg := range p.dataSourcePlannerConfig.RequiredFields { - node, err := buildRepresentationVariableNode(cfg, p.visitor.Definition) + node, err := buildRepresentationVariableNode(p.visitor.Definition, cfg, p.dataSourceConfig) if err != nil { p.visitor.Walker.StopWithInternalErr(err) return nil @@ -908,6 +919,15 @@ func (p *Planner) isInEntitiesSelectionSet() bool { return bytes.Equal(fieldName, []byte("_entities")) } +func (p *Planner) interfaceObjectTypeShouldBeRenamed(typeName string) (ok bool, newName string) { + for _, interfaceObjectCfg := range p.dataSourceConfig.FederationMetaData.InterfaceObjects { + if slices.Contains(interfaceObjectCfg.ImplementingTypeNames, typeName) { + return true, interfaceObjectCfg.InterfaceName + } + } + return false, "" +} + func (p *Planner) addOnTypeInlineFragment() { p.DebugPrint("addOnTypeInlineFragment") @@ -926,11 +946,13 @@ func (p *Planner) addOnTypeInlineFragment() { onTypeName := p.visitor.Config.Types.RenameTypeNameOnMatchBytes([]byte(p.lastFieldEnclosingTypeName)) // rename type name in case it is required by entity interface - var renamed bool - onTypeName, renamed = p.dataSourceConfig.RenameTypes.RenameTypeNameOnMatchBytesWithResult([]byte(p.lastFieldEnclosingTypeName)) + shouldRenameInterfaceObjectType, newName := p.interfaceObjectTypeShouldBeRenamed(p.lastFieldEnclosingTypeName) + if shouldRenameInterfaceObjectType { + onTypeName = []byte(newName) + } // we should not request a typename of interface object - if !renamed { + if !shouldRenameInterfaceObjectType { p.addTypenameToSelectionSet(p.nodes[len(p.nodes)-1].Ref) } @@ -1253,8 +1275,10 @@ func (p *Planner) configureObjectFieldSource(upstreamFieldRef, downstreamFieldRe } const ( - normalizationFailedErrMsg = "printOperation: normalization failed" - parseDocumentFailedErrMsg = "printOperation: parse %s failed" + normalizationFailedErrMsg = "upstream operation: normalization failed: %s" + printOperationFailedErrMsg = "upstream operation: failed to print: %s" + validationFailedErrMsg = "upstream operation: validation failed: %s" + parseDocumentFailedErrMsg = "upstream operation: parse %s failed" ) func (p *Planner) debugPrintOperationOnEnter(kind ast.NodeKind, ref int) { @@ -1427,14 +1451,14 @@ func (p *Planner) printOperation() []byte { // normalize upstream operation if !p.normalizeOperation(operation, definition, report) { - p.stopWithError(normalizationFailedErrMsg) + p.stopWithError(normalizationFailedErrMsg, report.Error()) return nil } validator := astvalidation.DefaultOperationValidator() validator.Validate(operation, definition, report) if report.HasErrors() { - p.stopWithError("validation failed: %s", report.Error()) + p.stopWithError(validationFailedErrMsg, report.Error()) return nil } @@ -1446,7 +1470,7 @@ func (p *Planner) printOperation() []byte { // print upstream operation err = astprinter.Print(operation, p.visitor.Definition, buf) if err != nil { - p.stopWithError(normalizationFailedErrMsg) + p.stopWithError(printOperationFailedErrMsg, report.Error()) return nil } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go index 0474e2b23..463c6b6da 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go @@ -14,8 +14,97 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { definition := EntityInterfacesDefinition planConfiguration := *EntityInterfacesPlanConfiguration(federationFactory) - t.Run("query 1 - Interface to interface object", func(t *testing.T) { + t.Run("query 0 - Interface object typename", func(t *testing.T) { + t.Run("run", RunTest( + definition, + ` + query _0_InterfaceObjectTypename { + accountLocations { + id + __typename + } + }`, + "_0_InterfaceObjectTypename", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {id}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + DataSource: &Source{}, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + Fields: []*resolve.Field{ + { + Name: []byte("accountLocations"), + Value: &resolve.Array{ + Path: []string{"accountLocations"}, + Item: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("id"), + Value: &resolve.Scalar{ + Path: []string{"id"}, + }, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + IsTypeName: true, + }, + }, + }, + Fetch: &resolve.SingleFetch{ + FetchID: 1, + DependsOnFetchIDs: []int{0}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4001/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Account {__typename}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Account")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + }, + }, + }, + }, + }, + planConfiguration, + WithMultiFetchPostProcessor(), + )) + }) + t.Run("query 1 - Interface to interface object", func(t *testing.T) { t.Run("run", RunTest( definition, ` @@ -75,7 +164,8 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { }, }, Fetch: &resolve.SingleFetch{ - SerialID: 1, + FetchID: 1, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Account {locations {country}}}}","variables":{"representations":[$$0$$]}}}`, Variables: []resolve.Variable{ @@ -89,14 +179,14 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Path: []string{"__typename"}, Value: "Account", }, - OnTypeNames: [][]byte{[]byte("Admin")}, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, }, { Name: []byte("id"), Value: &resolve.String{ Path: []string{"id"}, }, - OnTypeNames: [][]byte{[]byte("Admin")}, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, }, { Name: []byte("__typename"), @@ -104,14 +194,14 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Path: []string{"__typename"}, Value: "Account", }, - OnTypeNames: [][]byte{[]byte("Moderator")}, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, }, { Name: []byte("id"), Value: &resolve.String{ Path: []string{"id"}, }, - OnTypeNames: [][]byte{[]byte("Moderator")}, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, }, { Name: []byte("__typename"), @@ -119,14 +209,14 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Path: []string{"__typename"}, Value: "Account", }, - OnTypeNames: [][]byte{[]byte("User")}, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, }, { Name: []byte("id"), Value: &resolve.String{ Path: []string{"id"}, }, - OnTypeNames: [][]byte{[]byte("User")}, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, }, }, }), @@ -151,4 +241,161 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { }) + t.Run("query 10 - InterfaceObjectToConcreteTypeWithExternalField - no fragments", func(t *testing.T) { + t.Run("run", RunTest( + definition, + ` + query _10_InterfaceObjectToConcreteTypeWithExternalField_No_Fragments { + accountLocations { + id + title + } + }`, + "_10_InterfaceObjectToConcreteTypeWithExternalField_No_Fragments", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {id}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + DataSource: &Source{}, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + Fields: []*resolve.Field{ + { + Name: []byte("accountLocations"), + Value: &resolve.Array{ + Path: []string{"accountLocations"}, + Item: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("id"), + Value: &resolve.Scalar{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + { + Name: []byte("title"), + Value: &resolve.String{ + Path: []string{"title"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + }, + Fetch: &resolve.SerialFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{ + FetchID: 2, + DependsOnFetchIDs: []int{0}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4001/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {__typename} ... on Moderator {title __typename} ... on User {title __typename}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + &resolve.SingleFetch{ + FetchID: 1, + DependsOnFetchIDs: []int{0, 2}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4003/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {title}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + planConfiguration, + WithMultiFetchPostProcessor(), + )) + }) } diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go index 6e7af32d5..9d577b3be 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go @@ -2,6 +2,7 @@ package graphql_datasource import ( "bytes" + "slices" "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" @@ -15,7 +16,7 @@ type objectFields struct { fields *[]*resolve.Field } -func buildRepresentationVariableNode(cfg plan.FederationFieldConfiguration, definition *ast.Document) (*resolve.Object, error) { +func buildRepresentationVariableNode(definition *ast.Document, cfg plan.FederationFieldConfiguration, dsCfg plan.DataSourceConfiguration) (*resolve.Object, error) { key, report := plan.RequiredFieldsFragment(cfg.TypeName, cfg.SelectionSet, false) if report.HasErrors() { return nil, report @@ -23,9 +24,25 @@ func buildRepresentationVariableNode(cfg plan.FederationFieldConfiguration, defi walker := astvisitor.NewWalker(48) + var interfaceObjectTypeName *string + for _, interfaceObjCfg := range dsCfg.FederationMetaData.InterfaceObjects { + if slices.Contains(interfaceObjCfg.ImplementingTypeNames, cfg.TypeName) { + interfaceObjectTypeName = &interfaceObjCfg.InterfaceName + break + } + } + var entityInterfaceTypeName *string + for _, entityInterfaceCfg := range dsCfg.FederationMetaData.EntityInterfaces { + if slices.Contains(entityInterfaceCfg.ImplementingTypeNames, cfg.TypeName) { + entityInterfaceTypeName = &entityInterfaceCfg.InterfaceName + break + } + } + visitor := &representationVariableVisitor{ typeName: cfg.TypeName, - interfaceObjectTypeName: cfg.InterfaceObjectTypeName, + interfaceObjectTypeName: interfaceObjectTypeName, + entityInterfaceTypeName: entityInterfaceTypeName, addOnType: true, addTypeName: true, Walker: &walker, @@ -92,7 +109,8 @@ type representationVariableVisitor struct { rootObject *resolve.Object typeName string - interfaceObjectTypeName string + interfaceObjectTypeName *string + entityInterfaceTypeName *string addOnType bool addTypeName bool @@ -108,10 +126,10 @@ func (v *representationVariableVisitor) EnterDocument(key, definition *ast.Docum Name: []byte("__typename"), } - if v.interfaceObjectTypeName != "" { + if v.interfaceObjectTypeName != nil { typeNameField.Value = &resolve.StaticString{ Path: []string{"__typename"}, - Value: v.interfaceObjectTypeName, + Value: *v.interfaceObjectTypeName, } } else { typeNameField.Value = &resolve.String{ @@ -120,7 +138,7 @@ func (v *representationVariableVisitor) EnterDocument(key, definition *ast.Docum } if v.addOnType { - typeNameField.OnTypeNames = [][]byte{[]byte(v.typeName)} + v.addTypeNameToField(typeNameField) } fields = append(fields, typeNameField) @@ -153,12 +171,23 @@ func (v *representationVariableVisitor) EnterField(ref int) { } if v.addOnType && v.currentFields[len(v.currentFields)-1].isRoot { - currentField.OnTypeNames = [][]byte{[]byte(v.typeName)} + v.addTypeNameToField(currentField) } *v.currentFields[len(v.currentFields)-1].fields = append(*v.currentFields[len(v.currentFields)-1].fields, currentField) } +func (v *representationVariableVisitor) addTypeNameToField(field *resolve.Field) { + switch { + case v.interfaceObjectTypeName != nil: + field.OnTypeNames = [][]byte{[]byte(v.typeName), []byte(*v.interfaceObjectTypeName)} + case v.entityInterfaceTypeName != nil: + field.OnTypeNames = [][]byte{[]byte(v.typeName), []byte(*v.entityInterfaceTypeName)} + default: + field.OnTypeNames = [][]byte{[]byte(v.typeName)} + } +} + func (v *representationVariableVisitor) LeaveField(ref int) { if v.currentFields[len(v.currentFields)-1].popOnField == ref { v.currentFields = v.currentFields[:len(v.currentFields)-1] diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable_test.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable_test.go index 5319397da..ee15f3d72 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable_test.go @@ -11,14 +11,14 @@ import ( ) func TestBuildRepresentationVariableNode(t *testing.T) { - runTest := func(t *testing.T, definitionStr, keyStr string, expectedNode resolve.Node) { + runTest := func(t *testing.T, definitionStr, keyStr string, dsConfig plan.DataSourceConfiguration, expectedNode resolve.Node) { definition, _ := astparser.ParseGraphqlDocumentString(definitionStr) cfg := plan.FederationFieldConfiguration{ TypeName: "User", SelectionSet: keyStr, } - node, err := buildRepresentationVariableNode(cfg, &definition) + node, err := buildRepresentationVariableNode(&definition, cfg, dsConfig) require.NoError(t, err) require.Equal(t, expectedNode, node) @@ -32,7 +32,9 @@ func TestBuildRepresentationVariableNode(t *testing.T) { id: String! name: String! } - `, `id name`, + `, + `id name`, + plan.DataSourceConfiguration{}, &resolve.Object{ Nullable: true, Fields: []*resolve.Field{ @@ -61,6 +63,55 @@ func TestBuildRepresentationVariableNode(t *testing.T) { }) }) + t.Run("with interface object", func(t *testing.T) { + runTest(t, ` + scalar String + + type User { + id: String! + name: String! + } + `, + `id name`, + plan.DataSourceConfiguration{ + FederationMetaData: plan.FederationMetaData{ + InterfaceObjects: []plan.InterfaceObjectConfiguration{ + { + InterfaceName: "Account", + ImplementingTypeNames: []string{"User", "Admin"}, + }, + }, + }, + }, + &resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("name"), + Value: &resolve.String{ + Path: []string{"name"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + }, + }) + }) + t.Run("deeply nested", func(t *testing.T) { runTest(t, ` scalar String @@ -82,7 +133,9 @@ func TestBuildRepresentationVariableNode(t *testing.T) { zip: Float! } - `, `id name account { accoundID address(home: true) { zip } }`, + `, + `id name account { accoundID address(home: true) { zip } }`, + plan.DataSourceConfiguration{}, &resolve.Object{ Nullable: true, Fields: []*resolve.Field{ diff --git a/v2/pkg/engine/datasourcetesting/datasourcetesting.go b/v2/pkg/engine/datasourcetesting/datasourcetesting.go index fa5970cd7..f49ba9a99 100644 --- a/v2/pkg/engine/datasourcetesting/datasourcetesting.go +++ b/v2/pkg/engine/datasourcetesting/datasourcetesting.go @@ -80,6 +80,9 @@ func RunTest(definition, operation, operationName string, expectedPlan plan.Plan expectedBytes, _ := json.MarshalIndent(expectedPlan, "", " ") if string(expectedBytes) != string(actualBytes) { + // os.WriteFile("actual_plan.json", actualBytes, 0644) + // os.WriteFile("expected_plan.json", expectedBytes, 0644) + assert.Equal(t, expectedPlan, actualPlan) t.Error(cmp.Diff(string(expectedBytes), string(actualBytes))) } diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter.go b/v2/pkg/engine/plan/abstract_selection_rewriter.go index f153c3062..92732c932 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter.go @@ -195,11 +195,7 @@ func (r *fieldSelectionRewriter) rewriteUnionSelection(fieldRef int, fieldInfo s for _, inlineFragmentOnInterface := range fieldInfo.inlineFragmentsOnInterfaces { // we need to recursively flatten nested fragments on interfaces - r.flattenFragmentOnInterface( - inlineFragmentOnInterface.selectionSetInfo, - inlineFragmentOnInterface.typeNamesImplementingInterface, - unionTypeNames, - &newSelectionRefs) + r.flattenFragmentOnInterface(inlineFragmentOnInterface.selectionSetInfo, inlineFragmentOnInterface.typeNamesImplementingInterface, unionTypeNames, &newSelectionRefs, false) } // filter existing fragments by type names exists in the current datasource @@ -249,13 +245,31 @@ func (r *fieldSelectionRewriter) processInterfaceSelection(fieldRef int, interfa if !hasNode { return false, errors.New("unexpected error: interface type definition not found in the upstream schema") } + + var isInterfaceObject bool + var interfaceTypeNames []string + if node.Kind != ast.NodeKindInterfaceTypeDefinition { - return false, errors.New("unexpected error: node kind is not interface type definition in the upstream schema") + interfaceTypeNameStr := string(interfaceTypeName) + for _, k := range r.dsConfiguration.FederationMetaData.InterfaceObjects { + if k.InterfaceName == interfaceTypeNameStr { + isInterfaceObject = true + interfaceTypeNames = k.ImplementingTypeNames + break + } + } + + if !isInterfaceObject { + return false, errors.New("unexpected error: node kind is not interface type definition in the upstream schema") + } } - interfaceTypeNames, ok := r.upstreamDefinition.InterfaceTypeDefinitionImplementedByObjectWithNames(node.Ref) - if !ok { - return false, nil + if !isInterfaceObject { + var ok bool + interfaceTypeNames, ok = r.upstreamDefinition.InterfaceTypeDefinitionImplementedByObjectWithNames(node.Ref) + if !ok { + return false, nil + } } sort.Strings(interfaceTypeNames) @@ -348,22 +362,20 @@ func (r *fieldSelectionRewriter) interfaceFieldSelectionNeedsRewrite(selectionSe func (r *fieldSelectionRewriter) rewriteInterfaceSelection(fieldRef int, fieldInfo selectionSetInfo, interfaceTypeNames []string) error { newSelectionRefs := make([]int, 0, len(interfaceTypeNames)+1) // 1 for __typename - if fieldInfo.hasTypeNameSelection { - // we should preserve __typename if it was in the original query as it is explicitly requested - typeNameSelectionRef, _ := r.typeNameSelection() - newSelectionRefs = append(newSelectionRefs, typeNameSelectionRef) - } + shouldAddTypeName := fieldInfo.hasTypeNameSelection r.flattenFragmentOnInterface( fieldInfo, interfaceTypeNames, interfaceTypeNames, - &newSelectionRefs) + &newSelectionRefs, + shouldAddTypeName, + ) return r.replaceFieldSelections(fieldRef, newSelectionRefs) } -func (r *fieldSelectionRewriter) flattenFragmentOnInterface(selectionSetInfo selectionSetInfo, typeNamesImplementingInterfaceInCurrentDS []string, allowedTypeNames []string, selectionRefs *[]int) { +func (r *fieldSelectionRewriter) flattenFragmentOnInterface(selectionSetInfo selectionSetInfo, typeNamesImplementingInterfaceInCurrentDS []string, allowedTypeNames []string, selectionRefs *[]int, shouldAddTypeName bool) { if len(typeNamesImplementingInterfaceInCurrentDS) == 0 { return } @@ -377,7 +389,7 @@ func (r *fieldSelectionRewriter) flattenFragmentOnInterface(selectionSetInfo sel if selectionSetInfo.hasFields { for _, typeName := range filteredImplementingTypes { - *selectionRefs = append(*selectionRefs, r.createFragmentSelection(typeName, selectionSetInfo.fields)) + *selectionRefs = append(*selectionRefs, r.createFragmentSelection(typeName, selectionSetInfo.fields, shouldAddTypeName)) } } @@ -388,6 +400,7 @@ func (r *fieldSelectionRewriter) flattenFragmentOnInterface(selectionSetInfo sel } fragmentSelectionRef := r.operation.CopySelection(inlineFragmentInfo.selectionRef) + *selectionRefs = append(*selectionRefs, fragmentSelectionRef) } @@ -397,11 +410,6 @@ func (r *fieldSelectionRewriter) flattenFragmentOnInterface(selectionSetInfo sel continue } - r.flattenFragmentOnInterface( - inlineFragmentInfo.selectionSetInfo, - inlineFragmentInfo.typeNamesImplementingInterface, - filteredImplementingTypes, - selectionRefs, - ) + r.flattenFragmentOnInterface(inlineFragmentInfo.selectionSetInfo, inlineFragmentInfo.typeNamesImplementingInterface, filteredImplementingTypes, selectionRefs, false) } } diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go b/v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go index a7c297e3a..819e0ebb4 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go @@ -309,13 +309,18 @@ func (r *fieldSelectionRewriter) hasFieldOnDataSource(typeName string, fieldName r.dsConfiguration.HasChildNode(typeName, fieldName) } -func (r *fieldSelectionRewriter) createFragmentSelection(typeName string, fields []fieldSelection) (selectionRef int) { +func (r *fieldSelectionRewriter) createFragmentSelection(typeName string, fields []fieldSelection, addTypeName bool) (selectionRef int) { selectionRefs := make([]int, 0, len(fields)) for _, sharedField := range fields { newFieldSelectionRef := r.operation.CopySelection(sharedField.fieldSelectionRef) selectionRefs = append(selectionRefs, newFieldSelectionRef) } + if addTypeName { + typeNameSelectionRef, _ := r.typeNameSelection() + selectionRefs = append(selectionRefs, typeNameSelectionRef) + } + selectionSetRef := r.operation.AddSelectionSetToDocument(ast.SelectionSet{ SelectionRefs: selectionRefs, }) @@ -345,3 +350,18 @@ func (r *fieldSelectionRewriter) typeNameSelection() (selectionRef int, fieldRef Kind: ast.SelectionKindField, }), field.Ref } + +func (r *fieldSelectionRewriter) addTypeNameToExistingFragment(fragmentSelectionRef int) { + if r.operation.Selections[fragmentSelectionRef].Kind != ast.SelectionKindInlineFragment { + return + } + + inlineFragmentRef := r.operation.Selections[fragmentSelectionRef].Ref + inlineFragmentSelectionSetRef, ok := r.operation.InlineFragmentSelectionSet(inlineFragmentRef) + if !ok { + return + } + + typeNameSelectionRef, _ := r.typeNameSelection() + r.operation.AddSelectionRefToSelectionSet(inlineFragmentSelectionSetRef, typeNameSelectionRef) +} diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index 8bdb34b53..2c1b7e7fd 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -354,6 +354,10 @@ func (c *configurationVisitor) EnterField(ref int) { } isSubscription := c.isSubscription(root.Ref, currentPath) + if c.secondaryRun && c.pathIsPlannedOnAllPlanners(currentPath) { + return + } + plannerIdx, planned := c.planWithExistingPlanners(ref, typeName, fieldName, currentPath, parentPath, precedingParentPath) if !planned { plannerIdx, planned = c.addNewPlanner(ref, typeName, fieldName, currentPath, parentPath, isSubscription) @@ -386,7 +390,8 @@ func (c *configurationVisitor) addPlannerDependencies(fieldRef int, currentPlann } if !notified { if notifyPlannerIdx == currentPlannerIdx { - c.walker.StopWithInternalErr(fmt.Errorf("wrong fetch dependencies planner %d depends on itself", notifyPlannerIdx)) + continue + // c.walker.StopWithInternalErr(fmt.Errorf("wrong fetch dependencies planner %d depends on itself", notifyPlannerIdx)) } c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs = append(c.planners[notifyPlannerIdx].objectFetchConfiguration.dependsOnFetchIDs, currentPlannerIdx) @@ -473,6 +478,17 @@ func (c *configurationVisitor) handleProvidesSuggestions(ref int, typeName, fiel } } +func (c *configurationVisitor) pathIsPlannedOnAllPlanners(currentPath string) bool { + allPlannersHasPath := true + for _, plannerConfig := range c.planners { + if !plannerConfig.hasPath(currentPath) { + allPlannersHasPath = false + } + } + + return allPlannersHasPath +} + func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, fieldName, currentPath, parentPath, precedingParentPath string) (plannerIdx int, planned bool) { dsHashes := c.nodeSuggestions.SuggestionsForPath(typeName, fieldName, currentPath) @@ -517,6 +533,10 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field planningBehaviour.MergeAliasedRootNodes { // same parent + root node = root sibling + if !c.couldHandleFieldsRequiredByKey(plannerConfig.dataSourceConfiguration, typeName) { + return -1, false + } + c.addPath(plannerIdx, pathConfiguration{ path: currentPath, shouldWalkFields: true, @@ -530,11 +550,18 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field return plannerIdx, true } if plannerConfig.hasPath(parentPath) || plannerConfig.hasPath(precedingParentPath) { - if pathAdded := c.addPlannerPathForTypename(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour); pathAdded { + pathAdded, halted := c.addPlannerPathForTypename(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour) + switch { + case pathAdded: return plannerIdx, true + case halted: + return -1, false } if isProvided || hasChildNode || (hasRootNode && planningBehaviour.MergeAliasedRootNodes) { + if !c.couldHandleFieldsRequiredByKey(plannerConfig.dataSourceConfiguration, typeName) { + return -1, false + } // has parent path + has child node = child c.addPath(plannerIdx, pathConfiguration{ @@ -550,12 +577,20 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field return plannerIdx, true } - if pathAdded := c.addPlannerPathForUnionChildOfObjectParent(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour); pathAdded { + pathAdded, halted = c.addPlannerPathForUnionChildOfObjectParent(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour) + switch { + case pathAdded: return plannerIdx, true + case halted: + return -1, false } - if pathAdded := c.addPlannerPathForChildOfAbstractParent(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour); pathAdded { + pathAdded, halted = c.addPlannerPathForChildOfAbstractParent(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour) + switch { + case pathAdded: return plannerIdx, true + case halted: + return -1, false } } } @@ -601,14 +636,29 @@ func (c *configurationVisitor) addNewPlanner(ref int, typeName, fieldName, curre } } + isEntityInterface := false + for _, interfaceObjCfg := range config.FederationMetaData.EntityInterfaces { + if interfaceObjCfg.InterfaceName == typeName { + isEntityInterface = true + break + } + } + // we should handle a new planner for a __typename // only when it is the first field on a query - shouldHandleTypeName := fieldName == typeNameField && parentPath == "query" + // or we are on the entity interface object + shouldHandleTypeName := + fieldName == typeNameField && parentPath == "query" || + isEntityInterface if !shouldHandleTypeName && !config.HasRootNode(typeName, fieldName) { return -1, false } + if !c.couldHandleFieldsRequiredByKey(*config, typeName) { + return -1, false + } + planner := config.Factory.Planner(c.ctx) isParentAbstract := c.isParentTypeNodeAbstractType() @@ -751,23 +801,27 @@ func (c *configurationVisitor) LeaveField(ref int) { func (c *configurationVisitor) addPlannerPathForUnionChildOfObjectParent( plannerIndex int, currentPath string, fieldRef int, fieldName string, typeName string, planningBehaviour DataSourcePlanningBehavior, -) (pathAdded bool) { +) (pathAdded bool, halted bool) { if c.walker.EnclosingTypeDefinition.Kind != ast.NodeKindObjectTypeDefinition { - return false + return false, false } fieldDefRef, exists := c.definition.NodeFieldDefinitionByName(c.walker.EnclosingTypeDefinition, c.operation.FieldNameBytes(fieldRef)) if !exists { - return false + return false, false } fieldDefTypeName := c.definition.FieldDefinitionTypeNameBytes(fieldDefRef) node, ok := c.definition.NodeByName(fieldDefTypeName) if !ok { - return false + return false, false } if node.Kind == ast.NodeKindUnionTypeDefinition { + if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName) { + return false, true + } + c.addPath(plannerIndex, pathConfiguration{ path: currentPath, shouldWalkFields: true, @@ -776,31 +830,39 @@ func (c *configurationVisitor) addPlannerPathForUnionChildOfObjectParent( enclosingNode: c.walker.EnclosingTypeDefinition, dsHash: c.planners[plannerIndex].dataSourceConfiguration.Hash(), }) - return true + return true, false } - return false + + return false, false } func (c *configurationVisitor) addPlannerPathForChildOfAbstractParent( plannerIndex int, currentPath string, fieldRef int, fieldName string, typeName string, planningBehaviour DataSourcePlanningBehavior, -) (pathAdded bool) { +) (pathAdded bool, halted bool) { if !c.isParentTypeNodeAbstractType() { - return false + return false, false } - if c.addPlannerPathForTypename(plannerIndex, currentPath, fieldRef, fieldName, typeName, planningBehaviour) { - return true + pathAdded, halted = c.addPlannerPathForTypename(plannerIndex, currentPath, fieldRef, fieldName, typeName, planningBehaviour) + switch { + case pathAdded: + return true, false + case halted: + return false, true } // If the field is a root node in any of the data sources, the path shouldn't be handled here // NOTE: previously we were checking all ds, not sure if we need now for _, d := range c.dataSources { if d.HasRootNode(typeName, fieldName) { - return false + return false, false } } - // The path for this field should only be added if the parent path also exists on this planner + + if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName) { + return false, true + } c.addPath(plannerIndex, pathConfiguration{ path: currentPath, @@ -810,7 +872,8 @@ func (c *configurationVisitor) addPlannerPathForChildOfAbstractParent( enclosingNode: c.walker.EnclosingTypeDefinition, dsHash: c.planners[plannerIndex].dataSourceConfiguration.Hash(), }) - return true + + return true, false } // addPlannerPathForTypename adds a path for the __typename field @@ -818,17 +881,21 @@ func (c *configurationVisitor) addPlannerPathForChildOfAbstractParent( // otherwise it will be added to all planners and will cause visiting of incorrect selection sets func (c *configurationVisitor) addPlannerPathForTypename( plannerIndex int, currentPath string, fieldRef int, fieldName string, typeName string, planningBehaviour DataSourcePlanningBehavior, -) (pathAdded bool) { +) (pathAdded bool, halted bool) { if fieldName != typeNameField { - return false + return false, false } if !planningBehaviour.IncludeTypeNameFields { - return false + return false, false } if c.planners[plannerIndex].hasPath(currentPath) { // do not add a path for __typename if it already exists - return true + return true, false + } + + if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName) { + return false, true } c.addPath(plannerIndex, pathConfiguration{ @@ -838,7 +905,7 @@ func (c *configurationVisitor) addPlannerPathForTypename( fieldRef: fieldRef, dsHash: c.planners[plannerIndex].dataSourceConfiguration.Hash(), }) - return true + return true, false } func (c *configurationVisitor) isParentTypeNodeAbstractType() bool { @@ -888,6 +955,29 @@ func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config } } +// couldHandleFieldsRequiredByKey - checks wether we could plan the field now according to it's key requirements +// if no existing planners datasources could provide us with the required fields we should postpone planning of the field +func (c *configurationVisitor) couldHandleFieldsRequiredByKey(dsConfig DataSourceConfiguration, typeName string) bool { + possibleRequiredFields := dsConfig.RequiredFieldsByKey(typeName) + if len(possibleRequiredFields) == 0 { + return true + } + + for i := range c.planners { + if c.planners[i].dataSourceConfiguration.Hash() == dsConfig.Hash() { + return true + } + + for _, possibleRequiredFieldConfig := range possibleRequiredFields { + if c.planners[i].dataSourceConfiguration.HasKeyRequirement(typeName, possibleRequiredFieldConfig.SelectionSet) { + return true + } + } + } + + return false +} + func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, typeName string, possibleRequiredFields []FederationFieldConfiguration) (config FederationFieldConfiguration, planned bool) { if len(possibleRequiredFields) == 0 { return @@ -905,8 +995,6 @@ func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, type } } - // TODO: probably this should be an internal error - in case we haven't found any planner for the required fields - return FederationFieldConfiguration{}, false } @@ -919,6 +1007,7 @@ func (c *configurationVisitor) planAddingRequiredFields(currentPlannerIdx int, p requirements = make([]fieldsRequiredByPlanner, 0, 2) c.pendingRequiredFields[currentSelectionSet] = append(requirements, fieldsRequiredByPlanner{ requestedByPlannerIDs: []int{currentPlannerIdx}, + providedByPlannerID: providedByPlannerIdx, fieldSelections: fieldConfiguration.SelectionSet, }) return @@ -1005,6 +1094,18 @@ func (c *configurationVisitor) addRequiredFields(selectionSetRef int, requiredFi } func (c *configurationVisitor) addNodeSuggestionHint(fieldRef int, plannerIdx int) { + /* + Here we add hints for the node suggestions filter + to be able to select a proper datasource for the key field + It is required to make jumps via diffrerent keys between different subgraphs + */ + + // but we should not add hints for a __typename field cause it collides with + // entity interfaces functionality + if c.operation.FieldNameUnsafeString(fieldRef) == typeNameField { + return + } + dsHash := c.planners[plannerIdx].dataSourceConfiguration.Hash() c.nodeSuggestionHints = append(c.nodeSuggestionHints, NodeSuggestionHint{ diff --git a/v2/pkg/engine/plan/datasource_configuration.go b/v2/pkg/engine/plan/datasource_configuration.go index 654a902dd..b6d09ea3e 100644 --- a/v2/pkg/engine/plan/datasource_configuration.go +++ b/v2/pkg/engine/plan/datasource_configuration.go @@ -38,7 +38,6 @@ type DataSourceConfiguration struct { Custom json.RawMessage FederationMetaData FederationMetaData - RenameTypes TypeConfigurations hash DSHash } diff --git a/v2/pkg/engine/plan/datasource_filter_visitor.go b/v2/pkg/engine/plan/datasource_filter_visitor.go index 1a243c78e..936fa362f 100644 --- a/v2/pkg/engine/plan/datasource_filter_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_visitor.go @@ -393,6 +393,19 @@ func (f *collectNodesVisitor) EnterField(ref int) { hasRootNode := v.HasRootNode(typeName, fieldName) || (isTypeName && v.HasRootNodeWithTypename(typeName)) hasChildNode := v.HasChildNode(typeName, fieldName) || (isTypeName && v.HasChildNodeWithTypename(typeName)) + allowTypeName := true + // we should not select a typename on the interface object + for _, k := range v.FederationMetaData.InterfaceObjects { + if k.InterfaceName == typeName || slices.Contains(k.ImplementingTypeNames, typeName) { + allowTypeName = false + break + } + } + + if !allowTypeName && isTypeName { + continue + } + if hasRootNode || hasChildNode { node := NodeSuggestion{ TypeName: typeName, diff --git a/v2/pkg/engine/plan/federation_metadata.go b/v2/pkg/engine/plan/federation_metadata.go index deca2e231..a8621d8e9 100644 --- a/v2/pkg/engine/plan/federation_metadata.go +++ b/v2/pkg/engine/plan/federation_metadata.go @@ -1,16 +1,22 @@ package plan type FederationMetaData struct { - Keys FederationFieldConfigurations - Requires FederationFieldConfigurations - Provides FederationFieldConfigurations + Keys FederationFieldConfigurations + Requires FederationFieldConfigurations + Provides FederationFieldConfigurations + EntityInterfaces []InterfaceObjectConfiguration + InterfaceObjects []InterfaceObjectConfiguration +} + +type InterfaceObjectConfiguration struct { + InterfaceName string + ImplementingTypeNames []string } type FederationFieldConfiguration struct { - TypeName string - FieldName string - SelectionSet string - InterfaceObjectTypeName string + TypeName string + FieldName string + SelectionSet string } type FederationFieldConfigurations []FederationFieldConfiguration diff --git a/v2/pkg/engine/resolve/node_scalar.go b/v2/pkg/engine/resolve/node_scalar.go index ac465a9fe..9c0d16cbf 100644 --- a/v2/pkg/engine/resolve/node_scalar.go +++ b/v2/pkg/engine/resolve/node_scalar.go @@ -20,7 +20,6 @@ type String struct { Export *FieldExport `json:"export,omitempty"` UnescapeResponseJson bool `json:"unescape_response_json,omitempty"` IsTypeName bool `json:"is_type_name,omitempty"` - StaticValue string `json:"static_value,omitempty"` } func (_ *String) NodeKind() NodeKind { diff --git a/v2/pkg/federation/fixtures/federated_schema.golden b/v2/pkg/federation/fixtures/federated_schema.golden index 3fe499066..7b3443d5f 100644 --- a/v2/pkg/federation/fixtures/federated_schema.golden +++ b/v2/pkg/federation/fixtures/federated_schema.golden @@ -10,6 +10,7 @@ directive @requires(fields: _FieldSet!) on FIELD_DEFINITION directive @provides(fields: _FieldSet!) on FIELD_DEFINITION directive @key(fields: _FieldSet!) on OBJECT | INTERFACE directive @extends on OBJECT | INTERFACE +directive @interfaceObject on OBJECT union _Entity = Product | User From ce87a1fe68a3b51b6d4c79ff8ec0ddd652a03223 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Thu, 4 Jan 2024 18:16:31 +0200 Subject: [PATCH 16/27] fix rewriter test --- v2/pkg/engine/plan/abstract_selection_rewriter_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter_test.go b/v2/pkg/engine/plan/abstract_selection_rewriter_test.go index f1c5c85e1..977a3bd7d 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter_test.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter_test.go @@ -775,13 +775,14 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) { expectedOperation: ` query { iface { - __typename ... on Admin { name + __typename id } ... on User { name + __typename isUser } } From e5d4b61583d357db7b7080681c88de548f909fa6 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Thu, 4 Jan 2024 18:32:39 +0200 Subject: [PATCH 17/27] cleanup and fix tests --- .../graphql_datasource_federation_test.go | 14 -------------- v2/pkg/engine/plan/configuration_visitor.go | 15 --------------- 2 files changed, 29 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 282448a83..d921b9281 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 @@ -1873,13 +1873,6 @@ func TestGraphQLDataSourceFederation(t *testing.T) { thirdDatasourceConfiguration, }, DisableResolveFieldPositions: true, - Debug: plan.DebugConfiguration{ - PrintNodeSuggestions: true, - PrintPlanningPaths: true, - PrintQueryPlans: true, - - ConfigurationVisitor: true, - }, }, )) @@ -1906,13 +1899,6 @@ func TestGraphQLDataSourceFederation(t *testing.T) { firstDatasourceConfiguration, }, DisableResolveFieldPositions: true, - Debug: plan.DebugConfiguration{ - PrintNodeSuggestions: true, - PrintPlanningPaths: true, - PrintQueryPlans: true, - - ConfigurationVisitor: true, - }, }, )) }) diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index 2c1b7e7fd..0f3f10735 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -354,10 +354,6 @@ func (c *configurationVisitor) EnterField(ref int) { } isSubscription := c.isSubscription(root.Ref, currentPath) - if c.secondaryRun && c.pathIsPlannedOnAllPlanners(currentPath) { - return - } - plannerIdx, planned := c.planWithExistingPlanners(ref, typeName, fieldName, currentPath, parentPath, precedingParentPath) if !planned { plannerIdx, planned = c.addNewPlanner(ref, typeName, fieldName, currentPath, parentPath, isSubscription) @@ -478,17 +474,6 @@ func (c *configurationVisitor) handleProvidesSuggestions(ref int, typeName, fiel } } -func (c *configurationVisitor) pathIsPlannedOnAllPlanners(currentPath string) bool { - allPlannersHasPath := true - for _, plannerConfig := range c.planners { - if !plannerConfig.hasPath(currentPath) { - allPlannersHasPath = false - } - } - - return allPlannersHasPath -} - func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, fieldName, currentPath, parentPath, precedingParentPath string) (plannerIdx int, planned bool) { dsHashes := c.nodeSuggestions.SuggestionsForPath(typeName, fieldName, currentPath) From 931f36e1cd26c040a2c37daa02032ad9380bb3ab Mon Sep 17 00:00:00 2001 From: spetrunin Date: Thu, 4 Jan 2024 19:36:09 +0200 Subject: [PATCH 18/27] fix test config --- .../graphql_datasource/entity_interfaces_engine_config.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go index 1a9321957..f0587e2ec 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go +++ b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go @@ -96,8 +96,6 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config TypeName: "Query", FieldNames: []string{"allAccountsInterface", "allAccountsUnion", "user", "admin"}, }, - }, - ChildNodes: []plan.TypeField{ { TypeName: "Account", FieldNames: []string{"id", "title"}, From ed8f4584acfb97079ccd46d8c61b6f5a29a8ac0b Mon Sep 17 00:00:00 2001 From: spetrunin Date: Fri, 5 Jan 2024 12:43:06 +0200 Subject: [PATCH 19/27] always add a typename to an upstream query on interface object --- .../graphql_datasource/graphql_datasource.go | 24 +++++++++---------- ...ource_federation_entity_interfaces_test.go | 5 ++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 662fd0db3..b2ea17c0d 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -526,18 +526,18 @@ func (p *Planner) EnterSelectionSet(ref int) { p.addRepresentationsQuery() } - // if p.visitor.Walker.EnclosingTypeDefinition.Kind.IsAbstractType() { - // // Adding the typename to abstract (unions and interfaces) types is handled elsewhere - // return - // } - - // for _, selectionRef := range p.visitor.Operation.SelectionSets[ref].SelectionRefs { - // if p.visitor.Operation.Selections[selectionRef].Kind == ast.SelectionKindField { - // if p.visitor.Operation.FieldNameUnsafeString(p.visitor.Operation.Selections[selectionRef].Ref) == "__typename" { - // p.addTypenameToSelectionSet(set.Ref) - // } - // } - // } + if p.visitor.Walker.EnclosingTypeDefinition.Kind != ast.NodeKindInterfaceTypeDefinition { + return + } + + // handle adding typename for the InterfaceObject + typeName := p.visitor.Walker.EnclosingTypeDefinition.NameString(p.visitor.Definition) + for _, interfaceObjectCfg := range p.dataSourceConfig.FederationMetaData.InterfaceObjects { + if interfaceObjectCfg.InterfaceName == typeName { + p.addTypenameToSelectionSet(set.Ref) + return + } + } } func (p *Planner) addTypenameToSelectionSet(selectionSet int) { diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go index 463c6b6da..a60de8b29 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go @@ -30,7 +30,7 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {id}}"}}`, + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {__typename id}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, DataSource: &Source{}, }, @@ -257,7 +257,7 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Data: &resolve.Object{ Fetch: &resolve.SingleFetch{ FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {id}}"}}`, + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {__typename id}}"}}`, PostProcessing: DefaultPostProcessingConfiguration, DataSource: &Source{}, }, @@ -398,4 +398,5 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { WithMultiFetchPostProcessor(), )) }) + } From 04b17483fafa47a479b89c6bcc91d376c72b5aa6 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Fri, 5 Jan 2024 13:45:50 +0200 Subject: [PATCH 20/27] fix revisiting planner by fixing detecting missing path fix merging of __typename field --- ...ource_federation_entity_interfaces_test.go | 167 ++++++++++++++++++ v2/pkg/engine/plan/configuration_visitor.go | 31 +++- v2/pkg/engine/plan/visitor.go | 28 ++- 3 files changed, 205 insertions(+), 21 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go index a60de8b29..4d399561b 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go @@ -399,4 +399,171 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { )) }) + t.Run("query 10.1 - InterfaceObjectToConcreteTypeWithExternalField - no fragments, with typename", func(t *testing.T) { + t.Run("run", RunTest( + definition, + ` + query _10_InterfaceObjectToConcreteTypeWithExternalField_No_Fragments { + accountLocations { + id + title + __typename + } + }`, + "_10_InterfaceObjectToConcreteTypeWithExternalField_No_Fragments", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {__typename id}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + DataSource: &Source{}, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + Fields: []*resolve.Field{ + { + Name: []byte("accountLocations"), + Value: &resolve.Array{ + Path: []string{"accountLocations"}, + Item: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("id"), + Value: &resolve.Scalar{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + { + Name: []byte("title"), + Value: &resolve.String{ + Path: []string{"title"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + IsTypeName: true, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + }, + Fetch: &resolve.SerialFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{ + FetchID: 2, + DependsOnFetchIDs: []int{0}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4001/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {__typename} ... on Moderator {title __typename} ... on User {title __typename}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + &resolve.SingleFetch{ + FetchID: 1, + DependsOnFetchIDs: []int{0, 2}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4003/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {title}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + planConfiguration, + WithMultiFetchPostProcessor(), + )) + }) + } diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index 0f3f10735..a6eb10d71 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -3,6 +3,7 @@ package plan import ( "context" "fmt" + "slices" "strings" "github.com/pkg/errors" @@ -178,18 +179,22 @@ func (c *configurationVisitor) findPreviousRootPath(currentPath string) (previou } for i := len(c.addedPathTracker) - 1; i >= 0; i-- { - if strings.HasPrefix(currentPath, c.addedPathTracker[i].path) && c.addedPathTracker[i].isRootNode { + hasMatch := + strings.HasPrefix(currentPath, c.addedPathTracker[i].path) && + currentPath != c.addedPathTracker[i].path && + c.addedPathTracker[i].isRootNode + + if hasMatch { return c.addedPathTracker[i].path, true } } return "", false } -func (c *configurationVisitor) addMissingPath(path string, parentPath string, hash DSHash) { +func (c *configurationVisitor) addMissingPath(path string, parentPath string) { c.missingPathTracker[path] = missingPath{ path: path, precedingRootNodePath: parentPath, - dsHash: hash, } } @@ -748,11 +753,25 @@ func (c *configurationVisitor) addNewPlanner(ref int, typeName, fieldName, curre // handleMissingPath - records missing path for the case when we don't yet have a planner for the field func (c *configurationVisitor) handleMissingPath(typeName string, fieldName string, currentPath string) { - suggestedDataSourceHash, ok := c.nodeSuggestions.HasSuggestionForPath(typeName, fieldName, currentPath) - if ok { + suggestedDataSourceHashes := c.nodeSuggestions.SuggestionsForPath(typeName, fieldName, currentPath) + if len(suggestedDataSourceHashes) > 0 { parentPath, found := c.findPreviousRootPath(currentPath) if found { - c.addMissingPath(currentPath, parentPath, suggestedDataSourceHash) + c.addMissingPath(currentPath, parentPath) + return + } + + allPlannersHasPath := true + for i, _ := range c.planners { + if slices.Contains(suggestedDataSourceHashes, c.planners[i].dataSourceConfiguration.Hash()) { + if !c.planners[i].hasPath(currentPath) { + allPlannersHasPath = false + break + } + } + } + + if allPlannersHasPath { return } } diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index c46f97b73..e16da884a 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -297,21 +297,19 @@ func (v *Visitor) EnterField(ref int) { IncludeVariableName: skipIncludeInfo.includeVariableName, Info: v.resolveFieldInfo(ref, fieldDefinitionTypeRef, onTypeNames), } - *v.currentFields[len(v.currentFields)-1].fields = append(*v.currentFields[len(v.currentFields)-1].fields, v.currentField) - return - } - - path := v.resolveFieldPath(ref) - v.currentField = &resolve.Field{ - Name: fieldAliasOrName, - Value: v.resolveFieldValue(ref, fieldDefinitionTypeRef, true, path), - OnTypeNames: onTypeNames, - Position: v.resolveFieldPosition(ref), - SkipDirectiveDefined: skipIncludeInfo.skip, - SkipVariableName: skipIncludeInfo.skipVariableName, - IncludeDirectiveDefined: skipIncludeInfo.include, - IncludeVariableName: skipIncludeInfo.includeVariableName, - Info: v.resolveFieldInfo(ref, fieldDefinitionTypeRef, onTypeNames), + } else { + path := v.resolveFieldPath(ref) + v.currentField = &resolve.Field{ + Name: fieldAliasOrName, + Value: v.resolveFieldValue(ref, fieldDefinitionTypeRef, true, path), + OnTypeNames: onTypeNames, + Position: v.resolveFieldPosition(ref), + SkipDirectiveDefined: skipIncludeInfo.skip, + SkipVariableName: skipIncludeInfo.skipVariableName, + IncludeDirectiveDefined: skipIncludeInfo.include, + IncludeVariableName: skipIncludeInfo.includeVariableName, + Info: v.resolveFieldInfo(ref, fieldDefinitionTypeRef, onTypeNames), + } } // append the field to the current object From f8ada0b7ec75a3406945f6e04ee8860725fd0cc1 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Fri, 5 Jan 2024 15:17:17 +0200 Subject: [PATCH 21/27] fix args config --- .../entity_interfaces_engine_config.go | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go index f0587e2ec..d5f5e0047 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go +++ b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go @@ -330,6 +330,28 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config planConfiguration := plan.Configuration{ DataSources: dataSources, DisableResolveFieldPositions: true, + Fields: []plan.FieldConfiguration{ + { + TypeName: "Query", + FieldName: "user", + Arguments: []plan.ArgumentConfiguration{ + { + Name: "id", + SourceType: plan.FieldArgumentSource, + }, + }, + }, + { + TypeName: "Query", + FieldName: "admin", + Arguments: []plan.ArgumentConfiguration{ + { + Name: "id", + SourceType: plan.FieldArgumentSource, + }, + }, + }, + }, Debug: plan.DebugConfiguration{ PrintOperationTransformations: true, PrintQueryPlans: true, From 9efed92714e0e8806c0b295e8f4ea275e34ab075 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Fri, 5 Jan 2024 16:54:02 +0200 Subject: [PATCH 22/27] fix interface object to interface objects jumps --- ...ource_federation_entity_interfaces_test.go | 486 +++++++++++++++++- v2/pkg/engine/plan/configuration_visitor.go | 38 +- v2/pkg/engine/plan/visitor.go | 34 +- 3 files changed, 539 insertions(+), 19 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go index 4d399561b..9d1b43544 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go @@ -160,7 +160,7 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { }, }, }, - OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account"), []byte("Moderator"), []byte("User")}, }, }, Fetch: &resolve.SingleFetch{ @@ -241,6 +241,172 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { }) + t.Run("query 9.1 - Interface fragment on union to interface objects", func(t *testing.T) { + t.Skip("it works, but the test is broken") + + t.Run("run", RunTest( + definition, + ` + query _9_1_InterfaceFragmentOnUnionToInterfaceObjects { + allAccountsUnion { + ... on Account { + id + title + locations { + country + } + age + } + } + }`, + "_9_1_InterfaceFragmentOnUnionToInterfaceObjects", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {__typename id}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + DataSource: &Source{}, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + Fields: []*resolve.Field{ + { + Name: []byte("accountLocations"), + Value: &resolve.Array{ + Path: []string{"accountLocations"}, + Item: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("id"), + Value: &resolve.Scalar{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + { + Name: []byte("title"), + Value: &resolve.String{ + Path: []string{"title"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + }, + Fetch: &resolve.SerialFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{ + FetchID: 2, + DependsOnFetchIDs: []int{0}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4001/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {__typename} ... on Moderator {title __typename} ... on User {title __typename}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + &resolve.SingleFetch{ + FetchID: 1, + DependsOnFetchIDs: []int{0, 2}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4003/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {title}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + planConfiguration, + WithMultiFetchPostProcessor(), + )) + }) + t.Run("query 10 - InterfaceObjectToConcreteTypeWithExternalField - no fragments", func(t *testing.T) { t.Run("run", RunTest( definition, @@ -275,7 +441,7 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Value: &resolve.Scalar{ Path: []string{"id"}, }, - OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account"), []byte("Moderator"), []byte("User")}, }, { Name: []byte("title"), @@ -434,7 +600,7 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Value: &resolve.Scalar{ Path: []string{"id"}, }, - OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account"), []byte("Moderator"), []byte("User")}, }, { Name: []byte("title"), @@ -464,6 +630,20 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ Nullable: true, Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, { Name: []byte("__typename"), Value: &resolve.String{ @@ -492,11 +672,243 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { }, OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + &resolve.SingleFetch{ + FetchID: 1, + DependsOnFetchIDs: []int{0, 2}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4003/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {title}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ { Name: []byte("__typename"), Value: &resolve.String{ Path: []string{"__typename"}, }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + planConfiguration, + WithMultiFetchPostProcessor(), + )) + }) + + t.Run("query 14 Interface object to Interface object", func(t *testing.T) { + t.Run("run", RunTest( + definition, + ` + query _14_InterfaceObjectToInterfaceObject { + accountLocations { + age + } + }`, + "_14_InterfaceObjectToInterfaceObject", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {__typename id}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + DataSource: &Source{}, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + Fields: []*resolve.Field{ + { + Name: []byte("accountLocations"), + Value: &resolve.Array{ + Path: []string{"accountLocations"}, + Item: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("age"), + Value: &resolve.Integer{ + Path: []string{"age"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account"), []byte("Moderator"), []byte("User")}, + }, + }, + Fetch: &resolve.SingleFetch{ + FetchID: 1, + DependsOnFetchIDs: []int{0}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4004/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Account {age}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + }, + }), + }, + }, + RequiresEntityBatchFetch: true, + PostProcessing: EntitiesPostProcessingConfiguration, + DataSource: &Source{}, + SetTemplateOutputToNullOnVariableNull: true, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + }, + }, + }, + }, + }, + }, + }, + planConfiguration, + WithMultiFetchPostProcessor(), + )) + }) + + t.Run("query 14 Interface object to Interface object with typename", func(t *testing.T) { + t.Run("run", RunTest( + definition, + ` + query _14_InterfaceObjectToInterfaceObject { + accountLocations { + age + __typename + } + }`, + "_14_InterfaceObjectToInterfaceObject", + &plan.SynchronousResponsePlan{ + Response: &resolve.GraphQLResponse{ + Data: &resolve.Object{ + Fetch: &resolve.SingleFetch{ + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"{accountLocations {__typename id}}"}}`, + PostProcessing: DefaultPostProcessingConfiguration, + DataSource: &Source{}, + }, + DataSourceIdentifier: []byte("graphql_datasource.Source"), + }, + Fields: []*resolve.Field{ + { + Name: []byte("accountLocations"), + Value: &resolve.Array{ + Path: []string{"accountLocations"}, + Item: &resolve.Object{ + Fields: []*resolve.Field{ + { + Name: []byte("age"), + Value: &resolve.Integer{ + Path: []string{"age"}, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account"), []byte("Moderator"), []byte("User")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + IsTypeName: true, + }, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Moderator"), []byte("User")}, + }, + }, + Fetch: &resolve.ParallelFetch{ + Fetches: []resolve.Fetch{ + &resolve.SingleFetch{ + FetchID: 1, + DependsOnFetchIDs: []int{0}, + FetchConfiguration: resolve.FetchConfiguration{ + Input: `{"method":"POST","url":"http://localhost:4004/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Account {age}}}","variables":{"representations":[$$0$$]}}}`, + Variables: []resolve.Variable{ + &resolve.ResolvableObjectVariable{ + Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ + Nullable: true, + Fields: []*resolve.Field{ + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, }, { @@ -506,6 +918,36 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { }, OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, }, + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.StaticString{ + Path: []string{"__typename"}, + Value: "Account", + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, }, }), }, @@ -518,10 +960,10 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { DataSourceIdentifier: []byte("graphql_datasource.Source"), }, &resolve.SingleFetch{ - FetchID: 1, - DependsOnFetchIDs: []int{0, 2}, + FetchID: 2, + DependsOnFetchIDs: []int{0}, FetchConfiguration: resolve.FetchConfiguration{ - Input: `{"method":"POST","url":"http://localhost:4003/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {title}}}","variables":{"representations":[$$0$$]}}}`, + Input: `{"method":"POST","url":"http://localhost:4001/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){__typename ... on Admin {__typename} ... on Moderator {__typename} ... on User {__typename}}}","variables":{"representations":[$$0$$]}}}`, Variables: []resolve.Variable{ &resolve.ResolvableObjectVariable{ Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{ @@ -532,14 +974,42 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) { Value: &resolve.String{ Path: []string{"__typename"}, }, - OnTypeNames: [][]byte{[]byte("Admin")}, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, }, { Name: []byte("id"), Value: &resolve.String{ Path: []string{"id"}, }, - OnTypeNames: [][]byte{[]byte("Admin")}, + OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")}, + }, + { + Name: []byte("__typename"), + Value: &resolve.String{ + Path: []string{"__typename"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, + }, + { + Name: []byte("id"), + Value: &resolve.String{ + Path: []string{"id"}, + }, + OnTypeNames: [][]byte{[]byte("User"), []byte("Account")}, }, }, }), diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index a6eb10d71..111536578 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -49,6 +49,7 @@ type fieldsRequiredByPlanner struct { fieldSelections string requestedByPlannerIDs []int providedByPlannerID int + skipTypename bool } type arrayField struct { @@ -628,7 +629,11 @@ func (c *configurationVisitor) addNewPlanner(ref int, typeName, fieldName, curre isEntityInterface := false for _, interfaceObjCfg := range config.FederationMetaData.EntityInterfaces { - if interfaceObjCfg.InterfaceName == typeName { + hasMatch := + interfaceObjCfg.InterfaceName == typeName || + slices.Contains(interfaceObjCfg.ImplementingTypeNames, typeName) + + if hasMatch { isEntityInterface = true break } @@ -936,7 +941,7 @@ func (c *configurationVisitor) handleFieldRequiredByRequires(plannerIdx int, con requiredFieldsForTypeAndField := config.dataSourceConfiguration.RequiredFieldsByRequires(typeName, fieldName) for _, requiredFieldsConfiguration := range requiredFieldsForTypeAndField { - c.planAddingRequiredFields(plannerIdx, -1, requiredFieldsConfiguration) + c.planAddingRequiredFields(plannerIdx, -1, requiredFieldsConfiguration, false) var added bool config.requiredFields, added = appendRequiredFieldsConfigurationIfNotPresent(config.requiredFields, requiredFieldsConfiguration) if added { @@ -948,7 +953,15 @@ func (c *configurationVisitor) handleFieldRequiredByRequires(plannerIdx int, con func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config *plannerConfiguration, typeName string) { requiredFieldsForType := config.dataSourceConfiguration.RequiredFieldsByKey(typeName) if len(requiredFieldsForType) > 0 { - requiredFieldsConfiguration, planned := c.planKeyRequiredFields(plannerIdx, typeName, requiredFieldsForType) + isInterfaceObject := false + for _, interfaceObjCfg := range config.dataSourceConfiguration.FederationMetaData.InterfaceObjects { + if slices.Contains(interfaceObjCfg.ImplementingTypeNames, typeName) { + isInterfaceObject = true + break + } + } + + requiredFieldsConfiguration, planned := c.planKeyRequiredFields(plannerIdx, typeName, requiredFieldsForType, isInterfaceObject) if planned { var added bool config.requiredFields, added = appendRequiredFieldsConfigurationIfNotPresent(config.requiredFields, requiredFieldsConfiguration) @@ -982,7 +995,7 @@ func (c *configurationVisitor) couldHandleFieldsRequiredByKey(dsConfig DataSourc return false } -func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, typeName string, possibleRequiredFields []FederationFieldConfiguration) (config FederationFieldConfiguration, planned bool) { +func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, typeName string, possibleRequiredFields []FederationFieldConfiguration, forInterfaceObject bool) (config FederationFieldConfiguration, planned bool) { if len(possibleRequiredFields) == 0 { return } @@ -993,7 +1006,17 @@ func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, type } for _, possibleRequiredFieldConfig := range possibleRequiredFields { if c.planners[i].dataSourceConfiguration.HasKeyRequirement(typeName, possibleRequiredFieldConfig.SelectionSet) { - c.planAddingRequiredFields(currentPlannerIdx, i, possibleRequiredFieldConfig) + + isInterfaceObject := false + for _, interfaceObjCfg := range c.planners[i].dataSourceConfiguration.FederationMetaData.InterfaceObjects { + if slices.Contains(interfaceObjCfg.ImplementingTypeNames, typeName) { + isInterfaceObject = true + break + } + } + skipTypename := forInterfaceObject && isInterfaceObject + + c.planAddingRequiredFields(currentPlannerIdx, i, possibleRequiredFieldConfig, skipTypename) return possibleRequiredFieldConfig, true } } @@ -1002,7 +1025,7 @@ func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, type return FederationFieldConfiguration{}, false } -func (c *configurationVisitor) planAddingRequiredFields(currentPlannerIdx int, providedByPlannerIdx int, fieldConfiguration FederationFieldConfiguration) { +func (c *configurationVisitor) planAddingRequiredFields(currentPlannerIdx int, providedByPlannerIdx int, fieldConfiguration FederationFieldConfiguration, skipTypename bool) { currentSelectionSet := c.currentSelectionSet() requirements, hasRequirements := c.pendingRequiredFields[currentSelectionSet] @@ -1013,6 +1036,7 @@ func (c *configurationVisitor) planAddingRequiredFields(currentPlannerIdx int, p requestedByPlannerIDs: []int{currentPlannerIdx}, providedByPlannerID: providedByPlannerIdx, fieldSelections: fieldConfiguration.SelectionSet, + skipTypename: skipTypename, }) return } @@ -1066,7 +1090,7 @@ func (c *configurationVisitor) processPendingRequiredFields(selectionSetRef int) func (c *configurationVisitor) addRequiredFields(selectionSetRef int, requiredFieldsCfg fieldsRequiredByPlanner) { typeName := c.walker.EnclosingTypeDefinition.NameString(c.definition) - key, report := RequiredFieldsFragment(typeName, requiredFieldsCfg.fieldSelections, true) + key, report := RequiredFieldsFragment(typeName, requiredFieldsCfg.fieldSelections, !requiredFieldsCfg.skipTypename) if report.HasErrors() { c.walker.StopWithInternalErr(fmt.Errorf("failed to parse required fields for %s", typeName)) } diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index e16da884a..18295b684 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -279,7 +279,7 @@ func (v *Visitor) EnterField(ref int) { skipIncludeInfo := v.resolveSkipIncludeForField(ref) - onTypeNames := v.resolveOnTypeNames() + onTypeNames := v.resolveOnTypeNames(ref) if bytes.Equal(fieldName, literal.TYPENAME) { v.currentField = &resolve.Field{ @@ -328,7 +328,7 @@ func (v *Visitor) handleExistingField(currentFieldRef int, fieldDefinitionTypeRe } // merge on type names - onTypeNames := v.resolveOnTypeNames() + onTypeNames := v.resolveOnTypeNames(currentFieldRef) hasOnTypeNames := len(resolveField.OnTypeNames) > 0 && len(onTypeNames) > 0 if hasOnTypeNames { for _, t := range onTypeNames { @@ -494,7 +494,7 @@ func (v *Visitor) resolveSkipIncludeForField(fieldRef int) skipIncludeInfo { return skipIncludeInfo{} } -func (v *Visitor) resolveOnTypeNames() [][]byte { +func (v *Visitor) resolveOnTypeNames(fieldRef int) [][]byte { if len(v.Walker.Ancestors) < 2 { return nil } @@ -509,7 +509,7 @@ func (v *Visitor) resolveOnTypeNames() [][]byte { node, exists := v.Definition.NodeByName(typeName) // If not an interface, return the concrete type if !exists || !node.Kind.IsAbstractType() { - return [][]byte{v.Config.Types.RenameTypeNameOnMatchBytes(typeName)} + return v.addInterfaceObjectNameToTypeNames(fieldRef, typeName, [][]byte{v.Config.Types.RenameTypeNameOnMatchBytes(typeName)}) } if node.Kind == ast.NodeKindUnionTypeDefinition { // This should never be true. If it is, it's an error @@ -525,6 +525,32 @@ func (v *Visitor) resolveOnTypeNames() [][]byte { if len(onTypeNames) < 1 { return nil } + + return onTypeNames +} + +func (v *Visitor) addInterfaceObjectNameToTypeNames(fieldRef int, typeName []byte, onTypeNames [][]byte) [][]byte { + includeInterfaceObjectName := false + var interfaceObjectName string + for i, _ := range v.planners { + if !slices.ContainsFunc(v.planners[i].paths, func(path pathConfiguration) bool { + return path.fieldRef == fieldRef + }) { + continue + } + + for _, interfaceObjCfg := range v.planners[i].dataSourceConfiguration.FederationMetaData.InterfaceObjects { + if slices.Contains(interfaceObjCfg.ImplementingTypeNames, string(typeName)) { + includeInterfaceObjectName = true + interfaceObjectName = interfaceObjCfg.InterfaceName + break + } + } + } + if includeInterfaceObjectName { + onTypeNames = append(onTypeNames, []byte(interfaceObjectName)) + } + return onTypeNames } From 4a7354b1bd8c92bcfcda37892feb9fac6ea613b1 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Mon, 8 Jan 2024 13:52:07 +0200 Subject: [PATCH 23/27] align configuration naming --- .../entity_interfaces_engine_config.go | 18 +++++++++--------- .../graphql_datasource/graphql_datasource.go | 6 +++--- .../representation_variable.go | 8 ++++---- .../representation_variable_test.go | 6 +++--- .../engine/plan/abstract_selection_rewriter.go | 4 ++-- v2/pkg/engine/plan/configuration_visitor.go | 8 ++++---- .../engine/plan/datasource_filter_visitor.go | 2 +- v2/pkg/engine/plan/federation_metadata.go | 10 +++++----- v2/pkg/engine/plan/visitor.go | 4 ++-- 9 files changed, 33 insertions(+), 33 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go index d5f5e0047..08693325a 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go +++ b/v2/pkg/engine/datasource/graphql_datasource/entity_interfaces_engine_config.go @@ -113,10 +113,10 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config }), Factory: factory, FederationMetaData: plan.FederationMetaData{ - EntityInterfaces: []plan.InterfaceObjectConfiguration{ + EntityInterfaces: []plan.EntityInterfaceConfiguration{ { - InterfaceName: "Account", - ImplementingTypeNames: []string{"Admin", "Moderator", "User"}, + InterfaceTypeName: "Account", + ConcreteTypeNames: []string{"Admin", "Moderator", "User"}, }, }, Keys: plan.FederationFieldConfigurations{ @@ -195,10 +195,10 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config }), Factory: factory, FederationMetaData: plan.FederationMetaData{ - InterfaceObjects: []plan.InterfaceObjectConfiguration{ + InterfaceObjects: []plan.EntityInterfaceConfiguration{ { - InterfaceName: "Account", - ImplementingTypeNames: []string{"Admin", "Moderator", "User"}, + InterfaceTypeName: "Account", + ConcreteTypeNames: []string{"Admin", "Moderator", "User"}, }, }, Keys: plan.FederationFieldConfigurations{ @@ -293,10 +293,10 @@ func EntityInterfacesPlanConfiguration(factory plan.PlannerFactory) *plan.Config }), Factory: factory, FederationMetaData: plan.FederationMetaData{ - InterfaceObjects: []plan.InterfaceObjectConfiguration{ + InterfaceObjects: []plan.EntityInterfaceConfiguration{ { - InterfaceName: "Account", - ImplementingTypeNames: []string{"Admin", "Moderator", "User"}, + InterfaceTypeName: "Account", + ConcreteTypeNames: []string{"Admin", "Moderator", "User"}, }, }, Keys: plan.FederationFieldConfigurations{ diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index b2ea17c0d..b6ce6a636 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -533,7 +533,7 @@ func (p *Planner) EnterSelectionSet(ref int) { // handle adding typename for the InterfaceObject typeName := p.visitor.Walker.EnclosingTypeDefinition.NameString(p.visitor.Definition) for _, interfaceObjectCfg := range p.dataSourceConfig.FederationMetaData.InterfaceObjects { - if interfaceObjectCfg.InterfaceName == typeName { + if interfaceObjectCfg.InterfaceTypeName == typeName { p.addTypenameToSelectionSet(set.Ref) return } @@ -921,8 +921,8 @@ func (p *Planner) isInEntitiesSelectionSet() bool { func (p *Planner) interfaceObjectTypeShouldBeRenamed(typeName string) (ok bool, newName string) { for _, interfaceObjectCfg := range p.dataSourceConfig.FederationMetaData.InterfaceObjects { - if slices.Contains(interfaceObjectCfg.ImplementingTypeNames, typeName) { - return true, interfaceObjectCfg.InterfaceName + if slices.Contains(interfaceObjectCfg.ConcreteTypeNames, typeName) { + return true, interfaceObjectCfg.InterfaceTypeName } } return false, "" diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go index 9d577b3be..8b68126b8 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go @@ -26,15 +26,15 @@ func buildRepresentationVariableNode(definition *ast.Document, cfg plan.Federati var interfaceObjectTypeName *string for _, interfaceObjCfg := range dsCfg.FederationMetaData.InterfaceObjects { - if slices.Contains(interfaceObjCfg.ImplementingTypeNames, cfg.TypeName) { - interfaceObjectTypeName = &interfaceObjCfg.InterfaceName + if slices.Contains(interfaceObjCfg.ConcreteTypeNames, cfg.TypeName) { + interfaceObjectTypeName = &interfaceObjCfg.InterfaceTypeName break } } var entityInterfaceTypeName *string for _, entityInterfaceCfg := range dsCfg.FederationMetaData.EntityInterfaces { - if slices.Contains(entityInterfaceCfg.ImplementingTypeNames, cfg.TypeName) { - entityInterfaceTypeName = &entityInterfaceCfg.InterfaceName + if slices.Contains(entityInterfaceCfg.ConcreteTypeNames, cfg.TypeName) { + entityInterfaceTypeName = &entityInterfaceCfg.InterfaceTypeName break } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable_test.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable_test.go index ee15f3d72..66e4929fb 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable_test.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable_test.go @@ -75,10 +75,10 @@ func TestBuildRepresentationVariableNode(t *testing.T) { `id name`, plan.DataSourceConfiguration{ FederationMetaData: plan.FederationMetaData{ - InterfaceObjects: []plan.InterfaceObjectConfiguration{ + InterfaceObjects: []plan.EntityInterfaceConfiguration{ { - InterfaceName: "Account", - ImplementingTypeNames: []string{"User", "Admin"}, + InterfaceTypeName: "Account", + ConcreteTypeNames: []string{"User", "Admin"}, }, }, }, diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter.go b/v2/pkg/engine/plan/abstract_selection_rewriter.go index 92732c932..85716aaf4 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter.go @@ -252,9 +252,9 @@ func (r *fieldSelectionRewriter) processInterfaceSelection(fieldRef int, interfa if node.Kind != ast.NodeKindInterfaceTypeDefinition { interfaceTypeNameStr := string(interfaceTypeName) for _, k := range r.dsConfiguration.FederationMetaData.InterfaceObjects { - if k.InterfaceName == interfaceTypeNameStr { + if k.InterfaceTypeName == interfaceTypeNameStr { isInterfaceObject = true - interfaceTypeNames = k.ImplementingTypeNames + interfaceTypeNames = k.ConcreteTypeNames break } } diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index 111536578..4654121c7 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -630,8 +630,8 @@ func (c *configurationVisitor) addNewPlanner(ref int, typeName, fieldName, curre isEntityInterface := false for _, interfaceObjCfg := range config.FederationMetaData.EntityInterfaces { hasMatch := - interfaceObjCfg.InterfaceName == typeName || - slices.Contains(interfaceObjCfg.ImplementingTypeNames, typeName) + interfaceObjCfg.InterfaceTypeName == typeName || + slices.Contains(interfaceObjCfg.ConcreteTypeNames, typeName) if hasMatch { isEntityInterface = true @@ -955,7 +955,7 @@ func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config if len(requiredFieldsForType) > 0 { isInterfaceObject := false for _, interfaceObjCfg := range config.dataSourceConfiguration.FederationMetaData.InterfaceObjects { - if slices.Contains(interfaceObjCfg.ImplementingTypeNames, typeName) { + if slices.Contains(interfaceObjCfg.ConcreteTypeNames, typeName) { isInterfaceObject = true break } @@ -1009,7 +1009,7 @@ func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, type isInterfaceObject := false for _, interfaceObjCfg := range c.planners[i].dataSourceConfiguration.FederationMetaData.InterfaceObjects { - if slices.Contains(interfaceObjCfg.ImplementingTypeNames, typeName) { + if slices.Contains(interfaceObjCfg.ConcreteTypeNames, typeName) { isInterfaceObject = true break } diff --git a/v2/pkg/engine/plan/datasource_filter_visitor.go b/v2/pkg/engine/plan/datasource_filter_visitor.go index 936fa362f..2fcc7c948 100644 --- a/v2/pkg/engine/plan/datasource_filter_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_visitor.go @@ -396,7 +396,7 @@ func (f *collectNodesVisitor) EnterField(ref int) { allowTypeName := true // we should not select a typename on the interface object for _, k := range v.FederationMetaData.InterfaceObjects { - if k.InterfaceName == typeName || slices.Contains(k.ImplementingTypeNames, typeName) { + if k.InterfaceTypeName == typeName || slices.Contains(k.ConcreteTypeNames, typeName) { allowTypeName = false break } diff --git a/v2/pkg/engine/plan/federation_metadata.go b/v2/pkg/engine/plan/federation_metadata.go index a8621d8e9..790b3dfe0 100644 --- a/v2/pkg/engine/plan/federation_metadata.go +++ b/v2/pkg/engine/plan/federation_metadata.go @@ -4,13 +4,13 @@ type FederationMetaData struct { Keys FederationFieldConfigurations Requires FederationFieldConfigurations Provides FederationFieldConfigurations - EntityInterfaces []InterfaceObjectConfiguration - InterfaceObjects []InterfaceObjectConfiguration + EntityInterfaces []EntityInterfaceConfiguration + InterfaceObjects []EntityInterfaceConfiguration } -type InterfaceObjectConfiguration struct { - InterfaceName string - ImplementingTypeNames []string +type EntityInterfaceConfiguration struct { + InterfaceTypeName string + ConcreteTypeNames []string } type FederationFieldConfiguration struct { diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index 18295b684..d0f0d2003 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -540,9 +540,9 @@ func (v *Visitor) addInterfaceObjectNameToTypeNames(fieldRef int, typeName []byt } for _, interfaceObjCfg := range v.planners[i].dataSourceConfiguration.FederationMetaData.InterfaceObjects { - if slices.Contains(interfaceObjCfg.ImplementingTypeNames, string(typeName)) { + if slices.Contains(interfaceObjCfg.ConcreteTypeNames, string(typeName)) { includeInterfaceObjectName = true - interfaceObjectName = interfaceObjCfg.InterfaceName + interfaceObjectName = interfaceObjCfg.InterfaceTypeName break } } From 6ab2f6a63fe08ffe277ddfaabdb3417414534030 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Mon, 8 Jan 2024 20:18:50 +0200 Subject: [PATCH 24/27] fix handling __typename on a union fix matching planner by keys - add added path check --- v2/pkg/engine/plan/configuration_visitor.go | 42 ++++++++++++++----- .../engine/plan/datasource_filter_visitor.go | 3 ++ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index 4654121c7..d01182590 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -408,7 +408,7 @@ func (c *configurationVisitor) handleRequirements(plannerIdx int, parentPath str parentDSHash, ok := c.addedPathDSHash(parentPath) if ok && dsHash != parentDSHash { // add required fields for type (@key) - c.handleFieldsRequiredByKey(plannerIdx, plannerConfig, typeName) + c.handleFieldsRequiredByKey(plannerIdx, plannerConfig, typeName, parentPath) } // add required fields for field and type (@requires) @@ -496,11 +496,19 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field } } - // NOTE: revisit this logic // On a union we will never get a node suggestion because union type is not in the root or child nodes - if !hasSuggestion && plannerConfig.hasPath(parentPath) && fieldName == typeNameField { - if pathAdded := c.addPlannerPathForTypename(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour); pathAdded { + shouldHandleTypeNameOnUnion := + !hasSuggestion && fieldName == typeNameField && + c.walker.EnclosingTypeDefinition.Kind == ast.NodeKindUnionTypeDefinition && + plannerConfig.hasPath(parentPath) + + if shouldHandleTypeNameOnUnion { + pathAdded, halted := c.addPlannerPathForTypename(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour) + switch { + case pathAdded: return plannerIdx, true + case halted: + return -1, false } } @@ -950,7 +958,7 @@ func (c *configurationVisitor) handleFieldRequiredByRequires(plannerIdx int, con } } -func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config *plannerConfiguration, typeName string) { +func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config *plannerConfiguration, typeName string, parentPath string) { requiredFieldsForType := config.dataSourceConfiguration.RequiredFieldsByKey(typeName) if len(requiredFieldsForType) > 0 { isInterfaceObject := false @@ -961,7 +969,7 @@ func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config } } - requiredFieldsConfiguration, planned := c.planKeyRequiredFields(plannerIdx, typeName, requiredFieldsForType, isInterfaceObject) + requiredFieldsConfiguration, planned := c.planKeyRequiredFields(plannerIdx, typeName, parentPath, requiredFieldsForType, isInterfaceObject) if planned { var added bool config.requiredFields, added = appendRequiredFieldsConfigurationIfNotPresent(config.requiredFields, requiredFieldsConfiguration) @@ -981,6 +989,11 @@ func (c *configurationVisitor) couldHandleFieldsRequiredByKey(dsConfig DataSourc } for i := range c.planners { + // TODO: here + // if !c.planners[i].hasPath(parentPath) { + // continue + // } + if c.planners[i].dataSourceConfiguration.Hash() == dsConfig.Hash() { return true } @@ -995,7 +1008,7 @@ func (c *configurationVisitor) couldHandleFieldsRequiredByKey(dsConfig DataSourc return false } -func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, typeName string, possibleRequiredFields []FederationFieldConfiguration, forInterfaceObject bool) (config FederationFieldConfiguration, planned bool) { +func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, typeName string, parentPath string, possibleRequiredFields []FederationFieldConfiguration, forInterfaceObject bool) (config FederationFieldConfiguration, planned bool) { if len(possibleRequiredFields) == 0 { return } @@ -1004,6 +1017,11 @@ func (c *configurationVisitor) planKeyRequiredFields(currentPlannerIdx int, type if i == currentPlannerIdx { continue // skip current planner } + // we need to filter out the planners which do not have such parent path + // because we could have a planner with mathing datasource but on the wrong path + if !c.planners[i].hasPath(parentPath) { + continue + } for _, possibleRequiredFieldConfig := range possibleRequiredFields { if c.planners[i].dataSourceConfiguration.HasKeyRequirement(typeName, possibleRequiredFieldConfig.SelectionSet) { @@ -1130,15 +1148,19 @@ func (c *configurationVisitor) addNodeSuggestionHint(fieldRef int, plannerIdx in // but we should not add hints for a __typename field cause it collides with // entity interfaces functionality - if c.operation.FieldNameUnsafeString(fieldRef) == typeNameField { + fieldName := c.operation.FieldNameString(fieldRef) + if fieldName == typeNameField { return } dsHash := c.planners[plannerIdx].dataSourceConfiguration.Hash() + parentPath := c.walker.Path.DotDelimitedString() c.nodeSuggestionHints = append(c.nodeSuggestionHints, NodeSuggestionHint{ - fieldRef: fieldRef, - dsHash: dsHash, + fieldRef: fieldRef, + dsHash: dsHash, + fieldName: fieldName, + parentPath: parentPath, }) } diff --git a/v2/pkg/engine/plan/datasource_filter_visitor.go b/v2/pkg/engine/plan/datasource_filter_visitor.go index 2fcc7c948..e5aa1bc3d 100644 --- a/v2/pkg/engine/plan/datasource_filter_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_visitor.go @@ -145,6 +145,9 @@ func (n *NodeSuggestion) String() string { type NodeSuggestionHint struct { fieldRef int dsHash DSHash + + fieldName string + parentPath string } type NodeSuggestions []NodeSuggestion From 7e94223914e7601d0ba3ee5f94226d8416fef391 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Mon, 8 Jan 2024 21:00:58 +0200 Subject: [PATCH 25/27] add a check for presence of parent path for checking could handle requirements --- v2/pkg/engine/plan/configuration_visitor.go | 38 ++++++++++----------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index d01182590..8f0c45a3f 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -503,7 +503,7 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field plannerConfig.hasPath(parentPath) if shouldHandleTypeNameOnUnion { - pathAdded, halted := c.addPlannerPathForTypename(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour) + pathAdded, halted := c.addPlannerPathForTypename(plannerIdx, currentPath, parentPath, ref, fieldName, typeName, planningBehaviour) switch { case pathAdded: return plannerIdx, true @@ -532,7 +532,7 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field planningBehaviour.MergeAliasedRootNodes { // same parent + root node = root sibling - if !c.couldHandleFieldsRequiredByKey(plannerConfig.dataSourceConfiguration, typeName) { + if !c.couldHandleFieldsRequiredByKey(plannerConfig.dataSourceConfiguration, typeName, parentPath) { return -1, false } @@ -549,7 +549,7 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field return plannerIdx, true } if plannerConfig.hasPath(parentPath) || plannerConfig.hasPath(precedingParentPath) { - pathAdded, halted := c.addPlannerPathForTypename(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour) + pathAdded, halted := c.addPlannerPathForTypename(plannerIdx, currentPath, parentPath, ref, fieldName, typeName, planningBehaviour) switch { case pathAdded: return plannerIdx, true @@ -558,7 +558,7 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field } if isProvided || hasChildNode || (hasRootNode && planningBehaviour.MergeAliasedRootNodes) { - if !c.couldHandleFieldsRequiredByKey(plannerConfig.dataSourceConfiguration, typeName) { + if !c.couldHandleFieldsRequiredByKey(plannerConfig.dataSourceConfiguration, typeName, parentPath) { return -1, false } @@ -576,7 +576,7 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field return plannerIdx, true } - pathAdded, halted = c.addPlannerPathForUnionChildOfObjectParent(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour) + pathAdded, halted = c.addPlannerPathForUnionChildOfObjectParent(plannerIdx, currentPath, parentPath, ref, fieldName, typeName, planningBehaviour) switch { case pathAdded: return plannerIdx, true @@ -584,7 +584,7 @@ func (c *configurationVisitor) planWithExistingPlanners(ref int, typeName, field return -1, false } - pathAdded, halted = c.addPlannerPathForChildOfAbstractParent(plannerIdx, currentPath, ref, fieldName, typeName, planningBehaviour) + pathAdded, halted = c.addPlannerPathForChildOfAbstractParent(plannerIdx, currentPath, parentPath, ref, fieldName, typeName, planningBehaviour) switch { case pathAdded: return plannerIdx, true @@ -658,7 +658,7 @@ func (c *configurationVisitor) addNewPlanner(ref int, typeName, fieldName, curre return -1, false } - if !c.couldHandleFieldsRequiredByKey(*config, typeName) { + if !c.couldHandleFieldsRequiredByKey(*config, typeName, parentPath) { return -1, false } @@ -817,7 +817,7 @@ func (c *configurationVisitor) LeaveField(ref int) { } func (c *configurationVisitor) addPlannerPathForUnionChildOfObjectParent( - plannerIndex int, currentPath string, fieldRef int, fieldName string, typeName string, planningBehaviour DataSourcePlanningBehavior, + plannerIndex int, currentPath string, parentPath string, fieldRef int, fieldName string, typeName string, planningBehaviour DataSourcePlanningBehavior, ) (pathAdded bool, halted bool) { if c.walker.EnclosingTypeDefinition.Kind != ast.NodeKindObjectTypeDefinition { @@ -835,7 +835,7 @@ func (c *configurationVisitor) addPlannerPathForUnionChildOfObjectParent( } if node.Kind == ast.NodeKindUnionTypeDefinition { - if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName) { + if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName, parentPath) { return false, true } @@ -854,14 +854,14 @@ func (c *configurationVisitor) addPlannerPathForUnionChildOfObjectParent( } func (c *configurationVisitor) addPlannerPathForChildOfAbstractParent( - plannerIndex int, currentPath string, fieldRef int, fieldName string, typeName string, planningBehaviour DataSourcePlanningBehavior, + plannerIndex int, currentPath string, parentPath string, fieldRef int, fieldName string, typeName string, planningBehaviour DataSourcePlanningBehavior, ) (pathAdded bool, halted bool) { if !c.isParentTypeNodeAbstractType() { return false, false } - pathAdded, halted = c.addPlannerPathForTypename(plannerIndex, currentPath, fieldRef, fieldName, typeName, planningBehaviour) + pathAdded, halted = c.addPlannerPathForTypename(plannerIndex, currentPath, parentPath, fieldRef, fieldName, typeName, planningBehaviour) switch { case pathAdded: return true, false @@ -877,7 +877,7 @@ func (c *configurationVisitor) addPlannerPathForChildOfAbstractParent( } } - if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName) { + if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName, parentPath) { return false, true } @@ -897,7 +897,8 @@ func (c *configurationVisitor) addPlannerPathForChildOfAbstractParent( // adding __typename should be done only in case particular planner has parent path // otherwise it will be added to all planners and will cause visiting of incorrect selection sets func (c *configurationVisitor) addPlannerPathForTypename( - plannerIndex int, currentPath string, fieldRef int, fieldName string, typeName string, planningBehaviour DataSourcePlanningBehavior, + plannerIndex int, currentPath string, parentPath string, fieldRef int, fieldName string, typeName string, + planningBehaviour DataSourcePlanningBehavior, ) (pathAdded bool, halted bool) { if fieldName != typeNameField { return false, false @@ -911,7 +912,7 @@ func (c *configurationVisitor) addPlannerPathForTypename( return true, false } - if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName) { + if !c.couldHandleFieldsRequiredByKey(c.planners[plannerIndex].dataSourceConfiguration, typeName, parentPath) { return false, true } @@ -982,17 +983,16 @@ func (c *configurationVisitor) handleFieldsRequiredByKey(plannerIdx int, config // couldHandleFieldsRequiredByKey - checks wether we could plan the field now according to it's key requirements // if no existing planners datasources could provide us with the required fields we should postpone planning of the field -func (c *configurationVisitor) couldHandleFieldsRequiredByKey(dsConfig DataSourceConfiguration, typeName string) bool { +func (c *configurationVisitor) couldHandleFieldsRequiredByKey(dsConfig DataSourceConfiguration, typeName string, parentPath string) bool { possibleRequiredFields := dsConfig.RequiredFieldsByKey(typeName) if len(possibleRequiredFields) == 0 { return true } for i := range c.planners { - // TODO: here - // if !c.planners[i].hasPath(parentPath) { - // continue - // } + if !c.planners[i].hasPath(parentPath) { + continue + } if c.planners[i].dataSourceConfiguration.Hash() == dsConfig.Hash() { return true From 93ea5d2b4f05a16e00d0b4492686bbd30cce30d1 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Tue, 9 Jan 2024 15:11:58 +0200 Subject: [PATCH 26/27] go fmt --- v2/pkg/ast/ast_node_kind_test.go | 3 ++- v2/pkg/engine/datasource/httpclient/httpclient_test.go | 2 +- v2/pkg/engine/plan/configuration_visitor.go | 2 +- v2/pkg/engine/plan/visitor.go | 2 +- v2/pkg/operationreport/operationreport_test.go | 3 ++- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/v2/pkg/ast/ast_node_kind_test.go b/v2/pkg/ast/ast_node_kind_test.go index bbc632632..2999daf49 100644 --- a/v2/pkg/ast/ast_node_kind_test.go +++ b/v2/pkg/ast/ast_node_kind_test.go @@ -1,8 +1,9 @@ package ast import ( - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestNodeKindIsAbstractType(t *testing.T) { diff --git a/v2/pkg/engine/datasource/httpclient/httpclient_test.go b/v2/pkg/engine/datasource/httpclient/httpclient_test.go index 7322d8cae..3a78ad94d 100644 --- a/v2/pkg/engine/datasource/httpclient/httpclient_test.go +++ b/v2/pkg/engine/datasource/httpclient/httpclient_test.go @@ -4,7 +4,6 @@ import ( "bytes" "compress/gzip" "context" - "github.com/tidwall/sjson" "io" "net/http" "net/http/httptest" @@ -12,6 +11,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/tidwall/sjson" "github.com/wundergraph/graphql-go-tools/v2/internal/pkg/quotes" "github.com/wundergraph/graphql-go-tools/v2/pkg/lexer/literal" diff --git a/v2/pkg/engine/plan/configuration_visitor.go b/v2/pkg/engine/plan/configuration_visitor.go index 8f0c45a3f..33ff2da34 100644 --- a/v2/pkg/engine/plan/configuration_visitor.go +++ b/v2/pkg/engine/plan/configuration_visitor.go @@ -775,7 +775,7 @@ func (c *configurationVisitor) handleMissingPath(typeName string, fieldName stri } allPlannersHasPath := true - for i, _ := range c.planners { + for i := range c.planners { if slices.Contains(suggestedDataSourceHashes, c.planners[i].dataSourceConfiguration.Hash()) { if !c.planners[i].hasPath(currentPath) { allPlannersHasPath = false diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index d0f0d2003..0e6b6e23f 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -532,7 +532,7 @@ func (v *Visitor) resolveOnTypeNames(fieldRef int) [][]byte { func (v *Visitor) addInterfaceObjectNameToTypeNames(fieldRef int, typeName []byte, onTypeNames [][]byte) [][]byte { includeInterfaceObjectName := false var interfaceObjectName string - for i, _ := range v.planners { + for i := range v.planners { if !slices.ContainsFunc(v.planners[i].paths, func(path pathConfiguration) bool { return path.fieldRef == fieldRef }) { diff --git a/v2/pkg/operationreport/operationreport_test.go b/v2/pkg/operationreport/operationreport_test.go index afd5ef008..369d1d066 100644 --- a/v2/pkg/operationreport/operationreport_test.go +++ b/v2/pkg/operationreport/operationreport_test.go @@ -3,8 +3,9 @@ package operationreport import ( "errors" "fmt" - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestExternalErrorMessage(t *testing.T) { From 9bd1617c8815d8fa4501696cfa472d7090ca2308 Mon Sep 17 00:00:00 2001 From: spetrunin Date: Tue, 9 Jan 2024 15:25:33 +0200 Subject: [PATCH 27/27] cleanup --- .../plan/abstract_selection_rewriter_helpers.go | 15 --------------- v2/pkg/engine/plan/configuration.go | 10 ---------- 2 files changed, 25 deletions(-) diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go b/v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go index 819e0ebb4..3f5932c26 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go @@ -350,18 +350,3 @@ func (r *fieldSelectionRewriter) typeNameSelection() (selectionRef int, fieldRef Kind: ast.SelectionKindField, }), field.Ref } - -func (r *fieldSelectionRewriter) addTypeNameToExistingFragment(fragmentSelectionRef int) { - if r.operation.Selections[fragmentSelectionRef].Kind != ast.SelectionKindInlineFragment { - return - } - - inlineFragmentRef := r.operation.Selections[fragmentSelectionRef].Ref - inlineFragmentSelectionSetRef, ok := r.operation.InlineFragmentSelectionSet(inlineFragmentRef) - if !ok { - return - } - - typeNameSelectionRef, _ := r.typeNameSelection() - r.operation.AddSelectionRefToSelectionSet(inlineFragmentSelectionSetRef, typeNameSelectionRef) -} diff --git a/v2/pkg/engine/plan/configuration.go b/v2/pkg/engine/plan/configuration.go index 0b62587c9..1de493dcf 100644 --- a/v2/pkg/engine/plan/configuration.go +++ b/v2/pkg/engine/plan/configuration.go @@ -56,16 +56,6 @@ func (t *TypeConfigurations) RenameTypeNameOnMatchBytes(typeName []byte) []byte return typeName } -func (t *TypeConfigurations) RenameTypeNameOnMatchBytesWithResult(typeName []byte) (newTypeName []byte, renamed bool) { - str := string(typeName) - for i := range *t { - if (*t)[i].TypeName == str { - return []byte((*t)[i].RenameTo), true - } - } - return typeName, false -} - type TypeConfiguration struct { TypeName string // RenameTo modifies the TypeName