From: Kunal Mehta Date: Fri, 19 Sep 2014 19:10:53 +0000 (-0700) Subject: Move non-user specific things from Title::isValidMoveOperation() to MovePage X-Git-Tag: 1.31.0-rc.0~13862^2 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/exercices/modifier.php?a=commitdiff_plain;h=39fffcedb05ba9cb504fb493a7898588d3806e2b;p=lhc%2Fweb%2Fwiklou.git Move non-user specific things from Title::isValidMoveOperation() to MovePage Change-Id: Ieffeb0c7a15b202dcbdaf2a9d0b9bcdc10e360d2 --- diff --git a/includes/MovePage.php b/includes/MovePage.php index fdece8d5be..cea91d264b 100644 --- a/includes/MovePage.php +++ b/includes/MovePage.php @@ -42,6 +42,90 @@ class MovePage { $this->newTitle = $newTitle; } + /** + * Does various sanity checks that the move is + * valid. Only things based on the two titles + * should be checked here. + * + * @return Status + */ + public function isValidMove() { + global $wgContentHandlerUseDB; + $status = new Status(); + + if ( $this->oldTitle->equals( $this->newTitle ) ) { + $status->fatal( 'selfmove' ); + } + if ( !$this->oldTitle->isMovable() ) { + $status->fatal( 'immobile-source-namespace', $this->oldTitle->getNsText() ); + } + if ( $this->newTitle->isExternal() ) { + $status->fatal( 'immobile-target-namespace-iw' ); + } + if ( !$this->newTitle->isMovable() ) { + $status->fatal( 'immobile-target-namespace', $this->newTitle->getNsText() ); + } + + $oldid = $this->oldTitle->getArticleID(); + + if ( strlen( $this->newTitle->getDBkey() ) < 1 ) { + $errors[] = array( 'articleexists' ); + } + if ( + ( $this->oldTitle->getDBkey() == '' ) || + ( !$oldid ) || + ( $this->newTitle->getDBkey() == '' ) + ) { + $errors[] = array( 'badarticleerror' ); + } + + // Content model checks + if ( !$wgContentHandlerUseDB && + $this->oldTitle->getContentModel() !== $this->newTitle->getContentModel() ) { + // can't move a page if that would change the page's content model + $status->fatal( + 'bad-target-model', + ContentHandler::getLocalizedName( $this->oldTitle->getContentModel() ), + ContentHandler::getLocalizedName( $this->newTitle->getContentModel() ) + ); + } + + // Image-specific checks + if ( $this->oldTitle->inNamespace( NS_FILE ) ) { + $status->merge( $this->isValidFileMove() ); + } + + if ( $this->newTitle->inNamespace( NS_FILE ) && !$this->oldTitle->inNamespace( NS_FILE ) ) { + $status->fatal( 'nonfile-cannot-move-to-file' ); + } + + return $status; + } + + /** + * Sanity checks for when a file is being moved + * + * @return Status + */ + protected function isValidFileMove() { + $status = new Status(); + $file = wfLocalFile( $this->oldTitle ); + if ( $file->exists() ) { + if ( $this->newTitle->getText() != wfStripIllegalFilenameChars( $this->newTitle->getText() ) ) { + $status->fatal( 'imageinvalidfilename' ); + } + if ( !File::checkExtensionCompatibility( $file, $this->newTitle->getDBkey() ) ) { + $status->fatal( 'imagetypemismatch' ); + } + } + + if ( !$this->newTitle->inNamespace( NS_FILE ) ) { + $status->fatal( 'imagenocrossnamespace' ); + } + + return $status; + } + /** * @param User $user * @param string $reason diff --git a/includes/Title.php b/includes/Title.php index 74d78ba591..e8cda8530f 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -3589,7 +3589,7 @@ class Title { * Check whether a given move operation would be valid. * Returns true if ok, or a getUserPermissionsErrors()-like array otherwise * - * @todo move this into MovePage + * @todo finish moving this into MovePage * @param Title $nt The new title * @param bool $auth Indicates whether $wgUser's permissions * should be checked @@ -3597,60 +3597,18 @@ class Title { * @return array|bool True on success, getUserPermissionsErrors()-like array on failure */ public function isValidMoveOperation( &$nt, $auth = true, $reason = '' ) { - global $wgUser, $wgContentHandlerUseDB; + global $wgUser; - $errors = array(); - if ( !$nt ) { + if ( !( $nt instanceof Title ) ) { // Normally we'd add this to $errors, but we'll get // lots of syntax errors if $nt is not an object return array( array( 'badtitletext' ) ); } - if ( $this->equals( $nt ) ) { - $errors[] = array( 'selfmove' ); - } - if ( !$this->isMovable() ) { - $errors[] = array( 'immobile-source-namespace', $this->getNsText() ); - } - if ( $nt->isExternal() ) { - $errors[] = array( 'immobile-target-namespace-iw' ); - } - if ( !$nt->isMovable() ) { - $errors[] = array( 'immobile-target-namespace', $nt->getNsText() ); - } - - $oldid = $this->getArticleID(); - $newid = $nt->getArticleID(); - - if ( strlen( $nt->getDBkey() ) < 1 ) { - $errors[] = array( 'articleexists' ); - } - if ( - ( $this->getDBkey() == '' ) || - ( !$oldid ) || - ( $nt->getDBkey() == '' ) - ) { - $errors[] = array( 'badarticleerror' ); - } - - // Content model checks - if ( !$wgContentHandlerUseDB && - $this->getContentModel() !== $nt->getContentModel() ) { - // can't move a page if that would change the page's content model - $errors[] = array( - 'bad-target-model', - ContentHandler::getLocalizedName( $this->getContentModel() ), - ContentHandler::getLocalizedName( $nt->getContentModel() ) - ); - } - // Image-specific checks - if ( $this->getNamespace() == NS_FILE ) { - $errors = array_merge( $errors, $this->validateFileMoveOperation( $nt ) ); - } + $mp = new MovePage( $this, $nt ); + $errors = $mp->isValidMove()->getErrorsArray(); - if ( $nt->getNamespace() == NS_FILE && $this->getNamespace() != NS_FILE ) { - $errors[] = array( 'nonfile-cannot-move-to-file' ); - } + $newid = $nt->getArticleID(); if ( $auth ) { $errors = wfMergeErrorArrays( $errors, @@ -3700,6 +3658,7 @@ class Title { /** * Check if the requested move target is a valid file move target + * @todo move this to MovePage * @param Title $nt Target title * @return array List of errors */ @@ -3708,27 +3667,6 @@ class Title { $errors = array(); - // wfFindFile( $nt ) / wfLocalFile( $nt ) is not allowed until below - - $file = wfLocalFile( $this ); - if ( $file->exists() ) { - if ( $nt->getText() != wfStripIllegalFilenameChars( $nt->getText() ) ) { - $errors[] = array( 'imageinvalidfilename' ); - } - if ( !File::checkExtensionCompatibility( $file, $nt->getDBkey() ) ) { - $errors[] = array( 'imagetypemismatch' ); - } - } - - if ( $nt->getNamespace() != NS_FILE ) { - $errors[] = array( 'imagenocrossnamespace' ); - // From here we want to do checks on a file object, so if we can't - // create one, we must return. - return $errors; - } - - // wfFindFile( $nt ) / wfLocalFile( $nt ) is allowed below here - $destFile = wfLocalFile( $nt ); if ( !$wgUser->isAllowed( 'reupload-shared' ) && !$destFile->exists() && wfFindFile( $nt ) ) { $errors[] = array( 'file-exists-sharedrepo' ); @@ -3899,6 +3837,7 @@ class Title { * Checks if $this can be moved to a given Title * - Selects for update, so don't call it unless you mean business * + * @todo move to MovePage * @param Title $nt The new title to check * @return bool */ diff --git a/tests/phpunit/includes/MovePageTest.php b/tests/phpunit/includes/MovePageTest.php new file mode 100644 index 0000000000..027b877ce4 --- /dev/null +++ b/tests/phpunit/includes/MovePageTest.php @@ -0,0 +1,39 @@ +setMwGlobals( 'wgContentHandlerUseDB', false ); + $mp = new MovePage( + Title::newFromText( $old ), + Title::newFromText( $new ) + ); + $status = $mp->isValidMove(); + if ( $error === true ) { + $this->assertTrue( $status->isGood() ); + } else { + $this->assertTrue( $status->hasMessage( $error ) ); + } + } + + /** + * This should be kept in sync with TitleTest::provideTestIsValidMoveOperation + */ + public static function provideIsValidMove() { + return array( + // for MovePage::isValidMove + array( 'Test', 'Test', 'selfmove' ), + array( 'Special:FooBar', 'Test', 'immobile-source-namespace' ), + array( 'Test', 'Special:FooBar', 'immobile-target-namespace' ), + array( 'MediaWiki:Common.js', 'Help:Some wikitext page', 'bad-target-model' ), + array( 'Page', 'File:Test.jpg', 'nonfile-cannot-move-to-file' ), + // for MovePage::isValidFileMove + array( 'File:Test.jpg', 'Page', 'imagenocrossnamespace' ), + ); + } +}