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

Reply via email to