Skip to content

Commit

Permalink
fix normalization: do not extract default field argument values, extr…
Browse files Browse the repository at this point in the history
…act only variables default values (#824)
  • Loading branch information
devsergiy committed Jun 14, 2024
2 parents dc45868 + 2282f86 commit b78ed4c
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 74 deletions.
2 changes: 1 addition & 1 deletion execution/engine/execution_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ func TestExecutionEngine_Execute(t *testing.T) {
testNetHttpClient(t, roundTripperTestCase{
expectedHost: "example.com",
expectedPath: "/",
expectedBody: `{"query":"query($a: String, $b: String!){heroDefault(name: $a) heroDefaultRequired(name: $b)}","variables":{"b":"AnyRequired","a":"Any"}}`,
expectedBody: `{"query":"{heroDefault heroDefaultRequired}"}`,
sendResponseBody: `{"data":{"heroDefault":"R2D2","heroDefaultRequired":"R2D2"}}`,
sendStatusCode: 200,
}),
Expand Down
20 changes: 10 additions & 10 deletions v2/pkg/astnormalization/astnormalization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,9 @@ func TestNormalizeOperation(t *testing.T) {
run(t, testDefinition, `
query {
simple
}`, `query($a: String) {
simple(input: $a)
}`, ``, `{"a":"foo"}`)
}`, `query {
simple
}`, ``, ``)
})
t.Run("input list coercion inline", func(t *testing.T) {
run(t, inputCoercionForListDefinition, `
Expand Down Expand Up @@ -285,10 +285,10 @@ func TestNormalizeOperation(t *testing.T) {
fragment D on Dog {
name
}
query ($a: String) {
simple(input: $a)
query {
simple
...D
}`, ``, `{"a":"foo"}`)
}`, ``, ``)
})

}
Expand Down Expand Up @@ -679,7 +679,7 @@ func TestOperationNormalizer_NormalizeNamedOperation(t *testing.T) {
assert.Equal(t, `{"id":"1"}`, string(operation.Input.Variables))
})

t.Run("should properly extract default values and remove unmatched query", func(t *testing.T) {
t.Run("should not extract default values from query body and remove unmatched query", func(t *testing.T) {
schema := `
type Query {
operationA(input: String = "foo"): String
Expand All @@ -694,8 +694,8 @@ func TestOperationNormalizer_NormalizeNamedOperation(t *testing.T) {
operationB
}`

expectedQuery := `query B($a: String){
operationB(input: $a)
expectedQuery := `query B {
operationB
}`

definition := unsafeparser.ParseGraphqlDocumentStringWithBaseSchema(schema)
Expand All @@ -708,7 +708,7 @@ func TestOperationNormalizer_NormalizeNamedOperation(t *testing.T) {
actual, _ := astprinter.PrintStringIndent(&operation, &definition, " ")
assert.Equal(t, expectedQuery, actual)

expectedVariables := `{"a":"bar"}`
expectedVariables := ``
assert.Equal(t, expectedVariables, string(operation.Input.Variables))
})
}
Expand Down
28 changes: 0 additions & 28 deletions v2/pkg/astnormalization/variables_default_value_extraction.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/wundergraph/graphql-go-tools/v2/pkg/ast"
"github.com/wundergraph/graphql-go-tools/v2/pkg/astimport"
"github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor"
"github.com/wundergraph/graphql-go-tools/v2/pkg/internal/unsafebytes"
)

func extractVariablesDefaultValue(walker *astvisitor.Walker) *variablesDefaultValueExtractionVisitor {
Expand Down Expand Up @@ -81,15 +80,12 @@ func (v *variablesDefaultValueExtractionVisitor) EnterField(ref int) {

for _, definitionInputValueDefRef := range v.definition.FieldDefinitions[fieldDefRef].ArgumentsDefinition.Refs {
operationArgRef, exists := v.operation.FieldArgument(ref, v.definition.InputValueDefinitionNameBytes(definitionInputValueDefRef))

if exists {
operationArgValue := v.operation.ArgumentValue(operationArgRef)
if v.operation.ValueContainsVariable(operationArgValue) {
defTypeRef := v.definition.InputValueDefinitions[definitionInputValueDefRef].Type
v.traverseValue(operationArgValue, defTypeRef)
}
} else {
v.processDefaultFieldArguments(ref, definitionInputValueDefRef)
}
}
}
Expand Down Expand Up @@ -226,30 +222,6 @@ func (v *variablesDefaultValueExtractionVisitor) saveArgumentsWithTypeNotNull(op
v.variablesNamesUsedInPositionsExpectingNonNullType = append(v.variablesNamesUsedInPositionsExpectingNonNullType, v.operation.VariableValueNameBytes(operationVariableValueRef))
}

func (v *variablesDefaultValueExtractionVisitor) processDefaultFieldArguments(operationFieldRef, definitionInputValueDefRef int) {
if !v.definition.InputValueDefinitionHasDefaultValue(definitionInputValueDefRef) {
return
}

variableNameBytes := v.operation.GenerateUnusedVariableDefinitionName(v.Ancestors[0].Ref)
valueBytes, err := v.definition.ValueToJSON(v.definition.InputValueDefinitionDefaultValue(definitionInputValueDefRef))
if err != nil {
return
}
v.operation.Input.Variables, err = sjson.SetRawBytes(v.operation.Input.Variables, unsafebytes.BytesToString(variableNameBytes), valueBytes)
if err != nil {
v.StopWithInternalErr(err)
return
}

variableValueRef, argRef := v.operation.ImportVariableValueArgument(v.definition.InputValueDefinitionNameBytes(definitionInputValueDefRef), variableNameBytes)
defType := v.definition.InputValueDefinitions[definitionInputValueDefRef].Type
importedDefType := v.importer.ImportType(defType, v.definition, v.operation)

v.operation.AddArgumentToField(operationFieldRef, argRef)
v.operation.AddVariableDefinitionToOperationDefinition(v.operationRef, variableValueRef, importedDefType)
}

func (v *variablesDefaultValueExtractionVisitor) EnterDocument(operation, definition *ast.Document) {
v.operation, v.definition = operation, definition
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func TestVariablesDefaultValueExtraction(t *testing.T) {
mutation simple {
simple
}`, "", `
mutation simple($a: String) {
simple(input: $a)
}`, ``, `{"a":"foo"}`)
mutation simple {
simple
}`, ``, ``)
})

t.Run("value provided", func(t *testing.T) {
Expand All @@ -73,9 +73,9 @@ func TestVariablesDefaultValueExtraction(t *testing.T) {
mutation simple($a: String) {
mixed(a: $a, b: "bar")
}`, "", `
mutation simple($a: String, $b: String, $c: String!, $d: String) {
mixed(a: $a, b: "bar", input: $b, nonNullInput: $c, nullableWithNullDefault: $d)
}`, `{"a":"aaa"}`, `{"d":null,"c":"bar","b":"foo","a":"aaa"}`)
mutation simple($a: String) {
mixed(a: $a, b: "bar")
}`, `{"a":"aaa"}`, `{"a":"aaa"}`)
})
})

Expand Down Expand Up @@ -264,9 +264,9 @@ func TestVariablesDefaultValueExtraction(t *testing.T) {
mutation simple($a: String = "bar", $b: String = "bazz") {
mixed(a: $a, b: $b)
}`, "", `
mutation simple($a: String, $b: String, $c: String, $d: String!, $e: String) {
mixed(a: $a, b: $b, input: $c, nonNullInput: $d, nullableWithNullDefault: $e)
}`, `{"a":"aaa"}`, `{"e":null,"d":"bar","c":"foo","b":"bazz","a":"aaa"}`)
mutation simple($a: String, $b: String) {
mixed(a: $a, b: $b)
}`, `{"a":"aaa"}`, `{"b":"bazz","a":"aaa"}`)

})

Expand Down
29 changes: 17 additions & 12 deletions v2/pkg/engine/datasource/introspection_datasource/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,17 @@ type introspectionInput struct {
}

var (
lBrace = []byte("{")
rBrace = []byte("}")
comma = []byte(",")
requestTypeField = []byte(`"request_type":`)
onTypeField = []byte(`"on_type_name":"{{ .object.name }}"`)
typeNameField = []byte(`"type_name":"{{ .arguments.name }}"`)
includeDeprecatedField = []byte(`"include_deprecated":{{ .arguments.includeDeprecated }}`)
lBrace = []byte("{")
rBrace = []byte("}")
comma = []byte(",")
requestTypeField = []byte(`"request_type":`)
onTypeField = []byte(`"on_type_name":"{{ .object.name }}"`)
typeNameField = []byte(`"type_name":"{{ .arguments.name }}"`)
includeDeprecatedFieldArgument = []byte(`"include_deprecated":{{ .arguments.includeDeprecated }}`)
includeDeprecatedFalse = []byte(`"include_deprecated":false`)
)

func buildInput(fieldName string) string {
func buildInput(fieldName string, hasIncludeDeprecatedArgument bool) string {
buf := &bytes.Buffer{}
buf.Write(lBrace)

Expand All @@ -49,10 +50,10 @@ func buildInput(fieldName string) string {
buf.Write(typeNameField)
case fieldsFieldName:
writeRequestTypeField(buf, TypeFieldsRequestType)
writeOnTypeFields(buf)
writeOnTypeFields(buf, hasIncludeDeprecatedArgument)
case enumValuesFieldName:
writeRequestTypeField(buf, TypeEnumValuesRequestType)
writeOnTypeFields(buf)
writeOnTypeFields(buf, hasIncludeDeprecatedArgument)
default:
writeRequestTypeField(buf, SchemaRequestType)
}
Expand All @@ -67,9 +68,13 @@ func writeRequestTypeField(buf *bytes.Buffer, inputType requestType) {
buf.Write([]byte(strconv.Itoa(int(inputType))))
}

func writeOnTypeFields(buf *bytes.Buffer) {
func writeOnTypeFields(buf *bytes.Buffer, hasIncludeDeprecatedArgument bool) {
buf.Write(comma)
buf.Write(onTypeField)
buf.Write(comma)
buf.Write(includeDeprecatedField)
if hasIncludeDeprecatedArgument {
buf.Write(includeDeprecatedFieldArgument)
} else {
buf.Write(includeDeprecatedFalse)
}
}
14 changes: 8 additions & 6 deletions v2/pkg/engine/datasource/introspection_datasource/input_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,20 @@ import (
)

func TestBuildInput(t *testing.T) {
run := func(fieldName string, expectedJson string) func(t *testing.T) {
run := func(fieldName string, expectedJson string, hasDeprecatedArg bool) func(t *testing.T) {
t.Helper()
return func(t *testing.T) {
actualResult := buildInput(fieldName)
actualResult := buildInput(fieldName, hasDeprecatedArg)
assert.Equal(t, expectedJson, actualResult)
}
}

t.Run("schema introspection", run(schemaFieldName, `{"request_type":1}`))
t.Run("type introspection", run(typeFieldName, `{"request_type":2,"type_name":"{{ .arguments.name }}"}`))
t.Run("type fields", run(fieldsFieldName, `{"request_type":3,"on_type_name":"{{ .object.name }}","include_deprecated":{{ .arguments.includeDeprecated }}}`))
t.Run("type enum values", run(enumValuesFieldName, `{"request_type":4,"on_type_name":"{{ .object.name }}","include_deprecated":{{ .arguments.includeDeprecated }}}`))
t.Run("schema introspection", run(schemaFieldName, `{"request_type":1}`, false))
t.Run("type introspection", run(typeFieldName, `{"request_type":2,"type_name":"{{ .arguments.name }}"}`, false))
t.Run("type fields", run(fieldsFieldName, `{"request_type":3,"on_type_name":"{{ .object.name }}","include_deprecated":{{ .arguments.includeDeprecated }}}`, true))
t.Run("type enum values", run(enumValuesFieldName, `{"request_type":4,"on_type_name":"{{ .object.name }}","include_deprecated":{{ .arguments.includeDeprecated }}}`, true))
t.Run("type fields default for include deprecated", run(fieldsFieldName, `{"request_type":3,"on_type_name":"{{ .object.name }}","include_deprecated":false}`, false))
t.Run("type enum values for include deprecated", run(enumValuesFieldName, `{"request_type":4,"on_type_name":"{{ .object.name }}","include_deprecated":false}`, false))
}

func TestUnmarshalIntrospectionInput(t *testing.T) {
Expand Down
21 changes: 13 additions & 8 deletions v2/pkg/engine/datasource/introspection_datasource/planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ type Configuration struct {
}

type Planner[T Configuration] struct {
introspectionData *introspection.Data
v *plan.Visitor
rootField int
rootFieldName string
rootFielPath string
isArrayItem bool
introspectionData *introspection.Data
v *plan.Visitor
rootField int
rootFieldName string
rootFielPath string
hasIncludeDeprecatedArgument bool
isArrayItem bool
}

func (p *Planner[T]) UpstreamSchema(dataSourceConfig plan.DataSourceConfiguration[T]) (*ast.Document, bool) {
Expand Down Expand Up @@ -50,15 +51,19 @@ func (p *Planner[T]) EnterField(ref int) {
fieldName := p.v.Operation.FieldNameString(ref)
fieldAliasOrName := p.v.Operation.FieldAliasOrNameString(ref)
switch fieldName {
case typeFieldName, fieldsFieldName, enumValuesFieldName, schemaFieldName:
case fieldsFieldName, enumValuesFieldName:
p.hasIncludeDeprecatedArgument = p.v.Operation.FieldHasArguments(ref)

fallthrough
case typeFieldName, schemaFieldName:
p.rootField = ref
p.rootFieldName = fieldName
p.rootFielPath = fieldAliasOrName
}
}

func (p *Planner[T]) configureInput() string {
return buildInput(p.rootFieldName)
return buildInput(p.rootFieldName, p.hasIncludeDeprecatedArgument)
}

func (p *Planner[T]) ConfigureFetch() resolve.FetchConfiguration {
Expand Down

0 comments on commit b78ed4c

Please sign in to comment.