From ed0ff913c648b16c4471fc9a9441d1ee48cb5420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bistuer?= Date: Sun, 17 Apr 2016 18:55:55 +0700 Subject: [PATCH] Fixed #10506, #13793, #14891, #25201 -- Introduced new APIs to specify models' default and base managers. This deprecates use_for_related_fields. Old API: class CustomManager(models.Model): use_for_related_fields = True class Model(models.Model): custom_manager = CustomManager() New API: class Model(models.Model): custom_manager = CustomManager() class Meta: base_manager_name = 'custom_manager' Refs #20932, #25897. Thanks Carl Meyer for the guidance throughout this work. Thanks Tim Graham for writing the docs. --- django/contrib/gis/db/models/manager.py | 4 + django/db/migrations/state.py | 58 +-- django/db/models/base.py | 98 ++++- .../db/models/fields/related_descriptors.py | 47 ++- django/db/models/manager.py | 39 +- django/db/models/options.py | 90 +++- docs/internals/deprecation.txt | 6 + docs/ref/models/options.txt | 20 + docs/releases/1.10.txt | 23 ++ docs/topics/db/managers.txt | 172 +++----- tests/basic/tests.py | 9 +- tests/custom_managers/models.py | 12 + tests/custom_managers/tests.py | 33 +- tests/managers_regress/models.py | 3 + tests/managers_regress/tests.py | 386 ++++++++++++++++++ tests/many_to_one/tests.py | 13 +- tests/one_to_one/tests.py | 24 +- tests/proxy_models/models.py | 4 + 18 files changed, 815 insertions(+), 226 deletions(-) diff --git a/django/contrib/gis/db/models/manager.py b/django/contrib/gis/db/models/manager.py index 00ce7a3c8f..44f0140090 100644 --- a/django/contrib/gis/db/models/manager.py +++ b/django/contrib/gis/db/models/manager.py @@ -13,6 +13,10 @@ class GeoManager(Manager.from_queryset(GeoQuerySet)): # properly. use_for_related_fields = True + # No need to bother users with the use_for_related_fields + # deprecation for this manager which is itself deprecated. + silence_use_for_related_fields_deprecation = True + def __init__(self, *args, **kwargs): warnings.warn( "The GeoManager class is deprecated. Simply use a normal manager " diff --git a/django/db/migrations/state.py b/django/db/migrations/state.py index ca44a39ca3..d85a511ce4 100644 --- a/django/db/migrations/state.py +++ b/django/db/migrations/state.py @@ -476,47 +476,29 @@ class ModelState(object): if not any((isinstance(base, six.string_types) or issubclass(base, models.Model)) for base in bases): bases = (models.Model,) - # Constructs all managers on the model - managers_mapping = {} + managers = [] - def reconstruct_manager(mgr): - as_manager, manager_path, qs_path, args, kwargs = mgr.deconstruct() - if as_manager: - qs_class = import_string(qs_path) - instance = qs_class.as_manager() - else: - manager_class = import_string(manager_path) - instance = manager_class(*args, **kwargs) - # We rely on the ordering of the creation_counter of the original - # instance - name = force_text(mgr.name) - managers_mapping[name] = (mgr.creation_counter, instance) - - if hasattr(model, "_default_manager"): - default_manager_name = force_text(model._default_manager.name) - # Make sure the default manager is always the first + # Make sure the default manager is always first since ordering chooses + # the default manager. + if not model._default_manager.auto_created: if model._default_manager.use_in_migrations: - reconstruct_manager(model._default_manager) + default_manager = copy.copy(model._default_manager) + default_manager._set_creation_counter() + + # If the default manager doesn't have `use_in_migrations = True`, + # shim a default manager so another manager isn't promoted in its + # place. else: - # Force this manager to be the first and thus default - managers_mapping[default_manager_name] = (0, models.Manager()) - for manager in model._meta.managers: - if manager.name == "_base_manager" or not manager.use_in_migrations: - continue - reconstruct_manager(manager) - # Sort all managers by their creation counter but take only name and - # instance for further processing - managers = [ - (name, instance) for name, (cc, instance) in - sorted(managers_mapping.items(), key=lambda v: v[1]) - ] - # If the only manager on the model is the default manager defined - # by Django (`objects = models.Manager()`), this manager will not - # be added to the model state. - if managers == [('objects', models.Manager())]: - managers = [] - else: - managers = [] + default_manager = models.Manager() + default_manager.model = model + default_manager.name = model._default_manager.name + managers.append((force_text(default_manager.name), default_manager)) + + for manager in model._meta.managers: + if manager.use_in_migrations and manager is not model._default_manager: + manager = copy.copy(manager) + manager._set_creation_counter() + managers.append((force_text(manager.name), manager)) # Construct the new ModelState return cls( diff --git a/django/db/models/base.py b/django/db/models/base.py index f7735985dd..7b5e42eb42 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -24,11 +24,12 @@ from django.db.models.fields.related import ( ForeignObjectRel, ManyToOneRel, OneToOneField, lazy_related_operation, resolve_relation, ) -from django.db.models.manager import ensure_default_manager +from django.db.models.manager import Manager from django.db.models.options import Options from django.db.models.query import Q from django.db.models.utils import make_model_tuple from django.utils import six +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import ( force_str, force_text, python_2_unicode_compatible, ) @@ -345,10 +346,99 @@ class ModelBase(type): if get_absolute_url_override: setattr(cls, 'get_absolute_url', get_absolute_url_override) - ensure_default_manager(cls) + if not opts.managers or cls._requires_legacy_default_manager(): + if any(f.name == 'objects' for f in opts.fields): + raise ValueError( + "Model %s must specify a custom Manager, because it has a " + "field named 'objects'." % cls.__name__ + ) + manager = Manager() + manager.auto_created = True + cls.add_to_class('objects', manager) signals.class_prepared.send(sender=cls) + def _requires_legacy_default_manager(cls): # RemovedInDjango20Warning + opts = cls._meta + + if opts.manager_inheritance_from_future: + return False + + future_default_manager = opts.default_manager + + # Step 1: Locate a manager that would have been promoted + # to default manager with the legacy system. + for manager in opts.managers: + originating_model = manager._originating_model + if (cls is originating_model or cls._meta.proxy or + originating_model._meta.abstract): + + if manager is not cls._default_manager and not opts.default_manager_name: + warnings.warn( + "Managers from concrete parents will soon qualify as default " + "managers if they appear before any other managers in the " + "MRO. As a result, '{legacy_default_manager}' declared on " + "'{legacy_default_manager_model}' will no longer be the " + "default manager for '{model}' in favor of " + "'{future_default_manager}' declared on " + "'{future_default_manager_model}'. " + "You can redeclare '{legacy_default_manager}' on '{cls}' " + "to keep things the way they are or you can switch to the new " + "behavior right away by setting " + "`Meta.manager_inheritance_from_future` to `True`.".format( + cls=cls.__name__, + model=opts.label, + legacy_default_manager=manager.name, + legacy_default_manager_model=manager._originating_model._meta.label, + future_default_manager=future_default_manager.name, + future_default_manager_model=future_default_manager._originating_model._meta.label, + ), + RemovedInDjango20Warning, 2 + ) + + opts.default_manager_name = manager.name + opts._expire_cache() + + break + + # Step 2: Since there are managers but none of them qualified as + # default managers under the legacy system (meaning that there are + # managers from concrete parents that would be promoted under the + # new system), we need to create a new Manager instance for the + # 'objects' attribute as a deprecation shim. + else: + # If the "future" default manager was auto created there is no + # point warning the user since it's basically the same manager. + if not future_default_manager.auto_created: + warnings.warn( + "Managers from concrete parents will soon qualify as " + "default managers. As a result, the 'objects' manager " + "won't be created (or recreated) automatically " + "anymore on '{model}' and '{future_default_manager}' " + "declared on '{future_default_manager_model}' will be " + "promoted to default manager. You can declare " + "explicitly `objects = models.Manager()` on '{cls}' " + "to keep things the way they are or you can switch " + "to the new behavior right away by setting " + "`Meta.manager_inheritance_from_future` to `True`.".format( + cls=cls.__name__, + model=opts.label, + future_default_manager=future_default_manager.name, + future_default_manager_model=future_default_manager._originating_model._meta.label, + ), + RemovedInDjango20Warning, 2 + ) + + return True + + @property + def _base_manager(cls): + return cls._meta.base_manager + + @property + def _default_manager(cls): + return cls._meta.default_manager + class ModelState(object): """ @@ -896,8 +986,8 @@ class Model(six.with_metaclass(ModelBase)): order = '_order' if is_next else '-_order' order_field = self._meta.order_with_respect_to filter_args = order_field.get_filter_kwargs_for_object(self) - obj = self._default_manager.filter(**filter_args).filter(**{ - '_order__%s' % op: self._default_manager.values('_order').filter(**{ + obj = self.__class__._default_manager.filter(**filter_args).filter(**{ + '_order__%s' % op: self.__class__._default_manager.values('_order').filter(**{ self._meta.pk.name: self.pk }) }).order_by(order)[:1].get() diff --git a/django/db/models/fields/related_descriptors.py b/django/db/models/fields/related_descriptors.py index 3cc7e828b1..2bac02c003 100644 --- a/django/db/models/fields/related_descriptors.py +++ b/django/db/models/fields/related_descriptors.py @@ -104,11 +104,20 @@ class ForwardManyToOneDescriptor(object): return hasattr(instance, self.cache_name) def get_queryset(self, **hints): - manager = self.field.remote_field.model._default_manager - # If the related manager indicates that it should be used for - # related fields, respect that. - if not getattr(manager, 'use_for_related_fields', False): - manager = self.field.remote_field.model._base_manager + related_model = self.field.remote_field.model + + if (not related_model._meta.base_manager_name and + getattr(related_model._default_manager, 'use_for_related_fields', False)): + if not getattr(related_model._default_manager, 'silence_use_for_related_fields_deprecation', False): + warnings.warn( + "use_for_related_fields is deprecated, instead " + "set Meta.base_manager_name on '{}'.".format(related_model._meta.label), + RemovedInDjango20Warning, 2 + ) + manager = related_model._default_manager + else: + manager = related_model._base_manager + return manager.db_manager(hints=hints).all() def get_prefetch_queryset(self, instances, queryset=None): @@ -281,11 +290,20 @@ class ReverseOneToOneDescriptor(object): return hasattr(instance, self.cache_name) def get_queryset(self, **hints): - manager = self.related.related_model._default_manager - # If the related manager indicates that it should be used for - # related fields, respect that. - if not getattr(manager, 'use_for_related_fields', False): - manager = self.related.related_model._base_manager + related_model = self.related.related_model + + if (not related_model._meta.base_manager_name and + getattr(related_model._default_manager, 'use_for_related_fields', False)): + if not getattr(related_model._default_manager, 'silence_use_for_related_fields_deprecation', False): + warnings.warn( + "use_for_related_fields is deprecated, instead " + "set Meta.base_manager_name on '{}'.".format(related_model._meta.label), + RemovedInDjango20Warning, 2 + ) + manager = related_model._default_manager + else: + manager = related_model._base_manager + return manager.db_manager(hints=hints).all() def get_prefetch_queryset(self, instances, queryset=None): @@ -437,8 +455,10 @@ class ReverseManyToOneDescriptor(object): @cached_property def related_manager_cls(self): + related_model = self.rel.related_model + return create_reverse_many_to_one_manager( - self.rel.related_model._default_manager.__class__, + related_model._default_manager.__class__, self.rel, ) @@ -697,9 +717,10 @@ class ManyToManyDescriptor(ReverseManyToOneDescriptor): @cached_property def related_manager_cls(self): - model = self.rel.related_model if self.reverse else self.rel.model + related_model = self.rel.related_model if self.reverse else self.rel.model + return create_forward_many_to_many_manager( - model._default_manager.__class__, + related_model._default_manager.__class__, self.rel, reverse=self.reverse, ) diff --git a/django/db/models/manager.py b/django/db/models/manager.py index b8aa295ba3..5a8ad632d5 100644 --- a/django/db/models/manager.py +++ b/django/db/models/manager.py @@ -8,47 +8,14 @@ from django.utils import six from django.utils.encoding import python_2_unicode_compatible -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 and _base_manager attributes on the class. - """ - - 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'" % model.__name__ - ) - 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: - raise ValueError("Could not find a suitable base manager.") - - @python_2_unicode_compatible class BaseManager(object): # Tracks each time a Manager instance is created. Used to retain order. creation_counter = 0 + # Set to True for the 'objects' managers that are automatically created. + auto_created = False + #: If set to True the manager will be serialized into migrations and will #: thus be available in e.g. RunPython operations use_in_migrations = False diff --git a/django/db/models/options.py b/django/db/models/options.py index 389ba7c93e..ef13c496f8 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -10,6 +10,7 @@ from django.apps import apps from django.conf import settings from django.core.exceptions import FieldDoesNotExist from django.db import connections +from django.db.models import Manager from django.db.models.fields import AutoField from django.db.models.fields.proxy import OrderWrt from django.utils import six @@ -41,7 +42,8 @@ DEFAULT_NAMES = ( 'app_label', 'db_tablespace', 'abstract', 'managed', 'proxy', 'swappable', 'auto_created', 'index_together', 'apps', 'default_permissions', 'select_on_save', 'default_related_name', 'required_db_features', - 'required_db_vendor', + 'required_db_vendor', 'base_manager_name', 'default_manager_name', + 'manager_inheritance_from_future', ) @@ -73,9 +75,11 @@ 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', - 'managers', 'managers_map'} + FORWARD_PROPERTIES = { + 'fields', 'many_to_many', 'concrete_fields', 'local_concrete_fields', + '_forward_fields_map', 'managers', 'managers_map', 'base_manager', + 'default_manager', + } REVERSE_PROPERTIES = {'related_objects', 'fields_map', '_relation_tree'} default_apps = apps @@ -85,7 +89,10 @@ class Options(object): self.local_fields = [] self.local_many_to_many = [] self.private_fields = [] + self.manager_inheritance_from_future = False self.local_managers = [] + self.base_manager_name = None + self.default_manager_name = None self.model_name = None self.verbose_name = None self.verbose_name_plural = None @@ -368,6 +375,10 @@ class Options(object): manager.model = self.model managers.append((depth, manager.creation_counter, manager)) + # Used for deprecation of legacy manager inheritance, + # remove afterwards. (RemovedInDjango20Warning) + manager._originating_model = base + return make_immutable_fields_list( "managers", (m[2] for m in sorted(managers)), @@ -377,6 +388,77 @@ class Options(object): def managers_map(self): return {manager.name: manager for manager in reversed(self.managers)} + @cached_property + def base_manager(self): + base_manager_name = self.base_manager_name + if not base_manager_name: + # Get the first parent's base_manager_name if there's one. + for parent in self.model.mro()[1:]: + if hasattr(parent, '_meta'): + if parent._base_manager.name != '_base_manager': + base_manager_name = parent._base_manager.name + break + + if base_manager_name: + try: + return self.managers_map[base_manager_name] + except KeyError: + raise ValueError( + "%s has no manager named %r" % ( + self.object_name, + base_manager_name, + ) + ) + + # Deprecation shim for `use_for_related_fields`. + for i, base_manager_class in enumerate(self.default_manager.__class__.mro()): + if getattr(base_manager_class, 'use_for_related_fields', False): + if not getattr(base_manager_class, 'silence_use_for_related_fields_deprecation', False): + warnings.warn( + "use_for_related_fields is deprecated, instead " + "set Meta.base_manager_name on '{}'.".format(self.model._meta.label), + RemovedInDjango20Warning, 2 + ) + + if i == 0: + manager = self.default_manager + else: + manager = base_manager_class() + manager.name = '_base_manager' + manager.model = self.model + + return manager + + manager = Manager() + manager.name = '_base_manager' + manager.model = self.model + manager.auto_created = True + return manager + + @cached_property + def default_manager(self): + default_manager_name = self.default_manager_name + if not default_manager_name and not self.local_managers: + # Get the first parent's default_manager_name if there's one. + for parent in self.model.mro()[1:]: + if hasattr(parent, '_meta'): + default_manager_name = parent._meta.default_manager_name + break + + if default_manager_name: + try: + return self.managers_map[default_manager_name] + except KeyError: + raise ValueError( + "%s has no manager named %r" % ( + self.object_name, + default_manager_name, + ) + ) + + if self.managers: + return self.managers[0] + @cached_property def fields(self): """ diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 218e79d66f..d4bde35a46 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -175,6 +175,12 @@ details on these changes. * The ``escape`` filter will change to use ``django.utils.html.conditional_escape()``. +* ``Manager.use_for_related_fields`` will be removed. + +* Model ``Manager`` inheritance will follow MRO inheritance rules and the + ``Meta.manager_inheritance_from_future`` to opt-in to this behavior will be + removed. + .. _deprecation-removed-in-1.10: 1.10 diff --git a/docs/ref/models/options.txt b/docs/ref/models/options.txt index 677916bf5c..9c77899d09 100644 --- a/docs/ref/models/options.txt +++ b/docs/ref/models/options.txt @@ -35,6 +35,16 @@ Available ``Meta`` options or ``app_label.model_name`` you can use ``model._meta.label`` or ``model._meta.label_lower`` respectively. +``base_manager_name`` +--------------------- + +.. attribute:: Options.base_manager_name + +.. versionadded:: 1.10 + +The name of the manager to use for the model's +:attr:`~django.db.models.Model._base_manager`. + ``db_table`` ------------ @@ -95,6 +105,16 @@ Django quotes column and table names behind the scenes. setting, if set. If the backend doesn't support tablespaces, this option is ignored. +``default_manager_name`` +------------------------ + +.. attribute:: Options.default_manager_name + +.. versionadded:: 1.10 + +The name of the manager to use for the model's +:attr:`~django.db.models.Model._default_manager`. + ``default_related_name`` ------------------------ diff --git a/docs/releases/1.10.txt b/docs/releases/1.10.txt index fa5d3ff108..3fd6431462 100644 --- a/docs/releases/1.10.txt +++ b/docs/releases/1.10.txt @@ -432,6 +432,13 @@ Models * ``Model.__init__()`` now sets values of virtual fields from its keyword arguments. +* The new :attr:`Meta.base_manager_name + ` and + :attr:`Meta.default_manager_name + ` options allow controlling + the :attr:`~django.db.models.Model._base_manager` and + :attr:`~django.db.models.Model._default_manager`, respectively. + Requests and Responses ~~~~~~~~~~~~~~~~~~~~~~ @@ -1063,6 +1070,22 @@ always be applied as the last filter no matter where in the filter chain it appeared) is deprecated. The filter will change to immediately apply :func:`~django.utils.html.conditional_escape` in Django 2.0. +``Manager.use_for_related_fields`` and inheritance changes +---------------------------------------------------------- + +``Manager.use_for_related_fields`` is deprecated in favor of setting +:attr:`Meta.base_manager_name ` on +the model. + +Model ``Manager`` inheritance will follow MRO inheritance rules in Django 2.0, +changing the current behavior where managers defined on non-abstract base +classes aren't inherited by child classes. A deprecating warning with +instructions on how to adapt your code is raised if you have any affected +managers. You'll either redeclare a manager from an abstract model on the child +class to override the manager from the concrete model, or you'll set the +model's ``Meta.manager_inheritance_from_future=True`` option to opt-in to the +new inheritance behavior. + Miscellaneous ------------- diff --git a/docs/topics/db/managers.txt b/docs/topics/db/managers.txt index 80598f0c98..74c93a0edc 100644 --- a/docs/topics/db/managers.txt +++ b/docs/topics/db/managers.txt @@ -172,35 +172,59 @@ and ``Person.people.all()``, yielding predictable results. .. _default-managers: Default managers -~~~~~~~~~~~~~~~~ +---------------- + +.. attribute:: Model._default_manager If you use custom ``Manager`` objects, take note that the first ``Manager`` Django encounters (in the order in which they're defined in the model) has a special status. Django interprets the first ``Manager`` defined in a class as -the "default" ``Manager``, and several parts of Django -(including :djadmin:`dumpdata`) will use that ``Manager`` -exclusively for that model. As a result, it's a good idea to be careful in -your choice of default manager in order to avoid a situation where overriding -``get_queryset()`` results in an inability to retrieve objects you'd like to -work with. +the "default" ``Manager``, and several parts of Django (including +:djadmin:`dumpdata`) will use that ``Manager`` exclusively for that model. As a +result, it's a good idea to be careful in your choice of default manager in +order to avoid a situation where overriding ``get_queryset()`` results in an +inability to retrieve objects you'd like to work with. + +You can specify a custom default manager using :attr:`Meta.base_manager_name +`. + +If you're writing some code that must handle an unknown model, for example, in +a third-party app that implements a generic view, use this manager (or +:attr:`~Model._base_manager`) rather than assuming the model has an ``objects`` +manager. + +Base managers +------------- + +.. attribute:: Model._base_manager .. _managers-for-related-objects: Using managers for related object access ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -By default, Django uses an instance of a "plain" manager class when accessing -related objects (i.e. ``choice.poll``), not the default manager on the related -object. This is because Django needs to be able to retrieve the related -object, even if it would otherwise be filtered out (and hence be inaccessible) -by the default manager. +By default, Django uses an instance of the ``Model._base_manager`` manager +class when accessing related objects (i.e. ``choice.poll``), not the +``_default_manager`` on the related object. This is because Django needs to be +able to retrieve the related object, even if it would otherwise be filtered out +(and hence be inaccessible) by the default manager. -If the normal plain manager class (:class:`django.db.models.Manager`) is not -appropriate for your circumstances, you can force Django to use the same class -as the default manager for your model by setting the ``use_for_related_fields`` -attribute on the manager class. This is documented fully below_. +If the normal base manager class (:class:`django.db.models.Manager`) isn't +appropriate for your circumstances, you can tell Django which class to use by +setting :attr:`Meta.base_manager_name +`. -.. _below: manager-types_ +Don't filter away any results in this type of manager subclass +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This manager is used to access objects that are related to from some other +model. In those situations, Django has to be able to see all the objects for +the model it is fetching, so that *anything* which is referred to can be +retrieved. + +If you override the ``get_queryset()`` method and filter out any rows, Django +will return incorrect results. Don't do that. A manager that filters results +in ``get_queryset()`` is not appropriate for use as a default manager. .. _calling-custom-queryset-methods-from-manager: @@ -321,19 +345,21 @@ You may also store the generated class into a variable:: Custom managers and model inheritance ------------------------------------- -Here's how Django handles custom managers and -:ref:`model inheritance `: +Here's how Django handles custom managers and :ref:`model inheritance +`: -1. Managers from base classes are always inherited by the child class, +#. 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). -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. +#. If no managers are declared on a model and/or its parents, Django + automatically creates the ``objects`` manager. + +#. The default manager on a class is either the one chosen with + :attr:`Meta.default_manager_name + `, or the first manager + declared on the model, or the default manager of the first parent model. .. versionchanged:: 1.10 @@ -428,99 +454,3 @@ However, if you're overriding ``__getattr__`` or some other private method of your ``Manager`` object that controls object state, you should ensure that you don't affect the ability of your ``Manager`` to be copied. - -.. _manager-types: - -Controlling automatic manager types -=================================== - -This document has already mentioned a couple of places where Django creates a -manager class for you: `default managers`_ and the "plain" manager used to -`access related objects`_. There are other places in the implementation of -Django where temporary plain managers are needed. Those automatically created -managers will normally be instances of the :class:`django.db.models.Manager` -class. - -.. _default managers: manager-names_ -.. _access related objects: managers-for-related-objects_ - -Throughout this section, we will use the term "automatic manager" to mean a -manager that Django creates for you -- either as a default manager on a model -with no managers, or to use temporarily when accessing related objects. - -Sometimes this default class won't be the right choice. The default manager -may not have all the methods you need to work with your data. A custom manager -class of your own will allow you to create custom ``QuerySet`` objects to give -you the information you need. - -Django provides a way for custom manager developers to say that their manager -class should be used for automatic managers whenever it is the default manager -on a model. This is done by setting the ``use_for_related_fields`` attribute on -the manager class:: - - class MyManager(models.Manager): - use_for_related_fields = True - # ... - -If this attribute is set on the *default* manager for a model (only the -default manager is considered in these situations), Django will use that class -whenever it needs to automatically create a manager for the class. Otherwise, -it will use :class:`django.db.models.Manager`. - -.. admonition:: Historical Note - - Given the purpose for which it's used, the name of this attribute - (``use_for_related_fields``) might seem a little odd. Originally, the - attribute only controlled the type of manager used for related field - access, which is where the name came from. As it became clear the concept - was more broadly useful, the name hasn't been changed. This is primarily - so that existing code will :doc:`continue to work ` in - future Django versions. - -Writing correct managers for use in automatic manager instances ---------------------------------------------------------------- - -The ``use_for_related_fields`` feature is primarily for managers that need to -return a custom ``QuerySet`` subclass. In providing this functionality in your -manager, there are a couple of things to remember. - -Do not filter away any results in this type of manager subclass -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -One reason an automatic manager is used is to access objects that are related -to from some other model. In those situations, Django has to be able to see -all the objects for the model it is fetching, so that *anything* which is -referred to can be retrieved. - -If you override the ``get_queryset()`` method and filter out any rows, Django -will return incorrect results. Don't do that. A manager that filters results -in ``get_queryset()`` is not appropriate for use as an automatic manager. - -Set ``use_for_related_fields`` when you define the class -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -The ``use_for_related_fields`` attribute must be set on the manager *class*, not -on an *instance* of the class. The earlier example shows the correct way to set -it, whereas the following will not work:: - - # BAD: Incorrect code - class MyManager(models.Manager): - # ... - pass - - # Sets the attribute on an instance of MyManager. Django will - # ignore this setting. - mgr = MyManager() - mgr.use_for_related_fields = True - - class MyModel(models.Model): - # ... - objects = mgr - - # End of incorrect code. - -You also shouldn't change the attribute on the class object after it has been -used in a model, since the attribute's value is processed when the model class -is created and not subsequently reread. Set the attribute on the manager class -when it is first defined, as in the initial example of this section and -everything will work smoothly. diff --git a/tests/basic/tests.py b/tests/basic/tests.py index 6ff037a31a..a378961444 100644 --- a/tests/basic/tests.py +++ b/tests/basic/tests.py @@ -637,7 +637,7 @@ class SelectOnSaveTests(TestCase): # test this properly otherwise. Article's manager, because # proxy models use their parent model's _base_manager. - orig_class = Article._base_manager.__class__ + orig_class = Article._base_manager._queryset_class class FakeQuerySet(QuerySet): # Make sure the _update method below is in fact called. @@ -648,11 +648,8 @@ class SelectOnSaveTests(TestCase): super(FakeQuerySet, self)._update(*args, **kwargs) return 0 - class FakeManager(orig_class): - def get_queryset(self): - return FakeQuerySet(self.model) try: - Article._base_manager.__class__ = FakeManager + Article._base_manager._queryset_class = FakeQuerySet asos = ArticleSelectOnSave.objects.create(pub_date=datetime.now()) with self.assertNumQueries(3): asos.save() @@ -665,7 +662,7 @@ class SelectOnSaveTests(TestCase): with self.assertRaises(DatabaseError): asos.save(update_fields=['pub_date']) finally: - Article._base_manager.__class__ = orig_class + Article._base_manager._queryset_class = orig_class class ModelRefreshTests(TestCase): diff --git a/tests/custom_managers/models.py b/tests/custom_managers/models.py index 22ba66cb8f..51773b18b8 100644 --- a/tests/custom_managers/models.py +++ b/tests/custom_managers/models.py @@ -172,6 +172,18 @@ class Car(models.Model): return self.name +class FastCarAsBase(Car): + class Meta: + proxy = True + base_manager_name = 'fast_cars' + + +class FastCarAsDefault(Car): + class Meta: + proxy = True + default_manager_name = 'fast_cars' + + class RestrictedManager(models.Manager): def get_queryset(self): return super(RestrictedManager, self).get_queryset().filter(is_public=True) diff --git a/tests/custom_managers/tests.py b/tests/custom_managers/tests.py index ee1cc141a5..b7a433ef3b 100644 --- a/tests/custom_managers/tests.py +++ b/tests/custom_managers/tests.py @@ -6,8 +6,9 @@ from django.utils import six from .models import ( Book, Car, CustomManager, CustomQuerySet, DeconstructibleCustomManager, - FunPerson, OneToOneRestrictedModel, Person, PersonFromAbstract, - PersonManager, PublishedBookManager, RelatedModel, RestrictedModel, + FastCarAsBase, FastCarAsDefault, FunPerson, OneToOneRestrictedModel, + Person, PersonFromAbstract, PersonManager, PublishedBookManager, + RelatedModel, RestrictedModel, ) @@ -558,6 +559,34 @@ class TestCars(TestCase): ], lambda c: c.name ) + # explicit default manager + self.assertQuerysetEqual( + FastCarAsDefault.cars.order_by("name"), [ + "Corvette", + "Neon", + ], + lambda c: c.name + ) + self.assertQuerysetEqual( + FastCarAsDefault._default_manager.all(), [ + "Corvette", + ], + lambda c: c.name + ) + # explicit base manager + self.assertQuerysetEqual( + FastCarAsBase.cars.order_by("name"), [ + "Corvette", + "Neon", + ], + lambda c: c.name + ) + self.assertQuerysetEqual( + FastCarAsBase._base_manager.all(), [ + "Corvette", + ], + lambda c: c.name + ) class CustomManagersRegressTestCase(TestCase): diff --git a/tests/managers_regress/models.py b/tests/managers_regress/models.py index b39e69b9ad..37c3a37dcf 100644 --- a/tests/managers_regress/models.py +++ b/tests/managers_regress/models.py @@ -118,6 +118,9 @@ class Child5(AbstractBase3): class Child6(Child4): value = models.IntegerField() + class Meta: + manager_inheritance_from_future = True + class Child7(Parent): objects = models.Manager() diff --git a/tests/managers_regress/tests.py b/tests/managers_regress/tests.py index 3a51ff7542..7c1223a5f1 100644 --- a/tests/managers_regress/tests.py +++ b/tests/managers_regress/tests.py @@ -1,9 +1,13 @@ from __future__ import unicode_literals +import warnings + from django.db import models +from django.db.utils import DatabaseError from django.template import Context, Template from django.test import TestCase, override_settings from django.test.utils import isolate_apps +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_text from .models import ( @@ -160,3 +164,385 @@ class ManagersRegressionTests(TestCase): related = RelatedModel.objects.create(exact=False) relation = related.test_fk.create() self.assertEqual(related.test_fk.get(), relation) + + +@isolate_apps('managers_regress') +class TestManagerInheritance(TestCase): + def test_implicit_inheritance(self): + class CustomManager(models.Manager): + pass + + class AbstractModel(models.Model): + custom_manager = CustomManager() + + class Meta: + abstract = True + + class PlainModel(models.Model): + custom_manager = CustomManager() + + self.assertIsInstance(PlainModel._base_manager, models.Manager) + self.assertIsInstance(PlainModel._default_manager, CustomManager) + + class ModelWithAbstractParent(AbstractModel): + class Meta: + manager_inheritance_from_future = True + + self.assertIsInstance(ModelWithAbstractParent._base_manager, models.Manager) + self.assertIsInstance(ModelWithAbstractParent._default_manager, CustomManager) + + class ProxyModel(PlainModel): + class Meta: + manager_inheritance_from_future = True + proxy = True + + self.assertIsInstance(ProxyModel._base_manager, models.Manager) + self.assertIsInstance(ProxyModel._default_manager, CustomManager) + + class MTIModel(PlainModel): + class Meta: + manager_inheritance_from_future = True + + self.assertIsInstance(MTIModel._base_manager, models.Manager) + self.assertIsInstance(MTIModel._default_manager, CustomManager) + + def test_default_manager_inheritance(self): + class CustomManager(models.Manager): + pass + + class AbstractModel(models.Model): + another_manager = models.Manager() + custom_manager = CustomManager() + + class Meta: + default_manager_name = 'custom_manager' + abstract = True + + class PlainModel(models.Model): + another_manager = models.Manager() + custom_manager = CustomManager() + + class Meta: + default_manager_name = 'custom_manager' + + self.assertIsInstance(PlainModel._default_manager, CustomManager) + + class ModelWithAbstractParent(AbstractModel): + class Meta: + manager_inheritance_from_future = True + + self.assertIsInstance(ModelWithAbstractParent._default_manager, CustomManager) + + class ProxyModel(PlainModel): + class Meta: + manager_inheritance_from_future = True + proxy = True + + self.assertIsInstance(ProxyModel._default_manager, CustomManager) + + class MTIModel(PlainModel): + class Meta: + manager_inheritance_from_future = True + + self.assertIsInstance(MTIModel._default_manager, CustomManager) + + def test_base_manager_inheritance(self): + class CustomManager(models.Manager): + pass + + class AbstractModel(models.Model): + another_manager = models.Manager() + custom_manager = CustomManager() + + class Meta: + base_manager_name = 'custom_manager' + abstract = True + + class PlainModel(models.Model): + another_manager = models.Manager() + custom_manager = CustomManager() + + class Meta: + base_manager_name = 'custom_manager' + + self.assertIsInstance(PlainModel._base_manager, CustomManager) + + class ModelWithAbstractParent(AbstractModel): + class Meta: + manager_inheritance_from_future = True + + self.assertIsInstance(ModelWithAbstractParent._base_manager, CustomManager) + + class ProxyModel(PlainModel): + class Meta: + manager_inheritance_from_future = True + proxy = True + + self.assertIsInstance(ProxyModel._base_manager, CustomManager) + + class MTIModel(PlainModel): + class Meta: + manager_inheritance_from_future = True + + self.assertIsInstance(MTIModel._base_manager, CustomManager) + + +@isolate_apps('managers_regress') +class TestManagerDeprecations(TestCase): + def test_use_for_related_fields_on_geomanager(self): + from django.contrib.gis.db.models import GeoManager + + class MyModel(models.Model): + objects = GeoManager() + + # Shouldn't issue any warnings, since GeoManager itself will be + # deprecated at the same time as use_for_related_fields, there + # is no point annoying users with this deprecation. + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + MyModel._base_manager + self.assertEqual(len(warns), 0) + + def test_use_for_related_fields_for_base_manager(self): + class MyManager(models.Manager): + use_for_related_fields = True + + class MyModel(models.Model): + objects = MyManager() + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + MyModel._base_manager + self.assertEqual(len(warns), 1) + self.assertEqual( + str(warns[0].message), + "use_for_related_fields is deprecated, " + "instead set Meta.base_manager_name on " + "'managers_regress.MyModel'.", + ) + + # With the new base_manager_name API there shouldn't be any warnings. + class MyModel2(models.Model): + objects = MyManager() + + class Meta: + base_manager_name = 'objects' + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + MyModel2._base_manager + self.assertEqual(len(warns), 0) + + def test_use_for_related_fields_for_many_to_one(self): + class MyManager(models.Manager): + use_for_related_fields = True + + class MyRelModel(models.Model): + objects = MyManager() + + class MyModel(models.Model): + fk = models.ForeignKey(MyRelModel, on_delete=models.DO_NOTHING) + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + try: + MyModel(fk_id=42).fk + except DatabaseError: + pass + self.assertEqual(len(warns), 1) + self.assertEqual( + str(warns[0].message), + "use_for_related_fields is deprecated, " + "instead set Meta.base_manager_name on " + "'managers_regress.MyRelModel'.", + ) + + # With the new base_manager_name API there shouldn't be any warnings. + class MyRelModel2(models.Model): + objects = MyManager() + + class Meta: + base_manager_name = 'objects' + + class MyModel2(models.Model): + fk = models.ForeignKey(MyRelModel2, on_delete=models.DO_NOTHING) + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + try: + MyModel2(fk_id=42).fk + except DatabaseError: + pass + self.assertEqual(len(warns), 0) + + def test_use_for_related_fields_for_one_to_one(self): + class MyManager(models.Manager): + use_for_related_fields = True + + class MyRelModel(models.Model): + objects = MyManager() + + class MyModel(models.Model): + o2o = models.OneToOneField(MyRelModel, on_delete=models.DO_NOTHING) + objects = MyManager() + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + try: + MyModel(o2o_id=42).o2o + except DatabaseError: + pass + self.assertEqual(len(warns), 1) + self.assertEqual( + str(warns[0].message), + "use_for_related_fields is deprecated, " + "instead set Meta.base_manager_name on " + "'managers_regress.MyRelModel'.", + ) + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + try: + MyRelModel(pk=42).mymodel + except DatabaseError: + pass + self.assertEqual(len(warns), 1) + self.assertEqual( + str(warns[0].message), + "use_for_related_fields is deprecated, " + "instead set Meta.base_manager_name on " + "'managers_regress.MyModel'.", + ) + + # With the new base_manager_name API there shouldn't be any warnings. + class MyRelModel2(models.Model): + objects = MyManager() + + class Meta: + base_manager_name = 'objects' + + class MyModel2(models.Model): + o2o = models.OneToOneField(MyRelModel2, on_delete=models.DO_NOTHING) + objects = MyManager() + + class Meta: + base_manager_name = 'objects' + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + try: + MyModel2(o2o_id=42).o2o + except DatabaseError: + pass + try: + MyRelModel2(pk=42).mymodel2 + except DatabaseError: + pass + self.assertEqual(len(warns), 0) + + def test_legacy_objects_is_created(self): + class ConcreteParentWithoutManager(models.Model): + pass + + class ConcreteParentWithManager(models.Model): + default = models.Manager() + + class AbstractParent(models.Model): + default = models.Manager() + + class Meta: + abstract = True + + # Shouldn't complain since the inherited manager + # is basically the same that would have been created. + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + + class MyModel(ConcreteParentWithoutManager): + pass + self.assertEqual(len(warns), 0) + + # Should create 'objects' (set as default) and warn that + # it will no longer be the case in the future. + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + + class MyModel2(ConcreteParentWithManager): + pass + self.assertEqual(len(warns), 1) + self.assertEqual( + str(warns[0].message), + "Managers from concrete parents will soon qualify as default " + "managers. As a result, the 'objects' manager won't be created " + "(or recreated) automatically anymore on " + "'managers_regress.MyModel2' and 'default' declared on " + "'managers_regress.ConcreteParentWithManager' will be promoted " + "to default manager. You can declare explicitly " + "`objects = models.Manager()` on 'MyModel2' to keep things the " + "way they are or you can switch to the new behavior right away " + "by setting `Meta.manager_inheritance_from_future` to `True`.", + ) + + self.assertIs(MyModel2.objects, MyModel2._default_manager) + + # When there is a local manager we shouldn't get any warning + # and 'objects' shouldn't be created. + class MyModel3(ConcreteParentWithManager): + default = models.Manager() + self.assertIs(MyModel3.default, MyModel3._default_manager) + self.assertIsNone(getattr(MyModel3, 'objects', None)) + + # When there is an inherited manager we shouldn't get any warning + # and 'objects' shouldn't be created. + class MyModel4(AbstractParent, ConcreteParentWithManager): + pass + self.assertIs(MyModel4.default, MyModel4._default_manager) + self.assertIsNone(getattr(MyModel4, 'objects', None)) + + # With `manager_inheritance_from_future = True` 'objects' + # shouldn't be created. + class MyModel5(ConcreteParentWithManager): + class Meta: + manager_inheritance_from_future = True + self.assertIs(MyModel5.default, MyModel5._default_manager) + self.assertIsNone(getattr(MyModel5, 'objects', None)) + + def test_legacy_default_manager_promotion(self): + class ConcreteParent(models.Model): + concrete = models.Manager() + + class AbstractParent(models.Model): + abstract = models.Manager() + + class Meta: + abstract = True + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always', RemovedInDjango20Warning) + + class MyModel(ConcreteParent, AbstractParent): + pass + self.assertEqual(len(warns), 1) + self.assertEqual( + str(warns[0].message), + "Managers from concrete parents will soon qualify as default " + "managers if they appear before any other managers in the " + "MRO. As a result, 'abstract' declared on " + "'managers_regress.AbstractParent' will no longer be the " + "default manager for 'managers_regress.MyModel' in favor of " + "'concrete' declared on 'managers_regress.ConcreteParent'. " + "You can redeclare 'abstract' on 'MyModel' to keep things the " + "way they are or you can switch to the new behavior right " + "away by setting `Meta.manager_inheritance_from_future` to " + "`True`.", + ) + self.assertIs(MyModel.abstract, MyModel._default_manager) + + class MyModel2(ConcreteParent, AbstractParent): + abstract = models.Manager() + self.assertIs(MyModel2.abstract, MyModel2._default_manager) + + class MyModel3(ConcreteParent, AbstractParent): + class Meta: + manager_inheritance_from_future = True + self.assertIs(MyModel3.concrete, MyModel3._default_manager) diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py index eaa01bbfcb..20f2e0d56d 100644 --- a/tests/many_to_one/tests.py +++ b/tests/many_to_one/tests.py @@ -4,7 +4,7 @@ from copy import deepcopy from django.core.exceptions import FieldError, MultipleObjectsReturned from django.db import models, transaction from django.db.utils import IntegrityError -from django.test import TestCase +from django.test import TestCase, ignore_warnings from django.utils import six from django.utils.deprecation import RemovedInDjango20Warning from django.utils.translation import ugettext_lazy @@ -580,6 +580,7 @@ class ManyToOneTests(TestCase): with self.assertNumQueries(1): self.assertEqual(th.child_set.count(), 0) + @ignore_warnings(category=RemovedInDjango20Warning) # for use_for_related_fields deprecation def test_related_object(self): public_school = School.objects.create(is_public=True) public_student = Student.objects.create(school=public_school) @@ -608,6 +609,16 @@ class ManyToOneTests(TestCase): finally: School._default_manager.use_for_related_fields = False + School._meta.base_manager_name = 'objects' + School._meta._expire_cache() + try: + private_student = Student.objects.get(pk=private_student.pk) + with self.assertRaises(School.DoesNotExist): + private_student.school + finally: + School._meta.base_manager_name = None + School._meta._expire_cache() + def test_hasattr_related_object(self): # The exception raised on attribute access when a related object # doesn't exist should be an instance of a subclass of `AttributeError` diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py index ecb009f961..7e8c9c5dc3 100644 --- a/tests/one_to_one/tests.py +++ b/tests/one_to_one/tests.py @@ -1,7 +1,8 @@ from __future__ import unicode_literals from django.db import IntegrityError, connection, transaction -from django.test import TestCase +from django.test import TestCase, ignore_warnings +from django.utils.deprecation import RemovedInDjango20Warning from .models import ( Bar, Director, Favorites, HiddenPointer, ManualPrimaryKey, MultiModel, @@ -422,6 +423,7 @@ class OneToOneTests(TestCase): hasattr(Target, HiddenPointer._meta.get_field('target').remote_field.get_accessor_name()) ) + @ignore_warnings(category=RemovedInDjango20Warning) # for use_for_related_fields deprecation def test_related_object(self): public_school = School.objects.create(is_public=True) public_director = Director.objects.create(school=public_school, is_temp=False) @@ -473,6 +475,26 @@ class OneToOneTests(TestCase): finally: Director._default_manager.use_for_related_fields = False + School._meta.base_manager_name = 'objects' + School._meta._expire_cache() + try: + private_director = Director._base_manager.get(pk=private_director.pk) + with self.assertRaises(School.DoesNotExist): + private_director.school + finally: + School._meta.base_manager_name = None + School._meta._expire_cache() + + Director._meta.base_manager_name = 'objects' + Director._meta._expire_cache() + try: + private_school = School._base_manager.get(pk=private_school.pk) + with self.assertRaises(Director.DoesNotExist): + private_school.director + finally: + Director._meta.base_manager_name = None + Director._meta._expire_cache() + def test_hasattr_related_object(self): # The exception raised on attribute access when a related object # doesn't exist should be an instance of a subclass of `AttributeError` diff --git a/tests/proxy_models/models.py b/tests/proxy_models/models.py index cb3975d4ac..6960042d78 100644 --- a/tests/proxy_models/models.py +++ b/tests/proxy_models/models.py @@ -85,6 +85,8 @@ class StatusPerson(MyPerson): """ status = models.CharField(max_length=80) + objects = models.Manager() + # We can even have proxies of proxies (and subclass of those). @@ -96,6 +98,8 @@ class MyPersonProxy(MyPerson): class LowerStatusPerson(MyPersonProxy): status = models.CharField(max_length=80) + objects = models.Manager() + @python_2_unicode_compatible class User(models.Model):