From b68d0eae2e297b32ada0ecf65e34ac8b972b5960 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Thu, 19 Feb 2026 16:10:42 -0800 Subject: [PATCH] Add JSON field name resolution to planner PiperOrigin-RevId: 872610068 --- .../java/dev/cel/common/values/BUILD.bazel | 1 + .../common/values/ProtoCelValueConverter.java | 14 +++-- .../cel/common/values/ProtoMessageValue.java | 16 +++++- .../values/ProtoMessageValueProvider.java | 13 ++++- .../values/ProtoCelValueConverterTest.java | 5 +- .../common/values/ProtoMessageValueTest.java | 57 +++++++++++++------ .../cel/runtime/planner/PlannedProgram.java | 12 +++- .../cel/runtime/PlannerInterpreterTest.java | 6 -- .../runtime/planner/ProgramPlannerTest.java | 2 +- 9 files changed, 90 insertions(+), 36 deletions(-) diff --git a/common/src/main/java/dev/cel/common/values/BUILD.bazel b/common/src/main/java/dev/cel/common/values/BUILD.bazel index bacd9d1fc..53ffdda3d 100644 --- a/common/src/main/java/dev/cel/common/values/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/values/BUILD.bazel @@ -191,6 +191,7 @@ java_library( ":base_proto_cel_value_converter", ":values", "//:auto_value", + "//common:options", "//common/annotations", "//common/internal:cel_descriptor_pools", "//common/internal:dynamic_proto", diff --git a/common/src/main/java/dev/cel/common/values/ProtoCelValueConverter.java b/common/src/main/java/dev/cel/common/values/ProtoCelValueConverter.java index cf70f52e5..c7b829e13 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoCelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/ProtoCelValueConverter.java @@ -26,6 +26,7 @@ import com.google.protobuf.Message; import com.google.protobuf.MessageLiteOrBuilder; import com.google.protobuf.MessageOrBuilder; +import dev.cel.common.CelOptions; import dev.cel.common.annotations.Internal; import dev.cel.common.internal.CelDescriptorPool; import dev.cel.common.internal.DynamicProto; @@ -49,11 +50,12 @@ public final class ProtoCelValueConverter extends BaseProtoCelValueConverter { private final CelDescriptorPool celDescriptorPool; private final DynamicProto dynamicProto; + private final CelOptions celOptions; /** Constructs a new instance of ProtoCelValueConverter. */ public static ProtoCelValueConverter newInstance( - CelDescriptorPool celDescriptorPool, DynamicProto dynamicProto) { - return new ProtoCelValueConverter(celDescriptorPool, dynamicProto); + CelDescriptorPool celDescriptorPool, DynamicProto dynamicProto, CelOptions celOptions) { + return new ProtoCelValueConverter(celDescriptorPool, dynamicProto, celOptions); } @Override @@ -97,7 +99,8 @@ public Object toRuntimeValue(Object value) { WellKnownProto wellKnownProto = WellKnownProto.getByTypeName(message.getDescriptorForType().getFullName()).orElse(null); if (wellKnownProto == null) { - return ProtoMessageValue.create((Message) message, celDescriptorPool, this); + return ProtoMessageValue.create( + message, celDescriptorPool, this, celOptions.enableJsonFieldNames()); } return fromWellKnownProto(message, wellKnownProto); @@ -167,10 +170,13 @@ public Object fromProtoMessageFieldToCelValue(Message message, FieldDescriptor f return toRuntimeValue(result); } - private ProtoCelValueConverter(CelDescriptorPool celDescriptorPool, DynamicProto dynamicProto) { + private ProtoCelValueConverter( + CelDescriptorPool celDescriptorPool, DynamicProto dynamicProto, CelOptions celOptions) { Preconditions.checkNotNull(celDescriptorPool); Preconditions.checkNotNull(dynamicProto); + Preconditions.checkNotNull(celOptions); this.celDescriptorPool = celDescriptorPool; this.dynamicProto = dynamicProto; + this.celOptions = celOptions; } } diff --git a/common/src/main/java/dev/cel/common/values/ProtoMessageValue.java b/common/src/main/java/dev/cel/common/values/ProtoMessageValue.java index 3e809975e..e402bb429 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageValue.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageValue.java @@ -40,6 +40,8 @@ public abstract class ProtoMessageValue extends StructValue { abstract ProtoCelValueConverter protoCelValueConverter(); + abstract boolean enableJsonFieldNames(); + @Override public boolean isZeroValue() { return value().getDefaultInstanceForType().equals(value()); @@ -75,7 +77,8 @@ public Optional find(String field) { public static ProtoMessageValue create( Message value, CelDescriptorPool celDescriptorPool, - ProtoCelValueConverter protoCelValueConverter) { + ProtoCelValueConverter protoCelValueConverter, + boolean enableJsonFieldNames) { Preconditions.checkNotNull(value); Preconditions.checkNotNull(celDescriptorPool); Preconditions.checkNotNull(protoCelValueConverter); @@ -83,7 +86,8 @@ public static ProtoMessageValue create( value, StructTypeReference.create(value.getDescriptorForType().getFullName()), celDescriptorPool, - protoCelValueConverter); + protoCelValueConverter, + enableJsonFieldNames); } private FieldDescriptor findField( @@ -93,6 +97,14 @@ private FieldDescriptor findField( return fieldDescriptor; } + if (enableJsonFieldNames()) { + for (FieldDescriptor fd : descriptor.getFields()) { + if (fd.getJsonName().equals(fieldName)) { + return fd; + } + } + } + return celDescriptorPool .findExtensionDescriptor(descriptor, fieldName) .orElseThrow( diff --git a/common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java b/common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java index a05658c8f..b7895d845 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java @@ -38,6 +38,7 @@ public class ProtoMessageValueProvider implements CelValueProvider { private final ProtoAdapter protoAdapter; private final ProtoMessageFactory protoMessageFactory; private final ProtoCelValueConverter protoCelValueConverter; + private final CelOptions celOptions; @Override public CelValueConverter celValueConverter() { @@ -72,6 +73,14 @@ private FieldDescriptor findField(Descriptor descriptor, String fieldName) { return fieldDescriptor; } + if (celOptions.enableJsonFieldNames()) { + for (FieldDescriptor fd : descriptor.getFields()) { + if (fd.getJsonName().equals(fieldName)) { + return fd; + } + } + } + return protoMessageFactory .getDescriptorPool() .findExtensionDescriptor(descriptor, fieldName) @@ -91,7 +100,9 @@ public static ProtoMessageValueProvider newInstance( private ProtoMessageValueProvider(CelOptions celOptions, DynamicProto dynamicProto) { this.protoMessageFactory = dynamicProto.getProtoMessageFactory(); this.protoCelValueConverter = - ProtoCelValueConverter.newInstance(protoMessageFactory.getDescriptorPool(), dynamicProto); + ProtoCelValueConverter.newInstance( + protoMessageFactory.getDescriptorPool(), dynamicProto, celOptions); this.protoAdapter = new ProtoAdapter(dynamicProto, celOptions); + this.celOptions = celOptions; } } diff --git a/common/src/test/java/dev/cel/common/values/ProtoCelValueConverterTest.java b/common/src/test/java/dev/cel/common/values/ProtoCelValueConverterTest.java index 61a05cf89..a517931c2 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoCelValueConverterTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoCelValueConverterTest.java @@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; +import dev.cel.common.CelOptions; import dev.cel.common.internal.DefaultDescriptorPool; import dev.cel.common.internal.DefaultMessageFactory; import dev.cel.common.internal.DynamicProto; @@ -28,7 +29,9 @@ public class ProtoCelValueConverterTest { private static final ProtoCelValueConverter PROTO_CEL_VALUE_CONVERTER = ProtoCelValueConverter.newInstance( - DefaultDescriptorPool.INSTANCE, DynamicProto.create(DefaultMessageFactory.INSTANCE)); + DefaultDescriptorPool.INSTANCE, + DynamicProto.create(DefaultMessageFactory.INSTANCE), + CelOptions.DEFAULT); @Test public void unwrap_nullValue() { diff --git a/common/src/test/java/dev/cel/common/values/ProtoMessageValueTest.java b/common/src/test/java/dev/cel/common/values/ProtoMessageValueTest.java index ad61f0c4b..365dd32b4 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageValueTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageValueTest.java @@ -35,6 +35,7 @@ import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; import dev.cel.common.CelDescriptorUtil; +import dev.cel.common.CelOptions; import dev.cel.common.internal.CelDescriptorPool; import dev.cel.common.internal.DefaultDescriptorPool; import dev.cel.common.internal.DefaultMessageFactory; @@ -54,7 +55,9 @@ public final class ProtoMessageValueTest { private static final ProtoCelValueConverter PROTO_CEL_VALUE_CONVERTER = ProtoCelValueConverter.newInstance( - DefaultDescriptorPool.INSTANCE, DynamicProto.create(DefaultMessageFactory.INSTANCE)); + DefaultDescriptorPool.INSTANCE, + DynamicProto.create(DefaultMessageFactory.INSTANCE), + CelOptions.DEFAULT); @Test public void emptyProtoMessage() { @@ -62,7 +65,8 @@ public void emptyProtoMessage() { ProtoMessageValue.create( TestAllTypes.getDefaultInstance(), DefaultDescriptorPool.INSTANCE, - PROTO_CEL_VALUE_CONVERTER); + PROTO_CEL_VALUE_CONVERTER, + /* enableJsonFieldNames= */ false); assertThat(protoMessageValue.value()).isEqualTo(TestAllTypes.getDefaultInstance()); assertThat(protoMessageValue.isZeroValue()).isTrue(); @@ -74,7 +78,7 @@ public void constructProtoMessage() { TestAllTypes.newBuilder().setSingleBool(true).setSingleInt64(5L).build(); ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.value()).isEqualTo(testAllTypes); assertThat(protoMessageValue.isZeroValue()).isFalse(); @@ -90,7 +94,7 @@ public void findField_fieldIsSet_fieldExists() { .build(); ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.find("single_bool")).isPresent(); assertThat(protoMessageValue.find("single_int64")).isPresent(); @@ -103,7 +107,8 @@ public void findField_fieldIsUnset_fieldDoesNotExist() { ProtoMessageValue.create( TestAllTypes.getDefaultInstance(), DefaultDescriptorPool.INSTANCE, - PROTO_CEL_VALUE_CONVERTER); + PROTO_CEL_VALUE_CONVERTER, + /* enableJsonFieldNames= */ false); assertThat(protoMessageValue.find("single_int32")).isEmpty(); assertThat(protoMessageValue.find("single_uint64")).isEmpty(); @@ -116,7 +121,8 @@ public void findField_fieldIsUndeclared_throwsException() { ProtoMessageValue.create( TestAllTypes.getDefaultInstance(), DefaultDescriptorPool.INSTANCE, - PROTO_CEL_VALUE_CONVERTER); + PROTO_CEL_VALUE_CONVERTER, + /* enableJsonFieldNames= */ false); IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> protoMessageValue.select("bogus")); @@ -137,7 +143,7 @@ public void findField_extensionField_success() { TestAllTypes.newBuilder().setExtension(TestAllTypesExtensions.int32Ext, 1).build(); ProtoMessageValue protoMessageValue = - ProtoMessageValue.create(proto2Message, descriptorPool, PROTO_CEL_VALUE_CONVERTER); + ProtoMessageValue.create(proto2Message, descriptorPool, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.find("cel.expr.conformance.proto2.int32_ext")).isPresent(); } @@ -149,7 +155,7 @@ public void findField_extensionField_throwsWhenDescriptorMissing() { ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - proto2Message, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + proto2Message, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); IllegalArgumentException exception = assertThrows( @@ -193,7 +199,8 @@ private enum SelectFieldTestCase { ProtoMessageValue.create( NestedMessage.getDefaultInstance(), DefaultDescriptorPool.INSTANCE, - PROTO_CEL_VALUE_CONVERTER)), + PROTO_CEL_VALUE_CONVERTER, + /* enableJsonFieldNames= */ false)), NESTED_ENUM("standalone_enum", 1L); private final String fieldName; @@ -239,7 +246,7 @@ public void selectField_success(@TestParameter SelectFieldTestCase testCase) { ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.select(testCase.fieldName)).isEqualTo(testCase.value); } @@ -253,7 +260,8 @@ public void selectField_dynamicMessage_success() { ProtoMessageValue.create( DynamicMessage.newBuilder(testAllTypes).build(), DefaultDescriptorPool.INSTANCE, - PROTO_CEL_VALUE_CONVERTER); + PROTO_CEL_VALUE_CONVERTER, + /* enableJsonFieldNames= */ false); assertThat(protoMessageValue.select("single_int32_wrapper")).isEqualTo(5); } @@ -269,7 +277,7 @@ public void selectField_timestampNanosOutOfRange_success(int nanos) { ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.select("single_timestamp")) .isEqualTo(Instant.ofEpochSecond(0, nanos)); @@ -289,7 +297,7 @@ public void selectField_durationOutOfRange_success(int seconds, int nanos) { ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.select("single_duration")) .isEqualTo(Duration.ofSeconds(seconds, nanos)); @@ -334,7 +342,7 @@ public void selectField_jsonValue(@TestParameter SelectFieldJsonValueTestCase te ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.select("single_value")).isEqualTo(testCase.value); } @@ -351,7 +359,7 @@ public void selectField_jsonStruct() { ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.select("single_struct")).isEqualTo(ImmutableMap.of("a", false)); } @@ -368,7 +376,7 @@ public void selectField_jsonList() { ProtoMessageValue protoMessageValue = ProtoMessageValue.create( - testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, false); assertThat(protoMessageValue.select("list_value")).isEqualTo(ImmutableList.of(false)); } @@ -379,7 +387,8 @@ public void selectField_wrapperFieldUnset_returnsNull() { ProtoMessageValue.create( TestAllTypes.getDefaultInstance(), DefaultDescriptorPool.INSTANCE, - PROTO_CEL_VALUE_CONVERTER); + PROTO_CEL_VALUE_CONVERTER, + /* enableJsonFieldNames= */ false); assertThat(protoMessageValue.select("single_int64_wrapper")).isEqualTo(NullValue.NULL_VALUE); } @@ -390,9 +399,21 @@ public void celTypeTest() { ProtoMessageValue.create( TestAllTypes.getDefaultInstance(), DefaultDescriptorPool.INSTANCE, - PROTO_CEL_VALUE_CONVERTER); + PROTO_CEL_VALUE_CONVERTER, + /* enableJsonFieldNames= */ false); assertThat(protoMessageValue.celType()) .isEqualTo(StructTypeReference.create(TestAllTypes.getDescriptor().getFullName())); } + + @Test + public void findField_jsonName_success() { + TestAllTypes testAllTypes = TestAllTypes.newBuilder().setSingleInt32(42).build(); + + ProtoMessageValue protoMessageValue = + ProtoMessageValue.create( + testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER, true); + + assertThat(protoMessageValue.find("singleInt32")).isPresent(); + } } diff --git a/runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java b/runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java index 9fcd05447..8b419cab2 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/PlannedProgram.java @@ -110,12 +110,18 @@ private CelEvaluationException newCelEvaluationException(long exprId, Exception // Use the localized expr ID (most specific error location) LocalizedEvaluationException localized = (LocalizedEvaluationException) e; exprId = localized.exprId(); - builder = - CelEvaluationExceptionBuilder.newBuilder((CelRuntimeException) localized.getCause()); + Throwable cause = localized.getCause(); + if (cause instanceof CelRuntimeException) { + builder = + CelEvaluationExceptionBuilder.newBuilder((CelRuntimeException) localized.getCause()); + } else { + builder = CelEvaluationExceptionBuilder.newBuilder(cause.getMessage()).setCause(cause); + } } else if (e instanceof CelRuntimeException) { builder = CelEvaluationExceptionBuilder.newBuilder((CelRuntimeException) e); } else { - // Unhandled function dispatch failures wraps the original exception with a descriptive message + // Unhandled function dispatch failures wraps the original exception with a descriptive + // message // (e.g: "Function foo failed with...") // We need to unwrap the cause here to preserve the original exception message and its cause. Throwable cause = e.getCause() != null ? e.getCause() : e; diff --git a/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java b/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java index 13a0f51a1..498d9f797 100644 --- a/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java +++ b/runtime/src/test/java/dev/cel/runtime/PlannerInterpreterTest.java @@ -45,10 +45,4 @@ public void unknownResultSet() { // TODO: Unknown support not implemented yet skipBaselineVerification(); } - - @Override - public void jsonFieldNames() throws Exception { - // TODO: Support JSON field names for planner - skipBaselineVerification(); - } } diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java index e8f5c6662..657fb5ab3 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java @@ -98,7 +98,7 @@ public final class ProgramPlannerTest { private static final CelValueProvider VALUE_PROVIDER = ProtoMessageValueProvider.newInstance(CEL_OPTIONS, DYNAMIC_PROTO); private static final CelValueConverter CEL_VALUE_CONVERTER = - ProtoCelValueConverter.newInstance(DESCRIPTOR_POOL, DYNAMIC_PROTO); + ProtoCelValueConverter.newInstance(DESCRIPTOR_POOL, DYNAMIC_PROTO, CelOptions.DEFAULT); private static final CelContainer CEL_CONTAINER = CelContainer.newBuilder() .setName("cel.expr.conformance.proto3")