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 dc03d069c..766e22309 100644 --- a/common/src/main/java/dev/cel/common/values/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/values/BUILD.bazel @@ -24,6 +24,7 @@ CEL_VALUES_SOURCES = [ "NullValue.java", "OpaqueValue.java", "OptionalValue.java", + "SelectableValue.java", "StringValue.java", "StructValue.java", "TimestampValue.java", @@ -72,7 +73,9 @@ java_library( ":cel_byte_string", ":cel_value", "//:auto_value", + "//common:error_codes", "//common:options", + "//common:runtime_exception", "//common/annotations", "//common/types", "//common/types:type_providers", diff --git a/common/src/main/java/dev/cel/common/values/MapValue.java b/common/src/main/java/dev/cel/common/values/MapValue.java index 84a913dc2..ff1bb3f56 100644 --- a/common/src/main/java/dev/cel/common/values/MapValue.java +++ b/common/src/main/java/dev/cel/common/values/MapValue.java @@ -17,10 +17,13 @@ import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.errorprone.annotations.DoNotCall; import com.google.errorprone.annotations.Immutable; +import dev.cel.common.CelErrorCode; +import dev.cel.common.CelRuntimeException; import dev.cel.common.types.CelType; import dev.cel.common.types.MapType; import dev.cel.common.types.SimpleType; import java.util.Map; +import java.util.Optional; import java.util.function.BiFunction; import java.util.function.Function; import org.jspecify.nullness.Nullable; @@ -33,8 +36,8 @@ */ @Immutable(containerOf = {"K", "V"}) public abstract class MapValue extends CelValue - implements Map { - + implements Map, SelectableValue { + private static final MapType MAP_TYPE = MapType.create(SimpleType.DYN, SimpleType.DYN); @Override @@ -48,24 +51,30 @@ public boolean isZeroValue() { @Override @SuppressWarnings("unchecked") public V get(Object key) { - return get((K) key); + return select((K) key); } - public V get(K key) { - if (!has(key)) { - throw new IllegalArgumentException( - String.format("key '%s' is not present in map.", key.value())); - } - return value().get(key); + @Override + @SuppressWarnings("unchecked") + public V select(K field) { + return (V) + find(field) + .orElseThrow( + () -> + new CelRuntimeException( + new IllegalArgumentException( + String.format("key '%s' is not present in map.", field.value())), + CelErrorCode.ATTRIBUTE_NOT_FOUND)); } @Override - public CelType celType() { - return MAP_TYPE; + public Optional find(K field) { + return value().containsKey(field) ? Optional.of(value().get(field)) : Optional.empty(); } - public boolean has(K key) { - return value().containsKey(key); + @Override + public CelType celType() { + return MAP_TYPE; } /** diff --git a/common/src/main/java/dev/cel/common/values/OptionalValue.java b/common/src/main/java/dev/cel/common/values/OptionalValue.java index cf6247a99..45270ebb2 100644 --- a/common/src/main/java/dev/cel/common/values/OptionalValue.java +++ b/common/src/main/java/dev/cel/common/values/OptionalValue.java @@ -20,6 +20,7 @@ import dev.cel.common.types.OptionalType; import dev.cel.common.types.SimpleType; import java.util.NoSuchElementException; +import java.util.Optional; import org.jspecify.nullness.Nullable; /** @@ -29,7 +30,8 @@ */ @AutoValue @Immutable(containerOf = "E") -public abstract class OptionalValue extends CelValue { +public abstract class OptionalValue extends CelValue + implements SelectableValue { private static final OptionalType OPTIONAL_TYPE = OptionalType.create(SimpleType.DYN); /** Sentinel value representing an empty optional ('optional.none()' in CEL) */ @@ -65,51 +67,21 @@ public OptionalType celType() { *
  • map[?key] -> key in map ? optional{map[key]} : optional.none() * */ - @SuppressWarnings("unchecked") - public OptionalValue select(CelValue field) { - if (isZeroValue()) { - return EMPTY; - } - - CelValue celValue = value(); - if (celValue instanceof MapValue) { - MapValue mapValue = (MapValue) celValue; - if (!mapValue.has(field)) { - return EMPTY; - } - - return OptionalValue.create(mapValue.get(field)); - } else if (celValue instanceof StructValue) { - StructValue structValue = (StructValue) celValue; - StringValue stringField = (StringValue) field; - if (!structValue.hasField(stringField.value())) { - return EMPTY; - } - - return OptionalValue.create(structValue.select(stringField.value())); - } - - throw new UnsupportedOperationException("Unsupported select on: " + celValue); + @Override + public CelValue select(CelValue field) { + return find(field).orElse(EMPTY); } - /** Presence test with optional semantics on maps and structs. */ - @SuppressWarnings("unchecked") // Unchecked cast of MapValue flagged due to type erasure. - public boolean hasField(CelValue field) { + @Override + @SuppressWarnings("unchecked") + public Optional find(CelValue field) { if (isZeroValue()) { - return false; - } - - CelValue celValue = value(); - if (celValue instanceof MapValue) { - MapValue mapValue = (MapValue) celValue; - return mapValue.has(field); - } else if (celValue instanceof StructValue) { - StructValue structValue = (StructValue) celValue; - StringValue stringField = (StringValue) field; - return structValue.hasField(stringField.value()); + return Optional.empty(); } - throw new UnsupportedOperationException("Unsupported presence test on: " + celValue); + SelectableValue selectableValue = (SelectableValue) value(); + Optional selectedField = selectableValue.find(field); + return selectedField.map(OptionalValue::create); } public static OptionalValue create(E value) { 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 a1df28f8d..f9abdbea1 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageValue.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageValue.java @@ -23,11 +23,12 @@ import dev.cel.common.internal.CelDescriptorPool; import dev.cel.common.types.CelType; import dev.cel.common.types.StructTypeReference; +import java.util.Optional; /** ProtoMessageValue is a struct value with protobuf support. */ @AutoValue @Immutable -public abstract class ProtoMessageValue extends StructValue { +public abstract class ProtoMessageValue extends StructValue { @Override public abstract Message value(); @@ -45,23 +46,30 @@ public boolean isZeroValue() { } @Override - public boolean hasField(String fieldName) { + public CelValue select(StringValue field) { FieldDescriptor fieldDescriptor = - findField(celDescriptorPool(), value().getDescriptorForType(), fieldName); + findField(celDescriptorPool(), value().getDescriptorForType(), field.value()); - if (fieldDescriptor.isRepeated()) { - return value().getRepeatedFieldCount(fieldDescriptor) > 0; - } - - return value().hasField(fieldDescriptor); + return protoCelValueConverter().fromProtoMessageFieldToCelValue(value(), fieldDescriptor); } @Override - public CelValue select(String fieldName) { + public Optional find(StringValue field) { FieldDescriptor fieldDescriptor = - findField(celDescriptorPool(), value().getDescriptorForType(), fieldName); + findField(celDescriptorPool(), value().getDescriptorForType(), field.value()); - return protoCelValueConverter().fromProtoMessageFieldToCelValue(value(), fieldDescriptor); + // Selecting a field on a protobuf message yields a default value even if the field is not + // declared. Therefore, we must exhaustively test whether they are actually declared. + if (fieldDescriptor.isRepeated()) { + if (value().getRepeatedFieldCount(fieldDescriptor) == 0) { + return Optional.empty(); + } + } else if (!value().hasField(fieldDescriptor)) { + return Optional.empty(); + } + + return Optional.of( + protoCelValueConverter().fromProtoMessageFieldToCelValue(value(), fieldDescriptor)); } public static ProtoMessageValue create( diff --git a/common/src/main/java/dev/cel/common/values/SelectableValue.java b/common/src/main/java/dev/cel/common/values/SelectableValue.java new file mode 100644 index 000000000..5fa0a8939 --- /dev/null +++ b/common/src/main/java/dev/cel/common/values/SelectableValue.java @@ -0,0 +1,37 @@ +// Copyright 2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dev.cel.common.values; + +import java.util.Optional; + +/** + * SelectableValue is an interface for representing a value that supports field selection and + * presence tests. Few examples are structs, protobuf messages, maps and optional values. + */ +public interface SelectableValue { + + /** + * Performs field selection. The behavior depends on the concrete implementation of the value + * being selected. For structs and maps, this must throw an exception if the field does not exist. + * For optional values, this will return an {@code optional.none()}. + */ + CelValue select(T field); + + /** + * Finds the field. This will return an {@link Optional#empty()} if the field does not exist. This + * can be used for presence testing. + */ + Optional find(T field); +} diff --git a/common/src/main/java/dev/cel/common/values/StructValue.java b/common/src/main/java/dev/cel/common/values/StructValue.java index 058c7e836..8a2351ded 100644 --- a/common/src/main/java/dev/cel/common/values/StructValue.java +++ b/common/src/main/java/dev/cel/common/values/StructValue.java @@ -22,22 +22,11 @@ *

    Users may extend from this class to provide a custom struct that CEL can understand (ex: * POJOs). Custom struct implementations must provide all functionalities denoted in the CEL * specification, such as field selection, presence testing and new object creation. + * + *

    For an expression `e` selecting a field `f`, `e.f` must throw an exception if `f` does not + * exist in the struct (i.e: hasField returns false). If the field exists but is not set, the + * implementation should return an appropriate default value based on the struct's semantics. */ @Immutable -public abstract class StructValue extends CelValue { - - /** - * Performs field selection. For an expression `e` selecting a field `f`, `e.f` must throw an - * exception if `f` does not exist in the struct (i.e: hasField returns false). If the field - * exists but is not set, the implementation should return an appropriate default value based on - * the struct's semantics. - */ - public abstract CelValue select(String fieldName); - - /** - * Performs presence test on a field. - * - * @return true iff the field exists in the struct. - */ - public abstract boolean hasField(String fieldName); -} +public abstract class StructValue extends CelValue + implements SelectableValue {} diff --git a/common/src/test/java/dev/cel/common/values/ImmutableMapValueTest.java b/common/src/test/java/dev/cel/common/values/ImmutableMapValueTest.java index fe3c11381..ee91712ec 100644 --- a/common/src/test/java/dev/cel/common/values/ImmutableMapValueTest.java +++ b/common/src/test/java/dev/cel/common/values/ImmutableMapValueTest.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableMap; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; +import dev.cel.common.CelRuntimeException; import dev.cel.common.types.MapType; import dev.cel.common.types.SimpleType; import org.junit.Test; @@ -65,21 +66,21 @@ public void get_nonExistentKey_throws() { ImmutableMapValue mapValue = ImmutableMapValue.create(ImmutableMap.of(one, hello)); - IllegalArgumentException exception = - assertThrows(IllegalArgumentException.class, () -> mapValue.get(IntValue.create(100L))); - assertThat(exception).hasMessageThat().isEqualTo("key '100' is not present in map."); + CelRuntimeException exception = + assertThrows(CelRuntimeException.class, () -> mapValue.get(IntValue.create(100L))); + assertThat(exception).hasMessageThat().contains("key '100' is not present in map."); } @Test @TestParameters("{key: 1, expectedResult: true}") @TestParameters("{key: 100, expectedResult: false}") - public void has_success(long key, boolean expectedResult) { + public void find_success(long key, boolean expectedResult) { IntValue one = IntValue.create(1L); StringValue hello = StringValue.create("hello"); ImmutableMapValue mapValue = ImmutableMapValue.create(ImmutableMap.of(one, hello)); - assertThat(mapValue.has(IntValue.create(key))).isEqualTo(expectedResult); + assertThat(mapValue.find(IntValue.create(key)).isPresent()).isEqualTo(expectedResult); } @Test diff --git a/common/src/test/java/dev/cel/common/values/OptionalValueTest.java b/common/src/test/java/dev/cel/common/values/OptionalValueTest.java index 26af72bc3..953513100 100644 --- a/common/src/test/java/dev/cel/common/values/OptionalValueTest.java +++ b/common/src/test/java/dev/cel/common/values/OptionalValueTest.java @@ -15,6 +15,7 @@ package dev.cel.common.values; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableMap; @@ -25,6 +26,7 @@ import dev.cel.common.types.SimpleType; import dev.cel.common.types.StructTypeReference; import java.util.NoSuchElementException; +import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,6 +43,14 @@ public void emptyOptional() { assertThat(exception).hasMessageThat().isEqualTo("No value present"); } + @Test + public void optionalValue_selectEmpty() { + CelValue optionalValue = OptionalValue.EMPTY.select(StringValue.create("bogus")); + + assertThat(optionalValue).isEqualTo(OptionalValue.EMPTY); + assertThat(optionalValue.isZeroValue()).isTrue(); + } + @Test public void optionalValue_construct() { OptionalValue optionalValue = OptionalValue.create(IntValue.create(1L)); @@ -94,15 +104,10 @@ public void optSelectField_struct_returnsEmpty() { .isEqualTo(OptionalValue.EMPTY); } - @Test - public void hasField_onEmptyOptional() { - assertThat(OptionalValue.EMPTY.hasField(StringValue.create("bogus"))).isFalse(); - } - @Test @TestParameters("{key: 1, expectedResult: true}") @TestParameters("{key: 100, expectedResult: false}") - public void hasField_map_success(long key, boolean expectedResult) { + public void findField_map_success(long key, boolean expectedResult) { IntValue one = IntValue.create(1L); StringValue hello = StringValue.create("hello"); ImmutableMapValue mapValue = @@ -110,21 +115,27 @@ public void hasField_map_success(long key, boolean expectedResult) { OptionalValue> optionalValueContainingMap = OptionalValue.create(mapValue); - assertThat(optionalValueContainingMap.hasField(IntValue.create(key))).isEqualTo(expectedResult); + assertThat(optionalValueContainingMap.find(IntValue.create(key)).isPresent()) + .isEqualTo(expectedResult); } @Test @TestParameters("{field: 'data', expectedResult: true}") @TestParameters("{field: 'bogus', expectedResult: false}") - public void hasField_struct_success(String field, boolean expectedResult) { + public void findField_struct_success(String field, boolean expectedResult) { CelCustomStruct celCustomStruct = new CelCustomStruct(5L); OptionalValue optionalValueContainingStruct = OptionalValue.create(celCustomStruct); - assertThat(optionalValueContainingStruct.hasField(StringValue.create(field))) + assertThat(optionalValueContainingStruct.find(StringValue.create(field)).isPresent()) .isEqualTo(expectedResult); } + @Test + public void findField_onEmptyOptional() { + assertThat(OptionalValue.EMPTY.find(StringValue.create("bogus"))).isEmpty(); + } + @Test public void create_nullValue_throws() { assertThrows(NullPointerException.class, () -> OptionalValue.create(null)); @@ -138,7 +149,7 @@ public void celTypeTest() { } @SuppressWarnings("Immutable") // Test only - private static class CelCustomStruct extends StructValue { + private static class CelCustomStruct extends StructValue { private final long data; @Override @@ -157,17 +168,17 @@ public CelType celType() { } @Override - public CelValue select(String fieldName) { - if (fieldName.equals("data")) { - return IntValue.create(value()); - } - - throw new IllegalArgumentException("Invalid field name: " + fieldName); + public CelValue select(StringValue field) { + return find(field).get(); } @Override - public boolean hasField(String fieldName) { - return fieldName.equals("data"); + public Optional find(StringValue field) { + if (field.value().equals("data")) { + return Optional.of(IntValue.create(value())); + } + + return Optional.empty(); } private CelCustomStruct(long data) { diff --git a/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java b/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java index c34013c96..de08149f0 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java @@ -101,16 +101,23 @@ public void newValue_createProtoMessage_fieldsPopulated() { .get(); assertThat(protoMessageValue.isZeroValue()).isFalse(); - assertThat(protoMessageValue.select("single_int32")).isEqualTo(IntValue.create(1L)); - assertThat(protoMessageValue.select("single_int64")).isEqualTo(IntValue.create(2L)); - assertThat(protoMessageValue.select("single_uint32")).isEqualTo(UintValue.create(3L, false)); - assertThat(protoMessageValue.select("single_uint64")).isEqualTo(UintValue.create(4L, false)); - assertThat(protoMessageValue.select("single_double")).isEqualTo(DoubleValue.create(5.5d)); - assertThat(protoMessageValue.select("single_bool")).isEqualTo(BoolValue.create(true)); - assertThat(protoMessageValue.select("single_string")).isEqualTo(StringValue.create("hello")); - assertThat(protoMessageValue.select("single_timestamp")) + assertThat(protoMessageValue.select(StringValue.create("single_int32"))) + .isEqualTo(IntValue.create(1L)); + assertThat(protoMessageValue.select(StringValue.create("single_int64"))) + .isEqualTo(IntValue.create(2L)); + assertThat(protoMessageValue.select(StringValue.create("single_uint32"))) + .isEqualTo(UintValue.create(3L, false)); + assertThat(protoMessageValue.select(StringValue.create("single_uint64"))) + .isEqualTo(UintValue.create(4L, false)); + assertThat(protoMessageValue.select(StringValue.create("single_double"))) + .isEqualTo(DoubleValue.create(5.5d)); + assertThat(protoMessageValue.select(StringValue.create("single_bool"))) + .isEqualTo(BoolValue.create(true)); + assertThat(protoMessageValue.select(StringValue.create("single_string"))) + .isEqualTo(StringValue.create("hello")); + assertThat(protoMessageValue.select(StringValue.create("single_timestamp"))) .isEqualTo(TimestampValue.create(Instant.ofEpochSecond(50))); - assertThat(protoMessageValue.select("single_duration")) + assertThat(protoMessageValue.select(StringValue.create("single_duration"))) .isEqualTo(DurationValue.create(Duration.ofSeconds(100))); } @@ -130,9 +137,10 @@ public void newValue_createProtoMessage_unsignedLongFieldsPopulated() { .get(); assertThat(protoMessageValue.isZeroValue()).isFalse(); - assertThat(protoMessageValue.select("single_uint32").value()) + assertThat(protoMessageValue.select(StringValue.create("single_uint32")).value()) .isEqualTo(UnsignedLong.valueOf(3L)); - assertThat(protoMessageValue.select("single_uint64").value()).isEqualTo(UnsignedLong.MAX_VALUE); + assertThat(protoMessageValue.select(StringValue.create("single_uint64")).value()) + .isEqualTo(UnsignedLong.MAX_VALUE); } @Test @@ -163,13 +171,22 @@ public void newValue_createProtoMessage_wrappersPopulated() { .get(); assertThat(protoMessageValue.isZeroValue()).isFalse(); - assertThat(protoMessageValue.select("single_int32_wrapper").value()).isEqualTo(1L); - assertThat(protoMessageValue.select("single_int64_wrapper").value()).isEqualTo(2L); - assertThat(protoMessageValue.select("single_uint32_wrapper").value()).isEqualTo(3L); - assertThat(protoMessageValue.select("single_uint64_wrapper").value()).isEqualTo(4L); - assertThat(protoMessageValue.select("single_double_wrapper").value()).isEqualTo(5.5d); - assertThat(protoMessageValue.select("single_bool_wrapper").value()).isEqualTo(true); - assertThat(protoMessageValue.select("single_string_wrapper").value()).isEqualTo("hello"); + assertThat(protoMessageValue.select(StringValue.create("single_int32_wrapper")).value()) + .isEqualTo(1L); + assertThat(protoMessageValue.select(StringValue.create("single_int32_wrapper")).value()) + .isEqualTo(1L); + assertThat(protoMessageValue.select(StringValue.create("single_int64_wrapper")).value()) + .isEqualTo(2L); + assertThat(protoMessageValue.select(StringValue.create("single_uint32_wrapper")).value()) + .isEqualTo(3L); + assertThat(protoMessageValue.select(StringValue.create("single_uint64_wrapper")).value()) + .isEqualTo(4L); + assertThat(protoMessageValue.select(StringValue.create("single_double_wrapper")).value()) + .isEqualTo(5.5d); + assertThat(protoMessageValue.select(StringValue.create("single_bool_wrapper")).value()) + .isEqualTo(true); + assertThat(protoMessageValue.select(StringValue.create("single_string_wrapper")).value()) + .isEqualTo("hello"); } @Test @@ -186,7 +203,10 @@ public void newValue_createProtoMessage_extensionFieldsPopulated() { .get(); assertThat(protoMessageValue.isZeroValue()).isFalse(); - assertThat(protoMessageValue.select("dev.cel.testing.testdata.proto2.int32_ext").value()) + assertThat( + protoMessageValue + .select(StringValue.create("dev.cel.testing.testdata.proto2.int32_ext")) + .value()) .isEqualTo(1); } @@ -240,6 +260,7 @@ public void newValue_onCombinedProvider() { .get(); assertThat(protoMessageValue.isZeroValue()).isFalse(); - assertThat(protoMessageValue.select("single_int32")).isEqualTo(IntValue.create(1L)); + assertThat(protoMessageValue.select(StringValue.create("single_int32"))) + .isEqualTo(IntValue.create(1L)); } } 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 e8f03f95c..9b0f5b8ef 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageValueTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageValueTest.java @@ -15,6 +15,7 @@ package dev.cel.common.values; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; @@ -85,7 +86,7 @@ public void constructProtoMessage() { } @Test - public void hasField_fieldIsSet_success() { + public void findField_fieldIsSet_fieldExists() { TestAllTypes testAllTypes = TestAllTypes.newBuilder() .setSingleBool(true) @@ -96,26 +97,26 @@ public void hasField_fieldIsSet_success() { ProtoMessageValue.create( testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.hasField("single_bool")).isTrue(); - assertThat(protoMessageValue.hasField("single_int64")).isTrue(); - assertThat(protoMessageValue.hasField("repeated_int64")).isTrue(); + assertThat(protoMessageValue.find(StringValue.create("single_bool"))).isPresent(); + assertThat(protoMessageValue.find(StringValue.create("single_int64"))).isPresent(); + assertThat(protoMessageValue.find(StringValue.create("repeated_int64"))).isPresent(); } @Test - public void hasField_fieldIsUnset_success() { + public void findField_fieldIsUnset_fieldDoesNotExist() { ProtoMessageValue protoMessageValue = ProtoMessageValue.create( TestAllTypes.getDefaultInstance(), DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.hasField("single_int32")).isFalse(); - assertThat(protoMessageValue.hasField("single_uint64")).isFalse(); - assertThat(protoMessageValue.hasField("repeated_int32")).isFalse(); + assertThat(protoMessageValue.find(StringValue.create("single_int32"))).isEmpty(); + assertThat(protoMessageValue.find(StringValue.create("single_uint64"))).isEmpty(); + assertThat(protoMessageValue.find(StringValue.create("repeated_int32"))).isEmpty(); } @Test - public void hasField_fieldIsUndeclared_throwsException() { + public void findField_fieldIsUndeclared_throwsException() { ProtoMessageValue protoMessageValue = ProtoMessageValue.create( TestAllTypes.getDefaultInstance(), @@ -123,7 +124,9 @@ public void hasField_fieldIsUndeclared_throwsException() { PROTO_CEL_VALUE_CONVERTER); IllegalArgumentException exception = - assertThrows(IllegalArgumentException.class, () -> protoMessageValue.hasField("bogus")); + assertThrows( + IllegalArgumentException.class, + () -> protoMessageValue.select(StringValue.create("bogus"))); assertThat(exception) .hasMessageThat() .isEqualTo( @@ -132,7 +135,7 @@ public void hasField_fieldIsUndeclared_throwsException() { } @Test - public void hasField_extensionField_success() { + public void findField_extensionField_success() { CelDescriptorPool descriptorPool = DefaultDescriptorPool.create( CelDescriptorUtil.getAllDescriptorsFromFileDescriptor( @@ -148,11 +151,13 @@ public void hasField_extensionField_success() { ProtoMessageValue protoMessageValue = ProtoMessageValue.create(proto2Message, descriptorPool, protoCelValueConverter); - assertThat(protoMessageValue.hasField("dev.cel.testing.testdata.proto2.int32_ext")).isTrue(); + assertThat( + protoMessageValue.find(StringValue.create("dev.cel.testing.testdata.proto2.int32_ext"))) + .isPresent(); } @Test - public void hasField_extensionField_throwsWhenDescriptorMissing() { + public void findField_extensionField_throwsWhenDescriptorMissing() { Proto2Message proto2Message = Proto2Message.newBuilder().setExtension(MessagesProto2Extensions.int32Ext, 1).build(); @@ -163,7 +168,9 @@ public void hasField_extensionField_throwsWhenDescriptorMissing() { IllegalArgumentException exception = assertThrows( IllegalArgumentException.class, - () -> protoMessageValue.hasField("dev.cel.testing.testdata.proto2.int32_ext")); + () -> + protoMessageValue.select( + StringValue.create("dev.cel.testing.testdata.proto2.int32_ext"))); assertThat(exception) .hasMessageThat() .isEqualTo( @@ -253,7 +260,8 @@ public void selectField_success(@TestParameter SelectFieldTestCase testCase) { ProtoMessageValue.create( testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.select(testCase.fieldName)).isEqualTo(testCase.celValue); + assertThat(protoMessageValue.select(StringValue.create(testCase.fieldName))) + .isEqualTo(testCase.celValue); } @Test @@ -267,7 +275,8 @@ public void selectField_dynamicMessage_success() { DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.select("single_int32_wrapper")).isEqualTo(IntValue.create(5)); + assertThat(protoMessageValue.select(StringValue.create("single_int32_wrapper"))) + .isEqualTo(IntValue.create(5)); } @Test @@ -283,7 +292,7 @@ public void selectField_timestampNanosOutOfRange_success(int nanos) { ProtoMessageValue.create( testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.select("single_timestamp")) + assertThat(protoMessageValue.select(StringValue.create("single_timestamp"))) .isEqualTo(TimestampValue.create(Instant.ofEpochSecond(0, nanos))); } @@ -303,7 +312,7 @@ public void selectField_durationOutOfRange_success(int seconds, int nanos) { ProtoMessageValue.create( testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.select("single_duration")) + assertThat(protoMessageValue.select(StringValue.create("single_duration"))) .isEqualTo(DurationValue.create(Duration.ofSeconds(seconds, nanos))); } @@ -348,7 +357,8 @@ public void selectField_jsonValue(@TestParameter SelectFieldJsonValueTestCase te ProtoMessageValue.create( testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.select("single_value")).isEqualTo(testCase.celValue); + assertThat(protoMessageValue.select(StringValue.create("single_value"))) + .isEqualTo(testCase.celValue); } @Test @@ -365,7 +375,7 @@ public void selectField_jsonStruct() { ProtoMessageValue.create( testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.select("single_struct")) + assertThat(protoMessageValue.select(StringValue.create("single_struct"))) .isEqualTo( ImmutableMapValue.create( ImmutableMap.of(StringValue.create("a"), BoolValue.create(false)))); @@ -385,7 +395,7 @@ public void selectField_jsonList() { ProtoMessageValue.create( testAllTypes, DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.select("list_value")) + assertThat(protoMessageValue.select(StringValue.create("list_value"))) .isEqualTo(ImmutableListValue.create(ImmutableList.of(BoolValue.create(false)))); } @@ -397,7 +407,8 @@ public void selectField_wrapperFieldUnset_returnsNull() { DefaultDescriptorPool.INSTANCE, PROTO_CEL_VALUE_CONVERTER); - assertThat(protoMessageValue.select("single_int64_wrapper")).isEqualTo(NullValue.NULL_VALUE); + assertThat(protoMessageValue.select(StringValue.create("single_int64_wrapper"))) + .isEqualTo(NullValue.NULL_VALUE); } @Test diff --git a/common/src/test/java/dev/cel/common/values/StructValueTest.java b/common/src/test/java/dev/cel/common/values/StructValueTest.java index 863e6fafc..f2974992d 100644 --- a/common/src/test/java/dev/cel/common/values/StructValueTest.java +++ b/common/src/test/java/dev/cel/common/values/StructValueTest.java @@ -21,6 +21,7 @@ import com.google.testing.junit.testparameterinjector.TestParameters; import dev.cel.common.types.CelType; import dev.cel.common.types.StructTypeReference; +import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -50,7 +51,7 @@ public void selectField_success() { UserDefinedClass userDefinedPojo = new UserDefinedClass(5); CelCustomStruct celCustomStruct = new CelCustomStruct(userDefinedPojo); - assertThat(celCustomStruct.select("data")).isEqualTo(IntValue.create(5L)); + assertThat(celCustomStruct.select(StringValue.create("data"))).isEqualTo(IntValue.create(5L)); } @Test @@ -58,17 +59,19 @@ public void selectField_nonExistentField_throws() { UserDefinedClass userDefinedPojo = new UserDefinedClass(5); CelCustomStruct celCustomStruct = new CelCustomStruct(userDefinedPojo); - assertThrows(IllegalArgumentException.class, () -> celCustomStruct.select("bogus")); + assertThrows( + IllegalArgumentException.class, () -> celCustomStruct.select(StringValue.create("bogus"))); } @Test @TestParameters("{fieldName: 'data', expectedResult: true}") @TestParameters("{fieldName: 'bogus', expectedResult: false}") - public void hasField_success(String fieldName, boolean expectedResult) { + public void findField_success(String fieldName, boolean expectedResult) { UserDefinedClass userDefinedPojo = new UserDefinedClass(5); CelCustomStruct celCustomStruct = new CelCustomStruct(userDefinedPojo); - assertThat(celCustomStruct.hasField(fieldName)).isEqualTo(expectedResult); + assertThat(celCustomStruct.find(StringValue.create(fieldName)).isPresent()) + .isEqualTo(expectedResult); } @Test @@ -88,7 +91,7 @@ private UserDefinedClass(long data) { } @SuppressWarnings("Immutable") // Test only - private static class CelCustomStruct extends StructValue { + private static class CelCustomStruct extends StructValue { private final UserDefinedClass userDefinedClass; @Override @@ -107,17 +110,18 @@ public CelType celType() { } @Override - public CelValue select(String fieldName) { - if (fieldName.equals("data")) { - return IntValue.create(value().data); - } - - throw new IllegalArgumentException("Invalid field name: " + fieldName); + public CelValue select(StringValue field) { + return find(field) + .orElseThrow(() -> new IllegalArgumentException("Invalid field name: " + field)); } @Override - public boolean hasField(String fieldName) { - return fieldName.equals("data"); + public Optional find(StringValue field) { + if (field.value().equals("data")) { + return Optional.of(IntValue.create(value().data)); + } + + return Optional.empty(); } private CelCustomStruct(UserDefinedClass value) {