jenkins-bot has submitted this change and it was merged. Change subject: Remove the subscriptionLookupMode setting and old subscription lookups ......................................................................
Remove the subscriptionLookupMode setting and old subscription lookups To simplify the code, as asked for by the bug. Setting is unused in Wikimedia production. Manually tested. Bug: T118265 Change-Id: I3aebfbe3ec371042422bd163974f185dbf94604e --- M docs/options.wiki M docs/usagetracking-migration.wiki M repo/config/Wikibase.default.php D repo/includes/store/DualSubscriptionLookup.php D repo/includes/store/SiteLinkSubscriptionLookup.php M repo/includes/store/sql/ChangesSubscriptionSchemaUpdater.php M repo/maintenance/dispatchChanges.php D repo/tests/phpunit/includes/store/DualSubscriptionLookupTest.php D repo/tests/phpunit/includes/store/SiteLinkSubscriptionLookupTest.php M repo/tests/phpunit/includes/store/sql/ChangesSubscriptionTableBuilderTest.php 10 files changed, 2 insertions(+), 449 deletions(-) Approvals: Aude: Looks good to me, approved jenkins-bot: Verified diff --git a/docs/options.wiki b/docs/options.wiki index 0ab011a..aacf5d6 100644 --- a/docs/options.wiki +++ b/docs/options.wiki @@ -60,11 +60,6 @@ ;urlSchemes: Which URL schemes should be allowed in URL data values. The default is array( 'http', 'https' ). Other supported schemes are 'ftp' and 'mailto'. Schemes (protocols) added here will only have any effect if validation is supported for that protocol; that is, adding 'mailto' will work, while adding 'gopher' will do nothing. ;formatterUrlProperty: Property to be used on properties that defines a formatter URL which is used to link identifiers. The placeholder <code>$1</code> will be replaced by the identifier. Example: <code>https://www.wikidata.org/entity/$1</code> ;transformLegacyFormatOnExport: Whether entity revisions stored in a legacy format should be converted on the fly while exporting. Enabled per default. -;subscriptionLookupMode: how the repo determines which clients need to be notified about changes to which entity. Possible values: -:;<code>'sitelinks'</code>: Use only sitelinks from the wb_items_per_site table to track subscriptions (legacy mode). -:;<code>'subscriptions'</code>: Use explicit subscription tracking via the wb_changes_subscription table. -:;<code>'subscriptions+sitelinks'</code>: Use information from both tables (compatibility mode). -: Default is <code>'subscriptions'</code>. ;allowEntityImport: allow importing entities via Special:Import and importDump.php. Per default, imports are forbidden, since entities defined in another wiki would have or use IDs that conflict with entities defined locally. == Client Settings == diff --git a/docs/usagetracking-migration.wiki b/docs/usagetracking-migration.wiki index 3030aed..1e1aaf5 100644 --- a/docs/usagetracking-migration.wiki +++ b/docs/usagetracking-migration.wiki @@ -23,18 +23,12 @@ == Create subscription table on the repo == To set up the subscription tracking table wb_changes_subscription on the repo: -* Deploy the schema change and set subscriptionLookupMode = 'subscriptions+sitelinks'. -** If subscriptionLookupMode is 'subscriptions' or 'subscriptions+sitelinks', update.php will create the table automatically. -** If subscriptionLookupMode is 'sitelinks', apply repo/sql/changes_subscription.sql manually. +* Deploy the schema change by running update.php. * Run repo/maintenance/populateChangesSubscription.php to initialize the wb_changes_subscription table based on repowiki.wb_items_per_site. NOTE: If any clients already have usage tracking enabled, then updateSubscriptions.php can also be run and subscription tracking enabled for each of them at this point. See the instructions in section ''Start tracking client subscriptions based on entity usage'' below. - -NOTE: Once ''all'' clients have usage tracking ''and'' subscription tracking enabled (see -instructions below), subscriptionLookupMode should be set to 'subscriptions'. As long as some -clients do not have subscription tracking enabled, use subscriptionLookupMode = 'subscriptions+sitelinks'. == Start tracking entity usage on the client wikis == @@ -43,7 +37,6 @@ * Deploy the schema change by running update.php. * Run client/maintenance/populateEntityUsage.php to initialize the wbc_entity_usage table based on repowiki.wb_items_per_site. * If desired, enable arbitrary access by setting allowArbitraryDataAccess = true -** Caveat: as long as subscriptionLookupMode is 'sitelinks' on the repo, pages using items that are not connected to any page on the local wiki via a sitelink will not get purged automatically when the item is edited. == Start tracking client subscriptions based on entity usage == @@ -54,7 +47,6 @@ * Make sure the repo has the wb_changes_subscription table set up, see above. * Run client/maintenance/updateSubscriptions.php to put entries into repowiki.wb_changes_subscription based on the client wiki's wbc_entity_usage table. * If desired, enable arbitrary access by setting allowArbitraryDataAccess = true -** Caveat: as long as subscriptionLookupMode is 'sitelinks' on the repo, pages using items that are not connected to any page on the local wiki via a sitelink will not get purged automatically when the item is edited. == Start using subscriptions for dispatching == diff --git a/repo/config/Wikibase.default.php b/repo/config/Wikibase.default.php index 477b23c..a6d522a 100644 --- a/repo/config/Wikibase.default.php +++ b/repo/config/Wikibase.default.php @@ -94,16 +94,6 @@ // Property used as formatter to link identifiers 'formatterUrlProperty' => null, - /** - * Determines how subscription lookup is handled. Possible values: - * - * - 'sitelinks': Use only sitelinks to determine which wiki is subscribed to which entity. - * Use this mode if the wb_changes_subscription table does not exist. - * - 'subscriptions': use explicit subscriptions in the wb_changes_subscription table. - * - 'subscriptions+sitelinks': use a combination of both. - */ - 'subscriptionLookupMode' => 'subscriptions', - 'allowEntityImport' => false, /** diff --git a/repo/includes/store/DualSubscriptionLookup.php b/repo/includes/store/DualSubscriptionLookup.php deleted file mode 100644 index 080244e..0000000 --- a/repo/includes/store/DualSubscriptionLookup.php +++ /dev/null @@ -1,89 +0,0 @@ -<?php - -namespace Wikibase\Store; - -use Wikibase\DataModel\Entity\EntityId; -use Wikibase\DataModel\Entity\ItemId; - -/** - * Implementation of SubscriptionLookup based on two other SubscriptionLookup. - * This provides a unified view on two subscription mechanisms. - * - * @since 0.5 - * - * @licence GNU GPL v2+ - * @author Daniel Kinzler - */ -class DualSubscriptionLookup implements SubscriptionLookup { - - /** - * @var SubscriptionLookup - */ - private $primary; - - /** - * @var SubscriptionLookup - */ - private $secondary; - - /** - * @param SubscriptionLookup $primary - * @param SubscriptionLookup $secondary - */ - public function __construct( SubscriptionLookup $primary, SubscriptionLookup $secondary ) { - $this->primary = $primary; - $this->secondary = $secondary; - } - - /** - * Returns a list of entities a given site is subscribed to. - * - * This implementations combines the results of queries to both of the lookups - * provided to the constructor. - * - * @param string $siteId Site ID of the client site. - * @param EntityId[]|null $entityIds The entities we are interested in, or null for "any". - * - * @return EntityId[] a list of entity IDs the client wiki is subscribed to. - * The result is limited to entity ids also present in $entityIds, if given. - */ - public function getSubscriptions( $siteId, array $entityIds ) { - if ( empty( $entityIds ) ) { - return array(); - } - - $primarySubscriptions = $this->reIndexEntityIds( - $this->primary->getSubscriptions( $siteId, $entityIds ) - ); - - $entityIds = array_diff( $this->reIndexEntityIds( $entityIds ), $primarySubscriptions ); - - if ( empty( $entityIds ) ) { - return $primarySubscriptions; - } - - $secondarySubscriptions = $this->reIndexEntityIds( - $this->secondary->getSubscriptions( $siteId, $entityIds ) - ); - - $subscriptions = array_merge( $primarySubscriptions, $secondarySubscriptions ); - return $subscriptions; - } - - /** - * @param EntityId[] $entityIds - * - * @return ItemId[] The ItemIds from EntityId[], keyed by numeric id. - */ - private function reIndexEntityIds( array $entityIds ) { - $reindexed = array(); - - foreach ( $entityIds as $id ) { - $key = $id->getSerialization(); - $reindexed[$key] = $id; - } - - return $reindexed; - } - -} diff --git a/repo/includes/store/SiteLinkSubscriptionLookup.php b/repo/includes/store/SiteLinkSubscriptionLookup.php deleted file mode 100644 index 6fdcf8e..0000000 --- a/repo/includes/store/SiteLinkSubscriptionLookup.php +++ /dev/null @@ -1,80 +0,0 @@ -<?php - -namespace Wikibase\Store; - -use Wikibase\DataModel\Entity\EntityId; -use Wikibase\DataModel\Entity\ItemId; -use Wikibase\Lib\Store\SiteLinkLookup; - -/** - * Implementation of SubscriptionLookup based on a SiteLinkStore. - * - * @since 0.5 - * - * @licence GNU GPL v2+ - * @author Daniel Kinzler - */ -class SiteLinkSubscriptionLookup implements SubscriptionLookup { - - /** - * @var SiteLinkLookup - */ - private $siteLinkLookup; - - /** - * @param SiteLinkLookup $siteLinkLookup - */ - public function __construct( SiteLinkLookup $siteLinkLookup ) { - $this->siteLinkLookup = $siteLinkLookup; - } - - /** - * Returns a list of entities a given site is subscribed to. - * - * @param string $siteId Site ID of the client site. - * @param EntityId[]|null $entityIds The entities we are interested in, or null for "any". - * - * @return EntityId[] a list of entity IDs the client wiki is subscribed to. - * The result is limited to entity ids also present in $entityIds, if given. - */ - public function getSubscriptions( $siteId, array $entityIds ) { - // NOTE: non-Item ids are ignored, since only items can be subscribed to - // via sitelinks. - $entityIds = $this->getItemIds( $entityIds ); - $numericIds = array_keys( $entityIds ); - - if ( empty( $numericIds ) ) { - return array(); - } - - $links = $this->siteLinkLookup->getLinks( $numericIds, array( $siteId ) ); - - // collect the item IDs present in these links - $linkedItems = array(); - foreach ( $links as $link ) { - list( ,, $id ) = $link; - $linkedItems[$id] = $entityIds[$id]; - } - - return $linkedItems; - } - - /** - * @param EntityId[] $entityIds - * - * @return ItemId[] The ItemIds from EntityId[], keyed by numeric id. - */ - private function getItemIds( array $entityIds ) { - $reindexed = array(); - - foreach ( $entityIds as $id ) { - if ( $id instanceof ItemId ) { - $key = $id->getNumericId(); - $reindexed[$key] = $id; - } - } - - return $reindexed; - } - -} diff --git a/repo/includes/store/sql/ChangesSubscriptionSchemaUpdater.php b/repo/includes/store/sql/ChangesSubscriptionSchemaUpdater.php index 3b235e2..89d70fe 100644 --- a/repo/includes/store/sql/ChangesSubscriptionSchemaUpdater.php +++ b/repo/includes/store/sql/ChangesSubscriptionSchemaUpdater.php @@ -35,13 +35,6 @@ * @return bool */ public static function onSchemaUpdate( DatabaseUpdater $dbUpdater ) { - $mode = WikibaseRepo::getDefaultInstance()->getSettings()->getSetting( 'subscriptionLookupMode' ); - - if ( $mode !== 'subscriptions' && $mode !== 'subscriptions+sitelinks' ) { - // Use of the wb_changes_subscription table is disabled. - return true; - } - $changesSubscriptionSchemaUpdater = new self( $dbUpdater ); $changesSubscriptionSchemaUpdater->doSchemaUpdate(); diff --git a/repo/maintenance/dispatchChanges.php b/repo/maintenance/dispatchChanges.php index 1fcf394..c014c66 100644 --- a/repo/maintenance/dispatchChanges.php +++ b/repo/maintenance/dispatchChanges.php @@ -8,15 +8,11 @@ use Wikibase\Lib\Reporting\ObservableMessageReporter; use Wikibase\Lib\Reporting\ReportingExceptionHandler; use Wikibase\Lib\Store\ChangeLookup; -use Wikibase\Lib\Store\SiteLinkTable; use Wikibase\Repo\ChangeDispatcher; use Wikibase\Repo\Notifications\JobQueueChangeNotificationSender; use Wikibase\Repo\WikibaseRepo; -use Wikibase\Store\DualSubscriptionLookup; -use Wikibase\Store\SiteLinkSubscriptionLookup; use Wikibase\Store\Sql\SqlChangeDispatchCoordinator; use Wikibase\Store\Sql\SqlSubscriptionLookup; -use Wikibase\Store\SubscriptionLookup; $basePath = getenv( 'MW_INSTALL_PATH' ) !== false ? getenv( 'MW_INSTALL_PATH' ) : __DIR__ . '/../../../..'; @@ -77,7 +73,6 @@ $clientWikis = $settings->getSetting( 'localClientDatabases' ); $batchChunkFactor = $settings->getSetting( 'dispatchBatchChunkFactor' ); $batchCacheFactor = $settings->getSetting( 'dispatchBatchCacheFactor' ); - $subscriptionLookupMode = $settings->getSetting( 'subscriptionLookupMode' ); $batchSize = (int)$this->getOption( 'batch-size', 1000 ); $maxChunks = (int)$this->getOption( 'max-chunks', 15 ); @@ -120,7 +115,7 @@ $coordinator->setRandomness( $randomness ); $notificationSender = new JobQueueChangeNotificationSender( $repoDB, $clientWikis ); - $subscriptionLookup = $this->getSubscriptionLookup( $repoDB, $subscriptionLookupMode ); + $subscriptionLookup = new SqlSubscriptionLookup( wfGetLB() ); $dispatcher = new ChangeDispatcher( $coordinator, @@ -215,39 +210,6 @@ } $this->log( "Done, exiting after $c passes and $t seconds." ); - } - - /** - * @param string|bool $repoDB - * @param string $subscriptionLookupMode - * - * @return SubscriptionLookup - */ - private function getSubscriptionLookup( $repoDB, $subscriptionLookupMode ) { - $lookup = null; - $siteLinkSubscriptionLookup = null; - $sqlSubscriptionLookup = null; - - if ( $subscriptionLookupMode === 'sitelinks' - || $subscriptionLookupMode === 'subscriptions+sitelinks' - ) { - $this->log( "Using sitelinks to target notifications." ); - $siteLinkTable = new SiteLinkTable( 'wb_items_per_site', true, $repoDB ); - $lookup = $siteLinkSubscriptionLookup = new SiteLinkSubscriptionLookup( $siteLinkTable ); - } - - if ( $subscriptionLookupMode === 'subscriptions' - || $subscriptionLookupMode === 'subscriptions+sitelinks' - ) { - $this->log( "Using subscriptions to target notifications." ); - $lookup = $sqlSubscriptionLookup = new SqlSubscriptionLookup( wfGetLB() ); - } - - if ( $siteLinkSubscriptionLookup && $sqlSubscriptionLookup ) { - $lookup = new DualSubscriptionLookup( $siteLinkSubscriptionLookup, $sqlSubscriptionLookup ); - } - - return $lookup; } /** diff --git a/repo/tests/phpunit/includes/store/DualSubscriptionLookupTest.php b/repo/tests/phpunit/includes/store/DualSubscriptionLookupTest.php deleted file mode 100644 index 40e4eae..0000000 --- a/repo/tests/phpunit/includes/store/DualSubscriptionLookupTest.php +++ /dev/null @@ -1,110 +0,0 @@ -<?php - -namespace Wikibase\Test; - -use Wikibase\DataModel\Entity\EntityId; -use Wikibase\DataModel\Entity\ItemId; -use Wikibase\DataModel\Entity\PropertyId; -use Wikibase\Store\DualSubscriptionLookup; -use Wikibase\Store\SubscriptionLookup; - -/** - * @covers Wikibase\Store\DualSubscriptionLookup - * - * @group Wikibase - * @group WikibaseStore - * @group WikibaseChange - * @group WikibaseRepo - * - * @licence GNU GPL v2+ - * @author Daniel Kinzler - */ -class DualSubscriptionLookupTest extends \PHPUnit_Framework_TestCase { - - /** - * @param EntityId[] $subscriptions - * - * @return SubscriptionLookup - */ - private function getSubscriptionLookup( array $subscriptions ) { - $lookup = $this->getMock( 'Wikibase\Store\SubscriptionLookup' ); - - $lookup->expects( $this->any() ) - ->method( 'getSubscriptions' ) - ->will( $this->returnCallback( function ( $siteId, array $ids ) use ( $subscriptions ) { - return array_intersect( $subscriptions, $ids ); - } ) ); - - return $lookup; - } - - public function provideGetSubscriptions() { - $p1 = new PropertyId( 'P1' ); - $p3 = new PropertyId( 'P3' ); - $q2 = new ItemId( 'Q2' ); - $q7 = new ItemId( 'Q7' ); - - return array( - 'empty' => array( - array(), - array(), - array(), - array(), - ), - 'empty request' => array( - array( $p1 ), - array( $q7 ), - array(), - array(), - ), - 'empty lookups' => array( - array(), - array(), - array( $p1, $q2, $q7, $p3 ), - array(), - ), - 'partial' => array( - array( $p1 ), - array( $q7 ), - array( $p1, $q2, $q7, $p3 ), - array( $p1, $q7 ), - ), - 'primary covers' => array( - array( $p1 ), - array( $q7 ), - array( $p1 ), - array( $p1 ), - ), - 'secondary covers' => array( - array( $p1 ), - array( $q7 ), - array( $q7 ), - array( $q7 ), - ), - ); - } - - /** - * @dataProvider provideGetSubscriptions - */ - public function testGetSubscriptions( $primary, $secondary, $requested, $expected ) { - $primary = $this->getSubscriptionLookup( $primary ); - $secondary = $this->getSubscriptionLookup( $secondary ); - - $lookup = new DualSubscriptionLookup( $primary, $secondary ); - - $subscriptions = $lookup->getSubscriptions( 'enwiki', $requested ); - - $this->assertEquals( $this->getIdStrings( $expected ), $this->getIdStrings( $subscriptions ) ); - } - - private function getIdStrings( array $ids ) { - $strings = array_map( function ( EntityId $id ) { - return $id->getSerialization(); - }, $ids ); - - sort( $strings ); - return $strings; - } - -} diff --git a/repo/tests/phpunit/includes/store/SiteLinkSubscriptionLookupTest.php b/repo/tests/phpunit/includes/store/SiteLinkSubscriptionLookupTest.php deleted file mode 100644 index e8d60b6..0000000 --- a/repo/tests/phpunit/includes/store/SiteLinkSubscriptionLookupTest.php +++ /dev/null @@ -1,93 +0,0 @@ -<?php - -namespace Wikibase\Test; - -use Wikibase\DataModel\Entity\EntityId; -use Wikibase\DataModel\Entity\ItemId; -use Wikibase\DataModel\Entity\PropertyId; -use Wikibase\Lib\Store\SiteLinkLookup; -use Wikibase\Store\SiteLinkSubscriptionLookup; - -/** - * @covers Wikibase\Store\SiteLinkSubscriptionLookup - * - * @group Wikibase - * @group WikibaseStore - * @group WikibaseChange - * @group WikibaseRepo - * - * @licence GNU GPL v2+ - * @author Daniel Kinzler - */ -class SiteLinkSubscriptionLookupTest extends \PHPUnit_Framework_TestCase { - - /** - * @return SiteLinkLookup - */ - private function getSiteLinkLookup() { - $lookup = $this->getMock( 'Wikibase\Lib\Store\SiteLinkLookup' ); - - $lookup->expects( $this->any() ) - ->method( 'getLinks' ) - ->will( $this->returnCallback( array( $this, 'getLinks' ) ) ); - - return $lookup; - } - - public function getLinks( array $numericIds = array(), array $siteIds = array(), array $pageNames = array() ) { - $links = array( - array( 'enwiki', 'Foo', 1 ), - array( 'enwiki', 'Boo', 3 ), - array( 'enwiki', 'Queue', 7 ), - array( 'dewiki', 'Fuh', 2 ), - ); - - return array_filter( $links, function( $link ) use ( $numericIds, $siteIds, $pageNames ) { - if ( !empty( $numericIds ) && !in_array( $link[2], $numericIds ) ) { - return false; - } - - if ( !empty( $siteIds ) && !in_array( $link[0], $siteIds ) ) { - return false; - } - - if ( !empty( $pageNames ) && !in_array( $link[1], $pageNames ) ) { - return false; - } - - return true; - } ); - } - - public function testGetSubscriptions() { - $lookup = new SiteLinkSubscriptionLookup( $this->getSiteLinkLookup() ); - - $subscriptions = $lookup->getSubscriptions( 'enwiki', array( - new ItemId( 'Q1' ), - new ItemId( 'Q2' ), - new ItemId( 'Q7' ), - new PropertyId( 'P3' ), - ) ); - - $actual = array_map( function ( EntityId $id ) { - return $id->getSerialization(); - }, $subscriptions ); - - sort( $actual ); - - $expected = array( 'Q1', 'Q7' ); - - $this->assertEquals( $expected, $actual ); - } - - public function testGetSubscriptions_none() { - $lookup = new SiteLinkSubscriptionLookup( $this->getSiteLinkLookup() ); - - $subscriptions = $lookup->getSubscriptions( 'enwiki', array( - new PropertyId( 'P3' ), // will be skipped - ) ); - - $this->assertEmpty( $subscriptions ); - } - -} diff --git a/repo/tests/phpunit/includes/store/sql/ChangesSubscriptionTableBuilderTest.php b/repo/tests/phpunit/includes/store/sql/ChangesSubscriptionTableBuilderTest.php index 82708c8..f2cd9e6 100644 --- a/repo/tests/phpunit/includes/store/sql/ChangesSubscriptionTableBuilderTest.php +++ b/repo/tests/phpunit/includes/store/sql/ChangesSubscriptionTableBuilderTest.php @@ -25,13 +25,6 @@ const TABLE_NAME = 'wb_changes_subscription'; protected function setUp() { - $mode = WikibaseRepo::getDefaultInstance()->getSettings()->getSetting( 'subscriptionLookupMode' ); - - if ( $mode !== 'subscriptions' && $mode !== 'subscriptions+sitelinks' ) { - $this->markTestSkipped( 'Skipping test for ChangesSubscriptionTableBuilder, ' - . 'because usage of the wb_changes_subscription table is disabled.' ); - } - $this->tablesUsed[] = self::TABLE_NAME; $this->tablesUsed[] = 'wb_items_per_site'; -- To view, visit https://gerrit.wikimedia.org/r/261661 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3aebfbe3ec371042422bd163974f185dbf94604e Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Hoo man <[email protected]> Gerrit-Reviewer: Aude <[email protected]> Gerrit-Reviewer: Daniel Kinzler <[email protected]> Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
