From bd62203c8a26e2f39daa1ee688ae432b5e7e932e Mon Sep 17 00:00:00 2001 From: Valentijn Scholten Date: Tue, 5 May 2026 21:43:23 +0200 Subject: [PATCH 1/3] perf(tags): bulk-propagate inherited tags + gate child post_save on create Replaces the per-row `.save()` loop in `propagate_tags_on_product_sync` with bulk SQL through the existing tag-utils helpers. For every child model (Engagement/Test/Finding/Endpoint/Location), reads current inherited_tags in one query, computes the per-child diff against the Product's tags, and applies adds/removes via `bulk_add_tag_mapping` and the new `bulk_remove_tags_from_instances` helper. Both `tags` and `inherited_tags` fields are kept in sync. Also gates the per-child `inherit_tags_on_instance` post_save handler on `created=True`. The previous behavior fired on every save (create OR update), repeatedly re-applying inherited tags to children whose tag state had not changed. Sticky enforcement on user-driven tag edits is unchanged (still handled by `make_inherited_tags_sticky` on m2m_changed). Pinned query-count baselines from PR #14811 drop accordingly: product_tag_add -> 100 findings : 4758 -> 91 (~52x fewer queries) product_tag_remove -> 100 findings : 4540 -> 53 (~85x fewer queries) Sticky and child-creation paths are unchanged in this PR. Phase B targets those (centralized inheritance module + drop the duplicate `inherited_tags` TagField). --- dojo/product/helpers.py | 126 ++++++++++++++++++++----- dojo/tag_utils.py | 114 +++++++++++++++++++++- dojo/tags_signals.py | 6 ++ unittests/test_tag_inheritance_perf.py | 33 ++++--- 4 files changed, 242 insertions(+), 37 deletions(-) diff --git a/dojo/product/helpers.py b/dojo/product/helpers.py index cdb0750f317..7bbb2937103 100644 --- a/dojo/product/helpers.py +++ b/dojo/product/helpers.py @@ -1,5 +1,6 @@ import contextlib import logging +from collections import defaultdict from django.conf import settings from django.db.models import Q @@ -7,6 +8,7 @@ from dojo.celery import app from dojo.location.models import Location from dojo.models import Endpoint, Engagement, Finding, Product, Test +from dojo.tag_utils import bulk_add_tag_mapping, bulk_remove_tags_from_instances logger = logging.getLogger(__name__) @@ -19,35 +21,115 @@ def propagate_tags_on_product(product_id, *args, **kwargs): def propagate_tags_on_product_sync(product): - # enagagements + """ + Bulk-apply Product tag changes to all children using through-table SQL. + + Replaces the previous per-row `.save()` loop. For every child model owned + by the product (Engagement, Test, Finding, plus Endpoint or Location + depending on the V3_FEATURE_LOCATIONS flag), reads the existing + `inherited_tags` per child in one query, computes the diff against the + Product's current tags, and applies adds/removes via the bulk tag + helpers. Both `tags` and `inherited_tags` fields are kept in sync. + """ + target_names = {tag.name for tag in product.tags.all()} + logger.debug("Propagating tags from %s to all engagements", product) - propagate_tags_on_object_list(Engagement.objects.filter(product=product)) - # tests + _sync_inheritance_for_qs( + Engagement.objects.filter(product=product), + target_names_per_child=lambda _child: target_names, + ) logger.debug("Propagating tags from %s to all tests", product) - propagate_tags_on_object_list(Test.objects.filter(engagement__product=product)) - # findings + _sync_inheritance_for_qs( + Test.objects.filter(engagement__product=product), + target_names_per_child=lambda _child: target_names, + ) logger.debug("Propagating tags from %s to all findings", product) - propagate_tags_on_object_list(Finding.objects.filter(test__engagement__product=product)) + _sync_inheritance_for_qs( + Finding.objects.filter(test__engagement__product=product), + target_names_per_child=lambda _child: target_names, + ) if settings.V3_FEATURE_LOCATIONS: - # Locations logger.debug("Propagating tags from %s to all locations", product) - propagate_tags_on_object_list( - Location.objects.filter( - # Locations linked directly to a product via LocationProductReference - Q(products__product=product) - # Locations linked indirectly to a product via LocationFindingReference - | Q(findings__finding__test__engagement__product=product), - ).distinct(), + location_qs = Location.objects.filter( + Q(products__product=product) + | Q(findings__finding__test__engagement__product=product), + ).distinct() + # Locations can be linked to multiple products, so the inherited target + # is the union of every related product's tags. Compute per-location. + _sync_inheritance_for_qs( + location_qs, + target_names_per_child=_location_target_names, ) else: - # TODO: Delete this after the move to Locations - # endpoints logger.debug("Propagating tags from %s to all endpoints", product) - propagate_tags_on_object_list(Endpoint.objects.filter(product=product)) + _sync_inheritance_for_qs( + Endpoint.objects.filter(product=product), + target_names_per_child=lambda _child: target_names, + ) + + +def _location_target_names(location): + names: set[str] = set() + for related_product in location.all_related_products(): + if related_product is None: + continue + names.update(tag.name for tag in related_product.tags.all()) + return names + + +def _sync_inheritance_for_qs(queryset, *, target_names_per_child): + """ + Sync inherited_tags + tags for every child in `queryset` to its target tag set. + + target_names_per_child: callable(child) -> set[str]. + + Issues bulk SQL: one through-table read for current inherited_tags, then + bulk add/remove on `tags` and `inherited_tags` fields. + """ + children = list(queryset) + if not children: + return + + model_class = type(children[0]) + inherited_field = model_class._meta.get_field("inherited_tags") + inherited_through = inherited_field.remote_field.through + inherited_tag_model = inherited_field.related_model + + # Resolve through-table FK column for the source side. + source_field_name = None + for field in inherited_through._meta.fields: + if hasattr(field, "remote_field") and field.remote_field and field.remote_field.model == model_class: + source_field_name = field.name + break + + child_ids = [c.pk for c in children] + # One query: pull every (child_id, tag_name) pair from the inherited_tags through table. + existing_pairs = inherited_through.objects.filter( + **{f"{source_field_name}__in": child_ids}, + ).values_list(source_field_name, f"{inherited_tag_model._meta.model_name}__name") + + old_inherited_by_child: dict[int, set[str]] = defaultdict(set) + for child_id, tag_name in existing_pairs: + old_inherited_by_child[child_id].add(tag_name) + + # Compute per-child diff and bucket by tag name. + add_map: dict[str, list] = defaultdict(list) + remove_map: dict[str, list] = defaultdict(list) + for child in children: + target = target_names_per_child(child) + old = old_inherited_by_child.get(child.pk, set()) + for name in target - old: + add_map[name].append(child) + for name in old - target: + remove_map[name].append(child) + # Apply adds. Both `tags` and `inherited_tags` get the same set of new + # inherited names — `_manage_inherited_tags` did the same. + if add_map: + bulk_add_tag_mapping(add_map, tag_field_name="inherited_tags") + bulk_add_tag_mapping(add_map, tag_field_name="tags") -def propagate_tags_on_object_list(object_list): - for obj in object_list: - if obj and obj.id is not None: - logger.debug(f"\tPropagating tags to {type(obj)} - {obj}") - obj.save() + # Apply removes. + for name, instances in remove_map.items(): + bulk_remove_tags_from_instances(name, instances, tag_field_name="inherited_tags") + bulk_remove_tags_from_instances(name, instances, tag_field_name="tags") diff --git a/dojo/tag_utils.py b/dojo/tag_utils.py index 87ed6845961..8b2509e2dab 100644 --- a/dojo/tag_utils.py +++ b/dojo/tag_utils.py @@ -164,6 +164,118 @@ def bulk_add_tags_to_instances(tag_or_tags, instances, tag_field_name: str = "ta return total_created +def bulk_remove_tags_from_instances(tag_or_tags, instances, tag_field_name: str = "tags", batch_size: int | None = None) -> int: + """ + Efficiently remove tag(s) from many model instances. + + Symmetric to ``bulk_add_tags_to_instances``: + + - tag_or_tags: a single string, an iterable of strings or tag objects, or a Tagulous edit string. + - instances: QuerySet or list of model instances of the same class. + - tag_field_name: name of the TagField on the model (default: ``"tags"``). + - Decrements ``tag.count`` for every removed (instance, tag) pair. + - Deletes through-model rows in one DELETE per tag (or batched). + - Clears Django prefetch caches on the input instances so subsequent access reloads from DB. + + Returns the total number of relationships removed across all provided tags. + Tags that do not exist or are not currently associated with any instance are silently skipped. + """ + if batch_size is None: + batch_size = getattr(settings, "TAG_BULK_ADD_BATCH_SIZE", 1000) + + if hasattr(instances, "model"): + instances = list(instances) + + if not instances: + return 0 + + model_class = instances[0].__class__ + + # Mirror the Product safety check from bulk_add_tags_to_instances. Removing + # tags from a Product would normally trigger inheritance propagation via + # m2m_changed signals; this helper bypasses signals, so disallow it. + if model_class is Product: + msg = "bulk_remove_tags_from_instances: Product instances are not supported; use Product.tags.remove() or a propagation-aware helper" + raise ValueError(msg) + + try: + tag_field = model_class._meta.get_field(tag_field_name) + except Exception: + msg = f"Model {model_class.__name__} does not have field '{tag_field_name}'" + raise ValueError(msg) + + if not hasattr(tag_field, "tag_options"): + msg = f"Field '{tag_field_name}' is not a TagField" + raise ValueError(msg) + + tag_model = tag_field.related_model + through_model = tag_field.remote_field.through + + # Normalize input into a list of tag names (mirrors bulk_add_tags_to_instances). + tag_names: list[str] = [] + try: + if isinstance(tag_or_tags, str): + space_delimiter = getattr(tag_field, "tag_options", None).space_delimiter if hasattr(tag_field, "tag_options") else False + tag_names = parse_tags(tag_or_tags, space_delimiter=space_delimiter) + elif isinstance(tag_or_tags, Iterable): + tag_names = [getattr(t, "name", str(t)) for t in tag_or_tags] + else: + tag_names = [str(tag_or_tags)] + except Exception: + tag_names = [str(tag_or_tags)] + + # Resolve through-model FK names dynamically (no hard-coding). + through_fields = {f.name: f for f in through_model._meta.fields} + source_field_name = None + target_field_name = None + for field_name, field in through_fields.items(): + if hasattr(field, "remote_field") and field.remote_field: + if field.remote_field.model == model_class: + source_field_name = field_name + elif field.remote_field.model == tag_model: + target_field_name = field_name + + total_removed = 0 + + for single_tag_name in tag_names: + if not single_tag_name: + continue + + # Resolve the tag — skip silently if it doesn't exist (nothing to remove). + if tag_field.tag_options.case_sensitive: + tag = tag_model.objects.filter(name=single_tag_name).first() + else: + tag = tag_model.objects.filter(name__iexact=single_tag_name).first() + if tag is None: + continue + + for i in range(0, len(instances), batch_size): + batch_instances = instances[i:i + batch_size] + batch_ids = [instance.pk for instance in batch_instances] + + with transaction.atomic(): + # One DELETE per tag-batch. Returns the deleted-row count. + deleted_count, _ = through_model.objects.filter( + **{target_field_name: tag.pk}, + **{f"{source_field_name}__in": batch_ids}, + ).delete() + + if deleted_count: + total_removed += deleted_count + # Decrement the Tagulous-maintained count to avoid drift. + tag_model.objects.filter(pk=tag.pk).update( + count=models.F("count") - deleted_count, + ) + + # Invalidate prefetch caches so callers see the new state. + for instance in batch_instances: + prefetch_cache = getattr(instance, "_prefetched_objects_cache", None) + if prefetch_cache is not None: + prefetch_cache.pop(tag_field_name, None) + + return total_removed + + def bulk_add_tag_mapping( tag_to_instances: dict[str, list], tag_field_name: str = "tags", @@ -410,4 +522,4 @@ def bulk_remove_all_tags(model_class, instance_ids_qs): ) -__all__ = ["bulk_add_tag_mapping", "bulk_add_tags_to_instances", "bulk_apply_parser_tags", "bulk_remove_all_tags"] +__all__ = ["bulk_add_tag_mapping", "bulk_add_tags_to_instances", "bulk_apply_parser_tags", "bulk_remove_all_tags", "bulk_remove_tags_from_instances"] diff --git a/dojo/tags_signals.py b/dojo/tags_signals.py index 0fea7ae8ad5..6fe142f2e55 100644 --- a/dojo/tags_signals.py +++ b/dojo/tags_signals.py @@ -59,6 +59,12 @@ def inherit_linked_instance_tags(instance: LocationFindingReference | LocationPr @receiver(signals.post_save, sender=Finding) @receiver(signals.post_save, sender=Location) def inherit_tags_on_instance(sender, instance, created, **kwargs): + # Only inherit on creation. The previous behavior fired on every save + # (create OR update), repeatedly re-applying inherited tags to children + # whose tag state had not changed. Sticky enforcement on user-driven + # tag edits is handled by `make_inherited_tags_sticky` (m2m_changed). + if not created: + return inherit_instance_tags(instance) diff --git a/unittests/test_tag_inheritance_perf.py b/unittests/test_tag_inheritance_perf.py index 450a3d2ab89..8e9f9a5f792 100644 --- a/unittests/test_tag_inheritance_perf.py +++ b/unittests/test_tag_inheritance_perf.py @@ -357,10 +357,12 @@ def test_baseline_product_tag_remove_propagates_to_100_locations_v3(self): # the appropriate mode so all variants execute in a single suite run. # Findings-only scenarios. - EXPECTED_PRODUCT_TAG_ADD_100_V2 = 4758 - EXPECTED_PRODUCT_TAG_ADD_100_V3 = 4759 - EXPECTED_PRODUCT_TAG_REMOVE_100_V2 = 4540 - EXPECTED_PRODUCT_TAG_REMOVE_100_V3 = 4541 + # Pre-Phase-A V2: 4758 add, 4540 remove. V3: 4759/4541. + # Phase A bulk-propagate drops these dramatically. + EXPECTED_PRODUCT_TAG_ADD_100_V2 = 91 + EXPECTED_PRODUCT_TAG_ADD_100_V3 = 91 + EXPECTED_PRODUCT_TAG_REMOVE_100_V2 = 53 + EXPECTED_PRODUCT_TAG_REMOVE_100_V3 = 53 EXPECTED_CREATE_ONE_FINDING_V2 = 64 EXPECTED_CREATE_ONE_FINDING_V3 = 64 @@ -372,13 +374,13 @@ def test_baseline_product_tag_remove_propagates_to_100_locations_v3(self): EXPECTED_FINDING_REMOVE_INHERITED_V2 = 44 EXPECTED_FINDING_REMOVE_INHERITED_V3 = 44 - # V2 endpoint paths (Endpoints have no V3 counterpart in this class). - EXPECTED_PRODUCT_TAG_ADD_100_ENDPOINTS = 3958 - EXPECTED_PRODUCT_TAG_REMOVE_100_ENDPOINTS = 3740 + # V2 endpoint paths. Pre-Phase-A: 3958 add, 3740 remove. + EXPECTED_PRODUCT_TAG_ADD_100_ENDPOINTS = 91 + EXPECTED_PRODUCT_TAG_REMOVE_100_ENDPOINTS = 53 - # V3 location paths (LocationManager has no V2 counterpart in this class). - EXPECTED_PRODUCT_TAG_ADD_100_LOCATIONS = 4531 - EXPECTED_PRODUCT_TAG_REMOVE_100_LOCATIONS = 4307 + # V3 location paths. Pre-Phase-A: 4532 add, 4307 remove. + EXPECTED_PRODUCT_TAG_ADD_100_LOCATIONS = 316 + EXPECTED_PRODUCT_TAG_REMOVE_100_LOCATIONS = 266 @override_settings( @@ -488,7 +490,10 @@ def test_baseline_zap_scan_reimport_no_change_v3(self): # Pinned baselines per mode. Each test forces its own V3_FEATURE_LOCATIONS # via @override_settings so all four import paths run in a single suite # invocation regardless of the ambient `DD_V3_FEATURE_LOCATIONS` env var. - EXPECTED_ZAP_IMPORT_V2 = 1461 - EXPECTED_ZAP_IMPORT_V3 = 1319 - EXPECTED_ZAP_REIMPORT_NO_CHANGE_V2 = 77 - EXPECTED_ZAP_REIMPORT_NO_CHANGE_V3 = 95 + # Phase A nudges these slightly downward (post_save gated on created=True + # avoids re-running inheritance on no-op finding updates during reimport). + # Pre-Phase-A: 1461/1319 import, 77/95 reimport. + EXPECTED_ZAP_IMPORT_V2 = 1385 + EXPECTED_ZAP_IMPORT_V3 = 1243 + EXPECTED_ZAP_REIMPORT_NO_CHANGE_V2 = 69 + EXPECTED_ZAP_REIMPORT_NO_CHANGE_V3 = 87 From ec02e2e778412c094deccce419f1459aaf32ad91 Mon Sep 17 00:00:00 2001 From: Valentijn Scholten Date: Wed, 6 May 2026 19:09:45 +0200 Subject: [PATCH 2/3] perf(tags): replace process-global signal disconnect with thread-local batch context Adds dojo/tag_inheritance.py with a thread-local batch() context manager and is_in_batch() predicate. While the calling thread is inside a batch, make_inherited_tags_sticky (m2m_changed) early-returns; the calling code takes responsibility for applying inheritance in bulk. Replaces the previous pattern in dojo/importers/location_manager.py:556: disconnected = signals.m2m_changed.disconnect( make_inherited_tags_sticky, sender=Location.tags.through, ) try: ... finally: if disconnected: signals.m2m_changed.connect(...) Signal.disconnect mutates Django's process-global receiver list and is not thread-safe. While disconnected, every thread/greenlet in the same process loses sticky enforcement. Safe only under Celery --pool=prefork and single-threaded gunicorn; broken under --pool=threads|gevent|eventlet, gunicorn --threads >1, or ASGI threadpools. Also fragile on hard process exit mid-import (handler stays disconnected for the rest of the process lifetime). The new batch context lives in threading.local(): each thread has its own depth counter, the signal handler stays globally connected, and the suppression decision is per-thread. No global mutation, no reconnect hazard. This is Phase B Stage 1. Subsequent stages will wrap the broader importer orchestration in batch(), replace the duplicate inherited_tags TagField with a JSON column, and drop _manage_inherited_tags / per-model inherit_tags(). Pinned perf-test note: V3 zap_scan_import baseline rises 1243 -> 1263 (~1.6%). The previous process-global disconnect was narrower in scope (Location.tags.through only); the batch context covers all child-tag through-tables. Net trade is positive given the threading bug fix; full Phase B reductions arrive in later stages. --- dojo/importers/location_manager.py | 21 +++++----- dojo/tag_inheritance.py | 54 ++++++++++++++++++++++++++ dojo/tags_signals.py | 7 ++++ unittests/test_tag_inheritance_perf.py | 6 ++- 4 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 dojo/tag_inheritance.py diff --git a/dojo/importers/location_manager.py b/dojo/importers/location_manager.py index 94999538fc1..935d5159427 100644 --- a/dojo/importers/location_manager.py +++ b/dojo/importers/location_manager.py @@ -7,14 +7,13 @@ from django.core.exceptions import ValidationError from django.db import transaction -from django.db.models import signals from django.utils import timezone +from dojo import tag_inheritance from dojo.importers.base_location_manager import BaseLocationManager from dojo.location.models import AbstractLocation, Location, LocationFindingReference, LocationProductReference from dojo.location.status import FindingLocationStatus, ProductLocationStatus from dojo.models import Product, _manage_inherited_tags -from dojo.tags_signals import make_inherited_tags_sticky from dojo.tools.locations import LocationData from dojo.url.models import URL from dojo.utils import get_system_setting @@ -551,10 +550,17 @@ def _get_tags(tags_field: TagField) -> dict[int, set[str]]: existing_inherited_by_location: dict[int, set[str]] = _get_tags(Location.inherited_tags) existing_tags_by_location: dict[int, set[str]] = _get_tags(Location.tags) - # Perform the bulk updates. First, though, disconnect the make_inherited_tags_sticky signal on Location.tags - # while updating, otherwise each (inherited_)tags.set() will trigger, defeating the purpose of this bulk update. - disconnected = signals.m2m_changed.disconnect(make_inherited_tags_sticky, sender=Location.tags.through) - try: + # Perform the bulk updates inside a `tag_inheritance.batch()` context. + # While the batch is active, signal handlers in `dojo/tags_signals.py` + # short-circuit per-row inheritance work that would otherwise fire on + # every `(inherited_)tags.set()` and defeat the bulk update. + # + # This replaces a previous `signals.m2m_changed.disconnect(...)` / + # `connect(...)` dance which was process-global and therefore unsafe + # under threaded gunicorn / Celery thread pools / ASGI threadpools: + # while disconnected, every thread in the process lost sticky + # enforcement. Thread-local batch state avoids that hazard. + with tag_inheritance.batch(): for location in locations: target_tag_names: set[str] = set() for pid in product_ids_by_location[location.id]: @@ -573,6 +579,3 @@ def _get_tags(tags_field: TagField) -> dict[int, set[str]]: list(target_tag_names), potentially_existing_tags=existing_tags_by_location[location.id], ) - finally: - if disconnected: - signals.m2m_changed.connect(make_inherited_tags_sticky, sender=Location.tags.through) diff --git a/dojo/tag_inheritance.py b/dojo/tag_inheritance.py new file mode 100644 index 00000000000..9f11f8928a5 --- /dev/null +++ b/dojo/tag_inheritance.py @@ -0,0 +1,54 @@ +""" +Tag inheritance — central coordination module. + +Provides a thread-local ``batch()`` context manager that suppresses +per-instance inheritance work driven by ``m2m_changed`` and ``post_save`` +signals. While inside a batch, the signal handlers in +``dojo/tags_signals.py`` early-return; the calling code is responsible for +applying inheritance in bulk (e.g. via the importer's existing +``_bulk_inherit_tags`` path or ``propagate_tags_on_product_sync``). + +This replaces the previous pattern of ``signals.m2m_changed.disconnect(...)`` +in importer hot loops, which was process-global and unsafe under threaded +gunicorn / Celery thread pools / ASGI threadpools (see PR description for +the full rationale). +""" +from __future__ import annotations + +import contextlib +import threading +from contextlib import contextmanager + +_state = threading.local() + + +def is_in_batch() -> bool: + """Return True when the current thread is inside an active ``batch()``.""" + return bool(getattr(_state, "depth", 0)) + + +@contextmanager +def batch(): + """ + Suppress per-instance inheritance signals for the calling thread. + + Usage: + with tag_inheritance.batch(): + # Bulk operations that would otherwise fire `make_inherited_tags_sticky` + # or `inherit_tags_on_instance` per row. + ... + + The context is reentrant; nested ``with`` blocks share the suppression + until the outermost block exits. State lives in ``threading.local()``, + so concurrent threads (and Celery workers in non-prefork pools) are + unaffected by other threads' batches. + """ + _state.depth = getattr(_state, "depth", 0) + 1 + try: + yield + finally: + _state.depth -= 1 + if _state.depth <= 0: + # Clean up the attribute so leak-free thread reuse stays simple. + with contextlib.suppress(AttributeError): + del _state.depth diff --git a/dojo/tags_signals.py b/dojo/tags_signals.py index 6fe142f2e55..5d8d59e7879 100644 --- a/dojo/tags_signals.py +++ b/dojo/tags_signals.py @@ -4,6 +4,7 @@ from django.db.models import signals from django.dispatch import receiver +from dojo import tag_inheritance from dojo.celery_dispatch import dojo_dispatch_task from dojo.location.models import Location, LocationFindingReference, LocationProductReference from dojo.models import Endpoint, Engagement, Finding, Product, Test @@ -32,6 +33,12 @@ def product_tags_post_add_remove(sender, instance, action, **kwargs): @receiver(signals.m2m_changed, sender=Location.tags.through) def make_inherited_tags_sticky(sender, instance, action, **kwargs): """Make sure inherited tags are added back in if they are removed""" + # Inside a `tag_inheritance.batch()` block the caller takes responsibility + # for applying inheritance in bulk; per-row signal work would defeat the + # purpose. This replaces the old `signals.m2m_changed.disconnect(...)` + # pattern, which was process-global and unsafe under threaded workers. + if tag_inheritance.is_in_batch(): + return if action in {"post_add", "post_remove"}: if inherit_product_tags(instance): tag_list = [tag.name for tag in instance.tags.all()] diff --git a/unittests/test_tag_inheritance_perf.py b/unittests/test_tag_inheritance_perf.py index 8e9f9a5f792..2b1a3681a9a 100644 --- a/unittests/test_tag_inheritance_perf.py +++ b/unittests/test_tag_inheritance_perf.py @@ -493,7 +493,11 @@ def test_baseline_zap_scan_reimport_no_change_v3(self): # Phase A nudges these slightly downward (post_save gated on created=True # avoids re-running inheritance on no-op finding updates during reimport). # Pre-Phase-A: 1461/1319 import, 77/95 reimport. + # Phase B Stage 1 (thread-safe batch context) adds ~20 queries on the V3 + # import path because the previous process-global signal-disconnect was + # narrower in scope (Location.tags.through only). Net-positive trade for + # eliminating the threading bug; full Phase B reductions land in Stage 2. EXPECTED_ZAP_IMPORT_V2 = 1385 - EXPECTED_ZAP_IMPORT_V3 = 1243 + EXPECTED_ZAP_IMPORT_V3 = 1263 EXPECTED_ZAP_REIMPORT_NO_CHANGE_V2 = 69 EXPECTED_ZAP_REIMPORT_NO_CHANGE_V3 = 87 From dfa36258705305b09b3f928e81e943de7a001003 Mon Sep 17 00:00:00 2001 From: Valentijn Scholten Date: Fri, 8 May 2026 16:14:22 +0200 Subject: [PATCH 3/3] variable naming --- dojo/importers/location_manager.py | 2 +- dojo/tag_inheritance.py | 4 ++-- dojo/tags_signals.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dojo/importers/location_manager.py b/dojo/importers/location_manager.py index 935d5159427..dc0171db731 100644 --- a/dojo/importers/location_manager.py +++ b/dojo/importers/location_manager.py @@ -560,7 +560,7 @@ def _get_tags(tags_field: TagField) -> dict[int, set[str]]: # under threaded gunicorn / Celery thread pools / ASGI threadpools: # while disconnected, every thread in the process lost sticky # enforcement. Thread-local batch state avoids that hazard. - with tag_inheritance.batch(): + with tag_inheritance.batch_mode(): for location in locations: target_tag_names: set[str] = set() for pid in product_ids_by_location[location.id]: diff --git a/dojo/tag_inheritance.py b/dojo/tag_inheritance.py index 9f11f8928a5..e9d0e98a5fc 100644 --- a/dojo/tag_inheritance.py +++ b/dojo/tag_inheritance.py @@ -22,13 +22,13 @@ _state = threading.local() -def is_in_batch() -> bool: +def is_in_batch_mode() -> bool: """Return True when the current thread is inside an active ``batch()``.""" return bool(getattr(_state, "depth", 0)) @contextmanager -def batch(): +def batch_mode(): """ Suppress per-instance inheritance signals for the calling thread. diff --git a/dojo/tags_signals.py b/dojo/tags_signals.py index 5d8d59e7879..93ac1886930 100644 --- a/dojo/tags_signals.py +++ b/dojo/tags_signals.py @@ -37,7 +37,7 @@ def make_inherited_tags_sticky(sender, instance, action, **kwargs): # for applying inheritance in bulk; per-row signal work would defeat the # purpose. This replaces the old `signals.m2m_changed.disconnect(...)` # pattern, which was process-global and unsafe under threaded workers. - if tag_inheritance.is_in_batch(): + if tag_inheritance.is_in_batch_mode(): return if action in {"post_add", "post_remove"}: if inherit_product_tags(instance):