Skip to content

Commit b92f1f6

Browse files
l46kokcopybara-github
l46kok
authored and
copybara-github
committed
JSON field name resolution fix for shadowed cases
PiperOrigin-RevId: 890106179
1 parent 690d082 commit b92f1f6

File tree

13 files changed

+208
-52
lines changed

13 files changed

+208
-52
lines changed

‎bundle/src/test/java/dev/cel/bundle/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ java_library(
1717
deps = [
1818
"//:java_truth",
1919
"//bundle:cel",
20+
"//bundle:cel_experimental_factory",
2021
"//bundle:cel_impl",
2122
"//bundle:environment",
2223
"//bundle:environment_exception",
@@ -55,6 +56,7 @@ java_library(
5556
"//runtime:evaluation_listener",
5657
"//runtime:function_binding",
5758
"//runtime:unknown_attributes",
59+
"//testing/protos:single_file_extension_java_proto",
5860
"//testing/protos:single_file_java_proto",
5961
"@cel_spec//proto/cel/expr:checked_java_proto",
6062
"@cel_spec//proto/cel/expr:syntax_java_proto",

‎bundle/src/test/java/dev/cel/bundle/CelImplTest.java‎

Lines changed: 119 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
import dev.cel.expr.conformance.proto2.Proto2ExtensionScopedMessage;
9999
import dev.cel.expr.conformance.proto2.TestAllTypesExtensions;
100100
import dev.cel.expr.conformance.proto3.TestAllTypes;
101+
import dev.cel.extensions.CelExtensions;
101102
import dev.cel.parser.CelParserImpl;
102103
import dev.cel.parser.CelStandardMacro;
103104
import dev.cel.runtime.CelAttribute;
@@ -113,7 +114,8 @@
113114
import dev.cel.runtime.CelUnknownSet;
114115
import dev.cel.runtime.CelVariableResolver;
115116
import dev.cel.runtime.UnknownContext;
116-
import dev.cel.testing.testdata.SingleFileProto.SingleFile;
117+
import dev.cel.testing.testdata.SingleFile;
118+
import dev.cel.testing.testdata.SingleFileExtensionsProto;
117119
import dev.cel.testing.testdata.proto3.StandaloneGlobalEnum;
118120
import java.time.Instant;
119121
import java.util.ArrayList;
@@ -2142,20 +2144,90 @@ public void toBuilder_isImmutable() {
21422144
}
21432145

21442146
@Test
2145-
public void eval_withJsonFieldName() throws Exception {
2146-
Cel cel =
2147-
standardCelBuilderWithMacros()
2148-
.addVar("file", StructTypeReference.create(SingleFile.getDescriptor().getFullName()))
2149-
.addMessageTypes(SingleFile.getDescriptor())
2150-
.setOptions(CelOptions.current().enableJsonFieldNames(true).build())
2151-
.build();
2152-
CelAbstractSyntaxTree ast = cel.compile("file.camelCased").getAst();
2147+
public void eval_withJsonFieldName(@TestParameter RuntimeEnv runtimeEnv) throws Exception {
2148+
Cel cel = runtimeEnv.cel;
2149+
CelAbstractSyntaxTree ast =
2150+
cel.compile(
2151+
"file.int32_snake_case_json_name == 1 && "
2152+
+ "file.int64CamelCaseJsonName == 2 && "
2153+
+ "file.uint32DefaultJsonName == 3u && "
2154+
+ "file.`uint64-custom-json-name` == 4u && "
2155+
+ "file.single_string == 'shadows' && "
2156+
+ "file.singleString == 'shadowed'")
2157+
.getAst();
2158+
2159+
boolean result =
2160+
(boolean)
2161+
cel.createProgram(ast)
2162+
.eval(
2163+
ImmutableMap.of(
2164+
"file",
2165+
SingleFile.newBuilder()
2166+
.setInt32SnakeCaseJsonName(1)
2167+
.setInt64CamelCaseJsonName(2L)
2168+
.setUint32DefaultJsonName(3)
2169+
.setUint64CustomJsonName(4)
2170+
.setStringJsonNameShadows("shadows")
2171+
.setSingleString("shadowed")
2172+
.setExtension(SingleFileExtensionsProto.int64CamelCaseJsonName, 5L)
2173+
.build()));
21532174

2154-
Object result =
2155-
cel.createProgram(ast)
2156-
.eval(ImmutableMap.of("file", SingleFile.newBuilder().setSnakeCased("foo").build()));
2175+
assertThat(result).isTrue();
2176+
}
2177+
2178+
@Test
2179+
public void eval_withJsonFieldName_fieldsFallBack(@TestParameter RuntimeEnv runtimeEnv) throws Exception {
2180+
Cel cel = runtimeEnv.cel;
2181+
CelAbstractSyntaxTree ast =
2182+
cel.compile(
2183+
"dyn(file).int32_snake_case_json_name == 1 && "
2184+
+ "dyn(file).`uint64-custom-json-name` == 4u && "
2185+
+ "dyn(file).single_string == 'shadows' && "
2186+
+ "dyn(file).string_json_name_shadows == 'shadows' && "
2187+
+ "dyn(file).singleString == 'shadowed'")
2188+
.getAst();
2189+
2190+
boolean result =
2191+
(boolean)
2192+
cel.createProgram(ast)
2193+
.eval(
2194+
ImmutableMap.of(
2195+
"file",
2196+
SingleFile.newBuilder()
2197+
.setInt32SnakeCaseJsonName(1)
2198+
.setInt64CamelCaseJsonName(2L)
2199+
.setUint32DefaultJsonName(3)
2200+
.setUint64CustomJsonName(4)
2201+
.setStringJsonNameShadows("shadows")
2202+
.setSingleString("shadowed")
2203+
.build()));
21572204

2158-
assertThat(result).isEqualTo("foo");
2205+
assertThat(result).isTrue();
2206+
}
2207+
2208+
@Test
2209+
public void eval_withJsonFieldName_extensionFields(@TestParameter RuntimeEnv runtimeEnv) throws Exception {
2210+
Cel cel = runtimeEnv.cel;
2211+
CelAbstractSyntaxTree ast =
2212+
cel.compile(
2213+
"proto.getExt(file, dev.cel.testing.testdata.int64CamelCaseJsonName) == 5 &&"
2214+
+ " proto.getExt(file, dev.cel.testing.testdata.single_string) == 'foo'")
2215+
.getAst();
2216+
2217+
boolean result =
2218+
(boolean)
2219+
cel.createProgram(ast)
2220+
.eval(
2221+
ImmutableMap.of(
2222+
"file",
2223+
SingleFile.newBuilder()
2224+
.setInt64CamelCaseJsonName(2L)
2225+
.setExtension(SingleFileExtensionsProto.int64CamelCaseJsonName, 5L)
2226+
.setSingleString("This should not be used")
2227+
.setExtension(SingleFileExtensionsProto.singleString, "foo")
2228+
.build()));
2229+
2230+
assertThat(result).isTrue();
21592231
}
21602232

21612233
@Test
@@ -2171,7 +2243,7 @@ public void eval_withJsonFieldName_runtimeOptionDisabled_throws() throws Excepti
21712243
.addMessageTypes(SingleFile.getDescriptor())
21722244
.setOptions(CelOptions.current().enableJsonFieldNames(false).build())
21732245
.build();
2174-
CelAbstractSyntaxTree ast = celCompiler.compile("file.camelCased").getAst();
2246+
CelAbstractSyntaxTree ast = celCompiler.compile("file.int64CamelCaseJsonName").getAst();
21752247

21762248
CelEvaluationException e =
21772249
assertThrows(
@@ -2183,7 +2255,8 @@ public void eval_withJsonFieldName_runtimeOptionDisabled_throws() throws Excepti
21832255
assertThat(e)
21842256
.hasMessageThat()
21852257
.contains(
2186-
"field 'camelCased' is not declared in message 'dev.cel.testing.testdata.SingleFile");
2258+
"field 'int64CamelCaseJsonName' is not declared in message"
2259+
+ " 'dev.cel.testing.testdata.SingleFile");
21872260
}
21882261

21892262
@Test
@@ -2194,7 +2267,7 @@ public void compile_withJsonFieldName_astTagged() throws Exception {
21942267
.addMessageTypes(SingleFile.getDescriptor())
21952268
.setOptions(CelOptions.current().enableJsonFieldNames(true).build())
21962269
.build();
2197-
CelAbstractSyntaxTree ast = cel.compile("file.camelCased").getAst();
2270+
CelAbstractSyntaxTree ast = cel.compile("file.int64CamelCaseJsonName").getAst();
21982271

21992272
assertThat(ast.getSource().getExtensions())
22002273
.contains(
@@ -2243,4 +2316,34 @@ private static TypeProvider aliasingProvider(ImmutableMap<String, Type> typeAlia
22432316
}
22442317
};
22452318
}
2319+
2320+
private enum RuntimeEnv {
2321+
LEGACY(setupEnv(CelFactory.standardCelBuilder())),
2322+
PLANNER(setupEnv(CelExperimentalFactory.plannerCelBuilder()))
2323+
;
2324+
2325+
private final Cel cel;
2326+
2327+
private static Cel setupEnv(CelBuilder celBuilder) {
2328+
ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance();
2329+
SingleFileExtensionsProto.registerAllExtensions(extensionRegistry);
2330+
return celBuilder
2331+
.addVar("file", StructTypeReference.create(SingleFile.getDescriptor().getFullName()))
2332+
.addMessageTypes(SingleFile.getDescriptor())
2333+
.addFileTypes(SingleFileExtensionsProto.getDescriptor())
2334+
.addCompilerLibraries(CelExtensions.protos())
2335+
.setExtensionRegistry(extensionRegistry)
2336+
.setOptions(
2337+
CelOptions.current()
2338+
.enableJsonFieldNames(true)
2339+
.enableHeterogeneousNumericComparisons(true)
2340+
.enableQuotedIdentifierSyntax(true)
2341+
.build())
2342+
.build();
2343+
}
2344+
2345+
RuntimeEnv(Cel cel) {
2346+
this.cel = cel;
2347+
}
2348+
}
22462349
}

‎common/src/main/java/dev/cel/common/values/ProtoMessageValue.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,6 @@ public static ProtoMessageValue create(
9292

9393
private FieldDescriptor findField(
9494
CelDescriptorPool celDescriptorPool, Descriptor descriptor, String fieldName) {
95-
FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
96-
if (fieldDescriptor != null) {
97-
return fieldDescriptor;
98-
}
99-
10095
if (enableJsonFieldNames()) {
10196
for (FieldDescriptor fd : descriptor.getFields()) {
10297
if (fd.getJsonName().equals(fieldName)) {
@@ -105,6 +100,11 @@ private FieldDescriptor findField(
105100
}
106101
}
107102

103+
FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
104+
if (fieldDescriptor != null) {
105+
return fieldDescriptor;
106+
}
107+
108108
return celDescriptorPool
109109
.findExtensionDescriptor(descriptor, fieldName)
110110
.orElseThrow(

‎common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,6 @@ public Optional<Object> newValue(String structType, Map<String, Object> fields)
6868
}
6969

7070
private FieldDescriptor findField(Descriptor descriptor, String fieldName) {
71-
FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
72-
if (fieldDescriptor != null) {
73-
return fieldDescriptor;
74-
}
75-
7671
if (celOptions.enableJsonFieldNames()) {
7772
for (FieldDescriptor fd : descriptor.getFields()) {
7873
if (fd.getJsonName().equals(fieldName)) {
@@ -81,6 +76,11 @@ private FieldDescriptor findField(Descriptor descriptor, String fieldName) {
8176
}
8277
}
8378

79+
FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
80+
if (fieldDescriptor != null) {
81+
return fieldDescriptor;
82+
}
83+
8484
return protoMessageFactory
8585
.getDescriptorPool()
8686
.findExtensionDescriptor(descriptor, fieldName)

‎common/src/test/java/dev/cel/common/internal/DynamicProtoTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
import dev.cel.common.CelDescriptorUtil;
3838
import dev.cel.common.CelDescriptors;
3939
import dev.cel.testing.testdata.MultiFile;
40-
import dev.cel.testing.testdata.SingleFileProto.SingleFile;
40+
import dev.cel.testing.testdata.SingleFile;
4141
import java.io.IOException;
4242
import org.junit.Test;
4343
import org.junit.runner.RunWith;

‎common/src/test/java/dev/cel/common/types/ProtoMessageTypeProviderTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import dev.cel.common.types.StructType.Field;
2424
import dev.cel.expr.conformance.proto2.TestAllTypes;
2525
import dev.cel.expr.conformance.proto2.TestAllTypesExtensions;
26-
import dev.cel.testing.testdata.SingleFileProto.SingleFile;
26+
import dev.cel.testing.testdata.SingleFile;
2727
import java.util.Optional;
2828
import org.junit.Test;
2929
import org.junit.runner.RunWith;
@@ -269,8 +269,8 @@ public void findField_withJsonNameOption() {
269269
(ProtoMessageType) typeProvider.findType(SingleFile.getDescriptor().getFullName()).get();
270270

271271
// Note that these are the same fields, with json_name option set
272-
Optional<Field> snakeCasedField = msgType.findField("snake_cased");
273-
Optional<Field> jsonNameField = msgType.findField("camelCased");
272+
Optional<Field> snakeCasedField = msgType.findField("int64_camel_case_json_name");
273+
Optional<Field> jsonNameField = msgType.findField("int64CamelCaseJsonName");
274274

275275
assertThat(snakeCasedField).isEmpty();
276276
assertThat(jsonNameField).isPresent();

‎policy/src/test/java/dev/cel/policy/CelPolicyCompilerImplTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
import dev.cel.policy.PolicyTestHelper.TestYamlPolicy;
4646
import dev.cel.runtime.CelFunctionBinding;
4747
import dev.cel.runtime.CelLateFunctionBindings;
48-
import dev.cel.testing.testdata.SingleFileProto.SingleFile;
48+
import dev.cel.testing.testdata.SingleFile;
4949
import dev.cel.testing.testdata.proto3.StandaloneGlobalEnum;
5050
import java.io.IOException;
5151
import java.util.Map;

‎runtime/src/main/java/dev/cel/runtime/DescriptorMessageProvider.java‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -173,30 +173,28 @@ public Object hasField(Object message, String fieldName) {
173173
}
174174

175175
private FieldDescriptor findField(Descriptor descriptor, String fieldName) {
176-
FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
177-
if (fieldDescriptor == null) {
178-
Optional<FieldDescriptor> maybeFieldDescriptor =
179-
protoMessageFactory.getDescriptorPool().findExtensionDescriptor(descriptor, fieldName);
180-
if (maybeFieldDescriptor.isPresent()) {
181-
fieldDescriptor = maybeFieldDescriptor.get();
182-
}
183-
}
184-
185-
if (fieldDescriptor == null && celOptions.enableJsonFieldNames()) {
176+
if (celOptions.enableJsonFieldNames()) {
186177
for (FieldDescriptor fd : descriptor.getFields()) {
187178
if (fd.getJsonName().equals(fieldName)) {
188-
fieldDescriptor = fd;
189-
break;
179+
return fd;
190180
}
191181
}
192182
}
193183

194-
if (fieldDescriptor == null) {
195-
throw new IllegalArgumentException(
196-
String.format(
197-
"field '%s' is not declared in message '%s'", fieldName, descriptor.getFullName()));
184+
FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName);
185+
if (fieldDescriptor != null) {
186+
return fieldDescriptor;
187+
}
188+
fieldDescriptor =
189+
protoMessageFactory.getDescriptorPool().findExtensionDescriptor(descriptor, fieldName).orElse(null);
190+
if (fieldDescriptor != null) {
191+
return fieldDescriptor;
198192
}
199-
return fieldDescriptor;
193+
194+
195+
throw new IllegalArgumentException(
196+
String.format(
197+
"field '%s' is not declared in message '%s'", fieldName, descriptor.getFullName()));
200198
}
201199

202200
private static MessageOrBuilder assertFullProtoMessage(Object candidate, String fieldName) {

‎runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@
5959
import dev.cel.testing.testdata.MultiFile;
6060
import dev.cel.testing.testdata.MultiFileCelDescriptor;
6161
import dev.cel.testing.testdata.SimpleEnum;
62+
import dev.cel.testing.testdata.SingleFile;
6263
import dev.cel.testing.testdata.SingleFileCelDescriptor;
63-
import dev.cel.testing.testdata.SingleFileProto.SingleFile;
6464
import java.time.Duration;
6565
import java.time.Instant;
6666
import java.util.ArrayList;

‎testing/protos/BUILD.bazel‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ alias(
99
actual = "//testing/src/test/resources/protos:single_file_java_proto",
1010
)
1111

12+
alias(
13+
name = "single_file_extension_java_proto",
14+
actual = "//testing/src/test/resources/protos:single_file_extension_java_proto",
15+
)
16+
1217
alias(
1318
name = "multi_file_java_proto",
1419
actual = "//testing/src/test/resources/protos:multi_file_java_proto",

0 commit comments

Comments
 (0)