Merge pull request #23601 from netbox-community/23595-update-dependent-objects

Fixes #23595: Rebuild wireless, circuit and channel paths after raw writes

netbox/circuits/models/circuits.py

@@ -9,6 +9,7 @@ from django.utils.translation import gettext_lazy as _
 
 from circuits.choices import *
 from dcim.models import CabledObjectModel
+from dcim.utils import rebuild_paths
 from netbox.models import ChangeLoggedModel, OrganizationalModel, PrimaryModel
 from netbox.models.features import (
     ContactsMixin,
@@ -499,3 +500,10 @@ class CircuitTermination(
             )
         except CircuitTermination.DoesNotExist:
             return None
+
+    def update_dependent_objects(self):
+        """
+        Rebuild the CablePaths which traverse this CircuitTermination or its peer.
+        """
+        rebuild_paths([t for t in (self, self.get_peer_termination()) if t])
+    update_dependent_objects.alters_data = True

netbox/dcim/models/device_components.py

@@ -1,6 +1,7 @@
 from functools import cached_property
 
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation
+from django.contrib.contenttypes.models import ContentType
 from django.contrib.postgres.fields import ArrayField
 from django.contrib.postgres.indexes import GistIndex
 from django.core.exceptions import ObjectDoesNotExist, ValidationError
@@ -19,6 +20,7 @@ from dcim.models.mixins import (
     InterfaceValidationMixin,
     MaxFlowMixin,
 )
+from dcim.utils import rebuild_paths
 from netbox.choices import ColorChoices
 from netbox.models import NetBoxModel, OrganizationalModel
 from netbox.models.features import ChangeLoggingMixin
@@ -1425,6 +1427,35 @@ class Interface(
         if self.channels:
             self.clear_channel_cables()
 
+    def update_dependent_objects(self):
+        """
+        Reconcile a channel subinterface's mirrored cable attributes with its channelized parent, and retrace the
+        CablePaths which traverse this interface.
+        """
+        from dcim.models import CablePath, CableTermination
+
+        parent = None
+        if self.channel_id and self.parent_id:
+            parent = Interface.objects.filter(pk=self.parent_id, cable__isnull=False).select_related('cable').first()
+        if parent and parent.channels:
+            parent.cable.update_dependent_objects()
+        elif self.cable_id and not CableTermination.objects.filter(
+            termination_type=ContentType.objects.get_for_model(Interface), termination_id=self.pk
+        ).exists():
+            # A cable with no termination of this interface's own was mirrored from a former channelized parent
+            Interface.objects.filter(pk=self.pk).update(
+                cable=None, cable_end='', cable_connector=None, cable_positions=None
+            )
+            for cablepath in CablePath.objects.filter(_nodes__contains=self):
+                if self in cablepath.origins:
+                    cablepath.delete()
+        else:
+            return
+
+        # Retrace paths still traversing this interface from a former parent's cable
+        rebuild_paths([self])
+    update_dependent_objects.alters_data = True
+
     def propagate_channel_cables(self):
         """
         Mirror this channelized interface's cable attributes onto each of its channel subinterfaces, restricting each

netbox/dcim/tests/test_cablepaths.py

@@ -9,6 +9,7 @@ from dcim.svg import CableTraceSVG
 from dcim.tests.utils import BaseCablePathTestCase
 from dcim.utils import create_cablepaths, object_to_path_node
 from utilities.exceptions import AbortRequest
+from wireless.models import WirelessLink
 
 
 class LegacyCablePathTestCase(BaseCablePathTestCase):
@@ -3487,6 +3488,56 @@ class LegacyCablePathTestCase(BaseCablePathTestCase):
         self.assertIsNone(interface2.cable)
         self.assertPathIsNotSet(interface2)
 
+    def test_323_update_dependent_objects_extends_path_via_raw_circuit_termination(self):
+        """
+        [IF1] --C1-- [CT1] [CT2] --> [PN1], with CT2 written raw. Refs: #23595
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        providernetwork = ProviderNetwork.objects.create(name='Provider Network 1', provider=self.circuit.provider)
+        circuittermination1 = CircuitTermination.objects.create(
+            circuit=self.circuit,
+            termination=self.site,
+            term_side='A'
+        )
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[circuittermination1])
+        cable1.save()
+        self.assertCurrentPathExists((interface1, cable1, circuittermination1), is_complete=False)
+
+        circuittermination2 = CircuitTermination(circuit=self.circuit, termination=providernetwork, term_side='Z')
+        circuittermination2.cache_related_objects()
+        circuittermination2.save_base(raw=True)
+        self.assertCurrentPathExists((interface1, cable1, circuittermination1), is_complete=False)
+
+        self.update_dependent_objects(circuittermination2)
+
+        self.assertCurrentPathExists(
+            (interface1, cable1, circuittermination1, circuittermination2, providernetwork),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertEqual(CablePath.objects.count(), 1)
+
+    def test_324_update_dependent_objects_traces_raw_wireless_link(self):
+        """
+        [IF1] --WL1-- [IF2], with WL1 written raw. Refs: #23595
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1', type='ieee802.11ac')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2', type='ieee802.11ac')
+
+        wirelesslink1 = WirelessLink(interface_a=interface1, interface_b=interface2, ssid='Link 1')
+        wirelesslink1._interface_a_device = wirelesslink1._interface_b_device = self.device
+        wirelesslink1.save_base(raw=True)
+        self.assertEqual(CablePath.objects.count(), 0)
+
+        self.update_dependent_objects(wirelesslink1)
+
+        for interface in (interface1, interface2):
+            interface.refresh_from_db()
+            self.assertEqual(interface.wireless_link, wirelesslink1)
+        self.assertCurrentPathExists((interface1, wirelesslink1, interface2), is_complete=True, is_active=True)
+        self.assertCurrentPathExists((interface2, wirelesslink1, interface1), is_complete=True, is_active=True)
+        self.assertEqual(CablePath.objects.count(), 2)
+
     def test_401_exclude_midspan_devices(self):
         """
         [IF1] --C1-- [FP1][Test Device][RP1] --C2-- [RP2][Test Device][FP2] --C3-- [IF2]

netbox/dcim/tests/test_channelization.py

@@ -3,7 +3,7 @@ from unittest import mock
 
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ValidationError
-from django.db import connection, router
+from django.db import connection, router, transaction
 from django.test import Client, TestCase, TransactionTestCase
 from django.test.utils import CaptureQueriesContext
 from django.urls import reverse
@@ -850,6 +850,133 @@ class ChannelizedCablePathTestCase(BaseCablePathTestCase):
                 (peer, cable, channel), pk=peer_paths[peer.pk], is_complete=True, is_active=True
             )
 
+    def test_124_update_dependent_objects_mirrors_cable_onto_raw_channel(self):
+        """
+        A channel subinterface written raw must inherit its parent's cable and trace in both directions once
+        Interface.update_dependent_objects() is called. Refs: #23595
+        """
+        parent, channels = self._create_channelized_interface('et0', 4)
+        channels[3].delete()
+        far = [
+            Interface.objects.create(device=self.device, name=f'xe{i}', type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS)
+            for i in range(4)
+        ]
+        cable = Cable(profile=CableProfileChoices.BREAKOUT_1C4P_4C1P, a_terminations=[parent], b_terminations=far)
+        cable.clean()
+        cable.save()
+
+        channel = Interface(
+            device=self.device, name='et0:4', type=InterfaceTypeChoices.TYPE_CHANNEL, parent=parent, channel_id=4
+        )
+        channel.save_base(raw=True)
+        channel.refresh_from_db()
+        self.assertIsNone(channel.cable_id)
+        self.assertPathIsNotSet(channel)
+
+        self.update_dependent_objects(channel)
+
+        channel.refresh_from_db()
+        self.assertEqual(channel.cable_id, cable.pk)
+        self.assertEqual(channel.cable_positions, [4])
+        self.assertCurrentPathExists((channel, cable, far[3]), is_complete=True, is_active=True)
+        self.assertCurrentPathExists((far[3], cable, channel), is_complete=True, is_active=True)
+        self.assertEqual(CablePath.objects.count(), 8)
+
+    def _assert_hook_matches_save(self, channel, **attrs):
+        """
+        Assert that writing attrs to channel raw and calling update_dependent_objects() leaves the same cable
+        attributes and CablePaths as a normal save().
+        """
+        def snapshot():
+            interfaces = Interface.objects.order_by('pk').values_list(
+                'pk', 'cable_id', 'cable_end', 'cable_connector', 'cable_positions', '_path__path'
+            )
+            paths = sorted((p.path, p.is_complete, p.is_active, p.is_split) for p in CablePath.objects.all())
+            return list(interfaces), paths
+
+        class Rollback(Exception):
+            pass
+
+        try:
+            with transaction.atomic():
+                instance = Interface.objects.get(pk=channel.pk)
+                for name, value in attrs.items():
+                    setattr(instance, name, value)
+                instance.full_clean()
+                instance.save()
+                expected = snapshot()
+                raise Rollback
+        except Rollback:
+            pass
+
+        instance = Interface.objects.get(pk=channel.pk)
+        for name, value in attrs.items():
+            setattr(instance, name, value)
+        instance.save_base(raw=True)
+        self.update_dependent_objects(instance)
+
+        self.assertEqual(snapshot(), expected)
+
+    def _create_breakout(self, name, far_prefix):
+        parent, channels = self._create_channelized_interface(name, 4)
+        far = [
+            Interface.objects.create(
+                device=self.device, name=f'{far_prefix}{i}', type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS
+            )
+            for i in range(4)
+        ]
+        cable = Cable(profile=CableProfileChoices.BREAKOUT_1C4P_4C1P, a_terminations=[parent], b_terminations=far)
+        cable.clean()
+        cable.save()
+        return parent, channels, far, cable
+
+    def test_125_update_dependent_objects_clears_raw_detached_channel(self):
+        """
+        A channel subinterface detached by a raw write must drop its mirrored cable attributes and paths. Refs: #23595
+        """
+        _, channels, far, _ = self._create_breakout('et0', 'xe')
+
+        self._assert_hook_matches_save(
+            channels[3], parent=None, channel_id=None, type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS
+        )
+
+        channel = Interface.objects.get(pk=channels[3].pk)
+        self.assertIsNone(channel.cable_id)
+        self.assertPathIsNotSet(channel)
+        self.assertFalse(CablePath.objects.filter(_nodes__contains=channel).exists())
+
+    def test_126_update_dependent_objects_retraces_raw_reparented_channel(self):
+        """
+        A channel subinterface moved to another channelized parent by a raw write must leave no path from its former
+        parent's far end. Refs: #23595
+        """
+        parent1, channels1, far1, cable1 = self._create_breakout('et0', 'xe')
+        parent2, channels2, far2, cable2 = self._create_breakout('et1', 'ge')
+        channels2[3].delete()
+
+        self._assert_hook_matches_save(channels1[3], parent=Interface.objects.get(pk=parent2.pk), name='et1:4')
+
+        channel = Interface.objects.get(pk=channels1[3].pk)
+        self.assertEqual(channel.cable_id, cable2.pk)
+        self.assertCurrentPathExists((channel, cable2, far2[3]), is_complete=True, is_active=True)
+        self.assertCurrentPathExists((far2[3], cable2, channel), is_complete=True, is_active=True)
+        self.assertFalse(CablePath.objects.filter(_nodes__contains=cable1).filter(_nodes__contains=channel).exists())
+
+    def test_127_update_dependent_objects_clears_channel_of_uncabled_parent(self):
+        """
+        A channel subinterface moved by a raw write to a channelized parent with no cable must drop its mirrored
+        cable attributes and paths. Refs: #23595
+        """
+        _, channels, _, _ = self._create_breakout('et0', 'xe')
+        parent2, channels2 = self._create_channelized_interface('et1', 4)
+        channels2[3].delete()
+
+        self._assert_hook_matches_save(channels[3], parent=Interface.objects.get(pk=parent2.pk), name='et1:4')
+
+        channel = Interface.objects.get(pk=channels[3].pk)
+        self.assertIsNone(channel.cable_id)
+        self.assertFalse(CablePath.objects.filter(_nodes__contains=channel).exists())
+
 
 class ChannelizedInterfaceTestCase(TestCase):
     """

netbox/dcim/tests/utils.py

@@ -1,8 +1,13 @@
-from django.test import TestCase
+import uuid
+
+from django.test import RequestFactory, TestCase
 
 from circuits.models import *
+from core.models import ObjectChange
 from dcim.models import *
 from dcim.utils import object_to_path_node
+from netbox.context_managers import event_tracking
+from utilities.testing.utils import create_test_user
 
 __all__ = (
     'BaseCablePathTestCase',
@@ -101,3 +106,22 @@ class BaseCablePathTestCase(TestCase):
         if msg is None:
             msg = f"Path #{origin._path_id} set as origin on {origin}; should be None!"
         self.assertIsNone(origin._path_id, msg=msg)
+
+    def update_dependent_objects(self, obj):
+        """
+        Call update_dependent_objects() on a fresh copy of obj twice under change tracking, asserting that neither
+        call records an ObjectChange and that the second call leaves the CablePaths unchanged.
+        """
+        request = RequestFactory().get('/')
+        request.id = uuid.uuid4()
+        request.user = create_test_user()
+
+        def get_paths():
+            return sorted((p.path, p.is_complete, p.is_active, p.is_split) for p in CablePath.objects.all())
+
+        with event_tracking(request):
+            type(obj).objects.get(pk=obj.pk).update_dependent_objects()
+            paths = get_paths()
+            type(obj).objects.get(pk=obj.pk).update_dependent_objects()
+        self.assertEqual(get_paths(), paths)
+        self.assertFalse(ObjectChange.objects.filter(request_id=request.id).exists())

netbox/wireless/models.py

@@ -6,6 +6,7 @@ from django.utils.translation import gettext_lazy as _
 from dcim.choices import LinkStatusChoices
 from dcim.constants import WIRELESS_IFACE_TYPES
 from dcim.models.mixins import CachedScopeMixin
+from dcim.utils import create_cablepaths
 from netbox.models import NestedLtreeGroupModel, PrimaryModel
 from netbox.models.mixins import DistanceMixin
 
@@ -222,3 +223,18 @@ class WirelessLink(WirelessAuthenticationBase, DistanceMixin, PrimaryModel):
         self._interface_b_device = self.interface_b.device
 
         super().save(*args, **kwargs)
+
+    def update_dependent_objects(self):
+        """
+        Link both Interfaces to this WirelessLink and recreate the CablePaths which traverse it.
+        """
+        from dcim.models import Interface
+
+        Interface.objects.filter(pk__in=(self.interface_a_id, self.interface_b_id)).exclude(
+            wireless_link=self
+        ).update(wireless_link=self)
+        # Tracing identifies the far end by identity, so pass the instances held by this link
+        for interface in (self.interface_a, self.interface_b):
+            interface.wireless_link = self
+            create_cablepaths([interface])
+    update_dependent_objects.alters_data = True