Code

Ticket #19707: ticket_19707.diff

File ticket_19707.diff, 7.0 KB (added by akaariai, 15 months ago)
Line 
1diff --git a/django/db/__init__.py b/django/db/__init__.py
2index b198048..548c2d5 100644
3--- a/django/db/__init__.py
4+++ b/django/db/__init__.py
5@@ -42,8 +42,25 @@ backend = load_backend(connection.settings_dict['ENGINE'])
6 # Register an event that closes the database connection
7 # when a Django request is finished.
8 def close_connection(**kwargs):
9+    # Avoid circular imports
10+    from django.db import transaction
11     for conn in connections.all():
12         conn.close()
13+        # Make sure transaction_state isn't carried from one request to
14+        # another. The weird structure here is caused by TestCase turning
15+        # leave_transaction_management() to no-op.
16+        tx_state_len = len(conn.transaction_state)
17+        while tx_state_len:
18+            try:
19+                transaction.leave_transaction_management(using=conn.alias)
20+            except transaction.TransactionManagementError:
21+                # Even if we closed the connection above Django still thinks
22+                # might be a transaction going on...
23+                pass
24+            if len(conn.transaction_state) == tx_state_len:
25+                # It seems it was a no-op.
26+                break
27+            tx_state_len = len(conn.transaction_state)
28 signals.request_finished.connect(close_connection)
29 
30 # Register an event that resets connection.queries
31diff --git a/django/middleware/transaction.py b/django/middleware/transaction.py
32index 96b1538..7888839 100644
33--- a/django/middleware/transaction.py
34+++ b/django/middleware/transaction.py
35@@ -15,6 +15,7 @@ class TransactionMiddleware(object):
36     def process_exception(self, request, exception):
37         """Rolls back the database and leaves transaction management"""
38         if transaction.is_dirty():
39+            # This rollback might fail for closed connections for example.
40             transaction.rollback()
41         transaction.leave_transaction_management()
42 
43@@ -22,6 +23,21 @@ class TransactionMiddleware(object):
44         """Commits and leaves transaction management."""
45         if transaction.is_managed():
46             if transaction.is_dirty():
47-                transaction.commit()
48+                # Note: it is possible that the commit fails. If the reason is
49+                # closed connection or some similar reason, then there is
50+                # little hope to proceed nicely. However, in case of deferred
51+                # foreign key check failures we can still rollback(). In any
52+                # case we want to have an informative exception raised.
53+                try:
54+                    transaction.commit()
55+                except:
56+                    # If the rollback fails, the transaction state will be
57+                    # messed up. It doesn't matter, the connection will be set
58+                    # to clean state after the request finishes. And, we can't
59+                    # clean the state here properly even if we wanted to, the
60+                    # connection is in transaction but we can't rollback...
61+                    transaction.rollback()
62+                    transaction.leave_transaction_management()
63+                    raise
64             transaction.leave_transaction_management()
65         return response
66diff --git a/tests/regressiontests/middleware/tests.py b/tests/regressiontests/middleware/tests.py
67index a9a45c9..6c43641 100644
68--- a/tests/regressiontests/middleware/tests.py
69+++ b/tests/regressiontests/middleware/tests.py
70@@ -9,9 +9,9 @@ import warnings
71 
72 from django.conf import settings
73 from django.core import mail
74-from django.db import transaction
75-from django.http import HttpRequest
76-from django.http import HttpResponse, StreamingHttpResponse
77+from django.db import (transaction, connections, DEFAULT_DB_ALIAS,
78+                       IntegrityError)
79+from django.http import HttpRequest, HttpResponse, StreamingHttpResponse
80 from django.middleware.clickjacking import XFrameOptionsMiddleware
81 from django.middleware.common import CommonMiddleware, BrokenLinkEmailsMiddleware
82 from django.middleware.http import ConditionalGetMiddleware
83@@ -710,3 +710,22 @@ class TransactionMiddlewareTest(TransactionTestCase):
84         TransactionMiddleware().process_exception(self.request, None)
85         self.assertEqual(Band.objects.count(), 0)
86         self.assertFalse(transaction.is_dirty())
87+
88+    def test_failing_commit(self):
89+        # It is possible that connection.commit() fails. Check that
90+        # TransactionMiddleware handles such cases correctly.
91+        try:
92+            def raise_exception():
93+                raise IntegrityError()
94+            connections[DEFAULT_DB_ALIAS].commit = raise_exception
95+            transaction.enter_transaction_management()
96+            transaction.managed(True)
97+            Band.objects.create(name='The Beatles')
98+            self.assertTrue(transaction.is_dirty())
99+            with self.assertRaises(IntegrityError):
100+                TransactionMiddleware().process_response(self.request, None)
101+            self.assertEqual(Band.objects.count(), 0)
102+            self.assertFalse(transaction.is_dirty())
103+            self.assertFalse(transaction.is_managed())
104+        finally:
105+            del connections[DEFAULT_DB_ALIAS].commit
106diff --git a/tests/regressiontests/requests/tests.py b/tests/regressiontests/requests/tests.py
107index 799cd9b..a29fb36 100644
108--- a/tests/regressiontests/requests/tests.py
109+++ b/tests/regressiontests/requests/tests.py
110@@ -6,9 +6,12 @@ import warnings
111 from datetime import datetime, timedelta
112 from io import BytesIO
113 
114+from django.db import connection
115+from django.core import signals
116 from django.core.exceptions import SuspiciousOperation
117 from django.core.handlers.wsgi import WSGIRequest, LimitedStream
118 from django.http import HttpRequest, HttpResponse, parse_cookie, build_request_repr, UnreadablePostError
119+from django.test import TransactionTestCase
120 from django.test.client import FakePayload
121 from django.test.utils import override_settings, str_prefix
122 from django.utils import six
123@@ -524,3 +527,21 @@ class RequestsTests(unittest.TestCase):
124 
125         with self.assertRaises(UnreadablePostError):
126             request.body
127+
128+class TransactionRequestTests(TransactionTestCase):
129+    # Need to run the test under real transaction handling.
130+    def test_request_finished_db_state(self):
131+        # The GET below will not succeed, but it will give a response with
132+        # defined ._handler_class. That is needed for sending the request_finished
133+        # signal. However, the test client itself will not send request_finished
134+        # signals.
135+        response = self.client.get('/')
136+        # Taking a cursor will open a new connection.
137+        connection.cursor()
138+        connection.enter_transaction_management()
139+        signals.request_finished.send(sender=response._handler_class)
140+        # in-memory sqlite doesn't actually close connections when connections are closed.
141+        if connection.vendor != 'sqlite':
142+            self.assertIs(connection.connection, None)
143+        # And the transaction state is reset.
144+        self.assertEqual(len(connection.transaction_state), 0)