[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.

authorAurelien Campeas <aurelien.campeas@logilab.fr>
changesetbab17bccca16
branchdefault
phasedraft
hiddenyes
parent revision#74b9f52ccf55 [shared data] remove get/set_shared_data api
child revision#eb2dbe12a3bb [migractions] remove any session related leftovers
files modified by this revision
devtools/fake.py
hooks/notification.py
server/hook.py
server/repository.py
server/session.py
server/sources/native.py
# HG changeset patch
# User Aurelien Campeas <aurelien.campeas@logilab.fr>
# Date 1404226549 -7200
# Tue Jul 01 16:55:49 2014 +0200
# Node ID bab17bccca161dde5fed68bbb8fa0fe91c911e9c
# Parent 74b9f52ccf55b36236809a0165d9db9546b064eb
[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 --git a/devtools/fake.py b/devtools/fake.py
@@ -1,6 +1,6 @@
1 -# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
2 +# copyright 2003-2015 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
3  # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
4  #
5  # This file is part of CubicWeb.
6  #
7  # CubicWeb is free software: you can redistribute it and/or modify it under the
@@ -18,10 +18,12 @@
8  """Fake objects to ease testing of cubicweb without a fully working environment
9  """
10 
11  __docformat__ = "restructuredtext en"
12 
13 +from contextlib import contextmanager
14 +
15  from logilab.database import get_db_helper
16 
17  from cubicweb.req import RequestSessionBase
18  from cubicweb.cwvreg import CWRegistryStore
19  from cubicweb.web.request import ConnectionCubicWebRequestBase
@@ -157,10 +159,14 @@
20          return FakeCM()
21 
22      # for use with enabled_security context manager
23      read_security = write_security = True
24 
25 +    @contextmanager
26 +    def running_hooks_ops(self):
27 +        yield
28 +
29  class FakeRepo(object):
30      querier = None
31      def __init__(self, schema, vreg=None, config=None):
32          self.extids = {}
33          self.eids = {}
diff --git a/hooks/notification.py b/hooks/notification.py
@@ -1,6 +1,6 @@
34 -# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
35 +# copyright 2003-2015 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
36  # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
37  #
38  # This file is part of CubicWeb.
39  #
40  # CubicWeb is free software: you can redistribute it and/or modify it under the
@@ -163,11 +163,11 @@
41 
42 
43  class EntityUpdateHook(NotificationHook):
44      __regid__ = 'notifentityupdated'
45      __abstract__ = True # do not register by default
46 -    __select__ = NotificationHook.__select__ & hook.from_dbapi_query()
47 +    __select__ = NotificationHook.__select__ & hook.not_issued_from_hooks()
48      events = ('before_update_entity',)
49      skip_attrs = set()
50 
51      def __call__(self):
52          cnx = self._cw
@@ -198,11 +198,11 @@
53 
54  # supervising ##################################################################
55 
56  class SomethingChangedHook(NotificationHook):
57      __regid__ = 'supervising'
58 -    __select__ = NotificationHook.__select__ & hook.from_dbapi_query()
59 +    __select__ = NotificationHook.__select__ & hook.not_issued_from_hooks()
60      events = ('before_add_relation', 'before_delete_relation',
61                'after_add_entity', 'before_update_entity')
62 
63      def __call__(self):
64          dest = self._cw.vreg.config['supervising-addrs']
diff --git a/server/hook.py b/server/hook.py
@@ -318,21 +318,23 @@
65              else:
66                  entities = []
67                  eids_from_to = []
68              pruned = self.get_pruned_hooks(cnx, event,
69                                             entities, eids_from_to, kwargs)
70 +
71              # by default, hooks are executed with security turned off
72              with cnx.security_enabled(read=False):
73                  for _kwargs in _iter_kwargs(entities, eids_from_to, kwargs):
74                      hooks = sorted(self.filtered_possible_objects(pruned, cnx, **_kwargs),
75                                     key=lambda x: x.order)
76                      debug = server.DEBUG & server.DBG_HOOKS
77                      with cnx.security_enabled(write=False):
78 -                        for hook in hooks:
79 -                            if debug:
80 -                                print event, _kwargs, hook
81 -                            hook()
82 +                        with cnx.running_hooks_ops():
83 +                            for hook in hooks:
84 +                                if debug:
85 +                                    print event, _kwargs, hook
86 +                                hook()
87 
88      def get_pruned_hooks(self, cnx, event, entities, eids_from_to, kwargs):
89          """return a set of hooks that should not be considered by filtered_possible objects
90 
91          the idea is to make a first pass over all the hooks in the
@@ -423,14 +425,19 @@
92      if req is None:
93          return True # XXX how to deactivate server startup / shutdown event
94      return req.is_hook_activated(cls)
95 
96  @objectify_predicate
97 -def from_dbapi_query(cls, req, **kwargs):
98 -    if req.running_dbapi_query:
99 -        return 1
100 -    return 0
101 +def not_issued_from_hooks(cls, req, **kwargs):
102 +    if req.hooks_in_progress:
103 +        return 0
104 +    return 1
105 +
106 +from_dbapi_query = class_renamed('from_dbapi_query',
107 +                                 not_issued_from_hooks,
108 +                                 message='[3.21] ')
109 +
110 
111  class rechain(object):
112      def __init__(self, *iterators):
113          self.iterators = iterators
114      def __iter__(self):
diff --git a/server/repository.py b/server/repository.py
@@ -873,36 +873,37 @@
115          pendingrtypes = cnx.transaction_data.get('pendingrtypes', ())
116          # delete remaining relations: if user can delete the entity, he can
117          # delete all its relations without security checking
118          with cnx.security_enabled(read=False, write=False):
119              in_eids = ','.join([str(_e.eid) for _e in entities])
120 -            for rschema, _, role in entities[0].e_schema.relation_definitions():
121 -                if rschema.rule:
122 -                    continue # computed relation
123 -                rtype = rschema.type
124 -                if rtype in schema.VIRTUAL_RTYPES or rtype in pendingrtypes:
125 -                    continue
126 -                if role == 'subject':
127 -                    # don't skip inlined relation so they are regularly
128 -                    # deleted and so hooks are correctly called
129 -                    rql = 'DELETE X %s Y WHERE X eid IN (%s)' % (rtype, in_eids)
130 -                else:
131 -                    rql = 'DELETE Y %s X WHERE X eid IN (%s)' % (rtype, in_eids)
132 -                try:
133 -                    cnx.execute(rql, build_descr=False)
134 -                except ValidationError:
135 -                    raise
136 -                except Unauthorized:
137 -                    self.exception('Unauthorized exception while cascading delete for entity %s. '
138 -                                   'RQL: %s.\nThis should not happen since security is disabled here.',
139 -                                   entities, rql)
140 -                    raise
141 -                except Exception:
142 -                    if self.config.mode == 'test':
143 +            with cnx.running_hooks_ops():
144 +                for rschema, _, role in entities[0].e_schema.relation_definitions():
145 +                    if rschema.rule:
146 +                        continue # computed relation
147 +                    rtype = rschema.type
148 +                    if rtype in schema.VIRTUAL_RTYPES or rtype in pendingrtypes:
149 +                        continue
150 +                    if role == 'subject':
151 +                        # don't skip inlined relation so they are regularly
152 +                        # deleted and so hooks are correctly called
153 +                        rql = 'DELETE X %s Y WHERE X eid IN (%s)' % (rtype, in_eids)
154 +                    else:
155 +                        rql = 'DELETE Y %s X WHERE X eid IN (%s)' % (rtype, in_eids)
156 +                    try:
157 +                        cnx.execute(rql, build_descr=False)
158 +                    except ValidationError:
159                          raise
160 -                    self.exception('error while cascading delete for entity %s. RQL: %s',
161 -                                   entities, rql)
162 +                    except Unauthorized:
163 +                        self.exception('Unauthorized exception while cascading delete for entity %s. '
164 +                                       'RQL: %s.\nThis should not happen since security is disabled here.',
165 +                                       entities, rql)
166 +                        raise
167 +                    except Exception:
168 +                        if self.config.mode == 'test':
169 +                            raise
170 +                        self.exception('error while cascading delete for entity %s. RQL: %s',
171 +                                       entities, rql)
172 
173      def init_entity_caches(self, cnx, entity, source):
174          """add entity to connection entities cache and repo's extid cache.
175          Return entity's ext id if the source isn't the system source.
176          """
diff --git a/server/session.py b/server/session.py
@@ -1,6 +1,6 @@
177 -# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
178 +# copyright 2003-2015 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
179  # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
180  #
181  # This file is part of CubicWeb.
182  #
183  # CubicWeb is free software: you can redistribute it and/or modify it under the
@@ -386,12 +386,13 @@
184 
185      Holds all connection related data
186 
187      Database connection resources:
188 
189 -      :attr:`running_dbapi_query`, boolean flag telling if the executing query
190 -      is coming from a dbapi connection or is a query from within the repository
191 +      :attr:`hooks_in_progress`, boolean flag telling if the executing
192 +      query is coming from a repoapi connection or is a query from
193 +      within the repository (e.g. started by hooks)
194 
195        :attr:`cnxset`, the connections set to use to execute queries on sources.
196        If the transaction is read only, the connection set may be freed between
197        actual queries. This allows multiple connections with a reasonably low
198        connection set pool size.  Control mechanism is detailed below.
@@ -443,10 +444,11 @@
199        read/write security is currently activated.
200 
201      """
202 
203      is_request = False
204 +    hooks_in_progress = False
205      mode = 'read'
206 
207      def __init__(self, session, cnxid=None, session_handled=False):
208          # using super(Connection, self) confuse some test hack
209          RequestSessionBase.__init__(self, session.vreg)
@@ -482,12 +484,10 @@
210 
211          #: connection set used to execute queries on sources
212          self._cnxset = None
213          #: CnxSetTracker used to report cnxset usage
214          self._cnxset_tracker = CnxSetTracker()
215 -        #: is this connection from a client or internal to the repo
216 -        self.running_dbapi_query = True
217          # internal (root) session
218          self.is_internal_session = isinstance(session.user, InternalManager)
219 
220          #: dict containing arbitrary data cleared at the end of the transaction
221          self.transaction_data = {}
@@ -524,11 +524,11 @@
222              self.set_language(self.user.prefered_language())
223          else:
224              self._set_user(session.user)
225 
226 
227 -    # live cycle handling ####################################################
228 +    # life cycle handling ####################################################
229 
230      def __enter__(self):
231          assert self._open is None # first opening
232          self._open = True
233          return self
@@ -538,10 +538,22 @@
234          assert self._cnxset_count == 0
235          self.rollback()
236          self._open = False
237 
238 
239 +    @contextmanager
240 +    def running_hooks_ops(self):
241 +        """this context manager should be called whenever hooks or operations
242 +        are about to be run
243 +
244 +        It will help the undo logic record pertinent metadata or some
245 +        hooks to run (or not) depending on who/what issued the query.
246 +        """
247 +        prevmode = self.hooks_in_progress
248 +        self.hooks_in_progress = True
249 +        yield
250 +        self.hooks_in_progress = prevmode
251 
252      # shared data handling ###################################################
253 
254      @property
255      def data(self):
@@ -919,31 +931,11 @@
256          return self._read_security
257 
258      @read_security.setter
259      @_open_only
260      def read_security(self, activated):
261 -        oldmode = self._read_security
262          self._read_security = activated
263 -        # running_dbapi_query used to detect hooks triggered by a 'dbapi' query
264 -        # (eg not issued on the session). This is tricky since we the execution
265 -        # model of a (write) user query is:
266 -        #
267 -        # repository.execute (security enabled)
268 -        #  \-> querier.execute
269 -        #       \-> repo.glob_xxx (add/update/delete entity/relation)
270 -        #            \-> deactivate security before calling hooks
271 -        #                 \-> WE WANT TO CHECK QUERY NATURE HERE
272 -        #                      \-> potentially, other calls to querier.execute
273 -        #
274 -        # so we can't rely on simply checking session.read_security, but
275 -        # recalling the first transition from DEFAULT_SECURITY to something
276 -        # else (False actually) is not perfect but should be enough
277 -        #
278 -        # also reset running_dbapi_query to true when we go back to
279 -        # DEFAULT_SECURITY
280 -        self.running_dbapi_query = (oldmode is DEFAULT_SECURITY
281 -                                    or activated is DEFAULT_SECURITY)
282 
283      # undo support ############################################################
284 
285      @_open_only
286      def ertype_supports_undo(self, ertype):
@@ -1074,17 +1066,18 @@
287                  processed = []
288                  self.commit_state = 'precommit'
289                  if debug:
290                      print self.commit_state, '*' * 20
291                  try:
292 -                    while self.pending_operations:
293 -                        operation = self.pending_operations.pop(0)
294 -                        operation.processed = 'precommit'
295 -                        processed.append(operation)
296 -                        if debug:
297 -                            print operation
298 -                        operation.handle_event('precommit_event')
299 +                    with self.running_hooks_ops():
300 +                        while self.pending_operations:
301 +                            operation = self.pending_operations.pop(0)
302 +                            operation.processed = 'precommit'
303 +                            processed.append(operation)
304 +                            if debug:
305 +                                print operation
306 +                            operation.handle_event('precommit_event')
307                      self.pending_operations[:] = processed
308                      self.debug('precommit transaction %s done', self.connectionid)
309                  except BaseException:
310                      # if error on [pre]commit:
311                      #
@@ -1097,37 +1090,39 @@
312                      # instead of having to implements rollback, revertprecommit
313                      # and revertcommit, that will be enough in mont case.
314                      operation.failed = True
315                      if debug:
316                          print self.commit_state, '*' * 20
317 -                    for operation in reversed(processed):
318 -                        if debug:
319 -                            print operation
320 -                        try:
321 -                            operation.handle_event('revertprecommit_event')
322 -                        except BaseException:
323 -                            self.critical('error while reverting precommit',
324 -                                          exc_info=True)
325 +                    with self.running_hooks_ops():
326 +                        for operation in reversed(processed):
327 +                            if debug:
328 +                                print operation
329 +                            try:
330 +                                operation.handle_event('revertprecommit_event')
331 +                            except BaseException:
332 +                                self.critical('error while reverting precommit',
333 +                                              exc_info=True)
334                      # XXX use slice notation since self.pending_operations is a
335                      # read-only property.
336                      self.pending_operations[:] = processed + self.pending_operations
337                      self.rollback(free_cnxset)
338                      raise
339                  self.cnxset.commit()
340                  self.commit_state = 'postcommit'
341                  if debug:
342                      print self.commit_state, '*' * 20
343 -                while self.pending_operations:
344 -                    operation = self.pending_operations.pop(0)
345 -                    if debug:
346 -                        print operation
347 -                    operation.processed = 'postcommit'
348 -                    try:
349 -                        operation.handle_event('postcommit_event')
350 -                    except BaseException:
351 -                        self.critical('error while postcommit',
352 -                                      exc_info=sys.exc_info())
353 +                with self.running_hooks_ops():
354 +                    while self.pending_operations:
355 +                        operation = self.pending_operations.pop(0)
356 +                        if debug:
357 +                            print operation
358 +                        operation.processed = 'postcommit'
359 +                        try:
360 +                            operation.handle_event('postcommit_event')
361 +                        except BaseException:
362 +                            self.critical('error while postcommit',
363 +                                          exc_info=sys.exc_info())
364                  self.debug('postcommit transaction %s done', self.connectionid)
365                  return self.transaction_uuid(set=False)
366          finally:
367              self._session_timestamp.touch()
368              if free_cnxset:
diff --git a/server/sources/native.py b/server/sources/native.py
@@ -1108,11 +1108,11 @@
369          'tx_entity_actions' or 'tx_relation_action')
370          """
371          kwargs['tx_uuid'] = cnx.transaction_uuid()
372          kwargs['txa_action'] = action
373          kwargs['txa_order'] = cnx.transaction_inc_action_counter()
374 -        kwargs['txa_public'] = cnx.running_dbapi_query
375 +        kwargs['txa_public'] = not cnx.hooks_in_progress
376          self.doexec(cnx, self.sqlgen.insert(table, kwargs), kwargs)
377 
378      def _tx_info(self, cnx, txuuid):
379          """return transaction's time and user of the transaction with the given uuid.
380