Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/235689
Change subject: Clean up EntityChangeFactory
......................................................................
Clean up EntityChangeFactory
This fixes an issue I found while reviewing I8f646c1. The type of the
two objects is guaranteed to be the same, that's why checking one of
them is enough.
I also tried to clean up some more code, simplify data flow (e.g.
pass an EntityId instead of an Entity) and such in the same class.
Change-Id: I56dc68c153044c04e5f115025eaa7d696abf95ff
---
M lib/includes/changes/EntityChangeFactory.php
M lib/tests/phpunit/changes/EntityChangeFactoryTest.php
2 files changed, 26 insertions(+), 22 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/89/235689/1
diff --git a/lib/includes/changes/EntityChangeFactory.php
b/lib/includes/changes/EntityChangeFactory.php
index 20b186a..78fdeb1 100644
--- a/lib/includes/changes/EntityChangeFactory.php
+++ b/lib/includes/changes/EntityChangeFactory.php
@@ -2,13 +2,13 @@
namespace Wikibase\Lib\Changes;
-use InvalidArgumentException;
use MWException;
use Wikibase\ChangesTable;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Services\Diff\EntityDiffer;
use Wikibase\DataModel\Statement\StatementList;
+use Wikibase\DataModel\Statement\StatementListHolder;
use Wikibase\EntityChange;
use Wikibase\EntityFactory;
@@ -23,7 +23,7 @@
class EntityChangeFactory {
/**
- * @var array maps entity type IDs to subclasses of EntityChange
+ * @var string[] Maps entity type IDs to subclasses of EntityChange.
*/
private $changeClasses;
@@ -46,14 +46,15 @@
* @param ChangesTable $changesTable
* @param EntityFactory $entityFactory
* @param EntityDiffer $entityDiffer
- * @param array $changeClasses maps entity type IDs to subclasses of
EntityChange.
+ * @param string[] $changeClasses Maps entity type IDs to subclasses of
EntityChange.
* Entity types not mapped explicitly are assumed to use EntityChange
itself.
- *
- * @throws \InvalidArgumentException
*/
- public function __construct( ChangesTable $changesTable, EntityFactory
$entityFactory,
- EntityDiffer $entityDiffer, array $changeClasses = array() ) {
-
+ public function __construct(
+ ChangesTable $changesTable,
+ EntityFactory $entityFactory,
+ EntityDiffer $entityDiffer,
+ array $changeClasses = array()
+ ) {
$this->changeClasses = $changeClasses;
$this->changesTable = $changesTable;
$this->entityFactory = $entityFactory;
@@ -90,8 +91,7 @@
}
if ( !$instance->hasField( 'info' ) ) {
- $info = array();
- $instance->setField( 'info', $info );
+ $instance->setField( 'info', array() );
}
// Note: the change type determines how the client will
@@ -115,34 +115,39 @@
* @return EntityChange
* @throws MWException
*/
- public function newFromUpdate( $action, EntityDocument $oldEntity =
null, EntityDocument $newEntity = null, array $fields = null ) {
+ public function newFromUpdate(
+ $action,
+ EntityDocument $oldEntity = null,
+ EntityDocument $newEntity = null,
+ array $fields = null
+ ) {
if ( $oldEntity === null && $newEntity === null ) {
- throw new MWException( 'Either $oldEntity or $newEntity
must be give.' );
+ throw new MWException( 'Either $oldEntity or $newEntity
must be given' );
}
if ( $oldEntity === null ) {
$oldEntity = $this->entityFactory->newEmpty(
$newEntity->getType() );
- $theEntity = $newEntity;
+ $id = $newEntity->getId();
} elseif ( $newEntity === null ) {
$newEntity = $this->entityFactory->newEmpty(
$oldEntity->getType() );
- $theEntity = $oldEntity;
+ $id = $oldEntity->getId();
} elseif ( $oldEntity->getType() !== $newEntity->getType() ) {
throw new MWException( 'Entity type mismatch' );
} else {
- $theEntity = $newEntity;
+ $id = $newEntity->getId();
}
// don't include statements diff, since those are unused and
not helpful
// performance-wise to the dispatcher and change handling.
- $oldEntity->setStatements( new StatementList() );
- $newEntity->setStatements( new StatementList() );
+ if ( $oldEntity instanceof StatementListHolder ) {
+ $oldEntity->setStatements( new StatementList() );
+ $newEntity->setStatements( new StatementList() );
+ }
$diff = $this->entityDiffer->diffEntities( $oldEntity,
$newEntity );
- /**
- * @var EntityChange $instance
- */
- $instance = self::newForEntity( $action, $theEntity->getId(),
$fields );
+ /** @var EntityChange $instance */
+ $instance = self::newForEntity( $action, $id, $fields );
$instance->setDiff( $diff );
return $instance;
diff --git a/lib/tests/phpunit/changes/EntityChangeFactoryTest.php
b/lib/tests/phpunit/changes/EntityChangeFactoryTest.php
index 80530d8..6882e93 100644
--- a/lib/tests/phpunit/changes/EntityChangeFactoryTest.php
+++ b/lib/tests/phpunit/changes/EntityChangeFactoryTest.php
@@ -6,7 +6,6 @@
use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpRemove;
use Wikibase\ChangesTable;
-use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
--
To view, visit https://gerrit.wikimedia.org/r/235689
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I56dc68c153044c04e5f115025eaa7d696abf95ff
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
