OATHUserRepository: rely less on global state

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

Change-Id: I194345974146517c8216a81330cd930534d655e4
This commit is contained in:
Taavi Väänänen 2022-12-31 20:48:09 +02:00
parent ea97465b99
commit f3382dd3d1
No known key found for this signature in database
GPG key ID: EF242F709F912FBE
2 changed files with 56 additions and 35 deletions

View file

@ -1,7 +1,9 @@
<?php <?php
use MediaWiki\Config\ServiceOptions;
use MediaWiki\Extension\OATHAuth\OATHAuth; use MediaWiki\Extension\OATHAuth\OATHAuth;
use MediaWiki\Extension\OATHAuth\OATHUserRepository; use MediaWiki\Extension\OATHAuth\OATHUserRepository;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MediaWikiServices; use MediaWiki\MediaWikiServices;
return [ return [
@ -12,14 +14,18 @@ return [
); );
}, },
'OATHUserRepository' => static function ( MediaWikiServices $services ) { 'OATHUserRepository' => static function ( MediaWikiServices $services ) {
global $wgOATHAuthDatabase;
$auth = $services->getService( 'OATHAuth' );
return new OATHUserRepository( return new OATHUserRepository(
$services->getDBLoadBalancerFactory()->getMainLB( $wgOATHAuthDatabase ), new ServiceOptions(
new \HashBagOStuff( [ OATHUserRepository::CONSTRUCTOR_OPTIONS,
$services->getMainConfig(),
),
$services->getDBLoadBalancerFactory(),
new HashBagOStuff( [
'maxKey' => 5 'maxKey' => 5
] ), ] ),
$auth $services->getService( 'OATHAuth' ),
$services->getCentralIdLookupFactory(),
LoggerFactory::getInstance( 'authentication' )
); );
} }
]; ];

View file

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