From fb079a1bcf1a9eb442dbd746d787d573a89fdc4a Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Fri, 11 Aug 2006 06:44:49 +0000 Subject: Bug 347291: Make Bugzilla::User use Bugzilla::Object Patch By Max Kanat-Alexander r=LpSolit, a=myk --- Bugzilla/BugMail.pm | 2 +- Bugzilla/DB/Pg.pm | 43 ++++++++++++++++ Bugzilla/Flag.pm | 9 ++-- Bugzilla/FlagType.pm | 2 +- Bugzilla/Object.pm | 40 ++++++++++++--- Bugzilla/User.pm | 137 +++++++++++++++------------------------------------ 6 files changed, 122 insertions(+), 111 deletions(-) (limited to 'Bugzilla') 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 this would be C. 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 + +The name of the column that should be considered to be the unique +"name" of this object. The 'name' is a B that uniquely identifies +this Object in the database. Defaults to 'name'. When you specify +C<{name => $name}> to C, this is the column that will be +matched against in the DB. + +=item C + +The name of the column that represents the unique B ID +of this object in the database. Defaults to 'id'. + =item C The order that C and C should return objects in. This should be the name of a database column. Defaults to -C. +L. =item C @@ -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 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 Luser|Bugzilla/"user">. +C is an implementation of L, and thus +provides all the methods of L in addition to the +methods listed below. + =head1 CONSTANTS =over @@ -1541,26 +1501,7 @@ confirmation screen. =head1 METHODS -=over 4 - -=item C - -Creates a new C 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 - -Creates a new C object given the provided login. Returns -C 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 -- cgit v1.2.3-24-g4f1b