From af9d8f1de378c4ae49c3e06495f9ebdc68386dc6 Mon Sep 17 00:00:00 2001 From: Igor Malinovskiy Date: Mon, 13 Jul 2015 15:28:47 +0300 Subject: [PATCH] Use soft_delete() methods in DB api Use soft_delete() methods in DB api instead of manual models/query updates. Add custom soft_delete() implementations to provide possibility to update status. Partially implements bp cleanup-db-api-code Change-Id: Ib5861306abb92bf644eb4503236c194e92d90e2b --- manila/db/sqlalchemy/api.py | 108 ++++++------------------- manila/db/sqlalchemy/models.py | 8 ++ manila/db/sqlalchemy/query.py | 40 +++++++++ manila/tests/db/sqlalchemy/test_api.py | 14 ++++ 4 files changed, 86 insertions(+), 84 deletions(-) create mode 100644 manila/db/sqlalchemy/query.py diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 0bd3669d..82c413b5 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -23,6 +23,9 @@ import datetime import sys import warnings +# NOTE(uglide): Required to override default oslo_db Query class +import manila.db.sqlalchemy.query # noqa + from oslo_config import cfg from oslo_db import exception as db_exception from oslo_db import options as db_options @@ -34,7 +37,6 @@ from oslo_utils import uuidutils import six from sqlalchemy import or_ from sqlalchemy.orm import joinedload -from sqlalchemy.sql.expression import literal_column from sqlalchemy.sql.expression import true from sqlalchemy.sql import func @@ -995,10 +997,7 @@ def reservation_commit(context, reservations, project_id=None, user_id=None): if reservation.delta >= 0: usage.reserved -= reservation.delta usage.in_use += reservation.delta - reservation_query.update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + reservation_query.soft_delete(synchronize_session=False) @require_context @@ -1012,10 +1011,7 @@ def reservation_rollback(context, reservations, project_id=None, user_id=None): usage = usages[reservation.resource] if reservation.delta >= 0: usage.reserved -= reservation.delta - reservation_query.update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + reservation_query.soft_delete(synchronize_session=False) @require_admin_context @@ -1025,29 +1021,17 @@ def quota_destroy_all_by_project_and_user(context, project_id, user_id): model_query(context, models.ProjectUserQuota, session=session, read_deleted="no").\ filter_by(project_id=project_id).\ - filter_by(user_id=user_id).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + filter_by(user_id=user_id).soft_delete(synchronize_session=False) model_query(context, models.QuotaUsage, session=session, read_deleted="no").\ filter_by(project_id=project_id).\ - filter_by(user_id=user_id).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + filter_by(user_id=user_id).soft_delete(synchronize_session=False) model_query(context, models.Reservation, session=session, read_deleted="no").\ filter_by(project_id=project_id).\ - filter_by(user_id=user_id).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + filter_by(user_id=user_id).soft_delete(synchronize_session=False) @require_admin_context @@ -1057,34 +1041,22 @@ def quota_destroy_all_by_project(context, project_id): model_query(context, models.Quota, session=session, read_deleted="no").\ filter_by(project_id=project_id).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + soft_delete(synchronize_session=False) model_query(context, models.ProjectUserQuota, session=session, read_deleted="no").\ filter_by(project_id=project_id).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + soft_delete(synchronize_session=False) model_query(context, models.QuotaUsage, session=session, read_deleted="no").\ filter_by(project_id=project_id).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + soft_delete(synchronize_session=False) model_query(context, models.Reservation, session=session, read_deleted="no").\ filter_by(project_id=project_id).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + soft_delete(synchronize_session=False) @require_admin_context @@ -1101,10 +1073,7 @@ def reservation_expire(context): reservation.usage.reserved -= reservation.delta session.add(reservation.usage) - reservation_query.update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}, - synchronize_session=False) + reservation_query.soft_delete(synchronize_session=False) ################ @@ -1313,15 +1282,9 @@ def share_get_all_by_share_server(context, share_server_id, filters=None, def share_delete(context, share_id): session = get_session() share_ref = share_get(context, share_id, session) - share_ref.update({'deleted': True, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at'), - 'status': 'deleted'}) + share_ref.soft_delete(session=session, update_status=True) session.query(models.ShareMetadata).\ - filter_by(share_id=share_id).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}) + filter_by(share_id=share_id).soft_delete() session.query(models.ShareExportLocations).\ filter_by(share_id=share_id).soft_delete() @@ -1382,11 +1345,8 @@ def share_access_delete(context, access_id): session = get_session() with session.begin(): session.query(models.ShareAccessMapping).\ - filter_by(id=access_id).\ - update({'deleted': access_id, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at'), - 'state': models.ShareAccessMapping.STATE_DELETED}) + filter_by(id=access_id).soft_delete(update_status=True, + status_field_name='state') @require_context @@ -1438,11 +1398,7 @@ def share_snapshot_destroy(context, snapshot_id): session = get_session() with session.begin(): session.query(models.ShareSnapshot).\ - filter_by(id=snapshot_id).\ - update({'status': 'deleted', - 'deleted': snapshot_id, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}) + filter_by(id=snapshot_id).soft_delete(update_status=True) @require_context @@ -1578,10 +1534,7 @@ def share_metadata_get(context, share_id): @require_share_exists def share_metadata_delete(context, share_id, key): _share_metadata_get_query(context, share_id).\ - filter_by(key=key).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}) + filter_by(key=key).soft_delete() @require_context @@ -1625,8 +1578,7 @@ def _share_metadata_update(context, share_id, metadata, delete, session=None): meta_ref = _share_metadata_get_item(context, share_id, meta_key, session=session) - meta_ref.update({'deleted': 1}) - meta_ref.save(session=session) + meta_ref.soft_delete(session=session) meta_ref = None @@ -1978,10 +1930,7 @@ def share_server_delete(context, id): with session.begin(): server_ref = share_server_get(context, id, session=session) share_server_backend_details_delete(context, id, session=session) - server_ref.update({'deleted': server_ref.id, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at'), - 'status': constants.STATUS_DELETED}) + server_ref.soft_delete(session=session, update_status=True) @require_context @@ -2419,15 +2368,9 @@ def share_type_destroy(context, id): id) raise exception.ShareTypeInUse(share_type_id=id) model_query(context, models.ShareTypeExtraSpecs, session=session).\ - filter_by(share_type_id=id).update( - {'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}) + filter_by(share_type_id=id).soft_delete() model_query(context, models.ShareTypes, session=session).\ - filter_by(id=id).update( - {'deleted': id, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}) + filter_by(id=id).soft_delete() def _share_type_access_query(context, session=None): @@ -2519,10 +2462,7 @@ def share_type_extra_specs_delete(context, share_type_id, key): with session.begin(): _share_type_extra_specs_get_item(context, share_type_id, key, session) _share_type_extra_specs_query(context, share_type_id, session).\ - filter_by(key=key).\ - update({'deleted': 1, - 'deleted_at': timeutils.utcnow(), - 'updated_at': literal_column('updated_at')}) + filter_by(key=key).soft_delete() @require_context diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 8020789b..06d7c04c 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -47,6 +47,14 @@ class ManilaBase(models.ModelBase, model_dict[k] = v return model_dict + def soft_delete(self, session, update_status=False, + status_field_name='status'): + """Mark this object as deleted.""" + if update_status: + setattr(self, status_field_name, constants.STATUS_DELETED) + + return super(ManilaBase, self).soft_delete(session) + class Service(BASE, ManilaBase): """Represents a running service on a host.""" diff --git a/manila/db/sqlalchemy/query.py b/manila/db/sqlalchemy/query.py new file mode 100644 index 00000000..7362efa9 --- /dev/null +++ b/manila/db/sqlalchemy/query.py @@ -0,0 +1,40 @@ +# Copyright 2015 Mirantis Inc. +# All Rights Reserved. +# +# 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. + +from oslo_db.sqlalchemy import orm +import sqlalchemy + +from manila.common import constants + + +class Query(orm.Query): + def soft_delete(self, synchronize_session='evaluate', update_status=False, + status_field_name='status'): + if update_status: + setattr(self, status_field_name, constants.STATUS_DELETED) + + return super(Query, self).soft_delete(synchronize_session) + + +def get_maker(engine, autocommit=True, expire_on_commit=False): + """Return a SQLAlchemy sessionmaker using the given engine.""" + return sqlalchemy.orm.sessionmaker(bind=engine, + class_=orm.Session, + autocommit=autocommit, + expire_on_commit=expire_on_commit, + query_cls=Query) + +# NOTE(uglide): Monkey patch oslo_db get_maker() function to use custom Query +orm.get_maker = get_maker diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index 326d1f35..22158041 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -21,6 +21,7 @@ import six from manila import context from manila.db.sqlalchemy import api +from manila import exception from manila import test @@ -187,3 +188,16 @@ class SQLAlchemyAPIShareTestCase(test.TestCase): self.ctxt, test_host, test_id) self.assertEqual({}, actual_result) + + def test_custom_query(self): + share = api.share_create(self.ctxt, {'host': 'foobar'}) + test_access_values = { + 'share_id': share['id'], + 'access_type': 'ip', + 'access_to': 'fake', + } + share_access = api.share_access_create(self.ctxt, test_access_values) + + api.share_access_delete(self.ctxt, share_access.id) + self.assertRaises(exception.NotFound, api.share_access_get, + self.ctxt, share_access.id)