[MediaWiki-commits] [Gerrit] mediawiki...Wikibase[master]: Trim earlier in SpecialGoToLinkedPage and test it

2017-07-19 Thread jenkins-bot (Code Review)
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

2017-07-17 Thread WMDE
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'] = [