-
Notifications
You must be signed in to change notification settings - Fork 158
feat: add field argument mapping #1373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
ce23ea5
1ebb2c3
ea5c630
73a4ac5
cd47b12
1b8bddc
fdf63d0
a85a37b
546a496
e2c713b
75236fc
e4c44d3
f629d8d
9877606
94178d4
15dbb3f
f7a1c98
40a7750
0fbb313
1166211
1e8d2a5
7d265d7
a43dae9
2462573
0d27f89
55c1a82
f4e3312
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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" | ||
| ) | ||
|
|
@@ -245,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, | ||
|
|
@@ -353,7 +353,7 @@ type VariablesNormalizer struct { | |
| variablesExtractionVisitor *variablesExtractionVisitor | ||
| } | ||
|
|
||
| func NewVariablesNormalizer() *VariablesNormalizer { | ||
| func NewVariablesNormalizer(withFieldArgMapping bool) *VariablesNormalizer { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you think about introducing either an options or config argument? e.g. or just
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
| // delete unused modifying variables refs, | ||
| // so it is safer to run it sequentially with the extraction | ||
| thirdDeleteUnused := astvisitor.NewWalkerWithID(8, "DeleteUnusedVariables") | ||
|
|
@@ -367,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") | ||
|
|
@@ -382,22 +382,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.result, | ||
| } | ||
| } | ||
|
|
||
| type fragmentCycleVisitor struct { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,9 +1040,10 @@ 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{} | ||
| 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 +1061,163 @@ 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 not populated | ||
| assert.Nil(t, result.FieldArgumentMapping) | ||
| }) | ||
|
|
||
| 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!]! | ||
| node(id: ID!): Node | ||
| } | ||
| 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 { | ||
| name string | ||
| operation string | ||
| variables string | ||
| expectedMapping FieldArgumentMapping | ||
| }{ | ||
| { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing test case for complex input type, smth like this such query will be normalized into
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| 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{}, | ||
| }, | ||
| } | ||
|
|
||
| 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(true) | ||
| report := operationreport.Report{} | ||
| result := normalizer.NormalizeOperation(&operationDocument, &definitionDocument, &report) | ||
| require.False(t, report.HasErrors(), report.Error()) | ||
|
|
||
| assert.Equal(t, tc.expectedMapping, result.FieldArgumentMapping) | ||
| }) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -1089,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 | ||
|
|
||
|
|
@@ -1185,15 +1342,15 @@ 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...) | ||
| } | ||
|
|
||
| var runWithVariablesExtractionAndPreNormalize = func(t *testing.T, normalizeFunc registerNormalizeVariablesFunc, definition, operation, operationName, expectedOutput, variablesInput, expectedVariables string, prerequisites ...registerNormalizeFunc) { | ||
| t.Helper() | ||
|
|
||
| runWithVariablesAssertAndPreNormalize(t, func(walker *astvisitor.Walker) { | ||
| normalizeFunc(walker) | ||
| normalizeFunc(walker, false) | ||
| }, definition, operation, operationName, expectedOutput, variablesInput, expectedVariables, prerequisites...) | ||
| } | ||
|
|
||
|
|
@@ -1289,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) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| 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 | ||
|
|
||
| // VariablesNormalizerResult contains the results of variable normalization. | ||
| type VariablesNormalizerResult struct { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do you think about I propose to move VariablesNormalizer into its own file
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| // 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 | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.