[connection] replace .running_dbapi_query with .direct_query

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>
changeset8f51d143f9fa
branchdefault
phasedraft
hiddenyes
parent revision#7a1ce6ea5a14 [shared data] remove get/set_shared_data api
child revision#d038b33a459e [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 8f51d143f9fa13c47ecf02da506a0038919a2c16
# Parent 7a1ce6ea5a143aa3404b71d23d46cf93bf7a8cd2
[connection] replace .running_dbapi_query with .direct_query

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