From d9534a5d5685c05589639d43cdc1e1195bb7cbba Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 24 Dec 2013 15:26:47 -0500 Subject: [PATCH] API: Make more continuations unique API queries must be completely ordered for proper behavior; otherwise you may get into a situation where a query returns the same continuation value that was provided. Various modules that have been using timestamps in/as their continuation parameter can easily run into this problem. Normally we'd have to add additional fields to the relevant indexes to be able to make this work without having filesorting queries (which MySQL really doesn't do well, it fetches all matching rows and only applies the limit after[1]). But InnoDB has a "feature" where it effectively appends the table's primary key to all other indexes,[2] which makes these queries be properly indexed in that situation. Apparently we're ok with this, since Icc43b62f was merged depending on this feature. Also, this change fixes some MySQLisms and other oddities done to ApiQueryRecentChanges in Icc43b62f. [1]: https://dev.mysql.com/doc/refman/5.5/en/limit-optimization.html [2]: https://dev.mysql.com/doc/refman/5.5/en/innodb-table-and-index.html Bug: 24782 Change-Id: I4c9f8c0c2bfd831755d4fa20a18f93fef1effd28 --- RELEASE-NOTES-1.23 | 1 + includes/api/ApiQueryAllImages.php | 18 ++++++- includes/api/ApiQueryBlocks.php | 27 +++++++--- includes/api/ApiQueryCategoryMembers.php | 20 +++++++- includes/api/ApiQueryDeletedrevs.php | 60 ++++++++++++++-------- includes/api/ApiQueryLogEvents.php | 26 ++++++++-- includes/api/ApiQueryProtectedTitles.php | 32 ++++++++++-- includes/api/ApiQueryRecentChanges.php | 23 +++------ includes/api/ApiQueryUserContributions.php | 58 +++++++++++++-------- includes/api/ApiQueryWatchlist.php | 27 +++++++--- 10 files changed, 211 insertions(+), 81 deletions(-) diff --git a/RELEASE-NOTES-1.23 b/RELEASE-NOTES-1.23 index 947a0d611e..ce3168ef9a 100644 --- a/RELEASE-NOTES-1.23 +++ b/RELEASE-NOTES-1.23 @@ -248,6 +248,7 @@ production. included in all searches. * Added list=prefixsearch that works like action=opensearch but can be used as a generator. +* (bug 24782) Various modules will now use unique continuation parameters. === Languages updated in 1.23 === diff --git a/includes/api/ApiQueryAllImages.php b/includes/api/ApiQueryAllImages.php index 6e2c31f55d..229c07eb79 100644 --- a/includes/api/ApiQueryAllImages.php +++ b/includes/api/ApiQueryAllImages.php @@ -168,6 +168,20 @@ class ApiQueryAllImages extends ApiQueryGeneratorBase { $params['start'], $params['end'] ); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'img_name', $ascendingOrder ? 'newer' : 'older', null, null ); + + if ( !is_null( $params['continue'] ) ) { + $cont = explode( '|', $params['continue'] ); + $this->dieContinueUsageIf( count( $cont ) != 2 ); + $op = ( $ascendingOrder ? '>' : '<' ); + $continueTimestamp = $db->addQuotes( $db->timestamp( $cont[0] ) ); + $continueName = $db->addQuotes( $cont[1] ); + $this->addWhere( "img_timestamp $op $continueTimestamp OR " . + "(img_timestamp = $continueTimestamp AND " . + "img_name $op= $continueName)" + ); + } // Image filters if ( !is_null( $params['user'] ) ) { @@ -254,7 +268,7 @@ class ApiQueryAllImages extends ApiQueryGeneratorBase { if ( $params['sort'] == 'name' ) { $this->setContinueEnumParameter( 'continue', $row->img_name ); } else { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->img_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->img_timestamp|$row->img_name" ); } break; } @@ -270,7 +284,7 @@ class ApiQueryAllImages extends ApiQueryGeneratorBase { if ( $params['sort'] == 'name' ) { $this->setContinueEnumParameter( 'continue', $row->img_name ); } else { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->img_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->img_timestamp|$row->img_name" ); } break; } diff --git a/includes/api/ApiQueryBlocks.php b/includes/api/ApiQueryBlocks.php index 6cc018321e..cfca140c58 100644 --- a/includes/api/ApiQueryBlocks.php +++ b/includes/api/ApiQueryBlocks.php @@ -43,6 +43,7 @@ class ApiQueryBlocks extends ApiQueryBase { public function execute() { global $wgContLang; + $db = $this->getDB(); $params = $this->extractRequestParams(); $this->requireMaxOneParameter( $params, 'users', 'ip' ); @@ -61,9 +62,8 @@ class ApiQueryBlocks extends ApiQueryBase { $result = $this->getResult(); $this->addTables( 'ipblocks' ); - $this->addFields( 'ipb_auto' ); + $this->addFields( array( 'ipb_auto', 'ipb_id' ) ); - $this->addFieldsIf( 'ipb_id', $fld_id ); $this->addFieldsIf( array( 'ipb_address', 'ipb_user' ), $fld_user || $fld_userid ); $this->addFieldsIf( 'ipb_by_text', $fld_by ); $this->addFieldsIf( 'ipb_by', $fld_byid ); @@ -82,8 +82,21 @@ class ApiQueryBlocks extends ApiQueryBase { $params['start'], $params['end'] ); - - $db = $this->getDB(); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'ipb_id', $params['dir'], null, null ); + + if ( !is_null( $params['continue'] ) ) { + $cont = explode( '|', $params['continue'] ); + $this->dieContinueUsageIf( count( $cont ) != 2 ); + $op = ( $params['dir'] == 'newer' ? '>' : '<' ); + $continueTimestamp = $db->addQuotes( $db->timestamp( $cont[0] ) ); + $continueId = (int)$cont[1]; + $this->dieContinueUsageIf( $continueId != $cont[1] ); + $this->addWhere( "ipb_timestamp $op $continueTimestamp OR " . + "(ipb_timestamp = $continueTimestamp AND " . + "ipb_id $op= $continueId)" + ); + } if ( isset( $params['ids'] ) ) { $this->addWhereFld( 'ipb_id', $params['ids'] ); @@ -176,7 +189,7 @@ class ApiQueryBlocks extends ApiQueryBase { foreach ( $res as $row ) { if ( ++$count > $params['limit'] ) { // We've had enough - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->ipb_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->ipb_timestamp|$row->ipb_id" ); break; } $block = array(); @@ -234,7 +247,7 @@ class ApiQueryBlocks extends ApiQueryBase { } $fit = $result->addValue( array( 'query', $this->getModuleName() ), null, $block ); if ( !$fit ) { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->ipb_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->ipb_timestamp|$row->ipb_id" ); break; } } @@ -313,6 +326,7 @@ class ApiQueryBlocks extends ApiQueryBase { ), ApiBase::PARAM_ISMULTI => true ), + 'continue' => null, ); } @@ -350,6 +364,7 @@ class ApiQueryBlocks extends ApiQueryBase { 'Show only items that meet this criteria.', "For example, to see only indefinite blocks on IPs, set {$p}show=ip|!temp" ), + 'continue' => 'When more results are available, use this to continue', ); } diff --git a/includes/api/ApiQueryCategoryMembers.php b/includes/api/ApiQueryCategoryMembers.php index 4e942d6d19..424770fc5c 100644 --- a/includes/api/ApiQueryCategoryMembers.php +++ b/includes/api/ApiQueryCategoryMembers.php @@ -101,6 +101,22 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { $dir, $params['start'], $params['end'] ); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'cl_from', $dir, null, null ); + + if ( !is_null( $params['continue'] ) ) { + $cont = explode( '|', $params['continue'] ); + $this->dieContinueUsageIf( count( $cont ) != 2 ); + $op = ( $dir === 'newer' ? '>' : '<' ); + $db = $this->getDB(); + $continueTimestamp = $db->addQuotes( $db->timestamp( $cont[0] ) ); + $continueFrom = (int)$cont[1]; + $this->dieContinueUsageIf( $continueFrom != $cont[1] ); + $this->addWhere( "cl_timestamp $op $continueTimestamp OR " . + "(cl_timestamp = $continueTimestamp AND " . + "cl_from $op= $continueFrom)" + ); + } $this->addOption( 'USE INDEX', 'cl_timestamp' ); } else { @@ -186,7 +202,7 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { // @todo Security issue - if the user has no right to view next // title, it will still be shown if ( $params['sort'] == 'timestamp' ) { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->cl_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->cl_timestamp|$row->cl_from" ); } else { $sortkey = bin2hex( $row->cl_sortkey ); $this->setContinueEnumParameter( 'continue', @@ -229,7 +245,7 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase { null, $vals ); if ( !$fit ) { if ( $params['sort'] == 'timestamp' ) { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->cl_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->cl_timestamp|$row->cl_from" ); } else { $sortkey = bin2hex( $row->cl_sortkey ); $this->setContinueEnumParameter( 'continue', diff --git a/includes/api/ApiQueryDeletedrevs.php b/includes/api/ApiQueryDeletedrevs.php index 58db035ba4..2ca93f55ed 100644 --- a/includes/api/ApiQueryDeletedrevs.php +++ b/includes/api/ApiQueryDeletedrevs.php @@ -106,7 +106,7 @@ class ApiQueryDeletedrevs extends ApiQueryBase { } $this->addTables( 'archive' ); - $this->addFields( array( 'ar_title', 'ar_namespace', 'ar_timestamp', 'ar_deleted' ) ); + $this->addFields( array( 'ar_title', 'ar_namespace', 'ar_timestamp', 'ar_deleted', 'ar_id' ) ); $this->addFieldsIf( 'ar_parent_id', $fld_parentid ); $this->addFieldsIf( 'ar_rev_id', $fld_revid ); @@ -214,19 +214,33 @@ class ApiQueryDeletedrevs extends ApiQueryBase { } } - if ( !is_null( $params['continue'] ) && ( $mode == 'all' || $mode == 'revs' ) ) { + if ( !is_null( $params['continue'] ) ) { $cont = explode( '|', $params['continue'] ); - $this->dieContinueUsageIf( count( $cont ) != 3 ); - $ns = intval( $cont[0] ); - $this->dieContinueUsageIf( strval( $ns ) !== $cont[0] ); - $title = $db->addQuotes( $cont[1] ); - $ts = $db->addQuotes( $db->timestamp( $cont[2] ) ); $op = ( $dir == 'newer' ? '>' : '<' ); - $this->addWhere( "ar_namespace $op $ns OR " . - "(ar_namespace = $ns AND " . - "(ar_title $op $title OR " . - "(ar_title = $title AND " . - "ar_timestamp $op= $ts)))" ); + if ( $mode == 'all' || $mode == 'revs' ) { + $this->dieContinueUsageIf( count( $cont ) != 4 ); + $ns = intval( $cont[0] ); + $this->dieContinueUsageIf( strval( $ns ) !== $cont[0] ); + $title = $db->addQuotes( $cont[1] ); + $ts = $db->addQuotes( $db->timestamp( $cont[2] ) ); + $ar_id = (int)$cont[3]; + $this->dieContinueUsageIf( strval( $ar_id ) !== $cont[3] ); + $this->addWhere( "ar_namespace $op $ns OR " . + "(ar_namespace = $ns AND " . + "(ar_title $op $title OR " . + "(ar_title = $title AND " . + "(ar_timestamp $op $ts OR " . + "(ar_timestamp = $ts AND " . + "ar_id $op= $ar_id)))))" ); + } else { + $this->dieContinueUsageIf( count( $cont ) != 2 ); + $ts = $db->addQuotes( $db->timestamp( $cont[0] ) ); + $ar_id = (int)$cont[1]; + $this->dieContinueUsageIf( strval( $ar_id ) !== $cont[1] ); + $this->addWhere( "ar_timestamp $op $ts OR " . + "(ar_timestamp = $ts AND " . + "ar_id $op= $ar_id)" ); + } } $this->addOption( 'LIMIT', $limit + 1 ); @@ -236,12 +250,14 @@ class ApiQueryDeletedrevs extends ApiQueryBase { ); if ( $mode == 'all' ) { if ( $params['unique'] ) { + // @todo Does this work on non-MySQL? $this->addOption( 'GROUP BY', 'ar_title' ); } else { $sort = ( $dir == 'newer' ? '' : ' DESC' ); $this->addOption( 'ORDER BY', array( 'ar_title' . $sort, - 'ar_timestamp' . $sort + 'ar_timestamp' . $sort, + 'ar_id' . $sort, ) ); } } else { @@ -251,6 +267,8 @@ class ApiQueryDeletedrevs extends ApiQueryBase { $this->addWhereRange( 'ar_title', $dir, null, null ); } $this->addTimestampWhereRange( 'ar_timestamp', $dir, $params['start'], $params['end'] ); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'ar_id', $dir, null, null ); } $res = $this->select( __METHOD__ ); $pageMap = array(); // Maps ns&title to (fake) pageid @@ -260,10 +278,11 @@ class ApiQueryDeletedrevs extends ApiQueryBase { if ( ++$count > $limit ) { // We've had enough if ( $mode == 'all' || $mode == 'revs' ) { - $this->setContinueEnumParameter( 'continue', intval( $row->ar_namespace ) . '|' . - $row->ar_title . '|' . $row->ar_timestamp ); + $this->setContinueEnumParameter( 'continue', + "$row->ar_namespace|$row->ar_title|$row->ar_timestamp|$row->ar_id" + ); } else { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->ar_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->ar_timestamp|$row->ar_id" ); } break; } @@ -371,10 +390,11 @@ class ApiQueryDeletedrevs extends ApiQueryBase { } if ( !$fit ) { if ( $mode == 'all' || $mode == 'revs' ) { - $this->setContinueEnumParameter( 'continue', intval( $row->ar_namespace ) . '|' . - $row->ar_title . '|' . $row->ar_timestamp ); + $this->setContinueEnumParameter( 'continue', + "$row->ar_namespace|$row->ar_title|$row->ar_timestamp|$row->ar_id" + ); } else { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->ar_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->ar_timestamp|$row->ar_id" ); } break; } @@ -468,7 +488,7 @@ class ApiQueryDeletedrevs extends ApiQueryBase { 'namespace' => 'Only list pages in this namespace (3)', 'user' => 'Only list revisions by this user', 'excludeuser' => 'Don\'t list revisions by this user', - 'continue' => 'When more results are available, use this to continue (1, 3)', + 'continue' => 'When more results are available, use this to continue', 'unique' => 'List only one revision for each page (3)', 'tag' => 'Only list revisions tagged with this tag', ); diff --git a/includes/api/ApiQueryLogEvents.php b/includes/api/ApiQueryLogEvents.php index b607ca543e..ee7fbc006f 100644 --- a/includes/api/ApiQueryLogEvents.php +++ b/includes/api/ApiQueryLogEvents.php @@ -73,13 +73,14 @@ class ApiQueryLogEvents extends ApiQueryBase { 'log_title=page_title' ) ) ) ); $this->addFields( array( + 'log_id', 'log_type', 'log_action', 'log_timestamp', 'log_deleted', ) ); - $this->addFieldsIf( array( 'log_id', 'page_id' ), $this->fld_ids ); + $this->addFieldsIf( 'page_id', $this->fld_ids ); $this->addFieldsIf( array( 'log_user', 'log_user_text', 'user_name' ), $this->fld_user ); $this->addFieldsIf( 'log_user', $this->fld_userid ); $this->addFieldsIf( @@ -135,6 +136,21 @@ class ApiQueryLogEvents extends ApiQueryBase { $params['start'], $params['end'] ); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'log_id', $params['dir'], null, null ); + + if ( !is_null( $params['continue'] ) ) { + $cont = explode( '|', $params['continue'] ); + $this->dieContinueUsageIf( count( $cont ) != 2 ); + $op = ( $params['dir'] === 'newer' ? '>' : '<' ); + $continueTimestamp = $db->addQuotes( $db->timestamp( $cont[0] ) ); + $continueId = (int)$cont[1]; + $this->dieContinueUsageIf( $continueId != $cont[1] ); + $this->addWhere( "log_timestamp $op $continueTimestamp OR " . + "(log_timestamp = $continueTimestamp AND " . + "log_id $op= $continueId)" + ); + } $limit = $params['limit']; $this->addOption( 'LIMIT', $limit + 1 ); @@ -202,7 +218,7 @@ class ApiQueryLogEvents extends ApiQueryBase { if ( ++$count > $limit ) { // We've reached the one extra which shows that there are // additional pages to be had. Stop here... - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->log_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->log_timestamp|$row->log_id" ); break; } @@ -212,7 +228,7 @@ class ApiQueryLogEvents extends ApiQueryBase { } $fit = $result->addValue( array( 'query', $this->getModuleName() ), null, $vals ); if ( !$fit ) { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->log_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->log_timestamp|$row->log_id" ); break; } } @@ -497,7 +513,8 @@ class ApiQueryLogEvents extends ApiQueryBase { ApiBase::PARAM_MIN => 1, ApiBase::PARAM_MAX => ApiBase::LIMIT_BIG1, ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2 - ) + ), + 'continue' => null, ); } @@ -531,6 +548,7 @@ class ApiQueryLogEvents extends ApiQueryBase { 'prefix' => 'Filter entries that start with this prefix. Disabled in Miser Mode', 'limit' => 'How many total event entries to return', 'tag' => 'Only list event entries tagged with this tag', + 'continue' => 'When more results are available, use this to continue', ); } diff --git a/includes/api/ApiQueryProtectedTitles.php b/includes/api/ApiQueryProtectedTitles.php index 9cdd6b9104..6cf7ff3444 100644 --- a/includes/api/ApiQueryProtectedTitles.php +++ b/includes/api/ApiQueryProtectedTitles.php @@ -63,6 +63,27 @@ class ApiQueryProtectedTitles extends ApiQueryGeneratorBase { $this->addWhereFld( 'pt_namespace', $params['namespace'] ); $this->addWhereFld( 'pt_create_perm', $params['level'] ); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'pt_namespace', $params['dir'], null, null ); + $this->addWhereRange( 'pt_title', $params['dir'], null, null ); + + if ( !is_null( $params['continue'] ) ) { + $cont = explode( '|', $params['continue'] ); + $this->dieContinueUsageIf( count( $cont ) != 3 ); + $op = ( $params['dir'] === 'newer' ? '>' : '<' ); + $db = $this->getDB(); + $continueTimestamp = $db->addQuotes( $db->timestamp( $cont[0] ) ); + $continueNs = (int)$cont[1]; + $this->dieContinueUsageIf( $continueNs != $cont[1] ); + $continueTitle = $db->addQuotes( $cont[2] ); + $this->addWhere( "pt_timestamp $op $continueTimestamp OR " . + "(pt_timestamp = $continueTimestamp AND " . + "(pt_namespace $op $continueNs OR " . + "(pt_namespace = $continueNs AND " . + "pt_title $op= $continueTitle)))" + ); + } + if ( isset( $prop['user'] ) ) { $this->addTables( 'user' ); $this->addFields( 'user_name' ); @@ -83,7 +104,9 @@ class ApiQueryProtectedTitles extends ApiQueryGeneratorBase { if ( ++$count > $params['limit'] ) { // We've reached the one extra which shows that there are // additional pages to be had. Stop here... - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->pt_timestamp ) ); + $this->setContinueEnumParameter( 'continue', + "$row->pt_timestamp|$row->pt_namespace|$row->pt_title" + ); break; } @@ -122,8 +145,9 @@ class ApiQueryProtectedTitles extends ApiQueryGeneratorBase { $fit = $result->addValue( array( 'query', $this->getModuleName() ), null, $vals ); if ( !$fit ) { - $this->setContinueEnumParameter( 'start', - wfTimestamp( TS_ISO_8601, $row->pt_timestamp ) ); + $this->setContinueEnumParameter( 'continue', + "$row->pt_timestamp|$row->pt_namespace|$row->pt_title" + ); break; } } else { @@ -195,6 +219,7 @@ class ApiQueryProtectedTitles extends ApiQueryGeneratorBase { 'level' ) ), + 'continue' => null, ); } @@ -216,6 +241,7 @@ class ApiQueryProtectedTitles extends ApiQueryGeneratorBase { ' level - Adds the protection level', ), 'level' => 'Only list titles with these protection levels', + 'continue' => 'When more results are available, use this to continue', ); } diff --git a/includes/api/ApiQueryRecentChanges.php b/includes/api/ApiQueryRecentChanges.php index 79a8dc96f9..80352f2528 100644 --- a/includes/api/ApiQueryRecentChanges.php +++ b/includes/api/ApiQueryRecentChanges.php @@ -152,15 +152,12 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { if ( !is_null( $params['continue'] ) ) { $cont = explode( '|', $params['continue'] ); - if ( count( $cont ) != 2 ) { - $this->dieUsage( 'Invalid continue param. You should pass the ' . - 'original value returned by the previous query', '_badcontinue' ); - } - - $timestamp = $this->getDB()->addQuotes( wfTimestamp( TS_MW, $cont[0] ) ); + $this->dieContinueUsageIf( count( $cont ) != 2 ); + $db = $this->getDB(); + $timestamp = $db->addQuotes( $db->timestamp( $cont[0] ) ); $id = intval( $cont[1] ); + $this->dieContinueUsageIf( $id != $cont[1] ); $op = $params['dir'] === 'older' ? '<' : '>'; - $this->addWhere( "rc_timestamp $op $timestamp OR " . "(rc_timestamp = $timestamp AND " . @@ -254,6 +251,7 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { /* Add the fields we're concerned with to our query. */ $this->addFields( array( + 'rc_id', 'rc_timestamp', 'rc_namespace', 'rc_title', @@ -277,7 +275,6 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { ); } - $this->addFields( 'rc_id' ); /* Add fields to our query if they are specified as a needed parameter. */ $this->addFieldsIf( array( 'rc_this_oldid', 'rc_last_oldid' ), $this->fld_ids ); $this->addFieldsIf( 'rc_comment', $this->fld_comment || $this->fld_parsedcomment ); @@ -371,10 +368,7 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { if ( ++$count > $params['limit'] ) { // We've reached the one extra which shows that there are // additional pages to be had. Stop here... - $this->setContinueEnumParameter( - 'continue', - wfTimestamp( TS_ISO_8601, $row->rc_timestamp ) . '|' . $row->rc_id - ); + $this->setContinueEnumParameter( 'continue', "$row->rc_timestamp|$row->rc_id" ); break; } @@ -388,10 +382,7 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase { } $fit = $result->addValue( array( 'query', $this->getModuleName() ), null, $vals ); if ( !$fit ) { - $this->setContinueEnumParameter( - 'continue', - wfTimestamp( TS_ISO_8601, $row->rc_timestamp ) . '|' . $row->rc_id - ); + $this->setContinueEnumParameter( 'continue', "$row->rc_timestamp|$row->rc_id" ); break; } } else { diff --git a/includes/api/ApiQueryUserContributions.php b/includes/api/ApiQueryUserContributions.php index b58a951a34..e9fec4346c 100644 --- a/includes/api/ApiQueryUserContributions.php +++ b/includes/api/ApiQueryUserContributions.php @@ -108,22 +108,14 @@ class ApiQueryContributions extends ApiQueryBase { if ( ++$count > $limit ) { // We've reached the one extra which shows that there are // additional pages to be had. Stop here... - if ( $this->multiUserMode ) { - $this->setContinueEnumParameter( 'continue', $this->continueStr( $row ) ); - } else { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->rev_timestamp ) ); - } + $this->setContinueEnumParameter( 'continue', $this->continueStr( $row ) ); break; } $vals = $this->extractRowInfo( $row ); $fit = $this->getResult()->addValue( array( 'query', $this->getModuleName() ), null, $vals ); if ( !$fit ) { - if ( $this->multiUserMode ) { - $this->setContinueEnumParameter( 'continue', $this->continueStr( $row ) ); - } else { - $this->setContinueEnumParameter( 'start', wfTimestamp( TS_ISO_8601, $row->rev_timestamp ) ); - } + $this->setContinueEnumParameter( 'continue', $this->continueStr( $row ) ); break; } } @@ -167,18 +159,34 @@ class ApiQueryContributions extends ApiQueryBase { $this->addWhere( 'page_id=rev_page' ); // Handle continue parameter - if ( $this->multiUserMode && !is_null( $this->params['continue'] ) ) { + if ( !is_null( $this->params['continue'] ) ) { $continue = explode( '|', $this->params['continue'] ); - $this->dieContinueUsageIf( count( $continue ) != 2 ); $db = $this->getDB(); - $encUser = $db->addQuotes( $continue[0] ); - $encTS = $db->addQuotes( $db->timestamp( $continue[1] ) ); + if ( $this->multiUserMode ) { + $this->dieContinueUsageIf( count( $continue ) != 3 ); + $encUser = $db->addQuotes( array_shift( $continue ) ); + } else { + $this->dieContinueUsageIf( count( $continue ) != 2 ); + } + $encTS = $db->addQuotes( $db->timestamp( $continue[0] ) ); + $encId = (int)$continue[1]; + $this->dieContinueUsageIf( $encId != $continue[1] ); $op = ( $this->params['dir'] == 'older' ? '<' : '>' ); - $this->addWhere( - "rev_user_text $op $encUser OR " . - "(rev_user_text = $encUser AND " . - "rev_timestamp $op= $encTS)" - ); + if ( $this->multiUserMode ) { + $this->addWhere( + "rev_user_text $op $encUser OR " . + "(rev_user_text = $encUser AND " . + "(rev_timestamp $op $encTS OR " . + "(rev_timestamp = $encTS AND " . + "rev_id $op= $encId)))" + ); + } else { + $this->addWhere( + "rev_timestamp $op $encTS OR " . + "(rev_timestamp = $encTS AND " . + "rev_id $op= $encId)" + ); + } } // Don't include any revisions where we're not supposed to be able to @@ -209,6 +217,9 @@ class ApiQueryContributions extends ApiQueryBase { } $this->addTimestampWhereRange( 'rev_timestamp', $this->params['dir'], $this->params['start'], $this->params['end'] ); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'rev_id', $this->params['dir'], null, null ); + $this->addWhereFld( 'page_namespace', $this->params['namespace'] ); $show = $this->params['show']; @@ -242,6 +253,7 @@ class ApiQueryContributions extends ApiQueryBase { // ns+title checks if the user has access rights for this page // user_text is necessary if multiple users were specified $this->addFields( array( + 'rev_id', 'rev_timestamp', 'page_namespace', 'page_title', @@ -284,7 +296,6 @@ class ApiQueryContributions extends ApiQueryBase { $this->addTables( $tables ); $this->addFieldsIf( 'rev_page', $this->fld_ids ); - $this->addFieldsIf( 'rev_id', $this->fld_ids || $this->fld_flags ); $this->addFieldsIf( 'page_latest', $this->fld_flags ); // $this->addFieldsIf( 'rev_text_id', $this->fld_ids ); // Should this field be exposed? $this->addFieldsIf( 'rev_comment', $this->fld_comment || $this->fld_parsedcomment ); @@ -424,8 +435,11 @@ class ApiQueryContributions extends ApiQueryBase { } private function continueStr( $row ) { - return $row->rev_user_text . '|' . - wfTimestamp( TS_ISO_8601, $row->rev_timestamp ); + if ( $this->multiUserMode ) { + return "$row->rev_user_text|$row->rev_timestamp|$row->rev_id"; + } else { + return "$row->rev_timestamp|$row->rev_id"; + } } public function getCacheMode( $params ) { diff --git a/includes/api/ApiQueryWatchlist.php b/includes/api/ApiQueryWatchlist.php index 6baa87dca4..2e1ce6c7ca 100644 --- a/includes/api/ApiQueryWatchlist.php +++ b/includes/api/ApiQueryWatchlist.php @@ -86,6 +86,7 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { } $this->addFields( array( + 'rc_id', 'rc_namespace', 'rc_title', 'rc_timestamp', @@ -135,6 +136,22 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { $this->addTimestampWhereRange( 'rc_timestamp', $params['dir'], $params['start'], $params['end'] ); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'rc_id', $params['dir'], null, null ); + + if ( !is_null( $params['continue'] ) ) { + $cont = explode( '|', $params['continue'] ); + $this->dieContinueUsageIf( count( $cont ) != 2 ); + $op = ( $params['dir'] === 'newer' ? '>' : '<' ); + $continueTimestamp = $db->addQuotes( $db->timestamp( $cont[0] ) ); + $continueId = (int)$cont[1]; + $this->dieContinueUsageIf( $continueId != $cont[1] ); + $this->addWhere( "rc_timestamp $op $continueTimestamp OR " . + "(rc_timestamp = $continueTimestamp AND " . + "rc_id $op= $continueId)" + ); + } + $this->addWhereFld( 'wl_namespace', $params['namespace'] ); if ( !$params['allrev'] ) { @@ -236,10 +253,7 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { if ( ++$count > $params['limit'] ) { // We've reached the one extra which shows that there are // additional pages to be had. Stop here... - $this->setContinueEnumParameter( - 'start', - wfTimestamp( TS_ISO_8601, $row->rc_timestamp ) - ); + $this->setContinueEnumParameter( 'continue', "$row->rc_timestamp|$row->rc_id" ); break; } @@ -247,8 +261,7 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { $vals = $this->extractRowInfo( $row ); $fit = $this->getResult()->addValue( array( 'query', $this->getModuleName() ), null, $vals ); if ( !$fit ) { - $this->setContinueEnumParameter( 'start', - wfTimestamp( TS_ISO_8601, $row->rc_timestamp ) ); + $this->setContinueEnumParameter( 'continue', "$row->rc_timestamp|$row->rc_id" ); break; } } else { @@ -544,6 +557,7 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { 'token' => array( ApiBase::PARAM_TYPE => 'string' ), + 'continue' => null, ); } @@ -588,6 +602,7 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase { 'owner' => 'The name of the user whose watchlist you\'d like to access', 'token' => 'Give a security token (settable in preferences) to ' . 'allow access to another user\'s watchlist', + 'continue' => 'When more results are available, use this to continue', ); } -- 2.20.1