From 847d31afb23d2f84cae6ae04876f39dbcf6f6bad Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 25 Mar 2024 06:39:21 -0700 Subject: [PATCH] Add a convenience SomeMsg::parse(bytes) -> Result Rename .deserialize(&mut self) method to .clear_and_parse() (by marking the .deserialized deprecated pointing at the new name, will clean up usages separately) END_PUBLIC Per discussion in the team chat, parse/serialize is the most typical terminology for protobuf impls, we don't have much local reason to diverge. I'm proposing giving the 'better' name to the named ctor since I think that is the one that we expect people to reach for by default; it is generally cleaner than "new then deserialize" pattern since after a parse failure there's not any message still hanging around with implementation-defined contents, along with some other smaller ergonomics benefits. In C++ (when exceptions aren't enabled) all constructors must be infallible, so it can't have it. In Rust there's no language idiom reason why we shouldn't have an associated fn that returns Result. PiperOrigin-RevId: 618823998 --- rust/test/shared/serialization_test.rs | 27 +++++++++++----- src/google/protobuf/compiler/rust/message.cc | 34 ++++++++++++-------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/rust/test/shared/serialization_test.rs b/rust/test/shared/serialization_test.rs index 146326a96c..7f9a5100ca 100644 --- a/rust/test/shared/serialization_test.rs +++ b/rust/test/shared/serialization_test.rs @@ -31,9 +31,7 @@ fn serialize_deserialize_message() { let serialized = msg.serialize(); - let mut msg2 = TestAllTypes::new(); - assert!(msg2.deserialize(&serialized).is_ok()); - + let msg2 = TestAllTypes::parse(&serialized).unwrap(); assert_that!(msg.optional_int64(), eq(msg2.optional_int64())); assert_that!(msg.optional_bool(), eq(msg2.optional_bool())); assert_that!(msg.optional_bytes(), eq(msg2.optional_bytes())); @@ -41,15 +39,12 @@ fn serialize_deserialize_message() { #[test] fn deserialize_empty() { - let mut msg = TestAllTypes::new(); - assert!(msg.deserialize(&[]).is_ok()); + assert!(TestAllTypes::parse(&[]).is_ok()); } #[test] fn deserialize_error() { - let mut msg = TestAllTypes::new(); - let data = b"not a serialized proto"; - assert!(msg.deserialize(&*data).is_err()); + assert!(TestAllTypes::parse(b"not a serialized proto").is_err()); } #[test] @@ -61,3 +56,19 @@ fn set_bytes_with_serialized_data() { msg2.set_optional_bytes(msg.serialize()); assert_that!(msg2.optional_bytes(), eq(msg.serialize().as_ref())); } + +#[test] +fn deserialize_on_previously_allocated_message() { + let mut msg = TestAllTypes::new(); + msg.set_optional_int64(42); + msg.set_optional_bool(true); + msg.set_optional_bytes(b"serialize deserialize test"); + + let serialized = msg.serialize(); + + let mut msg2 = Box::new(TestAllTypes::new()); + assert!(msg2.clear_and_parse(&serialized).is_ok()); + assert_that!(msg.optional_int64(), eq(msg2.optional_int64())); + assert_that!(msg.optional_bool(), eq(msg2.optional_bool())); + assert_that!(msg.optional_bytes(), eq(msg2.optional_bytes())); +} diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc index 00dc6fa002..87f951bb2d 100644 --- a/src/google/protobuf/compiler/rust/message.cc +++ b/src/google/protobuf/compiler/rust/message.cc @@ -80,12 +80,12 @@ void MessageSerialize(Context& ctx, const Descriptor& msg) { ABSL_LOG(FATAL) << "unreachable"; } -void MessageDeserialize(Context& ctx, const Descriptor& msg) { +void MessageClearAndParse(Context& ctx, const Descriptor& msg) { switch (ctx.opts().kernel) { case Kernel::kCpp: ctx.Emit( { - {"deserialize_thunk", ThunkName(ctx, msg, "deserialize")}, + {"parse_thunk", ThunkName(ctx, msg, "parse")}, }, R"rs( let success = unsafe { @@ -94,17 +94,17 @@ void MessageDeserialize(Context& ctx, const Descriptor& msg) { data.len(), ); - $deserialize_thunk$(self.raw_msg(), data) + $parse_thunk$(self.raw_msg(), data) }; success.then_some(()).ok_or($pb$::ParseError) )rs"); return; case Kernel::kUpb: - ctx.Emit({{"deserialize_thunk", ThunkName(ctx, msg, "parse")}}, R"rs( + ctx.Emit({{"parse_thunk", ThunkName(ctx, msg, "parse")}}, R"rs( let arena = $pbr$::Arena::new(); let msg = unsafe { - $deserialize_thunk$(data.as_ptr(), data.len(), arena.raw()) + $parse_thunk$(data.as_ptr(), data.len(), arena.raw()) }; match msg { @@ -154,7 +154,7 @@ void MessageExterns(Context& ctx, const Descriptor& msg) { {"new_thunk", ThunkName(ctx, msg, "new")}, {"delete_thunk", ThunkName(ctx, msg, "delete")}, {"serialize_thunk", ThunkName(ctx, msg, "serialize")}, - {"deserialize_thunk", ThunkName(ctx, msg, "deserialize")}, + {"parse_thunk", ThunkName(ctx, msg, "parse")}, {"copy_from_thunk", ThunkName(ctx, msg, "copy_from")}, {"repeated_len_thunk", ThunkName(ctx, msg, "repeated_len")}, {"repeated_get_thunk", ThunkName(ctx, msg, "repeated_get")}, @@ -169,7 +169,7 @@ void MessageExterns(Context& ctx, const Descriptor& msg) { fn $new_thunk$() -> $pbi$::RawMessage; fn $delete_thunk$(raw_msg: $pbi$::RawMessage); fn $serialize_thunk$(raw_msg: $pbi$::RawMessage) -> $pbr$::SerializedData; - fn $deserialize_thunk$(raw_msg: $pbi$::RawMessage, data: $pbr$::SerializedData) -> bool; + fn $parse_thunk$(raw_msg: $pbi$::RawMessage, data: $pbr$::SerializedData) -> bool; fn $copy_from_thunk$(dst: $pbi$::RawMessage, src: $pbi$::RawMessage); fn $repeated_len_thunk$(raw: $pbi$::RawRepeatedField) -> usize; fn $repeated_add_thunk$(raw: $pbi$::RawRepeatedField) -> $pbi$::RawMessage; @@ -185,13 +185,13 @@ void MessageExterns(Context& ctx, const Descriptor& msg) { { {"new_thunk", ThunkName(ctx, msg, "new")}, {"serialize_thunk", ThunkName(ctx, msg, "serialize")}, - {"deserialize_thunk", ThunkName(ctx, msg, "parse")}, + {"parse_thunk", ThunkName(ctx, msg, "parse")}, {"minitable", UpbMinitableName(msg)}, }, R"rs( fn $new_thunk$(arena: $pbi$::RawArena) -> $pbi$::RawMessage; fn $serialize_thunk$(msg: $pbi$::RawMessage, arena: $pbi$::RawArena, len: &mut usize) -> $NonNull$; - fn $deserialize_thunk$(data: *const u8, size: usize, arena: $pbi$::RawArena) -> Option<$pbi$::RawMessage>; + fn $parse_thunk$(data: *const u8, size: usize, arena: $pbi$::RawArena) -> Option<$pbi$::RawMessage>; /// Opaque wrapper for this message's MiniTable. The only valid way to /// reference this static is with `std::ptr::addr_of!(..)`. static $minitable$: $pbr$::OpaqueMiniTable; @@ -683,7 +683,7 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { {{"Msg", RsSafeName(msg.name())}, {"Msg::new", [&] { MessageNew(ctx, msg); }}, {"Msg::serialize", [&] { MessageSerialize(ctx, msg); }}, - {"Msg::deserialize", [&] { MessageDeserialize(ctx, msg); }}, + {"Msg::clear_and_parse", [&] { MessageClearAndParse(ctx, msg); }}, {"Msg::drop", [&] { MessageDrop(ctx, msg); }}, {"Msg::debug", [&] { MessageDebug(ctx, msg); }}, {"Msg_externs", [&] { MessageExterns(ctx, msg); }}, @@ -1053,8 +1053,16 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { pub fn serialize(&self) -> $pbr$::SerializedData { self.as_view().serialize() } + #[deprecated = "Prefer Msg::parse(), or use the new name 'clear_and_parse' to parse into a pre-existing message."] pub fn deserialize(&mut self, data: &[u8]) -> Result<(), $pb$::ParseError> { - $Msg::deserialize$ + self.clear_and_parse(data) + } + pub fn clear_and_parse(&mut self, data: &[u8]) -> Result<(), $pb$::ParseError> { + $Msg::clear_and_parse$ + } + pub fn parse(data: &[u8]) -> Result { + let mut msg = Self::new(); + msg.clear_and_parse(data).map(|_| msg) } pub fn as_view(&self) -> $Msg$View { @@ -1143,7 +1151,7 @@ void GenerateThunksCc(Context& ctx, const Descriptor& msg) { {"new_thunk", ThunkName(ctx, msg, "new")}, {"delete_thunk", ThunkName(ctx, msg, "delete")}, {"serialize_thunk", ThunkName(ctx, msg, "serialize")}, - {"deserialize_thunk", ThunkName(ctx, msg, "deserialize")}, + {"parse_thunk", ThunkName(ctx, msg, "parse")}, {"copy_from_thunk", ThunkName(ctx, msg, "copy_from")}, {"repeated_len_thunk", ThunkName(ctx, msg, "repeated_len")}, {"repeated_get_thunk", ThunkName(ctx, msg, "repeated_get")}, @@ -1180,7 +1188,7 @@ void GenerateThunksCc(Context& ctx, const Descriptor& msg) { google::protobuf::rust_internal::SerializedData $serialize_thunk$($QualifiedMsg$* msg) { return google::protobuf::rust_internal::SerializeMsg(msg); } - bool $deserialize_thunk$($QualifiedMsg$* msg, + bool $parse_thunk$($QualifiedMsg$* msg, google::protobuf::rust_internal::SerializedData data) { return msg->ParseFromArray(data.data, data.len); }