API: Fix queries for list=allusers&auactiveusers

The query introduced to support the auactiveusers is itself broken (it
counts every edit multiple times when combined with the group filters or
auprop=groups or auprop=rights, or for users with multiple rows in
ipblocks) and it breaks auprop=groups and auprop=rights.

Instead, let's filter using the same cached data used by
Special:ActiveUsers and do the actual counting of recent "edits" in a
subquery. And for parity with Special:ActiveUsers, let's skip
RC_EXTERNAL when doing the count.

Also, it turns out the "recenteditcount" property in the result is
really more like "recentactions" since it counts any action that shows
up in recentchanges; the discrepancy between that and "editcount" can be
confusing if someone is doing a lot of logged actions that don't create
dummy revisions. Let's rename that, but we'll have to keep the old
property around for a while for BC.

Bug: 64505
Bug: 64507
Bug: 67301
Change-Id: I461e92819188c311cbb3853bc6bfad45962c8d7b

diff --git a/includes/api/ApiQueryAllUsers.php b/includes/api/ApiQueryAllUsers.php
index e32104f..d0980e6 100644 (file)
--- a/includes/api/ApiQueryAllUsers.php
+++ b/includes/api/ApiQueryAllUsers.php
@@ -45,9 +45,15 @@ class ApiQueryAllUsers extends ApiQueryBase {
        }
 
        public function execute() {
-               $db = $this->getDB();
                $params = $this->extractRequestParams();
 
+               if ( $params['activeusers'] ) {
+                       // Update active user cache
+                       SpecialActiveUsers::mergeActiveUsers( 600 );
+               }
+
+               $db = $this->getDB();
+
                $prop = $params['prop'];
                if ( !is_null( $prop ) ) {
                        $prop = array_flip( $prop );
@@ -71,9 +77,9 @@ class ApiQueryAllUsers extends ApiQueryBase {
                $from = is_null( $params['from'] ) ? null : $this->getCanonicalUserName( $params['from'] );
                $to = is_null( $params['to'] ) ? null : $this->getCanonicalUserName( $params['to'] );
 
-               # MySQL doesn't seem to use 'equality propagation' here, so like the
-               # ActiveUsers special page, we have to use rc_user_text for some cases.
-               $userFieldToSort = $params['activeusers'] ? 'rc_user_text' : 'user_name';
+               # MySQL can't figure out that 'user_name' and 'qcc_title' are the same
+               # despite the JOIN condition, so manually sort on the correct one.
+               $userFieldToSort = $params['activeusers'] ? 'qcc_title' : 'user_name';
 
                $this->addWhereRange( $userFieldToSort, $dir, $from, $to );
 
@@ -155,19 +161,32 @@ class ApiQueryAllUsers extends ApiQueryBase {
                }
 
                if ( $params['activeusers'] ) {
-                       $this->addTables( 'recentchanges' );
-
-                       $this->addJoinConds( array( 'recentchanges' => array(
-                               'INNER JOIN', 'rc_user_text=user_name'
+                       $activeUserSeconds = $this->getConfig()->get( 'ActiveUserDays' ) * 86400;
+
+                       // Filter query to only include users in the active users cache
+                       $this->addTables( 'querycachetwo' );
+                       $this->addJoinConds( array( 'querycachetwo' => array(
+                               'INNER JOIN', array(
+                                       'qcc_type' => 'activeusers',
+                                       'qcc_namespace' => NS_USER,
+                                       'qcc_title=user_name',
+                               ),
                        ) ) );
 
-                       $this->addFields( array( 'recentedits' => 'COUNT(*)' ) );
-
-                       $this->addWhere( 'rc_log_type IS NULL OR rc_log_type != ' . $db->addQuotes( 'newusers' ) );
-                       $timestamp = $db->timestamp( wfTimestamp( TS_UNIX ) - $this->getConfig()->get( 'ActiveUserDays' ) * 24 * 3600 );
-                       $this->addWhere( 'rc_timestamp >= ' . $db->addQuotes( $timestamp ) );
-
-                       $this->addOption( 'GROUP BY', $userFieldToSort );
+                       // Actually count the actions using a subquery (bug 64505 and bug 64507)
+                       $timestamp = $db->timestamp( wfTimestamp( TS_UNIX ) - $activeUserSeconds );
+                       $this->addFields( array(
+                               'recentactions' => '(' . $db->selectSQLText(
+                                       'recentchanges',
+                                       'COUNT(*)',
+                                       array(
+                                               'rc_user_text = user_name',
+                                               'rc_type != ' . $db->addQuotes( RC_EXTERNAL ), // no wikidata
+                                               'rc_log_type IS NULL OR rc_log_type != ' . $db->addQuotes( 'newusers' ),
+                                               'rc_timestamp >= ' . $db->addQuotes( $timestamp ),
+                                       )
+                               ) . ')'
+                       ) );
                }
 
                $this->addOption( 'LIMIT', $sqlLimit );
@@ -203,8 +222,13 @@ class ApiQueryAllUsers extends ApiQueryBase {
                        if ( $lastUser !== $row->user_name ) {
                                // Save the last pass's user data
                                if ( is_array( $lastUserData ) ) {
-                                       $fit = $result->addValue( array( 'query', $this->getModuleName() ),
-                                               null, $lastUserData );
+                                       if ( $params['activeusers'] && $lastUserData['recentactions'] === 0 ) {
+                                               // activeusers cache was out of date
+                                               $fit = true;
+                                       } else {
+                                               $fit = $result->addValue( array( 'query', $this->getModuleName() ),
+                                                       null, $lastUserData );
+                                       }
 
                                        $lastUserData = null;
 
@@ -241,7 +265,9 @@ class ApiQueryAllUsers extends ApiQueryBase {
                                        $lastUserData['editcount'] = intval( $row->user_editcount );
                                }
                                if ( $params['activeusers'] ) {
-                                       $lastUserData['recenteditcount'] = intval( $row->recentedits );
+                                       $lastUserData['recentactions'] = intval( $row->recentactions );
+                                       // @todo 'recenteditcount' is set for BC, remove in 1.25
+                                       $lastUserData['recenteditcount'] = $lastUserData['recentactions'];
                                }
                                if ( $fld_registration ) {
                                        $lastUserData['registration'] = $row->user_registration ?
@@ -302,7 +328,9 @@ class ApiQueryAllUsers extends ApiQueryBase {
                        }
                }
 
-               if ( is_array( $lastUserData ) ) {
+               if ( is_array( $lastUserData ) &&
+                       !( $params['activeusers'] && $lastUserData['recentactions'] === 0 )
+               ) {
                        $fit = $result->addValue( array( 'query', $this->getModuleName() ),
                                null, $lastUserData );
                        if ( !$fit ) {
@@ -397,7 +425,7 @@ class ApiQueryAllUsers extends ApiQueryBase {
                        '' => array(
                                'userid' => 'integer',
                                'name' => 'string',
-                               'recenteditcount' => array(
+                               'recentactions' => array(
                                        ApiBase::PROP_TYPE => 'integer',
                                        ApiBase::PROP_NULLABLE => true
                                )