From e0414f962e321b12e5363cd0c7c7a6942d89223a Mon Sep 17 00:00:00 2001 From: MusikAnimal Date: Tue, 3 Mar 2020 19:14:09 -0500 Subject: [PATCH] ApiWatch: Add support for expiries With this commit, the action=watch API accepts an 'expiry' parameter, analagous to the expiry accepted by action=userrights, action=block, etc. Bug: T245078 Change-Id: If37a772253082529cb290027da87098c1e6bf98c --- includes/api/ApiWatch.php | 70 ++++++++++++- includes/api/i18n/en.json | 2 + includes/api/i18n/qqq.json | 2 + includes/watcheditem/WatchedItem.php | 26 +++++ includes/watcheditem/WatchedItemStore.php | 16 +-- tests/phpunit/includes/api/ApiWatchTest.php | 98 ++++++++++++++++++- .../watcheditem/WatchedItemUnitTest.php | 12 +++ 7 files changed, 208 insertions(+), 18 deletions(-) diff --git a/includes/api/ApiWatch.php b/includes/api/ApiWatch.php index ce1408f4cba..8083e05e336 100644 --- a/includes/api/ApiWatch.php +++ b/includes/api/ApiWatch.php @@ -20,6 +20,8 @@ * @file */ +use Wikimedia\ParamValidator\ParamValidator; + /** * API module to allow users to watch a page * @@ -28,6 +30,15 @@ class ApiWatch extends ApiBase { private $mPageSet = null; + /** @var bool Whether watchlist expiries are enabled. */ + private $expiryEnabled; + + public function __construct( ApiMain $mainModule, $moduleName, $modulePrefix = '' ) { + parent::__construct( $mainModule, $moduleName, $modulePrefix ); + + $this->expiryEnabled = $this->getConfig()->get( 'WatchlistExpiry' ); + } + public function execute() { $user = $this->getUser(); if ( !$user->isLoggedIn() ) { @@ -94,6 +105,30 @@ class ApiWatch extends ApiBase { $continuationManager->setContinuationIntoResult( $this->getResult() ); } + /** + * Normalize the expiry into TS_MW format, or throw errors if it is invalid. + * @param string $expiryParam + * @return string + */ + private function normalizeAndValidateExpiry( string $expiryParam ): string { + $expiry = WatchedItem::normalizeExpiry( $expiryParam ); + + if ( $expiry === false ) { + $this->dieWithError( [ + 'apierror-invalidexpiry', + Message::plaintextParam( $expiryParam ) + ] ); + } + if ( $expiry < wfTimestampNow() ) { + $this->dieWithError( [ + 'apierror-pastexpiry', + Message::plaintextParam( $expiryParam ) + ] ); + } + + return $expiry; + } + private function watchTitle( Title $title, User $user, array $params, $compatibilityMode = false ) { @@ -108,7 +143,15 @@ class ApiWatch extends ApiBase { $status = UnwatchAction::doUnwatch( $title, $user ); $res['unwatched'] = $status->isOK(); } else { - $status = WatchAction::doWatch( $title, $user ); + $expiry = null; + + // NOTE: If an expiry parameter isn't given, any existing expiries remain unchanged. + if ( $this->expiryEnabled && isset( $params['expiry'] ) ) { + $expiry = $this->normalizeAndValidateExpiry( $params['expiry'] ); + $res['expiry'] = ApiResult::formatExpiry( $expiry ); + } + + $status = WatchAction::doWatch( $title, $user, User::CHECK_USER_RIGHTS, $expiry ); $res['watched'] = $status->isOK(); } @@ -153,14 +196,23 @@ class ApiWatch extends ApiBase { public function getAllowedParams( $flags = 0 ) { $result = [ 'title' => [ - ApiBase::PARAM_TYPE => 'string', - ApiBase::PARAM_DEPRECATED => true + ParamValidator::PARAM_TYPE => 'string', + ParamValidator::PARAM_DEPRECATED => true, + ], + 'expiry' => [ + ParamValidator::PARAM_TYPE => 'string', ], 'unwatch' => false, 'continue' => [ ApiBase::PARAM_HELP_MSG => 'api-help-param-continue', ], ]; + + // If expiry is not enabled, don't accept the parameter. + if ( !$this->expiryEnabled ) { + unset( $result['expiry'] ); + } + if ( $flags ) { $result += $this->getPageSet()->getFinalParams( $flags ); } @@ -169,14 +221,22 @@ class ApiWatch extends ApiBase { } protected function getExamplesMessages() { - return [ + // Logically expiry example should go before unwatch examples. + $examples = [ 'action=watch&titles=Main_Page&token=123ABC' => 'apihelp-watch-example-watch', + ]; + if ( $this->expiryEnabled ) { + $examples['action=watch&titles=Main_Page|Foo|Bar&expiry=1%20month&token=123ABC'] + = 'apihelp-watch-example-watch-expiry'; + } + + return array_merge( $examples, [ 'action=watch&titles=Main_Page&unwatch=&token=123ABC' => 'apihelp-watch-example-unwatch', 'action=watch&generator=allpages&gapnamespace=0&token=123ABC' => 'apihelp-watch-example-generator', - ]; + ] ); } public function getHelpUrls() { diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json index 5256f353595..6ce56c1294b 100644 --- a/includes/api/i18n/en.json +++ b/includes/api/i18n/en.json @@ -1577,8 +1577,10 @@ "apihelp-watch-summary": "Add or remove pages from the current user's watchlist.", "apihelp-watch-param-title": "The page to (un)watch. Use $1titles instead.", + "apihelp-watch-param-expiry": "Expiry timestamp to be applied to all given pages. May be relative (e.g. 5 months or 2 weeks) or absolute (e.g. 2014-09-18T12:34:56Z). Use infinite, indefinite, infinity, or never to watch indefinitely. Omit this parameter entirely to leave any current expiries unchanged.", "apihelp-watch-param-unwatch": "If set the page will be unwatched rather than watched.", "apihelp-watch-example-watch": "Watch the page Main Page.", + "apihelp-watch-example-watch-expiry": "Watch the pages Main Page, Foo, and Bar for one month.", "apihelp-watch-example-unwatch": "Unwatch the page Main Page.", "apihelp-watch-example-generator": "Watch the first few pages in the main namespace.", diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json index 620a564e0e3..df7ee60d9fc 100644 --- a/includes/api/i18n/qqq.json +++ b/includes/api/i18n/qqq.json @@ -1472,8 +1472,10 @@ "apihelp-validatepassword-example-2": "{{doc-apihelp-example|validatepassword}}", "apihelp-watch-summary": "{{doc-apihelp-summary|watch}}", "apihelp-watch-param-title": "{{doc-apihelp-param|watch|title}}", + "apihelp-watch-param-expiry": "{{doc-apihelp-param|watch|expiry}}", "apihelp-watch-param-unwatch": "{{doc-apihelp-param|watch|unwatch}}", "apihelp-watch-example-watch": "{{doc-apihelp-example|watch}}", + "apihelp-watch-example-watch-expiry": "{{doc-apihelp-example|watch}}", "apihelp-watch-example-unwatch": "{{doc-apihelp-example|watch}}", "apihelp-watch-example-generator": "{{doc-apihelp-example|watch}}", "apihelp-format-example-generic": "{{doc-apihelp-example|format|params=* $1 - Format name|paramstart=2|noseealso=1}}", diff --git a/includes/watcheditem/WatchedItem.php b/includes/watcheditem/WatchedItem.php index fa1372672df..45dd24ecc4d 100644 --- a/includes/watcheditem/WatchedItem.php +++ b/includes/watcheditem/WatchedItem.php @@ -126,4 +126,30 @@ class WatchedItem { $unix = MWTimestamp::convert( TS_UNIX, $this->getExpiry() ); return $unix < wfTimestamp(); } + + /** + * Normalize a user-inputted expiry. + * @since 1.35 + * @param string|null $expiry + * @return string|false|null Timestamp as TS_MW, 'infinity', false if invalid, or null + * when given null (which for the watchlist means the expiry should remain unchanged). + * @todo This logic should really be refactored with UserrightsPage::expiryToTimestamp(), + * along with the expiry logic used for blocking and page protection. + */ + public static function normalizeExpiry( ?string $expiry ) { + if ( $expiry === null ) { + return null; + } + if ( wfIsInfinity( $expiry ) ) { + return 'infinity'; + } + + $unix = strtotime( $expiry ); + if ( !$unix || $unix === -1 ) { + // Invalid expiry. + return false; + } + + return wfTimestamp( TS_MW, $unix ); + } } diff --git a/includes/watcheditem/WatchedItemStore.php b/includes/watcheditem/WatchedItemStore.php index c848b4f18d5..0a93ff057b8 100644 --- a/includes/watcheditem/WatchedItemStore.php +++ b/includes/watcheditem/WatchedItemStore.php @@ -897,8 +897,10 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac array $rows, ?string $expiry = null ): int { - if ( null === $expiry ) { - // null expiry means do nothing, 0 rows affected. + $expiry = WatchedItem::normalizeExpiry( $expiry ); + + if ( !$expiry ) { + // Either expiry was invalid or null (shouldn't change), 0 rows affected. return 0; } @@ -930,16 +932,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return $dbw->affectedRows(); } - // TODO: Refactor with Special:Userrights::expiryToTimestamp() ? - // Difference here is we need to accept null as being 'no changes'. - // Validation of the expiry should probably happen before this method is called, too. - $unix = strtotime( $expiry ); - if ( !$unix || $unix === -1 ) { - // Invalid expiry, 0 rows effected. - return 0; - } - $expiry = wfTimestamp( TS_MW, $unix ); - return $this->updateExpiries( $dbw, $expiry, $cond ); } diff --git a/tests/phpunit/includes/api/ApiWatchTest.php b/tests/phpunit/includes/api/ApiWatchTest.php index 0152f8b74be..1924b830013 100644 --- a/tests/phpunit/includes/api/ApiWatchTest.php +++ b/tests/phpunit/includes/api/ApiWatchTest.php @@ -1,18 +1,114 @@ setMwGlobals( [ + 'wgWatchlistExpiry' => true, + ] ); + } + protected function getTokens() { return $this->getTokenList( self::$users['sysop'] ); } + public function testWatch() { + $data = $this->doApiRequestWithToken( [ + 'action' => 'watch', + 'titles' => 'Talk:Test page', + 'expiry' => '99990101000000', + 'formatversion' => 2 + ] ); + + $res = $data[0]['watch'][0]; + $this->assertSame( 'Talk:Test page', $res['title'] ); + $this->assertSame( 1, $res['ns'] ); + $this->assertSame( '9999-01-01T00:00:00Z', $res['expiry'] ); + $this->assertTrue( $res['watched'] ); + + // Re-watch, changing the expiry to indefinite. + $data = $this->doApiRequestWithToken( [ + 'action' => 'watch', + 'titles' => 'Talk:Test page', + 'expiry' => 'indefinite', + 'formatversion' => 2 + ] ); + $res = $data[0]['watch'][0]; + $this->assertSame( 'infinity', $res['expiry'] ); + } + + public function testWatchWithExpiry() { + $store = MediaWikiServices::getInstance()->getWatchedItemStore(); + $user = $this->getTestUser()->getUser(); + + // First watch without expiry (indefinite). + $this->doApiRequestWithToken( [ + 'action' => 'watch', + 'titles' => 'UTPage', + ], null, $user ); + + // Ensure page was added to the user's watchlist, and expiry is null (not set). + [ $item ] = $store->getWatchedItemsForUser( $user ); + $this->assertSame( 'UTPage', $item->getLinkTarget()->getDBkey() ); + $this->assertNull( $item->getExpiry() ); + + // Re-watch, setting an expiry. + $expiry = '2 weeks'; + $expectedExpiry = strtotime( $expiry ); + $this->doApiRequestWithToken( [ + 'action' => 'watch', + 'titles' => 'UTPage', + 'expiry' => $expiry, + ], null, $user ); + [ $item ] = $store->getWatchedItemsForUser( $user ); + $this->assertGreaterThanOrEqual( + $expectedExpiry, + wfTimestamp( TS_UNIX, $item->getExpiry() ) + ); + + // Re-watch again, providing no expiry parameter, so expiry should remain unchanged. + $oldExpiry = $item->getExpiry(); + $this->doApiRequestWithToken( [ + 'action' => 'watch', + 'titles' => 'UTPage', + ], null, $user ); + [ $item ] = $store->getWatchedItemsForUser( $user ); + $this->assertSame( $oldExpiry, $item->getExpiry() ); + } + + public function testWatchInvalidExpiry() { + $this->expectException( ApiUsageException::class ); + $this->expectExceptionMessage( 'Invalid expiry time "invalid expiry".' ); + $this->doApiRequestWithToken( [ + 'action' => 'watch', + 'titles' => 'Talk:Test page', + 'expiry' => 'invalid expiry', + 'formatversion' => 2 + ] ); + } + + public function testWatchExpiryInPast() { + $this->expectException( ApiUsageException::class ); + $this->expectExceptionMessage( 'Expiry time "20010101000000" is in the past.' ); + $this->doApiRequestWithToken( [ + 'action' => 'watch', + 'titles' => 'Talk:Test page', + 'expiry' => '20010101000000', + 'formatversion' => 2 + ] ); + } + public function testWatchEdit() { $tokens = $this->getTokens(); diff --git a/tests/phpunit/includes/watcheditem/WatchedItemUnitTest.php b/tests/phpunit/includes/watcheditem/WatchedItemUnitTest.php index 0c5f7d53382..c1b8efd0c97 100644 --- a/tests/phpunit/includes/watcheditem/WatchedItemUnitTest.php +++ b/tests/phpunit/includes/watcheditem/WatchedItemUnitTest.php @@ -20,4 +20,16 @@ class WatchedItemUnitTest extends MediaWikiTestCase { $expired = new WatchedItem( $user, $target, null, '20010101000000' ); $this->assertTrue( $expired->isExpired() ); } + + public function testNormalizeExpiry() { + $this->assertNull( WatchedItem::normalizeExpiry( null ) ); + $this->assertSame( + 'infinity', + WatchedItem::normalizeExpiry( 'indefinite' ) + ); + $this->assertSame( + '20500101000000', + WatchedItem::normalizeExpiry( '2050-01-01 00:00' ) + ); + } }