From e6a6b601a52315726b612437e56118828a03aca4 Mon Sep 17 00:00:00 2001 From: Emre Koca <110906681+kocaemre@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:02:36 +0200 Subject: [PATCH 1/2] fix(product): defer list count annotations Signed-off-by: Emre Koca <110906681+kocaemre@users.noreply.github.com> --- dojo/product/ui/views.py | 43 ++++++--- unittests/test_product_list_pagination.py | 112 ++++++++++++++++++++++ 2 files changed, 143 insertions(+), 12 deletions(-) create mode 100644 unittests/test_product_list_pagination.py diff --git a/dojo/product/ui/views.py b/dojo/product/ui/views.py index 5d6924ae033..0ddf6cbb899 100644 --- a/dojo/product/ui/views.py +++ b/dojo/product/ui/views.py @@ -131,6 +131,24 @@ labels = get_labels() +def product_list_orders_by_findings_count(request): + order_values = request.GET.getlist("o") + for value in order_values: + for field in value.split(","): + if field.strip().lstrip("-") == "findings_count": + return True + return False + + +def annotate_product_findings_count(prods): + base_findings = Finding.objects.filter(test__engagement__product_id=OuterRef("pk"), active=True) + return prods.annotate( + findings_count=Coalesce( + build_count_subquery(base_findings, group_field="test__engagement__product_id"), Value(0), + ), + ) + + def product(request): prods = get_authorized_products("view") # perform all stuff for filtering and pagination first, before annotation/prefetching @@ -138,22 +156,16 @@ def product(request): # see https://code.djangoproject.com/ticket/23771 and https://code.djangoproject.com/ticket/25375 name_words = prods.values_list("name", flat=True) - base_findings = Finding.objects.filter(test__engagement__product_id=OuterRef("pk"), active=True) - prods = prods.annotate( - findings_count=Coalesce( - build_count_subquery(base_findings, group_field="test__engagement__product_id"), Value(0), - ), - ) - if settings.V3_FEATURE_LOCATIONS: - prods = prods.annotate( - location_host_count=Count("locations__location__url__host", distinct=True), - location_count=Count("locations", distinct=True), - ) + if product_list_orders_by_findings_count(request): + prods = annotate_product_findings_count(prods) filter_string_matching = get_system_setting("filter_string_matching", False) filter_class = ProductFilterWithoutObjectLookups if filter_string_matching else ProductFilter prod_filter = filter_class(request.GET, queryset=prods, user=request.user) - prod_list = get_page_items(request, prod_filter.qs, 25) + prod_qs = prod_filter.qs + if settings.V3_FEATURE_LOCATIONS: + prod_qs = prod_qs.distinct() + prod_list = get_page_items(request, prod_qs, 25) # perform annotation/prefetching by replacing the queryset in the page with an annotated/prefetched queryset. prod_list.object_list = prefetch_for_product(prod_list.object_list) @@ -201,7 +213,14 @@ def prefetch_for_product(prods): count_subquery(base_findings.filter(active=True, verified=True)), Value(0), ), + ).annotate( + findings_count=F("active_finding_count"), ) + if settings.V3_FEATURE_LOCATIONS: + prefetched_prods = prefetched_prods.annotate( + location_host_count=Count("locations__location__url__host", distinct=True), + location_count=Count("locations", distinct=True), + ) prefetched_prods = prefetched_prods.annotate( total_reimport_count=Coalesce( count_subquery( diff --git a/unittests/test_product_list_pagination.py b/unittests/test_product_list_pagination.py new file mode 100644 index 00000000000..343565e175f --- /dev/null +++ b/unittests/test_product_list_pagination.py @@ -0,0 +1,112 @@ +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from django.http import QueryDict +from django.test import RequestFactory, SimpleTestCase, override_settings + +from dojo.product.ui import views + + +class ProductListPaginationAnnotationTests(SimpleTestCase): + def test_product_list_ordering_detects_findings_count(self): + factory = RequestFactory() + + self.assertFalse(views.product_list_orders_by_findings_count(factory.get("/products"))) + self.assertFalse(views.product_list_orders_by_findings_count(factory.get("/products?o=name"))) + self.assertTrue(views.product_list_orders_by_findings_count(factory.get("/products?o=-findings_count"))) + self.assertTrue(views.product_list_orders_by_findings_count(factory.get("/products?o=name,-findings_count"))) + + @override_settings(V3_FEATURE_LOCATIONS=True) + @patch("dojo.product.ui.views.render") + @patch("dojo.product.ui.views.add_breadcrumb") + @patch("dojo.product.ui.views.get_system_setting", return_value=False) + @patch("dojo.product.ui.views.prefetch_for_product") + @patch("dojo.product.ui.views.get_page_items") + @patch("dojo.product.ui.views.ProductFilter") + @patch("dojo.product.ui.views.annotate_product_findings_count") + @patch("dojo.product.ui.views.get_authorized_products") + def test_product_list_defers_count_annotations_until_after_pagination( + self, + get_authorized_products, + annotate_product_findings_count, + product_filter, + get_page_items, + prefetch_for_product, + get_system_setting, + add_breadcrumb, + render, + ): + base_qs = MagicMock(name="base_qs") + base_qs.values_list.return_value = ["Product A"] + get_authorized_products.return_value = base_qs + + filtered_qs = MagicMock(name="filtered_qs") + distinct_qs = MagicMock(name="distinct_qs") + filtered_qs.distinct.return_value = distinct_qs + product_filter.return_value = SimpleNamespace(qs=filtered_qs) + + page = SimpleNamespace(object_list=distinct_qs) + get_page_items.return_value = page + prefetch_for_product.return_value = ["prefetched-page"] + render.return_value = SimpleNamespace(status_code=200) + + request = RequestFactory().get("/products") + request.user = MagicMock() + + views.product(request) + + get_system_setting.assert_called() + add_breadcrumb.assert_called_once() + annotate_product_findings_count.assert_not_called() + filtered_qs.distinct.assert_called_once_with() + get_page_items.assert_called_once_with(request, distinct_qs, 25) + prefetch_for_product.assert_called_once_with(distinct_qs) + + @patch("dojo.product.ui.views.render") + @patch("dojo.product.ui.views.add_breadcrumb") + @patch("dojo.product.ui.views.get_system_setting", return_value=False) + @patch("dojo.product.ui.views.prefetch_for_product") + @patch("dojo.product.ui.views.get_page_items") + @patch("dojo.product.ui.views.ProductFilter") + @patch("dojo.product.ui.views.annotate_product_findings_count") + @patch("dojo.product.ui.views.get_authorized_products") + def test_product_list_keeps_findings_count_annotation_when_sorting_by_it( + self, + get_authorized_products, + annotate_product_findings_count, + product_filter, + get_page_items, + prefetch_for_product, + get_system_setting, + add_breadcrumb, + render, + ): + base_qs = MagicMock(name="base_qs") + base_qs.values_list.return_value = ["Product A"] + annotated_qs = MagicMock(name="annotated_qs") + annotate_product_findings_count.return_value = annotated_qs + get_authorized_products.return_value = base_qs + + filtered_qs = MagicMock(name="filtered_qs") + distinct_qs = MagicMock(name="distinct_qs") + filtered_qs.distinct.return_value = distinct_qs + product_filter.return_value = SimpleNamespace(qs=filtered_qs) + + page = SimpleNamespace(object_list=distinct_qs) + get_page_items.return_value = page + prefetch_for_product.return_value = ["prefetched-page"] + render.return_value = SimpleNamespace(status_code=200) + + query = QueryDict("o=-findings_count") + request = RequestFactory().get("/products") + request.GET = query + request.user = MagicMock() + + views.product(request) + + get_system_setting.assert_called() + add_breadcrumb.assert_called_once() + annotate_product_findings_count.assert_called_once_with(base_qs) + product_filter.assert_called_once_with(request.GET, queryset=annotated_qs, user=request.user) + filtered_qs.distinct.assert_called_once_with() + get_page_items.assert_called_once_with(request, distinct_qs, 25) From b5fe86e591ece6c6e5ba6c931a364bb25bf41a14 Mon Sep 17 00:00:00 2001 From: Emre Koca <110906681+kocaemre@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:13:51 +0200 Subject: [PATCH 2/2] fix(product): count locations without joins Signed-off-by: Emre Koca <110906681+kocaemre@users.noreply.github.com> --- dojo/filters.py | 2 +- dojo/product/ui/views.py | 49 +++---- unittests/test_product_list_pagination.py | 154 +++++++--------------- 3 files changed, 76 insertions(+), 129 deletions(-) diff --git a/dojo/filters.py b/dojo/filters.py index f43562315e3..f62d2b3e27f 100644 --- a/dojo/filters.py +++ b/dojo/filters.py @@ -510,7 +510,7 @@ def filter_endpoints_host_base(queryset, name, value, statuses=None, endpoint_id if statuses: filters_kwargs["locations__status__in"] = statuses - return queryset.filter(**filters_kwargs) + return queryset.filter(**filters_kwargs).distinct() class FindingTagFilter(DojoFilter): diff --git a/dojo/product/ui/views.py b/dojo/product/ui/views.py index 0ddf6cbb899..c0033524ed8 100644 --- a/dojo/product/ui/views.py +++ b/dojo/product/ui/views.py @@ -14,7 +14,7 @@ from django.contrib.postgres.aggregates import StringAgg from django.core.exceptions import PermissionDenied, ValidationError from django.db import DEFAULT_DB_ALIAS, connection -from django.db.models import Count, DateField, F, OuterRef, Prefetch, Q, Subquery, Sum, Value +from django.db.models import Count, DateField, F, IntegerField, OuterRef, Prefetch, Q, Subquery, Sum, Value from django.db.models.functions import Coalesce from django.db.models.query import QuerySet from django.http import Http404, HttpRequest, HttpResponseRedirect, JsonResponse @@ -61,6 +61,7 @@ ) from dojo.jira import services as jira_services from dojo.labels import get_labels +from dojo.location.models import LocationProductReference from dojo.models import ( App_Analysis, Benchmark_Product_Summary, @@ -131,15 +132,6 @@ labels = get_labels() -def product_list_orders_by_findings_count(request): - order_values = request.GET.getlist("o") - for value in order_values: - for field in value.split(","): - if field.strip().lstrip("-") == "findings_count": - return True - return False - - def annotate_product_findings_count(prods): base_findings = Finding.objects.filter(test__engagement__product_id=OuterRef("pk"), active=True) return prods.annotate( @@ -149,6 +141,26 @@ def annotate_product_findings_count(prods): ) +def annotate_product_location_counts(prods): + location_refs = LocationProductReference.objects.filter(product_id=OuterRef("pk")) + return 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), + ), + ) + + def product(request): prods = get_authorized_products("view") # perform all stuff for filtering and pagination first, before annotation/prefetching @@ -156,16 +168,14 @@ def product(request): # see https://code.djangoproject.com/ticket/23771 and https://code.djangoproject.com/ticket/25375 name_words = prods.values_list("name", flat=True) - if product_list_orders_by_findings_count(request): - prods = annotate_product_findings_count(prods) + prods = annotate_product_findings_count(prods) + if settings.V3_FEATURE_LOCATIONS: + prods = annotate_product_location_counts(prods) filter_string_matching = get_system_setting("filter_string_matching", False) filter_class = ProductFilterWithoutObjectLookups if filter_string_matching else ProductFilter prod_filter = filter_class(request.GET, queryset=prods, user=request.user) - prod_qs = prod_filter.qs - if settings.V3_FEATURE_LOCATIONS: - prod_qs = prod_qs.distinct() - prod_list = get_page_items(request, prod_qs, 25) + prod_list = get_page_items(request, prod_filter.qs, 25) # perform annotation/prefetching by replacing the queryset in the page with an annotated/prefetched queryset. prod_list.object_list = prefetch_for_product(prod_list.object_list) @@ -213,14 +223,7 @@ def prefetch_for_product(prods): count_subquery(base_findings.filter(active=True, verified=True)), Value(0), ), - ).annotate( - findings_count=F("active_finding_count"), ) - if settings.V3_FEATURE_LOCATIONS: - prefetched_prods = prefetched_prods.annotate( - location_host_count=Count("locations__location__url__host", distinct=True), - location_count=Count("locations", distinct=True), - ) prefetched_prods = prefetched_prods.annotate( total_reimport_count=Coalesce( count_subquery( diff --git a/unittests/test_product_list_pagination.py b/unittests/test_product_list_pagination.py index 343565e175f..e7c590a21dd 100644 --- a/unittests/test_product_list_pagination.py +++ b/unittests/test_product_list_pagination.py @@ -1,112 +1,56 @@ -from types import SimpleNamespace -from unittest.mock import MagicMock, patch - -from django.http import QueryDict -from django.test import RequestFactory, SimpleTestCase, override_settings - -from dojo.product.ui import views - - -class ProductListPaginationAnnotationTests(SimpleTestCase): - def test_product_list_ordering_detects_findings_count(self): - factory = RequestFactory() - - self.assertFalse(views.product_list_orders_by_findings_count(factory.get("/products"))) - self.assertFalse(views.product_list_orders_by_findings_count(factory.get("/products?o=name"))) - self.assertTrue(views.product_list_orders_by_findings_count(factory.get("/products?o=-findings_count"))) - self.assertTrue(views.product_list_orders_by_findings_count(factory.get("/products?o=name,-findings_count"))) +from django.test import override_settings + +from dojo.filters import filter_endpoints_host_base +from dojo.location.models import LocationProductReference +from dojo.location.status import ProductLocationStatus +from dojo.models import Product, Product_Type +from dojo.product.ui.views import annotate_product_location_counts +from dojo.url.models import URL +from unittests.dojo_test_case import DojoTestCase, versioned_fixtures + + +@versioned_fixtures +class ProductListPaginationAnnotationTests(DojoTestCase): + fixtures = ["dojo_testdata.json"] + + @staticmethod + def _make_product_with_locations(): + product_type = Product_Type.objects.create(name="Product list location counts") + product = Product.objects.create( + name="Product with duplicate location hosts", + prod_type=product_type, + description="Regression test product", + ) + urls = [ + URL.create_location_from_value("https://duplicate.example/one"), + URL.create_location_from_value("https://duplicate.example/two"), + URL.create_location_from_value("https://other.example/"), + ] + for url in urls: + LocationProductReference.objects.create( + location=url.location, + product=product, + status=ProductLocationStatus.Active, + ) + return product @override_settings(V3_FEATURE_LOCATIONS=True) - @patch("dojo.product.ui.views.render") - @patch("dojo.product.ui.views.add_breadcrumb") - @patch("dojo.product.ui.views.get_system_setting", return_value=False) - @patch("dojo.product.ui.views.prefetch_for_product") - @patch("dojo.product.ui.views.get_page_items") - @patch("dojo.product.ui.views.ProductFilter") - @patch("dojo.product.ui.views.annotate_product_findings_count") - @patch("dojo.product.ui.views.get_authorized_products") - def test_product_list_defers_count_annotations_until_after_pagination( - self, - get_authorized_products, - annotate_product_findings_count, - product_filter, - get_page_items, - prefetch_for_product, - get_system_setting, - add_breadcrumb, - render, - ): - base_qs = MagicMock(name="base_qs") - base_qs.values_list.return_value = ["Product A"] - get_authorized_products.return_value = base_qs - - filtered_qs = MagicMock(name="filtered_qs") - distinct_qs = MagicMock(name="distinct_qs") - filtered_qs.distinct.return_value = distinct_qs - product_filter.return_value = SimpleNamespace(qs=filtered_qs) + def test_location_counts_use_subqueries_without_product_group_by(self): + product = self._make_product_with_locations() - page = SimpleNamespace(object_list=distinct_qs) - get_page_items.return_value = page - prefetch_for_product.return_value = ["prefetched-page"] - render.return_value = SimpleNamespace(status_code=200) + queryset = annotate_product_location_counts(Product.objects.filter(id=product.id)) + sql = str(queryset.query) + annotated = queryset.get() - request = RequestFactory().get("/products") - request.user = MagicMock() + self.assertEqual(annotated.location_count, 3) + self.assertEqual(annotated.location_host_count, 2) + self.assertNotIn('LEFT OUTER JOIN "dojo_locationproductreference"', sql) + self.assertNotIn('GROUP BY "dojo_product"', sql) - views.product(request) - - get_system_setting.assert_called() - add_breadcrumb.assert_called_once() - annotate_product_findings_count.assert_not_called() - filtered_qs.distinct.assert_called_once_with() - get_page_items.assert_called_once_with(request, distinct_qs, 25) - prefetch_for_product.assert_called_once_with(distinct_qs) - - @patch("dojo.product.ui.views.render") - @patch("dojo.product.ui.views.add_breadcrumb") - @patch("dojo.product.ui.views.get_system_setting", return_value=False) - @patch("dojo.product.ui.views.prefetch_for_product") - @patch("dojo.product.ui.views.get_page_items") - @patch("dojo.product.ui.views.ProductFilter") - @patch("dojo.product.ui.views.annotate_product_findings_count") - @patch("dojo.product.ui.views.get_authorized_products") - def test_product_list_keeps_findings_count_annotation_when_sorting_by_it( - self, - get_authorized_products, - annotate_product_findings_count, - product_filter, - get_page_items, - prefetch_for_product, - get_system_setting, - add_breadcrumb, - render, - ): - base_qs = MagicMock(name="base_qs") - base_qs.values_list.return_value = ["Product A"] - annotated_qs = MagicMock(name="annotated_qs") - annotate_product_findings_count.return_value = annotated_qs - get_authorized_products.return_value = base_qs - - filtered_qs = MagicMock(name="filtered_qs") - distinct_qs = MagicMock(name="distinct_qs") - filtered_qs.distinct.return_value = distinct_qs - product_filter.return_value = SimpleNamespace(qs=filtered_qs) - - page = SimpleNamespace(object_list=distinct_qs) - get_page_items.return_value = page - prefetch_for_product.return_value = ["prefetched-page"] - render.return_value = SimpleNamespace(status_code=200) - - query = QueryDict("o=-findings_count") - request = RequestFactory().get("/products") - request.GET = query - request.user = MagicMock() + @override_settings(V3_FEATURE_LOCATIONS=True) + def test_endpoint_host_filter_deduplicates_products(self): + product = self._make_product_with_locations() - views.product(request) + filtered = filter_endpoints_host_base(Product.objects.all(), "endpoints__host", "duplicate.example") - get_system_setting.assert_called() - add_breadcrumb.assert_called_once() - annotate_product_findings_count.assert_called_once_with(base_qs) - product_filter.assert_called_once_with(request.GET, queryset=annotated_qs, user=request.user) - filtered_qs.distinct.assert_called_once_with() - get_page_items.assert_called_once_with(request, distinct_qs, 25) + self.assertEqual(list(filtered.filter(id=product.id).values_list("id", flat=True)), [product.id])