Browse Source

fix(dcim): Retire superseded Cable Paths on Termination changes

Delete any path already originating at a node before tracing a fresh
one from it, so that replacing, extending or recreating a Cable's
Terminations no longer leaves the rows it supersedes behind. Repeated
changes now converge on the paths the current terminations trace.

Retire from the expanded origins so that a channelized end retires its
channels' paths rather than the parent's, which has none, and write the
replacements under a transaction so a failed trace cannot leave an
origin with neither its old path nor a new one.

Add regression coverage for replaced, extended and channelized ends and
for repeated identical REST writes.

Fixes #23121
Martin Hauser 7 hours ago
parent
commit
cf14a2573f

+ 28 - 3
netbox/dcim/management/commands/trace_paths.py

@@ -3,8 +3,17 @@ from django.core.management.color import no_style
 from django.db import connection
 from django.db.models import Q
 
-from dcim.models import CablePath, ConsolePort, ConsoleServerPort, Interface, PowerFeed, PowerOutlet, PowerPort
-from dcim.signals import create_cablepaths
+from dcim.models import (
+    CablePath,
+    CableTermination,
+    ConsolePort,
+    ConsoleServerPort,
+    Interface,
+    PowerFeed,
+    PowerOutlet,
+    PowerPort,
+)
+from dcim.utils import create_cablepaths
 
 ENDPOINT_MODELS = (
     ConsolePort,
@@ -80,8 +89,24 @@ class Command(BaseCommand):
                 continue
             self.stdout.write(f'Retracing {origins_count} cabled {model._meta.verbose_name_plural}...')
             i = 0
+            # Retrace each cable end as a group, so a repair preserves its shared origins instead of splitting them
+            seen = set()
             for i, obj in enumerate(origins, start=1):
-                create_cablepaths([obj])
+                # cable_end on the endpoint is cached and can drift, so resolve the end from its termination
+                group = [obj]
+                key = None
+                if obj.cable_id and (termination := obj.cable_terminations.first()):
+                    key = (termination.cable_id, termination.cable_end)
+                    if key in seen:
+                        continue
+                    group = [
+                        ct.termination for ct in CableTermination.objects.filter(
+                            cable_id=termination.cable_id, cable_end=termination.cable_end
+                        ).prefetch_related('termination')
+                    ]
+                create_cablepaths(group)
+                if key is not None:
+                    seen.add(key)
                 if not i % 100:
                     self.draw_progress_bar(i * 100 / origins_count)
             self.draw_progress_bar(100)

+ 23 - 0
netbox/dcim/tests/test_api.py

@@ -4741,6 +4741,29 @@ class CableTestCase(APIViewTestCases.APIViewTestCase):
             },
         ]
 
+    def test_repeated_put_does_not_accumulate_paths(self):
+        """
+        Repeating an identical PUT must leave the cable with the two paths its terminations trace.
+        """
+        self.add_permissions('dcim.change_cable')
+        cable = Cable.objects.get(label='Cable 1')
+        interface_a = Interface.objects.get(cable=cable, cable_end=CableEndChoices.SIDE_A)
+        interface_b = Interface.objects.get(cable=cable, cable_end=CableEndChoices.SIDE_B)
+        data = {
+            'status': cable.status,
+            'a_terminations': [{'object_type': 'dcim.interface', 'object_id': interface_a.pk}],
+            'b_terminations': [{'object_type': 'dcim.interface', 'object_id': interface_b.pk}],
+        }
+
+        for attempt in range(3):
+            with self.subTest(attempt=attempt):
+                response = self.client.put(self._get_detail_url(cable), data, format='json', **self.header)
+
+                self.assertHttpStatus(response, status.HTTP_200_OK)
+                for interface in (interface_a, interface_b):
+                    self.assertTrue(Interface.objects.get(pk=interface.pk)._path.is_complete)
+                self.assertEqual(CablePath.objects.filter(_nodes__contains=cable).count(), 2)
+
     def test_graphql_cable_termination_cached_filters(self):
         """
         Validate filtering cables by cached CableTermination relations via GraphQL:

+ 307 - 1
netbox/dcim/tests/test_cablepaths.py

@@ -1,8 +1,13 @@
+from io import StringIO
+
+from django.core.management import call_command
+
 from circuits.models import *
-from dcim.choices import LinkStatusChoices
+from dcim.choices import CableEndChoices, LinkStatusChoices
 from dcim.models import *
 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
 
 
@@ -2891,6 +2896,307 @@ class LegacyCablePathTestCase(BaseCablePathTestCase):
         # Verify _path is cleared on removed interface (#21127)
         interface3.refresh_from_db()
         self.assertPathIsNotSet(interface3)
+        self.assertEqual(CablePath.objects.count(), 2)
+
+    def test_304_replacing_a_termination_retires_superseded_paths(self):
+        """
+        [IF1] --C1-- [IF2] becomes [IF1] --C1-- [IF3], and back again
+
+        Each replacement must leave only the two paths the cable's current terminations trace.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        interface3 = Interface.objects.create(device=self.device, name='Interface 3')
+
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[interface2])
+        cable1.save()
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        for peer, detached in ((interface3, interface2), (interface2, interface3)):
+            with self.subTest(peer=peer.name):
+                cable1 = Cable.objects.get(pk=cable1.pk)
+                cable1.b_terminations = [peer]
+                cable1.full_clean()
+                cable1.save()
+
+                self.assertCurrentPathExists((interface1, cable1, peer), is_complete=True, is_active=True)
+                self.assertCurrentPathExists((peer, cable1, interface1), is_complete=True, is_active=True)
+                self.assertEqual(CablePath.objects.count(), 2)
+                detached.refresh_from_db()
+                self.assertIsNone(detached.cable)
+                self.assertPathIsNotSet(detached)
+
+    def test_305_adding_a_termination_retires_superseded_paths(self):
+        """
+        [IF1] --C1-- [IF2] gains a second B-side termination [IF3]
+
+        Extending an end must retire the paths whose destinations the extension supersedes.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        interface3 = Interface.objects.create(device=self.device, name='Interface 3')
+
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[interface2])
+        cable1.save()
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        cable1 = Cable.objects.get(pk=cable1.pk)
+        cable1.b_terminations = [interface2, interface3]
+        cable1.full_clean()
+        cable1.save()
+
+        self.assertCurrentPathExists(
+            (interface1, cable1, [interface2, interface3]), is_complete=True, is_active=True
+        )
+        path2 = self.assertPathExists(
+            ([interface2, interface3], cable1, interface1), is_complete=True, is_active=True
+        )
+        for interface in (interface2, interface3):
+            interface.refresh_from_db()
+            self.assertPathIsSet(interface, path2)
+        self.assertEqual(CablePath.objects.count(), 2)
+
+    def test_306_retracing_one_joint_origin_restores_the_whole_hop(self):
+        """
+        [IF1] --C1-- [IF2]
+                     [IF3]
+
+        trace_paths retraces a cable end as a unit, so repairing one co-origin restores the joint hop.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        interface3 = Interface.objects.create(device=self.device, name='Interface 3')
+
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[interface2, interface3])
+        cable1.save()
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        # trace_paths selects on a null _path
+        Interface.objects.filter(pk=interface2.pk).update(_path=None)
+
+        call_command('trace_paths', no_input=True, stdout=StringIO())
+
+        for interface in (interface1, interface2, interface3):
+            interface.refresh_from_db()
+            self.assertIsNotNone(interface._path_id, msg=f'{interface} left without a path')
+        self.assertPathExists(([interface2, interface3], cable1, interface1))
+        self.assertEqual(CablePath.objects.count(), 2)
+
+    def test_307_retracing_a_cable_end_retires_a_superseded_co_origin_path(self):
+        """
+        [IF1] --C1-- [IF2]
+                     [IF3]
+
+        A path superseded at a co-origin of the retraced end is retired, not left beside its replacement.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        interface3 = Interface.objects.create(device=self.device, name='Interface 3')
+
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[interface2, interface3])
+        cable1.save()
+        self.assertEqual(CablePath.objects.count(), 2)
+
+        # Seed the stale row a pre-fix install would hold, then restore both origins to the shared path
+        interface3 = Interface.objects.get(pk=interface3.pk)
+        joint_path = interface3._path
+        superseded = CablePath.from_origin([interface3])
+        superseded.save()
+        joint_path.save()
+
+        # trace_paths selects on a null _path
+        Interface.objects.filter(pk=interface2.pk).update(_path=None)
+
+        call_command('trace_paths', no_input=True, stdout=StringIO())
+
+        self.assertFalse(
+            CablePath.objects.filter(pk=superseded.pk).exists(), msg='the superseded path survived the retrace'
+        )
+        self.assertPathExists(([interface2, interface3], cable1, interface1))
+        self.assertEqual(CablePath.objects.count(), 2)
+
+    def test_308_retracing_a_partial_origin_group_retires_its_co_origins_paths(self):
+        """
+        Retracing one origin of a shared path must also replace its co-origin's stale paths.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        interface3 = Interface.objects.create(device=self.device, name='Interface 3')
+        cable = Cable(a_terminations=[interface1], b_terminations=[interface2, interface3])
+        cable.save()
+
+        interface3.refresh_from_db()
+        joint_path = interface3._path
+        superseded = CablePath.from_origin([interface3])
+        superseded.save()
+        joint_path.save()
+
+        interface2.refresh_from_db()
+        create_cablepaths([interface2])
+
+        self.assertFalse(CablePath.objects.filter(pk=superseded.pk).exists())
+        self.assertFalse(CablePath.objects.filter(pk=joint_path.pk).exists())
+        self.assertCurrentPathExists((interface2, cable, interface1), is_complete=True)
+        self.assertCurrentPathExists((interface3, cable, interface1), is_complete=True)
+        self.assertCurrentPathExists((interface1, cable, [interface2, interface3]), is_complete=True)
+        self.assertEqual(CablePath.objects.count(), 3)
+
+    def test_309_retracing_preserves_another_current_origin_group(self):
+        """
+        An origin group whose references the deletion does not clear is left intact, rows and all.
+        """
+        interfaces = [
+            Interface.objects.create(device=self.device, name=f'Interface {i}') for i in range(1, 5)
+        ]
+        interface1, interface2, interface3, interface4 = interfaces
+        cable = Cable(a_terminations=[interface1], b_terminations=interfaces[1:])
+        cable.save()
+        for interface in interfaces:
+            interface.refresh_from_db()
+
+        # Overlapping groups from earlier partial retraces, with interface3 and interface4 on the second
+        interface2._path.delete()
+        first = CablePath.from_origin([interface2, interface3])
+        first.save()
+        second = CablePath.from_origin([interface3, interface4])
+        second.save()
+
+        create_cablepaths([interface2])
+
+        self.assertFalse(CablePath.objects.filter(pk=first.pk).exists())
+        self.assertCurrentPathExists((interface2, cable, interface1), is_complete=True)
+        self.assertTrue(
+            CablePath.objects.filter(pk=second.pk).exists(), msg='the untouched origin group was replaced'
+        )
+        second.refresh_from_db()
+        self.assertEqual(second.path[0], [object_to_path_node(interface3), object_to_path_node(interface4)])
+        for interface in (interface3, interface4):
+            interface.refresh_from_db()
+            self.assertPathIsSet(interface, second)
+        self.assertEqual(CablePath.objects.count(), 3)
+
+    def test_313_recovery_follows_a_chain_of_cleared_references(self):
+        """
+        Recovering an origin can clear another group's references, whose origins are recovered in turn.
+        """
+        interfaces = [
+            Interface.objects.create(device=self.device, name=f'Interface {i}') for i in range(1, 5)
+        ]
+        interface1, interface2, interface3, interface4 = interfaces
+        cable = Cable(a_terminations=[interface1], b_terminations=interfaces[1:])
+        cable.save()
+        for interface in interfaces:
+            interface.refresh_from_db()
+
+        # interface3 references the first group, so losing it starts the chain
+        interface2._path.delete()
+        first = CablePath.from_origin([interface2, interface3])
+        first.save()
+        second = CablePath.from_origin([interface3, interface4])
+        second.save()
+        first.save()
+
+        create_cablepaths([interface2])
+
+        self.assertFalse(CablePath.objects.filter(pk__in=[first.pk, second.pk]).exists())
+        for interface in interfaces[1:]:
+            self.assertCurrentPathExists((interface, cable, interface1), is_complete=True)
+        self.assertEqual(CablePath.objects.count(), 4)
+
+    def test_310_recovery_preserves_the_requested_connector_groups(self):
+        """
+        A recovered group must not replace a connector group already rebuilt by this call.
+        """
+        interfaces = [
+            Interface.objects.create(device=self.device, name=f'Interface {i}') for i in range(1, 5)
+        ]
+        interface1, interface2, interface3, interface4 = interfaces
+        cable = Cable(a_terminations=[interface1], b_terminations=interfaces[1:])
+        cable.save()
+        for interface in interfaces:
+            interface.refresh_from_db()
+
+        CablePath.from_origin([interface2, interface3]).save()
+        # Exercise helper grouping without changing the persisted termination topology.
+        interface2.cable_connector = 1
+        interface3.cable_connector = 2
+        interface4.cable_connector = 2
+
+        create_cablepaths([interface2, interface3, interface4])
+
+        self.assertCurrentPathExists((interface2, cable, interface1), is_complete=True)
+        shared_path = self.assertPathExists(([interface3, interface4], cable, interface1), is_complete=True)
+        for interface in (interface3, interface4):
+            interface.refresh_from_db()
+            self.assertPathIsSet(interface, shared_path)
+        self.assertEqual(CablePath.objects.count(), 3)
+
+    def test_311_recovery_separates_origins_on_different_links(self):
+        """
+        A stale origin hop holding origins since moved apart is recovered as one group per current link.
+        """
+        interfaces = [
+            Interface.objects.create(device=self.device, name=f'Interface {i}') for i in range(1, 7)
+        ]
+        interface1, interface2, interface3, interface4, interface5, interface6 = interfaces
+        cable1 = Cable(
+            a_terminations=[interface1],
+            b_terminations=[interface2, interface3, interface4, interface5],
+        )
+        cable1.save()
+
+        # The hop as stored while all four shared cable1
+        interface2.refresh_from_db()
+        stale_nodes = interface2._path.path
+
+        # Move one origin through ordinary saves, so only the CablePath below is stale
+        cable1.b_terminations = [interface2, interface3, interface4]
+        cable1.save()
+        interface5.refresh_from_db()
+        cable2 = Cable(a_terminations=[interface6], b_terminations=[interface5])
+        cable2.save()
+
+        interface2.refresh_from_db()
+        interface5.refresh_from_db()
+        current_cable1 = interface2._path
+        current_cable2 = interface5._path
+
+        # Leave the obsolete row as their only originating path, so recovery gets the whole mixed hop
+        current_cable1.delete()
+        current_cable2.delete()
+        superseded = CablePath(path=stale_nodes, is_complete=True, is_active=True)
+        superseded.save()
+
+        create_cablepaths([Interface.objects.get(pk=interface2.pk)])
+
+        self.assertFalse(CablePath.objects.filter(pk=superseded.pk).exists())
+        self.assertCurrentPathExists((interface2, cable1, interface1), is_complete=True)
+        # The origins still sharing cable1 stay one group, and the moved one is recovered on its own cable
+        shared = self.assertPathExists(([interface3, interface4], cable1, interface1), is_complete=True)
+        for interface in (interface3, interface4):
+            interface.refresh_from_db()
+            self.assertPathIsSet(interface, shared)
+        self.assertCurrentPathExists((interface5, cable2, interface6), is_complete=True)
+        self.assertEqual(CablePath.objects.count(), 5)
+
+    def test_312_retracing_repairs_an_endpoint_whose_cable_end_drifted(self):
+        """
+        An endpoint whose denormalized cable_end no longer matches its CableTermination is still retraced.
+        """
+        interface1 = Interface.objects.create(device=self.device, name='Interface 1')
+        interface2 = Interface.objects.create(device=self.device, name='Interface 2')
+        cable1 = Cable(a_terminations=[interface1], b_terminations=[interface2])
+        cable1.save()
+
+        # The termination row still says B, which is the drift this command exists to repair
+        Interface.objects.filter(pk=interface2.pk).update(
+            _path=None, cable_end=CableEndChoices.SIDE_A
+        )
+
+        call_command('trace_paths', no_input=True, stdout=StringIO())
+
+        self.assertCurrentPathExists((interface2, cable1, interface1), is_complete=True)
+        self.assertEqual(CablePath.objects.count(), 2)
 
     def test_401_exclude_midspan_devices(self):
         """

+ 65 - 0
netbox/dcim/tests/test_cablepaths2.py

@@ -2785,3 +2785,68 @@ class CablePathTestCase(BaseCablePathTestCase):
             set(CableTermination.objects.filter(cable=cable1).values_list('pk', flat=True)),
             termination_pks
         )
+
+    def test_311_moving_a_midspan_termination_preserves_far_end_paths(self):
+        """
+        [IF1] --C1-- [FP1][RP1] --C3-- [RP2][FP2] --C2-- [IF2]
+        becomes
+        [IF1] --C1-- [FP1][RP1] --C3-- [RP3][FP3] --C4-- [IF3]
+
+        Rear ports originate no path, so moving a mid-span cable's end must retrace the rows which traverse it
+        rather than delete them: their origins cannot be recovered from the cable's own terminations.
+        """
+        interfaces = [
+            Interface.objects.create(device=self.device, name=f'Interface {i}') for i in range(1, 4)
+        ]
+        rear_ports = [
+            RearPort.objects.create(device=self.device, name=f'Rear Port {i}') for i in range(1, 4)
+        ]
+        front_ports = [
+            FrontPort.objects.create(device=self.device, name=f'Front Port {i}') for i in range(1, 4)
+        ]
+        for front_port, rear_port in zip(front_ports, rear_ports):
+            PortMapping.objects.create(
+                device=self.device,
+                front_port=front_port,
+                front_port_position=1,
+                rear_port=rear_port,
+                rear_port_position=1
+            )
+
+        cable1 = Cable(a_terminations=[interfaces[0]], b_terminations=[front_ports[0]])
+        cable1.clean()
+        cable1.save()
+        cable2 = Cable(a_terminations=[front_ports[1]], b_terminations=[interfaces[1]])
+        cable2.clean()
+        cable2.save()
+        cable3 = Cable(a_terminations=[rear_ports[0]], b_terminations=[rear_ports[1]])
+        cable3.clean()
+        cable3.save()
+        cable4 = Cable(a_terminations=[front_ports[2]], b_terminations=[interfaces[2]])
+        cable4.clean()
+        cable4.save()
+
+        before = (
+            interfaces[0], cable1, front_ports[0], rear_ports[0], cable3, rear_ports[1], front_ports[1], cable2,
+            interfaces[1],
+        )
+        self.assertCurrentPathExists(before, is_complete=True, is_active=True)
+        self.assertCurrentPathExists(tuple(reversed(before)), is_complete=True, is_active=True)
+        # Two for the completed link, one for the third interface stopping at its own rear port
+        self.assertEqual(CablePath.objects.count(), 3)
+
+        cable3 = Cable.objects.get(pk=cable3.pk)
+        cable3.b_terminations = [rear_ports[2]]
+        cable3.full_clean()
+        cable3.save()
+
+        after = (
+            interfaces[0], cable1, front_ports[0], rear_ports[0], cable3, rear_ports[2], front_ports[2], cable4,
+            interfaces[2],
+        )
+        self.assertCurrentPathExists(after, is_complete=True, is_active=True)
+        self.assertCurrentPathExists(tuple(reversed(after)), is_complete=True, is_active=True)
+        self.assertCurrentPathExists(
+            (interfaces[1], cable2, front_ports[1], rear_ports[1]), is_complete=False
+        )
+        self.assertEqual(CablePath.objects.count(), 3)

+ 84 - 0
netbox/dcim/tests/test_channelization.py

@@ -12,6 +12,7 @@ from rest_framework.test import APIClient
 from core.choices import ObjectChangeActionChoices
 from core.models import ObjectChange
 from dcim.choices import CableProfileChoices, InterfaceTypeChoices
+from dcim.exceptions import UnsupportedCablePath
 from dcim.filtersets import InterfaceFilterSet
 from dcim.models import (
     Cable,
@@ -27,6 +28,7 @@ from dcim.models import (
 from dcim.svg import CableTraceSVG
 from dcim.svg.cables import Connector
 from dcim.tests.utils import BaseCablePathTestCase
+from dcim.utils import create_cablepaths
 from users.constants import TOKEN_PREFIX
 from users.models import Token, User
 from utilities.ordering import naturalize_interface
@@ -468,6 +470,88 @@ class ChannelizedCablePathTestCase(BaseCablePathTestCase):
         self.assertIsNone(channel.cable_positions)
         self.assertPathIsNotSet(channel)
 
+    def test_114_replacing_a_far_end_retires_the_channels_superseded_paths(self):
+        """
+        Replacing one far-end interface of a breakout cable must leave one path per channel in each direction. The
+        channels are the real origins, so the rows the retrace supersedes can only be found after the expansion.
+        """
+        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)
+        ]
+        replacement = Interface.objects.create(
+            device=self.device, name='xe4', type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS
+        )
+
+        cable = Cable(
+            profile=CableProfileChoices.BREAKOUT_1C4P_4C1P,
+            a_terminations=[parent],
+            b_terminations=far,
+        )
+        cable.clean()
+        cable.save()
+        self.assertEqual(CablePath.objects.count(), 8)
+
+        cable = Cable.objects.get(pk=cable.pk)
+        cable.b_terminations = [*far[:3], replacement]
+        cable.clean()
+        cable.save()
+
+        self.assertEqual(CablePath.objects.count(), 8)
+        for channel, far_iface in zip(channels, [*far[:3], replacement]):
+            channel.refresh_from_db()
+            far_iface.refresh_from_db()
+            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)
+        far[3].refresh_from_db()
+        self.assertIsNone(far[3].cable)
+        self.assertPathIsNotSet(far[3])
+
+    def test_115_failed_channel_trace_restores_the_replaced_paths(self):
+        """
+        A trace that raises partway through a channelized end must leave every channel's stored path in place.
+        """
+        parent, channels = self._create_channelized_interface('et0', 2)
+        far = [
+            Interface.objects.create(device=self.device, name=f'xe{i}', type=InterfaceTypeChoices.TYPE_10GE_SFP_PLUS)
+            for i in range(2)
+        ]
+        cable = Cable(
+            profile=CableProfileChoices.BREAKOUT_1C2P_2C1P,
+            a_terminations=[parent],
+            b_terminations=far,
+        )
+        cable.clean()
+        cable.save()
+        self.assertEqual(CablePath.objects.count(), 4)
+
+        stored_paths = {}
+        for channel in channels:
+            channel.refresh_from_db()
+            stored_paths[channel.pk] = channel._path
+
+        traced = CablePath.from_origin
+
+        def failing_from_origin(terminations):
+            if terminations and terminations[0] == channels[1]:
+                # The first channel's path must already be replaced, or the rollback proves nothing
+                self.assertFalse(CablePath.objects.filter(pk=stored_paths[channels[0].pk].pk).exists())
+                raise UnsupportedCablePath('Simulated trace error')
+            return traced(terminations)
+
+        with mock.patch.object(CablePath, 'from_origin', side_effect=failing_from_origin):
+            with self.assertRaises(UnsupportedCablePath):
+                create_cablepaths([parent])
+
+        # Reaching a query at all proves the block kept its savepoint
+        self.assertEqual(CablePath.objects.count(), 4)
+        for channel in channels:
+            channel.refresh_from_db()
+            self.assertPathIsSet(channel, stored_paths[channel.pk])
+
 
 class ChannelizedInterfaceTestCase(TestCase):
     """

+ 2 - 2
netbox/dcim/tests/test_management_commands.py

@@ -49,7 +49,7 @@ class TracePathsTestCase(TestCase):
         self.assertIn('Finished.', out.getvalue())
 
     def test_retraces_missing_cabled_endpoint_path(self):
-        endpoint = object()
+        endpoint = SimpleNamespace(cable_id=None)
 
         class FakeQuerySet(list):
             def filter(self, *args, **kwargs):
@@ -81,7 +81,7 @@ class TracePathsTestCase(TestCase):
         self.assertIn('Finished.', out.getvalue())
 
     def test_progress_bar_drawn_every_100_endpoints(self):
-        endpoints = [object() for _ in range(100)]
+        endpoints = [SimpleNamespace(cable_id=None) for _ in range(100)]
 
         class FakeQuerySet(list):
             def filter(self, *args, **kwargs):

+ 15 - 0
netbox/dcim/tests/utils.py

@@ -64,6 +64,21 @@ class BaseCablePathTestCase(TestCase):
         cablepath = self._get_cablepath(nodes, **kwargs)
         self.assertIsNone(cablepath, msg='Unexpected CablePath found')
 
+    def assertCurrentPathExists(self, nodes, **kwargs):
+        """
+        Assert that the first node references a CablePath with the given route via _path, and return it.
+
+        :param nodes: Iterable of steps, the first being the originating path endpoint object
+        """
+        origin = type(nodes[0]).objects.get(pk=nodes[0].pk)
+        self.assertIsNotNone(origin._path_id, msg=f'No path set on originating endpoint {origin}')
+        # Matched on the route alone, so a flag mismatch does not report itself as a wrong route
+        cablepath = self._get_cablepath(nodes, pk=origin._path_id)
+        self.assertIsNotNone(cablepath, msg=f'Path #{origin._path_id} on {origin} does not match the expected route')
+        for attr, expected in kwargs.items():
+            self.assertEqual(getattr(cablepath, attr), expected, msg=f'Path #{cablepath.pk} on {origin}: {attr}')
+        return cablepath
+
     def assertPathIsSet(self, origin, cablepath, msg=None):
         """
         Assert that a specific CablePath instance is set as the path on the origin.

+ 61 - 13
netbox/dcim/utils.py

@@ -1,4 +1,4 @@
-from collections import defaultdict
+from collections import defaultdict, deque
 
 from django.apps import apps
 from django.contrib.contenttypes.models import ContentType
@@ -125,13 +125,64 @@ def path_node_to_object(repr):
     return ct.model_class().objects.filter(pk=object_id).first()
 
 
+def _replace_cablepaths(origin_groups):
+    """
+    Replace the path originating at each group of origins, preserving the co-origins of any path deleted
+    along the way.
+    """
+    from dcim.models import CablePath, PathEndpoint
+
+    # Filtering each group as it is popped stops a recovery from splitting one this call has already rebuilt
+    pending = deque(origin_groups)
+    processed = set()
+
+    # The savepoint must stay: Cable.save() turns UnsupportedCablePath into AbortRequest and callers keep querying
+    with transaction.atomic(using=router.db_for_write(CablePath)):
+        while pending:
+            origins = [obj for obj in pending.popleft() if object_to_path_node(obj) not in processed]
+            if not origins:
+                continue
+
+            # Trace first, so an unsupported topology raises before anything is deleted
+            path = CablePath.from_origin(origins)
+            nodes = {object_to_path_node(obj) for obj in origins}
+            processed.update(nodes)
+
+            # `overlap` takes the encoded nodes directly, and matches nothing for an empty set
+            for old_path in CablePath.objects.filter(_nodes__overlap=list(nodes)):
+                # `_nodes` matches a node anywhere in a path, including as another path's destination
+                if not old_path.path or not nodes.intersection(old_path.path[0]):
+                    continue
+
+                # Recover only what this delete strands, by link since from_origin() rejects a mixed hop
+                by_link = defaultdict(list)
+                for node in old_path.path[0]:
+                    if node in processed:
+                        continue
+                    origin = path_node_to_object(node)
+                    if origin is None:
+                        continue
+                    # Absence of the back-reference is not evidence that a pointer was cleared
+                    if isinstance(origin, PathEndpoint) and origin._path_id != old_path.pk:
+                        continue
+                    if link := origin.link:
+                        by_link[link].append(origin)
+                pending.extend(by_link.values())
+
+                old_path.delete()
+
+            if path:
+                path.save()
+
+
 def create_cablepaths(objects):
     """
-    Create CablePaths for all paths originating from the specified set of nodes.
+    Create CablePaths for all paths originating from the specified set of nodes, retiring any path which
+    already originates there.
 
     :param objects: Iterable of cabled objects (e.g. Interfaces)
     """
-    from dcim.models import CablePath, Interface
+    from dcim.models import Interface
 
     # Expand any channelized interface into its channel subinterfaces. A channelized parent originates no path of its
     # own; instead, each channel subinterface traces independently from the single connector position it occupies.
@@ -144,20 +195,17 @@ def create_cablepaths(objects):
         else:
             expanded.append(obj)
 
-    # Arrange objects by cable connector. All objects with a null connector are grouped together. Channel
-    # subinterfaces must each originate their own path, as sharing a connector would otherwise collapse a group of
-    # siblings into a single malformed path.
-    origins = defaultdict(list)
+    # A channel originates its own path, since sharing a connector would collapse siblings into one malformed path
+    origin_groups = []
+    connectors = defaultdict(list)
     for obj in expanded:
         if isinstance(obj, Interface) and obj.channel_id:
-            if cp := CablePath.from_origin([obj]):
-                cp.save()
+            origin_groups.append([obj])
         else:
-            origins[obj.cable_connector].append(obj)
+            connectors[obj.cable_connector].append(obj)
+    origin_groups.extend(connectors.values())
 
-    for connector, objects in origins.items():
-        if cp := CablePath.from_origin(objects):
-            cp.save()
+    _replace_cablepaths(origin_groups)
 
 
 def rebuild_paths(terminations):