Merge "Fix combination of parameters for update APIs"

cinder/api/contrib/types_manage.py

@@ -99,6 +99,18 @@ class VolumeTypesManageController(wsgi.Controller):
         if is_public is not None:
             is_public = strutils.bool_from_string(is_public, strict=True)
 
+        # If name specified, name can not be empty.
+        if name and len(name.strip()) == 0:
+            msg = _("Volume type name can not be empty.")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        # Name, description and is_public can not be None.
+        # Specify one of them, or a combination thereof.
+        if name is None and description is None and is_public is None:
+            msg = _("Specify volume type name, description, is_public or "
+                    "a combination thereof.")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
         try:
             volume_types.update(context, id, name, description,
                                 is_public=is_public)

cinder/api/schemas/group_types.py

@@ -48,16 +48,11 @@ update = {
         'group_type': {
             'type': 'object',
             'properties': {
-                'name': parameter_types.name,
+                'name': parameter_types.name_allow_zero_min_length,
                 'description': parameter_types.description,
                 'is_public': parameter_types.boolean,
             },
             'additionalProperties': False,
-            'anyOf': [
-                {'required': ['name']},
-                {'required': ['description']},
-                {'required': ['is_public']},
-            ]
         },
     },
     'required': ['group_type'],

cinder/api/schemas/volume_types.py

@@ -44,16 +44,11 @@ update = {
         'volume_type': {
             'type': 'object',
             'properties': {
-                'name': parameter_types.name,
+                'name': parameter_types.name_allow_zero_min_length,
                 'description': parameter_types.description,
                 'is_public': parameter_types.boolean,
             },
             'additionalProperties': False,
-            'anyOf': [
-                {'required': ['name']},
-                {'required': ['description']},
-                {'required': ['is_public']}
-            ]
         },
     },
     'required': ['volume_type'],

cinder/api/v3/group_types.py

@@ -102,6 +102,17 @@ class GroupTypesController(wsgi.Controller):
         if is_public is not None:
             is_public = strutils.bool_from_string(is_public, strict=True)
 
+        # If name specified, name can not be empty.
+        if name and len(name.strip()) == 0:
+            msg = _("Group type name can not be empty.")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        # Name, description and is_public can not be None.
+        # Specify one of them, or a combination thereof.
+        if name is None and description is None and is_public is None:
+            msg = _("Specify group type name, description or "
+                    "a combination thereof.")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
         try:
             group_types.update(context, id, name, description,
                                is_public=is_public)

cinder/tests/unit/api/contrib/test_types_manage.py

@@ -502,7 +502,7 @@ class VolumeTypesManageApiTest(test.TestCase):
             fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
         req.method = 'PUT'
 
-        self.assertRaises(exception.ValidationError,
+        self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller._update, req,
                           DEFAULT_VOLUME_TYPE, body=body)
 
@@ -513,7 +513,7 @@ class VolumeTypesManageApiTest(test.TestCase):
             fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
         req.method = 'PUT'
 
-        self.assertRaises(exception.ValidationError,
+        self.assertRaises(webob.exc.HTTPBadRequest,
                           self.controller._update, req,
                           DEFAULT_VOLUME_TYPE, body=body)
 
@@ -699,3 +699,38 @@ class VolumeTypesManageApiTest(test.TestCase):
         if expected_results.get('is_public') is not None:
             self.assertEqual(expected_results['is_public'],
                              results['volume_type']['is_public'])
+
+    def test_update_with_name_null(self):
+        body = {"volume_type": {"name": None}}
+        req = fakes.HTTPRequest.blank('/v2/%s/types/%s' % (
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+        req.method = 'PUT'
+
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller._update, req,
+                          DEFAULT_VOLUME_TYPE, body=body)
+
+    @ddt.data({"volume_type": {"name": None, "description": "description"}},
+              {"volume_type": {"name": None, "is_public": True}},
+              {"volume_type": {"description": "description",
+                               "is_public": True}})
+    def test_update_volume_type(self, body):
+        req = fakes.HTTPRequest.blank('/v2/%s/types/%s' % (
+            fake.PROJECT_ID, DEFAULT_VOLUME_TYPE))
+        req.method = 'PUT'
+        ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID, True)
+        req.environ['cinder.context'] = ctxt
+        volume_type_1 = volume_types.create(ctxt, 'volume_type')
+        res = self.controller._update(req, volume_type_1.get('id'), body=body)
+
+        expected_name = body['volume_type'].get('name')
+        if expected_name is not None:
+            self.assertEqual(expected_name, res['volume_type']['name'])
+
+        expected_is_public = body['volume_type'].get('is_public')
+        if expected_is_public is not None:
+            self.assertEqual(expected_is_public,
+                             res['volume_type']['is_public'])
+
+        self.assertEqual(body['volume_type'].get('description'),
+                         res['volume_type']['description'])

cinder/tests/unit/api/v3/test_group_types.py

@@ -357,6 +357,43 @@ class GroupTypesApiTest(test.TestCase):
             self.ctxt, type_id, 'group_type1', None,
             is_public=boolean_is_public)
 
+    def test_update_group_type_with_name_null(self):
+        req = fakes.HTTPRequest.blank(
+            '/v3/%s/types/%s' % (fake.PROJECT_ID, fake.GROUP_TYPE_ID),
+            version=mv.GROUP_TYPE)
+        req.environ['cinder.context'] = self.ctxt
+        body = {"group_type": {"name": None}}
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
+                          req, fake.GROUP_TYPE_ID, body=body)
+
+    @ddt.data({"group_type": {"name": None,
+                              "description": "description"}},
+              {"group_type": {"name": "test",
+                              "is_public": True}},
+              {"group_type": {"description": None,
+                              "is_public": True}})
+    def test_update_group_type(self, body):
+        req = fakes.HTTPRequest.blank(
+            '/v3/%s/types/%s' % (fake.PROJECT_ID, fake.GROUP_TYPE_ID),
+            version=mv.GROUP_TYPE)
+
+        group_type_1 = group_types.create(self.ctxt, 'group_type')
+
+        req.environ['cinder.context'] = self.ctxt
+        res = self.controller.update(req, group_type_1.get('id'), body=body)
+
+        expected_name = body['group_type'].get('name')
+        if expected_name is not None:
+            self.assertEqual(expected_name, res['group_type']['name'])
+
+        expected_is_public = body['group_type'].get('is_public')
+        if expected_is_public is not None:
+            self.assertEqual(expected_is_public,
+                             res['group_type']['is_public'])
+
+        self.assertEqual(body['group_type'].get('description'),
+                         res['group_type']['description'])
+
     def test_group_types_show(self):
         self.mock_object(group_types, 'get_group_type',
                          return_group_types_get_group_type)