Fixed #19069 -- Improved the error message when trying to query a swapped model.

Thanks to Preston Holmes for the suggestion.

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 @@ 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()

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 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

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')
+        Child1.objects.create(name='fred', data='a1')
-        a2 = Child1.objects.create(name='barney', data='a2')
+        Child1.objects.create(name='barney', data='a2')
-        b1 = Child2.objects.create(name='fred', data='b1', value=1)
+        Child2.objects.create(name='fred', data='b1', value=1)
-        b2 = Child2.objects.create(name='barney', data='b2', value=42)
+        Child2.objects.create(name='barney', data='b2', value=42)
-        c1 = Child3.objects.create(name='fred', data='c1', comment='yes')
+        Child3.objects.create(name='fred', data='c1', comment='yes')
-        c2 = Child3.objects.create(name='barney', data='c2', comment='no')
+        Child3.objects.create(name='barney', data='c2', comment='no')
-        d1 = Child4.objects.create(name='fred', data='d1')
+        Child4.objects.create(name='fred', data='d1')
-        d2 = Child4.objects.create(name='barney', data='d2')
+        Child4.objects.create(name='barney', data='d2')
-        e1 = Child5.objects.create(name='fred', comment='yes')
+        Child5.objects.create(name='fred', comment='yes')
-        e2 = Child5.objects.create(name='barney', comment='no')
+        Child5.objects.create(name='barney', comment='no')
-        f1 = Child6.objects.create(name='fred', data='f1', value=42)
+        Child6.objects.create(name='fred', data='f1', value=42)
-        f2 = Child6.objects.create(name='barney', data='f2', value=42)
+        Child6.objects.create(name='barney', data='f2', value=42)
-        g1 = Child7.objects.create(name='fred')
+        Child7.objects.create(name='fred')
-        g2 = Child7.objects.create(name='barney')
+        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