From a51c2c16dcd201ce4089e6073a0bf7b3882866f3 Mon Sep 17 00:00:00 2001 From: Daniel Vainio Date: Fri, 3 Oct 2025 16:40:48 +0300 Subject: [PATCH] Optimize dependencies list queryset query Annotate the queryset with active_package_versions instead of calling PackageVersion.is_removed, which avoids triggering an N+1 query. Refs. TS-2672 --- .../api/cyberstorm/serializers/package.py | 10 +++++- .../tests/test_package_version_list.py | 34 +++++++++++++++++-- .../api/cyberstorm/tests/test_query_count.py | 2 +- .../cyberstorm/views/package_version_list.py | 9 ++++- 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/django/thunderstore/api/cyberstorm/serializers/package.py b/django/thunderstore/api/cyberstorm/serializers/package.py index 5ed8ace4e..126509c03 100644 --- a/django/thunderstore/api/cyberstorm/serializers/package.py +++ b/django/thunderstore/api/cyberstorm/serializers/package.py @@ -67,7 +67,15 @@ class CyberstormPackageDependencySerializer(serializers.Serializer): name = serializers.CharField() namespace = serializers.CharField(source="package.namespace.name") version_number = serializers.CharField() - is_removed = serializers.BooleanField() + is_removed = serializers.SerializerMethodField() + + def get_is_removed(self, obj: PackageVersion) -> bool: + package_is_removed = not ( + obj.package.is_active and obj.package_has_active_versions + ) + if package_is_removed: + return True + return not obj.is_active def get_description(self, obj: PackageVersion) -> str: return ( diff --git a/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py b/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py index 2ccbae4e2..ef13b7ede 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py +++ b/django/thunderstore/api/cyberstorm/tests/test_package_version_list.py @@ -1,8 +1,6 @@ from typing import Optional import pytest -from django.db import connection -from django.test.utils import CaptureQueriesContext from rest_framework.test import APIClient from thunderstore.repository.factories import PackageFactory, PackageVersionFactory @@ -134,3 +132,35 @@ def test_package_version_dependencies_list_response(api_client: APIClient) -> No assert response.status_code == 200 assert response.json() == expected_data + + +@pytest.mark.django_db +@pytest.mark.parametrize( + "is_active, is_removed", + [ + (False, True), + (True, False), + ], +) +def test_package_version_dependencies_list_version_is_active( + api_client: APIClient, + is_active: bool, + is_removed: bool, +) -> None: + """Test the is_removed field in the PackageVersionDependenciesListAPIView.""" + + package = PackageFactory(name="TestPackage") + package_version = PackageVersionFactory(package=package) + + dependency = PackageVersionFactory(is_active=is_active) + package_version.dependencies.set([dependency.id]) + + namespace = package.namespace.name + package_name = package.name + + url = f"/api/cyberstorm/package/{namespace}/{package_name}/v/latest/dependencies/" + response = api_client.get(url) + + assert response.status_code == 200 + assert response.json()["results"][0]["is_removed"] == is_removed + assert response.json()["results"][0]["is_active"] == is_active diff --git a/django/thunderstore/api/cyberstorm/tests/test_query_count.py b/django/thunderstore/api/cyberstorm/tests/test_query_count.py index 49960e59c..93b3f10b2 100644 --- a/django/thunderstore/api/cyberstorm/tests/test_query_count.py +++ b/django/thunderstore/api/cyberstorm/tests/test_query_count.py @@ -233,5 +233,5 @@ def test_package_version_dependencies_query_count(api_client: APIClient) -> None client=api_client, method="get", path=path, - max_queries=25, # TODO: this could be optimized further + max_queries=MAX_QUERIES, ) diff --git a/django/thunderstore/api/cyberstorm/views/package_version_list.py b/django/thunderstore/api/cyberstorm/views/package_version_list.py index 927e06fbd..fbd0eb389 100644 --- a/django/thunderstore/api/cyberstorm/views/package_version_list.py +++ b/django/thunderstore/api/cyberstorm/views/package_version_list.py @@ -1,4 +1,4 @@ -from django.db.models import QuerySet +from django.db.models import Exists, OuterRef, QuerySet from rest_framework import serializers from rest_framework.generics import ListAPIView, get_object_or_404 @@ -54,6 +54,13 @@ def get_queryset(self) -> QuerySet[PackageVersion]: qs = ( package_version.dependencies.all() .select_related("package", "package__namespace") + .annotate( + package_has_active_versions=Exists( + PackageVersion.objects.filter( + package_id=OuterRef("package__pk"), is_active=True + ) + ) + ) .order_by("package__namespace__name", "package__name") )