review feedback

docs/plugins/development/models.md

@@ -203,6 +203,7 @@ 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.
 
 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

@@ -512,6 +512,14 @@ class Cable(PrimaryModel):
         """
         Recreate the CablePaths traversing this Cable from its current terminations.
         """
+        a_terminations, b_terminations = self.get_terminations()
+
+        # 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()
+
         rebuild_cable_paths(self)
 
     def get_terminations(self):

netbox/dcim/tests/test_cablepaths.py

@@ -3040,11 +3040,12 @@ class CableDependentObjectsTestCase(BaseCablePathTestCase):
     Test Cable.update_dependent_objects(), which retraces the paths of a Cable written to the database
     by a process that bypasses save() (e.g. a tool replaying serialized changes).
     """
-    def _create_cable_raw(self, termination_a, termination_b):
+    def _create_cable_raw(self, termination_a, termination_b, status=LinkStatusChoices.STATUS_CONNECTED):
         """
-        Write a Cable and its terminations directly to the database, bypassing Cable.save().
+        Write a Cable and its terminations directly to the database, bypassing Cable.save(). Unprofiled
+        cables only: the connector & positions a profile assigns are not replicated here.
         """
-        cable = Cable(status=LinkStatusChoices.STATUS_CONNECTED)
+        cable = Cable(status=status)
         cable.save_base(raw=True)
 
         for termination, cable_end in (
@@ -3073,6 +3074,59 @@ class CableDependentObjectsTestCase(BaseCablePathTestCase):
         self.assertPathExists((interface2, cable, interface1), is_complete=True, is_active=True)
         self.assertEqual(CablePath.objects.count(), 2)
 
+    def test_retrace_extends_path_via_pass_through(self):
+        """
+        [IF1] --C1-- [FP1] [RP1] --C2-- [IF2], with C2 written raw. Retracing from a termination which is not
+        itself a path endpoint must extend the existing incomplete path.
+        """
+        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()
+        self.assertPathExists((interface1, cable1, frontport1, rearport1), is_complete=False)
+
+        cable2 = self._create_cable_raw(rearport1, interface2)
+        self.assertEqual(CablePath.objects.count(), 1)
+
+        cable2.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_takes_status_from_database(self):
+        """
+        A raw write leaves no in-memory record of the Cable's status, so path activity must come from the
+        stored value.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+
+        cable = self._create_cable_raw(interface1, interface2, status=LinkStatusChoices.STATUS_PLANNED)
+        cable.update_dependent_objects()
+
+        self.assertPathExists((interface1, cable, interface2), is_complete=True, is_active=False)
+        self.assertPathExists((interface2, cable, interface1), is_complete=True, is_active=False)
+        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

@@ -468,6 +468,44 @@ class ChannelizedCablePathTestCase(BaseCablePathTestCase):
         self.assertIsNone(channel.cable_positions)
         self.assertPathIsNotSet(channel)
 
+    def test_114_update_dependent_objects_restores_channel_paths(self):
+        """
+        Cable.update_dependent_objects() must remirror the parent's cable attributes onto its channel
+        subinterfaces before retracing. Those attributes are written by a bulk update and so are never
+        change-logged: a caller replaying serialized changes leaves them empty, and the retrace would
+        otherwise expand the channelized origin to nothing and create no paths at all.
+        """
+        parent, channels = self._create_channelized_interface('et0', 4)
+        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()
+        self.assertEqual(CablePath.objects.count(), 8)
+
+        # Reduce the cable to the state a replayed create leaves behind: no paths, and no mirrored cable
+        # attributes on the channel subinterfaces
+        for cablepath in CablePath.objects.all():
+            cablepath.delete()
+        Interface.objects.filter(channel_id__isnull=False).update(
+            cable=None, cable_end='', cable_connector=None, cable_positions=None
+        )
+
+        Cable.objects.get(pk=cable.pk).update_dependent_objects()
+
+        self.assertEqual(CablePath.objects.count(), 8)
+        for i, (channel, far_iface) in enumerate(zip(channels, far), start=1):
+            channel.refresh_from_db()
+            far_iface.refresh_from_db()
+            self.assertEqual(channel.cable_id, cable.pk)
+            self.assertEqual(channel.cable_positions, [i])
+            forward = self.assertPathExists((channel, cable, far_iface), is_complete=True, is_active=True)
+            reverse = self.assertPathExists((far_iface, cable, channel), is_complete=True, is_active=True)
+            self.assertPathIsSet(channel, forward)
+            self.assertPathIsSet(far_iface, reverse)
+
 
 class ChannelizedInterfaceTestCase(TestCase):
     """