WikiPage::doModify: Reuse old revision while null-editing

The expected behavior for a null-edit is that the cache gets fully
purged as if another edit has happened, without actually inserting
a new revision into the database. Old implementation was to create
a new Revision instance, copy some of the private instance properties
from the old Revision instance, and skip database insertion. This
method is, unfortunately, prone to errors, as hooks and parsers
expect the given revision data to be equal to whatever the latest
revision is in the database, including timestamp and edit summary.

An alternative solution would be to maintain a method that copies
all the needed data from one revision to another; however, the extra
maintenance cost would be that it would be annoying to maintain it.
This implementation simply reuses the old Revision instance given
by the caller (WikiPage::doEditContent), as it is the latest
revision that hooks and parsers expect.

This patch fixes the error where, after null-editing, the
revision-related magic words, such as {{REVISIONDAY}}, are changed
to the values given by the null edit, instead of the latest revision.
Old behavior was caused by the parser parsing with incorrect
revision data.

Deprecate Revision::setUserIdAndName. It was a hack added in
147f79e for the same bug, addressing {{REVISIONUSER}}, but failed
to address the other magic words, including {{REVISIONDAY}}. I
failed to find any other usage of this instance method.

Bug: T135261
Change-Id: Ifce6a753effb98123574bff45ff59b5c9780f0c2

diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31
index 4bfcfcb..08e6109 100644 (file)
--- a/RELEASE-NOTES-1.31
+++ b/RELEASE-NOTES-1.31
@@ -92,8 +92,9 @@ changes to languages because of Phabricator reports.
   * Revision::selectArchiveFields() → Revision::getArchiveQueryInfo()
   * User::selectFields() → User::getQueryInfo()
   * WikiPage::selectFields() → WikiPage::getQueryInfo()
-* Due to significant refactoring, method ContribsPager::getUserCond() that had
-  no access restriction has been removed.
+  * Due to significant refactoring, method ContribsPager::getUserCond() that had
+    no access restriction has been removed.
+  * Revision::setUserIdAndName() was deprecated.
 
 == Compatibility ==
 MediaWiki 1.31 requires PHP 5.5.9 or later. There is experimental support for

diff --git a/includes/Revision.php b/includes/Revision.php
index 518360c..25c89c2 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -901,6 +901,7 @@ class Revision implements IDBAccessObject {
         * This should only be used for proposed revisions that turn out to be null edits
         *
         * @since 1.28
+        * @deprecated since 1.31, please reuse old Revision object
         * @param int $id User ID
         * @param string $name User name
         */

diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php
index 1047a93..95b7be2 100644 (file)
--- a/includes/page/WikiPage.php
+++ b/includes/page/WikiPage.php
@@ -1705,27 +1705,27 @@ class WikiPage implements Page, IDBAccessObject {
                        throw new MWException( "Could not find text for current revision {$oldid}." );
                }
 
-               // @TODO: pass content object?!
-               $revision = new Revision( [
-                       'page'       => $this->getId(),
-                       'title'      => $this->mTitle, // for determining the default content model
-                       'comment'    => $summary,
-                       'minor_edit' => $meta['minor'],
-                       'text'       => $meta['serialized'],
-                       'len'        => $newsize,
-                       'parent_id'  => $oldid,
-                       'user'       => $user->getId(),
-                       'user_text'  => $user->getName(),
-                       'timestamp'  => $now,
-                       'content_model' => $content->getModel(),
-                       'content_format' => $meta['serialFormat'],
-               ] );
-
                $changed = !$content->equals( $oldContent );
 
                $dbw = wfGetDB( DB_MASTER );
 
                if ( $changed ) {
+                       // @TODO: pass content object?!
+                       $revision = new Revision( [
+                               'page'       => $this->getId(),
+                               'title'      => $this->mTitle, // for determining the default content model
+                               'comment'    => $summary,
+                               'minor_edit' => $meta['minor'],
+                               'text'       => $meta['serialized'],
+                               'len'        => $newsize,
+                               'parent_id'  => $oldid,
+                               'user'       => $user->getId(),
+                               'user_text'  => $user->getName(),
+                               'timestamp'  => $now,
+                               'content_model' => $content->getModel(),
+                               'content_format' => $meta['serialFormat'],
+                       ] );
+
                        $prepStatus = $content->prepareSave( $this, $flags, $oldid, $user );
                        $status->merge( $prepStatus );
                        if ( !$status->isOK() ) {
@@ -1791,11 +1791,9 @@ class WikiPage implements Page, IDBAccessObject {
                } else {
                        // T34948: revision ID must be set to page {{REVISIONID}} and
                        // related variables correctly. Likewise for {{REVISIONUSER}} (T135261).
-                       $revision->setId( $this->getLatest() );
-                       $revision->setUserIdAndName(
-                               $this->getUser( Revision::RAW ),
-                               $this->getUserText( Revision::RAW )
-                       );
+                       // Since we don't insert a new revision into the database, the least
+                       // error-prone way is to reuse given old revision.
+                       $revision = $meta['oldRevision'];
                }
 
                if ( $changed ) {