List::get_all() ignores query parameters

[phayes]

For example:

let mut list_subscriptions = stripe::ListSubscriptions::new();
list_subscriptions.created = Some(range);
list_subscriptions.limit = Some(100);
let subs = stripe::Subscription::list(&client, list_subscriptions)?.get_all(&client)?;

If there are more than 100 invoices returned in the first fetch, then get_all() returns all invoices stored in Stripe, ignoring the created range query param.

I'm trying to track down a root cause, but so far the List looks like this:

 &invoices = List {
    data: [...],
    has_more: true,
    total_count: None,
    url: "/v1/invoices",
}

The URL does not properly contain the list_subscriptions.created = Some(range); data. It doesn't have this data because it's simply a deserialized copy of the stripe response, which looks like:

{
  "object": "list",
  "url": "/v1/invoices",
  "has_more": false,
   "data: [...],
}  

So what we probably need is a new method called something like params which allows re-passing in params to re-prime the List with the params.

[phayes]

I think the solution probably looks something like this:

1. Add params field to List

#[derive(Debug, Deserialize, Serialize)]
pub struct List<T> {
    pub data: Vec<T>,
    pub has_more: bool,
    pub total_count: Option<u64>,
    pub url: String,

    #[serde(default)]
    pub params: Option<String>,
}

impl<T> List<T> {
    pub fn params<P: serde::Serialize>(mut self, params: &P) -> Result<Self, Error> {
        self.params = Some(serde_qs::to_string(params).map_err(Error::serialize)?);
        Ok(self)
    }
}

2. Add a get_list method to Client

    /// Make a `GET` http request with url query parameters, returning a List that can be paginated
    pub fn get_list<T: DeserializeOwned + Send + 'static, P: serde::Serialize + Clone>(
        &self,
        path: &str,
        params: P,
    ) -> Response<List<T>> {
        let resp = self.send_blocking(self.inner.get_query(path, params.clone()));

        match resp {
            Ok(list) => list.params(params),
            Err(e) => Err(e)
        }
    }

3. Use get_list where appropriate

impl Invoice {
    ...

    /// You can list all invoices, or list the invoices for a specific customer.
    ///
    /// The invoices are returned sorted by creation date, with the most recently created invoices appearing first.
    pub fn list(client: &Client, params: ListInvoices<'_>) -> Response<List<Invoice>> {
        client.get_list("/invoices", &params)
    }
}

4. Make sure that List::get_next uses the build-in params

impl<T: DeserializeOwned + Send + 'static> List<T> {
    /// Prefer `List::next` when possible
    pub fn get_next(client: &Client, url: &str, last_id: &str) -> Response<List<T>> {
        if url.starts_with("/v1/") {
            // TODO: Maybe parse the URL?  Perhaps `List` should always parse its `url` field.
            let mut url = url.trim_start_matches("/v1/").to_string();
            if url.contains('?') {
                url.push_str(&format!("&starting_after={}", last_id));
            } else {
                url.push_str(&format!("?starting_after={}", last_id));
            }

            if let Some(params) = self.params {
                url.push_str(&format!("&{}", params));
            }
            client.get(&url)
        } else {
            err(Error::Unsupported("URL for fetching additional data uses different API version"))
        }
    }
}

I'd like some thoughts from the maintainers on this approach.

[phayes]

This is partially fixed in #194 . ~~It still needs to be solved for async. ~~ Async fix now in the PR as well.

[phayes]

Async fix also applied to PR #194 .