From e9f07c7e2bc8dcf66381ed588ddd955d77b7d26f Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Wed, 31 Jul 2013 13:20:07 +0800 Subject: Bug 877078: shift bugmail generation to the jobqueue r=sgreen, a=sgreen --- Bugzilla/BugMail.pm | 99 +++++++++++++++++++++++++++++++++++++++++-------- Bugzilla/Job/BugMail.pm | 31 ++++++++++++++++ Bugzilla/Job/Mailer.pm | 10 ----- Bugzilla/JobQueue.pm | 1 + Bugzilla/Object.pm | 50 ++++++++++++++++++++++++- 5 files changed, 163 insertions(+), 28 deletions(-) create mode 100644 Bugzilla/Job/BugMail.pm (limited to 'Bugzilla') diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 1169e94a7..e84a370c0 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -23,6 +23,7 @@ use Date::Parse; use Date::Format; use Scalar::Util qw(blessed); use List::MoreUtils qw(uniq); +use Storable qw(dclone); use constant BIT_DIRECT => 1; use constant BIT_WATCHING => 2; @@ -339,31 +340,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, - new_comments => \@send_comments, - threadingmarker => build_thread_marker($bug->id, $user->id, !$bug->lastdiffed), - bugmailtype => $bugmailtype + changer => $changer, + diffs => \@display_diffs, + changedfields => \@changedfields, + new_comments => \@send_comments, + threadingmarker => build_thread_marker($bug->id, $user->id, !$bug->lastdiffed), + 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); - + $template->process("email/bugmail-header.txt.tmpl", $vars, \$msg_header) || ThrowTemplateError($template->error()); $template->process("email/bugmail.txt.tmpl", $vars, \$msg_text) @@ -507,6 +553,27 @@ sub _get_new_bugmail_fields { 1; +=head1 NAME + +BugMail - Routines to generate email notifications when a bug is created or +modified. + +=head1 METHODS + +=over 4 + +=item C + +Serialises the variables required to generate bugmail and pushes the result to +the job-queue for processing by TheSchwartz. + +=item C + +When given serialised variables from the job-queue, recreates the objects from +the flattened hashes, generates the bugmail, and sends it. + +=back + =head1 B =over 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/Job/Mailer.pm b/Bugzilla/Job/Mailer.pm index 3d52f8428..8fce80dd0 100644 --- a/Bugzilla/Job/Mailer.pm +++ b/Bugzilla/Job/Mailer.pm @@ -43,13 +43,3 @@ sub work { } 1; - -=head1 B - -=over - -=item retry_delay - -=item work - -=back diff --git a/Bugzilla/JobQueue.pm b/Bugzilla/JobQueue.pm index 9365a7d56..d67066322 100644 --- a/Bugzilla/JobQueue.pm +++ b/Bugzilla/JobQueue.pm @@ -22,6 +22,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 0cecb8eca..9f305e929 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -113,7 +113,10 @@ sub _init { "SELECT $columns FROM $table WHERE $condition", undef, @values); } - $class->_cache_set($param, $object) if $object; + if ($object) { + $class->_serialisation_keys($object); + $class->_cache_set($param, $object); + } return $object; } @@ -144,6 +147,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; + return @{ $cache->{$class} }; +} + sub check { my ($invocant, $param) = @_; my $class = ref($invocant) || $invocant; @@ -199,6 +211,14 @@ sub new_from_list { return match($invocant, { $id_field => \@detainted_ids }); } +sub new_from_hash { + my $invocant = shift; + my $class = ref($invocant) || $invocant; + my $object = shift; + bless($object, $class); + return $object; +} + # 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. @@ -297,7 +317,8 @@ sub _do_list_select { my @untainted = @{ $values || [] }; trick_taint($_) foreach @untainted; my $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @untainted); - bless ($_, $class) foreach @$objects; + $class->_serialisation_keys($objects->[0]) if @$objects; + bless($_, $class) foreach @$objects; return $objects } @@ -474,6 +495,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 ###### ############################### @@ -1040,6 +1068,13 @@ template. Returns: A reference to an array of objects. +=item C + + Description: Create an object from the given hash. + + Params: $hashref - A reference to a hash which was created by + flatten_to_hash. + =item C =over @@ -1277,6 +1312,17 @@ that should be passed to the C function that is called. =back +=head2 Simple Methods + +=over + +=item C + +Returns a hashref suitable for serialisation and re-inflation with C. + +=back + + =head2 Simple Validators You can use these in your subclass L or L. -- cgit v1.2.3-24-g4f1b