From 9009b08d417ab7b13b199238bd56835bc06111c6 Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Wed, 22 Jul 2026 16:13:14 -0400 Subject: [PATCH 01/12] Replace totalRows pagination with cursors on the list endpoints 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) --- docs/api/rest-api.md | 35 ++++- phpstan-baseline.neon | 105 ++++++------- .../src/application/SchemaLookup.ts | 14 +- .../components/LayoutsPage/LayoutsPage.vue | 18 ++- .../components/MappingsPage/MappingsPage.vue | 18 ++- .../components/SchemasPage/SchemasPage.vue | 18 ++- .../src/composables/useCursorPagination.ts | 38 +++++ .../src/persistence/RestSchemaRepository.ts | 5 +- .../ext.neowiki/src/stores/SchemaStore.ts | 21 ++- .../MappingsPage/MappingsPage.spec.ts | 6 +- .../SchemasPage/SchemasPage.spec.ts | 21 ++- .../composables/useCursorPagination.spec.ts | 60 ++++++++ .../tests/stores/SchemaStore.spec.ts | 30 ++-- .../REST/CursorPaginationTrait.php | 96 ++++++++++++ .../REST/GetLayoutSummariesApi.php | 47 ++---- .../REST/GetMappingSummariesApi.php | 51 ++----- .../REST/GetSchemaSummariesApi.php | 47 ++---- src/NeoWikiExtension.php | 8 +- src/Persistence/LayoutNameLookup.php | 11 +- src/Persistence/MappingNameLookup.php | 13 +- .../MediaWiki/DatabaseLayoutNameLookup.php | 78 +++++----- .../MediaWiki/DatabaseMappingNameLookup.php | 51 +++++++ .../MediaWiki/DatabaseSchemaNameLookup.php | 48 ++++-- src/Persistence/SchemaNameLookup.php | 10 +- .../REST/GetLayoutSummariesApiTest.php | 142 ++++++++++++++++++ .../REST/GetMappingSummariesApiTest.php | 76 +++++++--- .../REST/GetSchemaSummariesApiTest.php | 113 +++++++++++--- tests/phpunit/NeoWikiIntegrationTestCase.php | 18 +++ .../DatabaseLayoutNameLookupTest.php | 99 ++++++++++++ .../DatabaseMappingNameLookupTest.php | 116 ++++++++++++++ .../DatabaseSchemaNameLookupTest.php | 68 ++++++++- 31 files changed, 1156 insertions(+), 325 deletions(-) create mode 100644 resources/ext.neowiki/src/composables/useCursorPagination.ts create mode 100644 resources/ext.neowiki/tests/composables/useCursorPagination.spec.ts create mode 100644 src/EntryPoints/REST/CursorPaginationTrait.php create mode 100644 tests/phpunit/EntryPoints/REST/GetLayoutSummariesApiTest.php create mode 100644 tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php create mode 100644 tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php diff --git a/docs/api/rest-api.md b/docs/api/rest-api.md index 8a749b79..e1174e7a 100644 --- a/docs/api/rest-api.md +++ b/docs/api/rest-api.md @@ -15,11 +15,16 @@ Every endpoint is also published as a complete [OpenAPI 3.0 description](#full-s ## Permissions -The Subject, page-subjects, subject-labels, Schema, Layout, RDF export, and entity-dereference read endpoints enforce -the caller's per-page `read` permission; page protection and `$wgNamespaceProtection` do not restrict them, because -MediaWiki's `read` action ignores both. When you may not read a page they respond as if the data were absent — a `null` -value, an empty list, or a `404` — never a `403`. `GET /subject-labels` omits the labels of Subjects whose page you -cannot read; because that filter runs per result, it caps `limit` at 50. +The Subject, page-subjects, subject-labels, Schema, Layout, Mapping, RDF export, and entity-dereference read endpoints +enforce the caller's per-page `read` permission; page protection and `$wgNamespaceProtection` do not restrict them, +because MediaWiki's `read` action ignores both. When you may not read a page they respond as if the data were absent — a +`null` value, an empty list, or a `404` — never a `403`. `GET /subject-labels` omits the labels of Subjects whose page +you cannot read; because that filter runs per result, it caps `limit` at 50. + +The `GET /schemas`, `GET /layouts`, and `GET /mappings` list endpoints paginate with an opaque cursor over the rows you +may read (see [Cursor pagination](#cursor-pagination)): a restricted Schema, Layout, or Mapping is skipped exactly like +one that does not exist, and no total count is reported, so nothing about restricted rows can be inferred from the +pagination. Subject write endpoints require per-page `edit` permission and answer `403` on denial. @@ -70,7 +75,7 @@ A Schema defines a Subject type and its properties. For the body shape, see | Endpoint | Description | |---|---| -| `GET /neowiki/v0/schemas` | List Schemas. Paginated with `limit` and `offset`. | +| `GET /neowiki/v0/schemas` | List Schemas. [Cursor-paginated](#cursor-pagination) with `limit` and `cursor`. | | `GET /neowiki/v0/schema/{schemaName}` | Fetch a Schema by name. | | `GET /neowiki/v0/schema-names/{search}` | Find Schema names by prefix. | @@ -80,7 +85,7 @@ A Layout defines how a Subject is displayed. | Endpoint | Description | |---|---| -| `GET /neowiki/v0/layouts` | List Layouts. Paginated with `limit` and `offset`. | +| `GET /neowiki/v0/layouts` | List Layouts. [Cursor-paginated](#cursor-pagination) with `limit` and `cursor`. | | `GET /neowiki/v0/layout/{layoutName}` | Fetch a Layout by name. | ### Mappings @@ -90,7 +95,7 @@ An ontology Mapping projects native Schemas to a target ontology. For the format | Endpoint | Description | |---|---| -| `GET /neowiki/v0/mappings` | List ontology Mappings, each with the names of its mapped Schemas. Paginated with `limit` and `offset`. | +| `GET /neowiki/v0/mappings` | List ontology Mappings, each with the names of its mapped Schemas. [Cursor-paginated](#cursor-pagination) with `limit` and `cursor`. | ### Query @@ -100,6 +105,20 @@ An ontology Mapping projects native Schemas to a target ontology. For the format +## Cursor pagination + +The Schema, Layout, and Mapping list endpoints paginate with an opaque cursor. Request up to `limit` items (1–50, +default 10); the response carries the items and a `nextCursor`: + +```json +{ "schemas": [ ... ], "nextCursor": "1462" } +``` + +Pass that value back as `cursor` to fetch the next page. `nextCursor` is `null` on the last page; a non-null cursor +means another page follows, though in rare cases (trailing items that fail to load) that page may come back empty. +Treat the cursor as opaque and do not construct one yourself; a malformed cursor is rejected with a `400`. Cursors stay +valid across item creation and deletion, and no total count is reported. + ## The `expand` parameter The Subject read and page-subjects read endpoints take an optional multi-valued `expand` query parameter (pipe-separated, diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 75002190..3f3c0c20 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -10,36 +10,6 @@ parameters: count: 1 path: src/NeoWikiExtension.php - - - message: "#^Cannot access property \\$page_title on array\\|stdClass\\|false\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php - - - - message: "#^Instantiated class TitleValue not found\\.$#" - count: 2 - path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php - - - - message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\Persistence\\\\MediaWiki\\\\DatabaseSchemaNameLookup\\:\\:dbResultToTitleValueArray\\(\\) has invalid return type TitleValue\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php - - - - message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\Persistence\\\\MediaWiki\\\\DatabaseSchemaNameLookup\\:\\:getFirstSchemaNames\\(\\) has invalid return type TitleValue\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php - - - - message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\Persistence\\\\MediaWiki\\\\DatabaseSchemaNameLookup\\:\\:getSchemaNamesMatching\\(\\) has invalid return type TitleValue\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php - - - - message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\Persistence\\\\MediaWiki\\\\DatabaseSchemaNameLookup\\:\\:searchSuggestionsToTitleArray\\(\\) has invalid return type TitleValue\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php - - message: "#^Call to method getPrefixedText\\(\\) on an unknown class Title\\.$#" count: 1 @@ -195,36 +165,6 @@ parameters: count: 1 path: src/GraphDatabasePlugins/Neo4j/Persistence/Neo4jQueryStore.php - - - message: "#^Cannot access property \\$page_title on array\\|stdClass\\|false\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php - - - - message: "#^Instantiated class TitleValue not found\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php - - - - message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\Persistence\\\\MediaWiki\\\\DatabaseLayoutNameLookup\\:\\:dbResultToTitleValueArray\\(\\) has invalid return type TitleValue\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php - - - - message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\Persistence\\\\MediaWiki\\\\DatabaseLayoutNameLookup\\:\\:getLayoutNames\\(\\) has invalid return type TitleValue\\.$#" - count: 1 - path: src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php - - - - message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\Persistence\\\\LayoutNameLookup\\:\\:getLayoutNames\\(\\) has invalid return type TitleValue\\.$#" - count: 1 - path: src/Persistence/LayoutNameLookup.php - - - - message: "#^Method ProfessionalWiki\\\\NeoWiki\\\\Persistence\\\\SchemaNameLookup\\:\\:getSchemaNamesMatching\\(\\) has invalid return type TitleValue\\.$#" - count: 1 - path: src/Persistence/SchemaNameLookup.php - - message: "#^Call to method getHeader\\(\\) on an unknown class WebRequest\\.$#" count: 1 @@ -299,3 +239,48 @@ parameters: message: "#^Parameter \\#3 \\$subject of function preg_replace expects array\\|string, string\\|null given\\.$#" count: 1 path: src/GraphDatabasePlugins/Neo4j/Application/KeywordCypherQueryValidator.php + + - + message: "#^Cannot access property \\$page_id on array\\|stdClass\\|false\\.$#" + count: 1 + path: src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php + + - + message: "#^Cannot access property \\$page_title on array\\|stdClass\\|false\\.$#" + count: 1 + path: src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php + + - + message: "#^Parameter \\#2 \\$op of method Wikimedia\\\\Rdbms\\\\IReadableDatabase\\:\\:expr\\(\\) expects '\\!\\='\\|'\\='\\|'\\\\\\\\x3C'\\|'\\\\\\\\x3C\\='\\|'\\\\\\\\x3E'\\|'\\\\\\\\x3E\\='\\|'LIKE'\\|'NOT LIKE', '\\>' given\\.$#" + count: 1 + path: src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php + + - + message: "#^Cannot access property \\$page_id on array\\|stdClass\\|false\\.$#" + count: 1 + path: src/Persistence/MediaWiki/DatabaseMappingNameLookup.php + + - + message: "#^Cannot access property \\$page_title on array\\|stdClass\\|false\\.$#" + count: 1 + path: src/Persistence/MediaWiki/DatabaseMappingNameLookup.php + + - + message: "#^Parameter \\#2 \\$op of method Wikimedia\\\\Rdbms\\\\IReadableDatabase\\:\\:expr\\(\\) expects '\\!\\='\\|'\\='\\|'\\\\\\\\x3C'\\|'\\\\\\\\x3C\\='\\|'\\\\\\\\x3E'\\|'\\\\\\\\x3E\\='\\|'LIKE'\\|'NOT LIKE', '\\>' given\\.$#" + count: 1 + path: src/Persistence/MediaWiki/DatabaseMappingNameLookup.php + + - + message: "#^Cannot access property \\$page_id on array\\|stdClass\\|false\\.$#" + count: 1 + path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php + + - + message: "#^Cannot access property \\$page_title on array\\|stdClass\\|false\\.$#" + count: 2 + path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php + + - + message: "#^Parameter \\#2 \\$op of method Wikimedia\\\\Rdbms\\\\IReadableDatabase\\:\\:expr\\(\\) expects '\\!\\='\\|'\\='\\|'\\\\\\\\x3C'\\|'\\\\\\\\x3C\\='\\|'\\\\\\\\x3E'\\|'\\\\\\\\x3E\\='\\|'LIKE'\\|'NOT LIKE', '\\>' given\\.$#" + count: 1 + path: src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php diff --git a/resources/ext.neowiki/src/application/SchemaLookup.ts b/resources/ext.neowiki/src/application/SchemaLookup.ts index 670d9add..f9dbf0e3 100644 --- a/resources/ext.neowiki/src/application/SchemaLookup.ts +++ b/resources/ext.neowiki/src/application/SchemaLookup.ts @@ -8,14 +8,14 @@ export interface SchemaSummary { export interface SchemaSummaryPage { schemas: SchemaSummary[]; - totalRows: number; + nextCursor: string | null; } export interface SchemaLookup { getSchema( schemaName: SchemaName ): Promise; getSchemaNames( search: string ): Promise; - getSchemaSummaries( offset: number, limit: number ): Promise; + getSchemaSummaries( cursor: string | null, limit: number ): Promise; } @@ -43,16 +43,20 @@ export class InMemorySchemaLookup implements SchemaLookup { return [ ...this.schemaNames ]; } - public async getSchemaSummaries( offset: number, limit: number ): Promise { + public async getSchemaSummaries( cursor: string | null, limit: number ): Promise { const summaries = [ ...this.schemas.values() ].map( ( schema ) => ( { name: schema.getName(), description: schema.getDescription(), propertyCount: [ ...schema.getPropertyDefinitions() ].length, } ) ); + // The cursor is opaque to callers; this fake encodes the next start index in it. + const start = cursor === null ? 0 : parseInt( cursor, 10 ); + const end = start + limit; + return { - schemas: summaries.slice( offset, offset + limit ), - totalRows: summaries.length, + schemas: summaries.slice( start, end ), + nextCursor: end < summaries.length ? String( end ) : null, }; } diff --git a/resources/ext.neowiki/src/components/LayoutsPage/LayoutsPage.vue b/resources/ext.neowiki/src/components/LayoutsPage/LayoutsPage.vue index 76040bc5..9e3d0268 100644 --- a/resources/ext.neowiki/src/components/LayoutsPage/LayoutsPage.vue +++ b/resources/ext.neowiki/src/components/LayoutsPage/LayoutsPage.vue @@ -113,6 +113,7 @@ import { CdxButton, CdxDialog, CdxIcon, CdxTable } from '@wikimedia/codex'; import type { TableColumn } from '@wikimedia/codex'; import { cdxIconAdd, cdxIconEdit, cdxIconTrash } from '@wikimedia/codex-icons'; import { NeoWikiExtension } from '@/NeoWikiExtension.ts'; +import { useCursorPagination } from '@/composables/useCursorPagination.ts'; import { useLayoutPermissions } from '@/composables/useLayoutPermissions.ts'; import { useLayoutStore } from '@/stores/LayoutStore.ts'; import { Layout } from '@/domain/Layout.ts'; @@ -128,10 +129,16 @@ const paginationSizeOptions: { value: number }[] = [ ]; const loading = ref( true ); -const totalRows = ref( 0 ); const isCreatorOpen = ref( false ); const pageSize = ref( paginationSizeOptions[ 0 ].value ); const lastOffset = ref( 0 ); +// Undefined while the end of the listing is unknown, which keeps CdxTable in its indeterminate +// pagination. Once a response carries a null cursor the exact count is known, and the table needs +// it: its indeterminate next-button heuristic (a short page) misses a listing that ends exactly on +// a page boundary. The count covers only rows this client has itself paged through, so it reveals +// nothing the row listing did not already. +const totalRows = ref( undefined ); +const { cursorFor, recordNextCursor } = useCursorPagination(); const { canCreateLayouts, canEditLayout: canEditLayouts, checkCreatePermission, checkEditPermission } = useLayoutPermissions(); const layoutStore = useLayoutStore(); @@ -194,11 +201,13 @@ async function fetchLayouts( offset: number, limit: number ): Promise { pageSize.value = limit; lastOffset.value = offset; + const cursor = cursorFor( offset ); + const cursorParam = cursor === null ? '' : `&cursor=${ encodeURIComponent( cursor ) }`; const restApiUrl = NeoWikiExtension.getInstance().getMediaWiki().util.wikiScript( 'rest' ); const httpClient = NeoWikiExtension.getInstance().newHttpClient(); const response = await httpClient.get( - `${ restApiUrl }/neowiki/v0/layouts?limit=${ limit }&offset=${ offset }` + `${ restApiUrl }/neowiki/v0/layouts?limit=${ limit }${ cursorParam }` ); if ( !response.ok ) { @@ -206,7 +215,7 @@ async function fetchLayouts( offset: number, limit: number ): Promise { return; } - const result: { layouts: LayoutSummary[]; totalRows: number } = await response.json(); + const result: { layouts: LayoutSummary[]; nextCursor: string | null } = await response.json(); rows.value = result.layouts.map( ( summary ) => ( { name: summary.name, @@ -215,7 +224,8 @@ async function fetchLayouts( offset: number, limit: number ): Promise { description: summary.description } ) ); - totalRows.value = result.totalRows; + recordNextCursor( offset, limit, result.nextCursor ); + totalRows.value = result.nextCursor === null ? offset + result.layouts.length : undefined; loading.value = false; } diff --git a/resources/ext.neowiki/src/components/MappingsPage/MappingsPage.vue b/resources/ext.neowiki/src/components/MappingsPage/MappingsPage.vue index 418d87db..6bcfe294 100644 --- a/resources/ext.neowiki/src/components/MappingsPage/MappingsPage.vue +++ b/resources/ext.neowiki/src/components/MappingsPage/MappingsPage.vue @@ -106,6 +106,7 @@ import { CdxButton, CdxDialog, CdxIcon, CdxTable } from '@wikimedia/codex'; import type { TableColumn } from '@wikimedia/codex'; import { cdxIconAdd, cdxIconEdit, cdxIconTrash } from '@wikimedia/codex-icons'; import { NeoWikiExtension } from '@/NeoWikiExtension.ts'; +import { useCursorPagination } from '@/composables/useCursorPagination.ts'; import { useMappingPermissions } from '@/composables/useMappingPermissions.ts'; import MappingCreatorDialog from './MappingCreatorDialog.vue'; import EditSummary from '@/components/common/EditSummary.vue'; @@ -118,10 +119,16 @@ const paginationSizeOptions: { value: number }[] = [ ]; const loading = ref( true ); -const totalRows = ref( 0 ); const pageSize = ref( paginationSizeOptions[ 0 ].value ); const lastOffset = ref( 0 ); const isCreatorOpen = ref( false ); +// Undefined while the end of the listing is unknown, which keeps CdxTable in its indeterminate +// pagination. Once a response carries a null cursor the exact count is known, and the table needs +// it: its indeterminate next-button heuristic (a short page) misses a listing that ends exactly on +// a page boundary. The count covers only rows this client has itself paged through, so it reveals +// nothing the row listing did not already. +const totalRows = ref( undefined ); +const { cursorFor, recordNextCursor } = useCursorPagination(); const isDeleteConfirmOpen = ref( false ); const deletingMappingName = ref( '' ); @@ -173,11 +180,13 @@ async function fetchMappings( offset: number, limit: number ): Promise { pageSize.value = limit; lastOffset.value = offset; + const cursor = cursorFor( offset ); + const cursorParam = cursor === null ? '' : `&cursor=${ encodeURIComponent( cursor ) }`; const restApiUrl = NeoWikiExtension.getInstance().getMediaWiki().util.wikiScript( 'rest' ); const httpClient = NeoWikiExtension.getInstance().newHttpClient(); const response = await httpClient.get( - `${ restApiUrl }/neowiki/v0/mappings?limit=${ limit }&offset=${ offset }` + `${ restApiUrl }/neowiki/v0/mappings?limit=${ limit }${ cursorParam }` ); if ( !response.ok ) { @@ -185,14 +194,15 @@ async function fetchMappings( offset: number, limit: number ): Promise { return; } - const result: { mappings: MappingSummary[]; totalRows: number } = await response.json(); + const result: { mappings: MappingSummary[]; nextCursor: string | null } = await response.json(); rows.value = result.mappings.map( ( summary ) => ( { name: summary.name, schemas: summary.schemas } ) ); - totalRows.value = result.totalRows; + recordNextCursor( offset, limit, result.nextCursor ); + totalRows.value = result.nextCursor === null ? offset + result.mappings.length : undefined; loading.value = false; } diff --git a/resources/ext.neowiki/src/components/SchemasPage/SchemasPage.vue b/resources/ext.neowiki/src/components/SchemasPage/SchemasPage.vue index 96b76658..72498756 100644 --- a/resources/ext.neowiki/src/components/SchemasPage/SchemasPage.vue +++ b/resources/ext.neowiki/src/components/SchemasPage/SchemasPage.vue @@ -108,6 +108,7 @@ import { CdxButton, CdxDialog, CdxIcon, CdxTable } from '@wikimedia/codex'; import type { TableColumn } from '@wikimedia/codex'; import { cdxIconAdd, cdxIconEdit, cdxIconTrash } from '@wikimedia/codex-icons'; import { NeoWikiExtension } from '@/NeoWikiExtension.ts'; +import { useCursorPagination } from '@/composables/useCursorPagination.ts'; import { useSchemaPermissions } from '@/composables/useSchemaPermissions.ts'; import { useSchemaStore } from '@/stores/SchemaStore.ts'; import { Schema } from '@/domain/Schema.ts'; @@ -124,10 +125,16 @@ const paginationSizeOptions: { value: number }[] = [ ]; const loading = ref( true ); -const totalRows = ref( 0 ); const isCreatorOpen = ref( false ); const pageSize = ref( paginationSizeOptions[ 0 ].value ); const lastOffset = ref( 0 ); +// Undefined while the end of the listing is unknown, which keeps CdxTable in its indeterminate +// pagination. Once a response carries a null cursor the exact count is known, and the table needs +// it: its indeterminate next-button heuristic (a short page) misses a listing that ends exactly on +// a page boundary. The count covers only rows this client has itself paged through, so it reveals +// nothing the row listing did not already. +const totalRows = ref( undefined ); +const { cursorFor, recordNextCursor } = useCursorPagination(); const { canEditSchema, canCreateSchemas, checkEditPermission, checkCreatePermission } = useSchemaPermissions(); const schemaStore = useSchemaStore(); @@ -173,11 +180,13 @@ async function fetchSchemas( offset: number, limit: number ): Promise { pageSize.value = limit; lastOffset.value = offset; + const cursor = cursorFor( offset ); + const cursorParam = cursor === null ? '' : `&cursor=${ encodeURIComponent( cursor ) }`; const restApiUrl = NeoWikiExtension.getInstance().getMediaWiki().util.wikiScript( 'rest' ); const httpClient = NeoWikiExtension.getInstance().newHttpClient(); const response = await httpClient.get( - `${ restApiUrl }/neowiki/v0/schemas?limit=${ limit }&offset=${ offset }` + `${ restApiUrl }/neowiki/v0/schemas?limit=${ limit }${ cursorParam }` ); if ( !response.ok ) { @@ -185,7 +194,7 @@ async function fetchSchemas( offset: number, limit: number ): Promise { return; } - const result: { schemas: SchemaSummary[]; totalRows: number } = await response.json(); + const result: { schemas: SchemaSummary[]; nextCursor: string | null } = await response.json(); rows.value = result.schemas.map( ( summary ) => ( { name: summary.name, @@ -193,7 +202,8 @@ async function fetchSchemas( offset: number, limit: number ): Promise { properties: summary.propertyCount } ) ); - totalRows.value = result.totalRows; + recordNextCursor( offset, limit, result.nextCursor ); + totalRows.value = result.nextCursor === null ? offset + result.schemas.length : undefined; loading.value = false; } diff --git a/resources/ext.neowiki/src/composables/useCursorPagination.ts b/resources/ext.neowiki/src/composables/useCursorPagination.ts new file mode 100644 index 00000000..309a6e4e --- /dev/null +++ b/resources/ext.neowiki/src/composables/useCursorPagination.ts @@ -0,0 +1,38 @@ +/** + * Maps CdxTable's offset-based server pagination onto the cursor-based listing endpoints. + * + * CdxTable requests pages as ( offset, limit ) pairs, while the Schema/Layout/Mapping listing + * endpoints page with an opaque cursor (see docs/api/rest-api.md). The map records, under each row + * offset, the cursor that resumes the listing there. A row offset is limit-independent, and + * CdxTable only ever requests offsets it has already walked to: navigation moves by the current + * page size or back to zero, and a page-size change re-requests the current offset with the new + * limit (so the recorded cursor keeps the served rows aligned with the "X–Y of many" label). + * Recording each response's nextCursor under the offset it unlocks therefore covers every + * reachable request. The consuming page keeps the table's total-rows undefined (indeterminate + * pagination) until a response carries a null cursor, then reports the now-known count — the + * indeterminate next-button heuristic (a short page) would miss a listing that ends exactly on a + * page boundary. + * + * Known limit: the pager buttons stay clickable while a fetch is in flight, so a second click + * before the response lands requests an offset with no recorded cursor and falls back to the + * first page. Accepted — the follow-up response overwrites the rows, and CdxTable offers no way + * to rewind its internal offset. + */ +interface CursorPagination { + cursorFor: ( offset: number ) => string | null; + recordNextCursor: ( offset: number, limit: number, nextCursor: string | null ) => void; +} + +export function useCursorPagination(): CursorPagination { + const cursorByOffset = new Map( [ [ 0, null ] ] ); + + function cursorFor( offset: number ): string | null { + return cursorByOffset.get( offset ) ?? null; + } + + function recordNextCursor( offset: number, limit: number, nextCursor: string | null ): void { + cursorByOffset.set( offset + limit, nextCursor ); + } + + return { cursorFor, recordNextCursor }; +} diff --git a/resources/ext.neowiki/src/persistence/RestSchemaRepository.ts b/resources/ext.neowiki/src/persistence/RestSchemaRepository.ts index ef772ae8..91ba84fa 100644 --- a/resources/ext.neowiki/src/persistence/RestSchemaRepository.ts +++ b/resources/ext.neowiki/src/persistence/RestSchemaRepository.ts @@ -48,9 +48,10 @@ export class RestSchemaRepository implements SchemaRepository { return await response.json(); } - public async getSchemaSummaries( offset: number, limit: number ): Promise { + public async getSchemaSummaries( cursor: string | null, limit: number ): Promise { + const cursorParam = cursor === null ? '' : `&cursor=${ encodeURIComponent( cursor ) }`; const response = await this.httpClient.get( - `${ this.mediaWikiRestApiUrl }/neowiki/v0/schemas?limit=${ limit }&offset=${ offset }`, + `${ this.mediaWikiRestApiUrl }/neowiki/v0/schemas?limit=${ limit }${ cursorParam }`, ); if ( !response.ok ) { diff --git a/resources/ext.neowiki/src/stores/SchemaStore.ts b/resources/ext.neowiki/src/stores/SchemaStore.ts index 138001f5..0d0aeec4 100644 --- a/resources/ext.neowiki/src/stores/SchemaStore.ts +++ b/resources/ext.neowiki/src/stores/SchemaStore.ts @@ -61,25 +61,24 @@ export const useSchemaStore = defineStore( 'schema', { return this.summariesRequest; }, - // Pages through the summaries endpoint (capped at 50) by request offset, not by - // loaded count: the endpoint counts every Schema page in totalRows but omits ones - // it cannot load (restricted or malformed), so advancing by summaries.length would - // re-request earlier names and duplicate entries. Stop once the offset passes the - // total. The in-flight request is released on completion so a later load (after the - // cache is cleared, or after a failure) starts fresh. + // Pages through the summaries endpoint (capped at 50) by following the response's + // cursor until it is null. The cursor, not the page length, decides whether more + // pages follow: a page can come back shorter than requested when a readable Schema + // fails to load (malformed). The in-flight request is released on completion so a + // later load (after the cache is cleared, or after a failure) starts fresh. async fetchAllSchemaSummaries(): Promise { const repository = NeoWikiExtension.getInstance().getSchemaRepository(); const pageSize = 50; const summaries: SchemaSummary[] = []; try { - const firstPage = await repository.getSchemaSummaries( 0, pageSize ); - summaries.push( ...firstPage.schemas ); + let cursor: string | null = null; - for ( let offset = pageSize; offset < firstPage.totalRows; offset += pageSize ) { - const page = await repository.getSchemaSummaries( offset, pageSize ); + do { + const page = await repository.getSchemaSummaries( cursor, pageSize ); summaries.push( ...page.schemas ); - } + cursor = page.nextCursor; + } while ( cursor !== null ); this.allSummaries = summaries; return summaries; diff --git a/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts b/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts index bb17ddde..541165f8 100644 --- a/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts +++ b/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts @@ -18,7 +18,7 @@ const checkCreatePermissionMock = vi.fn(); const checkEditPermissionMock = vi.fn(); const checkDeletePermissionMock = vi.fn(); -let mappingsResponse: { mappings: MappingSummary[]; totalRows: number } = { mappings: [], totalRows: 0 }; +let mappingsResponse: { mappings: MappingSummary[]; nextCursor: string | null } = { mappings: [], nextCursor: null }; vi.mock( '@/composables/useMappingPermissions.ts', () => ( { useMappingPermissions: () => ( { @@ -71,7 +71,7 @@ function findDeleteButtons( wrapper: VueWrapper ): VueWrapper[] { function mountComponent( summaries: MappingSummary[] = [] ): VueWrapper { mappingsResponse = { mappings: summaries, - totalRows: summaries.length, + nextCursor: null, }; setupMwMock( { functions: [ 'msg', 'util', 'message', 'notify' ], @@ -99,7 +99,7 @@ describe( 'MappingsPage', () => { checkCreatePermissionMock.mockClear(); checkEditPermissionMock.mockClear(); checkDeletePermissionMock.mockClear(); - mappingsResponse = { mappings: [], totalRows: 0 }; + mappingsResponse = { mappings: [], nextCursor: null }; } ); it( 'links each mapped schema name to its Schema page and shows the count', async () => { diff --git a/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts b/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts index fea18029..56168e0f 100644 --- a/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts +++ b/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts @@ -14,7 +14,7 @@ const canEditSchemaRef = ref( false ); const checkCreatePermissionMock = vi.fn(); const checkEditPermissionMock = vi.fn(); -let schemasResponse: { schemas: unknown[]; totalRows: number } = { schemas: [], totalRows: 0 }; +let schemasResponse: { schemas: unknown[]; nextCursor: string | null } = { schemas: [], nextCursor: null }; vi.mock( '@/composables/useSchemaPermissions.ts', () => ( { useSchemaPermissions: () => ( { @@ -82,7 +82,7 @@ function findDeleteButtons( wrapper: VueWrapper ): VueWrapper[] { function mountComponent( summaries: unknown[] = [] ): VueWrapper { schemasResponse = { schemas: summaries, - totalRows: summaries.length, + nextCursor: null, }; setupMwMock( { functions: [ 'msg', 'util', 'message', 'notify' ] } ); @@ -108,7 +108,7 @@ describe( 'SchemasPage', () => { checkEditPermissionMock.mockClear(); fetchSchemaMock.mockClear(); getSchemaMock.mockClear(); - schemasResponse = { schemas: [], totalRows: 0 }; + schemasResponse = { schemas: [], nextCursor: null }; } ); it( 'shows create button when user has create permission', async () => { @@ -147,6 +147,21 @@ describe( 'SchemasPage', () => { expect( wrapper.findComponent( SchemaCreatorDialog ).exists() ).toBe( false ); } ); + it( 'disables next when a full page ends the listing', async () => { + // A listing that ends exactly on a page boundary returns a full page with a null + // cursor. CdxTable's indeterminate mode would keep next enabled (its heuristic is a + // short page), so the component must switch the table to a known total. + const wrapper = mountComponent( Array.from( { length: 10 }, ( _value, index ) => ( + { name: `Schema${ index }`, description: '', propertyCount: 1 } + ) ) ); + await flushPromises(); + + const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + + expect( nextButton.attributes( 'disabled' ) ).toBeDefined(); + expect( wrapper.text() ).toContain( 'of 10' ); + } ); + it( 'shows empty value indicator for schemas without a description', async () => { const wrapper = mountComponent( [ { name: 'Person', description: '', propertyCount: 3 }, diff --git a/resources/ext.neowiki/tests/composables/useCursorPagination.spec.ts b/resources/ext.neowiki/tests/composables/useCursorPagination.spec.ts new file mode 100644 index 00000000..ae7df2a4 --- /dev/null +++ b/resources/ext.neowiki/tests/composables/useCursorPagination.spec.ts @@ -0,0 +1,60 @@ +import { describe, it, expect } from 'vitest'; +import { useCursorPagination } from '@/composables/useCursorPagination.ts'; + +describe( 'useCursorPagination', () => { + + it( 'starts the first page without a cursor', () => { + const { cursorFor } = useCursorPagination(); + + expect( cursorFor( 0 ) ).toBeNull(); + } ); + + it( 'serves the recorded cursor when the table moves to the next page', () => { + const { cursorFor, recordNextCursor } = useCursorPagination(); + + cursorFor( 0 ); + recordNextCursor( 0, 10, 'cursor-1' ); + + expect( cursorFor( 10 ) ).toBe( 'cursor-1' ); + } ); + + it( 'serves earlier cursors again when the table moves back', () => { + const { cursorFor, recordNextCursor } = useCursorPagination(); + + cursorFor( 0 ); + recordNextCursor( 0, 10, 'cursor-1' ); + cursorFor( 10 ); + recordNextCursor( 10, 10, 'cursor-2' ); + + expect( cursorFor( 0 ) ).toBeNull(); + expect( cursorFor( 10 ) ).toBe( 'cursor-1' ); + } ); + + it( 'serves the row-offset cursor across a page-size change', () => { + const { cursorFor, recordNextCursor } = useCursorPagination(); + + cursorFor( 0 ); + recordNextCursor( 0, 10, 'cursor-1' ); + + // CdxTable keeps its offset on a page-size change and re-requests ( 10, 20 ). The + // recorded cursor resumes at row 10 regardless of the limit, so the served rows match + // the table's "11–30" label. + expect( cursorFor( 10 ) ).toBe( 'cursor-1' ); + recordNextCursor( 10, 20, 'cursor-2' ); + + expect( cursorFor( 30 ) ).toBe( 'cursor-2' ); + } ); + + it( 'keeps refetching the current page on the same cursor', () => { + const { cursorFor, recordNextCursor } = useCursorPagination(); + + cursorFor( 0 ); + recordNextCursor( 0, 10, 'cursor-1' ); + cursorFor( 10 ); + recordNextCursor( 10, 10, 'cursor-2' ); + + // A refetch after an edit or delete re-requests the same offset. + expect( cursorFor( 10 ) ).toBe( 'cursor-1' ); + } ); + +} ); diff --git a/resources/ext.neowiki/tests/stores/SchemaStore.spec.ts b/resources/ext.neowiki/tests/stores/SchemaStore.spec.ts index 6d142c82..cdf74236 100644 --- a/resources/ext.neowiki/tests/stores/SchemaStore.spec.ts +++ b/resources/ext.neowiki/tests/stores/SchemaStore.spec.ts @@ -52,36 +52,36 @@ describe( 'SchemaStore getAllSchemaSummaries', () => { vi.restoreAllMocks(); } ); - it( 'pages through every schema summary across multiple pages', async () => { + it( 'pages through every schema summary by following the cursor', async () => { const getSchemaSummaries = vi.fn() - .mockResolvedValueOnce( { schemas: manySummaries( 50, 'A' ), totalRows: 60 } ) - .mockResolvedValueOnce( { schemas: manySummaries( 10, 'B' ), totalRows: 60 } ); + .mockResolvedValueOnce( { schemas: manySummaries( 50, 'A' ), nextCursor: 'cursor-1' } ) + .mockResolvedValueOnce( { schemas: manySummaries( 10, 'B' ), nextCursor: null } ); withRepository( { getSchemaSummaries } ); const result = await useSchemaStore().getAllSchemaSummaries(); expect( result ).toHaveLength( 60 ); - expect( getSchemaSummaries ).toHaveBeenNthCalledWith( 1, 0, 50 ); - expect( getSchemaSummaries ).toHaveBeenNthCalledWith( 2, 50, 50 ); + expect( getSchemaSummaries ).toHaveBeenNthCalledWith( 1, null, 50 ); + expect( getSchemaSummaries ).toHaveBeenNthCalledWith( 2, 'cursor-1', 50 ); } ); - it( 'advances by page size, not loaded count, when a page omits unloadable schemas', async () => { - // The endpoint counts 60 schema pages in totalRows but can only load 49 in the - // first window (one is restricted or malformed) and 10 in the second. + it( 'keeps following the cursor when a page omits unloadable schemas', async () => { + // A page can come back shorter than requested when a readable schema fails to load + // (malformed); the cursor, not the page length, decides whether more pages follow. const getSchemaSummaries = vi.fn() - .mockResolvedValueOnce( { schemas: manySummaries( 49, 'A' ), totalRows: 60 } ) - .mockResolvedValueOnce( { schemas: manySummaries( 10, 'B' ), totalRows: 60 } ); + .mockResolvedValueOnce( { schemas: manySummaries( 49, 'A' ), nextCursor: 'cursor-1' } ) + .mockResolvedValueOnce( { schemas: manySummaries( 10, 'B' ), nextCursor: null } ); withRepository( { getSchemaSummaries } ); const result = await useSchemaStore().getAllSchemaSummaries(); expect( result ).toHaveLength( 59 ); - expect( getSchemaSummaries ).toHaveBeenNthCalledWith( 2, 50, 50 ); + expect( getSchemaSummaries ).toHaveBeenNthCalledWith( 2, 'cursor-1', 50 ); expect( getSchemaSummaries ).toHaveBeenCalledTimes( 2 ); } ); it( 'caches the summaries and does not refetch on the next call', async () => { - const getSchemaSummaries = vi.fn().mockResolvedValue( { schemas: [ summary( 'A' ) ], totalRows: 1 } ); + const getSchemaSummaries = vi.fn().mockResolvedValue( { schemas: [ summary( 'A' ) ], nextCursor: null } ); withRepository( { getSchemaSummaries } ); const store = useSchemaStore(); @@ -92,7 +92,7 @@ describe( 'SchemaStore getAllSchemaSummaries', () => { } ); it( 'shares one in-flight request across concurrent callers', async () => { - const getSchemaSummaries = vi.fn().mockResolvedValue( { schemas: [ summary( 'A' ) ], totalRows: 1 } ); + const getSchemaSummaries = vi.fn().mockResolvedValue( { schemas: [ summary( 'A' ) ], nextCursor: null } ); withRepository( { getSchemaSummaries } ); const store = useSchemaStore(); @@ -104,7 +104,7 @@ describe( 'SchemaStore getAllSchemaSummaries', () => { it( 'releases the in-flight request after a failure so the next call retries', async () => { const getSchemaSummaries = vi.fn() .mockRejectedValueOnce( new Error( 'load failed' ) ) - .mockResolvedValueOnce( { schemas: [ summary( 'A' ) ], totalRows: 1 } ); + .mockResolvedValueOnce( { schemas: [ summary( 'A' ) ], nextCursor: null } ); withRepository( { getSchemaSummaries } ); const store = useSchemaStore(); @@ -116,7 +116,7 @@ describe( 'SchemaStore getAllSchemaSummaries', () => { } ); it( 'refetches summaries after a schema is saved', async () => { - const getSchemaSummaries = vi.fn().mockResolvedValue( { schemas: [ summary( 'A' ) ], totalRows: 1 } ); + const getSchemaSummaries = vi.fn().mockResolvedValue( { schemas: [ summary( 'A' ) ], nextCursor: null } ); const saveSchema = vi.fn().mockResolvedValue( undefined ); withRepository( { getSchemaSummaries, saveSchema } ); const store = useSchemaStore(); diff --git a/src/EntryPoints/REST/CursorPaginationTrait.php b/src/EntryPoints/REST/CursorPaginationTrait.php new file mode 100644 index 00000000..c9910017 --- /dev/null +++ b/src/EntryPoints/REST/CursorPaginationTrait.php @@ -0,0 +1,96 @@ +> + */ + private function paginationParamSettings(): array { + return [ + 'limit' => [ + self::PARAM_SOURCE => 'query', + ParamValidator::PARAM_TYPE => 'integer', + ParamValidator::PARAM_REQUIRED => false, + ParamValidator::PARAM_DEFAULT => 10, + IntegerDef::PARAM_MIN => 1, + IntegerDef::PARAM_MAX => 50, + self::PARAM_DESCRIPTION => 'Maximum number of items to return.', + ], + 'cursor' => [ + self::PARAM_SOURCE => 'query', + ParamValidator::PARAM_TYPE => 'string', + ParamValidator::PARAM_REQUIRED => false, + ParamValidator::PARAM_DEFAULT => null, + self::PARAM_DESCRIPTION => 'Opaque pagination cursor: the nextCursor of the previous response. Omit for the first page.', + ], + ]; + } + + private function pageIdFromCursor( ?string $cursor ): int { + if ( $cursor === null || $cursor === '' ) { + return 0; + } + + if ( !ctype_digit( $cursor ) ) { + // The structured body matches the other handlers' createHttpError responses. + throw new ResponseException( + $this->getResponseFactory()->createHttpError( 400, [ 'message' => 'Invalid cursor' ] ) + ); + } + + return (int)$cursor; + } + + /** + * Fills a page with up to $limit summaries and derives the follow-up cursor. The cursor is the + * page ID of the last consumed name, so names a load skips (readable but malformed) are not + * re-consumed by the next page. nextCursor is null exactly when the listing is exhausted. + * + * @param iterable $readableNames + * @param callable(mixed): ?array $loadSummary null when the name does not resolve to a listable item + * @return array{items: list, nextCursor: ?string} + */ + private function buildPage( iterable $readableNames, int $limit, callable $loadSummary ): array { + $items = []; + $lastPageId = 0; + $hasMore = false; + + foreach ( $readableNames as $pageId => $name ) { + if ( count( $items ) === $limit ) { + $hasMore = true; + break; + } + + $lastPageId = $pageId; + $summary = $loadSummary( $name ); + + if ( $summary === null ) { + continue; + } + + $items[] = $summary; + } + + return [ + 'items' => $items, + 'nextCursor' => $hasMore ? (string)$lastPageId : null, + ]; + } + +} diff --git a/src/EntryPoints/REST/GetLayoutSummariesApi.php b/src/EntryPoints/REST/GetLayoutSummariesApi.php index 1056df4d..df291c90 100644 --- a/src/EntryPoints/REST/GetLayoutSummariesApi.php +++ b/src/EntryPoints/REST/GetLayoutSummariesApi.php @@ -7,35 +7,32 @@ use MediaWiki\Rest\Response; use MediaWiki\Rest\SimpleHandler; use MediaWiki\Rest\StringStream; +use MediaWiki\Title\TitleValue; use ProfessionalWiki\NeoWiki\Domain\Layout\Layout; use ProfessionalWiki\NeoWiki\Domain\Layout\LayoutName; use ProfessionalWiki\NeoWiki\NeoWikiExtension; -use Wikimedia\ParamValidator\ParamValidator; -use Wikimedia\ParamValidator\TypeDef\IntegerDef; class GetLayoutSummariesApi extends SimpleHandler { + use CursorPaginationTrait; + public function run(): Response { $params = $this->getValidatedParams(); $extension = NeoWikiExtension::getInstance(); - $layoutNameLookup = $extension->getLayoutNameLookup(); $layoutLookup = $extension->getLayoutLookup(); - $summaries = []; - - foreach ( $layoutNameLookup->getLayoutNames( $params['limit'], $params['offset'] ) as $title ) { - $layout = $layoutLookup->getLayout( new LayoutName( $title->getText() ) ); - - if ( $layout === null ) { - continue; + $page = $this->buildPage( + $extension->getLayoutNameLookup()->getReadableLayoutNames( $this->pageIdFromCursor( $params['cursor'] ) ), + $params['limit'], + function ( TitleValue $title ) use ( $layoutLookup ): ?array { + $layout = $layoutLookup->getLayout( new LayoutName( $title->getText() ) ); + return $layout === null ? null : $this->layoutToSummary( $layout ); } - - $summaries[] = $this->layoutToSummary( $layout ); - } + ); $result = [ - 'layouts' => $summaries, - 'totalRows' => $layoutNameLookup->getLayoutCount(), + 'layouts' => $page['items'], + 'nextCursor' => $page['nextCursor'], ]; $response = $this->getResponseFactory()->create(); @@ -46,25 +43,7 @@ public function run(): Response { } public function getParamSettings(): array { - return [ - 'limit' => [ - self::PARAM_SOURCE => 'query', - ParamValidator::PARAM_TYPE => 'integer', - ParamValidator::PARAM_REQUIRED => false, - ParamValidator::PARAM_DEFAULT => 10, - IntegerDef::PARAM_MIN => 1, - IntegerDef::PARAM_MAX => 50, - self::PARAM_DESCRIPTION => 'Maximum number of items to return.', - ], - 'offset' => [ - self::PARAM_SOURCE => 'query', - ParamValidator::PARAM_TYPE => 'integer', - ParamValidator::PARAM_REQUIRED => false, - ParamValidator::PARAM_DEFAULT => 0, - IntegerDef::PARAM_MIN => 0, - self::PARAM_DESCRIPTION => 'Zero-based index of the first item to return.', - ], - ]; + return $this->paginationParamSettings(); } /** diff --git a/src/EntryPoints/REST/GetMappingSummariesApi.php b/src/EntryPoints/REST/GetMappingSummariesApi.php index cc892758..d68c0a95 100644 --- a/src/EntryPoints/REST/GetMappingSummariesApi.php +++ b/src/EntryPoints/REST/GetMappingSummariesApi.php @@ -8,37 +8,30 @@ use MediaWiki\Rest\SimpleHandler; use MediaWiki\Rest\StringStream; use ProfessionalWiki\NeoWiki\Domain\Mapping\Mapping; +use ProfessionalWiki\NeoWiki\Domain\Mapping\MappingName; use ProfessionalWiki\NeoWiki\NeoWikiExtension; -use Wikimedia\ParamValidator\ParamValidator; -use Wikimedia\ParamValidator\TypeDef\IntegerDef; class GetMappingSummariesApi extends SimpleHandler { + use CursorPaginationTrait; + public function run(): Response { $params = $this->getValidatedParams(); $extension = NeoWikiExtension::getInstance(); $mappingLookup = $extension->getMappingLookup(); - // The Mapping name lookup is shared with the RDF-projection path and exposes no SQL-level - // pagination or count. With one Mapping page per target ontology the name list stays small, - // so slicing it here beats widening that interface. - $names = $extension->getMappingNameLookup()->getMappingNames(); - - $summaries = []; - - foreach ( array_slice( $names, $params['offset'], $params['limit'] ) as $name ) { - $mapping = $mappingLookup->getMapping( $name ); - - if ( $mapping === null ) { - continue; + $page = $this->buildPage( + $extension->getMappingNameLookup()->getReadableMappingNames( $this->pageIdFromCursor( $params['cursor'] ) ), + $params['limit'], + function ( MappingName $name ) use ( $mappingLookup ): ?array { + $mapping = $mappingLookup->getMapping( $name ); + return $mapping === null ? null : $this->mappingToSummary( $mapping ); } - - $summaries[] = $this->mappingToSummary( $mapping ); - } + ); $result = [ - 'mappings' => $summaries, - 'totalRows' => count( $names ), + 'mappings' => $page['items'], + 'nextCursor' => $page['nextCursor'], ]; $response = $this->getResponseFactory()->create(); @@ -49,25 +42,7 @@ public function run(): Response { } public function getParamSettings(): array { - return [ - 'limit' => [ - self::PARAM_SOURCE => 'query', - ParamValidator::PARAM_TYPE => 'integer', - ParamValidator::PARAM_REQUIRED => false, - ParamValidator::PARAM_DEFAULT => 10, - IntegerDef::PARAM_MIN => 1, - IntegerDef::PARAM_MAX => 50, - self::PARAM_DESCRIPTION => 'Maximum number of items to return.', - ], - 'offset' => [ - self::PARAM_SOURCE => 'query', - ParamValidator::PARAM_TYPE => 'integer', - ParamValidator::PARAM_REQUIRED => false, - ParamValidator::PARAM_DEFAULT => 0, - IntegerDef::PARAM_MIN => 0, - self::PARAM_DESCRIPTION => 'Zero-based index of the first item to return.', - ], - ]; + return $this->paginationParamSettings(); } /** diff --git a/src/EntryPoints/REST/GetSchemaSummariesApi.php b/src/EntryPoints/REST/GetSchemaSummariesApi.php index 869101c1..382f9668 100644 --- a/src/EntryPoints/REST/GetSchemaSummariesApi.php +++ b/src/EntryPoints/REST/GetSchemaSummariesApi.php @@ -7,35 +7,32 @@ use MediaWiki\Rest\Response; use MediaWiki\Rest\SimpleHandler; use MediaWiki\Rest\StringStream; +use MediaWiki\Title\TitleValue; use ProfessionalWiki\NeoWiki\Domain\Schema\Schema; use ProfessionalWiki\NeoWiki\Domain\Schema\SchemaName; use ProfessionalWiki\NeoWiki\NeoWikiExtension; -use Wikimedia\ParamValidator\ParamValidator; -use Wikimedia\ParamValidator\TypeDef\IntegerDef; class GetSchemaSummariesApi extends SimpleHandler { + use CursorPaginationTrait; + public function run(): Response { $params = $this->getValidatedParams(); $extension = NeoWikiExtension::getInstance(); - $schemaNameLookup = $extension->getSchemaNameLookup(); $schemaLookup = $extension->getSchemaLookup(); - $summaries = []; - - foreach ( $schemaNameLookup->getSchemaNamesMatching( '', $params['limit'], $params['offset'] ) as $title ) { - $schema = $schemaLookup->getSchema( new SchemaName( $title->getText() ) ); - - if ( $schema === null ) { - continue; + $page = $this->buildPage( + $extension->getSchemaNameLookup()->getReadableSchemaNames( $this->pageIdFromCursor( $params['cursor'] ) ), + $params['limit'], + function ( TitleValue $title ) use ( $schemaLookup ): ?array { + $schema = $schemaLookup->getSchema( new SchemaName( $title->getText() ) ); + return $schema === null ? null : $this->schemaToSummary( $schema ); } - - $summaries[] = $this->schemaToSummary( $schema ); - } + ); $result = [ - 'schemas' => $summaries, - 'totalRows' => $schemaNameLookup->getSchemaCount(), + 'schemas' => $page['items'], + 'nextCursor' => $page['nextCursor'], ]; $response = $this->getResponseFactory()->create(); @@ -46,25 +43,7 @@ public function run(): Response { } public function getParamSettings(): array { - return [ - 'limit' => [ - self::PARAM_SOURCE => 'query', - ParamValidator::PARAM_TYPE => 'integer', - ParamValidator::PARAM_REQUIRED => false, - ParamValidator::PARAM_DEFAULT => 10, - IntegerDef::PARAM_MIN => 1, - IntegerDef::PARAM_MAX => 50, - self::PARAM_DESCRIPTION => 'Maximum number of items to return.', - ], - 'offset' => [ - self::PARAM_SOURCE => 'query', - ParamValidator::PARAM_TYPE => 'integer', - ParamValidator::PARAM_REQUIRED => false, - ParamValidator::PARAM_DEFAULT => 0, - IntegerDef::PARAM_MIN => 0, - self::PARAM_DESCRIPTION => 'Zero-based index of the first item to return.', - ], - ]; + return $this->paginationParamSettings(); } /** diff --git a/src/NeoWikiExtension.php b/src/NeoWikiExtension.php index d68efc5a..333b3cc3 100644 --- a/src/NeoWikiExtension.php +++ b/src/NeoWikiExtension.php @@ -493,7 +493,11 @@ public function getMappingLookup(): MappingLookup { } public function getMappingNameLookup(): MappingNameLookup { - return new DatabaseMappingNameLookup( db: $this->getDbConnection() ); + return new DatabaseMappingNameLookup( + db: $this->getDbConnection(), + readAuthorizer: $this->newPageReadAuthorizer( $this->getRequestAuthority() ), + titleFactory: MediaWikiServices::getInstance()->getTitleFactory(), + ); } public function getMappingPersistenceDeserializer(): MappingPersistenceDeserializer { @@ -1179,6 +1183,8 @@ public static function newGetSchemaSummariesApi(): GetSchemaSummariesApi { public function getLayoutNameLookup(): LayoutNameLookup { return new DatabaseLayoutNameLookup( db: $this->getDbConnection(), + readAuthorizer: $this->newPageReadAuthorizer( $this->getRequestAuthority() ), + titleFactory: MediaWikiServices::getInstance()->getTitleFactory(), ); } diff --git a/src/Persistence/LayoutNameLookup.php b/src/Persistence/LayoutNameLookup.php index 60e81134..e1098ba1 100644 --- a/src/Persistence/LayoutNameLookup.php +++ b/src/Persistence/LayoutNameLookup.php @@ -7,10 +7,13 @@ interface LayoutNameLookup { /** - * @return TitleValue[] + * The Layout names the caller may read, keyed by page ID, in page-ID order, starting after the + * given page ID. The summaries endpoint fills its page from this iterable and uses the keys as + * pagination cursor, so an unreadable Layout neither appears, nor takes page space, nor is + * inferable from the pagination (#1062). + * + * @return iterable */ - public function getLayoutNames( int $limit, int $offset = 0 ): array; - - public function getLayoutCount(): int; + public function getReadableLayoutNames( int $afterPageId = 0 ): iterable; } diff --git a/src/Persistence/MappingNameLookup.php b/src/Persistence/MappingNameLookup.php index 2d0a5bd2..e3f33ca1 100644 --- a/src/Persistence/MappingNameLookup.php +++ b/src/Persistence/MappingNameLookup.php @@ -9,10 +9,21 @@ interface MappingNameLookup { /** - * The names of every Mapping page, used to enumerate all Mappings. + * The names of every Mapping page, used to enumerate all Mappings. Unfiltered: the RDF-projection + * and DumpRdf path runs in a system context and must see every Mapping. * * @return MappingName[] */ public function getMappingNames(): array; + /** + * The Mapping names the caller may read, keyed by page ID, in page-ID order, starting after the + * given page ID. The summaries endpoint fills its page from this iterable and uses the keys as + * pagination cursor, so an unreadable Mapping neither appears, nor takes page space, nor is + * inferable from the pagination (#1062). + * + * @return iterable + */ + public function getReadableMappingNames( int $afterPageId = 0 ): iterable; + } diff --git a/src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php b/src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php index 5c47ad0a..762ca424 100644 --- a/src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php +++ b/src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php @@ -4,61 +4,61 @@ namespace ProfessionalWiki\NeoWiki\Persistence\MediaWiki; +use MediaWiki\Title\TitleFactory; +use MediaWiki\Title\TitleValue; +use ProfessionalWiki\NeoWiki\Application\PageReadAuthorizer; use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Persistence\LayoutNameLookup; -use MediaWiki\Title\TitleValue; use Wikimedia\Rdbms\IDatabase; -use Wikimedia\Rdbms\IResultWrapper; class DatabaseLayoutNameLookup implements LayoutNameLookup { + private const READABLE_NAMES_BATCH_SIZE = 100; + public function __construct( private readonly IDatabase $db, + private readonly PageReadAuthorizer $readAuthorizer, + private readonly TitleFactory $titleFactory, ) { } /** - * @return TitleValue[] + * Keyset pagination over page_id: each batch seeks past the last seen page ID instead of + * re-walking the namespace, so a request costs the batches it consumes, not the whole + * namespace, and pages stay stable when Layouts are created or deleted between requests. + * The raw DB query applies no visibility rules, so the per-title read check here is the sole + * read gate on the Layout listing; an unreadable Layout is never yielded and a cursor built + * from the yielded keys pages over readable Layouts only (#1062). + * + * @return iterable Readable Layout names keyed by page ID, in page-ID order. */ - public function getLayoutNames( int $limit, int $offset = 0 ): array { - $res = $this->db->select( - 'page', - [ 'page_title' ], - [ 'page_namespace' => NeoWikiExtension::NS_LAYOUT ], - __METHOD__, - [ - 'ORDER BY' => 'page_id ASC', - 'LIMIT' => $limit, - 'OFFSET' => $offset, - ] - ); - - return $this->dbResultToTitleValueArray( $res ); - } + public function getReadableLayoutNames( int $afterPageId = 0 ): iterable { + $lastPageId = $afterPageId; - public function getLayoutCount(): int { - /** @var string $count */ - $count = $this->db->selectField( - 'page', - 'COUNT(*)', - [ 'page_namespace' => NeoWikiExtension::NS_LAYOUT ], - __METHOD__ - ); - - return (int)$count; - } - - /** - * @return TitleValue[] - */ - private function dbResultToTitleValueArray( IResultWrapper $result ): array { - $titles = []; + do { + $res = $this->db->select( + 'page', + [ 'page_id', 'page_title' ], + [ + 'page_namespace' => NeoWikiExtension::NS_LAYOUT, + $this->db->expr( 'page_id', '>', $lastPageId ), + ], + __METHOD__, + [ + 'ORDER BY' => 'page_id ASC', + 'LIMIT' => self::READABLE_NAMES_BATCH_SIZE, + ] + ); - foreach ( $result as $row ) { - $titles[] = new TitleValue( NeoWikiExtension::NS_LAYOUT, $row->page_title ); - } + foreach ( $res as $row ) { + $lastPageId = (int)$row->page_id; + $title = new TitleValue( NeoWikiExtension::NS_LAYOUT, $row->page_title ); - return $titles; + if ( $this->readAuthorizer->authorizeReadByPageTitle( $this->titleFactory->newFromLinkTarget( $title ) ) ) { + yield $lastPageId => $title; + } + } + } while ( $res->numRows() === self::READABLE_NAMES_BATCH_SIZE ); } } diff --git a/src/Persistence/MediaWiki/DatabaseMappingNameLookup.php b/src/Persistence/MediaWiki/DatabaseMappingNameLookup.php index 97e0ee3e..8faccdf1 100644 --- a/src/Persistence/MediaWiki/DatabaseMappingNameLookup.php +++ b/src/Persistence/MediaWiki/DatabaseMappingNameLookup.php @@ -5,6 +5,8 @@ namespace ProfessionalWiki\NeoWiki\Persistence\MediaWiki; use InvalidArgumentException; +use MediaWiki\Title\TitleFactory; +use ProfessionalWiki\NeoWiki\Application\PageReadAuthorizer; use ProfessionalWiki\NeoWiki\Domain\Mapping\MappingName; use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Persistence\MappingNameLookup; @@ -12,8 +14,12 @@ class DatabaseMappingNameLookup implements MappingNameLookup { + private const READABLE_NAMES_BATCH_SIZE = 100; + public function __construct( private readonly IDatabase $db, + private readonly PageReadAuthorizer $readAuthorizer, + private readonly TitleFactory $titleFactory, ) { } @@ -47,4 +53,49 @@ public function getMappingNames(): array { return $names; } + /** + * Keyset pagination over page_id, mirroring the Schema and Layout name lookups: each batch + * seeks past the last seen page ID, an unreadable Mapping is never yielded, and a cursor built + * from the yielded keys pages over readable Mappings only (#1062). getMappingNames stays + * unfiltered for the system-context RDF-projection and DumpRdf path, mirroring the split + * between {@see NeoWikiExtension::getRdfProjectionNames} and + * NeoWikiExtension::filterReadableProjectionNames. A page whose title is not a usable Mapping + * name is skipped like in getMappingNames. + * + * @return iterable Readable Mapping names keyed by page ID, in page-ID order. + */ + public function getReadableMappingNames( int $afterPageId = 0 ): iterable { + $lastPageId = $afterPageId; + + do { + $res = $this->db->newSelectQueryBuilder() + ->select( [ 'page_id', 'page_title' ] ) + ->from( 'page' ) + ->where( [ + 'page_namespace' => NeoWikiExtension::NS_MAPPING, + $this->db->expr( 'page_id', '>', $lastPageId ), + ] ) + ->orderBy( 'page_id ASC' ) + ->limit( self::READABLE_NAMES_BATCH_SIZE ) + ->caller( __METHOD__ ) + ->fetchResultSet(); + + foreach ( $res as $row ) { + $lastPageId = (int)$row->page_id; + + try { + $name = new MappingName( str_replace( '_', ' ', $row->page_title ) ); + } catch ( InvalidArgumentException ) { + continue; + } + + $title = $this->titleFactory->newFromText( $name->getText(), NeoWikiExtension::NS_MAPPING ); + + if ( $title !== null && $this->readAuthorizer->authorizeReadByPageTitle( $title ) ) { + yield $lastPageId => $name; + } + } + } while ( $res->numRows() === self::READABLE_NAMES_BATCH_SIZE ); + } + } diff --git a/src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php b/src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php index 5699b544..483ccbe4 100644 --- a/src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php +++ b/src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php @@ -16,6 +16,8 @@ class DatabaseSchemaNameLookup implements SchemaNameLookup { + private const READABLE_NAMES_BATCH_SIZE = 100; + public function __construct( private readonly IDatabase $db, private readonly SearchEngine $searchEngine, @@ -54,16 +56,42 @@ private function filterReadable( array $titles ): array { ) ); } - public function getSchemaCount(): int { - /** @var string $count */ - $count = $this->db->selectField( - 'page', - 'COUNT(*)', - [ 'page_namespace' => NeoWikiExtension::NS_SCHEMA ], - __METHOD__ - ); - - return (int)$count; + /** + * Keyset pagination over page_id: each batch seeks past the last seen page ID instead of + * re-walking the namespace, so a request costs the batches it consumes, not the whole + * namespace, and pages stay stable when Schemas are created or deleted between requests. + * Unreadable Schemas are never yielded, so a cursor built from the yielded keys pages over + * readable Schemas only (#1062). + * + * @return iterable Readable Schema names keyed by page ID, in page-ID order. + */ + public function getReadableSchemaNames( int $afterPageId = 0 ): iterable { + $lastPageId = $afterPageId; + + do { + $res = $this->db->select( + 'page', + [ 'page_id', 'page_title' ], + [ + 'page_namespace' => NeoWikiExtension::NS_SCHEMA, + $this->db->expr( 'page_id', '>', $lastPageId ), + ], + __METHOD__, + [ + 'ORDER BY' => 'page_id ASC', + 'LIMIT' => self::READABLE_NAMES_BATCH_SIZE, + ] + ); + + foreach ( $res as $row ) { + $lastPageId = (int)$row->page_id; + $title = new TitleValue( NeoWikiExtension::NS_SCHEMA, $row->page_title ); + + if ( $this->readAuthorizer->authorizeReadByPageTitle( $this->titleFactory->newFromLinkTarget( $title ) ) ) { + yield $lastPageId => $title; + } + } + } while ( $res->numRows() === self::READABLE_NAMES_BATCH_SIZE ); } private function getSearchSuggestions( string $search, int $limit, int $offset ): SearchSuggestionSet { diff --git a/src/Persistence/SchemaNameLookup.php b/src/Persistence/SchemaNameLookup.php index 7660daee..3fc49672 100644 --- a/src/Persistence/SchemaNameLookup.php +++ b/src/Persistence/SchemaNameLookup.php @@ -11,6 +11,14 @@ interface SchemaNameLookup { */ public function getSchemaNamesMatching( string $search, int $limit, int $offset = 0 ): array; - public function getSchemaCount(): int; + /** + * The Schema names the caller may read, keyed by page ID, in page-ID order, starting after the + * given page ID. The summaries endpoint fills its page from this iterable and uses the keys as + * pagination cursor, so an unreadable Schema neither appears, nor takes page space, nor is + * inferable from the pagination (#1062). + * + * @return iterable + */ + public function getReadableSchemaNames( int $afterPageId = 0 ): iterable; } diff --git a/tests/phpunit/EntryPoints/REST/GetLayoutSummariesApiTest.php b/tests/phpunit/EntryPoints/REST/GetLayoutSummariesApiTest.php new file mode 100644 index 00000000..0a0224e6 --- /dev/null +++ b/tests/phpunit/EntryPoints/REST/GetLayoutSummariesApiTest.php @@ -0,0 +1,142 @@ +executeHandler( + new GetLayoutSummariesApi(), + new RequestData( [ 'method' => 'GET' ] ) + ); + + $this->assertSame( 200, $response->getStatusCode() ); + + $data = json_decode( $response->getBody()->getContents(), true ); + + $this->assertSame( [], $data['layouts'] ); + $this->assertNull( $data['nextCursor'] ); + } + + public function testReturnsLayoutSummaries(): void { + $this->createLayout( + 'Company card', + '{ "schema": "Company", "type": "infobox", "description": "A company layout", "displayRules": [] }' + ); + $this->createLayout( + 'Person card', + '{ "schema": "Person", "type": "infobox", "description": "A person layout", "displayRules": [] }' + ); + + $response = $this->executeHandler( + new GetLayoutSummariesApi(), + new RequestData( [ 'method' => 'GET' ] ) + ); + + $this->assertSame( 200, $response->getStatusCode() ); + + $data = json_decode( $response->getBody()->getContents(), true ); + + $this->assertCount( 2, $data['layouts'] ); + $this->assertNull( $data['nextCursor'] ); + + $byName = []; + foreach ( $data['layouts'] as $summary ) { + $byName[$summary['name']] = $summary; + } + + $this->assertSame( 'Company', $byName['Company card']['schema'] ); + $this->assertSame( 'infobox', $byName['Company card']['type'] ); + $this->assertSame( 'A company layout', $byName['Company card']['description'] ); + $this->assertSame( 0, $byName['Company card']['ruleCount'] ); + } + + public function testFollowingTheCursorWalksAllPages(): void { + $this->createLayout( 'Alpha', '{ "schema": "Alpha", "type": "infobox" }' ); + $this->createLayout( 'Beta', '{ "schema": "Beta", "type": "infobox" }' ); + $this->createLayout( 'Gamma', '{ "schema": "Gamma", "type": "infobox" }' ); + + $firstPage = json_decode( $this->executeHandler( + new GetLayoutSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'limit' => '2' ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertSame( [ 'Alpha', 'Beta' ], array_column( $firstPage['layouts'], 'name' ) ); + $this->assertIsString( $firstPage['nextCursor'] ); + + $secondPage = json_decode( $this->executeHandler( + new GetLayoutSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'limit' => '2', 'cursor' => $firstPage['nextCursor'] ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertSame( [ 'Gamma' ], array_column( $secondPage['layouts'], 'name' ) ); + $this->assertNull( $secondPage['nextCursor'] ); + } + + public function testRejectsMalformedCursor(): void { + $this->expectException( HttpException::class ); + $this->expectExceptionCode( 400 ); + + $this->executeHandler( + new GetLayoutSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'cursor' => 'not-a-cursor' ], + ] ) + ); + } + + public function testExcludesLayoutsTheRequestUserCannotReadWithoutLeavingAGapInThePage(): void { + // End-to-end guard for the #1062 count oracle: a Layout the request user may not read is + // skipped and a readable one after it fills its slot, so the page carries no trace of the + // restricted Layout. This exercises the handler's getRequestAuthority wiring, which the + // persistence-layer tests bypass by injecting an authority directly. + $this->createLayout( 'ReadableLayout', '{ "schema": "ReadableLayout", "type": "infobox" }' ); + $this->createLayout( 'RestrictedLayout', '{ "schema": "RestrictedLayout", "type": "infobox" }' ); + $this->createLayout( 'TrailingLayout', '{ "schema": "TrailingLayout", "type": "infobox" }' ); + + RequestContext::getMain()->setUser( $this->getTestUser()->getUser() ); + $this->setTemporaryHook( + 'getUserPermissionsErrors', + static function ( $title, $user, $action, &$result ): bool { + if ( $action === 'read' && $title->getDBkey() === 'RestrictedLayout' ) { + $result = [ 'badaccess-group0' ]; + return false; + } + return true; + } + ); + + $data = json_decode( $this->executeHandler( + new GetLayoutSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'limit' => '2' ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertSame( [ 'ReadableLayout', 'TrailingLayout' ], array_column( $data['layouts'], 'name' ) ); + $this->assertNull( $data['nextCursor'] ); + } + +} diff --git a/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php b/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php index 015db3fc..48c1bd16 100644 --- a/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php +++ b/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php @@ -4,6 +4,8 @@ namespace ProfessionalWiki\NeoWiki\Tests\EntryPoints\REST; +use MediaWiki\Context\RequestContext; +use MediaWiki\Rest\HttpException; use MediaWiki\Rest\RequestData; use MediaWiki\Tests\Rest\Handler\HandlerTestTrait; use ProfessionalWiki\NeoWiki\EntryPoints\REST\GetMappingSummariesApi; @@ -27,7 +29,7 @@ public function testReturnsEmptyResultWhenNoMappings(): void { $data = json_decode( $response->getBody()->getContents(), true ); $this->assertSame( [], $data['mappings'] ); - $this->assertSame( 0, $data['totalRows'] ); + $this->assertNull( $data['nextCursor'] ); } public function testReturnsMappingSummariesWithAlphabeticallySortedMappedSchemaNames(): void { @@ -66,8 +68,8 @@ public function testReturnsMappingSummariesWithAlphabeticallySortedMappedSchemaN $data = json_decode( $response->getBody()->getContents(), true ); - $this->assertSame( 2, $data['totalRows'] ); $this->assertCount( 2, $data['mappings'] ); + $this->assertNull( $data['nextCursor'] ); $byName = []; foreach ( $data['mappings'] as $summary ) { @@ -78,44 +80,78 @@ public function testReturnsMappingSummariesWithAlphabeticallySortedMappedSchemaN $this->assertSame( [ 'Manuscript' ], $byName['Dublin Core']['schemas'] ); } - public function testPaginationLimitsResults(): void { + public function testFollowingTheCursorWalksAllPages(): void { $this->createMapping( 'Alpha', '{"version":1,"schemas":{}}' ); $this->createMapping( 'Beta', '{"version":1,"schemas":{}}' ); $this->createMapping( 'Gamma', '{"version":1,"schemas":{}}' ); - $response = $this->executeHandler( + $firstPage = json_decode( $this->executeHandler( new GetMappingSummariesApi(), new RequestData( [ 'method' => 'GET', - 'queryParams' => [ 'limit' => '2', 'offset' => '0' ], + 'queryParams' => [ 'limit' => '2' ], ] ) - ); + )->getBody()->getContents(), true ); - $data = json_decode( $response->getBody()->getContents(), true ); + $this->assertSame( [ 'Alpha', 'Beta' ], array_column( $firstPage['mappings'], 'name' ) ); + $this->assertIsString( $firstPage['nextCursor'] ); - $this->assertCount( 2, $data['mappings'] ); - $this->assertSame( 3, $data['totalRows'] ); + $secondPage = json_decode( $this->executeHandler( + new GetMappingSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'limit' => '2', 'cursor' => $firstPage['nextCursor'] ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertSame( [ 'Gamma' ], array_column( $secondPage['mappings'], 'name' ) ); + $this->assertNull( $secondPage['nextCursor'] ); } - public function testPaginationOffset(): void { - $this->createMapping( 'Alpha', '{"version":1,"schemas":{}}' ); - $this->createMapping( 'Beta', '{"version":1,"schemas":{}}' ); - $this->createMapping( 'Gamma', '{"version":1,"schemas":{}}' ); + public function testRejectsMalformedCursor(): void { + $this->expectException( HttpException::class ); + $this->expectExceptionCode( 400 ); - $response = $this->executeHandler( + $this->executeHandler( new GetMappingSummariesApi(), new RequestData( [ 'method' => 'GET', - 'queryParams' => [ 'limit' => '2', 'offset' => '1' ], + 'queryParams' => [ 'cursor' => 'not-a-cursor' ], ] ) ); + } - $data = json_decode( $response->getBody()->getContents(), true ); + public function testExcludesMappingsTheRequestUserCannotReadWithoutLeavingAGapInThePage(): void { + // End-to-end guard for the #1062 count oracle: a Mapping the request user may not read is + // skipped and a readable one after it fills its slot, so the page carries no trace of the + // restricted Mapping. This exercises the handler's getRequestAuthority wiring, which the + // persistence-layer tests bypass by injecting an authority directly. + $this->createMapping( 'ReadableMapping', '{"version":1,"schemas":{}}' ); + $this->createMapping( 'RestrictedMapping', '{"version":1,"schemas":{}}' ); + $this->createMapping( 'TrailingMapping', '{"version":1,"schemas":{}}' ); + + RequestContext::getMain()->setUser( $this->getTestUser()->getUser() ); + $this->setTemporaryHook( + 'getUserPermissionsErrors', + static function ( $title, $user, $action, &$result ): bool { + if ( $action === 'read' && $title->getDBkey() === 'RestrictedMapping' ) { + $result = [ 'badaccess-group0' ]; + return false; + } + return true; + } + ); - $this->assertCount( 2, $data['mappings'] ); - $this->assertSame( 'Beta', $data['mappings'][0]['name'] ); - $this->assertSame( 'Gamma', $data['mappings'][1]['name'] ); - $this->assertSame( 3, $data['totalRows'] ); + $data = json_decode( $this->executeHandler( + new GetMappingSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'limit' => '2' ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertSame( [ 'ReadableMapping', 'TrailingMapping' ], array_column( $data['mappings'], 'name' ) ); + $this->assertNull( $data['nextCursor'] ); } } diff --git a/tests/phpunit/EntryPoints/REST/GetSchemaSummariesApiTest.php b/tests/phpunit/EntryPoints/REST/GetSchemaSummariesApiTest.php index b43825fb..1d653741 100644 --- a/tests/phpunit/EntryPoints/REST/GetSchemaSummariesApiTest.php +++ b/tests/phpunit/EntryPoints/REST/GetSchemaSummariesApiTest.php @@ -4,6 +4,8 @@ namespace ProfessionalWiki\NeoWiki\Tests\EntryPoints\REST; +use MediaWiki\Context\RequestContext; +use MediaWiki\Rest\HttpException; use MediaWiki\Rest\RequestData; use MediaWiki\Tests\Rest\Handler\HandlerTestTrait; use ProfessionalWiki\NeoWiki\EntryPoints\REST\GetSchemaSummariesApi; @@ -27,7 +29,7 @@ public function testReturnsEmptyResultWhenNoSchemas(): void { $data = json_decode( $response->getBody()->getContents(), true ); $this->assertSame( [], $data['schemas'] ); - $this->assertSame( 0, $data['totalRows'] ); + $this->assertNull( $data['nextCursor'] ); } public function testReturnsSchemaSummaries(): void { @@ -63,8 +65,8 @@ public function testReturnsSchemaSummaries(): void { $data = json_decode( $response->getBody()->getContents(), true ); - $this->assertSame( 2, $data['totalRows'] ); $this->assertCount( 2, $data['schemas'] ); + $this->assertNull( $data['nextCursor'] ); $byName = []; foreach ( $data['schemas'] as $summary ) { @@ -78,44 +80,119 @@ public function testReturnsSchemaSummaries(): void { $this->assertSame( 1, $byName['Person']['propertyCount'] ); } - public function testPaginationLimitsResults(): void { + public function testFollowingTheCursorWalksAllPages(): void { $this->createSchema( 'Alpha', '{"title":"Alpha","description":"First","propertyDefinitions":{}}' ); $this->createSchema( 'Beta', '{"title":"Beta","description":"Second","propertyDefinitions":{}}' ); $this->createSchema( 'Gamma', '{"title":"Gamma","description":"Third","propertyDefinitions":{}}' ); - $response = $this->executeHandler( + $firstPage = json_decode( $this->executeHandler( new GetSchemaSummariesApi(), new RequestData( [ 'method' => 'GET', - 'queryParams' => [ 'limit' => '2', 'offset' => '0' ], + 'queryParams' => [ 'limit' => '2' ], ] ) - ); + )->getBody()->getContents(), true ); - $data = json_decode( $response->getBody()->getContents(), true ); + $this->assertSame( [ 'Alpha', 'Beta' ], array_column( $firstPage['schemas'], 'name' ) ); + $this->assertIsString( $firstPage['nextCursor'] ); - $this->assertCount( 2, $data['schemas'] ); - $this->assertSame( 3, $data['totalRows'] ); + $secondPage = json_decode( $this->executeHandler( + new GetSchemaSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'limit' => '2', 'cursor' => $firstPage['nextCursor'] ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertSame( [ 'Gamma' ], array_column( $secondPage['schemas'], 'name' ) ); + $this->assertNull( $secondPage['nextCursor'] ); } - public function testPaginationOffset(): void { + public function testExactPageBoundaryEndsPagination(): void { $this->createSchema( 'Alpha', '{"title":"Alpha","description":"First","propertyDefinitions":{}}' ); $this->createSchema( 'Beta', '{"title":"Beta","description":"Second","propertyDefinitions":{}}' ); - $this->createSchema( 'Gamma', '{"title":"Gamma","description":"Third","propertyDefinitions":{}}' ); - $response = $this->executeHandler( + $data = json_decode( $this->executeHandler( + new GetSchemaSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'limit' => '2' ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertCount( 2, $data['schemas'] ); + $this->assertNull( $data['nextCursor'] ); + } + + /** + * @dataProvider cursorPastEndProvider + */ + public function testCursorPastTheEndReturnsAnEmptyLastPage( string $cursor ): void { + $this->createSchema( 'Alpha', '{"title":"Alpha","description":"First","propertyDefinitions":{}}' ); + + $data = json_decode( $this->executeHandler( new GetSchemaSummariesApi(), new RequestData( [ 'method' => 'GET', - 'queryParams' => [ 'limit' => '2', 'offset' => '1' ], + 'queryParams' => [ 'cursor' => $cursor ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertSame( [], $data['schemas'] ); + $this->assertNull( $data['nextCursor'] ); + } + + public static function cursorPastEndProvider(): array { + return [ + 'past the last page id' => [ '999999' ], + 'beyond integer range (saturates)' => [ '99999999999999999999999' ], + ]; + } + + public function testRejectsMalformedCursor(): void { + $this->expectException( HttpException::class ); + $this->expectExceptionCode( 400 ); + + $this->executeHandler( + new GetSchemaSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'cursor' => 'not-a-cursor' ], ] ) ); + } - $data = json_decode( $response->getBody()->getContents(), true ); + public function testExcludesSchemasTheRequestUserCannotReadWithoutLeavingAGapInThePage(): void { + // End-to-end guard for the #1062 count oracle: a Schema the request user may not read is + // skipped and a readable one after it fills its slot, so the page carries no trace of the + // restricted Schema. This exercises the handler's getRequestAuthority wiring, which the + // persistence-layer tests bypass by injecting an authority directly. + $this->createSchema( 'ReadableSchema', '{"title":"ReadableSchema","description":"","propertyDefinitions":{}}' ); + $this->createSchema( 'RestrictedSchema', '{"title":"RestrictedSchema","description":"","propertyDefinitions":{}}' ); + $this->createSchema( 'TrailingSchema', '{"title":"TrailingSchema","description":"","propertyDefinitions":{}}' ); + + RequestContext::getMain()->setUser( $this->getTestUser()->getUser() ); + $this->setTemporaryHook( + 'getUserPermissionsErrors', + static function ( $title, $user, $action, &$result ): bool { + if ( $action === 'read' && $title->getDBkey() === 'RestrictedSchema' ) { + $result = [ 'badaccess-group0' ]; + return false; + } + return true; + } + ); - $this->assertCount( 2, $data['schemas'] ); - $this->assertSame( 'Beta', $data['schemas'][0]['name'] ); - $this->assertSame( 'Gamma', $data['schemas'][1]['name'] ); - $this->assertSame( 3, $data['totalRows'] ); + $data = json_decode( $this->executeHandler( + new GetSchemaSummariesApi(), + new RequestData( [ + 'method' => 'GET', + 'queryParams' => [ 'limit' => '2' ], + ] ) + )->getBody()->getContents(), true ); + + $this->assertSame( [ 'ReadableSchema', 'TrailingSchema' ], array_column( $data['schemas'], 'name' ) ); + $this->assertNull( $data['nextCursor'] ); } } diff --git a/tests/phpunit/NeoWikiIntegrationTestCase.php b/tests/phpunit/NeoWikiIntegrationTestCase.php index 326e3e0f..f1cbd485 100644 --- a/tests/phpunit/NeoWikiIntegrationTestCase.php +++ b/tests/phpunit/NeoWikiIntegrationTestCase.php @@ -18,6 +18,7 @@ use ProfessionalWiki\NeoWiki\Domain\Page\PageSubjects; use ProfessionalWiki\NeoWiki\Domain\Subject\Subject; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectMap; +use ProfessionalWiki\NeoWiki\EntryPoints\Content\LayoutContent; use ProfessionalWiki\NeoWiki\EntryPoints\Content\MappingContent; use ProfessionalWiki\NeoWiki\EntryPoints\Content\SchemaContent; use ProfessionalWiki\NeoWiki\EntryPoints\Content\SubjectContent; @@ -81,6 +82,23 @@ protected function createSchema( string $name, string $json = null ): ?RevisionR return $updater->saveRevision( CommentStoreComment::newUnsavedComment( 'TODO' ) ); } + protected function createLayout( string $name, ?string $json = null ): ?RevisionRecord { + $wikiPage = MediaWikiServices::getInstance()->getWikiPageFactory()->newFromTitle( + Title::newFromText( $name, NeoWikiExtension::NS_LAYOUT ) + ); + + $updater = $wikiPage->newPageUpdater( $this->getTestSysop()->getUser() ); + + $updater->setContent( + 'main', + new LayoutContent( + $json ?? '{ "schema": "' . $name . '", "type": "infobox" }' + ) + ); + + return $updater->saveRevision( CommentStoreComment::newUnsavedComment( 'TODO' ) ); + } + protected function createMapping( string $name, string $json ): ?RevisionRecord { $wikiPage = MediaWikiServices::getInstance()->getWikiPageFactory()->newFromTitle( Title::newFromText( $name, NeoWikiExtension::NS_MAPPING ) diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php new file mode 100644 index 00000000..460feb5d --- /dev/null +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php @@ -0,0 +1,99 @@ + + */ + private array $pageIds = []; + + public function setUp(): void { + $this->tablesUsed[] = 'page'; + $this->truncateTables( $this->tablesUsed, $this->db ); + + foreach ( [ 'LayoutNameLookupTest1', 'LayoutNameLookupTest2', 'LayoutNameLookupTest3' ] as $name ) { + $this->pageIds[$name] = $this->createLayout( $name )->getPageId(); + } + } + + private function getLookup( ?Authority $authority = null ): DatabaseLayoutNameLookup { + return new DatabaseLayoutNameLookup( + db: $this->getDb(), + readAuthorizer: new AuthorityBasedPageReadAuthorizer( + $authority ?? $this->mockRegisteredUltimateAuthority(), + $this->getServiceContainer()->getTitleFactory(), + new NullLogger() + ), + titleFactory: $this->getServiceContainer()->getTitleFactory(), + ); + } + + public function testGetReadableLayoutNamesYieldsEveryLayoutKeyedByPageId(): void { + $this->assertSame( + [ + $this->pageIds['LayoutNameLookupTest1'] => 'LayoutNameLookupTest1', + $this->pageIds['LayoutNameLookupTest2'] => 'LayoutNameLookupTest2', + $this->pageIds['LayoutNameLookupTest3'] => 'LayoutNameLookupTest3', + ], + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( $this->getLookup()->getReadableLayoutNames() ) + ) + ); + } + + public function testGetReadableLayoutNamesStartsAfterTheGivenPageId(): void { + $this->assertSame( + [ + $this->pageIds['LayoutNameLookupTest3'] => 'LayoutNameLookupTest3', + ], + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( + $this->getLookup()->getReadableLayoutNames( $this->pageIds['LayoutNameLookupTest2'] ) + ) + ) + ); + } + + public function testGetReadableLayoutNamesOmitsUnreadableLayouts(): void { + // The denied Layout sits mid-list. It must not be yielded at all: the summaries endpoint + // fills its page from this iterable and builds its cursor from the yielded keys, so a + // skipped Layout neither takes page space nor is inferable from the pagination (#1062). + $denyHidden = static fn ( string $permission, ?PageIdentity $page = null ): bool => + $page === null || $page->getDBkey() !== 'LayoutNameLookupTest2'; + + $this->assertSame( + [ + $this->pageIds['LayoutNameLookupTest1'] => 'LayoutNameLookupTest1', + $this->pageIds['LayoutNameLookupTest3'] => 'LayoutNameLookupTest3', + ], + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( + $this->getLookup( $this->mockRegisteredAuthority( $denyHidden ) )->getReadableLayoutNames() + ) + ) + ); + } + +} diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php new file mode 100644 index 00000000..561e05b7 --- /dev/null +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php @@ -0,0 +1,116 @@ + + */ + private array $pageIds = []; + + public function setUp(): void { + $this->tablesUsed[] = 'page'; + $this->truncateTables( $this->tablesUsed, $this->db ); + + foreach ( [ 'MappingLookupTest1', 'MappingLookupTest2', 'MappingLookupTest3' ] as $name ) { + $this->pageIds[$name] = $this->createMapping( $name, '{"version":1,"schemas":{}}' )->getPageId(); + } + } + + private function getLookup( ?Authority $authority = null ): DatabaseMappingNameLookup { + return new DatabaseMappingNameLookup( + db: $this->getDb(), + readAuthorizer: new AuthorityBasedPageReadAuthorizer( + $authority ?? $this->mockRegisteredUltimateAuthority(), + $this->getServiceContainer()->getTitleFactory(), + new NullLogger() + ), + titleFactory: $this->getServiceContainer()->getTitleFactory(), + ); + } + + public function testGetMappingNamesReturnsEveryMappingUnfiltered(): void { + // getMappingNames stays unfiltered for the RDF-projection / DumpRdf path, which runs in a + // system context and must see every Mapping regardless of a request user's read rights. + $denyHidden = static fn ( string $permission, ?PageIdentity $page = null ): bool => + $page === null || $page->getDBkey() !== 'MappingLookupTest2'; + + $names = array_map( + static fn ( MappingName $name ): string => $name->getText(), + $this->getLookup( $this->mockRegisteredAuthority( $denyHidden ) )->getMappingNames() + ); + + $this->assertSame( + [ 'MappingLookupTest1', 'MappingLookupTest2', 'MappingLookupTest3' ], + $names + ); + } + + public function testGetReadableMappingNamesYieldsEveryMappingKeyedByPageId(): void { + $this->assertSame( + [ + $this->pageIds['MappingLookupTest1'] => 'MappingLookupTest1', + $this->pageIds['MappingLookupTest2'] => 'MappingLookupTest2', + $this->pageIds['MappingLookupTest3'] => 'MappingLookupTest3', + ], + array_map( + static fn ( MappingName $name ): string => $name->getText(), + iterator_to_array( $this->getLookup()->getReadableMappingNames() ) + ) + ); + } + + public function testGetReadableMappingNamesStartsAfterTheGivenPageId(): void { + $this->assertSame( + [ + $this->pageIds['MappingLookupTest3'] => 'MappingLookupTest3', + ], + array_map( + static fn ( MappingName $name ): string => $name->getText(), + iterator_to_array( + $this->getLookup()->getReadableMappingNames( $this->pageIds['MappingLookupTest2'] ) + ) + ) + ); + } + + public function testGetReadableMappingNamesOmitsUnreadableMappings(): void { + // The denied Mapping sits mid-list. It must not be yielded at all: the summaries endpoint + // fills its page from this iterable and builds its cursor from the yielded keys, so a + // skipped Mapping neither takes page space nor is inferable from the pagination (#1062). + $denyHidden = static fn ( string $permission, ?PageIdentity $page = null ): bool => + $page === null || $page->getDBkey() !== 'MappingLookupTest2'; + + $this->assertSame( + [ + $this->pageIds['MappingLookupTest1'] => 'MappingLookupTest1', + $this->pageIds['MappingLookupTest3'] => 'MappingLookupTest3', + ], + array_map( + static fn ( MappingName $name ): string => $name->getText(), + iterator_to_array( + $this->getLookup( $this->mockRegisteredAuthority( $denyHidden ) )->getReadableMappingNames() + ) + ) + ); + } + +} diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php index d6c5cef0..10b076c3 100644 --- a/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php @@ -22,14 +22,18 @@ class DatabaseSchemaNameLookupTest extends NeoWikiIntegrationTestCase { use NeoWikiMockAuthorityTrait; + /** + * @var array + */ + private array $pageIds = []; + public function setUp(): void { $this->tablesUsed[] = 'page'; $this->truncateTables( $this->tablesUsed, $this->db ); - $this->createSchema( 'SchemaNameLookupTest1' ); - $this->createSchema( 'SchemaNameLookupTest21' ); - $this->createSchema( 'SchemaNameLookupTest22' ); - $this->createSchema( 'SchemaNameLookupTest3' ); + foreach ( [ 'SchemaNameLookupTest1', 'SchemaNameLookupTest21', 'SchemaNameLookupTest22', 'SchemaNameLookupTest3' ] as $name ) { + $this->pageIds[$name] = $this->createSchema( $name )->getPageId(); + } } /** @@ -114,8 +118,60 @@ public function testLimitAndOffsetCombined(): void { ); } - public function testGetSchemaCount(): void { - $this->assertSame( 4, $this->getLookup()->getSchemaCount() ); + public function testGetReadableSchemaNamesYieldsEverySchemaKeyedByPageId(): void { + $this->assertSame( + [ + $this->pageIds['SchemaNameLookupTest1'] => 'SchemaNameLookupTest1', + $this->pageIds['SchemaNameLookupTest21'] => 'SchemaNameLookupTest21', + $this->pageIds['SchemaNameLookupTest22'] => 'SchemaNameLookupTest22', + $this->pageIds['SchemaNameLookupTest3'] => 'SchemaNameLookupTest3', + ], + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( $this->getLookup()->getReadableSchemaNames() ) + ) + ); + } + + public function testGetReadableSchemaNamesStartsAfterTheGivenPageId(): void { + $this->assertSame( + [ + $this->pageIds['SchemaNameLookupTest22'] => 'SchemaNameLookupTest22', + $this->pageIds['SchemaNameLookupTest3'] => 'SchemaNameLookupTest3', + ], + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( + $this->getLookup()->getReadableSchemaNames( $this->pageIds['SchemaNameLookupTest21'] ) + ) + ) + ); + } + + public function testGetReadableSchemaNamesOmitsUnreadableSchemas(): void { + // GateHiddenSchema is created before GateVisibleSchema so the denied row sits mid-list. A + // denied Schema must not be yielded at all: the summaries endpoint fills its page from this + // iterable and builds its cursor from the yielded keys, so a skipped Schema neither takes + // page space nor becomes inferable from the pagination (#1062). + $this->createSchema( 'GateHiddenSchema' ); + $visibleId = $this->createSchema( 'GateVisibleSchema' )->getPageId(); + + $denyHidden = static fn ( string $permission, ?PageIdentity $page = null ): bool => + $page === null || $page->getDBkey() !== 'GateHiddenSchema'; + + $this->assertSame( + [ + $this->pageIds['SchemaNameLookupTest1'] => 'SchemaNameLookupTest1', + $this->pageIds['SchemaNameLookupTest21'] => 'SchemaNameLookupTest21', + $this->pageIds['SchemaNameLookupTest22'] => 'SchemaNameLookupTest22', + $this->pageIds['SchemaNameLookupTest3'] => 'SchemaNameLookupTest3', + $visibleId => 'GateVisibleSchema', + ], + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( $this->getLookup( $this->mockRegisteredAuthority( $denyHidden ) )->getReadableSchemaNames() ) + ) + ); } public function testUnreadableSchemaNamesAreOmitted(): void { From 03aafe945bc6e93e46fcc6ef962c0f3bd3362485 Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Wed, 22 Jul 2026 16:35:45 -0400 Subject: [PATCH 02/12] Trim the cursor pagination doc to the contract 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) --- docs/api/rest-api.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/api/rest-api.md b/docs/api/rest-api.md index e1174e7a..faebf434 100644 --- a/docs/api/rest-api.md +++ b/docs/api/rest-api.md @@ -114,10 +114,8 @@ default 10); the response carries the items and a `nextCursor`: { "schemas": [ ... ], "nextCursor": "1462" } ``` -Pass that value back as `cursor` to fetch the next page. `nextCursor` is `null` on the last page; a non-null cursor -means another page follows, though in rare cases (trailing items that fail to load) that page may come back empty. -Treat the cursor as opaque and do not construct one yourself; a malformed cursor is rejected with a `400`. Cursors stay -valid across item creation and deletion, and no total count is reported. +Pass that value back as `cursor` to fetch the next page; `null` marks the last page. Do not construct a cursor +yourself — a malformed one is rejected with a `400`. Cursors stay valid while items are created and deleted. ## The `expand` parameter From cb0fd2b2da1ab1aca5c5c9394d3a99b8237187a1 Mon Sep 17 00:00:00 2001 From: Jeroen De Dauw Date: Thu, 23 Jul 2026 02:14:03 +0200 Subject: [PATCH 03/12] Test cursor back-fill over unloadable list rows 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 --- .../REST/CursorPaginationTrait.php | 5 +- .../REST/GetMappingSummariesApiTest.php | 63 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/EntryPoints/REST/CursorPaginationTrait.php b/src/EntryPoints/REST/CursorPaginationTrait.php index c9910017..a4f8c6ad 100644 --- a/src/EntryPoints/REST/CursorPaginationTrait.php +++ b/src/EntryPoints/REST/CursorPaginationTrait.php @@ -59,8 +59,9 @@ private function pageIdFromCursor( ?string $cursor ): int { /** * Fills a page with up to $limit summaries and derives the follow-up cursor. The cursor is the - * page ID of the last consumed name, so names a load skips (readable but malformed) are not - * re-consumed by the next page. nextCursor is null exactly when the listing is exhausted. + * page ID of the last served item: a name skipped mid-page (readable but malformed) sorts below + * it and is not revisited, while a malformed name after it is scanned and skipped afresh by the + * next page. nextCursor is null exactly when the listing is exhausted. * * @param iterable $readableNames * @param callable(mixed): ?array $loadSummary null when the name does not resolve to a listable item diff --git a/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php b/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php index 48c1bd16..bc1bd732 100644 --- a/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php +++ b/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php @@ -154,4 +154,67 @@ static function ( $title, $user, $action, &$result ): bool { $this->assertNull( $data['nextCursor'] ); } + public function testUnloadableMappingDoesNotConsumePageSpace(): void { + // A readable Mapping whose stored JSON cannot be parsed into a Mapping (here it is missing the + // required "schemas" key) is skipped by the summary loader, and a readable Mapping after it + // fills the freed slot, so the page still returns a full $limit items and reports no more to + // come. Such a page is reachable in production through XML import, which bypasses + // MappingContentHandler::validateSave (#1022). + $this->createMappingsWithUnloadableMiddle(); + + $page = $this->requestMappings( [ 'limit' => '2' ] ); + + $this->assertSame( [ 'Alpha', 'Gamma' ], array_column( $page['mappings'], 'name' ) ); + $this->assertNull( $page['nextCursor'] ); + } + + public function testWalkingPastAnUnloadableMappingNeverYieldsAnEmptyPage(): void { + // Walked one item at a time, the unloadable Mapping is skipped inside the page that reaches it + // rather than served as an empty page with a follow-up cursor: it never appears, and no page in + // the walk comes back empty while items still remain. + $this->createMappingsWithUnloadableMiddle(); + + $firstPage = $this->requestMappings( [ 'limit' => '1' ] ); + + $this->assertSame( [ 'Alpha' ], array_column( $firstPage['mappings'], 'name' ) ); + $this->assertIsString( $firstPage['nextCursor'] ); + + $secondPage = $this->requestMappings( [ 'limit' => '1', 'cursor' => $firstPage['nextCursor'] ] ); + + $this->assertSame( [ 'Gamma' ], array_column( $secondPage['mappings'], 'name' ) ); + $this->assertNull( $secondPage['nextCursor'] ); + } + + /** + * @return array{mappings: list}>, nextCursor: ?string} + */ + private function requestMappings( array $queryParams ): array { + return json_decode( + $this->executeHandler( + new GetMappingSummariesApi(), + new RequestData( [ 'method' => 'GET', 'queryParams' => $queryParams ] ) + )->getBody()->getContents(), + true + ); + } + + /** + * Creates three Mappings in page-ID order — Alpha, an unloadable Beta, then Gamma — so a page + * request has to skip a readable-but-unparseable Mapping sitting between two good ones. Beta's dump + * is derived from Alpha (a real list member) so no extra page pollutes the listing, and its content + * is broken by dropping the required "schemas" key. It goes in through XML import because + * MappingContentHandler::validateSave would reject such content on the edit path (#1022). + */ + private function createMappingsWithUnloadableMiddle(): void { + $this->markPageTableAsUsed(); + $this->createMapping( 'Alpha', '{"version":1,"schemas":{}}' ); + + $xml = $this->exportPageToXml( 'Mapping:Alpha' ); + $xml = str_replace( 'Mapping:Alpha', 'Mapping:Beta', $xml ); + $xml = str_replace( '"schemas"', '"schemaX"', $xml ); + $this->importXml( $xml ); + + $this->createMapping( 'Gamma', '{"version":1,"schemas":{}}' ); + } + } From 0bed228d1e9f5ae8b849f7584a6758e529d62928 Mon Sep 17 00:00:00 2001 From: Jeroen De Dauw Date: Thu, 23 Jul 2026 02:14:10 +0200 Subject: [PATCH 04/12] Test readable-name generators drain across batch boundaries 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 --- tests/phpunit/NeoWikiIntegrationTestCase.php | 47 ++++++++++++++ .../DatabaseLayoutNameLookupTest.php | 14 +++++ .../DatabaseMappingNameLookupTest.php | 14 +++++ .../DatabaseSchemaNameLookupTest.php | 63 +++++++++++++++++++ 4 files changed, 138 insertions(+) diff --git a/tests/phpunit/NeoWikiIntegrationTestCase.php b/tests/phpunit/NeoWikiIntegrationTestCase.php index f1cbd485..fea2144f 100644 --- a/tests/phpunit/NeoWikiIntegrationTestCase.php +++ b/tests/phpunit/NeoWikiIntegrationTestCase.php @@ -148,6 +148,53 @@ protected function markPageTableAsUsed(): void { } } + /** + * Bulk-inserts bare page rows — no revisions or content — straight into the page table with a + * single multi-row insert. The keyset name-lookup generators read only page_id and page_title and + * authorize by Title, so bare rows are enough to drive them past their batch size far more cheaply + * than creating that many real pages. Titles are $titlePrefix followed by a zero-padded counter + * (Foo001, Foo002, …). Returns each created title mapped to its assigned page ID, in page-ID order. + * + * @return array + */ + protected function createBarePages( int $namespace, string $titlePrefix, int $count ): array { + $titles = []; + $rows = []; + + for ( $i = 1; $i <= $count; $i++ ) { + $title = sprintf( '%s%03d', $titlePrefix, $i ); + $titles[] = $title; + $rows[] = [ + 'page_namespace' => $namespace, + 'page_title' => $title, + 'page_random' => 0.5, + 'page_touched' => $this->getDb()->timestamp(), + 'page_latest' => 0, + 'page_len' => 0, + 'page_is_redirect' => 0, + 'page_is_new' => 0, + ]; + } + + $this->getDb()->insert( 'page', $rows, __METHOD__ ); + + $pageIds = []; + + $result = $this->getDb()->newSelectQueryBuilder() + ->select( [ 'page_id', 'page_title' ] ) + ->from( 'page' ) + ->where( [ 'page_namespace' => $namespace, 'page_title' => $titles ] ) + ->orderBy( 'page_id ASC' ) + ->caller( __METHOD__ ) + ->fetchResultSet(); + + foreach ( $result as $row ) { + $pageIds[$row->page_title] = (int)$row->page_id; + } + + return $pageIds; + } + /** * Registers extra graph database plugins through the NeoWikiRegistration hook and rebuilds the singleton * so they are composed into the write paths, letting a test drive the real hook wiring with a backend of diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php index 460feb5d..bf60012f 100644 --- a/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php @@ -8,6 +8,7 @@ use MediaWiki\Permissions\Authority; use MediaWiki\Title\TitleValue; use ProfessionalWiki\NeoWiki\Infrastructure\AuthorityBasedPageReadAuthorizer; +use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\DatabaseLayoutNameLookup; use ProfessionalWiki\NeoWiki\Tests\NeoWikiIntegrationTestCase; use ProfessionalWiki\NeoWiki\Tests\NeoWikiMockAuthorityTrait; @@ -96,4 +97,17 @@ public function testGetReadableLayoutNamesOmitsUnreadableLayouts(): void { ); } + public function testGetReadableLayoutNamesDrainsPastTheBatchSize(): void { + // The generator pages the namespace in 100-row keyset batches. With more rows than one batch, + // it must keep querying past the first batch: every Layout is yielded, the lowest page ID + // first and the highest last. A single truncated batch would drop the tail. + $bulk = $this->createBarePages( NeoWikiExtension::NS_LAYOUT, 'BulkLayout', 120 ); + + $drained = iterator_to_array( $this->getLookup()->getReadableLayoutNames() ); + + $this->assertCount( count( $this->pageIds ) + count( $bulk ), $drained ); + $this->assertSame( $this->pageIds['LayoutNameLookupTest1'], array_key_first( $drained ) ); + $this->assertSame( max( $bulk ), array_key_last( $drained ) ); + } + } diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php index 561e05b7..99f2098f 100644 --- a/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php @@ -8,6 +8,7 @@ use MediaWiki\Permissions\Authority; use ProfessionalWiki\NeoWiki\Domain\Mapping\MappingName; use ProfessionalWiki\NeoWiki\Infrastructure\AuthorityBasedPageReadAuthorizer; +use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\DatabaseMappingNameLookup; use ProfessionalWiki\NeoWiki\Tests\NeoWikiIntegrationTestCase; use ProfessionalWiki\NeoWiki\Tests\NeoWikiMockAuthorityTrait; @@ -113,4 +114,17 @@ public function testGetReadableMappingNamesOmitsUnreadableMappings(): void { ); } + public function testGetReadableMappingNamesDrainsPastTheBatchSize(): void { + // The generator pages the namespace in 100-row keyset batches. With more rows than one batch, + // it must keep querying past the first batch: every Mapping is yielded, the lowest page ID + // first and the highest last. A single truncated batch would drop the tail. + $bulk = $this->createBarePages( NeoWikiExtension::NS_MAPPING, 'BulkMapping', 120 ); + + $drained = iterator_to_array( $this->getLookup()->getReadableMappingNames() ); + + $this->assertCount( count( $this->pageIds ) + count( $bulk ), $drained ); + $this->assertSame( $this->pageIds['MappingLookupTest1'], array_key_first( $drained ) ); + $this->assertSame( max( $bulk ), array_key_last( $drained ) ); + } + } diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php index 10b076c3..272149df 100644 --- a/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php @@ -216,4 +216,67 @@ public function testUnreadableSchemaNamesAreOmittedFromSearchResults(): void { $this->assertSame( [ 'SchemaNameLookupTest22' ], $names ); } + public function testGetReadableSchemaNamesDrainsEveryBatchInPageIdOrder(): void { + // The generator pages the namespace in 100-row keyset batches. With more rows than one batch, + // it must keep querying past the first batch and yield every Schema exactly once, in strictly + // ascending page-ID order — a single truncated batch would drop the tail. + $bulk = $this->createBarePages( NeoWikiExtension::NS_SCHEMA, 'BulkSchema', 120 ); + + $this->assertSame( + $this->expectedByPageId( $bulk ), + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( $this->getLookup()->getReadableSchemaNames() ) + ) + ); + } + + public function testGetReadableSchemaNamesContinuesPastAnUnreadableRowAtABatchBoundary(): void { + // The Schema whose page ID sits exactly on the first batch boundary (row 100, batch size 100) + // is denied. The generator advances its keyset anchor past every scanned row, readable or not, + // so the next batch still seeks beyond the denied row and returns rows 101+. The denied row is + // the only one absent; every later row still arrives. + $bulk = $this->createBarePages( NeoWikiExtension::NS_SCHEMA, 'BulkSchema', 120 ); + + $expected = $this->expectedByPageId( $bulk ); + // 100 = DatabaseSchemaNameLookup::READABLE_NAMES_BATCH_SIZE (private); the 100th row is the last of batch one. + $boundaryPageId = array_keys( $expected )[99]; + $boundaryTitle = $expected[$boundaryPageId]; + unset( $expected[$boundaryPageId] ); + + $denyBoundary = static fn ( string $permission, ?PageIdentity $page = null ): bool => + $page === null || $page->getDBkey() !== $boundaryTitle; + + $this->assertSame( + $expected, + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( + $this->getLookup( $this->mockRegisteredAuthority( $denyBoundary ) )->getReadableSchemaNames() + ) + ) + ); + } + + /** + * The 4 setUp Schemas then the bulk rows, each page ID mapped to its title, in page-ID order — + * the exact [pageId => name] map getReadableSchemaNames should yield when everything is readable. + * + * @param array $bulk + * @return array + */ + private function expectedByPageId( array $bulk ): array { + $expected = []; + + foreach ( $this->pageIds as $name => $id ) { + $expected[$id] = $name; + } + + foreach ( $bulk as $title => $id ) { + $expected[$id] = $title; + } + + return $expected; + } + } From 8319dc0efbfe7da9ada8a06236aa3d1e6599e72f Mon Sep 17 00:00:00 2001 From: Jeroen De Dauw Date: Thu, 23 Jul 2026 02:14:16 +0200 Subject: [PATCH 05/12] Test list pages keep next enabled while the listing continues 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 --- .../LayoutsPage/LayoutsPage.spec.ts | 120 ++++++++++++++++++ .../MappingsPage/MappingsPage.spec.ts | 23 +++- .../SchemasPage/SchemasPage.spec.ts | 19 ++- 3 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts diff --git a/resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts b/resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts new file mode 100644 index 00000000..1aab7f8f --- /dev/null +++ b/resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts @@ -0,0 +1,120 @@ +import { mount, VueWrapper, flushPromises } from '@vue/test-utils'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ref } from 'vue'; +import LayoutsPage from '@/components/LayoutsPage/LayoutsPage.vue'; +import { createI18nMock, setupMwMock } from '../../VueTestHelpers.ts'; + +interface LayoutSummary { + name: string; + schema: string; + type: string; + description: string; + ruleCount: number; +} + +const canCreateLayoutsRef = ref( false ); +const canEditLayoutRef = ref( false ); +const checkCreatePermissionMock = vi.fn(); +const checkEditPermissionMock = vi.fn(); + +let layoutsResponse: { layouts: LayoutSummary[]; nextCursor: string | null } = { layouts: [], nextCursor: null }; + +vi.mock( '@/composables/useLayoutPermissions.ts', () => ( { + useLayoutPermissions: () => ( { + canCreateLayouts: canCreateLayoutsRef, + canEditLayout: canEditLayoutRef, + checkCreatePermission: checkCreatePermissionMock, + checkEditPermission: checkEditPermissionMock, + } ), +} ) ); + +vi.mock( '@/stores/LayoutStore.ts', () => ( { + useLayoutStore: () => ( { + fetchLayout: vi.fn(), + getLayout: vi.fn(), + saveLayout: vi.fn(), + } ), +} ) ); + +vi.mock( '@/NeoWikiExtension.ts', () => ( { + NeoWikiExtension: { + getInstance: () => ( { + getMediaWiki: () => ( { + util: { wikiScript: () => '/rest.php' }, + } ), + newHttpClient: () => ( { + get: vi.fn().mockResolvedValue( { + ok: true, + json: () => Promise.resolve( layoutsResponse ), + } ), + } ), + } ), + }, +} ) ); + +function fullPage(): LayoutSummary[] { + return Array.from( { length: 10 }, ( _value, index ) => ( { + name: `Layout${ index }`, + schema: 'Person', + type: 'infobox', + description: '', + ruleCount: 0, + } ) ); +} + +function mountComponent( summaries: LayoutSummary[] = [], nextCursor: string | null = null ): VueWrapper { + layoutsResponse = { + layouts: summaries, + nextCursor: nextCursor, + }; + setupMwMock( { functions: [ 'msg', 'util', 'message', 'notify' ] } ); + + return mount( LayoutsPage, { + global: { + mocks: { $i18n: createI18nMock() }, + stubs: { + LayoutCreatorDialog: true, + LayoutEditorDialog: true, + EditSummary: true, + I18nSlot: true, + CdxIcon: true, + }, + }, + } ); +} + +describe( 'LayoutsPage', () => { + beforeEach( () => { + canCreateLayoutsRef.value = false; + canEditLayoutRef.value = false; + checkCreatePermissionMock.mockClear(); + checkEditPermissionMock.mockClear(); + layoutsResponse = { layouts: [], nextCursor: null }; + } ); + + it( 'disables next when a full page ends the listing', async () => { + // A listing that ends exactly on a page boundary returns a full page with a null cursor. + // CdxTable's indeterminate mode would keep next enabled (its heuristic is a short page), so + // the component must switch the table to a known total. + const wrapper = mountComponent( fullPage(), null ); + await flushPromises(); + + const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + + expect( nextButton.attributes( 'disabled' ) ).toBeDefined(); + expect( wrapper.text() ).toContain( 'of 10' ); + } ); + + it( 'keeps next enabled while the listing continues', async () => { + // A full page with a non-null cursor means more rows follow. The component must leave + // totalRows undefined so CdxTable stays in its indeterminate mode (next enabled, "of many" + // label); a known total here would wrongly disable next and hide the remaining pages. + const wrapper = mountComponent( fullPage(), 'next-page-cursor' ); + await flushPromises(); + + const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + + expect( nextButton.attributes( 'disabled' ) ).toBeUndefined(); + expect( wrapper.text() ).toContain( 'of many' ); + } ); +} ); diff --git a/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts b/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts index 541165f8..8ca74ea5 100644 --- a/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts +++ b/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts @@ -68,10 +68,10 @@ function findDeleteButtons( wrapper: VueWrapper ): VueWrapper[] { .filter( ( btn ) => btn.attributes( 'aria-label' ) === 'neowiki-mapping-delete' ); } -function mountComponent( summaries: MappingSummary[] = [] ): VueWrapper { +function mountComponent( summaries: MappingSummary[] = [], nextCursor: string | null = null ): VueWrapper { mappingsResponse = { mappings: summaries, - nextCursor: null, + nextCursor: nextCursor, }; setupMwMock( { functions: [ 'msg', 'util', 'message', 'notify' ], @@ -157,6 +157,25 @@ describe( 'MappingsPage', () => { expect( wrapper.text() ).toContain( 'neowiki-mappings-empty' ); } ); + it( 'keeps next enabled while the listing continues', async () => { + // A full page (the default size) with a non-null cursor means more rows follow. The + // component must leave totalRows undefined so CdxTable stays in its indeterminate mode + // (next enabled, "of many" label); a known total here would wrongly disable next and hide + // the remaining pages. + const wrapper = mountComponent( + Array.from( { length: 10 }, ( _value, index ) => ( + { name: `Mapping${ index }`, schemas: [] } + ) ), + 'next-page-cursor', + ); + await flushPromises(); + + const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + + expect( nextButton.attributes( 'disabled' ) ).toBeUndefined(); + expect( wrapper.text() ).toContain( 'of many' ); + } ); + it( 'shows the create button when the user may create mappings', async () => { canCreateMappingsRef.value = true; const wrapper = mountComponent(); diff --git a/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts b/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts index 56168e0f..10983112 100644 --- a/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts +++ b/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts @@ -79,10 +79,10 @@ function findDeleteButtons( wrapper: VueWrapper ): VueWrapper[] { .filter( ( btn ) => btn.attributes( 'aria-label' ) === 'neowiki-schema-delete' ); } -function mountComponent( summaries: unknown[] = [] ): VueWrapper { +function mountComponent( summaries: unknown[] = [], nextCursor: string | null = null ): VueWrapper { schemasResponse = { schemas: summaries, - nextCursor: null, + nextCursor: nextCursor, }; setupMwMock( { functions: [ 'msg', 'util', 'message', 'notify' ] } ); @@ -162,6 +162,21 @@ describe( 'SchemasPage', () => { expect( wrapper.text() ).toContain( 'of 10' ); } ); + it( 'keeps next enabled while the listing continues', async () => { + // A full page with a non-null cursor means more rows follow. The component must leave + // totalRows undefined so CdxTable stays in its indeterminate mode (next enabled, "of many" + // label); a known total here would wrongly disable next and hide the remaining pages. + const wrapper = mountComponent( Array.from( { length: 10 }, ( _value, index ) => ( + { name: `Schema${ index }`, description: '', propertyCount: 1 } + ) ), 'next-page-cursor' ); + await flushPromises(); + + const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + + expect( nextButton.attributes( 'disabled' ) ).toBeUndefined(); + expect( wrapper.text() ).toContain( 'of many' ); + } ); + it( 'shows empty value indicator for schemas without a description', async () => { const wrapper = mountComponent( [ { name: 'Person', description: '', propertyCount: 3 }, From 2e02d979e399a16ec9283bfc0f7d06651f0ad873 Mon Sep 17 00:00:00 2001 From: Jeroen De Dauw Date: Thu, 23 Jul 2026 16:50:37 +0200 Subject: [PATCH 06/12] Correct what the batch-boundary denial test discriminates 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 --- .../Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php index 272149df..583ba332 100644 --- a/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php @@ -233,9 +233,9 @@ public function testGetReadableSchemaNamesDrainsEveryBatchInPageIdOrder(): void public function testGetReadableSchemaNamesContinuesPastAnUnreadableRowAtABatchBoundary(): void { // The Schema whose page ID sits exactly on the first batch boundary (row 100, batch size 100) - // is denied. The generator advances its keyset anchor past every scanned row, readable or not, - // so the next batch still seeks beyond the denied row and returns rows 101+. The denied row is - // the only one absent; every later row still arrives. + // is denied. The drain's continue decision must count fetched rows, not yielded ones: batch + // one comes back full yet yields only 99, and the next batch must still be fetched, so rows + // 101+ arrive and the denied row is the only one absent. $bulk = $this->createBarePages( NeoWikiExtension::NS_SCHEMA, 'BulkSchema', 120 ); $expected = $this->expectedByPageId( $bulk ); From ec2f88e6ed9b8c0a83e3d73c7a9e51fc1548bfbf Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Fri, 24 Jul 2026 13:46:50 -0400 Subject: [PATCH 07/12] Assert the unloadable-Mapping fixture created its broken page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../REST/GetMappingSummariesApiTest.php | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php b/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php index bc1bd732..ace95025 100644 --- a/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php +++ b/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php @@ -9,6 +9,7 @@ use MediaWiki\Rest\RequestData; use MediaWiki\Tests\Rest\Handler\HandlerTestTrait; use ProfessionalWiki\NeoWiki\EntryPoints\REST\GetMappingSummariesApi; +use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Tests\NeoWikiIntegrationTestCase; /** @@ -206,15 +207,33 @@ private function requestMappings( array $queryParams ): array { * MappingContentHandler::validateSave would reject such content on the edit path (#1022). */ private function createMappingsWithUnloadableMiddle(): void { - $this->markPageTableAsUsed(); - $this->createMapping( 'Alpha', '{"version":1,"schemas":{}}' ); + $alpha = $this->createMapping( 'Alpha', '{"version":1,"schemas":{}}' )->getPageId(); $xml = $this->exportPageToXml( 'Mapping:Alpha' ); $xml = str_replace( 'Mapping:Alpha', 'Mapping:Beta', $xml ); $xml = str_replace( '"schemas"', '"schemaX"', $xml ); + // Guard the fixture's own premise: the title substitution must have produced a Beta dump and the + // content substitution must have removed the required key. If either misses (e.g. an export-format + // change), the import would recreate a loadable Alpha instead of an unloadable Beta. + $this->assertStringContainsString( 'Mapping:Beta', $xml ); + $this->assertStringNotContainsString( '"schemas"', $xml ); $this->importXml( $xml ); - $this->createMapping( 'Gamma', '{"version":1,"schemas":{}}' ); + $gamma = $this->createMapping( 'Gamma', '{"version":1,"schemas":{}}' )->getPageId(); + + // The import must have created a distinct Beta page sitting between Alpha and Gamma in page-ID + // order. Without this, a silently no-op import (Beta absent) leaves only Alpha and Gamma, whose + // listing is byte-identical to the asserted result — so both tests would pass while the skip + // branch they exist to pin never runs. + $beta = (int)$this->getDb()->newSelectQueryBuilder() + ->select( 'page_id' ) + ->from( 'page' ) + ->where( [ 'page_namespace' => NeoWikiExtension::NS_MAPPING, 'page_title' => 'Beta' ] ) + ->caller( __METHOD__ ) + ->fetchField(); + + $this->assertGreaterThan( $alpha, $beta, 'the imported unloadable Beta page must exist' ); + $this->assertGreaterThan( $beta, $gamma ); } } From 01485b66a28c240f24a8f7fc3e7d836fc5a52a28 Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Fri, 24 Jul 2026 13:46:58 -0400 Subject: [PATCH 08/12] Build bare page rows through InsertQueryBuilder 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) --- tests/phpunit/NeoWikiIntegrationTestCase.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/NeoWikiIntegrationTestCase.php b/tests/phpunit/NeoWikiIntegrationTestCase.php index fea2144f..8a411949 100644 --- a/tests/phpunit/NeoWikiIntegrationTestCase.php +++ b/tests/phpunit/NeoWikiIntegrationTestCase.php @@ -176,7 +176,11 @@ protected function createBarePages( int $namespace, string $titlePrefix, int $co ]; } - $this->getDb()->insert( 'page', $rows, __METHOD__ ); + $this->getDb()->newInsertQueryBuilder() + ->insertInto( 'page' ) + ->rows( $rows ) + ->caller( __METHOD__ ) + ->execute(); $pageIds = []; From 4efcb43e408755afcfe4a508c182769cacc94cb6 Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Fri, 24 Jul 2026 13:47:11 -0400 Subject: [PATCH 09/12] Harden the keyset-drain tests against the batch size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../MediaWiki/DatabaseLayoutNameLookup.php | 2 +- .../MediaWiki/DatabaseMappingNameLookup.php | 2 +- .../MediaWiki/DatabaseSchemaNameLookup.php | 2 +- .../DatabaseLayoutNameLookupTest.php | 45 +++++++++++++++---- .../DatabaseMappingNameLookupTest.php | 45 +++++++++++++++---- .../DatabaseSchemaNameLookupTest.php | 28 +++++++----- 6 files changed, 93 insertions(+), 31 deletions(-) diff --git a/src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php b/src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php index 762ca424..c3bcdf3a 100644 --- a/src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php +++ b/src/Persistence/MediaWiki/DatabaseLayoutNameLookup.php @@ -13,7 +13,7 @@ class DatabaseLayoutNameLookup implements LayoutNameLookup { - private const READABLE_NAMES_BATCH_SIZE = 100; + public const int READABLE_NAMES_BATCH_SIZE = 100; public function __construct( private readonly IDatabase $db, diff --git a/src/Persistence/MediaWiki/DatabaseMappingNameLookup.php b/src/Persistence/MediaWiki/DatabaseMappingNameLookup.php index 8faccdf1..e8191b25 100644 --- a/src/Persistence/MediaWiki/DatabaseMappingNameLookup.php +++ b/src/Persistence/MediaWiki/DatabaseMappingNameLookup.php @@ -14,7 +14,7 @@ class DatabaseMappingNameLookup implements MappingNameLookup { - private const READABLE_NAMES_BATCH_SIZE = 100; + public const int READABLE_NAMES_BATCH_SIZE = 100; public function __construct( private readonly IDatabase $db, diff --git a/src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php b/src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php index 483ccbe4..1cf05ae5 100644 --- a/src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php +++ b/src/Persistence/MediaWiki/DatabaseSchemaNameLookup.php @@ -16,7 +16,7 @@ class DatabaseSchemaNameLookup implements SchemaNameLookup { - private const READABLE_NAMES_BATCH_SIZE = 100; + public const int READABLE_NAMES_BATCH_SIZE = 100; public function __construct( private readonly IDatabase $db, diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php index bf60012f..82656c6d 100644 --- a/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseLayoutNameLookupTest.php @@ -97,17 +97,44 @@ public function testGetReadableLayoutNamesOmitsUnreadableLayouts(): void { ); } - public function testGetReadableLayoutNamesDrainsPastTheBatchSize(): void { - // The generator pages the namespace in 100-row keyset batches. With more rows than one batch, - // it must keep querying past the first batch: every Layout is yielded, the lowest page ID - // first and the highest last. A single truncated batch would drop the tail. - $bulk = $this->createBarePages( NeoWikiExtension::NS_LAYOUT, 'BulkLayout', 120 ); + public function testGetReadableLayoutNamesDrainsEveryBatchInPageIdOrder(): void { + // The generator pages the namespace in fixed-size keyset batches. With more rows than one batch, + // it must keep querying past the first batch and yield every Layout exactly once, in strictly + // ascending page-ID order — a single truncated batch would drop the tail. + $bulk = $this->createBarePages( + NeoWikiExtension::NS_LAYOUT, + 'BulkLayout', + DatabaseLayoutNameLookup::READABLE_NAMES_BATCH_SIZE + 20 + ); - $drained = iterator_to_array( $this->getLookup()->getReadableLayoutNames() ); + $this->assertSame( + $this->expectedByPageId( $bulk ), + array_map( + static fn ( TitleValue $title ): string => $title->getText(), + iterator_to_array( $this->getLookup()->getReadableLayoutNames() ) + ) + ); + } + + /** + * The setUp Layouts then the bulk rows, each page ID mapped to its title, in page-ID order — + * the exact [pageId => name] map getReadableLayoutNames should yield when everything is readable. + * + * @param array $bulk + * @return array + */ + private function expectedByPageId( array $bulk ): array { + $expected = []; + + foreach ( $this->pageIds as $name => $id ) { + $expected[$id] = $name; + } + + foreach ( $bulk as $title => $id ) { + $expected[$id] = $title; + } - $this->assertCount( count( $this->pageIds ) + count( $bulk ), $drained ); - $this->assertSame( $this->pageIds['LayoutNameLookupTest1'], array_key_first( $drained ) ); - $this->assertSame( max( $bulk ), array_key_last( $drained ) ); + return $expected; } } diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php index 99f2098f..5fce855a 100644 --- a/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseMappingNameLookupTest.php @@ -114,17 +114,44 @@ public function testGetReadableMappingNamesOmitsUnreadableMappings(): void { ); } - public function testGetReadableMappingNamesDrainsPastTheBatchSize(): void { - // The generator pages the namespace in 100-row keyset batches. With more rows than one batch, - // it must keep querying past the first batch: every Mapping is yielded, the lowest page ID - // first and the highest last. A single truncated batch would drop the tail. - $bulk = $this->createBarePages( NeoWikiExtension::NS_MAPPING, 'BulkMapping', 120 ); + public function testGetReadableMappingNamesDrainsEveryBatchInPageIdOrder(): void { + // The generator pages the namespace in fixed-size keyset batches. With more rows than one batch, + // it must keep querying past the first batch and yield every Mapping exactly once, in strictly + // ascending page-ID order — a single truncated batch would drop the tail. + $bulk = $this->createBarePages( + NeoWikiExtension::NS_MAPPING, + 'BulkMapping', + DatabaseMappingNameLookup::READABLE_NAMES_BATCH_SIZE + 20 + ); - $drained = iterator_to_array( $this->getLookup()->getReadableMappingNames() ); + $this->assertSame( + $this->expectedByPageId( $bulk ), + array_map( + static fn ( MappingName $name ): string => $name->getText(), + iterator_to_array( $this->getLookup()->getReadableMappingNames() ) + ) + ); + } + + /** + * The setUp Mappings then the bulk rows, each page ID mapped to its title, in page-ID order — + * the exact [pageId => name] map getReadableMappingNames should yield when everything is readable. + * + * @param array $bulk + * @return array + */ + private function expectedByPageId( array $bulk ): array { + $expected = []; + + foreach ( $this->pageIds as $name => $id ) { + $expected[$id] = $name; + } + + foreach ( $bulk as $title => $id ) { + $expected[$id] = $title; + } - $this->assertCount( count( $this->pageIds ) + count( $bulk ), $drained ); - $this->assertSame( $this->pageIds['MappingLookupTest1'], array_key_first( $drained ) ); - $this->assertSame( max( $bulk ), array_key_last( $drained ) ); + return $expected; } } diff --git a/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php b/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php index 583ba332..61a4cf34 100644 --- a/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php +++ b/tests/phpunit/Persistence/MediaWiki/DatabaseSchemaNameLookupTest.php @@ -217,10 +217,14 @@ public function testUnreadableSchemaNamesAreOmittedFromSearchResults(): void { } public function testGetReadableSchemaNamesDrainsEveryBatchInPageIdOrder(): void { - // The generator pages the namespace in 100-row keyset batches. With more rows than one batch, + // The generator pages the namespace in fixed-size keyset batches. With more rows than one batch, // it must keep querying past the first batch and yield every Schema exactly once, in strictly // ascending page-ID order — a single truncated batch would drop the tail. - $bulk = $this->createBarePages( NeoWikiExtension::NS_SCHEMA, 'BulkSchema', 120 ); + $bulk = $this->createBarePages( + NeoWikiExtension::NS_SCHEMA, + 'BulkSchema', + DatabaseSchemaNameLookup::READABLE_NAMES_BATCH_SIZE + 20 + ); $this->assertSame( $this->expectedByPageId( $bulk ), @@ -232,15 +236,19 @@ public function testGetReadableSchemaNamesDrainsEveryBatchInPageIdOrder(): void } public function testGetReadableSchemaNamesContinuesPastAnUnreadableRowAtABatchBoundary(): void { - // The Schema whose page ID sits exactly on the first batch boundary (row 100, batch size 100) - // is denied. The drain's continue decision must count fetched rows, not yielded ones: batch - // one comes back full yet yields only 99, and the next batch must still be fetched, so rows - // 101+ arrive and the denied row is the only one absent. - $bulk = $this->createBarePages( NeoWikiExtension::NS_SCHEMA, 'BulkSchema', 120 ); + // The Schema whose page ID sits exactly on the first batch boundary (the last row of the first + // full batch) is denied. The drain's continue decision must count fetched rows, not yielded ones: + // batch one comes back full yet yields one short, and the next batch must still be fetched, so the + // rows past the boundary arrive and the denied row is the only one absent. + $bulk = $this->createBarePages( + NeoWikiExtension::NS_SCHEMA, + 'BulkSchema', + DatabaseSchemaNameLookup::READABLE_NAMES_BATCH_SIZE + 20 + ); $expected = $this->expectedByPageId( $bulk ); - // 100 = DatabaseSchemaNameLookup::READABLE_NAMES_BATCH_SIZE (private); the 100th row is the last of batch one. - $boundaryPageId = array_keys( $expected )[99]; + // The last row of the first batch sits at index READABLE_NAMES_BATCH_SIZE - 1. + $boundaryPageId = array_keys( $expected )[DatabaseSchemaNameLookup::READABLE_NAMES_BATCH_SIZE - 1]; $boundaryTitle = $expected[$boundaryPageId]; unset( $expected[$boundaryPageId] ); @@ -259,7 +267,7 @@ public function testGetReadableSchemaNamesContinuesPastAnUnreadableRowAtABatchBo } /** - * The 4 setUp Schemas then the bulk rows, each page ID mapped to its title, in page-ID order — + * The setUp Schemas then the bulk rows, each page ID mapped to its title, in page-ID order — * the exact [pageId => name] map getReadableSchemaNames should yield when everything is readable. * * @param array $bulk From 7ed627527733f0daecd8e9785bb2bfa3399edb75 Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Fri, 24 Jul 2026 13:48:47 -0400 Subject: [PATCH 10/12] Share the pager "Next page" selector behind a test helper 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) --- resources/ext.neowiki/tests/VueTestHelpers.ts | 16 ++++++++++++++-- .../components/LayoutsPage/LayoutsPage.spec.ts | 6 +++--- .../components/MappingsPage/MappingsPage.spec.ts | 4 ++-- .../components/SchemasPage/SchemasPage.spec.ts | 6 +++--- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/resources/ext.neowiki/tests/VueTestHelpers.ts b/resources/ext.neowiki/tests/VueTestHelpers.ts index 3dbc46f2..5483ba1e 100644 --- a/resources/ext.neowiki/tests/VueTestHelpers.ts +++ b/resources/ext.neowiki/tests/VueTestHelpers.ts @@ -1,6 +1,6 @@ -import { mount, VueWrapper } from '@vue/test-utils'; +import { mount, VueWrapper, DOMWrapper } from '@vue/test-utils'; import { Component, DefineComponent } from 'vue'; -import { vi } from 'vitest'; +import { expect, vi } from 'vitest'; import { ValidationMessages, ValidationStatusType } from '@wikimedia/codex'; import { NeoWikiTestServices } from './NeoWikiTestServices.ts'; @@ -15,6 +15,18 @@ export function createI18nMock(): ReturnType { } ) ); } +/** + * Locates the CdxTable pager's "Next page" button, asserting it exists so a selector that matches + * nothing fails loudly here. A missed find() returns an empty DOMWrapper whose + * attributes( 'disabled' ) is undefined, which would silently satisfy a toBeUndefined() + * enabled-state assertion against a button that is not in the DOM. + */ +export function findNextPageButton( wrapper: VueWrapper ): DOMWrapper { + const button = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + expect( button.exists() ).toBe( true ); + return button; +} + export function createTestWrapper>( component: Component, props: InstanceType['$props'], diff --git a/resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts b/resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts index 1aab7f8f..49cf4d88 100644 --- a/resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts +++ b/resources/ext.neowiki/tests/components/LayoutsPage/LayoutsPage.spec.ts @@ -2,7 +2,7 @@ import { mount, VueWrapper, flushPromises } from '@vue/test-utils'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ref } from 'vue'; import LayoutsPage from '@/components/LayoutsPage/LayoutsPage.vue'; -import { createI18nMock, setupMwMock } from '../../VueTestHelpers.ts'; +import { createI18nMock, findNextPageButton, setupMwMock } from '../../VueTestHelpers.ts'; interface LayoutSummary { name: string; @@ -99,7 +99,7 @@ describe( 'LayoutsPage', () => { const wrapper = mountComponent( fullPage(), null ); await flushPromises(); - const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + const nextButton = findNextPageButton( wrapper ); expect( nextButton.attributes( 'disabled' ) ).toBeDefined(); expect( wrapper.text() ).toContain( 'of 10' ); @@ -112,7 +112,7 @@ describe( 'LayoutsPage', () => { const wrapper = mountComponent( fullPage(), 'next-page-cursor' ); await flushPromises(); - const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + const nextButton = findNextPageButton( wrapper ); expect( nextButton.attributes( 'disabled' ) ).toBeUndefined(); expect( wrapper.text() ).toContain( 'of many' ); diff --git a/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts b/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts index 8ca74ea5..4eb7df22 100644 --- a/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts +++ b/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ref } from 'vue'; import MappingsPage from '@/components/MappingsPage/MappingsPage.vue'; import MappingCreatorDialog from '@/components/MappingsPage/MappingCreatorDialog.vue'; -import { createI18nMock, setupMwMock } from '../../VueTestHelpers.ts'; +import { createI18nMock, findNextPageButton, setupMwMock } from '../../VueTestHelpers.ts'; import { CdxButton, CdxDialog } from '@wikimedia/codex'; interface MappingSummary { @@ -170,7 +170,7 @@ describe( 'MappingsPage', () => { ); await flushPromises(); - const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + const nextButton = findNextPageButton( wrapper ); expect( nextButton.attributes( 'disabled' ) ).toBeUndefined(); expect( wrapper.text() ).toContain( 'of many' ); diff --git a/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts b/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts index 10983112..b9279851 100644 --- a/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts +++ b/resources/ext.neowiki/tests/components/SchemasPage/SchemasPage.spec.ts @@ -4,7 +4,7 @@ import { ref } from 'vue'; import SchemasPage from '@/components/SchemasPage/SchemasPage.vue'; import SchemaCreatorDialog from '@/components/SchemasPage/SchemaCreatorDialog.vue'; import SchemaEditorDialog from '@/components/SchemaEditor/SchemaEditorDialog.vue'; -import { createI18nMock, setupMwMock } from '../../VueTestHelpers.ts'; +import { createI18nMock, findNextPageButton, setupMwMock } from '../../VueTestHelpers.ts'; import { CdxButton, CdxDialog } from '@wikimedia/codex'; import { Schema } from '@/domain/Schema.ts'; import { PropertyDefinitionList } from '@/domain/PropertyDefinitionList.ts'; @@ -156,7 +156,7 @@ describe( 'SchemasPage', () => { ) ) ); await flushPromises(); - const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + const nextButton = findNextPageButton( wrapper ); expect( nextButton.attributes( 'disabled' ) ).toBeDefined(); expect( wrapper.text() ).toContain( 'of 10' ); @@ -171,7 +171,7 @@ describe( 'SchemasPage', () => { ) ), 'next-page-cursor' ); await flushPromises(); - const nextButton = wrapper.find( '.cdx-table-pager button[aria-label="Next page"]' ); + const nextButton = findNextPageButton( wrapper ); expect( nextButton.attributes( 'disabled' ) ).toBeUndefined(); expect( wrapper.text() ).toContain( 'of many' ); From 35f4b9a6360323dd929b8dd023165b8b45329416 Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Fri, 24 Jul 2026 13:50:01 -0400 Subject: [PATCH 11/12] Cover the MappingsPage determinate pager flip 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) --- .../MappingsPage/MappingsPage.spec.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts b/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts index 4eb7df22..fdb9025a 100644 --- a/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts +++ b/resources/ext.neowiki/tests/components/MappingsPage/MappingsPage.spec.ts @@ -157,6 +157,23 @@ describe( 'MappingsPage', () => { expect( wrapper.text() ).toContain( 'neowiki-mappings-empty' ); } ); + it( 'disables next when a full page ends the listing', async () => { + // A listing that ends exactly on a page boundary returns a full page with a null cursor. + // CdxTable's indeterminate mode would keep next enabled (its heuristic is a short page), so + // the component must switch the table to a known total. + const wrapper = mountComponent( + Array.from( { length: 10 }, ( _value, index ) => ( + { name: `Mapping${ index }`, schemas: [] } + ) ), + ); + await flushPromises(); + + const nextButton = findNextPageButton( wrapper ); + + expect( nextButton.attributes( 'disabled' ) ).toBeDefined(); + expect( wrapper.text() ).toContain( 'of 10' ); + } ); + it( 'keeps next enabled while the listing continues', async () => { // A full page (the default size) with a non-null cursor means more rows follow. The // component must leave totalRows undefined so CdxTable stays in its indeterminate mode From 17c40dc9b801c011def7a9a07b0407c9c1004306 Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Fri, 24 Jul 2026 14:20:16 -0400 Subject: [PATCH 12/12] Assert the content substitution replaced the key, not that it is absent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content guard added in 544a9795 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) --- .../EntryPoints/REST/GetMappingSummariesApiTest.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php b/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php index ace95025..31a548cc 100644 --- a/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php +++ b/tests/phpunit/EntryPoints/REST/GetMappingSummariesApiTest.php @@ -213,10 +213,13 @@ private function createMappingsWithUnloadableMiddle(): void { $xml = str_replace( 'Mapping:Alpha', 'Mapping:Beta', $xml ); $xml = str_replace( '"schemas"', '"schemaX"', $xml ); // Guard the fixture's own premise: the title substitution must have produced a Beta dump and the - // content substitution must have removed the required key. If either misses (e.g. an export-format - // change), the import would recreate a loadable Alpha instead of an unloadable Beta. + // content substitution must have replaced the required key with its schemaX token. If either + // misses (e.g. an export-format change), the import would recreate a loadable Alpha instead of an + // unloadable Beta. Both guards assert a replacement TARGET, so each fails when its str_replace + // matched nothing — asserting the removed "schemas" token is absent would instead be tautological, + // since str_replace makes that true unconditionally. $this->assertStringContainsString( 'Mapping:Beta', $xml ); - $this->assertStringNotContainsString( '"schemas"', $xml ); + $this->assertStringContainsString( '"schemaX"', $xml ); $this->importXml( $xml ); $gamma = $this->createMapping( 'Gamma', '{"version":1,"schemas":{}}' )->getPageId();