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

And test this. This applies to both the class and the ZCML directive.

Also adjust IPermission to use NativeStringLine for ``id``, since
that's what ZCML has always been creating (it uses an Id, which
subclasses NativeStringLine), and what's typically written in source
code. Test that the permissions created from ZCML are valid with this
change.

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