Skip to content

Commit

Permalink
Include variables passed to directives in query plan steps (#206)
Browse files Browse the repository at this point in the history
* gofmt -s
* Add failing test for variables passed to @include
* Fix test
  • Loading branch information
JohnStarich authored Apr 24, 2024
1 parent 3c19e01 commit c1a3668
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 22 deletions.
30 changes: 16 additions & 14 deletions execute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
},
},
},
},
Expand Down Expand Up @@ -90,7 +91,6 @@ func TestExecutor_plansWithDependencies(t *testing.T) {
RootStep: &QueryPlanStep{
Then: []*QueryPlanStep{
{

// this is equivalent to
// query { user }
ParentType: typeNameQuery,
Expand Down Expand Up @@ -1146,7 +1146,6 @@ func TestExecutor_insertIntoInlineFragmentsList(t *testing.T) {
},
},
}, result)

}

func TestExecutor_insertIntoLists(t *testing.T) {
Expand Down Expand Up @@ -1760,7 +1759,6 @@ func TestExecutor_includeIf(t *testing.T) {
RootStep: &QueryPlanStep{
Then: []*QueryPlanStep{
{

// this is equivalent to
// query { user }
ParentType: typeNameQuery,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -2672,7 +2671,8 @@ func TestExecutor_plansWithManyDeepDependencies(t *testing.T) {
"id": "2",
"address": "Cats street",
},
}}},
}},
},
{
ParentType: "User",
InsertionPoint: []string{"user", "parent", "parent", "house", "cats"},
Expand All @@ -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",
},
},
},
},
},
Expand Down
79 changes: 74 additions & 5 deletions gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -89,7 +94,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
Expand Down Expand Up @@ -286,7 +290,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
Expand Down Expand Up @@ -331,7 +334,6 @@ func TestGateway(t *testing.T) {
RootStep: &QueryPlanStep{
Then: []*QueryPlanStep{
{

// this is equivalent to
// query { allUsers }
ParentType: typeNameQuery,
Expand Down Expand Up @@ -554,7 +556,6 @@ func TestGatewayExecuteRespectsOperationName(t *testing.T) {
RootStep: &QueryPlanStep{
Then: []*QueryPlanStep{
{

// this is equivalent to
// query { allUsers }
ParentType: typeNameQuery,
Expand Down Expand Up @@ -594,7 +595,6 @@ func TestGatewayExecuteRespectsOperationName(t *testing.T) {
RootStep: &QueryPlanStep{
Then: []*QueryPlanStep{
{

// this is equivalent to
// query { allUsers }
ParentType: typeNameQuery,
Expand Down Expand Up @@ -684,3 +684,72 @@ 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"))
assert.Equal(t, map[string]interface{}{
"returnUser": true,
}, input.Variables)
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())
}
8 changes: 5 additions & 3 deletions plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -524,7 +529,6 @@ func (p *MinQueriesPlanner) extractSelection(ctx *PlanningContext, config *extra
selection: selection.SelectionSet,
wrapper: newWrapper,
})

if err != nil {
return nil, err
}
Expand All @@ -541,7 +545,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
Expand Down Expand Up @@ -634,7 +637,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{}

Expand Down

0 comments on commit c1a3668

Please sign in to comment.