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

nova/api/openstack/compute/schemas/aggregates.py

@@ -16,11 +16,11 @@ import copy
 
 from nova.api.validation import parameter_types
 
-availability_zone = copy.deepcopy(parameter_types.name)
-availability_zone['type'] = ['string', 'null']
-availability_zone_with_leading_trailing_spaces = copy.deepcopy(parameter_types.
-    name_with_leading_trailing_spaces)
-availability_zone_with_leading_trailing_spaces['type'] = ['string', 'null']
+availability_zone = {'oneOf': [parameter_types.name, {'type': 'null'}]}
+availability_zone_with_leading_trailing_spaces = {
+    'oneOf': [parameter_types.name_with_leading_trailing_spaces,
+              {'type': 'null'}]
+}
 
 
 create = {

nova/api/validation/parameter_types.py

@@ -21,6 +21,14 @@ 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.
@@ -110,10 +118,12 @@ def _build_regex_range(ws=True, invert=False, exclude=None):
 valid_name_regex_base = '^(?![%s])[%s]*(?<![%s])$'
 
 
-valid_name_regex = valid_name_regex_base % (
-    _build_regex_range(ws=False, invert=True),
-    _build_regex_range(),
-    _build_regex_range(ws=False, invert=True))
+valid_name_regex = ValidationRegex(
+    valid_name_regex_base % (
+        _build_regex_range(ws=False, invert=True),
+        _build_regex_range(),
+        _build_regex_range(ws=False, invert=True)),
+    _("printable characters. Can not start or end with whitespace."))
 
 
 # This regex allows leading/trailing whitespace
@@ -122,26 +132,32 @@ 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 = valid_name_regex_base % (
-    _build_regex_range(ws=False, invert=True),
-    _build_regex_range(exclude=['!', '.', '@']),
-    _build_regex_range(ws=False, invert=True))
+valid_cell_name_regex = ValidationRegex(
+    valid_name_regex_base % (
+        _build_regex_range(ws=False, invert=True),
+        _build_regex_range(exclude=['!', '.', '@']),
+        _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 = (
+valid_cell_name_leading_trailing_spaces_regex = ValidationRegex(
     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 = (
+valid_name_leading_trailing_spaces_regex = ValidationRegex(
     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, re.UNICODE)
+valid_name_regex_obj = re.compile(valid_name_regex.regex, re.UNICODE)
 
 
 valid_description_regex_base = '^[%s]*$'
@@ -201,25 +217,25 @@ name = {
     # stored in the DB and Nova specific parameters.
     # This definition is used for all their parameters.
     'type': 'string', 'minLength': 1, 'maxLength': 255,
-    'pattern': valid_name_regex,
+    'format': 'name'
 }
 
 
 cell_name = {
     'type': 'string', 'minLength': 1, 'maxLength': 255,
-    'pattern': valid_cell_name_regex,
+    'format': 'cell_name'
 }
 
 
 cell_name_leading_trailing_spaces = {
     'type': 'string', 'minLength': 1, 'maxLength': 255,
-    'pattern': valid_cell_name_leading_trailing_spaces_regex,
+    'format': 'cell_name_with_leading_trailing_spaces'
 }
 
 
 name_with_leading_trailing_spaces = {
     'type': 'string', 'minLength': 1, 'maxLength': 255,
-    'pattern': valid_name_leading_trailing_spaces_regex,
+    'format': 'name_with_leading_trailing_spaces'
 }
 
 

nova/api/validation/validators.py

@@ -27,6 +27,7 @@ from oslo_utils import uuidutils
 import rfc3986
 import six
 
+from nova.api.validation import parameter_types
 from nova import exception
 from nova.i18n import _
 
@@ -75,6 +76,60 @@ 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
+    try:
+        if re.search(regex.regex, instance):
+            return True
+    except TypeError:
+        # The name must be string type. If instance isn't string type, the
+        # TypeError will be raised at here.
+        pass
+    raise exception.InvalidName(reason=regex.reason)
+
+
+@jsonschema.FormatChecker.cls_checks('name', exception.InvalidName)
+def _validate_name(instance):
+    regex = parameter_types.valid_name_regex
+    try:
+        if re.search(regex.regex, instance):
+            return True
+    except TypeError:
+        # The name must be string type. If instance isn't string type, the
+        # TypeError will be raised at here.
+        pass
+    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
+    try:
+        if re.search(regex.regex, instance):
+            return True
+    except TypeError:
+        # The name must be string type. If instance isn't string type, the
+        # TypeError will be raised at here.
+        pass
+    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
+    try:
+        if re.search(regex.regex, instance):
+            return True
+    except TypeError:
+        # The name must be string type. If instance isn't string type, the
+        # TypeError will be raised at here.
+        pass
+    raise exception.InvalidName(reason=regex.reason)
+
+
 def _soft_validate_additional_properties(validator,
                                          additional_properties_value,
                                          instance,
@@ -132,6 +187,41 @@ 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:
+            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
 
@@ -156,16 +246,18 @@ class _SchemaValidator(object):
 
         validator_cls = jsonschema.validators.extend(self.validator_org,
                                                      validators)
-        format_checker = jsonschema.FormatChecker()
+        format_checker = FormatChecker()
         self.validator = validator_cls(schema, format_checker=format_checker)
 
     def validate(self, *args, **kwargs):
         try:
             self.validator.validate(*args, **kwargs)
         except jsonschema.ValidationError as ex:
-            # NOTE: For whole OpenStack message consistency, this error
-            #       message has been written as the similar format of WSME.
-            if len(ex.path) > 0:
+            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.
                 detail = _("Invalid input for field/attribute %(path)s."
                            " Value: %(value)s. %(message)s") % {
                                'path': ex.path.pop(), 'value': ex.instance,

nova/exception.py

@@ -420,6 +420,11 @@ 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/api/openstack/compute/test_aggregates.py

@@ -16,6 +16,7 @@
 """Tests for the aggregates admin api."""
 
 import mock
+import uuid
 from webob import exc
 
 from nova.api.openstack.compute import aggregates as aggregates_v21
@@ -264,6 +265,19 @@ class AggregateTestCaseV21(test.NoDBTestCase):
                                      {"name": "test",
                                       "availability_zone": ""}})
 
+    @mock.patch('nova.compute.api.AggregateAPI.create_aggregate')
+    def test_create_with_none_availability_zone(self, mock_create_agg):
+        mock_create_agg.return_value = objects.Aggregate(self.context,
+                                                         name='test',
+                                                         uuid=uuid.uuid4(),
+                                                         hosts=[],
+                                                         metadata={})
+        body = {"aggregate": {"name": "test",
+                              "availability_zone": None}}
+        result = self.controller.create(self.req, body=body)
+        mock_create_agg.assert_called_once_with(self.context, 'test', None)
+        self.assertEqual(result['aggregate']['name'], 'test')
+
     def test_create_with_extra_invalid_arg(self):
         self.assertRaises(self.bad_request, self.controller.create,
                           self.req, body={"name": "test",
@@ -381,6 +395,21 @@ class AggregateTestCaseV21(test.NoDBTestCase):
         self.assertRaises(self.bad_request, self.controller.update,
                           self.req, "2", body=test_metadata)
 
+    @mock.patch('nova.compute.api.AggregateAPI.update_aggregate')
+    def test_update_with_none_availability_zone(self, mock_update_agg):
+        agg_id = uuid.uuid4()
+        mock_update_agg.return_value = objects.Aggregate(self.context,
+                                                         name='test',
+                                                         uuid=agg_id,
+                                                         hosts=[],
+                                                         metadata={})
+        body = {"aggregate": {"name": "test",
+                              "availability_zone": None}}
+        result = self.controller.update(self.req, agg_id, body=body)
+        mock_update_agg.assert_called_once_with(self.context, agg_id,
+                                                body['aggregate'])
+        self.assertEqual(result['aggregate']['name'], 'test')
+
     def test_update_with_bad_aggregate(self):
         test_metadata = {"aggregate": {"name": "test_name"}}
 

nova/tests/unit/test_api_validation.py

@@ -16,11 +16,13 @@ import copy
 import re
 
 import fixtures
+from jsonschema import exceptions as jsonschema_exc
 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
 
@@ -36,7 +38,7 @@ class FakeRequest(object):
 
 class ValidationRegex(test.NoDBTestCase):
     def test_cell_names(self):
-        cellre = re.compile(parameter_types.valid_cell_name_regex)
+        cellre = re.compile(parameter_types.valid_cell_name_regex.regex)
         self.assertTrue(cellre.search('foo'))
         self.assertFalse(cellre.search('foo.bar'))
         self.assertFalse(cellre.search('foo@bar'))
@@ -108,6 +110,33 @@ 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(jsonschema_exc.FormatError,
+                                format_checker.check, "   ", "name")
+        self.assertIsInstance(exc.cause, exception.InvalidName)
+        self.assertEqual("An invalid 'name' value was provided. The name must "
+                         "be: printable characters. "
+                         "Can not start or end with whitespace.",
+                         exc.cause.format_message())
+
+    def test_format_checker_failed_with_non_string(self):
+        checks = ["name", "name_with_leading_trailing_spaces",
+                  "cell_name", "cell_name_with_leading_trailing_spaces"]
+        format_checker = validators.FormatChecker()
+
+        for check in checks:
+            exc = self.assertRaises(jsonschema_exc.FormatError,
+                                    format_checker.check, None, "name")
+            self.assertIsInstance(exc.cause, exception.InvalidName)
+            self.assertEqual("An invalid 'name' value was provided. The name "
+                             "must be: printable characters. "
+                             "Can not start or end with whitespace.",
+                             exc.cause.format_message())
+
+
 class MicroversionsSchemaTestCase(APIValidationTestCase):
 
     def setUp(self):
@@ -668,6 +697,122 @@ 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):
@@ -701,54 +846,25 @@ class NameTestCase(APIValidationTestCase):
                                    req=FakeRequest()))
 
     def test_validate_name_fails(self):
-        detail = (u"Invalid input for field/attribute foo. Value:  ."
-                  " ' ' does not match .*")
-        self.check_validation_error(self.post, body={'foo': ' '},
-                                    expected_detail=detail)
+        error = ("An invalid 'name' value was provided. The name must be: "
+                 "printable characters. "
+                 "Can not start or end with whitespace.")
 
-        detail = ("Invalid input for field/attribute foo. Value:  server."
-                  " ' server' does not match .*")
-        self.check_validation_error(self.post, body={'foo': ' server'},
-                                    expected_detail=detail)
+        should_fail = (' ',
+                       ' server',
+                       'server ',
+                       u'a\xa0',  # trailing unicode space
+                       u'\uffff',  # non-printable unicode
+                       )
 
-        detail = ("Invalid input for field/attribute foo. Value: server ."
-                  " 'server ' does not match .*")
-        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)
+        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:
-            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=detail)
+                                        expected_detail=error)
         except ValueError:
             pass
 
@@ -799,34 +915,23 @@ class NameWithLeadingTrailingSpacesTestCase(APIValidationTestCase):
                                    req=FakeRequest()))
 
     def test_validate_name_fails(self):
-        detail = (u"Invalid input for field/attribute foo. Value:  ."
-                  u" ' ' does not match .*")
-        self.check_validation_error(self.post, body={'foo': ' '},
-                                    expected_detail=detail)
+        error = ("An invalid 'name' value was provided. The name must be: "
+                 "printable characters with at least one non space character")
 
-        # 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.
+        should_fail = (
+            ' ',
+            u'\xa0',  # unicode space
+            u'\uffff',  # non-printable unicode
+        )
 
-        # unicode space
-        detail = (u"Invalid input for field/attribute foo. Value: \xa0."
-                  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)
+        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:
-            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=detail)
+                                        expected_detail=error)
         except ValueError:
             pass
 
@@ -1193,7 +1298,7 @@ class Ipv6TestCase(APIValidationTestCase):
         detail = ("Invalid input for field/attribute foo. Value: localhost."
                   " 'localhost' is not a 'ipv6'")
         self.check_validation_error(self.post, body={'foo': 'localhost'},
-                                    expected_detail=detail)
+                                        expected_detail=detail)
 
         detail = ("Invalid input for field/attribute foo."
                   " Value: 192.168.0.100. '192.168.0.100' is not a 'ipv6'")