Skip to content
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

Design a good API for non-readable [de]serialization in serde_test #12

Closed
Marwes opened this issue Oct 13, 2017 · 4 comments · Fixed by serde-rs/serde#1084
Closed

Design a good API for non-readable [de]serialization in serde_test #12

Marwes opened this issue Oct 13, 2017 · 4 comments · Fixed by serde-rs/serde#1084
Labels
enhancement New feature or request

Comments

@Marwes
Copy link
Contributor

Marwes commented Oct 13, 2017

serde-rs/serde#1044 (comment)

The current API for creating serializers and deserializers aren't ideal and is therefore hidden. It is needed though so that proper tests can be created for crates which provide non-readable serializations.

cc serde-rs/serde#1044

@dtolnay dtolnay added the enhancement New feature or request label Oct 15, 2017
@dtolnay
Copy link
Member

dtolnay commented Oct 15, 2017

Here is an idea that uses the same mechanism as serde-rs/serde#1044 (comment) but is more nicely extensible.

struct Readable<T: ?Sized>(T);
struct Compact<T: ?Sized>(T);

trait Configure {
    fn readable(&self) -> &Readable<Self> { /* ... */ }
    fn compact(&self) -> &Compact<Self> { /* ... */ }
}

impl<T: ?Sized> Configure for T {}

impl<T: ?Sized> Serialize for Readable<T> where T: Serialize { /* ... */ }
impl<T: ?Sized> Serialize for Compact<T> where T: Serialize { /* ... */ }
impl<'de, T> Deserialize<'de> for Readable<T> where T: Deserialize<'de> { /* ... */ }
impl<'de, T> Deserialize<'de> for Compact<T> where T: Deserialize<'de> { /* ... */ }
use serde_test::Configure;

let s = /* ... */;

// In human readable formats, field `timestamp` serializes as ISO 8601.
assert_tokens(s.readable(), &[
    Token::Struct { name: "S", len: 1 },
    Token::Str("timestamp"),
    Token::String("2017-09-26T21:49:25Z"),
    Token::StructEnd,
]);

// In binary formats, field `timestamp` serializes in compact byte form.
assert_tokens(s.compact(), &[
    Token::Struct { name: "S", len: 1 },
    Token::Str("timestamp"),
    Token::Bytes(&[226, 152, 131]),
    Token::StructEnd,
]);

@dtolnay
Copy link
Member

dtolnay commented Oct 22, 2017

I ended up doing the thing where it would panic in serde_test if the type tries to check whether the format is human-readable. I don't think this will be disruptive, but we can revisit if I am mistaken.

@Marwes
Copy link
Contributor Author

Marwes commented Nov 1, 2017

@dtolnay Looked into this a bit but found that #12 and serde-rs/serde#1044 (comment) needs a lot of boilerplate to implement. This is because overriding is_human_readable requires all methods on Serializer, SerializeSeq, Deserializer, MapAccess as well as their associated types need to be overridden which is trivial in some cases but not all as the inner [de]serializer needs to re-wrapped withReadable or Compact wrapper in some methods. macros alleviate this somewhat but we are still talking of several hundred lines of boiler plate.

Am I missing something here? While it would arguably give a nice API the implementation takes a lot of effort for something this small and worse, it doesn't scale (easily) if more configuration were to be added later.

Start of implementation

https://gist.github.com/Marwes/c09c42899d01362d682daa67fecd34a7

@dtolnay
Copy link
Member

dtolnay commented Nov 1, 2017

Generally I have tried to optimize for nice APIs over effort of implementation. I have not found several hundred lines of boilerplate to be a maintenance burden. "Serializer adapters" where one serializer wraps another one is a really powerful concept -- serde-ignored and erased-serde and serde-encrypted-value are other examples of this pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

2 participants