diff options
| -rw-r--r-- | Bugzilla/BugMail.pm | 2 | ||||
| -rw-r--r-- | Bugzilla/DB/Pg.pm | 43 | ||||
| -rw-r--r-- | Bugzilla/Flag.pm | 9 | ||||
| -rw-r--r-- | Bugzilla/FlagType.pm | 2 | ||||
| -rw-r--r-- | Bugzilla/Object.pm | 40 | ||||
| -rw-r--r-- | Bugzilla/User.pm | 137 | ||||
| -rwxr-xr-x | editusers.cgi | 2 | ||||
| -rwxr-xr-x | importxml.pl | 6 | ||||
| -rwxr-xr-x | relogin.cgi | 2 |
9 files changed, 127 insertions, 116 deletions
diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index dd92cd3b4..98a8e92bf 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -168,7 +168,7 @@ sub ProcessOneBug { # Convert to names, for later display $values{'changer'} = $changer; - $values{'changername'} = Bugzilla::User->new_from_login($changer)->name; + $values{'changername'} = Bugzilla::User->new({name => $changer})->name; $values{'assigned_to'} = user_id_to_login($values{'assigned_to'}); $values{'reporter'} = user_id_to_login($values{'reporter'}); if ($values{'qa_contact'}) { diff --git a/Bugzilla/DB/Pg.pm b/Bugzilla/DB/Pg.pm index 7bf64ab9d..98bfc5903 100644 --- a/Bugzilla/DB/Pg.pm +++ b/Bugzilla/DB/Pg.pm @@ -224,6 +224,49 @@ sub bz_setup_database { # login_name, which we do with sql_istrcmp all over the place. $self->bz_add_index('profiles', 'profiles_login_name_lower_idx', {FIELDS => ['LOWER(login_name)'], TYPE => 'UNIQUE'}); + + # Now that Bugzilla::Object uses sql_istrcmp, other tables + # also need a LOWER() index. + _fix_case_differences('fielddefs', 'name'); + $self->bz_add_index('fielddefs', 'fielddefs_name_lower_idx', + {FIELDS => ['LOWER(name)'], TYPE => 'UNIQUE'}); + _fix_case_differences('keyworddefs', 'name'); + $self->bz_add_index('keyworddefs', 'keyworddefs_name_lower_idx', + {FIELDS => ['LOWER(name)'], TYPE => 'UNIQUE'}); + _fix_case_differences('products', 'name'); + $self->bz_add_index('products', 'products_name_lower_idx', + {FIELDS => ['LOWER(name)'], TYPE => 'UNIQUE'}); +} + +# Renames things that differ only in case. +sub _fix_case_differences { + my ($table, $field) = @_; + my $dbh = Bugzilla->dbh; + + my $duplicates = $dbh->selectcol_arrayref( + "SELECT DISTINCT LOWER($field) FROM $table + GROUP BY LOWER($field) HAVING COUNT(LOWER($field)) > 1"); + + foreach my $name (@$duplicates) { + my $dups = $dbh->selectcol_arrayref( + "SELECT $field FROM $table WHERE LOWER($field) = ?", + undef, $name); + my $primary = shift @$dups; + foreach my $dup (@$dups) { + my $new_name = "${dup}_"; + # Make sure the new name isn't *also* a duplicate. + while (1) { + last if (!$dbh->selectrow_array( + "SELECT 1 FROM $table WHERE LOWER($field) = ?", + undef, lc($new_name))); + $new_name .= "_"; + } + print "$table '$primary' and '$dup' have names that differ", + " only in case.\nRenaming '$dup' to '$new_name'...\n"; + $dbh->do("UPDATE $table SET $field = ? WHERE $field = ?", + undef, $new_name, $dup); + } + } } ##################################################################### diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 42d747d60..04623524e 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -330,7 +330,7 @@ sub validate { # We know the requestee exists because we ran # Bugzilla::User::match_field before getting here. - my $requestee = Bugzilla::User->new_from_login($login); + my $requestee = new Bugzilla::User({ name => $login }); # Throw an error if the user can't see the bug. # Note that if permissions on this bug are changed, @@ -593,7 +593,7 @@ sub modify { create({ type => $flag->type, setter => $setter, status => "?", - requestee => Bugzilla::User->new_from_login($login) }, + requestee => new Bugzilla::User({ name => $login }) }, $bug, $attachment, $timestamp); } } @@ -796,7 +796,8 @@ sub FormToNewFlags { push (@flags, { type => $flag_type , setter => $setter , status => $status , - requestee => Bugzilla::User->new_from_login($login) }); + requestee => + new Bugzilla::User({ name => $login }) }); last unless $flag_type->is_multiplicable; } } @@ -843,7 +844,7 @@ sub notify { if (scalar(@bug_in_groups) || $attachment_is_private) { my @new_cc_list; foreach my $cc (split(/[, ]+/, $flag->type->cc_list)) { - my $ccuser = Bugzilla::User->new_from_login($cc) || next; + my $ccuser = new Bugzilla::User({ name => $cc }) || next; next if (scalar(@bug_in_groups) && !$ccuser->can_see_bug($bug->bug_id)); next if $attachment_is_private diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index 9beeaf28e..47efbd68a 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -451,7 +451,7 @@ sub validate { foreach my $login (@requestees) { # We know the requestee exists because we ran # Bugzilla::User::match_field before getting here. - my $requestee = Bugzilla::User->new_from_login($login); + my $requestee = new Bugzilla::User({ name => $login }); # Throw an error if the user can't see the bug. if (!$requestee->can_see_bug($bug_id)) { diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index c250f80fd..fa6c4e9cb 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -21,7 +21,9 @@ package Bugzilla::Object; use Bugzilla::Util; use Bugzilla::Error; -use constant LIST_ORDER => 'name'; +use constant NAME_FIELD => 'name'; +use constant ID_FIELD => 'id'; +use constant LIST_ORDER => NAME_FIELD; ############################### #### Initialization #### @@ -35,12 +37,19 @@ sub new { return $object; } + +# Note: Because this uses sql_istrcmp, if you make a new object use +# Bugzilla::Object, make sure that you modify bz_setup_database +# in Bugzilla::DB::Pg appropriately, to add the right LOWER +# index. You can see examples already there. sub _init { my $class = shift; my ($param) = @_; my $dbh = Bugzilla->dbh; my $columns = join(',', $class->DB_COLUMNS); my $table = $class->DB_TABLE; + my $name_field = $class->NAME_FIELD; + my $id_field = $class->ID_FIELD; my $id = $param unless (ref $param eq 'HASH'); my $object; @@ -52,12 +61,13 @@ sub _init { $object = $dbh->selectrow_hashref(qq{ SELECT $columns FROM $table - WHERE id = ?}, undef, $id); + WHERE $id_field = ?}, undef, $id); } elsif (defined $param->{'name'}) { trick_taint($param->{'name'}); $object = $dbh->selectrow_hashref(qq{ SELECT $columns FROM $table - WHERE name = ?}, undef, $param->{'name'}); + WHERE } . $dbh->sql_istrcmp($name_field, '?'), + undef, $param->{'name'}); } else { ThrowCodeError('bad_arg', {argument => 'param', @@ -74,6 +84,7 @@ sub new_from_list { my $columns = join(',', $class->DB_COLUMNS); my $table = $class->DB_TABLE; my $order = $class->LIST_ORDER; + my $id_field = $class->ID_FIELD; my $objects; if (@$id_list) { @@ -85,7 +96,7 @@ sub new_from_list { push(@detainted_ids, $id); } $objects = $dbh->selectall_arrayref( - "SELECT $columns FROM $table WHERE id IN (" + "SELECT $columns FROM $table WHERE $id_field IN (" . join(',', @detainted_ids) . ") ORDER BY $order", {Slice=>{}}); } else { return []; @@ -152,9 +163,10 @@ sub get_all { my $dbh = Bugzilla->dbh; my $table = $class->DB_TABLE; my $order = $class->LIST_ORDER; + my $id_field = $class->ID_FIELD; my $ids = $dbh->selectcol_arrayref(qq{ - SELECT id FROM $table ORDER BY $order}); + SELECT $id_field FROM $table ORDER BY $order}); my $objects = $class->new_from_list($ids); return @$objects; @@ -206,11 +218,24 @@ for C<Bugzilla::Keyword> this would be C<keyworddefs>. The names of the columns that you want to read out of the database and into this object. This should be an array. +=item C<NAME_FIELD> + +The name of the column that should be considered to be the unique +"name" of this object. The 'name' is a B<string> that uniquely identifies +this Object in the database. Defaults to 'name'. When you specify +C<{name => $name}> to C<new()>, this is the column that will be +matched against in the DB. + +=item C<ID_FIELD> + +The name of the column that represents the unique B<integer> ID +of this object in the database. Defaults to 'id'. + =item C<LIST_ORDER> The order that C<new_from_list> and C<get_all> should return objects in. This should be the name of a database column. Defaults to -C<name>. +L</NAME_FIELD>. =item C<REQUIRED_CREATE_FIELDS> @@ -240,7 +265,8 @@ They must return the validated value. id of the object, from the database, that we want to read in. If you pass in a hash with C<name> key, then the value of the name key - is the name of the object from the DB. + is the case-insensitive name of the object from + the DB. Returns: A fully-initialized object. diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 6039e3fe6..01f2db4d9 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -48,7 +48,7 @@ use Bugzilla::Product; use Bugzilla::Classification; use Bugzilla::Field; -use base qw(Exporter); +use base qw(Bugzilla::Object Exporter); @Bugzilla::User::EXPORT = qw(insert_new_user is_available_username login_to_id user_id_to_login validate_password UserInGroup @@ -66,95 +66,51 @@ use constant USER_MATCH_SUCCESS => 1; use constant MATCH_SKIP_CONFIRM => 1; +use constant DEFAULT_USER => { + 'id' => 0, + 'name' => '', + 'login' => '', + 'showmybugslink' => 0, + 'disabledtext' => '', + 'disable_mail' => 0, +}; + +use constant DB_TABLE => 'profiles'; + +# XXX Note that Bugzilla::User->name does not return the same thing +# that you passed in for "name" to new(). That's because historically +# Bugzilla::User used "name" for the realname field. This should be +# fixed one day. +use constant DB_COLUMNS => ( + 'profiles.userid AS id', + 'profiles.login_name AS login', + 'profiles.realname AS name', + 'profiles.mybugslink AS showmybugslink', + 'profiles.disabledtext', + 'profiles.disable_mail', +); +use constant NAME_FIELD => 'login_name'; +use constant ID_FIELD => 'userid'; + ################################################################################ # Functions ################################################################################ sub new { my $invocant = shift; - my $user_id = shift; - - if ($user_id) { - my $uid = $user_id; - detaint_natural($user_id) - || ThrowCodeError('invalid_numeric_argument', - {argument => 'userID', - value => $uid, - function => 'Bugzilla::User::new'}); - return $invocant->_create("userid=?", $user_id); - } - else { - return $invocant->_create; - } -} - -# This routine is sort of evil. Nothing except the login stuff should -# be dealing with addresses as an input, and they can get the id as a -# side effect of the other sql they have to do anyway. -# Bugzilla::BugMail still does this, probably as a left over from the -# pre-id days. Provide this as a helper, but don't document it, and hope -# that it can go away. -# The request flag stuff also does this, but it really should be passing -# in the id it already had to validate (or the User.pm object, of course) -sub new_from_login { - my $invocant = shift; - my $login = shift; - - my $dbh = Bugzilla->dbh; - return $invocant->_create($dbh->sql_istrcmp('login_name', '?'), $login); -} - -# Internal helper for the above |new| methods -# $cond is a string (including a placeholder ?) for the search -# requirement for the profiles table -sub _create { - my $invocant = shift; my $class = ref($invocant) || $invocant; + my ($param) = @_; - my $cond = shift; - my $val = shift; - - # Allow invocation with no parameters to create a blank object - my $self = { - 'id' => 0, - 'name' => '', - 'login' => '', - 'showmybugslink' => 0, - 'disabledtext' => '', - 'flags' => {}, - 'disable_mail' => 0, - }; - bless ($self, $class); - return $self unless $cond && $val; - - # We're checking for validity here, so any value is OK - trick_taint($val); + my $user = DEFAULT_USER; + bless ($user, $class); + return $user unless $param; - my $dbh = Bugzilla->dbh; - - my ($id, $login, $name, $disabledtext, $mybugslink, $disable_mail) = - $dbh->selectrow_array(qq{SELECT userid, login_name, realname, - disabledtext, mybugslink, disable_mail - FROM profiles WHERE $cond}, - undef, $val); - - return undef unless defined $id; - - $self->{'id'} = $id; - $self->{'name'} = $name; - $self->{'login'} = $login; - $self->{'disabledtext'} = $disabledtext; - $self->{'showmybugslink'} = $mybugslink; - $self->{'disable_mail'} = $disable_mail; - - return $self; + return $class->SUPER::new(@_); } # Accessors for user attributes -sub id { $_[0]->{id}; } sub login { $_[0]->{login}; } sub email { $_[0]->{login} . Bugzilla->params->{'emailsuffix'}; } -sub name { $_[0]->{name}; } sub disabledtext { $_[0]->{'disabledtext'}; } sub is_disabled { $_[0]->disabledtext ? 1 : 0; } sub showmybugslink { $_[0]->{showmybugslink}; } @@ -844,7 +800,7 @@ sub match { # User objects. my $user_logins = $dbh->selectcol_arrayref($query, undef, ($wildstr, $wildstr)); foreach my $login_name (@$user_logins) { - push(@users, Bugzilla::User->new_from_login($login_name)); + push(@users, new Bugzilla::User({ name => $login_name })); } } else { # try an exact match @@ -884,7 +840,7 @@ sub match { my $user_logins = $dbh->selectcol_arrayref($query, undef, ($str, $str)); foreach my $login_name (@$user_logins) { - push(@users, Bugzilla::User->new_from_login($login_name)); + push(@users, new Bugzilla::User({ name => $login_name })); } } return \@users; @@ -1513,6 +1469,10 @@ there is currently no way to modify a user from this package. Note that the currently logged in user (if any) is available via L<Bugzilla-E<gt>user|Bugzilla/"user">. +C<Bugzilla::User> is an implementation of L<Bugzilla::Object>, and thus +provides all the methods of L<Bugzilla::Object> in addition to the +methods listed below. + =head1 CONSTANTS =over @@ -1541,26 +1501,7 @@ confirmation screen. =head1 METHODS -=over 4 - -=item C<new($userid)> - -Creates a new C<Bugzilla::User> object for the given user id. If no user -id was given, a blank object is created with no user attributes. - -If an id was given but there was no matching user found, undef is returned. - -=begin undocumented - -=item C<new_from_login($login)> - -Creates a new C<Bugzilla::User> object given the provided login. Returns -C<undef> if no matching user is found. - -This routine should not be required in general; most scripts should be using -userids instead. - -=end undocumented +=over =item C<id> diff --git a/editusers.cgi b/editusers.cgi index e53341c61..caea2186b 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -797,7 +797,7 @@ sub check_user { $vars->{'user_id'} = $otherUserID; } elsif ($otherUserLogin) { - $otherUser = Bugzilla::User->new_from_login($otherUserLogin); + $otherUser = new Bugzilla::User({ name => $otherUserLogin }); $vars->{'user_login'} = $otherUserLogin; } ($otherUser && $otherUser->id) || ThrowCodeError('invalid_user', $vars); diff --git a/importxml.pl b/importxml.pl index 4ca9c8745..e048aac40 100755 --- a/importxml.pl +++ b/importxml.pl @@ -179,7 +179,7 @@ sub flag_handler { my $type = ($attachid) ? "attachment" : "bug"; my $err = ''; - my $setter = Bugzilla::User->new_from_login($setter_login); + my $setter = new Bugzilla::User({ name => $setter_login }); my $requestee; my $requestee_id; @@ -195,7 +195,7 @@ sub flag_handler { } my $setter_id = $setter->id; if ( defined($requestee_login) ) { - $requestee = Bugzilla::User->new_from_login($requestee_login); + $requestee = new Bugzilla::User({ name => $requestee_login }); if ( $requestee ) { if ( !$requestee->can_see_bug($bugid) ) { $err .= "Requestee is not a member of bug group\n"; @@ -423,7 +423,7 @@ sub process_bug { my $root = $twig->root; my $maintainer = $root->{'att'}->{'maintainer'}; my $exporter_login = $root->{'att'}->{'exporter'}; - my $exporter = Bugzilla::User->new_from_login($exporter_login); + my $exporter = new Bugzilla::User({ name => $exporter_login }); my $urlbase = $root->{'att'}->{'urlbase'}; # We will store output information in this variable. diff --git a/relogin.cgi b/relogin.cgi index 566a1df37..e47dbe003 100755 --- a/relogin.cgi +++ b/relogin.cgi @@ -125,7 +125,7 @@ elsif ($action eq 'begin-sudo') { # Get & verify the target user (the user who we will be impersonating) my $target_user = - Bugzilla::User->new_from_login($cgi->param('target_login')); + new Bugzilla::User({ name => $cgi->param('target_login') }); unless (defined($target_user) && $target_user->id && $user->can_see_user($target_user)) |