Stop memoizing on request arguments in nova api

The request parameter is a django.http.HttpRequest instance object
which contains metadata about the request. This object is a new instance of
django.http.HttpRequest in every view, so memoizing with it memoizes during the
rendering of a single view only, thus preventing multiple api calls only
during the rendering of one view.

Some nova api memoized methods should memoize more persistantly with objects
which do not change after the rendering of each view, like the list of
extensions suported.

However, it is still a good idea to limit memoization of some api calls during
the rendering of a single view, if the query is subject to give different
results (i.e., the flavor_list can change when an administrator adds a flavor
so we need to retreive new data for each view in case a flavor gets added by a
cloud admin).

Implements blueprint: improve-horizon-caching
Co-Authored-By: Daniel Castellanos <luis.daniel.castellanos@intel.com>
Change-Id: I9f87df72cbf149e7b2d1763d7976cb845b1e98f9

horizon/test/tests/utils.py

@@ -373,6 +373,52 @@ class MemoizedTests(test.TestCase):
             cache_calls(1)
         self.assertEqual(1, len(values_list))
 
+    def test_memoized_with_request_call(self):
+
+        chorus = [
+            "I",
+            "Love",
+            "Rock 'n' Roll",
+            "put another coin",
+            "in the Jukebox Baby."
+        ]
+
+        leader = 'Joan Jett'
+        group = 'Blackhearts'
+
+        for position, chorus_line in enumerate(chorus):
+
+            changed_args = False
+
+            def some_func(some_param):
+                if not changed_args:
+                    self.assertEqual(some_param, chorus_line)
+                else:
+                    self.assertNotEqual(some_param, chorus_line)
+                    self.assertEqual(some_param, group)
+                return leader
+
+            @memoized.memoized_with_request(some_func, position)
+            def some_other_func(*args):
+                return args
+
+            # check chorus_copy[position] is replaced by some_func's
+            # output
+            output1 = some_other_func(*chorus)
+            self.assertEqual(output1[position], leader)
+
+            # Change args used to call the function
+            chorus_copy = list(chorus)
+            chorus_copy[position] = group
+            changed_args = True
+            # check that some_func is called with a different parameter, and
+            # that check chorus_copy[position] is replaced by some_func's
+            # output and some_other_func still called with the same parameters
+            output2 = some_other_func(*chorus_copy)
+            self.assertEqual(output2[position], leader)
+            # check that some_other_func returned a memoized list.
+            self.assertIs(output1, output2)
+
 
 class GetPageSizeTests(test.TestCase):
     def test_bad_session_value(self):

horizon/utils/memoized.py

@@ -104,3 +104,71 @@ def memoized(func):
 # it doesn't keep the instances in memory forever. We might want to separate
 # them in the future, however.
 memoized_method = memoized
+
+
+def memoized_with_request(request_func, request_index=0):
+    """Decorator for caching functions which receive a request argument
+
+    memoized functions with a request argument are memoized only during the
+    rendering of a single view because the request argument is a new request
+    instance on each view.
+
+    If you want a function to be memoized for multiple views use this
+    decorator.
+
+    It replaces the request argument in the call to the decorated function
+    with the result of calling request_func on that request object.
+
+    request_function is a function which will receive the request argument.
+
+    request_index indicates which argument of the decorated function is the
+    request object to pass into request_func, which will also be replaced
+    by the result of request_func being called.
+
+    your memoized function will instead receive request_func(request)
+    passed as argument at the request_index.
+
+    The intent of that function is to extract the information needed from the
+    request, and thus the memoizing will operate just on that part of the
+    request that is relevant to the function being memoized.
+
+    short example:
+
+        @memoized
+        def _get_api_client(username, token_id, project_id, auth_url)
+            return api_client.Client(username, token_id, project_id, auth_url)
+
+        def get_api_client(request):
+            return _api_client(request.user.username,
+                               request.user.token.id,
+                               request.user.tenant_id)
+
+        @memoized_with_request(get_api_client)
+        def some_api_function(api_client, *args, **kwargs):
+            # is like returning get_api_client(
+            #    request).some_method(*args, **kwargs)
+            # but with memoization.
+            return api_client.some_method(*args, **kwargs)
+
+        @memoized_with_request(get_api_client, 1)
+        def some_other_funt(param, api_client, other_param):
+            # The decorated function will be called this way:
+            #     some_other_funt(param, request, other_param)
+            # but will be called behind the scenes this way:
+            #     some_other_funt(param, get_api_client(request), other_param)
+            return api_client.some_method(param, other_param)
+
+    See openstack_dashboard.api.nova for a complete example.
+    """
+    def wrapper(func):
+        memoized_func = memoized(func)
+
+        @functools.wraps(func)
+        def wrapped(*args, **kwargs):
+            args = list(args)
+            request = args.pop(request_index)
+            args.insert(request_index, request_func(request))
+            return memoized_func(*args, **kwargs)
+
+        return wrapped
+    return wrapper

openstack_dashboard/api/nova.py

@@ -39,6 +39,7 @@ from horizon import conf
 from horizon import exceptions as horizon_exceptions
 from horizon.utils import functions as utils
 from horizon.utils.memoized import memoized  # noqa
+from horizon.utils.memoized import memoized_with_request  # noqa
 
 from openstack_dashboard.api import base
 from openstack_dashboard.api import network_base
@@ -55,6 +56,8 @@ VERSIONS.load_supported_version(2, {"client": nova_client, "version": 2})
 INSTANCE_ACTIVE_STATE = 'ACTIVE'
 VOLUME_STATE_AVAILABLE = "available"
 DEFAULT_QUOTA_NAME = 'default'
+INSECURE = getattr(settings, 'OPENSTACK_SSL_NO_VERIFY', False)
+CACERT = getattr(settings, 'OPENSTACK_SSL_CACERT', None)
 
 
 class VNCConsole(base.APIDictWrapper):
@@ -451,20 +454,31 @@ class FloatingIpManager(network_base.FloatingIpManager):
         return True
 
 
-@memoized
-def novaclient(request):
-    insecure = getattr(settings, 'OPENSTACK_SSL_NO_VERIFY', False)
-    cacert = getattr(settings, 'OPENSTACK_SSL_CACERT', None)
+def get_auth_params_from_request(request):
+    """Extracts the properties from the request object needed by the novaclient
+    call below. These will be used to memoize the calls to novaclient
+    """
+    return (
+        request.user.username,
+        request.user.token.id,
+        request.user.tenant_id,
+        base.url_for(request, 'compute')
+    )
+
+
+@memoized_with_request(get_auth_params_from_request)
+def novaclient(request_auth_params):
+    username, token_id, project_id, auth_url = request_auth_params
     c = nova_client.Client(VERSIONS.get_active_version()['version'],
-                           request.user.username,
-                           request.user.token.id,
-                           project_id=request.user.tenant_id,
-                           auth_url=base.url_for(request, 'compute'),
-                           insecure=insecure,
-                           cacert=cacert,
+                           username,
+                           token_id,
+                           project_id=project_id,
+                           auth_url=auth_url,
+                           insecure=INSECURE,
+                           cacert=CACERT,
                            http_log_debug=settings.DEBUG)
-    c.client.auth_token = request.user.token.id
-    c.client.management_url = base.url_for(request, 'compute')
+    c.client.auth_token = token_id
+    c.client.management_url = auth_url
     return c
 
 
@@ -575,10 +589,10 @@ def flavor_list_paged(request, is_public=True, get_extras=False, marker=None,
     return (flavors, has_more_data, has_prev_data)
 
 
-@memoized
-def flavor_access_list(request, flavor=None):
+@memoized_with_request(novaclient)
+def flavor_access_list(nova_api, flavor=None):
     """Get the list of access instance sizes (flavors)."""
-    return novaclient(request).flavor_access.list(flavor=flavor)
+    return nova_api.flavor_access.list(flavor=flavor)
 
 
 def add_tenant_to_flavor(request, flavor, tenant):
@@ -1022,27 +1036,25 @@ def interface_detach(request, server, port_id):
     return novaclient(request).servers.interface_detach(server, port_id)
 
 
-@memoized
-def list_extensions(request):
+@memoized_with_request(novaclient)
+def list_extensions(nova_api):
     """List all nova extensions, except the ones in the blacklist."""
-
     blacklist = set(getattr(settings,
                             'OPENSTACK_NOVA_EXTENSIONS_BLACKLIST', []))
     return [
         extension for extension in
-        nova_list_extensions.ListExtManager(novaclient(request)).show_all()
+        nova_list_extensions.ListExtManager(nova_api).show_all()
         if extension.name not in blacklist
     ]
 
 
-@memoized
-def extension_supported(extension_name, request):
+@memoized_with_request(list_extensions, 1)
+def extension_supported(extension_name, extensions):
     """Determine if nova supports a given extension name.
 
     Example values for the extension_name include AdminActions, ConsoleOutput,
     etc.
     """
-    extensions = list_extensions(request)
     for extension in extensions:
         if extension.name == extension_name:
             return True

openstack_dashboard/test/api_tests/nova_tests.py

@@ -521,20 +521,17 @@ class ComputeApiTests(test.APITestCase):
 
         self.assertIsNone(api_val)
 
+    @test.create_stubs({api.nova: ('flavor_access_list',)})
     def test_flavor_access_list(self):
         flavor_access = self.flavor_access.list()
         flavor = [f for f in self.flavors.list() if f.id ==
                   flavor_access[0].flavor_id][0]
 
-        novaclient = self.stub_novaclient()
-        novaclient.flavors = self.mox.CreateMockAnything()
-        novaclient.flavor_access = self.mox.CreateMockAnything()
-        novaclient.flavor_access.list(flavor=flavor).AndReturn(flavor_access)
+        api.nova.flavor_access_list(self.request, flavor)\
+            .AndReturn(flavor_access)
 
         self.mox.ReplayAll()
-
         api_flavor_access = api.nova.flavor_access_list(self.request, flavor)
-
         self.assertEqual(len(flavor_access), len(api_flavor_access))
         for access in api_flavor_access:
             self.assertIsInstance(access, nova_flavor_access.FlavorAccess)