Bug 904568: emails generated by jobqueue.pl unable to reference custom fields

r=simon, a=simon

2 files changed, 79 insertions, 56 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 842dacff4..302b58595 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -301,21 +301,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;
@@ -363,6 +353,10 @@ sub new {
     return $self;
 }
 
+sub initialize {
+    $_[0]->_create_cf_accessors();
+}
+
 sub cache_key {
     my $class = shift;
     my $key = $class->SUPER::cache_key(@_)
@@ -4316,6 +4310,16 @@ sub _multi_select_accessor {
 
 1;
 
+=head1 B<Methods>
+
+=over
+
+=item C<initialize>
+
+Ensures the accessors for custom fields are always created.
+
+=back
+
 =head1 B<Methods in need of POD>
 
 =over
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 9f305e929..add5887a6 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -46,21 +46,24 @@ 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;
+
+    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;
 }
 
-
 # Note: Because this uses sql_istrcmp, if you make a new object use
 # 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;
@@ -72,17 +75,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 {
@@ -109,15 +113,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;
+}
 
-    if ($object) {
-        $class->_serialisation_keys($object);
-        $class->_cache_set($param, $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);
     }
-    return $object;
+
+    # 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_cache
@@ -152,7 +188,7 @@ sub cache_key {
 sub _serialisation_keys {
     my ($class, $object) = @_;
     my $cache = Bugzilla->request_cache->{serialisation_keys} ||= {};
-    $cache->{$class} = [ keys %$object ] if $object;
+    $cache->{$class} = [ keys %$object ] if $object && !exists $cache->{$class};
     return @{ $cache->{$class} };
 }
 
@@ -189,36 +225,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 });
-}
-
-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.
@@ -318,8 +324,10 @@ sub _do_list_select {
     trick_taint($_) foreach @untainted;
     my $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @untainted);
     $class->_serialisation_keys($objects->[0]) if @$objects;
-    bless($_, $class) foreach @$objects;
-    return $objects
+    foreach my $object (@$objects) {
+        $object = $class->new_from_hash($object);
+    }
+    return $objects;
 }
 
 ###############################
@@ -1029,6 +1037,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