Task/sync with splunk fork (#94)

* Added in returning 400's for plan failure, data set as nil when it doesn't exist

* fixed go.mod

* tidied mods

* close channel being held open forever

* break on close

* Prioritize location selection for fragments

When selecting a location for resolving Fields within Fragment Spreads,
we should select the parent's location if possible to avoid executing
federation unnecessarily. This fixes the problem where we used to simply
select the first element in the array. The proposed fix will prioritize
the parent's location. If no priority is matched, then the first
location is still selected. Note that this creates uneven traffic for
one location.

* Query operation name (#4)

adds operation name to queries
tests that operation name is actually passed down to Query

* fix query strings by putting operationName back into them (#5)

* Broke out planning step for error handling; fixed memory leak (#81)

* Added in returning 400's for plan failure, data set as nil when it doesn't exist

* fixed go.mod

* tidied mods

* close channel being held open forever

* break on close

* changed installation type for run

* fix query strings by putting operationName back into them

* Fixed handling of multiple operations (#84)

* added test to respect operation name

* moved operation name to gateway execute

* only the plan for the right operation name is executed

* throw error if multiple plans with no operationName

* no pointers to slices

* ran gofmt

* simpler fix

* update unit tests

Co-authored-by: Brian Krueger <bk@splunk.com>
Co-authored-by: Alec Aivazis <alec@aivazis.com>

* Add graphql codes inside the extensions tag for all errors

* Add an option for setting location priorities

Co-authored-by: Brian Krueger <bkrueger@splunk.com>
Co-authored-by: Brian Krueger <bk@splunk.com>
Co-authored-by: Edwin Mo Song <emsong@splunk.com>
Co-authored-by: Erik Riiska <eriiska@splunk.com>
Co-authored-by: Alec Aivazis <alec@aivazis.com>
Co-authored-by: bardhi <bardho@gmail.com>
Co-authored-by: Bardhi Shtylla <bshtylla@splunk.com>
Co-authored-by: Jared Breeden <jbreeden@splunk.com>

Author: 9 people

execute.go

@@ -210,11 +210,17 @@ func executeStep(
 		}
 	}
 
+	operationName := ""
+	if plan != nil && plan.Operation != nil {
+		operationName = plan.Operation.Name
+	}
+
 	// fire the query
 	err := queryer.Query(ctx.RequestContext, &graphql.QueryInput{
 		Query:         step.QueryString,
 		QueryDocument: step.QueryDocument,
 		Variables:     variables,
+		OperationName: operationName,
 	}, &queryResult)
 	if err != nil {
 		log.Warn("Network Error: ", err)

execute_test.go

@@ -47,7 +47,8 @@ func TestExecutor_plansOfOne(t *testing.T) {
 								"hello",
 								"world",
 							},
-						}},
+						},
+						},
 					},
 				},
 			},
@@ -1537,6 +1538,7 @@ func TestExecutor_threadsVariables(t *testing.T) {
 		Plan: &QueryPlan{
 			Operation: &ast.OperationDefinition{
 				Operation:           ast.Query,
+				Name:                "hoopla",
 				VariableDefinitions: fullVariableDefs,
 			},
 			RootStep: &QueryPlanStep{
@@ -1571,6 +1573,7 @@ func TestExecutor_threadsVariables(t *testing.T) {
 								// and definitions
 								assert.Equal(t, ast.VariableDefinitionList{fullVariableDefs[0]}, input.QueryDocument.Operations[0].VariableDefinitions)
 								assert.Equal(t, "hello", input.Query)
+								assert.Equal(t, "hoopla", input.OperationName)
 
 								return map[string]interface{}{"values": []string{"world"}}, nil
 							},

gateway.go

@@ -15,15 +15,16 @@ import (
 // remote schemas into one, generating a query plan to execute based on an incoming request, and following
 // that plan
 type Gateway struct {
-	sources        []*graphql.RemoteSchema
-	schema         *ast.Schema
-	planner        QueryPlanner
-	executor       Executor
-	merger         Merger
-	middlewares    MiddlewareList
-	queryFields    []*QueryField
-	queryerFactory *QueryerFactory
-	queryPlanCache QueryPlanCache
+	sources            []*graphql.RemoteSchema
+	schema             *ast.Schema
+	planner            QueryPlanner
+	executor           Executor
+	merger             Merger
+	middlewares        MiddlewareList
+	queryFields        []*QueryField
+	queryerFactory     *QueryerFactory
+	queryPlanCache     QueryPlanCache
+	locationPriorities []string
 
 	// group up the list of middlewares at startup to avoid it during execution
 	requestMiddlewares  []graphql.NetworkMiddleware
@@ -149,6 +150,14 @@ func New(sources []*graphql.RemoteSchema, configs ...Option) (*Gateway, error) {
 		}
 	}
 
+	// if we have location priorities to assign
+	if gateway.locationPriorities != nil {
+		// if the planner can accept the priorities
+		if planner, ok := gateway.planner.(PlannerWithLocationPriorities); ok {
+			gateway.planner = planner.WithLocationPriorities(gateway.locationPriorities)
+		}
+	}
+
 	internal := gateway.internalSchema()
 	// find the field URLs before we merge schemas. We need to make sure to include
 	// the fields defined by the gateway's internal schema
@@ -256,6 +265,12 @@ func WithQueryerFactory(factory *QueryerFactory) Option {
 	}
 }
 
+func WithLocationPriorities(priorities []string) Option {
+	return func(g *Gateway) {
+		g.locationPriorities = priorities
+	}
+}
+
 var nodeField = &QueryField{
 	Name: "node",
 	Type: ast.NamedType("Node", &ast.Position{}),

gateway_test.go

@@ -127,6 +127,18 @@ func TestGateway(t *testing.T) {
 		assert.Equal(t, &factory, gateway.planner.(*MinQueriesPlanner).QueryerFactory)
 	})
 
+	t.Run("WithLocationPriorities", func(t *testing.T) {
+		priorities := []string{"url1", "url2"}
+
+		gateway, err := New(sources, WithLocationPriorities(priorities))
+		if err != nil {
+			t.Error(err.Error())
+			return
+		}
+
+		assert.Equal(t, priorities, gateway.locationPriorities)
+	})
+
 	t.Run("fieldURLs ignore introspection", func(t *testing.T) {
 		locations := fieldURLs(sources, true)
 

plan.go

@@ -60,6 +60,11 @@ type PlannerWithQueryerFactory interface {
 	WithQueryerFactory(*QueryerFactory) QueryPlanner
 }
 
+// PlannerWithLocationFactory is an interface for planners with configurable location priorities
+type PlannerWithLocationPriorities interface {
+	WithLocationPriorities(priorities []string) QueryPlanner
+}
+
 // QueryerFactory is a function that returns the queryer to use depending on the context
 type QueryerFactory func(ctx *PlanningContext, url string) graphql.Queryer
 
@@ -72,6 +77,7 @@ type Planner struct {
 // MinQueriesPlanner does the most basic level of query planning
 type MinQueriesPlanner struct {
 	Planner
+	LocationPriorities []string
 }
 
 // WithQueryerFactory returns a version of the planner with the factory set
@@ -80,6 +86,11 @@ func (p *MinQueriesPlanner) WithQueryerFactory(factory *QueryerFactory) QueryPla
 	return p
 }
 
+func (p *MinQueriesPlanner) WithLocationPriorities(priorities []string) QueryPlanner {
+	p.LocationPriorities = priorities
+	return p
+}
+
 // PlanningContext is the input struct to the Plan method
 type PlanningContext struct {
 	Query     string
@@ -244,7 +255,7 @@ func (p *MinQueriesPlanner) generatePlans(ctx *PlanningContext, query *ast.Query
 					}
 
 					// build up the query document
-					step.QueryDocument = plannerBuildQuery(step.ParentType, variableDefs, step.SelectionSet, step.FragmentDefinitions)
+					step.QueryDocument = plannerBuildQuery(plan.Operation.Name, step.ParentType, variableDefs, step.SelectionSet, step.FragmentDefinitions)
 
 					// we also need to turn the query into a string
 					queryString, err := graphql.PrintQuery(step.QueryDocument)
@@ -599,13 +610,41 @@ func (p *MinQueriesPlanner) wrapSelectionSet(config *extractSelectionConfig, loc
 	return ast.SelectionSet{selection}, nil
 }
 
+// selects one location out of possibleLocations, prioritizing the parent's location and the internal schema
+func (p *MinQueriesPlanner) selectLocation(possibleLocations []string, config *extractSelectionConfig) string {
+	// if this field can only be found in one location
+	if len(possibleLocations) == 1 {
+		return possibleLocations[0]
+		// the field can be found in many locations
+	} else {
+		// locations to prioritize first
+		priorities := make([]string, len(p.LocationPriorities), len(p.LocationPriorities)+2)
+		copy(priorities, p.LocationPriorities)
+		priorities = append(priorities, config.parentLocation, internalSchemaLocation)
+
+		for _, priority := range priorities {
+			// look to see if the current location is one of the possible locations
+			for _, location := range possibleLocations {
+				// if the location is the same as the parent
+				if location == priority {
+					// assign this field to the parents entry
+					return priority
+				}
+			}
+		}
+
+		// if we got here then this field can be found in multiple services and none of the top priority locations.
+		// for now, just use the first one
+		return possibleLocations[0]
+	}
+}
+
 func (p *MinQueriesPlanner) groupSelectionSet(config *extractSelectionConfig) (map[string]ast.SelectionSet, map[string]ast.FragmentDefinitionList, error) {
 
 	locationFields := map[string]ast.SelectionSet{}
 	locationFragments := map[string]ast.FragmentDefinitionList{}
 
 	// split each selection into groups of selection sets to be sent to a single service
-FieldLoop:
 	for _, selection := range config.selection {
 		// each kind of selection contributes differently to the final selection set
 		switch selection := selection.(type) {
@@ -628,30 +667,8 @@ FieldLoop:
 				return nil, nil, err
 			}
 
-			// if this field can only be found in one location
-			if len(possibleLocations) == 1 {
-				locationFields[possibleLocations[0]] = append(locationFields[possibleLocations[0]], field)
-				// the field can be found in many locations
-			} else {
-				// locations to prioritize first
-				for _, priority := range []string{config.parentLocation, internalSchemaLocation} {
-					// look to see if the current location is one of the possible locations
-					for _, location := range possibleLocations {
-						// if the location is the same as the parent
-						if location == priority {
-							// assign this field to the parents entry
-							locationFields[priority] = append(locationFields[priority], field)
-							// we're done with this field
-							continue FieldLoop
-						}
-					}
-				}
-
-				// if we got here then this field can be found in multiple services and none of the top priority locations.
-				// for now, just use the first one
-				locationFields[possibleLocations[0]] = append(locationFields[possibleLocations[0]], field)
-			}
-
+			location := p.selectLocation(possibleLocations, config)
+			locationFields[location] = append(locationFields[location], field)
 		case *ast.FragmentSpread:
 			log.Debug("Encountered fragment spread ", selection.Name)
 
@@ -691,8 +708,8 @@ FieldLoop:
 						return nil, nil, err
 					}
 
-					// add the field to the location
-					fragmentLocations[fieldLocations[0]] = append(fragmentLocations[fieldLocations[0]], field)
+					fieldLocation := p.selectLocation(fieldLocations, config)
+					fragmentLocations[fieldLocation] = append(fragmentLocations[fieldLocation], field)
 
 				case *ast.FragmentSpread, *ast.InlineFragment:
 					// non-field selections will be handled in the next tick
@@ -890,11 +907,12 @@ func (p *Planner) GetQueryer(ctx *PlanningContext, url string) graphql.Queryer {
 	return graphql.NewSingleRequestQueryer(url)
 }
 
-func plannerBuildQuery(parentType string, variables ast.VariableDefinitionList, selectionSet ast.SelectionSet, fragmentDefinitions ast.FragmentDefinitionList) *ast.QueryDocument {
+func plannerBuildQuery(operationName, parentType string, variables ast.VariableDefinitionList, selectionSet ast.SelectionSet, fragmentDefinitions ast.FragmentDefinitionList) *ast.QueryDocument {
 	log.Debug("Building Query: \n"+"\tParentType: ", parentType, " ")
 	// build up an operation for the query
 	operation := &ast.OperationDefinition{
 		VariableDefinitions: variables,
+		Name:                operationName,
 	}
 
 	// assign the right operation