Cache neutron extension list across requests

Also improves the warning message of UnhashableWarning
and test_neutron UnhashableWarning.

Closes-Bug: #1747204
Change-Id: I876e3219dac570e8f7b10c5c126df74ca73f5197

horizon/utils/memoized.py

@@ -99,8 +99,9 @@ def memoized(func):
             # that case, we can't cache anything and simply always call the
             # decorated function.
             warnings.warn(
-                "The key %r is not hashable and cannot be memoized."
-                % (key,), UnhashableKeyWarning, 2)
+                "The key of %s %s is not hashable and cannot be memoized: %r\n"
+                % (func.__module__, func.__name__, key),
+                UnhashableKeyWarning, 2)
             value = func(*args, **kwargs)
         return value
     return wrapped

openstack_dashboard/api/neutron.py

@@ -35,6 +35,7 @@ import six
 from horizon import exceptions
 from horizon import messages
 from horizon.utils.memoized import memoized
+from horizon.utils.memoized import memoized_with_request
 from openstack_dashboard.api import base
 from openstack_dashboard.api import nova
 from openstack_dashboard.contrib.developer.profiler import api as profiler
@@ -760,13 +761,22 @@ def get_ipver_str(ip_version):
     return IP_VERSION_DICT.get(ip_version, '')
 
 
-@memoized
-def neutronclient(request):
+def get_auth_params_from_request(request):
+    return (
+        request.user.token.id,
+        base.url_for(request, 'network'),
+        base.url_for(request, 'identity')
+    )
+
+
+@memoized_with_request(get_auth_params_from_request)
+def neutronclient(request_auth_params):
+    token_id, neutron_url, auth_url = request_auth_params
     insecure = getattr(settings, 'OPENSTACK_SSL_NO_VERIFY', False)
     cacert = getattr(settings, 'OPENSTACK_SSL_CACERT', None)
-    c = neutron_client.Client(token=request.user.token.id,
-                              auth_url=base.url_for(request, 'identity'),
-                              endpoint_url=base.url_for(request, 'network'),
+    c = neutron_client.Client(token=token_id,
+                              auth_url=auth_url,
+                              endpoint_url=neutron_url,
                               insecure=insecure, ca_cert=cacert)
     return c
 
@@ -1704,10 +1714,14 @@ def _server_get_addresses(request, server, ports, floating_ips, network_names):
 
 
 @profiler.trace
-@memoized
-def list_extensions(request):
+@memoized_with_request(neutronclient)
+def list_extensions(neutron_api):
+    """List neutron extensions.
+
+    :param request: django request object
+    """
     try:
-        extensions_list = neutronclient(request).list_extensions()
+        extensions_list = neutron_api.list_extensions()
     except exceptions.ServiceCatalogException:
         return {}
     if 'extensions' in extensions_list:
@@ -1717,10 +1731,13 @@ def list_extensions(request):
 
 
 @profiler.trace
-@memoized
 def is_extension_supported(request, extension_alias):
-    extensions = list_extensions(request)
+    """Check if a specified extension is supported.
 
+    :param request: django request object
+    :param extension_alias: neutron extension alias
+    """
+    extensions = list_extensions(request)
     for extension in extensions:
         if extension['alias'] == extension_alias:
             return True

openstack_dashboard/test/unit/api/test_neutron.py

@@ -13,6 +13,7 @@
 #    under the License.
 import copy
 
+import mock
 from mox3.mox import IsA
 import netaddr
 from neutronclient.common import exceptions as neutron_exc
@@ -393,13 +394,16 @@ class NeutronApiTests(test.APITestCase):
         for p in ret_val:
             self.assertIsInstance(p, api.neutron.Port)
 
-    def test_port_list_with_trunk_types(self):
+    @mock.patch.object(api.neutron, 'is_extension_supported')
+    def test_port_list_with_trunk_types(self, mock_is_extension_supported):
         ports = self.api_tp_ports.list()
         trunks = self.api_tp_trunks.list()
 
+        # list_extensions is decorated with memoized_with_request,
+        # stub_neutronclient is not called. We need to mock it separately.
+        mock_is_extension_supported.return_value = True  # trunk
+
         neutronclient = self.stub_neutronclient()
-        neutronclient.list_extensions() \
-            .AndReturn({'extensions': self.api_extensions.list()})
         neutronclient.list_ports().AndReturn({'ports': ports})
         neutronclient.list_trunks().AndReturn({'trunks': trunks})
         self.mox.ReplayAll()
@@ -428,13 +432,19 @@ class NeutronApiTests(test.APITestCase):
         self.assertEqual(expected_subport_ids, subport_ids)
         self.assertEqual(expected_normal_port_ids, normal_port_ids)
 
-    def test_port_list_with_trunk_types_without_trunk_extension(self):
-        extensions = [ext for ext in self.api_extensions.list()
-                      if ext['alias'] != 'trunk']
+        mock_is_extension_supported.assert_called_once_with(
+            test.IsHttpRequest(), 'trunk')
+
+    @mock.patch.object(api.neutron, 'is_extension_supported')
+    def test_port_list_with_trunk_types_without_trunk_extension(
+            self, mock_is_extension_supported):
         ports = self.api_tp_ports.list()
 
+        # list_extensions is decorated with memoized_with_request,
+        # the simpliest way is to mock it directly.
+        mock_is_extension_supported.return_value = False  # trunk
+
         neutronclient = self.stub_neutronclient()
-        neutronclient.list_extensions().AndReturn({'extensions': extensions})
         neutronclient.list_ports().AndReturn({'ports': ports})
         self.mox.ReplayAll()
 
@@ -447,6 +457,9 @@ class NeutronApiTests(test.APITestCase):
         # instances of Port class.
         self.assertTrue(all(isinstance(p, api.neutron.Port) for p in ret_val))
 
+        mock_is_extension_supported.assert_called_once_with(
+            test.IsHttpRequest(), 'trunk')
+
     def test_port_get(self):
         port = {'port': self.api_ports.first()}
         port_id = self.api_ports.first()['id']
@@ -715,12 +728,13 @@ class NeutronApiTests(test.APITestCase):
         api.neutron.router_remove_interface(
             self.request, router_id, port_id=fake_port)
 
-    def test_is_extension_supported(self):
-        neutronclient = self.stub_neutronclient()
-        neutronclient.list_extensions() \
-            .AndReturn({'extensions': self.api_extensions.list()})
-        self.mox.ReplayAll()
-
+    # stub_neutronclient does not work because api.neutron.list_extensions
+    # is decorated with memoized_with_request, so we need to mock
+    # neutronclient.v2_0.client directly.
+    @mock.patch('neutronclient.v2_0.client.Client.list_extensions')
+    def test_is_extension_supported(self, mock_list_extensions):
+        extensions = self.api_extensions.list()
+        mock_list_extensions.return_value = {'extensions': extensions}
         self.assertTrue(
             api.neutron.is_extension_supported(self.request, 'quotas'))
         self.assertFalse(