From ab8d5193a3fda921c6a68530d8dea98e481cd4cd Mon Sep 17 00:00:00 2001 From: Mingun Date: Fri, 20 Oct 2023 21:51:07 +0500 Subject: [PATCH] Always deserialize simpleTypes (for example, attribute values) as Some(...) This also matches behavior for elements, where `` is always will be Some(...) because tag is present --- Changelog.md | 3 ++ src/de/simple_type.rs | 35 +++++++++++++++---- tests/serde-issues.rs | 80 +++++++++++++++++++++++++++---------------- 3 files changed, 81 insertions(+), 37 deletions(-) diff --git a/Changelog.md b/Changelog.md index 8de5fa4e..f2eb68b8 100644 --- a/Changelog.md +++ b/Changelog.md @@ -30,6 +30,8 @@ serde >= 1.0.181 unit structs, unit variants). `123` 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 @@ -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 diff --git a/src/de/simple_type.rs b/src/de/simple_type.rs index 32d7585d..aa3be1e5 100644 --- a/src/de/simple_type.rs +++ b/src/de/simple_type.rs @@ -481,7 +481,32 @@ impl<'de, 'a> SeqAccess<'de> for ListIter<'de, 'a> { /// - attribute values (`<... ...="value" ...>`) /// - mixed text / CDATA content (`<...>text`) /// +/// 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 @@ -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(self, visitor: V) -> Result @@ -1225,7 +1246,7 @@ mod tests { err!(utf8, borrowed_bytes: Bytes = "<escaped 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" => ()); @@ -1311,7 +1332,7 @@ mod tests { unsupported!(borrowed_bytes: Bytes = "<escaped 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" => ()); diff --git a/tests/serde-issues.rs b/tests/serde-issues.rs index b1be5963..a3e95b9e 100644 --- a/tests/serde-issues.rs +++ b/tests/serde-issues.rs @@ -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>, @@ -26,31 +26,41 @@ 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#""#; assert_eq!( to_string(&OptionalAttributes { a: None, b: None }).unwrap(), - r#""# + xml ); assert_eq!( - to_string(&OptionalAttributes { + from_str::(xml).unwrap(), + OptionalAttributes { a: Some(""), - b: Some("") - }) - .unwrap(), - r#""# - ); - assert_eq!( - to_string(&OptionalAttributes { - a: Some("a"), - b: Some("b") - }) - .unwrap(), - r#""# + b: None + } ); + + let value = OptionalAttributes { + a: Some(""), + b: Some(""), + }; + let xml = r#""#; + assert_eq!(to_string(&value).unwrap(), xml); + assert_eq!(from_str::(xml).unwrap(), value); + + let value = OptionalAttributes { + a: Some("a"), + b: Some("b"), + }; + let xml = r#""#; + assert_eq!(to_string(&value).unwrap(), xml); + assert_eq!(from_str::(xml).unwrap(), value); } #[test] fn elements() { - #[derive(Serialize, Debug, PartialEq)] + #[derive(Debug, Deserialize, Serialize, PartialEq)] struct OptionalElements { a: Option<&'static str>, @@ -58,26 +68,36 @@ mod issue252 { b: Option<&'static str>, } + // Writing `` for a `None` we reflects serde_json behavior which also + // writes `a: null` for `None`, and reflect they deserialization asymmetry + let xml = ""; assert_eq!( to_string(&OptionalElements { a: None, b: None }).unwrap(), - r#""# + xml ); assert_eq!( - to_string(&OptionalElements { + from_str::(xml).unwrap(), + OptionalElements { a: Some(""), - b: Some("") - }) - .unwrap(), - r#""# - ); - assert_eq!( - to_string(&OptionalElements { - a: Some("a"), - b: Some("b") - }) - .unwrap(), - r#"ab"# + b: None + } ); + + let value = OptionalElements { + a: Some(""), + b: Some(""), + }; + let xml = ""; + assert_eq!(to_string(&value).unwrap(), xml); + assert_eq!(from_str::(xml).unwrap(), value); + + let value = OptionalElements { + a: Some("a"), + b: Some("b"), + }; + let xml = "ab"; + assert_eq!(to_string(&value).unwrap(), xml); + assert_eq!(from_str::(xml).unwrap(), value); } }