jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/349977 )
Change subject: Transform user ID from repo to client in change prop
......................................................................
Transform user ID from repo to client in change prop
On the repo side, save the user's central user ID (if they are part of
a central login system) into the change.
On the client side, transform this back to the user ID for the client
wiki. If they have not logged into the client wiki, it is not part of
a central system, etc., it will be transformed to 0.
Bug: T51315
Change-Id: Ie7b9c482cf6a0dd7215b34841efd86fb51be651a
---
M client/includes/RecentChanges/ExternalChangeFactory.php
M client/includes/RecentChanges/RecentChangeFactory.php
M client/includes/RecentChanges/RevisionData.php
M client/includes/WikibaseClient.php
M client/tests/phpunit/includes/RecentChanges/RecentChangeFactoryTest.php
M docs/change-propagation.wiki
A lib/includes/Changes/CentralIdLookupFactory.php
M lib/includes/Changes/Change.php
M lib/includes/Changes/ChangeRow.php
M lib/includes/Changes/EntityChange.php
M lib/tests/phpunit/Changes/EntityChangeTest.php
A lib/tests/phpunit/Changes/MockRepoClientCentralIdLookup.php
M repo/Wikibase.hooks.php
M repo/includes/Notifications/ChangeNotifier.php
M repo/includes/WikibaseRepo.php
M repo/tests/phpunit/includes/Notifications/ChangeNotifierTest.php
M repo/tests/phpunit/includes/Store/Sql/SqlChangeStoreTest.php
17 files changed, 523 insertions(+), 42 deletions(-)
Approvals:
WMDE-leszek: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/includes/RecentChanges/ExternalChangeFactory.php
b/client/includes/RecentChanges/ExternalChangeFactory.php
index 4dcdfbb..6f01fb1 100644
--- a/client/includes/RecentChanges/ExternalChangeFactory.php
+++ b/client/includes/RecentChanges/ExternalChangeFactory.php
@@ -123,6 +123,10 @@
throw new UnexpectedValueException( 'Invalid Wikibase
change' );
}
+ // TODO: Add central_user_id after
+ // Ie7b9c482cf6a0dd7215b34841efd86fb51be651a has been deployed
long
+ // enough that there are no rows without this.
+ // Bug: T51315
$keys = [ 'type', 'page_id', 'rev_id', 'parent_id', 'object_id'
];
foreach ( $keys as $key ) {
diff --git a/client/includes/RecentChanges/RecentChangeFactory.php
b/client/includes/RecentChanges/RecentChangeFactory.php
index 79a6afc..a5209e0 100644
--- a/client/includes/RecentChanges/RecentChangeFactory.php
+++ b/client/includes/RecentChanges/RecentChangeFactory.php
@@ -2,7 +2,9 @@
namespace Wikibase\Client\RecentChanges;
+use CentralIdLookup;
use Language;
+use LocalIdLookup;
use Message;
use MWException;
use RecentChange;
@@ -34,9 +36,30 @@
*/
private $siteLinkCommentCreator;
- public function __construct( Language $language, SiteLinkCommentCreator
$siteLinkCommentCreator ) {
+ /**
+ * @var CentralIdLookup|null
+ */
+ private $centralIdLookup;
+
+ /**
+ * @param Language $language Language to format in
+ * @param SiteLinkCommentCreator $siteLinkCommentCreator
+ * @param CentralIdLookup|null $centralIdLookup CentralIdLookup, or
null if
+ * this repository is not connected to a central user system (see
+ * Wikibase\Lib\Changes\CentralIdLookupFactory).
+ */
+ public function __construct(
+ Language $language,
+ SiteLinkCommentCreator $siteLinkCommentCreator,
+ $centralIdLookup ) {
+
$this->language = $language;
$this->siteLinkCommentCreator = $siteLinkCommentCreator;
+
+ // Required but should be null if the client is not connected
to a central
+ // user system
+ Assert::parameterType( 'CentralIdLookup|null',
$centralIdLookup, '$centralIdLookup' );
+ $this->centralIdLookup = $centralIdLookup;
}
/**
@@ -112,8 +135,12 @@
'wikibase-repo-change' => $metadata,
];
+ $repoUserId = $change->getUserId();
+ $clientUserId = $this->getClientUserId( $repoUserId, $metadata
);
+
$attribs = [
- 'rc_user' => 0,
+
+ 'rc_user' => $clientUserId,
'rc_user_text' => $userText,
'rc_comment' => $this->getEditCommentMulti( $change ),
'rc_type' => RC_EXTERNAL,
@@ -142,6 +169,53 @@
}
/**
+ * Gets the client's user ID from the repo user ID and EntityChange's
metadata
+ *
+ * @param int $repoUserId Original user ID from the repository
+ * @param array $metadata EntityChange metadata
+ *
+ * @return int User ID for the current (client) wiki
+ */
+ private function getClientUserId( $repoUserId, array $metadata ) {
+ if ( $repoUserId === 0 ) {
+ // Logged out on repo just copied to client
+ return 0;
+ } else {
+ // We decided to put 0 for users that can not be
matched. As a
+ // result, only users that are centralized and exist on
both wikis
+ // will be marked as logged in (and be mapped to the
local user).
+ //
+ // We don't currently auto-create the local account if
they've
+ // never logged into the client. This can be done with
+ // CentralAuth, but AFAIK there is not a portable way
to do this.
+
+ // Temporary compatibility until
Ie7b9c482cf6a0dd7215b34841efd86fb51be651a
+ // has been deployed long enough that all rows have it.
+ if ( array_key_exists( 'central_user_id', $metadata ) )
{
+ // See change-propagation.wiki for why it can
be 0 other than pre-deploy
+ // rows.
+ $centralUserId = $metadata['central_user_id'];
+ } else {
+ $centralUserId = 0;
+ }
+
+ if ( $centralUserId === 0 || $this->centralIdLookup ===
null ) {
+ return 0;
+ } else {
+ $clientUser =
$this->centralIdLookup->localUserFromCentralId(
+ $centralUserId
+ );
+
+ if ( $clientUser === null ) {
+ return 0;
+ } else {
+ return $clientUser->getId();
+ }
+ }
+ }
+ }
+
+ /**
* Builds change attribute specific to the given target page.
*
* @param EntityChange $change
diff --git a/client/includes/RecentChanges/RevisionData.php
b/client/includes/RecentChanges/RevisionData.php
index 37a291d..48ab89d 100644
--- a/client/includes/RecentChanges/RevisionData.php
+++ b/client/includes/RecentChanges/RevisionData.php
@@ -70,6 +70,17 @@
}
/**
+ * Gets the central user ID. This should be from CentralIdLookup,
+ * with the repo wiki and client wiki being part of the same central
+ * system.
+ *
+ * @return int
+ */
+ public function getCentralUserId() {
+ return $this->changeParams['central_user_id'];
+ }
+
+ /**
* @return int
*/
public function getPageId() {
diff --git a/client/includes/WikibaseClient.php
b/client/includes/WikibaseClient.php
index b2a9b58..0973a58 100644
--- a/client/includes/WikibaseClient.php
+++ b/client/includes/WikibaseClient.php
@@ -2,6 +2,7 @@
namespace Wikibase\Client;
+use Wikibase\Lib\Changes\CentralIdLookupFactory;
use DataTypes\DataTypeFactory;
use DataValues\Deserializers\DataValueDeserializer;
use DataValues\Geo\Values\GlobeCoordinateValue;
@@ -1223,7 +1224,8 @@
$this->getContentLanguage(),
$this->siteLookup,
$this->settings->getSetting( 'siteGlobalID' )
- )
+ ),
+
CentralIdLookupFactory::getInstance()->getCentralIdLookup()
);
}
diff --git
a/client/tests/phpunit/includes/RecentChanges/RecentChangeFactoryTest.php
b/client/tests/phpunit/includes/RecentChanges/RecentChangeFactoryTest.php
index 96b624f..a01d02a 100644
--- a/client/tests/phpunit/includes/RecentChanges/RecentChangeFactoryTest.php
+++ b/client/tests/phpunit/includes/RecentChanges/RecentChangeFactoryTest.php
@@ -2,11 +2,14 @@
namespace Wikibase\Client\Tests\RecentChanges;
+use CentralIdLookup;
use Diff\DiffOp\Diff\Diff;
use Diff\DiffOp\DiffOpChange;
use Diff\MapDiffer;
use Language;
+use Wikibase\Lib\Tests\Changes\MockRepoClientCentralIdLookup;
use SiteLookup;
+use Wikimedia\TestingAccessWrapper;
use Title;
use Wikibase\Client\RecentChanges\RecentChangeFactory;
use Wikibase\DataModel\Entity\EntityId;
@@ -30,12 +33,18 @@
/**
* @return RecentChangeFactory
*/
- private function newRecentChangeFactory() {
+ private function newRecentChangeFactoryHelper( $centralIdLookup ) {
$siteLookup = $this->getMock( SiteLookup::class );
$lang = Language::factory( 'qqx' );
$siteLinkCommentCreator = new SiteLinkCommentCreator( $lang,
$siteLookup, 'testwiki' );
- return new RecentChangeFactory( $lang, $siteLinkCommentCreator
);
+ return new RecentChangeFactory( $lang, $siteLinkCommentCreator,
$centralIdLookup );
+ }
+
+ private function newRecentChangeFactory() {
+ return $this->newRecentChangeFactoryHelper(
+ new MockRepoClientCentralIdLookup( /** isRepo= */ false
)
+ );
}
/**
@@ -112,6 +121,7 @@
$fields = [
'id' => '13',
+ 'user_id' => 3,
'time' => '20150202030303',
];
$metadata = [
@@ -153,7 +163,7 @@
'type' => 'wikibase-item~change',
'entity_type' => 'item',
],
- //'comment-html' => 'Generated Comment HTML',
// later
+ // 'comment-html' => 'Generated Comment HTML',
// later
] ),
'rc_comment' => $metadata['comment'],
'rc_timestamp' => $fields['time'],
@@ -222,7 +232,7 @@
$preparedAttr
],
- //'composite change' => array(),
+ // 'composite change' => array(),
];
}
@@ -276,6 +286,7 @@
$fields = array_merge( [
'id' => '13',
'time' => '20150202030303',
+ 'user_id' => 0,
], $fields );
$metadata = array_merge( [
@@ -316,7 +327,7 @@
array $fields,
array $metadata
) {
- //@todo: also check pre-generated HTML when I5439a76c is merged
+ // @todo: also check pre-generated HTML when I5439a76c is merged
$change = $this->makeItemChangeFromMetaData( $action, $diff,
$fields, $metadata );
@@ -369,7 +380,7 @@
'/* set-de-label:1| */ bla bla',
'change',
$emptyDiff,
- [],
+ [ 'user_id' => 1 ],
[
'comment' => '/* set-de-label:1| */ bla
bla',
]
@@ -378,7 +389,7 @@
'(wikibase-comment-sitelink-change:
dewiki:Dummy, dewiki:Bummy)',
'change',
$this->makeItemDiff( $linksDewikiDummy,
$linksDewikiBummy ),
- [],
+ [ 'user_id' => 1 ],
[
'comment' => '/* IGNORE-KITTENS:1| */
SILLY KITTENS',
]
@@ -387,7 +398,7 @@
'(wikibase-comment-sitelink-add: dewiki:Bummy)',
'change',
$this->makeItemDiff( $linksEmpty,
$linksDewikiBummy ),
- [],
+ [ 'user_id' => 1 ],
[
'comment' => '/* IGNORE-KITTENS:1| */
SILLY KITTENS',
]
@@ -396,7 +407,7 @@
'(wikibase-comment-sitelink-remove:
dewiki:Dummy)',
'change',
$this->makeItemDiff( $linksDewikiDummy,
$linksEmpty ),
- [],
+ [ 'user_id' => 1 ],
[
'comment' => '/* IGNORE-KITTENS:1| */
SILLY KITTENS',
]
@@ -419,7 +430,8 @@
'comment' =>
'/* set-en-description:1| */ Foo',
],
],
- ] ]
+ ] ],
+ 'user_id' => 1,
],
[]
],
@@ -448,4 +460,78 @@
$this->assertEquals( $expectedComment, $rc->getAttribute(
'rc_comment' ) );
}
+ /**
+ * @dataProvider provideGetClientUserId
+ */
+ public function testGetClientUserId( $expectedClientUserId,
$centralIdLookup, $repoUserId, $metadata ) {
+ $recentChangeFactory = $this->newRecentChangeFactoryHelper(
$centralIdLookup );
+
+ $recentChangeFactory = TestingAccessWrapper::newFromObject(
$recentChangeFactory );
+ $clientUserId = $recentChangeFactory->getClientUserId(
$repoUserId, $metadata );
+ $this->assertSame(
+ $expectedClientUserId,
+ $clientUserId
+ );
+ }
+
+ // * central = -1, repo = 1, client = 2
+
+ public function provideGetClientUserId() {
+ $centralIdLookup = new MockRepoClientCentralIdLookup(
+ /** isRepo= */ false
+ );
+
+ return [
+ 'Logged out on repo' => [
+ 0,
+ $centralIdLookup,
+ 0,
+ [ 'central_user_id' => 0 ],
+ ],
+
+ 'No central ID lookup (client wiki is not connected to
central ' .
+ 'user system' => [
+ 0,
+ null,
+ 3,
+ [ 'central_user_id' => -3 ],
+ ],
+
+ '0 central user ID although there is a repo user ID,
e.g.' .
+ ' Wikibase repo user not attached' => [
+ 0,
+ $centralIdLookup,
+ 5,
+ [ 'central_user_id' => 0 ],
+ ],
+
+ 'No central user ID because it is from a row created
before ' .
+ 'central_user_id was saved' =>
+ [
+ 0,
+ $centralIdLookup,
+ 7,
+ [],
+ ],
+
+ 'Invalid central ID so client user ID is 0' => [
+ 0,
+ $centralIdLookup,
+ 8,
+ [ 'central_user_id' => 3 ],
+ ],
+
+ 'Happy path; user ID is fully mapped' => [
+ 8,
+ $centralIdLookup,
+
+ // Would be 4 for mock, but it doesn't use
+ // this other than == or != 0.
+ 9,
+
+ [ 'central_user_id' => -4 ]
+ ],
+ ];
+ }
+
}
diff --git a/docs/change-propagation.wiki b/docs/change-propagation.wiki
index 0f7031a..b7b45dd 100644
--- a/docs/change-propagation.wiki
+++ b/docs/change-propagation.wiki
@@ -38,10 +38,11 @@
* change_time, a varbinary(14) the time at which the edit was made
* change_object_id, a varbinary(14) containing the entity ID
* change_revision_id, a int(10) containing the revision ID
-* change_user_id, a int(10) containing the user id
+* change_user_id, a int(10) containing the original (repository) user id, or 0
for logged out users.
* change_info, a mediumblob containing a JSON structure with additional
information about the change. Well known top level fields are:
** "diff": a serialized diff, as produced by EntityDiffer
** "metadata", a JSON object representing essential revision meta data, using
the following fields:
+*** "central_user_id": the central user ID (int). 0 if the repo is not
connected to a central user system, the action was by a logged out user, the
particular user is not attached on the repo, or the user is restricted (uses
AUDIENCE_PUBLIC)
*** "user_text": the user name (string)
*** "page_id": the id of the wiki page containing the entity on the repo (int)
*** "rev_id": the id of the revision created by this change on the repo (int)
diff --git a/lib/includes/Changes/CentralIdLookupFactory.php
b/lib/includes/Changes/CentralIdLookupFactory.php
new file mode 100644
index 0000000..4abc085
--- /dev/null
+++ b/lib/includes/Changes/CentralIdLookupFactory.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace Wikibase\Lib\Changes;
+
+use CentralIdLookup;
+use LocalIdLookup;
+
+/**
+ * @license GPL-2.0+
+ * @author Matthew Flaschen < [email protected] >
+ */
+class CentralIdLookupFactory {
+
+ /**
+ * Returns an instance of the factory
+ *
+ * @return CentralIdLookupFactory
+ */
+ public static function getInstance() {
+ return new CentralIdLookupFactory();
+ }
+
+ /**
+ * Returns a CentralIdLookup that is safe to use for cross-wiki
propagation, or
+ * null.
+ *
+ * @return CentralIdLookup|null
+ */
+ public function getCentralIdLookup() {
+ $centralIdLookup = CentralIdLookup::factory();
+
+ if ( $centralIdLookup !== null &&
+
+ // LocalIdLookup is the default for standalone wikis.
However,
+ // it will map to the wrong user unless repo and client
+ // are both using it, and using the same shared user
tables.
+ //
+ // See also T163277 and
https://phabricator.wikimedia.org/T170996 .
+ !( $centralIdLookup instanceof LocalIdLookup )
+ ) {
+ return $centralIdLookup;
+ }
+
+ return null;
+ }
+
+}
diff --git a/lib/includes/Changes/Change.php b/lib/includes/Changes/Change.php
index ef9a57c..86ac1df 100644
--- a/lib/includes/Changes/Change.php
+++ b/lib/includes/Changes/Change.php
@@ -30,6 +30,13 @@
public function getTime();
/**
+ * Original (repository) user id, or 0 for logged out users.
+ *
+ * @return int
+ */
+ public function getUserId();
+
+ /**
* @return int|null Number to be used as an identifier when persisting
the change.
*/
public function getId();
diff --git a/lib/includes/Changes/ChangeRow.php
b/lib/includes/Changes/ChangeRow.php
index 0fc4916..970b4a8 100644
--- a/lib/includes/Changes/ChangeRow.php
+++ b/lib/includes/Changes/ChangeRow.php
@@ -39,6 +39,15 @@
return $this->getField( 'time' );
}
+ /**
+ * Original (repository) user id, or 0 for logged out users.
+ *
+ * @return int
+ */
+ public function getUserId() {
+ return $this->getField( 'user_id' );
+ }
+
public function __construct( array $fields = [] ) {
$this->setFields( $fields );
}
diff --git a/lib/includes/Changes/EntityChange.php
b/lib/includes/Changes/EntityChange.php
index 7865fef..9d799d5 100644
--- a/lib/includes/Changes/EntityChange.php
+++ b/lib/includes/Changes/EntityChange.php
@@ -2,9 +2,11 @@
namespace Wikibase;
+use CentralIdLookup;
use Deserializers\Deserializer;
use Diff\DiffOp\DiffOp;
use Diff\DiffOpFactory;
+use LocalIdLookup;
use MWException;
use RecentChange;
use Revision;
@@ -25,6 +27,7 @@
* @author Jeroen De Dauw < [email protected] >
* @author Katie Filbert < [email protected] >
* @author Daniel Kinzler
+ * @author Matthew Flaschen < [email protected] >
*/
class EntityChange extends DiffChange {
@@ -104,6 +107,7 @@
'bot',
'rev_id',
'parent_id',
+ 'central_user_id',
'user_text',
'comment'
];
@@ -143,35 +147,69 @@
/**
* @param RecentChange $rc
+ * @param int $centralUserId Central user ID, or 0 if unknown or not
applicable
+ * (see docs/change-propagation.wiki)
*
* @todo rename to setRecentChangeInfo
*/
- public function setMetadataFromRC( RecentChange $rc ) {
+ public function setMetadataFromRC( RecentChange $rc, $centralUserId ) {
$this->setFields( [
'revision_id' => $rc->getAttribute( 'rc_this_oldid' ),
- 'user_id' => $rc->getAttribute( 'rc_user' ),
'time' => $rc->getAttribute( 'rc_timestamp' ),
] );
$this->setMetadata( [
- 'user_text' => $rc->getAttribute( 'rc_user_text' ),
'bot' => $rc->getAttribute( 'rc_bot' ),
'page_id' => $rc->getAttribute( 'rc_cur_id' ),
'rev_id' => $rc->getAttribute( 'rc_this_oldid' ),
'parent_id' => $rc->getAttribute( 'rc_last_oldid' ),
'comment' => $rc->getAttribute( 'rc_comment' ),
] );
+
+ $this->addUserMetadata(
+ $rc->getAttribute( 'rc_user' ),
+ $rc->getAttribute( 'rc_user_text' ),
+ $centralUserId
+ );
}
/**
- * @param User $user
+ * Add fields and metadata related to the user.
*
- * @todo rename to setUserInfo
+ * This does not touch other fields or metadata.
+ *
+ * @param int $repoUserId User ID on wiki where change was made, or 0
for anon
+ * @param string $repoUserText User text on wiki where change was made,
for either
+ * logged in user or anon
+ * @param int $centralUserId Central user ID, or 0 if unknown or not
applicable
+ * (see docs/change-propagation.wiki)
*/
- public function setMetadataFromUser( User $user ) {
+ private function addUserMetadata( $repoUserId, $repoUserText,
$centralUserId ) {
$this->setFields( [
- 'user_id' => $user->getId(),
+ 'user_id' => $repoUserId,
] );
+
+ $metadata = [
+ 'user_text' => $repoUserText,
+ 'central_user_id' => $centralUserId,
+ ];
+
+ $this->setMetadata( $metadata );
+ }
+
+ /**
+ * @todo rename to setUserInfo
+ *
+ * @param User $user User that made change
+ * @param int $centralUserId Central user ID, or 0 if unknown or not
applicable
+ * (see docs/change-propagation.wiki)
+ */
+ public function setMetadataFromUser( User $user, $centralUserId ) {
+ $this->addUserMetadata(
+ $user->getId(),
+ $user->getName(),
+ $centralUserId
+ );
// TODO: init page_id etc in getMetadata, not here!
$metadata = array_merge( [
@@ -182,15 +220,17 @@
$this->getMetadata()
);
- $metadata['user_text'] = $user->getName();
-
$this->setMetadata( $metadata );
}
- public function setRevisionInfo( Revision $revision ) {
+ /**
+ * @param Revision $revision Revision to populate EntityChange from
+ * @param int $centralUserId Central user ID, or 0 if unknown or not
applicable
+ * (see docs/change-propagation.wiki)
+ */
+ public function setRevisionInfo( Revision $revision, $centralUserId ) {
$this->setFields( [
'revision_id' => $revision->getId(),
- 'user_id' => $revision->getUser(),
'time' => $revision->getTimestamp(),
] );
@@ -205,12 +245,17 @@
}
$this->setMetadata( [
- 'user_text' => $revision->getUserText(),
'page_id' => $revision->getPage(),
'parent_id' => $revision->getParentId(),
'comment' => $revision->getComment(),
'rev_id' => $revision->getId(),
] );
+
+ $this->addUserMetadata(
+ $revision->getUser(),
+ $revision->getUserText(),
+ $centralUserId
+ );
}
/**
diff --git a/lib/tests/phpunit/Changes/EntityChangeTest.php
b/lib/tests/phpunit/Changes/EntityChangeTest.php
index 463b5ba..7b98075 100644
--- a/lib/tests/phpunit/Changes/EntityChangeTest.php
+++ b/lib/tests/phpunit/Changes/EntityChangeTest.php
@@ -8,6 +8,7 @@
use Revision;
use RuntimeException;
use stdClass;
+use Wikimedia\TestingAccessWrapper;
use User;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
@@ -155,7 +156,7 @@
$rc = RecentChange::newFromRow( $row );
$entityChange = $this->newEntityChange( new ItemId( 'Q7' ) );
- $entityChange->setMetadataFromRC( $rc );
+ $entityChange->setMetadataFromRC( $rc, 8 );
$this->assertEquals( 5, $entityChange->getField( 'revision_id'
), 'revision_id' );
$this->assertEquals( 7, $entityChange->getField( 'user_id' ),
'user_id' );
@@ -164,11 +165,59 @@
$this->assertEquals( 'Test!', $entityChange->getComment(),
'comment' );
$metadata = $entityChange->getMetadata();
+ $this->assertEquals( 8, $metadata['central_user_id'],
'central_user_id' );
$this->assertEquals( 3, $metadata['parent_id'], 'parent_id' );
$this->assertEquals( 6, $metadata['page_id'], 'page_id' );
$this->assertEquals( 5, $metadata['rev_id'], 'rev_id' );
$this->assertEquals( 1, $metadata['bot'], 'bot' );
$this->assertEquals( 'Mr. Kittens', $metadata['user_text'],
'user_text' );
+ }
+
+ /**
+ * @dataProvider provideTestAddUserMetadata
+ */
+ public function testAddUserMetadata( $repoUserId, $repoUserText,
$centralUserId ) {
+ $entityChange = $this->getMockBuilder( EntityChange::class )
+ ->setMethods( [
+ 'getCentralIdLookup',
+ 'setFields',
+ 'setMetadata',
+ ] )
+ ->getMock();
+
+ $entityChange->expects( $this->once() )
+ ->method( 'setFields' )
+ ->with( [
+ 'user_id' => $repoUserId,
+ ] );
+
+ $entityChange->expects( $this->once() )
+ ->method( 'setMetadata' )
+ ->with( [
+ 'user_text' => $repoUserText,
+ 'central_user_id' => $centralUserId,
+ ] );
+
+ $entityChange = TestingAccessWrapper::newFromObject(
$entityChange );
+ $entityChange->addUserMetadata( $repoUserId, $repoUserText,
$centralUserId );
+ }
+
+ // See MockRepoClientCentralIdLookup
+
+ public function provideTestAddUserMetadata() {
+ return [
+ [
+ 3,
+ 'Foo',
+ -3,
+ ],
+
+ [
+ 0,
+ '10.11.12.13',
+ 0,
+ ],
+ ];
}
public function testSetMetadataFromUser() {
@@ -191,13 +240,14 @@
'page_id' => 5, // will NOT be overwritten
] );
- $entityChange->setMetadataFromUser( $user );
+ $entityChange->setMetadataFromUser( $user, 3 );
$this->assertEquals( 7, $entityChange->getField( 'user_id' ),
'user_id' );
$metadata = $entityChange->getMetadata();
$this->assertEquals( 'Mr. Kittens', $metadata['user_text'],
'user_text' );
$this->assertEquals( 5, $metadata['page_id'], 'page_id should
be preserved' );
+ $this->assertArrayHasKey( 'central_user_id', $metadata,
'central_user_id should be initialized' );
$this->assertArrayHasKey( 'rev_id', $metadata, 'rev_id should
be initialized' );
}
@@ -226,7 +276,7 @@
'comment' => 'Test!',
] );
- $entityChange->setRevisionInfo( $revision );
+ $entityChange->setRevisionInfo( $revision, 8 );
$this->assertEquals( 5, $entityChange->getField( 'revision_id'
), 'revision_id' );
$this->assertEquals( 7, $entityChange->getField( 'user_id' ),
'user_id' );
@@ -235,6 +285,7 @@
$this->assertEquals( 'Test!', $entityChange->getComment(),
'comment' );
$metadata = $entityChange->getMetadata();
+ $this->assertEquals( 8, $metadata['central_user_id'],
'central_user_id' );
$this->assertEquals( 3, $metadata['parent_id'], 'parent_id' );
$this->assertEquals( 6, $metadata['page_id'], 'page_id' );
$this->assertEquals( 5, $metadata['rev_id'], 'rev_id' );
@@ -258,7 +309,7 @@
$change = new EntityChange( [ 'info' => [], 'type' => '~' ] );
$this->assertFalse( $change->hasField( 'object_id' ),
'precondition' );
- $change->setRevisionInfo( $revision );
+ $change->setRevisionInfo( $revision, 3 );
$this->assertSame( 'Q1', $change->getObjectId() );
}
diff --git a/lib/tests/phpunit/Changes/MockRepoClientCentralIdLookup.php
b/lib/tests/phpunit/Changes/MockRepoClientCentralIdLookup.php
new file mode 100644
index 0000000..6e34cbd
--- /dev/null
+++ b/lib/tests/phpunit/Changes/MockRepoClientCentralIdLookup.php
@@ -0,0 +1,75 @@
+<?php
+
+namespace Wikibase\Lib\Tests\Changes;
+
+use CentralIdLookup;
+use MWException;
+use User;
+
+/**
+ * Assumes a central system with only two repositories, a repo wiki and a
client.
+ *
+ * All IDs are multiples of the following scheme:
+ *
+ * central = -1, repo = 1, client = 2
+ *
+ * Takes a constructor parameter to specify which this is.
+ *
+ * We don't need the methods that operate by username, so we don't implement
them and
+ * instead reimplement the methods used by Wikibase.
+ */
+class MockRepoClientCentralIdLookup extends CentralIdLookup {
+ /**
+ * @param int Factor to multiply by to go from this wiki to the central
ID
+ */
+ private $toCentralFactor;
+
+ /**
+ * @param bool $isRepo True if this is the repo, false otherwise
+ */
+ public function __construct( $isRepo ) {
+ if ( $isRepo ) {
+ $this->toCentralFactor = -1;
+ } else {
+ $this->toCentralFactor = -0.5;
+ }
+ }
+
+ public function isAttached( User $user, $wikiId = null ) {
+ return true;
+ }
+
+ public function lookupCentralIds(
+ array $idToName, $audience = CentralIdLookup::AUDIENCE_PUBLIC,
$flags = CentralIdLookup::READ_NORMAL
+ ) {
+ throw new MWException( 'Not implemented' );
+ }
+
+ public function lookupUserNames(
+ array $nameToId, $audience = CentralIdLookup::AUDIENCE_PUBLIC,
$flags = CentralIdLookup::READ_NORMAL
+ ) {
+ throw new MWException( 'Not implemented' );
+ }
+
+ public function localUserFromCentralId(
+ $id, $audience = CentralIdLookup::AUDIENCE_PUBLIC, $flags =
CentralIdLookup::READ_NORMAL
+ ) {
+ if ( $id >= 0 ) {
+ // Invalid central ID
+ return null;
+ }
+
+ $localUserId = $id / $this->toCentralFactor;
+
+ return User::newFromId( $localUserId );
+ }
+
+ public function centralIdFromLocalUser(
+ User $user, $audience = CentralIdLookup::AUDIENCE_PUBLIC,
$flags = CentralIdLookup::READ_NORMAL
+ ) {
+ $localUserId = $user->getId();
+
+ return $localUserId * $this->toCentralFactor;
+ }
+
+}
diff --git a/repo/Wikibase.hooks.php b/repo/Wikibase.hooks.php
index f31a55c..a2b87fa 100644
--- a/repo/Wikibase.hooks.php
+++ b/repo/Wikibase.hooks.php
@@ -29,6 +29,7 @@
use Title;
use User;
use Wikibase\Lib\AutoCommentFormatter;
+use Wikibase\Lib\Changes\CentralIdLookupFactory;
use Wikibase\Lib\Store\EntityRevision;
use Wikibase\Lib\Store\Sql\EntityChangeLookup;
use Wikibase\Repo\Content\EntityHandler;
@@ -323,7 +324,17 @@
if ( $change ) {
$changeStore =
WikibaseRepo::getDefaultInstance()->getStore()->getChangeStore();
- $change->setMetadataFromRC( $recentChange );
+ $centralIdLookup =
CentralIdLookupFactory::getInstance()->getCentralIdLookup();
+ if ( $centralIdLookup === null ) {
+ $centralUserId = 0;
+ } else {
+ $repoUser =
$recentChange->getPerformer();
+ $centralUserId =
$centralIdLookup->centralIdFromLocalUser(
+ $repoUser
+ );
+ }
+
+ $change->setMetadataFromRC( $recentChange,
$centralUserId );
$changeStore->saveChange( $change );
}
}
diff --git a/repo/includes/Notifications/ChangeNotifier.php
b/repo/includes/Notifications/ChangeNotifier.php
index 65a1d0b..bac9b57 100644
--- a/repo/includes/Notifications/ChangeNotifier.php
+++ b/repo/includes/Notifications/ChangeNotifier.php
@@ -30,10 +30,18 @@
private $changeTransmitters;
/**
+ * @var CentralIdLookup|null
+ */
+ private $centralIdLookup;
+
+ /**
* @param EntityChangeFactory $changeFactory
* @param ChangeTransmitter[] $changeTransmitters
+ * @param CentralIdLookup|null $centralIdLookup CentralIdLookup, or
null if
+ * this repository is not connected to a central user system (see
+ * Wikibase\Lib\Changes\CentralIdLookupFactory).
*/
- public function __construct( EntityChangeFactory $changeFactory, array
$changeTransmitters ) {
+ public function __construct( EntityChangeFactory $changeFactory, array
$changeTransmitters, $centralIdLookup ) {
Assert::parameterElementType(
ChangeTransmitter::class,
$changeTransmitters,
@@ -42,6 +50,11 @@
$this->changeFactory = $changeFactory;
$this->changeTransmitters = $changeTransmitters;
+
+ // There is no type hint because it is a required parameter
that allows
+ // nulls.
+ Assert::parameterType( 'CentralIdLookup|null',
$centralIdLookup, '$centralIdLookup' );
+ $this->centralIdLookup = $centralIdLookup;
}
/**
@@ -64,7 +77,10 @@
$change = $this->changeFactory->newFromUpdate(
EntityChange::REMOVE, $content->getEntity() );
$change->setTimestamp( $timestamp );
- $change->setMetadataFromUser( $user );
+ $change->setMetadataFromUser(
+ $user,
+ $this->getCentralUserId( $user )
+ );
$this->transmitChange( $change );
@@ -89,13 +105,21 @@
}
$change = $this->changeFactory->newFromUpdate(
EntityChange::RESTORE, null, $content->getEntity() );
- $change->setRevisionInfo( $revision );
+
+ $change->setRevisionInfo(
+ $revision,
+ /* Will get set below in setMetadataFromUser */ 0
+ );
+
// We don't want the change entries of newly undeleted pages to
have
// the timestamp of the original change.
$change->setTimestamp( wfTimestampNow() );
$user = User::newFromId( $revision->getUser() );
- $change->setMetadataFromUser( $user );
+ $change->setMetadataFromUser(
+ $user,
+ $this->getCentralUserId( $user )
+ );
$this->transmitChange( $change );
@@ -123,7 +147,10 @@
}
$change = $this->changeFactory->newFromUpdate(
EntityChange::ADD, null, $content->getEntity() );
- $change->setRevisionInfo( $revision );
+ $change->setRevisionInfo(
+ $revision,
+ $this->getCentralUserId( User::newFromId(
$revision->getUser() ) )
+ );
// FIXME: RepoHooks::onRecentChangeSave currently adds to the
change later!
$this->transmitChange( $change );
@@ -154,7 +181,10 @@
return null;
}
- $change->setRevisionInfo( $current );
+ $change->setRevisionInfo(
+ $current,
+ $this->getCentralUserId( User::newFromId(
$current->getUser() ) )
+ );
// FIXME: RepoHooks::onRecentChangeSave currently adds to the
change later!
$this->transmitChange( $change );
@@ -163,6 +193,22 @@
}
/**
+ * Gets central user ID, or 0, from user
+ *
+ * @param User $user Repository user
+ * @return int Central user ID, or 0
+ */
+ protected function getCentralUserId( User $user ) {
+ if ( $this->centralIdLookup === null ) {
+ return 0;
+ } else {
+ return $this->centralIdLookup->centralIdFromLocalUser(
+ $user
+ );
+ }
+ }
+
+ /**
* Returns a EntityChange based on the old and new content object,
taking
* redirects into consideration.
*
diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php
index 8fdb534..1c86c28 100644
--- a/repo/includes/WikibaseRepo.php
+++ b/repo/includes/WikibaseRepo.php
@@ -2,6 +2,7 @@
namespace Wikibase\Repo;
+use Wikibase\Lib\Changes\CentralIdLookupFactory;
use DataTypes\DataTypeFactory;
use DataValues\DataValueFactory;
use DataValues\Deserializers\DataValueDeserializer;
@@ -1294,7 +1295,11 @@
$transmitters[] = new DatabaseChangeTransmitter(
$this->getStore()->getChangeStore() );
}
- return new ChangeNotifier( $this->getEntityChangeFactory(),
$transmitters );
+ return new ChangeNotifier(
+ $this->getEntityChangeFactory(),
+ $transmitters,
+
CentralIdLookupFactory::getInstance()->getCentralIdLookup()
+ );
}
/**
diff --git a/repo/tests/phpunit/includes/Notifications/ChangeNotifierTest.php
b/repo/tests/phpunit/includes/Notifications/ChangeNotifierTest.php
index 8c064ff..81d98db 100644
--- a/repo/tests/phpunit/includes/Notifications/ChangeNotifierTest.php
+++ b/repo/tests/phpunit/includes/Notifications/ChangeNotifierTest.php
@@ -14,6 +14,7 @@
use Wikibase\Repo\Notifications\ChangeNotifier;
use Wikibase\Repo\Notifications\ChangeTransmitter;
use Wikibase\Repo\WikibaseRepo;
+use Wikibase\Lib\Tests\Changes\MockRepoClientCentralIdLookup;
/**
* @covers Wikibase\Repo\Notifications\ChangeNotifier
@@ -34,7 +35,13 @@
->method( 'transmitChange' );
$changeFactory =
WikibaseRepo::getDefaultInstance()->getEntityChangeFactory();
- return new ChangeNotifier( $changeFactory, [ $changeTransmitter
] );
+ return new ChangeNotifier(
+ $changeFactory,
+ [ $changeTransmitter ],
+ new MockRepoClientCentralIdLookup(
+ /** isRepo= */ true
+ )
+ );
}
/**
diff --git a/repo/tests/phpunit/includes/Store/Sql/SqlChangeStoreTest.php
b/repo/tests/phpunit/includes/Store/Sql/SqlChangeStoreTest.php
index cb4cd5e..78c0bea 100644
--- a/repo/tests/phpunit/includes/Store/Sql/SqlChangeStoreTest.php
+++ b/repo/tests/phpunit/includes/Store/Sql/SqlChangeStoreTest.php
@@ -48,7 +48,7 @@
] );
$changeWithDataFromRC = $factory->newForEntity(
EntityChange::REMOVE, new ItemId( 'Q123' ) );
- $changeWithDataFromRC->setMetadataFromRC( $rc );
+ $changeWithDataFromRC->setMetadataFromRC( $rc, 9 );
return [
'Simple change' => [
@@ -80,8 +80,8 @@
'change_object_id' => 'Q123',
'change_revision_id' => '343',
'change_user_id' => '34',
- 'change_info' =>
'{"metadata":{"user_text":"BlackMagicIsEvil","bot":0,"page_id":2354,"rev_id":343,'
.
-
'"parent_id":897,"comment":"Fake data!"}}',
+ 'change_info' =>
'{"metadata":{"bot":0,"page_id":2354,"rev_id":343,' .
+
'"parent_id":897,"comment":"Fake
data!","user_text":"BlackMagicIsEvil","central_user_id":9}}',
],
$changeWithDataFromRC
]
--
To view, visit https://gerrit.wikimedia.org/r/349977
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie7b9c482cf6a0dd7215b34841efd86fb51be651a
Gerrit-PatchSet: 17
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aleksey Bekh-Ivanov (WMDE) <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Danny B. <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: Sbisson <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
