diff --git a/extension.json b/extension.json index cbc95fdd..7b5a9897 100644 --- a/extension.json +++ b/extension.json @@ -43,6 +43,7 @@ "RevisionFromEditComplete": "ProfessionalWiki\\NeoWiki\\EntryPoints\\NeoWikiHooks::onRevisionFromEditComplete", "PageDeleteComplete": "ProfessionalWiki\\NeoWiki\\EntryPoints\\NeoWikiHooks::onPageDeleteComplete", "RevisionUndeleted": "ProfessionalWiki\\NeoWiki\\EntryPoints\\NeoWikiHooks::onRevisionUndeleted", + "AfterImportPage": "ProfessionalWiki\\NeoWiki\\EntryPoints\\NeoWikiHooks::onAfterImportPage", "CodeEditorGetPageLanguage": "ProfessionalWiki\\NeoWiki\\EntryPoints\\NeoWikiHooks::onCodeEditorGetPageLanguage", diff --git a/src/Application/SubjectPageRebuilder.php b/src/Application/SubjectPageRebuilder.php index 724fa80f..f3506e01 100644 --- a/src/Application/SubjectPageRebuilder.php +++ b/src/Application/SubjectPageRebuilder.php @@ -7,6 +7,7 @@ use MediaWiki\Page\WikiPageFactory; use MediaWiki\Title\Title; use ProfessionalWiki\NeoWiki\EntryPoints\OnRevisionCreatedHandler; +use Wikimedia\Rdbms\IDBAccessObject; class SubjectPageRebuilder { @@ -17,7 +18,23 @@ public function __construct( } public function rebuild( Title $title ): PageRefreshOutcome { - $revision = $this->wikiPageFactory->newFromTitle( $title )->getRevisionRecord(); + return $this->rebuildWithReadFlags( $title, IDBAccessObject::READ_NORMAL ); + } + + /** + * Rebuilds from the primary database. Needed when rebuilding right after a write, such as on the + * import path: a replica can still be missing the page, or still carry the revision the import + * replaced, which would project outdated content. + */ + public function rebuildFromPrimary( Title $title ): PageRefreshOutcome { + return $this->rebuildWithReadFlags( $title, IDBAccessObject::READ_LATEST ); + } + + private function rebuildWithReadFlags( Title $title, int $readFlags ): PageRefreshOutcome { + $wikiPage = $this->wikiPageFactory->newFromTitle( $title ); + $wikiPage->loadPageData( $readFlags ); + + $revision = $wikiPage->getRevisionRecord(); if ( $revision === null ) { return PageRefreshOutcome::SkippedMissingRevision; diff --git a/src/EntryPoints/NeoWikiHooks.php b/src/EntryPoints/NeoWikiHooks.php index 8ee10f85..9784b818 100644 --- a/src/EntryPoints/NeoWikiHooks.php +++ b/src/EntryPoints/NeoWikiHooks.php @@ -13,6 +13,7 @@ use MediaWiki\Permissions\Authority; use MediaWiki\Revision\RevisionRecord; use MediaWiki\Revision\SlotRoleRegistry; +use MediaWiki\Title\ForeignTitle; use MediaWiki\Title\Title; use MediaWiki\User\UserIdentity; use ProfessionalWiki\NeoWiki\Domain\Page\PageId; @@ -184,6 +185,28 @@ public static function onRevisionFromEditComplete( $wikiPage->doPurge(); // clear cache } + /** + * Projects imported pages, which RevisionFromEditComplete does not cover, as imported revisions do + * not go through the edit path. WikiImporter fires this hook for every import path, once per page and + * only once all of the page's revisions are in, so one handler replaces what would otherwise be a + * special case per import path. Special:Import and the import API additionally project through + * RevisionFromEditComplete, because ImportReporter creates a null revision on top of the import; that + * reprojects the same content, making it redundant rather than harmful. + * + * @see AfterImportPageHook + * + * @param array $pageInfo + */ + public static function onAfterImportPage( + Title $title, + ForeignTitle $foreignTitle, + int $revCount, + int $sRevCount, + array $pageInfo + ): void { + NeoWikiExtension::getInstance()->newImportSubjectPageRebuilder()->rebuildFromPrimary( $title ); + } + public static function onCodeEditorGetPageLanguage( Title $title, ?string &$lang, ?string $model, ?string $format ): void { if ( in_array( $model, [ SubjectContent::CONTENT_MODEL_ID, SchemaContent::CONTENT_MODEL_ID, LayoutContent::CONTENT_MODEL_ID, MappingContent::CONTENT_MODEL_ID ] ) ) { $lang = 'json'; diff --git a/src/NeoWikiExtension.php b/src/NeoWikiExtension.php index cc9c3b11..ba8ec6b6 100644 --- a/src/NeoWikiExtension.php +++ b/src/NeoWikiExtension.php @@ -632,8 +632,20 @@ public function newSubjectContentRepository(): SubjectContentRepository { } public function newSubjectPageRebuilder(): SubjectPageRebuilder { + return $this->newSubjectPageRebuilderWith( $this->newRebuildStoreContentHandler() ); + } + + /** + * Import path: projects the current revision of a page like the rebuild path, but with the hook + * path's failure isolation, since a projection failure must not abort the user's import. + */ + public function newImportSubjectPageRebuilder(): SubjectPageRebuilder { + return $this->newSubjectPageRebuilderWith( $this->getStoreContentUC() ); + } + + private function newSubjectPageRebuilderWith( OnRevisionCreatedHandler $handler ): SubjectPageRebuilder { return new SubjectPageRebuilder( - $this->newRebuildStoreContentHandler(), + $handler, MediaWikiServices::getInstance()->getWikiPageFactory() ); } diff --git a/tests/phpunit/Application/SubjectPageRebuilderTest.php b/tests/phpunit/Application/SubjectPageRebuilderTest.php index 86a7a81f..c2efb47c 100644 --- a/tests/phpunit/Application/SubjectPageRebuilderTest.php +++ b/tests/phpunit/Application/SubjectPageRebuilderTest.php @@ -13,6 +13,7 @@ use ProfessionalWiki\NeoWiki\Application\PageRefreshOutcome; use ProfessionalWiki\NeoWiki\Application\SubjectPageRebuilder; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\SpyOnRevisionCreatedHandler; +use Wikimedia\Rdbms\IDBAccessObject; use WikiPage; /** @@ -22,8 +23,14 @@ class SubjectPageRebuilderTest extends TestCase { private SpyOnRevisionCreatedHandler $handler; + /** + * @var int[] + */ + private array $pageDataReads; + protected function setUp(): void { $this->handler = new SpyOnRevisionCreatedHandler(); + $this->pageDataReads = []; } public function testReturnsRefreshedWhenHandlerWritesPage(): void { @@ -68,9 +75,32 @@ public function testPassesRevisionAuthorToHandler(): void { $this->assertSame( $author, $this->handler->calls[0]['user'] ); } + public function testRebuildReadsPageStateFromReplica(): void { + $this->newRebuilder( $this->newRevisionByUser( new UserIdentityValue( 42, 'RevisionAuthor' ) ) ) + ->rebuild( Title::makeTitle( NS_MAIN, 'AnyPage' ) ); + + $this->assertSame( [ IDBAccessObject::READ_NORMAL ], $this->pageDataReads ); + } + + public function testRebuildFromPrimaryReadsPageStateFromPrimary(): void { + $this->newRebuilder( $this->newRevisionByUser( new UserIdentityValue( 42, 'RevisionAuthor' ) ) ) + ->rebuildFromPrimary( Title::makeTitle( NS_MAIN, 'AnyPage' ) ); + + $this->assertSame( + [ IDBAccessObject::READ_LATEST ], + $this->pageDataReads, + 'a rebuild right after a write must not be served the revision the write replaced' + ); + } + private function newRebuilder( ?RevisionRecord $revision ): SubjectPageRebuilder { $page = $this->createStub( WikiPage::class ); $page->method( 'getRevisionRecord' )->willReturn( $revision ); + $page->method( 'loadPageData' )->willReturnCallback( + function ( int $from ): void { + $this->pageDataReads[] = $from; + } + ); $factory = $this->createStub( WikiPageFactory::class ); $factory->method( 'newFromTitle' )->willReturn( $page ); diff --git a/tests/phpunit/EntryPoints/HookPathIsolatesProjectionFailureTest.php b/tests/phpunit/EntryPoints/HookPathIsolatesProjectionFailureTest.php index dbe6c5db..6a79aa20 100644 --- a/tests/phpunit/EntryPoints/HookPathIsolatesProjectionFailureTest.php +++ b/tests/phpunit/EntryPoints/HookPathIsolatesProjectionFailureTest.php @@ -16,9 +16,11 @@ use StatusValue; /** - * Guards the hook-facing write path at the wiring level: an edit and a delete must survive a backend - * that is down, and a failing backend must not starve the backends composed after it. Undeletes run - * through the same getStoreContentUC() wiring, so they are covered by the same guard. + * Guards the hook-facing write path at the wiring level: an edit, a delete and an import must survive a + * backend that is down, and a failing backend must not starve the backends composed after it. Undeletes + * run through the same getStoreContentUC() wiring, so they are covered by the same guard. Imports get + * their own case: they reach that wiring through a second factory method, which could be rewired to the + * propagating rebuild handler on its own. * * The counterpart of RebuildPathPropagatesProjectionFailureTest, which guards the opposite need on * the maintenance path. Both are wiring tests on purpose: the decorator has its own unit tests, but @@ -62,6 +64,19 @@ public function testDeleteCommitsWhenABackendIsDown(): void { $this->assertStatusGood( $status, 'the deletion should still commit while a backend is unreachable' ); } + public function testImportCommitsWhenABackendIsDown(): void { + $this->createPageWithSubjects( 'Import during outage', TestSubject::build() ); + $xml = $this->exportPageToXml( 'Import during outage' ); + $this->registerGraphDatabasePlugins( new ThrowingGraphDatabasePlugin() ); + + $this->importXml( str_replace( 'Import during outage', 'Imported during outage', $xml ) ); + + $this->assertTrue( + Title::newFromText( 'Imported during outage' )->exists(), + 'the import should still commit while a backend is unreachable' + ); + } + public function testFailingBackendDoesNotStarveTheBackendsAfterIt(): void { $spy = new SpyGraphDatabasePlugin(); $this->registerGraphDatabasePlugins( new ThrowingGraphDatabasePlugin(), $spy ); diff --git a/tests/phpunit/EntryPoints/ImportGraphProjectionTest.php b/tests/phpunit/EntryPoints/ImportGraphProjectionTest.php new file mode 100644 index 00000000..dd894489 --- /dev/null +++ b/tests/phpunit/EntryPoints/ImportGraphProjectionTest.php @@ -0,0 +1,120 @@ +setUpNeo4j(); + $this->createSchema( TestSubject::DEFAULT_SCHEMA_ID ); + $this->markPageTableAsUsed(); + } + + public function testImportedPageProjectsItsSubjects(): void { + $this->createPageWithSubjects( + 'Import source', + TestSubject::build( label: 'Imported subject' ) + ); + $xml = $this->exportPageToXml( 'Import source' ); + $this->emptyTheGraph(); + + $this->importXml( str_replace( 'Import source', 'Import target', $xml ) ); + + $this->assertSame( + [ 'Imported subject' ], + $this->readSubjectLabels( $this->pageIdOf( 'Import target' ) ) + ); + } + + public function testImportedPageNodeUsesTheTitleItWasImportedUnder(): void { + $this->createPageWithSubjects( 'Rename source', TestSubject::build() ); + $xml = $this->exportPageToXml( 'Rename source' ); + $this->emptyTheGraph(); + + $this->importXml( str_replace( 'Rename source', 'Rename target', $xml ) ); + + $this->assertSame( 'Rename target', $this->readPageNodeName( $this->pageIdOf( 'Rename target' ) ) ); + } + + public function testImportingNewerRevisionUpdatesProjectionOfExistingPage(): void { + $this->createPageWithSubjects( 'Update source', TestSubject::build( label: 'Before import' ) ); + $outdatedXml = $this->exportPageToXml( 'Update source' ); + $this->createPageWithSubjects( 'Update source', TestSubject::build( label: 'After import' ) ); + $currentXml = $this->exportPageToXml( 'Update source' ); + $this->emptyTheGraph(); + + $this->importXml( str_replace( 'Update source', 'Update target', $outdatedXml ) ); + $this->importXml( str_replace( 'Update source', 'Update target', $currentXml ) ); + + $this->assertSame( + [ 'After import' ], + $this->readSubjectLabels( $this->pageIdOf( 'Update target' ) ), + 'the projection must follow the newly imported revision, not the one it replaced' + ); + } + + public function testImportedPageWithoutSubjectSlotIsNotProjected(): void { + $this->insertPage( 'Plain source', 'Just wikitext, no subjects.' ); + $xml = $this->exportPageToXml( 'Plain source' ); + $this->emptyTheGraph(); + + $this->importXml( str_replace( 'Plain source', 'Plain target', $xml ) ); + + $this->assertSame( 0, $this->countPageNodes( $this->pageIdOf( 'Plain target' ) ) ); + } + + /** + * Stands in for importing into another wiki, whose graph knows nothing of the exported page. Without + * this the source page's own projection stays behind, and since Subject nodes are keyed globally, the + * source and target pages would share one Subject node — leaving the assertions unable to tell a + * subject the import projected from one the source page had already put there. + */ + private function emptyTheGraph(): void { + $this->writeGraph( 'MATCH (n) DETACH DELETE n' ); + } + + private function pageIdOf( string $pageName ): int { + $pageId = Title::newFromText( $pageName )->getArticleID(); + + $this->assertNotSame( 0, $pageId, "the import should have created {$pageName}" ); + + return $pageId; + } + + /** + * @return string[] + */ + private function readSubjectLabels( int $pageId ): array { + $result = $this->readGraph( + 'MATCH (page:Page {id: $pageId})-[:HasSubject]->(subject:Subject) RETURN subject.name AS name ORDER BY name', + [ 'pageId' => $pageId ] + ); + + return array_column( $result->toRecursiveArray(), 'name' ); + } + + private function countPageNodes( int $pageId ): int { + $result = $this->readGraph( + 'MATCH (page:Page {id: $pageId}) RETURN count(page) AS count', + [ 'pageId' => $pageId ] + ); + + return (int)$result->first()->toRecursiveArray()['count']; + } + +} diff --git a/tests/phpunit/NeoWikiIntegrationTestCase.php b/tests/phpunit/NeoWikiIntegrationTestCase.php index a9819fda..326e3e0f 100644 --- a/tests/phpunit/NeoWikiIntegrationTestCase.php +++ b/tests/phpunit/NeoWikiIntegrationTestCase.php @@ -4,9 +4,12 @@ namespace ProfessionalWiki\NeoWiki\Tests; +use DumpStringOutput; +use ImportStringSource; use Laudis\Neo4j\Databags\SummarizedResult; use MediaWiki\CommentStore\CommentStoreComment; use MediaWiki\Content\TextContent; +use MediaWiki\Deferred\DeferredUpdates; use MediaWiki\MediaWikiServices; use MediaWiki\Revision\RevisionRecord; use MediaWiki\Title\Title; @@ -24,6 +27,7 @@ use ProfessionalWiki\NeoWiki\Tests\Data\TestSchema; use ProfessionalWiki\NeoWiki\Tests\Data\TestSubject; use ProfessionalWiki\NeoWiki\Tests\TestDoubles\InMemorySchemaLookup; +use WikiExporter; class NeoWikiIntegrationTestCase extends MediaWikiIntegrationTestCase { @@ -89,6 +93,37 @@ protected function createMapping( string $name, string $json ): ?RevisionRecord return $updater->saveRevision( CommentStoreComment::newUnsavedComment( 'TODO' ) ); } + protected function exportPageToXml( string $pageName ): string { + $exporter = $this->getServiceContainer()->getWikiExporterFactory()->getWikiExporter( + $this->getDb(), + WikiExporter::FULL + ); + + $sink = new DumpStringOutput(); + $exporter->setOutputSink( $sink ); + $exporter->openStream(); + $exporter->pageByName( $pageName ); + $exporter->closeStream(); + + return (string)$sink; + } + + /** + * Imports without a reporter, as importDump.php does. Special:Import and the import API wrap the + * importer in an ImportReporter, which creates a null revision on top of the import and so happens + * to fire RevisionFromEditComplete as well. Importing bare keeps tests on the import path only. + */ + protected function importXml( string $xml ): void { + $importer = $this->getServiceContainer()->getWikiImporterFactory()->getWikiImporter( + new ImportStringSource( $xml ), + $this->getTestSysop()->getAuthority() + ); + + $importer->doImport(); + + DeferredUpdates::doUpdates(); + } + protected function markPageTableAsUsed(): void { if ( !in_array( 'page', $this->tablesUsed ) ) { $this->tablesUsed[] = 'page';