From dec980b6fadd3e9f6223cde74de90ca31a02263a Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Wed, 10 Jun 2026 06:17:15 +0000 Subject: [PATCH] Fail loudly on ignored referencing fields that match no ForeignKey The command used to silently skip ignore paths that did not match any FK pointing at the target model, so a typo or a stale path (e.g. after a model moved to another app) quietly changed collection behavior. Validate every configured path on each run, dry runs included. https://claude.ai/code/session_01NWap56ibZN8sey8mQfa8uw --- README.md | 5 ++++- django_gc/management/commands/gc.py | 13 ++++++++++++- tests/test_gc_command.py | 27 ++++++++++++++++++++++++++- tests/testapp/models.py | 11 +++++++++++ 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 7176bd8..dd49752 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,10 @@ class Attachment(models.Model): - `gc_enabled` (required, must be `True`) opts the model in. - `gc_ignored_referencing_fields` (optional) lists referencing foreign keys to - ignore when deciding whether a row is still in use. + ignore when deciding whether a row is still in use. Every listed path must + match an existing `ForeignKey` to the model — the command fails loudly on + paths that don't (including in dry-run mode), so stale configuration is + caught instead of silently changing behavior. - `gc_filter` (optional) is a callable that receives the base queryset and returns the queryset of garbage collection candidates. diff --git a/django_gc/management/commands/gc.py b/django_gc/management/commands/gc.py index 1c1ae06..8a7ccf5 100644 --- a/django_gc/management/commands/gc.py +++ b/django_gc/management/commands/gc.py @@ -107,14 +107,25 @@ def find_fk_fields( """Find all ForeignKey fields pointing to target_model, excluding ignored fields.""" fk_fields = [] ignored_set = set(ignored_fields) + matched_ignored = set() for model in apps.get_models(): for field in model._meta.get_fields(): if isinstance(field, models.ForeignKey) and field.related_model == target_model: field_path = f"{model._meta.label}.{field.name}" - if field_path not in ignored_set: + if field_path in ignored_set: + matched_ignored.add(field_path) + else: fk_fields.append((model, field)) + unmatched_ignored = ignored_set - matched_ignored + if unmatched_ignored: + raise CommandError( + f"Ignored referencing fields for {target_model._meta.label} do not match " + f"any ForeignKey pointing to it: {', '.join(sorted(unmatched_ignored))}. " + "The configuration is stale or contains a typo." + ) + return fk_fields diff --git a/tests/test_gc_command.py b/tests/test_gc_command.py index 34e6962..45cb531 100644 --- a/tests/test_gc_command.py +++ b/tests/test_gc_command.py @@ -2,7 +2,7 @@ from django.core.management import call_command from django.core.management.base import CommandError -from tests.testapp.models import Document, Reference +from tests.testapp.models import Cache, CacheLog, Document, Reference @pytest.mark.django_db @@ -42,6 +42,31 @@ def test_settings_config(settings) -> None: assert Document.objects.filter(pk=referenced.pk).exists() +@pytest.mark.django_db +def test_ignored_referencing_field_does_not_keep_row_alive() -> None: + cache = Cache.objects.create() + CacheLog.objects.create(cache=cache) + + call_command('gc', '--delete', '--model', 'testapp.Cache') + + assert not Cache.objects.exists() + # The ignored reference goes away with the cascade. + assert not CacheLog.objects.exists() + + +@pytest.mark.django_db +def test_unmatched_ignored_field_path_fails_loudly(settings) -> None: + settings.GARBAGE_COLLECTION_CONFIG = { + 'testapp.Reference': { + 'ignored_referencing_fields': ['testapp.Document.no_such_field'], + }, + } + + # Even a dry run reports the stale path. + with pytest.raises(CommandError, match='no_such_field'): + call_command('gc', '--model', 'testapp.Reference') + + @pytest.mark.django_db def test_collision_between_settings_and_model_config(settings) -> None: settings.GARBAGE_COLLECTION_CONFIG = { diff --git a/tests/testapp/models.py b/tests/testapp/models.py index c55df2b..a2be17e 100644 --- a/tests/testapp/models.py +++ b/tests/testapp/models.py @@ -11,3 +11,14 @@ class Document(models.Model): class Reference(models.Model): document = models.ForeignKey(Document, on_delete=models.CASCADE) + + +class Cache(models.Model): + """Garbage collected even while CacheLog rows point at it.""" + + gc_enabled = True + gc_ignored_referencing_fields = ['testapp.CacheLog.cache'] + + +class CacheLog(models.Model): + cache = models.ForeignKey(Cache, on_delete=models.CASCADE)