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

Serde support needs to be revisited #185

Closed
dbcfd opened this issue Jan 9, 2020 · 2 comments · Fixed by #237
Closed

Serde support needs to be revisited #185

dbcfd opened this issue Jan 9, 2020 · 2 comments · Fixed by #237

Comments

@dbcfd
Copy link

dbcfd commented Jan 9, 2020

PR #183 fixes some of this behavior with the existing interactions of serde and reader, but overall, the implementation needs to be revisited.

Here's the issues I've encountered so far

  • Inner enums without rename will fail to deserialize
  • Inner enums with rename attribute will fail to deserialize unless the rename is $value
  • flatten attribute does not work
  • Vector of enums will fail to deserialize, regardless of rename usage
  • Nested containers without rename of $value will fail to deserialize

I've been trying to make things work "better" in my PR, but the rest of these issues would take a significant overhaul of the code.

@dbcfd dbcfd changed the title Serde supports needs to be revisited Serde support needs to be revisited Jan 9, 2020
@tafia
Copy link
Owner

tafia commented Feb 1, 2020

Thank you for your issue, your PR and the time you have spent on it!

Serde is a hard topic, in particular for xmls. I agree the current state is not ideal and I am very open to revisit it. Before merging it, I already had many attempts. I recognize this is not perfect but it gets the job done for a lot of situations so I decided to release it.

Also sorry for the late answer, I really appreciate the time you have spent on it.

Regarding the PR, I have started to review it but there are several things I need to get done before. Also I think we could easily split it into several PR if possible because you add lot of new implementations, some rather trivial some more complicated.

@ralpha
Copy link

ralpha commented May 13, 2020

I'm checking out this crate as an alternative to serde-xml-rs as it is indeed faster.
(about 2.6 times faster then serde-xml-rs in my tests, so nice 😃 )
But I have come upon this issue.

Nested containers without rename of $value will fail to deserialize

I'm parsing all not captured tags values in a HashMap like this:

use serde_json::Value;
pub struct MyItem {
    pub id: i32,
    // ... Some more values here

    #[serde(flatten)]
    pub unknown: HashMap<String, Value>,
}

So this will deserialize all the tags in there respective values, and everything else in unknown.

But now I'm looking at the output and it does not save the values from the body of the tag. (even when I add the #[serde(rename = "$value")] line).
This is probably because the values are nested.

example output:

[
    (
        "not_listed_tag",
        Object(
            {}, // <-- Here is the problem. This should be the value of $value, aka the body
        ),
    ),
]

Or at least the value should be there somehow, like this for example:

Object(
    {
        "$value": "the body of the tag",
    }
)

serde-xml-rs does this correctly so fix should be in there somewhere ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants