[schema] fix composite deletion handling
Do not delete component entities if the composite is not deleted in
the same transaction.
Deletion semantics are thus: if the composite is deleted, the
components are deleted.
Closes #3463112.
--- a/doc/3.19.rst Fri Jan 24 13:08:53 2014 +0100
+++ b/doc/3.19.rst Tue Jan 21 18:30:16 2014 +0100
@@ -138,6 +138,11 @@
* ``set_cnxset`` and ``free_cnxset`` are deprecated. cnxset are now
automatically managed.
+* The implementation of cascading deletion when deleting `composite`
+ entities has changed. There comes a semantic change: merely deleting
+ a composite relation does not entail any more the deletion of the
+ component side of the relation.
+
Deprecated Code Drops
----------------------
--- a/hooks/integrity.py Fri Jan 24 13:08:53 2014 +0100
+++ b/hooks/integrity.py Tue Jan 21 18:30:16 2014 +0100
@@ -1,4 +1,4 @@
-# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
# contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
#
# This file is part of CubicWeb.
@@ -24,10 +24,10 @@
from threading import Lock
-from cubicweb import validation_error
+from cubicweb import validation_error, neg_role
from cubicweb.schema import (META_RTYPES, WORKFLOW_RTYPES,
RQLConstraint, RQLUniqueConstraint)
-from cubicweb.predicates import is_instance
+from cubicweb.predicates import is_instance, composite_etype
from cubicweb.uilib import soup2xhtml
from cubicweb.server import hook
@@ -309,69 +309,25 @@
self.entity.cw_edited['login'] = login.strip()
-# 'active' integrity hooks: you usually don't want to deactivate them, they are
-# not really integrity check, they maintain consistency on changes
-
-class _DelayedDeleteOp(hook.DataOperationMixIn, hook.Operation):
- """delete the object of composite relation except if the relation has
- actually been redirected to another composite
- """
- base_rql = None
-
- def precommit_event(self):
- session = self.session
- pendingeids = session.transaction_data.get('pendingeids', ())
- eids_by_etype_rtype = {}
- for eid, rtype in self.get_data():
- # don't do anything if the entity is being deleted
- if eid not in pendingeids:
- etype = session.entity_metas(eid)['type']
- key = (etype, rtype)
- if key not in eids_by_etype_rtype:
- eids_by_etype_rtype[key] = [str(eid)]
- else:
- eids_by_etype_rtype[key].append(str(eid))
- for (etype, rtype), eids in eids_by_etype_rtype.iteritems():
- # quite unexpectedly, not deleting too many entities at a time in
- # this operation benefits to the exec speed (possibly on the RQL
- # parsing side)
- start = 0
- incr = 500
- while start < len(eids):
- session.execute(self.base_rql % (etype, ','.join(eids[start:start+incr]), rtype))
- start += incr
-
-class _DelayedDeleteSEntityOp(_DelayedDeleteOp):
- """delete orphan subject entity of a composite relation"""
- base_rql = 'DELETE %s X WHERE X eid IN (%s), NOT X %s Y'
-
-class _DelayedDeleteOEntityOp(_DelayedDeleteOp):
- """check required object relation"""
- base_rql = 'DELETE %s X WHERE X eid IN (%s), NOT Y %s X'
-
-
class DeleteCompositeOrphanHook(hook.Hook):
- """delete the composed of a composite relation when this relation is deleted
+ """Delete the composed of a composite relation when the composite is
+ deleted (this is similar to the cascading ON DELETE CASCADE
+ semantics of sql).
"""
__regid__ = 'deletecomposite'
- events = ('before_delete_relation',)
+ __select__ = hook.Hook.__select__ & composite_etype()
+ events = ('before_delete_entity',)
category = 'activeintegrity'
def __call__(self):
- # if the relation is being delete, don't delete composite's components
- # automatically
- session = self._cw
- rtype = self.rtype
- rdef = session.rtype_eids_rdef(rtype, self.eidfrom, self.eidto)
- if (rdef.subject, rtype, rdef.object) in session.transaction_data.get('pendingrdefs', ()):
- return
- composite = rdef.composite
- if composite == 'subject':
- _DelayedDeleteOEntityOp.get_instance(self._cw).add_data(
- (self.eidto, rtype))
- elif composite == 'object':
- _DelayedDeleteSEntityOp.get_instance(self._cw).add_data(
- (self.eidfrom, rtype))
+ eid = self.entity.eid
+ for rdef, role in self.entity.e_schema.composite_rdef_roles:
+ rtype = rdef.rtype.type
+ target = getattr(rdef, neg_role(role))
+ expr = ('C %s X' % rtype) if role == 'subject' else ('X %s C' % rtype)
+ self._cw.execute('DELETE %s X WHERE C eid %%(c)s, %s' % (target, expr),
+ {'c': eid})
+
def registration_callback(vreg):
vreg.register_all(globals().values(), __name__)
--- a/hooks/syncschema.py Fri Jan 24 13:08:53 2014 +0100
+++ b/hooks/syncschema.py Tue Jan 21 18:30:16 2014 +0100
@@ -1123,20 +1123,20 @@
self._cw.transaction_data.setdefault(self.eidfrom, []).append(self.eidto)
-class BeforeDeleteConstrainedByHook(SyncSchemaHook):
- __regid__ = 'syncdelconstrainedby'
- __select__ = SyncSchemaHook.__select__ & hook.match_rtype('constrained_by')
- events = ('before_delete_relation',)
+class BeforeDeleteCWConstraintHook(SyncSchemaHook):
+ __regid__ = 'syncdelcwconstraint'
+ __select__ = SyncSchemaHook.__select__ & is_instance('CWConstraint')
+ events = ('before_delete_entity',)
def __call__(self):
- if self._cw.deleted_in_transaction(self.eidfrom):
- return
+ entity = self.entity
schema = self._cw.vreg.schema
- entity = self._cw.entity_from_eid(self.eidto)
- rdef = schema.schema_by_eid(self.eidfrom)
try:
+ # KeyError, e.g. composite chain deletion
+ rdef = schema.schema_by_eid(entity.reverse_constrained_by[0].eid)
+ # IndexError
cstr = rdef.constraint_by_type(entity.type)
- except IndexError:
+ except (IndexError, KeyError):
self._cw.critical('constraint type no more accessible')
else:
CWConstraintDelOp(self._cw, rdef=rdef, oldcstr=cstr)
--- a/hooks/test/data/schema.py Fri Jan 24 13:08:53 2014 +0100
+++ b/hooks/test/data/schema.py Tue Jan 21 18:30:16 2014 +0100
@@ -1,4 +1,4 @@
-# copyright 2003-2013 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
# contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
#
# This file is part of CubicWeb.
@@ -16,10 +16,23 @@
# You should have received a copy of the GNU Lesser General Public License along
# with CubicWeb. If not, see <http://www.gnu.org/licenses/>.
-from yams.buildobjs import RelationDefinition
+from yams.buildobjs import RelationDefinition, EntityType, String
class friend(RelationDefinition):
subject = ('CWUser', 'CWGroup')
object = ('CWUser', 'CWGroup')
symmetric = True
+class Folder(EntityType):
+ name = String()
+
+class parent(RelationDefinition):
+ subject = 'Folder'
+ object = 'Folder'
+ composite = 'object'
+ cardinality = '?*'
+
+class children(RelationDefinition):
+ subject = 'Folder'
+ object = 'Folder'
+ composite = 'subject'
--- a/hooks/test/unittest_integrity.py Fri Jan 24 13:08:53 2014 +0100
+++ b/hooks/test/unittest_integrity.py Tue Jan 21 18:30:16 2014 +0100
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
-# copyright 2003-2011 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
# contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
#
# This file is part of CubicWeb.
@@ -61,7 +61,7 @@
self.commit()
self.execute('DELETE Email X')
rset = self.execute('Any X WHERE X is EmailPart')
- self.assertEqual(len(rset), 1)
+ self.assertEqual(len(rset), 0)
self.commit()
rset = self.execute('Any X WHERE X is EmailPart')
self.assertEqual(len(rset), 0)
@@ -93,6 +93,42 @@
self.assertEqual(len(rset), 1)
self.assertEqual(rset.get_entity(0, 0).reverse_parts[0].messageid, '<2345>')
+ def test_composite_object_relation_deletion(self):
+ req = self.request()
+ root = req.create_entity('Folder', name=u'root')
+ a = req.create_entity('Folder', name=u'a', parent=root)
+ b = req.create_entity('Folder', name=u'b', parent=a)
+ c = req.create_entity('Folder', name=u'c', parent=root)
+ self.commit()
+ req = self.request()
+ req.execute('DELETE Folder F WHERE F name "a"')
+ req.execute('DELETE F parent R WHERE R name "root"')
+ self.commit()
+ req = self.request()
+ self.assertEqual([['root'], ['c']],
+ req.execute('Any NF WHERE F is Folder, F name NF').rows)
+ self.assertEqual([],
+ req.execute('Any NF,NP WHERE F parent P, F name NF, P name NP').rows)
+
+ def test_composite_subject_relation_deletion(self):
+ req = self.request()
+ root = req.create_entity('Folder', name=u'root')
+ a = req.create_entity('Folder', name=u'a')
+ b = req.create_entity('Folder', name=u'b')
+ c = req.create_entity('Folder', name=u'c')
+ root.cw_set(children=(a, c))
+ a.cw_set(children=b)
+ self.commit()
+ req = self.request()
+ req.execute('DELETE Folder F WHERE F name "a"')
+ req.execute('DELETE R children F WHERE R name "root"')
+ self.commit()
+ req = self.request()
+ self.assertEqual([['root'], ['c']],
+ req.execute('Any NF WHERE F is Folder, F name NF').rows)
+ self.assertEqual([],
+ req.execute('Any NF,NP WHERE F parent P, F name NF, P name NP').rows)
+
def test_unsatisfied_constraints(self):
releid = self.execute('SET U in_group G WHERE G name "owners", U login "admin"')[0][0]
with self.assertRaises(ValidationError) as cm:
--- a/hooks/test/unittest_syncschema.py Fri Jan 24 13:08:53 2014 +0100
+++ b/hooks/test/unittest_syncschema.py Tue Jan 21 18:30:16 2014 +0100
@@ -247,20 +247,16 @@
dbhelper = self.repo.system_source.dbhelper
sqlcursor = self.session.cnxset.cu
try:
- self.execute('INSERT CWConstraint X: X cstrtype CT, DEF constrained_by X '
- 'WHERE CT name "UniqueConstraint", DEF relation_type RT, DEF from_entity E,'
- 'RT name "name", E name "Workflow"')
+ eid = self.execute('INSERT CWConstraint X: X cstrtype CT, DEF constrained_by X '
+ 'WHERE CT name "UniqueConstraint", DEF relation_type RT, DEF from_entity E,'
+ 'RT name "name", E name "Workflow"').rows[0][0]
self.assertFalse(self.schema['Workflow'].has_unique_values('name'))
self.assertFalse(self.index_exists('Workflow', 'name', unique=True))
self.commit()
self.assertTrue(self.schema['Workflow'].has_unique_values('name'))
self.assertTrue(self.index_exists('Workflow', 'name', unique=True))
finally:
- self.execute('DELETE DEF constrained_by X WHERE X cstrtype CT, '
- 'CT name "UniqueConstraint", DEF relation_type RT, DEF from_entity E,'
- 'RT name "name", E name "Workflow"')
- self.assertTrue(self.schema['Workflow'].has_unique_values('name'))
- self.assertTrue(self.index_exists('Workflow', 'name', unique=True))
+ self.execute('DELETE CWConstraint C WHERE C eid %(eid)s', {'eid': eid})
self.commit()
self.assertFalse(self.schema['Workflow'].has_unique_values('name'))
self.assertFalse(self.index_exists('Workflow', 'name', unique=True))
--- a/server/test/unittest_hook.py Fri Jan 24 13:08:53 2014 +0100
+++ b/server/test/unittest_hook.py Tue Jan 21 18:30:16 2014 +0100
@@ -1,5 +1,5 @@
# -*- coding: utf-8 -*-
-# copyright 2003-2012 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
# contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
#
# This file is part of CubicWeb.
@@ -61,15 +61,10 @@
@clean_session_ops
def test_global_operation_order(self):
session = self.session
- op1 = integrity._DelayedDeleteOp(session)
- op2 = syncschema.RDefDelOp(session)
- # equivalent operation generated by op2 but replace it here by op3 so we
- # can check the result...
+ op1 = syncschema.RDefDelOp(session)
+ op2 = integrity._CheckORelationOp(session)
op3 = syncschema.MemSchemaNotifyChanges(session)
- op4 = integrity._DelayedDeleteOp(session)
- op5 = integrity._CheckORelationOp(session)
- self.assertEqual(session.pending_operations, [op1, op2, op4, op5, op3])
-
+ self.assertEqual([op1, op2, op3], session.pending_operations)
class HookCalled(Exception): pass
--- a/sobjects/test/unittest_supervising.py Fri Jan 24 13:08:53 2014 +0100
+++ b/sobjects/test/unittest_supervising.py Tue Jan 21 18:30:16 2014 +0100
@@ -1,5 +1,5 @@
# -*- coding: iso-8859-1 -*-
-# copyright 2003-2010 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
# contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
#
# This file is part of CubicWeb.
@@ -47,7 +47,7 @@
self.execute('DELETE Card B WHERE B title "une news !"')
self.execute('SET X bookmarked_by U WHERE X is Bookmark, U eid %(x)s', {'x': user.eid})
self.execute('SET X content "duh?" WHERE X is Comment')
- self.execute('DELETE X comments Y WHERE Y is Card, Y title "une autre news !"')
+ self.execute('DELETE Comment C WHERE C comments Y, Y is Card, Y title "une autre news !"')
# check only one supervision email operation
session = self.session
sentops = [op for op in session.pending_operations
@@ -75,7 +75,7 @@
* updated comment #EID (duh?)
http://testing.fr/cubicweb/comment/EID
-* deleted relation comments from comment #EID to card #EID''',
+* deleted comment #EID (duh?)''',
data)
# check prepared email
op._prepare_email()