[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>
changeset44655aefe401
branchdefault
phasedraft
hiddenyes
parent revision#4fc664064835 [connection] provide some missing documentation bits
child revision#a00d6e08ec5a [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 44655aefe401233c27783a11e55b687046f9ab7d
# Parent 4fc66406483515e1d4e618d81d3a74d42a98a786
[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
@@ -976,31 +976,24 @@
106          pendingrtypes = session.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 session.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 -                rtype = rschema.type
113 -                if rtype in schema.VIRTUAL_RTYPES or rtype in pendingrtypes:
114 -                    continue
115 -                if role == 'subject':
116 -                    # don't skip inlined relation so they are regularly
117 -                    # deleted and so hooks are correctly called
118 -                    rql = 'DELETE X %s Y WHERE X eid IN (%s)' % (rtype, in_eids)
119 -                else:
120 -                    rql = 'DELETE Y %s X WHERE X eid IN (%s)' % (rtype, in_eids)
121 -                try:
122 -                    session.execute(rql, build_descr=False)
123 -                except ValidationError:
124 -                    raise
125 -                except Unauthorized:
126 -                    self.exception('Unauthorized exception while cascading delete for entity %s. '
127 -                                   'RQL: %s.\nThis should not happen since security is disabled here.',
128 -                                   entities, rql)
129 -                    raise
130 -                except Exception:
131 -                    if self.config.mode == 'test':
132 +            with session.running_hooks_ops():
133 +                for rschema, _, role in entities[0].e_schema.relation_definitions():
134 +                    rtype = rschema.type
135 +                    if rtype in schema.VIRTUAL_RTYPES or rtype in pendingrtypes:
136 +                        continue
137 +                    if role == 'subject':
138 +                        # don't skip inlined relation so they are regularly
139 +                        # deleted and so hooks are correctly called
140 +                        rql = 'DELETE X %s Y WHERE X eid IN (%s)' % (rtype, in_eids)
141 +                    else:
142 +                        rql = 'DELETE Y %s X WHERE X eid IN (%s)' % (rtype, in_eids)
143 +                    try:
144 +                        session.execute(rql, build_descr=False)
145 +                    except ValidationError:
146                          raise
147                      self.exception('error while cascading delete for entity %s. RQL: %s',
148                                     entities, rql)
149 
150      def init_entity_caches(self, cnx, entity, source):
diff --git a/server/session.py b/server/session.py
@@ -386,12 +386,13 @@
151 
152      Holds all connection related data
153 
154      Database connection resources:
155 
156 -      :attr:`running_dbapi_query`, boolean flag telling if the executing query
157 -      is coming from a dbapi connection or is a query from within the repository
158 +      :attr:`direct_query`, boolean flag telling if the executing
159 +      query is coming from a repoapi connection or is a query from
160 +      within the repository (e.g. started by hooks)
161 
162        :attr:`cnxset`, the connections set to use to execute queries on sources.
163        If the transaction is read only, the connection set may be freed between
164        actual queries. This allows multiple connections with a reasonably low
165        connection set pool size.  Control mechanism is detailed below.
@@ -443,10 +444,11 @@
166        read/write security is currently activated.
167 
168      """
169 
170      is_request = False
171 +    direct_query = True
172      mode = 'read'
173 
174      def __init__(self, session, cnxid=None, session_handled=False):
175          # using super(Connection, self) confuse some test hack
176          RequestSessionBase.__init__(self, session.vreg)
@@ -482,12 +484,10 @@
177 
178          #: connection set used to execute queries on sources
179          self._cnxset = None
180          #: CnxSetTracker used to report cnxset usage
181          self._cnxset_tracker = CnxSetTracker()
182 -        #: is this connection from a client or internal to the repo
183 -        self.running_dbapi_query = True
184          # internal (root) session
185          self.is_internal_session = isinstance(session.user, InternalManager)
186 
187          #: dict containing arbitrary data cleared at the end of the transaction
188          self.transaction_data = {}
@@ -524,11 +524,11 @@
189              self.set_language(self.user.prefered_language())
190          else:
191              self._set_user(session.user)
192 
193 
194 -    # live cycle handling ####################################################
195 +    # life cycle handling ####################################################
196 
197      def __enter__(self):
198          assert self._open is None # first opening
199          self._open = True
200          return self
@@ -538,10 +538,22 @@
201          assert self._cnxset_count == 0
202          self.rollback()
203          self._open = False
204 
205 
206 +    @contextmanager
207 +    def running_hooks_ops(self):
208 +        """this context manager should be called whenever hooks or operations
209 +        are about to be run
210 +
211 +        It will help the undo logic record pertinent metadata or some
212 +        hooks to run (or not) depending on who/what issued the query.
213 +        """
214 +        prevmode = self.direct_query
215 +        self.direct_query = False
216 +        yield
217 +        self.direct_query = prevmode
218 
219      # shared data handling ###################################################
220 
221      @property
222      def data(self):
@@ -919,31 +931,11 @@
223          return self._read_security
224 
225      @read_security.setter
226      @_open_only
227      def read_security(self, activated):
228 -        oldmode = self._read_security
229          self._read_security = activated
230 -        # running_dbapi_query used to detect hooks triggered by a 'dbapi' query
231 -        # (eg not issued on the session). This is tricky since we the execution
232 -        # model of a (write) user query is:
233 -        #
234 -        # repository.execute (security enabled)
235 -        #  \-> querier.execute
236 -        #       \-> repo.glob_xxx (add/update/delete entity/relation)
237 -        #            \-> deactivate security before calling hooks
238 -        #                 \-> WE WANT TO CHECK QUERY NATURE HERE
239 -        #                      \-> potentially, other calls to querier.execute
240 -        #
241 -        # so we can't rely on simply checking session.read_security, but
242 -        # recalling the first transition from DEFAULT_SECURITY to something
243 -        # else (False actually) is not perfect but should be enough
244 -        #
245 -        # also reset running_dbapi_query to true when we go back to
246 -        # DEFAULT_SECURITY
247 -        self.running_dbapi_query = (oldmode is DEFAULT_SECURITY
248 -                                    or activated is DEFAULT_SECURITY)
249 
250      # undo support ############################################################
251 
252      @_open_only
253      def ertype_supports_undo(self, ertype):
@@ -1074,17 +1066,18 @@
254                  processed = []
255                  self.commit_state = 'precommit'
256                  if debug:
257                      print self.commit_state, '*' * 20
258                  try:
259 -                    while self.pending_operations:
260 -                        operation = self.pending_operations.pop(0)
261 -                        operation.processed = 'precommit'
262 -                        processed.append(operation)
263 -                        if debug:
264 -                            print operation
265 -                        operation.handle_event('precommit_event')
266 +                    with self.running_hooks_ops():
267 +                        while self.pending_operations:
268 +                            operation = self.pending_operations.pop(0)
269 +                            operation.processed = 'precommit'
270 +                            processed.append(operation)
271 +                            if debug:
272 +                                print operation
273 +                            operation.handle_event('precommit_event')
274                      self.pending_operations[:] = processed
275                      self.debug('precommit transaction %s done', self.connectionid)
276                  except BaseException:
277                      # if error on [pre]commit:
278                      #
@@ -1097,37 +1090,39 @@
279                      # instead of having to implements rollback, revertprecommit
280                      # and revertcommit, that will be enough in mont case.
281                      operation.failed = True
282                      if debug:
283                          print self.commit_state, '*' * 20
284 -                    for operation in reversed(processed):
285 -                        if debug:
286 -                            print operation
287 -                        try:
288 -                            operation.handle_event('revertprecommit_event')
289 -                        except BaseException:
290 -                            self.critical('error while reverting precommit',
291 -                                          exc_info=True)
292 +                    with self.running_hooks_ops():
293 +                        for operation in reversed(processed):
294 +                            if debug:
295 +                                print operation
296 +                            try:
297 +                                operation.handle_event('revertprecommit_event')
298 +                            except BaseException:
299 +                                self.critical('error while reverting precommit',
300 +                                              exc_info=True)
301                      # XXX use slice notation since self.pending_operations is a
302                      # read-only property.
303                      self.pending_operations[:] = processed + self.pending_operations
304                      self.rollback(free_cnxset)
305                      raise
306                  self.cnxset.commit()
307                  self.commit_state = 'postcommit'
308                  if debug:
309                      print self.commit_state, '*' * 20
310 -                while self.pending_operations:
311 -                    operation = self.pending_operations.pop(0)
312 -                    if debug:
313 -                        print operation
314 -                    operation.processed = 'postcommit'
315 -                    try:
316 -                        operation.handle_event('postcommit_event')
317 -                    except BaseException:
318 -                        self.critical('error while postcommit',
319 -                                      exc_info=sys.exc_info())
320 +                with self.running_hooks_ops():
321 +                    while self.pending_operations:
322 +                        operation = self.pending_operations.pop(0)
323 +                        if debug:
324 +                            print operation
325 +                        operation.processed = 'postcommit'
326 +                        try:
327 +                            operation.handle_event('postcommit_event')
328 +                        except BaseException:
329 +                            self.critical('error while postcommit',
330 +                                          exc_info=sys.exc_info())
331                  self.debug('postcommit transaction %s done', self.connectionid)
332                  return self.transaction_uuid(set=False)
333          finally:
334              self._session_timestamp.touch()
335              if free_cnxset:
diff --git a/server/sources/native.py b/server/sources/native.py
@@ -1107,11 +1107,11 @@
336          'tx_entity_actions' or 'tx_relation_action')
337          """
338          kwargs['tx_uuid'] = cnx.transaction_uuid()
339          kwargs['txa_action'] = action
340          kwargs['txa_order'] = cnx.transaction_inc_action_counter()
341 -        kwargs['txa_public'] = cnx.running_dbapi_query
342 +        kwargs['txa_public'] = cnx.direct_query
343          self.doexec(cnx, self.sqlgen.insert(table, kwargs), kwargs)
344 
345      def _tx_info(self, cnx, txuuid):
346          """return transaction's time and user of the transaction with the given uuid.
347