# HG changeset patch
# User Sylvain Thénault <sylvain.thenault@logilab.fr>
# Date 1285755486 -7200
# Node ID 843684a50e4808a474daee3a919c5efdec5d9b58
# Parent  1edfc0f3bb1d2910ee956b11b61ad9be21d2a0a0
[transaction] to avoid potential db corruption, we should rollback systematically in case of ValidationError

diff -r 1edfc0f3bb1d -r 843684a50e48 doc/book/en/devrepo/devcore/dbapi.rst
--- a/doc/book/en/devrepo/devcore/dbapi.rst	Wed Sep 29 12:17:26 2010 +0200
+++ b/doc/book/en/devrepo/devcore/dbapi.rst	Wed Sep 29 12:18:06 2010 +0200
@@ -22,9 +22,12 @@
 
 .. note::
 
-  While executing update queries (SET, INSERT, DELETE), if a query generates
-  an error related to security, a rollback is automatically done on the current
-  transaction.
+  If a query generates an error related to security (:exc:`Unauthorized`) or to
+  integrity (:exc:`ValidationError`), a rollback is automatically done on the
+  current transaction.
+
+  Also, a rollback is done if an error occurs during commit.
+
 
 Executing RQL queries from a view or a hook
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff -r 1edfc0f3bb1d -r 843684a50e48 server/querier.py
--- a/server/querier.py	Wed Sep 29 12:17:26 2010 +0200
+++ b/server/querier.py	Wed Sep 29 12:18:06 2010 +0200
@@ -32,8 +32,8 @@
 from rql.nodes import (Relation, VariableRef, Constant, SubQuery, Function,
                        Exists, Not)
 
-from cubicweb import Unauthorized, QueryError, UnknownEid, typed_eid
-from cubicweb import server
+from cubicweb import ValidationError, Unauthorized, QueryError, UnknownEid
+from cubicweb import server, typed_eid
 from cubicweb.rset import ResultSet
 
 from cubicweb.server.utils import cleanup_solutions
@@ -701,15 +701,9 @@
         # execute the plan
         try:
             results = plan.execute()
-        except Unauthorized:
-            # XXX this could be done in security's after_add_relation hooks
-            # since it's actually realy only needed there (other relations
-            # security is done *before* actual changes, and add/update entity
-            # security is done after changes but in an operation, and exception
-            # generated in operation's events properly generate a rollback on
-            # the session). Even though, this is done here for a better
-            # consistency: getting an Unauthorized exception means the
-            # transaction has been rollbacked
+        except (Unauthorized, ValidationError):
+            # getting an Unauthorized/ValidationError exception means the
+            # transaction must been rollbacked
             #
             # notes:
             # * we should not reset the pool here, since we don't want the
diff -r 1edfc0f3bb1d -r 843684a50e48 server/test/unittest_repository.py
--- a/server/test/unittest_repository.py	Wed Sep 29 12:17:26 2010 +0200
+++ b/server/test/unittest_repository.py	Wed Sep 29 12:18:06 2010 +0200
@@ -32,7 +32,7 @@
 from yams.constraints import UniqueConstraint
 
 from cubicweb import (BadConnectionId, RepositoryError, ValidationError,
-                      UnknownEid, AuthenticationError)
+                      UnknownEid, AuthenticationError, Unauthorized)
 from cubicweb.selectors import is_instance
 from cubicweb.schema import CubicWebSchema, RQLConstraint
 from cubicweb.dbapi import connect, multiple_connections_unfix
@@ -136,15 +136,39 @@
         repo.close(cnxid)
         self.assert_(repo.connect(u"barnabé", password=u"héhéhé".encode('UTF8')))
 
-    def test_invalid_entity_rollback(self):
+    def test_rollback_on_commit_error(self):
         cnxid = self.repo.connect(self.admlogin, password=self.admpassword)
-        # no group
         self.repo.execute(cnxid,
                           'INSERT CWUser X: X login %(login)s, X upassword %(passwd)s',
                           {'login': u"tutetute", 'passwd': 'tutetute'})
         self.assertRaises(ValidationError, self.repo.commit, cnxid)
         self.failIf(self.repo.execute(cnxid, 'CWUser X WHERE X login "tutetute"'))
 
+    def test_rollback_on_execute_validation_error(self):
+        class ValidationErrorAfterHook(Hook):
+            __regid__ = 'valerror-after-hook'
+            __select__ = Hook.__select__ & is_instance('CWGroup')
+            events = ('after_update_entity',)
+            def __call__(self):
+                raise ValidationError(self.entity.eid, {})
+        with self.temporary_appobjects(ValidationErrorAfterHook):
+            self.assertRaises(ValidationError,
+                              self.execute, 'SET X name "toto" WHERE X is CWGroup, X name "guests"')
+            self.failIf(self.execute('Any X WHERE X is CWGroup, X name "toto"'))
+
+    def test_rollback_on_execute_unauthorized(self):
+        class UnauthorizedAfterHook(Hook):
+            __regid__ = 'valerror-after-hook'
+            __select__ = Hook.__select__ & is_instance('CWGroup')
+            events = ('after_update_entity',)
+            def __call__(self):
+                raise Unauthorized()
+        with self.temporary_appobjects(UnauthorizedAfterHook):
+            self.assertRaises(Unauthorized,
+                              self.execute, 'SET X name "toto" WHERE X is CWGroup, X name "guests"')
+            self.failIf(self.execute('Any X WHERE X is CWGroup, X name "toto"'))
+
+
     def test_close(self):
         repo = self.repo
         cnxid = repo.connect(self.admlogin, password=self.admpassword)