Skip to content

Migrate low-touch cel::LegacyStructValue member functions to use the modern equivalent implementations. #2204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
Aug 12, 2026
+47 −66
Merged

Migrate low-touch cel::LegacyStructValue member functions to use the modern equivalent implementations. #2204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 43 additions & 60 deletions common/legacy_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,73 +850,67 @@ absl::Status LegacyStructValue::ConvertToJson(
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
google::protobuf::MessageFactory* absl_nonnull message_factory,
google::protobuf::Message* absl_nonnull json) const {
ABSL_DCHECK(message_ptr_ != nullptr);
ABSL_DCHECK(descriptor_pool != nullptr);
ABSL_DCHECK(message_factory != nullptr);
ABSL_DCHECK(json != nullptr);
ABSL_DCHECK_EQ(json->GetDescriptor()->well_known_type(),
google::protobuf::Descriptor::WELLKNOWNTYPE_VALUE);

auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);

return internal::MessageToJson(
*google::protobuf::DownCastMessage<google::protobuf::Message>(message_wrapper.message_ptr()),
descriptor_pool, message_factory, json);
return internal::MessageToJson(*message_ptr_, descriptor_pool,
message_factory, json);
}

absl::Status LegacyStructValue::ConvertToJsonObject(
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
google::protobuf::MessageFactory* absl_nonnull message_factory,
google::protobuf::Message* absl_nonnull json) const {
ABSL_DCHECK(message_ptr_ != nullptr);
ABSL_DCHECK(descriptor_pool != nullptr);
ABSL_DCHECK(message_factory != nullptr);
ABSL_DCHECK(json != nullptr);
ABSL_DCHECK_EQ(json->GetDescriptor()->well_known_type(),
google::protobuf::Descriptor::WELLKNOWNTYPE_STRUCT);

auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);

return internal::MessageToJson(
*google::protobuf::DownCastMessage<google::protobuf::Message>(message_wrapper.message_ptr()),
descriptor_pool, message_factory, json);
return internal::MessageToJson(*message_ptr_, descriptor_pool,
message_factory, json);
}

absl::Status LegacyStructValue::Equal(
const Value& other,
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
google::protobuf::MessageFactory* absl_nonnull message_factory,
google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const {
if (auto legacy_struct_value = common_internal::AsLegacyStructValue(other);
legacy_struct_value.has_value()) {
auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
if (ABSL_PREDICT_FALSE(legacy_type_info_ ==
TrivialTypeInfo::GetInstance())) {
return absl::UnimplementedError(
absl::StrCat("legacy access APIs missing for ", GetTypeName()));
}
auto other_message_wrapper =
AsMessageWrapper(legacy_struct_value->message_ptr(),
legacy_struct_value->legacy_type_info());
*result = BoolValue{GetGenericProtoAccessApisInstance().IsEqualTo(
message_wrapper, other_message_wrapper)};
return absl::OkStatus();
ABSL_DCHECK(message_ptr_ != nullptr);
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
return absl::UnimplementedError(
absl::StrCat("legacy access APIs missing for ", GetTypeName()));
}
if (auto struct_value = other.AsStruct(); struct_value.has_value()) {
return common_internal::StructValueEqual(
common_internal::LegacyStructValue(message_ptr_, legacy_type_info_),
*struct_value, descriptor_pool, message_factory, arena, result);
auto modern_value = UnsafeParsedMessageValue(message_ptr_);

// Unwrap the rhs if it's a legacy struct so the normal implementation
// doesn't hit the fallback abstract struct comparison.
if (auto other_legacy = common_internal::AsLegacyStructValue(other);
other_legacy) {
if (other_legacy->legacy_type_info_ == TrivialTypeInfo::GetInstance()) {
return absl::UnimplementedError(absl::StrCat(
"legacy access APIs missing for ", other_legacy->GetTypeName()));
}
auto other_message = UnsafeParsedMessageValue(other_legacy->message_ptr_);
return modern_value.Equal(other_message, descriptor_pool, message_factory,
arena, result);
}
*result = FalseValue();
return absl::OkStatus();

return modern_value.Equal(other, descriptor_pool, message_factory, arena,
result);
}

bool LegacyStructValue::IsZeroValue() const {
auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
ABSL_DCHECK(message_ptr_ != nullptr);
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
return false;
}
return GetGenericProtoAccessApisInstance()
.ListFields(message_wrapper)
.empty();
return UnsafeParsedMessageValue(message_ptr_).IsZeroValue();
}

absl::Status LegacyStructValue::GetFieldByName(
Expand Down Expand Up @@ -948,44 +942,33 @@ absl::Status LegacyStructValue::GetFieldByNumber(

absl::StatusOr<bool> LegacyStructValue::HasFieldByName(
absl::string_view name) const {
auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
ABSL_DCHECK(message_ptr_ != nullptr);
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
return NoSuchFieldError(name).NativeValue();
return NoSuchFieldError(name).ToStatus();
}
return GetGenericProtoAccessApisInstance().HasField(name, message_wrapper);
return UnsafeParsedMessageValue(message_ptr_).HasFieldByName(name);
}

absl::StatusOr<bool> LegacyStructValue::HasFieldByNumber(int64_t number) const {
return absl::UnimplementedError(
"access to fields by numbers is not available for legacy structs");
ABSL_DCHECK(message_ptr_ != nullptr);
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
return NoSuchFieldError(absl::StrCat(number)).ToStatus();
}
return UnsafeParsedMessageValue(message_ptr_).HasFieldByNumber(number);
}

absl::Status LegacyStructValue::ForEachField(
ForEachFieldCallback callback,
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
google::protobuf::MessageFactory* absl_nonnull message_factory,
google::protobuf::Arena* absl_nonnull arena) const {
auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
ABSL_DCHECK(message_ptr_ != nullptr);
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
return absl::UnimplementedError(
absl::StrCat("legacy access APIs missing for ", GetTypeName()));
}
const auto& access_apis = GetGenericProtoAccessApisInstance();
auto field_names = access_apis.ListFields(message_wrapper);
Value value;
for (const auto& field_name : field_names) {
CEL_ASSIGN_OR_RETURN(
auto cel_value,
access_apis.GetField(field_name, message_wrapper,
ProtoWrapperTypeOptions::kUnsetNull,
MemoryManagerRef::Pooling(arena)));
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, value));
CEL_ASSIGN_OR_RETURN(auto ok, callback(field_name, value));
if (!ok) {
break;
}
}
return absl::OkStatus();
return UnsafeParsedMessageValue(message_ptr_)
.ForEachField(callback, descriptor_pool, message_factory, arena);
}

absl::Status LegacyStructValue::Qualify(
Expand Down Expand Up @@ -1291,12 +1274,12 @@ const google::protobuf::Message* absl_nullable GetLegacyMessage(const Value& val

auto legacy = common_internal::GetLegacyStructValue(value);
const auto* legacy_type_info = legacy.legacy_type_info();
if (legacy_type_info == nullptr) {
return nullptr;
}
if (legacy_type_info != &GetGenericProtoTypeInfoInstance()) {
if (legacy_type_info == nullptr ||
legacy_type_info == TrivialTypeInfo::GetInstance()) {
return nullptr;
}
// This should not be possible using the normal public APIs, but possible
// if someone used the MessageWrapper class directly.
if (IsWellKnownMessageType(legacy.message_ptr()->GetDescriptor())) {
return nullptr;
}
Expand Down
1 change: 1 addition & 0 deletions eval/public/structs/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ cc_test(
":legacy_type_adapter",
":legacy_type_info_apis",
":proto_message_type_adapter",
":trivial_legacy_type_info",
"//base:attributes",
"//common:value",
"//common:value_testing",
Expand Down
9 changes: 3 additions & 6 deletions eval/public/structs/proto_message_type_adapter_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#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/trivial_legacy_type_info.h"
#include "eval/public/testing/matchers.h"
#include "eval/testutil/test_message.pb.h"
#include "extensions/protobuf/memory_manager.h"
Expand Down Expand Up @@ -1147,17 +1148,13 @@ TEST(ProtoMesssageTypeAdapter, QualifyMapIndexLeafWrongType) {
HasSubstr("Invalid map key type"))))));
}

TEST(ProtoMesssageTypeAdapter, InteropUnwrappingNotGeneric) {
TEST(ProtoMesssageTypeAdapter, InteropUnwrappingNotTestInstance) {
google::protobuf::Arena arena;
ProtoMessageTypeAdapter adapter(
google::protobuf::DescriptorPool::generated_pool()->FindMessageTypeByName(
"google.api.expr.runtime.TestMessage"),
google::protobuf::MessageFactory::generated_factory());

TestMessage message;
message.set_string_value("hello");
auto legacy_value = CelValue::CreateMessageWrapper(
CelValue::MessageWrapper(&message, &adapter));
CelValue::MessageWrapper(&message, TrivialTypeInfo::GetInstance()));
cel::Value modern_value;
ASSERT_THAT(cel::ModernValue(&arena, legacy_value, modern_value), IsOk());
auto unwrapped = cel::interop_internal::GetLegacyMessage(modern_value);
Expand Down
Loading