Skip to content

bugzilla
bugzilla
Commit fb079a1b authored by mkanat%bugzilla.org's avatar mkanat%bugzilla.org
Browse files
Options

Bug 347291: Make Bugzilla::User use Bugzilla::Object

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=myk
parent d89140b0
Hide whitespace changes
Inline Side-by-side
Showing with 127 additions and 116 deletions
Bugzilla/BugMail.pm
View file @ fb079a1b
......@@ -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'}) {
......
Bugzilla/DB/Pg.pm
View file @ fb079a1b
......@@ -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);
}
}
}
#####################################################################
......
Bugzilla/Flag.pm
View file @ fb079a1b
......@@ -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
......
Bugzilla/FlagType.pm
View file @ fb079a1b
......@@ -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)) {
......
Bugzilla/Object.pm
View file @ fb079a1b
......@@ -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.
......
Bugzilla/User.pm
View file @ fb079a1b
......@@ -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>
......
editusers.cgi
View file @ fb079a1b
......@@ -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);
......
importxml.pl
View file @ fb079a1b
......@@ -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.
......
relogin.cgi
View file @ fb079a1b
......@@ -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))
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment