From 4dee03497c28d6d81762df196ada6970c27b4363 Mon Sep 17 00:00:00 2001 From: Rod Manning Date: Wed, 3 Jan 2018 07:50:45 +1300 Subject: [PATCH] Add Django 2.0 Support (#154) * Add changes for django 2.0 Made the following changes to ensure compatibility with django 2.0: 1. Replaced function calls to `is_authenticated()` with reference to property `is_authenticated`. 2. Added try/except call to import `django.core.urlresolvers` (now called `django.urls`. Also added an `... as ...` statement to ensure that references in the code to `urlresolvers` don't need to be changed. 3. Fixed calls statement of `on_delete` arg to all ForeignKey fields. Note that previously a kwarg was acceptable, but this is now a positional arg, and the selected `on_delete` method has been retained. * Update tox tests and consequentual changes Updated tox.ini to also test django 2.0 on python 3+. Some changes made to previous commits required to ensure all tests passed: - Added `compat.py` to have a `is_authenticated()` function to check authentication. This was necessary as the property/method call for `is_authenticated` is no compatible between django 1.8 (LTS) and 2.0. Changed AuditLogMiddleware to call this compatibility function instead of the django built-ins as a result. - Changes made to `auditlog/models.py` to apply kwargs to both `to=` and `on_delete=` for consistency of handling in all version tested. Incorrect django version specified for tox.ini. Now fixed. * Add 'on_delete' kwarg to initial migration Added and re-arranged 'on_delete' and 'to' kwargs in initial migration to ensure compatbility with later versions of Django. Also included updated manifest with changes required due to django 2.0 work. * Add TestCase for compat.py Added simple test case for compat.py file. * Changes follow code review 2017-12-21 * More changes following code review 2017-12-28 1. Added detailed commentary to `compat.py` to ensure reason why `is_authenticated()` compatibility function is needed 2. Changed `hasattr` to `callable` in compat.is_authenticated() 3. Fixed typo in migration 0001 to use correct `on_delete` function --- MANIFEST | 4 +++ src/auditlog/compat.py | 20 ++++++++++++ src/auditlog/middleware.py | 3 +- src/auditlog/migrations/0001_initial.py | 2 +- src/auditlog/mixins.py | 5 ++- src/auditlog/models.py | 4 +-- src/auditlog_tests/models.py | 4 +-- src/auditlog_tests/test_settings.py | 2 ++ src/auditlog_tests/tests.py | 41 +++++++++++++++++++++++++ tox.ini | 4 ++- 10 files changed, 81 insertions(+), 8 deletions(-) create mode 100644 src/auditlog/compat.py diff --git a/MANIFEST b/MANIFEST index fcb8944..f2ed177 100644 --- a/MANIFEST +++ b/MANIFEST @@ -3,6 +3,7 @@ setup.py src/auditlog/__init__.py src/auditlog/admin.py src/auditlog/apps.py +src/auditlog/compat.py src/auditlog/diff.py src/auditlog/filters.py src/auditlog/middleware.py @@ -10,6 +11,9 @@ src/auditlog/mixins.py src/auditlog/models.py src/auditlog/receivers.py src/auditlog/registry.py +src/auditlog/management/__init__.py +src/auditlog/management/commands/__init__.py +src/auditlog/management/commands/auditlogflush.py src/auditlog/migrations/0001_initial.py src/auditlog/migrations/0002_auto_support_long_primary_keys.py src/auditlog/migrations/0003_logentry_remote_addr.py diff --git a/src/auditlog/compat.py b/src/auditlog/compat.py new file mode 100644 index 0000000..086b346 --- /dev/null +++ b/src/auditlog/compat.py @@ -0,0 +1,20 @@ +import django + +def is_authenticated(user): + """Return whether or not a User is authenticated. + + Function provides compatibility following deprecation of method call to + `is_authenticated()` in Django 2.0. + + This is *only* required to support Django < v1.10 (i.e. v1.9 and earlier), + as `is_authenticated` was introduced as a property in v1.10.s + """ + if not hasattr(user, 'is_authenticated'): + return False + if callable(user.is_authenticated): + # Will be callable if django.version < 2.0, but is only necessary in + # v1.9 and earlier due to change introduced in v1.10 making + # `is_authenticated` a property instead of a callable. + return user.is_authenticated() + else: + return user.is_authenticated diff --git a/src/auditlog/middleware.py b/src/auditlog/middleware.py index 7780bae..a266fee 100644 --- a/src/auditlog/middleware.py +++ b/src/auditlog/middleware.py @@ -8,6 +8,7 @@ from django.db.models.signals import pre_save from django.utils.functional import curry from django.apps import apps from auditlog.models import LogEntry +from auditlog.compat import is_authenticated # Use MiddlewareMixin when present (Django >= 1.10) try: @@ -41,7 +42,7 @@ class AuditlogMiddleware(MiddlewareMixin): threadlocal.auditlog['remote_addr'] = request.META.get('HTTP_X_FORWARDED_FOR').split(',')[0] # Connect signal for automatic logging - if hasattr(request, 'user') and hasattr(request.user, 'is_authenticated') and request.user.is_authenticated(): + if hasattr(request, 'user') and is_authenticated(request.user): set_actor = curry(self.set_actor, user=request.user, signal_duid=threadlocal.auditlog['signal_duid']) pre_save.connect(set_actor, sender=LogEntry, dispatch_uid=threadlocal.auditlog['signal_duid'], weak=False) diff --git a/src/auditlog/migrations/0001_initial.py b/src/auditlog/migrations/0001_initial.py index b973bb3..5a96248 100644 --- a/src/auditlog/migrations/0001_initial.py +++ b/src/auditlog/migrations/0001_initial.py @@ -25,7 +25,7 @@ class Migration(migrations.Migration): ('changes', models.TextField(verbose_name='change message', blank=True)), ('timestamp', models.DateTimeField(auto_now_add=True, verbose_name='timestamp')), ('actor', models.ForeignKey(related_name='+', on_delete=django.db.models.deletion.SET_NULL, verbose_name='actor', blank=True, to=settings.AUTH_USER_MODEL, null=True)), - ('content_type', models.ForeignKey(related_name='+', verbose_name='content type', to='contenttypes.ContentType')), + ('content_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', verbose_name='content type', to='contenttypes.ContentType')), ], options={ 'ordering': ['-timestamp'], diff --git a/src/auditlog/mixins.py b/src/auditlog/mixins.py index 3022b6d..3bea200 100644 --- a/src/auditlog/mixins.py +++ b/src/auditlog/mixins.py @@ -1,7 +1,10 @@ import json from django.conf import settings -from django.core import urlresolvers +try: + from django.core import urlresolvers +except ImportError: + from django import urls as urlresolvers try: from django.urls.exceptions import NoReverseMatch except ImportError: diff --git a/src/auditlog/models.py b/src/auditlog/models.py index 06528a3..2b93124 100644 --- a/src/auditlog/models.py +++ b/src/auditlog/models.py @@ -170,13 +170,13 @@ class LogEntry(models.Model): (DELETE, _("delete")), ) - content_type = models.ForeignKey('contenttypes.ContentType', on_delete=models.CASCADE, related_name='+', verbose_name=_("content type")) + content_type = models.ForeignKey(to='contenttypes.ContentType', on_delete=models.CASCADE, related_name='+', verbose_name=_("content type")) object_pk = models.CharField(db_index=True, max_length=255, verbose_name=_("object pk")) object_id = models.BigIntegerField(blank=True, db_index=True, null=True, verbose_name=_("object id")) object_repr = models.TextField(verbose_name=_("object representation")) action = models.PositiveSmallIntegerField(choices=Action.choices, verbose_name=_("action")) changes = models.TextField(blank=True, verbose_name=_("change message")) - actor = models.ForeignKey(settings.AUTH_USER_MODEL, blank=True, null=True, on_delete=models.SET_NULL, related_name='+', verbose_name=_("actor")) + actor = models.ForeignKey(to=settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, blank=True, null=True, related_name='+', verbose_name=_("actor")) remote_addr = models.GenericIPAddressField(blank=True, null=True, verbose_name=_("remote address")) timestamp = models.DateTimeField(auto_now_add=True, verbose_name=_("timestamp")) additional_data = JSONField(blank=True, null=True, verbose_name=_("additional data")) diff --git a/src/auditlog_tests/models.py b/src/auditlog_tests/models.py index 2311b80..d938bfe 100644 --- a/src/auditlog_tests/models.py +++ b/src/auditlog_tests/models.py @@ -66,7 +66,7 @@ class RelatedModel(models.Model): A model with a foreign key. """ - related = models.ForeignKey('self') + related = models.ForeignKey(to='self', on_delete=models.CASCADE) history = AuditlogHistoryField() @@ -124,7 +124,7 @@ class AdditionalDataIncludedModel(models.Model): label = models.CharField(max_length=100) text = models.TextField(blank=True) - related = models.ForeignKey(SimpleModel) + related = models.ForeignKey(to=SimpleModel, on_delete=models.CASCADE) history = AuditlogHistoryField() diff --git a/src/auditlog_tests/test_settings.py b/src/auditlog_tests/test_settings.py index 891b5a8..59541a4 100644 --- a/src/auditlog_tests/test_settings.py +++ b/src/auditlog_tests/test_settings.py @@ -8,6 +8,7 @@ SECRET_KEY = 'test' INSTALLED_APPS = [ 'django.contrib.auth', 'django.contrib.contenttypes', + 'django.contrib.sessions', 'auditlog', 'auditlog_tests', 'multiselectfield', @@ -15,6 +16,7 @@ INSTALLED_APPS = [ MIDDLEWARE_CLASSES = ( 'django.middleware.common.CommonMiddleware', + 'django.contrib.sessions.middleware.SessionMiddleware' 'auditlog.middleware.AuditlogMiddleware', ) diff --git a/src/auditlog_tests/tests.py b/src/auditlog_tests/tests.py index 9839496..3ad6e7e 100644 --- a/src/auditlog_tests/tests.py +++ b/src/auditlog_tests/tests.py @@ -1,5 +1,7 @@ import datetime +import django from django.conf import settings +from django.contrib import auth from django.contrib.auth.models import User, AnonymousUser from django.core.exceptions import ValidationError from django.db.models.signals import pre_save @@ -14,6 +16,7 @@ from auditlog_tests.models import SimpleModel, AltPrimaryKeyModel, UUIDPrimaryKe ProxyModel, SimpleIncludeModel, SimpleExcludeModel, SimpleMappingModel, RelatedModel, \ ManyRelatedModel, AdditionalDataIncludedModel, DateTimeFieldModel, ChoicesFieldModel, \ CharfieldTextfieldModel, PostgresArrayFieldModel +from auditlog import compat class SimpleModelTest(TestCase): @@ -578,3 +581,41 @@ class PostgresArrayFieldModelTest(TestCase): self.obj.save() self.assertTrue(self.obj.history.latest().changes_display_dict["arrayfield"][1] == "Green", msg="The human readable text 'Green' is displayed.") + + +class CompatibilityTest(TestCase): + """Test case for compatibility functions.""" + + def test_is_authenticated(self): + """Test that the 'is_authenticated' compatibility function is working. + + Bit of explanation: the `is_authenticated` property on request.user is + *always* set to 'False' for AnonymousUser, and it is *always* set to + 'True' for *any* other (i.e. identified/authenticated) user. + + So, the logic of this test is to ensure that compat.is_authenticated() + returns the correct value based on whether or not the User is an + anonymous user (simulating what goes on in the real request.user). + + """ + + # Test compat.is_authenticated for anonymous users + self.user = auth.get_user(self.client) + if django.VERSION < (1, 10): + assert self.user.is_anonymous() + else: + assert self.user.is_anonymous + assert not compat.is_authenticated(self.user) + + # Setup some other user, which is *not* anonymous, and check + # compat.is_authenticated + self.user = User.objects.create( + username="test.user", + email="test.user@mail.com", + password="auditlog" + ) + if django.VERSION < (1, 10): + assert not self.user.is_anonymous() + else: + assert not self.user.is_anonymous + assert compat.is_authenticated(self.user) diff --git a/tox.ini b/tox.ini index c39cb70..1fa28c2 100644 --- a/tox.ini +++ b/tox.ini @@ -3,6 +3,7 @@ envlist = {py27,py34,py35,py36}-django-18 {py27,py34,py35,py36}-django-110 {py27,py34,py35,py36}-django-111 + {py34,py35,py36}-django-20 [testenv] setenv = @@ -11,7 +12,8 @@ commands = coverage run --source src/auditlog src/runtests.py deps = django-18: Django>=1.8,<1.9 django-110: Django>=1.10,<1.11 - django-111: Django>=1.11 + django-111: Django>=1.11,<2.0 + django-20: Django>=2.0 -r{toxinidir}/requirements-test.txt basepython = py36: python3.6