mirror of
https://github.com/jazzband/django-auditlog.git
synced 2026-03-16 22:20:26 +00:00
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>
This commit is contained in:
parent
ecdefde9ed
commit
b640df67a3
7 changed files with 190 additions and 14 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
||||
|
|
|
|||
115
auditlog_tests/test_use_json_for_changes.py
Normal file
115
auditlog_tests/test_use_json_for_changes.py
Normal file
|
|
@ -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])
|
||||
|
|
@ -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
|
||||
------
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue