Drop absl::Status return from NewStructValueBuilder and `NewValue…

…Builder` PiperOrigin-RevId: 693806382
google · Nov 6, 2024 · 0834a15 · 0834a15
1 parent ada9b5f
commit 0834a15
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 76 deletions.
diff --git a/common/type_reflector_test.cc b/common/type_reflector_test.cc
@@ -222,11 +222,9 @@ TEST_P(TypeReflectorTest, JsonKeyCoverage) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_BoolValue) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.BoolValue"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.BoolValue");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", BoolValue(true)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", BoolValue(true)),
@@ -244,11 +242,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_BoolValue) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_Int32Value) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.Int32Value"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.Int32Value");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", IntValue(1)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", IntValue(1)),
@@ -272,11 +268,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_Int32Value) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_Int64Value) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.Int64Value"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.Int64Value");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", IntValue(1)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", IntValue(1)),
@@ -294,11 +288,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_Int64Value) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_UInt32Value) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.UInt32Value"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.UInt32Value");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", UintValue(1)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", UintValue(1)),
@@ -322,11 +314,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_UInt32Value) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_UInt64Value) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.UInt64Value"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.UInt64Value");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", UintValue(1)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", UintValue(1)),
@@ -344,11 +334,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_UInt64Value) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_FloatValue) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.FloatValue"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.FloatValue");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", DoubleValue(1)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", DoubleValue(1)),
@@ -366,11 +354,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_FloatValue) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_DoubleValue) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.DoubleValue"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.DoubleValue");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", DoubleValue(1)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", DoubleValue(1)),
@@ -388,11 +374,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_DoubleValue) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_StringValue) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.StringValue"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.StringValue");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", StringValue("foo")), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", StringValue("foo")),
@@ -410,11 +394,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_StringValue) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_BytesValue) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.BytesValue"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.BytesValue");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("value", BytesValue("foo")), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", BytesValue("foo")),
@@ -432,11 +414,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_BytesValue) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_Duration) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.Duration"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.Duration");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("seconds", IntValue(1)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", IntValue(1)),
@@ -467,11 +447,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_Duration) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_Timestamp) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.Timestamp"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.Timestamp");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName("seconds", IntValue(1)), IsOk());
   EXPECT_THAT(builder->SetFieldByName("does_not_exist", IntValue(1)),
@@ -502,11 +480,9 @@ TEST_P(TypeReflectorTest, NewValueBuilder_Timestamp) {
 }
 
 TEST_P(TypeReflectorTest, NewValueBuilder_Any) {
-  ASSERT_OK_AND_ASSIGN(
-      auto builder,
-      common_internal::NewValueBuilder(
-          memory_manager(), internal::GetTestingDescriptorPool(),
-          internal::GetTestingMessageFactory(), "google.protobuf.Any"));
+  auto builder = common_internal::NewValueBuilder(
+      memory_manager(), internal::GetTestingDescriptorPool(),
+      internal::GetTestingMessageFactory(), "google.protobuf.Any");
   ASSERT_THAT(builder, NotNull());
   EXPECT_THAT(builder->SetFieldByName(
                   "type_url",

diff --git a/common/value.cc b/common/value.cc
@@ -2552,7 +2552,7 @@ absl::Nonnull<MapValueBuilderPtr> NewMapValueBuilder(
   return common_internal::NewMapValueBuilder(arena);
 }
 
-absl::StatusOr<absl::Nullable<StructValueBuilderPtr>> NewStructValueBuilder(
+absl::Nullable<StructValueBuilderPtr> NewStructValueBuilder(
     absl::Nonnull<google::protobuf::Arena*> arena,
     absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
     absl::Nonnull<google::protobuf::MessageFactory*> message_factory,

diff --git a/common/value.h b/common/value.h
@@ -2655,7 +2655,7 @@ absl::Nonnull<MapValueBuilderPtr> NewMapValueBuilder(
 // message type with the name `name` in `descriptor_pool`. Returns an error if
 // `message_factory` is unable to provide a prototype for the descriptor
 // returned from `descriptor_pool`.
-absl::StatusOr<absl::Nullable<StructValueBuilderPtr>> NewStructValueBuilder(
+absl::Nullable<StructValueBuilderPtr> NewStructValueBuilder(
     absl::Nonnull<google::protobuf::Arena*> arena,
     absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
     absl::Nonnull<google::protobuf::MessageFactory*> message_factory,

diff --git a/common/values/struct_value_builder.cc b/common/values/struct_value_builder.cc
@@ -1647,7 +1647,7 @@ class StructValueBuilderImpl final : public StructValueBuilder {
 
 }  // namespace
 
-absl::StatusOr<absl::Nullable<cel::ValueBuilderPtr>> NewValueBuilder(
+absl::Nullable<cel::ValueBuilderPtr> NewValueBuilder(
     Allocator<> allocator,
     absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
     absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
@@ -1659,17 +1659,20 @@ absl::StatusOr<absl::Nullable<cel::ValueBuilderPtr>> NewValueBuilder(
   }
   absl::Nullable<const google::protobuf::Message*> prototype =
       message_factory->GetPrototype(descriptor);
-  if (prototype == nullptr) {
-    return absl::NotFoundError(absl::StrCat(
-        "unable to get prototype for descriptor: ", descriptor->full_name()));
+  ABSL_DCHECK(prototype != nullptr)
+      << "failed to get message prototype from factory, did you pass a dynamic "
+         "descriptor to the generated message factory? we consider this to be "
+         "a logic error and not a runtime error: "
+      << descriptor->full_name();
+  if (ABSL_PREDICT_FALSE(prototype == nullptr)) {
+    return nullptr;
   }
   return std::make_unique<ValueBuilderImpl>(allocator.arena(), descriptor_pool,
                                             message_factory,
                                             prototype->New(allocator.arena()));
 }
 
-absl::StatusOr<absl::Nullable<cel::StructValueBuilderPtr>>
-NewStructValueBuilder(
+absl::Nullable<cel::StructValueBuilderPtr> NewStructValueBuilder(
     Allocator<> allocator,
     absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
     absl::Nonnull<google::protobuf::MessageFactory*> message_factory,
@@ -1681,9 +1684,13 @@ NewStructValueBuilder(
   }
   absl::Nullable<const google::protobuf::Message*> prototype =
       message_factory->GetPrototype(descriptor);
-  if (prototype == nullptr) {
-    return absl::NotFoundError(absl::StrCat(
-        "unable to get prototype for descriptor: ", descriptor->full_name()));
+  ABSL_DCHECK(prototype != nullptr)
+      << "failed to get message prototype from factory, did you pass a dynamic "
+         "descriptor to the generated message factory? we consider this to be "
+         "a logic error and not a runtime error: "
+      << descriptor->full_name();
+  if (ABSL_PREDICT_FALSE(prototype == nullptr)) {
+    return nullptr;
   }
   return std::make_unique<StructValueBuilderImpl>(
       allocator.arena(), descriptor_pool, message_factory,

diff --git a/common/values/struct_value_builder.h b/common/values/struct_value_builder.h
@@ -16,7 +16,6 @@
 #define THIRD_PARTY_CEL_CPP_COMMON_VALUES_STRUCT_VALUE_BUILDER_H_
 
 #include "absl/base/nullability.h"
-#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "common/allocator.h"
 #include "common/value.h"
@@ -25,8 +24,7 @@
 
 namespace cel::common_internal {
 
-absl::StatusOr<absl::Nullable<cel::StructValueBuilderPtr>>
-NewStructValueBuilder(
+absl::Nullable<cel::StructValueBuilderPtr> NewStructValueBuilder(
     Allocator<> allocator,
     absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
     absl::Nonnull<google::protobuf::MessageFactory*> message_factory,

diff --git a/common/values/value_builder.h b/common/values/value_builder.h
@@ -16,7 +16,6 @@
 #define THIRD_PARTY_CEL_CPP_COMMON_VALUES_VALUE_BUILDER_H_
 
 #include "absl/base/nullability.h"
-#include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "common/allocator.h"
 #include "common/value.h"
@@ -26,7 +25,7 @@
 namespace cel::common_internal {
 
 // Like NewStructValueBuilder, but deals with well known types.
-absl::StatusOr<absl::Nullable<cel::ValueBuilderPtr>> NewValueBuilder(
+absl::Nullable<cel::ValueBuilderPtr> NewValueBuilder(
     Allocator<> allocator,
     absl::Nonnull<const google::protobuf::DescriptorPool*> descriptor_pool,
     absl::Nonnull<google::protobuf::MessageFactory*> message_factory,