Add ContentModelChangeFactory, implemented by PageCommandFactory

Bug: T253080 Change-Id: I62eda1163cd5b0472af912e8dbd5843df8303b8d
2020-05-30 19:10:58 +00:00 · 2020-05-30 19:10:58 +00:00 · d6a38d0f10
commit d6a38d0f10
parent 84390e9887
8 changed files with 180 additions and 73 deletions
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@ -46,6 +46,7 @@ use MediaWiki\Languages\LanguageNameUtils;
 use MediaWiki\Linker\LinkRenderer;
 use MediaWiki\Linker\LinkRendererFactory;
 use MediaWiki\Mail\IEmailer;
+use MediaWiki\Page\ContentModelChangeFactory;
 use MediaWiki\Page\MergeHistoryFactory;
 use MediaWiki\Page\MovePageFactory;
 use MediaWiki\Permissions\PermissionManager;
@ -607,6 +608,14 @@ class MediaWikiServices extends ServiceContainer {
 		return $this->getService( 'ContentLanguage' );
 	}

+	/**
+	 * @since 1.35
+	 * @return ContentModelChangeFactory
+	 */
+	public function getContentModelChangeFactory() : ContentModelChangeFactory {
+		return $this->getService( 'ContentModelChangeFactory' );
+	}
+
 	/**
 	 * @since 1.31
 	 * @return NameTableStore
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@ -76,6 +76,7 @@ use MediaWiki\Mail\Emailer;
 use MediaWiki\Mail\IEmailer;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Message\MessageFormatterFactory;
+use MediaWiki\Page\ContentModelChangeFactory;
 use MediaWiki\Page\MergeHistoryFactory;
 use MediaWiki\Page\MovePageFactory;
 use MediaWiki\Page\PageCommandFactory;
@ -222,6 +223,10 @@ return [
 			$services->getMainConfig()->get( 'LanguageCode' ) );
 	},

+	'ContentModelChangeFactory' => function ( MediaWikiServices $services ) : ContentModelChangeFactory {
+		return $services->getService( '_PageCommandFactory' );
+	},
+
 	'ContentModelStore' => function ( MediaWikiServices $services ) : NameTableStore {
 		return $services->getNameTableStoreFactory()->getContentModels();
 	},
--- a/includes/api/ApiChangeContentModel.php
+++ b/includes/api/ApiChangeContentModel.php
@ -1,5 +1,7 @@
 <?php

+use MediaWiki\MediaWikiServices;
+
 /**
 * Api module to change the content model of existing pages
 *
@ -28,12 +30,13 @@ class ApiChangeContentModel extends ApiBase {
 		$user = $this->getUser();

 		$this->checkUserRightsAny( 'editcontentmodel' );
-		$changer = new ContentModelChange(
-			$user,
-			$this->getPermissionManager(),
-			$wikiPage,
-			$newModel
-		);
+		$changer = MediaWikiServices::getInstance()
+			->getContentModelChangeFactory()
+			->newContentModelChange(
+				$user,
+				$wikiPage,
+				$newModel
+			);
 		// Status messages should be apierror-*
 		$changer->setMessagePrefix( 'apierror-' );
 		$errors = $changer->checkPermissions();
--- a/includes/content/ContentModelChange.php
+++ b/includes/content/ContentModelChange.php
@ -1,7 +1,10 @@
 <?php

-use MediaWiki\MediaWikiServices;
+use MediaWiki\Content\IContentHandlerFactory;
+use MediaWiki\HookContainer\HookContainer;
+use MediaWiki\HookContainer\HookRunner;
 use MediaWiki\Permissions\PermissionManager;
+use MediaWiki\Revision\RevisionLookup;
 use MediaWiki\Revision\SlotRecord;

 /**
@ -15,59 +18,66 @@ use MediaWiki\Revision\SlotRecord;
 */
 class ContentModelChange {

-	/**
-	 * @var User user making the change
-	 */
-	private $user;
+	/** @var IContentHandlerFactory */
+	private $contentHandlerFactory;

-	/**
-	 * @var PermissionManager permission manager to user
-	 */
+	/** @var HookRunner */
+	private $hookRunner;
+
+	/** @var PermissionManager */
 	private $permManager;

-	/**
-	 * @var WikiPage
-	 */
+	/** @var RevisionLookup */
+	private $revLookup;
+
+	/** @var User user making the change */
+	private $user;
+
+	/** @var WikiPage */
 	private $page;

-	/**
-	 * @var string
-	 */
+	/** @var string */
 	private $newModel;

-	/**
-	 * @var string[] tags to add
-	 */
+	/** @var string[] tags to add */
 	private $tags;

-	/**
-	 * @var Content
-	 */
+	/** @var Content */
 	private $newContent;

-	/**
-	 * @var int|false latest revision id, or false if creating
-	 */
+	/** @var int|false latest revision id, or false if creating */
 	private $latestRevId;

-	/**
-	 * @var string 'new' or 'change'
-	 */
+	/** @var string 'new' or 'change' */
 	private $logAction;

-	/**
-	 * @var string 'apierror-' or empty string, for status messages
-	 */
+	/** @var string 'apierror-' or empty string, for status messages */
 	private $msgPrefix;

+	/**
+	 * @param IContentHandlerFactory $contentHandlerFactory
+	 * @param HookContainer $hookContainer
+	 * @param PermissionManager $permManager
+	 * @param RevisionLookup $revLookup
+	 * @param User $user
+	 * @param WikiPage $page
+	 * @param string $newModel
+	 */
 	public function __construct(
-		User $user,
+		IContentHandlerFactory $contentHandlerFactory,
+		HookContainer $hookContainer,
 		PermissionManager $permManager,
+		RevisionLookup $revLookup,
+		User $user,
 		WikiPage $page,
-		$newModel
+		string $newModel
 	) {
-		$this->user = $user;
+		$this->contentHandlerFactory = $contentHandlerFactory;
+		$this->hookRunner = new HookRunner( $hookContainer );
 		$this->permManager = $permManager;
+		$this->revLookup = $revLookup;
+
+		$this->user = $user;
 		$this->page = $page;
 		$this->newModel = $newModel;

@ -140,11 +150,10 @@ class ContentModelChange {
 	 * @return Status
 	 */
 	private function createNewContent() {
-		$contentHandlerFactory = MediaWikiServices::getInstance()->getContentHandlerFactory();
-		$revLookup = MediaWikiServices::getInstance()->getRevisionLookup();
+		$contentHandlerFactory = $this->contentHandlerFactory;

 		$title = $this->page->getTitle();
-		$latestRevRecord = $revLookup->getRevisionByTitle( $title );
+		$latestRevRecord = $this->revLookup->getRevisionByTitle( $title );

 		if ( $latestRevRecord ) {
 			$latestContent = $latestRevRecord->getContent( SlotRecord::MAIN );
@ -256,7 +265,7 @@ class ContentModelChange {

 		$newContent = $this->newContent;

-		if ( !Hooks::runner()->onEditFilterMergedContent( $derivativeContext, $newContent,
+		if ( !$this->hookRunner->onEditFilterMergedContent( $derivativeContext, $newContent,
 			$status, $reason, $user, false )
 		) {
 			if ( $status->isGood() ) {
--- a/includes/page/ContentModelChangeFactory.php
+++ b/includes/page/ContentModelChangeFactory.php
@ -0,0 +1,45 @@
+<?php
+
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @author DannyS712
+ */
+
+namespace MediaWiki\Page;
+
+use ContentModelChange;
+use User;
+use WikiPage;
+
+/**
+ * @since 1.35
+ */
+interface ContentModelChangeFactory {
+
+	/**
+	 * @param User $user
+	 * @param WikiPage $wikipage
+	 * @param string $newContentModel
+	 * @return ContentModelChange
+	 */
+	public function newContentModelChange(
+		User $user,
+		WikiPage $wikipage,
+		string $newContentModel
+	) : ContentModelChange;
+}
--- a/includes/page/PageCommandFactory.php
+++ b/includes/page/PageCommandFactory.php
@ -22,6 +22,7 @@

 namespace MediaWiki\Page;

+use ContentModelChange;
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Content\IContentHandlerFactory;
 use MediaWiki\EditPage\SpamChecker;
@ -33,15 +34,17 @@ use MovePage;
 use NamespaceInfo;
 use RepoGroup;
 use Title;
+use User;
 use WatchedItemStoreInterface;
 use Wikimedia\Rdbms\ILoadBalancer;
+use WikiPage;

 /**
 * Common factory to construct page handling classes.
 *
 * @since 1.35
 */
-class PageCommandFactory implements MergeHistoryFactory, MovePageFactory {
+class PageCommandFactory implements ContentModelChangeFactory, MergeHistoryFactory, MovePageFactory {
 	/** @var ServiceOptions */
 	private $options;

@ -49,13 +52,13 @@ class PageCommandFactory implements MergeHistoryFactory, MovePageFactory {
 	private $loadBalancer;

 	/** @var NamespaceInfo */
-	private $nsInfo;
+	private $namespaceInfo;

 	/** @var WatchedItemStoreInterface */
 	private $watchedItemStore;

 	/** @var PermissionManager */
-	private $permManager;
+	private $permissionManager;

 	/** @var RepoGroup */
 	private $repoGroup;
@ -79,9 +82,9 @@ class PageCommandFactory implements MergeHistoryFactory, MovePageFactory {
 	public function __construct(
 		ServiceOptions $options,
 		ILoadBalancer $loadBalancer,
-		NamespaceInfo $nsInfo,
+		NamespaceInfo $namespaceInfo,
 		WatchedItemStoreInterface $watchedItemStore,
-		PermissionManager $permManager,
+		PermissionManager $permissionManager,
 		RepoGroup $repoGroup,
 		IContentHandlerFactory $contentHandlerFactory,
 		RevisionStore $revisionStore,
@ -92,9 +95,9 @@ class PageCommandFactory implements MergeHistoryFactory, MovePageFactory {

 		$this->options = $options;
 		$this->loadBalancer = $loadBalancer;
-		$this->nsInfo = $nsInfo;
+		$this->namespaceInfo = $namespaceInfo;
 		$this->watchedItemStore = $watchedItemStore;
-		$this->permManager = $permManager;
+		$this->permissionManager = $permissionManager;
 		$this->repoGroup = $repoGroup;
 		$this->contentHandlerFactory = $contentHandlerFactory;
 		$this->revisionStore = $revisionStore;
@ -102,6 +105,28 @@ class PageCommandFactory implements MergeHistoryFactory, MovePageFactory {
 		$this->hookContainer = $hookContainer;
 	}

+	/**
+	 * @param User $user
+	 * @param WikiPage $wikipage
+	 * @param string $newContentModel
+	 * @return ContentModelChange
+	 */
+	public function newContentModelChange(
+		User $user,
+		WikiPage $wikipage,
+		string $newContentModel
+	) : ContentModelChange {
+		return new ContentModelChange(
+			$this->contentHandlerFactory,
+			$this->hookContainer,
+			$this->permissionManager,
+			$this->revisionStore,
+			$user,
+			$wikipage,
+			$newContentModel
+		);
+	}
+
 	/**
 	 * @param Title $source
 	 * @param Title $destination
@ -122,7 +147,7 @@ class PageCommandFactory implements MergeHistoryFactory, MovePageFactory {
 			$destination,
 			$timestamp,
 			$this->loadBalancer,
-			$this->permManager,
+			$this->permissionManager,
 			$this->contentHandlerFactory,
 			$this->revisionStore,
 			$this->watchedItemStore,
@ -141,9 +166,9 @@ class PageCommandFactory implements MergeHistoryFactory, MovePageFactory {
 			$to,
 			$this->options,
 			$this->loadBalancer,
-			$this->nsInfo,
+			$this->namespaceInfo,
 			$this->watchedItemStore,
-			$this->permManager,
+			$this->permissionManager,
 			$this->repoGroup,
 			$this->contentHandlerFactory,
 			$this->revisionStore,
--- a/includes/specials/SpecialChangeContentModel.php
+++ b/includes/specials/SpecialChangeContentModel.php
@ -208,12 +208,13 @@ class SpecialChangeContentModel extends FormSpecialPage {
 		$this->title = Title::newFromText( $data['pagetitle'] );
 		$page = WikiPage::factory( $this->title );

-		$changer = new ContentModelChange(
-			$user,
-			MediaWikiServices::getInstance()->getPermissionManager(),
-			$page,
-			$data['model']
-		);
+		$changer = MediaWikiServices::getInstance()
+			->getContentModelChangeFactory()
+			->newContentModelChange(
+				$user,
+				$page,
+				$data['model']
+			);

 		$errors = $changer->checkPermissions();
 		if ( $errors ) {
--- a/tests/phpunit/includes/content/ContentModelChangeTest.php
+++ b/tests/phpunit/includes/content/ContentModelChangeTest.php
@ -4,7 +4,10 @@ use MediaWiki\MediaWikiServices;
 use MediaWiki\Permissions\PermissionManager;

 /**
+ * TODO convert to a pure unit test
+ *
 * @group Database
+ *
 * @author DannyS712
 */
 class ContentModelChangeTest extends MediaWikiTestCase {
@ -41,6 +44,16 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 		] );
 	}

+	private function newContentModelChange(
+		User $user,
+		WikiPage $page,
+		string $newModel
+	) {
+		return MediaWikiServices::getInstance()
+			->getContentModelChangeFactory()
+			->newContentModelChange( $user, $page, $newModel );
+	}
+
 	/**
 	 * Test that the content model needs to change
 	 *
@ -54,9 +67,8 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 			'Sanity check: `ExistingPage` should be wikitext'
 		);

-		$change = new ContentModelChange(
+		$change = $this->newContentModelChange(
 			$this->getTestUser( [ 'editcontentmodel' ] )->getUser(),
-			MediaWikiServices::getInstance()->getPermissionManager(),
 			$wikipage,
 			'wikitext'
 		);
@ -92,9 +104,8 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 			'Sanity check: `PageWithTextThatIsNotValidJSON` should be wikitext at first'
 		);

-		$change = new ContentModelChange(
+		$change = $this->newContentModelChange(
 			$this->getTestUser( [ 'editcontentmodel' ] )->getUser(),
-			MediaWikiServices::getInstance()->getPermissionManager(),
 			$wikipage,
 			'json'
 		);
@ -135,9 +146,8 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 			}
 		);

-		$change = new ContentModelChange(
+		$change = $this->newContentModelChange(
 			$this->getTestUser( [ 'editcontentmodel' ] )->getUser(),
-			MediaWikiServices::getInstance()->getPermissionManager(),
 			$wikipage,
 			'text'
 		);
@ -186,9 +196,8 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 			}
 		);

-		$change = new ContentModelChange(
+		$change = $this->newContentModelChange(
 			$this->getTestUser( [ 'editcontentmodel' ] )->getUser(),
-			MediaWikiServices::getInstance()->getPermissionManager(),
 			$wikipage,
 			'text'
 		);
@ -230,9 +239,8 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 			'Dummy:NoDirectEditing should start with the `testing` content model'
 		);

-		$change = new ContentModelChange(
+		$change = $this->newContentModelChange(
 			$this->getTestUser( [ 'editcontentmodel' ] )->getUser(),
-			MediaWikiServices::getInstance()->getPermissionManager(),
 			$wikipage,
 			'text'
 		);
@ -256,9 +264,8 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 	public function testCannotApplyTags() {
 		ChangeTags::defineTag( 'edit content model tag' );

-		$change = new ContentModelChange(
+		$change = $this->newContentModelChange(
 			$this->getTestUser( [ 'noapplychangetags' ] )->getUser(),
-			MediaWikiServices::getInstance()->getPermissionManager(),
 			$this->getExistingTestPage( 'ExistingPage' ),
 			'text'
 		);
@ -342,9 +349,13 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 				)
 			);

+		$services = MediaWikiServices::getInstance();
 		$change = new ContentModelChange(
-			$user,
+			$services->getContentHandlerFactory(),
+			$services->getHookContainer(),
 			$mock,
+			$services->getRevisionLookup(),
+			$user,
 			$wikipage,
 			$newContentModel
 		);
@ -372,9 +383,8 @@ class ContentModelChangeTest extends MediaWikiTestCase {
 			->with( $this->equalTo( 'editcontentmodel' ) )
 			->willReturn( true );

-		$change = new ContentModelChange(
+		$change = $this->newContentModelChange(
 			$mock,
-			MediaWikiServices::getInstance()->getPermissionManager(),
 			$this->getNonexistingTestPage( 'NonExistingPage' ),
 			'text'
 		);