summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--django/db/migrations/state.py3
-rw-r--r--django/db/models/base.py33
-rw-r--r--django/db/models/manager.py136
-rw-r--r--django/db/models/options.py48
-rw-r--r--docs/topics/db/managers.txt39
-rw-r--r--docs/topics/db/models.txt32
-rw-r--r--tests/auth_tests/test_basic.py13
-rw-r--r--tests/managers_regress/models.py4
-rw-r--r--tests/managers_regress/tests.py2
-rw-r--r--tests/many_to_one/tests.py4
-rw-r--r--tests/migrations/test_state.py8
-rw-r--r--tests/one_to_one/tests.py8
12 files changed, 112 insertions, 218 deletions
diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py
index 0134acde72..ca44a39ca3 100644
--- a/django/db/migrations/state.py
+++ b/django/db/migrations/state.py
@@ -500,8 +500,7 @@ class ModelState(object):
else:
# Force this manager to be the first and thus default
managers_mapping[default_manager_name] = (0, models.Manager())
- # Sort all managers by their creation counter
- for _, manager, _ in sorted(model._meta.managers):
+ for manager in model._meta.managers:
if manager.name == "_base_manager" or not manager.use_in_migrations:
continue
reconstruct_manager(manager)
diff --git a/django/db/models/base.py b/django/db/models/base.py
index 19dca3926d..f7735985dd 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -151,18 +151,6 @@ class ModelBase(type):
if is_proxy and base_meta and base_meta.swapped:
raise TypeError("%s cannot proxy the swapped model '%s'." % (name, base_meta.swapped))
- if getattr(new_class, '_default_manager', None):
- if not is_proxy:
- # Multi-table inheritance doesn't inherit default manager from
- # parents.
- new_class._default_manager = None
- new_class._base_manager = None
- else:
- # Proxy classes do inherit parent's default manager, if none is
- # set explicitly.
- new_class._default_manager = new_class._default_manager._copy_to_model(new_class)
- new_class._base_manager = new_class._base_manager._copy_to_model(new_class)
-
# Add all attributes to the class.
for obj_name, obj in attrs.items():
new_class.add_to_class(obj_name, obj)
@@ -217,7 +205,6 @@ class ModelBase(type):
inherited_attributes = set()
# Do the appropriate setup for any model parents.
for base in new_class.mro():
- original_base = base
if base not in parents or not hasattr(base, '_meta'):
# Things without _meta aren't functional models, so they're
# uninteresting parents.
@@ -294,14 +281,6 @@ class ModelBase(type):
# Pass any non-abstract parent classes onto child.
new_class._meta.parents.update(base_parents)
- # Inherit managers from the abstract base classes.
- new_class.copy_managers(base._meta.abstract_managers)
-
- # Proxy models inherit the non-abstract managers from their base,
- # unless they have redefined any of them.
- if is_proxy:
- new_class.copy_managers(original_base._meta.concrete_managers)
-
# Inherit private fields (like GenericForeignKey) from the parent
# class
for field in base._meta.private_fields:
@@ -330,15 +309,6 @@ class ModelBase(type):
new_class._meta.apps.register_model(new_class._meta.app_label, new_class)
return new_class
- def copy_managers(cls, base_managers):
- # This is in-place sorting of an Options attribute, but that's fine.
- base_managers.sort()
- for _, mgr_name, manager in base_managers: # NOQA (redefinition of _)
- val = getattr(cls, mgr_name, None)
- if not val or val is manager:
- new_manager = manager._copy_to_model(cls)
- cls.add_to_class(mgr_name, new_manager)
-
def add_to_class(cls, name, value):
# We should call the contribute_to_class method only if it's bound
if not inspect.isclass(value) and hasattr(value, 'contribute_to_class'):
@@ -376,6 +346,7 @@ class ModelBase(type):
setattr(cls, 'get_absolute_url', get_absolute_url_override)
ensure_default_manager(cls)
+
signals.class_prepared.send(sender=cls)
@@ -1263,7 +1234,7 @@ class Model(six.with_metaclass(ModelBase)):
""" Perform all manager checks. """
errors = []
- for __, manager, __ in cls._meta.managers:
+ for manager in cls._meta.managers:
errors.extend(manager.check(**kwargs))
return errors
diff --git a/django/db/models/manager.py b/django/db/models/manager.py
index 4636869a12..b8aa295ba3 100644
--- a/django/db/models/manager.py
+++ b/django/db/models/manager.py
@@ -8,43 +8,40 @@ from django.utils import six
from django.utils.encoding import python_2_unicode_compatible
-def ensure_default_manager(cls):
+def can_use_for_related_field(manager_class):
+ return manager_class is Manager or getattr(manager_class, 'use_for_related_fields', False)
+
+
+def ensure_default_manager(model):
"""
- 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).
+ Ensures that a Model subclass contains a default manager and sets the
+ _default_manager and _base_manager attributes on the class.
"""
- if cls._meta.swapped:
- setattr(cls, 'objects', SwappedManagerDescriptor(cls))
- return
- if not getattr(cls, '_default_manager', None):
- if any(f.name == 'objects' for f in cls._meta.fields):
+
+ if not model._meta.managers:
+ if any(f.name == 'objects' for f in model._meta.fields):
raise ValueError(
"Model %s must specify a custom Manager, because it has a "
- "field named 'objects'" % cls.__name__
+ "field named 'objects'" % model.__name__
)
- # Create the default manager, if needed.
- 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
+ model.add_to_class('objects', Manager())
+
+ model._default_manager = model._meta.managers[0]
+
+ # Just alias _base_manager if default manager is suitable.
+ if can_use_for_related_field(model._default_manager.__class__):
+ model._base_manager = model._default_manager
+
+ # Otherwise search for a suitable manager type in the default manager MRO.
+ else:
+ for base_manager_class in model._default_manager.__class__.mro()[1:]:
+ if can_use_for_related_field(base_manager_class):
+ model._base_manager = base_manager_class()
+ model._base_manager.name = '_base_manager'
+ model._base_manager.model = model
+ break
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."
- )
+ raise ValueError("Could not find a suitable base manager.")
@python_2_unicode_compatible
@@ -67,7 +64,6 @@ class BaseManager(object):
self._set_creation_counter()
self.model = None
self.name = None
- self._inherited = False
self._db = None
self._hints = {}
@@ -150,26 +146,13 @@ class BaseManager(object):
return type(class_name, (cls,), class_dict)
def contribute_to_class(self, model, name):
- # TODO: Use weakref because of possible memory leak / circular reference.
- self.model = model
if not self.name:
self.name = name
- # Only contribute the manager if the model is concrete
- if model._meta.abstract:
- setattr(model, name, AbstractManagerDescriptor(model))
- elif model._meta.swapped:
- setattr(model, name, SwappedManagerDescriptor(model))
- else:
- # if not model._meta.abstract and not model._meta.swapped:
- setattr(model, name, ManagerDescriptor(self))
- if (not getattr(model, '_default_manager', None) or
- self.creation_counter < model._default_manager.creation_counter):
- model._default_manager = self
+ self.model = model
+
+ setattr(model, name, ManagerDescriptor(self))
- abstract = False
- if model._meta.abstract or (self._inherited and not self.model._meta.proxy):
- abstract = True
- model._meta.managers.append((self.creation_counter, self, abstract))
+ model._meta.add_manager(self)
def _set_creation_counter(self):
"""
@@ -179,19 +162,6 @@ class BaseManager(object):
self.creation_counter = BaseManager.creation_counter
BaseManager.creation_counter += 1
- def _copy_to_model(self, model):
- """
- Makes a copy of the manager and assigns it to 'model', which should be
- a child of the existing model (used when inheriting a manager from an
- abstract base class).
- """
- assert issubclass(model, self.model)
- mgr = copy.copy(self)
- mgr._set_creation_counter()
- mgr.model = model
- mgr._inherited = True
- return mgr
-
def db_manager(self, using=None, hints=None):
obj = copy.copy(self)
obj._db = using or self._db
@@ -240,43 +210,29 @@ class Manager(BaseManager.from_queryset(QuerySet)):
class ManagerDescriptor(object):
- # This class ensures managers aren't accessible via model instances.
- # For example, Poll.objects works, but poll_obj.objects raises AttributeError.
+
def __init__(self, manager):
self.manager = manager
def __get__(self, instance, cls=None):
if instance is not None:
raise AttributeError("Manager isn't accessible via %s instances" % cls.__name__)
- return self.manager
-
-
-class AbstractManagerDescriptor(object):
- # This class provides a better error message when you try to access a
- # manager on an abstract model.
- def __init__(self, model):
- self.model = model
-
- def __get__(self, instance, cls=None):
- raise AttributeError("Manager isn't available; %s is abstract" % (
- self.model._meta.object_name,
- ))
+ if cls._meta.abstract:
+ raise AttributeError("Manager isn't available; %s is abstract" % (
+ cls._meta.object_name,
+ ))
-class SwappedManagerDescriptor(object):
- # This class provides a better error message when you try to access a
- # manager on a swapped model.
- def __init__(self, model):
- self.model = model
-
- def __get__(self, instance, cls=None):
- raise AttributeError(
- "Manager isn't available; '%s.%s' has been swapped for '%s'" % (
- self.model._meta.app_label,
- self.model._meta.object_name,
- self.model._meta.swapped,
+ if cls._meta.swapped:
+ raise AttributeError(
+ "Manager isn't available; '%s.%s' has been swapped for '%s'" % (
+ cls._meta.app_label,
+ cls._meta.object_name,
+ cls._meta.swapped,
+ )
)
- )
+
+ return cls._meta.managers_map[self.manager.name]
class EmptyManager(Manager):
diff --git a/django/db/models/options.py b/django/db/models/options.py
index 74a362e99c..389ba7c93e 100644
--- a/django/db/models/options.py
+++ b/django/db/models/options.py
@@ -1,5 +1,6 @@
from __future__ import unicode_literals
+import copy
import warnings
from bisect import bisect
from collections import OrderedDict, defaultdict
@@ -73,7 +74,8 @@ def make_immutable_fields_list(name, data):
@python_2_unicode_compatible
class Options(object):
FORWARD_PROPERTIES = {'fields', 'many_to_many', 'concrete_fields',
- 'local_concrete_fields', '_forward_fields_map'}
+ 'local_concrete_fields', '_forward_fields_map',
+ 'managers', 'managers_map'}
REVERSE_PROPERTIES = {'related_objects', 'fields_map', '_relation_tree'}
default_apps = apps
@@ -83,6 +85,7 @@ class Options(object):
self.local_fields = []
self.local_many_to_many = []
self.private_fields = []
+ self.local_managers = []
self.model_name = None
self.verbose_name = None
self.verbose_name_plural = None
@@ -122,12 +125,6 @@ class Options(object):
self.parents = OrderedDict()
self.auto_created = False
- # To handle various inheritance situations, we need to track where
- # managers came from (concrete or abstract base classes). `managers`
- # keeps a list of 3-tuples of the form:
- # (creation_counter, instance, abstract(=True))
- self.managers = []
-
# List of all lookups defined in ForeignKey 'limit_choices_to' options
# from *other* models. Needed for some admin checks. Internal use only.
self.related_fkey_lookups = []
@@ -154,20 +151,6 @@ class Options(object):
def installed(self):
return self.app_config is not None
- @property
- def abstract_managers(self):
- return [
- (counter, instance.name, instance) for counter, instance, abstract
- in self.managers if abstract
- ]
-
- @property
- def concrete_managers(self):
- return [
- (counter, instance.name, instance) for counter, instance, abstract
- in self.managers if not abstract
- ]
-
def contribute_to_class(self, cls, name):
from django.db import connection
from django.db.backends.utils import truncate_name
@@ -264,6 +247,10 @@ class Options(object):
auto = AutoField(verbose_name='ID', primary_key=True, auto_created=True)
model.add_to_class('id', auto)
+ def add_manager(self, manager):
+ self.local_managers.append(manager)
+ self._expire_cache()
+
def add_field(self, field, private=False, virtual=NOT_PROVIDED):
if virtual is not NOT_PROVIDED:
warnings.warn(
@@ -372,6 +359,25 @@ class Options(object):
return None
@cached_property
+ def managers(self):
+ managers = []
+ bases = (b for b in self.model.mro() if hasattr(b, '_meta'))
+ for depth, base in enumerate(bases):
+ for manager in base._meta.local_managers:
+ manager = copy.copy(manager)
+ manager.model = self.model
+ managers.append((depth, manager.creation_counter, manager))
+
+ return make_immutable_fields_list(
+ "managers",
+ (m[2] for m in sorted(managers)),
+ )
+
+ @cached_property
+ def managers_map(self):
+ return {manager.name: manager for manager in reversed(self.managers)}
+
+ @cached_property
def fields(self):
"""
Returns a list of all forward fields on the model and its parents,
diff --git a/docs/topics/db/managers.txt b/docs/topics/db/managers.txt
index a82b78e1a0..80598f0c98 100644
--- a/docs/topics/db/managers.txt
+++ b/docs/topics/db/managers.txt
@@ -321,33 +321,26 @@ You may also store the generated class into a variable::
Custom managers and model inheritance
-------------------------------------
-Class inheritance and model managers aren't quite a perfect match for each
-other. Managers are often specific to the classes they are defined on and
-inheriting them in subclasses isn't necessarily a good idea. Also, because the
-first manager declared is the *default manager*, it is important to allow that
-to be controlled. So here's how Django handles custom managers and
+Here's how Django handles custom managers and
:ref:`model inheritance <model-inheritance>`:
-1. Managers defined on non-abstract base classes are *not* inherited by
- child classes. If you want to reuse a manager from a non-abstract base,
- redeclare it explicitly on the child class. These sorts of managers are
- likely to be fairly specific to the class they are defined on, so
- inheriting them can often lead to unexpected results (particularly as
- far as the default manager goes). Therefore, they aren't passed onto
- child classes.
-
-2. Managers from abstract base classes are always inherited by the child
- class, using Python's normal name resolution order (names on the child
+1. Managers from base classes are always inherited by the child class,
+ using Python's normal name resolution order (names on the child
class override all others; then come names on the first parent class,
- and so on). Abstract base classes are designed to capture information
- and behavior that is common to their child classes. Defining common
- managers is an appropriate part of this common information.
+ and so on).
+
+2. The default manager on a class is either the first manager declared on the
+ class, if that exists, or the default manager of the first parent class in
+ the parent hierarchy, if that exists. If no manager is explicitly declared,
+ Django automatically creates the `objects` manager and it becomes the default
+ manager.
+
+.. versionchanged:: 1.10
-3. The default manager on a class is either the first manager declared on
- the class, if that exists, or the default manager of the first abstract
- base class in the parent hierarchy, if that exists. If no default
- manager is explicitly declared, Django's normal default manager is
- used.
+ In older versions, manager inheritance varied depending on the type of
+ model inheritance (i.e. :ref:`abstract-base-classes`,
+ :ref:`multi-table-inheritance`, or :ref:`proxy-models`), especially
+ with regards to electing the default manager.
These rules provide the necessary flexibility if you want to install a
collection of custom managers on a group of models, via an abstract base
diff --git a/docs/topics/db/models.txt b/docs/topics/db/models.txt
index 1dc1b17202..69562f6544 100644
--- a/docs/topics/db/models.txt
+++ b/docs/topics/db/models.txt
@@ -1287,33 +1287,19 @@ Differences between proxy inheritance and unmanaged models
Proxy model inheritance might look fairly similar to creating an unmanaged
model, using the :attr:`~django.db.models.Options.managed` attribute on a
-model's ``Meta`` class. The two alternatives are not quite the same and it's
-worth considering which one you should use.
+model's ``Meta`` class.
-One difference is that you can (and, in fact, must unless you want an empty
-model) specify model fields on models with ``Meta.managed=False``. You could,
-with careful setting of :attr:`Meta.db_table
-<django.db.models.Options.db_table>` create an unmanaged model that shadowed
-an existing model and add Python methods to it. However, that would be very
-repetitive and fragile as you need to keep both copies synchronized if you
+With careful setting of :attr:`Meta.db_table
+<django.db.models.Options.db_table>` you could create an unmanaged model that
+shadows an existing model and adds Python methods to it. However, that would be
+very repetitive and fragile as you need to keep both copies synchronized if you
make any changes.
-The other difference that is more important for proxy models, is how model
-managers are handled. Proxy models are intended to behave exactly like the
-model they are proxying for. So they inherit the parent model's managers,
-including the default manager. In the normal multi-table model inheritance
-case, children do not inherit managers from their parents as the custom
-managers aren't always appropriate when extra fields are involved. The
-:ref:`manager documentation <custom-managers-and-inheritance>` has more
-details about this latter case.
+On the other hand, proxy models are intended to behave exactly like the model
+they are proxying for. They are always in sync with the parent model since they
+directly inherit its fields and managers.
-When these two features were implemented, attempts were made to squash them
-into a single option. It turned out that interactions with inheritance, in
-general, and managers, in particular, made the API very complicated and
-potentially difficult to understand and use. It turned out that two options
-were needed in any case, so the current separation arose.
-
-So, the general rules are:
+The general rules are:
1. If you are mirroring an existing model or database table and don't want
all the original database table columns, use ``Meta.managed=False``.
diff --git a/tests/auth_tests/test_basic.py b/tests/auth_tests/test_basic.py
index 87122d9229..caefa9c7a0 100644
--- a/tests/auth_tests/test_basic.py
+++ b/tests/auth_tests/test_basic.py
@@ -3,29 +3,16 @@ from __future__ import unicode_literals
import warnings
-from django.apps import apps
from django.contrib.auth import get_user_model
from django.contrib.auth.models import AnonymousUser, User
from django.core.exceptions import ImproperlyConfigured
from django.db import IntegrityError
-from django.dispatch import receiver
from django.test import TestCase, override_settings
-from django.test.signals import setting_changed
from django.utils import translation
from .models import CustomUser
-@receiver(setting_changed)
-def user_model_swapped(**kwargs):
- if kwargs['setting'] == 'AUTH_USER_MODEL':
- from django.db.models.manager import ensure_default_manager
- # Reset User manager
- setattr(User, 'objects', User._default_manager)
- ensure_default_manager(User)
- apps.clear_cache()
-
-
class BasicTestCase(TestCase):
def test_user(self):
"Check that users can be created and can set their password"
diff --git a/tests/managers_regress/models.py b/tests/managers_regress/models.py
index bc2fa91c6a..b39e69b9ad 100644
--- a/tests/managers_regress/models.py
+++ b/tests/managers_regress/models.py
@@ -115,14 +115,12 @@ class Child5(AbstractBase3):
return self.name
-# Will inherit managers from AbstractBase1, but not Child4.
class Child6(Child4):
value = models.IntegerField()
-# Will not inherit default manager from parent.
class Child7(Parent):
- pass
+ objects = models.Manager()
# RelatedManagers
diff --git a/tests/managers_regress/tests.py b/tests/managers_regress/tests.py
index 746a59a9bb..3a51ff7542 100644
--- a/tests/managers_regress/tests.py
+++ b/tests/managers_regress/tests.py
@@ -50,7 +50,7 @@ class ManagersRegressionTests(TestCase):
])
self.assertQuerysetEqual(Child4.manager1.all(), ["<Child4: d1>", "<Child4: f1>"], ordered=False)
self.assertQuerysetEqual(Child5._default_manager.all(), ["<Child5: fred>"])
- self.assertQuerysetEqual(Child6._default_manager.all(), ["<Child6: f1>"])
+ self.assertQuerysetEqual(Child6._default_manager.all(), ["<Child6: f1>", "<Child6: f2>"], ordered=False)
self.assertQuerysetEqual(
Child7._default_manager.order_by('name'),
["<Child7: barney>", "<Child7: fred>"]
diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py
index c5a0e3f8f3..eaa01bbfcb 100644
--- a/tests/many_to_one/tests.py
+++ b/tests/many_to_one/tests.py
@@ -600,13 +600,13 @@ class ManyToOneTests(TestCase):
# If the manager is marked "use_for_related_fields", it'll get used instead
# of the "bare" queryset. Usually you'd define this as a property on the class,
# but this approximates that in a way that's easier in tests.
- School.objects.use_for_related_fields = True
+ School._default_manager.use_for_related_fields = True
try:
private_student = Student.objects.get(pk=private_student.pk)
with self.assertRaises(School.DoesNotExist):
private_student.school
finally:
- School.objects.use_for_related_fields = False
+ School._default_manager.use_for_related_fields = False
def test_hasattr_related_object(self):
# The exception raised on attribute access when a related object
diff --git a/tests/migrations/test_state.py b/tests/migrations/test_state.py
index aae2b71aac..f2e91267e9 100644
--- a/tests/migrations/test_state.py
+++ b/tests/migrations/test_state.py
@@ -262,13 +262,11 @@ class StateTests(SimpleTestCase):
self.assertEqual(len(new_apps.get_model("migrations", "SubTag")._meta.local_fields), 2)
Food = new_apps.get_model("migrations", "Food")
- managers = sorted(Food._meta.managers)
- self.assertEqual([mgr.name for _, mgr, _ in managers],
+ self.assertEqual([mgr.name for mgr in Food._meta.managers],
['default', 'food_mgr1', 'food_mgr2'])
- self.assertTrue(all(isinstance(mgr.name, six.text_type) for _, mgr, _ in managers))
- self.assertEqual([mgr.__class__ for _, mgr, _ in managers],
+ self.assertTrue(all(isinstance(mgr.name, six.text_type) for mgr in Food._meta.managers))
+ self.assertEqual([mgr.__class__ for mgr in Food._meta.managers],
[models.Manager, FoodManager, FoodManager])
- self.assertIs(managers[0][1], Food._default_manager)
def test_render_model_inheritance(self):
class Book(models.Model):
diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py
index 074a95f1b5..ecb009f961 100644
--- a/tests/one_to_one/tests.py
+++ b/tests/one_to_one/tests.py
@@ -457,21 +457,21 @@ class OneToOneTests(TestCase):
# If the manager is marked "use_for_related_fields", it'll get used instead
# of the "bare" queryset. Usually you'd define this as a property on the class,
# but this approximates that in a way that's easier in tests.
- School.objects.use_for_related_fields = True
+ School._default_manager.use_for_related_fields = True
try:
private_director = Director._base_manager.get(pk=private_director.pk)
with self.assertRaises(School.DoesNotExist):
private_director.school
finally:
- School.objects.use_for_related_fields = False
+ School._default_manager.use_for_related_fields = False
- Director.objects.use_for_related_fields = True
+ Director._default_manager.use_for_related_fields = True
try:
private_school = School._base_manager.get(pk=private_school.pk)
with self.assertRaises(Director.DoesNotExist):
private_school.director
finally:
- Director.objects.use_for_related_fields = False
+ Director._default_manager.use_for_related_fields = False
def test_hasattr_related_object(self):
# The exception raised on attribute access when a related object