Bene has uploaded a new change for review.
https://gerrit.wikimedia.org/r/234960
Change subject: Use a BadgeStore in SiteLinkTable to update badges
......................................................................
Use a BadgeStore in SiteLinkTable to update badges
This patch injects a BadgeStore into SiteLinkTable and reflects
all updates made on sitelinks also to the badges table.
Bug: T72229
Change-Id: I48eae6239671dbbc0111cf4695a48163705580a4
---
M client/includes/store/sql/DirectSqlStore.php
M lib/includes/store/sql/BadgeTable.php
M lib/includes/store/sql/SiteLinkTable.php
M lib/tests/phpunit/store/Sql/SiteLinkTableTest.php
M repo/includes/store/Store.php
M repo/includes/store/sql/ItemsPerSiteBuilder.php
M repo/includes/store/sql/SqlStore.php
M repo/maintenance/dispatchChanges.php
M repo/maintenance/rebuildItemsPerSite.php
M repo/tests/phpunit/includes/store/StoreTest.php
10 files changed, 171 insertions(+), 50 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/60/234960/1
diff --git a/client/includes/store/sql/DirectSqlStore.php
b/client/includes/store/sql/DirectSqlStore.php
index fdbdb71..4a8d18a 100644
--- a/client/includes/store/sql/DirectSqlStore.php
+++ b/client/includes/store/sql/DirectSqlStore.php
@@ -19,6 +19,7 @@
use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\Services\EntityId\EntityIdParser;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
+use Wikibase\Lib\Store\BadgeTable;
use Wikibase\Lib\Store\CachingEntityRevisionLookup;
use Wikibase\Lib\Store\CachingSiteLinkLookup;
use Wikibase\Lib\Store\EntityContentDataCodec;
@@ -278,8 +279,9 @@
*/
public function getSiteLinkLookup() {
if ( $this->siteLinkLookup === null ) {
+ $badgeTable = new BadgeTable( 'wb_badges_per_sitelink',
true );
$this->siteLinkLookup = new CachingSiteLinkLookup(
- new SiteLinkTable( 'wb_items_per_site', true,
$this->repoWiki ),
+ new SiteLinkTable( 'wb_items_per_site', true,
$badgeTable, $this->repoWiki ),
new HashBagOStuff()
);
}
diff --git a/lib/includes/store/sql/BadgeTable.php
b/lib/includes/store/sql/BadgeTable.php
index e4255f6..e4195c3 100644
--- a/lib/includes/store/sql/BadgeTable.php
+++ b/lib/includes/store/sql/BadgeTable.php
@@ -20,18 +20,14 @@
class BadgeTable extends DBAccessBase implements BadgeStore {
/**
- * @since 0.5
- *
* @var string
*/
- protected $table;
+ private $table;
/**
- * @since 0.5
- *
* @var bool
*/
- protected $readonly;
+ private $readonly;
/**
* @param string $table The table to use for the badges
diff --git a/lib/includes/store/sql/SiteLinkTable.php
b/lib/includes/store/sql/SiteLinkTable.php
index 8c1785e..49b1f8e 100644
--- a/lib/includes/store/sql/SiteLinkTable.php
+++ b/lib/includes/store/sql/SiteLinkTable.php
@@ -22,30 +22,32 @@
class SiteLinkTable extends DBAccessBase implements SiteLinkStore,
SiteLinkConflictLookup {
/**
- * @since 0.1
- *
* @var string
*/
- protected $table;
+ private $table;
/**
- * @since 0.3
- *
* @var bool
*/
- protected $readonly;
+ private $readonly;
+
+ /**
+ * @var BadgeStore
+ */
+ private $badgeStore;
/**
* @since 0.1
*
* @param string $table The table to use for the sitelinks
* @param bool $readonly Whether the table can be modified.
+ * @param BadgeStore $badgeStore A badge store to access badges
* @param string|bool $wiki The wiki's database to connect to.
* Must be a value LBFactory understands. Defaults to false,
which is the local wiki.
*
* @throws MWException
*/
- public function __construct( $table, $readonly, $wiki = false ) {
+ public function __construct( $table, $readonly, BadgeStore $badgeStore,
$wiki = false ) {
if ( !is_string( $table ) ) {
throw new MWException( '$table must be a string.' );
}
@@ -58,6 +60,7 @@
$this->table = $table;
$this->readonly = $readonly;
+ $this->badgeStore = $badgeStore;
$this->wiki = $wiki;
}
@@ -151,14 +154,30 @@
);
}
- $success = $dbw->insert(
+ $ok = $dbw->insert(
$this->table,
$insert,
__METHOD__,
array( 'IGNORE' )
);
- return $success && $dbw->affectedRows();
+ $ok = $ok && $dbw->affectedRows();
+
+ if ( !$ok ) {
+ return $ok;
+ }
+
+ return $this->saveBadges( $links );
+ }
+
+ private function saveBadges( array $links ) {
+ foreach ( $links as $siteLink ) {
+ if ( !$this->badgeStore->saveBadgesOfSiteLink(
$siteLink ) ) {
+ return false;
+ }
+ }
+
+ return true;
}
/**
@@ -183,7 +202,7 @@
$siteIds[] = $siteLink->getSiteId();
}
- $success = $dbw->delete(
+ $ok = $dbw->delete(
$this->table,
array(
'ips_item_id' => $item->getId()->getNumericId(),
@@ -192,7 +211,21 @@
__METHOD__
);
- return $success;
+ if ( !$ok ) {
+ return $ok;
+ }
+
+ return $this->deleteBadges( $links );
+ }
+
+ private function deleteBadges( array $links ) {
+ foreach ( $links as $siteLink ) {
+ if ( !$this->badgeStore->deleteBadgesOfSiteLink(
$siteLink ) ) {
+ return false;
+ }
+ }
+
+ return true;
}
/**
@@ -210,6 +243,8 @@
throw new MWException( 'Cannot write when in readonly
mode' );
}
+ $links = $this->getSiteLinksForItem( $itemId );
+
$dbw = $this->getConnection( DB_MASTER );
$ok = $dbw->delete(
@@ -220,7 +255,11 @@
$this->releaseConnection( $dbw );
- return $ok;
+ if ( !$ok ) {
+ return $ok;
+ }
+
+ return $this->deleteBadges( $links );
}
/**
@@ -351,7 +390,8 @@
$ok = $dbw->delete( $this->table, '*', __METHOD__ );
$this->releaseConnection( $dbw );
- return $ok;
+
+ return $this->badgeStore->clear();
}
/**
diff --git a/lib/tests/phpunit/store/Sql/SiteLinkTableTest.php
b/lib/tests/phpunit/store/Sql/SiteLinkTableTest.php
index 7113d55..318e1b9 100644
--- a/lib/tests/phpunit/store/Sql/SiteLinkTableTest.php
+++ b/lib/tests/phpunit/store/Sql/SiteLinkTableTest.php
@@ -4,6 +4,8 @@
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\SiteLink;
+use Wikibase\Lib\Store\BadgeStore;
use Wikibase\Lib\Store\SiteLinkTable;
/**
@@ -21,19 +23,49 @@
*/
class SiteLinkTableTest extends \MediaWikiTestCase {
- /**
- * @var SiteLinkTable
- */
- private $siteLinkTable;
-
protected function setUp() {
parent::setUp();
if ( !defined( 'WB_VERSION' ) ) {
$this->markTestSkipped( "Skipping because
WikibaseClient doesn't have a local site link table." );
}
+ }
- $this->siteLinkTable = new SiteLinkTable( 'wb_items_per_site',
false );
+ /**
+ * @param SiteLink[] $updatedLinks
+ * @param SiteLink[] $removedLinks
+ * @param bool $clear
+ * @return BadgeStore
+ */
+ private function newBadgeStore( array $updatedLinks, array
$removedLinks, $clear ) {
+ $badgeStore = $this->getMock( 'Wikibase\Lib\Store\BadgeStore' );
+
+ $i = 0;
+ foreach ( $removedLinks as $siteLink ) {
+ $badgeStore->expects( $this->at( $i++ ) )
+ ->method( 'deleteBadgesOfSiteLink' )
+ ->with( $siteLink )
+ ->will( $this->returnValue( true ) );
+ }
+
+ foreach ( $updatedLinks as $siteLink ) {
+ $badgeStore->expects( $this->at( $i++ ) )
+ ->method( 'saveBadgesOfSiteLink' )
+ ->with( $siteLink )
+ ->will( $this->returnValue( true ) );
+ }
+
+ if ( $clear ) {
+ $badgeStore->expects( $this->once() )
+ ->method( 'clear' )
+ ->will( $this->returnValue( true ) );
+ }
+
+ return $badgeStore;
+ }
+
+ private function newSiteLinkTable( array $updatedLinks = array(), array
$removedLinks = array(), $clear = false ) {
+ return new SiteLinkTable( 'wb_items_per_site', false,
$this->newBadgeStore( $updatedLinks, $removedLinks, $clear ) );
}
public function itemProvider() {
@@ -61,7 +93,8 @@
* @dataProvider itemProvider
*/
public function testSaveLinksOfItem( Item $item ) {
- $res = $this->siteLinkTable->saveLinksOfItem( $item );
+ $siteLinkTable = $this->newSiteLinkTable(
$item->getSiteLinkList()->toArray() );
+ $res = $siteLinkTable->saveLinksOfItem( $item );
$this->assertTrue( $res );
}
@@ -69,10 +102,11 @@
* @depends testSaveLinksOfItem
*/
public function testSaveLinksOfItem_duplicate() {
+ $siteLinkTable = $this->newSiteLinkTable();
$item = new Item( new ItemId( 'Q2' ) );
$item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Beer' );
- $res = $this->siteLinkTable->saveLinksOfItem( $item );
+ $res = $siteLinkTable->saveLinksOfItem( $item );
$this->assertFalse( $res );
}
@@ -83,22 +117,31 @@
$item->getSiteLinkList()->addNewSiteLink( 'dewiki', 'Bar' );
$item->getSiteLinkList()->addNewSiteLink( 'svwiki', 'Börk' );
- $this->siteLinkTable->saveLinksOfItem( $item );
+ $siteLinkTable = $this->newSiteLinkTable(
$item->getSiteLinkList()->toArray() );
+ $siteLinkTable->saveLinksOfItem( $item );
// modify links, and save again
- $item->getSiteLinkList()->removeLinkWithSiteId( 'enwiki' );
- $item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'FooK' );
+ $item->getSiteLinkList()->setNewSiteLink( 'enwiki', 'FooK' );
$item->getSiteLinkList()->removeLinkWithSiteId( 'dewiki' );
$item->getSiteLinkList()->addNewSiteLink( 'nlwiki', 'GrooK' );
- $this->siteLinkTable->saveLinksOfItem( $item );
+ $updated = array(
+ new SiteLink( 'enwiki', 'FooK' ),
+ new SiteLink( 'nlwiki', 'GrooK' )
+ );
+ $removed = array(
+ new SiteLink( 'enwiki', 'FooK' ),
+ new SiteLink( 'dewiki', 'Bar' )
+ );
+ $siteLinkTable = $this->newSiteLinkTable( $updated, $removed );
+ $siteLinkTable->saveLinksOfItem( $item );
// check that the update worked correctly
- $actualLinks = $this->siteLinkTable->getSiteLinksForItem(
$item->getId() );
+ $actualLinks = $siteLinkTable->getSiteLinksForItem(
$item->getId() );
$expectedLinks = $item->getSiteLinkList()->toArray();
- $missingLinks = array_udiff( $expectedLinks, $actualLinks,
array( $this->siteLinkTable, 'compareSiteLinks' ) );
- $extraLinks = array_udiff( $actualLinks, $expectedLinks, array(
$this->siteLinkTable, 'compareSiteLinks' ) );
+ $missingLinks = array_udiff( $expectedLinks, $actualLinks,
array( $siteLinkTable, 'compareSiteLinks' ) );
+ $extraLinks = array_udiff( $actualLinks, $expectedLinks, array(
$siteLinkTable, 'compareSiteLinks' ) );
$this->assertEmpty( $missingLinks, 'Missing links' );
$this->assertEmpty( $extraLinks, 'Extra links' );
@@ -109,7 +152,8 @@
* @dataProvider itemProvider
*/
public function testGetSiteLinksOfItem( Item $item ) {
- $siteLinks = $this->siteLinkTable->getSiteLinksForItem(
$item->getId() );
+ $siteLinkTable = $this->newSiteLinkTable();
+ $siteLinks = $siteLinkTable->getSiteLinksForItem(
$item->getId() );
$this->assertArrayEquals(
$item->getSiteLinkList()->toArray(),
@@ -122,10 +166,12 @@
* @dataProvider itemProvider
*/
public function testGetItemIdForSiteLink( Item $item ) {
+ $siteLinkTable = $this->newSiteLinkTable();
+
foreach ( $item->getSiteLinkList()->toArray() as $siteLink ) {
$this->assertEquals(
$item->getId(),
- $this->siteLinkTable->getItemIdForSiteLink(
$siteLink )
+ $siteLinkTable->getItemIdForSiteLink( $siteLink
)
);
}
}
@@ -135,10 +181,12 @@
* @dataProvider itemProvider
*/
public function testGetItemIdForLink( Item $item ) {
+ $siteLinkTable = $this->newSiteLinkTable();
+
foreach ( $item->getSiteLinkList()->toArray() as $siteLink ) {
$this->assertEquals(
$item->getId(),
- $this->siteLinkTable->getItemIdForLink(
$siteLink->getSiteId(), $siteLink->getPageName() )
+ $siteLinkTable->getItemIdForLink(
$siteLink->getSiteId(), $siteLink->getPageName() )
);
}
}
@@ -148,12 +196,14 @@
* @dataProvider itemProvider
*/
public function testDeleteLinksOfItem( Item $item ) {
+ $siteLinkTable = $this->newSiteLinkTable( array(),
$item->getSiteLinkList()->toArray() );
+
$this->assertTrue(
- $this->siteLinkTable->deleteLinksOfItem( $item->getId()
) !== false
+ $siteLinkTable->deleteLinksOfItem( $item->getId() ) !==
false
);
$this->assertEmpty(
- $this->siteLinkTable->getSiteLinksForItem(
$item->getId() )
+ $siteLinkTable->getSiteLinksForItem( $item->getId() )
);
}
@@ -162,12 +212,14 @@
* @dataProvider itemProvider
*/
public function testClear( Item $item ) {
+ $siteLinkTable = $this->newSiteLinkTable( array(), array(),
true );
+
$this->assertTrue(
- $this->siteLinkTable->clear()
+ $siteLinkTable->clear()
);
$this->assertEmpty(
- $this->siteLinkTable->getSiteLinksForItem(
$item->getId() )
+ $siteLinkTable->getSiteLinksForItem( $item->getId() )
);
}
diff --git a/repo/includes/store/Store.php b/repo/includes/store/Store.php
index 33f68a3..2face60 100644
--- a/repo/includes/store/Store.php
+++ b/repo/includes/store/Store.php
@@ -5,6 +5,7 @@
use Wikibase\DataModel\Services\Entity\EntityPrefetcher;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
use Wikibase\DataModel\Services\Lookup\EntityRedirectLookup;
+use Wikibase\Lib\Store\BadgeStore;
use Wikibase\Lib\Store\EntityInfoBuilderFactory;
use Wikibase\Lib\Store\EntityRevisionLookup;
use Wikibase\Lib\Store\EntityStore;
@@ -157,4 +158,11 @@
*/
public function getEntityPrefetcher();
+ /**
+ * @since 0.5
+ *
+ * @return BadgeStore
+ */
+ public function newBadgeStore();
+
}
diff --git a/repo/includes/store/sql/ItemsPerSiteBuilder.php
b/repo/includes/store/sql/ItemsPerSiteBuilder.php
index 9979864..d1ce859 100644
--- a/repo/includes/store/sql/ItemsPerSiteBuilder.php
+++ b/repo/includes/store/sql/ItemsPerSiteBuilder.php
@@ -10,7 +10,7 @@
use Wikibase\Repo\Store\EntityIdPager;
/**
- * Utility class for rebuilding the wb_items_per_site table.
+ * Utility class for rebuilding the wb_items_per_site and
wb_badges_per_sitelink table.
*
* @since 0.5
*
diff --git a/repo/includes/store/sql/SqlStore.php
b/repo/includes/store/sql/SqlStore.php
index 4998429..6af941e 100644
--- a/repo/includes/store/sql/SqlStore.php
+++ b/repo/includes/store/sql/SqlStore.php
@@ -13,6 +13,8 @@
use Wikibase\DataModel\Services\Lookup\EntityLookup;
use Wikibase\DataModel\Services\Lookup\EntityRedirectLookup;
use Wikibase\Lib\Reporting\ObservableMessageReporter;
+use Wikibase\Lib\Store\BadgeStore;
+use Wikibase\Lib\Store\BadgeTable;
use Wikibase\Lib\Store\CachingEntityRevisionLookup;
use Wikibase\Lib\Store\EntityContentDataCodec;
use Wikibase\Lib\Store\EntityInfoBuilderFactory;
@@ -315,7 +317,9 @@
$this->getUpdateScriptPath(
'AddBadgesPerSiteLink', $db->getType() )
);
- // TODO populate table
+ $updater->addPostDatabaseUpdateMaintenance(
+ 'Wikibase\Repo\Maintenance\RebuildItemsPerSite'
+ );
}
}
@@ -536,7 +540,7 @@
* @return SiteLinkStore
*/
public function newSiteLinkStore() {
- return new SiteLinkTable( 'wb_items_per_site', false );
+ return new SiteLinkTable( 'wb_items_per_site', false,
$this->newBadgeStore() );
}
/**
@@ -771,7 +775,7 @@
* @return SiteLinkConflictLookup
*/
public function getSiteLinkConflictLookup() {
- return new SiteLinkTable( 'wb_items_per_site', false );
+ return new SiteLinkTable( 'wb_items_per_site', false,
$this->newBadgeStore() );
}
/**
@@ -787,4 +791,11 @@
return $this->entityPrefetcher;
}
+ /**
+ * @return BadgeStore
+ */
+ public function newBadgeStore() {
+ return new BadgeTable( 'wb_badges_per_sitelink', false );
+ }
+
}
diff --git a/repo/maintenance/dispatchChanges.php
b/repo/maintenance/dispatchChanges.php
index 71b1316..300e6e0 100644
--- a/repo/maintenance/dispatchChanges.php
+++ b/repo/maintenance/dispatchChanges.php
@@ -7,6 +7,7 @@
use MWException;
use Wikibase\Lib\Reporting\ObservableMessageReporter;
use Wikibase\Lib\Reporting\ReportingExceptionHandler;
+use Wikibase\Lib\Store\BadgeTable;
use Wikibase\Lib\Store\SiteLinkTable;
use Wikibase\Repo\ChangeDispatcher;
use Wikibase\Repo\Notifications\JobQueueChangeNotificationSender;
@@ -229,7 +230,8 @@
|| $subscriptionLookupMode === 'subscriptions+sitelinks'
) {
$this->log( "Using sitelinks to target notifications."
);
- $siteLinkTable = new SiteLinkTable(
'wb_items_per_site', true, $repoDB );
+ $badgeTable = new BadgeTable( 'wb_badges_per_sitelink',
true );
+ $siteLinkTable = new SiteLinkTable(
'wb_items_per_site', true, $badgeTable, $repoDB );
$lookup = $siteLinkSubscriptionLookup = new
SiteLinkSubscriptionLookup( $siteLinkTable );
}
diff --git a/repo/maintenance/rebuildItemsPerSite.php
b/repo/maintenance/rebuildItemsPerSite.php
index 8773c71..0b244f1 100644
--- a/repo/maintenance/rebuildItemsPerSite.php
+++ b/repo/maintenance/rebuildItemsPerSite.php
@@ -4,6 +4,7 @@
use Maintenance;
use Wikibase\Lib\Reporting\ObservableMessageReporter;
+use Wikibase\Lib\Store\BadgeTable;
use Wikibase\Lib\Store\SiteLinkTable;
use Wikibase\Repo\Store\SQL\EntityPerPageIdPager;
use Wikibase\Repo\Store\SQL\ItemsPerSiteBuilder;
@@ -14,7 +15,7 @@
require_once $basePath . '/maintenance/Maintenance.php';
/**
- * Maintenance script for rebuilding the items_per_site table.
+ * Maintenance script for rebuilding the items_per_site and
badges_per_sitelink tables.
*
* @since 0.5
*
@@ -26,7 +27,7 @@
public function __construct() {
parent::__construct();
- $this->mDescription = 'Rebuild the items_per_site table';
+ $this->mDescription = 'Rebuild the items_per_site and
badges_per_sitelink tables';
$this->addOption( 'batch-size', "Number of rows to update per
batch (100 by default)", false, true );
}
@@ -47,7 +48,8 @@
array( $this, 'report' )
);
- $siteLinkTable = new SiteLinkTable( 'wb_items_per_site', false
);
+ $badgeTable = new BadgeTable( 'wb_badges_per_sitelink', false );
+ $siteLinkTable = new SiteLinkTable( 'wb_items_per_site', false,
$badgeTable );
// Use an uncached EntityLookup here to avoid memory leaks
$entityLookup =
WikibaseRepo::getDefaultInstance()->getEntityLookup( 'uncached' );
$entityPrefetcher =
WikibaseRepo::getDefaultInstance()->getStore()->getEntityPrefetcher();
diff --git a/repo/tests/phpunit/includes/store/StoreTest.php
b/repo/tests/phpunit/includes/store/StoreTest.php
index e46074c..001ade2 100644
--- a/repo/tests/phpunit/includes/store/StoreTest.php
+++ b/repo/tests/phpunit/includes/store/StoreTest.php
@@ -77,4 +77,12 @@
$this->assertInstanceOf( '\Wikibase\IdGenerator',
$store->newIdGenerator() );
}
+ /**
+ * @dataProvider instanceProvider
+ * @param Store $store
+ */
+ public function testNewBadgeStore( Store $store ) {
+ $this->assertInstanceOf( '\Wikibase\Lib\Store\BadgeStore',
$store->newBadgeStore() );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/234960
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I48eae6239671dbbc0111cf4695a48163705580a4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Bene <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
