Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fixed #7154 -- Inherit all model managers from abstract base classes.
Also added documentation describing how manager inheritance works (and when
manager aren't inherited). Based on some patches from sebastian_noack and
emulbreh.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@8851 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information
malcolmt committed Sep 2, 2008
1 parent cf5087f commit f31425e
Show file tree
Hide file tree
Showing 6 changed files with 211 additions and 9 deletions.
14 changes: 8 additions & 6 deletions django/db/models/base.py
Expand Up @@ -67,11 +67,7 @@ def __new__(cls, name, bases, attrs):
if not hasattr(meta, 'get_latest_by'):
new_class._meta.get_latest_by = base_meta.get_latest_by

old_default_mgr = None
if getattr(new_class, '_default_manager', None):
# We have a parent who set the default manager.
if new_class._default_manager.model._meta.abstract:
old_default_mgr = new_class._default_manager
new_class._default_manager = None

# Bail out early if we have already created this class.
Expand Down Expand Up @@ -111,6 +107,14 @@ def __new__(cls, name, bases, attrs):
% (field.name, name, base.__name__))
new_class.add_to_class(field.name, copy.deepcopy(field))

# Inherit managers from the abstract base classes.
base_managers = base._meta.abstract_managers
base_managers.sort()
for _, mgr_name, manager in base_managers:
val = getattr(new_class, mgr_name, None)
if not val or val is manager:
new_manager = manager._copy_to_model(new_class)
new_class.add_to_class(mgr_name, new_manager)
if abstract:
# Abstract base models can't be instantiated and don't appear in
# the list of models for an app. We do the final setup for them a
Expand All @@ -119,8 +123,6 @@ def __new__(cls, name, bases, attrs):
new_class.Meta = attr_meta
return new_class

if old_default_mgr and not new_class._default_manager:
new_class._default_manager = old_default_mgr._copy_to_model(new_class)
new_class._prepare()
register_models(new_class._meta.app_label, new_class)

Expand Down
18 changes: 15 additions & 3 deletions django/db/models/manager.py
Expand Up @@ -23,17 +23,27 @@ class Manager(object):

def __init__(self):
super(Manager, self).__init__()
# Increase the creation counter, and save our local copy.
self.creation_counter = Manager.creation_counter
Manager.creation_counter += 1
self._set_creation_counter()
self.model = None
self._inherited = False

def contribute_to_class(self, model, name):
# TODO: Use weakref because of possible memory leak / circular reference.
self.model = model
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
if model._meta.abstract or self._inherited:
model._meta.abstract_managers.append((self.creation_counter, name,
self))

def _set_creation_counter(self):
"""
Sets the creation counter value for this instance and increments the
class-level copy.
"""
self.creation_counter = Manager.creation_counter
Manager.creation_counter += 1

def _copy_to_model(self, model):
"""
Expand All @@ -43,7 +53,9 @@ def _copy_to_model(self, model):
"""
assert issubclass(model, self.model)
mgr = copy.copy(self)
mgr._set_creation_counter()
mgr.model = model
mgr._inherited = True
return mgr

#######################
Expand Down
3 changes: 3 additions & 0 deletions django/db/models/options.py
Expand Up @@ -44,6 +44,9 @@ def __init__(self, meta, app_label=None):
self.abstract = False
self.parents = SortedDict()
self.duplicate_targets = {}
# Managers that have been inherited from abstract base classes. These
# are passed onto any children.
self.abstract_managers = []

def contribute_to_class(self, cls, name):
from django.db import connection
Expand Down
32 changes: 32 additions & 0 deletions docs/topics/db/managers.txt
Expand Up @@ -189,3 +189,35 @@ attributes by giving it a ``use_for_related_fields`` property::


...

Custom managers and model inheritance
-------------------------------------

Class inheritance and model managers aren't quite a perfect match for each
other. Managers are often specific to the classes they are defined on and
inheriting them in subclasses isn't necessarily a good idea. Also, because the
first manager declared is the *default manager*, it is important to allow that
to be controlled. So here's how Django handles custom managers and
:ref:`model inheritance <model-inheritance>`:

1. Managers defined on non-abstract base classes are *not* inherited by
child classes. If you want to reuse a manager from a non-abstract base,
redeclare it explicitly on the child class. These sorts of managers are
likely to be fairly specific to the class they are defined on, so
inheriting them can often lead to unexpected results (particularly as
far as the default manager goes). Therefore, they aren't passed onto
child classes.

2. Managers from abstract base classes are always inherited by the child
class, using Python's normal name resolution order (names on the child
class override all others; then come names on the first parent class,
and so on). Abstract base classes are designed to capture information
and behaviour that is common to their child classes. Defining common
managers is an appropriate part of this common information.

3. The default manager on a class is either the first manager declared on
the class, if that exists, or the default manager of the first abstract
base class in the parent hierarchy, if that exists. If no default
manager is explicitly declared, Django's normal default manager is
used.

Empty file.
153 changes: 153 additions & 0 deletions tests/regressiontests/managers_regress/models.py
@@ -0,0 +1,153 @@
"""
Various edge-cases for model managers.
"""

from django.db import models

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)

class Meta:
abstract = True

# Custom managers
manager1 = OnlyFred()
manager2 = OnlyBarney()
objects = models.Manager()

class AbstractBase2(models.Model):
value = models.IntegerField()

class Meta:
abstract = True

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

class Meta:
abstract = True

class Parent(models.Model):
name = models.CharField(max_length=50)

manager = OnlyFred()

def __unicode__(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.
class Child1(AbstractBase1):
data = models.CharField(max_length=25)

def __unicode__(self):
return self.data

class Child2(AbstractBase1, AbstractBase2):
data = models.CharField(max_length=25)

def __unicode__(self):
return self.data

class Child3(AbstractBase1, AbstractBase3):
data = models.CharField(max_length=25)

def __unicode__(self):
return self.data

class Child4(AbstractBase1):
data = models.CharField(max_length=25)

# Should be the default manager, although the parent managers are
# inherited.
default = models.Manager()

def __unicode__(self):
return self.data

class Child5(AbstractBase3):
name = models.CharField(max_length=25)

default = OnlyFred()
objects = models.Manager()

def __unicode__(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

__test__ = {"API_TESTS": """
>>> 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.manager1.all()
[<Child1: a1>]
>>> Child1.manager2.all()
[<Child1: a2>]
>>> Child1._default_manager.all()
[<Child1: a1>]
>>> Child2._default_manager.all()
[<Child2: b1>]
>>> Child2.restricted.all()
[<Child2: b2>]
>>> Child3._default_manager.all()
[<Child3: c1>]
>>> Child3.manager1.all()
[<Child3: c1>]
>>> Child3.manager2.all()
[<Child3: c2>]
# Since Child6 inherits from Child4, the corresponding rows from f1 and f2 also
# appear here. This is the expected result.
>>> Child4._default_manager.order_by('data')
[<Child4: d1>, <Child4: d2>, <Child4: f1>, <Child4: f2>]
>>> Child4.manager1.all()
[<Child4: d1>, <Child4: f1>]
>>> Child5._default_manager.all()
[<Child5: fred>]
>>> Child6._default_manager.all()
[<Child6: f1>]
>>> Child7._default_manager.order_by('name')
[<Child7: barney>, <Child7: fred>]
"""}

0 comments on commit f31425e

Please sign in to comment.