Skip to content

Commit

Permalink
Add fix for when _default_manager is a custom TreeManager and new tes…
Browse files Browse the repository at this point in the history
…ts for custom TreeManagers.

Squashed commit of the following:

commit d55f90c8f0b0b6c90a91ba04b1db1e34af099593
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 02:32:20 2015 +0200

    re-enable _default_manager and remove test that fails for some other reason on django 1.4 django-mptt/django-mptt#371

commit 3939f182564c2d1b32a70efb7a5c910234da3887
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 02:20:58 2015 +0200

    Oh hi Travis, can you check this again to see how Django 1.4 likes _default_manager ?

commit 17957e03d2b6f9abbf7db890914194489af2929e
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 02:09:20 2015 +0200

    also assert that the classic version of .none() is compatible with the custom manager's queryset

commit 1cf707d0dae042387f3fae1e34739860f1dbdf2d
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 01:59:22 2015 +0200

    fix for django-mptt/django-mptt#371 by customizing test to behave differently if it's pre django 1.6

commit fcf90deeaf1f4e4d4a3bb9e8be265cd4e475d576
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 01:47:20 2015 +0200

    Re-add _default_manager but uncomment test that's generally failing on django 1.4

commit 073698bf2f13b5ab3b77916ce434a46a183c83f6
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 01:26:59 2015 +0200

    Hi Travis, please test if this works with Django 1.4 because it didnt B4

commit 04308a02f3a0fb2b3e2e9a8a55593de330c3aaaa
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 01:13:41 2015 +0200

    Renaming custom manager in doctests

commit 92737e9890babe7f310ad0bd4b0594d49c000eb8
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 01:04:19 2015 +0200

    Add optimization: If conventional "objects" attr exists, just use that and don't iterate everything else, also fixes django-mptt/django-mptt#369

commit 2b47ffd611158fa91c44398e584b2c5f1f1e96bf
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 01:03:42 2015 +0200

    Add test cases that fail when using _default_manager and objects django-mptt/django-mptt#369

commit 1ba49283070e2fda7d00bdd2680a0f9a039c890f
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 01:02:25 2015 +0200

    Add a custom method and a _default_manager that points to "objects" to reflect a practice that may or may not exist out there in the wild django-mptt/django-mptt#369

commit 00933d01a3974c088330fb77bfeb7e65dbecf185
Author: Benjamin Bach <benjaoming@gmail.com>
Date:   Thu Apr 23 01:01:56 2015 +0200

    add a single row on the Person test model
  • Loading branch information
LazarBekcic committed Apr 23, 2015
1 parent c470d90 commit 9ed48b4
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 23 deletions.
36 changes: 21 additions & 15 deletions mptt/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,21 +327,27 @@ def register(meta, cls, **kwargs):

# make sure we have a tree manager somewhere
tree_manager = None
for attr in sorted(dir(cls)):
try:
# HACK: avoid using getattr(cls, attr)
# because it calls __get__ on descriptors, which can cause nasty side effects
# with more inconsiderate apps.
# (e.g. django-tagging's TagField is a descriptor which can do db queries on getattr)
# ( ref: http://stackoverflow.com/questions/27790344 )
obj = cls.__dict__[attr]
except KeyError:
continue
if isinstance(obj, TreeManager):
tree_manager = obj
# prefer any locally defined manager (i.e. keep going if not local)
if obj.model is cls:
break
attrs = dir(cls)
if "objects" in attrs and isinstance(cls.objects, TreeManager):
tree_manager = cls.objects

# Go look for it somewhere else
else:
for attr in sorted(attrs):
try:
# HACK: avoid using getattr(cls, attr)
# because it calls __get__ on descriptors, which can cause nasty side effects
# with more inconsiderate apps.
# (e.g. django-tagging's TagField is a descriptor which can do db queries on getattr)
# ( ref: http://stackoverflow.com/questions/27790344 )
obj = cls.__dict__[attr]
except KeyError:
continue
if isinstance(obj, TreeManager):
tree_manager = obj
# prefer any locally defined manager (i.e. keep going if not local)
if obj.model is cls:
break
if tree_manager and tree_manager.model is not cls:
tree_manager = tree_manager._copy_to_model(cls)
elif tree_manager is None:
Expand Down
6 changes: 3 additions & 3 deletions tests/myapp/doctests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,11 @@

# TreeManager Methods #########################################################
# check that tree manager is the explicitly defined tree manager for Person
>>> Person._tree_manager == Person.my_tree_manager
>>> Person._tree_manager == Person.objects
True

# managers of non-abstract bases don't get inherited, so:
>>> Student._tree_manager == Student.my_tree_manager
>>> Student._tree_manager == Student.objects
False

>>> Student._tree_manager == Person._tree_manager
Expand Down Expand Up @@ -1554,7 +1554,7 @@ ValueError: Cannot insert a node which has already been saved.
>>> jane = Student.objects.create(name='Jane', parent=jeff)
>>> joe = Person.objects.create(name='Joe', parent=jane)
>>> julie = Student.objects.create(name='Julie', parent=jess)
>>> print_tree_details(Person.my_tree_manager.all())
>>> print_tree_details(Person.objects.all())
1 - 1 0 1 6
2 1 1 1 2 3
3 1 1 1 4 5
Expand Down
14 changes: 14 additions & 0 deletions tests/myapp/fixtures/persons.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[
{
"pk": 1,
"model": "myapp.person",
"fields": {
"rght": 16,
"name": "John Doe",
"parent": null,
"level": 0,
"lft": 1,
"tree_id": 1
}
}
]
30 changes: 27 additions & 3 deletions tests/myapp/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,27 @@
from django.db import models
from django.db.models.signals import pre_save
from django.utils.encoding import python_2_unicode_compatible
from django import VERSION as DJANGO_VERSION


import mptt
from mptt.fields import TreeForeignKey, TreeOneToOneField, TreeManyToManyField
from mptt.models import MPTTModel
from mptt.managers import TreeManager
from django.db.models.query import QuerySet
from django.db.models.query import QuerySet, EmptyQuerySet


class CustomTreeQueryset(QuerySet):
pass

def custom_method(self):
pass


class CustomEmptyTreeQueryset(EmptyQuerySet):
"""This is only used pre Django 1.6"""

def custom_method(self):
pass


class CustomTreeManager(TreeManager):
Expand All @@ -23,6 +34,16 @@ def get_queryset(self):
# Django 1.8 removed the fallbacks here.
return CustomTreeQueryset(model=self.model, using=self._db)

def get_empty_query_set(self):
# Pre 1.6 django, we needed a custom inheritor of EmptyQuerySet
# to pass custom methods. However, 1.6 introduced that EmptyQuerySet
# cannot be instantiated but instead passes through the methods
# of the custom QuerySet.
# See: https://code.djangoproject.com/ticket/22817
if DJANGO_VERSION < (1, 6):
return CustomEmptyTreeQueryset(model=self.model)
return self.get_queryset().none()


@python_2_unicode_compatible
class Category(MPTTModel):
Expand Down Expand Up @@ -124,7 +145,10 @@ class Person(MPTTModel):
parent = TreeForeignKey('self', null=True, blank=True, related_name='children')

# just testing it's actually possible to override the tree manager
my_tree_manager = CustomTreeManager()
objects = CustomTreeManager()

# This line is set because of https://github.com/django-mptt/django-mptt/issues/369
_default_manager = objects

def __str__(self):
return self.name
Expand Down
13 changes: 11 additions & 2 deletions tests/myapp/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,8 @@ def test_move_root_to_child(self):

class ManagerTests(TreeTestCase):
fixtures = ['categories.json',
'genres.json']
'genres.json',
'persons.json']

def test_all_managers_are_different(self):
# all tree managers should be different. otherwise, possible infinite recursion.
Expand Down Expand Up @@ -1234,8 +1235,16 @@ def test_custom_querysets(self):
"""
Test that a custom manager also provides custom querysets.
"""

self.assertTrue(isinstance(Person.objects.all(), CustomTreeQueryset))
self.assertTrue(isinstance(Person.objects.all()[0].get_children(), CustomTreeQueryset))
self.assertTrue(hasattr(Person.objects.none(), 'custom_method'))

# In Django 1.4, we would have had a custom type CustomEmptyTreeQueryset
# but this was abandoned in later versions. However, the best method is
# to just test if the custom method is available.
# self.assertTrue(hasattr(Person.objects.all()[0].get_children().none(), 'custom_method'))

self.assertEqual(
type(Person.objects.all()),
type(Person.objects.root_nodes())
Expand Down

0 comments on commit 9ed48b4

Please sign in to comment.