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
This commit is contained in:
Rod Manning 2018-01-03 07:50:45 +13:00 committed by Ryan Castner
parent e33aef85d5
commit 4dee03497c
10 changed files with 81 additions and 8 deletions

View file

@ -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

20
src/auditlog/compat.py Normal file
View file

@ -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

View file

@ -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)

View file

@ -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'],

View file

@ -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:

View file

@ -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"))

View file

@ -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()

View file

@ -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',
)

View file

@ -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)

View file

@ -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