From f898f8059467124953a7d6568d1ca4c3e69f5e16 Mon Sep 17 00:00:00 2001 From: Diederik van der Boor Date: Sun, 4 Feb 2018 13:12:40 +0100 Subject: [PATCH] Fixed unwanted manager replacement in Django 1.11 projects. Django 1.11 uses the old manager inheritance system, unless it's overwritten with manager_inheritance_from_future. With a class layout like: PolymorphicModel (abstract) PolymorphicMPTTModel (abstract) GenericCustomer (concrete, has objects = ...) CustomerGroupBase (abstract, has objects = ...) Partner (concrete, no manager) BranchPartner (concrete, no manager) The last level gets a normal Django Manager instead of the polymorphic manager. Because the PolymorphicModel had a base_objects manager, this was typically used as _default_manager. Now that the default manager is no longer affected, it's also easier to detect why the "objects" doesn't get the proper manager type. Using "manager_inheritance_from_future" is recommended instead to have both the right behavior and forward Django 2.x compatibility. --- polymorphic/base.py | 76 +++++++++++++++++++++++++------------ polymorphic/models.py | 1 - polymorphic/query.py | 2 +- polymorphic/tests/models.py | 7 +++- 4 files changed, 57 insertions(+), 29 deletions(-) diff --git a/polymorphic/base.py b/polymorphic/base.py index cd6e225..5ff3e0a 100644 --- a/polymorphic/base.py +++ b/polymorphic/base.py @@ -7,7 +7,10 @@ from __future__ import absolute_import import inspect import os import sys +import warnings +import django +from django.core.exceptions import ImproperlyConfigured from django.db import models from django.db.models.base import ModelBase from django.db.models.manager import ManagerDescriptor @@ -134,33 +137,56 @@ class PolymorphicModelBase(ModelBase): and its querysets from PolymorphicQuerySet - throw AssertionError if not""" if not issubclass(type(manager), PolymorphicManager): - e = 'PolymorphicModel: "' + model_name + '.' + manager_name + '" manager is of type "' + type(manager).__name__ - e += '", but must be a subclass of PolymorphicManager' - raise AssertionError(e) + if django.VERSION < (2, 0): + extra = "\nConsider using Meta.manager_inheritance_from_future = True for Django 1.x projects" + else: + extra = '' + e = ('PolymorphicModel: "{0}.{1}" manager is of type "{2}", but must be a subclass of' + ' PolymorphicManager.{extra}'.format( + model_name, manager_name, type(manager).__name__, extra=extra)) + raise ImproperlyConfigured(e) if not getattr(manager, 'queryset_class', None) or not issubclass(manager.queryset_class, PolymorphicQuerySet): - e = 'PolymorphicModel: "' + model_name + '.' + manager_name + '" (PolymorphicManager) has been instantiated with a queryset class which is' - e += ' not a subclass of PolymorphicQuerySet (which is required)' - raise AssertionError(e) + e = ('PolymorphicModel: "{0}.{1}" (PolymorphicManager) has been instantiated with a queryset class ' + 'which is not a subclass of PolymorphicQuerySet (which is required)'.format(model_name, manager_name)) + raise ImproperlyConfigured(e) return manager - # hack: a small patch to Django would be a better solution. - # Django's management command 'dumpdata' relies on non-polymorphic - # behaviour of the _default_manager. Therefore, we catch any access to _default_manager - # here and return the non-polymorphic default manager instead if we are called from 'dumpdata.py' - # Otherwise, the base objects will be upcasted to polymorphic models, and be outputted as such. - # (non-polymorphic default manager is 'base_objects' for polymorphic models). - # This way we don't need to patch django.core.management.commands.dumpdata - # for all supported Django versions. - if len(sys.argv) > 1 and sys.argv[1] == 'dumpdata': - # manage.py dumpdata is running + @property + def base_objects(self): + warnings.warn( + "Using PolymorphicModel.base_objects is deprecated.\n" + "Use {0}.objects.non_polymorphic() instead.".format(self.__class__.__name__), + DeprecationWarning) - def __getattribute__(self, name): - if name == '_default_manager': - frm = inspect.stack()[1] # frm[1] is caller file name, frm[3] is caller function name - if DUMPDATA_COMMAND in frm[1]: - return self.base_objects - # caller_mod_name = inspect.getmodule(frm[0]).__name__ # does not work with python 2.4 - # if caller_mod_name == 'django.core.management.commands.dumpdata': + # Create a manager so the API works as expected. Just don't register it + # anymore in the Model Meta, so it doesn't substitute our polymorphic + # manager as default manager for the third level of inheritance when + # that third level doesn't define a manager at all. + manager = models.Manager() + manager.name = 'base_objects' + manager.model = self + return manager - return super(PolymorphicModelBase, self).__getattribute__(name) - # TODO: investigate Django how this can be avoided + @property + def _default_manager(self): + if len(sys.argv) > 1 and sys.argv[1] == 'dumpdata': + # TODO: investigate Django how this can be avoided + # hack: a small patch to Django would be a better solution. + # Django's management command 'dumpdata' relies on non-polymorphic + # behaviour of the _default_manager. Therefore, we catch any access to _default_manager + # here and return the non-polymorphic default manager instead if we are called from 'dumpdata.py' + # Otherwise, the base objects will be upcasted to polymorphic models, and be outputted as such. + # (non-polymorphic default manager is 'base_objects' for polymorphic models). + # This way we don't need to patch django.core.management.commands.dumpdata + # for all supported Django versions. + frm = inspect.stack()[1] # frm[1] is caller file name, frm[3] is caller function name + if DUMPDATA_COMMAND in frm[1]: + return self.base_objects + + manager = super(PolymorphicModelBase, self)._default_manager + if not isinstance(manager, PolymorphicManager): + warnings.warn("{0}._default_manager is not a PolymorphicManager".format( + self.__class__.__name__ + ), RuntimeWarning) + + return manager diff --git a/polymorphic/models.py b/polymorphic/models.py index 893dddd..d8d53d9 100644 --- a/polymorphic/models.py +++ b/polymorphic/models.py @@ -54,7 +54,6 @@ class PolymorphicModel(six.with_metaclass(PolymorphicModelBase, models.Model)): # Note that Django 1.5 removes these managers because the model is abstract. # They are pretended to be there by the metaclass in PolymorphicModelBase.get_inherited_managers() objects = PolymorphicManager() - base_objects = models.Manager() class Meta: abstract = True diff --git a/polymorphic/query.py b/polymorphic/query.py index 6c87a93..c9b4c0d 100644 --- a/polymorphic/query.py +++ b/polymorphic/query.py @@ -379,7 +379,7 @@ class PolymorphicQuerySet(QuerySet): # Then we copy the extra() select fields from the base objects to the real objects. # TODO: defer(), only(): support for these would be around here for real_concrete_class, idlist in idlist_per_model.items(): - real_objects = real_concrete_class.base_objects.db_manager(self.db).filter(**{ + real_objects = real_concrete_class.objects.non_polymorphic().using(self.db).filter(**{ ('%s__in' % pk_name): idlist, }) real_objects.query.select_related = self.query.select_related # copy select related configuration to new qs diff --git a/polymorphic/tests/models.py b/polymorphic/tests/models.py index 60fc619..13fb2f9 100644 --- a/polymorphic/tests/models.py +++ b/polymorphic/tests/models.py @@ -191,7 +191,9 @@ class MROBase1(ShowFieldType, PolymorphicModel): class MROBase2(MROBase1): - pass # Django vanilla inheritance does not inherit MyManager as _default_manager here + class Meta: + # Django 1.x inheritance does not inherit MyManager as _default_manager here + manager_inheritance_from_future = True class MROBase3(models.Model): @@ -200,7 +202,8 @@ class MROBase3(models.Model): class MRODerived(MROBase2, MROBase3): - pass + class Meta: + manager_inheritance_from_future = True class ParentModelWithManager(PolymorphicModel):