Fixed #23550 -- Normalized get_queryset() of RelatedObjectDescriptor
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.
This commit is contained in:
parent
1fd6e13bf2
commit
e043aae9bb
|
@ -370,14 +370,16 @@ class SingleRelatedObjectDescriptor(object):
|
||||||
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)
|
||||||
|
@ -499,20 +501,16 @@ class ReverseSingleRelatedObjectDescriptor(object):
|
||||||
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
|
||||||
|
|
|
@ -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()
|
||||||
|
|
|
@ -4,7 +4,7 @@ from django.db import transaction, IntegrityError, connection
|
||||||
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):
|
||||||
|
@ -382,3 +382,59 @@ class OneToOneTests(TestCase):
|
||||||
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'))
|
||||||
|
|
Loading…
Reference in New Issue