Merge pull request #49 from zopefoundation/issue48

Fix default values for Permission's title and description in Py2

5 files changed, 45 insertions, 12 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index b4393de..310b09d 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,7 +5,13 @@
 4.2.1 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Fix the default values for ``Permission`` fields ``title`` and
+  ``description`` under Python 2. See `issue 48
+  <https://github.com/zopefoundation/zope.security/issues/48>`.
+
+- Change the ``IPermission.id`` from ``Text`` (unicode) to a
+  ``NativeStringLine``. This matches what ZCML creates and what is
+  usually written in source code.
 
 
 4.2.0 (2017-09-20)
diff --git a/src/zope/security/interfaces.py b/src/zope/security/interfaces.py
index 4c62bdc..1d646ee 100644
--- a/src/zope/security/interfaces.py
+++ b/src/zope/security/interfaces.py
@@ -52,7 +52,7 @@ means the name of the permission, or the special object
 
 from zope.interface import Interface, Attribute, implementer
 from zope.interface.common.interfaces import IException, IAttributeError
-from zope.schema import Text, TextLine
+from zope.schema import Text, TextLine, NativeStringLine
 from zope.security.i18n import ZopeMessageFactory as _
 
 #: The name (id) of the registered :class:`IPermission` utility that signifies
@@ -438,9 +438,14 @@ class IMemberAwareGroup(IMemberGetterGroup):
         """
 
 class IPermission(Interface):
-    """A permission object."""
+    """A permission object.
 
-    id = TextLine(
+    Note that the ZCML ``<permission>`` directive restricts the ``id`` to
+    be an identifier (a dotted name or a URI), but this interface allows
+    any native string.
+    """
+
+    id = NativeStringLine(
         title=_("Id"),
         description=_("Id as which this permission will be known and used."),
         readonly=True,
diff --git a/src/zope/security/permission.py b/src/zope/security/permission.py
index b54d285..88ba978 100644
--- a/src/zope/security/permission.py
+++ b/src/zope/security/permission.py
@@ -35,7 +35,7 @@ class Permission(object):
     Default implementation of :class:`zope.security.interfaces.IPermission`.
     """
 
-    def __init__(self, id, title="", description=""):
+    def __init__(self, id, title=u"", description=u""):
         self.id = id
         self.title = title
         self.description = description
diff --git a/src/zope/security/tests/test_permission.py b/src/zope/security/tests/test_permission.py
index 57e1d19..82ddea2 100644
--- a/src/zope/security/tests/test_permission.py
+++ b/src/zope/security/tests/test_permission.py
@@ -35,19 +35,23 @@ class PermissionTests(unittest.TestCase):
     def test_instance_conforms_to_IPermission(self):
         from zope.interface.verify import verifyObject
         from zope.security.interfaces import IPermission
+        from zope.schema import getValidationErrors
         verifyObject(IPermission, self._makeOne('testing'))
+        self.assertEqual([],
+                         getValidationErrors(IPermission,
+                                             self._makeOne('testing')))
 
     def test_ctor_only_id(self):
         permission = self._makeOne('testing')
-        self.assertEqual(permission.id, 'testing')
-        self.assertEqual(permission.title, '')
-        self.assertEqual(permission.description, '')
+        self.assertEqual(permission.id, u'testing')
+        self.assertEqual(permission.title, u'')
+        self.assertEqual(permission.description, u'')
 
     def test_ctor_w_title_and_description(self):
-        permission = self._makeOne('testing', 'TITLE', 'DESCRIPTION')
+        permission = self._makeOne('testing', u'TITLE', u'DESCRIPTION')
         self.assertEqual(permission.id, 'testing')
-        self.assertEqual(permission.title, 'TITLE')
-        self.assertEqual(permission.description, 'DESCRIPTION')
+        self.assertEqual(permission.title, u'TITLE')
+        self.assertEqual(permission.description, u'DESCRIPTION')
 
 
 class Test_checkPermission(PlacelessSetup, unittest.TestCase):
@@ -132,6 +136,24 @@ class Test_PermissionsVocabulary(PlacelessSetup, unittest.TestCase):
         self.assertEqual(sorted([x.token for x in vocabulary]),
                          ['testing', zope_Public])
 
+    def test_zcml_valid(self):
+        from zope.configuration import xmlconfig
+        import zope.security
+        from zope.interface.verify import verifyObject
+        from zope.security.interfaces import IPermission
+        from zope.schema import getValidationErrors
+
+
+        xmlconfig.file('configure.zcml', zope.security)
+        vocabulary = self._callFUT()
+        vocabulary = sorted(vocabulary, key=lambda term: term.token)
+        self.assertEqual(6, len(vocabulary))
+
+        for term in vocabulary:
+            p = term.value
+            __traceback_info__ = term.token, p
+            verifyObject(IPermission, p)
+            self.assertEqual([], getValidationErrors(IPermission, p))
 
 class Test_PermissionIdsVocabulary(PlacelessSetup, unittest.TestCase):
 
diff --git a/src/zope/security/zcml.py b/src/zope/security/zcml.py
index 78ef5cc..52bcf91 100644
--- a/src/zope/security/zcml.py
+++ b/src/zope/security/zcml.py
@@ -89,7 +89,7 @@ class IPermissionDirective(Interface):
         description=u"Provides a description for the object.",
         required=False)
 
-def permission(_context, id, title, description=''):
+def permission(_context, id, title, description=u''):
     from zope.security.interfaces import IPermission
     from zope.security.permission import Permission
     from zope.component.zcml import utility