From 15fa9559a9f3385dd533649b9d13bea08fe42825 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Sat, 6 Jun 2020 20:46:08 +0100
Subject: [PATCH] rdbms: Simplify MWLBFactory::getLBFactoryClass

* Convert to a single map.
* Simplify source code.
* Remove deprecation warning for something we're not likely to
  remove. This was embedded into LocalSettings.php files and
  should generally just keep working and is easy and cheap to do.
  Also note clear why one half warned and the other half didn't
  (because that's the one we happen to use in prod.)
* Simplify the tests. A lot of the boilerplate was no longer needed.
* Reduce abstraction in the test as was was not more complex
  than the source it tests.

Change-Id: If3e7e25dbf3bb408581fc16ac8e556b44b1855ad
---
 includes/db/MWLBFactory.php                   | 38 ++++-------------
 tests/phpunit/includes/db/MWLBFactoryTest.php | 41 ++++++-------------
 2 files changed, 22 insertions(+), 57 deletions(-)

diff --git a/includes/db/MWLBFactory.php b/includes/db/MWLBFactory.php
index 1434f2b2bd7..0d990f89d5a 100644
--- a/includes/db/MWLBFactory.php
+++ b/includes/db/MWLBFactory.php
@@ -293,46 +293,26 @@ abstract class MWLBFactory {
 	}
 
 	/**
-	 * Returns the LBFactory class to use and the load balancer configuration.
-	 *
-	 * @todo instead of this, use a ServiceContainer for managing the different implementations.
+	 * Decide which LBFactory class to use.
 	 *
+	 * @internal For use by ServiceWiring
 	 * @param array $config (e.g. $wgLBFactoryConf)
 	 * @return string Class name
-	 * @internal For use with service wiring
 	 */
 	public static function getLBFactoryClass( array $config ) {
-		// For configuration backward compatibility after removing
-		// underscores from class names in MediaWiki 1.23.
-		$bcClasses = [
-			'LBFactory_Simple' => 'LBFactorySimple',
-			'LBFactory_Single' => 'LBFactorySingle',
-			'LBFactory_Multi' => 'LBFactoryMulti'
-		];
-
-		$class = $config['class'];
-
-		if ( isset( $bcClasses[$class] ) ) {
-			wfDeprecatedMsg(
-				'$wgLBFactoryConf must be updated. ' .
-				"The class $class was renamed to {$bcClasses[$class]} in MediaWiki 1.23.",
-				'1.23'
-			);
-			$class = $bcClasses[$class];
-		}
-
-		// For configuration backward compatibility after moving classes to namespaces (1.29)
 		$compat = [
+			// For LocalSettings.php compat after removing underscores (since 1.23).
+			'LBFactory_Single' => Wikimedia\Rdbms\LBFactorySingle::class,
+			'LBFactory_Simple' => Wikimedia\Rdbms\LBFactorySimple::class,
+			'LBFactory_Multi' => Wikimedia\Rdbms\LBFactoryMulti::class,
+			// For LocalSettings.php compat after moving classes to namespaces (since 1.29).
 			'LBFactorySingle' => Wikimedia\Rdbms\LBFactorySingle::class,
 			'LBFactorySimple' => Wikimedia\Rdbms\LBFactorySimple::class,
 			'LBFactoryMulti' => Wikimedia\Rdbms\LBFactoryMulti::class
 		];
 
-		if ( isset( $compat[$class] ) ) {
-			$class = $compat[$class];
-		}
-
-		return $class;
+		$class = $config['class'];
+		return $compat[$class] ?? $class;
 	}
 
 	/**
diff --git a/tests/phpunit/includes/db/MWLBFactoryTest.php b/tests/phpunit/includes/db/MWLBFactoryTest.php
index 1cd7333eb5e..461a84e711f 100644
--- a/tests/phpunit/includes/db/MWLBFactoryTest.php
+++ b/tests/phpunit/includes/db/MWLBFactoryTest.php
@@ -19,7 +19,6 @@
  */
 
 use Wikimedia\Rdbms\DatabaseDomain;
-use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\LBFactorySimple;
 
 /**
@@ -30,38 +29,24 @@ use Wikimedia\Rdbms\LBFactorySimple;
  */
 class MWLBFactoryTest extends MediaWikiTestCase {
 	/**
-	 * @covers       MWLBFactory::getLBFactoryClass()
+	 * @covers MWLBFactory::getLBFactoryClass
 	 * @dataProvider getLBFactoryClassProvider
 	 */
-	public function testGetLBFactoryClass( $expected, $deprecated ) {
-		$mockDB = $this->getMockBuilder( IDatabase::class )
-			->disableOriginalConstructor()
-			->getMock();
-
-		$config = [
-			'class' => $deprecated,
-			'connection' => $mockDB,
-			# Various other parameters required:
-			'sectionsByDB' => [],
-			'sectionLoads' => [],
-			'serverTemplate' => [],
-		];
-
-		$this->filterDeprecated( '/\$wgLBFactoryConf must be updated/' );
-		$result = MWLBFactory::getLBFactoryClass( $config );
-
-		$this->assertEquals( $expected, $result );
+	public function testGetLBFactoryClass( $config, $expected ) {
+		$this->assertEquals(
+			$expected,
+			MWLBFactory::getLBFactoryClass( $config )
+		);
 	}
 
 	public function getLBFactoryClassProvider() {
-		return [
-			# Format: new class, old class
-			[ Wikimedia\Rdbms\LBFactorySimple::class, 'LBFactory_Simple' ],
-			[ Wikimedia\Rdbms\LBFactorySingle::class, 'LBFactory_Single' ],
-			[ Wikimedia\Rdbms\LBFactoryMulti::class, 'LBFactory_Multi' ],
-			[ Wikimedia\Rdbms\LBFactorySimple::class, 'LBFactorySimple' ],
-			[ Wikimedia\Rdbms\LBFactorySingle::class, 'LBFactorySingle' ],
-			[ Wikimedia\Rdbms\LBFactoryMulti::class, 'LBFactoryMulti' ],
+		yield 'undercore alias default' => [
+			[ 'class' => 'LBFactory_Simple' ],
+			Wikimedia\Rdbms\LBFactorySimple::class,
+		];
+		yield 'short alias multi' => [
+			[ 'class' => 'LBFactoryMulti' ],
+			Wikimedia\Rdbms\LBFactoryMulti::class,
 		];
 	}