openstack/cinder
Code Issues Proposed changes

Switch get_all_snapshots to use objects

Browse Source 
The following patch switches direct db calls in
volume/api.py get_all_snapshots to use SnapshotList
instead.

Partial-Implements: blueprint cinder-objects
Change-Id: Ifdccc81a087f9797fcfbf098d3ecea3875ad7bd9
This commit is contained in:
Thang Pham 2015-03-03 22:10:52 -05:00 committed by rick.chen
parent 8ae4bdc8f3
commit b8a58fb01f
6 changed files with 62 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/api/v1/snapshots.py
View File

@ -148,7 +148,7 @@ class SnapshotsController(wsgi.Controller):
snapshots = self.volume_api.get_all_snapshots(context,
search_opts=search_opts)
limited_list = common.limited(snapshots, req)
limited_list = common.limited(snapshots.objects, req)
req.cache_db_snapshots(limited_list)
res = [entity_maker(context, snapshot) for snapshot in limited_list]
return {'snapshots': res}

2
cinder/api/v2/snapshots.py
View File

@ -156,7 +156,7 @@ class SnapshotsController(wsgi.Controller):
snapshots = self.volume_api.get_all_snapshots(context,
search_opts=search_opts)
limited_list = common.limited(snapshots, req)
limited_list = common.limited(snapshots.objects, req)
req.cache_db_snapshots(limited_list)
res = [entity_maker(context, snapshot) for snapshot in limited_list]
return {'snapshots': res}

34
cinder/tests/unit/api/v1/test_snapshots.py
View File

@ -23,6 +23,7 @@ from cinder.api.v1 import snapshots
from cinder import context
from cinder import db
from cinder import exception
from cinder import objects
from cinder import test
from cinder.tests.unit.api import fakes
from cinder.tests.unit.api.v1 import stubs
@ -277,10 +278,29 @@ class SnapshotApiTest(test.TestCase):
req,
snapshot_id)
def test_snapshot_detail(self):
self.stubs.Set(volume.api.API,
"get_all_snapshots",
stub_snapshot_get_all)
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
@mock.patch('cinder.objects.Volume.get_by_id')
@mock.patch('cinder.objects.Snapshot.get_by_id')
@mock.patch('cinder.volume.api.API.get_all_snapshots')
def test_snapshot_detail(self, get_all_snapshots, snapshot_get_by_id,
volume_get_by_id, snapshot_metadata_get):
snapshot = {
'id': UUID,
'volume_id': 1,
'status': 'available',
'volume_size': 100,
'display_name': 'Default name',
'display_description': 'Default description',
'expected_attrs': ['metadata']
}
ctx = context.RequestContext('admin', 'fake', True)
snapshot_obj = fake_snapshot.fake_snapshot_obj(ctx, **snapshot)
fake_volume_obj = fake_volume.fake_volume_obj(ctx)
snapshot_get_by_id.return_value = snapshot_obj
volume_get_by_id.return_value = fake_volume_obj
snapshots = objects.SnapshotList(objects=[snapshot_obj])
get_all_snapshots.return_value = snapshots
req = fakes.HTTPRequest.blank('/v1/snapshots/detail')
resp_dict = self.controller.detail(req)
@ -390,7 +410,9 @@ class SnapshotApiTest(test.TestCase):
self.assertIn('snapshots', res)
self.assertEqual(1, len(res['snapshots']))
def test_list_snapshots_with_limit_and_offset(self):
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
def test_list_snapshots_with_limit_and_offset(self,
snapshot_metadata_get):
def list_snapshots_with_limit_and_offset(is_admin):
def stub_snapshot_get_all_by_project(context, project_id):
return [
@ -409,7 +431,7 @@ class SnapshotApiTest(test.TestCase):
self.assertIn('snapshots', res)
self.assertEqual(1, len(res['snapshots']))
self.assertEqual(2, res['snapshots'][0]['id'])
self.assertEqual('2', res['snapshots'][0]['id'])
# admin case
list_snapshots_with_limit_and_offset(is_admin=True)

33
cinder/tests/unit/api/v2/test_snapshots.py
View File

@ -23,6 +23,7 @@ from cinder.api.v2 import snapshots
from cinder import context
from cinder import db
from cinder import exception
from cinder import objects
from cinder import test
from cinder.tests.unit.api import fakes
from cinder.tests.unit.api.v2 import stubs
@ -289,9 +290,29 @@ class SnapshotApiTest(test.TestCase):
self.assertRaises(webob.exc.HTTPNotFound,
self.controller.show, req, snapshot_id)
def test_snapshot_detail(self):
self.stubs.Set(volume.api.API, "get_all_snapshots",
stub_snapshot_get_all)
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
@mock.patch('cinder.objects.Volume.get_by_id')
@mock.patch('cinder.objects.Snapshot.get_by_id')
@mock.patch('cinder.volume.api.API.get_all_snapshots')
def test_snapshot_detail(self, get_all_snapshots, snapshot_get_by_id,
volume_get_by_id, snapshot_metadata_get):
snapshot = {
'id': UUID,
'volume_id': 1,
'status': 'available',
'volume_size': 100,
'display_name': 'Default name',
'display_description': 'Default description',
'expected_attrs': ['metadata']
}
ctx = context.RequestContext('admin', 'fake', True)
snapshot_obj = fake_snapshot.fake_snapshot_obj(ctx, **snapshot)
fake_volume_obj = fake_volume.fake_volume_obj(ctx)
snapshot_get_by_id.return_value = snapshot_obj
volume_get_by_id.return_value = fake_volume_obj
snapshots = objects.SnapshotList(objects=[snapshot_obj])
get_all_snapshots.return_value = snapshots
req = fakes.HTTPRequest.blank('/v2/snapshots/detail')
resp_dict = self.controller.detail(req)
@ -401,7 +422,9 @@ class SnapshotApiTest(test.TestCase):
self.assertIn('snapshots', res)
self.assertEqual(1, len(res['snapshots']))
def test_list_snapshots_with_limit_and_offset(self):
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
def test_list_snapshots_with_limit_and_offset(self,
snapshot_metadata_get):
def list_snapshots_with_limit_and_offset(is_admin):
def stub_snapshot_get_all_by_project(context, project_id):
return [
@ -420,7 +443,7 @@ class SnapshotApiTest(test.TestCase):
self.assertIn('snapshots', res)
self.assertEqual(1, len(res['snapshots']))
self.assertEqual(2, res['snapshots'][0]['id'])
self.assertEqual('2', res['snapshots'][0]['id'])
# admin case
list_snapshots_with_limit_and_offset(is_admin=True)

6
cinder/volume/api.py
View File

@ -510,9 +510,9 @@ class API(base.Base):
if (context.is_admin and 'all_tenants' in search_opts):
# Need to remove all_tenants to pass the filtering below.
del search_opts['all_tenants']
snapshots = self.db.snapshot_get_all(context)
snapshots = objects.SnapshotList.get_all(context)
else:
snapshots = self.db.snapshot_get_all_by_project(
snapshots = objects.SnapshotList.get_all_by_project(
context, context.project_id)
if search_opts:
@ -526,7 +526,7 @@ class API(base.Base):
break
else:
results.append(snapshot)
snapshots = results
snapshots.objects = results
LOG.info(_LI("Get all snaphsots completed successfully."))
return snapshots

1
tools/lintstack.py
View File

@ -63,6 +63,7 @@ objects_ignore_codes = ["E0213", "E1101", "E1102"]
objects_ignore_messages = [
"No value passed for parameter 'id' in function call",
"Module 'cinder.objects' has no 'Snapshot' member",
"Module 'cinder.objects' has no 'SnapshotList' member",
]
objects_ignore_modules = ["cinder/objects/"]