Bug 372531: "match" should be a generic function in Bugzilla::Object

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

3 files changed, 84 insertions, 75 deletions

diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 83b39047d..c872cf77b 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -40,6 +40,10 @@ use File::Basename;
     BUGZILLA_VERSION
 
     bz_locations
+
+    IS_NULL
+    NOT_NULL
+
     CONTROLMAPNA
     CONTROLMAPSHOWN
     CONTROLMAPDEFAULT
@@ -148,6 +152,17 @@ use File::Basename;
 # Bugzilla version
 use constant BUGZILLA_VERSION => "3.1";
 
+# These are unique values that are unlikely to match a string or a number,
+# to be used in criteria for match() functions and other things. They start
+# and end with spaces because most Bugzilla stuff has trim() called on it,
+# so this is unlikely to match anything we get out of the DB.
+#
+# We can't use a reference, because Template Toolkit doesn't work with
+# them properly (constants.IS_NULL => {} just returns an empty string instead
+# of the reference).
+use constant IS_NULL  => '  __IS_NULL__  ';
+use constant NOT_NULL => '  __NOT_NULL__  ';
+
 #
 # ControlMap constants for group_control_map.
 # membercontol:othercontrol => meaning
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 9177ae423..1830784a9 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -487,81 +487,6 @@ sub run_create_validators {
 
 =over
 
-=item C<match>
-
-=over
-
-=item B<Description>
-
-Returns a list of fields that match the specified criteria.
-
-You should be using L<Bugzilla/get_fields> and
-L<Bugzilla/get_custom_field_names> instead of this function.
-
-=item B<Params>
-
-Takes named parameters in a hashref:
-
-=over
-
-=item C<name> - The name of the field.
-
-=item C<custom> - Boolean. True to only return custom fields. False
-to only return non-custom fields. 
-
-=item C<obsolete> - Boolean. True to only return obsolete fields.
-False to not return obsolete fields.
-
-=item C<type> - The type of the field. A C<FIELD_TYPE> constant from
-L<Bugzilla::Constants>.
-
-=item C<enter_bug> - Boolean. True to only return fields that appear
-on F<enter_bug.cgi>. False to only return fields that I<don't> appear
-on F<enter_bug.cgi>.
-
-=back
-
-=item B<Returns>
-
-A reference to an array of C<Bugzilla::Field> objects.
-
-=back
-
-=back
-
-=cut
-
-sub match {
-    my ($class, $criteria) = @_;
-  
-    my @terms;
-    if (defined $criteria->{name}) {
-        push(@terms, "name=" . Bugzilla->dbh->quote($criteria->{name}));
-    }
-    if (defined $criteria->{custom}) {
-        push(@terms, "custom=" . ($criteria->{custom} ? "1" : "0"));
-    }
-    if (defined $criteria->{obsolete}) {
-        push(@terms, "obsolete=" . ($criteria->{obsolete} ? "1" : "0"));
-    }
-    if (defined $criteria->{enter_bug}) {
-        push(@terms, "enter_bug=" . ($criteria->{enter_bug} ? '1' : '0'));
-    }
-    if (defined $criteria->{type}) {
-        push(@terms, "type = " . $criteria->{type});
-    }
-    my $where = (scalar(@terms) > 0) ? "WHERE " . join(" AND ", @terms) : "";
-
-    my $ids = Bugzilla->dbh->selectcol_arrayref(
-        "SELECT id FROM fielddefs $where", {Slice => {}});
-
-    return $class->new_from_list($ids);
-}
-
-=pod
-
-=over
-
 =item C<get_legal_field_values($field)>
 
 Description: returns all the legal values for a field that has a
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index ee1be2f9f..7d24c392a 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -23,6 +23,7 @@ use strict;
 
 package Bugzilla::Object;
 
+use Bugzilla::Constants;
 use Bugzilla::Util;
 use Bugzilla::Error;
 
@@ -134,6 +135,42 @@ sub new_from_list {
     return $objects;
 }
 
+# Note: Future extensions to this could be:
+#  * Accept arrays for an IN clause
+#  * Add a MATCH_JOIN constant so that we can join against
+#    certain other tables for the WHERE criteria.
+sub match {
+    my ($invocant, $criteria) = @_;
+    my $class = ref($invocant) || $invocant;
+    my $dbh   = Bugzilla->dbh;
+    my $id    = $class->ID_FIELD;
+    my $table = $class->DB_TABLE;
+
+    return [$class->get_all] if !$criteria;
+
+    my (@terms, @values);
+    foreach my $field (keys %$criteria) {
+        my $value = $criteria->{$field};
+        if ($value eq NOT_NULL) {
+            push(@terms, "$field IS NOT NULL");
+        }
+        elsif ($value eq IS_NULL) {
+            push(@terms, "$field IS NULL");
+        }
+        else {
+            push(@terms, "$field = ?");
+            push(@values, $value);
+        }
+    }
+
+    my $where = join(' AND ', @terms);
+    my $ids   = $dbh->selectcol_arrayref(
+        "SELECT $id FROM $table WHERE $where", undef, @values)
+        || [];
+
+    return $class->new_from_list($ids);
+}
+
 ###############################
 ####      Accessors      ######
 ###############################
@@ -462,6 +499,38 @@ A fully-initialized object.
 
  Returns:     A reference to an array of objects.
 
+=item C<match>
+
+=over
+
+=item B<Description>
+
+Gets a list of objects from the database based on certain criteria.
+
+Basically, a simple way of doing a sort of "SELECT" statement (like SQL)
+to get objects.
+
+All criteria are joined by C<AND>, so adding more criteria will give you
+a smaller set of results, not a larger set.
+
+=item B<Params>
+
+A hashref, where the keys are column names of the table, pointing to the 
+value that you want to match against for that column. 
+
+There are two special values, the constants C<NULL> and C<NOT_NULL>,
+which means "give me objects where this field is NULL or NOT NULL,
+respectively."
+
+If you don't specify any criteria, calling this function is the same
+as doing C<[$class-E<gt>get_all]>.
+
+=item B<Returns>
+
+An arrayref of objects, or an empty arrayref if there are no matches.
+
+=back
+
 =back
 
 =head2 Database Manipulation