Bug 622943: Simple auditing of changes to Bugzilla::Object subclass objects

r=dkl, a=mkanat

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 2bd5d8f3cf3778f113154e7a31e7af7384473a4b..c0ea6ca0d9f697026aae90524daa46dba6c0e980 100644 (file)
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -71,6 +71,8 @@ use base qw(Bugzilla::Object);
 use constant DB_TABLE   => 'attachments';
 use constant ID_FIELD   => 'attach_id';
 use constant LIST_ORDER => ID_FIELD;
+# Attachments are tracked in bugs_activity.
+use constant AUDIT_UPDATES => 0;
 
 sub DB_COLUMNS {
     my $dbh = Bugzilla->dbh;

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 42b316516279b075284efabd9f1e7d675c9af1fc..beafcfa8c3f11e3f507e7901849dbc0afa9a7141 100644 (file)
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -73,6 +73,8 @@ use constant DB_TABLE   => 'bugs';
 use constant ID_FIELD   => 'bug_id';
 use constant NAME_FIELD => 'alias';
 use constant LIST_ORDER => ID_FIELD;
+# Bugs have their own auditing table, bugs_activity.
+use constant AUDIT_UPDATES => 0;
 
 # This is a sub because it needs to call other subroutines.
 sub DB_COLUMNS {

diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm
index f3628ddb14dae76379979a9d9edff0fed454b8bc..1d42f04c634c1767fbd680b26c8fa1bc587ba24d 100644 (file)
--- a/Bugzilla/Comment.pm
+++ b/Bugzilla/Comment.pm
@@ -37,6 +37,9 @@ use Scalar::Util qw(blessed);
 ####    Initialization     ####
 ###############################
 
+# Updates of comments are audited in bugs_activity instead of audit_log.
+use constant AUDIT_UPDATES => 0;
+
 use constant DB_COLUMNS => qw(
     comment_id
     bug_id

diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 11521749e3468947cff6d3632f05e45cd24b7004..7001de0e842a0aafd85bacc764bf37db8882e4ff 100644 (file)
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -189,6 +189,9 @@ use Memoize;
     PRIVILEGES_REQUIRED_REPORTER
     PRIVILEGES_REQUIRED_ASSIGNEE
     PRIVILEGES_REQUIRED_EMPOWERED
+
+    AUDIT_CREATE
+    AUDIT_REMOVE
 );
 
 @Bugzilla::Constants::EXPORT_OK = qw(contenttypes);
@@ -575,6 +578,11 @@ use constant PRIVILEGES_REQUIRED_REPORTER  => 1;
 use constant PRIVILEGES_REQUIRED_ASSIGNEE  => 2;
 use constant PRIVILEGES_REQUIRED_EMPOWERED => 3;
 
+# Special field values used in the audit_log table to mean either
+# "we just created this object" or "we just deleted this object".
+use constant AUDIT_CREATE => '__create__';
+use constant AUDIT_REMOVE => '__remove__';
+
 sub bz_locations {
     # We know that Bugzilla/Constants.pm must be in %INC at this point.
     # So the only question is, what's the name of the directory

diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index 106d316a56ce8e8219bc9700a612e1c3539790ac..c5b16b68ef069fb1e641de0b474680a8789235dd 100644 (file)
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -507,6 +507,23 @@ use constant ABSTRACT_SCHEMA => {
         ],
     },
 
+    # Auditing
+    # --------
+
+    audit_log => {
+        FIELDS => [
+            user_id   => {TYPE => 'INT3',
+                          REFERENCES => {TABLE  => 'profiles',
+                                         COLUMN => 'userid'}},
+            class     => {TYPE => 'varchar(255)', NOTNULL => 1},
+            object_id => {TYPE => 'INT4', NOTNULL => 1},
+            field     => {TYPE => 'varchar(64)', NOTNULL => 1},
+            removed   => {TYPE => 'MEDIUMTEXT'},
+            added     => {TYPE => 'MEDIUMTEXT'},
+            at_time   => {TYPE => 'DATETIME', NOTNULL => 1},
+        ],
+    },
+
     # Keywords
     # --------
 

diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 13dfe6ad9c05250cc796e9ad79c37174fc690e96..c4bd6fef65f41d4a56899256364329342c906e74 100644 (file)
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -74,6 +74,8 @@ use base qw(Bugzilla::Object Exporter);
 
 use constant DB_TABLE => 'flags';
 use constant LIST_ORDER => 'id';
+# Flags are tracked in bugs_activity.
+use constant AUDIT_UPDATES => 0;
 
 use constant SKIP_REQUESTEE_ON_ERROR => 1;
 

diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 6c52894536d9b6e1e164943f32e734e360db72c4..1cad3e829008e52033d82bdacace49292c414d0b 100644 (file)
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -43,6 +43,7 @@ use constant VALIDATOR_DEPENDENCIES => {};
 # XXX At some point, this will be joined with FIELD_MAP.
 use constant REQUIRED_FIELD_MAP  => {};
 use constant EXTRA_REQUIRED_FIELDS => ();
+use constant AUDIT_UPDATES => 1;
 
 # This allows the JSON-RPC interface to return Bugzilla::Object instances
 # as though they were hashes. In the future, this may be modified to return
@@ -392,6 +393,8 @@ sub update {
                             { object => $self, old_object => $old_self,
                               changes => \%changes });
 
+    $self->audit_log(\%changes) if $self->AUDIT_UPDATES;
+
     $dbh->bz_commit_transaction();
 
     if (wantarray) {
@@ -406,11 +409,43 @@ sub remove_from_db {
     Bugzilla::Hook::process('object_before_delete', { object => $self });
     my $table = $self->DB_TABLE;
     my $id_field = $self->ID_FIELD;
-    Bugzilla->dbh->do("DELETE FROM $table WHERE $id_field = ?",
-                      undef, $self->id);
+    my $dbh = Bugzilla->dbh;
+    $dbh->bz_start_transaction();
+    $self->audit_log(AUDIT_REMOVE);
+    $dbh->do("DELETE FROM $table WHERE $id_field = ?", undef, $self->id);
+    $dbh->bz_commit_transaction();
     undef $self;
 }
 
+sub audit_log {
+    my ($self, $changes) = @_;
+    my $class = ref $self;
+    my $dbh = Bugzilla->dbh;
+    my $user_id = Bugzilla->user->id || undef;
+    my $sth = $dbh->prepare(
+        'INSERT INTO audit_log (user_id, class, object_id, field,
+                                removed, added, at_time) 
+              VALUES (?,?,?,?,?,?,LOCALTIMESTAMP(0))');
+    # During creation or removal, $changes is actually just a string
+    # indicating whether we're creating or removing the object.
+    if ($changes eq AUDIT_CREATE or $changes eq AUDIT_REMOVE) {
+        # We put the object's name in the "added" or "removed" field.
+        # We do this thing with NAME_FIELD because $self->name returns
+        # the wrong thing for Bugzilla::User.
+        my $name = $self->{$self->NAME_FIELD};
+        my @added_removed = $changes eq AUDIT_CREATE ? (undef, $name) 
+                                                     : ($name, undef);
+        $sth->execute($user_id, $class, $self->id, $changes, @added_removed);
+        return;
+    }
+
+    # During update, it's the actual %changes hash produced by update().
+    foreach my $field (keys %$changes) {
+        my ($from, $to) = @{ $changes->{$field} };
+        $sth->execute($user_id, $class, $self->id, $field, $from, $to);
+    }
+}
+
 ###############################
 ####      Subroutines    ######
 ###############################
@@ -522,6 +557,8 @@ sub insert_create_data {
 
     Bugzilla::Hook::process('object_end_of_create', { class => $class,
                                                       object => $object });
+    $object->audit_log(AUDIT_CREATE);
+
     return $object;
 }