From 9510597f26a738d4afe6a82b2b5b4f0249e7865e Mon Sep 17 00:00:00 2001
From: Thalia <thalia.e.chan@googlemail.com>
Date: Thu, 20 Jun 2019 11:29:01 +0100
Subject: [PATCH] Filter out duplicate autoblocks when checking for blocks

Follow-up to I7654907.

Bug: T225919
Change-Id: I67e72d6c88e3cbfd9515a016b2782d1d9b123775
---
 includes/block/BlockManager.php               | 29 +++++++++------
 includes/block/DatabaseBlock.php              |  8 +++++
 .../includes/block/BlockManagerTest.php       | 35 +++++++++++++++++++
 3 files changed, 61 insertions(+), 11 deletions(-)

diff --git a/includes/block/BlockManager.php b/includes/block/BlockManager.php
index 41ff893d57..abd2db24af 100644
--- 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 ba08d5461e..0f193240a0 100644
--- 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 39a5534027..40fe4c881d 100644
--- 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 ) ) );
+	}
 }
-- 
2.20.1