From 6e30c11017704052b45325a96bbebb97997e2355 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 25 Jul 2025 17:07:17 -0400 Subject: [PATCH] Fixes #19956: Prevent duplicate deletion records from cascading deletions --- netbox/core/signals.py | 24 ++++++++++++++++++++++ netbox/core/tests/test_changelog.py | 32 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/netbox/core/signals.py b/netbox/core/signals.py index 8ba8cc244..3d0317011 100644 --- a/netbox/core/signals.py +++ b/netbox/core/signals.py @@ -1,10 +1,12 @@ import logging +from threading import local from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.db.models.fields.reverse_related import ManyToManyRel, ManyToOneRel from django.db.models.signals import m2m_changed, post_save, pre_delete from django.dispatch import receiver, Signal +from django.core.signals import request_finished from django.utils.translation import gettext_lazy as _ from django_prometheus.models import model_deletes, model_inserts, model_updates @@ -42,6 +44,10 @@ clear_events = Signal() # Change logging & event handling # +# Used to track received signals per object +_signals_received = local() + + @receiver((post_save, m2m_changed)) def handle_changed_object(sender, instance, **kwargs): """ @@ -130,6 +136,16 @@ def handle_deleted_object(sender, instance, **kwargs): if request is None: return + # Check whether we've already processed a pre_delete signal for this object. (This can + # happen e.g. when both a parent object and its child are deleted simultaneously, due + # to cascading deletion.) + if not hasattr(_signals_received, 'pre_delete'): + _signals_received.pre_delete = set() + signature = (ContentType.objects.get_for_model(instance), instance.pk) + if signature in _signals_received.pre_delete: + return + _signals_received.pre_delete.add(signature) + # Record an ObjectChange if applicable if hasattr(instance, 'to_objectchange'): if hasattr(instance, 'snapshot') and not getattr(instance, '_prechange_snapshot', None): @@ -179,6 +195,14 @@ def handle_deleted_object(sender, instance, **kwargs): model_deletes.labels(instance._meta.model_name).inc() +@receiver(request_finished) +def clear_signal_history(sender, **kwargs): + """ + Clear out the signals history once the request is finished. + """ + _signals_received.pre_delete = set() + + @receiver(clear_events) def clear_events_queue(sender, **kwargs): """ diff --git a/netbox/core/tests/test_changelog.py b/netbox/core/tests/test_changelog.py index df8461076..4a00e4a25 100644 --- a/netbox/core/tests/test_changelog.py +++ b/netbox/core/tests/test_changelog.py @@ -346,6 +346,38 @@ class ChangeLogViewTest(ModelViewTestCase): self.assertEqual(changes[1].changed_object_type, ContentType.objects.get_for_model(Interface)) self.assertEqual(changes[2].changed_object_type, ContentType.objects.get_for_model(Device)) + def test_duplicate_deletions(self): + """ + Check that a cascading deletion event does not generate multiple "deleted" ObjectChange records for + the same object. + """ + role1 = DeviceRole(name='Role 1', slug='role-1') + role1.save() + role2 = DeviceRole(name='Role 2', slug='role-2', parent=role1) + role2.save() + pk_list = [role1.pk, role2.pk] + + # Delete both objects simultaneously + form_data = { + 'pk': pk_list, + 'confirm': True, + '_confirm': True, + } + request = { + 'path': reverse('dcim:devicerole_bulk_delete'), + 'data': post_data(form_data), + } + self.add_permissions('dcim.delete_devicerole') + self.assertHttpStatus(self.client.post(**request), 302) + + # This should result in exactly one change record per object + objectchanges = ObjectChange.objects.filter( + changed_object_type=ContentType.objects.get_for_model(DeviceRole), + changed_object_id__in=pk_list, + action=ObjectChangeActionChoices.ACTION_DELETE + ) + self.assertEqual(objectchanges.count(), 2) + class ChangeLogAPITest(APITestCase):