Skip to content

Commit

Permalink
Always deserialize simpleTypes (for example, attribute values) as Som…
Browse files Browse the repository at this point in the history
…e(...)

This also matches behavior for elements, where `<a/>` is always will be Some(...) because tag is present
  • Loading branch information
Mingun committed Oct 20, 2023
1 parent 569ac22 commit ab8d519
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 37 deletions.
3 changes: 3 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ serde >= 1.0.181
unit structs, unit variants). `<int>123<something-else/></int>` is no longer valid
content. Previously all data after `123` up to closing tag would be silently skipped.
- [#567]: Fixed incorrect deserialization of vectors of enums from sequences of tags.
- [#671]: Fixed deserialization of empty `simpleType`s (for example, attributes) into
`Option` fields: now they are always deserialized as `Some("")`.

### Misc Changes

Expand Down Expand Up @@ -58,6 +60,7 @@ serde >= 1.0.181
[#661]: https://github.com/tafia/quick-xml/pull/661
[#662]: https://github.com/tafia/quick-xml/pull/662
[#665]: https://github.com/tafia/quick-xml/pull/665
[#671]: https://github.com/tafia/quick-xml/issues/671


## 0.30.0 -- 2023-07-23
Expand Down
35 changes: 28 additions & 7 deletions src/de/simple_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,32 @@ impl<'de, 'a> SeqAccess<'de> for ListIter<'de, 'a> {
/// - attribute values (`<... ...="value" ...>`)
/// - mixed text / CDATA content (`<...>text<![CDATA[cdata]]></...>`)
///
/// This deserializer processes items as following:
/// - numbers are parsed from a text content using [`FromStr`];
/// - booleans converted from the text according to the XML [specification]:
/// - `"true"` and `"1"` converted to `true`;
/// - `"false"` and `"0"` converted to `false`;
/// - strings returned as is;
/// - characters also returned as strings. If string contain more than one character
/// or empty, it is responsibility of a type to return an error;
/// - `Option` always deserialized as `Some` using the same deserializer.
/// If attribute or text content is missed, then the deserializer even wouldn't
/// be used, so if it is used, then the value should be;
/// - units (`()`) and unit structs always deserialized successfully;
/// - newtype structs forwards deserialization to the inner type using the same
/// deserializer;
/// - sequences, tuples and tuple structs are deserialized as `xs:list`s. Only
/// sequences of primitive types is possible to deserialize this way and they
/// should be delimited by a space (` `, `\t`, `\r`, or `\n`);
/// - structs and maps returns [`DeError::Unsupported`];
/// - enums:
/// - unit variants: just return `()`;
/// - all other variants returns [`DeError::Unsupported`];
/// - identifiers are deserialized as strings.
///
/// [simple types]: https://www.w3.org/TR/xmlschema11-1/#Simple_Type_Definition
/// [`FromStr`]: std::str::FromStr
/// [specification]: https://www.w3.org/TR/xmlschema11-2/#boolean
pub struct SimpleTypeDeserializer<'de, 'a> {
/// - In case of attribute contains escaped attribute value
/// - In case of text contains unescaped text value
Expand Down Expand Up @@ -642,11 +667,7 @@ impl<'de, 'a> Deserializer<'de> for SimpleTypeDeserializer<'de, 'a> {
where
V: Visitor<'de>,
{
if self.content.is_empty() {
visitor.visit_none()
} else {
visitor.visit_some(self)
}
visitor.visit_some(self)
}

fn deserialize_unit<V>(self, visitor: V) -> Result<V::Value, Self::Error>
Expand Down Expand Up @@ -1225,7 +1246,7 @@ mod tests {
err!(utf8, borrowed_bytes: Bytes = "&lt;escaped&#32;string"
=> Unsupported("binary data content is not supported by XML format"));

simple!(utf8, option_none: Option<&str> = "" => None);
simple!(utf8, option_none: Option<&str> = "" => Some(""));
simple!(utf8, option_some: Option<&str> = "non-escaped string" => Some("non-escaped string"));

simple_only!(utf8, unit: () = "any data" => ());
Expand Down Expand Up @@ -1311,7 +1332,7 @@ mod tests {
unsupported!(borrowed_bytes: Bytes = "&lt;escaped&#32;string"
=> "binary data content is not supported by XML format");

utf16!(option_none: Option<()> = "" => None);
utf16!(option_none: Option<()> = "" => Some(()));
utf16!(option_some: Option<()> = "any data" => Some(()));

utf16!(unit: () = "any data" => ());
Expand Down
80 changes: 50 additions & 30 deletions tests/serde-issues.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mod issue252 {

#[test]
fn attributes() {
#[derive(Serialize, Debug, PartialEq)]
#[derive(Debug, Deserialize, Serialize, PartialEq)]
struct OptionalAttributes {
#[serde(rename = "@a")]
a: Option<&'static str>,
Expand All @@ -26,58 +26,78 @@ mod issue252 {
b: Option<&'static str>,
}

// Writing `a=""` for a `None` we reflects serde_json behavior which also
// writes `a: null` for `None`, and reflect they deserialization asymmetry
let xml = r#"<OptionalAttributes a=""/>"#;
assert_eq!(
to_string(&OptionalAttributes { a: None, b: None }).unwrap(),
r#"<OptionalAttributes a=""/>"#
xml
);
assert_eq!(
to_string(&OptionalAttributes {
from_str::<OptionalAttributes>(xml).unwrap(),
OptionalAttributes {
a: Some(""),
b: Some("")
})
.unwrap(),
r#"<OptionalAttributes a="" b=""/>"#
);
assert_eq!(
to_string(&OptionalAttributes {
a: Some("a"),
b: Some("b")
})
.unwrap(),
r#"<OptionalAttributes a="a" b="b"/>"#
b: None
}
);

let value = OptionalAttributes {
a: Some(""),
b: Some(""),
};
let xml = r#"<OptionalAttributes a="" b=""/>"#;
assert_eq!(to_string(&value).unwrap(), xml);
assert_eq!(from_str::<OptionalAttributes>(xml).unwrap(), value);

let value = OptionalAttributes {
a: Some("a"),
b: Some("b"),
};
let xml = r#"<OptionalAttributes a="a" b="b"/>"#;
assert_eq!(to_string(&value).unwrap(), xml);
assert_eq!(from_str::<OptionalAttributes>(xml).unwrap(), value);
}

#[test]
fn elements() {
#[derive(Serialize, Debug, PartialEq)]
#[derive(Debug, Deserialize, Serialize, PartialEq)]
struct OptionalElements {
a: Option<&'static str>,

#[serde(skip_serializing_if = "Option::is_none")]
b: Option<&'static str>,
}

// Writing `<a/>` for a `None` we reflects serde_json behavior which also
// writes `a: null` for `None`, and reflect they deserialization asymmetry
let xml = "<OptionalElements><a/></OptionalElements>";
assert_eq!(
to_string(&OptionalElements { a: None, b: None }).unwrap(),
r#"<OptionalElements><a/></OptionalElements>"#
xml
);
assert_eq!(
to_string(&OptionalElements {
from_str::<OptionalElements>(xml).unwrap(),
OptionalElements {
a: Some(""),
b: Some("")
})
.unwrap(),
r#"<OptionalElements><a/><b/></OptionalElements>"#
);
assert_eq!(
to_string(&OptionalElements {
a: Some("a"),
b: Some("b")
})
.unwrap(),
r#"<OptionalElements><a>a</a><b>b</b></OptionalElements>"#
b: None
}
);

let value = OptionalElements {
a: Some(""),
b: Some(""),
};
let xml = "<OptionalElements><a/><b/></OptionalElements>";
assert_eq!(to_string(&value).unwrap(), xml);
assert_eq!(from_str::<OptionalElements>(xml).unwrap(), value);

let value = OptionalElements {
a: Some("a"),
b: Some("b"),
};
let xml = "<OptionalElements><a>a</a><b>b</b></OptionalElements>";
assert_eq!(to_string(&value).unwrap(), xml);
assert_eq!(from_str::<OptionalElements>(xml).unwrap(), value);
}
}

Expand Down

0 comments on commit ab8d519

Please sign in to comment.