iddm / serde-aux

An auxiliary serde library providing helpful functions for serialisation and deserialisation for containers, struct fields and others.
MIT License
152 stars 26 forks source link

deserialize_struct_case_insensitive + serde::rename #8

Closed stanislav-tkach closed 3 years ago

stanislav-tkach commented 3 years ago

I have encountered strange behavior when deserialize_struct_case_insensitive is used in conjunction with serde::rename. Here is a slightly modified example taken from the deserialize_struct_case_insensitive documentation:

use serde::Deserialize;
use serde_aux::prelude::*;

#[derive(Deserialize, Debug)]
struct AnotherStruct {
    #[serde(rename = "fieldname")] 
    field_name: String,
}
#[derive(Deserialize, Debug)]
struct MyStruct {
    #[serde(deserialize_with = "deserialize_struct_case_insensitive")]
    another_struct: AnotherStruct,
}

fn main() {
    let s = r#"{ "another_struct": { "FiElDnAmE": "Test example" } }"#;
    let a: MyStruct = serde_json::from_str(s).unwrap();
    assert_eq!(a.another_struct.field_name, "Test example");
}

In this case everything works as expected, but if #[serde(rename = "fieldname")] is changed to #[serde(rename = "fieldName")] then I get the following error:

thread 'main' panicked at 'called Result::unwrap() on an Err value: Error("missing field fieldName", line: 1, column: 53)', src/main.rs:17:47

iddm commented 3 years ago

I am afraid these two are incompatible with each other. Serde creates some "aliases" field for each field in a struct for parsing, so it will go through the list of aliases trying to parse the field. I suppose this behaviour is either completely or at least partially disabled by specifying #[serde(deserialize_with)] because the deserializer, in this case, is different and it can do anything with a field. What it does now with #[serde(deserialize_with = "deserialize_struct_case_insensitive")] is just before parsing it changes the field name to lowercase and so it doesn't matter what the original name was, it is changed to lower case and then parsed, so the problem you encounter is valid. Unfortunately, there is nothing we can do - it will always be either one deserialiser or another, but it can't be two at the same time.

I might have a look again, perhaps, something has changed since then, but I do not guarantee a solution.

P.S. After a minute of thinking, there might be a solution which is simply not to lowercase a string but to compare it with ignoring the case. Let me try this out.

iddm commented 3 years ago

In the serde-aux it is impossible to do, unfortunately. I pass a lower-cased string to serde so that it handles all the stuff after. So if I change a field name there and pass it to serde which is instructed to parse a field with a rename, it simply won't work.

I was working on a patch to serde to allow that actually, but had some issues with that and stopped for some time. I may have another try there, but again, no guarantees. From the last investigation of serde, it simply was coded in a way that was making case-insensitive parsing either very hard or impossible.

stanislav-tkach commented 3 years ago

Thanks for the quick response! I expected that this issue couldn't be fixed outside of serde. Perhaps documentation can mention this pitfall?

iddm commented 3 years ago

Thanks for the quick response! I expected that this issue couldn't be fixed outside of serde. Perhaps documentation can mention this pitfall?

Sure thing. Thanks for the suggestion.

https://github.com/vityafx/serde-aux/commit/bd5960282a01190a71420244d683b2c2d600d135

iddm commented 3 years ago

Released a version with a documentation note: https://docs.rs/serde-aux/1.0.1/serde_aux/container_attributes/fn.deserialize_struct_case_insensitive.html#notes

If I find out I can do anything else to make this work, I'll comment on this issue and reopen it.