[Zope-CMF] GenericSetup can loose the base profile

Wichert Akkerman wichert at wiggy.net
Mon Nov 19 11:00:56 EST 2007


We are seeing a very interesting problem with the current profile
handling in GenericSetup. This is the scenario:

- we load a profile using runAllImportStepsFromProfile
- one of the import steps installs a product using CMFQuickInstaller
- this product loads another GenericSetup profile, but does a
  getImportContextID() and setImportContext() to make sure GS context
  is not destroyed during the process.

what happens is that runAllImportStepsFromProfile does not set
_import_context_id on the setup tool, so we get the default value: an
empty string. setImportContext() checks if the context id starts with
profile- and if not assumes we are setting the base profile. Result:
the base profile has disappeared.

What surprised me was that apparently "" *is* a valid profile id:
_getImportContext assumes that anything that does not start with
profile- is a snapshot profile. But it will also destroy the context
id in that scenario:

    # else snapshot
    context_id = context_id[len('snapshot-'):]

I modified that code to explicitly check for
context_id.startswith("snapshot-") (see patch below) but that broke a
number of tests which were using an context id of "" to get an empty
snapshot context.  As far as I can see nobody does that outside of the
tests so there should no risk of unexpected breakage anywhere.

Does anyone have a good reason not to make this change?

Wichert.



Index: tests/test_tool.py
===================================================================
--- tests/test_tool.py	(revision 81908)
+++ tests/test_tool.py	(working copy)
@@ -141,6 +141,15 @@
                          , 'profile-foo'
                          )
 
+    def test_setBaselineContext_empty_string( self ):
+
+        tool = self._makeOne('setup_tool')
+
+        self.assertRaises( KeyError
+                         , tool.setBaselineContext
+                         , ''
+                         )
+
     def test_setBaselineContext( self ):
 
         from Products.GenericSetup.tool import IMPORT_STEPS_XML
Index: CHANGES.txt
===================================================================
--- CHANGES.txt	(revision 81908)
+++ CHANGES.txt	(working copy)
@@ -2,6 +2,8 @@
 
   GenericSetup 1.4 (unreleased)
   
+    - Be more careful in checking context id validity.
+
     - Fire events before and after importing.
 
     - Use zcml to register import and export steps.
Index: tool.py
===================================================================
--- tool.py	(revision 81908)
+++ tool.py	(working copy)
@@ -954,11 +954,13 @@
                 should_purge = (info.get('type') != EXTENSION)
             return DirectoryImportContext(self, path, should_purge, encoding)
 
-        # else snapshot
-        context_id = context_id[len('snapshot-'):]
-        if should_purge is None:
-            should_purge = True
-        return SnapshotImportContext(self, context_id, should_purge, encoding)
+        elif context_id.startswith('snapshot-'):
+            context_id = context_id[len('snapshot-'):]
+            if should_purge is None:
+                should_purge = True
+            return SnapshotImportContext(self, context_id, should_purge, encoding)
+        else:
+            raise KeyError, 'Unknown context %s' % context_id
 
     security.declarePrivate('_updateImportStepsRegistry')
     def _updateImportStepsRegistry(self, context, encoding):

-- 
Wichert Akkerman <wichert at wiggy.net>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.


More information about the Zope-CMF mailing list