From 715d34bf43041f917e9500f62e6db144d48ebc50 Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Fri, 9 Oct 2026 17:57:51 -0700 Subject: [PATCH] Resolve known and unknown lite fields in ProtoLiteCelValueConverter PiperOrigin-RevId: 996865375 --- .../values/ProtoLiteCelValueConverter.java | 223 ++++++++++-------- .../common/values/ProtoMessageLiteValue.java | 81 +------ .../values/RawProtoMessageLiteValue.java | 23 +- .../ProtoLiteCelValueConverterTest.java | 142 +++++------ .../values/ProtoMessageLiteValueTest.java | 18 ++ .../values/RawProtoMessageLiteValueTest.java | 17 -- 6 files changed, 218 insertions(+), 286 deletions(-) diff --git a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java index 07709f603..481392466 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java @@ -29,7 +29,6 @@ import com.google.protobuf.MessageLite; import com.google.protobuf.WireFormat; import dev.cel.common.annotations.Internal; -import dev.cel.common.exceptions.CelAttributeNotFoundException; import dev.cel.common.internal.CelLiteDescriptorPool; import dev.cel.common.internal.WellKnownProto; import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor; @@ -164,7 +163,7 @@ Object getDefaultCelValue(FieldLiteDescriptor fieldDescriptor) { return toRuntimeValue(getDefaultValue(fieldDescriptor)); } - Optional findFieldDescriptor(String protoTypeName, int fieldNumber) { + private Optional findFieldDescriptor(String protoTypeName, int fieldNumber) { return descriptorPool .findDescriptor(protoTypeName) .flatMap(desc -> desc.findByFieldNumber(fieldNumber)); @@ -193,8 +192,7 @@ private static MessageLite mergeMessageLite( try { return builder.mergeFrom(bytes, ExtensionRegistryLite.getEmptyRegistry()).build(); } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + protoTypeName, e); + throw newDecodeException(protoTypeName, e); } } @@ -313,139 +311,155 @@ private Map.Entry readSingleMapEntry( * skipping every other field. Returns null if the field is absent (or a repeated field has only * empty packed records on the wire). */ - @Nullable Object readSingleField(ByteString bytes, FieldLiteDescriptor fieldDescriptor) - throws IOException { - return scanField(bytes, fieldDescriptor, null, null); + @Nullable Object readField( + ByteString bytes, String protoTypeName, FieldLiteDescriptor fieldDescriptor) { + return scanField(bytes, protoTypeName, fieldDescriptor, null, null); + } + + /** Reads {@code field}, by its own type information if {@code fieldDescriptor} is null. */ + private @Nullable Object readField( + ByteString bytes, + String protoTypeName, + @Nullable FieldLiteDescriptor fieldDescriptor, + SelectField field) { + if (fieldDescriptor != null) { + return readField(bytes, protoTypeName, fieldDescriptor); + } + SelectField.MapEntrySpec mapEntrySpec = field.mapEntrySpec(); + if (mapEntrySpec == null) { + return readField(bytes, protoTypeName, newFieldDescriptor(field)); + } + // The map entry type has no descriptor either, so its key and value are described by the spec. + return scanField( + bytes, + protoTypeName, + newFieldDescriptor(field), + newFieldDescriptor( + MAP_KEY_FIELD_NUMBER, + MAP_KEY_FIELD_NAME, + EncodingType.SINGULAR, + mapEntrySpec.keyTypeCode(), + /* protoTypeName= */ ""), + newFieldDescriptor( + MAP_VALUE_FIELD_NUMBER, + MAP_VALUE_FIELD_NAME, + EncodingType.SINGULAR, + mapEntrySpec.valueTypeCode(), + field.protoTypeName())); } /** Scans for {@code fieldDescriptor}; null map entry descriptors are looked up in the pool. */ private @Nullable Object scanField( ByteString bytes, + String protoTypeName, FieldLiteDescriptor fieldDescriptor, @Nullable FieldLiteDescriptor keyDescriptor, - @Nullable FieldLiteDescriptor valueDescriptor) - throws IOException { + @Nullable FieldLiteDescriptor valueDescriptor) { CodedInputStream inputStream = bytes.newCodedInput(); int targetFieldNumber = fieldDescriptor.getFieldNumber(); Object fieldValue = null; - for (int tag = inputStream.readTag(); tag != 0; tag = inputStream.readTag()) { - if (WireFormat.getTagFieldNumber(tag) != targetFieldNumber) { - skipWireField(tag, inputStream); - continue; - } - int tagWireType = WireFormat.getTagWireType(tag); - switch (fieldDescriptor.getEncodingType()) { - case SINGULAR: - fieldValue = readSingularField(tagWireType, inputStream, fieldDescriptor, fieldValue); - break; - case LIST: - fieldValue = readRepeatedField(tagWireType, inputStream, fieldDescriptor, fieldValue); - break; - case MAP: - if (keyDescriptor == null) { - // Looked up at the first entry since an absent map needs no entry descriptor. - MessageLiteDescriptor entryDescriptor = - descriptorPool.getDescriptorOrThrow(fieldDescriptor.getFieldProtoTypeName()); - keyDescriptor = entryDescriptor.getByFieldNumberOrThrow(MAP_KEY_FIELD_NUMBER); - valueDescriptor = entryDescriptor.getByFieldNumberOrThrow(MAP_VALUE_FIELD_NUMBER); - } - fieldValue = - readMapField( - tagWireType, - inputStream, - fieldDescriptor, - keyDescriptor, - valueDescriptor, - fieldValue); - break; - } - } - return fieldValue == null ? null : finalizeFieldValue(fieldValue); - } - - boolean hasSingleField(ByteString bytes, FieldLiteDescriptor fieldDescriptor) throws IOException { - int targetFieldNumber = fieldDescriptor.getFieldNumber(); - boolean isPackableList = - fieldDescriptor.getEncodingType().equals(EncodingType.LIST) && isPackable(fieldDescriptor); - CodedInputStream inputStream = bytes.newCodedInput(); - for (int tag = inputStream.readTag(); tag != 0; tag = inputStream.readTag()) { - int fieldNumber = WireFormat.getTagFieldNumber(tag); - if (fieldNumber != targetFieldNumber) { - skipWireField(tag, inputStream); - continue; - } - int tagWireType = WireFormat.getTagWireType(tag); - // In protobuf wire format, a zero-length entry for a singular field (e.g. empty string, - // bytes, or empty submessage) represents explicit presence on the wire. Only packed - // repeated fields with empty payload represent an empty/absent collection. - if (isPackableList && tagWireType == WireFormat.WIRETYPE_LENGTH_DELIMITED) { - int length = inputStream.readInt32(); - inputStream.skipRawBytes(length); - if (length > 0) { - return true; + try { + for (int tag = inputStream.readTag(); tag != 0; tag = inputStream.readTag()) { + if (WireFormat.getTagFieldNumber(tag) != targetFieldNumber) { + skipWireField(tag, inputStream); + continue; + } + int tagWireType = WireFormat.getTagWireType(tag); + switch (fieldDescriptor.getEncodingType()) { + case SINGULAR: + fieldValue = readSingularField(tagWireType, inputStream, fieldDescriptor, fieldValue); + break; + case LIST: + fieldValue = readRepeatedField(tagWireType, inputStream, fieldDescriptor, fieldValue); + break; + case MAP: + if (keyDescriptor == null) { + // Looked up at the first entry since an absent map needs no entry descriptor. + MessageLiteDescriptor entryDescriptor = + descriptorPool.getDescriptorOrThrow(fieldDescriptor.getFieldProtoTypeName()); + keyDescriptor = entryDescriptor.getByFieldNumberOrThrow(MAP_KEY_FIELD_NUMBER); + valueDescriptor = entryDescriptor.getByFieldNumberOrThrow(MAP_VALUE_FIELD_NUMBER); + } + fieldValue = + readMapField( + tagWireType, + inputStream, + fieldDescriptor, + keyDescriptor, + valueDescriptor, + fieldValue); + break; } - continue; } - skipWireField(tag, inputStream); - return true; + } catch (IOException e) { + throw newDecodeException(protoTypeName, e); } - return false; + return fieldValue == null ? null : finalizeFieldValue(fieldValue); } /** * Selects {@code field} from {@code bytes}, decoding it by the type information in {@code field} * for fields missing from the descriptor pool. Returns the field's default value if it's absent. - * Throws {@link CelAttributeNotFoundException} if {@code field} has no type code. */ - Object selectByFieldNumber(ByteString bytes, SelectField field) throws IOException { - if (field.typeCode() == SelectField.NO_TYPE_CODE) { - throw CelAttributeNotFoundException.forFieldResolution(field.fieldName()); - } - Object fieldValue = readFieldByNumber(bytes, field); + Object selectByFieldNumber(ByteString bytes, String protoTypeName, SelectField field) { + FieldLiteDescriptor fieldDescriptor = + findFieldDescriptor(protoTypeName, field.fieldNumber()).orElse(null); + Object fieldValue = readField(bytes, protoTypeName, fieldDescriptor, field); if (fieldValue != null) { return fieldValue; } if (field.defaultValue() != null) { return field.defaultValue(); } - return getDefaultCelValue(newFieldDescriptor(field)); + return getDefaultCelValue( + fieldDescriptor != null ? fieldDescriptor : newFieldDescriptor(field)); } /** * Finds {@code field} in {@code bytes}, decoding it by the type information in {@code field} for * fields missing from the descriptor pool. A field without a type code is decoded as a message. */ - Optional findByFieldNumber(ByteString bytes, SelectField field) throws IOException { - return Optional.ofNullable(readFieldByNumber(bytes, field)); + Optional findByFieldNumber(ByteString bytes, String protoTypeName, SelectField field) { + FieldLiteDescriptor fieldDescriptor = + findFieldDescriptor(protoTypeName, field.fieldNumber()).orElse(null); + return Optional.ofNullable(readField(bytes, protoTypeName, fieldDescriptor, field)); } /** Returns whether {@code field} is present in {@code bytes}. */ - boolean hasFieldByNumber(ByteString bytes, SelectField field) throws IOException { - return hasSingleField(bytes, newFieldDescriptor(field)); - } - - private @Nullable Object readFieldByNumber(ByteString bytes, SelectField field) - throws IOException { - SelectField.MapEntrySpec mapEntrySpec = field.mapEntrySpec(); - if (mapEntrySpec == null) { - return readSingleField(bytes, newFieldDescriptor(field)); + boolean hasFieldByNumber(ByteString bytes, String protoTypeName, SelectField field) { + FieldLiteDescriptor fieldDescriptor = + findFieldDescriptor(protoTypeName, field.fieldNumber()).orElse(null); + if (fieldDescriptor == null) { + fieldDescriptor = newFieldDescriptor(field); } - // The map entry type has no descriptor either, so its key and value are described by the spec. - return scanField( - bytes, - newFieldDescriptor(field), - newFieldDescriptor( - MAP_KEY_FIELD_NUMBER, - MAP_KEY_FIELD_NAME, - EncodingType.SINGULAR, - mapEntrySpec.keyTypeCode(), - /* protoTypeName= */ ""), - newFieldDescriptor( - MAP_VALUE_FIELD_NUMBER, - MAP_VALUE_FIELD_NAME, - EncodingType.SINGULAR, - mapEntrySpec.valueTypeCode(), - field.protoTypeName())); + boolean isPackableList = + fieldDescriptor.getEncodingType().equals(EncodingType.LIST) && isPackable(fieldDescriptor); + CodedInputStream inputStream = bytes.newCodedInput(); + try { + for (int tag = inputStream.readTag(); tag != 0; tag = inputStream.readTag()) { + if (WireFormat.getTagFieldNumber(tag) != field.fieldNumber()) { + skipWireField(tag, inputStream); + continue; + } + int tagWireType = WireFormat.getTagWireType(tag); + // In protobuf wire format, a zero-length entry for a singular field (e.g. empty string, + // bytes, or empty submessage) represents explicit presence on the wire. Only packed + // repeated fields with empty payload represent an empty/absent collection. + if (isPackableList && tagWireType == WireFormat.WIRETYPE_LENGTH_DELIMITED) { + int length = inputStream.readInt32(); + inputStream.skipRawBytes(length); + if (length > 0) { + return true; + } + continue; + } + skipWireField(tag, inputStream); + return true; + } + } catch (IOException e) { + throw newDecodeException(protoTypeName, e); + } + return false; } /** Describes {@code field} by the type information it carries. */ @@ -618,6 +632,11 @@ private static void skipWireField(int tag, CodedInputStream inputStream) throws inputStream.skipField(tag); } + private static IllegalArgumentException newDecodeException(String protoTypeName, IOException e) { + return new IllegalArgumentException( + "Failed to decode proto message of type: " + protoTypeName, e); + } + private ProtoLiteCelValueConverter(CelLiteDescriptorPool celLiteDescriptorPool) { this.descriptorPool = checkNotNull(celLiteDescriptorPool); } 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 b9cec7271..d8c23c8e0 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java @@ -25,7 +25,6 @@ import dev.cel.common.types.CelType; import dev.cel.common.types.StructTypeReference; import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor; -import java.io.IOException; import java.util.Objects; import java.util.Optional; import org.jspecify.annotations.Nullable; @@ -121,96 +120,40 @@ public final int hashCode() { @Override public Object select(String field) { - Optional fd = findFieldDescriptor(field); - if (!fd.isPresent()) { + FieldLiteDescriptor fd = findFieldDescriptor(field).orElse(null); + if (fd == null) { throw CelAttributeNotFoundException.of( String.format("field '%s' is not declared in message '%s'", field, celType().name())); } - Object fieldValue = readField(fd.get()); - return fieldValue != null - ? fieldValue - : protoLiteCelValueConverter().getDefaultCelValue(fd.get()); + Object fieldValue = readField(fd); + return fieldValue != null ? fieldValue : protoLiteCelValueConverter().getDefaultCelValue(fd); } @Override public Optional find(String field) { - Optional fd = findFieldDescriptor(field); - if (!fd.isPresent()) { - // Per SelectableValue#find, a field that doesn't exist is reported as absent. - return Optional.empty(); - } - return Optional.ofNullable(readField(fd.get())); + FieldLiteDescriptor fd = findFieldDescriptor(field).orElse(null); + // Per SelectableValue#find, a field that doesn't exist is reported as absent. + return Optional.ofNullable(fd).map(value -> readField(value)); } @Override public Object selectByFieldNumber(SelectField field) { - Optional fd = findFieldDescriptor(field); - if (fd.isPresent()) { - Object known = readField(fd.get()); - if (known != null) { - return known; - } - if (field.defaultValue() != null) { - return field.defaultValue(); - } - return protoLiteCelValueConverter().getDefaultCelValue(fd.get()); - } - try { - return protoLiteCelValueConverter().selectByFieldNumber(toByteString(), field); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } + return protoLiteCelValueConverter() + .selectByFieldNumber(toByteString(), celType().name(), field); } @Override public boolean hasFieldByNumber(SelectField field) { - Optional fd = findFieldDescriptor(field); - if (fd.isPresent()) { - return hasField(fd.get()); - } - try { - return protoLiteCelValueConverter().hasFieldByNumber(toByteString(), field); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } + return protoLiteCelValueConverter().hasFieldByNumber(toByteString(), celType().name(), field); } @Override public Optional findByFieldNumber(SelectField field) { - Optional fd = findFieldDescriptor(field); - if (fd.isPresent()) { - return Optional.ofNullable(readField(fd.get())); - } - try { - return protoLiteCelValueConverter().findByFieldNumber(toByteString(), field); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } + return protoLiteCelValueConverter().findByFieldNumber(toByteString(), celType().name(), field); } private @Nullable Object readField(FieldLiteDescriptor fd) { - try { - return protoLiteCelValueConverter().readSingleField(toByteString(), fd); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } - } - - private boolean hasField(FieldLiteDescriptor fd) { - try { - return protoLiteCelValueConverter().hasSingleField(toByteString(), fd); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } - } - - private Optional findFieldDescriptor(SelectField field) { - return protoLiteCelValueConverter().findFieldDescriptor(celType().name(), field.fieldNumber()); + return protoLiteCelValueConverter().readField(toByteString(), celType().name(), fd); } private Optional findFieldDescriptor(String fieldName) { diff --git a/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java b/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java index 77cae2c43..aa1440522 100644 --- a/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java +++ b/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java @@ -23,7 +23,6 @@ import dev.cel.common.exceptions.CelAttributeNotFoundException; import dev.cel.common.types.CelType; import dev.cel.common.types.StructTypeReference; -import java.io.IOException; import java.util.Locale; import java.util.Optional; @@ -114,32 +113,18 @@ private CelAttributeNotFoundException newUnoptimizedFieldResolutionException(Str @Override public Object selectByFieldNumber(SelectField field) { - try { - return protoLiteCelValueConverter().selectByFieldNumber(toByteString(), field); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } + return protoLiteCelValueConverter() + .selectByFieldNumber(toByteString(), celType().name(), field); } @Override public boolean hasFieldByNumber(SelectField field) { - try { - return protoLiteCelValueConverter().hasFieldByNumber(toByteString(), field); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } + return protoLiteCelValueConverter().hasFieldByNumber(toByteString(), celType().name(), field); } @Override public Optional findByFieldNumber(SelectField field) { - try { - return protoLiteCelValueConverter().findByFieldNumber(toByteString(), field); - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to decode proto message of type: " + celType().name(), e); - } + return protoLiteCelValueConverter().findByFieldNumber(toByteString(), celType().name(), field); } static RawProtoMessageLiteValue create( diff --git a/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java b/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java index ace0e5ebe..22da58d55 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java @@ -58,6 +58,8 @@ @RunWith(TestParameterInjector.class) public final class ProtoLiteCelValueConverterTest { + private static final String PROTO3_TEST_ALL_TYPES = "cel.expr.conformance.proto3.TestAllTypes"; + private static final CelLiteDescriptorPool EMPTY_DESCRIPTOR_POOL = new CelLiteDescriptorPool() { @Override @@ -174,18 +176,18 @@ private enum RepeatedFieldBytesTestCase { // repeated_int64 is declared unpacked in proto2 and packed in proto3. @Test - public void readSingleField_repeatedFields_packedBytesCombinations( + public void readField_repeatedFields_packedBytesCombinations( @TestParameter RepeatedFieldBytesTestCase testCase, @TestParameter({ "cel.expr.conformance.proto2.TestAllTypes", "cel.expr.conformance.proto3.TestAllTypes" }) - String messageName) - throws Exception { + String messageName) { FieldLiteDescriptor fd = fieldDescriptor(messageName, TestAllTypes.REPEATED_INT64_FIELD_NUMBER); Object result = - PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(ByteString.copyFrom(testCase.bytes), fd); + PROTO_LITE_CEL_VALUE_CONVERTER.readField( + ByteString.copyFrom(testCase.bytes), messageName, fd); assertThat(result).isEqualTo(ImmutableList.of(1L, 2L, 3L)); } @@ -236,16 +238,16 @@ private enum UnknownFieldsTestCase { } @Test - public void readSingleField_unknownFields_skipped(@TestParameter UnknownFieldsTestCase testCase) + public void readField_unknownFields_skipped(@TestParameter UnknownFieldsTestCase testCase) throws Exception { TestAllTypes parsedMsg = TestAllTypes.parseFrom(testCase.bytes, ExtensionRegistryLite.getEmptyRegistry()); FieldLiteDescriptor fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_INT64_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.SINGLE_INT64_FIELD_NUMBER); Object result = - PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(ByteString.copyFrom(testCase.bytes), fd); + PROTO_LITE_CEL_VALUE_CONVERTER.readField( + ByteString.copyFrom(testCase.bytes), PROTO3_TEST_ALL_TYPES, fd); assertThat(result).isNull(); assertThat(TextFormat.printer().printToString(parsedMsg).trim()) @@ -272,7 +274,7 @@ public void readSingleField_unknownFields_skipped(@TestParameter UnknownFieldsTe * } */ @Test - public void readSingleField_unknownFieldsPrecedeTarget_decodesTarget() throws Exception { + public void readField_unknownFieldsPrecedeTarget_decodesTarget() throws Exception { // Unknown fields precede the known map entries so that decoding must continue past them. byte[] unknownMessageBytes = { -96, -100, 1, 1, -83, -100, 1, 2, 0, 0, 0, -79, -100, 1, 3, 0, 0, 0, 0, 0, 0, 0, -70, -100, 1, @@ -283,12 +285,11 @@ public void readSingleField_unknownFieldsPrecedeTarget_decodesTarget() throws Ex TestAllTypes parsedMsg = TestAllTypes.parseFrom(unknownMessageBytes, ExtensionRegistryLite.getEmptyRegistry()); FieldLiteDescriptor fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.MAP_BOOL_DOUBLE_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.MAP_BOOL_DOUBLE_FIELD_NUMBER); Object result = - PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField( - ByteString.copyFrom(unknownMessageBytes), fd); + PROTO_LITE_CEL_VALUE_CONVERTER.readField( + ByteString.copyFrom(unknownMessageBytes), PROTO3_TEST_ALL_TYPES, fd); assertThat(TextFormat.printer().printToString(parsedMsg)) .isEqualTo( @@ -320,7 +321,7 @@ public void readSingleField_unknownFieldsPrecedeTarget_decodesTarget() throws Ex public void getDefaultCelValue_fieldDescriptor_returnsDefault() { FieldLiteDescriptor fieldDescriptor = DESCRIPTOR_POOL - .getDescriptorOrThrow("cel.expr.conformance.proto3.TestAllTypes") + .getDescriptorOrThrow(PROTO3_TEST_ALL_TYPES) .getByFieldNameOrThrow("single_string"); Object defaultValue = PROTO_LITE_CEL_VALUE_CONVERTER.getDefaultCelValue(fieldDescriptor); @@ -332,7 +333,7 @@ public void getDefaultCelValue_fieldDescriptor_returnsDefault() { public void getDefaultCelValue_nestedMessageWithoutDescriptor_returnsRawProtoMessageLiteValue() { FieldLiteDescriptor nestedMsgField = DESCRIPTOR_POOL - .getDescriptorOrThrow("cel.expr.conformance.proto3.TestAllTypes") + .getDescriptorOrThrow(PROTO3_TEST_ALL_TYPES) .getByFieldNameOrThrow("single_nested_message"); ProtoLiteCelValueConverter converterWithoutNested = ProtoLiteCelValueConverter.newInstance(EMPTY_DESCRIPTOR_POOL); @@ -347,8 +348,7 @@ public void getDefaultCelValue_nestedMessageWithoutDescriptor_returnsRawProtoMes } @Test - public void readSingleField_nestedMessageWithoutDescriptor_returnsRawProtoMessageLiteValue() - throws Exception { + public void readField_nestedMessageWithoutDescriptor_returnsRawProtoMessageLiteValue() { // DESCRIPTOR_POOL only registers TestAllTypesCelDescriptor, not NestedTestAllTypesCelDescriptor TestAllTypes msg = TestAllTypes.newBuilder() @@ -357,10 +357,10 @@ public void readSingleField_nestedMessageWithoutDescriptor_returnsRawProtoMessag .setPayload(TestAllTypes.newBuilder().setSingleInt64(42L))) .build(); FieldLiteDescriptor fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.ONEOF_TYPE_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.ONEOF_TYPE_FIELD_NUMBER); - Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(msg.toByteString(), fd); + Object result = + PROTO_LITE_CEL_VALUE_CONVERTER.readField(msg.toByteString(), PROTO3_TEST_ALL_TYPES, fd); assertThat(result).isInstanceOf(RawProtoMessageLiteValue.class); RawProtoMessageLiteValue rawValue = (RawProtoMessageLiteValue) result; @@ -370,7 +370,7 @@ public void readSingleField_nestedMessageWithoutDescriptor_returnsRawProtoMessag } @Test - public void readSingleField_splitSingularSubmessages_mergesAllOccurrences() throws Exception { + public void readField_splitSingularSubmessages_mergesAllOccurrences() throws Exception { ByteArrayOutputStream unknownFieldBaos = new ByteArrayOutputStream(); CodedOutputStream cos = CodedOutputStream.newInstance(unknownFieldBaos); cos.writeInt64(999, 42L); @@ -394,19 +394,18 @@ public void readSingleField_splitSingularSubmessages_mergesAllOccurrences() thro .build(); ByteString splitWireBytes = part1.toByteString().concat(part2.toByteString()); FieldLiteDescriptor nestedMessageFd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", - TestAllTypes.SINGLE_NESTED_MESSAGE_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.SINGLE_NESTED_MESSAGE_FIELD_NUMBER); FieldLiteDescriptor oneofTypeFd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.ONEOF_TYPE_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.ONEOF_TYPE_FIELD_NUMBER); ProtoMessageLiteValue nestedMsg = (ProtoMessageLiteValue) - PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(splitWireBytes, nestedMessageFd); + PROTO_LITE_CEL_VALUE_CONVERTER.readField( + splitWireBytes, PROTO3_TEST_ALL_TYPES, nestedMessageFd); RawProtoMessageLiteValue rawSubmessage = (RawProtoMessageLiteValue) - PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(splitWireBytes, oneofTypeFd); + PROTO_LITE_CEL_VALUE_CONVERTER.readField( + splitWireBytes, PROTO3_TEST_ALL_TYPES, oneofTypeFd); assertThat(nestedMsg.rawValue()).isNull(); assertThat(nestedMsg.select("bb")).isEqualTo(99L); @@ -424,54 +423,53 @@ public void readSingleField_splitSingularSubmessages_mergesAllOccurrences() thro @Test public void parseMessageLite_emptyBytes_returnsDefaultInstanceSingleton() { MessageLite parsed = - PROTO_LITE_CEL_VALUE_CONVERTER.parseMessageLite( - ByteString.EMPTY, "cel.expr.conformance.proto3.TestAllTypes"); + PROTO_LITE_CEL_VALUE_CONVERTER.parseMessageLite(ByteString.EMPTY, PROTO3_TEST_ALL_TYPES); assertThat(parsed).isSameInstanceAs(TestAllTypes.getDefaultInstance()); } @Test - public void readSingleField_emptyBytes_returnsNull() throws Exception { + public void readField_emptyBytes_returnsNull() { FieldLiteDescriptor fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_INT64_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.SINGLE_INT64_FIELD_NUMBER); - Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(ByteString.EMPTY, fd); + Object result = + PROTO_LITE_CEL_VALUE_CONVERTER.readField(ByteString.EMPTY, PROTO3_TEST_ALL_TYPES, fd); assertThat(result).isNull(); } @Test - public void readSingleField_absentField_returnsNull() throws Exception { + public void readField_absentField_returnsNull() { TestAllTypes proto = TestAllTypes.newBuilder().setSingleInt64(42L).build(); FieldLiteDescriptor fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_BOOL_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.SINGLE_BOOL_FIELD_NUMBER); - Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(proto.toByteString(), fd); + Object result = + PROTO_LITE_CEL_VALUE_CONVERTER.readField(proto.toByteString(), PROTO3_TEST_ALL_TYPES, fd); assertThat(result).isNull(); } @Test - public void readSingleField_emptyPackedRepeated_returnsNull() throws Exception { + public void readField_emptyPackedRepeated_returnsNull() throws Exception { ByteArrayOutputStream emptyPackedOut = new ByteArrayOutputStream(); CodedOutputStream emptyPackedCos = CodedOutputStream.newInstance(emptyPackedOut); emptyPackedCos.writeByteArray(TestAllTypes.REPEATED_INT32_FIELD_NUMBER, new byte[0]); emptyPackedCos.flush(); ByteString emptyPackedBytes = ByteString.copyFrom(emptyPackedOut.toByteArray()); FieldLiteDescriptor repeatedInt32Fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.REPEATED_INT32_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.REPEATED_INT32_FIELD_NUMBER); Object result = - PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(emptyPackedBytes, repeatedInt32Fd); + PROTO_LITE_CEL_VALUE_CONVERTER.readField( + emptyPackedBytes, PROTO3_TEST_ALL_TYPES, repeatedInt32Fd); assertThat(result).isNull(); } @SuppressWarnings("ImmutableEnumChecker") // Test only - private enum ReadSingleFieldTestCase { + private enum ReadFieldTestCase { SINGLE_STRING(TestAllTypes.SINGLE_STRING_FIELD_NUMBER, "target_str"), REPEATED_STRING(TestAllTypes.REPEATED_STRING_FIELD_NUMBER, ImmutableList.of("a", "b")), REPEATED_INT32(TestAllTypes.REPEATED_INT32_FIELD_NUMBER, ImmutableList.of(1L, 2L)), @@ -485,15 +483,15 @@ private enum ReadSingleFieldTestCase { private final int fieldNumber; private final Object expected; - ReadSingleFieldTestCase(int fieldNumber, Object expected) { + ReadFieldTestCase(int fieldNumber, Object expected) { this.fieldNumber = fieldNumber; this.expected = expected; } } @Test - public void readSingleField_skipsOtherFieldsAndDecodesTarget( - @TestParameter ReadSingleFieldTestCase testCase) throws Exception { + public void readField_skipsOtherFieldsAndDecodesTarget(@TestParameter ReadFieldTestCase testCase) + throws Exception { TestAllTypes part1 = TestAllTypes.newBuilder() .setSingleInt64(42L) @@ -530,16 +528,15 @@ public void readSingleField_skipsOtherFieldsAndDecodesTarget( .toByteString() .concat(part2.toByteString()) .concat(ByteString.copyFrom(extraWireOut.toByteArray())); - FieldLiteDescriptor fd = - fieldDescriptor("cel.expr.conformance.proto3.TestAllTypes", testCase.fieldNumber); + FieldLiteDescriptor fd = fieldDescriptor(PROTO3_TEST_ALL_TYPES, testCase.fieldNumber); - Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(bytes, fd); + Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readField(bytes, PROTO3_TEST_ALL_TYPES, fd); assertThat(result).isEqualTo(testCase.expected); } @Test - public void readSingleField_repeatedMapKey_convertsOnlyFinalValue() throws Exception { + public void readField_repeatedMapKey_convertsOnlyFinalValue() { // The first value is out of Timestamp's range, so converting it would throw. ByteString bytes = TestAllTypes.newBuilder() @@ -552,37 +549,24 @@ public void readSingleField_repeatedMapKey_convertsOnlyFinalValue() throws Excep .build() .toByteString()); FieldLiteDescriptor fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", - TestAllTypes.MAP_STRING_TIMESTAMP_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.MAP_STRING_TIMESTAMP_FIELD_NUMBER); - Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(bytes, fd); + Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readField(bytes, PROTO3_TEST_ALL_TYPES, fd); assertThat(result).isEqualTo(ImmutableMap.of("k", Instant.ofEpochSecond(100))); } - @Test - public void hasSingleField_emptyBytes_returnsFalse() throws Exception { - FieldLiteDescriptor singleInt64Fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_INT64_FIELD_NUMBER); - - boolean result = PROTO_LITE_CEL_VALUE_CONVERTER.hasSingleField(ByteString.EMPTY, singleInt64Fd); - - assertThat(result).isFalse(); - } - // repeated_int32 is declared unpacked in proto2 and packed in proto3. @Test - public void hasSingleField_emptyPackedRepeatedField_returnsFalse( + public void hasFieldByNumber_emptyPackedRepeatedField_returnsFalse( @TestParameter({ "cel.expr.conformance.proto2.TestAllTypes", "cel.expr.conformance.proto3.TestAllTypes" }) String messageName) throws Exception { - FieldLiteDescriptor repeatedInt32Fd = - fieldDescriptor(messageName, TestAllTypes.REPEATED_INT32_FIELD_NUMBER); + SelectField repeatedInt32 = + SelectField.create(TestAllTypes.REPEATED_INT32_FIELD_NUMBER, "repeated_int32"); ByteArrayOutputStream emptyPackedOut = new ByteArrayOutputStream(); CodedOutputStream emptyPackedCos = CodedOutputStream.newInstance(emptyPackedOut); emptyPackedCos.writeInt64(TestAllTypes.SINGLE_INT64_FIELD_NUMBER, 42L); @@ -591,16 +575,16 @@ public void hasSingleField_emptyPackedRepeatedField_returnsFalse( ByteString emptyPackedBytes = ByteString.copyFrom(emptyPackedOut.toByteArray()); boolean result = - PROTO_LITE_CEL_VALUE_CONVERTER.hasSingleField(emptyPackedBytes, repeatedInt32Fd); + PROTO_LITE_CEL_VALUE_CONVERTER.hasFieldByNumber( + emptyPackedBytes, messageName, repeatedInt32); assertThat(result).isFalse(); } @Test - public void hasSingleField_emptyPackedFollowedByPopulatedPacked_returnsTrue() throws Exception { - FieldLiteDescriptor repeatedInt32Fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.REPEATED_INT32_FIELD_NUMBER); + public void hasFieldByNumber_emptyPackedFollowedByPopulatedPacked_returnsTrue() throws Exception { + SelectField repeatedInt32 = + SelectField.create(TestAllTypes.REPEATED_INT32_FIELD_NUMBER, "repeated_int32"); ByteArrayOutputStream emptyThenPopulatedOut = new ByteArrayOutputStream(); CodedOutputStream emptyThenPopulatedCos = CodedOutputStream.newInstance(emptyThenPopulatedOut); emptyThenPopulatedCos.writeByteArray(TestAllTypes.REPEATED_INT32_FIELD_NUMBER, new byte[0]); @@ -610,29 +594,29 @@ public void hasSingleField_emptyPackedFollowedByPopulatedPacked_returnsTrue() th ByteString emptyThenPopulatedBytes = ByteString.copyFrom(emptyThenPopulatedOut.toByteArray()); boolean result = - PROTO_LITE_CEL_VALUE_CONVERTER.hasSingleField(emptyThenPopulatedBytes, repeatedInt32Fd); + PROTO_LITE_CEL_VALUE_CONVERTER.hasFieldByNumber( + emptyThenPopulatedBytes, PROTO3_TEST_ALL_TYPES, repeatedInt32); assertThat(result).isTrue(); } // Wire types 3 and 4 are START_GROUP and END_GROUP. @Test - public void readSingleField_groupInSkippedField_throwsUnsupportedOperationException( + public void readField_groupInSkippedField_throwsUnsupportedOperationException( @TestParameter({"3", "4"}) int groupWireType) { ByteString bytes = ByteString.copyFrom(new byte[] {(byte) ((1 << 3) | groupWireType)}); FieldLiteDescriptor fd = - fieldDescriptor( - "cel.expr.conformance.proto3.TestAllTypes", TestAllTypes.SINGLE_STRING_FIELD_NUMBER); + fieldDescriptor(PROTO3_TEST_ALL_TYPES, TestAllTypes.SINGLE_STRING_FIELD_NUMBER); UnsupportedOperationException e = assertThrows( UnsupportedOperationException.class, - () -> PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(bytes, fd)); + () -> PROTO_LITE_CEL_VALUE_CONVERTER.readField(bytes, PROTO3_TEST_ALL_TYPES, fd)); assertThat(e).hasMessageThat().isEqualTo("Groups are not supported"); } private static FieldLiteDescriptor fieldDescriptor(String messageName, int fieldNumber) { - return PROTO_LITE_CEL_VALUE_CONVERTER.findFieldDescriptor(messageName, fieldNumber).get(); + return DESCRIPTOR_POOL.getDescriptorOrThrow(messageName).getByFieldNumberOrThrow(fieldNumber); } } 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 ebabba660..bb93a19ce 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java @@ -644,6 +644,24 @@ public void selectByFieldNumber_knownField_returnsValue() { assertThat(result).isEqualTo("foo"); } + @Test + public void selectByFieldNumber_absentKnownMessageField_returnsDefaultMessage() { + ProtoMessageLiteValue val = + ProtoMessageLiteValue.create( + TestAllTypes.getDefaultInstance(), + "cel.expr.conformance.proto3.TestAllTypes", + PROTO_LITE_CEL_VALUE_CONVERTER); + + Object result = val.selectByFieldNumber(SelectField.create(21L, "single_nested_message", 11)); + + assertThat(result) + .isEqualTo( + ProtoMessageLiteValue.create( + NestedMessage.getDefaultInstance(), + "cel.expr.conformance.proto3.TestAllTypes.NestedMessage", + PROTO_LITE_CEL_VALUE_CONVERTER)); + } + @Test public void selectByFieldNumber_unknownWireField_decoded() throws Exception { ByteArrayOutputStream baos = new ByteArrayOutputStream(); diff --git a/common/src/test/java/dev/cel/common/values/RawProtoMessageLiteValueTest.java b/common/src/test/java/dev/cel/common/values/RawProtoMessageLiteValueTest.java index b9cc6382a..2edb0b6a4 100644 --- a/common/src/test/java/dev/cel/common/values/RawProtoMessageLiteValueTest.java +++ b/common/src/test/java/dev/cel/common/values/RawProtoMessageLiteValueTest.java @@ -272,23 +272,6 @@ public void selectByFieldNumber_absentWithDefaultValue_returnsDefault() { assertThat(val).isEqualTo("default"); } - @Test - public void selectByFieldNumber_unknownFieldWithoutTypeCode_throwsCelAttributeNotFoundException() - throws Exception { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - CodedOutputStream cos = CodedOutputStream.newInstance(baos); - cos.writeString(999, "unknown"); - cos.flush(); - RawProtoMessageLiteValue raw = - RawProtoMessageLiteValue.create( - ByteString.copyFrom(baos.toByteArray()), - "cel.expr.conformance.proto3.TestAllTypes", - EMPTY_CONVERTER); - SelectField selectField = SelectField.create(999L, "unknown_field"); - - assertThrows(CelAttributeNotFoundException.class, () -> raw.selectByFieldNumber(selectField)); - } - @Test public void hasFieldByNumber_wirePresent_returnsTrue() throws Exception { ByteArrayOutputStream baos = new ByteArrayOutputStream();