Add ParserOutput::{get,set}RenderId() and set render id in ContentRenderer

Set the render ID for each parse stored into cache so that we are able
to identify a specific parse when there are dependencies (for example
in an edit based on that parse).  This is recorded as a property added
to the ParserOutput, not the parent CacheTime interface.  Even though
the render ID is /related/ to the CacheTime interface, CacheTime is
also used directly as a parser cache key, and the UUID should not be
part of the lookup key.

In general we are trying to move the location where these cache
properties are set as early as possible, so we check at each location
to ensure we don't overwrite a previously-set value.  Eventually we
can convert most of these checks into assertions that the cache
properties have already been set (T350538).  The primary location for
setting cache properties is the ContentRenderer.

Moved setting the revision timestamp into ContentRenderer as well, as
it was set along the same code paths.  An extra parameter was added to
ContentRenderer::getParserOutput() to support this.

Added merge code to ParserOutput::mergeInternalMetaDataFrom() which
should ensure that cache time, revision, timestamp, and render id are
all set properly when multiple slots are combined together in MCR.

In order to ensure the render ID is set on all codepaths we needed to
plumb the GlobalIdGenerator service into ContentRenderer, ParserCache,
ParserCacheFactory, and RevisionOutputCache.  Eventually (T350538) it
should only be necessary in the ContentRenderer.

Bug: T350538
Bug: T349868
Followup-To: Ic9b7cc0fcf365e772b7d080d76a065e3fd585f80
Change-Id: I72c5e6f86b7f081ab5ce7a56f5365d2f75067a78

RELEASE-NOTES-1.42

@@ -500,6 +500,9 @@ because of Phabricator reports.
   for new projects.
 * Title::getBrokenLinksFrom() has been deprecated.
 * ReplicatedBagOStuff has been deprecated since 1.42.
+* The third argument to ContentRenderer::getParserOutput() now accepts a
+  RevisionRecord or WikiRevision; passing an integer revision id has been
+  deprecated and emits a warning.
 * ParserOutput::setLanguageLinks() has been deprecated.
 * ApiQueryBlockInfoTrait::addBlockInfoToQuery() will emit deprecation warnings
   and will soon stop working due to schema changes. Instead use

includes/Rest/Handler/Helper/HtmlInputTransformHelper.php

@@ -635,7 +635,7 @@ class HtmlInputTransformHelper {
 					return null;
 				}
 
-				$cachedRenderID = $this->parsoidOutputAccess->getParsoidRenderID( $parserOutput );
+				$cachedRenderID = ParsoidRenderID::newFromParserOutput( $parserOutput );
 				if ( $cachedRenderID->getKey() !== $renderID->getKey() ) {
 					$this->stats->increment( 'html_input_transform.original_html.given.as_renderid.' .
 						'stash_miss_pc_fallback.not_found.mismatch' );

includes/Rest/Handler/Helper/HtmlOutputRendererHelper.php

@@ -436,9 +436,7 @@ class HtmlOutputRendererHelper implements HtmlOutputHelper {
 			$this->authorizeWriteOrThrow( $this->authority, 'stashbasehtml', $this->page );
 
 			$isFakeRevision = $this->getRevisionId() === null;
-			$parsoidStashKey = ParsoidRenderID::newFromKey(
-				$this->parsoidOutputAccess->getParsoidRenderID( $parserOutput )
-			);
+			$parsoidStashKey = ParsoidRenderID::newFromParserOutput( $parserOutput );
 			$stashSuccess = $this->parsoidOutputStash->set(
 				$parsoidStashKey,
 				new SelserContext(
@@ -489,7 +487,7 @@ class HtmlOutputRendererHelper implements HtmlOutputHelper {
 	public function getETag( string $suffix = '' ): ?string {
 		$parserOutput = $this->getParserOutput();
 
-		$renderID = $this->parsoidOutputAccess->getParsoidRenderID( $parserOutput )->getKey();
+		$renderID = ParsoidRenderID::newFromParserOutput( $parserOutput )->getKey();
 
 		if ( $suffix !== '' ) {
 			$eTag = "$renderID/{$this->flavor}/$suffix";

includes/Revision/RenderedRevision.php

@@ -250,23 +250,19 @@ class RenderedRevision implements SlotRenderingProvider {
 	}
 
 	/**
-	 * @note This method exist to make duplicate parses easier to see during profiling
+	 * @note This method exists to make duplicate parses easier to see during profiling
 	 * @param Content $content
 	 * @param bool $withHtml
 	 * @return ParserOutput
 	 */
 	private function getSlotParserOutputUncached( Content $content, $withHtml ) {
-		$parserOutput = $this->contentRenderer->getParserOutput(
+		return $this->contentRenderer->getParserOutput(
 			$content,
 			$this->revision->getPage(),
-			$this->revision->getId(),
+			$this->revision,
 			$this->options,
 			$withHtml
 		);
-		// Save the rev_id and timestamp so that we don't have to load the revision row on view
-		$parserOutput->setCacheRevisionId( $this->revision->getId() );
-		$parserOutput->setTimestamp( $this->revision->getTimestamp() );
-		return $parserOutput;
 	}
 
 	/**

includes/ServiceWiring.php

@@ -614,7 +614,10 @@ return [
 	},
 
 	'ContentRenderer' => static function ( MediaWikiServices $services ): ContentRenderer {
-		return new ContentRenderer( $services->getContentHandlerFactory() );
+		return new ContentRenderer(
+			$services->getContentHandlerFactory(),
+			$services->getGlobalIdGenerator()
+		);
 	},
 
 	'ContentTransformer' => static function ( MediaWikiServices $services ): ContentTransformer {
@@ -1536,7 +1539,8 @@ return [
 			LoggerFactory::getInstance( 'ParserCache' ),
 			$options,
 			$services->getTitleFactory(),
-			$services->getWikiPageFactory()
+			$services->getWikiPageFactory(),
+			$services->getGlobalIdGenerator()
 		);
 	},
 

includes/api/ApiParse.php

@@ -149,13 +149,15 @@ class ApiParse extends ApiBase {
 	private function getContentParserOutput(
 		Content $content,
 		PageReference $page,
-		$revId,
+		?RevisionRecord $revision,
 		ParserOptions $popts
 	) {
 		$worker = new PoolCounterWorkViaCallback( 'ApiParser', $this->getPoolKey(),
 			[
-				'doWork' => function () use ( $content, $page, $revId, $popts ) {
-					return $this->contentRenderer->getParserOutput( $content, $page, $revId, $popts );
+				'doWork' => function () use ( $content, $page, $revision, $popts ) {
+					return $this->contentRenderer->getParserOutput(
+						$content, $page, $revision, $popts
+					);
 				},
 				'error' => function () {
 					$this->dieWithError( 'apierror-concurrency-limit' );
@@ -328,6 +330,7 @@ class ApiParse extends ApiBase {
 				$this->dieWithError( [ 'apierror-invalidtitle', wfEscapeWikiText( $title ) ] );
 			}
 			$revid = $params['revid'];
+			$rev = null;
 			if ( $revid !== null ) {
 				$rev = $this->revisionLookup->getRevisionById( $revid );
 				if ( !$rev ) {
@@ -436,9 +439,9 @@ class ApiParse extends ApiBase {
 
 			// Not cached (save or load)
 			if ( $params['pst'] ) {
-				$p_result = $this->getContentParserOutput( $this->pstContent, $titleObj, $revid, $popts );
+				$p_result = $this->getContentParserOutput( $this->pstContent, $titleObj, $rev, $popts );
 			} else {
-				$p_result = $this->getContentParserOutput( $this->content, $titleObj, $revid, $popts );
+				$p_result = $this->getContentParserOutput( $this->content, $titleObj, $rev, $popts );
 			}
 		}
 
@@ -822,13 +825,21 @@ class ApiParse extends ApiBase {
 				$this->content,
 				$pageId === null ? $page->getTitle()->getPrefixedText() : $this->msg( 'pageid', $pageId )
 			);
-			return $this->getContentParserOutput( $this->content, $page->getTitle(), $revId, $popts );
+			return $this->getContentParserOutput(
+				$this->content, $page->getTitle(),
+				$rev,
+				$popts
+			);
 		}
 
 		if ( $isDeleted ) {
 			// getParserOutput can't do revdeled revisions
 
-			$pout = $this->getContentParserOutput( $this->content, $page->getTitle(), $revId, $popts );
+			$pout = $this->getContentParserOutput(
+				$this->content, $page->getTitle(),
+				$rev,
+				$popts
+			);
 		} else {
 			// getParserOutput will save to Parser cache if able
 			$pout = $this->getPageParserOutput( $page, $revId, $popts, $suppressCache );

includes/api/ApiQueryRevisionsBase.php

@@ -669,7 +669,7 @@ abstract class ApiQueryRevisionsBase extends ApiQueryGeneratorBase {
 				$po = $this->contentRenderer->getParserOutput(
 					$content,
 					$title,
-					$revision->getId(),
+					$revision,
 					ParserOptions::newFromContext( $this->getContext() )
 				);
 				$text = $po->getText();

includes/content/Renderer/ContentRenderer.php

@@ -5,7 +5,9 @@ use Content;
 use MediaWiki\Content\IContentHandlerFactory;
 use MediaWiki\Page\PageReference;
 use MediaWiki\Parser\ParserOutput;
+use MediaWiki\Revision\RevisionRecord;
 use ParserOptions;
+use Wikimedia\UUID\GlobalIdGenerator;
 
 /**
  * A service to render content.
@@ -16,11 +18,18 @@ class ContentRenderer {
 	/** @var IContentHandlerFactory */
 	private $contentHandlerFactory;
 
+	private GlobalIdGenerator $globalIdGenerator;
+
 	/**
 	 * @param IContentHandlerFactory $contentHandlerFactory
+	 * @param GlobalIdGenerator $globalIdGenerator
 	 */
-	public function __construct( IContentHandlerFactory $contentHandlerFactory ) {
+	public function __construct(
+		IContentHandlerFactory $contentHandlerFactory,
+		GlobalIdGenerator $globalIdGenerator
+	) {
 		$this->contentHandlerFactory = $contentHandlerFactory;
+		$this->globalIdGenerator = $globalIdGenerator;
 	}
 
 	/**
@@ -28,22 +37,58 @@ class ContentRenderer {
 	 *
 	 * @param Content $content
 	 * @param PageReference $page
-	 * @param int|null $revId
+	 * @param RevisionRecord|int|null $revision
 	 * @param ParserOptions|null $parserOptions
 	 * @param bool $generateHtml
 	 *
 	 * @return ParserOutput
+	 * @note Passing an integer as $rev was deprecated in MW 1.42
 	 */
 	public function getParserOutput(
 		Content $content,
 		PageReference $page,
-		?int $revId = null,
+		$revision = null,
 		?ParserOptions $parserOptions = null,
 		bool $generateHtml = true
 	): ParserOutput {
+		$revId = null;
+		$revTimestamp = null;
+		if ( is_int( $revision ) ) {
+			wfDeprecated( __METHOD__ . ' with integer revision id', '1.42' );
+			$revId = $revision;
+		} elseif ( $revision !== null ) {
+			$revId = $revision->getId();
+			$revTimestamp = $revision->getTimestamp();
+		}
+		$cacheTime = wfTimestampNow();
 		$contentHandler = $this->contentHandlerFactory->getContentHandler( $content->getModel() );
 		$cpoParams = new ContentParseParams( $page, $revId, $parserOptions, $generateHtml );
 
-		return $contentHandler->getParserOutput( $content, $cpoParams );
+		$parserOutput = $contentHandler->getParserOutput( $content, $cpoParams );
+		// Set the cache parameters, if not previously set.
+		//
+		// It is expected that this will be where most are set for the first
+		// time, but a ContentHandler can (for example) use a content-based
+		// hash for the render id by setting it inside
+		// ContentHandler::getParserOutput(); any such custom render id
+		// will not be overwritten here.  Similarly, a ContentHandler can
+		// continue to use the semi-documented feature of ::setCacheTime(-1)
+		// to indicate "not cacheable", and that will not be overwritten
+		// either.
+		if ( !$parserOutput->hasCacheTime() ) {
+			$parserOutput->setCacheTime( $cacheTime );
+		}
+		if ( $parserOutput->getRenderId() === null ) {
+			$parserOutput->setRenderId( $this->globalIdGenerator->newUUIDv1() );
+		}
+		// Revision ID and Revision Timestamp are set here so that we don't
+		// have to load the revision row on view.
+		if ( $parserOutput->getCacheRevisionId() === null && $revId !== null ) {
+			$parserOutput->setCacheRevisionId( $revId );
+		}
+		if ( $parserOutput->getTimestamp() === null && $revTimestamp !== null ) {
+			$parserOutput->setTimestamp( $revTimestamp );
+		}
+		return $parserOutput;
 	}
 }

includes/parser/CacheTime.php

@@ -73,6 +73,13 @@ class CacheTime implements ParserCacheMetadata, JsonUnserializable {
 		return $this->mCacheTime;
 	}
 
+	/**
+	 * @return bool true if a cache time has been set
+	 */
+	public function hasCacheTime(): bool {
+		return $this->mCacheTime !== '';
+	}
+
 	/**
 	 * setCacheTime() sets the timestamp expressing when the page has been rendered.
 	 * This does not control expiry, see updateCacheExpiry() for that!

includes/parser/ParserCache.php

@@ -31,6 +31,7 @@ use MediaWiki\Parser\ParserCacheMetadata;
 use MediaWiki\Parser\ParserOutput;
 use MediaWiki\Title\TitleFactory;
 use Psr\Log\LoggerInterface;
+use Wikimedia\UUID\GlobalIdGenerator;
 
 /**
  * Cache for ParserOutput objects corresponding to the latest page revisions.
@@ -118,6 +119,8 @@ class ParserCache {
 
 	private ?ParserCacheFilter $filter = null;
 
+	private GlobalIdGenerator $globalIdGenerator;
+
 	/**
 	 * @var BagOStuff small in-process cache to store metadata.
 	 * It's needed multiple times during the request, for example
@@ -141,6 +144,7 @@ class ParserCache {
 	 * @param LoggerInterface $logger
 	 * @param TitleFactory $titleFactory
 	 * @param WikiPageFactory $wikiPageFactory
+	 * @param GlobalIdGenerator $globalIdGenerator
 	 */
 	public function __construct(
 		string $name,
@@ -151,7 +155,8 @@ class ParserCache {
 		IBufferingStatsdDataFactory $stats,
 		LoggerInterface $logger,
 		TitleFactory $titleFactory,
-		WikiPageFactory $wikiPageFactory
+		WikiPageFactory $wikiPageFactory,
+		GlobalIdGenerator $globalIdGenerator
 	) {
 		$this->name = $name;
 		$this->cache = $cache;
@@ -162,6 +167,7 @@ class ParserCache {
 		$this->logger = $logger;
 		$this->titleFactory = $titleFactory;
 		$this->wikiPageFactory = $wikiPageFactory;
+		$this->globalIdGenerator = $globalIdGenerator;
 		$this->metadataProcCache = new HashBagOStuff( [ 'maxKeys' => 2 ] );
 	}
 
@@ -410,6 +416,9 @@ class ParserCache {
 		$revId = null
 	) {
 		$page->assertWiki( PageRecord::LOCAL );
+		// T350538: Eventually we'll warn if the $cacheTime and $revId
+		// parameters are non-null here, since we *should* be getting
+		// them from the ParserOutput.
 
 		if ( !$parserOutput->hasText() ) {
 			throw new InvalidArgumentException( 'Attempt to cache a ParserOutput with no text set!' );
@@ -450,9 +459,23 @@ class ParserCache {
 			return;
 		}
 
-		$cacheTime = $cacheTime ?: wfTimestampNow();
-		$revId = $revId ?: $page->getLatest( PageRecord::LOCAL );
+		// Ensure cache properties are set in the ParserOutput
+		// T350538: These should be turned into assertions that the
+		// properties are already present.
+		if ( $cacheTime ) {
+			$parserOutput->setCacheTime( $cacheTime );
+		} else {
+			$cacheTime = $parserOutput->getCacheTime();
+		}
 
+		if ( $revId ) {
+			$parserOutput->setCacheRevisionId( $revId );
+		} elseif ( $parserOutput->getCacheRevisionId() ) {
+			$revId = $parserOutput->getCacheRevisionId();
+		} else {
+			$revId = $page->getLatest( PageRecord::LOCAL );
+			$parserOutput->setCacheRevisionId( $revId );
+		}
 		if ( !$revId ) {
 			$this->logger->debug(
 				'Parser output cannot be saved if the revision ID is not known',
@@ -462,14 +485,16 @@ class ParserCache {
 			return;
 		}
 
+		if ( !$parserOutput->getRenderId() ) {
+			$parserOutput->setRenderId( $this->globalIdGenerator->newUUIDv1() );
+		}
+
+		// Transfer cache properties to the cache metadata
 		$metadata = new CacheTime;
 		$metadata->recordOptions( $parserOutput->getUsedOptions() );
 		$metadata->updateCacheExpiry( $expire );
-
 		$metadata->setCacheTime( $cacheTime );
-		$parserOutput->setCacheTime( $cacheTime );
 		$metadata->setCacheRevisionId( $revId );
-		$parserOutput->setCacheRevisionId( $revId );
 
 		$parserOutputKey = $this->makeParserOutputKey(
 			$page,

includes/parser/ParserCacheFactory.php

@@ -32,6 +32,7 @@ use MediaWiki\Title\TitleFactory;
 use ParserCache;
 use Psr\Log\LoggerInterface;
 use WANObjectCache;
+use Wikimedia\UUID\GlobalIdGenerator;
 
 /**
  * Returns an instance of the ParserCache by its name.
@@ -70,6 +71,8 @@ class ParserCacheFactory {
 	/** @var WikiPageFactory */
 	private $wikiPageFactory;
 
+	private GlobalIdGenerator $globalIdGenerator;
+
 	/** @var ParserCache[] */
 	private $parserCaches = [];
 
@@ -98,6 +101,7 @@ class ParserCacheFactory {
 	 * @param ServiceOptions $options
 	 * @param TitleFactory $titleFactory
 	 * @param WikiPageFactory $wikiPageFactory
+	 * @param GlobalIdGenerator $globalIdGenerator
 	 */
 	public function __construct(
 		BagOStuff $parserCacheBackend,
@@ -108,7 +112,8 @@ class ParserCacheFactory {
 		LoggerInterface $logger,
 		ServiceOptions $options,
 		TitleFactory $titleFactory,
-		WikiPageFactory $wikiPageFactory
+		WikiPageFactory $wikiPageFactory,
+		GlobalIdGenerator $globalIdGenerator
 	) {
 		$this->parserCacheBackend = $parserCacheBackend;
 		$this->revisionOutputCacheBackend = $revisionOutputCacheBackend;
@@ -121,6 +126,7 @@ class ParserCacheFactory {
 		$this->options = $options;
 		$this->titleFactory = $titleFactory;
 		$this->wikiPageFactory = $wikiPageFactory;
+		$this->globalIdGenerator = $globalIdGenerator;
 	}
 
 	/**
@@ -140,7 +146,8 @@ class ParserCacheFactory {
 				$this->stats,
 				$this->logger,
 				$this->titleFactory,
-				$this->wikiPageFactory
+				$this->wikiPageFactory,
+				$this->globalIdGenerator
 			);
 
 			$filterConfig = $this->options->get( MainConfigNames::ParserCacheFilterConfig );
@@ -170,7 +177,8 @@ class ParserCacheFactory {
 				$this->options->get( MainConfigNames::CacheEpoch ),
 				$this->jsonCodec,
 				$this->stats,
-				$this->logger
+				$this->logger,
+				$this->globalIdGenerator
 			);
 
 			$this->revisionOutputCaches[$name] = $cache;

includes/parser/ParserOutput.php

@@ -94,12 +94,6 @@ class ParserOutput extends CacheTime implements ContentMetadataCollector {
 	 */
 	public const MW_MERGE_STRATEGY_UNION = 'union';
 
-	/**
-	 * @internal
-	 * String Key used to store the parsoid render ID in ParserOutput
-	 */
-	public const PARSOID_RENDER_ID_KEY = 'parsoid-render-id';
-
 	/**
 	 * @var string|null The output text
 	 */
@@ -1359,27 +1353,32 @@ class ParserOutput extends CacheTime implements ContentMetadataCollector {
 	}
 
 	/**
+	 * Store a unique rendering id for this ParserOutput.  This is used
+	 * whenever a client needs to record a dependency on a specific parse.
+	 * It is typically set only when a parser output is cached.
+	 *
+	 * @param string $renderId a UUID identifying a specific parse
 	 * @internal
-	 *
-	 * Store a unique rendering id for this output. This is used by the REST API
-	 * for stashing content to support editing use cases.
-	 *
-	 * @param ParsoidRenderID $parsoidRenderId
 	 */
-	public function setParsoidRenderId( ParsoidRenderID $parsoidRenderId ): void {
-		$this->setExtensionData( self::PARSOID_RENDER_ID_KEY, $parsoidRenderId->getKey() );
+	public function setRenderId( string $renderId ): void {
+		$this->setExtensionData( 'core:render-id', $renderId );
 	}
 
 	/**
-	 * @internal
-	 *
-	 * Return the Parsoid rendering id for this ParserOutput. This is only set
-	 * where the ParserOutput has been generated by Parsoid.
+	 * Return the unique rendering id for this ParserOutput. This is used
+	 * whenever a client needs to record a dependency on a specific parse.
 	 *
 	 * @return string|null
+	 * @internal
 	 */
-	public function getParsoidRenderId(): ?string {
-		return $this->getExtensionData( self::PARSOID_RENDER_ID_KEY );
+	public function getRenderId(): ?string {
+		// Backward-compatibility with old cache contents
+		// Can be removed after parser cache contents have expired
+		$old = $this->getExtensionData( 'parsoid-render-id' );
+		if ( $old !== null ) {
+			return ParsoidRenderId::newFromKey( $old )->getUniqueID();
+		}
+		return $this->getExtensionData( 'core:render-id' );
 	}
 
 	/**
@@ -2074,6 +2073,40 @@ class ParserOutput extends CacheTime implements ContentMetadataCollector {
 	public function mergeInternalMetaDataFrom( ParserOutput $source ): void {
 		$this->mWarnings = self::mergeMap( $this->mWarnings, $source->mWarnings ); // don't use getter
 		$this->mTimestamp = $this->useMaxValue( $this->mTimestamp, $source->getTimestamp() );
+		if ( $source->hasCacheTime() ) {
+			$sourceCacheTime = $source->getCacheTime();
+			if (
+				!$this->hasCacheTime() ||
+				// "undocumented use of -1 to mean not cacheable"
+				// deprecated, but still supported by ::setCacheTime()
+				strval( $sourceCacheTime ) === '-1' ||
+				(
+					strval( $this->getCacheTime() ) !== '-1' &&
+					// use newer of the two times
+					$this->getCacheTime() < $sourceCacheTime
+				)
+			) {
+				$this->setCacheTime( $sourceCacheTime );
+			}
+		}
+		if ( $source->getRenderId() !== null ) {
+			// Final render ID should be a function of all component POs
+			$rid = ( $this->getRenderId() ?? '' ) . $source->getRenderId();
+			$this->setRenderId( $rid );
+		}
+		if ( $source->getCacheRevisionId() !== null ) {
+			$sourceCacheRevisionId = $source->getCacheRevisionId();
+			$thisCacheRevisionId = $this->getCacheRevisionId();
+			if ( $thisCacheRevisionId === null ) {
+				$this->setCacheRevisionId( $sourceCacheRevisionId );
+			} elseif ( $sourceCacheRevisionId !== $thisCacheRevisionId ) {
+				// May throw an exception here in the future
+				wfDeprecated(
+					__METHOD__ . ": conflicting revision IDs " .
+					"$thisCacheRevisionId and $sourceCacheRevisionId"
+				);
+			}
+		}
 
 		foreach ( self::SPECULATIVE_FIELDS as $field ) {
 			if ( $this->$field && $source->$field && $this->$field !== $source->$field ) {

includes/parser/Parsoid/ParsoidOutputAccess.php

@@ -19,7 +19,6 @@
 
 namespace MediaWiki\Parser\Parsoid;
 
-use InvalidArgumentException;
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Content\IContentHandlerFactory;
 use MediaWiki\MainConfigNames;
@@ -125,24 +124,6 @@ class ParsoidOutputAccess {
 		return $this->siteConfig->getContentModelHandler( $model ) !== null;
 	}
 
-	/**
-	 * @internal
-	 *
-	 * @param ParserOutput $parserOutput
-	 *
-	 * @return ParsoidRenderID
-	 */
-	public function getParsoidRenderID( ParserOutput $parserOutput ): ParsoidRenderID {
-		// XXX: ParserOutput may be coming from the parser cache, so we need to be careful
-		// when we change how we store the render key in the ParserOutput object.
-		$renderId = $parserOutput->getParsoidRenderId();
-		if ( !$renderId ) {
-			throw new InvalidArgumentException( 'ParserOutput does not have a render ID' );
-		}
-
-		return ParsoidRenderID::newFromKey( $renderId );
-	}
-
 	private function handleUnsupportedContentModel( RevisionRecord $revision ): ?Status {
 		$mainSlot = $revision->getSlot( SlotRecord::MAIN );
 		$contentModel = $mainSlot->getModel();
@@ -238,7 +219,11 @@ class ParsoidOutputAccess {
 		// This is fast to generate so it's fine not to write this to parser cache.
 		$output->updateCacheExpiry( 0 );
 		// The render ID is required for rendering of dummy output: T311728.
-		$output->setExtensionData( ParserOutput::PARSOID_RENDER_ID_KEY, '0/dummy-output' );
+		$ts = wfTimestampNow();
+		$output->setCacheTime( $ts );
+		$output->setRenderId( 'dummy-output' );
+		$output->setCacheRevisionId( 0 );
+		$output->setTimestamp( $ts );
 		// Required in HtmlOutputRendererHelper::putHeaders when $forHtml
 		$output->setExtensionData(
 			PageBundleParserOutputConverter::PARSOID_PAGE_BUNDLE_KEY,

includes/parser/Parsoid/ParsoidParser.php

@@ -64,17 +64,14 @@ class ParsoidParser /* eventually this will extend \Parser */ {
 
 	/**
 	 * API users expect a ParsoidRenderID value set in the parser output's extension data.
-	 * @param int $revId
+	 * @param PageConfig $pageConfig
 	 * @param ParserOutput $parserOutput
 	 */
-	private function setParsoidRenderID( int $revId, ParserOutput $parserOutput ): void {
-		$parserOutput->setParsoidRenderId(
-			new ParsoidRenderID( $revId, $this->globalIdGenerator->newUUIDv1() )
-		);
-
-		$now = wfTimestampNow();
-		$parserOutput->setCacheRevisionId( $revId );
-		$parserOutput->setCacheTime( $now );
+	private function setParsoidRenderID( PageConfig $pageConfig, ParserOutput $parserOutput ): void {
+		$parserOutput->setRenderId( $this->globalIdGenerator->newUUIDv1() );
+		$parserOutput->setCacheRevisionId( $pageConfig->getRevisionId() );
+		$parserOutput->setTimestamp( $pageConfig->getRevisionTimestamp() );
+		$parserOutput->setCacheTime( wfTimestampNow() );
 	}
 
 	/**
@@ -169,7 +166,10 @@ class ParsoidParser /* eventually this will extend \Parser */ {
 
 		$revId = $pageConfig->getRevisionId();
 		if ( $revId !== null ) {
-			$this->setParsoidRenderID( $revId, $parserOutput );
+			// T350538: This shouldn't be necessary so long as ContentRenderer
+			// is involved in the call chain somewhere, and should be turned
+			// into an assertion (and ::setParsoidRenderID() removed).
+			$this->setParsoidRenderID( $pageConfig, $parserOutput );
 		}
 
 		// Copied from Parser.php::parse and should probably be abstracted

includes/parser/Parsoid/ParsoidRenderID.php

@@ -3,6 +3,7 @@
 namespace MediaWiki\Parser\Parsoid;
 
 use InvalidArgumentException;
+use MediaWiki\Parser\ParserOutput;
 
 /**
  * Represents the identity of a specific rendering of a specific revision
@@ -44,6 +45,23 @@ class ParsoidRenderID {
 		return new self( (int)$revisionID, $uniqueID );
 	}
 
+	/**
+	 * Create a ParsoidRenderID from the revision and render id stored in
+	 * a ParserOutput.
+	 * @param ParserOutput $parserOutput
+	 * @return self
+	 */
+	public static function newFromParserOutput( ParserOutput $parserOutput ): self {
+		$revisionID = $parserOutput->getCacheRevisionId();
+		$uniqueID = $parserOutput->getRenderId();
+
+		if ( $revisionID === null || $uniqueID === null ) {
+			throw new InvalidArgumentException( 'Missing render id' );
+		}
+
+		return new self( $revisionID, $uniqueID );
+	}
+
 	/**
 	 * This constructs a new render ID from the given ETag.
 	 *

includes/parser/RevisionOutputCache.php

@@ -33,6 +33,7 @@ use MediaWiki\Utils\MWTimestamp;
 use ParserOptions;
 use Psr\Log\LoggerInterface;
 use WANObjectCache;
+use Wikimedia\UUID\GlobalIdGenerator;
 
 /**
  * Cache for ParserOutput objects.
@@ -72,6 +73,8 @@ class RevisionOutputCache {
 	/** @var LoggerInterface */
 	private $logger;
 
+	private GlobalIdGenerator $globalIdGenerator;
+
 	/**
 	 * @param string $name
 	 * @param WANObjectCache $cache
@@ -80,6 +83,7 @@ class RevisionOutputCache {
 	 * @param JsonCodec $jsonCodec
 	 * @param IBufferingStatsdDataFactory $stats
 	 * @param LoggerInterface $logger
+	 * @param GlobalIdGenerator $globalIdGenerator
 	 */
 	public function __construct(
 		string $name,
@@ -88,7 +92,8 @@ class RevisionOutputCache {
 		string $cacheEpoch,
 		JsonCodec $jsonCodec,
 		IBufferingStatsdDataFactory $stats,
-		LoggerInterface $logger
+		LoggerInterface $logger,
+		GlobalIdGenerator $globalIdGenerator
 	) {
 		$this->name = $name;
 		$this->cache = $cache;
@@ -97,6 +102,7 @@ class RevisionOutputCache {
 		$this->jsonCodec = $jsonCodec;
 		$this->stats = $stats;
 		$this->logger = $logger;
+		$this->globalIdGenerator = $globalIdGenerator;
 	}
 
 	/**
@@ -242,11 +248,24 @@ class RevisionOutputCache {
 
 		$cacheKey = $this->makeParserOutputKey( $revision, $parserOptions );
 
-		$output->setCacheTime( $cacheTime ?: wfTimestampNow() );
-		$output->setCacheRevisionId( $revision->getId() );
-
-		// Save the timestamp so that we don't have to load the revision row on view
-		$output->setTimestamp( $revision->getTimestamp() );
+		// Ensure cache properties are set in the ParserOutput
+		// T350538: These should be turned into assertions that the
+		// properties are already present (and the $cacheTime argument
+		// removed).
+		if ( $cacheTime ) {
+			$output->setCacheTime( $cacheTime );
+		} else {
+			$cacheTime = $output->getCacheTime();
+		}
+		if ( !$output->getCacheRevisionId() ) {
+			$output->setCacheRevisionId( $revision->getId() );
+		}
+		if ( !$output->getRenderId() ) {
+			$output->setRenderId( $this->globalIdGenerator->newUUIDv1() );
+		}
+		if ( !$output->getTimestamp() ) {
+			$output->setTimestamp( $revision->getTimestamp() );
+		}
 
 		$msg = "Saved in RevisionOutputCache with key $cacheKey" .
 			" and timestamp $cacheTime" .

maintenance/renderDump.php

@@ -29,6 +29,7 @@
  */
 
 use MediaWiki\Permissions\UltimateAuthority;
+use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\User\User;
 
 require_once __DIR__ . '/Maintenance.php';
@@ -118,7 +119,16 @@ class DumpRenderer extends Maintenance {
 
 		$content = $rev->getContent();
 		$contentRenderer = $this->getServiceContainer()->getContentRenderer();
-		$output = $contentRenderer->getParserOutput( $content, $title, null, $options );
+		// ContentRenderer expects a RevisionRecord, and all we have is a
+		// WikiRevision from the dump.  Make a fake MutableRevisionRecord to
+		// satisfy it -- the only thing ::getParserOutput actually needs is
+		// the revision ID and revision timestamp.
+		$mutableRev = new MutableRevisionRecord( $rev->getTitle() );
+		$mutableRev->setId( $rev->getID() );
+		$mutableRev->setTimestamp( $rev->getTimestamp() );
+		$output = $contentRenderer->getParserOutput(
+			$content, $title, $mutableRev, $options
+		);
 
 		file_put_contents( $filename,
 			"<!DOCTYPE html>\n" .

tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php

@@ -17,6 +17,7 @@ use MediaWiki\Page\PageIdentity;
 use MediaWiki\Page\PageIdentityValue;
 use MediaWiki\Parser\ParserCacheFactory;
 use MediaWiki\Parser\Parsoid\ParsoidOutputAccess;
+use MediaWiki\Parser\Parsoid\ParsoidRenderID;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\MutableRevisionSlots;
 use MediaWiki\Revision\RevisionRecord;
@@ -1333,8 +1334,9 @@ class DerivedPageDataUpdaterTest extends MediaWikiIntegrationTestCase {
 		$this->assertGreaterThan( $rev->getTimestamp(), $parsoidCached->getCacheTime() );
 		$this->assertSame( $rev->getId(), $parsoidCached->getCacheRevisionId() );
 
-		// Check that getParsoidRenderID() doesn't throw, so we know that $parsoidCached is valid.
-		$this->getServiceContainer()->getParsoidOutputAccess()->getParsoidRenderID( $parsoidCached );
+		// Check that ParsoidRenderID::newFromParserOutput() doesn't throw,
+		// so we know that $parsoidCached is valid.
+		ParsoidRenderID::newFromParserOutput( $parsoidCached );
 
 		// The cached ParserOutput should not use the revision timestamp
 		// Create nwe ParserOptions object since we setUseParsoid() above

tests/phpunit/includes/page/ParserOutputAccessTest.php

@@ -81,7 +81,8 @@ class ParserOutputAccessTest extends MediaWikiIntegrationTestCase {
 			new NullStatsdDataFactory(),
 			new NullLogger(),
 			$this->getServiceContainer()->getTitleFactory(),
-			$this->getServiceContainer()->getWikiPageFactory()
+			$this->getServiceContainer()->getWikiPageFactory(),
+			$this->getServiceContainer()->getGlobalIdGenerator()
 		);
 
 		return $parserCache;
@@ -96,7 +97,8 @@ class ParserOutputAccessTest extends MediaWikiIntegrationTestCase {
 			'19900220000000',
 			new JsonCodec(),
 			new NullStatsdDataFactory(),
-			new NullLogger()
+			new NullLogger(),
+			$this->getServiceContainer()->getGlobalIdGenerator()
 		);
 
 		return $revisionOutputCache;

tests/phpunit/includes/parser/ParserCacheTest.php

@@ -28,6 +28,7 @@ use Psr\Log\LogLevel;
 use Psr\Log\NullLogger;
 use TestLogger;
 use Wikimedia\TestingAccessWrapper;
+use Wikimedia\UUID\GlobalIdGenerator;
 use WikiPage;
 
 /**
@@ -90,6 +91,8 @@ class ParserCacheTest extends MediaWikiIntegrationTestCase {
 			$wikiPageFactory = $this->createMock( WikiPageFactory::class );
 			$wikiPageFactory->method( 'newFromTitle' )->willReturn( $wikiPageMock );
 		}
+		$globalIdGenerator = $this->createMock( GlobalIdGenerator::class );
+		$globalIdGenerator->method( 'newUUIDv1' )->willReturn( 'uuid-uuid' );
 		return new ParserCache(
 			'test',
 			$storage ?: new HashBagOStuff(),
@@ -99,7 +102,8 @@ class ParserCacheTest extends MediaWikiIntegrationTestCase {
 			new NullStatsdDataFactory(),
 			$logger ?: new NullLogger(),
 			$this->createMock( TitleFactory::class ),
-			$wikiPageFactory
+			$wikiPageFactory,
+			$globalIdGenerator
 		);
 	}
 
@@ -124,6 +128,10 @@ class ParserCacheTest extends MediaWikiIntegrationTestCase {
 			$parserOutput->recordOption( $option );
 		}
 		$parserOutput->updateCacheExpiry( 4242 );
+		$parserOutput->setRenderId( 'dummy-render-id' );
+		$parserOutput->setCacheRevisionId( 0 );
+		// ParserOutput::getCacheTime() also sets it as a side effect
+		$parserOutput->setTimestamp( $parserOutput->getCacheTime() );
 		return $parserOutput;
 	}
 
@@ -683,6 +691,8 @@ class ParserCacheTest extends MediaWikiIntegrationTestCase {
 		$wikiPageMock->method( 'getContentModel' )->willReturn( CONTENT_MODEL_WIKITEXT );
 		$wikiPageFactory = $this->createMock( WikiPageFactory::class );
 		$wikiPageFactory->method( 'newFromTitle' )->willReturn( $wikiPageMock );
+		$globalIdGenerator = $this->createMock( GlobalIdGenerator::class );
+		$globalIdGenerator->method( 'newUUIDv1' )->willReturn( 'uuid-uuid' );
 		$cache = $this->getMockBuilder( ParserCache::class )
 			->setConstructorArgs( [
 				'test',
@@ -693,7 +703,8 @@ class ParserCacheTest extends MediaWikiIntegrationTestCase {
 				new NullStatsdDataFactory(),
 				new NullLogger(),
 				$this->createMock( TitleFactory::class ),
-				$wikiPageFactory
+				$wikiPageFactory,
+				$globalIdGenerator
 			] )
 			->onlyMethods( [ 'convertForCache' ] )
 			->getMock();

tests/phpunit/includes/parser/ParserOutputTest.php

@@ -992,6 +992,43 @@ EOF
 			'getUsedOptions' => [ 'Foo', 'Bar', 'Zoo' ],
 		] ];
 
+		// cache time
+		$someTime = "20240207202040";
+		$someLaterTime = "20240207202112";
+		$a = new ParserOutput();
+		$a->setCacheTime( $someTime );
+		$b = new ParserOutput();
+		yield 'only left cache time' => [ $a, $b, [ 'getCacheTime' => $someTime ] ];
+
+		$a = new ParserOutput();
+		$b = new ParserOutput();
+		$b->setCacheTime( $someTime );
+		yield 'only right cache time' => [ $a, $b, [ 'getCacheTime' => $someTime ] ];
+
+		$a = new ParserOutput();
+		$b = new ParserOutput();
+		$a->setCacheTime( $someLaterTime );
+		$b->setCacheTime( $someTime );
+		yield 'left has later cache time' => [ $a, $b, [ 'getCacheTime' => $someLaterTime ] ];
+
+		$a = new ParserOutput();
+		$b = new ParserOutput();
+		$a->setCacheTime( $someTime );
+		$b->setCacheTime( $someLaterTime );
+		yield 'right has later cache time' => [ $a, $b, [ 'getCacheTime' => $someLaterTime ] ];
+
+		$a = new ParserOutput();
+		$b = new ParserOutput();
+		$a->setCacheTime( -1 );
+		$b->setCacheTime( $someTime );
+		yield 'left is uncacheable' => [ $a, $b, [ 'getCacheTime' => "-1" ] ];
+
+		$a = new ParserOutput();
+		$b = new ParserOutput();
+		$a->setCacheTime( $someTime );
+		$b->setCacheTime( -1 );
+		yield 'right is uncacheable' => [ $a, $b, [ 'getCacheTime' => "-1" ] ];
+
 		// timestamp ------------
 		$a = new ParserOutput();
 		$a->setTimestamp( '20180101000011' );
@@ -1103,6 +1140,7 @@ EOF
 	 * @param array $expected
 	 */
 	public function testMergeInternalMetaDataFrom( ParserOutput $a, ParserOutput $b, $expected ) {
+		$this->filterDeprecated( '/^CacheTime::setCacheTime called with -1 as an argument/' );
 		$a->mergeInternalMetaDataFrom( $b );
 
 		$this->assertFieldValues( $a, $expected );
@@ -1320,4 +1358,47 @@ EOF
 		$this->assertEquals( [ 'baz.com' ], $po->getExtraCSPDefaultSrcs() );
 		$this->assertEquals( [ 'fred.com', 'xyzzy.com' ], $po->getExtraCSPStyleSrcs() );
 	}
+
+	/**
+	 * @covers \MediaWiki\Parser\ParserOutput::getCacheTime()
+	 * @covers \MediaWiki\Parser\ParserOutput::setCacheTime()
+	 */
+	public function testCacheTime() {
+		$po = new ParserOutput();
+
+		// Should not have a cache time yet
+		$this->assertFalse( $po->hasCacheTime() );
+		// But calling ::get assigns a cache time
+		$po->getCacheTime();
+		$this->assertTrue( $po->hasCacheTime() );
+		// Reset cache time
+		$po->setCacheTime( "20240207202040" );
+		$this->assertSame( "20240207202040", $po->getCacheTime() );
+	}
+
+	/**
+	 * @covers \MediaWiki\Parser\ParserOutput::getRenderId()
+	 * @covers \MediaWiki\Parser\ParserOutput::setRenderId()
+	 */
+	public function testRenderId() {
+		$po = new ParserOutput();
+
+		// Should be null when unset
+		$this->assertNull( $po->getRenderId() );
+
+		// Sanity check for setter and getter
+		$po->setRenderId( "TestRenderId" );
+		$this->assertEquals( "TestRenderId", $po->getRenderId() );
+	}
+
+	/**
+	 * @covers \MediaWiki\Parser\ParserOutput::getRenderId()
+	 */
+	public function testRenderIdBackCompat() {
+		$po = new ParserOutput();
+
+		// Parser cache used to contain extension data under a different name
+		$po->setExtensionData( 'parsoid-render-id', "1234/LegacyRenderId" );
+		$this->assertEquals( "LegacyRenderId", $po->getRenderId() );
+	}
 }

tests/phpunit/includes/parser/RevisionOutputCacheTest.php

@@ -24,6 +24,7 @@ use Psr\Log\NullLogger;
 use TestLogger;
 use WANObjectCache;
 use Wikimedia\TestingAccessWrapper;
+use Wikimedia\UUID\GlobalIdGenerator;
 
 /**
  * @covers \MediaWiki\Parser\RevisionOutputCache
@@ -74,6 +75,8 @@ class RevisionOutputCacheTest extends MediaWikiIntegrationTestCase {
 		$expiry = 3600,
 		$epoch = '19900220000000'
 	): RevisionOutputCache {
+		$globalIdGenerator = $this->createMock( GlobalIdGenerator::class );
+		$globalIdGenerator->method( 'newUUIDv1' )->willReturn( 'uuid-uuid' );
 		return new RevisionOutputCache(
 			'test',
 			new WANObjectCache( [ 'cache' => $storage ?: new HashBagOStuff() ] ),
@@ -81,7 +84,8 @@ class RevisionOutputCacheTest extends MediaWikiIntegrationTestCase {
 			$epoch,
 			new JsonCodec(),
 			new NullStatsdDataFactory(),
-			$logger ?: new NullLogger()
+			$logger ?: new NullLogger(),
+			$globalIdGenerator
 		);
 	}
 

tests/phpunit/includes/poolcounter/PoolWorkArticleViewCurrentTest.php

@@ -63,7 +63,8 @@ class PoolWorkArticleViewCurrentTest extends PoolWorkArticleViewTest {
 			$this->getServiceContainer()->getStatsdDataFactory(),
 			new NullLogger(),
 			$this->getServiceContainer()->getTitleFactory(),
-			$this->getServiceContainer()->getWikiPageFactory()
+			$this->getServiceContainer()->getWikiPageFactory(),
+			$this->getServiceContainer()->getGlobalIdGenerator()
 		);
 
 		return $this->parserCache;

tests/phpunit/includes/poolcounter/PoolWorkArticleViewOldTest.php

@@ -6,6 +6,7 @@ use MediaWiki\PoolCounter\PoolWorkArticleViewOld;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Status\Status;
 use Psr\Log\NullLogger;
+use Wikimedia\UUID\GlobalIdGenerator;
 
 /**
  * @covers \MediaWiki\PoolCounter\PoolWorkArticleViewOld
@@ -58,6 +59,8 @@ class PoolWorkArticleViewOldTest extends PoolWorkArticleViewTest {
 	 * @return RevisionOutputCache
 	 */
 	private function installRevisionOutputCache( $bag = null ) {
+		$globalIdGenerator = $this->createMock( GlobalIdGenerator::class );
+		$globalIdGenerator->method( 'newUUIDv1' )->willReturn( 'uuid-uuid' );
 		$this->cache = new RevisionOutputCache(
 			'test',
 			new WANObjectCache( [ 'cache' => $bag ?: new HashBagOStuff() ] ),
@@ -65,7 +68,8 @@ class PoolWorkArticleViewOldTest extends PoolWorkArticleViewTest {
 			'20200101223344',
 			new JsonCodec(),
 			new NullStatsdDataFactory(),
-			new NullLogger()
+			new NullLogger(),
+			$globalIdGenerator
 		);
 
 		return $this->cache;

tests/phpunit/integration/includes/Rest/Handler/Helper/HtmlInputTransformHelperTest.php

@@ -1078,7 +1078,7 @@ class HtmlInputTransformHelperTest extends MediaWikiIntegrationTestCase {
 		$popt = ParserOptions::newFromAnon();
 		$pout = $access->getParserOutput( $page, $popt )->getValue();
 
-		$key = $access->getParsoidRenderID( $pout );
+		$key = ParsoidRenderID::newFromParserOutput( $pout )->getKey();
 		$html = $pout->getRawText();
 
 		// Load the original data based on the ETag

tests/phpunit/integration/includes/Rest/Handler/Helper/HtmlOutputRendererHelperTest.php

@@ -83,10 +83,6 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 		return $count === null ? $this->any() : $this->exactly( $count );
 	}
 
-	public function getParsoidRenderID( ParserOutput $pout ) {
-		return new ParsoidRenderID( $pout->getCacheRevisionId(), $pout->getCacheTime() );
-	}
-
 	/**
 	 * @param LoggerInterface|null $logger
 	 *
@@ -106,7 +102,6 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 		$expectedCalls = [
 			'getParserOutput' => null,
 			'parseUncacheable' => null,
-			'getParsoidRenderID' => null
 		];
 
 		$parsoid = $this->createNoOpMock( ParsoidOutputAccess::class, array_keys( $expectedCalls ) );
@@ -128,9 +123,6 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 				return Status::newGood( $pout );
 			} );
 
-		$parsoid->method( 'getParsoidRenderID' )
-			->willReturnCallback( [ $this, 'getParsoidRenderID' ] );
-
 		$parsoid->expects( $this->exactlyOrAny( $expectedCalls[ 'parseUncacheable' ] ) )
 			->method( 'parseUncacheable' )
 			->willReturnCallback( function (
@@ -151,10 +143,6 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 				return Status::newGood( $pout );
 			} );
 
-		$parsoid->expects( $this->exactlyOrAny( $expectedCalls[ 'getParsoidRenderID' ] ) )
-			->method( 'getParsoidRenderID' )
-			->willReturnCallback( [ $this, 'getParsoidRenderID' ] );
-
 		return $parsoid;
 	}
 
@@ -186,19 +174,27 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 		PageIdentity $page,
 		string $version = null
 	): ParserOutput {
+		static $counter = 0;
 		$lang = $parserOpts->getTargetLanguage();
 		$lang = $lang ? $lang->getCode() : 'en';
 		$version ??= Parsoid::defaultHTMLVersion();
 
 		$html = "<!DOCTYPE html><html lang=\"$lang\"><body><div id='t3s7'>$html</div></body></html>";
 
+		$revTimestamp = null;
 		if ( $rev instanceof RevisionRecord ) {
+			$revTimestamp = $rev->getTimestamp();
 			$rev = $rev->getId();
 		}
 
 		$pout = new ParserOutput( $html );
 		$pout->setCacheRevisionId( $rev ?: $page->getLatest() );
 		$pout->setCacheTime( wfTimestampNow() ); // will use fake time
+		if ( $revTimestamp ) {
+			$pout->setTimestamp( $revTimestamp );
+		}
+		// We test that UUIDs are unique, so make a cheap unique UUID
+		$pout->setRenderId( 'bogus-uuid-' . strval( $counter++ ) );
 		$pout->setExtensionData( PageBundleParserOutputConverter::PARSOID_PAGE_BUNDLE_KEY, [
 			'parsoid' => [ 'ids' => [
 				't3s7' => [ 'dsr' => [ 0, 0, 0, 0 ] ],
@@ -570,7 +566,7 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 		// put HTML into the cache
 		$pout = $helper->getHtml();
 
-		$renderId = $this->getParsoidRenderID( $pout );
+		$renderId = ParsoidRenderID::newFromParserOutput( $pout );
 		$lastModified = $pout->getCacheTime();
 
 		if ( $rev ) {
@@ -629,8 +625,6 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 				$pout = $this->makeParserOutput( $parserOpts, $html, $rev, $page );
 				return Status::newGood( $pout );
 			} );
-		$poa->method( 'getParsoidRenderID' )
-			->willReturnCallback( [ $this, 'getParsoidRenderID' ] );
 
 		$helper = $this->newHelper( null, $poa );
 		$helper->init( $fakePage, self::PARAM_DEFAULTS, $this->newAuthority() );
@@ -639,7 +633,7 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 		$this->assertNull( $helper->getRevisionId() );
 
 		$pout = $helper->getHtml();
-		$renderId = $this->getParsoidRenderID( $pout );
+		$renderId = ParsoidRenderID::newFromParserOutput( $pout );
 		$lastModified = $pout->getCacheTime();
 
 		$this->assertStringContainsString( $renderId->getKey(), $helper->getETag() );
@@ -942,8 +936,6 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 				$pout = $this->makeParserOutput( $parserOpts, $html, $revision, $page );
 				return Status::newGood( $pout );
 			} );
-		$poa->method( 'getParsoidRenderID' )
-			->willReturnCallback( [ $this, 'getParsoidRenderID' ] );
 
 		$helper = $this->newHelper( null, $poa );
 
@@ -1133,7 +1125,7 @@ class HtmlOutputRendererHelperTest extends MediaWikiIntegrationTestCase {
 
 		$output = $helper->getHtml();
 		$this->assertStringContainsString( 'Dummy output', $output->getText() );
-		$this->assertSame( '0/dummy-output', $output->getExtensionData( 'parsoid-render-id' ) );
+		$this->assertSame( '0/dummy-output', ParsoidRenderID::newFromParserOutput( $output )->getKey() );
 	}
 
 	/**

tests/phpunit/integration/includes/Rest/Handler/ParsoidOutputAccessTest.php

@@ -8,6 +8,7 @@ use MediaWiki\Parser\Parsoid\PageBundleParserOutputConverter;
 use MediaWiki\Parser\Parsoid\ParsoidOutputAccess;
 use MediaWiki\Parser\Parsoid\ParsoidParser;
 use MediaWiki\Parser\Parsoid\ParsoidParserFactory;
+use MediaWiki\Parser\Parsoid\ParsoidRenderID;
 use MediaWiki\Rest\HttpException;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionAccessException;
@@ -186,7 +187,6 @@ class ParsoidOutputAccessTest extends MediaWikiIntegrationTestCase {
 	 * Tests that getParserOutput() will return output.
 	 *
 	 * @covers \MediaWiki\Parser\Parsoid\ParsoidOutputAccess::getParserOutput
-	 * @covers \MediaWiki\Parser\Parsoid\ParsoidOutputAccess::getParsoidRenderID
 	 */
 	public function testGetParserOutput() {
 		$this->resetServicesWithMockedParsoid( 1 );
@@ -201,8 +201,8 @@ class ParsoidOutputAccessTest extends MediaWikiIntegrationTestCase {
 
 		$output = $status->getValue();
 
-		// check that getParsoidRenderID() doesn't throw
-		$this->assertNotNull( $access->getParsoidRenderID( $output ) );
+		// check that ParsoidRenderID::newFromParserOutput()  doesn't throw
+		$this->assertNotNull( ParsoidRenderID::newFromParserOutput( $output ) );
 
 		// Ensure that we can still create a valid instance of PageBundle from the ParserOutput
 		$pageBundle = PageBundleParserOutputConverter::pageBundleFromParserOutput( $output );
@@ -340,7 +340,7 @@ class ParsoidOutputAccessTest extends MediaWikiIntegrationTestCase {
 
 		/** @var ParserOutput $parserOutput */
 		$parserOutput = $status->getValue();
-		$this->assertSame( '0/dummy-output', $parserOutput->getExtensionData( 'parsoid-render-id' ) );
+		$this->assertSame( '0/dummy-output', ParsoidRenderID::newFromParserOutput( $parserOutput )->getKey() );
 
 		$expTime = $parserOutput->getCacheExpiry();
 		$this->assertSame( 0, $expTime );
@@ -406,9 +406,9 @@ class ParsoidOutputAccessTest extends MediaWikiIntegrationTestCase {
 		$this->assertContainsHtml( self::MOCKED_HTML . ' of ' . self::WIKITEXT, $status1 );
 		$this->checkMetadata( $status1 );
 
-		// check that getParsoidRenderID() doesn't throw
+		// check that ParsoidRenderID::newFromParserOutput() doesn't throw
 		$output1 = $status1->getValue();
-		$this->assertNotNull( $access->getParsoidRenderID( $output1 ) );
+		$this->assertNotNull( ParsoidRenderID::newFromParserOutput( $output1 ) );
 	}
 
 	public static function provideSupportsContentModels() {
@@ -452,7 +452,7 @@ class ParsoidOutputAccessTest extends MediaWikiIntegrationTestCase {
 		/** @var ParserOutput $parserOutput */
 		$parserOutput = $status->getValue();
 		$this->assertStringContainsString( __METHOD__, $parserOutput->getRawText() );
-		$this->assertNotEmpty( $parserOutput->getExtensionData( 'parsoid-render-id' ) );
+		$this->assertNotEmpty( $parserOutput->getRenderId() );
 		$this->assertNotEmpty( $parserOutput->getCacheRevisionId() );
 		$this->assertNotEmpty( $parserOutput->getCacheTime() );
 	}
@@ -485,7 +485,7 @@ class ParsoidOutputAccessTest extends MediaWikiIntegrationTestCase {
 		/** @var ParserOutput $parserOutput */
 		$parserOutput = $status->getValue();
 		$this->assertStringContainsString( __METHOD__, $parserOutput->getRawText() );
-		$this->assertNotEmpty( $parserOutput->getExtensionData( 'parsoid-render-id' ) );
+		$this->assertNotEmpty( $parserOutput->getRenderId() );
 		$this->assertNotEmpty( $parserOutput->getCacheRevisionId() );
 		$this->assertNotEmpty( $parserOutput->getCacheTime() );
 	}
@@ -508,7 +508,7 @@ class ParsoidOutputAccessTest extends MediaWikiIntegrationTestCase {
 		/** @var ParserOutput $parserOutput */
 		$parserOutput = $status->getValue();
 		$this->assertStringContainsString( __METHOD__, $parserOutput->getRawText() );
-		$this->assertNotEmpty( $parserOutput->getExtensionData( 'parsoid-render-id' ) );
+		$this->assertNotEmpty( $parserOutput->getRenderId() );
 		$this->assertNotEmpty( $parserOutput->getCacheRevisionId() );
 		$this->assertNotEmpty( $parserOutput->getCacheTime() );
 	}
@@ -539,7 +539,7 @@ class ParsoidOutputAccessTest extends MediaWikiIntegrationTestCase {
 		/** @var ParserOutput $parserOutput */
 		$parserOutput = $status->getValue();
 		$this->assertStringContainsString( __METHOD__, $parserOutput->getRawText() );
-		$this->assertNotEmpty( $parserOutput->getExtensionData( 'parsoid-render-id' ) );
+		$this->assertNotEmpty( $parserOutput->getRenderId() );
 		// The revision ID is set to 0, so that's what is in the cache.
 		$this->assertSame( 0, $parserOutput->getCacheRevisionId() );
 		$this->assertNotEmpty( $parserOutput->getCacheTime() );

tests/phpunit/unit/includes/parser/ParserCacheFactoryTest.php

@@ -9,6 +9,7 @@ use MediaWiki\Parser\ParserCacheFactory;
 use MediaWiki\Parser\RevisionOutputCache;
 use MediaWiki\Title\TitleFactory;
 use Psr\Log\NullLogger;
+use Wikimedia\UUID\GlobalIdGenerator;
 
 /**
  * @covers \MediaWiki\Parser\ParserCacheFactory
@@ -35,7 +36,8 @@ class ParserCacheFactoryTest extends MediaWikiUnitTestCase {
 			new NullLogger(),
 			$options,
 			$this->createNoOpMock( TitleFactory::class ),
-			$this->createNoOpMock( WikiPageFactory::class )
+			$this->createNoOpMock( WikiPageFactory::class ),
+			$this->createNoOpMock( GlobalIdGenerator::class )
 		);
 	}