Use cached volumes in REST API extensions

This patch caches db volumes in the core volumes API so that they can be
used in the host and tenant attribute API extensions in order to lower
the response time by avoiding costly individual db queries for each
volume in the REST API contributions.

Change-Id: Ie0cbe929379df32fddbdce1953f13b6f26208bff
Partial-Bug: #1197612

diff --git a/cinder/api/contrib/volume_host_attribute.py b/cinder/api/contrib/volume_host_attribute.py
index 1d9e174d410875e063015110aaf1d624a969da83..18042b4690894f1a0d45377b7d6c6a520710624d 100644 (file)
--- a/cinder/api/contrib/volume_host_attribute.py
+++ b/cinder/api/contrib/volume_host_attribute.py
@@ -29,21 +29,18 @@ class VolumeHostAttributeController(wsgi.Controller):
         super(VolumeHostAttributeController, self).__init__(*args, **kwargs)
         self.volume_api = volume.API()
 
-    def _add_volume_host_attribute(self, context, resp_volume):
-        try:
-            db_volume = self.volume_api.get(context, resp_volume['id'])
-        except Exception:
-            return
-        else:
-            key = "%s:host" % Volume_host_attribute.alias
-            resp_volume[key] = db_volume['host']
+    def _add_volume_host_attribute(self, context, req, resp_volume):
+        db_volume = req.cached_resource_by_id(resp_volume['id'])
+        key = "%s:host" % Volume_host_attribute.alias
+        resp_volume[key] = db_volume['host']
 
     @wsgi.extends
     def show(self, req, resp_obj, id):
         context = req.environ['cinder.context']
         if authorize(context):
             resp_obj.attach(xml=VolumeHostAttributeTemplate())
-            self._add_volume_host_attribute(context, resp_obj.obj['volume'])
+            volume = resp_obj.obj['volume']
+            self._add_volume_host_attribute(context, req, volume)
 
     @wsgi.extends
     def detail(self, req, resp_obj):
@@ -51,7 +48,7 @@ class VolumeHostAttributeController(wsgi.Controller):
         if authorize(context):
             resp_obj.attach(xml=VolumeListHostAttributeTemplate())
             for volume in list(resp_obj.obj['volumes']):
-                self._add_volume_host_attribute(context, volume)
+                self._add_volume_host_attribute(context, req, volume)
 
 
 class Volume_host_attribute(extensions.ExtensionDescriptor):

diff --git a/cinder/api/contrib/volume_tenant_attribute.py b/cinder/api/contrib/volume_tenant_attribute.py
index 2660cdb726a2f66e701a6643241300b534850803..35b210ecb21c401d722d473c6531c164721a793e 100644 (file)
--- a/cinder/api/contrib/volume_tenant_attribute.py
+++ b/cinder/api/contrib/volume_tenant_attribute.py
@@ -27,21 +27,18 @@ class VolumeTenantAttributeController(wsgi.Controller):
         super(VolumeTenantAttributeController, self).__init__(*args, **kwargs)
         self.volume_api = volume.API()
 
-    def _add_volume_tenant_attribute(self, context, resp_volume):
-        try:
-            db_volume = self.volume_api.get(context, resp_volume['id'])
-        except Exception:
-            return
-        else:
-            key = "%s:tenant_id" % Volume_tenant_attribute.alias
-            resp_volume[key] = db_volume['project_id']
+    def _add_volume_tenant_attribute(self, context, req, resp_volume):
+        db_volume = req.cached_resource_by_id(resp_volume['id'])
+        key = "%s:tenant_id" % Volume_tenant_attribute.alias
+        resp_volume[key] = db_volume['project_id']
 
     @wsgi.extends
     def show(self, req, resp_obj, id):
         context = req.environ['cinder.context']
         if authorize(context):
             resp_obj.attach(xml=VolumeTenantAttributeTemplate())
-            self._add_volume_tenant_attribute(context, resp_obj.obj['volume'])
+            volume = resp_obj.obj['volume']
+            self._add_volume_tenant_attribute(context, req, volume)
 
     @wsgi.extends
     def detail(self, req, resp_obj):
@@ -49,7 +46,7 @@ class VolumeTenantAttributeController(wsgi.Controller):
         if authorize(context):
             resp_obj.attach(xml=VolumeListTenantAttributeTemplate())
             for volume in list(resp_obj.obj['volumes']):
-                self._add_volume_tenant_attribute(context, volume)
+                self._add_volume_tenant_attribute(context, req, volume)
 
 
 class Volume_tenant_attribute(extensions.ExtensionDescriptor):

diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py
index 9fe18f8f3dfc4f3d2bf9a278298535d5c21641c4..8364deb03fcec1c38296d176f0b403d729c60a26 100644 (file)
--- a/cinder/api/v1/volumes.py
+++ b/cinder/api/v1/volumes.py
@@ -275,6 +275,7 @@ class VolumeController(wsgi.Controller):
 
         try:
             vol = self.volume_api.get(context, id)
+            req.cache_resource(vol)
         except exception.NotFound:
             raise exc.HTTPNotFound()
 
@@ -330,6 +331,7 @@ class VolumeController(wsgi.Controller):
             self._add_visible_admin_metadata(context, volume)
 
         limited_list = common.limited(volumes, req)
+        req.cache_resource(limited_list)
         res = [entity_maker(context, vol) for vol in limited_list]
         return {'volumes': res}
 

diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py
index 5d9dd70f2b97ad7c8954c38f2bfbb8fad025fe66..18d201d788958cad725d93c273d38aa42bb14ac9 100644 (file)
--- a/cinder/api/v2/volumes.py
+++ b/cinder/api/v2/volumes.py
@@ -212,6 +212,7 @@ class VolumeController(wsgi.Controller):
 
         try:
             vol = self.volume_api.get(context, id)
+            req.cache_resource(vol)
         except exception.NotFound:
             msg = _("Volume could not be found")
             raise exc.HTTPNotFound(explanation=msg)
@@ -285,6 +286,7 @@ class VolumeController(wsgi.Controller):
             volumes = self._view_builder.detail_list(req, limited_list)
         else:
             volumes = self._view_builder.summary_list(req, limited_list)
+        req.cache_resource(limited_list)
         return volumes
 
     def _image_uuid_from_href(self, image_href):

diff --git a/cinder/tests/api/fakes.py b/cinder/tests/api/fakes.py
index 3451ec55b76c7c1a2979dd5aa524377ec6f13d56..08f494de6a15b4d97956facf276b543695245e80 100644 (file)
--- a/cinder/tests/api/fakes.py
+++ b/cinder/tests/api/fakes.py
@@ -137,7 +137,7 @@ class HTTPRequest(webob.Request):
     def blank(cls, *args, **kwargs):
         kwargs['base_url'] = 'http://localhost/v1'
         use_admin_context = kwargs.pop('use_admin_context', False)
-        out = webob.Request.blank(*args, **kwargs)
+        out = os_wsgi.Request.blank(*args, **kwargs)
         out.environ['cinder.context'] = FakeRequestContext(
             'fake_user',
             'fake',

diff --git a/cinder/tests/api/v1/test_volumes.py b/cinder/tests/api/v1/test_volumes.py
index 6a6d5aa1a5d290ebaef6c692addcfe88ed943b40..57c4d82b01b9d7a9c47c6f8a69b047753da98937 100644 (file)
--- a/cinder/tests/api/v1/test_volumes.py
+++ b/cinder/tests/api/v1/test_volumes.py
@@ -386,6 +386,8 @@ class VolumeApiTest(test.TestCase):
                                                                  1, 1, 1),
                                  'size': 1}]}
         self.assertEqual(res_dict, expected)
+        # Finally test that we cached the returned volumes
+        self.assertEqual(1, len(req.cached_resource()))
 
     def test_volume_list_with_admin_metadata(self):
         volume = stubs.stub_volume("1")
@@ -451,6 +453,8 @@ class VolumeApiTest(test.TestCase):
                                                                  1, 1, 1),
                                  'size': 1}]}
         self.assertEqual(res_dict, expected)
+        # Finally test that we cached the returned volumes
+        self.assertEqual(1, len(req.cached_resource()))
 
     def test_volume_list_detail_with_admin_metadata(self):
         volume = stubs.stub_volume("1")
@@ -638,6 +642,8 @@ class VolumeApiTest(test.TestCase):
                                                                1, 1, 1),
                                'size': 1}}
         self.assertEqual(res_dict, expected)
+        # Finally test that we cached the returned volume
+        self.assertIsNotNone(req.cached_resource_by_id('1'))
 
     def test_volume_show_no_attachments(self):
         def stub_volume_get(self, context, volume_id):
@@ -701,6 +707,8 @@ class VolumeApiTest(test.TestCase):
                           self.controller.show,
                           req,
                           1)
+        # Finally test that we did not cache anything
+        self.assertIsNone(req.cached_resource_by_id('1'))
 
     def test_volume_detail_limit_offset(self):
         def volume_detail_limit_offset(is_admin):

diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py
index f4a8fe4244ded4d81a7ff1b6cdaa1dc9991b30f8..8d2ba7b2f605209cf3145df3604e699b8b8c31a7 100644 (file)
--- a/cinder/tests/api/v2/test_volumes.py
+++ b/cinder/tests/api/v2/test_volumes.py
@@ -435,6 +435,8 @@ class VolumeApiTest(test.TestCase):
             ]
         }
         self.assertEqual(res_dict, expected)
+        # Finally test that we cached the returned volumes
+        self.assertEqual(1, len(req.cached_resource()))
 
     def test_volume_list_detail(self):
         self.stubs.Set(volume_api.API, 'get_all',
@@ -482,6 +484,8 @@ class VolumeApiTest(test.TestCase):
             ]
         }
         self.assertEqual(res_dict, expected)
+        # Finally test that we cached the returned volumes
+        self.assertEqual(1, len(req.cached_resource()))
 
     def test_volume_list_detail_with_admin_metadata(self):
         volume = stubs.stub_volume("1")
@@ -869,6 +873,8 @@ class VolumeApiTest(test.TestCase):
             }
         }
         self.assertEqual(res_dict, expected)
+        # Finally test that we cached the returned volume
+        self.assertIsNotNone(req.cached_resource_by_id('1'))
 
     def test_volume_show_no_attachments(self):
         def stub_volume_get(self, context, volume_id):
@@ -915,6 +921,8 @@ class VolumeApiTest(test.TestCase):
         req = fakes.HTTPRequest.blank('/v2/volumes/1')
         self.assertRaises(webob.exc.HTTPNotFound, self.controller.show,
                           req, 1)
+        # Finally test that nothing was cached
+        self.assertIsNone(req.cached_resource_by_id('1'))
 
     def test_volume_show_with_admin_metadata(self):
         volume = stubs.stub_volume("1")