2
0
Эх сурвалжийг харах

#22270 - Skip cascade-deleted objects when clearing reverse SET_NULL relations

Arthur 1 сар өмнө
parent
commit
499397139b

+ 8 - 0
netbox/core/signals.py

@@ -225,7 +225,15 @@ def handle_deleted_object(sender, instance, **kwargs):
                 # We only care about triggering the m2m_changed signal for models which support
                 # We only care about triggering the m2m_changed signal for models which support
                 # change logging
                 # change logging
                 continue
                 continue
+            related_object_type = ContentType.objects.get_for_model(related_model)
             for obj in related_model.objects.filter(**{related_field_name: instance.pk}):
             for obj in related_model.objects.filter(**{related_field_name: instance.pk}):
+                # Skip any related object that is itself being deleted as part of this same
+                # operation (e.g. a sibling caught up in the same cascade). Its deletion has
+                # already been recorded, so nulling the FK and saving here would record an
+                # UPDATE ObjectChange *after* the object's DELETE, corrupting the changelog and
+                # breaking branch replay. (Ref: #22270)
+                if (related_object_type, obj.pk) in _signals_received.pre_delete:
+                    continue
                 obj.snapshot()  # Ensure the change record includes the "before" state
                 obj.snapshot()  # Ensure the change record includes the "before" state
                 if type(relation) is ManyToManyRel:
                 if type(relation) is ManyToManyRel:
                     getattr(obj, related_field_name).remove(instance)
                     getattr(obj, related_field_name).remove(instance)

+ 43 - 1
netbox/core/tests/test_signals.py

@@ -12,7 +12,8 @@ from core import signals
 from core.choices import DataSourceStatusChoices, JobStatusChoices, ObjectChangeActionChoices
 from core.choices import DataSourceStatusChoices, JobStatusChoices, ObjectChangeActionChoices
 from core.models import ConfigRevision, DataSource, ObjectChange, ObjectType
 from core.models import ConfigRevision, DataSource, ObjectChange, ObjectType
 from core.signals import _signals_received, clear_events, post_sync
 from core.signals import _signals_received, clear_events, post_sync
-from dcim.models import Site, SiteGroup
+from dcim.choices import InterfaceTypeChoices
+from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site, SiteGroup
 from extras.models import Tag
 from extras.models import Tag
 from extras.validators import CustomValidator
 from extras.validators import CustomValidator
 from netbox.context import events_queue
 from netbox.context import events_queue
@@ -204,6 +205,47 @@ class HandleDeletedObjectSignalTestCase(TestCase):
         self.assertEqual(oc.prechange_data['group'], group_pk)
         self.assertEqual(oc.prechange_data['group'], group_pk)
         self.assertIsNone(oc.postchange_data['group'])
         self.assertIsNone(oc.postchange_data['group'])
 
 
+    def test_cascade_delete_does_not_record_update_after_delete(self):
+        # Regression test for #22270. Deleting a device cascades to all of its interfaces.
+        # When the LAG interface's pre_delete fires, it clears the `lag` FK (SET_NULL) on its
+        # member interfaces and records a change. If a member is itself being deleted in the
+        # same cascade, that update must not be written *after* the member's delete record —
+        # an UPDATE-after-DELETE corrupts the changelog and breaks branch replay.
+        manufacturer = Manufacturer.objects.create(name='Manufacturer', slug='manufacturer')
+        device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Device Type', slug='device-type')
+        role = DeviceRole.objects.create(name='Role', slug='role')
+        site = Site.objects.create(name='Site', slug='site')
+        device = Device.objects.create(name='Device', site=site, device_type=device_type, role=role)
+        # Members are created before the LAG so their PKs are lower; the cascade fires their
+        # pre_delete (and records their DELETE) before the LAG's pre_delete runs.
+        member1 = Interface.objects.create(device=device, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
+        member2 = Interface.objects.create(device=device, name='eth1', type=InterfaceTypeChoices.TYPE_1GE_FIXED)
+        lag = Interface.objects.create(device=device, name='lag0', type=InterfaceTypeChoices.TYPE_LAG)
+        member1.lag = lag
+        member1.save()
+        member2.lag = lag
+        member2.save()
+        member_pks = (member1.pk, member2.pk)
+        interface_type = ContentType.objects.get_for_model(Interface)
+
+        request = _build_request(self.user)
+        with event_tracking(request):
+            device.delete()
+
+        for member_pk in member_pks:
+            actions = list(
+                ObjectChange.objects.filter(
+                    request_id=request.id,
+                    changed_object_type=interface_type,
+                    changed_object_id=member_pk,
+                ).order_by('time', 'pk').values_list('action', flat=True)
+            )
+            self.assertIn(ObjectChangeActionChoices.ACTION_DELETE, actions)
+            # The member's final change record must be its deletion, with no trailing update.
+            self.assertEqual(actions[-1], ObjectChangeActionChoices.ACTION_DELETE)
+            delete_index = actions.index(ObjectChangeActionChoices.ACTION_DELETE)
+            self.assertNotIn(ObjectChangeActionChoices.ACTION_UPDATE, actions[delete_index + 1:])
+
 
 
 class ClearSignalHistorySignalTestCase(TestCase):
 class ClearSignalHistorySignalTestCase(TestCase):
     """
     """