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 23821e278..1559f7ad8 100644 --- a/common/src/main/java/dev/cel/common/values/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/values/BUILD.bazel @@ -329,6 +329,7 @@ java_library( deps = [ ":base_proto_cel_value_converter", ":optimized_selectable", + ":preadapted_list", ":select_field", ":values", "//:auto_value", @@ -360,6 +361,7 @@ cel_android_library( deps = [ ":base_proto_cel_value_converter_android", ":optimized_selectable_android", + ":preadapted_list_android", ":select_field_android", ":values_android", "//:auto_value", 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 c4b9dacaa..81fae97a4 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java @@ -25,6 +25,7 @@ import com.google.protobuf.ByteString; import com.google.protobuf.CodedInputStream; import com.google.protobuf.ExtensionRegistryLite; +import com.google.protobuf.Internal.EnumLite; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.MessageLite; import com.google.protobuf.WireFormat; @@ -163,16 +164,28 @@ Object getDefaultCelValue(FieldLiteDescriptor fieldDescriptor) { return toRuntimeValue(getDefaultValue(fieldDescriptor)); } - private Optional findFieldDescriptor(String protoTypeName, int fieldNumber) { - return descriptorPool - .findDescriptor(protoTypeName) - .flatMap(desc -> desc.findByFieldNumber(fieldNumber)); + // A selected field's default value: the one in the SelectField, else its type's. + Object getDefaultCelValue(SelectField field, FieldLiteDescriptor fieldDescriptor) { + return field.defaultValue() != null + ? field.defaultValue() + : getDefaultCelValue(fieldDescriptor); + } + + @Nullable FieldLiteDescriptor findFieldDescriptor(String protoTypeName, int fieldNumber) { + MessageLiteDescriptor descriptor = descriptorPool.findDescriptor(protoTypeName).orElse(null); + return descriptor == null ? null : descriptor.findByFieldNumber(fieldNumber); } - Optional findFieldDescriptor(String protoTypeName, String fieldName) { + @Nullable FieldLiteDescriptor findFieldDescriptor(String protoTypeName, String fieldName) { + MessageLiteDescriptor descriptor = descriptorPool.findDescriptor(protoTypeName).orElse(null); + return descriptor == null ? null : descriptor.findByFieldName(fieldName); + } + + @Nullable MessageLiteDescriptor findDescriptorWithGetters(String protoTypeName) { return descriptorPool .findDescriptor(protoTypeName) - .flatMap(desc -> desc.findByFieldName(fieldName)); + .filter(MessageLiteDescriptor::hasGetters) + .orElse(null); } MessageLite parseMessageLite(ByteString bytes, String protoTypeName) { @@ -316,15 +329,8 @@ private Map.Entry readSingleMapEntry( 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); - } + /** Reads {@code field} using the type information it carries. */ + private @Nullable Object readField(ByteString bytes, String protoTypeName, SelectField field) { SelectField.MapEntrySpec mapEntrySpec = field.mapEntrySpec(); if (mapEntrySpec == null) { return readField(bytes, protoTypeName, newFieldDescriptor(field)); @@ -355,6 +361,9 @@ private Map.Entry readSingleMapEntry( FieldLiteDescriptor fieldDescriptor, @Nullable FieldLiteDescriptor keyDescriptor, @Nullable FieldLiteDescriptor valueDescriptor) { + if (fieldDescriptor.getProtoFieldType().equals(FieldLiteDescriptor.Type.GROUP)) { + throw new UnsupportedOperationException("Groups are not supported"); + } CodedInputStream inputStream = bytes.newCodedInput(); // Submessages alias bytes; unwrapping parses or copies them, so no view outlives evaluation. inputStream.enableAliasing(true); @@ -400,40 +409,31 @@ private Map.Entry readSingleMapEntry( } /** - * 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. + * Selects {@code field}, which its message's descriptor doesn't have, from {@code bytes} by the + * type information it carries. Returns the field's default value if it's absent. */ 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( - fieldDescriptor != null ? fieldDescriptor : newFieldDescriptor(field)); + Object fieldValue = readField(bytes, protoTypeName, field); + return fieldValue != null ? fieldValue : getDefaultCelValue(field, 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. + * Finds {@code field}, which its message's descriptor doesn't have, in {@code bytes} by the type + * information it carries. A field without a type code is decoded as a message. */ Optional findByFieldNumber(ByteString bytes, String protoTypeName, SelectField field) { - FieldLiteDescriptor fieldDescriptor = - findFieldDescriptor(protoTypeName, field.fieldNumber()).orElse(null); - return Optional.ofNullable(readField(bytes, protoTypeName, fieldDescriptor, field)); + return Optional.ofNullable(readField(bytes, protoTypeName, field)); } /** Returns whether {@code field} is present in {@code bytes}. */ boolean hasFieldByNumber(ByteString bytes, String protoTypeName, SelectField field) { - FieldLiteDescriptor fieldDescriptor = - findFieldDescriptor(protoTypeName, field.fieldNumber()).orElse(null); + FieldLiteDescriptor fieldDescriptor = findFieldDescriptor(protoTypeName, field.fieldNumber()); if (fieldDescriptor == null) { fieldDescriptor = newFieldDescriptor(field); } + if (fieldDescriptor.getProtoFieldType().equals(FieldLiteDescriptor.Type.GROUP)) { + throw new UnsupportedOperationException("Groups are not supported"); + } boolean isPackableList = fieldDescriptor.getEncodingType().equals(EncodingType.LIST) && isPackable(fieldDescriptor); CodedInputStream inputStream = bytes.newCodedInput(); @@ -464,6 +464,108 @@ boolean hasFieldByNumber(ByteString bytes, String protoTypeName, SelectField fie return false; } + /** + * Reads {@code fieldDescriptor} of {@code message} with its generated getter rather than from + * wire bytes: its CEL value, or null if it's absent. + */ + @Nullable Object readFieldByGetter( + MessageLite message, MessageLiteDescriptor descriptor, FieldLiteDescriptor fieldDescriptor) { + Object value = readGetter(message, descriptor, fieldDescriptor.getFieldNumber()); + if (value == null) { + return null; + } + switch (fieldDescriptor.getEncodingType()) { + case LIST: + List list = (List) value; + switch (fieldDescriptor.getProtoFieldType()) { + case INT64: + case SINT64: + case SFIXED64: + case DOUBLE: + case BOOL: + case STRING: + // These elements are their own CEL values, so the immutable list needn't be copied. + return CelPreAdaptedList.wrap(list); + default: + break; + } + ImmutableList.Builder listBuilder = + ImmutableList.builderWithExpectedSize(list.size()); + for (int i = 0; i < list.size(); i++) { + listBuilder.add(fromGetterValue(list.get(i), fieldDescriptor)); + } + return listBuilder.build(); + case MAP: + Map map = (Map) value; + MessageLiteDescriptor entryDescriptor = + descriptorPool.getDescriptorOrThrow(fieldDescriptor.getFieldProtoTypeName()); + FieldLiteDescriptor keyDescriptor = + entryDescriptor.getByFieldNumberOrThrow(MAP_KEY_FIELD_NUMBER); + FieldLiteDescriptor valueDescriptor = + entryDescriptor.getByFieldNumberOrThrow(MAP_VALUE_FIELD_NUMBER); + ImmutableMap.Builder mapBuilder = + ImmutableMap.builderWithExpectedSize(map.size()); + for (Map.Entry entry : map.entrySet()) { + mapBuilder.put( + fromGetterValue(entry.getKey(), keyDescriptor), + fromGetterValue(entry.getValue(), valueDescriptor)); + } + return mapBuilder.buildOrThrow(); + case SINGULAR: + return fromGetterValue(value, fieldDescriptor); + } + throw new IllegalStateException( + "Unexpected encoding type: " + fieldDescriptor.getEncodingType()); + } + + /** Like {@link #hasFieldByNumber}, through the field's generated getter. */ + boolean hasByGetter(MessageLite message, MessageLiteDescriptor descriptor, SelectField field) { + return readGetter(message, descriptor, field.fieldNumber()) != null; + } + + // A field's value as its getter returns it, or null if it's unset or an empty repeated or map. + private static @Nullable Object readGetter( + MessageLite message, MessageLiteDescriptor descriptor, int fieldNumber) { + Object value = descriptor.getFieldValue(message, fieldNumber); + boolean isEmpty = + (value instanceof List && ((List) value).isEmpty()) + || (value instanceof Map && ((Map) value).isEmpty()); + return isEmpty ? null : value; + } + + // Converts a field's (or element's) Java value as returned by its getter to its CEL value. + private Object fromGetterValue(Object value, FieldLiteDescriptor fieldDescriptor) { + switch (fieldDescriptor.getProtoFieldType()) { + case UINT32: + case FIXED32: + return UnsignedLong.fromLongBits(Integer.toUnsignedLong((Integer) value)); + case UINT64: + case FIXED64: + return UnsignedLong.fromLongBits((Long) value); + case ENUM: + // Open enums are read as their numbers; closed ones as enum constants. + return value instanceof Integer + ? ((Integer) value).longValue() + : (long) ((EnumLite) value).getNumber(); + case MESSAGE: + MessageLite message = (MessageLite) value; + String protoTypeName = fieldDescriptor.getFieldProtoTypeName(); + MessageLiteDescriptor descriptor = + descriptorPool.findDescriptor(protoTypeName).orElse(null); + if (descriptor == null) { + // Without a descriptor, only this submessage is serialized, rather than its whole root. + return RawProtoMessageLiteValue.create(message.toByteString(), protoTypeName, this); + } + if (descriptorPool.findDescriptor(message).orElse(null) != descriptor) { + // The pool decodes this type into another class, such as an older version of it. + return readMessageField(message.toByteString(), protoTypeName); + } + return toRuntimeValue(message, descriptor); + default: + return super.toRuntimeValue(value); + } + } + /** Describes {@code field} by the type information it carries, built once and kept on it. */ private static FieldLiteDescriptor newFieldDescriptor(SelectField field) { Object cached = field.liteFieldDescriptor(); 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 d8c23c8e0..c34fbe250 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValue.java @@ -25,6 +25,7 @@ import dev.cel.common.types.CelType; import dev.cel.common.types.StructTypeReference; import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor; +import dev.cel.protobuf.CelLiteDescriptor.MessageLiteDescriptor; import java.util.Objects; import java.util.Optional; import org.jspecify.annotations.Nullable; @@ -40,7 +41,8 @@ *

An instance is backed by either a materialized {@link #rawValue()} or unparsed {@link * #wireBytes()} (exactly one is non-null). Wire-backed instances decode selected fields directly * from {@link #wireBytes()} and lazily parse the full {@link MessageLite} only if {@link #value()} - * is invoked. + * is invoked. Materialized instances read fields through their descriptor's generated getters if it + * has them, and otherwise, as for fields it doesn't know, decode them from their serialized bytes. * *

Implements {@link OptimizedSelectable} so that select chains can address fields by number: * @@ -120,7 +122,7 @@ public final int hashCode() { @Override public Object select(String field) { - FieldLiteDescriptor fd = findFieldDescriptor(field).orElse(null); + FieldLiteDescriptor fd = findFieldDescriptor(field); if (fd == null) { throw CelAttributeNotFoundException.of( String.format("field '%s' is not declared in message '%s'", field, celType().name())); @@ -131,35 +133,65 @@ public Object select(String field) { @Override public Optional find(String field) { - 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)); + return Optional.ofNullable(findFieldDescriptor(field)).map(this::readField); } @Override public Object selectByFieldNumber(SelectField field) { + FieldLiteDescriptor fd = findFieldDescriptor(field.fieldNumber()); + if (fd != null) { + Object value = readField(fd); + return value != null ? value : protoLiteCelValueConverter().getDefaultCelValue(field, fd); + } return protoLiteCelValueConverter() .selectByFieldNumber(toByteString(), celType().name(), field); } @Override public boolean hasFieldByNumber(SelectField field) { + MessageLite message = rawValue(); + if (message != null && findFieldDescriptor(field.fieldNumber()) != null) { + MessageLiteDescriptor descriptor = findDescriptorWithGetters(); + if (descriptor != null) { + return protoLiteCelValueConverter().hasByGetter(message, descriptor, field); + } + } return protoLiteCelValueConverter().hasFieldByNumber(toByteString(), celType().name(), field); } @Override public Optional findByFieldNumber(SelectField field) { + FieldLiteDescriptor fd = findFieldDescriptor(field.fieldNumber()); + if (fd != null) { + return Optional.ofNullable(readField(fd)); + } return protoLiteCelValueConverter().findByFieldNumber(toByteString(), celType().name(), field); } private @Nullable Object readField(FieldLiteDescriptor fd) { + MessageLite message = rawValue(); + if (message != null) { + MessageLiteDescriptor descriptor = findDescriptorWithGetters(); + if (descriptor != null) { + return protoLiteCelValueConverter().readFieldByGetter(message, descriptor, fd); + } + } return protoLiteCelValueConverter().readField(toByteString(), celType().name(), fd); } - private Optional findFieldDescriptor(String fieldName) { + private @Nullable FieldLiteDescriptor findFieldDescriptor(String fieldName) { return protoLiteCelValueConverter().findFieldDescriptor(celType().name(), fieldName); } + private @Nullable FieldLiteDescriptor findFieldDescriptor(int fieldNumber) { + return protoLiteCelValueConverter().findFieldDescriptor(celType().name(), fieldNumber); + } + + private @Nullable MessageLiteDescriptor findDescriptorWithGetters() { + return protoLiteCelValueConverter().findDescriptorWithGetters(celType().name()); + } + static ProtoMessageLiteValue create( MessageLite value, String typeName, ProtoLiteCelValueConverter protoLiteCelValueConverter) { checkNotNull(value); 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 22da58d55..c7946078d 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoLiteCelValueConverterTest.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.primitives.UnsignedLong; +import com.google.common.truth.Expect; import com.google.protobuf.BoolValue; import com.google.protobuf.ByteString; import com.google.protobuf.BytesValue; @@ -42,7 +43,9 @@ import com.google.testing.junit.testparameterinjector.TestParameterInjector; import dev.cel.common.internal.CelLiteDescriptorPool; import dev.cel.common.internal.DefaultLiteDescriptorPool; +import dev.cel.expr.conformance.proto2.TestAllTypes.NestedEnum; import dev.cel.expr.conformance.proto3.NestedTestAllTypes; +import dev.cel.expr.conformance.proto3.NestedTestAllTypesCelDescriptor; import dev.cel.expr.conformance.proto3.TestAllTypes; import dev.cel.expr.conformance.proto3.TestAllTypes.NestedMessage; import dev.cel.expr.conformance.proto3.TestAllTypesCelDescriptor; @@ -53,11 +56,15 @@ import java.util.Map; import java.util.NoSuchElementException; import java.util.Optional; +import java.util.function.Supplier; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @RunWith(TestParameterInjector.class) public final class ProtoLiteCelValueConverterTest { + @Rule public final Expect expect = Expect.create(); + private static final String PROTO3_TEST_ALL_TYPES = "cel.expr.conformance.proto3.TestAllTypes"; private static final CelLiteDescriptorPool EMPTY_DESCRIPTOR_POOL = @@ -87,6 +94,16 @@ public MessageLiteDescriptor getDescriptorOrThrow(String protoTypeName) { private static final ProtoLiteCelValueConverter PROTO_LITE_CEL_VALUE_CONVERTER = ProtoLiteCelValueConverter.newInstance(DESCRIPTOR_POOL); + private static final ProtoLiteCelValueConverter ALL_TYPES_CONVERTER = + ProtoLiteCelValueConverter.newInstance( + DefaultLiteDescriptorPool.newInstance( + ImmutableSet.of( + TestAllTypesCelDescriptor.getDescriptor(), + NestedTestAllTypesCelDescriptor.getDescriptor(), + dev.cel.expr.conformance.proto2.TestAllTypesCelDescriptor.getDescriptor(), + dev.cel.expr.conformance.proto2.NestedTestAllTypesCelDescriptor + .getDescriptor()))); + @Test public void fromProtoMessageToCelValue_withTestMessage_convertsToProtoMessageLiteValueFromProtoMessage() { @@ -616,6 +633,141 @@ public void readField_groupInSkippedField_throwsUnsupportedOperationException( assertThat(e).hasMessageThat().isEqualTo("Groups are not supported"); } + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum GetterParityTestCase { + PROTO3_DEFAULT(TestAllTypes.getDefaultInstance()), + PROTO3_POPULATED( + TestAllTypes.newBuilder() + .setSingleInt32(Integer.MIN_VALUE) + .setSingleInt64(Long.MIN_VALUE) + .setSingleUint32(-1) + .setSingleUint64(-1L) + .setSingleSint32(-5) + .setSingleFixed32(-1) + .setSingleSfixed64(-10L) + .setSingleFloat(-0.0f) + .setSingleDouble(Double.NaN) + .setSingleBool(true) + .setSingleString("s") + .setSingleBytes(ByteString.copyFromUtf8("b")) + .setOptionalBool(false) + .setSingleNestedEnumValue(42) + .setStandaloneMessage(NestedMessage.newBuilder().setBb(1)) + .setSingleDuration(Duration.newBuilder().setSeconds(3)) + .setSingleTimestamp(Timestamp.newBuilder().setSeconds(4)) + .setSingleInt64Wrapper(Int64Value.of(7)) + .setSingleStringWrapper(StringValue.of("")) + .setFieldMask(FieldMask.newBuilder().addPaths("p")) + .addRepeatedInt32(-1) + .addRepeatedInt64(2L) + .addRepeatedUint64(-1L) + .addRepeatedSint64(-6L) + .addRepeatedSfixed64(-10L) + .addRepeatedFloat(1.5f) + .addRepeatedDouble(-0.0) + .addRepeatedString("r") + .addRepeatedBytes(ByteString.copyFromUtf8("rb")) + .addRepeatedNestedMessage(NestedMessage.newBuilder().setBb(2)) + .addRepeatedNestedEnumValue(43) + .putMapBoolBool(true, false) + .putMapInt32Uint64(-1, -1L) + .putMapInt32EnumValue(1, 99) + .putMapStringMessage("m", NestedMessage.newBuilder().setBb(3).build()) + .putMapInt64NestedType(5L, NestedTestAllTypes.getDefaultInstance()) + .setOneofBool(false) + .build()), + PROTO2_POPULATED( + dev.cel.expr.conformance.proto2.TestAllTypes.newBuilder() + .setSingleInt32(-32) + .setSingleBool(false) + .setSingleString("") + .setStandaloneEnum(NestedEnum.BAZ) + .addRepeatedNestedEnum(NestedEnum.BAR) + .putMapInt32Enum(1, NestedEnum.BAZ) + .setOneofType(dev.cel.expr.conformance.proto2.NestedTestAllTypes.getDefaultInstance()) + .build()); + + private final MessageLite message; + + GetterParityTestCase(MessageLite message) { + this.message = message; + } + } + + @Test + public void fieldReads_throughGetters_matchWireBytes( + @TestParameter GetterParityTestCase testCase) { + ProtoMessageLiteValue throughGetters = + (ProtoMessageLiteValue) ALL_TYPES_CONVERTER.toRuntimeValue(testCase.message); + ProtoMessageLiteValue fromWire = + ProtoMessageLiteValue.create( + testCase.message.toByteString(), throughGetters.celType().name(), ALL_TYPES_CONVERTER); + + for (FieldLiteDescriptor field : + DESCRIPTOR_POOL.findDescriptor(testCase.message).get().getFieldDescriptors()) { + String name = field.getFieldName(); + SelectField selectField = SelectField.create(field.getFieldNumber(), name); + expect + .withMessage(name) + .that(outcome(() -> throughGetters.find(name))) + .isEqualTo(outcome(() -> fromWire.find(name))); + expect + .withMessage(name) + .that(outcome(() -> throughGetters.select(name))) + .isEqualTo(outcome(() -> fromWire.select(name))); + expect + .withMessage(name) + .that(outcome(() -> throughGetters.findByFieldNumber(selectField))) + .isEqualTo(outcome(() -> fromWire.findByFieldNumber(selectField))); + expect + .withMessage(name) + .that(outcome(() -> throughGetters.selectByFieldNumber(selectField))) + .isEqualTo(outcome(() -> fromWire.selectByFieldNumber(selectField))); + expect + .withMessage(name) + .that(outcome(() -> throughGetters.hasFieldByNumber(selectField))) + .isEqualTo(outcome(() -> fromWire.hasFieldByNumber(selectField))); + } + } + + @Test + public void selectByFieldNumber_unknownClosedEnumValueInMemory_readsAsUnset() throws Exception { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + CodedOutputStream output = CodedOutputStream.newInstance(bytes); + output.writeEnum(dev.cel.expr.conformance.proto2.TestAllTypes.STANDALONE_ENUM_FIELD_NUMBER, 99); + output.flush(); + dev.cel.expr.conformance.proto2.TestAllTypes message = + dev.cel.expr.conformance.proto2.TestAllTypes.parseFrom( + bytes.toByteArray(), ExtensionRegistryLite.getEmptyRegistry()); + ProtoMessageLiteValue inMemory = + (ProtoMessageLiteValue) ALL_TYPES_CONVERTER.toRuntimeValue(message); + ProtoMessageLiteValue fromWire = + ProtoMessageLiteValue.create( + message.toByteString(), inMemory.celType().name(), ALL_TYPES_CONVERTER); + SelectField field = + SelectField.create( + dev.cel.expr.conformance.proto2.TestAllTypes.STANDALONE_ENUM_FIELD_NUMBER, + "standalone_enum"); + + boolean inMemoryHasField = inMemory.hasFieldByNumber(field); + Object inMemoryValue = inMemory.selectByFieldNumber(field); + Object wireValue = fromWire.selectByFieldNumber(field); + + // Like full protobuf, getters don't see a value missing from the client's closed enum. + assertThat(inMemoryHasField).isFalse(); + assertThat(inMemoryValue).isEqualTo(0L); + assertThat(wireValue).isEqualTo(99L); + } + + // The value read, or the class of the exception thrown reading it. + private static Object outcome(Supplier read) { + try { + return read.get(); + } catch (RuntimeException e) { + return e.getClass(); + } + } + private static FieldLiteDescriptor fieldDescriptor(String messageName, int fieldNumber) { 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 bb93a19ce..b12be4335 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueTest.java @@ -51,7 +51,10 @@ import java.io.IOException; import java.time.Duration; import java.time.Instant; +import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.function.Consumer; import org.junit.Test; import org.junit.runner.RunWith; @@ -525,23 +528,24 @@ public void selectField_success(@TestParameter SelectFieldTestCase testCase) { assertThat(selectedValue).isEqualTo(testCase.value); } + @SuppressWarnings("ImmutableEnumChecker") // Test only private enum ContainerFieldTestCase { REPEATED( SelectField.create(TestAllTypes.REPEATED_INT64_FIELD_NUMBER, "repeated_int64", 3), - ImmutableList.class), + container -> ((List) container).clear()), MAP( SelectField.createMap( TestAllTypes.MAP_INT64_INT64_FIELD_NUMBER, "map_int64_int64", SelectField.MapEntrySpec.create(3, 3)), - ImmutableMap.class); + container -> ((Map) container).clear()); private final SelectField selectField; - private final Class immutableType; + private final Consumer clear; - ContainerFieldTestCase(SelectField selectField, Class immutableType) { + ContainerFieldTestCase(SelectField selectField, Consumer clear) { this.selectField = selectField; - this.immutableType = immutableType; + this.clear = clear; } } @@ -556,7 +560,7 @@ public void selectField_containerField_returnsImmutableContainer( Object selectedValue = protoMessageValue.select(testCase.selectField.fieldName()); - assertThat(selectedValue).isInstanceOf(testCase.immutableType); + assertThrows(UnsupportedOperationException.class, () -> testCase.clear.accept(selectedValue)); } @Test @@ -570,7 +574,7 @@ public void selectByFieldNumber_containerField_returnsImmutableContainer( Object selectedValue = protoMessageValue.selectByFieldNumber(testCase.selectField); - assertThat(selectedValue).isInstanceOf(testCase.immutableType); + assertThrows(UnsupportedOperationException.class, () -> testCase.clear.accept(selectedValue)); } @SuppressWarnings("ImmutableEnumChecker") // Test only diff --git a/java_lite_proto_cel_library.bzl b/java_lite_proto_cel_library.bzl index efed0bedc..b56fd7b66 100644 --- a/java_lite_proto_cel_library.bzl +++ b/java_lite_proto_cel_library.bzl @@ -27,7 +27,6 @@ def java_lite_proto_cel_library( Args: name: name of this target. deps: The list of proto_library rules to generate Java code for. - proto_src: Name of the proto_library target. java_descriptor_class_suffix (optional): Suffix for the Java class name of the generated CEL lite descriptor. Default is "CelLiteDescriptor". debug: (optional) If true, prints additional information during codegen for debugging purposes. diff --git a/protobuf/src/main/java/dev/cel/protobuf/BUILD.bazel b/protobuf/src/main/java/dev/cel/protobuf/BUILD.bazel index b2dac98e7..1aa8003aa 100644 --- a/protobuf/src/main/java/dev/cel/protobuf/BUILD.bazel +++ b/protobuf/src/main/java/dev/cel/protobuf/BUILD.bazel @@ -36,6 +36,7 @@ java_library( deps = [ "//common/annotations", "@maven//:com_google_errorprone_error_prone_annotations", + "@maven//:org_jspecify_jspecify", "@maven_android//:com_google_protobuf_protobuf_javalite", ], ) diff --git a/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java b/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java index 14973505c..034353445 100644 --- a/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java +++ b/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptor.java @@ -17,17 +17,21 @@ import static java.lang.Math.ceil; import com.google.errorprone.annotations.Immutable; +import com.google.protobuf.ByteString; import com.google.protobuf.MessageLite; import com.google.protobuf.WireFormat; import dev.cel.common.annotations.Internal; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; -import java.util.Optional; +import java.util.Set; import java.util.function.Supplier; +import org.jspecify.annotations.Nullable; /** * Base class for code generated CEL lite descriptors to extend from. @@ -37,6 +41,10 @@ @Internal @Immutable public abstract class CelLiteDescriptor { + private static final Set defaultScalarValues = + Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(false, 0, 0L, 0.0f, 0.0d, "", ByteString.EMPTY))); + @SuppressWarnings("Immutable") // Copied to unmodifiable map private final Map protoFqnToDescriptors; @@ -73,6 +81,13 @@ public static final class MessageLiteDescriptor { @SuppressWarnings("Immutable") // Does not alter the descriptor content private final Supplier messageBuilderSupplier; + // The generated descriptor declaring this message, and this message's position in it. + @SuppressWarnings("Immutable") // Set once, before the descriptor is published + private @Nullable CelLiteDescriptor owner; + + @SuppressWarnings("Immutable") // Set once, before the descriptor is published + private int index; + public String getProtoTypeName() { return fullyQualifiedProtoTypeName; } @@ -81,12 +96,12 @@ public List getFieldDescriptors() { return fieldLiteDescriptors; } - public Optional findByFieldNumber(int fieldNumber) { - return Optional.ofNullable(fieldNumberToFieldDescriptors.get(fieldNumber)); + public @Nullable FieldLiteDescriptor findByFieldNumber(int fieldNumber) { + return fieldNumberToFieldDescriptors.get(fieldNumber); } - public Optional findByFieldName(String fieldName) { - return Optional.ofNullable(fieldNameToFieldDescriptors.get(fieldName)); + public @Nullable FieldLiteDescriptor findByFieldName(String fieldName) { + return fieldNameToFieldDescriptors.get(fieldName); } public FieldLiteDescriptor getByFieldNameOrThrow(String fieldName) { @@ -94,9 +109,11 @@ public FieldLiteDescriptor getByFieldNameOrThrow(String fieldName) { } public FieldLiteDescriptor getByFieldNumberOrThrow(int fieldNumber) { - return findByFieldNumber(fieldNumber) - .orElseThrow( - () -> new NoSuchElementException("Could not find field number: " + fieldNumber)); + FieldLiteDescriptor fd = fieldNumberToFieldDescriptors.get(fieldNumber); + if (fd == null) { + throw new NoSuchElementException("Could not find field number: " + fieldNumber); + } + return fd; } /** Gets the builder for the message. Returns null for maps. */ @@ -104,6 +121,21 @@ public MessageLite.Builder newMessageBuilder() { return messageBuilderSupplier.get(); } + /** Whether {@link #getFieldValue} reads this message's fields with their generated getters. */ + public boolean hasGetters() { + return owner != null; + } + + /** + * Reads field {@code fieldNumber} of {@code message}, an instance of this message type, with + * its generated getter: the Java value (an open enum's number; a repeated or map field's list + * or map, possibly empty), or null if a singular field is unset. Requires {@link #hasGetters} + * and one of this message type's fields; throws for groups, which CEL doesn't support. + */ + public @Nullable Object getFieldValue(MessageLite message, int fieldNumber) { + return Objects.requireNonNull(owner).getFieldValue(index, message, fieldNumber); + } + /** * CEL Library Internals. Do not use. * @@ -353,10 +385,30 @@ public FieldLiteDescriptor( } } + /** + * Implemented by generated descriptors to read field {@code fieldNumber} of {@code message}, an + * instance of their {@code messageIndex}th message type, with its generated getter. See {@link + * MessageLiteDescriptor#getFieldValue}. + */ + protected abstract @Nullable Object getFieldValue( + int messageIndex, Object message, int fieldNumber); + + /** Returns {@code value}, or null if it's its type's default (unset, for implicit presence). */ + protected static @Nullable Object nonDefault(Object value) { + // Float and Double equality compares bits, so -0.0 and NaN count as set, as in protobuf. + return defaultScalarValues.contains(value) ? null : value; + } + protected CelLiteDescriptor(String version, List messageInfoList) { Map protoFqnMap = new HashMap<>(getMapInitialCapacity(messageInfoList.size())); - for (MessageLiteDescriptor msgInfo : messageInfoList) { + for (int i = 0; i < messageInfoList.size(); i++) { + MessageLiteDescriptor msgInfo = messageInfoList.get(i); + // A descriptor reused from another CelLiteDescriptor keeps reading through that one. + if (msgInfo.owner == null) { + msgInfo.owner = this; + msgInfo.index = i; + } protoFqnMap.put(msgInfo.getProtoTypeName(), msgInfo); } diff --git a/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptorGenerator.java b/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptorGenerator.java index 8c4eaea1c..60370b9db 100644 --- a/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptorGenerator.java +++ b/protobuf/src/main/java/dev/cel/protobuf/CelLiteDescriptorGenerator.java @@ -117,7 +117,7 @@ public Integer call() throws Exception { private ImmutableList codegenCelLiteDescriptors( FileDescriptor targetFileDescriptor) throws Exception { - String javaPackageName = GeneratorNames.getFileJavaPackage(targetFileDescriptor.toProto()); + String javaPackageName = GeneratorNames.getFileJavaPackage(targetFileDescriptor); String javaClassName; List descriptors = targetFileDescriptor.getMessageTypes(); diff --git a/protobuf/src/main/java/dev/cel/protobuf/LiteDescriptorCodegenMetadata.java b/protobuf/src/main/java/dev/cel/protobuf/LiteDescriptorCodegenMetadata.java index 19372b9b4..4c92be776 100644 --- a/protobuf/src/main/java/dev/cel/protobuf/LiteDescriptorCodegenMetadata.java +++ b/protobuf/src/main/java/dev/cel/protobuf/LiteDescriptorCodegenMetadata.java @@ -103,6 +103,10 @@ public String getProtoFieldTypeEnumName() { public abstract String getFieldProtoTypeName(); + // Java expression reading this field from a message named m with its generated getter, or + // empty for groups, which CEL doesn't support. + public abstract String getGetterExpression(); + abstract FieldLiteDescriptor.JavaType getJavaType(); abstract FieldLiteDescriptor.Type getProtoFieldType(); @@ -131,6 +135,8 @@ abstract static class Builder { abstract Builder setFieldProtoTypeName(String fieldProtoTypeName); + abstract Builder setGetterExpression(String getterExpression); + abstract FieldLiteDescriptorMetadata build(); } diff --git a/protobuf/src/main/java/dev/cel/protobuf/ProtoDescriptorCollector.java b/protobuf/src/main/java/dev/cel/protobuf/ProtoDescriptorCollector.java index c2fe20557..c949b092b 100644 --- a/protobuf/src/main/java/dev/cel/protobuf/ProtoDescriptorCollector.java +++ b/protobuf/src/main/java/dev/cel/protobuf/ProtoDescriptorCollector.java @@ -18,8 +18,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.protobuf.Descriptors; import com.google.protobuf.Descriptors.Descriptor; +import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor.JavaType; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.GeneratorNames; @@ -35,6 +35,18 @@ */ final class ProtoDescriptorCollector { + private static final ImmutableSet FORBIDDEN_ACCESSOR_NAMES = + ImmutableSet.of( + "Class", + "DefaultInstanceForType", + "ParserForType", + "SerializedSize", + "AllFields", + "DescriptorForType", + "InitializationErrorString", + "UnknownFields", + "CachedSize"); + private final DebugPrinter debugPrinter; ImmutableList collectCodegenMetadata(Descriptor descriptor) { @@ -49,14 +61,15 @@ ImmutableList collectCodegenMetadata(Descriptor d for (Descriptor messageDescriptor : descriptorList) { LiteDescriptorCodegenMetadata.Builder descriptorCodegenBuilder = LiteDescriptorCodegenMetadata.newBuilder(); - for (Descriptors.FieldDescriptor fieldDescriptor : messageDescriptor.getFields()) { + for (FieldDescriptor fieldDescriptor : messageDescriptor.getFields()) { FieldLiteDescriptorMetadata.Builder fieldDescriptorCodegenBuilder = FieldLiteDescriptorMetadata.newBuilder() .setFieldNumber(fieldDescriptor.getNumber()) .setFieldName(fieldDescriptor.getName()) .setIsPacked(fieldDescriptor.isPacked()) .setJavaType(adaptJavaType(fieldDescriptor.getJavaType())) - .setProtoFieldType(adaptFieldProtoType(fieldDescriptor.getType())); + .setProtoFieldType(adaptFieldProtoType(fieldDescriptor.getType())) + .setGetterExpression(getterExpression(fieldDescriptor)); switch (fieldDescriptor.getJavaType()) { case ENUM: @@ -92,9 +105,8 @@ ImmutableList collectCodegenMetadata(Descriptor d descriptorCodegenBuilder.setProtoTypeName(messageDescriptor.getFullName()); // Maps are resolved as an actual Java map, and doesn't have a MessageLite.Builder associated. if (!messageDescriptor.getOptions().getMapEntry()) { - String sanitizedJavaClassName = - GeneratorNames.getBytecodeClassName(messageDescriptor).replace('$', '.'); - descriptorCodegenBuilder.setJavaClassName(sanitizedJavaClassName); + descriptorCodegenBuilder.setJavaClassName( + GeneratorNames.getQualifiedClassName(messageDescriptor)); } descriptorListBuilder.add(descriptorCodegenBuilder.build()); @@ -107,8 +119,83 @@ static ProtoDescriptorCollector newInstance(DebugPrinter debugPrinter) { return new ProtoDescriptorCollector(debugPrinter); } - private static FieldLiteDescriptor.Type adaptFieldProtoType( - Descriptors.FieldDescriptor.Type type) { + /** + * Returns a Java expression reading {@code field} from a message named {@code m} with its + * generated getter: null if the field is unset, and open enums as their numbers. Empty for + * groups, which CEL doesn't support. + */ + private static String getterExpression(FieldDescriptor field) { + if (field.getType() == FieldDescriptor.Type.GROUP) { + return ""; + } + String name = accessorName(field); + if (field.isMapField()) { + FieldDescriptor value = field.getMessageType().findFieldByNumber(2); + return "m.get" + name + (isOpenEnum(value) ? "Value" : "") + "Map()"; + } + String suffix = isOpenEnum(field) ? "Value" : ""; + if (field.isRepeated()) { + return "m.get" + name + suffix + "List()"; + } + String getter = "m.get" + name + suffix + "()"; + // Qualified so that each generated class doesn't add its own DEX method reference. + return field.hasPresence() + ? "m.has" + name + "() ? " + getter + " : null" + : "CelLiteDescriptor.nonDefault(" + getter + ")"; + } + + private static boolean isOpenEnum(FieldDescriptor field) { + return field.getJavaType() == JavaType.ENUM && !field.legacyEnumFieldTreatedAsClosed(); + } + + // Mirrors protobuf's Java generator (compiler/java/context.cc): fields whose accessors would + // clash with another field's get their numbers appended. + private static String accessorName(FieldDescriptor field) { + String name = capitalizedName(field); + for (FieldDescriptor other : field.getContainingType().getFields()) { + if (other != field) { + String otherName = capitalizedName(other); + if (clashesOneWay(field, name, other, otherName) + || clashesOneWay(other, otherName, field, name)) { + return name + field.getNumber(); + } + } + } + return name; + } + + // Whether other's getter is named like field's; or, if only field is repeated, like field's + // getXCount() or getXList(); or, if field is an open enum, like its getXValue(). + private static boolean clashesOneWay( + FieldDescriptor field, String name, FieldDescriptor other, String otherName) { + return otherName.equals(name) + || (field.isRepeated() + && !other.isRepeated() + && (otherName.equals(name + "Count") || otherName.equals(name + "List"))) + || (isOpenEnum(field) && otherName.equals(name + "Value")); + } + + // Mirrors protobuf's Java generator (compiler/java/names.cc): underscores and digits capitalize + // the next letter, and names clashing with inherited methods get a trailing underscore. + private static String capitalizedName(FieldDescriptor field) { + String fieldName = field.getName(); + StringBuilder name = new StringBuilder(fieldName.length() + 1); + boolean capitalizeNext = true; + for (char c : fieldName.toCharArray()) { + if (c == '_') { + capitalizeNext = true; + } else { + name.append(capitalizeNext ? Character.toUpperCase(c) : c); + capitalizeNext = Character.isDigit(c); + } + } + if (FORBIDDEN_ACCESSOR_NAMES.contains(name.toString())) { + name.append('_'); + } + return name.toString(); + } + + private static FieldLiteDescriptor.Type adaptFieldProtoType(FieldDescriptor.Type type) { switch (type) { case DOUBLE: return FieldLiteDescriptor.Type.DOUBLE; diff --git a/protobuf/src/main/java/dev/cel/protobuf/templates/cel_lite_descriptor_template.txt b/protobuf/src/main/java/dev/cel/protobuf/templates/cel_lite_descriptor_template.txt index 2d8515146..12c13d50a 100644 --- a/protobuf/src/main/java/dev/cel/protobuf/templates/cel_lite_descriptor_template.txt +++ b/protobuf/src/main/java/dev/cel/protobuf/templates/cel_lite_descriptor_template.txt @@ -70,6 +70,44 @@ public final class ${descriptor_class_name} extends CelLiteDescriptor { return Collections.unmodifiableList(descriptors); } + @java.lang.Override + protected java.lang.Object getFieldValue( + int messageIndex, java.lang.Object message, int fieldNumber) { + switch (messageIndex) { + <#list descriptor_metadata_list as descriptor_metadata> + <#if descriptor_metadata.javaClassName?? && descriptor_metadata.fieldDescriptors?has_content> + case ${descriptor_metadata?index}: + return getFieldValue${descriptor_metadata?index}( + (${descriptor_metadata.javaClassName}) message, fieldNumber); + + + default: + return null; + } + } +<#-- A method per message bounds each by one message's fields: HotSpot won't JIT over 8000 B. --> +<#-- java.lang names are qualified: the proto's package may declare its own Object or Override. --> +<#list descriptor_metadata_list as descriptor_metadata> +<#if descriptor_metadata.javaClassName?? && descriptor_metadata.fieldDescriptors?has_content> + + private static java.lang.Object getFieldValue${descriptor_metadata?index}( + ${descriptor_metadata.javaClassName} m, int fieldNumber) { + switch (fieldNumber) { + <#list descriptor_metadata.fieldDescriptors as field_descriptor> + case ${field_descriptor.fieldNumber}: + <#if field_descriptor.getterExpression?has_content> + return ${field_descriptor.getterExpression}; + <#else> + throw new java.lang.UnsupportedOperationException("Groups are not supported"); + + + default: + return null; + } + } + + + private ${descriptor_class_name}() { super("${version}", newDescriptors()); } diff --git a/protobuf/src/test/java/dev/cel/protobuf/BUILD.bazel b/protobuf/src/test/java/dev/cel/protobuf/BUILD.bazel index 635379aab..0df6ded78 100644 --- a/protobuf/src/test/java/dev/cel/protobuf/BUILD.bazel +++ b/protobuf/src/test/java/dev/cel/protobuf/BUILD.bazel @@ -12,6 +12,8 @@ java_test( deps = [ "//:java_truth", "//protobuf:cel_lite_descriptor", + "//testing/protos:accessor_names_cel_java_proto_lite", + "//testing/protos:accessor_names_java_proto_lite", "//testing/protos:test_all_types_cel_java_proto3_lite", "@cel_spec//proto/cel/expr/conformance/proto3:test_all_types_java_proto_lite", "@maven//:com_google_testparameterinjector_test_parameter_injector", @@ -24,15 +26,20 @@ java_test( name = "proto_descriptor_collector_test", srcs = ["ProtoDescriptorCollectorTest.java"], test_class = "dev.cel.protobuf.ProtoDescriptorCollectorTest", - runtime_deps = ["@maven//:com_google_protobuf_protobuf_java"], + # Also a runtime dep so that, in OSS, full protobuf precedes the lite runtime (from + # lite_descriptor_codegen_metadata) on the classpath: both define DescriptorProtos. + runtime_deps = ["@maven//:com_google_protobuf_protobuf_java"], # keep deps = [ "//:java_truth", "//protobuf:debug_printer", "//protobuf:lite_descriptor_codegen_metadata", "//protobuf:proto_descriptor_collector", + "//testing/protos:accessor_names_java_proto", "//testing/protos:multi_file_java_proto", + "@cel_spec//proto/cel/expr/conformance/proto2:test_all_types_java_proto", "@cel_spec//proto/cel/expr/conformance/proto3:test_all_types_java_proto", "@maven//:com_google_guava_guava", + "@maven//:com_google_protobuf_protobuf_java", "@maven//:com_google_testparameterinjector_test_parameter_injector", "@maven//:junit_junit", ], diff --git a/protobuf/src/test/java/dev/cel/protobuf/CelLiteDescriptorTest.java b/protobuf/src/test/java/dev/cel/protobuf/CelLiteDescriptorTest.java index 90969a17d..283fe42c7 100644 --- a/protobuf/src/test/java/dev/cel/protobuf/CelLiteDescriptorTest.java +++ b/protobuf/src/test/java/dev/cel/protobuf/CelLiteDescriptorTest.java @@ -25,8 +25,9 @@ import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor.EncodingType; import dev.cel.protobuf.CelLiteDescriptor.FieldLiteDescriptor.JavaType; import dev.cel.protobuf.CelLiteDescriptor.MessageLiteDescriptor; +import dev.cel.testing.testdata.AccessorNames; +import dev.cel.testing.testdata.AccessorNamesCelLiteDescriptor; import java.util.Map; -import java.util.Optional; import org.junit.Test; import org.junit.runner.RunWith; @@ -76,23 +77,23 @@ public void findByFieldName_declaredField_returnsDescriptor() { .getProtoTypeNamesToDescriptors() .get("cel.expr.conformance.proto3.TestAllTypes"); - Optional fieldLiteDescriptor = + FieldLiteDescriptor fieldLiteDescriptor = testAllTypesDescriptor.findByFieldName("single_string"); - assertThat(fieldLiteDescriptor.map(FieldLiteDescriptor::getFieldNumber)).hasValue(14); + assertThat(fieldLiteDescriptor.getFieldNumber()).isEqualTo(14); } @Test - public void findByFieldName_undeclaredField_returnsEmpty() { + public void findByFieldName_undeclaredField_returnsNull() { MessageLiteDescriptor testAllTypesDescriptor = TEST_ALL_TYPES_CEL_LITE_DESCRIPTOR .getProtoTypeNamesToDescriptors() .get("cel.expr.conformance.proto3.TestAllTypes"); - Optional fieldLiteDescriptor = + FieldLiteDescriptor fieldLiteDescriptor = testAllTypesDescriptor.findByFieldName("undeclared_field"); - assertThat(fieldLiteDescriptor).isEmpty(); + assertThat(fieldLiteDescriptor).isNull(); } @Test @@ -215,6 +216,23 @@ public void protoFieldType_numbersAndWireTypes(@TestParameter ProtoFieldTypeTest assertThat(testCase.type.toWireFormatFieldType()).isEqualTo(testCase.expectedWireType); } + @Test + public void getFieldValue_messageAfterRoot_readsField() { + AccessorNames.Nested message = AccessorNames.Nested.newBuilder().setX(7).build(); + + Object value = + accessorNamesDescriptor("AccessorNames.Nested") + .getFieldValue(message, AccessorNames.Nested.X_FIELD_NUMBER); + + assertThat(value).isEqualTo(7); + } + + private static MessageLiteDescriptor accessorNamesDescriptor(String name) { + return AccessorNamesCelLiteDescriptor.getDescriptor() + .getProtoTypeNamesToDescriptors() + .get("dev.cel.testing.testdata." + name); + } + @Test public void protoFieldType_forNumber_roundTripAllTypes( @TestParameter FieldLiteDescriptor.Type type) { diff --git a/protobuf/src/test/java/dev/cel/protobuf/ProtoDescriptorCollectorTest.java b/protobuf/src/test/java/dev/cel/protobuf/ProtoDescriptorCollectorTest.java index 9e1e30005..d132a8a7f 100644 --- a/protobuf/src/test/java/dev/cel/protobuf/ProtoDescriptorCollectorTest.java +++ b/protobuf/src/test/java/dev/cel/protobuf/ProtoDescriptorCollectorTest.java @@ -14,12 +14,18 @@ package dev.cel.protobuf; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.protobuf.Descriptors.Descriptor; +import com.google.testing.junit.testparameterinjector.TestParameter; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import dev.cel.expr.conformance.proto3.NestedTestAllTypes; import dev.cel.expr.conformance.proto3.TestAllTypes; +import dev.cel.protobuf.LiteDescriptorCodegenMetadata.FieldLiteDescriptorMetadata; +import dev.cel.testing.testdata.AccessorNames; import dev.cel.testing.testdata.MultiFile; import org.junit.Test; import org.junit.runner.RunWith; @@ -42,6 +48,103 @@ public void collectCodegenMetadata_containsAllDescriptors() { assertThat(nestedTestAllTypesDescriptors).hasSize(1); } + @SuppressWarnings("ImmutableEnumChecker") // Test only + private enum GetterExpressionTestCase { + REPEATED_CLASHING_WITH_COUNT(AccessorNames.getDescriptor(), "foo", "m.getFoo1List()"), + CLASHING_WITH_REPEATED_COUNT( + AccessorNames.getDescriptor(), + "foo_count", + "CelLiteDescriptor.nonDefault(m.getFooCount2())"), + REPEATED_CLASHING_WITH_LIST(AccessorNames.getDescriptor(), "bar", "m.getBar3List()"), + CLASHING_WITH_REPEATED_LIST( + AccessorNames.getDescriptor(), "bar_list", "CelLiteDescriptor.nonDefault(m.getBarList4())"), + OPEN_ENUM_CLASHING_WITH_VALUE( + AccessorNames.getDescriptor(), "kind", "CelLiteDescriptor.nonDefault(m.getKind5Value())"), + CLASHING_WITH_OPEN_ENUM_VALUE( + AccessorNames.getDescriptor(), + "kind_value", + "CelLiteDescriptor.nonDefault(m.getKindValue6())"), + MAP_CLASHING_WITH_COUNT(AccessorNames.getDescriptor(), "baz", "m.getBaz7Map()"), + CLASHING_WITH_MAP_COUNT( + AccessorNames.getDescriptor(), + "baz_count", + "CelLiteDescriptor.nonDefault(m.getBazCount8())"), + FORBIDDEN_NAME( + AccessorNames.getDescriptor(), "class", "CelLiteDescriptor.nonDefault(m.getClass_())"), + SAME_CAPITALIZED_NAME( + AccessorNames.getDescriptor(), + "field1name", + "CelLiteDescriptor.nonDefault(m.getField1Name10())"), + OTHER_SAME_CAPITALIZED_NAME( + AccessorNames.getDescriptor(), + "field1_name", + "CelLiteDescriptor.nonDefault(m.getField1Name11())"), + SINGULAR_NEAR_MISS( + AccessorNames.getDescriptor(), "qux", "CelLiteDescriptor.nonDefault(m.getQux())"), + SINGULAR_COUNT_NEAR_MISS( + AccessorNames.getDescriptor(), + "qux_count", + "CelLiteDescriptor.nonDefault(m.getQuxCount())"), + NON_ENUM_VALUE_NEAR_MISS( + AccessorNames.getDescriptor(), + "qux_value", + "CelLiteDescriptor.nonDefault(m.getQuxValue())"), + REPEATED_NEAR_MISS(AccessorNames.getDescriptor(), "quux", "m.getQuuxList()"), + REPEATED_COUNT_NEAR_MISS(AccessorNames.getDescriptor(), "quux_count", "m.getQuuxCountList()"), + FORBIDDEN_CASE_NEAR_MISS( + AccessorNames.getDescriptor(), + "serializedsize", + "CelLiteDescriptor.nonDefault(m.getSerializedsize())"), + MESSAGE_WITH_PRESENCE( + AccessorNames.getDescriptor(), "nested", "m.hasNested() ? m.getNested() : null"), + OPEN_ENUM_REPEATED( + TestAllTypes.getDescriptor(), "repeated_nested_enum", "m.getRepeatedNestedEnumValueList()"), + OPEN_ENUM_MAP_VALUE( + TestAllTypes.getDescriptor(), "map_int32_enum", "m.getMapInt32EnumValueMap()"), + CLOSED_ENUM_SINGULAR( + dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor(), + "standalone_enum", + "m.hasStandaloneEnum() ? m.getStandaloneEnum() : null"), + CLOSED_ENUM_REPEATED( + dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor(), + "repeated_nested_enum", + "m.getRepeatedNestedEnumList()"), + CLOSED_ENUM_MAP_VALUE( + dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor(), + "map_int32_enum", + "m.getMapInt32EnumMap()"), + GROUP(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor(), "nestedgroup", ""); + + private final Descriptor descriptor; + private final String fieldName; + private final String expectedExpression; + + GetterExpressionTestCase(Descriptor descriptor, String fieldName, String expectedExpression) { + this.descriptor = descriptor; + this.fieldName = fieldName; + this.expectedExpression = expectedExpression; + } + } + + @Test + public void collectCodegenMetadata_getterExpression( + @TestParameter GetterExpressionTestCase testCase) { + ProtoDescriptorCollector collector = + ProtoDescriptorCollector.newInstance(DebugPrinter.newInstance(false)); + + ImmutableList descriptors = + collector.collectCodegenMetadata(testCase.descriptor); + + ImmutableMap expressionsByFieldName = + descriptors.get(0).getFieldDescriptors().stream() + .collect( + toImmutableMap( + FieldLiteDescriptorMetadata::getFieldName, + FieldLiteDescriptorMetadata::getGetterExpression)); + assertThat(expressionsByFieldName) + .containsEntry(testCase.fieldName, testCase.expectedExpression); + } + @Test public void collectCodegenMetadata_withProtoDependencies_containsAllDescriptors() { ProtoDescriptorCollector collector = diff --git a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java index e850a6817..149c0afe8 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelLiteRuntimeVersionSkewTest.java @@ -27,6 +27,7 @@ import com.google.protobuf.CodedOutputStream; import com.google.protobuf.ExtensionRegistryLite; import com.google.protobuf.Int64Value; +import com.google.protobuf.MessageLite; import com.google.protobuf.NullValue; import com.google.protobuf.Struct; import com.google.protobuf.UnknownFieldSet; @@ -301,7 +302,8 @@ public void setUp() { } } - CelLiteDescriptor clientDescriptor = new CelLiteDescriptor("v1", allMsgDescs.build()) {}; + CelLiteDescriptor clientDescriptor = + newClientCelLiteDescriptor("v1", fullDescriptor, allMsgDescs.build()); ProtoMessageLiteValueProvider clientValueProvider = ProtoMessageLiteValueProvider.newInstance( clientDescriptor, NestedTestAllTypesCelDescriptor.getDescriptor()); @@ -2253,7 +2255,10 @@ public void partialDescriptorRuntime_missingChildDescriptor_evaluatesFromWireByt .setStandardFunctions(CelStandardFunctions.ALL_STANDARD_FUNCTIONS) .setValueProvider( ProtoMessageLiteValueProvider.newInstance( - new CelLiteDescriptor("partial", withoutChildMessages) {})) + newClientCelLiteDescriptor( + "partial", + TestAllTypesCelDescriptor.getDescriptor(), + withoutChildMessages))) .setContainer(CEL_CONTAINER) .build(); CelAbstractSyntaxTree optimizedAst = @@ -2301,7 +2306,10 @@ public void partialDescriptorRuntime_unoptimizedAst_throwsDescriptiveException( .setStandardFunctions(CelStandardFunctions.ALL_STANDARD_FUNCTIONS) .setValueProvider( ProtoMessageLiteValueProvider.newInstance( - new CelLiteDescriptor("partial", withoutChildMessages) {})) + newClientCelLiteDescriptor( + "partial", + TestAllTypesCelDescriptor.getDescriptor(), + withoutChildMessages))) .setContainer(CEL_CONTAINER) .build(); CelAbstractSyntaxTree unoptimizedAst = serverCompiler.compile(expression).getAst(); @@ -2513,7 +2521,8 @@ private static Object evalProto2( MessageLiteDescriptor clientProto2MsgDesc = buildClientTestAllTypesDescriptor(fullProto2MsgDesc, "single_int32"); CelLiteDescriptor clientProto2Descriptor = - new CelLiteDescriptor("v1_proto2", ImmutableList.of(clientProto2MsgDesc)) {}; + newClientCelLiteDescriptor( + "v1_proto2", fullProto2Descriptor, ImmutableList.of(clientProto2MsgDesc)); CelLiteRuntime clientProto2Runtime = CelLiteRuntimeFactory.newLiteRuntimeBuilder() .setStandardFunctions(CelStandardFunctions.ALL_STANDARD_FUNCTIONS) @@ -2574,6 +2583,22 @@ private static MessageLiteDescriptor buildClientTestAllTypesDescriptor( fullMsgDesc.getProtoTypeName(), clientFields.build(), fullMsgDesc::newMessageBuilder); } + // Reads the client's message types through the full descriptor's getters for the same types. + private static CelLiteDescriptor newClientCelLiteDescriptor( + String version, + CelLiteDescriptor fullDescriptor, + ImmutableList descriptors) { + return new CelLiteDescriptor(version, descriptors) { + @Override + protected @Nullable Object getFieldValue(int messageIndex, Object message, int fieldNumber) { + return fullDescriptor + .getProtoTypeNamesToDescriptors() + .get(descriptors.get(messageIndex).getProtoTypeName()) + .getFieldValue((MessageLite) message, fieldNumber); + } + }; + } + private static CelLiteRuntime newRuntimeWithoutNestedMessageDescriptor() { CelLiteDescriptor fullDescriptor = TestAllTypesCelDescriptor.getDescriptor(); MessageLiteDescriptor clientTestAllTypesDesc = @@ -2582,7 +2607,8 @@ private static CelLiteRuntime newRuntimeWithoutNestedMessageDescriptor() { .getProtoTypeNamesToDescriptors() .get(TestAllTypes.getDescriptor().getFullName())); CelLiteDescriptor partialDescriptor = - new CelLiteDescriptor("v1_no_nested", ImmutableList.of(clientTestAllTypesDesc)) {}; + newClientCelLiteDescriptor( + "v1_no_nested", fullDescriptor, ImmutableList.of(clientTestAllTypesDesc)); return CelLiteRuntimeFactory.newLiteRuntimeBuilder() .setStandardFunctions(CelStandardFunctions.ALL_STANDARD_FUNCTIONS) .setValueProvider(ProtoMessageLiteValueProvider.newInstance(partialDescriptor)) @@ -2599,8 +2625,8 @@ private static CelLiteRuntime newRuntimeWithoutFieldDescriptor(String excludedFi .get(TestAllTypes.getDescriptor().getFullName()), excludedFieldName); CelLiteDescriptor partialDescriptor = - new CelLiteDescriptor( - "v1_no_" + excludedFieldName, ImmutableList.of(clientTestAllTypesDesc)) {}; + newClientCelLiteDescriptor( + "v1_no_" + excludedFieldName, fullDescriptor, ImmutableList.of(clientTestAllTypesDesc)); return CelLiteRuntimeFactory.newLiteRuntimeBuilder() .setStandardFunctions(CelStandardFunctions.ALL_STANDARD_FUNCTIONS) .setValueProvider(ProtoMessageLiteValueProvider.newInstance(partialDescriptor)) @@ -2632,7 +2658,13 @@ private static CelLiteRuntime newOlderClassV1Runtime() { ImmutableList.of(v1SingleInt32Field), NestedMessage::newBuilder); CelLiteDescriptor v1OlderClassDescriptor = - new CelLiteDescriptor("v1_older_class", ImmutableList.of(v1OlderClassTestAllTypesDesc)) {}; + new CelLiteDescriptor("v1_older_class", ImmutableList.of(v1OlderClassTestAllTypesDesc)) { + @Override + protected @Nullable Object getFieldValue( + int messageIndex, Object message, int fieldNumber) { + return fieldNumber == 1 ? nonDefault(((NestedMessage) message).getBb()) : null; + } + }; return CelLiteRuntimeFactory.newLiteRuntimeBuilder() .setStandardFunctions(CelStandardFunctions.ALL_STANDARD_FUNCTIONS) .setValueProvider( diff --git a/testing/protos/BUILD.bazel b/testing/protos/BUILD.bazel index 17706ca45..ac0c691b0 100644 --- a/testing/protos/BUILD.bazel +++ b/testing/protos/BUILD.bazel @@ -29,6 +29,21 @@ alias( actual = "//testing/src/test/resources/protos:message_with_enum_java_proto", ) +alias( + name = "accessor_names_java_proto", + actual = "//testing/src/test/resources/protos:accessor_names_java_proto", +) + +alias( + name = "accessor_names_java_proto_lite", + actual = "//testing/src/test/resources/protos:accessor_names_java_proto_lite", +) + +alias( + name = "accessor_names_cel_java_proto_lite", + actual = "//testing/src/test/resources/protos:accessor_names_cel_java_proto_lite", +) + alias( name = "multi_file_cel_java_proto_lite", actual = "//testing/src/test/resources/protos:multi_file_cel_java_proto_lite", diff --git a/testing/src/test/resources/protos/BUILD.bazel b/testing/src/test/resources/protos/BUILD.bazel index 1fac2e1f0..2717e70ff 100644 --- a/testing/src/test/resources/protos/BUILD.bazel +++ b/testing/src/test/resources/protos/BUILD.bazel @@ -1,3 +1,4 @@ +load("@com_google_protobuf//bazel:java_lite_proto_library.bzl", "java_lite_proto_library") load("@com_google_protobuf//bazel:java_proto_library.bzl", "java_proto_library") load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library") load("//:java_lite_proto_cel_library.bzl", "java_lite_proto_cel_library") @@ -67,6 +68,26 @@ java_proto_library( ], ) +proto_library( + name = "accessor_names_proto", + srcs = ["accessor_names.proto"], +) + +java_proto_library( + name = "accessor_names_java_proto", + deps = [":accessor_names_proto"], +) + +java_lite_proto_library( + name = "accessor_names_java_proto_lite", + deps = [":accessor_names_proto"], +) + +java_lite_proto_cel_library( + name = "accessor_names_cel_java_proto_lite", + deps = [":accessor_names_proto"], +) + java_lite_proto_cel_library( name = "multi_file_cel_java_proto_lite", deps = [ diff --git a/testing/src/test/resources/protos/accessor_names.proto b/testing/src/test/resources/protos/accessor_names.proto new file mode 100644 index 000000000..e62a45cef --- /dev/null +++ b/testing/src/test/resources/protos/accessor_names.proto @@ -0,0 +1,71 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +package dev.cel.testing.testdata; + +option java_multiple_files = true; +option java_outer_classname = "AccessorNamesProto"; +option java_package = "dev.cel.testing.testdata"; + +// Fields whose Java accessor names protobuf doesn't derive by plain +// camel-casing, and near misses that it doesn't rename, for testing generated +// getters, which fail to compile if misnamed. +message AccessorNames { + enum Kind { + KIND_UNSPECIFIED = 0; + KIND_ON = 1; + } + + message Nested { + int32 x = 1; + } + + // Clashes with the getFooCount() of the repeated foo. + repeated int32 foo = 1; + int64 foo_count = 2; + + // Clashes with the getBarList() of the repeated bar. + repeated string bar = 3; + string bar_list = 4; + + // Clashes with the getKindValue() of the open enum kind. + Kind kind = 5; + int32 kind_value = 6; + + // Clashes with the getBazCount() of the map baz. + map baz = 7; + int32 baz_count = 8; + + // Clashes with Object.getClass(), so it gets a trailing underscore. + int32 class = 9; + + // A digit capitalizes the next letter, so both are Field1Name and get their + // numbers. + string field1name = 10; + string field1_name = 11; + + // Near misses: qux is neither repeated nor an enum, quux_count is repeated + // too, and only exact names of inherited methods get a trailing underscore. + int32 qux = 12; + int32 qux_count = 13; + int32 qux_value = 14; + repeated int32 quux = 15; + repeated int32 quux_count = 16; + int32 serializedsize = 17; + + // A message type after the root one, read through its own generated method. + Nested nested = 18; +}