Loading
Encode an offset in policy store pagination cursors
What does this MR do and why?
Implements task 1 of https://gitlab.com/gitlab-org/gitlab/-/work_items/629392.
Third in a stack. Targets !256235 (closed), which targets !255370 (merged). Review that order.
The cursor encoded a page number, so the rows a cursor pointed at depended on the page size used for the previous request. A client that changed first mid-walk skipped or repeated policies — which is why the field description told callers to keep first constant. That caveat is now gone, because cursors carry a row offset instead.
ListServiceacceptsoffset:alongside thepage:it already took. The REST endpoint renders page-numberLinkheaders frompayload[:page]and cannot express a cursor this way, so both vocabularies have to coexist. An explicit offset wins over a page.pageis only reported on the page path. An arbitrary offset does not land on a page boundary, so reporting the page it falls inside would mislead anything building next/previous links from it —offset: 1, per_page: 2is neither page 1 nor page 2. It isnilwhen the caller paginated by offset, so misuse fails loudly rather than quietly.- The resolver advances by the page size the store actually used (
payload[:offset] + payload[:per_page]), not the one requested. The store clampsper_page; advancing by a larger requestedfirstwould step straight over rows it declined to serve. These agree today only because the field'smax_page_size, the schema default and the port'sMAX_PER_PAGEare all 100 — raising any one of them would otherwise have silently skipped rows. - The ceiling is the port's existing
MAX_OFFSETrather than a page count, so the reject-don't-clamp rule from the parent MR now readsoffset <= MAX_OFFSET. A cursor decoding to0is now valid — it is the first row, where one MR ago it was a malformed page number.
Reviewer notes
page_numberdivides by the requestedper_page, not the one the port reports. Anidslookup bypasses pagination and reportsper_page: 0, so dividing by the port's value raisesZeroDivisionError. There is a comment on it; theidsspecs cover it.- Not addressed here: an offset cursor is position-based, so policies created or deleted mid-walk shift the window and rows can be skipped or repeated. That was equally true of page cursors and is equally undocumented — worth a follow-up on the work item rather than this MR.
References
- Issue: https://gitlab.com/gitlab-org/gitlab/-/work_items/629392
- Parent MR: !256235 (closed)
- Pattern followed:
Resolvers::Wikis::WikiPagesResolver
Screenshots or screen recordings
N/A — GraphQL resolver, service and schema description only, no UI.
How to set up and validate locally
- Enable the Policy Store experiment: the instance-level
policy_store_experiment_enabledapplication setting plus the organization setting. - Create at least three policies in an organization.
- In GraphiQL, request
policies(first: 1)and notepageInfo.endCursor. - Resume with
policies(first: 2, after: "<that cursor>")and confirm you get the second and third policies — no skip, no repeat. The old page-number cursor read this as "page 2 of size 2" and skipped one. - Confirm
after: "MA=="(offset 0) serves the first page, and a cursor aboveMAX_OFFSETreturnsInvalid pagination cursor. - Confirm the REST endpoint is unaffected:
GET /api/v4/organizations/:id/security/policy_store?page=2&per_page=1still returns correctX-PageandLinkheaders. - Run:
bundle exec rspec ee/spec/graphql/resolvers/govern/policies_resolver_spec.rbbundle exec rspec ee/spec/services/security/security_orchestration_policies/policy_store/list_service_spec.rbbundle exec rspec ee/spec/requests/api/govern/policies_spec.rb
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist.