ApiComparePages: Don't try to find next/prev of a deleted revision
authorBrad Jorsch <bjorsch@wikimedia.org>
Wed, 7 Nov 2018 16:01:26 +0000 (11:01 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Mon, 3 Dec 2018 20:31:34 +0000 (15:31 -0500)
RevisionStore::getPreviousRevision() and ::getNextRevision() don't
handle being passed a RevisionArchiveRecord very well. Even if they
would, it's not clear whether the user wants to be comparing with the
next/previous deleted revision or the next/previous revision even if not
deleted. So let's just make it an error, at least for now.

Bug: T208929
Change-Id: I151019e336bda92aa4040ba4162eb2588c909652

includes/api/ApiComparePages.php
includes/api/i18n/en.json
includes/api/i18n/qqq.json
tests/phpunit/includes/api/ApiComparePagesTest.php

index 76b7bce..97b21b9 100644 (file)
@@ -22,6 +22,7 @@
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Revision\RevisionArchiveRecord;
 use MediaWiki\Revision\RevisionStore;
 use MediaWiki\Revision\SlotRecord;
 
@@ -61,6 +62,11 @@ class ApiComparePages extends ApiBase {
                        if ( !$fromRelRev ) {
                                $this->dieWithError( 'apierror-compare-relative-to-nothing' );
                        }
+                       if ( $params['torelative'] !== 'cur' && $fromRelRev instanceof RevisionArchiveRecord ) {
+                               // RevisionStore's getPreviousRevision/getNextRevision blow up
+                               // when passed an RevisionArchiveRecord for a deleted page
+                               $this->dieWithError( [ 'apierror-compare-relative-to-deleted', $params['torelative'] ] );
+                       }
                        switch ( $params['torelative'] ) {
                                case 'prev':
                                        // Swap 'from' and 'to'
index 83bb6e6..0028dfc 100644 (file)
        "apierror-compare-nofromrevision": "No 'from' revision. Specify <var>fromrev</var>, <var>fromtitle</var>, or <var>fromid</var>.",
        "apierror-compare-notext": "Parameter <var>$1</var> cannot be used without <var>$2</var>.",
        "apierror-compare-notorevision": "No 'to' revision. Specify <var>torev</var>, <var>totitle</var>, or <var>toid</var>.",
+       "apierror-compare-relative-to-deleted": "Cannot use <kbd>torelative=$1</kbd> relative to a deleted revision.",
        "apierror-compare-relative-to-nothing": "No 'from' revision for <var>torelative</var> to be relative to.",
        "apierror-contentserializationexception": "Content serialization failed: $1",
        "apierror-contenttoobig": "The content you supplied exceeds the article size limit of $1 {{PLURAL:$1|kilobyte|kilobytes}}.",
index 83427ba..660e0fa 100644 (file)
        "apierror-compare-nofromrevision": "{{doc-apierror}}",
        "apierror-compare-notext": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter that is not allowed without <var>totext-&#x7B;role}</var>/<var>fromtext-&#x7B;role}</var>.\n* $2 - The specific <var>totext-&#x7B;role}</var>/<var>fromtext-&#x7B;role}</var> parameter that must be present.",
        "apierror-compare-notorevision": "{{doc-apierror}}",
+       "apierror-compare-relative-to-deleted": "{{doc-apierror}}",
        "apierror-compare-relative-to-nothing": "{{doc-apierror}}",
        "apierror-contentserializationexception": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text, may end with punctuation. Currently this is probably English, hopefully we'll fix that in the future.",
        "apierror-contenttoobig": "{{doc-apierror}}\n\nParameters:\n* $1 - Maximum article size in kilobytes.",
index 3709c21..e8cd342 100644 (file)
@@ -801,6 +801,30 @@ class ApiComparePagesTest extends ApiTestCase {
                                [],
                                'nosuchrevid',
                        ],
+                       'Error, deleted revision ID and torelative=prev' => [
+                               [
+                                       'fromrev' => '{{REPL:revC2}}',
+                                       'torelative' => 'prev',
+                               ],
+                               [],
+                               'compare-relative-to-deleted', true
+                       ],
+                       'Error, deleted revision ID and torelative=next' => [
+                               [
+                                       'fromrev' => '{{REPL:revC2}}',
+                                       'torelative' => 'next',
+                               ],
+                               [],
+                               'compare-relative-to-deleted', true
+                       ],
+                       'Deleted revision ID and torelative=cur' => [
+                               [
+                                       'fromrev' => '{{REPL:revC2}}',
+                                       'torelative' => 'cur',
+                               ],
+                               [],
+                               'nosuchrevid', true
+                       ],
                        'Error, revision-deleted content' => [
                                [
                                        'fromrev' => '{{REPL:revA2}}',