Skip to content

Fix and re-enable keyset pagination

Andreas Brandl requested to merge ab/keyset-two-step into master

What does this MR do?

This MR addresses what has been reverted with !21698 (merged). The revert disabled keyset pagination completely.

This change:

  1. re-enables keyset pagination for the project API, re-adds request specs
  2. removes an optimization for signalling the last page in a collection has been reached
  3. splits keyset pagination into two steps.

Optimization for signalling the last page, split pagination into two steps

We had an optimization in place that was already "peeking ahead" of the current page and was able to notice that the last page of a collection has been reached. As a result, the pagination header with a link to the next page was not rendered anymore (signalling the last page).

Now this makes it difficult to implement keyset pagination in a way that is compatible with the existing assumptions around pagination. In particular, "peeking ahead" means using a larger LIMIT (+1) to retrieve the first record from the next page (if any). This record has to be removed prior to delivering the payload for the current page.

This led to all kinds of problems, in particular because the implementation had to already execute the query to figure out the pagination information (for the next page).

In this MR, we split keyset pagination into two simple steps:

  1. Apply a LIMIT to the query (very similar to offset pagination, but only a LIMIT, no OFFSET)
  2. Once the query has been built fully and shortly before rendering JSON, use the last record of the page to render the pagination header information (a link to the next page).

We also remove the last page optimization here in order to make both pagination implementations compatible and make it easier (and less dangerous) to use both pagination strategies across various endpoints (which may have their own assumptions).

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by 🤖 GitLab Bot 🤖

Merge request reports