From 82c3afa5a45952123743a62d057e42844da85097 Mon Sep 17 00:00:00 2001
From: He Jie Xu <hejie.xu@intel.com>
Date: Wed, 17 Feb 2016 16:26:19 +0800
Subject: [PATCH] Generate better validation error message when using name
 regexes

There are some regexes used in the json-schema for complex string validation.
When the validation failed, json-schema lib didn't generate a useful error
info for the user, it shows the regex to the user. But regex is really
unreadable for normal user. This patch override the default FormatChecker to
support passed in custom error message. This required using custom format
checker instead of using 'pattern'.

For aggregates API, it enabled 'null' in the name input. As the 'format'
keyword works for all allowed types and name format check will only validate
string type, so this patch change the schema to use 'oneOf' keyword, then
the 'format' will only against on string type.

Change-Id: Ic0e608b8a18b635bfcd936f57f14c9f54e1ef8b4
Partial-Bug: #1541691
---
 .../openstack/compute/schemas/aggregates.py   | 10 +++----
 .../api/openstack/compute/test_aggregates.py  | 29 +++++++++++++++++++
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/nova/api/openstack/compute/schemas/aggregates.py b/nova/api/openstack/compute/schemas/aggregates.py
index 52f0a6878..5283429e7 100644
--- a/nova/api/openstack/compute/schemas/aggregates.py
+++ b/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 = {
diff --git a/nova/tests/unit/api/openstack/compute/test_aggregates.py b/nova/tests/unit/api/openstack/compute/test_aggregates.py
index d3d2534e5..0275e6429 100644
--- a/nova/tests/unit/api/openstack/compute/test_aggregates.py
+++ b/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
@@ -263,6 +264,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",
@@ -380,6 +394,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"}}