Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support to nested prefetch lookups #36

Merged
merged 1 commit into from
Jan 19, 2024
Merged

Add support to nested prefetch lookups #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions django_virtual_models/fields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,11 @@ def hydrate_queryset(
# always include the "back reference" field name in the Prefetch's lookup list
# to avoid N+1s in internal Django prefetch code
field_to_prefetch = self.lookup if self.lookup else self.field_name
field_descriptor = getattr(self.parent.model_cls, field_to_prefetch)
if type(field_descriptor) == ReverseManyToOneDescriptor: # don't use isinstance
back_reference = field_descriptor.rel.field.name
new_lookup_list.append(back_reference)
if "__" not in field_to_prefetch:
field_descriptor = getattr(self.parent.model_cls, field_to_prefetch)
if type(field_descriptor) == ReverseManyToOneDescriptor: # don't use isinstance
back_reference = field_descriptor.rel.field.name
new_lookup_list.append(back_reference)

# defer fields on prefetch_queryset
prefetch_queryset = _defer_fields(
Expand Down
12 changes: 6 additions & 6 deletions tests/optimization/test_exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Meta:
def get_lesson_titles_from_method(self, course: Annotated[Course, hints.Virtual("lessons")]):
...

@v.hints.from_types_of(get_lesson_title_list, "course")
@hints.from_types_of(get_lesson_title_list, "course")
def get_lesson_titles_from_function(self, course, get_lesson_title_list_helper):
...

Expand Down Expand Up @@ -166,7 +166,7 @@ def get_lesson_title_list(course): # no type annotation here
...

class BrokenCourseSerializer(BaseCourseSerializer):
@v.hints.from_types_of(get_lesson_title_list, "course")
@hints.from_types_of(get_lesson_title_list, "course")
def get_lesson_titles_from_function(self, course, get_lesson_title_list_helper):
...

Expand All @@ -189,7 +189,7 @@ def get_lesson_titles_from_function(self, course, get_lesson_title_list_helper):

def test_function_with_wrong_param_name_raises_exception(self):
class BrokenCourseSerializer(BaseCourseSerializer):
@v.hints.from_types_of(
@hints.from_types_of(
get_lesson_title_list, obj_param_name="course_blabla" # wrong param name
)
def get_lesson_titles_from_function(self, course, get_lesson_title_list_helper):
Expand Down Expand Up @@ -219,7 +219,7 @@ def get_lesson_title_list(course: Course): # no Annotated here
...

class BrokenCourseSerializer(BaseCourseSerializer):
@v.hints.from_types_of(get_lesson_title_list, "course")
@hints.from_types_of(get_lesson_title_list, "course")
def get_lesson_titles_from_function(self, course, get_lesson_title_list_helper):
...

Expand All @@ -244,7 +244,7 @@ def get_lesson_title_list(course: Annotated[Course, ()]): # wrong Annotated her
...

class BrokenCourseSerializer(BaseCourseSerializer):
@v.hints.from_types_of(get_lesson_title_list, "course")
@hints.from_types_of(get_lesson_title_list, "course")
def get_lesson_titles_from_function(self, course, get_lesson_title_list_helper):
...

Expand Down Expand Up @@ -276,7 +276,7 @@ def get_lesson_title_list(
...

class BrokenCourseSerializer(BaseCourseSerializer):
@v.hints.from_types_of(get_lesson_title_list, "course")
@hints.from_types_of(get_lesson_title_list, "course")
def get_lesson_titles_from_function(self, course, get_lesson_title_list_helper):
...

Expand Down
55 changes: 52 additions & 3 deletions tests/optimization/test_lookup_finder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ class Meta:
deferred_fields = ["description"]


class NestedFacilitatorUsers(v.VirtualModel):
class Meta:
model = User


class VirtualLesson(v.VirtualModel):
facilitator_users = NestedFacilitatorUsers(lookup="course__facilitators")

class Meta:
model = Lesson


class NestedAssignmentSerializer(serializers.ModelSerializer):
email = serializers.EmailField()
lessons_total = serializers.IntegerField()
Expand Down Expand Up @@ -149,11 +161,33 @@ def get_user_assignment(self, obj, serializer_cls):
return serializer_cls(obj.user_assignment[0]).data
return None

@v.hints.from_types_of(get_lesson_title_list, "course")
@hints.from_types_of(get_lesson_title_list, "course")
def get_lesson_titles(self, course, get_lesson_title_list_helper):
return get_lesson_title_list_helper(course)


class LessonSerializer(serializers.ModelSerializer):
facilitator_users = serializers.SerializerMethodField()

class Meta:
model = Lesson
virtual_model = VirtualLesson
fields = [
"title",
"content",
"facilitator_users",
]

@hints.defined_on_virtual_model()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the hint even though we have this field on the field list?

Copy link
Member Author

@fjsj fjsj Jan 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hints are always necessary if the field is not a concrete model field. The idea is to be explicit on what should be found at the virtual model instead of the model.

def get_facilitator_users(self, lesson):
if hasattr(lesson, "facilitator_users"):
return list({u.email for u in lesson.facilitator_users})

# this won't run because it's defined on virtual model,
# but one could add fallback code here:
return None


class LookupFinderTests(TestCase):
def setUp(self):
super().setUp()
Expand Down Expand Up @@ -264,6 +298,21 @@ class Meta:
]
)

def test_prefetch_with_nested_lookup(self):
qs = Lesson.objects.all()
serializer_instance = LessonSerializer(instance=qs, many=True)
virtual_model = VirtualLesson()

lookup_list = LookupFinder(
serializer_instance=serializer_instance,
virtual_model=virtual_model,
).recursively_find_lookup_list()

optimized_qs = virtual_model.get_optimized_queryset(qs=qs, lookup_list=lookup_list)
with self.assertNumQueries(3):
lesson_list = list(optimized_qs)
assert len(lesson_list) == 9

def test_ignored_nested_serializer_with_noop(self):
"""
Sometimes one needs a nested serializer generated dynamically.
Expand Down Expand Up @@ -375,7 +424,7 @@ class Meta:
@override_settings(DEBUG=True)
def test_prefetch_hints_block_queries_on_serializer_evaluation(self):
class BrokenCourseSerializer(CourseSerializer):
@v.hints.from_types_of(get_lesson_title_list, "course")
@hints.from_types_of(get_lesson_title_list, "course")
def get_lesson_titles(self, course, get_lesson_title_list_helper):
list(course.lessons.order_by("title")) # new query

Expand All @@ -398,7 +447,7 @@ def get_lesson_titles(self, course, get_lesson_title_list_helper):
@override_settings(DEBUG=True)
def test_prefetch_hints_does_not_block_queries_if_false(self):
class BrokenCourseSerializer(CourseSerializer):
@v.hints.from_types_of(get_lesson_title_list, "course")
@hints.from_types_of(get_lesson_title_list, "course")
def get_lesson_titles(self, course, get_lesson_title_list_helper):
list(course.lessons.order_by("title")) # new query

Expand Down
Loading