Allow to disable change log (history) for attributes (#298)

Make it possible to exclude attributes from change log updates.

Some attributes, e.g. backup locking, are changed frequently but the change 
log of these irrelevant. They just bloat the change log. This commit makes it
possible to configure such attributes so that the changes are not logged.

.github/workflows/tests.yml

@@ -95,7 +95,7 @@ jobs:
         run: |
           # This will use Python's standard unit test discovery feature.
           pipenv run python -m unittest discover adminapi -v
-          pipenv run python -Wall -m serveradmin test --noinput --parallel
+          pipenv run python -Wall -m serveradmin test --noinput --parallel=1
           # Build sphinx docs, error on warning
           cd docs
           SPHINXBUILD='pipenv run sphinx-build' SPHINXOPTS='-W' make html

serveradmin/serverdb/fixtures/attribute.json

@@ -0,0 +1,70 @@
+[
+  {
+    "model": "serverdb.servertype",
+    "pk": "project",
+    "fields": {
+      "description": "Project information",
+      "ip_addr_type": "null"
+    }
+  },
+  {
+    "model": "serverdb.attribute",
+    "pk": "last_updated",
+    "fields": {
+      "type": "datetime",
+      "multi": false,
+      "hovertext": "",
+      "group": "project",
+      "help_link": null,
+      "readonly": false,
+      "target_servertype": null,
+      "reversed_attribute": null,
+      "clone": false,
+      "history": false,
+      "regexp": "\\A.*\\Z"
+    }
+  },
+  {
+    "model": "serverdb.attribute",
+    "pk": "owner",
+    "fields": {
+      "type": "string",
+      "multi": false,
+      "hovertext": "",
+      "group": "project",
+      "help_link": null,
+      "readonly": false,
+      "target_servertype": null,
+      "reversed_attribute": null,
+      "clone": false,
+      "history": true,
+      "regexp": "\\A.*\\Z"
+    }
+  },
+  {
+    "model": "serverdb.servertypeattribute",
+    "pk": 1,
+    "fields": {
+      "servertype": "project",
+      "attribute": "owner",
+      "related_via_attribute": null,
+      "consistent_via_attribute": null,
+      "required": false,
+      "default_value": null,
+      "default_visible": true
+    }
+  },
+  {
+    "model": "serverdb.servertypeattribute",
+    "pk": 2,
+    "fields": {
+      "servertype": "project",
+      "attribute": "last_updated",
+      "related_via_attribute": null,
+      "consistent_via_attribute": null,
+      "required": false,
+      "default_value": null,
+      "default_visible": true
+    }
+  }
+]

serveradmin/serverdb/migrations/0015_attribute_history_field.py

@@ -0,0 +1,19 @@
+# Generated by Django 3.2.18 on 2023-03-02 09:12
+
+from django.db import migrations, models
+import serveradmin.serverdb.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('serverdb', '0014_delete_deprecated_change_models'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='attribute',
+            name='history',
+            field=models.BooleanField(default=True, help_text='Log changes to this attribute')
+        ),
+    ]

serveradmin/serverdb/models.py

@@ -250,6 +250,9 @@ def __init__(self, *args, **kwargs):
         limit_choices_to=dict(type='relation'),
     )
     clone = models.BooleanField(null=False, default=False)
+    history = models.BooleanField(
+        null=False, default=True,
+        help_text='Log changes to this attribute')
     regexp = models.CharField(max_length=1024, validators=REGEX_VALIDATORS)
     _compiled_regexp = None
 

serveradmin/serverdb/query_committer.py

@@ -1,6 +1,6 @@
 """Serveradmin - Query Committer
 
-Copyright (c) 2022 InnoGames GmbH
+Copyright (c) 2023 InnoGames GmbH
 """
 
 import logging
@@ -80,7 +80,6 @@ def commit_query(created=[], changed=[], deleted=[], app=None, user=None):
     #       changes elsewhere by changing to the isolation level
     #       # "repeatable read".
     with transaction.atomic():
-        change_commit = ChangeCommit.objects.create(app=app, user=user)
         changed_servers = _fetch_servers(set(c['object_id'] for c in changed))
         unchanged_objects = _materialize(changed_servers, joined_attributes)
 
@@ -123,7 +122,7 @@ def commit_query(created=[], changed=[], deleted=[], app=None, user=None):
             created_objects, changed_objects, deleted_objects
         )
 
-        _log_changes(change_commit, changed, created_objects, deleted_objects)
+        _log_changes(user, app, changed, created_objects, deleted_objects)
 
     post_commit.send_robust(
         commit_query, created=created, changed=changed, deleted=deleted
@@ -433,30 +432,42 @@ def _acl_violations(changed_objects, obj, acl):
     return violations or None
 
 
-def _log_changes(commit, changed, created_objects, deleted_objects):
+def _log_changes(user, app, changed, created_objects, deleted_objects):
+    changes = list()
+    commit = ChangeCommit(user=user, app=app)
+
+    excl_attrs = Attribute.objects.filter(history=False).values_list(flat=True)
     for updates in changed:
-        Change.objects.create(
-            object_id=updates['object_id'],
-            change_type=Change.Type.CHANGE,
-            change_json=updates,
-            commit=commit,
-        )
+        # At least one attribute aside from object_id has changed.
+        if len(updates.keys() ^ excl_attrs) > 1:
+            # Get changes for attributes that should be logged.
+            to_log = {k: v for k, v in updates.items() if k not in excl_attrs}
+            changes.append(Change(
+                object_id=updates['object_id'],
+                change_type=Change.Type.CHANGE,
+                change_json=to_log,
+                commit=commit,
+            ))
 
     for attributes in deleted_objects.values():
-        Change.objects.create(
+        changes.append(Change(
             object_id=attributes['object_id'],
             change_type=Change.Type.DELETE,
             change_json=attributes,
             commit=commit,
-        )
+        ))
 
     for obj in created_objects.values():
-        Change.objects.create(
+        changes.append(Change(
             object_id=obj['object_id'],
             change_type=Change.Type.CREATE,
             change_json=obj,
             commit=commit,
-        )
+        ))
+
+    if changes:
+        commit.save()
+        Change.objects.bulk_create(changes)
 
 
 def _fetch_servers(object_ids):

serveradmin/serverdb/tests/test_attribute.py

@@ -0,0 +1,59 @@
+from django.contrib.auth.models import User
+from django.test import TransactionTestCase
+from django.utils.timezone import now
+from faker import Faker
+from faker.providers import internet
+
+from serveradmin.dataset import Query
+from serveradmin.serverdb.models import Change
+
+
+class TestAttributeHistory(TransactionTestCase):
+    fixtures = ['auth_user.json', 'attribute.json']
+
+    def setUp(self):
+        super().setUp()
+        self.faker = Faker()
+        self.faker.add_provider(internet)
+
+    def test_attribute_history_is_logged(self):
+        project = Query().new_object('project')
+        project['hostname'] = self.faker.hostname()
+        project['owner'] = 'john.doe' # Attribute with history enabled
+        project.commit(user=User.objects.first())
+
+        projects = Query({'hostname': project['hostname']}, ['owner'])
+        projects.update(owner='max.mustermann')
+        projects.commit(user=User.objects.first())
+        oid = projects.get()['object_id']
+
+        change = Change.objects.last()
+        self.assertEqual(change.change_json, {"owner": {"new": "max.mustermann", "old": "john.doe", "action": "update"}, "object_id": oid})
+
+    def test_attribute_history_is_not_logged(self):
+        project = Query().new_object('project')
+        project['hostname'] = self.faker.hostname()
+        project['last_updated'] = now()  # Attribute with history disabled
+        project.commit(user=User.objects.first())
+
+        projects = Query({'hostname': project['hostname']}, ['last_updated'])
+        projects.update(last_updated=now())
+        projects.commit(user=User.objects.first())
+        oid = projects.get()['object_id']
+
+        self.assertEqual(Change.objects.filter(change_type=Change.Type.CHANGE, object_id=oid).count(), 0)
+
+    def test_only_required_attribute_history_is_logged(self):
+        project = Query().new_object('project')
+        project['hostname'] = self.faker.hostname()
+        project['owner'] = 'john.doe'  # Attribute with history enabled
+        project['last_updated'] = now()  # Attribute with history disabled
+        project.commit(user=User.objects.first())
+
+        projects = Query({'hostname': project['hostname']}, ['owner', 'last_updated'])
+        projects.update(owner='max.mustermann', last_updated=now())
+        projects.commit(user=User.objects.first())
+        oid = projects.get()['object_id']
+
+        change = Change.objects.last()
+        self.assertEqual(change.change_json, {"owner": {"new": "max.mustermann", "old": "john.doe", "action": "update"}, "object_id": oid})