Skip to content

Commit

Permalink
Introduce ProtoUnsetFieldOptions to support unset handling like C++
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 655712342
  • Loading branch information
TristonianJones authored and copybara-github committed Jul 24, 2024
1 parent 882f631 commit 77d7853
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 1 deletion.
26 changes: 25 additions & 1 deletion common/src/main/java/dev/cel/common/CelOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -95,6 +107,8 @@ public abstract class CelOptions {

public abstract boolean unwrapWellKnownTypesOnFunctionDispatch();

public abstract ProtoUnsetFieldOptions fromProtoUnsetFieldOption();

public abstract Builder toBuilder();

public ImmutableSet<ExprFeatures> toExprFeatures() {
Expand Down Expand Up @@ -185,7 +199,8 @@ public static Builder newBuilder() {
.enableUnknownTracking(false)
.enableCelValue(false)
.comprehensionMaxIterations(-1)
.unwrapWellKnownTypesOnFunctionDispatch(true);
.unwrapWellKnownTypesOnFunctionDispatch(true)
.fromProtoUnsetFieldOption(ProtoUnsetFieldOptions.BIND_DEFAULT);
}

/**
Expand Down Expand Up @@ -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();
}
}
8 changes: 8 additions & 0 deletions runtime/src/main/java/dev/cel/runtime/Activation.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
33 changes: 33 additions & 0 deletions runtime/src/test/java/dev/cel/runtime/ActivationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> map = new HashMap<>();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -102,13 +116,32 @@ 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);
assertThat(activation.resolve("map_int32_int64")).isInstanceOf(Map.class);
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 =
Expand Down

0 comments on commit 77d7853

Please sign in to comment.