Fixed #2698 -- Fixed deleting in the presence of custom managers.

A custom manager on a related object that filtered away objects would prevent
those objects being deleted via the relation. This is now fixed.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@10104 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/db/models/base.py

@@ -519,7 +519,17 @@ class Model(object):
                 else:
                     sub_obj._collect_sub_objects(seen_objs, self.__class__, related.field.null)
             else:
-                for sub_obj in getattr(self, rel_opts_name).all():
+                # To make sure we can access all elements, we can't use the
+                # normal manager on the related object. So we work directly
+                # with the descriptor object.
+                for cls in self.__class__.mro():
+                    if rel_opts_name in cls.__dict__:
+                        rel_descriptor = cls.__dict__[rel_opts_name]
+                        break
+                else:
+                    raise AssertionError("Should never get here.")
+                delete_qs = rel_descriptor.delete_manager(self).all()
+                for sub_obj in delete_qs:
                     sub_obj._collect_sub_objects(seen_objs, self.__class__, related.field.null)
 
         # Handle any ancestors (for the model-inheritance case). We do this by

django/db/models/fields/related.py

@@ -188,7 +188,7 @@ class SingleRelatedObjectDescriptor(object):
             return getattr(instance, self.cache_name)
         except AttributeError:
             params = {'%s__pk' % self.related.field.name: instance._get_pk_val()}
-            rel_obj = self.related.model._default_manager.get(**params)
+            rel_obj = self.related.model._base_manager.get(**params)
             setattr(instance, self.cache_name, rel_obj)
             return rel_obj
 
@@ -296,13 +296,36 @@ class ForeignRelatedObjectsDescriptor(object):
         if instance is None:
             return self
 
+        return self.create_manager(instance,
+                self.related.model._default_manager.__class__)
+
+    def __set__(self, instance, value):
+        if instance is None:
+            raise AttributeError, "Manager must be accessed via instance"
+
+        manager = self.__get__(instance)
+        # If the foreign key can support nulls, then completely clear the related set.
+        # Otherwise, just move the named objects into the set.
+        if self.related.field.null:
+            manager.clear()
+        manager.add(*value)
+
+    def delete_manager(self, instance):
+        """
+        Returns a queryset based on the related model's base manager (rather
+        than the default manager, as returned by __get__). Used by
+        Model.delete().
+        """
+        return self.create_manager(instance,
+                self.related.model._base_manager.__class__)
+
+    def create_manager(self, instance, superclass):
+        """
+        Creates the managers used by other methods (__get__() and delete()).
+        """
         rel_field = self.related.field
         rel_model = self.related.model
 
-        # Dynamically create a class that subclasses the related
-        # model's default manager.
-        superclass = self.related.model._default_manager.__class__
-
         class RelatedManager(superclass):
             def get_query_set(self):
                 return superclass.get_query_set(self).filter(**(self.core_filters))
@@ -352,17 +375,6 @@ class ForeignRelatedObjectsDescriptor(object):
 
         return manager
 
-    def __set__(self, instance, value):
-        if instance is None:
-            raise AttributeError, "Manager must be accessed via instance"
-
-        manager = self.__get__(instance)
-        # If the foreign key can support nulls, then completely clear the related set.
-        # Otherwise, just move the named objects into the set.
-        if self.related.field.null:
-            manager.clear()
-        manager.add(*value)
-
 def create_many_related_manager(superclass, through=False):
     """Creates a manager that subclasses 'superclass' (which is a Manager)
     and adds behavior for many-to-many related objects."""

tests/regressiontests/custom_managers_regress/models.py

@@ -11,9 +11,27 @@ class RestrictedManager(models.Manager):
     def get_query_set(self):
         return super(RestrictedManager, self).get_query_set().filter(is_public=True)
 
-class RestrictedClass(models.Model):
+class RelatedModel(models.Model):
+    name = models.CharField(max_length=50)
+
+    def __unicode__(self):
+        return self.name
+
+class RestrictedModel(models.Model):
     name = models.CharField(max_length=50)
     is_public = models.BooleanField(default=False)
+    related = models.ForeignKey(RelatedModel)
+
+    objects = RestrictedManager()
+    plain_manager = models.Manager()
+
+    def __unicode__(self):
+        return self.name
+
+class OneToOneRestrictedModel(models.Model):
+    name = models.CharField(max_length=50)
+    is_public = models.BooleanField(default=False)
+    related = models.OneToOneField(RelatedModel)
 
     objects = RestrictedManager()
     plain_manager = models.Manager()
@@ -24,15 +42,41 @@ class RestrictedClass(models.Model):
 __test__ = {"tests": """
 Even though the default manager filters out some records, we must still be able
 to save (particularly, save by updating existing records) those filtered
-instances. This is a regression test for #FIXME.
->>> obj = RestrictedClass.objects.create(name="hidden")
+instances. This is a regression test for #8990, #9527
+>>> related = RelatedModel.objects.create(name="xyzzy")
+>>> obj = RestrictedModel.objects.create(name="hidden", related=related)
 >>> obj.name = "still hidden"
 >>> obj.save()
 
 # If the hidden object wasn't seen during the save process, there would now be
 # two objects in the database.
->>> RestrictedClass.plain_manager.count()
+>>> RestrictedModel.plain_manager.count()
 1
 
+Deleting related objects should also not be distracted by a restricted manager
+on the related object. This is a regression test for #2698.
+>>> RestrictedModel.plain_manager.all().delete()
+>>> for name, public in (('one', True), ('two', False), ('three', False)):
+...     _ = RestrictedModel.objects.create(name=name, is_public=public, related=related)
+
+# Reload the RelatedModel instance, just to avoid any instance artifacts.
+>>> obj = RelatedModel.objects.get(name="xyzzy")
+>>> obj.delete()
+
+# All of the RestrictedModel instances should have been deleted, since they
+# *all* pointed to the RelatedModel. If the default manager is used, only the
+# public one will be deleted.
+>>> RestrictedModel.plain_manager.all()
+[]
+
+# The same test case as the last one, but for one-to-one models, which are
+# implemented slightly different internally, so it's a different code path.
+>>> obj = RelatedModel.objects.create(name="xyzzy")
+>>> _ = OneToOneRestrictedModel.objects.create(name="foo", is_public=False, related=obj)
+>>> obj = RelatedModel.objects.get(name="xyzzy")
+>>> obj.delete()
+>>> OneToOneRestrictedModel.plain_manager.all()
+[]
+
 """
 }