Support deserializing types nested within enums

[maghoff]

Hi! Thanks for this library! I am pleasantly surprised by everything it lets me do.

However, I have found a hitch. When deserializing an enum, it seems that the type information is somehow lost, and everything falls back to being Strings. See this test:

#[test]
fn deserialize_i32_in_struct_in_enum() {
    #[derive(Deserialize, Debug, PartialEq, Eq)]
    struct ItemStruct {
        field: i32
    }

    #[derive(Deserialize, Debug, PartialEq, Eq)]
    #[serde(tag = "tag")]
    enum Test {
        Item(ItemStruct)
    };

    let result = Test::Item(ItemStruct{ field: 42 });

    assert_eq!(serde_urlencoded::from_str("tag=Item&field=42"), Ok(result));
}

It currently fails with the run-time error invalid type: string "42", expected i32:

---- deserialize_i32_in_struct_in_enum stdout ----
        thread 'deserialize_i32_in_struct_in_enum' panicked at 'assertion failed: `(left == right)` (left: `Err(Error { err: "invalid type: string \"42\", expected i32" })`, right: `Ok(Item(ItemStruct { field: 42 }))`)', tests/test_deserialize.rs:79
note: Run with `RUST_BACKTRACE=1` for a backtrace.

I have committed this test along with a similar succeeding one with String to my fork. (See also the entire patch)

[nox]

I don't understand why you would expect this to work. Can you explain why you think it should?

[maghoff]

Thanks for getting back to me :) I'll present the background more detailed and then try to clarify how I expect this to work.

I have tried illustrating how and why I expect this to work by writing a failing test and a succeeding test, which I linked in the original description. The only difference between the two tests is the type of the field embedded in ItemStruct.

When the type is String, the test succeeds. When the type is i32, the test fails in deserialization with an error indicating that deserialization requires field field to be of a string type. The serialized data in this case is 42 which could be interpreted as an i32 or a String equally well, but the deserializer demands that it must be deserialized as a string.

This surprises me; I expect the deserializer to be able to interpret data as an integer when I attempt to deserialize into an integer field.

When the situation is less elaborate, notably not including an enum, the difference between an i32 and a String is indeed handled in deserializing: Playground link. This leads me to expect that the difference between an i32 and a String should also be handled in more elaborate situations.

This is a high level understanding of the deserializer, there may well exist a perfectly reasonable limitation that makes this unreasonable, but which I do not understand. I do not understand why it is not allowed to deserialize into an i32 that is nested inside a struct inside an enum while it is allowed to deserialize into an i32 that is nested directly inside a struct, and it is allowed to deserialize into a String that is nested inside a struct inside an enum.

Does this clearly explain my expectations?

[nox]

Where is your patch exactly?

[maghoff]

I have made a fork with changes visible in the patch at the link I posted in the initial description: https://github.com/nox/serde_urlencoded/compare/master...maghoff:master

This is not a patch in the sense that it is a proposed change that will fix my problem.

Instead it contains two unit tests to describe my problem: One that fails and a similar one that succeeds. This issue is to request that the necessary changes be made to support both the unit tests.

[nox]

Oh I see. I don't think this crate will support that ever because that would mean buffering things in case the tag didn't come first, and I don't want this crate to have to do any buffering. Did you check whether serde_qs support this use case already?

[maghoff]

Fair enough, buffering is a bummer! :) (Although I do think this would be similar for all other deserializers that support this)

I did try serde_qs without success at the time I initially posted this issue, but it's been a while, so I might try again tonight.

[maghoff]

As a side note, I would be happy if this were supported only in the cases where the tag does indeed come first. But I would understand if you would not want to offer that kind of an interface.

[nox]

That's a reasonable request (well, the original issue is reasonable too), I'll think about it.

[maghoff]

I have now been able to try out serde_qs again. It seems to support deserializing tagged enums, but not as the top-level type. This adds some extra syntax to the query string. So, in serde_qs, the feature I am looking for is kind of within reach, but not quite there :slightly_smiling_face:

Elaborating a bit, what I have asked for in this issue looks like this: tag=Item&field=42, while what I can get with serde_qs looks like this: item[tag]=Item&item[val][field]=42. A lot less elegant, I think.

Relevant unit test in serde_qs: https://github.com/samscott89/serde_qs/blob/93d0f47e2aa0fc31a196726163001e31f4824722/tests/test_deserialize.rs#L282-L319

This comment just to inform you of the state of serde_qs with regards to enums.

[nox]

Cool!

[maghoff]

The issue has not been addressed, it seems like this has been closed in error.

Please reopen or state clearly the intention to dismiss.

[nox]

Oh I see. I don't think this crate will support that ever because that would mean buffering things in case the tag didn't come first, and I don't want this crate to have to do any buffering.

[maghoff]

Re-reading this, I can't shake the feeling that a central point has not come across, so I will take the liberty to make it again, more clearly:

serde_urlencoded currently supports deserializing tagged enums. For example tag=Item&field=value can be sucessfully deserialized to the value Item { field: "value" } of the following type:

enum Test {
    Item {
        field: String
    }
}

You can see this in action on the playground, functioning as desired: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8acdbb53b4721cbd12147246b0c37ecc

This requires no bufring, and is a very useful feature!

---

My request here is about completing this support such that it also works when the nested type is i32. Playground link to failing example.

[nox]

Oh, ok.

[caolan]

I ran into this problem myself today. To anyone else arriving here looking for a solution, I have been able to use the flatten workaround described in the serde_qs docs to deserialize nested enum fields using plain serde_urlencoded too.

It involves adding a serde field attribute with a custom deserializer for only that field. It's probably not a great solution if you need to do this frequently in your code as it will add clutter, but for occasional use it's fine.