Skip to content

Commit 4681818

Browse files
l46kokcopybara-github
authored andcommitted
Remove DeferredConversion from ProtoLiteCelValueConverter.
PiperOrigin-RevId: 996749918
1 parent eb9055b commit 4681818

2 files changed

Lines changed: 32 additions & 33 deletions

File tree

‎common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java‎

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ private Map.Entry<Object, Object> readSingleMapEntry(
328328
int tagWireType = WireFormat.getTagWireType(tag);
329329
fieldValue = readFieldValue(tagWireType, inputStream, fieldDescriptor, fieldValue);
330330
}
331-
return fieldValue == null ? null : resolveFieldValue(finalizeFieldValue(fieldValue));
331+
return fieldValue == null ? null : finalizeFieldValue(fieldValue);
332332
}
333333

334334
boolean hasSingleField(ByteString bytes, FieldLiteDescriptor fieldDescriptor) throws IOException {
@@ -430,7 +430,7 @@ boolean hasFieldByNumber(ByteString bytes, SelectField field) throws IOException
430430
valueDescriptor,
431431
mapValues);
432432
}
433-
return mapValues == null ? null : resolveFieldValue(finalizeFieldValue(mapValues));
433+
return mapValues == null ? null : finalizeFieldValue(mapValues);
434434
}
435435

436436
/** Describes {@code field} by the type information it carries. */
@@ -480,38 +480,26 @@ private static FieldLiteDescriptor newFieldDescriptor(
480480
protoTypeName);
481481
}
482482

483-
/** Returns the CEL value of a scanned field, completing any conversion that was deferred. */
484-
private Object resolveFieldValue(Object fieldValue) {
485-
if (fieldValue instanceof DeferredConversion) {
486-
return toRuntimeValue(((DeferredConversion) fieldValue).value);
487-
}
488-
return fieldValue;
489-
}
490-
491483
/**
492484
* Converts a value accumulated while scanning a field into its final immutable form.
493485
*
494486
* <p>Repeated and map fields accumulate into mutable containers, which are copied into immutable
495487
* ones. Well-known types other than FieldMask (see {@link #isStructLike}) are kept as parsed
496-
* {@link MessageLite}s until the scan completes so that split occurrences can be merged, and
497-
* values holding them are wrapped in a {@link DeferredConversion}. All other messages are wrapped
498-
* as CEL values as soon as they are read.
488+
* {@link MessageLite}s until the scan completes, so that split occurrences can be merged and map
489+
* values replaced by a repeated key are never converted. All other messages are wrapped as CEL
490+
* values as soon as they are read.
499491
*/
500-
private static Object finalizeFieldValue(Object accumulatedValue) {
492+
private Object finalizeFieldValue(Object accumulatedValue) {
501493
if (accumulatedValue instanceof List) {
502494
ImmutableList<?> list = ImmutableList.copyOf((List<?>) accumulatedValue);
503-
return Iterables.any(list, MessageLite.class::isInstance)
504-
? new DeferredConversion(list)
505-
: list;
495+
return Iterables.any(list, MessageLite.class::isInstance) ? toRuntimeValue(list) : list;
506496
}
507497
if (accumulatedValue instanceof Map) {
508498
ImmutableMap<?, ?> map = ImmutableMap.copyOf((Map<?, ?>) accumulatedValue);
509-
return Iterables.any(map.values(), MessageLite.class::isInstance)
510-
? new DeferredConversion(map)
511-
: map;
499+
return Iterables.any(map.values(), MessageLite.class::isInstance) ? toRuntimeValue(map) : map;
512500
}
513501
return accumulatedValue instanceof MessageLite
514-
? new DeferredConversion(accumulatedValue)
502+
? toRuntimeValue(accumulatedValue)
515503
: accumulatedValue;
516504
}
517505

@@ -672,18 +660,6 @@ static void skipWireField(int tag, CodedInputStream inputStream) throws IOExcept
672660
}
673661
}
674662

675-
/**
676-
* A field value holding well-known type messages, whose conversion to CEL values is deferred
677-
* until {@link #resolveFieldValue}.
678-
*/
679-
private static final class DeferredConversion {
680-
private final Object value;
681-
682-
private DeferredConversion(Object value) {
683-
this.value = checkNotNull(value);
684-
}
685-
}
686-
687663
private ProtoLiteCelValueConverter(CelLiteDescriptorPool celLiteDescriptorPool) {
688664
this.descriptorPool = checkNotNull(celLiteDescriptorPool);
689665
}

‎common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,29 @@ public void readSingleField_skipsOtherFieldsAndDecodesTarget(
539539
assertThat(result).isEqualTo(testCase.expected);
540540
}
541541

542+
@Test
543+
public void readSingleField_repeatedMapKey_convertsOnlyFinalValue() throws Exception {
544+
// The first value is out of Timestamp's range, so converting it would throw.
545+
ByteString bytes =
546+
TestAllTypes.newBuilder()
547+
.putMapStringTimestamp("k", Timestamp.newBuilder().setSeconds(1L << 60).build())
548+
.build()
549+
.toByteString()
550+
.concat(
551+
TestAllTypes.newBuilder()
552+
.putMapStringTimestamp("k", Timestamp.newBuilder().setSeconds(100).build())
553+
.build()
554+
.toByteString());
555+
FieldLiteDescriptor fd =
556+
fieldDescriptor(
557+
"cel.expr.conformance.proto3.TestAllTypes",
558+
TestAllTypes.MAP_STRING_TIMESTAMP_FIELD_NUMBER);
559+
560+
Object result = PROTO_LITE_CEL_VALUE_CONVERTER.readSingleField(bytes, fd);
561+
562+
assertThat(result).isEqualTo(ImmutableMap.of("k", Instant.ofEpochSecond(100)));
563+
}
564+
542565
@Test
543566
public void hasSingleField_emptyBytes_returnsFalse() throws Exception {
544567
FieldLiteDescriptor singleInt64Fd =

0 commit comments

Comments
 (0)