[repository] refactor/cleanup entity deletion methods stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Tue, 14 Jun 2011 15:37:09 +0200
branchstable
changeset 7501 2983dd24494a
parent 7500 cb0f4da64e86
child 7502 e7190f7e850e
[repository] refactor/cleanup entity deletion methods * kill unused Repository.delete_info_multi (no leading _) * kill unnecessary system_source.delete_info (almost no benefit over delete_info_multi) * stop giving entities list *and* extid list, gather them from the entity when needed * move pending eids handling into glob_delete_entities, so one can call it directly without worrying about that
server/migractions.py
server/repository.py
server/sources/__init__.py
server/sources/native.py
server/ssplanner.py
--- a/server/migractions.py	Tue Jun 14 13:46:36 2011 +0200
+++ b/server/migractions.py	Tue Jun 14 15:37:09 2011 +0200
@@ -944,23 +944,19 @@
             # triggered by schema synchronization hooks.
             session = self.session
             for rdeftype in ('CWRelation', 'CWAttribute'):
-                thispending = set()
-                for eid, in self.sqlexec('SELECT cw_eid FROM cw_%s '
-                                         'WHERE cw_from_entity=%%(eid)s OR '
-                                         ' cw_to_entity=%%(eid)s' % rdeftype,
-                                         {'eid': oldeid}, ask_confirm=False):
-                    # we should add deleted eids into pending eids else we may
-                    # get some validation error on commit since integrity hooks
-                    # may think some required relation is missing... This also ensure
-                    # repository caches are properly cleanup
-                    hook.CleanupDeletedEidsCacheOp.get_instance(session).add_data(eid)
-                    # and don't forget to remove record from system tables
-                    self.repo.system_source.delete_info(
-                        session, session.entity_from_eid(eid, rdeftype),
-                        'system', None)
-                    thispending.add(eid)
-                self.sqlexec('DELETE FROM cw_%s '
-                             'WHERE cw_from_entity=%%(eid)s OR '
+                thispending = set( (eid for eid, in self.sqlexec(
+                    'SELECT cw_eid FROM cw_%s WHERE cw_from_entity=%%(eid)s OR '
+                    ' cw_to_entity=%%(eid)s' % rdeftype,
+                    {'eid': oldeid}, ask_confirm=False)) )
+                # we should add deleted eids into pending eids else we may
+                # get some validation error on commit since integrity hooks
+                # may think some required relation is missing... This also ensure
+                # repository caches are properly cleanup
+                hook.CleanupDeletedEidsCacheOp.get_instance(session).union(thispending)
+                # and don't forget to remove record from system tables
+                entities = [session.entity_from_eid(eid, rdeftype) for eid in thispending]
+                self.repo.system_source.delete_info_multi(session, entities, 'system')
+                self.sqlexec('DELETE FROM cw_%s WHERE cw_from_entity=%%(eid)s OR '
                              'cw_to_entity=%%(eid)s' % rdeftype,
                              {'eid': oldeid}, ask_confirm=False)
                 # now we have to manually cleanup relations pointing to deleted
--- a/server/repository.py	Tue Jun 14 13:46:36 2011 +0200
+++ b/server/repository.py	Tue Jun 14 15:37:09 2011 +0200
@@ -1097,17 +1097,6 @@
         hook.CleanupDeletedEidsCacheOp.get_instance(session).add_data(entity.eid)
         self._delete_info(session, entity, sourceuri, extid, scleanup)
 
-    def delete_info_multi(self, session, entities, sourceuri, extids, scleanup=None):
-        """same as delete_info but accepts a list of entities and
-        extids with the same etype and belonging to the same source
-        """
-        # mark eid as being deleted in session info and setup cache update
-        # operation
-        op = hook.CleanupDeletedEidsCacheOp.get_instance(session)
-        for entity in entities:
-            op.add_data(entity.eid)
-        self._delete_info_multi(session, entities, sourceuri, extids, scleanup)
-
     def _delete_info(self, session, entity, sourceuri, extid, scleanup=None):
         """delete system information on deletion of an entity:
         * delete all remaining relations from/to this entity
@@ -1139,16 +1128,15 @@
                 except:
                     self.exception('error while cascading delete for entity %s '
                                    'from %s. RQL: %s', entity, sourceuri, rql)
-        self.system_source.delete_info(session, entity, sourceuri, extid)
+        self.system_source.delete_info_multi(session, [entity], sourceuri)
 
-    def _delete_info_multi(self, session, entities, sourceuri, extids, scleanup=None):
+    def _delete_info_multi(self, session, entities, sourceuri, scleanup=None):
         """same as _delete_info but accepts a list of entities with
         the same etype and belinging to the same source.
         """
         pendingrtypes = session.transaction_data.get('pendingrtypes', ())
         # delete remaining relations: if user can delete the entity, he can
         # delete all its relations without security checking
-        assert entities and len(entities) == len(extids)
         with security_enabled(session, read=False, write=False):
             eids = [_e.eid for _e in entities]
             in_eids = ','.join((str(eid) for eid in eids))
@@ -1170,7 +1158,7 @@
                 except:
                     self.exception('error while cascading delete for entity %s '
                                    'from %s. RQL: %s', entities, sourceuri, rql)
-        self.system_source.delete_info_multi(session, entities, sourceuri, extids)
+        self.system_source.delete_info_multi(session, entities, sourceuri)
 
     def locate_relation_source(self, session, subject, rtype, object):
         subjsource = self.source_from_eid(subject, session)
@@ -1345,6 +1333,12 @@
 
     def glob_delete_entities(self, session, eids):
         """delete a list of  entities and all related entities from the repository"""
+        # mark eids as being deleted in session info and setup cache update
+        # operation (register pending eids before actual deletion to avoid
+        # multiple call to glob_delete_entities)
+        op = hook.CleanupDeletedEidsCacheOp.get_instance(session)
+        eids = eids - op._container
+        op._container |= eids
         data_by_etype_source = {} # values are ([list of eids],
                                   #             [list of extid],
                                   #             [list of entities])
@@ -1356,27 +1350,23 @@
 
         for eid in eids:
             etype, sourceuri, extid = self.type_and_source_from_eid(eid, session)
+            # XXX should cache entity's cw_metainformation
             entity = session.entity_from_eid(eid, etype)
-            _key = (etype, sourceuri)
-            if _key not in data_by_etype_source:
-                data_by_etype_source[_key] = ([eid], [extid], [entity])
-            else:
-                _data = data_by_etype_source[_key]
-                _data[0].append(eid)
-                _data[1].append(extid)
-                _data[2].append(entity)
-        for (etype, sourceuri), (eids, extids, entities) in data_by_etype_source.iteritems():
+            try:
+                data_by_etype_source[(etype, sourceuri)].append(entity)
+            except KeyError:
+                data_by_etype_source[(etype, sourceuri)] = [entity]
+        for (etype, sourceuri), entities in data_by_etype_source.iteritems():
             if server.DEBUG & server.DBG_REPO:
-                print 'DELETE entities', etype, eids
-            #print 'DELETE entities', etype, len(eids)
+                print 'DELETE entities', etype, [entity.eid for entity in entities]
             source = self.sources_by_uri[sourceuri]
             if source.should_call_hooks:
                 self.hm.call_hooks('before_delete_entity', session, entities=entities)
-            self._delete_info_multi(session, entities, sourceuri, extids) # xxx
+            self._delete_info_multi(session, entities, sourceuri)
             source.delete_entities(session, entities)
             if source.should_call_hooks:
                 self.hm.call_hooks('after_delete_entity', session, entities=entities)
-        # don't clear cache here this is done in a hook on commit
+        # don't clear cache here, it is done in a hook on commit
 
     def glob_add_relation(self, session, subject, rtype, object):
         """add a relation to the repository"""
--- a/server/sources/__init__.py	Tue Jun 14 13:46:36 2011 +0200
+++ b/server/sources/__init__.py	Tue Jun 14 15:37:09 2011 +0200
@@ -462,19 +462,12 @@
         """mark entity as being modified, fulltext reindex if needed"""
         raise NotImplementedError()
 
-    def delete_info(self, session, entity, uri, extid):
-        """delete system information on deletion of an entity by transfering
-        record from the entities table to the deleted_entities table
+    def delete_info_multi(self, session, entities, uri):
+        """delete system information on deletion of a list of entities with the
+        same etype and belinging to the same source
         """
         raise NotImplementedError()
 
-    def delete_info_multi(self, session, entities, uri, extids):
-        """ame as delete_info but accepts a list of entities with
-        the same etype and belinging to the same source.
-        """
-        for entity, extid in itertools.izip(entities, extids):
-            self.delete_info(session, entity, uri, extid)
-
     def modified_entities(self, session, etypes, mtime):
         """return a 2-uple:
         * list of (etype, eid) of entities of the given types which have been
--- a/server/sources/native.py	Tue Jun 14 13:46:36 2011 +0200
+++ b/server/sources/native.py	Tue Jun 14 15:37:09 2011 +0200
@@ -970,31 +970,13 @@
         attrs = {'eid': entity.eid, 'mtime': datetime.now()}
         self.doexec(session, self.sqlgen.update('entities', attrs, ['eid']), attrs)
 
-    def delete_info(self, session, entity, uri, extid):
-        """delete system information on deletion of an entity:
+    def delete_info_multi(self, session, entities, uri):
+        """delete system information on deletion of a list of entities with the
+        same etype and belinging to the same source
+
         * update the fti
-        * remove record from the entities table
-        * transfer it to the deleted_entities table if the entity's type is
-          multi-sources
-        """
-        self.fti_unindex_entities(session, [entity])
-        attrs = {'eid': entity.eid}
-        self.doexec(session, self.sqlgen.delete('entities', attrs), attrs)
-        if not entity.__regid__ in self.multisources_etypes:
-            return
-        if extid is not None:
-            assert isinstance(extid, str), type(extid)
-            extid = b64encode(extid)
-        attrs = {'type': entity.__regid__, 'eid': entity.eid, 'extid': extid,
-                 'source': uri, 'dtime': datetime.now()}
-        self.doexec(session, self.sqlgen.insert('deleted_entities', attrs), attrs)
-
-    def delete_info_multi(self, session, entities, uri, extids):
-        """delete system information on deletion of an entity:
-        * update the fti
-        * remove record from the entities table
-        * transfer it to the deleted_entities table if the entity's type is
-          multi-sources
+        * remove record from the `entities` table
+        * transfer it to the `deleted_entities`
         """
         self.fti_unindex_entities(session, entities)
         attrs = {'eid': '(%s)' % ','.join([str(_e.eid) for _e in entities])}
@@ -1003,7 +985,8 @@
             return
         attrs = {'type': entities[0].__regid__,
                  'source': uri, 'dtime': datetime.now()}
-        for entity, extid in itertools.izip(entities, extids):
+        for entity in entities:
+            extid = entity.cw_metainformation()['extid']
             if extid is not None:
                 assert isinstance(extid, str), type(extid)
                 extid = b64encode(extid)
--- a/server/ssplanner.py	Tue Jun 14 13:46:36 2011 +0200
+++ b/server/ssplanner.py	Tue Jun 14 15:37:09 2011 +0200
@@ -28,7 +28,6 @@
 from cubicweb.schema import VIRTUAL_RTYPES
 from cubicweb.rqlrewrite import add_types_restriction
 from cubicweb.server.session import security_enabled
-from cubicweb.server.hook import CleanupDeletedEidsCacheOp
 from cubicweb.server.edition import EditedEntity
 
 READ_ONLY_RTYPES = set(('eid', 'has_text', 'is', 'is_instance_of', 'identity'))
@@ -521,13 +520,7 @@
         if results:
             todelete = frozenset(typed_eid(eid) for eid, in results)
             session = self.plan.session
-            # mark eids as being deleted in session info and setup cache update
-            # operation (register pending eids before actual deletion to avoid
-            # multiple call to glob_delete_entities)
-            op = CleanupDeletedEidsCacheOp.get_instance(session)
-            actual = todelete - op._container
-            op._container |= actual
-            session.repo.glob_delete_entities(session, actual)
+            session.repo.glob_delete_entities(session, todelete)
         return results
 
 class DeleteRelationsStep(Step):