Bug 545541: New Hook: object_columns

r=mkanat, a=mkanat (module owner)

7 files changed, 90 insertions, 13 deletions

diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm
index 9c2fbdb38..95fb4bf82 100644
--- a/Bugzilla/Field/Choice.pm
+++ b/Bugzilla/Field/Choice.pm
@@ -142,7 +142,7 @@ sub create {
     my $class = shift;
     my ($params) = @_;
     foreach my $key (keys %$params) {
-        if (!grep {$_ eq $key} $class->DB_COLUMNS) {
+        if (!grep {$_ eq $key} $class->_get_db_columns) {
             delete $params->{$key};
         }
     }
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index d33b14d31..4f042cb74 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -444,7 +444,7 @@ sub create {
     $timestamp ||= Bugzilla->dbh->selectrow_array('SELECT NOW()');
 
     my $params = {};
-    my @columns = grep { $_ ne 'id' } $class->DB_COLUMNS;
+    my @columns = grep { $_ ne 'id' } $class->_get_db_columns;
     $params->{$_} = $flag->{$_} foreach @columns;
 
     $params->{creation_date} = $params->{modification_date} = $timestamp;
diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm
index faef9f07d..2f4d2a7ff 100644
--- a/Bugzilla/Hook.pm
+++ b/Bugzilla/Hook.pm
@@ -181,6 +181,8 @@ takes a C<modules> parameter, just like L</auth_login_methods>.
 
 =head2 bug_columns
 
+B<DEPRECATED> Use L</object_columns> instead.
+
 This allows you to add new fields that will show up in every L<Bugzilla::Bug>
 object. Note that you will also need to use the L</bug_fields> hook in
 conjunction with this hook to make this work.
@@ -602,6 +604,44 @@ The value being set on the object.
 
 =back
 
+=head2 object_columns
+
+This hook allows you to add new "fields" to existing Bugzilla objects,
+that correspond to columns in their tables.
+
+For example, if you added an C<example> column to the "bugs" table, you
+would have to also add an C<example> field to the C<Bugzilla::Bug> object
+in order to access that data via Bug objects.
+
+Don't do anything slow inside this hook--it's called several times on
+every page of Bugzilla.
+
+Params:
+
+=over
+
+=item C<class>
+
+The name of the class that this hook is being called on. You can check this 
+like C<< if ($class->isa('Some::Class')) >> in your code, to add new
+fields only for certain classes.
+
+=item C<columns>
+
+An arrayref. Add the string names of columns to this array to add new
+values to objects. 
+
+For example, if you add an C<example> column to a particular table
+(using L</install_update_db>), and then push the string C<example> into 
+this array for the object that uses that table, then you can access the
+information in that column via C<< $object->{example} >> on all objects
+of that type.
+
+This arrayref does not contain the standard column names--you cannot modify
+or remove standard object columns using this hook.
+
+=back
+
 =head2 object_end_of_create_validators
 
 Called at the end of L<Bugzilla::Object/run_create_validators>. You can
@@ -688,7 +728,7 @@ Params:
 
 The name of the class that C<VALIDATORS> was called on. You can check this 
 like C<< if ($class->isa('Some::Class')) >> in your code, to add
-validators only for certain classes
+validators only for certain classes.
 
 =item C<validators>
 
diff --git a/Bugzilla/Keyword.pm b/Bugzilla/Keyword.pm
index f4742bebd..882adaf02 100644
--- a/Bugzilla/Keyword.pm
+++ b/Bugzilla/Keyword.pm
@@ -78,7 +78,8 @@ sub get_all_with_bug_count {
     my $class = shift;
     my $dbh = Bugzilla->dbh;
     my $keywords =
-      $dbh->selectall_arrayref('SELECT ' . join(', ', DB_COLUMNS) . ',
+      $dbh->selectall_arrayref('SELECT ' 
+                                      . join(', ', $class->_get_db_columns) . ',
                                        COUNT(keywords.bug_id) AS bug_count
                                   FROM keyworddefs
                              LEFT JOIN keywords
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index bf8d69d5a..e7763157a 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -59,7 +59,7 @@ sub _init {
     my $class = shift;
     my ($param) = @_;
     my $dbh = Bugzilla->dbh;
-    my $columns = join(',', $class->DB_COLUMNS);
+    my $columns = join(',', $class->_get_db_columns);
     my $table   = $class->DB_TABLE;
     my $name_field = $class->NAME_FIELD;
     my $id_field   = $class->ID_FIELD;
@@ -241,7 +241,7 @@ sub match {
 sub _do_list_select {
     my ($class, $where, $values, $postamble) = @_;
     my $table = $class->DB_TABLE;
-    my $cols  = join(',', $class->DB_COLUMNS);
+    my $cols  = join(',', $class->_get_db_columns);
     my $order = $class->LIST_ORDER;
 
     my $sql = "SELECT $cols FROM $table";
@@ -487,12 +487,33 @@ sub get_all {
 
 sub check_boolean { return $_[1] ? 1 : 0 }
 
-# For some classes, VALIDATORS takes time to generate, so we cache it. Also,
-# this allows the object_validators hook to only run once per request, 
-# instead of every time we call set() on a class of objects.
-#
-# This method is intentionally private and should only be called by
-# Bugzilla::Object.
+####################
+# Constant Helpers #
+####################
+
+# For some classes, some constants take time to generate, so we cache them
+# and only access them through the below methods. This also allows certain
+# hooks to only run once per request instead of multiple times on each
+# page.
+
+sub _get_db_columns {
+    my $invocant = shift;
+    my $class = ref($invocant) || $invocant;
+    my $cache = Bugzilla->request_cache;
+    my $cache_key = "object_${class}_db_columns";
+    return @{ $cache->{$cache_key} } if $cache->{$cache_key};
+    # Currently you can only add new columns using object_columns, not
+    # remove or modify existing columns, because removing columns would
+    # almost certainly cause Bugzilla to function improperly.
+    my @add_columns;
+    Bugzilla::Hook::process('object_columns',
+                            { class => $class, columns => \@add_columns });
+    my @columns = ($invocant->DB_COLUMNS, @add_columns);
+    $cache->{$cache_key} = \@columns;
+    return @{ $cache->{$cache_key} };
+}
+
+# This method is private and should only be called by Bugzilla::Object.
 sub _get_validators {
     my $invocant = shift;
     my $class = ref($invocant) || $invocant;
@@ -554,6 +575,12 @@ for C<Bugzilla::Keyword> this would be C<keyworddefs>.
 The names of the columns that you want to read out of the database
 and into this object. This should be an array.
 
+I<Note>: Though normally you will never need to access this constant's data 
+directly in your subclass, if you do, you should access it by calling the
+C<_get_db_columns> method instead of accessing the constant directly. (The
+only exception to this rule is calling C<SUPER::DB_COLUMNS> from within
+your own C<DB_COLUMNS> subroutine in a subclass.)
+
 =item C<NAME_FIELD>
 
 The name of the column that should be considered to be the unique
diff --git a/contrib/console.pl b/contrib/console.pl
index e9d06cd94..408fdef61 100755
--- a/contrib/console.pl
+++ b/contrib/console.pl
@@ -96,7 +96,7 @@ sub get_object {
     elsif (m/^\d+$/) {
         @results = ($class->new($_));
     }
-    elsif (m/\w/i && grep {$_ eq 'name'} ($class->DB_COLUMNS)) {
+    elsif (m/\w/i && grep {$_ eq 'name'} ($class->_get_db_columns)) {
         @results = @{$class->match({name => $_})};
     }
     else {
diff --git a/extensions/Example/Extension.pm b/extensions/Example/Extension.pm
index 5f4cdf995..0e4e0f722 100644
--- a/extensions/Example/Extension.pm
+++ b/extensions/Example/Extension.pm
@@ -333,6 +333,15 @@ sub object_before_set {
     }
 }
 
+sub object_columns {
+    my ($self, $args) = @_;
+    my ($class, $columns) = @$args{qw(class columns)};
+
+    if ($class->isa('Bugzilla::ExampleObject')) {
+        push(@$columns, 'example');
+    }
+}
+
 sub object_end_of_create_validators {
     my ($self, $args) = @_;