diff --git a/docs/extending/extending.md b/docs/extending/extending.md index 4cec426de..a69e45458 100644 --- a/docs/extending/extending.md +++ b/docs/extending/extending.md @@ -168,10 +168,71 @@ the Page node. No new revision is created. `rebuild()` returns a `PageRefreshOut - `SkippedUnreadableSubjects` — the page's subject slot holds content NeoWiki cannot read as Subjects. - `SkippedUnreadablePageProperties` — the page's properties could not be built, for instance because a provider or the page's own parse threw. +- `SkippedUnpublishableRevision` — a registered revision policy publishes no revision of the page. A graph store that fails is logged and skipped, exactly as on a normal page save; only request timeouts and wiki-database errors throw. +### Choosing which revision NeoWiki publishes + +By default NeoWiki publishes each page's latest revision: that is the revision it projects to the graph stores and +exports as RDF. An approval extension shows readers an approved revision instead, and registers a `RevisionPolicy` so +NeoWiki publishes the same one. + +```php +public function onNeoWikiRegistration( NeoWikiRegistrar $registrar ): void { + $registrar->setRevisionPolicy( new MyApprovalPolicy( $this->approvalLookup ) ); +} +``` + +```php +class MyApprovalPolicy implements RevisionPolicy { + + public function publishesRevision( RevisionRecord $revision ): bool { + return $this->approvalLookup->isApproved( $revision ); + } + + public function publishedRevision( RevisionRecord $revision ): ?RevisionRecord { + return $this->approvalLookup->lastApprovedRevisionOf( $revision->getPage() ); + } + + public function revisionIsReadableBy( RevisionRecord $revision, Authority $viewer ): bool { + return $this->approvalLookup->isApproved( $revision ) + || $viewer->isAllowed( 'my-extension-see-drafts' ); + } + +} +``` + +`publishesRevision()` is asked as a revision is written; return false and the graph keeps whatever it published +before. `publishedRevision()` is asked when a page is reprojected; return the argument to leave the projection alone, +or `null` when the page has nothing publishable. `revisionIsReadableBy()` is asked only when a caller names a revision +itself, which neither publishing method can intercept — a revision it refuses answers exactly like one that does not +exist. + +- **A policy answers for the wiki, not for a viewer.** The graph and the RDF export are one state every reader sees. +- **Only one extension can decide this.** A second policy is refused with a warning in the `NeoWiki` log channel; the + first one keeps deciding. +- **Subject writes read the latest revision**, so a contributor still edits what they last saved. Schemas are the + exception: a Subject is validated against the Schema revision the policy publishes, while the Schema editor shows + the latest one ([#1392](https://github.com/ProfessionalWiki/NeoWiki/issues/1392)). + +Call [`newPageRebuilder()->rebuild( $title )`](#refreshing-a-pages-data-without-an-edit) whenever your extension +changes which revision it approves; nothing else tells NeoWiki the answer has changed. `publishedRevision()` is also +asked on every Schema, Layout and Mapping read and every RDF export, so keep it cheap. + +Two gaps: + +- Schemas and Mappings are read through a cache keyed on the page's latest revision id, so an approval change with + no accompanying page edit does not take effect until that page is edited or the cache entry expires + ([#1392](https://github.com/ProfessionalWiki/NeoWiki/issues/1392)). +- The RDF export stops answering for a page once approval is withdrawn, but what was published stays in the graph + stores until the page is deleted; a rebuild does not remove it + ([#1391](https://github.com/ProfessionalWiki/NeoWiki/issues/1391)). Revocation is not a retraction mechanism. + +Subject reads over REST, and the parse-time accessors, are not yet covered +([#1390](https://github.com/ProfessionalWiki/NeoWiki/issues/1390)). + ### Graph Database Backends NeoWiki currently supports Neo4j only, but the graph projection is an extension point: implement diff --git a/maintenance/DumpRdf.php b/maintenance/DumpRdf.php index 5505a0058..500561104 100644 --- a/maintenance/DumpRdf.php +++ b/maintenance/DumpRdf.php @@ -102,7 +102,7 @@ private function dumpPage( } if ( $page === null ) { - $this->error( "Skipped page $pageId: it no longer exists or its subject slot does not hold Subject data" ); + $this->error( "Skipped page $pageId: it no longer exists, its subject slot does not hold Subject data, or the registered revision policy publishes no revision of it" ); return false; } diff --git a/src/Application/FailureIsolatingRevisionPolicy.php b/src/Application/FailureIsolatingRevisionPolicy.php new file mode 100644 index 000000000..6e95406c3 --- /dev/null +++ b/src/Application/FailureIsolatingRevisionPolicy.php @@ -0,0 +1,104 @@ +isDeleted( RevisionRecord::DELETED_TEXT ) ) { + return false; + } + + try { + return $this->policy->publishesRevision( $revision ); + } catch ( Throwable $exception ) { + $this->logFailure( 'publishesRevision', $revision, $exception ); + return false; + } + } + + public function publishedRevision( RevisionRecord $revision ): ?RevisionRecord { + try { + $published = $this->policy->publishedRevision( $revision ); + } catch ( Throwable $exception ) { + $this->logFailure( 'publishedRevision', $revision, $exception ); + return null; + } + + if ( $published === null ) { + return null; + } + + if ( $published->getPageId() !== $revision->getPageId() ) { + $this->logger->error( + 'The revision policy {class} named revision {published} of page {publishedPage} for page {page}; ' + . 'a policy may only name a revision of the page it was asked about. Publishing nothing for it.', + [ + 'class' => $this->policy::class, + 'published' => $published->getId(), + 'publishedPage' => $published->getPageId(), + 'page' => $revision->getPageId(), + ] + ); + return null; + } + + if ( $published->isDeleted( RevisionRecord::DELETED_TEXT ) ) { + return null; + } + + return $published; + } + + public function revisionIsReadableBy( RevisionRecord $revision, Authority $viewer ): bool { + try { + return $this->policy->revisionIsReadableBy( $revision, $viewer ); + } catch ( Throwable $exception ) { + $this->logFailure( 'revisionIsReadableBy', $revision, $exception ); + return false; + } + } + + private function logFailure( string $method, RevisionRecord $revision, Throwable $exception ): void { + $this->logger->error( + 'The revision policy {class} threw from {method} for revision {revision} of page {page}; ' + . 'treating the page as publishing nothing. {message}', + [ + 'class' => $this->policy::class, + 'method' => $method, + 'revision' => $revision->getId(), + 'page' => $revision->getPageId(), + 'message' => $exception->getMessage(), + 'exception' => $exception, + ] + ); + } + +} diff --git a/src/Application/GraphRebuild/RebuildProgress.php b/src/Application/GraphRebuild/RebuildProgress.php index 691cf02ea..41abcfce9 100644 --- a/src/Application/GraphRebuild/RebuildProgress.php +++ b/src/Application/GraphRebuild/RebuildProgress.php @@ -34,8 +34,8 @@ public function pageProjected( int $pageId ): void { } /** - * The page held no Subject to project after all — it has no current revision, or its latest one - * dropped the subject slot. Nothing failed, and nothing was projected. + * The page was not projected, and nothing failed: it has no current revision, its subject slot + * does not hold Subject data, or the registered revision policy publishes no revision of it. */ public function pageSkipped( int $pageId ): void { $this->cursor = $pageId; diff --git a/src/Application/NullRevisionPolicy.php b/src/Application/NullRevisionPolicy.php new file mode 100644 index 000000000..bbcef8498 --- /dev/null +++ b/src/Application/NullRevisionPolicy.php @@ -0,0 +1,29 @@ +rebuildWithReadFlags( $title, IDBAccessObject::READ_NORMAL ); + return $this->rebuildWithReadFlags( $title, IDBAccessObject::READ_NORMAL, substitute: true ); } /** @@ -27,10 +36,10 @@ public function rebuild( Title $title ): PageRefreshOutcome { * replaced, which would project outdated content. */ public function rebuildFromPrimary( Title $title ): PageRefreshOutcome { - return $this->rebuildWithReadFlags( $title, IDBAccessObject::READ_LATEST ); + return $this->rebuildWithReadFlags( $title, IDBAccessObject::READ_LATEST, substitute: false ); } - private function rebuildWithReadFlags( Title $title, int $readFlags ): PageRefreshOutcome { + private function rebuildWithReadFlags( Title $title, int $readFlags, bool $substitute ): PageRefreshOutcome { $wikiPage = $this->wikiPageFactory->newFromTitle( $title ); $wikiPage->loadPageData( $readFlags ); @@ -40,6 +49,16 @@ private function rebuildWithReadFlags( Title $title, int $readFlags ): PageRefre return PageRefreshOutcome::SkippedMissingRevision; } + if ( $substitute ) { + $published = $this->revisionPolicy->publishedRevision( $revision ); + + if ( $published === null ) { + return PageRefreshOutcome::SkippedUnpublishableRevision; + } + + $revision = $published; + } + return $this->handler->onRevisionCreated( $revision ); } diff --git a/src/Application/PageRefreshOutcome.php b/src/Application/PageRefreshOutcome.php index 13b575164..2f4b10fa7 100644 --- a/src/Application/PageRefreshOutcome.php +++ b/src/Application/PageRefreshOutcome.php @@ -15,6 +15,7 @@ enum PageRefreshOutcome: string { case SkippedMissingRevision = 'skippedMissingRevision'; case SkippedUnreadableSubjects = 'skippedUnreadableSubjects'; case SkippedUnreadablePageProperties = 'skippedUnreadablePageProperties'; + case SkippedUnpublishableRevision = 'skippedUnpublishableRevision'; /** * Why the page was not written, phrased to complete "Skipped : ...". @@ -26,6 +27,7 @@ public function skipReason(): string { self::SkippedMissingRevision => 'no current revision', self::SkippedUnreadableSubjects => 'its subject slot does not hold Subject data', self::SkippedUnreadablePageProperties => 'its page properties could not be built', + self::SkippedUnpublishableRevision => 'the registered revision policy does not publish it', self::Refreshed => throw new LogicException( 'Refreshed is not a skip' ), }; } diff --git a/src/Application/Rdf/RdfPageLoader.php b/src/Application/Rdf/RdfPageLoader.php index ffb46666f..b5522e4b6 100644 --- a/src/Application/Rdf/RdfPageLoader.php +++ b/src/Application/Rdf/RdfPageLoader.php @@ -7,6 +7,7 @@ use MediaWiki\Page\WikiPageFactory; use MediaWiki\Revision\RevisionRecord; use MediaWiki\Title\Title; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; use ProfessionalWiki\NeoWiki\Domain\Page\Page; use ProfessionalWiki\NeoWiki\Domain\Page\PageId; use ProfessionalWiki\NeoWiki\Domain\Page\PageSubjects; @@ -15,20 +16,23 @@ use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\MediaWikiSubjectRepository; /** - * Loads a {@see Page} domain object from a page's current revision so it can be projected to RDF. + * Loads a {@see Page} domain object from the revision the registered revision policy publishes, so it + * can be projected to RDF. * The revision slot is the source of truth (as in OnRevisionCreatedHandler and the graph rebuild), * so the export reflects the stored data rather than the secondary graph projection. * * Every page is exported, with the Subjects it holds and with none when it holds none, which keeps the * RDF surfaces describing the same pages as the graph databases. Returns null when the page does not - * exist, and for the one page state the write path also refuses to touch: a subject slot holding content - * that is not Subject data, which the export must not describe as a page without Subjects. + * exist, when the policy publishes no revision of it, and for the one page state the write path also + * refuses to touch: a subject slot holding content that is not Subject data, which the export must not + * describe as a page without Subjects. */ class RdfPageLoader { public function __construct( private readonly WikiPageFactory $wikiPageFactory, private readonly PagePropertiesBuilder $pagePropertiesBuilder, + private readonly RevisionPolicy $revisionPolicy, ) { } @@ -49,7 +53,9 @@ public function loadByTitle( Title $title ): ?Page { return null; } - return $this->buildPage( $revision ); + $published = $this->revisionPolicy->publishedRevision( $revision ); + + return $published === null ? null : $this->buildPage( $published ); } private function buildPage( RevisionRecord $revision ): ?Page { diff --git a/src/Application/RevisionPolicy.php b/src/Application/RevisionPolicy.php new file mode 100644 index 000000000..b0f5401b2 --- /dev/null +++ b/src/Application/RevisionPolicy.php @@ -0,0 +1,51 @@ +policy !== null ) { + $this->logger->warning( + 'Ignoring the revision policy registered by {class}: a policy is already registered, ' + . 'and only one extension can decide which revision a page publishes.', + [ 'class' => $policy::class ] + ); + return; + } + + $this->policy = $policy; + } + + public function getPolicy(): RevisionPolicy { + return $this->policy ?? new NullRevisionPolicy(); + } + +} diff --git a/src/EntryPoints/NeoWikiRegistrar.php b/src/EntryPoints/NeoWikiRegistrar.php index bb5bcdc20..38f725a6d 100644 --- a/src/EntryPoints/NeoWikiRegistrar.php +++ b/src/EntryPoints/NeoWikiRegistrar.php @@ -4,6 +4,8 @@ namespace ProfessionalWiki\NeoWiki\EntryPoints; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicyRegistry; use ProfessionalWiki\NeoWiki\Domain\EditNotice\SubjectEditNoticeProvider; use ProfessionalWiki\NeoWiki\Domain\EditNotice\SubjectEditNoticeProviderRegistry; use ProfessionalWiki\NeoWiki\Domain\GraphDatabase\GraphDatabasePlugin; @@ -26,6 +28,7 @@ public function __construct( private GraphDatabasePluginRegistry $graphDatabasePluginRegistry, private RdfValueMapperRegistry $rdfValueMapperRegistry, private SubjectEditNoticeProviderRegistry $subjectEditNoticeProviderRegistry, + private RevisionPolicyRegistry $revisionPolicyRegistry, ) { } @@ -58,6 +61,18 @@ public function addSubjectEditNoticeProvider( SubjectEditNoticeProvider $provide $this->subjectEditNoticeProviderRegistry->addProvider( $provider ); } + /** + * Registers which revision of a page NeoWiki publishes: projects to the graph stores and exports + * as RDF. For approval extensions, which show readers an approved revision rather than the newest + * one. + * + * Only one extension can decide this, so unlike the other registrations this is a single slot: a + * second policy is refused with a warning and the first one keeps deciding. + */ + public function setRevisionPolicy( RevisionPolicy $policy ): void { + $this->revisionPolicyRegistry->setPolicy( $policy ); + } + public function addPagePropertyProvider( PagePropertyProvider $provider ): void { $this->pagePropertyProviderRegistry->addProvider( $provider ); } diff --git a/src/EntryPoints/OnRevisionCreatedHandler.php b/src/EntryPoints/OnRevisionCreatedHandler.php index 51f85d0fb..290c13c07 100644 --- a/src/EntryPoints/OnRevisionCreatedHandler.php +++ b/src/EntryPoints/OnRevisionCreatedHandler.php @@ -6,6 +6,7 @@ use MediaWiki\Revision\RevisionRecord; use ProfessionalWiki\NeoWiki\Application\PageRefreshOutcome; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; use ProfessionalWiki\NeoWiki\Domain\GraphDatabase\GraphDatabasePlugin; use ProfessionalWiki\NeoWiki\Domain\Page\Page; use ProfessionalWiki\NeoWiki\Domain\Page\PageId; @@ -23,6 +24,7 @@ public function __construct( private readonly GraphDatabasePlugin $graphDatabasePlugin, private readonly SubjectPageIndex $subjectPageIndex, private readonly PagePropertiesSource $pagePropertiesSource, + private readonly RevisionPolicy $revisionPolicy, private readonly LoggerInterface $logger, ) { } @@ -30,6 +32,11 @@ public function __construct( /** * Indexes which Subjects the page holds, and projects the page with them — and with none when it * holds none: every page gets a Page node, so its Page Properties are queryable. + * + * The index records where a Subject lives and is written for every revision, published or not: + * every id-keyed read and write addresses its page through it, so a Subject missing from it cannot + * be edited, moved or deleted, and its id reads as free. Publishing is decided separately, and only + * for the graph, which is the surface readers query. */ public function onRevisionCreated( RevisionRecord $revisionRecord ): PageRefreshOutcome { if ( $revisionRecord->getPageId() === 0 ) { @@ -67,6 +74,13 @@ private function refreshPage( RevisionRecord $revisionRecord, ?SubjectContent $c // it or not at all, and a Subject too broken to deserialize is still indexed. $this->subjectPageIndex->setSubjectsOfPage( $pageId, $content?->getSubjectIds() ?? [] ); + // Indexed either way, projected only when published: what the graph already holds is what the + // policy last published, and leaving it there is the point. Withdrawing it belongs to page + // deletion, not to somebody saving a draft on a page that has nothing published yet. + if ( !$this->revisionPolicy->publishesRevision( $revisionRecord ) ) { + return PageRefreshOutcome::SkippedUnpublishableRevision; + } + $subjects = $content?->getPageSubjects() ?? PageSubjects::newEmpty(); // Null only from the isolating source the hook path is given, which has already logged the diff --git a/src/EntryPoints/REST/GetSubjectApi.php b/src/EntryPoints/REST/GetSubjectApi.php index 393dbaf0b..dd42dff8b 100644 --- a/src/EntryPoints/REST/GetSubjectApi.php +++ b/src/EntryPoints/REST/GetSubjectApi.php @@ -49,10 +49,10 @@ private function newGetSubjectQuery( RestGetSubjectPresenter $presenter, ?int $r $revision = MediaWikiServices::getInstance()->getRevisionLookup()->getRevisionById( $revisionId ); - // A revision on an unreadable page answers exactly like a nonexistent revision: - // revision ids are sequential, so any distinguishable answer is a sweepable - // existence oracle over restricted pages (#1046). - if ( $revision === null || !$this->revisionPageIsReadable( $revision->getPageId() ) ) { + // A revision the viewer may not see answers exactly like a nonexistent one: revision ids are + // sequential, so any distinguishable answer is a sweepable existence oracle over restricted + // pages (#1046), and over the unapproved revisions an approval extension hides. + if ( $revision === null || !$this->revisionIsReadable( $revision ) ) { return $this->getResponseFactory()->createHttpError( 404, [ 'message' => 'Revision not found: ' . $revisionId, ] ); @@ -61,6 +61,17 @@ private function newGetSubjectQuery( RestGetSubjectPresenter $presenter, ?int $r return NeoWikiExtension::getInstance()->newGetSubjectQueryForRevision( $presenter, $revision, $this->getAuthority() ); } + /** + * A caller who names a revision is asking to see that one, so the registered revision policy is + * asked directly whether this viewer may. Without one registered every revision of a readable page + * is readable, as before. + */ + private function revisionIsReadable( RevisionRecord $revision ): bool { + return $this->revisionPageIsReadable( $revision->getPageId() ) + && NeoWikiExtension::getInstance()->getRevisionPolicy() + ->revisionIsReadableBy( $revision, $this->getAuthority() ); + } + private function revisionPageIsReadable( int $pageId ): bool { return NeoWikiExtension::getInstance() ->newPageReadAuthorizer( $this->getAuthority() ) diff --git a/src/NeoWikiExtension.php b/src/NeoWikiExtension.php index cd73ac28d..603b9e018 100644 --- a/src/NeoWikiExtension.php +++ b/src/NeoWikiExtension.php @@ -83,6 +83,9 @@ use ProfessionalWiki\NeoWiki\Application\SubjectWriteAuthorizer; use ProfessionalWiki\NeoWiki\Application\LastEditorPagesRebuilder; use ProfessionalWiki\NeoWiki\Application\PageRebuilder; +use ProfessionalWiki\NeoWiki\Application\FailureIsolatingRevisionPolicy; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicyRegistry; use ProfessionalWiki\NeoWiki\Application\SubjectIdMinter; use ProfessionalWiki\NeoWiki\Application\SubjectRepository; use ProfessionalWiki\NeoWiki\Application\SubjectResolver; @@ -225,6 +228,7 @@ class NeoWikiExtension { private CompositeGraphDatabasePlugin $graphDatabasePlugin; private CompositeGraphDatabasePlugin $isolatingGraphDatabasePlugin; private GraphDatabasePluginRegistry $graphDatabasePluginRegistry; + private RevisionPolicyRegistry $revisionPolicyRegistry; private ?Neo4jPlugin $neo4jPlugin = null; /** @var array|null Keys are store names */ private ?array $sparqlPlugins = null; @@ -305,6 +309,23 @@ public function getRdfValueMapperRegistry(): RdfValueMapperRegistry { return $this->rdfValueMapperRegistry; } + public function getRevisionPolicyRegistry(): RevisionPolicyRegistry { + if ( !isset( $this->revisionPolicyRegistry ) ) { + $this->revisionPolicyRegistry = new RevisionPolicyRegistry( LoggerFactory::getInstance( 'NeoWiki' ) ); + } + + $this->ensureExtensionsRegistered(); + + return $this->revisionPolicyRegistry; + } + + public function getRevisionPolicy(): RevisionPolicy { + return new FailureIsolatingRevisionPolicy( + $this->getRevisionPolicyRegistry()->getPolicy(), + LoggerFactory::getInstance( 'NeoWiki' ) + ); + } + private function ensureExtensionsRegistered(): void { if ( $this->extensionsRegistered ) { return; @@ -321,6 +342,7 @@ private function ensureExtensionsRegistered(): void { $this->getGraphDatabasePluginRegistry(), $this->getRdfValueMapperRegistry(), $this->getSubjectEditNoticeProviderRegistry(), + $this->getRevisionPolicyRegistry(), ) ] ); } @@ -390,6 +412,11 @@ public function getStoreContentUC(): OnRevisionCreatedHandler { * on. */ private function newRebuildStoreContentHandler(): OnRevisionCreatedHandler { + // The null index is load-bearing, not merely unneeded: PageRebuilder::rebuild() hands this handler + // the revision the policy publishes, and the handler indexes whatever it is given. A real index + // here would replace the latest revision's Subject set with the published one, making every + // Subject a draft added unaddressable — the exact failure keeping the index out of the policy + // exists to prevent. Only rebuildFromPrimary(), which does not substitute, may index. return $this->newStoreContentHandler( $this->getGraphDatabasePlugin(), new NullSubjectPageIndex(), @@ -406,6 +433,7 @@ private function newStoreContentHandler( $graphDatabasePlugin, $subjectPageIndex, $pagePropertiesSource, + $this->getRevisionPolicy(), LoggerFactory::getInstance( 'NeoWiki' ), ); } @@ -446,6 +474,7 @@ public function newRdfPageLoader(): RdfPageLoader { return new RdfPageLoader( MediaWikiServices::getInstance()->getWikiPageFactory(), $this->getPagePropertiesBuilder(), + $this->getRevisionPolicy(), ); } @@ -982,7 +1011,8 @@ public function isConfigPage( Title $title ): bool { public function getPageContentFetcher(): PageContentFetcher { return new PageContentFetcher( MediaWikiServices::getInstance()->getTitleParser(), - MediaWikiServices::getInstance()->getRevisionLookup() + MediaWikiServices::getInstance()->getRevisionLookup(), + $this->getRevisionPolicy() ); } @@ -1084,7 +1114,8 @@ public function newHookPageRebuilder(): PageRebuilder { private function newPageRebuilderWith( OnRevisionCreatedHandler $handler ): PageRebuilder { return new PageRebuilder( $handler, - MediaWikiServices::getInstance()->getWikiPageFactory() + MediaWikiServices::getInstance()->getWikiPageFactory(), + $this->getRevisionPolicy() ); } diff --git a/src/Persistence/MediaWiki/PageContentFetcher.php b/src/Persistence/MediaWiki/PageContentFetcher.php index a42274dc6..df7ff2468 100644 --- a/src/Persistence/MediaWiki/PageContentFetcher.php +++ b/src/Persistence/MediaWiki/PageContentFetcher.php @@ -14,12 +14,23 @@ use MediaWiki\Title\Title; use MediaWiki\Title\TitleParser; use MediaWiki\Title\TitleValue; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; +/** + * The content a page publishes, which is its latest revision unless a registered revision policy + * substitutes another. Everything read through here is configuration the whole wiki reads — Schemas, + * Layouts, Mappings and the on-wiki configuration page — so on a wiki running an approval extension + * an unapproved edit to one does not take effect until it is approved. + * + * Editing surfaces do not read through here: the Schema editor loads the page source from core's own + * REST endpoint, so it still shows and saves over the latest revision. + */ class PageContentFetcher { public function __construct( private readonly TitleParser $titleParser, - private readonly RevisionLookup $revisionLookup + private readonly RevisionLookup $revisionLookup, + private readonly RevisionPolicy $revisionPolicy ) { } @@ -36,6 +47,7 @@ public function getPageContent( } $revision = $this->revisionLookup->getRevisionByTitle( Title::newFromLinkTarget( $titleValue ) ); + $revision = $revision === null ? null : $this->revisionPolicy->publishedRevision( $revision ); try { return $revision?->getContent( $slotName, RevisionRecord::FOR_THIS_USER, $authority ); diff --git a/tests/phpunit/Application/FailureIsolatingRevisionPolicyTest.php b/tests/phpunit/Application/FailureIsolatingRevisionPolicyTest.php new file mode 100644 index 000000000..bc4fb09d8 --- /dev/null +++ b/tests/phpunit/Application/FailureIsolatingRevisionPolicyTest.php @@ -0,0 +1,135 @@ +newRevision( pageId: self::PAGE_ID, id: 10 ); + $published = $this->newRevision( pageId: self::PAGE_ID, id: 9 ); + $policy = $this->isolate( FixedRevisionPolicy::publishing( $published ) ); + + $this->assertTrue( $policy->publishesRevision( $revision ) ); + $this->assertSame( $published, $policy->publishedRevision( $revision ) ); + $this->assertTrue( $policy->revisionIsReadableBy( $revision, $this->createStub( Authority::class ) ) ); + } + + public function testAThrowingPolicyPublishesNothing(): void { + $policy = $this->isolate( $this->newThrowingPolicy() ); + $revision = $this->newRevision( pageId: self::PAGE_ID, id: 10 ); + + $this->assertFalse( $policy->publishesRevision( $revision ) ); + $this->assertNull( $policy->publishedRevision( $revision ) ); + } + + public function testAThrowingPolicyHidesEveryRevision(): void { + $policy = $this->isolate( $this->newThrowingPolicy() ); + + $this->assertFalse( $policy->revisionIsReadableBy( + $this->newRevision( pageId: self::PAGE_ID, id: 10 ), + $this->createStub( Authority::class ) + ) ); + } + + public function testAThrowingPolicyIsLogged(): void { + $logger = new TestLogger(); + + ( new FailureIsolatingRevisionPolicy( $this->newThrowingPolicy(), $logger ) ) + ->publishesRevision( $this->newRevision( pageId: self::PAGE_ID, id: 10 ) ); + + $this->assertTrue( $logger->hasErrorRecords() ); + } + + public function testARevisionOfAnotherPageIsRefused(): void { + $logger = new TestLogger(); + $policy = new FailureIsolatingRevisionPolicy( + $this->newPolicyNaming( $this->newRevision( pageId: self::OTHER_PAGE_ID, id: 9 ) ), + $logger + ); + + $published = $policy->publishedRevision( $this->newRevision( pageId: self::PAGE_ID, id: 10 ) ); + + $this->assertNull( $published ); + $this->assertTrue( $logger->hasErrorRecords(), 'a contract violation is logged' ); + } + + public function testASuppressedRevisionIsNeverPublished(): void { + $suppressed = $this->newRevision( pageId: self::PAGE_ID, id: 9, textSuppressed: true ); + $policy = $this->isolate( FixedRevisionPolicy::publishing( $suppressed ) ); + + $this->assertNull( $policy->publishedRevision( $this->newRevision( pageId: self::PAGE_ID, id: 10 ) ) ); + $this->assertFalse( $policy->publishesRevision( $suppressed ) ); + } + + public function testANullAnswerStaysNull(): void { + $policy = $this->isolate( FixedRevisionPolicy::publishingNothing() ); + + $this->assertNull( $policy->publishedRevision( $this->newRevision( pageId: self::PAGE_ID, id: 10 ) ) ); + } + + private function isolate( RevisionPolicy $policy ): FailureIsolatingRevisionPolicy { + return new FailureIsolatingRevisionPolicy( $policy, new NullLogger() ); + } + + /** + * A policy that breaks the contract by naming the same revision for every page it is asked about. + * FixedRevisionPolicy answers per page, as a real extension does, so it cannot stand in here. + */ + private function newPolicyNaming( RevisionRecord $published ): RevisionPolicy { + return new class( $published ) extends NullRevisionPolicy { + public function __construct( private readonly RevisionRecord $published ) { + } + + public function publishedRevision( RevisionRecord $revision ): ?RevisionRecord { + return $this->published; + } + }; + } + + private function newThrowingPolicy(): RevisionPolicy { + return new class extends NullRevisionPolicy { + public function publishesRevision( RevisionRecord $revision ): bool { + throw new RuntimeException( 'approval table missing' ); + } + + public function publishedRevision( RevisionRecord $revision ): ?RevisionRecord { + throw new RuntimeException( 'approval table missing' ); + } + + public function revisionIsReadableBy( RevisionRecord $revision, Authority $viewer ): bool { + throw new RuntimeException( 'approval table missing' ); + } + }; + } + + private function newRevision( int $pageId, int $id, bool $textSuppressed = false ): RevisionRecord { + $revision = $this->createStub( RevisionRecord::class ); + $revision->method( 'getPageId' )->willReturn( $pageId ); + $revision->method( 'getId' )->willReturn( $id ); + $revision->method( 'isDeleted' )->willReturnCallback( + static fn ( int $field ): bool => $textSuppressed && ( $field & RevisionRecord::DELETED_TEXT ) !== 0 + ); + + return $revision; + } + +} diff --git a/tests/phpunit/Application/PageRebuilderTest.php b/tests/phpunit/Application/PageRebuilderTest.php index 8c5fc6394..bf011bf86 100644 --- a/tests/phpunit/Application/PageRebuilderTest.php +++ b/tests/phpunit/Application/PageRebuilderTest.php @@ -10,7 +10,10 @@ use MediaWiki\Title\Title; use PHPUnit\Framework\TestCase; use ProfessionalWiki\NeoWiki\Application\PageRefreshOutcome; +use ProfessionalWiki\NeoWiki\Application\NullRevisionPolicy; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; use ProfessionalWiki\NeoWiki\Application\PageRebuilder; +use ProfessionalWiki\NeoWiki\Tests\TestDoubles\FixedRevisionPolicy; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\SpyOnRevisionCreatedHandler; use Wikimedia\Rdbms\IDBAccessObject; use WikiPage; @@ -111,7 +114,38 @@ public function testRebuildFromPrimaryReadsPageStateFromPrimary(): void { ); } - private function newRebuilder( ?RevisionRecord $revision ): PageRebuilder { + public function testRebuildProjectsTheRevisionThePolicyPublishes(): void { + $latest = $this->newRevision(); + $published = $this->newRevision(); + + $this->newRebuilder( $latest, FixedRevisionPolicy::publishing( $published ) ) + ->rebuild( Title::makeTitle( NS_MAIN, 'Reprojected page' ) ); + + $this->assertSame( [ $published ], $this->handler->calls ); + } + + public function testRebuildWritesNothingWhenThePolicyPublishesNoRevision(): void { + $outcome = $this->newRebuilder( $this->newRevision(), FixedRevisionPolicy::publishingNothing() ) + ->rebuild( Title::makeTitle( NS_MAIN, 'Page with nothing published' ) ); + + $this->assertSame( PageRefreshOutcome::SkippedUnpublishableRevision, $outcome ); + $this->assertSame( [], $this->handler->calls ); + } + + /** + * An import or undelete writes a revision rather than reprojecting a page, so it takes the same + * path a save does: the handler is handed what was written and decides whether to publish it. + */ + public function testRebuildFromPrimaryDoesNotSubstitute(): void { + $written = $this->newRevision(); + + $this->newRebuilder( $written, FixedRevisionPolicy::publishing( $this->newRevision() ) ) + ->rebuildFromPrimary( Title::makeTitle( NS_MAIN, 'Imported page' ) ); + + $this->assertSame( [ $written ], $this->handler->calls ); + } + + private function newRebuilder( ?RevisionRecord $revision, ?RevisionPolicy $policy = null ): PageRebuilder { $page = $this->createStub( WikiPage::class ); $page->method( 'getRevisionRecord' )->willReturn( $revision ); $page->method( 'loadPageData' )->willReturnCallback( @@ -123,7 +157,7 @@ function ( int $from ): void { $factory = $this->createStub( WikiPageFactory::class ); $factory->method( 'newFromTitle' )->willReturn( $page ); - return new PageRebuilder( $this->handler, $factory ); + return new PageRebuilder( $this->handler, $factory, $policy ?? new NullRevisionPolicy() ); } private function newRevision(): RevisionRecord { diff --git a/tests/phpunit/Application/PageRefreshWithoutEditTest.php b/tests/phpunit/Application/PageRefreshWithoutEditTest.php index 97100ce50..890ba5af1 100644 --- a/tests/phpunit/Application/PageRefreshWithoutEditTest.php +++ b/tests/phpunit/Application/PageRefreshWithoutEditTest.php @@ -9,6 +9,7 @@ use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; use ProfessionalWiki\NeoWiki\Tests\NeoWikiIntegrationTestCase; +use ProfessionalWiki\NeoWiki\Tests\TestDoubles\FixedRevisionPolicy; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\MutablePagePropertyProvider; /** @@ -53,6 +54,31 @@ public function testRefreshUpdatesThePagePropertiesOfAPageWithoutSubjects(): voi $this->assertSame( 'approved', $this->readApprovalState( $pageId ) ); } + /** + * Through NeoWikiExtension's own wiring, not a hand-built handler: the registered policy has to reach + * both PageRebuilder and the handler behind it, or a rebuild after approval would project the draft. + */ + public function testRefreshProjectsTheRevisionTheRegisteredPolicyPublishes(): void { + $approved = $this->createPageWithSubjects( self::PAGE_NAME, TestSubject::build() ); + $this->createPageWithSubjects( self::PAGE_NAME ); + $this->registerRevisionPolicy( FixedRevisionPolicy::publishing( $approved ) ); + + $outcome = $this->refreshPage(); + + $this->assertSame( PageRefreshOutcome::Refreshed, $outcome ); + $this->assertTrue( + $this->pageHoldsSubjectInGraph( $approved->getPageId() ), + 'the approved revision holds a Subject the draft removed' + ); + } + + public function testRefreshWritesNothingWhenTheRegisteredPolicyPublishesNoRevision(): void { + $this->createPageWithSubjects( self::PAGE_NAME, TestSubject::build() ); + $this->registerRevisionPolicy( FixedRevisionPolicy::publishingNothing() ); + + $this->assertSame( PageRefreshOutcome::SkippedUnpublishableRevision, $this->refreshPage() ); + } + public function testRefreshOfAMissingPageWritesNothing(): void { $outcome = $this->refreshPage(); @@ -65,6 +91,15 @@ private function refreshPage(): PageRefreshOutcome { ->rebuild( Title::newFromText( self::PAGE_NAME ) ); } + private function pageHoldsSubjectInGraph( int $pageId ): bool { + $result = $this->readGraph( + 'MATCH (page:Page {id: $pageId})-[:HasSubject]->(subject:Subject) RETURN count(subject) AS subjects', + [ 'pageId' => $pageId ] + ); + + return ( $result->first()->toRecursiveArray()['subjects'] ?? 0 ) > 0; + } + private function readApprovalState( int $pageId ): ?string { $result = $this->readGraph( 'MATCH (page:Page {id: $pageId}) RETURN page.approvalState AS approvalState', diff --git a/tests/phpunit/Application/Rdf/RdfPageLoaderTest.php b/tests/phpunit/Application/Rdf/RdfPageLoaderTest.php index 0102dff6a..0844c6d47 100644 --- a/tests/phpunit/Application/Rdf/RdfPageLoaderTest.php +++ b/tests/phpunit/Application/Rdf/RdfPageLoaderTest.php @@ -9,10 +9,14 @@ use MediaWiki\Revision\RevisionRecord; use MediaWiki\Revision\RevisionSlots; use MediaWiki\Title\Title; +use ProfessionalWiki\NeoWiki\Application\NullRevisionPolicy; use ProfessionalWiki\NeoWiki\Application\Rdf\RdfPageLoader; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\MediaWikiSubjectRepository; +use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; use ProfessionalWiki\NeoWiki\Tests\NeoWikiIntegrationTestCase; +use ProfessionalWiki\NeoWiki\Tests\TestDoubles\FixedRevisionPolicy; use WikiPage; /** @@ -37,6 +41,36 @@ public function testMissingPageIsNotLoaded(): void { $this->assertNull( $loader->loadByTitle( Title::makeTitle( NS_MAIN, 'Page that does not exist' ) ) ); } + public function testExportsTheRevisionTheRevisionPolicyPublishes(): void { + $this->createSchema( TestSubject::DEFAULT_SCHEMA_ID ); + $approved = $this->createPageWithSubjects( 'Exported from its approved revision', TestSubject::build() ); + $draft = $this->createPageWithSubjects( 'Exported from its approved revision' ); + + $page = $this->newLoaderFor( $draft, FixedRevisionPolicy::publishing( $approved ) ) + ->loadByTitle( Title::makeTitle( NS_MAIN, 'Exported from its approved revision' ) ); + + $this->assertNotNull( $page ); + $this->assertTrue( + $page->getSubjects()->hasSubjects(), + 'the approved revision holds a Subject the draft removed' + ); + } + + public function testPageWithNoPublishableRevisionIsNotLoaded(): void { + // The revision must be one that would otherwise load, or the null proves nothing: a slot that + // does not hold Subject data already yields null through the pass-through path. + $this->createSchema( TestSubject::DEFAULT_SCHEMA_ID ); + $revision = $this->createPageWithSubjects( 'Page with nothing published', TestSubject::build() ); + + $loader = $this->newLoaderFor( $revision, FixedRevisionPolicy::publishingNothing() ); + + $this->assertNull( $loader->loadByTitle( Title::makeTitle( NS_MAIN, 'Page with nothing published' ) ) ); + $this->assertNotNull( + $this->newLoaderFor( $revision )->loadByTitle( Title::makeTitle( NS_MAIN, 'Page with nothing published' ) ), + 'the same revision loads when the policy publishes it' + ); + } + private function newRevisionWithSubjectSlotContent(): RevisionRecord { $slots = $this->createStub( RevisionSlots::class ); $slots->method( 'getContent' )->willReturn( new WikitextContent( 'Not Subject data.' ) ); @@ -50,14 +84,18 @@ private function newRevisionWithSubjectSlotContent(): RevisionRecord { return $revision; } - private function newLoaderFor( ?RevisionRecord $revision ): RdfPageLoader { + private function newLoaderFor( ?RevisionRecord $revision, ?RevisionPolicy $policy = null ): RdfPageLoader { $page = $this->createStub( WikiPage::class ); $page->method( 'getRevisionRecord' )->willReturn( $revision ); $factory = $this->createStub( WikiPageFactory::class ); $factory->method( 'newFromTitle' )->willReturn( $page ); - return new RdfPageLoader( $factory, NeoWikiExtension::getInstance()->getPagePropertiesBuilder() ); + return new RdfPageLoader( + $factory, + NeoWikiExtension::getInstance()->getPagePropertiesBuilder(), + $policy ?? new NullRevisionPolicy() + ); } } diff --git a/tests/phpunit/Application/RevisionPolicyRegistryTest.php b/tests/phpunit/Application/RevisionPolicyRegistryTest.php new file mode 100644 index 000000000..88e69409b --- /dev/null +++ b/tests/phpunit/Application/RevisionPolicyRegistryTest.php @@ -0,0 +1,53 @@ +assertInstanceOf( NullRevisionPolicy::class, ( new RevisionPolicyRegistry() )->getPolicy() ); + } + + public function testUsesTheRegisteredPolicy(): void { + $policy = FixedRevisionPolicy::publishingNothing(); + $registry = new RevisionPolicyRegistry(); + + $registry->setPolicy( $policy ); + + $this->assertSame( $policy, $registry->getPolicy() ); + } + + public function testKeepsTheFirstPolicyWhenASecondIsRegistered(): void { + $first = FixedRevisionPolicy::publishingNothing(); + $registry = new RevisionPolicyRegistry(); + + $registry->setPolicy( $first ); + $registry->setPolicy( FixedRevisionPolicy::publishingNothing() ); + + $this->assertSame( $first, $registry->getPolicy() ); + } + + public function testWarnsWhenASecondPolicyIsRegistered(): void { + $logger = new TestLogger(); + $registry = new RevisionPolicyRegistry( $logger ); + + $registry->setPolicy( FixedRevisionPolicy::publishingNothing() ); + $this->assertFalse( $logger->hasWarningRecords(), 'the first registration is not a warning' ); + + $registry->setPolicy( FixedRevisionPolicy::publishingNothing() ); + + $this->assertCount( 1, $logger->records ); + } + +} diff --git a/tests/phpunit/EntryPoints/NeoWikiRegistrarTest.php b/tests/phpunit/EntryPoints/NeoWikiRegistrarTest.php index 665bcdc9f..0b46ea832 100644 --- a/tests/phpunit/EntryPoints/NeoWikiRegistrarTest.php +++ b/tests/phpunit/EntryPoints/NeoWikiRegistrarTest.php @@ -11,6 +11,8 @@ use ProfessionalWiki\NeoWiki\Domain\PropertyType\PropertyTypeRegistry; use ProfessionalWiki\NeoWiki\Domain\PropertyType\Types\TextType; use ProfessionalWiki\NeoWiki\Domain\Rdf\RdfValueMapperRegistry; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicyRegistry; +use ProfessionalWiki\NeoWiki\Tests\TestDoubles\FixedRevisionPolicy; use ProfessionalWiki\NeoWiki\EntryPoints\NeoWikiRegistrar; use ProfessionalWiki\NeoWiki\GraphDatabasePlugins\Neo4j\Persistence\Neo4jValueBuilderRegistry; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\SpyGraphDatabasePlugin; @@ -79,6 +81,16 @@ public function testAddSubjectEditNoticeProviderRegistersInRegistry(): void { $this->assertSame( [ $provider ], $noticeRegistry->getProviders() ); } + public function testRegistersTheRevisionPolicy(): void { + $policyRegistry = new RevisionPolicyRegistry(); + $registrar = $this->newRegistrar( revisionPolicyRegistry: $policyRegistry ); + $policy = FixedRevisionPolicy::publishingNothing(); + + $registrar->setRevisionPolicy( $policy ); + + $this->assertSame( $policy, $policyRegistry->getPolicy() ); + } + private function newRegistrar( ?PropertyTypeRegistry $propertyTypeRegistry = null, ?Neo4jValueBuilderRegistry $valueBuilderRegistry = null, @@ -86,6 +98,7 @@ private function newRegistrar( ?GraphDatabasePluginRegistry $graphDatabasePluginRegistry = null, ?RdfValueMapperRegistry $rdfValueMapperRegistry = null, ?SubjectEditNoticeProviderRegistry $subjectEditNoticeProviderRegistry = null, + ?RevisionPolicyRegistry $revisionPolicyRegistry = null, ): NeoWikiRegistrar { return new NeoWikiRegistrar( propertyTypeRegistry: $propertyTypeRegistry ?? new PropertyTypeRegistry(), @@ -94,6 +107,7 @@ private function newRegistrar( graphDatabasePluginRegistry: $graphDatabasePluginRegistry ?? new GraphDatabasePluginRegistry(), rdfValueMapperRegistry: $rdfValueMapperRegistry ?? new RdfValueMapperRegistry(), subjectEditNoticeProviderRegistry: $subjectEditNoticeProviderRegistry ?? new SubjectEditNoticeProviderRegistry(), + revisionPolicyRegistry: $revisionPolicyRegistry ?? new RevisionPolicyRegistry(), ); } diff --git a/tests/phpunit/EntryPoints/OnRevisionCreatedHandlerTest.php b/tests/phpunit/EntryPoints/OnRevisionCreatedHandlerTest.php index 26092f675..7a138dba8 100644 --- a/tests/phpunit/EntryPoints/OnRevisionCreatedHandlerTest.php +++ b/tests/phpunit/EntryPoints/OnRevisionCreatedHandlerTest.php @@ -9,7 +9,9 @@ use MediaWiki\Revision\RevisionAccessException; use MediaWiki\Revision\RevisionRecord; use MediaWiki\Revision\RevisionSlots; +use ProfessionalWiki\NeoWiki\Application\NullRevisionPolicy; use ProfessionalWiki\NeoWiki\Application\PageRefreshOutcome; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; use ProfessionalWiki\NeoWiki\Domain\GraphDatabase\FailureIsolatingGraphDatabasePlugin; use ProfessionalWiki\NeoWiki\Domain\GraphDatabase\GraphDatabasePlugin; use ProfessionalWiki\NeoWiki\Domain\Page\PageId; @@ -21,6 +23,7 @@ use ProfessionalWiki\NeoWiki\PagePropertiesBuilder; use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; use ProfessionalWiki\NeoWiki\Tests\NeoWikiIntegrationTestCase; +use ProfessionalWiki\NeoWiki\Tests\TestDoubles\FixedRevisionPolicy; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\SpyGraphDatabasePlugin; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\SpySubjectPageIndex; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\ThrowingGraphDatabasePlugin; @@ -189,6 +192,34 @@ public function testGraphDeletionHappensEvenWhenTheIndexRemovalFails(): void { $this->assertEquals( [ new PageId( self::DELETED_PAGE_ID ) ], $this->graphStore->deletedPageIds ); } + public function testDoesNotProjectARevisionThePolicyDoesNotPublish(): void { + $revision = $this->createPageWithSubjects( 'Page with an unpublished revision', TestSubject::build() ); + + $outcome = $this->newHandlerWithPolicy( FixedRevisionPolicy::publishingNothing() ) + ->onRevisionCreated( $revision ); + + $this->assertSame( PageRefreshOutcome::SkippedUnpublishableRevision, $outcome ); + $this->assertSame( [], $this->graphStore->savedPages ); + $this->assertSame( [], $this->graphStore->deletedPageIds, 'what was published stays published' ); + } + + /** + * The index says where a Subject lives, not whether it is published. Every id-keyed read and write + * addresses its page through it, so a Subject left out of it cannot be edited, moved or deleted, + * and its id reads as free. + */ + public function testIndexesASubjectEvenWhenItsRevisionIsNotPublished(): void { + $revision = $this->createPageWithSubjects( 'Page whose draft adds a subject', TestSubject::build() ); + + $this->newHandlerWithPolicy( FixedRevisionPolicy::publishingNothing() ) + ->onRevisionCreated( $revision ); + + $this->assertSame( + [ $revision->getPageId() => [ TestSubject::ZERO_GUID ] ], + $this->subjectPageIndex->indexedSubjectsByPageId + ); + } + private function newFailingProviderRegistry(): PagePropertyProviderRegistry { $registry = new PagePropertyProviderRegistry(); $registry->addProvider( new class implements PagePropertyProvider { @@ -224,10 +255,15 @@ private function newHandler(): OnRevisionCreatedHandler { return $this->newHandlerWith( $this->graphStore ); } + private function newHandlerWithPolicy( RevisionPolicy $policy ): OnRevisionCreatedHandler { + return $this->newHandlerWith( $this->graphStore, revisionPolicy: $policy ); + } + private function newHandlerWith( GraphDatabasePlugin $graphStore, ?PagePropertyProviderRegistry $providerRegistry = null, - bool $isolatePageProperties = false + bool $isolatePageProperties = false, + ?RevisionPolicy $revisionPolicy = null ): OnRevisionCreatedHandler { $pageProperties = $this->newPagePropertiesBuilder( $providerRegistry ?? new PagePropertyProviderRegistry() ); @@ -237,6 +273,7 @@ private function newHandlerWith( $isolatePageProperties ? new FailureIsolatingPagePropertiesSource( $pageProperties, $this->logger ) : $pageProperties, + $revisionPolicy ?? new NullRevisionPolicy(), $this->logger ); } @@ -246,6 +283,7 @@ private function newHandlerWithFailingIndex(): OnRevisionCreatedHandler { $this->graphStore, new ThrowingSubjectPageIndex(), $this->newPagePropertiesBuilder( new PagePropertyProviderRegistry() ), + new NullRevisionPolicy(), $this->logger ); } diff --git a/tests/phpunit/EntryPoints/REST/GetSubjectApiTest.php b/tests/phpunit/EntryPoints/REST/GetSubjectApiTest.php index cc486b512..55b358197 100644 --- a/tests/phpunit/EntryPoints/REST/GetSubjectApiTest.php +++ b/tests/phpunit/EntryPoints/REST/GetSubjectApiTest.php @@ -6,17 +6,20 @@ use MediaWiki\Page\PageIdentity; use MediaWiki\Rest\RequestData; +use MediaWiki\Rest\Response; use MediaWiki\Tests\Rest\Handler\HandlerTestTrait; use ProfessionalWiki\NeoWiki\Domain\Schema\SchemaName; use ProfessionalWiki\NeoWiki\Domain\Subject\StatementList; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectLabel; use ProfessionalWiki\NeoWiki\Domain\Subject\SubjectMap; use ProfessionalWiki\NeoWiki\EntryPoints\REST\GetSubjectApi; +use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Tests\Data\TestRelation; use ProfessionalWiki\NeoWiki\Tests\Data\TestStatement; use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; use ProfessionalWiki\NeoWiki\Tests\NeoWikiIntegrationTestCase; use ProfessionalWiki\NeoWiki\Tests\NeoWikiMockAuthorityTrait; +use ProfessionalWiki\NeoWiki\Tests\TestDoubles\FixedRevisionPolicy; /** * @covers \ProfessionalWiki\NeoWiki\EntryPoints\REST\GetSubjectApi @@ -205,6 +208,58 @@ public function testReturns404ForNonExistentRevision(): void { $this->assertSame( 404, $response->getStatusCode() ); } + /** + * A revision the registered policy hides answers exactly like one that does not exist, so the + * sequential revision ids cannot be swept to find out which drafts a page has. + */ + public function testRevisionHiddenByTheRevisionPolicyIsIndistinguishableFromAnAbsentRevision(): void { + $revisionId = $this->createPageWithSubjects( + 'GetSubjectApiTest_HiddenRevision', + mainSubject: TestSubject::build( + id: 'sTestGSA1111251', + schemaName: new SchemaName( 'GetSubjectApiTestSchema' ) + ) + )->getId(); + + [ $hiddenResponse, $absentResponse ] = $this->runWithRevisionPolicyHidingEveryRevision( + fn (): array => [ + $this->getSubjectAtRevision( 'sTestGSA1111251', (string)$revisionId ), + $this->getSubjectAtRevision( 'sTestGSA1111251', '999999999' ), + ] + ); + + $this->assertSame( 404, $hiddenResponse->getStatusCode() ); + $this->assertSame( $absentResponse->getStatusCode(), $hiddenResponse->getStatusCode() ); + + $hidden = json_decode( $hiddenResponse->getBody()->getContents(), true ); + $absent = json_decode( $absentResponse->getBody()->getContents(), true ); + + // Only the revision id embedded in the message may differ between "hidden" and "nonexistent". + $absent['message'] = str_replace( '999999999', (string)$revisionId, $absent['message'] ); + $this->assertSame( $absent, $hidden ); + } + + private function getSubjectAtRevision( string $subjectId, string $revisionId ): Response { + return $this->executeHandler( + new GetSubjectApi(), + new RequestData( [ + 'method' => 'GET', + 'pathParams' => [ 'subjectId' => $subjectId ], + 'queryParams' => [ 'revisionId' => $revisionId ], + ] ) + ); + } + + private function runWithRevisionPolicyHidingEveryRevision( callable $fn ): mixed { + $this->registerRevisionPolicy( FixedRevisionPolicy::hidingEveryRevision() ); + + try { + return $fn(); + } finally { + NeoWikiExtension::resetInstance(); + } + } + public function testFullExpansion(): void { $firstPageId = $this->createPageWithSubjects( 'GetSubjectApiTest0000', diff --git a/tests/phpunit/NeoWikiIntegrationTestCase.php b/tests/phpunit/NeoWikiIntegrationTestCase.php index ee9b72b46..ff28585b5 100644 --- a/tests/phpunit/NeoWikiIntegrationTestCase.php +++ b/tests/phpunit/NeoWikiIntegrationTestCase.php @@ -28,6 +28,7 @@ use ProfessionalWiki\NeoWiki\EntryPoints\Content\SchemaContent; use ProfessionalWiki\NeoWiki\EntryPoints\Content\SubjectContent; use ProfessionalWiki\NeoWiki\EntryPoints\NeoWikiRegistrar; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; use ProfessionalWiki\NeoWiki\NeoWikiExtension; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\Subject\MediaWikiSubjectRepository; use ProfessionalWiki\NeoWiki\Tests\Data\TestSchema; @@ -50,6 +51,7 @@ class NeoWikiIntegrationTestCase extends MediaWikiIntegrationTestCase { /** @var PagePropertyProvider[] */ private array $registeredPagePropertyProviders = []; + private ?RevisionPolicy $registeredRevisionPolicy = null; /** * The singleton pins a SchemaLookup whose cache is keyed by page and revision id, and those ids @@ -372,6 +374,16 @@ protected function registerPagePropertyProviders( PagePropertyProvider ...$provi $this->registerWithNeoWiki(); } + /** + * Registers the revision policy through the NeoWikiRegistration hook and rebuilds the singleton, so + * the real wiring hands it to every path that publishes. + */ + protected function registerRevisionPolicy( RevisionPolicy $policy ): void { + $this->registeredRevisionPolicy = $policy; + + $this->registerWithNeoWiki(); + } + /** * Everything registered so far goes through one hook handler, replacing the previous one. * setTemporaryHook clears the hook before adding its handler, so registering plugins and providers @@ -381,10 +393,11 @@ protected function registerPagePropertyProviders( PagePropertyProvider ...$provi private function registerWithNeoWiki(): void { $plugins = $this->registeredGraphDatabasePlugins; $providers = $this->registeredPagePropertyProviders; + $policy = $this->registeredRevisionPolicy; $this->setTemporaryHook( 'NeoWikiRegistration', - static function ( NeoWikiRegistrar $registrar ) use ( $plugins, $providers ): void { + static function ( NeoWikiRegistrar $registrar ) use ( $plugins, $providers, $policy ): void { foreach ( $plugins as $name => $plugin ) { $registrar->addGraphDatabasePlugin( $name, $plugin ); } @@ -392,6 +405,10 @@ static function ( NeoWikiRegistrar $registrar ) use ( $plugins, $providers ): vo foreach ( $providers as $provider ) { $registrar->addPagePropertyProvider( $provider ); } + + if ( $policy !== null ) { + $registrar->setRevisionPolicy( $policy ); + } } ); diff --git a/tests/phpunit/Persistence/MediaWiki/PageContentFetcherTest.php b/tests/phpunit/Persistence/MediaWiki/PageContentFetcherTest.php index b15a47847..fee6588af 100644 --- a/tests/phpunit/Persistence/MediaWiki/PageContentFetcherTest.php +++ b/tests/phpunit/Persistence/MediaWiki/PageContentFetcherTest.php @@ -12,7 +12,10 @@ use MediaWiki\Title\Title; use MediaWiki\Title\TitleParser; use PHPUnit\Framework\TestCase; +use ProfessionalWiki\NeoWiki\Application\NullRevisionPolicy; +use ProfessionalWiki\NeoWiki\Application\RevisionPolicy; use ProfessionalWiki\NeoWiki\Persistence\MediaWiki\PageContentFetcher; +use ProfessionalWiki\NeoWiki\Tests\TestDoubles\FixedRevisionPolicy; /** * @covers \ProfessionalWiki\NeoWiki\Persistence\MediaWiki\PageContentFetcher @@ -33,7 +36,37 @@ public function setUp(): void { $this->revisionRecord = $this->createMock( RevisionRecord::class ); $this->content = $this->createMock( Content::class ); - $this->pageContentFetcher = new PageContentFetcher( $this->titleParser, $this->revisionLookup ); + $this->pageContentFetcher = $this->newFetcherWith( new NullRevisionPolicy() ); + } + + public function testReadsTheRevisionTheRevisionPolicyPublishes(): void { + $approved = $this->createMock( RevisionRecord::class ); + $approvedContent = $this->createMock( Content::class ); + $approved->method( 'getContent' )->willReturn( $approvedContent ); + + $this->titleParser->method( 'parseTitle' )->willReturn( Title::newFromText( 'test title' )->getTitleValue() ); + $this->revisionLookup->method( 'getRevisionByTitle' )->willReturn( $this->revisionRecord ); + $this->revisionRecord->method( 'getContent' )->willReturn( $this->content ); + + $content = $this->newFetcherWith( FixedRevisionPolicy::publishing( $approved ) ) + ->getPageContent( 'test title', $this->authority ); + + $this->assertSame( $approvedContent, $content ); + } + + public function testReadsNothingWhenThePolicyPublishesNoRevisionOfThePage(): void { + $this->titleParser->method( 'parseTitle' )->willReturn( Title::newFromText( 'test title' )->getTitleValue() ); + $this->revisionLookup->method( 'getRevisionByTitle' )->willReturn( $this->revisionRecord ); + $this->revisionRecord->method( 'getContent' )->willReturn( $this->content ); + + $content = $this->newFetcherWith( FixedRevisionPolicy::publishingNothing() ) + ->getPageContent( 'test title', $this->authority ); + + $this->assertNull( $content ); + } + + private function newFetcherWith( RevisionPolicy $policy ): PageContentFetcher { + return new PageContentFetcher( $this->titleParser, $this->revisionLookup, $policy ); } public function testGetPageContentWithGivenAuthority(): void { diff --git a/tests/phpunit/TestDoubles/FixedRevisionPolicy.php b/tests/phpunit/TestDoubles/FixedRevisionPolicy.php new file mode 100644 index 000000000..017202b17 --- /dev/null +++ b/tests/phpunit/TestDoubles/FixedRevisionPolicy.php @@ -0,0 +1,57 @@ +publishes; + } + + /** + * Answers per page, as an approval extension does: the fixed revision stands in only for revisions of + * its own page, and any other page — a Schema page the projection resolves, say — passes through. + */ + public function publishedRevision( RevisionRecord $revision ): ?RevisionRecord { + if ( !$this->substitutes ) { + return $revision; + } + + if ( $this->published === null || $this->published->getPageId() === $revision->getPageId() ) { + return $this->published; + } + + return $revision; + } + + public function revisionIsReadableBy( RevisionRecord $revision, Authority $viewer ): bool { + return $this->readable; + } + +}