From 11d973fbe36369cba4b450536c6dff57c0cd6d21 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Thu, 24 Dec 2015 15:21:33 +0900 Subject: [PATCH] Fix network duplication check logic Previously api.neutron.Network object was compared by using _apidict but it did not work when for example the order of subnet list is different. This commit changes the comparison logic to use network ID explicitly. In addition, it moves the logic to fetch external networks to api.neutron. Change-Id: Ie3a42e29c32c17a7f3bf1596b0e09cb73eae9d2a Closes-Bug: #1528465 --- openstack_dashboard/api/base.py | 5 -- openstack_dashboard/api/neutron.py | 8 +++- .../dashboards/project/networks/views.py | 11 +---- .../test/api_tests/neutron_tests.py | 47 ++++++++++++++++++- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/openstack_dashboard/api/base.py b/openstack_dashboard/api/base.py index ea45eefbda..30974a2d89 100644 --- a/openstack_dashboard/api/base.py +++ b/openstack_dashboard/api/base.py @@ -166,11 +166,6 @@ class APIDictWrapper(object): def __repr__(self): return "<%s: %s>" % (self.__class__.__name__, self._apidict) - def __cmp__(self, other): - if hasattr(other, '_apidict'): - return cmp(self._apidict, other._apidict) - return cmp(self._apidict, other) - def to_dict(self): return self._apidict diff --git a/openstack_dashboard/api/neutron.py b/openstack_dashboard/api/neutron.py index 3c31baef8f..f819a83f68 100644 --- a/openstack_dashboard/api/neutron.py +++ b/openstack_dashboard/api/neutron.py @@ -613,7 +613,8 @@ def network_list(request, **params): return [Network(n) for n in networks] -def network_list_for_tenant(request, tenant_id, **params): +def network_list_for_tenant(request, tenant_id, include_external=False, + **params): """Return a network list available for the tenant. The list contains networks owned by the tenant and public networks. @@ -632,6 +633,11 @@ def network_list_for_tenant(request, tenant_id, **params): # both owner networks and public networks in a single API call. networks += network_list(request, shared=True, **params) + if include_external: + fetched_net_ids = [n.id for n in networks] + ext_nets = network_list(request, **{'router:external': True}) + networks += [n for n in ext_nets if n.id not in fetched_net_ids] + return networks diff --git a/openstack_dashboard/dashboards/project/networks/views.py b/openstack_dashboard/dashboards/project/networks/views.py index aed9080fd0..86b67d21d4 100644 --- a/openstack_dashboard/dashboards/project/networks/views.py +++ b/openstack_dashboard/dashboards/project/networks/views.py @@ -48,15 +48,8 @@ class IndexView(tables.DataTableView): def get_data(self): try: tenant_id = self.request.user.tenant_id - networks = api.neutron.network_list_for_tenant(self.request, - tenant_id) - # List Public networks - for network in api.neutron.network_list( - self.request, **{'router:external': True} - ): - if network not in networks: - networks.append(network) - + networks = api.neutron.network_list_for_tenant( + self.request, tenant_id, include_external=True) except Exception: networks = [] msg = _('Network list can not be retrieved.') diff --git a/openstack_dashboard/test/api_tests/neutron_tests.py b/openstack_dashboard/test/api_tests/neutron_tests.py index 06613ddbb8..a14a20701c 100644 --- a/openstack_dashboard/test/api_tests/neutron_tests.py +++ b/openstack_dashboard/test/api_tests/neutron_tests.py @@ -12,9 +12,11 @@ # License for the specific language governing permissions and limitations # under the License. import copy - import uuid +from mox3.mox import IsA # noqa + +from django import http from django.test.utils import override_settings from neutronclient.common import exceptions as neutron_exc @@ -38,6 +40,49 @@ class NeutronApiTests(test.APITestCase): for n in ret_val: self.assertIsInstance(n, api.neutron.Network) + @test.create_stubs({api.neutron: ('network_list', + 'subnet_list')}) + def _test_network_list_for_tenant(self, include_external): + all_networks = self.networks.list() + tenant_id = '1' + api.neutron.network_list( + IsA(http.HttpRequest), + tenant_id=tenant_id, + shared=False).AndReturn([ + network for network in all_networks + if network['tenant_id'] == tenant_id + ]) + api.neutron.network_list( + IsA(http.HttpRequest), + shared=True).AndReturn([ + network for network in all_networks + if network.get('shared') + ]) + if include_external: + api.neutron.network_list( + IsA(http.HttpRequest), + **{'router:external': True}).AndReturn([ + network for network in all_networks + if network.get('router:external') + ]) + self.mox.ReplayAll() + + ret_val = api.neutron.network_list_for_tenant( + self.request, tenant_id, + include_external=include_external) + expected = [n for n in all_networks + if (n['tenant_id'] == tenant_id or + n['shared'] or + (include_external and n['router:external']))] + self.assertEqual(set(n.id for n in expected), + set(n.id for n in ret_val)) + + def test_network_list_for_tenant(self): + self._test_network_list_for_tenant(include_external=False) + + def test_network_list_for_tenant_with_external(self): + self._test_network_list_for_tenant(include_external=True) + def test_network_get(self): network = {'network': self.api_networks.first()} subnet = {'subnet': self.api_subnets.first()}