From e864528795f9d938931b1c3143b93e3aecb59bb2 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 10 Sep 2026 11:27:03 -0700 Subject: [PATCH] Fix #23160 - Change-log the disconnect on terminating objects when a Cable is deleted --- netbox/dcim/models/cables.py | 24 ++++++++++++++++--- netbox/dcim/signals.py | 45 +++++++++++++++--------------------- 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/netbox/dcim/models/cables.py b/netbox/dcim/models/cables.py index 8ab9cafb2..5d7646da5 100644 --- a/netbox/dcim/models/cables.py +++ b/netbox/dcim/models/cables.py @@ -74,13 +74,29 @@ class CableBundle(PrimaryModel): # Cables # +class CableQuerySet(RestrictedQuerySet): + + def delete(self): + # Track these Cables as being deleted for the duration, as Cable.delete() does for a single + # instance: a queryset delete never calls it. Between them the two cover every deletion, as a + # Cable is never itself cascade-deleted (nothing points at it with on_delete=CASCADE). + pks = list(self.values_list('pk', flat=True)) + for pk in pks: + Cable._track_deletion(pk) + try: + return super().delete() + finally: + for pk in pks: + Cable._untrack_deletion(pk) + + class Cable(PrimaryModel): """ A physical connection between two endpoints. """ - # Per-thread tracking of Cable PKs currently in delete(); referenced by - # dcim.signals.nullify_connected_endpoints to skip per-CableTermination - # cable path retracing during cascade (retrace_cable_paths handles it once). + # Per-thread tracking of Cable PKs currently being deleted; referenced by + # dcim.signals.nullify_connected_endpoints to record the disconnect on each terminating object and + # to skip per-CableTermination path retracing during the cascade (retrace_cable_paths does it once). _deletion_tracking = threading.local() type = models.CharField( @@ -150,6 +166,8 @@ class Cable(PrimaryModel): clone_fields = ('tenant', 'type', 'profile', 'bundle') + objects = CableQuerySet.as_manager() + class Meta: ordering = ('pk',) verbose_name = _('cable') diff --git a/netbox/dcim/signals.py b/netbox/dcim/signals.py index 449aa7249..682e11721 100644 --- a/netbox/dcim/signals.py +++ b/netbox/dcim/signals.py @@ -2,7 +2,7 @@ import logging from django.db import transaction from django.db.models import Q -from django.db.models.signals import post_delete, post_save, pre_delete, pre_save +from django.db.models.signals import post_delete, post_save, pre_save from django.dispatch import receiver from dcim.choices import CableEndChoices, LinkStatusChoices @@ -306,24 +306,6 @@ def retrace_cable_paths(instance, **kwargs): cablepath.retrace() -@receiver(pre_delete, sender=Cable) -def track_cable_deletion(instance, **kwargs): - """ - Flag the Cable as being deleted for nullify_connected_endpoints() below, which runs for each of its - cascaded CableTerminations. Tracking here rather than only in Cable.delete() covers a queryset delete, - which never calls the model's delete() method. - """ - Cable._track_deletion(instance.pk) - - -@receiver(post_delete, sender=Cable) -def untrack_cable_deletion(instance, **kwargs): - # Registered after retrace_cable_paths() so that the flag is still set while it runs. A delete that raises - # between the two signals leaves the PK tracked; Cable.delete() clears it in a finally, and for a queryset - # delete the transaction rolls back with only this stale entry left behind. - Cable._untrack_deletion(instance.pk) - - @receiver((post_delete, post_save), sender=PortMapping) def update_passthrough_port_paths(instance, **kwargs): """ @@ -350,7 +332,13 @@ def nullify_connected_endpoints(instance, **kwargs): # propagate the cable back onto the channel subinterfaces we are about to clear. termination = None if Cable._is_being_deleted(instance.cable_id): - termination = model.objects.filter(pk=instance.termination_id).first() + # The change record serializes the object twice (before and after), and ComponentModel.save() + # re-caches its denormalized references off the parent device: fetch both up front so neither + # costs a round trip per terminating object. + queryset = model.objects.filter(pk=instance.termination_id).prefetch_related('tags') + if hasattr(model, 'device'): + queryset = queryset.select_related('device__site', 'device__location', 'device__rack') + termination = queryset.first() if termination is not None: termination.cable_id = instance.cable_id @@ -358,6 +346,7 @@ def nullify_connected_endpoints(instance, **kwargs): termination.cable_connector = instance.connector termination.cable_positions = instance.positions termination.snapshot() + # clear_cable_termination() also clears the mirrored attributes on any channel subinterfaces termination.clear_cable_termination(instance) update_fields = ['cable', 'cable_end', 'cable_connector', 'cable_positions', 'last_updated'] @@ -378,13 +367,15 @@ def nullify_connected_endpoints(instance, **kwargs): cable_positions=None, ) - # If the removed termination was a channelized interface, also clear the cable attributes mirrored onto its channel - # subinterfaces. This must happen before the retrace below so that each channel's (now dead) path is torn down - # rather than rebuilt from a stale cable reference. - if model is Interface: - Interface.objects.filter(parent_id=instance.termination_id, channel_id__isnull=False).update( - cable=None, cable_end='', cable_connector=None, cable_positions=None - ) + # If the removed termination was a channelized interface, also clear the cable attributes mirrored onto + # its channel subinterfaces. This must happen before the retrace below so that each channel's (now dead) + # path is torn down rather than rebuilt from a stale cable reference. These writes are deliberately not + # change-logged: propagate_channel_cables() doesn't log the mirrored attributes when it sets them + # either, so a channel subinterface has no recorded cable state for this to contradict. + if model is Interface: + Interface.objects.filter(parent_id=instance.termination_id, channel_id__isnull=False).update( + cable=None, cable_end='', cable_connector=None, cable_positions=None + ) # If the parent Cable is being deleted in this same operation, skip the # per-termination retrace; retrace_cable_paths() will retrace each affected