From 06d9407beee3cd1c210948c4ddbf2b8c0214fe75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergiy=20=F0=9F=87=BA=F0=9F=87=A6?= <818351+devsergiy@users.noreply.github.com> Date: Mon, 18 Nov 2024 22:14:29 +0200 Subject: [PATCH] fix: fix regression on removing null variables which was undefined (#988) --- execution/engine/execution_engine_test.go | 70 +++++++++++++++++++ .../graphql_datasource/graphql_datasource.go | 14 ++-- .../graphql_datasource_test.go | 49 +++++++++++++ 3 files changed, 129 insertions(+), 4 deletions(-) diff --git a/execution/engine/execution_engine_test.go b/execution/engine/execution_engine_test.go index d310fc5b9..db18169fd 100644 --- a/execution/engine/execution_engine_test.go +++ b/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() diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 04b0a833a..3f20aaf02 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/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 { diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go index c1cef5c52..f770f85e6 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go +++ b/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