-
Notifications
You must be signed in to change notification settings - Fork 72
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
feat(serde_with): make BytesOrString
adjustable
#793
base: master
Are you sure you want to change the base?
Changes from all 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 | ||||
---|---|---|---|---|---|---|
|
@@ -80,6 +80,59 @@ create_format!( | |||||
Unpadded | ||||||
); | ||||||
|
||||||
/// When serializing a value of a type, | ||||||
/// that allows multiple types during deserialization, | ||||||
/// prefer a specific type. | ||||||
pub trait TypePreference: SerializeAs<[u8]> {} | ||||||
|
||||||
/// Prefer serializing it as ASCII string. | ||||||
pub struct PreferAsciiString; | ||||||
|
||||||
impl TypePreference for PreferAsciiString {} | ||||||
|
||||||
impl SerializeAs<[u8]> for PreferAsciiString { | ||||||
fn serialize_as<S>(source: &[u8], serializer: S) -> Result<S::Ok, S::Error> | ||||||
where | ||||||
S: Serializer, | ||||||
{ | ||||||
match core::str::from_utf8(source) { | ||||||
Ok(text) if text.is_ascii() => serializer.serialize_str(text), | ||||||
_ => serializer.serialize_bytes(source), | ||||||
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. As discussed, these
Suggested change
|
||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
/// Prefer serializing it as bytes. | ||||||
pub struct PreferBytes; | ||||||
|
||||||
impl TypePreference for PreferBytes {} | ||||||
|
||||||
impl SerializeAs<[u8]> for PreferBytes { | ||||||
fn serialize_as<S>(source: &[u8], serializer: S) -> Result<S::Ok, S::Error> | ||||||
where | ||||||
S: Serializer, | ||||||
{ | ||||||
serializer.serialize_bytes(source) | ||||||
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.
Suggested change
|
||||||
} | ||||||
} | ||||||
|
||||||
/// Prefer serializing it as string. | ||||||
pub struct PreferString; | ||||||
|
||||||
impl TypePreference for PreferString {} | ||||||
|
||||||
impl SerializeAs<[u8]> for PreferString { | ||||||
fn serialize_as<S>(source: &[u8], serializer: S) -> Result<S::Ok, S::Error> | ||||||
where | ||||||
S: Serializer, | ||||||
{ | ||||||
match core::str::from_utf8(source) { | ||||||
Ok(text) => serializer.serialize_str(text), | ||||||
_ => serializer.serialize_bytes(source), | ||||||
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.
Suggested change
|
||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
/// Specify how lenient the deserialization process should be | ||||||
/// | ||||||
/// Formats which make use of this trait should specify how it affects the deserialization behavior. | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ use core::{ | |
use expect_test::expect; | ||
use serde::{Deserialize, Serialize}; | ||
use serde_with::{ | ||
formats::{CommaSeparator, Flexible, Strict}, | ||
formats::{CommaSeparator, Flexible, PreferAsciiString, PreferString, Strict}, | ||
serde_as, BoolFromInt, BytesOrString, DisplayFromStr, IfIsHumanReadable, Map, | ||
NoneAsEmptyString, OneOrMany, Same, Seq, StringWithSeparator, | ||
}; | ||
|
@@ -477,7 +477,7 @@ fn test_none_as_empty_string() { | |
} | ||
|
||
#[test] | ||
fn test_bytes_or_string() { | ||
fn test_bytes_or_string_as_bytes() { | ||
#[serde_as] | ||
#[derive(Debug, Serialize, Deserialize, PartialEq)] | ||
struct S(#[serde_as(as = "BytesOrString")] Vec<u8>); | ||
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. Can you please add a test for an explicit |
||
|
@@ -491,8 +491,64 @@ fn test_bytes_or_string() { | |
3 | ||
]"#]], | ||
); | ||
check_deserialization(S(vec![72, 101, 108, 108, 111]), r#""Hello""#); | ||
check_deserialization(S(vec![70, 111, 111, 98, 97, 114]), r#""Foobar""#); | ||
} | ||
|
||
#[test] | ||
fn test_bytes_or_string_as_string() { | ||
#[serde_as] | ||
#[derive(Debug, Serialize, Deserialize, PartialEq)] | ||
struct S(#[serde_as(as = "BytesOrString<PreferString>")] Vec<u8>); | ||
|
||
is_equal(S(vec![72, 101, 108, 108, 111]), expect![[r#""Hello""#]]); | ||
|
||
check_deserialization(S(vec![0xf0, 0x9f, 0xa6, 0xa6]), r#""🦦""#); | ||
is_equal(S(vec![0xf0, 0x9f, 0xa6, 0xa6]), expect![[r#""🦦""#]]); | ||
|
||
is_equal( | ||
S(vec![0, 255]), | ||
expect![[r#" | ||
[ | ||
0, | ||
255 | ||
]"#]], | ||
); | ||
check_deserialization(S(vec![87, 111, 114, 108, 100]), r#""World""#); | ||
} | ||
|
||
#[test] | ||
fn test_bytes_or_string_as_ascii_string() { | ||
#[serde_as] | ||
#[derive(Debug, Serialize, Deserialize, PartialEq)] | ||
struct S(#[serde_as(as = "BytesOrString<PreferAsciiString>")] Vec<u8>); | ||
|
||
is_equal(S(vec![72, 101, 108, 108, 111]), expect![[r#""Hello""#]]); | ||
|
||
check_deserialization(S(vec![0xf0, 0x9f, 0xa6, 0xa6]), r#""🦦""#); | ||
is_equal( | ||
S(vec![0xf0, 0x9f, 0xa6, 0xa6]), | ||
expect![[r#" | ||
[ | ||
240, | ||
159, | ||
166, | ||
166 | ||
]"#]], | ||
); | ||
|
||
is_equal( | ||
S(vec![0, 255]), | ||
expect![[r#" | ||
[ | ||
0, | ||
255 | ||
]"#]], | ||
); | ||
check_deserialization(S(vec![87, 111, 114, 108, 100]), r#""World""#); | ||
} | ||
|
||
#[test] | ||
fn test_bytes_or_string_nested() { | ||
#[serde_as] | ||
#[derive(Debug, Serialize, Deserialize, PartialEq)] | ||
struct SVec(#[serde_as(as = "Vec<BytesOrString>")] Vec<Vec<u8>>); | ||
|
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.
That's neat to just reuse the
SerializeAs
trait for the behavior.Does the trait need to be fully public? We can't make it private, since it appears in public bounds, but seal all implementations. Similar to
serde_with/serde_with/src/base64.rs
Lines 160 to 168 in bc20634
I would like to keep the serialization and deserialization behavior matching if possible. For that a fully public trait feels unnecessary. Having ASCII and unicode strings supported feels sufficient for now. If it turns out that more freedom is necessary, it can be unsealed later.