From be2ac5845e0266bdcc89fd970550cca0067c2be5 Mon Sep 17 00:00:00 2001 From: Jason Held Date: Wed, 22 Jan 2020 12:09:44 -0500 Subject: [PATCH] Bugfix: DJANGO_EASY_AUDIT_DATABASE_ALIAS support. (#119) --- README.md | 7 ++ easyaudit/settings.py | 3 + easyaudit/signals/model_signals.py | 179 ++++++++++++++++------------- easyaudit/tests/test_app/tests.py | 6 +- 4 files changed, 114 insertions(+), 81 deletions(-) diff --git a/README.md b/README.md index f950f860..33916888 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,13 @@ Below are some of the settings you may want to use. These should be defined in y - ['event_type', 'content_type', 'user', 'datetime', ] for CRUDEventAdmin - ['login_type', 'user', 'datetime', ] for LoginEventAdmin - ['method', 'user', 'datetime', ] for RequestEventAdmin + +* `DJANGO_EASY_AUDIT_DATABASE_ALIAS` + + By default it is the django `default` database alias. But for projects that have split databases, + this is necessary in order to keep database atomicity concerns in check during signal handlers. + + To clarify, this is only _truly_ necessary for the model signals. ## What does it do diff --git a/easyaudit/settings.py b/easyaudit/settings.py index c95c7cfd..27d193a9 100644 --- a/easyaudit/settings.py +++ b/easyaudit/settings.py @@ -1,5 +1,7 @@ + from importlib import import_module +import django.db.utils from django.apps import apps from django.conf import settings from django.contrib.auth.models import Permission @@ -83,6 +85,7 @@ def get_model_list(class_list): # project defined callbacks CRUD_DIFFERENCE_CALLBACKS = [] CRUD_DIFFERENCE_CALLBACKS = getattr(settings, 'DJANGO_EASY_AUDIT_CRUD_DIFFERENCE_CALLBACKS', CRUD_DIFFERENCE_CALLBACKS) +DATABASE_ALIAS = getattr(settings, 'DJANGO_EASY_AUDIT_DATABASE_ALIAS', django.db.utils.DEFAULT_DB_ALIAS) # the callbacks could come in as an iterable of strings, where each string is the package.module.function for idx, callback in enumerate(CRUD_DIFFERENCE_CALLBACKS): if not callable(callback): # keep as is if it is callable diff --git a/easyaudit/signals/model_signals.py b/easyaudit/signals/model_signals.py index 60a796e2..3eb8bb14 100644 --- a/easyaudit/signals/model_signals.py +++ b/easyaudit/signals/model_signals.py @@ -1,6 +1,7 @@ import json import logging +from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser from django.contrib.contenttypes.models import ContentType @@ -14,7 +15,7 @@ get_current_user from easyaudit.models import CRUDEvent from easyaudit.settings import REGISTERED_CLASSES, UNREGISTERED_CLASSES, \ - WATCH_MODEL_EVENTS, CRUD_DIFFERENCE_CALLBACKS + WATCH_MODEL_EVENTS, CRUD_DIFFERENCE_CALLBACKS, DATABASE_ALIAS from easyaudit.utils import model_delta logger = logging.getLogger(__name__) @@ -49,7 +50,7 @@ def pre_save(sender, instance, raw, using, update_fields, **kwargs): return try: - with transaction.atomic(): + with transaction.atomic(using=using): if not should_audit(instance): return False try: @@ -89,25 +90,30 @@ def pre_save(sender, instance, raw, using, update_fields, **kwargs): # create crud event only if all callbacks returned True if create_crud_event and not created: c_t = ContentType.objects.get_for_model(instance) - sid = transaction.savepoint() - try: - with transaction.atomic(): - crud_event = CRUDEvent.objects.create( - event_type=event_type, - object_repr=str(instance), - object_json_repr=object_json_repr, - changed_fields=changed_fields, - content_type_id=c_t.id, - object_id=instance.pk, - user_id=getattr(user, 'id', None), - datetime=timezone.now(), - user_pk_as_string=str(user.pk) if user else user - ) - except Exception as e: - logger.exception( - "easy audit had a pre-save exception on CRUDEvent creation. instance: {}, instance pk: {}".format( - instance, instance.pk)) - transaction.savepoint_rollback(sid) + + def crud_flow(): + try: + # atomicity based on the easyaudit database alias + with transaction.atomic(using=DATABASE_ALIAS): + crud_event = CRUDEvent.objects.create( + event_type=event_type, + object_repr=str(instance), + object_json_repr=object_json_repr, + changed_fields=changed_fields, + content_type_id=c_t.id, + object_id=instance.pk, + user_id=getattr(user, 'id', None), + datetime=timezone.now(), + user_pk_as_string=str(user.pk) if user else user + ) + except Exception as e: + logger.exception( + "easy audit had a pre-save exception on CRUDEvent creation. instance: {}, instance pk: {}".format( + instance, instance.pk)) + if getattr(settings, "TEST", False): + crud_flow() + else: + transaction.on_commit(crud_flow, using=using) except Exception: logger.exception('easy audit had a pre-save exception.') @@ -119,7 +125,7 @@ def post_save(sender, instance, created, raw, using, update_fields, **kwargs): return try: - with transaction.atomic(): + with transaction.atomic(using=using): if not should_audit(instance): return False object_json_repr = serializers.serialize("json", [instance]) @@ -149,24 +155,28 @@ def post_save(sender, instance, created, raw, using, update_fields, **kwargs): # create crud event only if all callbacks returned True if create_crud_event and created: c_t = ContentType.objects.get_for_model(instance) - sid = transaction.savepoint() - try: - with transaction.atomic(): - crud_event = CRUDEvent.objects.create( - event_type=event_type, - object_repr=str(instance), - object_json_repr=object_json_repr, - content_type_id=c_t.id, - object_id=instance.pk, - user_id=getattr(user, 'id', None), - datetime=timezone.now(), - user_pk_as_string=str(user.pk) if user else user - ) - except Exception as e: - logger.exception( - "easy audit had a pre-save exception on CRUDEvent creation. instance: {}, instance pk: {}".format( - instance, instance.pk)) - transaction.savepoint_rollback(sid) + + def crud_flow(): + try: + with transaction.atomic(using=DATABASE_ALIAS): + crud_event = CRUDEvent.objects.create( + event_type=event_type, + object_repr=str(instance), + object_json_repr=object_json_repr, + content_type_id=c_t.id, + object_id=instance.pk, + user_id=getattr(user, 'id', None), + datetime=timezone.now(), + user_pk_as_string=str(user.pk) if user else user + ) + except Exception as e: + logger.exception( + "easy audit had a pre-save exception on CRUDEvent creation. instance: {}, instance pk: {}".format( + instance, instance.pk)) + if getattr(settings, "TEST", False): + crud_flow() + else: + transaction.on_commit(crud_flow, using=using) except Exception: logger.exception('easy audit had a post-save exception.') @@ -191,7 +201,7 @@ def _m2m_rev_field_name(model1, model2): def m2m_changed(sender, instance, action, reverse, model, pk_set, using, **kwargs): """https://docs.djangoproject.com/es/1.10/ref/signals/#m2m-changed""" try: - with transaction.atomic(): + with transaction.atomic(using=using): if not should_audit(instance): return False @@ -228,25 +238,29 @@ def m2m_changed(sender, instance, action, reverse, model, pk_set, using, **kwarg if isinstance(user, AnonymousUser): user = None c_t = ContentType.objects.get_for_model(instance) - sid = transaction.savepoint() - try: - with transaction.atomic(): - crud_event = CRUDEvent.objects.create( - event_type=event_type, - object_repr=str(instance), - object_json_repr=object_json_repr, - content_type_id=c_t.id, - object_id=instance.pk, - user_id=getattr(user, 'id', None), - datetime=timezone.now(), - user_pk_as_string=str(user.pk) if user else user - ) - except Exception as e: - logger.exception( - "easy audit had a pre-save exception on CRUDEvent creation. instance: {}, instance pk: {}".format( - instance, instance.pk)) - transaction.savepoint_rollback(sid) + def crud_flow(): + try: + with transaction.atomic(using=DATABASE_ALIAS): + crud_event = CRUDEvent.objects.create( + event_type=event_type, + object_repr=str(instance), + object_json_repr=object_json_repr, + content_type_id=c_t.id, + object_id=instance.pk, + user_id=getattr(user, 'id', None), + datetime=timezone.now(), + user_pk_as_string=str(user.pk) if user else user + ) + except Exception as e: + logger.exception( + "easy audit had a pre-save exception on CRUDEvent creation. instance: {}, instance pk: {}".format( + instance, instance.pk)) + + if getattr(settings, "TEST", False): + crud_flow() + else: + transaction.on_commit(crud_flow, using=using) except Exception: logger.exception('easy audit had an m2m-changed exception.') @@ -254,7 +268,7 @@ def m2m_changed(sender, instance, action, reverse, model, pk_set, using, **kwarg def post_delete(sender, instance, using, **kwargs): """https://docs.djangoproject.com/es/1.10/ref/signals/#post-delete""" try: - with transaction.atomic(): + with transaction.atomic(using=using): if not should_audit(instance): return False @@ -271,26 +285,31 @@ def post_delete(sender, instance, using, **kwargs): if isinstance(user, AnonymousUser): user = None c_t = ContentType.objects.get_for_model(instance) - sid = transaction.savepoint() - try: - with transaction.atomic(): - # crud event - crud_event = CRUDEvent.objects.create( - event_type=CRUDEvent.DELETE, - object_repr=str(instance), - object_json_repr=object_json_repr, - content_type_id=c_t.id, - object_id=instance.pk, - user_id=getattr(user, 'id', None), - datetime=timezone.now(), - user_pk_as_string=str(user.pk) if user else user - ) - - except Exception as e: - logger.exception( - "easy audit had a pre-save exception on CRUDEvent creation. instance: {}, instance pk: {}".format( - instance, instance.pk)) - transaction.savepoint_rollback(sid) + + def crud_flow(): + try: + with transaction.atomic(using=DATABASE_ALIAS): + # crud event + crud_event = CRUDEvent.objects.create( + event_type=CRUDEvent.DELETE, + object_repr=str(instance), + object_json_repr=object_json_repr, + content_type_id=c_t.id, + object_id=instance.pk, + user_id=getattr(user, 'id', None), + datetime=timezone.now(), + user_pk_as_string=str(user.pk) if user else user + ) + + except Exception as e: + logger.exception( + "easy audit had a pre-save exception on CRUDEvent creation. instance: {}, instance pk: {}".format( + instance, instance.pk)) + + if getattr(settings, "TEST", False): + crud_flow() + else: + transaction.on_commit(crud_flow, using=using) except Exception: logger.exception('easy audit had a post-delete exception.') diff --git a/easyaudit/tests/test_app/tests.py b/easyaudit/tests/test_app/tests.py index 027aa1cd..01c7b5d2 100644 --- a/easyaudit/tests/test_app/tests.py +++ b/easyaudit/tests/test_app/tests.py @@ -1,7 +1,8 @@ # -*- coding: utf-8 -*- import json import re -from django.test import TestCase +from django.test import TestCase, override_settings + try: # Django 2.0 from django.urls import reverse except: # Django < 2.0 @@ -20,6 +21,7 @@ TEST_ADMIN_PASSWORD = 'password' +@override_settings(TEST=True) class TestAuditModels(TestCase): def test_create_model(self): @@ -49,6 +51,7 @@ def test_m2m_model(self): self.assertEqual(data['fields']['test_m2m'], [obj.id]) +@override_settings(TEST=True) class TestMiddleware(TestCase): def _setup_user(self, email, password): user = User(username=email) @@ -100,6 +103,7 @@ def test_manual_set_user(self): self.assertEqual(crud_event.user, None) +@override_settings(TEST=True) class TestAuditAdmin(TestCase): def _setup_superuser(self, email, password):