fix(product): defer list count annotations

[kocaemre]

⚠️ Pre-Approval check ⚠️

This is a focused bugfix for an open issue: fixes #15378.

Description

Defers Product list count annotations until after filtering and pagination unless the request explicitly sorts by findings_count.

With V3_FEATURE_LOCATIONS enabled, the Product list currently adds finding and location count annotations before pagination. That can make PostgreSQL evaluate expensive correlated finding-count subqueries for intermediate rows created by location joins, even though the page only renders 25 products.

This PR:

• keeps the findings_count annotation before pagination only when the Product list is sorted by that field;
• applies distinct() to the filtered v3 product queryset before pagination so location joins do not duplicate products;
• moves rendered page-only findings_count, location_host_count, and location_count annotations into prefetch_for_product().

Test results

• .venv/bin/python manage.py test unittests.test_product_list_pagination -v 2
• .venv/bin/python -m ruff check --config ruff.toml dojo/product/ui/views.py unittests/test_product_list_pagination.py
• .venv/bin/python -m py_compile dojo/product/ui/views.py unittests/test_product_list_pagination.py
• /root/.local/share/uv/python/cpython-3.13-linux-x86_64-gnu/bin/python3.13 -m py_compile dojo/product/ui/views.py unittests/test_product_list_pagination.py
• git diff --check HEAD~1..HEAD

The Django test run passed with the existing local warning that components/node_modules is missing from STATICFILES_DIRS.

Documentation

No documentation update needed; this is a Product list query-planning/performance bugfix.

Checklist

• Make sure to rebase your PR against the very latest dev.
• Features/Changes should be submitted against the dev.
• Bugfixes should be submitted against the bugfix branch.
• Give a meaningful name to your PR, as it may end up being used in the release notes.
• Your code is Ruff compliant (see ruff.toml).
• Your code is python 3.13 compliant.
• If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
• Model changes must include the necessary migrations in the dojo/db_migrations folder.
• Add applicable tests to the unit tests.
• Add the proper label to categorize your PR.

[rossops]

Closed accidentally due to the bugfix branch getting deleted by some faulty release automation.

[valentijnscholten]

Thanks for digging into #15378 — the slow query is real and the location annotations are indeed involved. But I don't think this diff fixes it, and I'd like to suggest a much smaller change instead. I compiled the querysets against Django 5.2 to look at the generated SQL.

The counting side is already covered

Two mechanisms already exist:

1. get_page_items() in dojo/utils.py calls get_page_items_and_count(..., do_count=False), so there is no extra total_count query for this page.
2. Django masks unused annotations out of the Paginator's own COUNT query (Query.get_aggregation() → set_annotation_mask(aggregates)). With only the findings_count subquery annotated, count() compiles to literally SELECT COUNT(*) FROM dojo_product — the subquery is dropped.

So "annotations before pagination are expensive" isn't the mechanism here. A correlated Subquery annotation before pagination is nearly free.

The actual cause is narrower

The two Count("locations…", distinct=True) annotations add a LEFT JOIN and force a GROUP BY — and Django puts the correlated findings subquery into that GROUP BY, by ordinal:

GROUP BY "dojo_product"."id", ..., 3   -- 3 = the findings_count subquery

That is the 60,168 loops in the issue: the subquery becomes a grouping key, so it is evaluated once per joined location row instead of once per product. The aggregate also makes has_existing_aggregation true, which defeats the count-query masking above, so the Paginator's count wraps the same join + group and pays it a second time.

Why this PR doesn't remove the fan-out

prefetch_for_product() already carries six correlated subqueries (active_engagement_count, closed_engagement_count, last_engagement_date, active_finding_count, active_verified_finding_count, total_reimport_count). Moving the location aggregates into that queryset reproduces the same shape in the page query, now with ~7 subqueries as grouping keys — and LIMIT 25 is applied after the grouping, so it doesn't bound the scan:

SELECT DISTINCT product.*, <7 correlated subqueries>,
       COUNT(DISTINCT loc.host), COUNT(DISTINCT loc.id)
FROM dojo_product LEFT OUTER JOIN dojo_locationproductreference ...
GROUP BY product.id, product.name, 5, 4, ...
ORDER BY product.name LIMIT 25

So the PR fixes the count query and leaves the dominant page query fanning out the same way. An EXPLAIN (ANALYZE, BUFFERS) on a dataset like the reporter's would confirm, but the mechanism their plan shows is unchanged by this diff.

Suggested fix

Keep aggregates off the product queryset entirely and count LocationProductReference rows per product with subqueries — build_count_subquery() in dojo/query_utils.py exists for exactly this trap (see the comment in that file), and dojo/product_type/ui/views.py already uses it this way. No join, no GROUP BY, and the counts can then live pre- or post-pagination without any of the rest of this diff:

location_refs = LocationProductReference.objects.filter(product_id=OuterRef("pk"))
prods = prods.annotate(
    location_count=Coalesce(build_count_subquery(location_refs, group_field="product_id"), Value(0)),
    location_host_count=Coalesce(
        Subquery(
            location_refs.order_by().values("product_id")
            .annotate(c=Count("location__url__host", distinct=True))
            .order_by("product_id").values("c")[:1],
            output_field=IntegerField(),
        ),
        Value(0),
    ),
)

Smaller points, if you rework it:

• product_list_orders_by_findings_count() hand-parses the o parameter, reimplementing what the OrderingFilter in dojo/product/ui/filters.py already owns. It becomes unnecessary once the location counts are subqueries.
• findings_count=F("active_finding_count") emits the identical COALESCE((SELECT COUNT(...))) twice in the SELECT list, and in the sort path findings_count ends up annotated twice (no error, since it's a model property rather than a field, but it's confusing). Converging the template on one of the two names would be cleaner.
• The unconditional .distinct() costs every request with V3_FEATURE_LOCATIONS a SELECT COUNT(*) FROM (SELECT DISTINCT ...) for pagination. The duplicate-row risk really comes from the host filter in dojo/filters.py (locations__location__url__host__icontains can match several references per product; the plain locations__location= filter can't, thanks to the unique_location_and_product constraint), so dedup belongs in those filter methods rather than on every request.
• The tests are SimpleTestCase + MagicMock asserting the view's internal call order. They don't touch a database, so they can't demonstrate the fix, and they'll break on any harmless refactor. assertNumQueries / CaptureQueriesContext asserting that the page query has no GROUP BY, plus a query test showing a host filter doesn't duplicate products, is what would actually pin the behaviour.

[kocaemre]

Thanks for the detailed SQL analysis — I reworked this in the narrower direction you suggested.

Latest commit now:

• keeps findings_count as a correlated subquery annotation for ordering/filtering;
• replaces the v3 location Count(...) join aggregates with subquery-backed location_count / location_host_count, using build_count_subquery() for location_count;
• removes the previous unconditional Product-list .distinct();
• scopes deduplication to the endpoints__host filter path, where multiple matching location references can duplicate a product;
• replaces the mock call-order tests with database-backed regression coverage that checks the annotated queryset counts correctly and does not add a top-level Product GROUP BY / location-reference join.

Local verification:

DD_DATABASE_HOST=127.0.0.1 DD_DATABASE_PORT=5432 DD_DATABASE_USER=defectdojo DD_DATABASE_PASSWORD=*** DD_DATABASE_NAME=test_defectdojo DD_TEST_DATABASE_NAME=test_defectdojo .venv/bin/python manage.py test unittests.test_product_list_pagination -v 2 --keepdb
# Ran 2 tests in 3.726s — OK

.venv/bin/python -m ruff check --config ruff.toml dojo/product/ui/views.py dojo/filters.py unittests/test_product_list_pagination.py
# All checks passed!

.venv/bin/python -m py_compile dojo/product/ui/views.py dojo/filters.py unittests/test_product_list_pagination.py
git diff --check

The test run still reports the existing local components/node_modules staticfiles warning, but the focused tests pass.

[dryrunsecurity]

This pull request contains a critical finding where the sensitive file 'dojo/filters.py' was modified by an unauthorized author, 'kocaemre'. Although the issue is flagged as failing, it is not currently set to block the merge.

🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/filters.py (drs_a9c8652b)

Vulnerability	Configured Sensitive Codepath Modified by Non-Allowed Author
Description	File 'dojo/filters.py' matches configured sensitive codepath pattern 'dojo/filters.py' and was modified by 'kocaemre' (commit b5fe86e) who is not in the allowed authors list.

We've notified @mtesauro.

---

Comment to provide feedback on these findings.

Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]

Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing

All finding details can be found in the DryRun Security Dashboard.