diff --git a/docs/api/graph-model.md b/docs/api/graph-model.md index 88dd041fc..187cf99ef 100644 --- a/docs/api/graph-model.md +++ b/docs/api/graph-model.md @@ -61,13 +61,15 @@ their Schema (e.g., `:Subject:Person`, `:Subject:Company`). The Schema label cha | Property | Neo4j Type | Description | |----------|------------|-------------| -| `id` | string | Subject ID, 15 characters starting with `s` (unique) | +| `id` | string | Subject ID as stored: the bare local id, 15 characters starting with `s` (unique) | | `name` | string | Subject label | | `wiki_id` | string | [MediaWiki Wiki ID](https://www.mediawiki.org/wiki/Manual:Wiki_ID) of the wiki that owns the Subject | Unlike page ids, Subject ids are globally unique nanoids ([ADR 14](../adr/014-improved-id-format.md)), so a Subject's identity is its `id` alone. The `wiki_id` is carried only for per-wiki query filtering in a shared graph; Subject-id -namespacing is deferred ([ADR 22](../adr/022-multi-wiki-node-identity.md)). +namespacing is deferred ([ADR 22](../adr/022-multi-wiki-node-identity.md)). The source-qualified `(source, localId)` +reference form ([ADR 23](../adr/023-subject-sources.md)) is derived at the system boundaries; Subject nodes keep +storing the bare id. ### Dynamic properties diff --git a/docs/api/rest-api.md b/docs/api/rest-api.md index 72bd5c9de..cbb2e878b 100644 --- a/docs/api/rest-api.md +++ b/docs/api/rest-api.md @@ -41,6 +41,10 @@ Read, change, and validate Subjects. New Subjects are created on a page — see | `POST /neowiki/v0/subject/{subjectId}/validate` | Check whether a change to a Subject is valid, without saving it. | | `GET /neowiki/v0/subject-labels` | Find Subjects of a Schema by label; returns `id`/`label` pairs. | +`{subjectId}` accepts a Subject id in either form described in +[Subject format](subject-format.md#id-formats): the bare local form or the source-qualified +`source:localId` form. An id that explicitly names the local wiki's source is treated as its bare form. + ### Pages and Subjects A page holds one optional main Subject and an ordered list of child Subjects. These endpoints create diff --git a/docs/api/subject-format.md b/docs/api/subject-format.md index 03a47f975..7ded98583 100644 --- a/docs/api/subject-format.md +++ b/docs/api/subject-format.md @@ -145,7 +145,7 @@ Array of Relation objects, each pointing to another subject. | Field | Type | Required | Description | |-------|------|-------------|-------------| | `id` | string | Yes | Unique identifier for this relation | -| `target` | string | Yes | Subject ID of the target subject | +| `target` | string | Yes | Subject ID of the target subject, in either id form (bare means local) | | `properties` | object | No | Key-value pairs of relation properties. Only included when non-empty. | Relation properties example: @@ -172,10 +172,22 @@ Relation properties example: ### Subject IDs -Subject IDs are 15-character nanoid-style identifiers that are lexicographically sortable by creation time. -They start with `s`. +A Subject id has two textual forms ([ADR 23](../adr/023-subject-sources.md)): -Example: `s1demo5sssssss1` +- The **bare local form**: a 15-character nanoid-style identifier starting with `s`, lexicographically + sortable by creation time. Local Subjects are always stored in this form. + + Example: `s1demo5sssssss1` + +- The **source-qualified form** `source:localId`, referencing a Subject from a specific Source. The part + before the first `:` is the source key (`[A-Za-z0-9+_-]+`; for wikis, the + [MediaWiki Wiki ID](https://www.mediawiki.org/wiki/Manual:Wiki_ID)); the rest is the localId, whose + grammar is owned by its Source. Source keys are case-sensitive: keys that differ only in case denote + different Sources. Until Sources can define their own grammars, localIds are restricted to + URL-path-safe characters (RFC 3986 pchar, excluding percent-encoding). An id that explicitly names the + local source is equivalent to its bare form and is normalized to it. + + Example: `otherwiki:s1demo5sssssss1` ### Relation IDs @@ -191,6 +203,8 @@ See [ADR 014](../adr/014-improved-id-format.md) for details on the ID format. `GET /rest.php/neowiki/v0/subject/{subjectId}` +`{subjectId}` accepts either id form; returned ids are canonical, so local Subjects always appear bare. + Returns the same statement format as storage, with additional fields: - `requestedId`: The ID that was requested - Each subject includes an `id` field. Passing `?expand=page` adds the page fields `pageId`, diff --git a/docs/glossary.md b/docs/glossary.md index f3dbe7673..8a339b6b2 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -25,7 +25,9 @@ Data about one thing. Similar to an Item in Wikibase or a Page/SubObject in SMW. Subjects have -- An `id`: persistent identifier. Subject IDs start with `s` and are always 15 characters long ([ADR 14](adr/014-improved-id-format.md)) +- An `id`: persistent identifier. Local Subject IDs are bare nanoids starting with `s`, always 15 characters + long ([ADR 14](adr/014-improved-id-format.md)). A Subject id may also be source-qualified as + `source:localId`, where the bare form means the local source ([ADR 23](adr/023-subject-sources.md)) - A `type`: reference to a Schema. Example: Person, Company, Product, etc. - A `label`: the name of the subject. Example: "John Doe". This is a string, not a reference to a page. - `statements`: a list of Statements diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index d9a058a36..864930303 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -166,7 +166,7 @@ parameters: path: src/Persistence/MediaWiki/Subject/SubjectContentDataDeserializer.php - - message: "#^Parameter \\#1 \\$text of class ProfessionalWiki\\\\NeoWiki\\\\Domain\\\\Subject\\\\SubjectId constructor expects string, mixed given\\.$#" + message: "#^Parameter \\#1 \\$text of method ProfessionalWiki\\\\NeoWiki\\\\Domain\\\\Subject\\\\SubjectIdParser\\:\\:parse\\(\\) expects string, mixed given\\.$#" count: 1 path: src/Persistence/MediaWiki/Subject/SubjectContentDataDeserializer.php diff --git a/resources/ext.neowiki/src/components/NeoWikiApp.vue b/resources/ext.neowiki/src/components/NeoWikiApp.vue index 5e8ea647f..ca2c8e27f 100644 --- a/resources/ext.neowiki/src/components/NeoWikiApp.vue +++ b/resources/ext.neowiki/src/components/NeoWikiApp.vue @@ -21,21 +21,14 @@ diff --git a/resources/ext.neowiki/src/components/viewData.ts b/resources/ext.neowiki/src/components/viewData.ts new file mode 100644 index 000000000..2364537fd --- /dev/null +++ b/resources/ext.neowiki/src/components/viewData.ts @@ -0,0 +1,52 @@ +import { SubjectId } from '@/domain/SubjectId'; +import { SubjectIdParser } from '@/domain/SubjectIdParser'; + +export interface ViewData { + id: string; + element: HTMLElement; + subjectId: SubjectId; + canEditSubject: boolean; + viewType?: string; + layoutName?: string; +} + +export async function getViewsData( + elements: NodeListOf, + subjectIdParser: SubjectIdParser, + canEditSubject: ( subjectId: SubjectId ) => Promise, +): Promise { + const viewsData: ViewData[] = []; + + for ( const element of elements ) { + const viewData = await getViewData( element, subjectIdParser, canEditSubject ); + if ( viewData ) { + viewsData.push( viewData ); + } + } + return viewsData; +} + +async function getViewData( + element: HTMLElement, + subjectIdParser: SubjectIdParser, + canEditSubject: ( subjectId: SubjectId ) => Promise, +): Promise { + if ( !element.dataset.mwNeowikiSubjectId ) { + return null; + } + + try { + const subjectId = subjectIdParser.parse( element.dataset.mwNeowikiSubjectId ); + return { + id: subjectId.text, + element: element, + subjectId: subjectId, + canEditSubject: await canEditSubject( subjectId ), + viewType: element.dataset.mwNeowikiViewType, + layoutName: element.dataset.mwNeowikiLayoutName, + }; + } catch ( error ) { + console.error( error ); + return null; + } +} diff --git a/resources/ext.neowiki/src/domain/SubjectId.ts b/resources/ext.neowiki/src/domain/SubjectId.ts index 1fe0d227e..bac815b43 100644 --- a/resources/ext.neowiki/src/domain/SubjectId.ts +++ b/resources/ext.neowiki/src/domain/SubjectId.ts @@ -1,17 +1,37 @@ +const BARE_PATTERN = /^s[123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]{14}$/; +const QUALIFIED_PATTERN = /^([A-Za-z0-9+_-]+):([A-Za-z0-9._~!$&'()*+,;=:@-]+)$/; + export class SubjectId { public readonly text: string; + /** + * The source key, or null for a local Subject (bare id form). + */ + public readonly source: string | null; + + public readonly localId: string; + public constructor( text: string ) { - if ( !SubjectId.isValid( text ) ) { - throw new Error( 'Subject ID has the wrong format. ID: ' + text ); + if ( BARE_PATTERN.test( text ) ) { + this.source = null; + this.localId = text; + } else { + const match = QUALIFIED_PATTERN.exec( text ); + + if ( match === null ) { + throw new Error( 'Subject ID has the wrong format. ID: ' + text ); + } + + this.source = match[ 1 ]; + this.localId = match[ 2 ]; } this.text = text; } public static isValid( text: string ): boolean { - return /^s[123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]{14}$/.test( text ); + return BARE_PATTERN.test( text ) || QUALIFIED_PATTERN.test( text ); } } diff --git a/resources/ext.neowiki/src/domain/SubjectIdParser.ts b/resources/ext.neowiki/src/domain/SubjectIdParser.ts new file mode 100644 index 000000000..0dddd7be7 --- /dev/null +++ b/resources/ext.neowiki/src/domain/SubjectIdParser.ts @@ -0,0 +1,33 @@ +import { SubjectId } from './SubjectId'; + +/** + * Parses Subject ID strings at system boundaries. An id that explicitly names + * the local source canonicalizes to the bare form, so one Subject never has + * two textual identities. + */ +export class SubjectIdParser { + + public constructor( private readonly localSourceKey: string ) { + } + + public parse( text: string ): SubjectId { + const id = new SubjectId( text ); + + if ( id.source === this.localSourceKey ) { + return this.newLocalId( id.localId, text ); + } + + return id; + } + + private newLocalId( localId: string, originalText: string ): SubjectId { + const bareId = new SubjectId( localId ); + + if ( bareId.source !== null ) { + throw new Error( 'Local Subject IDs must be bare: ' + originalText ); + } + + return bareId; + } + +} diff --git a/resources/ext.neowiki/src/persistence/RestSubjectRepository.ts b/resources/ext.neowiki/src/persistence/RestSubjectRepository.ts index 680d7f41c..bb8153892 100644 --- a/resources/ext.neowiki/src/persistence/RestSubjectRepository.ts +++ b/resources/ext.neowiki/src/persistence/RestSubjectRepository.ts @@ -158,7 +158,7 @@ export class RestSubjectRepository implements SubjectRepository { } private async fetchSubjectBundle( id: SubjectId ): Promise { - let url = `${ this.mediaWikiRestApiUrl }/neowiki/v0/subject/${ id.text }?expand=page|relations`; + let url = `${ this.mediaWikiRestApiUrl }/neowiki/v0/subject/${ encodeURIComponent( id.text ) }?expand=page|relations`; if ( this.revisionId !== undefined ) { url += `&revisionId=${ this.revisionId }`; @@ -249,7 +249,7 @@ export class RestSubjectRepository implements SubjectRepository { public async updateSubject( id: SubjectId, label: string, statements: StatementList, comment?: string ): Promise { const response = await this.httpClient.put( - `${ this.mediaWikiRestApiUrl }/neowiki/v0/subject/${ id.text }`, + `${ this.mediaWikiRestApiUrl }/neowiki/v0/subject/${ encodeURIComponent( id.text ) }`, { label, statements: statementsToJson( statements ), @@ -273,7 +273,7 @@ export class RestSubjectRepository implements SubjectRepository { public async deleteSubject( id: SubjectId, comment?: string ): Promise { const response = await this.httpClient.delete( - `${ this.mediaWikiRestApiUrl }/neowiki/v0/subject/${ id.text }`, + `${ this.mediaWikiRestApiUrl }/neowiki/v0/subject/${ encodeURIComponent( id.text ) }`, { headers: { 'Content-Type': 'application/json', @@ -306,7 +306,7 @@ export class RestSubjectRepository implements SubjectRepository { statements: StatementList, ): Promise { return this.runValidation( - `${ this.mediaWikiRestApiUrl }/neowiki/v0/subject/${ id.text }/validate`, + `${ this.mediaWikiRestApiUrl }/neowiki/v0/subject/${ encodeURIComponent( id.text ) }/validate`, { label, statements: statementsToJson( statements ) }, ); } diff --git a/resources/ext.neowiki/tests/components/viewData.spec.ts b/resources/ext.neowiki/tests/components/viewData.spec.ts new file mode 100644 index 000000000..c42d8b794 --- /dev/null +++ b/resources/ext.neowiki/tests/components/viewData.spec.ts @@ -0,0 +1,69 @@ +import { describe, expect, it, vi } from 'vitest'; +import { getViewsData } from '@/components/viewData'; +import { SubjectIdParser } from '@/domain/SubjectIdParser'; + +const parser = new SubjectIdParser( 'testwiki' ); + +function viewElements( ...subjectIds: string[] ): NodeListOf { + const container = document.createElement( 'div' ); + + for ( const subjectId of subjectIds ) { + const element = document.createElement( 'div' ); + element.setAttribute( 'data-mw-neowiki-subject-id', subjectId ); + container.appendChild( element ); + } + + return container.querySelectorAll( 'div' ); +} + +describe( 'getViewsData', () => { + + it( 'uses a bare subject id as the view key', async () => { + const viewsData = await getViewsData( viewElements( 's11111111111111' ), parser, async () => true ); + + expect( viewsData ).toHaveLength( 1 ); + expect( viewsData[ 0 ].id ).toBe( 's11111111111111' ); + expect( viewsData[ 0 ].subjectId.text ).toBe( 's11111111111111' ); + } ); + + it( 'canonicalizes an explicitly-local-qualified id to the bare key', async () => { + const viewsData = await getViewsData( viewElements( 'testwiki:s11111111111111' ), parser, async () => true ); + + expect( viewsData ).toHaveLength( 1 ); + expect( viewsData[ 0 ].id ).toBe( 's11111111111111' ); + expect( viewsData[ 0 ].subjectId.text ).toBe( 's11111111111111' ); + } ); + + it( 'keeps a foreign-qualified id qualified', async () => { + const viewsData = await getViewsData( viewElements( 'enwiki:Q42' ), parser, async () => true ); + + expect( viewsData ).toHaveLength( 1 ); + expect( viewsData[ 0 ].id ).toBe( 'enwiki:Q42' ); + } ); + + it( 'skips elements with an invalid id instead of failing', async () => { + const consoleError = vi.spyOn( console, 'error' ).mockImplementation( () => undefined ); + + const viewsData = await getViewsData( + viewElements( 'not-a-subject-id', 's11111111111111' ), + parser, + async () => true, + ); + + expect( viewsData ).toHaveLength( 1 ); + expect( viewsData[ 0 ].id ).toBe( 's11111111111111' ); + expect( consoleError ).toHaveBeenCalled(); + + consoleError.mockRestore(); + } ); + + it( 'skips elements without a subject id', async () => { + const container = document.createElement( 'div' ); + container.appendChild( document.createElement( 'div' ) ); + + const viewsData = await getViewsData( container.querySelectorAll( 'div' ), parser, async () => true ); + + expect( viewsData ).toHaveLength( 0 ); + } ); + +} ); diff --git a/resources/ext.neowiki/tests/domain/SubjectId.unit.spec.ts b/resources/ext.neowiki/tests/domain/SubjectId.unit.spec.ts index c3dd847f0..86e62d175 100644 --- a/resources/ext.neowiki/tests/domain/SubjectId.unit.spec.ts +++ b/resources/ext.neowiki/tests/domain/SubjectId.unit.spec.ts @@ -10,9 +10,23 @@ describe( 'Subject', () => { expect( subjectId.text ).toBe( GUID ); } ); - // TODO It seems vitest does not support toThrowError - // it( 'throws exception when given an invalid GUID', () => { - // const GUID = '7777-0000-000000000001'; - // expect( () => new SubjectId( GUID ) ).toThrowError(); - // } ); + it( 'throws when given an invalid ID', () => { + expect( () => new SubjectId( '7777-0000-000000000001' ) ).toThrowError(); + } ); + + it( 'has local source and itself as localId for a bare ID', () => { + const subjectId = new SubjectId( 's11111111111111' ); + + expect( subjectId.source ).toBeNull(); + expect( subjectId.localId ).toBe( 's11111111111111' ); + } ); + + it( 'exposes source and localId for a qualified ID', () => { + const subjectId = new SubjectId( 'enwiki:Q42' ); + + expect( subjectId.text ).toBe( 'enwiki:Q42' ); + expect( subjectId.source ).toBe( 'enwiki' ); + expect( subjectId.localId ).toBe( 'Q42' ); + } ); + } ); diff --git a/resources/ext.neowiki/tests/domain/SubjectIdParser.unit.spec.ts b/resources/ext.neowiki/tests/domain/SubjectIdParser.unit.spec.ts new file mode 100644 index 000000000..67abd5011 --- /dev/null +++ b/resources/ext.neowiki/tests/domain/SubjectIdParser.unit.spec.ts @@ -0,0 +1,47 @@ +// @vitest-environment node +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { SubjectIdParser } from '@/domain/SubjectIdParser'; + +interface Vector { + name: string; + input: string; + valid: boolean; + canonicalText?: string; + source?: string | null; + localId?: string; +} + +const vectors = JSON.parse( + readFileSync( new URL( '../../../../tests/subject-id-vectors.json', import.meta.url ), 'utf8' ), +) as { localSourceKey: string; cases: Vector[] }; + +const parser = new SubjectIdParser( vectors.localSourceKey ); +const validCases = vectors.cases.filter( ( vector ) => vector.valid ) as Required[]; +const invalidCases = vectors.cases.filter( ( vector ) => !vector.valid ); + +describe( 'SubjectIdParser', () => { + + it.each( validCases )( 'parses: $name', ( { input, canonicalText, source, localId } ) => { + const id = parser.parse( input ); + + expect( id.text ).toBe( canonicalText ); + expect( id.source ).toBe( source ); + expect( id.localId ).toBe( localId ); + } ); + + it.each( validCases )( 'canonical text parses to itself: $name', ( { canonicalText } ) => { + expect( parser.parse( canonicalText ).text ).toBe( canonicalText ); + } ); + + it.each( invalidCases )( 'rejects: $name', ( { input } ) => { + expect( () => parser.parse( input ) ).toThrowError(); + } ); + + it( 'accepts a local source key outside the key grammar', () => { + const weirdKeyParser = new SubjectIdParser( 'weird~key' ); + + expect( weirdKeyParser.parse( 's11111111111111' ).text ).toBe( 's11111111111111' ); + } ); + +} ); diff --git a/resources/ext.neowiki/tests/persistence/RestSubjectRepository.neo4j.spec.ts b/resources/ext.neowiki/tests/persistence/RestSubjectRepository.neo4j.spec.ts index 152bba013..4356219bb 100644 --- a/resources/ext.neowiki/tests/persistence/RestSubjectRepository.neo4j.spec.ts +++ b/resources/ext.neowiki/tests/persistence/RestSubjectRepository.neo4j.spec.ts @@ -66,6 +66,25 @@ describe( 'RestSubjectRepository', () => { } } ); + it( 'percent-encodes qualified subject ids in the request URL', async () => { + const qualifiedId = 'enwiki:Q42'; + const inMemoryHttpClient = new InMemoryHttpClient( { + 'https://example.com/rest.php/neowiki/v0/subject/enwiki%3AQ42?expand=page|relations': + new Response( JSON.stringify( { + requestedId: qualifiedId, + subjects: { + [ qualifiedId ]: { ...subjectResponse, id: qualifiedId }, + }, + } ), { status: 200 } ), + } ); + + const repository = newRepository( 'https://example.com/rest.php', inMemoryHttpClient ); + + const subject = await repository.getSubject( new SubjectId( qualifiedId ) ); + + expect( subject?.getId().text ).toBe( qualifiedId ); + } ); + it( 'returns existing subject', async () => { const inMemoryHttpClient = new InMemoryHttpClient( { 'https://example.com/rest.php/neowiki/v0/subject/s11111111111111?expand=page|relations': diff --git a/src/Application/Actions/SetMainSubject/SetMainSubjectAction.php b/src/Application/Actions/SetMainSubject/SetMainSubjectAction.php index 5f873e598..7a9069de3 100644 --- a/src/Application/Actions/SetMainSubject/SetMainSubjectAction.php +++ b/src/Application/Actions/SetMainSubject/SetMainSubjectAction.php @@ -10,6 +10,7 @@ use ProfessionalWiki\NeoWiki\Domain\Page\PageSubjects; use ProfessionalWiki\NeoWiki\Domain\Subject\Subject; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; readonly class SetMainSubjectAction { @@ -17,6 +18,7 @@ public function __construct( private SetMainSubjectPresenter $presenter, private SubjectRepository $subjectRepository, private SubjectWriteAuthorizer $writeAuthorizer, + private SubjectIdParser $subjectIdParser, ) { } @@ -35,7 +37,7 @@ public function setMainSubject( SetMainSubjectRequest $request ): void { return; } - $this->promoteToMain( $pageSubjects, $previousMain, new SubjectId( $request->subjectId ), $pageId, $request->comment ); + $this->promoteToMain( $pageSubjects, $previousMain, $this->subjectIdParser->parse( $request->subjectId ), $pageId, $request->comment ); } private function clearMain( PageSubjects $pageSubjects, ?Subject $previousMain, PageId $pageId, ?string $comment ): void { diff --git a/src/Application/Actions/SetSubjectsOrdering/SetSubjectsOrderingAction.php b/src/Application/Actions/SetSubjectsOrdering/SetSubjectsOrderingAction.php index 38dc614a8..664db211a 100644 --- a/src/Application/Actions/SetSubjectsOrdering/SetSubjectsOrderingAction.php +++ b/src/Application/Actions/SetSubjectsOrdering/SetSubjectsOrderingAction.php @@ -10,6 +10,7 @@ use ProfessionalWiki\NeoWiki\Domain\Page\PageId; use ProfessionalWiki\NeoWiki\Domain\Page\PageSubjects; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; use RuntimeException; readonly class SetSubjectsOrderingAction { @@ -18,6 +19,7 @@ public function __construct( private SetSubjectsOrderingPresenter $presenter, private SubjectRepository $subjectRepository, private SubjectWriteAuthorizer $writeAuthorizer, + private SubjectIdParser $subjectIdParser, ) { } @@ -28,18 +30,23 @@ public function setOrdering( SetSubjectsOrderingRequest $request ): void { throw new RuntimeException( 'You do not have the necessary permissions to change the subject ordering' ); } + try { + $mainSubjectId = $request->mainSubjectId === null ? null : $this->subjectIdParser->parse( $request->mainSubjectId ); + $childSubjectIds = array_map( fn ( string $id ) => $this->subjectIdParser->parse( $id ), $request->childSubjectIds ); + } catch ( InvalidArgumentException $e ) { + $this->presenter->presentInvalidOrdering( $e->getMessage() ); + return; + } + $pageSubjects = $this->subjectRepository->getSubjectsByPageId( $pageId ); - if ( $this->matchesCurrent( $pageSubjects, $request ) ) { + if ( $this->matchesCurrent( $pageSubjects, $mainSubjectId, $childSubjectIds ) ) { $this->presenter->presentNoChange(); return; } try { - $pageSubjects->setOrdering( - $request->mainSubjectId === null ? null : new SubjectId( $request->mainSubjectId ), - array_map( fn ( string $id ) => new SubjectId( $id ), $request->childSubjectIds ) - ); + $pageSubjects->setOrdering( $mainSubjectId, $childSubjectIds ); } catch ( InvalidArgumentException $e ) { $this->presenter->presentInvalidOrdering( $e->getMessage() ); return; @@ -49,12 +56,16 @@ public function setOrdering( SetSubjectsOrderingRequest $request ): void { $this->presenter->presentOrderingChanged(); } - private function matchesCurrent( PageSubjects $pageSubjects, SetSubjectsOrderingRequest $request ): bool { + /** + * @param SubjectId[] $childSubjectIds + */ + private function matchesCurrent( PageSubjects $pageSubjects, ?SubjectId $mainSubjectId, array $childSubjectIds ): bool { $currentMainId = $pageSubjects->getMainSubject()?->id->text; - if ( $currentMainId !== $request->mainSubjectId ) { + if ( $currentMainId !== $mainSubjectId?->text ) { return false; } - return $pageSubjects->getChildSubjects()->getIdsAsTextArray() === $request->childSubjectIds; + return $pageSubjects->getChildSubjects()->getIdsAsTextArray() + === array_map( fn ( SubjectId $id ) => $id->text, $childSubjectIds ); } } diff --git a/src/Application/Queries/GetSubject/GetSubjectQuery.php b/src/Application/Queries/GetSubject/GetSubjectQuery.php index 014e5cc54..61ca6da0c 100644 --- a/src/Application/Queries/GetSubject/GetSubjectQuery.php +++ b/src/Application/Queries/GetSubject/GetSubjectQuery.php @@ -8,7 +8,7 @@ use ProfessionalWiki\NeoWiki\Application\SubjectLookup; use ProfessionalWiki\NeoWiki\Domain\Subject\StatementList; use ProfessionalWiki\NeoWiki\Domain\Subject\Subject; -use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; readonly class GetSubjectQuery { @@ -16,6 +16,7 @@ public function __construct( private GetSubjectPresenter $presenter, private SubjectLookup $subjectLookup, private PageIdentifiersLookup $pageIdentifiersLookup, + private SubjectIdParser $subjectIdParser, ) { } @@ -24,7 +25,7 @@ public function execute( bool $includePageIdentifiers, bool $includeReferencedSubjects ): void { - $subject = $this->subjectLookup->getSubject( new SubjectId( $subjectId ) ); // TODO: error handling on invalid ID + $subject = $this->subjectLookup->getSubject( $this->subjectIdParser->parse( $subjectId ) ); // TODO: error handling on invalid ID if ( $subject === null ) { $this->presenter->presentSubjectNotFound(); diff --git a/src/Application/Queries/ValidateSubjectUpdate/ValidateSubjectUpdateQuery.php b/src/Application/Queries/ValidateSubjectUpdate/ValidateSubjectUpdateQuery.php index aa4327880..9ca06331c 100644 --- a/src/Application/Queries/ValidateSubjectUpdate/ValidateSubjectUpdateQuery.php +++ b/src/Application/Queries/ValidateSubjectUpdate/ValidateSubjectUpdateQuery.php @@ -10,7 +10,7 @@ use ProfessionalWiki\NeoWiki\Application\Subject\Exception\SubjectNotFoundException; use ProfessionalWiki\NeoWiki\Application\SubjectRepository; use ProfessionalWiki\NeoWiki\Application\Validation\SubjectValidator; -use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectLabel; use ProfessionalWiki\NeoWiki\Domain\Validation\Violation; @@ -22,6 +22,7 @@ public function __construct( private SubjectValidator $subjectValidator, private StatementListBuilder $statementListBuilder, private SelectStatementResolver $selectStatementResolver, + private SubjectIdParser $subjectIdParser, ) { } @@ -34,7 +35,7 @@ public function __construct( * @throws SubjectNotFoundException when the subject does not exist. */ public function validate( string $subjectId, string $label, array $statements ): array { - $id = new SubjectId( $subjectId ); + $id = $this->subjectIdParser->parse( $subjectId ); $subject = $this->subjectRepository->getSubject( $id ); if ( $subject === null ) { diff --git a/src/Application/StatementListBuilder.php b/src/Application/StatementListBuilder.php index fa95aadb3..cb9f8d944 100644 --- a/src/Application/StatementListBuilder.php +++ b/src/Application/StatementListBuilder.php @@ -11,7 +11,7 @@ use ProfessionalWiki\NeoWiki\Domain\Schema\PropertyName; use ProfessionalWiki\NeoWiki\Domain\Statement; use ProfessionalWiki\NeoWiki\Domain\Subject\StatementList; -use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; use ProfessionalWiki\NeoWiki\Domain\Value\BooleanValue; use ProfessionalWiki\NeoWiki\Domain\Value\NeoValue; use ProfessionalWiki\NeoWiki\Domain\Value\NumberValue; @@ -26,6 +26,7 @@ public function __construct( private PropertyTypeLookup $propertyTypeLookup, private IdGenerator $idGenerator, + private SubjectIdParser $subjectIdParser, ) { } @@ -77,7 +78,7 @@ private function deserializeRelationValue( array $json ): RelationValue { if ( is_array( $relation ) ) { $relations[] = new Relation( id: $this->buildRelationId( $relation ), - targetId: new SubjectId( $relation['target'] ), + targetId: $this->subjectIdParser->parse( $relation['target'] ), properties: new RelationProperties( $relation['properties'] ?? [] ) ); } diff --git a/src/Application/SubjectResolver.php b/src/Application/SubjectResolver.php index 1b189fdb5..04922796d 100644 --- a/src/Application/SubjectResolver.php +++ b/src/Application/SubjectResolver.php @@ -8,23 +8,20 @@ use ProfessionalWiki\NeoWiki\Domain\Page\PageSubjects; use ProfessionalWiki\NeoWiki\Domain\Relation\Relation; use ProfessionalWiki\NeoWiki\Domain\Subject\Subject; -use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; class SubjectResolver { public function __construct( private readonly SubjectContentRepository $subjectContentRepository, private readonly SubjectLookup $subjectLookup, + private readonly SubjectIdParser $subjectIdParser, ) { } public function resolveById( string $subjectIdText ): ?Subject { - if ( !SubjectId::isValid( $subjectIdText ) ) { - return null; - } - try { - return $this->subjectLookup->getSubject( new SubjectId( $subjectIdText ) ); + return $this->subjectLookup->getSubject( $this->subjectIdParser->parse( $subjectIdText ) ); } catch ( \Exception ) { return null; } diff --git a/src/Domain/Subject/SubjectId.php b/src/Domain/Subject/SubjectId.php index 21b7fd20d..4591d5d30 100644 --- a/src/Domain/Subject/SubjectId.php +++ b/src/Domain/Subject/SubjectId.php @@ -9,10 +9,21 @@ readonly class SubjectId implements Stringable { + private const string BARE_PATTERN = '/^s[123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]{14}\z/'; + private const string QUALIFIED_PATTERN = '/^([A-Za-z0-9+_-]+):([A-Za-z0-9._~!$&\'()*+,;=:@-]+)\z/'; + public string $text; + private ?string $source; + private string $localId; public function __construct( string $text ) { - if ( !self::isValid( $text ) ) { + if ( preg_match( self::BARE_PATTERN, $text ) === 1 ) { + $this->source = null; + $this->localId = $text; + } elseif ( preg_match( self::QUALIFIED_PATTERN, $text, $matches ) === 1 ) { + $this->source = $matches[1]; + $this->localId = $matches[2]; + } else { throw new \InvalidArgumentException( "Subject ID has the wrong format: '$text'" ); } @@ -28,7 +39,19 @@ public static function createNew( IdGenerator $idGenerator ): self { } public static function isValid( string $text ): bool { - return preg_match( '/^s[123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]{14}$/', $text ) === 1; + return preg_match( self::BARE_PATTERN, $text ) === 1 + || preg_match( self::QUALIFIED_PATTERN, $text ) === 1; + } + + /** + * The source key, or null for a local Subject (bare id form). + */ + public function getSource(): ?string { + return $this->source; + } + + public function getLocalId(): string { + return $this->localId; } public function __toString(): string { diff --git a/src/Domain/Subject/SubjectIdParser.php b/src/Domain/Subject/SubjectIdParser.php new file mode 100644 index 000000000..36c532863 --- /dev/null +++ b/src/Domain/Subject/SubjectIdParser.php @@ -0,0 +1,42 @@ +getSource() === $this->localSourceKey ) { + return $this->newLocalId( $id->getLocalId(), $text ); + } + + return $id; + } + + private function newLocalId( string $localId, string $originalText ): SubjectId { + $bareId = new SubjectId( $localId ); + + if ( $bareId->getSource() !== null ) { + throw new \InvalidArgumentException( "Local Subject IDs must be bare: '$originalText'" ); + } + + return $bareId; + } + +} diff --git a/src/EntryPoints/NeoWikiHooks.php b/src/EntryPoints/NeoWikiHooks.php index 8ee10f858..3ba339cdb 100644 --- a/src/EntryPoints/NeoWikiHooks.php +++ b/src/EntryPoints/NeoWikiHooks.php @@ -163,6 +163,7 @@ static function ( Parser $parser, string ...$args ): string|array { new SubjectResolver( $extension->newSubjectContentRepository(), $extension->getSubjectRepository(), + $extension->getSubjectIdParser(), ) ); return $parserFunction->handle( $parser, ...$args ); diff --git a/src/EntryPoints/REST/DeleteSubjectApi.php b/src/EntryPoints/REST/DeleteSubjectApi.php index 60f942b99..7b6361e65 100644 --- a/src/EntryPoints/REST/DeleteSubjectApi.php +++ b/src/EntryPoints/REST/DeleteSubjectApi.php @@ -7,7 +7,6 @@ use MediaWiki\Rest\HttpException; use MediaWiki\Rest\Response; use MediaWiki\Rest\SimpleHandler; -use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Presentation\CsrfValidator; use Wikimedia\ParamValidator\ParamValidator; @@ -29,9 +28,11 @@ public function run( string $subjectId ): Response { $body = $this->getValidatedBody() ?? []; $comment = $body['comment'] ?? null; + $extension = NeoWikiExtension::getInstance(); + try { - NeoWikiExtension::getInstance()->newDeleteSubjectAction( $this->getAuthority() )->deleteSubject( - new SubjectId( $subjectId ), + $extension->newDeleteSubjectAction( $this->getAuthority() )->deleteSubject( + $extension->getSubjectIdParser()->parse( $subjectId ), $comment ); } catch ( \RuntimeException $e ) { @@ -50,7 +51,7 @@ public function getParamSettings(): array { self::PARAM_SOURCE => 'path', ParamValidator::PARAM_TYPE => 'string', ParamValidator::PARAM_REQUIRED => true, - self::PARAM_DESCRIPTION => 'Persistent identifier of the Subject. 15 characters, starting with "s".', + self::PARAM_DESCRIPTION => 'Persistent identifier of the Subject: a bare local id ("s" + 14 characters) or a source-qualified id ("source:localId").', ], ]; } diff --git a/src/EntryPoints/REST/GetSubjectApi.php b/src/EntryPoints/REST/GetSubjectApi.php index b88b84048..2793cb5ed 100644 --- a/src/EntryPoints/REST/GetSubjectApi.php +++ b/src/EntryPoints/REST/GetSubjectApi.php @@ -61,7 +61,7 @@ public function getParamSettings(): array { self::PARAM_SOURCE => 'path', ParamValidator::PARAM_TYPE => 'string', ParamValidator::PARAM_REQUIRED => true, - self::PARAM_DESCRIPTION => 'Persistent identifier of the Subject. 15 characters, starting with "s".', + self::PARAM_DESCRIPTION => 'Persistent identifier of the Subject: a bare local id ("s" + 14 characters) or a source-qualified id ("source:localId").', ], 'revisionId' => [ self::PARAM_SOURCE => 'query', diff --git a/src/EntryPoints/REST/ReplaceSubjectApi.php b/src/EntryPoints/REST/ReplaceSubjectApi.php index 0164d45f8..0e34342df 100644 --- a/src/EntryPoints/REST/ReplaceSubjectApi.php +++ b/src/EntryPoints/REST/ReplaceSubjectApi.php @@ -10,7 +10,6 @@ use MediaWiki\Rest\SimpleHandler; use ProfessionalWiki\NeoWiki\Application\Subject\Exception\SubjectEditNotAuthorizedException; use ProfessionalWiki\NeoWiki\Application\Subject\Exception\SubjectNotFoundException; -use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Presentation\CsrfValidator; use ProfessionalWiki\NeoWiki\Presentation\RestReplaceSubjectPresenter; @@ -33,9 +32,11 @@ public function run( string $subjectId ): Response { $presenter = new RestReplaceSubjectPresenter(); + $extension = NeoWikiExtension::getInstance(); + try { - NeoWikiExtension::getInstance()->newReplaceSubjectAction( $presenter, $this->getAuthority() )->replace( - new SubjectId( $subjectId ), + $extension->newReplaceSubjectAction( $presenter, $this->getAuthority() )->replace( + $extension->getSubjectIdParser()->parse( $subjectId ), $body['label'], $body['statements'], $body['comment'] ?? null @@ -68,7 +69,7 @@ public function getParamSettings(): array { self::PARAM_SOURCE => 'path', ParamValidator::PARAM_TYPE => 'string', ParamValidator::PARAM_REQUIRED => true, - self::PARAM_DESCRIPTION => 'Persistent identifier of the Subject. 15 characters, starting with "s".', + self::PARAM_DESCRIPTION => 'Persistent identifier of the Subject: a bare local id ("s" + 14 characters) or a source-qualified id ("source:localId").', ], ]; } diff --git a/src/EntryPoints/REST/SetMainSubjectApi.php b/src/EntryPoints/REST/SetMainSubjectApi.php index d011c1fcb..bd7f7d66e 100644 --- a/src/EntryPoints/REST/SetMainSubjectApi.php +++ b/src/EntryPoints/REST/SetMainSubjectApi.php @@ -87,7 +87,7 @@ public function getBodyParamSettings(): array { self::PARAM_SOURCE => 'body', ParamValidator::PARAM_TYPE => 'string', ParamValidator::PARAM_REQUIRED => false, - self::PARAM_DESCRIPTION => 'Subject ID (15 characters, starting with "s") to promote to Main Subject, or null to clear the Main Subject.', + self::PARAM_DESCRIPTION => 'Subject ID (bare local or source-qualified form) to promote to Main Subject, or null to clear the Main Subject.', ], 'comment' => [ self::PARAM_SOURCE => 'body', diff --git a/src/EntryPoints/REST/SetSubjectsOrderingApi.php b/src/EntryPoints/REST/SetSubjectsOrderingApi.php index 42ee47e38..bc3ebe37d 100644 --- a/src/EntryPoints/REST/SetSubjectsOrderingApi.php +++ b/src/EntryPoints/REST/SetSubjectsOrderingApi.php @@ -100,7 +100,7 @@ public function getBodyParamSettings(): array { self::PARAM_SOURCE => 'body', ParamValidator::PARAM_TYPE => 'string', ParamValidator::PARAM_REQUIRED => false, - self::PARAM_DESCRIPTION => 'Subject ID (15 characters, starting with "s") to be the Main Subject, or null to clear it.', + self::PARAM_DESCRIPTION => 'Subject ID (bare local or source-qualified form) to be the Main Subject, or null to clear it.', ], 'childSubjectIds' => [ self::PARAM_SOURCE => 'body', diff --git a/src/EntryPoints/REST/ValidateSubjectUpdateApi.php b/src/EntryPoints/REST/ValidateSubjectUpdateApi.php index 42289d45f..b2cd12f1c 100644 --- a/src/EntryPoints/REST/ValidateSubjectUpdateApi.php +++ b/src/EntryPoints/REST/ValidateSubjectUpdateApi.php @@ -51,7 +51,7 @@ public function getParamSettings(): array { self::PARAM_SOURCE => 'path', ParamValidator::PARAM_TYPE => 'string', ParamValidator::PARAM_REQUIRED => true, - self::PARAM_DESCRIPTION => 'Persistent identifier of the Subject. 15 characters, starting with "s".', + self::PARAM_DESCRIPTION => 'Persistent identifier of the Subject: a bare local id ("s" + 14 characters) or a source-qualified id ("source:localId").', ], ]; } diff --git a/src/EntryPoints/Scribunto/ScribuntoLuaLibrary.php b/src/EntryPoints/Scribunto/ScribuntoLuaLibrary.php index 8b04496fd..f34703459 100644 --- a/src/EntryPoints/Scribunto/ScribuntoLuaLibrary.php +++ b/src/EntryPoints/Scribunto/ScribuntoLuaLibrary.php @@ -29,6 +29,7 @@ private function getSubjectDataLookup(): SubjectDataLookup { new SubjectResolver( $extension->newSubjectContentRepository(), $extension->getSubjectRepository(), + $extension->getSubjectIdParser(), ), ); } diff --git a/src/NeoWikiExtension.php b/src/NeoWikiExtension.php index b7bef8649..c22eed1c2 100644 --- a/src/NeoWikiExtension.php +++ b/src/NeoWikiExtension.php @@ -75,6 +75,7 @@ use ProfessionalWiki\NeoWiki\EntryPoints\REST\ExportPageRdfApi; use ProfessionalWiki\NeoWiki\GraphDatabasePlugins\Neo4j\Persistence\Neo4jWriteQueryEngine; use ProfessionalWiki\NeoWiki\Domain\PropertyType\PropertyTypeLookup; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; use ProfessionalWiki\NeoWiki\Domain\PropertyType\PropertyTypeRegistry; use ProfessionalWiki\NeoWiki\EntryPoints\NeoWikiRegistrar; use ProfessionalWiki\NeoWiki\EntryPoints\OnRevisionCreatedHandler; @@ -238,7 +239,14 @@ private function ensureExtensionsRegistered(): void { } public function newSubjectContentDataDeserializer(): SubjectContentDataDeserializer { - return new SubjectContentDataDeserializer( new StatementDeserializer( $this->getPropertyTypeLookup() ) ); + return new SubjectContentDataDeserializer( + new StatementDeserializer( $this->getPropertyTypeLookup(), $this->getSubjectIdParser() ), + $this->getSubjectIdParser() + ); + } + + public function getSubjectIdParser(): SubjectIdParser { + return new SubjectIdParser( $this->config->wikiId ); } public function getPagePropertyProviderRegistry(): PagePropertyProviderRegistry { @@ -678,7 +686,8 @@ private function getIdGenerator(): IdGenerator { public function getStatementListBuilder(): StatementListBuilder { return new StatementListBuilder( propertyTypeLookup: $this->getPropertyTypeLookup(), - idGenerator: $this->getIdGenerator() + idGenerator: $this->getIdGenerator(), + subjectIdParser: $this->getSubjectIdParser() ); } @@ -706,6 +715,7 @@ public function newSetMainSubjectAction( SetMainSubjectPresenter $presenter, Aut presenter: $presenter, subjectRepository: $this->getSubjectRepository(), writeAuthorizer: $this->newSubjectWriteAuthorizer( $authority ), + subjectIdParser: $this->getSubjectIdParser(), ); } @@ -714,6 +724,7 @@ public function newSetSubjectsOrderingAction( SetSubjectsOrderingPresenter $pres presenter: $presenter, subjectRepository: $this->getSubjectRepository(), writeAuthorizer: $this->newSubjectWriteAuthorizer( $authority ), + subjectIdParser: $this->getSubjectIdParser(), ); } @@ -830,6 +841,7 @@ public function newGetSubjectQuery( RestGetSubjectPresenter $presenter ): GetSub presenter: $presenter, subjectLookup: $this->getSubjectRepository(), pageIdentifiersLookup: $this->getPageIdentifiersLookup(), + subjectIdParser: $this->getSubjectIdParser(), ); } @@ -843,6 +855,7 @@ public function newGetSubjectQueryForRevision( RestGetSubjectPresenter $presente primaryRevision: $revision, ), pageIdentifiersLookup: $this->getPageIdentifiersLookup(), + subjectIdParser: $this->getSubjectIdParser(), ); } @@ -896,6 +909,7 @@ public function newValidateSubjectUpdateQuery(): ValidateSubjectUpdateQuery { subjectValidator: $this->getSubjectValidator(), statementListBuilder: $this->getStatementListBuilder(), selectStatementResolver: $this->getSelectStatementResolver(), + subjectIdParser: $this->getSubjectIdParser(), ); } diff --git a/src/Persistence/MediaWiki/Subject/StatementDeserializer.php b/src/Persistence/MediaWiki/Subject/StatementDeserializer.php index 603ddc6ce..4e72c0e31 100644 --- a/src/Persistence/MediaWiki/Subject/StatementDeserializer.php +++ b/src/Persistence/MediaWiki/Subject/StatementDeserializer.php @@ -9,7 +9,7 @@ use ProfessionalWiki\NeoWiki\Domain\Relation\RelationProperties; use ProfessionalWiki\NeoWiki\Domain\Schema\PropertyName; use ProfessionalWiki\NeoWiki\Domain\Statement; -use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; use ProfessionalWiki\NeoWiki\Domain\Value\BooleanValue; use ProfessionalWiki\NeoWiki\Domain\Value\NeoValue; use ProfessionalWiki\NeoWiki\Domain\Value\NumberValue; @@ -28,7 +28,8 @@ class StatementDeserializer { public function __construct( - private readonly PropertyTypeLookup $propertyTypeLookup + private readonly PropertyTypeLookup $propertyTypeLookup, + private readonly SubjectIdParser $subjectIdParser, ) { } @@ -59,7 +60,7 @@ private function deserializeRelationValue( array $json ): RelationValue { foreach ( $json as $relation ) { $relations[] = new Relation( id: new RelationId( $relation['id'] ), - targetId: new SubjectId( $relation['target'] ), + targetId: $this->subjectIdParser->parse( $relation['target'] ), properties: new RelationProperties( $relation['properties'] ?? [] ) ); } diff --git a/src/Persistence/MediaWiki/Subject/SubjectContentDataDeserializer.php b/src/Persistence/MediaWiki/Subject/SubjectContentDataDeserializer.php index 4c47fc25f..ee6d40da8 100644 --- a/src/Persistence/MediaWiki/Subject/SubjectContentDataDeserializer.php +++ b/src/Persistence/MediaWiki/Subject/SubjectContentDataDeserializer.php @@ -8,7 +8,7 @@ use ProfessionalWiki\NeoWiki\Domain\Schema\SchemaName; use ProfessionalWiki\NeoWiki\Domain\Subject\StatementList; use ProfessionalWiki\NeoWiki\Domain\Subject\Subject; -use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectLabel; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectMap; @@ -16,6 +16,7 @@ class SubjectContentDataDeserializer { public function __construct( private readonly StatementDeserializer $statementDeserializer, + private readonly SubjectIdParser $subjectIdParser, ) { } @@ -33,7 +34,7 @@ public function deserialize( string $json ): PageSubjects { ); } - $mainSubjectId = new SubjectId( $jsonArray['mainSubject'] ); + $mainSubjectId = $this->subjectIdParser->parse( $jsonArray['mainSubject'] ); return new PageSubjects( $subjects->getSubject( $mainSubjectId ), @@ -55,7 +56,7 @@ private function deserializeSubjects( array $jsonArray ): SubjectMap { private function deserializeSubject( string $id, array $jsonArray ): Subject { return new Subject( - id: new SubjectId( $id ), + id: $this->subjectIdParser->parse( $id ), label: new SubjectLabel( $jsonArray['label'] ), schemaName: new SchemaName( $jsonArray['schema'] ), statements: $this->buildStatementList( $jsonArray ), diff --git a/tests/phpunit/Application/Actions/CreateSubjectActionTest.php b/tests/phpunit/Application/Actions/CreateSubjectActionTest.php index 2e2d24a1d..8a71762b5 100644 --- a/tests/phpunit/Application/Actions/CreateSubjectActionTest.php +++ b/tests/phpunit/Application/Actions/CreateSubjectActionTest.php @@ -29,6 +29,7 @@ use ProfessionalWiki\NeoWiki\Domain\PropertyType\PropertyTypeRegistry; use ProfessionalWiki\NeoWiki\Infrastructure\IdGenerator; use ProfessionalWiki\NeoWiki\Application\SubjectWriteAuthorizer; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\Data\TestRelation; use ProfessionalWiki\NeoWiki\Tests\Data\TestStatement; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemorySchemaLookup; @@ -68,7 +69,8 @@ private function newCreateSubjectAction( bool $validationEnforced = false ): Cre $this->authorizer, new StatementListBuilder( $registry, - $this->idGenerator + $this->idGenerator, + TestData::newSubjectIdParser() ), $this->schemaLookup, new SelectStatementResolver( new SelectValueResolver() ), diff --git a/tests/phpunit/Application/Actions/ReplaceSubjectActionTest.php b/tests/phpunit/Application/Actions/ReplaceSubjectActionTest.php index bbbc61e90..f197328cb 100644 --- a/tests/phpunit/Application/Actions/ReplaceSubjectActionTest.php +++ b/tests/phpunit/Application/Actions/ReplaceSubjectActionTest.php @@ -31,6 +31,7 @@ use ProfessionalWiki\NeoWiki\Domain\Value\StringValue; use ProfessionalWiki\NeoWiki\Application\SubjectWriteAuthorizer; use ProfessionalWiki\NeoWiki\Domain\Value\UnregisteredTypeValue; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\Data\TestStatement; use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemoryPageIdentifiersLookup; @@ -64,7 +65,8 @@ private function newAction( $registry = PropertyTypeRegistry::withCoreTypes(); $builder = new StatementListBuilder( propertyTypeLookup: $registry, - idGenerator: new StubIdGenerator( '11111111111127' ) + idGenerator: new StubIdGenerator( '11111111111127' ), + subjectIdParser: TestData::newSubjectIdParser() ); return new ReplaceSubjectAction( subjectRepository: $this->subjectRepository, diff --git a/tests/phpunit/Application/Actions/SetMainSubjectActionTest.php b/tests/phpunit/Application/Actions/SetMainSubjectActionTest.php index e64a30778..397d8b46e 100644 --- a/tests/phpunit/Application/Actions/SetMainSubjectActionTest.php +++ b/tests/phpunit/Application/Actions/SetMainSubjectActionTest.php @@ -11,6 +11,7 @@ use ProfessionalWiki\NeoWiki\Domain\Page\PageId; use ProfessionalWiki\NeoWiki\Domain\Page\PageSubjects; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectMap; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemorySubjectRepository; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\SpySubjectWriteAuthorizer; @@ -112,6 +113,7 @@ public function testThrowsWhenUserMayNotEditSubject(): void { presenter: $this->newSpyPresenter(), subjectRepository: new InMemorySubjectRepository(), writeAuthorizer: new SpySubjectWriteAuthorizer( allowed: false ), + subjectIdParser: TestData::newSubjectIdParser(), ); $this->expectException( \RuntimeException::class ); @@ -137,6 +139,7 @@ private function newAction( SetMainSubjectPresenter $presenter, InMemorySubjectR presenter: $presenter, subjectRepository: $repository, writeAuthorizer: new SpySubjectWriteAuthorizer( allowed: true ), + subjectIdParser: TestData::newSubjectIdParser(), ); } diff --git a/tests/phpunit/Application/Actions/SetSubjectsOrderingActionTest.php b/tests/phpunit/Application/Actions/SetSubjectsOrderingActionTest.php index 698b7056d..633b71e61 100644 --- a/tests/phpunit/Application/Actions/SetSubjectsOrderingActionTest.php +++ b/tests/phpunit/Application/Actions/SetSubjectsOrderingActionTest.php @@ -11,6 +11,7 @@ use ProfessionalWiki\NeoWiki\Domain\Page\PageId; use ProfessionalWiki\NeoWiki\Domain\Page\PageSubjects; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectMap; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemorySubjectRepository; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\SpySubjectWriteAuthorizer; @@ -109,6 +110,31 @@ public function testReportsNoChangeWhenOrderingMatchesCurrent(): void { $this->assertEquals( $before, $repository->getSubjectsByPageId( new PageId( self::PAGE_ID ) ) ); } + public function testReportsNoChangeWhenQualifiedLocalIdsMatchCurrentOrdering(): void { + $repository = $this->newRepositoryWithMainAndThreeChildren(); + $presenter = $this->newSpyPresenter(); + + $this->newAction( $presenter, $repository )->setOrdering( + new SetSubjectsOrderingRequest( + pageId: self::PAGE_ID, + mainSubjectId: $this->qualifiedLocalIdFor( self::MAIN_ID ), + childSubjectIds: [ + $this->qualifiedLocalIdFor( self::FIRST_ID ), + $this->qualifiedLocalIdFor( self::SECOND_ID ), + $this->qualifiedLocalIdFor( self::THIRD_ID ), + ], + ) + ); + + $this->assertTrue( $presenter->noChange ); + $this->assertFalse( $presenter->changed ); + $this->assertSame( 1, $repository->savePageSubjectsCallCount ); + } + + private function qualifiedLocalIdFor( string $bareId ): string { + return TestData::LOCAL_SOURCE_KEY . ':' . $bareId; + } + public function testReportsInvalidOrderingOnUnknownId(): void { $repository = $this->newRepositoryWithMainAndThreeChildren(); $before = $repository->getSubjectsByPageId( new PageId( self::PAGE_ID ) ); @@ -190,6 +216,7 @@ public function testThrowsWhenUserMayNotEditSubject(): void { presenter: $this->newSpyPresenter(), subjectRepository: new InMemorySubjectRepository(), writeAuthorizer: new SpySubjectWriteAuthorizer( allowed: false ), + subjectIdParser: TestData::newSubjectIdParser(), ); $this->expectException( RuntimeException::class ); @@ -225,6 +252,7 @@ private function newAction( SetSubjectsOrderingPresenter $presenter, InMemorySub presenter: $presenter, subjectRepository: $repository, writeAuthorizer: new SpySubjectWriteAuthorizer( allowed: true ), + subjectIdParser: TestData::newSubjectIdParser(), ); } diff --git a/tests/phpunit/Application/Queries/GetSubject/GetSubjectQueryTest.php b/tests/phpunit/Application/Queries/GetSubject/GetSubjectQueryTest.php index 4b5180936..3c9786e85 100644 --- a/tests/phpunit/Application/Queries/GetSubject/GetSubjectQueryTest.php +++ b/tests/phpunit/Application/Queries/GetSubject/GetSubjectQueryTest.php @@ -18,6 +18,7 @@ use ProfessionalWiki\NeoWiki\Domain\Value\NumberValue; use ProfessionalWiki\NeoWiki\Domain\Value\RelationValue; use ProfessionalWiki\NeoWiki\Domain\PropertyType\Types\RelationType; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\Data\TestRelation; use ProfessionalWiki\NeoWiki\Tests\Data\TestStatement; use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; @@ -56,6 +57,7 @@ public function testPresentsSubjectInHappyPathResponse(): void { ), ), new InMemoryPageIdentifiersLookup(), + TestData::newSubjectIdParser(), ); $query->execute( @@ -128,6 +130,7 @@ public function testPresentsSubjectNotFound(): void { $spyPresenter, new InMemorySubjectLookup(), new InMemoryPageIdentifiersLookup(), + TestData::newSubjectIdParser(), ); $query->execute( @@ -150,6 +153,7 @@ public function testIncludePageIdentifiers(): void { [ new SubjectId( TestSubject::ZERO_GUID ), new PageIdentifiers( new PageId( 1 ), 'wrong title', 0 ) ], [ $subject->id, new PageIdentifiers( new PageId( 42 ), 'right title', 12 ) ], ] ), + TestData::newSubjectIdParser(), ); $query->execute( @@ -198,6 +202,7 @@ public function testIncludeReferencedSubjects(): void { [ $subject->id, new PageIdentifiers( new PageId( 42 ), 'subject title', 0 ) ], [ $referencedSubject->id, new PageIdentifiers( new PageId( 1337 ), 'referenced title', 12 ) ], ] ), + TestData::newSubjectIdParser(), ); $query->execute( diff --git a/tests/phpunit/Application/StatementListBuilderTest.php b/tests/phpunit/Application/StatementListBuilderTest.php index 9434a07e4..b6f726789 100644 --- a/tests/phpunit/Application/StatementListBuilderTest.php +++ b/tests/phpunit/Application/StatementListBuilderTest.php @@ -9,6 +9,7 @@ use ProfessionalWiki\NeoWiki\Domain\PropertyType\PropertyTypeRegistry; use ProfessionalWiki\NeoWiki\Domain\Schema\PropertyName; use ProfessionalWiki\NeoWiki\Domain\Value\UnregisteredTypeValue; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\StubIdGenerator; /** @@ -19,7 +20,8 @@ class StatementListBuilderTest extends TestCase { private function newBuilder(): StatementListBuilder { return new StatementListBuilder( propertyTypeLookup: PropertyTypeRegistry::withCoreTypes(), - idGenerator: new StubIdGenerator( '11111111111111' ) + idGenerator: new StubIdGenerator( '11111111111111' ), + subjectIdParser: TestData::newSubjectIdParser() ); } diff --git a/tests/phpunit/Application/SubjectResolverTest.php b/tests/phpunit/Application/SubjectResolverTest.php index 5330ccbcb..be4abb1a7 100644 --- a/tests/phpunit/Application/SubjectResolverTest.php +++ b/tests/phpunit/Application/SubjectResolverTest.php @@ -18,6 +18,7 @@ use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectId; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectLabel; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectMap; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemorySubjectContentRepository; /** @@ -47,7 +48,7 @@ public function testResolveByIdReturnsSubject(): void { $lookup = $this->createStub( SubjectLookup::class ); $lookup->method( 'getSubject' )->willReturn( $subject ); - $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup ); + $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup, TestData::newSubjectIdParser() ); $this->assertSame( $subject, $resolver->resolveById( self::SUBJECT_ID ) ); } @@ -55,7 +56,8 @@ public function testResolveByIdReturnsSubject(): void { public function testResolveByIdReturnsNullForInvalidId(): void { $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), - $this->createStub( SubjectLookup::class ) + $this->createStub( SubjectLookup::class ), + TestData::newSubjectIdParser() ); $this->assertNull( $resolver->resolveById( 'invalid' ) ); @@ -65,7 +67,7 @@ public function testResolveByIdReturnsNullWhenLookupReturnsNull(): void { $lookup = $this->createStub( SubjectLookup::class ); $lookup->method( 'getSubject' )->willReturn( null ); - $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup ); + $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup, TestData::newSubjectIdParser() ); $this->assertNull( $resolver->resolveById( self::SUBJECT_ID ) ); } @@ -74,7 +76,7 @@ public function testResolveByIdReturnsNullWhenLookupThrows(): void { $lookup = $this->createStub( SubjectLookup::class ); $lookup->method( 'getSubject' )->willThrowException( new \RuntimeException( 'db error' ) ); - $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup ); + $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup, TestData::newSubjectIdParser() ); $this->assertNull( $resolver->resolveById( self::SUBJECT_ID ) ); } @@ -84,7 +86,8 @@ public function testResolveMainByTitleReturnsMainSubject(): void { $resolver = new SubjectResolver( $this->repositoryWithMainSubject( $subject ), - $this->createStub( SubjectLookup::class ) + $this->createStub( SubjectLookup::class ), + TestData::newSubjectIdParser() ); $this->assertSame( $subject, $resolver->resolveMainByTitle( $this->createStub( Title::class ) ) ); @@ -93,7 +96,8 @@ public function testResolveMainByTitleReturnsMainSubject(): void { public function testResolveMainByTitleReturnsNullWhenNoContent(): void { $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), - $this->createStub( SubjectLookup::class ) + $this->createStub( SubjectLookup::class ), + TestData::newSubjectIdParser() ); $this->assertNull( $resolver->resolveMainByTitle( $this->createStub( Title::class ) ) ); @@ -104,7 +108,8 @@ public function testGetPageSubjectsByTitleReturnsPageSubjects(): void { $resolver = new SubjectResolver( $this->repositoryWithMainSubject( $subject ), - $this->createStub( SubjectLookup::class ) + $this->createStub( SubjectLookup::class ), + TestData::newSubjectIdParser() ); $pageSubjects = $resolver->getPageSubjectsByTitle( $this->createStub( Title::class ) ); @@ -116,7 +121,8 @@ public function testGetPageSubjectsByTitleReturnsPageSubjects(): void { public function testGetPageSubjectsByTitleReturnsNullWhenNoContent(): void { $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), - $this->createStub( SubjectLookup::class ) + $this->createStub( SubjectLookup::class ), + TestData::newSubjectIdParser() ); $this->assertNull( $resolver->getPageSubjectsByTitle( $this->createStub( Title::class ) ) ); @@ -128,7 +134,7 @@ public function testResolveRelationLabelReturnsLabelWhenTargetExists(): void { $lookup = $this->createStub( SubjectLookup::class ); $lookup->method( 'getSubject' )->willReturn( $target ); - $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup ); + $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup, TestData::newSubjectIdParser() ); $relation = new Relation( id: new RelationId( 'r1test5cccccccc' ), @@ -143,7 +149,7 @@ public function testResolveRelationLabelFallsBackToIdWhenTargetNotFound(): void $lookup = $this->createStub( SubjectLookup::class ); $lookup->method( 'getSubject' )->willReturn( null ); - $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup ); + $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup, TestData::newSubjectIdParser() ); $relation = new Relation( id: new RelationId( 'r1test5cccccccc' ), @@ -158,7 +164,7 @@ public function testResolveRelationLabelFallsBackToIdWhenLookupThrows(): void { $lookup = $this->createStub( SubjectLookup::class ); $lookup->method( 'getSubject' )->willThrowException( new \RuntimeException( 'db error' ) ); - $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup ); + $resolver = new SubjectResolver( new InMemorySubjectContentRepository(), $lookup, TestData::newSubjectIdParser() ); $relation = new Relation( id: new RelationId( 'r1test5cccccccc' ), diff --git a/tests/phpunit/Data/TestData.php b/tests/phpunit/Data/TestData.php index 382c3e9e6..5e79c0f48 100644 --- a/tests/phpunit/Data/TestData.php +++ b/tests/phpunit/Data/TestData.php @@ -4,10 +4,18 @@ namespace ProfessionalWiki\NeoWiki\Tests\Data; +use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectIdParser; + class TestData { + public const string LOCAL_SOURCE_KEY = 'testwiki'; + public static function getFileContents( string $fileName ): string { return file_get_contents( __DIR__ . '/../../../DemoData/' . $fileName ); } + public static function newSubjectIdParser(): SubjectIdParser { + return new SubjectIdParser( self::LOCAL_SOURCE_KEY ); + } + } diff --git a/tests/phpunit/Domain/Subject/SubjectIdParserTest.php b/tests/phpunit/Domain/Subject/SubjectIdParserTest.php new file mode 100644 index 000000000..922b1aae3 --- /dev/null +++ b/tests/phpunit/Domain/Subject/SubjectIdParserTest.php @@ -0,0 +1,72 @@ +newParser()->parse( $input ); + + $this->assertSame( $canonicalText, $id->text ); + $this->assertSame( $source, $id->getSource() ); + $this->assertSame( $localId, $id->getLocalId() ); + } + + /** + * @dataProvider validVectorProvider + */ + public function testCanonicalTextParsesToItself( string $input, string $canonicalText ): void { + $this->assertSame( $canonicalText, $this->newParser()->parse( $canonicalText )->text ); + } + + /** + * @dataProvider invalidVectorProvider + */ + public function testRejectsInvalidVector( string $input ): void { + $this->expectException( \InvalidArgumentException::class ); + $this->newParser()->parse( $input ); + } + + public function testAcceptsLocalSourceKeyOutsideTheKeyGrammar(): void { + $parser = new SubjectIdParser( 'weird~key' ); + + $this->assertSame( 's11111111111111', $parser->parse( 's11111111111111' )->text ); + } + + private function newParser(): SubjectIdParser { + return new SubjectIdParser( self::vectors()['localSourceKey'] ); + } + + public static function validVectorProvider(): iterable { + foreach ( self::vectors()['cases'] as $case ) { + if ( $case['valid'] ) { + yield $case['name'] => [ $case['input'], $case['canonicalText'], $case['source'], $case['localId'] ]; + } + } + } + + public static function invalidVectorProvider(): iterable { + foreach ( self::vectors()['cases'] as $case ) { + if ( !$case['valid'] ) { + yield $case['name'] => [ $case['input'] ]; + } + } + } + + private static function vectors(): array { + return json_decode( file_get_contents( __DIR__ . '/../../../subject-id-vectors.json' ), true ); + } + +} diff --git a/tests/phpunit/Domain/Subject/SubjectIdTest.php b/tests/phpunit/Domain/Subject/SubjectIdTest.php index 097a3b3a8..0c470b09e 100644 --- a/tests/phpunit/Domain/Subject/SubjectIdTest.php +++ b/tests/phpunit/Domain/Subject/SubjectIdTest.php @@ -38,6 +38,25 @@ public function testEquals( string $validGuid ): void { ); } + public function testBareIdHasLocalSourceAndItselfAsLocalId(): void { + $subjectId = new SubjectId( 's11111111111111' ); + + $this->assertNull( $subjectId->getSource() ); + $this->assertSame( 's11111111111111', $subjectId->getLocalId() ); + } + + public function testQualifiedIdExposesSourceAndLocalId(): void { + $subjectId = new SubjectId( 'enwiki:Q42' ); + + $this->assertSame( 'enwiki:Q42', $subjectId->text ); + $this->assertSame( 'enwiki', $subjectId->getSource() ); + $this->assertSame( 'Q42', $subjectId->getLocalId() ); + } + + public function testIsValidAcceptsQualifiedForm(): void { + $this->assertTrue( SubjectId::isValid( 'enwiki:Q42' ) ); + } + /** * @dataProvider invalidGuidProvider */ diff --git a/tests/phpunit/EntryPoints/NeoWikiValueParserFunctionTest.php b/tests/phpunit/EntryPoints/NeoWikiValueParserFunctionTest.php index f328500d1..4dc7c7819 100644 --- a/tests/phpunit/EntryPoints/NeoWikiValueParserFunctionTest.php +++ b/tests/phpunit/EntryPoints/NeoWikiValueParserFunctionTest.php @@ -28,6 +28,7 @@ use ProfessionalWiki\NeoWiki\Domain\Value\StringValue; use ProfessionalWiki\NeoWiki\Domain\Value\UnregisteredTypeValue; use ProfessionalWiki\NeoWiki\EntryPoints\NeoWikiValueParserFunction; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemorySubjectContentRepository; /** @@ -73,7 +74,7 @@ private function createLookupReturning( Subject $subject ): SubjectLookup { private function createPF( SubjectContentRepository $repo, ?SubjectLookup $lookup = null ): NeoWikiValueParserFunction { return new NeoWikiValueParserFunction( - new SubjectResolver( $repo, $lookup ?? $this->createDummyLookup() ) + new SubjectResolver( $repo, $lookup ?? $this->createDummyLookup(), TestData::newSubjectIdParser() ) ); } diff --git a/tests/phpunit/EntryPoints/Scribunto/SubjectDataLookupTest.php b/tests/phpunit/EntryPoints/Scribunto/SubjectDataLookupTest.php index 7d5a23011..b10bca2b6 100644 --- a/tests/phpunit/EntryPoints/Scribunto/SubjectDataLookupTest.php +++ b/tests/phpunit/EntryPoints/Scribunto/SubjectDataLookupTest.php @@ -26,6 +26,7 @@ use ProfessionalWiki\NeoWiki\Domain\Value\StringValue; use ProfessionalWiki\NeoWiki\Domain\Value\UnregisteredTypeValue; use ProfessionalWiki\NeoWiki\EntryPoints\Scribunto\SubjectDataLookup; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemorySubjectContentRepository; /** @@ -57,7 +58,8 @@ private function resolverWithMainSubject( Subject $subject, ?SubjectLookup $subj private function resolverWithPageSubjects( ?PageSubjects $pageSubjects, ?SubjectLookup $subjectLookup = null ): SubjectResolver { return new SubjectResolver( new InMemorySubjectContentRepository( $pageSubjects ), - $subjectLookup ?? $this->createStub( SubjectLookup::class ) + $subjectLookup ?? $this->createStub( SubjectLookup::class ), + TestData::newSubjectIdParser() ); } diff --git a/tests/phpunit/Persistence/MediaWiki/Subject/StatementDeserializerTest.php b/tests/phpunit/Persistence/MediaWiki/Subject/StatementDeserializerTest.php index 2e08c9eac..79b1a5bc9 100644 --- a/tests/phpunit/Persistence/MediaWiki/Subject/StatementDeserializerTest.php +++ b/tests/phpunit/Persistence/MediaWiki/Subject/StatementDeserializerTest.php @@ -17,6 +17,7 @@ use ProfessionalWiki\NeoWiki\Domain\Value\StringValue; use ProfessionalWiki\NeoWiki\Domain\Value\UnregisteredTypeValue; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\StatementDeserializer; +use ProfessionalWiki\NeoWiki\Tests\Data\TestData; /** * @covers \ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\StatementDeserializer @@ -45,7 +46,7 @@ public function testDeserializesNumber(): void { * Core types only: no extension is loaded, so "color" is an unregistered type. */ private function newDeserializer(): StatementDeserializer { - return new StatementDeserializer( PropertyTypeRegistry::withCoreTypes() ); + return new StatementDeserializer( PropertyTypeRegistry::withCoreTypes(), TestData::newSubjectIdParser() ); } public function testDeserializesText(): void { diff --git a/tests/phpunit/Persistence/MediaWiki/Subject/SubjectContentDataDeserializerTest.php b/tests/phpunit/Persistence/MediaWiki/Subject/SubjectContentDataDeserializerTest.php index 8f087b654..2d873293a 100644 --- a/tests/phpunit/Persistence/MediaWiki/Subject/SubjectContentDataDeserializerTest.php +++ b/tests/phpunit/Persistence/MediaWiki/Subject/SubjectContentDataDeserializerTest.php @@ -16,7 +16,6 @@ use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectLabel; use ProfessionalWiki\NeoWiki\Domain\Value\RelationValue; use ProfessionalWiki\NeoWiki\NeoWikiExtension; -use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\StatementDeserializer; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\SubjectContentDataDeserializer; use ProfessionalWiki\NeoWiki\Tests\Data\TestData; @@ -35,9 +34,7 @@ public function testNodeExampleSmokeTest(): void { } private function newDeserializer(): SubjectContentDataDeserializer { - return new SubjectContentDataDeserializer( - new StatementDeserializer( NeoWikiExtension::getInstance()->getPropertyTypeLookup() ) - ); + return NeoWikiExtension::getInstance()->newSubjectContentDataDeserializer(); } public function testMinimalJson(): void { diff --git a/tests/phpunit/Persistence/MediaWiki/Subject/SubjectContentDataSerializerTest.php b/tests/phpunit/Persistence/MediaWiki/Subject/SubjectContentDataSerializerTest.php index 6f0da52b0..d7de2902b 100644 --- a/tests/phpunit/Persistence/MediaWiki/Subject/SubjectContentDataSerializerTest.php +++ b/tests/phpunit/Persistence/MediaWiki/Subject/SubjectContentDataSerializerTest.php @@ -19,6 +19,7 @@ use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectMap; use ProfessionalWiki\NeoWiki\Domain\Value\RelationValue; use ProfessionalWiki\NeoWiki\Domain\Value\StringValue; +use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\StatementDeserializer; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\SubjectContentDataDeserializer; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\SubjectContentDataSerializer; @@ -66,11 +67,7 @@ public function testSerializeMinimalSubject(): void { ); } - public function testSerializeFullSubject(): void { - $serializer = new SubjectContentDataSerializer(); - - $this->assertSame( - '{ + private const string FULL_PAGE_JSON = '{ "mainSubject": "sTestSCDST11112", "subjects": { "sTestSCDST11112": { @@ -84,7 +81,7 @@ public function testSerializeFullSubject(): void { ] }, "founder": { - "type": "string", + "type": "text", "value": [ "John Doe" ] @@ -125,11 +122,27 @@ public function testSerializeFullSubject(): void { } } } -}', +}'; + + public function testSerializeFullSubject(): void { + $serializer = new SubjectContentDataSerializer(); + + $this->assertSame( + self::FULL_PAGE_JSON, $serializer->serialize( $this->newFullSubjectMap() ) ); } + public function testLocalOnlyPageSlotJsonRoundTripsByteIdentically(): void { + $deserializer = NeoWikiExtension::getInstance()->newSubjectContentDataDeserializer(); + $serializer = new SubjectContentDataSerializer(); + + $this->assertSame( + self::FULL_PAGE_JSON, + $serializer->serialize( $deserializer->deserialize( self::FULL_PAGE_JSON ) ) + ); + } + private function newFullSubjectMap(): PageSubjects { $subjects = new SubjectMap( TestSubject::build( @@ -182,7 +195,7 @@ private function newFullSubjectMap(): PageSubjects { ), new Statement( property: new PropertyName( 'founder' ), - propertyType: 'string', + propertyType: 'text', value: new StringValue( 'John Doe' ) ), ] ) @@ -225,7 +238,10 @@ public function testStatementOfUnregisteredTypeRoundTripsByteIdentically(): void * Core types only: no extension is loaded, so "color" is an unregistered type. */ private function roundTrip( string $contentJson ): string { - $deserializer = new SubjectContentDataDeserializer( new StatementDeserializer( PropertyTypeRegistry::withCoreTypes() ) ); + $deserializer = new SubjectContentDataDeserializer( + new StatementDeserializer( PropertyTypeRegistry::withCoreTypes(), TestData::newSubjectIdParser() ), + TestData::newSubjectIdParser() + ); return ( new SubjectContentDataSerializer() )->serialize( $deserializer->deserialize( $contentJson ) ); } diff --git a/tests/phpunit/TestDoubles/InMemorySubjectRepository.php b/tests/phpunit/TestDoubles/InMemorySubjectRepository.php index d668ad5de..40b4fdc24 100644 --- a/tests/phpunit/TestDoubles/InMemorySubjectRepository.php +++ b/tests/phpunit/TestDoubles/InMemorySubjectRepository.php @@ -29,6 +29,8 @@ class InMemorySubjectRepository implements SubjectRepository { public int $updateSubjectCallCount = 0; + public int $savePageSubjectsCallCount = 0; + public function getSubject( SubjectId $subjectId ): ?Subject { return $this->subjects[$subjectId->text] ?? null; } @@ -53,6 +55,7 @@ public function getSubjectsByPageId( PageId $pageId ): PageSubjects { } public function savePageSubjects( PageSubjects $pageSubjects, PageId $pageId, ?string $comment = null ): void { + $this->savePageSubjectsCallCount++; $this->subjectsByPage[$pageId->id] = $pageSubjects; $this->comments[$pageId->id] = $comment; diff --git a/tests/subject-id-vectors.json b/tests/subject-id-vectors.json new file mode 100644 index 000000000..521a56478 --- /dev/null +++ b/tests/subject-id-vectors.json @@ -0,0 +1,47 @@ +{ + "description": "Shared SubjectId parse vectors consumed by both test suites (SubjectIdParserTest.php and SubjectIdParser.unit.spec.ts). Each suite constructs its parser with localSourceKey. Valid cases assert canonicalText, source (null means local) and localId; invalid cases assert rejection. This file encodes the T1 interim source-key and localId grammars; when T2 delegates grammar to Sources, change this file first, watch both suites go red, then fix both implementations.", + "localSourceKey": "testwiki", + "cases": [ + { "name": "bare local id", "input": "sEpfwJLnxyQy6vR", "valid": true, "canonicalText": "sEpfwJLnxyQy6vR", "source": null, "localId": "sEpfwJLnxyQy6vR" }, + { "name": "bare id may contain lowercase o", "input": "soooooooooooooo", "valid": true, "canonicalText": "soooooooooooooo", "source": null, "localId": "soooooooooooooo" }, + { "name": "explicitly-local-qualified canonicalizes to bare", "input": "testwiki:s11111111111111", "valid": true, "canonicalText": "s11111111111111", "source": null, "localId": "s11111111111111" }, + { "name": "source keys are case-sensitive so uppercase local key is a different source", "input": "TESTWIKI:s11111111111111", "valid": true, "canonicalText": "TESTWIKI:s11111111111111", "source": "TESTWIKI", "localId": "s11111111111111" }, + { "name": "foreign source with opaque localId", "input": "enwiki:Q42", "valid": true, "canonicalText": "enwiki:Q42", "source": "enwiki", "localId": "Q42" }, + { "name": "nanoid-shaped localId under a foreign source stays qualified", "input": "enwiki:s11111111111111", "valid": true, "canonicalText": "enwiki:s11111111111111", "source": "enwiki", "localId": "s11111111111111" }, + { "name": "qualified form splits on the first colon only", "input": "wikidata:Q42:rev5", "valid": true, "canonicalText": "wikidata:Q42:rev5", "source": "wikidata", "localId": "Q42:rev5" }, + { "name": "source key that looks like a bare id", "input": "s11111111111111:Q1", "valid": true, "canonicalText": "s11111111111111:Q1", "source": "s11111111111111", "localId": "Q1" }, + { "name": "uppercase and underscore in source key", "input": "IMS_Wiki:123", "valid": true, "canonicalText": "IMS_Wiki:123", "source": "IMS_Wiki", "localId": "123" }, + { "name": "hyphen joiner in source key from the dbname-prefix form", "input": "enwiki-tenant1:Q42", "valid": true, "canonicalText": "enwiki-tenant1:Q42", "source": "enwiki-tenant1", "localId": "Q42" }, + { "name": "plus in source key is an installer-legal dbname character", "input": "wiki+en:Q42", "valid": true, "canonicalText": "wiki+en:Q42", "source": "wiki+en", "localId": "Q42" }, + { "name": "leading digit in source key", "input": "9wiki:abc", "valid": true, "canonicalText": "9wiki:abc", "source": "9wiki", "localId": "abc" }, + { "name": "leading hyphen in source key is an installer-legal dbname character", "input": "-wiki:x", "valid": true, "canonicalText": "-wiki:x", "source": "-wiki", "localId": "x" }, + { "name": "localId may use RFC 3986 pchar sub-delims", "input": "enwiki:user@example=v1,2;x!y", "valid": true, "canonicalText": "enwiki:user@example=v1,2;x!y", "source": "enwiki", "localId": "user@example=v1,2;x!y" }, + { "name": "empty string", "input": "", "valid": false }, + { "name": "bare id must not contain 0", "input": "s01111111111111", "valid": false }, + { "name": "bare id must not contain I", "input": "sIIIIIIIIIIIIII", "valid": false }, + { "name": "bare id must not contain l", "input": "sllllllllllllll", "valid": false }, + { "name": "bare id must not contain O", "input": "sOOOOOOOOOOOOOO", "valid": false }, + { "name": "bare id too short", "input": "s1111111111111", "valid": false }, + { "name": "bare id too long", "input": "s111111111111111", "valid": false }, + { "name": "relation id prefix", "input": "r11111111111111", "valid": false }, + { "name": "uppercase subject prefix", "input": "S11111111111111", "valid": false }, + { "name": "leading whitespace", "input": " s11111111111111", "valid": false }, + { "name": "trailing whitespace", "input": "s11111111111111 ", "valid": false }, + { "name": "bare id with trailing newline", "input": "s11111111111111\n", "valid": false }, + { "name": "qualified id with trailing newline", "input": "enwiki:Q42\n", "valid": false }, + { "name": "explicitly-local-qualified localId must be a bare nanoid", "input": "testwiki:notananoid", "valid": false }, + { "name": "explicitly-local-qualified localId must not itself be qualified", "input": "testwiki:a:b", "valid": false }, + { "name": "empty source key", "input": ":s11111111111111", "valid": false }, + { "name": "empty localId", "input": "enwiki:", "valid": false }, + { "name": "lone separator", "input": ":", "valid": false }, + { "name": "space in source key", "input": "my wiki:x", "valid": false }, + { "name": "slash in source key", "input": "en/wiki:x", "valid": false }, + { "name": "tilde in source key", "input": "weird~key:s11111111111111", "valid": false }, + { "name": "non-ASCII source key", "input": "wikiä:x", "valid": false }, + { "name": "slash in localId", "input": "enwiki:a/b", "valid": false }, + { "name": "space in localId", "input": "enwiki:a b", "valid": false }, + { "name": "percent in localId", "input": "enwiki:a%2Fb", "valid": false }, + { "name": "question mark in localId", "input": "enwiki:x?y", "valid": false }, + { "name": "hash in localId", "input": "enwiki:doc#1", "valid": false } + ] +}