From 3a48c79773cec1349609ba563e96d472989e309d Mon Sep 17 00:00:00 2001 From: John Starich Date: Sat, 20 Apr 2024 20:55:45 -0500 Subject: [PATCH 1/4] gofmt -s --- execute_test.go | 30 ++++++++++++++++-------------- gateway_test.go | 5 ----- plan.go | 3 --- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/execute_test.go b/execute_test.go index 7424c9b..5881de4 100644 --- a/execute_test.go +++ b/execute_test.go @@ -44,13 +44,14 @@ func TestExecutor_plansOfOne(t *testing.T) { }, }, // return a known value we can test against - Queryer: &graphql.MockSuccessQueryer{Value: map[string]interface{}{ - "values": []string{ - "hello", - "world", + Queryer: &graphql.MockSuccessQueryer{ + Value: map[string]interface{}{ + "values": []string{ + "hello", + "world", + }, }, }, - }, }, }, }, @@ -90,7 +91,6 @@ func TestExecutor_plansWithDependencies(t *testing.T) { RootStep: &QueryPlanStep{ Then: []*QueryPlanStep{ { - // this is equivalent to // query { user } ParentType: typeNameQuery, @@ -1146,7 +1146,6 @@ func TestExecutor_insertIntoInlineFragmentsList(t *testing.T) { }, }, }, result) - } func TestExecutor_insertIntoLists(t *testing.T) { @@ -1760,7 +1759,6 @@ func TestExecutor_includeIf(t *testing.T) { RootStep: &QueryPlanStep{ Then: []*QueryPlanStep{ { - // this is equivalent to // query { user } ParentType: typeNameQuery, @@ -2020,6 +2018,7 @@ func TestExecutor_threadsVariables(t *testing.T) { t.Error(err.Error()) } } + func TestFindInsertionPoint_rootList(t *testing.T) { t.Parallel() // in this example, the step before would have just resolved (need to be inserted at) @@ -2525,7 +2524,7 @@ func TestFindInsertionPoint_handlesNullObjects(t *testing.T) { func TestSingleObjectWithColonInID(t *testing.T) { t.Parallel() - var source = make(map[string]interface{}) + source := make(map[string]interface{}) _ = json.Unmarshal([]byte( `{"hello": {"id": "Thing:1337", "firstName": "Foo", "lastName": "bar"}}`), &source, @@ -2672,7 +2671,8 @@ func TestExecutor_plansWithManyDeepDependencies(t *testing.T) { "id": "2", "address": "Cats street", }, - }}}, + }}, + }, { ParentType: "User", InsertionPoint: []string{"user", "parent", "parent", "house", "cats"}, @@ -2683,10 +2683,12 @@ func TestExecutor_plansWithManyDeepDependencies(t *testing.T) { Definition: definitionFactory("String"), }, }, - Queryer: &graphql.MockSuccessQueryer{Value: map[string]interface{}{ - "node": map[string]interface{}{ - "id": "3", "name": "kitty", - }}, + Queryer: &graphql.MockSuccessQueryer{ + Value: map[string]interface{}{ + "node": map[string]interface{}{ + "id": "3", "name": "kitty", + }, + }, }, }, }, diff --git a/gateway_test.go b/gateway_test.go index 0fdf2d1..86f34d3 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -89,7 +89,6 @@ func TestGateway(t *testing.T) { gateway, err := New([]*graphql.RemoteSchema{sources[0]}, func(schema *Gateway) { schema.sources = append(schema.sources, sources[1]) }) - if err != nil { t.Error(err.Error()) return @@ -286,7 +285,6 @@ func TestGateway(t *testing.T) { // build a query plan that the executor will follow response, err := gateway.Execute(reqCtx, plan) - if err != nil { t.Errorf("Encountered error executing plan: %s", err.Error()) return @@ -331,7 +329,6 @@ func TestGateway(t *testing.T) { RootStep: &QueryPlanStep{ Then: []*QueryPlanStep{ { - // this is equivalent to // query { allUsers } ParentType: typeNameQuery, @@ -554,7 +551,6 @@ func TestGatewayExecuteRespectsOperationName(t *testing.T) { RootStep: &QueryPlanStep{ Then: []*QueryPlanStep{ { - // this is equivalent to // query { allUsers } ParentType: typeNameQuery, @@ -594,7 +590,6 @@ func TestGatewayExecuteRespectsOperationName(t *testing.T) { RootStep: &QueryPlanStep{ Then: []*QueryPlanStep{ { - // this is equivalent to // query { allUsers } ParentType: typeNameQuery, diff --git a/plan.go b/plan.go index 749780d..10fba65 100644 --- a/plan.go +++ b/plan.go @@ -524,7 +524,6 @@ func (p *MinQueriesPlanner) extractSelection(ctx *PlanningContext, config *extra selection: selection.SelectionSet, wrapper: newWrapper, }) - if err != nil { return nil, err } @@ -541,7 +540,6 @@ func (p *MinQueriesPlanner) extractSelection(ctx *PlanningContext, config *extra } func (p *MinQueriesPlanner) wrapSelectionSet(ctx *PlanningContext, config *extractSelectionConfig, locationFragments map[string]ast.FragmentDefinitionList, location string, selectionSet ast.SelectionSet) (ast.SelectionSet, error) { - ctx.Gateway.logger.Debug("wrapping selection", config.wrapper) // pointers required to nest the @@ -634,7 +632,6 @@ func (p *MinQueriesPlanner) selectLocation(possibleLocations []string, config *e } func (p *MinQueriesPlanner) groupSelectionSet(ctx *PlanningContext, config *extractSelectionConfig) (map[string]ast.SelectionSet, map[string]ast.FragmentDefinitionList, error) { - locationFields := map[string]ast.SelectionSet{} locationFragments := map[string]ast.FragmentDefinitionList{} From edd365bf1c726b5408f67d7e26cf3971e4ba81ca Mon Sep 17 00:00:00 2001 From: John Starich Date: Sat, 20 Apr 2024 20:55:36 -0500 Subject: [PATCH 2/4] Add failing test for variables passed to @include --- gateway_test.go | 71 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/gateway_test.go b/gateway_test.go index 86f34d3..d865db5 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -3,11 +3,16 @@ package gateway import ( "context" "errors" + "fmt" + "net/http" + "net/http/httptest" "strings" "testing" "github.com/nautilus/graphql" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vektah/gqlparser/v2" "github.com/vektah/gqlparser/v2/ast" ) @@ -679,3 +684,69 @@ func TestFieldURLs_concat(t *testing.T) { } assert.Equal(t, []string{"url2"}, urlLocations3) } + +// Verifies fix for https://github.com/nautilus/gateway/issues/199 +func TestIncludeIfVariable(t *testing.T) { + t.Parallel() + schema, err := graphql.LoadSchema(` +type Query { + hello: String + user: User +} + +type User { + firstName: String +} +`) + require.NoError(t, err) + queryerFactory := QueryerFactory(func(ctx *PlanningContext, url string) graphql.Queryer { + return graphql.QueryerFunc(func(input *graphql.QueryInput) (interface{}, error) { + query := gqlparser.MustLoadQuery(schema, input.Query) + var operation ast.OperationDefinition + if assert.Len(t, query.Operations, 1) { + operation = *query.Operations[0] + } + assert.Len(t, operation.VariableDefinitions, 1) + assert.NotNil(t, operation.VariableDefinitions.ForName("returnUser")) + return map[string]interface{}{ + "hello": "world", + "user": map[string]interface{}{ + "firstName": "foo", + }, + }, nil + }) + }) + gateway, err := New([]*graphql.RemoteSchema{ + {Schema: schema, URL: "url1"}, + }, WithQueryerFactory(&queryerFactory)) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(fmt.Sprintf(` + { + "query": %q, + "variables": { + "returnUser": true + } + } + `, ` + query($returnUser: Boolean!) { + hello + user @include(if: $returnUser) { + firstName + } + } + `))) + resp := httptest.NewRecorder() + gateway.GraphQLHandler(resp, req) + assert.Equal(t, http.StatusOK, resp.Code) + assert.JSONEq(t, ` + { + "data": { + "hello": "world", + "user": { + "firstName": "foo" + } + } + } + `, resp.Body.String()) +} From de14216fcdadbe1122ce25c091f436096e84aa30 Mon Sep 17 00:00:00 2001 From: John Starich Date: Tue, 23 Apr 2024 22:03:32 -0500 Subject: [PATCH 3/4] Fix test --- plan.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plan.go b/plan.go index 10fba65..3ee4d5f 100644 --- a/plan.go +++ b/plan.go @@ -450,6 +450,11 @@ func (p *MinQueriesPlanner) extractSelection(ctx *PlanningContext, config *extra for _, variable := range graphql.ExtractVariables(selection.Arguments) { config.step.Variables.Add(variable) } + for _, directive := range selection.Directives { + for _, variable := range graphql.ExtractVariables(directive.Arguments) { + config.step.Variables.Add(variable) + } + } // add it to the list finalSelection = append(finalSelection, selection) From 1eb81d7ad901b924ea61312f379319a31e15cbef Mon Sep 17 00:00:00 2001 From: John Starich Date: Tue, 23 Apr 2024 22:22:13 -0500 Subject: [PATCH 4/4] Assert on variables too --- gateway_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gateway_test.go b/gateway_test.go index d865db5..23eee9b 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -708,6 +708,9 @@ type User { } assert.Len(t, operation.VariableDefinitions, 1) assert.NotNil(t, operation.VariableDefinitions.ForName("returnUser")) + assert.Equal(t, map[string]interface{}{ + "returnUser": true, + }, input.Variables) return map[string]interface{}{ "hello": "world", "user": map[string]interface{}{