Skip to content

Commit 900d450

Browse files
fengyuwusongclaude
andauthored
fix: preserve nullable nested input presence in gRPC compiler (#1447)
## Summary - treat omitted and null nested input messages as absent during protobuf compilation - preserve explicit empty objects instead of conflating them with null - add a regression test covering omitted, null, empty-object and explicit-value cases ## Testing - go test ./v2/pkg/engine/datasource/grpc_datasource Refs wundergraph/cosmo#2650 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * More accurate detection of present vs absent nested inputs, stricter enforcement of required fields, and correct skipping of empty optional scalar values during gRPC datasource compilation. * **Tests** * Added tests exercising omitted, null, empty-object, and explicit nested input scenarios to validate compiler behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 11a28cf commit 900d450

2 files changed

Lines changed: 158 additions & 4 deletions

File tree

v2/pkg/engine/datasource/grpc_datasource/compiler.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,7 @@ func (p *RPCCompiler) buildProtoMessage(inputMessage Message, rpcMessage *RPCMes
10211021
fieldMsg protoref.Message
10221022
err error
10231023
)
1024+
fieldData := data.Get(rpcField.JSONPath)
10241025

10251026
switch {
10261027
case rpcField.IsListType:
@@ -1031,7 +1032,7 @@ func (p *RPCCompiler) buildProtoMessage(inputMessage Message, rpcMessage *RPCMes
10311032
// ListOfBoolean is_published = 1;
10321033
// ListOfListOfString related_topics = 2;
10331034
// }
1034-
if !data.Get(rpcField.JSONPath).Exists() {
1035+
if !isNonNullValue(fieldData) {
10351036
if !rpcField.Optional {
10361037
return nil, fmt.Errorf("field %s is required but has no value", rpcField.JSONPath)
10371038
}
@@ -1056,8 +1057,8 @@ func (p *RPCCompiler) buildProtoMessage(inputMessage Message, rpcMessage *RPCMes
10561057
// If the field is optional, we are handling a scalar value that is wrapped in a message
10571058
// as protobuf scalar types are not nullable.
10581059

1059-
if !data.Get(rpcField.JSONPath).Exists() {
1060-
// If we don't have a value for an optional field, we skip it to provide a null message.
1060+
if !isNonNullValue(fieldData) {
1061+
// If we don't have a concrete value for an optional field, leave it unset (absent).
10611062
continue
10621063
}
10631064

@@ -1072,7 +1073,15 @@ func (p *RPCCompiler) buildProtoMessage(inputMessage Message, rpcMessage *RPCMes
10721073
return nil, err
10731074
}
10741075
default:
1075-
fieldMsg, err = p.buildProtoMessage(p.doc.Messages[field.MessageRef], rpcField.Message, data.Get(rpcField.JSONPath))
1076+
if !isNonNullValue(fieldData) {
1077+
if !rpcField.Optional {
1078+
return nil, fmt.Errorf("field %s is required but has no value", rpcField.JSONPath)
1079+
}
1080+
1081+
continue
1082+
}
1083+
1084+
fieldMsg, err = p.buildProtoMessage(p.doc.Messages[field.MessageRef], rpcField.Message, fieldData)
10761085
if err != nil {
10771086
return nil, err
10781087
}
@@ -1103,6 +1112,10 @@ func (p *RPCCompiler) buildProtoMessage(inputMessage Message, rpcMessage *RPCMes
11031112
return message, nil
11041113
}
11051114

1115+
func isNonNullValue(data gjson.Result) bool {
1116+
return data.Exists() && data.Type != gjson.Null
1117+
}
1118+
11061119
// buildListMessage creates a new protobuf message, which reflects a wrapper type to work with a list in GraphQL.
11071120
// A list wrapper type has an inner message type, which contains a repeated field.
11081121
// We need this to make sure we can differentiate between a null list and an empty list, as repeated fields are not nullable.
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package grpcdatasource
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
"github.com/tidwall/gjson"
8+
"google.golang.org/protobuf/reflect/protoreflect"
9+
)
10+
11+
const protoSchemaWithOptionalNestedInputs = `
12+
syntax = "proto3";
13+
package rule.v1;
14+
15+
import "google/protobuf/wrappers.proto";
16+
17+
service RuleService {
18+
rpc UpdateRule(UpdateRuleRequest) returns (UpdateRuleResponse) {}
19+
}
20+
21+
message UpdateRuleRequest {
22+
ConditionsInput conditions = 1;
23+
google.protobuf.StringValue params = 2;
24+
}
25+
26+
message ConditionsInput {
27+
string key = 1;
28+
}
29+
30+
message UpdateRuleResponse {}
31+
`
32+
33+
func TestCompileOptionalNestedInputsTreatsNullAsAbsent(t *testing.T) {
34+
t.Parallel()
35+
36+
testCases := []struct {
37+
name string
38+
input string
39+
expectConditions bool
40+
expectConditionsKey bool
41+
expectParams bool
42+
}{
43+
{
44+
name: "omitted optional fields stay absent",
45+
input: `{}`,
46+
expectConditions: false,
47+
expectParams: false,
48+
},
49+
{
50+
name: "null optional fields stay absent",
51+
input: `{"conditions":null,"params":null}`,
52+
expectConditions: false,
53+
expectParams: false,
54+
},
55+
{
56+
name: "explicit empty object keeps nested message",
57+
input: `{"conditions":{}}`,
58+
expectConditions: true,
59+
expectConditionsKey: false,
60+
expectParams: false,
61+
},
62+
{
63+
name: "explicit nested value is preserved",
64+
input: `{"conditions":{"key":"status"},"params":"{\"value\":\"1212\"}"}`,
65+
expectConditions: true,
66+
expectConditionsKey: true,
67+
expectParams: true,
68+
},
69+
}
70+
71+
for _, tc := range testCases {
72+
t.Run(tc.name, func(t *testing.T) {
73+
t.Parallel()
74+
compiler, err := NewProtoCompiler(protoSchemaWithOptionalNestedInputs, nil)
75+
require.NoError(t, err)
76+
77+
inputMessage, ok := compiler.doc.MessageByName("UpdateRuleRequest")
78+
require.True(t, ok)
79+
80+
request, err := compiler.buildProtoMessage(inputMessage, optionalNestedInputsMessage(), gjson.Parse(tc.input))
81+
require.NoError(t, err)
82+
83+
conditionsDesc := request.Descriptor().Fields().ByName(protoreflect.Name("conditions"))
84+
require.NotNil(t, conditionsDesc)
85+
require.Equal(t, tc.expectConditions, request.Has(conditionsDesc))
86+
87+
if tc.expectConditions {
88+
conditions := request.Get(conditionsDesc).Message()
89+
keyDesc := conditions.Descriptor().Fields().ByName(protoreflect.Name("key"))
90+
require.NotNil(t, keyDesc)
91+
require.Equal(t, tc.expectConditionsKey, conditions.Has(keyDesc))
92+
if tc.expectConditionsKey {
93+
require.Equal(t, "status", conditions.Get(keyDesc).String())
94+
}
95+
}
96+
97+
paramsDesc := request.Descriptor().Fields().ByName(protoreflect.Name("params"))
98+
require.NotNil(t, paramsDesc)
99+
require.Equal(t, tc.expectParams, request.Has(paramsDesc))
100+
101+
if tc.expectParams {
102+
params := request.Get(paramsDesc).Message()
103+
valueDesc := params.Descriptor().Fields().ByName(protoreflect.Name("value"))
104+
require.NotNil(t, valueDesc)
105+
require.True(t, params.Has(valueDesc))
106+
require.Equal(t, `{"value":"1212"}`, params.Get(valueDesc).String())
107+
}
108+
})
109+
}
110+
}
111+
112+
func optionalNestedInputsMessage() *RPCMessage {
113+
return &RPCMessage{
114+
Name: "UpdateRuleRequest",
115+
Fields: RPCFields{
116+
{
117+
Name: "conditions",
118+
ProtoTypeName: DataTypeMessage,
119+
JSONPath: "conditions",
120+
Optional: true,
121+
Message: &RPCMessage{
122+
Name: "ConditionsInput",
123+
Fields: RPCFields{
124+
{
125+
Name: "key",
126+
ProtoTypeName: DataTypeString,
127+
JSONPath: "key",
128+
Optional: true,
129+
},
130+
},
131+
},
132+
},
133+
{
134+
Name: "params",
135+
ProtoTypeName: DataTypeString,
136+
JSONPath: "params",
137+
Optional: true,
138+
},
139+
},
140+
}
141+
}

0 commit comments

Comments
 (0)