From e9a140e61d6fb160098a38657daf9476d8fd6950 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Tue, 27 Aug 2013 00:05:05 +0800 Subject: Bug 904568: emails generated by jobqueue.pl unable to reference custom fields r=simon, a=simon --- Bugzilla/Bug.pm | 24 +++++++----- Bugzilla/Object.pm | 111 +++++++++++++++++++++++++++++++---------------------- 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 + +=over + +=item C + +Ensures the accessors for custom fields are always created. + +=back + =head1 B =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 + +=over + +=item B + +Abstract method to allow subclasses to perform initialization tasks after an +object has been created. + +=back + =item C =over -- cgit v1.2.3-24-g4f1b