From a8a98a2cde7299a34050a756d312540f348d9b1d Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Fri, 9 Oct 2026 15:34:15 -0700 Subject: [PATCH] Remove DeferredConversion from ProtoLiteCelValueConverter. PiperOrigin-RevId: 996797670 --- .../values/ProtoLiteCelValueConverter.java | 42 ++++--------------- .../ProtoLiteCelValueConverterTest.java | 23 ++++++++++ 2 files changed, 32 insertions(+), 33 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 fa63abf61..6aea2fb98 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java @@ -328,7 +328,7 @@ private Map.Entry readSingleMapEntry( int tagWireType = WireFormat.getTagWireType(tag); fieldValue = readFieldValue(tagWireType, inputStream, fieldDescriptor, fieldValue); } - return fieldValue == null ? null : resolveFieldValue(finalizeFieldValue(fieldValue)); + return fieldValue == null ? null : finalizeFieldValue(fieldValue); } boolean hasSingleField(ByteString bytes, FieldLiteDescriptor fieldDescriptor) throws IOException { @@ -430,7 +430,7 @@ boolean hasFieldByNumber(ByteString bytes, SelectField field) throws IOException valueDescriptor, mapValues); } - return mapValues == null ? null : resolveFieldValue(finalizeFieldValue(mapValues)); + return mapValues == null ? null : finalizeFieldValue(mapValues); } /** Describes {@code field} by the type information it carries. */ @@ -480,38 +480,26 @@ private static FieldLiteDescriptor newFieldDescriptor( protoTypeName); } - /** Returns the CEL value of a scanned field, completing any conversion that was deferred. */ - private Object resolveFieldValue(Object fieldValue) { - if (fieldValue instanceof DeferredConversion) { - return toRuntimeValue(((DeferredConversion) fieldValue).value); - } - return fieldValue; - } - /** * Converts a value accumulated while scanning a field into its final immutable form. * *

Repeated and map fields accumulate into mutable containers, which are copied into immutable * ones. Well-known types other than FieldMask (see {@link #isStructLike}) are kept as parsed - * {@link MessageLite}s until the scan completes so that split occurrences can be merged, and - * values holding them are wrapped in a {@link DeferredConversion}. All other messages are wrapped - * as CEL values as soon as they are read. + * {@link MessageLite}s until the scan completes, so that split occurrences can be merged and map + * values replaced by a repeated key are never converted. All other messages are wrapped as CEL + * values as soon as they are read. */ - private static Object finalizeFieldValue(Object accumulatedValue) { + private Object finalizeFieldValue(Object accumulatedValue) { if (accumulatedValue instanceof List) { ImmutableList list = ImmutableList.copyOf((List) accumulatedValue); - return Iterables.any(list, MessageLite.class::isInstance) - ? new DeferredConversion(list) - : list; + return Iterables.any(list, MessageLite.class::isInstance) ? toRuntimeValue(list) : list; } if (accumulatedValue instanceof Map) { ImmutableMap map = ImmutableMap.copyOf((Map) accumulatedValue); - return Iterables.any(map.values(), MessageLite.class::isInstance) - ? new DeferredConversion(map) - : map; + return Iterables.any(map.values(), MessageLite.class::isInstance) ? toRuntimeValue(map) : map; } return accumulatedValue instanceof MessageLite - ? new DeferredConversion(accumulatedValue) + ? toRuntimeValue(accumulatedValue) : accumulatedValue; } @@ -672,18 +660,6 @@ static void skipWireField(int tag, CodedInputStream inputStream) throws IOExcept } } - /** - * A field value holding well-known type messages, whose conversion to CEL values is deferred - * until {@link #resolveFieldValue}. - */ - private static final class DeferredConversion { - private final Object value; - - private DeferredConversion(Object value) { - this.value = checkNotNull(value); - } - } - private ProtoLiteCelValueConverter(CelLiteDescriptorPool celLiteDescriptorPool) { this.descriptorPool = checkNotNull(celLiteDescriptorPool); } 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 a812affa1..c22980ac5 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java @@ -539,6 +539,29 @@ public void readSingleField_skipsOtherFieldsAndDecodesTarget( assertThat(result).isEqualTo(testCase.expected); } + @Test + public void readSingleField_repeatedMapKey_convertsOnlyFinalValue() throws Exception { + // The first value is out of Timestamp's range, so converting it would throw. + ByteString bytes = + TestAllTypes.newBuilder() + .putMapStringTimestamp("k", Timestamp.newBuilder().setSeconds(1L << 60).build()) + .build() + .toByteString() + .concat( + TestAllTypes.newBuilder() + .putMapStringTimestamp("k", Timestamp.newBuilder().setSeconds(100).build()) + .build() + .toByteString()); + FieldLiteDescriptor fd = + fieldDescriptor( + "cel.expr.conformance.proto3.TestAllTypes", + TestAllTypes.MAP_STRING_TIMESTAMP_FIELD_NUMBER); + + Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(bytes, fd); + + assertThat(result).isEqualTo(ImmutableMap.of("k", Instant.ofEpochSecond(100))); + } + @Test public void hasSingleField_emptyBytes_returnsFalse() throws Exception { FieldLiteDescriptor singleInt64Fd =