jenkins-bot has submitted this change and it was merged.
Change subject: Utilize LinkBatch for gender, NS_USER, and NS_USER_TALK lookups
......................................................................
Utilize LinkBatch for gender, NS_USER, and NS_USER_TALK lookups
Also split the query method out of UserNameBatch to simplify writing
the tests.
Change-Id: I55932951a90ea2295a3918f7b45490c4122c2770
---
M Flow.php
M container.php
M includes/Data/UserNameBatch.php
M includes/DbFactory.php
A tests/UserNameBatchTest.php
5 files changed, 191 insertions(+), 15 deletions(-)
Approvals:
Matthias Mullie: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Flow.php b/Flow.php
index 551538b..79f599b 100755
--- a/Flow.php
+++ b/Flow.php
@@ -136,6 +136,9 @@
$wgAutoloadClasses['Flow\Contributions\Formatter'] = $dir .
'includes/Contributions/Formatter.php';
$wgAutoloadClasses['Flow\Data\UserNameListener'] = $dir .
'includes/Data/UserNameBatch.php';
$wgAutoloadClasses['Flow\Data\UserNameBatch'] = $dir .
'includes/Data/UserNameBatch.php';
+$wgAutoloadClasses['Flow\Data\UserNameQuery'] = $dir .
'includes/Data/UserNameBatch.php';
+$wgAutoloadClasses['Flow\Data\OneStepUserNameQuery'] = $dir .
'includes/Data/UserNameBatch.php';
+$wgAutoloadClasses['Flow\Data\TwoStepUserNameQuery'] = $dir .
'includes/Data/UserNameBatch.php';
// database interaction for singular models
$wgAutoloadClasses['Flow\Data\RevisionStorage'] = $dir .
'includes/Data/RevisionStorage.php';
diff --git a/container.php b/container.php
index 68c8b54..b4d0daf 100644
--- a/container.php
+++ b/container.php
@@ -107,7 +107,7 @@
} );
// Batched username loader
$c['repository.username'] = $c->share( function( $c ) {
- return new Flow\Data\UserNameBatch;
+ return new Flow\Data\UserNameBatch( new Flow\Data\TwoStepUsernameQuery(
$c['db.factory'] ) );
} );
// Per wiki workflow definitions (types of workflows)
$c['storage.definition'] = $c->share( function( $c ) {
diff --git a/includes/Data/UserNameBatch.php b/includes/Data/UserNameBatch.php
index f0618c6..279ace2 100644
--- a/includes/Data/UserNameBatch.php
+++ b/includes/Data/UserNameBatch.php
@@ -5,6 +5,8 @@
*/
namespace Flow\Data;
+use Flow\DbFactory;
+
/**
* Listen for loaded objects and pre-load their user id fields into
* a batch username loader.
@@ -72,9 +74,12 @@
protected $usernames = array();
/**
+ * @param DbFactory $dbFactory Provides access to database objects.
Note that
+ * the LinkBatch will not use this dbFactory.
* @param array $queued map from wikiid to list of userid's to request
*/
- public function __construct( array $queued = array() ) {
+ public function __construct( UserNameQuery $query, array $queued =
array() ) {
+ $this->query = $query;
foreach ( $queued as $wiki => $userIds ) {
$this->queued[$wiki] = array_map( 'intval', $userIds );
}
@@ -129,16 +134,16 @@
if ( isset( $this->usernames[$wiki] ) ) {
$queued = array_diff( $queued, array_keys(
$this->usernames[$wiki] ) );
}
- $res = $this->query( $wiki, $queued );
+ $res = $this->query->execute( $wiki, $queued );
unset( $this->queued[$wiki] );
if ( $res ) {
- $found = array();
+ $usernames = array();
foreach ( $res as $row ) {
$id = (int)$row->user_id;
- $this->usernames[$wiki][$id] = $row->user_name;
- $found[] = $id;
+ $this->usernames[$wiki][$id] = $usernames[$id]
= $row->user_name;
}
- $missing = array_diff( $queued, $found );
+ $this->resolveUserPages( $wiki, $usernames );
+ $missing = array_diff( $queued, array_keys( $usernames
) );
} else {
$missing = $queued;
}
@@ -148,13 +153,54 @@
}
/**
+ * Update in-process title existence cache with NS_USER and
+ * NS_USER_TALK pages related to the provided usernames.
+ *
+ * @param string $wiki Wiki the users belong to
+ * @param array $usernames List of user names
+ */
+ protected function resolveUserPages( $wiki, array $usernames ) {
+ // LinkBatch currently only supports the current wiki
+ if ( $wiki !== wfWikiId() || !$usernames ) {
+ return;
+ }
+
+ $lb = new \LinkBatch();
+ foreach ( $usernames as $name ) {
+ $user = User::newFromName( $name );
+ if ( $user ) {
+ $lb->addObj( $user->getUserPage() );
+ $lb->addObj( $user->getTalkPage() );
+ }
+ }
+ $lb->setCaller( __METHOD__ );
+ $lb->execute();
+ }
+}
+
+interface UsernameQuery {
+ /**
+ * @param string $wiki wiki id
+ * @param array $userIds List of user ids to lookup
+ * @return bool|ResultWrapper Containing objects with user_id and
+ * user_name properies.
+ */
+ function execute( $wiki, array $userIds );
+}
+
+class TwoStepUsernameQuery implements UsernameQuery {
+ public function __construct( DbFactory $dbFactory ) {
+ $this->dbFactory = $dbFactory;
+ }
+
+ /**
* Look up usernames while respecting ipblocks with two queries
*
* @param string $wiki
* @param array $userIds
*/
- protected function query( $wiki, array $userIds ) {
- $dbr = wfGetDB( DB_SLAVE, array(), $wiki );
+ public function execute( $wiki, array $userIds ) {
+ $dbr = $this->dbFactory->getWikiDB( DB_SLAVE, array(), $wiki );
$res = $dbr->select(
'ipblocks',
'ipb_user',
@@ -183,6 +229,12 @@
__METHOD__
);
}
+}
+
+class OneStepUsernameQuery implements UsernameQuery {
+ public function __construct( DbFactory $dbFactory ) {
+ $this->dbFactory = $dbFactory;
+ }
/**
* Look up usernames while respecting ipblocks with one query.
@@ -191,8 +243,8 @@
* @param string $wiki
* @param array $userIds
*/
- protected function querySingle( $wiki, array $userIds ) {
- $dbr = wfGetDB( DB_SLAVE, array(), $wiki );
+ public function execute( $wiki, array $userIds ) {
+ $dbr = $this->dbFactory->getWikiDb( DB_SLAVE, array(), $wiki );
return $dbr->select(
/* table */ array( 'user', 'ipblocks' ),
/* select */ array( 'user_id', 'user_name' ),
diff --git a/includes/DbFactory.php b/includes/DbFactory.php
index b9c18c7..af59e42 100644
--- a/includes/DbFactory.php
+++ b/includes/DbFactory.php
@@ -3,7 +3,15 @@
namespace Flow;
/**
- * All classes within Flow that need to access the Flow db will go through
here.
+ * All classes within Flow that need to access the Flow db will go through
+ * this class. Having it separated into an object greatly simplifies testing
+ * anything that needs to talk to the database.
+ *
+ * The factory receives, in its constructor, the wiki name and cluster name
+ * that flow specific data is stored on. Multiple wiki's can and should be
+ * using the same wiki name and cluster to share flow specific data. These
values
+ * are used. The $wiki parameter of getDB and getLB must be null to receive
+ * the flow database.
*
* To access core tables, use wfGetDB() etc. This is solely for Flow-specific
* data, which may live on a separate database.
@@ -29,9 +37,9 @@
}
/**
- * @param int $db Index of the connection to get (DB_MASTER, DB_SLAVE or
- * specific server index)
- * @param mixed $groups Query groups
+ * @param integer $db index of the connection to get.
DB_MASTER|DB_SLAVE.
+ * @param mixed $groups query groups. An array of group names that this
query
+ * belongs to.
* @return \DatabaseBase
*/
public function getDB( $db, $groups = array() ) {
@@ -49,6 +57,32 @@
}
}
+ /**
+ * Mockable version of wfGetDB.
+ *
+ * @param integer $db index of the connection to get.
DB_MASTER|DB_SLAVE.
+ * @param mixed $groups query groups. An array of group names that this
query
+ * belongs to.
+ * @param string|false $wiki The wiki ID, or false for the current wiki
+ * @return \DatabaseBase
+ */
+ public function getWikiDB( $db, $groups = array(), $wiki = false ) {
+ return wfGetDB( $db, $groups, $wiki );
+ }
+
+ /**
+ * Mockable version of wfGetLB.
+ *
+ * @param string $wiki wiki ID, or false for the current wiki
+ * @return \LoadBalancer
+ */
+ public function getWikiLB( $wiki = false ) {
+ return wfGetLB( $wiki );
+ }
+
+ /**
+ * Wait for the slaves of the Flow database
+ */
public function waitForSlaves() {
wfWaitForSlaves( false, $this->wiki, $this->cluster );
}
diff --git a/tests/UserNameBatchTest.php b/tests/UserNameBatchTest.php
new file mode 100644
index 0000000..2d735ee
--- /dev/null
+++ b/tests/UserNameBatchTest.php
@@ -0,0 +1,87 @@
+<?php
+
+namespace Flow\Tests;
+
+use Flow\Data\UserNameBatch;
+use Flow\Data\UserNameQuery;
+
+/**
+ * @group Database
+ */
+class UserNameBatchTest extends \MediaWikiTestCase {
+
+ public function testAllowsAddingNames() {
+ $batch = new UserNameBatch( $this->createUncalledQuery() );
+ $batch->add( 'fakewiki', 42, 'Whale' );
+ $this->assertEquals( 'Whale', $batch->get( 'fakewiki', 42 ) );
+ }
+
+ static public function acceptsStringOrIntIdsProvider() {
+ return array(
+ array( 42, 42 ),
+ array( 42, '42' ),
+ array( '42', 42 ),
+ array( '42', '42' ),
+ );
+ }
+
+ /**
+ * @dataProvider acceptsStringOrIntIdsProvider
+ */
+ public function testAcceptsStringOrIntIds( $a, $b ) {
+ $batch = new UserNameBatch( $this->createUncalledQuery() );
+ $batch->add( 'fakewiki', $a, 'Whale' );
+ $this->assertEquals( 'Whale', $batch->get( 'fakewiki', $b ) );
+ }
+
+ public function testQueueUsernames() {
+ $query = $this->getMock( 'Flow\Data\UsernameQuery' );
+ $query->expects( $this->once() )
+ ->method( 'execute' )
+ ->with( 'fakewiki', array( 12, 27, 18 ) );
+
+ $batch = new UserNameBatch( $query );
+ $batch->add( 'fakewiki', 12 );
+ $batch->add( 'fakewiki', '27' );
+ $batch->add( 'fakewiki', 18 );
+ $batch->resolve( 'fakewiki' );
+ }
+
+ public function testMissingAsFalse() {
+ $query = $this->getMock( 'Flow\Data\UsernameQuery' );
+ $query->expects( $this->once() )
+ ->method( 'execute' )
+ ->with( 'fakewiki', array( 42 ) );
+ $batch = new UserNameBatch( $query );
+
+ $this->assertEquals( false, $batch->get( 'fakewiki', 42 ) );
+ }
+
+ public function testPartialMissingAsFalse() {
+ $query = $this->getMock( 'Flow\Data\UsernameQuery' );
+ $query->expects( $this->once() )
+ ->method( 'execute' )
+ ->with( 'fakewiki', array( 610, 408 ) )
+ ->will( $this->returnValue( array(
+ (object)array( 'user_id' => '408', 'user_name'
=> 'chuck' )
+ ) ) );
+
+ $batch = new UserNameBatch( $query );
+ $batch->add( 'fakewiki', 610 );
+ $batch->add( 'fakewiki', 408 );
+
+ $this->assertEquals( false, $batch->get( 'fakewiki', 610 ) );
+ }
+
+ /**
+ * Create a mock UsernameQuery that must not be called
+ * @return UsernameQuery
+ */
+ protected function createUncalledQuery() {
+ $query = $this->getMock( 'Flow\Data\UsernameQuery' );
+ $query->expects( $this->never() )
+ ->method( 'execute' );
+
+ return $query;
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/107717
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I55932951a90ea2295a3918f7b45490c4122c2770
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: Werdna <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
