From 9d252f3310a8fe507188bb065d7e785a51d0a587 Mon Sep 17 00:00:00 2001 From: Jan-Jelle Kester Date: Wed, 3 Jun 2015 15:50:41 +0200 Subject: [PATCH 1/6] Respect default values and non-string objects in diffs --- src/auditlog/diff.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/auditlog/diff.py b/src/auditlog/diff.py index b8194e2..2db3c3c 100644 --- a/src/auditlog/diff.py +++ b/src/auditlog/diff.py @@ -1,15 +1,15 @@ from __future__ import unicode_literals from django.core.exceptions import ObjectDoesNotExist -from django.db.models import Model +from django.db.models import Model, NOT_PROVIDED from django.utils.encoding import smart_text def model_instance_diff(old, new, **kwargs): """ Calculate the differences between two model instances. One of the instances may be None (i.e., a newly - created model or deleted model). This will cause all fields with a value to have changed (from None). - + created model or deleted model). This will cause all fields with a value to have changed (from the fields default + value). """ from auditlog.registry import auditlog @@ -48,17 +48,17 @@ def model_instance_diff(old, new, **kwargs): for field in fields: try: - old_value = smart_text(getattr(old, field.name, None)) + old_value = getattr(old, field.name, None) except ObjectDoesNotExist: - old_value = None + old_value = field.default if field.default is not NOT_PROVIDED else None try: - new_value = smart_text(getattr(new, field.name, None)) + new_value = getattr(new, field.name, None) except ObjectDoesNotExist: new_value = None if old_value != new_value: - diff[field.name] = (old_value, new_value) + diff[field.name] = (smart_text(old_value), smart_text(new_value)) if len(diff) == 0: diff = None From 18a04cfe7ed2ffc48879831f0c85cda346ee12fb Mon Sep 17 00:00:00 2001 From: Jan-Jelle Kester Date: Wed, 3 Jun 2015 16:06:25 +0200 Subject: [PATCH 2/6] Use _meta API in diffs where possible --- src/auditlog/diff.py | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/auditlog/diff.py b/src/auditlog/diff.py index 2db3c3c..3f11513 100644 --- a/src/auditlog/diff.py +++ b/src/auditlog/diff.py @@ -5,6 +5,26 @@ from django.db.models import Model, NOT_PROVIDED from django.utils.encoding import smart_text +def get_fields_in_model(instance): + """ + Returns the list of fields in the given model instance. Checks whether to use the official _meta API or use the raw + data. This method excludes many to many fields. + + :param instance: The model instance to get the fields for + :type instance: Model + :return: The list of fields for the given model (instance) + :rtype: list + """ + assert isinstance(instance, Model) + + # Check if the Django 1.8 _meta API is available + use_api = hasattr(instance._meta, 'get_fields') and callable(instance._meta.get_fields) + + if use_api: + return [f for f in instance._meta.get_fields() if not f.many_to_many] + return instance._meta.fields + + def model_instance_diff(old, new, **kwargs): """ Calculate the differences between two model instances. One of the instances may be None (i.e., a newly @@ -21,13 +41,13 @@ def model_instance_diff(old, new, **kwargs): diff = {} if old is not None and new is not None: - fields = set(old._meta.fields + new._meta.fields) + fields = set(get_fields_in_model(old) + get_fields_in_model(new)) model_fields = auditlog.get_model_fields(new._meta.model) elif old is not None: - fields = set(old._meta.fields) + fields = set(get_fields_in_model(old)) model_fields = auditlog.get_model_fields(old._meta.model) elif new is not None: - fields = set(new._meta.fields) + fields = set(get_fields_in_model(new)) model_fields = auditlog.get_model_fields(new._meta.model) else: fields = set() From a689823b2629952a6a66458a1daa7a74ea8f62ca Mon Sep 17 00:00:00 2001 From: Jan-Jelle Kester Date: Wed, 22 Jul 2015 00:03:54 +0200 Subject: [PATCH 3/6] Exclude AuditlogHistoryField from diffs --- src/auditlog/diff.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/auditlog/diff.py b/src/auditlog/diff.py index 3f11513..b25dac3 100644 --- a/src/auditlog/diff.py +++ b/src/auditlog/diff.py @@ -15,13 +15,14 @@ def get_fields_in_model(instance): :return: The list of fields for the given model (instance) :rtype: list """ + from auditlog.models import AuditlogHistoryField assert isinstance(instance, Model) # Check if the Django 1.8 _meta API is available use_api = hasattr(instance._meta, 'get_fields') and callable(instance._meta.get_fields) if use_api: - return [f for f in instance._meta.get_fields() if not f.many_to_many] + return [f for f in instance._meta.get_fields() if not (f.many_to_many or isinstance(f, AuditlogHistoryField))] return instance._meta.fields From 7d6380206fc3ae1b015dd3e9878c175603c32d0e Mon Sep 17 00:00:00 2001 From: Jan-Jelle Kester Date: Wed, 22 Jul 2015 00:16:33 +0200 Subject: [PATCH 4/6] Extract field tracking test, do not track any relation to LogEntry --- src/auditlog/diff.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/auditlog/diff.py b/src/auditlog/diff.py index b25dac3..15643d1 100644 --- a/src/auditlog/diff.py +++ b/src/auditlog/diff.py @@ -5,6 +5,29 @@ from django.db.models import Model, NOT_PROVIDED from django.utils.encoding import smart_text +def track_field(field): + """ + Returns whether the given field should be tracked by Auditlog. + + Untracked fields are many-to-many relations and relations to the Auditlog LogEntry model. + + :param field: The field to check. + :type field: Field + :return: Whether the given field should be tracked. + :rtype: bool + """ + from auditlog.models import LogEntry + # Do not track many to many relations + if field.many_to_many: + return False + + # Do not track relations to LogEntry + if getattr(field, 'rel') is not None and field.rel.to == LogEntry: + return False + + return True + + def get_fields_in_model(instance): """ Returns the list of fields in the given model instance. Checks whether to use the official _meta API or use the raw @@ -15,14 +38,13 @@ def get_fields_in_model(instance): :return: The list of fields for the given model (instance) :rtype: list """ - from auditlog.models import AuditlogHistoryField assert isinstance(instance, Model) # Check if the Django 1.8 _meta API is available use_api = hasattr(instance._meta, 'get_fields') and callable(instance._meta.get_fields) if use_api: - return [f for f in instance._meta.get_fields() if not (f.many_to_many or isinstance(f, AuditlogHistoryField))] + return [f for f in instance._meta.get_fields() if track_field(f)] return instance._meta.fields From 882415ea188279e351d5b8f9872ee7607512831a Mon Sep 17 00:00:00 2001 From: Jan-Jelle Kester Date: Wed, 22 Jul 2015 00:17:04 +0200 Subject: [PATCH 5/6] Explicitly specify sudo in travis.yml --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 78b9652..2aeedd8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,3 +9,4 @@ install: - "pip install -r requirements.txt" - "pip install Django==$DJANGO_VERSION" script: "python src/runtests.py" +sudo: false From ccaf3925e478662cee17fabc3cd7dadde59206f0 Mon Sep 17 00:00:00 2001 From: Jan-Jelle Kester Date: Wed, 22 Jul 2015 00:32:27 +0200 Subject: [PATCH 6/6] Provide fallback for when a field has no 'rel' attribute --- src/auditlog/diff.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auditlog/diff.py b/src/auditlog/diff.py index 15643d1..7346d8d 100644 --- a/src/auditlog/diff.py +++ b/src/auditlog/diff.py @@ -22,7 +22,7 @@ def track_field(field): return False # Do not track relations to LogEntry - if getattr(field, 'rel') is not None and field.rel.to == LogEntry: + if getattr(field, 'rel', None) is not None and field.rel.to == LogEntry: return False return True