prost-wkt icon indicating copy to clipboard operation
prost-wkt copied to clipboard

Add prost-wkt-derive to derive MessageSerde

Open wtzhang23 opened this issue 1 year ago • 6 comments

Motivation

  • Could help for #63
  • Support protoc-gen-*

Summary of Changes

  • Messages expose their serialization/deserialization methods in a trait-object safe way by using erased_serde
  • Manually implement Serialize and Deserialize for the Any type
    • Serialization involves serializing the type url + value, where the value calls the trait-object's erased_serialize method
    • Deserialization involves lazily deserializing the value into serde_value in case the type url is present afterwards. Once the type url and generic value are both deserialized, the generic value is deserialized using the deserializer callback found in the corresponding entry found in inventory

Breaking Changes

Due to the amount of breaking changes, this PR requires a major version bump.

  • Remove prost-wkt-build and introduce prost-wkt-derive, which provides a derive proc-macro which used in combination with message_attribute and enable_type_names will generate impls of the MessageSerde trait for messages generated by prost_build.
  • Require the use of prost::Name to determine the typeurl of messages (#57). This is breaking as the default impl of the trait leaves the package empty.
  • Removed unused MessageSerde methods and introduce the as_erased_serialize method, which is used since rust-lang/rust#65991 is still in progress

TODO

  • Optimize querying the inventory by caching type url calls instead of creating a new string.

wtzhang23 avatar Aug 30 '24 22:08 wtzhang23

Thanks for creating this PR @wtzhang23! It contains quite a few interesting changes. Can you give a short description of the main changes that are implemented?

fdeantoni avatar Sep 12 '24 11:09 fdeantoni

Updated description

wtzhang23 avatar Sep 12 '24 15:09 wtzhang23

I really like this PR in that it completely removes the need for wkt-build! I'm thinking having this included in a new 0.7.0 release. I created a new branch 0.7.x where I merged this PR. Besides the TODO item you listed here, are there any other enhancements you might want to include? We can then look at including those in the 0.7.0 release as well. Let me know what you think!

fdeantoni avatar Mar 21 '25 11:03 fdeantoni

It's been a while since I took a look, but it may be convenient to have a feature flag for this and provide a mechanism for users who prefer explicit registration of message types. Especially for the case where someone uses a library that does not register their protos or doesn't want to rely on ctor. Though not a hard requirement imo.

wtzhang23 avatar Mar 21 '25 13:03 wtzhang23

Feature flag is indeed possible but I think I would still prefer to make 0.7 use this method and dropping the build generated method entirely. I'm worried that keeping them both will be a maintenance burden which I really want to avoid if I can :)

fdeantoni avatar Apr 18 '25 07:04 fdeantoni

I think I was thinking more in the perspective of at runtime rather than using the build script but this is definitely more of a feature request than related to blocking 0.7. I'm a fan of explicitness so it could be useful to explicitly define which protos are supported to be parsed via Any (e.g. when using it as a plugin system). Though I guess one could argue you could build that yourself if needed through a long match on typeurl. Sorry for the red herring. As for any immediate recommendations for 0.7, can't think of any at the moment.

wtzhang23 avatar Apr 18 '25 17:04 wtzhang23