Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #19069 -- Improved the error message when trying to query a swa…

…pped model.

Thanks to Preston Holmes for the suggestion.
  • Loading branch information...
commit cc337a74f1808b216fff96f1695d8b066d2636f6 1 parent 12f39be
@freakboy3742 freakboy3742 authored
View
37 django/db/models/manager.py
@@ -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 @@ def contribute_to_class(self, model, name):
# 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 @@ def __get__(self, instance, type=None):
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()
View
13 tests/regressiontests/managers_regress/models.py
@@ -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 Meta:
manager2 = OnlyBarney()
objects = models.Manager()
+
class AbstractBase2(models.Model):
value = models.IntegerField()
@@ -38,6 +42,7 @@ class Meta:
# 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
View
168 tests/regressiontests/managers_regress/tests.py
@@ -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 @@ def test_managers(self):
"<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
Please sign in to comment.
Something went wrong with that request. Please try again.