ApiComparePages: Don't error with no prev/next rev

Prior to I700edfa76, torelative=prev on the first revision of a page
would "diff" from an empty revision, and torelative=next on the latest
revision would diff to that same latest revision. People were depending
on that behavior, so restore it.

Bug: T203433
Change-Id: Ie81b58c196998a8047322740fe1d1fa44eff8526

diff --git a/includes/api/ApiComparePages.php b/includes/api/ApiComparePages.php
index 76b7bce..3a92b7c 100644 (file)
--- a/includes/api/ApiComparePages.php
+++ b/includes/api/ApiComparePages.php
@@ -64,16 +64,48 @@ class ApiComparePages extends ApiBase {
                        switch ( $params['torelative'] ) {
                                case 'prev':
                                        // Swap 'from' and 'to'
-                                       list( $toRev, $toRelRev2, $toValsRev ) = [ $fromRev, $fromRelRev, $fromValsRev ];
-                                       $fromRev = $this->revisionStore->getPreviousRevision( $fromRelRev );
+                                       list( $toRev, $toRelRev, $toValsRev ) = [ $fromRev, $fromRelRev, $fromValsRev ];
+                                       $fromRev = $this->revisionStore->getPreviousRevision( $toRelRev );
                                        $fromRelRev = $fromRev;
                                        $fromValsRev = $fromRev;
+                                       if ( !$fromRev ) {
+                                               $title = Title::newFromLinkTarget( $toRelRev->getPageAsLinkTarget() );
+                                               $this->addWarning( [
+                                                       'apiwarn-compare-no-prev',
+                                                       wfEscapeWikiText( $title->getPrefixedText() ),
+                                                       $toRelRev->getId()
+                                               ] );
+
+                                               // (T203433) Create an empty dummy revision as the "previous".
+                                               // The main slot has to exist, the rest will be handled by DifferenceEngine.
+                                               $fromRev = $this->revisionStore->newMutableRevisionFromArray( [
+                                                       'title' => $title ?: Title::makeTitle( NS_SPECIAL, 'Badtitle/' . __METHOD__ )
+                                               ] );
+                                               $fromRev->setContent(
+                                                       SlotRecord::MAIN,
+                                                       $toRelRev->getContent( SlotRecord::MAIN, RevisionRecord::RAW )
+                                                               ->getContentHandler()
+                                                               ->makeEmptyContent()
+                                               );
+                                       }
                                        break;
 
                                case 'next':
                                        $toRev = $this->revisionStore->getNextRevision( $fromRelRev );
                                        $toRelRev = $toRev;
                                        $toValsRev = $toRev;
+                                       if ( !$toRev ) {
+                                               $title = Title::newFromLinkTarget( $fromRelRev->getPageAsLinkTarget() );
+                                               $this->addWarning( [
+                                                       'apiwarn-compare-no-next',
+                                                       wfEscapeWikiText( $title->getPrefixedText() ),
+                                                       $fromRelRev->getId()
+                                               ] );
+
+                                               // (T203433) The web UI treats "next" as "cur" in this case.
+                                               // Avoid repeating metadata by making a MutableRevisionRecord with no changes.
+                                               $toRev = MutableRevisionRecord::newFromParentRevision( $fromRelRev );
+                                       }
                                        break;
 
                                case 'cur':
@@ -93,10 +125,12 @@ class ApiComparePages extends ApiBase {
                        list( $toRev, $toRelRev, $toValsRev ) = $this->getDiffRevision( 'to', $params );
                }
 
-               // Handle missing from or to revisions
+               // Handle missing from or to revisions (should never happen)
+               // @codeCoverageIgnoreStart
                if ( !$fromRev || !$toRev ) {
                        $this->dieWithError( 'apierror-baddiff' );
                }
+               // @codeCoverageIgnoreEnd
 
                // Handle revdel
                if ( !$fromRev->audienceCan(

diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json
index 25bf3f7..88b5d68 100644 (file)
--- a/includes/api/i18n/en.json
+++ b/includes/api/i18n/en.json
@@ -1887,6 +1887,8 @@
        "apiwarn-badurlparam": "Could not parse <var>$1urlparam</var> for $2. Using only width and height.",
        "apiwarn-badutf8": "The value passed for <var>$1</var> contains invalid or non-normalized data. Textual data should be valid, NFC-normalized Unicode without C0 control characters other than HT (\\t), LF (\\n), and CR (\\r).",
        "apiwarn-checktoken-percentencoding": "Check that symbols such as \"+\" in the token are properly percent-encoded in the URL.",
+       "apiwarn-compare-no-next": "Revision $2 is the latest revision of $1, there is no revision for <kbd>torelative=next</kbd> to compare to.",
+       "apiwarn-compare-no-prev": "Revision $2 is the earliest revision of $1, there is no revision for <kbd>torelative=prev</kbd> to compare to.",
        "apiwarn-compare-nocontentmodel": "No content model could be determined, assuming $1.",
        "apiwarn-deprecation-deletedrevs": "<kbd>list=deletedrevs</kbd> has been deprecated. Please use <kbd>prop=deletedrevisions</kbd> or <kbd>list=alldeletedrevisions</kbd> instead.",
        "apiwarn-deprecation-httpsexpected": "HTTP used when HTTPS was expected.",

diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json
index d279330..2407781 100644 (file)
--- a/includes/api/i18n/qqq.json
+++ b/includes/api/i18n/qqq.json
@@ -1774,6 +1774,8 @@
        "apiwarn-badurlparam": "{{doc-apierror}}\n\nParameters:\n* $1 - Module parameter prefix, e.g. \"bl\".\n* $2 - Image title.",
        "apiwarn-badutf8": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n{{doc-important|Do not translate \"\\t\", \"\\n\", and \"\\r\"}}",
        "apiwarn-checktoken-percentencoding": "{{doc-apierror}}",
+       "apiwarn-compare-no-next": "{{doc-apierror}}\n\nParameters:\n* $1 - Title of the page.\n* $2 - Revision ID.",
+       "apiwarn-compare-no-prev": "{{doc-apierror}}\n\nParameters:\n* $1 - Title of the page.\n* $2 - Revision ID.",
        "apiwarn-compare-nocontentmodel": "{{doc-apierror}}\n\nParameters:\n* $1 - Content model being assumed.",
        "apiwarn-deprecation-deletedrevs": "{{doc-apierror}}",
        "apiwarn-deprecation-httpsexpected": "{{doc-apierror}}",

diff --git a/tests/phpunit/includes/api/ApiComparePagesTest.php b/tests/phpunit/includes/api/ApiComparePagesTest.php
index 3709c21..e7be467 100644 (file)
--- a/tests/phpunit/includes/api/ApiComparePagesTest.php
+++ b/tests/phpunit/includes/api/ApiComparePagesTest.php
@@ -553,6 +553,62 @@ class ApiComparePagesTest extends ApiTestCase {
                                        ]
                                ],
                        ],
+                       'Relative diff, no prev' => [
+                               [
+                                       'fromrev' => '{{REPL:revA1}}',
+                                       'torelative' => 'prev',
+                                       'prop' => 'ids|rel|diff|title|user|comment',
+                               ],
+                               [
+                                       'warnings' => [
+                                               [
+                                                       'code' => 'compare-no-prev',
+                                                       'module' => 'compare',
+                                               ],
+                                       ],
+                                       'compare' => [
+                                               'toid' => '{{REPL:pageA}}',
+                                               'torevid' => '{{REPL:revA1}}',
+                                               'tons' => 0,
+                                               'totitle' => 'ApiComparePagesTest A',
+                                               'touser' => '{{REPL:creator}}',
+                                               'touserid' => '{{REPL:creatorid}}',
+                                               'tocomment' => 'Test for ApiComparePagesTest: A 1',
+                                               'toparsedcomment' => 'Test for ApiComparePagesTest: A 1',
+                                               'next' => '{{REPL:revA2}}',
+                                               'body' => '<tr><td colspan="2" class="diff-lineno" id="mw-diff-left-l1" >Line 1:</td>' . "\n"
+                                                       . '<td colspan="2" class="diff-lineno">Line 1:</td></tr>' . "\n"
+                                                       . '<tr><td class=\'diff-marker\'>−</td><td class=\'diff-deletedline\'><div> </div></td><td class=\'diff-marker\'>+</td><td class=\'diff-addedline\'><div><ins class="diffchange diffchange-inline">A 1</ins></div></td></tr>' . "\n",
+                                       ],
+                               ],
+                       ],
+                       'Relative diff, no next' => [
+                               [
+                                       'fromrev' => '{{REPL:revA4}}',
+                                       'torelative' => 'next',
+                                       'prop' => 'ids|rel|diff|title|user|comment',
+                               ],
+                               [
+                                       'warnings' => [
+                                               [
+                                                       'code' => 'compare-no-next',
+                                                       'module' => 'compare',
+                                               ],
+                                       ],
+                                       'compare' => [
+                                               'fromid' => '{{REPL:pageA}}',
+                                               'fromrevid' => '{{REPL:revA4}}',
+                                               'fromns' => 0,
+                                               'fromtitle' => 'ApiComparePagesTest A',
+                                               'fromuser' => '{{REPL:creator}}',
+                                               'fromuserid' => '{{REPL:creatorid}}',
+                                               'fromcomment' => 'Test for ApiComparePagesTest: A 4',
+                                               'fromparsedcomment' => 'Test for ApiComparePagesTest: A 4',
+                                               'prev' => '{{REPL:revA3}}',
+                                               'body' => '',
+                                       ],
+                               ],
+                       ],
                        'Diff for specific slots' => [
                                // @todo Use a page with multiple slots here
                                [
@@ -874,24 +930,6 @@ class ApiComparePagesTest extends ApiTestCase {
                                [],
                                'missingcontent'
                        ],
-                       'Error, Relative diff, no prev' => [
-                               [
-                                       'fromrev' => '{{REPL:revA1}}',
-                                       'torelative' => 'prev',
-                                       'prop' => 'ids',
-                               ],
-                               [],
-                               'baddiff'
-                       ],
-                       'Error, Relative diff, no next' => [
-                               [
-                                       'fromrev' => '{{REPL:revA4}}',
-                                       'torelative' => 'next',
-                                       'prop' => 'ids',
-                               ],
-                               [],
-                               'baddiff'
-                       ],
                        'Error, section diff with no revision' => [
                                [
                                        'fromtitle' => 'ApiComparePagesTest F',