From 3d5539720768e4e49d5fb9634a1031287abe1285 Mon Sep 17 00:00:00 2001 From: daniel Date: Sun, 1 Oct 2023 14:03:37 +0200 Subject: [PATCH] Move creation of BlockErrorFormatter into FormatterFactory The idea is that all formatters that need the user language or other request specific context should be instantiated by FormatterFactory. Change-Id: I8334cc89dcf0f293298b82e004116be50a90f0d1 --- RELEASE-NOTES-1.42 | 9 ++ autoload.php | 2 + includes/MediaWikiServices.php | 2 + includes/Permissions/PermissionManager.php | 1 - includes/Permissions/UserAuthority.php | 1 - includes/ServiceWiring.php | 23 ++- includes/api/ApiBase.php | 5 +- includes/block/AbstractBlock.php | 5 +- includes/block/BlockErrorFormatter.php | 75 +++++----- includes/context/ContextSource.php | 11 ++ includes/context/IContextSource.php | 5 +- includes/context/RequestContext.php | 9 ++ includes/editpage/EditPage.php | 4 + includes/exception/UserBlockedError.php | 15 +- includes/language/FormatterFactory.php | 34 ++++- includes/language/LazyLocalizationContext.php | 58 ++++++++ includes/language/LocalizationContext.php | 40 ++++++ includes/user/User.php | 7 +- .../includes/MediaWikiServicesTest.php | 1 + .../block/BlockErrorFormatterTest.php | 132 +++++++++++++----- .../exception/UserBlockedErrorTest.php | 13 +- .../PermissionStatusIntegrationTest.php | 6 +- .../language/FormatterFactoryTest.php | 45 ++++++ 23 files changed, 397 insertions(+), 106 deletions(-) create mode 100644 includes/language/LazyLocalizationContext.php create mode 100644 includes/language/LocalizationContext.php create mode 100644 tests/phpunit/unit/includes/language/FormatterFactoryTest.php diff --git a/RELEASE-NOTES-1.42 b/RELEASE-NOTES-1.42 index 801c599263d..9c48662cf9d 100644 --- a/RELEASE-NOTES-1.42 +++ b/RELEASE-NOTES-1.42 @@ -456,6 +456,15 @@ because of Phabricator reports. Use PageUpdater instance. * SpecialBlock::getSuggestedDurations() has been deprecated, use Language::getBlockDurations() instead. +* MediaWikiServices::getBlockErrorFormatter() has been deprecated, use + MediaWikiServices::getFormatterFactory()->getBlockErrorFormatter() instead. +* (T352284) Some user options-related classes were moved over to the existing + MediaWiki\User\Options namespace. Old names are deprecated and kept as + aliases for backwards-compatibility: + - DefaultOptionsLookup + - UserOptionsLookup + - UserOptionsManager + - StaticUserOptionsLookup * Database::listViews() has been deprecated. This was previously used to filter views out of the return value of Database::listTables(). Now listTables() will not include views. MediaWiki does not use views. diff --git a/autoload.php b/autoload.php index a78e1e57ac7..f229e2d440b 100644 --- a/autoload.php +++ b/autoload.php @@ -1527,6 +1527,8 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Json\\JsonUnserializableTrait' => __DIR__ . '/includes/json/JsonUnserializableTrait.php', 'MediaWiki\\Json\\JsonUnserializer' => __DIR__ . '/includes/json/JsonUnserializer.php', 'MediaWiki\\Language\\FormatterFactory' => __DIR__ . '/includes/language/FormatterFactory.php', + 'MediaWiki\\Language\\LazyLocalizationContext' => __DIR__ . '/includes/language/LazyLocalizationContext.php', + 'MediaWiki\\Language\\LocalizationContext' => __DIR__ . '/includes/language/LocalizationContext.php', 'MediaWiki\\Language\\RawMessage' => __DIR__ . '/includes/language/RawMessage.php', 'MediaWiki\\Languages\\Data\\CrhExceptions' => __DIR__ . '/includes/languages/data/CrhExceptions.php', 'MediaWiki\\Languages\\Data\\Names' => __DIR__ . '/includes/languages/data/Names.php', diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index d8ddbb13beb..9c55f256933 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -773,8 +773,10 @@ class MediaWikiServices extends ServiceContainer { /** * @since 1.35 + * @deprecated since 1.42, use getFormatterFactory()->getBlockErrorFormatter() instead. */ public function getBlockErrorFormatter(): BlockErrorFormatter { + wfDeprecated( __METHOD__, '1.42' ); return $this->getService( 'BlockErrorFormatter' ); } diff --git a/includes/Permissions/PermissionManager.php b/includes/Permissions/PermissionManager.php index 0ed7e7dfab4..885a017d97c 100644 --- a/includes/Permissions/PermissionManager.php +++ b/includes/Permissions/PermissionManager.php @@ -833,7 +833,6 @@ class PermissionManager { $messages = $this->blockErrorFormatter->getMessages( $block, $user, - $context->getLanguage(), $context->getRequest()->getIP() ); diff --git a/includes/Permissions/UserAuthority.php b/includes/Permissions/UserAuthority.php index 11b8cc0078b..216811e1ea0 100644 --- a/includes/Permissions/UserAuthority.php +++ b/includes/Permissions/UserAuthority.php @@ -304,7 +304,6 @@ class UserAuthority implements Authority { $messages = $this->blockErrorFormatter->getMessages( $userBlock, $this->actor, - $this->uiContext->getLanguage(), $this->request->getIP() ); diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index aa337e13b06..7737a906e7a 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -103,6 +103,7 @@ use MediaWiki\JobQueue\JobFactory; use MediaWiki\JobQueue\JobQueueGroupFactory; use MediaWiki\Json\JsonCodec; use MediaWiki\Language\FormatterFactory; +use MediaWiki\Language\LazyLocalizationContext; use MediaWiki\Languages\LanguageConverterFactory; use MediaWiki\Languages\LanguageFactory; use MediaWiki\Languages\LanguageFallback; @@ -369,10 +370,10 @@ return [ }, 'BlockErrorFormatter' => static function ( MediaWikiServices $services ): BlockErrorFormatter { - return new BlockErrorFormatter( - $services->getTitleFormatter(), - $services->getHookContainer(), - $services->getUserIdentityUtils() + return $services->getFormatterFactory()->getBlockErrorFormatter( + new LazyLocalizationContext( static function () { + return RequestContext::getMain(); + } ) ); }, @@ -843,7 +844,13 @@ return [ }, 'FormatterFactory' => static function ( MediaWikiServices $services ): FormatterFactory { - return new FormatterFactory( $services->getMessageCache() ); + return new FormatterFactory( + $services->getMessageCache(), + $services->getTitleFormatter(), + $services->getHookContainer(), + $services->getUserIdentityUtils(), + $services->getLanguageFactory() + ); }, 'GenderCache' => static function ( MediaWikiServices $services ): GenderCache { @@ -1704,7 +1711,11 @@ return [ $services->getGroupPermissionsLookup(), $services->getUserGroupManager(), $services->getBlockManager(), - $services->getBlockErrorFormatter(), + $services->getFormatterFactory()->getBlockErrorFormatter( + new LazyLocalizationContext( static function () { + return RequestContext::getMain(); + } ) + ), $services->getHookContainer(), $services->getUserCache(), $services->getRedirectLookup(), diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 3cf1a0a3bda..be747d195d8 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -1556,12 +1556,13 @@ abstract class ApiBase extends ContextSource { * @return never */ public function dieBlocked( Block $block ) { - $blockErrorFormatter = MediaWikiServices::getInstance()->getBlockErrorFormatter(); + $blockErrorFormatter = MediaWikiServices::getInstance()->getFormatterFactory() + ->getBlockErrorFormatter( $this->getContext() ); $msg = $blockErrorFormatter->getMessage( $block, $this->getUser(), - $this->getLanguage(), + null, $this->getRequest()->getIP() ); diff --git a/includes/block/AbstractBlock.php b/includes/block/AbstractBlock.php index adfd4be7fc9..684f6c55676 100644 --- a/includes/block/AbstractBlock.php +++ b/includes/block/AbstractBlock.php @@ -425,10 +425,11 @@ abstract class AbstractBlock implements Block { public function getPermissionsError( IContextSource $context ) { wfDeprecated( __METHOD__, '1.35' ); $message = MediaWikiServices::getInstance() - ->getBlockErrorFormatter()->getMessage( + ->getFormatterFactory()->getBlockErrorFormatter( $context ) + ->getMessage( $this, $context->getUser(), - $context->getLanguage(), + null, $context->getRequest()->getIP() ); return array_merge( [ $message->getKey() ], $message->getParams() ); diff --git a/includes/block/BlockErrorFormatter.php b/includes/block/BlockErrorFormatter.php index 61653a80ce7..cb23a6f42f4 100644 --- a/includes/block/BlockErrorFormatter.php +++ b/includes/block/BlockErrorFormatter.php @@ -24,6 +24,8 @@ use Language; use MediaWiki\CommentStore\CommentStoreComment; use MediaWiki\HookContainer\HookContainer; use MediaWiki\HookContainer\HookRunner; +use MediaWiki\Language\LocalizationContext; +use MediaWiki\Languages\LanguageFactory; use MediaWiki\Page\PageReferenceValue; use MediaWiki\Title\TitleFormatter; use MediaWiki\User\UserIdentity; @@ -38,30 +40,32 @@ use Message; */ class BlockErrorFormatter { - /** @var TitleFormatter */ - private $titleFormatter; + private TitleFormatter $titleFormatter; + private HookRunner $hookRunner; + private UserIdentityUtils $userIdentityUtils; + private LocalizationContext $uiContext; + private LanguageFactory $languageFactory; - /** @var HookRunner */ - private $hookRunner; - - /** - * @var UserIdentityUtils - */ - private $userIdentityUtils; - - /** - * @param TitleFormatter $titleFormatter - * @param HookContainer $hookContainer - * @param UserIdentityUtils $userIdentityUtils - */ public function __construct( TitleFormatter $titleFormatter, HookContainer $hookContainer, - UserIdentityUtils $userIdentityUtils + UserIdentityUtils $userIdentityUtils, + LanguageFactory $languageFactory, + LocalizationContext $uiContext ) { $this->titleFormatter = $titleFormatter; $this->hookRunner = new HookRunner( $hookContainer ); $this->userIdentityUtils = $userIdentityUtils; + + $this->languageFactory = $languageFactory; + $this->uiContext = $uiContext; + } + + /** + * @return Language + */ + private function getLanguage(): Language { + return $this->languageFactory->getLanguage( $this->uiContext->getLanguageCode() ); } /** @@ -74,19 +78,19 @@ class BlockErrorFormatter { * * @param Block $block * @param UserIdentity $user - * @param Language $language + * @param mixed $language Unused since 1.42 * @param string $ip * @return Message */ public function getMessage( Block $block, UserIdentity $user, - Language $language, + $language, string $ip ): Message { $key = $this->getBlockErrorMessageKey( $block, $user ); - $params = $this->getBlockErrorMessageParams( $block, $user, $language, $ip ); - return new Message( $key, $params ); + $params = $this->getBlockErrorMessageParams( $block, $user, $ip ); + return $this->uiContext->msg( $key, $params ); } /** @@ -95,19 +99,17 @@ class BlockErrorFormatter { * @since 1.42 * @param Block $block * @param UserIdentity $user - * @param Language $language * @param string $ip * @return Message[] */ public function getMessages( Block $block, UserIdentity $user, - Language $language, string $ip ): array { $messages = []; foreach ( $block->toArray() as $singleBlock ) { - $messages[] = $this->getMessage( $singleBlock, $user, $language, $ip ); + $messages[] = $this->getMessage( $singleBlock, $user, null, $ip ); } return $messages; @@ -144,16 +146,16 @@ class BlockErrorFormatter { * @since 1.35 * @param Block $block * @param UserIdentity $user - * @param Language $language * @return mixed[] See getBlockErrorInfo */ private function getFormattedBlockErrorInfo( Block $block, - UserIdentity $user, - Language $language + UserIdentity $user ) { $info = $this->getBlockErrorInfo( $block ); + $language = $this->getLanguage(); + $info['expiry'] = $language->formatExpiry( $info['expiry'], true, 'infinity', $user ); $info['timestamp'] = $language->userTimeAndDate( $info['timestamp'], $user ); $info['blockerName'] = $language->embedBidi( $info['blockerName'] ); @@ -185,16 +187,17 @@ class BlockErrorFormatter { * case their page cannot be linked. * * @param ?UserIdentity $blocker - * @param Language $language * @return string Link to the blocker's page; blocker's name if not a local user */ - private function formatBlockerLink( ?UserIdentity $blocker, Language $language ) { + private function formatBlockerLink( ?UserIdentity $blocker ) { if ( !$blocker ) { // TODO should we say something? This is just matching the code before // the refactoring in late July 2021 return ''; } + $language = $this->getLanguage(); + if ( $blocker->getId() === 0 ) { // Foreign user // TODO what about blocks placed by IPs? Shouldn't we check based on @@ -245,7 +248,6 @@ class BlockErrorFormatter { * * @param Block $block * @param UserIdentity $user - * @param Language $language * @param string $ip * @return mixed[] Params used by standard block error messages, in order: * - blockerLink: Link to the blocker's user page, if any; otherwise same as blockerName @@ -260,30 +262,26 @@ class BlockErrorFormatter { private function getBlockErrorMessageParams( Block $block, UserIdentity $user, - Language $language, string $ip ) { - $info = $this->getFormattedBlockErrorInfo( $block, $user, $language ); + $info = $this->getFormattedBlockErrorInfo( $block, $user ); // Add params that are specific to the standard block errors $info['ip'] = $ip; - $info['blockerLink'] = $this->formatBlockerLink( - $block->getBlocker(), - $language - ); + $info['blockerLink'] = $this->formatBlockerLink( $block->getBlocker() ); // Display the CompositeBlock identifier as a message containing relevant block IDs if ( $block instanceof CompositeBlock ) { - $ids = $language->commaList( array_map( + $ids = $this->getLanguage()->commaList( array_map( static function ( $id ) { return '#' . $id; }, array_filter( $info['identifier'], 'is_int' ) ) ); if ( $ids === '' ) { - $idsMsg = new Message( 'blockedtext-composite-no-ids', [], $language ); + $idsMsg = $this->uiContext->msg( 'blockedtext-composite-no-ids', [] ); } else { - $idsMsg = new Message( 'blockedtext-composite-ids', [ $ids ], $language ); + $idsMsg = $this->uiContext->msg( 'blockedtext-composite-ids', [ $ids ] ); } $info['identifier'] = $idsMsg->plain(); } @@ -307,4 +305,5 @@ class BlockErrorFormatter { return $params; } + } diff --git a/includes/context/ContextSource.php b/includes/context/ContextSource.php index 8f772f4e38d..135150b2c7e 100644 --- a/includes/context/ContextSource.php +++ b/includes/context/ContextSource.php @@ -25,6 +25,7 @@ use MediaWiki\Request\WebRequest; use MediaWiki\Session\CsrfTokenSet; use MediaWiki\Title\Title; use MediaWiki\User\User; +use Wikimedia\Bcp47Code\Bcp47Code; use Wikimedia\NonSerializable\NonSerializableTrait; /** @@ -170,6 +171,16 @@ abstract class ContextSource implements IContextSource { return $this->getContext()->getLanguage(); } + /** + * @since 1.42 + * @stable to override + * @note When overriding, keep consistent with getLanguage()! + * @return Bcp47Code + */ + public function getLanguageCode(): Bcp47Code { + return $this->getLanguage(); + } + /** * @since 1.18 * @stable to override diff --git a/includes/context/IContextSource.php b/includes/context/IContextSource.php index 2c301259691..298b03a8235 100644 --- a/includes/context/IContextSource.php +++ b/includes/context/IContextSource.php @@ -19,6 +19,7 @@ */ use MediaWiki\Config\Config; +use MediaWiki\Language\LocalizationContext; use MediaWiki\Output\OutputPage; use MediaWiki\Permissions\Authority; use MediaWiki\Request\WebRequest; @@ -61,7 +62,7 @@ use MediaWiki\User\User; * @stable to type * @author Happy-melon */ -interface IContextSource extends MessageLocalizer, CsrfTokenSetProvider { +interface IContextSource extends LocalizationContext, CsrfTokenSetProvider { /** * @return WebRequest @@ -121,7 +122,7 @@ interface IContextSource extends MessageLocalizer, CsrfTokenSetProvider { public function getAuthority(): Authority; /** - * @return Language + * @inheritDoc * @since 1.19 */ public function getLanguage(); diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php index 0ec856c13d9..0645478a605 100644 --- a/includes/context/RequestContext.php +++ b/includes/context/RequestContext.php @@ -37,6 +37,7 @@ use MediaWiki\Title\Title; use MediaWiki\User\User; use Wikimedia\Assert\Assert; use Wikimedia\AtEase\AtEase; +use Wikimedia\Bcp47Code\Bcp47Code; use Wikimedia\IPUtils; use Wikimedia\NonSerializable\NonSerializableTrait; use Wikimedia\ScopedCallback; @@ -489,6 +490,14 @@ class RequestContext implements IContextSource, MutableContext { return $this->lang; } + /** + * @since 1.42 + * @return Bcp47Code + */ + public function getLanguageCode() { + return $this->getLanguage(); + } + /** * @param Skin $skin */ diff --git a/includes/editpage/EditPage.php b/includes/editpage/EditPage.php index 1321994c920..99235262ed5 100644 --- a/includes/editpage/EditPage.php +++ b/includes/editpage/EditPage.php @@ -32,6 +32,7 @@ use IContextSource; use IDBAccessObject; use LogPage; use ManualLogEntry; +use MediaWiki\Block\BlockErrorFormatter; use MediaWiki\Cache\LinkBatchFactory; use MediaWiki\CommentStore\CommentStore; use MediaWiki\CommentStore\CommentStoreComment; @@ -447,6 +448,7 @@ class EditPage implements IEditObject { private TempUserCreator $tempUserCreator; private UserFactory $userFactory; private IConnectionProvider $connectionProvider; + private BlockErrorFormatter $blockErrorFormatter; /** @var User|null */ private $placeholderTempUser; @@ -516,6 +518,8 @@ class EditPage implements IEditObject { $this->restrictionStore = $services->getRestrictionStore(); $this->commentStore = $services->getCommentStore(); $this->connectionProvider = $services->getConnectionProvider(); + $this->blockErrorFormatter = $services->getFormatterFactory() + ->getBlockErrorFormatter( $this->context ); // XXX: Restore this deprecation as soon as TwoColConflict is fixed (T305028) // $this->deprecatePublicProperty( 'textbox2', '1.38', __CLASS__ ); diff --git a/includes/exception/UserBlockedError.php b/includes/exception/UserBlockedError.php index f8c9b93533c..8dfdeb79177 100644 --- a/includes/exception/UserBlockedError.php +++ b/includes/exception/UserBlockedError.php @@ -35,26 +35,27 @@ class UserBlockedError extends ErrorPageError { * @stable to call * @param Block $block * @param UserIdentity|null $user - * @param Language|null $language + * @param mixed $language Unused since 1.42 * @param string|null $ip */ public function __construct( Block $block, UserIdentity $user = null, - Language $language = null, + $language = null, $ip = null ) { - if ( $user === null || $language === null || $ip === null ) { + $context = RequestContext::getMain(); + if ( $user === null || $ip === null ) { // If any of these are not passed in, use the global context - $context = RequestContext::getMain(); $user = $context->getUser(); - $language = $context->getLanguage(); $ip = $context->getRequest()->getIP(); } // @todo This should be passed in via the constructor - $messages = MediaWikiServices::getInstance()->getBlockErrorFormatter() - ->getMessages( $block, $user, $language, $ip ); + $messages = MediaWikiServices::getInstance()->getFormatterFactory() + ->getBlockErrorFormatter( $context ) + ->getMessages( $block, $user, $ip ); + if ( count( $messages ) === 1 ) { $message = $messages[0]; } else { diff --git a/includes/language/FormatterFactory.php b/includes/language/FormatterFactory.php index a93d38cb369..c792debbeba 100644 --- a/includes/language/FormatterFactory.php +++ b/includes/language/FormatterFactory.php @@ -2,7 +2,12 @@ namespace MediaWiki\Language; +use MediaWiki\Block\BlockErrorFormatter; +use MediaWiki\HookContainer\HookContainer; +use MediaWiki\Languages\LanguageFactory; use MediaWiki\Status\StatusFormatter; +use MediaWiki\Title\TitleFormatter; +use MediaWiki\User\UserIdentityUtils; use MessageCache; use MessageLocalizer; @@ -14,16 +19,37 @@ use MessageLocalizer; class FormatterFactory { private MessageCache $messageCache; + private TitleFormatter $titleFormatter; + private HookContainer $hookContainer; + private UserIdentityUtils $userIdentityUtils; + private LanguageFactory $languageFactory; - /** - * @param MessageCache $messageCache - */ - public function __construct( MessageCache $messageCache ) { + public function __construct( + MessageCache $messageCache, + TitleFormatter $titleFormatter, + HookContainer $hookContainer, + UserIdentityUtils $userIdentityUtils, + LanguageFactory $languageFactory + ) { $this->messageCache = $messageCache; + $this->titleFormatter = $titleFormatter; + $this->hookContainer = $hookContainer; + $this->userIdentityUtils = $userIdentityUtils; + $this->languageFactory = $languageFactory; } public function getStatusFormatter( MessageLocalizer $messageLocalizer ): StatusFormatter { return new StatusFormatter( $messageLocalizer, $this->messageCache ); } + public function getBlockErrorFormatter( LocalizationContext $context ): BlockErrorFormatter { + return new BlockErrorFormatter( + $this->titleFormatter, + $this->hookContainer, + $this->userIdentityUtils, + $this->languageFactory, + $context + ); + } + } diff --git a/includes/language/LazyLocalizationContext.php b/includes/language/LazyLocalizationContext.php new file mode 100644 index 00000000000..81be6aa4f02 --- /dev/null +++ b/includes/language/LazyLocalizationContext.php @@ -0,0 +1,58 @@ +instantiator = $instantiator; + } + + private function resolve(): LocalizationContext { + if ( !$this->context ) { + $this->context = ( $this->instantiator )(); + } + + return $this->context; + } + + public function getLanguageCode() { + return $this->resolve()->getLanguageCode(); + } + + public function msg( $key, ...$params ) { + return $this->resolve()->msg( $key, ...$params ); + } +} diff --git a/includes/language/LocalizationContext.php b/includes/language/LocalizationContext.php new file mode 100644 index 00000000000..532a2393056 --- /dev/null +++ b/includes/language/LocalizationContext.php @@ -0,0 +1,40 @@ +getRequest(); $uiContext = RequestContext::getMain(); + $services = MediaWikiServices::getInstance(); $this->mThisAsAuthority = new UserAuthority( $this, $request, $uiContext, - MediaWikiServices::getInstance()->getPermissionManager(), - MediaWikiServices::getInstance()->getRateLimiter(), - MediaWikiServices::getInstance()->getBlockErrorFormatter() + $services->getPermissionManager(), + $services->getRateLimiter(), + $services->getFormatterFactory()->getBlockErrorFormatter( $uiContext ) ); } diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php index 48962ec6bee..82a9e11febc 100644 --- a/tests/phpunit/includes/MediaWikiServicesTest.php +++ b/tests/phpunit/includes/MediaWikiServicesTest.php @@ -19,6 +19,7 @@ use Wikimedia\Services\SalvageableService; */ class MediaWikiServicesTest extends MediaWikiIntegrationTestCase { private $deprecatedServices = [ + 'BlockErrorFormatter', 'ConfigRepository', 'ConfiguredReadOnlyMode', ]; diff --git a/tests/phpunit/includes/block/BlockErrorFormatterTest.php b/tests/phpunit/includes/block/BlockErrorFormatterTest.php index 07ac13ec8eb..72493d8db24 100644 --- a/tests/phpunit/includes/block/BlockErrorFormatterTest.php +++ b/tests/phpunit/includes/block/BlockErrorFormatterTest.php @@ -1,5 +1,6 @@ setLanguage( + $this->getServiceContainer() + ->getLanguageFactory()->getLanguage( 'qqx' ) + ); + + return $context; + } + + private function getBlockErrorFormatter( IContextSource $context ): BlockErrorFormatter { + return $this->getServiceContainer() + ->getFormatterFactory()->getBlockErrorFormatter( $context ); + } + protected function setUp(): void { parent::setUp(); - $db = $this->createNoOpMock( IDatabase::class, [ 'getInfinity' ] ); + $db = $this->createMock( IDatabase::class ); $db->method( 'getInfinity' )->willReturn( 'infinity' ); - $lbFactory = $this->createMock( LBFactory::class ); + $db->method( 'decodeExpiry' )->willReturnArgument( 0 ); + + $lb = $this->createNoOpMock( + LoadBalancer::class, + [ 'getConnectionRef', 'getConnection' ] + ); + $lb->method( 'getConnectionRef' )->willReturn( $db ); + $lb->method( 'getConnection' )->willReturn( $db ); + + $lbFactory = $this->createNoOpMock( + LBFactory::class, + [ 'getReplicaDatabase', 'getPrimaryDatabase', 'getMainLB', ] + ); $lbFactory->method( 'getReplicaDatabase' )->willReturn( $db ); + $lbFactory->method( 'getPrimaryDatabase' )->willReturn( $db ); + $lbFactory->method( 'getMainLB' )->willReturn( $lb ); $this->setService( 'DBLoadBalancerFactory', $lbFactory ); } /** * @dataProvider provideTestGetMessage */ - public function testGetMessage( $block, $expectedKey, $expectedParams ) { - $context = new DerivativeContext( RequestContext::getMain() ); + public function testGetMessage( $blockClass, $blockData, $expectedKey, $expectedParams ) { + $block = $this->makeBlock( + $blockClass, + $blockData + ); + $context = $this->getContext(); - $formatter = $this->getServiceContainer()->getBlockErrorFormatter(); + $formatter = $this->getBlockErrorFormatter( $context ); $message = $formatter->getMessage( $block, $context->getUser(), - $this->getServiceContainer()->getLanguageFactory()->getLanguage( 'qqx' ), + $context->getLanguage(), '1.2.3.4' ); @@ -45,28 +84,29 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { $timestamp = '20000101000000'; $expiry = '20010101000000'; - $databaseBlock = new DatabaseBlock( [ + $databaseBlock = [ 'timestamp' => $timestamp, 'expiry' => $expiry, 'reason' => 'Test reason.', - ] ); + ]; - $systemBlock = new SystemBlock( [ + $systemBlock = [ 'timestamp' => $timestamp, 'systemBlock' => 'test', 'reason' => new Message( 'proxyblockreason' ), - ] ); + ]; - $compositeBlock = new CompositeBlock( [ + $compositeBlock = [ 'timestamp' => $timestamp, 'originalBlocks' => [ - $databaseBlock, - $systemBlock + [ DatabaseBlock::class, $databaseBlock ], + [ SystemBlock::class, $systemBlock ] ] - ] ); + ]; return [ 'Database block' => [ + DatabaseBlock::class, $databaseBlock, 'blockedtext', [ @@ -81,11 +121,12 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { ], ], 'Database block (autoblock)' => [ - new DatabaseBlock( [ + DatabaseBlock::class, + [ 'timestamp' => $timestamp, 'expiry' => $expiry, 'auto' => true, - ] ), + ], 'autoblockedtext', [ '', @@ -99,11 +140,12 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { ], ], 'Database block (partial block)' => [ - new DatabaseBlock( [ + DatabaseBlock::class, + [ 'timestamp' => $timestamp, 'expiry' => $expiry, 'sitewide' => false, - ] ), + ], 'blockedtext-partial', [ '', @@ -117,6 +159,7 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { ], ], 'System block (type \'test\')' => [ + SystemBlock::class, $systemBlock, 'systemblockedtext', [ @@ -131,11 +174,12 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { ], ], 'System block (type \'test\') with reason parameters' => [ - new SystemBlock( [ + SystemBlock::class, + [ 'timestamp' => $timestamp, 'systemBlock' => 'test', 'reason' => new Message( 'softblockrangesreason', [ '1.2.3.4' ] ), - ] ), + ], 'systemblockedtext', [ '', @@ -149,6 +193,7 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { ], ], 'Composite block (original blocks not inserted)' => [ + CompositeBlock::class, $compositeBlock, 'blockedtext-composite', [ @@ -177,7 +222,7 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { $context = RequestContext::getMain(); - $formatter = $this->getServiceContainer()->getBlockErrorFormatter(); + $formatter = $this->getBlockErrorFormatter( $context ); $this->assertContains( $expected, $formatter->getMessage( @@ -209,14 +254,18 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { /** * @dataProvider provideTestGetMessages */ - public function testGetMessages( $block, $expectedKeys ) { - $context = new DerivativeContext( RequestContext::getMain() ); + public function testGetMessages( $blockClass, $blockData, $expectedKeys ) { + $block = $this->makeBlock( + $blockClass, + $blockData + ); - $formatter = $this->getServiceContainer()->getBlockErrorFormatter(); + $context = $this->getContext(); + + $formatter = $this->getBlockErrorFormatter( $context ); $messages = $formatter->getMessages( $block, $context->getUser(), - $this->getServiceContainer()->getLanguageFactory()->getLanguage( 'qqx' ), '1.2.3.4' ); @@ -229,40 +278,57 @@ class BlockErrorFormatterTest extends MediaWikiIntegrationTestCase { $timestamp = '20000101000000'; $expiry = '20010101000000'; - $databaseBlock = new DatabaseBlock( [ + $databaseBlock = [ 'timestamp' => $timestamp, 'expiry' => $expiry, 'reason' => 'Test reason.', - ] ); + ]; - $systemBlock = new SystemBlock( [ + $systemBlock = [ 'timestamp' => $timestamp, 'systemBlock' => 'test', 'reason' => new Message( 'proxyblockreason' ), - ] ); + ]; - $compositeBlock = new CompositeBlock( [ + $compositeBlock = [ 'timestamp' => $timestamp, 'originalBlocks' => [ - $databaseBlock, - $systemBlock + [ DatabaseBlock::class, $databaseBlock ], + [ SystemBlock::class, $systemBlock ] ] - ] ); + ]; return [ 'Database block' => [ + DatabaseBlock::class, $databaseBlock, [ 'blockedtext' ], ], 'System block (type \'test\')' => [ + SystemBlock::class, $systemBlock, [ 'systemblockedtext' ], ], 'Composite block (original blocks not inserted)' => [ + CompositeBlock::class, $compositeBlock, [ 'blockedtext', 'systemblockedtext' ], ], ]; } + + /** + * @param string $blockClass + * @param array $blockData + * + * @return mixed + */ + private function makeBlock( $blockClass, $blockData ) { + foreach ( $blockData['originalBlocks'] ?? [] as $key => $originalBlock ) { + $blockData['originalBlocks'][$key] = $this->makeBlock( ...$originalBlock ); + } + + return new $blockClass( $blockData ); + } } diff --git a/tests/phpunit/includes/exception/UserBlockedErrorTest.php b/tests/phpunit/includes/exception/UserBlockedErrorTest.php index d9177e93e89..797dd07d637 100644 --- a/tests/phpunit/includes/exception/UserBlockedErrorTest.php +++ b/tests/phpunit/includes/exception/UserBlockedErrorTest.php @@ -2,6 +2,7 @@ use MediaWiki\Block\AbstractBlock; use MediaWiki\Block\BlockErrorFormatter; +use MediaWiki\Language\FormatterFactory; use MediaWiki\Language\RawMessage; use MediaWiki\User\UserIdentity; @@ -16,13 +17,13 @@ class UserBlockedErrorTest extends MediaWikiIntegrationTestCase { $mockBlockErrorFormatter = $this->createMock( BlockErrorFormatter::class ); $mockBlockErrorFormatter->expects( $this->once() ) ->method( 'getMessages' ) - ->with( $expectedBlock, $expectedUser, $expectedLanguage, $expectedIp ) + ->with( $expectedBlock, $expectedUser, $expectedIp ) ->willReturn( $returnMessages ); - $this->overrideMwServices( null, [ - 'BlockErrorFormatter' => static function () use ( $mockBlockErrorFormatter ) { - return $mockBlockErrorFormatter; - } - ] ); + + $formatterFactory = $this->createNoOpMock( FormatterFactory::class, [ 'getBlockErrorFormatter' ] ); + $formatterFactory->method( 'getBlockErrorFormatter' )->willReturn( $mockBlockErrorFormatter ); + + $this->setService( 'FormatterFactory', $formatterFactory ); } public function testConstructionProvidedOnlyBlockParameter() { diff --git a/tests/phpunit/integration/includes/Permissions/PermissionStatusIntegrationTest.php b/tests/phpunit/integration/includes/Permissions/PermissionStatusIntegrationTest.php index 0782435898f..50ff6424e9a 100644 --- a/tests/phpunit/integration/includes/Permissions/PermissionStatusIntegrationTest.php +++ b/tests/phpunit/integration/includes/Permissions/PermissionStatusIntegrationTest.php @@ -23,6 +23,7 @@ namespace MediaWiki\Tests\Integration\Permissions; use MediaWiki\Block\Block; use MediaWiki\Block\BlockErrorFormatter; use MediaWiki\CommentStore\CommentStoreComment; +use MediaWiki\Language\FormatterFactory; use MediaWiki\Language\RawMessage; use MediaWiki\Permissions\PermissionStatus; use MediaWiki\User\UserIdentityValue; @@ -101,7 +102,10 @@ class PermissionStatusIntegrationTest extends MediaWikiIntegrationTestCase { $blockErrorFormatter = $this->createNoOpMock( BlockErrorFormatter::class, [ 'getMessages' ] ); $blockErrorFormatter->method( 'getMessages' )->willReturn( [ new RawMessage( 'testing' ) ] ); - $this->setService( 'BlockErrorFormatter', $blockErrorFormatter ); + $formatterFactory = $this->createNoOpMock( FormatterFactory::class, [ 'getBlockErrorFormatter' ] ); + $formatterFactory->method( 'getBlockErrorFormatter' )->willReturn( $blockErrorFormatter ); + + $this->setService( 'FormatterFactory', $formatterFactory ); $status = $this->makeBlockedStatus(); $block = $status->getBlock(); diff --git a/tests/phpunit/unit/includes/language/FormatterFactoryTest.php b/tests/phpunit/unit/includes/language/FormatterFactoryTest.php new file mode 100644 index 00000000000..2a03ec5717e --- /dev/null +++ b/tests/phpunit/unit/includes/language/FormatterFactoryTest.php @@ -0,0 +1,45 @@ +createNoOpMock( MessageCache::class ), + $this->createNoOpMock( TitleFormatter::class ), + $this->createNoOpMock( HookContainer::class ), + $this->createNoOpMock( UserIdentityUtils::class ), + $this->createNoOpMock( LanguageFactory::class ) + ); + } + + public function testGetStatusFormatter() { + $factory = $this->getFactory(); + $factory->getStatusFormatter( + $this->createNoOpMock( MessageLocalizer::class ) + ); + + // Just make sure the getter works. + // This protects against constructor signature changes. + $this->addToAssertionCount( 1 ); + } + + public function testGetBlockErrorFormatter() { + $factory = $this->getFactory(); + $factory->getBlockErrorFormatter( + $this->createNoOpMock( IContextSource::class ) + ); + + // Just make sure the getter works. + // This protects against constructor signature changes. + $this->addToAssertionCount( 1 ); + } +}