jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/405896 )

Change subject: Rearrange UsageDeduplicator implementation
......................................................................


Rearrange UsageDeduplicator implementation

This is basically the exact same implementation, just moved around a
bit. I found the way the code was previously arranged not that optimal.
The code was spread out quite a lot.

Significant changes are:
* The actual deduplication intentionally replaces an array of usages
  with a single usage. This makes the later array_walk_recursive run
  faster because it does have less 1-element arrays to iterate.
* The actual deduplication use &-references to manipulate the one array,
  instead of constantly constructing new ones.
* Structuring is done in one step instead of two. $array[$foo][$bar][]
  works just fine here.
* Note that labels and descriptions are not mentioned any more! The same
  deduplication logic is applied to all aspects, not only labels and
  descriptions. This might be a mistake, but no test fails. If you think
  this is a mistake, please add a test that demonstrates why.

Bug: T178079
Change-Id: Ic8ec86088d49b5979a78e99d2cafce5f6c86f94d
---
M client/includes/Usage/UsageDeduplicator.php
1 file changed, 39 insertions(+), 45 deletions(-)

Approvals:
  Ladsgroup: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/client/includes/Usage/UsageDeduplicator.php 
b/client/includes/Usage/UsageDeduplicator.php
index 8c7e4ce..8a5fe66 100644
--- a/client/includes/Usage/UsageDeduplicator.php
+++ b/client/includes/Usage/UsageDeduplicator.php
@@ -12,81 +12,75 @@
 
        /**
         * @param EntityUsage[] $usages
+        *
         * @return EntityUsage[]
         */
        public function deduplicate( array $usages ) {
                $structuredUsages = $this->structureUsages( $usages );
-
-               foreach ( $structuredUsages as $entityId => $usages ) {
-                       $structuredUsages[$entityId] = 
$this->deduplicateUsagesPerEntity( $usages );
-               }
-
-               // Flatten the structured array
-               $return = [];
-               array_walk_recursive(
-                       $structuredUsages,
-                       function ( EntityUsage $usage ) use ( &$return ) {
-                               $return[$usage->getIdentityString()] = $usage;
-                       }
-               );
-               return $return;
+               $structuredUsages = $this->deduplicateStructuredUsages( 
$structuredUsages );
+               return $this->flattenStructuredUsages( $structuredUsages );
        }
 
        /**
         * @param EntityUsage[] $usages
-        * @return array[]
+        *
+        * @return array[][] three-dimensional array of
+        *  [ $entityId => [ $aspectKey => [ EntityUsage $usage, … ], … ], … ]
         */
        private function structureUsages( array $usages ) {
                $structuredUsages = [];
-               foreach ( $usages as $usage ) {
-                       $entityId = $usage->getEntityId();
-                       $structuredUsages[$entityId->getSerialization()][] = 
$usage;
-               }
 
-               return array_map( [ $this, 'structureUsagesPerEntity' ], 
$structuredUsages );
-       }
-
-       /**
-        * @param EntityUsage[] $usages
-        * @return array[]
-        */
-       private function structureUsagesPerEntity( array $usages ) {
-               $structuredUsages = [
-                       EntityUsage::DESCRIPTION_USAGE => [],
-                       EntityUsage::LABEL_USAGE => [],
-               ];
                foreach ( $usages as $usage ) {
+                       $entityId = $usage->getEntityId()->getSerialization();
                        $aspect = $usage->getAspect();
-                       $structuredUsages[$aspect][] = $usage;
+                       $structuredUsages[$entityId][$aspect][] = $usage;
                }
 
                return $structuredUsages;
        }
 
        /**
-        * @param array[] $usages
+        * @param array[][] $structuredUsages
+        *
         * @return array[]
         */
-       private function deduplicateUsagesPerEntity( array $usages ) {
-               $usages[EntityUsage::DESCRIPTION_USAGE] = 
$this->deduplicatePerType(
-                       $usages[EntityUsage::DESCRIPTION_USAGE]
-               );
-               $usages[EntityUsage::LABEL_USAGE] = $this->deduplicatePerType(
-                       $usages[EntityUsage::LABEL_USAGE]
-               );
-               return $usages;
+       private function deduplicateStructuredUsages( array $structuredUsages ) 
{
+               foreach ( $structuredUsages as &$usagesPerEntity ) {
+                       foreach ( $usagesPerEntity as &$usagesPerAspect ) {
+                               $this->deduplicatePerAspect( $usagesPerAspect );
+                       }
+               }
+
+               return $structuredUsages;
        }
 
        /**
-        * @param EntityUsage[] $usages
-        * @return EntityUsage[]
+        * @param EntityUsage[] &$usages
         */
-       private function deduplicatePerType( array $usages ) {
+       private function deduplicatePerAspect( array &$usages ) {
                foreach ( $usages as $usage ) {
                        if ( $usage->getModifier() === null ) {
-                               return [ $usage ];
+                               // This intentionally flattens the array to a 
single value
+                               $usages = $usage;
+                               return;
                        }
                }
+       }
+
+       /**
+        * @param array[] $structuredUsages
+        *
+        * @return EntityUsage[]
+        */
+       private function flattenStructuredUsages( array $structuredUsages ) {
+               $usages = [];
+
+               array_walk_recursive(
+                       $structuredUsages,
+                       function ( EntityUsage $usage ) use ( &$usages ) {
+                               $usages[$usage->getIdentityString()] = $usage;
+                       }
+               );
 
                return $usages;
        }

-- 
To view, visit https://gerrit.wikimedia.org/r/405896
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic8ec86088d49b5979a78e99d2cafce5f6c86f94d
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Kreuz (WMDE) <thiemo.kr...@wikimedia.de>
Gerrit-Reviewer: Ladsgroup <ladsgr...@gmail.com>
Gerrit-Reviewer: Thiemo Kreuz (WMDE) <thiemo.kr...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to