From ea6b82f1303f86e5b62ea23985cc47cea5454f9b Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Tue, 5 Jul 2011 11:06:37 -0400 Subject: Bug 658929 - User autocomplete is very slow when there are lots of users in the profiles table r/a=mkanat --- Bugzilla/Auth.pm | 2 +- Bugzilla/DB/Schema.pm | 2 ++ Bugzilla/Install/DB.pm | 13 ++++++++++ Bugzilla/User.pm | 35 +++++++++++++++++++++----- Bugzilla/WebService/User.pm | 4 +-- editusers.cgi | 2 +- template/en/default/admin/users/list.html.tmpl | 6 ++--- template/en/default/global/messages.html.tmpl | 2 ++ token.cgi | 2 +- 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})}); } -- cgit v1.2.1