Pmiazga has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/372473 )
Change subject: Verify the existance of `url` key when parsing lang objects
......................................................................
Verify the existance of `url` key when parsing lang objects
Some langLink retrieved from API do not contain url property, only
code and title. If that happens do not throw PHP notice error. Log
the error and skip broken lang object.
Changes:
- extracted langObject validation to a separate function
- added url property existence check
- renamed $code to $index as $langLinks is an array without keys
- introduced MobileContext::LOGGER_CHANNEL as it's used in couple
places
Bug: T172316
Change-Id: I4ef5b1ad4a37b96407f7f758680a6177cafdf128
---
M includes/MobileContext.php
M includes/specials/SpecialMobileLanguages.php
M tests/phpunit/specials/SpecialMobileLanguagesTest.php
3 files changed, 67 insertions(+), 6 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend
refs/changes/73/372473/1
diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index fbadd60..a04e2ca 100644
--- a/includes/MobileContext.php
+++ b/includes/MobileContext.php
@@ -21,7 +21,7 @@
const LAZY_LOAD_IMAGES_COOKIE_VALUE = 'A';
const LAZY_LOAD_REFERENCES_COOKIE_NAME = 'mfLazyLoadReferences';
const LAZY_LOAD_REFERENCES_COOKIE_VALUE = 'A';
-
+ const LOGGER_CHANNEL = 'mobile';
/**
* Saves the testing mode user has opted in: 'beta' or 'stable'
* @var string $mobileMode
diff --git a/includes/specials/SpecialMobileLanguages.php
b/includes/specials/SpecialMobileLanguages.php
index 91e9783..735645e 100644
--- a/includes/specials/SpecialMobileLanguages.php
+++ b/includes/specials/SpecialMobileLanguages.php
@@ -58,15 +58,14 @@
// Set the name of each language based on the system
list of language names
$languageMap = Language::fetchLanguageNames();
$languages = $page['langlinks'];
- foreach ( $page['langlinks'] as $code => $langObject ) {
- if ( !isset( $languageMap[$langObject['lang']]
) ) {
- // Bug T93500: DB might still have
preantiquated rows with bogus languages
- unset( $languages[$code] );
+ foreach ( $page['langlinks'] as $index => $langObject )
{
+ if ( !$this->isLanguageObjectValid(
$languageMap, $langObject ) ) {
+ unset( $languages[$index] );
continue;
}
$langObject['langname'] =
$languageMap[$langObject['lang']];
$langObject['url'] =
MobileContext::singleton()->getMobileUrl( $langObject['url'] );
- $languages[$code] = $langObject;
+ $languages[$index] = $langObject;
}
$compareLanguage = function ( $a, $b ) {
return strcasecmp( $a['langname'],
$b['langname'] );
@@ -80,6 +79,33 @@
}
/**
+ * Verify if passed language object contains all necessary information
+ *
+ * @see https://phabricator.wikimedia.org/T93500
+ * @see https://phabricator.wikimedia.org/T172316
+ * @param array $languageMap array of language names, indexed by code.
+ * @param array $langObject array of lang objects
+ * @return bool
+ */
+ private function isLanguageObjectValid( $languageMap, $langObject ) {
+ if ( !isset( $languageMap[$langObject['lang']] ) ) {
+ // Bug T93500: DB might still have preantiquated rows
with bogus languages
+ return false;
+ }
+ if ( !array_key_exists( 'url', $langObject ) ) {
+ // Bug T172316: Some lang objects do not have url. We
would like to log those instances
+ \MediaWiki\Logger\LoggerFactory::getInstance(
MobileContext::LOGGER_CHANNEL )->warning(
+ '`url` key is undefined in language object',
+ [
+ 'uri' =>
RequestContext::getMain()->getRequest()->getFullRequestURL(),
+ 'langObject' => $langObject,
+ ]
+ );
+ return false;
+ }
+ return true;
+ }
+ /**
* Returns an array of language variants that the page is available in
* @return array
*/
diff --git a/tests/phpunit/specials/SpecialMobileLanguagesTest.php
b/tests/phpunit/specials/SpecialMobileLanguagesTest.php
index 385bb48..1f8d27c 100644
--- a/tests/phpunit/specials/SpecialMobileLanguagesTest.php
+++ b/tests/phpunit/specials/SpecialMobileLanguagesTest.php
@@ -115,6 +115,7 @@
/**
* @dataProvider providerProcessLanguages
* @covers SpecialMobileLanguages::processLanguages
+ * @covers SpecialMobileLanguages::isLanguageObjectValid
*/
public function testProcessLanguages( $langlinks, $expected ) {
$apiResult = [
@@ -132,4 +133,38 @@
'Property langname should be added, URL should be
transformed, and languages should be sorted.'
);
}
+
+ /**
+ * Test an edge case when URL key is missing in langObject
+ *
+ * @covers SpecialMobileLanguages::processLanguages
+ * @covers SpecialMobileLanguages::isLanguageObjectValid
+ */
+ public function testProcessLanguagesWhenURLKeyIsMissing() {
+ $testUri = 'http://localhost/test';
+ $langObject = [
+ 'lang' => 'pl',
+ 'title' => 'Polski'
+ ];
+ $expected = [];
+
+ $loggerMock = $this->getMock( \Psr\Log\LoggerInterface::class );
+ $loggerMock->expects( $this->once() )
+ ->method( 'warning' )
+ ->with( $this->isType( 'string' ), $this->equalTo(
+ [
+ 'uri' => $testUri,
+ 'langObject' => $langObject
+ ]
+ ) );
+ $this->setLogger( MobileContext::LOGGER_CHANNEL, $loggerMock );
+
+ $requestMock = $this->getMock( FauxRequest::class );
+ $requestMock->expects( $this->once() )
+ ->method( 'getFullRequestURL' )
+ ->willReturn( $testUri );
+
+ RequestContext::getMain()->setRequest( $requestMock );
+ $this->testProcessLanguages( [ $langObject ], $expected );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/372473
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ef5b1ad4a37b96407f7f758680a6177cafdf128
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Pmiazga <pmia...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
