Skip to content

Commit 62c1978

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Migrate gaer::CelProtoWrapper::CreateMessage to just call the modern
equivalent. Add overload for specifying the expected message factory and descriptor pool. PiperOrigin-RevId: 971405326
1 parent ff777e1 commit 62c1978

15 files changed

Lines changed: 296 additions & 129 deletions

‎common/legacy_value.cc‎

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,9 @@ using ::cel::interop_internal::TrivialTypeInfo;
7474
using ::google::api::expr::runtime::CelList;
7575
using ::google::api::expr::runtime::CelMap;
7676
using ::google::api::expr::runtime::CelValue;
77-
using ::google::api::expr::runtime::CreateCelValueFromField;
7877
using ::google::api::expr::runtime::GetGenericProtoTypeInfoInstance;
7978
using ::google::api::expr::runtime::LegacyTypeInfoApis;
8079
using ::google::api::expr::runtime::MessageWrapper;
81-
using ::google::api::expr::runtime::internal::GetGenericProtoAccessApisInstance;
8280
using ::google::api::expr::runtime::internal::MaybeWrapValueToMessage;
8381

8482
absl::Status InvalidMapKeyTypeError(ValueKind kind) {
@@ -262,6 +260,11 @@ CelValue LegacyTrivialStructValue(google::protobuf::Arena* absl_nonnull arena,
262260
}
263261
if (auto parsed_message_value = value.AsParsedMessage();
264262
parsed_message_value) {
263+
if (interop_internal::IsUnsafeParsedMessageValue(*parsed_message_value)) {
264+
return CelValue::CreateMessageWrapper(
265+
AsMessageWrapper(cel::to_address(*parsed_message_value),
266+
&GetGenericProtoTypeInfoInstance()));
267+
}
265268
auto maybe_cloned = parsed_message_value->Clone(arena);
266269
return CelValue::CreateMessageWrapper(MessageWrapper(
267270
cel::to_address(maybe_cloned), &GetGenericProtoTypeInfoInstance()));
@@ -923,17 +926,26 @@ absl::Status LegacyStructValue::GetFieldByName(
923926
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
924927
google::protobuf::MessageFactory* absl_nonnull message_factory,
925928
google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const {
926-
auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
927929
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
928930
*result = NoSuchFieldError(name);
929931
return absl::OkStatus();
930932
}
931-
CEL_ASSIGN_OR_RETURN(auto cel_value,
932-
GetGenericProtoAccessApisInstance().GetField(
933-
name, message_wrapper, unboxing_options,
934-
MemoryManagerRef::Pooling(arena)));
935-
CEL_RETURN_IF_ERROR(ModernValue(arena, cel_value, *result));
936-
return absl::OkStatus();
933+
934+
ParsedMessageValue parsed_message = UnsafeParsedMessageValue(message_ptr_);
935+
const auto* descriptor = parsed_message.GetDescriptor();
936+
const auto* field = descriptor->FindFieldByName(name);
937+
if (field == nullptr) {
938+
field = descriptor->file()->pool()->FindExtensionByPrintableName(descriptor,
939+
name);
940+
if (field == nullptr) {
941+
*result = NoSuchFieldError(name);
942+
return absl::OkStatus();
943+
}
944+
}
945+
946+
return interop_internal::WrapLegacyMessageField(
947+
message_ptr_, field, unboxing_options, descriptor_pool, message_factory,
948+
arena, result);
937949
}
938950

939951
absl::Status LegacyStructValue::GetFieldByNumber(
@@ -985,7 +997,6 @@ absl::Status LegacyStructValue::Qualify(
985997
if (ABSL_PREDICT_FALSE(qualifiers.empty())) {
986998
return absl::InvalidArgumentError("invalid select qualifier path.");
987999
}
988-
auto message_wrapper = AsMessageWrapper(message_ptr_, legacy_type_info_);
9891000
if (ABSL_PREDICT_FALSE(legacy_type_info_ == TrivialTypeInfo::GetInstance())) {
9901001
absl::string_view field_name = absl::visit(
9911002
absl::Overload(
@@ -1000,12 +1011,13 @@ absl::Status LegacyStructValue::Qualify(
10001011
*count = -1;
10011012
return absl::OkStatus();
10021013
}
1003-
CEL_ASSIGN_OR_RETURN(auto legacy_result,
1004-
GetGenericProtoAccessApisInstance().Qualify(
1005-
qualifiers, message_wrapper, presence_test,
1006-
MemoryManager::Pooling(arena)));
1007-
CEL_RETURN_IF_ERROR(ModernValue(arena, legacy_result.value, *result));
1008-
*count = legacy_result.qualifier_count;
1014+
1015+
ParsedMessageValue parsed_message = UnsafeParsedMessageValue(message_ptr_);
1016+
CEL_RETURN_IF_ERROR(parsed_message.Qualify(qualifiers, presence_test,
1017+
descriptor_pool, message_factory,
1018+
arena, result, count));
1019+
1020+
interop_internal::WrapLegacyFieldAccessResult(arena, result);
10091021
return absl::OkStatus();
10101022
}
10111023

@@ -1311,12 +1323,17 @@ const google::protobuf::Message* absl_nullable GetLegacyMessage(const Value& val
13111323
absl::Status WrapLegacyMessageField(
13121324
const google::protobuf::Message* absl_nonnull message,
13131325
const google::protobuf::FieldDescriptor* absl_nonnull field_descriptor,
1314-
ProtoWrapperTypeOptions unboxing_option, google::protobuf::Arena* arena,
1326+
ProtoWrapperTypeOptions unboxing_option,
1327+
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
1328+
google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* arena,
13151329
Value* absl_nonnull out) {
1316-
CEL_ASSIGN_OR_RETURN(CelValue result,
1317-
CreateCelValueFromField(message, field_descriptor,
1318-
unboxing_option, arena));
1319-
return ModernValue(arena, result, *out);
1330+
ParsedMessageValue parsed_message = UnsafeParsedMessageValue(message);
1331+
CEL_RETURN_IF_ERROR(parsed_message.GetField(field_descriptor, unboxing_option,
1332+
descriptor_pool, message_factory,
1333+
arena, out));
1334+
WrapLegacyFieldAccessResult(arena, out);
1335+
1336+
return absl::OkStatus();
13201337
}
13211338

13221339
} // namespace interop_internal

‎common/legacy_value.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ class MessageFactory;
6565

6666
namespace cel::interop_internal {
6767

68+
inline bool IsUnsafeParsedMessageValue(const cel::ParsedMessageValue& value) {
69+
return value.is_unsafe();
70+
}
71+
6872
// Returns the underlying `google::protobuf::Message` of a `cel::Value` if it is a legacy
6973
// message with the default type info, or `nullptr` otherwise.
7074
const google::protobuf::Message* absl_nullable GetLegacyMessage(const Value& value);
@@ -82,7 +86,9 @@ void WrapLegacyFieldAccessResult(google::protobuf::Arena* absl_nonnull arena,
8286
absl::Status WrapLegacyMessageField(
8387
const google::protobuf::Message* absl_nonnull message,
8488
const google::protobuf::FieldDescriptor* absl_nonnull field_descriptor,
85-
ProtoWrapperTypeOptions unboxing_option, google::protobuf::Arena* arena,
89+
ProtoWrapperTypeOptions unboxing_option,
90+
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
91+
google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* arena,
8692
Value* absl_nonnull out);
8793

8894
absl::StatusOr<Value> FromLegacyValue(

‎common/values/legacy_map_value.cc‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -401,21 +401,24 @@ class LegacyParsedJsonMapValue final
401401
if (arena == nullptr) {
402402
arena = arena_;
403403
}
404-
if (auto status =
405-
google::api::expr::runtime::CelValue::CheckMapKeyType(key);
406-
!status.ok()) {
407-
status.IgnoreError();
408-
return std::nullopt;
409-
}
410404
Value modern_key;
411405
if (ABSL_PREDICT_FALSE(!ModernValue(arena, key, modern_key).ok())) {
412406
return std::nullopt;
413407
}
414408
Value modern_val;
415-
auto status_or_found = value_.Find(
416-
modern_key, google::protobuf::DescriptorPool::generated_pool(),
417-
google::protobuf::MessageFactory::generated_factory(), arena, &modern_val);
418-
if (!status_or_found.ok() || !*status_or_found) {
409+
// Call custom map FindDirectly. MapValue normally handles coercing error
410+
// results to value types, so emulate that here.
411+
//
412+
// We know that the descriptor pool and message factory aren't needed here,
413+
// so fine to use generated.
414+
auto found =
415+
Find(modern_key, google::protobuf::DescriptorPool::generated_pool(),
416+
google::protobuf::MessageFactory::generated_factory(), arena, &modern_val);
417+
if (!found.ok()) {
418+
return google::api::expr::runtime::CreateErrorValue(arena,
419+
found.status());
420+
}
421+
if (!(*found) && !modern_val.IsError()) {
419422
return std::nullopt;
420423
}
421424
return UnsafeLegacyValue(modern_val, /*stable=*/false, arena);

‎common/values/parsed_message_value.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@
4848

4949
namespace cel {
5050

51+
namespace interop_internal {
52+
bool IsUnsafeParsedMessageValue(const ParsedMessageValue& value);
53+
}
54+
5155
class MessageValue;
5256
class StructValue;
5357
class Value;
@@ -189,6 +193,8 @@ class ParsedMessageValue final
189193
friend class common_internal::StructValueMixin<ParsedMessageValue>;
190194
friend ParsedMessageValue UnsafeParsedMessageValue(
191195
const google::protobuf::Message* absl_nonnull value);
196+
friend bool interop_internal::IsUnsafeParsedMessageValue(
197+
const ParsedMessageValue& value);
192198

193199
explicit ParsedMessageValue(
194200
const google::protobuf::Message* absl_nonnull value ABSL_ATTRIBUTE_LIFETIME_BOUND)

‎eval/eval/select_step.cc‎

Lines changed: 43 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,22 @@ absl::Status WrappedStructGet(
8686
ProtoWrapperTypeOptions unboxing_option,
8787
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
8888
google::protobuf::MessageFactory* absl_nonnull message_factory,
89-
google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) {
90-
if (const google::protobuf::Message* message =
91-
cel::interop_internal::GetLegacyMessage(target);
92-
message != nullptr) {
93-
CelValue::MessageWrapper message_wrapper(
94-
message, &GetGenericProtoTypeInfoInstance());
95-
CEL_ASSIGN_OR_RETURN(CelValue cel_value,
96-
internal::GetGenericProtoAccessApisInstance().GetField(
97-
field, message_wrapper, unboxing_option,
98-
cel::MemoryManagerRef::Pooling(arena)));
99-
return cel::ModernValue(arena, cel_value, *result);
89+
google::protobuf::Arena* absl_nonnull arena,
90+
bool enable_use_new_field_select_implementation,
91+
Value* absl_nonnull result) {
92+
if (!enable_use_new_field_select_implementation) {
93+
if (const google::protobuf::Message* message =
94+
cel::interop_internal::GetLegacyMessage(target);
95+
message != nullptr) {
96+
CelValue::MessageWrapper message_wrapper(
97+
message, &GetGenericProtoTypeInfoInstance());
98+
CEL_ASSIGN_OR_RETURN(
99+
CelValue cel_value,
100+
internal::GetGenericProtoAccessApisInstance().GetField(
101+
field, message_wrapper, unboxing_option,
102+
cel::MemoryManagerRef::Pooling(arena)));
103+
return cel::ModernValue(arena, cel_value, *result);
104+
}
100105
}
101106
return target.GetStruct().GetFieldByName(
102107
field, unboxing_option, descriptor_pool, message_factory, arena, result);
@@ -132,7 +137,9 @@ absl::Status PerformGet(const Value& target, absl::string_view field,
132137
ProtoWrapperTypeOptions unboxing_option,
133138
const google::protobuf::DescriptorPool* descriptor_pool,
134139
google::protobuf::MessageFactory* message_factory,
135-
google::protobuf::Arena* arena, Value& result) {
140+
google::protobuf::Arena* arena,
141+
bool enable_use_new_field_select_implementation,
142+
Value& result) {
136143
switch (target.kind()) {
137144
case ValueKind::kMap: {
138145
auto status = target.GetMap().Get(field_value, descriptor_pool,
@@ -143,9 +150,9 @@ absl::Status PerformGet(const Value& target, absl::string_view field,
143150
return absl::OkStatus();
144151
}
145152
case ValueKind::kStruct: {
146-
auto status =
147-
WrappedStructGet(target, field, unboxing_option, descriptor_pool,
148-
message_factory, arena, &result);
153+
auto status = WrappedStructGet(
154+
target, field, unboxing_option, descriptor_pool, message_factory,
155+
arena, enable_use_new_field_select_implementation, &result);
149156
if (!status.ok()) {
150157
result = ErrorValue(std::move(status));
151158
}
@@ -161,7 +168,9 @@ absl::Status PerformOptionalGet(const Value& target, absl::string_view field,
161168
ProtoWrapperTypeOptions unboxing_option,
162169
const google::protobuf::DescriptorPool* descriptor_pool,
163170
google::protobuf::MessageFactory* message_factory,
164-
google::protobuf::Arena* arena, Value& result) {
171+
google::protobuf::Arena* arena,
172+
bool enable_use_new_field_select_implementation,
173+
Value& result) {
165174
switch (target.kind()) {
166175
case ValueKind::kMap: {
167176
CEL_ASSIGN_OR_RETURN(
@@ -182,9 +191,9 @@ absl::Status PerformOptionalGet(const Value& target, absl::string_view field,
182191
result = OptionalValue::None();
183192
return absl::OkStatus();
184193
}
185-
CEL_RETURN_IF_ERROR(WrappedStructGet(target, field, unboxing_option,
186-
descriptor_pool, message_factory,
187-
arena, &result));
194+
CEL_RETURN_IF_ERROR(WrappedStructGet(
195+
target, field, unboxing_option, descriptor_pool, message_factory,
196+
arena, enable_use_new_field_select_implementation, &result));
188197

189198
ABSL_DCHECK(!result.IsUnknown());
190199
result = OptionalValue::Of(std::move(result), arena);
@@ -247,7 +256,7 @@ absl::Status SelectStep::Evaluate(ExecutionFrame* frame) const {
247256
optional_arg = arg.GetOptional();
248257
}
249258

250-
if (!(optional_arg || arg->Is<MapValue>() || arg->Is<StructValue>())) {
259+
if (!(optional_arg || arg.IsMap() || arg.IsStruct())) {
251260
frame->value_stack().PopAndPush(cel::ErrorValue(InvalidSelectTargetError()),
252261
std::move(result_trail));
253262
return absl::OkStatus();
@@ -290,7 +299,8 @@ absl::Status SelectStep::Evaluate(ExecutionFrame* frame) const {
290299
optional_arg->Value(&value);
291300
auto status = PerformOptionalGet(
292301
value, field_, field_value_, unboxing_option_, frame->descriptor_pool(),
293-
frame->message_factory(), frame->arena(), result);
302+
frame->message_factory(), frame->arena(),
303+
frame->options().enable_use_new_field_select_implementation, result);
294304
if (!status.ok()) {
295305
result = ErrorValue(std::move(status));
296306
}
@@ -300,7 +310,8 @@ absl::Status SelectStep::Evaluate(ExecutionFrame* frame) const {
300310

301311
CEL_RETURN_IF_ERROR(PerformGet(
302312
arg, field_, field_value_, unboxing_option_, frame->descriptor_pool(),
303-
frame->message_factory(), frame->arena(), result));
313+
frame->message_factory(), frame->arena(),
314+
frame->options().enable_use_new_field_select_implementation, result));
304315
frame->value_stack().PopAndPush(std::move(result), std::move(result_trail));
305316
return absl::OkStatus();
306317
}
@@ -380,19 +391,20 @@ class DirectSelectStep : public DirectExpressionStep {
380391
}
381392
Value value;
382393
optional_arg->Value(&value);
383-
auto status =
384-
PerformOptionalGet(value, field_, field_value_, unboxing_option_,
385-
frame.descriptor_pool(), frame.message_factory(),
386-
frame.arena(), result);
394+
auto status = PerformOptionalGet(
395+
value, field_, field_value_, unboxing_option_,
396+
frame.descriptor_pool(), frame.message_factory(), frame.arena(),
397+
frame.options().enable_use_new_field_select_implementation, result);
387398
if (!status.ok()) {
388399
result = ErrorValue(std::move(status));
389400
}
390401
return absl::OkStatus();
391402
}
392403

393-
return PerformGet(result, field_, field_value_, unboxing_option_,
394-
frame.descriptor_pool(), frame.message_factory(),
395-
frame.arena(), result);
404+
return PerformGet(
405+
result, field_, field_value_, unboxing_option_, frame.descriptor_pool(),
406+
frame.message_factory(), frame.arena(),
407+
frame.options().enable_use_new_field_select_implementation, result);
396408
}
397409

398410
private:
@@ -495,7 +507,8 @@ absl::Status ProtoSelectStep::EvaluateLegacyMessageGetField(
495507
return absl::OkStatus();
496508
}
497509
return cel::interop_internal::WrapLegacyMessageField(
498-
legacy_message, field_descriptor_, unboxing_option_, frame->arena(),
510+
legacy_message, field_descriptor_, unboxing_option_,
511+
frame->descriptor_pool(), frame->message_factory(), frame->arena(),
499512
&frame->value_stack().Peek());
500513
}
501514

‎eval/internal/cel_value_equal_test.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ const std::vector<CelValue>& ValueExamples1() {
131131
result->push_back(CelValue::CreateMap(&CelMapExample1()));
132132
result->push_back(CelValue::CreateCelTypeView("type"));
133133

134+
ABSL_CHECK_EQ(arena.SpaceUsed(), 0) << "Arena should not be used.";
135+
134136
return result.release();
135137
}();
136138
return *examples;

‎eval/public/cel_options.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ cel::RuntimeOptions ConvertToRuntimeOptions(const InterpreterOptions& options) {
4545
options.enable_fast_builtins,
4646
options.enable_precision_preserving_double_format,
4747
options.enable_typed_field_access,
48+
options.enable_use_new_field_select_implementation,
4849
};
4950
}
5051

‎eval/public/cel_options.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,16 @@ struct InterpreterOptions {
223223
// path for field access when the type is known at plan time, instead of using
224224
// the generic field access implementation.
225225
bool enable_typed_field_access = false;
226+
227+
// Temporary flag to gate using a new field selection implementation for
228+
// protos.
229+
//
230+
// For the cel::Runtime APIs, this is a no-op.
231+
//
232+
// For google::api::expr::runtime::CelExpression, this will enable updated
233+
// implementations for field access on protobuf messages, aligned with the
234+
// cel::Value implementation.
235+
bool enable_use_new_field_select_implementation = false;
226236
};
227237
// LINT.ThenChange(//depot/google3/runtime/runtime_options.h)
228238

‎eval/public/structs/BUILD‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,17 @@ cc_library(
3030
deps = [
3131
":cel_proto_wrap_util",
3232
":proto_message_type_adapter",
33+
":trivial_legacy_type_info_internal",
34+
"//common:value",
3335
"//eval/public:cel_value",
3436
"//eval/public:message_wrapper",
3537
"//internal:proto_time_encoding",
36-
"@com_google_absl//absl/types:optional",
38+
"@com_google_absl//absl/base:no_destructor",
39+
"@com_google_absl//absl/base:nullability",
40+
"@com_google_absl//absl/log:absl_check",
41+
"@com_google_absl//absl/log:absl_log",
42+
"@com_google_absl//absl/status",
43+
"@com_google_absl//absl/status:statusor",
3744
"@com_google_protobuf//:duration_cc_proto",
3845
"@com_google_protobuf//:protobuf",
3946
"@com_google_protobuf//:timestamp_cc_proto",

0 commit comments

Comments
 (0)