diff --git a/Bugzilla.pm b/Bugzilla.pm index d54f97491..742f2ca87 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -42,6 +42,7 @@ use Bugzilla::Auth::Persist::Cookie; use Bugzilla::CGI; use Bugzilla::DB; use Bugzilla::Install::Localconfig qw(read_localconfig); +use Bugzilla::JobQueue; use Bugzilla::Template; use Bugzilla::User; use Bugzilla::Error; @@ -51,6 +52,7 @@ use Bugzilla::Flag; use File::Basename; use File::Spec::Functions; +use DateTime::TimeZone; use Safe; # This creates the request cache for non-mod_perl installations. @@ -82,11 +84,14 @@ use constant SHUTDOWNHTML_EXIT_SILENTLY => [ sub init_page { (binmode STDOUT, ':utf8') if Bugzilla->params->{'utf8'}; + + if (${^TAINT}) { # Some environment variables are not taint safe delete @::ENV{'PATH', 'IFS', 'CDPATH', 'ENV', 'BASH_ENV'}; # Some modules throw undefined errors (notably File::Spec::Win32) if # PATH is undefined. $ENV{'PATH'} = ''; + } # IIS prints out warnings to the webpage, so ignore them, or log them # to a file if the file exists. @@ -223,6 +228,10 @@ sub sudo_request { # NOTE: If you want to log the start of an sudo session, do it here. } +sub page_requires_login { + return $_[0]->request_cache->{page_requires_login}; +} + sub login { my ($class, $type) = @_; @@ -233,6 +242,13 @@ sub login { if (!defined $type || $type == LOGIN_NORMAL) { $type = $class->params->{'requirelogin'} ? LOGIN_REQUIRED : LOGIN_NORMAL; } + + # Allow templates to know that we're in a page that always requires + # login. + if ($type == LOGIN_REQUIRED) { + $class->request_cache->{page_requires_login} = 1; + } + my $authenticated_user = $authorizer->login($type); # At this point, we now know if a real person is logged in. @@ -311,6 +327,12 @@ sub logout_request { # there. Don't rely on it: use Bugzilla->user->login instead! } +sub job_queue { + my $class = shift; + $class->request_cache->{job_queue} ||= Bugzilla::JobQueue->new(); + return $class->request_cache->{job_queue}; +} + sub dbh { my $class = shift; # If we're not connected, then we must want the main db @@ -346,7 +368,7 @@ sub error_mode { $class->request_cache->{error_mode} = $newval; } return $class->request_cache->{error_mode} - || Bugzilla::Constants::ERROR_MODE_WEBPAGE; + || (i_am_cgi() ? ERROR_MODE_WEBPAGE : ERROR_MODE_DIE); } sub usage_mode { @@ -371,7 +393,7 @@ sub usage_mode { $class->request_cache->{usage_mode} = $newval; } return $class->request_cache->{usage_mode} - || Bugzilla::Constants::USAGE_MODE_BROWSER; + || (i_am_cgi()? USAGE_MODE_BROWSER : USAGE_MODE_CMDLINE); } sub installation_mode { @@ -458,6 +480,16 @@ sub hook_args { return $class->request_cache->{hook_args}; } +sub local_timezone { + my $class = shift; + + if (!defined $class->request_cache->{local_timezone}) { + $class->request_cache->{local_timezone} = + DateTime::TimeZone->new(name => 'local'); + } + return $class->request_cache->{local_timezone}; +} + sub request_cache { if ($ENV{MOD_PERL}) { require Apache2::RequestUtil; @@ -602,6 +634,13 @@ Logs in a user, returning a C object, or C if there is no logged in user. See L, and L. +=item C + +If the current page always requires the user to log in (for example, +C or any page called with C) then +this will return something true. Otherwise it will return false. (This is +set when you call L.) + =item C Logs out the current user, which involves invalidating user sessions and @@ -694,4 +733,16 @@ is unreadable or is not valid perl, we C. If you are running inside a code hook (see L) this is how you get the arguments passed to the hook. +=item C + +Returns the local timezone of the Bugzilla installation, +as a DateTime::TimeZone object. This detection is very time +consuming, so we cache this information for future references. + +=item C + +Returns a L that you can use for queueing jobs. +Will throw an error if job queueing is not correctly configured on +this Bugzilla installation. + =back diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index 962e04fc1..5842d6c56 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -28,23 +28,26 @@ package Bugzilla::Attachment; =head1 NAME -Bugzilla::Attachment - a file related to a bug that a user has uploaded - to the Bugzilla server +Bugzilla::Attachment - Bugzilla attachment class. =head1 SYNOPSIS use Bugzilla::Attachment; # Get the attachment with the given ID. - my $attachment = Bugzilla::Attachment->get($attach_id); + my $attachment = new Bugzilla::Attachment($attach_id); # Get the attachments with the given IDs. - my $attachments = Bugzilla::Attachment->get_list($attach_ids); + my $attachments = Bugzilla::Attachment->new_from_list($attach_ids); =head1 DESCRIPTION -This module defines attachment objects, which represent files related to bugs -that users upload to the Bugzilla server. +Attachment.pm represents an attachment object. It is an implementation +of L, and thus provides all methods that +L provides. + +The methods that are specific to C are listed +below. =cut @@ -55,82 +58,44 @@ use Bugzilla::User; use Bugzilla::Util; use Bugzilla::Field; -sub get { - my $invocant = shift; - my $id = shift; +use base qw(Bugzilla::Object); - my $attachments = _retrieve([$id]); - my $self = $attachments->[0]; - bless($self, ref($invocant) || $invocant) if $self; +############################### +#### Initialization #### +############################### - return $self; -} +use constant DB_TABLE => 'attachments'; +use constant ID_FIELD => 'attach_id'; +use constant LIST_ORDER => ID_FIELD; -sub get_list { - my $invocant = shift; - my $ids = shift; - - my $attachments = _retrieve($ids); - foreach my $attachment (@$attachments) { - bless($attachment, ref($invocant) || $invocant); - } - - return $attachments; -} - -sub _retrieve { - my ($ids) = @_; - - return [] if scalar(@$ids) == 0; - - my @columns = ( - 'attachments.attach_id AS id', - 'attachments.bug_id AS bug_id', - 'attachments.description AS description', - 'attachments.mimetype AS contenttype', - 'attachments.submitter_id AS attacher_id', - Bugzilla->dbh->sql_date_format('attachments.creation_ts', - '%Y.%m.%d %H:%i') . " AS attached", - 'attachments.modification_time', - 'attachments.filename AS filename', - 'attachments.ispatch AS ispatch', - 'attachments.isurl AS isurl', - 'attachments.isobsolete AS isobsolete', - 'attachments.isprivate AS isprivate' - ); - my $columns = join(", ", @columns); +sub DB_COLUMNS { my $dbh = Bugzilla->dbh; - my $records = $dbh->selectall_arrayref( - "SELECT $columns - FROM attachments - WHERE " - . Bugzilla->dbh->sql_in('attach_id', $ids) - . " ORDER BY attach_id", - { Slice => {} }); - return $records; + + return qw( + attach_id + bug_id + description + filename + isobsolete + ispatch + isprivate + isurl + mimetype + modification_time + submitter_id), + $dbh->sql_date_format('attachments.creation_ts', '%Y.%m.%d %H:%i') . ' AS creation_ts'; } +############################### +#### Accessors ###### +############################### + =pod =head2 Instance Properties =over -=item C - -the unique identifier for the attachment - -=back - -=cut - -sub id { - my $self = shift; - return $self->{id}; -} - -=over - =item C the ID of the bug to which the attachment is attached @@ -139,8 +104,6 @@ the ID of the bug to which the attachment is attached =cut -# XXX Once Bug.pm slims down sufficiently this should become a reference -# to a bug object. sub bug_id { my $self = shift; return $self->{bug_id}; @@ -148,6 +111,24 @@ sub bug_id { =over +=item C + +the bug object to which the attachment is attached + +=back + +=cut + +sub bug { + my $self = shift; + + require Bugzilla::Bug; + $self->{bug} = Bugzilla::Bug->new($self->bug_id); + return $self->{bug}; +} + +=over + =item C user-provided text describing the attachment @@ -173,7 +154,7 @@ the attachment's MIME media type sub contenttype { my $self = shift; - return $self->{contenttype}; + return $self->{mimetype}; } =over @@ -189,7 +170,7 @@ the user who attached the attachment sub attacher { my $self = shift; return $self->{attacher} if exists $self->{attacher}; - $self->{attacher} = new Bugzilla::User($self->{attacher_id}); + $self->{attacher} = new Bugzilla::User($self->{submitter_id}); return $self->{attacher}; } @@ -205,7 +186,7 @@ the date and time on which the attacher attached the attachment sub attached { my $self = shift; - return $self->{attached}; + return $self->{creation_ts}; } =over @@ -351,7 +332,7 @@ sub data { FROM attach_data WHERE id = ?", undef, - $self->{id}); + $self->id); # If there's no attachment data in the database, the attachment is stored # in a local file, so retrieve it from there. @@ -396,7 +377,7 @@ sub datasize { Bugzilla->dbh->selectrow_array("SELECT LENGTH(thedata) FROM attach_data WHERE id = ?", - undef, $self->{id}) || 0; + undef, $self->id) || 0; # If there's no attachment data in the database, either the attachment # is stored in a local file, and so retrieve its size from the file, @@ -430,6 +411,34 @@ sub flags { return $self->{flags}; } +=over + +=item C + +Return all flag types available for this attachment as well as flags +already set, grouped by flag type. + +=back + +=cut + +sub flag_types { + my $self = shift; + return $self->{flag_types} if exists $self->{flag_types}; + + my $vars = { target_type => 'attachment', + product_id => $self->bug->product_id, + component_id => $self->bug->component_id, + attach_id => $self->id }; + + $self->{flag_types} = Bugzilla::Flag::_flag_types($vars); + return $self->{flag_types}; +} + +############################### +#### Validators ###### +############################### + # Instance methods; no POD documentation here yet because the only ones so far # are private. @@ -468,9 +477,7 @@ sub _validate_filename { sub _validate_data { my ($throw_error, $hr_vars) = @_; my $cgi = Bugzilla->cgi; - my $maxsize = $cgi->param('ispatch') ? Bugzilla->params->{'maxpatchsize'} - : Bugzilla->params->{'maxattachmentsize'}; - $maxsize *= 1024; # Convert from K + my $fh; # Skip uploading into a local variable if the user wants to upload huge # attachments into local files. @@ -514,6 +521,7 @@ sub _validate_data { } # Make sure the attachment does not exceed the maximum permitted size + my $maxsize = Bugzilla->params->{'maxattachmentsize'} * 1024; # Convert from K my $len = $data ? length($data) : 0; if ($maxsize && $len > $maxsize) { my $vars = { filesize => sprintf("%.0f", $len/1024) }; @@ -547,7 +555,7 @@ Returns: a reference to an array of attachment objects. =cut sub get_attachments_by_bug { - my ($class, $bug_id) = @_; + my ($class, $bug_id, $vars) = @_; my $user = Bugzilla->user; my $dbh = Bugzilla->dbh; @@ -564,7 +572,25 @@ sub get_attachments_by_bug { my $attach_ids = $dbh->selectcol_arrayref("SELECT attach_id FROM attachments WHERE bug_id = ? $and_restriction", undef, @values); - my $attachments = Bugzilla::Attachment->get_list($attach_ids); + + my $attachments = Bugzilla::Attachment->new_from_list($attach_ids); + + # To avoid $attachment->flags to run SQL queries itself for each + # attachment listed here, we collect all the data at once and + # populate $attachment->{flags} ourselves. + if ($vars->{preload}) { + $_->{flags} = [] foreach @$attachments; + my %att = map { $_->id => $_ } @$attachments; + + my $flags = Bugzilla::Flag->match({ bug_id => $bug_id, + target_type => 'attachment' }); + + # Exclude flags for private attachments you cannot see. + @$flags = grep {exists $att{$_->attach_id}} @$flags; + + push(@{$att{$_->attach_id}->{flags}}, $_) foreach @$flags; + $attachments = [sort {$a->id <=> $b->id} values %att]; + } return $attachments; } @@ -731,10 +757,9 @@ sub validate_obsolete { detaint_natural($attachid) || ThrowCodeError('invalid_attach_id_to_obsolete', $vars); - my $attachment = Bugzilla::Attachment->get($attachid); - # Make sure the attachment exists in the database. - ThrowUserError('invalid_attach_id', $vars) unless $attachment; + my $attachment = new Bugzilla::Attachment($attachid) + || ThrowUserError('invalid_attach_id', $vars); # Check that the user can view and edit this attachment. $attachment->validate_can_edit($bug->product_id); @@ -756,10 +781,13 @@ sub validate_obsolete { return @obsolete_attachments; } +############################### +#### Constructors ##### +############################### =pod -=item C +=item C Description: inserts an attachment from CGI input for the given bug. @@ -776,7 +804,8 @@ Returns: the ID of the new attachment. =cut -sub insert_attachment_for_bug { +# FIXME: needs to follow the way Object->create() works. +sub create { my ($class, $throw_error, $bug, $user, $timestamp, $hr_vars) = @_; my $cgi = Bugzilla->cgi; @@ -934,7 +963,7 @@ sub insert_attachment_for_bug { $timestamp, $fieldid, 0, 1)); } - my $attachment = Bugzilla::Attachment->get($attachid); + my $attachment = new Bugzilla::Attachment($attachid); # 1. Add flags, if any. To avoid dying if something goes wrong # while processing flags, we will eval() flag validation. diff --git a/Bugzilla/Auth/Login/Stack.pm b/Bugzilla/Auth/Login/Stack.pm index d51003861..a5752f22b 100644 --- a/Bugzilla/Auth/Login/Stack.pm +++ b/Bugzilla/Auth/Login/Stack.pm @@ -26,16 +26,24 @@ use fields qw( _stack successful ); +use Hash::Util qw(lock_keys); +use Bugzilla::Hook; sub new { my $class = shift; my $self = $class->SUPER::new(@_); my $list = shift; + my %methods = map { $_ => "Bugzilla/Auth/Login/$_.pm" } split(',', $list); + lock_keys(%methods); + Bugzilla::Hook::process('auth-login_methods', { modules => \%methods }); + $self->{_stack} = []; foreach my $login_method (split(',', $list)) { - require "Bugzilla/Auth/Login/${login_method}.pm"; - push(@{$self->{_stack}}, - "Bugzilla::Auth::Login::$login_method"->new(@_)); + my $module = $methods{$login_method}; + require $module; + $module =~ s|/|::|g; + $module =~ s/.pm$//; + push(@{$self->{_stack}}, $module->new(@_)); } return $self; } diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm index 420bad16b..c533252d3 100644 --- a/Bugzilla/Auth/Persist/Cookie.pm +++ b/Bugzilla/Auth/Persist/Cookie.pm @@ -161,6 +161,7 @@ sub clear_browser_cookies { my $cgi = Bugzilla->cgi; $cgi->remove_cookie('Bugzilla_login'); $cgi->remove_cookie('Bugzilla_logincookie'); + $cgi->remove_cookie('sudo'); } 1; diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm index 4221fce98..e3dffcd02 100644 --- a/Bugzilla/Auth/Verify/DB.pm +++ b/Bugzilla/Auth/Verify/DB.pm @@ -53,14 +53,9 @@ sub check_credentials { "SELECT cryptpassword FROM profiles WHERE userid = ?", undef, $user_id); - # Wide characters cause crypt to die - if (Bugzilla->params->{'utf8'}) { - utf8::encode($password) if utf8::is_utf8($password); - } - # Using the internal crypted password as the salt, # crypt the password the user entered. - my $entered_password_crypted = crypt($password, $real_password_crypted); + my $entered_password_crypted = bz_crypt($password, $real_password_crypted); return { failure => AUTH_LOGINFAILED } if $entered_password_crypted ne $real_password_crypted; @@ -69,6 +64,16 @@ sub check_credentials { # password tokens they may have generated. Bugzilla::Token::DeletePasswordTokens($user_id, "user_logged_in"); + # If their old password was using crypt() or some different hash + # than we're using now, convert the stored password to using + # whatever hashing system we're using now. + my $current_algorithm = PASSWORD_DIGEST_ALGORITHM; + if ($real_password_crypted !~ /{\Q$current_algorithm\E}$/) { + my $new_crypted = bz_crypt($password); + $dbh->do('UPDATE profiles SET cryptpassword = ? WHERE userid = ?', + undef, $new_crypted, $user_id); + } + return $login_data; } diff --git a/Bugzilla/Auth/Verify/Stack.pm b/Bugzilla/Auth/Verify/Stack.pm index 577b5a22f..c23b532fd 100644 --- a/Bugzilla/Auth/Verify/Stack.pm +++ b/Bugzilla/Auth/Verify/Stack.pm @@ -21,16 +21,24 @@ use fields qw( _stack successful ); +use Hash::Util qw(lock_keys); +use Bugzilla::Hook; sub new { my $class = shift; my $list = shift; my $self = $class->SUPER::new(@_); + my %methods = map { $_ => "Bugzilla/Auth/Verify/$_.pm" } split(',', $list); + lock_keys(%methods); + Bugzilla::Hook::process('auth-verify_methods', { modules => \%methods }); + $self->{_stack} = []; foreach my $verify_method (split(',', $list)) { - require "Bugzilla/Auth/Verify/${verify_method}.pm"; - push(@{$self->{_stack}}, - "Bugzilla::Auth::Verify::$verify_method"->new(@_)); + my $module = $methods{$verify_method}; + require $module; + $module =~ s|/|::|g; + $module =~ s/.pm$//; + push(@{$self->{_stack}}, $module->new(@_)); } return $self; } diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 1c0a8e47c..217f0d5f6 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -47,14 +47,15 @@ use Bugzilla::Status; use List::Util qw(min); use Storable qw(dclone); +use URI; +use URI::QueryParam; use base qw(Bugzilla::Object Exporter); @Bugzilla::Bug::EXPORT = qw( - bug_alias_to_id ValidateBugID + bug_alias_to_id RemoveVotes CheckIfVotedConfirmed LogActivityEntry editable_bug_fields - SPECIAL_STATUS_WORKFLOW_ACTIONS ); ##################################################################### @@ -72,7 +73,8 @@ sub DB_COLUMNS { my @custom = grep {$_->type != FIELD_TYPE_MULTI_SELECT} Bugzilla->active_custom_fields; my @custom_names = map {$_->name} @custom; - return qw( + + my @columns = (qw( alias assigned_to bug_file_loc @@ -100,7 +102,11 @@ sub DB_COLUMNS { 'reporter AS reporter_id', $dbh->sql_date_format('creation_ts', '%Y.%m.%d %H:%i') . ' AS creation_ts', $dbh->sql_date_format('deadline', '%Y-%m-%d') . ' AS deadline', - @custom_names; + @custom_names); + + Bugzilla::Hook::process("bug-columns", { columns => \@columns }); + + return @columns; } use constant REQUIRED_CREATE_FIELDS => qw( @@ -145,6 +151,9 @@ sub VALIDATORS { elsif ($field->type == FIELD_TYPE_FREETEXT) { $validator = \&_check_freetext_field; } + elsif ($field->type == FIELD_TYPE_BUG_ID) { + $validator = \&_check_bugid_field; + } else { $validator = \&_check_default_field; } @@ -225,13 +234,6 @@ use constant UPDATE_COMMENT_COLUMNS => qw( # activity table. use constant MAX_LINE_LENGTH => 254; -use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw( - none - duplicate - change_resolution - clearresolution -); - ##################################################################### sub new { @@ -239,6 +241,11 @@ sub new { my $class = ref($invocant) || $invocant; my $param = shift; + # Remove leading "#" mark if we've just been passed an id. + if (!ref $param && $param =~ /^#(\d+)$/) { + $param = $1; + } + # If we get something that looks like a word (not a number), # make it the "name" param. if (!defined $param || (!ref($param) && $param !~ /^\d+$/)) { @@ -263,15 +270,126 @@ sub new { # if the bug wasn't found in the database. if (!$self) { my $error_self = {}; + if (ref $param) { + $error_self->{bug_id} = $param->{name}; + $error_self->{error} = 'InvalidBugId'; + } + else { + $error_self->{bug_id} = $param; + $error_self->{error} = 'NotFound'; + } bless $error_self, $class; - $error_self->{'bug_id'} = ref($param) ? $param->{name} : $param; - $error_self->{'error'} = 'NotFound'; return $error_self; } return $self; } +sub check { + my $class = shift; + my ($id, $field) = @_; + + ThrowUserError('improper_bug_id_field_value', { field => $field }) unless defined $id; + + # Bugzilla::Bug throws lots of special errors, so we don't call + # SUPER::check, we just call our new and do our own checks. + my $self = $class->new(trim($id)); + # For error messages, use the id that was returned by new(), because + # it's cleaned up. + $id = $self->id; + + if ($self->{error}) { + if ($self->{error} eq 'NotFound') { + ThrowUserError("bug_id_does_not_exist", { bug_id => $id }); + } + if ($self->{error} eq 'InvalidBugId') { + ThrowUserError("improper_bug_id_field_value", + { bug_id => $id, + field => $field }); + } + } + + unless ($field && $field =~ /^(dependson|blocked|dup_id)$/) { + $self->check_is_visible; + } + return $self; +} + +sub check_is_visible { + my $self = shift; + my $user = Bugzilla->user; + + if (!$user->can_see_bug($self->id)) { + # The error the user sees depends on whether or not they are + # logged in (i.e. $user->id contains the user's positive integer ID). + if ($user->id) { + ThrowUserError("bug_access_denied", { bug_id => $self->id }); + } else { + ThrowUserError("bug_access_query", { bug_id => $self->id }); + } + } +} + +sub match { + my $class = shift; + my ($params) = @_; + + # Allow matching certain fields by name (in addition to matching by ID). + my %translate_fields = ( + assigned_to => 'Bugzilla::User', + qa_contact => 'Bugzilla::User', + reporter => 'Bugzilla::User', + product => 'Bugzilla::Product', + component => 'Bugzilla::Component', + ); + my %translated; + + foreach my $field (keys %translate_fields) { + my @ids; + # Convert names to ids. We use "exists" everywhere since people can + # legally specify "undef" to mean IS NULL (even though most of these + # fields can't be NULL, people can still specify it...). + if (exists $params->{$field}) { + my $names = $params->{$field}; + my $type = $translate_fields{$field}; + my $param = $type eq 'Bugzilla::User' ? 'login_name' : 'name'; + # We call Bugzilla::Object::match directly to avoid the + # Bugzilla::User::match implementation which is different. + my $objects = Bugzilla::Object::match($type, { $param => $names }); + push(@ids, map { $_->id } @$objects); + } + # You can also specify ids directly as arguments to this function, + # so include them in the list if they have been specified. + if (exists $params->{"${field}_id"}) { + my $current_ids = $params->{"${field}_id"}; + my @id_array = ref $current_ids ? @$current_ids : ($current_ids); + push(@ids, @id_array); + } + # We do this "or" instead of a "scalar(@ids)" to handle the case + # when people passed only invalid object names. Otherwise we'd + # end up with a SUPER::match call with zero criteria (which dies). + if (exists $params->{$field} or exists $params->{"${field}_id"}) { + $translated{$field} = scalar(@ids) == 1 ? $ids[0] : \@ids; + } + } + + # The user fields don't have an _id on the end of them in the database, + # but the product & component fields do, so we have to have separate + # code to deal with the different sets of fields here. + foreach my $field (qw(assigned_to qa_contact reporter)) { + delete $params->{"${field}_id"}; + $params->{$field} = $translated{$field} + if exists $translated{$field}; + } + foreach my $field (qw(product component)) { + delete $params->{$field}; + $params->{"${field}_id"} = $translated{$field} + if exists $translated{$field}; + } + + return $class->SUPER::match(@_); +} + # Docs for create() (there's no POD in this file yet, but we very # much need this documented right now): # @@ -424,6 +542,10 @@ sub create { $dbh->do('INSERT INTO longdescs (' . join(',', @columns) . ") VALUES ($qmarks)", undef, @values); + Bugzilla::Hook::process('bug-end_of_create', { bug => $bug, + timestamp => $timestamp, + }); + $dbh->bz_commit_transaction(); # Because MySQL doesn't support transactions on the fulltext table, @@ -493,6 +615,33 @@ sub run_create_validators { return $params; } +sub set_all { + my ($self, $args) = @_; + + # For security purposes, and because lots of other checks depend on it, + # we set the product first before anything else. + my $product_change = 0; + if ($args->{product}) { + my $changed = $self->set_product($args->{product}, + { component => $args->{component}, + version => $args->{version}, + target_milestone => $args->{target_milestone}, + change_confirmed => $args->{confirm_product_change}, + other_bugs => $args->{other_bugs}, + }); + # that will be used later to check strict isolation + $product_change = $changed; + } + + # add/remove groups + $self->remove_group($_) foreach @{$args->{remove_group}}; + $self->add_group($_) foreach @{$args->{add_group}}; + + # this is temporary until all related code is moved from + # process_bug.cgi to set_all + return $product_change; +} + sub update { my $self = shift; @@ -502,8 +651,7 @@ sub update { # inside this function. my $delta_ts = shift || $dbh->selectrow_array("SELECT NOW()"); - my $old_bug = $self->new($self->id); - my $changes = $self->SUPER::update(@_); + my ($changes, $old_bug) = $self->SUPER::update(@_); # Certain items in $changes have to be fixed so that they hold # a name instead of an ID. @@ -685,6 +833,25 @@ sub update { } } + # See Also + my ($removed_see, $added_see) = + diff_arrays($old_bug->see_also, $self->see_also); + + if (scalar @$removed_see) { + $dbh->do('DELETE FROM bug_see_also WHERE bug_id = ? AND ' + . $dbh->sql_in('value', [('?') x @$removed_see]), + undef, $self->id, @$removed_see); + } + foreach my $url (@$added_see) { + $dbh->do('INSERT INTO bug_see_also (bug_id, value) VALUES (?,?)', + undef, $self->id, $url); + } + # If any changes were found, record it in the activity log + if (scalar @$removed_see || scalar @$added_see) { + $changes->{see_also} = [join(', ', @$removed_see), + join(', ', @$added_see)]; + } + # Log bugs_activity items # XXX Eventually, when bugs_activity is able to track the dupe_id, # this code should go below the duplicates-table-updating code below. @@ -734,6 +901,10 @@ sub update { delete $self->{'_old_assigned_to'}; delete $self->{'_old_qa_contact'}; + # Also flush the visible_bugs cache for this bug as the user's + # relationship with this bug may have changed. + delete Bugzilla->user->{_visible_bugs_cache}->{$self->id}; + return $changes; } @@ -811,7 +982,6 @@ sub remove_from_db { # - keywords # - longdescs # - votes - # Also included are custom multi-select fields. # Also, the attach_data table uses attachments.attach_id as a foreign # key, and so indirectly depends on a bug deletion too. @@ -844,13 +1014,6 @@ sub remove_from_db { $dbh->do("DELETE FROM bugs WHERE bug_id = ?", undef, $bug_id); $dbh->do("DELETE FROM longdescs WHERE bug_id = ?", undef, $bug_id); - # Delete entries from custom multi-select fields. - my @multi_selects = Bugzilla->get_fields({custom => 1, type => FIELD_TYPE_MULTI_SELECT}); - - foreach my $field (@multi_selects) { - $dbh->do("DELETE FROM bug_" . $field->name . " WHERE bug_id = ?", undef, $bug_id); - } - $dbh->bz_commit_transaction(); # The bugs_fulltext table doesn't support transactions. @@ -1069,9 +1232,10 @@ sub _check_comment { sub _check_commentprivacy { my ($invocant, $comment_privacy) = @_; - my $insider_group = Bugzilla->params->{"insidergroup"}; - return ($insider_group && Bugzilla->user->in_group($insider_group) - && $comment_privacy) ? 1 : 0; + if ($comment_privacy && !Bugzilla->user->is_insider) { + ThrowUserError('user_not_insider'); + } + return $comment_privacy ? 1 : 0; } sub _check_comment_type { @@ -1123,11 +1287,13 @@ sub _check_dependencies { my %deps_in = (dependson => $depends_on || '', blocked => $blocks || ''); foreach my $type qw(dependson blocked) { - my @bug_ids = split(/[\s,]+/, $deps_in{$type}); + my @bug_ids = ref($deps_in{$type}) + ? @{$deps_in{$type}} + : split(/[\s,]+/, $deps_in{$type}); # Eliminate nulls. @bug_ids = grep {$_} @bug_ids; - # We do Validate up here to make sure all aliases are converted to IDs. - ValidateBugID($_, $type) foreach @bug_ids; + # We do this up here to make sure all aliases are converted to IDs. + @bug_ids = map { $invocant->check($_, $type)->id } @bug_ids; my @check_access = @bug_ids; # When we're updating a bug, only added or removed bug_ids are @@ -1149,11 +1315,10 @@ sub _check_dependencies { my $user = Bugzilla->user; foreach my $modified_id (@check_access) { - ValidateBugID($modified_id); + my $delta_bug = $invocant->check($modified_id); # Under strict isolation, you can't modify a bug if you can't # edit it, even if you can see it. if (Bugzilla->params->{"strict_isolation"}) { - my $delta_bug = new Bugzilla::Bug($modified_id); if (!$user->can_edit_product($delta_bug->{'product_id'})) { ThrowUserError("illegal_change_deps", {field => $type}); } @@ -1176,17 +1341,18 @@ sub _check_dup_id { $dupe_of = trim($dupe_of); $dupe_of || ThrowCodeError('undefined_field', { field => 'dup_id' }); - # Validate the bug ID. The second argument will force ValidateBugID() to - # only make sure that the bug exists, and convert the alias to the bug ID + # Validate the bug ID. The second argument will force check() to only + # make sure that the bug exists, and convert the alias to the bug ID # if a string is passed. Group restrictions are checked below. - ValidateBugID($dupe_of, 'dup_id'); + my $dupe_of_bug = $self->check($dupe_of, 'dup_id'); + $dupe_of = $dupe_of_bug->id; # If the dupe is unchanged, we have nothing more to check. return $dupe_of if ($self->dup_id && $self->dup_id == $dupe_of); # If we come here, then the duplicate is new. We have to make sure # that we can view/change it (issue A on bug 96085). - check_is_visible($dupe_of); + $dupe_of_bug->check_is_visible; # Make sure a loop isn't created when marking this bug # as duplicate. @@ -1218,7 +1384,6 @@ sub _check_dup_id { # Should we add the reporter to the CC list of the new bug? # If he can see the bug... if ($self->reporter->can_see_bug($dupe_of)) { - my $dupe_of_bug = new Bugzilla::Bug($dupe_of); # We only add him if he's not the reporter of the other bug. $self->{_add_dup_cc} = 1 if $dupe_of_bug->reporter->id != $self->reporter->id; @@ -1243,9 +1408,7 @@ sub _check_dup_id { my $vars = {}; my $template = Bugzilla->template; # Ask the user what they want to do about the reporter. - $vars->{'cclist_accessible'} = $dbh->selectrow_array( - q{SELECT cclist_accessible FROM bugs WHERE bug_id = ?}, - undef, $dupe_of); + $vars->{'cclist_accessible'} = $dupe_of_bug->cclist_accessible; $vars->{'original_bug_id'} = $dupe_of; $vars->{'duplicate_bug_id'} = $self->id; print $cgi->header(); @@ -1656,6 +1819,12 @@ sub _check_select_field { return $value; } +sub _check_bugid_field { + my ($invocant, $value, $field) = @_; + return undef if !$value; + return $invocant->check($value, $field)->id; +} + ##################################################################### # Class Accessors ##################################################################### @@ -1663,14 +1832,15 @@ sub _check_select_field { sub fields { my $class = shift; - return ( + my @fields = + ( # Standard Fields # Keep this ordering in sync with bugzilla.dtd. qw(bug_id alias creation_ts short_desc delta_ts reporter_accessible cclist_accessible classification_id classification product component version rep_platform op_sys - bug_status resolution dup_id + bug_status resolution dup_id see_also bug_file_loc status_whiteboard keywords priority bug_severity target_milestone dependson blocked votes everconfirmed @@ -1682,6 +1852,9 @@ sub fields { # Custom Fields map { $_->name } Bugzilla->active_custom_fields ); + Bugzilla::Hook::process("bug-fields", {'fields' => \@fields} ); + + return @fields; } ##################################################################### @@ -1730,11 +1903,6 @@ sub set_assigned_to { } sub reset_assigned_to { my $self = shift; - if (Bugzilla->params->{'commentonreassignbycomponent'} - && !$self->{added_comments}) - { - ThrowUserError('comment_required'); - } my $comp = $self->component_obj; $self->set_assigned_to($comp->default_assignee); } @@ -1975,11 +2143,6 @@ sub set_qa_contact { } sub reset_qa_contact { my $self = shift; - if (Bugzilla->params->{'commentonreassignbycomponent'} - && !$self->{added_comments}) - { - ThrowUserError('comment_required'); - } my $comp = $self->component_obj; $self->set_qa_contact($comp->default_qa_contact); } @@ -2028,11 +2191,6 @@ sub clear_resolution { if (!$self->status->is_open) { ThrowUserError('resolution_cant_clear', { bug_id => $self->id }); } - if (Bugzilla->params->{'commentonclearresolution'} - && $self->resolution && !$self->{added_comments}) - { - ThrowUserError('comment_required'); - } $self->{'resolution'} = ''; $self->_clear_dup_id; } @@ -2280,6 +2438,99 @@ sub remove_group { @$current_groups = grep { $_->id != $group->id } @$current_groups; } +sub add_see_also { + my ($self, $input) = @_; + $input = trim($input); + + # We assume that the URL is an HTTP URL if there is no (something):// + # in front. + my $uri = new URI($input); + if (!$uri->scheme) { + # This works better than setting $uri->scheme('http'), because + # that creates URLs like "http:domain.com" and doesn't properly + # differentiate the path from the domain. + $uri = new URI("http://$input"); + } + elsif ($uri->scheme ne 'http' && $uri->scheme ne 'https') { + ThrowUserError('bug_url_invalid', { url => $input, reason => 'http' }); + } + + my $result; + # Launchpad URLs + if ($uri->authority =~ /launchpad.net$/) { + # Launchpad bug URLs can look like various things: + # https://bugs.launchpad.net/ubuntu/+bug/1234 + # https://launchpad.net/bugs/1234 + # All variations end with either "/bugs/1234" or "/+bug/1234" + if ($uri->path =~ m|bugs?/(\d+)$|) { + # This is the shortest standard URL form for Launchpad bugs, + # and so we reduce all URLs to this. + $result = "https://launchpad.net/bugs/$1"; + } + else { + ThrowUserError('bug_url_invalid', + { url => $input, reason => 'id' }); + } + } + # Bugzilla URLs + else { + if ($uri->path !~ /show_bug\.cgi$/) { + ThrowUserError('bug_url_invalid', + { url => $input, reason => 'show_bug' }); + } + + my $bug_id = $uri->query_param('id'); + # We don't currently allow aliases, because we can't check to see + # if somebody's putting both an alias link and a numeric ID link. + # When we start validating the URL by accessing the other Bugzilla, + # we can allow aliases. + detaint_natural($bug_id); + if (!$bug_id) { + ThrowUserError('bug_url_invalid', + { url => $input, reason => 'id' }); + } + + # Make sure that "id" is the only query parameter. + $uri->query("id=$bug_id"); + # And remove any # part if there is one. + $uri->fragment(undef); + $result = $uri->canonical->as_string; + } + + if (length($result) > MAX_BUG_URL_LENGTH) { + ThrowUserError('bug_url_too_long', { url => $result }); + } + + # We only add the new URI if it hasn't been added yet. URIs are + # case-sensitive, but most of our DBs are case-insensitive, so we do + # this check case-insensitively. + if (!grep { lc($_) eq lc($result) } @{ $self->see_also }) { + my $privs; + my $can = $self->check_can_change_field('see_also', '', $result, \$privs); + if (!$can) { + ThrowUserError('illegal_change', { field => 'see_also', + newvalue => $result, + privs => $privs }); + } + + push(@{ $self->see_also }, $result); + } +} + +sub remove_see_also { + my ($self, $url) = @_; + my $see_also = $self->see_also; + my @new_see_also = grep { lc($_) ne lc($url) } @$see_also; + my $privs; + my $can = $self->check_can_change_field('see_also', $see_also, \@new_see_also, \$privs); + if (!$can) { + ThrowUserError('illegal_change', { field => 'see_also', + oldvalue => $url, + privs => $privs }); + } + $self->{see_also} = \@new_see_also; +} + ##################################################################### # Instance Accessors ##################################################################### @@ -2347,22 +2598,8 @@ sub attachments { return $self->{'attachments'} if exists $self->{'attachments'}; return [] if $self->{'error'}; - my $attachments = Bugzilla::Attachment->get_attachments_by_bug($self->bug_id); - $_->{'flags'} = [] foreach @$attachments; - my %att = map { $_->id => $_ } @$attachments; - - # Retrieve all attachment flags at once for this bug, and group them - # by attachment. We populate attachment flags here to avoid querying - # the DB for each attachment individually later. - my $flags = Bugzilla::Flag->match({ 'bug_id' => $self->bug_id, - 'target_type' => 'attachment' }); - - # Exclude flags for private attachments you cannot see. - @$flags = grep {exists $att{$_->attach_id}} @$flags; - - push(@{$att{$_->attach_id}->{'flags'}}, $_) foreach @$flags; - - $self->{'attachments'} = [sort {$a->id <=> $b->id} values %att]; + $self->{'attachments'} = + Bugzilla::Attachment->get_attachments_by_bug($self->bug_id, {preload => 1}); return $self->{'attachments'}; } @@ -2469,28 +2706,12 @@ sub flag_types { return $self->{'flag_types'} if exists $self->{'flag_types'}; return [] if $self->{'error'}; - # The types of flags that can be set on this bug. - # If none, no UI for setting flags will be displayed. - my $flag_types = Bugzilla::FlagType::match( - {'target_type' => 'bug', - 'product_id' => $self->{'product_id'}, - 'component_id' => $self->{'component_id'} }); - - $_->{'flags'} = [] foreach @$flag_types; - my %flagtypes = map { $_->id => $_ } @$flag_types; - - # Retrieve all bug flags at once for this bug and group them - # by flag types. - my $flags = Bugzilla::Flag->match({ 'bug_id' => $self->bug_id, - 'target_type' => 'bug' }); - - # Call the internal 'type_id' variable instead of the method - # to not create a flagtype object. - push(@{$flagtypes{$_->{'type_id'}}->{'flags'}}, $_) foreach @$flags; - - $self->{'flag_types'} = - [sort {$a->sortkey <=> $b->sortkey || $a->name cmp $b->name} values %flagtypes]; + my $vars = { target_type => 'bug', + product_id => $self->{product_id}, + component_id => $self->{component_id}, + bug_id => $self->bug_id }; + $self->{'flag_types'} = Bugzilla::Flag::_flag_types($vars); return $self->{'flag_types'}; } @@ -2581,6 +2802,14 @@ sub reporter { return $self->{'reporter'}; } +sub see_also { + my ($self) = @_; + return [] if $self->{'error'}; + $self->{'see_also'} ||= Bugzilla->dbh->selectcol_arrayref( + 'SELECT value FROM bug_see_also WHERE bug_id = ?', undef, $self->id); + return $self->{'see_also'}; +} + sub status { my $self = shift; return undef if $self->{'error'}; @@ -2850,12 +3079,18 @@ sub editable_bug_fields { # XXX - When Bug::update() will be implemented, we should make this routine # a private method. +# Join with bug_status and bugs tables to show bugs with open statuses first, +# and then the others sub EmitDependList { my ($myfield, $targetfield, $bug_id) = (@_); my $dbh = Bugzilla->dbh; my $list_ref = $dbh->selectcol_arrayref( - "SELECT $targetfield FROM dependencies - WHERE $myfield = ? ORDER BY $targetfield", + "SELECT $targetfield + FROM dependencies + INNER JOIN bugs ON dependencies.$targetfield = bugs.bug_id + INNER JOIN bug_status ON bugs.bug_status = bug_status.value + WHERE $myfield = ? + ORDER BY is_open DESC, $targetfield", undef, $bug_id); return $list_ref; } @@ -2903,18 +3138,27 @@ sub GetComments { INNER JOIN profiles ON profiles.userid = longdescs.who WHERE longdescs.bug_id = ?'; + if ($start) { - $query .= ' AND longdescs.bug_when > ? - AND longdescs.bug_when <= ?'; - push(@args, ($start, $end)); + $query .= ' AND longdescs.bug_when > ?'; + push(@args, $start); } + if ($end) { + $query .= ' AND longdescs.bug_when <= ?'; + push(@args, $end); + } + $query .= " ORDER BY longdescs.bug_when $sort_order"; my $sth = $dbh->prepare($query); $sth->execute(@args); + # Cache the users we look up + my %users; + while (my $comment_ref = $sth->fetchrow_hashref()) { my %comment = %$comment_ref; - $comment{'author'} = new Bugzilla::User($comment{'userid'}); + $users{$comment{'userid'}} ||= new Bugzilla::User($comment{'userid'}); + $comment{'author'} = $users{$comment{'userid'}}; # If raw data is requested, do not format 'special' comments. $comment{'body'} = format_comment(\%comment) unless $raw; @@ -2929,34 +3173,20 @@ sub GetComments { return \@comments; } -# Format language specific comments. This routine must not update -# $comment{'body'} itself, see BugMail::prepare_comments(). +# Format language specific comments. sub format_comment { my $comment = shift; + my $template = Bugzilla->template_inner; + my $vars = {comment => $comment}; my $body; - if ($comment->{'type'} == CMT_DUPE_OF) { - $body = $comment->{'body'} . "\n\n" . - get_text('bug_duplicate_of', { dupe_of => $comment->{'extra_data'} }); - } - elsif ($comment->{'type'} == CMT_HAS_DUPE) { - $body = get_text('bug_has_duplicate', { dupe => $comment->{'extra_data'} }); - } - elsif ($comment->{'type'} == CMT_POPULAR_VOTES) { - $body = get_text('bug_confirmed_by_votes'); - } - elsif ($comment->{'type'} == CMT_MOVED_TO) { - $body = $comment->{'body'} . "\n\n" . - get_text('bug_moved_to', { login => $comment->{'extra_data'} }); - } - else { - $body = $comment->{'body'}; - } + $template->process("bug/format_comment.txt.tmpl", $vars, \$body) + || ThrowTemplateError($template->error()); return $body; } # Get the activity of a bug, starting from $starttime (if given). -# This routine assumes ValidateBugID has been previously called. +# This routine assumes Bugzilla::Bug->check has been previously called. sub GetBugActivity { my ($bug_id, $attach_id, $starttime) = @_; my $dbh = Bugzilla->dbh; @@ -3224,7 +3454,7 @@ sub RemoveVotes { undef, ($votes, $id)); } # Now return the array containing emails to be sent. - return \@messages; + return @messages; } # If a user votes for a bug, or the number of votes required to @@ -3420,59 +3650,6 @@ sub check_can_change_field { # Field Validation # -# Validates and verifies a bug ID, making sure the number is a -# positive integer, that it represents an existing bug in the -# database, and that the user is authorized to access that bug. -# We detaint the number here, too. -sub ValidateBugID { - my ($id, $field) = @_; - my $dbh = Bugzilla->dbh; - my $user = Bugzilla->user; - - ThrowUserError('improper_bug_id_field_value', { field => $field }) unless defined $id; - - # Get rid of leading '#' (number) mark, if present. - $id =~ s/^\s*#//; - # Remove whitespace - $id = trim($id); - - # If the ID isn't a number, it might be an alias, so try to convert it. - my $alias = $id; - if (!detaint_natural($id)) { - $id = bug_alias_to_id($alias); - $id || ThrowUserError("improper_bug_id_field_value", - {'bug_id' => $alias, - 'field' => $field }); - } - - # Modify the calling code's original variable to contain the trimmed, - # converted-from-alias ID. - $_[0] = $id; - - # First check that the bug exists - $dbh->selectrow_array("SELECT bug_id FROM bugs WHERE bug_id = ?", undef, $id) - || ThrowUserError("bug_id_does_not_exist", {'bug_id' => $id}); - - unless ($field && $field =~ /^(dependson|blocked|dup_id)$/) { - check_is_visible($id); - } -} - -sub check_is_visible { - my $id = shift; - my $user = Bugzilla->user; - - return if $user->can_see_bug($id); - - # The error the user sees depends on whether or not they are logged in - # (i.e. $user->id contains the user's positive integer ID). - if ($user->id) { - ThrowUserError("bug_access_denied", {'bug_id' => $id}); - } else { - ThrowUserError("bug_access_query", {'bug_id' => $id}); - } -} - # Validate and return a hash of dependencies sub ValidateDependencies { my $fields = {}; diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index ff830a86b..7f322f1b7 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -217,6 +217,15 @@ sub Send { $values{'blocked'} = join(",", @$blockedlist); + my $grouplist = $dbh->selectcol_arrayref( + ' SELECT name FROM groups + INNER JOIN bug_group_map + ON groups.id = bug_group_map.group_id + AND bug_group_map.bug_id = ?', + undef, ($id)); + + $values{'bug_group'} = join(', ', @$grouplist); + my @args = ($id); # If lastdiffed is NULL, then we don't limit the search on time. @@ -376,7 +385,7 @@ sub Send { } } - my ($raw_comments, $anyprivate, $count) = get_comments_by_bug($id, $start, $end); + my ($comments, $anyprivate) = get_comments_by_bug($id, $start, $end); ########################################################################### # Start of email filtering code @@ -438,7 +447,6 @@ sub Send { } } - if (Bugzilla->params->{"supportwatchers"}) { # Find all those user-watching anyone on the current list, who is not # on it already themselves. my $involved = join(",", keys %recipients); @@ -453,8 +461,7 @@ sub Send { $recipients{$watch->[0]}->{$role} |= BIT_WATCHING if $bits & BIT_DIRECT; } - push (@{$watching{$watch->[0]}}, $watch->[1]); - } + push(@{$watching{$watch->[0]}}, $watch->[1]); } # Global watcher @@ -471,10 +478,6 @@ sub Send { my @sent; my @excluded; - # Some comments are language specific. We cache them here. - my %comments; - my %commentArray; - foreach my $user_id (keys %recipients) { my %rels_which_want; my $sent_mail = 0; @@ -483,25 +486,14 @@ sub Send { # Deleted users must be excluded. next unless $user; - # What's the language chosen by this user for email? - my $lang = $user->settings->{'lang'}->{'value'}; - if ($user->can_see_bug($id)) { - # It's time to format language specific comments. - unless (exists $comments{$lang}) { - Bugzilla->template_inner($lang); - $comments{$lang} = prepare_comments($raw_comments, $count); - $commentArray{$lang} = $raw_comments; - Bugzilla->template_inner(""); - } - # Go through each role the user has and see if they want mail in # that role. foreach my $relationship (keys %{$recipients{$user_id}}) { if ($user->wants_bug_mail($id, $relationship, $diffs, - $comments{$lang}, + $comments, $deptext, $changer, !$start)) @@ -519,9 +511,7 @@ sub Send { # If we are using insiders, and the comment is private, only send # to insiders my $insider_ok = 1; - $insider_ok = 0 if (Bugzilla->params->{"insidergroup"} && - ($anyprivate != 0) && - (!$user->groups->{Bugzilla->params->{"insidergroup"}})); + $insider_ok = 0 if $anyprivate && !$user->is_insider; # We shouldn't send mail if this is a dependency mail (i.e. there # is something in @depbugs), and any of the depending bugs are not @@ -551,8 +541,7 @@ sub Send { fields => \%fielddescription, diffs => \@diffparts, diffar => \@diff_array, - newcomm => $comments{$lang} || [], - commarr => $commentArray{$lang} || [], + newcomm => $comments, anypriv => $anyprivate, isnew => !$start, id => $id, @@ -579,11 +568,11 @@ sub sendMail { my %arguments = @_; my ($user, $hlRef, $relRef, $valueRef, $dmhRef, $fdRef, - $diffRef, $diffArray, $newcomments, $commentArray, $anyprivate, $isnew, + $diffRef, $diffArray, $newcomments, $anyprivate, $isnew, $id, $watchingRef ) = @arguments{qw( user headers rels values defhead fields - diffs diffar newcomm commarr anypriv isnew + diffs diffar newcomm anypriv isnew id watch )}; @@ -606,13 +595,12 @@ sub sendMail ($diff->{'fieldname'} eq 'estimated_time' || $diff->{'fieldname'} eq 'remaining_time' || $diff->{'fieldname'} eq 'work_time' || - $diff->{'fieldname'} eq 'deadline')) { - if ($user->groups->{Bugzilla->params->{"timetrackinggroup"}}) { - $add_diff = 1; - } - } elsif ($diff->{'isprivate'} && - Bugzilla->params->{'insidergroup'} && - !$user->groups->{Bugzilla->params->{'insidergroup'}}) { + $diff->{'fieldname'} eq 'deadline')) + { + $add_diff = 1 if $user->is_timetracker; + } elsif ($diff->{'isprivate'} + && !$user->is_insider) + { $add_diff = 0; } else { $add_diff = 1; @@ -628,7 +616,7 @@ sub sendMail } } - if ($difftext eq "" && $newcomments eq "" && !$isnew) { + if (!$difftext && !$newcomments && !@$newcomments && !$isnew) { # Whoops, no differences! return 0; } @@ -636,18 +624,14 @@ sub sendMail # If an attachment was created, then add an URL. (Note: the 'g'lobal # replace should work with comments with multiple attachments.) - if ($newcomments =~ /Created an attachment \(/) { - my $showattachurlbase = - Bugzilla->params->{'urlbase'} . "attachment.cgi?id="; - - $newcomments =~ s/(Created an attachment \(id=([0-9]+)\))/$1\n --> \(${showattachurlbase}$2\)/g; - for (@$commentArray) - { - $_->{body} =~ s/Created an attachment \(id=([0-9]+)\)/Created an attachment<\/a>/g; - } + my $showattachurlbase = + Bugzilla->params->{'urlbase'} . "attachment.cgi?id="; + for (@$newcomments) + { + $_->{body} =~ s/Created an attachment \(id=([0-9]+)\)/Created an attachment<\/a>/g; } - my $diffs = $difftext . "\n\n" . $newcomments; + my $diffs = $difftext; if ($isnew) { my $head = ""; foreach my $f (@headerlist) { @@ -656,9 +640,7 @@ sub sendMail # If there isn't anything to show, don't include this header. next unless $value; # Only send estimated_time if it is enabled and the user is in the group. - if (($f ne 'estimated_time' && $f ne 'deadline') - || $user->groups->{Bugzilla->params->{'timetrackinggroup'}}) - { + if (($f ne 'estimated_time' && $f ne 'deadline') || $user->is_timetracker) { my $desc = $fielddescription{$f}; $head .= multiline_sprintf(FORMAT_DOUBLE, ["$desc:", $value], FORMAT_2_SIZE); @@ -679,8 +661,6 @@ sub sendMail push @watchingrel, 'None' unless @watchingrel; push @watchingrel, map { user_id_to_login($_) } @$watchingRef; - my $threadingmarker = build_thread_marker($id, $user->id, $isnew); - my $vars = { isnew => $isnew, to => $user->email, @@ -708,9 +688,8 @@ sub sendMail reportername => Bugzilla::User->new({name => $values{'reporter'}})->name, diffs => $diffs, diffarray => $diffArray, - threadingmarker => $threadingmarker, - newcomments => $newcomments, - commentarray => $commentArray, + new_comments => $newcomments, + threadingmarker => build_thread_marker($id, $user->id, $isnew), }; my $msg; @@ -749,32 +728,15 @@ sub get_comments_by_bug { my $raw = 1; # Do not format comments which are not of type CMT_NORMAL. my $comments = Bugzilla::Bug::GetComments($id, "oldest_to_newest", $start, $end, $raw); + foreach my $comment (@$comments) { + $comment->{count} = $count++; + } + if (Bugzilla->params->{'insidergroup'}) { $anyprivate = 1 if scalar(grep {$_->{'isprivate'} > 0} @$comments); } - return ($comments, $anyprivate, $count); -} - -# Prepare comments for the given language. -sub prepare_comments { - my ($raw_comments, $count) = @_; - - my $result = ""; - foreach my $comment (@$raw_comments) { - if ($count) { - $comment->{seqnum} = $count; - $result .= "\n\n--- Comment #$count from " . $comment->{'author'}->identity . - " " . format_time($comment->{'time'}) . " ---\n"; - } - # Format language specific comments. We don't update $comment->{'body'} - # directly, otherwise it would grow everytime you call format_comment() - # with a different language as some text may be appended to the existing one. - my $body = Bugzilla::Bug::format_comment($comment); - $result .= ($comment->{'already_wrapped'} ? $body : wrap_comment($body)); - $count++; - } - return $result; + return ($comments, $anyprivate); } 1; diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index 1799786d3..c0aba5cd4 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -121,6 +121,10 @@ sub canonicalise_query { # Leave this key out if it's in the exclude list next if lsearch(\@exclude, $key) != -1; + # Remove the Boolean Charts for standard query.cgi fields + # They are listed in the query URL already + next if $key =~ /^(field|type|value)(-\d+){3}$/; + my $esc_key = url_quote($key); foreach my $value ($self->param($key)) { @@ -137,7 +141,7 @@ sub canonicalise_query { sub clean_search_url { my $self = shift; - # Delete any empty URL parameter + # Delete any empty URL parameter. my @cgi_params = $self->param; foreach my $param (@cgi_params) { @@ -156,8 +160,50 @@ sub clean_search_url { # Delete certain parameters if the associated parameter is empty. $self->delete('bugidtype') if !$self->param('bug_id'); - $self->delete('emailtype1') if !$self->param('email1'); - $self->delete('emailtype2') if !$self->param('email2'); + + # Delete leftovers from the login form + $self->delete('Bugzilla_remember', 'GoAheadAndLogIn'); + + foreach my $num (1,2) { + # If there's no value in the email field, delete the related fields. + if (!$self->param("email$num")) { + foreach my $field qw(type assigned_to reporter qa_contact + cc longdesc) + { + $self->delete("email$field$num"); + } + } + } + + # chfieldto is set to "Now" by default in query.cgi. But if none + # of the other chfield parameters are set, it's meaningless. + if (!defined $self->param('chfieldfrom') && !$self->param('chfield') + && !defined $self->param('chfieldvalue')) + { + $self->delete('chfieldto'); + } + + # cmdtype "doit" is the default from query.cgi, but it's only meaningful + # if there's a remtype parameter. + if (defined $self->param('cmdtype') && $self->param('cmdtype') eq 'doit' + && !defined $self->param('remtype')) + { + $self->delete('cmdtype'); + } + + # "Reuse same sort as last time" is actually the default, so we don't + # need it in the URL. + if ($self->param('order') + && $self->param('order') eq 'Reuse same sort as last time') + { + $self->delete('order'); + } + + # And now finally, if query_format is our only parameter, that + # really means we have no parameters, so we should delete query_format. + if ($self->param('query_format') && scalar($self->param()) == 1) { + $self->delete('query_format'); + } } # Overwrite to ensure nph doesn't get set, and unset HEADERS_ONCE @@ -233,17 +279,41 @@ sub header { return $self->SUPER::header(@_) || ""; } -# CGI.pm is not utf8-aware and passes data as bytes instead of UTF-8 strings. sub param { my $self = shift; - if (Bugzilla->params->{'utf8'} && scalar(@_) == 1) { - if (wantarray) { - return map { _fix_utf8($_) } $self->SUPER::param(@_); + + # When we are just requesting the value of a parameter... + if (scalar(@_) == 1) { + my @result = $self->SUPER::param(@_); + + # Also look at the URL parameters, after we look at the POST + # parameters. This is to allow things like login-form submissions + # with URL parameters in the form's "target" attribute. + if (!scalar(@result) + && $self->request_method && $self->request_method eq 'POST') + { + @result = $self->SUPER::url_param(@_); } - else { - return _fix_utf8(scalar $self->SUPER::param(@_)); + + # Fix UTF-8-ness of input parameters. + if (Bugzilla->params->{'utf8'}) { + @result = map { _fix_utf8($_) } @result; } + + return wantarray ? @result : $result[0]; + } + # And for various other functions in CGI.pm, we need to correctly + # return the URL parameters in addition to the POST parameters when + # asked for the list of parameters. + elsif (!scalar(@_) && $self->request_method + && $self->request_method eq 'POST') + { + my @post_params = $self->SUPER::param; + my @url_params = $self->url_param; + my %params = map { $_ => 1 } (@post_params, @url_params); + return keys %params; } + return $self->SUPER::param(@_); } diff --git a/Bugzilla/Chart.pm b/Bugzilla/Chart.pm index a119e4b7c..1f232f310 100644 --- a/Bugzilla/Chart.pm +++ b/Bugzilla/Chart.pm @@ -382,8 +382,7 @@ sub getSeriesIDs { sub getVisibleSeries { my %cats; - # List of groups the user is in; use -1 to make sure it's not empty. - my $grouplist = join(", ", (-1, values(%{Bugzilla->user->groups}))); + my $grouplist = Bugzilla->user->groups_as_string; # Get all visible series my $dbh = Bugzilla->dbh; diff --git a/Bugzilla/Classification.pm b/Bugzilla/Classification.pm index 37ae3a0cc..a7f59b4bb 100644 --- a/Bugzilla/Classification.pm +++ b/Bugzilla/Classification.pm @@ -13,77 +13,115 @@ # The Original Code is the Bugzilla Bug Tracking System. # # Contributor(s): Tiago R. Mello -# +# Frédéric Buclin use strict; package Bugzilla::Classification; +use Bugzilla::Constants; use Bugzilla::Util; use Bugzilla::Error; use Bugzilla::Product; +use base qw(Bugzilla::Object); + ############################### #### Initialization #### ############################### +use constant DB_TABLE => 'classifications'; +use constant LIST_ORDER => 'sortkey, name'; + use constant DB_COLUMNS => qw( - classifications.id - classifications.name - classifications.description - classifications.sortkey + id + name + description + sortkey ); -our $columns = join(", ", DB_COLUMNS); +use constant REQUIRED_CREATE_FIELDS => qw( + name +); + +use constant UPDATE_COLUMNS => qw( + name + description + sortkey +); + +use constant VALIDATORS => { + name => \&_check_name, + description => \&_check_description, + sortkey => \&_check_sortkey, +}; + +############################### +#### Constructors ##### +############################### +sub remove_from_db { + my $self = shift; + my $dbh = Bugzilla->dbh; + + ThrowUserError("classification_not_deletable") if ($self->id == 1); + + $dbh->bz_start_transaction(); + # Reclassify products to the default classification, if needed. + $dbh->do("UPDATE products SET classification_id = 1 + WHERE classification_id = ?", undef, $self->id); + + $dbh->do("DELETE FROM classifications WHERE id = ?", undef, $self->id); + + $dbh->bz_commit_transaction(); + +} + +############################### +#### Validators #### +############################### + +sub _check_name { + my ($invocant, $name) = @_; + + $name = trim($name); + $name || ThrowUserError('classification_not_specified'); + + if (length($name) > MAX_CLASSIFICATION_SIZE) { + ThrowUserError('classification_name_too_long', {'name' => $name}); + } + + my $classification = new Bugzilla::Classification({name => $name}); + if ($classification && (!ref $invocant || $classification->id != $invocant->id)) { + ThrowUserError("classification_already_exists", { name => $classification->name }); + } + return $name; +} + +sub _check_description { + my ($invocant, $description) = @_; + + $description = trim($description || ''); + return $description; +} + +sub _check_sortkey { + my ($invocant, $sortkey) = @_; + + $sortkey ||= 0; + my $stored_sortkey = $sortkey; + if (!detaint_natural($sortkey) || $sortkey > MAX_SMALLINT) { + ThrowUserError('classification_invalid_sortkey', { 'sortkey' => $stored_sortkey }); + } + return $sortkey; +} ############################### #### Methods #### ############################### -sub new { - my $invocant = shift; - my $class = ref($invocant) || $invocant; - my $self = {}; - bless($self, $class); - return $self->_init(@_); -} - -sub _init { - my $self = shift; - my ($param) = @_; - my $dbh = Bugzilla->dbh; - - my $id = $param unless (ref $param eq 'HASH'); - my $classification; - - if (defined $id) { - detaint_natural($id) - || ThrowCodeError('param_must_be_numeric', - {function => 'Bugzilla::Classification::_init'}); - - $classification = $dbh->selectrow_hashref(qq{ - SELECT $columns FROM classifications - WHERE id = ?}, undef, $id); - - } elsif (defined $param->{'name'}) { - - trick_taint($param->{'name'}); - $classification = $dbh->selectrow_hashref(qq{ - SELECT $columns FROM classifications - WHERE name = ?}, undef, $param->{'name'}); - } else { - ThrowCodeError('bad_arg', - {argument => 'param', - function => 'Bugzilla::Classification::_init'}); - } - - return undef unless (defined $classification); - - foreach my $field (keys %$classification) { - $self->{$field} = $classification->{$field}; - } - return $self; -} +sub set_name { $_[0]->set('name', $_[1]); } +sub set_description { $_[0]->set('description', $_[1]); } +sub set_sortkey { $_[0]->set('sortkey', $_[1]); } sub product_count { my $self = shift; @@ -116,46 +154,9 @@ sub products { #### Accessors #### ############################### -sub id { return $_[0]->{'id'}; } -sub name { return $_[0]->{'name'}; } sub description { return $_[0]->{'description'}; } sub sortkey { return $_[0]->{'sortkey'}; } -############################### -#### Subroutines #### -############################### - -sub get_all_classifications { - my $dbh = Bugzilla->dbh; - - my $ids = $dbh->selectcol_arrayref(q{ - SELECT id FROM classifications ORDER BY sortkey, name}); - - my @classifications; - foreach my $id (@$ids) { - push @classifications, new Bugzilla::Classification($id); - } - return @classifications; -} - -sub check_classification { - my ($class_name) = @_; - - unless ($class_name) { - ThrowUserError("classification_not_specified"); - } - - my $classification = - new Bugzilla::Classification({name => $class_name}); - - unless ($classification) { - ThrowUserError("classification_doesnt_exist", - { name => $class_name }); - } - - return $classification; -} - 1; __END__ @@ -174,18 +175,18 @@ Bugzilla::Classification - Bugzilla classification class. my $id = $classification->id; my $name = $classification->name; my $description = $classification->description; + my $sortkey = $classification->sortkey; my $product_count = $classification->product_count; my $products = $classification->products; - my $hash_ref = Bugzilla::Classification::get_all_classifications(); - my $classification = $hash_ref->{1}; - - my $classification = - Bugzilla::Classification::check_classification('AcmeClass'); - =head1 DESCRIPTION -Classification.pm represents a Classification object. +Classification.pm represents a classification object. It is an +implementation of L, and thus provides all methods +that L provides. + +The methods that are specific to C are listed +below. A Classification is a higher-level grouping of Products. @@ -193,20 +194,6 @@ A Classification is a higher-level grouping of Products. =over -=item C - - Description: The constructor is used to load an existing - classification by passing a classification - id or classification name using a hash. - - Params: $param - If you pass an integer, the integer is the - classification_id from the database that we - want to read in. If you pass in a hash with - 'name' key, then the value of the name key - is the name of a classification from the DB. - - Returns: A Bugzilla::Classification object. - =item C Description: Returns the total number of products that belong to @@ -226,27 +213,4 @@ A Classification is a higher-level grouping of Products. =back -=head1 SUBROUTINES - -=over - -=item C - - Description: Returns all classifications. - - Params: none. - - Returns: Bugzilla::Classification object list. - -=item C - - Description: Checks if the classification name passed in is a - valid classification. - - Params: $classification_name - String with a classification name. - - Returns: Bugzilla::Classification object. - -=back - =cut diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm index 619eb05a8..14f10bed9 100644 --- a/Bugzilla/Config.pm +++ b/Bugzilla/Config.pm @@ -34,6 +34,7 @@ use strict; use base qw(Exporter); use Bugzilla::Constants; +use Bugzilla::Hook; use Data::Dumper; use File::Temp; @@ -54,15 +55,21 @@ our %params; # Load in the param definitions sub _load_params { my $panels = param_panels(); + my %hook_panels; foreach my $panel (keys %$panels) { my $module = $panels->{$panel}; eval("require $module") || die $@; - my @new_param_list = "$module"->get_param_list(); + my @new_param_list = $module->get_param_list(); + $hook_panels{lc($panel)} = { params => \@new_param_list }; foreach my $item (@new_param_list) { $params{$item->{'name'}} = $item; } push(@param_list, @new_param_list); } + # This hook is also called in editparams.cgi. This call here is required + # to make SetParam work. + Bugzilla::Hook::process('config-modify_panels', + { panels => \%hook_panels }); } # END INIT CODE @@ -77,7 +84,8 @@ sub param_panels { $param_panels->{$module} = "Bugzilla::Config::$module" unless $module eq 'Common'; } # Now check for any hooked params - Bugzilla::Hook::process('config', { config => $param_panels }); + Bugzilla::Hook::process('config-add_panels', + { panel_modules => $param_panels }); return $param_panels; } diff --git a/Bugzilla/Config/Admin.pm b/Bugzilla/Config/Admin.pm index 838e53295..d4e822816 100644 --- a/Bugzilla/Config/Admin.pm +++ b/Bugzilla/Config/Admin.pm @@ -49,20 +49,14 @@ sub get_param_list { { name => 'allowemailchange', type => 'b', - default => 0 + default => 1 }, { name => 'allowuserdeletion', type => 'b', default => 0 - }, - - { - name => 'supportwatchers', - type => 'b', - default => 0 - } ); + }); return @param_list; } diff --git a/Bugzilla/Config/Attachment.pm b/Bugzilla/Config/Attachment.pm index 42c5c2538..7dd9f0d49 100644 --- a/Bugzilla/Config/Attachment.pm +++ b/Bugzilla/Config/Attachment.pm @@ -65,13 +65,6 @@ sub get_param_list { default => 0 }, - { - name => 'maxpatchsize', - type => 't', - default => '1000', - checker => \&check_numeric - }, - { name => 'maxattachmentsize', type => 't', diff --git a/Bugzilla/Config/BugChange.pm b/Bugzilla/Config/BugChange.pm index aec6e2428..0e518b689 100644 --- a/Bugzilla/Config/BugChange.pm +++ b/Bugzilla/Config/BugChange.pm @@ -80,24 +80,12 @@ sub get_param_list { default => 0 }, - { - name => 'commentonclearresolution', - type => 'b', - default => 0 - }, - { name => 'commentonchange_resolution', type => 'b', default => 0 }, - { - name => 'commentonreassignbycomponent', - type => 'b', - default => 0 - }, - { name => 'commentonduplicate', type => 'b', diff --git a/Bugzilla/Config/BugFields.pm b/Bugzilla/Config/BugFields.pm index a6ad75c2b..cca01071b 100644 --- a/Bugzilla/Config/BugFields.pm +++ b/Bugzilla/Config/BugFields.pm @@ -53,12 +53,6 @@ sub get_param_list { default => 0 }, - { - name => 'showallproducts', - type => 'b', - default => 0 - }, - { name => 'usetargetmilestone', type => 'b', @@ -89,6 +83,12 @@ sub get_param_list { default => 0 }, + { + name => 'use_see_also', + type => 'b', + default => 1 + }, + { name => 'defaultpriority', type => 's', diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm index 5b2cabb93..b285b3bc9 100644 --- a/Bugzilla/Config/Common.pm +++ b/Bugzilla/Config/Common.pm @@ -35,7 +35,6 @@ package Bugzilla::Config::Common; use strict; use Socket; -use Time::Zone; use Bugzilla::Util; use Bugzilla::Constants; @@ -49,8 +48,8 @@ use base qw(Exporter); check_sslbase check_priority check_severity check_platform check_opsys check_shadowdb check_urlbase check_webdotbase check_netmask check_user_verify_class check_image_converter - check_mail_delivery_method check_notification check_timezone check_utf8 - check_bug_status check_smtp_auth + check_mail_delivery_method check_notification check_utf8 + check_bug_status check_smtp_auth check_theschwartz_available check_maxattachmentsize ); @@ -278,10 +277,7 @@ sub check_user_verify_class { for my $class (split /,\s*/, $list) { my $res = check_multi($class, $entry); return $res if $res; - if ($class eq 'DB') { - # No params - } - elsif ($class eq 'RADIUS') { + if ($class eq 'RADIUS') { eval "require Authen::Radius"; return "Error requiring Authen::Radius: '$@'" if $@; return "RADIUS servername (RADIUS_server) is missing" unless Bugzilla->params->{"RADIUS_server"}; @@ -293,9 +289,6 @@ sub check_user_verify_class { return "LDAP servername (LDAPserver) is missing" unless Bugzilla->params->{"LDAPserver"}; return "LDAPBaseDN is empty" unless Bugzilla->params->{"LDAPBaseDN"}; } - else { - return "Unknown user_verify_class '$class' in check_user_verify_class"; - } } return ""; } @@ -352,14 +345,6 @@ sub check_notification { return ""; } -sub check_timezone { - my $tz = shift; - unless (defined(tz_offset($tz))) { - return "must be empty or a legal timezone name, such as PDT or JST"; - } - return ""; -} - sub check_smtp_auth { my $username = shift; if ($username) { @@ -369,6 +354,15 @@ sub check_smtp_auth { return ""; } +sub check_theschwartz_available { + if (!eval { require TheSchwartz; require Daemon::Generic; }) { + return "Using the job queue requires that you have certain Perl" + . " modules installed. See the output of checksetup.pl" + . " for more information"; + } + return ""; +} + # OK, here are the parameter definitions themselves. # # Each definition is a hash with keys: diff --git a/Bugzilla/Config/Core.pm b/Bugzilla/Config/Core.pm index b307dd7f5..6d413b965 100644 --- a/Bugzilla/Config/Core.pm +++ b/Bugzilla/Config/Core.pm @@ -87,13 +87,6 @@ sub get_param_list { default => '/' }, - { - name => 'timezone', - type => 't', - default => '', - checker => \&check_timezone - }, - { name => 'utf8', type => 'b', diff --git a/Bugzilla/Config/MTA.pm b/Bugzilla/Config/MTA.pm index 37d99d967..c7843e286 100644 --- a/Bugzilla/Config/MTA.pm +++ b/Bugzilla/Config/MTA.pm @@ -57,6 +57,13 @@ sub get_param_list { default => 'bugzilla-daemon' }, + { + name => 'use_mailer_queue', + type => 'b', + default => 0, + checker => \&check_theschwartz_available, + }, + { name => 'sendmailnow', type => 'b', @@ -90,7 +97,6 @@ sub get_param_list { default => 7, checker => \&check_numeric }, - { name => 'globalwatchers', type => 't', diff --git a/Bugzilla/Config/Query.pm b/Bugzilla/Config/Query.pm index 74e265037..fbfdb4c22 100644 --- a/Bugzilla/Config/Query.pm +++ b/Bugzilla/Config/Query.pm @@ -77,7 +77,7 @@ sub get_param_list { { name => 'specific_search_allow_empty_words', type => 'b', - default => 0 + default => 1 } ); diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 0644e102d..8c21317ea 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -101,6 +101,7 @@ use File::Basename; POS_EVENTS EVT_OTHER EVT_ADDED_REMOVED EVT_COMMENT EVT_ATTACHMENT EVT_ATTACHMENT_DATA EVT_PROJ_MANAGEMENT EVT_OPENED_CLOSED EVT_KEYWORD EVT_CC EVT_DEPEND_BLOCK + EVT_BUG_CREATED NEG_EVENTS EVT_UNCONFIRMED EVT_CHANGED_BY_ME @@ -122,6 +123,8 @@ use File::Basename; FIELD_TYPE_MULTI_SELECT FIELD_TYPE_TEXTAREA FIELD_TYPE_DATETIME + FIELD_TYPE_BUG_ID + FIELD_TYPE_BUG_URLS USAGE_MODE_BROWSER USAGE_MODE_CMDLINE @@ -149,9 +152,16 @@ use File::Basename; MAX_SMALLINT MAX_LEN_QUERY_NAME + MAX_CLASSIFICATION_SIZE + MAX_PRODUCT_SIZE MAX_MILESTONE_SIZE MAX_COMPONENT_SIZE + MAX_FIELD_VALUE_SIZE MAX_FREETEXT_LENGTH + MAX_BUG_URL_LENGTH + + PASSWORD_DIGEST_ALGORITHM + PASSWORD_SALT_LENGTH ); @Bugzilla::Constants::EXPORT_OK = qw(contenttypes); @@ -159,7 +169,7 @@ use File::Basename; # CONSTANTS # # Bugzilla version -use constant BUGZILLA_VERSION => "3.2.4"; +use constant BUGZILLA_VERSION => "3.4"; # These are unique values that are unlikely to match a string or a number, # to be used in criteria for match() functions and other things. They start @@ -306,11 +316,12 @@ use constant EVT_OPENED_CLOSED => 6; use constant EVT_KEYWORD => 7; use constant EVT_CC => 8; use constant EVT_DEPEND_BLOCK => 9; +use constant EVT_BUG_CREATED => 10; use constant POS_EVENTS => EVT_OTHER, EVT_ADDED_REMOVED, EVT_COMMENT, EVT_ATTACHMENT, EVT_ATTACHMENT_DATA, EVT_PROJ_MANAGEMENT, EVT_OPENED_CLOSED, EVT_KEYWORD, - EVT_CC, EVT_DEPEND_BLOCK; + EVT_CC, EVT_DEPEND_BLOCK, EVT_BUG_CREATED; use constant EVT_UNCONFIRMED => 50; use constant EVT_CHANGED_BY_ME => 51; @@ -352,6 +363,8 @@ use constant FIELD_TYPE_SINGLE_SELECT => 2; use constant FIELD_TYPE_MULTI_SELECT => 3; use constant FIELD_TYPE_TEXTAREA => 4; use constant FIELD_TYPE_DATETIME => 5; +use constant FIELD_TYPE_BUG_ID => 6; +use constant FIELD_TYPE_BUG_URLS => 7; # The maximum number of days a token will remain valid. use constant MAX_TOKEN_AGE => 3; @@ -421,15 +434,36 @@ use constant MAX_SMALLINT => 32767; # The longest that a saved search name can be. use constant MAX_LEN_QUERY_NAME => 64; +# The longest classification name allowed. +use constant MAX_CLASSIFICATION_SIZE => 64; + +# The longest product name allowed. +use constant MAX_PRODUCT_SIZE => 64; + # The longest milestone name allowed. use constant MAX_MILESTONE_SIZE => 20; # The longest component name allowed. use constant MAX_COMPONENT_SIZE => 64; +# The maximum length for values of -type field. + +=head1 SYNOPSIS + + my $field = new Bugzilla::Field({name => 'bug_status'}); + + my $choice = new Bugzilla::Field::Choice->type($field)->new(1); + + my $choices = Bugzilla::Field::Choice->type($field)->new_from_list([1,2,3]); + my $choices = Bugzilla::Field::Choice->type($field)->get_all(); + my $choices = Bugzilla::Field::Choice->type($field->match({ sortkey => 10 }); + +=head1 DESCRIPTION + +This is an implementation of L, but with a twist. +You can't call any class methods (such as C, C, etc.) +directly on C itself. Instead, you have to +call Ctype($field)> to get the class +you're going to instantiate, and then you call the methods on that. + +We do that because each field has its own database table for its values, so +each value type needs its own class. + +See the L for examples of how this works. + +=head1 METHODS + +=head2 Class Factory + +In object-oriented design, a "class factory" is a method that picks +and returns the right class for you, based on an argument that you pass. + +=over + +=item C + +Takes a single argument, which is either the name of a field from the +C table, or a L object representing a field. + +Returns an appropriate subclass of C that you +can now call class methods on (like C, C, C, etc.) + +B: YOU CANNOT CALL CLASS METHODS ON C. You +must call C to get a class you can call methods on. + +=back + +=head2 Accessors + +These are in addition to the standard L accessors. + +=over + +=item C + +The key that determines the sort order of this item. + +=item C + +The L object that this field value belongs to. + +=item C + +Tells you which values in B fields appear (become visible) when this +value is set in its field. + +Returns a hashref of arrayrefs. The hash keys are the names of fields, +and the values are arrays of C objects, +representing values that this value controls the visibility of, for +that field. + +=back diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index a0c9567a2..130756459 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -180,7 +180,7 @@ sub attachment { return undef unless $self->attach_id; require Bugzilla::Attachment; - $self->{'attachment'} ||= Bugzilla::Attachment->get($self->attach_id); + $self->{'attachment'} ||= new Bugzilla::Attachment($self->attach_id); return $self->{'attachment'}; } @@ -515,7 +515,7 @@ sub snapshot { 'attach_id' => $attach_id }); my @summaries; foreach my $flag (@$flags) { - my $summary = $flag->type->name . $flag->status; + my $summary = $flag->setter->nick . ':' . $flag->type->name . $flag->status; $summary .= "(" . $flag->requestee->login . ")" if $flag->requestee; push(@summaries, $summary); } @@ -625,10 +625,13 @@ sub update_activity { my ($bug_id, $attach_id, $timestamp, $old_summaries, $new_summaries) = @_; my $dbh = Bugzilla->dbh; - $old_summaries = join(", ", @$old_summaries); - $new_summaries = join(", ", @$new_summaries); - my ($removed, $added) = diff_strings($old_summaries, $new_summaries); - if ($removed ne $added) { + my ($removed, $added) = diff_arrays($old_summaries, $new_summaries); + if (scalar @$removed || scalar @$added) { + # Remove flag requester/setter information + foreach (@$removed, @$added) { s/^[^:]+:// } + + $removed = join(", ", @$removed); + $added = join(", ", @$added); my $field_id = get_field_id('flagtypes.name'); $dbh->do('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added) @@ -1106,14 +1109,13 @@ sub notify { foreach my $to (keys %recipients) { # Add threadingmarker to allow flag notification emails to be the # threaded similar to normal bug change emails. - my $user_id = $recipients{$to} ? $recipients{$to}->id : 0; - my $threadingmarker = build_thread_marker($bug->id, $user_id); + my $thread_user_id = $recipients{$to} ? $recipients{$to}->id : 0; my $vars = { 'flag' => $flag, 'to' => $to, 'bug' => $bug, 'attachment' => $attachment, - 'threadingmarker' => $threadingmarker }; + 'threadingmarker' => build_thread_marker($bug->id, $thread_user_id) }; my $lang = $recipients{$to} ? $recipients{$to}->settings->{'lang'}->{'value'} : $default_lang; @@ -1159,6 +1161,44 @@ sub CancelRequests { \@old_summaries, \@new_summaries); } +# This is an internal function used by $bug->flag_types +# and $attachment->flag_types to collect data about available +# flag types and existing flags set on them. You should never +# call this function directly. +sub _flag_types { + my $vars = shift; + + my $target_type = $vars->{target_type}; + my $flags; + + # Retrieve all existing flags for this bug/attachment. + if ($target_type eq 'bug') { + my $bug_id = delete $vars->{bug_id}; + $flags = Bugzilla::Flag->match({target_type => 'bug', bug_id => $bug_id}); + } + elsif ($target_type eq 'attachment') { + my $attach_id = delete $vars->{attach_id}; + $flags = Bugzilla::Flag->match({attach_id => $attach_id}); + } + else { + ThrowCodeError('bad_arg', {argument => 'target_type', + function => 'Bugzilla::Flag::_flag_types'}); + } + + # Get all available flag types for the given product and component. + my $flag_types = Bugzilla::FlagType::match($vars); + + $_->{flags} = [] foreach @$flag_types; + my %flagtypes = map { $_->id => $_ } @$flag_types; + + # Group existing flags per type. + # Call the internal 'type_id' variable instead of the method + # to not create a flagtype object. + push(@{$flagtypes{$_->{type_id}}->{flags}}, $_) foreach @$flags; + + return [sort {$a->sortkey <=> $b->sortkey || $a->name cmp $b->name} values %flagtypes]; +} + =head1 SEE ALSO =over diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm index d9f49c074..2e8a975d2 100644 --- a/Bugzilla/Group.pm +++ b/Bugzilla/Group.pm @@ -103,7 +103,7 @@ sub grant_direct { my ($self, $type) = @_; $self->{grant_direct} ||= {}; return $self->{grant_direct}->{$type} - if defined $self->{members_direct}->{$type}; + if defined $self->{grant_direct}->{$type}; my $dbh = Bugzilla->dbh; my $ids = $dbh->selectcol_arrayref( @@ -198,7 +198,30 @@ sub is_active_bug_group { sub _rederive_regexp { my ($self) = @_; - RederiveRegexp($self->user_regexp, $self->id); + + my $dbh = Bugzilla->dbh; + my $sth = $dbh->prepare("SELECT userid, login_name, group_id + FROM profiles + LEFT JOIN user_group_map + ON user_group_map.user_id = profiles.userid + AND group_id = ? + AND grant_type = ? + AND isbless = 0"); + my $sthadd = $dbh->prepare("INSERT INTO user_group_map + (user_id, group_id, grant_type, isbless) + VALUES (?, ?, ?, 0)"); + my $sthdel = $dbh->prepare("DELETE FROM user_group_map + WHERE user_id = ? AND group_id = ? + AND grant_type = ? and isbless = 0"); + $sth->execute($self->id, GRANT_REGEXP); + my $regexp = $self->user_regexp; + while (my ($uid, $login, $present) = $sth->fetchrow_array) { + if ($regexp ne '' and $login =~ /$regexp/i) { + $sthadd->execute($uid, $self->id, GRANT_REGEXP) unless $present; + } else { + $sthdel->execute($uid, $self->id, GRANT_REGEXP) if $present; + } + } } sub members_non_inherited { @@ -215,6 +238,33 @@ sub members_non_inherited { return $self->{members_non_inherited}; } +sub flatten_group_membership { + my ($self, @groups) = @_; + + my $dbh = Bugzilla->dbh; + my $sth; + my @groupidstocheck = @groups; + my %groupidschecked = (); + $sth = $dbh->prepare("SELECT member_id FROM group_group_map + WHERE grantor_id = ? + AND grant_type = " . GROUP_MEMBERSHIP); + while (my $node = shift @groupidstocheck) { + $sth->execute($node); + my $member; + while (($member) = $sth->fetchrow_array) { + if (!$groupidschecked{$member}) { + $groupidschecked{$member} = 1; + push @groupidstocheck, $member; + push @groups, $member unless grep $_ == $member, @groups; + } + } + } + return \@groups; +} + + + + ################################ ##### Module Subroutines ### ################################ @@ -266,35 +316,6 @@ sub ValidateGroupName { return $ret; } -# This sub is not perldoc'ed because we expect it to go away and -# just become the _rederive_regexp private method. -sub RederiveRegexp { - my ($regexp, $gid) = @_; - my $dbh = Bugzilla->dbh; - my $sth = $dbh->prepare("SELECT userid, login_name, group_id - FROM profiles - LEFT JOIN user_group_map - ON user_group_map.user_id = profiles.userid - AND group_id = ? - AND grant_type = ? - AND isbless = 0"); - my $sthadd = $dbh->prepare("INSERT INTO user_group_map - (user_id, group_id, grant_type, isbless) - VALUES (?, ?, ?, 0)"); - my $sthdel = $dbh->prepare("DELETE FROM user_group_map - WHERE user_id = ? AND group_id = ? - AND grant_type = ? and isbless = 0"); - $sth->execute($gid, GRANT_REGEXP); - while (my ($uid, $login, $present) = $sth->fetchrow_array()) { - if (($regexp =~ /\S+/) && ($login =~ m/$regexp/i)) - { - $sthadd->execute($uid, $gid, GRANT_REGEXP) unless $present; - } else { - $sthdel->execute($uid, $gid, GRANT_REGEXP) if $present; - } - } -} - ############################### ### Validators ### ############################### @@ -400,4 +421,12 @@ Returns an arrayref of L objects representing people who are the group regular expression, or they have been actually added to the group manually. +=item C + +Accepts a list of groups and returns a list of all the groups whose members +inherit membership in any group on the list. So, we can determine if a user +is in any of the groups input to flatten_group_membership by querying the +user_group_map for any user with DIRECT or REGEXP membership IN() the list +of groups returned. + =back diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm index de33cf581..5bc2e7716 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -170,6 +170,74 @@ This describes what hooks exist in Bugzilla currently. They are mostly in alphabetical order, but some related hooks are near each other instead of being alphabetical. +=head2 auth-login_methods + +This allows you to add new login types to Bugzilla. +(See L.) + +Params: + +=over + +=item C + +This is a hash--a mapping from login-type "names" to the actual module on +disk. The keys will be all the values that were passed to +L for the C parameter. The values are the +actual path to the module on disk. (For example, if the key is C, the +value is F.) + +For your extension, the path will start with +F. (See the code in the example extension.) + +If your login type is in the hash as a key, you should set that key to the +right path to your module. That module's C method will be called, +probably with empty parameters. If your login type is I in the hash, +you should not set it. + +You will be prevented from adding new keys to the hash, so make sure your +key is in there before you modify it. (In other words, you can't add in +login methods that weren't passed to L.) + +=back + +=head2 auth-verify_methods + +This works just like L except it's for +login verification methods (See L.) It also +takes a C parameter, just like L. + +=head2 bug-columns + +This allows you to add new fields that will show up in every L +object. Note that you will also need to use the L hook in +conjunction with this hook to make this work. + +Params: + +=over + +=item C - An arrayref containing an array of column names. Push +your column name(s) onto the array. + +=back + +=head2 bug-end_of_create + +This happens at the end of L, after all other changes are +made to the database. This occurs inside a database transaction. + +Params: + +=over + +=item C - The changed bug object, with all fields set to their updated +values. + +=item C - The timestamp used for all updates in this transaction. + +=back + =head2 bug-end_of_update This happens at the end of L, after all other changes are @@ -189,6 +257,23 @@ C<$changes-E{field} = [old, new]> =back +=head2 bug-fields + +Allows the addition of database fields from the bugs table to the standard +list of allowable fields in a L object, so that +you can call the field as a method. + +Note: You should add here the names of any fields you added in L. + +Params: + +=over + +=item C - A arrayref containing an array of column names. Push +your column name(s) onto the array. + +=back + =head2 buglist-columns This happens in buglist.cgi after the standard columns have been defined and @@ -233,6 +318,51 @@ See L. =back +=head2 config-add_panels + +If you want to add new panels to the Parameters administrative interface, +this is where you do it. + +Params: + +=over + +=item C + +A hashref, where the keys are the "name" of the module and the value +is the Perl module containing that config module. For example, if +the name is C, the value would be C. + +For your extension, the Perl module name must start with +C. (See the code in the example +extension.) + +=back + +=head2 config-modify_panels + +This is how you modify already-existing panels in the Parameters +administrative interface. For example, if you wanted to add a new +Auth method (modifying Bugzilla::Config::Auth) this is how you'd +do it. + +Params: + +=over + +=item C + +A hashref, where the keys are lower-case panel "names" (like C, +C, etc.) and the values are hashrefs. The hashref contains a +single key, C. C is an arrayref--the return value from +C for that module. You can modify C and +your changes will be reflected in the interface. + +Adding new keys to C will have no effect. You should use +L if you want to add new panels. + +=back + =head2 enter_bug-entrydefaultvars This happens right before the template is loaded on enter_bug.cgi. @@ -336,6 +466,18 @@ database when run. =back +=head2 mailer-before_send + +Called right before L sends a message to the MTA. + +Params: + +=over + +=item C - The C object that's about to be sent. + +=back + =head2 product-confirm_delete Called before displaying the confirmation message when deleting a product. diff --git a/Bugzilla/Install.pm b/Bugzilla/Install.pm index e93fdd546..bd408764d 100644 --- a/Bugzilla/Install.pm +++ b/Bugzilla/Install.pm @@ -63,6 +63,8 @@ sub SETTINGS { # 2007-07-02 altlist@gmail.com -- Bug 225731 quote_replies => { options => ['quoted_reply', 'simple_reply', 'off'], default => "quoted_reply" }, + # 2008-08-27 LpSolit@gmail.com -- Bug 182238 + timezone => { subclass => 'Timezone', default => 'local' }, # 2008-12-22 vfilippov@custis.ru -- Custis Bug 17481 remind_me_about_worktime => { options => ['on', 'off'], default => 'on' }, remind_me_about_flags => { options => ['on', 'off'], default => 'on' }, @@ -275,7 +277,7 @@ sub create_admin { my $admin_group = new Bugzilla::Group({ name => 'admin' }); my $admin_inheritors = - Bugzilla::User->flatten_group_membership($admin_group->id); + Bugzilla::Group->flatten_group_membership($admin_group->id); my $admin_group_ids = join(',', @$admin_inheritors); my ($admin_count) = $dbh->selectrow_array( diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index 7cb6e5b01..574eb7519 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -91,6 +91,22 @@ sub update_fielddefs_definition { } } + $dbh->bz_add_column('fielddefs', 'visibility_field_id', {TYPE => 'INT3'}); + $dbh->bz_add_column('fielddefs', 'visibility_value_id', {TYPE => 'INT2'}); + $dbh->bz_add_column('fielddefs', 'value_field_id', {TYPE => 'INT3'}); + $dbh->bz_add_index('fielddefs', 'fielddefs_value_field_id_idx', + ['value_field_id']); + + # Bug 344878 + if (!$dbh->bz_column_info('fielddefs', 'buglist')) { + $dbh->bz_add_column('fielddefs', 'buglist', + {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'}); + # Set non-multiselect custom fields as valid buglist fields + # Note that default fields will be handled in Field.pm + $dbh->do('UPDATE fielddefs SET buglist = 1 WHERE custom = 1 AND type != ' . FIELD_TYPE_MULTI_SELECT); + } + + # Remember, this is not the function for adding general table changes. # That is below. Add new changes to the fielddefs table above this # comment. @@ -531,6 +547,22 @@ sub update_table_definitions { $dbh->bz_alter_column('series', 'query', { TYPE => 'MEDIUMTEXT', NOTNULL => 1 }); + # Add FK to multi select field tables + _add_foreign_keys_to_multiselects(); + + # 2008-07-28 tfu@redhat.com - Bug 431669 + $dbh->bz_alter_column('group_control_map', 'product_id', + { TYPE => 'INT2', NOTNULL => 1 }); + + # 2008-09-07 LpSolit@gmail.com - Bug 452893 + _fix_illegal_flag_modification_dates(); + + _add_visiblity_value_to_value_tables(); + + # 2009-03-02 arbingersys@gmail.com - Bug 423613 + $dbh->bz_add_index('profiles', 'profiles_extern_id_idx', + {TYPE => 'UNIQUE', FIELDS => [qw(extern_id)]}); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ @@ -2986,7 +3018,25 @@ sub _initialize_workflow { # Make sure the bug status used by the 'duplicate_or_move_bug_status' # parameter has all the required transitions set. - Bugzilla::Status::add_missing_bug_status_transitions(); + my $dup_status = Bugzilla->params->{'duplicate_or_move_bug_status'}; + my $status_id = $dbh->selectrow_array( + 'SELECT id FROM bug_status WHERE value = ?', undef, $dup_status); + # There's a minor chance that this status isn't in the DB. + $status_id || return; + + my $missing_statuses = $dbh->selectcol_arrayref( + 'SELECT id FROM bug_status + LEFT JOIN status_workflow ON old_status = id + AND new_status = ? + WHERE old_status IS NULL', undef, $status_id); + + my $sth = $dbh->prepare('INSERT INTO status_workflow + (old_status, new_status) VALUES (?, ?)'); + + foreach my $old_status_id (@$missing_statuses) { + next if ($old_status_id == $status_id); + $sth->execute($old_status_id, $status_id); + } } sub _make_lang_setting_dynamic { @@ -3085,6 +3135,25 @@ sub _check_content_length { } } +sub _add_foreign_keys_to_multiselects { + my $dbh = Bugzilla->dbh; + + my $names = $dbh->selectcol_arrayref( + 'SELECT name + FROM fielddefs + WHERE type = ' . FIELD_TYPE_MULTI_SELECT); + + foreach my $name (@$names) { + $dbh->bz_add_fk("bug_$name", "bug_id", {TABLE => 'bugs', + COLUMN => 'bug_id', + DELETE => 'CASCADE',}); + + $dbh->bz_add_fk("bug_$name", "value", {TABLE => $name, + COLUMN => 'value', + DELETE => 'RESTRICT',}); + } +} + sub _populate_bugs_fulltext { my $dbh = Bugzilla->dbh; @@ -3140,6 +3209,29 @@ sub _populate_bugs_fulltext } } +sub _fix_illegal_flag_modification_dates { + my $dbh = Bugzilla->dbh; + + my $rows = $dbh->do('UPDATE flags SET modification_date = creation_date + WHERE modification_date < creation_date'); + # If no rows are affected, $dbh->do returns 0E0 instead of 0. + print "$rows flags had an illegal modification date. Fixed!\n" if ($rows =~ /^\d+$/); +} + +sub _add_visiblity_value_to_value_tables { + my $dbh = Bugzilla->dbh; + my @standard_fields = + qw(bug_status resolution priority bug_severity op_sys rep_platform); + my $custom_fields = $dbh->selectcol_arrayref( + 'SELECT name FROM fielddefs WHERE custom = 1 AND type IN(?,?)', + undef, FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT); + foreach my $field (@standard_fields, @$custom_fields) { + $dbh->bz_add_column($field, 'visibility_value_id', {TYPE => 'INT2'}); + $dbh->bz_add_index($field, "${field}_visibility_value_id_idx", + ['visibility_value_id']); + } +} + 1; __END__ diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm index b36f8f088..d9d1e63c4 100644 --- a/Bugzilla/Install/Filesystem.pm +++ b/Bugzilla/Install/Filesystem.pm @@ -64,6 +64,7 @@ sub FILESYSTEM { my $libdir = bz_locations()->{'libpath'}; my $extlib = bz_locations()->{'ext_libpath'}; my $skinsdir = bz_locations()->{'skinsdir'}; + my $localconfig = bz_locations()->{'localconfig'}; my $ws_group = Bugzilla->localconfig->{'webservergroup'}; @@ -116,8 +117,11 @@ sub FILESYSTEM { 'customfield.pl' => { perms => $owner_executable }, 'email_in.pl' => { perms => $ws_executable }, 'sanitycheck.pl' => { perms => $ws_executable }, + 'jobqueue.pl' => { perms => $owner_executable }, 'install-module.pl' => { perms => $owner_executable }, + "$localconfig.old" => { perms => $owner_readable }, + 'docs/makedocs.pl' => { perms => $owner_executable }, 'docs/style.css' => { perms => $ws_readable }, 'docs/*/rel_notes.txt' => { perms => $ws_readable }, @@ -294,12 +298,8 @@ EOT # It's harmless if it isn't accessible... "$datadir/.htaccess" => { perms => $ws_readable, contents => < - allow from all - EOT @@ -378,6 +378,11 @@ EOT unlink "$datadir/versioncache"; } + if (-e "$datadir/duplicates.rdf") { + print "Removing duplicates.rdf...\n"; + unlink "$datadir/duplicates.rdf"; + unlink "$datadir/duplicates-old.rdf"; + } } # A simple helper for creating "empty" CSS files. diff --git a/Bugzilla/Install/Localconfig.pm b/Bugzilla/Install/Localconfig.pm index 8857b7521..5a5f138ca 100644 --- a/Bugzilla/Install/Localconfig.pm +++ b/Bugzilla/Install/Localconfig.pm @@ -199,13 +199,6 @@ EOT }, ); -use constant OLD_LOCALCONFIG_VARS => qw( - mysqlpath - contenttypes - pages - severities platforms opsys priorities -); - sub read_localconfig { my ($include_deprecated) = @_; my $filename = bz_locations()->{'localconfig'}; @@ -233,9 +226,27 @@ Please fix the error in your 'localconfig' file. Alternately, rename your EOT } - my @vars = map($_->{name}, LOCALCONFIG_VARS); - push(@vars, OLD_LOCALCONFIG_VARS) if $include_deprecated; - foreach my $var (@vars) { + my @read_symbols; + if ($include_deprecated) { + # First we have to get the whole symbol table + my $safe_root = $s->root; + my %safe_package; + { no strict 'refs'; %safe_package = %{$safe_root . "::"}; } + # And now we read the contents of every var in the symbol table. + # However: + # * We only include symbols that start with an alphanumeric + # character. This excludes symbols like "_<./localconfig" + # that show up in some perls. + # * We ignore the INC symbol, which exists in every package. + # * Perl 5.10 imports a lot of random symbols that all + # contain "::", and we want to ignore those. + @read_symbols = grep { /^[A-Za-z0-1]/ and !/^INC$/ and !/::/ } + (keys %safe_package); + } + else { + @read_symbols = map($_->{name}, LOCALCONFIG_VARS); + } + foreach my $var (@read_symbols) { my $glob = $s->varglob($var); # We can't get the type of a variable out of a Safe automatically. # We can only get the glob itself. So we figure out its type this @@ -302,11 +313,6 @@ sub update_localconfig { } } - my @old_vars; - foreach my $name (OLD_LOCALCONFIG_VARS) { - push(@old_vars, $name) if defined $localconfig->{$name}; - } - if (!$localconfig->{'interdiffbin'} && $output) { print <{name} eq $var, LOCALCONFIG_VARS); + } + my $filename = bz_locations->{'localconfig'}; + # Move any custom or old variables into a separate file. if (scalar @old_vars) { + my $filename_old = "$filename.old"; + open(my $old_file, ">>$filename_old") || die "$filename_old: $!"; + local $Data::Dumper::Purity = 1; + foreach my $var (@old_vars) { + print $old_file Data::Dumper->Dump([$localconfig->{$var}], + ["*$var"]) . "\n\n"; + } + close $old_file; my $oldstuff = join(', ', @old_vars); print <{'localconfig'}; - my $fh = new IO::File($filename, '>>') || die "$filename: $!"; - $fh->seek(0, SEEK_END); + # Re-write localconfig + open(my $fh, ">$filename") || die "$filename: $!"; foreach my $var (LOCALCONFIG_VARS) { - if (grep($_ eq $var->{name}, @new_vars)) { print $fh "\n", $var->{desc}, Data::Dumper->Dump([$localconfig->{$var->{name}}], ["*$var->{name}"]); } - } + if (@new_vars) { my $newstuff = join(', ', @new_vars); print < +=item C -Description: Reads the localconfig file and returns all valid - values in a hashref. +=over -Params: C<$include_deprecated> - C if you want the returned - hashref to also include variables listed in - C, if they exist. Generally - this is only for use by C. +=item B -Returns: A hashref of the localconfig variables. If an array - is defined, it will be an arrayref in the returned hash. If a - hash is defined, it will be a hashref in the returned hash. - Only includes variables specified in C - (and C if C<$include_deprecated> is - specified). +Reads the localconfig file and returns all valid values in a hashref. -=item C 1 })> +=item B + +=over + +=item C<$include_deprecated> + +C if you want the returned hashref to include *any* variable +currently defined in localconfig, even if it doesn't exist in +C. Generally this is is only for use +by L. + +=back + +=item B + +A hashref of the localconfig variables. If an array is defined in +localconfig, it will be an arrayref in the returned hash. If a +hash is defined, it will be a hashref in the returned hash. +Only includes variables specified in C, unless +C<$include_deprecated> is true. + +=back + + +=item C Description: Adds any new variables to localconfig that aren't currently defined there. Also optionally prints out a message about vars that *should* be there and aren't. Exits the program if it adds any new vars. -Params: C - C if the function should display informational +Params: C<$output> - C if the function should display informational output and warnings. It will always display errors or any message which would cause program execution to halt. diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm index bf833846b..ade86002b 100644 --- a/Bugzilla/Install/Requirements.pm +++ b/Bugzilla/Install/Requirements.pm @@ -25,6 +25,7 @@ package Bugzilla::Install::Requirements; use strict; +use Bugzilla::Constants; use Bugzilla::Install::Util qw(vers_cmp install_string); use List::Util qw(max); use Safe; @@ -40,7 +41,9 @@ our @EXPORT = qw( install_command ); -use Bugzilla::Constants; +# This is how many *'s are in the top of each "box" message printed +# by checksetup.pl. +use constant TABLE_WIDTH => 71; # The below two constants are subroutines so that they can implement # a hook. Other than that they are actually constants. @@ -64,11 +67,31 @@ sub REQUIRED_MODULES { # Require CGI 3.21 for -httponly support, see bug 368502. version => (vers_cmp($perl_ver, '5.10') > -1) ? '3.33' : '3.21' }, + { + package => 'Digest-SHA', + module => 'Digest::SHA', + version => 0 + }, { package => 'TimeDate', module => 'Date::Format', version => '2.21' }, + # 0.28 fixed some important bugs in DateTime. + { + package => 'DateTime', + module => 'DateTime', + version => '0.28' + }, + # 0.79 is required to work on Windows Vista and Windows Server 2008. + # As correctly detecting the flavor of Windows is not easy, + # we require this version for all Windows installations. + # 0.71 fixes a major bug affecting all platforms. + { + package => 'DateTime-TimeZone', + module => 'DateTime::TimeZone', + version => ON_WINDOWS ? '0.79' : '0.71' + }, { package => 'PathTools', module => 'File::Spec', @@ -79,15 +102,18 @@ sub REQUIRED_MODULES { module => 'DBI', version => '1.41' }, + # 2.22 fixes various problems related to UTF8 strings in hash keys, + # as well as line endings on Windows. { package => 'Template-Toolkit', module => 'Template', - version => '2.15' + version => '2.22' }, { package => 'Email-Send', module => 'Email::Send', - version => ON_WINDOWS ? '2.16' : '2.00' + version => ON_WINDOWS ? '2.16' : '2.00', + blacklist => ['^2\.196$'] }, { package => 'Email-MIME', @@ -105,6 +131,11 @@ sub REQUIRED_MODULES { module => 'Email::MIME::Modifier', version => '1.442' }, + { + package => 'URI', + module => 'URI', + version => 0 + }, ); my $all_modules = _get_extension_requirements( @@ -231,6 +262,20 @@ sub OPTIONAL_MODULES { feature => 'Inbound Email' }, + # Mail Queueing + { + package => 'TheSchwartz', + module => 'TheSchwartz', + version => 0, + feature => 'Mail Queueing', + }, + { + package => 'Daemon-Generic', + module => 'Daemon::Generic', + version => 0, + feature => 'Mail Queueing', + }, + # mod_perl { package => 'mod_perl', @@ -332,143 +377,90 @@ sub _get_activestate_build_id { sub print_module_instructions { my ($check_results, $output) = @_; - # We only print these notes if we have to. - if ((!$output && @{$check_results->{missing}}) - || ($output && $check_results->{any_missing})) - { - - if (ON_WINDOWS) { + # First we print the long explanatory messages. - print "\n* NOTE: You must run any commands listed below as " - . ROOT_USER . ".\n\n"; - - my $perl_ver = sprintf('%vd', $^V); - - # URL when running Perl 5.8.x. - my $url_to_theory58S = 'http://theoryx5.uwinnipeg.ca/ppms'; - my $repo_up_cmd = -'* *'; - # Packages for Perl 5.10 are not compatible with Perl 5.8. - if (vers_cmp($perl_ver, '5.10') > -1) { - $url_to_theory58S = 'http://cpan.uwinnipeg.ca/PPMPackages/10xx/'; - } - # ActivePerl older than revision 819 require an additional command. - if (_get_activestate_build_id() < 819) { - $repo_up_cmd = <{missing}}) { - print <{missing}}) { + print install_string('modules_message_required'); } if (!$check_results->{one_dbd}) { - print <{dbd}); - printf "%10s: \%s\n", $db_modules{$db}->{name}, $command; - print ' ' x 12 . "Minimum version required: " - . $db_modules{$db}->{dbd}->{version} . "\n"; - } - print "\n"; + print install_string('modules_message_db'); } - return unless $output; - - if (my @missing = @{$check_results->{optional}}) { - print <{optional}} and $output) { + print install_string('modules_message_optional'); # Now we have to determine how large the table cols will be. my $longest_name = max(map(length($_->{package}), @missing)); # The first column header is at least 11 characters long. $longest_name = 11 if $longest_name < 11; - # The table is 71 characters long. There are seven mandatory + # The table is TABLE_WIDTH characters long. There are seven mandatory # characters (* and space) in the string. So, we have a total - # of 64 characters to work with. - my $remaining_space = 64 - $longest_name; - print '*' x 71 . "\n"; + # of TABLE_WIDTH - 7 characters to work with. + my $remaining_space = (TABLE_WIDTH - 7) - $longest_name; + print '*' x TABLE_WIDTH . "\n"; printf "* \%${longest_name}s * %-${remaining_space}s *\n", 'MODULE NAME', 'ENABLES FEATURE(S)'; - print '*' x 71 . "\n"; + print '*' x TABLE_WIDTH . "\n"; foreach my $package (@missing) { printf "* \%${longest_name}s * %-${remaining_space}s *\n", $package->{package}, $package->{feature}; } - print '*' x 71 . "\n"; + } - print "COMMANDS TO INSTALL:\n\n"; + # We only print the PPM repository note if we have to. + if ((!$output && @{$check_results->{missing}}) + || ($output && $check_results->{any_missing})) + { + if (ON_WINDOWS) { + my $perl_ver = sprintf('%vd', $^V); + + # URL when running Perl 5.8.x. + my $url_to_theory58S = 'http://theoryx5.uwinnipeg.ca/ppms'; + # Packages for Perl 5.10 are not compatible with Perl 5.8. + if (vers_cmp($perl_ver, '5.10') > -1) { + $url_to_theory58S = 'http://cpan.uwinnipeg.ca/PPMPackages/10xx/'; + } + print install_string('ppm_repo_add', + { theory_url => $url_to_theory58S }); + # ActivePerls older than revision 819 require an additional command. + if (_get_activestate_build_id() < 819) { + print install_string('ppm_repo_up'); + } + } + + # If any output was required, we want to close the "table" + print "*" x TABLE_WIDTH . "\n"; + } + + # And now we print the actual installation commands. + + if (my @missing = @{$check_results->{optional}} and $output) { + print install_string('commands_optional') . "\n\n"; foreach my $module (@missing) { my $command = install_command($module); printf "%15s: $command\n", $module->{package}; } + print "\n"; + } + + if (!$check_results->{one_dbd}) { + print install_string('commands_dbd') . "\n"; + my %db_modules = %{DB_MODULE()}; + foreach my $db (keys %db_modules) { + my $command = install_command($db_modules{$db}->{dbd}); + printf "%10s: \%s\n", $db_modules{$db}->{name}, $command; + } + print "\n"; + } + + if (my @missing = @{$check_results->{missing}}) { + print install_string('commands_required') . "\n"; + foreach my $package (@missing) { + my $command = install_command($package); + print " $command\n"; + } } if ($output && $check_results->{any_missing} && !ON_WINDOWS) { diff --git a/Bugzilla/Install/Util.pm b/Bugzilla/Install/Util.pm index 9cec8c435..250ab9157 100644 --- a/Bugzilla/Install/Util.pm +++ b/Bugzilla/Install/Util.pm @@ -31,6 +31,7 @@ use Bugzilla::Constants; use File::Basename; use POSIX qw(setlocale LC_CTYPE); use Safe; +use Scalar::Util qw(tainted); use base qw(Exporter); our @EXPORT_OK = qw( @@ -109,7 +110,7 @@ sub install_string { foreach my $key (@replace_keys) { my $replacement = $vars->{$key}; die "'$key' in '$string_id' is tainted: '$replacement'" - if is_tainted($replacement); + if tainted($replacement); # We don't want people to start getting clever and inserting # ##variable## into their values. So we check if any other # key is listed in the *replacement* string, before doing @@ -354,10 +355,6 @@ sub trick_taint { return (defined($_[0])); } -sub is_tainted { - return not eval { my $foo = join('',@_), kill 0; 1; }; -} - __END__ =head1 NAME diff --git a/Bugzilla/Job/Mailer.pm b/Bugzilla/Job/Mailer.pm new file mode 100644 index 000000000..24b589d80 --- /dev/null +++ b/Bugzilla/Job/Mailer.pm @@ -0,0 +1,57 @@ +# -*- Mode: perl; indent-tabs-mode: nil -*- +# +# The contents of this file are subject to the Mozilla Public +# License Version 1.1 (the "License"); you may not use this file +# except in compliance with the License. You may obtain a copy of +# the License at http://www.mozilla.org/MPL/ +# +# Software distributed under the License is distributed on an "AS +# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or +# implied. See the License for the specific language governing +# rights and limitations under the License. +# +# The Original Code is the Bugzilla Bug Tracking System. +# +# The Initial Developer of the Original Code is Mozilla Corporation. +# Portions created by the Initial Developer are Copyright (C) 2008 +# Mozilla Corporation. All Rights Reserved. +# +# Contributor(s): +# Mark Smith +# Max Kanat-Alexander + +package Bugzilla::Job::Mailer; +use strict; +use Bugzilla::Mailer; +BEGIN { eval "use base qw(TheSchwartz::Worker)"; } + +# The longest we expect a job to possibly take, in seconds. +use constant grab_for => 300; +# We don't want email to fail permanently very easily. Retry for 30 days. +use constant max_retries => 725; + +# The first few retries happen quickly, but after that we wait an hour for +# each retry. +sub retry_delay { + my $num_retries = shift; + if ($num_retries < 5) { + return (10, 30, 60, 300, 600)[$num_retries]; + } + # One hour + return 60*60; +} + +sub work { + my ($class, $job) = @_; + my $msg = $job->arg->{msg}; + my $success = eval { MessageToMTA($msg, 1); 1; }; + if (!$success) { + $job->failed($@); + undef $@; + } + else { + $job->completed; + } +} + +1; diff --git a/Bugzilla/JobQueue.pm b/Bugzilla/JobQueue.pm new file mode 100644 index 000000000..102f58bc6 --- /dev/null +++ b/Bugzilla/JobQueue.pm @@ -0,0 +1,108 @@ +# -*- Mode: perl; indent-tabs-mode: nil -*- +# +# The contents of this file are subject to the Mozilla Public +# License Version 1.1 (the "License"); you may not use this file +# except in compliance with the License. You may obtain a copy of +# the License at http://www.mozilla.org/MPL/ +# +# Software distributed under the License is distributed on an "AS +# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or +# implied. See the License for the specific language governing +# rights and limitations under the License. +# +# The Original Code is the Bugzilla Bug Tracking System. +# +# The Initial Developer of the Original Code is Mozilla Corporation. +# Portions created by the Initial Developer are Copyright (C) 2008 +# Mozilla Corporation. All Rights Reserved. +# +# Contributor(s): +# Mark Smith +# Max Kanat-Alexander + +package Bugzilla::JobQueue; + +use strict; + +use Bugzilla::Constants; +use Bugzilla::Error; +use Bugzilla::Install::Util qw(install_string); +BEGIN { eval "use base qw(TheSchwartz)"; } + +# This maps job names for Bugzilla::JobQueue to the appropriate modules. +# If you add new types of jobs, you should add a mapping here. +use constant JOB_MAP => { + send_mail => 'Bugzilla::Job::Mailer', +}; + +sub new { + my $class = shift; + + if (!eval { require TheSchwartz; }) { + ThrowCodeError('jobqueue_not_configured'); + } + + my $lc = Bugzilla->localconfig; + my $self = $class->SUPER::new( + databases => [{ + dsn => Bugzilla->dbh->{private_bz_dsn}, + user => $lc->{db_user}, + pass => $lc->{db_pass}, + prefix => 'ts_', + }], + ); + + return $self; +} + +# A way to get access to the underlying databases directly. +sub bz_databases { + my $self = shift; + my @hashes = keys %{ $self->{databases} }; + return map { $self->driver_for($_) } @hashes; +} + +# inserts a job into the queue to be processed and returns immediately +sub insert { + my $self = shift; + my $job = shift; + + my $mapped_job = JOB_MAP->{$job}; + ThrowCodeError('jobqueue_no_job_mapping', { job => $job }) + if !$mapped_job; + unshift(@_, $mapped_job); + + my $retval = $self->SUPER::insert(@_); + # XXX Need to get an error message here if insert fails, but + # I don't see any way to do that in TheSchwartz. + ThrowCodeError('jobqueue_insert_failed', { job => $job, errmsg => $@ }) + if !$retval; + + return $retval; +} + +1; + +__END__ + +=head1 NAME + +Bugzilla::JobQueue - Interface between Bugzilla and TheSchwartz. + +=head1 SYNOPSIS + + use Bugzilla; + + my $obj = Bugzilla->job_queue(); + $obj->insert('send_mail', { msg => $message }); + +=head1 DESCRIPTION + +Certain tasks should be done asyncronously. The job queue system allows +Bugzilla to use some sort of service to schedule jobs to happen asyncronously. + +=head2 Inserting a Job + +See the synopsis above for an easy to follow example on how to insert a +job into the queue. Give it a name and some arguments and the job will +be sent away to be done later. diff --git a/Bugzilla/JobQueue/Runner.pm b/Bugzilla/JobQueue/Runner.pm new file mode 100644 index 000000000..06b8a7a94 --- /dev/null +++ b/Bugzilla/JobQueue/Runner.pm @@ -0,0 +1,99 @@ +# -*- Mode: perl; indent-tabs-mode: nil -*- +# +# The contents of this file are subject to the Mozilla Public +# License Version 1.1 (the "License"); you may not use this file +# except in compliance with the License. You may obtain a copy of +# the License at http://www.mozilla.org/MPL/ +# +# Software distributed under the License is distributed on an "AS +# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or +# implied. See the License for the specific language governing +# rights and limitations under the License. +# +# The Original Code is the Bugzilla Bug Tracking System. +# +# The Initial Developer of the Original Code is Mozilla Corporation. +# Portions created by the Initial Developer are Copyright (C) 2008 +# Mozilla Corporation. All Rights Reserved. +# +# Contributor(s): +# Mark Smith +# Max Kanat-Alexander + +# XXX In order to support Windows, we have to make gd_redirect_output +# use Log4Perl or something instead of calling "logger". We probably +# also need to use Win32::Daemon or something like that to daemonize. + +package Bugzilla::JobQueue::Runner; + +use strict; +use File::Basename; +use Pod::Usage; + +use Bugzilla::Constants; +use Bugzilla::JobQueue; +use Bugzilla::Util qw(get_text); +BEGIN { eval "use base qw(Daemon::Generic)"; } + +# Required because of a bug in Daemon::Generic where it won't use the +# "version" key from DAEMON_CONFIG. +our $VERSION = BUGZILLA_VERSION; + +use constant DAEMON_CONFIG => ( + progname => basename($0), + pidfile => bz_locations()->{datadir} . '/' . basename($0) . '.pid', + version => BUGZILLA_VERSION, +); + +sub gd_preconfig { + return DAEMON_CONFIG; +} + +sub gd_usage { + pod2usage({ -verbose => 0, -exitval => 'NOEXIT' }); + return 0 +} + +sub gd_check { + my $self = shift; + + # Get a count of all the jobs currently in the queue. + my $jq = Bugzilla->job_queue(); + my @dbs = $jq->bz_databases(); + my $count = 0; + foreach my $driver (@dbs) { + $count += $driver->select_one('SELECT COUNT(*) FROM ts_job', []); + } + print get_text('job_queue_depth', { count => $count }) . "\n"; +} + +sub gd_run { + my $self = shift; + + my $jq = Bugzilla->job_queue(); + $jq->set_verbose($self->{debug}); + foreach my $module (values %{ Bugzilla::JobQueue::JOB_MAP() }) { + eval "use $module"; + $jq->can_do($module); + } + $jq->work; +} + +1; + +__END__ + +=head1 NAME + +Bugzilla::JobQueue::Runner - A class representing the daemon that runs the +job queue. + +=head1 SYNOPSIS + + use Bugzilla::JobQueue::Runner; + Bugzilla::JobQueue::Runner->new(); + +=head1 DESCRIPTION + +This is a subclass of L that is used by L +to run the Bugzilla job queue. diff --git a/Bugzilla/Mailer.pm b/Bugzilla/Mailer.pm index 645e65e4e..610523b8a 100644 --- a/Bugzilla/Mailer.pm +++ b/Bugzilla/Mailer.pm @@ -39,6 +39,7 @@ use base qw(Exporter); use Bugzilla::Constants; use Bugzilla::Error; +use Bugzilla::Hook; use Bugzilla::Util; use Date::Format qw(time2str); @@ -52,10 +53,15 @@ use Email::MIME::Modifier; use Email::Send; sub MessageToMTA { - my ($msg) = (@_); + my ($msg, $send_now) = (@_); my $method = Bugzilla->params->{'mail_delivery_method'}; return if $method eq 'None'; + if (Bugzilla->params->{'use_mailer_queue'} and !$send_now) { + Bugzilla->job_queue->insert('send_mail', { msg => $msg }); + return; + } + my $email; if (ref $msg) { $email = $msg; @@ -71,6 +77,17 @@ sub MessageToMTA { $email = new Email::MIME($msg); } + # We add this header to uniquely identify all email that we + # send as coming from this Bugzilla installation. + # + # We don't use correct_urlbase, because we want this URL to + # *always* be the same for this Bugzilla, in every email, + # and some emails we send when we're logged out (in which case + # some emails might get urlbase while the logged-in emails might + # get sslbase). Also, we want this to stay the same even if + # the admin changes the "ssl" parameter. + $email->header_set('X-Bugzilla-URL', Bugzilla->params->{'urlbase'}); + # We add this header to mark the mail as "auto-generated" and # thus to hopefully avoid auto replies. $email->header_set('Auto-Submitted', 'auto-generated'); @@ -157,6 +174,8 @@ sub MessageToMTA { Debug => Bugzilla->params->{'smtp_debug'}; } + Bugzilla::Hook::process('mailer-before_send', { email => $email }); + if ($method eq "Test") { my $filename = bz_locations()->{'datadir'} . '/mailer.testfile'; open TESTFILE, '>>', $filename; diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index d616bb2da..b0490ff9e 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -63,7 +63,10 @@ sub _init { my $name_field = $class->NAME_FIELD; my $id_field = $class->ID_FIELD; - my $id = $param unless (ref $param eq 'HASH'); + my $id = $param; + if (ref $param eq 'HASH') { + $id = $param->{id}; + } my $object; if (defined $id) { @@ -114,12 +117,10 @@ sub check { if (!ref $param) { $param = { name => $param }; } - # Don't allow empty names. - if (exists $param->{name}) { - $param->{name} = trim($param->{name}); - $param->{name} || ThrowUserError('object_name_not_specified', - { class => $class }); - } + # Don't allow empty names or ids. + my $check_param = exists $param->{id} ? $param->{id} : $param->{name}; + $check_param = trim($check_param); + $check_param || ThrowUserError('object_not_specified', { class => $class }); my $obj = $class->new($param) || ThrowUserError('object_does_not_exist', {%$param, class => $class}); return $obj; @@ -155,9 +156,30 @@ sub match { return [$class->get_all] if !$criteria; - my (@terms, @values); + my (@terms, @values, $postamble); foreach my $field (keys %$criteria) { my $value = $criteria->{$field}; + + # allow for LIMIT and OFFSET expressions via the criteria. + next if $field eq 'OFFSET'; + if ( $field eq 'LIMIT' ) { + next unless defined $value; + $postamble = $dbh->sql_limit( $value, $criteria->{OFFSET} ); + next; + } + elsif ( $field eq 'WHERE' ) { + # the WHERE value is a hashref where the keys are + # "column_name operator ?" and values are the placeholder's + # value (either a scalar or an array of values). + foreach my $k (keys %$value) { + push(@terms, $k); + my @this_value = ref($value->{$k}) ? @{ $value->{$k} } + : ($value->{$k}); + push(@values, @this_value); + } + next; + } + if (ref $value eq 'ARRAY') { # IN () is invalid SQL, and if we have an empty list # to match against, we're just returning an empty @@ -180,12 +202,12 @@ sub match { } } - my $where = join(' AND ', @terms); - return $class->_do_list_select($where, \@values); + my $where = join(' AND ', @terms) if scalar @terms; + return $class->_do_list_select($where, \@values, $postamble); } sub _do_list_select { - my ($class, $where, $values) = @_; + my ($class, $where, $values, $postamble) = @_; my $table = $class->DB_TABLE; my $cols = join(',', $class->DB_COLUMNS); my $order = $class->LIST_ORDER; @@ -196,6 +218,8 @@ sub _do_list_select { } $sql .= " ORDER BY $order"; + $sql .= " $postamble" if $postamble; + my $dbh = Bugzilla->dbh; my $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @$values); bless ($_, $class) foreach @$objects; @@ -237,6 +261,14 @@ sub set { $self->{$field} = $value; } +sub set_all { + my ($self, $params) = @_; + foreach my $key (keys %$params) { + my $method = "set_$key"; + $self->$method($params->{$key}); + } +} + sub update { my $self = shift; @@ -280,9 +312,22 @@ sub update { $dbh->bz_commit_transaction(); + if (wantarray) { + return (\%changes, $old_self); + } + return \%changes; } +sub remove_from_db { + my $self = shift; + my $table = $self->DB_TABLE; + my $id_field = $self->ID_FIELD; + Bugzilla->dbh->do("DELETE FROM $table WHERE $id_field = ?", + undef, $self->id); + undef $self; +} + ############################### #### Subroutines ###### ############################### @@ -511,7 +556,9 @@ as the value in the L column). If you pass in a hashref, you can pass a C key. The value of the C key is the case-insensitive name of the object -(from L) in the DB. +(from L) in the DB. You can also pass in an C key +which will be interpreted as the id of the object you want (overriding the +C key). B @@ -601,6 +648,26 @@ There are two special values, the constants C and C, which means "give me objects where this field is NULL or NOT NULL, respectively." +In addition to the column keys, there are a few special keys that +can be used to rig the underlying database queries. These are +C, C, and C. + +The value for the C key is expected to be an integer defining +the number of objects to return, while the value for C defines +the position, relative to the number of objects the query would normally +return, at which to begin the result set. If C is defined without +a corresponding C it is silently ignored. + +The C key provides a mechanism for adding arbitrary WHERE +clauses to the underlying query. Its value is expected to a hash +reference whose keys are the columns, operators and placeholders, and the +values are the placeholders' bind value. For example: + + WHERE => { 'some_column >= ?' => $some_value } + +would constrain the query to only those objects in the table whose +'some_column' column has a value greater than or equal to $some_value. + If you don't specify any criteria, calling this function is the same as doing C<[$class-Eget_all]>. @@ -698,6 +765,8 @@ updated, and they will only be updated if their values have changed. =item B +B + A hashref showing what changed during the update. The keys are the column names from L. If a field was not changed, it will not be in the hash at all. If the field was changed, the key will point to an arrayref. @@ -706,14 +775,27 @@ will be the new value. If there were no changes, we return a reference to an empty hash. -=back +B + +Returns a list, where the first item is the above hashref. The second item +is the object as it was in the database before update() was called. (This +is mostly useful to subclasses of C that are implementing +C.) =back -=head2 Subclass Helpers +=item C -These functions are intended only for use by subclasses. If -you call them from anywhere else, they will throw a C. +Removes this object from the database. Will throw an error if you can't +remove it for some reason. The object will then be destroyed, as it is +not safe to use the object after it has been removed from the database. + +=back + +=head2 Mutators + +These are used for updating the values in objects, before calling +C. =over @@ -734,9 +816,11 @@ C will call it with C<($value, $field)> as arguments, after running the validator for this particular field. C<_set_global_validator> does not return anything. - See L for more information. +B: This function is intended only for use by subclasses. If +you call it from anywhere else, it will throw a C. + =item B =over @@ -752,6 +836,27 @@ be the same as the name of the field in L, if it exists there. =back + +=item C + +=over + +=item B + +This is a convenience function which is simpler than calling many different +C functions in a row. You pass a hashref of parameters and it calls +C for every item in the hashref. + +=item B + +Takes a hashref of the fields that need to be set, pointing to the value +that should be passed to the C function that is called. + +=item B (nothing) + +=back + + =back =head2 Simple Validators diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm index 95a0e3840..de2d96708 100644 --- a/Bugzilla/Product.pm +++ b/Bugzilla/Product.pm @@ -13,22 +13,27 @@ # The Original Code is the Bugzilla Bug Tracking System. # # Contributor(s): Tiago R. Mello +# Frédéric Buclin use strict; package Bugzilla::Product; -use Bugzilla::Version; -use Bugzilla::Milestone; - use Bugzilla::Constants; use Bugzilla::Util; -use Bugzilla::Group; use Bugzilla::Error; - +use Bugzilla::Group; +use Bugzilla::Version; +use Bugzilla::Milestone; +use Bugzilla::Field; +use Bugzilla::Status; use Bugzilla::Install::Requirements; +use Bugzilla::Mailer; +use Bugzilla::Series; -use base qw(Bugzilla::Object); +# Currently, we only implement enough of the Bugzilla::Field::Choice +# interface to control the visibility of other fields. +use base qw(Bugzilla::Field::Choice); use constant DEFAULT_CLASSIFICATION_ID => 1; @@ -37,24 +42,87 @@ use constant DEFAULT_CLASSIFICATION_ID => 1; ############################### use constant DB_TABLE => 'products'; +# Reset these back to the Bugzilla::Object defaults, instead of the +# Bugzilla::Field::Choice defaults. +use constant NAME_FIELD => 'name'; +use constant LIST_ORDER => 'name'; use constant DB_COLUMNS => qw( - products.id - products.name - products.classification_id - products.description - products.milestoneurl - products.disallownew - products.votesperuser - products.maxvotesperbug - products.votestoconfirm - products.defaultmilestone + id + name + classification_id + description + milestoneurl + disallownew + votesperuser + maxvotesperbug + votestoconfirm + defaultmilestone ); +use constant REQUIRED_CREATE_FIELDS => qw( + name + description + version +); + +use constant UPDATE_COLUMNS => qw( + name + description + defaultmilestone + milestoneurl + disallownew + votesperuser + maxvotesperbug + votestoconfirm +); + +use constant VALIDATORS => { + classification => \&_check_classification, + name => \&_check_name, + description => \&_check_description, + version => \&_check_version, + defaultmilestone => \&_check_default_milestone, + milestoneurl => \&_check_milestone_url, + disallownew => \&Bugzilla::Object::check_boolean, + votesperuser => \&_check_votes_per_user, + maxvotesperbug => \&_check_votes_per_bug, + votestoconfirm => \&_check_votes_to_confirm, + create_series => \&Bugzilla::Object::check_boolean +}; + ############################### #### Constructors ##### ############################### +sub create { + my $class = shift; + my $dbh = Bugzilla->dbh; + + $dbh->bz_start_transaction(); + + $class->check_required_create_fields(@_); + + my $params = $class->run_create_validators(@_); + # Some fields do not exist in the DB as is. + $params->{classification_id} = delete $params->{classification}; + my $version = delete $params->{version}; + my $create_series = delete $params->{create_series}; + + my $product = $class->insert_create_data($params); + + # Add the new version and milestone into the DB as valid values. + Bugzilla::Version::create($version, $product); + Bugzilla::Milestone->create({name => $params->{defaultmilestone}, product => $product}); + + # Create groups and series for the new product, if requested. + $product->_create_bug_group() if Bugzilla->params->{'makeproductgroups'}; + $product->_create_series() if $create_series; + + $dbh->bz_commit_transaction(); + return $product; +} + # This is considerably faster than calling new_from_list three times # for each product in the list, particularly with hundreds or thousands # of products. @@ -78,10 +146,523 @@ sub preload { } } +sub update { + my $self = shift; + my $dbh = Bugzilla->dbh; + + # Don't update the DB if something goes wrong below -> transaction. + $dbh->bz_start_transaction(); + my ($changes, $old_self) = $self->SUPER::update(@_); + + # We also have to fix votes. + my @msgs; # Will store emails to send to voters. + if ($changes->{maxvotesperbug} || $changes->{votesperuser} || $changes->{votestoconfirm}) { + # We cannot |use| these modules, due to dependency loops. + require Bugzilla::Bug; + import Bugzilla::Bug qw(RemoveVotes CheckIfVotedConfirmed); + require Bugzilla::User; + import Bugzilla::User qw(user_id_to_login); + + # 1. too many votes for a single user on a single bug. + my @toomanyvotes_list = (); + if ($self->max_votes_per_bug < $self->votes_per_user) { + my $votes = $dbh->selectall_arrayref( + 'SELECT votes.who, votes.bug_id + FROM votes + INNER JOIN bugs + ON bugs.bug_id = votes.bug_id + WHERE bugs.product_id = ? + AND votes.vote_count > ?', + undef, ($self->id, $self->max_votes_per_bug)); + + foreach my $vote (@$votes) { + my ($who, $id) = (@$vote); + # If some votes are removed, RemoveVotes() returns a list + # of messages to send to voters. + push(@msgs, RemoveVotes($id, $who, 'votes_too_many_per_bug')); + my $name = user_id_to_login($who); + + push(@toomanyvotes_list, {id => $id, name => $name}); + } + } + $changes->{'too_many_votes'} = \@toomanyvotes_list; + + # 2. too many total votes for a single user. + # This part doesn't work in the general case because RemoveVotes + # doesn't enforce votesperuser (except per-bug when it's less + # than maxvotesperbug). See Bugzilla::Bug::RemoveVotes(). + + my $votes = $dbh->selectall_arrayref( + 'SELECT votes.who, votes.vote_count + FROM votes + INNER JOIN bugs + ON bugs.bug_id = votes.bug_id + WHERE bugs.product_id = ?', + undef, $self->id); + + my %counts; + foreach my $vote (@$votes) { + my ($who, $count) = @$vote; + if (!defined $counts{$who}) { + $counts{$who} = $count; + } else { + $counts{$who} += $count; + } + } + my @toomanytotalvotes_list = (); + foreach my $who (keys(%counts)) { + if ($counts{$who} > $self->votes_per_user) { + my $bug_ids = $dbh->selectcol_arrayref( + 'SELECT votes.bug_id + FROM votes + INNER JOIN bugs + ON bugs.bug_id = votes.bug_id + WHERE bugs.product_id = ? + AND votes.who = ?', + undef, ($self->id, $who)); + + foreach my $bug_id (@$bug_ids) { + # RemoveVotes() returns a list of messages to send + # in case some voters had too many votes. + push(@msgs, RemoveVotes($bug_id, $who, 'votes_too_many_per_user')); + my $name = user_id_to_login($who); + + push(@toomanytotalvotes_list, {id => $bug_id, name => $name}); + } + } + } + $changes->{'too_many_total_votes'} = \@toomanytotalvotes_list; + + # 3. enough votes to confirm + my $bug_list = + $dbh->selectcol_arrayref('SELECT bug_id FROM bugs WHERE product_id = ? + AND bug_status = ? AND votes >= ?', + undef, ($self->id, 'UNCONFIRMED', $self->votes_to_confirm)); + + my @updated_bugs = (); + foreach my $bug_id (@$bug_list) { + my $confirmed = CheckIfVotedConfirmed($bug_id); + push (@updated_bugs, $bug_id) if $confirmed; + } + $changes->{'confirmed_bugs'} = \@updated_bugs; + } + + # Also update group settings. + if ($self->{check_group_controls}) { + require Bugzilla::Bug; + import Bugzilla::Bug qw(LogActivityEntry); + + my $old_settings = $old_self->group_controls; + my $new_settings = $self->group_controls; + my $timestamp = $dbh->selectrow_array('SELECT NOW()'); + + foreach my $gid (keys %$new_settings) { + my $old_setting = $old_settings->{$gid} || {}; + my $new_setting = $new_settings->{$gid}; + # If all new settings are 0 for a given group, we delete the entry + # from group_control_map, so we have to track it here. + my $all_zero = 1; + my @fields; + my @values; + + foreach my $field ('entry', 'membercontrol', 'othercontrol', 'canedit', + 'editcomponents', 'editbugs', 'canconfirm') + { + my $old_value = $old_setting->{$field}; + my $new_value = $new_setting->{$field}; + $all_zero = 0 if $new_value; + next if (defined $old_value && $old_value == $new_value); + push(@fields, $field); + # The value has already been validated. + detaint_natural($new_value); + push(@values, $new_value); + } + # Is there anything to update? + next unless scalar @fields; + + if ($all_zero) { + $dbh->do('DELETE FROM group_control_map + WHERE product_id = ? AND group_id = ?', + undef, $self->id, $gid); + } + else { + if (exists $old_setting->{group}) { + # There is already an entry in the DB. + my $set_fields = join(', ', map {"$_ = ?"} @fields); + $dbh->do("UPDATE group_control_map SET $set_fields + WHERE product_id = ? AND group_id = ?", + undef, (@values, $self->id, $gid)); + } + else { + # No entry yet. + my $fields = join(', ', @fields); + # +2 because of the product and group IDs. + my $qmarks = join(',', ('?') x (scalar @fields + 2)); + $dbh->do("INSERT INTO group_control_map (product_id, group_id, $fields) + VALUES ($qmarks)", undef, ($self->id, $gid, @values)); + } + } + + # If the group is mandatory, restrict all bugs to it. + if ($new_setting->{membercontrol} == CONTROLMAPMANDATORY) { + my $bug_ids = + $dbh->selectcol_arrayref('SELECT bugs.bug_id + FROM bugs + LEFT JOIN bug_group_map + ON bug_group_map.bug_id = bugs.bug_id + AND group_id = ? + WHERE product_id = ? + AND bug_group_map.bug_id IS NULL', + undef, $gid, $self->id); + + if (scalar @$bug_ids) { + my $sth = $dbh->prepare('INSERT INTO bug_group_map (bug_id, group_id) + VALUES (?, ?)'); + + foreach my $bug_id (@$bug_ids) { + $sth->execute($bug_id, $gid); + # Add this change to the bug history. + LogActivityEntry($bug_id, 'bug_group', '', + $new_setting->{group}->name, + Bugzilla->user->id, $timestamp); + } + push(@{$changes->{'group_controls'}->{'now_mandatory'}}, + {name => $new_setting->{group}->name, + bug_count => scalar @$bug_ids}); + } + } + # If the group can no longer be used to restrict bugs, remove them. + elsif ($new_setting->{membercontrol} == CONTROLMAPNA) { + my $bug_ids = + $dbh->selectcol_arrayref('SELECT bugs.bug_id + FROM bugs + INNER JOIN bug_group_map + ON bug_group_map.bug_id = bugs.bug_id + WHERE product_id = ? AND group_id = ?', + undef, $self->id, $gid); + + if (scalar @$bug_ids) { + $dbh->do('DELETE FROM bug_group_map WHERE group_id = ? AND ' . + $dbh->sql_in('bug_id', $bug_ids), undef, $gid); + + # Add this change to the bug history. + foreach my $bug_id (@$bug_ids) { + LogActivityEntry($bug_id, 'bug_group', + $old_setting->{group}->name, '', + Bugzilla->user->id, $timestamp); + } + push(@{$changes->{'group_controls'}->{'now_na'}}, + {name => $old_setting->{group}->name, + bug_count => scalar @$bug_ids}); + } + } + } + } + $dbh->bz_commit_transaction(); + # Changes have been committed. + delete $self->{check_group_controls}; + + # Now that changes have been committed, we can send emails to voters. + foreach my $msg (@msgs) { + MessageToMTA($msg); + } + + return $changes; +} + +sub remove_from_db { + my $self = shift; + my $user = Bugzilla->user; + my $dbh = Bugzilla->dbh; + + $dbh->bz_start_transaction(); + + $self->_check_if_controller(); + + if ($self->bug_count) { + if (Bugzilla->params->{'allowbugdeletion'}) { + require Bugzilla::Bug; + foreach my $bug_id (@{$self->bug_ids}) { + # Note that we allow the user to delete bugs he can't see, + # which is okay, because he's deleting the whole Product. + my $bug = new Bugzilla::Bug($bug_id); + $bug->remove_from_db(); + } + } + else { + ThrowUserError('product_has_bugs', { nb => $self->bug_count }); + } + } + + # XXX - This line can go away as soon as bug 427455 is fixed. + $dbh->do("DELETE FROM group_control_map WHERE product_id = ?", undef, $self->id); + $dbh->do("DELETE FROM products WHERE id = ?", undef, $self->id); + + $dbh->bz_commit_transaction(); + + # We have to delete these internal variables, else we get + # the old lists of products and classifications again. + delete $user->{selectable_products}; + delete $user->{selectable_classifications}; + +} + +############################### +#### Validators #### +############################### + +sub _check_classification { + my ($invocant, $classification_name) = @_; + + my $classification_id = 1; + if (Bugzilla->params->{'useclassification'}) { + my $classification = Bugzilla::Classification->check($classification_name); + $classification_id = $classification->id; + } + return $classification_id; +} + +sub _check_name { + my ($invocant, $name) = @_; + + $name = trim($name); + $name || ThrowUserError('product_blank_name'); + + if (length($name) > MAX_PRODUCT_SIZE) { + ThrowUserError('product_name_too_long', {'name' => $name}); + } + + my $product = new Bugzilla::Product({name => $name}); + if ($product && (!ref $invocant || $product->id != $invocant->id)) { + # Check for exact case sensitive match: + if ($product->name eq $name) { + ThrowUserError('product_name_already_in_use', {'product' => $product->name}); + } + else { + ThrowUserError('product_name_diff_in_case', {'product' => $name, + 'existing_product' => $product->name}); + } + } + return $name; +} + +sub _check_description { + my ($invocant, $description) = @_; + + $description = trim($description); + $description || ThrowUserError('product_must_have_description'); + return $description; +} + +sub _check_version { + my ($invocant, $version) = @_; + + $version = trim($version); + $version || ThrowUserError('product_must_have_version'); + # We will check the version length when Bugzilla::Version->create will do it. + return $version; +} + +sub _check_default_milestone { + my ($invocant, $milestone) = @_; + + # Do nothing if target milestones are not in use. + unless (Bugzilla->params->{'usetargetmilestone'}) { + return (ref $invocant) ? $invocant->default_milestone : '---'; + } + + $milestone = trim($milestone); + + if (ref $invocant) { + # The default milestone must be one of the existing milestones. + my $mil_obj = new Bugzilla::Milestone({name => $milestone, product => $invocant}); + + $mil_obj || ThrowUserError('product_must_define_defaultmilestone', + {product => $invocant->name, + milestone => $milestone}); + } + else { + $milestone ||= '---'; + } + return $milestone; +} + +sub _check_milestone_url { + my ($invocant, $url) = @_; + + # Do nothing if target milestones are not in use. + unless (Bugzilla->params->{'usetargetmilestone'}) { + return (ref $invocant) ? $invocant->milestone_url : ''; + } + + $url = trim($url || ''); + return $url; +} + +sub _check_votes_per_user { + return _check_votes(@_, 0); +} + +sub _check_votes_per_bug { + return _check_votes(@_, 10000); +} + +sub _check_votes_to_confirm { + return _check_votes(@_, 0); +} + +# This subroutine is only used internally by other _check_votes_* validators. +sub _check_votes { + my ($invocant, $votes, $field, $default) = @_; + + detaint_natural($votes); + # On product creation, if the number of votes is not a valid integer, + # we silently fall back to the given default value. + # If the product already exists and the change is illegal, we complain. + if (!defined $votes) { + if (ref $invocant) { + ThrowUserError('product_illegal_votes', {field => $field, votes => $_[1]}); + } + else { + $votes = $default; + } + } + return $votes; +} + +##################################### +# Implement Bugzilla::Field::Choice # +##################################### + +sub field { + my $invocant = shift; + my $class = ref $invocant || $invocant; + my $cache = Bugzilla->request_cache; + $cache->{"field_$class"} ||= new Bugzilla::Field({ name => 'product' }); + return $cache->{"field_$class"}; +} + +use constant is_default => 0; + ############################### #### Methods #### ############################### +sub _create_bug_group { + my $self = shift; + my $dbh = Bugzilla->dbh; + + my $group_name = $self->name; + while (new Bugzilla::Group({name => $group_name})) { + $group_name .= '_'; + } + my $group_description = get_text('bug_group_description', {product => $self}); + + my $group = Bugzilla::Group->create({name => $group_name, + description => $group_description, + isbuggroup => 1}); + + # Associate the new group and new product. + $dbh->do('INSERT INTO group_control_map + (group_id, product_id, entry, membercontrol, othercontrol, canedit) + VALUES (?, ?, ?, ?, ?, ?)', + undef, ($group->id, $self->id, Bugzilla->params->{'useentrygroupdefault'}, + CONTROLMAPDEFAULT, CONTROLMAPNA, 0)); +} + +sub _create_series { + my $self = shift; + + my @series; + # We do every status, every resolution, and an "opened" one as well. + foreach my $bug_status (@{get_legal_field_values('bug_status')}) { + push(@series, [$bug_status, "bug_status=" . url_quote($bug_status)]); + } + + foreach my $resolution (@{get_legal_field_values('resolution')}) { + next if !$resolution; + push(@series, [$resolution, "resolution=" . url_quote($resolution)]); + } + + my @openedstatuses = BUG_STATE_OPEN; + my $query = join("&", map { "bug_status=" . url_quote($_) } @openedstatuses); + push(@series, [get_text('series_all_open'), $query]); + + foreach my $sdata (@series) { + my $series = new Bugzilla::Series(undef, $self->name, + get_text('series_subcategory'), + $sdata->[0], Bugzilla->user->id, 1, + $sdata->[1] . "&product=" . url_quote($self->name), 1); + $series->writeToDatabase(); + } +} + +sub set_name { $_[0]->set('name', $_[1]); } +sub set_description { $_[0]->set('description', $_[1]); } +sub set_default_milestone { $_[0]->set('defaultmilestone', $_[1]); } +sub set_milestone_url { $_[0]->set('milestoneurl', $_[1]); } +sub set_disallow_new { $_[0]->set('disallownew', $_[1]); } +sub set_votes_per_user { $_[0]->set('votesperuser', $_[1]); } +sub set_votes_per_bug { $_[0]->set('maxvotesperbug', $_[1]); } +sub set_votes_to_confirm { $_[0]->set('votestoconfirm', $_[1]); } + +sub set_group_controls { + my ($self, $group, $settings) = @_; + + $group->is_active_bug_group + || ThrowUserError('product_illegal_group', {group => $group}); + + scalar(keys %$settings) + || ThrowCodeError('product_empty_group_controls', {group => $group}); + + # We store current settings for this group. + my $gs = $self->group_controls->{$group->id}; + # If there is no entry for this group yet, create a default hash. + unless (defined $gs) { + $gs = { entry => 0, + membercontrol => CONTROLMAPNA, + othercontrol => CONTROLMAPNA, + canedit => 0, + editcomponents => 0, + editbugs => 0, + canconfirm => 0, + group => $group }; + } + + # Both settings must be defined, or none of them can be updated. + if (defined $settings->{membercontrol} && defined $settings->{othercontrol}) { + # Legality of control combination is a function of + # membercontrol\othercontrol + # NA SH DE MA + # NA + - - - + # SH + + + + + # DE + - + + + # MA - - - + + foreach my $field ('membercontrol', 'othercontrol') { + my ($is_legal) = grep { $settings->{$field} == $_ } + (CONTROLMAPNA, CONTROLMAPSHOWN, CONTROLMAPDEFAULT, CONTROLMAPMANDATORY); + defined $is_legal || ThrowCodeError('product_illegal_group_control', + { field => $field, value => $settings->{$field} }); + } + unless ($settings->{membercontrol} == $settings->{othercontrol} + || $settings->{membercontrol} == CONTROLMAPSHOWN + || ($settings->{membercontrol} == CONTROLMAPDEFAULT + && $settings->{othercontrol} != CONTROLMAPSHOWN)) + { + ThrowUserError('illegal_group_control_combination', {groupname => $group->name}); + } + $gs->{membercontrol} = $settings->{membercontrol}; + $gs->{othercontrol} = $settings->{othercontrol}; + } + + foreach my $field ('entry', 'canedit', 'editcomponents', 'editbugs', 'canconfirm') { + next unless defined $settings->{$field}; + $gs->{$field} = $settings->{$field} ? 1 : 0; + } + $self->{group_controls}->{$group->id} = $gs; + $self->{check_group_controls} = 1; +} + sub components { my $self = shift; my $dbh = Bugzilla->dbh; @@ -99,25 +680,33 @@ sub components { } sub group_controls { - my $self = shift; + my ($self, $full_data) = @_; my $dbh = Bugzilla->dbh; - if (!defined $self->{group_controls}) { - my $query = qq{SELECT - groups.id, - group_control_map.entry, - group_control_map.membercontrol, - group_control_map.othercontrol, - group_control_map.canedit, - group_control_map.editcomponents, - group_control_map.editbugs, - group_control_map.canconfirm + # By default, we don't return groups which are not listed in + # group_control_map. If $full_data is true, then we also + # return groups whose settings could be set for the product. + my $where_or_and = 'WHERE'; + my $and_or_where = 'AND'; + if ($full_data) { + $where_or_and = 'AND'; + $and_or_where = 'WHERE'; + } + + # If $full_data is true, we collect all the data in all cases, + # even if the cache is already populated. + # $full_data is never used except in the very special case where + # all configurable bug groups are displayed to administrators, + # so we don't care about collecting all the data again in this case. + if (!defined $self->{group_controls} || $full_data) { + # Include name to the list, to allow us sorting data more easily. + my $query = qq{SELECT id, name, entry, membercontrol, othercontrol, + canedit, editcomponents, editbugs, canconfirm FROM groups LEFT JOIN group_control_map - ON groups.id = group_control_map.group_id - WHERE group_control_map.product_id = ? - AND groups.isbuggroup != 0 - ORDER BY groups.name}; + ON id = group_id + $where_or_and product_id = ? + $and_or_where isbuggroup = 1}; $self->{group_controls} = $dbh->selectall_hashref($query, 'id', undef, $self->id); @@ -126,6 +715,21 @@ sub group_controls { my $groups = Bugzilla::Group->new_from_list(\@gids); $self->{group_controls}->{$_->id}->{group} = $_ foreach @$groups; } + + # We never cache bug counts, for the same reason as above. + if ($full_data) { + my $counts = + $dbh->selectall_arrayref('SELECT group_id, COUNT(bugs.bug_id) AS bug_count + FROM bug_group_map + INNER JOIN bugs + ON bugs.bug_id = bug_group_map.bug_id + WHERE bugs.product_id = ? ' . + $dbh->sql_group_by('group_id'), + {'Slice' => {}}, $self->id); + foreach my $data (@$counts) { + $self->{group_controls}->{$data->{group_id}}->{bug_count} = $data->{bug_count}; + } + } return $self->{group_controls}; } @@ -341,7 +945,10 @@ below. Description: Returns a hash (group id as key) with all product group controls. - Params: none. + Params: $full_data (optional, false by default) - when true, + the number of bugs per group applicable to the product + is also returned. Moreover, bug groups which have no + special settings for the product are also returned. Returns: A hash with group id as key and hash containing a Bugzilla::Group object and the properties of group diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index ad88ce9e7..549ec5cef 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -33,7 +33,13 @@ use strict; package Bugzilla::Search; use base qw(Exporter); -@Bugzilla::Search::EXPORT = qw(IsValidQueryType); +@Bugzilla::Search::EXPORT = qw( + EMPTY_COLUMN + + IsValidQueryType + split_order_term + translate_old_column +); use Bugzilla::Error; use Bugzilla::Util; @@ -47,33 +53,126 @@ use Bugzilla::Keyword; use Date::Format; use Date::Parse; +# A SELECTed expression that we use as a placeholder if somebody selects +# for the X, Y, or Z axis in report.cgi. +use constant EMPTY_COLUMN => '-1'; + # Some fields are not sorted on themselves, but on other fields. # We need to have a list of these fields and what they map to. # Each field points to an array that contains the fields mapped # to, in order. use constant SPECIAL_ORDER => { - 'bugs.target_milestone' => [ 'ms_order.sortkey','ms_order.value' ], - 'bugs.bug_status' => [ 'bug_status.sortkey','bug_status.value' ], - 'bugs.rep_platform' => [ 'rep_platform.sortkey','rep_platform.value' ], - 'bugs.priority' => [ 'priority.sortkey','priority.value' ], - 'bugs.op_sys' => [ 'op_sys.sortkey','op_sys.value' ], - 'bugs.resolution' => [ 'resolution.sortkey', 'resolution.value' ], - 'bugs.bug_severity' => [ 'bug_severity.sortkey','bug_severity.value' ] + 'target_milestone' => [ 'ms_order.sortkey','ms_order.value' ], }; # When we add certain fields to the ORDER BY, we need to then add a # table join to the FROM statement. This hash maps input fields to # the join statements that need to be added. use constant SPECIAL_ORDER_JOIN => { - 'bugs.target_milestone' => 'LEFT JOIN milestones AS ms_order ON ms_order.value = bugs.target_milestone AND ms_order.product_id = bugs.product_id', - 'bugs.bug_status' => 'LEFT JOIN bug_status ON bug_status.value = bugs.bug_status', - 'bugs.rep_platform' => 'LEFT JOIN rep_platform ON rep_platform.value = bugs.rep_platform', - 'bugs.priority' => 'LEFT JOIN priority ON priority.value = bugs.priority', - 'bugs.op_sys' => 'LEFT JOIN op_sys ON op_sys.value = bugs.op_sys', - 'bugs.resolution' => 'LEFT JOIN resolution ON resolution.value = bugs.resolution', - 'bugs.bug_severity' => 'LEFT JOIN bug_severity ON bug_severity.value = bugs.bug_severity' + 'target_milestone' => 'LEFT JOIN milestones AS ms_order ON ms_order.value = bugs.target_milestone AND ms_order.product_id = bugs.product_id', }; +# This constant defines the columns that can be selected in a query +# and/or displayed in a bug list. Column records include the following +# fields: +# +# 1. id: a unique identifier by which the column is referred in code; +# +# 2. name: The name of the column in the database (may also be an expression +# that returns the value of the column); +# +# 3. title: The title of the column as displayed to users. +# +# Note: There are a few hacks in the code that deviate from these definitions. +# In particular, when the list is sorted by the "votes" field the word +# "DESC" is added to the end of the field to sort in descending order, +# and the redundant short_desc column is removed when the client +# requests "all" columns. +# +# This is really a constant--that is, once it's been called once, the value +# will always be the same unless somebody adds a new custom field. But +# we have to do a lot of work inside the subroutine to get the data, +# and we don't want it to happen at compile time, so we have it as a +# subroutine. +sub COLUMNS { + my $dbh = Bugzilla->dbh; + my $cache = Bugzilla->request_cache; + return $cache->{search_columns} if defined $cache->{search_columns}; + + # These are columns that don't exist in fielddefs, but are valid buglist + # columns. (Also see near the bottom of this function for the definition + # of short_short_desc.) + my %columns = ( + relevance => { title => 'Relevance' }, + assigned_to_realname => { title => 'Assignee' }, + reporter_realname => { title => 'Reporter' }, + qa_contact_realname => { title => 'QA Contact' }, + ); + + # Next we define columns that have special SQL instead of just something + # like "bugs.bug_id". + my $actual_time = '(SUM(ldtime.work_time)' + . ' * COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id))'; + my %special_sql = ( + deadline => $dbh->sql_date_format('bugs.deadline', '%Y-%m-%d'), + actual_time => $actual_time, + + percentage_complete => + "(CASE WHEN $actual_time + bugs.remaining_time = 0.0" + . " THEN 0.0" + . " ELSE 100" + . " * ($actual_time / ($actual_time + bugs.remaining_time))" + . " END)", + ); + + # Backward-compatibility for old field names. Goes new_name => old_name. + # These are here and not in translate_old_column because the rest of the + # code actually still uses the old names, while the fielddefs table uses + # the new names (which is not the case for the fields handled by + # translate_old_column). + my %old_names = ( + creation_ts => 'opendate', + delta_ts => 'changeddate', + work_time => 'actual_time', + ); + + # Fields that are email addresses + my @email_fields = qw(assigned_to reporter qa_contact); + # Other fields that are stored in the bugs table as an id, but + # should be displayed using their name. + my @id_fields = qw(product component classification); + + foreach my $col (@email_fields) { + my $sql = "map_${col}.login_name"; + if (!Bugzilla->user->id) { + $sql = $dbh->sql_string_until($sql, $dbh->quote('@')); + } + $special_sql{$col} = $sql; + $columns{"${col}_realname"}->{name} = "map_${col}.realname"; + } + + foreach my $col (@id_fields) { + $special_sql{$col} = "map_${col}s.name"; + } + + # Do the actual column-getting from fielddefs, now. + foreach my $field (Bugzilla->get_fields({ obsolete => 0, buglist => 1 })) { + my $id = $field->name; + $id = $old_names{$id} if exists $old_names{$id}; + my $sql = 'bugs.' . $field->name; + $sql = $special_sql{$id} if exists $special_sql{$id}; + $columns{$id} = { name => $sql, title => $field->description }; + } + + # The short_short_desc column is identical to short_desc + $columns{'short_short_desc'} = $columns{'short_desc'}; + + Bugzilla::Hook::process("buglist-columns", { columns => \%columns }); + + $cache->{search_columns} = \%columns; + return $cache->{search_columns}; +} + # Create a new Search # Note that the param argument may be modified by Bugzilla::Search sub new { @@ -90,26 +189,18 @@ sub new { sub init { my $self = shift; - my $fieldsref = $self->{'fields'}; + my @fields = @{ $self->{'fields'} || [] }; my $params = $self->{'params'}; $self->{'user'} ||= Bugzilla->user; my $user = $self->{'user'}; - my $orderref = $self->{'order'} || 0; - my @inputorder; - @inputorder = @$orderref if $orderref; + my @inputorder = @{ $self->{'order'} || [] }; my @orderby; - my $debug = 0; - my @debugdata; - if ($params->param('debug')) { $debug = 1; } - - my @fields; my @supptables; my @wherepart; my @having; my @groupby; - @fields = @$fieldsref if $fieldsref; my @specialchart; my @andlist; my %chartfields; @@ -117,55 +208,64 @@ sub init { my %special_order = %{SPECIAL_ORDER()}; my %special_order_join = %{SPECIAL_ORDER_JOIN()}; - my @select_fields = Bugzilla->get_fields({ type => FIELD_TYPE_SINGLE_SELECT, - obsolete => 0 }); + my @select_fields = + Bugzilla->get_fields({ type => FIELD_TYPE_SINGLE_SELECT }); - my @multi_select_fields = Bugzilla->get_fields({ type => FIELD_TYPE_MULTI_SELECT, + my @multi_select_fields = Bugzilla->get_fields({ + type => [FIELD_TYPE_MULTI_SELECT, FIELD_TYPE_BUG_URLS], obsolete => 0 }); foreach my $field (@select_fields) { my $name = $field->name; - $special_order{"bugs.$name"} = [ "$name.sortkey", "$name.value" ], - $special_order_join{"bugs.$name"} = + next if $name eq 'product'; # products don't have sortkeys. + $special_order{$name} = [ "$name.sortkey", "$name.value" ], + $special_order_join{$name} = "LEFT JOIN $name ON $name.value = bugs.$name"; } my $dbh = Bugzilla->dbh; + # All items that are in the ORDER BY must be in the SELECT. + foreach my $orderitem (@inputorder) { + my $column_name = split_order_term($orderitem); + if (!grep($_ eq $column_name, @fields)) { + push(@fields, $column_name); + } + } + # First, deal with all the old hard-coded non-chart-based poop. - if (grep(/map_assigned_to/, @$fieldsref)) { + if (grep(/^assigned_to/, @fields)) { push @supptables, "INNER JOIN profiles AS map_assigned_to " . "ON bugs.assigned_to = map_assigned_to.userid"; } - if (grep(/map_reporter/, @$fieldsref)) { + if (grep(/^reporter/, @fields)) { push @supptables, "INNER JOIN profiles AS map_reporter " . "ON bugs.reporter = map_reporter.userid"; } - if (grep(/map_qa_contact/, @$fieldsref)) { + if (grep(/^qa_contact/, @fields)) { push @supptables, "LEFT JOIN profiles AS map_qa_contact " . "ON bugs.qa_contact = map_qa_contact.userid"; } - if (lsearch($fieldsref, 'map_products.name') >= 0) { + if (grep($_ eq 'product' || $_ eq 'classification', @fields)) + { push @supptables, "INNER JOIN products AS map_products " . "ON bugs.product_id = map_products.id"; } - if (lsearch($fieldsref, 'map_classifications.name') >= 0) { - push @supptables, "INNER JOIN products AS map_products " . - "ON bugs.product_id = map_products.id"; + if (grep($_ eq 'classification', @fields)) { push @supptables, "INNER JOIN classifications AS map_classifications " . "ON map_products.classification_id = map_classifications.id"; } - if (lsearch($fieldsref, 'map_components.name') >= 0) { + if (grep($_ eq 'component', @fields)) { push @supptables, "INNER JOIN components AS map_components " . "ON bugs.component_id = map_components.id"; } - if (grep($_ =~/AS (actual_time|percentage_complete)$/, @$fieldsref)) { + if (grep($_ eq 'actual_time' || $_ eq 'percentage_complete', @fields)) { push(@supptables, "LEFT JOIN longdescs AS ldtime " . "ON ldtime.bug_id = bugs.bug_id"); } @@ -220,10 +320,9 @@ sub init { } } - my @legal_fields = ("product", "version", "rep_platform", "op_sys", - "bug_status", "resolution", "priority", "bug_severity", - "assigned_to", "reporter", "component", "classification", - "target_milestone", "bug_group"); + my @legal_fields = ("product", "version", "assigned_to", "reporter", + "component", "classification", "target_milestone", + "bug_group"); # Include custom select fields. push(@legal_fields, map { $_->name } @select_fields); @@ -253,15 +352,7 @@ sub init { next; } my $type = $params->param("emailtype$id"); - if ($type eq "exact") { - $type = "anyexact"; - foreach my $name (split(',', $email)) { - $name = trim($name); - if ($name) { - login_to_id($name, THROW_ERROR); - } - } - } + $type = "anyexact" if ($type eq "exact"); my @clist; foreach my $field ("assigned_to", "reporter", "cc", "qa_contact") { @@ -274,9 +365,17 @@ sub init { } if (@clist) { push(@specialchart, \@clist); - } else { - ThrowUserError("missing_email_type", - { email => $email }); + } + else { + # No field is selected. Nothing to see here. + next; + } + + if ($type eq "anyexact") { + foreach my $name (split(',', $email)) { + $name = trim($name); + login_to_id($name, THROW_ERROR) if $name; + } } } @@ -319,8 +418,22 @@ sub init { my $sql_chvalue = $chvalue ne '' ? $dbh->quote($chvalue) : ''; trick_taint($sql_chvalue); if(!@chfield) { - push(@wherepart, "bugs.delta_ts >= $sql_chfrom") if ($sql_chfrom); - push(@wherepart, "bugs.delta_ts <= $sql_chto") if ($sql_chto); + if ($sql_chfrom) { + my $term = "bugs.delta_ts >= $sql_chfrom"; + push(@wherepart, $term); + $self->search_description({ + field => 'delta_ts', type => 'greaterthaneq', + value => $chfieldfrom, term => $term, + }); + } + if ($sql_chto) { + my $term = "bugs.delta_ts <= $sql_chto"; + push(@wherepart, $term); + $self->search_description({ + field => 'delta_ts', type => 'lessthaneq', + value => $chfieldto, term => $term, + }); + } } else { my $bug_creation_clause; my @list; @@ -330,8 +443,22 @@ sub init { # Treat [Bug creation] differently because we need to look # at bugs.creation_ts rather than the bugs_activity table. my @l; - push(@l, "bugs.creation_ts >= $sql_chfrom") if($sql_chfrom); - push(@l, "bugs.creation_ts <= $sql_chto") if($sql_chto); + if ($sql_chfrom) { + my $term = "bugs.creation_ts >= $sql_chfrom"; + push(@l, $term); + $self->search_description({ + field => 'creation_ts', type => 'greaterthaneq', + value => $chfieldfrom, term => $term, + }); + } + if ($sql_chto) { + my $term = "bugs.creation_ts <= $sql_chto"; + push(@l, $term); + $self->search_description({ + field => 'creation_ts', type => 'lessthaneq', + value => $chfieldto, term => $term, + }); + } $bug_creation_clause = "(" . join(' AND ', @l) . ")"; } else { push(@actlist, get_field_id($f)); @@ -343,18 +470,39 @@ sub init { if(@actlist) { my $extra = " actcheck.bug_id = bugs.bug_id"; push(@list, "(actcheck.bug_when IS NOT NULL)"); - if($sql_chfrom) { - $extra .= " AND actcheck.bug_when >= $sql_chfrom"; - } - if($sql_chto) { - $extra .= " AND actcheck.bug_when <= $sql_chto"; - } - if($sql_chvalue) { - $extra .= " AND actcheck.added = $sql_chvalue"; - } + + my $from_term = " AND actcheck.bug_when >= $sql_chfrom"; + $extra .= $from_term if $sql_chfrom; + my $to_term = " AND actcheck.bug_when <= $sql_chto"; + $extra .= $to_term if $sql_chto; + my $value_term = " AND actcheck.added = $sql_chvalue"; + $extra .= $value_term if $sql_chvalue; + push(@supptables, "LEFT JOIN bugs_activity AS actcheck " . "ON $extra AND " . $dbh->sql_in('actcheck.fieldid', \@actlist)); + + foreach my $field (@chfield) { + next if $field eq "[Bug creation]"; + if ($sql_chvalue) { + $self->search_description({ + field => $field, type => 'changedto', + value => $chvalue, term => $value_term, + }); + } + if ($sql_chfrom) { + $self->search_description({ + field => $field, type => 'changedafter', + value => $chfieldfrom, term => $from_term, + }); + } + if ($sql_chvalue) { + $self->search_description({ + field => $field, type => 'changedbefore', + value => $chfieldto, term => $to_term, + }); + } + } } # Now that we're done using @list to determine if there are any @@ -369,7 +517,7 @@ sub init { my $sql_deadlinefrom; my $sql_deadlineto; - if ($user->in_group(Bugzilla->params->{'timetrackinggroup'})) { + if ($user->is_timetracker) { my $deadlinefrom; my $deadlineto; @@ -380,7 +528,12 @@ sub init { format => 'YYYY-MM-DD'}); $sql_deadlinefrom = $dbh->quote($deadlinefrom); trick_taint($sql_deadlinefrom); - push(@wherepart, "bugs.deadline >= $sql_deadlinefrom"); + my $term = "bugs.deadline >= $sql_deadlinefrom"; + push(@wherepart, $term); + $self->search_description({ + field => 'deadline', type => 'greaterthaneq', + value => $deadlinefrom, term => $term, + }); } if ($params->param('deadlineto')){ @@ -390,11 +543,16 @@ sub init { format => 'YYYY-MM-DD'}); $sql_deadlineto = $dbh->quote($deadlineto); trick_taint($sql_deadlineto); - push(@wherepart, "bugs.deadline <= $sql_deadlineto"); + my $term = "bugs.deadline <= $sql_deadlineto"; + push(@wherepart, $term); + $self->search_description({ + field => 'deadline', type => 'lessthaneq', + value => $deadlineto, term => $term, + }); } } - foreach my $f ("short_desc", "long_desc", "bug_file_loc", + foreach my $f ("short_desc", "longdesc", "bug_file_loc", "status_whiteboard") { if (defined $params->param($f)) { my $s = trim($params->param($f)); @@ -416,8 +574,6 @@ sub init { my $chartid; my $sequence = 0; - # $type_id is used by the code that queries for attachment flags. - my $type_id = 0; my $f; my $ff; my $t; @@ -460,6 +616,7 @@ sub init { "^(?:deadline|creation_ts|delta_ts),(?:lessthan|greaterthan|equals|notequals),(?:-|\\+)?(?:\\d+)(?:[dDwWmMyY])\$" => \&_timestamp_compare, "^commenter,(?:equals|anyexact),(%\\w+%)" => \&_commenter_exact, "^commenter," => \&_commenter, + # The _ is allowed for backwards-compatibility with 3.2 and lower. "^long_?desc," => \&_long_desc, "^longdescs\.isprivate," => \&_longdescs_isprivate, "^work_time,changedby" => \&_work_time_changedby, @@ -541,12 +698,6 @@ sub init { $params->param("field$chart-$row-$col", shift(@$ref)); $params->param("type$chart-$row-$col", shift(@$ref)); $params->param("value$chart-$row-$col", shift(@$ref)); - if ($debug) { - push(@debugdata, "$row-$col = " . - $params->param("field$chart-$row-$col") . ' | ' . - $params->param("type$chart-$row-$col") . ' | ' . - $params->param("value$chart-$row-$col") . ' *'); - } $col++; } @@ -654,6 +805,7 @@ sub init { $params->param("field$chart-$row-$col") ; $col++) { $f = $params->param("field$chart-$row-$col") || "noop"; + my $original_f = $f; # Saved for search_description $t = $params->param("type$chart-$row-$col") || "noop"; $v = $params->param("value$chart-$row-$col"); $v = "" if !defined $v; @@ -680,24 +832,21 @@ sub init { foreach my $key (@funcnames) { if ("$f,$t,$rhs" =~ m/$key/) { my $ref = $funcsbykey{$key}; - if ($debug) { - push(@debugdata, "$key ($f / $t / $rhs) =>"); - } $ff = $f; if ($f !~ /\./) { $ff = "bugs.$f"; } $self->$ref(%func_args); - if ($debug) { - push(@debugdata, "$f / $t / $v / " . - ($term || "undef") . " *"); - } if ($term) { last; } } } if ($term) { + $self->search_description({ + field => $original_f, type => $t, value => $v, + term => $term, + }); push(@orlist, $term); } else { @@ -726,13 +875,6 @@ sub init { # to other parts of the query, so we want to create it before we # write the FROM clause. foreach my $orderitem (@inputorder) { - # Some fields have 'AS' aliases. The aliases go in the ORDER BY, - # not the whole fields. - # XXX - Ideally, we would get just the aliases in @inputorder, - # and we'd never have to deal with this. - if ($orderitem =~ /\s+AS\s+(.+)$/i) { - $orderitem = $1; - } BuildOrderBy(\%special_order, $orderitem, \@orderby); } # Now JOIN the correct tables in the FROM clause. @@ -740,9 +882,9 @@ sub init { # cleaner to do it this way. foreach my $orderitem (@inputorder) { # Grab the part without ASC or DESC. - my @splitfield = split(/\s+/, $orderitem); - if ($special_order_join{$splitfield[0]}) { - push(@supptables, $special_order_join{$splitfield[0]}); + my $column_name = split_order_term($orderitem); + if ($special_order_join{$column_name}) { + push(@supptables, $special_order_join{$column_name}); } } @@ -771,14 +913,17 @@ sub init { # Make sure we create a legal SQL query. @andlist = ("1 = 1") if !@andlist; - my $query = "SELECT " . join(', ', @fields) . + my @sql_fields = map { $_ eq EMPTY_COLUMN ? EMPTY_COLUMN + : COLUMNS->{$_}->{name} . ' AS ' . $_ } @fields; + my $query = "SELECT " . join(', ', @sql_fields) . " FROM $suppstring" . " LEFT JOIN bug_group_map " . " ON bug_group_map.bug_id = bugs.bug_id "; if ($user->id) { - if (%{$user->groups}) { - $query .= " AND bug_group_map.group_id NOT IN (" . join(',', values(%{$user->groups})) . ") "; + if (scalar @{ $user->groups }) { + $query .= " AND bug_group_map.group_id NOT IN (" + . $user->groups_as_string . ") "; } $query .= " LEFT JOIN cc ON cc.bug_id = bugs.bug_id AND cc.who = " . $user->id; @@ -797,17 +942,21 @@ sub init { } } - foreach my $field (@fields, @orderby) { - next if ($field =~ /(AVG|SUM|COUNT|MAX|MIN|VARIANCE)\s*\(/i || - $field =~ /^\d+$/ || $field eq "bugs.bug_id" || - $field =~ /^(relevance|actual_time|percentage_complete)/); - # The structure of fields is of the form: - # [foo AS] {bar | bar.baz} [ASC | DESC] - # Only the mandatory part bar OR bar.baz is of interest. - # But for Oracle, it needs the real name part instead. - my $regexp = $dbh->GROUPBY_REGEXP; - if ($field =~ /$regexp/i) { - push(@groupby, $1) if !grep($_ eq $1, @groupby); + # For some DBs, every field in the SELECT must be in the GROUP BY. + foreach my $field (@fields) { + # These fields never go into the GROUP BY (bug_id goes in + # explicitly, below). + next if (grep($_ eq $field, EMPTY_COLUMN, + qw(bug_id actual_time percentage_complete))); + my $col = COLUMNS->{$field}->{name}; + push(@groupby, $col) if !grep($_ eq $col, @groupby); + } + # And all items from ORDER BY must be in the GROUP BY. The above loop + # doesn't catch items that were put into the ORDER BY from SPECIAL_ORDER. + foreach my $item (@inputorder) { + my $column_name = split_order_term($item); + if ($special_order{$column_name}) { + push(@groupby, @{ $special_order{$column_name} }); } } $query .= ") " . $dbh->sql_group_by("bugs.bug_id", join(', ', @groupby)); @@ -822,7 +971,6 @@ sub init { } $self->{'sql'} = $query; - $self->{'debugdata'} = \@debugdata; } ############################################################################### @@ -926,9 +1074,13 @@ sub getSQL { return $self->{'sql'}; } -sub getDebugData { - my $self = shift; - return $self->{'debugdata'}; +sub search_description { + my ($self, $params) = @_; + my $desc = $self->{'search_description'} ||= []; + if ($params) { + push(@$desc, $params); + } + return $self->{'search_description'}; } sub pronoun { @@ -988,9 +1140,7 @@ sub IsValidQueryType sub BuildOrderBy { my ($special_order, $orderitem, $stringlist, $reverseorder) = (@_); - my @twopart = split(/\s+/, $orderitem); - my $orderfield = $twopart[0]; - my $orderdirection = $twopart[1] || ""; + my ($orderfield, $orderdirection) = split_order_term($orderitem); if ($reverseorder) { # If orderdirection is empty or ASC... @@ -1017,6 +1167,40 @@ sub BuildOrderBy { push(@$stringlist, trim($orderfield . ' ' . $orderdirection)); } +# Splits out "asc|desc" from a sort order item. +sub split_order_term { + my $fragment = shift; + $fragment =~ /^(.+?)(?:\s+(ASC|DESC))?$/i; + my ($column_name, $direction) = (lc($1), uc($2)); + $direction ||= ""; + return wantarray ? ($column_name, $direction) : $column_name; +} + +# Used to translate old SQL fragments from buglist.cgi's "order" argument +# into our modern field IDs. +sub translate_old_column { + my ($column) = @_; + # All old SQL fragments have a period in them somewhere. + return $column if $column !~ /\./; + + if ($column =~ /\bAS\s+(\w+)$/i) { + return $1; + } + # product, component, classification, assigned_to, qa_contact, reporter + elsif ($column =~ /map_(\w+?)s?\.(login_)?name/i) { + return $1; + } + + # If it doesn't match the regexps above, check to see if the old + # SQL fragment matches the SQL of an existing column + foreach my $key (%{ COLUMNS() }) { + next unless exists COLUMNS->{$key}->{name}; + return $key if COLUMNS->{$key}->{name} eq $column; + } + + return $column; +} + ##################################################################### # Search Functions ##################################################################### @@ -1032,7 +1216,7 @@ sub _contact_exact_group { my $group = $1; my $groupid = Bugzilla::Group::ValidateGroupName( $group, ($user)); $groupid || ThrowUserError('invalid_group_name',{name => $group}); - my @childgroups = @{$user->flatten_group_membership($groupid)}; + my @childgroups = @{Bugzilla::Group->flatten_group_membership($groupid)}; my $table = "user_group_map_$$chartid"; push (@$supptables, "LEFT JOIN user_group_map AS $table " . "ON $table.user_id = bugs.$$f " . @@ -1104,7 +1288,7 @@ sub _cc_exact_group { my $group = $1; my $groupid = Bugzilla::Group::ValidateGroupName( $group, ($user)); $groupid || ThrowUserError('invalid_group_name',{name => $group}); - my @childgroups = @{$user->flatten_group_membership($groupid)}; + my @childgroups = @{Bugzilla::Group->flatten_group_membership($groupid)}; my $chartseq = $$chartid; if ($$chartid eq "") { $chartseq = "CC$$sequence"; @@ -1237,7 +1421,7 @@ sub _content_matches my $l = FULLTEXT_BUGLIST_LIMIT; my $table = "bugs_fulltext_$$chartid"; my $comments_col = "comments"; - $comments_col = "comments_noprivate" unless $self->{user}->is_insider; + $comments_col = "comments_noprivate" unless $self->{'user'}->is_insider; # Create search terms to add to the SELECT and WHERE clauses. my $text = stem_text($$v); @@ -1424,8 +1608,8 @@ sub _percentage_complete { } if ($oper ne "noop") { my $table = "longdescs_$$chartid"; - if(lsearch($fields, "bugs.remaining_time") == -1) { - push(@$fields, "bugs.remaining_time"); + if (!grep($_ eq 'remaining_time', @$fields)) { + push(@$fields, "remaining_time"); } push(@$supptables, "LEFT JOIN longdescs AS $table " . "ON $table.bug_id = bugs.bug_id"); @@ -2159,7 +2343,7 @@ sub LookupNamedQuery "SELECT group_id FROM namedquery_group_map WHERE namedquery_id = ?", undef, $id ); - if (!grep { $_ == $group } values %{$user->groups}) + if (!grep { $_->id == $group } @{ $user->groups }) { ThrowUserError("missing_query", { queryname => $name, diff --git a/Bugzilla/Series.pm b/Bugzilla/Series.pm index 95d0a8f50..1aaf287ce 100644 --- a/Bugzilla/Series.pm +++ b/Bugzilla/Series.pm @@ -33,7 +33,6 @@ package Bugzilla::Series; use Bugzilla::Error; use Bugzilla::Util; -use Bugzilla::User; sub new { my $invocant = shift; diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm index f8b77331c..289e17260 100644 --- a/Bugzilla/Status.pm +++ b/Bugzilla/Status.pm @@ -22,35 +22,99 @@ use strict; package Bugzilla::Status; -use base qw(Bugzilla::Object Exporter); -@Bugzilla::Status::EXPORT = qw(BUG_STATE_OPEN is_open_state closed_bug_statuses); +use Bugzilla::Error; + +use base qw(Bugzilla::Field::Choice Exporter); +@Bugzilla::Status::EXPORT = qw( + BUG_STATE_OPEN + SPECIAL_STATUS_WORKFLOW_ACTIONS + + is_open_state + closed_bug_statuses +); ################################ ##### Initialization ##### ################################ -use constant DB_TABLE => 'bug_status'; - -use constant DB_COLUMNS => qw( - id - value - sortkey - isactive - is_open +use constant SPECIAL_STATUS_WORKFLOW_ACTIONS => qw( + none + duplicate + change_resolution + clearresolution ); -use constant NAME_FIELD => 'value'; -use constant LIST_ORDER => 'sortkey, value'; +use constant DB_TABLE => 'bug_status'; + +# This has all the standard Bugzilla::Field::Choice columns plus "is_open" +sub DB_COLUMNS { + return ($_[0]->SUPER::DB_COLUMNS, 'is_open'); +} + +sub VALIDATORS { + my $invocant = shift; + my $validators = $invocant->SUPER::VALIDATORS; + $validators->{is_open} = \&Bugzilla::Object::check_boolean; + $validators->{value} = \&_check_value; + return $validators; +} + +######################### +# Database Manipulation # +######################### + +sub create { + my $class = shift; + my $self = $class->SUPER::create(@_); + add_missing_bug_status_transitions(); + return $self; +} + +sub remove_from_db { + my $self = shift; + my $dbh = Bugzilla->dbh; + my $id = $self->id; + $dbh->bz_start_transaction(); + $self->SUPER::remove_from_db(); + $dbh->do('DELETE FROM status_workflow + WHERE old_status = ? OR new_status = ?', + undef, $id, $id); + $dbh->bz_commit_transaction(); +} ############################### ##### Accessors #### ############################### -sub name { return $_[0]->{'value'}; } -sub sortkey { return $_[0]->{'sortkey'}; } sub is_active { return $_[0]->{'isactive'}; } sub is_open { return $_[0]->{'is_open'}; } +sub is_static { + my $self = shift; + if ($self->name eq 'UNCONFIRMED' + || $self->name eq Bugzilla->params->{'duplicate_or_move_bug_status'}) + { + return 1; + } + return 0; +} + +############## +# Validators # +############## + +sub _check_value { + my $invocant = shift; + my $value = $invocant->SUPER::_check_value(@_); + + if (grep { lc($value) eq lc($_) } SPECIAL_STATUS_WORKFLOW_ACTIONS) { + ThrowUserError('fieldvalue_reserved_word', + { field => $invocant->field, value => $value }); + } + return $value; +} + + ############################### ##### Methods #### ############################### diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index b57dddf0b..aa79c5124 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -48,7 +48,6 @@ use Cwd qw(abs_path); use MIME::Base64; use MIME::QuotedPrint qw(encode_qp); use Encode qw(encode); -# for time2str - replace by TT Date plugin?? use Date::Format (); use File::Basename qw(dirname); use File::Find; @@ -179,7 +178,7 @@ sub template_exists # If you want to modify this routine, read the comments carefully sub quoteUrls { - my ($text, $curr_bugid) = (@_); + my ($text, $curr_bugid, $already_wrapped) = (@_); return $text unless $text; # We use /g for speed, but uris can have other things inside them @@ -194,6 +193,10 @@ sub quoteUrls { my $chr1 = chr(1); $text =~ s/\0/$chr1\0/g; + # If the comment is already wrapped, we should ignore newlines when + # looking for matching regexps. Else we should take them into account. + my $s = $already_wrapped ? qr/\s/ : qr/[[:blank:]]/; + # However, note that adding the title (for buglinks) can affect things # In particular, attachment matches go before bug titles, so that titles # with 'attachment 1' don't double match. @@ -212,7 +215,7 @@ sub quoteUrls { map { qr/$_/ } grep($_, Bugzilla->params->{'urlbase'}, Bugzilla->params->{'sslbase'})) . ')'; $text =~ s~\b(${urlbase_re}\Qshow_bug.cgi?id=\E([0-9]+)(\#c([0-9]+))?)\b - ~($things[$count++] = get_bug_link($3, $1, $5)) && + ~($things[$count++] = get_bug_link($3, $1, { comment_num => $5 })) && ("\0\0" . ($count-1) . "\0\0") ~egox; @@ -259,7 +262,7 @@ sub quoteUrls { ("\0\0" . ($count-1) . "\0\0") ~egmx; - $text =~ s~\b(attachment\s*\#?\s*(\d+)) + $text =~ s~\b(attachment$s*\#?$s*(\d+)) ~($things[$count++] = get_attachment_link($2, $1)) && ("\0\0" . ($count-1) . "\0\0") ~egmxi; @@ -272,12 +275,12 @@ sub quoteUrls { # Also, we can't use $bug_re?$comment_re? because that will match the # empty string my $bug_word = get_text('term', { term => 'bug' }); - my $bug_re = qr/\Q$bug_word\E\s*\#?\s*(\d+)/i; - my $comment_re = qr/comment\s*\#?\s*(\d+)/i; - $text =~ s~\b($bug_re(?:\s*,?\s*$comment_re)?|$comment_re) + my $bug_re = qr/\Q$bug_word\E$s*\#?$s*(\d+)/i; + my $comment_re = qr/comment$s*\#?$s*(\d+)/i; + $text =~ s~\b($bug_re(?:$s*,?$s*$comment_re)?|$comment_re) ~ # We have several choices. $1 here is the link, and $2-4 are set # depending on which part matched - (defined($2) ? get_bug_link($2,$1,$3) : + (defined($2) ? get_bug_link($2, $1, { comment_num => $3 }) : "$1") ~egox; @@ -342,7 +345,7 @@ sub get_attachment_link { # comment in the bug sub get_bug_link { - my ($bug_num, $link_text, $comment_num) = @_; + my ($bug_num, $link_text, $options) = @_; my $dbh = Bugzilla->dbh; if (!defined($bug_num) || ($bug_num eq "")) { @@ -351,11 +354,15 @@ sub get_bug_link { my $quote_bug_num = html_quote($bug_num); detaint_natural($bug_num) || return "<invalid bug number: $quote_bug_num>"; - my ($bug_state, $bug_res, $bug_desc) = - $dbh->selectrow_array('SELECT bugs.bug_status, resolution, short_desc + my ($bug_alias, $bug_state, $bug_res, $bug_desc) = + $dbh->selectrow_array('SELECT bugs.alias, bugs.bug_status, bugs.resolution, bugs.short_desc FROM bugs WHERE bugs.bug_id = ?', undef, $bug_num); + if ($options->{use_alias} && $link_text =~ /^\d+$/ && $bug_alias) { + $link_text = $bug_alias; + } + if ($bug_state) { # Initialize these variables to be "" so that we don't get warnings # if we don't change them below (which is highly likely). @@ -378,8 +385,8 @@ sub get_bug_link { $title = html_quote(clean_text($title)); my $linkval = "show_bug.cgi?id=$bug_num"; - if (defined $comment_num) { - $linkval .= "#c$comment_num"; + if ($options->{comment_num}) { + $linkval .= "#c" . $options->{comment_num}; } return qq{$pre$link_text$post}; } @@ -588,20 +595,20 @@ sub create { css_class_quote => \&Bugzilla::Util::css_class_quote , quoteUrls => [ sub { - my ($context, $bug) = @_; + my ($context, $bug, $already_wrapped) = @_; return sub { my $text = shift; - return quoteUrls($text, $bug); + return quoteUrls($text, $bug, $already_wrapped); }; }, 1 ], bug_link => [ sub { - my ($context, $bug) = @_; + my ($context, $bug, $options) = @_; return sub { my $text = shift; - return get_bug_link($bug, $text); + return get_bug_link($bug, $text, $options); }; }, 1 @@ -665,7 +672,15 @@ sub create { }, # Format a time for display (more info in Bugzilla::Util) - time => \&Bugzilla::Util::format_time, + time => [ sub { + my ($context, $format, $timezone) = @_; + return sub { + my $time = shift; + return format_time($time, $format, $timezone); + }; + }, + 1 + ], # Bug 120030: Override html filter to obscure the '@' in user # visible strings. @@ -703,6 +718,8 @@ sub create { html_light => \&Bugzilla::Util::html_light_quote, + email => \&Bugzilla::Util::email_filter, + # iCalendar contentline filter ics => [ sub { my ($context, @args) = @_; @@ -741,6 +758,10 @@ sub create { $var =~ s/\>/>/g; $var =~ s/\"/\"/g; $var =~ s/\&/\&/g; + # Now remove extra whitespace, and wrap it to 72 characters. + my $collapse_filter = $Template::Filters::FILTERS->{collapse}; + $var = $collapse_filter->($var); + $var = wrap_comment($var, 72); return $var; }, diff --git a/Bugzilla/Testopia/Classification.pm b/Bugzilla/Testopia/Classification.pm index 8270cf6c8..b14cefa14 100644 --- a/Bugzilla/Testopia/Classification.pm +++ b/Bugzilla/Testopia/Classification.pm @@ -46,9 +46,9 @@ sub user_visible_products { $query .= "AND group_control_map.membercontrol = " . CONTROLMAPMANDATORY . " "; } - if (%{Bugzilla->user->groups}) { + if (@{Bugzilla->user->groups}) { $query .= "AND group_id NOT IN(" . - join(',', values(%{Bugzilla->user->groups})) . ") "; + join(',', map { $_->id } @{Bugzilla->user->groups}) . ") "; } $query .= "WHERE group_id IS NULL AND products.classification_id= ? ORDER BY products.name"; diff --git a/Bugzilla/Testopia/Environment/Category.pm b/Bugzilla/Testopia/Environment/Category.pm index e36b1bfc7..3de1c009c 100644 --- a/Bugzilla/Testopia/Environment/Category.pm +++ b/Bugzilla/Testopia/Environment/Category.pm @@ -234,9 +234,9 @@ sub get_env_product_list { $query .= "AND group_control_map.membercontrol = " . CONTROLMAPMANDATORY . " "; } - if ( %{ Bugzilla->user->groups } ) { + if ( @{ Bugzilla->user->groups } ) { $query .= "AND group_id NOT IN(" - . join( ',', values( %{ Bugzilla->user->groups } ) ) . ") "; + . join( ',', map { $_->id } @{ Bugzilla->user->groups } ) . ") "; } $query .= "LEFT OUTER JOIN test_environment_category AS tec diff --git a/Bugzilla/Testopia/TestRun.pm b/Bugzilla/Testopia/TestRun.pm index 74cf7766e..95f0d8bf0 100644 --- a/Bugzilla/Testopia/TestRun.pm +++ b/Bugzilla/Testopia/TestRun.pm @@ -824,9 +824,9 @@ sub get_distinct_builds { $query .= "AND group_control_map.membercontrol = " . CONTROLMAPMANDATORY . " "; } - if (%{Bugzilla->user->groups}) { + if (@{Bugzilla->user->groups}) { $query .= "AND group_id NOT IN(" . - join(',', values(%{Bugzilla->user->groups})) . ") "; + join(',', map { $_->id } @{Bugzilla->user->groups}) . ") "; } $query .= "WHERE group_id IS NULL AND build.isactive = 1 ORDER BY build.name"; diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm index a8862bd5a..4aee0561e 100644 --- a/Bugzilla/Token.pm +++ b/Bugzilla/Token.pm @@ -76,7 +76,7 @@ sub issue_new_user_account_token { my ($token, $token_ts) = _create_token(undef, 'account', $login_name); $vars->{'email'} = $login_name . Bugzilla->params->{'emailsuffix'}; - $vars->{'token_ts'} = $token_ts; + $vars->{'expiration_ts'} = ctime($token_ts + MAX_TOKEN_AGE * 86400); $vars->{'token'} = $token; my $message; @@ -105,10 +105,7 @@ sub IssueEmailChangeToken { $vars->{'oldemailaddress'} = $old_email . $email_suffix; $vars->{'newemailaddress'} = $new_email . $email_suffix; - - $vars->{'max_token_age'} = MAX_TOKEN_AGE; - $vars->{'token_ts'} = $token_ts; - + $vars->{'expiration_ts'} = ctime($token_ts + MAX_TOKEN_AGE * 86400); $vars->{'token'} = $token; $vars->{'emailaddress'} = $old_email . $email_suffix; @@ -153,8 +150,10 @@ sub IssuePasswordToken { $vars->{'token'} = $token; $vars->{'emailaddress'} = $user->email; - $vars->{'max_token_age'} = MAX_TOKEN_AGE; - $vars->{'token_ts'} = $token_ts; + $vars->{'expiration_ts'} = ctime($token_ts + MAX_TOKEN_AGE * 86400); + # The user is not logged in (else he wouldn't request a new password). + # So we have to pass this information to the template. + $vars->{'timezone'} = $user->timezone; my $message = ""; $template->process("account/password/forgotten-password.txt.tmpl", @@ -288,6 +287,9 @@ sub Cancel { $vars->{'token'} = $token; $vars->{'tokentype'} = $tokentype; $vars->{'issuedate'} = $issuedate; + # The user is probably not logged in. + # So we have to pass this information to the template. + $vars->{'timezone'} = $user->timezone; $vars->{'eventdata'} = $eventdata; $vars->{'cancelaction'} = $cancelaction; diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 3429e7c57..f0e35beb1 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -48,6 +48,10 @@ use Bugzilla::User::Setting; use Bugzilla::Product; use Bugzilla::Classification; use Bugzilla::Field; +use Bugzilla::Group; + +use Scalar::Util qw(blessed); +use DateTime::TimeZone; use base qw(Bugzilla::Object Exporter); @Bugzilla::User::EXPORT = qw(is_available_username @@ -362,6 +366,22 @@ sub settings { return $self->{'settings'}; } +sub timezone { + my $self = shift; + + if (!defined $self->{timezone}) { + my $tz = $self->settings->{timezone}->{value}; + if ($tz eq 'local') { + # The user wants the local timezone of the server. + $self->{timezone} = Bugzilla->local_timezone; + } + else { + $self->{timezone} = DateTime::TimeZone->new(name => $tz); + } + } + return $self->{timezone}; +} + sub flush_queries_cache { my $self = shift; @@ -374,75 +394,62 @@ sub groups { my $self = shift; return $self->{groups} if defined $self->{groups}; - return {} unless $self->id; + return [] unless $self->id; my $dbh = Bugzilla->dbh; - my $groups = $dbh->selectcol_arrayref(q{SELECT DISTINCT groups.name, group_id - FROM groups, user_group_map - WHERE groups.id=user_group_map.group_id - AND user_id=? - AND isbless=0}, - { Columns=>[1,2] }, - $self->id); + my $groups_to_check = $dbh->selectcol_arrayref( + q{SELECT DISTINCT group_id + FROM user_group_map + WHERE user_id = ? AND isbless = 0}, undef, $self->id); - # The above gives us an arrayref [name, id, name, id, ...] - # Convert that into a hashref - my %groups = @$groups; - my @groupidstocheck = values(%groups); - my %groupidschecked = (); my $rows = $dbh->selectall_arrayref( - "SELECT DISTINCT groups.name, groups.id, member_id + "SELECT DISTINCT grantor_id, member_id FROM group_group_map - INNER JOIN groups - ON groups.id = grantor_id WHERE grant_type = " . GROUP_MEMBERSHIP); - my %group_names = (); - my %group_membership = (); - foreach my $row (@$rows) { - my ($member_name, $grantor_id, $member_id) = @$row; - # Just save the group names - $group_names{$grantor_id} = $member_name; - # And group membership - push (@{$group_membership{$member_id}}, $grantor_id); + my %group_membership; + foreach my $row (@$rows) { + my ($grantor_id, $member_id) = @$row; + push (@{ $group_membership{$member_id} }, $grantor_id); } # Let's walk the groups hierarchy tree (using FIFO) # On the first iteration it's pre-filled with direct groups # membership. Later on, each group can add its own members into the # FIFO. Circular dependencies are eliminated by checking - # $groupidschecked{$member_id} hash values. + # $checked_groups{$member_id} hash values. # As a result, %groups will have all the groups we are the member of. - while ($#groupidstocheck >= 0) { + my %checked_groups; + my %groups; + while (scalar(@$groups_to_check) > 0) { # Pop the head group from FIFO - my $member_id = shift @groupidstocheck; + my $member_id = shift @$groups_to_check; # Skip the group if we have already checked it - if (!$groupidschecked{$member_id}) { + if (!$checked_groups{$member_id}) { # Mark group as checked - $groupidschecked{$member_id} = 1; + $checked_groups{$member_id} = 1; # Add all its members to the FIFO check list # %group_membership contains arrays of group members # for all groups. Accessible by group number. - foreach my $newgroupid (@{$group_membership{$member_id}}) { - push @groupidstocheck, $newgroupid - if (!$groupidschecked{$newgroupid}); - } - # Note on if clause: we could have group in %groups from 1st - # query and do not have it in second one - $groups{$group_names{$member_id}} = $member_id - if $group_names{$member_id} && $member_id; + my $members = $group_membership{$member_id}; + my @new_to_check = grep(!$checked_groups{$_}, @$members); + push(@$groups_to_check, @new_to_check); + + $groups{$member_id} = 1; } } - $self->{groups} = \%groups; + + $self->{groups} = Bugzilla::Group->new_from_list([keys %groups]); return $self->{groups}; } sub groups_as_string { my $self = shift; - return (join(',',values(%{$self->groups})) || '-1'); + my @ids = map { $_->id } @{ $self->groups }; + return scalar(@ids) ? join(',', @ids) : '-1'; } sub bless_groups { @@ -451,54 +458,45 @@ sub bless_groups { return $self->{'bless_groups'} if defined $self->{'bless_groups'}; return [] unless $self->id; - my $dbh = Bugzilla->dbh; - my $query; - my $connector; - my @bindValues; - if ($self->in_group('editusers')) { # Users having editusers permissions may bless all groups. - $query = 'SELECT DISTINCT id, name, description FROM groups'; - $connector = 'WHERE'; + $self->{'bless_groups'} = [Bugzilla::Group->get_all]; + return $self->{'bless_groups'}; } - else { + + my $dbh = Bugzilla->dbh; + # Get all groups for the user where: # + They have direct bless privileges # + They are a member of a group that inherits bless privs. - $query = q{ - SELECT DISTINCT groups.id, groups.name, groups.description + my @group_ids = map {$_->id} @{ $self->groups }; + @group_ids = (-1) if !@group_ids; + my $query = + 'SELECT DISTINCT groups.id FROM groups, user_group_map, group_group_map AS ggm WHERE user_group_map.user_id = ? - AND ((user_group_map.isbless = 1 + AND ( (user_group_map.isbless = 1 AND groups.id=user_group_map.group_id) OR (groups.id = ggm.grantor_id - AND ggm.grant_type = ? - AND ggm.member_id IN(} . - $self->groups_as_string . - q{)))}; - $connector = 'AND'; - @bindValues = ($self->id, GROUP_BLESS); - } + AND ggm.grant_type = ' . GROUP_BLESS . ' + AND ' . $dbh->sql_in('ggm.member_id', \@group_ids) + . ') )'; # If visibilitygroups are used, restrict the set of groups. - if (!$self->in_group('editusers') - && Bugzilla->params->{'usevisibilitygroups'}) - { + if (Bugzilla->params->{'usevisibilitygroups'}) { + return [] if !$self->visible_groups_as_string; # Users need to see a group in order to bless it. - my $visibleGroups = join(', ', @{$self->visible_groups_direct()}) - || return $self->{'bless_groups'} = []; - $query .= " $connector id in ($visibleGroups)"; + $query .= " AND " + . $dbh->sql_in('groups.id', $self->visible_groups_inherited); } - $query .= ' ORDER BY name'; - - return $self->{'bless_groups'} = - $dbh->selectall_arrayref($query, {'Slice' => {}}, @bindValues); + my $ids = $dbh->selectcol_arrayref($query, undef, $self->id); + return $self->{'bless_groups'} = Bugzilla::Group->new_from_list($ids); } sub in_group { my ($self, $group, $product_id) = @_; - if (exists $self->groups->{$group}) { + if (scalar grep($_->name eq $group, @{ $self->groups })) { return 1; } elsif ($product_id && detaint_natural($product_id)) { @@ -527,8 +525,7 @@ sub in_group { sub in_group_id { my ($self, $id) = @_; - my %j = reverse(%{$self->groups}); - return exists $j{$id} ? 1 : 0; + return grep($_->id == $id, @{ $self->groups }) ? 1 : 0; } sub get_products_by_permission { @@ -590,44 +587,72 @@ sub can_edit_product { } sub can_see_bug { - my ($self, $bugid) = @_; + my ($self, $bug_id) = @_; + return @{ $self->visible_bugs([$bug_id]) } ? 1 : 0; +} + +sub visible_bugs { + my ($self, $bugs) = @_; + # Allow users to pass in Bug objects and bug ids both. + my @bug_ids = map { blessed $_ ? $_->id : $_ } @$bugs; + + # We only check the visibility of bugs that we haven't + # checked yet. + # Bugzilla::Bug->update automatically removes updated bugs + # from the cache to force them to be checked again. + my $visible_cache = $self->{_visible_bugs_cache} ||= {}; + my @check_ids = grep(!exists $visible_cache->{$_}, @bug_ids); + + if (@check_ids) { my $dbh = Bugzilla->dbh; - my $sth = $self->{sthCanSeeBug}; - my $userid = $self->id; - # Get fields from bug, presence of user on cclist, and determine if - # the user is missing any groups required by the bug. The prepared query - # is cached because this may be called for every row in buglists or - # every bug in a dependency list. - unless ($sth) { - $sth = $dbh->prepare("SELECT 1, reporter, assigned_to, qa_contact, - reporter_accessible, cclist_accessible, - COUNT(cc.who), COUNT(bug_group_map.bug_id) + my $user_id = $self->id; + my $sth; + # Speed up the can_see_bug case. + if (scalar(@check_ids) == 1) { + $sth = $self->{_sth_one_visible_bug}; + } + $sth ||= $dbh->prepare( + # This checks for groups that the bug is in that the user + # *isn't* in. Then, in the Perl code below, we check if + # the user can otherwise access the bug (for example, by being + # the assignee or QA Contact). + # + # The DISTINCT exists because the bug could be in *several* + # groups that the user isn't in, but they will all return the + # same result for bug_group_map.bug_id (so DISTINCT filters + # out duplicate rows). + "SELECT DISTINCT bugs.bug_id, reporter, assigned_to, qa_contact, + reporter_accessible, cclist_accessible, cc.who, + bug_group_map.bug_id FROM bugs LEFT JOIN cc ON cc.bug_id = bugs.bug_id - AND cc.who = $userid + AND cc.who = $user_id LEFT JOIN bug_group_map ON bugs.bug_id = bug_group_map.bug_id - AND bug_group_map.group_ID NOT IN(" . - $self->groups_as_string . - ") WHERE bugs.bug_id = ? - AND creation_ts IS NOT NULL " . - $dbh->sql_group_by('bugs.bug_id', 'reporter, ' . - 'assigned_to, qa_contact, reporter_accessible, ' . - 'cclist_accessible')); - } - $sth->execute($bugid); - my ($ready, $reporter, $owner, $qacontact, $reporter_access, $cclist_access, - $isoncclist, $missinggroup) = $sth->fetchrow_array(); - $sth->finish; - $self->{sthCanSeeBug} = $sth; - return ($ready - && ((($reporter == $userid) && $reporter_access) + AND bug_group_map.group_id NOT IN (" + . $self->groups_as_string . ') + WHERE bugs.bug_id IN (' . join(',', ('?') x @check_ids) . ') + AND creation_ts IS NOT NULL '); + if (scalar(@check_ids) == 1) { + $self->{_sth_one_visible_bug} = $sth; + } + + $sth->execute(@check_ids); + while (my $row = $sth->fetchrow_arrayref) { + my ($bug_id, $reporter, $owner, $qacontact, $reporter_access, + $cclist_access, $isoncclist, $missinggroup) = @$row; + $visible_cache->{$bug_id} ||= + ((($reporter == $user_id) && $reporter_access) || (Bugzilla->params->{'useqacontact'} - && $qacontact && ($qacontact == $userid)) - || ($owner == $userid) + && $qacontact && ($qacontact == $user_id)) + || ($owner == $user_id) || ($isoncclist && $cclist_access) - || (!$missinggroup))); + || !$missinggroup) ? 1 : 0; + } + } + + return [grep { $visible_cache->{blessed $_ ? $_->id : $_} } @$bugs]; } sub can_see_product { @@ -677,20 +702,12 @@ sub get_selectable_products { sub get_selectable_classifications { my ($self) = @_; - if (defined $self->{selectable_classifications}) { - return $self->{selectable_classifications}; - } - + if (!defined $self->{selectable_classifications}) { my $products = $self->get_selectable_products; + my %class_ids = map { $_->classification_id => 1 } @$products; - my $class; - foreach my $product (@$products) { - $class->{$product->classification_id} ||= - new Bugzilla::Classification($product->classification_id); + $self->{selectable_classifications} = Bugzilla::Classification->new_from_list([keys %class_ids]); } - my @sorted_class = sort {$a->sortkey <=> $b->sortkey - || lc($a->name) cmp lc($b->name)} (values %$class); - $self->{selectable_classifications} = \@sorted_class; return $self->{selectable_classifications}; } @@ -839,7 +856,7 @@ sub visible_groups_inherited { return $self->{visible_groups_inherited} if defined $self->{visible_groups_inherited}; return [] unless $self->id; my @visgroups = @{$self->visible_groups_direct}; - @visgroups = @{$self->flatten_group_membership(@visgroups)}; + @visgroups = @{Bugzilla::Group->flatten_group_membership(@visgroups)}; $self->{visible_groups_inherited} = \@visgroups; return $self->{visible_groups_inherited}; } @@ -856,7 +873,7 @@ sub visible_groups_direct { my $sth; if (Bugzilla->params->{'usevisibilitygroups'}) { - my $glist = join(',',(-1,values(%{$self->groups}))); + my $glist = $self->groups_as_string; $sth = $dbh->prepare("SELECT DISTINCT grantor_id FROM group_group_map WHERE member_id IN($glist) @@ -901,7 +918,7 @@ sub queryshare_groups { } } else { - @queryshare_groups = values(%{$self->groups}); + @queryshare_groups = map { $_->id } @{ $self->groups }; } } @@ -1020,36 +1037,12 @@ sub can_bless { if (!scalar(@_)) { # If we're called without an argument, just return # whether or not we can bless at all. - return scalar(@{$self->bless_groups}) ? 1 : 0; + return scalar(@{ $self->bless_groups }) ? 1 : 0; } # Otherwise, we're checking a specific group my $group_id = shift; - return (grep {$$_{'id'} eq $group_id} (@{$self->bless_groups})) ? 1 : 0; -} - -sub flatten_group_membership { - my ($self, @groups) = @_; - - my $dbh = Bugzilla->dbh; - my $sth; - my @groupidstocheck = @groups; - my %groupidschecked = (); - $sth = $dbh->prepare("SELECT member_id FROM group_group_map - WHERE grantor_id = ? - AND grant_type = " . GROUP_MEMBERSHIP); - while (my $node = shift @groupidstocheck) { - $sth->execute($node); - my $member; - while (($member) = $sth->fetchrow_array) { - if (!$groupidschecked{$member}) { - $groupidschecked{$member} = 1; - push @groupidstocheck, $member; - push @groups, $member unless grep $_ == $member, @groups; - } - } - } - return \@groups; + return grep($_->id == $group_id, @{ $self->bless_groups }) ? 1 : 0; } sub match { @@ -1121,7 +1114,6 @@ sub match { && (Bugzilla->params->{'usermatchmode'} eq 'search') && (length($str) >= 3)) { - $str = lc($str); trick_taint($str); my $query = "SELECT DISTINCT login_name FROM profiles "; @@ -1130,8 +1122,8 @@ sub match { ON user_group_map.user_id = profiles.userid "; } $query .= " WHERE (" . - $dbh->sql_position('?', 'LOWER(login_name)') . " > 0" . " OR " . - $dbh->sql_position('?', 'LOWER(realname)') . " > 0) "; + $dbh->sql_iposition('?', 'login_name') . " > 0" . " OR " . + $dbh->sql_iposition('?', 'realname') . " > 0) "; if (Bugzilla->params->{'usevisibilitygroups'}) { $query .= " AND isbless = 0" . " AND group_id IN(" . @@ -1429,8 +1421,9 @@ our %names_to_events = ( # Note: the "+" signs before the constants suppress bareword quoting. sub wants_bug_mail { my $self = shift; - my ($bug_id, $relationship, $fieldDiffs, $commentField, $dependencyText, + my ($bug_id, $relationship, $fieldDiffs, $comments, $dependencyText, $changer, $bug_is_new) = @_; + my $comments_concatenated = join("\n", map { $_->{body} } (@$comments)); # Make a list of the events which have happened during this bug change, # from the point of view of this user. @@ -1465,20 +1458,24 @@ sub wants_bug_mail { } } + if ($bug_is_new) { + # Notify about new bugs. + $events{+EVT_BUG_CREATED} = 1; + # You role is new if the bug itself is. # Only makes sense for the assignee, QA contact and the CC list. - if ($bug_is_new - && ($relationship == REL_ASSIGNEE + if ($relationship == REL_ASSIGNEE || $relationship == REL_QA - || $relationship == REL_CC)) + || $relationship == REL_CC) { $events{+EVT_ADDED_REMOVED} = 1; } + } - if ($commentField =~ /Created an attachment \(/) { + if ($comments_concatenated =~ /Created an attachment \(/) { $events{+EVT_ATTACHMENT} = 1; } - elsif ($commentField ne '') { + elsif (defined($$comments[0])) { $events{+EVT_COMMENT} = 1; } @@ -1587,6 +1584,17 @@ sub is_global_watcher { return $self->{'is_global_watcher'}; } +sub is_timetracker { + my $self = shift; + + if (!defined $self->{'is_timetracker'}) { + my $tt_group = Bugzilla->params->{'timetrackinggroup'}; + $self->{'is_timetracker'} = + ($tt_group && $self->in_group($tt_group)) ? 1 : 0; + } + return $self->{'is_timetracker'}; +} + sub get_userlist { my $self = shift; @@ -1925,12 +1933,15 @@ value - the value of this setting for this user. Will be the same is_default - a boolean to indicate whether the user has chosen to make a preference for themself or use the site default. +=item C + +Returns the timezone used to display dates and times to the user, +as a DateTime::TimeZone object. + =item C -Returns a hashref of group names for groups the user is a member of. The keys -are the names of the groups, whilst the values are the respective group ids. -(This is so that a set of all groupids for groups the user is in can be -obtained by Cgroups})>.) +Returns an arrayref of L objects representing +groups that this user is a member of. =item C @@ -1950,12 +1961,11 @@ Determines whether or not a user is in the given group by id. =item C -Returns an arrayref of hashes of C entries, where the keys of each hash -are the names of C, C and C columns of the C -table. +Returns an arrayref of L objects. + The arrayref consists of the groups the user can bless, taking into account that having editusers permissions means that you can bless all groups, and -that you need to be aware of a group in order to bless a group. +that you need to be able to see a group in order to bless it. =item C @@ -2083,14 +2093,6 @@ Returns a reference to an array of users. The array is populated with hashrefs containing the login, identity and visibility. Users that are not visible to this user will have 'visible' set to zero. -=item C - -Accepts a list of groups and returns a list of all the groups whose members -inherit membership in any group on the list. So, we can determine if a user -is in any of the groups input to flatten_group_membership by querying the -user_group_map for any user with DIRECT or REGEXP membership IN() the list -of groups returned. - =item C Returns a reference to an array of group objects. Groups the user belong to diff --git a/Bugzilla/User/Setting/Timezone.pm b/Bugzilla/User/Setting/Timezone.pm new file mode 100644 index 000000000..d75b1113b --- /dev/null +++ b/Bugzilla/User/Setting/Timezone.pm @@ -0,0 +1,71 @@ +# -*- Mode: perl; indent-tabs-mode: nil -*- +# +# The contents of this file are subject to the Mozilla Public +# License Version 1.1 (the "License"); you may not use this file +# except in compliance with the License. You may obtain a copy of +# the License at http://www.mozilla.org/MPL/ +# +# Software distributed under the License is distributed on an "AS +# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or +# implied. See the License for the specific language governing +# rights and limitations under the License. +# +# The Original Code is the Bugzilla Bug Tracking System. +# +# The Initial Developer of the Original Code is Frédéric Buclin. +# Portions created by Frédéric Buclin are Copyright (c) 2008 Frédéric Buclin. +# All rights reserved. +# +# Contributor(s): Frédéric Buclin + +package Bugzilla::User::Setting::Timezone; + +use strict; + +use DateTime::TimeZone; + +use base qw(Bugzilla::User::Setting); + +use Bugzilla::Constants; + +sub legal_values { + my ($self) = @_; + + return $self->{'legal_values'} if defined $self->{'legal_values'}; + + my @timezones = DateTime::TimeZone->all_names; + # Remove old formats, such as CST6CDT, EST, EST5EDT. + @timezones = grep { $_ =~ m#.+/.+#} @timezones; + # Append 'local' to the list, which will use the timezone + # given by the server. + push(@timezones, 'local'); + + return $self->{'legal_values'} = \@timezones; +} + +1; + +__END__ + +=head1 NAME + +Bugzilla::User::Setting::Timezone - Object for a user preference setting for desired timezone + +=head1 DESCRIPTION + +Timezone.pm extends Bugzilla::User::Setting and implements a class specialized for +setting the desired timezone. + +=head1 METHODS + +=over + +=item C + +Description: Returns all legal timezones + +Params: none + +Returns: A reference to an array containing the names of all legal timezones + +=back diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 1be64ac5d..28b3b4ec2 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -31,13 +31,13 @@ package Bugzilla::Util; use strict; use base qw(Exporter); -@Bugzilla::Util::EXPORT = qw(is_tainted trick_taint detaint_natural +@Bugzilla::Util::EXPORT = qw(trick_taint detaint_natural detaint_signed html_quote url_quote xml_quote css_class_quote html_light_quote url_decode i_am_cgi get_netaddr correct_urlbase lsearch ssl_require_redirect use_attachbase - diff_arrays diff_strings + diff_arrays trim wrap_hard wrap_comment find_wrap_point format_time format_time_decimal validate_date validate_time @@ -50,19 +50,14 @@ use Bugzilla::Constants; use Date::Parse; use Date::Format; +use DateTime; +use DateTime::TimeZone; +use Digest; +use Email::Address; +use Scalar::Util qw(tainted); use Text::Wrap; use Lingua::Stem::RuUTF8; -# This is from the perlsec page, slightly modified to remove a warning -# From that page: -# This function makes use of the fact that the presence of -# tainted data anywhere within an expression renders the -# entire expression tainted. -# Don't ask me how it works... -sub is_tainted { - return not eval { my $foo = join('',@_), kill 0; 1; }; -} - sub trick_taint { require Carp; Carp::confess("Undef to trick_taint") unless defined $_[0]; @@ -177,6 +172,20 @@ sub html_light_quote { } } +sub email_filter { + my ($toencode) = @_; + if (!Bugzilla->user->id) { + my @emails = Email::Address->parse($toencode); + if (scalar @emails) { + my @hosts = map { quotemeta($_->host) } @emails; + my $hosts_re = join('|', @hosts); + $toencode =~ s/\@(?:$hosts_re)//g; + return $toencode; + } + } + return $toencode; +} + # This originally came from CGI.pm, by Lincoln D. Stein sub url_quote { my ($toencode) = (@_); @@ -337,22 +346,6 @@ sub trim { return $str; } -sub diff_strings { - my ($oldstr, $newstr) = @_; - - # Split the old and new strings into arrays containing their values. - s/[\s,]+/ /gso for $oldstr, $newstr; - my @old = split(" ", $oldstr); - my @new = split(" ", $newstr); - - my ($rem, $add) = diff_arrays(\@old, \@new); - - my $removed = join (", ", @$rem); - my $added = join (", ", @$add); - - return ($removed, $added); -} - sub wrap_comment { my ($comment, $cols) = @_; my $wrappedcomment = ""; @@ -414,38 +407,55 @@ sub wrap_hard { } sub format_time { - my ($date, $format) = @_; + my ($date, $format, $timezone) = @_; # If $format is undefined, try to guess the correct date format. - my $show_timezone; if (!defined($format)) { if ($date =~ m/^(\d{4})[-\.](\d{2})[-\.](\d{2}) (\d{2}):(\d{2})(:(\d{2}))?$/) { my $sec = $7; if (defined $sec) { - $format = "%Y-%m-%d %T"; + $format = "%Y-%m-%d %T %Z"; } else { - $format = "%Y-%m-%d %R"; + $format = "%Y-%m-%d %R %Z"; } } else { - # Default date format. See Date::Format for other formats available. - $format = "%Y-%m-%d %R"; + # Default date format. See DateTime for other formats available. + $format = "%Y-%m-%d %R %Z"; } - # By default, we want the timezone to be displayed. - $show_timezone = 1; - } - else { - # Search for %Z or %z, meaning we want the timezone to be displayed. - # Till bug 182238 gets fixed, we assume Bugzilla->params->{'timezone'} - # is used. - $show_timezone = ($format =~ s/\s?%Z$//i); } - # str2time($date) is undefined if $date has an invalid date format. - my $time = str2time($date); + # strptime($date) returns an empty array if $date has an invalid date format. + my @time = strptime($date); - if (defined $time) { - $date = time2str($format, $time); - $date .= " " . Bugzilla->params->{'timezone'} if $show_timezone; + unless (scalar @time) { + # If an unknown timezone is passed (such as MSK, for Moskow), strptime() is + # unable to parse the date. We try again, but we first remove the timezone. + $date =~ s/\s+\S+$//; + @time = strptime($date); + } + + if (scalar @time) { + # Fix a bug in strptime() where seconds can be undefined in some cases. + $time[0] ||= 0; + + # strptime() counts years from 1900, and months from 0 (January). + # We have to fix both values. + my $dt = DateTime->new({year => 1900 + $time[5], + month => ++$time[4], + day => $time[3], + hour => $time[2], + minute => $time[1], + # DateTime doesn't like fractional seconds. + second => int($time[0]), + # If importing, use the specified timezone, otherwise + # use the timezone specified by the server. + time_zone => Bugzilla->local_timezone->offset_as_string($time[6]) + || Bugzilla->local_timezone}); + + # Now display the date using the given timezone, + # or the user's timezone if none is given. + $dt->set_time_zone($timezone || Bugzilla->user->timezone); + $date = $dt->strftime($format); } else { # Don't let invalid (time) strings to be passed to templates! @@ -475,33 +485,56 @@ sub file_mod_time { } sub bz_crypt { - my ($password) = @_; + my ($password, $salt) = @_; - # The list of characters that can appear in a salt. Salts and hashes - # are both encoded as a sequence of characters from a set containing - # 64 characters, each one of which represents 6 bits of the salt/hash. - # The encoding is similar to BASE64, the difference being that the - # BASE64 plus sign (+) is replaced with a forward slash (/). - my @saltchars = (0..9, 'A'..'Z', 'a'..'z', '.', '/'); - - # Generate the salt. We use an 8 character (48 bit) salt for maximum - # security on systems whose crypt uses MD5. Systems with older - # versions of crypt will just use the first two characters of the salt. - my $salt = ''; - for ( my $i=0 ; $i < 8 ; ++$i ) { - $salt .= $saltchars[rand(64)]; + my $algorithm; + if (!defined $salt) { + # If you don't use a salt, then people can create tables of + # hashes that map to particular passwords, and then break your + # hashing very easily if they have a large-enough table of common + # (or even uncommon) passwords. So we generate a unique salt for + # each password in the database, and then just prepend it to + # the hash. + $salt = generate_random_password(PASSWORD_SALT_LENGTH); + $algorithm = PASSWORD_DIGEST_ALGORITHM; } + # We append the algorithm used to the string. This is good because then + # we can change the algorithm being used, in the future, without + # disrupting the validation of existing passwords. Also, this tells + # us if a password is using the old "crypt" method of hashing passwords, + # because the algorithm will be missing from the string. + if ($salt =~ /{([^}]+)}$/) { + $algorithm = $1; + } + + my $crypted_password; + if (!$algorithm) { # Wide characters cause crypt to die if (Bugzilla->params->{'utf8'}) { utf8::encode($password) if utf8::is_utf8($password); } # Crypt the password. - my $cryptedpassword = crypt($password, $salt); + $crypted_password = crypt($password, $salt); + + # HACK: Perl has bug where returned crypted password is considered + # tainted. See http://rt.perl.org/rt3/Public/Bug/Display.html?id=59998 + unless(tainted($password) || tainted($salt)) { + trick_taint($crypted_password); + } + } + else { + my $hasher = Digest->new($algorithm); + # We only want to use the first characters of the salt, no + # matter how long of a salt we may have been passed. + $salt = substr($salt, 0, PASSWORD_SALT_LENGTH); + $hasher->add($password, $salt); + $crypted_password = $salt . $hasher->b64digest . "{$algorithm}"; + } # Return the crypted password. - return $cryptedpassword; + return $crypted_password; } sub generate_random_password { @@ -637,7 +670,6 @@ Bugzilla::Util - Generic utility functions for bugzilla use Bugzilla::Util; # Functions for dealing with variable tainting - $rv = is_tainted($var); trick_taint($var); detaint_natural($var); detaint_signed($var); @@ -646,6 +678,7 @@ Bugzilla::Util - Generic utility functions for bugzilla html_quote($var); url_quote($var); xml_quote($var); + email_filter($var); # Functions for decoding $rv = url_decode($var); @@ -663,7 +696,6 @@ Bugzilla::Util - Generic utility functions for bugzilla # Functions for manipulating strings $val = trim(" abc "); - ($removed, $added) = diff_strings($old, $new); $wrapped = wrap_comment($comment); # Functions for formatting time @@ -701,10 +733,6 @@ with care> to avoid security holes. =over 4 -=item C - -Determines whether a particular variable is tainted - =item C Tricks perl into untainting a particular variable. @@ -767,6 +795,12 @@ is kept separate from html_quote partly for compatibility with previous code Converts the %xx encoding from the given URL back to its original form. +=item C + +Removes the hostname from email addresses in the string, if the user +currently viewing Bugzilla is logged out. If the user is logged-in, +this filter just returns the input string. + =back =head2 Environment and Location @@ -842,14 +876,6 @@ If the item is not in the list, returns -1. Removes any leading or trailing whitespace from a string. This routine does not modify the existing string. -=item C - -Takes two strings containing a list of comma- or space-separated items -and returns what items were removed from or added to the new one, -compared to the old one. Returns a list, where the first entry is a scalar -containing removed items, and the second entry is a scalar containing added -items. - =item C Wraps a string, so that a line is I longer than C<$size>. @@ -920,15 +946,13 @@ A string. =item C -Takes a time, converts it to the desired format and appends the timezone -as defined in editparams.cgi, if desired. This routine will be expanded -in the future to adjust for user preferences regarding what timezone to -display times in. +Takes a time and converts it to the desired format and timezone. +If no format is given, the routine guesses the correct one and returns +an empty array if it cannot. If no timezone is given, the user's timezone +is used, as defined in his preferences. This routine is mainly called from templates to filter dates, see -"FILTER time" in Templates.pm. In this case, $format is undefined and -the routine has to "guess" the date format that was passed to $dbh->sql_date_format(). - +"FILTER time" in L. =item C @@ -953,12 +977,14 @@ of the "mtime" parameter of the perl "stat" function. =over 4 -=item C +=item C -Takes a string and returns a Ced value for it, using a random salt. +Takes a string and returns a hashed (encrypted) value for it, using a +random salt. An optional salt string may also be passed in. -Please always use this function instead of the built-in perl "crypt" -when initially encrypting a password. +Please always use this function instead of the built-in perl C +function, when checking or setting a password. Bugzilla does not use +C. =begin undocumented diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm index 0e429241d..504418321 100644 --- a/Bugzilla/WebService.pm +++ b/Bugzilla/WebService.pm @@ -14,21 +14,16 @@ # # Contributor(s): Marc Schumann # Max Kanat-Alexander +# Rosie Clarkson +# +# Portions © Crown copyright 2009 - Rosie Clarkson (development@planningportal.gov.uk) for the Planning Portal +# This is the base class for $self in WebService method calls. For the +# actual RPC server, see Bugzilla::WebService::Server and its subclasses. package Bugzilla::WebService; - use strict; -use Bugzilla::WebService::Constants; -use Bugzilla::Util; use Date::Parse; - -sub fail_unimplemented { - my $this = shift; - - die SOAP::Fault - ->faultcode(ERROR_UNIMPLEMENTED) - ->faultstring('Service Unimplemented'); -} +use XMLRPC::Lite; sub datetime_format { my ($self, $date_string) = @_; @@ -42,32 +37,6 @@ sub datetime_format { return $iso_datetime; } -sub handle_login { - my ($classes, $action, $uri, $method) = @_; - - my $class = $classes->{$uri}; - eval "require $class"; - - return if $class->login_exempt($method); - Bugzilla->login(); - - # Even though we check for the need to redirect in - # Bugzilla->login() we check here again since Bugzilla->login() - # does not know what the current XMLRPC method is. Therefore - # ssl_require_redirect in Bugzilla->login() will have returned - # false if system was configured to redirect for authenticated - # sessions and the user was not yet logged in. - # So here we pass in the method name to ssl_require_redirect so - # it can then check for the extra case where the method equals - # User.login, which we would then need to redirect if not - # over a secure connection. - my $full_method = $uri . "." . $method; - Bugzilla->cgi->require_https(Bugzilla->params->{'sslbase'}) - if ssl_require_redirect($full_method); - - return; -} - # For some methods, we shouldn't call Bugzilla->login before we call them use constant LOGIN_EXEMPT => { }; @@ -77,64 +46,12 @@ sub login_exempt { return $class->LOGIN_EXEMPT->{$method}; } -1; - -package Bugzilla::WebService::XMLRPC::Transport::HTTP::CGI; -use strict; -eval { require XMLRPC::Transport::HTTP; }; -our @ISA = qw(XMLRPC::Transport::HTTP::CGI); - -sub initialize { - my $self = shift; - my %retval = $self->SUPER::initialize(@_); - $retval{'serializer'} = Bugzilla::WebService::XMLRPC::Serializer->new; - return %retval; -} - -sub make_response { - my $self = shift; - - $self->SUPER::make_response(@_); - - # XMLRPC::Transport::HTTP::CGI doesn't know about Bugzilla carrying around - # its cookies in Bugzilla::CGI, so we need to copy them over. - foreach (@{Bugzilla->cgi->{'Bugzilla_cookie_list'}}) { - $self->response->headers->push_header('Set-Cookie', $_); +sub type { + my ($self, $type, $value) = @_; + if ($type eq 'dateTime') { + $value = $self->datetime_format($value); } -} - -1; - -# This package exists to fix a UTF-8 bug in SOAP::Lite. -# See http://rt.cpan.org/Public/Bug/Display.html?id=32952. -package Bugzilla::WebService::XMLRPC::Serializer; -use strict; -# We can't use "use base" because XMLRPC::Serializer doesn't return -# a true value. -eval { require XMLRPC::Lite; }; -our @ISA = qw(XMLRPC::Serializer); - -sub new { - my $class = shift; - my $self = $class->SUPER::new(@_); - # This fixes UTF-8. - $self->{'_typelookup'}->{'base64'} = - [10, sub { !utf8::is_utf8($_[0]) && $_[0] =~ /[^\x09\x0a\x0d\x20-\x7f]/}, - 'as_base64']; - # This makes arrays work right even though we're a subclass. - # (See http://rt.cpan.org//Ticket/Display.html?id=34514) - $self->{'_encodingStyle'} = ''; - return $self; -} - -sub as_string { - my $self = shift; - my ($value) = @_; - # Something weird happens with XML::Parser when we have upper-ASCII - # characters encoded as UTF-8, and this fixes it. - utf8::encode($value) if utf8::is_utf8($value) - && $value =~ /^[\x00-\xff]+$/; - return $self->SUPER::as_string($value); + return XMLRPC::Data->type($type)->value($value); } 1; @@ -285,3 +202,92 @@ an error 302, there won't be an error -302. Sometimes a function will throw an error that doesn't have a specific error code. In this case, the code will be C<-32000> if it's a "fatal" error, and C<32000> if it's a "transient" error. + +=head1 COMMON PARAMETERS + +Many Webservice methods take similar arguments. Instead of re-writing +the documentation for each method, we document the parameters here, once, +and then refer back to this documentation from the individual methods +where these parameters are used. + +=head2 Limiting What Fields Are Returned + +Many WebService methods return an array of structs with various +fields in the structs. (For example, L +returns a list of C that have fields like C, C, +C, etc.) + +These parameters allow you to limit what fields are present in +the structs, to possibly improve performance or save some bandwidth. + +=over + +=item C (array) + +An array of strings, representing the (case-sensitive) names of fields. +Only the fields specified in this hash will be returned, the rest will +not be included. + +If you specify an empty array, then this function will return empty +hashes. + +Invalid field names are ignored. + +Example: + + User.get( ids => [1], include_fields => ['id', 'name'] ) + +would return something like: + + { users => [{ id => 1, name => 'user@domain.com' }] } + +=item C (array) + +An array of strings, representing the (case-sensitive) names of fields. +The fields specified will not be included in the returned hashes. + +If you specify all the fields, then this function will return empty +hashes. + +Invalid field names are ignored. + +Specifying fields here overrides C, so if you specify a +field in both, it will be excluded, not included. + +Example: + + User.get( ids => [1], exclude_fields => ['name'] ) + +would return something like: + + { users => [{ id => 1, real_name => 'John Smith' }] } + +=back + + +=head1 EXTENSIONS TO THE XML-RPC STANDARD + +=head2 Undefined Values + +Normally, XML-RPC does not allow empty values for C, C, or +C fields. Bugzilla does--it treats empty values as +C (called C or C in some programming languages). + +Bugzilla also accepts an element called C<< >>, as specified by +the XML-RPC extension here: L, +which is always considered to be C, no matter what it contains. + +Bugzilla does not use C<< >> values in returned data, because currently +most clients do not support C<< >>. Instead, any fields with C +values will be stripped from the response completely. Therefore +B. + +=begin private + +nil is implemented by XMLRPC::Lite, in XMLRPC::Deserializer::decode_value +in the CPAN SVN since 14th Dec 2008 +L and in Fedora's +perl-SOAP-Lite package in versions 0.68-1 and above. + +=end private diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 5df7c7d19..e456cc930 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -16,17 +16,18 @@ # Max Kanat-Alexander # Mads Bondo Dydensborg # Tsahi Asher +# Noura Elhawary package Bugzilla::WebService::Bug; use strict; use base qw(Bugzilla::WebService); -import SOAP::Data qw(type); use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Field; use Bugzilla::WebService::Constants; +use Bugzilla::WebService::Util qw(filter validate); use Bugzilla::Bug; use Bugzilla::BugMail; use Bugzilla::Util qw(trim); @@ -39,70 +40,203 @@ use Bugzilla::Util qw(trim); # make sense to somebody who's not intimately familiar with the inner workings # of Bugzilla. (These are the field names that the WebService uses.) use constant FIELD_MAP => { - status => 'bug_status', - severity => 'bug_severity', + creation_time => 'creation_ts', description => 'comment', - summary => 'short_desc', + id => 'bug_id', + last_change_time => 'delta_ts', platform => 'rep_platform', + severity => 'bug_severity', + status => 'bug_status', + summary => 'short_desc', + url => 'bug_file_loc', + whiteboard => 'status_whiteboard', + limit => 'LIMIT', + offset => 'OFFSET', }; -use constant GLOBAL_SELECT_FIELDS => qw( - bug_severity - bug_status - op_sys - priority - rep_platform - resolution -); - use constant PRODUCT_SPECIFIC_FIELDS => qw(version target_milestone component); ###################################################### # Add aliases here for old method name compatibility # ###################################################### -BEGIN { *get_bugs = \&get } +BEGIN { + # In 3.0, get was called get_bugs + *get_bugs = \&get; + # Before 3.4rc1, "history" was get_history. + *get_history = \&history; +} ########### # Methods # ########### +sub comments { + my ($self, $params) = validate(@_, 'ids', 'comment_ids'); + + if (!(defined $params->{ids} || defined $params->{comment_ids})) { + ThrowCodeError('params_required', + { function => 'Bug.comments', + params => ['ids', 'comment_ids'] }); + } + + my $bug_ids = $params->{ids} || []; + my $comment_ids = $params->{comment_ids} || []; + + my $dbh = Bugzilla->dbh; + my $user = Bugzilla->user; + + my %bugs; + foreach my $bug_id (@$bug_ids) { + my $bug = Bugzilla::Bug->check($bug_id); + # We want the API to always return comments in the same order. + my $comments = Bugzilla::Bug::GetComments( + $bug->id, 'oldest_to_newest', $params->{new_since}); + my @result; + foreach my $comment (@$comments) { + next if $comment->{isprivate} && !$user->is_insider; + $comment->{bug_id} = $bug->id; + push(@result, $self->_translate_comment($comment, $params)); + } + $bugs{$bug->id}{'comments'} = \@result; + } + + my %comments; + if (scalar @$comment_ids) { + my @ids = map { trim($_) } @$comment_ids; + my @sql_ids = map { $dbh->quote($_) } @ids; + my $comment_data = $dbh->selectall_arrayref( + 'SELECT comment_id AS id, bug_id, who, bug_when AS time, + isprivate, thetext AS body, type, extra_data + FROM longdescs WHERE ' . $dbh->sql_in('comment_id', \@sql_ids), + {Slice=>{}}); + + # See if we were passed any invalid comment ids. + my %got_ids = map { $_->{id} => 1 } @$comment_data; + foreach my $comment_id (@ids) { + if (!$got_ids{$comment_id}) { + ThrowUserError('comment_id_invalid', { id => $comment_id }); + } + } + + # Now make sure that we can see all the associated bugs. + my %got_bug_ids = map { $_->{bug_id} => 1 } @$comment_data; + Bugzilla::Bug->check($_) foreach (keys %got_bug_ids); + + foreach my $comment (@$comment_data) { + if ($comment->{isprivate} && !$user->is_insider) { + ThrowUserError('comment_is_private', { id => $comment->{id} }); + } + $comment->{author} = new Bugzilla::User($comment->{who}); + $comment->{body} = Bugzilla::Bug::format_comment($comment); + $comments{$comment->{id}} = + $self->_translate_comment($comment, $params); + } + } + + return { bugs => \%bugs, comments => \%comments }; +} + +# Helper for Bug.comments +sub _translate_comment { + my ($self, $comment, $filters) = @_; + return filter $filters, { + id => $self->type('int', $comment->{id}), + bug_id => $self->type('int', $comment->{bug_id}), + author => $self->type('string', $comment->{author}->login), + time => $self->type('dateTime', $comment->{'time'}), + is_private => $self->type('boolean', $comment->{isprivate}), + text => $self->type('string', $comment->{body}), + }; +} + sub get { - my ($self, $params) = @_; + my ($self, $params) = validate(@_, 'ids'); + + my $ids = $params->{ids}; + defined $ids || ThrowCodeError('param_required', { param => 'ids' }); + + my @bugs; + my @faults; + foreach my $bug_id (@$ids) { + my $bug; + if ($params->{permissive}) { + eval { $bug = Bugzilla::Bug->check($bug_id); }; + if ($@) { + push(@faults, {id => $bug_id, + faultString => $@->faultstring, + faultCode => $@->faultcode, + } + ); + undef $@; + next; + } + } + else { + $bug = Bugzilla::Bug->check($bug_id); + } + push(@bugs, $self->_bug_to_hash($bug)); + } + + return { bugs => \@bugs, faults => \@faults }; +} + +# this is a function that gets bug activity for list of bug ids +# it can be called as the following: +# $call = $rpc->call( 'Bug.history', { ids => [1,2] }); +sub history { + my ($self, $params) = validate(@_, 'ids'); + my $ids = $params->{ids}; defined $ids || ThrowCodeError('param_required', { param => 'ids' }); my @return; + foreach my $bug_id (@$ids) { - ValidateBugID($bug_id); - my $bug = new Bugzilla::Bug($bug_id); - - # Timetracking fields are deleted if the user doesn't belong to - # the corresponding group. - unless (Bugzilla->user->in_group(Bugzilla->params->{'timetrackinggroup'})) { - delete $bug->{'estimated_time'}; - delete $bug->{'remaining_time'}; - delete $bug->{'deadline'}; - } - # This is done in this fashion in order to produce a stable API. - # The internals of Bugzilla::Bug are not stable enough to just - # return them directly. - my $creation_ts = $self->datetime_format($bug->creation_ts); - my $delta_ts = $self->datetime_format($bug->delta_ts); my %item; - $item{'creation_time'} = type('dateTime')->value($creation_ts); - $item{'last_change_time'} = type('dateTime')->value($delta_ts); - $item{'internals'} = $bug; - $item{'id'} = type('int')->value($bug->bug_id); - $item{'summary'} = type('string')->value($bug->short_desc); + my $bug = Bugzilla::Bug->check($bug_id); + $bug_id = $bug->id; + $item{id} = $self->type('int', $bug_id); + my ($activity) = Bugzilla::Bug::GetBugActivity($bug_id); + + my @history; + foreach my $changeset (@$activity) { + my %bug_history; + $bug_history{when} = $self->type('dateTime', + $self->datetime_format($changeset->{when})); + $bug_history{who} = $self->type('string', $changeset->{who}); + $bug_history{changes} = []; + foreach my $change (@{ $changeset->{changes} }) { + my $attach_id = delete $change->{attachid}; + if ($attach_id) { + $change->{attachment_id} = $self->type('int', $attach_id); + } + $change->{removed} = $self->type('string', $change->{removed}); + $change->{added} = $self->type('string', $change->{added}); + $change->{field_name} = $self->type('string', + delete $change->{fieldname}); + # This is going to go away in the future from GetBugActivity + # so we shouldn't put it in the API. + delete $change->{field}; + push (@{$bug_history{changes}}, $change); + } + + push (@history, \%bug_history); + } + + $item{history} = \@history; + + # alias is returned in case users passes a mixture of ids and aliases + # then they get to know which bug activity relates to which value + # they passed if (Bugzilla->params->{'usebugaliases'}) { - $item{'alias'} = type('string')->value($bug->alias); + $item{alias} = $self->type('string', $bug->alias); } else { # For API reasons, we always want the value to appear, we just # don't want it to have a value if aliases are turned off. - $item{'alias'} = undef; + $item{alias} = undef; } push(@return, \%item); @@ -111,39 +245,70 @@ sub get { return { bugs => \@return }; } +sub search { + my ($self, $params) = @_; + + if ( defined($params->{offset}) and !defined($params->{limit}) ) { + ThrowCodeError('param_required', + { param => 'limit', function => 'Bug.search()' }); + } + + $params = _map_fields($params); + + # Do special search types for certain fields. + if ( my $bug_when = delete $params->{delta_ts} ) { + $params->{WHERE}->{'delta_ts >= ?'} = $bug_when; + } + if (my $when = delete $params->{creation_ts}) { + $params->{WHERE}->{'creation_ts >= ?'} = $when; + } + if (my $votes = delete $params->{votes}) { + $params->{WHERE}->{'votes >= ?'} = $votes; + } + if (my $summary = delete $params->{short_desc}) { + my @strings = ref $summary ? @$summary : ($summary); + my @likes = ("short_desc LIKE ?") x @strings; + my $clause = join(' OR ', @likes); + $params->{WHERE}->{"($clause)"} = [map { "\%$_\%" } @strings]; + } + if (my $whiteboard = delete $params->{status_whiteboard}) { + my @strings = ref $whiteboard ? @$whiteboard : ($whiteboard); + my @likes = ("status_whiteboard LIKE ?") x @strings; + my $clause = join(' OR ', @likes); + $params->{WHERE}->{"($clause)"} = [map { "\%$_\%" } @strings]; + } + + my $bugs = Bugzilla::Bug->match($params); + my $visible = Bugzilla->user->visible_bugs($bugs); + my @hashes = map { $self->_bug_to_hash($_) } @$visible; + return { bugs => \@hashes }; +} sub create { my ($self, $params) = @_; Bugzilla->login(LOGIN_REQUIRED); - my %field_values; - foreach my $field (keys %$params) { - my $field_name = FIELD_MAP->{$field} || $field; - $field_values{$field_name} = $params->{$field}; - } - + $params = _map_fields($params); # WebService users can't set the creation date of a bug. - delete $field_values{'creation_ts'}; + delete $params->{'creation_ts'}; - my $bug = Bugzilla::Bug->create(\%field_values); + my $bug = Bugzilla::Bug->create($params); Bugzilla::BugMail::Send($bug->bug_id, { changer => $bug->reporter->login }); - return { id => type('int')->value($bug->bug_id) }; + return { id => $self->type('int', $bug->bug_id) }; } sub legal_values { my ($self, $params) = @_; my $field = FIELD_MAP->{$params->{field}} || $params->{field}; - my @custom_select = Bugzilla->get_fields( - {custom => 1, type => [FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT]}); - # We only want field names. - @custom_select = map {$_->name} @custom_select; + my @global_selects = Bugzilla->get_fields( + {type => [FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT]}); my $values; - if (grep($_ eq $field, GLOBAL_SELECT_FIELDS, @custom_select)) { + if (grep($_->name eq $field, @global_selects)) { $values = get_legal_field_values($field); } elsif (grep($_ eq $field, PRODUCT_SPECIFIC_FIELDS)) { @@ -173,7 +338,7 @@ sub legal_values { my @result; foreach my $val (@$values) { - push(@result, type('string')->value($val)); + push(@result, $self->type('string', $val)); } return { values => \@result }; @@ -188,13 +353,11 @@ sub add_comment { # Check parameters defined $params->{id} || ThrowCodeError('param_required', { param => 'id' }); - ValidateBugID($params->{id}); - my $comment = $params->{comment}; (defined $comment && trim($comment) ne '') || ThrowCodeError('param_required', { param => 'comment' }); - my $bug = new Bugzilla::Bug($params->{id}); + my $bug = Bugzilla::Bug->check($params->{id}); Bugzilla->user->can_edit_product($bug->product_id) || ThrowUserError("product_edit_denied", {product => $bug->product}); @@ -202,11 +365,139 @@ sub add_comment { # Append comment $bug->add_comment($comment, { isprivate => $params->{private}, work_time => $params->{work_time} }); + + # Capture the call to bug->update (which creates the new comment) in + # a transaction so we're sure to get the correct comment_id. + + my $dbh = Bugzilla->dbh; + $dbh->bz_start_transaction(); + $bug->update(); + my $new_comment_id = $dbh->bz_last_key('longdescs', 'comment_id'); + + $dbh->bz_commit_transaction(); + # Send mail. Bugzilla::BugMail::Send($bug->bug_id, { changer => Bugzilla->user->login }); - return undef; + + return { id => $self->type('int', $new_comment_id) }; +} + +sub update_see_also { + my ($self, $params) = @_; + + my $user = Bugzilla->login(LOGIN_REQUIRED); + + # Check parameters + $params->{ids} + || ThrowCodeError('param_required', { param => 'id' }); + my ($add, $remove) = @$params{qw(add remove)}; + ($add || $remove) + or ThrowCodeError('params_required', { params => ['add', 'remove'] }); + + my @bugs; + foreach my $id (@{ $params->{ids} }) { + my $bug = Bugzilla::Bug->check($id); + $user->can_edit_product($bug->product_id) + || ThrowUserError("product_edit_denied", + { product => $bug->product }); + push(@bugs, $bug); + if ($remove) { + $bug->remove_see_also($_) foreach @$remove; + } + if ($add) { + $bug->add_see_also($_) foreach @$add; + } + } + + my %changes; + foreach my $bug (@bugs) { + my $change = $bug->update(); + if (my $see_also = $change->{see_also}) { + $changes{$bug->id}->{see_also} = { + removed => [split(', ', $see_also->[0])], + added => [split(', ', $see_also->[1])], + }; + } + else { + # We still want a changes entry, for API consistency. + $changes{$bug->id}->{see_also} = { added => [], removed => [] }; + } + + Bugzilla::BugMail::Send($bug->id, { changer => $user->login }); + } + + return { changes => \%changes }; +} + +############################## +# Private Helper Subroutines # +############################## + +# A helper for get() and search(). +sub _bug_to_hash { + my ($self, $bug) = @_; + + # Timetracking fields are deleted if the user doesn't belong to + # the corresponding group. + unless (Bugzilla->user->is_timetracker) { + delete $bug->{'estimated_time'}; + delete $bug->{'remaining_time'}; + delete $bug->{'deadline'}; + } + + # This is done in this fashion in order to produce a stable API. + # The internals of Bugzilla::Bug are not stable enough to just + # return them directly. + my %item; + $item{'internals'} = $bug; + $item{'creation_time'} = $self->type('dateTime', $bug->creation_ts); + $item{'last_change_time'} = $self->type('dateTime', $bug->delta_ts); + $item{'id'} = $self->type('int', $bug->bug_id); + $item{'summary'} = $self->type('string', $bug->short_desc); + $item{'assigned_to'} = $self->type('string', $bug->assigned_to->login); + $item{'resolution'} = $self->type('string', $bug->resolution); + $item{'status'} = $self->type('string', $bug->bug_status); + $item{'is_open'} = $self->type('boolean', $bug->status->is_open); + $item{'severity'} = $self->type('string', $bug->bug_severity); + $item{'priority'} = $self->type('string', $bug->priority); + $item{'product'} = $self->type('string', $bug->product); + $item{'component'} = $self->type('string', $bug->component); + $item{'dupe_of'} = $self->type('int', $bug->dup_id); + + # if we do not delete this key, additional user info, including their + # real name, etc, will wind up in the 'internals' hashref + delete $item{internals}->{assigned_to_obj}; + + if (Bugzilla->params->{'usebugaliases'}) { + $item{'alias'} = $self->type('string', $bug->alias); + } + else { + # For API reasons, we always want the value to appear, we just + # don't want it to have a value if aliases are turned off. + $item{'alias'} = undef; + } + + return \%item; +} + +# Convert WebService API field names to internal DB field names. +# Used by create() and search(). +sub _map_fields { + my ($params) = @_; + + my %field_values; + foreach my $field (keys %$params) { + my $field_name = FIELD_MAP->{$field} || $field; + $field_values{$field_name} = $params->{$field}; + } + + unless (Bugzilla->user->is_timetracker) { + delete @field_values{qw(estimated_time remaining_time deadline)}; + } + + return \%field_values; } 1; @@ -283,6 +574,130 @@ You specified a field that doesn't exist or isn't a drop-down field. =over + +=item C + +B + +=over + +=item B + +This allows you to get data about comments, given a list of bugs +and/or comment ids. + +=item B + +B: At least one of C or C is required. + +In addition to the parameters below, this method also accepts the +standard L and +L arguments. + +=over + +=item C + +C An array that can contain both bug IDs and bug aliases. +All of the comments (that are visible to you) will be returned for the +specified bugs. + +=item C + +C An array of integer comment_ids. These comments will be +returned individually, separate from any other comments in their +respective bugs. + +=item C + +C If specified, the method will only return comments I +than this time. This only affects comments returned from the C +argument. You will always be returned all comments you request in the +C argument, even if they are older than this date. + +=back + +=item B + +Two items are returned: + +=over + +=item C + +This is used for bugs specified in C. This is a hash, +where the keys are the numeric ids of the bugs, and the value is +a hash with a single key, C, which is an array of comments. +(The format of comments is described below.) + +Note that any individual bug will only be returned once, so if you +specify an id multiple times in C, it will still only be +returned once. + +=item C + +Each individual comment requested in C is returned here, +in a hash where the numeric comment id is the key, and the value +is the comment. (The format of comments is described below.) + +=back + +A "comment" as described above is a hash that contains the following +keys: + +=over + +=item id + +C The globally unique ID for the comment. + +=item bug_id + +C The ID of the bug that this comment is on. + +=item text + +C The actual text of the comment. + +=item author + +C The login name of the comment's author. + +=item time + +C The time (in Bugzilla's timezone) that the comment was added. + +=item is_private + +C True if this comment is private (only visible to a certain +group called the "insidergroup"), False otherwise. + +=back + +=item B + +This method can throw all the same errors as L. In addition, +it can also throw the following errors: + +=over + +=item 110 (Comment Is Private) + +You specified the id of a private comment in the C +argument, and you are not in the "insider group" that can see +private comments. + +=item 111 (Invalid Comment ID) + +You specified an id in the C argument that is invalid--either +you specified something that wasn't a number, or there is no comment with +that id. + +=back + +=back + + =item C B @@ -312,12 +727,99 @@ Note that it's possible for aliases to be disabled in Bugzilla, in which case you will be told that you have specified an invalid bug_id if you try to specify an alias. (It will be error 100.) +=item C B + +C Normally, if you request any inaccessible or invalid bug ids, +Bug.get will throw an error. If this parameter is True, instead of throwing an +error we return an array of hashes with a C, C and C +for each bug that fails, and return normal information for the other bugs that +were accessible. + =back =item B -A hash containing a single element, C. This is an array of hashes. -Each hash contains the following items: +Two items are returned: + +=over + +=item C + +An array of hashes that contains information about the bugs with +the valid ids. Each hash contains the following items: + +=over + +=item alias + +C The alias of this bug. If there is no alias or aliases are +disabled in this Bugzilla, this will be an empty string. + +=item assigned_to + +C The login name of the user to whom the bug is assigned. + +=item component + +C The name of the current component of this bug. + +=item creation_time + +C When the bug was created. + +=item dupe_of + +C The bug ID of the bug that this bug is a duplicate of. If this bug +isn't a duplicate of any bug, this will be an empty int. + +=item id + +C The numeric bug_id of this bug. + +=item internals B + +A hash. The internals of a L object. This is extremely +unstable, and you should only rely on this if you absolutely have to. The +structure of the hash may even change between point releases of Bugzilla. + +=item is_open + +C Returns true (1) if this bug is open, false (0) if it is closed. + +=item last_change_time + +C When the bug was last changed. + +=item priority + +C The priority of the bug. + +=item product + +C The name of the product this bug is in. + +=item resolution + +C The current resolution of the bug, or an empty string if the bug is open. + +=item severity + +C The current severity of the bug. + +=item status + +C The current status of the bug. + +=item summary + +C The summary of this bug. + +=back + +=item C B + +An array of hashes that contains invalid bug ids with error messages +returned for them. Each hash contains the following items: =over @@ -325,28 +827,19 @@ Each hash contains the following items: C The numeric bug_id of this bug. -=item alias +=item faultString -C The alias of this bug. If there is no alias or aliases are -disabled in this Bugzilla, this will be an empty string. +c This will only be returned for invalid bugs if the C +argument was set when calling Bug.get, and it is an error indicating that +the bug id was invalid. -=item summary +=item faultCode -C The summary of this bug. +c This will only be returned for invalid bugs if the C +argument was set when calling Bug.get, and it is the error code for the +invalid bug error. -=item creation_time - -C When the bug was created. - -=item last_change_time - -C When the bug was last changed. - -=item internals B - -A hash. The internals of a L object. This is extremely -unstable, and you should only rely on this if you absolutely have to. The -structure of the hash may even change between point releases of Bugzilla. +=back =back @@ -369,16 +862,353 @@ You do not have access to the bug_id you specified. =back +=item B + +=over + +=item C argument added to this method's params in Bugzilla B<3.4>. + +=item The following properties were added to this method's return values +in Bugzilla B<3.4>: + +=over + +=item For C + +=over + +=item assigned_to + +=item component + +=item dupe_of + +=item is_open + +=item priority + +=item product + +=item resolution + +=item severity + +=item status + +=back + +=item C + +=back + +=back + + +=back + +=item C + +B + +=over + +=item B + +Gets the history of changes for particular bugs in the database. + +=item B + +=over + +=item C + +An array of numbers and strings. + +If an element in the array is entirely numeric, it represents a bug_id +from the Bugzilla database to fetch. If it contains any non-numeric +characters, it is considered to be a bug alias instead, and the data bug +with that alias will be loaded. + +Note that it's possible for aliases to be disabled in Bugzilla, in which +case you will be told that you have specified an invalid bug_id if you +try to specify an alias. (It will be error 100.) + +=back + +=item B + +A hash containing a single element, C. This is an array of hashes, +containing the following keys: + +=over + +=item id + +C The numeric id of the bug. + +=item alias + +C The alias of this bug. If there is no alias or aliases are +disabled in this Bugzilla, this will be undef. + +=item history + +C An array of hashes, each hash having the following keys: + +=over + +=item when + +C The date the bug activity/change happened. + +=item who + +C The login name of the user who performed the bug change. + +=item changes + +C An array of hashes which contain all the changes that happened +to the bug at this time (as specified by C). Each hash contains +the following items: + +=over + +=item field_name + +C The name of the bug field that has changed. + +=item removed + +C The previous value of the bug field which has been deleted +by the change. + +=item added + +C The new value of the bug field which has been added by the change. + +=item attachment_id + +C The id of the attachment that was changed. This only appears if +the change was to an attachment, otherwise C will not be +present in this hash. + +=back + =back =back +=item B + +The same as L. + +=item B + +=over + +=item Added in Bugzilla B<3.4>. + +=back + +=back + + +=item C + +B + +=over + +=item B + +Allows you to search for bugs based on particular criteria. + +=item B + +Unless otherwise specified in the description of a parameter, bugs are +returned if they match I the criteria you specify in these +parameters. That is, we don't match against substrings--if a bug is in +the "Widgets" product and you ask for bugs in the "Widg" product, you +won't get anything. + +Criteria are joined in a logical AND. That is, you will be returned +bugs that match I of the criteria, not bugs that match I of +the criteria. + +Each parameter can be either the type it says, or an array of the types +it says. If you pass an array, it means "Give me bugs with I of +these values." For example, if you wanted bugs that were in either +the "Foo" or "Bar" products, you'd pass: + + product => ['Foo', 'Bar'] + +Some Bugzillas may treat your arguments case-sensitively, depending +on what database system they are using. Most commonly, though, Bugzilla is +not case-sensitive with the arguments passed (because MySQL is the +most-common database to use with Bugzilla, and MySQL is not case sensitive). + +=over + +=item C + +C The unique alias for this bug. Note that you can search +by alias even if the alias field is disabled in this Bugzilla, but +it's likely that there won't be any aliases set on bugs, in that case. + +=item C + +C The login name of a user that a bug is assigned to. + +=item C + +C The name of the Component that the bug is in. Note that +if there are multiple Compoonents with the same name, and you search +for that name, bugs in I those Components will be returned. If you +don't want this, be sure to also specify the C argument. + +=item C + +C Searches for bugs that were created at this time or later. +May not be an array. + +=item C + +C The numeric id of the bug. + +=item C + +C Searches for bugs that were modified at this time or later. +May not be an array. + +=item C + +C Limit the number of results returned to C records. + +=item C + +C Used in conjunction with the C argument, C defines +the starting position for the search. For example, given a search that +would return 100 bugs, setting C to 10 and C to 10 would return +bugs 11 through 20 from the set of 100. + +=item C + +C The "Operating System" field of a bug. + +=item C + +C The Platform (sometimes called "Hardware") field of a bug. + +=item C + +C The Priority field on a bug. + +=item C + +C The name of the Product that the bug is in. + +=item C + +C The login name of the user who reported the bug. + +=item C + +C The current resolution--only set if a bug is closed. You can +find open bugs by searching for bugs with an empty resolution. + +=item C + +C The Severity field on a bug. + +=item C + +C The current status of a bug (not including its resolution, +if it has one, which is a separate field above). + +=item C + +C Searches for substrings in the single-line Summary field on +bugs. If you specify an array, then bugs whose summaries match I of the +passed substrings will be returned. + +Note that unlike searching in the Bugzilla UI, substrings are not split +on spaces. So searching for C will match "This is a foo bar" +but not "This foo is a bar". C<['foo', 'bar']>, would, however, match +the second item. + +=item C + +C The Target Milestone field of a bug. Note that even if this +Bugzilla does not have the Target Milestone field enabled, you can +still search for bugs by Target Milestone. However, it is likely that +in that case, most bugs will not have a Target Milestone set (it +defaults to "---" when the field isn't enabled). + +=item C + +C The login name of the bug's QA Contact. Note that even if +this Bugzilla does not have the QA Contact field enabled, you can +still search for bugs by QA Contact (though it is likely that no bug +will have a QA Contact set, if the field is disabled). + +=item C + +C The "URL" field of a bug. + +=item C + +C The Version field of a bug. + +=item C + +C Searches for bugs with this many votes or greater. May not +be an array. + +=item C + +C Search the "Status Whiteboard" field on bugs for a substring. +Works the same as the C field described above, but searches the +Status Whiteboard field. + +=back + +=item B + +The same as L. + +Note that you will only be returned information about bugs that you +can see. Bugs that you can't see will be entirely excluded from the +results. So, if you want to see private bugs, you will have to first +log in and I call this method. + +=item B + +Currently, this function doesn't throw any special errors (other than +the ones that all webservice functions can throw). If you specify +an invalid value for a particular field, you just won't get any results +for that value. + +=item B + +=over + +=item Added in Bugzilla B<3.4>. + +=back + +=back + + +=back =head2 Bug Creation and Modification =over -=item C B + +=item C + +B =over @@ -550,6 +1380,10 @@ be ignored. =back +=item B + +A hash with one element, C whose value is the id of the newly-created comment. + =item B =over @@ -567,6 +1401,10 @@ The id you specified doesn't exist in the database. You did not have the necessary rights to edit the bug. +=item 113 (Can't Make Private Comments) + +You tried to add a private comment, but don't have the necessary rights. + =back =item B @@ -575,6 +1413,117 @@ You did not have the necessary rights to edit the bug. =item Added in Bugzilla B<3.2>. +=item Modified to return the new comment's id in Bugzilla B<3.4> + +=item Modified to throw an error if you try to add a private comment +but can't, in Bugzilla B<3.4>. + +=back + +=back + + +=item C + +B + +=over + +=item B + +Adds or removes URLs for the "See Also" field on bugs. These URLs must +point to some valid bug in some Bugzilla installation or in Launchpad. + +=item B + +=over + +=item C + +Array of Cs or Cs. The ids or aliases of bugs that you want +to modify. + +=item C + +Array of Cs. URLs to Bugzilla bugs. These URLs will be added to +the See Also field. They must be valid URLs to C in a +Bugzilla installation or to a bug filed at launchpad.net. + +If the URLs don't start with C or C, it will be assumed +that C should be added to the beginning of the string. + +It is safe to specify URLs that are already in the "See Also" field on +a bug--they will just be silently ignored. + +=item C + +Array of Cs. These URLs will be removed from the See Also field. +You must specify the full URL that you want removed. However, matching +is done case-insensitively, so you don't have to specify the URL in +exact case, if you don't want to. + +If you specify a URL that is not in the See Also field of a particular bug, +it will just be silently ignored. Invaild URLs are currently silently ignored, +though this may change in some future version of Bugzilla. + +=back + +NOTE: If you specify the same URL in both C and C, it will +be I. (That is, C overrides C.) + +=item B + +C, a hash where the keys are numeric bug ids and the contents +are a hash with one key, C. C points to a hash, which +contains two keys, C and C. These are arrays of strings, +representing the actual changes that were made to the bug. + +Here's a diagram of what the return value looks like for updating +bug ids 1 and 2: + + { + changes => { + 1 => { + see_also => { + added => (an array of bug URLs), + removed => (an array of bug URLs), + } + }, + 2 => { + see_also => { + added => (an array of bug URLs), + removed => (an array of bug URLs), + } + } + } + } + +This return value allows you to tell what this method actually did. It is in +this format to be compatible with the return value of a future C +method. + +=item B + +This method can throw all of the errors that L throws, plus: + +=over + +=item 108 (Bug Edit Denied) + +You did not have the necessary rights to edit the bug. + +=item 112 (Invalid Bug URL) + +One of the URLs you provided did not look like a valid bug URL. + +=back + +=item B + +=over + +=item Added in Bugzilla B<3.4>. + =back =back diff --git a/Bugzilla/WebService/Bugzilla.pm b/Bugzilla/WebService/Bugzilla.pm index 53e0d7d79..b1ab8f34f 100644 --- a/Bugzilla/WebService/Bugzilla.pm +++ b/Bugzilla/WebService/Bugzilla.pm @@ -22,9 +22,8 @@ use strict; use base qw(Bugzilla::WebService); use Bugzilla::Constants; use Bugzilla::Hook; -import SOAP::Data qw(type); -use Time::Zone; +use DateTime; # Basic info that is needed before logins use constant LOGIN_EXEMPT => { @@ -33,26 +32,52 @@ use constant LOGIN_EXEMPT => { }; sub version { - return { version => type('string')->value(BUGZILLA_VERSION) }; + my $self = shift; + return { version => $self->type('string', BUGZILLA_VERSION) }; } sub extensions { + my $self = shift; my $extensions = Bugzilla::Hook::enabled_plugins(); foreach my $name (keys %$extensions) { my $info = $extensions->{$name}; - foreach my $data (keys %$info) - { - $extensions->{$name}->{$data} = type('string')->value($info->{$data}); + foreach my $data (keys %$info) { + $extensions->{$name}->{$data} = + $self->type('string', $info->{$data}); } } return { extensions => $extensions }; } sub timezone { - my $offset = tz_offset(); + my $self = shift; + my $offset = Bugzilla->local_timezone->offset_for_datetime(DateTime->now()); $offset = (($offset / 60) / 60) * 100; $offset = sprintf('%+05d', $offset); - return { timezone => type('string')->value($offset) }; + return { timezone => $self->type('string', $offset) }; +} + +sub time { + my ($self) = @_; + my $dbh = Bugzilla->dbh; + + my $db_time = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); + my $now_utc = DateTime->now(); + + my $tz = Bugzilla->local_timezone; + my $now_local = $now_utc->clone->set_time_zone($tz); + my $tz_offset = $tz->offset_for_datetime($now_local); + + return { + db_time => $self->type('dateTime', $db_time), + web_time => $self->type('dateTime', $now_local), + web_time_utc => $self->type('dateTime', $now_utc), + tz_name => $self->type('string', $tz->name), + tz_offset => $self->type('string', + $tz->offset_as_string($tz_offset)), + tz_short_name => $self->type('string', + $now_local->time_zone_short_name), + }; } 1; @@ -127,7 +152,8 @@ extension =item C -B +B This method may be removed in a future version of Bugzilla. +Use L instead. =over @@ -146,4 +172,80 @@ string in (+/-)XXXX (RFC 2822) format. =back + +=item C