Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

[1.0.X] Fixed #2698 -- Fixed deleting in the presence of custom manag…

…ers.

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

Backport of r10104 from trunk.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.0.X@10106 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit ebfe7faaa3efef952d092b332b79f182ea5f45d1 1 parent 083a720
@malcolmt malcolmt authored
View
12 django/db/models/base.py
@@ -437,7 +437,17 @@ def _collect_sub_objects(self, seen_objs, parent=None, nullable=False):
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
View
44 django/db/models/fields/related.py
@@ -181,7 +181,7 @@ def __get__(self, instance, instance_type=None):
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
@@ -289,13 +289,36 @@ def __get__(self, instance, instance_type=None):
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))
@@ -345,17 +368,6 @@ def clear(self):
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."""
View
52 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 @@ def __unicode__(self):
__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()
+[]
+
"""
}
Please sign in to comment.
Something went wrong with that request. Please try again.