Skip to content

Commit

Permalink
Fixed #23550 -- Normalized get_queryset() of RelatedObjectDescriptor
Browse files Browse the repository at this point in the history
and ReverseSingleRelatedObjectDescriptor so they actually return QuerySet
instances.

Also ensured that SingleRelatedObjectDescriptor.get_queryset() accounts
for use_for_related_fields=True.

This cleanup lays the groundwork for #23533.

Thanks Anssi Kääriäinen for the review.
  • Loading branch information
loic committed Sep 23, 2014
1 parent 1fd6e13 commit e043aae
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 15 deletions.
26 changes: 12 additions & 14 deletions django/db/models/fields/related.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -370,14 +370,16 @@ def is_cached(self, instance):
return hasattr(instance, self.cache_name) return hasattr(instance, self.cache_name)


def get_queryset(self, **hints): def get_queryset(self, **hints):
# Gotcha: we return a `Manager` instance (i.e. not a `QuerySet`)! manager = self.related.model._default_manager
return self.related.model._base_manager.db_manager(hints=hints) # If the related manager indicates that it should be used for
# related fields, respect that.
if not getattr(manager, 'use_for_related_fields', False):
manager = self.related.model._base_manager
return manager.db_manager(hints=hints).all()


def get_prefetch_queryset(self, instances, queryset=None): def get_prefetch_queryset(self, instances, queryset=None):
if queryset is None: if queryset is None:
# Despite its name `get_queryset()` returns an instance of queryset = self.get_queryset()
# `Manager`, therefore we call `all()` to normalize to `QuerySet`.
queryset = self.get_queryset().all()
queryset._add_hints(instance=instances[0]) queryset._add_hints(instance=instances[0])


rel_obj_attr = attrgetter(self.related.field.attname) rel_obj_attr = attrgetter(self.related.field.attname)
Expand Down Expand Up @@ -499,20 +501,16 @@ def is_cached(self, instance):
return hasattr(instance, self.cache_name) return hasattr(instance, self.cache_name)


def get_queryset(self, **hints): def get_queryset(self, **hints):
rel_mgr = self.field.rel.to._default_manager.db_manager(hints=hints) manager = self.field.rel.to._default_manager
# If the related manager indicates that it should be used for # If the related manager indicates that it should be used for
# related fields, respect that. # related fields, respect that.
if getattr(rel_mgr, 'use_for_related_fields', False): if not getattr(manager, 'use_for_related_fields', False):
# Gotcha: we return a `Manager` instance (i.e. not a `QuerySet`)! manager = self.field.rel.to._base_manager
return rel_mgr return manager.db_manager(hints=hints).all()
else:
return QuerySet(self.field.rel.to, hints=hints)


def get_prefetch_queryset(self, instances, queryset=None): def get_prefetch_queryset(self, instances, queryset=None):
if queryset is None: if queryset is None:
# Despite its name `get_queryset()` may return an instance of queryset = self.get_queryset()
# `Manager`, therefore we call `all()` to normalize to `QuerySet`.
queryset = self.get_queryset().all()
queryset._add_hints(instance=instances[0]) queryset._add_hints(instance=instances[0])


rel_obj_attr = self.field.get_foreign_related_value rel_obj_attr = self.field.get_foreign_related_value
Expand Down
21 changes: 21 additions & 0 deletions tests/one_to_one/models.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -96,3 +96,24 @@ class Pointer2(models.Model):


class HiddenPointer(models.Model): class HiddenPointer(models.Model):
target = models.OneToOneField(Target, related_name='hidden+') target = models.OneToOneField(Target, related_name='hidden+')


# Test related objects visibility.
class SchoolManager(models.Manager):
def get_queryset(self):
return super(SchoolManager, self).get_queryset().filter(is_public=True)


class School(models.Model):
is_public = models.BooleanField(default=False)
objects = SchoolManager()


class DirectorManager(models.Manager):
def get_queryset(self):
return super(DirectorManager, self).get_queryset().filter(is_temp=False)

class Director(models.Model):
is_temp = models.BooleanField(default=False)
school = models.OneToOneField(School)
objects = DirectorManager()
58 changes: 57 additions & 1 deletion tests/one_to_one/tests.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from django.test import TestCase from django.test import TestCase


from .models import (Bar, Favorites, HiddenPointer, ManualPrimaryKey, MultiModel, from .models import (Bar, Favorites, HiddenPointer, ManualPrimaryKey, MultiModel,
Place, RelatedModel, Restaurant, Target, UndergroundBar, Waiter) Place, RelatedModel, Restaurant, School, Director, Target, UndergroundBar, Waiter)




class OneToOneTests(TestCase): class OneToOneTests(TestCase):
Expand Down Expand Up @@ -382,3 +382,59 @@ def test_hidden_accessor(self):
self.assertFalse( self.assertFalse(
hasattr(Target, HiddenPointer._meta.get_field('target').related.get_accessor_name()) hasattr(Target, HiddenPointer._meta.get_field('target').related.get_accessor_name())
) )

def test_related_object(self):
public_school = School.objects.create(is_public=True)
public_director = Director.objects.create(school=public_school, is_temp=False)

private_school = School.objects.create(is_public=False)
private_director = Director.objects.create(school=private_school, is_temp=True)

# Only one school is available via all() due to the custom default manager.
self.assertQuerysetEqual(
School.objects.all(),
["<School: School object>"]
)

# Only one director is available via all() due to the custom default manager.
self.assertQuerysetEqual(
Director.objects.all(),
["<Director: Director object>"]
)

self.assertEqual(public_director.school, public_school)
self.assertEqual(public_school.director, public_director)

# Make sure the base manager is used so that the related objects
# is still accessible even if the default manager doesn't normally
# allow it.
self.assertEqual(private_director.school, private_school)

# Make sure the base manager is used so that an student can still access
# its related school even if the default manager doesn't normally
# allow it.
self.assertEqual(private_school.director, private_director)

# If the manager is marked "use_for_related_fields", it'll get used instead
# of the "bare" queryset. Usually you'd define this as a property on the class,
# but this approximates that in a way that's easier in tests.
School.objects.use_for_related_fields = True
try:
private_director = Director._base_manager.get(pk=private_director.pk)
self.assertRaises(School.DoesNotExist, lambda: private_director.school)
finally:
School.objects.use_for_related_fields = False

Director.objects.use_for_related_fields = True
try:
private_school = School._base_manager.get(pk=private_school.pk)
self.assertRaises(Director.DoesNotExist, lambda: private_school.director)
finally:
Director.objects.use_for_related_fields = False

def test_hasattr_related_object(self):
# The exception raised on attribute access when a related object
# doesn't exist should be an instance of a subclass of `AttributeError`
# refs #21563
self.assertFalse(hasattr(Director(), 'director'))
self.assertFalse(hasattr(School(), 'school'))

0 comments on commit e043aae

Please sign in to comment.