From f17213c72fb81e1c26f3d208bdbcbdcff7c48530 Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Sun, 8 Apr 2018 22:00:32 +0300 Subject: [PATCH] Test ApiMove One bug caught: the watch and unwatch params were not being honored. They have now been removed. Depends-On: Ia21a974f2b463afc9324182137b95c80db86a6aa Change-Id: I0e214339c9ae3f0fb5a40c88a84190bc32503151 (cherry picked from commit 80c7158f06b61acf5e8d8f46ee45c4c49829837b) --- RELEASE-NOTES-1.31 | 3 + includes/api/ApiMove.php | 13 - tests/phpunit/includes/api/ApiMoveTest.php | 393 +++++++++++++++++++++ 3 files changed, 396 insertions(+), 13 deletions(-) create mode 100644 tests/phpunit/includes/api/ApiMoveTest.php diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31 index de2abddfc7..4af03ee726 100644 --- a/RELEASE-NOTES-1.31 +++ b/RELEASE-NOTES-1.31 @@ -136,6 +136,9 @@ production. * (T185058) The 'name' value to tgprop for action=query&list=tags has been removed. It has never made a difference in the output, the name was always returned regardless. +* The 'watch' and 'unwatch' parameters for action=move have been removed. They + were deprecated and also accidentally nonfunctional since 1.17 in 2010. Use + 'watchlist' instead. === Action API internal changes in 1.31 === * ApiBase::getProfileDBTime was removed (deprecated since 1.25) diff --git a/includes/api/ApiMove.php b/includes/api/ApiMove.php index 281456434a..f6b6b35df2 100644 --- a/includes/api/ApiMove.php +++ b/includes/api/ApiMove.php @@ -99,7 +99,6 @@ class ApiMove extends ApiBase { // a redirect to the new title. This is not safe, but what we did before was // even worse: we just determined whether a redirect should have been created, // and reported that it was created if it should have, without any checks. - // Also note that isRedirect() is unreliable because of T39209. $r['redirectcreated'] = $fromTitle->exists(); $r['moveoverredirect'] = $toTitleExists; @@ -152,10 +151,6 @@ class ApiMove extends ApiBase { $watch = 'preferences'; if ( isset( $params['watchlist'] ) ) { $watch = $params['watchlist']; - } elseif ( $params['watch'] ) { - $watch = 'watch'; - } elseif ( $params['unwatch'] ) { - $watch = 'unwatch'; } // Watch pages @@ -250,14 +245,6 @@ class ApiMove extends ApiBase { 'movetalk' => false, 'movesubpages' => false, 'noredirect' => false, - 'watch' => [ - ApiBase::PARAM_DFLT => false, - ApiBase::PARAM_DEPRECATED => true, - ], - 'unwatch' => [ - ApiBase::PARAM_DFLT => false, - ApiBase::PARAM_DEPRECATED => true, - ], 'watchlist' => [ ApiBase::PARAM_DFLT => 'preferences', ApiBase::PARAM_TYPE => [ diff --git a/tests/phpunit/includes/api/ApiMoveTest.php b/tests/phpunit/includes/api/ApiMoveTest.php new file mode 100644 index 0000000000..fb697ffd6a --- /dev/null +++ b/tests/phpunit/includes/api/ApiMoveTest.php @@ -0,0 +1,393 @@ +assertTrue( $toTitle->exists(), + "Destination {$toTitle->getPrefixedText()} does not exist" ); + + if ( in_array( 'noredirect', $opts ) ) { + $this->assertFalse( $fromTitle->exists(), + "Source {$fromTitle->getPrefixedText()} exists" ); + } else { + $this->assertTrue( $fromTitle->exists(), + "Source {$fromTitle->getPrefixedText()} does not exist" ); + $this->assertTrue( $fromTitle->isRedirect(), + "Source {$fromTitle->getPrefixedText()} is not a redirect" ); + + $target = Revision::newFromTitle( $fromTitle )->getContent()->getRedirectTarget(); + $this->assertSame( $toTitle->getPrefixedText(), $target->getPrefixedText() ); + } + + $this->assertSame( $id, $toTitle->getArticleId() ); + } + + /** + * Shortcut function to create a page and return its id. + * + * @param string $name Page to create + * @return int ID of created page + */ + protected function createPage( $name ) { + return $this->editPage( $name, 'Content' )->value['revision']->getPage(); + } + + public function testFromWithFromid() { + $this->setExpectedException( ApiUsageException::class, + 'The parameters "from" and "fromid" can not be used together.' ); + + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => 'Some page', + 'fromid' => 123, + 'to' => 'Some other page', + ] ); + } + + public function testMove() { + $name = ucfirst( __FUNCTION__ ); + + $id = $this->createPage( $name ); + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + ] ); + + $this->assertMoved( $name, "$name 2", $id ); + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + } + + public function testMoveById() { + $name = ucfirst( __FUNCTION__ ); + + $id = $this->createPage( $name ); + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'fromid' => $id, + 'to' => "$name 2", + ] ); + + $this->assertMoved( $name, "$name 2", $id ); + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + } + + public function testMoveNonexistent() { + $this->setExpectedException( ApiUsageException::class, + "The page you specified doesn't exist." ); + + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => 'Nonexistent page', + 'to' => 'Different page' + ] ); + } + + public function testMoveNonexistentId() { + $this->setExpectedException( ApiUsageException::class, + 'There is no page with ID 2147483647.' ); + + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'fromid' => pow( 2, 31 ) - 1, + 'to' => 'Different page', + ] ); + } + + public function testMoveToInvalidPageName() { + $this->setExpectedException( ApiUsageException::class, 'Bad title "[".' ); + + $name = ucfirst( __FUNCTION__ ); + $id = $this->createPage( $name ); + + try { + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => '[', + ] ); + } finally { + $this->assertSame( $id, Title::newFromText( $name )->getArticleId() ); + } + } + + // @todo File moving + + public function testPingLimiter() { + global $wgRateLimits; + + $this->setExpectedException( ApiUsageException::class, + "You've exceeded your rate limit. Please wait some time and try again." ); + + $name = ucfirst( __FUNCTION__ ); + + $this->setMwGlobals( 'wgMainCacheType', 'hash' ); + + $this->stashMwGlobals( 'wgRateLimits' ); + $wgRateLimits['move'] = [ '&can-bypass' => false, 'user' => [ 1, 60 ] ]; + + $id = $this->createPage( $name ); + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + ] ); + + $this->assertMoved( $name, "$name 2", $id ); + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + + try { + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => "$name 2", + 'to' => "$name 3", + ] ); + } finally { + $this->assertSame( $id, Title::newFromText( "$name 2" )->getArticleId() ); + $this->assertFalse( Title::newFromText( "$name 3" )->exists(), + "\"$name 3\" should not exist" ); + } + } + + public function testTagsNoPermission() { + $this->setExpectedException( ApiUsageException::class, + 'You do not have permission to apply change tags along with your changes.' ); + + $name = ucfirst( __FUNCTION__ ); + + ChangeTags::defineTag( 'custom tag' ); + + $this->setGroupPermissions( 'user', 'applychangetags', false ); + + $id = $this->createPage( $name ); + + try { + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + 'tags' => 'custom tag', + ] ); + } finally { + $this->assertSame( $id, Title::newFromText( $name )->getArticleId() ); + $this->assertFalse( Title::newFromText( "$name 2" )->exists(), + "\"$name 2\" should not exist" ); + } + } + + public function testSelfMove() { + $this->setExpectedException( ApiUsageException::class, + 'The title is the same; cannot move a page over itself.' ); + + $name = ucfirst( __FUNCTION__ ); + $this->createPage( $name ); + + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => $name, + ] ); + } + + public function testMoveTalk() { + $name = ucfirst( __FUNCTION__ ); + + $id = $this->createPage( $name ); + $talkId = $this->createPage( "Talk:$name" ); + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + 'movetalk' => '', + ] ); + + $this->assertMoved( $name, "$name 2", $id ); + $this->assertMoved( "Talk:$name", "Talk:$name 2", $talkId ); + + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + } + + public function testMoveTalkFailed() { + $name = ucfirst( __FUNCTION__ ); + + $id = $this->createPage( $name ); + $talkId = $this->createPage( "Talk:$name" ); + $talkDestinationId = $this->createPage( "Talk:$name 2" ); + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + 'movetalk' => '', + ] ); + + $this->assertMoved( $name, "$name 2", $id ); + $this->assertSame( $talkId, Title::newFromText( "Talk:$name" )->getArticleId() ); + $this->assertSame( $talkDestinationId, + Title::newFromText( "Talk:$name 2" )->getArticleId() ); + $this->assertSame( [ [ + 'message' => 'articleexists', + 'params' => [], + 'code' => 'articleexists', + 'type' => 'error', + ] ], $res[0]['move']['talkmove-errors'] ); + + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + } + + public function testMoveSubpages() { + global $wgNamespacesWithSubpages; + + $name = ucfirst( __FUNCTION__ ); + + $this->stashMwGlobals( 'wgNamespacesWithSubpages' ); + $wgNamespacesWithSubpages[NS_MAIN] = true; + + $pages = [ $name, "$name/1", "$name/2", "Talk:$name", "Talk:$name/1", "Talk:$name/3" ]; + $ids = []; + foreach ( array_merge( $pages, [ "$name/error", "$name 2/error" ] ) as $page ) { + $ids[$page] = $this->createPage( $page ); + } + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + 'movetalk' => '', + 'movesubpages' => '', + ] ); + + foreach ( $pages as $page ) { + $this->assertMoved( $page, str_replace( $name, "$name 2", $page ), $ids[$page] ); + } + + $this->assertSame( $ids["$name/error"], + Title::newFromText( "$name/error" )->getArticleId() ); + $this->assertSame( $ids["$name 2/error"], + Title::newFromText( "$name 2/error" )->getArticleId() ); + + $results = array_merge( $res[0]['move']['subpages'], $res[0]['move']['subpages-talk'] ); + foreach ( $results as $arr ) { + if ( $arr['from'] === "$name/error" ) { + $this->assertSame( [ [ + 'message' => 'articleexists', + 'params' => [], + 'code' => 'articleexists', + 'type' => 'error' + ] ], $arr['errors'] ); + } else { + $this->assertSame( str_replace( $name, "$name 2", $arr['from'] ), $arr['to'] ); + } + $this->assertCount( 2, $arr ); + } + + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + } + + public function testMoveNoPermission() { + $this->setExpectedException( ApiUsageException::class, + 'You must be a registered user and [[Special:UserLogin|logged in]] to move a page.' ); + + $name = ucfirst( __FUNCTION__ ); + + $id = $this->createPage( $name ); + + $user = new User(); + + try { + $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + ], null, $user ); + } finally { + $this->assertSame( $id, Title::newFromText( "$name" )->getArticleId() ); + $this->assertFalse( Title::newFromText( "$name 2" )->exists(), + "\"$name 2\" should not exist" ); + } + } + + public function testSuppressRedirect() { + $name = ucfirst( __FUNCTION__ ); + + $id = $this->createPage( $name ); + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + 'noredirect' => '', + ] ); + + $this->assertMoved( $name, "$name 2", $id, 'noredirect' ); + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + } + + public function testSuppressRedirectNoPermission() { + $name = ucfirst( __FUNCTION__ ); + + $this->setGroupPermissions( 'sysop', 'suppressredirect', false ); + + $id = $this->createPage( $name ); + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => $name, + 'to' => "$name 2", + 'noredirect' => '', + ] ); + + $this->assertMoved( $name, "$name 2", $id ); + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + } + + public function testMoveSubpagesError() { + $name = ucfirst( __FUNCTION__ ); + + // Subpages are allowed in talk but not main + $idBase = $this->createPage( "Talk:$name" ); + $idSub = $this->createPage( "Talk:$name/1" ); + + $res = $this->doApiRequestWithToken( [ + 'action' => 'move', + 'from' => "Talk:$name", + 'to' => $name, + 'movesubpages' => '', + ] ); + + $this->assertMoved( "Talk:$name", $name, $idBase ); + $this->assertSame( $idSub, Title::newFromText( "Talk:$name/1" )->getArticleId() ); + $this->assertFalse( Title::newFromText( "$name/1" )->exists(), + "\"$name/1\" should not exist" ); + + $this->assertSame( [ 'errors' => [ [ + 'message' => 'namespace-nosubpages', + 'params' => [ '' ], + 'code' => 'namespace-nosubpages', + 'type' => 'error', + ] ] ], $res[0]['move']['subpages'] ); + + $this->assertArrayNotHasKey( 'warnings', $res[0] ); + } +} -- 2.20.1