From: Vishvananda Ishaya <vishvananda@gmail.com>
Date: Tue, 19 Aug 2014 23:59:02 +0000 (-0700)
Subject: Cache snapshots in request for extension
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4f0d1d36136bcb33f578b2999812f36cadd71cec;p=openstack-build%2Fcinder-build.git

Cache snapshots in request for extension

The extended snapshots extension makes a separate db request for
snapshots which is horribly inefficent. In addition it causes an
issue when using all_tenants=1, because the second request doesn't
pass in the filter. Fix this by caching the snapshot in the request
object like we do for volumes and retrieve the cached object in the
extension.

Change-Id: Ia943438c8d48accb7f62c43f07642c3da2fe34ab
Resolves-bug: #1358960
---

diff --git a/cinder/api/contrib/extended_snapshot_attributes.py b/cinder/api/contrib/extended_snapshot_attributes.py
index 34e0151c5..56cc24367 100644
--- a/cinder/api/contrib/extended_snapshot_attributes.py
+++ b/cinder/api/contrib/extended_snapshot_attributes.py
@@ -15,13 +15,9 @@
 """The Extended Snapshot Attributes API extension."""
 
 
-from webob import exc
-
 from cinder.api import extensions
 from cinder.api.openstack import wsgi
 from cinder.api import xmlutil
-from cinder import exception
-from cinder.i18n import _
 from cinder.openstack.common import log as logging
 from cinder import volume
 
@@ -43,10 +39,11 @@ class ExtendedSnapshotAttributesController(wsgi.Controller):
         rval = dict((snapshot['id'], snapshot) for snapshot in snapshots)
         return rval
 
-    def _extend_snapshot(self, snapshot, data):
+    def _extend_snapshot(self, req, resp_snap):
+        db_snap = req.cached_resource_by_id(resp_snap['id'])
         for attr in ['project_id', 'progress']:
             key = "%s:%s" % (Extended_snapshot_attributes.alias, attr)
-            snapshot[key] = data[attr]
+            resp_snap[key] = db_snap[attr]
 
     @wsgi.extends
     def show(self, req, resp_obj, id):
@@ -54,15 +51,8 @@ class ExtendedSnapshotAttributesController(wsgi.Controller):
         if authorize(context):
             # Attach our slave template to the response object
             resp_obj.attach(xml=ExtendedSnapshotAttributeTemplate())
-
-            try:
-                snapshot = self.volume_api.get_snapshot(context, id)
-            except exception.NotFound:
-                explanation = _("Snapshot not found.")
-                raise exc.HTTPNotFound(explanation=explanation)
-
-            self._extend_snapshot(snapshot=resp_obj.obj['snapshot'],
-                                  data=snapshot)
+            snapshot = resp_obj.obj['snapshot']
+            self._extend_snapshot(req, snapshot)
 
     @wsgi.extends
     def detail(self, req, resp_obj):
@@ -70,18 +60,8 @@ class ExtendedSnapshotAttributesController(wsgi.Controller):
         if authorize(context):
             # Attach our slave template to the response object
             resp_obj.attach(xml=ExtendedSnapshotAttributesTemplate())
-
-            snapshots = list(resp_obj.obj.get('snapshots', []))
-            db_snapshots = self._get_snapshots(context)
-
-            for snapshot_object in snapshots:
-                try:
-                    snapshot_data = db_snapshots[snapshot_object['id']]
-                except KeyError:
-                    continue
-
-                self._extend_snapshot(snapshot=snapshot_object,
-                                      data=snapshot_data)
+            for snapshot in list(resp_obj.obj['snapshots']):
+                self._extend_snapshot(req, snapshot)
 
 
 class Extended_snapshot_attributes(extensions.ExtensionDescriptor):
diff --git a/cinder/api/v1/snapshots.py b/cinder/api/v1/snapshots.py
index 7960230f6..7b944afc7 100644
--- a/cinder/api/v1/snapshots.py
+++ b/cinder/api/v1/snapshots.py
@@ -107,6 +107,7 @@ class SnapshotsController(wsgi.Controller):
 
         try:
             vol = self.volume_api.get_snapshot(context, id)
+            req.cache_resource(vol)
         except exception.NotFound:
             raise exc.HTTPNotFound()
 
@@ -152,6 +153,7 @@ class SnapshotsController(wsgi.Controller):
         snapshots = self.volume_api.get_all_snapshots(context,
                                                       search_opts=search_opts)
         limited_list = common.limited(snapshots, req)
+        req.cache_resource(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 68c72f685..efe2c780c 100644
--- a/cinder/api/v2/snapshots.py
+++ b/cinder/api/v2/snapshots.py
@@ -107,6 +107,7 @@ class SnapshotsController(wsgi.Controller):
 
         try:
             vol = self.volume_api.get_snapshot(context, id)
+            req.cache_resource(vol)
         except exception.NotFound:
             msg = _("Snapshot could not be found")
             raise exc.HTTPNotFound(explanation=msg)
@@ -160,6 +161,7 @@ class SnapshotsController(wsgi.Controller):
         snapshots = self.volume_api.get_all_snapshots(context,
                                                       search_opts=search_opts)
         limited_list = common.limited(snapshots, req)
+        req.cache_resource(limited_list)
         res = [entity_maker(context, snapshot) for snapshot in limited_list]
         return {'snapshots': res}
 
diff --git a/cinder/tests/api/test_router.py b/cinder/tests/api/test_router.py
index ddc286505..48747a86f 100644
--- a/cinder/tests/api/test_router.py
+++ b/cinder/tests/api/test_router.py
@@ -32,10 +32,12 @@ class FakeController(object):
         self.ext_mgr = ext_mgr
 
     def index(self, req):
-        return {}
+        obj_type = req.path.split("/")[3]
+        return {obj_type: []}
 
     def detail(self, req):
-        return {}
+        obj_type = req.path.split("/")[3]
+        return {obj_type: []}
 
 
 def create_resource(ext_mgr):