summaryrefslogtreecommitdiffstats
path: root/Bugzilla
diff options
context:
space:
mode:
authormkanat%bugzilla.org <>2006-08-11 08:44:49 +0200
committermkanat%bugzilla.org <>2006-08-11 08:44:49 +0200
commitfb079a1bcf1a9eb442dbd746d787d573a89fdc4a (patch)
tree644d05740726c4e746f0f77b3cb8b66e49100f39 /Bugzilla
parentd89140b00e196de6d4d6f2c8e3b163784b5c15ff (diff)
downloadbugzilla-fb079a1bcf1a9eb442dbd746d787d573a89fdc4a.tar.gz
bugzilla-fb079a1bcf1a9eb442dbd746d787d573a89fdc4a.tar.xz
Bug 347291: Make Bugzilla::User use Bugzilla::Object
Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=myk
Diffstat (limited to 'Bugzilla')
-rw-r--r--Bugzilla/BugMail.pm2
-rw-r--r--Bugzilla/DB/Pg.pm43
-rw-r--r--Bugzilla/Flag.pm9
-rw-r--r--Bugzilla/FlagType.pm2
-rw-r--r--Bugzilla/Object.pm40
-rw-r--r--Bugzilla/User.pm137
6 files changed, 122 insertions, 111 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>