Skip to content

Migrate repeated/map field accesses to use modern implementation. #2217

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

Open
copybara-service wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Migrate repeated/map field accesses to use modern implementation. #2217

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
5 changes: 2 additions & 3 deletions common/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,10 +793,7 @@ cc_library(
"//eval/internal:cel_value_equal",
"//eval/public:cel_value",
"//eval/public:message_wrapper",
"//eval/public/containers:field_backed_list_impl",
"//eval/public/containers:field_backed_map_impl",
"//eval/public/structs:cel_proto_wrap_util",
"//eval/public/structs:legacy_type_adapter",
"//eval/public/structs:legacy_type_info_apis",
"//eval/public/structs:proto_message_type_adapter",
"//eval/public/structs:trivial_legacy_type_info_internal",
Expand Down Expand Up @@ -865,6 +862,8 @@ cc_test(
":value_kind",
":value_testing",
"//base:attributes",
"//eval/public:cel_value",
"//eval/public/structs:proto_message_type_adapter",
"//internal:parse_text_proto",
"//internal:status_macros",
"//internal:testing",
Expand Down
156 changes: 100 additions & 56 deletions common/legacy_value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,20 @@
#include "common/unknown.h"
#include "common/value.h"
#include "common/value_kind.h"
#include "common/values/legacy_list_value.h"
#include "common/values/legacy_map_value.h"
#include "common/values/list_value_builder.h"
#include "common/values/map_value_builder.h"
#include "common/values/values.h"
#include "eval/internal/cel_value_equal.h"
#include "eval/public/cel_value.h"
#include "eval/public/containers/field_backed_list_impl.h"
#include "eval/public/containers/field_backed_map_impl.h"
#include "eval/public/message_wrapper.h"
#include "eval/public/structs/cel_proto_wrap_util.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_internal.h"
#include "internal/json.h"
#include "internal/status_macros.h"
#include "internal/well_known_types.h"
#include "runtime/runtime_options.h"
#include "google/protobuf/arena.h"
#include "google/protobuf/descriptor.h"
Expand All @@ -76,13 +74,9 @@ using ::cel::interop_internal::TrivialTypeInfo;
using ::google::api::expr::runtime::CelList;
using ::google::api::expr::runtime::CelMap;
using ::google::api::expr::runtime::CelValue;
using ::google::api::expr::runtime::CreateCelValueFromField;
using ::google::api::expr::runtime::FieldBackedListImpl;
using ::google::api::expr::runtime::FieldBackedMapImpl;
using ::google::api::expr::runtime::GetGenericProtoTypeInfoInstance;
using ::google::api::expr::runtime::LegacyTypeInfoApis;
using ::google::api::expr::runtime::MessageWrapper;
using ::google::api::expr::runtime::internal::GetGenericProtoAccessApisInstance;
using ::google::api::expr::runtime::internal::MaybeWrapValueToMessage;

absl::Status InvalidMapKeyTypeError(ValueKind kind) {
Expand Down Expand Up @@ -284,19 +278,17 @@ CelValue LegacyTrivialListValue(google::protobuf::Arena* absl_nonnull arena,
}
if (auto parsed_repeated_field_value = value.AsParsedRepeatedField();
parsed_repeated_field_value) {
auto maybe_cloned = parsed_repeated_field_value->Clone(arena);
return CelValue::CreateList(google::protobuf::Arena::Create<FieldBackedListImpl>(
arena, &maybe_cloned.message(), maybe_cloned.field(), arena));
auto wrapped = common_internal::WrapLegacyParsedRepeatedField(
*parsed_repeated_field_value, arena);
return CelValue::CreateList(
common_internal::AsLegacyListValue(wrapped)->cel_list());
}
if (auto parsed_json_list_value = value.AsParsedJsonList();
parsed_json_list_value) {
auto maybe_cloned = parsed_json_list_value->Clone(arena);
return CelValue::CreateList(google::protobuf::Arena::Create<FieldBackedListImpl>(
arena, cel::to_address(maybe_cloned),
well_known_types::GetListValueReflectionOrDie(
maybe_cloned->GetDescriptor())
.GetValuesDescriptor(),
arena));
auto wrapped = common_internal::WrapLegacyParsedJsonList(
*parsed_json_list_value, arena);
return CelValue::CreateList(
common_internal::AsLegacyListValue(wrapped)->cel_list());
}
if (auto custom_list_value = value.AsCustomList(); custom_list_value) {
auto status_or_compat_list = common_internal::MakeCompatListValue(
Expand All @@ -322,19 +314,17 @@ CelValue LegacyTrivialMapValue(google::protobuf::Arena* absl_nonnull arena,
}
if (auto parsed_map_field_value = value.AsParsedMapField();
parsed_map_field_value) {
auto maybe_cloned = parsed_map_field_value->Clone(arena);
return CelValue::CreateMap(google::protobuf::Arena::Create<FieldBackedMapImpl>(
arena, &maybe_cloned.message(), maybe_cloned.field(), arena));
auto wrapped = common_internal::WrapLegacyParsedMapField(
*parsed_map_field_value, arena);
return CelValue::CreateMap(
common_internal::AsLegacyMapValue(wrapped)->cel_map());
}
if (auto parsed_json_map_value = value.AsParsedJsonMap();
parsed_json_map_value) {
auto maybe_cloned = parsed_json_map_value->Clone(arena);
return CelValue::CreateMap(google::protobuf::Arena::Create<FieldBackedMapImpl>(
arena, cel::to_address(maybe_cloned),
well_known_types::GetStructReflectionOrDie(
maybe_cloned->GetDescriptor())
.GetFieldsDescriptor(),
arena));
auto wrapped =
common_internal::WrapLegacyParsedJsonMap(*parsed_json_map_value, arena);
return CelValue::CreateMap(
common_internal::AsLegacyMapValue(wrapped)->cel_map());
}
if (auto custom_map_value = value.AsCustomMap(); custom_map_value) {
auto status_or_compat_map = common_internal::MakeCompatMapValue(
Expand All @@ -352,6 +342,25 @@ CelValue LegacyTrivialMapValue(google::protobuf::Arena* absl_nonnull arena,
value.GetRuntimeType().DebugString()))));
}

LegacyStructValue ParsedMessageToLegacyStructValue(
const ParsedMessageValue& parsed_message) {
return LegacyStructValue(cel::to_address(parsed_message),
&GetGenericProtoTypeInfoInstance());
}

LegacyStructValue MakeLegacyStructValue(
const google::protobuf::Message* absl_nonnull message,
const LegacyTypeInfoApis* legacy_type_info) {
// Guard against edge cases where a custom implementation of Message
// misbehaves.
// Modern value handles this with DCHECKs on value creation, legacy value
// would allow it and just report an ErrorValue on accesses.
if (message->GetReflection() == nullptr || legacy_type_info == nullptr) {
legacy_type_info = TrivialTypeInfo::GetInstance();
}
return LegacyStructValue(message, legacy_type_info);
}

} // namespace

google::api::expr::runtime::CelValue UnsafeLegacyValue(
Expand Down Expand Up @@ -394,10 +403,6 @@ google::api::expr::runtime::CelValue UnsafeLegacyValue(
}
}

} // namespace common_internal

namespace common_internal {

std::string LegacyListValue::DebugString() const {
return CelValue::CreateList(impl_).DebugString();
}
Expand Down Expand Up @@ -837,10 +842,8 @@ absl::Status LegacyStructValue::SerializeTo(
ABSL_DCHECK(message_factory != nullptr);
ABSL_DCHECK(output != nullptr);

auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
if (ABSL_PREDICT_TRUE(
message_wrapper.message_ptr()->SerializePartialToZeroCopyStream(
output))) {
message_ptr_->SerializePartialToZeroCopyStream(output))) {
return absl::OkStatus();
}
return absl::UnknownError("failed to serialize protocol buffer message");
Expand Down Expand Up @@ -918,17 +921,26 @@ absl::Status LegacyStructValue::GetFieldByName(
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 {
auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
*result = NoSuchFieldError(name);
return absl::OkStatus();
}
CEL_ASSIGN_OR_RETURN(auto cel_value,
GetGenericProtoAccessApisInstance().GetField(
name, message_wrapper, unboxing_options,
MemoryManagerRef::Pooling(arena)));
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, *result));
return absl::OkStatus();

ParsedMessageValue parsed_message = UnsafeParsedMessageValue(message_ptr_);
const auto* descriptor = parsed_message.GetDescriptor();
const auto* field = descriptor->FindFieldByName(name);
if (field == nullptr) {
field = descriptor->file()->pool()->FindExtensionByPrintableName(descriptor,
name);
if (field == nullptr) {
*result = NoSuchFieldError(name);
return absl::OkStatus();
}
}

return interop_internal::WrapLegacyMessageField(
message_ptr_, field, unboxing_options, descriptor_pool, message_factory,
arena, result);
}

absl::Status LegacyStructValue::GetFieldByNumber(
Expand Down Expand Up @@ -980,7 +992,6 @@ absl::Status LegacyStructValue::Qualify(
if (ABSL_PREDICT_FALSE(qualifiers.empty())) {
return absl::InvalidArgumentError("invalid select qualifier path.");
}
auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
absl::string_view field_name = absl::visit(
absl::Overload(
Expand All @@ -995,12 +1006,13 @@ absl::Status LegacyStructValue::Qualify(
*count = -1;
return absl::OkStatus();
}
CEL_ASSIGN_OR_RETURN(auto legacy_result,
GetGenericProtoAccessApisInstance().Qualify(
qualifiers, message_wrapper, presence_test,
MemoryManager::Pooling(arena)));
CEL_RETURN_IF_ERROR(ModernValue(arena, legacy_result.value, *result));
*count = legacy_result.qualifier_count;

ParsedMessageValue parsed_message = UnsafeParsedMessageValue(message_ptr_);
CEL_RETURN_IF_ERROR(parsed_message.Qualify(qualifiers, presence_test,
descriptor_pool, message_factory,
arena, result, count));

interop_internal::WrapLegacyFieldAccessResult(arena, result);
return absl::OkStatus();
}

Expand Down Expand Up @@ -1035,7 +1047,7 @@ absl::Status ModernValue(google::protobuf::Arena* arena,
return absl::OkStatus();
case CelValue::Type::kMessage: {
auto message_wrapper = legacy_value.MessageWrapperOrDie();
result = common_internal::LegacyStructValue(
result = common_internal::MakeLegacyStructValue(
google::protobuf::DownCastMessage<google::protobuf::Message>(
message_wrapper.message_ptr()),
message_wrapper.legacy_type_info());
Expand Down Expand Up @@ -1153,7 +1165,7 @@ absl::StatusOr<Value> FromLegacyValue(google::protobuf::Arena* arena,
legacy_value.BytesOrDie().value());
case CelValue::Type::kMessage: {
auto message_wrapper = legacy_value.MessageWrapperOrDie();
return common_internal::LegacyStructValue(
return common_internal::MakeLegacyStructValue(
google::protobuf::DownCastMessage<google::protobuf::Message>(
message_wrapper.message_ptr()),
message_wrapper.legacy_type_info());
Expand Down Expand Up @@ -1262,6 +1274,33 @@ google::api::expr::runtime::CelValue ModernValueToLegacyValueOrDie(
return std::move(*status_or_value);
}

void WrapLegacyFieldAccessResult(google::protobuf::Arena* absl_nonnull arena,
Value* absl_nonnull result) {
if (result->IsParsedMessage()) {
*result = common_internal::ParsedMessageToLegacyStructValue(
result->GetParsedMessage());
} else if (result->IsParsedRepeatedField()) {
*result =
WrapLegacyParsedRepeatedField(result->GetParsedRepeatedField(), arena);
} else if (result->IsParsedJsonList()) {
*result = WrapLegacyParsedJsonList(result->GetParsedJsonList(), arena);
} else if (result->IsParsedMapField()) {
*result = WrapLegacyParsedMapField(result->GetParsedMapField(), arena);
} else if (result->IsParsedJsonMap()) {
*result = WrapLegacyParsedJsonMap(result->GetParsedJsonMap(), arena);
} else if (result->IsList()) {
auto is_empty = result->GetList().IsEmpty();
if (is_empty.ok() && *is_empty) {
*result = CustomListValue(common_internal::EmptyCompatListValue(), arena);
}
} else if (result->IsMap()) {
auto is_empty = result->GetMap().IsEmpty();
if (is_empty.ok() && *is_empty) {
*result = CustomMapValue(common_internal::EmptyCompatMapValue(), arena);
}
}
}

TypeValue CreateTypeValueFromView(google::protobuf::Arena* arena,
absl::string_view input) {
return TypeValue(common_internal::LegacyRuntimeType(input));
Expand Down Expand Up @@ -1289,12 +1328,17 @@ const google::protobuf::Message* absl_nullable GetLegacyMessage(const Value& val
absl::Status WrapLegacyMessageField(
const google::protobuf::Message* absl_nonnull message,
const google::protobuf::FieldDescriptor* absl_nonnull field_descriptor,
ProtoWrapperTypeOptions unboxing_option, google::protobuf::Arena* arena,
ProtoWrapperTypeOptions unboxing_option,
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* arena,
Value* absl_nonnull out) {
CEL_ASSIGN_OR_RETURN(CelValue result,
CreateCelValueFromField(message, field_descriptor,
unboxing_option, arena));
return ModernValue(arena, result, *out);
ParsedMessageValue parsed_message = UnsafeParsedMessageValue(message);
CEL_RETURN_IF_ERROR(parsed_message.GetField(field_descriptor, unboxing_option,
descriptor_pool, message_factory,
arena, out));
WrapLegacyFieldAccessResult(arena, out);

return absl::OkStatus();
}

} // namespace interop_internal
Expand Down
15 changes: 14 additions & 1 deletion common/legacy_value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,32 @@ google::api::expr::runtime::CelValue UnsafeLegacyValue(

} // namespace cel

namespace proto2 {
class MessageFactory;
} // namespace proto2

namespace cel::interop_internal {

// Returns the underlying `google::protobuf::Message` of a `cel::Value` if it is a legacy
// message with the default type info, or `nullptr` otherwise.
const google::protobuf::Message* absl_nullable GetLegacyMessage(const Value& value);

// Helper for wrapping a field accesses for the legacy runtime.
//
// Adapts the output to avoid further allocations when converting to a legacy
// value when possible.
void WrapLegacyFieldAccessResult(google::protobuf::Arena* absl_nonnull arena,
Value* absl_nonnull result);

// Access a field on a legacy message value, writing the result to `out`.
// Prefers wrapping legacy values instead of using the modern value
// representation.
absl::Status WrapLegacyMessageField(
const google::protobuf::Message* absl_nonnull message,
const google::protobuf::FieldDescriptor* absl_nonnull field_descriptor,
ProtoWrapperTypeOptions unboxing_option, google::protobuf::Arena* arena,
ProtoWrapperTypeOptions unboxing_option,
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* arena,
Value* absl_nonnull out);

absl::StatusOr<Value> FromLegacyValue(
Expand Down
21 changes: 15 additions & 6 deletions common/value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <string>
#include <type_traits>
#include <utility>
#include <variant>

#include "google/protobuf/struct.pb.h"
#include "absl/base/attributes.h"
Expand Down Expand Up @@ -1512,10 +1513,14 @@ Value WrapFieldImpl(
ABSL_DCHECK(!IsWellKnownMessageType(message->GetDescriptor()));

const auto* reflection = message->GetReflection();
if (ABSL_PREDICT_FALSE(reflection == nullptr)) {
// This only happens for special implementations of Message that
// should not normally be used with CEL.
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("failed to get reflection for message type: ",
message->GetDescriptor()->full_name())));
}
if (field->is_map()) {
if (reflection->FieldSize(*message, field) == 0) {
return MapValue();
}
if constexpr (Unsafe::value) {
return UnsafeParsedMapFieldValue(message, field);
} else {
Expand All @@ -1524,9 +1529,6 @@ Value WrapFieldImpl(
}
}
if (field->is_repeated()) {
if (reflection->FieldSize(*message, field) == 0) {
return ListValue();
}
if constexpr (Unsafe::value) {
return UnsafeParsedRepeatedFieldValue(message, field);
} else {
Expand Down Expand Up @@ -1653,6 +1655,13 @@ Value WrapRepeatedFieldImpl(
ABSL_DCHECK(arena != nullptr);

const auto* reflection = message->GetReflection();
if (ABSL_PREDICT_FALSE(reflection == nullptr)) {
// This only happens for special implementations of Message that
// should not normally be used with CEL.
return ErrorValue(absl::InvalidArgumentError(
absl::StrCat("failed to get reflection for message type: ",
message->GetDescriptor()->full_name())));
}
const int size = reflection->FieldSize(*message, field);
if (ABSL_PREDICT_FALSE(index < 0 || index >= size)) {
return ErrorValue(absl::InvalidArgumentError(
Expand Down
Loading
Loading