Anomie has uploaded a new change for review. https://gerrit.wikimedia.org/r/103589
Change subject: API: Make more continuations unique ...................................................................... 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 --- M RELEASE-NOTES-1.23 M includes/api/ApiQueryAllImages.php M includes/api/ApiQueryBlocks.php M includes/api/ApiQueryCategoryMembers.php M includes/api/ApiQueryDeletedrevs.php M includes/api/ApiQueryLogEvents.php M includes/api/ApiQueryProtectedTitles.php M includes/api/ApiQueryRecentChanges.php M includes/api/ApiQueryUserContributions.php M includes/api/ApiQueryWatchlist.php 10 files changed, 212 insertions(+), 82 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/89/103589/1 diff --git a/RELEASE-NOTES-1.23 b/RELEASE-NOTES-1.23 index db80ad3..f86d128 100644 --- a/RELEASE-NOTES-1.23 +++ b/RELEASE-NOTES-1.23 @@ -90,6 +90,7 @@ properly for all parameters. * ApiQueryBase::titlePartToKey allows an extra parameter that indicates the namespace in order to properly capitalize the title part. +* (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 4095bd8..ae4e171 100644 --- a/includes/api/ApiQueryAllImages.php +++ b/includes/api/ApiQueryAllImages.php @@ -168,6 +168,20 @@ $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 @@ 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 @@ 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 57f76bc..2eeca0c 100644 --- a/includes/api/ApiQueryBlocks.php +++ b/includes/api/ApiQueryBlocks.php @@ -43,6 +43,7 @@ public function execute() { global $wgContLang; + $db = $this->getDB(); $params = $this->extractRequestParams(); $this->requireMaxOneParameter( $params, 'users', 'ip' ); @@ -61,9 +62,8 @@ $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 @@ $params['start'], $params['end'] ); + // Include in ORDER BY for uniqueness + $this->addWhereRange( 'ipb_id', $params['dir'], null, null ); - $db = $this->getDB(); + 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 @@ 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 @@ } $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 @@ ), ApiBase::PARAM_ISMULTI => true ), + 'continue' => null, ); } @@ -350,6 +364,7 @@ '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 271558b..eac236e 100644 --- a/includes/api/ApiQueryCategoryMembers.php +++ b/includes/api/ApiQueryCategoryMembers.php @@ -101,6 +101,22 @@ $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 @@ // @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 @@ 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 4e4b2cc..e6406b9 100644 --- a/includes/api/ApiQueryDeletedrevs.php +++ b/includes/api/ApiQueryDeletedrevs.php @@ -102,7 +102,7 @@ $this->addTables( 'archive' ); $this->addWhere( 'ar_deleted = 0' ); - $this->addFields( array( 'ar_title', 'ar_namespace', 'ar_timestamp' ) ); + $this->addFields( array( 'ar_title', 'ar_namespace', 'ar_timestamp', 'ar_id' ) ); $this->addFieldsIf( 'ar_parent_id', $fld_parentid ); $this->addFieldsIf( 'ar_rev_id', $fld_revid ); @@ -188,19 +188,33 @@ $db->addQuotes( $params['excludeuser'] ) ); } - 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 ); @@ -210,12 +224,14 @@ ); 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 { @@ -225,6 +241,8 @@ $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 @@ -234,10 +252,11 @@ 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; } @@ -310,10 +329,11 @@ } 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; } @@ -407,7 +427,7 @@ '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 356fa3e..6e4d5b9 100644 --- a/includes/api/ApiQueryLogEvents.php +++ b/includes/api/ApiQueryLogEvents.php @@ -74,13 +74,14 @@ '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( @@ -117,6 +118,21 @@ $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 ); @@ -172,7 +188,7 @@ 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; } @@ -182,7 +198,7 @@ } $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; } } @@ -444,7 +460,8 @@ ApiBase::PARAM_MIN => 1, ApiBase::PARAM_MAX => ApiBase::LIMIT_BIG1, ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2 - ) + ), + 'continue' => null, ); } @@ -475,6 +492,7 @@ '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 ea350ad..ec288d6 100644 --- a/includes/api/ApiQueryProtectedTitles.php +++ b/includes/api/ApiQueryProtectedTitles.php @@ -63,6 +63,27 @@ $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 @@ 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 @@ $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 @@ 'level' ) ), + 'continue' => null, ); } @@ -216,6 +241,7 @@ ' 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 02a05e8..3a0999a 100644 --- a/includes/api/ApiQueryRecentChanges.php +++ b/includes/api/ApiQueryRecentChanges.php @@ -153,15 +153,12 @@ 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 " . @@ -256,6 +253,7 @@ /* Add the fields we're concerned with to our query. */ $this->addFields( array( + 'rc_id', 'rc_timestamp', 'rc_namespace', 'rc_title', @@ -279,7 +277,6 @@ ); } - $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 ); @@ -343,10 +340,7 @@ 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; } @@ -360,10 +354,7 @@ } $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 1c9a93c..9424721 100644 --- a/includes/api/ApiQueryUserContributions.php +++ b/includes/api/ApiQueryUserContributions.php @@ -103,22 +103,14 @@ 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; } } @@ -162,18 +154,34 @@ $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)" + ); + } } if ( !$user->isAllowed( 'hideuser' ) ) { @@ -194,6 +202,9 @@ } $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']; @@ -217,6 +228,7 @@ // 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', @@ -259,7 +271,6 @@ $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 ); @@ -383,8 +394,11 @@ } 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 f9af75a..0d4ac75 100644 --- a/includes/api/ApiQueryWatchlist.php +++ b/includes/api/ApiQueryWatchlist.php @@ -85,6 +85,7 @@ } $this->addFields( array( + 'rc_id', 'rc_namespace', 'rc_title', 'rc_timestamp', @@ -137,6 +138,22 @@ $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'] ) { @@ -209,10 +226,7 @@ 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; } @@ -220,8 +234,7 @@ $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 { @@ -460,7 +473,8 @@ ), 'token' => array( ApiBase::PARAM_TYPE => 'string' - ) + ), + 'continue' => null, ); } @@ -504,7 +518,8 @@ ), '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' + 'allow access to another user\'s watchlist', + 'continue' => 'When more results are available, use this to continue', ); } -- To view, visit https://gerrit.wikimedia.org/r/103589 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4c9f8c0c2bfd831755d4fa20a18f93fef1effd28 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Anomie <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
