From eff9f92f0192595d484fc02cc2e702636758a29c Mon Sep 17 00:00:00 2001 From: Valeriy Ponomaryov Date: Thu, 28 May 2015 13:38:05 +0300 Subject: [PATCH] Remove unused attr status from models Models "security_services" and "network_allocations" have attr "status" that is not used indeed. So, remove it from models and add appropriate migrations. Closes-Bug: #1459660 Change-Id: Idb3a69916e8052b16c9daebb9bb67b09d1714c46 --- .../api/share/test_security_services.py | 1 - manila/api/v1/security_service.py | 30 ++++---- manila/api/views/security_service.py | 31 ++++----- .../533646c7af38_remove_unused_attr_status.py | 68 +++++++++++++++++++ manila/db/sqlalchemy/models.py | 6 -- manila/tests/api/v1/test_security_service.py | 3 +- .../tests/network/test_security_service_db.py | 3 - 7 files changed, 96 insertions(+), 46 deletions(-) create mode 100644 manila/db/migrations/alembic/versions/533646c7af38_remove_unused_attr_status.py diff --git a/contrib/tempest/tempest/api/share/test_security_services.py b/contrib/tempest/tempest/api/share/test_security_services.py index ec00c2b3c6..cd400bebb5 100644 --- a/contrib/tempest/tempest/api/share/test_security_services.py +++ b/contrib/tempest/tempest/api/share/test_security_services.py @@ -82,7 +82,6 @@ class SecurityServiceListMixin(object): @test.attr(type=["gate", "smoke"]) def test_list_security_services_detailed_filter_by_ss_attributes(self): search_opts = { - 'status': 'NEW', 'name': 'ss_ldap', 'type': 'ldap', 'user': 'fake_user', diff --git a/manila/api/v1/security_service.py b/manila/api/v1/security_service.py index 6871b6f033..c1c49b6cd3 100644 --- a/manila/api/v1/security_service.py +++ b/manila/api/v1/security_service.py @@ -98,6 +98,9 @@ class SecurityServiceController(wsgi.Controller): search_opts = {} search_opts.update(req.GET) + # NOTE(vponomaryov): remove 'status' from search opts + # since it was removed from security service model. + search_opts.pop('status', None) if 'share_network_id' in search_opts: share_nw = db.share_network_get(context, search_opts['share_network_id']) @@ -141,7 +144,7 @@ class SecurityServiceController(wsgi.Controller): return security_services def _get_security_services_search_options(self): - return ('status', 'name', 'id', 'type', 'user', + return ('name', 'id', 'type', 'user', 'server', 'dns_ip', 'domain', ) def _share_servers_dependent_on_sn_exist(self, context, @@ -173,23 +176,18 @@ class SecurityServiceController(wsgi.Controller): except exception.NotFound: raise exc.HTTPNotFound() - if security_service['status'].lower() in ['new', 'inactive']: - update_dict = security_service_data - if self._share_servers_dependent_on_sn_exist(context, id): - for item in update_dict: - if item not in valid_update_keys: - msg = _("Cannot update security service %s. It is " - "attached to share network with share server " - "associated. Only 'name' and 'description' " - "fields are available for update.") % id - raise exc.HTTPForbidden(explanation=msg) - else: - update_dict = dict([(key, security_service_data[key]) - for key in valid_update_keys - if key in security_service_data]) + if self._share_servers_dependent_on_sn_exist(context, id): + for item in security_service_data: + if item not in valid_update_keys: + msg = _("Cannot update security service %s. It is " + "attached to share network with share server " + "associated. Only 'name' and 'description' " + "fields are available for update.") % id + raise exc.HTTPForbidden(explanation=msg) policy.check_policy(context, RESOURCE_NAME, 'update', security_service) - security_service = db.security_service_update(context, id, update_dict) + security_service = db.security_service_update( + context, id, security_service_data) return self._view_builder.detail(req, security_service) def create(self, req, body): diff --git a/manila/api/views/security_service.py b/manila/api/views/security_service.py index ce284447c2..81852c31cb 100644 --- a/manila/api/views/security_service.py +++ b/manila/api/views/security_service.py @@ -14,6 +14,7 @@ # under the License. from manila.api import common +from manila.common import constants class ViewBuilder(common.ViewBuilder): @@ -36,29 +37,23 @@ class ViewBuilder(common.ViewBuilder): 'id': security_service.get('id'), 'name': security_service.get('name'), 'type': security_service.get('type'), - 'status': security_service.get('status') + # NOTE(vponomaryov): attr "status" was removed from model and + # is left in view for compatibility purposes since it affects + # user-facing API. This should be removed right after no one + # uses it anymore. + 'status': constants.STATUS_NEW, } } def detail(self, request, security_service): """Detailed view of a single security service.""" - return { - 'security_service': { - 'id': security_service.get('id'), - 'name': security_service.get('name'), - 'created_at': security_service.get('created_at'), - 'updated_at': security_service.get('updated_at'), - 'status': security_service.get('status'), - 'description': security_service.get('description'), - 'dns_ip': security_service.get('dns_ip'), - 'server': security_service.get('server'), - 'domain': security_service.get('domain'), - 'user': security_service.get('user'), - 'password': security_service.get('password'), - 'type': security_service.get('type'), - 'project_id': security_service.get('project_id'), - } - } + view = self.summary(request, security_service) + keys = ( + 'created_at', 'updated_at', 'description', 'dns_ip', 'server', + 'domain', 'user', 'password', 'project_id') + for key in keys: + view['security_service'][key] = security_service.get(key) + return view def _list_view(self, func, request, security_services): """Provide a view for a list of security services.""" diff --git a/manila/db/migrations/alembic/versions/533646c7af38_remove_unused_attr_status.py b/manila/db/migrations/alembic/versions/533646c7af38_remove_unused_attr_status.py new file mode 100644 index 0000000000..cfed0f0c05 --- /dev/null +++ b/manila/db/migrations/alembic/versions/533646c7af38_remove_unused_attr_status.py @@ -0,0 +1,68 @@ +# Copyright 2015 Mirantis, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Remove unused attr status + +Revision ID: 533646c7af38 +Revises: 3a482171410f +Create Date: 2015-05-28 13:13:47.651353 + +""" + +# revision identifiers, used by Alembic. +revision = '533646c7af38' +down_revision = '3a482171410f' + +from alembic import op +from oslo_log import log +import sqlalchemy as sql + +from manila.common import constants +from manila.i18n import _LE + +LOG = log.getLogger(__name__) +COLUMN_NAME = 'status' +TABLE_NAMES = ('network_allocations', 'security_services') + + +def upgrade(): + for t_name in TABLE_NAMES: + try: + op.drop_column(t_name, COLUMN_NAME) + except Exception: + LOG.error(_LE("Column '%s' could not be dropped"), COLUMN_NAME) + raise + + +def downgrade(): + for t_name in TABLE_NAMES: + try: + op.add_column( + t_name, + sql.Column( + COLUMN_NAME, + # NOTE(vponomaryov): original type of attr was enum. But + # alembic is buggy with enums [1], so use string type + # instead. Anyway we have no reason to keep enum/constraint + # on specific set of possible statuses because they have + # not been used. + # [1] - https://bitbucket.org/zzzeek/alembic/ + # issue/89/opadd_column-and-opdrop_column-should + sql.String(255), + default=constants.STATUS_NEW, + ), + ) + except Exception: + LOG.error(_LE("Column '%s' could not be added"), COLUMN_NAME) + raise diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 02c6fe9065..8f39f9e402 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -349,9 +349,6 @@ class SecurityService(BASE, ManilaBase): password = Column(String(255), nullable=True) name = Column(String(255), nullable=True) description = Column(String(255), nullable=True) - status = Column(Enum(constants.STATUS_NEW, constants.STATUS_ACTIVE, - constants.STATUS_ERROR), - default=constants.STATUS_NEW) class ShareNetwork(BASE, ManilaBase): @@ -468,9 +465,6 @@ class NetworkAllocation(BASE, ManilaBase): mac_address = Column(String(32), nullable=True) share_server_id = Column(String(36), ForeignKey('share_servers.id'), nullable=False) - status = Column(Enum(constants.STATUS_NEW, constants.STATUS_ACTIVE, - constants.STATUS_ERROR), - default=constants.STATUS_NEW) class DriverPrivateData(BASE, ManilaBase): diff --git a/manila/tests/api/v1/test_security_service.py b/manila/tests/api/v1/test_security_service.py index a6e08ab6a4..1fa4a3a7d1 100644 --- a/manila/tests/api/v1/test_security_service.py +++ b/manila/tests/api/v1/test_security_service.py @@ -43,7 +43,7 @@ class ShareApiTest(test.TestCase): "domain": "fake-domain", "user": "fake-user", "password": "fake-password", - "status": "new", + "status": constants.STATUS_NEW, "project_id": "fake", } self.ss_ldap = { @@ -66,7 +66,6 @@ class ShareApiTest(test.TestCase): 'server': 'fake-server', 'dns_ip': '1.1.1.1', 'domain': 'fake-domain', - 'status': 'new', 'type': constants.SECURITY_SERVICES_ALLOWED_TYPES[0], } self.check_policy_patcher = mock.patch( diff --git a/manila/tests/network/test_security_service_db.py b/manila/tests/network/test_security_service_db.py index 23a183a5bc..1fe70a4801 100644 --- a/manila/tests/network/test_security_service_db.py +++ b/manila/tests/network/test_security_service_db.py @@ -15,7 +15,6 @@ from oslo_db import exception as db_exception -from manila.common import constants from manila import context from manila.db import api as db_api from manila import exception @@ -33,7 +32,6 @@ security_service_dict = { 'password': 'fake password', 'name': 'whatever', 'description': 'nevermind', - 'status': constants.STATUS_NEW, } @@ -101,7 +99,6 @@ class SecurityServiceDBTest(test.TestCase): 'password': 'new password', 'name': 'new whatever', 'description': 'new nevermind', - 'status': constants.STATUS_ERROR, } db_api.security_service_create(self.fake_context,