Merge pull request #215 from movio/fragment-spread

Fix serialisation bug when a fragment spread was not exhaustive
movio · Jun 20, 2023 · 2f9e0bf · 2f9e0bf
2 parents eff66a8 + 259157a
commit 2f9e0bf
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 27 deletions.
diff --git a/execution_result.go b/execution_result.go
@@ -321,41 +321,38 @@ func formatResponseDataRec(schema *ast.Schema, selectionSet ast.SelectionSet, re
 		filteredSelectionSet := unionAndTrimSelectionSet(objectTypename, schema, selectionSet)
 
 		for i, selection := range filteredSelectionSet {
+			var innerBody []byte
 			switch selection := selection.(type) {
 			case *ast.InlineFragment:
-				innerBody := formatResponseDataRec(schema, selection.SelectionSet, result, true)
-				buf.Write(innerBody)
-
+				innerBody = formatResponseDataRec(schema, selection.SelectionSet, result, true)
 			case *ast.FragmentSpread:
-				innerBody := formatResponseDataRec(schema, selection.Definition.SelectionSet, result, true)
-				buf.Write(innerBody)
+				innerBody = formatResponseDataRec(schema, selection.Definition.SelectionSet, result, true)
 			case *ast.Field:
 				field := selection
+				var innerBuf bytes.Buffer
+				fmt.Fprintf(&innerBuf, `"%s":`, field.Alias)
 				fieldData, ok := result[field.Alias]
-				buf.WriteString(fmt.Sprintf(`"%s":`, field.Alias))
 				if !ok {
-					buf.WriteString("null")
-					if i < len(filteredSelectionSet)-1 {
-						buf.WriteString(",")
-					}
-					continue
-				}
-				if field.SelectionSet != nil && len(field.SelectionSet) > 0 {
-					innerBody := formatResponseDataRec(schema, field.SelectionSet, fieldData, false)
-					buf.Write(innerBody)
+					innerBuf.WriteString("null")
+				} else if field.SelectionSet != nil && len(field.SelectionSet) > 0 {
+					val := formatResponseDataRec(schema, field.SelectionSet, fieldData, false)
+					innerBuf.Write(val)
 				} else {
 					fieldJSON, err := json.Marshal(&fieldData)
 					if err != nil {
 						// We panic here because the data we're working on has already come through
 						// from downstream services as JSON. We should never get to this point with invalid JSON.
 						log.Panicf("invalid json when formatting response: %v", err)
 					}
-
-					buf.Write(fieldJSON)
+					innerBuf.Write(fieldJSON)
 				}
+				innerBody = innerBuf.Bytes()
 			}
-			if i < len(filteredSelectionSet)-1 {
-				buf.WriteString(",")
+			if len(innerBody) > 0 {
+				if i > 0 {
+					buf.WriteString(",")
+				}
+				buf.Write(innerBody)
 			}
 		}
 		if !insideFragment {
@@ -364,23 +361,21 @@ func formatResponseDataRec(schema *ast.Schema, selectionSet ast.SelectionSet, re
 	case []interface{}:
 		buf.WriteString("[")
 		for i, v := range result {
-			innerBody := formatResponseDataRec(schema, selectionSet, v, false)
-			buf.Write(innerBody)
-
-			if i < len(result)-1 {
+			if i > 0 {
 				buf.WriteString(",")
 			}
+			innerBody := formatResponseDataRec(schema, selectionSet, v, false)
+			buf.Write(innerBody)
 		}
 		buf.WriteString("]")
 	case []map[string]interface{}:
 		buf.WriteString("[")
 		for i, v := range result {
-			innerBody := formatResponseDataRec(schema, selectionSet, v, false)
-			buf.Write(innerBody)
-
-			if i < len(result)-1 {
+			if i > 0 {
 				buf.WriteString(",")
 			}
+			innerBody := formatResponseDataRec(schema, selectionSet, v, false)
+			buf.Write(innerBody)
 		}
 		buf.WriteString("]")
 	}

diff --git a/execution_result_test.go b/execution_result_test.go
@@ -1558,6 +1558,85 @@ func TestFormatResponseBody(t *testing.T) {
 		require.JSONEq(t, expectedJSON, string(bodyJSON))
 	})
 
+	t.Run("multiple implementations with incomplete fragment spreads", 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 {
+				id
+				...GizmoDetails
+			}
+		}
+
+		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 implementation fragment spreads (bottom fragment matches)", func(t *testing.T) {
 		ddl := `
 			interface Gizmo {