diff --git a/netbox/extras/models/scripts.py b/netbox/extras/models/scripts.py index 02f3902d1..042744fc6 100644 --- a/netbox/extras/models/scripts.py +++ b/netbox/extras/models/scripts.py @@ -3,7 +3,8 @@ import logging from functools import cached_property from django.contrib.contenttypes.fields import GenericRelation -from django.db import models +from django.contrib.contenttypes.models import ContentType +from django.db import models, router, transaction from django.db.models import Q from django.urls import reverse from django.utils.translation import gettext_lazy as _ @@ -11,7 +12,7 @@ from django.utils.translation import gettext_lazy as _ from core.choices import ManagedFileRootPathChoices from core.models import ManagedFile from extras.utils import is_script -from netbox.models.features import EventRulesMixin, JobsMixin +from netbox.models.features import EventRulesMixin, JobsMixin, batch_delete_jobs from utilities.querysets import RestrictedQuerySet from .mixins import PythonModuleMixin @@ -119,6 +120,29 @@ class ScriptModule(PythonModuleMixin, JobsMixin, ManagedFile): def __str__(self): return self.python_name + def delete(self, using=None, *args, **kwargs): + # Job is imported here rather than at module level to avoid a circular import + # (core.models.jobs -> core.signals -> extras.events -> extras.models -> this module). + from core.models import Job + + # Deleting a ScriptModule cascades (via the Script.module FK) to its child Scripts, and + # Django's collector would materialize every one of those Scripts' Jobs to delete them. + # A module's scripts can accumulate thousands of jobs, exhausting memory. Batch-delete + # the child Scripts' jobs up front, in a single queryset (no per-script loop), before + # delegating to the cascade. The transaction rolls the job deletions back if the parent + # delete fails; note it does not cover ManagedFile.delete removing the file from disk, + # which happens before the DB delete and is not transactional. See #22812. + using = using or router.db_for_write(self.__class__, instance=self) + with transaction.atomic(using=using): + script_type = ContentType.objects.get_for_model(Script, for_concrete_model=False) + child_jobs = Job.objects.using(using).filter( + object_type=script_type, + object_id__in=self.scripts.values_list('pk', flat=True), + ) + batch_delete_jobs(child_jobs) + return super().delete(using, *args, **kwargs) + delete.alters_data = True + @property def ordered_scripts(self): script_objects = {s.name: s for s in self.scripts.all()} diff --git a/netbox/extras/tests/test_scripts_deletion.py b/netbox/extras/tests/test_scripts_deletion.py new file mode 100644 index 000000000..7419de359 --- /dev/null +++ b/netbox/extras/tests/test_scripts_deletion.py @@ -0,0 +1,306 @@ +import uuid +from unittest import mock + +from django.contrib.contenttypes.models import ContentType +from django.db import router +from django.db.models import QuerySet +from django.test import TestCase, override_settings +from django.urls import reverse + +from core.choices import JobStatusChoices, ManagedFileRootPathChoices +from core.models import DataSource, Job +from extras.models import Script, ScriptModule +from extras.validators import CustomValidator +from netbox.models.deletion import ConfirmCollector, CountOnly +from utilities.exceptions import AbortRequest +from utilities.testing import TestCase as ViewTestCase + + +class ScriptDeletionTestCase(TestCase): + """ + Regression tests for #22812: deleting a JobsMixin object (Script, ScriptModule, DataSource) + with many associated Jobs must not load every Job into memory at once. + """ + @classmethod + def setUpTestData(cls): + cls.script_ct = ContentType.objects.get_for_model(Script, for_concrete_model=False) + + def _create_module(self): + return ScriptModule.objects.create( + file_root=ManagedFileRootPathChoices.SCRIPTS, + file_path=f'test_{uuid.uuid4().hex[:8]}.py', + ) + + def _create_script(self, module=None): + module = module or self._create_module() + script = Script.objects.create(module=module, name=f'S{uuid.uuid4().hex[:8]}') + return module, script + + def _add_jobs(self, obj, count, object_type=None): + object_type = object_type or ContentType.objects.get_for_model(type(obj), for_concrete_model=False) + Job.objects.bulk_create([ + Job( + object_type=object_type, + object_id=obj.pk, + name='testjob', + status=JobStatusChoices.STATUS_COMPLETED, + job_id=uuid.uuid4(), + data={'output': 'x' * 50}, + ) + for _ in range(count) + ]) + + def test_delete_script_deletes_all_jobs(self): + _, script = self._create_script() + self._add_jobs(script, 2500) + self.assertEqual(script.jobs.count(), 2500) + + script.delete() + + self.assertFalse(Script.objects.filter(pk=script.pk).exists()) + self.assertEqual(Job.objects.filter(object_type=self.script_ct, object_id=script.pk).count(), 0) + + def test_delete_script_batches_jobs(self): + _, script = self._create_script() + self._add_jobs(script, 5) + + job_delete_calls = [] + original_delete = QuerySet.delete + + def counting_delete(qs, *args, **kwargs): + if qs.model is Job: + job_delete_calls.append(len(qs)) + return original_delete(qs, *args, **kwargs) + + with mock.patch('netbox.models.features.JOB_DELETE_BATCH_SIZE', 2): + with mock.patch.object(QuerySet, 'delete', counting_delete): + script.delete() + + # 5 jobs at a batch size of 2 => three batched deletes (2, 2, 1) + self.assertEqual(job_delete_calls, [2, 2, 1]) + self.assertEqual(Job.objects.filter(object_type=self.script_ct, object_id=script.pk).count(), 0) + + def test_delete_scriptmodule_cascades_to_scripts_and_jobs(self): + module, script = self._create_script() + self._add_jobs(script, 100) + + module.delete() + + self.assertFalse(ScriptModule.objects.filter(pk=module.pk).exists()) + self.assertFalse(Script.objects.filter(pk=script.pk).exists()) + self.assertEqual(Job.objects.filter(object_type=self.script_ct, object_id=script.pk).count(), 0) + + def test_delete_scriptmodule_batches_child_script_jobs(self): + # The reporter's actual path: a script is only removable via the UI by deleting its + # ScriptModule. The module's delete must batch the child Script's jobs. + module, script = self._create_script() + self._add_jobs(script, 5) + + job_delete_calls = [] + original_delete = QuerySet.delete + + def counting_delete(qs, *args, **kwargs): + if qs.model is Job: + job_delete_calls.append(len(qs)) + return original_delete(qs, *args, **kwargs) + + with mock.patch('netbox.models.features.JOB_DELETE_BATCH_SIZE', 2): + with mock.patch.object(QuerySet, 'delete', counting_delete): + module.delete() + + # 5 child-script jobs at a batch size of 2 => three batched deletes (2, 2, 1). The module + # has no jobs of its own, so JobsMixin.delete adds no further Job deletes. + self.assertEqual(job_delete_calls, [2, 2, 1]) + self.assertFalse(Script.objects.filter(pk=script.pk).exists()) + self.assertEqual(Job.objects.filter(object_type=self.script_ct, object_id=script.pk).count(), 0) + + def test_delete_datasource_deletes_jobs(self): + datasource = DataSource.objects.create(name='DS', type='local', source_url='/tmp/test') + self._add_jobs(datasource, 100) + ds_ct = ContentType.objects.get_for_model(DataSource, for_concrete_model=False) + self.assertEqual(Job.objects.filter(object_type=ds_ct, object_id=datasource.pk).count(), 100) + + datasource.delete() + + self.assertFalse(DataSource.objects.filter(pk=datasource.pk).exists()) + self.assertEqual(Job.objects.filter(object_type=ds_ct, object_id=datasource.pk).count(), 0) + + def test_soft_delete_preserves_jobs(self): + _, script = self._create_script() + self._add_jobs(script, 10) + + script.delete(soft_delete=True) + + script.refresh_from_db() + self.assertFalse(script.is_executable) + self.assertEqual(Job.objects.filter(object_type=self.script_ct, object_id=script.pk).count(), 10) + + @override_settings(PROTECTION_RULES={'extras.script': [CustomValidator({'name': {'eq': ''}})]}) + def test_delete_rolls_back_jobs_on_parent_failure(self): + # A protection rule that no real script can satisfy (name must be empty) makes the + # cascade's pre_delete handler raise AbortRequest *after* JobsMixin.delete has already + # batch-deleted the jobs. JobsMixin.delete wraps the batch loop and super().delete() in a + # transaction, so the job deletions must roll back, leaving no orphaned partial state. + # This exercises the real deletion-abort path rather than mocking Django internals. + _, script = self._create_script() + self._add_jobs(script, 10) + + with self.assertRaises(AbortRequest): + script.delete() + + self.assertTrue(Script.objects.filter(pk=script.pk).exists()) + self.assertEqual(Job.objects.filter(object_type=self.script_ct, object_id=script.pk).count(), 10) + + @override_settings(PROTECTION_RULES={'extras.script': [CustomValidator({'name': {'eq': ''}})]}) + def test_delete_scriptmodule_rolls_back_child_jobs_on_failure(self): + # Same abort path via the module: the protection rule fires when the cascade pre_deletes + # the child Script, after ScriptModule.delete has already batch-deleted that script's jobs. + # The transaction must roll those job deletions back, leaving no orphaned partial state. + module, script = self._create_script() + self._add_jobs(script, 10) + + with self.assertRaises(AbortRequest): + module.delete() + + self.assertTrue(ScriptModule.objects.filter(pk=module.pk).exists()) + self.assertTrue(Script.objects.filter(pk=script.pk).exists()) + self.assertEqual(Job.objects.filter(object_type=self.script_ct, object_id=script.pk).count(), 10) + + +class ConfirmCollectorTestCase(TestCase): + """ + #22812: the delete-confirmation page must not materialize every dependent Job. + """ + def _create_script_with_jobs(self, count): + module = ScriptModule.objects.create( + file_root=ManagedFileRootPathChoices.SCRIPTS, + file_path=f'test_{uuid.uuid4().hex[:8]}.py', + ) + script = Script.objects.create(module=module, name=f'S{uuid.uuid4().hex[:8]}') + ct = ContentType.objects.get_for_model(Script, for_concrete_model=False) + Job.objects.bulk_create([ + Job(object_type=ct, object_id=script.pk, name='j', status=JobStatusChoices.STATUS_COMPLETED, + job_id=uuid.uuid4(), data={'output': 'x' * 50}) + for _ in range(count) + ]) + return script + + def test_confirm_collector_counts_jobs_without_instantiating(self): + script = self._create_script_with_jobs(500) + + init_calls = [] + original_init = Job.__init__ + + def counting_init(self, *args, **kwargs): + init_calls.append(1) + original_init(self, *args, **kwargs) + + with mock.patch.object(Job, '__init__', counting_init): + collector = ConfirmCollector(using=router.db_for_write(Script)) + collector.collect([script]) + + # No Job rows were instantiated; the relation was counted instead. + self.assertEqual(len(init_calls), 0) + self.assertNotIn(Job, collector.data) + self.assertEqual(collector.generic_relation_counts.get(Job), 500) + # The non-job cascade (the Script itself) is still collected. + self.assertIn(Script, collector.data) + + def test_count_only_wrapper(self): + # CountOnly reports its count via len() but iterates empty, so it slots into the + # dependent-objects mapping as a non-expandable, non-materializing row. + wrapper = CountOnly(3000) + self.assertEqual(len(wrapper), 3000) + self.assertEqual(list(wrapper), []) + self.assertTrue(wrapper.count_only) + + def test_confirm_collector_omits_jobs_when_none(self): + # A jobless object must not record a zero count, or the confirmation page would show a + # spurious "0 jobs" row (#22812 regression). + datasource = DataSource.objects.create(name='DS', type='local', source_url='/tmp/test') + + collector = ConfirmCollector(using=router.db_for_write(DataSource)) + collector.collect([datasource]) + + self.assertNotIn(Job, collector.generic_relation_counts) + + +class ObjectDeleteViewCountsTestCase(ViewTestCase): + """ + #22812: the delete-confirmation view must report a JobsMixin object's jobs as a count + (via CountOnly) without materializing them, and _get_dependent_objects must keep returning + a single dict. + """ + def test_get_dependent_objects_returns_count_only_for_jobs(self): + from netbox.views.generic.object_views import ObjectDeleteView + + module = ScriptModule.objects.create( + file_root=ManagedFileRootPathChoices.SCRIPTS, + file_path=f'test_{uuid.uuid4().hex[:8]}.py', + ) + script = Script.objects.create(module=module, name=f'S{uuid.uuid4().hex[:8]}') + ct = ContentType.objects.get_for_model(Script, for_concrete_model=False) + Job.objects.bulk_create([ + Job( + object_type=ct, object_id=script.pk, name='j', + status=JobStatusChoices.STATUS_COMPLETED, job_id=uuid.uuid4(), + ) + for _ in range(50) + ]) + + view = ObjectDeleteView() + view.queryset = ScriptModule.objects.all() + dependent_objects = view._get_dependent_objects(module) + + # Single dict returned (not a tuple); jobs represented as a CountOnly. + self.assertIsInstance(dependent_objects, dict) + self.assertIn(Job, dependent_objects) + self.assertIsInstance(dependent_objects[Job], CountOnly) + self.assertEqual(len(dependent_objects[Job]), 50) + + @override_settings(EXEMPT_VIEW_PERMISSIONS=['*']) + def test_confirm_page_renders_job_count(self): + module = ScriptModule.objects.create( + file_root=ManagedFileRootPathChoices.SCRIPTS, + file_path=f'test_{uuid.uuid4().hex[:8]}.py', + ) + script = Script.objects.create(module=module, name=f'S{uuid.uuid4().hex[:8]}') + ct = ContentType.objects.get_for_model(Script, for_concrete_model=False) + Job.objects.bulk_create([ + Job( + object_type=ct, object_id=script.pk, name='j', + status=JobStatusChoices.STATUS_COMPLETED, job_id=uuid.uuid4(), + ) + for _ in range(50) + ]) + + # ScriptModule is a proxy over core.ManagedFile, so the delete view requires the + # concrete model's permission (core.delete_managedfile), not extras.delete_scriptmodule. + self.add_permissions('core.delete_managedfile') + url = reverse('extras:scriptmodule_delete', kwargs={'pk': module.pk}) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + + # Assert on the rendered context, not brittle HTML substrings: Job is present in + # dependent_objects as a CountOnly reporting the true count, so the confirmation page + # renders it as a summarized (non-expandable) row without materializing 50 Job rows. + dependent_objects = response.context['dependent_objects'] + self.assertIn(Job, dependent_objects) + self.assertIsInstance(dependent_objects[Job], CountOnly) + self.assertEqual(len(dependent_objects[Job]), 50) + self.assertTrue(dependent_objects[Job].count_only) + + def test_get_dependent_objects_omits_jobs_when_none(self): + from netbox.views.generic.object_views import ObjectDeleteView + + # A module with no jobs must not produce a CountOnly(0) entry (#22812 regression). + module = ScriptModule.objects.create( + file_root=ManagedFileRootPathChoices.SCRIPTS, + file_path=f'test_{uuid.uuid4().hex[:8]}.py', + ) + + view = ObjectDeleteView() + view.queryset = ScriptModule.objects.all() + dependent_objects = view._get_dependent_objects(module) + + self.assertNotIn(Job, dependent_objects) diff --git a/netbox/netbox/constants.py b/netbox/netbox/constants.py index 037059583..096ae145b 100644 --- a/netbox/netbox/constants.py +++ b/netbox/netbox/constants.py @@ -74,3 +74,10 @@ CENSOR_TOKEN_CHANGED = '***CHANGED***' # Placeholder text for empty tables EMPTY_TABLE_TEXT = 'No results found' + +# Batch size for deleting a JobsMixin object's associated jobs during cascade deletion. Job +# cannot be fast-deleted (a global pre_delete receiver forces per-instance signals), so deleting +# in chunks bounds the work per delete cycle rather than building one huge collection and running +# one long DELETE. 1000 matches EXPORT_CHUNK_SIZE and, in benchmarking a 200k-job deletion, was +# the fastest of 100/1000/5000 while keeping peak memory flat. See #22812. +JOB_DELETE_BATCH_SIZE = 1000 diff --git a/netbox/netbox/models/deletion.py b/netbox/netbox/models/deletion.py index e87321ae3..b0600e9e2 100644 --- a/netbox/netbox/models/deletion.py +++ b/netbox/netbox/models/deletion.py @@ -8,6 +8,72 @@ from django.utils.translation import gettext as _ logger = logging.getLogger("netbox.models.deletion") +class CountOnly: + """ + A stand-in for a list of dependent instances that reports a count without holding any + instances. Used on the delete-confirmation page for high-cardinality relations (e.g. a + JobsMixin object's jobs) which we deliberately do not materialize (see #22812). It is a + lenient, empty iterable: `len()` returns the true row count, but iterating yields nothing, + so it slots into the same `{model: }` mapping as real instance lists and renders + as a non-expandable row. + """ + # Template flag: distinguishes a count-only entry (no instances to list) from a real list, + # so the confirmation page can render it without an expand/collapse affordance. + count_only = True + + def __init__(self, count): + self.count = count + + def __len__(self): + return self.count + + def __iter__(self): + return iter(()) + + +class ConfirmCollector(Collector): + """ + A display-only Collector used to enumerate the objects that would be deleted along with a + given object, for rendering the delete confirmation page. It behaves like Django's stock + Collector (preserving the full FK cascade graph and its ProtectedError/RestrictedError + behavior) except that it does not descend into the `jobs` GenericRelation. A JobsMixin + object can accumulate thousands of Jobs, each carrying large data/log_entries payloads; + materializing them all just to render a confirmation page can exhaust memory (see #22812). + Instead, the related Jobs are counted and recorded in `generic_relation_counts`. + + This is intentionally specific to Job, the only high-cardinality GenericRelation in the + data model; it is not a general count-out over every GenericRelation. If another relation + ever needs the same treatment, extend the check in collect() (and the matching write-path + batching in JobsMixin/ScriptModule.delete) rather than assuming this already handles it. + """ + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.generic_relation_counts = {} + + def collect(self, objs, source=None, *args, **kwargs): + """ + Override collect() to count the `jobs` GenericRelation rather than descend into it. + + Django's Collector offers no per-relation skip hook, so we intercept the one call it + makes when cascading into a GenericRelation: collect(sub_objs, source=model, ...), where + `sub_objs` is a queryset of the related model. When that model is Job, we count the rows + instead of collecting (and thus instantiating) them, and forward every other call to the + stock implementation untouched. A directly-deleted Job (top-level call, source=None) + still collects normally. + """ + from core.models import Job + + if source is not None and getattr(objs, 'model', None) is Job: + # Django calls this branch for the jobs relation even when there are none; only record + # a count when there are actually jobs, so jobless objects don't get a spurious + # "0 jobs" row on the delete-confirmation page. + count = objs.count() + if count: + self.generic_relation_counts[Job] = self.generic_relation_counts.get(Job, 0) + count + return None + return super().collect(objs, source=source, *args, **kwargs) + + class CustomCollector(Collector): """ Override Django's stock Collector to handle GenericRelations and ensure proper ordering of cascading deletions. diff --git a/netbox/netbox/models/features.py b/netbox/netbox/models/features.py index 1556a22a1..1cf46d013 100644 --- a/netbox/netbox/models/features.py +++ b/netbox/netbox/models/features.py @@ -5,7 +5,7 @@ from functools import cached_property from django.contrib.contenttypes.fields import GenericForeignKey, GenericRelation from django.contrib.contenttypes.models import ContentType from django.core.validators import ValidationError -from django.db import models +from django.db import models, router, transaction from django.db.models import Q from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -17,7 +17,7 @@ from extras.constants import CUSTOMFIELD_EMPTY_VALUES from extras.managers import NetBoxTaggableManager, NetBoxTaggableManagerField from extras.utils import is_taggable from netbox.config import get_config -from netbox.constants import CORE_APPS +from netbox.constants import CORE_APPS, JOB_DELETE_BATCH_SIZE from netbox.models.deletion import DeleteMixin from netbox.plugins import PluginConfig from netbox.registry import registry @@ -43,6 +43,7 @@ __all__ = ( 'NotificationsMixin', 'SyncedDataMixin', 'TagsMixin', + 'batch_delete_jobs', 'get_model_features', 'has_feature', 'model_is_public', @@ -449,9 +450,39 @@ class NotificationsMixin(models.Model): abstract = True +def batch_delete_jobs(job_queryset): + """ + Delete the Jobs in `job_queryset` in JOB_DELETE_BATCH_SIZE chunks. Job cannot be fast-deleted + (a global pre_delete receiver forces per-instance signals), so a single delete would build one + huge collection of Job instances and run one very long DELETE; batching bounds the per-cycle + work. Callers are responsible for wrapping this in a transaction. As with the prior cascade + behavior, this bulk delete does not invoke Job.delete() and therefore does not cancel the + backing RQ job. See #22812. + """ + from core.models import Job + + # Route writes to the same database the queryset reads from. In JobsMixin.delete the queryset + # is bound to the instance's DB while Job.objects would otherwise use the router default; if + # those diverge the deleted rows never leave the read side and the loop below never terminates. + jobs = Job.objects.using(job_queryset.db) + + job_pks = job_queryset.order_by('pk').values_list('pk', flat=True) + # Re-slice the queryset each iteration: it re-queries after each batch delete, so the + # remaining set shrinks and the loop terminates (do not hoist this into a cursor). + while pks := list(job_pks[:JOB_DELETE_BATCH_SIZE]): + # only('pk'): the batch still can't fast-delete, so each Job in the batch is instantiated; + # loading just the PK avoids pulling the large data/log_entries payloads into memory. + jobs.filter(pk__in=pks).only('pk').delete() + + class JobsMixin(models.Model): """ Enables support for job results. + + Note: for the job-batching in delete() to run, JobsMixin must precede DeleteMixin in a + model's MRO. DeleteMixin.delete() drives its own collector and does not call super(), so a + model declared as e.g. `class Foo(NetBoxModel, JobsMixin)` would reach DeleteMixin first and + bypass the batching. Core models that combine both (e.g. DataSource) list JobsMixin first. """ jobs = GenericRelation( to='core.Job', @@ -463,6 +494,16 @@ class JobsMixin(models.Model): class Meta: abstract = True + def delete(self, using=None, *args, **kwargs): + # Delete associated jobs in batches so the cascade never has to load thousands of Job + # rows into memory at once. Wrapped in a transaction so that a failure in the parent + # delete rolls the job deletions back as well. See #22812. + using = using or router.db_for_write(self.__class__, instance=self) + with transaction.atomic(using=using): + batch_delete_jobs(self.jobs.using(using)) + return super().delete(using, *args, **kwargs) + delete.alters_data = True + def get_latest_jobs(self): """ Return a list of the most recent jobs for this instance. diff --git a/netbox/netbox/views/generic/object_views.py b/netbox/netbox/views/generic/object_views.py index 3e9690030..e815a45cb 100644 --- a/netbox/netbox/views/generic/object_views.py +++ b/netbox/netbox/views/generic/object_views.py @@ -4,7 +4,6 @@ from collections import defaultdict from django.contrib import messages from django.db import router, transaction from django.db.models import ProtectedError, RestrictedError -from django.db.models.deletion import Collector from django.http import HttpResponse from django.shortcuts import redirect, render from django.urls import reverse @@ -13,6 +12,7 @@ from django.utils.safestring import mark_safe from django.utils.translation import gettext as _ from core.signals import clear_events +from netbox.models.deletion import ConfirmCollector, CountOnly from netbox.object_actions import BulkDelete, BulkEdit, CloneObject, DeleteObject, EditObject from utilities.error_handlers import handle_protectederror from utilities.exceptions import AbortRequest, PermissionsViolation @@ -385,14 +385,19 @@ class ObjectDeleteView(GetReturnURLMixin, BaseObjectView): def _get_dependent_objects(self, obj): """ - Returns a dictionary mapping of dependent objects (organized by model) which will be deleted as a result of - deleting the requested object. + Returns a dictionary mapping each dependent model to the objects (of that model) which will + be deleted as a result of deleting the requested object. + + Values are normally a list of instances. For high-cardinality relations that we do not + materialize to avoid excessive memory use (currently a JobsMixin object's jobs, see + #22812), the value is a `CountOnly` — a lenient empty iterable whose `len()` is the true + row count, so it renders as a non-expandable row alongside the itemized relations. Args: obj: The object to return dependent objects for """ using = router.db_for_write(obj._meta.model) - collector = Collector(using=using) + collector = ConfirmCollector(using=using) collector.collect([obj]) # Compile a mapping of models to instances @@ -406,7 +411,13 @@ class ObjectDeleteView(GetReturnURLMixin, BaseObjectView): continue dependent_objects[model].append(instances) - return dict(dependent_objects) + # Add count-only entries for relations the collector enumerated by count rather than by + # instance (e.g. jobs), so they render as non-expandable rows in the same mapping. + dependent_objects = dict(dependent_objects) + for model, count in collector.generic_relation_counts.items(): + dependent_objects[model] = CountOnly(count) + + return dependent_objects def _handle_protected_objects(self, obj, protected_objects, request, exc): """ diff --git a/netbox/templates/htmx/delete_form.html b/netbox/templates/htmx/delete_form.html index eb267e44c..c07e83d0a 100644 --- a/netbox/templates/htmx/delete_form.html +++ b/netbox/templates/htmx/delete_form.html @@ -24,31 +24,38 @@

{% for model, instances in dependent_objects.items %} -
-

- -

-
-
-
- {% for instance in instances %} - {% with url=instance.get_absolute_url %} - {{ instance }} - {% endwith %} - {% endfor %} + {% with object_count=instances|length %} +
+

+ {# High-cardinality relations (e.g. jobs) are summarized by count and are not #} + {# expandable, since their instances are intentionally not loaded (see #22812). #} + {% if instances.count_only %} + + {% else %} + + {% endif %} +

+ {% if not instances.count_only %} +
+
+
+ {% for instance in instances %} + {% with url=instance.get_absolute_url %} + {{ instance }} + {% endwith %} + {% endfor %} +
+
-
+ {% endif %}
-
+ {% endwith %} {% endfor %}
{% endif %}