# HG changeset patch # User Aurelien Campeas # Date 1404226549 -7200 # Node ID 91e63306e277d0db942c5d09f3b0b4f43ebbc3bb # Parent 31327bd269318c13990649796fc62ccb7784d0fe [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. diff -r 31327bd26931 -r 91e63306e277 devtools/fake.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): diff -r 31327bd26931 -r 91e63306e277 hooks/notification.py --- 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') diff -r 31327bd26931 -r 91e63306e277 server/hook.py --- 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): diff -r 31327bd26931 -r 91e63306e277 server/repository.py --- 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. diff -r 31327bd26931 -r 91e63306e277 server/session.py --- 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: diff -r 31327bd26931 -r 91e63306e277 server/sources/native.py --- 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):