summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMalcolm Tredinnick <malcolm.tredinnick@gmail.com>2009-03-15 03:45:32 +0000
committerMalcolm Tredinnick <malcolm.tredinnick@gmail.com>2009-03-15 03:45:32 +0000
commit97321c7645f200333bc43ac2f4a22d78dd4154ea (patch)
treea579968ea9ebbca4b53a6056f52bfa9f3cfb3a26
parent0eac35ce119f108d2ffaeea745101cc3c4a85b98 (diff)
[1.0.X] 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.) Backport of r10056 from trunk. git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.0.X@10059 bcc190cf-cafb-0310-a4f2-bffc1f526a37
-rw-r--r--django/db/models/base.py3
-rw-r--r--django/db/models/manager.py26
-rw-r--r--tests/regressiontests/custom_managers_regress/__init__.py0
-rw-r--r--tests/regressiontests/custom_managers_regress/models.py38
4 files changed, 65 insertions, 2 deletions
diff --git a/django/db/models/base.py b/django/db/models/base.py
index 62ebbab7f0..dec0316947 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -69,6 +69,7 @@ class ModelBase(type):
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 @@ class Model(object):
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
diff --git a/django/db/models/manager.py b/django/db/models/manager.py
index 683a8f8d10..b9b4978e69 100644
--- a/django/db/models/manager.py
+++ b/django/db/models/manager.py
@@ -5,8 +5,16 @@ from django.db.models import signals
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)
diff --git a/tests/regressiontests/custom_managers_regress/__init__.py b/tests/regressiontests/custom_managers_regress/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/regressiontests/custom_managers_regress/__init__.py
diff --git a/tests/regressiontests/custom_managers_regress/models.py b/tests/regressiontests/custom_managers_regress/models.py
new file mode 100644
index 0000000000..67fed84447
--- /dev/null
+++ b/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
+
+"""
+}