Commit cc337a74 authored by Russell Keith-Magee's avatar Russell Keith-Magee
Browse files

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

Thanks to Preston Holmes for the suggestion.
parent 12f39be5

django/db/models/manager.py

+35 −2
Original line number Diff line number Diff line
@@ -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

+13 −0
Original line number Diff line number Diff line
@@ -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

+153 −15
Original line number Diff line number Diff line 
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