From fa1a02d8116098b5143acaae6feac731c49501d0 Mon Sep 17 00:00:00 2001
From: Radomir Dopieralski <openstack@sheep.art.pl>
Date: Tue, 18 Nov 2014 11:49:21 +0100
Subject: [PATCH] Optimize heat.Resource.get_by_node

Use memoization and a dict to cache the node-resource relation.
Also use LookupError instead of http.NotFound, so that we get proper
tracebacks.

Change-Id: Ic71c2fecaaca07ba070d896070efe8b094df8a78
---
 tuskar_ui/api/heat.py                   | 28 +++++++++++--------------
 tuskar_ui/infrastructure/nodes/tabs.py  | 14 ++++++-------
 tuskar_ui/infrastructure/nodes/tests.py |  4 ++--
 tuskar_ui/infrastructure/roles/views.py |  5 +++--
 4 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/tuskar_ui/api/heat.py b/tuskar_ui/api/heat.py
index 96c0bde48..c8ab6466d 100644
--- a/tuskar_ui/api/heat.py
+++ b/tuskar_ui/api/heat.py
@@ -16,7 +16,6 @@ import urlparse
 from django.conf import settings
 from django.utils.translation import ugettext_lazy as _
 import heatclient
-from horizon import exceptions
 from horizon.utils import memoized
 from openstack_dashboard.api import base
 from openstack_dashboard.api import heat
@@ -398,7 +397,15 @@ class Resource(base.APIResourceWrapper):
             self._role = kwargs['role']
 
     @classmethod
-    def get_by_node(cls, request, node, all_resources=None):
+    @memoized.memoized
+    def _resources_by_nodes(cls, request):
+        return dict(
+            (resource.physical_resource_id, resource)
+            for resource in cls.list_all_resources(request)
+        )
+
+    @classmethod
+    def get_by_node(cls, request, node):
         """Return the specified Heat Resource given a Node
 
         :param request: request object
@@ -407,22 +414,11 @@ class Resource(base.APIResourceWrapper):
         :param node: node to match
         :type  node: tuskar_ui.api.node.Node
 
-        :return: matching Resource, or None if no Resource matches
-                 the Node
+        :return: matching Resource, or raises LookupError if no
+                 resource matches the node
         :rtype:  tuskar_ui.api.heat.Resource
         """
-        # TODO(tzumainn): this is terribly inefficient, but I don't see a
-        # better way.  Maybe if Heat set some node metadata. . . ?
-        if node.instance_uuid:
-            if all_resources is None:
-                resource_list = cls.list_all_resources(request)
-            else:
-                resource_list = all_resources
-            for resource in resource_list:
-                if resource.physical_resource_id == node.instance_uuid:
-                    return resource
-        msg = _('Could not find resource matching node "%s"') % node.uuid
-        raise exceptions.NotFound(msg)
+        return cls._resources_by_nodes(request)[node.instance_uuid]
 
     @classmethod
     def list_all_resources(cls, request):
diff --git a/tuskar_ui/infrastructure/nodes/tabs.py b/tuskar_ui/infrastructure/nodes/tabs.py
index e549b4c78..0feb61d06 100644
--- a/tuskar_ui/infrastructure/nodes/tabs.py
+++ b/tuskar_ui/infrastructure/nodes/tabs.py
@@ -16,7 +16,6 @@ import itertools
 
 from django.core import urlresolvers
 from django.utils.translation import ugettext_lazy as _
-from horizon import exceptions
 from horizon import tabs
 from horizon.utils import functions
 from openstack_dashboard.api import base as api_base
@@ -242,16 +241,16 @@ class ProvisionedTab(BaseTab):
         nodes, prev, more = self._nodes_info
 
         if nodes:
-            all_resources = api.heat.Resource.list_all_resources(self.request)
             for node in nodes:
                 try:
                     resource = api.heat.Resource.get_by_node(
-                        self.request, node, all_resources=all_resources)
+                        self.request, node)
+                except LookupError:
+                    node.role_name = '-'
+                else:
                     node.role_name = resource.role.name
                     node.role_id = resource.role.id
                     node.stack_id = resource.stack.id
-                except exceptions.NotFound:
-                    node.role_name = '-'
 
         return nodes
 
@@ -302,10 +301,11 @@ class DetailOverviewTab(tabs.Tab):
         context = {'node': node}
         try:
             resource = api.heat.Resource.get_by_node(self.request, node)
+        except LookupError:
+            pass
+        else:
             context['role'] = resource.role
             context['stack'] = resource.stack
-        except exceptions.NotFound:
-            pass
         if node.instance_uuid:
             if api_base.is_service_enabled(self.request, 'metering'):
                 # Meter configuration in the following format:
diff --git a/tuskar_ui/infrastructure/nodes/tests.py b/tuskar_ui/infrastructure/nodes/tests.py
index 8d2f757bb..081e83907 100644
--- a/tuskar_ui/infrastructure/nodes/tests.py
+++ b/tuskar_ui/infrastructure/nodes/tests.py
@@ -250,8 +250,8 @@ class NodesTests(test.BaseAdminViewTests, helpers.APITestCase):
             }),
             patch('tuskar_ui.api.heat.Resource', **{
                 'spec_set': ['get_by_node'],
-                'get_by_node.side_effect': (
-                    self._raise_horizon_exception_not_found),
+                'get_by_node.side_effect': lambda *args, **kwargs: {}[None],
+                # Raises LookupError
             }),
         ) as (mock_node, mock_heat):
             res = self.client.get(
diff --git a/tuskar_ui/infrastructure/roles/views.py b/tuskar_ui/infrastructure/roles/views.py
index a19aa72d9..19fe4b4bd 100644
--- a/tuskar_ui/infrastructure/roles/views.py
+++ b/tuskar_ui/infrastructure/roles/views.py
@@ -92,11 +92,12 @@ class DetailView(horizon_tables.DataTableView, RoleMixin, StackMixin):
             # by getting the resource for all nodes at once
             try:
                 resource = api.heat.Resource.get_by_node(self.request, node)
+            except LookupError:
+                node.role_name = '-'
+            else:
                 node.role_name = resource.role.name
                 node.role_id = resource.role.id
                 node.stack_id = resource.stack.id
-            except horizon_exceptions.NotFound:
-                node.role_name = '-'
 
         return nodes