serde-xml-rs icon indicating copy to clipboard operation
serde-xml-rs copied to clipboard

Unnecessary $value generated

Open Dgame opened this issue 6 years ago • 3 comments

Consider this piece of code:

use serde_derive::{Serialize, Deserialize};
use serde_json::Value;
use std::collections::HashMap;

#[derive(Serialize, Deserialize, Debug)]
#[serde(rename_all = "PascalCase")]
struct Person {
    name: String,
    #[serde(flatten)]
    extra: HashMap<String, Value>,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(rename_all = "PascalCase")]
struct Root {
    person: Person
}

fn main() {
    let xml = r#"<Root><Person><ArtId id="42">Männlich</ArtId><Name>Mustermann</Name><Vorname>Max</Vorname></Person></Root>"#;
    let root: Root = serde_xml_rs::from_str(xml).unwrap();
    dbg!(&root);
}

I would've expected the following output:

Root {
    person: Person {
        name: "Mustermann",
        extra: {
            "Vorname": String("Max"),
            "ArtId": Object(
                {
                    "$value": String(
                        "Male",
                    ),
                    "id": String(
                        "42",
                    ),
                },
            ),
        },
    },
}

but instead I get this:

Root {
    person: Person {
        name: "Mustermann",
        extra: {
            "Vorname": Object(
                {
                    "$value": String(
                        "Max",
                    ),
                },
            ),
            "ArtId": Object(
                {
                    "$value": String(
                        "Male",
                    ),
                    "id": String(
                        "42",
                    ),
                },
            ),
        },
    },
}

Although there are no attributes for the tag "Vorname" it's converted to an object/map with $value as key. Is that intended? It's not compatible with e.g. serde_json:

    let value: Value = serde_xml_rs::from_str(xml).unwrap();
    dbg!(&value);
    let root: Root = serde_json::from_value(value).unwrap();
    dbg!(&root);

it will print an error:

thread 'main' panicked at 'called Result::unwrap() on an Err value: Error("invalid type: map, expected a string", line: 0, column: 0)', src/libcore/result.rs:997:5

Dgame avatar Jun 04 '19 12:06 Dgame

First of all, serde(flatten) doesn't work in that way. It flattens the serialized representation not the deserialized one. See https://serde.rs/attr-flatten.html

Secondly, the problem with the last code block isn't in the Vorname tag but the Name tag. When deserializing the XML string to the serde_json::Value type, serde_xml_rs doesn't know that the contents of the Name tag should be mapped to a string and not an object. We see this with the debug output of value.

&value = Object(
    {
        "Person": Object(
            {
                "ArtId": Object(
                    {
                        "$value": String(
                            "Männlich",
                        ),
                        "id": String(
                            "42",
                        ),
                    },
                ),
                "Name": Object(
                    {
                        "$value": String(
                            "Mustermann",
                        ),
                    },
                ),
                "Vorname": Object(
                    {
                        "$value": String(
                            "Max",
                        ),
                    },
                ),
            },
        ),
    },
)

I do not believe that any of this reveals a bug in serde-xml-rs.

Please comment if I am missing something.

punkstarman avatar Jun 06 '19 09:06 punkstarman

My point is, since there are no attributes, it shouldn't be serialized as an object. Currently I'm using quickxml_to_serde to avoid this problem:

let value = quickxml_to_serde::xml_string_to_json(xml.to_string());
dbg!(&value);

will print

&value = Object(
    {
        "Root": Object(
            {
                "Person": Object(
                    {
                        "ArtId": Object(
                            {
                                "#text": String(
                                    "Männlich",
                                ),
                                "@id": Number(
                                    42,
                                ),
                            },
                        ),
                        "Name": String(
                            "Mustermann",
                        ),
                        "Vorname": String(
                            "Max",
                        ),
                    },
                ),
            },
        ),
    },
)

As you can see, Name and Vorname are correctly interpreted as String. If that is not intended by your lib, then closing this issue was correct. But I do believe that der result of quickxml_to_serde is what most users would expect.

Dgame avatar Jun 06 '19 12:06 Dgame

Sorry, I was thrown off by the mention of serde(flatten). In order to avoid further confusion, I will edit the title of this issue.

After further testing, it would appear that serde_xml_rs generates $value fields in cases where this would be avoidable.

punkstarman avatar Jun 11 '19 19:06 punkstarman