jenkins-bot has submitted this change and it was merged.
Change subject: Consolidate duplicate code in UsageAccumulator classes
......................................................................
Consolidate duplicate code in UsageAccumulator classes
An other possibility is to keep the interface as it is and add a
base class with the shared methods. But this means the base class
can have *more* methods than the interface and I think this is not
a good idea. Therefor I'm proposing this solution.
Change-Id: I3902dfc8e493066fcc9a0337247885884d25d909
---
M client/includes/Usage/HashUsageAccumulator.php
M client/includes/Usage/ParserOutputUsageAccumulator.php
M client/includes/Usage/UsageAccumulator.php
M client/tests/phpunit/includes/Hooks/DataUpdateHookHandlersTest.php
4 files changed, 65 insertions(+), 161 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
Objections:
Adrian Lang: There's a problem with this change, please improve
diff --git a/client/includes/Usage/HashUsageAccumulator.php
b/client/includes/Usage/HashUsageAccumulator.php
index e819b92..d328a2d 100644
--- a/client/includes/Usage/HashUsageAccumulator.php
+++ b/client/includes/Usage/HashUsageAccumulator.php
@@ -2,11 +2,6 @@
namespace Wikibase\Client\Usage;
-use Wikibase\DataModel\Entity\EntityId;
-use Wikibase\DataModel\Entity\EntityIdValue;
-use Wikibase\DataModel\Snak\PropertyValueSnak;
-use Wikibase\DataModel\Snak\Snak;
-
/**
* This implementation of the UsageAccumulator interface simply wraps
* an array containing the usage information.
@@ -14,7 +9,7 @@
* @license GPL 2+
* @author Daniel Kinzler
*/
-class HashUsageAccumulator implements UsageAccumulator {
+class HashUsageAccumulator extends UsageAccumulator {
/**
* @var EntityUsage[]
@@ -22,87 +17,22 @@
private $usages = array();
/**
- * Registers usage of the given aspect of the given entity.
+ * @see UsageAccumulator::addUsage
*
- * @param EntityId $id
- * @param string $aspect Use the EntityUsage::XXX_USAGE constants.
+ * @param EntityUsage $usage
*/
- public function addUsage( EntityId $id, $aspect ) {
- $usage = new EntityUsage( $id, $aspect );
-
+ public function addUsage( EntityUsage $usage ) {
$key = $usage->getIdentityString();
$this->usages[$key] = $usage;
}
/**
- * @see UsageAccumulator::getUsage()
+ * @see UsageAccumulator::getUsage
*
* @return EntityUsage[]
*/
public function getUsages() {
return $this->usages;
- }
-
- /**
- * @see UsageAccumulator::addLabelUsageForSnaks
- *
- * @param Snak[] $snaks
- */
- public function addLabelUsageForSnaks( array $snaks ) {
- foreach ( $snaks as $snak ) {
- if ( $snak instanceof PropertyValueSnak ) {
- $value = $snak->getDataValue();
-
- if ( $value instanceof EntityIdValue ) {
- $this->addLabelUsage(
$value->getEntityId() );
- }
- }
- }
- }
-
- /**
- * @see UsageAccumulator::addLabelUsage
- *
- * @param EntityId $id
- */
- public function addLabelUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::LABEL_USAGE );
- }
-
- /**
- * @see UsageAccumulator::addTitleUsage
- *
- * @param EntityId $id
- */
- public function addTitleUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::TITLE_USAGE );
- }
-
- /**
- * @see UsageAccumulator::addSitelinksUsage
- *
- * @param EntityId $id
- */
- public function addSiteLinksUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::SITELINK_USAGE );
- }
-
- /**
- * @see UsageAccumulator::addOtherUsage
- *
- * @param EntityId $id
- */
- public function addOtherUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::OTHER_USAGE );
- }
-
- /**
- * @see UsageAccumulator::addAllUsage
- *
- * @param EntityId $id
- */
- public function addAllUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::ALL_USAGE );
}
}
diff --git a/client/includes/Usage/ParserOutputUsageAccumulator.php
b/client/includes/Usage/ParserOutputUsageAccumulator.php
index d89a57f..33c1b7a 100644
--- a/client/includes/Usage/ParserOutputUsageAccumulator.php
+++ b/client/includes/Usage/ParserOutputUsageAccumulator.php
@@ -3,10 +3,6 @@
namespace Wikibase\Client\Usage;
use ParserOutput;
-use Wikibase\DataModel\Entity\EntityId;
-use Wikibase\DataModel\Entity\EntityIdValue;
-use Wikibase\DataModel\Snak\PropertyValueSnak;
-use Wikibase\DataModel\Snak\Snak;
/**
* This implementation of the UsageAccumulator interface acts as a wrapper
around
@@ -16,7 +12,7 @@
* @license GPL 2+
* @author Daniel Kinzler
*/
-class ParserOutputUsageAccumulator implements UsageAccumulator {
+class ParserOutputUsageAccumulator extends UsageAccumulator {
/**
* @var ParserOutput
@@ -28,91 +24,25 @@
}
/**
- * Registers usage of the given aspect of the given entity.
+ * @see UsageAccumulator::addUsage
*
- * @param EntityId $id
- * @param string $aspect Use the EntityUsage::XXX_USAGE constants.
+ * @param EntityUsage $usage
*/
- public function addUsage( EntityId $id, $aspect ) {
+ public function addUsage( EntityUsage $usage ) {
$usages = $this->getUsages();
- $usage = new EntityUsage( $id, $aspect );
-
$key = $usage->getIdentityString();
$usages[$key] = $usage;
-
$this->parserOutput->setExtensionData( 'wikibase-entity-usage',
$usages );
}
/**
- * @see UsageAccumulator::getUsage()
+ * @see UsageAccumulator::getUsage
*
* @return EntityUsage[]
*/
public function getUsages() {
$usages = $this->parserOutput->getExtensionData(
'wikibase-entity-usage' );
- return $usages === null ? array() : $usages;
- }
-
- /**
- * @see UsageAccumulator::addLabelUsageForSnaks
- *
- * @param Snak[] $snaks
- */
- public function addLabelUsageForSnaks( array $snaks ) {
- foreach ( $snaks as $snak ) {
- if ( $snak instanceof PropertyValueSnak ) {
- $value = $snak->getDataValue();
-
- if ( $value instanceof EntityIdValue ) {
- $this->addLabelUsage(
$value->getEntityId() );
- }
- }
- }
- }
-
- /**
- * @see UsageAccumulator::addLabelUsage
- *
- * @param EntityId $id
- */
- public function addLabelUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::LABEL_USAGE );
- }
-
- /**
- * @see UsageAccumulator::addTitleUsage
- *
- * @param EntityId $id
- */
- public function addTitleUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::TITLE_USAGE );
- }
-
- /**
- * @see UsageAccumulator::addSitelinksUsage
- *
- * @param EntityId $id
- */
- public function addSiteLinksUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::SITELINK_USAGE );
- }
-
- /**
- * @see UsageAccumulator::addOtherUsage
- *
- * @param EntityId $id
- */
- public function addOtherUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::OTHER_USAGE );
- }
-
- /**
- * @see UsageAccumulator::addAllUsage
- *
- * @param EntityId $id
- */
- public function addAllUsage( EntityId $id ) {
- $this->addUsage( $id, EntityUsage::ALL_USAGE );
+ return $usages ?: array();
}
}
diff --git a/client/includes/Usage/UsageAccumulator.php
b/client/includes/Usage/UsageAccumulator.php
index bffcf5c..d744c4f 100644
--- a/client/includes/Usage/UsageAccumulator.php
+++ b/client/includes/Usage/UsageAccumulator.php
@@ -3,15 +3,25 @@
namespace Wikibase\Client\Usage;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\EntityIdValue;
+use Wikibase\DataModel\Snak\PropertyValueSnak;
use Wikibase\DataModel\Snak\Snak;
/**
- * Interface for objects accumulating usage tracking information for a given
page.
+ * Interface and base class for objects accumulating usage tracking
information for a given page.
*
* @license GPL 2+
* @author Daniel Kinzler
+ * @author Thiemo Mättig
*/
-interface UsageAccumulator {
+abstract class UsageAccumulator {
+
+ /**
+ * Registers usage of the given aspect of the given entity.
+ *
+ * @param EntityUsage $usage
+ */
+ abstract public function addUsage( EntityUsage $usage );
/**
* Registers the usage of entity's labels (in the local content
language), if the provided
@@ -23,14 +33,40 @@
*
* @param Snak[] $snaks
*/
- public function addLabelUsageForSnaks( array $snaks );
+ public function addLabelUsageForSnaks( array $snaks ) {
+ foreach ( $snaks as $snak ) {
+ $this->addLabelUsageForSnak( $snak );
+ }
+ }
+
+ /**
+ * Registers the usage of an entity's label (in the local content
language), if the provided
+ * snak is a PropertyValueSnak that contains an EntityIdValue.
+ *
+ * @note We track any EntityIdValue as a label usage. This is making
assumptions about what the
+ * respective formatter actually does. Ideally, the formatter itself
would perform the tracking,
+ * but that seems nasty to model.
+ *
+ * @param Snak $snak
+ */
+ public function addLabelUsageForSnak( Snak $snak ) {
+ if ( $snak instanceof PropertyValueSnak ) {
+ $value = $snak->getDataValue();
+
+ if ( $value instanceof EntityIdValue ) {
+ $this->addLabelUsage( $value->getEntityId() );
+ }
+ }
+ }
/**
* Registers the usage of an entity's label (in the local content
language).
*
* @param EntityId $id
*/
- public function addLabelUsage( EntityId $id );
+ public function addLabelUsage( EntityId $id ) {
+ $this->addUsage( new EntityUsage( $id, EntityUsage::LABEL_USAGE
) );
+ }
/**
* Registers the usage of an entity's local page title, e.g. to refer to
@@ -38,14 +74,18 @@
*
* @param EntityId $id
*/
- public function addTitleUsage( EntityId $id );
+ public function addTitleUsage( EntityId $id ) {
+ $this->addUsage( new EntityUsage( $id, EntityUsage::TITLE_USAGE
) );
+ }
/**
* Registers the usage of an entity's sitelinks, e.g. to generate
language links.
*
* @param EntityId $id
*/
- public function addSiteLinksUsage( EntityId $id );
+ public function addSiteLinksUsage( EntityId $id ) {
+ $this->addUsage( new EntityUsage( $id,
EntityUsage::SITELINK_USAGE ) );
+ }
/**
* Registers the usage of other (i.e. not label, sitelink, or title) of
an
@@ -54,7 +94,9 @@
*
* @param EntityId $id
*/
- public function addOtherUsage( EntityId $id );
+ public function addOtherUsage( EntityId $id ) {
+ $this->addUsage( new EntityUsage( $id, EntityUsage::OTHER_USAGE
) );
+ }
/**
* Registers the usage of any/all data of an entity (e.g. when accessed
@@ -62,13 +104,15 @@
*
* @param EntityId $id
*/
- public function addAllUsage( EntityId $id );
+ public function addAllUsage( EntityId $id ) {
+ $this->addUsage( new EntityUsage( $id, EntityUsage::ALL_USAGE )
);
+ }
/**
* Returns all entity usages previously registered via addXxxUsage()
*
* @return EntityUsage[]
*/
- public function getUsages();
+ abstract public function getUsages();
}
diff --git a/client/tests/phpunit/includes/Hooks/DataUpdateHookHandlersTest.php
b/client/tests/phpunit/includes/Hooks/DataUpdateHookHandlersTest.php
index ab6ef54..c2783e2 100644
--- a/client/tests/phpunit/includes/Hooks/DataUpdateHookHandlersTest.php
+++ b/client/tests/phpunit/includes/Hooks/DataUpdateHookHandlersTest.php
@@ -118,7 +118,7 @@
foreach ( $usages as $aspect => $entityIds ) {
foreach ( $entityIds as $id ) {
- $acc->addUsage( $id, $aspect );
+ $acc->addUsage( new EntityUsage( $id,
$aspect ) );
}
}
}
--
To view, visit https://gerrit.wikimedia.org/r/191336
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3902dfc8e493066fcc9a0337247885884d25d909
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: Adrian Lang <adrian.he...@wikimedia.de>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Hoo man <h...@online.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
