jenkins-bot has submitted this change and it was merged.
Change subject: Much simpler usage aspect intersection calculation
......................................................................
Much simpler usage aspect intersection calculation
Change-Id: I4c505600e065c0b46e7de506b50017d619f65043
---
M client/includes/Usage/EntityUsage.php
M client/includes/Usage/UsageAspectTransformer.php
M client/tests/phpunit/includes/Usage/EntityUsageTest.php
3 files changed, 54 insertions(+), 74 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/includes/Usage/EntityUsage.php
b/client/includes/Usage/EntityUsage.php
index 24fea43..5876d7a 100644
--- a/client/includes/Usage/EntityUsage.php
+++ b/client/includes/Usage/EntityUsage.php
@@ -146,6 +146,16 @@
}
/**
+ * @param string $aspectKey
+ *
+ * @return string One of the EntityUsage::..._USAGE constants with the
modifier split off.
+ */
+ public static function stripModifier( $aspectKey ) {
+ // This is about twice as fast compared to calling
$this->splitAspectKey.
+ return strstr( $aspectKey, '.', true ) ?: $aspectKey;
+ }
+
+ /**
* Splits the given aspect key into aspect and modifier (if any).
* This is the inverse of makeAspectKey().
*
@@ -162,16 +172,17 @@
return $parts;
}
+
/**
* Composes an aspect key from aspect and modifier (if any).
* This is the inverse of splitAspectKey().
*
* @param string $aspect
- * @param string $modifier
+ * @param string|null $modifier
*
* @return string "$aspect.$modifier"
*/
- public static function makeAspectKey( $aspect, $modifier ) {
+ public static function makeAspectKey( $aspect, $modifier = null ) {
if ( $modifier === null ) {
return $aspect;
}
diff --git a/client/includes/Usage/UsageAspectTransformer.php
b/client/includes/Usage/UsageAspectTransformer.php
index b695274..2bf5e20 100644
--- a/client/includes/Usage/UsageAspectTransformer.php
+++ b/client/includes/Usage/UsageAspectTransformer.php
@@ -22,6 +22,7 @@
*
* @license GPL 2+
* @author Daniel Kinzler
+ * @author Thiemo Mättig
*/
class UsageAspectTransformer {
@@ -132,94 +133,54 @@
* - If a modified aspect A.xx is present in $aspect and the unmodified
aspect A is present in
* $relevant, the modified aspect A.xx is included in the result.
*
- * @param string[] $aspects
- * @param string[] $relevant
+ * @param string[] $aspectKeys Array of aspect keys, with modifiers
applied.
+ * @param string[] $relevant Array of aspect keys, with modifiers
applied.
*
- * @return string[] Aspect keys, with modifiers applied
+ * @return string[] Array of aspect keys, with modifiers applied.
*/
- private function getFilteredAspects( array $aspects, array $relevant ) {
- if ( empty( $aspects ) || empty( $relevant ) ) {
+ private function getFilteredAspects( array $aspectKeys, array $relevant
) {
+ if ( empty( $aspectKeys ) || empty( $relevant ) ) {
return array();
}
- if ( in_array( 'X', $aspects ) ) {
+ if ( in_array( EntityUsage::ALL_USAGE, $aspectKeys ) ) {
return $relevant;
- } elseif ( in_array( 'X', $relevant ) ) {
- return $aspects;
+ } elseif ( in_array( EntityUsage::ALL_USAGE, $relevant ) ) {
+ return $aspectKeys;
}
- // group modified aspects into "bins" for matching with
unmodified aspects, e.g.
- // array( 'X' => array( 'X' ), 'L' => array( 'L.de', 'L.ru' ) )
- $aspectBins = $this->binAspects( $aspects );
- $relevantBins = $this->binAspects( $relevant );
+ $directMatches = array_intersect( $relevant, $aspectKeys );
- // matches 'L.xx' in $aspects to 'L' in $relevant
- $leftMatches = $this->matchBins( $aspectBins, $relevant );
+ // This turns the array into an associative array of aspect
keys (with modifiers) as keys,
+ // the values being meaningless (a.k.a. HashSet).
+ $aspects = array_flip( $directMatches );
- // matches 'L.xx' in $relevant to 'L' in $aspects
- $rightMatches = $this->matchBins( $relevantBins, $aspects );
+ // Matches 'L.xx' in $aspects to 'L' in $relevant.
+ $this->intersectAspectsIntoKeys( $aspectKeys, $relevant,
$aspects );
- // matches 'L.xx' in $relevant to 'L.xx' in $aspects
- $directMatches = array_intersect( $relevant, $aspects );
+ // Matches 'L.xx' in $relevant to 'L' in $aspects.
+ $this->intersectAspectsIntoKeys( $relevant, $aspectKeys,
$aspects );
- // combine, sort, and uniquify the results
- $matches = array_merge(
- $directMatches,
- $leftMatches,
- $rightMatches
- );
-
- sort( $matches );
- return array_unique( $matches );
+ ksort( $aspects );
+ return array_keys( $aspects );
}
/**
- * Collects aspects into bins, each bin containing all the
modifications of a given aspect.
- * This is useful for matching modified aspects against unmodified ones.
- *
- * @example array( 'X', 'L.de', 'L.ru' ) becomes
- * array( 'X' => array( 'X' ), 'L' => array( 'L.de', 'L.ru' ) ).
- *
- * @param string[] $aspects
- *
- * @return string[][] An associative array mapping aspect names to
groups of modifications
- * of that aspect.
+ * @param string[] $aspectKeys Array of aspect keys, with modifiers
applied.
+ * @param string[] $relevant Array of aspects (without modifiers).
+ * @param array &$aspects Associative array of aspect keys (with
modifiers) as keys, the values
+ * being meaningless (a.k.a. HashSet).
*/
- private function binAspects( $aspects ) {
- $bags = array();
+ private function intersectAspectsIntoKeys( array $aspectKeys, array
$relevant, array &$aspects ) {
+ $relevant = array_flip( $relevant );
- foreach ( $aspects as $a ) {
- list( $key, ) = EntityUsage::splitAspectKey( $a );
- $bags[$key][] = $a;
+ foreach ( $aspectKeys as $aspectKey ) {
+ $aspect = EntityUsage::stripModifier( $aspectKey );
+
+ if ( array_key_exists( $aspect, $relevant ) ) {
+ $aspects[$aspectKey] = null;
+ }
}
-
- return $bags;
- }
-
- /**
- * Match the names $aspects against the aspects used as keys to in
$bins.
- * The result is constructed by merging the bins associated with
matching keys.
- *
- * @example: If $aspects = array( 'T', 'L', 'S' ) and
- * $bins = array( 'L' => array( 'L.de', 'L.ru' ), 'T' => array( 'T' ),
'X' => array( 'X' ) ),
- * the result would be array( 'L.de', 'L.ru', 'T' ), which is the union
of the bins
- * associated with the 'L' and 'T' keys.
- *
- * @param string[][] $bins An associative array mapping aspect names to
groups of modifications
- * of that aspect, as returned by binAspects().
- *
- * @param string[] $aspects A list of aspect names
- *
- * @return string[]
- */
- private function matchBins( array $bins, array $aspects ) {
- // keep the bins with keys also present in $aspects
- $matchingBins = array_intersect_key( $bins, array_flip(
$aspects ) );
-
- // merge the matching bins into a single list
- $matchingAspects = array_reduce( $matchingBins, 'array_merge',
array() );
-
- return $matchingAspects;
}
}
diff --git a/client/tests/phpunit/includes/Usage/EntityUsageTest.php
b/client/tests/phpunit/includes/Usage/EntityUsageTest.php
index 5f5d39e..fc90d0d 100644
--- a/client/tests/phpunit/includes/Usage/EntityUsageTest.php
+++ b/client/tests/phpunit/includes/Usage/EntityUsageTest.php
@@ -56,7 +56,7 @@
$this->assertEquals( "$aspect.$modifier",
$usage->getAspectKey() );
}
- public function provideSplitAspectKey() {
+ public function aspectKeyProvider() {
return array(
array( 'L', array( 'L', null ) ),
array( 'L.x', array( 'L', 'x' ) ),
@@ -65,7 +65,15 @@
}
/**
- * @dataProvider provideSplitAspectKey
+ * @dataProvider aspectKeyProvider
+ */
+ public function testStripModifier( $aspectKey, $expectedParts ) {
+ $aspect = EntityUsage::stripModifier( $aspectKey );
+ $this->assertEquals( $expectedParts[0], $aspect );
+ }
+
+ /**
+ * @dataProvider aspectKeyProvider
*/
public function testSplitAspectKey( $aspectKey, $expectedParts ) {
$parts = EntityUsage::splitAspectKey( $aspectKey );
--
To view, visit https://gerrit.wikimedia.org/r/212406
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4c505600e065c0b46e7de506b50017d619f65043
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits