Support for multi-valued select elements

[trombonehero]

Thanks for Rocket's request guard mechanism: it's shockingly elegant. However, when inputting form data, there doesn't seem to be any support for <select multiple>...</select> elements. I'd like to be able to write something like:

#[derive(FromForm)]
struct Foo {
    names: Vec<String>
}

so that the form data foo=a&foo=b&foo=c turns into a Vec of "a", "b" and "c". Currently, if I try to use the above struct, I see:

the trait `rocket::request::FromFormValue<'_>` is not implemented for `std::vec::Vec<std::string::String>`

and if I change Vec to Option, I only see the last value of foo (in this case, "c"): the other values are silently dropped.

[MrWinstead]

Trombonehero (or whomever is interested),

Do you see yourself needing any other collection type other than just a vector as a container in your structure for the first implementation? If std::Vec is good enough, would the following behavior make sense?

#[derive(FromForm)]
struct Foo {
    names: Vec<String>
}

Non-rust-like Pseudocode:

method parse_vector 
    returns Vector of type T
    input form_variable_name of type String, raw_form of type String
    encounters error condition T::CannotParseFromString
  Vector to_return;
  MultiKeyHashMap form_keys_values = form_split(raw_form);

  foreach formitem in form_keys_values.get_all_values(form_variable_name)
    to_return.push(T::fromstring(formitem));
  end foreach

  return to_return;
end method

Which means that if there were no elements specified or if the specified elements were all empty, we'd give the user an empty vector. If any of the elements can't be converted into the specified type, we'll report a parsing error.

[trombonehero]

Hi,

That does, indeed, sound like what I'm thinking of. I have been using HashSet as well, for situations in which I don't care about ordering but do want to compute differences (e.g., "roles that we should add to a user" and "roles that we should remove from a user"), but that should be straightforward enough to get from a Vec.

I think the difficulty with this issue is that it would require a generalization of FromFormValue that accumulates values rather than mapping them. Perhaps "difficulty" isn't the right word, but it would be an API change.

For reference, my current approach (which I'd like to replace with #[derive(FromForm)]) is:

    fn from_form_items(form_items: &mut request::FormItems<'f>) -> Result<Self, Self::Error> {
        let mut update = UserUpdate {
            name: String::new(),
            email: String::new(),
            phone: None,
            roles: HashSet::new(),
        };

        for (k, v) in form_items {
            let key: &str = &*k;
            let value = v.into();

            match key {
                "name" => update.name = value,
                "email" => update.email = value,
                "phone" => update.phone = Some(value),
                "roles" => { update.roles.insert(value); },
                _ => {
                    return Err(Error::InvalidData(format!["invalid form data name: '{}'", key]));
                },
            }
        }

        Ok(update)
    }

[mehcode]

I think it'd be important to not constrain this to Vec and allow anything that implements FromIterator.

[MrWinstead]

@trombonehero, That makes sense.

@mehcode, I was wondering what the generic trait was for Rust in this case. I think there's a question then for whether we should support deeper nested types than a two-level generic. I'm not good enough at rust to design some sort of recursive generic unroller where each step of the generic has either a FromString or FromIterator trait, but I could see a future implementation of that.

Oh, and perhaps for the maintainers, the current API for this doesn't allow a struct implementing the FromFormValue to store state between multiple calls to from_form_value. This would be super-easy to implement if I could change that API to allow for a reference to self. Thus, I'm going to add that in for the first iteration; just let me know if you disagree and I'll change it.

[MrWinstead]

Branch 205 in my fork of the project has an initial implementation for vectors of items which have the FromStr trait.

In order for this to work, I added an argument in the trait which allows for a reference to the instance which is generated in codegen/src/decorators/derive_form.rs around line 180. The problem with this is now there are tons of unused variable warnings for the default types which don't use the backreference. The alternative to this would have been to modify the arm generation and inspect the field type to see if it is something which has the Extend trait and create a separate code path which passes in the backreference and also instantiates the instance.

Where my experience falls short is in how to use the syntax crate and inspect a node Path's type's existing traits. If someone knew how to do that, I'd put it on line 169 in codegen/src/decorators/derive_form.rs.

I'll let these comments simmer a little before submitting a PR.

[nCrazed]

Could the current behaviour be changed to aggregate all values under the same name while adjusting the method signature from:

fn from_form_value(form_value: &'v RawStr) -> Result<Self, Self::Error>

to:

fn from_form_value(form_value: Vec<&'v RawStr>) -> Result<Self, Self::Error>

This would have the added benefit of allowing a single value field implementation to verify that there's actually only one incoming value.

[nCrazed]

This can then further be expanded to something like:

fn from_form_value(form_value: Vec<&'v Field>) -> Result<Self, Self::Error>

where Field contains the full input field name (e.g. name, name[0], name[0][title], name[0][first] etc.) and the vector contains all the input fields matching the root name.

So given a request to ?simple=one&names=John&names=Jane&other[0]=x&other[2]=y and form:

#[derive(FromForm)]
struct Foo {
    simple: String,
    names: Vec<String>,
    other: HashMap<usize,String>,
}

the implementations would look something like this:

  fn from_form_value(form_value: Vec<&'v Field>) -> Result<String, Self::Error> {
    Ok(form_values.first().value)
  }

  fn from_form_value(form_value: Vec<&'v Field>) -> Result<Vec<String>, Self::Error> {
    Ok(form_values.iter().map(|x|x.value).collect())
  }

  fn from_form_value(form_value: Vec<&'v Field>) -> Result<HashMap<usize,String>, Self::Error> {
    let mut map: HashMap<usize,String> = HashMap::new();
    for field in form_value: {
        let (key, tail) = field.leftmost_key(); // allow arbitrary depth
        map.insert(key, field.value);
    }
    Ok(map)
  }

yielding:

Foo {
    simple: "one",
    names: ["John","Jane"],
    other: [0 => "x", 1 => "y"],
}

[SergioBenitez]

Unfortunately, we won't be able to get this in for 0.4: pushing to 0.5.

[mjanda]

Hello,
any news on this issue? Just want to know if I should wait for this or implement some workaround (probably collecting values into single comma separated field and parsing it from there - fortunately, my use case is trival, just a bunch of checkboxes with numerical values).

Thank you

[jebrosen]

Nobody has really worked on an integrated solution as far as I know - you have to implement FromForm manually or work around it another way like you described.

[mjanda]

@jebrosen Thank you for suggesting FromForm implementation. Somehow I managed to hack together my own derive that works with Vec.

[brownjohnf]

@mjanda I'm looking for something similar; would you mind sharing your solution?

[TotalKrill]

Since this seems to have a work-around, and is on the list for 0.5 milestones, could this maybe be pushed to 0.6? I would very much like to see a release of 0.5 expedited.

[trombonehero]

Hooray! Wonderful...