From 6f4c6eb8a28dfa60460ee7b90d37d570f0ec443d Mon Sep 17 00:00:00 2001 From: Kevin Alberts Date: Wed, 11 Jan 2017 20:07:10 +0100 Subject: [PATCH] Make it so Django's timezone-aware DateTimeFields remain unchanged when Django resets the timezone. DateFields are not timezone-aware, so they do not need fixing. Django converts the timezone of DateTimeFields to UTC when the field gets saved. This makes it so when you update a model which includes a DateTimeField, and your server is running in another timezone, the AuditLog will think you changed the timestamp, while it actually is the same time, but in another timezone. This commit adds a specific check in the model_instance_diff function for DateTimeField models (and any subclasses of it), which converts the old and new values to UTC before comparing them to see if they've actually changed. It also adds tests to see if the code works properly. The extra test_setting (USE_TZ) is added because timezone support is disabled if it is not specified, this setting enables it. --- src/auditlog/diff.py | 30 +++++++++---- src/auditlog_tests/models.py | 13 ++++++ src/auditlog_tests/test_settings.py | 2 + src/auditlog_tests/tests.py | 67 ++++++++++++++++++++++++++++- 4 files changed, 102 insertions(+), 10 deletions(-) diff --git a/src/auditlog/diff.py b/src/auditlog/diff.py index e01bc18..7486c80 100644 --- a/src/auditlog/diff.py +++ b/src/auditlog/diff.py @@ -1,7 +1,8 @@ from __future__ import unicode_literals from django.core.exceptions import ObjectDoesNotExist -from django.db.models import Model, NOT_PROVIDED +from django.db.models import Model, NOT_PROVIDED, DateTimeField +from django.utils import timezone from django.utils.encoding import smart_text @@ -97,15 +98,26 @@ def model_instance_diff(old, new): fields = filtered_fields for field in fields: - try: - old_value = smart_text(getattr(old, field.name, None)) - except ObjectDoesNotExist: - old_value = field.default if field.default is not NOT_PROVIDED else None + if isinstance(field, DateTimeField): + # DateTimeFields are timezone-aware, so we need to convert the field + # to its naive form before we can accuratly compare them for changes. + old_value = field.to_python(getattr(old, field.name, None)) + if old_value is not None: + old_value = timezone.make_naive(old_value, timezone.utc) - try: - new_value = smart_text(getattr(new, field.name, None)) - except ObjectDoesNotExist: - new_value = None + new_value = field.to_python(getattr(new, field.name, None)) + if new_value is not None: + new_value = timezone.make_naive(new_value, timezone.utc) + else: + try: + old_value = smart_text(getattr(old, field.name, None)) + except ObjectDoesNotExist: + old_value = field.default if field.default is not NOT_PROVIDED else None + + try: + new_value = smart_text(getattr(new, field.name, None)) + except ObjectDoesNotExist: + new_value = None if old_value != new_value: diff[field.name] = (smart_text(old_value), smart_text(new_value)) diff --git a/src/auditlog_tests/models.py b/src/auditlog_tests/models.py index d301cf2..3243564 100644 --- a/src/auditlog_tests/models.py +++ b/src/auditlog_tests/models.py @@ -106,6 +106,18 @@ class AdditionalDataIncludedModel(models.Model): } return object_details + +class DateTimeFieldModel(models.Model): + """ + A model with a DateTimeField, used to test DateTimeField + changes are detected properly. + """ + label = models.CharField(max_length=100) + timestamp = models.DateTimeField() + + history = AuditlogHistoryField() + + auditlog.register(SimpleModel) auditlog.register(AltPrimaryKeyModel) auditlog.register(ProxyModel) @@ -115,3 +127,4 @@ auditlog.register(ManyRelatedModel.related.through) auditlog.register(SimpleIncludeModel, include_fields=['label']) auditlog.register(SimpleExcludeModel, exclude_fields=['text']) auditlog.register(AdditionalDataIncludedModel) +auditlog.register(DateTimeFieldModel) diff --git a/src/auditlog_tests/test_settings.py b/src/auditlog_tests/test_settings.py index 13ae141..711a844 100644 --- a/src/auditlog_tests/test_settings.py +++ b/src/auditlog_tests/test_settings.py @@ -24,3 +24,5 @@ DATABASES = { } ROOT_URLCONF = [] + +USE_TZ = True diff --git a/src/auditlog_tests/tests.py b/src/auditlog_tests/tests.py index 3b3d2ae..fe350e2 100644 --- a/src/auditlog_tests/tests.py +++ b/src/auditlog_tests/tests.py @@ -4,11 +4,14 @@ from django.core.exceptions import ValidationError from django.db.models.signals import pre_save from django.http import HttpResponse from django.test import TestCase, RequestFactory +from django.utils import timezone + from auditlog.middleware import AuditlogMiddleware from auditlog.models import LogEntry from auditlog.registry import auditlog from auditlog_tests.models import SimpleModel, AltPrimaryKeyModel, ProxyModel, \ - SimpleIncludeModel, SimpleExcludeModel, RelatedModel, ManyRelatedModel, AdditionalDataIncludedModel + SimpleIncludeModel, SimpleExcludeModel, RelatedModel, ManyRelatedModel, AdditionalDataIncludedModel, \ + DateTimeFieldModel class SimpleModelTest(TestCase): @@ -220,6 +223,68 @@ class AdditionalDataModelTest(TestCase): msg="Related model's id is logged") +class DateTimeFieldModelTest(TestCase): + """Tests if DateTimeField changes are recognised correctly""" + + utc_plus_one = timezone.get_fixed_timezone(datetime.timedelta(hours=1)) + + def test_model_with_same_time(self): + timestamp = datetime.datetime(2017, 1, 10, 12, 0, tzinfo=timezone.utc) + dtm = DateTimeFieldModel(label='DateTimeField model', timestamp=timestamp) + dtm.save() + self.assertTrue(dtm.history.count() == 1, msg="There is one log entry") + + # Change timestamp to same datetime and timezone + timestamp = datetime.datetime(2017, 1, 10, 12, 0, tzinfo=timezone.utc) + dtm.timestamp = timestamp + dtm.save() + + # Nothing should have changed + self.assertTrue(dtm.history.count() == 1, msg="There is one log entry") + + def test_model_with_different_timezone(self): + timestamp = datetime.datetime(2017, 1, 10, 12, 0, tzinfo=timezone.utc) + dtm = DateTimeFieldModel(label='DateTimeField model', timestamp=timestamp) + dtm.save() + self.assertTrue(dtm.history.count() == 1, msg="There is one log entry") + + # Change timestamp to same datetime in another timezone + timestamp = datetime.datetime(2017, 1, 10, 13, 0, tzinfo=self.utc_plus_one) + dtm.timestamp = timestamp + dtm.save() + + # Nothing should have changed + self.assertTrue(dtm.history.count() == 1, msg="There is one log entry") + + def test_model_with_different_time(self): + timestamp = datetime.datetime(2017, 1, 10, 12, 0, tzinfo=timezone.utc) + dtm = DateTimeFieldModel(label='DateTimeField model', timestamp=timestamp) + dtm.save() + self.assertTrue(dtm.history.count() == 1, msg="There is one log entry") + + # Change timestamp to another datetime in the same timezone + timestamp = datetime.datetime(2017, 1, 10, 13, 0, tzinfo=timezone.utc) + dtm.timestamp = timestamp + dtm.save() + + # The time should have changed. + self.assertTrue(dtm.history.count() == 2, msg="There are two log entries") + + def test_model_with_different_time_and_timezone(self): + timestamp = datetime.datetime(2017, 1, 10, 12, 0, tzinfo=timezone.utc) + dtm = DateTimeFieldModel(label='DateTimeField model', timestamp=timestamp) + dtm.save() + self.assertTrue(dtm.history.count() == 1, msg="There is one log entry") + + # Change timestamp to another datetime and another timezone + timestamp = datetime.datetime(2017, 1, 10, 14, 0, tzinfo=self.utc_plus_one) + dtm.timestamp = timestamp + dtm.save() + + # The time should have changed. + self.assertTrue(dtm.history.count() == 2, msg="There are two log entries") + + class UnregisterTest(TestCase): def setUp(self): auditlog.unregister(SimpleModel)