Filter out duplicate autoblocks when checking for blocks

Follow-up to I7654907.

Bug: T225919
Change-Id: I67e72d6c88e3cbfd9515a016b2782d1d9b123775

diff --git a/includes/block/BlockManager.php b/includes/block/BlockManager.php
index 41ff893..abd2db2 100644 (file)
--- a/includes/block/BlockManager.php
+++ b/includes/block/BlockManager.php
@@ -223,25 +223,32 @@ class BlockManager {
        }
 
        /**
-        * Given a list of blocks, return a list blocks where each block either has a
-        * unique ID or has ID null.
+        * Given a list of blocks, return a list of unique blocks.
+        *
+        * This usually means that each block has a unique ID. For a block with ID null,
+        * if it's an autoblock, it will be filtered out if the parent block is present;
+        * if not, it is assumed to be a unique system block, and kept.
         *
         * @param AbstractBlock[] $blocks
         * @return AbstractBlock[]
         */
        private function getUniqueBlocks( $blocks ) {
-               $blockIds = [];
-               $uniqueBlocks = [];
+               $systemBlocks = [];
+               $databaseBlocks = [];
+
                foreach ( $blocks as $block ) {
-                       $id = $block->getId();
-                       if ( $id === null ) {
-                               $uniqueBlocks[] = $block;
-                       } elseif ( !isset( $blockIds[$id] ) ) {
-                               $uniqueBlocks[] = $block;
-                               $blockIds[$block->getId()] = true;
+                       if ( $block instanceof SystemBlock ) {
+                               $systemBlocks[] = $block;
+                       } elseif ( $block->getType() === DatabaseBlock::TYPE_AUTO ) {
+                               if ( !isset( $databaseBlocks[$block->getParentBlockId()] ) ) {
+                                       $databaseBlocks[$block->getParentBlockId()] = $block;
+                               }
+                       } else {
+                               $databaseBlocks[$block->getId()] = $block;
                        }
                }
-               return $uniqueBlocks;
+
+               return array_merge( $systemBlocks, $databaseBlocks );
        }
 
        /**

diff --git a/includes/block/DatabaseBlock.php b/includes/block/DatabaseBlock.php
index ba08d54..0f19324 100644 (file)
--- a/includes/block/DatabaseBlock.php
+++ b/includes/block/DatabaseBlock.php
@@ -1045,6 +1045,14 @@ class DatabaseBlock extends AbstractBlock {
                return $this;
        }
 
+       /**
+        * @since 1.34
+        * @return int|null If this is an autoblock, ID of the parent block; otherwise null
+        */
+       public function getParentBlockId() {
+               return $this->mParentBlockId;
+       }
+
        /**
         * Get/set a flag determining whether the master is used for reads
         *

diff --git a/tests/phpunit/includes/block/BlockManagerTest.php b/tests/phpunit/includes/block/BlockManagerTest.php
index 39a5534..40fe4c8 100644 (file)
--- a/tests/phpunit/includes/block/BlockManagerTest.php
+++ b/tests/phpunit/includes/block/BlockManagerTest.php
@@ -2,6 +2,7 @@
 
 use MediaWiki\Block\BlockManager;
 use MediaWiki\Block\DatabaseBlock;
+use MediaWiki\Block\SystemBlock;
 
 /**
  * @group Blocking
@@ -288,4 +289,38 @@ class BlockManagerTest extends MediaWikiTestCase {
                        ],
                ];
        }
+
+       /**
+        * @covers ::getUniqueBlocks
+        */
+       public function testGetUniqueBlocks() {
+               $blockId = 100;
+
+               $class = new ReflectionClass( BlockManager::class );
+               $method = $class->getMethod( 'getUniqueBlocks' );
+               $method->setAccessible( true );
+
+               $blockManager = $this->getBlockManager( [] );
+
+               $block = $this->getMockBuilder( DatabaseBlock::class )
+                       ->setMethods( [ 'getId' ] )
+                       ->getMock();
+               $block->expects( $this->any() )
+                       ->method( 'getId' )
+                       ->willReturn( $blockId );
+
+               $autoblock = $this->getMockBuilder( DatabaseBlock::class )
+                       ->setMethods( [ 'getParentBlockId', 'getType' ] )
+                       ->getMock();
+               $autoblock->expects( $this->any() )
+                       ->method( 'getParentBlockId' )
+                       ->willReturn( $blockId );
+               $autoblock->expects( $this->any() )
+                       ->method( 'getType' )
+                       ->willReturn( DatabaseBlock::TYPE_AUTO );
+
+               $blocks = [ $block, $block, $autoblock, new SystemBlock() ];
+
+               $this->assertSame( 2, count( $method->invoke( $blockManager, $blocks ) ) );
+       }
 }