Merge "Move Content::prepareSave to ContentHandler"

includes/Storage/PageUpdater.php

@@ -35,6 +35,7 @@ use LogicException;
 use ManualLogEntry;
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Content\IContentHandlerFactory;
+use MediaWiki\Content\ValidationParams;
 use MediaWiki\HookContainer\HookContainer;
 use MediaWiki\HookContainer\HookRunner;
 use MediaWiki\Page\PageIdentity;
@@ -1107,10 +1108,9 @@ class PageUpdater {
 			$content = $slot->getContent();
 
 			// XXX: We may push this up to the "edit controller" level, see T192777.
-			// XXX: prepareSave() and isValid() could live in SlotRoleHandler
-			// XXX: PrepareSave should not take a WikiPage!
-			$legacyUser = self::toLegacyUser( $this->author );
-			$prepStatus = $content->prepareSave( $wikiPage, $this->flags, $oldid, $legacyUser );
+			$contentHandler = $this->contentHandlerFactory->getContentHandler( $content->getModel() );
+			$validationParams = new ValidationParams( $wikiPage, $this->flags, $oldid );
+			$prepStatus = $contentHandler->validateSave( $content, $validationParams );
 
 			// TODO: MCR: record which problem arose in which slot.
 			$status->merge( $prepStatus );

includes/content/AbstractContent.php

@@ -30,6 +30,7 @@ use MediaWiki\Content\IContentHandlerFactory;
 use MediaWiki\Content\Renderer\ContentParseParams;
 use MediaWiki\Content\Transform\PreloadTransformParamsValue;
 use MediaWiki\Content\Transform\PreSaveTransformParamsValue;
+use MediaWiki\Content\ValidationParams;
 use MediaWiki\MediaWikiServices;
 
 /**
@@ -444,8 +445,8 @@ abstract class AbstractContent implements Content {
 	}
 
 	/**
-	 * @stable to override
 	 * @since 1.21
+	 * @deprecated since 1.38. Use ContentHandler::validateSave instead.
 	 *
 	 * @param WikiPage $page
 	 * @param int $flags
@@ -456,11 +457,27 @@ abstract class AbstractContent implements Content {
 	 * @see Content::prepareSave
 	 */
 	public function prepareSave( WikiPage $page, $flags, $parentRevId, User $user ) {
-		if ( $this->isValid() ) {
-			return Status::newGood();
-		} else {
-			return Status::newFatal( "invalid-content-data" );
+		$detectPSDeprecatedOverride = MWDebug::detectDeprecatedOverride(
+			$this,
+			self::class,
+			'prepareSave'
+		);
+
+		if ( $detectPSDeprecatedOverride ) {
+			if ( $this->isValid() ) {
+				return Status::newGood();
+			} else {
+				return Status::newFatal( "invalid-content-data" );
+			}
 		}
+
+		$validationParams = new ValidationParams( $page, $flags, $parentRevId );
+		$statusValue = $this->getContentHandler()->validateSave(
+			$this,
+			$validationParams
+		);
+
+		return Status::wrap( $statusValue );
 	}
 
 	/**

includes/content/Content.php

@@ -440,6 +440,7 @@ interface Content {
 	 * performed. This means that $page may not yet know a page ID.
 	 *
 	 * @since 1.21
+	 * @deprecated since 1.38. Use ContentHandler::validateSave instead.
 	 *
 	 * @param WikiPage $page The page to be saved.
 	 * @param int $flags Bitfield for use with EDIT_XXX constants, see WikiPage::doUserEditContent()

includes/content/ContentHandler.php

@@ -29,6 +29,7 @@
 use MediaWiki\Content\Renderer\ContentParseParams;
 use MediaWiki\Content\Transform\PreloadTransformParams;
 use MediaWiki\Content\Transform\PreSaveTransformParams;
+use MediaWiki\Content\ValidationParams;
 use MediaWiki\HookContainer\ProtectedHookAccessorTrait;
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
@@ -1612,6 +1613,59 @@ abstract class ContentHandler {
 		);
 	}
 
+	/**
+	 * Validate content for saving it.
+	 *
+	 * This may be used to check the content's consistency with global state. This function should
+	 * NOT write any information to the database.
+	 *
+	 * Note that this method will usually be called inside the same transaction
+	 * bracket that will be used to save the new revision, so the revision passed
+	 * in is probably unsaved (has no id) and might belong to unsaved page.
+	 *
+	 * @since 1.38
+	 * @stable to override
+	 *
+	 * @param Content $content
+	 * @param ValidationParams $validationParams
+	 *
+	 * @return StatusValue A status object indicating if content can be saved in the given revision.
+	 */
+	public function validateSave(
+		Content $content,
+		ValidationParams $validationParams
+	) {
+		$detectPSDeprecatedOverride = MWDebug::detectDeprecatedOverride(
+			$content,
+			AbstractContent::class,
+			'prepareSave'
+		);
+
+		if ( $detectPSDeprecatedOverride ) {
+			$services = MediaWikiServices::getInstance();
+			$page = $validationParams->getPageIdentity();
+			$user = RequestContext::getMain()->getUser();
+
+			if ( !( $page instanceof WikiPage ) ) {
+				$wikiPageFactory = $services->getWikiPageFactory();
+				$page = $wikiPageFactory->newFromTitle( $page );
+			}
+
+			return $content->prepareSave(
+				$page,
+				$validationParams->getFlags(),
+				$validationParams->getParentRevisionId(),
+				$user
+			);
+		}
+
+		if ( $content->isValid() ) {
+			return StatusValue::newGood();
+		} else {
+			return StatusValue::newFatal( "invalid-content-data" );
+		}
+	}
+
 	/**
 	 * Returns a ParserOutput object containing information derived from this content.
 	 * Most importantly, unless $cpoParams->getGenerateHtml was false, the return value contains an

includes/content/ValidationParams.php

@@ -0,0 +1,51 @@
+<?php
+namespace MediaWiki\Content;
+
+use MediaWiki\Page\PageIdentity;
+
+/**
+ * @since 1.38
+ * An object to hold validation params.
+ */
+class ValidationParams {
+	/** @var PageIdentity */
+	private $pageIdentity;
+
+	/** @var int */
+	private $flags;
+
+	/** @var int */
+	private $parentRevId;
+
+	public function __construct( PageIdentity $pageIdentity, int $flags, int $parentRevId = -1 ) {
+		$this->pageIdentity = $pageIdentity;
+		$this->flags = $flags;
+		$this->parentRevId = $parentRevId;
+	}
+
+	/**
+	 *
+	 * @return PageIdentity
+	 */
+	public function getPageIdentity(): PageIdentity {
+		return $this->pageIdentity;
+	}
+
+	/**
+	 *
+	 * @return int
+	 */
+	public function getFlags(): int {
+		return $this->flags;
+	}
+
+	/**
+	 * @deprecated since 1.38. Born soft-deprecated as we will move usage of it
+	 * to MultiContentSaveHook in ProofreadPage (only one place of usage).
+	 *
+	 * @return int
+	 */
+	public function getParentRevisionId(): int {
+		return $this->parentRevId;
+	}
+}

includes/page/PageCommandFactory.php

@@ -342,11 +342,11 @@ class PageCommandFactory implements
 			$this->repoGroup,
 			$this->undeletePageLogger,
 			$this->revisionStore,
-			$this->userFactory,
 			$this->wikiPageFactory,
 			$page,
 			$authority,
-			$this->pageUpdaterFactory
+			$this->pageUpdaterFactory,
+			$this->contentHandlerFactory
 		);
 	}
 }

includes/page/UndeletePage.php

@@ -26,6 +26,8 @@ use HTMLCacheUpdateJob;
 use JobQueueGroup;
 use LocalFile;
 use ManualLogEntry;
+use MediaWiki\Content\IContentHandlerFactory;
+use MediaWiki\Content\ValidationParams;
 use MediaWiki\HookContainer\HookContainer;
 use MediaWiki\HookContainer\HookRunner;
 use MediaWiki\Permissions\Authority;
@@ -33,7 +35,6 @@ use MediaWiki\Permissions\PermissionStatus;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\Revision\RevisionStore;
 use MediaWiki\Storage\PageUpdaterFactory;
-use MediaWiki\User\UserFactory;
 use Psr\Log\LoggerInterface;
 use ReadOnlyError;
 use ReadOnlyMode;
@@ -68,8 +69,6 @@ class UndeletePage {
 	private $repoGroup;
 	/** @var RevisionStore */
 	private $revisionStore;
-	/** @var UserFactory */
-	private $userFactory;
 	/** @var WikiPageFactory */
 	private $wikiPageFactory;
 
@@ -91,6 +90,8 @@ class UndeletePage {
 	private $tags = [];
 	/** @var PageUpdaterFactory */
 	private $pageUpdaterFactory;
+	/** @var IContentHandlerFactory */
+	private $contentHandlerFactory;
 
 	/**
 	 * @param HookContainer $hookContainer
@@ -100,11 +101,11 @@ class UndeletePage {
 	 * @param RepoGroup $repoGroup
 	 * @param LoggerInterface $logger
 	 * @param RevisionStore $revisionStore
-	 * @param UserFactory $userFactory
 	 * @param WikiPageFactory $wikiPageFactory
 	 * @param ProperPageIdentity $page
 	 * @param Authority $performer
 	 * @param PageUpdaterFactory $pageUpdaterFactory
+	 * @param IContentHandlerFactory $contentHandlerFactory
 	 */
 	public function __construct(
 		HookContainer $hookContainer,
@@ -114,11 +115,11 @@ class UndeletePage {
 		RepoGroup $repoGroup,
 		LoggerInterface $logger,
 		RevisionStore $revisionStore,
-		UserFactory $userFactory,
 		WikiPageFactory $wikiPageFactory,
 		ProperPageIdentity $page,
 		Authority $performer,
-		PageUpdaterFactory $pageUpdaterFactory
+		PageUpdaterFactory $pageUpdaterFactory,
+		IContentHandlerFactory $contentHandlerFactory
 	) {
 		$this->hookRunner = new HookRunner( $hookContainer );
 		$this->jobQueueGroup = $jobQueueGroup;
@@ -127,12 +128,12 @@ class UndeletePage {
 		$this->repoGroup = $repoGroup;
 		$this->logger = $logger;
 		$this->revisionStore = $revisionStore;
-		$this->userFactory = $userFactory;
 		$this->wikiPageFactory = $wikiPageFactory;
 
 		$this->page = $page;
 		$this->performer = $performer;
 		$this->pageUpdaterFactory = $pageUpdaterFactory;
+		$this->contentHandlerFactory = $contentHandlerFactory;
 	}
 
 	/**
@@ -413,14 +414,12 @@ class UndeletePage {
 				$this->page
 			);
 
-			// TODO: The User isn't used for anything in prepareSave()! We should drop it.
-			$legacyRevUser = $this->userFactory->newFromUserIdentity( $revision->getUser( RevisionRecord::RAW ) );
-
 			foreach ( $revision->getSlotRoles() as $role ) {
 				$content = $revision->getContent( $role, RevisionRecord::RAW );
-
-				// NOTE: article ID may not be known yet. prepareSave() should not modify the database.
-				$status = $content->prepareSave( $wikiPage, 0, -1, $legacyRevUser );
+				// NOTE: article ID may not be known yet. validateSave() should not modify the database.
+				$contentHandler = $this->contentHandlerFactory->getContentHandler( $content->getModel() );
+				$validationParams = new ValidationParams( $wikiPage, 0 );
+				$status = $contentHandler->validateSave( $content, $validationParams );
 				if ( !$status->isOK() ) {
 					$dbw->endAtomic( __METHOD__ );
 

tests/phpunit/includes/content/ContentHandlerTest.php

@@ -1,6 +1,9 @@
 <?php
 
+use MediaWiki\Content\ValidationParams;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\Page\PageIdentity;
+use MediaWiki\Page\PageIdentityValue;
 use Wikimedia\TestingAccessWrapper;
 
 /**
@@ -635,4 +638,35 @@ class ContentHandlerTest extends MediaWikiIntegrationTestCase {
 		$pageViewLanguage = $contentHandler->getPageViewLanguage( $title );
 		$this->assertEquals( $expected, $pageViewLanguage->getCode() );
 	}
+
+	public function provideValidateSave() {
+		yield 'wikitext' => [
+			new WikitextContent( 'hello world' ),
+			true
+		];
+
+		yield 'valid json' => [
+			new JsonContent( '{ "0": "bar" }' ),
+			true
+		];
+
+		yield 'invalid json' => [
+			new JsonContent( 'foo' ),
+			false
+		];
+	}
+
+	/**
+	 * @dataProvider provideValidateSave
+	 * @covers ContentHandler::validateSave
+	 */
+	public function testValidateSave( $content, $expectedResult ) {
+		$page = new PageIdentityValue( 0, 1, 'Foo', PageIdentity::LOCAL );
+		$contentHandlerFactory = MediaWikiServices::getInstance()->getContentHandlerFactory();
+		$contentHandler = $contentHandlerFactory->getContentHandler( $content->getModel() );
+		$validateParams = new ValidationParams( $page, 0 );
+
+		$status = $contentHandler->validateSave( $content, $validateParams );
+		$this->assertEquals( $status->isOk(), $expectedResult );
+	}
 }