Addshore has uploaded a new change for review. https://gerrit.wikimedia.org/r/312257
Change subject: Use new db schema ...................................................................... Use new db schema The new schema introduced by this commit is a slightly altered version of that discussed in: https://phabricator.wikimedia.org/T145412#2653021 Changes: - wiki id is not recorded, instead a language is simply stored. There is no central sites table, and thus a numeric id for a site is hard, unless we introduce our own sites table or some other global sites table. Without either of these we would have to store the whole id eg. enwiktionary, but I can see little need for this and just 'en' should be fine. - There is no unique index accross language, namespace and key as per the discussed issues where multiple different titles could be normalized to be have the same key. There is however a regular index accross these fields. The functionality of Cognate remains the same before and after this change so there is nothing blocking merging this right away. Future todos: - Actually handle multiple namespaces (this was never been done) but the fact this is not done is now even more aparant. - Introduce some normalizing of titles -> keys Bug: T145412 Change-Id: I01b4612a2e6454f620e7787d29364d0556759e1c --- A db/addCognateNamespaces.sql A db/addCognateTitles.sql D db/addInterLanguageTable.sql M src/CognateHooks.php M src/CognateStore.php M src/hooks/CognatePageHookHandler.php M tests/phpunit/CognateStoreTest.php M tests/phpunit/hooks/CognatePageHookHandlerTest.php 8 files changed, 96 insertions(+), 70 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Cognate refs/changes/57/312257/1 diff --git a/db/addCognateNamespaces.sql b/db/addCognateNamespaces.sql new file mode 100644 index 0000000..a5e859e --- /dev/null +++ b/db/addCognateNamespaces.sql @@ -0,0 +1,11 @@ +-- Add cognate_namespaces table + +CREATE TABLE IF NOT EXISTS /*_*/cognate_namespaces ( + cgns_id INT unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT, + cgns_language VARBINARY(32) NOT NULL, + cgns_namespace_name INT NOT NULL, + cgns_namespace_id INT NOT NULL, + cgns_virtual_id INT NOT NULL + )/*$wgDBTableOptions*/; + +CREATE UNIQUE INDEX /*i*/cgns_namespaces ON /*_*/cognate_namespaces (cgns_language, cgns_namespace_id, cgns_virtual_id); \ No newline at end of file diff --git a/db/addCognateTitles.sql b/db/addCognateTitles.sql new file mode 100644 index 0000000..1a62a15 --- /dev/null +++ b/db/addCognateTitles.sql @@ -0,0 +1,12 @@ +-- Add cognate_titles table + +CREATE TABLE IF NOT EXISTS /*_*/cognate_titles ( + cgti_id INT unsigned NOT NULL PRIMARY KEY AUTO_INCREMENT, + cgns_language VARBINARY(32) NOT NULL, + cgti_virtual_namespace INT NOT NULL, + cgti_title VARBINARY(255), + cgti_key VARBINARY(255) NOT NULL + )/*$wgDBTableOptions*/; + +CREATE INDEX /*i*/cgti_keys ON /*_*/cognate_titles (cgns_language, cgti_virtual_namespace, cgti_key); +CREATE UNIQUE INDEX /*i*/cgti_titles ON /*_*/cognate_titles (cgns_language, cgti_virtual_namespace, cgti_title); \ No newline at end of file diff --git a/db/addInterLanguageTable.sql b/db/addInterLanguageTable.sql deleted file mode 100644 index 79b0fb6..0000000 --- a/db/addInterLanguageTable.sql +++ /dev/null @@ -1,9 +0,0 @@ --- Add article assessments table - -CREATE TABLE IF NOT EXISTS /*_*/inter_language_titles ( - ilt_language VARCHAR(20) NOT NULL, - ilt_title VARCHAR(255) NOT NULL - )/*$wgDBTableOptions*/; - -CREATE INDEX /*i*/ilt_language ON /*_*/inter_language_titles (ilt_language); -CREATE UNIQUE INDEX /*i*/ilt_title_language ON /*_*/inter_language_titles (ilt_title, ilt_language); \ No newline at end of file diff --git a/src/CognateHooks.php b/src/CognateHooks.php index 1bd91ce..dba5c72 100644 --- a/src/CognateHooks.php +++ b/src/CognateHooks.php @@ -68,23 +68,23 @@ /** * Run database updates * - * Only runs the update when $wgCognateWiki is false (i.e. for testing and - * when updating the "main" wiktionary project. + * Only runs the update when both $wgCognateDb and $wgCognateCluster are false + * i.e. for testing. * * @param DatabaseUpdater $updater DatabaseUpdater object * @return bool */ public static function onLoadExtensionSchemaUpdates( DatabaseUpdater $updater = null ) { - global $wgCognateWiki; + global $wgCognateDb, $wgCognateCluster; - if ( $wgCognateWiki ) { - return true; + if ( $wgCognateDb === false && $wgCognateCluster === false ) { + $updater->addExtensionUpdate( + [ 'addTable', 'cognate_titles', __DIR__ . '/../db/addCognateTitles.sql', true ] + ); + $updater->addExtensionUpdate( + [ 'addTable', 'cognate_namespaces', __DIR__ . '/../db/addCognateNamespaces.sql', true ] + ); } - - $dbDir = __DIR__ . '/../db'; - $updater->addExtensionUpdate( - array( 'addTable', 'inter_language_titles', "$dbDir/addInterLanguageTable.sql", true ) - ); return true; } diff --git a/src/CognateStore.php b/src/CognateStore.php index 287c58c..fac0592 100644 --- a/src/CognateStore.php +++ b/src/CognateStore.php @@ -1,8 +1,11 @@ <?php +use MediaWiki\Linker\LinkTarget; + /** * @license GNU GPL v2+ * @author Gabriel Birke < gabriel.bi...@wikimedia.de > + * @author Addshore */ class CognateStore { @@ -11,44 +14,46 @@ */ private $loadBalancer; - const TABLE_NAME = 'inter_language_titles'; + const TABLE_TITLES = 'cognate_titles'; /** - * @param LoadBalancer $loadBalancer + * @param ILoadBalancer $loadBalancer */ - public function __construct( LoadBalancer $loadBalancer ) { + public function __construct( ILoadBalancer $loadBalancer ) { $this->loadBalancer = $loadBalancer; } /** * @param string $language Language code, taken from $wgLanguageCode - * @param string $title Page title + * @param LinkTarget $linkTarget * @return bool */ - public function savePage( $language, $title ) { + public function savePage( $language, LinkTarget $linkTarget ) { $pageData = [ - 'ilt_language' => $language, - 'ilt_title' => $title + 'cgti_site' => $language, + 'cgti_title' => $linkTarget->getDBkey(), + 'cgti_virtual_namespace' => 0,// TODO map virtual namespaces + 'cgti_key' => $linkTarget->getDBkey(),// TODO normalize ]; $dbw = $this->loadBalancer->getConnectionRef( DB_MASTER ); - $result = $dbw->insert( self::TABLE_NAME, $pageData, __METHOD__, [ 'IGNORE' ] ); + $result = $dbw->insert( self::TABLE_TITLES, $pageData, __METHOD__, [ 'IGNORE' ] ); return $result; } /** * @param string $language Language code, taken from $wgLanguageCode - * @param string $title Page title + * @param LinkTarget $linkTarget * @return bool */ - public function deletePage( $language, $title ) { + public function deletePage( $language, LinkTarget $linkTarget ) { $pageData = [ - 'ilt_language' => $language, - 'ilt_title' => $title + 'cgti_site' => $language, + 'cgti_title' => $linkTarget->getDBkey(), + 'cgti_virtual_namespace' => 0,// TODO map virtual namespaces ]; - $dbw = $this->loadBalancer->getConnection( DB_MASTER ); - $result = $dbw->delete( self::TABLE_NAME, $pageData, __METHOD__ ); - $this->loadBalancer->reuseConnection( $dbw ); + $dbw = $this->loadBalancer->getConnectionRef( DB_MASTER ); + $result = $dbw->delete( self::TABLE_TITLES, $pageData, __METHOD__ ); return $result; } @@ -57,20 +62,25 @@ * Get the language codes where a translations is available * * @param string $language Language code to exclude - * @param string $title Page title + * @param LinkTarget $linkTarget * @return array language codes */ - public function getTranslationsForPage( $language, $title ) { + public function getTranslationsForPage( $language, LinkTarget $linkTarget ) { $languages = []; $dbr = $this->loadBalancer->getConnectionRef( DB_SLAVE ); - $result = $dbr->select( self::TABLE_NAME, ['ilt_language'], [ - 'ilt_language != ' . $dbr->addQuotes( $language ), - 'ilt_title' => $title - ] ); + $result = $dbr->select( + self::TABLE_TITLES, + [ 'cgti_site' ], + [ + 'cgti_site != ' . $dbr->addQuotes( $language ), + 'cgti_key' => $linkTarget->getDBkey(),// TODO normalize + 'cgti_virtual_namespace' => 0,// TODO map virtual namespaces + ] + ); while ( $row = $result->fetchRow() ) { - $languages[] = $row[ 'ilt_language' ]; + $languages[] = str_replace( 'wiktionary', '', $row[ 'cgti_site' ] ); } return $languages; diff --git a/src/hooks/CognatePageHookHandler.php b/src/hooks/CognatePageHookHandler.php index 1fb288f..80e32df 100644 --- a/src/hooks/CognatePageHookHandler.php +++ b/src/hooks/CognatePageHookHandler.php @@ -61,7 +61,7 @@ ) { if ( $article->getTitle()->inNamespaces( $this->namespaces ) ) { $store = MediaWikiServices::getInstance()->getService( 'CognateStore' ); - $store->savePage( $this->languageCode, $article->getTitle()->getDBkey() ); + $store->savePage( $this->languageCode, $article->getTitle()->getTitleValue() ); } } @@ -84,7 +84,7 @@ $updates[] = new MWCallableUpdate( function () use ( $title, $language ){ $store = MediaWikiServices::getInstance()->getService( 'CognateStore' ); - $store->deletePage( $language, $title->getDBkey() ); + $store->deletePage( $language, $title->getTitleValue() ); }, __METHOD__ ); @@ -108,7 +108,7 @@ ) { if ( $title->inNamespaces( $this->namespaces ) ) { $store = MediaWikiServices::getInstance()->getService( 'CognateStore' ); - $store->savePage( $this->languageCode, $title->getDBkey() ); + $store->savePage( $this->languageCode, $title->getTitleValue() ); } } diff --git a/tests/phpunit/CognateStoreTest.php b/tests/phpunit/CognateStoreTest.php index bf8ec45..65ef2ef 100644 --- a/tests/phpunit/CognateStoreTest.php +++ b/tests/phpunit/CognateStoreTest.php @@ -1,5 +1,7 @@ <?php +use MediaWiki\MediaWikiServices; + /** * @license GNU GPL v2+ * @author Gabriel Birke < gabriel.bi...@wikimedia.de > @@ -12,47 +14,47 @@ protected function setUp() { parent::setUp(); - $this->tablesUsed = [ 'inter_language_titles' ]; - $this->store = new CognateStore( wfGetLB(), false ); + $this->tablesUsed = [ CognateStore::TABLE_TITLES ]; + $this->store = MediaWikiServices::getInstance()->getService( 'CognateStore' ); } public function testSavePageCreatesNewEntry() { - $this->store->savePage( 'en', 'My_test_page' ); + $this->store->savePage( 'en', new TitleValue( 0, 'My_test_page' ) ); $this->assertSelect( - 'inter_language_titles', - [ 'ilt_language', 'ilt_title' ], - [ 'ilt_title != "UTPage"' ], - [ [ 'en', 'My_test_page' ] ] + 'cognate_titles', + [ 'cgti_site', 'cgti_title', 'cgti_key', 'cgti_virtual_namespace' ], + [ 'cgti_title != "UTPage"' ], + [ [ 'enwiktionary', 'My_test_page', 'My_test_page', 0 ] ] ); } public function testSavePageWithExistingEntryIgnoresErrors() { - $this->store->savePage( 'en', 'My_second_test_page' ); - $this->store->savePage( 'en', 'My_second_test_page' ); + $this->store->savePage( 'en', new TitleValue( 0, 'My_second_test_page' ) ); + $this->store->savePage( 'en', new TitleValue( 0, 'My_second_test_page' ) ); $this->assertSelect( - 'inter_language_titles', - [ 'ilt_language', 'ilt_title' ], - [ 'ilt_title != "UTPage"' ], - [ [ 'en', 'My_second_test_page' ] ] + 'cognate_titles', + [ 'cgti_site', 'cgti_title', 'cgti_key', 'cgti_virtual_namespace' ], + [ 'cgti_title != "UTPage"' ], + [ [ 'enwiktionary', 'My_second_test_page', 'My_second_test_page', 0 ] ] ); } public function testGetTranslationsForPageReturnsAllLanguages() { - $this->store->savePage( 'en', 'My_test_page' ); - $this->store->savePage( 'en', 'Another_unrelated_page' ); - $this->store->savePage( 'de', 'My_test_page' ); - $this->store->savePage( 'eo', 'My_test_page' ); - $languages = $this->store->getTranslationsForPage( 'en', 'My_test_page' ); + $this->store->savePage( 'en', new TitleValue( 0, 'My_test_page' ) ); + $this->store->savePage( 'en', new TitleValue( 0, 'Another_unrelated_page' ) ); + $this->store->savePage( 'de', new TitleValue( 0, 'My_test_page' ) ); + $this->store->savePage( 'eo', new TitleValue( 0, 'My_test_page' ) ); + $languages = $this->store->getTranslationsForPage( 'en', new TitleValue( 0, 'My_test_page' ) ); $this->assertArrayEquals( [ 'de', 'eo' ], $languages ); } public function testSaveAndDeletePageResultsInNoEntry() { - $this->store->savePage( 'en', 'My_test_page' ); - $this->store->deletePage( 'en', 'My_test_page' ); + $this->store->savePage( 'en', new TitleValue( 0, 'My_test_page' ) ); + $this->store->deletePage( 'en', new TitleValue( 0, 'My_test_page' ) ); $this->assertSelect( - 'inter_language_titles', - [ 'ilt_language', 'ilt_title' ], - [ 'ilt_title != "UTPage"' ], + 'cognate_titles', + [ 'cgti_site', 'cgti_title', 'cgti_key', 'cgti_virtual_namespace' ], + [ 'cgti_title != "UTPage"' ], [] ); } diff --git a/tests/phpunit/hooks/CognatePageHookHandlerTest.php b/tests/phpunit/hooks/CognatePageHookHandlerTest.php index 3834b01..3e40c4d 100644 --- a/tests/phpunit/hooks/CognatePageHookHandlerTest.php +++ b/tests/phpunit/hooks/CognatePageHookHandlerTest.php @@ -34,7 +34,7 @@ ->method( 'deletePage' ); $this->store->expects( $this->once() ) ->method( 'savePage' ) - ->with( 'abc2', 'ArticleDbKey' ); + ->with( 'abc2', new TitleValue( 0, 'ArticleDbKey' ) ); $this->call_onPageContentSaveComplete( [ 0 ], 'abc2', new TitleValue( 0, 'ArticleDbKey' ) ); } @@ -80,7 +80,7 @@ public function test_onWikiPageDeletionUpdates_namespaceMatch() { $this->store->expects( $this->once() ) ->method( 'deletePage' ) - ->with( 'abc2', 'ArticleDbKey' ); + ->with( 'abc2', new TitleValue( 0, 'ArticleDbKey' ) ); $this->store->expects( $this->never() ) ->method( 'savePage' ); @@ -146,7 +146,7 @@ ->method( 'deletePage' ); $this->store->expects( $this->once() ) ->method( 'savePage' ) - ->with( 'abc2', 'ArticleDbKey' ); + ->with( 'abc2', new TitleValue( 0, 'ArticleDbKey' ) ); $this->call_onArticleUndelete( [ 0 ], -- To view, visit https://gerrit.wikimedia.org/r/312257 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I01b4612a2e6454f620e7787d29364d0556759e1c Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Cognate Gerrit-Branch: master Gerrit-Owner: Addshore <addshorew...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
