Merge pull request #241 from spookylukey/fix_transaction_management

Fix transaction management - issue #225

README.md

@@ -90,6 +90,11 @@ Consider any moment in life that you could have been writing **unit tests** for
 
 The easiest way to add features is to write a plugin. Please create an issue to discuss whether your plugin idea is a core plugin (`wiki.plugins.*`) or external plugin. If there are additions needed to the plugin API, we can discuss that as well!
 
+To run the tests, run "python runtests.py" after installing the requirements. Better, install "tox" (using "pip install tox") and then just run "tox" to run the test suite on multiple environments.
+
+To run specific tests, call runtests.py with the arguments that you would pass to the normal "manage.py test" command.
+
+
 Manifesto
 ---------
 

runtests.py

+#!/usr/bin/env python
 import sys
+import django
 from django.conf import settings
 
 settings.configure(
@@ -45,14 +47,23 @@ settings.configure(
     SOUTH_TESTS_MIGRATE=True,
 )
 
-from django.test.simple import DjangoTestSuiteRunner
-test_runner = DjangoTestSuiteRunner(verbosity=1)
 
 # If you use South for migrations, uncomment this to monkeypatch
 # syncdb to get migrations to run.
 from south.management.commands import patch_for_test_db_setup
 patch_for_test_db_setup()
 
-failures = test_runner.run_tests(['wiki', ])
-if failures:
-    sys.exit(failures)
+from django.core.management import execute_from_command_line
+argv = [sys.argv[0], "test"]
+
\ No newline at end of file
+if len(sys.argv) == 1:
+    # Nothing following 'runtests.py':
+    if django.VERSION < (1,6):
+        argv.append("wiki")
+    else:
+        argv.append("wiki.tests")
+else:
+    # Allow tests to be specified:
+    argv.extend(sys.argv[1:])
+
+execute_from_command_line(argv)

setup.py

+#!/usr/bin/env python
 # -*- coding: utf-8 -*-
 import os
 from wiki import VERSION

tox.ini

+[tox]
+envlist = py27-django14, py27-django15, py27-django16
+
+[testenv]
+commands = ./runtests.py
+deps =
+     Markdown==2.3.1
+     Pillow==2.3.0
+     South==0.8.4
+     argparse==1.2.1
+     django-classy-tags==0.4
+     django-mptt==0.6.0
+     django-sekizai==0.7
+     sorl-thumbnail==11.12
+     wiki==0.0.23
+     wsgiref==0.1.2
+
+[testenv:py27-django14]
+basepython = python2.7
+deps =
+     Django==1.4.10
+     {[testenv]deps}
+
+[testenv:py27-django15]
+basepython = python2.7
+deps =
+     Django==1.5.5
+     {[testenv]deps}
+
+[testenv:py27-django16]
+basepython = python2.7
+deps =
+     Django==1.6.1
+     {[testenv]deps}

wiki/core/compat.py

 """Abstraction layer to deal with Django related changes in order to keep
 compatibility with several Django versions simultaneously."""
 from django import VERSION as DJANGO_VERSION
+from django.db import transaction
 from django.conf import settings as django_settings
+
     
 # Django 1.5+
 if DJANGO_VERSION >= (1,5):
@@ -17,3 +19,18 @@ def get_user_model():
     else:
         from django.contrib.auth.models import User
         return User
+
+
+# Django 1.6 transaction API, required for 1.8+
+def nop_decorator(func):
+    return func
+
+# Where these are used in code, both old and new methods for transactions appear
+# to be used, but only one will actually do anything. When only Django 1.8+
+# is supported, transaction_commit_on_success can be deleted.
+try:
+    atomic = transaction.atomic # Does it exist?
+    transaction_commit_on_success = nop_decorator
+except AttributeError:
+    atomic = nop_decorator
+    transaction_commit_on_success = transaction.commit_on_success

wiki/models/urlpath.py

@@ -6,13 +6,7 @@ from django.contrib.contenttypes.models import ContentType
 from django.contrib.sites.models import Site
 from django.core.exceptions import ValidationError
 from django.core.urlresolvers import reverse
-from django.db import models, transaction
-
-#Django 1.6 transaction API, required for 1.8+
-try:
-   notrans=transaction.non_atomic_requests 
-except:
-   notrans=transaction.commit_manually
+from django.db import models
 
 from django.db.models.signals import post_save, pre_delete
 from django.utils.translation import ugettext_lazy as _, ugettext
@@ -22,6 +16,7 @@ from mptt.models import MPTTModel
 
 from wiki import managers
 from wiki.conf import settings
+from wiki.core.compat import atomic, transaction_commit_on_success
 from wiki.core.exceptions import NoRootURL, MultipleRootURLs
 from wiki.models.article import ArticleRevision, ArticleForObject, Article
 
@@ -115,24 +110,16 @@ class URLPath(MPTTModel):
                 return ancestor
         return None
     
-    @notrans
+    @atomic
+    @transaction_commit_on_success
     def delete_subtree(self):
         """
         NB! This deletes this urlpath, its children, and ALL of the related
         articles. This is a purged delete and CANNOT be undone.
         """
-        try:
-            for descendant in self.get_descendants(include_self=True).order_by("-level"):
-                print "deleting " , descendant
-                descendant.article.delete()
-            
-            transaction.commit()
-        except:
-            transaction.rollback()
-            log.exception("Exception deleting article subtree.")
-            
-        
-    
+        for descendant in self.get_descendants(include_self=True).order_by("-level"):
+            descendant.article.delete()
+
     @classmethod
     def root(cls):
         site = Site.objects.get_current()
@@ -231,8 +218,10 @@ class URLPath(MPTTModel):
         else:
             root = root_nodes[0]
         return root
-        
+
     @classmethod
+    @atomic
+    @transaction_commit_on_success
     def create_article(cls, parent, slug, site=None, title="Root", article_kwargs={}, **kwargs):
         """Utility function:
         Create a new urlpath with an article and a new revision for the article"""

wiki/views/article.py

 # -*- coding: utf-8 -*-
 import difflib
+import logging
 
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required
@@ -19,18 +20,13 @@ from wiki.core.plugins import registry as plugin_registry
 from wiki.core.diff import simple_merge
 from wiki.decorators import get_article, json_view
 from django.core.urlresolvers import reverse
-from django.db import transaction
-
-#Django 1.6 transaction API, required for 1.8+
-try:
-   notrans=transaction.non_atomic_requests 
-except:
-   notrans=transaction.commit_manually
 
 from wiki.core.exceptions import NoRootURL
 from wiki.core import permissions
 from django.http import Http404
 
+log = logging.getLogger(__name__)
+
 
 class ArticleView(ArticleMixin, TemplateView):
 
@@ -67,7 +63,6 @@ class Create(FormView, ArticleMixin):
         form.fields['slug'].widget = forms.TextInputPrepend(prepend='/'+self.urlpath.path)
         return form
 
-    @notrans
     def form_valid(self, form):
         user=None
         ip_address = None
@@ -77,6 +72,7 @@ class Create(FormView, ArticleMixin):
                 ip_address = self.request.META.get('REMOTE_ADDR', None)
         elif settings.LOG_IPS_ANONYMOUS:
             ip_address = self.request.META.get('REMOTE_ADDR', None)
+
         try:
             self.newpath = models.URLPath.create_article(
                 self.urlpath,
@@ -94,20 +90,15 @@ class Create(FormView, ArticleMixin):
                                 'other_write': self.article.other_write,
                                 })
             messages.success(self.request, _(u"New article '%s' created.") % self.newpath.article.current_revision.title)
-
-            transaction.commit()
-        # TODO: Handle individual exceptions better and give good feedback.
         except Exception, e:
-            transaction.rollback()
+            log.exception("Exception creating article.")
             if self.request.user.is_superuser:
                 messages.error(self.request, _(u"There was an error creating this article: %s") % str(e))
             else:
                 messages.error(self.request, _(u"There was an error creating this article."))
-            transaction.commit()
             return redirect('wiki:get', '')
 
         url = self.get_success_url()
-        transaction.commit()
         return url
 
     def get_success_url(self):