fix: panic when normalizing multi operation documents (#1433)

router-tests/normalization_cache_test.go

@@ -64,6 +64,73 @@ func TestNormalizationCache(t *testing.T) {
 	})
 }
 
+func TestNormalizationCacheWithMultiOperationDocument(t *testing.T) {
+	t.Parallel()
+
+	t.Run("Should identify correct document after removing unused operations during normalization", func(t *testing.T) {
+
+		document := `query A {
+  a: employee(id: 1) {
+    id
+    details {
+      pets {
+        name
+      }
+    }
+  }
+}
+
+query B ($id: Int!) {
+  b: employee(id: $id) {
+    id
+    details {
+      pets {
+        name
+      }
+    }
+  }
+}`
+
+		testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) {
+			res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				OperationName: []byte(`"A"`),
+				Query:         document,
+				Variables:     []byte(`{"id": 1234}`),
+			})
+			require.NoError(t, err)
+			require.Equal(t, "MISS", res.Response.Header.Get(core.NormalizationCacheHeader))
+			require.Equal(t, `{"data":{"a":{"id":1,"details":{"pets":null}}}}`, res.Body)
+
+			res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				OperationName: []byte(`"A"`),
+				Query:         document,
+				Variables:     []byte(`{"id": 12345}`),
+			})
+			require.NoError(t, err)
+			require.Equal(t, "HIT", res.Response.Header.Get(core.NormalizationCacheHeader))
+			require.Equal(t, `{"data":{"a":{"id":1,"details":{"pets":null}}}}`, res.Body)
+
+			res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				OperationName: []byte(`"B"`),
+				Query:         document,
+				Variables:     []byte(`{"id": 1}`),
+			})
+			require.NoError(t, err)
+			require.Equal(t, "MISS", res.Response.Header.Get(core.NormalizationCacheHeader))
+			require.Equal(t, `{"data":{"b":{"id":1,"details":{"pets":null}}}}`, res.Body)
+
+			res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				OperationName: []byte(`"B"`),
+				Query:         document,
+				Variables:     []byte(`{"id": 3}`),
+			})
+			require.NoError(t, err)
+			require.Equal(t, "HIT", res.Response.Header.Get(core.NormalizationCacheHeader))
+			require.Equal(t, `{"data":{"b":{"id":3,"details":{"pets":[{"name":"Snappy"}]}}}}`, res.Body)
+		})
+	})
+}
+
 func TestDefaultValuesForSkipInclude(t *testing.T) {
 	t.Parallel()
 

router-tests/persisted_operations_test.go

@@ -95,6 +95,66 @@ func TestPersistedOperationPOExtensionNotTransmittedToSubgraph(t *testing.T) {
 	})
 }
 
+func TestPersistedNormalizationCacheWithMultiOperationDocument(t *testing.T) {
+	t.Parallel()
+
+	t.Run("Should identify correct document after removing unused operations during normalization", func(t *testing.T) {
+
+		testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) {
+			header := make(http.Header)
+			header.Add("graphql-client-name", "my-client")
+			res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				OperationName: []byte(`"A"`),
+				Extensions:    []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "724399f210ef3f16e6e5427a70bb9609ecea7297e99c3e9241d5912d04eabe60"}}`),
+				Header:        header,
+			})
+			require.NoError(t, err)
+			require.Equal(t, `{"data":{"a":{"id":1,"details":{"pets":null}}}}`, res.Body)
+			require.Equal(t, "MISS", res.Response.Header.Get(core.PersistedOperationCacheHeader))
+			require.Equal(t, "MISS", res.Response.Header.Get(core.ExecutionPlanCacheHeader))
+
+			header = make(http.Header)
+			header.Add("graphql-client-name", "my-client")
+			res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				OperationName: []byte(`"A"`),
+				Extensions:    []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "724399f210ef3f16e6e5427a70bb9609ecea7297e99c3e9241d5912d04eabe60"}}`),
+				Header:        header,
+			})
+			require.NoError(t, err)
+			require.Equal(t, `{"data":{"a":{"id":1,"details":{"pets":null}}}}`, res.Body)
+			require.Equal(t, "HIT", res.Response.Header.Get(core.PersistedOperationCacheHeader))
+			require.Equal(t, "HIT", res.Response.Header.Get(core.ExecutionPlanCacheHeader))
+
+			header = make(http.Header)
+			header.Add("graphql-client-name", "my-client")
+			res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				OperationName: []byte(`"B"`),
+				Variables:     []byte(`{"id": 1}`),
+				Extensions:    []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "724399f210ef3f16e6e5427a70bb9609ecea7297e99c3e9241d5912d04eabe60"}}`),
+				Header:        header,
+			})
+			require.NoError(t, err)
+			require.Equal(t, `{"data":{"b":{"id":1,"details":{"pets":null}}}}`, res.Body)
+			require.Equal(t, "MISS", res.Response.Header.Get(core.PersistedOperationCacheHeader))
+			require.Equal(t, "MISS", res.Response.Header.Get(core.ExecutionPlanCacheHeader))
+
+			header = make(http.Header)
+			header.Add("graphql-client-name", "my-client")
+			res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
+				OperationName: []byte(`"B"`),
+				Variables:     []byte(`{"id": 1}`),
+				Extensions:    []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "724399f210ef3f16e6e5427a70bb9609ecea7297e99c3e9241d5912d04eabe60"}}`),
+				Header:        header,
+			})
+			require.NoError(t, err)
+			require.Equal(t, `{"data":{"b":{"id":1,"details":{"pets":null}}}}`, res.Body)
+			require.Equal(t, "HIT", res.Response.Header.Get(core.PersistedOperationCacheHeader))
+			require.Equal(t, "HIT", res.Response.Header.Get(core.ExecutionPlanCacheHeader))
+		})
+	})
+
+}
+
 func TestPersistedOperationsCache(t *testing.T) {
 	t.Parallel()
 

router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/724399f210ef3f16e6e5427a70bb9609ecea7297e99c3e9241d5912d04eabe60.json

@@ -0,0 +1 @@
+{"version":1,"body":"query A {\n  a: employee(id: 1) {\n    id\n    details {\n      pets {\n        name\n      }\n    }\n  }\n}\n\nquery B ($id: Int!) {\n  b: employee(id: $id) {\n    id\n    details {\n      pets {\n        name\n      }\n    }\n  }\n}"}

router/core/operation_processor.go

@@ -757,6 +757,13 @@ func (o *OperationKit) NormalizeVariables() error {
 		}
 	}
 
+	// Assuming the user sends a multi-operation document
+	// During normalization, we removed the unused operations from the document
+	// This will always lead to operation definitions of a length of 1 even when multiple operations are sent
+	if o.parsedOperation.NormalizationCacheHit {
+		o.operationDefinitionRef = 0
+	}
+
 	// Print the operation without the operation name to get the pure normalized form
 	// Afterward we can calculate the operation ID that is used as a stable identifier for analytics
 
@@ -790,11 +797,6 @@ func (o *OperationKit) NormalizeVariables() error {
 
 	o.kit.normalizedOperation.Reset()
 
-	// Reset the operation name to the original name and print the operation again
-	// to get the final normalized form of the operation
-
-	o.kit.doc.OperationDefinitions[o.operationDefinitionRef].Name = o.originalOperationNameRef
-
 	err = o.kit.printer.Print(o.kit.doc, o.kit.normalizedOperation)
 	if err != nil {
 		return err
@@ -825,7 +827,9 @@ func (o *OperationKit) loadPersistedOperationFromCache(clientName string) (ok bo
 	o.parsedOperation.InternalID = entry.operationID
 	o.parsedOperation.NormalizedRepresentation = entry.normalizedRepresentation
 	o.parsedOperation.Type = entry.operationType
-	o.operationDefinitionRef = entry.operationDefinitionRef
+	//  We will always only have a single operation definition in the document
+	// Because we removed the unused operations during normalization
+	o.operationDefinitionRef = 0
 	err = o.setAndParseOperationDoc()
 	if err != nil {
 		return false, err