Bug 877078: shift bugmail generation to the jobqueue

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<enqueue>
+
+Serialises the variables required to generate bugmail and pushes the result to
+the job-queue for processing by TheSchwartz.
+
+=item C<dequeue>
+
+When given serialised variables from the job-queue, recreates the objects from
+the flattened hashes, generates the bugmail, and sends it.
+
+=back
+
 =head1 B<Methods in need of POD>
 
 =over

Bugzilla/Job/BugMail.pm

+# 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;

Bugzilla/Job/Mailer.pm

@@ -43,13 +43,3 @@ sub work {
 }
 
 1;
-
-=head1 B<Methods in need of POD>
-
-=over
-
-=item retry_delay
-
-=item work
-
-=back

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

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<new_from_hash($hashref)>
+
+  Description: Create an object from the given hash.
+
+  Params:      $hashref - A reference to a hash which was created by
+                          flatten_to_hash.
+
 =item C<match>
 
 =over
@@ -1277,6 +1312,17 @@ that should be passed to the C<set_> function that is called.
 
 =back
 
+=head2 Simple Methods
+
+=over
+
+=item C<flatten_to_hash>
+
+Returns a hashref suitable for serialisation and re-inflation with C<new_from_hash>.
+
+=back
+
+
 =head2 Simple Validators
 
 You can use these in your subclass L</VALIDATORS> or L</UPDATE_VALIDATORS>.

t/011pod.t

@@ -43,6 +43,7 @@ use constant MODULE_WHITELIST => qw(
     Bugzilla::Auth::Verify::
     Bugzilla::BugUrl::
     Bugzilla::Config::
+    Bugzilla::Job::
 );
 
 # Capture the TESTOUT from Test::More or Test::Builder for printing errors.