p31829507
/
django
mirror of https://github.com/django/django.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fixed #9964 -- Ensure that all database operations make transactions dirty, not just write operations. Many thanks to Shai Berger for his work and persistence on this issue. 

This is BACKWARDS INCOMPATIBLE for anyone relying on the current behavior that allows manually managed read-only transactions to be left dangling without a manual commit or rollback.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15493 bcc190cf-cafb-0310-a4f2-bffc1f526a37
This commit is contained in:
Russell Keith-Magee 2011-02-12 13:03:34 +00:00
parent d1cd53d9cf
commit 6314a1b42e
11 changed files with 271 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
AUTHORS
View File

@ -78,6 +78,7 @@ answer newbie questions, and generally made Django that much better:
Esdras Beleza <linux@esdrasbeleza.com>
Chris Bennett <chrisrbennett@yahoo.com>
James Bennett
Shai Berger <shai@platonix.com>
Julian Bez
Arvis Bickovskis <viestards.lists@gmail.com>
Natalia Bidart <nataliabidart@gmail.com>

5
django/db/backends/__init__.py
View File

@ -245,10 +245,11 @@ class BaseDatabaseWrapper(local):
self.connection = None
def cursor(self):
cursor = self._cursor()
if (self.use_debug_cursor or
(self.use_debug_cursor is None and settings.DEBUG)):
return self.make_debug_cursor(cursor)
cursor = self.make_debug_cursor(self._cursor())
else:
cursor = util.CursorWrapper(self._cursor(), self)
return cursor
def make_debug_cursor(self, cursor):

29
django/db/backends/util.py
View File

@ -5,12 +5,28 @@ from time import time
from django.utils.hashcompat import md5_constructor
from django.utils.log import getLogger
logger = getLogger('django.db.backends')
class CursorDebugWrapper(object):
def __init__(self, cursor, db):
class CursorWrapper(object):
def __init__(self, cursor, connection):
self.cursor = cursor
self.db = db # Instance of a BaseDatabaseWrapper subclass
self.connection = connection
def __getattr__(self, attr):
if self.connection.is_managed():
self.connection.set_dirty()
if attr in self.__dict__:
return self.__dict__[attr]
else:
return getattr(self.cursor, attr)
def __iter__(self):
return iter(self.cursor)
class CursorDebugWrapper(CursorWrapper):
def execute(self, sql, params=()):
start = time()
@ -19,8 +35,8 @@ class CursorDebugWrapper(object):
finally:
stop = time()
duration = stop - start
sql = self.db.ops.last_executed_query(self.cursor, sql, params)
self.db.queries.append({
sql = self.connection.ops.last_executed_query(self.cursor, sql, params)
self.connection.queries.append({
'sql': sql,
'time': "%.3f" % duration,
})
@ -35,7 +51,7 @@ class CursorDebugWrapper(object):
finally:
stop = time()
duration = stop - start
self.db.queries.append({
self.connection.queries.append({
'sql': '%s times: %s' % (len(param_list), sql),
'time': "%.3f" % duration,
})
@ -52,6 +68,7 @@ class CursorDebugWrapper(object):
def __iter__(self):
return iter(self.cursor)
###############################################
# Converters from database (string) to Python #
###############################################

36
docs/releases/1.3.txt
View File

@ -604,6 +604,42 @@ domain):
.. _corresponding deprecated features section: loading_of_translations_from_the_project_directory_
Transaction management
~~~~~~~~~~~~~~~~~~~~~~
When using managed transactions -- that is, anything but the default
autocommit mode -- it is important when a transaction is marked as
"dirty". Dirty transactions are committed by the
:func:`~django.db.transaction.commit_on_success` decorator or the
:class:`~django.middleware.transaction.TransactionMiddleware`, and
:func:`~django.db.transaction.commit_manually` forces them to be
closed explicitly; clean transactions "get a pass", which means they
are usually rolled back at the end of a request when the connection is
closed.
Until Django 1.3, transactions were only marked dirty when Django was
aware of a modifying operation performed in them; that is, either some
model was saved, some bulk update or delete was performed, or the user
explicitly called ``transaction.set_dirty()``. In Django 1.3, a
transaction is marked dirty when *any* database operation is
performed.
As a result of this change, you no longer need to set a transaction
dirty explicitly when you execute raw SQL or use a data-modifying
``SELECT``. However, you *do* need to explicitly close any read-only
transactions that are being managed using
:func:`~django.db.transaction.commit_manually`. For example::
@transaction.commit_manually
def my_view(request, name):
obj = get_object_or_404(MyObject, name__iexact=name)
return render_to_response('template', {'object':obj})
Prior to Django 1.3, this would work without error. However, under
Django 1.3, this will raise a :class:`TransactionManagementError` because
the read operation that retrieves the ``MyObject`` instance leaves the
transaction in a dirty state.
.. _deprecated-features-1.3:
Features deprecated in 1.3

36
docs/topics/db/sql.txt
View File

@ -233,37 +233,17 @@ alias::
Transactions and raw SQL
------------------------
If you are using transaction decorators (such as ``commit_on_success``) to
wrap your views and provide transaction control, you don't have to make a
manual call to ``transaction.commit_unless_managed()`` -- you can manually
commit if you want to, but you aren't required to, since the decorator will
commit for you. However, if you don't manually commit your changes, you will
need to manually mark the transaction as dirty, using
``transaction.set_dirty()``::
@commit_on_success
def my_custom_sql_view(request, value):
from django.db import connection, transaction
cursor = connection.cursor()
When you make a raw SQL call, Django will automatically mark the
current transaction as dirty. You must then ensure that the
transaction containing those calls is closed correctly. See :ref:`the
notes on the requirements of Django's transaction handling
<topics-db-transactions-requirements>` for more details.
# Data modifying operation
cursor.execute("UPDATE bar SET foo = 1 WHERE baz = %s", [value])
.. versionchanged:: 1.3
# Since we modified data, mark the transaction as dirty
transaction.set_dirty()
# Data retrieval operation. This doesn't dirty the transaction,
# so no call to set_dirty() is required.
cursor.execute("SELECT foo FROM bar WHERE baz = %s", [value])
row = cursor.fetchone()
return render_to_response('template.html', {'row': row})
The call to ``set_dirty()`` is made automatically when you use the Django ORM
to make data modifying database calls. However, when you use raw SQL, Django
has no way of knowing if your SQL modifies data or not. The manual call to
``set_dirty()`` ensures that Django knows that there are modifications that
must be committed.
Prior to Django 1.3, it was necessary to manually mark a transaction
as dirty using ``transaction.set_dirty()`` when using raw SQL calls.
Connections and cursors
-----------------------

19
docs/topics/db/transactions.txt
View File

@ -187,6 +187,25 @@ managers, too.
def viewfunc2(request):
....
.. _topics-db-transactions-requirements:
Requirements for transaction handling
=====================================
.. versionadded:: 1.3
Django requires that every transaction that is opened is closed before
the completion of a request. If you are using :func:`autocommit` (the
default commit mode) or :func:`commit_on_success`, this will be done
for you automatically. However, if you are manually managing
transactions (using the :func:`commit_manually` decorator), you must
ensure that the transaction is either committed or rolled back before
a request is completed.
This applies to all database operations, not just write operations. Even
if your transaction only reads from the database, the transaction must
be committed or rolled back before you complete a request.
How to globally deactivate transaction management
=================================================

1
tests/regressiontests/delete_regress/tests.py
View File

@ -62,6 +62,7 @@ class DeleteLockingTest(TransactionTestCase):
Book.objects.filter(pagecount__lt=250).delete()
transaction.commit()
self.assertEqual(1, Book.objects.count())
transaction.commit()
class DeleteCascadeTests(TestCase):

1
tests/regressiontests/fixtures_regress/tests.py
View File

@ -610,6 +610,7 @@ class TestTicket11101(TransactionTestCase):
self.assertEqual(Thingy.objects.count(), 1)
transaction.rollback()
self.assertEqual(Thingy.objects.count(), 0)
transaction.commit()
@skipUnlessDBFeature('supports_transactions')
def test_ticket_11101(self):

0
tests/regressiontests/transactions_regress/__init__.py Normal file
View File

4
tests/regressiontests/transactions_regress/models.py Normal file
View File

@ -0,0 +1,4 @@
from django.db import models
class Mod(models.Model):
fld = models.IntegerField()

164
tests/regressiontests/transactions_regress/tests.py Normal file
View File

@ -0,0 +1,164 @@
from django.core.exceptions import ImproperlyConfigured
from django.db import connection, transaction
from django.db.transaction import commit_on_success, commit_manually, TransactionManagementError
from django.test import TransactionTestCase, skipUnlessDBFeature
from models import Mod
class TestTransactionClosing(TransactionTestCase):
"""
Tests to make sure that transactions are properly closed
when they should be, and aren't left pending after operations
have been performed in them. Refs #9964.
"""
def test_raw_committed_on_success(self):
"""
Make sure a transaction consisting of raw SQL execution gets
committed by the commit_on_success decorator.
"""
@commit_on_success
def raw_sql():
"Write a record using raw sql under a commit_on_success decorator"
cursor = connection.cursor()
cursor.execute("INSERT into transactions_regress_mod (id,fld) values (17,18)")
raw_sql()
# Rollback so that if the decorator didn't commit, the record is unwritten
transaction.rollback()
try:
# Check that the record is in the DB
obj = Mod.objects.get(pk=17)
self.assertEqual(obj.fld, 18)
except Mod.DoesNotExist:
self.fail("transaction with raw sql not committed")
def test_commit_manually_enforced(self):
"""
Make sure that under commit_manually, even "read-only" transaction require closure
(commit or rollback), and a transaction left pending is treated as an error.
"""
@commit_manually
def non_comitter():
"Execute a managed transaction with read-only operations and fail to commit"
_ = Mod.objects.count()
self.assertRaises(TransactionManagementError, non_comitter)
def test_commit_manually_commit_ok(self):
"""
Test that under commit_manually, a committed transaction is accepted by the transaction
management mechanisms
"""
@commit_manually
def committer():
"""
Perform a database query, then commit the transaction
"""
_ = Mod.objects.count()
transaction.commit()
try:
committer()
except TransactionManagementError:
self.fail("Commit did not clear the transaction state")
def test_commit_manually_rollback_ok(self):
"""
Test that under commit_manually, a rolled-back transaction is accepted by the transaction
management mechanisms
"""
@commit_manually
def roller_back():
"""
Perform a database query, then rollback the transaction
"""
_ = Mod.objects.count()
transaction.rollback()
try:
roller_back()
except TransactionManagementError:
self.fail("Rollback did not clear the transaction state")
def test_commit_manually_enforced_after_commit(self):
"""
Test that under commit_manually, if a transaction is committed and an operation is
performed later, we still require the new transaction to be closed
"""
@commit_manually
def fake_committer():
"Query, commit, then query again, leaving with a pending transaction"
_ = Mod.objects.count()
transaction.commit()
_ = Mod.objects.count()
self.assertRaises(TransactionManagementError, fake_committer)
@skipUnlessDBFeature('supports_transactions')
def test_reuse_cursor_reference(self):
"""
Make sure transaction closure is enforced even when the queries are performed
through a single cursor reference retrieved in the beginning
(this is to show why it is wrong to set the transaction dirty only when a cursor
is fetched from the connection).
"""
@commit_on_success
def reuse_cursor_ref():
"""
Fetch a cursor, perform an query, rollback to close the transaction,
then write a record (in a new transaction) using the same cursor object
(reference). All this under commit_on_success, so the second insert should
be committed.
"""
cursor = connection.cursor()
cursor.execute("INSERT into transactions_regress_mod (id,fld) values (1,2)")
transaction.rollback()
cursor.execute("INSERT into transactions_regress_mod (id,fld) values (1,2)")
reuse_cursor_ref()
# Rollback so that if the decorator didn't commit, the record is unwritten
transaction.rollback()
try:
# Check that the record is in the DB
obj = Mod.objects.get(pk=1)
self.assertEquals(obj.fld, 2)
except Mod.DoesNotExist:
self.fail("After ending a transaction, cursor use no longer sets dirty")
def test_failing_query_transaction_closed(self):
"""
Make sure that under commit_on_success, a transaction is rolled back even if
the first database-modifying operation fails.
This is prompted by http://code.djangoproject.com/ticket/6669 (and based on sample
code posted there to exemplify the problem): Before Django 1.3,
transactions were only marked "dirty" by the save() function after it successfully
wrote the object to the database.
"""
from django.contrib.auth.models import User
@transaction.commit_on_success
def create_system_user():
"Create a user in a transaction"
user = User.objects.create_user(username='system', password='iamr00t', email='root@SITENAME.com')
# Redundant, just makes sure the user id was read back from DB
Mod.objects.create(fld=user.id)
# Create a user
create_system_user()
try:
# The second call to create_system_user should fail for violating a unique constraint
# (it's trying to re-create the same user)
create_system_user()
except:
pass
else:
raise ImproperlyConfigured('Unique constraint not enforced on django.contrib.auth.models.User')
try:
# Try to read the database. If the last transaction was indeed closed,
# this should cause no problems
_ = User.objects.all()[0]
except:
self.fail("A transaction consisting of a failed operation was not closed.")