Merge "Revert "Generate better validation error message when using name regexes""

nova/api/validation/parameter_types.py

@@ -21,14 +21,6 @@ import unicodedata
 
 import six
 
-from nova.i18n import _
-
-
-class ValidationRegex(object):
-    def __init__(self, regex, reason):
-        self.regex = regex
-        self.reason = reason
-
 
 def _is_printable(char):
     """determine if a unicode code point is printable.
@@ -118,12 +110,10 @@ def _build_regex_range(ws=True, invert=False, exclude=None):
 valid_name_regex_base = '^(?![%s])[%s]*(?<![%s])$'
 
 
-valid_name_regex = ValidationRegex(
+valid_name_regex = valid_name_regex_base % (
-    valid_name_regex_base % (
     _build_regex_range(ws=False, invert=True),
     _build_regex_range(),
-        _build_regex_range(ws=False, invert=True)),
+    _build_regex_range(ws=False, invert=True))
-    _("printable characters. Can not start or end with whitespace."))
 
 
 # This regex allows leading/trailing whitespace
@@ -132,32 +122,26 @@ valid_name_leading_trailing_spaces_regex_base = (
     "^[%(ws)s]*[%(no_ws)s][%(no_ws)s%(ws)s]+[%(no_ws)s][%(ws)s]*$")
 
 
-valid_cell_name_regex = ValidationRegex(
+valid_cell_name_regex = valid_name_regex_base % (
-    valid_name_regex_base % (
     _build_regex_range(ws=False, invert=True),
     _build_regex_range(exclude=['!', '.', '@']),
-        _build_regex_range(ws=False, invert=True)),
+    _build_regex_range(ws=False, invert=True))
-    _("printable characters except !, ., @. "
-      "Can not start or end with whitespace."))
 
 
 # cell's name disallow '!',  '.' and '@'.
-valid_cell_name_leading_trailing_spaces_regex = ValidationRegex(
+valid_cell_name_leading_trailing_spaces_regex = (
     valid_name_leading_trailing_spaces_regex_base % {
         'ws': _build_regex_range(exclude=['!', '.', '@']),
-        'no_ws': _build_regex_range(ws=False, exclude=['!', '.', '@'])},
+        'no_ws': _build_regex_range(ws=False, exclude=['!', '.', '@'])})
-    _("printable characters except !, ., @, "
-      "with at least one non space character"))
 
 
-valid_name_leading_trailing_spaces_regex = ValidationRegex(
+valid_name_leading_trailing_spaces_regex = (
     valid_name_leading_trailing_spaces_regex_base % {
         'ws': _build_regex_range(),
-        'no_ws': _build_regex_range(ws=False)},
+        'no_ws': _build_regex_range(ws=False)})
-    _("printable characters with at least one non space character"))
 
 
-valid_name_regex_obj = re.compile(valid_name_regex.regex, re.UNICODE)
+valid_name_regex_obj = re.compile(valid_name_regex, re.UNICODE)
 
 
 valid_description_regex_base = '^[%s]*$'
@@ -217,25 +201,25 @@ name = {
     # stored in the DB and Nova specific parameters.
     # This definition is used for all their parameters.
     'type': 'string', 'minLength': 1, 'maxLength': 255,
-    'format': 'name'
+    'pattern': valid_name_regex,
 }
 
 
 cell_name = {
     'type': 'string', 'minLength': 1, 'maxLength': 255,
-    'format': 'cell_name'
+    'pattern': valid_cell_name_regex,
 }
 
 
 cell_name_leading_trailing_spaces = {
     'type': 'string', 'minLength': 1, 'maxLength': 255,
-    'format': 'cell_name_with_leading_trailing_spaces'
+    'pattern': valid_cell_name_leading_trailing_spaces_regex,
 }
 
 
 name_with_leading_trailing_spaces = {
     'type': 'string', 'minLength': 1, 'maxLength': 255,
-    'format': 'name_with_leading_trailing_spaces'
+    'pattern': valid_name_leading_trailing_spaces_regex,
 }
 
 

nova/api/validation/validators.py

@@ -27,7 +27,6 @@ from oslo_utils import uuidutils
 import rfc3986
 import six
 
-from nova.api.validation import parameter_types
 from nova import exception
 from nova.i18n import _
 
@@ -76,44 +75,6 @@ def _validate_uri(instance):
                                 require_authority=True)
 
 
-@jsonschema.FormatChecker.cls_checks('name_with_leading_trailing_spaces',
-                                     exception.InvalidName)
-def _validate_name_with_leading_trailing_spaces(instance):
-    regex = parameter_types.valid_name_leading_trailing_spaces_regex
-    if re.search(regex.regex, instance):
-        return True
-    else:
-        raise exception.InvalidName(reason=regex.reason)
-
-
-@jsonschema.FormatChecker.cls_checks('name', exception.InvalidName)
-def _validate_name(instance):
-    regex = parameter_types.valid_name_regex
-    if re.search(regex.regex, instance):
-        return True
-    else:
-        raise exception.InvalidName(reason=regex.reason)
-
-
-@jsonschema.FormatChecker.cls_checks('cell_name_with_leading_trailing_spaces',
-                                     exception.InvalidName)
-def _validate_cell_name_with_leading_trailing_spaces(instance):
-    regex = parameter_types.valid_cell_name_leading_trailing_spaces_regex
-    if re.search(regex.regex, instance):
-        return True
-    else:
-        raise exception.InvalidName(reason=regex.reason)
-
-
-@jsonschema.FormatChecker.cls_checks('cell_name', exception.InvalidName)
-def _validate_cell_name(instance):
-    regex = parameter_types.valid_cell_name_regex
-    if re.search(regex.regex, instance):
-        return True
-    else:
-        raise exception.InvalidName(reason=regex.reason)
-
-
 def _soft_validate_additional_properties(validator,
                                          additional_properties_value,
                                          instance,
@@ -171,44 +132,6 @@ def _soft_validate_additional_properties(validator,
             del instance[prop]
 
 
-class FormatChecker(jsonschema.FormatChecker):
-    """A FormatChecker can output the message from cause exception
-
-       We need understandable validation errors messages for users. When a
-       custom checker has an exception, the FormatChecker will output a
-       readable message provided by the checker.
-    """
-
-    def check(self, instance, format):
-        """Check whether the instance conforms to the given format.
-
-        :argument instance: the instance to check
-        :type: any primitive type (str, number, bool)
-        :argument str format: the format that instance should conform to
-        :raises: :exc:`FormatError` if instance does not conform to format
-        """
-
-        if format not in self.checkers:
-            return
-
-        # For safety reasons custom checkers can be registered with
-        # allowed exception types. Anything else will fall into the
-        # default formatter.
-        func, raises = self.checkers[format]
-        result, cause = None, None
-
-        try:
-            result = func(instance)
-        except raises as e:
-            if isinstance(e, exception.NovaException):
-                raise
-            else:
-                cause = e
-        if not result:
-            msg = "%r is not a %r" % (instance, format)
-            raise jsonschema_exc.FormatError(msg, cause=cause)
-
-
 class _SchemaValidator(object):
     """A validator class
 
@@ -233,20 +156,16 @@ class _SchemaValidator(object):
 
         validator_cls = jsonschema.validators.extend(self.validator_org,
                                                      validators)
-        format_checker = FormatChecker()
+        format_checker = jsonschema.FormatChecker()
         self.validator = validator_cls(schema, format_checker=format_checker)
 
     def validate(self, *args, **kwargs):
         try:
             self.validator.validate(*args, **kwargs)
-        except exception.InvalidName as ex:
-            raise exception.ValidationError(detail=ex.format_message())
         except jsonschema.ValidationError as ex:
-            if isinstance(ex.cause, exception.InvalidName):
-                detail = ex.cause.format_message()
-            elif len(ex.path) > 0:
             # NOTE: For whole OpenStack message consistency, this error
             #       message has been written as the similar format of WSME.
+            if len(ex.path) > 0:
                 detail = _("Invalid input for field/attribute %(path)s."
                            " Value: %(value)s. %(message)s") % {
                                'path': ex.path.pop(), 'value': ex.instance,

nova/exception.py

@@ -420,11 +420,6 @@ class InvalidStrTime(Invalid):
     msg_fmt = _("Invalid datetime string: %(reason)s")
 
 
-class InvalidName(Invalid):
-    msg_fmt = _("An invalid 'name' value was provided. "
-                "The name must be: %(reason)s")
-
-
 class InstanceInvalidState(Invalid):
     msg_fmt = _("Instance %(instance_uuid)s in %(attr)s %(state)s. Cannot "
                 "%(method)s while the instance is in this state.")

nova/tests/unit/test_api_validation.py

@@ -21,7 +21,6 @@ import six
 from nova.api.openstack import api_version_request as api_version
 from nova.api import validation
 from nova.api.validation import parameter_types
-from nova.api.validation import validators
 from nova import exception
 from nova import test
 
@@ -37,7 +36,7 @@ class FakeRequest(object):
 
 class ValidationRegex(test.NoDBTestCase):
     def test_cell_names(self):
-        cellre = re.compile(parameter_types.valid_cell_name_regex.regex)
+        cellre = re.compile(parameter_types.valid_cell_name_regex)
         self.assertTrue(cellre.search('foo'))
         self.assertFalse(cellre.search('foo.bar'))
         self.assertFalse(cellre.search('foo@bar'))
@@ -109,18 +108,6 @@ class APIValidationTestCase(test.NoDBTestCase):
             self.fail('Any exception does not happen.')
 
 
-class FormatCheckerTestCase(test.NoDBTestCase):
-
-    def test_format_checker_failed(self):
-        format_checker = validators.FormatChecker()
-        exc = self.assertRaises(exception.InvalidName,
-                                format_checker.check, "   ", "name")
-        self.assertEqual("An invalid 'name' value was provided. The name must "
-                         "be: printable characters. "
-                         "Can not start or end with whitespace.",
-                         exc.format_message())
-
-
 class MicroversionsSchemaTestCase(APIValidationTestCase):
 
     def setUp(self):
@@ -681,122 +668,6 @@ class HostnameIPaddressTestCase(APIValidationTestCase):
                                     expected_detail=detail)
 
 
-class CellNameTestCase(APIValidationTestCase):
-
-    def setUp(self):
-        super(CellNameTestCase, self).setUp()
-        schema = {
-            'type': 'object',
-            'properties': {
-                'foo': parameter_types.cell_name,
-            },
-        }
-
-        @validation.schema(request_body_schema=schema)
-        def post(req, body):
-            return 'Validation succeeded.'
-
-        self.post = post
-
-    def test_validate_name(self):
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': 'abc'},
-                                   req=FakeRequest()))
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': 'my server'},
-                                   req=FakeRequest()))
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': u'\u0434'}, req=FakeRequest()))
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': u'\u0434\u2006\ufffd'},
-                                   req=FakeRequest()))
-
-    def test_validate_name_fails(self):
-        error = ("An invalid 'name' value was provided. The name must be: "
-                 "printable characters except !, ., @. "
-                 "Can not start or end with whitespace.")
-
-        should_fail = (' ',
-                       ' server',
-                       'server ',
-                       u'a\xa0',  # trailing unicode space
-                       u'\uffff',  # non-printable unicode
-                       'abc!def',
-                       'abc.def',
-                       'abc@def')
-
-        for item in should_fail:
-            self.check_validation_error(self.post, body={'foo': item},
-                                    expected_detail=error)
-
-        # four-byte unicode, if supported by this python build
-        try:
-            self.check_validation_error(self.post, body={'foo': u'\U00010000'},
-                                        expected_detail=error)
-        except ValueError:
-            pass
-
-
-class CellNameLeadingTrailingSpacesTestCase(APIValidationTestCase):
-
-    def setUp(self):
-        super(CellNameLeadingTrailingSpacesTestCase, self).setUp()
-        schema = {
-            'type': 'object',
-            'properties': {
-                'foo': parameter_types.cell_name_leading_trailing_spaces,
-            },
-        }
-
-        @validation.schema(request_body_schema=schema)
-        def post(req, body):
-            return 'Validation succeeded.'
-
-        self.post = post
-
-    def test_validate_name(self):
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': 'abc'},
-                                   req=FakeRequest()))
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': 'my server'},
-                                   req=FakeRequest()))
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': u'\u0434'}, req=FakeRequest()))
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': u'\u0434\u2006\ufffd'},
-                                   req=FakeRequest()))
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': '  my server'},
-                                   req=FakeRequest()))
-        self.assertEqual('Validation succeeded.',
-                         self.post(body={'foo': 'my server  '},
-                                   req=FakeRequest()))
-
-    def test_validate_name_fails(self):
-        error = ("An invalid 'name' value was provided. The name must be: "
-                 "printable characters except !, ., @, "
-                 "with at least one non space character")
-
-        should_fail = (
-            ' ',
-            u'\uffff',  # non-printable unicode
-            'abc!def',
-            'abc.def',
-            'abc@def')
-
-        for item in should_fail:
-            self.check_validation_error(self.post, body={'foo': item},
-                                    expected_detail=error)
-
-        # four-byte unicode, if supported by this python build
-        try:
-            self.check_validation_error(self.post, body={'foo': u'\U00010000'},
-                                        expected_detail=error)
-        except ValueError:
-            pass
-
-
 class NameTestCase(APIValidationTestCase):
 
     def setUp(self):
@@ -830,25 +701,54 @@ class NameTestCase(APIValidationTestCase):
                                    req=FakeRequest()))
 
     def test_validate_name_fails(self):
-        error = ("An invalid 'name' value was provided. The name must be: "
+        detail = (u"Invalid input for field/attribute foo. Value:  ."
-                 "printable characters. "
+                  " ' ' does not match .*")
-                 "Can not start or end with whitespace.")
+        self.check_validation_error(self.post, body={'foo': ' '},
+                                    expected_detail=detail)
 
-        should_fail = (' ',
+        detail = ("Invalid input for field/attribute foo. Value:  server."
-                       ' server',
+                  " ' server' does not match .*")
-                       'server ',
+        self.check_validation_error(self.post, body={'foo': ' server'},
-                       u'a\xa0',  # trailing unicode space
+                                    expected_detail=detail)
-                       u'\uffff',  # non-printable unicode
-                       )
 
-        for item in should_fail:
+        detail = ("Invalid input for field/attribute foo. Value: server ."
-            self.check_validation_error(self.post, body={'foo': item},
+                  " 'server ' does not match .*")
-                                    expected_detail=error)
+        self.check_validation_error(self.post, body={'foo': 'server '},
+                                    expected_detail=detail)
+
+        detail = ("Invalid input for field/attribute foo. Value:  a."
+                  " ' a' does not match .*")
+        self.check_validation_error(self.post, body={'foo': ' a'},
+                                    expected_detail=detail)
+
+        detail = ("Invalid input for field/attribute foo. Value: a ."
+                  " 'a ' does not match .*")
+        self.check_validation_error(self.post, body={'foo': 'a '},
+                                    expected_detail=detail)
+
+        # NOTE(stpierre): Quoting for the unicode values in the error
+        # messages below gets *really* messy, so we just wildcard it
+        # out. (e.g., '.* does not match'). In practice, we don't
+        # particularly care about that part of the error message.
+
+        # trailing unicode space
+        detail = (u"Invalid input for field/attribute foo. Value: a\xa0."
+                  u' .* does not match .*')
+        self.check_validation_error(self.post, body={'foo': u'a\xa0'},
+                                    expected_detail=detail)
+
+        # non-printable unicode
+        detail = (u"Invalid input for field/attribute foo. Value: \uffff."
+                  u" .* does not match .*")
+        self.check_validation_error(self.post, body={'foo': u'\uffff'},
+                                    expected_detail=detail)
 
         # four-byte unicode, if supported by this python build
         try:
+            detail = (u"Invalid input for field/attribute foo. Value: "
+                      u"\U00010000. .* does not match .*")
             self.check_validation_error(self.post, body={'foo': u'\U00010000'},
-                                        expected_detail=error)
+                                        expected_detail=detail)
         except ValueError:
             pass
 
@@ -899,23 +799,34 @@ class NameWithLeadingTrailingSpacesTestCase(APIValidationTestCase):
                                    req=FakeRequest()))
 
     def test_validate_name_fails(self):
-        error = ("An invalid 'name' value was provided. The name must be: "
+        detail = (u"Invalid input for field/attribute foo. Value:  ."
-                 "printable characters with at least one non space character")
+                  u" ' ' does not match .*")
+        self.check_validation_error(self.post, body={'foo': ' '},
+                                    expected_detail=detail)
 
-        should_fail = (
+        # NOTE(stpierre): Quoting for the unicode values in the error
-            ' ',
+        # messages below gets *really* messy, so we just wildcard it
-            u'\xa0',  # unicode space
+        # out. (e.g., '.* does not match'). In practice, we don't
-            u'\uffff',  # non-printable unicode
+        # particularly care about that part of the error message.
-        )
 
-        for item in should_fail:
+        # unicode space
-            self.check_validation_error(self.post, body={'foo': item},
+        detail = (u"Invalid input for field/attribute foo. Value: \xa0."
-                                    expected_detail=error)
+                  u' .* does not match .*')
+        self.check_validation_error(self.post, body={'foo': u'\xa0'},
+                                    expected_detail=detail)
+
+        # non-printable unicode
+        detail = (u"Invalid input for field/attribute foo. Value: \uffff."
+                  u" .* does not match .*")
+        self.check_validation_error(self.post, body={'foo': u'\uffff'},
+                                    expected_detail=detail)
 
         # four-byte unicode, if supported by this python build
         try:
+            detail = (u"Invalid input for field/attribute foo. Value: "
+                      u"\U00010000. .* does not match .*")
             self.check_validation_error(self.post, body={'foo': u'\U00010000'},
-                                        expected_detail=error)
+                                        expected_detail=detail)
         except ValueError:
             pass