From f6d76f5b3d2744dade309b5b42e3f0546db4c861 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Tue, 2 Dec 2025 18:22:32 +0100 Subject: [PATCH 01/22] feat: provide map of field arguments in operation context --- router/core/context.go | 14 +++-- router/core/graphql_prehandler.go | 5 ++ router/core/operation_planner.go | 100 ++++++++++++++++++++++++++++++ router/core/websocket.go | 1 + 4 files changed, 116 insertions(+), 4 deletions(-) diff --git a/router/core/context.go b/router/core/context.go index 6065cfffc4..fd6d244066 100644 --- a/router/core/context.go +++ b/router/core/context.go @@ -482,6 +482,7 @@ type OperationContext interface { Hash() uint64 // Content is the content of the operation Content() string + FieldArguments() map[string]map[string]any // Variables is the variables of the operation Variables() *astjson.Value // ClientInfo returns information about the client that initiated this operation @@ -524,10 +525,11 @@ type operationContext struct { // RawContent is the raw content of the operation rawContent string // Content is the normalized content of the operation - content string - variables *astjson.Value - files []*httpclient.FileUpload - clientInfo *ClientInfo + content string + fieldArguments map[string]map[string]any + variables *astjson.Value + files []*httpclient.FileUpload + clientInfo *ClientInfo // preparedPlan is the prepared plan of the operation preparedPlan *planWithMetaData traceOptions resolve.TraceOptions @@ -558,6 +560,10 @@ func (o *operationContext) Variables() *astjson.Value { return o.variables } +func (o *operationContext) FieldArguments() map[string]map[string]any { + return o.fieldArguments +} + func (o *operationContext) Files() []*httpclient.FileUpload { return o.files } diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index bbc3473034..9f30255298 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -908,6 +908,11 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v requestContext.operation.rawContent = operationKit.parsedOperation.Request.Query requestContext.operation.content = operationKit.parsedOperation.NormalizedRepresentation requestContext.operation.variables, err = variablesParser.ParseBytes(operationKit.parsedOperation.Request.Variables) + requestContext.operation.fieldArguments = mapFieldArguments( + operationKit.kit.doc, + requestContext.operation.variables, + operationKit.parsedOperation.RemapVariables, + ) if err != nil { rtrace.AttachErrToSpan(engineNormalizeSpan, err) if !requestContext.operation.traceOptions.ExcludeNormalizeStats { diff --git a/router/core/operation_planner.go b/router/core/operation_planner.go index 40c8d6701e..a9c70005f7 100644 --- a/router/core/operation_planner.go +++ b/router/core/operation_planner.go @@ -6,6 +6,7 @@ import ( "golang.org/x/sync/singleflight" + "github.com/wundergraph/astjson" "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" @@ -92,6 +93,105 @@ func (p *OperationPlanner) preparePlan(ctx *operationContext) (*planWithMetaData return out, nil } +// mapFieldArguments returns all field arguments inside doc as a map. +// +// The key of the map is a dot-notated path, where each element refers +// to a field. The inner map holds the argument names as keys. +// map["rootfield1.subfield1"]["arg1"] +// map["rootfield1.subfield1"]["arg2"] +// map["rootfield1.subfield2"]["arg1"] +// map["rootfield1.subfield2"]["arg2"] +// +// The value of the inner map is the literal value of the argument. +// Variables will be resolved in case they have been used for arguments. +func mapFieldArguments(doc *ast.Document, vars *astjson.Value, remapVariables map[string]string) map[string]map[string]any { + // TODO: Currently we only map root field arguments. I.e. + // map["rootfield1"]["arg1"] + // map["rootfield2"]["arg1"] + // Needs to extended to support subfields, like + // map["rootfield1.subfield1"]["arg1"] + selectionSet := doc.OperationDefinitions[0].SelectionSet + fields := doc.SelectionSetFieldSelections(selectionSet) + + args := make(map[string]map[string]any, len(fields)) + + for _, field := range fields { + fieldRef := doc.Selections[field].Ref + fieldName := doc.FieldNameString(fieldRef) + fieldArgs := doc.FieldArguments(fieldRef) + + for _, fieldArg := range fieldArgs { + m := make(map[string]any, len(fieldArgs)) + args[fieldName] = m + + argName := doc.ArgumentNameString(fieldArg) + val := doc.Arguments[fieldArg].Value + args[fieldName][argName] = getArgValue(doc, val, vars, remapVariables) + } + } + + return args +} + +// getArgValue returns the actual value of val. +// It resolves variables in case these have been used for arguments, +// else it will use values from doc. +func getArgValue(doc *ast.Document, val ast.Value, variables *astjson.Value, remapVariables map[string]string) any { + if val.Kind != ast.ValueKindVariable { + // TODO delete this comment. + // I observed we never actually hit this code path because the operation parser + // automically creates variables and maps them to arguments, even if no initial variables are provided. + // We should probably still have this in place just in case. + // Maybe there is a better way than to to use ValueToJSON but I haven't found one yet. + actualValue, err := doc.ValueToJSON(val) + if err != nil { + // TODO error handling + return nil + } + // TODO: Type cast to what this actually is, it returns only []byte atm + return actualValue + } + + varName := doc.VariableValueNameString(val.Ref) + originalVarName := varName + if remapVariables != nil { + if original, ok := remapVariables[varName]; ok { + originalVarName = original + } + } + + varValue := variables.Get(originalVarName) + switch varValue.Type() { + case astjson.TypeNumber: + return varValue.GetInt() + case astjson.TypeString: + return string(varValue.GetStringBytes()) + case astjson.TypeObject: + // TODO maybe create map out of varValue to give hook developers a better experience. + // The problem is this would be a nested operation because objects can contain all kinds + // of children elements, such as numbers, strings, other objects, etc. + // Right now we only return the astjson type directly and leave it to the hook developer + // to work with that. + return varValue.GetObject() + case astjson.TypeArray: + // TODO maybe create slice out of varValue to give hook developers a better experience. + // The problem is this would be a nested operation because arrays can contain all kinds + // of children elements, such as numbers, strings, other objects, etc. + // Right now we only return the astjson type directly and leave it to the hook developer + // to work with that. + return varValue.GetArray() + case astjson.TypeFalse: + // TODO hypothetically written, needs testing + return false + case astjson.TypeTrue: + // TODO hypothetically written, needs testing + return true + default: + // TODO test for type = null + return nil + } +} + type PlanOptions struct { ClientInfo *ClientInfo TraceOptions resolve.TraceOptions diff --git a/router/core/websocket.go b/router/core/websocket.go index b1999dd813..bb1975feb8 100644 --- a/router/core/websocket.go +++ b/router/core/websocket.go @@ -926,6 +926,7 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi if err != nil { return nil, nil, err } + opContext.fieldArguments = mapFieldArguments(operationKit.kit.doc, opContext.variables, opContext.remapVariables) startValidation := time.Now() From ec45e9e0bc463a446eb8e6634cecb32301df71cd Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Wed, 10 Dec 2025 10:34:35 +0100 Subject: [PATCH 02/22] chore: use a walker --- router/core/field_argument_visitor.go | 129 ++++++++++++++++++++++++ router/core/graphql_prehandler.go | 1 + router/core/operation_planner.go | 138 ++++++++------------------ router/core/websocket.go | 2 +- 4 files changed, 170 insertions(+), 100 deletions(-) create mode 100644 router/core/field_argument_visitor.go diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go new file mode 100644 index 0000000000..69349f2936 --- /dev/null +++ b/router/core/field_argument_visitor.go @@ -0,0 +1,129 @@ +package core + +import ( + "strings" + + "github.com/wundergraph/astjson" + "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" + "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" +) + +type fieldArgumentsVisitor struct { + walker *astvisitor.Walker + operation *ast.Document + definition *ast.Document + variables *astjson.Value + remapVariables map[string]string + fieldArguments map[string]map[string]any +} + +func (v *fieldArgumentsVisitor) EnterDocument(operation, definition *ast.Document) { + v.operation = operation + v.definition = definition +} + +func (v *fieldArgumentsVisitor) EnterArgument(ref int) { + // skip if we don't deal with field arguments (e.g. directive arguments) + anc := v.walker.Ancestors[len(v.walker.Ancestors)-1] + if anc.Kind != ast.NodeKindField { + return + } + + // build path from ancestors + var pathParts []string + for _, anc := range v.walker.Ancestors { + if anc.Kind == ast.NodeKindField { + fieldName := v.operation.FieldNameString(anc.Ref) + pathParts = append(pathParts, fieldName) + } + } + fieldPath := strings.Join(pathParts, ".") + + if v.fieldArguments[fieldPath] == nil { + v.fieldArguments[fieldPath] = make(map[string]any) + } + + argName := v.operation.ArgumentNameString(ref) + val := v.operation.Arguments[ref].Value + v.fieldArguments[fieldPath][argName] = getArgValue(v.operation, val, v.variables, v.remapVariables) +} + +func mapFieldArguments(operation *ast.Document, definition *ast.Document, + vars *astjson.Value, remapVariables map[string]string) map[string]map[string]any { + walker := astvisitor.NewWalker(48) + + visitor := &fieldArgumentsVisitor{ + walker: &walker, + variables: vars, + remapVariables: remapVariables, + fieldArguments: make(map[string]map[string]any), + } + + walker.RegisterEnterDocumentVisitor(visitor) + walker.RegisterEnterArgumentVisitor(visitor) + + report := &operationreport.Report{} + walker.Walk(operation, definition, report) + + return visitor.fieldArguments +} + +// getArgValue returns the actual value of val. +// It resolves variables in case these have been used for arguments, +// else it will use values from doc. +func getArgValue(doc *ast.Document, val ast.Value, variables *astjson.Value, remapVariables map[string]string) any { + if val.Kind != ast.ValueKindVariable { + // TODO delete this comment. + // I observed we never actually hit this code path because the operation parser + // automically creates variables and maps them to arguments, even if no initial variables are provided. + // We should probably still have this in place just in case. + // Maybe there is a better way than to to use ValueToJSON but I haven't found one yet. + actualValue, err := doc.ValueToJSON(val) + if err != nil { + // TODO error handling + return nil + } + // TODO: Type cast to what this actually is, it returns only []byte atm + return actualValue + } + + varName := doc.VariableValueNameString(val.Ref) + originalVarName := varName + if remapVariables != nil { + if original, ok := remapVariables[varName]; ok { + originalVarName = original + } + } + + varValue := variables.Get(originalVarName) + switch varValue.Type() { + case astjson.TypeNumber: + return varValue.GetInt() + case astjson.TypeString: + return string(varValue.GetStringBytes()) + case astjson.TypeObject: + // TODO maybe create map out of varValue to give hook developers a better experience. + // The problem is this would be a nested operation because objects can contain all kinds + // of children elements, such as numbers, strings, other objects, etc. + // Right now we only return the astjson type directly and leave it to the hook developer + // to work with that. + return varValue.GetObject() + case astjson.TypeArray: + // TODO maybe create slice out of varValue to give hook developers a better experience. + // The problem is this would be a nested operation because arrays can contain all kinds + // of children elements, such as numbers, strings, other objects, etc. + // Right now we only return the astjson type directly and leave it to the hook developer + // to work with that. + return varValue.GetArray() + case astjson.TypeFalse: + // TODO hypothetically written, needs testing + return false + case astjson.TypeTrue: + // TODO hypothetically written, needs testing + return true + default: + // TODO test for type = null + return nil + } +} diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index 9f30255298..3e6bb68b1f 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -910,6 +910,7 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v requestContext.operation.variables, err = variablesParser.ParseBytes(operationKit.parsedOperation.Request.Variables) requestContext.operation.fieldArguments = mapFieldArguments( operationKit.kit.doc, + h.executor.ClientSchema, requestContext.operation.variables, operationKit.parsedOperation.RemapVariables, ) diff --git a/router/core/operation_planner.go b/router/core/operation_planner.go index a9c70005f7..09537314ad 100644 --- a/router/core/operation_planner.go +++ b/router/core/operation_planner.go @@ -6,7 +6,6 @@ import ( "golang.org/x/sync/singleflight" - "github.com/wundergraph/astjson" "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" @@ -93,104 +92,45 @@ func (p *OperationPlanner) preparePlan(ctx *operationContext) (*planWithMetaData return out, nil } -// mapFieldArguments returns all field arguments inside doc as a map. -// -// The key of the map is a dot-notated path, where each element refers -// to a field. The inner map holds the argument names as keys. -// map["rootfield1.subfield1"]["arg1"] -// map["rootfield1.subfield1"]["arg2"] -// map["rootfield1.subfield2"]["arg1"] -// map["rootfield1.subfield2"]["arg2"] -// -// The value of the inner map is the literal value of the argument. -// Variables will be resolved in case they have been used for arguments. -func mapFieldArguments(doc *ast.Document, vars *astjson.Value, remapVariables map[string]string) map[string]map[string]any { - // TODO: Currently we only map root field arguments. I.e. - // map["rootfield1"]["arg1"] - // map["rootfield2"]["arg1"] - // Needs to extended to support subfields, like - // map["rootfield1.subfield1"]["arg1"] - selectionSet := doc.OperationDefinitions[0].SelectionSet - fields := doc.SelectionSetFieldSelections(selectionSet) - - args := make(map[string]map[string]any, len(fields)) - - for _, field := range fields { - fieldRef := doc.Selections[field].Ref - fieldName := doc.FieldNameString(fieldRef) - fieldArgs := doc.FieldArguments(fieldRef) - - for _, fieldArg := range fieldArgs { - m := make(map[string]any, len(fieldArgs)) - args[fieldName] = m - - argName := doc.ArgumentNameString(fieldArg) - val := doc.Arguments[fieldArg].Value - args[fieldName][argName] = getArgValue(doc, val, vars, remapVariables) - } - } - - return args -} - -// getArgValue returns the actual value of val. -// It resolves variables in case these have been used for arguments, -// else it will use values from doc. -func getArgValue(doc *ast.Document, val ast.Value, variables *astjson.Value, remapVariables map[string]string) any { - if val.Kind != ast.ValueKindVariable { - // TODO delete this comment. - // I observed we never actually hit this code path because the operation parser - // automically creates variables and maps them to arguments, even if no initial variables are provided. - // We should probably still have this in place just in case. - // Maybe there is a better way than to to use ValueToJSON but I haven't found one yet. - actualValue, err := doc.ValueToJSON(val) - if err != nil { - // TODO error handling - return nil - } - // TODO: Type cast to what this actually is, it returns only []byte atm - return actualValue - } - - varName := doc.VariableValueNameString(val.Ref) - originalVarName := varName - if remapVariables != nil { - if original, ok := remapVariables[varName]; ok { - originalVarName = original - } - } - - varValue := variables.Get(originalVarName) - switch varValue.Type() { - case astjson.TypeNumber: - return varValue.GetInt() - case astjson.TypeString: - return string(varValue.GetStringBytes()) - case astjson.TypeObject: - // TODO maybe create map out of varValue to give hook developers a better experience. - // The problem is this would be a nested operation because objects can contain all kinds - // of children elements, such as numbers, strings, other objects, etc. - // Right now we only return the astjson type directly and leave it to the hook developer - // to work with that. - return varValue.GetObject() - case astjson.TypeArray: - // TODO maybe create slice out of varValue to give hook developers a better experience. - // The problem is this would be a nested operation because arrays can contain all kinds - // of children elements, such as numbers, strings, other objects, etc. - // Right now we only return the astjson type directly and leave it to the hook developer - // to work with that. - return varValue.GetArray() - case astjson.TypeFalse: - // TODO hypothetically written, needs testing - return false - case astjson.TypeTrue: - // TODO hypothetically written, needs testing - return true - default: - // TODO test for type = null - return nil - } -} +// // mapFieldArguments returns all field arguments inside doc as a map. +// // +// // The key of the map is a dot-notated path, where each element refers +// // to a field. The inner map holds the argument names as keys. +// // map["rootfield1.subfield1"]["arg1"] +// // map["rootfield1.subfield1"]["arg2"] +// // map["rootfield1.subfield2"]["arg1"] +// // map["rootfield1.subfield2"]["arg2"] +// // +// // The value of the inner map is the literal value of the argument. +// // Variables will be resolved in case they have been used for arguments. +// func mapFieldArguments(doc *ast.Document, vars *astjson.Value, remapVariables map[string]string) map[string]map[string]any { +// // TODO: Currently we only map root field arguments. I.e. +// // map["rootfield1"]["arg1"] +// // map["rootfield2"]["arg1"] +// // Needs to extended to support subfields, like +// // map["rootfield1.subfield1"]["arg1"] +// selectionSet := doc.OperationDefinitions[0].SelectionSet +// fields := doc.SelectionSetFieldSelections(selectionSet) + +// args := make(map[string]map[string]any, len(fields)) + +// for _, field := range fields { +// fieldRef := doc.Selections[field].Ref +// fieldName := doc.FieldNameString(fieldRef) +// fieldArgs := doc.FieldArguments(fieldRef) + +// for _, fieldArg := range fieldArgs { +// m := make(map[string]any, len(fieldArgs)) +// args[fieldName] = m + +// argName := doc.ArgumentNameString(fieldArg) +// val := doc.Arguments[fieldArg].Value +// args[fieldName][argName] = getArgValue(doc, val, vars, remapVariables) +// } +// } + +// return args +// } type PlanOptions struct { ClientInfo *ClientInfo diff --git a/router/core/websocket.go b/router/core/websocket.go index bb1975feb8..365994a350 100644 --- a/router/core/websocket.go +++ b/router/core/websocket.go @@ -926,7 +926,7 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi if err != nil { return nil, nil, err } - opContext.fieldArguments = mapFieldArguments(operationKit.kit.doc, opContext.variables, opContext.remapVariables) + opContext.fieldArguments = mapFieldArguments(operationKit.kit.doc, h.operationProcessor.executor.ClientSchema, opContext.variables, opContext.remapVariables) startValidation := time.Now() From c9cc1e64f2a0a55b0bd1552e297d61d426e5c314 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 11 Dec 2025 13:53:54 +0100 Subject: [PATCH 03/22] fix: only map arguments when needed --- router/core/graph_server.go | 2 ++ router/core/graphql_prehandler.go | 19 +++++++++++++------ router/core/router_config.go | 4 ++++ router/core/websocket.go | 17 ++++++++++++++++- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/router/core/graph_server.go b/router/core/graph_server.go index 280508c6bf..6002cc17b2 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -1447,6 +1447,7 @@ func (s *graphServer) buildGraphMux( ComputeOperationSha256: computeSha256, ApolloCompatibilityFlags: &s.apolloCompatibilityFlags, DisableVariablesRemapping: s.engineExecutionConfiguration.DisableVariablesRemapping, + MapFieldArguments: s.subscriptionHooks.needFieldArgumentMapping(), ExprManager: exprManager, OmitBatchExtensions: s.batchingConfig.OmitExtensions, @@ -1472,6 +1473,7 @@ func (s *graphServer) buildGraphMux( WebSocketConfiguration: s.webSocketConfiguration, ClientHeader: s.clientHeader, DisableVariablesRemapping: s.engineExecutionConfiguration.DisableVariablesRemapping, + MapFieldArguments: s.subscriptionHooks.needFieldArgumentMapping(), ApolloCompatibilityFlags: s.apolloCompatibilityFlags, }) diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index 3e6bb68b1f..fab50ce630 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -67,6 +67,7 @@ type PreHandlerOptions struct { ComputeOperationSha256 bool ApolloCompatibilityFlags *config.ApolloCompatibilityFlags DisableVariablesRemapping bool + MapFieldArguments bool ExprManager *expr.Manager OmitBatchExtensions bool @@ -102,6 +103,7 @@ type PreHandler struct { apolloCompatibilityFlags *config.ApolloCompatibilityFlags variableParsePool astjson.ParserPool disableVariablesRemapping bool + mapFieldArguments bool exprManager *expr.Manager omitBatchExtensions bool @@ -160,6 +162,7 @@ func NewPreHandler(opts *PreHandlerOptions) *PreHandler { computeOperationSha256: opts.ComputeOperationSha256, apolloCompatibilityFlags: opts.ApolloCompatibilityFlags, disableVariablesRemapping: opts.DisableVariablesRemapping, + mapFieldArguments: opts.MapFieldArguments, exprManager: opts.ExprManager, omitBatchExtensions: opts.OmitBatchExtensions, @@ -908,12 +911,16 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v requestContext.operation.rawContent = operationKit.parsedOperation.Request.Query requestContext.operation.content = operationKit.parsedOperation.NormalizedRepresentation requestContext.operation.variables, err = variablesParser.ParseBytes(operationKit.parsedOperation.Request.Variables) - requestContext.operation.fieldArguments = mapFieldArguments( - operationKit.kit.doc, - h.executor.ClientSchema, - requestContext.operation.variables, - operationKit.parsedOperation.RemapVariables, - ) + + if h.mapFieldArguments { + requestContext.operation.fieldArguments = mapFieldArguments( + operationKit.kit.doc, + h.executor.ClientSchema, + requestContext.operation.variables, + requestContext.operation.remapVariables, + ) + } + if err != nil { rtrace.AttachErrToSpan(engineNormalizeSpan, err) if !requestContext.operation.traceOptions.ExcludeNormalizeStats { diff --git a/router/core/router_config.go b/router/core/router_config.go index 319216a18a..afbe03db78 100644 --- a/router/core/router_config.go +++ b/router/core/router_config.go @@ -46,6 +46,10 @@ type onReceiveEventsHooks struct { timeout time.Duration } +func (h *subscriptionHooks) needFieldArgumentMapping() bool { + return len(h.onStart.handlers) > 0 || len(h.onPublishEvents.handlers) > 0 || len(h.onReceiveEvents.handlers) > 0 +} + type Config struct { clusterName string instanceID string diff --git a/router/core/websocket.go b/router/core/websocket.go index 365994a350..2c91631fe5 100644 --- a/router/core/websocket.go +++ b/router/core/websocket.go @@ -63,6 +63,7 @@ type WebsocketMiddlewareOptions struct { ClientHeader config.ClientHeader DisableVariablesRemapping bool + MapFieldArguments bool ApolloCompatibilityFlags config.ApolloCompatibilityFlags } @@ -85,6 +86,7 @@ func NewWebsocketMiddleware(ctx context.Context, opts WebsocketMiddlewareOptions config: opts.WebSocketConfiguration, clientHeader: opts.ClientHeader, disableVariablesRemapping: opts.DisableVariablesRemapping, + mapFieldArguments: opts.MapFieldArguments, apolloCompatibilityFlags: opts.ApolloCompatibilityFlags, } if opts.WebSocketConfiguration != nil && opts.WebSocketConfiguration.AbsintheProtocol.Enabled { @@ -265,6 +267,7 @@ type WebsocketHandler struct { clientHeader config.ClientHeader disableVariablesRemapping bool + mapFieldArguments bool apolloCompatibilityFlags config.ApolloCompatibilityFlags } @@ -372,6 +375,7 @@ func (h *WebsocketHandler) handleUpgradeRequest(w http.ResponseWriter, r *http.R ForwardUpgradeHeaders: h.forwardUpgradeHeadersConfig, ForwardQueryParams: h.forwardQueryParamsConfig, DisableVariablesRemapping: h.disableVariablesRemapping, + MapFieldArguments: h.mapFieldArguments, ApolloCompatibilityFlags: h.apolloCompatibilityFlags, }) err = handler.Initialize() @@ -711,6 +715,7 @@ type WebSocketConnectionHandlerOptions struct { ForwardUpgradeHeaders forwardConfig ForwardQueryParams forwardConfig DisableVariablesRemapping bool + MapFieldArguments bool ApolloCompatibilityFlags config.ApolloCompatibilityFlags } @@ -748,6 +753,7 @@ type WebSocketConnectionHandler struct { forwardQueryParams *forwardConfig disableVariablesRemapping bool + mapFieldArguments bool apolloCompatibilityFlags config.ApolloCompatibilityFlags @@ -789,6 +795,7 @@ func NewWebsocketConnectionHandler(ctx context.Context, opts WebSocketConnection forwardInitialPayload: opts.ForwardInitialPayload, plannerOptions: opts.PlanOptions, disableVariablesRemapping: opts.DisableVariablesRemapping, + mapFieldArguments: opts.MapFieldArguments, apolloCompatibilityFlags: opts.ApolloCompatibilityFlags, clientInfoFromInitialPayload: opts.ClientInfoFromInitialPayload, } @@ -926,7 +933,15 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi if err != nil { return nil, nil, err } - opContext.fieldArguments = mapFieldArguments(operationKit.kit.doc, h.operationProcessor.executor.ClientSchema, opContext.variables, opContext.remapVariables) + + if h.mapFieldArguments { + opContext.fieldArguments = mapFieldArguments( + operationKit.kit.doc, + h.operationProcessor.executor.ClientSchema, + opContext.variables, + opContext.remapVariables, + ) + } startValidation := time.Now() From 9bf3562098cec2478c793984099c014e2e8fb173 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 11 Dec 2025 15:29:20 +0100 Subject: [PATCH 04/22] chore: clean up --- router/core/context.go | 13 ++- router/core/field_argument_visitor.go | 150 +++++++++++++------------- router/core/field_arguments.go | 31 ++++++ router/core/graphql_prehandler.go | 13 +-- router/core/websocket.go | 13 +-- 5 files changed, 129 insertions(+), 91 deletions(-) create mode 100644 router/core/field_arguments.go diff --git a/router/core/context.go b/router/core/context.go index fd6d244066..de0a40ad7f 100644 --- a/router/core/context.go +++ b/router/core/context.go @@ -482,17 +482,16 @@ type OperationContext interface { Hash() uint64 // Content is the content of the operation Content() string - FieldArguments() map[string]map[string]any - // Variables is the variables of the operation + // FieldArguments allow access to a GraphQL operation's field arguments. + FieldArguments() *FieldArguments + // Variables allow access to GraphQL operation variables used by clients. Variables() *astjson.Value // ClientInfo returns information about the client that initiated this operation ClientInfo() ClientInfo - // Sha256Hash returns the SHA256 hash of the original operation // It is important to note that this hash is not calculated just because this method has been called // and is only calculated based on other existing logic (such as if sha256Hash is used in expressions) Sha256Hash() string - // QueryPlanStats returns some statistics about the query plan for the operation // if called too early in request chain, it may be inaccurate for modules, using // in Middleware is recommended @@ -526,7 +525,7 @@ type operationContext struct { rawContent string // Content is the normalized content of the operation content string - fieldArguments map[string]map[string]any + fieldArguments FieldArguments variables *astjson.Value files []*httpclient.FileUpload clientInfo *ClientInfo @@ -560,8 +559,8 @@ func (o *operationContext) Variables() *astjson.Value { return o.variables } -func (o *operationContext) FieldArguments() map[string]map[string]any { - return o.fieldArguments +func (o *operationContext) FieldArguments() *FieldArguments { + return &o.fieldArguments } func (o *operationContext) Files() []*httpclient.FileUpload { diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go index 69349f2936..6412f079ed 100644 --- a/router/core/field_argument_visitor.go +++ b/router/core/field_argument_visitor.go @@ -1,12 +1,14 @@ package core import ( + "fmt" "strings" "github.com/wundergraph/astjson" "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" + "go.uber.org/zap" ) type fieldArgumentsVisitor struct { @@ -15,7 +17,7 @@ type fieldArgumentsVisitor struct { definition *ast.Document variables *astjson.Value remapVariables map[string]string - fieldArguments map[string]map[string]any + fieldArguments map[string]map[string]*astjson.Value } func (v *fieldArgumentsVisitor) EnterDocument(operation, definition *ast.Document) { @@ -30,100 +32,104 @@ func (v *fieldArgumentsVisitor) EnterArgument(ref int) { return } - // build path from ancestors + fieldPath := v.dotPathFromAncestors() + + if v.fieldArguments[fieldPath] == nil { + v.fieldArguments[fieldPath] = make(map[string]*astjson.Value) + } + + argName := v.operation.ArgumentNameString(ref) + argVal := v.operation.Arguments[ref].Value + + resolvedArgVal, err := v.resolveArgValue(argVal) + if err != nil { + // TODO: Log error somehow + return + } + + v.fieldArguments[fieldPath][argName] = resolvedArgVal +} + +func (v *fieldArgumentsVisitor) dotPathFromAncestors() string { var pathParts []string + for _, anc := range v.walker.Ancestors { if anc.Kind == ast.NodeKindField { fieldName := v.operation.FieldNameString(anc.Ref) pathParts = append(pathParts, fieldName) } } - fieldPath := strings.Join(pathParts, ".") - if v.fieldArguments[fieldPath] == nil { - v.fieldArguments[fieldPath] = make(map[string]any) + return strings.Join(pathParts, ".") +} + +// resolveArgValue returns the value of val as astjson.Value. +func (v *fieldArgumentsVisitor) resolveArgValue(val ast.Value) (*astjson.Value, error) { + if val.Kind != ast.ValueKindVariable { + // Normally we never hit this code path because val.Kind should always be ast.ValueKindVariable. + // The operation parser automically creates variables and maps them to arguments, + // even if no initial variables are provided. + // We should still be able to deal with arguments directly, + // if for some reason they are not mapped to variables. + return v.getArgValueFromDoc(val) } - argName := v.operation.ArgumentNameString(ref) - val := v.operation.Arguments[ref].Value - v.fieldArguments[fieldPath][argName] = getArgValue(v.operation, val, v.variables, v.remapVariables) + return v.getArgValueFromVars(val) +} + +func (v *fieldArgumentsVisitor) getArgValueFromDoc(val ast.Value) (*astjson.Value, error) { + mval, err := v.operation.ValueToJSON(val) + if err != nil { + return nil, fmt.Errorf("marshal ast value: %w", err) + } + + res, err := astjson.ParseBytes(mval) + if err != nil { + return nil, fmt.Errorf("parse marshalled data as astjson.Value: %w", err) + } + + return res, nil } -func mapFieldArguments(operation *ast.Document, definition *ast.Document, - vars *astjson.Value, remapVariables map[string]string) map[string]map[string]any { +func (v *fieldArgumentsVisitor) getArgValueFromVars(val ast.Value) (*astjson.Value, error) { + varName := v.operation.VariableValueNameString(val.Ref) + originalVarName := varName + if v.remapVariables != nil { + if original, ok := v.remapVariables[varName]; ok { + originalVarName = original + } + } + + return v.variables.Get(originalVarName), nil +} + +type mapFieldArgumentsOpts struct { + operation *ast.Document + definition *ast.Document + vars *astjson.Value + remapVariables map[string]string + logger *zap.Logger +} + +func mapFieldArguments(opts mapFieldArgumentsOpts) FieldArguments { walker := astvisitor.NewWalker(48) visitor := &fieldArgumentsVisitor{ walker: &walker, - variables: vars, - remapVariables: remapVariables, - fieldArguments: make(map[string]map[string]any), + variables: opts.vars, + remapVariables: opts.remapVariables, + fieldArguments: make(map[string]map[string]*astjson.Value), } walker.RegisterEnterDocumentVisitor(visitor) walker.RegisterEnterArgumentVisitor(visitor) report := &operationreport.Report{} - walker.Walk(operation, definition, report) - - return visitor.fieldArguments -} + walker.Walk(opts.operation, opts.definition, report) -// getArgValue returns the actual value of val. -// It resolves variables in case these have been used for arguments, -// else it will use values from doc. -func getArgValue(doc *ast.Document, val ast.Value, variables *astjson.Value, remapVariables map[string]string) any { - if val.Kind != ast.ValueKindVariable { - // TODO delete this comment. - // I observed we never actually hit this code path because the operation parser - // automically creates variables and maps them to arguments, even if no initial variables are provided. - // We should probably still have this in place just in case. - // Maybe there is a better way than to to use ValueToJSON but I haven't found one yet. - actualValue, err := doc.ValueToJSON(val) - if err != nil { - // TODO error handling - return nil - } - // TODO: Type cast to what this actually is, it returns only []byte atm - return actualValue + res := FieldArguments{ + data: visitor.fieldArguments, } - varName := doc.VariableValueNameString(val.Ref) - originalVarName := varName - if remapVariables != nil { - if original, ok := remapVariables[varName]; ok { - originalVarName = original - } - } - - varValue := variables.Get(originalVarName) - switch varValue.Type() { - case astjson.TypeNumber: - return varValue.GetInt() - case astjson.TypeString: - return string(varValue.GetStringBytes()) - case astjson.TypeObject: - // TODO maybe create map out of varValue to give hook developers a better experience. - // The problem is this would be a nested operation because objects can contain all kinds - // of children elements, such as numbers, strings, other objects, etc. - // Right now we only return the astjson type directly and leave it to the hook developer - // to work with that. - return varValue.GetObject() - case astjson.TypeArray: - // TODO maybe create slice out of varValue to give hook developers a better experience. - // The problem is this would be a nested operation because arrays can contain all kinds - // of children elements, such as numbers, strings, other objects, etc. - // Right now we only return the astjson type directly and leave it to the hook developer - // to work with that. - return varValue.GetArray() - case astjson.TypeFalse: - // TODO hypothetically written, needs testing - return false - case astjson.TypeTrue: - // TODO hypothetically written, needs testing - return true - default: - // TODO test for type = null - return nil - } + return res } diff --git a/router/core/field_arguments.go b/router/core/field_arguments.go new file mode 100644 index 0000000000..f97a7ccce7 --- /dev/null +++ b/router/core/field_arguments.go @@ -0,0 +1,31 @@ +package core + +import "github.com/wundergraph/astjson" + +// FieldArguments allow access to GraphQL field arguments used by clients. +type FieldArguments struct { + data map[string]map[string]*astjson.Value +} + +// Get will return the value of argument a from field f. +// +// The field needs to be a dot notated path to the position of the field. +// For example if you want to access arg1 on field2 on the operation +// +// subscription { +// mySub(arg1: "val1", arg2: "val2") { +// field1 +// field2(arg1: "val3", arg2: "val4") +// } +// +// You need to call Get("mySub.field2", "arg1") . +// If f or a cannot be found nil is returned. +func (fa *FieldArguments) Get(f string, a string) *astjson.Value { + args, found := fa.data[f] + if !found { + return nil + } + + v, _ := args[a] + return v +} diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index fab50ce630..5d4e0252b2 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -913,12 +913,13 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v requestContext.operation.variables, err = variablesParser.ParseBytes(operationKit.parsedOperation.Request.Variables) if h.mapFieldArguments { - requestContext.operation.fieldArguments = mapFieldArguments( - operationKit.kit.doc, - h.executor.ClientSchema, - requestContext.operation.variables, - requestContext.operation.remapVariables, - ) + requestContext.operation.fieldArguments = mapFieldArguments(mapFieldArgumentsOpts{ + operation: operationKit.kit.doc, + definition: h.executor.ClientSchema, + vars: requestContext.operation.variables, + remapVariables: requestContext.operation.remapVariables, + logger: h.log, + }) } if err != nil { diff --git a/router/core/websocket.go b/router/core/websocket.go index 2c91631fe5..8efc554127 100644 --- a/router/core/websocket.go +++ b/router/core/websocket.go @@ -935,12 +935,13 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi } if h.mapFieldArguments { - opContext.fieldArguments = mapFieldArguments( - operationKit.kit.doc, - h.operationProcessor.executor.ClientSchema, - opContext.variables, - opContext.remapVariables, - ) + opContext.fieldArguments = mapFieldArguments(mapFieldArgumentsOpts{ + operation: operationKit.kit.doc, + definition: h.operationProcessor.executor.ClientSchema, + vars: opContext.variables, + remapVariables: opContext.remapVariables, + logger: h.logger, + }) } startValidation := time.Now() From 8477e1a523099fbe32c7326539011fb22dc783d5 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:31:36 +0100 Subject: [PATCH 05/22] chore: rename FieldArguments() to Arguments() --- router/core/context.go | 8 ++++---- router/core/field_argument_visitor.go | 4 ++-- router/core/field_arguments.go | 16 +++++++++++----- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/router/core/context.go b/router/core/context.go index de0a40ad7f..fe667b8a76 100644 --- a/router/core/context.go +++ b/router/core/context.go @@ -482,8 +482,8 @@ type OperationContext interface { Hash() uint64 // Content is the content of the operation Content() string - // FieldArguments allow access to a GraphQL operation's field arguments. - FieldArguments() *FieldArguments + // Arguments allow access to a GraphQL operation's field arguments. + Arguments() *Arguments // Variables allow access to GraphQL operation variables used by clients. Variables() *astjson.Value // ClientInfo returns information about the client that initiated this operation @@ -525,7 +525,7 @@ type operationContext struct { rawContent string // Content is the normalized content of the operation content string - fieldArguments FieldArguments + fieldArguments Arguments variables *astjson.Value files []*httpclient.FileUpload clientInfo *ClientInfo @@ -559,7 +559,7 @@ func (o *operationContext) Variables() *astjson.Value { return o.variables } -func (o *operationContext) FieldArguments() *FieldArguments { +func (o *operationContext) Arguments() *Arguments { return &o.fieldArguments } diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go index 6412f079ed..d91cd0ef8f 100644 --- a/router/core/field_argument_visitor.go +++ b/router/core/field_argument_visitor.go @@ -111,7 +111,7 @@ type mapFieldArgumentsOpts struct { logger *zap.Logger } -func mapFieldArguments(opts mapFieldArgumentsOpts) FieldArguments { +func mapFieldArguments(opts mapFieldArgumentsOpts) Arguments { walker := astvisitor.NewWalker(48) visitor := &fieldArgumentsVisitor{ @@ -127,7 +127,7 @@ func mapFieldArguments(opts mapFieldArgumentsOpts) FieldArguments { report := &operationreport.Report{} walker.Walk(opts.operation, opts.definition, report) - res := FieldArguments{ + res := Arguments{ data: visitor.fieldArguments, } diff --git a/router/core/field_arguments.go b/router/core/field_arguments.go index f97a7ccce7..d3d7a05335 100644 --- a/router/core/field_arguments.go +++ b/router/core/field_arguments.go @@ -2,15 +2,20 @@ package core import "github.com/wundergraph/astjson" -// FieldArguments allow access to GraphQL field arguments used by clients. -type FieldArguments struct { +// Arguments allow access to GraphQL field arguments used by clients. +type Arguments struct { data map[string]map[string]*astjson.Value } // Get will return the value of argument a from field f. // -// The field needs to be a dot notated path to the position of the field. -// For example if you want to access arg1 on field2 on the operation +// To access the an argument of a root level field, you need to pass the +// name of the field as the first argument to Get and the name of the argument +// as the second argument, e.g. Get("rootfield_name", "argument_name") . +// +// The field needs to be a dot notated path to the position of the field, +// if it is nested. +// For example you can access arg1 on field2 on the operation // // subscription { // mySub(arg1: "val1", arg2: "val2") { @@ -19,8 +24,9 @@ type FieldArguments struct { // } // // You need to call Get("mySub.field2", "arg1") . +// // If f or a cannot be found nil is returned. -func (fa *FieldArguments) Get(f string, a string) *astjson.Value { +func (fa *Arguments) Get(f string, a string) *astjson.Value { args, found := fa.data[f] if !found { return nil From c5d995fbb0ae550504abb50bc98f08f1e95eac3f Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:31:43 +0100 Subject: [PATCH 06/22] chore: add unit tests --- router/core/field_argument_visitor_test.go | 290 +++++++++++++++++++++ 1 file changed, 290 insertions(+) create mode 100644 router/core/field_argument_visitor_test.go diff --git a/router/core/field_argument_visitor_test.go b/router/core/field_argument_visitor_test.go new file mode 100644 index 0000000000..7653b8c1c5 --- /dev/null +++ b/router/core/field_argument_visitor_test.go @@ -0,0 +1,290 @@ +package core + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wundergraph/astjson" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" + "github.com/wundergraph/graphql-go-tools/v2/pkg/asttransform" + "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" +) + +func TestMapFieldArguments(t *testing.T) { + testCases := []struct { + name string + schema string + operation string + variables string + assertions func(t *testing.T, result Arguments) + }{ + { + name: "root field arguments with variables are accessible", + schema: ` + type Query { + user(id: ID!): User + } + type User { + id: ID! + name: String! + } + `, + operation: ` + query GetUser($userId: ID!) { + user(id: $userId) { + id + name + } + } + `, + variables: `{"userId": "123"}`, + assertions: func(t *testing.T, result Arguments) { + idArg := result.Get("user", "id") + require.NotNil(t, idArg, "expected 'id' argument on 'user' field") + assert.Equal(t, "123", string(idArg.GetStringBytes())) + }, + }, + { + name: "root field arguments without variables are accessible", + schema: ` + type Query { + user(id: ID!): User + } + type User { + id: ID! + name: String! + } + `, + operation: ` + query GetUser { + user(id: "123") { + id + name + } + } + `, + variables: `{}`, + assertions: func(t *testing.T, result Arguments) { + idArg := result.Get("user", "id") + require.NotNil(t, idArg, "expected 'id' argument on 'user' field") + assert.Equal(t, "123", string(idArg.GetStringBytes())) + }, + }, + { + name: "nested field arguments are accessible", + schema: ` + type Query { + user(id: ID!): User + } + type User { + id: ID! + posts(limit: Int!, offset: Int): [Post!]! + } + type Post { + id: ID! + title: String! + } + `, + operation: ` + query GetUserPosts($userId: ID!, $limit: Int!, $offset: Int) { + user(id: $userId) { + id + posts(limit: $limit, offset: $offset) { + id + title + } + } + } + `, + variables: `{"userId": "user-1", "limit": 10, "offset": 5}`, + assertions: func(t *testing.T, result Arguments) { + // Assert root field argument + userIdArg := result.Get("user", "id") + require.NotNil(t, userIdArg) + assert.Equal(t, "user-1", string(userIdArg.GetStringBytes())) + + // Assert nested field arguments (dot notation path) + limitArg := result.Get("user.posts", "limit") + require.NotNil(t, limitArg, "expected 'limit' argument on 'user.posts' field") + assert.Equal(t, 10, limitArg.GetInt()) + + offsetArg := result.Get("user.posts", "offset") + require.NotNil(t, offsetArg, "expected 'offset' argument on 'user.posts' field") + assert.Equal(t, 5, offsetArg.GetInt()) + }, + }, + { + name: "non-existent field returns nil", + schema: ` + type Query { + hello: String + } + `, + operation: ` + query { + hello + } + `, + variables: `{}`, + assertions: func(t *testing.T, result Arguments) { + arg := result.Get("hello", "someArg") + require.Nil(t, arg, "expected nil for non-existent argument") + + arg = result.Get("nonExistent", "arg") + require.Nil(t, arg, "expected nil for non-existent field") + }, + }, + { + name: "multiple root fields with arguments", + schema: ` + type Query { + user(id: ID!): User + post(slug: String!): Post + } + type User { + id: ID! + } + type Post { + slug: String! + } + `, + operation: ` + query GetUserAndPost($userId: ID!, $postSlug: String!) { + user(id: $userId) { + id + } + post(slug: $postSlug) { + slug + } + } + `, + variables: `{"userId": "user-123", "postSlug": "my-post"}`, + assertions: func(t *testing.T, result Arguments) { + userIdArg := result.Get("user", "id") + require.NotNil(t, userIdArg) + assert.Equal(t, "user-123", string(userIdArg.GetStringBytes())) + + postSlugArg := result.Get("post", "slug") + require.NotNil(t, postSlugArg) + assert.Equal(t, "my-post", string(postSlugArg.GetStringBytes())) + }, + }, + { + name: "array argument is accessible", + schema: ` + type Query { + users(ids: [ID!]!): [User!]! + } + type User { + id: ID! + name: String! + } + `, + operation: ` + query GetUsers($userIds: [ID!]!) { + users(ids: $userIds) { + id + name + } + } + `, + variables: `{"userIds": ["user-1", "user-2", "user-3"]}`, + assertions: func(t *testing.T, result Arguments) { + idsArg := result.Get("users", "ids") + require.NotNil(t, idsArg, "expected 'ids' argument on 'users' field") + + // Verify it's an array + arr := idsArg.GetArray() + require.Len(t, arr, 3) + assert.Equal(t, "user-1", string(arr[0].GetStringBytes())) + assert.Equal(t, "user-2", string(arr[1].GetStringBytes())) + assert.Equal(t, "user-3", string(arr[2].GetStringBytes())) + }, + }, + { + name: "object argument is accessible", + schema: ` + type Query { + users(filter: UserFilter!): [User!]! + } + input UserFilter { + name: String + age: Int + active: Boolean! + } + type User { + id: ID! + name: String! + } + `, + operation: ` + query GetUsers($filter: UserFilter!) { + users(filter: $filter) { + id + name + } + } + `, + variables: `{"filter": {"name": "John", "age": 30, "active": true}}`, + assertions: func(t *testing.T, result Arguments) { + filterArg := result.Get("users", "filter") + require.NotNil(t, filterArg, "expected 'filter' argument on 'users' field") + + // Verify it's an object and access its fields + obj := filterArg.GetObject() + require.NotNil(t, obj) + + nameVal := filterArg.Get("name") + require.NotNil(t, nameVal) + assert.Equal(t, "John", string(nameVal.GetStringBytes())) + + ageVal := filterArg.Get("age") + require.NotNil(t, ageVal) + assert.Equal(t, 30, ageVal.GetInt()) + + activeVal := filterArg.Get("active") + require.NotNil(t, activeVal) + assert.True(t, activeVal.GetBool()) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Parse schema + schema, report := astparser.ParseGraphqlDocumentString(tc.schema) + require.False(t, report.HasErrors(), "failed to parse schema") + err := asttransform.MergeDefinitionWithBaseSchema(&schema) + require.NoError(t, err) + + // Parse operation + operation, report := astparser.ParseGraphqlDocumentString(tc.operation) + require.False(t, report.HasErrors(), "failed to parse operation") + + // Set variables before normalization (like the router does) + operation.Input.Variables = []byte(tc.variables) + + // Normalize operation (merges provided variables with extracted inline literals) + rep := &operationreport.Report{} + norm := astnormalization.NewNormalizer(true, true) + norm.NormalizeOperation(&operation, &schema, rep) + require.False(t, rep.HasErrors(), "failed to normalize operation") + + // Use normalized variables (includes both provided and extracted variables) + vars, err := astjson.ParseBytes(operation.Input.Variables) + require.NoError(t, err) + + // Call mapFieldArguments + result := mapFieldArguments(mapFieldArgumentsOpts{ + operation: &operation, + definition: &schema, + vars: vars, + }) + + // Run assertions + tc.assertions(t, result) + }) + } +} From 4800a8c78c00d2b4d5fe354bf7a4d09a36f7db9f Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:43:32 +0100 Subject: [PATCH 07/22] chore: improve godoc --- router/core/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/router/core/context.go b/router/core/context.go index fe667b8a76..3caa9c6a13 100644 --- a/router/core/context.go +++ b/router/core/context.go @@ -482,9 +482,9 @@ type OperationContext interface { Hash() uint64 // Content is the content of the operation Content() string - // Arguments allow access to a GraphQL operation's field arguments. + // Arguments allow access to GraphQL operation field arguments. Arguments() *Arguments - // Variables allow access to GraphQL operation variables used by clients. + // Variables allow access to GraphQL operation variables. Variables() *astjson.Value // ClientInfo returns information about the client that initiated this operation ClientInfo() ClientInfo From 130bd22200fe4d28829047d31dcb8d6e1b8c626c Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:43:49 +0100 Subject: [PATCH 08/22] chore: add router tests --- .../modules/start_subscription_test.go | 72 +++++++++++++++ router-tests/modules/stream_publish_test.go | 48 ++++++++++ router-tests/modules/stream_receive_test.go | 87 +++++++++++++++++++ 3 files changed, 207 insertions(+) diff --git a/router-tests/modules/start_subscription_test.go b/router-tests/modules/start_subscription_test.go index b517c287f1..0181e72da3 100644 --- a/router-tests/modules/start_subscription_test.go +++ b/router-tests/modules/start_subscription_test.go @@ -711,4 +711,76 @@ func TestStartSubscriptionHook(t *testing.T) { assert.Equal(t, int32(1), customModule.HookCallCount.Load()) }) }) + + t.Run("Test StartSubscription hook can access field arguments", func(t *testing.T) { + t.Parallel() + + // This test verifies that the subscription start hook can access GraphQL field arguments + // via ctx.Operation().Arguments(). + + var capturedEmployeeID int + + customModule := &start_subscription.StartSubscriptionModule{ + HookCallCount: &atomic.Int32{}, + Callback: func(ctx core.SubscriptionOnStartHandlerContext) error { + args := ctx.Operation().Arguments() + if args != nil { + employeeIDArg := args.Get("employeeUpdatedMyKafka", "employeeID") + if employeeIDArg != nil { + capturedEmployeeID = employeeIDArg.GetInt() + } + } + return nil + }, + } + + cfg := config.Config{ + Graph: config.Graph{}, + Modules: map[string]interface{}{ + "startSubscriptionModule": customModule, + }, + } + + testenv.Run(t, &testenv.Config{ + RouterConfigJSONTemplate: testenv.ConfigWithEdfsKafkaJSONTemplate, + EnableKafka: true, + RouterOptions: []core.Option{ + core.WithModulesConfig(cfg.Modules), + core.WithCustomModules(&start_subscription.StartSubscriptionModule{}), + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + var subscriptionOne struct { + employeeUpdatedMyKafka struct { + ID float64 `graphql:"id"` + } `graphql:"employeeUpdatedMyKafka(employeeID: $employeeID)"` + } + + surl := xEnv.GraphQLWebSocketSubscriptionURL() + client := graphql.NewSubscriptionClient(surl) + + vars := map[string]interface{}{ + "employeeID": 7, + } + subscriptionOneID, err := client.Subscribe(&subscriptionOne, vars, func(dataValue []byte, errValue error) error { + return nil + }) + require.NoError(t, err) + require.NotEmpty(t, subscriptionOneID) + + clientRunCh := make(chan error) + go func() { + clientRunCh <- client.Run() + }() + + xEnv.WaitForSubscriptionCount(1, time.Second*10) + + require.NoError(t, client.Close()) + testenv.AwaitChannelWithT(t, time.Second*10, clientRunCh, func(t *testing.T, err error) { + require.NoError(t, err) + }, "unable to close client before timeout") + + assert.Equal(t, int32(1), customModule.HookCallCount.Load()) + assert.Equal(t, 7, capturedEmployeeID, "expected to capture employeeID argument value") + }) + }) } diff --git a/router-tests/modules/stream_publish_test.go b/router-tests/modules/stream_publish_test.go index 35d9e16766..564596e88e 100644 --- a/router-tests/modules/stream_publish_test.go +++ b/router-tests/modules/stream_publish_test.go @@ -358,4 +358,52 @@ func TestPublishHook(t *testing.T) { require.Equal(t, []byte("3"), header.Value) }) }) + + t.Run("Test Publish hook can access field arguments", func(t *testing.T) { + t.Parallel() + + // This test verifies that the publish hook can access GraphQL field arguments + // via ctx.Operation().Arguments(). + + var capturedEmployeeID int + + customModule := stream_publish.PublishModule{ + HookCallCount: &atomic.Int32{}, + Callback: func(ctx core.StreamPublishEventHandlerContext, events datasource.StreamEvents) (datasource.StreamEvents, error) { + args := ctx.Operation().Arguments() + if args != nil { + employeeIDArg := args.Get("updateEmployeeMyKafka", "employeeID") + if employeeIDArg != nil { + capturedEmployeeID = employeeIDArg.GetInt() + } + } + return events, nil + }, + } + + cfg := config.Config{ + Graph: config.Graph{}, + Modules: map[string]interface{}{ + "publishModule": customModule, + }, + } + + testenv.Run(t, &testenv.Config{ + RouterConfigJSONTemplate: testenv.ConfigWithEdfsKafkaJSONTemplate, + EnableKafka: true, + RouterOptions: []core.Option{ + core.WithModulesConfig(cfg.Modules), + core.WithCustomModules(&stream_publish.PublishModule{}), + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + events.KafkaEnsureTopicExists(t, xEnv, time.Second, "employeeUpdated") + resOne := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ + Query: `mutation { updateEmployeeMyKafka(employeeID: 5, update: {name: "test"}) { success } }`, + }) + require.JSONEq(t, `{"data":{"updateEmployeeMyKafka":{"success":true}}}`, resOne.Body) + + assert.Equal(t, int32(1), customModule.HookCallCount.Load()) + assert.Equal(t, 5, capturedEmployeeID, "expected to capture employeeID argument value") + }) + }) } diff --git a/router-tests/modules/stream_receive_test.go b/router-tests/modules/stream_receive_test.go index c71d3019bb..595c548b60 100644 --- a/router-tests/modules/stream_receive_test.go +++ b/router-tests/modules/stream_receive_test.go @@ -963,4 +963,91 @@ func TestReceiveHook(t *testing.T) { assert.Equal(t, int32(3), customModule.HookCallCount.Load()) }) }) + + t.Run("Test Receive hook can access field arguments", func(t *testing.T) { + t.Parallel() + + // This test verifies that the receive hook can access GraphQL field arguments + // via ctx.Operation().Arguments(). + + var capturedEmployeeID int + + customModule := stream_receive.StreamReceiveModule{ + HookCallCount: &atomic.Int32{}, + Callback: func(ctx core.StreamReceiveEventHandlerContext, events datasource.StreamEvents) (datasource.StreamEvents, error) { + args := ctx.Operation().Arguments() + if args != nil { + employeeIDArg := args.Get("employeeUpdatedMyKafka", "employeeID") + if employeeIDArg != nil { + capturedEmployeeID = employeeIDArg.GetInt() + } + } + return events, nil + }, + } + + cfg := config.Config{ + Graph: config.Graph{}, + Modules: map[string]interface{}{ + "streamReceiveModule": customModule, + }, + } + + testenv.Run(t, &testenv.Config{ + RouterConfigJSONTemplate: testenv.ConfigWithEdfsKafkaJSONTemplate, + EnableKafka: true, + RouterOptions: []core.Option{ + core.WithModulesConfig(cfg.Modules), + core.WithCustomModules(&stream_receive.StreamReceiveModule{}), + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + topics := []string{"employeeUpdated"} + events.KafkaEnsureTopicExists(t, xEnv, time.Second, topics...) + + var subscriptionOne struct { + employeeUpdatedMyKafka struct { + ID float64 `graphql:"id"` + } `graphql:"employeeUpdatedMyKafka(employeeID: 3)"` + } + + surl := xEnv.GraphQLWebSocketSubscriptionURL() + client := graphql.NewSubscriptionClient(surl) + + type kafkaSubscriptionArgs struct { + dataValue []byte + errValue error + } + subscriptionArgsCh := make(chan kafkaSubscriptionArgs) + subscriptionOneID, err := client.Subscribe(&subscriptionOne, nil, func(dataValue []byte, errValue error) error { + subscriptionArgsCh <- kafkaSubscriptionArgs{ + dataValue: dataValue, + errValue: errValue, + } + return nil + }) + require.NoError(t, err) + require.NotEmpty(t, subscriptionOneID) + + clientRunCh := make(chan error) + go func() { + clientRunCh <- client.Run() + }() + + xEnv.WaitForSubscriptionCount(1, Timeout) + + events.ProduceKafkaMessage(t, xEnv, Timeout, topics[0], `{"__typename":"Employee","id": 1,"update":{"name":"foo"}}`) + + testenv.AwaitChannelWithT(t, Timeout, subscriptionArgsCh, func(t *testing.T, args kafkaSubscriptionArgs) { + require.NoError(t, args.errValue) + }) + + require.NoError(t, client.Close()) + testenv.AwaitChannelWithT(t, Timeout, clientRunCh, func(t *testing.T, err error) { + require.NoError(t, err) + }, "unable to close client before timeout") + + assert.Equal(t, int32(1), customModule.HookCallCount.Load()) + assert.Equal(t, 3, capturedEmployeeID, "expected to capture employeeID argument value") + }) + }) } From 9f0c25f29c22a52f63ebc253185674812116b7da Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:54:31 +0100 Subject: [PATCH 09/22] chore: simplify Get method --- router/core/field_arguments.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/router/core/field_arguments.go b/router/core/field_arguments.go index d3d7a05335..eb3de43e66 100644 --- a/router/core/field_arguments.go +++ b/router/core/field_arguments.go @@ -27,11 +27,14 @@ type Arguments struct { // // If f or a cannot be found nil is returned. func (fa *Arguments) Get(f string, a string) *astjson.Value { + if fa == nil { + return nil + } + args, found := fa.data[f] if !found { return nil } - v, _ := args[a] - return v + return args[a] } From b28d9a7a668a16fa87502af179b6284832d868fc Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 12 Dec 2025 09:55:57 +0100 Subject: [PATCH 10/22] chore: remove garbage, add comments --- .../core/{field_arguments.go => arguments.go} | 3 ++ router/core/operation_planner.go | 40 ------------------- 2 files changed, 3 insertions(+), 40 deletions(-) rename router/core/{field_arguments.go => arguments.go} (86%) diff --git a/router/core/field_arguments.go b/router/core/arguments.go similarity index 86% rename from router/core/field_arguments.go rename to router/core/arguments.go index eb3de43e66..41e15a50c5 100644 --- a/router/core/field_arguments.go +++ b/router/core/arguments.go @@ -4,6 +4,9 @@ import "github.com/wundergraph/astjson" // Arguments allow access to GraphQL field arguments used by clients. type Arguments struct { + // First key is the path to the field in dot notation + // i.e. root_field.subfield1.subfield2. + // Second argument is the name of the argument of that field. data map[string]map[string]*astjson.Value } diff --git a/router/core/operation_planner.go b/router/core/operation_planner.go index d5b6b2ac66..38c3b6aac5 100644 --- a/router/core/operation_planner.go +++ b/router/core/operation_planner.go @@ -92,46 +92,6 @@ func (p *OperationPlanner) preparePlan(ctx *operationContext) (*planWithMetaData return out, nil } -// // mapFieldArguments returns all field arguments inside doc as a map. -// // -// // The key of the map is a dot-notated path, where each element refers -// // to a field. The inner map holds the argument names as keys. -// // map["rootfield1.subfield1"]["arg1"] -// // map["rootfield1.subfield1"]["arg2"] -// // map["rootfield1.subfield2"]["arg1"] -// // map["rootfield1.subfield2"]["arg2"] -// // -// // The value of the inner map is the literal value of the argument. -// // Variables will be resolved in case they have been used for arguments. -// func mapFieldArguments(doc *ast.Document, vars *astjson.Value, remapVariables map[string]string) map[string]map[string]any { -// // TODO: Currently we only map root field arguments. I.e. -// // map["rootfield1"]["arg1"] -// // map["rootfield2"]["arg1"] -// // Needs to extended to support subfields, like -// // map["rootfield1.subfield1"]["arg1"] -// selectionSet := doc.OperationDefinitions[0].SelectionSet -// fields := doc.SelectionSetFieldSelections(selectionSet) - -// args := make(map[string]map[string]any, len(fields)) - -// for _, field := range fields { -// fieldRef := doc.Selections[field].Ref -// fieldName := doc.FieldNameString(fieldRef) -// fieldArgs := doc.FieldArguments(fieldRef) - -// for _, fieldArg := range fieldArgs { -// m := make(map[string]any, len(fieldArgs)) -// args[fieldName] = m - -// argName := doc.ArgumentNameString(fieldArg) -// val := doc.Arguments[fieldArg].Value -// args[fieldName][argName] = getArgValue(doc, val, vars, remapVariables) -// } -// } - -// return args -// } - type PlanOptions struct { ClientInfo *ClientInfo TraceOptions resolve.TraceOptions From a63ff06f1c96a5a24000818df072d5028e254478 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 12 Dec 2025 10:42:51 +0100 Subject: [PATCH 11/22] fix: log a warning when value cant resolve --- router/core/field_argument_visitor.go | 6 +- router/core/field_argument_visitor_test.go | 69 ++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go index d91cd0ef8f..520c3f9126 100644 --- a/router/core/field_argument_visitor.go +++ b/router/core/field_argument_visitor.go @@ -18,6 +18,7 @@ type fieldArgumentsVisitor struct { variables *astjson.Value remapVariables map[string]string fieldArguments map[string]map[string]*astjson.Value + logger *zap.Logger } func (v *fieldArgumentsVisitor) EnterDocument(operation, definition *ast.Document) { @@ -43,7 +44,9 @@ func (v *fieldArgumentsVisitor) EnterArgument(ref int) { resolvedArgVal, err := v.resolveArgValue(argVal) if err != nil { - // TODO: Log error somehow + v.logger. + With(zap.String("fieldPath", fieldPath), zap.String("argName", argName)). + Warn("failed to resolve argument value", zap.Error(err)) return } @@ -119,6 +122,7 @@ func mapFieldArguments(opts mapFieldArgumentsOpts) Arguments { variables: opts.vars, remapVariables: opts.remapVariables, fieldArguments: make(map[string]map[string]*astjson.Value), + logger: opts.logger, } walker.RegisterEnterDocumentVisitor(visitor) diff --git a/router/core/field_argument_visitor_test.go b/router/core/field_argument_visitor_test.go index 7653b8c1c5..3cdf177f8f 100644 --- a/router/core/field_argument_visitor_test.go +++ b/router/core/field_argument_visitor_test.go @@ -6,10 +6,14 @@ 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" "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" "github.com/wundergraph/graphql-go-tools/v2/pkg/asttransform" "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" ) func TestMapFieldArguments(t *testing.T) { @@ -288,3 +292,68 @@ func TestMapFieldArguments(t *testing.T) { }) } } + +func TestMapFieldArguments_InvalidValueLogsWarning(t *testing.T) { + // This test verifies that when resolveArgValue fails (e.g., due to an invalid AST value), + // a warning is logged and the argument is skipped (not added to the result). + + schema := ` + type Query { + user(id: ID!): User + } + type User { + id: ID! + } + ` + + operation := ` + query { + user(id: "123") { + id + } + } + ` + + // Parse schema + schemaDef, report := astparser.ParseGraphqlDocumentString(schema) + require.False(t, report.HasErrors(), "failed to parse schema") + err := asttransform.MergeDefinitionWithBaseSchema(&schemaDef) + require.NoError(t, err) + + // Parse operation WITHOUT normalization to keep the inline literal + op, report := astparser.ParseGraphqlDocumentString(operation) + require.False(t, report.HasErrors(), "failed to parse operation") + + // Corrupt the AST: set an invalid value kind that will cause ValueToJSON to fail + // Find the argument and set its value to an invalid kind + for i := range op.Arguments { + // Set to an invalid/unhandled value kind + op.Arguments[i].Value.Kind = ast.ValueKind(255) // Invalid kind + } + + // Set up observed logger to capture warnings + observedCore, observedLogs := observer.New(zapcore.WarnLevel) + logger := zap.New(observedCore) + + // Parse empty variables + vars, err := astjson.ParseBytes([]byte(`{}`)) + require.NoError(t, err) + + // Call mapFieldArguments - should log a warning due to invalid value + result := mapFieldArguments(mapFieldArgumentsOpts{ + operation: &op, + definition: &schemaDef, + vars: vars, + logger: logger, + }) + + // Verify warning was logged + logs := observedLogs.All() + require.Len(t, logs, 1, "expected exactly one warning log") + assert.Equal(t, "failed to resolve argument value", logs[0].Message) + assert.Equal(t, zapcore.WarnLevel, logs[0].Level) + + // Verify the argument was skipped (not added to result) + idArg := result.Get("user", "id") + assert.Nil(t, idArg, "expected argument to be skipped due to error") +} From d6fcf639a191717a2a0b378c90224f525f6711d4 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 12 Dec 2025 11:26:25 +0100 Subject: [PATCH 12/22] chore: add test verifying direct value resolving This test verifies that values of arguments can be extracted directly from the AST when no variable remapping has taken place. Usually this is not the case in reality but if for whatever reason it will be reality one day, this method is proven to work with that. --- router/core/field_argument_visitor_test.go | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/router/core/field_argument_visitor_test.go b/router/core/field_argument_visitor_test.go index 3cdf177f8f..652d97436f 100644 --- a/router/core/field_argument_visitor_test.go +++ b/router/core/field_argument_visitor_test.go @@ -357,3 +357,58 @@ func TestMapFieldArguments_InvalidValueLogsWarning(t *testing.T) { idArg := result.Get("user", "id") assert.Nil(t, idArg, "expected argument to be skipped due to error") } + +func TestMapFieldArguments_InlineLiteralWithoutNormalization(t *testing.T) { + // This test verifies that inline literal arguments are correctly extracted + // via getArgValueFromDoc when normalization is skipped. + // Normally, normalization converts inline literals to variables, but this test + // exercises the code path for non-variable argument values. + + schema := ` + type Query { + user(id: ID!, active: Boolean!): User + } + type User { + id: ID! + } + ` + + operation := ` + query { + user(id: "inline-123", active: true) { + id + } + } + ` + + // Parse schema + schemaDef, report := astparser.ParseGraphqlDocumentString(schema) + require.False(t, report.HasErrors(), "failed to parse schema") + err := asttransform.MergeDefinitionWithBaseSchema(&schemaDef) + require.NoError(t, err) + + // Parse operation WITHOUT normalization to keep inline literals as non-variable values + op, report := astparser.ParseGraphqlDocumentString(operation) + require.False(t, report.HasErrors(), "failed to parse operation") + + // Parse empty variables (inline values are in the AST, not in variables) + vars, err := astjson.ParseBytes([]byte(`{}`)) + require.NoError(t, err) + + // Call mapFieldArguments - should extract values via getArgValueFromDoc + result := mapFieldArguments(mapFieldArgumentsOpts{ + operation: &op, + definition: &schemaDef, + vars: vars, + }) + + // Verify the string argument was correctly extracted + idArg := result.Get("user", "id") + require.NotNil(t, idArg, "expected 'id' argument to be accessible") + assert.Equal(t, "inline-123", string(idArg.GetStringBytes())) + + // Verify the boolean argument was correctly extracted + activeArg := result.Get("user", "active") + require.NotNil(t, activeArg, "expected 'active' argument to be accessible") + assert.True(t, activeArg.GetBool()) +} From cdca088efc4a66451fe8ae7a03e16fa23015ad49 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 12 Dec 2025 11:39:59 +0100 Subject: [PATCH 13/22] chore: nil checks --- router/core/field_argument_visitor.go | 16 +++++++++++++++- router/core/graphql_prehandler.go | 17 +++++++++-------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go index 520c3f9126..fc7cad8080 100644 --- a/router/core/field_argument_visitor.go +++ b/router/core/field_argument_visitor.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/pkg/errors" "github.com/wundergraph/astjson" "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" @@ -27,6 +28,10 @@ func (v *fieldArgumentsVisitor) EnterDocument(operation, definition *ast.Documen } func (v *fieldArgumentsVisitor) EnterArgument(ref int) { + if len(v.walker.Ancestors) == 0 { + return + } + // skip if we don't deal with field arguments (e.g. directive arguments) anc := v.walker.Ancestors[len(v.walker.Ancestors)-1] if anc.Kind != ast.NodeKindField { @@ -95,6 +100,10 @@ func (v *fieldArgumentsVisitor) getArgValueFromDoc(val ast.Value) (*astjson.Valu } func (v *fieldArgumentsVisitor) getArgValueFromVars(val ast.Value) (*astjson.Value, error) { + if v.variables == nil { + return nil, errors.New("value kind is of type 'variable' but no variables are set") + } + varName := v.operation.VariableValueNameString(val.Ref) originalVarName := varName if v.remapVariables != nil { @@ -117,12 +126,17 @@ type mapFieldArgumentsOpts struct { func mapFieldArguments(opts mapFieldArgumentsOpts) Arguments { walker := astvisitor.NewWalker(48) + logger := opts.logger + if logger != nil { + logger = zap.NewNop() + } + visitor := &fieldArgumentsVisitor{ walker: &walker, variables: opts.vars, remapVariables: opts.remapVariables, fieldArguments: make(map[string]map[string]*astjson.Value), - logger: opts.logger, + logger: logger, } walker.RegisterEnterDocumentVisitor(visitor) diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index fd8ae89097..ba93dffce7 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -911,6 +911,15 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v requestContext.operation.content = operationKit.parsedOperation.NormalizedRepresentation requestContext.operation.variables, err = variablesParser.ParseBytes(operationKit.parsedOperation.Request.Variables) + if err != nil { + rtrace.AttachErrToSpan(engineNormalizeSpan, err) + if !requestContext.operation.traceOptions.ExcludeNormalizeStats { + httpOperation.traceTimings.EndNormalize() + } + engineNormalizeSpan.End() + return err + } + if h.mapFieldArguments { requestContext.operation.fieldArguments = mapFieldArguments(mapFieldArgumentsOpts{ operation: operationKit.kit.doc, @@ -921,14 +930,6 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v }) } - if err != nil { - rtrace.AttachErrToSpan(engineNormalizeSpan, err) - if !requestContext.operation.traceOptions.ExcludeNormalizeStats { - httpOperation.traceTimings.EndNormalize() - } - engineNormalizeSpan.End() - return err - } requestContext.operation.normalizationTime = time.Since(startNormalization) requestContext.expressionContext.Request.Operation.NormalizationTime = requestContext.operation.normalizationTime setTelemetryAttributes(normalizeCtx, requestContext, expr.BucketNormalizationTime) From b9cbf97a8ee6bc6d1f26042cd853e6c9c180d18f Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 12 Dec 2025 11:54:40 +0100 Subject: [PATCH 14/22] fix: support aliased fields --- router/core/arguments.go | 21 +++++++++--- router/core/field_argument_visitor.go | 18 ++++++++-- router/core/field_argument_visitor_test.go | 39 ++++++++++++++++++++++ 3 files changed, 70 insertions(+), 8 deletions(-) diff --git a/router/core/arguments.go b/router/core/arguments.go index 41e15a50c5..e145474829 100644 --- a/router/core/arguments.go +++ b/router/core/arguments.go @@ -12,12 +12,14 @@ type Arguments struct { // Get will return the value of argument a from field f. // -// To access the an argument of a root level field, you need to pass the -// name of the field as the first argument to Get and the name of the argument +// To access an argument of a root level field, you need to pass the +// response key of the field as the first argument to Get and the name of the argument // as the second argument, e.g. Get("rootfield_name", "argument_name") . // -// The field needs to be a dot notated path to the position of the field, -// if it is nested. +// The response key is the alias if present, otherwise the field name. +// For aliased fields like "myAlias: user(id: 1)", use the alias "myAlias" in the path. +// +// The field path uses dot notation for nested fields. // For example you can access arg1 on field2 on the operation // // subscription { @@ -28,7 +30,16 @@ type Arguments struct { // // You need to call Get("mySub.field2", "arg1") . // -// If f or a cannot be found nil is returned. +// For aliased fields: +// +// query { +// a: user(id: "1") { name } +// b: user(id: "2") { name } +// } +// +// You need to call Get("a", "id") or Get("b", "id") respectively. +// +// If fa is nil, or f or a cannot be found, nil is returned. func (fa *Arguments) Get(f string, a string) *astjson.Value { if fa == nil { return nil diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go index fc7cad8080..24a9089b19 100644 --- a/router/core/field_argument_visitor.go +++ b/router/core/field_argument_visitor.go @@ -63,14 +63,26 @@ func (v *fieldArgumentsVisitor) dotPathFromAncestors() string { for _, anc := range v.walker.Ancestors { if anc.Kind == ast.NodeKindField { - fieldName := v.operation.FieldNameString(anc.Ref) - pathParts = append(pathParts, fieldName) + // Use the response key (alias if present, otherwise field name) + // to handle aliased fields and repeated selections correctly + responseKey := v.fieldResponseKey(anc.Ref) + pathParts = append(pathParts, responseKey) } } return strings.Join(pathParts, ".") } +// fieldResponseKey returns the response key for a field (alias if present, otherwise field name). +// This ensures unique paths for aliased fields like: a: user(id: 1) and b: user(id: 2) +func (v *fieldArgumentsVisitor) fieldResponseKey(fieldRef int) string { + alias := v.operation.FieldAliasString(fieldRef) + if alias != "" { + return alias + } + return v.operation.FieldNameString(fieldRef) +} + // resolveArgValue returns the value of val as astjson.Value. func (v *fieldArgumentsVisitor) resolveArgValue(val ast.Value) (*astjson.Value, error) { if val.Kind != ast.ValueKindVariable { @@ -127,7 +139,7 @@ func mapFieldArguments(opts mapFieldArgumentsOpts) Arguments { walker := astvisitor.NewWalker(48) logger := opts.logger - if logger != nil { + if logger == nil { logger = zap.NewNop() } diff --git a/router/core/field_argument_visitor_test.go b/router/core/field_argument_visitor_test.go index 652d97436f..d9871b3962 100644 --- a/router/core/field_argument_visitor_test.go +++ b/router/core/field_argument_visitor_test.go @@ -253,6 +253,45 @@ func TestMapFieldArguments(t *testing.T) { assert.True(t, activeVal.GetBool()) }, }, + { + name: "aliased fields have unique paths", + schema: ` + type Query { + user(id: ID!): User + } + type User { + id: ID! + name: String! + } + `, + operation: ` + query GetUsers($id1: ID!, $id2: ID!) { + a: user(id: $id1) { + id + name + } + b: user(id: $id2) { + id + name + } + } + `, + variables: `{"id1": "user-1", "id2": "user-2"}`, + assertions: func(t *testing.T, result Arguments) { + // Access arguments using the alias, not the field name + aIdArg := result.Get("a", "id") + require.NotNil(t, aIdArg, "expected 'id' argument on aliased field 'a'") + assert.Equal(t, "user-1", string(aIdArg.GetStringBytes())) + + bIdArg := result.Get("b", "id") + require.NotNil(t, bIdArg, "expected 'id' argument on aliased field 'b'") + assert.Equal(t, "user-2", string(bIdArg.GetStringBytes())) + + // Using the field name should not find the arguments + userIdArg := result.Get("user", "id") + assert.Nil(t, userIdArg, "expected nil when using field name instead of alias") + }, + }, } for _, tc := range testCases { From a14b7c455b954773aab0be93408ed66d711913c5 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 12 Dec 2025 13:07:24 +0100 Subject: [PATCH 15/22] fix: check report for errors --- router/core/arguments.go | 3 ++- router/core/context.go | 11 +++++++---- router/core/field_argument_visitor.go | 5 +++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/router/core/arguments.go b/router/core/arguments.go index e145474829..b181092ddd 100644 --- a/router/core/arguments.go +++ b/router/core/arguments.go @@ -7,6 +7,7 @@ type Arguments struct { // First key is the path to the field in dot notation // i.e. root_field.subfield1.subfield2. // Second argument is the name of the argument of that field. + // This map can be nil if no arguments are mapped. data map[string]map[string]*astjson.Value } @@ -41,7 +42,7 @@ type Arguments struct { // // If fa is nil, or f or a cannot be found, nil is returned. func (fa *Arguments) Get(f string, a string) *astjson.Value { - if fa == nil { + if fa == nil || fa.data == nil { return nil } diff --git a/router/core/context.go b/router/core/context.go index 80bd184d78..b8299c9a09 100644 --- a/router/core/context.go +++ b/router/core/context.go @@ -524,11 +524,14 @@ type operationContext struct { // RawContent is the raw content of the operation rawContent string // Content is the normalized content of the operation - content string + content string + // fieldArguments are the arguments of the operation. + // These are not mapped by default, only when certain custom modules require them. fieldArguments Arguments - variables *astjson.Value - files []*httpclient.FileUpload - clientInfo *ClientInfo + // variables are the variables of the operation + variables *astjson.Value + files []*httpclient.FileUpload + clientInfo *ClientInfo // preparedPlan is the prepared plan of the operation preparedPlan *planWithMetaData traceOptions resolve.TraceOptions diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go index 24a9089b19..146d6dcde5 100644 --- a/router/core/field_argument_visitor.go +++ b/router/core/field_argument_visitor.go @@ -156,6 +156,11 @@ func mapFieldArguments(opts mapFieldArgumentsOpts) Arguments { report := &operationreport.Report{} walker.Walk(opts.operation, opts.definition, report) + if report.HasErrors() { + logger.Warn("failed to map field arguments, no arguments will be available", + zap.Error(errors.New(report.Error()))) + return Arguments{} + } res := Arguments{ data: visitor.fieldArguments, From 33d8dfc63d45d0f0758866bda5fa4a4c2972a05d Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 12 Dec 2025 14:04:28 +0100 Subject: [PATCH 16/22] chore: fix typo in comments --- router/core/field_argument_visitor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go index 146d6dcde5..2a13f8f926 100644 --- a/router/core/field_argument_visitor.go +++ b/router/core/field_argument_visitor.go @@ -87,7 +87,7 @@ func (v *fieldArgumentsVisitor) fieldResponseKey(fieldRef int) string { func (v *fieldArgumentsVisitor) resolveArgValue(val ast.Value) (*astjson.Value, error) { if val.Kind != ast.ValueKindVariable { // Normally we never hit this code path because val.Kind should always be ast.ValueKindVariable. - // The operation parser automically creates variables and maps them to arguments, + // The operation parser automatically creates variables and maps them to arguments, // even if no initial variables are provided. // We should still be able to deal with arguments directly, // if for some reason they are not mapped to variables. From 0766e768da35e01def5d1e5724a8c101afe9b4f2 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Fri, 2 Jan 2026 10:19:08 +0100 Subject: [PATCH 17/22] chore: use custom type for field args --- router/core/arguments.go | 13 ++++++++----- router/core/field_argument_visitor.go | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/router/core/arguments.go b/router/core/arguments.go index b181092ddd..99a455a444 100644 --- a/router/core/arguments.go +++ b/router/core/arguments.go @@ -2,13 +2,15 @@ package core import "github.com/wundergraph/astjson" +// fieldArgs is a collection of field arguments with their names +// as keys and their corresponding values. +type fieldArgs map[string]*astjson.Value + // Arguments allow access to GraphQL field arguments used by clients. type Arguments struct { - // First key is the path to the field in dot notation - // i.e. root_field.subfield1.subfield2. - // Second argument is the name of the argument of that field. - // This map can be nil if no arguments are mapped. - data map[string]map[string]*astjson.Value + // data holds a map which contains all field arguments + // for any given field of an operation. + data map[string]fieldArgs } // Get will return the value of argument a from field f. @@ -28,6 +30,7 @@ type Arguments struct { // field1 // field2(arg1: "val3", arg2: "val4") // } +// } // // You need to call Get("mySub.field2", "arg1") . // diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go index 2a13f8f926..6de0b1f626 100644 --- a/router/core/field_argument_visitor.go +++ b/router/core/field_argument_visitor.go @@ -18,7 +18,7 @@ type fieldArgumentsVisitor struct { definition *ast.Document variables *astjson.Value remapVariables map[string]string - fieldArguments map[string]map[string]*astjson.Value + fieldArguments map[string]fieldArgs logger *zap.Logger } @@ -147,7 +147,7 @@ func mapFieldArguments(opts mapFieldArgumentsOpts) Arguments { walker: &walker, variables: opts.vars, remapVariables: opts.remapVariables, - fieldArguments: make(map[string]map[string]*astjson.Value), + fieldArguments: make(map[string]fieldArgs), logger: logger, } From 5dbc44af2cf6cae5a0b255d808f612f2d1a6f689 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Mon, 26 Jan 2026 18:04:55 +0100 Subject: [PATCH 18/22] feat: use engines field arg mapping --- router/core/arguments.go | 63 +++++++- router/core/cache_warmup.go | 2 +- router/core/context.go | 5 + router/core/field_argument_visitor.go | 170 --------------------- router/core/field_argument_visitor_test.go | 166 ++++++++------------ router/core/graphql_prehandler.go | 16 +- router/core/operation_processor.go | 31 ++-- router/core/operation_processor_test.go | 2 +- router/core/websocket.go | 15 +- 9 files changed, 168 insertions(+), 302 deletions(-) delete mode 100644 router/core/field_argument_visitor.go diff --git a/router/core/arguments.go b/router/core/arguments.go index 99a455a444..daf31e0ae1 100644 --- a/router/core/arguments.go +++ b/router/core/arguments.go @@ -1,6 +1,11 @@ package core -import "github.com/wundergraph/astjson" +import ( + "strings" + + "github.com/wundergraph/astjson" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization" +) // fieldArgs is a collection of field arguments with their names // as keys and their corresponding values. @@ -13,6 +18,62 @@ type Arguments struct { data map[string]fieldArgs } +// NewArgumentsFromMapping creates Arguments using the cached field argument mapping +// and the request's variable values. This is O(m) where m is the number of arguments, +// compared to the previous O(n) AST walk where n is the number of AST nodes. +// +// The mapping parameter maps "fieldPath.argumentName" to "variableName". +// For example: {"user.posts.limit": "a", "user.id": "userId"} +// +// The variables parameter contains the JSON-parsed variables from the request. +// The remapVariables parameter maps new variable names to original variable names. +func NewArgumentsFromMapping( + mapping astnormalization.FieldArgumentMapping, + variables *astjson.Value, + remapVariables map[string]string, +) Arguments { + if len(mapping) == 0 { + return Arguments{} + } + + data := make(map[string]fieldArgs, len(mapping)) + + for key, varName := range mapping { + // key format: "fieldPath.argumentName" (e.g., "user.posts.limit") + // We need to split by the last "." to separate field path from argument name + lastDot := strings.LastIndex(key, ".") + if lastDot == -1 { + // No dot found, skip this entry (shouldn't happen with valid data) + continue + } + + fieldPath := key[:lastDot] + argName := key[lastDot+1:] + + // Look up the original variable name if remapping is in effect + originalVarName := varName + if remapVariables != nil { + if original, ok := remapVariables[varName]; ok { + originalVarName = original + } + } + + // Get the variable value from the parsed variables + var argValue *astjson.Value + if variables != nil { + argValue = variables.Get(originalVarName) + } + + // Initialize the field's argument map if needed + if data[fieldPath] == nil { + data[fieldPath] = make(fieldArgs) + } + data[fieldPath][argName] = argValue + } + + return Arguments{data: data} +} + // Get will return the value of argument a from field f. // // To access an argument of a root level field, you need to pass the diff --git a/router/core/cache_warmup.go b/router/core/cache_warmup.go index b914567291..2319a60f94 100644 --- a/router/core/cache_warmup.go +++ b/router/core/cache_warmup.go @@ -295,7 +295,7 @@ func (c *CacheWarmupPlanningProcessor) ProcessOperation(ctx context.Context, ope return nil, err } - _, _, err = k.NormalizeVariables() + _, _, _, err = k.NormalizeVariables() if err != nil { return nil, err } diff --git a/router/core/context.go b/router/core/context.go index b8299c9a09..31ce6526d0 100644 --- a/router/core/context.go +++ b/router/core/context.go @@ -18,6 +18,7 @@ import ( "github.com/wundergraph/astjson" + "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasource/httpclient" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" @@ -528,6 +529,10 @@ type operationContext struct { // fieldArguments are the arguments of the operation. // These are not mapped by default, only when certain custom modules require them. fieldArguments Arguments + // fieldArgumentMapping maps field arguments to their variable names. + // This is populated during variable normalization and used to create fieldArguments + // with O(m) complexity instead of O(n) AST walk. + fieldArgumentMapping astnormalization.FieldArgumentMapping // variables are the variables of the operation variables *astjson.Value files []*httpclient.FileUpload diff --git a/router/core/field_argument_visitor.go b/router/core/field_argument_visitor.go deleted file mode 100644 index 6de0b1f626..0000000000 --- a/router/core/field_argument_visitor.go +++ /dev/null @@ -1,170 +0,0 @@ -package core - -import ( - "fmt" - "strings" - - "github.com/pkg/errors" - "github.com/wundergraph/astjson" - "github.com/wundergraph/graphql-go-tools/v2/pkg/ast" - "github.com/wundergraph/graphql-go-tools/v2/pkg/astvisitor" - "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" - "go.uber.org/zap" -) - -type fieldArgumentsVisitor struct { - walker *astvisitor.Walker - operation *ast.Document - definition *ast.Document - variables *astjson.Value - remapVariables map[string]string - fieldArguments map[string]fieldArgs - logger *zap.Logger -} - -func (v *fieldArgumentsVisitor) EnterDocument(operation, definition *ast.Document) { - v.operation = operation - v.definition = definition -} - -func (v *fieldArgumentsVisitor) EnterArgument(ref int) { - if len(v.walker.Ancestors) == 0 { - return - } - - // skip if we don't deal with field arguments (e.g. directive arguments) - anc := v.walker.Ancestors[len(v.walker.Ancestors)-1] - if anc.Kind != ast.NodeKindField { - return - } - - fieldPath := v.dotPathFromAncestors() - - if v.fieldArguments[fieldPath] == nil { - v.fieldArguments[fieldPath] = make(map[string]*astjson.Value) - } - - argName := v.operation.ArgumentNameString(ref) - argVal := v.operation.Arguments[ref].Value - - resolvedArgVal, err := v.resolveArgValue(argVal) - if err != nil { - v.logger. - With(zap.String("fieldPath", fieldPath), zap.String("argName", argName)). - Warn("failed to resolve argument value", zap.Error(err)) - return - } - - v.fieldArguments[fieldPath][argName] = resolvedArgVal -} - -func (v *fieldArgumentsVisitor) dotPathFromAncestors() string { - var pathParts []string - - for _, anc := range v.walker.Ancestors { - if anc.Kind == ast.NodeKindField { - // Use the response key (alias if present, otherwise field name) - // to handle aliased fields and repeated selections correctly - responseKey := v.fieldResponseKey(anc.Ref) - pathParts = append(pathParts, responseKey) - } - } - - return strings.Join(pathParts, ".") -} - -// fieldResponseKey returns the response key for a field (alias if present, otherwise field name). -// This ensures unique paths for aliased fields like: a: user(id: 1) and b: user(id: 2) -func (v *fieldArgumentsVisitor) fieldResponseKey(fieldRef int) string { - alias := v.operation.FieldAliasString(fieldRef) - if alias != "" { - return alias - } - return v.operation.FieldNameString(fieldRef) -} - -// resolveArgValue returns the value of val as astjson.Value. -func (v *fieldArgumentsVisitor) resolveArgValue(val ast.Value) (*astjson.Value, error) { - if val.Kind != ast.ValueKindVariable { - // Normally we never hit this code path because val.Kind should always be ast.ValueKindVariable. - // The operation parser automatically creates variables and maps them to arguments, - // even if no initial variables are provided. - // We should still be able to deal with arguments directly, - // if for some reason they are not mapped to variables. - return v.getArgValueFromDoc(val) - } - - return v.getArgValueFromVars(val) -} - -func (v *fieldArgumentsVisitor) getArgValueFromDoc(val ast.Value) (*astjson.Value, error) { - mval, err := v.operation.ValueToJSON(val) - if err != nil { - return nil, fmt.Errorf("marshal ast value: %w", err) - } - - res, err := astjson.ParseBytes(mval) - if err != nil { - return nil, fmt.Errorf("parse marshalled data as astjson.Value: %w", err) - } - - return res, nil -} - -func (v *fieldArgumentsVisitor) getArgValueFromVars(val ast.Value) (*astjson.Value, error) { - if v.variables == nil { - return nil, errors.New("value kind is of type 'variable' but no variables are set") - } - - varName := v.operation.VariableValueNameString(val.Ref) - originalVarName := varName - if v.remapVariables != nil { - if original, ok := v.remapVariables[varName]; ok { - originalVarName = original - } - } - - return v.variables.Get(originalVarName), nil -} - -type mapFieldArgumentsOpts struct { - operation *ast.Document - definition *ast.Document - vars *astjson.Value - remapVariables map[string]string - logger *zap.Logger -} - -func mapFieldArguments(opts mapFieldArgumentsOpts) Arguments { - walker := astvisitor.NewWalker(48) - - logger := opts.logger - if logger == nil { - logger = zap.NewNop() - } - - visitor := &fieldArgumentsVisitor{ - walker: &walker, - variables: opts.vars, - remapVariables: opts.remapVariables, - fieldArguments: make(map[string]fieldArgs), - logger: logger, - } - - walker.RegisterEnterDocumentVisitor(visitor) - walker.RegisterEnterArgumentVisitor(visitor) - - report := &operationreport.Report{} - walker.Walk(opts.operation, opts.definition, report) - if report.HasErrors() { - logger.Warn("failed to map field arguments, no arguments will be available", - zap.Error(errors.New(report.Error()))) - return Arguments{} - } - - res := Arguments{ - data: visitor.fieldArguments, - } - - return res -} diff --git a/router/core/field_argument_visitor_test.go b/router/core/field_argument_visitor_test.go index d9871b3962..bca8b19d90 100644 --- a/router/core/field_argument_visitor_test.go +++ b/router/core/field_argument_visitor_test.go @@ -6,14 +6,10 @@ 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" "github.com/wundergraph/graphql-go-tools/v2/pkg/astparser" "github.com/wundergraph/graphql-go-tools/v2/pkg/asttransform" "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" - "go.uber.org/zap/zaptest/observer" ) func TestMapFieldArguments(t *testing.T) { @@ -315,27 +311,42 @@ func TestMapFieldArguments(t *testing.T) { norm.NormalizeOperation(&operation, &schema, rep) require.False(t, rep.HasErrors(), "failed to normalize operation") + // Then normalize variables using VariablesNormalizer which returns the field argument mapping + varNorm := astnormalization.NewVariablesNormalizer() + result := varNorm.NormalizeOperation(&operation, &schema, rep) + require.False(t, rep.HasErrors(), "failed to normalize variables") + // Use normalized variables (includes both provided and extracted variables) vars, err := astjson.ParseBytes(operation.Input.Variables) require.NoError(t, err) - // Call mapFieldArguments - result := mapFieldArguments(mapFieldArgumentsOpts{ - operation: &operation, - definition: &schema, - vars: vars, - }) + // Create Arguments from the mapping (O(m) complexity) + arguments := NewArgumentsFromMapping( + result.FieldArgumentMapping, + vars, + nil, // no remapping in tests + ) // Run assertions - tc.assertions(t, result) + tc.assertions(t, arguments) }) } } -func TestMapFieldArguments_InvalidValueLogsWarning(t *testing.T) { - // This test verifies that when resolveArgValue fails (e.g., due to an invalid AST value), - // a warning is logged and the argument is skipped (not added to the result). +func TestNewArgumentsFromMapping_NilMapping(t *testing.T) { + // Test that nil mapping returns empty Arguments + result := NewArgumentsFromMapping(nil, nil, nil) + assert.Nil(t, result.Get("user", "id")) +} + +func TestNewArgumentsFromMapping_EmptyMapping(t *testing.T) { + // Test that empty mapping returns empty Arguments + result := NewArgumentsFromMapping(astnormalization.FieldArgumentMapping{}, nil, nil) + assert.Nil(t, result.Get("user", "id")) +} +func TestNewArgumentsFromMapping_WithRemapping(t *testing.T) { + // Test that variable remapping works correctly schema := ` type Query { user(id: ID!): User @@ -345,109 +356,60 @@ func TestMapFieldArguments_InvalidValueLogsWarning(t *testing.T) { } ` - operation := ` - query { - user(id: "123") { - id - } - } - ` - // Parse schema schemaDef, report := astparser.ParseGraphqlDocumentString(schema) require.False(t, report.HasErrors(), "failed to parse schema") err := asttransform.MergeDefinitionWithBaseSchema(&schemaDef) require.NoError(t, err) - // Parse operation WITHOUT normalization to keep the inline literal - op, report := astparser.ParseGraphqlDocumentString(operation) + // Parse operation + operation, report := astparser.ParseGraphqlDocumentString(` + query GetUser($userId: ID!) { + user(id: $userId) { + id + } + } + `) require.False(t, report.HasErrors(), "failed to parse operation") - // Corrupt the AST: set an invalid value kind that will cause ValueToJSON to fail - // Find the argument and set its value to an invalid kind - for i := range op.Arguments { - // Set to an invalid/unhandled value kind - op.Arguments[i].Value.Kind = ast.ValueKind(255) // Invalid kind - } - - // Set up observed logger to capture warnings - observedCore, observedLogs := observer.New(zapcore.WarnLevel) - logger := zap.New(observedCore) - - // Parse empty variables - vars, err := astjson.ParseBytes([]byte(`{}`)) - require.NoError(t, err) + // Set variables + operation.Input.Variables = []byte(`{"userId": "123"}`) - // Call mapFieldArguments - should log a warning due to invalid value - result := mapFieldArguments(mapFieldArgumentsOpts{ - operation: &op, - definition: &schemaDef, - vars: vars, - logger: logger, - }) - - // Verify warning was logged - logs := observedLogs.All() - require.Len(t, logs, 1, "expected exactly one warning log") - assert.Equal(t, "failed to resolve argument value", logs[0].Message) - assert.Equal(t, zapcore.WarnLevel, logs[0].Level) - - // Verify the argument was skipped (not added to result) - idArg := result.Get("user", "id") - assert.Nil(t, idArg, "expected argument to be skipped due to error") -} + // First normalize the operation + rep := &operationreport.Report{} + norm := astnormalization.NewNormalizer(true, true) + norm.NormalizeOperation(&operation, &schemaDef, rep) + require.False(t, rep.HasErrors(), "failed to normalize operation") -func TestMapFieldArguments_InlineLiteralWithoutNormalization(t *testing.T) { - // This test verifies that inline literal arguments are correctly extracted - // via getArgValueFromDoc when normalization is skipped. - // Normally, normalization converts inline literals to variables, but this test - // exercises the code path for non-variable argument values. + // Then normalize variables to get the mapping + varNorm := astnormalization.NewVariablesNormalizer() + normResult := varNorm.NormalizeOperation(&operation, &schemaDef, rep) + require.False(t, rep.HasErrors(), "failed to normalize variables") - schema := ` - type Query { - user(id: ID!, active: Boolean!): User - } - type User { - id: ID! - } - ` - - operation := ` - query { - user(id: "inline-123", active: true) { - id - } - } - ` - - // Parse schema - schemaDef, report := astparser.ParseGraphqlDocumentString(schema) - require.False(t, report.HasErrors(), "failed to parse schema") - err := asttransform.MergeDefinitionWithBaseSchema(&schemaDef) + // Parse variables + vars, err := astjson.ParseBytes(operation.Input.Variables) require.NoError(t, err) - // Parse operation WITHOUT normalization to keep inline literals as non-variable values - op, report := astparser.ParseGraphqlDocumentString(operation) - require.False(t, report.HasErrors(), "failed to parse operation") + // Test with remapping: simulate that "userId" was remapped to "a" + // We need to provide the original name so the lookup works + remapVariables := map[string]string{ + "a": "userId", // new name -> original name + } - // Parse empty variables (inline values are in the AST, not in variables) - vars, err := astjson.ParseBytes([]byte(`{}`)) - require.NoError(t, err) + // Modify the mapping to use the remapped name + modifiedMapping := astnormalization.FieldArgumentMapping{} + for k, v := range normResult.FieldArgumentMapping { + if v == "userId" { + modifiedMapping[k] = "a" // simulate remapping + } else { + modifiedMapping[k] = v + } + } - // Call mapFieldArguments - should extract values via getArgValueFromDoc - result := mapFieldArguments(mapFieldArgumentsOpts{ - operation: &op, - definition: &schemaDef, - vars: vars, - }) + result := NewArgumentsFromMapping(modifiedMapping, vars, remapVariables) - // Verify the string argument was correctly extracted + // The lookup should use the original variable name idArg := result.Get("user", "id") - require.NotNil(t, idArg, "expected 'id' argument to be accessible") - assert.Equal(t, "inline-123", string(idArg.GetStringBytes())) - - // Verify the boolean argument was correctly extracted - activeArg := result.Get("user", "active") - require.NotNil(t, activeArg, "expected 'active' argument to be accessible") - assert.True(t, activeArg.GetBool()) + require.NotNil(t, idArg, "expected 'id' argument on 'user' field") + assert.Equal(t, "123", string(idArg.GetStringBytes())) } diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index ba93dffce7..ccafff4b83 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -786,7 +786,7 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v * Normalize the variables */ - cached, uploadsMapping, err := operationKit.NormalizeVariables() + cached, uploadsMapping, fieldArgMapping, err := operationKit.NormalizeVariables() if err != nil { rtrace.AttachErrToSpan(engineNormalizeSpan, err) @@ -801,6 +801,8 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v engineNormalizeSpan.End() return err } + // Store the field argument mapping for later use when creating Arguments + requestContext.operation.fieldArgumentMapping = fieldArgMapping engineNormalizeSpan.SetAttributes(otel.WgVariablesNormalizationCacheHit.Bool(cached)) requestContext.operation.variablesNormalizationCacheHit = cached @@ -921,13 +923,11 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v } if h.mapFieldArguments { - requestContext.operation.fieldArguments = mapFieldArguments(mapFieldArgumentsOpts{ - operation: operationKit.kit.doc, - definition: h.executor.ClientSchema, - vars: requestContext.operation.variables, - remapVariables: requestContext.operation.remapVariables, - logger: h.log, - }) + requestContext.operation.fieldArguments = NewArgumentsFromMapping( + requestContext.operation.fieldArgumentMapping, + requestContext.operation.variables, + requestContext.operation.remapVariables, + ) } requestContext.operation.normalizationTime = time.Since(startNormalization) diff --git a/router/core/operation_processor.go b/router/core/operation_processor.go index 8ec9ba5bd1..2ade9d930d 100644 --- a/router/core/operation_processor.go +++ b/router/core/operation_processor.go @@ -780,6 +780,10 @@ type VariablesNormalizationCacheEntry struct { // request spec for file uploads. uploadsMapping []uploads.UploadPathMapping + // fieldArgumentMapping maps field arguments to their variable names for fast lookup. + // This is populated during variable normalization and cached to avoid repeated AST walks. + fieldArgumentMapping astnormalization.FieldArgumentMapping + // reparse indicates whether the operation document needs to be reparsed from // its string representation when retrieved from the cache. reparse bool @@ -907,10 +911,10 @@ func (o *OperationKit) normalizeVariablesCacheKey() uint64 { } // NormalizeVariables normalizes variables and returns a slice of upload mappings -// if any of them were present in a query. +// if any of them were present in a query, as well as the field argument mapping. // If normalized values were found in the cache, it skips normalization and returns the caching set to true. // If an error is returned, then caching is set to false. -func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.UploadPathMapping, err error) { +func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.UploadPathMapping, fieldArgMapping astnormalization.FieldArgumentMapping, err error) { cacheKey := o.normalizeVariablesCacheKey() if o.cache != nil && o.cache.variablesNormalizationCache != nil { entry, ok := o.cache.variablesNormalizationCache.Get(cacheKey) @@ -921,10 +925,10 @@ func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.Uplo if entry.reparse { if err = o.setAndParseOperationDoc(); err != nil { - return false, nil, err + return false, nil, nil, err } } - return true, entry.uploadsMapping, nil + return true, entry.uploadsMapping, entry.fieldArgumentMapping, nil } } @@ -935,11 +939,14 @@ func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.Uplo copy(operationRawBytesBefore, o.kit.doc.Input.RawBytes) report := &operationreport.Report{} - uploadsMapping := o.kit.variablesNormalizer.NormalizeOperation(o.kit.doc, o.operationProcessor.executor.ClientSchema, report) + normalizerResult := o.kit.variablesNormalizer.NormalizeOperation(o.kit.doc, o.operationProcessor.executor.ClientSchema, report) if report.HasErrors() { - return false, nil, &reportError{report: report} + return false, nil, nil, &reportError{report: report} } + uploadsMapping := normalizerResult.UploadsMapping + fieldArgumentMapping := normalizerResult.FieldArgumentMapping + // Assuming the user sends a multi-operation document // During normalization, we removed the unused operations from the document // This will always lead to operation definitions of a length of 1 even when multiple operations are sent @@ -959,14 +966,14 @@ func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.Uplo err = o.kit.printer.Print(o.kit.doc, o.kit.normalizedOperation) if err != nil { - return false, nil, err + return false, nil, nil, err } // Reset the doc with the original name o.kit.doc.OperationDefinitions[o.operationDefinitionRef].Name = nameRef _, err = o.kit.keyGen.Write(o.kit.normalizedOperation.Bytes()) if err != nil { - return false, nil, err + return false, nil, nil, err } o.parsedOperation.ID = o.kit.keyGen.Sum64() o.kit.keyGen.Reset() @@ -976,6 +983,7 @@ func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.Uplo if o.cache != nil && o.cache.variablesNormalizationCache != nil { entry := VariablesNormalizationCacheEntry{ uploadsMapping: uploadsMapping, + fieldArgumentMapping: fieldArgumentMapping, id: o.parsedOperation.ID, normalizedRepresentation: o.parsedOperation.NormalizedRepresentation, variables: o.parsedOperation.Request.Variables, @@ -984,14 +992,14 @@ func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.Uplo o.cache.variablesNormalizationCache.Set(cacheKey, entry, 1) } - return false, uploadsMapping, nil + return false, uploadsMapping, fieldArgumentMapping, nil } o.kit.normalizedOperation.Reset() err = o.kit.printer.Print(o.kit.doc, o.kit.normalizedOperation) if err != nil { - return false, nil, err + return false, nil, nil, err } o.parsedOperation.NormalizedRepresentation = o.kit.normalizedOperation.String() @@ -1000,6 +1008,7 @@ func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.Uplo if o.cache != nil && o.cache.variablesNormalizationCache != nil { entry := VariablesNormalizationCacheEntry{ uploadsMapping: uploadsMapping, + fieldArgumentMapping: fieldArgumentMapping, id: o.parsedOperation.ID, normalizedRepresentation: o.parsedOperation.NormalizedRepresentation, variables: o.parsedOperation.Request.Variables, @@ -1008,7 +1017,7 @@ func (o *OperationKit) NormalizeVariables() (cached bool, mapping []uploads.Uplo o.cache.variablesNormalizationCache.Set(cacheKey, entry, 1) } - return false, uploadsMapping, nil + return false, uploadsMapping, fieldArgumentMapping, nil } func (o *OperationKit) remapVariablesCacheKey(disabled bool) uint64 { diff --git a/router/core/operation_processor_test.go b/router/core/operation_processor_test.go index 589fcdf50c..d92bdc6bd1 100644 --- a/router/core/operation_processor_test.go +++ b/router/core/operation_processor_test.go @@ -261,7 +261,7 @@ func TestNormalizeVariablesOperationProcessor(t *testing.T) { _, err = kit.NormalizeOperation("test", false) require.NoError(t, err) - _, _, err = kit.NormalizeVariables() + _, _, _, err = kit.NormalizeVariables() require.NoError(t, err) assert.Equal(t, tc.ExpectedNormalizedRepresentation, kit.parsedOperation.NormalizedRepresentation) diff --git a/router/core/websocket.go b/router/core/websocket.go index 337a3ebd2d..2a45b15703 100644 --- a/router/core/websocket.go +++ b/router/core/websocket.go @@ -912,12 +912,13 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi } opContext.normalizationCacheHit = operationKit.parsedOperation.NormalizationCacheHit - cached, _, err := operationKit.NormalizeVariables() + cached, _, fieldArgMapping, err := operationKit.NormalizeVariables() if err != nil { opContext.normalizationTime = time.Since(startNormalization) return nil, nil, err } opContext.variablesNormalizationCacheHit = cached + opContext.fieldArgumentMapping = fieldArgMapping cached, err = operationKit.RemapVariables(h.disableVariablesRemapping) if err != nil { @@ -938,13 +939,11 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi } if h.mapFieldArguments { - opContext.fieldArguments = mapFieldArguments(mapFieldArgumentsOpts{ - operation: operationKit.kit.doc, - definition: h.operationProcessor.executor.ClientSchema, - vars: opContext.variables, - remapVariables: opContext.remapVariables, - logger: h.logger, - }) + opContext.fieldArguments = NewArgumentsFromMapping( + opContext.fieldArgumentMapping, + opContext.variables, + opContext.remapVariables, + ) } startValidation := time.Now() From 9395881c591dfd7373629d81c5c43452488de430 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Tue, 27 Jan 2026 14:13:30 +0100 Subject: [PATCH 19/22] fix: avoid map creation during request processing --- router/core/arguments.go | 82 ++++------------- ...ment_visitor_test.go => arguments_test.go} | 87 ++----------------- router/core/context.go | 5 -- router/core/graphql_prehandler.go | 6 +- router/core/websocket.go | 6 +- 5 files changed, 29 insertions(+), 157 deletions(-) rename router/core/{field_argument_visitor_test.go => arguments_test.go} (76%) diff --git a/router/core/arguments.go b/router/core/arguments.go index daf31e0ae1..10aaf87220 100644 --- a/router/core/arguments.go +++ b/router/core/arguments.go @@ -1,77 +1,29 @@ package core import ( - "strings" - "github.com/wundergraph/astjson" "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization" ) -// fieldArgs is a collection of field arguments with their names -// as keys and their corresponding values. -type fieldArgs map[string]*astjson.Value - // Arguments allow access to GraphQL field arguments used by clients. type Arguments struct { - // data holds a map which contains all field arguments - // for any given field of an operation. - data map[string]fieldArgs + // mapping maps "fieldPath.argumentName" to "variableName". + // For example: {"user.posts.limit": "a", "user.id": "userId"} + mapping astnormalization.FieldArgumentMapping + + // variables contains the JSON-parsed variables from the request. + variables *astjson.Value } -// NewArgumentsFromMapping creates Arguments using the cached field argument mapping -// and the request's variable values. This is O(m) where m is the number of arguments, -// compared to the previous O(n) AST walk where n is the number of AST nodes. -// -// The mapping parameter maps "fieldPath.argumentName" to "variableName". -// For example: {"user.posts.limit": "a", "user.id": "userId"} -// -// The variables parameter contains the JSON-parsed variables from the request. -// The remapVariables parameter maps new variable names to original variable names. -func NewArgumentsFromMapping( +// NewArguments creates an Arguments instance. +func NewArguments( mapping astnormalization.FieldArgumentMapping, variables *astjson.Value, - remapVariables map[string]string, ) Arguments { - if len(mapping) == 0 { - return Arguments{} - } - - data := make(map[string]fieldArgs, len(mapping)) - - for key, varName := range mapping { - // key format: "fieldPath.argumentName" (e.g., "user.posts.limit") - // We need to split by the last "." to separate field path from argument name - lastDot := strings.LastIndex(key, ".") - if lastDot == -1 { - // No dot found, skip this entry (shouldn't happen with valid data) - continue - } - - fieldPath := key[:lastDot] - argName := key[lastDot+1:] - - // Look up the original variable name if remapping is in effect - originalVarName := varName - if remapVariables != nil { - if original, ok := remapVariables[varName]; ok { - originalVarName = original - } - } - - // Get the variable value from the parsed variables - var argValue *astjson.Value - if variables != nil { - argValue = variables.Get(originalVarName) - } - - // Initialize the field's argument map if needed - if data[fieldPath] == nil { - data[fieldPath] = make(fieldArgs) - } - data[fieldPath][argName] = argValue + return Arguments{ + mapping: mapping, + variables: variables, } - - return Arguments{data: data} } // Get will return the value of argument a from field f. @@ -106,14 +58,18 @@ func NewArgumentsFromMapping( // // If fa is nil, or f or a cannot be found, nil is returned. func (fa *Arguments) Get(f string, a string) *astjson.Value { - if fa == nil || fa.data == nil { + if fa == nil || len(fa.mapping) == 0 || fa.variables == nil { return nil } - args, found := fa.data[f] - if !found { + // Build the mapping key: "fieldPath.argumentName" + key := f + "." + a + + // Look up variable name from mapping + varName, ok := fa.mapping[key] + if !ok { return nil } - return args[a] + return fa.variables.Get(varName) } diff --git a/router/core/field_argument_visitor_test.go b/router/core/arguments_test.go similarity index 76% rename from router/core/field_argument_visitor_test.go rename to router/core/arguments_test.go index bca8b19d90..47d8342394 100644 --- a/router/core/field_argument_visitor_test.go +++ b/router/core/arguments_test.go @@ -12,7 +12,7 @@ import ( "github.com/wundergraph/graphql-go-tools/v2/pkg/operationreport" ) -func TestMapFieldArguments(t *testing.T) { +func TestArgumentMapping(t *testing.T) { testCases := []struct { name string schema string @@ -320,96 +320,21 @@ func TestMapFieldArguments(t *testing.T) { vars, err := astjson.ParseBytes(operation.Input.Variables) require.NoError(t, err) - // Create Arguments from the mapping (O(m) complexity) - arguments := NewArgumentsFromMapping( - result.FieldArgumentMapping, - vars, - nil, // no remapping in tests - ) + arguments := NewArguments(result.FieldArgumentMapping, vars) - // Run assertions tc.assertions(t, arguments) }) } } -func TestNewArgumentsFromMapping_NilMapping(t *testing.T) { +func TestNewArguments_NilMapping(t *testing.T) { // Test that nil mapping returns empty Arguments - result := NewArgumentsFromMapping(nil, nil, nil) + result := NewArguments(nil, nil) assert.Nil(t, result.Get("user", "id")) } -func TestNewArgumentsFromMapping_EmptyMapping(t *testing.T) { +func TestNewArguments_EmptyMapping(t *testing.T) { // Test that empty mapping returns empty Arguments - result := NewArgumentsFromMapping(astnormalization.FieldArgumentMapping{}, nil, nil) + result := NewArguments(astnormalization.FieldArgumentMapping{}, nil) assert.Nil(t, result.Get("user", "id")) } - -func TestNewArgumentsFromMapping_WithRemapping(t *testing.T) { - // Test that variable remapping works correctly - schema := ` - type Query { - user(id: ID!): User - } - type User { - id: ID! - } - ` - - // Parse schema - schemaDef, report := astparser.ParseGraphqlDocumentString(schema) - require.False(t, report.HasErrors(), "failed to parse schema") - err := asttransform.MergeDefinitionWithBaseSchema(&schemaDef) - require.NoError(t, err) - - // Parse operation - operation, report := astparser.ParseGraphqlDocumentString(` - query GetUser($userId: ID!) { - user(id: $userId) { - id - } - } - `) - require.False(t, report.HasErrors(), "failed to parse operation") - - // Set variables - operation.Input.Variables = []byte(`{"userId": "123"}`) - - // First normalize the operation - rep := &operationreport.Report{} - norm := astnormalization.NewNormalizer(true, true) - norm.NormalizeOperation(&operation, &schemaDef, rep) - require.False(t, rep.HasErrors(), "failed to normalize operation") - - // Then normalize variables to get the mapping - varNorm := astnormalization.NewVariablesNormalizer() - normResult := varNorm.NormalizeOperation(&operation, &schemaDef, rep) - require.False(t, rep.HasErrors(), "failed to normalize variables") - - // Parse variables - vars, err := astjson.ParseBytes(operation.Input.Variables) - require.NoError(t, err) - - // Test with remapping: simulate that "userId" was remapped to "a" - // We need to provide the original name so the lookup works - remapVariables := map[string]string{ - "a": "userId", // new name -> original name - } - - // Modify the mapping to use the remapped name - modifiedMapping := astnormalization.FieldArgumentMapping{} - for k, v := range normResult.FieldArgumentMapping { - if v == "userId" { - modifiedMapping[k] = "a" // simulate remapping - } else { - modifiedMapping[k] = v - } - } - - result := NewArgumentsFromMapping(modifiedMapping, vars, remapVariables) - - // The lookup should use the original variable name - idArg := result.Get("user", "id") - require.NotNil(t, idArg, "expected 'id' argument on 'user' field") - assert.Equal(t, "123", string(idArg.GetStringBytes())) -} diff --git a/router/core/context.go b/router/core/context.go index 31ce6526d0..b8299c9a09 100644 --- a/router/core/context.go +++ b/router/core/context.go @@ -18,7 +18,6 @@ import ( "github.com/wundergraph/astjson" - "github.com/wundergraph/graphql-go-tools/v2/pkg/astnormalization" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/datasource/httpclient" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/resolve" @@ -529,10 +528,6 @@ type operationContext struct { // fieldArguments are the arguments of the operation. // These are not mapped by default, only when certain custom modules require them. fieldArguments Arguments - // fieldArgumentMapping maps field arguments to their variable names. - // This is populated during variable normalization and used to create fieldArguments - // with O(m) complexity instead of O(n) AST walk. - fieldArgumentMapping astnormalization.FieldArgumentMapping // variables are the variables of the operation variables *astjson.Value files []*httpclient.FileUpload diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index ccafff4b83..261cdf8490 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -802,7 +802,6 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v return err } // Store the field argument mapping for later use when creating Arguments - requestContext.operation.fieldArgumentMapping = fieldArgMapping engineNormalizeSpan.SetAttributes(otel.WgVariablesNormalizationCacheHit.Bool(cached)) requestContext.operation.variablesNormalizationCacheHit = cached @@ -923,10 +922,9 @@ func (h *PreHandler) handleOperation(w http.ResponseWriter, req *http.Request, v } if h.mapFieldArguments { - requestContext.operation.fieldArguments = NewArgumentsFromMapping( - requestContext.operation.fieldArgumentMapping, + requestContext.operation.fieldArguments = NewArguments( + fieldArgMapping, requestContext.operation.variables, - requestContext.operation.remapVariables, ) } diff --git a/router/core/websocket.go b/router/core/websocket.go index 2a45b15703..7a8fadcca7 100644 --- a/router/core/websocket.go +++ b/router/core/websocket.go @@ -918,7 +918,6 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi return nil, nil, err } opContext.variablesNormalizationCacheHit = cached - opContext.fieldArgumentMapping = fieldArgMapping cached, err = operationKit.RemapVariables(h.disableVariablesRemapping) if err != nil { @@ -939,10 +938,9 @@ func (h *WebSocketConnectionHandler) parseAndPlan(registration *SubscriptionRegi } if h.mapFieldArguments { - opContext.fieldArguments = NewArgumentsFromMapping( - opContext.fieldArgumentMapping, + opContext.fieldArguments = NewArguments( + fieldArgMapping, opContext.variables, - opContext.remapVariables, ) } From 9f0e470965625e7b9ee13256fce4e44a9a58b861 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Mon, 2 Feb 2026 13:07:51 +0100 Subject: [PATCH 20/22] feat: populate option to disable field arg mapping --- router/core/arguments.go | 77 +++++++++++++++++++++--------- router/core/graph_server.go | 1 + router/core/operation_processor.go | 5 +- 3 files changed, 59 insertions(+), 24 deletions(-) diff --git a/router/core/arguments.go b/router/core/arguments.go index 10aaf87220..eab6788119 100644 --- a/router/core/arguments.go +++ b/router/core/arguments.go @@ -26,50 +26,81 @@ func NewArguments( } } -// Get will return the value of argument a from field f. +// Get will return the value of the field argument at path. // -// To access an argument of a root level field, you need to pass the -// response key of the field as the first argument to Get and the name of the argument -// as the second argument, e.g. Get("rootfield_name", "argument_name") . +// To access a specific field argument you need to provide +// the path in it's GraphQL operation via dot notation, +// prefixed by the root levels type. // -// The response key is the alias if present, otherwise the field name. -// For aliased fields like "myAlias: user(id: 1)", use the alias "myAlias" in the path. +// Get("rootfield_operation_type.rootfield_name.other.fields.argument_name") // -// The field path uses dot notation for nested fields. -// For example you can access arg1 on field2 on the operation +// To access the storeId field argument of the operation // // subscription { -// mySub(arg1: "val1", arg2: "val2") { -// field1 -// field2(arg1: "val3", arg2: "val4") -// } +// orderUpdated(storeId: 1) { +// id +// status +// } // } // -// You need to call Get("mySub.field2", "arg1") . +// you need to call Get("subscription.orderUpdated.storeId") . +// You can also access deeper nested fields. +// For example you can access the categoryId field of the operation // -// For aliased fields: +// subscription { +// orderUpdated(storeId: 1) { +// lineItems(categoryId: 2) { +// id +// name +// } +// } +// } +// +// by calling Get("subscription.orderUpdated.lineItems.categoryId") . +// +// If you use aliases in operation you need to provide the alias name +// instead of the field name. // // query { -// a: user(id: "1") { name } -// b: user(id: "2") { name } +// a: user(id: "1") { name } +// b: user(id: "2") { name } +// } +// +// You need to call Get("query.a.id") or Get("query.b.id") respectively. +// +// If you want to access field arguments of fragments, you need to +// access it on one of the fields where the fragment is resolved. +// +// fragment GoldTrophies on RaceDrivers { +// trophies(color:"gold") { +// title +// } +// } +// +// subscription { +// driversFinish { +// name +// ... GoldTrophies +// } // } // -// You need to call Get("a", "id") or Get("b", "id") respectively. +// If you want to access the "color" field argument, you need to +// call Get("subscription.driversFinish.trophies.color") . +// The same concept applies to inline fragments. // // If fa is nil, or f or a cannot be found, nil is returned. -func (fa *Arguments) Get(f string, a string) *astjson.Value { +func (fa *Arguments) Get(path string) *astjson.Value { if fa == nil || len(fa.mapping) == 0 || fa.variables == nil { return nil } - // Build the mapping key: "fieldPath.argumentName" - key := f + "." + a - - // Look up variable name from mapping - varName, ok := fa.mapping[key] + // Look up variable name from field argument map + varName, ok := fa.mapping[path] if !ok { return nil } + // Use the name to get the actual value from + // the operation contexts variables. return fa.variables.Get(varName) } diff --git a/router/core/graph_server.go b/router/core/graph_server.go index 76bb4bbe1c..7ed37d22a9 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -1295,6 +1295,7 @@ func (s *graphServer) buildGraphMux( ApolloRouterCompatibilityFlags: s.apolloRouterCompatibilityFlags, DisableExposingVariablesContentOnValidationError: s.engineExecutionConfiguration.DisableExposingVariablesContentOnValidationError, ComplexityLimits: s.securityConfiguration.ComplexityLimits, + EnableFieldArgumentMapping: s.subscriptionHooks.needFieldArgumentMapping(), }) operationPlanner := NewOperationPlanner(executor, gm.planCache) diff --git a/router/core/operation_processor.go b/router/core/operation_processor.go index 2ade9d930d..b19f08d87e 100644 --- a/router/core/operation_processor.go +++ b/router/core/operation_processor.go @@ -125,6 +125,7 @@ type OperationProcessorOptions struct { ComplexityLimits *config.ComplexityLimits ParserTokenizerLimits astparser.TokenizerLimits OperationNameLengthLimit int + EnableFieldArgumentMapping bool } // OperationProcessor provides shared resources to the parseKit and OperationKit. @@ -1408,6 +1409,7 @@ type parseKitOptions struct { apolloCompatibilityFlags config.ApolloCompatibilityFlags apolloRouterCompatibilityFlags config.ApolloRouterCompatibilityFlags disableExposingVariablesContentOnValidationError bool + enableFieldArgumentMapping bool } func createParseKit(i int, options *parseKitOptions) *parseKit { @@ -1423,7 +1425,7 @@ func createParseKit(i int, options *parseKitOptions) *parseKit { astnormalization.WithRemoveFragmentDefinitions(), astnormalization.WithRemoveUnusedVariables(), ), - variablesNormalizer: astnormalization.NewVariablesNormalizer(), + variablesNormalizer: astnormalization.NewVariablesNormalizer(options.enableFieldArgumentMapping), variablesRemapper: astnormalization.NewVariablesMapper(), printer: &astprinter.Printer{}, normalizedOperation: &bytes.Buffer{}, @@ -1462,6 +1464,7 @@ func NewOperationProcessor(opts OperationProcessorOptions) *OperationProcessor { apolloCompatibilityFlags: opts.ApolloCompatibilityFlags, apolloRouterCompatibilityFlags: opts.ApolloRouterCompatibilityFlags, disableExposingVariablesContentOnValidationError: opts.DisableExposingVariablesContentOnValidationError, + enableFieldArgumentMapping: opts.EnableFieldArgumentMapping, }, } for i := 0; i < opts.ParseKitPoolSize; i++ { From b102472aa8d3b440a88b04fd8df71c42045126f7 Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Mon, 2 Feb 2026 16:01:33 +0100 Subject: [PATCH 21/22] chore: use corresponding graph-go-tools version Only temporary until the engine changes have been merged. Then the go.mod files here will be updated again. --- router-tests/go.mod | 2 +- router-tests/go.sum | 4 +-- .../modules/start_subscription_test.go | 2 +- router-tests/modules/stream_publish_test.go | 2 +- router-tests/modules/stream_receive_test.go | 2 +- router/core/arguments_test.go | 34 +++++++++---------- router/go.mod | 2 +- router/go.sum | 4 +-- 8 files changed, 26 insertions(+), 26 deletions(-) diff --git a/router-tests/go.mod b/router-tests/go.mod index d3940063ef..bcbf779c4c 100644 --- a/router-tests/go.mod +++ b/router-tests/go.mod @@ -26,7 +26,7 @@ require ( github.com/wundergraph/cosmo/demo/pkg/subgraphs/projects v0.0.0-20250715110703-10f2e5f9c79e github.com/wundergraph/cosmo/router v0.0.0-20251125205644-175f80c4e6d9 github.com/wundergraph/cosmo/router-plugin v0.0.0-20250808194725-de123ba1c65e - github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241 + github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.242.0.20260202140551-546a496bbb69 go.opentelemetry.io/otel v1.36.0 go.opentelemetry.io/otel/sdk v1.36.0 go.opentelemetry.io/otel/sdk/metric v1.36.0 diff --git a/router-tests/go.sum b/router-tests/go.sum index 23ed1909fb..1c7e6eee22 100644 --- a/router-tests/go.sum +++ b/router-tests/go.sum @@ -352,8 +352,8 @@ github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/ github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= github.com/wundergraph/astjson v0.0.0-20250106123708-be463c97e083 h1:8/D7f8gKxTBjW+SZK4mhxTTBVpxcqeBgWF1Rfmltbfk= github.com/wundergraph/astjson v0.0.0-20250106123708-be463c97e083/go.mod h1:eOTL6acwctsN4F3b7YE+eE2t8zcJ/doLm9sZzsxxxrE= -github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241 h1:ch/8hfDaw4oz1Cx3Wb+OUl4qiAo17OdGhYMdRYnX8Is= -github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241/go.mod h1:mX25ASEQiKamxaFSK6NZihh0oDCigIuzro30up4mFH4= +github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.242.0.20260202140551-546a496bbb69 h1:HFqBQSMA3VjRFy/LjtKFd+vmO6tvj6BLuotC+dugEqU= +github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.242.0.20260202140551-546a496bbb69/go.mod h1:mX25ASEQiKamxaFSK6NZihh0oDCigIuzro30up4mFH4= github.com/xrash/smetrics v0.0.0-20250705151800-55b8f293f342 h1:FnBeRrxr7OU4VvAzt5X7s6266i6cSVkkFPS0TuXWbIg= github.com/xrash/smetrics v0.0.0-20250705151800-55b8f293f342/go.mod h1:Ohn+xnUBiLI6FVj/9LpzZWtj1/D6lUovWYBkxHVV3aM= github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= diff --git a/router-tests/modules/start_subscription_test.go b/router-tests/modules/start_subscription_test.go index 0181e72da3..a63083e414 100644 --- a/router-tests/modules/start_subscription_test.go +++ b/router-tests/modules/start_subscription_test.go @@ -725,7 +725,7 @@ func TestStartSubscriptionHook(t *testing.T) { Callback: func(ctx core.SubscriptionOnStartHandlerContext) error { args := ctx.Operation().Arguments() if args != nil { - employeeIDArg := args.Get("employeeUpdatedMyKafka", "employeeID") + employeeIDArg := args.Get("subscription.employeeUpdatedMyKafka.employeeID") if employeeIDArg != nil { capturedEmployeeID = employeeIDArg.GetInt() } diff --git a/router-tests/modules/stream_publish_test.go b/router-tests/modules/stream_publish_test.go index 564596e88e..7b06572096 100644 --- a/router-tests/modules/stream_publish_test.go +++ b/router-tests/modules/stream_publish_test.go @@ -372,7 +372,7 @@ func TestPublishHook(t *testing.T) { Callback: func(ctx core.StreamPublishEventHandlerContext, events datasource.StreamEvents) (datasource.StreamEvents, error) { args := ctx.Operation().Arguments() if args != nil { - employeeIDArg := args.Get("updateEmployeeMyKafka", "employeeID") + employeeIDArg := args.Get("subscription.updateEmployeeMyKafka.employeeID") if employeeIDArg != nil { capturedEmployeeID = employeeIDArg.GetInt() } diff --git a/router-tests/modules/stream_receive_test.go b/router-tests/modules/stream_receive_test.go index 595c548b60..3936d92b6c 100644 --- a/router-tests/modules/stream_receive_test.go +++ b/router-tests/modules/stream_receive_test.go @@ -977,7 +977,7 @@ func TestReceiveHook(t *testing.T) { Callback: func(ctx core.StreamReceiveEventHandlerContext, events datasource.StreamEvents) (datasource.StreamEvents, error) { args := ctx.Operation().Arguments() if args != nil { - employeeIDArg := args.Get("employeeUpdatedMyKafka", "employeeID") + employeeIDArg := args.Get("subscription.employeeUpdatedMyKafka.employeeID") if employeeIDArg != nil { capturedEmployeeID = employeeIDArg.GetInt() } diff --git a/router/core/arguments_test.go b/router/core/arguments_test.go index 47d8342394..a7f1428792 100644 --- a/router/core/arguments_test.go +++ b/router/core/arguments_test.go @@ -41,7 +41,7 @@ func TestArgumentMapping(t *testing.T) { `, variables: `{"userId": "123"}`, assertions: func(t *testing.T, result Arguments) { - idArg := result.Get("user", "id") + idArg := result.Get("query.user.id") require.NotNil(t, idArg, "expected 'id' argument on 'user' field") assert.Equal(t, "123", string(idArg.GetStringBytes())) }, @@ -67,7 +67,7 @@ func TestArgumentMapping(t *testing.T) { `, variables: `{}`, assertions: func(t *testing.T, result Arguments) { - idArg := result.Get("user", "id") + idArg := result.Get("query.user.id") require.NotNil(t, idArg, "expected 'id' argument on 'user' field") assert.Equal(t, "123", string(idArg.GetStringBytes())) }, @@ -101,16 +101,16 @@ func TestArgumentMapping(t *testing.T) { variables: `{"userId": "user-1", "limit": 10, "offset": 5}`, assertions: func(t *testing.T, result Arguments) { // Assert root field argument - userIdArg := result.Get("user", "id") + userIdArg := result.Get("query.user.id") require.NotNil(t, userIdArg) assert.Equal(t, "user-1", string(userIdArg.GetStringBytes())) // Assert nested field arguments (dot notation path) - limitArg := result.Get("user.posts", "limit") + limitArg := result.Get("query.user.posts.limit") require.NotNil(t, limitArg, "expected 'limit' argument on 'user.posts' field") assert.Equal(t, 10, limitArg.GetInt()) - offsetArg := result.Get("user.posts", "offset") + offsetArg := result.Get("query.user.posts.offset") require.NotNil(t, offsetArg, "expected 'offset' argument on 'user.posts' field") assert.Equal(t, 5, offsetArg.GetInt()) }, @@ -129,10 +129,10 @@ func TestArgumentMapping(t *testing.T) { `, variables: `{}`, assertions: func(t *testing.T, result Arguments) { - arg := result.Get("hello", "someArg") + arg := result.Get("query.hello.someArg") require.Nil(t, arg, "expected nil for non-existent argument") - arg = result.Get("nonExistent", "arg") + arg = result.Get("query.nonExistent.arg") require.Nil(t, arg, "expected nil for non-existent field") }, }, @@ -162,11 +162,11 @@ func TestArgumentMapping(t *testing.T) { `, variables: `{"userId": "user-123", "postSlug": "my-post"}`, assertions: func(t *testing.T, result Arguments) { - userIdArg := result.Get("user", "id") + userIdArg := result.Get("query.user.id") require.NotNil(t, userIdArg) assert.Equal(t, "user-123", string(userIdArg.GetStringBytes())) - postSlugArg := result.Get("post", "slug") + postSlugArg := result.Get("query.post.slug") require.NotNil(t, postSlugArg) assert.Equal(t, "my-post", string(postSlugArg.GetStringBytes())) }, @@ -192,7 +192,7 @@ func TestArgumentMapping(t *testing.T) { `, variables: `{"userIds": ["user-1", "user-2", "user-3"]}`, assertions: func(t *testing.T, result Arguments) { - idsArg := result.Get("users", "ids") + idsArg := result.Get("query.users.ids") require.NotNil(t, idsArg, "expected 'ids' argument on 'users' field") // Verify it's an array @@ -229,7 +229,7 @@ func TestArgumentMapping(t *testing.T) { `, variables: `{"filter": {"name": "John", "age": 30, "active": true}}`, assertions: func(t *testing.T, result Arguments) { - filterArg := result.Get("users", "filter") + filterArg := result.Get("query.users.filter") require.NotNil(t, filterArg, "expected 'filter' argument on 'users' field") // Verify it's an object and access its fields @@ -275,16 +275,16 @@ func TestArgumentMapping(t *testing.T) { variables: `{"id1": "user-1", "id2": "user-2"}`, assertions: func(t *testing.T, result Arguments) { // Access arguments using the alias, not the field name - aIdArg := result.Get("a", "id") + aIdArg := result.Get("query.a.id") require.NotNil(t, aIdArg, "expected 'id' argument on aliased field 'a'") assert.Equal(t, "user-1", string(aIdArg.GetStringBytes())) - bIdArg := result.Get("b", "id") + bIdArg := result.Get("query.b.id") require.NotNil(t, bIdArg, "expected 'id' argument on aliased field 'b'") assert.Equal(t, "user-2", string(bIdArg.GetStringBytes())) // Using the field name should not find the arguments - userIdArg := result.Get("user", "id") + userIdArg := result.Get("query.user.id") assert.Nil(t, userIdArg, "expected nil when using field name instead of alias") }, }, @@ -312,7 +312,7 @@ func TestArgumentMapping(t *testing.T) { require.False(t, rep.HasErrors(), "failed to normalize operation") // Then normalize variables using VariablesNormalizer which returns the field argument mapping - varNorm := astnormalization.NewVariablesNormalizer() + varNorm := astnormalization.NewVariablesNormalizer(true) result := varNorm.NormalizeOperation(&operation, &schema, rep) require.False(t, rep.HasErrors(), "failed to normalize variables") @@ -330,11 +330,11 @@ func TestArgumentMapping(t *testing.T) { func TestNewArguments_NilMapping(t *testing.T) { // Test that nil mapping returns empty Arguments result := NewArguments(nil, nil) - assert.Nil(t, result.Get("user", "id")) + assert.Nil(t, result.Get("query.user.id")) } func TestNewArguments_EmptyMapping(t *testing.T) { // Test that empty mapping returns empty Arguments result := NewArguments(astnormalization.FieldArgumentMapping{}, nil) - assert.Nil(t, result.Get("user", "id")) + assert.Nil(t, result.Get("query.user.id")) } diff --git a/router/go.mod b/router/go.mod index e330485902..eccad2bc88 100644 --- a/router/go.mod +++ b/router/go.mod @@ -31,7 +31,7 @@ require ( github.com/tidwall/gjson v1.18.0 github.com/tidwall/sjson v1.2.5 github.com/twmb/franz-go v1.16.1 - github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241 + github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.242.0.20260202140551-546a496bbb69 // Do not upgrade, it renames attributes we rely on go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.58.0 go.opentelemetry.io/contrib/propagators/b3 v1.23.0 diff --git a/router/go.sum b/router/go.sum index f4d1a1ecc9..4f5fce810c 100644 --- a/router/go.sum +++ b/router/go.sum @@ -322,8 +322,8 @@ github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/ github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= github.com/wundergraph/astjson v0.0.0-20250106123708-be463c97e083 h1:8/D7f8gKxTBjW+SZK4mhxTTBVpxcqeBgWF1Rfmltbfk= github.com/wundergraph/astjson v0.0.0-20250106123708-be463c97e083/go.mod h1:eOTL6acwctsN4F3b7YE+eE2t8zcJ/doLm9sZzsxxxrE= -github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241 h1:ch/8hfDaw4oz1Cx3Wb+OUl4qiAo17OdGhYMdRYnX8Is= -github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241/go.mod h1:mX25ASEQiKamxaFSK6NZihh0oDCigIuzro30up4mFH4= +github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.242.0.20260202140551-546a496bbb69 h1:HFqBQSMA3VjRFy/LjtKFd+vmO6tvj6BLuotC+dugEqU= +github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.242.0.20260202140551-546a496bbb69/go.mod h1:mX25ASEQiKamxaFSK6NZihh0oDCigIuzro30up4mFH4= github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4= github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= github.com/yuin/gopher-lua v1.1.1 h1:kYKnWBjvbNP4XLT3+bPEwAXJx262OhaHDWDVOPjL46M= From 72b7375f0ad0a38e4921f4403b49fd4eda5d395c Mon Sep 17 00:00:00 2001 From: Dominik Korittki <23359034+dkorittki@users.noreply.github.com> Date: Mon, 2 Feb 2026 16:47:01 +0100 Subject: [PATCH 22/22] fix: prefix with mutation --- router-tests/modules/stream_publish_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/router-tests/modules/stream_publish_test.go b/router-tests/modules/stream_publish_test.go index 4e093a2f6b..af93b4c6f6 100644 --- a/router-tests/modules/stream_publish_test.go +++ b/router-tests/modules/stream_publish_test.go @@ -372,7 +372,7 @@ func TestPublishHook(t *testing.T) { Callback: func(ctx core.StreamPublishEventHandlerContext, events datasource.StreamEvents) (datasource.StreamEvents, error) { args := ctx.Operation().Arguments() if args != nil { - employeeIDArg := args.Get("subscription.updateEmployeeMyKafka.employeeID") + employeeIDArg := args.Get("mutation.updateEmployeeMyKafka.employeeID") if employeeIDArg != nil { capturedEmployeeID = employeeIDArg.GetInt() }