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 <[EMAIL PROTECTED]> It is simple to make things.
http://www.wiggy.net/ It is hard to make things simple.
_______________________________________________
Zope-CMF maillist - Zope-CMF@lists.zope.org
http://mail.zope.org/mailman/listinfo/zope-cmf
See http://collector.zope.org/CMF for bug reports and feature requests
