Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/176356
Change subject: [WIP] Clean up UsageTablePrimer and related
......................................................................
[WIP] Clean up UsageTablePrimer and related
Two problems to be discussed:
1. Why must the addExtensionUpdate be static?
2. Why is the table name passed around as a variable in so many odd
places?
Change-Id: Icf85ad8ed940a95ba741f2640d06ab506d563dd4
---
M client/includes/Usage/Sql/SqlUsageTracker.php
R client/includes/Usage/Sql/SqlUsageTrackerSchemaUpdater.php
M client/includes/Usage/Sql/UsageTablePrimer.php
M client/includes/Usage/Sql/UsageTableUpdater.php
M client/includes/Usage/UsageTracker.php
M client/tests/phpunit/includes/Usage/EntityUsageTest.php
M client/tests/phpunit/includes/Usage/HashUsageAccumulatorTest.php
M client/tests/phpunit/includes/Usage/ParserOutputUsageAccumulatorTest.php
M client/tests/phpunit/includes/Usage/Sql/SqlUsageTrackerTest.php
M client/tests/phpunit/includes/Usage/Sql/UsageTablePrimerTest.php
M client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php
M client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php
M client/tests/phpunit/includes/Usage/UsageLookupContractTester.php
M client/tests/phpunit/includes/Usage/UsageTrackerContractTester.php
14 files changed, 55 insertions(+), 70 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/56/176356/1
diff --git a/client/includes/Usage/Sql/SqlUsageTracker.php
b/client/includes/Usage/Sql/SqlUsageTracker.php
index b29c94b..b907c77 100644
--- a/client/includes/Usage/Sql/SqlUsageTracker.php
+++ b/client/includes/Usage/Sql/SqlUsageTracker.php
@@ -54,7 +54,7 @@
* @return UsageTableUpdater
*/
private function newTableUpdater( DatabaseBase $db ) {
- return new UsageTableUpdater( $db, 'wbc_entity_usage',
$this->batchSize );
+ return new UsageTableUpdater( $db, $this->batchSize );
}
/**
@@ -187,7 +187,7 @@
}
$res = $db->select(
- 'wbc_entity_usage',
+ UsageTracker::TABLE_NAME,
array( 'eu_aspect', 'eu_entity_id' ),
array( 'eu_page_id' => (int)$pageId ),
__METHOD__
@@ -239,7 +239,7 @@
$db = $this->connectionManager->getReadConnection();
$res = $db->select(
- 'wbc_entity_usage',
+ UsageTracker::TABLE_NAME,
array( 'DISTINCT eu_page_id' ),
$where,
__METHOD__
@@ -291,7 +291,7 @@
$db = $this->connectionManager->getReadConnection();
$res = $db->select(
- 'wbc_entity_usage',
+ UsageTracker::TABLE_NAME,
array( 'eu_entity_id' ),
$where,
__METHOD__
diff --git a/client/includes/Usage/Sql/SqlUsageTrackerSchemaUpdate.php
b/client/includes/Usage/Sql/SqlUsageTrackerSchemaUpdater.php
similarity index 71%
rename from client/includes/Usage/Sql/SqlUsageTrackerSchemaUpdate.php
rename to client/includes/Usage/Sql/SqlUsageTrackerSchemaUpdater.php
index 79fe713..662581a 100644
--- a/client/includes/Usage/Sql/SqlUsageTrackerSchemaUpdate.php
+++ b/client/includes/Usage/Sql/SqlUsageTrackerSchemaUpdater.php
@@ -3,6 +3,8 @@
namespace Wikibase\Client\Usage\Sql;
use DatabaseUpdater;
+use MWException;
+use Wikibase\Client\Usage\UsageTracker;
use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\Entity\EntityIdParser;
@@ -48,36 +50,27 @@
* Applies any schema updates
*/
public function doSchemaUpdate() {
- $table = 'wbc_entity_usage';
-
- if ( !$this->dbUpdater->tableExists( $table ) ) {
+ if ( !$this->dbUpdater->tableExists( UsageTracker::TABLE_NAME )
) {
$db = $this->dbUpdater->getDB();
$script = $this->getUpdateScriptPath( 'entity_usage',
$db->getType() );
- $this->dbUpdater->addExtensionTable( $table, $script );
+ $this->dbUpdater->addExtensionTable(
UsageTracker::TABLE_NAME, $script );
// Register function for populating the table.
- // Note that this must be done with a static function,
- // for reasons that do not need explaining at this
juncture.
- $this->dbUpdater->addExtensionUpdate( array(
- array( __CLASS__, 'fillUsageTable' ),
- $table
- ) );
+ $this->dbUpdater->addExtensionUpdate( array( array(
$this, 'fillUsageTable' ) ) );
}
}
/**
- * Static wrapper for UsageTablePrimer::fillUsageTable
+ * Static wrapper for SqlUsageTrackerSchemaUpdater::doSchemaUpdate
*
- * @param DatabaseUpdater $dbUpdater
- * @param string $table
+ * @param DatabaseUpdater $dbUpdater Unused.
*/
- public static function fillUsageTable( DatabaseUpdater $dbUpdater,
$table ) {
+ public function fillUsageTable( DatabaseUpdater $dbUpdater ) {
$idParser =
WikibaseClient::getDefaultInstance()->getEntityIdParser();
$primer = new UsageTablePrimer(
$idParser,
wfGetLB(), // would be nice to pass in
$dbUpdater->getDB().
- $table,
1000
);
@@ -98,7 +91,7 @@
}
}
- throw new \MWException( "Could not find schema update script
'$name'." );
+ throw new MWException( "Could not find schema update script
'$name'." );
}
}
diff --git a/client/includes/Usage/Sql/UsageTablePrimer.php
b/client/includes/Usage/Sql/UsageTablePrimer.php
index a41d584..5d1dd31 100644
--- a/client/includes/Usage/Sql/UsageTablePrimer.php
+++ b/client/includes/Usage/Sql/UsageTablePrimer.php
@@ -8,6 +8,7 @@
use LoadBalancer;
use ResultWrapper;
use Wikibase\Client\Usage\EntityUsage;
+use Wikibase\Client\Usage\UsageTracker;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\Lib\Reporting\ExceptionHandler;
@@ -38,11 +39,6 @@
private $loadBalancer;
/**
- * @var string
- */
- private $usageTableName;
-
- /**
* @var int
*/
private $batchSize;
@@ -60,23 +56,17 @@
/**
* @param EntityIdParser $idParser
* @param LoadBalancer $loadBalancer
- * @param string $usageTableName
* @param int $batchSize
*
* @throws InvalidArgumentException
*/
- public function __construct( EntityIdParser $idParser, LoadBalancer
$loadBalancer, $usageTableName, $batchSize = 1000 ) {
- if ( !is_string( $usageTableName ) ) {
- throw new InvalidArgumentException( '$usageTableName
must be a string' );
- }
-
+ public function __construct( EntityIdParser $idParser, LoadBalancer
$loadBalancer, $batchSize = 1000 ) {
if ( !is_int( $batchSize ) || $batchSize < 1 ) {
throw new InvalidArgumentException( '$batchSize must be
an integer >= 1' );
}
$this->idParser = $idParser;
$this->loadBalancer = $loadBalancer;
- $this->usageTableName = $usageTableName;
$this->batchSize = $batchSize;
$this->exceptionHandler = new LogWarningExceptionHandler();
@@ -86,7 +76,7 @@
/**
* @param MessageReporter $progressReporter
*/
- public function setProgressReporter( $progressReporter ) {
+ public function setProgressReporter( MessageReporter $progressReporter
) {
$this->progressReporter = $progressReporter;
}
@@ -100,7 +90,7 @@
/**
* @param ExceptionHandler $exceptionHandler
*/
- public function setExceptionHandler( $exceptionHandler ) {
+ public function setExceptionHandler( ExceptionHandler $exceptionHandler
) {
$this->exceptionHandler = $exceptionHandler;
}
@@ -159,7 +149,7 @@
$c = 0;
foreach ( $entityPerPage as $pageId => $entityId ) {
$db->insert(
- $this->usageTableName,
+ UsageTracker::TABLE_NAME,
array(
'eu_page_id' => (int)$pageId,
'eu_aspect' => EntityUsage::ALL_USAGE,
@@ -219,10 +209,10 @@
$this->exceptionHandler->handleException(
$ex,
'badEntityId',
- __METHOD__ .': ' .
- 'Failed to parse entity ID: ' .
+ __METHOD__ . ': ' . 'Failed to parse
entity ID: ' .
$row->pp_value . ' at page ' .
- $row->pp_page );
+ $row->pp_page
+ );
}
}
diff --git a/client/includes/Usage/Sql/UsageTableUpdater.php
b/client/includes/Usage/Sql/UsageTableUpdater.php
index 3b8185a..5e63ae4 100644
--- a/client/includes/Usage/Sql/UsageTableUpdater.php
+++ b/client/includes/Usage/Sql/UsageTableUpdater.php
@@ -5,6 +5,7 @@
use DatabaseBase;
use InvalidArgumentException;
use Wikibase\Client\Usage\EntityUsage;
+use Wikibase\Client\Usage\UsageTracker;
/**
* Helper class for updating the wb_entity_usage table.
@@ -21,33 +22,22 @@
private $connection;
/**
- * @var string
- */
- private $tableName;
-
- /**
* @var int
*/
private $batchSize;
/**
* @param DatabaseBase $connection
- * @param string $tableName
* @param int $batchSize
*
- * @throws \InvalidArgumentException
+ * @throws InvalidArgumentException
*/
- public function __construct( DatabaseBase $connection, $tableName,
$batchSize ) {
- if ( !is_string( $tableName ) ) {
- throw new InvalidArgumentException( '$tableName must be
a string' );
- }
-
+ public function __construct( DatabaseBase $connection, $batchSize ) {
if ( !is_int( $batchSize ) || $batchSize < 1 ) {
throw new InvalidArgumentException( '$batchSize must be
an integer >= 1' );
}
$this->connection = $connection;
- $this->tableName = $tableName;
$this->batchSize = $batchSize;
}
@@ -187,7 +177,7 @@
foreach ( $batches as $batch ) {
$this->connection->delete(
- $this->tableName,
+ UsageTracker::TABLE_NAME,
array(
'eu_page_id' => (int)$pageId,
'eu_aspect' => (string)$aspect,
@@ -222,7 +212,7 @@
foreach ( $batches as $rows ) {
$this->connection->insert(
- $this->tableName,
+ UsageTracker::TABLE_NAME,
$rows,
__METHOD__
);
@@ -248,7 +238,7 @@
foreach ( $batches as $batch ) {
$this->connection->delete(
- $this->tableName,
+ UsageTracker::TABLE_NAME,
array(
'eu_entity_id' => $batch,
),
diff --git a/client/includes/Usage/UsageTracker.php
b/client/includes/Usage/UsageTracker.php
index aa38d7a..01db17e 100644
--- a/client/includes/Usage/UsageTracker.php
+++ b/client/includes/Usage/UsageTracker.php
@@ -14,6 +14,8 @@
*/
interface UsageTracker {
+ const TABLE_NAME = 'wbc_entity_usage';
+
/**
* Updates entity usage information for the given page.
*
@@ -22,8 +24,8 @@
*
* See docs/usagetracking.wiki for details.
*
- * @return EntityUsage[] Usages before the update
* @throws UsageTrackerException
+ * @return EntityUsage[] Usages before the update
*/
public function trackUsedEntities( $pageId, array $usages );
diff --git a/client/tests/phpunit/includes/Usage/EntityUsageTest.php
b/client/tests/phpunit/includes/Usage/EntityUsageTest.php
index 4ef3427..a2cf399 100644
--- a/client/tests/phpunit/includes/Usage/EntityUsageTest.php
+++ b/client/tests/phpunit/includes/Usage/EntityUsageTest.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase\Client\Tests\Usage;
use PHPUnit_Framework_TestCase;
diff --git a/client/tests/phpunit/includes/Usage/HashUsageAccumulatorTest.php
b/client/tests/phpunit/includes/Usage/HashUsageAccumulatorTest.php
index 7d6a9e6..32ccfd1 100644
--- a/client/tests/phpunit/includes/Usage/HashUsageAccumulatorTest.php
+++ b/client/tests/phpunit/includes/Usage/HashUsageAccumulatorTest.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase\Client\Usage\Tests;
use Wikibase\Client\Usage\HashUsageAccumulator;
diff --git
a/client/tests/phpunit/includes/Usage/ParserOutputUsageAccumulatorTest.php
b/client/tests/phpunit/includes/Usage/ParserOutputUsageAccumulatorTest.php
index bfb3a1b..0da18cd 100644
--- a/client/tests/phpunit/includes/Usage/ParserOutputUsageAccumulatorTest.php
+++ b/client/tests/phpunit/includes/Usage/ParserOutputUsageAccumulatorTest.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase\Client\Usage\Tests;
use ParserOutput;
diff --git a/client/tests/phpunit/includes/Usage/Sql/SqlUsageTrackerTest.php
b/client/tests/phpunit/includes/Usage/Sql/SqlUsageTrackerTest.php
index f332d6b..aef78eb 100644
--- a/client/tests/phpunit/includes/Usage/Sql/SqlUsageTrackerTest.php
+++ b/client/tests/phpunit/includes/Usage/Sql/SqlUsageTrackerTest.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase\Client\Tests\Usage\Sql;
use PHPUnit_Framework_Assert as Assert;
@@ -6,6 +7,7 @@
use Wikibase\Client\Tests\Usage\UsageLookupContractTester;
use Wikibase\Client\Tests\Usage\UsageTrackerContractTester;
use Wikibase\Client\Usage\Sql\SqlUsageTracker;
+use Wikibase\Client\Usage\UsageTracker;
use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\Entity\BasicEntityIdParser;
@@ -42,7 +44,7 @@
$this->markTestSkipped( 'Skipping test for
SqlUsageTracker, because the useLegacyUsageIndex option is set.' );
}
- $this->tablesUsed[] = 'wbc_entity_usage';
+ $this->tablesUsed[] = UsageTracker::TABLE_NAME;
parent::setUp();
diff --git a/client/tests/phpunit/includes/Usage/Sql/UsageTablePrimerTest.php
b/client/tests/phpunit/includes/Usage/Sql/UsageTablePrimerTest.php
index 250d959..853fd51 100644
--- a/client/tests/phpunit/includes/Usage/Sql/UsageTablePrimerTest.php
+++ b/client/tests/phpunit/includes/Usage/Sql/UsageTablePrimerTest.php
@@ -1,12 +1,15 @@
<?php
+
namespace Wikibase\Client\Tests\Usage\Sql;
use PHPUnit_Framework_MockObject_Matcher;
use PHPUnit_Framework_MockObject_Matcher_Invocation;
use Wikibase\Client\Usage\Sql\UsageTablePrimer;
+use Wikibase\Client\Usage\UsageTracker;
use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\Lib\Reporting\ExceptionHandler;
+use Wikibase\Lib\Reporting\MessageReporter;
/**
* @covers Wikibase\Client\Usage\Sql\UsageTablePrimer
@@ -21,24 +24,22 @@
*/
class UsageTablePrimerTest extends \MediaWikiTestCase {
- private $tableName = 'wbc_entity_usage';
-
public function setUp() {
if (
WikibaseClient::getDefaultInstance()->getSettings()->getSetting(
'useLegacyUsageIndex' ) ) {
$this->markTestSkipped( 'Skipping test for
UsageTablePrimer, because the useLegacyUsageIndex option is set.' );
}
- $this->tablesUsed[] = $this->tableName;
+ $this->tablesUsed[] = UsageTracker::TABLE_NAME;
$this->tablesUsed[] = 'page_props';
parent::setUp();
}
- private function getUsageTablePrimer( $bastchSize = 10 ) {
+ private function getUsageTablePrimer( $batchSize = 10 ) {
$loadBalancer = wfGetLB();
$idParser = new BasicEntityIdParser();
- return new UsageTablePrimer( $idParser, $loadBalancer,
$this->tableName, $bastchSize );
+ return new UsageTablePrimer( $idParser, $loadBalancer,
$batchSize );
}
public function testFillUsageTable() {
@@ -89,7 +90,7 @@
private function fetchAllUsageStrings() {
$db = wfGetDB( DB_MASTER );
- $res = $db->select( $this->tableName, "*", '', __METHOD__ );
+ $res = $db->select( UsageTracker::TABLE_NAME, '*', '',
__METHOD__ );
$usages = array();
foreach ( $res as $row ) {
@@ -102,7 +103,7 @@
}
/**
- * @param PHPUnit_Framework_MockObject_Matcher $matcher
+ * @param PHPUnit_Framework_MockObject_Matcher_Invocation $matcher
*
* @return ExceptionHandler
*/
@@ -115,9 +116,9 @@
}
/**
- * @param PHPUnit_Framework_MockObject_Matcher $matcher
+ * @param PHPUnit_Framework_MockObject_Matcher_Invocation $matcher
*
- * @return ExceptionHandler
+ * @return MessageReporter
*/
private function getMessageReporter(
PHPUnit_Framework_MockObject_Matcher_Invocation $matcher ) {
$mock = $this->getMock(
'Wikibase\Lib\Reporting\MessageReporter' );
diff --git a/client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php
b/client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php
index 69c9220..a21ad1e 100644
--- a/client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php
+++ b/client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase\Client\Tests\Usage\Sql;
use DatabaseBase;
@@ -6,6 +7,7 @@
use PHPUnit_Framework_TestCase;
use Wikibase\Client\Usage\EntityUsage;
use Wikibase\Client\Usage\Sql\UsageTableUpdater;
+use Wikibase\Client\Usage\UsageTracker;
use Wikibase\Client\WikibaseClient;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\ItemId;
@@ -23,14 +25,12 @@
*/
class UsageTableUpdaterTest extends \MediaWikiTestCase {
- private $tableName = 'wbc_entity_usage';
-
protected function setUp() {
if (
WikibaseClient::getDefaultInstance()->getSettings()->getSetting(
'useLegacyUsageIndex' ) ) {
$this->markTestSkipped( 'Skipping test for
UsageTableUpdater, because the useLegacyUsageIndex option is set.' );
}
- $this->tablesUsed[] = $this->tableName;
+ $this->tablesUsed[] = UsageTracker::TABLE_NAME;
parent::setUp();
}
@@ -117,7 +117,7 @@
}
private function getUsageTableUpdater( $batchSize = 1000 ) {
- return new UsageTableUpdater( wfGetDB( DB_WRITE ),
$this->tableName, $batchSize );
+ return new UsageTableUpdater( wfGetDB( DB_WRITE ), $batchSize );
}
public function testUpdateUsage() {
@@ -233,7 +233,8 @@
* @return bool
*/
private function rowExists( DatabaseBase $db, $conditions ) {
- $count = $db->selectRowCount( $this->tableName, '*',
$conditions );
+ $count = $db->selectRowCount( UsageTracker::TABLE_NAME, '*',
$conditions );
return $count > 0;
}
+
}
diff --git
a/client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php
b/client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php
index 0f38d66..7bb3783 100644
--- a/client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php
+++ b/client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase\Client\Usage\Tests;
use PHPUnit_Framework_Assert as Assert;
diff --git a/client/tests/phpunit/includes/Usage/UsageLookupContractTester.php
b/client/tests/phpunit/includes/Usage/UsageLookupContractTester.php
index 7791451..6390ea4 100644
--- a/client/tests/phpunit/includes/Usage/UsageLookupContractTester.php
+++ b/client/tests/phpunit/includes/Usage/UsageLookupContractTester.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase\Client\Tests\Usage;
use PHPUnit_Framework_Assert as Assert;
diff --git a/client/tests/phpunit/includes/Usage/UsageTrackerContractTester.php
b/client/tests/phpunit/includes/Usage/UsageTrackerContractTester.php
index 72824f7..923b48e 100644
--- a/client/tests/phpunit/includes/Usage/UsageTrackerContractTester.php
+++ b/client/tests/phpunit/includes/Usage/UsageTrackerContractTester.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase\Client\Tests\Usage;
use PHPUnit_Framework_Assert as Assert;
--
To view, visit https://gerrit.wikimedia.org/r/176356
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf85ad8ed940a95ba741f2640d06ab506d563dd4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
