From f1ef46fc9079dd23f038c44726870926c35fb283 Mon Sep 17 00:00:00 2001 From: Jaimin2687 Date: Tue, 22 Sep 2026 17:05:57 +0530 Subject: [PATCH 1/3] perf(api): eliminate N+1 queries in product and asset list endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProductViewSet and AssetViewSet.get_queryset() returned a bare queryset with zero prefetching, so every serialized relation was lazy-loaded per product: tags, product_meta, authorized_users, regulations, and the active-finding count — roughly 6 extra queries per product in the response. Apply the same optimization strategy the Product UI views already use: - select_related for the platform/lifecycle/origin FKs (SlugRelatedField reads .value on the related object) - prefetch_related for tags, product_meta, authorized_users, regulations - annotate active_finding_count via a correlated subquery using the project's own build_count_subquery utility — the Product.findings_count cached_property already checks for this attribute before falling back to a per-product count() The only remaining per-product query is open_findings_list(), which returns a variable-length list of finding IDs and cannot be collapsed into a scalar annotation. This is a known limitation (the model method carries a TODO comment) that requires either a PostgreSQL-specific ArrayAgg or deprecating the findings_list field to resolve. Add a regression test (test_api_product_prefetch) that creates 1 vs 5 products with full relation graphs and asserts the query-count growth equals exactly the number of extra products — proving all N+1 sources except the documented open_findings_list are eliminated. --- dojo/asset/api/views.py | 34 ++++++- dojo/product/api/views.py | 40 +++++++- unittests/test_api_product_prefetch.py | 126 +++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 unittests/test_api_product_prefetch.py diff --git a/dojo/asset/api/views.py b/dojo/asset/api/views.py index c3657582a9b..fe5cae13013 100644 --- a/dojo/asset/api/views.py +++ b/dojo/asset/api/views.py @@ -1,5 +1,8 @@ from datetime import datetime +from functools import partial +from django.db.models import OuterRef, Value +from django.db.models.functions import Coalesce from django_filters.rest_framework import DjangoFilterBackend from drf_spectacular.utils import extend_schema, extend_schema_view from rest_framework import mixins, status, viewsets @@ -18,6 +21,7 @@ ) from dojo.authorization import api_permissions as permissions from dojo.models import ( + Finding, Product, Product_API_Scan_Configuration, ) @@ -25,6 +29,7 @@ get_authorized_product_api_scan_configurations, get_authorized_products, ) +from dojo.query_utils import build_count_subquery from dojo.utils import async_delete, get_setting @@ -72,7 +77,34 @@ class AssetViewSet( ) def get_queryset(self): - return get_authorized_products("view").distinct() + base_findings = Finding.objects.filter( + test__engagement__product_id=OuterRef("pk"), + ) + count_subquery = partial( + build_count_subquery, + group_field="test__engagement__product_id", + ) + return ( + get_authorized_products("view") + .select_related( + "platform", + "lifecycle", + "origin", + ) + .prefetch_related( + "tags", + "product_meta", + "authorized_users", + "regulations", + ) + .annotate( + active_finding_count=Coalesce( + count_subquery(base_findings.filter(active=True)), + Value(0), + ), + ) + .distinct() + ) def destroy(self, request, *args, **kwargs): instance = self.get_object() diff --git a/dojo/product/api/views.py b/dojo/product/api/views.py index 79d04df4375..e4a4900ef63 100644 --- a/dojo/product/api/views.py +++ b/dojo/product/api/views.py @@ -1,5 +1,8 @@ from datetime import datetime +from functools import partial +from django.db.models import OuterRef, Value +from django.db.models.functions import Coalesce from django_filters.rest_framework import DjangoFilterBackend from drf_spectacular.utils import extend_schema, extend_schema_view from rest_framework import mixins, status, viewsets @@ -17,7 +20,7 @@ schema_with_prefetch, ) from dojo.authorization import api_permissions as permissions -from dojo.models import Endpoint, Product, Product_API_Scan_Configuration +from dojo.models import Endpoint, Finding, Product, Product_API_Scan_Configuration from dojo.product.api.filters import ApiProductFilter from dojo.product.api.serializer import ( ProductAPIScanConfigurationSerializer, @@ -27,6 +30,7 @@ get_authorized_product_api_scan_configurations, get_authorized_products, ) +from dojo.query_utils import build_count_subquery from dojo.utils import async_delete, get_setting @@ -81,7 +85,39 @@ class ProductViewSet( ) def get_queryset(self): - return get_authorized_products("view").distinct() + base_findings = Finding.objects.filter( + test__engagement__product_id=OuterRef("pk"), + ) + count_subquery = partial( + build_count_subquery, + group_field="test__engagement__product_id", + ) + return ( + get_authorized_products("view") + .select_related( + # SlugRelatedField reads .value on the related object — without + # select_related each product fires a separate lookup query. + "platform", + "lifecycle", + "origin", + ) + .prefetch_related( + "tags", + "product_meta", + "authorized_users", + "regulations", + ) + .annotate( + # Product.findings_count (a @cached_property) checks for this + # attribute first, so the annotation satisfies it in bulk and the + # per-product fallback query is never reached. + active_finding_count=Coalesce( + count_subquery(base_findings.filter(active=True)), + Value(0), + ), + ) + .distinct() + ) def destroy(self, request, *args, **kwargs): instance = self.get_object() diff --git a/unittests/test_api_product_prefetch.py b/unittests/test_api_product_prefetch.py new file mode 100644 index 00000000000..3838c22e8ba --- /dev/null +++ b/unittests/test_api_product_prefetch.py @@ -0,0 +1,126 @@ +from django.db import connection +from django.test.utils import CaptureQueriesContext +from django.utils.timezone import now +from rest_framework.test import APITestCase + +from dojo.models import ( + Dojo_User, + DojoMeta, + Engagement, + Finding, + Product, + Product_Type, + Regulation, + Test, + Test_Type, +) + + +class TestProductListNPlusOne(APITestCase): + + """ + Regression: the /api/v2/products/ and /api/v3/assets/ list endpoints must + load all serialized relations in bulk so that the query count does not grow + with the number of products in the response. + + Before the fix, each product triggered separate queries for tags, + product_meta, authorized_users, regulations, and the active-finding count + — a classic N+1. The optimized get_queryset() uses select_related for + SlugRelatedField FKs, prefetch_related for M2M/reverse-FK relations, and a + correlated subquery annotation for the finding count. + + Each test asserts that the query count with 1 product is identical to the + query count with 5 products, proving zero per-product query growth. + """ + + @classmethod + def setUpTestData(cls): + cls.user = Dojo_User.objects.create( + username="prodprefetch_user", is_staff=True, is_superuser=True, + ) + cls.prod_type = Product_Type.objects.create(name="ProdPrefetch PT") + cls.test_type = Test_Type.objects.create(name="ProdPrefetch TT") + cls.regulation = Regulation.objects.create( + name="ProdPrefetch Reg", acronym="PPR", category="privacy", + jurisdiction="international", + ) + + def setUp(self): + self.client.force_authenticate(user=self.user) + + def _create_product(self, suffix): + """Create a product with every relation the serializer renders.""" + product = Product.objects.create( + name=f"ProdPrefetch Product {suffix}", + prod_type=self.prod_type, + description="N+1 test product", + ) + product.tags.add("prefetch-tag-a", "prefetch-tag-b") + product.authorized_users.add(self.user) + product.regulations.add(self.regulation) + DojoMeta.objects.create(product=product, name="key", value="val") + + engagement = Engagement.objects.create( + name=f"ProdPrefetch Eng {suffix}", + product=product, + target_start=now(), + target_end=now(), + ) + test = Test.objects.create( + title=f"ProdPrefetch Test {suffix}", + engagement=engagement, + test_type=self.test_type, + target_start=now(), + target_end=now(), + ) + Finding.objects.create( + title=f"ProdPrefetch Finding {suffix}", + test=test, + reporter=self.user, + severity="High", + active=True, + ) + return product + + def _query_count(self, url): + with CaptureQueriesContext(connection) as ctx: + response = self.client.get(url) + self.assertEqual(response.status_code, 200, response.content[:2000]) + return len(ctx.captured_queries) + + def _assert_constant_query_count(self, url): + self._create_product("baseline") + # Warm-up request: fills ContentType cache and other one-time lookups. + self._query_count(url) + with_one = self._query_count(url) + + extra_products = 4 + for i in range(extra_products): + self._create_product(f"extra-{i}") + with_five = self._query_count(url) + + per_product_growth = with_five - with_one + # Product.open_findings_list() fires one query per product — a known + # limitation (see the TODO comment on the method) that requires either + # a PostgreSQL-specific ArrayAgg or an API-breaking field removal to + # resolve. All other relations are bulk-loaded, so the per-product + # cost is exactly 1 query. Anything above that signals a new N+1. + self.assertEqual( + per_product_growth, + extra_products, + f"{url}: expected +{extra_products} queries for {extra_products} " + f"extra products (open_findings_list), got +{per_product_growth}", + ) + + def test_product_list_query_count_constant(self): + self._assert_constant_query_count("/api/v2/products/") + + @classmethod + def _v3_enabled(cls): + from django.conf import settings # noqa: PLC0415 + return getattr(settings, "V3_FEATURE_LOCATIONS", False) + + def test_asset_list_query_count_constant(self): + if not self._v3_enabled(): + self.skipTest("V3_FEATURE_LOCATIONS is disabled") + self._assert_constant_query_count("/api/v3/assets/") From 0b3f2108b8fbfc2260fff042c2ed2a5586d8e329 Mon Sep 17 00:00:00 2001 From: Jaimin2687 Date: Tue, 22 Sep 2026 18:19:41 +0530 Subject: [PATCH 2/3] fix(jira): add deterministic ordering to JIRA querysets to prevent flaky CI tests --- dojo/authorization/query_registrations.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dojo/authorization/query_registrations.py b/dojo/authorization/query_registrations.py index c4927e83cbb..5d2238e042a 100644 --- a/dojo/authorization/query_registrations.py +++ b/dojo/authorization/query_registrations.py @@ -281,14 +281,14 @@ def _get_authorized_jira_projects(permission, user=None): if user is None or getattr(user, "is_anonymous", False): return JIRA_Project.objects.none() if _is_unrestricted(user, permission_to_action(permission)): - return JIRA_Project.objects.all() + return JIRA_Project.objects.all().order_by("id") authorized_products = _authorized_product_ids(user) authorized_product_types = _authorized_product_type_ids(user) return JIRA_Project.objects.filter( Q(product__id__in=authorized_products) | Q(product__prod_type__id__in=authorized_product_types) | Q(engagement__product__id__in=authorized_products), - ).distinct() + ).distinct().order_by("id") register_auth_filter("jira_link.get_authorized_jira_projects", _get_authorized_jira_projects) @@ -299,13 +299,13 @@ def _get_authorized_jira_issues(permission): if user is None or getattr(user, "is_anonymous", False): return JIRA_Issue.objects.none() if _is_unrestricted(user, permission_to_action(permission)): - return JIRA_Issue.objects.all() + return JIRA_Issue.objects.all().order_by("id") authorized_products = _authorized_product_ids(user) return JIRA_Issue.objects.filter( Q(engagement__product__id__in=authorized_products) | Q(finding__test__engagement__product__id__in=authorized_products) | Q(finding_group__test__engagement__product__id__in=authorized_products), - ) + ).order_by("id") register_auth_filter("jira_link.get_authorized_jira_issues", _get_authorized_jira_issues) From 3c5fa984d7fa2d6615a735a4fa9ff2ad066c104e Mon Sep 17 00:00:00 2001 From: Jaimin2687 Date: Tue, 22 Sep 2026 19:28:01 +0530 Subject: [PATCH 3/3] fix(test): correct V3 API testing route, auth, and query expectations --- unittests/test_api_product_prefetch.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/unittests/test_api_product_prefetch.py b/unittests/test_api_product_prefetch.py index 3838c22e8ba..f1d0f6ec4e5 100644 --- a/unittests/test_api_product_prefetch.py +++ b/unittests/test_api_product_prefetch.py @@ -47,6 +47,7 @@ def setUpTestData(cls): def setUp(self): self.client.force_authenticate(user=self.user) + self.client.force_login(self.user) def _create_product(self, suffix): """Create a product with every relation the serializer renders.""" @@ -88,7 +89,7 @@ def _query_count(self, url): self.assertEqual(response.status_code, 200, response.content[:2000]) return len(ctx.captured_queries) - def _assert_constant_query_count(self, url): + def _assert_constant_query_count(self, url, expected_growth=4): self._create_product("baseline") # Warm-up request: fills ContentType cache and other one-time lookups. self._query_count(url) @@ -107,9 +108,9 @@ def _assert_constant_query_count(self, url): # cost is exactly 1 query. Anything above that signals a new N+1. self.assertEqual( per_product_growth, - extra_products, - f"{url}: expected +{extra_products} queries for {extra_products} " - f"extra products (open_findings_list), got +{per_product_growth}", + expected_growth, + f"{url}: expected +{expected_growth} queries for {extra_products} " + f"extra products, got +{per_product_growth}", ) def test_product_list_query_count_constant(self): @@ -123,4 +124,4 @@ def _v3_enabled(cls): def test_asset_list_query_count_constant(self): if not self._v3_enabled(): self.skipTest("V3_FEATURE_LOCATIONS is disabled") - self._assert_constant_query_count("/api/v3/assets/") + self._assert_constant_query_count("/api/v3/assets", expected_growth=0)