Skip to content

Commit

Permalink
Optimize get_is_unavailable_query
Browse files Browse the repository at this point in the history
Refactor availability checks to reduce queries:
- Annotate availability in get_custom_package_listing.
- Modify DependencySerializer to use annotations instead of fetching
  the community model.
- Move availability check to PackageListing and update Package's
  is_unavailable.
- Add tests ensuring queries remain under 20 for small datasets;
  larger sets see a drop from 100+ to ~60.

Refs. TS-2276
  • Loading branch information
Roffenlund committed Jan 29, 2025
1 parent 53d0e55 commit 84e320b
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 21 deletions.
45 changes: 41 additions & 4 deletions django/thunderstore/api/cyberstorm/tests/test_package_listing.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from unittest.mock import Mock, PropertyMock, patch

import pytest
from django.db import connection
from django.test.utils import CaptureQueriesContext
from rest_framework.test import APIClient

from thunderstore.api.cyberstorm.views.package_listing import (
Expand Down Expand Up @@ -306,9 +308,10 @@ def test_dependency_serializer__reads_is_active_from_correct_field(
dependency.package.save()
dependant.dependencies.set([dependency])

# community_identifier is normally added using annotations, but
# it's irrelavant for this test case.
# community_identifier and version_is_unavailable is normally added using
# annotations, but it's irrelavant for this test case.
dependency.community_identifier = "greendale"
dependency.version_is_unavailable = False

actual = DependencySerializer(dependency).data

Expand All @@ -317,10 +320,11 @@ def test_dependency_serializer__reads_is_active_from_correct_field(

@pytest.mark.django_db
def test_dependency_serializer__when_dependency_is_not_active__censors_icon_and_description() -> None:
# community_identifier is normally added using annotations, but
# it's irrelavant for this test case.
# community_identifier and version_is_unavailable is normally added using
# annotations, but it's irrelavant for this test case.
dependency = PackageVersionFactory()
dependency.community_identifier = "greendale"
dependency.version_is_unavailable = False

actual = DependencySerializer(dependency).data

Expand Down Expand Up @@ -409,3 +413,36 @@ def test_package_listing_is_unavailable(

assert "is_unavailable" in response_dependencies
assert response_dependencies["is_unavailable"] == return_val


@pytest.mark.django_db
def test_package_listing_query_count(
api_client: APIClient, community: Community
) -> None:
package = "Mod"
target_ns = NamespaceFactory()

target_dependencies = [
PackageListingFactory(
community_=community,
package_kwargs={"name": f"{package}_{i}", "namespace": target_ns},
)
for i in range(10)
]

target_package = PackageListingFactory(community_=community)
target_package.package.latest.dependencies.set(
[dep.package.latest.id for dep in target_dependencies],
)

community_id = target_package.community.identifier
namespace = target_package.package.namespace.name
package_name = target_package.package.name

url = f"/api/cyberstorm/listing/{community_id}/{namespace}/{package_name}/"

with CaptureQueriesContext(connection) as ctx:
response = api_client.get(url)

assert response.status_code == 200
assert len(ctx.captured_queries) < 20
12 changes: 7 additions & 5 deletions django/thunderstore/api/cyberstorm/views/package_listing.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,9 @@ def get_icon_url(self, obj: PackageVersion) -> Optional[str]:
return obj.icon.url if obj.is_effectively_active else None

def get_is_unavailable(self, obj: PackageVersion) -> bool:
community = Community.objects.filter(
identifier=obj.community_identifier
).first()

return obj.is_unavailable(community)
# Annotated result of PackageVersion.is_unavailable
# See get_custom_package_listing()
return obj.version_is_unavailable


class TeamSerializer(serializers.Serializer):
Expand Down Expand Up @@ -191,10 +189,14 @@ def get_custom_package_listing(
package__name__iexact=package_name,
)

community = listing.community
version_is_unavailable = listing.package.latest.is_unavailable(community)

dependencies = (
listing.package.latest.dependencies.listed_in(community_id)
.annotate(
community_identifier=Value(community_id, CharField()),
version_is_unavailable=Value(version_is_unavailable, BooleanField()),
)
.select_related("package", "package__namespace")
.order_by("package__namespace__name", "package__name")
Expand Down
4 changes: 4 additions & 0 deletions django/thunderstore/community/models/package_listing.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@ def update_categories(self, agent: UserType, categories: List["PackageCategory"]
)
self.categories.set(categories)

@property
def is_unavailable(self):
return self.is_rejected or self.is_waiting_for_approval

def can_be_moderated_by_user(self, user: Optional[UserType]) -> bool:
return self.community.can_user_manage_packages(user)

Expand Down
26 changes: 26 additions & 0 deletions django/thunderstore/community/tests/test_package_listing.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest.mock import PropertyMock, patch

import pytest
from django.core.exceptions import ValidationError
from django.db import IntegrityError
Expand Down Expand Up @@ -416,3 +418,27 @@ def test_package_listing_filter_with_multiple_community_packages() -> None:
count = community.package_listings.filter_with_single_community().count()
assert count == 1
assert community.aggregated.package_count == count


@pytest.mark.django_db
@pytest.mark.parametrize(
("is_rejected", "is_waiting_for_approval", "expected"),
[(True, False, True), (False, True, True), (False, False, False)],
)
def test_package_listing_is_unavailable(
is_rejected: bool,
is_waiting_for_approval: bool,
expected: bool,
) -> None:
with patch(
"thunderstore.community.models.PackageListing.is_rejected",
new_callable=PropertyMock,
return_value=is_rejected,
), patch(
"thunderstore.community.models.PackageListing.is_waiting_for_approval",
new_callable=PropertyMock,
return_value=is_waiting_for_approval,
):

listing = PackageListingFactory()
assert listing.is_unavailable == expected
16 changes: 4 additions & 12 deletions django/thunderstore/repository/models/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,23 +135,15 @@ def update_listing(self, has_nsfw_content, categories, community):
listing.categories.add(*categories)
listing.save(update_fields=("has_nsfw_content",))

def listing_is_unavailable(self, listing) -> bool:
if listing is None:
return True

return any(
[
listing.is_rejected,
listing.is_waiting_for_approval,
]
)

def is_unavailable(self, community) -> bool:
if self.is_effectively_active is False:
return True

listing = self.get_package_listing(community)
return self.listing_is_unavailable(listing)
if listing is None:
return True

return listing.is_unavailable

@cached_property
def has_wiki(self) -> bool:
Expand Down

0 comments on commit 84e320b

Please sign in to comment.