(bug 56849) Deprecate dangerous edittime-based content update functions

Existing edit conflict logic relies on second-resolution timestamps,
which can fail in many exciting ways.  This patch provides new support
functions which take an explicit revision ID instead of a timestamp.

No functionality is changed.  Hopefully.

TODO:
* Make use of this code by replacing all edittime and starttime logic
with an explicit baseRevId.

Change-Id: I83cdd4adaaa67f81e933654228eccdc9fcdf9009

diff --git a/includes/Revision.php b/includes/Revision.php
index 86c3057..c5f3a15 100644 (file)
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -1731,6 +1731,7 @@ class Revision implements IDBAccessObject {
         * 50 revisions for the sake of performance.
         *
         * @since 1.20
+        * @deprecated since 1.24
         *
         * @param DatabaseBase|int $db The Database to perform the check on. May be given as a
         *        Database object or a database identifier usable with wfGetDB.

diff --git a/includes/WikiPage.php b/includes/WikiPage.php
index 8fe948b..676d8d5 100644 (file)
--- a/includes/WikiPage.php
+++ b/includes/WikiPage.php
@@ -1387,6 +1387,8 @@ class WikiPage implements Page, IDBAccessObject {
         * If the given revision is newer than the currently set page_latest,
         * update the page record. Otherwise, do nothing.
         *
+        * @deprecated since 1.24, use updateRevisionOn instead
+        *
         * @param DatabaseBase $dbw
         * @param Revision $revision
         * @return bool
@@ -1528,11 +1530,45 @@ class WikiPage implements Page, IDBAccessObject {
         * @return Content New complete article content, or null if error.
         *
         * @since 1.21
+        * @deprecated since 1.24, use replaceSectionAtRev instead
         */
        public function replaceSectionContent( $section, Content $sectionContent, $sectionTitle = '',
                $edittime = null ) {
                wfProfileIn( __METHOD__ );
 
+               $baseRevId = null;
+               if ( $edittime && $section !== 'new' ) {
+                       $dbw = wfGetDB( DB_MASTER );
+                       $rev = Revision::loadFromTimestamp( $dbw, $this->mTitle, $edittime );
+                       if ( !$rev ) {
+                               wfDebug( __METHOD__ . " given bad revision time for page " .
+                                       $this->getId() . "; edittime: $edittime)\n" );
+                               wfProfileOut( __METHOD__ );
+                               return null;
+                       }
+                       $baseRevId = $rev->getId();
+               }
+
+               wfProfileOut( __METHOD__ );
+               return $this->replaceSectionAtRev( $section, $sectionContent, $sectionTitle, $baseRevId );
+       }
+
+       /**
+        * @param string|null|bool $section Null/false, a section number (0, 1, 2, T1, T2, ...) or "new".
+        * @param Content $sectionContent New content of the section.
+        * @param string $sectionTitle New section's subject, only if $section is "new".
+        * @param string $baseRevId integer|null
+        *
+        * @throws MWException
+        * @return Content New complete article content, or null if error.
+        *
+        * @since 1.24
+        */
+       public function replaceSectionAtRev( $section, Content $sectionContent,
+               $sectionTitle = '', $baseRevId = null
+       ) {
+               wfProfileIn( __METHOD__ );
+
                if ( strval( $section ) == '' ) {
                        // Whole-page edit; let the whole text through
                        $newContent = $sectionContent;
@@ -1544,11 +1580,12 @@ class WikiPage implements Page, IDBAccessObject {
                        }
 
                        // Bug 30711: always use current version when adding a new section
-                       if ( is_null( $edittime ) || $section == 'new' ) {
+                       if ( is_null( $baseRevId ) || $section == 'new' ) {
                                $oldContent = $this->getContent();
                        } else {
+                               // TODO: try DB_READ first
                                $dbw = wfGetDB( DB_MASTER );
-                               $rev = Revision::loadFromTimestamp( $dbw, $this->mTitle, $edittime );
+                               $rev = Revision::loadFromId( $dbw, $baseRevId );
 
                                if ( !$rev ) {
                                        wfDebug( "WikiPage::replaceSection asked for bogus section (page: " .

diff --git a/tests/phpunit/includes/WikiPageTest.php b/tests/phpunit/includes/WikiPageTest.php
index 37f1975..78457d2 100644 (file)
--- a/tests/phpunit/includes/WikiPageTest.php
+++ b/tests/phpunit/includes/WikiPageTest.php
@@ -817,6 +817,22 @@ more stuff
                $this->assertEquals( $expected, is_null( $c ) ? null : trim( $c->getNativeData() ) );
        }
 
+       /**
+        * @dataProvider dataReplaceSection
+        * @covers WikiPage::replaceSectionAtRev
+        */
+       public function testReplaceSectionAtRev( $title, $model, $text, $section,
+               $with, $sectionTitle, $expected
+       ) {
+               $page = $this->createPage( $title, $text, $model );
+               $baseRevId = $page->getLatest();
+
+               $content = ContentHandler::makeContent( $with, $page->getTitle(), $page->getContentModel() );
+               $c = $page->replaceSectionAtRev( $section, $content, $sectionTitle, $baseRevId );
+
+               $this->assertEquals( $expected, is_null( $c ) ? null : trim( $c->getNativeData() ) );
+       }
+
        /* @todo FIXME: fix this!
        public function testGetUndoText() {
        $this->checkHasDiff3();