Bug 373442: Add referential integrity against the profiles table in some more simple places

Bugzilla/DB.pm

@@ -43,6 +43,7 @@ use Bugzilla::Error;
 use Bugzilla::DB::Schema;
 
 use List::Util qw(max);
+use Storable qw(dclone);
 
 #####################################################################
 # Constants
@@ -428,6 +429,23 @@ sub bz_populate_enum_tables {
     }
 }
 
+sub bz_setup_foreign_keys {
+    my ($self) = @_;
+
+    # We use _bz_schema because bz_add_table has removed all REFERENCES
+    # items from _bz_real_schema.
+    my @tables = $self->_bz_schema->get_table_list();
+    foreach my $table (@tables) {
+        my @columns = $self->_bz_schema->get_table_columns($table);
+        foreach my $column (@columns) {
+            my $def = $self->_bz_schema->get_column_abstract($table, $column);
+            if ($def->{REFERENCES}) {
+                $self->bz_add_fk($table, $column, $def->{REFERENCES});
+            }
+        }
+    }
+}
+
 #####################################################################
 # Schema Modification Methods
 #####################################################################
@@ -463,6 +481,24 @@ sub bz_add_column {
     }
 }
 
+sub bz_add_fk {
+    my ($self, $table, $column, $def) = @_;
+
+    my $col_def = $self->bz_column_info($table, $column);
+    if (!$col_def->{REFERENCES}) {
+        $self->_check_references($table, $column, $def->{TABLE},
+                                 $def->{COLUMN});
+        print get_text('install_fk_add',
+                       { table => $table, column => $column, fk => $def }) 
+            . "\n" if Bugzilla->usage_mode == USAGE_MODE_CMDLINE;
+        my @sql = $self->_bz_real_schema->get_add_fk_sql($table, $column, $def);
+        $self->do($_) foreach @sql;
+        $col_def->{REFERENCES} = $def;
+        $self->_bz_real_schema->set_column($table, $column, $col_def);
+        $self->_bz_store_real_schema;
+    }
+}
+
 sub bz_alter_column {
     my ($self, $table, $name, $new_def, $set_nulls_to) = @_;
 
@@ -515,11 +551,10 @@ sub bz_alter_column_raw {
     my @statements = $self->_bz_real_schema->get_alter_column_ddl(
         $table, $name, $new_def,
         defined $set_nulls_to ? $self->quote($set_nulls_to) : undef);
-    my $new_ddl = $self->_bz_schema->get_display_ddl($table, $name, $new_def);
+    my $new_ddl = $self->_bz_schema->get_type_ddl($new_def);
     print "Updating column $name in table $table ...\n";
     if (defined $current_def) {
-        my $old_ddl = $self->_bz_schema->get_display_ddl($table, $name, 
-                                                         $current_def);
+        my $old_ddl = $self->_bz_schema->get_type_ddl($current_def);
         print "Old: $old_ddl\n";
     }
     print "New: $new_ddl\n";
@@ -569,8 +604,17 @@ sub bz_add_table {
 
     if (!$table_exists) {
         $self->_bz_add_table_raw($name);
-        $self->_bz_real_schema->add_table($name,
-            $self->_bz_schema->get_table_abstract($name));
+        my $table_def = dclone($self->_bz_schema->get_table_abstract($name));
+
+        my %fields = @{$table_def->{FIELDS}};
+        foreach my $col (keys %fields) {
+            # Foreign Key references have to be added by Install::DB after
+            # initial table creation, because column names have changed
+            # over history and it's impossible to keep track of that info
+            # in ABSTRACT_SCHEMA.
+            delete $fields{$col}->{REFERENCES};
+        }
+        $self->_bz_real_schema->add_table($name, $table_def);
         $self->_bz_store_real_schema;
     }
 }
@@ -751,7 +795,10 @@ sub _bz_get_initial_schema {
 
 sub bz_column_info {
     my ($self, $table, $column) = @_;
-    return $self->_bz_real_schema->get_column_abstract($table, $column);
+    my $def = $self->_bz_real_schema->get_column_abstract($table, $column);
+    # We dclone it so callers can't modify the Schema.
+    $def = dclone($def) if defined $def;
+    return $def;
 }
 
 sub bz_index_info {
@@ -1050,6 +1097,39 @@ sub _bz_populate_enum_table {
     }
 }
 
+# This is used before adding a foreign key to a column, to make sure
+# that the database won't fail adding the key.
+sub _check_references {
+    my ($self, $table, $column, $foreign_table, $foreign_column) = @_;
+
+    my $bad_values = $self->selectcol_arrayref(
+        "SELECT DISTINCT $table.$column 
+           FROM $table LEFT JOIN $foreign_table
+                ON $table.$column = $foreign_table.$foreign_column
+          WHERE $foreign_table.$foreign_column IS NULL
+                AND $table.$column IS NOT NULL");
+
+    if (@$bad_values) {
+        my $values = join(', ', @$bad_values);
+        print <<EOT;
+
+ERROR: There are invalid values for the $column column in the $table 
+table. (These values do not exist in the $foreign_table table, in the 
+$foreign_column column.)
+
+Before continuing with checksetup, you will need to fix these values,
+either by deleting these rows from the database, or changing the values
+of $column in $table to point to valid values in $foreign_table.$foreign_column.
+
+The bad values from the $table.$column column are:
+$values
+
+EOT
+        # I just picked a number above 2, to be considered "abnormal exit."
+        exit 3;
+    }
+}
+
 1;
 
 __END__

Bugzilla/DB/Schema/Mysql.pm

@@ -176,17 +176,10 @@ sub get_alter_column_ddl {
         # keys are not allowed.
         delete $new_def_copy{PRIMARYKEY};
     }
-    # CHANGE COLUMN doesn't support REFERENCES
-    delete $new_def_copy{REFERENCES};
 
     my $new_ddl = $self->get_type_ddl(\%new_def_copy);
     my @statements;
 
-    # Drop the FK if the new definition doesn't have one.
-    if ($old_def->{REFERENCES} && !$new_def->{REFERENCES}) {
-        push(@statements, $self->_get_drop_fk_sql($table, $column, $old_def));
-    }
-
     push(@statements, "UPDATE $table SET $column = $set_nulls_to
                         WHERE $column IS NULL") if defined $set_nulls_to;
     push(@statements, "ALTER TABLE $table CHANGE COLUMN 
@@ -196,10 +189,6 @@ sub get_alter_column_ddl {
         push(@statements, "ALTER TABLE $table DROP PRIMARY KEY");
     }
 
-    # Add the FK if the new definition has one and the old definition doesn't.
-    if ($new_def->{REFERENCES} && !$old_def->{REFERENCES}) {
-        push(@statements, $self->_get_add_fk_sql($table, $column, $new_def));
-    }
     return @statements;
 }
 

Bugzilla/Install/DB.pm

@@ -49,39 +49,6 @@ sub indicate_progress {
     }
 }
 
-# This is used before adding a foreign key to a column, to make sure
-# that the database won't fail adding the key.
-sub check_references {
-    my ($table, $column, $foreign_table, $foreign_column) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $bad_values = $dbh->selectcol_arrayref(
-        "SELECT DISTINCT $table.$column 
-           FROM $table LEFT JOIN $foreign_table
-                ON $table.$column = $foreign_table.$foreign_column
-          WHERE $foreign_table.$foreign_column IS NULL");
-
-    if (@$bad_values) {
-        my $values = join(', ', @$bad_values);
-        print <<EOT;
-
-ERROR: There are invalid values for the $column column in the $table 
-table. (These values do not exist in the $foreign_table table, in the 
-$foreign_column column.)
-
-Before continuing with checksetup, you will need to fix these values,
-either by deleting these rows from the database, or changing the values
-of $column in $table to point to valid values in $foreign_table.$foreign_column.
-
-The bad values from the $table.$column column are:
-$values
-
-EOT
-        # I just picked a number above 2, to be considered "abnormal exit."
-        exit 3;
-    }
-}
-
 # NOTE: This is NOT the function for general table updates. See
 # update_table_definitions for that. This is only for the fielddefs table.
 sub update_fielddefs_definition {
@@ -413,9 +380,9 @@ sub update_table_definitions {
     if ($dbh->bz_column_info('components', 'initialqacontact')->{NOTNULL}) {
         $dbh->bz_alter_column('components', 'initialqacontact', 
                               {TYPE => 'INT3'});
+    }
     $dbh->do("UPDATE components SET initialqacontact = NULL " .
               "WHERE initialqacontact = 0");
-    }
 
     _migrate_email_prefs_to_new_table();
     _initialize_dependency_tree_changes_email_pref();
@@ -552,25 +519,13 @@ sub update_table_definitions {
     $dbh->bz_add_column('milestones', 'id',
         {TYPE => 'MEDIUMSERIAL', NOTNULL => 1, PRIMARYKEY => 1});
 
-    # Referential Integrity begins here
-    check_references('profiles_activity', 'userid', 'profiles', 'userid');
-    $dbh->bz_alter_column('profiles_activity', 'userid',
-        {TYPE => 'INT3', NOTNULL => 1,  REFERENCES =>
-        {TABLE  => 'profiles', COLUMN => 'userid', DELETE => 'CASCADE'}});
-    check_references('profiles_activity', 'who', 'profiles', 'userid');
-    $dbh->bz_alter_column('profiles_activity', 'who',
-        {TYPE => 'INT3', NOTNULL => 1, REFERENCES =>
-        {TABLE  => 'profiles', COLUMN => 'userid'}});
-    check_references('profiles_activity', 'fieldid', 'fielddefs', 'id');
-    $dbh->bz_alter_column('profiles_activity', 'fieldid',
-        {TYPE => 'INT3', NOTNULL => 1, REFERENCES =>
-        {TABLE  => 'fielddefs', COLUMN => 'id'}});
-
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
     ################################################################
 
     Bugzilla::Hook::process('install-update_db');
+
+    $dbh->bz_setup_foreign_keys();
 }
 
 # Subroutines should be ordered in the order that they are called.

template/en/default/global/messages.html.tmpl

@@ -382,6 +382,9 @@
   [% ELSIF message_tag == "install_file_perms_fix" %]
     Fixing file permissions...
 
+  [% ELSIF message_tag == "install_fk_add" %]
+    Adding foreign key: [% table FILTER html %].[% column FILTER html %] -> [% fk.TABLE FILTER html %].[% fk.COLUMN FILTER html %]...
+
   [% ELSIF message_tag == "install_group_create" %]
     Creating group [% name FILTER html %]...