From: Thang Pham Date: Wed, 4 Mar 2015 03:10:52 +0000 (-0500) Subject: Switch get_all_snapshots to use objects X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=b8a58fb01f25428d367d35d6025d9046abbfe8fc;p=openstack-build%2Fcinder-build.git Switch get_all_snapshots to use objects 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 --- diff --git a/cinder/api/v1/snapshots.py b/cinder/api/v1/snapshots.py index 0998e0f02..0667231ff 100644 --- a/cinder/api/v1/snapshots.py +++ b/cinder/api/v1/snapshots.py @@ -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} diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py index 159a05c19..d3976629d 100644 --- a/cinder/api/v2/snapshots.py +++ b/cinder/api/v2/snapshots.py @@ -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} diff --git a/cinder/tests/unit/api/v1/test_snapshots.py b/cinder/tests/unit/api/v1/test_snapshots.py index 6e2984644..3d498412d 100644 --- a/cinder/tests/unit/api/v1/test_snapshots.py +++ b/cinder/tests/unit/api/v1/test_snapshots.py @@ -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) diff --git a/cinder/tests/unit/api/v2/test_snapshots.py b/cinder/tests/unit/api/v2/test_snapshots.py index b10d1ca88..82ff94f05 100644 --- a/cinder/tests/unit/api/v2/test_snapshots.py +++ b/cinder/tests/unit/api/v2/test_snapshots.py @@ -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) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 32ffd6b26..c4b470c24 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -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 diff --git a/tools/lintstack.py b/tools/lintstack.py index d1994fc73..05b5a98b5 100755 --- a/tools/lintstack.py +++ b/tools/lintstack.py @@ -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/"]