Decouple LegacyTypeProvider and cel::TypeProvider

eval/public/structs/BUILD

@@ -229,26 +229,12 @@ cc_test(
 
 cc_library(
     name = "legacy_type_provider",
-    srcs = ["legacy_type_provider.cc"],
     hdrs = ["legacy_type_provider.h"],
     deps = [
         ":legacy_type_adapter",
         ":legacy_type_info_apis",
-        "//common:legacy_value",
-        "//common:memory",
-        "//common:type",
-        "//common:value",
-        "//eval/public:message_wrapper",
-        "//extensions/protobuf:memory_manager",
-        "//internal:status_macros",
         "@com_google_absl//absl/base:core_headers",
-        "@com_google_absl//absl/base:nullability",
-        "@com_google_absl//absl/status",
-        "@com_google_absl//absl/status:statusor",
-        "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:string_view",
-        "@com_google_absl//absl/types:optional",
-        "@com_google_protobuf//:protobuf",
     ],
 )
 
@@ -359,15 +345,11 @@ cc_test(
     deps = [
         ":legacy_type_info_apis",
         ":protobuf_descriptor_type_provider",
-        "//common:type",
         "//eval/public:cel_value",
         "//eval/public/testing:matchers",
         "//extensions/protobuf:memory_manager",
         "//internal:testing",
-        "//internal:testing_descriptor_pool",
-        "//internal:testing_message_factory",
         "@com_google_absl//absl/status:status_matchers",
-        "@com_google_cel_spec//proto/cel/expr/conformance/proto3:test_all_types_cc_proto",
         "@com_google_protobuf//:protobuf",
         "@com_google_protobuf//:wrappers_cc_proto",
     ],
@@ -413,12 +395,10 @@ cc_test(
         ":legacy_type_adapter",
         ":legacy_type_info_apis",
         ":legacy_type_provider",
-        ":proto_message_type_adapter",
         ":trivial_legacy_type_info",
-        "//common:type",
-        "//eval/testutil:test_message_cc_proto",
         "//internal:testing",
         "@com_google_absl//absl/strings:string_view",
+        "@com_google_absl//absl/types:optional",
     ],
 )
 

eval/public/structs/legacy_type_provider.h

@@ -18,24 +18,20 @@
 #include <optional>
 
 #include "absl/base/attributes.h"
-#include "absl/base/nullability.h"
-#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
-#include "common/type.h"
-#include "common/type_reflector.h"
-#include "common/value.h"
 #include "eval/public/structs/legacy_type_adapter.h"
 #include "eval/public/structs/legacy_type_info_apis.h"
-#include "google/protobuf/arena.h"
-#include "google/protobuf/message.h"
 
 namespace google::api::expr::runtime {
 
-// An internal extension of cel::TypeProvider that also deals with legacy types.
+// Provides legacy type adapters for google::api::expr::runtime::CelValue
+// wrapping messages.
 //
-// Note: This API is not finalized. Consult the CEL team before introducing new
-// implementations.
-class LegacyTypeProvider : public cel::TypeReflector {
+// Note: internal use of this type is limited to interop with the Legacy
+// (CelValue) runtime with an internal implementation.
+//
+// New implementations cannot be registered with the evaluator.
+class LegacyTypeProvider {
  public:
   virtual ~LegacyTypeProvider() = default;
 
@@ -60,19 +56,6 @@ class LegacyTypeProvider : public cel::TypeReflector {
       ABSL_ATTRIBUTE_UNUSED absl::string_view name) const {
     return std::nullopt;
   }
-
-  absl::StatusOr<absl_nullable cel::ValueBuilderPtr> NewValueBuilder(
-      absl::string_view name,
-      google::protobuf::MessageFactory* absl_nonnull message_factory,
-      google::protobuf::Arena* absl_nonnull arena) const final;
-
- protected:
-  absl::StatusOr<std::optional<cel::Type>> FindTypeImpl(
-      absl::string_view name) const final;
-
-  absl::StatusOr<std::optional<cel::StructTypeField>>
-  FindStructTypeFieldByNameImpl(absl::string_view type,
-                                absl::string_view name) const final;
 };
 
 }  // namespace google::api::expr::runtime

eval/public/structs/legacy_type_provider_test.cc

@@ -17,12 +17,10 @@
 #include <optional>
 
 #include "absl/strings/string_view.h"
-#include "common/type.h"
+#include "absl/types/optional.h"
 #include "eval/public/structs/legacy_type_adapter.h"
 #include "eval/public/structs/legacy_type_info_apis.h"
-#include "eval/public/structs/proto_message_type_adapter.h"
 #include "eval/public/structs/trivial_legacy_type_info.h"
-#include "eval/testutil/test_message.pb.h"
 #include "internal/testing.h"
 
 namespace google::api::expr::runtime {
@@ -73,23 +71,5 @@ TEST(LegacyTypeProviderTest, NonEmptyTypeProviderProvidesSomeTypes) {
   EXPECT_EQ(provider.ProvideLegacyTypeInfo("other"), std::nullopt);
 }
 
-TEST(LegacyTypeProviderTest, FindStructTypeFieldByName) {
-  ProtoMessageTypeAdapter adapter(TestMessage::descriptor(), nullptr);
-  LegacyTypeProviderTestImpl provider(&adapter);
-
-  ASSERT_OK_AND_ASSIGN(
-      absl::optional<cel::StructTypeField> field,
-      provider.FindStructTypeFieldByName("test", "int32_value"));
-  ASSERT_TRUE(field.has_value());
-  EXPECT_EQ(field->name(), "int32_value");
-  EXPECT_EQ(field->number(), 1);
-  EXPECT_EQ(field->GetType(), cel::IntType());
-
-  ASSERT_OK_AND_ASSIGN(
-      absl::optional<cel::StructTypeField> not_found_field,
-      provider.FindStructTypeFieldByName("test", "unknown_field"));
-  EXPECT_FALSE(not_found_field.has_value());
-}
-
 }  // namespace
 }  // namespace google::api::expr::runtime

eval/public/structs/proto_message_type_adapter.h

@@ -28,6 +28,7 @@
 #include "eval/public/structs/legacy_type_info_apis.h"
 #include "google/protobuf/arena.h"
 #include "google/protobuf/descriptor.h"
+#include "google/protobuf/message.h"
 
 namespace google::api::expr::runtime {
 
@@ -40,7 +41,7 @@ class ProtoMessageTypeAdapter : public LegacyTypeInfoApis,
                                 public LegacyTypeAccessApis,
                                 public LegacyTypeMutationApis {
  public:
-  ProtoMessageTypeAdapter(const google::protobuf::Descriptor* descriptor,
+  ProtoMessageTypeAdapter(const google::protobuf::Descriptor* absl_nonnull descriptor,
                           google::protobuf::MessageFactory* message_factory)
       : message_factory_(message_factory), descriptor_(descriptor) {}
 
@@ -107,6 +108,10 @@ class ProtoMessageTypeAdapter : public LegacyTypeInfoApis,
   std::vector<absl::string_view> ListFields(
       const CelValue::MessageWrapper& instance) const override;
 
+  const google::protobuf::Descriptor* absl_nonnull descriptor() const {
+    return descriptor_;
+  }
+
  private:
   // Helper for standardizing error messages for SetField operation.
   absl::Status ValidateSetFieldOp(bool assertion, absl::string_view field,
@@ -117,7 +122,7 @@ class ProtoMessageTypeAdapter : public LegacyTypeInfoApis,
                         google::protobuf::Message* message) const;
 
   google::protobuf::MessageFactory* message_factory_;
-  const google::protobuf::Descriptor* descriptor_;
+  const google::protobuf::Descriptor* absl_nonnull descriptor_;
 };
 
 // Creates a CelValue from the given field on the proto message. This is the

eval/public/structs/protobuf_descriptor_type_provider_test.cc

@@ -18,15 +18,11 @@
 
 #include "google/protobuf/wrappers.pb.h"
 #include "absl/status/status_matchers.h"
-#include "common/type.h"
 #include "eval/public/cel_value.h"
 #include "eval/public/structs/legacy_type_info_apis.h"
 #include "eval/public/testing/matchers.h"
 #include "extensions/protobuf/memory_manager.h"
 #include "internal/testing.h"
-#include "internal/testing_descriptor_pool.h"
-#include "internal/testing_message_factory.h"
-#include "cel/expr/conformance/proto3/test_all_types.pb.h"
 #include "google/protobuf/arena.h"
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/message.h"
@@ -35,7 +31,6 @@ namespace google::api::expr::runtime {
 namespace {
 
 using ::absl_testing::IsOk;
-using ::cel::expr::conformance::proto3::TestAllTypes;
 using ::cel::extensions::ProtoMemoryManager;
 
 TEST(ProtobufDescriptorProvider, Basic) {
@@ -101,60 +96,5 @@ TEST(ProtobufDescriptorProvider, NotFound) {
   ASSERT_FALSE(type_info.has_value());
 }
 
-TEST(ProtobufDescriptorProvider, FindType) {
-  ProtobufDescriptorProvider provider(
-      cel::internal::GetTestingDescriptorPool(),
-      cel::internal::GetTestingMessageFactory());
-  ASSERT_OK_AND_ASSIGN(std::optional<cel::Type> wrapper_type,
-                       provider.FindType("google.protobuf.Int64Value"));
-  ASSERT_TRUE(wrapper_type.has_value());
-  EXPECT_TRUE(wrapper_type->Is<cel::IntWrapperType>());
-  EXPECT_EQ(wrapper_type->name(), "google.protobuf.Int64Value");
-
-  ASSERT_OK_AND_ASSIGN(
-      std::optional<cel::Type> msg_type,
-      provider.FindType("cel.expr.conformance.proto3.TestAllTypes"));
-  ASSERT_TRUE(msg_type.has_value());
-  EXPECT_TRUE(msg_type->Is<cel::MessageType>());
-  EXPECT_EQ(msg_type->name(), "cel.expr.conformance.proto3.TestAllTypes");
-}
-
-TEST(ProtobufDescriptorProvider, FindStructTypeFieldByName) {
-  ProtobufDescriptorProvider provider(
-      google::protobuf::DescriptorPool::generated_pool(),
-      google::protobuf::MessageFactory::generated_factory());
-  ASSERT_OK_AND_ASSIGN(std::optional<cel::StructTypeField> field,
-                       provider.FindStructTypeFieldByName(
-                           "google.protobuf.Int64Value", "value"));
-  ASSERT_TRUE(field.has_value());
-  EXPECT_EQ(field->name(), "value");
-  EXPECT_EQ(field->number(), 1);
-  EXPECT_EQ(field->GetType(), cel::IntType());
-}
-
-TEST(ProtobufDescriptorProvider, FindTypeNotFound) {
-  ProtobufDescriptorProvider provider(
-      google::protobuf::DescriptorPool::generated_pool(),
-      google::protobuf::MessageFactory::generated_factory());
-  ASSERT_OK_AND_ASSIGN(std::optional<cel::Type> type,
-                       provider.FindType("UnknownType"));
-  EXPECT_FALSE(type.has_value());
-}
-
-TEST(ProtobufDescriptorProvider, FindStructTypeFieldByNameNotFound) {
-  ProtobufDescriptorProvider provider(
-      google::protobuf::DescriptorPool::generated_pool(),
-      google::protobuf::MessageFactory::generated_factory());
-  ASSERT_OK_AND_ASSIGN(std::optional<cel::StructTypeField> field,
-                       provider.FindStructTypeFieldByName(
-                           "google.protobuf.Int64Value", "unknown_field"));
-  EXPECT_FALSE(field.has_value());
-
-  ASSERT_OK_AND_ASSIGN(
-      std::optional<cel::StructTypeField> field2,
-      provider.FindStructTypeFieldByName("UnknownType", "value"));
-  EXPECT_FALSE(field2.has_value());
-}
-
 }  // namespace
 }  // namespace google::api::expr::runtime

runtime/internal/BUILD

@@ -181,10 +181,43 @@ cc_library(
 
 cc_library(
     name = "legacy_runtime_type_provider",
+    srcs = ["legacy_runtime_type_provider.cc"],
     hdrs = ["legacy_runtime_type_provider.h"],
     deps = [
+        "//common:legacy_value",
+        "//common:memory",
+        "//common:type",
+        "//common:value",
+        "//eval/public:message_wrapper",
+        "//eval/public/structs:legacy_type_adapter",
+        "//eval/public/structs:legacy_type_info_apis",
+        "//eval/public/structs:proto_message_type_adapter",
         "//eval/public/structs:protobuf_descriptor_type_provider",
+        "//extensions/protobuf:memory_manager",
+        "//internal:status_macros",
         "@com_google_absl//absl/base:nullability",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/strings",
+        "@com_google_absl//absl/strings:string_view",
+        "@com_google_absl//absl/types:optional",
+        "@com_google_protobuf//:protobuf",
+    ],
+)
+
+cc_test(
+    name = "legacy_runtime_type_provider_test",
+    srcs = ["legacy_runtime_type_provider_test.cc"],
+    deps = [
+        ":legacy_runtime_type_provider",
+        "//common:type",
+        "//common:value",
+        "//internal:testing",
+        "//internal:testing_descriptor_pool",
+        "//internal:testing_message_factory",
+        "@com_google_absl//absl/status:status_matchers",
+        "@com_google_absl//absl/types:optional",
+        "@com_google_cel_spec//proto/cel/expr/conformance/proto3:test_all_types_cc_proto",
         "@com_google_protobuf//:protobuf",
     ],
 )