openstack
/
manila
Code Issues Proposed changes

db: Rename some methods 

Rename a number of APIs for consistency purposes:

- get_all_expired_transfers ->
    transfer_get_all_expired
- get_all_expired_shares ->
    share_get_all_expired
- get_shares_in_recycle_bin_by_share_server ->
    share_get_all_soft_deleted
- get_shares_in_recycle_bin_by_network ->
    share_get_all_soft_deleted_by_network
- share_transfer_get ->
    transfer_get

Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Change-Id: I805ed06feefd189768cf4b0fec9b8ff011694216
This commit is contained in:
Stephen Finucane 2023-06-01 10:13:49 +01:00
parent 8ed1c72d57
commit 2a3bbe9be8
8 changed files with 56 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
manila/db/api.py
View File

@ -454,11 +454,10 @@ def share_get_all_by_share_server(context, share_server_id, filters=None,
sort_dir=sort_dir)
def get_shares_in_recycle_bin_by_share_server(
context, share_server_id, filters=None,
sort_key=None, sort_dir=None):
def share_get_all_soft_deleted(
context, share_server_id, filters=None, sort_key=None, sort_dir=None):
"""Returns all shares in recycle bin with given share server ID."""
return IMPL.get_shares_in_recycle_bin_by_share_server(
return IMPL.share_get_all_soft_deleted(
context, share_server_id, filters=filters, sort_key=sort_key,
sort_dir=sort_dir)
@ -471,11 +470,10 @@ def share_get_all_by_share_server_with_count(
sort_dir=sort_dir)
def get_shares_in_recycle_bin_by_network(
context, share_network_id, filters=None,
sort_key=None, sort_dir=None):
def share_get_all_soft_deleted_by_network(
context, share_network_id, filters=None, sort_key=None, sort_dir=None):
"""Returns all shares in recycle bin with given share network ID."""
return IMPL.get_shares_in_recycle_bin_by_network(
return IMPL.share_get_all_soft_deleted_by_network(
context, share_network_id, filters=filters, sort_key=sort_key,
sort_dir=sort_dir)
@ -497,9 +495,9 @@ def share_restore(context, share_id):
###################
def share_transfer_get(context, transfer_id):
def transfer_get(context, transfer_id):
"""Get a share transfer record or raise if it does not exist."""
return IMPL.share_transfer_get(context, transfer_id)
return IMPL.transfer_get(context, transfer_id)
def transfer_get_all(context, limit=None, sort_key=None,
@ -520,6 +518,11 @@ def transfer_get_all_by_project(context, project_id,
filters=filters, offset=offset)
def transfer_get_all_expired(context):
"""Get all expired transfers DB records."""
return IMPL.transfer_get_all_expired(context)
def transfer_create(context, values):
"""Create an entry in the transfers table."""
return IMPL.transfer_create(context, values)
@ -1292,14 +1295,9 @@ def share_server_get_all_unused_deletable(context, host, updated_before):
updated_before)
def get_all_expired_shares(context):
def share_get_all_expired(context):
"""Get all expired share DB records."""
return IMPL.get_all_expired_shares(context)
def get_all_expired_transfers(context):
"""Get all expired transfers DB records."""
return IMPL.get_all_expired_transfers(context)
return IMPL.share_get_all_expired(context)
def share_server_backend_details_set(context, share_server_id, server_details):

39
manila/db/sqlalchemy/api.py
View File

@ -2375,7 +2375,7 @@ def _share_get_all_with_filters(context, project_id=None, share_server_id=None,
@require_admin_context
def get_all_expired_shares(context):
def share_get_all_expired(context):
query = (
_share_get_query(context).join(
models.ShareInstance,
@ -2468,7 +2468,7 @@ def share_get_all_by_share_server(context, share_server_id, filters=None,
@require_context
def get_shares_in_recycle_bin_by_share_server(
def share_get_all_soft_deleted(
context, share_server_id, filters=None, sort_key=None, sort_dir=None):
"""Returns list of shares in recycle bin with given share server."""
if filters is None:
@ -2491,7 +2491,7 @@ def share_get_all_by_share_server_with_count(
@require_context
def get_shares_in_recycle_bin_by_network(
def share_get_all_soft_deleted_by_network(
context, share_network_id, filters=None, sort_key=None, sort_dir=None):
"""Returns list of shares in recycle bin with given share network."""
if filters is None:
@ -2580,7 +2580,7 @@ def _transfer_get(context, transfer_id, resource_type='share',
@context_manager.reader
def share_transfer_get(context, transfer_id, read_deleted=False):
def transfer_get(context, transfer_id, read_deleted=False):
return _transfer_get(context, transfer_id, read_deleted=read_deleted)
@ -2624,6 +2624,19 @@ def transfer_get_all_by_project(context, project_id,
filters=filters, offset=offset)
@require_admin_context
def transfer_get_all_expired(context):
session = get_session()
with session.begin():
query = model_query(context, models.Transfer, session=session)
expires_at_attr = getattr(models.Transfer, 'expires_at', None)
now_time = timeutils.utcnow()
query = query.filter(expires_at_attr.op('<=')(now_time))
result = query.all()
return result
@require_context
@handle_db_data_error
def transfer_create(context, values):
@ -2655,7 +2668,7 @@ def transfer_destroy(context, transfer_id,
session = get_session()
with session.begin():
update = {'status': constants.STATUS_AVAILABLE}
transfer = share_transfer_get(context, transfer_id)
transfer = transfer_get(context, transfer_id)
if transfer['resource_type'] == 'share':
if update_share_status:
share_update(context, transfer['resource_id'], update)
@ -2671,7 +2684,7 @@ def transfer_accept(context, transfer_id, user_id, project_id,
accept_snapshots=False):
session = get_session()
with session.begin():
share_id = share_transfer_get(context, transfer_id)['resource_id']
share_id = transfer_get(context, transfer_id)['resource_id']
update = {'status': constants.STATUS_AVAILABLE,
'user_id': user_id,
'project_id': project_id,
@ -2700,7 +2713,7 @@ def transfer_accept_rollback(context, transfer_id, user_id,
project_id, rollback_snap=False):
session = get_session()
with session.begin():
share_id = share_transfer_get(
share_id = transfer_get(
context, transfer_id, read_deleted=True)['resource_id']
update = {'status': constants.STATUS_AWAITING_TRANSFER,
'user_id': user_id,
@ -2726,18 +2739,6 @@ def transfer_accept_rollback(context, transfer_id, user_id,
'accepted': 0})
@require_admin_context
def get_all_expired_transfers(context):
session = get_session()
with session.begin():
query = model_query(context, models.Transfer, session=session)
expires_at_attr = getattr(models.Transfer, 'expires_at', None)
now_time = timeutils.utcnow()
query = query.filter(expires_at_attr.op('<=')(now_time))
result = query.all()
return result
###################

7
manila/share/api.py
View File

@ -2797,9 +2797,8 @@ class API(base.Base):
shares = self.db.share_get_all_by_share_server(
context, share_server['id'])
shares_in_recycle_bin = (
self.db.get_shares_in_recycle_bin_by_share_server(
context, share_server['id']))
shares_in_recycle_bin = self.db.share_get_all_soft_deleted(
context, share_server['id'])
if len(shares) == 0:
msg = _("Share server %s does not have shares."
@ -3273,7 +3272,7 @@ class API(base.Base):
utils.validate_service_host(admin_ctx, backend_host)
shares_in_recycle_bin = (
self.db.get_shares_in_recycle_bin_by_network(
self.db.share_get_all_soft_deleted_by_network(
context, share_network['id']))
if shares_in_recycle_bin:
msg = _("Some shares with share network %(sn_id)s have "

4
manila/share/manager.py
View File

@ -3611,7 +3611,7 @@ class ShareManager(manager.SchedulerDependentManager):
@utils.require_driver_initialized
def delete_expired_share(self, ctxt):
LOG.debug("Check for expired share in recycle bin to delete.")
expired_shares = self.db.get_all_expired_shares(ctxt)
expired_shares = self.db.share_get_all_expired(ctxt)
for share in expired_shares:
if share['status'] == constants.STATUS_ERROR_DELETING:
@ -3629,7 +3629,7 @@ class ShareManager(manager.SchedulerDependentManager):
spacing=CONF.check_for_expired_transfers)
def delete_expired_transfers(self, ctxt):
LOG.info("Checking for expired transfers.")
expired_transfers = self.db.get_all_expired_transfers(ctxt)
expired_transfers = self.db.transfer_get_all_expired(ctxt)
for transfer in expired_transfers:
LOG.debug("Transfer %s has expired, will be destroyed.",

16
manila/tests/db/sqlalchemy/test_api.py
View File

@ -428,7 +428,7 @@ class ShareDatabaseAPITestCase(test.TestCase):
share_network_id=share_network['id'],
is_soft_deleted=True)
actual_result = db_api.get_shares_in_recycle_bin_by_share_server(
actual_result = db_api.share_get_all_soft_deleted(
self.ctxt, share_server['id'])
self.assertEqual(1, len(actual_result))
@ -441,7 +441,7 @@ class ShareDatabaseAPITestCase(test.TestCase):
share_network_id=share_network['id'],
is_soft_deleted=True)
actual_result = db_api.get_shares_in_recycle_bin_by_network(
actual_result = db_api.share_get_all_soft_deleted_by_network(
self.ctxt, share_network['id'])
self.assertEqual(1, len(actual_result))
@ -755,7 +755,7 @@ class ShareDatabaseAPITestCase(test.TestCase):
is_soft_deleted=True,
scheduled_to_be_deleted_at=time2)
shares = [share1, share2, share3]
result = db_api.get_all_expired_shares(self.ctxt)
result = db_api.share_get_all_expired(self.ctxt)
self.assertEqual(1, len(result))
self.assertEqual(shares[2]['id'], result[0]['id'])
@ -5289,20 +5289,20 @@ class TransfersTestCase(test.TestCase):
share_id = share['id']
self._create_transfer(resource_id=share_id)
def test_share_transfer_get(self):
def test_transfer_get(self):
share_id = db_utils.create_share(size=1, user_id=self.user_id,
project_id=self.project_id)['id']
transfer_id = self._create_transfer(resource_id=share_id)
transfer = db_api.share_transfer_get(self.ctxt, transfer_id)
transfer = db_api.transfer_get(self.ctxt, transfer_id)
self.assertEqual(share_id, transfer['resource_id'])
new_ctxt = context.RequestContext(user_id='new_user_id',
project_id='new_project_id')
self.assertRaises(exception.TransferNotFound,
db_api.share_transfer_get, new_ctxt, transfer_id)
db_api.transfer_get, new_ctxt, transfer_id)
transfer = db_api.share_transfer_get(new_ctxt.elevated(), transfer_id)
transfer = db_api.transfer_get(new_ctxt.elevated(), transfer_id)
self.assertEqual(share_id, transfer['resource_id'])
def test_transfer_get_all(self):
@ -5360,7 +5360,7 @@ class TransfersTestCase(test.TestCase):
new_ctxt = context.RequestContext(user_id='new_user_id',
project_id='new_project_id')
transfer = db_api.share_transfer_get(new_ctxt.elevated(), transfer_id)
transfer = db_api.transfer_get(new_ctxt.elevated(), transfer_id)
self.assertEqual(share['project_id'], transfer['source_project_id'])
self.assertFalse(transfer['accepted'])
self.assertIsNone(transfer['destination_project_id'])

2
manila/tests/share/test_api.py
View File

@ -4899,7 +4899,7 @@ class ShareAPITestCase(test.TestCase):
db_api, 'share_get_all_by_share_server',
mock.Mock(return_value=[fake_share]))
mock_shares_in_recycle_bin_get_all = self.mock_object(
db_api, 'get_shares_in_recycle_bin_by_share_server',
db_api, 'share_get_all_soft_deleted',
mock.Mock(return_value=[]))
mock_get_type = self.mock_object(
share_types, 'get_share_type', mock.Mock(return_value=share_type))

8
manila/tests/share/test_manager.py
View File

@ -4215,13 +4215,13 @@ class ShareManagerTestCase(test.TestCase):
@ddt.data("available", "error_deleting")
def test_delete_expired_share(self, share_status):
self.mock_object(db, 'get_all_expired_shares',
self.mock_object(db, 'share_get_all_expired',
mock.Mock(return_value=[{"id": "share1",
"status": share_status}, ]))
self.mock_object(db, 'share_update')
self.mock_object(api.API, 'delete')
self.share_manager.delete_expired_share(self.context)
db.get_all_expired_shares.assert_called_once_with(self.context)
db.share_get_all_expired.assert_called_once_with(self.context)
share1 = {"id": "share1", "status": share_status}
if share1["status"] == "error_deleting":
db.share_update.assert_called_once_with(
@ -4230,12 +4230,12 @@ class ShareManagerTestCase(test.TestCase):
self.context, share1)
def test_delete_expired_transfers(self):
self.mock_object(db, 'get_all_expired_transfers',
self.mock_object(db, 'transfer_get_all_expired',
mock.Mock(return_value=[{"id": "transfer1",
"name": "test_tr"}, ]))
self.mock_object(transfer_api.API, 'delete')
self.share_manager.delete_expired_transfers(self.context)
db.get_all_expired_transfers.assert_called_once_with(self.context)
db.transfer_get_all_expired.assert_called_once_with(self.context)
transfer1 = {"id": "transfer1", "name": "test_tr"}
transfer_api.API.delete.assert_called_once_with(
self.context, transfer_id=transfer1["id"])

6
manila/transfer/api.py
View File

@ -66,12 +66,12 @@ class API(base.Base):
super().__init__()
def get(self, context, transfer_id):
transfer = self.db.share_transfer_get(context, transfer_id)
transfer = self.db.transfer_get(context, transfer_id)
return transfer
def delete(self, context, transfer_id):
"""Delete a share transfer."""
transfer = self.db.share_transfer_get(context, transfer_id)
transfer = self.db.transfer_get(context, transfer_id)
policy.check_policy(context, 'share_transfer', 'delete', target_obj={
'project_id': transfer['source_project_id']})
update_share_status = True
@ -331,7 +331,7 @@ class API(base.Base):
"""Accept a share that has been offered for transfer."""
# We must use an elevated context to make sure we can find the
# transfer.
transfer = self.db.share_transfer_get(context.elevated(), transfer_id)
transfer = self.db.transfer_get(context.elevated(), transfer_id)
crypt_hash = self._get_crypt_hash(transfer['salt'], auth_key)
if crypt_hash != transfer['crypt_hash']: