diff options
| -rw-r--r-- | Bugzilla/Bug.pm | 14 | ||||
| -rw-r--r-- | Bugzilla/BugMail.pm | 81 | ||||
| -rw-r--r-- | Bugzilla/Job/BugMail.pm | 31 | ||||
| -rw-r--r-- | Bugzilla/JobQueue.pm | 1 | ||||
| -rw-r--r-- | Bugzilla/Object.pm | 115 | ||||
| -rw-r--r-- | extensions/BMO/lib/Reports/EmailQueue.pm | 25 |
6 files changed, 200 insertions, 67 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 6f3107d5c..0b17a02e6 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -313,21 +313,11 @@ use constant EXTRA_REQUIRED_FIELDS => qw(creation_ts target_milestone cc qa_cont ##################################################################### -# This and "new" catch every single way of creating a bug, so that we -# can call _create_cf_accessors. -sub _do_list_select { - my $invocant = shift; - $invocant->_create_cf_accessors(); - return $invocant->SUPER::_do_list_select(@_); -} - sub new { my $invocant = shift; my $class = ref($invocant) || $invocant; my $param = shift; - $class->_create_cf_accessors(); - # Remove leading "#" mark if we've just been passed an id. if (!ref $param && $param =~ /^#(\d+)$/) { $param = $1; @@ -376,6 +366,10 @@ sub new { return $self; } +sub initialize { + $_[0]->_create_cf_accessors(); +} + sub cache_key { my $class = shift; my $key = $class->SUPER::cache_key(@_) diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 3d3bd2dbf..a96989056 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -49,6 +49,7 @@ use Date::Format; use Scalar::Util qw(blessed); use List::MoreUtils qw(uniq firstidx); use Sys::Hostname; +use Storable qw(dclone); use constant BIT_DIRECT => 1; use constant BIT_WATCHING => 2; @@ -418,30 +419,76 @@ sub sendMail { $bugmailtype = "dep_changed" if $dep_only; my $vars = { - date => $date, - to_user => $user, - bug => $bug, - reasons => \@reasons, - reasons_watch => \@reasons_watch, - reasonsheader => join(" ", @headerrel), + date => $date, + to_user => $user, + bug => $bug, + reasons => \@reasons, + reasons_watch => \@reasons_watch, + reasonsheader => join(" ", @headerrel), reasonswatchheader => join(" ", @watchingrel), - changer => $changer, - diffs => \@display_diffs, - changedfields => \@changedfields, - changedfieldnames => \@changedfieldnames, - new_comments => \@send_comments, - threadingmarker => build_thread_marker($bug->id, $user->id, !$bug->lastdiffed), - referenced_bugs => $referenced_bugs, - bugmailtype => $bugmailtype + changer => $changer, + diffs => \@display_diffs, + changedfields => \@changedfields, + changedfieldnames => \@changedfieldnames, + new_comments => \@send_comments, + threadingmarker => build_thread_marker($bug->id, $user->id, !$bug->lastdiffed), + referenced_bugs => $referenced_bugs, + bugmailtype => $bugmailtype, }; - my $msg = _generate_bugmail($user, $vars); - MessageToMTA($msg); + + if (Bugzilla->params->{'use_mailer_queue'}) { + enqueue($vars); + } else { + MessageToMTA(_generate_bugmail($vars)); + } return 1; } +sub enqueue { + my ($vars) = @_; + # we need to flatten all objects to a hash before pushing to the job queue. + # the hashes need to be inflated in the dequeue method. + $vars->{bug} = _flatten_object($vars->{bug}); + $vars->{to_user} = $vars->{to_user}->flatten_to_hash; + $vars->{changer} = _flatten_object($vars->{changer}); + $vars->{new_comments} = [ map { _flatten_object($_) } @{ $vars->{new_comments} } ]; + foreach my $diff (@{ $vars->{diffs} }) { + $diff->{who} = _flatten_object($diff->{who}); + } + Bugzilla->job_queue->insert('bug_mail', { vars => $vars }); +} + +sub dequeue { + my ($payload) = @_; + # clone the payload so we can modify it without impacting TheSchwartz's + # ability to process the job when we've finished + my $vars = dclone($payload); + # inflate objects + $vars->{bug} = Bugzilla::Bug->new_from_hash($vars->{bug}); + $vars->{to_user} = Bugzilla::User->new_from_hash($vars->{to_user}); + $vars->{changer} = Bugzilla::User->new_from_hash($vars->{changer}); + $vars->{new_comments} = [ map { Bugzilla::Comment->new_from_hash($_) } @{ $vars->{new_comments} } ]; + foreach my $diff (@{ $vars->{diffs} }) { + $diff->{who} = Bugzilla::User->new_from_hash($diff->{who}); + } + # generate bugmail and send + MessageToMTA(_generate_bugmail($vars), 1); +} + +sub _flatten_object { + my ($object) = @_; + # nothing to do if it's already flattened + return $object unless blessed($object); + # the same objects are used for each recipient, so cache the flattened hash + my $cache = Bugzilla->request_cache->{bugmail_flat_objects} ||= {}; + my $key = blessed($object) . '-' . $object->id; + return $cache->{$key} ||= $object->flatten_to_hash; +} + sub _generate_bugmail { - my ($user, $vars) = @_; + my ($vars) = @_; + my $user = $vars->{to_user}; my $template = Bugzilla->template_inner($user->setting('lang')); my ($msg_text, $msg_html, $msg_header); diff --git a/Bugzilla/Job/BugMail.pm b/Bugzilla/Job/BugMail.pm new file mode 100644 index 000000000..9c176b005 --- /dev/null +++ b/Bugzilla/Job/BugMail.pm @@ -0,0 +1,31 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Job::BugMail; + +use 5.10.1; +use strict; + +use Bugzilla::BugMail; +BEGIN { eval "use parent qw(Bugzilla::Job::Mailer)"; } + +sub work { + my ($class, $job) = @_; + my $success = eval { + Bugzilla::BugMail::dequeue($job->arg->{vars}); + 1; + }; + if (!$success) { + $job->failed($@); + undef $@; + } + else { + $job->completed; + } +} + +1; diff --git a/Bugzilla/JobQueue.pm b/Bugzilla/JobQueue.pm index 053928dd0..a94598b87 100644 --- a/Bugzilla/JobQueue.pm +++ b/Bugzilla/JobQueue.pm @@ -35,6 +35,7 @@ use fields qw(_worker_pidfile); # If you add new types of jobs, you should add a mapping here. use constant JOB_MAP => { send_mail => 'Bugzilla::Job::Mailer', + bug_mail => 'Bugzilla::Job::BugMail', }; # Without a driver cache TheSchwartz opens a new database connection diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 34134a69f..b1bc229bf 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -59,10 +59,13 @@ sub TO_JSON { return { %{ $_[0] } }; } sub new { my $invocant = shift; my $class = ref($invocant) || $invocant; - my $object = $class->_init(@_); - bless($object, $class) if $object; + my $param = shift; - Bugzilla::Hook::process('object_end_of_new', { object => $object }); + my $object = $class->_cache_get($param); + return $object if $object; + + $object = $class->new_from_hash($class->_load_from_db($param)); + $class->_cache_set($param, $object); return $object; } @@ -72,11 +75,9 @@ sub new { # Bugzilla::Object, make sure that you modify bz_setup_database # in Bugzilla::DB::Pg appropriately, to add the right LOWER # index. You can see examples already there. -sub _init { +sub _load_from_db { my $class = shift; my ($param) = @_; - my $object = $class->_cache_get($param); - return $object if $object; my $dbh = Bugzilla->dbh; my $columns = join(',', $class->_get_db_columns); my $table = $class->DB_TABLE; @@ -88,17 +89,18 @@ sub _init { $id = $param->{id}; } + my $object_data; if (defined $id) { # We special-case if somebody specifies an ID, so that we can # validate it as numeric. detaint_natural($id) || ThrowCodeError('param_must_be_numeric', - {function => $class . '::_init'}); + {function => $class . '::_load_from_db'}); # Too large integers make PostgreSQL crash. return if $id > MAX_INT_32; - $object = $dbh->selectrow_hashref(qq{ + $object_data = $dbh->selectrow_hashref(qq{ SELECT $columns FROM $table WHERE $id_field = ?}, undef, $id); } else { @@ -125,12 +127,47 @@ sub _init { } map { trick_taint($_) } @values; - $object = $dbh->selectrow_hashref( + $object_data = $dbh->selectrow_hashref( "SELECT $columns FROM $table WHERE $condition", undef, @values); } + return $object_data; +} - $class->_cache_set($param, $object) if $object; - return $object; +sub new_from_list { + my $invocant = shift; + my $class = ref($invocant) || $invocant; + my ($id_list) = @_; + my $id_field = $class->ID_FIELD; + + my @detainted_ids; + foreach my $id (@$id_list) { + detaint_natural($id) || + ThrowCodeError('param_must_be_numeric', + {function => $class . '::new_from_list'}); + # Too large integers make PostgreSQL crash. + next if $id > MAX_INT_32; + push(@detainted_ids, $id); + } + + # We don't do $invocant->match because some classes have + # their own implementation of match which is not compatible + # with this one. However, match() still needs to have the right $invocant + # in order to do $class->DB_TABLE and so on. + return match($invocant, { $id_field => \@detainted_ids }); +} + +sub new_from_hash { + my $invocant = shift; + my $class = ref($invocant) || $invocant; + my $object_data = shift || return; + $class->_serialisation_keys($object_data); + bless($object_data, $class); + $object_data->initialize(); + return $object_data; +} + +sub initialize { + # abstract } # Provides a mechanism for objects to be cached in the request_cahce @@ -171,6 +208,15 @@ sub cache_key { } } +# To support serialisation, we need to capture the keys in an object's default +# hashref. +sub _serialisation_keys { + my ($class, $object) = @_; + my $cache = Bugzilla->request_cache->{serialisation_keys} ||= {}; + $cache->{$class} = [ keys %$object ] if $object && !exists $cache->{$class}; + return @{ $cache->{$class} }; +} + sub check { my ($invocant, $param) = @_; my $class = ref($invocant) || $invocant; @@ -204,28 +250,6 @@ sub check { return $obj; } -sub new_from_list { - my $invocant = shift; - my $class = ref($invocant) || $invocant; - my ($id_list) = @_; - my $id_field = $class->ID_FIELD; - - my @detainted_ids; - foreach my $id (@$id_list) { - detaint_natural($id) || - ThrowCodeError('param_must_be_numeric', - {function => $class . '::new_from_list'}); - # Too large integers make PostgreSQL crash. - next if $id > MAX_INT_32; - push(@detainted_ids, $id); - } - # We don't do $invocant->match because some classes have - # their own implementation of match which is not compatible - # with this one. However, match() still needs to have the right $invocant - # in order to do $class->DB_TABLE and so on. - return match($invocant, { $id_field => \@detainted_ids }); -} - # Note: Future extensions to this could be: # * Add a MATCH_JOIN constant so that we can join against # certain other tables for the WHERE criteria. @@ -324,8 +348,11 @@ sub _do_list_select { my @untainted = @{ $values || [] }; trick_taint($_) foreach @untainted; my $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @untainted); - bless ($_, $class) foreach @$objects; - return $objects + $class->_serialisation_keys($objects->[0]) if @$objects; + foreach my $object (@$objects) { + $object = $class->new_from_hash($object); + } + return $objects; } ############################### @@ -503,6 +530,13 @@ sub audit_log { } } +sub flatten_to_hash { + my $self = shift; + my $class = blessed($self); + my %hash = map { $_ => $self->{$_} } $class->_serialisation_keys; + return \%hash; +} + ############################### #### Subroutines ###### ############################### @@ -1030,6 +1064,17 @@ database matching the parameters you passed in. =back +=item C<initialize> + +=over + +=item B<Description> + +Abstract method to allow subclasses to perform initialization tasks after an +object has been created. + +=back + =item C<check> =over diff --git a/extensions/BMO/lib/Reports/EmailQueue.pm b/extensions/BMO/lib/Reports/EmailQueue.pm index 1bf2ca003..f1383aac7 100644 --- a/extensions/BMO/lib/Reports/EmailQueue.pm +++ b/extensions/BMO/lib/Reports/EmailQueue.pm @@ -39,11 +39,26 @@ sub report { $vars->{'jobs'} = $dbh->selectall_arrayref($query, { Slice => {} }); foreach my $job (@{ $vars->{'jobs'} }) { eval { - my $msg = _cond_thaw(delete $job->{'arg'})->{msg}; - if (ref($msg) && blessed($msg) eq 'Email::MIME') { - $job->{'subject'} = $msg->header('subject'); - } else { - ($job->{'subject'}) = $msg =~ /\nSubject: ([^\n]+)/; + my ($recipient, $description); + my $arg = _cond_thaw(delete $job->{arg}); + + if (exists $arg->{vars}) { + my $vars = $arg->{vars}; + $recipient = $vars->{to_user}->{login_name}; + $description = '[Bug ' . $vars->{bug}->{bug_id} . '] ' . $vars->{bug}->{short_desc}; + } elsif (exists $arg->{msg}) { + my $msg = $arg->{msg}; + if (ref($msg) && blessed($msg) eq 'Email::MIME') { + $recipient = $msg->header('to'); + $description = $msg->header('subject'); + } else { + ($recipient) = $msg =~ /\nTo: ([^\n]+)/; + ($description) = $msg =~ /\nSubject: ([^\n]+)/; + } + } + + if ($recipient) { + $job->{subject} = "<$recipient> $description"; } }; } |