Ladsgroup has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/393286 )
Change subject: Transmit compact diff instead of suppressed diff
......................................................................
Transmit compact diff instead of suppressed diff
Bug: T113468
Change-Id: I83c611866750f646a0591f3d4d5c873615af0998
---
M client/includes/Changes/AffectedPagesFinder.php
M client/tests/phpunit/includes/Changes/AffectedPagesFinderTest.php
M lib/includes/Changes/DiffChange.php
M lib/includes/Changes/EntityChange.php
M lib/includes/Changes/EntityChangeFactory.php
5 files changed, 46 insertions(+), 51 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/86/393286/1
diff --git a/client/includes/Changes/AffectedPagesFinder.php
b/client/includes/Changes/AffectedPagesFinder.php
index 99632b5..4456545 100644
--- a/client/includes/Changes/AffectedPagesFinder.php
+++ b/client/includes/Changes/AffectedPagesFinder.php
@@ -115,21 +115,21 @@
}
/**
- * @param EntityChange|EntityDiffChangedAspects $change
+ * @param EntityChange $change
*
* @return string[]
*/
- public function getChangedAspects( $change ) {
+ public function getChangedAspects( EntityChange $change ) {
$aspects = [];
- if ( $change instanceof EntityChange ) {
- $diff = $change->getDiff();
- $diffAspects = ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff( $diff );
- } elseif ( $change instanceof EntityDiffChangedAspects ) {
- $diffAspects = $change;
+ $info = $change->getInfo();
+ // We might unserialize old EntityChange which doesn't have
getAspectsDiff method
+ if ( array_key_exists( 'compactDiff', $info ) ) {
+ $diffAspects = $change->getAspectsDiff();
} else {
- throw new InvalidArgumentException(
'AffectedPagesFinder::getChangedAspects accepts ' .
- 'EntityChange or EntityDiffChangedAspects' );
+ $diffAspects = ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff(
+ $change->getDiff()
+ );
}
if ( $diffAspects->getSiteLinkChanges() !== [] ) {
diff --git a/client/tests/phpunit/includes/Changes/AffectedPagesFinderTest.php
b/client/tests/phpunit/includes/Changes/AffectedPagesFinderTest.php
index 59c9432..8997d85 100644
--- a/client/tests/phpunit/includes/Changes/AffectedPagesFinderTest.php
+++ b/client/tests/phpunit/includes/Changes/AffectedPagesFinderTest.php
@@ -223,24 +223,6 @@
$this->assertEquals( $expected, $actual );
}
- /**
- * @dataProvider getChangedAspectsProvider
- */
- public function testGetChangedAspectsUsingEntityChangeAspects(
- array $expected,
- EntityChange $change,
- $trackUsagesInAllLanguages = false
- ) {
- $referencedPagesFinder = $this->getAffectedPagesFinder( [], [],
$trackUsagesInAllLanguages );
-
- $aspects = ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff( $change->getDiff() );
- $actual = $referencedPagesFinder->getChangedAspects( $aspects );
-
- sort( $expected );
- sort( $actual );
- $this->assertEquals( $expected, $actual );
- }
-
public function getAffectedUsagesByPageProvider() {
$labelUsageDe = EntityUsage::LABEL_USAGE . '.de';
$labelUsageEn = EntityUsage::LABEL_USAGE . '.en';
diff --git a/lib/includes/Changes/DiffChange.php
b/lib/includes/Changes/DiffChange.php
index 7c651f6..3009050 100644
--- a/lib/includes/Changes/DiffChange.php
+++ b/lib/includes/Changes/DiffChange.php
@@ -3,6 +3,8 @@
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.
@@ -37,4 +39,28 @@
$this->setField( 'info', $info );
}
+ /**
+ * @param string $cache set to 'cache' to cache the unserialized
aspects diff.
+ *
+ * @return EntityDiffChangedAspects
+ */
+ public function getAspectsDiff( $cache = 'no' ) {
+ $info = $this->getInfo( $cache );
+
+ if ( !array_key_exists( 'compactDiff', $info ) ) {
+ // 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() );
+ } else {
+ return $info['compactDiff'];
+ }
+ }
+
+ public function setAspectsDiff( EntityDiffChangedAspects $diff ) {
+ $info = $this->getInfo();
+ $info['compactDiff'] = $diff;
+ $this->setField( 'info', $info );
+ }
+
}
diff --git a/lib/includes/Changes/EntityChange.php
b/lib/includes/Changes/EntityChange.php
index 9c62ac9..96571dd 100644
--- a/lib/includes/Changes/EntityChange.php
+++ b/lib/includes/Changes/EntityChange.php
@@ -16,6 +16,7 @@
use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Services\Diff\EntityTypeAwareDiffOpFactory;
use Wikibase\DataModel\Statement\Statement;
+use Wikibase\Lib\Changes\EntityDiffChangedAspects;
use Wikibase\Repo\WikibaseRepo;
/**
@@ -292,6 +293,14 @@
}
}
+ if ( isset( $info['compactDiff'] ) ) {
+ $diff = $info['compactDiff'];
+
+ if ( $diff instanceof EntityDiffChangedAspects ) {
+ $info['compactDiff'] = serialize( $diff );
+ }
+ }
+
// Make sure we never serialize objects.
// This is a lot of overhead, so we only do it during testing.
if ( defined( 'MW_PHPUNIT_TEST' ) ) {
diff --git a/lib/includes/Changes/EntityChangeFactory.php
b/lib/includes/Changes/EntityChangeFactory.php
index 82c2aa9..c5a945e 100644
--- a/lib/includes/Changes/EntityChangeFactory.php
+++ b/lib/includes/Changes/EntityChangeFactory.php
@@ -140,9 +140,6 @@
throw new MWException( 'Either $oldEntity or $newEntity
must be given' );
}
- $this->minimizeEntityForDiffing( $oldEntity );
- $this->minimizeEntityForDiffing( $newEntity );
-
if ( $oldEntity === null ) {
$id = $newEntity->getId();
$diff = $this->entityDiffer->getConstructionDiff(
$newEntity );
@@ -157,29 +154,10 @@
}
$instance = $this->newForEntity( $action, $id );
- $instance->setDiff( $diff );
+ $aspectsDiff = ( new EntityDiffChangedAspectsFactory()
)->newFromEntityDiff( $diff );
+ $instance->setAspectsDiff( $aspectsDiff );
return $instance;
- }
-
- /**
- * Hack: Don't include statement, description and alias diffs, since
those are unused and not
- * helpful performance-wise to the dispatcher and change handling.
- *
- * @fixme Implement T113468 and remove this.
- *
- * @param EntityDocument|null $entity
- */
- private function minimizeEntityForDiffing( EntityDocument $entity =
null ) {
- if ( $entity instanceof StatementListProvider ) {
- $entity->getStatements()->clear();
- }
-
- if ( $entity instanceof FingerprintProvider ) {
- $fingerprint = $entity->getFingerprint();
-
- $fingerprint->setAliasGroups( new AliasGroupList() );
- }
}
}
--
To view, visit https://gerrit.wikimedia.org/r/393286
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I83c611866750f646a0591f3d4d5c873615af0998
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Ladsgroup <ladsgr...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
