[schema] fix composite deletion handling
authorAurelien Campeas <aurelien.campeas@logilab.fr>
Tue, 21 Jan 2014 18:30:16 +0100
changeset 9548 be001628edad
parent 9547 43aace16a953
child 9549 a1f94c2d2a39
[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.
doc/3.19.rst
hooks/integrity.py
hooks/syncschema.py
hooks/test/data/schema.py
hooks/test/unittest_integrity.py
hooks/test/unittest_syncschema.py
server/test/unittest_hook.py
sobjects/test/unittest_supervising.py
--- 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()