Modify change field to be a json field. (#407)

* Modify ``change`` field to be a json field.

Storing the object changes as a json is preferred because it allows SQL
queries to access the change values. This work moves the burden of
handling json objects from an implementation of python's json library in
this package and puts it instead onto the ORM. Ultimately, having the
text field store the changes was leaving them less accessible to external
systems and code that is written outside the scope of the django
auditlog.

This change was accomplished by updating the field type on the model and
then removing the JSON dumps invocations on write and JSON loads
invocations on read. Test were updated to assert equality of
dictionaries rather than equality of JSON parsable text.

Separately, it was asserted that postgres will make these changes to
existing data. Therefore, existing postgres installations should update the
type of existing field values without issue.

* Add test coverage for messages exceeding char len

The "Modify change field to be a json field" commit reduced test
coverage on the mixins.py file by 0.03%. The reduction in coverage was
the result of reducing the number of operations required to achieve the
desired state. An additional test was added to increase previously
uncovered code. The net effect is an increase in test case coverage.

* Add line to changelog

Better markdown formatting

Co-authored-by: Hasan Ramezani <hasan.r67@gmail.com>

* Update CHANGELOG text format

More specific language in the improvement section regarding `LogEntry.change`

Co-authored-by: Hasan Ramezani <hasan.r67@gmail.com>

* Update migration to show Django version 4.0

Co-authored-by: Hasan Ramezani <hasan.r67@gmail.com>

* Update CHANGELOG to show breaking change

Running the migration to update the field type of `LogEntry.change` is a breaking change.

Co-authored-by: Hasan Ramezani <hasan.r67@gmail.com>

* Update serial order of migrations

* Adjust manager method for compatibility

The create log method on the LogEntry manager required an additional
kwarg for a call to create an instance regardless of a change or not.
This felt brittle anyway. The reason it had worked prior to these
changes was that the `change` kwarg was sending a string "null" and
not a None when there were no changes.

Co-authored-by: Hasan Ramezani <hasan.r67@gmail.com>

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changes
 
+#### Breaking Changes
+
+- feat: Change `LogEntry.change` field type to `JSONField` rather than `TextField`. This change include a migration that may take time to run depending on the number of records on your `LogEntry` table ([#407](https://github.com/jazzband/django-auditlog/pull/407))
+
 ## Next Release
 
 #### Improvements

auditlog/migrations/0015_alter_logentry_changes.py

@@ -0,0 +1,18 @@
+# Generated by Django 4.0 on 2022-08-04 15:41
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("auditlog", "0014_logentry_cid"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="logentry",
+            name="changes",
+            field=models.JSONField(null=True, verbose_name="change message"),
+        ),
+    ]

auditlog/models.py

@@ -26,13 +26,15 @@ class LogEntryManager(models.Manager):
     Custom manager for the :py:class:`LogEntry` model.
     """
 
-    def log_create(self, instance, **kwargs):
+    def log_create(self, instance, force_log: bool = False, **kwargs):
         """
         Helper method to create a new log entry. This method automatically populates some fields when no
         explicit value is given.
 
         :param instance: The model instance to log a change for.
         :type instance: Model
+        :param force_log: Create a LogEntry even if no changes exist.
+        :type force_log: bool
         :param kwargs: Field overrides for the :py:class:`LogEntry` object.
         :return: The new log entry or `None` if there were no changes.
         :rtype: LogEntry
@@ -42,7 +44,7 @@ def log_create(self, instance, **kwargs):
         changes = kwargs.get("changes", None)
         pk = self._get_pk_value(instance)
 
-        if changes is not None:
+        if changes is not None or force_log:
             kwargs.setdefault(
                 "content_type", ContentType.objects.get_for_model(instance)
             )
@@ -350,7 +352,7 @@ class Action:
     action = models.PositiveSmallIntegerField(
         choices=Action.choices, verbose_name=_("action"), db_index=True
     )
-    changes = models.TextField(blank=True, verbose_name=_("change message"))
+    changes = models.JSONField(null=True, verbose_name=_("change message"))
     actor = models.ForeignKey(
         to=settings.AUTH_USER_MODEL,
         on_delete=models.SET_NULL,
@@ -399,10 +401,7 @@ def changes_dict(self):
         """
         :return: The changes recorded in this log entry as a dictionary object.
         """
-        try:
-            return json.loads(self.changes) or {}
-        except ValueError:
-            return {}
+        return self.changes or {}
 
     @property
     def changes_str(self, colon=": ", arrow=" \u2192 ", separator="; "):

auditlog/receivers.py

@@ -1,4 +1,3 @@
-import json
 from functools import wraps
 
 from django.conf import settings
@@ -115,7 +114,8 @@ def _create_log_entry(
             LogEntry.objects.log_create(
                 instance,
                 action=action,
-                changes=json.dumps(changes),
+                changes=changes,
+                force_log=force_log,
             )
     except BaseException as e:
         error = e

auditlog_tests/tests.py

@@ -105,9 +105,9 @@ def update(self, obj):
         obj.save()
 
     def check_update_log_entry(self, obj, history):
-        self.assertJSONEqual(
+        self.assertDictEqual(
             history.changes,
-            '{"boolean": ["False", "True"]}',
+            {"boolean": ["False", "True"]},
             msg="The change is correctly logged",
         )
 
@@ -120,9 +120,9 @@ def test_update_specific_field_supplied_via_save_method(self):
         obj.save(update_fields=["boolean"])
 
         # This implicitly asserts there is only one UPDATE change since the `.get` would fail otherwise.
-        self.assertJSONEqual(
+        self.assertDictEqual(
             obj.history.get(action=LogEntry.Action.UPDATE).changes,
-            '{"boolean": ["False", "True"]}',
+            {"boolean": ["False", "True"]},
             msg=(
                 "Object modifications that are not saved to DB are not logged "
                 "when using the `update_fields`."
@@ -153,9 +153,9 @@ def test_django_update_fields_edge_cases(self):
         obj.integer = 1
         obj.boolean = True
         obj.save(update_fields=None)
-        self.assertJSONEqual(
+        self.assertDictEqual(
             obj.history.get(action=LogEntry.Action.UPDATE).changes,
-            '{"boolean": ["False", "True"], "integer": ["None", "1"]}',
+            {"boolean": ["False", "True"], "integer": ["None", "1"]},
             msg="The 2 fields changed are correctly logged",
         )
 
@@ -537,9 +537,9 @@ def test_specified_save_fields_are_ignored_if_not_included(self):
         obj.text = "Newer text"
         obj.save(update_fields=["text", "label"])
 
-        self.assertJSONEqual(
+        self.assertDictEqual(
             obj.history.get(action=LogEntry.Action.UPDATE).changes,
-            '{"label": ["Initial label", "New label"]}',
+            {"label": ["Initial label", "New label"]},
             msg="Only the label was logged, regardless of multiple entries in `update_fields`",
         )
 
@@ -1395,7 +1395,27 @@ def _create_log_entry(self, action, changes):
         return LogEntry.objects.log_create(
             SimpleModel.objects.create(),  # doesn't affect anything
             action=action,
-            changes=json.dumps(changes),
+            changes=changes,
+        )
+
+    def test_change_msg_create_when_exceeds_max_len(self):
+        log_entry = self._create_log_entry(
+            LogEntry.Action.CREATE,
+            {
+                "Camelopardalis": [None, "Giraffe"],
+                "Capricornus": [None, "Sea goat"],
+                "Equuleus": [None, "Little horse"],
+                "Horologium": [None, "Clock"],
+                "Microscopium": [None, "Microscope"],
+                "Reticulum": [None, "Net"],
+                "Telescopium": [None, "Telescope"],
+            },
+        )
+
+        self.assertEqual(
+            self.admin.msg_short(log_entry),
+            "7 changes: Camelopardalis, Capricornus, Equuleus, Horologium, "
+            "Microscopium, ..",
         )
 
     def test_changes_msg_delete(self):
@@ -1569,9 +1589,9 @@ def test_update(self):
 
         history = obj.history.get(action=LogEntry.Action.UPDATE)
 
-        self.assertJSONEqual(
+        self.assertDictEqual(
             history.changes,
-            '{"json": ["{}", "{\'quantity\': \'1\'}"]}',
+            {"json": ["{}", "{'quantity': '1'}"]},
             msg="The change is correctly logged",
         )
 
@@ -1910,7 +1930,7 @@ def test_access_log(self):
         self.assertEqual(
             log_entry.action, LogEntry.Action.ACCESS, msg="Action is 'ACCESS'"
         )
-        self.assertEqual(log_entry.changes, "null")
+        self.assertIsNone(log_entry.changes)
         self.assertEqual(log_entry.changes_dict, {})