Merge pull request #29 from zopefoundation/issue6

Make GlobalObject only allow dotted names. 

5 files changed, 84 insertions, 69 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 66f0250..3cd9280 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -27,6 +27,20 @@ Changes
   doctests on both Python 2 and Python 3. See `issue 21
   <https://github.com/zopefoundation/zope.configuration/issues/21>`_.
 
+- Fix ``GlobalObject`` and ``GlobalInterface`` fields to only accept
+  dotted names instead of names with ``/``. Previously, slash
+  delimited names could result in incorrect imports. See `issue 6
+  <https://github.com/zopefoundation/zope.configuration/issues/6>`_.
+
+- Fix the schema fields to include the ``value`` and ``field`` values
+  on exceptions they raise.
+
+- Make ``zope.configuration.fields.PythonIdentifier`` subclass
+  ``PythonIdentifier`` from ``zope.schema``. It now implements ``fromBytes``
+  always produces a native string, and validates the value in
+  ``fromUnicode``. See `issue 28
+  <https://github.com/zopefoundation/zope.configuration/issues/28>`_.
+
 4.1.0 (2017-04-26)
 ------------------
 
diff --git a/docs/api/fields.rst b/docs/api/fields.rst
index dd542dc..928a632 100644
--- a/docs/api/fields.rst
+++ b/docs/api/fields.rst
@@ -33,17 +33,17 @@
    .. doctest::
 
       >>> for value in (u'foo', u'foo3', u'foo_', u'_foo3', u'foo_3', u'foo3_'):
-      ...     field._validate(value)
+      ...     _ = field.fromUnicode(value)
       >>> from zope.schema import ValidationError
       >>> for value in (u'3foo', u'foo:', u'\\', u''):
       ...     try:
-      ...         field._validate(value)
+      ...         field.fromUnicode(value)
       ...     except ValidationError:
-      ...         print('Validation Error')
-      Validation Error
-      Validation Error
-      Validation Error
-      Validation Error
+      ...         print('Validation Error ' + repr(value))
+      Validation Error '3foo'
+      Validation Error 'foo:'
+      Validation Error '\\'
+      Validation Error ''
 
 .. autoclass:: GlobalObject
    :members:
diff --git a/setup.py b/setup.py
index fc9a6b9..00c5a0f 100644
--- a/setup.py
+++ b/setup.py
@@ -27,8 +27,9 @@ def read(*rnames):
 TESTS_REQUIRE = [
     'manuel',
     # We test the specific exceptions raised, which
-    # chang from version to version.
-    'zope.schema >= 4.8.0',
+    # change from version to version, and we need the behaviour
+    # in DottedName that allows underscores.
+    'zope.schema >= 4.9.0',
     'zope.testing',
     'zope.testrunner',
 ]
diff --git a/src/zope/configuration/fields.py b/src/zope/configuration/fields.py
index bb27c07..b27a236 100644
--- a/src/zope/configuration/fields.py
+++ b/src/zope/configuration/fields.py
@@ -14,45 +14,47 @@
 """Configuration-specific schema fields
 """
 import os
-import re
 import sys
 import warnings
 
 from zope.interface import implementer
 from zope.schema import Bool as schema_Bool
+from zope.schema import DottedName
 from zope.schema import Field
 from zope.schema import InterfaceField
 from zope.schema import List
+from zope.schema import PythonIdentifier as schema_PythonIdentifier
 from zope.schema import Text
-from zope.schema import TextLine
 from zope.schema import ValidationError
 from zope.schema.interfaces import IFromUnicode
+from zope.schema.interfaces import InvalidValue
 
 from zope.configuration.exceptions import ConfigurationError
 from zope.configuration.interfaces import InvalidToken
 
 
-PYIDENTIFIER_REGEX = u'\\A[a-zA-Z_]+[a-zA-Z0-9_]*\\Z'
-pyidentifierPattern = re.compile(PYIDENTIFIER_REGEX)
-
-
-@implementer(IFromUnicode)
-class PythonIdentifier(TextLine):
-    """This field describes a python identifier, i.e. a variable name.
+class PythonIdentifier(schema_PythonIdentifier):
+    """
+    This class is like `zope.schema.PythonIdentifier`, but does not allow empty strings.
     """
-    def fromUnicode(self, u):
-        return u.strip()
 
     def _validate(self, value):
         super(PythonIdentifier, self)._validate(value)
-        if pyidentifierPattern.match(value) is None:
-            raise ValidationError(value)
+        if not value:
+            raise ValidationError(value).with_field_and_value(self, value)
 
 
 @implementer(IFromUnicode)
 class GlobalObject(Field):
-    """An object that can be accessed as a module global.
     """
+    An object that can be accessed as a module global.
+
+    The special value ``*`` indicates a value of `None`; this is
+    not validated against the *value_type*.
+    """
+
+    _DOT_VALIDATOR = DottedName()
+
     def __init__(self, value_type=None, **kw):
         self.value_type = value_type
         super(GlobalObject, self).__init__(**kw)
@@ -62,17 +64,26 @@ class GlobalObject(Field):
         if self.value_type is not None:
             self.value_type.validate(value)
 
-    def fromUnicode(self, u):
-        name = str(u.strip())
+    def fromUnicode(self, value):
+        name = str(value.strip())
 
         # special case, mostly for interfaces
         if name == '*':
             return None
 
         try:
+            # Leading dots are allowed here to indicate current
+            # package.
+            to_validate = name[1:] if name.startswith('.') else name
+            self._DOT_VALIDATOR.validate(to_validate)
+        except ValidationError as v:
+            v.with_field_and_value(self, name)
+            raise
+
+        try:
             value = self.context.resolve(name)
         except ConfigurationError as v:
-            raise ValidationError(v)
+            raise ValidationError(v).with_field_and_value(self, name)
 
         self.validate(value)
         return value
@@ -99,7 +110,7 @@ class Tokens(List):
                 try:
                     v = vt.fromUnicode(s)
                 except ValidationError as v:
-                    raise InvalidToken("%s in %s" % (v, u))
+                    raise InvalidToken("%s in %s" % (v, u)).with_field_and_value(self, s)
                 else:
                     values.append(v)
         else:
@@ -131,13 +142,15 @@ class Bool(schema_Bool):
     Values may be input (in upper or lower case) as any of:
        yes, no, y, n, true, false, t, or f.
     """
-    def fromUnicode(self, u):
-        u = u.lower()
-        if u in ('1', 'true', 'yes', 't', 'y'):
+    def fromUnicode(self, value):
+        value = value.lower()
+        if value in ('1', 'true', 'yes', 't', 'y'):
             return True
-        if u in ('0', 'false', 'no', 'f', 'n'):
+        if value in ('0', 'false', 'no', 'f', 'n'):
             return False
-        raise ValidationError
+        # Unlike the superclass, anything else is invalid.
+        raise InvalidValue().with_field_and_value(self, value)
+
 
 
 @implementer(IFromUnicode)
diff --git a/src/zope/configuration/tests/test_fields.py b/src/zope/configuration/tests/test_fields.py
index 7eeaeec..734572e 100644
--- a/src/zope/configuration/tests/test_fields.py
+++ b/src/zope/configuration/tests/test_fields.py
@@ -36,39 +36,6 @@ class _ConformsToIFromUnicode(object):
         verifyObject(IFromUnicode, self._makeOne())
 
 
-class PythonIdentifierTests(unittest.TestCase, _ConformsToIFromUnicode):
-
-    def _getTargetClass(self):
-        from zope.configuration.fields import PythonIdentifier
-        return PythonIdentifier
-
-    def _makeOne(self, *args, **kw):
-        return self._getTargetClass()(*args, **kw)
-
-    def test_fromUnicode_empty(self):
-        pi = self._makeOne()
-        self.assertEqual(pi.fromUnicode(''), '')
-
-    def test_fromUnicode_normal(self):
-        pi = self._makeOne()
-        self.assertEqual(pi.fromUnicode('normal'), 'normal')
-
-    def test_fromUnicode_strips_ws(self):
-        pi = self._makeOne()
-        self.assertEqual(pi.fromUnicode('   '), '')
-        self.assertEqual(pi.fromUnicode(' normal  '), 'normal')
-
-    def test__validate_miss(self):
-        from zope.schema import ValidationError
-        pi = self._makeOne()
-        with self.assertRaises(ValidationError):
-            pi._validate(u'not-an-identifier')
-
-    def test__validate_hit(self):
-        pi = self._makeOne()
-        pi._validate(u'is_an_identifier')
-
-
 class GlobalObjectTests(unittest.TestCase, _ConformsToIFromUnicode):
 
     def _getTargetClass(self):
@@ -107,9 +74,12 @@ class GlobalObjectTests(unittest.TestCase, _ConformsToIFromUnicode):
         go = self._makeOne()
         context = Context()
         bound = go.bind(context)
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(ValidationError) as exc:
             bound.fromUnicode('tried')
         self.assertEqual(context._resolved, 'tried')
+        ex = exc.exception
+        self.assertIs(ex.field, bound)
+        self.assertEqual(ex.value, 'tried')
 
     def test_fromUnicode_w_resolve_success(self):
         _target = object()
@@ -141,6 +111,16 @@ class GlobalObjectTests(unittest.TestCase, _ConformsToIFromUnicode):
             bound.fromUnicode('tried')
         self.assertEqual(context._resolved, 'tried')
 
+    def test_fromUnicode_rejects_slash(self):
+        from zope.schema import ValidationError
+        _target = object()
+        field = self._makeOne()
+        with self.assertRaises(ValidationError) as exc:
+            field.fromUnicode('foo/bar')
+        ex = exc.exception
+        self.assertIs(ex.field, field)
+        self.assertEqual(ex.value, 'foo/bar')
+
 
 class GlobalInterfaceTests(unittest.TestCase, _ConformsToIFromUnicode):
 
@@ -179,9 +159,13 @@ class TokensTests(unittest.TestCase, _ConformsToIFromUnicode):
         from zope.schema import Int
         from zope.configuration.interfaces import InvalidToken
         tok = self._makeOne(value_type=Int(min=0))
-        with self.assertRaises(InvalidToken):
+        with self.assertRaises(InvalidToken) as exc:
             tok.fromUnicode(u' 1 -1 3 ')
 
+        ex = exc.exception
+        self.assertIs(ex.field, tok)
+        self.assertEqual(ex.value, '-1')
+
 
 class PathTests(unittest.TestCase, _ConformsToIFromUnicode):
 
@@ -234,10 +218,13 @@ class BoolTests(unittest.TestCase, _ConformsToIFromUnicode):
             self.assertEqual(bo.fromUnicode(value), False)
 
     def test_fromUnicode_w_invalid(self):
-        from zope.schema import ValidationError
+        from zope.schema.interfaces import InvalidValue
         bo = self._makeOne()
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(InvalidValue) as exc:
             bo.fromUnicode('notvalid')
+        ex = exc.exception
+        self.assertIs(ex.field, bo)
+        self.assertEqual(ex.value, 'notvalid')
 
 
 class MessageIDTests(unittest.TestCase, _ConformsToIFromUnicode):