Split out edit/create methods from doEditContent()
authorAaron Schulz <aschulz@wikimedia.org>
Wed, 4 Nov 2015 01:12:15 +0000 (17:12 -0800)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 7 Dec 2015 03:49:40 +0000 (19:49 -0800)
* Make the method sizes a bit more manageable.
  This will be useful for replacing the begin/commit
  calls later (with startAtomic/endAtomic).
* Cleaned up a few inconsistencies in code style.

Change-Id: I8d66503a5575ca369cd5feb56058af7d24001629

includes/page/WikiPage.php

index 4fbb845..eadabac 100644 (file)
@@ -1663,211 +1663,170 @@ class WikiPage implements Page, IDBAccessObject {
         * @since 1.21
         * @throws MWException
         */
-       public function doEditContent( Content $content, $summary, $flags = 0, $baseRevId = false,
+       public function doEditContent(
+               Content $content, $summary, $flags = 0, $baseRevId = false,
                User $user = null, $serialFormat = null
        ) {
-               global $wgUser, $wgUseAutomaticEditSummaries, $wgUseRCPatrol, $wgUseNPPatrol;
+               global $wgUser, $wgUseAutomaticEditSummaries;
 
                // Low-level sanity check
                if ( $this->mTitle->getText() === '' ) {
                        throw new MWException( 'Something is trying to edit an article with an empty title' );
                }
-
-               if ( !$content->getContentHandler()->canBeUsedOn( $this->getTitle() ) ) {
+               // Make sure the given content type is allowed for this page
+               if ( !$content->getContentHandler()->canBeUsedOn( $this->mTitle ) ) {
                        return Status::newFatal( 'content-not-allowed-here',
                                ContentHandler::getLocalizedName( $content->getModel() ),
-                               $this->getTitle()->getPrefixedText() );
+                               $this->mTitle->getPrefixedText()
+                       );
                }
 
-               $user = is_null( $user ) ? $wgUser : $user;
-               $status = Status::newGood( array() );
-
                // Load the data from the master database if needed.
                // The caller may already loaded it from the master or even loaded it using
                // SELECT FOR UPDATE, so do not override that using clear().
                $this->loadPageData( 'fromdbmaster' );
 
+               $user = $user ?: $wgUser;
                $flags = $this->checkFlags( $flags );
 
-               // handle hook
+               // Trigger pre-save hook (using provided edit summary)
+               $status = Status::newGood( array() );
                $hook_args = array( &$this, &$user, &$content, &$summary,
                                                        $flags & EDIT_MINOR, null, null, &$flags, &$status );
-
+               // Check if the hook rejected the attempted save
                if ( !Hooks::run( 'PageContentSave', $hook_args )
-                       || !ContentHandler::runLegacyHooks( 'ArticleSave', $hook_args ) ) {
-
-                       wfDebug( __METHOD__ . ": ArticleSave or ArticleSaveContent hook aborted save!\n" );
-
+                       || !ContentHandler::runLegacyHooks( 'ArticleSave', $hook_args )
+               ) {
                        if ( $status->isOK() ) {
+                               // Hook returned false but didn't call fatal(); use generic message
                                $status->fatal( 'edit-hook-aborted' );
                        }
 
                        return $status;
                }
 
-               // Silently ignore EDIT_MINOR if not allowed
-               $isminor = ( $flags & EDIT_MINOR ) && $user->isAllowed( 'minoredit' );
-               $bot = $flags & EDIT_FORCE_BOT;
-
                $old_revision = $this->getRevision(); // current revision
                $old_content = $this->getContent( Revision::RAW ); // current revision's content
 
-               $oldsize = $old_content ? $old_content->getSize() : 0;
-               $oldid = $this->getLatest();
-               $oldIsRedirect = $this->isRedirect();
-               $oldcountable = $this->isCountable();
-
-               $handler = $content->getContentHandler();
-
-               // Provide autosummaries if one is not provided and autosummaries are enabled.
-               if ( $wgUseAutomaticEditSummaries && $flags & EDIT_AUTOSUMMARY && $summary == '' ) {
-                       if ( !$old_content ) {
-                               $old_content = null;
-                       }
+               // Provide autosummaries if one is not provided and autosummaries are enabled
+               if ( $wgUseAutomaticEditSummaries && ( $flags & EDIT_AUTOSUMMARY ) && $summary == '' ) {
+                       $handler = $content->getContentHandler();
                        $summary = $handler->getAutosummary( $old_content, $content, $flags );
                }
 
+               // Get the pre-save transform content and final parser output
                $editInfo = $this->prepareContentForEdit( $content, null, $user, $serialFormat );
-               $serialized = $editInfo->pst;
-
-               /**
-                * @var Content $content
-                */
-               $content = $editInfo->pstContent;
-               $newsize = $content->getSize();
-
-               $dbw = wfGetDB( DB_MASTER );
-               $now = wfTimestampNow();
+               $pstContent = $editInfo->pstContent; // Content object
+               $meta = array(
+                       'bot' => ( $flags & EDIT_FORCE_BOT ),
+                       'minor' => ( $flags & EDIT_MINOR ) && $user->isAllowed( 'minoredit' ),
+                       'serialized' => $editInfo->pst,
+                       'serialFormat' => $serialFormat,
+                       'baseRevId' => $baseRevId,
+                       'oldRevision' => $old_revision,
+                       'oldContent' => $old_content,
+                       'oldId' => $this->getLatest(),
+                       'oldIsRedirect' => $this->isRedirect(),
+                       'oldCountable' => $this->isCountable()
+               );
 
+               // Actually create the revision and create/update the page
                if ( $flags & EDIT_UPDATE ) {
-                       // Update article, but only if changed.
-                       $status->value['new'] = false;
-
-                       if ( !$oldid ) {
-                               // Article gone missing
-                               wfDebug( __METHOD__ . ": EDIT_UPDATE specified but article doesn't exist\n" );
-                               $status->fatal( 'edit-gone-missing' );
-
-                               return $status;
-                       } elseif ( !$old_content ) {
-                               // Sanity check for bug 37225
-                               throw new MWException( "Could not find text for current revision {$oldid}." );
-                       }
+                       $status = $this->doModify( $pstContent, $flags, $user, $summary, $meta );
+               } else {
+                       $status = $this->doCreate( $pstContent, $flags, $user, $summary, $meta );
+               }
 
-                       $revision = new Revision( array(
-                               'page'       => $this->getId(),
-                               'title'      => $this->getTitle(), // for determining the default content model
-                               'comment'    => $summary,
-                               'minor_edit' => $isminor,
-                               'text'       => $serialized,
-                               'len'        => $newsize,
-                               'parent_id'  => $oldid,
-                               'user'       => $user->getId(),
-                               'user_text'  => $user->getName(),
-                               'timestamp'  => $now,
-                               'content_model' => $content->getModel(),
-                               'content_format' => $serialFormat,
-                       ) ); // XXX: pass content object?!
-
-                       $changed = !$content->equals( $old_content );
-
-                       if ( $changed ) {
-                               $prepStatus = $content->prepareSave( $this, $flags, $oldid, $user );
-                               $status->merge( $prepStatus );
-
-                               if ( !$status->isOK() ) {
-                                       return $status;
-                               }
+               // Trigger post-save hook
+               $revision = $status->value['revision']; // new revision
+               $hook_args = array( &$this, &$user, $pstContent, $summary,
+                       $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId );
+               ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $hook_args );
+               Hooks::run( 'PageContentSaveComplete', $hook_args );
 
-                               $dbw->begin( __METHOD__ );
-                               // Get the latest page_latest value while locking it.
-                               // Do a CAS style check to see if it's the same as when this method
-                               // started. If it changed then bail out before touching the DB.
-                               $latestNow = $this->lockAndGetLatest();
-                               if ( $latestNow != $oldid ) {
-                                       $dbw->commit( __METHOD__ );
-                                       // Page updated or deleted in the mean time
-                                       $status->fatal( 'edit-conflict' );
-
-                                       return $status;
-                               }
+               // Promote user to any groups they meet the criteria for
+               DeferredUpdates::addCallableUpdate( function () use ( $user ) {
+                       $user->addAutopromoteOnceGroups( 'onEdit' );
+                       $user->addAutopromoteOnceGroups( 'onView' ); // b/c
+               } );
 
-                               // At this point we are now comitted to returning an OK
-                               // status unless some DB query error or other exception comes up.
-                               // This way callers don't have to call rollback() if $status is bad
-                               // unless they actually try to catch exceptions (which is rare).
+               return $status;
+       }
 
-                               $revisionId = $revision->insertOn( $dbw );
+       /**
+        * @param Content $content Pre-save transform content
+        * @param integer $flags
+        * @param User $user
+        * @param string $summary
+        * @param array $meta
+        * @return Status
+        * @throws DBUnexpectedError
+        * @throws Exception
+        * @throws FatalError
+        * @throws MWException
+        */
+       private function doModify(
+               Content $content, $flags, User $user, $summary, array $meta
+       ) {
+               global $wgUseRCPatrol;
 
-                               // Update page_latest and friends to reflect the new revision
-                               if ( !$this->updateRevisionOn( $dbw, $revision, null, $oldIsRedirect ) ) {
-                                       $dbw->rollback( __METHOD__ );
-                                       throw new MWException( "Failed to update page row to use new revision." );
-                               }
+               // Update article, but only if changed.
+               $status = Status::newGood( array( 'new' => false, 'revision' => null ) );
 
-                               Hooks::run( 'NewRevisionFromEditComplete',
-                                       array( $this, $revision, $baseRevId, $user ) );
+               // Convenience variables
+               $now = wfTimestampNow();
+               $oldid = $meta['oldId'];
+               /** @var $oldContent Content|null */
+               $oldContent = $meta['oldContent'];
+               $newsize = $content->getSize();
 
-                               // Update recentchanges
-                               if ( !( $flags & EDIT_SUPPRESS_RC ) ) {
-                                       // Mark as patrolled if the user can do so
-                                       $patrolled = $wgUseRCPatrol && !count(
-                                               $this->mTitle->getUserPermissionsErrors( 'autopatrol', $user ) );
-                                       // Add RC row to the DB
-                                       RecentChange::notifyEdit(
-                                               $now, $this->mTitle, $isminor, $user, $summary,
-                                               $oldid, $this->getTimestamp(), $bot, '', $oldsize, $newsize,
-                                               $revisionId, $patrolled
-                                       );
-                               }
+               if ( !$oldid ) {
+                       // Article gone missing
+                       $status->fatal( 'edit-gone-missing' );
 
-                               $user->incEditCount();
+                       return $status;
+               } elseif ( !$oldContent ) {
+                       // Sanity check for bug 37225
+                       throw new MWException( "Could not find text for current revision {$oldid}." );
+               }
 
-                               $dbw->commit( __METHOD__ );
-                               $this->mTimestamp = $now;
-                       } else {
-                               // Bug 32948: revision ID must be set to page {{REVISIONID}} and
-                               // related variables correctly
-                               $revision->setId( $this->getLatest() );
-                       }
+               // @TODO: pass content object?!
+               $revision = new Revision( array(
+                       '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'],
+               ) );
 
-                       // Update links tables, site stats, etc.
-                       $this->doEditUpdates(
-                               $revision,
-                               $user,
-                               array(
-                                       'changed' => $changed,
-                                       'oldcountable' => $oldcountable,
-                                       'oldrevision' => $old_revision
-                               )
-                       );
-
-                       if ( !$changed ) {
-                               $status->warning( 'edit-no-change' );
-                               $revision = null;
-                               // Update page_touched, this is usually implicit in the page update
-                               // Other cache updates are done in onArticleEdit()
-                               $this->mTitle->invalidateCache( $now );
-                       }
-               } else {
-                       // Create new article
-                       $status->value['new'] = true;
+               $changed = !$content->equals( $oldContent );
 
+               if ( $changed ) {
                        $prepStatus = $content->prepareSave( $this, $flags, $oldid, $user );
                        $status->merge( $prepStatus );
                        if ( !$status->isOK() ) {
                                return $status;
                        }
 
+                       $dbw = wfGetDB( DB_MASTER );
                        $dbw->begin( __METHOD__ );
+                       // Get the latest page_latest value while locking it.
+                       // Do a CAS style check to see if it's the same as when this method
+                       // started. If it changed then bail out before touching the DB.
+                       $latestNow = $this->lockAndGetLatest();
+                       if ( $latestNow != $oldid ) {
+                               $dbw->commit( __METHOD__ );
+                               // Page updated or deleted in the mean time
+                               $status->fatal( 'edit-conflict' );
 
-                       // Add the page record unless one already exists for the title
-                       $newid = $this->insertOn( $dbw );
-                       if ( $newid === false ) {
-                               $dbw->commit( __METHOD__ ); // nothing inserted
-                               $status->fatal( 'edit-already-exists' );
-
-                               return $status; // nothing done
+                               return $status;
                        }
 
                        // At this point we are now comitted to returning an OK
@@ -1875,46 +1834,37 @@ class WikiPage implements Page, IDBAccessObject {
                        // This way callers don't have to call rollback() if $status is bad
                        // unless they actually try to catch exceptions (which is rare).
 
-                       // Save the revision text...
-                       $revision = new Revision( array(
-                               'page'       => $newid,
-                               'title'      => $this->getTitle(), // for determining the default content model
-                               'comment'    => $summary,
-                               'minor_edit' => $isminor,
-                               'text'       => $serialized,
-                               'len'        => $newsize,
-                               'user'       => $user->getId(),
-                               'user_text'  => $user->getName(),
-                               'timestamp'  => $now,
-                               'content_model' => $content->getModel(),
-                               'content_format' => $serialFormat,
-                       ) );
+                       // Save the revision text
                        $revisionId = $revision->insertOn( $dbw );
-
-                       // Bug 37225: use accessor to get the text as Revision may trim it
-                       $content = $revision->getContent(); // sanity; get normalized version
-
-                       if ( $content ) {
-                               $newsize = $content->getSize();
-                       }
-
-                       // Update the page record with revision data
-                       if ( !$this->updateRevisionOn( $dbw, $revision, 0 ) ) {
+                       // Update page_latest and friends to reflect the new revision
+                       if ( !$this->updateRevisionOn( $dbw, $revision, null, $meta['oldIsRedirect'] ) ) {
                                $dbw->rollback( __METHOD__ );
                                throw new MWException( "Failed to update page row to use new revision." );
                        }
 
-                       Hooks::run( 'NewRevisionFromEditComplete', array( $this, $revision, false, $user ) );
+                       Hooks::run( 'NewRevisionFromEditComplete',
+                               array( $this, $revision, $meta['baseRevId'], $user ) );
 
                        // Update recentchanges
                        if ( !( $flags & EDIT_SUPPRESS_RC ) ) {
                                // Mark as patrolled if the user can do so
-                               $patrolled = ( $wgUseRCPatrol || $wgUseNPPatrol ) && !count(
-                                       $this->mTitle->getUserPermissionsErrors( 'autopatrol', $user ) );
+                               $patrolled = $wgUseRCPatrol && !count(
+                                               $this->mTitle->getUserPermissionsErrors( 'autopatrol', $user ) );
                                // Add RC row to the DB
-                               RecentChange::notifyNew(
-                                       $now, $this->mTitle, $isminor, $user, $summary, $bot,
-                                       '', $newsize, $revisionId, $patrolled
+                               RecentChange::notifyEdit(
+                                       $now,
+                                       $this->mTitle,
+                                       $revision->isMinor(),
+                                       $user,
+                                       $summary,
+                                       $oldid,
+                                       $this->getTimestamp(),
+                                       $meta['bot'],
+                                       '',
+                                       $oldContent ? $oldContent->getSize() : 0,
+                                       $newsize,
+                                       $revisionId,
+                                       $patrolled
                                );
                        }
 
@@ -1922,35 +1872,140 @@ class WikiPage implements Page, IDBAccessObject {
 
                        $dbw->commit( __METHOD__ );
                        $this->mTimestamp = $now;
+               } else {
+                       // Bug 32948: revision ID must be set to page {{REVISIONID}} and
+                       // related variables correctly
+                       $revision->setId( $this->getLatest() );
+               }
+
+               // Update links tables, site stats, etc.
+               $this->doEditUpdates(
+                       $revision,
+                       $user,
+                       array(
+                               'changed' => $changed,
+                               'oldcountable' => $meta['oldCountable'],
+                               'oldrevision' => $meta['oldRevision']
+                       )
+               );
+
+               if ( $changed ) {
+                       // Return the new revision to the caller
+                       $status->value['revision'] = $revision;
+               } else {
+                       $status->warning( 'edit-no-change' );
+                       // Update page_touched as updateRevisionOn() was not called.
+                       // Other cache updates are managed in onArticleEdit() via doEditUpdates().
+                       $this->mTitle->invalidateCache( $now );
+               }
+
+               return $status;
+       }
 
-                       // Update links, etc.
-                       $this->doEditUpdates(
-                               $revision,
+       /**
+        * @param Content $content Pre-save transform content
+        * @param integer $flags
+        * @param User $user
+        * @param string $summary
+        * @param array $meta
+        * @return Status
+        * @throws DBUnexpectedError
+        * @throws Exception
+        * @throws FatalError
+        * @throws MWException
+        */
+       private function doCreate(
+               Content $content, $flags, User $user, $summary, array $meta
+       ) {
+               global $wgUseRCPatrol, $wgUseNPPatrol;
+
+               $status = Status::newGood( array( 'new' => true, 'revision' => null ) );
+
+               $now = wfTimestampNow();
+               $newsize = $content->getSize();
+               $prepStatus = $content->prepareSave( $this, $flags, $meta['oldId'], $user );
+               $status->merge( $prepStatus );
+               if ( !$status->isOK() ) {
+                       return $status;
+               }
+
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->begin( __METHOD__ );
+
+               // Add the page record unless one already exists for the title
+               $newid = $this->insertOn( $dbw );
+               if ( $newid === false ) {
+                       $dbw->commit( __METHOD__ ); // nothing inserted
+                       $status->fatal( 'edit-already-exists' );
+
+                       return $status; // nothing done
+               }
+
+               // At this point we are now comitted to returning an OK
+               // status unless some DB query error or other exception comes up.
+               // This way callers don't have to call rollback() if $status is bad
+               // unless they actually try to catch exceptions (which is rare).
+
+               // @TODO: pass content object?!
+               $revision = new Revision( array(
+                       'page'       => $newid,
+                       'title'      => $this->mTitle, // for determining the default content model
+                       'comment'    => $summary,
+                       'minor_edit' => $meta['minor'],
+                       'text'       => $meta['serialized'],
+                       'len'        => $newsize,
+                       'user'       => $user->getId(),
+                       'user_text'  => $user->getName(),
+                       'timestamp'  => $now,
+                       'content_model' => $content->getModel(),
+                       'content_format' => $meta['serialFormat'],
+               ) );
+
+               // Save the revision text...
+               $revisionId = $revision->insertOn( $dbw );
+               // Update the page record with revision data
+               if ( !$this->updateRevisionOn( $dbw, $revision, 0 ) ) {
+                       $dbw->rollback( __METHOD__ );
+                       throw new MWException( "Failed to update page row to use new revision." );
+               }
+
+               Hooks::run( 'NewRevisionFromEditComplete', array( $this, $revision, false, $user ) );
+
+               // Update recentchanges
+               if ( !( $flags & EDIT_SUPPRESS_RC ) ) {
+                       // Mark as patrolled if the user can do so
+                       $patrolled = ( $wgUseRCPatrol || $wgUseNPPatrol ) &&
+                               !count( $this->mTitle->getUserPermissionsErrors( 'autopatrol', $user ) );
+                       // Add RC row to the DB
+                       RecentChange::notifyNew(
+                               $now,
+                               $this->mTitle,
+                               $revision->isMinor(),
                                $user,
-                               array( 'created' => true, 'oldrevision' => $old_revision )
+                               $summary,
+                               $meta['bot'],
+                               '',
+                               $newsize,
+                               $revisionId,
+                               $patrolled
                        );
+               }
 
-                       $hook_args = array( &$this, &$user, $content, $summary,
-                                                               $flags & EDIT_MINOR, null, null, &$flags, $revision );
+               $user->incEditCount();
 
-                       ContentHandler::runLegacyHooks( 'ArticleInsertComplete', $hook_args );
-                       Hooks::run( 'PageContentInsertComplete', $hook_args );
-               }
+               $dbw->commit( __METHOD__ );
+               $this->mTimestamp = $now;
 
-               // Return the new revision (or null) to the caller
-               $status->value['revision'] = $revision;
+               // Update links, etc.
+               $this->doEditUpdates( $revision, $user, array( 'created' => true ) );
 
                $hook_args = array( &$this, &$user, $content, $summary,
-                       $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $baseRevId );
-
-               ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $hook_args );
-               Hooks::run( 'PageContentSaveComplete', $hook_args );
+                       $flags & EDIT_MINOR, null, null, &$flags, $revision );
+               ContentHandler::runLegacyHooks( 'ArticleInsertComplete', $hook_args );
+               Hooks::run( 'PageContentInsertComplete', $hook_args );
 
-               // Promote user to any groups they meet the criteria for
-               DeferredUpdates::addCallableUpdate( function () use ( $user ) {
-                       $user->addAutopromoteOnceGroups( 'onEdit' );
-                       $user->addAutopromoteOnceGroups( 'onView' ); // b/c
-               } );
+               // Return the new revision to the caller
+               $status->value['revision'] = $revision;
 
                return $status;
        }