jplatte / serde_html_form

Rust crate for (de-)serialization of `application/x-www-form-urlencoded` data.
MIT License
23 stars 3 forks source link

Feature Request: Support for deserializing `Option<Vec<T>>` #11

Closed goshleg closed 9 months ago

goshleg commented 9 months ago

Hello!

First off, I'd like to express my gratitude for your work on this crate; it's been immensely useful in my project!

I'm reaching out to discuss a potential enhancement regarding the deserialization functionality. While the library excels at handling Vec<String>, I've noticed that it doesn't entirely support deserializing form fields into Option<Vec<String>>.

Current behavior: Deserialization directly into Vec works seamlessly. However, I encounter issues when a form field is optional. Trying to deserialize a single value like value=1 into Option<Vec<String>> results in Error("invalid type: string \"1\", expected a sequence"), which suggests an unsupported case for single-item arrays. Nonetheless, if the form submits multiple items such as value=1&value=2, the deserialization proceeds correctly, yielding Some(Vec<String>) with the expected vector.

fn main() {
    #[derive(Debug, PartialEq, Deserialize)]
    struct Form {
        #[serde(default)]
        value: Option<Vec<String>>,
    }

    // Works well
    let form: Form = serde_html_form::from_str("value=1&value=2").unwrap();
    assert_eq!(
        form,
        Form {
            value: Some(vec![1.to_string(), 2.to_string()])
        }
    );

    // Fails
    let form: Result<Form, serde_html_form::de::Error> =
        serde_html_form::from_str("value=1");
    assert!(form.is_err());
}

Desired Behavior: Ideally, the library should allow an optional form field to be deserialized into Option<Vec<String>>. When a field is present, it should handle any number of values (whether it is just one or multiple), encapsulating them into a Some(Vec<String>).

Workaround: I have written custom deserialization logic, and here all assertion works:

fn deserialize_vec_or_single<'de, T, D>(
    deserializer: D,
) -> Result<Option<Vec<T>>, D::Error>
where
    T: Deserialize<'de>,
    D: Deserializer<'de>,
{
    // Visitor to handle either a single element or a sequence of elements.
    struct VecOrSingleVisitor<T> {
        marker: PhantomData<fn() -> T>,
    }

    impl<'de, T> Visitor<'de> for VecOrSingleVisitor<T>
    where
        T: Deserialize<'de>,
    {
        type Value = Option<Vec<T>>;

        fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
            formatter.write_str("a single value or a list of values")
        }

        // Handle a single value.
        fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
        where
            E: Error,
        {
            T::deserialize(value.into_deserializer()).map(|v| Some(vec![v]))
        }

        // Handle empty field which can represent an empty vector.
        fn visit_none<E>(self) -> Result<Self::Value, E>
        where
            E: Error,
        {
            Ok(Some(Vec::new()))
        }

        // Handle a present field which will attempt to deserialize as a Vec<T>.
        fn visit_some<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
        where
            D: Deserializer<'de>,
        {
            Deserialize::deserialize(deserializer).map(Some)
        }

        // Handle a sequence of values.
        fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
        where
            A: SeqAccess<'de>,
        {
            let mut vec = Vec::new();

            while let Some(elem) = seq.next_element()? {
                vec.push(elem);
            }

            Ok(Some(vec))
        }
    }

    // Now we inform the deserializer that we are going to use our custom visitor.
    deserializer.deserialize_any(VecOrSingleVisitor {
        marker: PhantomData,
    })
}

fn main() {
    #[derive(Debug, PartialEq, Deserialize)]
    struct Form {
        #[serde(default, deserialize_with = "deserialize_vec_or_single")]
        value: Option<Vec<String>>,
    }

    let form: Form = serde_html_form::from_str("value=1&value=2").unwrap();
    assert_eq!(
        form,
        Form {
            value: Some(vec![1.to_string(), 2.to_string()])
        }
    );

    let form: Form = serde_html_form::from_str("value=1").unwrap();
    assert_eq!(
        form,
        Form {
            value: Some(vec![1.to_string()])
        }
    );

    let form: Form = serde_html_form::from_str("").unwrap();
    assert_eq!(form, Form { value: None });
}

Benefits: Implementing this feature would simplify dealing with optional multi-value form fields, avoiding the need for cumbersome boilerplate or manual parsing. It would be great, if single values can also be interpreted as a vector with one element, adding native support for this feature would enable more expressive and robust form handling in applications using the serde_html_form crate without resorting to custom implementations.

I am also open to further discussion and willing to collaborate on realizing this feature.

Thank you for your time and for considering my suggestion.

jplatte commented 9 months ago

Hi, as the documentation says you can use Vec<T> in combination with #[serde(default)], which doesn't require much boilerplate at all. Still, this is something I would like to support if possible, as I previously wrote here.

goshleg commented 9 months ago

@jplatte, thanks for getting back to me so fast!

I see your point about keeping the parser stable and preventing new issues. It makes complete sense. The Vec<T> and #[serde(default)] combo seems to be a neat workaround to manage optional vectors without cluttering things up too much, given the library's existing functionality.

It's great to notice the conversation around this topic in the thread. A bit more clarity in the documentation could go a long way right now.

jplatte commented 9 months ago

Fixed and fix released in serde_html_form 0.2.4. There's also a follow-up issue #13 that I'd like to get feedback on.