Bug 658929 - User autocomplete is very slow when there are lots of users in the profiles table

r/a=mkanat

9 files changed, 54 insertions, 14 deletions

diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm
index e6127d32a..938541f56 100644
--- a/Bugzilla/Auth.pm
+++ b/Bugzilla/Auth.pm
@@ -86,7 +86,7 @@ sub login {
 
     # Make sure the user isn't disabled.
     my $user = $login_info->{user};
-    if ($user->disabledtext) {
+    if (!$user->is_enabled) {
         return $self->_handle_login_result({ failure => AUTH_DISABLED,
                                               user    => $user }, $type);
     }
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 1603dbc4b..85a430f10 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -885,6 +885,8 @@ use constant ABSTRACT_SCHEMA => {
             mybugslink     => {TYPE => 'BOOLEAN', NOTNULL => 1,
                                DEFAULT => 'TRUE'},
             extern_id      => {TYPE => 'varchar(64)'},
+            is_enabled     => {TYPE => 'BOOLEAN', NOTNULL => 1, 
+                               DEFAULT => 'TRUE'}, 
         ],
         INDEXES => [
             profiles_login_name_idx => {FIELDS => ['login_name'],
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index af5ddaf82..af276882c 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -651,6 +651,9 @@ sub update_table_definitions {
 
     _populate_bug_see_also_class();
 
+    # 2011-06-15 dkl@mozilla.com - Bug 658929
+    _migrate_disabledtext_boolean();
+
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
     ################################################################
@@ -3573,6 +3576,16 @@ sub _populate_bug_see_also_class {
     $dbh->bz_commit_transaction();
 }
 
+sub _migrate_disabledtext_boolean {
+    my $dbh = Bugzilla->dbh;
+    if (!$dbh->bz_column_info('profiles', 'is_enabled')) {
+        $dbh->bz_add_column("profiles", 'is_enabled',
+                            {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'TRUE'});
+        $dbh->do("UPDATE profiles SET is_enabled = 0 
+                  WHERE disabledtext != ''");
+    }
+}
+
 1;
 
 __END__
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 89cd7ce72..d21314604 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -82,6 +82,7 @@ use constant DEFAULT_USER => {
     'showmybugslink' => 0,
     'disabledtext'   => '',
     'disable_mail'   => 0,
+    'is_enabled'     => 1, 
 };
 
 use constant DB_TABLE => 'profiles';
@@ -98,6 +99,7 @@ use constant DB_COLUMNS => (
     'profiles.disabledtext',
     'profiles.disable_mail',
     'profiles.extern_id',
+    'profiles.is_enabled', 
 );
 use constant NAME_FIELD => 'login_name';
 use constant ID_FIELD   => 'userid';
@@ -110,6 +112,7 @@ use constant VALIDATORS => {
     login_name    => \&check_login_name_for_creation,
     realname      => \&_check_realname,
     extern_id     => \&_check_extern_id,
+    is_enabled    => \&_check_is_enabled, 
 };
 
 sub UPDATE_COLUMNS {
@@ -120,11 +123,18 @@ sub UPDATE_COLUMNS {
         login_name
         realname
         extern_id
+        is_enabled
     );
     push(@cols, 'cryptpassword') if exists $self->{cryptpassword};
     return @cols;
 };
 
+use constant VALIDATOR_DEPENDENCIES => {
+    is_enabled => ['disabledtext'], 
+};
+
+use constant EXTRA_REQUIRED_FIELDS => qw(is_enabled);
+
 ################################################################################
 # Functions
 ################################################################################
@@ -178,7 +188,7 @@ sub update {
 
     # XXX Can update profiles_activity here as soon as it understands
     #     field names like login_name.
-
+    
     return $changes;
 }
 
@@ -238,11 +248,20 @@ sub _check_password {
 
 sub _check_realname { return trim($_[1]) || ''; }
 
+sub _check_is_enabled {
+    my ($invocant, $is_enabled, undef, $params) = @_;
+    # is_enabled is set automatically on creation depending on whether 
+    # disabledtext is empty (enabled) or not empty (disabled).
+    # When updating the user, is_enabled is set by calling set_disabledtext().
+    # Any value passed into this validator is ignored.
+    my $disabledtext = ref($invocant) ? $invocant->disabledtext : $params->{disabledtext};
+    return $disabledtext ? 0 : 1;
+}
+
 ################################################################################
 # Mutators
 ################################################################################
 
-sub set_disabledtext { $_[0]->set('disabledtext', $_[1]); }
 sub set_disable_mail { $_[0]->set('disable_mail', $_[1]); }
 sub set_extern_id    { $_[0]->set('extern_id', $_[1]); }
 
@@ -261,6 +280,10 @@ sub set_name {
 
 sub set_password { $_[0]->set('cryptpassword', $_[1]); }
 
+sub set_disabledtext {
+    $_[0]->set('disabledtext', $_[1]);
+    $_[0]->set('is_enabled', $_[1] ? 0 : 1);
+}
 
 ################################################################################
 # Methods
@@ -272,7 +295,7 @@ sub login { $_[0]->{login_name}; }
 sub extern_id { $_[0]->{extern_id}; }
 sub email { $_[0]->login . Bugzilla->params->{'emailsuffix'}; }
 sub disabledtext { $_[0]->{'disabledtext'}; }
-sub is_disabled { $_[0]->disabledtext ? 1 : 0; }
+sub is_enabled { $_[0]->{'is_enabled'} ? 1 : 0; }
 sub showmybugslink { $_[0]->{showmybugslink}; }
 sub email_disabled { $_[0]->{disable_mail}; }
 sub email_enabled { !($_[0]->{disable_mail}); }
@@ -1341,7 +1364,7 @@ sub match {
                       "AND group_id IN(" .
                       join(', ', (-1, @{$user->visible_groups_inherited})) . ") ";
         }
-        $query    .= " AND disabledtext = '' " if $exclude_disabled;
+        $query    .= " AND is_enabled = 1 " if $exclude_disabled;
         $query    .= $dbh->sql_limit($limit) if $limit;
 
         # Execute the query, retrieve the results, and make them into
@@ -1376,7 +1399,7 @@ sub match {
                       " AND group_id IN(" .
                 join(', ', (-1, @{$user->visible_groups_inherited})) . ") ";
         }
-        $query     .= " AND disabledtext = '' " if $exclude_disabled;
+        $query     .= " AND is_enabled = 1 " if $exclude_disabled;
         $query     .= $dbh->sql_limit($limit) if $limit;
         my $user_ids = $dbh->selectcol_arrayref($query, undef, ($str, $str));
         @users = @{Bugzilla::User->new_from_list($user_ids)};
@@ -1781,7 +1804,7 @@ sub get_userlist {
                   "AND group_id IN(" .
                   join(', ', (-1, @{$self->visible_groups_inherited})) . ")";
     }
-    $query    .= " WHERE disabledtext = '' ";
+    $query    .= " WHERE is_enabled = 1 ";
     $query    .= $dbh->sql_group_by('userid', 'login_name, realname');
 
     my $sth = $dbh->prepare($query);
diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm
index b9434059d..2d9276cc3 100644
--- a/Bugzilla/WebService/User.pm
+++ b/Bugzilla/WebService/User.pm
@@ -218,7 +218,7 @@ sub get {
                 real_name => $self->type('string', $_->name),
                 name      => $self->type('string', $_->login),
                 email     => $self->type('string', $_->email),
-                can_login => $self->type('boolean', $_->is_disabled ? 0 : 1),
+                can_login => $self->type('boolean', $_->is_enabled ? 1 : 0),
                 email_enabled     => $self->type('boolean', $_->email_enabled),
                 login_denied_text => $self->type('string', $_->disabledtext),
             }} @$in_group;
@@ -231,7 +231,7 @@ sub get {
                 real_name => $self->type('string', $_->name),
                 name      => $self->type('string', $_->login),
                 email     => $self->type('string', $_->email),
-                can_login => $self->type('boolean', $_->is_disabled ? 0 : 1),
+                can_login => $self->type('boolean', $_->is_enabled ? 1 : 0),
             }} @$in_group;
     }
 
diff --git a/editusers.cgi b/editusers.cgi
index ad70e08a8..2efdd01c8 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -77,7 +77,7 @@ if ($action eq 'search') {
     my $matchstr      = $cgi->param('matchstr');
     my $matchtype     = $cgi->param('matchtype');
     my $grouprestrict = $cgi->param('grouprestrict') || '0';
-    my $query = 'SELECT DISTINCT userid, login_name, realname, disabledtext ' .
+    my $query = 'SELECT DISTINCT userid, login_name, realname, is_enabled ' .
                 'FROM profiles';
     my @bindValues;
     my $nextCondition;
diff --git a/template/en/default/admin/users/list.html.tmpl b/template/en/default/admin/users/list.html.tmpl
index cb05e827b..3f745a458 100644
--- a/template/en/default/admin/users/list.html.tmpl
+++ b/template/en/default/admin/users/list.html.tmpl
@@ -69,7 +69,7 @@
 [% FOREACH thisuser = users %]
   [% IF !thisuser.realname %]
     [%# We cannot pass one class now and one class later. %]
-    [% SET classes = (thisuser.disabledtext ? "bz_inactive missing" : "missing") %]
+    [% SET classes = (thisuser.is_enabled ? "missing" : "bz_inactive missing") %]
     [% overrides.realname.login_name.${thisuser.login_name} = {
            content          => "missing"
            override_content => 1
@@ -77,7 +77,7 @@
            override_class   => 1
        }
     %]
-  [% ELSIF thisuser.disabledtext %]
+  [% ELSIF !thisuser.is_enabled %]
     [% overrides.realname.login_name.${thisuser.login_name} = {
            class          => "bz_inactive"
            override_class => 1
@@ -85,7 +85,7 @@
     %]
   [% END %]
 
-  [% IF thisuser.disabledtext %]
+  [% IF !thisuser.is_enabled %]
     [% overrides.login_name.login_name.${thisuser.login_name} = {
            class          => "bz_inactive"
            override_class => 1
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index 4beb04891..01eb32651 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -60,6 +60,8 @@
               A new password has been set.
             [% ELSIF field == 'disabledtext' %]
               The disable text has been modified.
+            [% ELSIF field == 'is_enabled' %]
+              The user has been [% otheruser.is_enabled ? 'enabled' : 'disabled' %].
             [% ELSIF field == 'extern_id' %]
               The user's External Login ID has been modified.
             [% ELSIF field == 'disable_mail' %]
diff --git a/token.cgi b/token.cgi
index 560930385..3522834aa 100755
--- a/token.cgi
+++ b/token.cgi
@@ -114,7 +114,7 @@ if ( $action eq 'reqpw' ) {
     $user_account = Bugzilla::User->check($login_name);
 
     # Make sure the user account is active.
-    if ($user_account->is_disabled) {
+    if (!$user_account->is_enabled) {
         ThrowUserError('account_disabled',
                        {disabled_reason => get_text('account_disabled', {account => $login_name})});
     }