From 77d78530081736db644422aa4e7693c61e1346cb Mon Sep 17 00:00:00 2001 From: Tristan Swadell Date: Wed, 24 Jul 2024 15:03:19 -0700 Subject: [PATCH] Introduce ProtoUnsetFieldOptions to support unset handling like C++ PiperOrigin-RevId: 655712342 --- .../main/java/dev/cel/common/CelOptions.java | 26 ++++++++++++++- .../main/java/dev/cel/runtime/Activation.java | 8 +++++ .../java/dev/cel/runtime/ActivationTest.java | 33 +++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/common/src/main/java/dev/cel/common/CelOptions.java b/common/src/main/java/dev/cel/common/CelOptions.java index fb5ec6ad..c5288aa2 100644 --- a/common/src/main/java/dev/cel/common/CelOptions.java +++ b/common/src/main/java/dev/cel/common/CelOptions.java @@ -30,6 +30,18 @@ @Immutable public abstract class CelOptions { + /** + * ProtoUnsetFieldOptions describes how to handle Activation.fromProto() calls where proto message + * fields may be unset and should either be handled perhaps as absent or as the default proto + * value. + */ + public enum ProtoUnsetFieldOptions { + // Do not bind a field if it is unset. Repeated fields are bound as empty list. + SKIP, + // Bind the (proto api) default value for a field. + BIND_DEFAULT; + } + public static final CelOptions DEFAULT = current().build(); public static final CelOptions LEGACY = newBuilder().disableCelStandardEquality(true).build(); @@ -95,6 +107,8 @@ public abstract class CelOptions { public abstract boolean unwrapWellKnownTypesOnFunctionDispatch(); + public abstract ProtoUnsetFieldOptions fromProtoUnsetFieldOption(); + public abstract Builder toBuilder(); public ImmutableSet toExprFeatures() { @@ -185,7 +199,8 @@ public static Builder newBuilder() { .enableUnknownTracking(false) .enableCelValue(false) .comprehensionMaxIterations(-1) - .unwrapWellKnownTypesOnFunctionDispatch(true); + .unwrapWellKnownTypesOnFunctionDispatch(true) + .fromProtoUnsetFieldOption(ProtoUnsetFieldOptions.BIND_DEFAULT); } /** @@ -479,6 +494,15 @@ public abstract static class Builder { @Deprecated public abstract Builder unwrapWellKnownTypesOnFunctionDispatch(boolean value); + /** + * Configure how unset proto fields are handled when evaluating over a protobuf message where + * fields are intended to be treated as top-level variables. Defaults to binding all fields to + * their default value if unset. + * + * @see ProtoUnsetFieldOptions + */ + public abstract Builder fromProtoUnsetFieldOption(ProtoUnsetFieldOptions value); + public abstract CelOptions build(); } } diff --git a/runtime/src/main/java/dev/cel/runtime/Activation.java b/runtime/src/main/java/dev/cel/runtime/Activation.java index 1894c16c..0ea70662 100644 --- a/runtime/src/main/java/dev/cel/runtime/Activation.java +++ b/runtime/src/main/java/dev/cel/runtime/Activation.java @@ -172,7 +172,15 @@ public static Activation fromProto(Message message, CelOptions celOptions) { new ProtoAdapter( DynamicProto.create(DefaultMessageFactory.INSTANCE), celOptions.enableUnsignedLongs()); + boolean skipUnsetFields = + celOptions.fromProtoUnsetFieldOption().equals(CelOptions.ProtoUnsetFieldOptions.SKIP); + for (FieldDescriptor field : message.getDescriptorForType().getFields()) { + // If skipping unset fields and the field is not repeated, then continue. + if (skipUnsetFields && !field.isRepeated() && !msgFieldValues.containsKey(field)) { + continue; + } + // Get the value of the field set on the message, if present, otherwise use reflection to // get the default value for the field using the FieldDescriptor. Object fieldValue = msgFieldValues.getOrDefault(field, message.getField(field)); diff --git a/runtime/src/test/java/dev/cel/runtime/ActivationTest.java b/runtime/src/test/java/dev/cel/runtime/ActivationTest.java index 5eb10737..04bc1cfb 100644 --- a/runtime/src/test/java/dev/cel/runtime/ActivationTest.java +++ b/runtime/src/test/java/dev/cel/runtime/ActivationTest.java @@ -35,6 +35,13 @@ public final class ActivationTest { private static final CelOptions TEST_OPTIONS = CelOptions.current().enableTimestampEpoch(true).enableUnsignedLongs(true).build(); + private static final CelOptions TEST_OPTIONS_SKIP_UNSET_FIELDS = + CelOptions.current() + .enableTimestampEpoch(true) + .enableUnsignedLongs(true) + .fromProtoUnsetFieldOption(CelOptions.ProtoUnsetFieldOptions.SKIP) + .build(); + @Test public void copyOf_success_withNullEntries() { Map map = new HashMap<>(); @@ -64,6 +71,13 @@ public void fromProto_unsetScalarField() { assertThat(activation.resolve("bb")).isEqualTo(0); } + @Test + public void fromProto_unsetScalarField_skipUnsetFields() { + Activation activation = + Activation.fromProto(NestedMessage.getDefaultInstance(), TEST_OPTIONS_SKIP_UNSET_FIELDS); + assertThat(activation.resolve("bb")).isNull(); + } + @Test public void fromProto_unsetAnyField() { // An unset Any field is the only field which cannot be accurately published into an Activation, @@ -102,6 +116,17 @@ public void fromProto_unsetRepeatedField() { assertThat((List) activation.resolve("repeated_nested_message")).isEmpty(); } + @Test + public void fromProto_unsetRepeatedField_skipUnsetFields() { + Activation activation = + Activation.fromProto(TestAllTypes.getDefaultInstance(), TEST_OPTIONS_SKIP_UNSET_FIELDS); + assertThat(activation.resolve("repeated_int64")).isInstanceOf(List.class); + assertThat((List) activation.resolve("repeated_int64")).isEmpty(); + + assertThat(activation.resolve("repeated_nested_message")).isInstanceOf(List.class); + assertThat((List) activation.resolve("repeated_nested_message")).isEmpty(); + } + @Test public void fromProto_unsetMapField() { Activation activation = Activation.fromProto(TestAllTypes.getDefaultInstance(), TEST_OPTIONS); @@ -109,6 +134,14 @@ public void fromProto_unsetMapField() { assertThat((Map) activation.resolve("map_int32_int64")).isEmpty(); } + @Test + public void fromProto_unsetMapField_skipUnsetFields() { + Activation activation = + Activation.fromProto(TestAllTypes.getDefaultInstance(), TEST_OPTIONS_SKIP_UNSET_FIELDS); + assertThat(activation.resolve("map_int32_int64")).isInstanceOf(Map.class); + assertThat((Map) activation.resolve("map_int32_int64")).isEmpty(); + } + @Test public void fromProto_unsignedLongField_unsignedResult() { Activation activation =