jenkins-bot has submitted this change and it was merged. Change subject: Add modifier field to EntityUsage ......................................................................
Add modifier field to EntityUsage Bug: T93055 Change-Id: I1b4918dc023389eeb034ec87b7309c2c871c1619 (cherry picked from commit 2df6b4184b4666878df8eec0d006768aca7784ba) --- M client/includes/Changes/ChangeHandler.php M client/includes/Usage/EntityUsage.php M client/includes/Usage/PageEntityUsages.php M client/includes/Usage/Sql/UsageTableUpdater.php M client/includes/Usage/UsageAccumulator.php M client/tests/phpunit/includes/Usage/EntityUsageTest.php M client/tests/phpunit/includes/Usage/PageEntityUsagesTest.php M client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php M client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php 9 files changed, 197 insertions(+), 36 deletions(-) Approvals: Aude: Looks good to me, approved jenkins-bot: Verified diff --git a/client/includes/Changes/ChangeHandler.php b/client/includes/Changes/ChangeHandler.php index 236f6f0..90df9ae 100644 --- a/client/includes/Changes/ChangeHandler.php +++ b/client/includes/Changes/ChangeHandler.php @@ -184,7 +184,7 @@ } /** - * @param string[] $aspects + * @param string[] $aspects Usage aspects (without modifier), see EntityUsage. * * @return string[] A list of actions, as defined by the self::XXXX_ACTION constants. */ diff --git a/client/includes/Usage/EntityUsage.php b/client/includes/Usage/EntityUsage.php index ae283ba..75bc3df 100644 --- a/client/includes/Usage/EntityUsage.php +++ b/client/includes/Usage/EntityUsage.php @@ -28,7 +28,8 @@ /** * Usage flag indicating that the entity's label in the local content language was used. - * This would be the case when showing the label of a referenced entity. + * This would be the case when showing the label of a referenced entity. Note that + * label usage is typically tracked with a modifier specifying the label's language code. */ const LABEL_USAGE = 'L'; @@ -77,18 +78,26 @@ private $aspect; /** + * @var null|string + */ + private $modifier; + + /** * @param EntityId $entityId * @param string $aspect use the EntityUsage::XXX_USAGE constants + * @param string|null $modifier for further qualifying the usage aspect (e.g. a language code + * may be used along with the LABEL_USAGE aspect. * * @throws InvalidArgumentException */ - public function __construct( EntityId $entityId, $aspect ) { + public function __construct( EntityId $entityId, $aspect, $modifier = null ) { if ( !array_key_exists( $aspect, self::$aspects ) ) { throw new InvalidArgumentException( '$aspect must use one of the XXX_USAGE constants!' ); } $this->entityId = $entityId; $this->aspect = $aspect; + $this->modifier = $modifier; } /** @@ -96,6 +105,23 @@ */ public function getAspect() { return $this->aspect; + } + + /** + * @return null|string + */ + public function getModifier() { + return $this->modifier; + } + + /** + * Returns the aspect with the modifier applied. + * @see makeAspectKey + * + * @return string + */ + public function getAspectKey() { + return self::makeAspectKey( $this->aspect, $this->modifier ); } /** @@ -109,7 +135,7 @@ * @return string */ public function getIdentityString() { - return $this->getEntityId()->getSerialization() . '#' . $this->getAspect(); + return $this->getEntityId()->getSerialization() . '#' . $this->getAspectKey(); } /** @@ -119,4 +145,38 @@ return $this->getIdentityString(); } + /** + * Splits the given aspect key into aspect and modifier (if any). + * This is the inverse of makeAspectKey(). + * + * @param string $aspectKey + * + * @return string[] list( $aspect, $modifier ) + */ + public static function splitAspectKey( $aspectKey ) { + $parts = explode( '.', $aspectKey, 2 ); + + if ( !isset( $parts[1] ) ) { + $parts[1] = null; + } + + return $parts; + } + /** + * Composes an aspect key from aspect and modifier (if any). + * This is the inverse of splitAspectKey(). + * + * @param string $aspect + * @param string $modifier + * + * @return string "$aspect.$modifier" + */ + public static function makeAspectKey( $aspect, $modifier ) { + if ( $modifier !== null ) { + return "$aspect.$modifier"; + } else { + return $aspect; + } + } + } diff --git a/client/includes/Usage/PageEntityUsages.php b/client/includes/Usage/PageEntityUsages.php index 4eca198..5977ee3 100644 --- a/client/includes/Usage/PageEntityUsages.php +++ b/client/includes/Usage/PageEntityUsages.php @@ -81,14 +81,37 @@ /** * Collects all usage aspects present on the page. + * Modifiers are not considered, use getAspects() if modifiers should be included. * - * string[] aspect names (sorted) + * @see getAspectKeys() + * + * string[] Sorted list of aspect names (without modifiers). */ public function getAspects() { $aspects = array(); foreach ( $this->usages as $usage ) { $aspect = $usage->getAspect(); + $aspects[$aspect] = true; + } + + ksort( $aspects ); + return array_keys( $aspects ); + } + + /** + * Collects all usage aspects present on the page. + * Aspect keys will include modifiers, use getAspects() if modifiers are not desired. + * + * @see getAspects() + * + * string[] Sorted list of full aspect names with modifiers. + */ + public function getAspectKeys() { + $aspects = array(); + + foreach ( $this->usages as $usage ) { + $aspect = $usage->getAspectKey(); $aspects[$aspect] = true; } @@ -132,11 +155,14 @@ /** * Returns the aspects used by the given entity on the page - * represented by this PageEntityUsages object. + * represented by this PageEntityUsages object. They aspects + * will include any modifiers. + * + * @todo: change this into getUsageAspectKeys * * @param EntityId $id * - * @return string[] List of aspect names, sorted. + * @return string[] List of aspect keys, sorted. */ public function getUsageAspects( EntityId $id ) { $aspects = array(); @@ -148,7 +174,7 @@ } sort( $aspects ); - return $aspects; + return array_values( array_unique( $aspects ) ); } public function __toString() { diff --git a/client/includes/Usage/Sql/UsageTableUpdater.php b/client/includes/Usage/Sql/UsageTableUpdater.php index fab22b2..c625a03 100644 --- a/client/includes/Usage/Sql/UsageTableUpdater.php +++ b/client/includes/Usage/Sql/UsageTableUpdater.php @@ -155,7 +155,7 @@ /** * Collects the entity id strings contained in the given list of EntityUsages into - * bins based on the usage's aspect. + * bins based on the usage's aspect and modifier. * * @param EntityUsage[] $usages * @@ -170,7 +170,7 @@ throw new InvalidArgumentException( '$usages must contain EntityUsage objects.' ); } - $aspect = $usage->getAspect(); + $aspect = $usage->getAspectKey(); $bins[$aspect][] = $usage->getEntityId()->getSerialization(); } @@ -195,7 +195,7 @@ $rows[] = array( 'eu_page_id' => (int)$pageId, - 'eu_aspect' => $usage->getAspect(), + 'eu_aspect' => $usage->getAspectKey(), 'eu_entity_id' => $usage->getEntityId()->getSerialization(), 'eu_touched' => wfTimestamp( TS_MW, $touched ), ); diff --git a/client/includes/Usage/UsageAccumulator.php b/client/includes/Usage/UsageAccumulator.php index d744c4f..51c1f2a 100644 --- a/client/includes/Usage/UsageAccumulator.php +++ b/client/includes/Usage/UsageAccumulator.php @@ -24,7 +24,7 @@ abstract public function addUsage( EntityUsage $usage ); /** - * Registers the usage of entity's labels (in the local content language), if the provided + * Registers the usage of entity's labels (in the given language), if the provided * snaks are PropertyValueSnaks that contain EntityIdValues. * * @note We track any EntityIdValue as a label usage. This is making assumptions about what the @@ -32,15 +32,16 @@ * but that seems nasty to model. * * @param Snak[] $snaks + * @param string|null $language */ - public function addLabelUsageForSnaks( array $snaks ) { + public function addLabelUsageForSnaks( array $snaks, $language = null ) { foreach ( $snaks as $snak ) { - $this->addLabelUsageForSnak( $snak ); + $this->addLabelUsageForSnak( $snak, $language ); } } /** - * Registers the usage of an entity's label (in the local content language), if the provided + * Registers the usage of an entity's label (in the given 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 @@ -48,24 +49,26 @@ * but that seems nasty to model. * * @param Snak $snak + * @param string|null $language */ - public function addLabelUsageForSnak( Snak $snak ) { + public function addLabelUsageForSnak( Snak $snak, $language = null ) { if ( $snak instanceof PropertyValueSnak ) { $value = $snak->getDataValue(); if ( $value instanceof EntityIdValue ) { - $this->addLabelUsage( $value->getEntityId() ); + $this->addLabelUsage( $value->getEntityId(), $language ); } } } /** - * Registers the usage of an entity's label (in the local content language). + * Registers the usage of an entity's label (in the given language). * * @param EntityId $id + * @param string|null $language */ - public function addLabelUsage( EntityId $id ) { - $this->addUsage( new EntityUsage( $id, EntityUsage::LABEL_USAGE ) ); + public function addLabelUsage( EntityId $id, $language = null ) { + $this->addUsage( new EntityUsage( $id, EntityUsage::LABEL_USAGE, $language ) ); } /** diff --git a/client/tests/phpunit/includes/Usage/EntityUsageTest.php b/client/tests/phpunit/includes/Usage/EntityUsageTest.php index a2cf399..5f5d39e 100644 --- a/client/tests/phpunit/includes/Usage/EntityUsageTest.php +++ b/client/tests/phpunit/includes/Usage/EntityUsageTest.php @@ -18,16 +18,73 @@ */ class EntityUsageTest extends PHPUnit_Framework_TestCase { - public function testGetters() { + + public function testGetEntityId() { $id = new ItemId( 'Q7' ); $aspect = EntityUsage::ALL_USAGE; $usage = new EntityUsage( $id, $aspect ); $this->assertEquals( $id, $usage->getEntityId() ); - $this->assertEquals( $aspect, $usage->getAspect() ); + } + public function testGetAspect() { + $id = new ItemId( 'Q7' ); + $aspect = EntityUsage::ALL_USAGE; + + $usage = new EntityUsage( $id, $aspect ); + $this->assertEquals( $aspect, $usage->getAspect() ); + } + + public function testGetIdentityString() { + $id = new ItemId( 'Q7' ); + $aspect = EntityUsage::ALL_USAGE; + + $usage = new EntityUsage( $id, $aspect ); $this->assertInternalType( 'string', $usage->getIdentityString() ); } + public function testGetAspectKey() { + $id = new ItemId( 'Q7' ); + $aspect = EntityUsage::LABEL_USAGE; + $modifier = 'ru'; + + $usage = new EntityUsage( $id, $aspect ); + $this->assertEquals( $aspect, $usage->getAspectKey() ); + + $usage = new EntityUsage( $id, $aspect, $modifier ); + $this->assertEquals( "$aspect.$modifier", $usage->getAspectKey() ); + } + + public function provideSplitAspectKey() { + return array( + array( 'L', array( 'L', null ) ), + array( 'L.x', array( 'L', 'x' ) ), + array( 'L.x.y', array( 'L', 'x.y' ) ), + ); + } + + /** + * @dataProvider provideSplitAspectKey + */ + public function testSplitAspectKey( $aspectKey, $expectedParts ) { + $parts = EntityUsage::splitAspectKey( $aspectKey ); + $this->assertEquals( $expectedParts, $parts ); + } + + public function provideMakeAspectKey() { + return array( + array( 'L', null, 'L' ), + array( 'L', 'x', 'L.x' ), + ); + } + + /** + * @dataProvider provideMakeAspectKey + */ + public function testMakeAspectKey( $aspect, $modifier, $expectedKey ) { + $key = EntityUsage::makeAspectKey( $aspect, $modifier ); + $this->assertEquals( $expectedKey, $key ); + } + } diff --git a/client/tests/phpunit/includes/Usage/PageEntityUsagesTest.php b/client/tests/phpunit/includes/Usage/PageEntityUsagesTest.php index d923d26..dff49f2 100644 --- a/client/tests/phpunit/includes/Usage/PageEntityUsagesTest.php +++ b/client/tests/phpunit/includes/Usage/PageEntityUsagesTest.php @@ -24,7 +24,8 @@ $usages = array( new EntityUsage( $q7, EntityUsage::ALL_USAGE ), - new EntityUsage( $q11, EntityUsage::LABEL_USAGE ), + new EntityUsage( $q11, EntityUsage::LABEL_USAGE, 'de' ), + new EntityUsage( $q11, EntityUsage::LABEL_USAGE, 'en' ), new EntityUsage( $q11, EntityUsage::TITLE_USAGE ), ); @@ -38,11 +39,24 @@ EntityUsage::ALL_USAGE, ); - $this->assertEquals( $expectedAspects, $pageUsages->getAspects() ); - $this->assertEquals( array( 'Q11' => $q11, 'Q7' => $q7 ), $pageUsages->getEntityIds() ); - $this->assertEquals( array( 'Q11#L', 'Q11#T', 'Q7#X' ), array_keys( $pageUsages->getUsages() ) ); + $expectedAspectKeys = array( + EntityUsage::LABEL_USAGE . '.de', + EntityUsage::LABEL_USAGE . '.en', + EntityUsage::TITLE_USAGE, + EntityUsage::ALL_USAGE, + ); - $this->assertEquals( array( EntityUsage::ALL_USAGE ), $pageUsages->getUsageAspects( $q7 ) ); + $expectedAspectsQ11 = array( + EntityUsage::LABEL_USAGE, + EntityUsage::TITLE_USAGE, + ); + + $this->assertEquals( $expectedAspects, $pageUsages->getAspects(), 'getAspects' ); + $this->assertEquals( $expectedAspectKeys, $pageUsages->getAspectKeys(), 'getAspectKeys' ); + $this->assertEquals( array( 'Q11' => $q11, 'Q7' => $q7 ), $pageUsages->getEntityIds(), 'getEntityIds' ); + $this->assertEquals( array( 'Q11#L.de', 'Q11#L.en', 'Q11#T', 'Q7#X' ), array_keys( $pageUsages->getUsages() ), 'getUsages' ); + + $this->assertEquals( $expectedAspectsQ11, $pageUsages->getUsageAspects( $q11 ), 'getUsageAspects' ); } } diff --git a/client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php b/client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php index 6377951..24d16be 100644 --- a/client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php +++ b/client/tests/phpunit/includes/Usage/Sql/UsageTableUpdaterTest.php @@ -39,7 +39,8 @@ $key = "Q$i"; $id = new ItemId( $key ); - $usages["$key#L"] = new EntityUsage( $id, EntityUsage::LABEL_USAGE ); + $usages["$key#L.de"] = new EntityUsage( $id, EntityUsage::LABEL_USAGE, 'de' ); + $usages["$key#L.en"] = new EntityUsage( $id, EntityUsage::LABEL_USAGE, 'en' ); $usages["$key#T"] = new EntityUsage( $id, EntityUsage::TITLE_USAGE ); } @@ -77,7 +78,7 @@ foreach ( $usages as $key => $usage ) { $row = array( 'eu_entity_id' => $usage->getEntityId()->getSerialization(), - 'eu_aspect' => $usage->getAspect() + 'eu_aspect' => $usage->getAspectKey() ); if ( $pageId > 0 ) { @@ -89,7 +90,7 @@ } if ( is_int( $key ) ) { - $key = $usage->getEntityId()->getSerialization() . '#' . $usage->getAspect(); + $key = $usage->getEntityId()->getSerialization() . '#' . $usage->getAspectKey(); } $rows[$key] = $row; diff --git a/client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php b/client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php index cfb0356..50421a5 100644 --- a/client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php +++ b/client/tests/phpunit/includes/Usage/UsageAccumulatorContractTester.php @@ -47,8 +47,8 @@ $q3 = new ItemId( 'Q3' ); $q4 = new ItemId( 'Q4' ); $expected = array( - new EntityUsage( $q4, EntityUsage::LABEL_USAGE ), - new EntityUsage( $q2, EntityUsage::LABEL_USAGE ), + new EntityUsage( $q4, EntityUsage::LABEL_USAGE, 'xx' ), + new EntityUsage( $q2, EntityUsage::LABEL_USAGE, 'xx' ), new EntityUsage( $q2, EntityUsage::TITLE_USAGE ), new EntityUsage( $q2, EntityUsage::SITELINK_USAGE ), new EntityUsage( $q2, EntityUsage::OTHER_USAGE ), @@ -64,9 +64,9 @@ $this->usageAccumulator->addLabelUsageForSnaks( array( new PropertyValueSnak( new PropertyId( 'P4' ), new EntityIdValue( $q4 ) ), new PropertyValueSnak( new PropertyId( 'P5' ), new StringValue( 'string' ) ), - ) ); + ), 'xx' ); - $expected = new EntityUsage( $q4, EntityUsage::LABEL_USAGE ); + $expected = new EntityUsage( $q4, EntityUsage::LABEL_USAGE, 'xx' ); $usages = $this->usageAccumulator->getUsages(); $this->assertContainsUsage( $expected, $usages ); @@ -74,9 +74,9 @@ private function testAddAndGetLabelUsage() { $q2 = new ItemId( 'Q2' ); - $this->usageAccumulator->addLabelUsage( $q2 ); + $this->usageAccumulator->addLabelUsage( $q2, 'xx' ); - $expected = new EntityUsage( $q2, EntityUsage::LABEL_USAGE ); + $expected = new EntityUsage( $q2, EntityUsage::LABEL_USAGE, 'xx' ); $usages = $this->usageAccumulator->getUsages(); $this->assertContainsUsage( $expected, $usages ); -- To view, visit https://gerrit.wikimedia.org/r/215076 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1b4918dc023389eeb034ec87b7309c2c871c1619 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: wmf/1.26wmf6 Gerrit-Owner: JanZerebecki <jan.wikime...@zerebecki.de> Gerrit-Reviewer: Aude <aude.w...@gmail.com> Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de> Gerrit-Reviewer: JanZerebecki <jan.wikime...@zerebecki.de> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits