perf(api): eliminate notes N+1 in finding/engagement/test/risk_acceptance list endpoints

dojo/engagement/api/views.py

@@ -33,6 +33,7 @@
     NoteHistory,
     Notes,
 )
+from dojo.notes.helper import notes_prefetch
 from dojo.product.queries import get_authorized_engagement_presets
 from dojo.risk_acceptance import api as ra_api
 from dojo.utils import (
@@ -77,7 +78,7 @@ def destroy(self, request, *args, **kwargs):
     def get_queryset(self):
         return (
             get_authorized_engagements("view")
-            .prefetch_related("notes", "risk_acceptance", "files")
+            .prefetch_related(notes_prefetch(), "risk_acceptance", "files")
             .distinct()
         )
 

dojo/finding/api/views.py

@@ -69,6 +69,7 @@
     NoteHistory,
     Notes,
 )
+from dojo.notes.helper import notes_prefetch
 from dojo.risk_acceptance import api as ra_api
 from dojo.utils import (
     generate_file_response,
@@ -192,7 +193,7 @@ def get_queryset(self):
                 "locations__location__url",
                 "reviewers",
                 "found_by",
-                "notes",
+                notes_prefetch(),
                 "risk_acceptance_set",
                 "test",
                 "tags",
@@ -216,7 +217,7 @@ def get_queryset(self):
                 "endpoints",
                 "reviewers",
                 "found_by",
-                "notes",
+                notes_prefetch(),
                 "risk_acceptance_set",
                 "test",
                 "tags",

dojo/notes/helper.py

@@ -1,8 +1,28 @@
 import logging
 
+from django.db.models import Prefetch
+
+from dojo.notes.models import NoteHistory, Notes
+
 logger = logging.getLogger(__name__)
 
 
+def notes_prefetch(lookup="notes"):
+    """
+    Prefetch for relations rendered by NoteSerializer.
+
+    NoteSerializer renders author/editor/note_type on each note and
+    current_editor/note_type on each history entry; a flat prefetch of the
+    notes relation leaves those to lazy-load with one query per object (N+1).
+    """
+    return Prefetch(
+        lookup,
+        queryset=Notes.objects.select_related("author", "editor", "note_type").prefetch_related(
+            Prefetch("history", queryset=NoteHistory.objects.select_related("current_editor", "note_type")),
+        ),
+    )
+
+
 def delete_related_notes(obj):
     if not hasattr(obj, "notes"):
         logger.warning(f"Attempted to delete notes from object type {type(obj)} without 'notes' attribute.")

dojo/risk_acceptance/api/views.py

@@ -15,6 +15,7 @@
 from dojo.api_v2.views import PrefetchDojoModelViewSet
 from dojo.authorization import api_permissions as permissions
 from dojo.models import NoteHistory, Notes, Risk_Acceptance
+from dojo.notes.helper import notes_prefetch
 from dojo.risk_acceptance.api.filters import ApiRiskAcceptanceFilter
 from dojo.risk_acceptance.api.serializer import (
     RiskAcceptanceProofSerializer,
@@ -51,7 +52,7 @@ def get_queryset(self):
         return (
             get_authorized_risk_acceptances("edit")
             .prefetch_related(
-                "notes", "engagement_set", "owner", "accepted_findings",
+                notes_prefetch(), "engagement_set", "owner", "accepted_findings",
             )
             .distinct()
         )

dojo/test/api/views.py

@@ -18,6 +18,7 @@
     Test_Import,
     Test_Type,
 )
+from dojo.notes.helper import notes_prefetch
 from dojo.risk_acceptance import api as ra_api
 from dojo.test.api.filters import ApiTestFilter, TestImportAPIFilter
 from dojo.test.api.serializer import (
@@ -58,7 +59,7 @@ def risk_application_model_class(self):
     def get_queryset(self):
         return (
             get_authorized_tests("view")
-            .prefetch_related("notes", "files")
+            .prefetch_related(notes_prefetch(), "files")
             .distinct()
         )
 

unittests/test_api_notes_nplusone.py

@@ -0,0 +1,85 @@
+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,
+    Engagement,
+    Finding,
+    NoteHistory,
+    Notes,
+    Product,
+    Product_Type,
+    Risk_Acceptance,
+    Test,
+    Test_Type,
+)
+
+
+class TestNotesListNPlusOne(APITestCase):
+
+    """
+    Regression: list endpoints that embed NoteSerializer (findings, engagements, tests,
+    risk_acceptance) must load note authors/editors/history in bulk via notes_prefetch().
+    A flat prefetch of the notes relation leaves NoteSerializer to lazy-load author, editor,
+    note_type and history__current_editor per note, so the query count grows with every note.
+    Each test pins the fix by asserting the query count is identical with 1 and with 5 notes.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = Dojo_User.objects.create(username="np1_user", is_staff=True, is_superuser=True)
+        cls.prod_type = Product_Type.objects.create(name="NP1 Product Type")
+        cls.product = Product.objects.create(name="NP1 Product", prod_type=cls.prod_type, description="NP1")
+        cls.engagement = Engagement.objects.create(
+            name="NP1 Engagement", product=cls.product, target_start=now(), target_end=now(),
+        )
+        cls.test_type = Test_Type.objects.create(name="NP1 Test Type")
+        cls.test = Test.objects.create(
+            title="NP1 Test", engagement=cls.engagement, test_type=cls.test_type,
+            target_start=now(), target_end=now(),
+        )
+        cls.finding = Finding.objects.create(title="NP1 Finding", test=cls.test, reporter=cls.user, severity="High")
+        cls.risk_acceptance = Risk_Acceptance.objects.create(name="NP1 RA", owner=cls.user)
+        cls.risk_acceptance.engagement_set.add(cls.engagement)
+
+    def setUp(self):
+        self.client.force_authenticate(user=self.user)
+
+    def _add_note(self, obj):
+        # An edited note with a history entry exercises every relation NoteSerializer renders.
+        note = Notes.objects.create(entry="np1 note", author=self.user, edited=True, editor=self.user)
+        history = NoteHistory.objects.create(data="np1 history", current_editor=self.user)
+        note.history.add(history)
+        obj.notes.add(note)
+
+    def _query_count(self, url):
+        with CaptureQueriesContext(connection) as ctx:
+            response = self.client.get(url)
+        self.assertEqual(response.status_code, 200, response.content[:1000])
+        return len(ctx.captured_queries)
+
+    def _assert_count_independent_of_notes(self, obj, url):
+        self._add_note(obj)
+        self._query_count(url)  # warm-up: fills ContentType and other first-request caches
+        with_one_note = self._query_count(url)
+        for _ in range(4):
+            self._add_note(obj)
+        with_five_notes = self._query_count(url)
+        self.assertEqual(
+            with_one_note, with_five_notes,
+            f"{url} ran {with_five_notes - with_one_note} extra queries for 4 extra notes (N+1 on notes)",
+        )
+
+    def test_finding_list_query_count_independent_of_notes(self):
+        self._assert_count_independent_of_notes(self.finding, f"/api/v2/findings/?id={self.finding.id}")
+
+    def test_engagement_list_query_count_independent_of_notes(self):
+        self._assert_count_independent_of_notes(self.engagement, f"/api/v2/engagements/?id={self.engagement.id}")
+
+    def test_test_list_query_count_independent_of_notes(self):
+        self._assert_count_independent_of_notes(self.test, f"/api/v2/tests/?id={self.test.id}")
+
+    def test_risk_acceptance_list_query_count_independent_of_notes(self):
+        self._assert_count_independent_of_notes(self.risk_acceptance, f"/api/v2/risk_acceptance/?id={self.risk_acceptance.id}")