Merge "Some fixes to page updater docs"
[lhc/web/wiklou.git] / includes / Storage / DerivedPageDataUpdater.php
index ff5541d..2d53682 100644 (file)
@@ -48,9 +48,13 @@ use MediaWiki\Revision\SlotRoleRegistry;
 use MediaWiki\Revision\SlotRecord;
 use MediaWiki\User\UserIdentity;
 use MessageCache;
+use MWCallableUpdate;
 use ParserCache;
 use ParserOptions;
 use ParserOutput;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 use RecentChangesUpdateJob;
 use ResourceLoaderWikiModule;
 use Revision;
@@ -59,7 +63,7 @@ use SiteStatsUpdate;
 use Title;
 use User;
 use Wikimedia\Assert\Assert;
-use Wikimedia\Rdbms\LBFactory;
+use Wikimedia\Rdbms\ILBFactory;
 use WikiPage;
 
 /**
@@ -75,7 +79,7 @@ use WikiPage;
  *
  * DerivedPageDataUpdater instances are designed to be cached inside a WikiPage instance,
  * and re-used by callback code over the course of an update operation. It's a stepping stone
- * one the way to a more complete refactoring of WikiPage.
+ * on the way to a more complete refactoring of WikiPage.
  *
  * When using a DerivedPageDataUpdater, the following life cycle must be observed:
  * grabCurrentRevision (optional), prepareContent (optional), prepareUpdate (required
@@ -93,7 +97,7 @@ use WikiPage;
  * @since 1.32
  * @ingroup Page
  */
-class DerivedPageDataUpdater implements IDBAccessObject {
+class DerivedPageDataUpdater implements IDBAccessObject, LoggerAwareInterface {
 
        /**
         * @var UserIdentity|null
@@ -131,10 +135,15 @@ class DerivedPageDataUpdater implements IDBAccessObject {
        private $messageCache;
 
        /**
-        * @var LBFactory
+        * @var ILBFactory
         */
        private $loadbalancerFactory;
 
+       /**
+        * @var LoggerInterface
+        */
+       private $logger;
+
        /**
         * @var string see $wgArticleCountMethod
         */
@@ -267,7 +276,7 @@ class DerivedPageDataUpdater implements IDBAccessObject {
         * @param JobQueueGroup $jobQueueGroup
         * @param MessageCache $messageCache
         * @param Language $contLang
-        * @param LBFactory $loadbalancerFactory
+        * @param ILBFactory $loadbalancerFactory
         */
        public function __construct(
                WikiPage $wikiPage,
@@ -278,7 +287,7 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                JobQueueGroup $jobQueueGroup,
                MessageCache $messageCache,
                Language $contLang,
-               LBFactory $loadbalancerFactory
+               ILBFactory $loadbalancerFactory
        ) {
                $this->wikiPage = $wikiPage;
 
@@ -292,6 +301,11 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                // XXX only needed for waiting for replicas to catch up; there should be a narrower
                // interface for that.
                $this->loadbalancerFactory = $loadbalancerFactory;
+               $this->logger = new NullLogger();
+       }
+
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
        }
 
        /**
@@ -566,7 +580,6 @@ class DerivedPageDataUpdater implements IDBAccessObject {
         */
        public function isContentDeleted() {
                if ( $this->revision ) {
-                       // XXX: if that revision is the current revision, this should be skipped
                        return $this->revision->isDeleted( RevisionRecord::DELETED_TEXT );
                } else {
                        // If the content has not been saved yet, it cannot have been deleted yet.
@@ -849,11 +862,12 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                if ( $stashedEdit ) {
                        /** @var ParserOutput $output */
                        $output = $stashedEdit->output;
-
                        // TODO: this should happen when stashing the ParserOutput, not now!
                        $output->setCacheTime( $stashedEdit->timestamp );
 
                        $renderHints['known-revision-output'] = $output;
+
+                       $this->logger->debug( __METHOD__ . ': using stashed edit output...' );
                }
 
                // NOTE: we want a canonical rendering, so don't pass $this->user or ParserOptions
@@ -1067,6 +1081,11 @@ class DerivedPageDataUpdater implements IDBAccessObject {
         *    See DataUpdate::getCauseAction(). (default 'unknown')
         *  - causeAgent: name of the user who caused the update. See DataUpdate::getCauseAgent().
         *    (string, default 'unknown')
+        *  - known-revision-output: a combined canonical ParserOutput for the revision, perhaps
+        *    from some cache. The caller is responsible for ensuring that the ParserOutput indeed
+        *    matched the $rev and $options. This mechanism is intended as a temporary stop-gap,
+        *    for the time until caches have been changed to store RenderedRevision states instead
+        *    of ParserOutput objects. (default: null) (since 1.33)
         */
        public function prepareUpdate( RevisionRecord $revision, array $options = [] ) {
                Assert::parameter(
@@ -1213,14 +1232,17 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                if ( $this->renderedRevision ) {
                        $this->renderedRevision->updateRevision( $revision );
                } else {
-
                        // NOTE: we want a canonical rendering, so don't pass $this->user or ParserOptions
                        // NOTE: the revision is either new or current, so we can bypass audience checks.
                        $this->renderedRevision = $this->revisionRenderer->getRenderedRevision(
                                $this->revision,
                                null,
                                null,
-                               [ 'use-master' => $this->useMaster(), 'audience' => RevisionRecord::RAW ]
+                               [
+                                       'use-master' => $this->useMaster(),
+                                       'audience' => RevisionRecord::RAW,
+                                       'known-revision-output' => $options['known-revision-output'] ?? null
+                               ]
                        );
 
                        // XXX: Since we presumably are dealing with the current revision,
@@ -1423,12 +1445,18 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                }
 
                // Defer the getCannonicalParserOutput() call triggered by getSecondaryDataUpdates()
-               DeferredUpdates::addCallableUpdate( function () {
-                       $this->doSecondaryDataUpdates( [
-                               // T52785 do not update any other pages on a null edit
-                               'recursive' => $this->options['changed']
-                       ] );
-               } );
+               // by wrapping the code that schedules the secondary updates in a callback itself
+               $wrapperUpdate = new MWCallableUpdate(
+                       function () {
+                               $this->doSecondaryDataUpdates( [
+                                       // T52785 do not update any other pages on a null edit
+                                       'recursive' => $this->options['changed']
+                               ] );
+                       },
+                       __METHOD__
+               );
+               $wrapperUpdate->setTransactionRoundRequirement( $wrapperUpdate::TRX_ROUND_ABSENT );
+               DeferredUpdates::addUpdate( $wrapperUpdate );
 
                // TODO: MCR: check if *any* changed slot supports categories!
                if ( $this->rcWatchCategoryMembership
@@ -1569,24 +1597,15 @@ class DerivedPageDataUpdater implements IDBAccessObject {
         *     current page or otherwise depend on it (default: false)
         *   - defer: one of the DeferredUpdates constants, or false to run immediately after waiting
         *     for replication of the changes from the SecondaryDataUpdates hooks (default: false)
-        *   - transactionTicket: a transaction ticket from LBFactory::getEmptyTransactionTicket(),
-        *     only when defer is false (default: null)
         * @since 1.32
         */
        public function doSecondaryDataUpdates( array $options = [] ) {
                $this->assertHasRevision( __METHOD__ );
-               $options += [
-                       'recursive' => false,
-                       'defer' => false,
-                       'transactionTicket' => null,
-               ];
+               $options += [ 'recursive' => false, 'defer' => false ];
                $deferValues = [ false, DeferredUpdates::PRESEND, DeferredUpdates::POSTSEND ];
                if ( !in_array( $options['defer'], $deferValues, true ) ) {
                        throw new InvalidArgumentException( 'Invalid value for defer: ' . $options['defer'] );
                }
-               Assert::parameterType(
-                       'integer|null', $options['transactionTicket'], '$options[\'transactionTicket\']' );
-
                $updates = $this->getSecondaryDataUpdates( $options['recursive'] );
 
                $triggeringUser = $this->options['triggeringUser'] ?? $this->user;
@@ -1596,14 +1615,6 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                $causeAction = $this->options['causeAction'] ?? 'unknown';
                $causeAgent = $this->options['causeAgent'] ?? 'unknown';
                $legacyRevision = new Revision( $this->revision );
-               $ticket = $options['transactionTicket'];
-
-               if ( $options['defer'] === false && $ticket !== null ) {
-                       // For legacy hook handlers doing updates via LinksUpdateConstructed, make sure
-                       // any pending writes they made get flushed before the doUpdate() calls below.
-                       // This avoids snapshot-clearing errors in LinksUpdate::acquirePageLock().
-                       $this->loadbalancerFactory->commitAndWaitForReplication( __METHOD__, $ticket );
-               }
 
                foreach ( $updates as $update ) {
                        if ( $update instanceof DataUpdate ) {
@@ -1613,13 +1624,16 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                                $update->setRevision( $legacyRevision );
                                $update->setTriggeringUser( $triggeringUser );
                        }
+               }
 
-                       if ( $options['defer'] === false ) {
-                               if ( $update instanceof DataUpdate && $ticket !== null ) {
-                                       $update->setTransactionTicket( $ticket );
-                               }
-                               $update->doUpdate();
-                       } else {
+               if ( $options['defer'] === false ) {
+                       // T221577: flush any transaction; each update needs outer transaction scope
+                       $this->loadbalancerFactory->commitMasterChanges( __METHOD__ );
+                       foreach ( $updates as $update ) {
+                               DeferredUpdates::attemptUpdate( $update, $this->loadbalancerFactory );
+                       }
+               } else {
+                       foreach ( $updates as $update ) {
                                DeferredUpdates::addUpdate( $update, $options['defer'] );
                        }
                }