From: Luis A. Garcia Date: Tue, 5 Nov 2013 20:41:02 +0000 (+0000) Subject: Use cached volumes in REST API extensions X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=9ed0f39fceaaa7a56e74055b54b7f6124e9e7f11;p=openstack-build%2Fcinder-build.git 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 1d9e174d4..18042b469 100644 --- 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 2660cdb72..35b210ecb 100644 --- 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 9fe18f8f3..8364deb03 100644 --- 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 5d9dd70f2..18d201d78 100644 --- 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 3451ec55b..08f494de6 100644 --- 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 6a6d5aa1a..57c4d82b0 100644 --- 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 f4a8fe424..8d2ba7b2f 100644 --- 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")