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

@@ -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
 

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
 
 

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.')

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()}