Bug 339382: Make Bugzilla::Field use Bugzilla::Object

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

11 files changed, 54 insertions, 79 deletions

diff --git a/Bugzilla.pm b/Bugzilla.pm
index c15035928..0ffa1d1b9 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -302,12 +302,13 @@ sub switch_to_main_db {
 sub get_fields {
     my $class = shift;
     my $criteria = shift;
-    return Bugzilla::Field::match($criteria);
+    return @{Bugzilla::Field->match($criteria)};
 }
 
 sub custom_field_names {
     # Get a list of custom fields and convert it into a list of their names.
-    return map($_->{name}, Bugzilla::Field::match({ custom=>1, obsolete=>0 }));
+    return map($_->{name}, 
+               @{Bugzilla::Field->match({ custom=>1, obsolete=>0 })});
 }
 
 sub request_cache {
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 64da19af4..ba69932e9 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -876,7 +876,7 @@ sub GetBugActivity {
           FROM bugs_activity
                $suppjoins
      LEFT JOIN fielddefs
-            ON bugs_activity.fieldid = fielddefs.fieldid
+            ON bugs_activity.fieldid = fielddefs.id
     INNER JOIN profiles
             ON profiles.userid = bugs_activity.who
          WHERE bugs_activity.bug_id = ?
diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index 3597b3f46..1b2fb5429 100644
--- a/Bugzilla/BugMail.pm
+++ b/Bugzilla/BugMail.pm
@@ -224,7 +224,7 @@ sub ProcessOneBug {
                    bugs_activity.added, bugs_activity.attach_id, fielddefs.name
               FROM bugs_activity
         INNER JOIN fielddefs
-                ON fielddefs.fieldid = bugs_activity.fieldid
+                ON fielddefs.id = bugs_activity.fieldid
         INNER JOIN profiles
                 ON profiles.userid = bugs_activity.who
              WHERE bugs_activity.bug_id = ?
@@ -277,7 +277,7 @@ sub ProcessOneBug {
         INNER JOIN dependencies
                 ON bugs_activity.bug_id = dependencies.dependson
         INNER JOIN fielddefs
-                ON fielddefs.fieldid = bugs_activity.fieldid
+                ON fielddefs.id = bugs_activity.fieldid
              WHERE dependencies.blocked = ?
                AND (fielddefs.name = 'bug_status'
                     OR fielddefs.name = 'resolution')
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 94e457dd0..5396a3d20 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -451,7 +451,7 @@ use constant ABSTRACT_SCHEMA => {
 
     fielddefs => {
         FIELDS => [
-            fieldid     => {TYPE => 'MEDIUMSERIAL', NOTNULL => 1,
+            id          => {TYPE => 'MEDIUMSERIAL', NOTNULL => 1,
                             PRIMARYKEY => 1},
             name        => {TYPE => 'varchar(64)', NOTNULL => 1},
             type        => {TYPE => 'INT2', NOTNULL => 1,
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index e964141d4..e870ec01d 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -38,18 +38,18 @@ Bugzilla::Field - a particular piece of information about bugs
   use Bugzilla::Field;
 
   # Display information about non-obsolete custom fields.
-  # Bugzilla->get_fields() is a wrapper around Bugzilla::Field::match(),
+  # Bugzilla->get_fields() is a wrapper around Bugzilla::Field->match(),
   # so both methods take the same arguments.
-  print Dumper(Bugzilla::Field::match({ obsolete => 1, custom => 1 }));
+  print Dumper(Bugzilla::Field->match({ obsolete => 1, custom => 1 }));
 
   # Create a custom field.
   my $field = Bugzilla::Field::create("hilarity", "Hilarity");
-  print "$field->{description} is a custom field\n";
+  print $field->description . " is a custom field\n";
 
   # Instantiate a Field object for an existing field.
-  my $field = new Bugzilla::Field('qacontact_accessible');
-  if ($field->{obsolete}) {
-      print "$field->{description} is obsolete\n";
+  my $field = new Bugzilla::Field({name => 'qacontact_accessible'});
+  if ($field->obsolete) {
+      print $field->description . " is obsolete\n";
   }
 
   # Validation Routines
@@ -63,21 +63,28 @@ of information that Bugzilla stores about bugs.
 
 This package also provides functions for dealing with CGI form fields.
 
+C<Bugzilla::Field> is an implementation of L<Bugzilla::Object>, and
+so provides all of the methods available in L<Bugzilla::Object>,
+in addition to what is documented here.
+
 =cut
 
 package Bugzilla::Field;
 
 use strict;
 
-use base qw(Exporter);
+use base qw(Exporter Bugzilla::Object);
 @Bugzilla::Field::EXPORT = qw(check_field get_field_id get_legal_field_values);
 
 use Bugzilla::Util;
 use Bugzilla::Constants;
 use Bugzilla::Error;
 
+use constant DB_TABLE   => 'fielddefs';
+use constant LIST_ORDER => 'sortkey, name';
+
 use constant DB_COLUMNS => (
-    'fieldid AS id',
+    'id',
     'name',
     'description',
     'type',
@@ -86,52 +93,18 @@ use constant DB_COLUMNS => (
     'enter_bug',
 );
 
-our $columns = join(", ", DB_COLUMNS);
-
-sub new {
-    my $invocant = shift;
-    my $name = shift;
-    my $self = shift || Bugzilla->dbh->selectrow_hashref(
-                            "SELECT $columns FROM fielddefs WHERE name = ?",
-                            undef,
-                            $name
-                        );
-    bless($self, $invocant);
-    return $self;
-}
-
 =pod
 
 =head2 Instance Properties
 
 =over
 
-=item C<id>
-
-the unique identifier for the field;
-
-=back
-
-=cut
-
-sub id { return $_[0]->{id} }
-
-=over
-
 =item C<name>
 
 the name of the field in the database; begins with "cf_" if field
 is a custom field, but test the value of the boolean "custom" property
 to determine if a given field is a custom field;
 
-=back
-
-=cut
-
-sub name { return $_[0]->{name} }
-
-=over
-
 =item C<description>
 
 a short string describing the field; displayed to Bugzilla users
@@ -242,7 +215,7 @@ sub create {
               );
     $sth->execute($name, $desc, $sortkey, $type, $custom);
 
-    return new Bugzilla::Field($name);
+    return new Bugzilla::Field({name => $name});
 }
 
 
@@ -262,14 +235,14 @@ Params:    C<$criteria> - hash reference - the criteria to match against.
            Note: Bugzilla->get_fields() and Bugzilla->custom_field_names
            wrap this method for most callers.
 
-Returns:   a list of field objects.
+Returns:   A reference to an array of C<Bugzilla::Field> objects.
 
 =back
 
 =cut
 
 sub match {
-    my ($criteria) = @_;
+    my ($class, $criteria) = @_;
   
     my @terms;
     if (defined $criteria->{name}) {
@@ -286,13 +259,10 @@ sub match {
     }
     my $where = (scalar(@terms) > 0) ? "WHERE " . join(" AND ", @terms) : "";
   
-    my $records = Bugzilla->dbh->selectall_arrayref(
-                      "SELECT $columns FROM fielddefs $where ORDER BY sortkey",
-                      { Slice => {}}
-                  );
-    # Generate a array of field objects from the array of field records.
-    my @fields = map( new Bugzilla::Field(undef, $_), @$records );
-    return @fields;
+    my $ids = Bugzilla->dbh->selectcol_arrayref(
+        "SELECT id FROM fielddefs $where", {Slice => {}});
+
+    return $class->new_from_list($ids);
 }
 
 =pod
@@ -371,7 +341,7 @@ sub check_field {
         return 0 if $no_warn; # We don't want an error to be thrown; return.
         trick_taint($name);
 
-        my $field = new Bugzilla::Field($name);
+        my $field = new Bugzilla::Field({ name => $name });
         my $field_desc = $field ? $field->description : $name;
         ThrowCodeError('illegal_field', { field => $field_desc });
     }
@@ -401,7 +371,7 @@ sub get_field_id {
     my $dbh = Bugzilla->dbh;
 
     trick_taint($name);
-    my $id = $dbh->selectrow_array('SELECT fieldid FROM fielddefs
+    my $id = $dbh->selectrow_array('SELECT id FROM fielddefs
                                     WHERE name = ?', undef, $name);
 
     ThrowCodeError('invalid_field_name', {field => $name}) unless $id;
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index bdf764f36..673deaa30 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -1276,7 +1276,7 @@ sub init {
 
     # get a list of field names to verify the user-submitted chart fields against
     %chartfields = @{$dbh->selectcol_arrayref(
-        q{SELECT name, fieldid FROM fielddefs}, { Columns=>[1,2] })};
+        q{SELECT name, id FROM fielddefs}, { Columns=>[1,2] })};
 
     $row = 0;
     for ($chart=-1 ;
diff --git a/checksetup.pl b/checksetup.pl
index bb6e46245..5c2d889f4 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -1627,7 +1627,7 @@ my $headernum = 1;
 sub AddFDef {
     my ($name, $description, $mailhead) = (@_);
 
-    my $sth = $dbh->prepare("SELECT fieldid FROM fielddefs " .
+    my $sth = $dbh->prepare("SELECT id FROM fielddefs " .
                             "WHERE name = ?");
     $sth->execute($name);
     my ($fieldid) = ($sth->fetchrow_array());
@@ -1640,12 +1640,16 @@ sub AddFDef {
         $dbh->do(q{UPDATE fielddefs
                       SET name = ?, description = ?,
                           mailhead = ?, sortkey = ?
-                    WHERE fieldid = ?}, undef,
+                    WHERE id = ?}, undef,
                  $name, $description, $mailhead, $headernum, $fieldid);
     }
     $headernum++;
 }
 
+# Change the name of the fieldid column to id, so that fielddefs
+# can use Bugzilla::Object easily. We have to do this up here, because
+# otherwise adding these field definitions will fail.
+$dbh->bz_rename_column('fielddefs', 'fieldid', 'id');
 
 # Note that all of these entries are unconditional, from when get_field_id
 # used to create an entry if it wasn't found. New fielddef columns should
@@ -1718,7 +1722,7 @@ my $new_field_name = 'days_elapsed';
 my $field_description = 'Days since bug changed';
 
 my ($old_field_id, $old_field_name) =
-    $dbh->selectrow_array('SELECT fieldid, name
+    $dbh->selectrow_array('SELECT id, name
                            FROM fielddefs
                            WHERE description = ?',
                            undef, $field_description);
@@ -1765,7 +1769,7 @@ if ($old_field_id && ($old_field_name ne $new_field_name)) {
     # Now that saved searches have been fixed, we can fix the field name.
     print "Fixing the 'fielddefs' table...\n";
     print "New field name: " . $new_field_name . "\n";
-    $dbh->do('UPDATE fielddefs SET name = ? WHERE fieldid = ?',
+    $dbh->do('UPDATE fielddefs SET name = ? WHERE id = ?',
               undef, ($new_field_name, $old_field_id));
 }
 AddFDef($new_field_name, $field_description, 0);
@@ -2107,13 +2111,13 @@ if ($dbh->bz_column_info('bugs_activity', 'field')) {
     while (my ($f) = ($sth->fetchrow_array())) {
         my $q = $dbh->quote($f);
         my $s2 =
-            $dbh->prepare("SELECT fieldid FROM fielddefs WHERE name = $q");
+            $dbh->prepare("SELECT id FROM fielddefs WHERE name = $q");
         $s2->execute();
         my ($id) = ($s2->fetchrow_array());
         if (!$id) {
             $dbh->do("INSERT INTO fielddefs (name, description) VALUES " .
                      "($q, $q)");
-            $id = $dbh->bz_last_key('fielddefs', 'fieldid');
+            $id = $dbh->bz_last_key('fielddefs', 'id');
         }
         $dbh->do("UPDATE bugs_activity SET fieldid = $id WHERE field = $q");
     }
@@ -2546,10 +2550,10 @@ if ($dbh->bz_column_info('bugs_activity', 'oldvalue')) {
     $dbh->bz_add_column("bugs_activity", "removed", {TYPE => "TINYTEXT"});
     $dbh->bz_add_column("bugs_activity", "added", {TYPE => "TINYTEXT"});
 
-    # Need to get fieldid's for the fields that have multiple values
+    # Need to get field id's for the fields that have multiple values
     my @multi = ();
     foreach my $f ("cc", "dependson", "blocked", "keywords") {
-        my $sth = $dbh->prepare("SELECT fieldid " .
+        my $sth = $dbh->prepare("SELECT id " .
                                 "FROM fielddefs " .
                                 "WHERE name = '$f'");
         $sth->execute();
@@ -2960,13 +2964,13 @@ if ($dbh->bz_column_info("profiles", "groupset")) {
     # Replace old activity log groupset records with lists of names of groups.
     # Start by defining the bug_group field and getting its id.
     AddFDef("bug_group", "Group", 0);
-    $sth = $dbh->prepare("SELECT fieldid " .
+    $sth = $dbh->prepare("SELECT id " .
                            "FROM fielddefs " .
                           "WHERE name = " . $dbh->quote('bug_group'));
     $sth->execute();
     my ($bgfid) = $sth->fetchrow_array;
     # Get the field id for the old groupset field
-    $sth = $dbh->prepare("SELECT fieldid " .
+    $sth = $dbh->prepare("SELECT id " .
                            "FROM fielddefs " .
                           "WHERE name = " . $dbh->quote('groupset'));
     $sth->execute();
@@ -3108,10 +3112,10 @@ if ($dbh->bz_table_info("attachstatuses")
     
     # Get IDs for the old attachment status and new flag fields.
     my ($old_field_id) = $dbh->selectrow_array(
-        "SELECT fieldid FROM fielddefs WHERE name='attachstatusdefs.name'")
+        "SELECT id FROM fielddefs WHERE name='attachstatusdefs.name'")
         || 0;
     
-    $sth = $dbh->prepare("SELECT fieldid FROM fielddefs " . 
+    $sth = $dbh->prepare("SELECT id FROM fielddefs " . 
                          "WHERE name='flagtypes.name'");
     $sth->execute();
     my $new_field_id = $sth->fetchrow_arrayref()->[0];
diff --git a/collectstats.pl b/collectstats.pl
index 808badea9..1672f679a 100755
--- a/collectstats.pl
+++ b/collectstats.pl
@@ -79,7 +79,7 @@ my $old_resolutions =
     $dbh->selectcol_arrayref('SELECT bugs_activity.added
                                 FROM bugs_activity
                           INNER JOIN fielddefs
-                                  ON fielddefs.fieldid = bugs_activity.fieldid
+                                  ON fielddefs.id = bugs_activity.fieldid
                            LEFT JOIN resolution
                                   ON resolution.value = bugs_activity.added
                                WHERE fielddefs.name = ?
@@ -90,7 +90,7 @@ my $old_resolutions =
                               SELECT bugs_activity.removed
                                 FROM bugs_activity
                           INNER JOIN fielddefs
-                                  ON fielddefs.fieldid = bugs_activity.fieldid
+                                  ON fielddefs.id = bugs_activity.fieldid
                            LEFT JOIN resolution
                                   ON resolution.value = bugs_activity.removed
                                WHERE fielddefs.name = ?
@@ -449,7 +449,7 @@ FIN
             $query = qq{SELECT bugs_activity.removed 
                           FROM bugs_activity 
                     INNER JOIN fielddefs 
-                            ON bugs_activity.fieldid = fielddefs.fieldid 
+                            ON bugs_activity.fieldid = fielddefs.id 
                          WHERE fielddefs.name = ? 
                            AND bugs_activity.bug_id = ? 
                            AND bugs_activity.bug_when >= } . 
diff --git a/customfield.pl b/customfield.pl
index 2821425c7..033ac517a 100755
--- a/customfield.pl
+++ b/customfield.pl
@@ -75,7 +75,7 @@ $name =~ /^cf_/
   or $name = "cf_" . $name;
 
 # Exit gracefully if there is already a field with the given name.
-if (scalar(Bugzilla::Field::match({ name=>$name })) > 0) {
+if ( new Bugzilla::Field({name => $name}) ) {
     print "There is already a field named $name.  Please choose " .
           "a different name.\n";
     exit;
diff --git a/editusers.cgi b/editusers.cgi
index 9a4bfd5c0..4d7292391 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -755,7 +755,7 @@ if ($action eq 'search') {
                 profiles_activity.newvalue AS added
          FROM profiles_activity
          INNER JOIN profiles ON profiles_activity.who = profiles.userid
-         INNER JOIN fielddefs ON fielddefs.fieldid = profiles_activity.fieldid
+         INNER JOIN fielddefs ON fielddefs.id = profiles_activity.fieldid
          WHERE profiles_activity.userid = ?
          ORDER BY profiles_activity.profiles_when",
         {'Slice' => {}},
diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index aeeee5ea9..85b6f5fa2 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -391,7 +391,7 @@ CrossCheck('classifications', 'id',
 CrossCheck("keyworddefs", "id",
            ["keywords", "keywordid"]);
 
-CrossCheck("fielddefs", "fieldid",
+CrossCheck("fielddefs", "id",
            ["bugs_activity", "fieldid"],
            ['profiles_activity', 'fieldid']);