Ladsgroup has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/394044 )
Change subject: Avoid using EntityDiffChangedAspectsFactory like a static
constructor
......................................................................
Avoid using EntityDiffChangedAspectsFactory like a static constructor
Let's please be honest and use an actual static constructor method when
code really is called like a static constructor method.
I understand that ( new ClassName() )->methodName() is not meant to stay
like this. Which is why I'm changing it right now, before it becomes
technical debt we need to track and come back to.
We can change it back to use the factory (when it starts behaving like
an actual factory) any time. The fact that I'm introducing two static
constructor methods does not block us from doing so in the future.
Bug: T113468
Change-Id: Id17259c2c62f60a73a01d2243e72d0d6203dff6e
---
M client/includes/Changes/AffectedPagesFinder.php
M client/tests/phpunit/includes/Changes/ChangeRunCoalescerTest.php
M lib/includes/Changes/DiffChange.php
M lib/includes/Changes/EntityChange.php
M lib/includes/Changes/EntityChangeFactory.php
M lib/includes/Changes/EntityDiffChangedAspects.php
M lib/includes/Changes/EntityDiffChangedAspectsFactory.php
M lib/tests/phpunit/Changes/EntityDiffChangedAspectsFactoryTest.php
M lib/tests/phpunit/Changes/ItemChangeTest.php
9 files changed, 26 insertions(+), 25 deletions(-)
Approvals:
Ladsgroup: Verified; Looks good to me, approved
diff --git a/client/includes/Changes/AffectedPagesFinder.php
b/client/includes/Changes/AffectedPagesFinder.php
index 134661c..8c2d502 100644
--- a/client/includes/Changes/AffectedPagesFinder.php
+++ b/client/includes/Changes/AffectedPagesFinder.php
@@ -119,7 +119,6 @@
*/
public function getChangedAspects( EntityChange $change ) {
$aspects = [];
-
$diffAspects = $change->getCompactDiff();
if ( $diffAspects->getSiteLinkChanges() !== [] ) {
diff --git a/client/tests/phpunit/includes/Changes/ChangeRunCoalescerTest.php
b/client/tests/phpunit/includes/Changes/ChangeRunCoalescerTest.php
index 8081361..d8773cf 100644
--- a/client/tests/phpunit/includes/Changes/ChangeRunCoalescerTest.php
+++ b/client/tests/phpunit/includes/Changes/ChangeRunCoalescerTest.php
@@ -10,7 +10,7 @@
use Wikibase\DataModel\SiteLink;
use Wikibase\EntityChange;
use Wikibase\ItemChange;
-use Wikibase\Lib\Changes\EntityDiffChangedAspectsFactory;
+use Wikibase\Lib\Changes\EntityDiffChangedAspects;
use Wikibase\Lib\Store\EntityRevisionLookup;
use Wikibase\Lib\Tests\MockRepository;
use Wikibase\Lib\Tests\Changes\TestChanges;
@@ -132,7 +132,7 @@
}
$change->setEntityId( new ItemId( $values['object_id'] ) );
- $diffAspects = ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff( $diff );
+ $diffAspects = EntityDiffChangedAspects::newFromEntityDiff(
$diff );
$change->setCompactDiff( $diffAspects );
return $change;
diff --git a/lib/includes/Changes/DiffChange.php
b/lib/includes/Changes/DiffChange.php
index 281c93d..8bab9a0 100644
--- a/lib/includes/Changes/DiffChange.php
+++ b/lib/includes/Changes/DiffChange.php
@@ -2,9 +2,7 @@
namespace Wikibase;
-use Diff\DiffOp\Diff\Diff;
use Wikibase\Lib\Changes\EntityDiffChangedAspects;
-use Wikibase\Lib\Changes\EntityDiffChangedAspectsFactory;
/**
* Class for changes that can be represented as a Diff.
@@ -25,7 +23,7 @@
// This shouldn't happen, but we should be robust
against corrupt, incomplete
// obsolete instances in the database, etc.
wfLogWarning( 'Cannot get the diff when it has not been
set yet.' );
- return ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff( new Diff() );
+ return EntityDiffChangedAspects::newEmpty();
} else {
return $info['compactDiff'];
}
diff --git a/lib/includes/Changes/EntityChange.php
b/lib/includes/Changes/EntityChange.php
index d8f398d..c696d7a 100644
--- a/lib/includes/Changes/EntityChange.php
+++ b/lib/includes/Changes/EntityChange.php
@@ -2,7 +2,6 @@
namespace Wikibase;
-use Diff\DiffOp\Diff\Diff;
use MWException;
use RecentChange;
use Revision;
@@ -10,7 +9,6 @@
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\Lib\Changes\EntityDiffChangedAspects;
-use Wikibase\Lib\Changes\EntityDiffChangedAspectsFactory;
/**
* Represents a change for an entity; to be extended by various change subtypes
@@ -308,12 +306,8 @@
$info = parent::unserializeInfo( $serialization );
- if ( isset( $info['compactDiff'] ) && is_string(
$info['compactDiff'] ) &&
- $info['compactDiff']
- ) {
- $compactDiff = ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff(
- new Diff()
- );
+ if ( isset( $info['compactDiff'] ) && is_string(
$info['compactDiff'] ) ) {
+ $compactDiff = EntityDiffChangedAspects::newEmpty();
$compactDiff->unserialize( $info['compactDiff'] );
$info['compactDiff'] = $compactDiff;
}
diff --git a/lib/includes/Changes/EntityChangeFactory.php
b/lib/includes/Changes/EntityChangeFactory.php
index ba2b3cd..8284283 100644
--- a/lib/includes/Changes/EntityChangeFactory.php
+++ b/lib/includes/Changes/EntityChangeFactory.php
@@ -151,7 +151,7 @@
}
$instance = $this->newForEntity( $action, $id );
- $aspectsDiff = ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff( $diff );
+ $aspectsDiff = EntityDiffChangedAspects::newFromEntityDiff(
$diff );
$instance->setCompactDiff( $aspectsDiff );
return $instance;
diff --git a/lib/includes/Changes/EntityDiffChangedAspects.php
b/lib/includes/Changes/EntityDiffChangedAspects.php
index 2777364..ae20864 100644
--- a/lib/includes/Changes/EntityDiffChangedAspects.php
+++ b/lib/includes/Changes/EntityDiffChangedAspects.php
@@ -2,6 +2,7 @@
namespace Wikibase\Lib\Changes;
+use Diff\DiffOp\Diff\Diff;
use MWException;
use Serializable;
use Wikimedia\Assert\Assert;
@@ -90,6 +91,22 @@
}
/**
+ * @return self
+ */
+ public static function newEmpty() {
+ return ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff( new Diff() );
+ }
+
+ /**
+ * @param Diff $entityDiff
+ *
+ * @return self
+ */
+ public static function newFromEntityDiff( Diff $entityDiff ) {
+ return ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff( $entityDiff );
+ }
+
+ /**
* Language codes of the labels that changed (added, removed or updated)
*
* @return string[]
diff --git a/lib/includes/Changes/EntityDiffChangedAspectsFactory.php
b/lib/includes/Changes/EntityDiffChangedAspectsFactory.php
index e033352..2bb54c1 100644
--- a/lib/includes/Changes/EntityDiffChangedAspectsFactory.php
+++ b/lib/includes/Changes/EntityDiffChangedAspectsFactory.php
@@ -12,17 +12,14 @@
use Wikibase\DataModel\Services\Diff\ItemDiff;
/**
- * Factory for EntityDiffChangedAspects.
- *
* @license GPL-2.0+
* @author Marius Hoch
*/
class EntityDiffChangedAspectsFactory {
/**
- * Get an EntityDiffChangedAspects instance from an EntityDiff.
- *
* @param Diff $entityDiff
+ *
* @return EntityDiffChangedAspects
*/
public function newFromEntityDiff( Diff $entityDiff ) {
diff --git a/lib/tests/phpunit/Changes/EntityDiffChangedAspectsFactoryTest.php
b/lib/tests/phpunit/Changes/EntityDiffChangedAspectsFactoryTest.php
index a0e87f8..bfa17a2 100644
--- a/lib/tests/phpunit/Changes/EntityDiffChangedAspectsFactoryTest.php
+++ b/lib/tests/phpunit/Changes/EntityDiffChangedAspectsFactoryTest.php
@@ -17,7 +17,6 @@
use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Statement\StatementList;
use Wikibase\Lib\Changes\EntityDiffChangedAspects;
-use Wikibase\Lib\Changes\EntityDiffChangedAspectsFactory;
/**
* @covers Wikibase\Lib\Changes\EntityDiffChangedAspectsFactory
@@ -336,8 +335,7 @@
$entityDiffer = new EntityDiffer();
$entityDiff = $entityDiffer->diffEntities( $oldEntity,
$newEntity );
- $factory = new EntityDiffChangedAspectsFactory();
- $entityDiffChangedAspects = $factory->newFromEntityDiff(
$entityDiff );
+ $entityDiffChangedAspects =
EntityDiffChangedAspects::newFromEntityDiff( $entityDiff );
$actual = $entityDiffChangedAspects->toArray();
$this->sortSubArrays( $actual );
@@ -351,8 +349,7 @@
// Add some unknown change
$entityDiff->addOperations( [ new DiffOpAdd( 1 ) ] );
- $factory = new EntityDiffChangedAspectsFactory();
- $entityDiffChangedAspects = $factory->newFromEntityDiff(
$entityDiff );
+ $entityDiffChangedAspects =
EntityDiffChangedAspects::newFromEntityDiff( $entityDiff );
$actual = $entityDiffChangedAspects->toArray();
$expectedDiff = [
diff --git a/lib/tests/phpunit/Changes/ItemChangeTest.php
b/lib/tests/phpunit/Changes/ItemChangeTest.php
index 67b2e7f..2fce9bb 100644
--- a/lib/tests/phpunit/Changes/ItemChangeTest.php
+++ b/lib/tests/phpunit/Changes/ItemChangeTest.php
@@ -8,7 +8,6 @@
use Wikibase\DataModel\Services\Diff\ItemDiff;
use Wikibase\EntityChange;
use Wikibase\ItemChange;
-use Wikibase\Lib\Changes\EntityDiffChangedAspectsFactory;
/**
* @covers Wikibase\ItemChange
--
To view, visit https://gerrit.wikimedia.org/r/394044
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id17259c2c62f60a73a01d2243e72d0d6203dff6e
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Kreuz (WMDE) <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Ladsgroup <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
