Bug 339667: Attachment data not deleted when deleting a component or a product - Patch by Frédéric Buclin <LpSolit@gmail.com> r=wicked a=justdave

2 files changed, 51 insertions, 3 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 185e50b44..810b1daf9 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -234,11 +234,15 @@ sub remove_from_db {
     # - longdescs
     # - votes
 
+    # Also, the attach_data table uses attachments.attach_id as a foreign
+    # key, and so indirectly depends on a bug deletion too.
+
     $dbh->bz_lock_tables('attachments WRITE', 'bug_group_map WRITE',
                          'bugs WRITE', 'bugs_activity WRITE', 'cc WRITE',
                          'dependencies WRITE', 'duplicates WRITE',
                          'flags WRITE', 'keywords WRITE',
-                         'longdescs WRITE', 'votes WRITE');
+                         'longdescs WRITE', 'votes WRITE',
+                         'attach_data WRITE');
 
     $dbh->do("DELETE FROM bug_group_map WHERE bug_id = ?", undef, $bug_id);
     $dbh->do("DELETE FROM bugs_activity WHERE bug_id = ?", undef, $bug_id);
@@ -251,6 +255,17 @@ sub remove_from_db {
     $dbh->do("DELETE FROM keywords WHERE bug_id = ?", undef, $bug_id);
     $dbh->do("DELETE FROM longdescs WHERE bug_id = ?", undef, $bug_id);
     $dbh->do("DELETE FROM votes WHERE bug_id = ?", undef, $bug_id);
+
+    # The attach_data table doesn't depend on bugs.bug_id directly.
+    my $attach_ids =
+        $dbh->selectcol_arrayref("SELECT attach_id FROM attachments
+                                  WHERE bug_id = ?", undef, $bug_id);
+
+    if (scalar(@$attach_ids)) {
+        $dbh->do("DELETE FROM attach_data WHERE id IN (" .
+                 join(",", @$attach_ids) . ")");
+    }
+
     # Several of the previous tables also depend on attach_id.
     $dbh->do("DELETE FROM attachments WHERE bug_id = ?", undef, $bug_id);
     $dbh->do("DELETE FROM bugs WHERE bug_id = ?", undef, $bug_id);
diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index 15861b9d8..b887deeff 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -249,7 +249,7 @@ if (defined $cgi->param('rescanallBugMail')) {
 # Remove all references to deleted bugs
 ###########################################################################
 
-if (defined $cgi->param('remove_invalid_references')) {
+if (defined $cgi->param('remove_invalid_bug_references')) {
     Status("OK, now removing all references to deleted bugs.");
 
     $dbh->bz_lock_tables('attachments WRITE', 'bug_group_map WRITE',
@@ -280,6 +280,30 @@ if (defined $cgi->param('remove_invalid_references')) {
     Status("All references to deleted bugs have been removed.");
 }
 
+###########################################################################
+# Remove all references to deleted attachments
+###########################################################################
+
+if (defined $cgi->param('remove_invalid_attach_references')) {
+    Status("OK, now removing all references to deleted attachments.");
+
+    $dbh->bz_lock_tables('attachments WRITE', 'attach_data WRITE');
+
+    my $attach_ids =
+        $dbh->selectcol_arrayref('SELECT attach_data.id
+                                    FROM attach_data
+                               LEFT JOIN attachments
+                                      ON attachments.attach_id = attach_data.id
+                                   WHERE attachments.attach_id IS NULL');
+
+    if (scalar(@$attach_ids)) {
+        $dbh->do('DELETE FROM attach_data WHERE id IN (' .
+                 join(',', @$attach_ids) . ')');
+    }
+
+    $dbh->bz_unlock_tables();
+    Status("All references to deleted attachments have been removed.");
+}
 
 print "OK, now running sanity checks.<p>\n";
 
@@ -348,7 +372,13 @@ sub CrossCheck {
         }
         # References to non existent bugs can be safely removed, bug 288461
         if ($table eq 'bugs' && $has_bad_references) {
-            print qq{<a href="sanitycheck.cgi?remove_invalid_references=1">Remove invalid references to non existent bugs.</a><p>\n};
+            print qq{<a href="sanitycheck.cgi?remove_invalid_bug_references=1">
+                     Remove invalid references to non existent bugs.</a><p>\n};
+        }
+        # References to non existent attachments can be safely removed.
+        if ($table eq 'attachments' && $has_bad_references) {
+            print qq{<a href="sanitycheck.cgi?remove_invalid_attach_references=1">
+                     Remove invalid references to non existent attachments.</a><p>\n};
         }
     }
 }
@@ -453,6 +483,9 @@ CrossCheck('whine_events', 'id',
            ['whine_queries', 'eventid'],
            ['whine_schedules', 'eventid']);
 
+CrossCheck('attachments', 'attach_id',
+           ['attach_data', 'id']);
+
 ###########################################################################
 # Perform double field referential (cross) checks
 ###########################################################################