diff --git a/common/BUILD b/common/BUILD index 818854692..937a27f45 100644 --- a/common/BUILD +++ b/common/BUILD @@ -886,7 +886,6 @@ cc_test( "@com_google_absl//absl/time", "@com_google_absl//absl/types:optional", "@com_google_cel_spec//proto/cel/expr/conformance/proto3:test_all_types_cc_proto", - "@com_google_protobuf//:field_mask_cc_proto", "@com_google_protobuf//:protobuf", "@com_google_protobuf//:struct_cc_proto", "@com_google_protobuf//:type_cc_proto", diff --git a/common/values/parsed_message_value.cc b/common/values/parsed_message_value.cc index a2d02dfd0..8a2b8030d 100644 --- a/common/values/parsed_message_value.cc +++ b/common/values/parsed_message_value.cc @@ -34,11 +34,12 @@ #include "base/attribute.h" #include "common/memory.h" #include "common/value.h" -#include "common/values/values.h" #include "extensions/protobuf/internal/qualify.h" +#include "internal/empty_descriptors.h" #include "internal/json.h" #include "internal/message_equality.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" @@ -50,6 +51,8 @@ namespace cel { namespace { +using ::cel::well_known_types::ValueReflection; + template std::enable_if_t, const google::protobuf::Message* absl_nonnull> @@ -57,6 +60,14 @@ EmptyParsedMessageValue() { return &T::default_instance(); } +template +std::enable_if_t< + std::conjunction_v, + std::negation>>, + const google::protobuf::Message* absl_nonnull> +EmptyParsedMessageValue() { + return internal::GetEmptyDefaultInstance(); +} } // namespace @@ -103,8 +114,12 @@ absl::Status ParsedMessageValue::ConvertToJson( ABSL_DCHECK_EQ(json->GetDescriptor()->well_known_type(), google::protobuf::Descriptor::WELLKNOWNTYPE_VALUE); + ValueReflection value_reflection; + CEL_RETURN_IF_ERROR(value_reflection.Initialize(json->GetDescriptor())); + google::protobuf::Message* json_object = value_reflection.MutableStructValue(json); + return internal::MessageToJson(*value_, descriptor_pool, message_factory, - json); + json_object); } absl::Status ParsedMessageValue::ConvertToJsonObject( diff --git a/common/values/parsed_message_value_test.cc b/common/values/parsed_message_value_test.cc index 14c76f684..7a84f82ba 100644 --- a/common/values/parsed_message_value_test.cc +++ b/common/values/parsed_message_value_test.cc @@ -14,9 +14,9 @@ #include -#include "google/protobuf/field_mask.pb.h" #include "google/protobuf/struct.pb.h" #include "absl/status/status_matchers.h" +#include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "common/memory.h" #include "common/type.h" @@ -74,19 +74,6 @@ TEST_F(ParsedMessageValueTest, SerializeTo) { EXPECT_THAT(std::move(output).Consume(), IsEmpty()); } -TEST_F(ParsedMessageValueTest, ConvertToJsonFieldMask) { - ParsedMessageValue value = - MakeParsedMessage(R"pb(paths: "foo.bar" - paths: "baz")pb"); - google::protobuf::Message* json = - DynamicParseTextProto(R"pb()pb"); - ASSERT_THAT(value.ConvertToJson(descriptor_pool(), message_factory(), - cel::to_address(json)), - IsOk()); - EXPECT_THAT(*json, EqualsTextProto( - R"pb(string_value: "foo.bar,baz")pb")); -} - TEST_F(ParsedMessageValueTest, ConvertToJson) { MessageValue value = MakeParsedMessage(); auto json = DynamicParseTextProto(R"pb()pb"); diff --git a/conformance/BUILD b/conformance/BUILD index 6bd2dd6ac..6f11f345b 100644 --- a/conformance/BUILD +++ b/conformance/BUILD @@ -177,6 +177,7 @@ _TESTS_TO_SKIP = [ "enums/legacy_proto2/select_big,select_neg", # Skip until fixed. + "wrappers/field_mask/to_json", "wrappers/empty/to_json", "fields/qualified_identifier_resolution/map_value_repeat_key_heterogeneous", "parse/receiver_function_names", diff --git a/eval/public/structs/BUILD b/eval/public/structs/BUILD index 504f8aa7f..4e4d5481c 100644 --- a/eval/public/structs/BUILD +++ b/eval/public/structs/BUILD @@ -71,7 +71,6 @@ cc_library( "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/functional:overload", "@com_google_absl//absl/log:absl_check", - "@com_google_absl//absl/log:absl_log", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", @@ -97,22 +96,24 @@ cc_test( ], deps = [ ":cel_proto_wrap_util", + ":protobuf_value_factory", ":trivial_legacy_type_info", "//eval/public:cel_value", + "//eval/public:message_wrapper", "//eval/public/containers:container_backed_list_impl", "//eval/public/containers:container_backed_map_impl", "//eval/testutil:test_message_cc_proto", "//internal:proto_time_encoding", + "//internal:status_macros", "//internal:testing", "//testutil:util", + "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/time", - "@com_google_absl//absl/types:span", "@com_google_protobuf//:any_cc_proto", "@com_google_protobuf//:duration_cc_proto", "@com_google_protobuf//:empty_cc_proto", - "@com_google_protobuf//:field_mask_cc_proto", "@com_google_protobuf//:protobuf", "@com_google_protobuf//:struct_cc_proto", "@com_google_protobuf//:wrappers_cc_proto", @@ -210,13 +211,13 @@ cc_test( "//eval/public/containers:container_backed_map_impl", "//eval/testutil:test_message_cc_proto", "//internal:proto_time_encoding", + "//internal:status_macros", "//internal:testing", "//testutil:util", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/time", - "@com_google_absl//absl/types:span", "@com_google_protobuf//:any_cc_proto", "@com_google_protobuf//:duration_cc_proto", "@com_google_protobuf//:empty_cc_proto", diff --git a/eval/public/structs/cel_proto_wrap_util.cc b/eval/public/structs/cel_proto_wrap_util.cc index c06e0e32e..7bfe81fe6 100644 --- a/eval/public/structs/cel_proto_wrap_util.cc +++ b/eval/public/structs/cel_proto_wrap_util.cc @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -32,7 +31,6 @@ #include "absl/base/optimization.h" #include "absl/functional/overload.h" #include "absl/log/absl_check.h" -#include "absl/log/absl_log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/cord.h" @@ -52,7 +50,6 @@ #include "internal/well_known_types.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" -#include "google/protobuf/json/json.h" #include "google/protobuf/message.h" #include "google/protobuf/message_lite.h" @@ -82,8 +79,7 @@ using google::protobuf::Descriptor; using google::protobuf::DescriptorPool; using google::protobuf::Message; using google::protobuf::MessageFactory; -using google::protobuf::json::MessageToJsonString; -using google::protobuf::json::PrintOptions; + // kMaxIntJSON is defined as the Number.MAX_SAFE_INTEGER value per EcmaScript 6. constexpr int64_t kMaxIntJSON = (1ll << 53) - 1; @@ -102,27 +98,6 @@ static bool IsJSONSafe(uint64_t i) { return i <= static_cast(kMaxIntJSON); } -static std::optional GetFieldMaskJsonString( - const google::protobuf::Message& message) { - // TODO(b/540507668): Refactor to pipe descriptor_pool through - // ValueFromValue to use internal::MessageToJson. - PrintOptions json_options; - std::string json_str; - auto status = MessageToJsonString(message, &json_str, json_options); - if (!status.ok()) { - ABSL_LOG(ERROR) << "Failed to convert FieldMask to JSON: " << status; - return std::nullopt; - } - // If JSON marshalling is correct, we know we'll always get a plain - // JSON string value and it shouldn't contain any escapes that we need - // to interpret. - if (json_str.size() >= 2 && json_str.front() == '"' && - json_str.back() == '"') { - return json_str.substr(1, json_str.size() - 2); - } - return json_str; -} - // Map implementation wrapping google.protobuf.ListValue class DynamicList : public CelList { public: @@ -1104,20 +1079,6 @@ google::protobuf::Message* ValueFromValue(google::protobuf::Message* message, co return message; } } break; - case CelValue::Type::kMessage: { - const google::protobuf::Message* message_ptr = value.MessageOrDie(); - if (message_ptr->GetDescriptor()->full_name() == - "google.protobuf.FieldMask") { - std::optional fm_str = - GetFieldMaskJsonString(*message_ptr); - if (fm_str.has_value()) { - reflection.SetStringValue(message, *fm_str); - return message; - } - return nullptr; - } - return nullptr; - } break; case CelValue::Type::kNullType: reflection.SetNullValue(message); return message; @@ -1268,20 +1229,6 @@ bool ValueFromValue(Value* json, const CelValue& value, google::protobuf::Arena* return ListFromValue(json->mutable_list_value(), value, arena); case CelValue::Type::kMap: return StructFromValue(json->mutable_struct_value(), value, arena); - case CelValue::Type::kMessage: { - const google::protobuf::Message* message_ptr = value.MessageOrDie(); - if (message_ptr->GetDescriptor()->full_name() == - "google.protobuf.FieldMask") { - std::optional fm_str = - GetFieldMaskJsonString(*message_ptr); - if (fm_str.has_value()) { - json->set_string_value(*fm_str); - return true; - } - return false; - } - return false; - } case CelValue::Type::kNullType: json->set_null_value(protobuf::NULL_VALUE); return true; @@ -1307,7 +1254,7 @@ google::protobuf::Message* AnyFromValue(const google::protobuf::Message* prototy case CelValue::Type::kBytes: { BytesValue v; type_name = v.GetTypeName(); - v.set_value(value.BytesOrDie().value()); + v.set_value(std::string(value.BytesOrDie().value())); payload = v.SerializeAsCord(); } break; case CelValue::Type::kDouble: { @@ -1333,7 +1280,7 @@ google::protobuf::Message* AnyFromValue(const google::protobuf::Message* prototy case CelValue::Type::kString: { StringValue v; type_name = v.GetTypeName(); - v.set_value(value.StringOrDie().value()); + v.set_value(std::string(value.StringOrDie().value())); payload = v.SerializeAsCord(); } break; case CelValue::Type::kTimestamp: { diff --git a/eval/public/structs/cel_proto_wrap_util_test.cc b/eval/public/structs/cel_proto_wrap_util_test.cc index 703801198..59597fe8f 100644 --- a/eval/public/structs/cel_proto_wrap_util_test.cc +++ b/eval/public/structs/cel_proto_wrap_util_test.cc @@ -15,7 +15,6 @@ #include "eval/public/structs/cel_proto_wrap_util.h" #include -#include #include #include #include @@ -25,32 +24,30 @@ #include "google/protobuf/any.pb.h" #include "google/protobuf/duration.pb.h" #include "google/protobuf/empty.pb.h" -#include "google/protobuf/field_mask.pb.h" #include "google/protobuf/struct.pb.h" #include "google/protobuf/wrappers.pb.h" +#include "absl/base/no_destructor.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/time/time.h" -#include "absl/types/span.h" #include "eval/public/cel_value.h" #include "eval/public/containers/container_backed_list_impl.h" #include "eval/public/containers/container_backed_map_impl.h" +#include "eval/public/message_wrapper.h" +#include "eval/public/structs/protobuf_value_factory.h" #include "eval/public/structs/trivial_legacy_type_info.h" #include "eval/testutil/test_message.pb.h" #include "internal/proto_time_encoding.h" +#include "internal/status_macros.h" #include "internal/testing.h" #include "testutil/util.h" -#include "google/protobuf/arena.h" #include "google/protobuf/dynamic_message.h" #include "google/protobuf/message.h" -#include "google/protobuf/text_format.h" namespace google::api::expr::runtime::internal { namespace { -using ::google::protobuf::FieldMask; -using ::google::protobuf::TextFormat; using ::testing::Eq; using ::testing::UnorderedPointwise; @@ -439,74 +436,6 @@ TEST_F(CelProtoWrapperTest, UnwrapInvalidAny) { UnwrapMessageToValue(&any, &ProtobufValueFactoryImpl, arena()).IsError()); } -TEST_F(CelProtoWrapperTest, WrapFieldMaskToValue) { - FieldMask field_mask; - ASSERT_TRUE(TextFormat::ParseFromString(R"pb( - paths: "foo.bar" paths: "baz" - )pb", - &field_mask)); - CelValue value = ProtobufValueFactoryImpl(&field_mask); - - Value expected_message; - ASSERT_TRUE(TextFormat::ParseFromString(R"pb(string_value: "foo.bar,baz")pb", - &expected_message)); - - ExpectWrappedMessage(value, expected_message); -} - -TEST_F(CelProtoWrapperTest, WrapMapWithFieldMaskToAny) { - const std::string kField = "field_mask"; - FieldMask field_mask; - ASSERT_TRUE(TextFormat::ParseFromString(R"pb( - paths: "foo.bar" paths: "baz" - )pb", - &field_mask)); - CelValue value = ProtobufValueFactoryImpl(&field_mask); - - std::vector> args = { - {CelValue::CreateString(CelValue::StringHolder(&kField)), value}}; - ASSERT_OK_AND_ASSIGN( - std::unique_ptr cel_map, - CreateContainerBackedMap( - absl::Span>(args.data(), args.size()))); - CelValue cel_value = CelValue::CreateMap(cel_map.get()); - - Struct expected_struct; - ASSERT_TRUE( - TextFormat::ParseFromString(R"pb( - fields { - key: "field_mask" - value { string_value: "foo.bar,baz" } - } - )pb", - &expected_struct)); - Any expected_message; - ASSERT_TRUE(expected_message.PackFrom(expected_struct)); - - ExpectWrappedMessage(cel_value, expected_message); -} - -TEST_F(CelProtoWrapperTest, WrapListWithFieldMaskToAny) { - FieldMask field_mask; - ASSERT_TRUE(TextFormat::ParseFromString(R"pb( - paths: "foo.bar" paths: "baz" - )pb", - &field_mask)); - CelValue value = ProtobufValueFactoryImpl(&field_mask); - - std::vector list_entries = {value}; - ContainerBackedListImpl cel_list(list_entries); - CelValue list_value = CelValue::CreateList(&cel_list); - - ListValue expected_list; - ASSERT_TRUE(TextFormat::ParseFromString( - R"pb(values { string_value: "foo.bar,baz" })pb", &expected_list)); - Any expected_message; - ASSERT_TRUE(expected_message.PackFrom(expected_list)); - - ExpectWrappedMessage(list_value, expected_message); -} - // Test support of google.protobuf.Value wrappers in CelValue. TEST_F(CelProtoWrapperTest, UnwrapBoolWrapper) { bool value = true; diff --git a/eval/public/structs/cel_proto_wrapper_test.cc b/eval/public/structs/cel_proto_wrapper_test.cc index 3ec9c9ac7..b9fcd6b51 100644 --- a/eval/public/structs/cel_proto_wrapper_test.cc +++ b/eval/public/structs/cel_proto_wrapper_test.cc @@ -1,7 +1,6 @@ #include "eval/public/structs/cel_proto_wrapper.h" #include -#include #include #include #include @@ -17,15 +16,14 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/time/time.h" -#include "absl/types/span.h" #include "eval/public/cel_value.h" #include "eval/public/containers/container_backed_list_impl.h" #include "eval/public/containers/container_backed_map_impl.h" #include "eval/testutil/test_message.pb.h" #include "internal/proto_time_encoding.h" +#include "internal/status_macros.h" #include "internal/testing.h" #include "testutil/util.h" -#include "google/protobuf/arena.h" #include "google/protobuf/dynamic_message.h" #include "google/protobuf/message.h"