diff --git a/manila/api/v2/share_group_types.py b/manila/api/v2/share_group_types.py index ae2720d4a3..fe9114ca9b 100644 --- a/manila/api/v2/share_group_types.py +++ b/manila/api/v2/share_group_types.py @@ -138,6 +138,8 @@ class ShareGroupTypesController(wsgi.Controller): context, name) except exception.ShareGroupTypeExists as err: raise webob.exc.HTTPConflict(explanation=six.text_type(err)) + except exception.ShareTypeDoesNotExist as err: + raise webob.exc.HTTPNotFound(explanation=six.text_type(err)) except exception.NotFound: raise webob.exc.HTTPNotFound() return self._view_builder.show(req, share_group_type) diff --git a/manila/db/api.py b/manila/db/api.py index 385c88c834..570a02997e 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -894,6 +894,11 @@ def share_type_get_by_name(context, name): return IMPL.share_type_get_by_name(context, name) +def share_type_get_by_name_or_id(context, name_or_id): + """Get share type by name or ID and return None if not found.""" + return IMPL.share_type_get_by_name_or_id(context, name_or_id) + + def share_type_access_get_all(context, type_id): """Get all share type access of a share type.""" return IMPL.share_type_access_get_all(context, type_id) diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index db7f36ccc0..bb942bfd95 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -3537,10 +3537,24 @@ def _share_type_get_by_name(context, name, session=None): @require_context def share_type_get_by_name(context, name): """Return a dict describing specific share_type.""" - return _share_type_get_by_name(context, name) +@require_context +def share_type_get_by_name_or_id(context, name_or_id): + """Return a dict describing specific share_type using its name or ID. + + :returns: ShareType object or None if not found + """ + try: + return _share_type_get(context, name_or_id) + except exception.ShareTypeNotFound: + try: + return _share_type_get_by_name(context, name_or_id) + except exception.ShareTypeNotFoundByName: + return None + + @require_admin_context def share_type_destroy(context, id): session = get_session() @@ -4201,10 +4215,13 @@ def share_group_type_create(context, values, projects=None): values['group_specs'] = _metadata_refs( values.get('group_specs'), models.ShareGroupTypeSpecs) mappings = [] - for item in values.get('share_types') or []: + for item in values.get('share_types', []): + share_type = share_type_get_by_name_or_id(context, item) + if not share_type: + raise exception.ShareTypeDoesNotExist(share_type=item) mapping = models.ShareGroupTypeShareTypeMapping() mapping['id'] = six.text_type(uuidutils.generate_uuid()) - mapping['share_type_id'] = item + mapping['share_type_id'] = share_type['id'] mapping['share_group_type_id'] = values['id'] mappings.append(mapping) @@ -4214,6 +4231,8 @@ def share_group_type_create(context, values, projects=None): share_group_type_ref.save(session=session) except db_exception.DBDuplicateEntry: raise exception.ShareGroupTypeExists(type_id=values['name']) + except exception.ShareTypeDoesNotExist: + raise except Exception as e: raise db_exception.DBError(e) diff --git a/manila/exception.py b/manila/exception.py index f2959a3b2c..d0b813b3e8 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -619,6 +619,10 @@ class ShareTypeExists(ManilaException): message = _("Share Type %(id)s already exists.") +class ShareTypeDoesNotExist(NotFound): + message = _("Share Type %(share_type)s does not exist.") + + class ShareGroupTypeExists(ManilaException): message = _("Share group type %(type_id)s already exists.") diff --git a/manila/tests/api/v2/test_share_group_types.py b/manila/tests/api/v2/test_share_group_types.py index 319289e5e3..abecc498d5 100644 --- a/manila/tests/api/v2/test_share_group_types.py +++ b/manila/tests/api/v2/test_share_group_types.py @@ -390,6 +390,19 @@ class ShareGroupTypesAPITest(test.TestCase): webob.exc.HTTPBadRequest, self.controller._create, req, fake_body) + def test_create_provided_share_type_does_not_exist(self): + req = fake_request('/v2/fake/share-group-types', admin=True) + fake_body = { + 'share_group_type': { + 'name': GROUP_TYPE_1['name'], + 'share_types': SHARE_TYPE_ID + '_does_not_exist', + } + } + + self.assertRaises( + webob.exc.HTTPNotFound, + self.controller._create, req, fake_body) + class ShareGroupTypeAccessTest(test.TestCase): diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index d989794b18..e783793138 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -2576,3 +2576,37 @@ class PurgeDeletedTest(test.TestCase): type_row = db_api.model_query(self.context, models.ShareTypes).count() self.assertEqual(0, s_row + type_row) + + +class ShareTypeAPITestCase(test.TestCase): + + def setUp(self): + super(self.__class__, self).setUp() + self.ctxt = context.RequestContext( + user_id='user_id', project_id='project_id', is_admin=True) + + def test_share_type_get_by_name_or_id_found_by_id(self): + share_type = db_utils.create_share_type() + + result = db_api.share_type_get_by_name_or_id( + self.ctxt, share_type['id']) + + self.assertIsNotNone(result) + self.assertEqual(share_type['id'], result['id']) + + def test_share_type_get_by_name_or_id_found_by_name(self): + name = uuidutils.generate_uuid() + db_utils.create_share_type(name=name) + + result = db_api.share_type_get_by_name_or_id(self.ctxt, name) + + self.assertIsNotNone(result) + self.assertEqual(name, result['name']) + self.assertNotEqual(name, result['id']) + + def test_share_type_get_by_name_or_id_when_does_not_exist(self): + fake_id = uuidutils.generate_uuid() + + result = db_api.share_type_get_by_name_or_id(self.ctxt, fake_id) + + self.assertIsNone(result) diff --git a/manila/tests/test_exception.py b/manila/tests/test_exception.py index bc1f9c6f6b..d882d6f3b3 100644 --- a/manila/tests/test_exception.py +++ b/manila/tests/test_exception.py @@ -469,6 +469,13 @@ class ManilaExceptionResponseCode404(test.TestCase): self.assertEqual(404, e.code) self.assertIn(share_type_name, e.msg) + def test_share_type_does_not_exist(self): + # verify response code for exception.ShareTypeDoesNotExist + share_type = "fake_share_type_1234" + e = exception.ShareTypeDoesNotExist(share_type=share_type) + self.assertEqual(404, e.code) + self.assertIn(share_type, e.msg) + def test_share_type_extra_specs_not_found(self): # verify response code for exception.ShareTypeExtraSpecsNotFound share_type_id = "fake_share_type_id" diff --git a/manila_tempest_tests/tests/api/admin/test_share_group_types.py b/manila_tempest_tests/tests/api/admin/test_share_group_types.py index a6bda73401..532e773ff0 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_group_types.py +++ b/manila_tempest_tests/tests/api/admin/test_share_group_types.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt from tempest import config from tempest.lib.common.utils import data_utils import testtools @@ -27,6 +28,7 @@ CONF = config.CONF @testtools.skipUnless( CONF.share.run_share_group_tests, 'Share Group tests disabled.') @base.skip_if_microversion_lt(constants.MIN_SHARE_GROUP_MICROVERSION) +@ddt.ddt class ShareGroupTypesTest(base.BaseSharesAdminTest): @classmethod @@ -44,13 +46,14 @@ class ShareGroupTypesTest(base.BaseSharesAdminTest): cls.share_type2 = share_type['share_type'] @tc.attr(base.TAG_POSITIVE, base.TAG_API) - def test_create_get_delete_share_group_type_min(self): + @ddt.data('id', 'name') + def test_create_get_delete_share_group_type_min(self, st_key): name = data_utils.rand_name("tempest-manila") # Create share group type sg_type_c = self.create_share_group_type( name=name, - share_types=self.share_type['id'], + share_types=self.share_type[st_key], cleanup_in_class=False, version=constants.MIN_SHARE_GROUP_MICROVERSION) @@ -76,12 +79,13 @@ class ShareGroupTypesTest(base.BaseSharesAdminTest): share_group_type_id=sg_type_r['id']) @tc.attr(base.TAG_POSITIVE, base.TAG_API) - def test_create_share_group_type_multiple_share_types_min(self): + @ddt.data('id', 'name') + def test_create_share_group_type_multiple_share_types_min(self, st_key): name = data_utils.rand_name("tempest-manila") sg_type = self.create_share_group_type( name=name, - share_types=[self.share_type['id'], self.share_type2['id']], + share_types=[self.share_type[st_key], self.share_type2[st_key]], cleanup_in_class=False, version=constants.MIN_SHARE_GROUP_MICROVERSION) diff --git a/manila_tempest_tests/tests/api/admin/test_share_group_types_negative.py b/manila_tempest_tests/tests/api/admin/test_share_group_types_negative.py index e7f682402d..18e8cdde2f 100644 --- a/manila_tempest_tests/tests/api/admin/test_share_group_types_negative.py +++ b/manila_tempest_tests/tests/api/admin/test_share_group_types_negative.py @@ -40,10 +40,19 @@ class ShareGroupTypesAdminNegativeTest(base.BaseSharesMixedTest): client=cls.admin_shares_v2_client) @tc.attr(base.TAG_NEGATIVE, base.TAG_API) - def test_create_share_ggroup_with_nonexistent_share_type(self): + def test_create_share_group_type_without_name(self): self.assertRaises( lib_exc.BadRequest, self.admin_shares_v2_client.create_share_group_type, + name=None, + share_types=data_utils.rand_name("fake")) + + @tc.attr(base.TAG_NEGATIVE, base.TAG_API) + def test_create_share_group_type_with_nonexistent_share_type(self): + self.assertRaises( + lib_exc.NotFound, + self.admin_shares_v2_client.create_share_group_type, + name=data_utils.rand_name("sgt_name_should_have_not_been_created"), share_types=data_utils.rand_name("fake")) @tc.attr(base.TAG_NEGATIVE, base.TAG_API)