fix update_cable_path

docs/plugins/development/models.md

@@ -195,6 +195,7 @@ class MyModel(NetBoxModel):
 
     def update_dependent_objects(self):
         # Recreate any objects derived from this one
+        ...
 ```
 
 The method is optional; callers should check for its presence before calling it. NetBox never calls it during a normal save.
@@ -203,7 +204,8 @@ Two constraints apply to an implementation:
 
 * It must derive its work entirely from the database. The in-memory state a normal `save()` relies on (which fields changed, for instance) is not available to a caller replaying serialized data.
 * It must be idempotent and safe to call when nothing needs to change, as a caller will generally invoke it for every object it has written.
-* Exceptions propagate to the caller unchanged. `Cable.update_dependent_objects()`, for instance, raises `UnsupportedCablePath` where `Cable.save()` converts it to `AbortRequest`: the hook is not tied to a request, so it is for the caller to decide how a failure is handled.
+
+Exceptions raised by an implementation propagate to the caller unchanged: `Cable.update_dependent_objects()` raises `UnsupportedCablePath` where `Cable.save()` converts it to `AbortRequest`. The hook is not tied to a request, so it is for the caller to decide how a failure is handled.
 
 The caller is responsible for calling the method only once every related object is in place: `Cable.update_dependent_objects()` retraces the cable's paths, which requires its `CableTermination` objects to exist.
 

netbox/dcim/models/cables.py

@@ -9,7 +9,7 @@ from django.contrib.postgres.fields import ArrayField
 from django.contrib.postgres.indexes import GinIndex
 from django.core.exceptions import ValidationError
 from django.core.validators import MaxValueValidator, MinValueValidator
-from django.db import models, router
+from django.db import models, router, transaction
 from django.dispatch import Signal
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
@@ -30,7 +30,7 @@ from utilities.querysets import RestrictedQuerySet, chunked_update
 from utilities.serialization import deserialize_object, serialize_object
 from wireless.models import WirelessLink
 
-from .device_components import FrontPort, PathEndpoint, PortMapping, RearPort
+from .device_components import FrontPort, Interface, PathEndpoint, PortMapping, RearPort
 
 __all__ = (
     'Cable',
@@ -512,15 +512,15 @@ class Cable(PrimaryModel):
         """
         Recreate the CablePaths traversing this Cable from its current terminations.
         """
-        a_terminations, b_terminations = self.get_terminations()
+        with transaction.atomic(using=router.db_for_write(CablePath)):
 
-        # A channelized parent mirrors its cable attributes onto its channel subinterfaces with a bulk write,
-        # which emits no change record: remirror them, or the retrace below expands the parent to nothing
-        for termination in (*a_terminations, *b_terminations):
-            if getattr(termination, 'channels', None):
-                termination.propagate_channel_cables()
+            # A channelized parent mirrors its cable attributes onto its channel subinterfaces with a bulk
+            # write, which emits no change record: remirror them, or the retrace expands it to nothing
+            for ct in CableTermination.objects.filter(cable=self).prefetch_related('termination'):
+                if isinstance(ct.termination, Interface) and ct.termination.channels:
+                    ct.termination.propagate_channel_cables()
 
-        rebuild_cable_paths(self)
+            rebuild_cable_paths(self)
 
     def get_terminations(self):
         """

netbox/dcim/tests/test_cablepaths.py

@@ -3127,6 +3127,124 @@ class CableDependentObjectsTestCase(BaseCablePathTestCase):
         self.assertPathExists((interface2, cable, interface1), is_complete=True, is_active=False)
         self.assertEqual(CablePath.objects.count(), 2)
 
+    def test_retrace_preserves_path_through_pass_through(self):
+        """
+        [IF1] --C1-- [FP1] [RP1] --C2-- [IF2]. Retracing C1, whose B side is not a path endpoint, must
+        preserve the reverse path originating at IF2.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        rearport1 = RearPort.objects.create(device=self.device, name='Rear Port 1')
+        frontport1 = FrontPort.objects.create(device=self.device, name='Front Port 1')
+        PortMapping.objects.create(
+            device=self.device,
+            front_port=frontport1,
+            front_port_position=1,
+            rear_port=rearport1,
+            rear_port_position=1
+        )
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[frontport1])
+        cable1.save()
+        cable2 = Cable(a_terminations=[rearport1], b_terminations=[interface2])
+        cable2.save()
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        Cable.objects.get(pk=cable1.pk).update_dependent_objects()
+
+        self.assertPathExists(
+            (interface1, cable1, frontport1, rearport1, cable2, interface2),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertPathExists(
+            (interface2, cable2, rearport1, frontport1, cable1, interface1),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertEqual(CablePath.objects.count(), 2)
+
+    def test_retrace_preserves_paths_of_mid_span_cable(self):
+        """
+        [IF1] --C1-- [FP1] [RP1] --C2-- [RP2] [FP2] --C3-- [IF2]. Retracing C2, which originates nothing
+        itself (both sides are rear ports), must preserve both paths.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        ports = {}
+        for i in (1, 2):
+            ports[f'rear{i}'] = RearPort.objects.create(device=self.device, name=f'Rear Port {i}')
+            ports[f'front{i}'] = FrontPort.objects.create(device=self.device, name=f'Front Port {i}')
+            PortMapping.objects.create(
+                device=self.device,
+                front_port=ports[f'front{i}'],
+                front_port_position=1,
+                rear_port=ports[f'rear{i}'],
+                rear_port_position=1
+            )
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[ports['front1']])
+        cable1.save()
+        cable2 = Cable(a_terminations=[ports['rear1']], b_terminations=[ports['rear2']])
+        cable2.save()
+        cable3 = Cable(a_terminations=[ports['front2']], b_terminations=[interface2])
+        cable3.save()
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        # Twice: with no path endpoint of its own, every path this Cable carries is restored from the
+        # origins of the paths it replaces, so a repeat call must neither duplicate nor drop them
+        for _ in range(2):
+            Cable.objects.get(pk=cable2.pk).update_dependent_objects()
+
+        self.assertPathExists(
+            (
+                interface1, cable1, ports['front1'], ports['rear1'], cable2, ports['rear2'], ports['front2'],
+                cable3, interface2
+            ),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertPathExists(
+            (
+                interface2, cable3, ports['front2'], ports['rear2'], cable2, ports['rear1'], ports['front1'],
+                cable1, interface1
+            ),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertEqual(CablePath.objects.count(), 2)
+
+    def test_retrace_preserves_path_through_circuit(self):
+        """
+        [IF1] --C1-- [CT1] [CT2] --C2-- [IF2]. Retracing C1, whose B side is a circuit termination, must
+        preserve the reverse path originating at IF2.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        circuittermination1 = CircuitTermination.objects.create(
+            circuit=self.circuit, termination=self.site, term_side='A'
+        )
+        circuittermination2 = CircuitTermination.objects.create(
+            circuit=self.circuit, termination=self.site, term_side='Z'
+        )
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[circuittermination1])
+        cable1.save()
+        cable2 = Cable(a_terminations=[circuittermination2], b_terminations=[interface2])
+        cable2.save()
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        Cable.objects.get(pk=cable1.pk).update_dependent_objects()
+
+        self.assertPathExists(
+            (interface1, cable1, circuittermination1, circuittermination2, cable2, interface2),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertPathExists(
+            (interface2, cable2, circuittermination2, circuittermination1, cable1, interface1),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertEqual(CablePath.objects.count(), 2)
+
     def test_retrace_is_idempotent(self):
         interface1 = Interface.objects.create(device=self.device, name='Interface 1')
         interface2 = Interface.objects.create(device=self.device, name='Interface 2')

netbox/dcim/tests/test_channelization.py

@@ -19,9 +19,12 @@ from dcim.models import (
     Device,
     DeviceRole,
     DeviceType,
+    FrontPort,
     Interface,
     InterfaceTemplate,
     Manufacturer,
+    PortMapping,
+    RearPort,
     Site,
 )
 from dcim.svg import CableTraceSVG
@@ -506,6 +509,45 @@ class ChannelizedCablePathTestCase(BaseCablePathTestCase):
             self.assertPathIsSet(channel, forward)
             self.assertPathIsSet(far_iface, reverse)
 
+    def test_115_channelizing_cabled_interface_preserves_far_side_path(self):
+        """
+        [IF1] --C1-- [FP1] [RP1] --C2-- [IF2]. Channelizing IF1 retraces C1's paths; the path originating at
+        IF2 traverses C1 without terminating to it, and must survive the retrace.
+        """
+        interface1 = Interface.objects.create(
+            device=self.device, name='et0', type=InterfaceTypeChoices.TYPE_40GE_QSFP_PLUS
+        )
+        interface2 = Interface.objects.create(
+            device=self.device, name='xe0', type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS
+        )
+        rearport = RearPort.objects.create(device=self.device, name='rear')
+        frontport = FrontPort.objects.create(device=self.device, name='front')
+        PortMapping.objects.create(
+            device=self.device,
+            front_port=frontport,
+            front_port_position=1,
+            rear_port=rearport,
+            rear_port_position=1
+        )
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[frontport])
+        cable1.save()
+        cable2 = Cable(a_terminations=[rearport], b_terminations=[interface2])
+        cable2.save()
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        # Channelize the near-end interface. It originates no path of its own from here on, but the far end's
+        # path to it is unaffected. Must be refetched: the in-memory instance predates the cable.
+        interface1.refresh_from_db()
+        interface1.channels = 4
+        interface1.save()
+
+        self.assertPathExists(
+            (interface2, cable2, rearport, frontport, cable1, interface1),
+            is_complete=True,
+            is_active=True
+        )
+        self.assertEqual(CablePath.objects.count(), 1)
+
 
 class ChannelizedInterfaceTestCase(TestCase):
     """

netbox/dcim/utils.py

@@ -177,32 +177,51 @@ def rebuild_paths(terminations):
 
 def rebuild_cable_paths(cable):
     """
-    Delete and rebuild every CablePath traversing the given Cable, tracing freshly from the Cable's current
-    terminations in both directions. Used when the channelization of a terminated interface changes (e.g. a channel
-    subinterface is added, moved, or removed) without the Cable itself being modified.
+    Delete and rebuild every CablePath affected by the given Cable, tracing freshly from the Cable's current
+    terminations and from the origins of the affected paths. Used when a Cable's connectivity must be reconciled
+    without its own save() having traced it: the channelization of a terminated interface has changed, or the Cable
+    was written by a process which bypasses save().
     """
     from dcim.choices import CableEndChoices
     from dcim.models import CablePath, CableTermination, PathEndpoint
 
     with transaction.atomic(using=router.db_for_write(CablePath)):
-        # Delete existing paths individually so each clears its `_path` back-reference on the originating endpoints.
-        for cp in CablePath.objects.filter(_nodes__contains=cable):
-            cp.delete()
-
         a_terminations, b_terminations = [], []
-        for ct in CableTermination.objects.filter(cable=cable):
+        for ct in CableTermination.objects.filter(cable=cable).prefetch_related('termination'):
             if ct.cable_end == CableEndChoices.SIDE_A:
                 a_terminations.append(ct.termination)
             else:
                 b_terminations.append(ct.termination)
 
+        # Every path traversing the Cable, plus those traversing a termination which is not itself a path endpoint:
+        # the latter may not reach the Cable yet (e.g. an incomplete path through a pass-through port which this
+        # Cable completes).
+        affected = {cp.pk: cp for cp in CablePath.objects.filter(_nodes__contains=cable)}
+        for termination in (*a_terminations, *b_terminations):
+            if not isinstance(termination, PathEndpoint):
+                affected.update({cp.pk: cp for cp in CablePath.objects.filter(_nodes__contains=termination)})
+
+        # Record each affected path's originating node(s) before deleting it. A path which merely passes through
+        # the Cable originates elsewhere, and can only be retraced from its own origins.
+        origins = {tuple(cp.path[0]): cp.origins for cp in affected.values()}
+
+        # Delete existing paths individually so each clears its `_path` back-reference on the originating endpoints.
+        for cp in affected.values():
+            cp.delete()
+
+        # Trace from the Cable's own terminations first, so that a channelized origin is expanded into its channel
+        # subinterfaces exactly once
+        retraced = set()
         for nodes in (a_terminations, b_terminations):
-            if not nodes:
-                continue
-            if isinstance(nodes[0], PathEndpoint):
+            if nodes and isinstance(nodes[0], PathEndpoint):
+                create_cablepaths(nodes)
+                retraced.add(tuple(object_to_path_node(node) for node in nodes))
+
+        # Restore any affected path the tracing above did not reproduce
+        retraced |= {tuple(cp.path[0]) for cp in CablePath.objects.filter(_nodes__contains=cable)}
+        for key, nodes in origins.items():
+            if key not in retraced:
                 create_cablepaths(nodes)
-            else:
-                rebuild_paths(nodes)
 
 
 def update_interface_parents(device, interface_templates, module=None):