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

Serialization of enums adds extra tags #346

Closed
Storyyeller opened this issue Dec 17, 2021 · 6 comments · Fixed by #490
Closed

Serialization of enums adds extra tags #346

Storyyeller opened this issue Dec 17, 2021 · 6 comments · Fixed by #490
Labels
bug serde Issues related to mapping from Rust types to XML

Comments

@Storyyeller
Copy link

Storyyeller commented Dec 17, 2021

Currently, we have some types using quick_xml and serde to parse XML, which works well. However, we want to also serialize them back to XML, and the XML that quick_xml produces differs from the original (expected) format, and I can't figure out any way to fix it.

Our type definitions look like this:

#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct Contactinfo {
    pub bugurl: String,
}

#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct Include {
    pub name: String,
    #[serde(default)]
    pub groups: String,
}

#[derive(Clone, Debug, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
pub enum ManifestChild {
    Include(Include),
    Contactinfo(Contactinfo),

    #[serde(other)]
    Unknown,
}

#[derive(Clone, Debug, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
pub struct Manifest {
    #[serde(default, rename = "$value")]
    pub children: Vec<ManifestChild>,
}

And we can parse XML with it just fine:

let text = r#"
<?xml version="1.0" encoding="UTF-8"?>
<manifest>
  <include name="bar"/>
  <contactinfo bugurl="foo"/>
  <include name="hello" groups="world"/>
</manifest>"#;

    let result = quick_xml::de::from_str(text);
    let m: Manifest = result.unwrap();

    let s = quick_xml::se::to_string(&m).unwrap();
    println!("{}", s);

However, the output XML is in the wrong format, wrapping all the enum contents in an extra tag. How can we get rid of this and make it serialize in the same way as the input xml? Also, why is Manifest capitalized in the output when it isn't in the input? It seems like serialization ignores our #[serde(rename_all = "kebab-case")] annotations!

Here is what the output looks like with quick_xml 0.23alpha3:

<Manifest><include><Include name="bar"/></include><contactinfo><Contactinfo bugurl="foo"/></contactinfo><include><Include name="hello" groups="world"/></include></Manifest>
@spikespaz
Copy link

Really need this to be fixed. Currently this is the best-working serializer for XML but it still doesn't cut the cake.

@Storyyeller
Copy link
Author

So far the best workaround I came up with involved writing a combination of four wrapper types with hundreds of lines of boilerplate to change the way it serializes.

@spikespaz
Copy link

@Storyyeller Would you mind sharing that code? Could really use it and want to save time.

@spikespaz
Copy link

spikespaz commented Jan 10, 2022

Seems like there was already a pill request that fixes this but it is made on a tree that is 199 commits ago.... Makes me a little annoyed because the reason the repo owner denied the PR is that they didn't "like" it.

#205

@Mingun Mingun added bug serde Issues related to mapping from Rust types to XML labels May 21, 2022
@Mingun
Copy link
Collaborator

Mingun commented May 25, 2022

It seems like serialization ignores our #[serde(rename_all = "kebab-case")] annotations!

The reason, why Manifest in XML was capitalized in that that rename_all annotation applied only to struct fields, but not to struct name, and root tag inferred from the top-level struct name that you trying to serialize.

Since 0.20.0 you can choose root tag name using with_root(name) constructor.

The other questions still need an investigation, I'll probably look at them after finishing proper deserializer support.

Mingun added a commit to Mingun/quick-xml that referenced this issue Oct 1, 2022
Fixes tafia#252 - ok
Fixes tafia#280 - ok
Fixes tafia#287 - ok
Fixes tafia#343 - ok
Fixes tafia#346 - not ok
Fixes tafia#361 - ok
Partially addresses tafia#368
Fixes tafia#429 - ok
Fixes tafia#430 - ok
Mingun added a commit to Mingun/quick-xml that referenced this issue Oct 1, 2022
Fixes tafia#252
Fixes tafia#280
Fixes tafia#287
Fixes tafia#343
Fixes tafia#346
Fixes tafia#361
Partially addresses tafia#368
Fixes tafia#429
Fixes tafia#430
@Mingun
Copy link
Collaborator

Mingun commented Oct 1, 2022

@Storyyeller, when #490 will be merged, the following code will do what you want:

#[test]
fn issue346() {
    #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
    pub struct Contactinfo {
        #[serde(rename = "@bugurl")]
        pub bugurl: String,
    }

    #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
    pub struct Include {
        #[serde(rename = "@name")]
        pub name: String,

        #[serde(rename = "@groups", default)]
        #[serde(skip_serializing_if = "str::is_empty")]
        pub groups: String,
    }

    #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
    #[serde(rename_all = "kebab-case")]
    pub enum ManifestChild {
        Include(Include),
        Contactinfo(Contactinfo),

        #[serde(other)]
        Unknown,
    }

    #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
    #[serde(rename_all = "kebab-case")]
    #[serde(rename = "manifest")]
    pub struct Manifest {
        #[serde(default, rename = "$value")]
        pub children: Vec<ManifestChild>,
    }

    let text = r#"
<?xml version="1.0" encoding="UTF-8"?>
<manifest>
  <include name="bar"/>
  <contactinfo bugurl="foo"/>
  <include name="hello" groups="world"/>
</manifest>"#;

    let result = quick_xml::de::from_str(text);
    let m: Manifest = result.unwrap();
    assert_eq!(m, Manifest {
        children: vec![
            ManifestChild::Include(Include { name: "bar".to_string(), groups: "".to_string() }),
            ManifestChild::Contactinfo(Contactinfo { bugurl: "foo".to_string() }),
            ManifestChild::Include(Include { name: "hello".to_string(), groups: "world".to_string() }),
        ]
    });

    assert_eq!(
        quick_xml::se::to_string(&m).unwrap(),
        "\
        <manifest>\
            <include name=\"bar\"/>\
            <contactinfo bugurl=\"foo\"/>\
            <include name=\"hello\" groups=\"world\"/>\
        </manifest>"
    )
}

Mingun added a commit to Mingun/quick-xml that referenced this issue Oct 24, 2022
Fixes tafia#252
Fixes tafia#280
Fixes tafia#287
Fixes tafia#343
Fixes tafia#346
Fixes tafia#361
Partially addresses tafia#368
Fixes tafia#429
Fixes tafia#430

Fixes all tests
Mingun added a commit to Mingun/quick-xml that referenced this issue Oct 25, 2022
Fixes tafia#252
Fixes tafia#280
Fixes tafia#287
Fixes tafia#343
Fixes tafia#346
Fixes tafia#361
Partially addresses tafia#368
Fixes tafia#429
Fixes tafia#430

Fixes all tests
Mingun added a commit to Mingun/quick-xml that referenced this issue Oct 26, 2022
Fixes tafia#252
Fixes tafia#280
Fixes tafia#287
Fixes tafia#343
Fixes tafia#346
Fixes tafia#361
Partially addresses tafia#368
Fixes tafia#429
Fixes tafia#430

Fixes all tests

Co-authored-by: Daniel Alley <dalley@redhat.com>
JOSEPHGILBY pushed a commit to JOSEPHGILBY/quick-xml that referenced this issue Nov 5, 2022
Fixes tafia#252
Fixes tafia#280
Fixes tafia#287
Fixes tafia#343
Fixes tafia#346
Fixes tafia#361
Partially addresses tafia#368
Fixes tafia#429
Fixes tafia#430

Fixes all tests

Co-authored-by: Daniel Alley <dalley@redhat.com>
JOSEPHGILBY pushed a commit to JOSEPHGILBY/quick-xml that referenced this issue Nov 5, 2022
Fixes tafia#252
Fixes tafia#280
Fixes tafia#287
Fixes tafia#343
Fixes tafia#346
Fixes tafia#361
Partially addresses tafia#368
Fixes tafia#429
Fixes tafia#430

Fixes all tests

Co-authored-by: Daniel Alley <dalley@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug serde Issues related to mapping from Rust types to XML
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants