From 8d2c7e3db7fddb7313454de6d9d446e1c1be4e93 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Tue, 29 Apr 2025 14:59:20 -0700 Subject: [PATCH] Add BaseProtoMessageValueProvider to decouple full/lite protobuf value providers PiperOrigin-RevId: 752895239 --- .../java/dev/cel/common/values/BUILD.bazel | 63 +++++++++++++++++-- .../values/BaseProtoMessageValueProvider.java | 31 +++++++++ .../cel/common/values/CelValueConverter.java | 2 + .../cel/common/values/CelValueProvider.java | 30 --------- .../values/CombinedCelValueProvider.java | 60 ++++++++++++++++++ .../values/ProtoMessageLiteValueProvider.java | 5 +- .../values/ProtoMessageValueProvider.java | 23 +++---- .../java/dev/cel/common/values/BUILD.bazel | 1 + .../ProtoMessageLiteValueProviderTest.java | 2 +- .../values/ProtoMessageValueProviderTest.java | 15 +---- .../cel/common/values/StructValueTest.java | 51 +++++++++++++++ common/values/BUILD.bazel | 20 ++++++ .../src/main/java/dev/cel/runtime/BUILD.bazel | 13 ++-- .../cel/runtime/CelLiteRuntimeBuilder.java | 7 ++- .../dev/cel/runtime/CelRuntimeBuilder.java | 7 ++- .../dev/cel/runtime/CelRuntimeLegacyImpl.java | 15 ++--- .../runtime/CelValueRuntimeTypeProvider.java | 42 +++++++++++-- .../java/dev/cel/runtime/LiteRuntimeImpl.java | 15 +---- 18 files changed, 294 insertions(+), 108 deletions(-) create mode 100644 common/src/main/java/dev/cel/common/values/BaseProtoMessageValueProvider.java create mode 100644 common/src/main/java/dev/cel/common/values/CombinedCelValueProvider.java 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 904789836..1b05b89f4 100644 --- a/common/src/main/java/dev/cel/common/values/BUILD.bazel +++ b/common/src/main/java/dev/cel/common/values/BUILD.bazel @@ -89,6 +89,32 @@ cel_android_library( ], ) +java_library( + name = "combined_cel_value_provider", + srcs = ["CombinedCelValueProvider.java"], + tags = [ + ], + deps = [ + "//common/values:cel_value", + "//common/values:cel_value_provider", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven//:com_google_guava_guava", + ], +) + +cel_android_library( + name = "combined_cel_value_provider_android", + srcs = ["CombinedCelValueProvider.java"], + tags = [ + ], + deps = [ + ":cel_value_android", + "//common/values:cel_value_provider_android", + "@maven//:com_google_errorprone_error_prone_annotations", + "@maven_android//:com_google_guava_guava", + ], +) + java_library( name = "values", srcs = CEL_VALUES_SOURCES, @@ -207,14 +233,13 @@ java_library( tags = [ ], deps = [ + ":base_proto_message_value_provider", ":cel_value", - ":cel_value_provider", ":proto_message_value", - "//common:error_codes", - "//common:runtime_exception", "//common/annotations", "//common/internal:dynamic_proto", "//common/internal:proto_message_factory", + "//common/values:base_proto_cel_value_converter", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_protobuf_protobuf_java", ], @@ -280,11 +305,12 @@ java_library( tags = [ ], deps = [ + ":base_proto_message_value_provider", ":cel_value", - ":cel_value_provider", ":proto_message_lite_value", "//common/internal:cel_lite_descriptor_pool", "//common/internal:default_lite_descriptor_pool", + "//common/values:base_proto_cel_value_converter", "//protobuf:cel_lite_descriptor", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", @@ -298,14 +324,41 @@ cel_android_library( tags = [ ], deps = [ + ":base_proto_message_value_provider_android", ":cel_value_android", - ":cel_value_provider_android", ":proto_message_lite_value_android", "//common/internal:cel_lite_descriptor_pool_android", "//common/internal:default_lite_descriptor_pool_android", + "//common/values:base_proto_cel_value_converter_android", "//protobuf:cel_lite_descriptor", "@maven//:com_google_errorprone_error_prone_annotations", "@maven_android//:com_google_guava_guava", "@maven_android//:com_google_protobuf_protobuf_javalite", ], ) + +java_library( + name = "base_proto_message_value_provider", + srcs = ["BaseProtoMessageValueProvider.java"], + tags = [ + ], + deps = [ + "//common/annotations", + "//common/values:base_proto_cel_value_converter", + "//common/values:cel_value_provider", + "@maven//:com_google_errorprone_error_prone_annotations", + ], +) + +cel_android_library( + name = "base_proto_message_value_provider_android", + srcs = ["BaseProtoMessageValueProvider.java"], + tags = [ + ], + deps = [ + "//common/annotations", + "//common/values:base_proto_cel_value_converter_android", + "//common/values:cel_value_provider_android", + "@maven//:com_google_errorprone_error_prone_annotations", + ], +) diff --git a/common/src/main/java/dev/cel/common/values/BaseProtoMessageValueProvider.java b/common/src/main/java/dev/cel/common/values/BaseProtoMessageValueProvider.java new file mode 100644 index 000000000..f42a16179 --- /dev/null +++ b/common/src/main/java/dev/cel/common/values/BaseProtoMessageValueProvider.java @@ -0,0 +1,31 @@ +// Copyright 2025 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. + +package dev.cel.common.values; + +import com.google.errorprone.annotations.Immutable; +import dev.cel.common.annotations.Internal; + +/** + * {@code BaseProtoMessageValueProvider} is a common parent to {@code ProtoMessageValueProvider} and + * {@code ProtoMessageLiteValueProvider}. + * + *

CEL-Java internals. Do not use. Use one of the inherited variants mentioned above. + */ +@Internal +@Immutable +public abstract class BaseProtoMessageValueProvider implements CelValueProvider { + + public abstract BaseProtoCelValueConverter protoCelValueConverter(); +} diff --git a/common/src/main/java/dev/cel/common/values/CelValueConverter.java b/common/src/main/java/dev/cel/common/values/CelValueConverter.java index 15db4f562..1489d7b70 100644 --- a/common/src/main/java/dev/cel/common/values/CelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/CelValueConverter.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.primitives.UnsignedLong; +import com.google.errorprone.annotations.Immutable; import dev.cel.common.annotations.Internal; import java.util.Map; import java.util.Map.Entry; @@ -31,6 +32,7 @@ */ @SuppressWarnings("unchecked") // Unchecked cast of generics due to type-erasure (ex: MapValue). @Internal +@Immutable abstract class CelValueConverter { /** Adapts a {@link CelValue} to a plain old Java Object. */ diff --git a/common/src/main/java/dev/cel/common/values/CelValueProvider.java b/common/src/main/java/dev/cel/common/values/CelValueProvider.java index 0e896e7ac..e5080e180 100644 --- a/common/src/main/java/dev/cel/common/values/CelValueProvider.java +++ b/common/src/main/java/dev/cel/common/values/CelValueProvider.java @@ -14,8 +14,6 @@ package dev.cel.common.values; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; import com.google.errorprone.annotations.Immutable; import java.util.Map; import java.util.Optional; @@ -31,32 +29,4 @@ public interface CelValueProvider { * special cases such as wrappers where its primitive is returned. */ Optional newValue(String structType, Map fields); - - /** - * The {@link CombinedCelValueProvider} takes one or more {@link CelValueProvider} instances and - * attempts to create a {@link CelValue} instance for a given struct type name by calling each - * value provider in the order that they are provided to the constructor. - */ - @Immutable - final class CombinedCelValueProvider implements CelValueProvider { - private final ImmutableList celValueProviders; - - public CombinedCelValueProvider(CelValueProvider first, CelValueProvider second) { - Preconditions.checkNotNull(first); - Preconditions.checkNotNull(second); - celValueProviders = ImmutableList.of(first, second); - } - - @Override - public Optional newValue(String structType, Map fields) { - for (CelValueProvider provider : celValueProviders) { - Optional newValue = provider.newValue(structType, fields); - if (newValue.isPresent()) { - return newValue; - } - } - - return Optional.empty(); - } - } } diff --git a/common/src/main/java/dev/cel/common/values/CombinedCelValueProvider.java b/common/src/main/java/dev/cel/common/values/CombinedCelValueProvider.java new file mode 100644 index 000000000..a674beb8d --- /dev/null +++ b/common/src/main/java/dev/cel/common/values/CombinedCelValueProvider.java @@ -0,0 +1,60 @@ +// Copyright 2025 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. + +package dev.cel.common.values; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.Immutable; +import java.util.Map; +import java.util.Optional; + +/** + * The {@link CombinedCelValueProvider} takes one or more {@link CelValueProvider} instances and + * attempts to create a {@link CelValue} instance for a given struct type name by calling each value + * provider in the order that they are provided to the constructor. + */ +@Immutable +public final class CombinedCelValueProvider implements CelValueProvider { + private final ImmutableList celValueProviders; + + /** Combines the provided first and second {@link CelValueProvider}. */ + public static CombinedCelValueProvider combine(CelValueProvider... providers) { + checkArgument(providers.length >= 2, "You must provide two or more providers"); + return new CombinedCelValueProvider(ImmutableList.copyOf(providers)); + } + + @Override + public Optional newValue(String structType, Map fields) { + for (CelValueProvider provider : celValueProviders) { + Optional newValue = provider.newValue(structType, fields); + if (newValue.isPresent()) { + return newValue; + } + } + + return Optional.empty(); + } + + /** Returns the underlying {@link CelValueProvider}s in order. */ + public ImmutableList valueProviders() { + return celValueProviders; + } + + private CombinedCelValueProvider(ImmutableList providers) { + celValueProviders = checkNotNull(providers); + } +} 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 bc8e588d8..90f667698 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValueProvider.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageLiteValueProvider.java @@ -30,11 +30,12 @@ * fully qualified name and its fields to populate. */ @Immutable -public class ProtoMessageLiteValueProvider implements CelValueProvider { +public class ProtoMessageLiteValueProvider extends BaseProtoMessageValueProvider { private final CelLiteDescriptorPool descriptorPool; private final ProtoLiteCelValueConverter protoLiteCelValueConverter; - public ProtoLiteCelValueConverter getProtoLiteCelValueConverter() { + @Override + public BaseProtoCelValueConverter protoCelValueConverter() { return protoLiteCelValueConverter; } diff --git a/common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java b/common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java index d7bab0d7f..1b2bb112d 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java +++ b/common/src/main/java/dev/cel/common/values/ProtoMessageValueProvider.java @@ -18,8 +18,6 @@ import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Message; -import dev.cel.common.CelErrorCode; -import dev.cel.common.CelRuntimeException; import dev.cel.common.annotations.Internal; import dev.cel.common.internal.DynamicProto; import dev.cel.common.internal.ProtoAdapter; @@ -35,19 +33,23 @@ */ @Immutable @Internal -public class ProtoMessageValueProvider implements CelValueProvider { +public class ProtoMessageValueProvider extends BaseProtoMessageValueProvider { private final ProtoAdapter protoAdapter; private final ProtoMessageFactory protoMessageFactory; private final ProtoCelValueConverter protoCelValueConverter; - public ProtoCelValueConverter getProtoCelValueConverter() { + @Override + public BaseProtoCelValueConverter protoCelValueConverter() { return protoCelValueConverter; } @Override public Optional newValue(String structType, Map fields) { + Message.Builder builder = protoMessageFactory.newBuilder(structType).orElse(null); + if (builder == null) { + return Optional.empty(); + } try { - Message.Builder builder = getMessageBuilderOrThrow(structType); Descriptor descriptor = builder.getDescriptorForType(); for (Map.Entry entry : fields.entrySet()) { FieldDescriptor fieldDescriptor = findField(descriptor, entry.getKey()); @@ -63,17 +65,6 @@ public Optional newValue(String structType, Map fields } } - private Message.Builder getMessageBuilderOrThrow(String messageName) { - return protoMessageFactory - .newBuilder(messageName) - .orElseThrow( - () -> - new CelRuntimeException( - new IllegalArgumentException( - String.format("cannot resolve '%s' as a message", messageName)), - CelErrorCode.ATTRIBUTE_NOT_FOUND)); - } - private FieldDescriptor findField(Descriptor descriptor, String fieldName) { FieldDescriptor fieldDescriptor = descriptor.findFieldByName(fieldName); if (fieldDescriptor != null) { 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 23580157a..3eb0d91a8 100644 --- a/common/src/test/java/dev/cel/common/values/BUILD.bazel +++ b/common/src/test/java/dev/cel/common/values/BUILD.bazel @@ -28,6 +28,7 @@ java_library( "//common/values:cel_byte_string", "//common/values:cel_value", "//common/values:cel_value_provider", + "//common/values:combined_cel_value_provider", "//common/values:proto_message_lite_value", "//common/values:proto_message_lite_value_provider", "//common/values:proto_message_value", 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 74a288f14..fad6cd6cb 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueProviderTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageLiteValueProviderTest.java @@ -49,6 +49,6 @@ public void newValue_emptyFields_success() { @Test public void getProtoLiteCelValueConverter() { - assertThat(VALUE_PROVIDER.getProtoLiteCelValueConverter()).isNotNull(); + assertThat(VALUE_PROVIDER.protoCelValueConverter()).isNotNull(); } } 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 88fc0ae16..e8273dfae 100644 --- a/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java +++ b/common/src/test/java/dev/cel/common/values/ProtoMessageValueProviderTest.java @@ -22,14 +22,12 @@ import com.google.common.primitives.UnsignedLong; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import dev.cel.common.CelDescriptorUtil; -import dev.cel.common.CelRuntimeException; import dev.cel.common.internal.CelDescriptorPool; import dev.cel.common.internal.DefaultDescriptorPool; import dev.cel.common.internal.DefaultMessageFactory; import dev.cel.common.internal.DynamicProto; import dev.cel.common.internal.ProtoMessageFactory; import dev.cel.common.internal.ProtoTimeUtils; -import dev.cel.common.values.CelValueProvider.CombinedCelValueProvider; import dev.cel.expr.conformance.proto2.TestAllTypes; import dev.cel.expr.conformance.proto2.TestAllTypesExtensions; import java.time.Duration; @@ -206,18 +204,11 @@ public void newValue_createProtoMessage_extensionFieldsPopulated() { } @Test - public void newValue_invalidMessageName_throws() { + public void newValue_invalidMessageName_returnsEmpty() { ProtoMessageValueProvider protoMessageValueProvider = ProtoMessageValueProvider.newInstance(DYNAMIC_PROTO); - CelRuntimeException e = - assertThrows( - CelRuntimeException.class, - () -> protoMessageValueProvider.newValue("bogus", ImmutableMap.of())); - - assertThat(e) - .hasMessageThat() - .isEqualTo("java.lang.IllegalArgumentException: cannot resolve 'bogus' as a message"); + assertThat(protoMessageValueProvider.newValue("bogus", ImmutableMap.of())).isEmpty(); } @Test @@ -245,7 +236,7 @@ public void newValue_onCombinedProvider() { ProtoMessageValueProvider protoMessageValueProvider = ProtoMessageValueProvider.newInstance(DYNAMIC_PROTO); CelValueProvider combinedProvider = - new CombinedCelValueProvider(celValueProvider, protoMessageValueProvider); + CombinedCelValueProvider.combine(celValueProvider, protoMessageValueProvider); ProtoMessageValue protoMessageValue = (ProtoMessageValue) diff --git a/common/src/test/java/dev/cel/common/values/StructValueTest.java b/common/src/test/java/dev/cel/common/values/StructValueTest.java index bd4e820b8..acb2d6d56 100644 --- a/common/src/test/java/dev/cel/common/values/StructValueTest.java +++ b/common/src/test/java/dev/cel/common/values/StructValueTest.java @@ -26,10 +26,12 @@ import dev.cel.bundle.CelFactory; import dev.cel.common.CelAbstractSyntaxTree; import dev.cel.common.CelOptions; +import dev.cel.common.internal.DynamicProto; import dev.cel.common.types.CelType; import dev.cel.common.types.CelTypeProvider; import dev.cel.common.types.SimpleType; import dev.cel.common.types.StructType; +import dev.cel.expr.conformance.proto3.TestAllTypes; import java.util.Map; import java.util.Optional; import org.junit.Test; @@ -176,6 +178,55 @@ public void evaluate_usingCustomClass_selectField() throws Exception { assertThat(result).isEqualTo(5L); } + @Test + public void evaluate_usingMultipleProviders_selectFieldFromCustomClass() throws Exception { + Cel cel = + CelFactory.standardCelBuilder() + .setOptions(CelOptions.current().enableCelValue(true).build()) + .setTypeProvider(CUSTOM_STRUCT_TYPE_PROVIDER) + .setValueProvider( + CombinedCelValueProvider.combine( + ProtoMessageValueProvider.newInstance( + DynamicProto.create(typeName -> Optional.empty())), + CUSTOM_STRUCT_VALUE_PROVIDER)) + .build(); + CelAbstractSyntaxTree ast = cel.compile("custom_struct{data: 5}.data").getAst(); + + Object result = cel.createProgram(ast).eval(); + + assertThat(result).isEqualTo(5L); + } + + @Test + public void evaluate_usingMultipleProviders_selectFieldFromProtobufMessage() throws Exception { + Cel cel = + CelFactory.standardCelBuilder() + .setOptions(CelOptions.current().enableCelValue(true).build()) + .addMessageTypes(TestAllTypes.getDescriptor()) + .setTypeProvider(CUSTOM_STRUCT_TYPE_PROVIDER) + .setValueProvider( + CombinedCelValueProvider.combine( + ProtoMessageValueProvider.newInstance( + // Note: this is unideal. Future iterations should make DynamicProto + // completely an internal concern, and not expose it at all. + DynamicProto.create( + typeName -> { + if (typeName.equals(TestAllTypes.getDescriptor().getFullName())) { + return Optional.of(TestAllTypes.newBuilder()); + } + return Optional.empty(); + })), + CUSTOM_STRUCT_VALUE_PROVIDER)) + .build(); + CelAbstractSyntaxTree ast = + cel.compile("cel.expr.conformance.proto3.TestAllTypes{single_string: 'foo'}.single_string") + .getAst(); + + String result = (String) cel.createProgram(ast).eval(); + + assertThat(result).isEqualTo("foo"); + } + @SuppressWarnings("Immutable") // Test only private static class CelCustomStructValue extends StructValue { diff --git a/common/values/BUILD.bazel b/common/values/BUILD.bazel index e80f30293..08bad9298 100644 --- a/common/values/BUILD.bazel +++ b/common/values/BUILD.bazel @@ -27,6 +27,16 @@ cel_android_library( exports = ["//common/src/main/java/dev/cel/common/values:cel_value_provider_android"], ) +java_library( + name = "combined_cel_value_provider", + exports = ["//common/src/main/java/dev/cel/common/values:combined_cel_value_provider"], +) + +cel_android_library( + name = "combined_cel_value_provider_android", + exports = ["//common/src/main/java/dev/cel/common/values:combined_cel_value_provider_android"], +) + java_library( name = "values", exports = ["//common/src/main/java/dev/cel/common/values"], @@ -82,3 +92,13 @@ cel_android_library( name = "proto_message_lite_value_provider_android", exports = ["//common/src/main/java/dev/cel/common/values:proto_message_lite_value_provider_android"], ) + +java_library( + name = "base_proto_message_value_provider", + exports = ["//common/src/main/java/dev/cel/common/values:base_proto_message_value_provider"], +) + +cel_android_library( + name = "base_proto_message_value_provider_android", + exports = ["//common/src/main/java/dev/cel/common/values:base_proto_message_value_provider_android"], +) diff --git a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel index f8bd3a72f..dea69bad0 100644 --- a/runtime/src/main/java/dev/cel/runtime/BUILD.bazel +++ b/runtime/src/main/java/dev/cel/runtime/BUILD.bazel @@ -721,11 +721,7 @@ java_library( "//:auto_value", "//common:cel_ast", "//common:options", - "//common/internal:default_lite_descriptor_pool", - "//common/values:base_proto_cel_value_converter", "//common/values:cel_value_provider", - "//common/values:proto_message_lite_value", - "//common/values:proto_message_lite_value_provider", "@maven//:com_google_code_findbugs_annotations", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", @@ -753,11 +749,7 @@ cel_android_library( "//:auto_value", "//common:cel_ast_android", "//common:options", - "//common/internal:default_lite_descriptor_pool_android", - "//common/values:base_proto_cel_value_converter_android", "//common/values:cel_value_provider_android", - "//common/values:proto_message_lite_value_android", - "//common/values:proto_message_lite_value_provider_android", "@maven//:com_google_code_findbugs_annotations", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", @@ -858,8 +850,10 @@ java_library( "//common/annotations", "//common/values", "//common/values:base_proto_cel_value_converter", + "//common/values:base_proto_message_value_provider", "//common/values:cel_value", "//common/values:cel_value_provider", + "//common/values:combined_cel_value_provider", "@maven//:com_google_errorprone_error_prone_annotations", "@maven//:com_google_guava_guava", "@maven_android//:com_google_protobuf_protobuf_javalite", @@ -876,10 +870,13 @@ cel_android_library( "//common:runtime_exception", "//common/annotations", "//common/values:base_proto_cel_value_converter_android", + "//common/values:base_proto_message_value_provider_android", "//common/values:cel_value_android", "//common/values:cel_value_provider_android", + "//common/values:combined_cel_value_provider_android", "//common/values:values_android", "@maven//:com_google_errorprone_error_prone_annotations", + "@maven_android//:com_google_guava_guava", "@maven_android//:com_google_protobuf_protobuf_javalite", ], ) diff --git a/runtime/src/main/java/dev/cel/runtime/CelLiteRuntimeBuilder.java b/runtime/src/main/java/dev/cel/runtime/CelLiteRuntimeBuilder.java index 20a92fefe..155188ac3 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelLiteRuntimeBuilder.java +++ b/runtime/src/main/java/dev/cel/runtime/CelLiteRuntimeBuilder.java @@ -41,7 +41,12 @@ public interface CelLiteRuntimeBuilder { @CanIgnoreReturnValue CelLiteRuntimeBuilder addFunctionBindings(Iterable bindings); - /** Sets the {@link CelValueProvider} for resolving struct values during evaluation. */ + /** + * Sets the {@link CelValueProvider} for resolving struct values during evaluation. Multiple + * providers can be combined using {@code CombinedCelValueProvider}. Note that if you intend to + * support proto messages in addition to custom struct values, protobuf value provider must be + * configured first before the custom value provider. + */ @CanIgnoreReturnValue CelLiteRuntimeBuilder setValueProvider(CelValueProvider celValueProvider); diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeBuilder.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeBuilder.java index 28c0ae0e6..e1e3c1b51 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeBuilder.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeBuilder.java @@ -140,9 +140,10 @@ public interface CelRuntimeBuilder { CelRuntimeBuilder setTypeFactory(Function typeFactory); /** - * Sets the {@code celValueProvider} for resolving values during evaluation. The provided value - * provider will be used first before falling back to the built-in {@link - * dev.cel.common.values.ProtoMessageValueProvider} for resolving protobuf messages. + * Sets the {@link CelValueProvider} for resolving struct values during evaluation. Multiple + * providers can be combined using {@code CombinedCelValueProvider}. Note that if you intend to + * support proto messages in addition to custom struct values, protobuf value provider must be + * configured first before the custom value provider. * *

Note {@link CelOptions#enableCelValue()} must be enabled or this method will be a no-op. */ diff --git a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java index a8ecd4b27..bba92c07f 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/CelRuntimeLegacyImpl.java @@ -291,18 +291,13 @@ public CelRuntimeLegacyImpl build() { RuntimeTypeProvider runtimeTypeProvider; if (options.enableCelValue()) { - ProtoMessageValueProvider messageValueProvider = - ProtoMessageValueProvider.newInstance(dynamicProto); - if (celValueProvider != null) { - celValueProvider = - new CelValueProvider.CombinedCelValueProvider(celValueProvider, messageValueProvider); - } else { - celValueProvider = messageValueProvider; + CelValueProvider messageValueProvider = celValueProvider; + + if (messageValueProvider == null) { + messageValueProvider = ProtoMessageValueProvider.newInstance(dynamicProto); } - runtimeTypeProvider = - CelValueRuntimeTypeProvider.newInstance( - celValueProvider, messageValueProvider.getProtoCelValueConverter()); + runtimeTypeProvider = CelValueRuntimeTypeProvider.newInstance(messageValueProvider); } else { runtimeTypeProvider = new DescriptorMessageProvider(runtimeTypeFactory, options); } diff --git a/runtime/src/main/java/dev/cel/runtime/CelValueRuntimeTypeProvider.java b/runtime/src/main/java/dev/cel/runtime/CelValueRuntimeTypeProvider.java index 46f08b866..98e34458e 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelValueRuntimeTypeProvider.java +++ b/runtime/src/main/java/dev/cel/runtime/CelValueRuntimeTypeProvider.java @@ -14,14 +14,18 @@ package dev.cel.runtime; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.errorprone.annotations.Immutable; import com.google.protobuf.MessageLite; import dev.cel.common.CelErrorCode; import dev.cel.common.CelRuntimeException; import dev.cel.common.annotations.Internal; import dev.cel.common.values.BaseProtoCelValueConverter; +import dev.cel.common.values.BaseProtoMessageValueProvider; import dev.cel.common.values.CelValue; import dev.cel.common.values.CelValueProvider; +import dev.cel.common.values.CombinedCelValueProvider; import dev.cel.common.values.SelectableValue; import dev.cel.common.values.StringValue; import java.util.Map; @@ -34,10 +38,36 @@ final class CelValueRuntimeTypeProvider implements RuntimeTypeProvider { private final CelValueProvider valueProvider; private final BaseProtoCelValueConverter protoCelValueConverter; + private static final BaseProtoCelValueConverter DEFAULT_CEL_VALUE_CONVERTER = + new BaseProtoCelValueConverter() { + @Override + public CelValue fromProtoMessageToCelValue(String protoTypeName, MessageLite msg) { + throw new UnsupportedOperationException( + "A value provider must be provided in the runtime to handle protobuf messages"); + } + }; + + static CelValueRuntimeTypeProvider newInstance(CelValueProvider valueProvider) { + BaseProtoCelValueConverter converter = DEFAULT_CEL_VALUE_CONVERTER; + + // Find the underlying ProtoCelValueConverter. + // This is required because DefaultInterpreter works with a resolved protobuf messages directly + // in evaluation flow. + // A new runtime should not directly depend on protobuf, thus this will not be needed in the + // future. + if (valueProvider instanceof BaseProtoMessageValueProvider) { + converter = ((BaseProtoMessageValueProvider) valueProvider).protoCelValueConverter(); + } else if (valueProvider instanceof CombinedCelValueProvider) { + converter = + ((CombinedCelValueProvider) valueProvider) + .valueProviders().stream() + .filter(p -> p instanceof BaseProtoMessageValueProvider) + .map(p -> ((BaseProtoMessageValueProvider) p).protoCelValueConverter()) + .findFirst() + .orElse(DEFAULT_CEL_VALUE_CONVERTER); + } - static CelValueRuntimeTypeProvider newInstance( - CelValueProvider valueProvider, BaseProtoCelValueConverter protoCelValueConverter) { - return new CelValueRuntimeTypeProvider(valueProvider, protoCelValueConverter); + return new CelValueRuntimeTypeProvider(valueProvider, converter); } @Override @@ -48,7 +78,7 @@ public Object createMessage(String messageName, Map values) { .orElseThrow( () -> new NoSuchElementException( - "Could not generate a new value for message name: " + messageName))); + String.format("cannot resolve '%s' as a message", messageName)))); } @Override @@ -125,7 +155,7 @@ private static void throwInvalidFieldSelection(String fieldName) { private CelValueRuntimeTypeProvider( CelValueProvider valueProvider, BaseProtoCelValueConverter protoCelValueConverter) { - this.valueProvider = valueProvider; - this.protoCelValueConverter = protoCelValueConverter; + this.valueProvider = checkNotNull(valueProvider); + this.protoCelValueConverter = checkNotNull(protoCelValueConverter); } } diff --git a/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java b/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java index 055c97e44..71e457627 100644 --- a/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java +++ b/runtime/src/main/java/dev/cel/runtime/LiteRuntimeImpl.java @@ -23,11 +23,7 @@ import javax.annotation.concurrent.ThreadSafe; import dev.cel.common.CelAbstractSyntaxTree; import dev.cel.common.CelOptions; -import dev.cel.common.internal.DefaultLiteDescriptorPool; -import dev.cel.common.values.BaseProtoCelValueConverter; import dev.cel.common.values.CelValueProvider; -import dev.cel.common.values.ProtoLiteCelValueConverter; -import dev.cel.common.values.ProtoMessageLiteValueProvider; import java.util.Arrays; import java.util.HashMap; import java.util.Optional; @@ -148,19 +144,10 @@ public CelLiteRuntime build() { dispatcher.add( overloadId, func.getArgTypes(), (args) -> func.getDefinition().apply(args))); - BaseProtoCelValueConverter protoCelValueConverter; - if (celValueProvider instanceof ProtoMessageLiteValueProvider) { - protoCelValueConverter = - ((ProtoMessageLiteValueProvider) celValueProvider).getProtoLiteCelValueConverter(); - } else { - protoCelValueConverter = - ProtoLiteCelValueConverter.newInstance(DefaultLiteDescriptorPool.newInstance()); - } - Interpreter interpreter = new DefaultInterpreter( TypeResolver.create(), - CelValueRuntimeTypeProvider.newInstance(celValueProvider, protoCelValueConverter), + CelValueRuntimeTypeProvider.newInstance(celValueProvider), dispatcher, celOptions);