From e043aae9bb2e3fa244f59b07fc16f57476cb80ff Mon Sep 17 00:00:00 2001 From: Loic Bistuer Date: Wed, 24 Sep 2014 00:41:32 +0700 Subject: [PATCH] Fixed #23550 -- Normalized get_queryset() of RelatedObjectDescriptor and ReverseSingleRelatedObjectDescriptor so they actually return QuerySet instances. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- django/db/models/fields/related.py | 26 +++++++------- tests/one_to_one/models.py | 21 +++++++++++ tests/one_to_one/tests.py | 58 +++++++++++++++++++++++++++++- 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index bd52d8ec40..af6bf57b8a 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -370,14 +370,16 @@ class SingleRelatedObjectDescriptor(object): return hasattr(instance, self.cache_name) def get_queryset(self, **hints): - # Gotcha: we return a `Manager` instance (i.e. not a `QuerySet`)! - return self.related.model._base_manager.db_manager(hints=hints) + manager = self.related.model._default_manager + # 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): if queryset is None: - # Despite its name `get_queryset()` returns an instance of - # `Manager`, therefore we call `all()` to normalize to `QuerySet`. - queryset = self.get_queryset().all() + queryset = self.get_queryset() queryset._add_hints(instance=instances[0]) rel_obj_attr = attrgetter(self.related.field.attname) @@ -499,20 +501,16 @@ class ReverseSingleRelatedObjectDescriptor(object): return hasattr(instance, self.cache_name) 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 # related fields, respect that. - if getattr(rel_mgr, 'use_for_related_fields', False): - # Gotcha: we return a `Manager` instance (i.e. not a `QuerySet`)! - return rel_mgr - else: - return QuerySet(self.field.rel.to, hints=hints) + if not getattr(manager, 'use_for_related_fields', False): + manager = self.field.rel.to._base_manager + return manager.db_manager(hints=hints).all() def get_prefetch_queryset(self, instances, queryset=None): if queryset is None: - # Despite its name `get_queryset()` may return an instance of - # `Manager`, therefore we call `all()` to normalize to `QuerySet`. - queryset = self.get_queryset().all() + queryset = self.get_queryset() queryset._add_hints(instance=instances[0]) rel_obj_attr = self.field.get_foreign_related_value diff --git a/tests/one_to_one/models.py b/tests/one_to_one/models.py index ccb6851bc3..88b0ef6dcf 100644 --- a/tests/one_to_one/models.py +++ b/tests/one_to_one/models.py @@ -96,3 +96,24 @@ class Pointer2(models.Model): class HiddenPointer(models.Model): 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() diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py index da0fb5259f..d9eef0b133 100644 --- a/tests/one_to_one/tests.py +++ b/tests/one_to_one/tests.py @@ -4,7 +4,7 @@ from django.db import transaction, IntegrityError, connection from django.test import TestCase 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): @@ -382,3 +382,59 @@ class OneToOneTests(TestCase): self.assertFalse( 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(), + [""] + ) + + # Only one director is available via all() due to the custom default manager. + self.assertQuerysetEqual( + Director.objects.all(), + [""] + ) + + 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'))