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 0998e0f0250648304ecbd4d08c7a4427b5289f65..0667231fff49b5f91a5b064b7ed067becca11c9d 100644 (file)
--- 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 159a05c1960d10d07a2aeff1050cb1f9e6e6519b..d3976629d72299a9ee5b741096edc71e2d977143 100644 (file)
--- 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 6e2984644fabc3ba72c7d2f462f48d88fdbd1deb..3d498412d5aea4d2e50d37a62ccc752572bc1b76 100644 (file)
--- 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 b10d1ca88300b8570a24d1d724e74ac78108b255..82ff94f05d8f4a276d2ea8ec77af56a2028fc20d 100644 (file)
--- 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 32ffd6b26513654551df5e4b1effc3605b03983d..c4b470c244d7789b283b6e1da6c09b73811755b6 100644 (file)
--- 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 d1994fc73bcb63424eb30fd2570acac1c616e1c0..05b5a98b57f7231d46ac53cc79d7d3a295ba1a24 100755 (executable)
--- 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/"]