# HG changeset patch
# User Sylvain Thénault <sylvain.thenault@logilab.fr>
# Date 1285951787 -7200
# Node ID 9f91d09ee5fa5ccb3404da5e6f8ff7fffd4b2528
# Parent  19ebe0b994d62d298ca947b7115121bcc5c519a0
[repo transaction] fix rollback behaviour as discussed on the mailing-list: instead of rollbacking automatically on Unauthorized/ValidationError, mark the transaction as uncommitable and disallow commiting

diff -r 19ebe0b994d6 -r 9f91d09ee5fa doc/book/en/devrepo/devcore/dbapi.rst
--- a/doc/book/en/devrepo/devcore/dbapi.rst	Fri Oct 01 17:04:09 2010 +0200
+++ b/doc/book/en/devrepo/devcore/dbapi.rst	Fri Oct 01 18:49:47 2010 +0200
@@ -23,10 +23,11 @@
 .. note::
 
   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.
+  integrity (:exc:`ValidationError`), the transaction can still continue but you
+  won't be able to commit it, a rollback will be necessary to start a new
+  transaction.
 
-  Also, a rollback is done if an error occurs during commit.
+  Also, a rollback is automatically done if an error occurs during commit.
 
 
 Executing RQL queries from a view or a hook
diff -r 19ebe0b994d6 -r 9f91d09ee5fa server/querier.py
--- a/server/querier.py	Fri Oct 01 17:04:09 2010 +0200
+++ b/server/querier.py	Fri Oct 01 18:49:47 2010 +0200
@@ -711,7 +711,7 @@
             # * don't rollback if we're in the commit process, will be handled
             #   by the session
             if session.commit_state is None:
-                session.rollback(reset_pool=False)
+                session.commit_state = 'uncommitable'
             raise
         # build a description for the results if necessary
         descr = ()
diff -r 19ebe0b994d6 -r 9f91d09ee5fa server/session.py
--- a/server/session.py	Fri Oct 01 17:04:09 2010 +0200
+++ b/server/session.py	Fri Oct 01 18:49:47 2010 +0200
@@ -31,7 +31,7 @@
 from rql.nodes import ETYPE_PYOBJ_MAP, etype_from_pyobj
 from yams import BASE_TYPES
 
-from cubicweb import Binary, UnknownEid, schema
+from cubicweb import Binary, UnknownEid, QueryError, schema
 from cubicweb.req import RequestSessionBase
 from cubicweb.dbapi import ConnectionProperties
 from cubicweb.utils import make_uid, RepeatList
@@ -726,7 +726,10 @@
             self._touch()
             self.debug('commit session %s done (no db activity)', self.id)
             return
-        if self.commit_state:
+        cstate = self.commit_state
+        if cstate == 'uncommitable':
+            raise QueryError('transaction must be rollbacked')
+        if cstate is not None:
             return
         # on rollback, an operation should have the following state
         # information:
diff -r 19ebe0b994d6 -r 9f91d09ee5fa server/test/unittest_repository.py
--- a/server/test/unittest_repository.py	Fri Oct 01 17:04:09 2010 +0200
+++ b/server/test/unittest_repository.py	Fri Oct 01 18:49:47 2010 +0200
@@ -32,7 +32,7 @@
 from yams.constraints import UniqueConstraint
 
 from cubicweb import (BadConnectionId, RepositoryError, ValidationError,
-                      UnknownEid, AuthenticationError, Unauthorized)
+                      UnknownEid, AuthenticationError, Unauthorized, QueryError)
 from cubicweb.selectors import is_instance
 from cubicweb.schema import CubicWebSchema, RQLConstraint
 from cubicweb.dbapi import connect, multiple_connections_unfix
@@ -154,6 +154,10 @@
         with self.temporary_appobjects(ValidationErrorAfterHook):
             self.assertRaises(ValidationError,
                               self.execute, 'SET X name "toto" WHERE X is CWGroup, X name "guests"')
+            self.failUnless(self.execute('Any X WHERE X is CWGroup, X name "toto"'))
+            ex = self.assertRaises(QueryError, self.commit)
+            self.assertEqual(str(ex), 'transaction must be rollbacked')
+            self.rollback()
             self.failIf(self.execute('Any X WHERE X is CWGroup, X name "toto"'))
 
     def test_rollback_on_execute_unauthorized(self):
@@ -166,6 +170,10 @@
         with self.temporary_appobjects(UnauthorizedAfterHook):
             self.assertRaises(Unauthorized,
                               self.execute, 'SET X name "toto" WHERE X is CWGroup, X name "guests"')
+            self.failUnless(self.execute('Any X WHERE X is CWGroup, X name "toto"'))
+            ex = self.assertRaises(QueryError, self.commit)
+            self.assertEqual(str(ex), 'transaction must be rollbacked')
+            self.rollback()
             self.failIf(self.execute('Any X WHERE X is CWGroup, X name "toto"'))