Bug 17453: Enumerators in Bugzilla are not cross-DB compatible. This removes all 'enum' types in the database from Bugzilla.

Patch By Max Kanat-Alexander <mkanat@kerio.com> r=joel, a=justdave

4 files changed, 211 insertions, 222 deletions

diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index a1058fbe5..8a08ef618 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -50,14 +50,26 @@ use Date::Parse;
 # Each field points to an array that contains the fields mapped 
 # to, in order.
 our %specialorder = (
-    'bugs.target_milestone' => [ 'ms_order.sortkey','ms_order.value' ]
+    'bugs.target_milestone' => [ 'ms_order.sortkey','ms_order.value' ],
+    'bugs.bug_status' => [ 'bug_status.sortkey','bug_status.value' ],
+    'bugs.rep_platform' => [ 'rep_platform.sortkey','rep_platform.value' ],
+    'bugs.priority' => [ 'priority.sortkey','priority.value' ],
+    'bugs.op_sys' => [ 'op_sys.sortkey','op_sys.value' ],
+    'bugs.resolution' => [ 'resolution.sortkey', 'resolution.value' ],
+    'bugs.bug_severity' => [ 'bug_severity.sortkey','bug_severity.value' ]
 );
 
 # When we add certain fields to the ORDER BY, we need to then add a
 # table join to the FROM statement. This hash maps input fields to 
 # the join statements that ned to be added.
 our %specialorderjoin = (
-    'bugs.target_milestone' => 'LEFT JOIN milestones AS ms_order ON ms_order.value = bugs.target_milestone AND ms_order.product_id = bugs.product_id'
+    'bugs.target_milestone' => 'LEFT JOIN milestones AS ms_order ON ms_order.value = bugs.target_milestone AND ms_order.product_id = bugs.product_id',
+    'bugs.bug_status' => 'LEFT JOIN bug_status ON bug_status.value = bugs.bug_status',
+    'bugs.rep_platform' => 'LEFT JOIN rep_platform ON rep_platform.value = bugs.rep_platform',
+    'bugs.priority' => 'LEFT JOIN priority ON priority.value = bugs.priority',
+    'bugs.op_sys' => 'LEFT JOIN op_sys ON op_sys.value = bugs.op_sys',
+    'bugs.resolution' => 'LEFT JOIN resolution ON resolution.value = bugs.resolution',
+    'bugs.bug_severity' => 'LEFT JOIN bug_severity ON bug_severity.value = bugs.bug_severity'
 );
 
 # Create a new Search
diff --git a/checksetup.pl b/checksetup.pl
index 2aa2a6cc1..cd734a54a 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -29,6 +29,8 @@
 #                 Shane H. W. Travis <travis@sedsystems.ca>
 #                 Gervase Markham <gerv@gerv.net>
 #                 Erik Stambaugh <erik@dasbistro.com>
+#                 Dave Lawrence <dkl@redhat.com>
+#                 Max Kanat-Alexander <mkanat@kerio.com>
 #
 #
 # Direct any questions on this source code to
@@ -696,101 +698,19 @@ LocalVar('db_check', '
 $db_check = 1;
 ');
 
-
-LocalVar('severities', '
-#
-# Which bug and feature-request severities do you want?
-#
-@severities = (
-        "blocker",
-        "critical",
-        "major",
-        "normal",
-        "minor",
-        "trivial",
-        "enhancement"
-);
-');
-
-
-
-LocalVar('priorities', '
-#
-# Which priorities do you want to assign to bugs and feature-request?
-#
-@priorities = (
-        "P1",
-        "P2",
-        "P3",
-        "P4",
-        "P5"
-);
-');
-
-
-
-LocalVar('opsys', '
-#
-# What operatings systems may your products run on?
-#
-@opsys = (
-        "All",
-        "Windows 3.1",
-        "Windows 95",
-        "Windows 98",
-        "Windows ME",  # Millenium Edition (upgrade of 98)
-        "Windows 2000",
-        "Windows NT",
-        "Windows XP",
-        "Windows Server 2003",
-        "Mac System 7",
-        "Mac System 7.5",
-        "Mac System 7.6.1",
-        "Mac System 8.0",
-        "Mac System 8.5",
-        "Mac System 8.6",
-        "Mac System 9.x",
-        "Mac OS X 10.0",
-        "Mac OS X 10.1",
-        "Mac OS X 10.2",
-        "Mac OS X 10.3",
-        "Linux",
-        "BSD/OS",
-        "FreeBSD",
-        "NetBSD",
-        "OpenBSD",
-        "AIX",
-        "BeOS",
-        "HP-UX",
-        "IRIX",
-        "Neutrino",
-        "OpenVMS",
-        "OS/2",
-        "OSF/1",
-        "Solaris",
-        "SunOS",
-        "other"
-);
-');
-
-
-
-LocalVar('platforms', '
-#
-# What hardware platforms may your products run on?
-#
-@platforms = (
-        "All",
-        "DEC",
-        "HP",
-        "Macintosh",
-        "PC",
-        "SGI",
-        "Sun",
-        "Other"
-);
-');
-
+my @deprecatedvars;
+push(@deprecatedvars, '@severities') if (LocalVarExists('severities'));
+push(@deprecatedvars, '@priorities') if (LocalVarExists('priorities'));
+push(@deprecatedvars, '@opsys') if (LocalVarExists('opsys'));
+push(@deprecatedvars, '@platforms') if (LocalVarExists('platforms'));
+
+if (@deprecatedvars) {
+    print "\nThe following settings in your localconfig file",
+          " are no longer used:\n  " . join(", ", @deprecatedvars) .
+          "\nThis data is now controlled through the Bugzilla",
+          " administrative interface.\nWe recommend you remove these",
+          " settings from localconfig after checksetup\nruns successfully.\n";
+}
 if (LocalVarExists('mysqlpath')) {
     print "\nThe \$mysqlpath setting in your localconfig file ",
           "is no longer required.\nWe recommend you remove it.\n";
@@ -817,10 +737,22 @@ my $my_db_name = ${*{$main::{'db_name'}}{SCALAR}};
 my $my_index_html = ${*{$main::{'index_html'}}{SCALAR}};
 my $my_create_htaccess = ${*{$main::{'create_htaccess'}}{SCALAR}};
 my $my_webservergroup = ${*{$main::{'webservergroup'}}{SCALAR}};
-my @my_severities = @{*{$main::{'severities'}}{ARRAY}};
-my @my_priorities = @{*{$main::{'priorities'}}{ARRAY}};
-my @my_platforms = @{*{$main::{'platforms'}}{ARRAY}};
-my @my_opsys = @{*{$main::{'opsys'}}{ARRAY}};
+# mkanat@kerio.com - bug 17453
+# The following values have been removed from localconfig.
+# However, if we are upgrading from a Bugzilla with enums to a 
+# Bugzilla without enums, we use these values one more time so 
+# that we correctly populate the tables.
+my @my_severities;
+@my_severities = @{*{$main::{'severities'}}{ARRAY}} 
+    if exists($main::{'severities'});
+my @my_priorities;
+@my_priorities = @{*{$main::{'priorities'}}{ARRAY}}
+    if exists($main::{'priorities'});
+my @my_platforms;
+@my_platforms = @{*{$main::{'platforms'}}{ARRAY}}
+    if exists($main::{'platforms'});
+my @my_opsys;
+@my_opsys = @{*{$main::{'opsys'}}{ARRAY}} if exists($main::{'opsys'});
 
 if ($my_webservergroup && !$silent) {
     if ($^O !~ /MSWin32/i) {
@@ -1771,27 +1703,23 @@ $table{flagexclusions} =
     INDEX(type_id, product_id, component_id)
    ';
 
-#
-# Apostrophe's are not supportied in the enum types.
-# See http://bugzilla.mozilla.org/show_bug.cgi?id=27309
-#
 $table{bugs} =
    'bug_id mediumint not null auto_increment primary key,
     assigned_to mediumint not null, # This is a comment.
     bug_file_loc text,
-    bug_severity enum($my_severities) not null,
-    bug_status enum("UNCONFIRMED", "NEW", "ASSIGNED", "REOPENED", "RESOLVED", "VERIFIED", "CLOSED") not null,
+    bug_severity varchar(64) not null,
+    bug_status varchar(64) not null,
     creation_ts datetime not null,
     delta_ts datetime not null,
     short_desc mediumtext not null,
-    op_sys enum($my_opsys) not null,
-    priority enum($my_priorities) not null,
+    op_sys varchar(64) not null,
+    priority varchar(64) not null,
     product_id smallint not null,
-    rep_platform enum($my_platforms),
+    rep_platform varchar(64),
     reporter mediumint not null,
     version varchar(64) not null,
     component_id smallint not null,
-    resolution enum("", "FIXED", "INVALID", "WONTFIX", "LATER", "REMIND", "DUPLICATE", "WORKSFORME", "MOVED") not null,
+    resolution varchar(64) not null,
     target_milestone varchar(20) not null default "---",
     qa_contact mediumint not null,
     status_whiteboard mediumtext not null,
@@ -2150,6 +2078,63 @@ $table{whine_events} =
      subject        varchar(128),
      body           mediumtext';
 
+# mkanat@kerio.com - bug 17453
+# Below are all the old enumerations converted to tables
+$table{bug_status} =
+    'id             smallint        auto_increment primary key,
+     value          varchar(64)     not null,
+     sortkey        smallint        not null default 0,
+     isactive       tinyint(1)      not null default 1,
+
+     unique(value),
+     index(sortkey, value)';
+
+$table{rep_platform} = 
+    'id             smallint        auto_increment primary key,
+     value          varchar(64)     not null,
+     sortkey        smallint        not null default 0,
+     isactive       tinyint(1)      not null default 1,
+
+     unique(value),
+     index(sortkey, value)';
+
+$table{resolution} = 
+    'id             smallint        auto_increment primary key,
+     value          varchar(64)     not null,
+     sortkey        smallint        not null default 0,
+     isactive       tinyint(1)      not null default 1,
+
+     unique(value),
+     index(sortkey, value)';
+
+$table{op_sys} = 
+    'id             smallint        auto_increment primary key,
+     value          varchar(64)     not null,
+     sortkey        smallint        not null default 0,
+     isactive       tinyint(1)      not null default 1,
+
+     unique(value),
+     index(sortkey, value)';
+
+$table{bug_severity} = 
+    'id             smallint        auto_increment primary key,
+     value          varchar(64)     not null,
+     sortkey        smallint        not null default 0,
+     isactive       tinyint(1)      not null default 1,
+
+     unique(value),
+     index(sortkey, value)';
+
+$table{priority} = 
+    'id             smallint        auto_increment primary key,
+     value          varchar(64)     not null,
+     sortkey        smallint        not null default 0,
+     isactive       tinyint(1)      not null default 1,
+
+     unique(value),
+     index(sortkey, value)';
+
+
 ###########################################################################
 # Create tables
 ###########################################################################
@@ -2183,26 +2168,11 @@ $sth = $dbh->table_info(undef, undef, undef, "TABLE");
 my @tables = @{$dbh->selectcol_arrayref($sth, { Columns => [3] })};
 #print 'Tables: ', join " ", @tables, "\n";
 
-# add lines here if you add more --LOCAL-- config vars that end up in the enums:
-
-my $my_severities = '"' . join('", "', @my_severities) . '"';
-my $my_priorities = '"' . join('", "', @my_priorities) . '"';
-my $my_opsys      = '"' . join('", "', @my_opsys)      . '"';
-my $my_platforms  = '"' . join('", "', @my_platforms)  . '"';
-
-# go throught our %table hash and create missing tables
+# go through our %table hash and create missing tables
 while (my ($tabname, $fielddef) = each %table) {
     next if grep /^$tabname$/, @tables;
     print "Creating table $tabname ...\n";
 
-    # add lines here if you add more --LOCAL-- config vars that end up in
-    # the enums:
-
-    $fielddef =~ s/\$my_severities/$my_severities/;
-    $fielddef =~ s/\$my_priorities/$my_priorities/;
-    $fielddef =~ s/\$my_opsys/$my_opsys/;
-    $fielddef =~ s/\$my_platforms/$my_platforms/;
-
     $dbh->do("CREATE TABLE $tabname (\n$fielddef\n) TYPE = MYISAM")
         or die "Could not create table '$tabname'. Please check your '$my_db_driver' access.\n";
 }
@@ -2412,45 +2382,67 @@ sub DropIndexes ($)
     }
 
 }
-#
-# Check if the enums in the bugs table return the same values that are defined
-# in the various locally changeable variables. If this is true, then alter the
-# table definition.
-#
-
-sub CheckEnumField ($$@)
-{
-    my ($table, $field, @against) = @_;
 
-    my $ref = GetFieldDef($table, $field);
-    #print "0: $$ref[0]   1: $$ref[1]   2: $$ref[2]   3: $$ref[3]  4: $$ref[4]\n";
-    
-    $_ = "enum('" . join("','", @against) . "')";
-    if ($$ref[1] ne $_) {
-        print "Updating field $field in table $table ...\n";
-        $_ .= " NOT NULL" if $$ref[3];
-        $dbh->do("ALTER TABLE $table
-                  CHANGE $field
-                  $field $_");
-    }
-}
+# mkanat@kerio.com - bug 17453
+# Create the values for the tables that hold what used to be enum types.
+# Don't populate the tables if the table isn't empty.
+sub PopulateEnumTable ($@) {
+    my ($table, @valuelist) = @_;
 
+    # If we encounter any of the keys in this hash, they are 
+    # automatically set to isactive=0
+    my %defaultinactive = ('---' => 1);
 
+    # Check if there are any table entries
+    my $query = "SELECT COUNT(id) FROM $table";
+    my $sth = $dbh->prepare($query);
+    $sth->execute();
 
-#
-# This code changes the enum types of some SQL tables whenever you change
-# some --LOCAL-- variables. Once you have a running system, to add new 
-# severities, priorities, operating systems and platforms, add them to 
-# the localconfig file and then re-run checksetup.pl which will make the 
-# necessary changes to your database. Additions to these fields in
-# checksetup.pl after the initial installation of bugzilla on a system
-# are ignored.
-#
+    # If the table is empty...
+    if ( !$sth->fetchrow_array() ) {
+        my $insert = $dbh->prepare("INSERT INTO $table"
+            . " (value,sortkey,isactive) VALUES (?,?,?)");
+        my $sortorder = 0;
+        foreach my $value (@valuelist) {
+            $sortorder = $sortorder + 100;
+            my $isactive = !exists($defaultinactive{$value});
+            print "Inserting value '$value' in table $table" 
+                . " with sortkey $sortorder...\n";
+            $insert->execute($value, $sortorder, $isactive);
+        }
+    }
+}
 
-CheckEnumField('bugs', 'bug_severity', @my_severities);
-CheckEnumField('bugs', 'priority',     @my_priorities);
-CheckEnumField('bugs', 'op_sys',       @my_opsys);
-CheckEnumField('bugs', 'rep_platform', @my_platforms);
+# mkanat@kerio.com - bug 17453
+# Set default values for what used to be the enum types.
+# These values are no longer stored in localconfig.
+# However, if we are upgrading from a Bugzilla with enums to a 
+# Bugzilla without enums, we use the localconfig values one more time.
+
+# The values that you see here are ONLY DEFAULTS. They are only used
+# the FIRST time you run checksetup. After that, they are either 
+# controlled through the Bugzilla UI or through the DB.
+@my_severities = ('blocker','critical','major','normal','minor',
+                 'trivial','enhancement') if !@my_severities;
+@my_priorities = ("P1","P2","P3","P4","P5") if !@my_priorities;
+@my_opsys = ("---","All","Windows","Mac OS","Linux","Other") if !@my_opsys;
+@my_platforms = ("---","All","PC","Macintosh","Other") if !@my_platforms;
+
+PopulateEnumTable('bug_severity', @my_severities);
+PopulateEnumTable('priority', @my_priorities);
+PopulateEnumTable('op_sys', @my_opsys);
+PopulateEnumTable('rep_platform', @my_platforms);
+
+# The resolution and bug_status lists are absolute. On an upgrade from
+# a Bugzilla with enums, whatever is in the enum will be replaced with
+# this. This is because Bugzilla depends on the exact names of these 
+# resolutions in order to function properly.
+my @states = ("UNCONFIRMED","NEW","ASSIGNED","REOPENED","RESOLVED",
+             "VERIFIED","CLOSED");
+my @resolutions = ("","FIXED","INVALID","WONTFIX","LATER","REMIND",
+                  "DUPLICATE","WORKSFORME","MOVED");
+PopulateEnumTable('bug_status', @states);
+PopulateEnumTable('resolution', @resolutions);
 
 
 ###########################################################################
@@ -2936,16 +2928,6 @@ AddField('profiles', 'mybugslink', 'tinyint not null default 1');
 AddField('namedqueries', 'linkinfooter', 'tinyint not null');
 
 
-# 2000-02-12 Added a new state to bugs, UNCONFIRMED.  Added ability to confirm
-# a vote via bugs.  Added user bits to control which users can confirm bugs
-# by themselves, and which users can edit bugs without their names on them.
-# Added a user field which controls which groups a user can put other users 
-# into.
-
-my @resolutions = ("", "FIXED", "INVALID", "WONTFIX", "LATER", "REMIND",
-                  "DUPLICATE", "WORKSFORME", "MOVED");
-CheckEnumField('bugs', 'resolution', @resolutions);
-
 if (($_ = GetFieldDef('components', 'initialowner')) and ($_->[1] eq 'tinytext')) {
     $sth = $dbh->prepare("SELECT program, value, initialowner, initialqacontact FROM components");
     $sth->execute();
@@ -3005,11 +2987,6 @@ if (($_ = GetFieldDef('components', 'initialqacontact')) and ($_->[1] eq 'tinyte
 }
 
 
-
-my @states = ("UNCONFIRMED", "NEW", "ASSIGNED", "REOPENED", "RESOLVED",
-              "VERIFIED", "CLOSED");
-CheckEnumField('bugs', 'bug_status', @states);
-
 if (!GetFieldDef('bugs', 'everconfirmed')) {
     AddField('bugs', 'everconfirmed',  'tinyint not null');
     $dbh->do("UPDATE bugs SET everconfirmed = 1, delta_ts = delta_ts");
@@ -4276,6 +4253,16 @@ if (!GetFieldDef('longdescs', 'already_wrapped')) {
                   AND SUBSTRING(thetext FROM 1 FOR 80) LIKE '% %'});
 }
 
+# 2001-09-03 dkl@redhat.com bug 17453
+# Moved enum types to separate tables so we need change the old enum types to 
+# standard varchars in the bugs table.
+ChangeFieldType ('bugs', 'bug_status', 'varchar(64) not null');
+ChangeFieldType ('bugs', 'resolution', 'varchar(64) not null');
+ChangeFieldType ('bugs', 'priority', 'varchar(64) not null');
+ChangeFieldType ('bugs', 'bug_severity', 'varchar(64) not null');
+ChangeFieldType ('bugs', 'rep_platform', 'varchar(64) not null');
+ChangeFieldType ('bugs', 'op_sys', 'varchar(64) not null');
+
 # If you had to change the --TABLE-- definition in any way, then add your
 # differential change code *** A B O V E *** this comment.
 #
diff --git a/globals.pl b/globals.pl
index 9ea60c48b..f6f15566d 100644
--- a/globals.pl
+++ b/globals.pl
@@ -22,7 +22,9 @@
 #                 Jacob Steenhagen <jake@bugzilla.org>
 #                 Bradley Baetz <bbaetz@student.usyd.edu.au>
 #                 Christopher Aillon <christopher@aillon.com>
-#                 Joel Peshkin <bugreport@peshkin.net> 
+#                 Joel Peshkin <bugreport@peshkin.net>
+#                 Dave Lawrence <dkl@redhat.com>
+#                 Max Kanat-Alexander <mkanat@kerio.com>
 
 # Contains some global variables and routines used throughout bugzilla.
 
@@ -225,12 +227,12 @@ sub GenerateVersionTable {
     }
     @::log_columns = (sort(@::log_columns));
 
-    @::legal_priority = SplitEnumType($cols->{"priority,type"});
-    @::legal_severity = SplitEnumType($cols->{"bug_severity,type"});
-    @::legal_platform = SplitEnumType($cols->{"rep_platform,type"});
-    @::legal_opsys = SplitEnumType($cols->{"op_sys,type"});
-    @::legal_bug_status = SplitEnumType($cols->{"bug_status,type"});
-    @::legal_resolution = SplitEnumType($cols->{"resolution,type"});
+    @::legal_priority   = get_legal_field_values("priority");
+    @::legal_severity   = get_legal_field_values("bug_severity");
+    @::legal_platform   = get_legal_field_values("rep_platform");
+    @::legal_opsys      = get_legal_field_values("op_sys");
+    @::legal_bug_status = get_legal_field_values("bug_status");
+    @::legal_resolution = get_legal_field_values("resolution");
 
     # 'settable_resolution' is the list of resolutions that may be set 
     # directly by hand in the bug form. Start with the list of legal 
@@ -1026,22 +1028,13 @@ sub LearnAboutColumns {
     return \%a;
 }
 
-
-
-# If the above returned a enum type, take that type and parse it into the
-# list of values.  Assumes that enums don't ever contain an apostrophe!
-
-sub SplitEnumType {
-    my ($str) = (@_);
-    my @result = ();
-    if ($str =~ /^enum\((.*)\)$/) {
-        my $guts = $1 . ",";
-        while ($guts =~ /^\'([^\']*)\',(.*)$/) {
-            push @result, $1;
-            $guts = $2;
-        }
-    }
-    return @result;
+# Returns a list of all the legal values for a field that has a
+# list of legal values, like rep_platform or resolution.
+sub get_legal_field_values {
+    my ($field) = @_;
+    my $dbh = Bugzilla->dbh;
+    my $result_ref = $dbh->selectcol_arrayref("SELECT value FROM $field");
+    return @$result_ref;
 }
 
 sub UserInGroup {
diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index 163f0f5cb..d05c65b1f 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -20,6 +20,7 @@
 #
 # Contributor(s): Terry Weissman <terry@mozilla.org>
 #                 Matthew Tuck <matty@chariot.net.au>
+#                 Max Kanat-Alexander <mkanat@kerio.com>
 
 use strict;
 
@@ -200,29 +201,6 @@ if (defined $cgi->param('rescanallBugMail')) {
 print "OK, now running sanity checks.<p>\n";
 
 ###########################################################################
-# Check enumeration values
-###########################################################################
-
-# This one goes first, because if this is wrong, then the below tests
-# will probably fail too
-
-# This isn't extensible. Thats OK; we're not adding any more enum fields
-Status("Checking for invalid enumeration values");
-foreach my $field (("bug_severity", "bug_status", "op_sys",
-                    "priority", "rep_platform", "resolution")) {
-    # undefined enum values in mysql are an empty string which equals 0
-    SendSQL("SELECT bug_id FROM bugs WHERE $field=0 ORDER BY bug_id");
-    my @invalid;
-    while (MoreSQLData()) {
-        push (@invalid, FetchOneColumn());
-    }
-    if (@invalid) {
-        Alert("Bug(s) found with invalid $field value: ".
-              BugListLinks(@invalid));
-    }
-}
-
-###########################################################################
 # Perform referential (cross) checks
 ###########################################################################
 
@@ -352,6 +330,25 @@ CrossCheck("products", "id",
            ["flaginclusions", "product_id", "type_id"],
            ["flagexclusions", "product_id", "type_id"]);
 
+# Check the former enum types -mkanat@kerio.com
+CrossCheck("bug_status", "value",
+            ["bugs", "bug_status"]);
+
+CrossCheck("resolution", "value",
+            ["bugs", "resolution"]);
+
+CrossCheck("bug_severity", "value",
+            ["bugs", "bug_severity"]);
+
+CrossCheck("op_sys", "value",
+            ["bugs", "op_sys"]);
+
+CrossCheck("priority", "value",
+            ["bugs", "priority"]);
+
+CrossCheck("rep_platform", "value",
+            ["bugs", "rep_platform"]);
+
 CrossCheck('series', 'series_id',
            ['series_data', 'series_id']);