Fix for issue #225 (exception when running with ATOMIC_REQUESTS), and the same…

Fix for issue #225 (exception when running with ATOMIC_REQUESTS), and the same applied to deleting subtrees

All explicit transaction handling has been moved to the model layer, and
uses transaction.commit_on_success or transaction.atomic, depending on the
Django version.

wiki/compat.py

+from django.db import transaction
+
+# 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
@@ -21,6 +15,7 @@ from mptt.fields import TreeForeignKey
 from mptt.models import MPTTModel
 
 from wiki import managers
+from wiki.compat import atomic, transaction_commit_on_success
 from wiki.conf import settings
 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.")
-            
-        
-    
     @classmethod
     def root(cls):
         site = Site.objects.get_current()
@@ -233,6 +220,8 @@ class URLPath(MPTTModel):
         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):