jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/356481 )
Change subject: When page properties are not available do not throw exceptions
......................................................................
When page properties are not available do not throw exceptions
There is an edge case where a page does not have any page properties
and an api request results in a notice, warning and exception.
To support testing and readability I've broken out the offending code
and added tests and fixed the edge case.
Bug: T161026
Bug: T166530
Change-Id: I52e991f5bd97edd09db837257673e73077a981ad
---
M includes/api/ApiMobileView.php
M tests/phpunit/api/ApiMobileViewTest.php
2 files changed, 87 insertions(+), 9 deletions(-)
Approvals:
Pmiazga: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/api/ApiMobileView.php b/includes/api/ApiMobileView.php
index 5897db7..d1beb22 100644
--- a/includes/api/ApiMobileView.php
+++ b/includes/api/ApiMobileView.php
@@ -43,6 +43,27 @@
}
/**
+ * Obtain the requested page properties.
+ * @param string $propNames requested list of pageprops separated by
'|'. If '*'
+ * all page props will be returned.
+ * @param array $data data available as returned by getData
+ * @return Array associative
+ */
+ public function getMobileViewPageProps( $propNames, $data ) {
+ if ( array_key_exists( 'pageprops', $data ) ) {
+ if ( $propNames == '*' ) {
+ $pageProps = $data['pageprops'];
+ } else {
+ $pageProps = array_intersect_key(
$data['pageprops'],
+ array_flip( explode( '|', $propNames )
) );
+ }
+ } else {
+ $pageProps = [];
+ }
+ return $pageProps;
+ }
+
+ /**
* Execute the requested Api actions.
* @todo: Write some unit tests for API results
*/
@@ -122,18 +143,13 @@
$this->addXAnalyticsItem( 'page_id',
(string)$data['id'] );
}
if ( isset( $prop['pageprops'] ) ) {
- $propNames = $params['pageprops'];
- if ( $propNames == '*' && isset( $data['pageprops'] ) )
{
- $pageProps = $data['pageprops'];
- } else {
- $propNames = explode( '|', $propNames );
- $pageProps = array_intersect_key(
$data['pageprops'], array_flip( $propNames ) );
- }
- ApiResult::setArrayType( $pageProps, 'assoc' );
+ $mvPageProps = $this->getMobileViewPageProps(
$params['pageprops'], $data );
+ ApiResult::setArrayType( $mvPageProps, 'assoc' );
$resultObj->addValue( null, $moduleName,
- [ 'pageprops' => $pageProps ]
+ [ 'pageprops' => $mvPageProps ]
);
}
+
if ( isset( $prop['description'] ) && isset(
$data['pageprops']['wikibase_item'] ) ) {
$desc = ExtMobileFrontend::getWikibaseDescription(
$data['pageprops']['wikibase_item']
diff --git a/tests/phpunit/api/ApiMobileViewTest.php
b/tests/phpunit/api/ApiMobileViewTest.php
index 45e996f..1d43f3c 100644
--- a/tests/phpunit/api/ApiMobileViewTest.php
+++ b/tests/phpunit/api/ApiMobileViewTest.php
@@ -544,6 +544,68 @@
$this->assertFalse( $isSVG->invokeArgs( $api, [ null ] ) );
}
+ public function provideGetMobileViewPageProps() {
+ return [
+ // Request all available page properties
+ [
+ '*',
+ [
+ 'pageprops' => [ 'wikibase_item' =>
'Q76', 'notoc' => true ],
+ ],
+ [ 'wikibase_item' => 'Q76', 'notoc' => true ],
+ ],
+ // Request non-existent property
+ [
+ 'monkey',
+ [
+ 'pageprops' => [ 'wikibase_item' =>
'Q76', 'notoc' => true ],
+ ],
+ [],
+ ],
+ // Filter out available page properties with '|'
+ [
+ 'wikibase_item|notoc',
+ [
+ 'pageprops' => [ 'wikibase_item' =>
'Q76', 'notoc' => true ],
+ ],
+ [ 'wikibase_item' => 'Q76', 'notoc' => true ],
+ ],
+ // Filter out available page properties without '|'
+ [
+ 'wikibase_item',
+ [
+ 'pageprops' => [ 'wikibase_item' =>
'Q76', 'notoc' => true ],
+ ],
+ [ 'wikibase_item' => 'Q76' ],
+ ],
+ // When no page properties available (T161026)
+ [
+ 'wikibase_item',
+ [
+ 'title' => 'Foo'
+ ],
+ [],
+ ],
+ // Request all from nothing
+ [
+ '*',
+ [],
+ [],
+ ]
+ ];
+ }
+ /**
+ * @dataProvider provideGetMobileViewPageProps
+ * @covers ApiMobileView::getMobileViewPageProps
+ */
+ public function testGetMobileViewPageProps( $requested, $available,
$returned ) {
+ $context = new RequestContext();
+ $api = new ApiMobileView( new ApiMain( $context ), 'mobileview'
);
+ $actual = $api->getMobileViewPageProps( $requested, $available
);
+
+ $this->assertEquals( $returned, $actual );
+ }
+
private static function getNonPublicMethod( $className, $methodName ) {
$reflectionClass = new ReflectionClass( $className );
$method = $reflectionClass->getMethod( $methodName );
--
To view, visit https://gerrit.wikimedia.org/r/356481
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I52e991f5bd97edd09db837257673e73077a981ad
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Pmiazga <pmia...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
