From 19a2280dc826961383f7fb90a22649b41dc92eac Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Thu, 12 Nov 2026 14:33:50 -0800 Subject: [PATCH] Adds conformance tests for setting numeric fields to numbers of another type. `TestAllTypes{single_uint64: dyn(1)}` must fail, as the spec has no implicit conversions between int, uint, and double. The tests also cover wrapper fields, list elements, map keys and values, and wrapper messages, along with a few cases where the types match. Also adds the tests to the conformance matrix. PiperOrigin-RevId: 1020449183 --- common/BUILD.bazel | 5 + common/internal/BUILD.bazel | 5 + .../src/main/java/dev/cel/common/BUILD.bazel | 16 + .../java/dev/cel/common/internal/BUILD.bazel | 21 +- .../internal/DefaultLiteDescriptorPool.java | 12 +- .../dev/cel/common/internal/ProtoAdapter.java | 82 ++--- .../cel/common/internal/ProtoLiteAdapter.java | 47 +-- .../java/dev/cel/common/values/BUILD.bazel | 12 + .../values/ProtoLiteCelValueConverter.java | 11 +- .../values/ProtoMessageLiteValueProvider.java | 251 +++++++++++++- .../values/RawProtoMessageLiteValue.java | 2 +- .../cel/common/internal/ProtoAdapterTest.java | 16 + .../java/dev/cel/common/values/BUILD.bazel | 2 + .../ProtoLiteCelValueConverterTest.java | 12 + .../ProtoMessageLiteValueProviderTest.java | 308 +++++++++++++++++- .../values/ProtoMessageValueProviderTest.java | 37 ++- .../test/java/dev/cel/conformance/BUILD.bazel | 83 +---- .../extensions/CelOptionalLibraryTest.java | 15 - .../dev/cel/protobuf/CelLiteDescriptor.java | 12 +- .../cel/runtime/planner/ProgramPlanner.java | 12 +- .../cel/runtime/CelLiteInterpreterTest.java | 56 +--- .../runtime/CelLiteRuntimeAndroidTest.java | 41 +++ .../dev/cel/runtime/CelLiteRuntimeTest.java | 25 +- .../runtime/planner/ProgramPlannerTest.java | 10 + .../java/dev/cel/testing/compiled/BUILD.bazel | 12 + 25 files changed, 863 insertions(+), 242 deletions(-) diff --git a/common/BUILD.bazel b/common/BUILD.bazel index 1c751bf53..16f8812ee 100644 --- a/common/BUILD.bazel +++ b/common/BUILD.bazel @@ -166,6 +166,11 @@ java_library( exports = ["//common/src/main/java/dev/cel/common:proto_json_adapter"], ) +cel_android_library( + name = "proto_json_adapter_android", + exports = ["//common/src/main/java/dev/cel/common:proto_json_adapter_android"], +) + java_library( name = "source", visibility = ["//:internal"], diff --git a/common/internal/BUILD.bazel b/common/internal/BUILD.bazel index 7c33e56b9..2fdcdbc25 100644 --- a/common/internal/BUILD.bazel +++ b/common/internal/BUILD.bazel @@ -42,6 +42,11 @@ java_library( exports = ["//common/src/main/java/dev/cel/common/internal:proto_lite_adapter"], ) +cel_android_library( + name = "proto_lite_adapter_android", + exports = ["//common/src/main/java/dev/cel/common/internal:proto_lite_adapter_android"], +) + java_library( name = "proto_equality", exports = ["//common/src/main/java/dev/cel/common/internal:proto_equality"], diff --git a/common/src/main/java/dev/cel/common/BUILD.bazel b/common/src/main/java/dev/cel/common/BUILD.bazel index d69cb0b1f..741af88aa 100644 --- a/common/src/main/java/dev/cel/common/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/BUILD.bazel @@ -439,6 +439,22 @@ java_library( ], ) +cel_android_library( + name = "proto_json_adapter_android", + srcs = ["CelProtoJsonAdapter.java"], + tags = [ + ], + deps = [ + "//common/internal:date_time_helpers_android", + "//common/internal:proto_time_utils_android", + "//common/values:cel_byte_string", + "//common/values:values_android", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven_android//:com_google_guava_guava", + "@maven_android//:com_google_protobuf_protobuf_javalite", + ], +) + java_library( name = "source_location", srcs = ["CelSourceLocation.java"], diff --git a/common/src/main/java/dev/cel/common/internal/BUILD.bazel b/common/src/main/java/dev/cel/common/internal/BUILD.bazel index 9de68482e..9a2d56fba 100644 --- a/common/src/main/java/dev/cel/common/internal/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/internal/BUILD.bazel @@ -184,7 +184,6 @@ java_library( ":well_known_proto", "//common:options", "//common/annotations", - "//common/exceptions:numeric_overflow", "//common/values", "//common/values:cel_byte_string", "@maven//:com_google_code_findbugs_annotations", @@ -214,6 +213,26 @@ java_library( ], ) +cel_android_library( + name = "proto_lite_adapter_android", + srcs = ["ProtoLiteAdapter.java"], + tags = [ + ], + deps = [ + ":well_known_proto_android", + "//common:options", + "//common:proto_json_adapter_android", + "//common/annotations", + "//common/exceptions:numeric_overflow", + "//common/internal:proto_time_utils_android", + "//common/values:cel_byte_string", + "//common/values:values_android", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven_android//:com_google_guava_guava", + "@maven_android//:com_google_protobuf_protobuf_javalite", + ], +) + java_library( name = "proto_equality", srcs = ["ProtoEquality.java"], diff --git a/common/src/main/java/dev/cel/common/internal/DefaultLiteDescriptorPool.java b/common/src/main/java/dev/cel/common/internal/DefaultLiteDescriptorPool.java index d28da7234..d37af5729 100644 --- a/common/src/main/java/dev/cel/common/internal/DefaultLiteDescriptorPool.java +++ b/common/src/main/java/dev/cel/common/internal/DefaultLiteDescriptorPool.java @@ -49,6 +49,8 @@ @Immutable @Internal public final class DefaultLiteDescriptorPool implements CelLiteDescriptorPool { + private static final String STRUCT_FIELDS_ENTRY = "google.protobuf.Struct.FieldsEntry"; + private final ImmutableMap protoFqnToMessageInfo; private final ImmutableMap, MessageLiteDescriptor> classToMessageInfo; @@ -163,7 +165,7 @@ private static MessageLiteDescriptor newMessageInfo(WellKnownProto wellKnownProt /* encodingType= */ EncodingType.MAP, /* protoFieldType= */ FieldLiteDescriptor.Type.MESSAGE, /* isPacked= */ false, - /* fieldProtoTypeName= */ "google.protobuf.Struct.FieldsEntry")); + /* fieldProtoTypeName= */ STRUCT_FIELDS_ENTRY)); break; case JSON_VALUE: messageBuilder = Value::newBuilder; @@ -268,6 +270,14 @@ private DefaultLiteDescriptorPool(ImmutableSet descriptors) { protoFqnMapBuilder.put(wellKnownProto.typeName(), wktMessageInfo); classMapBuilder.put(wellKnownProto.messageClass(), wktMessageInfo); } + // Struct's map entry, which `google.protobuf.Struct{fields: ...}` literals are encoded with. + protoFqnMapBuilder.put( + STRUCT_FIELDS_ENTRY, + CelLiteDescriptor.mapEntry( + /* fullName= */ STRUCT_FIELDS_ENTRY, + /* keyType= */ FieldLiteDescriptor.Type.STRING, + /* valueType= */ FieldLiteDescriptor.Type.MESSAGE, + /* valueTypeName= */ WellKnownProto.JSON_VALUE.typeName())); for (CelLiteDescriptor descriptor : descriptors) { protoFqnMapBuilder.putAll(descriptor.getProtoTypeNamesToDescriptors()); diff --git a/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java b/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java index b1b56afe1..711812698 100644 --- a/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java +++ b/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java @@ -15,8 +15,9 @@ package dev.cel.common.internal; import static com.google.common.base.Preconditions.checkNotNull; +import static dev.cel.common.internal.ProtoLiteAdapter.intCheckedCast; +import static dev.cel.common.internal.ProtoLiteAdapter.unsignedIntCheckedCast; -import com.google.common.primitives.Ints; import com.google.common.primitives.UnsignedInts; import com.google.common.primitives.UnsignedLong; import com.google.errorprone.annotations.CheckReturnValue; @@ -33,7 +34,6 @@ import com.google.protobuf.MessageOrBuilder; import dev.cel.common.CelOptions; import dev.cel.common.annotations.Internal; -import dev.cel.common.exceptions.CelNumericOverflowException; import dev.cel.common.values.CelByteString; import dev.cel.common.values.NullValue; import java.util.ArrayList; @@ -308,23 +308,21 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) { case SFIXED32: case SINT32: case INT32: - return unwrapAndConvert(INT_CONVERTER); + return unwrapAndConvert(fieldDescriptor, Long.class, INT_CONVERTER); case FIXED32: case UINT32: if (celOptions.enableUnsignedLongs()) { - return unwrapAndConvert(UNSIGNED_UINT32_CONVERTER); + return unwrapAndConvert(fieldDescriptor, UnsignedLong.class, UNSIGNED_UINT32_CONVERTER); } - return unwrapAndConvert(SIGNED_UINT32_CONVERTER); + return unwrapAndConvert(fieldDescriptor, Long.class, SIGNED_UINT32_CONVERTER); case FIXED64: case UINT64: if (celOptions.enableUnsignedLongs()) { - return unwrapAndConvert(UNSIGNED_UINT64_CONVERTER); + return unwrapAndConvert(fieldDescriptor, UnsignedLong.class, UNSIGNED_UINT64_CONVERTER); } - return BidiConverter.of( - BidiConverter.IDENTITY.forwardConverter(), - value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value))); + return unwrapAndCheck(fieldDescriptor, Long.class); case FLOAT: - return unwrapAndConvert(DOUBLE_CONVERTER); + return unwrapAndConvert(fieldDescriptor, Double.class, DOUBLE_CONVERTER); case BYTES: if (celOptions.evaluateCanonicalTypesToNativeValues()) { return BidiConverter.of( @@ -336,25 +334,24 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) { BidiConverter.IDENTITY.forwardConverter(), value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value))); case DOUBLE: + return unwrapAndCheck(fieldDescriptor, Double.class); case SFIXED64: case SINT64: case INT64: + return unwrapAndCheck(fieldDescriptor, Long.class); case STRING: case BOOL: return BidiConverter.of( BidiConverter.IDENTITY.forwardConverter(), value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value))); case ENUM: - return BidiConverter.of( + return BidiConverter.of( value -> (long) ((EnumValueDescriptor) value).getNumber(), - number -> { - if (number > Integer.MAX_VALUE || number < Integer.MIN_VALUE) { - throw new IllegalArgumentException("Enum value out of int32 range: " + number); - } - return fieldDescriptor - .getEnumType() - .findValueByNumberCreatingIfUnknown(number.intValue()); - }); + value -> + fieldDescriptor + .getEnumType() + .findValueByNumberCreatingIfUnknown( + intCheckedCast((Long) unwrapNumber(fieldDescriptor, Long.class, value)))); case MESSAGE: return BidiConverter.of( this::adaptProtoToValue, @@ -401,9 +398,9 @@ public Message adaptValueToProto(Object value, String protoTypeName) { switch (wellKnownProto) { case ANY_VALUE: if (value instanceof Message) { - protoTypeName = ((Message) value).getDescriptorForType().getFullName(); + return Any.pack((Message) value); } - return protoLiteAdapter.adaptValueToAny(value, protoTypeName); + return protoLiteAdapter.adaptValueToAny(value); default: return (Message) protoLiteAdapter.adaptValueToWellKnownProto(value, wellKnownProto); } @@ -430,22 +427,6 @@ private static String typeName(Descriptor protoType) { return protoType.getFullName(); } - private static int intCheckedCast(long value) { - try { - return Ints.checkedCast(value); - } catch (IllegalArgumentException e) { - throw new CelNumericOverflowException(e); - } - } - - private static int unsignedIntCheckedCast(long value) { - try { - return UnsignedInts.checkedCast(value); - } catch (IllegalArgumentException e) { - throw new CelNumericOverflowException(e); - } - } - private Object maybeUnwrap(Object value) { if (value instanceof Message) { return adaptProtoToValue((MessageOrBuilder) value); @@ -454,9 +435,32 @@ private Object maybeUnwrap(Object value) { } private BidiConverter unwrapAndConvert( - final BidiConverter original) { + FieldDescriptor fieldDescriptor, Class celType, BidiConverter original) { return BidiConverter.of( original.forwardConverter()::convert, - value -> original.backwardConverter().convert((Number) maybeUnwrap(value))); + value -> + original + .backwardConverter() + .convert((Number) unwrapNumber(fieldDescriptor, celType, value))); + } + + private BidiConverter unwrapAndCheck( + FieldDescriptor fieldDescriptor, Class celType) { + return BidiConverter.of( + BidiConverter.IDENTITY.forwardConverter(), + value -> unwrapNumber(fieldDescriptor, celType, value)); + } + + // Per the spec, numbers aren't converted between int, uint, and double when setting a field. + private Object unwrapNumber(FieldDescriptor fieldDescriptor, Class celType, Object value) { + Object celValue = maybeUnwrap(value); + if ((celValue instanceof Long || celValue instanceof UnsignedLong || celValue instanceof Double) + && !celType.isInstance(celValue)) { + throw new IllegalArgumentException( + String.format( + "unexpected value type for field '%s' in message '%s'", + fieldDescriptor.getName(), fieldDescriptor.getContainingType().getFullName())); + } + return celValue; } } diff --git a/common/src/main/java/dev/cel/common/internal/ProtoLiteAdapter.java b/common/src/main/java/dev/cel/common/internal/ProtoLiteAdapter.java index 8131d0eec..57800ae68 100644 --- a/common/src/main/java/dev/cel/common/internal/ProtoLiteAdapter.java +++ b/common/src/main/java/dev/cel/common/internal/ProtoLiteAdapter.java @@ -32,7 +32,6 @@ import com.google.protobuf.Int32Value; import com.google.protobuf.Int64Value; import com.google.protobuf.ListValue; -import com.google.protobuf.Message; import com.google.protobuf.MessageLite; import com.google.protobuf.MessageLiteOrBuilder; import com.google.protobuf.NullValue; @@ -110,11 +109,7 @@ public MessageLite adaptValueToWellKnownProto(Object value, WellKnownProto wellK } } - public Any adaptValueToAny(Object value, String typeName) { - if (value instanceof MessageLite) { - return packAnyMessage((MessageLite) value, typeName); - } - + public Any adaptValueToAny(Object value) { if (value instanceof dev.cel.common.values.NullValue) { return packAnyMessage( Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build(), WellKnownProto.JSON_VALUE); @@ -234,7 +229,7 @@ private ImmutableMap adaptJsonStructToValue(Struct struct) { .collect(toImmutableMap(Entry::getKey, e -> adaptJsonToValue(e.getValue()))); } - private Message adaptValueToDouble(Object value) { + private MessageLite adaptValueToDouble(Object value) { if (value instanceof Double) { return DoubleValue.of((Double) value); } @@ -244,7 +239,7 @@ private Message adaptValueToDouble(Object value) { throw new IllegalArgumentException("Unexpected value type: " + value); } - private Message adaptValueToFloat(Object value) { + private MessageLite adaptValueToFloat(Object value) { if (value instanceof Double) { return FloatValue.of(((Double) value).floatValue()); } @@ -254,7 +249,7 @@ private Message adaptValueToFloat(Object value) { throw new IllegalArgumentException("Unexpected value type: " + value); } - private Message adaptValueToInt32(Object value) { + private MessageLite adaptValueToInt32(Object value) { if (value instanceof Integer) { return Int32Value.of((Integer) value); } @@ -264,7 +259,7 @@ private Message adaptValueToInt32(Object value) { throw new IllegalArgumentException("Unexpected value type: " + value); } - private Message adaptValueToInt64(Object value) { + private MessageLite adaptValueToInt64(Object value) { if (value instanceof Integer) { return Int64Value.of(((Integer) value).longValue()); } @@ -275,30 +270,22 @@ private Message adaptValueToInt64(Object value) { throw new IllegalArgumentException("Unexpected value type: " + value); } - private Message adaptValueToUint32(Object value) { + private MessageLite adaptValueToUint32(Object value) { if (value instanceof Integer) { return UInt32Value.of((Integer) value); } // Per the spec, an int isn't a uint, unless unsigned longs are disabled. if (!celOptions.enableUnsignedLongs() && value instanceof Long) { - try { - return UInt32Value.of(unsignedIntCheckedCast((Long) value)); - } catch (IllegalArgumentException e) { - throw new CelNumericOverflowException(e); - } + return UInt32Value.of(unsignedIntCheckedCast((Long) value)); } if (value instanceof UnsignedLong) { - try { - return UInt32Value.of(unsignedIntCheckedCast(((UnsignedLong) value).longValue())); - } catch (IllegalArgumentException e) { - throw new CelNumericOverflowException(e); - } + return UInt32Value.of(unsignedIntCheckedCast(((UnsignedLong) value).longValue())); } throw new IllegalArgumentException("Unexpected value type: " + value); } - private Message adaptValueToUint64(Object value) { + private MessageLite adaptValueToUint64(Object value) { if (value instanceof Integer) { return UInt64Value.of(UnsignedInts.toLong((Integer) value)); } @@ -312,7 +299,8 @@ private Message adaptValueToUint64(Object value) { throw new IllegalArgumentException("Unexpected value type: " + value); } - private static int intCheckedCast(long value) { + /** Narrows {@code value} to an int32, or throws {@link CelNumericOverflowException}. */ + public static int intCheckedCast(long value) { try { return Ints.checkedCast(value); } catch (IllegalArgumentException e) { @@ -320,7 +308,8 @@ private static int intCheckedCast(long value) { } } - private static int unsignedIntCheckedCast(long value) { + /** Narrows {@code value} to a uint32, or throws {@link CelNumericOverflowException}. */ + public static int unsignedIntCheckedCast(long value) { try { return UnsignedInts.checkedCast(value); } catch (IllegalArgumentException e) { @@ -329,14 +318,12 @@ private static int unsignedIntCheckedCast(long value) { } private static Any packAnyMessage(MessageLite msg, WellKnownProto wellKnownProto) { - return packAnyMessage(msg, wellKnownProto.typeName()); + return packAny(msg.toByteString(), wellKnownProto.typeName()); } - private static Any packAnyMessage(MessageLite msg, String typeUrl) { - return Any.newBuilder() - .setValue(msg.toByteString()) - .setTypeUrl("type.googleapis.com/" + typeUrl) - .build(); + /** Packs {@code value}, a serialized message of type {@code typeName}, into an {@link Any}. */ + public static Any packAny(ByteString value, String typeName) { + return Any.newBuilder().setValue(value).setTypeUrl("type.googleapis.com/" + typeName).build(); } private Timestamp adaptValueToProtoTimestamp(Object value) { 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 53d3788bc..4f158f8d6 100644 --- a/common/src/main/java/dev/cel/common/values/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/values/BUILD.bazel @@ -393,10 +393,16 @@ java_library( deps = [ ":base_proto_message_value_provider", ":proto_message_lite_value", + "//common:options", "//common/annotations", "//common/internal:cel_lite_descriptor_pool", "//common/internal:default_lite_descriptor_pool", + "//common/internal:proto_lite_adapter", + "//common/internal:proto_time_utils", + "//common/internal:well_known_proto", + "//common/values", "//common/values:base_proto_cel_value_converter", + "//common/values:cel_byte_string", "//protobuf:cel_lite_descriptor", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", @@ -412,10 +418,16 @@ cel_android_library( deps = [ ":base_proto_message_value_provider_android", ":proto_message_lite_value_android", + "//common:options", "//common/annotations", "//common/internal:cel_lite_descriptor_pool_android", "//common/internal:default_lite_descriptor_pool_android", + "//common/internal:proto_lite_adapter_android", + "//common/internal:proto_time_utils_android", + "//common/internal:well_known_proto_android", "//common/values:base_proto_cel_value_converter_android", + "//common/values:cel_byte_string", + "//common/values:values_android", "//protobuf:cel_lite_descriptor", "@maven//:com_google_errorprone_error_prone_annotations", "@maven_android//:com_google_guava_guava", 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 40d74924d..7033bf323 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java @@ -62,8 +62,8 @@ public final class ProtoLiteCelValueConverter extends BaseProtoCelValueConverter { private static final String MAP_KEY_FIELD_NAME = "key"; private static final String MAP_VALUE_FIELD_NAME = "value"; - private static final int MAP_KEY_FIELD_NUMBER = 1; - private static final int MAP_VALUE_FIELD_NUMBER = 2; + static final int MAP_KEY_FIELD_NUMBER = 1; + static final int MAP_VALUE_FIELD_NUMBER = 2; // Protobuf's default recursion limit. Nested Anys are unwrapped in a loop since each payload is // parsed afresh, outside that limit. private static final int MAX_ANY_NESTING_DEPTH = 100; @@ -145,7 +145,8 @@ private Object mergeOrReadMessageField( ByteString bytes, String fieldProtoTypeName, @Nullable Object existingValue, boolean isView) { MessageLiteDescriptor descriptor = descriptorPool.findDescriptor(fieldProtoTypeName).orElse(null); - if (descriptor == null) { + // An Any can name a map entry, which can't be built. + if (descriptor == null || descriptor.isMapEntry()) { if (existingValue instanceof RawProtoMessageLiteValue) { bytes = ((RawProtoMessageLiteValue) existingValue).toByteString().concat(bytes); } @@ -166,7 +167,7 @@ private Object mergeOrReadMessageField( // Unlike other WellKnownProtos (which unbox to CEL scalars/containers), FieldMask is // represented as a standard struct message so field selection (e.g., mask.paths) works. - private static boolean isStructLike(@Nullable WellKnownProto wellKnownProto) { + static boolean isStructLike(@Nullable WellKnownProto wellKnownProto) { return wellKnownProto == null || wellKnownProto == WellKnownProto.FIELD_MASK; } @@ -188,7 +189,7 @@ static MessageLite parseMessageLite(ByteString bytes, MessageLiteDescriptor desc return mergeMessageLite(descriptor.newMessageBuilder(), bytes, descriptor.getProtoTypeName()); } - private static MessageLite mergeMessageLite( + static MessageLite mergeMessageLite( MessageLite.Builder builder, ByteString bytes, String protoTypeName) { try { return builder.mergeFrom(bytes, ExtensionRegistryLite.getEmptyRegistry()).build(); diff --git a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValueProvider.java b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValueProvider.java index 041d850a6..bb6255482 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValueProvider.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValueProvider.java @@ -14,27 +14,47 @@ package dev.cel.common.values; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.common.collect.ImmutableSet; +import com.google.common.primitives.UnsignedLong; import com.google.errorprone.annotations.Immutable; +import com.google.protobuf.ByteString; +import com.google.protobuf.CodedOutputStream; import com.google.protobuf.MessageLite; +import dev.cel.common.CelOptions; import dev.cel.common.annotations.Beta; import dev.cel.common.internal.CelLiteDescriptorPool; import dev.cel.common.internal.DefaultLiteDescriptorPool; +import dev.cel.common.internal.ProtoLiteAdapter; +import dev.cel.common.internal.ProtoTimeUtils; +import dev.cel.common.internal.WellKnownProto; import dev.cel.protobuf.CelLiteDescriptor; +import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor; +import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor.EncodingType; import dev.cel.protobuf.CelLiteDescriptor.MessageLiteDescriptor; +import java.io.IOException; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; /** - * {@code ProtoMessageValueProvider} constructs new instances of protobuf lite-message given its + * {@code ProtoMessageLiteValueProvider} constructs new instances of protobuf lite-message given its * fully qualified name and its fields to populate. + * + *

As when parsing protobuf, a number outside a closed (proto2) enum's values is kept as an + * unknown field, so the field reads back as its default. */ @Immutable @Beta public class ProtoMessageLiteValueProvider extends BaseProtoMessageValueProvider { + // Smaller than CodedOutputStream's default of 4KB, as most messages and map entries are small. + private static final int OUTPUT_BUFFER_SIZE = 256; + private final CelLiteDescriptorPool descriptorPool; private final ProtoLiteCelValueConverter protoLiteCelValueConverter; + private final ProtoLiteAdapter protoLiteAdapter; @Override public BaseProtoCelValueConverter protoCelValueConverter() { @@ -44,20 +64,233 @@ public BaseProtoCelValueConverter protoCelValueConverter() { @Override public Optional newValue(String structType, Map fields) { MessageLiteDescriptor descriptor = descriptorPool.findDescriptor(structType).orElse(null); - if (descriptor == null) { + // Like unknown types, map entries can't be created, as they aren't messages in CEL. + if (descriptor == null || descriptor.isMapEntry()) { return Optional.empty(); } - if (!fields.isEmpty()) { - // TODO: Add support for this - throw new UnsupportedOperationException( - "Message creation with prepopulated fields is not supported yet."); + // Encoded for the generated message's parser to read, so CelLiteDescriptor needs no setters. + ByteString.Output bytes = ByteString.newOutput(); + CodedOutputStream output = CodedOutputStream.newInstance(bytes, OUTPUT_BUFFER_SIZE); + try { + for (Map.Entry field : fields.entrySet()) { + FieldLiteDescriptor fd = descriptor.findByFieldName(field.getKey()); + if (fd == null) { + throw new IllegalArgumentException( + String.format( + "field '%s' is not declared in message '%s'", field.getKey(), structType)); + } + try { + writeField(output, fd, field.getValue()); + } catch (ClassCastException e) { + throw new IllegalArgumentException( + String.format( + "unexpected value type for field '%s' in message '%s'", + field.getKey(), structType), + e); + } + } + output.flush(); + } catch (IOException e) { + // Unreachable, as the output is in memory. + throw new AssertionError(e); } - - MessageLite message = descriptor.newMessageBuilder().build(); + // build() fails on a missing proto2 required field as in the full runtime, unlike + // parseMessageLite for empty bytes. + MessageLite message = + ProtoLiteCelValueConverter.mergeMessageLite( + descriptor.newMessageBuilder(), bytes.toByteString(), structType); + // As in the full runtime, out-of-range Duration and Timestamp literals are errors. + ProtoTimeUtils.validateWellKnownProto(message); return Optional.of(protoLiteCelValueConverter.toRuntimeValue(message)); } + private void writeField(CodedOutputStream output, FieldLiteDescriptor fd, Object value) + throws IOException { + if (value instanceof NullValue) { + // As in ProtoAdapter: null unsets a message field, is JSON null in Any and Value, else fails. + String typeName = fd.getFieldProtoTypeName(); + if (fd.getEncodingType() != EncodingType.SINGULAR + || fd.getProtoFieldType() != FieldLiteDescriptor.Type.MESSAGE + || typeName.equals(WellKnownProto.JSON_STRUCT_VALUE.typeName()) + || typeName.equals(WellKnownProto.JSON_LIST_VALUE.typeName())) { + throw new IllegalArgumentException( + String.format("null cannot be assigned to field '%s'", fd.getFieldName())); + } + if (!holdsJsonNull(typeName)) { + return; + } + } + switch (fd.getEncodingType()) { + case SINGULAR: + writeValue(output, fd, value); + break; + case LIST: + // Unpacked, which parsers accept for packed fields too. As in ProtoAdapter, nulls are + // dropped from lists of anything but Any and Value. + boolean keepsNulls = holdsJsonNull(fd.getFieldProtoTypeName()); + for (Object element : (List) value) { + if (keepsNulls || !(element instanceof NullValue)) { + writeValue(output, fd, element); + } + } + break; + case MAP: + writeMap(output, fd, (Map) value); + break; + } + } + + private void writeMap(CodedOutputStream output, FieldLiteDescriptor fd, Map map) + throws IOException { + MessageLiteDescriptor entry = descriptorPool.getDescriptorOrThrow(fd.getFieldProtoTypeName()); + FieldLiteDescriptor keyFd = + entry.getByFieldNumberOrThrow(ProtoLiteCelValueConverter.MAP_KEY_FIELD_NUMBER); + FieldLiteDescriptor valueFd = + entry.getByFieldNumberOrThrow(ProtoLiteCelValueConverter.MAP_VALUE_FIELD_NUMBER); + // As in ProtoAdapter, entries with null values are dropped unless the values are Any or Value. + boolean keepsNulls = holdsJsonNull(valueFd.getFieldProtoTypeName()); + // One stream encodes every entry, as each new one allocates a buffer. + ByteString.Output entryBytes = ByteString.newOutput(); + CodedOutputStream entryOutput = CodedOutputStream.newInstance(entryBytes, OUTPUT_BUFFER_SIZE); + for (Map.Entry mapEntry : map.entrySet()) { + if (keepsNulls || !(mapEntry.getValue() instanceof NullValue)) { + writeValue(entryOutput, keyFd, mapEntry.getKey()); + writeValue(entryOutput, valueFd, mapEntry.getValue()); + entryOutput.flush(); + output.writeBytes(fd.getFieldNumber(), entryBytes.toByteString()); + entryBytes.reset(); + } + } + } + + private void writeValue(CodedOutputStream output, FieldLiteDescriptor fd, Object value) + throws IOException { + int number = fd.getFieldNumber(); + switch (fd.getProtoFieldType()) { + case MESSAGE: + writeMessage(output, number, fd.getFieldProtoTypeName(), value); + break; + case DOUBLE: + output.writeDouble(number, (Double) value); + break; + case FLOAT: + output.writeFloat(number, ((Double) value).floatValue()); + break; + case INT64: + output.writeInt64(number, (Long) value); + break; + case SINT64: + output.writeSInt64(number, (Long) value); + break; + case SFIXED64: + output.writeSFixed64(number, (Long) value); + break; + case INT32: + case ENUM: + // An enum is encoded as an int32, whose range CEL also requires of it. + output.writeInt32(number, ProtoLiteAdapter.intCheckedCast((Long) value)); + break; + case SINT32: + output.writeSInt32(number, ProtoLiteAdapter.intCheckedCast((Long) value)); + break; + case SFIXED32: + output.writeSFixed32(number, ProtoLiteAdapter.intCheckedCast((Long) value)); + break; + case UINT64: + output.writeUInt64(number, ((UnsignedLong) value).longValue()); + break; + case FIXED64: + output.writeFixed64(number, ((UnsignedLong) value).longValue()); + break; + case UINT32: + output.writeUInt32( + number, ProtoLiteAdapter.unsignedIntCheckedCast(((UnsignedLong) value).longValue())); + break; + case FIXED32: + output.writeFixed32( + number, ProtoLiteAdapter.unsignedIntCheckedCast(((UnsignedLong) value).longValue())); + break; + case BOOL: + output.writeBool(number, (Boolean) value); + break; + case STRING: + output.writeString(number, (String) value); + break; + case BYTES: + output.writeByteArray(number, ((CelByteString) value).toByteArray()); + break; + default: + throw new UnsupportedOperationException("Groups are not supported"); + } + } + + // MessageLite values are serialized straight into the output, without an intermediate ByteString. + private void writeMessage( + CodedOutputStream output, int number, String protoTypeName, Object value) throws IOException { + WellKnownProto wellKnownProto = WellKnownProto.getByTypeName(protoTypeName).orElse(null); + if (ProtoLiteCelValueConverter.isStructLike(wellKnownProto)) { + // Fails closed on a message of another type, which only dyn or an unchecked AST can assign. + String valueTypeName = protoTypeNameOf(value); + checkArgument( + valueTypeName.equals(protoTypeName), + "expected a message of type '%s', got '%s'", + protoTypeName, + valueTypeName); + if (value instanceof WireMessageLite) { + output.writeBytes(number, ((WireMessageLite) value).toByteString()); + } else { + output.writeMessage(number, (MessageLite) value); + } + return; + } + switch (wellKnownProto) { + case ANY_VALUE: + if (!(value instanceof MessageLite || value instanceof WireMessageLite)) { + output.writeMessage(number, protoLiteAdapter.adaptValueToAny(value)); + return; + } + String valueTypeName = protoTypeNameOf(value); + checkArgument( + !valueTypeName.equals(RawProtoMessageLiteValue.UNKNOWN_MESSAGE_TYPE_NAME), + "cannot pack a message of unknown type into google.protobuf.Any"); + output.writeMessage(number, ProtoLiteAdapter.packAny(toByteString(value), valueTypeName)); + return; + case EMPTY: + // Values of google.protobuf.Empty are empty maps in CEL. + checkArgument( + ((Map) value).isEmpty(), "expected an empty map for google.protobuf.Empty"); + output.writeBytes(number, ByteString.EMPTY); + return; + default: + output.writeMessage( + number, protoLiteAdapter.adaptValueToWellKnownProto(value, wellKnownProto)); + } + } + + // A message's type name, or the unknown-message sentinel if its class isn't registered. + private String protoTypeNameOf(Object message) { + if (message instanceof WireMessageLite) { + return ((WireMessageLite) message).protoTypeName(); + } + return descriptorPool + .findDescriptor((MessageLite) message) + .map(MessageLiteDescriptor::getProtoTypeName) + .orElse(RawProtoMessageLiteValue.UNKNOWN_MESSAGE_TYPE_NAME); + } + + private static ByteString toByteString(Object message) { + return message instanceof WireMessageLite + ? ((WireMessageLite) message).toByteString() + : ((MessageLite) message).toByteString(); + } + + // Whether null is a JSON value of this type, rather than unsetting a field of it. + private static boolean holdsJsonNull(String protoTypeName) { + return protoTypeName.equals(WellKnownProto.ANY_VALUE.typeName()) + || protoTypeName.equals(WellKnownProto.JSON_VALUE.typeName()); + } + public static ProtoMessageLiteValueProvider newInstance(CelLiteDescriptor... descriptors) { return newInstance(ImmutableSet.copyOf(descriptors)); } @@ -74,5 +307,7 @@ private ProtoMessageLiteValueProvider( ProtoLiteCelValueConverter protoLiteCelValueConverter, CelLiteDescriptorPool descriptorPool) { this.protoLiteCelValueConverter = protoLiteCelValueConverter; this.descriptorPool = descriptorPool; + // Lite values are always native (e.g. java.time, UnsignedLong), as CelOptions.DEFAULT expects. + this.protoLiteAdapter = new ProtoLiteAdapter(CelOptions.DEFAULT); } } 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 81643d46c..816e87015 100644 --- a/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java +++ b/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java @@ -41,7 +41,7 @@ abstract class RawProtoMessageLiteValue extends StructValue implements OptimizedSelectable, WireMessageLite { - private static final String UNKNOWN_MESSAGE_TYPE_NAME = "cel.@unknownMessage"; + static final String UNKNOWN_MESSAGE_TYPE_NAME = "cel.@unknownMessage"; @Override public abstract ByteString toByteString(); diff --git a/common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java b/common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java index be7e4f042..279787537 100644 --- a/common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java +++ b/common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java @@ -239,6 +239,22 @@ public void adaptValueToProto_unsignedLongsDisabled_acceptsSignedUintWrappers() .isEqualTo(UInt64Value.of(1)); } + @Test + @SuppressWarnings("deprecation") // Signed uints are only enabled through a deprecated option. + public void adaptValueToFieldType_unsignedLongsDisabled_acceptsSignedUints() { + ProtoAdapter protoAdapter = + new ProtoAdapter(DYNAMIC_PROTO, CelOptions.current().enableUnsignedLongs(false).build()); + + assertThat( + protoAdapter.adaptValueToFieldType( + UInt32Value.getDescriptor().findFieldByName("value"), 1L)) + .hasValue(1); + assertThat( + protoAdapter.adaptValueToFieldType( + UInt64Value.getDescriptor().findFieldByName("value"), 1L)) + .hasValue(1L); + } + @Test public void adaptValueToProto_asymmetricJsonConversion() { ProtoAdapter protoAdapter = new ProtoAdapter(DYNAMIC_PROTO, CelOptions.DEFAULT); diff --git a/common/src/test/java/dev/cel/common/values/BUILD.bazel b/common/src/test/java/dev/cel/common/values/BUILD.bazel index 3d7f61df1..338091e9e 100644 --- a/common/src/test/java/dev/cel/common/values/BUILD.bazel +++ b/common/src/test/java/dev/cel/common/values/BUILD.bazel @@ -18,6 +18,7 @@ java_library( "//common:options", "//common/exceptions:attribute_not_found", "//common/exceptions:invalid_argument", + "//common/exceptions:numeric_overflow", "//common/internal:cel_descriptor_pools", "//common/internal:cel_lite_descriptor_pool", "//common/internal:default_lite_descriptor_pool", @@ -49,6 +50,7 @@ java_library( "@maven//:com_google_guava_guava_testlib", "@maven//:com_google_protobuf_protobuf_java", "@maven//:com_google_testparameterinjector_test_parameter_injector", + "@maven//:com_google_truth_extensions_truth_proto_extension", "@maven//:junit_junit", "@maven_android//:com_google_protobuf_protobuf_javalite", ], 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 f4c80a862..7cb9bef36 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java @@ -210,6 +210,18 @@ public void toRuntimeValue_anyWithUnregisteredMessage_returnsRawValueOfPackedTyp assertThat(rawValue.toByteString()).isEqualTo(msg.toByteString()); } + @Test + public void toRuntimeValue_anyWithMapEntry_returnsRawValueOfPackedType() { + String mapEntryTypeName = PROTO3_TEST_ALL_TYPES + ".MapStringStringEntry"; + Any any = Any.newBuilder().setTypeUrl("type.googleapis.com/" + mapEntryTypeName).build(); + + Object adaptedValue = PROTO_LITE_CEL_VALUE_CONVERTER.toRuntimeValue(any); + + assertThat(adaptedValue).isInstanceOf(RawProtoMessageLiteValue.class); + assertThat(((RawProtoMessageLiteValue) adaptedValue).protoTypeName()) + .isEqualTo(mapEntryTypeName); + } + @Test public void toRuntimeValue_anyNestedToMaxDepth_unpacks() { Object adaptedValue = PROTO_LITE_CEL_VALUE_CONVERTER.toRuntimeValue(nestedAny(100)); diff --git a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueProviderTest.java b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueProviderTest.java index bbe116c80..b58324001 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueProviderTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueProviderTest.java @@ -15,40 +15,336 @@ package dev.cel.common.values; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat; +import static org.junit.Assert.assertThrows; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.primitives.UnsignedLong; +import com.google.protobuf.Any; +import com.google.protobuf.ByteString; +import com.google.protobuf.Empty; +import com.google.protobuf.FieldMask; +import com.google.protobuf.Int64Value; +import com.google.protobuf.Struct; +import com.google.protobuf.UInt32Value; +import com.google.protobuf.UninitializedMessageException; +import com.google.protobuf.Value; +import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; +import dev.cel.common.exceptions.CelNumericOverflowException; +import dev.cel.common.internal.DefaultLiteDescriptorPool; +import dev.cel.common.internal.ProtoTimeUtils; import dev.cel.common.types.StructTypeReference; +import dev.cel.expr.conformance.proto2.TestRequiredCelDescriptor; +import dev.cel.expr.conformance.proto3.NestedTestAllTypes; import dev.cel.expr.conformance.proto3.TestAllTypes; +import dev.cel.expr.conformance.proto3.TestAllTypes.NestedEnum; +import dev.cel.expr.conformance.proto3.TestAllTypes.NestedMessage; import dev.cel.expr.conformance.proto3.TestAllTypesCelDescriptor; +import java.time.Instant; +import java.util.Map; import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @RunWith(TestParameterInjector.class) public class ProtoMessageLiteValueProviderTest { + private static final String TEST_ALL_TYPES = "cel.expr.conformance.proto3.TestAllTypes"; + private static final NestedMessage NESTED = NestedMessage.newBuilder().setBb(7).build(); + private static final Value JSON_NULL = + Value.newBuilder().setNullValue(com.google.protobuf.NullValue.NULL_VALUE).build(); + private static final ProtoLiteCelValueConverter EMPTY_CONVERTER = + ProtoLiteCelValueConverter.newInstance(DefaultLiteDescriptorPool.newInstance()); + private static final WireMessageLite WIRE_NESTED = + RawProtoMessageLiteValue.create( + NESTED.toByteString(), + "cel.expr.conformance.proto3.TestAllTypes.NestedMessage", + EMPTY_CONVERTER); + private static final WireMessageLite WIRE_OF_UNKNOWN_TYPE = + RawProtoMessageLiteValue.create(NESTED.toByteString(), EMPTY_CONVERTER); private static final ProtoMessageLiteValueProvider VALUE_PROVIDER = - ProtoMessageLiteValueProvider.newInstance(TestAllTypesCelDescriptor.getDescriptor()); + ProtoMessageLiteValueProvider.newInstance( + TestAllTypesCelDescriptor.getDescriptor(), TestRequiredCelDescriptor.getDescriptor()); @Test - public void newValue_unknownType_returnsEmpty() { - assertThat(VALUE_PROVIDER.newValue("unknownType", ImmutableMap.of())).isEmpty(); + public void newValue_unknownOrMapEntryType_returnsEmpty( + @TestParameter({ + "unknownType", + "google.protobuf.Struct.FieldsEntry", + "cel.expr.conformance.proto3.TestAllTypes.MapStringStringEntry" + }) + String structType) { + assertThat(VALUE_PROVIDER.newValue(structType, ImmutableMap.of())).isEmpty(); } @Test public void newValue_emptyFields_success() { - Optional value = - VALUE_PROVIDER.newValue("cel.expr.conformance.proto3.TestAllTypes", ImmutableMap.of()); + Optional value = VALUE_PROVIDER.newValue(TEST_ALL_TYPES, ImmutableMap.of()); ProtoMessageLiteValue protoMessageLiteValue = (ProtoMessageLiteValue) value.get(); assertThat(protoMessageLiteValue.value()).isEqualTo(TestAllTypes.getDefaultInstance()); assertThat(protoMessageLiteValue.isZeroValue()).isTrue(); assertThat(protoMessageLiteValue.celType()) - .isEqualTo(StructTypeReference.create("cel.expr.conformance.proto3.TestAllTypes")); + .isEqualTo(StructTypeReference.create(TEST_ALL_TYPES)); + } + + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum FieldTestCase { + INT32("single_int32", -1L, TestAllTypes.newBuilder().setSingleInt32(-1)), + INT64("single_int64", Long.MIN_VALUE, TestAllTypes.newBuilder().setSingleInt64(Long.MIN_VALUE)), + UINT32( + "single_uint32", + UnsignedLong.valueOf(4294967295L), + TestAllTypes.newBuilder().setSingleUint32(-1)), + UINT64("single_uint64", UnsignedLong.MAX_VALUE, TestAllTypes.newBuilder().setSingleUint64(-1L)), + SINT32("single_sint32", -2L, TestAllTypes.newBuilder().setSingleSint32(-2)), + SINT64("single_sint64", -3L, TestAllTypes.newBuilder().setSingleSint64(-3L)), + FIXED32( + "single_fixed32", + UnsignedLong.valueOf(4294967295L), + TestAllTypes.newBuilder().setSingleFixed32(-1)), + FIXED64( + "single_fixed64", UnsignedLong.MAX_VALUE, TestAllTypes.newBuilder().setSingleFixed64(-1L)), + SFIXED32("single_sfixed32", -4L, TestAllTypes.newBuilder().setSingleSfixed32(-4)), + SFIXED64("single_sfixed64", -5L, TestAllTypes.newBuilder().setSingleSfixed64(-5L)), + FLOAT("single_float", 1.5d, TestAllTypes.newBuilder().setSingleFloat(1.5f)), + FLOAT_OVERFLOW( + "single_float", 1.4e55d, TestAllTypes.newBuilder().setSingleFloat(Float.POSITIVE_INFINITY)), + DOUBLE("single_double", -2.5d, TestAllTypes.newBuilder().setSingleDouble(-2.5d)), + BOOL("single_bool", true, TestAllTypes.newBuilder().setSingleBool(true)), + STRING("single_string", "héllo", TestAllTypes.newBuilder().setSingleString("héllo")), + BYTES( + "single_bytes", + CelByteString.copyFromUtf8("abc"), + TestAllTypes.newBuilder().setSingleBytes(ByteString.copyFromUtf8("abc"))), + ENUM("standalone_enum", 2L, TestAllTypes.newBuilder().setStandaloneEnum(NestedEnum.BAZ)), + OPEN_ENUM_UNKNOWN_VALUE( + "standalone_enum", 99L, TestAllTypes.newBuilder().setStandaloneEnumValue(99)), + IMPLICIT_PRESENCE_DEFAULT("single_int32", 0L, TestAllTypes.newBuilder()), + EXPLICIT_PRESENCE_DEFAULT( + "optional_bool", false, TestAllTypes.newBuilder().setOptionalBool(false)), + MESSAGE( + "single_nested_message", NESTED, TestAllTypes.newBuilder().setSingleNestedMessage(NESTED)), + WIRE_MESSAGE( + "single_nested_message", + WIRE_NESTED, + TestAllTypes.newBuilder().setSingleNestedMessage(NESTED)), + NULL_MESSAGE("single_nested_message", NullValue.NULL_VALUE, TestAllTypes.newBuilder()), + NULL_VALUE( + "single_value", NullValue.NULL_VALUE, TestAllTypes.newBuilder().setSingleValue(JSON_NULL)), + NULL_ANY( + "single_any", + NullValue.NULL_VALUE, + TestAllTypes.newBuilder().setSingleAny(Any.pack(JSON_NULL))), + WRAPPER( + "single_int64_wrapper", + 5L, + TestAllTypes.newBuilder().setSingleInt64Wrapper(Int64Value.of(5))), + UINT32_WRAPPER( + "single_uint32_wrapper", + UnsignedLong.valueOf(4294967295L), + TestAllTypes.newBuilder().setSingleUint32Wrapper(UInt32Value.of(-1))), + ANY_MESSAGE("single_any", NESTED, TestAllTypes.newBuilder().setSingleAny(Any.pack(NESTED))), + ANY_WIRE_MESSAGE( + "single_any", WIRE_NESTED, TestAllTypes.newBuilder().setSingleAny(Any.pack(NESTED))), + ANY_PRIMITIVE( + "single_any", 1L, TestAllTypes.newBuilder().setSingleAny(Any.pack(Int64Value.of(1)))), + JSON_STRUCT( + "single_struct", + ImmutableMap.of("a", true), + TestAllTypes.newBuilder() + .setSingleStruct( + Struct.newBuilder().putFields("a", Value.newBuilder().setBoolValue(true).build()))), + FIELD_MASK( + "field_mask", + FieldMask.newBuilder().addPaths("a.b").build(), + TestAllTypes.newBuilder().setFieldMask(FieldMask.newBuilder().addPaths("a.b"))), + EMPTY( + "empty", ImmutableMap.of(), TestAllTypes.newBuilder().setEmpty(Empty.getDefaultInstance())), + REPEATED_SCALAR( + "repeated_int32", + ImmutableList.of(1L, -2L), + TestAllTypes.newBuilder().addRepeatedInt32(1).addRepeatedInt32(-2)), + REPEATED_MESSAGE_DROPS_NULLS( + "repeated_nested_message", + ImmutableList.of(NESTED, NullValue.NULL_VALUE), + TestAllTypes.newBuilder().addRepeatedNestedMessage(NESTED)), + REPEATED_ANY_KEEPS_NULLS( + "repeated_any", + ImmutableList.of(1L, NullValue.NULL_VALUE), + TestAllTypes.newBuilder() + .addRepeatedAny(Any.pack(Int64Value.of(1))) + .addRepeatedAny(Any.pack(JSON_NULL))), + MAP( + "map_string_message", + ImmutableMap.of("a", NESTED, "b", NestedMessage.getDefaultInstance()), + TestAllTypes.newBuilder() + .putMapStringMessage("a", NESTED) + .putMapStringMessage("b", NestedMessage.getDefaultInstance())), + MAP_DROPS_NULLS( + "map_bool_timestamp", + ImmutableMap.of(true, NullValue.NULL_VALUE, false, Instant.EPOCH), + TestAllTypes.newBuilder().putMapBoolTimestamp(false, ProtoTimeUtils.TIMESTAMP_EPOCH)), + MAP_VALUE_KEEPS_NULLS( + "map_string_value", + ImmutableMap.of("k", NullValue.NULL_VALUE), + TestAllTypes.newBuilder().putMapStringValue("k", JSON_NULL)), + ONEOF_MESSAGE_LAST( + ImmutableMap.of("oneof_bool", true, "oneof_msg", NESTED), + TestAllTypes.newBuilder().setOneofMsg(NESTED)), + ONEOF_BOOL_LAST( + ImmutableMap.of("oneof_msg", NESTED, "oneof_bool", true), + TestAllTypes.newBuilder().setOneofBool(true)); + + private final ImmutableMap fields; + private final TestAllTypes expected; + + FieldTestCase(String fieldName, Object value, TestAllTypes.Builder expected) { + this(ImmutableMap.of(fieldName, value), expected); + } + + FieldTestCase(ImmutableMap fields, TestAllTypes.Builder expected) { + this.fields = fields; + this.expected = expected.build(); + } + } + + @Test + public void newValue_fields_success(@TestParameter FieldTestCase testCase) { + TestAllTypes message = newTestAllTypes(testCase.fields); + + assertThat(message).isEqualTo(testCase.expected); + } + + @Test + public void newValue_structLiteral_returnsMap() { + ImmutableMap fields = ImmutableMap.of("fields", ImmutableMap.of("a", 1.0d)); + + assertThat(VALUE_PROVIDER.newValue("google.protobuf.Struct", fields)) + .hasValue(ImmutableMap.of("a", 1.0d)); + } + + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum InvalidFieldTestCase { + UNDECLARED_FIELD("undeclared", 1L, IllegalArgumentException.class, "is not declared"), + INT32_OVERFLOW("single_int32", 1L << 31, CelNumericOverflowException.class, "Out of range"), + SINT32_UNDERFLOW( + "single_sint32", -(1L << 31) - 1, CelNumericOverflowException.class, "Out of range"), + SFIXED32_OVERFLOW( + "single_sfixed32", 1L << 31, CelNumericOverflowException.class, "Out of range"), + UINT32_OVERFLOW( + "single_uint32", + UnsignedLong.valueOf(1L << 32), + CelNumericOverflowException.class, + "out of range"), + FIXED32_OVERFLOW( + "single_fixed32", + UnsignedLong.valueOf(1L << 32), + CelNumericOverflowException.class, + "out of range"), + NULL_SCALAR( + "single_int32", + NullValue.NULL_VALUE, + IllegalArgumentException.class, + "null cannot be assigned"), + NULL_MAP( + "map_string_string", + NullValue.NULL_VALUE, + IllegalArgumentException.class, + "null cannot be assigned"), + NULL_LIST( + "repeated_nested_message", + NullValue.NULL_VALUE, + IllegalArgumentException.class, + "null cannot be assigned"), + NULL_STRUCT( + "single_struct", + NullValue.NULL_VALUE, + IllegalArgumentException.class, + "null cannot be assigned"), + NULL_LIST_VALUE( + "list_value", + NullValue.NULL_VALUE, + IllegalArgumentException.class, + "null cannot be assigned"), + MESSAGE_OF_ANOTHER_TYPE( + "single_nested_message", + TestAllTypes.getDefaultInstance(), + IllegalArgumentException.class, + "expected a message of type"), + UNREGISTERED_MESSAGE_CLASS( + "single_nested_message", + NestedTestAllTypes.getDefaultInstance(), + IllegalArgumentException.class, + "expected a message of type"), + WIRE_MESSAGE_OF_UNKNOWN_TYPE( + "single_nested_message", + WIRE_OF_UNKNOWN_TYPE, + IllegalArgumentException.class, + "expected a message of type"), + FIELD_MASK_OF_ANOTHER_TYPE( + "field_mask", NESTED, IllegalArgumentException.class, "expected a message of type"), + ANY_OF_UNKNOWN_TYPE( + "single_any", WIRE_OF_UNKNOWN_TYPE, IllegalArgumentException.class, "unknown type"), + NON_EMPTY_MAP_AS_EMPTY( + "empty", ImmutableMap.of("a", 1L), IllegalArgumentException.class, "expected an empty map"), + SCALAR_OF_ANOTHER_TYPE( + "single_int64", "a", IllegalArgumentException.class, "unexpected value type"); + + private final String fieldName; + private final Object value; + private final Class expectedException; + private final String expectedMessage; + + InvalidFieldTestCase( + String fieldName, + Object value, + Class expectedException, + String expectedMessage) { + this.fieldName = fieldName; + this.value = value; + this.expectedException = expectedException; + this.expectedMessage = expectedMessage; + } + } + + @Test + public void newValue_invalidField_throws(@TestParameter InvalidFieldTestCase testCase) { + ImmutableMap fields = ImmutableMap.of(testCase.fieldName, testCase.value); + + Exception e = + assertThrows( + testCase.expectedException, () -> VALUE_PROVIDER.newValue(TEST_ALL_TYPES, fields)); + assertThat(e).hasMessageThat().contains(testCase.expectedMessage); + } + + @Test + public void newValue_missingRequiredField_throws() { + assertThrows( + UninitializedMessageException.class, + () -> + VALUE_PROVIDER.newValue("cel.expr.conformance.proto2.TestRequired", ImmutableMap.of())); + } + + @Test + public void newValue_outOfRangeDurationLiteral_throws() { + ImmutableMap fields = ImmutableMap.of("seconds", 315576000001L); + + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, + () -> VALUE_PROVIDER.newValue("google.protobuf.Duration", fields)); + assertThat(e).hasMessageThat().contains("is not valid"); } @Test public void getProtoLiteCelValueConverter() { assertThat(VALUE_PROVIDER.protoCelValueConverter()).isNotNull(); } + + private static TestAllTypes newTestAllTypes(Map fields) { + Object value = VALUE_PROVIDER.newValue(TEST_ALL_TYPES, fields).get(); + return (TestAllTypes) ((ProtoMessageLiteValue) value).value(); + } } 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 106a6508a..00eca2635 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java @@ -20,6 +20,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.primitives.UnsignedLong; +import com.google.protobuf.UInt64Value; +import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; import dev.cel.common.CelDescriptorUtil; @@ -113,8 +115,7 @@ public void newValue_createProtoMessage_unsignedLongFieldsPopulated() { VALUE_PROVIDER .newValue( TestAllTypes.getDescriptor().getFullName(), - ImmutableMap.of( - "single_uint32", 3, "single_uint64", UnsignedLong.MAX_VALUE.longValue())) + ImmutableMap.of("single_uint32", 3, "single_uint64", UnsignedLong.MAX_VALUE)) .get(); assertThat(protoMessageValue.isZeroValue()).isFalse(); @@ -194,6 +195,38 @@ public void newValue_invalidField_throws() { + " 'cel.expr.conformance.proto2.TestAllTypes'"); } + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum MismatchedNumericTypeTestCase { + INT_TO_UINT32("single_uint32", 1L), + INT_TO_FLOAT("single_float", 1L), + INT_TO_MAP_DOUBLE_VALUE("map_bool_double", ImmutableMap.of(true, 1L)), + UINT_TO_ENUM("standalone_enum", UnsignedLong.valueOf(1L)), + UINT_TO_MAP_INT32_KEY("map_int32_bool", ImmutableMap.of(UnsignedLong.valueOf(1L), true)), + UINT_WRAPPER_TO_INT64("single_int64", UInt64Value.of(1L)), + DOUBLE_TO_REPEATED_UINT64("repeated_uint64", ImmutableList.of(1.0d)); + + private final String fieldName; + private final Object value; + + MismatchedNumericTypeTestCase(String fieldName, Object value) { + this.fieldName = fieldName; + this.value = value; + } + } + + @Test + public void newValue_mismatchedNumericType_throws( + @TestParameter MismatchedNumericTypeTestCase testCase) { + String typeName = TestAllTypes.getDescriptor().getFullName(); + ImmutableMap fields = ImmutableMap.of(testCase.fieldName, testCase.value); + + IllegalArgumentException e = + assertThrows( + IllegalArgumentException.class, () -> VALUE_PROVIDER.newValue(typeName, fields)); + + assertThat(e).hasMessageThat().contains("unexpected value type for field"); + } + @Test public void newValue_onCombinedProvider() { CelValueProvider celValueProvider = (structType, fields) -> Optional.empty(); diff --git a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel index 1860b19cf..7559594ac 100644 --- a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel +++ b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel @@ -199,78 +199,31 @@ _TESTS_TO_SKIP_PLANNER = [ "enums/strong_proto3", ] -_TESTS_TO_SKIP_PLANNER_SELECT_OPT = _TESTS_TO_SKIP_PLANNER + [ - # TODO: Optional field selection on proto messages is not yet supported by - # SelectOptimizer. - "optionals", -] - _TESTS_TO_SKIP_LITE_BASE = _TESTS_TO_SKIP_PLANNER + [ # Proto2 extensions are not supported in CelLiteRuntime. "proto2_ext", "proto2/extensions_has", "proto2/extensions_get", - # TODO: Proto equality after dyn() unwrapping is not yet supported in - # CelLiteRuntime. + # TODO: Message equality is not yet supported in CelLiteRuntime. "comparisons/eq_literal/not_eq_dyn_proto2_msg_null,not_eq_dyn_proto3_msg_null", - "comparisons/eq_wrapper/eq_proto_different_types", - "comparisons/ne_literal/ne_proto_different_types", - - # TODO: Message creation with prepopulated fields is not yet supported in - # CelLiteRuntime. - "block_ext/basic/optional_message", - "comparisons/eq_wrapper/eq_bool,eq_bytes,eq_double,eq_float,eq_int32,eq_int64,eq_string,eq_uint32,eq_uint64", - "comparisons/eq_wrapper/eq_proto2,eq_proto3,eq_proto2_missing_fields_neq,eq_proto3_missing_fields_neq,eq_proto_nan_equal", + "comparisons/eq_wrapper/eq_proto2,eq_proto3,eq_proto2_missing_fields_neq,eq_proto3_missing_fields_neq,eq_proto_nan_equal,eq_proto_different_types", "comparisons/eq_wrapper/eq_proto2_any_unpack_equal,eq_proto2_any_unpack_not_equal,eq_proto2_any_unpack_bytewise_fallback_not_equal,eq_proto2_any_unpack_bytewise_fallback_equal,eq_proto3_any_unpack_equal,eq_proto3_any_unpack_not_equal,eq_proto3_any_unpack_bytewise_fallback_not_equal,eq_proto3_any_unpack_bytewise_fallback_equal", - "comparisons/ne_literal/ne_proto2,ne_proto3,ne_proto2_missing_fields_neq,ne_proto3_missing_fields_neq,ne_proto_nan_not_equal", + "comparisons/ne_literal/ne_proto2,ne_proto3,ne_proto2_missing_fields_neq,ne_proto3_missing_fields_neq,ne_proto_nan_not_equal,ne_proto_different_types", "comparisons/ne_literal/ne_proto2_any_unpack,ne_proto2_any_unpack_bytewise_fallback,ne_proto3_any_unpack,ne_proto3_any_unpack_bytewise_fallback", - "dynamic/int32/literal,literal_no_field_access,field_assign_proto2,field_assign_proto2_zero,field_assign_proto2_max,field_assign_proto2_min,field_assign_proto2_range,field_read_proto2,field_read_proto2_zero,field_assign_proto3,field_assign_proto3_zero,field_assign_proto3_max,field_assign_proto3_min,field_assign_proto3_range,field_read_proto3,field_read_proto3_zero", - "dynamic/int64/literal,literal_no_field_access,field_assign_proto2,field_assign_proto2_zero,field_assign_proto3,field_assign_proto3_zero", - "dynamic/uint32/literal,literal_no_field_access,field_assign_proto2,field_assign_proto2_zero,field_assign_proto2_max,field_assign_proto2_range,field_assign_proto3,field_assign_proto3_zero,field_assign_proto3_max,field_assign_proto3_range,field_read_proto2,field_read_proto2_zero", - "dynamic/uint64/literal,literal_no_field_access,field_assign_proto2,field_assign_proto2_zero,field_assign_proto3,field_assign_proto3_zero,field_read_proto2,field_read_proto2_zero", - "dynamic/float/literal,literal_not_double,literal_no_field_access,field_assign_proto2,field_assign_proto2_zero,field_assign_proto2_subnorm,field_assign_proto2_round_to_zero,field_assign_proto2_range,field_read_proto2,field_read_proto2_zero,field_assign_proto3,field_assign_proto3_zero,field_assign_proto3_subnorm,field_assign_proto3_round_to_zero,field_assign_proto3_range,field_read_proto3,field_read_proto3_zero", - "dynamic/double/literal,literal_no_field_access,field_assign_proto2,field_assign_proto2_zero,field_assign_proto2_range,field_read_proto2,field_read_proto2_zero,field_assign_proto3,field_assign_proto3_zero,field_assign_proto3_range,field_read_proto3,field_read_proto3_zero", - "dynamic/bool/literal,literal_no_field_access,field_assign_proto2,field_assign_proto2_false,field_assign_proto3,field_assign_proto3_false", - "dynamic/string/literal,literal_no_field_access,literal_unicode,field_assign_proto2,field_assign_proto2_empty,field_assign_proto3,field_assign_proto3_empty", - "dynamic/bytes/literal,literal_no_field_access,literal_unicode,field_assign_proto2,field_assign_proto2_empty,field_assign_proto3,field_assign_proto3_empty", - "dynamic/list/literal,literal_no_field_access,literal_empty,field_assign_proto2,field_assign_proto2_empty,field_read_proto2,field_read_proto2_empty,field_assign_proto3,field_assign_proto3_empty,field_read_proto3,field_read_proto3_empty", - "dynamic/struct/literal,literal_no_field_access,literal_empty,field_assign_proto2,field_assign_proto2_empty,field_assign_proto2_bad,field_read_proto2,field_read_proto2_empty,field_assign_proto3,field_assign_proto3_empty,field_assign_proto3_bad,field_read_proto3,field_read_proto3_empty", - "dynamic/value_null/literal,literal_no_field_access,field_assign_proto2,field_read_proto2,field_assign_proto3,field_read_proto3", - "dynamic/value_number/literal,literal_no_field_access,literal_zero,field_assign_proto2,field_assign_proto2_zero,field_read_proto2,field_read_proto2_zero,field_assign_proto3,field_assign_proto3_zero,field_read_proto3,field_read_proto3_zero", - "dynamic/value_string/literal,literal_no_field_access,literal_empty,field_assign_proto2,field_assign_proto2_empty,field_read_proto2,field_read_proto2_zero,field_assign_proto3,field_assign_proto3_empty,field_read_proto3,field_read_proto3_zero", - "dynamic/value_bool/literal,literal_no_field_access,literal_false,field_assign_proto2,field_assign_proto2_false,field_read_proto2,field_read_proto2_false,field_assign_proto3,field_assign_proto3_false,field_read_proto3,field_read_proto3_false", - "dynamic/value_struct/literal,literal_no_field_access,literal_empty,field_assign_proto2,field_assign_proto2_empty,field_read_proto2,field_read_proto2_empty,field_assign_proto3,field_assign_proto3_empty,field_read_proto3,field_read_proto3_empty", - "dynamic/value_list/literal,literal_no_field_access,literal_empty,field_assign_proto2,field_assign_proto2_empty,field_read_proto2,field_read_proto2_empty,field_assign_proto3,field_assign_proto3_empty,field_read_proto3,field_read_proto3_empty", - "dynamic/any/literal,literal_no_field_access,field_assign_proto2,field_read_proto2,field_assign_proto3,field_read_proto3", - "dynamic/complex", - "enums/legacy_proto2/assign_standalone_name,assign_standalone_int,assign_standalone_int_too_big,assign_standalone_int_too_neg,assign_repeated_enum,list_enum_as_list_int,enum_as_int", - "enums/legacy_proto3/assign_standalone_name,assign_standalone_int,assign_standalone_int_big,assign_standalone_int_neg,assign_standalone_int_too_big,assign_standalone_int_too_neg,assign_repeated_enum,list_enum_as_list_int,enum_as_int", - "optionals/optionals/struct_map_optindex_field,struct_map_optindex_field_nested,struct_list_optindex_field,struct_optindex_value,optional_struct_optindex_value,optional_struct_optindex_index_value", - "parse/nest/message_literal", - "parse/repeat/message_literal", - "parse/whitespace", - "parse/comments", - "parse/struct_field_names", - "proto2/literal_singular", - "proto2/literal_wellknown", - "proto2/set_null", - "proto2/quoted_fields", - "proto2/has/repeated_none_explicit,repeated_one,repeated_many,map_none_explicit,map_one_default,map_one,map_many,required,optional_set_no_default,optional_set_with_default,optional_set_to_default,optional_message_set,optional_enum_set,optional_enum_set_zero,oneof_other_set,oneof_set,oneof_set_default", - "proto3/literal_singular", - "proto3/literal_wellknown", - "proto3/set_null", - "proto3/quoted_fields", - "proto3/has/repeated_none_explicit,repeated_one,repeated_many,map_none_explicit,map_one_default,map_one,map_many,single_set,single_set_to_default,single_message_set,single_message_set_to_default,single_enum_set,single_enum_set_zero,oneof_other_set,oneof_set,oneof_set_default", - "type_deductions/complex_initializers/struct", - "type_deductions/field_access/int_field", - "wrappers", -] - -_TESTS_TO_SKIP_LITE_SELECT_OPT = _TESTS_TO_SKIP_LITE_BASE + [ - # TODO: Optional field selection on proto messages is not yet supported by - # SelectOptimizer. - "optionals", + "proto2/quoted_fields/set_field_with_quoted_name", + "proto2/set_null/single_message,single_duration,single_timestamp", + "proto3/quoted_fields/set_field", + "proto3/set_null/single_message,single_duration,single_timestamp", + "wrappers/bool/to_null", + "wrappers/bytes/to_null", + "wrappers/double/to_null", + "wrappers/float/to_null", + "wrappers/int32/to_null", + "wrappers/int64/to_null", + "wrappers/string/to_null", + "wrappers/uint32/to_null", + "wrappers/uint64/to_null", ] _TESTS_TO_SKIP_LITE = _TESTS_TO_SKIP_LITE_BASE + [ @@ -309,7 +262,7 @@ conformance_test( data = _ALL_TESTS, enable_select_optimization = True, runtime = RUNTIME.PLANNER, - skip_tests = _TESTS_TO_SKIP_PLANNER_SELECT_OPT, + skip_tests = _TESTS_TO_SKIP_PLANNER, ) conformance_test( @@ -324,5 +277,5 @@ conformance_test( data = _ALL_TESTS, enable_select_optimization = True, runtime = RUNTIME.LITE, - skip_tests = _TESTS_TO_SKIP_LITE_SELECT_OPT, + skip_tests = _TESTS_TO_SKIP_LITE_BASE, ) diff --git a/extensions/src/test/java/dev/cel/extensions/CelOptionalLibraryTest.java b/extensions/src/test/java/dev/cel/extensions/CelOptionalLibraryTest.java index a08d861f2..daf5090de 100644 --- a/extensions/src/test/java/dev/cel/extensions/CelOptionalLibraryTest.java +++ b/extensions/src/test/java/dev/cel/extensions/CelOptionalLibraryTest.java @@ -758,9 +758,6 @@ public void optionalFieldSelection_onProtoMessage_returnsOptionalValue() throws @Test public void optionalFieldSelection_onProtoMessage_listValue() throws Exception { - // TODO: Message creation with prepopulated fields is not yet supported in - // CelLiteRuntime. - assumeFalse(testMode.isLite()); TestCel cel = newCelBuilder().build(); CelAbstractSyntaxTree ast = compile( @@ -773,9 +770,6 @@ public void optionalFieldSelection_onProtoMessage_listValue() throws Exception { @Test public void optionalFieldSelection_onProtoMessage_indexValue() throws Exception { - // TODO: Message creation with prepopulated fields is not yet supported in - // CelLiteRuntime. - assumeFalse(testMode.isLite()); TestCel cel = newCelBuilder().build(); CelAbstractSyntaxTree ast = compile( @@ -1450,9 +1444,6 @@ public void optionalMessageCreation_fieldValueIsEmpty_returnsEmptyMessage() thro @Test public void optionalMessageCreation_fieldValueIsPresent_returnsMessage() throws Exception { - // TODO: Message creation with prepopulated fields is not yet supported in - // CelLiteRuntime. - assumeFalse(testMode.isLite()); TestCel cel = newCelBuilder().build(); CelAbstractSyntaxTree ast = compile(cel, "TestAllTypes{?single_double_wrapper: optional.ofNonZeroValue(5.0)}"); @@ -1490,9 +1481,6 @@ public void optionalMessageCreation_fieldValueContainsEmptyMap_returnsEmptyMessa @Test public void optionalMessageCreation_fieldValueContainsMap_returnsEmptyMessage() throws Exception { - // TODO: Message creation with prepopulated fields is not yet supported in - // CelLiteRuntime. - assumeFalse(testMode.isLite()); TestCel cel = newCelBuilder() .addVar( @@ -1614,9 +1602,6 @@ public void optionalListCreation_containsMixedTypeElements_success() throws Exce @Test public void optionalListCreation_withinProtoMessage_success() throws Exception { - // TODO: Message creation with prepopulated fields is not yet supported in - // CelLiteRuntime. - assumeFalse(testMode.isLite()); TestCel cel = newCelBuilder() .addVar("m", MapType.create(SimpleType.STRING, SimpleType.DYN)) diff --git a/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java b/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java index e2b299445..d05b92a91 100644 --- a/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java +++ b/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java @@ -45,6 +45,9 @@ public abstract class CelLiteDescriptor { Collections.unmodifiableSet( new HashSet<>(Arrays.asList(false, 0, 0L, 0.0f, 0.0d, "", ByteString.EMPTY))); + // Map entries have no generated class to build. + private static final Supplier NO_BUILDER = () -> null; + @SuppressWarnings("Immutable") // Copied to unmodifiable map private final Map protoFqnToDescriptors; @@ -121,6 +124,11 @@ public MessageLite.Builder newMessageBuilder() { return messageBuilderSupplier.get(); } + /** Whether this describes a map field's entry, which has no message builder. */ + public boolean isMapEntry() { + return messageBuilderSupplier.equals(NO_BUILDER); + } + /** Whether {@link #getFieldValue} reads this message's fields with their generated getters. */ public boolean hasGetters() { return owner != null; @@ -398,7 +406,7 @@ public FieldLiteDescriptor( } /** Describes map entry type {@code fullName}, whose key and value are fields 1 and 2. */ - protected static MessageLiteDescriptor mapEntry( + public static MessageLiteDescriptor mapEntry( String fullName, FieldLiteDescriptor.Type keyType, FieldLiteDescriptor.Type valueType, @@ -415,7 +423,7 @@ protected static MessageLiteDescriptor mapEntry( valueType, false, valueTypeName)), - () -> null); + NO_BUILDER); } protected CelLiteDescriptor(String version, List messageInfoList) { diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java index 4a4175778..64ccaab56 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java @@ -52,6 +52,7 @@ import dev.cel.common.types.CelType; import dev.cel.common.types.CelTypeProvider; import dev.cel.common.types.SimpleType; +import dev.cel.common.types.StructTypeReference; import dev.cel.common.types.TypeType; import dev.cel.common.values.CelValueConverter; import dev.cel.common.values.CelValueProvider; @@ -662,13 +663,10 @@ private ResolvedFunction resolveFunction( } private CelType resolveStructType(CelExpr expr, PlannerContext ctx) { - CelType checkedType = ctx.typeMap().get(expr.id()); - if (checkedType != null) { - CelKind kind = checkedType.kind(); - // Type-checked ASTs do not need a type-provider lookup as long as it's of expected kind. - if (isValidStructKind(kind)) { - return checkedType; - } + // Checked ASTs reference the resolved message name, including for well-known types. + CelReference reference = ctx.referenceMap().get(expr.id()); + if (reference != null) { + return StructTypeReference.create(reference.name()); } CelStruct struct = expr.struct(); diff --git a/runtime/src/test/java/dev/cel/runtime/CelLiteInterpreterTest.java b/runtime/src/test/java/dev/cel/runtime/CelLiteInterpreterTest.java index bc0fde200..6a4167790 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelLiteInterpreterTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelLiteInterpreterTest.java @@ -17,6 +17,7 @@ import com.google.testing.junit.testparameterinjector.TestParameterInjector; import dev.cel.common.CelOptions; import dev.cel.common.values.ProtoMessageLiteValueProvider; +import dev.cel.expr.conformance.proto3.NestedTestAllTypesCelDescriptor; import dev.cel.expr.conformance.proto3.TestAllTypesCelDescriptor; import dev.cel.extensions.CelOptionalLibrary; import dev.cel.testing.BaseInterpreterTest; @@ -35,7 +36,8 @@ protected CelRuntimeBuilder newBaseRuntimeBuilder(CelOptions celOptions) { .setValueProvider( ProtoMessageLiteValueProvider.newInstance( dev.cel.expr.conformance.proto2.TestAllTypesCelDescriptor.getDescriptor(), - TestAllTypesCelDescriptor.getDescriptor())) + TestAllTypesCelDescriptor.getDescriptor(), + NestedTestAllTypesCelDescriptor.getDescriptor())) .addLibraries(CelOptionalLibrary.INSTANCE) .addFileTypes(TEST_FILE_DESCRIPTORS) .addLateBoundFunctions("record") @@ -54,58 +56,6 @@ public void dynamicMessage_dynamicDescriptor() throws Exception { skipBaselineVerification(); } - // All the tests below rely on message creation with fields populated. They are excluded for time - // being until this support is added. - @Override - public void nullAssignability() throws Exception { - skipBaselineVerification(); - } - - @Override - public void wrappers() throws Exception { - skipBaselineVerification(); - } - - @Override - public void jsonConversions() { - skipBaselineVerification(); - } - - @Override - public void nestedEnums() { - skipBaselineVerification(); - } - - @Override - public void messages() throws Exception { - skipBaselineVerification(); - } - - @Override - public void packUnpackAny() { - skipBaselineVerification(); - } - - @Override - public void lists() throws Exception { - skipBaselineVerification(); - } - - @Override - public void maps() throws Exception { - skipBaselineVerification(); - } - - @Override - public void jsonValueTypes() { - skipBaselineVerification(); - } - - @Override - public void messages_error() { - skipBaselineVerification(); - } - @Override public void jsonFieldNames() { // json_name field option is not yet supported in lite runtime diff --git a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeAndroidTest.java b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeAndroidTest.java index f1a9f4908..f4d5da19b 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeAndroidTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeAndroidTest.java @@ -30,8 +30,10 @@ import com.google.protobuf.Int32Value; import com.google.protobuf.Int64Value; import com.google.protobuf.StringValue; +import com.google.protobuf.Struct; import com.google.protobuf.UInt32Value; import com.google.protobuf.UInt64Value; +import com.google.protobuf.Value; import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; @@ -875,6 +877,45 @@ public void eval_optionalExtension() throws Exception { assertThat(runtime.createProgram(ast).eval()).isEqualTo(1L); } + @Test + public void eval_messageCreation_returnsMessageLite() throws Exception { + CelLiteRuntime runtime = + CelLiteRuntimeFactory.newLiteRuntimeBuilder() + .setStandardFunctions(CelStandardFunctions.ALL_STANDARD_FUNCTIONS) + .setValueProvider( + ProtoMessageLiteValueProvider.newInstance( + TestAllTypesCelLiteDescriptor.getDescriptor())) + .build(); + // Expr: cel.expr.conformance.proto3.TestAllTypes{single_int32: -1, ..., single_value: ...} + CelAbstractSyntaxTree ast = readCheckedExpr("compiled_proto3_message_creation"); + + TestAllTypes result = (TestAllTypes) runtime.createProgram(ast).eval(); + + assertThat(result) + .isEqualTo( + TestAllTypes.newBuilder() + .setSingleInt32(-1) + .setSingleSint64(-2L) + .setSingleFixed32(-1) + .setSingleFloat(1.5f) + .setSingleString("foo") + .setSingleBytes(ByteString.copyFromUtf8("abc")) + .setStandaloneEnumValue(2) + .setOptionalBool(false) + .setSingleNestedMessage(TestAllTypes.NestedMessage.newBuilder().setBb(7).build()) + .addRepeatedInt64(1L) + .addRepeatedInt64(2L) + .putMapStringString("k", "v") + .setSingleInt64Wrapper(Int64Value.of(5)) + .setSingleUint32Wrapper(UInt32Value.of(-1)) + .setSingleDuration(ProtoTimeUtils.toProtoDuration(Duration.ofSeconds(1))) + .setSingleStruct( + Struct.newBuilder() + .putFields("a", Value.newBuilder().setNumberValue(1.0).build())) + .setSingleValue(Value.newBuilder().setStringValue("x").build()) + .build()); + } + private enum CelOptionsTestCase { UNSIGNED_LONG_DISABLED(newBaseTestOptions().enableUnsignedLongs(false).build()), UNWRAP_WKT_DISABLED(newBaseTestOptions().unwrapWellKnownTypesOnFunctionDispatch(false).build()), diff --git a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeTest.java b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeTest.java index 89aed454f..755a08435 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeTest.java @@ -117,14 +117,25 @@ public void messageCreation_emptyMessage() throws Exception { @Test public void messageCreation_fieldsPopulated() throws Exception { - CelAbstractSyntaxTree ast = CEL_COMPILER.compile("TestAllTypes{single_int32: 4}").getAst(); - Program program = CEL_RUNTIME.createProgram(ast); - - CelEvaluationException e = assertThrows(CelEvaluationException.class, program::eval); + CelAbstractSyntaxTree ast = + CEL_COMPILER + .compile( + "TestAllTypes{single_int32: 4, repeated_string: ['a'], map_string_string: {'k':" + + " 'v'}, single_nested_message: TestAllTypes.NestedMessage{bb: 7}," + + " single_int64_wrapper: 5}") + .getAst(); - assertThat(e) - .hasMessageThat() - .contains("Message creation with prepopulated fields is not supported yet."); + TestAllTypes message = (TestAllTypes) CEL_RUNTIME.createProgram(ast).eval(); + + assertThat(message) + .isEqualTo( + TestAllTypes.newBuilder() + .setSingleInt32(4) + .addRepeatedString("a") + .putMapStringString("k", "v") + .setSingleNestedMessage(NestedMessage.newBuilder().setBb(7)) + .setSingleInt64Wrapper(Int64Value.of(5)) + .build()); } @Test diff --git a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java index 24160aef1..84515b88b 100644 --- a/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java +++ b/runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java @@ -709,6 +709,16 @@ public void plan_createStruct_oneofSetTwice_lastFieldWins( assertThat(result.getKindCase()).isEqualTo(expectedCase); } + @Test + public void plan_createStruct_enumOutOfRange_throwsNumericOverflow() throws Exception { + CelAbstractSyntaxTree ast = compile("TestAllTypes{standalone_enum: 5000000000}"); + Program program = PLANNER.plan(ast); + + CelEvaluationException e = assertThrows(CelEvaluationException.class, program::eval); + + assertThat(e.getErrorCode()).isEqualTo(CelErrorCode.NUMERIC_OVERFLOW); + } + @Test public void plan_call_zeroArgs() throws Exception { CelAbstractSyntaxTree ast = compile("zero()"); diff --git a/testing/src/main/java/dev/cel/testing/compiled/BUILD.bazel b/testing/src/main/java/dev/cel/testing/compiled/BUILD.bazel index 2c5921175..cddef42cc 100644 --- a/testing/src/main/java/dev/cel/testing/compiled/BUILD.bazel +++ b/testing/src/main/java/dev/cel/testing/compiled/BUILD.bazel @@ -63,6 +63,7 @@ java_library( ":compiled_proto2_select_repeated_fields", ":compiled_proto2_select_wrappers", ":compiled_proto3_deep_traversal", + ":compiled_proto3_message_creation", ":compiled_proto3_select_map_fields", ":compiled_proto3_select_primitives", ":compiled_proto3_select_primitives_all_ored", @@ -106,6 +107,17 @@ compile_cel( proto_srcs = ["@cel_conformance//proto/cel/expr/conformance/proto3:test_all_types_proto"], ) +compile_cel( + name = "compiled_proto3_message_creation", + expression = "cel.expr.conformance.proto3.TestAllTypes{single_int32: -1, single_sint64: -2, single_fixed32: 4294967295u, " + + "single_float: 1.5, single_string: 'foo', single_bytes: b'abc', standalone_enum: 2, optional_bool: false, " + + "single_nested_message: cel.expr.conformance.proto3.TestAllTypes.NestedMessage{bb: 7}, repeated_int64: [1, 2], " + + "map_string_string: {'k': 'v'}, single_int64_wrapper: 5, single_uint32_wrapper: google.protobuf.UInt32Value{value: 4294967295u}, " + + "single_duration: duration('1s'), single_struct: google.protobuf.Struct{fields: {'a': 1.0}}, " + + "single_value: google.protobuf.Value{string_value: 'x'}}", + proto_srcs = ["@cel_conformance//proto/cel/expr/conformance/proto3:test_all_types_proto"], +) + compile_cel( name = "compiled_extensions", environment = "//testing/environment:all_extensions",