jenkins-bot has submitted this change and it was merged.

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 custom namespaces, it has already been set out in the
   ticket and will not be hard to implement in the future.
 - Introduce some normalizing of titles -> keys

Bug: T145412
Change-Id: I01b4612a2e6454f620e7787d29364d0556759e1c
---
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
7 files changed, 139 insertions(+), 98 deletions(-)

Approvals:
  Hoo man: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/db/addCognateTitles.sql b/db/addCognateTitles.sql
new file mode 100644
index 0000000..e6c4ff4
--- /dev/null
+++ b/db/addCognateTitles.sql
@@ -0,0 +1,11 @@
+-- Add cognate_titles table
+
+CREATE TABLE IF NOT EXISTS /*_*/cognate_titles (
+  cgti_site VARBINARY(32) NOT NULL,
+  cgti_namespace INT NOT NULL,
+  cgti_title VARBINARY(255),
+  cgti_key VARBINARY(255) NOT NULL,
+  PRIMARY KEY (cgti_site, cgti_namespace, cgti_title)
+  )/*$wgDBTableOptions*/;
+
+CREATE INDEX /*i*/cgti_keys ON /*_*/cognate_titles (cgti_site, cgti_namespace, 
cgti_key);
\ 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..9bc2d4b 100644
--- a/src/CognateHooks.php
+++ b/src/CognateHooks.php
@@ -50,11 +50,12 @@
                        return true;
                }
 
+               /** @var CognateStore $store */
                $store = MediaWikiServices::getInstance()->getService( 
'CognateStore' );
-               $dbKey = $title->getDBkey();
-               $languages = $store->getTranslationsForPage( $wgLanguageCode, 
$dbKey );
+               $languages = $store->getLinksForPage( $wgLanguageCode, $title );
 
-               foreach( $languages as $lang ) {
+               $dbKey = $title->getDBkey();
+               foreach ( $languages as $lang ) {
                        if ( !isset( $links[$lang] ) ) {
                                $links[$lang] = $lang . ':' . $dbKey;
                        }
@@ -68,23 +69,20 @@
        /**
         * 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 ]
+                       );
                }
-
-               $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..53e16e0 100644
--- a/src/CognateStore.php
+++ b/src/CognateStore.php
@@ -1,79 +1,88 @@
 <?php
 
+use MediaWiki\Linker\LinkTarget;
+
 /**
  * @license GNU GPL v2+
  * @author Gabriel Birke < gabriel.bi...@wikimedia.de >
+ * @author Addshore
  */
 class CognateStore {
 
        /**
-        * @var LoadBalancer
+        * @var ILoadBalancer
         */
        private $loadBalancer;
 
-       const TABLE_NAME = 'inter_language_titles';
+       const TITLES_TABLE_NAME = '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 string $siteLinkPrefix The prefix for generated links
+        * @param LinkTarget $linkTarget
+        *
         * @return bool
         */
-       public function savePage( $language, $title ) {
+       public function savePage( $siteLinkPrefix, LinkTarget $linkTarget ) {
                $pageData = [
-                       'ilt_language' => $language,
-                       'ilt_title' => $title
+                       'cgti_site' => $siteLinkPrefix,
+                       'cgti_title' => $linkTarget->getDBkey(),
+                       'cgti_namespace' => $linkTarget->getNamespace(),
+                       '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::TITLES_TABLE_NAME, $pageData, 
__METHOD__, [ 'IGNORE' ] );
 
                return $result;
        }
 
        /**
-        * @param string $language Language code, taken from $wgLanguageCode
-        * @param string $title Page title
+        * @param string $siteLinkPrefix The prefix for generated links
+        * @param LinkTarget $linkTarget
+        *
         * @return bool
         */
-       public function deletePage( $language, $title ) {
+       public function deletePage( $siteLinkPrefix, LinkTarget $linkTarget ) {
                $pageData = [
-                       'ilt_language' => $language,
-                       'ilt_title' => $title
+                       'cgti_site' => $siteLinkPrefix,
+                       'cgti_title' => $linkTarget->getDBkey(),
+                       'cgti_namespace' => $linkTarget->getNamespace(),
                ];
-               $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::TITLES_TABLE_NAME, $pageData, 
__METHOD__ );
 
-               return $result;
+               return (bool)$result;
        }
 
        /**
-        * Get the language codes where a translations is available
-        *
-        * @param string $language Language code to exclude
-        * @param string $title Page title
-        * @return array language codes
+        * @param string $siteLinkPrefix The prefix for generated links
+        * @param LinkTarget $linkTarget
+        * @return string[] language codes
         */
-       public function getTranslationsForPage( $language, $title ) {
-               $languages = [];
-
+       public function getLinksForPage( $siteLinkPrefix, LinkTarget 
$linkTarget ) {
                $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::TITLES_TABLE_NAME,
+                       [ 'cgti_site' ],
+                       [
+                               'cgti_site != ' . $dbr->addQuotes( 
$siteLinkPrefix ),
+                               'cgti_key' => $linkTarget->getDBkey(),// TODO 
normalize
+                               'cgti_namespace' => $linkTarget->getNamespace(),
+                       ]
+               );
 
+               $siteLinkPrefixes = [];
                while ( $row = $result->fetchRow() ) {
-                       $languages[] = $row[ 'ilt_language' ];
+                       $siteLinkPrefixes[] = $row[ 'cgti_site' ];
                }
 
-               return $languages;
+               return $siteLinkPrefixes;
        }
 
 }
diff --git a/src/hooks/CognatePageHookHandler.php 
b/src/hooks/CognatePageHookHandler.php
index 1fb288f..ae09a79 100644
--- a/src/hooks/CognatePageHookHandler.php
+++ b/src/hooks/CognatePageHookHandler.php
@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\Linker\LinkTarget;
 use MediaWiki\MediaWikiServices;
 use Wikimedia\Assert\Assert;
 
@@ -34,7 +35,7 @@
         * Occurs after the save page request has been processed.
         * @see 
https://www.mediawiki.org/wiki/Manual:Hooks/PageContentSaveComplete
         *
-        * @param WikiPage $article
+        * @param WikiPage $page
         * @param User $user
         * @param Content $content
         * @param string $summary
@@ -47,7 +48,7 @@
         * @param integer $baseRevId
         */
        public function onPageContentSaveComplete(
-               WikiPage $article,
+               WikiPage $page,
                User $user,
                Content $content,
                $summary,
@@ -59,9 +60,9 @@
                Status $status,
                $baseRevId
        ) {
-               if ( $article->getTitle()->inNamespaces( $this->namespaces ) ) {
-                       $store = MediaWikiServices::getInstance()->getService( 
'CognateStore' );
-                       $store->savePage( $this->languageCode, 
$article->getTitle()->getDBkey() );
+               $titleValue = $page->getTitle()->getTitleValue();
+               if ( $this->isActionableTarget( $titleValue ) ) {
+                       $this->getStore()->savePage( $this->languageCode, 
$titleValue );
                }
        }
 
@@ -78,17 +79,26 @@
                Content $content = null,
                array &$updates
        ) {
-               $title = $page->getTitle();
+               $titleValue = $page->getTitle()->getTitleValue();
                $language = $this->languageCode;
-               if ( $title->inNamespaces( $this->namespaces ) ) {
-                       $updates[] = new MWCallableUpdate(
-                               function () use ( $title, $language ){
-                                       $store = 
MediaWikiServices::getInstance()->getService( 'CognateStore' );
-                                       $store->deletePage( $language, 
$title->getDBkey() );
-                               },
-                               __METHOD__
-                       );
+               if ( $this->isActionableTarget( $titleValue ) ) {
+                       $updates[] = $this->newDeferrableDelete( $titleValue, 
$language );
                }
+       }
+
+       /**
+        * @param TitleValue $titleValue
+        * @param string $language
+        *
+        * @return MWCallableUpdate
+        */
+       private function newDeferrableDelete( TitleValue $titleValue, $language 
) {
+               return new MWCallableUpdate(
+                       function () use ( $language, $titleValue ){
+                               $this->getStore()->deletePage( $language, 
$titleValue );
+                       },
+                       __METHOD__
+               );
        }
 
        /**
@@ -106,10 +116,30 @@
                $comment,
                $oldPageId
        ) {
-               if ( $title->inNamespaces( $this->namespaces ) ) {
-                       $store = MediaWikiServices::getInstance()->getService( 
'CognateStore' );
-                       $store->savePage( $this->languageCode, 
$title->getDBkey() );
+               $titleValue = $title->getTitleValue();
+               if ( $this->isActionableTarget( $titleValue ) ) {
+                       $this->getStore()->savePage( $this->languageCode, 
$titleValue );
                }
        }
 
+       /**
+        * Actionable targets have a namespace id that is:
+        *  - One of the default MediaWiki (between NS_MAIN and NS_CATEGORY_TALK
+        *  - Defined as a namespace to record in the configuration
+        * @param LinkTarget $linkTarget
+        * @return bool
+        */
+       private function isActionableTarget( LinkTarget $linkTarget ) {
+               $namespace = $linkTarget->getNamespace();
+               return in_array( $namespace, $this->namespaces ) &&
+                       $namespace >= NS_MAIN && $namespace <= NS_CATEGORY_TALK;
+       }
+
+       /**
+        * @return CognateStore
+        */
+       private function getStore() {
+               return MediaWikiServices::getInstance()->getService( 
'CognateStore' );
+       }
+
 }
\ No newline at end of file
diff --git a/tests/phpunit/CognateStoreTest.php 
b/tests/phpunit/CognateStoreTest.php
index bf8ec45..f585ebd 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::TITLES_TABLE_NAME ];
+               $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_namespace' ],
+                       [ 'cgti_title != "UTPage"' ],
+                       [ [ 'en', '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_namespace' ],
+                       [ 'cgti_title != "UTPage"' ],
+                       [ [ 'en', '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->getLinksForPage( '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_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: merged
Gerrit-Change-Id: I01b4612a2e6454f620e7787d29364d0556759e1c
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/Cognate
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Gabriel Birke <gabriel.bi...@wikimedia.de>
Gerrit-Reviewer: Hoo man <h...@online.de>
Gerrit-Reviewer: Jcrespo <jcre...@wikimedia.org>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Springle <sprin...@wikimedia.org>
Gerrit-Reviewer: WMDE-Fisch <christoph.jau...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to