From 2065c3df2c30ad758fb172fdb9fbcfdc00e7e990 Mon Sep 17 00:00:00 2001
From: DannyS712 <DannyS712.enwiki@gmail.com>
Date: Sun, 14 Mar 2021 20:37:37 +0000
Subject: [PATCH] BlockPermissionChecker: inject BlockUtils

Instead of using AbstractBlock::parseTarget, which
just calls the BlockUtils service but makes it impossible
to convert the tests to unit tests

Follow-up will actually convert the tests to unit tests

Change-Id: I939c44811669e7a09dcffc7589e3b1176a2f68d4
---
 includes/ServiceWiring.php                           |  1 +
 includes/block/BlockPermissionChecker.php            | 12 +++++-------
 includes/block/BlockPermissionCheckerFactory.php     | 11 ++++++++---
 .../includes/block/BlockPermissionCheckerTest.php    |  2 ++
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index ac49815b833..1e609905824 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -232,6 +232,7 @@ return [
 				BlockPermissionCheckerFactory::CONSTRUCTOR_OPTIONS,
 				$services->getMainConfig()
 			),
+			$services->getBlockUtils(),
 			$services->getUserFactory()
 		);
 	},
diff --git a/includes/block/BlockPermissionChecker.php b/includes/block/BlockPermissionChecker.php
index af49ce9be60..f2cb746e8d4 100644
--- a/includes/block/BlockPermissionChecker.php
+++ b/includes/block/BlockPermissionChecker.php
@@ -59,24 +59,22 @@ class BlockPermissionChecker {
 		'EnableUserEmail',
 	];
 
-	/**
-	 * @var ServiceOptions
-	 */
+	/** @var ServiceOptions */
 	private $options;
 
-	/**
-	 * @var UserFactory
-	 */
+	/** @var UserFactory */
 	private $userFactory;
 
 	/**
 	 * @param ServiceOptions $options
+	 * @param BlockUtils $blockUtils
 	 * @param UserFactory $userFactory
 	 * @param UserIdentity|string|null $target
 	 * @param Authority $performer
 	 */
 	public function __construct(
 		ServiceOptions $options,
+		BlockUtils $blockUtils,
 		UserFactory $userFactory,
 		$target,
 		Authority $performer
@@ -84,7 +82,7 @@ class BlockPermissionChecker {
 		$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
 		$this->options = $options;
 		$this->userFactory = $userFactory;
-		list( $this->target, $this->targetType ) = AbstractBlock::parseTarget( $target );
+		list( $this->target, $this->targetType ) = $blockUtils->parseBlockTarget( $target );
 		$this->performer = $performer;
 	}
 
diff --git a/includes/block/BlockPermissionCheckerFactory.php b/includes/block/BlockPermissionCheckerFactory.php
index 4df08a171f4..ea6b40308e2 100644
--- a/includes/block/BlockPermissionCheckerFactory.php
+++ b/includes/block/BlockPermissionCheckerFactory.php
@@ -42,21 +42,25 @@ class BlockPermissionCheckerFactory {
 	 */
 	public const CONSTRUCTOR_OPTIONS = BlockPermissionChecker::CONSTRUCTOR_OPTIONS;
 
-	/**
-	 * @var UserFactory
-	 */
+	/** @var BlockUtils */
+	private $blockUtils;
+
+	/** @var UserFactory */
 	private $userFactory;
 
 	/**
 	 * @param ServiceOptions $options
+	 * @param BlockUtils $blockUtils
 	 * @param UserFactory $userFactory
 	 */
 	public function __construct(
 		ServiceOptions $options,
+		BlockUtils $blockUtils,
 		UserFactory $userFactory
 	) {
 		$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
 		$this->options = $options;
+		$this->blockUtils = $blockUtils;
 		$this->userFactory = $userFactory;
 	}
 
@@ -72,6 +76,7 @@ class BlockPermissionCheckerFactory {
 	) {
 		return new BlockPermissionChecker(
 			$this->options,
+			$this->blockUtils,
 			$this->userFactory,
 			$target,
 			$performer
diff --git a/tests/phpunit/includes/block/BlockPermissionCheckerTest.php b/tests/phpunit/includes/block/BlockPermissionCheckerTest.php
index 69bb99fd1dd..b4573168d36 100644
--- a/tests/phpunit/includes/block/BlockPermissionCheckerTest.php
+++ b/tests/phpunit/includes/block/BlockPermissionCheckerTest.php
@@ -8,6 +8,8 @@ use MediaWiki\MediaWikiServices;
  * @group Database
  * @group Blocking
  * @coversDefaultClass \MediaWiki\Block\BlockPermissionChecker
+ *
+ * TODO convert to Unit tests, all dependencies are injected
  */
 class BlockPermissionCheckerTest extends MediaWikiLangTestCase {
 	/**