fix: fix regression on removing null variables which was undefined (#988

)

execution/engine/execution_engine_test.go

@@ -750,6 +750,76 @@ func TestExecutionEngine_Execute(t *testing.T) {
 		expectedResponse: `{"data":{"heroes":["Human","Droid"]}}`,
 	}))
 
+	t.Run("execute operation with null and omitted input variables", runWithoutError(ExecutionEngineTestCase{
+		schema: func(t *testing.T) *graphql.Schema {
+			t.Helper()
+			schema := `
+			type Query {
+				heroes(names: [String!], height: String): [String!]
+			}`
+			parseSchema, err := graphql.NewSchemaFromString(schema)
+			require.NoError(t, err)
+			return parseSchema
+		}(t),
+		operation: func(t *testing.T) graphql.Request {
+			return graphql.Request{
+				OperationName: "MyHeroes",
+				Variables:     []byte(`{"height": null}`),
+				Query: `query MyHeroes($heroNames: [String!], $height: String){
+						heroes(names: $heroNames, height: $height)
+					}`,
+			}
+		},
+		dataSources: []plan.DataSource{
+			mustGraphqlDataSourceConfiguration(t,
+				"id",
+				mustFactory(t,
+					testNetHttpClient(t, roundTripperTestCase{
+						expectedHost:     "example.com",
+						expectedPath:     "/",
+						expectedBody:     `{"query":"query($heroNames: [String!], $height: String){heroes(names: $heroNames, height: $height)}","variables":{"height":null}}`,
+						sendResponseBody: `{"data":{"heroes":[]}}`,
+						sendStatusCode:   200,
+					}),
+				),
+				&plan.DataSourceMetadata{
+					RootNodes: []plan.TypeField{
+						{TypeName: "Query", FieldNames: []string{"heroes"}},
+					},
+				},
+				mustConfiguration(t, graphql_datasource.ConfigurationInput{
+					Fetch: &graphql_datasource.FetchConfiguration{
+						URL:    "https://example.com/",
+						Method: "POST",
+					},
+					SchemaConfiguration: mustSchemaConfig(
+						t,
+						nil,
+						`type Query { heroes(names: [String!], height: String): [String!] }`,
+					),
+				}),
+			),
+		},
+		fields: []plan.FieldConfiguration{
+			{
+				TypeName:  "Query",
+				FieldName: "heroes",
+				Path:      []string{"heroes"},
+				Arguments: []plan.ArgumentConfiguration{
+					{
+						Name:       "names",
+						SourceType: plan.FieldArgumentSource,
+					},
+					{
+						Name:       "height",
+						SourceType: plan.FieldArgumentSource,
+					},
+				},
+			},
+		},
+		expectedResponse: `{"data":{"heroes":[]}}`,
+	}))
+
 	t.Run("execute operation with null variable on required type", runWithAndCompareError(ExecutionEngineTestCase{
 		schema: func(t *testing.T) *graphql.Schema {
 			t.Helper()

v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go

@@ -1636,23 +1636,29 @@ func (s *Source) compactAndUnNullVariables(input []byte) []byte {
 	if err != nil {
 		return input
 	}
+
+	// if variables are null or empty object, do nothing
 	if bytes.Equal(variables, []byte("null")) || bytes.Equal(variables, []byte("{}")) {
 		return input
 	}
+
+	// remove null variables which actually was undefined in the original user request
 	variables = s.cleanupVariables(variables, undefinedVariables)
-	if bytes.ContainsAny(variables, " \t\n\r") {
+
+	// compact
+	if !bytes.ContainsAny(variables, " \t\n\r") {
 		buf := bytes.NewBuffer(make([]byte, 0, len(variables)))
 		if err := json.Compact(buf, variables); err != nil {
 			return variables
 		}
 		variables = buf.Bytes()
-		input, _ = jsonparser.Set(input, variables, "body", "variables")
 	}
+
+	input, _ = jsonparser.Set(input, variables, "body", "variables")
 	return input
 }
 
-// cleanupVariables removes null variables and empty objects from the input if removeNullVariables is true
-// otherwise returns the input as is
+// cleanupVariables removes null variables which listed in the list of undefinedVariables
 func (s *Source) cleanupVariables(variables []byte, undefinedVariables []string) []byte {
 	// remove null variables from JSON: {"a":null,"b":1} -> {"b":1}
 	if len(undefinedVariables) == 0 {

v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go

@@ -8302,6 +8302,55 @@ func runTestOnTestDefinition(t *testing.T, operation, operationName string, expe
 	return RunTest(testDefinition, operation, operationName, expectedPlan, config)
 }
 
+func TestSource_Load(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, _ := io.ReadAll(r.Body)
+		_, _ = fmt.Fprint(w, string(body))
+	}))
+	defer ts.Close()
+
+	t.Run("null variables", func(t *testing.T) {
+		var (
+			src       = &Source{httpClient: &http.Client{}}
+			serverUrl = ts.URL
+			variables = []byte(`{"a":null,"b":"b","c":{}}`)
+		)
+
+		t.Run("should not touch null variables", func(t *testing.T) {
+			var input []byte
+			input = httpclient.SetInputBodyWithPath(input, variables, "variables")
+			input = httpclient.SetInputURL(input, []byte(serverUrl))
+
+			buf := bytes.NewBuffer(nil)
+
+			require.NoError(t, src.Load(context.Background(), input, buf))
+			assert.Equal(t, `{"variables":{"a":null,"b":"b","c":{}}}`, buf.String())
+		})
+	})
+	t.Run("remove undefined variables", func(t *testing.T) {
+		var (
+			src       = &Source{httpClient: &http.Client{}}
+			serverUrl = ts.URL
+			variables = []byte(`{"a":null,"b":null, "c": null}`)
+		)
+		t.Run("should remove undefined variables and leave null variables", func(t *testing.T) {
+			var input []byte
+			input = httpclient.SetInputBodyWithPath(input, variables, "variables")
+			input = httpclient.SetInputURL(input, []byte(serverUrl))
+			buf := bytes.NewBuffer(nil)
+
+			undefinedVariables := []string{"a", "c"}
+			ctx := context.Background()
+			var err error
+			input, err = httpclient.SetUndefinedVariables(input, undefinedVariables)
+			assert.NoError(t, err)
+
+			require.NoError(t, src.Load(ctx, input, buf))
+			assert.Equal(t, `{"variables":{"b":null}}`, buf.String())
+		})
+	})
+}
+
 type ExpectedFile struct {
 	Name string
 	Size int64