-
-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
maxId in DefaultPaginationParams should be able to be set externally #698
Comments
@SuperSonicHub1 Pagination is a high-level abstraction for link headers in Mastodon API, and I agree that it's useless in some cases where users want to set min_id, max_id, or limit manually. Therefore, I'm also offering a method named
This seems like a bug. I will fix it in another PR |
I ideally would like to not have to reimplement I'll propose a solution. Here's what the code currently looks like: The main issue here is that after our first request, we're passing both our original parameters and the new ones given to us by Mastodon in the Link header. Here's how I would solve this. async next(params?: Params): Promise<IteratorResult<Result>> {
if (this.nextUrl == undefined) {
return { done: true, value: undefined };
}
const response: Response<Result> = await this.http.request({
method: 'get',
// if no params specified, use link header
url: params ? this.initialUrl : this.nextUrl,
params: params ?? this.nextParams,
});
this.nextUrl = this.pluckNext(response.headers?.link as string);
this.nextParams = {};
return {
done: false,
value: response.data,
};
} Now, after our initial sending of parameters, If you want, I can make a PR. |
@SuperSonicHub1 Sorry for my bad understanding! I haven't realised that using both user-provided params and the Link header causes a duplication of query parameters. The snippet you've shown looks neat. I'm willing to review it if you send me a PR 👍 |
This stops the duplication of query parameters in our request URL, as Mastodon already gives us the correct parameters in the `Link` header. In some cases, a mixture of parameters from `params` and the `Link` URL could cause the iterator to get caught in an infinite loop. Fixes neet#698.
…rameters Now that query parameter duplication has been fixed, users should be able to safely use `maxId` and `sinceId` in their code. Fixes neet#698.
This stops the duplication of query parameters in our request URL, as Mastodon already gives us the correct parameters in the `Link` header. In some cases, a mixture of parameters from `params` and the `Link` URL could cause the iterator to get caught in an infinite loop. Fixes neet#698.
…rameters Now that query parameter duplication has been fixed, users should be able to safely use `maxId` and `sinceId` in their code. Fixes neet#698.
## [4.6.9](v4.6.8...v4.6.9) (2022-11-26) ### Bug Fixes * Actually send user-specified headers in `HttpAxiosImpl` ([b602fe4](b602fe4)), closes [#697](#697) * Encode URI components in serializer ([8a2be08](8a2be08)) * **fetch:** Normalize header to be lowercase ([f7fce11](f7fce11)) * Fix failing test in [#702](#702) ([a2a4c26](a2a4c26)) * read next link from response ([68c71bd](68c71bd)) * Set `Paginator.nextParams` to an empty object after first request ([4ce57a6](4ce57a6)), closes [#698](#698) * Unmark `DefaultPaginationParams.{maxId, sinceId}` as internal parameters ([257359e](257359e)), closes [#698](#698) * Update Notification.type types ([#706](#706)) ([50c5c5c](50c5c5c))
🎉 This issue has been resolved in version 4.6.9 🎉 The release is available on: Your semantic-release bot 📦🚀 |
masto.js/src/repositories/repository.ts
Lines 1 to 10 in ebdaee8
There is no way to reverse the pagination direction of
/api/v1/accounts/:id/statuses
, making the ability to set themax_id
parameter necessary to create a resumable application that downloads all of someone's statuses.If an end user does manually set
maxId
, theirAsyncIterable
will get caught in an endless loop due to Masto setting themax_id
parameter twice (in fact, everything's repeated):https://pony.social/api/v1/accounts/106500315995032015/statuses?exclude_replies=false&exclude_replies=false&limit=100&limit=100&max_id=109323073972972141&max_id=109332395072165340&pinned=false&pinned=false
. Seeing asmax_id
isn't an internal detail of the Mastodon API (it's actually older thanmin_id
), users should be able to set it without running into issues like I did.Also,
maxId
doens't properly handle anull
input; instead of not including themax_id
parameter, it sendsmax_id=null
, which Mastodon doesn't understand.The text was updated successfully, but these errors were encountered: