[MediaWiki-commits] [Gerrit] mediawiki...Wikibase[master]: Trim earlier in SpecialGoToLinkedPage and test it
jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/365562 ) Change subject: Trim earlier in SpecialGoToLinkedPage and test it .. Trim earlier in SpecialGoToLinkedPage and test it The trimming of the individual values from the sites array was always done, but very late. Now it's all done as early as possible. I also replaced an other trim() with the one from the stringNormalizer. Plus I changed the test and added a space after a comma to actually test this. Bug: T166473 Change-Id: I70d9e1e35e2c04988af8eb32c3ec2975eddc3662 --- M lib/tests/phpunit/Store/EntityInfoBuilderTest.php M repo/includes/Specials/SpecialGoToLinkedPage.php M repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php 3 files changed, 17 insertions(+), 17 deletions(-) Approvals: Aleksey Bekh-Ivanov (WMDE): Looks good to me, approved jenkins-bot: Verified diff --git a/lib/tests/phpunit/Store/EntityInfoBuilderTest.php b/lib/tests/phpunit/Store/EntityInfoBuilderTest.php index 252050e..e245e90 100644 --- a/lib/tests/phpunit/Store/EntityInfoBuilderTest.php +++ b/lib/tests/phpunit/Store/EntityInfoBuilderTest.php @@ -313,7 +313,7 @@ ], 'Q7' => [ 'id' => 'Q2', 'type' => Item::ENTITY_TYPE, 'labels' => $this->makeLanguageValueRecords( [ 'de' => 'label:Q2/de' ] ), - ] + ], ]; $builder = $this->newEntityInfoBuilder( $ids ); diff --git a/repo/includes/Specials/SpecialGoToLinkedPage.php b/repo/includes/Specials/SpecialGoToLinkedPage.php index 7cdeb06..9078eba 100644 --- a/repo/includes/Specials/SpecialGoToLinkedPage.php +++ b/repo/includes/Specials/SpecialGoToLinkedPage.php @@ -79,34 +79,34 @@ /** * @param string|null $subPage * -* @return array array( string[] $sites, string $itemString ) +* @return array [ string[] $sites, string $itemString ] */ private function getArguments( $subPage ) { $request = $this->getRequest(); $parts = ( $subPage === '' ) ? [] : explode( '/', $subPage, 2 ); - $sites = explode( - ',', - trim( $request->getVal( 'site', isset( $parts[0] ) ? $parts[0] : '' ) ) + $sites = array_map( + [ $this->stringNormalizer, 'trimToNFC' ], + explode( ',', $request->getVal( 'site', isset( $parts[0] ) ? $parts[0] : '' ) ) ); - $itemString = trim( $request->getVal( 'itemid', isset( $parts[1] ) ? $parts[1] : 0 ) ); + $itemString = $this->stringNormalizer->trimToNFC( + $request->getVal( 'itemid', isset( $parts[1] ) ? $parts[1] : 0 ) + ); return [ $sites, $itemString ]; } /** * @param string $site -* @param string|null $itemString +* @param string $itemString * * @return string|null the URL to redirect to or null if the sitelink does not exist */ - private function getTargetUrl( $site, $itemString = null ) { + private function getTargetUrl( $site, $itemString ) { $itemId = $this->getItemId( $itemString ); if ( $site === '' || $itemId === null ) { return null; } - - $site = $this->stringNormalizer->trimToNFC( $site ); if ( !$this->siteLookup->getSite( $site ) ) { // HACK: If the site ID isn't known, add "wiki" to it; this allows the wikipedia @@ -191,7 +191,7 @@ parent::execute( $subPage ); list( $sites, $itemString ) = $this->getArguments( $subPage ); - if ( !empty( $sites ) || !empty( $itemString ) ) { + if ( $itemString !== '' ) { foreach ( $sites as $site ) { $url = $this->getTargetUrl( $site, $itemString ); if ( $url !== null ) { diff --git a/repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php b/repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php index 9553676..c48350c 100644 --- a/repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php +++ b/repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php @@ -171,12 +171,12 @@ } public function requestWithRedirectProvider() { - $cases = []; - $cases['found'] = [ 'dewiki/Q23', 'http://dewiki.com/TestPageName' ]; - $cases['foundEntityRedirect'] = [ 'dewiki/Q24', 'http://dewiki.com/TestPageName' ]; - $cases['foundWithSiteIdHack'] = [ 'de/Q23', 'http://dewiki.com/TestPageName' ]; - $cases['foundInFallbackChain'] = [
[MediaWiki-commits] [Gerrit] mediawiki...Wikibase[master]: Trim earlier in SpecialGoToLinkedPage and test it
Thiemo Mättig (WMDE) has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/365562 ) Change subject: Trim earlier in SpecialGoToLinkedPage and test it .. Trim earlier in SpecialGoToLinkedPage and test it The trimming of the individual values from the sites array was always done, but very late. Now it's all done as early as possible. I also replaced an other trim() with the one from the stringNormalizer. Plus I changed the test and added a space after a comma to actually test this. Bug: T166473 Change-Id: I70d9e1e35e2c04988af8eb32c3ec2975eddc3662 --- M lib/tests/phpunit/Store/EntityInfoBuilderTest.php M repo/includes/Specials/SpecialGoToLinkedPage.php M repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php 3 files changed, 17 insertions(+), 17 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/62/365562/1 diff --git a/lib/tests/phpunit/Store/EntityInfoBuilderTest.php b/lib/tests/phpunit/Store/EntityInfoBuilderTest.php index 252050e..e245e90 100644 --- a/lib/tests/phpunit/Store/EntityInfoBuilderTest.php +++ b/lib/tests/phpunit/Store/EntityInfoBuilderTest.php @@ -313,7 +313,7 @@ ], 'Q7' => [ 'id' => 'Q2', 'type' => Item::ENTITY_TYPE, 'labels' => $this->makeLanguageValueRecords( [ 'de' => 'label:Q2/de' ] ), - ] + ], ]; $builder = $this->newEntityInfoBuilder( $ids ); diff --git a/repo/includes/Specials/SpecialGoToLinkedPage.php b/repo/includes/Specials/SpecialGoToLinkedPage.php index 7cdeb06..9078eba 100644 --- a/repo/includes/Specials/SpecialGoToLinkedPage.php +++ b/repo/includes/Specials/SpecialGoToLinkedPage.php @@ -79,34 +79,34 @@ /** * @param string|null $subPage * -* @return array array( string[] $sites, string $itemString ) +* @return array [ string[] $sites, string $itemString ] */ private function getArguments( $subPage ) { $request = $this->getRequest(); $parts = ( $subPage === '' ) ? [] : explode( '/', $subPage, 2 ); - $sites = explode( - ',', - trim( $request->getVal( 'site', isset( $parts[0] ) ? $parts[0] : '' ) ) + $sites = array_map( + [ $this->stringNormalizer, 'trimToNFC' ], + explode( ',', $request->getVal( 'site', isset( $parts[0] ) ? $parts[0] : '' ) ) ); - $itemString = trim( $request->getVal( 'itemid', isset( $parts[1] ) ? $parts[1] : 0 ) ); + $itemString = $this->stringNormalizer->trimToNFC( + $request->getVal( 'itemid', isset( $parts[1] ) ? $parts[1] : 0 ) + ); return [ $sites, $itemString ]; } /** * @param string $site -* @param string|null $itemString +* @param string $itemString * * @return string|null the URL to redirect to or null if the sitelink does not exist */ - private function getTargetUrl( $site, $itemString = null ) { + private function getTargetUrl( $site, $itemString ) { $itemId = $this->getItemId( $itemString ); if ( $site === '' || $itemId === null ) { return null; } - - $site = $this->stringNormalizer->trimToNFC( $site ); if ( !$this->siteLookup->getSite( $site ) ) { // HACK: If the site ID isn't known, add "wiki" to it; this allows the wikipedia @@ -191,7 +191,7 @@ parent::execute( $subPage ); list( $sites, $itemString ) = $this->getArguments( $subPage ); - if ( !empty( $sites ) || !empty( $itemString ) ) { + if ( $itemString !== '' ) { foreach ( $sites as $site ) { $url = $this->getTargetUrl( $site, $itemString ); if ( $url !== null ) { diff --git a/repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php b/repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php index 9553676..c48350c 100644 --- a/repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php +++ b/repo/tests/phpunit/includes/Specials/SpecialGoToLinkedPageTest.php @@ -171,12 +171,12 @@ } public function requestWithRedirectProvider() { - $cases = []; - $cases['found'] = [ 'dewiki/Q23', 'http://dewiki.com/TestPageName' ]; - $cases['foundEntityRedirect'] = [ 'dewiki/Q24', 'http://dewiki.com/TestPageName' ]; - $cases['foundWithSiteIdHack'] = [ 'de/Q23', 'http://dewiki.com/TestPageName' ]; - $cases['foundInFallbackChain'] = [