From 7d9b29a56ddc1a793a02d55743c372a8256b97aa Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Sun, 15 Mar 2009 03:41:33 +0000 Subject: [PATCH] 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 --- django/db/models/base.py | 3 +- django/db/models/manager.py | 26 ++++++++++++- .../custom_managers_regress/__init__.py | 0 .../custom_managers_regress/models.py | 38 +++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/regressiontests/custom_managers_regress/__init__.py create mode 100644 tests/regressiontests/custom_managers_regress/models.py 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 4e8c6e94fb..6869ad2eb5 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 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 + +""" +}