Merge pull request #350 from narJH27/object-list-views-stale-results-…

…issue-344

Bug Fix For Issue #344

nsot/api/views.py

@@ -287,32 +287,12 @@ def perform_destroy(self, instance):
         change = models.Change.objects.create(
             obj=instance, user=self.request.user, event='Delete'
         )
-        force_delete = qpbool(
-            self.request.query_params.get('force_delete', False)
-        )
 
         try:
             instance.delete()
         except exc.ProtectedError as err:
-            if force_delete:
-                new_parent = instance.parent
-                # Check if the network does not have a parent, check that it's
-                # children are not leaf nodes. If so, raise an error.
-                if not new_parent:
-                    children = instance.get_children()
-                    for child in children:
-                        if child.is_leaf_node():
-                            raise exc.Conflict('You cannot forcefully delete a'
-                                               ' network that does not have a'
-                                               ' parent, and whose children'
-                                               ' are leaf nodes.')
-                # Otherwise, update all children to use the new parent and
-                # delete the old parent of these child nodes.
-                err.protected_objects.update(parent=new_parent)
-                models.Network.objects.filter(pk=instance.pk).delete()
-            else:
-                change.delete()
-                raise exc.Conflict(err.args[0])
+            change.delete()
+            raise exc.Conflict(err.args[0])
 
 
 class SiteViewSet(NsotViewSet):
@@ -664,6 +644,28 @@ def reserved(self, request, site_pk=None, *args, **kwargs):
         objects = models.Network.objects.reserved()
         return self.list(request, queryset=objects, *args, **kwargs)
 
+    # Shoutout to jathanism for suggesting the move of perform_destroy(()
+    # to the NetworkViewSet to make it more explicit.
+    def perform_destroy(self, instance):
+        """
+        Overload default to handle non-serializer exceptions, and log Change
+        events.
+        :param instance:
+            Model instance to delete
+        """
+        log.debug('NetworkViewSet.perform_destroy() obj = %r', instance)
+        change = models.Change.objects.create(
+            obj=instance, user=self.request.user, event='Delete'
+        )
+        force_delete = qpbool(
+            self.request.query_params.get('force_delete', False)
+        )
+        try:
+            instance.delete(force_delete=force_delete)
+        except exc.ProtectedError as err:
+            change.delete()
+            raise exc.Conflict(err.args[0])
+
 
 class InterfaceViewSet(ResourceViewSet):
     """

nsot/models/network.py

@@ -552,6 +552,33 @@ def clean_fields(self, exclude=None):
         self.prefix_length = network.prefixlen
         self.state = self.clean_state(self.state)
 
+    # Shoutout to jathanism for this code.
+    def delete(self, **kwargs):
+        force_delete = kwargs.pop('force_delete', False)
+
+        try:
+            super(Network, self).delete(**kwargs)
+        except exc.ProtectedError as err:
+            if force_delete:
+                new_parent = self.parent
+                # Check if the network does not have a parent, check that it's
+                # children are not leaf nodes. If so, raise an error.
+                if not new_parent:
+                    children = self.get_children()
+                    for child in children:
+                        if child.is_leaf_node():
+                            raise exc.Conflict(
+                                'You cannot forcefully delete a network that'
+                                'does not have a parent, and whose children '
+                                ' are leaf nodes.'
+                            )
+                # Otherwise, update all children to use the new parent and
+                # delete the old parent of these child nodes.
+                err.protected_objects.update(parent=new_parent)
+                super(Network, self).delete(**kwargs)
+            else:
+                raise
+
     def save(self, *args, **kwargs):
         """This is stuff we want to happen upon save."""
         self.full_clean()  # First validate fields are correct
@@ -588,3 +615,26 @@ def to_dict(self):
             'state': self.state,
             'attributes': self.get_attributes(),
         }
+
+
+# Signals
+def refresh_assignment_interface_networks(sender, instance, **kwargs):
+    """This signal fires each time a Network object is saved. Upon save,
+    the signal iterates through all the child networks of the network
+    being saved and cleans the addresses and networks assigned to the
+    interfaces (if any) to which these child networks have been assigned.
+
+    We need to clean the addresses on an Interface upon a call to save()
+    on Network due to the Interface model caching _addresses & _networks
+    which causes the update on the Network object to not cascade onto the
+    corresponding Interface object."""
+    for child in instance.children.all():
+        for assignment in child.assignments.all():
+            assignment.interface.clean_addresses()
+            assignment.interface.save()
+
+
+models.signals.post_save.connect(
+    refresh_assignment_interface_networks, sender=Network,
+    dispatch_uid='refresh_interface_assignment_networks_post_save_network'
+)

tests/api_tests/test_networks.py

@@ -421,7 +421,8 @@ def test_deletion(site, client):
     net2_obj_uri = site.detail_uri('network', id=net2['id'])
 
     # Don't allow delete when there's an attached subnet/ip
-    assert_error(client.delete(net1_obj_uri), status.HTTP_409_CONFLICT)
+    assert_error(client.delete(net1_obj_uri), 
+        status.HTTP_409_CONFLICT)
 
     # Delete the child Network
     client.delete(net2_obj_uri)
@@ -470,7 +471,8 @@ def test_force_deletion(site, client):
         - force delete /23 should raise an error.
     """
     # Delete /24 will fail, because it has a child.
-    assert_error(client.destroy(net3_obj_uri), status.HTTP_409_CONFLICT)
+    assert_error(client.destroy(net3_obj_uri),
+        status.HTTP_409_CONFLICT)
 
     # Forcefully delete the /24
     assert_deleted(client.destroy(net3_obj_uri, force_delete=True))

tests/model_tests/test_interfaces.py

@@ -286,6 +286,30 @@ def test_device_hostname(device):
                                          name='eth0')
 
 
+def test_interface_networks_refresh(device):
+    """Test the interface parent networks refresh upon reparenting of a
+    Network object"""
+    cidr = '10.1.1.1/32'
+    parent_network = models.Network.objects.create(
+        cidr='10.1.1.0/24', site=device.site
+    )
+    intf_address = models.Network.objects.create(
+        cidr=cidr, site=device.site
+    )
+    intf = models.Interface.objects.create(device=device, name='eth0')
+    intf.assign_address(cidr)
+    intf.clean_addresses()
+    intf.save()
+    assert intf.get_networks() == ['10.1.1.0/24']
+
+    new_parent_network = models.Network.objects.create(
+        cidr='10.1.1.0/27', site=device.site
+    )
+    new_parent_network.save()
+
+    intf_obj = models.Interface.objects.get(device=device, name='eth0')
+    assert intf_obj.get_networks() == ['10.1.1.0/27']
+
 # TODO(jathan): This isn't implemented yet, but the idea is that there will be
 # pluggable parenting/inheritance strategies, with the "SNMP index" strategy as
 # the default/built-in (e.g. snmp_index, snmp_parent_index).