`List::get_all()` ignores query parameters
phayes opened this issue · 3 comments
phayes commented
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 commented
I think the solution probably looks something like this:
- 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)
}
}
- Add a
get_list
method toClient
/// 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)
}
}
- 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", ¶ms)
}
}
- 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.