From b640df67a35e456143e3cced354dbaf2f12c2b6f Mon Sep 17 00:00:00 2001 From: The Alchemist Date: Wed, 30 Apr 2025 05:20:27 -0400 Subject: [PATCH] new setting: STORE_JSON_CHANGES that intelligently store JSON (#719) * Branch that implements issue #675, basically, storing the correct JSON type that corresponds to the Python type (None -> null, 1 -> 1', not "1"`). It's driven by a setting, AUDITLOG_ STORE_JSON_CHANGES , as recommended by @hramezani * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * code formatting tweaks from @hramezani * increasing test coverage * added usage for AUDITLOG_STORE_JSON_CHANGES setting * updated CHANGELOG with info on AUDITLOG_STORE_JSON_CHANGES field * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * added another test for wrong setting type * should not have committed temporary test changes * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 + auditlog/conf.py | 5 + auditlog/diff.py | 55 ++++++++-- auditlog/receivers.py | 15 ++- auditlog/registry.py | 3 + auditlog_tests/test_use_json_for_changes.py | 115 ++++++++++++++++++++ docs/source/usage.rst | 9 ++ 7 files changed, 190 insertions(+), 14 deletions(-) create mode 100644 auditlog_tests/test_use_json_for_changes.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 37b2f3f..274f48a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ #### Improvements +- feat: Support storing JSON in the changes field when ```AUDITLOG_STORE_JSON_CHANGES``` is enabled. ([#719](https://github.com/jazzband/django-auditlog/pull/719)) + #### Fixes ## 3.1.2 (2025-04-26) diff --git a/auditlog/conf.py b/auditlog/conf.py index dceedd1..01d9f29 100644 --- a/auditlog/conf.py +++ b/auditlog/conf.py @@ -55,3 +55,8 @@ settings.AUDITLOG_DISABLE_REMOTE_ADDR = getattr( settings.AUDITLOG_CHANGE_DISPLAY_TRUNCATE_LENGTH = getattr( settings, "AUDITLOG_CHANGE_DISPLAY_TRUNCATE_LENGTH", 140 ) + +# Use pure JSON for changes field +settings.AUDITLOG_STORE_JSON_CHANGES = getattr( + settings, "AUDITLOG_STORE_JSON_CHANGES", False +) diff --git a/auditlog/diff.py b/auditlog/diff.py index 2fd44a9..5a46731 100644 --- a/auditlog/diff.py +++ b/auditlog/diff.py @@ -51,7 +51,7 @@ def get_fields_in_model(instance): return [f for f in instance._meta.get_fields() if track_field(f)] -def get_field_value(obj, field): +def get_field_value(obj, field, use_json_for_changes=False): """ Gets the value of a given model instance field. @@ -79,18 +79,23 @@ def get_field_value(obj, field): value = django_timezone.make_naive(value, timezone=timezone.utc) elif isinstance(field, JSONField): value = field.to_python(getattr(obj, field.name, None)) - try: - value = json.dumps(value, sort_keys=True, cls=field.encoder) - except TypeError: - pass + if not use_json_for_changes: + try: + value = json.dumps(value, sort_keys=True, cls=field.encoder) + except TypeError: + pass elif (field.one_to_one or field.many_to_one) and hasattr(field, "rel_class"): value = smart_str( getattr(obj, field.get_attname(), None), strings_only=True ) else: - value = smart_str(getattr(obj, field.name, None)) - if type(value).__name__ == "__proxy__": - value = str(value) + value = getattr(obj, field.name, None) + + if not use_json_for_changes: + value = smart_str(value) + if type(value).__name__ == "__proxy__": + value = str(value) + except ObjectDoesNotExist: value = ( field.default @@ -101,6 +106,18 @@ def get_field_value(obj, field): return value +def is_primitive(obj) -> bool: + """ + Checks if the given object is a primitive Python type that can be safely serialized to JSON. + + :param obj: The object to check + :return: True if the object is a primitive type, False otherwise + :rtype: bool + """ + primitive_types = (type(None), bool, int, float, str, list, tuple, dict, set) + return isinstance(obj, primitive_types) + + def mask_str(value: str) -> str: """ Masks the first half of the input string to remove sensitive data. @@ -115,7 +132,10 @@ def mask_str(value: str) -> str: def model_instance_diff( - old: Optional[Model], new: Optional[Model], fields_to_check=None + old: Optional[Model], + new: Optional[Model], + fields_to_check=None, + use_json_for_changes=False, ): """ Calculates the differences between two model instances. One of the instances may be ``None`` @@ -189,8 +209,8 @@ def model_instance_diff( fields = filtered_fields for field in fields: - old_value = get_field_value(old, field) - new_value = get_field_value(new, field) + old_value = get_field_value(old, field, use_json_for_changes) + new_value = get_field_value(new, field, use_json_for_changes) if old_value != new_value: if model_fields and field.name in model_fields["mask_fields"]: @@ -199,7 +219,18 @@ def model_instance_diff( mask_str(smart_str(new_value)), ) else: - diff[field.name] = (smart_str(old_value), smart_str(new_value)) + if not use_json_for_changes: + diff[field.name] = (smart_str(old_value), smart_str(new_value)) + else: + # TODO: should we handle the case where the value is a django Model specifically? + # for example, could create a list of ids for ManyToMany fields + + # this maintains the behavior of the original code + if not is_primitive(old_value): + old_value = smart_str(old_value) + if not is_primitive(new_value): + new_value = smart_str(new_value) + diff[field.name] = (old_value, new_value) if len(diff) == 0: diff = None diff --git a/auditlog/receivers.py b/auditlog/receivers.py index b1fc817..acc02e8 100644 --- a/auditlog/receivers.py +++ b/auditlog/receivers.py @@ -43,6 +43,7 @@ def log_create(sender, instance, created, **kwargs): sender=sender, diff_old=None, diff_new=instance, + use_json_for_changes=settings.AUDITLOG_STORE_JSON_CHANGES, ) @@ -101,7 +102,14 @@ def log_access(sender, instance, **kwargs): def _create_log_entry( - action, instance, sender, diff_old, diff_new, fields_to_check=None, force_log=False + action, + instance, + sender, + diff_old, + diff_new, + fields_to_check=None, + force_log=False, + use_json_for_changes=False, ): pre_log_results = pre_log.send( sender, @@ -117,7 +125,10 @@ def _create_log_entry( changes = None try: changes = model_instance_diff( - diff_old, diff_new, fields_to_check=fields_to_check + diff_old, + diff_new, + fields_to_check=fields_to_check, + use_json_for_changes=use_json_for_changes, ) if force_log or changes: diff --git a/auditlog/registry.py b/auditlog/registry.py index b472f72..2da9997 100644 --- a/auditlog/registry.py +++ b/auditlog/registry.py @@ -371,6 +371,9 @@ class AuditlogModelRegistry: model=model, m2m_fields=m2m_fields, exclude_fields=exclude_fields ) + if not isinstance(settings.AUDITLOG_STORE_JSON_CHANGES, bool): + raise TypeError("Setting 'AUDITLOG_STORE_JSON_CHANGES' must be a boolean") + self._register_models(settings.AUDITLOG_INCLUDE_TRACKING_MODELS) diff --git a/auditlog_tests/test_use_json_for_changes.py b/auditlog_tests/test_use_json_for_changes.py new file mode 100644 index 0000000..b9b3720 --- /dev/null +++ b/auditlog_tests/test_use_json_for_changes.py @@ -0,0 +1,115 @@ +from django.test import TestCase, override_settings +from test_app.models import JSONModel, RelatedModel, SimpleModel + +from auditlog.registry import AuditlogModelRegistry + + +class JSONForChangesTest(TestCase): + + def setUp(self): + self.test_auditlog = AuditlogModelRegistry() + + @override_settings(AUDITLOG_STORE_JSON_CHANGES="str") + def test_wrong_setting_type(self): + with self.assertRaisesMessage( + TypeError, "Setting 'AUDITLOG_STORE_JSON_CHANGES' must be a boolean" + ): + self.test_auditlog.register_from_settings() + + @override_settings(AUDITLOG_STORE_JSON_CHANGES=True) + def test_use_json_for_changes_with_simplemodel(self): + self.test_auditlog.register_from_settings() + + smm = SimpleModel() + smm.save() + changes_dict = smm.history.latest().changes_dict + + # compare the id, text, boolean and datetime fields + id_field_changes = changes_dict["id"] + self.assertIsNone(id_field_changes[0]) + self.assertIsInstance( + id_field_changes[1], int + ) # the id depends on state of the database + + text_field_changes = changes_dict["text"] + self.assertEqual(text_field_changes, [None, ""]) + + boolean_field_changes = changes_dict["boolean"] + self.assertEqual(boolean_field_changes, [None, False]) + + # datetime should be serialized to string + datetime_field_changes = changes_dict["datetime"] + self.assertIsNone(datetime_field_changes[0]) + self.assertIsInstance(datetime_field_changes[1], str) + + @override_settings(AUDITLOG_STORE_JSON_CHANGES=True) + def test_use_json_for_changes_with_jsonmodel(self): + self.test_auditlog.register_from_settings() + + json_model = JSONModel() + json_model.json = {"test_key": "test_value"} + json_model.save() + changes_dict = json_model.history.latest().changes_dict + + id_field_changes = changes_dict["json"] + self.assertEqual(id_field_changes, [None, {"test_key": "test_value"}]) + + @override_settings(AUDITLOG_STORE_JSON_CHANGES=True) + def test_use_json_for_changes_with_jsonmodel_with_empty_list(self): + self.test_auditlog.register_from_settings() + + json_model = JSONModel() + json_model.json = [] + json_model.save() + changes_dict = json_model.history.latest().changes_dict + + id_field_changes = changes_dict["json"] + self.assertEqual(id_field_changes, [None, []]) + + @override_settings(AUDITLOG_STORE_JSON_CHANGES=True) + def test_use_json_for_changes_with_jsonmodel_with_complex_data(self): + self.test_auditlog.register_from_settings() + + json_model = JSONModel() + json_model.json = { + "key": "test_value", + "key_dict": {"inner_key": "inner_value"}, + "key_tuple": ("item1", "item2", "item3"), + } + json_model.save() + changes_dict = json_model.history.latest().changes_dict + + id_field_changes = changes_dict["json"] + self.assertEqual( + id_field_changes, + [ + None, + { + "key": "test_value", + "key_dict": {"inner_key": "inner_value"}, + "key_tuple": [ + "item1", + "item2", + "item3", + ], # tuple is converted to list, that's ok + }, + ], + ) + + @override_settings(AUDITLOG_STORE_JSON_CHANGES=True) + def test_use_json_for_changes_with_jsonmodel_with_related_model(self): + self.test_auditlog.register_from_settings() + + simple = SimpleModel.objects.create() + one_simple = SimpleModel.objects.create() + related_model = RelatedModel.objects.create( + one_to_one=simple, related=one_simple + ) + related_model.save() + changes_dict = related_model.history.latest().changes_dict + + field_related_changes = changes_dict["related"] + self.assertEqual(field_related_changes, [None, one_simple.id]) + + field_one_to_one_changes = changes_dict["one_to_one"] + self.assertEqual(field_one_to_one_changes, [None, simple.id]) diff --git a/docs/source/usage.rst b/docs/source/usage.rst index 46bf038..de731bb 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -337,6 +337,15 @@ Negative values: No truncation occurs, and the full string is displayed. .. versionadded:: 3.1.0 +**AUDITLOG_STORE_JSON_CHANGES** + +This configuration variable defines whether to store changes as JSON. + +This means that primitives such as booleans, integers, etc. will be represented using their JSON equivalents. For example, instead of storing +`None` as a string, it will be stored as a JSON `null` in the `changes` field. Same goes for other primitives. + +.. versionadded:: 3.2.0 + Actors ------