diff --git a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java index f864d355c..694df2b0b 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java @@ -133,19 +133,23 @@ public final String toString() { @Override public Object select(String field) { - FieldLiteDescriptor fd = descriptor().findByFieldName(field); - if (fd == null) { - throw CelAttributeNotFoundException.of( - String.format("field '%s' is not declared in message '%s'", field, protoTypeName())); - } + FieldLiteDescriptor fd = findFieldDescriptor(field); Object fieldValue = readField(fd); return fieldValue != null ? fieldValue : protoLiteCelValueConverter().getDefaultCelValue(fd); } @Override public Optional find(String field) { - // Per SelectableValue#find, a field that doesn't exist is reported as absent. - return Optional.ofNullable(descriptor().findByFieldName(field)).map(this::readField); + return Optional.ofNullable(readField(findFieldDescriptor(field))); + } + + private FieldLiteDescriptor findFieldDescriptor(String field) { + FieldLiteDescriptor fd = descriptor().findByFieldName(field); + if (fd == null) { + throw CelAttributeNotFoundException.of( + String.format("field '%s' is not declared in message '%s'", field, protoTypeName())); + } + return fd; } @Override diff --git a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java index df5fd1df9..a5a7c1d47 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java @@ -368,32 +368,35 @@ public void select_undeclaredField_throwsCelAttributeNotFoundException() { + " 'cel.expr.conformance.proto3.TestAllTypes'"); } - @SuppressWarnings("ImmutableEnumChecker") // Test only - private enum FindFieldTestCase { - PRESENT("single_string", Optional.of("foo")), - ABSENT("single_int64", Optional.empty()), - UNDECLARED("undeclared_field", Optional.empty()); + @Test + public void find_undeclaredField_throwsCelAttributeNotFoundException() { + ProtoMessageLiteValue messageLiteValue = + ProtoMessageLiteValue.create( + TestAllTypes.getDefaultInstance(), + TEST_ALL_TYPES_DESCRIPTOR, + PROTO_LITE_CEL_VALUE_CONVERTER); - private final String fieldName; - private final Optional expected; + CelAttributeNotFoundException thrown = + assertThrows( + CelAttributeNotFoundException.class, () -> messageLiteValue.find("undeclared_field")); - FindFieldTestCase(String fieldName, Optional expected) { - this.fieldName = fieldName; - this.expected = expected; - } + assertThat(thrown) + .hasMessageThat() + .isEqualTo( + "field 'undeclared_field' is not declared in message" + + " 'cel.expr.conformance.proto3.TestAllTypes'"); } @Test - public void find_returnsValueOnlyIfFieldIsPresent(@TestParameter FindFieldTestCase testCase) { + public void find_returnsValueOnlyIfFieldIsPresent() { ProtoMessageLiteValue messageLiteValue = ProtoMessageLiteValue.create( TestAllTypes.newBuilder().setSingleString("foo").build(), TEST_ALL_TYPES_DESCRIPTOR, PROTO_LITE_CEL_VALUE_CONVERTER); - Optional result = messageLiteValue.find(testCase.fieldName); - - assertThat(result).isEqualTo(testCase.expected); + assertThat(messageLiteValue.find("single_string")).hasValue("foo"); + assertThat(messageLiteValue.find("single_int64")).isEmpty(); } @SuppressWarnings("ImmutableEnumChecker") // Test only