Commit 968cc5d
authored
perf(tag inheritance): batch_mode + per-batch bulk during import + reorganize (#14877)
* perf(tags): batch_mode + per-batch bulk inheritance during import (Phase B Stage 2)
Wraps the import / reimport hot loop in `tag_inheritance.batch_mode()` and
bulk-applies inherited Product tags per batch *before* `post_process_findings_batch`
dispatches, so rules / deduplication see inherited tags on `finding.tags`.
Changes:
- `process_findings` (DefaultImporter + DefaultReImporter) now runs its
finding-creation loop inside `batch_mode()`. Per batch, right after
`apply_import_tags_for_batch`, calls a new helper
`apply_inherited_tags_for_findings(batch_findings)` that bulk-syncs
inherited tags on the batch's Findings plus the Endpoints (V2) / Locations
(V3) reachable from them via FK chain. Inheritance is therefore applied to
the persisted children before the post-process task dispatches.
- `inherit_instance_tags` in `dojo/tags_signals.py` now early-returns when
`tag_inheritance.is_in_batch_mode()`, so the batch wrap transparently
suppresses per-row inheritance work for any caller — including
`bulk_create` cleanup paths that invoke it manually. `inherit_tags_on_instance`
post_save delegates to that helper, so the gate also covers signal-driven
fires.
- `EndpointManager.get_or_create_endpoints` replaces its per-row
`inherit_instance_tags(ep)` cleanup loop with a single
`apply_inherited_tags_for_endpoints(created)` bulk call. Inside the importer
the per-batch helper already covers these endpoints via
`Endpoint.status_endpoint.finding`; the bulk call is kept as a defensive
hook for any non-importer caller.
- `propagate_tags_on_product_sync` (used by the product-tag-toggle Celery
task) gains an early-exit when neither system-wide nor per-product
inheritance is enabled, eliminating ~9 wasted reads per call on
inheritance-off products. State transitions (toggling either flag) still
trigger a full sweep through their existing signal handlers.
- `Location` gains `iter_related_products()`: a related-manager
(`self.products` + `self.findings`) implementation of `all_related_products()`
that returns `list[Product]`. Callers that pre-issue
`prefetch_related("products__product__tags",
"findings__finding__test__engagement__product__tags")` get zero extra
queries per Location. The existing JOIN'd `all_related_products()` is kept
for the per-instance signal path where prefetching is not possible.
- `_inherited_tag_names_for_location` (the per-Location callback used to
compute the inherited target set) switches to `iter_related_products()`;
both call sites (`propagate_tags_on_product_sync` V3 branch and
`apply_inherited_tags_for_findings` V3 branch) now prefetch the chain.
Query-count impact on `unittests/test_tag_inheritance_perf.py` (pins updated
in this commit):
| Hot path | Before | After | Δ |
|-----------------------------------------|--------:|-------:|------:|
| ZAP scan import V2 (19 findings) | 1385 | 477 | -908 |
| ZAP scan import V3 | 1263 | 945 | -318 |
| ZAP reimport no-change V2 | 69 | 75 | +6 |
| ZAP reimport no-change V3 | 87 | 102 | +15 |
| Product tag add → 100 locations (V3) | 316 | 125 | -191 |
| Product tag remove → 100 locations (V3) | 266 | 75 | -191 |
Small reimport-no-change regressions are the unavoidable per-batch helper
read cost (2 reads × Finding + 2 reads × Endpoint/Location + 1 product tags
read). Real-work imports drop significantly because per-row
`_manage_inherited_tags` work no longer fires inside the loop.
* refactor(tags): centralize inheritance helpers in dojo/tag_inheritance
Move scattered tag-inheritance logic into dojo/tag_inheritance.py without
changing runtime behavior:
- _manage_inherited_tags relocated from dojo/models.py (replaced by a
lazy-import shim to avoid an import cycle through dojo.utils).
- get_products, inherit_product_tags, get_products_to_inherit_tags_from,
propagate_inheritance, inherit_instance_tags, inherit_linked_instance_tags
relocated from dojo/tags_signals.py; receivers there now delegate.
- propagate_tags_on_product_sync, apply_inherited_tags_for_endpoints,
apply_inherited_tags_for_findings, _sync_inheritance_for_qs and
_inherited_tag_names_for_location relocated from dojo/product/helpers.py;
the Celery wrapper propagate_tags_on_product stays in product/helpers.py.
Backward-compat re-exports preserved at every original import site so
external callers (dojo/location, dojo/importers, unittests, pro/) keep
working unchanged.
Adjusts unittests.test_tag_inheritance mock targets to the new module
path. Bumps EXPECTED_ZAP_IMPORT_V2/V3 baselines (477->470, 945->938) to
the actual query counts; the prior pin was already drifting on this
branch before the move.
* refactor(tags): group tag modules under dojo/tags/
Move the three tag-related top-level modules into a dedicated package:
dojo/tag_inheritance.py -> dojo/tags/inheritance.py
dojo/tag_utils.py -> dojo/tags/utils.py
dojo/tags_signals.py -> dojo/tags/signals.py
Update all internal call sites to the new canonical paths (apps,
models, importers, finding, product/helpers, utils_cascade_delete,
settings, unittests). No backward-compat shims left at the old paths.
Also drops the unused dojo/tag/ stub directory that only contained
stale .pyc files.
No logic change. All tag inheritance and tag bulk unit tests pass.
* rename batch_mode to suppressed
* perf(tags): check cached system_setting before per-product flag
Tag inheritance is usually enabled system-wide first, then optionally
overridden per product. The system setting is cached, so check it first
and short-circuit the related-product walk / per-product attribute read
when it's already True.
Sites updated:
- dojo/tags/inheritance.py: inherit_product_tags,
get_products_to_inherit_tags_from, and the three early-return guards
inside propagate_tags_on_product_sync / apply_inherited_tags_for_*.
- dojo/location/models.py: Location.products_to_inherit_tags_from.
- dojo/importers/location_manager.py: bulk-inherit early-return.
Also picks up the in-flight rename of batch_mode() -> suppress() /
is_in_batch_mode() -> is_suppressed() in the two importers.
Behavior unchanged. All tag inheritance / perf / bulk unit tests pass
(36 + 20 + 29).
* refactor(tags): simplify is_tag_inheritance_enabled, drop linked-instance wrapper
- is_tag_inheritance_enabled now delegates to get_products_to_inherit_tags_from
(no products eligible -> not enabled). Centralizes the "which products
contribute inherited tags" decision in one place; the None-product filter
moves into get_products_to_inherit_tags_from so both call sites get it.
- Drop the inherit_linked_instance_tags wrapper: single signal caller now
inlines tag_inheritance.inherit_instance_tags(instance.location).
- Trim dead backward-compat re-exports from dojo/tags/signals.py; reroute
the test imports to dojo.tags.inheritance directly.
Behavior unchanged. test_tag_inheritance (36) and test_tag_inheritance_perf
(20) pass.
* refactor(tags): merge propagate_inheritance into inherit_instance_tags
Combine the two-step gate (propagate_inheritance returning bool, then
caller writing) into a single inherit_instance_tags() with an optional
force=False kwarg that bypasses the suppress() check. The m2m
make_inherited_tags_sticky receiver now also routes through this single
entry point.
Wrap the inner instance.inherit_tags(tag_list) call in
suppress_tag_inheritance() to short-circuit m2m_changed re-entry. The
re-entrant signal previously did a redundant in-sync check using a
stale get_tag_list() cached value; suppressing it both eliminates the
recursion risk and removes per-row redundant queries:
- test_baseline_zap_scan_import_v2: 470 -> 463
- test_baseline_zap_scan_import_v3: 938 -> 931
- test_create_one_finding_v2/v3: 64 -> 61
- test_create_100_findings_v2/v3: 4024 -> 3724
- test_finding_add_user_tag_v2/v3: 17 -> 16
- test_finding_remove_inherited_v2/v3: 44 -> 40
Also picks up the rename of the context manager
suppress() -> suppress_tag_inheritance(). Unit tests for the early-exit
optimization rewritten against the merged function (still 4 cases).
* refactor(tags): diff-based _sync_inherited_tags, drop per-model wrappers
Rewrite the per-row inheritance primitive as a pure diff:
current_inherited = obj.inherited_tags.all()
target = incoming_inherited_tags
remove (current - target), add (target - current)
+ sticky re-add any target name missing from obj.tags
Drops the "rebuild full tag_list, set() everything" approach and removes
the `existing_tags_hint` parameter entirely. Writes are wrapped in
`suppress_tag_inheritance()` so the m2m_changed signal can't dispatch
make_inherited_tags_sticky back into this function.
Other cleanup:
- Rename `_manage_inherited_tags` -> `_sync_inherited_tags`.
- Drop per-model `inherit_tags(self, existing_tags_hint)` methods on
Endpoint / Engagement / Test / Finding / Location. The signal path
(`inherit_instance_tags`) and `LocationManager._bulk_inherit_tags` now
call `_sync_inherited_tags(instance, incoming)` directly.
- Drop the unused `existing_tags_by_location` dict in
`LocationManager._bulk_inherit_tags` (one fewer through-table read).
- Unit tests rewritten against the diff primitive (4 cases) plus a
no-products early-exit test for `inherit_instance_tags`.
Perf baselines (test_tag_inheritance_perf):
EXPECTED_ZAP_IMPORT_V2: 463 -> 422
EXPECTED_ZAP_IMPORT_V3: 931 -> 809
EXPECTED_ZAP_REIMPORT_NO_CHANGE_V3:102 -> 101
EXPECTED_CREATE_ONE_FINDING_V2/V3: 61 -> 55
EXPECTED_CREATE_100_FINDINGS_V2/V3: 3724 -> 3124 (-600)
EXPECTED_FINDING_REMOVE_INHERITED_V2/V3: 40 -> 18
EXPECTED_FINDING_ADD_USER_TAG_V2/V3: 16 -> 17 (+1)
* add comments
* refactor(tags): consolidate location inheritance, drop dojo/product/helpers.py
- New `apply_inherited_tags_for_locations(locations, *, product)` mirrors the
endpoint/finding helpers. Replaces `LocationManager._bulk_inherit_tags`;
callsite delegates. Drops the redundant outer `suppress_tag_inheritance()`
(writes inside `_sync_inherited_tags` are already wrapped) and the bulk
pre-fetch of existing inherited tags (the through-table primitive does
this read once already).
- `_inherited_tag_names_for_location` now filters contributing products by
their own `enable_product_tag_inheritance` flag. Previously tags from
flag-off products linked to the same Location could leak in via
`propagate_tags_on_product_sync` / `apply_inherited_tags_for_findings`.
- Move `propagate_tags_on_product` Celery task into `dojo.tags.inheritance`
alongside its `_sync` counterpart; delete `dojo/product/helpers.py`. Keep
a `propagate_tags_on_product_deprecated` alias under the old task name so
in-flight tasks complete after upgrade.
- Rename `_sync_inheritance_for_qs` kwarg `target_names_per_child` ->
`target_tag_names_per_child` for clarity.
- Update import sites + perf baseline V3 import: 809 -> 445 queries.
* comments
* comments
* refactor(tags): rename inherit_instance_tags -> auto_inherit_product_tags, move to signals
Renames `inherit_instance_tags` to `auto_inherit_product_tags` and relocates
it from `dojo.tags.inheritance` to `dojo.tags.signals`. The function is the
signal-driven entrypoint that applies a Product's tags to a child instance;
moving it next to the signal receivers (and renaming for clarity) makes the
auto-inheritance flow self-contained.
Of all the inheritance helpers it is the only one that early-returns when
`suppress_tag_inheritance()` is active — the other helpers use the context
manager around their writes for reentrancy. That gate semantic is exclusive
to the signal path, so colocation fits.
Also combines `inherit_tags_on_linked_instance` into `inherit_tags_on_instance`
with sender-based target dispatch and a `created=True` gate. Previously the
ref handler fired on every save, including `set_status` updates that don't
change the Location's related-product set; those now correctly no-op.
Drops the unused `force=` parameter on the function while renaming.
* make tag accumulator mandatory param
* resolve duplicate task name
* perf(tags): pk-based _sync_inheritance_for_ids; skip full-row fetch
Replaces `_sync_inheritance_for_qs(queryset, target_tag_names_per_child=callable)`
with `_sync_inheritance_for_ids(model_class, child_ids, target_tag_names)`.
The previous implementation called `list(queryset)` to materialize every child
as a full model instance just so the bulk helpers (`bulk_add_tag_mapping`,
`bulk_remove_tags_from_instances`) had an `instance.pk` and `instance.__class__`
to work with. For Finding (70+ columns) this dominated wall-clock time on big
products — a real-world product with ~14000 findings took ~22s for a single
`propagate_tags_on_product_sync`.
The new path:
- Accepts an iterable of pks; constant-target callers pass
`values_list("pk", flat=True)` directly, skipping all ORM hydration.
- Builds bare `model_class(pk=pid)` stubs (cached per pk) only for the rows
whose inherited set actually needs to change, not for every row scanned.
- Accepts `target_tag_names` as either a `set[str]` (constant target, hoisted
out of the loop — no per-row function call for the
product → engagement/test/finding/endpoint propagation paths) or a
`Callable[[int], set[str]]` for the Location case, where each row's target
is the per-row union of its linked Products' tags.
- For Locations, callers materialize the prefetched instances into a
`{pk: location}` dict and close over it in the callback — the prefetch
chain still runs once upfront, but the primitive itself only sees pks.
- Adds an `add_map`/`remove_map` skip when `target == old` so rows already
in sync don't even allocate a stub.
Also adds direct query-count baselines for `propagate_tags_on_product_sync`
in `test_tag_inheritance_perf.py` so future regressions on the sweep path
fail loudly (V2 = 9 queries, V3 = 18 queries on a product with 100 findings
plus 100 endpoints or locations).
ZAP baselines drop slightly as a side effect of the early `target == old`
skip (V2 import 422 → 420, V3 import 445 → 444, V2 reimport-no-change 75 →
74, V3 reimport-no-change 101 → 100). The major wall-clock win is invisible
to query counters — it's the avoided Finding ORM hydration on large
products.
* rename
* perf(tags): skip redundant inheritance on no-change reimport
Two gates eliminate ~14 wasted queries on a reimport that creates no new
findings or location refs (the common "scheduled rescan, nothing changed"
path):
1. `DefaultReImporter.process_findings` now tracks newly-created findings
in `new_findings_in_batch` (populated in the else branch of the
matched/unmatched dispatch) and passes ONLY those to
`apply_inherited_tags_for_findings`. Matched/existing findings already
had inheritance applied at their original creation, so re-running the
through-table read + Location prefetch chain on them is pure overhead.
2. `LocationManager._persist_locations` now skips
`apply_inherited_tags_for_locations` when no new `LocationProductReference`
rows were created. New `LocationFindingReference`s only add findings
within `self._product`, so they can't change a Location's Product
membership; only a new product ref can. When `all_product_refs` is empty,
the Location's inherited target set is unchanged and the helper would do
a costly no-op read for nothing.
Net effect on the pinned ZAP reimport-no-change baselines:
- V2: 75 → 69 (matches the pre-Phase-A baseline of 69)
- V3: 101 → 81 (beats the pre-Phase-A baseline of 87)
* test(tags): add reimport-with-new-findings perf baseline
Imports the 10-finding ZAP subset first, then reimports the 19-finding full
report so 9 findings are created during reimport while 10 are matched.
Pins V2 = 169 queries, V3 = 198 queries. Captures the realistic "scheduled
rescan with drift" path that the no-change baseline doesn't exercise.1 parent 80974b3 commit 968cc5d
22 files changed
Lines changed: 942 additions & 591 deletions
File tree
- dojo
- finding
- importers
- location
- product
- settings
- tags
- unittests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
97 | | - | |
| 97 | + | |
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
742 | 742 | | |
743 | 743 | | |
744 | 744 | | |
745 | | - | |
| 745 | + | |
746 | 746 | | |
747 | 747 | | |
748 | 748 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
97 | | - | |
| 97 | + | |
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
36 | | - | |
| 36 | + | |
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
22 | 24 | | |
23 | 25 | | |
24 | 26 | | |
| |||
161 | 163 | | |
162 | 164 | | |
163 | 165 | | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
164 | 179 | | |
165 | 180 | | |
166 | 181 | | |
| |||
266 | 281 | | |
267 | 282 | | |
268 | 283 | | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
269 | 288 | | |
270 | 289 | | |
271 | 290 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
24 | 24 | | |
25 | 25 | | |
26 | 26 | | |
27 | | - | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
28 | 30 | | |
29 | 31 | | |
30 | 32 | | |
| |||
263 | 265 | | |
264 | 266 | | |
265 | 267 | | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
266 | 281 | | |
267 | 282 | | |
268 | 283 | | |
| |||
302 | 317 | | |
303 | 318 | | |
304 | 319 | | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
305 | 325 | | |
306 | 326 | | |
307 | 327 | | |
| |||
384 | 404 | | |
385 | 405 | | |
386 | 406 | | |
| 407 | + | |
| 408 | + | |
387 | 409 | | |
388 | 410 | | |
389 | 411 | | |
| |||
422 | 444 | | |
423 | 445 | | |
424 | 446 | | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
425 | 455 | | |
426 | 456 | | |
427 | 457 | | |
| |||
949 | 979 | | |
950 | 980 | | |
951 | 981 | | |
952 | | - | |
| 982 | + | |
953 | 983 | | |
954 | 984 | | |
955 | 985 | | |
| |||
971 | 1001 | | |
972 | 1002 | | |
973 | 1003 | | |
974 | | - | |
975 | | - | |
976 | | - | |
977 | | - | |
978 | | - | |
979 | | - | |
980 | | - | |
| 1004 | + | |
| 1005 | + | |
981 | 1006 | | |
982 | | - | |
| 1007 | + | |
983 | 1008 | | |
984 | 1009 | | |
985 | 1010 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
| 17 | + | |
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| |||
231 | 231 | | |
232 | 232 | | |
233 | 233 | | |
234 | | - | |
235 | | - | |
236 | | - | |
237 | | - | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
238 | 244 | | |
239 | 245 | | |
240 | 246 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | 12 | | |
14 | 13 | | |
15 | 14 | | |
16 | | - | |
| 15 | + | |
17 | 16 | | |
18 | 17 | | |
19 | | - | |
20 | 18 | | |
21 | 19 | | |
22 | | - | |
23 | | - | |
24 | | - | |
| 20 | + | |
25 | 21 | | |
26 | 22 | | |
27 | 23 | | |
| |||
214 | 210 | | |
215 | 211 | | |
216 | 212 | | |
217 | | - | |
218 | | - | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
219 | 225 | | |
220 | 226 | | |
221 | 227 | | |
| |||
477 | 483 | | |
478 | 484 | | |
479 | 485 | | |
480 | | - | |
481 | | - | |
482 | | - | |
483 | | - | |
484 | | - | |
485 | | - | |
486 | | - | |
487 | | - | |
488 | | - | |
489 | | - | |
490 | | - | |
491 | | - | |
492 | | - | |
493 | | - | |
494 | | - | |
495 | | - | |
496 | | - | |
497 | | - | |
498 | | - | |
499 | | - | |
500 | | - | |
501 | | - | |
502 | | - | |
503 | | - | |
504 | | - | |
505 | | - | |
506 | | - | |
507 | | - | |
508 | | - | |
509 | | - | |
510 | | - | |
511 | | - | |
512 | | - | |
513 | | - | |
514 | | - | |
515 | | - | |
516 | | - | |
517 | | - | |
518 | | - | |
519 | | - | |
520 | | - | |
521 | | - | |
522 | | - | |
523 | | - | |
524 | | - | |
525 | | - | |
526 | | - | |
527 | | - | |
528 | | - | |
529 | | - | |
530 | | - | |
531 | | - | |
532 | | - | |
533 | | - | |
534 | | - | |
535 | | - | |
536 | | - | |
537 | | - | |
538 | | - | |
539 | | - | |
540 | | - | |
541 | | - | |
542 | | - | |
543 | | - | |
544 | | - | |
545 | | - | |
546 | | - | |
547 | | - | |
548 | | - | |
549 | | - | |
550 | | - | |
551 | | - | |
552 | | - | |
553 | | - | |
554 | | - | |
555 | | - | |
556 | | - | |
557 | | - | |
558 | | - | |
559 | | - | |
560 | | - | |
561 | | - | |
562 | | - | |
563 | | - | |
564 | | - | |
565 | | - | |
566 | | - | |
567 | | - | |
568 | | - | |
569 | | - | |
570 | | - | |
571 | | - | |
572 | | - | |
573 | | - | |
574 | | - | |
575 | | - | |
576 | | - | |
577 | | - | |
578 | | - | |
579 | | - | |
580 | | - | |
581 | | - | |
0 commit comments