ConsequencesFactory: Avoid creating Session object during service wiring

Service wiring should only depend on config, not on request state.

Creating a session object during service wiring causes issues with entry
points such as opensearch_desc.php that disable the session.

Bug: T340113
Change-Id: I2450b0b6821ff0b097e283ff660a0b8aeea9590a
This commit is contained in:
Abijeet 2023-06-27 19:47:47 +05:30
parent 683681eae6
commit b1e404fc79
3 changed files with 29 additions and 11 deletions

View file

@ -20,6 +20,7 @@ use MediaWiki\Extension\AbuseFilter\Consequences\Consequence\Warn;
use MediaWiki\Extension\AbuseFilter\FilterUser;
use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
use MediaWiki\Session\Session;
use MediaWiki\Session\SessionManager;
use MediaWiki\User\UserEditTracker;
use MediaWiki\User\UserFactory;
use MediaWiki\User\UserGroupManager;
@ -64,9 +65,6 @@ class ConsequencesFactory {
/** @var FilterUser */
private $filterUser;
/** @var Session */
private $session;
/** @var MessageLocalizer */
private $messageLocalizer;
@ -79,6 +77,9 @@ class ConsequencesFactory {
/** @var UserNameUtils */
private $userNameUtils;
/** @var ?Session */
private $session;
/**
* @todo This might drag in unwanted dependencies. The alternative is to use ObjectFactory, but that's harder
* to understand for humans and static analysis tools, so do that only if the dependencies list starts growing.
@ -91,7 +92,6 @@ class ConsequencesFactory {
* @param ChangeTagger $changeTagger
* @param BlockAutopromoteStore $blockAutopromoteStore
* @param FilterUser $filterUser
* @param Session $session
* @param MessageLocalizer $messageLocalizer
* @param UserEditTracker $userEditTracker
* @param UserFactory $userFactory
@ -107,7 +107,6 @@ class ConsequencesFactory {
ChangeTagger $changeTagger,
BlockAutopromoteStore $blockAutopromoteStore,
FilterUser $filterUser,
Session $session,
MessageLocalizer $messageLocalizer,
UserEditTracker $userEditTracker,
UserFactory $userFactory,
@ -123,7 +122,6 @@ class ConsequencesFactory {
$this->changeTagger = $changeTagger;
$this->blockAutopromoteStore = $blockAutopromoteStore;
$this->filterUser = $filterUser;
$this->session = $session;
$this->messageLocalizer = $messageLocalizer;
$this->userEditTracker = $userEditTracker;
$this->userFactory = $userFactory;
@ -222,7 +220,7 @@ class ConsequencesFactory {
* @return Warn
*/
public function newWarn( Parameters $params, string $message ): Warn {
return new Warn( $params, $message, $this->session );
return new Warn( $params, $message, $this->getSession() );
}
/**
@ -242,4 +240,23 @@ class ConsequencesFactory {
public function newTag( Parameters $params, array $tags ): Tag {
return new Tag( $params, $tags, $this->changeTagger );
}
/**
* @param Session $session
* @return void
*/
public function setSession( Session $session ): void {
$this->session = $session;
}
/**
* @return Session
*/
private function getSession(): Session {
if ( $this->session === null ) {
$this->session = SessionManager::getGlobalSession();
}
return $this->session;
}
}

View file

@ -43,7 +43,6 @@ use MediaWiki\Extension\AbuseFilter\Watcher\EmergencyWatcher;
use MediaWiki\Extension\AbuseFilter\Watcher\UpdateHitCountWatcher;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MediaWikiServices;
use MediaWiki\Session\SessionManager;
use MediaWiki\WikiMap\WikiMap;
use Wikimedia\Equivset\Equivset;
@ -211,7 +210,6 @@ return [
$services->get( ChangeTagger::SERVICE_NAME ),
$services->get( BlockAutopromoteStore::SERVICE_NAME ),
$services->get( FilterUser::SERVICE_NAME ),
SessionManager::getGlobalSession(),
// TODO: Use a proper MessageLocalizer once available (T247127)
RequestContext::getMain(),
$services->getUserEditTracker(),

View file

@ -39,7 +39,8 @@ class ConsequencesFactoryTest extends MediaWikiUnitTestCase {
'BlockCIDRLimit' => [],
]
);
return new ConsequencesFactory(
$consequencesFactory = new ConsequencesFactory(
$opts,
new NullLogger(),
$this->createMock( BlockUserFactory::class ),
@ -49,12 +50,14 @@ class ConsequencesFactoryTest extends MediaWikiUnitTestCase {
$this->createMock( ChangeTagger::class ),
$this->createMock( BlockAutopromoteStore::class ),
$this->createMock( FilterUser::class ),
$this->createMock( Session::class ),
$this->createMock( MessageLocalizer::class ),
$this->createMock( UserEditTracker::class ),
$this->createMock( UserFactory::class ),
$this->createMock( UserNameUtils::class )
);
$consequencesFactory->setSession( $this->createMock( Session::class ) );
return $consequencesFactory;
}
/**