From a52e7964735128b48868fb8b7fef7f4d7f8e75ed Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Tue, 10 Sep 2019 03:24:18 +0100
Subject: [PATCH] Revision: Inject Logger instead of LoggerSpi

For consistency with other services and entry points.
The caller decides on the channel name. This also narrows the
surface area for the DI layer by only requiring LoggerInterface
which is significantly lighter and easier to mock than an entire
LoggerProvider/Spi.

Change-Id: Ia71f2e1c0a1a08c9edf0736eb627c5d4a2dde8d5
---
 includes/Revision/RevisionStoreFactory.php    | 16 ++++++++--------
 includes/ServiceWiring.php                    |  2 +-
 .../Revision/RevisionStoreFactoryTest.php     | 19 +++----------------
 3 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/includes/Revision/RevisionStoreFactory.php b/includes/Revision/RevisionStoreFactory.php
index acecee1294..1de4d7fdd0 100644
--- a/includes/Revision/RevisionStoreFactory.php
+++ b/includes/Revision/RevisionStoreFactory.php
@@ -27,7 +27,7 @@ namespace MediaWiki\Revision;
 
 use ActorMigration;
 use CommentStore;
-use MediaWiki\Logger\Spi as LoggerSpi;
+use Psr\Log\LoggerInterface;
 use MediaWiki\Storage\BlobStoreFactory;
 use MediaWiki\Storage\NameTableStoreFactory;
 use WANObjectCache;
@@ -54,8 +54,8 @@ class RevisionStoreFactory {
 	private $dbLoadBalancerFactory;
 	/** @var WANObjectCache */
 	private $cache;
-	/** @var LoggerSpi */
-	private $loggerProvider;
+	/** @var LoggerInterface */
+	private $logger;
 
 	/** @var CommentStore */
 	private $commentStore;
@@ -84,7 +84,7 @@ class RevisionStoreFactory {
 	 * @param CommentStore $commentStore
 	 * @param ActorMigration $actorMigration
 	 * @param int $migrationStage
-	 * @param LoggerSpi $loggerProvider
+	 * @param LoggerInterface $logger
 	 * @param bool $contentHandlerUseDB see {@link $wgContentHandlerUseDB}. Must be the same
 	 *        for all wikis in the cluster. Will go away after MCR migration.
 	 */
@@ -97,7 +97,7 @@ class RevisionStoreFactory {
 		CommentStore $commentStore,
 		ActorMigration $actorMigration,
 		$migrationStage,
-		LoggerSpi $loggerProvider,
+		LoggerInterface $logger,
 		$contentHandlerUseDB
 	) {
 		Assert::parameterType( 'integer', $migrationStage, '$migrationStage' );
@@ -109,7 +109,7 @@ class RevisionStoreFactory {
 		$this->commentStore = $commentStore;
 		$this->actorMigration = $actorMigration;
 		$this->mcrMigrationStage = $migrationStage;
-		$this->loggerProvider = $loggerProvider;
+		$this->logger = $logger;
 		$this->contentHandlerUseDB = $contentHandlerUseDB;
 	}
 
@@ -118,7 +118,7 @@ class RevisionStoreFactory {
 	 *
 	 * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one
 	 *
-	 * @return RevisionStore for the given wikiId with all necessary services and a logger
+	 * @return RevisionStore for the given wikiId with all necessary services
 	 */
 	public function getRevisionStore( $dbDomain = false ) {
 		Assert::parameterType( 'string|boolean', $dbDomain, '$dbDomain' );
@@ -137,7 +137,7 @@ class RevisionStoreFactory {
 			$dbDomain
 		);
 
-		$store->setLogger( $this->loggerProvider->getLogger( 'RevisionStore' ) );
+		$store->setLogger( $this->logger );
 		$store->setContentHandlerUseDB( $this->contentHandlerUseDB );
 
 		return $store;
diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 0b0aaf54c8..6e9adcdc70 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -634,7 +634,7 @@ return [
 			$services->getCommentStore(),
 			$services->getActorMigration(),
 			$config->get( 'MultiContentRevisionSchemaMigrationStage' ),
-			LoggerFactory::getProvider(),
+			LoggerFactory::getInstance( 'RevisionStore' ),
 			$config->get( 'ContentHandlerUseDB' )
 		);
 
diff --git a/tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php b/tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php
index f4d324dde7..99332d2488 100644
--- a/tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php
+++ b/tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php
@@ -4,7 +4,6 @@ namespace MediaWiki\Tests\Revision;
 
 use ActorMigration;
 use CommentStore;
-use MediaWiki\Logger\Spi as LoggerSpi;
 use MediaWiki\Revision\RevisionStore;
 use MediaWiki\Revision\RevisionStoreFactory;
 use MediaWiki\Revision\SlotRoleRegistry;
@@ -35,7 +34,7 @@ class RevisionStoreFactoryTest extends \MediaWikiIntegrationTestCase {
 			$this->getMockCommentStore(),
 			ActorMigration::newMigration(),
 			MIGRATION_OLD,
-			$this->getMockLoggerSpi(),
+			new NullLogger(),
 			true
 		);
 		$this->assertTrue( true );
@@ -65,7 +64,7 @@ class RevisionStoreFactoryTest extends \MediaWikiIntegrationTestCase {
 		$cache = $this->getHashWANObjectCache();
 		$commentStore = $this->getMockCommentStore();
 		$actorMigration = ActorMigration::newMigration();
-		$loggerProvider = $this->getMockLoggerSpi();
+		$logger = new NullLogger();
 
 		$factory = new RevisionStoreFactory(
 			$lbFactory,
@@ -76,7 +75,7 @@ class RevisionStoreFactoryTest extends \MediaWikiIntegrationTestCase {
 			$commentStore,
 			$actorMigration,
 			$mcrMigrationStage,
-			$loggerProvider,
+			$logger,
 			$contentHandlerUseDb
 		);
 
@@ -178,16 +177,4 @@ class RevisionStoreFactoryTest extends \MediaWikiIntegrationTestCase {
 		return new WANObjectCache( [ 'cache' => new \HashBagOStuff() ] );
 	}
 
-	/**
-	 * @return \PHPUnit_Framework_MockObject_MockObject|LoggerSpi
-	 */
-	private function getMockLoggerSpi() {
-		$mock = $this->getMock( LoggerSpi::class );
-
-		$mock->method( 'getLogger' )
-			->willReturn( new NullLogger() );
-
-		return $mock;
-	}
-
 }
-- 
2.20.1