Gergő Tisza has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/392748 )
Change subject: Deduplicate projects into their own table ...................................................................... Deduplicate projects into their own table Create a reading_list_project table for projects (domains). Use it to normalize the project column and not waste space e.g. storing 'en.wikipedia.org' a million times; also to validate projects (the table needs to be pregenerated, projects that are not found in it are rejected). The extension is agnostic about what values are supposed to be used as projects, but a maintenance script is provided which puts the wikifarm's wgCanonicalServer values (things like 'https://en.wikipedia.org') into the new table. Change-Id: I047d1493f1d9f51d733c1925a38c080829589f35 --- M i18n/en.json M i18n/qqq.json A maintenance/populateProjectsFromSiteMatrix.php A sql/patches/02-add-reading_list_project.sql M sql/readinglists.sql M src/Api/ApiQueryReadingListEntries.php M src/Api/ApiReadingListsCreateEntry.php M src/Doc/ReadingListEntryRow.php M src/HookHandler.php M src/ReadingListRepository.php M tests/src/ReadingListRepositoryTest.php 11 files changed, 277 insertions(+), 61 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ReadingLists refs/changes/48/392748/1 diff --git a/i18n/en.json b/i18n/en.json index ee02621..d66ee58 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -21,6 +21,7 @@ "readinglists-db-error-list-limit": "Users cannot have more than $1 {{PLURAL:$1|list|lists}}.", "readinglists-db-error-entry-limit": "List $1 cannot have more than $2 {{PLURAL:$2|entry|entries}}.", "readinglists-db-error-too-long": "Value for field $1 cannot be longer than $2 bytes.", + "readinglists-db-error-no-such-project": "'$1' is not a recognized project.", "readinglists-apierror-project-title-param": "<var>$1project</var> and <var>$1title</var> must be used together.", "readinglists-apierror-too-old": "Timestamps passed to <var>$1changedsince</var> cannot be older than <kbd>$2</kbd>.", "apihelp-query+readinglists-summary": "List or filter the user's reading lists and show metadata about them.", diff --git a/i18n/qqq.json b/i18n/qqq.json index 5334ebb..d4ed940 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -22,6 +22,7 @@ "readinglists-db-error-list-limit": "Error message used when the user has as many or more lists than the maximum allowed, and tries to add another one.\n\nParameters:\n* $1 - the maximum allowed number of lists per user.", "readinglists-db-error-entry-limit": "Error message used when the user has as many or more entries in the given list than the maximum allowed, and tries to add another one.\n\nParameters:\n* $1 - the ID of the list in question.\n$2 - the maximum allowed number of entries per list.", "readinglists-db-error-too-long": "Error message used when the user tries to set list/entry string fields to a longer value than what the database schema allows.\n\nParameters:\n* $1 - DB field name.\n$2 - DB field maximum length (in bytes).", + "readinglists-db-error-no-such-project": "Error message used when the user tries to add a new list entry, but the project does not match any of the known ones.\n\nParameters:\n* $1 - the project.", "readinglists-apierror-project-title-param": "{{doc-apierror}}\n$1 is the module prefix.", "readinglists-apierror-too-old": "{{doc-apierror}}\n$1 is the module prefix, $2 is the expiry date for deleted lists/entries.", "apihelp-query+readinglists-summary": "{{doc-apihelp-summary|query+readinglists}}", diff --git a/maintenance/populateProjectsFromSiteMatrix.php b/maintenance/populateProjectsFromSiteMatrix.php new file mode 100644 index 0000000..0a5fb0e --- /dev/null +++ b/maintenance/populateProjectsFromSiteMatrix.php @@ -0,0 +1,102 @@ +<?php + +namespace MediaWiki\Extensions\ReadingLists\Maintenance; + +use Generator; +use Maintenance; +use MediaWiki\Extensions\ReadingLists\Utils; +use MediaWiki\MediaWikiServices; +use SiteMatrix; + +require_once getenv( 'MW_INSTALL_PATH' ) !== false + ? getenv( 'MW_INSTALL_PATH' ) . '/maintenance/Maintenance.php' + : __DIR__ . '/../../../maintenance/Maintenance.php'; + +/** + * Maintenance script for populating the reading_list_project table. + * If the table is already populated, add new entries (old entries that don't exist anymore are + * left in place). + * Uses the SiteMatrix extension as the data source. + * @ingroup Maintenance + */ +class PopulateProjectsFromSiteMatrix extends Maintenance { + + /** @var SiteMatrix */ + private $siteMatrix; + + public function __construct() { + parent::__construct(); + $this->addDescription( + 'Populate (or update) the reading_list_project table from SiteMatrix data.' ); + $this->setBatchSize( 100 ); + $this->requireExtension( 'SiteMatrix' ); + $this->siteMatrix = new SiteMatrix(); + } + + /** + * @inheritDoc + */ + public function execute() { + $services = MediaWikiServices::getInstance(); + $loadBalancerFactory = $services->getDBLoadBalancerFactory(); + $dbw = Utils::getDB( DB_MASTER, $services ); + $inserted = 0; + + $this->output( "populating...\n" ); + foreach ( $this->generateAllowedDomains() as list( $project ) ) { + $dbw->insert( + 'reading_list_project', + [ 'rlp_project' => $project ], + __METHOD__, + [ 'IGNORE' ] + ); + if ( $dbw->affectedRows() ) { + $inserted++; + if ( $inserted % $this->mBatchSize ) { + $loadBalancerFactory->waitForReplication(); + } + } + } + $this->output( "inserted $inserted projects\n" ); + } + + /** + * List all sites known to SiteMatrix. + * @return Generator [ language, site ] + */ + private function generateSites() { + foreach ( $this->siteMatrix->getSites() as $site ) { + foreach ( $this->siteMatrix->getLangList() as $lang ) { + if ( !$this->siteMatrix->exist( $lang, $site ) ) { + continue; + } + yield [ $lang, $site ]; + } + } + foreach ( $this->siteMatrix->getSpecials() as $special ) { + list( $lang, $site ) = $special; + yield [ $lang, $site ]; + } + } + + /** + * List all sites that are safe to add to the reading list. + * @return Generator [ domain, dbname ] + */ + private function generateAllowedDomains() { + foreach ( $this->generateSites() as list( $lang, $site ) ) { + $dbName = $this->siteMatrix->getDBName( $lang, $site ); + $domain = $this->siteMatrix->getCanonicalUrl( $lang, $site ); + + if ( $this->siteMatrix->isPrivate( $dbName ) ) { + continue; + } + + yield [ $domain, $dbName ]; + } + } + +} + +$maintClass = PopulateProjectsFromSiteMatrix::class; +require_once RUN_MAINTENANCE_IF_MAIN; diff --git a/sql/patches/02-add-reading_list_project.sql b/sql/patches/02-add-reading_list_project.sql new file mode 100644 index 0000000..35fbd15 --- /dev/null +++ b/sql/patches/02-add-reading_list_project.sql @@ -0,0 +1,31 @@ +-- Table for storing projects (domains) efficiently. +CREATE TABLE /*_*/reading_list_project ( + rlp_id INTEGER UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, + -- Wiki project domain. + rlp_project VARBINARY(255) NOT NULL +) /*$wgDBTableOptions*/; +CREATE UNIQUE INDEX /*i*/rlp_project ON /*_*/reading_list_project (rlp_project); + +-- migration step 1: create new column, drop old indexes (can't create new ones yet b/c uniqueness) +ALTER TABLE reading_list_entry + -- Reference to reading_list_project.rlp_id. + ADD COLUMN rle_rlp_id INTEGER UNSIGNED NOT NULL; +DROP INDEX /*i*/rle_user_project_title ON /*_*/reading_list_entry; +DROP INDEX /*i*/rle_list_project_title ON /*_*/reading_list_entry; + +-- migration step 2: copy old column data to new table, write ids into new column +-- add new indexes as we are uniqe now +INSERT IGNORE INTO reading_list_project (rlp_project) + SELECT rle_project FROM reading_list_entry; +UPDATE reading_list_entry + JOIN reading_list_project ON rle_project = rlp_project + SET rle_rlp_id = rlp_id; +-- For getting all lists of a given user which contain a specified page. +CREATE INDEX /*i*/rle_user_project_title ON /*_*/reading_list_entry (rle_user_id, rle_rlp_id, rle_title); +-- For ensuring there are no duplicate pages on a single list. +CREATE UNIQUE INDEX /*i*/rle_list_project_title ON /*_*/reading_list_entry (rle_rl_id, rle_rlp_id, rle_title); + +-- migration step 3: drop old data +ALTER TABLE reading_list_entry + DROP COLUMN rle_project; + diff --git a/sql/readinglists.sql b/sql/readinglists.sql index 29d125f..c860397 100644 --- a/sql/readinglists.sql +++ b/sql/readinglists.sql @@ -42,8 +42,8 @@ rle_rl_id INTEGER UNSIGNED NOT NULL, -- Central ID of user, denormalized for the benefit of the /pages/ route. rle_user_id INTEGER UNSIGNED NOT NULL, - -- Wiki project domain. - rle_project VARCHAR(255) BINARY NOT NULL, + -- Reference to reading_list_project.rlp_id. + rle_rlp_id INTEGER UNSIGNED NOT NULL, -- Page title. -- We can't easily use page ids due to the cross-wiki nature of the project; -- also, page ids don't age well when content is deleted/moved. @@ -59,14 +59,14 @@ -- For getting all entries in a list and for syncing list entries that changed since a given date. CREATE INDEX /*i*/rle_list_updated ON /*_*/reading_list_entry (rle_rl_id, rle_date_updated); -- For getting all lists of a given user which contain a specified page. -CREATE INDEX /*i*/rle_user_project_title ON /*_*/reading_list_entry (rle_user_id, rle_project, rle_title); +CREATE INDEX /*i*/rle_user_project_title ON /*_*/reading_list_entry (rle_user_id, rle_rlp_id, rle_title); -- For ensuring there are no duplicate pages on a single list. -CREATE UNIQUE INDEX /*i*/rle_list_project_title ON /*_*/reading_list_entry (rle_rl_id, rle_project, rle_title); +CREATE UNIQUE INDEX /*i*/rle_list_project_title ON /*_*/reading_list_entry (rle_rl_id, rle_rlp_id, rle_title); --- TODO use lookup table to deduplicate domains --- -- Table for storing domains efficiently. --- CREATE TABLE /*_*/reading_list_domain ( --- rld_id INTEGER UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, --- rld_domain VARBINARY(255) NOT NULL --- ) /*$wgDBTableOptions*/; --- CREATE UNIQUE INDEX /*i*/rld_domain ON /*_*/reading_list_domain (rld_domain); +-- Table for storing projects (domains) efficiently. +CREATE TABLE /*_*/reading_list_project ( + rlp_id INTEGER UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, + -- Wiki project domain. + rlp_project VARCHAR(255) BINARY NOT NULL +) /*$wgDBTableOptions*/; +CREATE UNIQUE INDEX /*i*/rlp_project ON /*_*/reading_list_project (rlp_project); diff --git a/src/Api/ApiQueryReadingListEntries.php b/src/Api/ApiQueryReadingListEntries.php index acc9b77..eadac97 100644 --- a/src/Api/ApiQueryReadingListEntries.php +++ b/src/Api/ApiQueryReadingListEntries.php @@ -200,7 +200,7 @@ return [ 'id' => (int)$row->rle_id, 'listId' => (int)$row->rle_rl_id, - 'project' => $row->rle_project, + 'project' => $row->rlp_project, 'title' => $row->rle_title, 'created' => wfTimestamp( TS_ISO_8601, $row->rle_date_created ), 'updated' => wfTimestamp( TS_ISO_8601, $row->rle_date_updated ), @@ -213,7 +213,7 @@ * @return Title|string */ private function getResultTitle( $row ) { - $interwikiPrefix = $this->getReverseInterwikiLookup()->lookup( $row->rle_project ); + $interwikiPrefix = $this->getReverseInterwikiLookup()->lookup( $row->rlp_project ); if ( is_string( $interwikiPrefix ) ) { if ( $interwikiPrefix === '' ) { $title = Title::newFromText( $row->rle_title ); @@ -236,7 +236,7 @@ // For lack of a better option let's create an invalid title. // ApiPageSet::populateFromTitles() is not documented to accept strings // but it will actually work. - return 'Invalid project|' . $row->rle_project . '|' . $row->rle_title; + return 'Invalid project|' . $row->rlp_project . '|' . $row->rle_title; } } diff --git a/src/Api/ApiReadingListsCreateEntry.php b/src/Api/ApiReadingListsCreateEntry.php index 8221f97..d476c42 100644 --- a/src/Api/ApiReadingListsCreateEntry.php +++ b/src/Api/ApiReadingListsCreateEntry.php @@ -56,7 +56,7 @@ 'project' => [ self::PARAM_TYPE => 'string', self::PARAM_REQUIRED => true, - self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rle_project'], + self::PARAM_MAX_BYTES => ReadingListRepository::$fieldLength['rlp_project'], ], 'title' => [ self::PARAM_TYPE => 'string', diff --git a/src/Doc/ReadingListEntryRow.php b/src/Doc/ReadingListEntryRow.php index edace03..277c5e9 100644 --- a/src/Doc/ReadingListEntryRow.php +++ b/src/Doc/ReadingListEntryRow.php @@ -22,8 +22,15 @@ /** @var string Central ID of user. */ public $rle_user_id; - /** @var string Wiki project domain. */ - public $rle_project; + /** @var int Reference to reading_list_project.rlp_id. */ + public $rle_rlp_id; + + /** + * Wiki project domain. + * Only present when joined with reading_list_project (used for deduplicated storage). + * @var string + */ + public $rlp_project; /** * Page title. diff --git a/src/HookHandler.php b/src/HookHandler.php index b0cc0aa..f8333b8 100644 --- a/src/HookHandler.php +++ b/src/HookHandler.php @@ -16,6 +16,7 @@ public static $testTables = [ 'reading_list', 'reading_list_entry', + 'reading_list_project', ]; /** @@ -44,6 +45,8 @@ $patchDir = "$baseDir/sql/patches"; $updater->addExtensionTable( 'reading_list', "$baseDir/sql/readinglists.sql" ); $updater->dropExtensionTable( 'reading_list_sortkey', "$patchDir/01-drop-sortkeys.sql" ); + $updater->addExtensionTable( 'reading_list_project', + "$patchDir/02-add-reading_list_project.sql" ); } return true; } diff --git a/src/ReadingListRepository.php b/src/ReadingListRepository.php index 794d16c..ed4351e 100644 --- a/src/ReadingListRepository.php +++ b/src/ReadingListRepository.php @@ -10,6 +10,7 @@ use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; +use Wikimedia\Rdbms\FakeResultWrapper; use Wikimedia\Rdbms\IDatabase; // @codingStandardsIgnoreStart MediaWiki.Classes.UnusedUseStatement.UnusedUse use Wikimedia\Rdbms\IResultWrapper; @@ -39,7 +40,7 @@ 'rl_color' => 6, 'rl_image' => 255, 'rl_icon' => 32, - 'rle_project' => 255, + 'rlp_project' => 255, 'rle_title' => 255, ]; @@ -357,7 +358,7 @@ */ public function addListEntry( $id, $project, $title ) { $this->assertUser(); - $this->assertFieldLength( 'rle_project', $project ); + $this->assertFieldLength( 'rlp_project', $project ); $this->assertFieldLength( 'rle_title', $title ); $this->selectValidList( $id, self::READ_LOCKING ); if ( $this->entryLimit && $this->getEntryCount( $id, self::READ_LATEST ) >= $this->entryLimit ) { @@ -365,15 +366,21 @@ [ $id, $this->entryLimit ] ); } + $projectId = $this->getProjectId( $project ); + if ( !$projectId ) { + throw new ReadingListRepositoryException( 'readinglists-db-error-no-such-project', + [ $project ] ); + } + // due to the combination of soft deletion + unique constraint on - // rle_rl_id + rle_project + rle_title, recreation needs special handling + // rle_rl_id + rle_rlp_id + rle_title, recreation needs special handling /** @var ReadingListEntryRow $row */ $row = $this->dbw->selectRow( 'reading_list_entry', [ 'rle_id', 'rle_deleted' ], [ 'rle_rl_id' => $id, - 'rle_project' => $project, + 'rle_rlp_id' => $projectId, 'rle_title' => $title, ], __METHOD__, @@ -386,7 +393,7 @@ [ 'rle_rl_id' => $id, 'rle_user_id' => $this->userId, - 'rle_project' => $project, + 'rle_rlp_id' => $projectId, 'rle_title' => $title, 'rle_date_created' => $this->dbw->timestamp(), 'rle_date_updated' => $this->dbw->timestamp(), @@ -460,9 +467,10 @@ } $res = $this->dbr->select( - 'reading_list_entry', + [ 'reading_list_entry', 'reading_list_project' ], $this->getListEntryFields(), [ + 'rle_rlp_id = rlp_id', 'rle_rl_id' => $ids, 'rle_user_id' => $this->userId, 'rle_deleted' => 0, @@ -579,10 +587,11 @@ public function getListEntriesByDateUpdated( $date, $limit = 1000, $offset = 0 ) { $this->assertUser(); $res = $this->dbr->select( - [ 'reading_list', 'reading_list_entry' ], + [ 'reading_list', 'reading_list_entry', 'reading_list_project' ], $this->getListEntryFields(), [ 'rl_id = rle_rl_id', + 'rle_rlp_id = rlp_id', 'rl_user_id' => $this->userId, 'rl_deleted' => 0, 'rle_date_updated > ' . $this->dbr->addQuotes( $this->dbr->timestamp( $date ) ), @@ -668,13 +677,18 @@ */ public function getListsByPage( $project, $title, $limit = 1000, $offset = 0 ) { $this->assertUser(); + $projectId = $this->getProjectId( $project ); + if ( !$projectId ) { + return new FakeResultWrapper( [] ); + } + $res = $this->dbr->select( [ 'reading_list', 'reading_list_entry' ], $this->getListFields(), [ 'rl_id = rle_rl_id', 'rl_user_id' => $this->userId, - 'rle_project' => $project, + 'rle_rlp_id' => $projectId, 'rle_title' => $title, 'rl_deleted' => 0, 'rle_deleted' => 0, @@ -722,6 +736,7 @@ /** * Get this list of reading_list_entry fields that normally need to be selected. + * Can only be used with queries that join on reading_list_project. * @return array */ private function getListEntryFields() { @@ -729,7 +744,8 @@ 'rle_id', 'rle_rl_id', // returning rle_user_id is pointless as lists are only available to the owner - 'rle_project', + // skip rle_rlp_id, it's only needed for the join + 'rlp_project', 'rle_title', 'rle_date_created', 'rle_date_updated', @@ -814,6 +830,20 @@ } /** + * Look up a project ID. + * @param string $project + * @return int|null + */ + private function getProjectId( $project ) { + $id = $this->dbr->selectField( + 'reading_list_project', + 'rlp_id', + [ 'rlp_project' => $project ] + ); + return $id === false ? null : (int)$id; + } + + /** * Returns the number of (non-deleted) list entries of the given list. * Verifying that the list is valid is caller's responsibility. * @param int $id List id diff --git a/tests/src/ReadingListRepositoryTest.php b/tests/src/ReadingListRepositoryTest.php index dbaed2b..94a968d 100644 --- a/tests/src/ReadingListRepositoryTest.php +++ b/tests/src/ReadingListRepositoryTest.php @@ -408,6 +408,7 @@ public function testAddListEntry() { $repository = new ReadingListRepository( 1, $this->db, $this->db, $this->lbFactory ); $repository->setupForUser(); + list( $projectId ) = $this->addProjects( [ 'en.wikipedia.org' ] ); list( $listId, $deletedListId ) = $this->addLists( 1, [ [ 'rl_name' => 'foo', @@ -423,7 +424,7 @@ /** @var ReadingListEntryRow $row */ $row = $this->db->selectRow( 'reading_list_entry', '*', [ 'rle_id' => $entryId ] ); $this->assertEquals( 1, $row->rle_user_id ); - $this->assertEquals( 'en.wikipedia.org', $row->rle_project ); + $this->assertEquals( $projectId, $row->rle_rlp_id ); $this->assertEquals( 'Foo', $row->rle_title ); $this->assertTimestampEquals( wfTimestampNow(), $row->rle_date_created ); $this->assertTimestampEquals( wfTimestampNow(), $row->rle_date_updated ); @@ -435,7 +436,7 @@ /** @var ReadingListEntryRow $row */ $row = $this->db->selectRow( 'reading_list_entry', '*', [ 'rle_id' => $entryId2 ] ); $this->assertEquals( 1, $row->rle_user_id ); - $this->assertEquals( 'en.wikipedia.org', $row->rle_project ); + $this->assertEquals( $projectId, $row->rle_rlp_id ); $this->assertEquals( 'Foo', $row->rle_title ); $this->assertTimestampEquals( wfTimestampNow(), $row->rle_date_created ); $this->assertTimestampEquals( wfTimestampNow(), $row->rle_date_updated ); @@ -462,6 +463,11 @@ $repository->addListEntry( $listId, 'en.wikipedia.org', 'Foo' ); } ); + $this->assertFailsWith( 'readinglists-db-error-no-such-project', + function () use ( $repository, $listId ) { + $repository->addListEntry( $listId, 'nosuch.project.org', 'Foo' ); + } + ); } // @codingStandardsIgnoreLine MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName @@ -470,6 +476,7 @@ $repository->setLimits( null, 1 ); $repository->setupForUser(); + $this->addProjects( [ 'en.wikipedia.org' ] ); list( $listId ) = $this->addLists( 1, [ [ 'rl_name' => 'foo', @@ -494,7 +501,7 @@ $this->addListEntries( $defaultId, 1, [ [ 'rle_user_id' => 1, - 'rle_project' => 'foo', + 'rlp_project' => 'foo', 'rle_title' => 'Foo', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), @@ -507,28 +514,28 @@ 'rl_name' => 'test', 'entries' => [ [ - 'rle_project' => 'foo', + 'rlp_project' => 'foo', 'rle_title' => 'bar', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), 'rle_deleted' => 0, ], [ - 'rle_project' => 'foo2', + 'rlp_project' => 'foo2', 'rle_title' => 'bar2', 'rle_date_created' => '20100101000000', 'rle_date_updated' => '20120101000000', 'rle_deleted' => 0, ], [ - 'rle_project' => 'foo3', + 'rlp_project' => 'foo3', 'rle_title' => 'bar3', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), 'rle_deleted' => 0, ], [ - 'rle_project' => 'foo4', + 'rlp_project' => 'foo4', 'rle_title' => 'bar4', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), @@ -548,8 +555,8 @@ $this->assertTimestampEquals( $expected['rle_date_updated'], $actual['rle_date_updated'], "Mismatch in item $n" ); unset( $expected['rle_date_created'], $expected['rle_date_updated'] ); - unset( $actual['rle_id'], $actual['rle_date_created'], - $actual['rle_date_updated'] ); + unset( $actual['rle_id'], $actual['rle_rlp_id'], + $actual['rle_date_created'], $actual['rle_date_updated'] ); $this->assertArrayEquals( $expected, $actual, false, true ); }; $compare = function ( $expected, $res ) use ( $compareResultItems ) { @@ -562,7 +569,7 @@ $expectedData = [ [ 'rle_rl_id' => $defaultId, - 'rle_project' => 'foo', + 'rlp_project' => 'foo', 'rle_title' => 'Foo', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), @@ -570,7 +577,7 @@ ], [ 'rle_rl_id' => $listId, - 'rle_project' => 'foo', + 'rlp_project' => 'foo', 'rle_title' => 'bar', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), @@ -578,7 +585,7 @@ ], [ 'rle_rl_id' => $listId, - 'rle_project' => 'foo2', + 'rlp_project' => 'foo2', 'rle_title' => 'bar2', 'rle_date_created' => '20100101000000', 'rle_date_updated' => '20120101000000', @@ -586,7 +593,7 @@ ], [ 'rle_rl_id' => $listId, - 'rle_project' => 'foo3', + 'rlp_project' => 'foo3', 'rle_title' => 'bar3', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), @@ -630,6 +637,7 @@ public function testDeleteListEntry() { $repository = new ReadingListRepository( 1, $this->db, $this->db, $this->lbFactory ); $repository->setupForUser(); + list( $fooProjectId ) = $this->addProjects( [ 'foo' ] ); list( $listId, $deletedListId ) = $this->addLists( 1, [ [ 'rl_is_default' => 0, @@ -645,21 +653,21 @@ ] ); list( $fooId, $foo2Id, $deletedId ) = $this->addListEntries( $listId, 1, [ [ - 'rle_project' => 'foo', + 'rlp_project' => 'foo', 'rle_title' => 'bar', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), 'rle_deleted' => 0, ], [ - 'rle_project' => 'foo2', + 'rlp_project' => 'foo2', 'rle_title' => 'bar2', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), 'rle_deleted' => 0, ], [ - 'rle_project' => 'foo3', + 'rlp_project' => 'foo3', 'rle_title' => 'bar3', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), @@ -668,7 +676,7 @@ ] ); list( $parentDeletedId ) = $this->addListEntries( $deletedListId, 1, [ [ - 'rle_project' => 'foo4', + 'rlp_project' => 'foo4', 'rle_title' => 'bar4', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), @@ -682,7 +690,7 @@ /** @var ReadingListEntryRow $row */ $row = $this->db->selectRow( 'reading_list_entry', '*', [ 'rle_rl_id' => $listId, 'rle_deleted' => 1 ] ); - $this->assertEquals( 'foo', $row->rle_project ); + $this->assertEquals( $fooProjectId, $row->rle_rlp_id ); $this->assertTimestampEquals( wfTimestampNow(), $row->rle_date_updated ); $this->assertFailsWith( 'readinglists-db-error-no-such-list-entry', @@ -750,18 +758,18 @@ 'rl_name' => 'one', 'entries' => [ [ - 'rle_project' => 'new', + 'rlp_project' => 'new', 'rle_title' => 'new', 'rle_date_updated' => '20150101000000', ], [ - 'rle_project' => 'deleted', + 'rlp_project' => 'deleted', 'rle_title' => 'deleted', 'rle_deleted' => 1, 'rle_date_updated' => '20150101000000', ], [ - 'rle_project' => 'old', + 'rlp_project' => 'old', 'rle_title' => 'old', 'rle_date_updated' => '20080101000000', ], @@ -771,7 +779,7 @@ 'rl_name' => 'two', 'entries' => [ [ - 'rle_project' => 'other', + 'rlp_project' => 'other', 'rle_title' => 'other', 'rle_date_updated' => '20150101000000', ], @@ -782,7 +790,7 @@ 'rl_deleted' => 1, 'entries' => [ [ - 'rle_project' => 'parent deleted', + 'rlp_project' => 'parent deleted', 'rle_title' => 'parent deleted', 'rle_date_updated' => '20150101000000', ], @@ -811,18 +819,18 @@ $repository = new ReadingListRepository( null, $this->db, $this->db, $this->lbFactory ); $entries = [ [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'kept', 'rle_date_updated' => wfTimestampNow(), ], [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'deleted-new', 'rle_date_updated' => '20150101000000', 'rle_deleted' => 1, ], [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'deleted-old', 'rle_date_updated' => '20080101000000', 'rle_deleted' => 1, @@ -871,7 +879,7 @@ 'rl_name' => 'first', 'entries' => [ [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'o', ], ], @@ -881,7 +889,7 @@ 'rl_name' => 'second', 'entries' => [ [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'x', 'rle_deleted' => 1, ], @@ -893,7 +901,7 @@ 'rl_deleted' => 1, 'entries' => [ [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'x', ], ], @@ -903,11 +911,11 @@ 'rl_name' => 'fourth', 'entries' => [ [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'x', ], [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'o', ], ], @@ -917,11 +925,11 @@ 'rl_name' => 'fifth', 'entries' => [ [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'x', ], [ - 'rle_project' => '-', + 'rlp_project' => '-', 'rle_title' => 'o', ], ], @@ -1033,6 +1041,7 @@ /** * Creates reading_list_entry rows from the given data, with some magic fields: * - missing list ids will be filled automatically + * - 'rlp_project' will be handled appropriately * @param int $listId The list to add entries to * @param int $userId Th central ID of the list owner * @param array[] $entries Array of rows for reading_list_entry, with some magic fields @@ -1047,11 +1056,43 @@ if ( !isset( $entry['rle_user_id'] ) ) { $entry['rle_user_id'] = $userId; } + if ( isset( $entry['rlp_project'] ) ) { + list( $projectId ) = $this->addProjects( [ $entry['rlp_project'] ] ); + unset( $entry['rlp_project'] ); + $entry['rle_rlp_id'] = $projectId; + } $this->db->insert( 'reading_list_entry', $entry ); $entryId = $this->db->insertId(); $entryIds[] = $entryId; } return $entryIds; + } + + /** + * Creates reading_list_project rows from the given data. + * @param string[] $projects + * @return int[] Project IDs + */ + private function addProjects( array $projects ) { + $ids = []; + foreach ( $projects as $project ) { + $this->db->insert( + 'reading_list_project', + [ 'rlp_project' => $project ], + __METHOD__, + [ 'IGNORE' ] + ); + $projectId = $this->db->insertId(); + if ( !$projectId ) { + $projectId = $this->db->selectField( + 'reading_list_project', + 'rlp_id', + [ 'rlp_project' => $project ] + ); + } + $ids[] = $projectId; + } + return $ids; } private function addDataForAnotherUser() { @@ -1064,14 +1105,14 @@ 'rl_deleted' => 0, 'entries' => [ [ - 'rle_project' => 'foo', + 'rlp_project' => 'foo', 'rle_title' => 'bar', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), 'rle_deleted' => 0, ], [ - 'rle_project' => 'foo2', + 'rlp_project' => 'foo2', 'rle_title' => 'bar2', 'rle_date_created' => wfTimestampNow(), 'rle_date_updated' => wfTimestampNow(), -- To view, visit https://gerrit.wikimedia.org/r/392748 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I047d1493f1d9f51d733c1925a38c080829589f35 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/ReadingLists Gerrit-Branch: master Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
