RevisionStore: Avoid exception on prev/next of deleted revision

This makes RevisionStore a bit more robust against use with deleted
revisions.

Note that this does not make I151019e336bda redundant, since detecting
this situation early and providing meaningful error message is useful.

Bug: T208929
Change-Id: I7635ddc0176e21d873eacadebe02603b1fe51c38

diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php
index 6d3b72c..73881cd 100644 (file)
--- a/includes/Revision/RevisionStore.php
+++ b/includes/Revision/RevisionStore.php
@@ -2517,45 +2517,78 @@ class RevisionStore
        }
 
        /**
-        * Get previous revision for this title
+        * Get the revision before $rev in the page's history, if any.
+        * Will return null for the first revision but also for deleted or unsaved revisions.
         *
         * MCR migration note: this replaces Revision::getPrevious
         *
+        * @see Title::getPreviousRevisionID
+        * @see PageArchive::getPreviousRevision
+        *
         * @param RevisionRecord $rev
         * @param Title|null $title if known (optional)
         *
         * @return RevisionRecord|null
         */
        public function getPreviousRevision( RevisionRecord $rev, Title $title = null ) {
+               if ( !$rev->getId() || !$rev->getPageId() ) {
+                       // revision is unsaved or otherwise incomplete
+                       return null;
+               }
+
+               if ( $rev instanceof RevisionArchiveRecord ) {
+                       // revision is deleted, so it's not part of the page history
+                       return null;
+               }
+
                if ( $title === null ) {
+                       // this would fail for deleted revisions
                        $title = $this->getTitle( $rev->getPageId(), $rev->getId() );
                }
+
                $prev = $title->getPreviousRevisionID( $rev->getId() );
-               if ( $prev ) {
-                       return $this->getRevisionByTitle( $title, $prev );
+               if ( !$prev ) {
+                       return null;
                }
-               return null;
+
+               return $this->getRevisionByTitle( $title, $prev );
        }
 
        /**
-        * Get next revision for this title
+        * Get the revision after $rev in the page's history, if any.
+        * Will return null for the latest revision but also for deleted or unsaved revisions.
         *
         * MCR migration note: this replaces Revision::getNext
         *
+        * @see Title::getNextRevisionID
+        *
         * @param RevisionRecord $rev
         * @param Title|null $title if known (optional)
         *
         * @return RevisionRecord|null
         */
        public function getNextRevision( RevisionRecord $rev, Title $title = null ) {
+               if ( !$rev->getId() || !$rev->getPageId() ) {
+                       // revision is unsaved or otherwise incomplete
+                       return null;
+               }
+
+               if ( $rev instanceof RevisionArchiveRecord ) {
+                       // revision is deleted, so it's not part of the page history
+                       return null;
+               }
+
                if ( $title === null ) {
+                       // this would fail for deleted revisions
                        $title = $this->getTitle( $rev->getPageId(), $rev->getId() );
                }
+
                $next = $title->getNextRevisionID( $rev->getId() );
-               if ( $next ) {
-                       return $this->getRevisionByTitle( $title, $next );
+               if ( !$next ) {
+                       return null;
                }
-               return null;
+
+               return $this->getRevisionByTitle( $title, $next );
        }
 
        /**

diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
index 0d6a439..4ece51b 100644 (file)
--- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
+++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
@@ -12,7 +12,9 @@ use MediaWiki\Linker\LinkTarget;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\IncompleteRevisionException;
 use MediaWiki\Revision\MutableRevisionRecord;
+use MediaWiki\Revision\RevisionArchiveRecord;
 use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Revision\RevisionSlots;
 use MediaWiki\Revision\RevisionStore;
 use MediaWiki\Revision\SlotRecord;
 use MediaWiki\Storage\BlobStoreFactory;
@@ -1354,6 +1356,44 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
                );
        }
 
+       public function provideNonHistoryRevision() {
+               $title = Title::newFromText( __METHOD__ );
+               $rev = new MutableRevisionRecord( $title );
+               yield [ $rev ];
+
+               $user = new UserIdentityValue( 7, 'Frank', 0 );
+               $comment = CommentStoreComment::newUnsavedComment( 'Test' );
+               $row = (object)[
+                       'ar_id' => 3,
+                       'ar_rev_id' => 34567,
+                       'ar_page_id' => 5,
+                       'ar_deleted' => 0,
+                       'ar_minor_edit' => 0,
+                       'ar_timestamp' => '20180101020202',
+               ];
+               $slots = new RevisionSlots( [] );
+               $rev = new RevisionArchiveRecord( $title, $user, $comment, $row, $slots );
+               yield [ $rev ];
+       }
+
+       /**
+        * @dataProvider provideNonHistoryRevision
+        * @covers \MediaWiki\Revision\RevisionStore::getPreviousRevision
+        */
+       public function testGetPreviousRevision_bad( RevisionRecord $rev ) {
+               $store = MediaWikiServices::getInstance()->getRevisionStore();
+               $this->assertNull( $store->getPreviousRevision( $rev ) );
+       }
+
+       /**
+        * @dataProvider provideNonHistoryRevision
+        * @covers \MediaWiki\Revision\RevisionStore::getNextRevision
+        */
+       public function testGetNextRevision_bad( RevisionRecord $rev ) {
+               $store = MediaWikiServices::getInstance()->getRevisionStore();
+               $this->assertNull( $store->getNextRevision( $rev ) );
+       }
+
        /**
         * @covers \MediaWiki\Revision\RevisionStore::getTimestampFromId
         */