mirror of
https://github.com/django/django.git
synced 2024-12-01 15:42:04 +01:00
Fixed #19069 -- Improved the error message when trying to query a swapped model.
Thanks to Preston Holmes for the suggestion.
This commit is contained in:
Russell Keith-Magee
parent
12f39be508
commit
cc337a74f1
@ -13,7 +13,11 @@ def ensure_default_manager(sender, **kwargs):
|
||||
_default_manager if it's not a subclass of Manager).
|
||||
"""
|
||||
cls = sender
|
||||
if cls._meta.abstract or cls._meta.swapped:
|
||||
if cls._meta.abstract:
|
||||
setattr(cls, 'objects', AbstractManagerDescriptor(cls))
|
||||
return
|
||||
elif cls._meta.swapped:
|
||||
setattr(cls, 'objects', SwappedManagerDescriptor(cls))
|
||||
return
|
||||
if not getattr(cls, '_default_manager', None):
|
||||
# Create the default manager, if needed.
|
||||
@ -58,7 +62,12 @@ class Manager(object):
|
||||
# TODO: Use weakref because of possible memory leak / circular reference.
|
||||
self.model = model
|
||||
# Only contribute the manager if the model is concrete
|
||||
if not model._meta.abstract and not model._meta.swapped:
|
||||
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
|
||||
@ -224,6 +233,30 @@ class ManagerDescriptor(object):
|
||||
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, type=None):
|
||||
raise AttributeError("Manager isn't available; %s is abstract" % (
|
||||
self.model._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, type=None):
|
||||
raise AttributeError("Manager isn't available; %s has been swapped for '%s'" % (
|
||||
self.model._meta.object_name, self.model._meta.swapped
|
||||
))
|
||||
|
||||
|
||||
class EmptyManager(Manager):
|
||||
def get_query_set(self):
|
||||
return self.get_empty_query_set()
|
||||
|
||||
@ -10,14 +10,17 @@ class OnlyFred(models.Manager):
|
||||
def get_query_set(self):
|
||||
return super(OnlyFred, self).get_query_set().filter(name='fred')
|
||||
|
||||
|
||||
class OnlyBarney(models.Manager):
|
||||
def get_query_set(self):
|
||||
return super(OnlyBarney, self).get_query_set().filter(name='barney')
|
||||
|
||||
|
||||
class Value42(models.Manager):
|
||||
def get_query_set(self):
|
||||
return super(Value42, self).get_query_set().filter(value=42)
|
||||
|
||||
|
||||
class AbstractBase1(models.Model):
|
||||
name = models.CharField(max_length=50)
|
||||
|
||||
@ -29,6 +32,7 @@ class AbstractBase1(models.Model):
|
||||
manager2 = OnlyBarney()
|
||||
objects = models.Manager()
|
||||
|
||||
|
||||
class AbstractBase2(models.Model):
|
||||
value = models.IntegerField()
|
||||
|
||||
@ -38,6 +42,7 @@ class AbstractBase2(models.Model):
|
||||
# Custom manager
|
||||
restricted = Value42()
|
||||
|
||||
|
||||
# No custom manager on this class to make sure the default case doesn't break.
|
||||
class AbstractBase3(models.Model):
|
||||
comment = models.CharField(max_length=50)
|
||||
@ -45,6 +50,7 @@ class AbstractBase3(models.Model):
|
||||
class Meta:
|
||||
abstract = True
|
||||
|
||||
|
||||
@python_2_unicode_compatible
|
||||
class Parent(models.Model):
|
||||
name = models.CharField(max_length=50)
|
||||
@ -54,6 +60,7 @@ class Parent(models.Model):
|
||||
def __str__(self):
|
||||
return self.name
|
||||
|
||||
|
||||
# Managers from base classes are inherited and, if no manager is specified
|
||||
# *and* the parent has a manager specified, the first one (in the MRO) will
|
||||
# become the default.
|
||||
@ -64,6 +71,7 @@ class Child1(AbstractBase1):
|
||||
def __str__(self):
|
||||
return self.data
|
||||
|
||||
|
||||
@python_2_unicode_compatible
|
||||
class Child2(AbstractBase1, AbstractBase2):
|
||||
data = models.CharField(max_length=25)
|
||||
@ -71,6 +79,7 @@ class Child2(AbstractBase1, AbstractBase2):
|
||||
def __str__(self):
|
||||
return self.data
|
||||
|
||||
|
||||
@python_2_unicode_compatible
|
||||
class Child3(AbstractBase1, AbstractBase3):
|
||||
data = models.CharField(max_length=25)
|
||||
@ -78,6 +87,7 @@ class Child3(AbstractBase1, AbstractBase3):
|
||||
def __str__(self):
|
||||
return self.data
|
||||
|
||||
|
||||
@python_2_unicode_compatible
|
||||
class Child4(AbstractBase1):
|
||||
data = models.CharField(max_length=25)
|
||||
@ -89,6 +99,7 @@ class Child4(AbstractBase1):
|
||||
def __str__(self):
|
||||
return self.data
|
||||
|
||||
|
||||
@python_2_unicode_compatible
|
||||
class Child5(AbstractBase3):
|
||||
name = models.CharField(max_length=25)
|
||||
@ -99,10 +110,12 @@ class Child5(AbstractBase3):
|
||||
def __str__(self):
|
||||
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
|
||||
|
||||
@ -1,26 +1,42 @@
|
||||
from __future__ import absolute_import
|
||||
import copy
|
||||
|
||||
from django.conf import settings
|
||||
from django.db import models
|
||||
from django.db.models.loading import cache
|
||||
from django.test import TestCase
|
||||
from django.test.utils import override_settings
|
||||
|
||||
from .models import Child1, Child2, Child3, Child4, Child5, Child6, Child7
|
||||
from .models import (
|
||||
Child1,
|
||||
Child2,
|
||||
Child3,
|
||||
Child4,
|
||||
Child5,
|
||||
Child6,
|
||||
Child7,
|
||||
AbstractBase1,
|
||||
AbstractBase2,
|
||||
AbstractBase3,
|
||||
)
|
||||
|
||||
|
||||
class ManagersRegressionTests(TestCase):
|
||||
def test_managers(self):
|
||||
a1 = Child1.objects.create(name='fred', data='a1')
|
||||
a2 = Child1.objects.create(name='barney', data='a2')
|
||||
b1 = Child2.objects.create(name='fred', data='b1', value=1)
|
||||
b2 = Child2.objects.create(name='barney', data='b2', value=42)
|
||||
c1 = Child3.objects.create(name='fred', data='c1', comment='yes')
|
||||
c2 = Child3.objects.create(name='barney', data='c2', comment='no')
|
||||
d1 = Child4.objects.create(name='fred', data='d1')
|
||||
d2 = Child4.objects.create(name='barney', data='d2')
|
||||
e1 = Child5.objects.create(name='fred', comment='yes')
|
||||
e2 = Child5.objects.create(name='barney', comment='no')
|
||||
f1 = Child6.objects.create(name='fred', data='f1', value=42)
|
||||
f2 = Child6.objects.create(name='barney', data='f2', value=42)
|
||||
g1 = Child7.objects.create(name='fred')
|
||||
g2 = Child7.objects.create(name='barney')
|
||||
Child1.objects.create(name='fred', data='a1')
|
||||
Child1.objects.create(name='barney', data='a2')
|
||||
Child2.objects.create(name='fred', data='b1', value=1)
|
||||
Child2.objects.create(name='barney', data='b2', value=42)
|
||||
Child3.objects.create(name='fred', data='c1', comment='yes')
|
||||
Child3.objects.create(name='barney', data='c2', comment='no')
|
||||
Child4.objects.create(name='fred', data='d1')
|
||||
Child4.objects.create(name='barney', data='d2')
|
||||
Child5.objects.create(name='fred', comment='yes')
|
||||
Child5.objects.create(name='barney', comment='no')
|
||||
Child6.objects.create(name='fred', data='f1', value=42)
|
||||
Child6.objects.create(name='barney', data='f2', value=42)
|
||||
Child7.objects.create(name='fred')
|
||||
Child7.objects.create(name='barney')
|
||||
|
||||
self.assertQuerysetEqual(Child1.manager1.all(), ["<Child1: a1>"])
|
||||
self.assertQuerysetEqual(Child1.manager2.all(), ["<Child1: a2>"])
|
||||
@ -54,3 +70,125 @@ class ManagersRegressionTests(TestCase):
|
||||
"<Child7: fred>"
|
||||
]
|
||||
)
|
||||
|
||||
def test_abstract_manager(self):
|
||||
# Accessing the manager on an abstract model should
|
||||
# raise an attribute error with an appropriate message.
|
||||
try:
|
||||
AbstractBase3.objects.all()
|
||||
self.fail('Should raise an AttributeError')
|
||||
except AttributeError as e:
|
||||
# This error message isn't ideal, but if the model is abstract and
|
||||
# a lot of the class instantiation logic isn't invoked; if the
|
||||
# manager is implied, then we don't get a hook to install the
|
||||
# error-raising manager.
|
||||
self.assertEqual(str(e), "type object 'AbstractBase3' has no attribute 'objects'")
|
||||
|
||||
def test_custom_abstract_manager(self):
|
||||
# Accessing the manager on an abstract model with an custom
|
||||
# manager should raise an attribute error with an appropriate
|
||||
# message.
|
||||
try:
|
||||
AbstractBase2.restricted.all()
|
||||
self.fail('Should raise an AttributeError')
|
||||
except AttributeError as e:
|
||||
self.assertEqual(str(e), "Manager isn't available; AbstractBase2 is abstract")
|
||||
|
||||
def test_explicit_abstract_manager(self):
|
||||
# Accessing the manager on an abstract model with an explicit
|
||||
# manager should raise an attribute error with an appropriate
|
||||
# message.
|
||||
try:
|
||||
AbstractBase1.objects.all()
|
||||
self.fail('Should raise an AttributeError')
|
||||
except AttributeError as e:
|
||||
self.assertEqual(str(e), "Manager isn't available; AbstractBase1 is abstract")
|
||||
|
||||
def test_swappable_manager(self):
|
||||
try:
|
||||
# This test adds dummy models to the app cache. These
|
||||
# need to be removed in order to prevent bad interactions
|
||||
# with the flush operation in other tests.
|
||||
old_app_models = copy.deepcopy(cache.app_models)
|
||||
old_app_store = copy.deepcopy(cache.app_store)
|
||||
|
||||
settings.TEST_SWAPPABLE_MODEL = 'managers_regress.Parent'
|
||||
|
||||
class SwappableModel(models.Model):
|
||||
class Meta:
|
||||
swappable = 'TEST_SWAPPABLE_MODEL'
|
||||
|
||||
# Accessing the manager on a swappable model should
|
||||
# raise an attribute error with a helpful message
|
||||
try:
|
||||
SwappableModel.objects.all()
|
||||
self.fail('Should raise an AttributeError')
|
||||
except AttributeError as e:
|
||||
self.assertEqual(str(e), "Manager isn't available; SwappableModel has been swapped for 'managers_regress.Parent'")
|
||||
|
||||
finally:
|
||||
del settings.TEST_SWAPPABLE_MODEL
|
||||
cache.app_models = old_app_models
|
||||
cache.app_store = old_app_store
|
||||
|
||||
def test_custom_swappable_manager(self):
|
||||
try:
|
||||
# This test adds dummy models to the app cache. These
|
||||
# need to be removed in order to prevent bad interactions
|
||||
# with the flush operation in other tests.
|
||||
old_app_models = copy.deepcopy(cache.app_models)
|
||||
old_app_store = copy.deepcopy(cache.app_store)
|
||||
|
||||
settings.TEST_SWAPPABLE_MODEL = 'managers_regress.Parent'
|
||||
|
||||
class SwappableModel(models.Model):
|
||||
|
||||
stuff = models.Manager()
|
||||
|
||||
class Meta:
|
||||
swappable = 'TEST_SWAPPABLE_MODEL'
|
||||
|
||||
# Accessing the manager on a swappable model with an
|
||||
# explicit manager should raise an attribute error with a
|
||||
# helpful message
|
||||
try:
|
||||
SwappableModel.stuff.all()
|
||||
self.fail('Should raise an AttributeError')
|
||||
except AttributeError as e:
|
||||
self.assertEqual(str(e), "Manager isn't available; SwappableModel has been swapped for 'managers_regress.Parent'")
|
||||
|
||||
finally:
|
||||
del settings.TEST_SWAPPABLE_MODEL
|
||||
cache.app_models = old_app_models
|
||||
cache.app_store = old_app_store
|
||||
|
||||
def test_explicit_swappable_manager(self):
|
||||
try:
|
||||
# This test adds dummy models to the app cache. These
|
||||
# need to be removed in order to prevent bad interactions
|
||||
# with the flush operation in other tests.
|
||||
old_app_models = copy.deepcopy(cache.app_models)
|
||||
old_app_store = copy.deepcopy(cache.app_store)
|
||||
|
||||
settings.TEST_SWAPPABLE_MODEL = 'managers_regress.Parent'
|
||||
|
||||
class SwappableModel(models.Model):
|
||||
|
||||
objects = models.Manager()
|
||||
|
||||
class Meta:
|
||||
swappable = 'TEST_SWAPPABLE_MODEL'
|
||||
|
||||
# Accessing the manager on a swappable model with an
|
||||
# explicit manager should raise an attribute error with a
|
||||
# helpful message
|
||||
try:
|
||||
SwappableModel.objects.all()
|
||||
self.fail('Should raise an AttributeError')
|
||||
except AttributeError as e:
|
||||
self.assertEqual(str(e), "Manager isn't available; SwappableModel has been swapped for 'managers_regress.Parent'")
|
||||
|
||||
finally:
|
||||
del settings.TEST_SWAPPABLE_MODEL
|
||||
cache.app_models = old_app_models
|
||||
cache.app_store = old_app_store
|
||||
|
||||
Loading…
Reference in New Issue
Block a user