diff --git a/netbox/core/signals.py b/netbox/core/signals.py index 4b537b2d4..8ba8cc244 100644 --- a/netbox/core/signals.py +++ b/netbox/core/signals.py @@ -162,6 +162,12 @@ def handle_deleted_object(sender, instance, **kwargs): getattr(obj, related_field_name).remove(instance) elif type(relation) is ManyToOneRel and relation.field.null is True: setattr(obj, related_field_name, None) + # make sure the object hasn't been deleted - in case of + # deletion chaining of related objects + try: + obj.refresh_from_db() + except DoesNotExist: + continue obj.save() # Enqueue the object for event processing diff --git a/netbox/core/tests/test_changelog.py b/netbox/core/tests/test_changelog.py index 4914dbaf3..df8461076 100644 --- a/netbox/core/tests/test_changelog.py +++ b/netbox/core/tests/test_changelog.py @@ -6,12 +6,13 @@ from rest_framework import status from core.choices import ObjectChangeActionChoices from core.models import ObjectChange, ObjectType from dcim.choices import SiteStatusChoices -from dcim.models import Site +from dcim.models import Site, CableTermination, Device, DeviceType, DeviceRole, Interface, Cable from extras.choices import * from extras.models import CustomField, CustomFieldChoiceSet, Tag from utilities.testing import APITestCase from utilities.testing.utils import create_tags, post_data from utilities.testing.views import ModelViewTestCase +from dcim.models import Manufacturer class ChangeLogViewTest(ModelViewTestCase): @@ -270,6 +271,81 @@ class ChangeLogViewTest(ModelViewTestCase): # Check that no ObjectChange records have been created self.assertEqual(ObjectChange.objects.count(), 0) + def test_ordering_genericrelation(self): + # Create required objects first + manufacturer = Manufacturer.objects.create(name='Manufacturer 1') + device_type = DeviceType.objects.create( + manufacturer=manufacturer, + model='Model 1', + slug='model-1' + ) + device_role = DeviceRole.objects.create( + name='Role 1', + slug='role-1' + ) + site = Site.objects.create( + name='Site 1', + slug='site-1' + ) + + # Create two devices + device1 = Device.objects.create( + name='Device 1', + device_type=device_type, + role=device_role, + site=site + ) + device2 = Device.objects.create( + name='Device 2', + device_type=device_type, + role=device_role, + site=site + ) + + # Create interfaces on both devices + interface1 = Interface.objects.create( + device=device1, + name='eth0', + type='1000base-t' + ) + interface2 = Interface.objects.create( + device=device2, + name='eth0', + type='1000base-t' + ) + + # Create a cable between the interfaces + _ = Cable.objects.create( + a_terminations=[interface1], + b_terminations=[interface2], + status='connected' + ) + + # Delete device1 + request = { + 'path': reverse('dcim:device_delete', kwargs={'pk': device1.pk}), + 'data': post_data({'confirm': True}), + } + self.add_permissions( + 'dcim.delete_device', + 'dcim.delete_interface', + 'dcim.delete_cable', + 'dcim.delete_cabletermination' + ) + response = self.client.post(**request) + self.assertHttpStatus(response, 302) + + # Get the ObjectChange records for delete actions ordered by time + changes = ObjectChange.objects.filter( + action=ObjectChangeActionChoices.ACTION_DELETE + ).order_by('time')[:3] + + # Verify the order of deletion + self.assertEqual(len(changes), 3) + self.assertEqual(changes[0].changed_object_type, ContentType.objects.get_for_model(CableTermination)) + 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)) + class ChangeLogAPITest(APITestCase): diff --git a/netbox/netbox/models/deletion.py b/netbox/netbox/models/deletion.py new file mode 100644 index 000000000..10416b748 --- /dev/null +++ b/netbox/netbox/models/deletion.py @@ -0,0 +1,90 @@ +import logging + +from django.contrib.contenttypes.fields import GenericRelation +from django.db import router +from django.db.models.deletion import Collector + +logger = logging.getLogger("netbox.models.deletion") + + +class CustomCollector(Collector): + """ + Custom collector that handles GenericRelations correctly. + """ + + def collect( + self, + objs, + source=None, + nullable=False, + collect_related=True, + source_attr=None, + reverse_dependency=False, + keep_parents=False, + fail_on_restricted=True, + ): + """ + Override collect to first collect standard dependencies, + then add GenericRelations to the dependency graph. + """ + # Call parent collect first to get all standard dependencies + super().collect( + objs, + source=source, + nullable=nullable, + collect_related=collect_related, + source_attr=source_attr, + reverse_dependency=reverse_dependency, + keep_parents=keep_parents, + fail_on_restricted=fail_on_restricted, + ) + + # Track which GenericRelations we've already processed to prevent infinite recursion + processed_relations = set() + + # Now add GenericRelations to the dependency graph + for _, instances in list(self.data.items()): + for instance in instances: + # Get all GenericRelations for this model + for field in instance._meta.private_fields: + if isinstance(field, GenericRelation): + # Create a unique key for this relation + relation_key = f"{instance._meta.model_name}.{field.name}" + if relation_key in processed_relations: + continue + processed_relations.add(relation_key) + + # Add the model that the generic relation points to as a dependency + self.add_dependency(field.related_model, instance, reverse_dependency=True) + + +class DeleteMixin: + """ + Mixin to override the model delete function to use our custom collector. + """ + + def delete(self, using=None, keep_parents=False): + """ + Override delete to use our custom collector. + """ + using = using or router.db_for_write(self.__class__, instance=self) + assert self._get_pk_val() is not None, "%s object can't be deleted because its %s attribute is set to None." % ( + self._meta.object_name, + self._meta.pk.attname, + ) + + collector = CustomCollector(using=using) + collector.collect([self], keep_parents=keep_parents) + + return collector.delete() + + delete.alters_data = True + + @classmethod + def verify_mro(cls, instance): + """ + Verify that this mixin is first in the MRO. + """ + mro = instance.__class__.__mro__ + if mro.index(cls) != 0: + raise RuntimeError(f"{cls.__name__} must be first in the MRO. Current MRO: {mro}") diff --git a/netbox/netbox/models/features.py b/netbox/netbox/models/features.py index 25f23c9d3..79145ce70 100644 --- a/netbox/netbox/models/features.py +++ b/netbox/netbox/models/features.py @@ -16,6 +16,7 @@ from extras.choices import * from extras.constants import CUSTOMFIELD_EMPTY_VALUES from extras.utils import is_taggable from netbox.config import get_config +from netbox.models.deletion import DeleteMixin from netbox.registry import registry from netbox.signals import post_clean from utilities.json import CustomFieldJSONEncoder @@ -45,7 +46,7 @@ __all__ = ( # Feature mixins # -class ChangeLoggingMixin(models.Model): +class ChangeLoggingMixin(DeleteMixin, models.Model): """ Provides change logging support for a model. Adds the `created` and `last_updated` fields. """