From a9bc9f8a04a6e6e2d8144d72debcd0bfdb07e8a8 Mon Sep 17 00:00:00 2001 From: Umherirrender Date: Sat, 23 Oct 2021 23:33:01 +0200 Subject: [PATCH] Use ObjectFactory in LanguageConverterFactory Use the object factory to inject service into TrivialLanguageConverter class. The other language converter classes does not use service directly. The classes are using indirectly services in some function of the parent class, which needs injection as well (tracked with T294185). Needs fallback to global state in TrivialLanguageConverter, because the class is used in extensions integration tests Change-Id: If72d054d062a4f357e12c5b168e118bfafffd626 --- includes/ServiceWiring.php | 1 + .../language/LanguageConverterFactory.php | 95 ++++++++++++++----- .../language/TrivialLanguageConverter.php | 8 +- .../LanguageConverterFactoryTest.php | 45 ++++++--- .../LanguageConverterIntegrationTest.php | 12 ++- .../languages/LanguageConverterTestTrait.php | 12 ++- 6 files changed, 128 insertions(+), 45 deletions(-) diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 023d708e642..7abc2adc722 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -747,6 +747,7 @@ return [ $isConversionDisabled = $services->getMainConfig()->get( 'DisableLangConversion' ); $isTitleConversionDisabled = $services->getMainConfig()->get( 'DisableTitleConversion' ); return new LanguageConverterFactory( + $services->getObjectFactory(), $usePigLatinVariant, $isConversionDisabled, $isTitleConversionDisabled, diff --git a/includes/language/LanguageConverterFactory.php b/includes/language/LanguageConverterFactory.php index cede0015e6c..c00edd023f7 100644 --- a/includes/language/LanguageConverterFactory.php +++ b/includes/language/LanguageConverterFactory.php @@ -35,6 +35,7 @@ use TgConverter; use TlyConverter; use TrivialLanguageConverter; use UzConverter; +use Wikimedia\ObjectFactory; use ZhConverter; /** @@ -49,22 +50,58 @@ class LanguageConverterFactory { /** * @var array */ - private $converterClasses = [ - 'ban' => BanConverter::class, - 'crh' => CrhConverter::class, - 'gan' => GanConverter::class, - 'iu' => IuConverter::class, - 'kk' => KkConverter::class, - 'ku' => KuConverter::class, - 'shi' => ShiConverter::class, - 'sr' => SrConverter::class, - 'tg' => TgConverter::class, - 'tly' => TlyConverter::class, - 'uz' => UzConverter::class, - 'zh' => ZhConverter::class, + private $converterList = [ + 'ban' => [ + 'class' => BanConverter::class, + ], + 'crh' => [ + 'class' => CrhConverter::class, + ], + 'gan' => [ + 'class' => GanConverter::class, + ], + 'iu' => [ + 'class' => IuConverter::class, + ], + 'kk' => [ + 'class' => KkConverter::class, + ], + 'ku' => [ + 'class' => KuConverter::class, + ], + 'shi' => [ + 'class' => ShiConverter::class, + ], + 'sr' => [ + 'class' => SrConverter::class, + ], + 'tg' => [ + 'class' => TgConverter::class, + ], + 'tly' => [ + 'class' => TlyConverter::class, + ], + 'uz' => [ + 'class' => UzConverter::class, + ], + 'zh' => [ + 'class' => ZhConverter::class, + ], ]; - private $defaultConverterClass = TrivialLanguageConverter::class; + private const DEFAULT_CONVERTER = [ + 'class' => TrivialLanguageConverter::class, + 'services' => [ + 'TitleFormatter', + ] + ]; + + private const EN_CONVERTER = [ + 'class' => EnConverter::class, + ]; + + /** @var ObjectFactory */ + private $objectFactory; /** * @var bool Whether to disable language variant conversion. @@ -82,6 +119,7 @@ class LanguageConverterFactory { private $defaultLanguage; /** + * @param ObjectFactory $objectFactory * @param bool $usePigLatinVariant should pig variant of English be used * @param bool $isConversionDisabled Whether to disable language variant conversion * @param bool $isTitleConversionDisabled Whether to disable language variant conversion for links @@ -91,11 +129,13 @@ class LanguageConverterFactory { * @internal Should be called from MediaWikiServices only. */ public function __construct( + ObjectFactory $objectFactory, $usePigLatinVariant, $isConversionDisabled, $isTitleConversionDisabled, callable $defaultLanguage ) { + $this->objectFactory = $objectFactory; if ( $usePigLatinVariant ) { - $this->converterClasses['en'] = EnConverter::class; + $this->converterList['en'] = self::EN_CONVERTER; } $this->isConversionDisabled = $isConversionDisabled; $this->isTitleConversionDisabled = $isTitleConversionDisabled; @@ -103,14 +143,23 @@ class LanguageConverterFactory { } /** - * Returns Converter's class name for given language code + * Returns Converter instance for given language object * - * @param string $code code for which class name should be provided - * @return string + * @param Language $lang + * @return ILanguageConverter */ - private function classFromCode( string $code ): string { - $code = mb_strtolower( $code ); - return $this->converterClasses[$code] ?? $this->defaultConverterClass; + private function instantiateConverter( Language $lang ): ILanguageConverter { + $code = mb_strtolower( $lang->getCode() ); + $spec = $this->converterList[$code] ?? self::DEFAULT_CONVERTER; + // ObjectFactory::createObject accepts an array, not just a callable (phan bug) + // @phan-suppress-next-line PhanTypeInvalidCallableArrayKey,PhanTypeInvalidCallableArraySize + return $this->objectFactory->createObject( + $spec, + [ + 'assertClass' => ILanguageConverter::class, + 'extraArgs' => [ $lang ], + ] + ); } /** @@ -127,9 +176,7 @@ class LanguageConverterFactory { if ( isset( $this->cache[$lang->getCode()] ) ) { return $this->cache[$lang->getCode()]; } - $class = $this->classFromCode( $lang->getCode() ); - - $converter = new $class( $lang ); + $converter = $this->instantiateConverter( $lang ); $this->cache[$lang->getCode()] = $converter; return $converter; } diff --git a/includes/language/TrivialLanguageConverter.php b/includes/language/TrivialLanguageConverter.php index 72426bda226..0c45fe6f17c 100644 --- a/includes/language/TrivialLanguageConverter.php +++ b/includes/language/TrivialLanguageConverter.php @@ -51,12 +51,16 @@ class TrivialLanguageConverter implements ILanguageConverter { * and should be called for LanguageConverterFactory only * * @param Language $langobj + * @param TitleFormatter|null $titleFormatter * * @internal */ - public function __construct( $langobj ) { + public function __construct( + Language $langobj, + TitleFormatter $titleFormatter = null + ) { $this->language = $langobj; - $this->titleFormatter = MediaWikiServices::getInstance()->getTitleFormatter(); + $this->titleFormatter = $titleFormatter ?? MediaWikiServices::getInstance()->getTitleFormatter(); } public function autoConvert( $text, $variant = false ) { diff --git a/tests/phpunit/languages/LanguageConverterFactoryTest.php b/tests/phpunit/languages/LanguageConverterFactoryTest.php index acc76f83ab0..70f3904a058 100644 --- a/tests/phpunit/languages/LanguageConverterFactoryTest.php +++ b/tests/phpunit/languages/LanguageConverterFactoryTest.php @@ -12,7 +12,7 @@ use Wikimedia\TestingAccessWrapper; class LanguageConverterFactoryTest extends MediaWikiLangTestCase { /** * @covers ::__construct - * @covers ::classFromCode + * @covers ::instantiateConverter * @covers ::getLanguageConverter * @dataProvider codeProvider */ @@ -26,9 +26,15 @@ class LanguageConverterFactoryTest extends MediaWikiLangTestCase { $manualLevel ) { $lang = MediaWikiServices::getInstance()->getLanguageFactory()->getLanguage( $code ); - $factory = new LanguageConverterFactory( false, false, false, static function () use ( $lang ) { - return $lang; - } ); + $factory = new LanguageConverterFactory( + MediaWikiServices::getInstance()->getObjectFactory(), + false, + false, + false, + static function () use ( $lang ) { + return $lang; + } + ); $this->assertFalse( $factory->isConversionDisabled() ); $this->assertFalse( $factory->isTitleConversionDisabled() ); $this->assertFalse( $factory->isLinkConversionDisabled() ); @@ -48,14 +54,20 @@ class LanguageConverterFactoryTest extends MediaWikiLangTestCase { /** * @covers ::__construct - * @covers ::classFromCode + * @covers ::instantiateConverter * @covers ::getLanguageConverter */ public function testCreateFromCodeEnPigLatin() { $lang = MediaWikiServices::getInstance()->getLanguageFactory()->getLanguage( 'en' ); - $factory = new LanguageConverterFactory( true, false, false, static function () use ( $lang ) { - return $lang; - } ); + $factory = new LanguageConverterFactory( + MediaWikiServices::getInstance()->getObjectFactory(), + true, + false, + false, + static function () use ( $lang ) { + return $lang; + } + ); $this->assertFalse( $factory->isConversionDisabled() ); $this->assertFalse( $factory->isTitleConversionDisabled() ); $this->assertFalse( $factory->isLinkConversionDisabled() ); @@ -77,13 +89,14 @@ class LanguageConverterFactoryTest extends MediaWikiLangTestCase { /** * @covers ::__construct - * @covers ::classFromCode + * @covers ::instantiateConverter * @covers ::getLanguageConverter * @dataProvider booleanProvider */ public function testDisabledBooleans( $pigLatinDisabled, $conversionDisabled, $titleDisabled ) { $lang = MediaWikiServices::getInstance()->getLanguageFactory()->getLanguage( 'en' ); $factory = new LanguageConverterFactory( + MediaWikiServices::getInstance()->getObjectFactory(), !$pigLatinDisabled, $conversionDisabled, $titleDisabled, @@ -123,14 +136,20 @@ class LanguageConverterFactoryTest extends MediaWikiLangTestCase { /** * @covers ::__construct - * @covers ::classFromCode + * @covers ::instantiateConverter * @covers ::getLanguageConverter */ public function testDefaultContentLanguageFallback() { $lang = MediaWikiServices::getInstance()->getLanguageFactory()->getLanguage( 'en' ); - $factory = new LanguageConverterFactory( false, false, false, static function () use ( $lang ) { - return $lang; - } ); + $factory = new LanguageConverterFactory( + MediaWikiServices::getInstance()->getObjectFactory(), + false, + false, + false, + static function () use ( $lang ) { + return $lang; + } + ); $this->assertFalse( $factory->isConversionDisabled() ); $this->assertFalse( $factory->isTitleConversionDisabled() ); $this->assertFalse( $factory->isLinkConversionDisabled() ); diff --git a/tests/phpunit/languages/LanguageConverterIntegrationTest.php b/tests/phpunit/languages/LanguageConverterIntegrationTest.php index b61e56db4b1..dc4313cd7cd 100644 --- a/tests/phpunit/languages/LanguageConverterIntegrationTest.php +++ b/tests/phpunit/languages/LanguageConverterIntegrationTest.php @@ -13,9 +13,15 @@ class LanguageConverterIntegrationTest extends MediaWikiIntegrationTestCase { protected function setUp(): void { parent::setUp(); - $this->factory = new LanguageConverterFactory( false, false, false, static function () { - $language = MediaWikiServices::getInstance()->getContentLanguage(); - } ); + $this->factory = new LanguageConverterFactory( + MediaWikiServices::getInstance()->getObjectFactory(), + false, + false, + false, + static function () { + return MediaWikiServices::getInstance()->getContentLanguage(); + } + ); } /** diff --git a/tests/phpunit/languages/LanguageConverterTestTrait.php b/tests/phpunit/languages/LanguageConverterTestTrait.php index d425685633f..06435090f0b 100644 --- a/tests/phpunit/languages/LanguageConverterTestTrait.php +++ b/tests/phpunit/languages/LanguageConverterTestTrait.php @@ -24,9 +24,15 @@ trait LanguageConverterTestTrait { $language = MediaWikiServices::getInstance()->getLanguageFactory() ->getLanguage( $code ); - $factory = new LanguageConverterFactory( false, false, false, static function () use ( $language ) { - return $language; - } ); + $factory = new LanguageConverterFactory( + MediaWikiServices::getInstance()->getObjectFactory(), + false, + false, + false, + static function () use ( $language ) { + return $language; + } + ); return $factory->getLanguageConverter( $language ); }