Add polymorphic guard to solve deletion problems.

Includes docs and robust tests.

Author: bckohan

.gitignore

@@ -61,6 +61,9 @@ local_settings.py
 db.sqlite3
 db.sqlite3-journal
 
+# Test migrations (generated dynamically by tests)
+src/polymorphic/tests/test_migrations/migrations/0*.py
+
 # Flask stuff:
 instance/
 .webassets-cache

docs/api/index.rst

@@ -16,6 +16,7 @@ API Documentation
    polymorphic.formsets
    polymorphic.managers
    polymorphic.models
+   polymorphic.deletion
    polymorphic.query
    polymorphic.showfields
    polymorphic.templatetags

docs/api/polymorphic.deletion.rst

@@ -0,0 +1,12 @@
+polymorphic.models
+==================
+
+.. automodule:: polymorphic.deletion
+
+.. autoclass:: polymorphic.deletion.PolymorphicGuard
+    :members: __call__
+    :show-inheritance:
+
+.. autoclass:: polymorphic.deletion.PolymorphicGuardSerializer
+    :members:
+    :show-inheritance:

docs/deletion.rst

@@ -0,0 +1,51 @@
+Deletion
+========
+
+.. versionadded:: 4.5.0
+
+There is nothing special about deleting polymorphic models. The same rules apply as to
+:ref:`the deletion of normal Django models <topics-db-queries-delete>` that have parent/child
+relationships up and down a model inheritance hierarchy. Django must walk the model inheritance and
+relationship graph and collect all of the affected objects so that it can correctly order deletion
+SQL statements to respect database constraints and issue signals.
+
+The polymorphic deletion logic is the same as the normal Django deletion logic because Django
+already walks the model inheritance hierarchy. :class:`~polymorphic.query.PolymorphicQuerySet` and
+:class:`~polymorphic.managers.PolymorphicManager` disrupt this process by confusing Django's graph
+walker by returning concrete subclass instances instead of base class instances when it attempts to
+walk reverse relationships to polymorphic models. To prevent this confusion,
+:pypi:`django-polymorphic` wraps the :attr:`~django.db.models.ForeignKey.on_delete` handlers of
+reverse relations to polymorphic models with :class:`~polymorphic.deletion.PolymorphicGuard`
+which disables polymorphic behavior on the related querysets during collection.
+
+**You may define your polymorphic models as you normally would using the standard Django**
+:attr:`~django.db.models.ForeignKey.on_delete` **actions**.
+:class:`~polymorphic.models.PolymorphicModel` will automatically wrap the actions for you. actions
+wrapped with :class:`~polymorphic.deletion.PolymorphicGuard` serialize in migrations as the
+underlying wrapped action. This ensures migrations generated by versions of
+:pypi:`django-polymorphic` after 4.5.0 should be the same as with prior versions. The guard is also
+unnecessary during migrations because Django generates basic managers instead of using the default
+polymorphic managers.
+
+It is a design goal of :pypi:`django-polymorphic` that deletion should just work without any special
+treatment. However if you encounter attribute errors or database integrity errors during deletion
+you may manually wrap the :attr:`~django.db.models.ForeignKey.on_delete` action of reverse relations
+to polymorphic models with :class:`~polymorphic.deletion.PolymorphicGuard` to disable polymorphic
+behavior during deletion collection. If you encounter an issue like this
+`please report it to us <https://github.com/jazzband/django-polymorphic/issues>`_. For example:
+
+.. code-block:: python
+
+    from polymorphic.models import PolymorphicModel
+    from polymorphic.deletion import PolymorphicGuard
+    from django.db import models
+
+    class MyModel(models.Model):
+        # ...
+
+    class RelatedModel(PolymorphicModel):
+        my_model = models.ForeignKey(
+            MyModel,
+            on_delete=PolymorphicGuard(models.CASCADE),
+        )
+

docs/index.rst

@@ -119,6 +119,7 @@ Advanced topics
    formsets
    migrating
    managers
+   deletion
    advanced
    changelog
    api/index

justfile

@@ -97,6 +97,7 @@ build: build-docs-html
 # regenerate test migrations using the lowest version of Django
 make-test-migrations:
     - rm src/polymorphic/tests/migrations/00*.py
+    - rm src/polymorphic/tests/deletion/migrations/00*.py
     uv run --isolated --resolution lowest-direct --script ./manage.py makemigrations
 
 # open the html documentation

pyproject.toml

@@ -143,6 +143,7 @@ show_missing = true
 dev = [
     "coverage>=7.6.1",
     "dj-database-url>=2.2.0",
+    "django-test-migrations>=1.5.0",
     "ipdb>=0.13.13",
     "ipython>=8.18.1",
     "mypy>=1.14.1",

src/polymorphic/base.py

@@ -10,6 +10,7 @@
 from django.db import models
 from django.db.models.base import ModelBase
 
+from .deletion import PolymorphicGuard
 from .managers import PolymorphicManager
 from .query import PolymorphicQuerySet
 
@@ -75,6 +76,14 @@ def __new__(cls, model_name, bases, attrs, **kwargs):
         if new_class._meta.pk:
             new_class.polymorphic_primary_key_name = new_class._meta.pk.name
 
+        # wrap on_delete handlers of reverse relations back to this model with the
+        # polymorphic deletion guard
+        for fk in new_class._meta.fields:
+            if isinstance(fk, (models.ForeignKey, models.OneToOneField)) and not isinstance(
+                fk.remote_field.on_delete, PolymorphicGuard
+            ):
+                fk.remote_field.on_delete = PolymorphicGuard(fk.remote_field.on_delete)
+
         return new_class
 
     @classmethod

src/polymorphic/deletion.py

@@ -0,0 +1,64 @@
+"""
+Classes and utilities for handling deletions in polymorphic models.
+"""
+
+from django.db.migrations.serializer import BaseSerializer, serializer_factory
+from django.db.migrations.writer import MigrationWriter
+
+from .query import PolymorphicQuerySet
+
+
+class PolymorphicGuard:
+    """
+    Wrap an :attr:`django.db.models.ForeignKey.on_delete` callable
+    (CASCADE/PROTECT/SET_NULL/SET(...)/custom), but serialize as the underlying
+    callable.
+
+    :param action: The :attr:`django.db.models.ForeignKey.on_delete` callable to wrap.
+    """
+
+    def __init__(self, action):
+        if not callable(action):
+            raise TypeError("action must be callable")
+        self.action = action
+
+    def __call__(self, collector, field, sub_objs, using):
+        """
+        This guard wraps an on_delete action to ensure that any polymorphic queryset
+        passed to it is converted to a non-polymorphic queryset before proceeding.
+        This prevents issues with cascading deletes on polymorphic models.
+
+        This guard should be automatically applied to reverse relations such that
+
+        .. code-block:: python
+
+            class MyModel(PolymorphicModel):
+                related = models.ForeignKey(
+                    OtherModel,
+                    on_delete=models.CASCADE # <- equal to PolymorphicGuard(models.CASCADE)
+                )
+
+        """
+        if isinstance(sub_objs, PolymorphicQuerySet) and not sub_objs.polymorphic_disabled:
+            sub_objs = sub_objs.non_polymorphic()
+        return self.action(collector, field, sub_objs, using)
+
+
+class PolymorphicGuardSerializer(BaseSerializer):
+    """
+    A serializer for PolymorphicGuard that serializes the underlying action.
+
+    There is no need to serialize the PolymorphicGuard itself, as it is just a wrapper
+    that ensures that polymorphic querysets are converted to non-polymorphic but no
+    polymorphic managers are present in migrations. This also ensures that new
+    migrations will not be generated.
+    """
+
+    def serialize(self):
+        """
+        Serialize the underlying action of the PolymorphicGuard.
+        """
+        return serializer_factory(self.value.action).serialize()
+
+
+MigrationWriter.register_serializer(PolymorphicGuard, PolymorphicGuardSerializer)

src/polymorphic/query.py

@@ -546,3 +546,12 @@ def get_real_instances(self, base_result_objects=None):
             return olist
         clist = PolymorphicQuerySet._p_list_class(olist)
         return clist
+
+    def delete(self):
+        """
+        Deletion will be done non-polymorphically because Django's multi-table deletion
+        mechanism is already walking the class hierarchy and producing a correct
+        deletion graph. Introducing polymorphic querysets into the deletion process
+        disrupts the model hierarchy/relationship traversal.
+        """
+        return QuerySet.delete(self.non_polymorphic())