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
(...?diff&oldid=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 unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id7aab6ec65a23128fffe4c9e366213603a7e59c6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
