From ce23ea58ddaeffe20aac59c61571e17cdbb36f10 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Mon, 26 Jan 2026 18:07:03 +0100 Subject: [PATCH 01/21] feat: store field arg maps to variable normalization --- v2/pkg/astnormalization/astnormalization.go | 14 ++- .../astnormalization/astnormalization_test.go | 116 +++++++++++++++++- .../field_argument_mapping.go | 16 +++ .../astnormalization/variables_extraction.go | 51 ++++++++ 4 files changed, 189 insertions(+), 8 deletions(-) create mode 100644 v2/pkg/astnormalization/field_argument_mapping.go diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index 04631f8398..aa73a477e1 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -70,7 +70,6 @@ package astnormalization import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" - "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" ) @@ -382,22 +381,25 @@ func NewVariablesNormalizer() *VariablesNormalizer { } } -func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) []uploads.UploadPathMapping { +func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) VariablesNormalizerResult { v.firstDetectUnused.Walk(operation, definition, report) if report.HasErrors() { - return nil + return VariablesNormalizerResult{} } v.secondExtract.Walk(operation, definition, report) if report.HasErrors() { - return nil + return VariablesNormalizerResult{} } v.thirdDeleteUnused.Walk(operation, definition, report) if report.HasErrors() { - return nil + return VariablesNormalizerResult{} } v.fourthCoerce.Walk(operation, definition, report) - return v.variablesExtractionVisitor.uploadsPath + return VariablesNormalizerResult{ + UploadsMapping: v.variablesExtractionVisitor.uploadsPath, + FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping, + } } type fragmentCycleVisitor struct { diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index 2bc22e84ac..dfe2934446 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1042,7 +1042,7 @@ func TestVariablesNormalizer(t *testing.T) { normalizer := NewVariablesNormalizer() report := operationreport.Report{} - uploadsMapping := normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) + result := normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) require.False(t, report.HasErrors(), report.Error()) out := unsafeprinter.Print(&operationDocument) @@ -1060,7 +1060,119 @@ func TestVariablesNormalizer(t *testing.T) { {VariableName: "a", OriginalUploadPath: "variables.varTwo.oneList.0.value", NewUploadPath: "variables.a.two.oneList.0.value"}, {VariableName: "a", OriginalUploadPath: "variables.varTwo.one.list.0", NewUploadPath: "variables.a.two.one.list.0"}, {VariableName: "a", OriginalUploadPath: "variables.varTwo.one.value", NewUploadPath: "variables.a.two.one.value"}, - }, uploadsMapping) + }, result.UploadsMapping) + + // Verify field argument mapping is populated + assert.Equal(t, "a", result.FieldArgumentMapping["hello.arg"]) + }) + + t.Run("field argument mapping", func(t *testing.T) { + const fieldArgMappingSchema = ` + type Query { + user(id: ID!, active: Boolean): User + users(name: String!, limit: Int!, offset: Int): [User!]! + } + type User { + id: ID! + name: String! + posts(limit: Int!): [Post!]! + } + type Post { + id: ID! + title: String! + } + ` + + testCases := []struct { + name string + operation string + variables string + expectedMapping FieldArgumentMapping + }{ + { + name: "user provided variable", + operation: `query GetUser($userId: ID!) { + user(id: $userId) { name } + }`, + variables: `{"userId": "123"}`, + expectedMapping: FieldArgumentMapping{"user.id": "userId"}, + }, + { + name: "inline literal extracted", + operation: `query GetUser { + user(id: "123") { name } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{"user.id": "a"}, + }, + { + name: "multiple inline literals", + operation: `query GetUsers { + a: user(id: "user-1") { name } + b: user(id: "user-2") { name } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{"a.id": "a", "b.id": "b"}, + }, + { + name: "nested field arguments", + operation: `query GetUserPosts($userId: ID!, $limit: Int!) { + user(id: $userId) { + posts(limit: $limit) { title } + } + }`, + variables: `{"userId": "123", "limit": 10}`, + expectedMapping: FieldArgumentMapping{"user.id": "userId", "user.posts.limit": "limit"}, + }, + { + name: "aliased fields with variables", + operation: `query GetUsers($id1: ID!, $id2: ID!) { + a: user(id: $id1) { name } + b: user(id: $id2) { name } + }`, + variables: `{"id1": "user-1", "id2": "user-2"}`, + expectedMapping: FieldArgumentMapping{"a.id": "id1", "b.id": "id2"}, + }, + { + name: "multiple arguments on single field", + operation: `query SearchUsers($name: String!, $limit: Int!, $offset: Int) { + users(name: $name, limit: $limit, offset: $offset) { id } + }`, + variables: `{"name": "john", "limit": 10, "offset": 5}`, + expectedMapping: FieldArgumentMapping{"users.name": "name", "users.limit": "limit", "users.offset": "offset"}, + }, + { + name: "mixed inline and variables", + operation: `query GetUser($userId: ID!) { + user(id: $userId, active: true) { name } + }`, + variables: `{"userId": "123"}`, + expectedMapping: FieldArgumentMapping{"user.id": "userId", "user.active": "a"}, + }, + { + name: "empty mapping when no arguments", + operation: `query GetUser { + user { name } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + definitionDocument := unsafeparser.ParseGraphqlDocumentStringWithBaseSchema(fieldArgMappingSchema) + operationDocument := unsafeparser.ParseGraphqlDocumentString(tc.operation) + operationDocument.Input.Variables = []byte(tc.variables) + + normalizer := NewVariablesNormalizer() + report := operationreport.Report{} + result := normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) + require.False(t, report.HasErrors(), report.Error()) + + assert.Equal(t, tc.expectedMapping, result.FieldArgumentMapping) + }) + } }) } diff --git a/v2/pkg/astnormalization/field_argument_mapping.go b/v2/pkg/astnormalization/field_argument_mapping.go new file mode 100644 index 0000000000..033aaf9fd7 --- /dev/null +++ b/v2/pkg/astnormalization/field_argument_mapping.go @@ -0,0 +1,16 @@ +package astnormalization + +import "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" + +// FieldArgumentMapping maps field arguments to their variable names. +// Key: "fieldPath.argumentName" (e.g., "user.posts.limit") +// Value: variable name after extraction (e.g., "a", "b", "userId") +type FieldArgumentMapping map[string]string + +// VariablesNormalizerResult contains the results of variable normalization. +type VariablesNormalizerResult struct { + // UploadsMapping tracks file upload variables and how their paths change during normalization. + UploadsMapping []uploads.UploadPathMapping + // FieldArgumentMapping maps field arguments to their variable names for fast lookup. + FieldArgumentMapping FieldArgumentMapping +} diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 6a8e7b8165..ef980a14fa 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -3,6 +3,7 @@ package astnormalization import ( "bytes" "fmt" + "strings" "github.com/buger/jsonparser" "github.com/tidwall/sjson" @@ -34,6 +35,10 @@ type variablesExtractionVisitor struct { extractedVariableTypeRefs []int uploadFinder *uploads.UploadFinder uploadsPath []uploads.UploadPathMapping + // fieldArgumentMapping maps field arguments to their variable names. + // Key: "fieldPath.argumentName" (e.g., "user.posts.limit") + // Value: variable name after extraction (e.g., "a", "b", "userId") + fieldArgumentMapping FieldArgumentMapping } func (v *variablesExtractionVisitor) EnterArgument(ref int) { @@ -63,6 +68,8 @@ func (v *variablesExtractionVisitor) EnterArgument(ref int) { if len(uploadsMapping) > 0 { v.uploadsPath = append(v.uploadsPath, uploadsMapping...) } + // Record the field argument mapping for existing variables + v.recordFieldArgumentMapping(ref) return } @@ -141,12 +148,56 @@ func (v *variablesExtractionVisitor) EnterArgument(ref int) { v.operation.OperationDefinitions[v.Ancestors[0].Ref].VariableDefinitions.Refs = append(v.operation.OperationDefinitions[v.Ancestors[0].Ref].VariableDefinitions.Refs, newVariableRef) v.operation.OperationDefinitions[v.Ancestors[0].Ref].HasVariableDefinitions = true + + // Record the field argument mapping for the newly extracted variable + v.recordFieldArgumentMappingWithVarName(ref, string(variableNameBytes)) } func (v *variablesExtractionVisitor) EnterDocument(operation, definition *ast.Document) { v.operation, v.definition = operation, definition v.extractedVariables = v.extractedVariables[:0] v.extractedVariableTypeRefs = v.extractedVariableTypeRefs[:0] + v.fieldArgumentMapping = make(FieldArgumentMapping) +} + +// buildFieldPath builds the field path from the walker's ancestors. +// It returns a dot-separated path of field names/aliases (e.g., "user.posts"). +func (v *variablesExtractionVisitor) buildFieldPath() string { + var parts []string + for _, anc := range v.Ancestors { + if anc.Kind == ast.NodeKindField { + alias := v.operation.FieldAliasString(anc.Ref) + if alias != "" { + parts = append(parts, alias) + } else { + parts = append(parts, v.operation.FieldNameString(anc.Ref)) + } + } + } + return strings.Join(parts, ".") +} + +// recordFieldArgumentMapping records the field argument mapping for an existing variable. +func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int) { + fieldPath := v.buildFieldPath() + if fieldPath == "" { + return + } + argName := v.operation.ArgumentNameString(ref) + key := fieldPath + "." + argName + varName := v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) + v.fieldArgumentMapping[key] = varName +} + +// recordFieldArgumentMappingWithVarName records the field argument mapping for a newly extracted variable. +func (v *variablesExtractionVisitor) recordFieldArgumentMappingWithVarName(ref int, varName string) { + fieldPath := v.buildFieldPath() + if fieldPath == "" { + return + } + argName := v.operation.ArgumentNameString(ref) + key := fieldPath + "." + argName + v.fieldArgumentMapping[key] = varName } func (v *variablesExtractionVisitor) variableExists(variableValue []byte, inputValueDefinition int) (exists bool, name []byte, definition int) { From 1ebb2c30eb51d6dd021e5445e7758ca7fe12ea6f Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Tue, 27 Jan 2026 14:41:05 +0100 Subject: [PATCH 02/21] chore: reduce two functions to one --- .../astnormalization/variables_extraction.go | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index ef980a14fa..98ae2eec1d 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -69,7 +69,7 @@ func (v *variablesExtractionVisitor) EnterArgument(ref int) { v.uploadsPath = append(v.uploadsPath, uploadsMapping...) } // Record the field argument mapping for existing variables - v.recordFieldArgumentMapping(ref) + v.recordFieldArgumentMapping(ref, "") return } @@ -150,7 +150,7 @@ func (v *variablesExtractionVisitor) EnterArgument(ref int) { v.operation.OperationDefinitions[v.Ancestors[0].Ref].HasVariableDefinitions = true // Record the field argument mapping for the newly extracted variable - v.recordFieldArgumentMappingWithVarName(ref, string(variableNameBytes)) + v.recordFieldArgumentMapping(ref, string(variableNameBytes)) } func (v *variablesExtractionVisitor) EnterDocument(operation, definition *ast.Document) { @@ -177,23 +177,16 @@ func (v *variablesExtractionVisitor) buildFieldPath() string { return strings.Join(parts, ".") } -// recordFieldArgumentMapping records the field argument mapping for an existing variable. -func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int) { +// recordFieldArgumentMapping records the currently visited field argument +// of v in v.fieldArgumentMapping, alongside its matching variable name. +// If varName is empty, it looks up the variable name from the operation. +func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName string) { fieldPath := v.buildFieldPath() if fieldPath == "" { return } - argName := v.operation.ArgumentNameString(ref) - key := fieldPath + "." + argName - varName := v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) - v.fieldArgumentMapping[key] = varName -} - -// recordFieldArgumentMappingWithVarName records the field argument mapping for a newly extracted variable. -func (v *variablesExtractionVisitor) recordFieldArgumentMappingWithVarName(ref int, varName string) { - fieldPath := v.buildFieldPath() - if fieldPath == "" { - return + if varName == "" { + varName = v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) } argName := v.operation.ArgumentNameString(ref) key := fieldPath + "." + argName From ea5c6306aee78b87728fb500cbdb356463828b44 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Wed, 28 Jan 2026 18:04:57 +0100 Subject: [PATCH 03/21] fix: handle fragments --- v2/pkg/ast/path.go | 6 +- .../astnormalization/astnormalization_test.go | 177 +++++++++++++++--- .../astnormalization/variables_extraction.go | 23 +-- ...ation_rule_validate_empty_selection_set.go | 2 +- .../graphql_datasource/graphql_datasource.go | 2 +- .../representation_variable.go | 2 +- .../plan/abstract_selection_rewriter.go | 2 +- ...datasource_filter_collect_nodes_visitor.go | 4 +- .../datasource_filter_resolvable_visitor.go | 2 +- v2/pkg/engine/plan/key_fields_visitor.go | 2 +- v2/pkg/engine/plan/node_selection_visitor.go | 2 +- v2/pkg/engine/plan/path_builder_visitor.go | 12 +- v2/pkg/engine/plan/provides_fields_visitor.go | 2 +- v2/pkg/engine/plan/required_fields_visitor.go | 2 +- v2/pkg/engine/plan/visitor.go | 6 +- 15 files changed, 182 insertions(+), 64 deletions(-) diff --git a/v2/pkg/ast/path.go b/v2/pkg/ast/path.go index 19493ec60c..130c883f36 100644 --- a/v2/pkg/ast/path.go +++ b/v2/pkg/ast/path.go @@ -128,7 +128,7 @@ func (p Path) String() string { return out } -func (p Path) DotDelimitedString() string { +func (p Path) DotDelimitedString(includeFragmentRef bool) string { builder := strings.Builder{} toGrow := 0 @@ -160,7 +160,9 @@ func (p Path) DotDelimitedString() string { } case InlineFragmentName: builder.WriteString(InlineFragmentPathPrefix) - builder.WriteString(strconv.Itoa(p[i].FragmentRef)) + if includeFragmentRef { + builder.WriteString(strconv.Itoa(p[i].FragmentRef)) + } builder.WriteString(unsafebytes.BytesToString(p[i].FieldName)) } } diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index dfe2934446..1634b38756 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1063,7 +1063,7 @@ func TestVariablesNormalizer(t *testing.T) { }, result.UploadsMapping) // Verify field argument mapping is populated - assert.Equal(t, "a", result.FieldArgumentMapping["hello.arg"]) + assert.Equal(t, "a", result.FieldArgumentMapping["mutation.hello.arg"]) }) t.Run("field argument mapping", func(t *testing.T) { @@ -1071,16 +1071,36 @@ func TestVariablesNormalizer(t *testing.T) { type Query { user(id: ID!, active: Boolean): User users(name: String!, limit: Int!, offset: Int): [User!]! + node(id: ID!): Node } - type User { + interface Node { + id: ID! + User: UserContainer + } + type UserContainer { + posts(limit: Int!): [Post!]! + } + type User implements Node { id: ID! name: String! posts(limit: Int!): [Post!]! + items(limit: Int!): [Item!]! + User: UserContainer + } + type Admin implements Node { + id: ID! + role: String! + items(limit: Int!): [Item!]! + User: UserContainer } type Post { id: ID! title: String! } + type Item { + id: ID! + name: String! + } ` testCases := []struct { @@ -1090,64 +1110,80 @@ func TestVariablesNormalizer(t *testing.T) { expectedMapping FieldArgumentMapping }{ { - name: "user provided variable", + name: "mapping references the name of the variable", operation: `query GetUser($userId: ID!) { user(id: $userId) { name } }`, variables: `{"userId": "123"}`, - expectedMapping: FieldArgumentMapping{"user.id": "userId"}, + expectedMapping: FieldArgumentMapping{"query.user.id": "userId"}, }, { - name: "inline literal extracted", + name: "mapping references an extracted variable on literal values", operation: `query GetUser { user(id: "123") { name } }`, variables: `{}`, - expectedMapping: FieldArgumentMapping{"user.id": "a"}, + expectedMapping: FieldArgumentMapping{"query.user.id": "a"}, }, { - name: "multiple inline literals", + name: "mapping references an extracted variable on multiple literal values", operation: `query GetUsers { - a: user(id: "user-1") { name } - b: user(id: "user-2") { name } + firstAlias: user(id: "user-1") { name } + secondAlias: user(id: "user-2") { name } }`, - variables: `{}`, - expectedMapping: FieldArgumentMapping{"a.id": "a", "b.id": "b"}, + variables: `{}`, + expectedMapping: FieldArgumentMapping{ + "query.firstAlias.id": "a", + "query.secondAlias.id": "b", + }, }, { - name: "nested field arguments", + name: "mapping references an extracted variable on multiple variable values", + operation: `query GetUsers($id1: ID!, $id2: ID!) { + firstAlias: user(id: $id1) { name } + secondAlias: user(id: $id2) { name } + }`, + variables: `{"id1": "user-1", "id2": "user-2"}`, + expectedMapping: FieldArgumentMapping{ + "query.firstAlias.id": "id1", + "query.secondAlias.id": "id2", + }, + }, + { + name: "mapping correctly builds paths on nested field arguments", operation: `query GetUserPosts($userId: ID!, $limit: Int!) { user(id: $userId) { posts(limit: $limit) { title } } }`, - variables: `{"userId": "123", "limit": 10}`, - expectedMapping: FieldArgumentMapping{"user.id": "userId", "user.posts.limit": "limit"}, - }, - { - name: "aliased fields with variables", - operation: `query GetUsers($id1: ID!, $id2: ID!) { - a: user(id: $id1) { name } - b: user(id: $id2) { name } - }`, - variables: `{"id1": "user-1", "id2": "user-2"}`, - expectedMapping: FieldArgumentMapping{"a.id": "id1", "b.id": "id2"}, + variables: `{"userId": "123", "limit": 10}`, + expectedMapping: FieldArgumentMapping{ + "query.user.id": "userId", + "query.user.posts.limit": "limit", + }, }, { - name: "multiple arguments on single field", + name: "multiple variables refs used correctly in mapping", operation: `query SearchUsers($name: String!, $limit: Int!, $offset: Int) { users(name: $name, limit: $limit, offset: $offset) { id } }`, - variables: `{"name": "john", "limit": 10, "offset": 5}`, - expectedMapping: FieldArgumentMapping{"users.name": "name", "users.limit": "limit", "users.offset": "offset"}, + variables: `{"name": "john", "limit": 10, "offset": 5}`, + expectedMapping: FieldArgumentMapping{ + "query.users.name": "name", + "query.users.limit": "limit", + "query.users.offset": "offset", + }, }, { - name: "mixed inline and variables", + name: "mixed inline and variables get correctly mapped", operation: `query GetUser($userId: ID!) { user(id: $userId, active: true) { name } }`, - variables: `{"userId": "123"}`, - expectedMapping: FieldArgumentMapping{"user.id": "userId", "user.active": "a"}, + variables: `{"userId": "123"}`, + expectedMapping: FieldArgumentMapping{ + "query.user.id": "userId", + "query.user.active": "a", + }, }, { name: "empty mapping when no arguments", @@ -1157,6 +1193,89 @@ func TestVariablesNormalizer(t *testing.T) { variables: `{}`, expectedMapping: FieldArgumentMapping{}, }, + { + name: "inline fragment prefixed with dollar sign in paths", + operation: `query GetNode($nodeId: ID!, $limit: Int!) { + node(id: $nodeId) { + ... on User { + posts(limit: $limit) { title } + } + } + }`, + variables: `{"nodeId": "123", "limit": 10}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": "nodeId", + "query.node.$User.posts.limit": "limit", + }, + }, + { + name: "same field in different inline fragments requires type name to avoid collision", + operation: `query GetNode($nodeId: ID!) { + node(id: $nodeId) { + ... on User { + items(limit: 10) { name } + } + ... on Admin { + items(limit: 5) { name } + } + } + }`, + variables: `{"nodeId": "123"}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": "nodeId", + "query.node.$User.items.limit": "a", + "query.node.$Admin.items.limit": "b", + }, + }, + { + name: "inline fragment and field with same name requires prefix to avoid collision", + operation: `query GetNode($nodeId: ID!) { + node(id: $nodeId) { + ... on User { + posts(limit: 10) { title } + } + User { + posts(limit: 5) { title } + } + } + }`, + variables: `{"nodeId": "123"}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": "nodeId", + "query.node.$User.posts.limit": "a", + "query.node.User.posts.limit": "b", + }, + }, + { + name: "named fragment with variable field arg value", + operation: `fragment UserFieldsPost on User { + posts(limit: $limitVar) { title } + } + query GetUser($limitVar: Int!) { + user { + ...UserFieldsPost + } + }`, + variables: `{"limitVar": 11}`, + expectedMapping: FieldArgumentMapping{ + "User.posts.limit": "limitVar", + }, + }, + { + name: "named fragment with literal field arg value", + operation: `fragment UserFieldsPost on User { + posts(limit: 10) { title } + } + query GetUser { + user { + ...UserFieldsPost + } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{ + "User.posts.limit": "a", + }, + }, } for _, tc := range testCases { diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 98ae2eec1d..847fa5a37b 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -3,7 +3,6 @@ package astnormalization import ( "bytes" "fmt" - "strings" "github.com/buger/jsonparser" "github.com/tidwall/sjson" @@ -42,6 +41,11 @@ type variablesExtractionVisitor struct { } func (v *variablesExtractionVisitor) EnterArgument(ref int) { + if v.Ancestors[0].Kind == ast.NodeKindFragmentDefinition { + v.recordFieldArgumentMapping(ref, "") + return + } + if len(v.Ancestors) == 0 || v.Ancestors[0].Kind != ast.NodeKindOperationDefinition { return } @@ -163,18 +167,7 @@ func (v *variablesExtractionVisitor) EnterDocument(operation, definition *ast.Do // buildFieldPath builds the field path from the walker's ancestors. // It returns a dot-separated path of field names/aliases (e.g., "user.posts"). func (v *variablesExtractionVisitor) buildFieldPath() string { - var parts []string - for _, anc := range v.Ancestors { - if anc.Kind == ast.NodeKindField { - alias := v.operation.FieldAliasString(anc.Ref) - if alias != "" { - parts = append(parts, alias) - } else { - parts = append(parts, v.operation.FieldNameString(anc.Ref)) - } - } - } - return strings.Join(parts, ".") + return v.Path.DotDelimitedString(false) } // recordFieldArgumentMapping records the currently visited field argument @@ -186,6 +179,10 @@ func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName return } if varName == "" { + // TODO handle literals on named fragments + if v.operation.Arguments[ref].Value.Kind != ast.ValueKindVariable { + return + } varName = v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) } argName := v.operation.ArgumentNameString(ref) diff --git a/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go b/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go index ce6dd4e8c4..eb71e593b4 100644 --- a/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go +++ b/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go @@ -32,6 +32,6 @@ func (r *emptySelectionSetVisitor) EnterDocument(operation, definition *ast.Docu func (r *emptySelectionSetVisitor) EnterSelectionSet(ref int) { if r.operation.SelectionSetIsEmpty(ref) { - r.Walker.StopWithInternalErr(fmt.Errorf("astvalidation selection set on path %s is empty", r.Walker.Path.DotDelimitedString())) + r.Walker.StopWithInternalErr(fmt.Errorf("astvalidation selection set on path %s is empty", r.Walker.Path.DotDelimitedString(true))) } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 23dea1a6ac..97af5e372b 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -783,7 +783,7 @@ func (p *Planner[T]) allowField(ref int) bool { // In addition, we skip field if its path are equal to planner parent path // This is required to correctly plan on datasource which has corresponding child/root node, // but we don't need to add it to the query as we are in the nested request - currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(), fieldAliasOrName) + currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(true), fieldAliasOrName) if p.dataSourcePlannerConfig.ParentPath != "query" && p.dataSourcePlannerConfig.ParentPath == currentPath { p.DebugPrint("allowField: false path:", currentPath, "is equal to parent path", p.dataSourcePlannerConfig.ParentPath) return false diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go index 18cb3ad204..83ad1a6ece 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go @@ -210,7 +210,7 @@ func (v *representationVariableVisitor) EnterField(ref int) { } fieldDefinitionType := v.definition.FieldDefinitionType(fieldDefinition) - currentPath := v.Walker.Path.DotDelimitedString() + "." + string(fieldName) + currentPath := v.Walker.Path.DotDelimitedString(true) + "." + string(fieldName) // if path was remapped during the planning we update its path fieldPath := string(fieldName) diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter.go b/v2/pkg/engine/plan/abstract_selection_rewriter.go index 65c0f87011..c98298c622 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter.go @@ -694,7 +694,7 @@ type AbstractFieldPathCollector struct { } func (v *AbstractFieldPathCollector) EnterField(ref int) { - parentPath := v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + parentPath := v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) currentFieldName := v.operation.FieldNameString(ref) currentPath := parentPath + "." + currentFieldName diff --git a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go index b9d47ad3ba..2d03b6a9ca 100644 --- a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go @@ -577,9 +577,9 @@ func (f *treeBuilderVisitor) collectFieldInfo(fieldRef int) { fieldName := f.operation.FieldNameUnsafeString(fieldRef) fieldAliasOrName := f.operation.FieldAliasOrNameString(fieldRef) isTypeName := fieldName == typeNameField - parentPath := f.walker.Path.DotDelimitedString() + parentPath := f.walker.Path.DotDelimitedString(true) onFragment := f.walker.Path.EndsWithFragment() - parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) currentPath := fmt.Sprintf("%s.%s", parentPath, fieldAliasOrName) currentPathWithoutFragments := fmt.Sprintf("%s.%s", parentPathWithoutFragment, fieldAliasOrName) diff --git a/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go b/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go index 8efbe79960..4a392bbb26 100644 --- a/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go @@ -42,7 +42,7 @@ func (f *nodesResolvableVisitor) EnterField(ref int) { } } - parentPath := f.walker.Path.DotDelimitedString() + parentPath := f.walker.Path.DotDelimitedString(true) currentPath := parentPath + "." + fieldAliasOrName _, found := f.nodes.HasSuggestionForPath(typeName, fieldName, currentPath) diff --git a/v2/pkg/engine/plan/key_fields_visitor.go b/v2/pkg/engine/plan/key_fields_visitor.go index 1e018d56dd..29bb05f5ca 100644 --- a/v2/pkg/engine/plan/key_fields_visitor.go +++ b/v2/pkg/engine/plan/key_fields_visitor.go @@ -198,7 +198,7 @@ func (v *keyInfoVisitor) EnterField(ref int) { fieldName := v.input.key.FieldNameUnsafeString(ref) typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) - parentPath := v.input.parentPath + strings.TrimPrefix(v.walker.Path.DotDelimitedString(), v.input.typeName) + parentPath := v.input.parentPath + strings.TrimPrefix(v.walker.Path.DotDelimitedString(true), v.input.typeName) currentPath := parentPath + "." + fieldName hasRootNode := v.input.dataSource.HasRootNode(typeName, fieldName) diff --git a/v2/pkg/engine/plan/node_selection_visitor.go b/v2/pkg/engine/plan/node_selection_visitor.go index 8f9f80fb48..c2a09e8b82 100644 --- a/v2/pkg/engine/plan/node_selection_visitor.go +++ b/v2/pkg/engine/plan/node_selection_visitor.go @@ -222,7 +222,7 @@ func (c *nodeSelectionVisitor) handleEnterField(fieldRef int, handleRequires boo c.debugPrint("EnterField ref:", fieldRef, "fieldName:", fieldName, "typeName:", typeName, "requires:", handleRequires) - parentPath := c.walker.Path.DotDelimitedString() + parentPath := c.walker.Path.DotDelimitedString(true) currentPath := parentPath + "." + fieldAliasOrName suggestions := c.nodeSuggestions.SuggestionsForPath(typeName, fieldName, currentPath) diff --git a/v2/pkg/engine/plan/path_builder_visitor.go b/v2/pkg/engine/plan/path_builder_visitor.go index 50f74d0ae3..78f40d3800 100644 --- a/v2/pkg/engine/plan/path_builder_visitor.go +++ b/v2/pkg/engine/plan/path_builder_visitor.go @@ -362,8 +362,8 @@ func (c *pathBuilderVisitor) EnterSelectionSet(ref int) { return } - parentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString() - currentPath := c.walker.Path.DotDelimitedString() + parentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString(true) + currentPath := c.walker.Path.DotDelimitedString(true) typeName := c.operation.InlineFragmentTypeConditionNameString(ancestor.Ref) node, ok := c.definition.NodeByNameStr(typeName) @@ -440,7 +440,7 @@ func (c *pathBuilderVisitor) EnterField(fieldRef int) { c.debugPrint("EnterField ref:", fieldRef, "fieldName:", fieldName, "typeName:", typeName) - parentPath := c.walker.Path.DotDelimitedString() + parentPath := c.walker.Path.DotDelimitedString(true) // we need to also check preceding path for inline fragments // as for the field within inline fragment the parent path will include type condition in a path // but planner path still will not include it @@ -458,7 +458,7 @@ func (c *pathBuilderVisitor) EnterField(fieldRef int) { precedingPath = c.walker.Path[:i-1] } if precedingPath != nil { - precedingParentPath = precedingPath.DotDelimitedString() + precedingParentPath = precedingPath.DotDelimitedString(true) } currentPath := parentPath + "." + fieldAliasOrName @@ -630,7 +630,7 @@ func (c *pathBuilderVisitor) fieldHasAuthorizationRule(typeName, fieldName strin } func (c *pathBuilderVisitor) fieldIsChildNode(plannerIdx int) bool { - path := c.walker.Path.DotDelimitedString() + path := c.walker.Path.DotDelimitedString(true) plannerPath := c.planners[plannerIdx].ParentPath() fieldPath := strings.TrimPrefix(path, plannerPath) return strings.ContainsAny(fieldPath, ".") @@ -926,7 +926,7 @@ func (c *pathBuilderVisitor) addNewPlanner(fieldRef int, typeName, fieldName, cu plannerPath := parentPath if isParentFragment { - precedingFragmentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString() + precedingFragmentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString(true) // if the parent is a fragment, we add the preceding parent path as well // to be able to walk selection sets in the fragment paths = append([]pathConfiguration{ diff --git a/v2/pkg/engine/plan/provides_fields_visitor.go b/v2/pkg/engine/plan/provides_fields_visitor.go index 50cf19eed2..d28c1fec7c 100644 --- a/v2/pkg/engine/plan/provides_fields_visitor.go +++ b/v2/pkg/engine/plan/provides_fields_visitor.go @@ -101,7 +101,7 @@ func (v *providesVisitor) EnterField(ref int) { fieldName := v.input.providesFieldSet.FieldNameUnsafeString(ref) typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) - currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) parentPath := v.input.parentPath + strings.TrimPrefix(currentPathWithoutFragments, v.pathPrefix) currentPath := parentPath + "." + fieldName diff --git a/v2/pkg/engine/plan/required_fields_visitor.go b/v2/pkg/engine/plan/required_fields_visitor.go index 2123605015..597460a8d0 100644 --- a/v2/pkg/engine/plan/required_fields_visitor.go +++ b/v2/pkg/engine/plan/required_fields_visitor.go @@ -319,7 +319,7 @@ func (v *requiredFieldsVisitor) addRequiredField(keyRef int, fieldName ast.ByteS Name: v.config.operation.Input.AppendInputBytes(fullAliasName), } - currentPath := v.Walker.Path.DotDelimitedString() + "." + string(fieldName) + currentPath := v.Walker.Path.DotDelimitedString(true) + "." + string(fieldName) v.mapping[currentPath] = string(fullAliasName) } diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index 1f8a469d05..3a82cfc898 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -145,7 +145,7 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any v.pathCache[kind] = make(map[int]string) } if path == "" { - path = v.Walker.Path.DotDelimitedString() + path = v.Walker.Path.DotDelimitedString(true) if kind == astvisitor.EnterField || kind == astvisitor.LeaveField { path = path + "." + v.Operation.FieldAliasOrNameString(ref) } @@ -253,9 +253,9 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any } func (v *Visitor) currentFullPath(skipFragments bool) string { - path := v.Walker.Path.DotDelimitedString() + path := v.Walker.Path.DotDelimitedString(true) if skipFragments { - path = v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + path = v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) } if v.Walker.CurrentKind == ast.NodeKindField { From 73a4ac5a72ef9260c274c69f1f8044e66b70622f Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 29 Jan 2026 14:00:08 +0100 Subject: [PATCH 04/21] fix: support literal values --- .../astnormalization/astnormalization_test.go | 308 +++++++++--------- .../field_argument_mapping.go | 15 +- .../astnormalization/variables_extraction.go | 38 ++- 3 files changed, 189 insertions(+), 172 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index 1634b38756..e3a02ec8ec 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1063,7 +1063,7 @@ func TestVariablesNormalizer(t *testing.T) { }, result.UploadsMapping) // Verify field argument mapping is populated - assert.Equal(t, "a", result.FieldArgumentMapping["mutation.hello.arg"]) + assert.Equal(t, FieldArgumentValue{VariableName: "a"}, result.FieldArgumentMapping["mutation.hello.arg"]) }) t.Run("field argument mapping", func(t *testing.T) { @@ -1109,158 +1109,158 @@ func TestVariablesNormalizer(t *testing.T) { variables string expectedMapping FieldArgumentMapping }{ - { - name: "mapping references the name of the variable", - operation: `query GetUser($userId: ID!) { - user(id: $userId) { name } - }`, - variables: `{"userId": "123"}`, - expectedMapping: FieldArgumentMapping{"query.user.id": "userId"}, - }, - { - name: "mapping references an extracted variable on literal values", - operation: `query GetUser { - user(id: "123") { name } - }`, - variables: `{}`, - expectedMapping: FieldArgumentMapping{"query.user.id": "a"}, - }, - { - name: "mapping references an extracted variable on multiple literal values", - operation: `query GetUsers { - firstAlias: user(id: "user-1") { name } - secondAlias: user(id: "user-2") { name } - }`, - variables: `{}`, - expectedMapping: FieldArgumentMapping{ - "query.firstAlias.id": "a", - "query.secondAlias.id": "b", - }, - }, - { - name: "mapping references an extracted variable on multiple variable values", - operation: `query GetUsers($id1: ID!, $id2: ID!) { - firstAlias: user(id: $id1) { name } - secondAlias: user(id: $id2) { name } - }`, - variables: `{"id1": "user-1", "id2": "user-2"}`, - expectedMapping: FieldArgumentMapping{ - "query.firstAlias.id": "id1", - "query.secondAlias.id": "id2", - }, - }, - { - name: "mapping correctly builds paths on nested field arguments", - operation: `query GetUserPosts($userId: ID!, $limit: Int!) { - user(id: $userId) { - posts(limit: $limit) { title } - } - }`, - variables: `{"userId": "123", "limit": 10}`, - expectedMapping: FieldArgumentMapping{ - "query.user.id": "userId", - "query.user.posts.limit": "limit", - }, - }, - { - name: "multiple variables refs used correctly in mapping", - operation: `query SearchUsers($name: String!, $limit: Int!, $offset: Int) { - users(name: $name, limit: $limit, offset: $offset) { id } - }`, - variables: `{"name": "john", "limit": 10, "offset": 5}`, - expectedMapping: FieldArgumentMapping{ - "query.users.name": "name", - "query.users.limit": "limit", - "query.users.offset": "offset", - }, - }, - { - name: "mixed inline and variables get correctly mapped", - operation: `query GetUser($userId: ID!) { - user(id: $userId, active: true) { name } - }`, - variables: `{"userId": "123"}`, - expectedMapping: FieldArgumentMapping{ - "query.user.id": "userId", - "query.user.active": "a", - }, - }, - { - name: "empty mapping when no arguments", - operation: `query GetUser { - user { name } - }`, - variables: `{}`, - expectedMapping: FieldArgumentMapping{}, - }, - { - name: "inline fragment prefixed with dollar sign in paths", - operation: `query GetNode($nodeId: ID!, $limit: Int!) { - node(id: $nodeId) { - ... on User { - posts(limit: $limit) { title } - } - } - }`, - variables: `{"nodeId": "123", "limit": 10}`, - expectedMapping: FieldArgumentMapping{ - "query.node.id": "nodeId", - "query.node.$User.posts.limit": "limit", - }, - }, - { - name: "same field in different inline fragments requires type name to avoid collision", - operation: `query GetNode($nodeId: ID!) { - node(id: $nodeId) { - ... on User { - items(limit: 10) { name } - } - ... on Admin { - items(limit: 5) { name } - } - } - }`, - variables: `{"nodeId": "123"}`, - expectedMapping: FieldArgumentMapping{ - "query.node.id": "nodeId", - "query.node.$User.items.limit": "a", - "query.node.$Admin.items.limit": "b", - }, - }, - { - name: "inline fragment and field with same name requires prefix to avoid collision", - operation: `query GetNode($nodeId: ID!) { - node(id: $nodeId) { - ... on User { - posts(limit: 10) { title } - } - User { - posts(limit: 5) { title } - } - } - }`, - variables: `{"nodeId": "123"}`, - expectedMapping: FieldArgumentMapping{ - "query.node.id": "nodeId", - "query.node.$User.posts.limit": "a", - "query.node.User.posts.limit": "b", - }, - }, - { - name: "named fragment with variable field arg value", - operation: `fragment UserFieldsPost on User { - posts(limit: $limitVar) { title } - } - query GetUser($limitVar: Int!) { - user { - ...UserFieldsPost - } - }`, - variables: `{"limitVar": 11}`, - expectedMapping: FieldArgumentMapping{ - "User.posts.limit": "limitVar", - }, - }, + // { + // name: "mapping references the name of the variable", + // operation: `query GetUser($userId: ID!) { + // user(id: $userId) { name } + // }`, + // variables: `{"userId": "123"}`, + // expectedMapping: FieldArgumentMapping{"query.user.id": FieldArgumentValue{VariableName: "userId"}}, + // }, + // { + // name: "mapping references an extracted variable on literal values", + // operation: `query GetUser { + // user(id: "123") { name } + // }`, + // variables: `{}`, + // expectedMapping: FieldArgumentMapping{"query.user.id": FieldArgumentValue{VariableName: "a"}}, + // }, + // { + // name: "mapping references an extracted variable on multiple literal values", + // operation: `query GetUsers { + // firstAlias: user(id: "user-1") { name } + // secondAlias: user(id: "user-2") { name } + // }`, + // variables: `{}`, + // expectedMapping: FieldArgumentMapping{ + // "query.firstAlias.id": FieldArgumentValue{VariableName: "a"}, + // "query.secondAlias.id": FieldArgumentValue{VariableName: "b"}, + // }, + // }, + // { + // name: "mapping references an extracted variable on multiple variable values", + // operation: `query GetUsers($id1: ID!, $id2: ID!) { + // firstAlias: user(id: $id1) { name } + // secondAlias: user(id: $id2) { name } + // }`, + // variables: `{"id1": "user-1", "id2": "user-2"}`, + // expectedMapping: FieldArgumentMapping{ + // "query.firstAlias.id": FieldArgumentValue{VariableName: "id1"}, + // "query.secondAlias.id": FieldArgumentValue{VariableName: "id2"}, + // }, + // }, + // { + // name: "mapping correctly builds paths on nested field arguments", + // operation: `query GetUserPosts($userId: ID!, $limit: Int!) { + // user(id: $userId) { + // posts(limit: $limit) { title } + // } + // }`, + // variables: `{"userId": "123", "limit": 10}`, + // expectedMapping: FieldArgumentMapping{ + // "query.user.id": FieldArgumentValue{VariableName: "userId"}, + // "query.user.posts.limit": FieldArgumentValue{VariableName: "limit"}, + // }, + // }, + // { + // name: "multiple variables refs used correctly in mapping", + // operation: `query SearchUsers($name: String!, $limit: Int!, $offset: Int) { + // users(name: $name, limit: $limit, offset: $offset) { id } + // }`, + // variables: `{"name": "john", "limit": 10, "offset": 5}`, + // expectedMapping: FieldArgumentMapping{ + // "query.users.name": FieldArgumentValue{VariableName: "name"}, + // "query.users.limit": FieldArgumentValue{VariableName: "limit"}, + // "query.users.offset": FieldArgumentValue{VariableName: "offset"}, + // }, + // }, + // { + // name: "mixed inline and variables get correctly mapped", + // operation: `query GetUser($userId: ID!) { + // user(id: $userId, active: true) { name } + // }`, + // variables: `{"userId": "123"}`, + // expectedMapping: FieldArgumentMapping{ + // "query.user.id": FieldArgumentValue{VariableName: "userId"}, + // "query.user.active": FieldArgumentValue{VariableName: "a"}, + // }, + // }, + // { + // name: "empty mapping when no arguments", + // operation: `query GetUser { + // user { name } + // }`, + // variables: `{}`, + // expectedMapping: FieldArgumentMapping{}, + // }, + // { + // name: "inline fragment prefixed with dollar sign in paths", + // operation: `query GetNode($nodeId: ID!, $limit: Int!) { + // node(id: $nodeId) { + // ... on User { + // posts(limit: $limit) { title } + // } + // } + // }`, + // variables: `{"nodeId": "123", "limit": 10}`, + // expectedMapping: FieldArgumentMapping{ + // "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, + // "query.node.$User.posts.limit": FieldArgumentValue{VariableName: "limit"}, + // }, + // }, + // { + // name: "same field in different inline fragments requires type name to avoid collision", + // operation: `query GetNode($nodeId: ID!) { + // node(id: $nodeId) { + // ... on User { + // items(limit: 10) { name } + // } + // ... on Admin { + // items(limit: 5) { name } + // } + // } + // }`, + // variables: `{"nodeId": "123"}`, + // expectedMapping: FieldArgumentMapping{ + // "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, + // "query.node.$User.items.limit": FieldArgumentValue{VariableName: "a"}, + // "query.node.$Admin.items.limit": FieldArgumentValue{VariableName: "b"}, + // }, + // }, + // { + // name: "inline fragment and field with same name requires prefix to avoid collision", + // operation: `query GetNode($nodeId: ID!) { + // node(id: $nodeId) { + // ... on User { + // posts(limit: 10) { title } + // } + // User { + // posts(limit: 5) { title } + // } + // } + // }`, + // variables: `{"nodeId": "123"}`, + // expectedMapping: FieldArgumentMapping{ + // "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, + // "query.node.$User.posts.limit": FieldArgumentValue{VariableName: "a"}, + // "query.node.User.posts.limit": FieldArgumentValue{VariableName: "b"}, + // }, + // }, + // { + // name: "named fragment with variable field arg value", + // operation: `fragment UserFieldsPost on User { + // posts(limit: $limitVar) { title } + // } + // query GetUser($limitVar: Int!) { + // user { + // ...UserFieldsPost + // } + // }`, + // variables: `{"limitVar": 11}`, + // expectedMapping: FieldArgumentMapping{ + // "User.posts.limit": FieldArgumentValue{VariableName: "limitVar"}, + // }, + // }, { name: "named fragment with literal field arg value", operation: `fragment UserFieldsPost on User { @@ -1273,7 +1273,7 @@ func TestVariablesNormalizer(t *testing.T) { }`, variables: `{}`, expectedMapping: FieldArgumentMapping{ - "User.posts.limit": "a", + "User.posts.limit": FieldArgumentValue{LiteralValue: []byte("10")}, }, }, } diff --git a/v2/pkg/astnormalization/field_argument_mapping.go b/v2/pkg/astnormalization/field_argument_mapping.go index 033aaf9fd7..63cfa05833 100644 --- a/v2/pkg/astnormalization/field_argument_mapping.go +++ b/v2/pkg/astnormalization/field_argument_mapping.go @@ -2,10 +2,17 @@ package astnormalization import "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" -// FieldArgumentMapping maps field arguments to their variable names. -// Key: "fieldPath.argumentName" (e.g., "user.posts.limit") -// Value: variable name after extraction (e.g., "a", "b", "userId") -type FieldArgumentMapping map[string]string +// FieldArgumentValue represents either a variable reference or a literal value. +// If LiteralValue is non-nil, it's a literal; otherwise use VariableName. +type FieldArgumentValue struct { + VariableName string // Set when value is from a variable (e.g., "userId", "a") + LiteralValue []byte // Set when value is a literal (JSON encoded, e.g., []byte("10")) +} + +// FieldArgumentMapping maps field arguments to their values. +// Key: "fieldPath.argumentName" (e.g., "User.posts.limit") +// Value: either a variable name or a literal value +type FieldArgumentMapping map[string]FieldArgumentValue // VariablesNormalizerResult contains the results of variable normalization. type VariablesNormalizerResult struct { diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 847fa5a37b..2b0a5525ba 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -34,10 +34,7 @@ type variablesExtractionVisitor struct { extractedVariableTypeRefs []int uploadFinder *uploads.UploadFinder uploadsPath []uploads.UploadPathMapping - // fieldArgumentMapping maps field arguments to their variable names. - // Key: "fieldPath.argumentName" (e.g., "user.posts.limit") - // Value: variable name after extraction (e.g., "a", "b", "userId") - fieldArgumentMapping FieldArgumentMapping + fieldArgumentMapping FieldArgumentMapping } func (v *variablesExtractionVisitor) EnterArgument(ref int) { @@ -171,23 +168,36 @@ func (v *variablesExtractionVisitor) buildFieldPath() string { } // recordFieldArgumentMapping records the currently visited field argument -// of v in v.fieldArgumentMapping, alongside its matching variable name. -// If varName is empty, it looks up the variable name from the operation. +// of v in v.fieldArgumentMapping, alongside its matching variable name or literal value. +// If varName is empty, it looks up the variable name from the operation or stores the literal value. func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName string) { fieldPath := v.buildFieldPath() if fieldPath == "" { return } - if varName == "" { - // TODO handle literals on named fragments - if v.operation.Arguments[ref].Value.Kind != ast.ValueKindVariable { - return - } - varName = v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) - } argName := v.operation.ArgumentNameString(ref) key := fieldPath + "." + argName - v.fieldArgumentMapping[key] = varName + + if varName != "" { + // Variable name was provided (from extraction) + v.fieldArgumentMapping[key] = FieldArgumentValue{VariableName: varName} + return + } + + if v.operation.Arguments[ref].Value.Kind == ast.ValueKindVariable { + // Existing variable reference + varName = v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) + v.fieldArgumentMapping[key] = FieldArgumentValue{VariableName: varName} + return + } + + // Literal value - store the JSON-encoded value + valueBytes, err := v.operation.ValueToJSON(v.operation.Arguments[ref].Value) + if err != nil { + return // Skip on error + } + v.fieldArgumentMapping[key] = FieldArgumentValue{LiteralValue: valueBytes} + } func (v *variablesExtractionVisitor) variableExists(variableValue []byte, inputValueDefinition int) (exists bool, name []byte, definition int) { From cd47b128a01e8d2b6ed8b657021c9f1fb34cdc62 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 29 Jan 2026 14:58:31 +0100 Subject: [PATCH 05/21] chore: move mapping to operation normalizer --- v2/pkg/astnormalization/astnormalization.go | 17 ++++++++++++++--- v2/pkg/astnormalization/variables_extraction.go | 5 ----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index aa73a477e1..9ee3a76160 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -106,6 +106,7 @@ type OperationNormalizer struct { operationWalkers []walkerStage removeOperationDefinitionsVisitor *removeOperationDefinitionsVisitor + variablesExtractionVisitor *variablesExtractionVisitor options options definitionNormalizer *DefinitionNormalizer @@ -244,7 +245,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { if o.options.extractVariables { extractVariablesWalker := astvisitor.NewWalkerWithID(8, "ExtractVariables") - extractVariables(&extractVariablesWalker) + o.variablesExtractionVisitor = extractVariables(&extractVariablesWalker) o.operationWalkers = append(o.operationWalkers, walkerStage{ name: "extractVariables", walker: &extractVariablesWalker, @@ -344,6 +345,17 @@ func (o *OperationNormalizer) NormalizeNamedOperation(operation, definition *ast } } +// GetFieldArgumentMapping returns the field argument mapping after normalization. +// Returns nil if extractVariables option was not enabled or normalization hasn't run. +// Note: Paths reflect the normalized operation structure (after fragment inlining), +// e.g., "query.user.$User.posts.limit" for a field originally in a named fragment. +func (o *OperationNormalizer) GetFieldArgumentMapping() FieldArgumentMapping { + if o.variablesExtractionVisitor == nil { + return nil + } + return o.variablesExtractionVisitor.fieldArgumentMapping +} + type VariablesNormalizer struct { firstDetectUnused *astvisitor.Walker secondExtract *astvisitor.Walker @@ -397,8 +409,7 @@ func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Docu v.fourthCoerce.Walk(operation, definition, report) return VariablesNormalizerResult{ - UploadsMapping: v.variablesExtractionVisitor.uploadsPath, - FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping, + UploadsMapping: v.variablesExtractionVisitor.uploadsPath, } } diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 2b0a5525ba..a2975d3093 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -38,11 +38,6 @@ type variablesExtractionVisitor struct { } func (v *variablesExtractionVisitor) EnterArgument(ref int) { - if v.Ancestors[0].Kind == ast.NodeKindFragmentDefinition { - v.recordFieldArgumentMapping(ref, "") - return - } - if len(v.Ancestors) == 0 || v.Ancestors[0].Kind != ast.NodeKindOperationDefinition { return } From 1b8bddc7163a203ac252d55948401736fd3b925d Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 29 Jan 2026 16:49:15 +0100 Subject: [PATCH 06/21] Revert "chore: move mapping to operation normalizer" This reverts commit cd47b128a01e8d2b6ed8b657021c9f1fb34cdc62. --- v2/pkg/astnormalization/astnormalization.go | 17 +++-------------- v2/pkg/astnormalization/variables_extraction.go | 5 +++++ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index 9ee3a76160..aa73a477e1 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -106,7 +106,6 @@ type OperationNormalizer struct { operationWalkers []walkerStage removeOperationDefinitionsVisitor *removeOperationDefinitionsVisitor - variablesExtractionVisitor *variablesExtractionVisitor options options definitionNormalizer *DefinitionNormalizer @@ -245,7 +244,7 @@ func (o *OperationNormalizer) setupOperationWalkers() { if o.options.extractVariables { extractVariablesWalker := astvisitor.NewWalkerWithID(8, "ExtractVariables") - o.variablesExtractionVisitor = extractVariables(&extractVariablesWalker) + extractVariables(&extractVariablesWalker) o.operationWalkers = append(o.operationWalkers, walkerStage{ name: "extractVariables", walker: &extractVariablesWalker, @@ -345,17 +344,6 @@ func (o *OperationNormalizer) NormalizeNamedOperation(operation, definition *ast } } -// GetFieldArgumentMapping returns the field argument mapping after normalization. -// Returns nil if extractVariables option was not enabled or normalization hasn't run. -// Note: Paths reflect the normalized operation structure (after fragment inlining), -// e.g., "query.user.$User.posts.limit" for a field originally in a named fragment. -func (o *OperationNormalizer) GetFieldArgumentMapping() FieldArgumentMapping { - if o.variablesExtractionVisitor == nil { - return nil - } - return o.variablesExtractionVisitor.fieldArgumentMapping -} - type VariablesNormalizer struct { firstDetectUnused *astvisitor.Walker secondExtract *astvisitor.Walker @@ -409,7 +397,8 @@ func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Docu v.fourthCoerce.Walk(operation, definition, report) return VariablesNormalizerResult{ - UploadsMapping: v.variablesExtractionVisitor.uploadsPath, + UploadsMapping: v.variablesExtractionVisitor.uploadsPath, + FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping, } } diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index a2975d3093..2b0a5525ba 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -38,6 +38,11 @@ type variablesExtractionVisitor struct { } func (v *variablesExtractionVisitor) EnterArgument(ref int) { + if v.Ancestors[0].Kind == ast.NodeKindFragmentDefinition { + v.recordFieldArgumentMapping(ref, "") + return + } + if len(v.Ancestors) == 0 || v.Ancestors[0].Kind != ast.NodeKindOperationDefinition { return } From fdf63d07a4b447a601ac4e646ff37b6e01571ed7 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 30 Jan 2026 16:01:40 +0100 Subject: [PATCH 07/21] fix: expose literal values as astjson object --- .../astnormalization/astnormalization_test.go | 333 +++++++++--------- .../field_argument_mapping.go | 10 +- .../astnormalization/variables_extraction.go | 13 +- 3 files changed, 193 insertions(+), 163 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index e3a02ec8ec..0ee8ce1f58 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -8,6 +8,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/wundergraph/astjson" + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" @@ -1109,171 +1111,186 @@ func TestVariablesNormalizer(t *testing.T) { variables string expectedMapping FieldArgumentMapping }{ - // { - // name: "mapping references the name of the variable", - // operation: `query GetUser($userId: ID!) { - // user(id: $userId) { name } - // }`, - // variables: `{"userId": "123"}`, - // expectedMapping: FieldArgumentMapping{"query.user.id": FieldArgumentValue{VariableName: "userId"}}, - // }, - // { - // name: "mapping references an extracted variable on literal values", - // operation: `query GetUser { - // user(id: "123") { name } - // }`, - // variables: `{}`, - // expectedMapping: FieldArgumentMapping{"query.user.id": FieldArgumentValue{VariableName: "a"}}, - // }, - // { - // name: "mapping references an extracted variable on multiple literal values", - // operation: `query GetUsers { - // firstAlias: user(id: "user-1") { name } - // secondAlias: user(id: "user-2") { name } - // }`, - // variables: `{}`, - // expectedMapping: FieldArgumentMapping{ - // "query.firstAlias.id": FieldArgumentValue{VariableName: "a"}, - // "query.secondAlias.id": FieldArgumentValue{VariableName: "b"}, - // }, - // }, - // { - // name: "mapping references an extracted variable on multiple variable values", - // operation: `query GetUsers($id1: ID!, $id2: ID!) { - // firstAlias: user(id: $id1) { name } - // secondAlias: user(id: $id2) { name } - // }`, - // variables: `{"id1": "user-1", "id2": "user-2"}`, - // expectedMapping: FieldArgumentMapping{ - // "query.firstAlias.id": FieldArgumentValue{VariableName: "id1"}, - // "query.secondAlias.id": FieldArgumentValue{VariableName: "id2"}, - // }, - // }, - // { - // name: "mapping correctly builds paths on nested field arguments", - // operation: `query GetUserPosts($userId: ID!, $limit: Int!) { - // user(id: $userId) { - // posts(limit: $limit) { title } - // } - // }`, - // variables: `{"userId": "123", "limit": 10}`, - // expectedMapping: FieldArgumentMapping{ - // "query.user.id": FieldArgumentValue{VariableName: "userId"}, - // "query.user.posts.limit": FieldArgumentValue{VariableName: "limit"}, - // }, - // }, - // { - // name: "multiple variables refs used correctly in mapping", - // operation: `query SearchUsers($name: String!, $limit: Int!, $offset: Int) { - // users(name: $name, limit: $limit, offset: $offset) { id } - // }`, - // variables: `{"name": "john", "limit": 10, "offset": 5}`, - // expectedMapping: FieldArgumentMapping{ - // "query.users.name": FieldArgumentValue{VariableName: "name"}, - // "query.users.limit": FieldArgumentValue{VariableName: "limit"}, - // "query.users.offset": FieldArgumentValue{VariableName: "offset"}, - // }, - // }, - // { - // name: "mixed inline and variables get correctly mapped", - // operation: `query GetUser($userId: ID!) { - // user(id: $userId, active: true) { name } - // }`, - // variables: `{"userId": "123"}`, - // expectedMapping: FieldArgumentMapping{ - // "query.user.id": FieldArgumentValue{VariableName: "userId"}, - // "query.user.active": FieldArgumentValue{VariableName: "a"}, - // }, - // }, - // { - // name: "empty mapping when no arguments", - // operation: `query GetUser { - // user { name } - // }`, - // variables: `{}`, - // expectedMapping: FieldArgumentMapping{}, - // }, - // { - // name: "inline fragment prefixed with dollar sign in paths", - // operation: `query GetNode($nodeId: ID!, $limit: Int!) { - // node(id: $nodeId) { - // ... on User { - // posts(limit: $limit) { title } - // } - // } - // }`, - // variables: `{"nodeId": "123", "limit": 10}`, - // expectedMapping: FieldArgumentMapping{ - // "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, - // "query.node.$User.posts.limit": FieldArgumentValue{VariableName: "limit"}, - // }, - // }, - // { - // name: "same field in different inline fragments requires type name to avoid collision", - // operation: `query GetNode($nodeId: ID!) { - // node(id: $nodeId) { - // ... on User { - // items(limit: 10) { name } - // } - // ... on Admin { - // items(limit: 5) { name } - // } - // } - // }`, - // variables: `{"nodeId": "123"}`, - // expectedMapping: FieldArgumentMapping{ - // "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, - // "query.node.$User.items.limit": FieldArgumentValue{VariableName: "a"}, - // "query.node.$Admin.items.limit": FieldArgumentValue{VariableName: "b"}, - // }, - // }, - // { - // name: "inline fragment and field with same name requires prefix to avoid collision", - // operation: `query GetNode($nodeId: ID!) { - // node(id: $nodeId) { - // ... on User { - // posts(limit: 10) { title } - // } - // User { - // posts(limit: 5) { title } - // } - // } - // }`, - // variables: `{"nodeId": "123"}`, - // expectedMapping: FieldArgumentMapping{ - // "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, - // "query.node.$User.posts.limit": FieldArgumentValue{VariableName: "a"}, - // "query.node.User.posts.limit": FieldArgumentValue{VariableName: "b"}, - // }, - // }, - // { - // name: "named fragment with variable field arg value", - // operation: `fragment UserFieldsPost on User { - // posts(limit: $limitVar) { title } - // } - // query GetUser($limitVar: Int!) { - // user { - // ...UserFieldsPost - // } - // }`, - // variables: `{"limitVar": 11}`, - // expectedMapping: FieldArgumentMapping{ - // "User.posts.limit": FieldArgumentValue{VariableName: "limitVar"}, - // }, - // }, { - name: "named fragment with literal field arg value", - operation: `fragment UserFieldsPost on User { - posts(limit: 10) { title } - } + name: "mapping references the name of the variable", + operation: ` + query GetUser($userId: ID!) { + user(id: $userId) { name } + }`, + variables: `{"userId": "123"}`, + expectedMapping: FieldArgumentMapping{"query.user.id": FieldArgumentValue{VariableName: "userId"}}, + }, + { + name: "mapping references an extracted variable on literal values", + operation: ` + query GetUser { + user(id: "123") { name } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{"query.user.id": FieldArgumentValue{VariableName: "a"}}, + }, + { + name: "mapping references an extracted variable on multiple literal values", + operation: ` + query GetUsers { + firstAlias: user(id: "user-1") { name } + secondAlias: user(id: "user-2") { name } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{ + "query.firstAlias.id": FieldArgumentValue{VariableName: "a"}, + "query.secondAlias.id": FieldArgumentValue{VariableName: "b"}, + }, + }, + { + name: "mapping references an extracted variable on multiple variable values", + operation: ` + query GetUsers($id1: ID!, $id2: ID!) { + firstAlias: user(id: $id1) { name } + secondAlias: user(id: $id2) { name } + }`, + variables: `{"id1": "user-1", "id2": "user-2"}`, + expectedMapping: FieldArgumentMapping{ + "query.firstAlias.id": FieldArgumentValue{VariableName: "id1"}, + "query.secondAlias.id": FieldArgumentValue{VariableName: "id2"}, + }, + }, + { + name: "mapping correctly builds paths on nested field arguments", + operation: ` + query GetUserPosts($userId: ID!, $limit: Int!) { + user(id: $userId) { + posts(limit: $limit) { title } + } + }`, + variables: `{"userId": "123", "limit": 10}`, + expectedMapping: FieldArgumentMapping{ + "query.user.id": FieldArgumentValue{VariableName: "userId"}, + "query.user.posts.limit": FieldArgumentValue{VariableName: "limit"}, + }, + }, + { + name: "multiple variables refs used correctly in mapping", + operation: ` + query SearchUsers($name: String!, $limit: Int!, $offset: Int) { + users(name: $name, limit: $limit, offset: $offset) { id } + }`, + variables: `{"name": "john", "limit": 10, "offset": 5}`, + expectedMapping: FieldArgumentMapping{ + "query.users.name": FieldArgumentValue{VariableName: "name"}, + "query.users.limit": FieldArgumentValue{VariableName: "limit"}, + "query.users.offset": FieldArgumentValue{VariableName: "offset"}, + }, + }, + { + name: "mixed inline and variables get correctly mapped", + operation: ` + query GetUser($userId: ID!) { + user(id: $userId, active: true) { name } + }`, + variables: `{"userId": "123"}`, + expectedMapping: FieldArgumentMapping{ + "query.user.id": FieldArgumentValue{VariableName: "userId"}, + "query.user.active": FieldArgumentValue{VariableName: "a"}, + }, + }, + { + name: "empty mapping when no arguments", + operation: ` query GetUser { + user { name } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{}, + }, + { + name: "inline fragment prefixed with dollar sign in paths", + operation: ` + query GetNode($nodeId: ID!, $limit: Int!) { + node(id: $nodeId) { + ... on User { + posts(limit: $limit) { title } + } + } + }`, + variables: `{"nodeId": "123", "limit": 10}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, + "query.node.$User.posts.limit": FieldArgumentValue{VariableName: "limit"}, + }, + }, + { + name: "same field in different inline fragments requires type name to avoid collision", + operation: ` + query GetNode($nodeId: ID!) { + node(id: $nodeId) { + ... on User { + items(limit: 10) { name } + } + ... on Admin { + items(limit: 5) { name } + } + } + }`, + variables: `{"nodeId": "123"}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, + "query.node.$User.items.limit": FieldArgumentValue{VariableName: "a"}, + "query.node.$Admin.items.limit": FieldArgumentValue{VariableName: "b"}, + }, + }, + { + name: "inline fragment and field with same name requires prefix to avoid collision", + operation: ` + query GetNode($nodeId: ID!) { + node(id: $nodeId) { + ... on User { + posts(limit: 10) { title } + } + User { + posts(limit: 5) { title } + } + } + }`, + variables: `{"nodeId": "123"}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, + "query.node.$User.posts.limit": FieldArgumentValue{VariableName: "a"}, + "query.node.User.posts.limit": FieldArgumentValue{VariableName: "b"}, + }, + }, + { + name: "named fragment with variable field arg value", + operation: ` + fragment UserFieldsPost on User { + posts(limit: $limitVar) { title } + } + query GetUser($limitVar: Int!) { user { ...UserFieldsPost } }`, - variables: `{}`, + variables: `{"limitVar": 11}`, + expectedMapping: FieldArgumentMapping{ + "User.posts.limit": FieldArgumentValue{VariableName: "limitVar"}, + }, + }, + { + name: "named fragment with literal and variable field arg value", + operation: ` + fragment UserFields on User { + posts(limit: 10) { title } + items(limit: $limitVar) { name } + } + query GetUser($limitVar: Int!) { + user { + ...UserFields + } + }`, + variables: `{"limitVar": 12}`, expectedMapping: FieldArgumentMapping{ - "User.posts.limit": FieldArgumentValue{LiteralValue: []byte("10")}, + "User.posts.limit": FieldArgumentValue{LiteralValue: astjson.MustParse("10")}, + "User.items.limit": FieldArgumentValue{VariableName: "limitVar"}, }, }, } diff --git a/v2/pkg/astnormalization/field_argument_mapping.go b/v2/pkg/astnormalization/field_argument_mapping.go index 63cfa05833..276b037609 100644 --- a/v2/pkg/astnormalization/field_argument_mapping.go +++ b/v2/pkg/astnormalization/field_argument_mapping.go @@ -1,12 +1,16 @@ package astnormalization -import "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" +import ( + "github.com/wundergraph/astjson" + + "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" +) // FieldArgumentValue represents either a variable reference or a literal value. // If LiteralValue is non-nil, it's a literal; otherwise use VariableName. type FieldArgumentValue struct { - VariableName string // Set when value is from a variable (e.g., "userId", "a") - LiteralValue []byte // Set when value is a literal (JSON encoded, e.g., []byte("10")) + VariableName string + LiteralValue *astjson.Value } // FieldArgumentMapping maps field arguments to their values. diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 2b0a5525ba..c3e3ff44e6 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -7,6 +7,8 @@ import ( "github.com/buger/jsonparser" "github.com/tidwall/sjson" + "github.com/wundergraph/astjson" + "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/astnormalization/uploads" @@ -194,10 +196,17 @@ func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName // Literal value - store the JSON-encoded value valueBytes, err := v.operation.ValueToJSON(v.operation.Arguments[ref].Value) if err != nil { - return // Skip on error + // Skip on error + return + } + + astValue, err := astjson.ParseBytesWithoutCache(valueBytes) + if err != nil { + // Skip on error + return } - v.fieldArgumentMapping[key] = FieldArgumentValue{LiteralValue: valueBytes} + v.fieldArgumentMapping[key] = FieldArgumentValue{LiteralValue: astValue} } func (v *variablesExtractionVisitor) variableExists(variableValue []byte, inputValueDefinition int) (exists bool, name []byte, definition int) { From a85a37b1414544ae3400a12853df2aa916fc60e6 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Mon, 2 Feb 2026 12:07:30 +0100 Subject: [PATCH 08/21] chore: remove fragment support The router makes sure to only pass normalized operations to the variable normalizer. This means in practice that both, named and inlined fragments, have already been inlined into the query and the variable normalizer visitor never sees any fragments. Therefore we don't need to explicitely support them for field argument mapping. I removed it because it would be expensive (if used), adds code complexity and in practice is dead code. --- v2/pkg/ast/path.go | 6 +- .../astnormalization/astnormalization_test.go | 120 ++---------------- .../field_argument_mapping.go | 18 +-- .../astnormalization/variables_extraction.go | 44 ++----- ...ation_rule_validate_empty_selection_set.go | 2 +- .../graphql_datasource/graphql_datasource.go | 2 +- .../representation_variable.go | 2 +- .../plan/abstract_selection_rewriter.go | 2 +- ...datasource_filter_collect_nodes_visitor.go | 4 +- .../datasource_filter_resolvable_visitor.go | 2 +- v2/pkg/engine/plan/key_fields_visitor.go | 2 +- v2/pkg/engine/plan/node_selection_visitor.go | 2 +- v2/pkg/engine/plan/path_builder_visitor.go | 12 +- v2/pkg/engine/plan/provides_fields_visitor.go | 2 +- v2/pkg/engine/plan/required_fields_visitor.go | 2 +- v2/pkg/engine/plan/visitor.go | 6 +- 16 files changed, 52 insertions(+), 176 deletions(-) diff --git a/v2/pkg/ast/path.go b/v2/pkg/ast/path.go index 130c883f36..19493ec60c 100644 --- a/v2/pkg/ast/path.go +++ b/v2/pkg/ast/path.go @@ -128,7 +128,7 @@ func (p Path) String() string { return out } -func (p Path) DotDelimitedString(includeFragmentRef bool) string { +func (p Path) DotDelimitedString() string { builder := strings.Builder{} toGrow := 0 @@ -160,9 +160,7 @@ func (p Path) DotDelimitedString(includeFragmentRef bool) string { } case InlineFragmentName: builder.WriteString(InlineFragmentPathPrefix) - if includeFragmentRef { - builder.WriteString(strconv.Itoa(p[i].FragmentRef)) - } + builder.WriteString(strconv.Itoa(p[i].FragmentRef)) builder.WriteString(unsafebytes.BytesToString(p[i].FieldName)) } } diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index 0ee8ce1f58..38446423cd 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -8,8 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/wundergraph/astjson" - "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" @@ -1065,7 +1063,7 @@ func TestVariablesNormalizer(t *testing.T) { }, result.UploadsMapping) // Verify field argument mapping is populated - assert.Equal(t, FieldArgumentValue{VariableName: "a"}, result.FieldArgumentMapping["mutation.hello.arg"]) + assert.Equal(t, "a", result.FieldArgumentMapping["mutation.hello.arg"]) }) t.Run("field argument mapping", func(t *testing.T) { @@ -1118,7 +1116,7 @@ func TestVariablesNormalizer(t *testing.T) { user(id: $userId) { name } }`, variables: `{"userId": "123"}`, - expectedMapping: FieldArgumentMapping{"query.user.id": FieldArgumentValue{VariableName: "userId"}}, + expectedMapping: FieldArgumentMapping{"query.user.id": "userId"}, }, { name: "mapping references an extracted variable on literal values", @@ -1127,7 +1125,7 @@ func TestVariablesNormalizer(t *testing.T) { user(id: "123") { name } }`, variables: `{}`, - expectedMapping: FieldArgumentMapping{"query.user.id": FieldArgumentValue{VariableName: "a"}}, + expectedMapping: FieldArgumentMapping{"query.user.id": "a"}, }, { name: "mapping references an extracted variable on multiple literal values", @@ -1138,8 +1136,8 @@ func TestVariablesNormalizer(t *testing.T) { }`, variables: `{}`, expectedMapping: FieldArgumentMapping{ - "query.firstAlias.id": FieldArgumentValue{VariableName: "a"}, - "query.secondAlias.id": FieldArgumentValue{VariableName: "b"}, + "query.firstAlias.id": "a", + "query.secondAlias.id": "b", }, }, { @@ -1151,8 +1149,8 @@ func TestVariablesNormalizer(t *testing.T) { }`, variables: `{"id1": "user-1", "id2": "user-2"}`, expectedMapping: FieldArgumentMapping{ - "query.firstAlias.id": FieldArgumentValue{VariableName: "id1"}, - "query.secondAlias.id": FieldArgumentValue{VariableName: "id2"}, + "query.firstAlias.id": "id1", + "query.secondAlias.id": "id2", }, }, { @@ -1165,8 +1163,8 @@ func TestVariablesNormalizer(t *testing.T) { }`, variables: `{"userId": "123", "limit": 10}`, expectedMapping: FieldArgumentMapping{ - "query.user.id": FieldArgumentValue{VariableName: "userId"}, - "query.user.posts.limit": FieldArgumentValue{VariableName: "limit"}, + "query.user.id": "userId", + "query.user.posts.limit": "limit", }, }, { @@ -1177,9 +1175,9 @@ func TestVariablesNormalizer(t *testing.T) { }`, variables: `{"name": "john", "limit": 10, "offset": 5}`, expectedMapping: FieldArgumentMapping{ - "query.users.name": FieldArgumentValue{VariableName: "name"}, - "query.users.limit": FieldArgumentValue{VariableName: "limit"}, - "query.users.offset": FieldArgumentValue{VariableName: "offset"}, + "query.users.name": "name", + "query.users.limit": "limit", + "query.users.offset": "offset", }, }, { @@ -1190,8 +1188,8 @@ func TestVariablesNormalizer(t *testing.T) { }`, variables: `{"userId": "123"}`, expectedMapping: FieldArgumentMapping{ - "query.user.id": FieldArgumentValue{VariableName: "userId"}, - "query.user.active": FieldArgumentValue{VariableName: "a"}, + "query.user.id": "userId", + "query.user.active": "a", }, }, { @@ -1203,96 +1201,6 @@ func TestVariablesNormalizer(t *testing.T) { variables: `{}`, expectedMapping: FieldArgumentMapping{}, }, - { - name: "inline fragment prefixed with dollar sign in paths", - operation: ` - query GetNode($nodeId: ID!, $limit: Int!) { - node(id: $nodeId) { - ... on User { - posts(limit: $limit) { title } - } - } - }`, - variables: `{"nodeId": "123", "limit": 10}`, - expectedMapping: FieldArgumentMapping{ - "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, - "query.node.$User.posts.limit": FieldArgumentValue{VariableName: "limit"}, - }, - }, - { - name: "same field in different inline fragments requires type name to avoid collision", - operation: ` - query GetNode($nodeId: ID!) { - node(id: $nodeId) { - ... on User { - items(limit: 10) { name } - } - ... on Admin { - items(limit: 5) { name } - } - } - }`, - variables: `{"nodeId": "123"}`, - expectedMapping: FieldArgumentMapping{ - "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, - "query.node.$User.items.limit": FieldArgumentValue{VariableName: "a"}, - "query.node.$Admin.items.limit": FieldArgumentValue{VariableName: "b"}, - }, - }, - { - name: "inline fragment and field with same name requires prefix to avoid collision", - operation: ` - query GetNode($nodeId: ID!) { - node(id: $nodeId) { - ... on User { - posts(limit: 10) { title } - } - User { - posts(limit: 5) { title } - } - } - }`, - variables: `{"nodeId": "123"}`, - expectedMapping: FieldArgumentMapping{ - "query.node.id": FieldArgumentValue{VariableName: "nodeId"}, - "query.node.$User.posts.limit": FieldArgumentValue{VariableName: "a"}, - "query.node.User.posts.limit": FieldArgumentValue{VariableName: "b"}, - }, - }, - { - name: "named fragment with variable field arg value", - operation: ` - fragment UserFieldsPost on User { - posts(limit: $limitVar) { title } - } - query GetUser($limitVar: Int!) { - user { - ...UserFieldsPost - } - }`, - variables: `{"limitVar": 11}`, - expectedMapping: FieldArgumentMapping{ - "User.posts.limit": FieldArgumentValue{VariableName: "limitVar"}, - }, - }, - { - name: "named fragment with literal and variable field arg value", - operation: ` - fragment UserFields on User { - posts(limit: 10) { title } - items(limit: $limitVar) { name } - } - query GetUser($limitVar: Int!) { - user { - ...UserFields - } - }`, - variables: `{"limitVar": 12}`, - expectedMapping: FieldArgumentMapping{ - "User.posts.limit": FieldArgumentValue{LiteralValue: astjson.MustParse("10")}, - "User.items.limit": FieldArgumentValue{VariableName: "limitVar"}, - }, - }, } for _, tc := range testCases { diff --git a/v2/pkg/astnormalization/field_argument_mapping.go b/v2/pkg/astnormalization/field_argument_mapping.go index 276b037609..a6a0cd0d56 100644 --- a/v2/pkg/astnormalization/field_argument_mapping.go +++ b/v2/pkg/astnormalization/field_argument_mapping.go @@ -1,22 +1,14 @@ package astnormalization import ( - "github.com/wundergraph/astjson" - "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" ) -// FieldArgumentValue represents either a variable reference or a literal value. -// If LiteralValue is non-nil, it's a literal; otherwise use VariableName. -type FieldArgumentValue struct { - VariableName string - LiteralValue *astjson.Value -} - -// FieldArgumentMapping maps field arguments to their values. -// Key: "fieldPath.argumentName" (e.g., "User.posts.limit") -// Value: either a variable name or a literal value -type FieldArgumentMapping map[string]FieldArgumentValue +// FieldArgumentMapping maps the path of field arguments inside an operation +// to the name of their mapped variables. +// Key: "rootOperationType.fieldPath.argumentName" (e.g., "query.posts.limit") +// Value: name of variable of field argument after variable normalization. +type FieldArgumentMapping map[string]string // VariablesNormalizerResult contains the results of variable normalization. type VariablesNormalizerResult struct { diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index c3e3ff44e6..e495795b8e 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -7,8 +7,6 @@ import ( "github.com/buger/jsonparser" "github.com/tidwall/sjson" - "github.com/wundergraph/astjson" - "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/astnormalization/uploads" @@ -40,11 +38,6 @@ type variablesExtractionVisitor struct { } func (v *variablesExtractionVisitor) EnterArgument(ref int) { - if v.Ancestors[0].Kind == ast.NodeKindFragmentDefinition { - v.recordFieldArgumentMapping(ref, "") - return - } - if len(v.Ancestors) == 0 || v.Ancestors[0].Kind != ast.NodeKindOperationDefinition { return } @@ -163,17 +156,11 @@ func (v *variablesExtractionVisitor) EnterDocument(operation, definition *ast.Do v.fieldArgumentMapping = make(FieldArgumentMapping) } -// buildFieldPath builds the field path from the walker's ancestors. -// It returns a dot-separated path of field names/aliases (e.g., "user.posts"). -func (v *variablesExtractionVisitor) buildFieldPath() string { - return v.Path.DotDelimitedString(false) -} - // recordFieldArgumentMapping records the currently visited field argument // of v in v.fieldArgumentMapping, alongside its matching variable name or literal value. // If varName is empty, it looks up the variable name from the operation or stores the literal value. func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName string) { - fieldPath := v.buildFieldPath() + fieldPath := v.Path.DotDelimitedString() if fieldPath == "" { return } @@ -182,31 +169,22 @@ func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName if varName != "" { // Variable name was provided (from extraction) - v.fieldArgumentMapping[key] = FieldArgumentValue{VariableName: varName} + v.fieldArgumentMapping[key] = varName return } - if v.operation.Arguments[ref].Value.Kind == ast.ValueKindVariable { - // Existing variable reference - varName = v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) - v.fieldArgumentMapping[key] = FieldArgumentValue{VariableName: varName} - return - } - - // Literal value - store the JSON-encoded value - valueBytes, err := v.operation.ValueToJSON(v.operation.Arguments[ref].Value) - if err != nil { - // Skip on error - return - } - - astValue, err := astjson.ParseBytesWithoutCache(valueBytes) - if err != nil { - // Skip on error + if v.operation.Arguments[ref].Value.Kind != ast.ValueKindVariable { + // We expect the operation on this visitor to be normalized before the visitor walks it. + // This means that values of any field arguments should be extracted to variables. + // If we still land here it means the query hasn't been normalized before. + // In this case we ignore mapping the field argument, i.e. we do not support mapping field + // arguments with literal values. + // It's not impossible to do so, just expensive and not needed at the moment. return } - v.fieldArgumentMapping[key] = FieldArgumentValue{LiteralValue: astValue} + varName = v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) + v.fieldArgumentMapping[key] = varName } func (v *variablesExtractionVisitor) variableExists(variableValue []byte, inputValueDefinition int) (exists bool, name []byte, definition int) { diff --git a/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go b/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go index eb71e593b4..ce6dd4e8c4 100644 --- a/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go +++ b/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go @@ -32,6 +32,6 @@ func (r *emptySelectionSetVisitor) EnterDocument(operation, definition *ast.Docu func (r *emptySelectionSetVisitor) EnterSelectionSet(ref int) { if r.operation.SelectionSetIsEmpty(ref) { - r.Walker.StopWithInternalErr(fmt.Errorf("astvalidation selection set on path %s is empty", r.Walker.Path.DotDelimitedString(true))) + r.Walker.StopWithInternalErr(fmt.Errorf("astvalidation selection set on path %s is empty", r.Walker.Path.DotDelimitedString())) } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 97af5e372b..23dea1a6ac 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -783,7 +783,7 @@ func (p *Planner[T]) allowField(ref int) bool { // In addition, we skip field if its path are equal to planner parent path // This is required to correctly plan on datasource which has corresponding child/root node, // but we don't need to add it to the query as we are in the nested request - currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(true), fieldAliasOrName) + currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(), fieldAliasOrName) if p.dataSourcePlannerConfig.ParentPath != "query" && p.dataSourcePlannerConfig.ParentPath == currentPath { p.DebugPrint("allowField: false path:", currentPath, "is equal to parent path", p.dataSourcePlannerConfig.ParentPath) return false diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go index 83ad1a6ece..18cb3ad204 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go @@ -210,7 +210,7 @@ func (v *representationVariableVisitor) EnterField(ref int) { } fieldDefinitionType := v.definition.FieldDefinitionType(fieldDefinition) - currentPath := v.Walker.Path.DotDelimitedString(true) + "." + string(fieldName) + currentPath := v.Walker.Path.DotDelimitedString() + "." + string(fieldName) // if path was remapped during the planning we update its path fieldPath := string(fieldName) diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter.go b/v2/pkg/engine/plan/abstract_selection_rewriter.go index c98298c622..65c0f87011 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter.go @@ -694,7 +694,7 @@ type AbstractFieldPathCollector struct { } func (v *AbstractFieldPathCollector) EnterField(ref int) { - parentPath := v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + parentPath := v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString() currentFieldName := v.operation.FieldNameString(ref) currentPath := parentPath + "." + currentFieldName diff --git a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go index 2d03b6a9ca..b9d47ad3ba 100644 --- a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go @@ -577,9 +577,9 @@ func (f *treeBuilderVisitor) collectFieldInfo(fieldRef int) { fieldName := f.operation.FieldNameUnsafeString(fieldRef) fieldAliasOrName := f.operation.FieldAliasOrNameString(fieldRef) isTypeName := fieldName == typeNameField - parentPath := f.walker.Path.DotDelimitedString(true) + parentPath := f.walker.Path.DotDelimitedString() onFragment := f.walker.Path.EndsWithFragment() - parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() currentPath := fmt.Sprintf("%s.%s", parentPath, fieldAliasOrName) currentPathWithoutFragments := fmt.Sprintf("%s.%s", parentPathWithoutFragment, fieldAliasOrName) diff --git a/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go b/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go index 4a392bbb26..8efbe79960 100644 --- a/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go @@ -42,7 +42,7 @@ func (f *nodesResolvableVisitor) EnterField(ref int) { } } - parentPath := f.walker.Path.DotDelimitedString(true) + parentPath := f.walker.Path.DotDelimitedString() currentPath := parentPath + "." + fieldAliasOrName _, found := f.nodes.HasSuggestionForPath(typeName, fieldName, currentPath) diff --git a/v2/pkg/engine/plan/key_fields_visitor.go b/v2/pkg/engine/plan/key_fields_visitor.go index 29bb05f5ca..1e018d56dd 100644 --- a/v2/pkg/engine/plan/key_fields_visitor.go +++ b/v2/pkg/engine/plan/key_fields_visitor.go @@ -198,7 +198,7 @@ func (v *keyInfoVisitor) EnterField(ref int) { fieldName := v.input.key.FieldNameUnsafeString(ref) typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) - parentPath := v.input.parentPath + strings.TrimPrefix(v.walker.Path.DotDelimitedString(true), v.input.typeName) + parentPath := v.input.parentPath + strings.TrimPrefix(v.walker.Path.DotDelimitedString(), v.input.typeName) currentPath := parentPath + "." + fieldName hasRootNode := v.input.dataSource.HasRootNode(typeName, fieldName) diff --git a/v2/pkg/engine/plan/node_selection_visitor.go b/v2/pkg/engine/plan/node_selection_visitor.go index c2a09e8b82..8f9f80fb48 100644 --- a/v2/pkg/engine/plan/node_selection_visitor.go +++ b/v2/pkg/engine/plan/node_selection_visitor.go @@ -222,7 +222,7 @@ func (c *nodeSelectionVisitor) handleEnterField(fieldRef int, handleRequires boo c.debugPrint("EnterField ref:", fieldRef, "fieldName:", fieldName, "typeName:", typeName, "requires:", handleRequires) - parentPath := c.walker.Path.DotDelimitedString(true) + parentPath := c.walker.Path.DotDelimitedString() currentPath := parentPath + "." + fieldAliasOrName suggestions := c.nodeSuggestions.SuggestionsForPath(typeName, fieldName, currentPath) diff --git a/v2/pkg/engine/plan/path_builder_visitor.go b/v2/pkg/engine/plan/path_builder_visitor.go index 78f40d3800..50f74d0ae3 100644 --- a/v2/pkg/engine/plan/path_builder_visitor.go +++ b/v2/pkg/engine/plan/path_builder_visitor.go @@ -362,8 +362,8 @@ func (c *pathBuilderVisitor) EnterSelectionSet(ref int) { return } - parentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString(true) - currentPath := c.walker.Path.DotDelimitedString(true) + parentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString() + currentPath := c.walker.Path.DotDelimitedString() typeName := c.operation.InlineFragmentTypeConditionNameString(ancestor.Ref) node, ok := c.definition.NodeByNameStr(typeName) @@ -440,7 +440,7 @@ func (c *pathBuilderVisitor) EnterField(fieldRef int) { c.debugPrint("EnterField ref:", fieldRef, "fieldName:", fieldName, "typeName:", typeName) - parentPath := c.walker.Path.DotDelimitedString(true) + parentPath := c.walker.Path.DotDelimitedString() // we need to also check preceding path for inline fragments // as for the field within inline fragment the parent path will include type condition in a path // but planner path still will not include it @@ -458,7 +458,7 @@ func (c *pathBuilderVisitor) EnterField(fieldRef int) { precedingPath = c.walker.Path[:i-1] } if precedingPath != nil { - precedingParentPath = precedingPath.DotDelimitedString(true) + precedingParentPath = precedingPath.DotDelimitedString() } currentPath := parentPath + "." + fieldAliasOrName @@ -630,7 +630,7 @@ func (c *pathBuilderVisitor) fieldHasAuthorizationRule(typeName, fieldName strin } func (c *pathBuilderVisitor) fieldIsChildNode(plannerIdx int) bool { - path := c.walker.Path.DotDelimitedString(true) + path := c.walker.Path.DotDelimitedString() plannerPath := c.planners[plannerIdx].ParentPath() fieldPath := strings.TrimPrefix(path, plannerPath) return strings.ContainsAny(fieldPath, ".") @@ -926,7 +926,7 @@ func (c *pathBuilderVisitor) addNewPlanner(fieldRef int, typeName, fieldName, cu plannerPath := parentPath if isParentFragment { - precedingFragmentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString(true) + precedingFragmentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString() // if the parent is a fragment, we add the preceding parent path as well // to be able to walk selection sets in the fragment paths = append([]pathConfiguration{ diff --git a/v2/pkg/engine/plan/provides_fields_visitor.go b/v2/pkg/engine/plan/provides_fields_visitor.go index d28c1fec7c..50cf19eed2 100644 --- a/v2/pkg/engine/plan/provides_fields_visitor.go +++ b/v2/pkg/engine/plan/provides_fields_visitor.go @@ -101,7 +101,7 @@ func (v *providesVisitor) EnterField(ref int) { fieldName := v.input.providesFieldSet.FieldNameUnsafeString(ref) typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) - currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() parentPath := v.input.parentPath + strings.TrimPrefix(currentPathWithoutFragments, v.pathPrefix) currentPath := parentPath + "." + fieldName diff --git a/v2/pkg/engine/plan/required_fields_visitor.go b/v2/pkg/engine/plan/required_fields_visitor.go index 597460a8d0..2123605015 100644 --- a/v2/pkg/engine/plan/required_fields_visitor.go +++ b/v2/pkg/engine/plan/required_fields_visitor.go @@ -319,7 +319,7 @@ func (v *requiredFieldsVisitor) addRequiredField(keyRef int, fieldName ast.ByteS Name: v.config.operation.Input.AppendInputBytes(fullAliasName), } - currentPath := v.Walker.Path.DotDelimitedString(true) + "." + string(fieldName) + currentPath := v.Walker.Path.DotDelimitedString() + "." + string(fieldName) v.mapping[currentPath] = string(fullAliasName) } diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index 3a82cfc898..1f8a469d05 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -145,7 +145,7 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any v.pathCache[kind] = make(map[int]string) } if path == "" { - path = v.Walker.Path.DotDelimitedString(true) + path = v.Walker.Path.DotDelimitedString() if kind == astvisitor.EnterField || kind == astvisitor.LeaveField { path = path + "." + v.Operation.FieldAliasOrNameString(ref) } @@ -253,9 +253,9 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any } func (v *Visitor) currentFullPath(skipFragments bool) string { - path := v.Walker.Path.DotDelimitedString(true) + path := v.Walker.Path.DotDelimitedString() if skipFragments { - path = v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + path = v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString() } if v.Walker.CurrentKind == ast.NodeKindField { From 546a496bbb6943c647ea59793751db4663cc1ea8 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Mon, 2 Feb 2026 15:05:51 +0100 Subject: [PATCH 09/21] feat: make field arg mapping configurable Adds the ability to enable or disable field arg mapping when the variable normalizer is created. Since this option is only useful on the variable normalizer it is disabled in code for the variable extraction on the operation normalizer. --- v2/pkg/astnormalization/astnormalization.go | 9 ++--- .../astnormalization/astnormalization_test.go | 19 ++++++----- .../astnormalization/variables_extraction.go | 34 +++++++++++++++---- .../variables_extraction_test.go | 16 ++++----- .../astnormalization/variables_mapper_test.go | 2 +- .../variables_unused_deletion_test.go | 2 +- 6 files changed, 52 insertions(+), 30 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index aa73a477e1..0aeec14968 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -244,7 +244,8 @@ func (o *OperationNormalizer) setupOperationWalkers() { if o.options.extractVariables { extractVariablesWalker := astvisitor.NewWalkerWithID(8, "ExtractVariables") - extractVariables(&extractVariablesWalker) + // disabling field arg mapping as it's only necessary on the variable normalizer + extractVariables(&extractVariablesWalker, false) o.operationWalkers = append(o.operationWalkers, walkerStage{ name: "extractVariables", walker: &extractVariablesWalker, @@ -352,7 +353,7 @@ type VariablesNormalizer struct { variablesExtractionVisitor *variablesExtractionVisitor } -func NewVariablesNormalizer() *VariablesNormalizer { +func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { // delete unused modifying variables refs, // so it is safer to run it sequentially with the extraction thirdDeleteUnused := astvisitor.NewWalkerWithID(8, "DeleteUnusedVariables") @@ -366,7 +367,7 @@ func NewVariablesNormalizer() *VariablesNormalizer { detectVariableUsage(&firstDetectUnused, del) secondExtract := astvisitor.NewWalkerWithID(8, "ExtractVariables") - variablesExtractionVisitor := extractVariables(&secondExtract) + variablesExtractionVisitor := extractVariables(&secondExtract, withFieldArgMapping) extractVariablesDefaultValue(&secondExtract) fourthCoerce := astvisitor.NewWalkerWithID(0, "VariablesCoercion") @@ -398,7 +399,7 @@ func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Docu return VariablesNormalizerResult{ UploadsMapping: v.variablesExtractionVisitor.uploadsPath, - FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping, + FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping.result, } } diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index 38446423cd..3c8d6352d2 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1018,7 +1018,7 @@ func TestVariablesNormalizer(t *testing.T) { operationDocument := unsafeparser.ParseGraphqlDocumentString(input) operationDocument.Input.Variables = []byte(`{}`) - normalizer := NewVariablesNormalizer() + normalizer := NewVariablesNormalizer(false) report := operationreport.Report{} normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) require.False(t, report.HasErrors(), report.Error()) @@ -1040,7 +1040,8 @@ func TestVariablesNormalizer(t *testing.T) { operationDocument := unsafeparser.ParseGraphqlDocumentString(`mutation Foo($varOne: [Input2!]! $varTwo: Input2!) { hello(arg: {twoList: $varOne two: $varTwo}) }`) operationDocument.Input.Variables = []byte(`{"varOne":[{"oneList":[{"list":[null,null],"value":null}],"one":{"list":[null],"value":null}}],"varTwo":{"oneList":[{"list":[null,null],"value":null}],"one":{"list":[null],"value":null}}}`) - normalizer := NewVariablesNormalizer() + // create normalizer without field arg mapping + normalizer := NewVariablesNormalizer(false) report := operationreport.Report{} result := normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) require.False(t, report.HasErrors(), report.Error()) @@ -1062,8 +1063,8 @@ func TestVariablesNormalizer(t *testing.T) { {VariableName: "a", OriginalUploadPath: "variables.varTwo.one.value", NewUploadPath: "variables.a.two.one.value"}, }, result.UploadsMapping) - // Verify field argument mapping is populated - assert.Equal(t, "a", result.FieldArgumentMapping["mutation.hello.arg"]) + // Verify field argument mapping is not populated + assert.Nil(t, result.FieldArgumentMapping) }) t.Run("field argument mapping", func(t *testing.T) { @@ -1209,7 +1210,7 @@ func TestVariablesNormalizer(t *testing.T) { operationDocument := unsafeparser.ParseGraphqlDocumentString(tc.operation) operationDocument.Input.Variables = []byte(tc.variables) - normalizer := NewVariablesNormalizer() + normalizer := NewVariablesNormalizer(true) report := operationreport.Report{} result := normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) require.False(t, report.HasErrors(), report.Error()) @@ -1245,7 +1246,7 @@ var mustString = func(str string, err error) string { } type registerNormalizeFunc func(walker *astvisitor.Walker) -type registerNormalizeVariablesFunc func(walker *astvisitor.Walker) *variablesExtractionVisitor +type registerNormalizeVariablesFunc func(walker *astvisitor.Walker, withFieldArgMapping bool) *variablesExtractionVisitor type registerNormalizeVariablesDefaulValueFunc func(walker *astvisitor.Walker) *variablesDefaultValueExtractionVisitor type registerNormalizeDeleteVariablesFunc func(walker *astvisitor.Walker) *deleteUnusedVariablesVisitor @@ -1341,7 +1342,7 @@ var runWithVariablesExtraction = func(t *testing.T, normalizeFunc registerNormal t.Helper() runWithVariablesAssert(t, func(walker *astvisitor.Walker) { - normalizeFunc(walker) + normalizeFunc(walker, false) }, definition, operation, operationName, expectedOutput, variablesInput, expectedVariables, additionalNormalizers...) } @@ -1349,7 +1350,7 @@ var runWithVariablesExtractionAndPreNormalize = func(t *testing.T, normalizeFunc t.Helper() runWithVariablesAssertAndPreNormalize(t, func(walker *astvisitor.Walker) { - normalizeFunc(walker) + normalizeFunc(walker, false) }, definition, operation, operationName, expectedOutput, variablesInput, expectedVariables, prerequisites...) } @@ -1445,7 +1446,7 @@ var runWithExpectedErrors = func(t *testing.T, normalizeFunc registerNormalizeVa report := operationreport.Report{} walker := astvisitor.NewWalker(48) - normalizeFunc(&walker) + normalizeFunc(&walker, false) for _, fn := range additionalNormalizers { fn(&walker) diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index e495795b8e..66513a36ca 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -14,10 +14,13 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/internal/unsafebytes" ) -func extractVariables(walker *astvisitor.Walker) *variablesExtractionVisitor { +func extractVariables(walker *astvisitor.Walker, withFieldArgMapping bool) *variablesExtractionVisitor { visitor := &variablesExtractionVisitor{ Walker: walker, uploadFinder: uploads.NewUploadFinder(), + fieldArgumentMapping: fieldArgMappingOption{ + enabled: withFieldArgMapping, + }, } walker.RegisterEnterDocumentVisitor(visitor) walker.RegisterEnterArgumentVisitor(visitor) @@ -34,7 +37,12 @@ type variablesExtractionVisitor struct { extractedVariableTypeRefs []int uploadFinder *uploads.UploadFinder uploadsPath []uploads.UploadPathMapping - fieldArgumentMapping FieldArgumentMapping + fieldArgumentMapping fieldArgMappingOption +} + +type fieldArgMappingOption struct { + enabled bool + result FieldArgumentMapping } func (v *variablesExtractionVisitor) EnterArgument(ref int) { @@ -65,7 +73,9 @@ func (v *variablesExtractionVisitor) EnterArgument(ref int) { v.uploadsPath = append(v.uploadsPath, uploadsMapping...) } // Record the field argument mapping for existing variables - v.recordFieldArgumentMapping(ref, "") + if v.fieldArgumentMapping.enabled { + v.recordFieldArgumentMapping(ref, "") + } return } @@ -146,20 +156,30 @@ func (v *variablesExtractionVisitor) EnterArgument(ref int) { v.operation.OperationDefinitions[v.Ancestors[0].Ref].HasVariableDefinitions = true // Record the field argument mapping for the newly extracted variable - v.recordFieldArgumentMapping(ref, string(variableNameBytes)) + if v.fieldArgumentMapping.enabled { + v.recordFieldArgumentMapping(ref, string(variableNameBytes)) + } } func (v *variablesExtractionVisitor) EnterDocument(operation, definition *ast.Document) { v.operation, v.definition = operation, definition v.extractedVariables = v.extractedVariables[:0] v.extractedVariableTypeRefs = v.extractedVariableTypeRefs[:0] - v.fieldArgumentMapping = make(FieldArgumentMapping) + if v.fieldArgumentMapping.enabled { + v.fieldArgumentMapping.result = make(FieldArgumentMapping) + } } // recordFieldArgumentMapping records the currently visited field argument // of v in v.fieldArgumentMapping, alongside its matching variable name or literal value. // If varName is empty, it looks up the variable name from the operation or stores the literal value. func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName string) { + // Guard to prevent nil panics. + // If v.fieldArgumentMapping.result is nil, then field argument mapping is disabled. + if v.fieldArgumentMapping.result == nil { + return + } + fieldPath := v.Path.DotDelimitedString() if fieldPath == "" { return @@ -169,7 +189,7 @@ func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName if varName != "" { // Variable name was provided (from extraction) - v.fieldArgumentMapping[key] = varName + v.fieldArgumentMapping.result[key] = varName return } @@ -184,7 +204,7 @@ func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName } varName = v.operation.VariableValueNameString(v.operation.Arguments[ref].Value.Ref) - v.fieldArgumentMapping[key] = varName + v.fieldArgumentMapping.result[key] = varName } func (v *variablesExtractionVisitor) variableExists(variableValue []byte, inputValueDefinition int) (exists bool, name []byte, definition int) { diff --git a/v2/pkg/astnormalization/variables_extraction_test.go b/v2/pkg/astnormalization/variables_extraction_test.go index 4066e296e8..ca0e86441f 100644 --- a/v2/pkg/astnormalization/variables_extraction_test.go +++ b/v2/pkg/astnormalization/variables_extraction_test.go @@ -559,8 +559,8 @@ func TestVariablesExtraction(t *testing.T) { t.Run("arg has inline object value with upload passed via variable", func(t *testing.T) { var visitor *variablesExtractionVisitor - register := func(walker *astvisitor.Walker) *variablesExtractionVisitor { - visitor = extractVariables(walker) + register := func(walker *astvisitor.Walker, withFieldArgMapping bool) *variablesExtractionVisitor { + visitor = extractVariables(walker, withFieldArgMapping) return visitor } @@ -580,8 +580,8 @@ func TestVariablesExtraction(t *testing.T) { t.Run("arg has inline objects with variables which have nested file uploads", func(t *testing.T) { var visitor *variablesExtractionVisitor - register := func(walker *astvisitor.Walker) *variablesExtractionVisitor { - visitor = extractVariables(walker) + register := func(walker *astvisitor.Walker, withFieldArgMapping bool) *variablesExtractionVisitor { + visitor = extractVariables(walker, withFieldArgMapping) return visitor } @@ -616,8 +616,8 @@ func TestVariablesExtraction(t *testing.T) { t.Run("arg of type upload", func(t *testing.T) { var visitor *variablesExtractionVisitor - register := func(walker *astvisitor.Walker) *variablesExtractionVisitor { - visitor = extractVariables(walker) + register := func(walker *astvisitor.Walker, withFieldArgMapping bool) *variablesExtractionVisitor { + visitor = extractVariables(walker, withFieldArgMapping) return visitor } @@ -637,8 +637,8 @@ func TestVariablesExtraction(t *testing.T) { t.Run("arg has nested upload in a variable", func(t *testing.T) { var visitor *variablesExtractionVisitor - register := func(walker *astvisitor.Walker) *variablesExtractionVisitor { - visitor = extractVariables(walker) + register := func(walker *astvisitor.Walker, withFieldArgMapping bool) *variablesExtractionVisitor { + visitor = extractVariables(walker, withFieldArgMapping) return visitor } diff --git a/v2/pkg/astnormalization/variables_mapper_test.go b/v2/pkg/astnormalization/variables_mapper_test.go index f11f704010..6b399a5460 100644 --- a/v2/pkg/astnormalization/variables_mapper_test.go +++ b/v2/pkg/astnormalization/variables_mapper_test.go @@ -47,7 +47,7 @@ func TestVariablesMapper(t *testing.T) { WithRemoveFragmentDefinitions(), WithRemoveUnusedVariables(), ) - variablesNormalizer := NewVariablesNormalizer() + variablesNormalizer := NewVariablesNormalizer(false) testCases := []struct { name string diff --git a/v2/pkg/astnormalization/variables_unused_deletion_test.go b/v2/pkg/astnormalization/variables_unused_deletion_test.go index c0323b978f..c372045567 100644 --- a/v2/pkg/astnormalization/variables_unused_deletion_test.go +++ b/v2/pkg/astnormalization/variables_unused_deletion_test.go @@ -41,7 +41,7 @@ func TestVariablesUnusedDeletion(t *testing.T) { del := deleteUnusedVariables(&secondWalker) detectVariableUsage(&firstWalker, del) - extractVariables(&firstWalker) + extractVariables(&firstWalker, false) rep := &operationreport.Report{} firstWalker.Walk(&operationDocument, &definitionDocument, rep) From e4c44d3bab22c5c38571f70ef7104fdb85d37012 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Tue, 10 Feb 2026 09:43:46 +0100 Subject: [PATCH 10/21] fix: record mapping for already extracted literals --- .../astnormalization/astnormalization_test.go | 30 +++++++++++++++++++ .../astnormalization/variables_extraction.go | 6 ++++ 2 files changed, 36 insertions(+) diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index 3c8d6352d2..c6e71a5e58 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1202,6 +1202,36 @@ func TestVariablesNormalizer(t *testing.T) { variables: `{}`, expectedMapping: FieldArgumentMapping{}, }, + { + name: "reused literal values are recorded for all field arguments", + operation: ` + query GetUsers { + firstAlias: user(id: "123") { name } + secondAlias: user(id: "123") { name } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{ + "query.firstAlias.id": "a", + "query.secondAlias.id": "a", + }, + }, + { + name: "multiple reused literal values with different values are all recorded", + operation: ` + query GetUsers { + firstAlias: user(id: "123") { name } + secondAlias: user(id: "123") { name } + thirdAlias: user(id: "456") { name } + fourthAlias: user(id: "456") { name } + }`, + variables: `{}`, + expectedMapping: FieldArgumentMapping{ + "query.firstAlias.id": "a", + "query.secondAlias.id": "a", + "query.thirdAlias.id": "b", + "query.fourthAlias.id": "b", + }, + }, } for _, tc := range testCases { diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 66513a36ca..9989152cf0 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -91,6 +91,12 @@ func (v *variablesExtractionVisitor) EnterArgument(ref int) { value := v.operation.AddVariableValue(variable) v.operation.Arguments[ref].Value.Kind = ast.ValueKindVariable v.operation.Arguments[ref].Value.Ref = value + + // Record the field argument mapping for already extracted variable + if v.fieldArgumentMapping.enabled { + v.recordFieldArgumentMapping(ref, string(name)) + } + return } variableNameBytes := v.operation.GenerateUnusedVariableDefinitionName(v.Ancestors[0].Ref) From f629d8d17289f70a640be08c94319899f26d7913 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Tue, 10 Feb 2026 10:02:16 +0100 Subject: [PATCH 11/21] fix: use valid operation for test --- v2/pkg/astnormalization/astnormalization_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index c6e71a5e58..3f8f9fb811 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1070,6 +1070,7 @@ func TestVariablesNormalizer(t *testing.T) { t.Run("field argument mapping", func(t *testing.T) { const fieldArgMappingSchema = ` type Query { + viewer: User user(id: ID!, active: Boolean): User users(name: String!, limit: Int!, offset: Int): [User!]! node(id: ID!): Node @@ -1196,8 +1197,8 @@ func TestVariablesNormalizer(t *testing.T) { { name: "empty mapping when no arguments", operation: ` - query GetUser { - user { name } + query GetViewer { + viewer { id name } }`, variables: `{}`, expectedMapping: FieldArgumentMapping{}, From 98776065d6a834831505a756595696b4e29ed928 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Wed, 11 Feb 2026 16:32:58 +0100 Subject: [PATCH 12/21] fix: support inline fragments --- v2/pkg/ast/path.go | 6 +- .../astnormalization/astnormalization_test.go | 136 ++++++++++++++++++ .../astnormalization/variables_extraction.go | 2 +- ...ation_rule_validate_empty_selection_set.go | 2 +- .../graphql_datasource/graphql_datasource.go | 2 +- .../representation_variable.go | 2 +- .../plan/abstract_selection_rewriter.go | 2 +- ...datasource_filter_collect_nodes_visitor.go | 4 +- .../datasource_filter_resolvable_visitor.go | 2 +- v2/pkg/engine/plan/key_fields_visitor.go | 2 +- v2/pkg/engine/plan/node_selection_visitor.go | 2 +- v2/pkg/engine/plan/path_builder_visitor.go | 12 +- v2/pkg/engine/plan/provides_fields_visitor.go | 2 +- v2/pkg/engine/plan/required_fields_visitor.go | 2 +- v2/pkg/engine/plan/visitor.go | 6 +- 15 files changed, 161 insertions(+), 23 deletions(-) diff --git a/v2/pkg/ast/path.go b/v2/pkg/ast/path.go index 19493ec60c..79f60b6b55 100644 --- a/v2/pkg/ast/path.go +++ b/v2/pkg/ast/path.go @@ -128,7 +128,7 @@ func (p Path) String() string { return out } -func (p Path) DotDelimitedString() string { +func (p Path) DotDelimitedString(useFragmentRefs bool) string { builder := strings.Builder{} toGrow := 0 @@ -160,7 +160,9 @@ func (p Path) DotDelimitedString() string { } case InlineFragmentName: builder.WriteString(InlineFragmentPathPrefix) - builder.WriteString(strconv.Itoa(p[i].FragmentRef)) + if useFragmentRefs { + builder.WriteString(strconv.Itoa(p[i].FragmentRef)) + } builder.WriteString(unsafebytes.BytesToString(p[i].FieldName)) } } diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index 3f8f9fb811..89856a37aa 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1233,6 +1233,142 @@ func TestVariablesNormalizer(t *testing.T) { "query.fourthAlias.id": "b", }, }, + { + name: "inline fragment with arguments includes dollar prefix in path", + operation: ` + query GetNode($id: ID!, $limit: Int!) { + node(id: $id) { + id + ... on User { + name + posts(limit: $limit) { title } + } + } + }`, + variables: `{"id": "123", "limit": 10}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": "id", + "query.node.$User.posts.limit": "limit", + }, + }, + { + name: "multiple inline fragments with arguments each get dollar prefix", + operation: ` + query GetNode($id: ID!, $userLimit: Int!, $adminLimit: Int!) { + node(id: $id) { + id + ... on User { + name + items(limit: $userLimit) { name } + } + ... on Admin { + role + items(limit: $adminLimit) { name } + } + } + }`, + variables: `{"id": "123", "userLimit": 5, "adminLimit": 10}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": "id", + "query.node.$User.items.limit": "userLimit", + "query.node.$Admin.items.limit": "adminLimit", + }, + }, + { + name: "inline fragment with literal argument gets extracted variable", + operation: ` + query GetNode($id: ID!) { + node(id: $id) { + id + ... on User { + name + posts(limit: 20) { title } + } + } + }`, + variables: `{"id": "123"}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": "id", + "query.node.$User.posts.limit": "a", + }, + }, + { + name: "nested inline fragments include all dollar prefixes in path", + operation: ` + query GetNode($id: ID!, $limit: Int!) { + node(id: $id) { + ... on User { + User { + posts(limit: $limit) { title } + } + } + } + }`, + variables: `{"id": "123", "limit": 10}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": "id", + "query.node.$User.User.posts.limit": "limit", + }, + }, + { + name: "inline fragment in aliased field includes alias in path", + operation: ` + query GetNode($id: ID!, $limit: Int!) { + nodeAlias: node(id: $id) { + id + ... on User { + name + posts(limit: $limit) { title } + } + } + }`, + variables: `{"id": "123", "limit": 10}`, + expectedMapping: FieldArgumentMapping{ + "query.nodeAlias.id": "id", + "query.nodeAlias.$User.posts.limit": "limit", + }, + }, + { + name: "multiple inline fragments with mixed variables and literals", + operation: ` + query GetNode($id: ID!, $userLimit: Int!) { + node(id: $id) { + id + ... on User { + items(limit: $userLimit) { name } + } + ... on Admin { + items(limit: 15) { name } + } + } + }`, + variables: `{"id": "123", "userLimit": 5}`, + expectedMapping: FieldArgumentMapping{ + "query.node.id": "id", + "query.node.$User.items.limit": "userLimit", + "query.node.$Admin.items.limit": "a", + }, + }, + { + // There is no hard requirement for excluding directive arguments + // but at the moment this is not supported because it would involve more + // complex changes to the variable normalizer. + name: "directive arguments are not included in field argument mapping", + operation: ` + query GetUser($userId: ID!, $limit: Int!, $includeItems: Boolean!, $skipPosts: Boolean!) { + user(id: $userId) { + name + posts(limit: $limit) @skip(if: $skipPosts) { title } + items(limit: 10) @include(if: $includeItems) { name } + } + }`, + variables: `{"userId": "123", "limit": 5, "includeItems": true, "skipPosts": false}`, + expectedMapping: FieldArgumentMapping{ + "query.user.id": "userId", + "query.user.posts.limit": "limit", + "query.user.items.limit": "a", + }, + }, } for _, tc := range testCases { diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 9989152cf0..56f2aa2a48 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -186,7 +186,7 @@ func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName return } - fieldPath := v.Path.DotDelimitedString() + fieldPath := v.Path.DotDelimitedString(false) if fieldPath == "" { return } diff --git a/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go b/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go index ce6dd4e8c4..eb71e593b4 100644 --- a/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go +++ b/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go @@ -32,6 +32,6 @@ func (r *emptySelectionSetVisitor) EnterDocument(operation, definition *ast.Docu func (r *emptySelectionSetVisitor) EnterSelectionSet(ref int) { if r.operation.SelectionSetIsEmpty(ref) { - r.Walker.StopWithInternalErr(fmt.Errorf("astvalidation selection set on path %s is empty", r.Walker.Path.DotDelimitedString())) + r.Walker.StopWithInternalErr(fmt.Errorf("astvalidation selection set on path %s is empty", r.Walker.Path.DotDelimitedString(true))) } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 8d46cfb6e1..4a63797c4e 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -782,7 +782,7 @@ func (p *Planner[T]) allowField(ref int) bool { // In addition, we skip field if its path are equal to planner parent path // This is required to correctly plan on datasource which has corresponding child/root node, // but we don't need to add it to the query as we are in the nested request - currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(), fieldAliasOrName) + currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(true), fieldAliasOrName) if p.dataSourcePlannerConfig.ParentPath != "query" && p.dataSourcePlannerConfig.ParentPath == currentPath { p.DebugPrint("allowField: false path:", currentPath, "is equal to parent path", p.dataSourcePlannerConfig.ParentPath) return false diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go index 18cb3ad204..83ad1a6ece 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go @@ -210,7 +210,7 @@ func (v *representationVariableVisitor) EnterField(ref int) { } fieldDefinitionType := v.definition.FieldDefinitionType(fieldDefinition) - currentPath := v.Walker.Path.DotDelimitedString() + "." + string(fieldName) + currentPath := v.Walker.Path.DotDelimitedString(true) + "." + string(fieldName) // if path was remapped during the planning we update its path fieldPath := string(fieldName) diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter.go b/v2/pkg/engine/plan/abstract_selection_rewriter.go index 2cf814ee5e..46d41d22fa 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter.go @@ -726,7 +726,7 @@ type AbstractFieldPathCollector struct { } func (v *AbstractFieldPathCollector) EnterField(ref int) { - parentPath := v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + parentPath := v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) currentFieldName := v.operation.FieldNameString(ref) currentPath := parentPath + "." + currentFieldName diff --git a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go index b9d47ad3ba..2d03b6a9ca 100644 --- a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go @@ -577,9 +577,9 @@ func (f *treeBuilderVisitor) collectFieldInfo(fieldRef int) { fieldName := f.operation.FieldNameUnsafeString(fieldRef) fieldAliasOrName := f.operation.FieldAliasOrNameString(fieldRef) isTypeName := fieldName == typeNameField - parentPath := f.walker.Path.DotDelimitedString() + parentPath := f.walker.Path.DotDelimitedString(true) onFragment := f.walker.Path.EndsWithFragment() - parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) currentPath := fmt.Sprintf("%s.%s", parentPath, fieldAliasOrName) currentPathWithoutFragments := fmt.Sprintf("%s.%s", parentPathWithoutFragment, fieldAliasOrName) diff --git a/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go b/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go index 8efbe79960..4a392bbb26 100644 --- a/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go @@ -42,7 +42,7 @@ func (f *nodesResolvableVisitor) EnterField(ref int) { } } - parentPath := f.walker.Path.DotDelimitedString() + parentPath := f.walker.Path.DotDelimitedString(true) currentPath := parentPath + "." + fieldAliasOrName _, found := f.nodes.HasSuggestionForPath(typeName, fieldName, currentPath) diff --git a/v2/pkg/engine/plan/key_fields_visitor.go b/v2/pkg/engine/plan/key_fields_visitor.go index 1e018d56dd..29bb05f5ca 100644 --- a/v2/pkg/engine/plan/key_fields_visitor.go +++ b/v2/pkg/engine/plan/key_fields_visitor.go @@ -198,7 +198,7 @@ func (v *keyInfoVisitor) EnterField(ref int) { fieldName := v.input.key.FieldNameUnsafeString(ref) typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) - parentPath := v.input.parentPath + strings.TrimPrefix(v.walker.Path.DotDelimitedString(), v.input.typeName) + parentPath := v.input.parentPath + strings.TrimPrefix(v.walker.Path.DotDelimitedString(true), v.input.typeName) currentPath := parentPath + "." + fieldName hasRootNode := v.input.dataSource.HasRootNode(typeName, fieldName) diff --git a/v2/pkg/engine/plan/node_selection_visitor.go b/v2/pkg/engine/plan/node_selection_visitor.go index bbcffd3c43..8fb8588aba 100644 --- a/v2/pkg/engine/plan/node_selection_visitor.go +++ b/v2/pkg/engine/plan/node_selection_visitor.go @@ -222,7 +222,7 @@ func (c *nodeSelectionVisitor) handleEnterField(fieldRef int, handleRequires boo c.debugPrint("EnterField ref:", fieldRef, "fieldName:", fieldName, "typeName:", typeName, "requires:", handleRequires) - parentPath := c.walker.Path.DotDelimitedString() + parentPath := c.walker.Path.DotDelimitedString(true) currentPath := parentPath + "." + fieldAliasOrName suggestions := c.nodeSuggestions.SuggestionsForPath(typeName, fieldName, currentPath) diff --git a/v2/pkg/engine/plan/path_builder_visitor.go b/v2/pkg/engine/plan/path_builder_visitor.go index 7ccb90bb60..0f2f3de077 100644 --- a/v2/pkg/engine/plan/path_builder_visitor.go +++ b/v2/pkg/engine/plan/path_builder_visitor.go @@ -362,8 +362,8 @@ func (c *pathBuilderVisitor) EnterSelectionSet(ref int) { return } - parentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString() - currentPath := c.walker.Path.DotDelimitedString() + parentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString(true) + currentPath := c.walker.Path.DotDelimitedString(true) typeName := c.operation.InlineFragmentTypeConditionNameString(ancestor.Ref) node, ok := c.definition.NodeByNameStr(typeName) @@ -439,7 +439,7 @@ func (c *pathBuilderVisitor) EnterField(fieldRef int) { c.debugPrint("EnterField ref:", fieldRef, "fieldName:", fieldName, "typeName:", typeName) - parentPath := c.walker.Path.DotDelimitedString() + parentPath := c.walker.Path.DotDelimitedString(true) // we need to also check preceding path for inline fragments // as for the field within inline fragment the parent path will include type condition in a path // but planner path still will not include it @@ -457,7 +457,7 @@ func (c *pathBuilderVisitor) EnterField(fieldRef int) { precedingPath = c.walker.Path[:i-1] } if precedingPath != nil { - precedingParentPath = precedingPath.DotDelimitedString() + precedingParentPath = precedingPath.DotDelimitedString(true) } currentPath := parentPath + "." + fieldAliasOrName @@ -629,7 +629,7 @@ func (c *pathBuilderVisitor) fieldHasAuthorizationRule(typeName, fieldName strin } func (c *pathBuilderVisitor) fieldIsChildNode(plannerIdx int) bool { - path := c.walker.Path.DotDelimitedString() + path := c.walker.Path.DotDelimitedString(true) plannerPath := c.planners[plannerIdx].ParentPath() fieldPath := strings.TrimPrefix(path, plannerPath) return strings.ContainsAny(fieldPath, ".") @@ -925,7 +925,7 @@ func (c *pathBuilderVisitor) addNewPlanner(fieldRef int, typeName, fieldName, cu plannerPath := parentPath if isParentFragment { - precedingFragmentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString() + precedingFragmentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString(true) // if the parent is a fragment, we add the preceding parent path as well // to be able to walk selection sets in the fragment paths = append([]pathConfiguration{ diff --git a/v2/pkg/engine/plan/provides_fields_visitor.go b/v2/pkg/engine/plan/provides_fields_visitor.go index 50cf19eed2..d28c1fec7c 100644 --- a/v2/pkg/engine/plan/provides_fields_visitor.go +++ b/v2/pkg/engine/plan/provides_fields_visitor.go @@ -101,7 +101,7 @@ func (v *providesVisitor) EnterField(ref int) { fieldName := v.input.providesFieldSet.FieldNameUnsafeString(ref) typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) - currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) parentPath := v.input.parentPath + strings.TrimPrefix(currentPathWithoutFragments, v.pathPrefix) currentPath := parentPath + "." + fieldName diff --git a/v2/pkg/engine/plan/required_fields_visitor.go b/v2/pkg/engine/plan/required_fields_visitor.go index 2123605015..597460a8d0 100644 --- a/v2/pkg/engine/plan/required_fields_visitor.go +++ b/v2/pkg/engine/plan/required_fields_visitor.go @@ -319,7 +319,7 @@ func (v *requiredFieldsVisitor) addRequiredField(keyRef int, fieldName ast.ByteS Name: v.config.operation.Input.AppendInputBytes(fullAliasName), } - currentPath := v.Walker.Path.DotDelimitedString() + "." + string(fieldName) + currentPath := v.Walker.Path.DotDelimitedString(true) + "." + string(fieldName) v.mapping[currentPath] = string(fullAliasName) } diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index dbd89a7ee6..3e278162c5 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -151,7 +151,7 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any v.pathCache[kind] = make(map[int]string) } if path == "" { - path = v.Walker.Path.DotDelimitedString() + path = v.Walker.Path.DotDelimitedString(true) if kind == astvisitor.EnterField || kind == astvisitor.LeaveField { path = path + "." + v.Operation.FieldAliasOrNameString(ref) } @@ -259,9 +259,9 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any } func (v *Visitor) currentFullPath(skipFragments bool) string { - path := v.Walker.Path.DotDelimitedString() + path := v.Walker.Path.DotDelimitedString(true) if skipFragments { - path = v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + path = v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) } if v.Walker.CurrentKind == ast.NodeKindField { From 15dbb3f58dc99298f4fd2e344c313eca27260161 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 12 Feb 2026 11:31:07 +0100 Subject: [PATCH 13/21] fix: add missing argument after main merge --- v2/pkg/engine/plan/node_selection_visitor.go | 4 ++-- v2/pkg/engine/plan/required_fields_provided_visitor.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/v2/pkg/engine/plan/node_selection_visitor.go b/v2/pkg/engine/plan/node_selection_visitor.go index a6aa3a15df..72c448e692 100644 --- a/v2/pkg/engine/plan/node_selection_visitor.go +++ b/v2/pkg/engine/plan/node_selection_visitor.go @@ -716,13 +716,13 @@ func (c *nodeSelectionVisitor) rewriteSelectionSetHavingAbstractFragments(fieldR rewriter, err := newFieldSelectionRewriter(c.operation, c.definition, ds, options...) if err != nil { - c.walker.StopWithInternalErr(fmt.Errorf("failed to create field selection rewriter for field %s at path %s: %w", c.operation.FieldNameString(fieldRef), c.walker.Path.DotDelimitedString(), err)) + c.walker.StopWithInternalErr(fmt.Errorf("failed to create field selection rewriter for field %s at path %s: %w", c.operation.FieldNameString(fieldRef), c.walker.Path.DotDelimitedString(true), err)) return } result, err := rewriter.RewriteFieldSelection(fieldRef, c.walker.EnclosingTypeDefinition) if err != nil { - c.walker.StopWithInternalErr(fmt.Errorf("failed to rewrite field selection for field %s at path %s: %w", c.operation.FieldNameString(fieldRef), c.walker.Path.DotDelimitedString(), err)) + c.walker.StopWithInternalErr(fmt.Errorf("failed to rewrite field selection for field %s at path %s: %w", c.operation.FieldNameString(fieldRef), c.walker.Path.DotDelimitedString(true), err)) return } diff --git a/v2/pkg/engine/plan/required_fields_provided_visitor.go b/v2/pkg/engine/plan/required_fields_provided_visitor.go index 557e099526..3ce4a75f9b 100644 --- a/v2/pkg/engine/plan/required_fields_provided_visitor.go +++ b/v2/pkg/engine/plan/required_fields_provided_visitor.go @@ -82,7 +82,7 @@ func (v *requiredFieldsProvidedVisitor) EnterField(ref int) { typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) currentFieldName := v.key.FieldNameUnsafeString(ref) - currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() + currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) // remove the parent type name from the path because we are walking a fragment with the required fields parentPath := v.input.parentPath + strings.TrimPrefix(currentPathWithoutFragments, v.input.typeName) currentPath := parentPath + "." + currentFieldName From f7a1c98e7627acc8ead7e9a6ee0f9848f3c32535 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:45:07 +0100 Subject: [PATCH 14/21] chore: preserve original method signature --- v2/pkg/ast/path.go | 17 +++++++++++++++-- v2/pkg/astnormalization/variables_extraction.go | 2 +- ...eration_rule_validate_empty_selection_set.go | 2 +- .../graphql_datasource/graphql_datasource.go | 2 +- .../representation_variable.go | 2 +- .../engine/plan/abstract_selection_rewriter.go | 2 +- .../datasource_filter_collect_nodes_visitor.go | 4 ++-- .../datasource_filter_resolvable_visitor.go | 2 +- v2/pkg/engine/plan/key_fields_visitor.go | 2 +- v2/pkg/engine/plan/node_selection_visitor.go | 6 +++--- v2/pkg/engine/plan/path_builder_visitor.go | 12 ++++++------ v2/pkg/engine/plan/provides_fields_visitor.go | 2 +- .../plan/required_fields_provided_visitor.go | 2 +- v2/pkg/engine/plan/required_fields_visitor.go | 2 +- v2/pkg/engine/plan/visitor.go | 6 +++--- 15 files changed, 39 insertions(+), 26 deletions(-) diff --git a/v2/pkg/ast/path.go b/v2/pkg/ast/path.go index 79f60b6b55..aa133fbc99 100644 --- a/v2/pkg/ast/path.go +++ b/v2/pkg/ast/path.go @@ -128,7 +128,20 @@ func (p Path) String() string { return out } -func (p Path) DotDelimitedString(useFragmentRefs bool) string { +// DotDelimitedString returns a dot-separated string representation of the path. +// Inline fragments include their reference number, e.g., "query.user.$1User.name". +func (p Path) DotDelimitedString() string { + return p.dotDelimitedString(true) +} + +// DotDelimitedStringWithoutFragmentRefs returns a dot-separated string representation of the path. +// Unlike DotDelimitedString, inline fragments omit their reference number, +// e.g., "query.user.$User.name". +func (p Path) DotDelimitedStringWithoutFragmentRefs() string { + return p.dotDelimitedString(false) +} + +func (p Path) dotDelimitedString(includeFragmentRefs bool) string { builder := strings.Builder{} toGrow := 0 @@ -160,7 +173,7 @@ func (p Path) DotDelimitedString(useFragmentRefs bool) string { } case InlineFragmentName: builder.WriteString(InlineFragmentPathPrefix) - if useFragmentRefs { + if includeFragmentRefs { builder.WriteString(strconv.Itoa(p[i].FragmentRef)) } builder.WriteString(unsafebytes.BytesToString(p[i].FieldName)) diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 56f2aa2a48..1bc27edd08 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -186,7 +186,7 @@ func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName return } - fieldPath := v.Path.DotDelimitedString(false) + fieldPath := v.Path.DotDelimitedStringWithoutFragmentRefs() if fieldPath == "" { return } diff --git a/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go b/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go index eb71e593b4..ce6dd4e8c4 100644 --- a/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go +++ b/v2/pkg/astvalidation/operation_rule_validate_empty_selection_set.go @@ -32,6 +32,6 @@ func (r *emptySelectionSetVisitor) EnterDocument(operation, definition *ast.Docu func (r *emptySelectionSetVisitor) EnterSelectionSet(ref int) { if r.operation.SelectionSetIsEmpty(ref) { - r.Walker.StopWithInternalErr(fmt.Errorf("astvalidation selection set on path %s is empty", r.Walker.Path.DotDelimitedString(true))) + r.Walker.StopWithInternalErr(fmt.Errorf("astvalidation selection set on path %s is empty", r.Walker.Path.DotDelimitedString())) } } diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 4a63797c4e..8d46cfb6e1 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -782,7 +782,7 @@ func (p *Planner[T]) allowField(ref int) bool { // In addition, we skip field if its path are equal to planner parent path // This is required to correctly plan on datasource which has corresponding child/root node, // but we don't need to add it to the query as we are in the nested request - currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(true), fieldAliasOrName) + currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(), fieldAliasOrName) if p.dataSourcePlannerConfig.ParentPath != "query" && p.dataSourcePlannerConfig.ParentPath == currentPath { p.DebugPrint("allowField: false path:", currentPath, "is equal to parent path", p.dataSourcePlannerConfig.ParentPath) return false diff --git a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go index 83ad1a6ece..18cb3ad204 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go +++ b/v2/pkg/engine/datasource/graphql_datasource/representation_variable.go @@ -210,7 +210,7 @@ func (v *representationVariableVisitor) EnterField(ref int) { } fieldDefinitionType := v.definition.FieldDefinitionType(fieldDefinition) - currentPath := v.Walker.Path.DotDelimitedString(true) + "." + string(fieldName) + currentPath := v.Walker.Path.DotDelimitedString() + "." + string(fieldName) // if path was remapped during the planning we update its path fieldPath := string(fieldName) diff --git a/v2/pkg/engine/plan/abstract_selection_rewriter.go b/v2/pkg/engine/plan/abstract_selection_rewriter.go index 46d41d22fa..2cf814ee5e 100644 --- a/v2/pkg/engine/plan/abstract_selection_rewriter.go +++ b/v2/pkg/engine/plan/abstract_selection_rewriter.go @@ -726,7 +726,7 @@ type AbstractFieldPathCollector struct { } func (v *AbstractFieldPathCollector) EnterField(ref int) { - parentPath := v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + parentPath := v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString() currentFieldName := v.operation.FieldNameString(ref) currentPath := parentPath + "." + currentFieldName diff --git a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go index ac639e2759..82f05fddea 100644 --- a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go @@ -592,9 +592,9 @@ func (f *treeBuilderVisitor) collectFieldInfo(fieldRef int) { fieldName := f.operation.FieldNameUnsafeString(fieldRef) fieldAliasOrName := f.operation.FieldAliasOrNameString(fieldRef) isTypeName := fieldName == typeNameField - parentPath := f.walker.Path.DotDelimitedString(true) + parentPath := f.walker.Path.DotDelimitedString() onFragment := f.walker.Path.EndsWithFragment() - parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() currentPath := fmt.Sprintf("%s.%s", parentPath, fieldAliasOrName) currentPathWithoutFragments := fmt.Sprintf("%s.%s", parentPathWithoutFragment, fieldAliasOrName) diff --git a/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go b/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go index 4a392bbb26..8efbe79960 100644 --- a/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_resolvable_visitor.go @@ -42,7 +42,7 @@ func (f *nodesResolvableVisitor) EnterField(ref int) { } } - parentPath := f.walker.Path.DotDelimitedString(true) + parentPath := f.walker.Path.DotDelimitedString() currentPath := parentPath + "." + fieldAliasOrName _, found := f.nodes.HasSuggestionForPath(typeName, fieldName, currentPath) diff --git a/v2/pkg/engine/plan/key_fields_visitor.go b/v2/pkg/engine/plan/key_fields_visitor.go index 036d7ca63a..0707dcb9dd 100644 --- a/v2/pkg/engine/plan/key_fields_visitor.go +++ b/v2/pkg/engine/plan/key_fields_visitor.go @@ -197,7 +197,7 @@ func (v *keyInfoVisitor) EnterField(ref int) { fieldName := v.input.key.FieldNameUnsafeString(ref) typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) - parentPath := v.input.parentPath + strings.TrimPrefix(v.walker.Path.DotDelimitedString(true), v.input.typeName) + parentPath := v.input.parentPath + strings.TrimPrefix(v.walker.Path.DotDelimitedString(), v.input.typeName) currentPath := parentPath + "." + fieldName hasRootNode := v.input.dataSource.HasRootNode(typeName, fieldName) diff --git a/v2/pkg/engine/plan/node_selection_visitor.go b/v2/pkg/engine/plan/node_selection_visitor.go index 72c448e692..db8403cd3c 100644 --- a/v2/pkg/engine/plan/node_selection_visitor.go +++ b/v2/pkg/engine/plan/node_selection_visitor.go @@ -221,7 +221,7 @@ func (c *nodeSelectionVisitor) handleEnterField(fieldRef int, handleRequires boo c.debugPrint("EnterField ref:", fieldRef, "fieldName:", fieldName, "typeName:", typeName, "requires:", handleRequires) - parentPath := c.walker.Path.DotDelimitedString(true) + parentPath := c.walker.Path.DotDelimitedString() currentPath := parentPath + "." + fieldAliasOrName suggestions := c.nodeSuggestions.SuggestionsForPath(typeName, fieldName, currentPath) @@ -716,13 +716,13 @@ func (c *nodeSelectionVisitor) rewriteSelectionSetHavingAbstractFragments(fieldR rewriter, err := newFieldSelectionRewriter(c.operation, c.definition, ds, options...) if err != nil { - c.walker.StopWithInternalErr(fmt.Errorf("failed to create field selection rewriter for field %s at path %s: %w", c.operation.FieldNameString(fieldRef), c.walker.Path.DotDelimitedString(true), err)) + c.walker.StopWithInternalErr(fmt.Errorf("failed to create field selection rewriter for field %s at path %s: %w", c.operation.FieldNameString(fieldRef), c.walker.Path.DotDelimitedString(), err)) return } result, err := rewriter.RewriteFieldSelection(fieldRef, c.walker.EnclosingTypeDefinition) if err != nil { - c.walker.StopWithInternalErr(fmt.Errorf("failed to rewrite field selection for field %s at path %s: %w", c.operation.FieldNameString(fieldRef), c.walker.Path.DotDelimitedString(true), err)) + c.walker.StopWithInternalErr(fmt.Errorf("failed to rewrite field selection for field %s at path %s: %w", c.operation.FieldNameString(fieldRef), c.walker.Path.DotDelimitedString(), err)) return } diff --git a/v2/pkg/engine/plan/path_builder_visitor.go b/v2/pkg/engine/plan/path_builder_visitor.go index 8945aa08b5..b66b41375a 100644 --- a/v2/pkg/engine/plan/path_builder_visitor.go +++ b/v2/pkg/engine/plan/path_builder_visitor.go @@ -362,8 +362,8 @@ func (c *pathBuilderVisitor) EnterSelectionSet(ref int) { return } - parentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString(true) - currentPath := c.walker.Path.DotDelimitedString(true) + parentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString() + currentPath := c.walker.Path.DotDelimitedString() typeName := c.operation.InlineFragmentTypeConditionNameString(ancestor.Ref) node, ok := c.definition.NodeByNameStr(typeName) @@ -439,7 +439,7 @@ func (c *pathBuilderVisitor) EnterField(fieldRef int) { c.debugPrint("EnterField ref:", fieldRef, "fieldName:", fieldName, "typeName:", typeName) - parentPath := c.walker.Path.DotDelimitedString(true) + parentPath := c.walker.Path.DotDelimitedString() // we need to also check preceding path for inline fragments // as for the field within inline fragment the parent path will include type condition in a path // but planner path still will not include it @@ -457,7 +457,7 @@ func (c *pathBuilderVisitor) EnterField(fieldRef int) { precedingPath = c.walker.Path[:i-1] } if precedingPath != nil { - precedingParentPath = precedingPath.DotDelimitedString(true) + precedingParentPath = precedingPath.DotDelimitedString() } currentPath := parentPath + "." + fieldAliasOrName @@ -629,7 +629,7 @@ func (c *pathBuilderVisitor) fieldHasAuthorizationRule(typeName, fieldName strin } func (c *pathBuilderVisitor) fieldIsChildNode(plannerIdx int) bool { - path := c.walker.Path.DotDelimitedString(true) + path := c.walker.Path.DotDelimitedString() plannerPath := c.planners[plannerIdx].ParentPath() fieldPath := strings.TrimPrefix(path, plannerPath) return strings.ContainsAny(fieldPath, ".") @@ -906,7 +906,7 @@ func (c *pathBuilderVisitor) addNewPlanner(fieldRef int, typeName, fieldName, cu plannerPath := parentPath if isParentFragment { - precedingFragmentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString(true) + precedingFragmentPath := c.walker.Path[:len(c.walker.Path)-1].DotDelimitedString() // if the parent is a fragment, we add the preceding parent path as well // to be able to walk selection sets in the fragment paths = append([]pathConfiguration{ diff --git a/v2/pkg/engine/plan/provides_fields_visitor.go b/v2/pkg/engine/plan/provides_fields_visitor.go index 95a9ebbc1b..060e54817f 100644 --- a/v2/pkg/engine/plan/provides_fields_visitor.go +++ b/v2/pkg/engine/plan/provides_fields_visitor.go @@ -101,7 +101,7 @@ func (v *providesVisitor) EnterField(ref int) { fieldName := v.providesFieldSet.FieldNameUnsafeString(ref) typeName := v.walker.EnclosingTypeDefinition.NameString(v.definition) - currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() // remove the parent type name from the path because we are walking a fragment with the provided fields parentPath := v.parentPath + strings.TrimPrefix(currentPathWithoutFragments, v.parentTypeName) currentPath := parentPath + "." + fieldName diff --git a/v2/pkg/engine/plan/required_fields_provided_visitor.go b/v2/pkg/engine/plan/required_fields_provided_visitor.go index 3ce4a75f9b..557e099526 100644 --- a/v2/pkg/engine/plan/required_fields_provided_visitor.go +++ b/v2/pkg/engine/plan/required_fields_provided_visitor.go @@ -82,7 +82,7 @@ func (v *requiredFieldsProvidedVisitor) EnterField(ref int) { typeName := v.walker.EnclosingTypeDefinition.NameString(v.input.definition) currentFieldName := v.key.FieldNameUnsafeString(ref) - currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + currentPathWithoutFragments := v.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() // remove the parent type name from the path because we are walking a fragment with the required fields parentPath := v.input.parentPath + strings.TrimPrefix(currentPathWithoutFragments, v.input.typeName) currentPath := parentPath + "." + currentFieldName diff --git a/v2/pkg/engine/plan/required_fields_visitor.go b/v2/pkg/engine/plan/required_fields_visitor.go index 597460a8d0..2123605015 100644 --- a/v2/pkg/engine/plan/required_fields_visitor.go +++ b/v2/pkg/engine/plan/required_fields_visitor.go @@ -319,7 +319,7 @@ func (v *requiredFieldsVisitor) addRequiredField(keyRef int, fieldName ast.ByteS Name: v.config.operation.Input.AppendInputBytes(fullAliasName), } - currentPath := v.Walker.Path.DotDelimitedString(true) + "." + string(fieldName) + currentPath := v.Walker.Path.DotDelimitedString() + "." + string(fieldName) v.mapping[currentPath] = string(fullAliasName) } diff --git a/v2/pkg/engine/plan/visitor.go b/v2/pkg/engine/plan/visitor.go index 3e278162c5..dbd89a7ee6 100644 --- a/v2/pkg/engine/plan/visitor.go +++ b/v2/pkg/engine/plan/visitor.go @@ -151,7 +151,7 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any v.pathCache[kind] = make(map[int]string) } if path == "" { - path = v.Walker.Path.DotDelimitedString(true) + path = v.Walker.Path.DotDelimitedString() if kind == astvisitor.EnterField || kind == astvisitor.LeaveField { path = path + "." + v.Operation.FieldAliasOrNameString(ref) } @@ -259,9 +259,9 @@ func (v *Visitor) AllowVisitor(kind astvisitor.VisitorKind, ref int, visitor any } func (v *Visitor) currentFullPath(skipFragments bool) string { - path := v.Walker.Path.DotDelimitedString(true) + path := v.Walker.Path.DotDelimitedString() if skipFragments { - path = v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString(true) + path = v.Walker.Path.WithoutInlineFragmentNames().DotDelimitedString() } if v.Walker.CurrentKind == ast.NodeKindField { From 40a7750874af14a9c1c828baed7c132a299f3ea7 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 12 Feb 2026 15:20:25 +0100 Subject: [PATCH 15/21] chore: add path string creation tests --- v2/pkg/ast/path_test.go | 219 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 219 insertions(+) create mode 100644 v2/pkg/ast/path_test.go diff --git a/v2/pkg/ast/path_test.go b/v2/pkg/ast/path_test.go new file mode 100644 index 0000000000..f7577b53a9 --- /dev/null +++ b/v2/pkg/ast/path_test.go @@ -0,0 +1,219 @@ +package ast_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" +) + +func TestPath_DotDelimitedString(t *testing.T) { + tests := []struct { + name string + path ast.Path + want string + wantNoRef string + }{ + { + name: "returns operation type for root query path", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + }, + want: "query", + wantNoRef: "query", + }, + { + name: "returns operation type for root mutation path", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("mutation")}, + }, + want: "mutation", + wantNoRef: "mutation", + }, + { + name: "returns operation type for root subscription path", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("subscription")}, + }, + want: "subscription", + wantNoRef: "subscription", + }, + { + name: "converts empty field name to query as fallback", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("")}, + }, + want: "query", + wantNoRef: "query", + }, + { + name: "joins operation and field with dot separator", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("user")}, + }, + want: "query.user", + wantNoRef: "query.user", + }, + { + name: "nested query fields contain all elements in path", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("user")}, + {Kind: ast.FieldName, FieldName: []byte("name")}, + }, + want: "query.user.name", + wantNoRef: "query.user.name", + }, + { + name: "nested mutation fields contain all elements in path", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("mutation")}, + {Kind: ast.FieldName, FieldName: []byte("createUser")}, + {Kind: ast.FieldName, FieldName: []byte("id")}, + }, + want: "mutation.createUser.id", + wantNoRef: "mutation.createUser.id", + }, + { + name: "nested subscription fields contain all elements in path", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("subscription")}, + {Kind: ast.FieldName, FieldName: []byte("userUpdated")}, + {Kind: ast.FieldName, FieldName: []byte("name")}, + }, + want: "subscription.userUpdated.name", + wantNoRef: "subscription.userUpdated.name", + }, + { + name: "includes field aliases in path", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("myUser")}, // alias is stored in path + {Kind: ast.FieldName, FieldName: []byte("email")}, + }, + want: "query.myUser.email", + wantNoRef: "query.myUser.email", + }, + { + name: "array indexes are represented as numbers", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("users")}, + {Kind: ast.ArrayIndex, ArrayIndex: 0}, + {Kind: ast.FieldName, FieldName: []byte("name")}, + }, + want: "query.users.0.name", + wantNoRef: "query.users.0.name", + }, + { + name: "multiple array indexes are all included in sequence", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("matrix")}, + {Kind: ast.ArrayIndex, ArrayIndex: 0}, + {Kind: ast.ArrayIndex, ArrayIndex: 1}, + }, + want: "query.matrix.0.1", + wantNoRef: "query.matrix.0.1", + }, + { + name: "inline fragments are prefixed with dollar sign and include fragment ref", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("node")}, + {Kind: ast.InlineFragmentName, FieldName: []byte("User"), FragmentRef: 1}, + {Kind: ast.FieldName, FieldName: []byte("name")}, + }, + want: "query.node.$1User.name", + wantNoRef: "query.node.$User.name", + }, + { + name: "multiple inline fragments each include their own ref number", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("search")}, + {Kind: ast.InlineFragmentName, FieldName: []byte("User"), FragmentRef: 1}, + {Kind: ast.FieldName, FieldName: []byte("profile")}, + {Kind: ast.InlineFragmentName, FieldName: []byte("PublicProfile"), FragmentRef: 2}, + {Kind: ast.FieldName, FieldName: []byte("bio")}, + }, + want: "query.search.$1User.profile.$2PublicProfile.bio", + wantNoRef: "query.search.$User.profile.$PublicProfile.bio", + }, + { + name: "inline fragments work in subscription operations", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("subscription")}, + {Kind: ast.FieldName, FieldName: []byte("messageAdded")}, + {Kind: ast.InlineFragmentName, FieldName: []byte("TextMessage"), FragmentRef: 1}, + {Kind: ast.FieldName, FieldName: []byte("text")}, + }, + want: "subscription.messageAdded.$1TextMessage.text", + wantNoRef: "subscription.messageAdded.$TextMessage.text", + }, + { + name: "combines fields, array indexes, and fragments in correct order", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("items")}, + {Kind: ast.ArrayIndex, ArrayIndex: 0}, + {Kind: ast.InlineFragmentName, FieldName: []byte("Product"), FragmentRef: 1}, + {Kind: ast.FieldName, FieldName: []byte("variants")}, + {Kind: ast.ArrayIndex, ArrayIndex: 2}, + {Kind: ast.FieldName, FieldName: []byte("price")}, + }, + want: "query.items.0.$1Product.variants.2.price", + wantNoRef: "query.items.0.$Product.variants.2.price", + }, + { + name: "handles deeply nested paths with multiple fragments and arrays", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("organization")}, + {Kind: ast.FieldName, FieldName: []byte("teams")}, + {Kind: ast.ArrayIndex, ArrayIndex: 1}, + {Kind: ast.InlineFragmentName, FieldName: []byte("EngineeringTeam"), FragmentRef: 1}, + {Kind: ast.FieldName, FieldName: []byte("members")}, + {Kind: ast.ArrayIndex, ArrayIndex: 0}, + {Kind: ast.InlineFragmentName, FieldName: []byte("Developer"), FragmentRef: 2}, + {Kind: ast.FieldName, FieldName: []byte("languages")}, + }, + want: "query.organization.teams.1.$1EngineeringTeam.members.0.$2Developer.languages", + wantNoRef: "query.organization.teams.1.$EngineeringTeam.members.0.$Developer.languages", + }, + { + name: "combines aliases and inline fragments", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("mutation")}, + {Kind: ast.FieldName, FieldName: []byte("myUser")}, // aliased field + {Kind: ast.InlineFragmentName, FieldName: []byte("AdminUser"), FragmentRef: 1}, + {Kind: ast.FieldName, FieldName: []byte("permissions")}, + }, + want: "mutation.myUser.$1AdminUser.permissions", + wantNoRef: "mutation.myUser.$AdminUser.permissions", + }, + { + name: "zero fragment refs are included in output", + path: ast.Path{ + {Kind: ast.FieldName, FieldName: []byte("query")}, + {Kind: ast.FieldName, FieldName: []byte("entity")}, + {Kind: ast.InlineFragmentName, FieldName: []byte("Node"), FragmentRef: 0}, + {Kind: ast.FieldName, FieldName: []byte("id")}, + }, + want: "query.entity.$0Node.id", + wantNoRef: "query.entity.$Node.id", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.path.DotDelimitedString() + assert.Equal(t, tt.want, got) + + gotNoRef := tt.path.DotDelimitedStringWithoutFragmentRefs() + assert.Equal(t, tt.wantNoRef, gotNoRef) + }) + } +} From 0fbb3135b2a5aa01ac38d6470ff29e7f441e8252 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 12 Feb 2026 15:46:56 +0100 Subject: [PATCH 16/21] chore: add godoc to variable normalizer methods --- v2/pkg/astnormalization/astnormalization.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index 0aeec14968..7cd605b33a 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -353,6 +353,9 @@ type VariablesNormalizer struct { variablesExtractionVisitor *variablesExtractionVisitor } +// NewVariablesNormalizer creates a new variable normalizer. +// If withFieldArgMapping is true then the normalizer creates a +// mapping of field arguments as a "side product" when NormalizeOperation is called. func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { // delete unused modifying variables refs, // so it is safer to run it sequentially with the extraction @@ -382,6 +385,13 @@ func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { } } +// NormalizeOperation processes GraphQL operation variables. +// It detects and removes unused variables, extracts variables from inline values +// and coerces variable types. It modifies the operation in place and +// returns metadata including field argument mappings and upload paths. +// Field argument mapping is done only when v is configured to do so via NewVariablesNormalizer, +// else VariablesNormalizerResult.FieldArgumentMapping will be nil. +// Any errors encountered during normalization are reported via the report parameter. func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) VariablesNormalizerResult { v.firstDetectUnused.Walk(operation, definition, report) if report.HasErrors() { From 1166211916f0ceef289a8c2e22b03b69048164ea Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 12 Feb 2026 15:50:36 +0100 Subject: [PATCH 17/21] chore: use string concat on path build instead of Sprintf --- .../datasource/graphql_datasource/graphql_datasource.go | 2 +- v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go index 8d46cfb6e1..5c0734865c 100644 --- a/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go +++ b/v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go @@ -782,7 +782,7 @@ func (p *Planner[T]) allowField(ref int) bool { // In addition, we skip field if its path are equal to planner parent path // This is required to correctly plan on datasource which has corresponding child/root node, // but we don't need to add it to the query as we are in the nested request - currentPath := fmt.Sprintf("%s.%s", p.visitor.Walker.Path.DotDelimitedString(), fieldAliasOrName) + currentPath := p.visitor.Walker.Path.DotDelimitedString() + "." + fieldAliasOrName if p.dataSourcePlannerConfig.ParentPath != "query" && p.dataSourcePlannerConfig.ParentPath == currentPath { p.DebugPrint("allowField: false path:", currentPath, "is equal to parent path", p.dataSourcePlannerConfig.ParentPath) return false diff --git a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go index 82f05fddea..6f981d9523 100644 --- a/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go +++ b/v2/pkg/engine/plan/datasource_filter_collect_nodes_visitor.go @@ -596,8 +596,8 @@ func (f *treeBuilderVisitor) collectFieldInfo(fieldRef int) { onFragment := f.walker.Path.EndsWithFragment() parentPathWithoutFragment := f.walker.Path.WithoutInlineFragmentNames().DotDelimitedString() - currentPath := fmt.Sprintf("%s.%s", parentPath, fieldAliasOrName) - currentPathWithoutFragments := fmt.Sprintf("%s.%s", parentPathWithoutFragment, fieldAliasOrName) + currentPath := parentPath + "." + fieldAliasOrName + currentPathWithoutFragments := parentPathWithoutFragment + "." + fieldAliasOrName f.fieldInfo[fieldRef] = fieldInfo{ typeName: typeName, From 7d265d76134674b41ce95f17ffdcccb92f6fad5e Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 19 Feb 2026 12:59:34 +0100 Subject: [PATCH 18/21] chore: update godoc/comments + result type name --- v2/pkg/astnormalization/astnormalization.go | 16 +++++++++------- v2/pkg/astnormalization/astnormalization_test.go | 16 ++++++++++++++++ .../astnormalization/field_argument_mapping.go | 4 ++-- v2/pkg/astnormalization/variables_extraction.go | 11 ++++------- 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index 7cd605b33a..92a9658602 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -387,27 +387,29 @@ func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { // NormalizeOperation processes GraphQL operation variables. // It detects and removes unused variables, extracts variables from inline values -// and coerces variable types. It modifies the operation in place and +// and coerces variable values. +// https://spec.graphql.org/September2025/#sec-Coercing-Variable-Values +// It modifies the operation in place and // returns metadata including field argument mappings and upload paths. -// Field argument mapping is done only when v is configured to do so via NewVariablesNormalizer, +// Field argument mapping is done only when it is enabled in v, // else VariablesNormalizerResult.FieldArgumentMapping will be nil. // Any errors encountered during normalization are reported via the report parameter. -func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) VariablesNormalizerResult { +func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) VariablesNormalizationResult { v.firstDetectUnused.Walk(operation, definition, report) if report.HasErrors() { - return VariablesNormalizerResult{} + return VariablesNormalizationResult{} } v.secondExtract.Walk(operation, definition, report) if report.HasErrors() { - return VariablesNormalizerResult{} + return VariablesNormalizationResult{} } v.thirdDeleteUnused.Walk(operation, definition, report) if report.HasErrors() { - return VariablesNormalizerResult{} + return VariablesNormalizationResult{} } v.fourthCoerce.Walk(operation, definition, report) - return VariablesNormalizerResult{ + return VariablesNormalizationResult{ UploadsMapping: v.variablesExtractionVisitor.uploadsPath, FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping.result, } diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index 89856a37aa..9c5d705e6b 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1073,8 +1073,13 @@ func TestVariablesNormalizer(t *testing.T) { viewer: User user(id: ID!, active: Boolean): User users(name: String!, limit: Int!, offset: Int): [User!]! + paginatedUsers(paging: Paging!): [String!]! node(id: ID!): Node } + input Paging { + limit: Int! + offset: Int! + } interface Node { id: ID! User: UserContainer @@ -1369,6 +1374,17 @@ func TestVariablesNormalizer(t *testing.T) { "query.user.items.limit": "a", }, }, + { + name: "inline input object argument is extracted as a variable", + operation: ` + query Search($limit: Int!, $offset: Int!) { + paginatedUsers(paging: {limit: $limit, offset: $offset}) + }`, + variables: `{"limit": 10, "offset": 5}`, + expectedMapping: FieldArgumentMapping{ + "query.paginatedUsers.paging": "a", + }, + }, } for _, tc := range testCases { diff --git a/v2/pkg/astnormalization/field_argument_mapping.go b/v2/pkg/astnormalization/field_argument_mapping.go index a6a0cd0d56..d460cb4a5b 100644 --- a/v2/pkg/astnormalization/field_argument_mapping.go +++ b/v2/pkg/astnormalization/field_argument_mapping.go @@ -10,8 +10,8 @@ import ( // Value: name of variable of field argument after variable normalization. type FieldArgumentMapping map[string]string -// VariablesNormalizerResult contains the results of variable normalization. -type VariablesNormalizerResult struct { +// VariablesNormalizationResult contains the results of variable normalization. +type VariablesNormalizationResult struct { // UploadsMapping tracks file upload variables and how their paths change during normalization. UploadsMapping []uploads.UploadPathMapping // FieldArgumentMapping maps field arguments to their variable names for fast lookup. diff --git a/v2/pkg/astnormalization/variables_extraction.go b/v2/pkg/astnormalization/variables_extraction.go index 1bc27edd08..f3950b42c5 100644 --- a/v2/pkg/astnormalization/variables_extraction.go +++ b/v2/pkg/astnormalization/variables_extraction.go @@ -181,7 +181,8 @@ func (v *variablesExtractionVisitor) EnterDocument(operation, definition *ast.Do // If varName is empty, it looks up the variable name from the operation or stores the literal value. func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName string) { // Guard to prevent nil panics. - // If v.fieldArgumentMapping.result is nil, then field argument mapping is disabled. + // v.fieldArgumentMapping.result should always have a value if we are here + // but if this changes by accident this prevents a nil panic. if v.fieldArgumentMapping.result == nil { return } @@ -200,12 +201,8 @@ func (v *variablesExtractionVisitor) recordFieldArgumentMapping(ref int, varName } if v.operation.Arguments[ref].Value.Kind != ast.ValueKindVariable { - // We expect the operation on this visitor to be normalized before the visitor walks it. - // This means that values of any field arguments should be extracted to variables. - // If we still land here it means the query hasn't been normalized before. - // In this case we ignore mapping the field argument, i.e. we do not support mapping field - // arguments with literal values. - // It's not impossible to do so, just expensive and not needed at the moment. + // We should not land here because at this point all argument values should have become variables. + // If we land here for whatever reason we ignore this field argument. return } From a43dae9b97ddc7c9b2a242f661cf8d57074c3e4a Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 19 Feb 2026 13:11:23 +0100 Subject: [PATCH 19/21] chore: seperate file for VariablesNormalizer --- v2/pkg/astnormalization/astnormalization.go | 70 -------------- .../field_argument_mapping.go | 19 ---- .../variables_normalization.go | 92 +++++++++++++++++++ 3 files changed, 92 insertions(+), 89 deletions(-) delete mode 100644 v2/pkg/astnormalization/field_argument_mapping.go create mode 100644 v2/pkg/astnormalization/variables_normalization.go diff --git a/v2/pkg/astnormalization/astnormalization.go b/v2/pkg/astnormalization/astnormalization.go index 92a9658602..7989b055e5 100644 --- a/v2/pkg/astnormalization/astnormalization.go +++ b/v2/pkg/astnormalization/astnormalization.go @@ -345,76 +345,6 @@ func (o *OperationNormalizer) NormalizeNamedOperation(operation, definition *ast } } -type VariablesNormalizer struct { - firstDetectUnused *astvisitor.Walker - secondExtract *astvisitor.Walker - thirdDeleteUnused *astvisitor.Walker - fourthCoerce *astvisitor.Walker - variablesExtractionVisitor *variablesExtractionVisitor -} - -// NewVariablesNormalizer creates a new variable normalizer. -// If withFieldArgMapping is true then the normalizer creates a -// mapping of field arguments as a "side product" when NormalizeOperation is called. -func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { - // delete unused modifying variables refs, - // so it is safer to run it sequentially with the extraction - thirdDeleteUnused := astvisitor.NewWalkerWithID(8, "DeleteUnusedVariables") - del := deleteUnusedVariables(&thirdDeleteUnused) - - // register variable usage detection on the first stage - // and pass usage information to the deletion visitor - // so it keeps variables that are defined but not used at all - // ensuring that validation can still catch them - firstDetectUnused := astvisitor.NewWalkerWithID(8, "DetectVariableUsage") - detectVariableUsage(&firstDetectUnused, del) - - secondExtract := astvisitor.NewWalkerWithID(8, "ExtractVariables") - variablesExtractionVisitor := extractVariables(&secondExtract, withFieldArgMapping) - extractVariablesDefaultValue(&secondExtract) - - fourthCoerce := astvisitor.NewWalkerWithID(0, "VariablesCoercion") - inputCoercionForList(&fourthCoerce) - - return &VariablesNormalizer{ - firstDetectUnused: &firstDetectUnused, - secondExtract: &secondExtract, - thirdDeleteUnused: &thirdDeleteUnused, - fourthCoerce: &fourthCoerce, - variablesExtractionVisitor: variablesExtractionVisitor, - } -} - -// NormalizeOperation processes GraphQL operation variables. -// It detects and removes unused variables, extracts variables from inline values -// and coerces variable values. -// https://spec.graphql.org/September2025/#sec-Coercing-Variable-Values -// It modifies the operation in place and -// returns metadata including field argument mappings and upload paths. -// Field argument mapping is done only when it is enabled in v, -// else VariablesNormalizerResult.FieldArgumentMapping will be nil. -// Any errors encountered during normalization are reported via the report parameter. -func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) VariablesNormalizationResult { - v.firstDetectUnused.Walk(operation, definition, report) - if report.HasErrors() { - return VariablesNormalizationResult{} - } - v.secondExtract.Walk(operation, definition, report) - if report.HasErrors() { - return VariablesNormalizationResult{} - } - v.thirdDeleteUnused.Walk(operation, definition, report) - if report.HasErrors() { - return VariablesNormalizationResult{} - } - v.fourthCoerce.Walk(operation, definition, report) - - return VariablesNormalizationResult{ - UploadsMapping: v.variablesExtractionVisitor.uploadsPath, - FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping.result, - } -} - type fragmentCycleVisitor struct { *astvisitor.Walker diff --git a/v2/pkg/astnormalization/field_argument_mapping.go b/v2/pkg/astnormalization/field_argument_mapping.go deleted file mode 100644 index d460cb4a5b..0000000000 --- a/v2/pkg/astnormalization/field_argument_mapping.go +++ /dev/null @@ -1,19 +0,0 @@ -package astnormalization - -import ( - "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" -) - -// FieldArgumentMapping maps the path of field arguments inside an operation -// to the name of their mapped variables. -// Key: "rootOperationType.fieldPath.argumentName" (e.g., "query.posts.limit") -// Value: name of variable of field argument after variable normalization. -type FieldArgumentMapping map[string]string - -// VariablesNormalizationResult contains the results of variable normalization. -type VariablesNormalizationResult struct { - // UploadsMapping tracks file upload variables and how their paths change during normalization. - UploadsMapping []uploads.UploadPathMapping - // FieldArgumentMapping maps field arguments to their variable names for fast lookup. - FieldArgumentMapping FieldArgumentMapping -} diff --git a/v2/pkg/astnormalization/variables_normalization.go b/v2/pkg/astnormalization/variables_normalization.go new file mode 100644 index 0000000000..67cdab4686 --- /dev/null +++ b/v2/pkg/astnormalization/variables_normalization.go @@ -0,0 +1,92 @@ +package astnormalization + +import ( + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization/uploads" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" + "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" +) + +type VariablesNormalizer struct { + firstDetectUnused *astvisitor.Walker + secondExtract *astvisitor.Walker + thirdDeleteUnused *astvisitor.Walker + fourthCoerce *astvisitor.Walker + variablesExtractionVisitor *variablesExtractionVisitor +} + +// NewVariablesNormalizer creates a new variable normalizer. +// If withFieldArgMapping is true then the normalizer creates a +// mapping of field arguments as a "side product" when NormalizeOperation is called. +func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { + // delete unused modifying variables refs, + // so it is safer to run it sequentially with the extraction + thirdDeleteUnused := astvisitor.NewWalkerWithID(8, "DeleteUnusedVariables") + del := deleteUnusedVariables(&thirdDeleteUnused) + + // register variable usage detection on the first stage + // and pass usage information to the deletion visitor + // so it keeps variables that are defined but not used at all + // ensuring that validation can still catch them + firstDetectUnused := astvisitor.NewWalkerWithID(8, "DetectVariableUsage") + detectVariableUsage(&firstDetectUnused, del) + + secondExtract := astvisitor.NewWalkerWithID(8, "ExtractVariables") + variablesExtractionVisitor := extractVariables(&secondExtract, withFieldArgMapping) + extractVariablesDefaultValue(&secondExtract) + + fourthCoerce := astvisitor.NewWalkerWithID(0, "VariablesCoercion") + inputCoercionForList(&fourthCoerce) + + return &VariablesNormalizer{ + firstDetectUnused: &firstDetectUnused, + secondExtract: &secondExtract, + thirdDeleteUnused: &thirdDeleteUnused, + fourthCoerce: &fourthCoerce, + variablesExtractionVisitor: variablesExtractionVisitor, + } +} + +// NormalizeOperation processes GraphQL operation variables. +// It detects and removes unused variables, extracts variables from inline values +// and coerces variable values. +// https://spec.graphql.org/September2025/#sec-Coercing-Variable-Values +// It modifies the operation in place and +// returns metadata including field argument mappings and upload paths. +// Field argument mapping is done only when it is enabled in v, +// else VariablesNormalizerResult.FieldArgumentMapping will be nil. +// Any errors encountered during normalization are reported via the report parameter. +func (v *VariablesNormalizer) NormalizeOperation(operation, definition *ast.Document, report *operationreport.Report) VariablesNormalizationResult { + v.firstDetectUnused.Walk(operation, definition, report) + if report.HasErrors() { + return VariablesNormalizationResult{} + } + v.secondExtract.Walk(operation, definition, report) + if report.HasErrors() { + return VariablesNormalizationResult{} + } + v.thirdDeleteUnused.Walk(operation, definition, report) + if report.HasErrors() { + return VariablesNormalizationResult{} + } + v.fourthCoerce.Walk(operation, definition, report) + + return VariablesNormalizationResult{ + UploadsMapping: v.variablesExtractionVisitor.uploadsPath, + FieldArgumentMapping: v.variablesExtractionVisitor.fieldArgumentMapping.result, + } +} + +// FieldArgumentMapping maps the path of field arguments inside an operation +// to the name of their mapped variables. +// Key: "rootOperationType.fieldPath.argumentName" (e.g., "query.posts.limit") +// Value: name of variable of field argument after variable normalization. +type FieldArgumentMapping map[string]string + +// VariablesNormalizationResult contains the results of variable normalization. +type VariablesNormalizationResult struct { + // UploadsMapping tracks file upload variables and how their paths change during normalization. + UploadsMapping []uploads.UploadPathMapping + // FieldArgumentMapping maps field arguments to their variable names for fast lookup. + FieldArgumentMapping FieldArgumentMapping +} From 24625732bc333af24736969d005b2f03e0e18806 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 19 Feb 2026 13:16:07 +0100 Subject: [PATCH 20/21] chore: Add options type for VariablesNormalizer --- v2/pkg/astnormalization/variables_normalization.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/v2/pkg/astnormalization/variables_normalization.go b/v2/pkg/astnormalization/variables_normalization.go index 67cdab4686..06d7a51ee4 100644 --- a/v2/pkg/astnormalization/variables_normalization.go +++ b/v2/pkg/astnormalization/variables_normalization.go @@ -15,10 +15,15 @@ type VariablesNormalizer struct { variablesExtractionVisitor *variablesExtractionVisitor } +// VariablesNormalizerOptions allows to configure a VariablesNormalizer. +type VariablesNormalizerOptions struct { + // EnableFieldArgumentMapping enables field argument mapping. + // If true it contains the map as part of the NormalizeOperation result. + EnableFieldArgumentMapping bool +} + // NewVariablesNormalizer creates a new variable normalizer. -// If withFieldArgMapping is true then the normalizer creates a -// mapping of field arguments as a "side product" when NormalizeOperation is called. -func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { +func NewVariablesNormalizer(options VariablesNormalizerOptions) *VariablesNormalizer { // delete unused modifying variables refs, // so it is safer to run it sequentially with the extraction thirdDeleteUnused := astvisitor.NewWalkerWithID(8, "DeleteUnusedVariables") @@ -32,7 +37,7 @@ func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { detectVariableUsage(&firstDetectUnused, del) secondExtract := astvisitor.NewWalkerWithID(8, "ExtractVariables") - variablesExtractionVisitor := extractVariables(&secondExtract, withFieldArgMapping) + variablesExtractionVisitor := extractVariables(&secondExtract, options.EnableFieldArgumentMapping) extractVariablesDefaultValue(&secondExtract) fourthCoerce := astvisitor.NewWalkerWithID(0, "VariablesCoercion") From 0d27f891fe0c75c91dc544fec2b9486465bf3a9d Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 19 Feb 2026 13:22:53 +0100 Subject: [PATCH 21/21] fix: fix broken tests --- v2/pkg/astnormalization/astnormalization_test.go | 6 +++--- v2/pkg/astnormalization/variables_mapper_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/v2/pkg/astnormalization/astnormalization_test.go b/v2/pkg/astnormalization/astnormalization_test.go index 9c5d705e6b..3c13fd7280 100644 --- a/v2/pkg/astnormalization/astnormalization_test.go +++ b/v2/pkg/astnormalization/astnormalization_test.go @@ -1018,7 +1018,7 @@ func TestVariablesNormalizer(t *testing.T) { operationDocument := unsafeparser.ParseGraphqlDocumentString(input) operationDocument.Input.Variables = []byte(`{}`) - normalizer := NewVariablesNormalizer(false) + normalizer := NewVariablesNormalizer(VariablesNormalizerOptions{EnableFieldArgumentMapping: false}) report := operationreport.Report{} normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) require.False(t, report.HasErrors(), report.Error()) @@ -1041,7 +1041,7 @@ func TestVariablesNormalizer(t *testing.T) { operationDocument.Input.Variables = []byte(`{"varOne":[{"oneList":[{"list":[null,null],"value":null}],"one":{"list":[null],"value":null}}],"varTwo":{"oneList":[{"list":[null,null],"value":null}],"one":{"list":[null],"value":null}}}`) // create normalizer without field arg mapping - normalizer := NewVariablesNormalizer(false) + normalizer := NewVariablesNormalizer(VariablesNormalizerOptions{EnableFieldArgumentMapping: false}) report := operationreport.Report{} result := normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) require.False(t, report.HasErrors(), report.Error()) @@ -1393,7 +1393,7 @@ func TestVariablesNormalizer(t *testing.T) { operationDocument := unsafeparser.ParseGraphqlDocumentString(tc.operation) operationDocument.Input.Variables = []byte(tc.variables) - normalizer := NewVariablesNormalizer(true) + normalizer := NewVariablesNormalizer(VariablesNormalizerOptions{EnableFieldArgumentMapping: true}) report := operationreport.Report{} result := normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) require.False(t, report.HasErrors(), report.Error()) diff --git a/v2/pkg/astnormalization/variables_mapper_test.go b/v2/pkg/astnormalization/variables_mapper_test.go index 6b399a5460..1d65d19f5c 100644 --- a/v2/pkg/astnormalization/variables_mapper_test.go +++ b/v2/pkg/astnormalization/variables_mapper_test.go @@ -47,7 +47,7 @@ func TestVariablesMapper(t *testing.T) { WithRemoveFragmentDefinitions(), WithRemoveUnusedVariables(), ) - variablesNormalizer := NewVariablesNormalizer(false) + variablesNormalizer := NewVariablesNormalizer(VariablesNormalizerOptions{EnableFieldArgumentMapping: false}) testCases := []struct { name string