[connection] replace .running_dbapi_query with .hooks_in_progress
authorAurelien Campeas <aurelien.campeas@logilab.fr>
Tue, 01 Jul 2014 16:55:49 +0200
changeset 10351 91e63306e277
parent 10350 31327bd26931
child 10352 bab2befaac9b
[connection] replace .running_dbapi_query with .hooks_in_progress The thing was badly named. It tries to help distinguish between queries issued directly by the programmer (e.g in the views: cnx.execute(...)) from queries issued from the hooks, operations ... or even the repository or the native source objects. It worked heuristically being associated with the security being disabled. We provide a better name and an implementation distinct from the security management methods. Related to #3933480.
devtools/fake.py
hooks/notification.py
server/hook.py
server/repository.py
server/session.py
server/sources/native.py
--- a/devtools/fake.py	Tue May 05 08:41:19 2015 +0200
+++ b/devtools/fake.py	Tue Jul 01 16:55:49 2014 +0200
@@ -1,4 +1,4 @@
-# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2015 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -20,6 +20,8 @@
 
 __docformat__ = "restructuredtext en"
 
+from contextlib import contextmanager
+
 from logilab.database import get_db_helper
 
 from cubicweb.req import RequestSessionBase
@@ -159,6 +161,10 @@
     # for use with enabled_security context manager
     read_security = write_security = True
 
+    @contextmanager
+    def running_hooks_ops(self):
+        yield
+
 class FakeRepo(object):
     querier = None
     def __init__(self, schema, vreg=None, config=None):
--- a/hooks/notification.py	Tue May 05 08:41:19 2015 +0200
+++ b/hooks/notification.py	Tue Jul 01 16:55:49 2014 +0200
@@ -1,4 +1,4 @@
-# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2015 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -165,7 +165,7 @@
 class EntityUpdateHook(NotificationHook):
     __regid__ = 'notifentityupdated'
     __abstract__ = True # do not register by default
-    __select__ = NotificationHook.__select__ & hook.from_dbapi_query()
+    __select__ = NotificationHook.__select__ & hook.issued_from_user_query()
     events = ('before_update_entity',)
     skip_attrs = set()
 
@@ -200,7 +200,7 @@
 
 class SomethingChangedHook(NotificationHook):
     __regid__ = 'supervising'
-    __select__ = NotificationHook.__select__ & hook.from_dbapi_query()
+    __select__ = NotificationHook.__select__ & hook.issued_from_user_query()
     events = ('before_add_relation', 'before_delete_relation',
               'after_add_entity', 'before_update_entity')
 
--- a/server/hook.py	Tue May 05 08:41:19 2015 +0200
+++ b/server/hook.py	Tue Jul 01 16:55:49 2014 +0200
@@ -320,6 +320,7 @@
                 eids_from_to = []
             pruned = self.get_pruned_hooks(cnx, event,
                                            entities, eids_from_to, kwargs)
+
             # by default, hooks are executed with security turned off
             with cnx.security_enabled(read=False):
                 for _kwargs in _iter_kwargs(entities, eids_from_to, kwargs):
@@ -327,10 +328,11 @@
                                    key=lambda x: x.order)
                     debug = server.DEBUG & server.DBG_HOOKS
                     with cnx.security_enabled(write=False):
-                        for hook in hooks:
-                            if debug:
-                                print event, _kwargs, hook
-                            hook()
+                        with cnx.running_hooks_ops():
+                            for hook in hooks:
+                                if debug:
+                                    print event, _kwargs, hook
+                                hook()
 
     def get_pruned_hooks(self, cnx, event, entities, eids_from_to, kwargs):
         """return a set of hooks that should not be considered by filtered_possible objects
@@ -425,10 +427,13 @@
     return req.is_hook_activated(cls)
 
 @objectify_predicate
-def from_dbapi_query(cls, req, **kwargs):
-    if req.running_dbapi_query:
-        return 1
-    return 0
+def issued_from_user_query(cls, req, **kwargs):
+    return 0 if req.hooks_in_progress else 1
+
+from_dbapi_query = class_renamed('from_dbapi_query',
+                                 issued_from_user_query,
+                                 message='[3.21] ')
+
 
 class rechain(object):
     def __init__(self, *iterators):
--- a/server/repository.py	Tue May 05 08:41:19 2015 +0200
+++ b/server/repository.py	Tue Jul 01 16:55:49 2014 +0200
@@ -875,32 +875,33 @@
         # delete all its relations without security checking
         with cnx.security_enabled(read=False, write=False):
             in_eids = ','.join([str(_e.eid) for _e in entities])
-            for rschema, _, role in entities[0].e_schema.relation_definitions():
-                if rschema.rule:
-                    continue # computed relation
-                rtype = rschema.type
-                if rtype in schema.VIRTUAL_RTYPES or rtype in pendingrtypes:
-                    continue
-                if role == 'subject':
-                    # don't skip inlined relation so they are regularly
-                    # deleted and so hooks are correctly called
-                    rql = 'DELETE X %s Y WHERE X eid IN (%s)' % (rtype, in_eids)
-                else:
-                    rql = 'DELETE Y %s X WHERE X eid IN (%s)' % (rtype, in_eids)
-                try:
-                    cnx.execute(rql, build_descr=False)
-                except ValidationError:
-                    raise
-                except Unauthorized:
-                    self.exception('Unauthorized exception while cascading delete for entity %s. '
-                                   'RQL: %s.\nThis should not happen since security is disabled here.',
-                                   entities, rql)
-                    raise
-                except Exception:
-                    if self.config.mode == 'test':
+            with cnx.running_hooks_ops():
+                for rschema, _, role in entities[0].e_schema.relation_definitions():
+                    if rschema.rule:
+                        continue # computed relation
+                    rtype = rschema.type
+                    if rtype in schema.VIRTUAL_RTYPES or rtype in pendingrtypes:
+                        continue
+                    if role == 'subject':
+                        # don't skip inlined relation so they are regularly
+                        # deleted and so hooks are correctly called
+                        rql = 'DELETE X %s Y WHERE X eid IN (%s)' % (rtype, in_eids)
+                    else:
+                        rql = 'DELETE Y %s X WHERE X eid IN (%s)' % (rtype, in_eids)
+                    try:
+                        cnx.execute(rql, build_descr=False)
+                    except ValidationError:
                         raise
-                    self.exception('error while cascading delete for entity %s. RQL: %s',
-                                   entities, rql)
+                    except Unauthorized:
+                        self.exception('Unauthorized exception while cascading delete for entity %s. '
+                                       'RQL: %s.\nThis should not happen since security is disabled here.',
+                                       entities, rql)
+                        raise
+                    except Exception:
+                        if self.config.mode == 'test':
+                            raise
+                        self.exception('error while cascading delete for entity %s. RQL: %s',
+                                       entities, rql)
 
     def init_entity_caches(self, cnx, entity, source):
         """add entity to connection entities cache and repo's extid cache.
--- a/server/session.py	Tue May 05 08:41:19 2015 +0200
+++ b/server/session.py	Tue Jul 01 16:55:49 2014 +0200
@@ -1,4 +1,4 @@
-# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2015 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -388,8 +388,9 @@
 
     Database connection resources:
 
-      :attr:`running_dbapi_query`, boolean flag telling if the executing query
-      is coming from a dbapi connection or is a query from within the repository
+      :attr:`hooks_in_progress`, boolean flag telling if the executing
+      query is coming from a repoapi connection or is a query from
+      within the repository (e.g. started by hooks)
 
       :attr:`cnxset`, the connections set to use to execute queries on sources.
       If the transaction is read only, the connection set may be freed between
@@ -445,6 +446,7 @@
     """
 
     is_request = False
+    hooks_in_progress = False
     mode = 'read'
 
     def __init__(self, session, cnxid=None, session_handled=False):
@@ -484,8 +486,6 @@
         self._cnxset = None
         #: CnxSetTracker used to report cnxset usage
         self._cnxset_tracker = CnxSetTracker()
-        #: is this connection from a client or internal to the repo
-        self.running_dbapi_query = True
         # internal (root) session
         self.is_internal_session = isinstance(session.user, InternalManager)
 
@@ -526,7 +526,7 @@
             self._set_user(session.user)
 
 
-    # live cycle handling ####################################################
+    # life cycle handling ####################################################
 
     def __enter__(self):
         assert self._open is None # first opening
@@ -539,7 +539,18 @@
         self.rollback()
         self._open = False
 
+    @contextmanager
+    def running_hooks_ops(self):
+        """this context manager should be called whenever hooks or operations
+        are about to be run (but after hook selection)
 
+        It will help the undo logic record pertinent metadata or some
+        hooks to run (or not) depending on who/what issued the query.
+        """
+        prevmode = self.hooks_in_progress
+        self.hooks_in_progress = True
+        yield
+        self.hooks_in_progress = prevmode
 
     # shared data handling ###################################################
 
@@ -943,27 +954,7 @@
     @read_security.setter
     @_open_only
     def read_security(self, activated):
-        oldmode = self._read_security
         self._read_security = activated
-        # running_dbapi_query used to detect hooks triggered by a 'dbapi' query
-        # (eg not issued on the session). This is tricky since we the execution
-        # model of a (write) user query is:
-        #
-        # repository.execute (security enabled)
-        #  \-> querier.execute
-        #       \-> repo.glob_xxx (add/update/delete entity/relation)
-        #            \-> deactivate security before calling hooks
-        #                 \-> WE WANT TO CHECK QUERY NATURE HERE
-        #                      \-> potentially, other calls to querier.execute
-        #
-        # so we can't rely on simply checking session.read_security, but
-        # recalling the first transition from DEFAULT_SECURITY to something
-        # else (False actually) is not perfect but should be enough
-        #
-        # also reset running_dbapi_query to true when we go back to
-        # DEFAULT_SECURITY
-        self.running_dbapi_query = (oldmode is DEFAULT_SECURITY
-                                    or activated is DEFAULT_SECURITY)
 
     # undo support ############################################################
 
@@ -1098,13 +1089,14 @@
                 if debug:
                     print self.commit_state, '*' * 20
                 try:
-                    while self.pending_operations:
-                        operation = self.pending_operations.pop(0)
-                        operation.processed = 'precommit'
-                        processed.append(operation)
-                        if debug:
-                            print operation
-                        operation.handle_event('precommit_event')
+                    with self.running_hooks_ops():
+                        while self.pending_operations:
+                            operation = self.pending_operations.pop(0)
+                            operation.processed = 'precommit'
+                            processed.append(operation)
+                            if debug:
+                                print operation
+                            operation.handle_event('precommit_event')
                     self.pending_operations[:] = processed
                     self.debug('precommit transaction %s done', self.connectionid)
                 except BaseException:
@@ -1121,14 +1113,15 @@
                     operation.failed = True
                     if debug:
                         print self.commit_state, '*' * 20
-                    for operation in reversed(processed):
-                        if debug:
-                            print operation
-                        try:
-                            operation.handle_event('revertprecommit_event')
-                        except BaseException:
-                            self.critical('error while reverting precommit',
-                                          exc_info=True)
+                    with self.running_hooks_ops():
+                        for operation in reversed(processed):
+                            if debug:
+                                print operation
+                            try:
+                                operation.handle_event('revertprecommit_event')
+                            except BaseException:
+                                self.critical('error while reverting precommit',
+                                              exc_info=True)
                     # XXX use slice notation since self.pending_operations is a
                     # read-only property.
                     self.pending_operations[:] = processed + self.pending_operations
@@ -1138,16 +1131,17 @@
                 self.commit_state = 'postcommit'
                 if debug:
                     print self.commit_state, '*' * 20
-                while self.pending_operations:
-                    operation = self.pending_operations.pop(0)
-                    if debug:
-                        print operation
-                    operation.processed = 'postcommit'
-                    try:
-                        operation.handle_event('postcommit_event')
-                    except BaseException:
-                        self.critical('error while postcommit',
-                                      exc_info=sys.exc_info())
+                with self.running_hooks_ops():
+                    while self.pending_operations:
+                        operation = self.pending_operations.pop(0)
+                        if debug:
+                            print operation
+                        operation.processed = 'postcommit'
+                        try:
+                            operation.handle_event('postcommit_event')
+                        except BaseException:
+                            self.critical('error while postcommit',
+                                          exc_info=sys.exc_info())
                 self.debug('postcommit transaction %s done', self.connectionid)
                 return self.transaction_uuid(set=False)
         finally:
--- a/server/sources/native.py	Tue May 05 08:41:19 2015 +0200
+++ b/server/sources/native.py	Tue Jul 01 16:55:49 2014 +0200
@@ -1110,7 +1110,7 @@
         kwargs['tx_uuid'] = cnx.transaction_uuid()
         kwargs['txa_action'] = action
         kwargs['txa_order'] = cnx.transaction_inc_action_counter()
-        kwargs['txa_public'] = cnx.running_dbapi_query
+        kwargs['txa_public'] = not cnx.hooks_in_progress
         self.doexec(cnx, self.sqlgen.insert(table, kwargs), kwargs)
 
     def _tx_info(self, cnx, txuuid):