[MediaWiki-commits] [Gerrit] API: Fix list=allusers with multiple values for augroup - change (mediawiki/core)

2014-09-06 Thread Anomie (Code Review) 
Anomie has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/158889

Change subject: API: Fix list=allusers with multiple values for augroup
..

API: Fix list=allusers with multiple values for augroup

The existing query only works with a single value for augroup, or mostly
if it's combined with auprop=groups or auprop=rights (since most users
don't have every possible group).

When used with multiple values for augroup, it will raise an error if it
happens to encounter enough users who have more than one of the
specified groups. And further, it will lead to repeated groups for these
users if combined with auprop=groups or auprop=rights.

To avoid both these issues, let's use EXISTS instead of a JOIN to test
augroup. While auexcludegroup doesn't have this problem, we may as well
make the same change there, too. And doing that, there's no reason to
continue with an error when both augroup and auexcludegroup are used.

Bug: 70496
Change-Id: Ia7086ce87012c22651ac4c7a3f75558347276226
---
M includes/api/ApiQueryAllUsers.php
1 file changed, 6 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/89/158889/1

diff --git a/includes/api/ApiQueryAllUsers.php 
b/includes/api/ApiQueryAllUsers.php
index dfef286..2a88215 100644
--- a/includes/api/ApiQueryAllUsers.php
+++ b/includes/api/ApiQueryAllUsers.php
@@ -111,35 +111,20 @@
}
}
 
-   if ( !is_null( $params['group'] ) && !is_null( 
$params['excludegroup'] ) ) {
-   $this->dieUsage( 'group and excludegroup cannot be used 
together', 'group-excludegroup' );
-   }
-
if ( !is_null( $params['group'] ) && count( $params['group'] ) 
) {
-   $useIndex = false;
// Filter only users that belong to a given group
-   $this->addTables( 'user_groups', 'ug1' );
-   $this->addJoinConds( array( 'ug1' => array( 'INNER 
JOIN', array( 'ug1.ug_user=user_id',
-   'ug1.ug_group' => $params['group'] ) ) ) );
+   $this->addWhere( 'EXISTS (' . $db->selectSQLText(
+   'user_groups', '1', array( 'ug_user=user_id', 
'ug_group' => $params['group'] )
+   ) . ')' );
}
 
if ( !is_null( $params['excludegroup'] ) && count( 
$params['excludegroup'] ) ) {
-   $useIndex = false;
// Filter only users don't belong to a given group
$this->addTables( 'user_groups', 'ug1' );
 
-   if ( count( $params['excludegroup'] ) == 1 ) {
-   $exclude = array( 'ug1.ug_group' => 
$params['excludegroup'][0] );
-   } else {
-   $exclude = array( $db->makeList(
-   array( 'ug1.ug_group' => 
$params['excludegroup'] ),
-   LIST_OR
-   ) );
-   }
-   $this->addJoinConds( array( 'ug1' => array( 'LEFT OUTER 
JOIN',
-   array_merge( array( 'ug1.ug_user=user_id' ), 
$exclude )
-   ) ) );
-   $this->addWhere( 'ug1.ug_user IS NULL' );
+   $this->addWhere( 'NOT EXISTS (' . $db->selectSQLText(
+   'user_groups', '1', array( 'ug_user=user_id', 
'ug_group' => $params['excludegroup'] )
+   ) . ')' );
}
 
if ( $params['witheditsonly'] ) {

-- 
To view, visit https://gerrit.wikimedia.org/r/158889
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia7086ce87012c22651ac4c7a3f75558347276226
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] API: Fix list=allusers with multiple values for augroup - change (mediawiki/core)

2014-09-12 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged.

Change subject: API: Fix list=allusers with multiple values for augroup
..


API: Fix list=allusers with multiple values for augroup

The existing query only works with a single value for augroup, or mostly
if it's combined with auprop=groups or auprop=rights (since most users
don't have every possible group).

When used with multiple values for augroup, it will raise an error if it
happens to encounter enough users who have more than one of the
specified groups. And further, it will lead to repeated groups for these
users if combined with auprop=groups or auprop=rights.

To avoid both these issues, let's use EXISTS instead of a JOIN to test
augroup. While auexcludegroup doesn't have this problem, we may as well
make the same change there, too. And doing that, there's no reason to
continue with an error when both augroup and auexcludegroup are used.

Bug: 70496
Change-Id: Ia7086ce87012c22651ac4c7a3f75558347276226
---
M includes/api/ApiQueryAllUsers.php
1 file changed, 6 insertions(+), 23 deletions(-)

Approvals:
  Aaron Schulz: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/api/ApiQueryAllUsers.php 
b/includes/api/ApiQueryAllUsers.php
index dfef286..c12f6bd 100644
--- a/includes/api/ApiQueryAllUsers.php
+++ b/includes/api/ApiQueryAllUsers.php
@@ -111,35 +111,18 @@
}
}
 
-   if ( !is_null( $params['group'] ) && !is_null( 
$params['excludegroup'] ) ) {
-   $this->dieUsage( 'group and excludegroup cannot be used 
together', 'group-excludegroup' );
-   }
-
if ( !is_null( $params['group'] ) && count( $params['group'] ) 
) {
-   $useIndex = false;
// Filter only users that belong to a given group
-   $this->addTables( 'user_groups', 'ug1' );
-   $this->addJoinConds( array( 'ug1' => array( 'INNER 
JOIN', array( 'ug1.ug_user=user_id',
-   'ug1.ug_group' => $params['group'] ) ) ) );
+   $this->addWhere( 'EXISTS (' . $db->selectSQLText(
+   'user_groups', '1', array( 'ug_user=user_id', 
'ug_group' => $params['group'] )
+   ) . ')' );
}
 
if ( !is_null( $params['excludegroup'] ) && count( 
$params['excludegroup'] ) ) {
-   $useIndex = false;
// Filter only users don't belong to a given group
-   $this->addTables( 'user_groups', 'ug1' );
-
-   if ( count( $params['excludegroup'] ) == 1 ) {
-   $exclude = array( 'ug1.ug_group' => 
$params['excludegroup'][0] );
-   } else {
-   $exclude = array( $db->makeList(
-   array( 'ug1.ug_group' => 
$params['excludegroup'] ),
-   LIST_OR
-   ) );
-   }
-   $this->addJoinConds( array( 'ug1' => array( 'LEFT OUTER 
JOIN',
-   array_merge( array( 'ug1.ug_user=user_id' ), 
$exclude )
-   ) ) );
-   $this->addWhere( 'ug1.ug_user IS NULL' );
+   $this->addWhere( 'NOT EXISTS (' . $db->selectSQLText(
+   'user_groups', '1', array( 'ug_user=user_id', 
'ug_group' => $params['excludegroup'] )
+   ) . ')' );
}
 
if ( $params['witheditsonly'] ) {

-- 
To view, visit https://gerrit.wikimedia.org/r/158889
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia7086ce87012c22651ac4c7a3f75558347276226
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie 
Gerrit-Reviewer: Aaron Schulz 
Gerrit-Reviewer: Anomie 
Gerrit-Reviewer: Legoktm 
Gerrit-Reviewer: Springle 
Gerrit-Reviewer: Yurik 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits