summaryrefslogtreecommitdiffstats
path: root/Bugzilla
diff options
context:
space:
mode:
authorByron Jones <bjones@mozilla.com>2013-10-21 09:46:30 +0200
committerByron Jones <bjones@mozilla.com>2013-10-21 09:46:30 +0200
commite3edbc52e592519697bfb0130a3ef13a7744a5d2 (patch)
tree3dda3c21c300429e06b42eb6e0c4f67d92103081 /Bugzilla
parent5d096b6f18c5e42f3246c9a652c276fabda5f6f3 (diff)
downloadbugzilla-e3edbc52e592519697bfb0130a3ef13a7744a5d2.tar.gz
bugzilla-e3edbc52e592519697bfb0130a3ef13a7744a5d2.tar.xz
Bug 910565: backport bug 877078 to bmo (shift bugmail generation to the jobqueue)
Diffstat (limited to 'Bugzilla')
-rw-r--r--Bugzilla/Bug.pm14
-rw-r--r--Bugzilla/BugMail.pm81
-rw-r--r--Bugzilla/Job/BugMail.pm31
-rw-r--r--Bugzilla/JobQueue.pm1
-rw-r--r--Bugzilla/Object.pm115
5 files changed, 180 insertions, 62 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