jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/369881 )
Change subject: Fix handling of coalesced changes in InjectRCRecordsJob
......................................................................
Fix handling of coalesced changes in InjectRCRecordsJob
Instead of representing changes using their IDs, we represent them
using their complete field data.
However, composite changes represent their children using their IDs.
Bug: T171370
Bug: T172394
Change-Id: Iaa98722779507bb57d449649f1147314da685312
---
M client/WikibaseClient.php
M client/includes/Changes/InjectRCRecordsJob.php
M client/includes/WikibaseClient.php
M client/tests/phpunit/includes/Changes/InjectRCRecordsJobTest.php
M client/tests/phpunit/includes/Changes/WikiPageUpdaterTest.php
M client/tests/phpunit/includes/WikibaseClientTest.php
M lib/includes/Changes/ChangeRow.php
M lib/includes/Changes/EntityChange.php
M lib/includes/Changes/EntityChangeFactory.php
M lib/tests/phpunit/Changes/EntityChangeFactoryTest.php
M lib/tests/phpunit/Changes/EntityChangeTest.php
M lib/tests/phpunit/Changes/TestChanges.php
M lib/tests/phpunit/Store/Sql/EntityChangeLookupTest.php
M repo/includes/WikibaseRepo.php
14 files changed, 427 insertions(+), 21 deletions(-)
Approvals:
jenkins-bot: Verified
Thiemo Mättig (WMDE): Looks good to me, approved
diff --git a/client/WikibaseClient.php b/client/WikibaseClient.php
index 012905d..836ac30 100644
--- a/client/WikibaseClient.php
+++ b/client/WikibaseClient.php
@@ -189,6 +189,7 @@
$job = new Wikibase\Client\Changes\InjectRCRecordsJob(
$mwServices->getDBLoadBalancerFactory(),
$wbServices->getStore()->getEntityChangeLookup(),
+ $wbServices->getEntityChangeFactory(),
$wbServices->getRecentChangeFactory(),
$params
);
diff --git a/client/includes/Changes/InjectRCRecordsJob.php
b/client/includes/Changes/InjectRCRecordsJob.php
index 54c4781..6a50900 100644
--- a/client/includes/Changes/InjectRCRecordsJob.php
+++ b/client/includes/Changes/InjectRCRecordsJob.php
@@ -14,6 +14,7 @@
use Wikibase\Client\Store\TitleFactory;
use Wikibase\Client\WikibaseClient;
use Wikibase\EntityChange;
+use Wikibase\Lib\Changes\EntityChangeFactory;
use Wikibase\Lib\Store\Sql\EntityChangeLookup;
use Wikimedia\Assert\Assert;
use Wikimedia\Rdbms\LBFactory;
@@ -37,6 +38,11 @@
* @var EntityChangeLookup
*/
private $changeLookup;
+
+ /**
+ * @var EntityChangeFactory
+ */
+ private $changeFactory;
/**
* @var RecentChangeFactory
@@ -82,8 +88,13 @@
$pages[$id] = [ $t->getNamespace(), $t->getDBkey() ];
}
+ // Note: Avoid serializing Change objects. Original changes can
be restored
+ // from $changeData['info']['change-ids'], see getChange().
+ $changeData = $change->getFields();
+ $changeData['info'] = $change->getSerializedInfo( [ 'changes' ]
);
+
$params = [
- 'change' => $change->getId(),
+ 'change' => $changeData,
'pages' => $pages
];
@@ -91,7 +102,6 @@
'wikibase-InjectRCRecords',
$params
);
-
}
/**
@@ -100,6 +110,7 @@
*
* @param LBFactory $lbFactory
* @param EntityChangeLookup $changeLookup
+ * @param EntityChangeFactory $changeFactory
* @param RecentChangeFactory $rcFactory
* @param array $params Needs to have two keys: "change": the id of the
change,
* "pages": array of pages, represented as $pageId => [ $namespace,
$dbKey ].
@@ -107,6 +118,7 @@
public function __construct(
LBFactory $lbFactory,
EntityChangeLookup $changeLookup,
+ EntityChangeFactory $changeFactory,
RecentChangeFactory $rcFactory,
array $params
) {
@@ -118,6 +130,13 @@
'$params',
'$params[\'change\'] not set.'
);
+ // TODO: disallow integer once T172394 has been deployed and
old jobs have cleared the queue.
+ Assert::parameterType(
+ 'integer|array',
+ $params['change'],
+ '$params[\'change\']'
+ );
+
Assert::parameter(
isset( $params['pages'] ),
'$params',
@@ -131,6 +150,7 @@
$this->lbFactory = $lbFactory;
$this->changeLookup = $changeLookup;
+ $this->changeFactory = $changeFactory;
$this->rcFactory = $rcFactory;
$this->titleFactory = new TitleFactory();
@@ -182,16 +202,31 @@
*/
private function getChange() {
$params = $this->getParams();
- $changeId = $params['change'];
+ $changeData = $params['change'];
- $this->logger->debug( __FUNCTION__ . ": loading change
$changeId." );
+ if ( is_int( $changeData ) ) {
+ // TODO: this can be removed once T172394 has been
deployed
+ // and old jobs have cleared the queue.
+ $this->logger->debug( __FUNCTION__ . ": loading change
$changeData." );
- $changes = $this->changeLookup->loadByChangeIds( [ $changeId ]
);
+ $changes = $this->changeLookup->loadByChangeIds( [
$changeData ] );
- $change = reset( $changes );
+ $change = reset( $changes );
- if ( !$change ) {
- $this->logger->error( __FUNCTION__ . ": failed to load
change $changeId." );
+ if ( !$change ) {
+ $this->logger->error( __FUNCTION__ . ": failed
to load change $changeData." );
+ }
+ } else {
+ $change = $this->changeFactory->newFromFieldData(
$params['change'] );
+
+ // If the current change was composed of other child
changes, restore the
+ // child objects.
+ $info = $change->getInfo();
+ if ( isset( $info['change-ids'] ) && !isset(
$info['changes'] ) ) {
+ $children =
$this->changeLookup->loadByChangeIds( $info['change-ids'] );
+ $info['changes'] = $children;
+ $change->setField( 'info', $info );
+ }
}
return $change;
@@ -209,7 +244,7 @@
$titles = [];
foreach ( $pages as $pageId => list( $namespace, $dbKey ) ) {
- $titles[] = $this->titleFactory->makeTitle( $namespace,
$dbKey );
+ $titles[$pageId] = $this->titleFactory->makeTitle(
$namespace, $dbKey );
}
return $titles;
diff --git a/client/includes/WikibaseClient.php
b/client/includes/WikibaseClient.php
index 083f143..164f09f 100644
--- a/client/includes/WikibaseClient.php
+++ b/client/includes/WikibaseClient.php
@@ -1072,7 +1072,7 @@
/**
* @return EntityChangeFactory
*/
- private function getEntityChangeFactory() {
+ public function getEntityChangeFactory() {
//TODO: take this from a setting or registry.
$changeClasses = [
Item::ENTITY_TYPE => ItemChange::class,
@@ -1081,6 +1081,7 @@
return new EntityChangeFactory(
$this->getEntityDiffer(),
+ $this->getEntityIdParser(),
$changeClasses
);
}
diff --git a/client/tests/phpunit/includes/Changes/InjectRCRecordsJobTest.php
b/client/tests/phpunit/includes/Changes/InjectRCRecordsJobTest.php
index e182b76..41c54bf 100644
--- a/client/tests/phpunit/includes/Changes/InjectRCRecordsJobTest.php
+++ b/client/tests/phpunit/includes/Changes/InjectRCRecordsJobTest.php
@@ -9,9 +9,19 @@
use Wikibase\Client\RecentChanges\RecentChangeFactory;
use Wikibase\Client\RecentChanges\RecentChangesDuplicateDetector;
use Wikibase\Client\Store\TitleFactory;
+use Wikibase\Client\WikibaseClient;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
+use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Services\Diff\EntityDiffer;
+use Wikibase\DataModel\Services\Diff\ItemDiff;
use Wikibase\EntityChange;
+use Wikibase\ItemChange;
+use Wikibase\Lib\Changes\EntityChangeFactory;
use Wikibase\Lib\Store\Sql\EntityChangeLookup;
+use Wikibase\Lib\Tests\Changes\EntityChangeFactoryTest;
use Wikimedia\Rdbms\LBFactory;
+use Wikimedia\TestingAccessWrapper;
/**
* @covers Wikibase\Client\Changes\InjectRCRecordsJob
@@ -43,21 +53,39 @@
}
/**
- * @param EntityChange $change
+ * @param EntityChange[] $knownChanges
*
* @return PHPUnit_Framework_MockObject_MockObject|EntityChangeLookup
*/
- private function getEntityChangeLookupMock( EntityChange $change ) {
+ private function getEntityChangeLookupMock( array $knownChanges = [] ) {
$changeLookup = $this->getMockBuilder(
EntityChangeLookup::class )
->disableOriginalConstructor()
->getMock();
+ $changes = [];
+ foreach ( $knownChanges as $change ) {
+ $id = $change->getId();
+ $changes[$id] = $change;
+ }
+
$changeLookup->expects( $this->any() )
->method( 'loadByChangeIds' )
- ->with( [ $change->getId() ] )
- ->will( $this->returnValue( [ $change ] ) );
+ ->will( $this->returnCallback( function ( $ids ) use (
$changes ) {
+ return array_values( array_intersect_key(
$changes, array_flip( $ids ) ) );
+ } ) );
return $changeLookup;
+ }
+
+ /**
+ * @return EntityChangeFactory
+ */
+ private function getEntityChangeFactory() {
+ return new EntityChangeFactory(
+ new EntityDiffer(),
+ new BasicEntityIdParser(),
+ [ Item::ENTITY_TYPE => ItemChange::class ]
+ );
}
/**
@@ -111,6 +139,10 @@
->will( $this->returnValue( $text ) );
$title->expects( $this->any() )
+ ->method( 'getPrefixedDBkey' )
+ ->will( $this->returnValue( $text ) );
+
+ $title->expects( $this->any() )
->method( 'getNamespace' )
->will( $this->returnValue( 0 ) );
@@ -119,10 +151,13 @@
/**
* @param int $id
+ * @param array $fields
*
* @return PHPUnit_Framework_MockObject_MockObject|EntityChange
*/
- private function getEntityChangeMock( $id = 77 ) {
+ private function getEntityChangeMock( $id = 77, array $fields = [] ) {
+ $info = isset( $fields['info'] ) ? $fields['info'] : [];
+
$change = $this->getMockBuilder( EntityChange::class )
->disableOriginalConstructor()
->getMock();
@@ -130,6 +165,18 @@
$change->expects( $this->any() )
->method( 'getId' )
->will( $this->returnValue( $id ) );
+
+ $change->expects( $this->any() )
+ ->method( 'getFields' )
+ ->will( $this->returnValue( $fields ) );
+
+ $change->expects( $this->any() )
+ ->method( 'getInfo' )
+ ->will( $this->returnValue( $info ) );
+
+ $change->expects( $this->any() )
+ ->method( 'getSerializedInfo' )
+ ->will( $this->returnValue( json_encode( $info ) ) );
return $change;
}
@@ -156,12 +203,221 @@
return $LBFactory;
}
+ public function provideMakeJobSpecification() {
+ $title = $this->getTitleMock( 'Foo', 21 );
+ $changeFactory = $this->getEntityChangeFactory();
+ $itemId = new ItemId( 'Q7' );
+
+ $child1 = $changeFactory->newFromFieldData( [
+ 'id' => 101,
+ 'type' => 'wikibase-item~change',
+ 'object_id' => $itemId->getSerialization(),
+ ] );
+
+ $child2 = $changeFactory->newFromFieldData( [
+ 'id' => 102,
+ 'type' => 'wikibase-item~change',
+ 'object_id' => $itemId->getSerialization(),
+ ] );
+
+ $diff = new ItemDiff();
+
+ return [
+ 'mock change' => [
+ [ $title ],
+ $this->getEntityChangeMock(
+ 17,
+ [
+ 'id' => 17,
+ 'object_id' => 'Q7',
+ 'type' =>
'wikibase-item~change',
+ 'Test' => 'Kitten',
+ 'info' => []
+ ]
+ ),
+ ],
+ 'simple change with ID' => [
+ [ $title ],
+ $changeFactory->newForEntity(
+ 'change',
+ new ItemId( 'Q7' ),
+ [ 'id' => 7 ]
+ ),
+ ],
+ 'simple change with data but no ID' => [
+ [ $title ],
+ $changeFactory->newForEntity(
+ 'change',
+ $itemId,
+ [
+ 'type' =>
'wikibase-item~change',
+ 'object_id' =>
$itemId->getSerialization(),
+ 'info' => [
+ 'diff' => $diff,
+ 'stuff' => 'Fun Stuff',
+ 'metadata' => [
+ 'foo' =>
'Kitten'
+ ]
+ ]
+ ]
+ ),
+ ],
+ 'composite change without ID' => [
+ [ $title ],
+ $changeFactory->newForEntity(
+ 'change',
+ $itemId,
+ [
+ 'type' =>
'wikibase-item~change',
+ 'object_id' =>
$itemId->getSerialization(),
+ 'info' => [
+ 'diff' => $diff,
+ 'change-ids' => [ 101,
102 ],
+ 'changes' => [ $child1,
$child2 ],
+ ]
+ ]
+ ),
+ [ $child1, $child2 ]
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideMakeJobSpecification()
+ * @param Title[] $titles
+ * @param EntityChange $change
+ */
+ public function testMakeJobSpecification( array $titles, EntityChange
$change, array $knownChanges = [] ) {
+ $spec = InjectRCRecordsJob::makeJobSpecification( $titles,
$change );
+
+ $changeLookup = $this->getEntityChangeLookupMock( $knownChanges
);
+ $changeFactory = $this->getEntityChangeFactory();
+ $rcFactory = $this->getRCFactoryMock();
+
+ $job = new InjectRCRecordsJob(
+ $this->getLBFactoryMock(),
+ $changeLookup,
+ $changeFactory,
+ $rcFactory,
+ $spec->getParams()
+ );
+
+ $actualChange = TestingAccessWrapper::newFromObject( $job
)->getChange();
+
+ $this->assertEquals( $change->getId(), $actualChange->getId(),
'Change ID' );
+ $this->assertEquals( $change->getFields(),
$actualChange->getFields(), 'Change Fields' );
+
+ $actualTitles = TestingAccessWrapper::newFromObject( $job
)->getTitles();
+
+ $this->assertEquals(
+ $this->getTitleIDs( $titles ),
+ array_keys( $actualTitles ),
+ 'Title ID'
+ );
+ $this->assertEquals(
+ $this->getTitleDBKeys( $titles ),
+ array_values( $this->getTitleDBKeys( $actualTitles ) ),
+ 'Title DBKey'
+ );
+ }
+
+ public function provideConstruction() {
+ $change = $this->getEntityChangeMock(
+ 17,
+ [
+ 'id' => 17,
+ 'object_id' => 'Q7',
+ 'type' => 'wikibase-item~change',
+ 'Test' => 'Kitten',
+ 'info' => []
+ ]
+ );
+ $title = $this->getTitleMock( 'Foo', 21 );
+
+ return [
+ // TODO: drop the change ID test case once T172394 has
been deployed
+ // and old jobs have cleared the queue.
+ 'job spec using change ID' => [
+ [
+ 'change' => $change->getId(),
+ 'pages' => $this->getPageSpecData( [
$title ] )
+ ],
+ $change,
+ [ $title ],
+ ],
+ 'job spec using change field data' => [
+ [
+ 'change' => $change->getFields(),
+ 'pages' => $this->getPageSpecData( [
$title ] )
+ ],
+ $change,
+ [ $title ],
+ ],
+ ];
+ }
+
+ /**
+ * @param Title[] $titles
+ *
+ * @return array[]
+ */
+ private function getPageSpecData( array $titles ) {
+ $pages = [];
+
+ foreach ( $titles as $title ) {
+ $id = $title->getArticleId();
+ $pages[$id] = [ $title->getNamespace(),
$title->getDBkey() ];
+ }
+
+ return $pages;
+ }
+
+ /**
+ * @dataProvider provideConstruction()
+ */
+ public function testConstruction(
+ array $params,
+ EntityChange $expectedChange,
+ array $expectedTitles
+ ) {
+ $changeLookup = $this->getEntityChangeLookupMock( [
$expectedChange ] );
+ $changeFactory = $this->getEntityChangeFactory();
+ $rcFactory = $this->getRCFactoryMock();
+
+ $job = new InjectRCRecordsJob(
+ $this->getLBFactoryMock(),
+ $changeLookup,
+ $changeFactory,
+ $rcFactory,
+ $params
+ );
+
+ $actualChange = TestingAccessWrapper::newFromObject( $job
)->getChange();
+
+ $this->assertEquals( $expectedChange->getId(),
$actualChange->getId(), 'Change ID' );
+ $this->assertEquals( $expectedChange->getFields(),
$actualChange->getFields(), 'Change Fields' );
+
+ $actualTitles = TestingAccessWrapper::newFromObject( $job
)->getTitles();
+
+ $this->assertEquals(
+ $this->getTitleIDs( $expectedTitles ),
+ array_keys( $actualTitles ),
+ 'Title ID'
+ );
+ $this->assertEquals(
+ $this->getTitleDBKeys( $expectedTitles ),
+ array_values( $this->getTitleDBKeys( $actualTitles ) ),
+ 'Title DBKey'
+ );
+ }
+
public function testRun() {
$title = $this->getTitleMock( 'Foo', 21 );
$change = $this->getEntityChangeMock( 17 );
$rc = $this->getRecentChangeMock();
- $changeLookup = $this->getEntityChangeLookupMock( $change );
+ $changeLookup = $this->getEntityChangeLookupMock( [ $change ] );
+ $changeFactory = $this->getEntityChangeFactory();
$rcFactory = $this->getRCFactoryMock();
@@ -186,6 +442,7 @@
$job = new InjectRCRecordsJob(
$this->getLBFactoryMock(),
$changeLookup,
+ $changeFactory,
$rcFactory,
$params
);
@@ -199,7 +456,8 @@
public function testRun_batch() {
$change = $this->getEntityChangeMock();
$rc = $this->getRecentChangeMock();
- $changeLookup = $this->getEntityChangeLookupMock( $change );
+ $changeLookup = $this->getEntityChangeLookupMock( [ $change ] );
+ $changeFactory = $this->getEntityChangeFactory();
$rcFactory = $this->getRCFactoryMock();
@@ -223,6 +481,7 @@
$job = new InjectRCRecordsJob(
$lbFactory,
$changeLookup,
+ $changeFactory,
$rcFactory,
$params
);
@@ -233,4 +492,32 @@
$job->run();
}
+ /**
+ * @param Title[] $titles
+ *
+ * @return int[]
+ */
+ private function getTitleIDs( array $titles ) {
+ return array_map(
+ function( Title $title ) {
+ return $title->getArticleId();
+ },
+ $titles
+ );
+ }
+
+ /**
+ * @param Title[] $titles
+ *
+ * @return string[]
+ */
+ private function getTitleDBKeys( array $titles ) {
+ return array_map(
+ function( Title $title ) {
+ return $title->getPrefixedDBkey();
+ },
+ $titles
+ );
+ }
+
}
diff --git a/client/tests/phpunit/includes/Changes/WikiPageUpdaterTest.php
b/client/tests/phpunit/includes/Changes/WikiPageUpdaterTest.php
index 5315a06..37e33d5 100644
--- a/client/tests/phpunit/includes/Changes/WikiPageUpdaterTest.php
+++ b/client/tests/phpunit/includes/Changes/WikiPageUpdaterTest.php
@@ -123,6 +123,14 @@
->method( 'getId' )
->will( $this->returnValue( $id ) );
+ $change->expects( $this->any() )
+ ->method( 'getFields' )
+ ->will( $this->returnValue( [ 'id' => $id, 'info' => []
] ) );
+
+ $change->expects( $this->any() )
+ ->method( 'getSerializedInfo' )
+ ->will( $this->returnValue( '{}' ) );
+
return $change;
}
@@ -229,7 +237,7 @@
$this->assertArrayHasKey( 'change', $params,
'$params["change"]' );
$this->assertArrayHasKey( 'pages', $params,
'$params["pages"]' );
- $this->assertSame( $change->getId(),
$params["change"] );
+ $this->assertSame( $change->getId(),
$params['change']['id'] );
$pages += $params['pages']; // addition uses
keys, array_merge does not
} ) );
diff --git a/client/tests/phpunit/includes/WikibaseClientTest.php
b/client/tests/phpunit/includes/WikibaseClientTest.php
index 4e51f2a..5465207 100644
--- a/client/tests/phpunit/includes/WikibaseClientTest.php
+++ b/client/tests/phpunit/includes/WikibaseClientTest.php
@@ -29,6 +29,7 @@
use Wikibase\Client\LangLinkHandler;
use Wikibase\LanguageFallbackChain;
use Wikibase\LanguageFallbackChainFactory;
+use Wikibase\Lib\Changes\EntityChangeFactory;
use Wikibase\Lib\ContentLanguages;
use Wikibase\Lib\DataTypeDefinitions;
use Wikibase\Lib\EntityTypeDefinitions;
@@ -299,6 +300,11 @@
$this->assertInstanceOf( RestrictedEntityLookup::class,
$restrictedEntityLookup );
}
+ public function testGetEntityChangeFactory() {
+ $entityChangeFactory =
$this->getWikibaseClient()->getEntityChangeFactory();
+ $this->assertInstanceOf( EntityChangeFactory::class,
$entityChangeFactory );
+ }
+
public function propertyOrderUrlProvider() {
return [
[ 'page-url' ],
diff --git a/lib/includes/Changes/ChangeRow.php
b/lib/includes/Changes/ChangeRow.php
index 8158770..0fc4916 100644
--- a/lib/includes/Changes/ChangeRow.php
+++ b/lib/includes/Changes/ChangeRow.php
@@ -114,9 +114,12 @@
}
/**
+ * @param string[] $skipKeys Keys of the info array to skip during
serialization. Useful for
+ * omitting undesired or unserializable data from the
serialization.
+ *
* @return string JSON
*/
- abstract public function getSerializedInfo();
+ abstract public function getSerializedInfo( $skipKeys = [] );
/**
* Unserializes the info field using json_decode.
diff --git a/lib/includes/Changes/EntityChange.php
b/lib/includes/Changes/EntityChange.php
index 6a8211f..7865fef 100644
--- a/lib/includes/Changes/EntityChange.php
+++ b/lib/includes/Changes/EntityChange.php
@@ -223,11 +223,15 @@
/**
* @see ChangeRow::getSerializedInfo
*
+ * @param string[] $skipKeys
+ *
* @return string JSON
*/
- public function getSerializedInfo() {
+ public function getSerializedInfo( $skipKeys = [] ) {
$info = $this->getInfo();
+ $info = array_diff_key( $info, array_flip( $skipKeys ) );
+
if ( isset( $info['diff'] ) ) {
$diff = $info['diff'];
diff --git a/lib/includes/Changes/EntityChangeFactory.php
b/lib/includes/Changes/EntityChangeFactory.php
index 6fa052d..8fb9411 100644
--- a/lib/includes/Changes/EntityChangeFactory.php
+++ b/lib/includes/Changes/EntityChangeFactory.php
@@ -5,12 +5,14 @@
use MWException;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Services\Diff\EntityDiffer;
use Wikibase\DataModel\Statement\StatementListProvider;
use Wikibase\DataModel\Term\AliasGroupList;
use Wikibase\DataModel\Term\FingerprintProvider;
use Wikibase\DataModel\Term\TermList;
use Wikibase\EntityChange;
+use Wikimedia\Assert\Assert;
/**
* Factory for EntityChange objects
@@ -27,20 +29,28 @@
private $entityDiffer;
/**
+ * @var EntityIdParser
+ */
+ private $idParser;
+
+ /**
* @var string[] Maps entity type IDs to subclasses of EntityChange.
*/
private $changeClasses;
/**
* @param EntityDiffer $entityDiffer
+ * @param EntityIdParser $idParser
* @param string[] $changeClasses Maps entity type IDs to subclasses of
EntityChange.
* Entity types not mapped explicitly are assumed to use EntityChange
itself.
*/
public function __construct(
EntityDiffer $entityDiffer,
+ EntityIdParser $idParser,
array $changeClasses = []
) {
$this->entityDiffer = $entityDiffer;
+ $this->idParser = $idParser;
$this->changeClasses = $changeClasses;
}
@@ -90,6 +100,20 @@
}
/**
+ * @param array $fields all data fields, including at least 'type' and
'object_id'.
+ * @return EntityChange
+ */
+ public function newFromFieldData( array $fields ) {
+ Assert::parameter( isset( $fields['type'] ),
'$fields[\'type\']', 'must be set' );
+ Assert::parameter( isset( $fields['object_id'] ),
'$fields[\'object_id\']', 'must be set' );
+
+ $action = explode( '~', $fields['type'] )[1];
+ $entityId = $this->idParser->parse( $fields['object_id'] );
+
+ return $this->newForEntity( $action, $entityId, $fields );
+ }
+
+ /**
* Constructs an EntityChange from the given old and new Entity.
*
* @param string $action The action name
diff --git a/lib/tests/phpunit/Changes/EntityChangeFactoryTest.php
b/lib/tests/phpunit/Changes/EntityChangeFactoryTest.php
index 3ffbabf..0ca1f57 100644
--- a/lib/tests/phpunit/Changes/EntityChangeFactoryTest.php
+++ b/lib/tests/phpunit/Changes/EntityChangeFactoryTest.php
@@ -5,6 +5,7 @@
use Diff\DiffOp\Diff\Diff;
use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpRemove;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
@@ -41,6 +42,7 @@
$factory = new EntityChangeFactory(
new EntityDiffer(),
+ new BasicEntityIdParser(),
$changeClasses
);
@@ -88,6 +90,26 @@
$this->assertEquals( $entityId, $change->getEntityId() );
}
+ /**
+ * @dataProvider newForEntityProvider
+ *
+ * @param string $action
+ * @param EntityId $entityId
+ * @param string $expectedClass
+ */
+ public function testNewFromFieldData( $action, EntityId $entityId,
$expectedClass ) {
+ $fields = [];
+ $fields['type'] = 'wikibase-' . $entityId->getEntityType() .
'~' . $action;
+ $fields['object_id'] = $entityId->getSerialization();
+
+ $factory = $this->getEntityChangeFactory();
+
+ $change = $factory->newFromFieldData( $fields );
+ $this->assertInstanceOf( $expectedClass, $change );
+ $this->assertEquals( $action, $change->getAction() );
+ $this->assertEquals( $entityId, $change->getEntityId() );
+ }
+
public function testNewFromUpdate() {
$itemId = new ItemId( 'Q1' );
diff --git a/lib/tests/phpunit/Changes/EntityChangeTest.php
b/lib/tests/phpunit/Changes/EntityChangeTest.php
index 9deb658..463b5ba 100644
--- a/lib/tests/phpunit/Changes/EntityChangeTest.php
+++ b/lib/tests/phpunit/Changes/EntityChangeTest.php
@@ -280,6 +280,13 @@
$this->assertSame( $expected, $change->getSerializedInfo() );
}
+ public function testSerializeSkips() {
+ $info = [ 'field' => 'value', 'evil' => 'nope!' ];
+ $expected = '{"field":"value"}';
+ $change = new EntityChange( [ 'info' => $info ] );
+ $this->assertSame( $expected, $change->getSerializedInfo( [
'evil' ] ) );
+ }
+
public function testDoesNotSerializeObjects() {
$info = [ 'array' => [ 'object' => new EntityChange() ] ];
$change = new EntityChange( [ 'info' => $info ] );
diff --git a/lib/tests/phpunit/Changes/TestChanges.php
b/lib/tests/phpunit/Changes/TestChanges.php
index 9cfabdc..e876cc0 100644
--- a/lib/tests/phpunit/Changes/TestChanges.php
+++ b/lib/tests/phpunit/Changes/TestChanges.php
@@ -3,6 +3,7 @@
namespace Wikibase\Lib\Tests\Changes;
use Wikibase\ChangeRow;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\Property;
@@ -37,6 +38,7 @@
$factory = new EntityChangeFactory(
new EntityDiffer(),
+ new BasicEntityIdParser(),
$changeClasses
);
diff --git a/lib/tests/phpunit/Store/Sql/EntityChangeLookupTest.php
b/lib/tests/phpunit/Store/Sql/EntityChangeLookupTest.php
index 23d691f..5c1fb36 100644
--- a/lib/tests/phpunit/Store/Sql/EntityChangeLookupTest.php
+++ b/lib/tests/phpunit/Store/Sql/EntityChangeLookupTest.php
@@ -2,6 +2,7 @@
namespace Wikibase\Lib\Tests\Store\Sql;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\ItemIdParser;
use Wikibase\DataModel\Services\Diff\EntityDiffer;
use Wikibase\EntityChange;
@@ -24,7 +25,11 @@
private function newEntityChangeLookup( $wiki ) {
return new EntityChangeLookup(
- new EntityChangeFactory( new EntityDiffer(), [ 'item'
=> EntityChange::class ] ),
+ new EntityChangeFactory(
+ new EntityDiffer(),
+ new BasicEntityIdParser(),
+ [ 'item' => EntityChange::class ]
+ ),
new ItemIdParser(),
$wiki
);
diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php
index 025bd48..0da1df2 100644
--- a/repo/includes/WikibaseRepo.php
+++ b/repo/includes/WikibaseRepo.php
@@ -639,6 +639,7 @@
return new EntityChangeFactory(
$this->getEntityDiffer(),
+ $this->getEntityIdParser(),
$changeClasses
);
}
--
To view, visit https://gerrit.wikimedia.org/r/369881
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaa98722779507bb57d449649f1147314da685312
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Catrope <r...@wikimedia.org>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
