Move Content::prepareSave to ContentHandler

Update method name in ContentHandler, soft-deprecate method on Content.
This will require making a semi-backwards-incompatible
change no matter what, we don't really have a great way
of hard-deprecating overriding methods.
Replace all callers of Content::prepareSave in core.

Add tests for ContentHandler::validateSave.

Bug: T287159
Change-Id: I7f23e6e97b1c7d27a6aaefdb88b19b2fc6e8b3a8
This commit is contained in:
Roman Stolar 2021-10-22 15:48:23 +03:00
parent 044cd72f5a
commit 5461404d69
8 changed files with 180 additions and 24 deletions

View file

@ -34,6 +34,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;
@ -1106,10 +1107,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 );

View file

@ -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 );
}
/**

View file

@ -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()

View file

@ -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

View file

@ -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;
}
}

View file

@ -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
);
}
}

View file

@ -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__ );

View file

@ -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 );
}
}