[Checkins] SVN: zope.app.container/branches/3.5/ bugfix backport from zope.container (#227617)

Christophe Combelles ccomb at free.fr
Sat Apr 24 09:29:58 EDT 2010


Log message for revision 111357:
  bugfix backport from zope.container (#227617)
  

Changed:
  U   zope.app.container/branches/3.5/CHANGES.txt
  U   zope.app.container/branches/3.5/src/zope/app/container/contained.py
  U   zope.app.container/branches/3.5/src/zope/app/container/tests/test_contained.py

-=-
Modified: zope.app.container/branches/3.5/CHANGES.txt
===================================================================
--- zope.app.container/branches/3.5/CHANGES.txt	2010-04-24 13:11:01 UTC (rev 111356)
+++ zope.app.container/branches/3.5/CHANGES.txt	2010-04-24 13:29:57 UTC (rev 111357)
@@ -5,8 +5,10 @@
 3.5.7 (unreleased)
 ------------------
 
-- ...
+- never fail if the suggested name is in a wrong type (#227617)
 
+- checkName first checks the parameter type before the emptiness
+
 3.5.6 (2008-08-06)
 ------------------
 

Modified: zope.app.container/branches/3.5/src/zope/app/container/contained.py
===================================================================
--- zope.app.container/branches/3.5/src/zope/app/container/contained.py	2010-04-24 13:11:01 UTC (rev 111356)
+++ zope.app.container/branches/3.5/src/zope/app/container/contained.py	2010-04-24 13:29:57 UTC (rev 111357)
@@ -714,47 +714,44 @@
         >>> container['foo'] = 'bar'
         >>> from zope.app.container.contained import NameChooser
 
-        All these names are invalid:
+        An invalid name raises a UserError:
 
         >>> NameChooser(container).checkName('+foo', object())
         Traceback (most recent call last):
         ...
         UserError: Names cannot begin with '+' or '@' or contain '/'
-        >>> NameChooser(container).checkName('@foo', object())
-        Traceback (most recent call last):
-        ...
-        UserError: Names cannot begin with '+' or '@' or contain '/'
-        >>> NameChooser(container).checkName('f/oo', object())
-        Traceback (most recent call last):
-        ...
-        UserError: Names cannot begin with '+' or '@' or contain '/'
+
+        A name that already exists raises a UserError:
+
         >>> NameChooser(container).checkName('foo', object())
         Traceback (most recent call last):
         ...
         UserError: The given name is already being used
+
+        A name must be a string or unicode string:
+
         >>> NameChooser(container).checkName(2, object())
         Traceback (most recent call last):
         ...
         TypeError: ('Invalid name type', <type 'int'>)
 
-        This one is ok:
+        A correct name returns True:
 
         >>> NameChooser(container).checkName('2', object())
         True
 
-
         """
 
+        if isinstance(name, str):
+            name = unicode(name)
+        elif not isinstance(name, unicode):
+            raise TypeError("Invalid name type", type(name))
+
         if not name:
             raise UserError(
                 _("An empty name was provided. Names cannot be empty.")
                 )
 
-        if isinstance(name, str):
-            name = unicode(name)
-        elif not isinstance(name, unicode):
-            raise TypeError("Invalid name type", type(name))
-
         if name[:1] in '+@' or '/' in name:
             raise UserError(
                 _("Names cannot begin with '+' or '@' or contain '/'")
@@ -772,33 +769,51 @@
         """See zope.app.container.interfaces.INameChooser
 
         The name chooser is expected to choose a name without error
-        
+
         We create and populate a dummy container
 
         >>> from zope.app.container.sample import SampleContainer
         >>> container = SampleContainer()
-        >>> container['foo.old.rst'] = 'rst doc'
+        >>> container['foobar.old'] = 'rst doc'
 
         >>> from zope.app.container.contained import NameChooser
-        >>> NameChooser(container).chooseName('+ at +@foo.old.rst', object())
-        u'foo.old-2.rst'
-        >>> NameChooser(container).chooseName('+ at +@foo/foo', object())
+
+        the suggested name is converted to unicode:
+
+        >>> NameChooser(container).chooseName('foobar', object())
+        u'foobar'
+
+        If it already exists, a number is appended but keeps the same extension:
+
+        >>> NameChooser(container).chooseName('foobar.old', object())
+        u'foobar-2.old'
+
+        Bad characters are turned into dashes:
+
+        >>> NameChooser(container).chooseName('foo/foo', object())
         u'foo-foo'
-        >>> NameChooser(container).chooseName('', object())
-        u'object'
-        >>> NameChooser(container).chooseName('@+@', object())
-        u'object'
 
+        If no name is suggested, it is based on the object type:
+
+        >>> NameChooser(container).chooseName('', [])
+        u'list'
+
         """
 
         container = self.context
 
-        # remove characters that checkName does not allow
-        name = unicode(name.replace('/', '-').lstrip('+@'))
+        # convert to unicode and remove characters that checkName does not allow
+        try:
+            name = unicode(name)
+        except:
+            name = u''
+        name = name.replace('/', '-').lstrip('+@')
 
         if not name:
             name = unicode(object.__class__.__name__)
-        
+
+        # for an existing name, append a number.
+        # We should keep client's os.path.extsep (not ours), we assume it's '.'
         dot = name.rfind('.')
         if dot >= 0:
             suffix = name[dot:]
@@ -806,7 +821,6 @@
         else:
             suffix = ''
 
-
         n = name + suffix
         i = 1
         while n in container:

Modified: zope.app.container/branches/3.5/src/zope/app/container/tests/test_contained.py
===================================================================
--- zope.app.container/branches/3.5/src/zope/app/container/tests/test_contained.py	2010-04-24 13:11:01 UTC (rev 111356)
+++ zope.app.container/branches/3.5/src/zope/app/container/tests/test_contained.py	2010-04-24 13:29:57 UTC (rev 111357)
@@ -23,10 +23,14 @@
 from persistent import Persistent
 
 import zope.interface
+import zope.component
 from zope.testing import doctest
 
-from zope.app.container.contained import ContainedProxy
+from zope.exceptions.interfaces import UserError
+from zope.app.container.contained import ContainedProxy, NameChooser
 from zope.app.testing import placelesssetup
+from zope.app.container.sample import SampleContainer
+from zope.app.container.interfaces import IContainer
 
 class MyOb(Persistent):
     pass
@@ -313,15 +317,87 @@
 
     >>> p.__dict__ is c.__dict__
     True
-    
+
     """
 
+
+class TestNameChooser(unittest.TestCase):
+    def test_checkName(self):
+        container = SampleContainer()
+        container['foo'] = 'bar'
+        checkName = NameChooser(container).checkName
+
+        # invalid type for the name
+        self.assertRaises(TypeError, checkName, 2, object())
+        self.assertRaises(TypeError, checkName, [], object())
+        self.assertRaises(TypeError, checkName, None, object())
+        self.assertRaises(TypeError, checkName, None, None)
+
+        # invalid names
+        self.assertRaises(UserError, checkName, '+foo', object())
+        self.assertRaises(UserError, checkName, '@foo', object())
+        self.assertRaises(UserError, checkName, 'f/oo', object())
+        self.assertRaises(UserError, checkName, '', object())
+
+        # existing names
+        self.assertRaises(UserError, checkName, 'foo', object())
+        self.assertRaises(UserError, checkName, u'foo', object())
+
+        # correct names
+        self.assertEqual(True, checkName('2', object()))
+        self.assertEqual(True, checkName(u'2', object()))
+        self.assertEqual(True, checkName('other', object()))
+        self.assertEqual(True, checkName(u'reserved', object()))
+        self.assertEqual(True, checkName(u'r\xe9served', object()))
+
+
+    def test_chooseName(self):
+        container = SampleContainer()
+        container['foo.old.rst'] = 'rst doc'
+        nc = NameChooser(container)
+
+        # correct name without changes
+        self.assertEqual(nc.chooseName('foobar.rst', None),
+                         u'foobar.rst')
+        self.assertEqual(nc.chooseName(u'\xe9', None),
+                         u'\xe9')
+
+        # automatically modified named
+        self.assertEqual(nc.chooseName('foo.old.rst', None),
+                         u'foo.old-2.rst')
+        self.assertEqual(nc.chooseName('+ at +@foo.old.rst', None),
+                         u'foo.old-2.rst')
+        self.assertEqual(nc.chooseName('+ at +@foo/foo+@', None),
+                         u'foo-foo+@')
+
+        # empty name
+        self.assertEqual(nc.chooseName('', None), u'NoneType')
+        self.assertEqual(nc.chooseName('@+@', []), u'list')
+
+        # if the name is not a string it is converted
+        self.assertEqual(nc.chooseName(None, None), u'None')
+        self.assertEqual(nc.chooseName(2, None), u'2')
+        self.assertEqual(nc.chooseName([], None), u'[]')
+        container['None'] = 'something'
+        self.assertEqual(nc.chooseName(None, None), u'None-2')
+        container['None-2'] = 'something'
+        self.assertEqual(nc.chooseName(None, None), u'None-3')
+
+        # even if the given name cannot be converted to unicode
+        class BadBoy:
+            def __unicode__(self):
+                raise Exception
+
+        self.assertEqual(nc.chooseName(BadBoy(), set()), u'set')
+
+
 def test_suite():
     return unittest.TestSuite((
         doctest.DocTestSuite('zope.app.container.contained',
                              setUp=placelesssetup.setUp,
                              tearDown=placelesssetup.tearDown),
         doctest.DocTestSuite(optionflags=doctest.NORMALIZE_WHITESPACE),
+        unittest.makeSuite(TestNameChooser),
         ))
 
 if __name__ == '__main__': unittest.main()



More information about the checkins mailing list