Purge data when delete tile

CHANGES.rst

@@ -6,7 +6,9 @@ There's a frood who really knows where his towel is.
 1.6b5 (unreleased)
 ^^^^^^^^^^^^^^^^^^
 
-- Fix bug responsible to make Cover objects bigger, by deleting tile annotations of deleted tiles (fixes `#765 <https://github.com/collective/collective.cover/issues/765>`_).
+- Fix purging of tile annotations when removing tiles from the cover layout.
+  This solves exponential growth of cover objects when using versioning,
+  leading to check in/check out (plone.app.iterate) timeouts on backends using proxy servers (fixes `#765 <https://github.com/collective/collective.cover/issues/765>`_).
   [rodfersou]
 
 - Do not auto include package dependencies, but declare them explicitly.

src/collective/cover/content.py

@@ -151,26 +151,24 @@ def get_referenced_objects(self):
         return refs
 
     def purge_deleted_tiles(self):
-        """Check if there are any annotation of deleted tiles and destroy it.
-
-           Cover tiles are Browser Views, and data are saved in annotations.
-           When delete a tile, the annotations are not deleted, it keeps making
-           Cover object bigger;
-           This method purge all tile annotation data for tiles where there are
-           no reference in layout.
+        """Purge annotations of tiles that are no longer referenced on
+        the layout.
         """
         layout_tiles = self.list_tiles()
         annotations = IAnnotations(self)
 
         for key in annotations:
             if not key.startswith(ANNOTATION_PREFIXES):
                 continue
+
+            # XXX: we need to remove tile annotations at low level as
+            #      there's no information available on the tile type
+            #      (it's no longer in the layout and we can only infer
+            #      its id); this could lead to issues in the future if
+            #      a different storage is used (see plone.tiles code)
             tile_id = key.split('.')[-1]
-            if tile_id in layout_tiles:
-                continue
-            # XXX: We can't use tile API because there is no way to know the tile type
-            # for deleted tiles
-            del annotations[key]
+            if tile_id not in layout_tiles:
+                del annotations[key]
 
 
 @indexer(ICover)

src/collective/cover/tests/test_base_tile.py

@@ -7,8 +7,6 @@
 from collective.cover.tiles.permissions import ITilesPermissions
 from cStringIO import StringIO
 from plone.tiles.interfaces import ITileDataManager
-from zope.annotation import IAnnotations
-from zope.component import eventtesting
 from zope.component import getMultiAdapter
 from zope.configuration.xmlconfig import xmlconfig
 
@@ -49,7 +47,6 @@ def _register_tile(self):
 
     def setUp(self):
         super(BaseTileTestCase, self).setUp()
-        eventtesting.setUp()
         self._register_tile()
         self.tile = PersistentCoverTile(self.cover, self.request)
         self.tile.__name__ = u'collective.cover.base'
@@ -71,9 +68,14 @@ def test_default_configuration(self):
     def test_accepted_content_types(self):
         self.assertEqual(self.tile.accepted_ct(), ALL_CONTENT_TYPES)
 
-    def test_delete_tile_persistent_data(self):
-        eventtesting.clearEvents()
-
+    def test_delete_tile_removes_persistent_data(self):
+        # https://github.com/collective/collective.cover/issues/765
+        from collective.cover.config import CONFIGURATION_PREFIX
+        from collective.cover.config import PERMISSIONS_PREFIX
+        from zope.annotation import IAnnotations
+        from zope.component import eventtesting
+        from zope.lifecycleevent import IObjectModifiedEvent
+        eventtesting.setUp()
         annotations = IAnnotations(self.tile.context)
 
         data_mgr = ITileDataManager(self.tile)
@@ -82,24 +84,25 @@ def test_delete_tile_persistent_data(self):
         self.assertEqual(data_mgr.get()['test'], 'data')
 
         permissions = getMultiAdapter(
-            (self.tile.context, self.request, self.tile), ITilesPermissions)
+            (self.cover, self.request, self.tile), ITilesPermissions)
         permissions.set_allowed_edit('masters_of_the_universe')
-        self.assertIn('plone.tiles.permission.test', annotations)
+        self.assertIn(PERMISSIONS_PREFIX + '.test', annotations)
 
         configuration = getMultiAdapter(
-            (self.tile.context, self.request, self.tile), ITilesConfigurationScreen)
+            (self.cover, self.request, self.tile), ITilesConfigurationScreen)
         configuration.set_configuration({'uuid': 'c1d2e3f4g5jrw'})
-        self.assertIn('plone.tiles.configuration.test', annotations)
+        self.assertIn(CONFIGURATION_PREFIX + '.test', annotations)
 
         # Call the delete method
+        eventtesting.clearEvents()
         self.tile.delete()
 
-        # Now we should not see the stored data anymore
+        # Now we should not see the persistent data anymore
         self.assertNotIn('test', data_mgr.get())
-        self.assertNotIn('plone.tiles.permission.test', annotations)
-        self.assertNotIn('plone.tiles.configuration.test', annotations)
-
-        events = eventtesting.getEvents()
+        self.assertNotIn(PERMISSIONS_PREFIX + '.test', annotations)
+        self.assertNotIn(CONFIGURATION_PREFIX + '.test', annotations)
 
         # Finally, test that ObjectModifiedEvent was fired for the cover
-        self.assertEqual(events[0].object, self.cover)
+        events = eventtesting.getEvents()
+        self.assertEqual(len(events), 1)
+        self.assertTrue(IObjectModifiedEvent.providedBy(events[0]))

src/collective/cover/tests/test_upgrades.py

@@ -670,7 +670,7 @@ def test_purge_deleted_tiles(self):
         from zope.annotation import IAnnotations
         title = u'Purge Deleted Tiles'
         step = self._get_upgrade_step(title)
-        assert step is not None
+        self.assertIsNotNone(step)
 
         # simulate state on previous version
         cover = self._create_cover('test-cover', 'Empty layout')
@@ -681,7 +681,6 @@ def test_purge_deleted_tiles(self):
 
         # run the upgrade step to validate the update
         self._do_upgrade_step(step)
-
         self.assertNotIn(key, annotations)
 
     @unittest.skipIf(IS_PLONE_5, 'Upgrade step not supported under Plone 5')
@@ -692,16 +691,11 @@ def test_register_resource(self):
         self.assertIsNotNone(step)
 
         # simulate state on previous version
-        from collective.cover.upgrades.v19 import TO_REGISTER
-
+        from collective.cover.upgrades.v19 import JS
         js_tool = api.portal.get_tool('portal_javascripts')
-        js_tool.unregisterResource(TO_REGISTER)
-
-        js_ids = js_tool.getResourceIds()
-        self.assertNotIn(TO_REGISTER, js_ids)
+        js_tool.unregisterResource(JS)
+        self.assertNotIn(JS, js_tool.getResourceIds())
 
         # run the upgrade step to validate the update
         self._do_upgrade_step(step)
-
-        js_ids = js_tool.getResourceIds()
-        self.assertIn(TO_REGISTER, js_ids)
+        self.assertIn(JS, js_tool.getResourceIds())

src/collective/cover/upgrades/v19/__init__.py

@@ -4,25 +4,23 @@
 from plone import api
 
 
-TO_REGISTER = '++resource++collective.cover/js/layout_edit.js'
+JS = '++resource++collective.cover/js/layout_edit.js'
 
 
 def purge_deleted_tiles(context):
     """Purge all annotations of deleted tiles."""
+    results = api.content.find(object_provides=ICover.__identifier__)
+    logger.info('About to update {0} objects'.format(len(results)))
 
-    covers = context.portal_catalog(object_provides=ICover.__identifier__)
-    logger.info('About to update {0} objects'.format(len(covers)))
-    for cover in covers:
-        obj = cover.getObject()
+    for b in results:
+        obj = b.getObject()
         obj.purge_deleted_tiles()
-        msg = 'Cover {0} updated'
-        logger.info(msg.format(cover.getPath()))
-
-    logger.info('Done')
+        logger.info('Purged annotations on ' + b.getPath())
 
 
 def register_resource(setup_tool):
     """Add layout_edit.js to registered resources."""
     js_tool = api.portal.get_tool('portal_javascripts')
-    js_tool.registerResource(id=TO_REGISTER, authenticated=True)
-    logger.info('Resource registered.')
+    js_tool.registerScript(id=JS, compression='none', authenticated=True)
+    assert JS in js_tool.getResourceIds()
+    logger.info('Script registered')