Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Use plain model.Manager, or suitable proxy, for model saving.

We can't use the default manager in Model.save_base(), since we need to
retrieve existing objects which might be filtered out by that manager. We now
always use a plain Manager instance at that point (or something that can
replace it, such as a GeoManager), making all existing rows in the
database visible to the saving code.

The logic for detecting a "suitable replacement" plain base is the same as for
related fields: if the use_for_related_fields is set on the manager subclass,
we can use it. The general requirement here is that we want a base class that
returns the appropriate QuerySet subclass, but does not restrict the rows
returned.

Fixed #8990, #9527.

Refs #2698 (which is not fixed by this change, but it's the first part of a
larger change to fix that bug.)

git-svn-id: http://code.djangoproject.com/svn/django/trunk@10056 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 7d9b29a56ddc1a793a02d55743c372a8256b97aa 1 parent 957c721
@malcolmt malcolmt authored
View
3  django/db/models/base.py
@@ -69,6 +69,7 @@ def __new__(cls, name, bases, attrs):
if getattr(new_class, '_default_manager', None):
new_class._default_manager = None
+ new_class._base_manager = None
# Bail out early if we have already created this class.
m = get_model(new_class._meta.app_label, name, False)
@@ -369,7 +370,7 @@ def save_base(self, raw=False, cls=None, force_insert=False,
pk_val = self._get_pk_val(meta)
pk_set = pk_val is not None
record_exists = True
- manager = cls._default_manager
+ manager = cls._base_manager
if pk_set:
# Determine whether a record with the primary key already exists.
if (force_update or (not force_insert and
View
26 django/db/models/manager.py
@@ -5,8 +5,16 @@
from django.db.models.fields import FieldDoesNotExist
def ensure_default_manager(sender, **kwargs):
+ """
+ Ensures that a Model subclass contains a default manager and sets the
+ _default_manager attribute on the class. Also sets up the _base_manager
+ points to a plain Manager instance (which could be the same as
+ _default_manager if it's not a subclass of Manager).
+ """
cls = sender
- if not getattr(cls, '_default_manager', None) and not cls._meta.abstract:
+ if cls._meta.abstract:
+ return
+ if not getattr(cls, '_default_manager', None):
# Create the default manager, if needed.
try:
cls._meta.get_field('objects')
@@ -14,6 +22,22 @@ def ensure_default_manager(sender, **kwargs):
except FieldDoesNotExist:
pass
cls.add_to_class('objects', Manager())
+ cls._base_manager = cls.objects
+ elif not getattr(cls, '_base_manager', None):
+ default_mgr = cls._default_manager.__class__
+ if (default_mgr is Manager or
+ getattr(default_mgr, "use_for_related_fields", False)):
+ cls._base_manager = cls._default_manager
+ else:
+ # Default manager isn't a plain Manager class, or a suitable
+ # replacement, so we walk up the base class hierarchy until we hit
+ # something appropriate.
+ for base_class in default_mgr.mro()[1:]:
+ if (base_class is Manager or
+ getattr(base_class, "use_for_related_fields", False)):
+ cls.add_to_class('_base_manager', base_class())
+ return
+ raise AssertionError("Should never get here. Please report a bug, including your model and model manager setup.")
signals.class_prepared.connect(ensure_default_manager)
View
0  tests/regressiontests/custom_managers_regress/__init__.py
No changes.
View
38 tests/regressiontests/custom_managers_regress/models.py
@@ -0,0 +1,38 @@
+"""
+Regression tests for custom manager classes.
+"""
+
+from django.db import models
+
+class RestrictedManager(models.Manager):
+ """
+ A manager that filters out non-public instances.
+ """
+ def get_query_set(self):
+ return super(RestrictedManager, self).get_query_set().filter(is_public=True)
+
+class RestrictedClass(models.Model):
+ name = models.CharField(max_length=50)
+ is_public = models.BooleanField(default=False)
+
+ objects = RestrictedManager()
+ plain_manager = models.Manager()
+
+ def __unicode__(self):
+ return self.name
+
+__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")
+>>> 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()
+1
+
+"""
+}
Please sign in to comment.
Something went wrong with that request. Please try again.