From 36c9a1606bda5ec93550d09ab4f2731706075e0a Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Thu, 30 Oct 2025 16:28:25 -0400 Subject: [PATCH 1/3] Avoid n+1 queries on video.playlists serializer field --- learning_resources/filters_test.py | 9 ++++++--- learning_resources/models.py | 18 ++++++++++++++++++ learning_resources/serializers.py | 6 +----- learning_resources/serializers_test.py | 22 +++++++++++++++++++++- 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/learning_resources/filters_test.py b/learning_resources/filters_test.py index a496d899a6..711d919604 100644 --- a/learning_resources/filters_test.py +++ b/learning_resources/filters_test.py @@ -448,15 +448,18 @@ def test_learning_resource_filter_course_features(client): """Test that the resource_content_tag filter works""" resource_with_exams = LearningResourceFactory.create( - content_tags=LearningResourceContentTagFactory.create_batch(1, name="Exams") + content_tags=LearningResourceContentTagFactory.create_batch(1, name="Exams"), + resource_type=LearningResourceType.video.name, ) resource_with_notes = LearningResourceFactory.create( content_tags=LearningResourceContentTagFactory.create_batch( 1, name="Lecture Notes" - ) + ), + resource_type=LearningResourceType.video.name, ) LearningResourceFactory.create( - content_tags=LearningResourceContentTagFactory.create_batch(1, name="Other") + content_tags=LearningResourceContentTagFactory.create_batch(1, name="Other"), + resource_type=LearningResourceType.video.name, ) results = client.get(f"{RESOURCE_API_URL}?course_feature=exams").json()["results"] diff --git a/learning_resources/models.py b/learning_resources/models.py index f9c946d360..fc0661e68d 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -390,6 +390,13 @@ def for_serialization(self, *, user: Optional["User"] = None): ), to_attr="_podcasts", ), + Prefetch( + "parents", + queryset=LearningResourceRelationship.objects.filter( + relation_type=LearningResourceRelationTypes.PLAYLIST_VIDEOS.value, + ), + to_attr="_playlists", + ), Prefetch( "children", queryset=LearningResourceRelationship.objects.select_related( @@ -604,6 +611,17 @@ def podcasts(self) -> list["LearningResourceRelationship"]: ), ) + @cached_property + def playlists(self) -> list["LearningResourceRelationship"]: + """Return a list of playlists that the resource is in""" + return getattr( + self, + "_playlists", + self.parents.filter( + relation_type=LearningResourceRelationTypes.PLAYLIST_VIDEOS.value, + ), + ) + class Meta: unique_together = (("platform", "readable_id", "resource_type"),) diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index 9b13d0291b..0f56349eca 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -1097,11 +1097,7 @@ class VideoResourceSerializer(LearningResourceBaseSerializer): def get_playlists(self, instance) -> list[str]: """Get the playlist id(s) the video belongs to""" - return list( - instance.parents.filter( - relation_type=constants.LearningResourceRelationTypes.PLAYLIST_VIDEOS.value - ).values_list("parent__id", flat=True) - ) + return [playlist.parent_id for playlist in instance.playlists] class VideoPlaylistResourceSerializer(LearningResourceBaseSerializer): diff --git a/learning_resources/serializers_test.py b/learning_resources/serializers_test.py index c153f0ce11..dbff1d657b 100644 --- a/learning_resources/serializers_test.py +++ b/learning_resources/serializers_test.py @@ -33,7 +33,11 @@ LearningResourcePriceFactory, LearningResourceRunFactory, ) -from learning_resources.models import ContentFile, LearningResource +from learning_resources.models import ( + ContentFile, + LearningResource, + LearningResourceRelationship, +) from main.test_utils import assert_json_equal, drf_datetime from main.utils import frontend_absolute_url @@ -141,6 +145,22 @@ def test_serialize_podcast_episode_to_json(): ) +def test_serialize_video_resource_playlists_to_json(): + """ + Verify that a serialized video resource has the correct playlist data + """ + playlist = factories.VideoPlaylistFactory.create() + video = factories.VideoFactory.create() + LearningResourceRelationship.objects.get_or_create( + parent=playlist.learning_resource, + child=video.learning_resource, + relation_type=LearningResourceRelationTypes.PLAYLIST_VIDEOS.value, + ) + serializer = serializers.VideoResourceSerializer(instance=video.learning_resource) + assert len(video.learning_resource.playlists) == 1 + assert serializer.data["playlists"] == [playlist.learning_resource.id] + + @pytest.mark.parametrize("has_context", [True, False]) @pytest.mark.parametrize( ("params", "detail_key", "specific_serializer_cls", "detail_serializer_cls"), From f75b92919b1163336f92216b5d82b1ba274dcdbf Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Thu, 30 Oct 2025 16:35:17 -0400 Subject: [PATCH 2/3] One more unit test --- learning_resources/serializers_test.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/learning_resources/serializers_test.py b/learning_resources/serializers_test.py index dbff1d657b..06a065356b 100644 --- a/learning_resources/serializers_test.py +++ b/learning_resources/serializers_test.py @@ -157,10 +157,24 @@ def test_serialize_video_resource_playlists_to_json(): relation_type=LearningResourceRelationTypes.PLAYLIST_VIDEOS.value, ) serializer = serializers.VideoResourceSerializer(instance=video.learning_resource) - assert len(video.learning_resource.playlists) == 1 assert serializer.data["playlists"] == [playlist.learning_resource.id] +def test_serialize_podcast_episode_playlists_to_json(): + """ + Verify that a serialized podcast episode resource has the correct podcast data + """ + podcast = factories.PodcastFactory.create() + podcast_episode = factories.PodcastEpisodeFactory.create() + LearningResourceRelationship.objects.get_or_create( + parent=podcast.learning_resource, + child=podcast_episode.learning_resource, + relation_type=LearningResourceRelationTypes.PODCAST_EPISODES.value, + ) + serializer = serializers.PodcastEpisodeSerializer(instance=podcast_episode) + assert serializer.data["podcasts"] == [podcast.learning_resource.id] + + @pytest.mark.parametrize("has_context", [True, False]) @pytest.mark.parametrize( ("params", "detail_key", "specific_serializer_cls", "detail_serializer_cls"), From 5c0ce89899ebcfc3fec9f9dc9fa2d732f6926ba3 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Thu, 30 Oct 2025 17:46:13 -0400 Subject: [PATCH 3/3] Fix tests --- learning_resources/views_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/learning_resources/views_test.py b/learning_resources/views_test.py index ae5356958c..274d702eb6 100644 --- a/learning_resources/views_test.py +++ b/learning_resources/views_test.py @@ -194,7 +194,7 @@ def test_program_detail_endpoint(client, django_assert_num_queries, url): """Test program endpoint""" program = ProgramFactory.create() assert program.learning_resource.children.count() > 0 - with django_assert_num_queries(20): # should be same # regardless of child count + with django_assert_num_queries(21): # should be same # regardless of child count resp = client.get(reverse(url, args=[program.learning_resource.id])) assert resp.data.get("title") == program.learning_resource.title assert resp.data.get("resource_type") == LearningResourceType.program.name @@ -239,7 +239,7 @@ def test_no_excess_queries(rf, user, mocker, django_assert_num_queries, course_c request = rf.get("/") request.user = user - with django_assert_num_queries(22): + with django_assert_num_queries(23): view = CourseViewSet(request=request) results = view.get_queryset().all() assert len(results) == course_count