Handle empty fragment spread being first in query

The previous fix only handled cases where an empty fragment returned nothing after a field had already been written, if it was first and returned nothing the subsequent fields would write an invalid comma ahead of the innerBody.
movio · Jun 20, 2023 · 77261c0 · 77261c0
1 parent 2f9e0bf
commit 77261c0
Show file tree

Hide file tree

Showing 2 changed files with 168 additions and 2 deletions.
diff --git a/execution_result.go b/execution_result.go
@@ -320,7 +320,8 @@ func formatResponseDataRec(schema *ast.Schema, selectionSet ast.SelectionSet, re
 		objectTypename := extractAndCastTypenameField(result)
 		filteredSelectionSet := unionAndTrimSelectionSet(objectTypename, schema, selectionSet)
 
-		for i, selection := range filteredSelectionSet {
+		itemWritten := false
+		for _, selection := range filteredSelectionSet {
 			var innerBody []byte
 			switch selection := selection.(type) {
 			case *ast.InlineFragment:
@@ -349,10 +350,11 @@ func formatResponseDataRec(schema *ast.Schema, selectionSet ast.SelectionSet, re
 				innerBody = innerBuf.Bytes()
 			}
 			if len(innerBody) > 0 {
-				if i > 0 {
+				if itemWritten {
 					buf.WriteString(",")
 				}
 				buf.Write(innerBody)
+				itemWritten = true
 			}
 		}
 		if !insideFragment {

diff --git a/execution_result_test.go b/execution_result_test.go
@@ -1637,6 +1637,170 @@ func TestFormatResponseBody(t *testing.T) {
 		require.JSONEq(t, expectedJSON, string(bodyJSON))
 	})
 
+	t.Run("multiple implementations with empty fragment spreads before other fields", func(t *testing.T) {
+		ddl := `
+			interface Gizmo {
+				id: ID!
+				name: String!
+			}
+
+			type Gadget implements Gizmo {
+				id: ID!
+				name: String!
+				owner: String!
+			}
+
+			type Tool implements Gizmo {
+				id: ID!
+				name: String!
+				category: String!
+			}
+
+			type Query {
+				gizmos: [Gizmo!]!
+			}
+		`
+
+		result := jsonToInterfaceMap(`{
+			"gizmos": [
+				{
+					"id": "GADGET1",
+					"name": "Gadget #1",
+					"owner": "Bob",
+					"__typename": "Gadget",
+					"_bramble__typename": "Gadget"
+				},
+				{
+					"id": "GADGET2",
+					"name": "Gadget #2",
+					"category": "Plastic",
+					"__typename": "Tool",
+					"_bramble__typename": "Tool"
+				}
+			]
+		}`)
+
+		schema := gqlparser.MustLoadSchema(&ast.Source{Name: "fixture", Input: ddl})
+
+		query := `
+		query Gizmo {
+			gizmos {
+				...GizmoDetails
+				id
+			}
+		}
+
+		fragment GizmoDetails on Gizmo {
+			... on Gadget {
+				name
+				owner
+			}
+		}`
+
+		expectedJSON := `
+		{
+			"gizmos": [
+				{
+					"id": "GADGET1",
+					"name": "Gadget #1",
+					"owner": "Bob"
+				},
+				{
+					"id": "GADGET2"
+				}
+			]
+		}`
+
+		document := gqlparser.MustLoadQuery(schema, query)
+		bodyJSON := formatResponseData(schema, document.Operations[0].SelectionSet, result)
+		require.JSONEq(t, expectedJSON, string(bodyJSON))
+	})
+
+	t.Run("multiple implementations with multiple empty fragment spreads around other fields", func(t *testing.T) {
+		ddl := `
+			interface Gizmo {
+				id: ID!
+				name: String!
+			}
+
+			type Gadget implements Gizmo {
+				id: ID!
+				name: String!
+				owner: String!
+			}
+
+			type Tool implements Gizmo {
+				id: ID!
+				name: String!
+				category: String!
+			}
+
+			type Query {
+				gizmos: [Gizmo!]!
+			}
+		`
+
+		result := jsonToInterfaceMap(`{
+			"gizmos": [
+				{
+					"id": "GADGET1",
+					"name": "Gadget #1",
+					"owner": "Bob",
+					"__typename": "Gadget",
+					"_bramble__typename": "Gadget"
+				},
+				{
+					"id": "GADGET2",
+					"name": "Gadget #2",
+					"category": "Plastic",
+					"__typename": "Tool",
+					"_bramble__typename": "Tool"
+				}
+			]
+		}`)
+
+		schema := gqlparser.MustLoadSchema(&ast.Source{Name: "fixture", Input: ddl})
+
+		query := `
+		query Gizmo {
+			gizmos {
+				...GizmoName
+				id
+				...GizmoDetails
+			}
+		}
+
+		fragment GizmoName on Gizmo {
+			... on Gadget {
+				name
+			}
+		}
+
+		fragment GizmoDetails on Gizmo {
+			... on Gadget {
+				owner
+			}
+		}`
+
+		expectedJSON := `
+		{
+			"gizmos": [
+				{
+					"id": "GADGET1",
+					"name": "Gadget #1",
+					"owner": "Bob"
+				},
+				{
+					"id": "GADGET2"
+				}
+			]
+		}`
+
+		document := gqlparser.MustLoadQuery(schema, query)
+		bodyJSON := formatResponseData(schema, document.Operations[0].SelectionSet, result)
+		require.JSONEq(t, expectedJSON, string(bodyJSON))
+	})
+
 	t.Run("multiple implementation fragment spreads (bottom fragment matches)", func(t *testing.T) {
 		ddl := `
 			interface Gizmo {