Update LegacyRuntimeTypeProvider to use modern ValueBuilder.

PiperOrigin-RevId: 962225090

Author: jnthntatum

runtime/internal/BUILD

@@ -185,20 +185,14 @@ cc_library(
     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",

runtime/internal/legacy_runtime_type_provider.cc

@@ -20,21 +20,15 @@
 #include <utility>
 
 #include "absl/base/nullability.h"
-#include "absl/status/status.h"
 #include "absl/status/statusor.h"
-#include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
-#include "absl/types/optional.h"
 #include "common/legacy_value.h"
-#include "common/memory.h"
 #include "common/type.h"
 #include "common/type_introspector.h"
 #include "common/value.h"
+#include "common/values/value_builder.h"
 #include "eval/public/message_wrapper.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 "extensions/protobuf/memory_manager.h"
 #include "internal/status_macros.h"
 #include "google/protobuf/arena.h"
 #include "google/protobuf/descriptor.h"
@@ -44,61 +38,43 @@ namespace cel::runtime_internal {
 
 namespace {
 
-using google::api::expr::runtime::LegacyTypeAdapter;
 using google::api::expr::runtime::LegacyTypeInfoApis;
 using google::api::expr::runtime::MessageWrapper;
 
 class LegacyValueBuilder final : public cel::ValueBuilder {
  public:
-  LegacyValueBuilder(cel::MemoryManagerRef memory_manager,
-                     LegacyTypeAdapter adapter, MessageWrapper::Builder builder)
-      : memory_manager_(memory_manager),
-        adapter_(adapter),
-        builder_(std::move(builder)) {}
+  LegacyValueBuilder(google::protobuf::Arena* absl_nonnull arena,
+                     cel::ValueBuilderPtr builder)
+      : arena_(arena), builder_(std::move(builder)) {}
 
-  absl::StatusOr<absl::optional<cel::ErrorValue>> SetFieldByName(
+  absl::StatusOr<std::optional<cel::ErrorValue>> SetFieldByName(
       absl::string_view name, cel::Value value) override {
-    CEL_ASSIGN_OR_RETURN(
-        auto legacy_value,
-        LegacyValue(cel::extensions::ProtoMemoryManagerArena(memory_manager_),
-                    value),
-        _.With(cel::ErrorValueReturn()));
-    CEL_RETURN_IF_ERROR(adapter_.mutation_apis()->SetField(
-                            name, legacy_value, memory_manager_, builder_))
-        .With(cel::ErrorValueReturn());
-    return std::nullopt;
+    return builder_->SetFieldByName(name, std::move(value));
   }
 
-  absl::StatusOr<absl::optional<cel::ErrorValue>> SetFieldByNumber(
+  absl::StatusOr<std::optional<cel::ErrorValue>> SetFieldByNumber(
       int64_t number, cel::Value value) override {
-    CEL_ASSIGN_OR_RETURN(
-        auto legacy_value,
-        LegacyValue(cel::extensions::ProtoMemoryManagerArena(memory_manager_),
-                    value),
-        _.With(cel::ErrorValueReturn()));
-    CEL_RETURN_IF_ERROR(adapter_.mutation_apis()->SetFieldByNumber(
-                            number, legacy_value, memory_manager_, builder_))
-        .With(cel::ErrorValueReturn());
-    return std::nullopt;
+    return builder_->SetFieldByNumber(number, std::move(value));
   }
 
   absl::StatusOr<cel::Value> Build() && override {
-    CEL_ASSIGN_OR_RETURN(auto value,
-                         adapter_.mutation_apis()->AdaptFromWellKnownType(
-                             memory_manager_, std::move(builder_)),
+    CEL_ASSIGN_OR_RETURN(auto value, std::move(*builder_).Build(),
                          _.With(cel::ErrorValueReturn()));
-    CEL_ASSIGN_OR_RETURN(
-        auto result,
-        cel::ModernValue(
-            cel::extensions::ProtoMemoryManagerArena(memory_manager_), value),
-        _.With(cel::ErrorValueReturn()));
-    return result;
+    if (value.Is<MessageValue>()) {
+      // Make the value behave like a legacy message. Minimizes further
+      // legacy/modern conversions (e.g. on return and when accessing fields).
+      CEL_ASSIGN_OR_RETURN(auto legacy_value, LegacyValue(arena_, value),
+                           _.With(cel::ErrorValueReturn()));
+      CEL_ASSIGN_OR_RETURN(auto result, ModernValue(arena_, legacy_value),
+                           _.With(cel::ErrorValueReturn()));
+      return result;
+    }
+    return value;
   }
 
  private:
-  cel::MemoryManagerRef memory_manager_;
-  LegacyTypeAdapter adapter_;
-  MessageWrapper::Builder builder_;
+  google::protobuf::Arena* const arena_;
+  cel::ValueBuilderPtr builder_;
 };
 
 }  // namespace
@@ -108,26 +84,12 @@ LegacyRuntimeTypeProvider::NewValueBuilder(
     absl::string_view name,
     google::protobuf::MessageFactory* absl_nonnull message_factory,
     google::protobuf::Arena* absl_nonnull arena) const {
-  auto type_adapter = ProvideLegacyType(name);
-
-  if (!type_adapter.has_value()) {
+  auto builder = common_internal::NewValueBuilder(arena, descriptor_pool_,
+                                                  message_factory, name);
+  if (builder == nullptr) {
     return nullptr;
   }
-
-  // We know the implementation should not do this, but can't prove it to type
-  // system.
-  // Defensive checks but impractical to exercise.
-  const auto* mutation_apis = type_adapter->mutation_apis();
-  if (mutation_apis == nullptr) {
-    return absl::FailedPreconditionError(
-        absl::StrCat("LegacyTypeMutationApis missing for type: ", name));
-  }
-
-  CEL_ASSIGN_OR_RETURN(
-      auto builder,
-      mutation_apis->NewInstance(cel::MemoryManagerRef::Pooling(arena)));
-  return std::make_unique<LegacyValueBuilder>(
-      cel::MemoryManagerRef::Pooling(arena), *type_adapter, std::move(builder));
+  return std::make_unique<LegacyValueBuilder>(arena, std::move(builder));
 }
 
 absl::StatusOr<std::optional<Type>> LegacyRuntimeTypeProvider::FindTypeImpl(
@@ -175,12 +137,7 @@ LegacyRuntimeTypeProvider::FindStructTypeFieldByNameImpl(
         field_desc->name, field_desc->number, cel::DynType{});
   }
 
-  const auto* mutation_apis = (*type_info)->GetMutationApis(MessageWrapper());
-  if (mutation_apis == nullptr || !mutation_apis->DefinesField(name)) {
-    return std::nullopt;
-  }
-
-  return cel::common_internal::BasicStructTypeField(name, 0, cel::DynType{});
+  return std::nullopt;
 }
 
 }  // namespace cel::runtime_internal

runtime/internal/legacy_runtime_type_provider.h

@@ -37,8 +37,9 @@ class LegacyRuntimeTypeProvider final
   LegacyRuntimeTypeProvider(
       const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
       google::protobuf::MessageFactory* absl_nullable message_factory)
-      : google::api::expr::runtime::ProtobufDescriptorProvider(
-            descriptor_pool, message_factory) {}
+      : google::api::expr::runtime::ProtobufDescriptorProvider(descriptor_pool,
+                                                               message_factory),
+        descriptor_pool_(descriptor_pool) {}
 
   absl::StatusOr<absl_nullable ValueBuilderPtr> NewValueBuilder(
       absl::string_view name,
@@ -51,6 +52,9 @@ class LegacyRuntimeTypeProvider final
 
   absl::StatusOr<std::optional<StructTypeField>> FindStructTypeFieldByNameImpl(
       absl::string_view type, absl::string_view name) const override;
+
+ private:
+  const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool_;
 };
 
 }  // namespace cel::runtime_internal

runtime/internal/legacy_runtime_type_provider_test.cc

@@ -86,7 +86,7 @@ TEST(LegacyRuntimeTypeProviderTest, FindStructTypeFieldByNameNotFound) {
   EXPECT_FALSE(field2.has_value());
 }
 
-TEST(LegacyRuntimeTypeProviderTest, NewValueBuilder) {
+TEST(LegacyRuntimeTypeProviderTest, NewValueBuilderMessage) {
   LegacyRuntimeTypeProvider provider(cel::internal::GetTestingDescriptorPool(),
                                      cel::internal::GetTestingMessageFactory());
   google::protobuf::Arena arena;
@@ -100,10 +100,33 @@ TEST(LegacyRuntimeTypeProviderTest, NewValueBuilder) {
                        builder->SetFieldByName("single_int64", IntValue(42)));
   EXPECT_FALSE(field_result.has_value());
 
+  ASSERT_OK_AND_ASSIGN(auto field_result2,
+                       builder->SetFieldByNumber(1, IntValue(100)));
+  EXPECT_FALSE(field_result2.has_value());
+
   ASSERT_OK_AND_ASSIGN(auto value, std::move(*builder).Build());
   EXPECT_TRUE(value.Is<StructValue>());
 }
 
+TEST(LegacyRuntimeTypeProviderTest, NewValueBuilderWellKnownType) {
+  LegacyRuntimeTypeProvider provider(cel::internal::GetTestingDescriptorPool(),
+                                     cel::internal::GetTestingMessageFactory());
+  google::protobuf::Arena arena;
+  ASSERT_OK_AND_ASSIGN(auto builder,
+                       provider.NewValueBuilder(
+                           "google.protobuf.Int64Value",
+                           cel::internal::GetTestingMessageFactory(), &arena));
+  ASSERT_NE(builder, nullptr);
+
+  ASSERT_OK_AND_ASSIGN(auto field_result,
+                       builder->SetFieldByName("value", IntValue(42)));
+  EXPECT_FALSE(field_result.has_value());
+
+  ASSERT_OK_AND_ASSIGN(auto value, std::move(*builder).Build());
+  ASSERT_TRUE(value.Is<IntValue>());
+  EXPECT_EQ(value.As<IntValue>()->NativeValue(), 42);
+}
+
 TEST(LegacyRuntimeTypeProviderTest, NewValueBuilderNotFound) {
   LegacyRuntimeTypeProvider provider(
       google::protobuf::DescriptorPool::generated_pool(),