API: Make more continuations unique
authorBrad Jorsch <bjorsch@wikimedia.org>
Tue, 24 Dec 2013 20:26:47 +0000 (15:26 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Fri, 11 Apr 2014 14:50:16 +0000 (10:50 -0400)
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
includes/api/ApiQueryAllImages.php
includes/api/ApiQueryBlocks.php
includes/api/ApiQueryCategoryMembers.php
includes/api/ApiQueryDeletedrevs.php
includes/api/ApiQueryLogEvents.php
includes/api/ApiQueryProtectedTitles.php
includes/api/ApiQueryRecentChanges.php
includes/api/ApiQueryUserContributions.php
includes/api/ApiQueryWatchlist.php

index 947a0d6..ce3168e 100644 (file)
@@ -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 ===
 
index 6e2c31f..229c07e 100644 (file)
@@ -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;
                                }
index 6cc0183..cfca140 100644 (file)
@@ -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',
                );
        }
 
index 4e942d6..424770f 100644 (file)
@@ -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',
index 58db035..2ca93f5 100644 (file)
@@ -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',
                );
index b607ca5..ee7fbc0 100644 (file)
@@ -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',
                );
        }
 
index 9cdd6b9..6cf7ff3 100644 (file)
@@ -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',
                );
        }
 
index 79a8dc9..80352f2 100644 (file)
@@ -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 {
index b58a951..e9fec43 100644 (file)
@@ -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 ) {
index 6baa87d..2e1ce6c 100644 (file)
@@ -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',
                );
        }