Repository: jensneuse/graphql-go-tools
Pull request #121 (2019-12-01 12:59:34)

Bugfix/list planning with append path
This PR fixes nested path building for http data source with extra path args.


Patch:
diff --git a/pkg/execution/datasource_http_json.go b/pkg/execution/datasource_http_json.go
index ea3e81d6..dfeb6b3d 100644
--- a/pkg/execution/datasource_http_json.go
+++ b/pkg/execution/datasource_http_json.go
@@ -32,10 +32,10 @@ func (h *HttpJsonDataSourcePlanner) DirectiveDefinition() []byte {
 }
 
 func (h *HttpJsonDataSourcePlanner) OverrideRootFieldPath(path []string) []string {
-	if len(path) == 1 {
+	if len(path) <= 1 {
 		return nil
 	}
-	return path
+	return path[1:]
 }
 
 func (h *HttpJsonDataSourcePlanner) DirectiveName() []byte {
diff --git a/pkg/execution/planning.go b/pkg/execution/planning.go
index c94373eb..992429dd 100644
--- a/pkg/execution/planning.go
+++ b/pkg/execution/planning.go
@@ -403,50 +403,34 @@ func (p *planningVisitor) fieldPath(ref int) []string {
 		}
 	}
 
-	var fields []int
-	for i := 0;i<len(p.Ancestors);i++{
-		if p.Ancestors[i].Kind == ast.NodeKindField {
-			fields = append(fields,p.Ancestors[i].Ref)
-		}
+	def, ok := p.FieldDefinition(ref)
+	if !ok {
+		return path
 	}
-
-	fields = append(fields,ref)
-
-	var basePath []string
-	for _,i := range fields {
-		def, ok := p.FieldDefinition(i)
-		if !ok {
-			continue
-		}
-		pathDirective, ok := p.definition.FieldDefinitionDirectiveByName(def, []byte("path"))
-		if !ok {
-			continue
-		}
-		appendValue, ok := p.definition.DirectiveArgumentValueByName(pathDirective, []byte("append"))
-		if ok && appendValue.Kind == ast.ValueKindList {
-			for _, j := range p.definition.ListValues[appendValue.Ref].Refs {
-				listValue := p.definition.Values[j]
-				if listValue.Kind != ast.ValueKindString {
-					continue
-				}
-				basePath = append(basePath, p.definition.StringValueContentString(listValue.Ref))
+	pathDirective, ok := p.definition.FieldDefinitionDirectiveByName(def, []byte("path"))
+	if !ok {
+		return path
+	}
+	appendValue, ok := p.definition.DirectiveArgumentValueByName(pathDirective, []byte("append"))
+	if ok && appendValue.Kind == ast.ValueKindList {
+		for _, j := range p.definition.ListValues[appendValue.Ref].Refs {
+			listValue := p.definition.Values[j]
+			if listValue.Kind != ast.ValueKindString {
+				continue
 			}
+			path = append(path, p.definition.StringValueContentString(listValue.Ref))
 		}
-		prependValue, ok := p.definition.DirectiveArgumentValueByName(pathDirective, []byte("prepend"))
-		if ok {
-			for _, j := range p.definition.ListValues[prependValue.Ref].Refs {
-				listValue := p.definition.Values[j]
-				if listValue.Kind != ast.ValueKindString {
-					continue
-				}
-				basePath = append([]string{p.definition.StringValueContentString(listValue.Ref)}, basePath...)
+	}
+	prependValue, ok := p.definition.DirectiveArgumentValueByName(pathDirective, []byte("prepend"))
+	if ok {
+		for _, j := range p.definition.ListValues[prependValue.Ref].Refs {
+			listValue := p.definition.Values[j]
+			if listValue.Kind != ast.ValueKindString {
+				continue
 			}
+			path = append([]string{p.definition.StringValueContentString(listValue.Ref)}, path...)
 		}
 	}
 
-	if len(basePath) != 0 {
-		path = append(basePath, path...)
-	}
-
 	return path
 }


Test patch:
diff --git a/pkg/execution/planning_test.go b/pkg/execution/planning_test.go
index 065fdf98..1356eb69 100644
--- a/pkg/execution/planning_test.go
+++ b/pkg/execution/planning_test.go
@@ -507,10 +507,10 @@ func TestPlanner_Plan(t *testing.T) {
 						},
 						Fields: []Field{
 							{
-								Name: []byte("withPath"),
+								Name:        []byte("withPath"),
 								HasResolver: true,
 								Value: &Value{
-									Path:       []string{"subObject","withPath"},
+									Path:       []string{"subObject"},
 									QuoteValue: true,
 								},
 							},
@@ -519,6 +519,137 @@ func TestPlanner_Plan(t *testing.T) {
 				},
 			},
 		}))
+	t.Run("HTTPJSONDataSource list withoutPath", run(withBaseSchema(HTTPJSONDataSourceSchema), `
+					query ListWithoutPath {
+						listItems {
+							id
+						}
+					}
+					`,
+		ResolverDefinitions{
+			{
+				TypeName:  literal.QUERY,
+				FieldName: []byte("listItems"),
+				DataSourcePlannerFactory: func() DataSourcePlanner {
+					return &HttpJsonDataSourcePlanner{}
+				},
+			},
+		},
+		&Object{
+			operationType: ast.OperationTypeQuery,
+			Fields: []Field{
+				{
+					Name: []byte("data"),
+					Value: &Object{
+						Fetch: &SingleFetch{
+							BufferName: "listItems",
+							Source: &DataSourceInvocation{
+								DataSource: &HttpJsonDataSource{},
+								Args: []Argument{
+									&StaticVariableArgument{
+										Name:  []byte("host"),
+										Value: []byte("httpbin.org"),
+									},
+									&StaticVariableArgument{
+										Name:  []byte("url"),
+										Value: []byte("/anything"),
+									},
+									&StaticVariableArgument{
+										Name:  []byte("method"),
+										Value: []byte("GET"),
+									},
+								},
+							},
+						},
+						Fields: []Field{
+							{
+								Name:        []byte("listItems"),
+								HasResolver: true,
+								Value: &List{
+									Value: &Object{
+										Fields: []Field{
+											{
+												Name: []byte("id"),
+												Value: &Value{
+													Path:       []string{"id"},
+													QuoteValue: true,
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		}))
+	t.Run("HTTPJSONDataSource list withPath", run(withBaseSchema(HTTPJSONDataSourceSchema), `
+					query ListWithPath {
+						listWithPath {
+							id
+						}
+					}
+					`,
+		ResolverDefinitions{
+			{
+				TypeName:  literal.QUERY,
+				FieldName: []byte("listWithPath"),
+				DataSourcePlannerFactory: func() DataSourcePlanner {
+					return &HttpJsonDataSourcePlanner{}
+				},
+			},
+		},
+		&Object{
+			operationType: ast.OperationTypeQuery,
+			Fields: []Field{
+				{
+					Name: []byte("data"),
+					Value: &Object{
+						Fetch: &SingleFetch{
+							BufferName: "listWithPath",
+							Source: &DataSourceInvocation{
+								DataSource: &HttpJsonDataSource{},
+								Args: []Argument{
+									&StaticVariableArgument{
+										Name:  []byte("host"),
+										Value: []byte("httpbin.org"),
+									},
+									&StaticVariableArgument{
+										Name:  []byte("url"),
+										Value: []byte("/anything"),
+									},
+									&StaticVariableArgument{
+										Name:  []byte("method"),
+										Value: []byte("GET"),
+									},
+								},
+							},
+						},
+						Fields: []Field{
+							{
+								Name:        []byte("listWithPath"),
+								HasResolver: true,
+								Value: &List{
+									Path: []string{"items"},
+									Value: &Object{
+										Fields: []Field{
+											{
+												Name: []byte("id"),
+												Value: &Value{
+													Path:       []string{"id"},
+													QuoteValue: true,
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		}))
 	t.Run("HTTPJSONDataSource withHeaders", run(withBaseSchema(HTTPJSONDataSourceSchema), `
 					query WithHeader {
 						withHeaders
@@ -2544,7 +2675,18 @@ type Query {
 			]
         )
 	withPath: String!
-		@path(prepend: ["subObject"])
+		@path(append: ["subObject"])
+		@HttpJsonDataSource(
+            host: "httpbin.org"
+            url: "/anything"
+        )
+	listItems: [ListItem]
+		@HttpJsonDataSource(
+            host: "httpbin.org"
+            url: "/anything"
+        )
+	listWithPath: [ListItem]
+		@path(append: ["items"])
 		@HttpJsonDataSource(
             host: "httpbin.org"
             url: "/anything"
@@ -2553,6 +2695,10 @@ type Query {
     __type(name: String!): __Type
 }
 
+type ListItem {
+	id: String!
+}
+
 input WithBodyInput {
 	foo: String!
 }