Query param of type: array with items.type: string leads to to_string() and unsatisfied trait on &&Vec<String>

[smndtrl]

While working with Scaleways API spec for Accounts, I encountered a array of strings in the URL. To me it is not clear how exactly it should work. The doc suggests it ends up as string in the URL &project_ids=[string], so I assume they want it joined with a , and not &project_ids=1&project_ids=2.

From Swaggers docs on OpenAPI serialization, the default style is style: form and explode: true aka &project_ids=1&project_ids=2. That would, I think, require a change here

progenitor/progenitor-impl/src/method.rs

Line 843 in 91564cf

OperationParameterKind::Query(required) => {

to account for Vec<_> and iterate + push for that case.

Any thoughts?

[MichielHegemans]

I ran into the same issue and I've been working on it a bit. The Typify https://github.com/oxidecomputer/typify?tab=readme-ov-file#arrays crate mentions that a type: array in the JSON schema can be parsed as 3 different types:

• Vec
• HashSet
• (T, T, ...) (Depending on size)

Which are all doable, but need their own code paths.

Next issue is that it is determining the capacity of the Vec for the query to make based on the amount of params the function gets, however if we input an exploded version into the query vec this size will potentially be larger, or empty if the vec is empty. This is will not affect the functionality though, but it's a minor performance impact.

If we push a non-exploded version (1,2,3) the size is not an issue.

Progenitor also does not seem to grab the explode information from openapiv3::Parameter::Query, but it is available and can easily be added to the OperationParameter struct to be processed.

[antifuchs]

It looks to me like the implementation of the code fragment you linked is wrong for any kind of array type: Vec doesn't have a ToString implementation for any T.

But even there was a .to_string() impl for vectors, the way progenitor tries to do that (by pushing api_parameter_name=<value.to_string()> to the query string) makes it impossible to pass even the RFC6570/3.2.8 kind of api parameter in a vector (where it should encode api_parameter_name=foo&api_parameter_name=bar): You could try to make a newtype that has a ToString impl, which could create a query string fragment, interspersing the values with &api_parameter_name=... which then would get escaped by the reqwest query builder.

I think this might need a new trait for converting to/modifying a query string? Maybe something like this sketch (not tested, of course!):

trait AddToQuery {
  fn add_to_query(self, query: &mut Vec<(String, String)>, name: &str);
}

impl<T: ToString> AddToQuery for T {
  fn add_to_query(self, query: &mut Vec<(String, String)>, name: &str) {
    query.push(name, self.to_string());
  }
}

impl<T: ToString, I: IntoIterator<Item=T>> AddToQuery for T {
  fn add_to_query(self, query: &mut Vec<(String, String)>, name: &str) {
    for item in self.into_iter() {
      query.push(name, item.to_string());
    }
  }
}

...or something.

[iganev]

Any fix or workaround for this yet?

[mat813]

I am trying to interop with netbox, and its openapi has a lot of parameters that are of type array, like this:

  /api/dcim/devices/:
    get:
      operationId: dcim_devices_list
      description: Get a list of device objects.
      parameters:
      - in: query
        name: asset_tag
        schema:
          type: array
          items:
            type: string
        explode: true
        style: form

The generated code is:

        if let Some(v) = &asset_tag {
            query.push(("asset_tag", v.to_string()));
        }

Which gives the error:

8533 |             query.push(("asset_tag", v.to_string()));
     |                                        ^^^^^^^^^ method cannot be called on `&&Vec<String>` due to unsatisfied trait bounds

With type: form and explode: true (which are both the default) it should probably be something like this:

        if let Some(v) = asset_tag {
            for value in v.into_iter() {
                query.push(("asset_tag", value.to_string()));
            }
        }

And if explode had been false, it should probably have been something like:

        if let Some(v) = asset_tag {
            query.push(("asset_tag", v.into_iter().map(|v| v.to_string()).collect::<Vec<String>>().join(",")));
        }

[ahl]

I did some exploration on fixed for this issue:

Background

The type associated with a query parameter has some (many?) constraints that might be most effectively enforced when we do the initial parsing, but we'll make do with processing them here.

Within the OpenAPI spec, the type may be one of these classes of entities:

1. A scalar that can be rendered as a string (e.g. a string or number)
2. An array of scalars
3. An object whose property values are scalars

This leaves a variety of our internal types to consider. For the first category, we can see if the type has a Display impl. The second category includes Vec, Set, and arrays (i.e. of fixed length). The third includes Objects (whose properties are all scalars) and Maps (whose values are scalars).

In addition, we need to consider enums whose variants might correspond to one of the three categories. There are 4 serializations of enums: external, internal, adjacent and untagged. Sufficiently constrained enums of any of these variants could be valid:

• External: only simple or newtype variants where the type is a scalar
• Internal: where all properties of each variants' type are scalars
• Adjacent: only simple or newtype variants where the type is a scalar
• Untagged: each variant must be one of the classes above

In addition (!), OpenAPI imposes still more constraints according to the values of the form and explode properties:

style	explode	scalar	array	object
form	false	ok	ok	ok
form	true	ok	ok	ok
spaceDelimited	false	-	ok	-
spaceDelimited	true	-	-	-
pipeDelimited	false	-	ok	-
pipeDelimited	true	-	-	-
deepObject	false	-	-	-
deepObject	true	-	-	ok

Attempt 1: unpicking types

My first idea was much as described in #724: add explicit support depending on the type of the parameter. This is fine, but narrow. In particular, as I tried to get the Subsonic API in #994 working, I noticed this category of query construction:

        "parameters": [
          {
            "name": "id",
            "in": "query",
            "required": true,
            "schema": {
              "oneOf": [
                {
                  "type": "string"
                },
                { 
                  "type": "array",
                  "items": {
                    "type": "string"
                  }
                }
              ]
            }
          },  

This type translates into (or something equivalent):

#[serde(untagged)]
enum OperationId {
    Variant0(String),
    Variant2(Vec<String>),
}

With this approach we'd need to pick the type apart by generating a match, etc. Side note: this construction is pointless. Literally the encoding of a single id and multiple ids is identical!

This also made me realize that a thorough approach would require us to pick our way through other types such as newtype wrappers. Which led to the next approach:

Attempt 2: custom trait

The second idea I had (though didn't explore) was to have a QueryParam trait that we could derive on types used in query position. Not terrible; but logistically a little tricky. But this led me to thinking of using a similar trait, already derived on all our types!

Attempt 3: serde

serde::Serialize already knows how to access all the members of a type, so could we makde a custom serde::Serializer that properly serializes types? We could even use different implementations to improve progenitor support for the OpenAPI style/explode table.

I prototyped this approach which included some nifty use of marker types:

pub trait Style {}

pub mod style {
    use crate::Style;

    pub enum Form {}
    impl Style for Form {}
    pub enum SpaceDelimited {}
    impl Style for SpaceDelimited {}
    pub enum PipeDelimited {}
    impl Style for PipeDelimited {}
    pub enum DeepObject {}
    impl Style for DeepObject {}
}

pub trait Explode {}
pub mod explode {
    use crate::Explode;

    pub enum True {}
    impl Explode for True {}
    pub enum False {}
    impl Explode for False {}
}

I thought that it would be neat to have progenitor generate query parameter code like this:

    .query(QuerySerialize::<style::Form, explode::True>::new("query-param-name", query_param))

This worked fine for style=form and explode=true, but I ran into trouble with other encodings from that table. In particular, the reqwest::RequestBuilder::query() relies on url::Url::query_pairs_mut which in turn uses form_urlencoded::Serializer. This assumes a particular encoding, and can't--for example--produce the unescaped characters needed e.g. for the style=form and explode=false encoding.

Attempt 4: Serializer just for style=form / explode=true

Since handling other encodings with reqwest is going to be tricky, I think it would suffice to put a subset of the prototype above into the progenitor-client crate and then to generate the code as above.