Avoid try…catch where if…else will do when using LanguageFactory

Needed for I869af06896b9757af18488b916211c5a41a8c563, where I am
trying to change LanguageFactory in MediaWiki core not to use
MWException.

I truly feel mocked after working with this code. See if you can find
the two lines in this diff where the meaningful changes are.

Change-Id: Ifcb31dbb7113ce57526f06558cde2abedee317d7
This commit is contained in:
Bartosz Dziewoński 2022-11-18 20:26:02 +01:00
parent 4fde31757c
commit 1b7a46ff4f
7 changed files with 54 additions and 22 deletions

View file

@ -46,6 +46,7 @@ return [
new ServiceOptions( MathWikibaseConnector::CONSTRUCTOR_OPTIONS, $services->getMainConfig() ), new ServiceOptions( MathWikibaseConnector::CONSTRUCTOR_OPTIONS, $services->getMainConfig() ),
WikibaseClient::getRepoLinker( $services ), WikibaseClient::getRepoLinker( $services ),
$services->getLanguageFactory(), $services->getLanguageFactory(),
$services->getLanguageNameUtils(),
WikibaseClient::getEntityRevisionLookup( $services ), WikibaseClient::getEntityRevisionLookup( $services ),
WikibaseClient::getFallbackLabelDescriptionLookupFactory( $services ), WikibaseClient::getFallbackLabelDescriptionLookupFactory( $services ),
WikibaseClient::getSite( $services ), WikibaseClient::getSite( $services ),

View file

@ -360,6 +360,7 @@
"services": [ "services": [
"Math.WikibaseConnector", "Math.WikibaseConnector",
"LanguageFactory", "LanguageFactory",
"LanguageNameUtils",
"TitleFactory" "TitleFactory"
] ]
} }

View file

@ -6,7 +6,7 @@ use DataValues\StringValue;
use InvalidArgumentException; use InvalidArgumentException;
use MediaWiki\Config\ServiceOptions; use MediaWiki\Config\ServiceOptions;
use MediaWiki\Languages\LanguageFactory; use MediaWiki\Languages\LanguageFactory;
use MWException; use MediaWiki\Languages\LanguageNameUtils;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Site; use Site;
use Wikibase\Client\RepoLinker; use Wikibase\Client\RepoLinker;
@ -51,6 +51,9 @@ class MathWikibaseConnector {
/** @var LanguageFactory */ /** @var LanguageFactory */
private $languageFactory; private $languageFactory;
/** @var LanguageNameUtils */
private $languageNameUtils;
/** @var EntityRevisionLookup */ /** @var EntityRevisionLookup */
private $entityRevisionLookup; private $entityRevisionLookup;
@ -85,6 +88,7 @@ class MathWikibaseConnector {
* @param ServiceOptions $options * @param ServiceOptions $options
* @param RepoLinker $repoLinker * @param RepoLinker $repoLinker
* @param LanguageFactory $languageFactory * @param LanguageFactory $languageFactory
* @param LanguageNameUtils $languageNameUtils
* @param EntityRevisionLookup $entityRevisionLookup * @param EntityRevisionLookup $entityRevisionLookup
* @param FallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory * @param FallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory
* @param Site $site * @param Site $site
@ -96,6 +100,7 @@ class MathWikibaseConnector {
ServiceOptions $options, ServiceOptions $options,
RepoLinker $repoLinker, RepoLinker $repoLinker,
LanguageFactory $languageFactory, LanguageFactory $languageFactory,
LanguageNameUtils $languageNameUtils,
EntityRevisionLookup $entityRevisionLookup, EntityRevisionLookup $entityRevisionLookup,
FallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory, FallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory,
Site $site, Site $site,
@ -106,6 +111,7 @@ class MathWikibaseConnector {
$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS ); $options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
$this->repoLinker = $repoLinker; $this->repoLinker = $repoLinker;
$this->languageFactory = $languageFactory; $this->languageFactory = $languageFactory;
$this->languageNameUtils = $languageNameUtils;
$this->entityRevisionLookup = $entityRevisionLookup; $this->entityRevisionLookup = $entityRevisionLookup;
$this->labelDescriptionLookupFactory = $labelDescriptionLookupFactory; $this->labelDescriptionLookupFactory = $labelDescriptionLookupFactory;
$this->site = $site; $this->site = $site;
@ -167,9 +173,9 @@ class MathWikibaseConnector {
* id does not exist * id does not exist
*/ */
public function fetchWikibaseFromId( string $qid, string $langCode ): MathWikibaseInfo { public function fetchWikibaseFromId( string $qid, string $langCode ): MathWikibaseInfo {
try { if ( $this->languageNameUtils->isValidCode( $langCode ) ) {
$lang = $this->languageFactory->getLanguage( $langCode ); $lang = $this->languageFactory->getLanguage( $langCode );
} catch ( MWException $e ) { } else {
throw new InvalidArgumentException( "Invalid language code specified." ); throw new InvalidArgumentException( "Invalid language code specified." );
} }

View file

@ -6,9 +6,9 @@ use Html;
use MediaWiki\Extension\Math\MathWikibaseConnector; use MediaWiki\Extension\Math\MathWikibaseConnector;
use MediaWiki\Extension\Math\MathWikibaseInfo; use MediaWiki\Extension\Math\MathWikibaseInfo;
use MediaWiki\Languages\LanguageFactory; use MediaWiki\Languages\LanguageFactory;
use MediaWiki\Languages\LanguageNameUtils;
use MediaWiki\Rest\Response; use MediaWiki\Rest\Response;
use MediaWiki\Rest\SimpleHandler; use MediaWiki\Rest\SimpleHandler;
use MWException;
use Title; use Title;
use TitleFactory; use TitleFactory;
use Wikimedia\ParamValidator\ParamValidator; use Wikimedia\ParamValidator\ParamValidator;
@ -21,21 +21,27 @@ class Popup extends SimpleHandler {
/** @var LanguageFactory */ /** @var LanguageFactory */
private $languageFactory; private $languageFactory;
/** @var LanguageNameUtils */
private $languageNameUtils;
/** @var Title|null */ /** @var Title|null */
private $specialPageTitle; private $specialPageTitle;
/** /**
* @param MathWikibaseConnector $wikibase * @param MathWikibaseConnector $wikibase
* @param LanguageFactory $languageFactory * @param LanguageFactory $languageFactory
* @param LanguageNameUtils $languageNameUtils
* @param TitleFactory $titleFactory * @param TitleFactory $titleFactory
*/ */
public function __construct( public function __construct(
MathWikibaseConnector $wikibase, MathWikibaseConnector $wikibase,
LanguageFactory $languageFactory, LanguageFactory $languageFactory,
LanguageNameUtils $languageNameUtils,
TitleFactory $titleFactory TitleFactory $titleFactory
) { ) {
$this->wikibase = $wikibase; $this->wikibase = $wikibase;
$this->languageFactory = $languageFactory; $this->languageFactory = $languageFactory;
$this->languageNameUtils = $languageNameUtils;
$this->specialPageTitle = $titleFactory->newFromText( 'Special:MathWikibase' ); $this->specialPageTitle = $titleFactory->newFromText( 'Special:MathWikibase' );
} }
@ -45,9 +51,9 @@ class Popup extends SimpleHandler {
$uselang = 'en'; $uselang = 'en';
} }
$rf = $this->getResponseFactory(); $rf = $this->getResponseFactory();
try { if ( $this->languageNameUtils->isValidCode( $uselang ) ) {
$langObj = $this->languageFactory->getLanguage( $uselang ); $langObj = $this->languageFactory->getLanguage( $uselang );
} catch ( MWException $e ) { } else {
return $rf->createHttpError( 400, [ 'message' => 'Invalid language code.' ] ); return $rf->createHttpError( 400, [ 'message' => 'Invalid language code.' ] );
} }

View file

@ -3,9 +3,8 @@
namespace MediaWiki\Extension\Math\Tests; namespace MediaWiki\Extension\Math\Tests;
use DataValues\StringValue; use DataValues\StringValue;
use MediaWiki\Languages\LanguageFactory; use MediaWiki\Languages\LanguageNameUtils;
use MediaWiki\Logger\LoggerFactory; use MediaWiki\Logger\LoggerFactory;
use MWException;
use Wikibase\DataModel\Entity\BasicEntityIdParser; use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\EntityIdParsingException; use Wikibase\DataModel\Entity\EntityIdParsingException;
use Wikibase\DataModel\Entity\Item; use Wikibase\DataModel\Entity\Item;
@ -30,10 +29,10 @@ class MathWikibaseConnectorTest extends MathWikibaseConnectorTestFactory {
} }
public function testFetchInvalidLanguage() { public function testFetchInvalidLanguage() {
$languageFactory = $this->createMock( LanguageFactory::class ); $languageNameUtils = $this->createMock( LanguageNameUtils::class );
$languageFactory->method( 'getLanguage' ) $languageNameUtils->method( 'isValidCode' )
->willThrowException( new MWException( 'Invalid code' ) ); ->willReturn( false );
$mathWikibase = $this->getWikibaseConnector( $languageFactory ); $mathWikibase = $this->getWikibaseConnector( null, $languageNameUtils );
$this->expectException( 'InvalidArgumentException' ); $this->expectException( 'InvalidArgumentException' );
$this->expectErrorMessage( 'Invalid language code specified.' ); $this->expectErrorMessage( 'Invalid language code specified.' );
@ -44,7 +43,7 @@ class MathWikibaseConnectorTest extends MathWikibaseConnectorTestFactory {
$entityRevisionLookup = $this->createMock( EntityRevisionLookup::class ); $entityRevisionLookup = $this->createMock( EntityRevisionLookup::class );
$entityRevisionLookup->method( 'getEntityRevision' ) $entityRevisionLookup->method( 'getEntityRevision' )
->willThrowException( new StorageException( 'Invalid code' ) ); ->willThrowException( new StorageException( 'Invalid code' ) );
$mathWikibase = $this->getWikibaseConnector( null, null, $entityRevisionLookup ); $mathWikibase = $this->getWikibaseConnector( null, null, null, $entityRevisionLookup );
$this->expectException( 'InvalidArgumentException' ); $this->expectException( 'InvalidArgumentException' );
$this->expectErrorMessage( 'Non-existing Wikibase ID.' ); $this->expectErrorMessage( 'Non-existing Wikibase ID.' );
@ -79,6 +78,7 @@ class MathWikibaseConnectorTest extends MathWikibaseConnectorTestFactory {
// non-existing properties should not result in errors on initialization // non-existing properties should not result in errors on initialization
$mathWikibase = $this->getWikibaseConnector( $mathWikibase = $this->getWikibaseConnector(
null,
null, null,
null, null,
$revisionLookupMock, $revisionLookupMock,
@ -104,7 +104,7 @@ class MathWikibaseConnectorTest extends MathWikibaseConnectorTestFactory {
} }
} ); } );
$mathWikibase = $this->getWikibaseConnector( null, null, null, null, $parserMock ); $mathWikibase = $this->getWikibaseConnector( null, null, null, null, null, $parserMock );
$this->expectException( 'InvalidArgumentException' ); $this->expectException( 'InvalidArgumentException' );
$this->expectErrorMessage( 'Invalid Wikibase ID.' ); $this->expectErrorMessage( 'Invalid Wikibase ID.' );
$mathWikibase->fetchWikibaseFromId( '1', 'en' ); $mathWikibase->fetchWikibaseFromId( '1', 'en' );

View file

@ -8,6 +8,7 @@ use MediaWiki\Config\ServiceOptions;
use MediaWiki\Extension\Math\MathFormatter; use MediaWiki\Extension\Math\MathFormatter;
use MediaWiki\Extension\Math\MathWikibaseConnector; use MediaWiki\Extension\Math\MathWikibaseConnector;
use MediaWiki\Languages\LanguageFactory; use MediaWiki\Languages\LanguageFactory;
use MediaWiki\Languages\LanguageNameUtils;
use MediaWikiUnitTestCase; use MediaWikiUnitTestCase;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Site; use Site;
@ -75,12 +76,17 @@ class MathWikibaseConnectorTestFactory extends MediaWikiUnitTestCase {
$languageFactoryMock->method( 'getLanguage' ) $languageFactoryMock->method( 'getLanguage' )
->with( 'en' ) ->with( 'en' )
->willReturn( $languageMock ); ->willReturn( $languageMock );
$languageNameUtilsMock = self::createMock( LanguageNameUtils::class );
$languageNameUtilsMock->method( 'isValidCode' )
->with( 'en' )
->willReturn( true );
$fallbackLabelDescriptionLookupFactoryMock->method( 'newLabelDescriptionLookup' ) $fallbackLabelDescriptionLookupFactoryMock->method( 'newLabelDescriptionLookup' )
->with( $languageMock ) ->with( $languageMock )
->willReturnCallback( [ $this, 'newLabelDescriptionLookup' ] ); ->willReturnCallback( [ $this, 'newLabelDescriptionLookup' ] );
return self::getWikibaseConnector( return self::getWikibaseConnector(
$languageFactoryMock, $languageFactoryMock,
$languageNameUtilsMock,
$fallbackLabelDescriptionLookupFactoryMock, $fallbackLabelDescriptionLookupFactoryMock,
$revisionLookupMock, $revisionLookupMock,
$logger, $logger,
@ -90,6 +96,7 @@ class MathWikibaseConnectorTestFactory extends MediaWikiUnitTestCase {
public function getWikibaseConnector( public function getWikibaseConnector(
LanguageFactory $languageFactory = null, LanguageFactory $languageFactory = null,
LanguageNameUtils $languageNameUtils = null,
FallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory = null, FallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory = null,
EntityRevisionLookup $entityRevisionLookupMock = null, EntityRevisionLookup $entityRevisionLookupMock = null,
LoggerInterface $logger = null, LoggerInterface $logger = null,
@ -102,6 +109,10 @@ class MathWikibaseConnectorTestFactory extends MediaWikiUnitTestCase {
self::createMock( EntityRevisionLookup::class ); self::createMock( EntityRevisionLookup::class );
$languageFactory = $languageFactory ?: self::createMock( LanguageFactory::class ); $languageFactory = $languageFactory ?: self::createMock( LanguageFactory::class );
if ( !$languageNameUtils ) {
$languageNameUtils = self::createMock( LanguageNameUtils::class );
$languageNameUtils->method( 'isValidCode' )->willReturn( true );
}
$site = self::createMock( Site::class ); $site = self::createMock( Site::class );
$site->method( 'getGlobalId' )->willReturn( '' ); $site->method( 'getGlobalId' )->willReturn( '' );
@ -129,6 +140,7 @@ class MathWikibaseConnectorTestFactory extends MediaWikiUnitTestCase {
] ), ] ),
$repoConnector, $repoConnector,
$languageFactory, $languageFactory,
$languageNameUtils,
$entityRevisionLookup, $entityRevisionLookup,
$labelDescriptionLookupFactory, $labelDescriptionLookupFactory,
$site, $site,

View file

@ -4,6 +4,7 @@ namespace MediaWiki\Extension\Math\Tests;
use MediaWiki\Extension\Math\Rest\Popup; use MediaWiki\Extension\Math\Rest\Popup;
use MediaWiki\Languages\LanguageFactory; use MediaWiki\Languages\LanguageFactory;
use MediaWiki\Languages\LanguageNameUtils;
use MediaWiki\Rest\Handler; use MediaWiki\Rest\Handler;
use MediaWiki\Rest\HttpException; use MediaWiki\Rest\HttpException;
use MediaWiki\Rest\RequestData; use MediaWiki\Rest\RequestData;
@ -45,13 +46,13 @@ class PopupTest extends MathWikibaseConnectorTestFactory {
} }
public function testInvalidLanguage() { public function testInvalidLanguage() {
$languageFactoryMock = $this->createMock( LanguageFactory::class ); $languageNameUtilsMock = $this->createMock( LanguageNameUtils::class );
$languageFactoryMock->expects( $this->once() ) $languageNameUtilsMock->expects( $this->once() )
->method( 'getLanguage' ) ->method( 'isValidCode' )
->with( 'tmp' ) ->with( 'tmp' )
->willThrowException( new \MWException() ); ->willReturn( false );
$popupHandler = $this->getPopup( $languageFactoryMock ); $popupHandler = $this->getPopup( null, $languageNameUtilsMock );
$response = $this->executeHandler( $popupHandler, $this->getRequest( '1', 'tmp' ) ); $response = $this->executeHandler( $popupHandler, $this->getRequest( '1', 'tmp' ) );
$this->assertEquals( 400, $response->getStatusCode() ); $this->assertEquals( 400, $response->getStatusCode() );
$data = json_decode( $response->getBody(), true ); $data = json_decode( $response->getBody(), true );
@ -62,7 +63,7 @@ class PopupTest extends MathWikibaseConnectorTestFactory {
* @dataProvider provideItemSetups * @dataProvider provideItemSetups
*/ */
public function testExistingId( Item $item ) { public function testExistingId( Item $item ) {
$popupHandler = $this->getPopup( null, $item ); $popupHandler = $this->getPopup( null, null, $item );
$request = $this->getRequest( '1', 'en' ); $request = $this->getRequest( '1', 'en' );
$data = $this->executeHandlerAndGetBodyData( $popupHandler, $request ); $data = $this->executeHandlerAndGetBodyData( $popupHandler, $request );
@ -109,12 +110,17 @@ class PopupTest extends MathWikibaseConnectorTestFactory {
private function getPopup( private function getPopup(
LanguageFactory $languageFactoryMock = null, LanguageFactory $languageFactoryMock = null,
LanguageNameUtils $languageNameUtilsMock = null,
Item $item = null Item $item = null
): Popup { ): Popup {
$languageFactoryMock = $languageFactoryMock ?: $this->createMock( LanguageFactory::class ); $languageFactoryMock = $languageFactoryMock ?: $this->createMock( LanguageFactory::class );
if ( !$languageNameUtilsMock ) {
$languageNameUtilsMock = self::createMock( LanguageNameUtils::class );
$languageNameUtilsMock->method( 'isValidCode' )->willReturn( true );
}
$mathWikibaseConnectorMock = $item ? $mathWikibaseConnectorMock = $item ?
$this->getWikibaseConnectorWithExistingItems( new EntityRevision( $item ) ) : $this->getWikibaseConnectorWithExistingItems( new EntityRevision( $item ) ) :
$this->getWikibaseConnector( $languageFactoryMock ); $this->getWikibaseConnector( $languageFactoryMock, $languageNameUtilsMock );
$titleMock = $this->createMock( Title::class ); $titleMock = $this->createMock( Title::class );
$titleMock->method( 'getLocalURL' )->willReturn( 'special/Q1' ); $titleMock->method( 'getLocalURL' )->willReturn( 'special/Q1' );
@ -124,7 +130,7 @@ class PopupTest extends MathWikibaseConnectorTestFactory {
->method( 'newFromText' ) ->method( 'newFromText' )
->willReturn( $titleMock ); ->willReturn( $titleMock );
return new Popup( $mathWikibaseConnectorMock, $languageFactoryMock, $titleFactoryMock ); return new Popup( $mathWikibaseConnectorMock, $languageFactoryMock, $languageNameUtilsMock, $titleFactoryMock );
} }
public function provideItemSetups(): array { public function provideItemSetups(): array {