Skip to content

Optimize community download count aggregation #1263

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

Open
ethangreen-dev wants to merge 1 commit into
base: master
Choose a base branch
from
+9 −3
Open

Optimize community download count aggregation #1263

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
12 changes: 9 additions & 3 deletions django/thunderstore/community/models/community.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.contrib.postgres.fields import ArrayField
from django.core.exceptions import ValidationError
from django.db import models, transaction
from django.db.models import Manager, QuerySet
from django.db.models import Manager, QuerySet, Sum
from django.urls import reverse
from django.utils.functional import cached_property

Expand Down Expand Up @@ -366,7 +366,13 @@ def update_for_community(cls, community: Community) -> None:
listings = listings.filter_with_single_community()

community.aggregated_fields.package_count = listings.count()
community.aggregated_fields.download_count = sum(
listing.total_downloads for listing in listings

from thunderstore.repository.models import PackageVersion

community.aggregated_fields.download_count = (
PackageVersion.objects.filter(
package_id__in=listings.values("package_id")
).aggregate(total=Sum("downloads"))["total"]
Comment on lines 368 to +375
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

files="$(fd -i 'package_listing.py')"
[ -n "$files" ] || { echo "package_listing.py not found"; exit 1; }

echo "== PackageListing model / Meta constraints =="
while IFS= read -r file; do
  echo "-- $file --"
  rg -n -C5 --type=py 'class PackageListing\b|class Meta\b|constraints\s*=|UniqueConstraint|unique_together|community\s*=|package\s*=' "$file"
done <<< "$files"

echo
echo "== Any PackageListing uniqueness declarations elsewhere =="
rg -n -C3 --type=py 'PackageListing|UniqueConstraint|unique_together'

Repository: thunderstore-io/Thunderstore

Length of output: 189

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the actual PackageListing model file
echo "== Locating PackageListing model =="
fd -type f -name "*.py" | xargs grep -l "class PackageListing" | head -5

echo
echo "== PackageListing model definition with Meta class =="
fd -type f -name "*.py" -path "*/models/*" | xargs grep -l "class PackageListing" | while read file; do
  echo "-- $file --"
  cat -n "$file"
done

Repository: thunderstore-io/Thunderstore

Length of output: 297

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find files containing PackageListing class definition
echo "== Files containing PackageListing class =="
find . -name "*.py" -type f -exec grep -l "class PackageListing" {} \; | head -10

echo
echo "== PackageListing model file in community/models =="
cat -n django/thunderstore/community/models/package_listing.py | head -100

Repository: thunderstore-io/Thunderstore

Length of output: 5272

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Continue reading the PackageListing model to find the Meta class
echo "== Full PackageListing model including Meta class =="
wc -l django/thunderstore/community/models/package_listing.py
echo
sed -n '100,300p' django/thunderstore/community/models/package_listing.py | cat -n

Repository: thunderstore-io/Thunderstore

Length of output: 7972

Consolidate download aggregation logic into the Package model.

The (package, community) uniqueness is already enforced at the database level, so the download count aggregation is safe. However, the Sum logic is now duplicated—it's defined both here and in Package.downloads. Consider using Package.downloads directly or extracting the aggregation into a shared helper to avoid divergence.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@django/thunderstore/community/models/community.py` around lines 368 - 375,
The download aggregation logic in community.aggregated_fields is duplicated with
Package.downloads; replace the inline Sum over PackageVersion (the block using
PackageVersion.objects.filter(package_id__in=listings.values("package_id")).aggregate(...))
by calling the existing aggregation on the Package model (e.g., use
Package.downloads for the set of packages represented by listings) or extract
the Sum logic into a shared helper function and call it from both places so the
aggregation lives in one spot; update references around
community.aggregated_fields.package_count and listings to use the chosen single
aggregation implementation.

or 0
)
community.aggregated_fields.save()
Loading