Merge pull request #213 from lucaswiman/unset-middleware-request

Unset middleware request

Author: macro1

AUTHORS.rst

@@ -31,6 +31,7 @@ Authors
 - Trey Hunner
 - Ulysses Vilela
 - vnagendra
+- Lucas Wiman
 
 Background
 ==========

CHANGES.rst

@@ -1,6 +1,10 @@
 Changes
 =======
 
+tip (unreleased)
+----------------
+- Clear the threadlocal request object when processing the response to prevent test interactions. (gh-213)
+
 1.8.0 (2016-02-02)
 ------------------
 - History tracking can be inherited by passing `inherit=True`. (gh-63)

simple_history/middleware.py

@@ -11,3 +11,8 @@ class HistoryRequestMiddleware(object):
 
     def process_request(self, request):
         HistoricalRecords.thread.request = request
+
+    def process_response(self, request, response):
+        if hasattr(HistoricalRecords.thread, 'request'):
+            del HistoricalRecords.thread.request
+        return response

simple_history/tests/models.py

@@ -311,3 +311,35 @@ class TrackedConcreteBase(models.Model):
 
 class UntrackedConcreteBase(models.Model):
     pass
+
+
+class TrackedWithAbstractBase(TrackedAbstractBaseA):
+    pass
+
+
+class TrackedWithConcreteBase(TrackedConcreteBase):
+    pass
+
+
+class TrackedWithTrackedAbstractAndUntrackedConcreteBase(TrackedAbstractBaseA, UntrackedConcreteBase):
+    pass
+
+
+class BaseTrackedWithIndirectTrackedAbstractBase(TrackedAbstractBaseA):
+    pass
+
+
+class TrackedWithIndirectTrackedAbstractBase(BaseTrackedWithIndirectTrackedAbstractBase):
+    pass
+
+
+class BaseTrackedWithIndirectTrackedConcreteBase(TrackedAbstractBaseA):
+    pass
+
+
+class TrackedWithIndirectTrackedConcreteBase(BaseTrackedWithIndirectTrackedConcreteBase):
+    pass
+
+
+class TrackedWithAbstractBaseToRegister(TrackedAbstractBaseA):
+    pass

simple_history/tests/tests/test_admin.py

@@ -4,6 +4,7 @@
 from django_webtest import WebTest
 from django.contrib.admin import AdminSite
 from django.contrib.messages.storage.fallback import FallbackStorage
+from django.db.transaction import atomic
 from django.test.utils import override_settings
 from django.test.client import RequestFactory
 from django import VERSION
@@ -191,12 +192,53 @@ def test_middleware_saves_user(self):
         }
         with override_settings(**overridden_settings):
             self.login()
-            poll = Poll.objects.create(question="why?", pub_date=today)
-            historical_poll = poll.history.all()[0]
-            self.assertEqual(historical_poll.history_user, self.user,
+            form = self.app.get(reverse('admin:tests_book_add')).form
+            form["isbn"] = "9780147_513731"
+            form.submit()
+            book = Book.objects.get()
+            historical_book = book.history.all()[0]
+
+            self.assertEqual(historical_book.history_user, self.user,
                              "Middleware should make the request available to "
                              "retrieve history_user.")
 
+    def test_middleware_unsets_request(self):
+        overridden_settings = {
+            'MIDDLEWARE_CLASSES':
+                settings.MIDDLEWARE_CLASSES
+                + ['simple_history.middleware.HistoryRequestMiddleware'],
+        }
+        with override_settings(**overridden_settings):
+            self.login()
+            self.app.get(reverse('admin:tests_book_add'))
+            self.assertFalse(hasattr(HistoricalRecords.thread, 'request'))
+
+    def test_rolled_back_user_does_not_lead_to_foreign_key_error(self):
+        # This test simulates the rollback of a user after a request (which
+        # happens, e.g. in test cases), and verifies that subsequently
+        # creating a new entry does not fail with a foreign key error.
+
+        overridden_settings = {
+            'MIDDLEWARE_CLASSES':
+                settings.MIDDLEWARE_CLASSES
+                + ['simple_history.middleware.HistoryRequestMiddleware'],
+        }
+        with override_settings(**overridden_settings):
+            self.login()
+            self.assertEqual(
+                self.app.get(reverse('admin:tests_book_add')).status_code,
+                200,
+            )
+
+            book = Book.objects.create(isbn="9780147_513731")
+
+        historical_book = book.history.all()[0]
+
+        self.assertIsNone(
+            historical_book.history_user,
+            "No way to know of request, history_user should be unset.",
+        )
+
     def test_middleware_anonymous_user(self):
         overridden_settings = {
             'MIDDLEWARE_CLASSES':

simple_history/tests/tests/test_models.py

@@ -20,8 +20,12 @@
     HistoricalState, HistoricalCustomFKError, Series, SeriesWork, PollInfo,
     UserAccessorDefault, UserAccessorOverride, Employee, Country, Province,
     City, Contact, ContactRegister,
-    TrackedAbstractBaseA, TrackedAbstractBaseB, UntrackedAbstractBase,
-    TrackedConcreteBase, UntrackedConcreteBase,
+    TrackedAbstractBaseA, TrackedAbstractBaseB,
+    TrackedWithAbstractBase, TrackedWithConcreteBase,
+    TrackedWithTrackedAbstractAndUntrackedConcreteBase,
+    TrackedWithIndirectTrackedAbstractBase,
+    TrackedWithIndirectTrackedConcreteBase,
+    TrackedWithAbstractBaseToRegister,
 )
 from ..external.models import ExternalModel2, ExternalModel4
 
@@ -786,18 +790,12 @@ def test_custom_table_name_from_register(self):
 class TestTrackingInheritance(TestCase):
 
     def test_tracked_abstract_base(self):
-        class TrackedWithAbstractBase(TrackedAbstractBaseA):
-            pass
-
         self.assertEqual(
             [f.attname for f in TrackedWithAbstractBase.history.model._meta.fields],
             ['id', 'history_id', 'history_date', 'history_user_id', 'history_type'],
         )
 
     def test_tracked_concrete_base(self):
-        class TrackedWithConcreteBase(TrackedConcreteBase):
-            pass
-
         self.assertEqual(
             [f.attname for f in TrackedWithConcreteBase.history.model._meta.fields],
             ['id', 'trackedconcretebase_ptr_id', 'history_id', 'history_date', 'history_user_id', 'history_type'],
@@ -809,21 +807,12 @@ class TrackedWithMultipleAbstractBases(TrackedAbstractBaseA, TrackedAbstractBase
                 pass
 
     def test_tracked_abstract_and_untracked_concrete_base(self):
-        class TrackedWithTrackedAbstractAndUntrackedConcreteBase(TrackedAbstractBaseA, UntrackedConcreteBase):
-            pass
-
         self.assertEqual(
             [f.attname for f in TrackedWithTrackedAbstractAndUntrackedConcreteBase.history.model._meta.fields],
             ['id', 'untrackedconcretebase_ptr_id', 'history_id', 'history_date', 'history_user_id', 'history_type'],
         )
 
     def test_indirect_tracked_abstract_base(self):
-        class BaseTrackedWithIndirectTrackedAbstractBase(TrackedAbstractBaseA):
-            pass
-
-        class TrackedWithIndirectTrackedAbstractBase(BaseTrackedWithIndirectTrackedAbstractBase):
-            pass
-
         self.assertEqual(
             [f.attname for f in TrackedWithIndirectTrackedAbstractBase.history.model._meta.fields],
             [
@@ -832,12 +821,6 @@ class TrackedWithIndirectTrackedAbstractBase(BaseTrackedWithIndirectTrackedAbstr
         )
 
     def test_indirect_tracked_concrete_base(self):
-        class BaseTrackedWithIndirectTrackedConcreteBase(TrackedAbstractBaseA):
-            pass
-
-        class TrackedWithIndirectTrackedConcreteBase(BaseTrackedWithIndirectTrackedConcreteBase):
-            pass
-
         self.assertEqual(
             [f.attname for f in TrackedWithIndirectTrackedConcreteBase.history.model._meta.fields],
             [
@@ -846,8 +829,5 @@ class TrackedWithIndirectTrackedConcreteBase(BaseTrackedWithIndirectTrackedConcr
         )
 
     def test_registering_with_tracked_abstract_base(self):
-        class TrackedWithAbstractBaseToRegister(TrackedAbstractBaseA):
-            pass
-
         with self.assertRaises(exceptions.MultipleRegistrationsError):
             register(TrackedWithAbstractBaseToRegister)