-
Notifications
You must be signed in to change notification settings - Fork 231
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.
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
Blanket implementation for DomainType #571
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,13 @@ | |
//! Note that the Prost library and the TryFrom method have their own set of errors. These are | ||
//! merged into a custom Error type defined in this crate for easier handling. | ||
//! | ||
//! Requirements: | ||
//! * The DomainType trait requires the struct to implement the Clone trait. | ||
//! * The DomainType trait requires the struct to have a #[rawtype(MYRAWTYPE)] attribute set where | ||
//! the MYRAWTYPE is a structure that has the prost::Message trait implemented. (protobuf type) | ||
//! * The DomainType trait requires that the TryFrom<RawType> implemented on the structure has an | ||
//! error type that implements Into<BoxError>. (The current implementations with anomaly are | ||
//! fine.) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually some of the comments need updating now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll do that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor updates in the documentation: removed references to the DomainType macro. I hope this is it. :) |
||
//! | ||
//! How to implement a DomainType struct: | ||
//! 1. Implement your struct based on your expectations for the developer | ||
|
@@ -43,37 +50,95 @@ | |
//! | ||
//! Note: the `[rawtype()]` parameter is similar to how `serde` implements serialization through a | ||
//! `[serde(with="")]` interim type. | ||
//! | ||
|
||
use crate::Error; | ||
use crate::{Error, Kind}; | ||
use anomaly::BoxError; | ||
use bytes::{Buf, BufMut}; | ||
use prost::Message; | ||
use prost::{encoding::encoded_len_varint, Message}; | ||
use std::convert::{TryFrom, TryInto}; | ||
|
||
/// DomainType trait allows protobuf encoding and decoding for domain types | ||
pub trait DomainType<T: Message + From<Self>>: Sized { | ||
pub trait DomainType<T: Message + From<Self> + Default> | ||
where | ||
Self: Sized + Clone + TryFrom<T>, | ||
<Self as TryFrom<T>>::Error: Into<BoxError>, | ||
{ | ||
/// Encodes the DomainType into a buffer. | ||
/// | ||
/// The DomainType will be consumed. | ||
fn encode<B: BufMut>(self, buf: &mut B) -> Result<(), Error>; | ||
/// This function replaces the Prost::Message encode() function for DomainTypes. | ||
fn encode<B: BufMut>(&self, buf: &mut B) -> Result<(), Error> { | ||
T::from(self.clone()) | ||
.encode(buf) | ||
.map_err(|e| Kind::EncodeMessage.context(e).into()) | ||
} | ||
|
||
/// Encodes the DomainType with a length-delimiter to a buffer. | ||
/// | ||
/// The DomainType will be consumed. | ||
/// An error will be returned if the buffer does not have sufficient capacity. | ||
fn encode_length_delimited<B: BufMut>(self, buf: &mut B) -> Result<(), Error>; | ||
/// | ||
/// This function replaces the Prost::Message encode_length_delimited() function for | ||
/// DomainTypes. | ||
fn encode_length_delimited<B: BufMut>(&self, buf: &mut B) -> Result<(), Error> { | ||
T::from(self.clone()) | ||
.encode_length_delimited(buf) | ||
.map_err(|e| Kind::EncodeMessage.context(e).into()) | ||
} | ||
|
||
/// Decodes an instance of the message from a buffer and then converts it into DomainType. | ||
/// | ||
/// The entire buffer will be consumed. | ||
fn decode<B: Buf>(buf: B) -> Result<Self, Error>; | ||
/// | ||
/// This function replaces the Prost::Message decode() function for DomainTypes. | ||
fn decode<B: Buf>(buf: B) -> Result<Self, Error> { | ||
T::decode(buf).map_or_else( | ||
|e| Err(Kind::DecodeMessage.context(e).into()), | ||
|t| Self::try_from(t).map_err(|e| Kind::TryIntoDomainType.context(e).into()), | ||
) | ||
} | ||
|
||
/// Decodes a length-delimited instance of the message from the buffer. | ||
/// | ||
/// The entire buffer will be consumed. | ||
fn decode_length_delimited<B: Buf>(buf: B) -> Result<Self, Error>; | ||
/// | ||
/// This function replaces the Prost::Message decode_length_delimited() function for | ||
/// DomainTypes. | ||
fn decode_length_delimited<B: Buf>(buf: B) -> Result<Self, Error> { | ||
T::decode_length_delimited(buf).map_or_else( | ||
|e| Err(Kind::DecodeMessage.context(e).into()), | ||
|t| Self::try_from(t).map_err(|e| Kind::TryIntoDomainType.context(e).into()), | ||
) | ||
} | ||
|
||
/// Returns the encoded length of the message without a length delimiter. | ||
/// | ||
/// The DomainType will be consumed. | ||
fn encoded_len(self) -> usize; | ||
/// This function replaces the Prost::Message encoded_len() function for DomainTypes. | ||
fn encoded_len(&self) -> usize { | ||
T::from(self.clone()).encoded_len() | ||
} | ||
|
||
/// Encodes the DomainType into a protobuf-encoded Vec<u8> | ||
fn encode_vec(&self) -> Result<Vec<u8>, Error> { | ||
let mut wire = Vec::with_capacity(self.encoded_len()); | ||
self.encode(&mut wire).map(|_| wire) | ||
} | ||
|
||
/// Decodes a protobuf-encoded instance of the message from a Vec<u8> and then converts it into | ||
/// DomainType. | ||
fn decode_vec(v: &[u8]) -> Result<Self, Error> { | ||
Self::decode(v) | ||
} | ||
|
||
/// Encodes the DomainType with a length-delimiter to a Vec<u8> protobuf-encoded message. | ||
fn encode_length_delimited_vec(&self) -> Result<Vec<u8>, Error> { | ||
let len = self.encoded_len(); | ||
let lenu64 = len.try_into().map_err(|e| Kind::EncodeMessage.context(e))?; | ||
let mut wire = Vec::with_capacity(len + encoded_len_varint(lenu64)); | ||
self.encode_length_delimited(&mut wire).map(|_| wire) | ||
} | ||
|
||
/// Decodes a protobuf-encoded instance of the message with a length-delimiter from a Vec<u8> | ||
/// and then converts it into DomainType. | ||
fn decode_length_delimited_vec(v: &[u8]) -> Result<Self, Error> { | ||
Self::decode_length_delimited(v) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely. I was also wondering if we need the whole proc_macro at all.
You can replace all these two lines that look like this
with:
impl DomainType<MyRawType> for ActualType {}
.As a consequence you can delete the whole macro code:
tendermint-rs/proto-derive/src/lib.rs
Lines 10 to 56 in e892067
And in fact the whole proto-derive crate can be deleted, without losing any functionality or any further consequences. I would argue whole code becomes easier to navigate as the macro creates some unnecessary indirection for a single line that explains what is happing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, true. Annotations seem cleaner to me, because I don't expect to look at the derive function. Serde does the same thing and it worked out pretty well there.
Also, we need to do some kind of JSON serialization for these domain types too and I didn't have time to get into the details of how we're going to do it. Maybe the derive macro helps, maybe not. Well, currently not really, it's just syntactic sugar.
I'll open this up to the team, I'm happy to do it either way, I just really wanted to do annotations. :)