fix optimisation with super session that may lead to integrity loss
at some point I've decided to stop ensuring ?1 cardinality was respected
when adding a new relation using a super session, to avoid the cost of
the delete query. That was yet discussable because it introduced unexpected
difference between execute and unsafe_execute, which is imo not worth it.
Also, now that rql() in migration script default to unsafe_execute, we
definitly don't want that implicit behaviour change (which already cause
bug when for instance adding another default workflow for an entity type:
without that fix we end up with *two* default workflows while the schema
tells we can have only one.
IMO we should go to the direction that super session skip all security
check, but nothing else, unless explicitly asked.
--- a/hooks/email.py Thu Feb 18 15:42:29 2010 +0100
+++ b/hooks/email.py Fri Feb 19 09:34:14 2010 +0100
@@ -8,7 +8,6 @@
__docformat__ = "restructuredtext en"
from cubicweb.server import hook
-from cubicweb.server.repository import ensure_card_respected
from logilab.common.compat import any
@@ -27,11 +26,6 @@
def precommit_event(self):
if self.condition():
- # we've to handle cardinaly by ourselves since we're using unsafe_execute
- # but use session.execute and not session.unsafe_execute to check we
- # can change the relation
- ensure_card_respected(self.session.execute, self.session,
- self.entity.eid, self.rtype, self.email.eid)
self.session.unsafe_execute(
'SET X %s Y WHERE X eid %%(x)s, Y eid %%(y)s' % self.rtype,
{'x': self.entity.eid, 'y': self.email.eid}, 'x')
--- a/server/repository.py Thu Feb 18 15:42:29 2010 +0100
+++ b/server/repository.py Fri Feb 19 09:34:14 2010 +0100
@@ -101,14 +101,11 @@
this kind of behaviour has to be done in the repository so we don't have
hooks order hazardness
"""
- # skip delete queries (only?) if session is an internal session. This is
- # hooks responsability to ensure they do not violate relation's cardinality
- if session.is_super_session:
- return
- ensure_card_respected(session.unsafe_execute, session, eidfrom, rtype, eidto)
-
-
-def ensure_card_respected(execute, session, eidfrom, rtype, eidto):
+ # XXX now that rql in migraction default to unsafe_execute we don't want to
+ # skip that anymore. Also we should imo rely on the orm to first fetch
+ # existing entity if any then delete it
+ #if session.is_super_session:
+ # return
card = session.schema_rproperty(rtype, eidfrom, eidto, 'cardinality')
# one may be tented to check for neweids but this may cause more than one
# relation even with '1?' cardinality if thoses relations are added in the
@@ -118,12 +115,12 @@
# consistency.
if card[0] in '1?':
rschema = session.repo.schema.rschema(rtype)
- if not rschema.inlined:
- execute('DELETE X %s Y WHERE X eid %%(x)s,NOT Y eid %%(y)s' % rtype,
- {'x': eidfrom, 'y': eidto}, 'x')
+ if not rschema.inlined: # inlined relations will be implicitly deleted
+ session.unsafe_execute('DELETE X %s Y WHERE X eid %%(x)s, NOT Y eid %%(y)s' % rtype,
+ {'x': eidfrom, 'y': eidto}, 'x')
if card[1] in '1?':
- execute('DELETE X %s Y WHERE NOT X eid %%(x)s, Y eid %%(y)s' % rtype,
- {'x': eidfrom, 'y': eidto}, 'y')
+ session.unsafe_execute('DELETE X %s Y WHERE NOT X eid %%(x)s, Y eid %%(y)s' % rtype,
+ {'x': eidfrom, 'y': eidto}, 'y')
class Repository(object):