From f3382dd3d175185e536d2da10b28258cf27d49da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Taavi=20V=C3=A4=C3=A4n=C3=A4nen?= <hi@taavi.wtf>
Date: Sat, 31 Dec 2022 20:48:09 +0200
Subject: [PATCH] OATHUserRepository: rely less on global state

Inject more stuff into OATHUserRepository properly. Also done other misc
cleanup on that class.

Change-Id: I194345974146517c8216a81330cd930534d655e4
---
 ServiceWiring.php          | 16 +++++---
 src/OATHUserRepository.php | 75 +++++++++++++++++++++++---------------
 2 files changed, 56 insertions(+), 35 deletions(-)

diff --git a/ServiceWiring.php b/ServiceWiring.php
index 3c6b3d81..0a7f8ca5 100644
--- a/ServiceWiring.php
+++ b/ServiceWiring.php
@@ -1,7 +1,9 @@
 <?php
 
+use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Extension\OATHAuth\OATHAuth;
 use MediaWiki\Extension\OATHAuth\OATHUserRepository;
+use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 
 return [
@@ -12,14 +14,18 @@ return [
 		);
 	},
 	'OATHUserRepository' => static function ( MediaWikiServices $services ) {
-		global $wgOATHAuthDatabase;
-		$auth = $services->getService( 'OATHAuth' );
 		return new OATHUserRepository(
-			$services->getDBLoadBalancerFactory()->getMainLB( $wgOATHAuthDatabase ),
-			new \HashBagOStuff( [
+			new ServiceOptions(
+				OATHUserRepository::CONSTRUCTOR_OPTIONS,
+				$services->getMainConfig(),
+			),
+			$services->getDBLoadBalancerFactory(),
+			new HashBagOStuff( [
 				'maxKey' => 5
 			] ),
-			$auth
+			$services->getService( 'OATHAuth' ),
+			$services->getCentralIdLookupFactory(),
+			LoggerFactory::getInstance( 'authentication' )
 		);
 	}
 ];
diff --git a/src/OATHUserRepository.php b/src/OATHUserRepository.php
index bc6700a0..42aea423 100644
--- a/src/OATHUserRepository.php
+++ b/src/OATHUserRepository.php
@@ -21,42 +21,64 @@ namespace MediaWiki\Extension\OATHAuth;
 use BagOStuff;
 use ConfigException;
 use FormatJson;
-use MediaWiki\Logger\LoggerFactory;
-use MediaWiki\MediaWikiServices;
+use MediaWiki\Config\ServiceOptions;
+use MediaWiki\User\CentralId\CentralIdLookupFactory;
 use MWException;
+use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
 use RequestContext;
 use User;
 use Wikimedia\Rdbms\IDatabase;
-use Wikimedia\Rdbms\ILoadBalancer;
+use Wikimedia\Rdbms\LBFactory;
 
-class OATHUserRepository {
-	/** @var ILoadBalancer */
-	protected $lb;
+class OATHUserRepository implements LoggerAwareInterface {
+	/** @var ServiceOptions */
+	private ServiceOptions $options;
+
+	/** @var LBFactory */
+	private LBFactory $lbFactory;
 
 	/** @var BagOStuff */
-	protected $cache;
+	private BagOStuff $cache;
 
-	/**
-	 * @var OATHAuth
-	 */
-	protected $auth;
+	/** @var OATHAuth */
+	private OATHAuth $auth;
+
+	/** @var CentralIdLookupFactory */
+	private CentralIdLookupFactory $centralIdLookupFactory;
 
 	/** @var LoggerInterface */
-	private $logger;
+	private LoggerInterface $logger;
+
+	/** @internal Only public for service wiring use. */
+	public const CONSTRUCTOR_OPTIONS = [
+		'OATHAuthDatabase',
+	];
 
 	/**
 	 * OATHUserRepository constructor.
-	 * @param ILoadBalancer $lb
+	 * @param ServiceOptions $options
+	 * @param LBFactory $lbFactory
 	 * @param BagOStuff $cache
 	 * @param OATHAuth $auth
+	 * @param CentralIdLookupFactory $centralIdLookupFactory
+	 * @param LoggerInterface $logger
 	 */
-	public function __construct( ILoadBalancer $lb, BagOStuff $cache, OATHAuth $auth ) {
-		$this->lb = $lb;
+	public function __construct(
+		ServiceOptions $options,
+		LBFactory $lbFactory,
+		BagOStuff $cache,
+		OATHAuth $auth,
+		CentralIdLookupFactory $centralIdLookupFactory,
+		LoggerInterface $logger
+	) {
+		$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
+		$this->options = $options;
+		$this->lbFactory = $lbFactory;
 		$this->cache = $cache;
 		$this->auth = $auth;
-
-		$this->setLogger( LoggerFactory::getInstance( 'authentication' ) );
+		$this->centralIdLookupFactory = $centralIdLookupFactory;
+		$this->setLogger( $logger );
 	}
 
 	/**
@@ -77,9 +99,7 @@ class OATHUserRepository {
 		if ( !$oathUser ) {
 			$oathUser = new OATHUser( $user, [] );
 
-			$uid = MediaWikiServices::getInstance()
-				->getCentralIdLookupFactory()
-				->getLookup()
+			$uid = $this->centralIdLookupFactory->getLookup()
 				->centralIdFromLocalUser( $user );
 			$res = $this->getDB( DB_REPLICA )->selectRow(
 				'oathauth_users',
@@ -125,9 +145,7 @@ class OATHUserRepository {
 			'oathauth_users',
 			'id',
 			[
-				'id' => MediaWikiServices::getInstance()
-					->getCentralIdLookupFactory()
-					->getLookup()
+				'id' => $this->centralIdLookupFactory->getLookup()
 					->centralIdFromLocalUser( $user->getUser() ),
 				'module' => $user->getModule()->getName(),
 				'data' => FormatJson::encode( $data )
@@ -164,9 +182,7 @@ class OATHUserRepository {
 	public function remove( OATHUser $user, $clientInfo, bool $self ) {
 		$this->getDB( DB_PRIMARY )->delete(
 			'oathauth_users',
-			[ 'id' => MediaWikiServices::getInstance()
-				->getCentralIdLookupFactory()
-				->getLookup()
+			[ 'id' => $this->centralIdLookupFactory->getLookup()
 				->centralIdFromLocalUser( $user->getUser() ) ],
 			__METHOD__
 		);
@@ -186,9 +202,8 @@ class OATHUserRepository {
 	 * @param int $index DB_PRIMARY/DB_REPLICA
 	 * @return IDatabase
 	 */
-	private function getDB( $index ) {
-		global $wgOATHAuthDatabase;
-
-		return $this->lb->getConnectionRef( $index, [], $wgOATHAuthDatabase );
+	private function getDB( int $index ): IDatabase {
+		$db = $this->options->get( 'OATHAuthDatabase' );
+		return $this->lbFactory->getMainLB( $db )->getConnectionRef( $index, [], $db );
 	}
 }