Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

[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
  • Loading branch information...
commit 97321c7645f200333bc43ac2f4a22d78dd4154ea 1 parent 0eac35c
Malcolm Tredinnick authored March 15, 2009
3  django/db/models/base.py
@@ -69,6 +69,7 @@ def __new__(cls, name, bases, attrs):
69 69
 
70 70
         if getattr(new_class, '_default_manager', None):
71 71
             new_class._default_manager = None
  72
+            new_class._base_manager = None
72 73
 
73 74
         # Bail out early if we have already created this class.
74 75
         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,
369 370
         pk_val = self._get_pk_val(meta)
370 371
         pk_set = pk_val is not None
371 372
         record_exists = True
372  
-        manager = cls._default_manager
  373
+        manager = cls._base_manager
373 374
         if pk_set:
374 375
             # Determine whether a record with the primary key already exists.
375 376
             if (force_update or (not force_insert and
26  django/db/models/manager.py
@@ -5,8 +5,16 @@
5 5
 from django.db.models.fields import FieldDoesNotExist
6 6
 
7 7
 def ensure_default_manager(sender, **kwargs):
  8
+    """
  9
+    Ensures that a Model subclass contains a default manager  and sets the
  10
+    _default_manager attribute on the class. Also sets up the _base_manager
  11
+    points to a plain Manager instance (which could be the same as
  12
+    _default_manager if it's not a subclass of Manager).
  13
+    """
8 14
     cls = sender
9  
-    if not getattr(cls, '_default_manager', None) and not cls._meta.abstract:
  15
+    if cls._meta.abstract:
  16
+        return
  17
+    if not getattr(cls, '_default_manager', None):
10 18
         # Create the default manager, if needed.
11 19
         try:
12 20
             cls._meta.get_field('objects')
@@ -14,6 +22,22 @@ def ensure_default_manager(sender, **kwargs):
14 22
         except FieldDoesNotExist:
15 23
             pass
16 24
         cls.add_to_class('objects', Manager())
  25
+        cls._base_manager = cls.objects
  26
+    elif not getattr(cls, '_base_manager', None):
  27
+        default_mgr = cls._default_manager.__class__
  28
+        if (default_mgr is Manager or
  29
+                getattr(default_mgr, "use_for_related_fields", False)):
  30
+            cls._base_manager = cls._default_manager
  31
+        else:
  32
+            # Default manager isn't a plain Manager class, or a suitable
  33
+            # replacement, so we walk up the base class hierarchy until we hit
  34
+            # something appropriate.
  35
+            for base_class in default_mgr.mro()[1:]:
  36
+                if (base_class is Manager or
  37
+                        getattr(base_class, "use_for_related_fields", False)):
  38
+                    cls.add_to_class('_base_manager', base_class())
  39
+                    return
  40
+            raise AssertionError("Should never get here. Please report a bug, including your model and model manager setup.")
17 41
 
18 42
 signals.class_prepared.connect(ensure_default_manager)
19 43
 
0  tests/regressiontests/custom_managers_regress/__init__.py
No changes.
38  tests/regressiontests/custom_managers_regress/models.py
... ...
@@ -0,0 +1,38 @@
  1
+"""
  2
+Regression tests for custom manager classes.
  3
+"""
  4
+
  5
+from django.db import models
  6
+
  7
+class RestrictedManager(models.Manager):
  8
+    """
  9
+    A manager that filters out non-public instances.
  10
+    """
  11
+    def get_query_set(self):
  12
+        return super(RestrictedManager, self).get_query_set().filter(is_public=True)
  13
+
  14
+class RestrictedClass(models.Model):
  15
+    name = models.CharField(max_length=50)
  16
+    is_public = models.BooleanField(default=False)
  17
+
  18
+    objects = RestrictedManager()
  19
+    plain_manager = models.Manager()
  20
+
  21
+    def __unicode__(self):
  22
+        return self.name
  23
+
  24
+__test__ = {"tests": """
  25
+Even though the default manager filters out some records, we must still be able
  26
+to save (particularly, save by updating existing records) those filtered
  27
+instances. This is a regression test for #FIXME.
  28
+>>> obj = RestrictedClass.objects.create(name="hidden")
  29
+>>> obj.name = "still hidden"
  30
+>>> obj.save()
  31
+
  32
+# If the hidden object wasn't seen during the save process, there would now be
  33
+# two objects in the database.
  34
+>>> RestrictedClass.plain_manager.count()
  35
+1
  36
+
  37
+"""
  38
+}

0 notes on commit 97321c7

Please sign in to comment.
Something went wrong with that request. Please try again.