Repository: openwisp/openwisp-network-topology (language: python)

Issue:
[bug] Link and Node should belong to same topologies
It is possible to create link in a topology with nodes belonging to different topology. 

This can be seen in the following setUp method of a tes class
https://github.com/openwisp/openwisp-network-topology/blob/7daffcfa70abdb4f0f123d4cf30d88852409b196/openwisp_network_topology/integrations/device/tests.py#L312-L328

On line `327`, the node belongs to `topology` object while the link is getting created with `t` object. 

We need to add a validation test in AbstractLink class. 


[bug] Link and Node should belong to same topologies
It is possible to create link in a topology with nodes belonging to different topology. 

This can be seen in the following setUp method of a tes class
https://github.com/openwisp/openwisp-network-topology/blob/7daffcfa70abdb4f0f123d4cf30d88852409b196/openwisp_network_topology/integrations/device/tests.py#L312-L328

On line `327`, the node belongs to `topology` object while the link is getting created with `t` object. 

We need to add a validation test in AbstractLink class. 




Discussion / hints:
While working on #129 I wrote a validation function for this but later decided to open a separated issue for this. 

```diff
diff --git a/openwisp_network_topology/base/link.py b/openwisp_network_topology/base/link.py
index 8eb2b97..66b7551 100644
--- a/openwisp_network_topology/base/link.py
+++ b/openwisp_network_topology/base/link.py
@@ -77,6 +77,7 @@ class AbstractLink(ShareableOrgMixin, TimeStampedEditableModel):
 
     def full_clean(self, *args, **kwargs):
         self.validate_organization()
+        self.validate_topology()
         return super().full_clean(*args, **kwargs)
 
     def clean(self):
@@ -116,6 +117,23 @@ class AbstractLink(ShareableOrgMixin, TimeStampedEditableModel):
                 )
             self.organization_id = self.topology.organization_id
 
+    def validate_topology(self):
+        if self.source.topology_id != self.topology_id:
+            raise ValidationError(
+                {
+                    'source': _(
+                        'Source node and link should have same topology.'
+                    )
+                }
+            )
+        if self.target.topology_id != self.topology_id:
+            raise ValidationError(
+                {
+                    'target': _(
+                        'Target node and link should have same topology.'
+                    )
+                }
+            )
```
While working on #129 I wrote a validation function for this but later decided to open a separated issue for this. 

```diff
diff --git a/openwisp_network_topology/base/link.py b/openwisp_network_topology/base/link.py
index 8eb2b97..66b7551 100644
--- a/openwisp_network_topology/base/link.py
+++ b/openwisp_network_topology/base/link.py
@@ -77,6 +77,7 @@ class AbstractLink(ShareableOrgMixin, TimeStampedEditableModel):
 
     def full_clean(self, *args, **kwargs):
         self.validate_organization()
+        self.validate_topology()
         return super().full_clean(*args, **kwargs)
 
     def clean(self):
@@ -116,6 +117,23 @@ class AbstractLink(ShareableOrgMixin, TimeStampedEditableModel):
                 )
             self.organization_id = self.topology.organization_id
 
+    def validate_topology(self):
+        if self.source.topology_id != self.topology_id:
+            raise ValidationError(
+                {
+                    'source': _(
+                        'Source node and link should have same topology.'
+                    )
+                }
+            )
+        if self.target.topology_id != self.topology_id:
+            raise ValidationError(
+                {
+                    'target': _(
+                        'Target node and link should have same topology.'
+                    )
+                }
+            )
```

Fix patch:
diff --git a/openwisp_network_topology/base/link.py b/openwisp_network_topology/base/link.py
index 26fa1e2c..9e5ac763 100644
--- a/openwisp_network_topology/base/link.py
+++ b/openwisp_network_topology/base/link.py
@@ -77,6 +77,7 @@ def __str__(self):
 
     def full_clean(self, *args, **kwargs):
         self.organization = self.topology.organization
+        self.validate_topology()
         return super().full_clean(*args, **kwargs)
 
     def clean(self):
@@ -89,6 +90,15 @@ def clean(self):
             if attr in self.properties:
                 del self.properties[attr]
 
+    def validate_topology(self):
+        errors = {}
+        if self.source.topology_id != self.topology_id:
+            errors['source'] = _('Source node and link should have same topology.')
+        if self.target.topology_id != self.topology_id:
+            errors['target'] = _('Target node and link should have same topology.')
+        if errors:
+            raise ValidationError(errors)
+
     def json(self, dict=False, original=False, **kwargs):
         """
         Returns a NetJSON NetworkGraph Link object.


Test patch:
diff --git a/openwisp_network_topology/integrations/device/tests.py b/openwisp_network_topology/integrations/device/tests.py
index 99266d2e..4b9e842b 100644
--- a/openwisp_network_topology/integrations/device/tests.py
+++ b/openwisp_network_topology/integrations/device/tests.py
@@ -3,6 +3,7 @@
 import django
 import swapper
 from django.contrib.auth import get_user_model
+from django.core.exceptions import ValidationError
 from django.core.management import call_command
 from django.db import transaction
 from django.test import TransactionTestCase
@@ -310,8 +311,6 @@ def prefix(self):
         return 'admin:{0}'.format(self.app_label)
 
     def setUp(self):
-        org = self._create_org()
-        t = self._create_topology(organization=org)
         topology, device, cert = self._create_test_env(parser='netdiff.OpenvpnParser')
         node1 = self._init_test_node(topology, common_name=cert.common_name)
         node2 = self._init_test_node(topology, addresses=['netjson_id2'], label='test2')
@@ -324,7 +323,7 @@ def setUp(self):
         )
         link.full_clean()
         link.save()
-        self._create_link(topology=t, source=node1, target=node2)
+        self._create_link(topology=topology, source=node1, target=node2)
         self.client.force_login(self.user_model.objects.get(username='admin'))
 
     def test_node_change_list_queries(self):
@@ -336,3 +335,23 @@ def test_link_change_list_queries(self):
         path = reverse('{0}_link_changelist'.format(self.prefix))
         with self.assertNumQueries(7):
             self.client.get(path)
+
+    def test_link_node_different_topology(self):
+        Topology.objects.all().delete()
+        org = self._create_org()
+        topology1 = self._create_topology(organization=org)
+        topology2 = self._create_topology(organization=org)
+        node1 = self._init_test_node(topology2)
+        node2 = self._init_test_node(topology2)
+        link = Link(
+            source=node1,
+            target=node2,
+            topology=topology1,
+            organization=topology1.organization,
+            cost=1,
+        )
+        with self.assertRaises(ValidationError) as context:
+            # Raises ValidationError if link belongs to diff topologies
+            link.full_clean()
+        self.assertIn('source', context.exception.error_dict)
+        self.assertIn('target', context.exception.error_dict)