[MediaWiki-commits] [Gerrit] Wikibase parsing of diff=... parameter was different from core - change (mediawiki...Wikibase)
jenkins-bot has submitted this change and it was merged.
Change subject: Wikibase parsing of diff=... parameter was different from core
..
Wikibase parsing of diff=... parameter was different from core
Core logik is basically:
* Check if the diff parameter does exists.
* If yes, check if it is equal to 'prev'.
* If it's equal to 'next'.
* Convert everything else to a number.
* If that conversion returns 0 compare to the current revision.
* Else try to use the number to find a revision.
* That may fail (e.g. for diff=-1).
In other words: In core every non-numeric value including 'cur',
'curr', 'c', '0' and even the empty string and null
(...?diffoldid=123 will work) does exactly the same as 'cur'.
Bug: 61749
Change-Id: Id7aab6ec65a23128fffe4c9e366213603a7e59c6
---
M repo/includes/ContentRetriever.php
M repo/includes/actions/ViewEntityAction.php
M repo/tests/phpunit/includes/ContentRetrieverTest.php
3 files changed, 158 insertions(+), 69 deletions(-)
Approvals:
Tobias Gritschacher: Looks good to me, approved
WikidataJenkins: Verified
jenkins-bot: Verified
diff --git a/repo/includes/ContentRetriever.php
b/repo/includes/ContentRetriever.php
index 10f0eb0..5ca5495 100644
--- a/repo/includes/ContentRetriever.php
+++ b/repo/includes/ContentRetriever.php
@@ -5,7 +5,6 @@
use Article;
use Content;
use Revision;
-use Title;
use WebRequest;
/**
@@ -18,6 +17,7 @@
*
* @licence GNU GPL v2+
* @author Katie Filbert aude.w...@gmail.com
+ * @author Thiemo Mättig thiemo.maet...@wikimedia.de
*/
class ContentRetriever {
@@ -30,20 +30,18 @@
* @todo split out the get revision id stuff, add tests and see if
* any core code can be shared here
*
-* @param Article $article
-* @param Title $title
* @param WebRequest $request
+* @param Article $article
*
* @return Content|null
*/
- public function getContentForRequest( Article $article, Title $title,
WebRequest $request ) {
- $queryValues = $request-getQueryValues();
- $oldId = $article-getOldID();
+ public function getContentForRequest( WebRequest $request, Article
$article ) {
+ $revision = $article-getRevisionFetched();
- if ( array_key_exists( 'diff', $queryValues ) ) {
- $revision = $this-getDiffRevision( $oldId,
$queryValues['diff'] );
- } else {
- $revision = Revision::newFromTitle( $title, $oldId );
+ if ( $request-getCheck( 'diff' ) ) {
+ $oldId = $revision-getId();
+ $diffValue = $request-getVal( 'diff' );
+ $revision = $this-resolveDiffRevision( $oldId,
$diffValue, $article );
}
return $revision !== null ?
@@ -56,53 +54,39 @@
* @since 0.5
*
* @param int $oldId
-* @param string|int $diffValue
+* @param int|string $diffValue
+* @param Article $article
*
* @return Revision|null
*/
- public function getDiffRevision( $oldId, $diffValue ) {
- if ( $this-isSpecialDiffParam( $diffValue ) ) {
- return $this-resolveDiffRevision( $oldId, $diffValue );
+ protected function resolveDiffRevision( $oldId, $diffValue, Article
$article )
+ {
+ $newid = $this-resolveNewid( $oldId, $diffValue, $article );
+
+ if ( !$newid ) {
+ $newid = $article-getTitle()-getLatestRevID();
}
- if ( is_numeric( $diffValue ) ) {
- $revId = (int)$diffValue;
- return Revision::newFromId( $revId );
- }
-
- // uses default handling for revision not found
- // @todo error handling could be improved!
- return null;
+ return Revision::newFromId( $newid );
}
/**
-* @param mixed $diffValue
+* Get the revision ID specified in the diff parameter or prev/next
revision of oldid
*
-* @return boolean
-*/
- protected function isSpecialDiffParam( $diffValue ) {
- return in_array( $diffValue, array( 'prev', 'next', 'cur', '0'
), true );
- }
-
- /**
-* For non-revision ids in the diff request param, get the correct
revision
+* @since 0.5
*
* @param int $oldId
-* @param string|int $diffValue
+* @param int|string $diffValue
+* @param Article $article
*
-* @return Revision
+* @return Bool|int
*/
- protected function resolveDiffRevision( $oldId, $diffValue ) {
- $oldIdRev = Revision::newFromId( $oldId );
-
- if ( $diffValue === 0 || $diffValue === 'cur' ) {
-
[MediaWiki-commits] [Gerrit] Wikibase parsing of diff=... parameter was different from core - change (mediawiki...Wikibase)
Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/115310
Change subject: Wikibase parsing of diff=... parameter was different from core
..
Wikibase parsing of diff=... parameter was different from core
Core logik is basically:
* Check if the diff parameter does exists.
* If yes, check if it is equal to 'prev'.
* If it's equal to 'next'.
* Convert everything else to a number.
* If that conversion returns 0 compare to the current revision.
* Else try to use the number to find a revision.
* That may fail (e.g. for diff=-1).
In other words: In core every non-numeric value including 'cur',
'curr', 'c', '0' and even the empty string and null
(...?diffoldid=123 will work) does exactly the same as 'cur'.
Bug: 61749
Change-Id: Id7aab6ec65a23128fffe4c9e366213603a7e59c6
---
M repo/includes/ContentRetriever.php
M repo/tests/phpunit/includes/ContentRetrieverTest.php
2 files changed, 15 insertions(+), 40 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/10/115310/1
diff --git a/repo/includes/ContentRetriever.php
b/repo/includes/ContentRetriever.php
index 10f0eb0..0bb1088 100644
--- a/repo/includes/ContentRetriever.php
+++ b/repo/includes/ContentRetriever.php
@@ -61,48 +61,20 @@
* @return Revision|null
*/
public function getDiffRevision( $oldId, $diffValue ) {
- if ( $this-isSpecialDiffParam( $diffValue ) ) {
- return $this-resolveDiffRevision( $oldId, $diffValue );
+ $oldRevision = Revision::newFromId( $oldId );
+
+ if ( $diffValue === 'prev' ) {
+ return $oldRevision;
+ } else if ( $diffValue === 'next' ) {
+ return $oldRevision-getNext();
}
- if ( is_numeric( $diffValue ) ) {
- $revId = (int)$diffValue;
- return Revision::newFromId( $revId );
+ // All remaining non-numeric values including 'cur' become 0,
see DifferenceEngine
+ $revId = intval( $diffValue );
+ if ( $revId === 0 ) {
+ $revId = $oldRevision-getTitle()-getLatestRevID();
}
-
- // uses default handling for revision not found
- // @todo error handling could be improved!
- return null;
- }
-
- /**
-* @param mixed $diffValue
-*
-* @return boolean
-*/
- protected function isSpecialDiffParam( $diffValue ) {
- return in_array( $diffValue, array( 'prev', 'next', 'cur', '0'
), true );
- }
-
- /**
-* For non-revision ids in the diff request param, get the correct
revision
-*
-* @param int $oldId
-* @param string|int $diffValue
-*
-* @return Revision
-*/
- protected function resolveDiffRevision( $oldId, $diffValue ) {
- $oldIdRev = Revision::newFromId( $oldId );
-
- if ( $diffValue === 0 || $diffValue === 'cur' ) {
- $curId = $oldIdRev-getTitle()-getLatestRevID();
- return Revision::newFromId( $curId );
- } elseif ( $diffValue === 'next' ) {
- return $oldIdRev-getNext();
- }
-
- return $oldIdRev;
+ return Revision::newFromId( $revId );
}
}
diff --git a/repo/tests/phpunit/includes/ContentRetrieverTest.php
b/repo/tests/phpunit/includes/ContentRetrieverTest.php
index 6d904b3..d72b9e4 100644
--- a/repo/tests/phpunit/includes/ContentRetrieverTest.php
+++ b/repo/tests/phpunit/includes/ContentRetrieverTest.php
@@ -88,8 +88,11 @@
array( $content2, $revIds[0], array( 'diff' = 'next',
'oldid' = $revIds[0] ), $title, 'diff=next' ),
array( $content2, $revIds[1], array( 'diff' = 'prev',
'oldid' = $revIds[1] ), $title, 'diff=prev' ),
array( $content3, $revIds[1], array( 'diff' = 'cur',
'oldid' = $revIds[1] ), $title, 'diff=cur' ),
+ array( $content3, $revIds[1], array( 'diff' = 'c',
'oldid' = $revIds[1] ), $title, 'diff=c' ),
+ array( $content3, $revIds[1], array( 'diff' = '0',
'oldid' = $revIds[1] ), $title, 'diff=0' ),
+ array( $content3, $revIds[1], array( 'diff' = '',
'oldid' = $revIds[1] ), $title, 'diff=' ),
array( $content3, $revIds[2], array(), $title, 'no
query params' ),
- array( null, $revIds[1], array( 'diff' = 'kitten',
'oldid' = $revIds[0] ), $title, 'diff=kitten' )
+ array( null, $revIds[1], array( 'diff' = '-1', 'oldid'
= $revIds[0] ), $title, 'diff=-1' )
);
}
--
To view, visit https://gerrit.wikimedia.org/r/115310
To
