[Checkins] SVN: Products.GenericSetup/trunk/Products/GenericSetup/ add a test demonstrating that importing a tool with a class not present in the environment raises a cryptic error. fix by showing a more helpful warning and not trying to check if an existing tool matches the desired class if that class couldn't be imported. (see http://dev.plone.org/plone/ticket/9962 for an example of the confusion this was causing)

David Glick davidglick at onenw.org
Wed Mar 3 01:12:59 EST 2010


Log message for revision 109586:
  add a test demonstrating that importing a tool with a class not present in the environment raises a cryptic error. fix by showing a more helpful warning and not trying to check if an existing tool matches the desired class if that class couldn't be imported. (see http://dev.plone.org/plone/ticket/9962 for an example of the confusion this was causing)

Changed:
  U   Products.GenericSetup/trunk/Products/GenericSetup/CHANGES.txt
  U   Products.GenericSetup/trunk/Products/GenericSetup/tests/test_tool.py
  U   Products.GenericSetup/trunk/Products/GenericSetup/tool.py

-=-
Modified: Products.GenericSetup/trunk/Products/GenericSetup/CHANGES.txt
===================================================================
--- Products.GenericSetup/trunk/Products/GenericSetup/CHANGES.txt	2010-03-03 03:56:10 UTC (rev 109585)
+++ Products.GenericSetup/trunk/Products/GenericSetup/CHANGES.txt	2010-03-03 06:12:58 UTC (rev 109586)
@@ -4,6 +4,9 @@
 1.6.0 (unreleased)
 ------------------
 
+- Don't try to reinitialize a tool if an instance of the tool exists but the
+  desired tool class was not resolvable. Show a warning instead of failing.
+
 - Removed backwards compatibility code for no longer supported Zope versions.
 
 

Modified: Products.GenericSetup/trunk/Products/GenericSetup/tests/test_tool.py
===================================================================
--- Products.GenericSetup/trunk/Products/GenericSetup/tests/test_tool.py	2010-03-03 03:56:10 UTC (rev 109585)
+++ Products.GenericSetup/trunk/Products/GenericSetup/tests/test_tool.py	2010-03-03 06:12:58 UTC (rev 109586)
@@ -1400,7 +1400,27 @@
         self.failUnless( isinstance( aq_base( site._getOb( 'obligatory' ) )
                                    , DummyTool ) )
 
+    def test_required_tools_missing_class_with_replacement( self ):
+        
+        from Products.GenericSetup.tool import TOOLSET_XML
+        from Products.GenericSetup.tool import importToolset
 
+        site = self._initSite()
+        
+        obligatory = AnotherDummyTool()
+        obligatory._setId( 'obligatory' )
+        site._setObject( 'obligatory', obligatory )
+        
+        self.assertEqual( len( site.objectIds() ), 2 )
+
+        context = DummyImportContext( site, tool=site.setup_tool )
+        context._files[ TOOLSET_XML ] = _BAD_CLASS_TOOLSET_XML
+
+        importToolset( context )
+
+        self.assertEqual( len( site.objectIds() ), 2 )
+
+
 class DummyTool( Folder ):
 
     pass
@@ -1464,6 +1484,15 @@
 </tool-setup>
 """
 
+_BAD_CLASS_TOOLSET_XML = """\
+<?xml version="1.0"?>
+<tool-setup>
+ <required
+    tool_id="obligatory"
+    class="foobar" />
+</tool-setup>
+""" 
+
 def test_suite():
     # reimport to make sure tests are run from Products
     from Products.GenericSetup.tests.test_tool import SetupToolTests

Modified: Products.GenericSetup/trunk/Products/GenericSetup/tool.py
===================================================================
--- Products.GenericSetup/trunk/Products/GenericSetup/tool.py	2010-03-03 03:56:10 UTC (rev 109585)
+++ Products.GenericSetup/trunk/Products/GenericSetup/tool.py	2010-03-03 06:12:58 UTC (rev 109586)
@@ -113,6 +113,8 @@
 
         tool_id = str(info['id'])
         tool_class = _resolveDottedName(info['class'])
+        if tool_class is None:
+            logger.info('Class %(class)s not found for tool %(id)s' % info)
 
         existing = getattr(aq_base(site), tool_id, None)
         # Don't even initialize the tool again, if it already exists.
@@ -132,6 +134,8 @@
 
             site._setObject(tool_id, new_tool)
         else:
+            if tool_class is None:
+                continue
             unwrapped = aq_base(existing)
             if not isinstance(unwrapped, tool_class):
                 site._delObject(tool_id)



More information about the checkins mailing list