Replace totalRows pagination with cursors on the list endpoints#1131
Open
alistair3149 wants to merge 13 commits into
Open
Replace totalRows pagination with cursors on the list endpoints#1131alistair3149 wants to merge 13 commits into
alistair3149 wants to merge 13 commits into
Conversation
GET /schemas, /layouts, and /mappings filtered unreadable rows out of their pages but reported an unfiltered namespace count, so totalRows - visible leaked the number of read-restricted Schemas, Layouts, and Mappings. The endpoints now paginate with an opaque cursor over readable rows only: limit plus an optional cursor in, items plus nextCursor out. Restricted rows are skipped exactly like nonexistent ones, take no page space, and never surface in the cursor, so nothing about them is inferable from the pagination. The persistence lookups page the namespace by keyset (page_id) in batches and re-check read permission per title, so a request costs the rows it serves rather than the whole namespace. Offset pagination over a read-filtered list would re-scan from the start on every page, which stops scaling once a deployment accumulates thousands of Schemas. The admin tables map CdxTable's offset-based server pagination onto cursors with a per-row-offset cursor history, run the indeterminate "X-Y of many" pagination until the end is known, then report the exact count (the indeterminate next-button heuristic misses a listing that ends exactly on a page boundary). The schema picker's full enumeration follows nextCursor instead of counting rows. Fixes #1062 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The empty-page caveat narrated mechanism, and the no-total fact already lives in the Permissions section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alistair3149
force-pushed
the
fix/1062-totalrows-count-oracle
branch
from
July 22, 2026 20:44
bf75b00 to
03aafe9
Compare
alistair3149
marked this pull request as ready for review
July 22, 2026 20:57
buildPage skips a list row whose summary fails to load without spending page space, so a full page stays full and a one-at-a-time walk never yields an empty page mid-listing. Pin this on the Mapping endpoint with an XML-imported Mapping missing its schemas key, and correct the trait doc: the cursor is the last served item's page ID, so a malformed row after it is re-scanned and skipped by the next page. Review-fix tests for PR #1131. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Schema, Layout and Mapping name lookups page their namespace in 100-row keyset batches, but no test exceeded a single batch. Drive each past 100 rows via a new bulk bare-page insert helper and assert every row is yielded once in page-ID order. The Schema case also denies the row sitting on the batch boundary, showing the keyset anchor advances past unreadable rows so later batches still arrive. Review-fix tests for PR #1131. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Schema, Layout and Mapping list pages leave totalRows undefined on a full page carrying a non-null cursor, keeping CdxTable indeterminate so Next stays enabled; only the determinate null-cursor flip was covered. Pin the indeterminate case for all three pages, adding a first spec for LayoutsPage (which also covers its determinate flip). Review-fix tests for PR #1131. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The denied row at the boundary does not distinguish anchor-advance variants (an anchor that only advances on yields re-scans one row and produces identical output); what it uniquely pins is that the drain's continue decision counts fetched rows, not yielded ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both testUnloadableMappingDoesNotConsumePageSpace and testWalkingPastAnUnloadableMappingNeverYieldsAnEmptyPage assert result sets that are byte-identical to the listing you get when the imported Beta page does not exist at all. So a silently no-op import — a future import-time validation, an export-format change that makes the str_replace miss, a slot/model change — would leave only Alpha and Gamma, keep both tests green, and never run the loader's skip branch they exist to pin. Guard the fixture's premise: assert the two substitutions took effect on the dump, then assert the import created a distinct Beta page sitting between Alpha and Gamma in page-ID order. Also drop the no-op markPageTableAsUsed() call — the class has no setUp truncation and MW 1.43 resets changed tables via ChangedTablesTracker, so it did nothing and no sibling test in the class uses it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
IDatabase::insert() is marked @internal for callers outside the rdbms library, which puts it on the same removal track as the other raw write methods (MW 1.43 turns such deprecations into test failures). Use the query builder, matching the newSelectQueryBuilder() read three lines below in the same helper and the only other raw-row insert in the suite (DatabaseDeletedSubjectPageIdsLookupTest). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The drain and batch-boundary tests hardcoded the private READABLE_NAMES_BATCH_SIZE of 100 in three ways (120 bulk rows, the boundary index [99], prose comments). Raising the constant above the row count would turn every one of them into a single-batch test that still passes, letting the exact while(false) truncation mutation they were written to kill survive again. Expose the constant and derive the bulk row count (BATCH_SIZE + 20) and the boundary index (BATCH_SIZE - 1) from it, so a changed batch size fails the tests instead of hollowing them out. Give the Layout and Mapping drains the Schema test's full ordered [pageId => name] assertion and matching name (DrainsEveryBatchInPageIdOrder) — the previous count + first-key + last-key shape passed a mid-run wrong-order or wrong-pairing regression that the Schema variant caught. Drop the stale "4 setUp Schemas" count from the doc-block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Codex-internal pager selector and its English "Next page" label were copy-pasted across five sites in three specs. A missed find() returns an empty DOMWrapper whose attributes( 'disabled' ) is undefined, so an enabled-state toBeUndefined() assertion would silently pass against a button that is not in the DOM. Extract findNextPageButton(), which owns the selector and asserts the button exists, and call it from all five sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The PR's premise that "only the null-cursor determinate flip was untested" held for SchemasPage, but MappingsPage had no pager test at all and gained only the indeterminate half. A totalRows.value = undefined mutation (drop the determinate branch) survived the whole MappingsPage suite, shipping a listing that ends on a page boundary with Next enabled forever and a click serving an empty page. Add the determinate test mirroring SchemasPage: a full page with a null cursor disables Next and shows "of 10". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The content guard added in 544a979 asserted assertStringNotContainsString( '"schemas"', $xml ) immediately after str_replace( '"schemas"', '"schemaX"', $xml ) — a post-condition str_replace makes true unconditionally, so it could never fail and guarded nothing (an independent review caught this). Assert the replacement token instead, assertStringContainsString( '"schemaX"', $xml ), which fails precisely when the substitution matched nothing and Beta would stay loadable. This mirrors the title guard, which already checks its replacement target. Verified by sabotage: a non-matching content needle now fails the guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…count-oracle # Conflicts: # resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts # resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts # resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1062.
The Problem
GET /schemas,GET /layouts, andGET /mappingsfiltered unreadable rows out of their pages but reported an unfiltered namespace count intotalRows. Subtracting the visible rows from that total gave any caller the number of read-restricted Schemas, Layouts, and Mappings. That is the count oracle of #1062, plus the third instance on/mappingsnoted there.Why Drop
totalRowsand Paginate by CursorTwo dead ends shaped the fix:
totalRowsthat does not leak is the count of rows the caller may read. Computing that count takes a per-title permission check across the entire namespace on every request, and each check fires the ACL hook that per-page read restriction depends on.Dropping
totalRowsand paginating by cursor resolves both at once. With no total there is nothing to leak, so the oracle closes structurally rather than by computing a safe number. A cursor resumes the listing where it left off, so a request costs only the rows it serves.What Changed
API contract (all three list endpoints). Requests take
limit(1–50, default 10) and an optional opaquecursor; responses are{ schemas|layouts|mappings: [...], "nextCursor": string|null }, wherenullmeans the listing is exhausted. A malformed cursor gets a structured400. The endpoints skip restricted rows exactly like nonexistent ones: no page space, no trace in the cursor, which only ever carries the page ID of a row the caller was served.Persistence. The three name lookups are now keyset generators:
getReadable*Names( int $afterPageId = 0 ): iterable<pageId, name>. Each batch seeks past the last seen page ID (WHERE page_id > cursor ORDER BY page_id LIMIT 100) and re-checks per-titleread(RIGOR_FULL) against the request authority.MappingNameLookup::getMappingNames()stays unfiltered for the system-context RDF-projection and DumpRdf path. A sharedCursorPaginationTraitcarries the param settings, cursor parsing, and page-fill loop; the loop skips readable-but-malformed rows, so a short page always means the end.Frontend. The new
useCursorPaginationcomposable maps CdxTable's offset-based server pagination onto cursors through a per-row-offset cursor history. Row offsets are limit-independent, so a page-size change stays aligned with the offset CdxTable keeps. The tables run the indeterminate "X–Y of many" pagination until a null cursor reveals the exact count, then switch to it. Without the switch, CdxTable's next-button heuristic (a short page) would miss a listing that ends exactly on a page boundary. Full enumeration in the schema picker followsnextCursorinstead of counting rows.Docs. New Cursor pagination section in
docs/api/rest-api.md; the Permissions section now also lists the Mapping read endpoint (a gap noted on #1062).Follow-up filed: #1130, search-first pickers and tables, the remaining large-namespace bottleneck.
Testing
getMappingNamespinned unfiltered.getUserPermissionsErrorshook on the request authority for all three endpoints. A restricted row leaves no gap in the page and no trace in the cursor; sabotage checks confirm the denial and determinate-flip tests fail when the fix is removed.phpcs,phpstan,eslint, andstylelintclean.Manual Browser Check
Special:Schemason a wiki with more than 10 Schemas (the demo seed has 23).Special:LayoutsandSpecial:Mappings(short single pages: "Showing the last N results", Next disabled).