Mark for transaction rollback after APIException occurs.

[lovelydinosaur]

Superseeds #1204.

Could do with either an integration test, or a nice clear example from someone of currently failing behaviour with steps to replicate.

[lovelydinosaur]

Okay this'd need the @transaction.atomic decorator around dispatch. Also tried this approach, which looks nicer...

# initial bits of our dispatch method
try:
     with transaction.atomic():
        # main body of our dispatch method
except Exception as exc:
    response = self.handle_exception(exc)

(And the following in compat...)

if not hasattr(transaction, 'atomic'):
    transaction.atomic = transaction.commit_on_success

However, fails with AssertionError: 4 != 2 : 4 queries executed, 2 expected and similar errors.

[lovelydinosaur]

Actually note that the two additional queries are the savepoint and release.

QUERY = 'SAVEPOINT "s140735288746368_x1"' - PARAMS = ()

...and...

QUERY = 'RELEASE SAVEPOINT "s140735288746368_x1"' - PARAMS = ()

[lovelydinosaur]

I'm actually not convinced this is exactly the right thing to do in any case.

It does look to me like we're probably not respecting ATOMIC_REQUESTS = True as things stand, since we're handling the exceptions, but otherwise forcibly turning on atomic transactions for all APIViews doesn't seem correct. It's not the default for regular django views, and I'm not convinced it should be the default for us.

[lovelydinosaur]

To be more specific, not respecting ATOMIC_REQUESTS = True if an APIException is raised, although it's not even clear if that's problematic or not.

[lovelydinosaur]

Dropping the 2.4 milestone from this - what the desired behavior actually should be is not as clear cut as first assumed.

[Xinkai]

According to https://docs.djangoproject.com/en/dev/releases/1.6/#savepoints-and-assertnumqueries, assertNumQueries needs adjust to work with ATOMIC_REQUESTS.

I monkey-patched django.test.testcases._AssertNumQueriesContext so that it compares num + 2 with captured query count. All tests pass.

So I am thinking maybe we can provide another settings.py inside the tests folder, which enables atomic requests. Also monkey patch the test tools so that assertNumQueries always passes when atom requests is enabled, since the non-transaction-related query count shouldn't change.

[lovelydinosaur]

Closing in favour of issue #2034 as I don't think this PR takes the right approach. Needs more consideration.