Skip to content

Commit 75590a9

Browse files
valentijnscholtenMaffoochclaude
authored
feat(cache): L1 singleton settings cache + Celery-scoped authorization caching (#15066)
* feat(cache): shared two-tier (L1+L2) read-through cache for singleton config - new dojo/caching.py: dojo_settings_cache(key, timeout) read-through over an in-process L1 tier (off|thread|process, TTL'd) on top of django.core.cache (L2). Resolves L1 -> L2 -> getter; None is never cached. invalidate_dojo_settings_cache; model_to_cache_dict/cache_dict_to_model (store dicts, not pickled instances). Configurable: DD_SETTINGS_CACHE_ENABLED / DD_CACHE_L1_MODE / DD_CACHE_L1_TTL / DD_CACHE_L2_TTL (-1 disables a tier). - System_Settings read path served through the cache (dict, rebuilt per call); write paths (no_cache=True) unchanged; busted on save via post_save receiver. - disabled in unit tests (compose overrides) so assertNumQueries counts stay deterministic; the cache itself is covered by unittests/test_caching.py. No new dependencies; no behavior change on non-cache paths. * feat(cache): wire shared Redis cache backend for L2 (cross-process invalidation) The L1+L2 singleton cache (dojo/caching.py) needs its L2 tier to be a cross-process store so cache invalidation (e.g. on System_Settings save in uwsgi) propagates to celery workers. Without it Django defaults to per-process LocMemCache, so a worker keeps serving a stale System_Settings (e.g. enable_deduplication=False) and async dedup silently no-ops -> dedupe/ close_old_findings_dedupe/questionnaire integration tests fail. - Add DD_CACHE_URL env; when set, configure django.core.cache RedisCache. - Default DD_CACHE_URL=redis://valkey:6379/1 in compose (uwsgi/celery/beat). - Mount source into the integration celeryworker so it runs the same code as uwsgi (enables local reproduction of worker-side cache behavior). * fix(ci): drop integration celeryworker source mount (media volume race) Sharing defectdojo_media_integration_tests between uwsgi and celeryworker races on mkdir media/threat at startup (file exists), flaking the worker container. The mount was only a local-repro aid and is unnecessary in CI, where the worker runs the image built from PR source. Revert to the original worker definition; the Redis L2 fix is unaffected. * fix(cache): don't hit L2 backend on invalidate when L2 disabled Unit tests run with no Redis (DD_SETTINGS_CACHE_L2_TTL=-1) but inherited DD_CACHE_URL from base compose, so System_Settings post_save invalidation called cache.delete against a nonexistent valkey -> rest-framework setUpClass ConnectionError. invalidate_dojo_settings_cache now skips cache.delete when L2 is disabled (consistent with the read path, which already skips L2). Also blank DD_CACHE_URL in the unit-test overrides so the backend is LocMemCache. Adds test_invalidate_skips_l2_when_disabled. * refactor(cache): slim System_Settings middleware to L1-reset + error banner The decorator (dojo.caching) now owns all System_Settings caching, so the middleware no longer keeps its own per-request thread-local instance. It only: (a) resets the request-scoped L1 tier at request start, (b) surfaces a DB-read error for the banner. Renamed DojoSytemSettingsMiddleware -> DojoSettingsManagerMiddleware (it governs the shared settings cache lifecycle, not just System_Settings). Manager.get() reads straight through the cache. To keep assertNumQueries deterministic without the thread-local, unit tests now run with L1 ON (request/test-scoped) and L2 OFF instead of both disabled: L1 is reset per request (middleware) and per test (dojo_test_case setUp via the new refresh_system_settings_cache helper, replacing middleware.load()). This mirrors the old thread-local's per-request memoization. Rewrote the middleware tests for the new contract (cache via decorator, instance rebuilt per call, L1 reset, error banner). * test: rebaseline query counts for per-task L1 cache reset The L1 settings cache is reset per celery task (DojoAsyncTask), so eager-mode tests now faithfully mirror real async workers: each task re-reads System_Settings (from L2) instead of sharing one request-wide read as the old middleware thread-local did. assertNumQueries baselines shift accordingly: - Single cached reads now save a query (metrics 27->25/41->40, fp_history 7->6). - Bulk create/import where many per-finding tasks run inline read once per task (tag_inheritance create_100 3124->3224; zap import/reimport +3-4; importer performance +2-8). In production these run in workers, off the request path. Counts verified locally against the CI unit-test cache config (L1 on, L2 off). * test: pin L2 off via override_settings on query-count tests The query-count assertions must measure in-process (L1) behavior deterministically, independent of whether a shared L2 (Redis) cache is configured/warm in the run environment. Relying on the unit-test compose env (DD_SETTINGS_CACHE_L2_TTL=-1) left the counts env-dependent (e.g. running via run-unittest in dev mode, or the manage.py-test perf path, would see L2 on). Add an explicit @override_settings(SETTINGS_CACHE_L1_TTL=30, SETTINGS_CACHE_L2_TTL=-1) on the count-asserting classes so they are self-contained. No count change (compose already disabled L2 in CI). * feat(cache): drop L2 (shared) tier for singleton settings cache; L1-only The singleton settings cache (dojo/caching.py) becomes a single in-process L1 tier; the shared django.core.cache (L2) tier is removed. Cross-process/worker freshness is provided entirely by resetting L1 at each request and task boundary (middleware + DojoAsyncTask), so each request/task reads a singleton from the DB at most once and never serves a value cached during a prior request/task. Why no L2 for settings singletons: - The dedupe correctness fix only needs each task to see current config, which the per-task L1 reset already guarantees by re-reading the DB -- a shared L2 + cross-process invalidation is not required for correctness. - L2 added real cost and complexity for little gain on these tiny, hot singletons: a Redis dependency, pickled-vs-dict storage and version-keying, cross-process invalidation, and Redis-down fragility -- while the headline win (avoiding repeated reads within a bulk loop) comes from L1, not L2. - Trade-off: one small indexed DB read per singleton per request/task instead of amortising across processes via Redis. Acceptable for low-cardinality config. Removes SETTINGS_CACHE_L2_TTL and the L2 read/set/delete paths. DD_CACHE_URL and the Redis default django cache backend stay (used by other framework caching). Unit-test overrides drop the now-defunct L2 TTL var. test_caching rewritten to the L1-only contract. * feat(cache): guard against saving cache-derived System_Settings snapshots System_Settings.objects.get() returns a read-only snapshot rebuilt from the in-process (L1) cache; saving it could overwrite concurrent changes with stale field values. Tag such instances and make System_Settings.save() fail loudly instead of silently persisting a stale snapshot. - dojo/caching.py: add READ_ONLY_CACHE_MARKER, ReadOnlyCachedInstanceError, and a read_only= param on cache_dict_to_model (default off, so tuner/features keep saving their rebuilt instances for their read-modify-write flows). - System_Settings.save() raises ReadOnlyCachedInstanceError when the instance is tagged; get_cached_system_settings() tags the snapshot read_only=True. - Migrate get()->modify->save() writer sites (incl. the system_settings() test helper) to objects.get(no_cache=True) for a fresh, saveable row. - Document the singleton manager .get() semantics (ignores filter kwargs, never raises DoesNotExist; data migrations use apps.get_model, so get_or_create is unaffected). Fix stale L1/L2 comments left by the drop-L2 change. - test_caching: cover the save-guard and the read_only flag. * feat(cache): cache authorizations in Celery tasks via cache_for_request_or_task @cache_for_request is a no-op in Celery (no request), so per-user authorization filtering re-runs on every call inside a task -- notably the rules engine, which runs authorized querysets under impersonate(rule.owner). Add a request-or-task read-through cache and move the authorized-queryset/permission getters onto it. - dojo/request_cache: cache_for_request_or_task resolves the store as request cache -> task cache -> no caching. The task cache is a per-thread store active only inside a DojoAsyncTask (begin/end), so management commands / shells that never hit a task boundary do not cache. The key folds get_current_user().pk so results keyed on user=None (resolved downstream) cannot leak between users on a reused worker thread or within a task that impersonates multiple owners. - dojo/celery: DojoAsyncTask installs a fresh task cache before each task and drops it afterwards (finally), bounding staleness to a single task. - Migrate all get_authorized_* / authorized_*_id_set / get_*_permissions getters from @cache_for_request to @cache_for_request_or_task. - test_caching: cover task-scope caching, per-user isolation, and reset. * test(perf): align importer perf query counts to CI-measured values The counts resolved during the rebase onto dev were the branch's pre-rebase values; ~90 commits of upstream drift shifted the actuals by +1..+3 per step. Update TestDojoImporterPerformanceSmall / *SmallLocations expected_num_queries to the values reported by the CI test-performance job. (The auth-getter cache migration does not touch importer paths, so it contributes no delta here.) * feat(cache): add opt-in cache_none to dojo_settings_cache By default a None result is not cached (so the next call retries). Pass cache_none=True for getters where None is a legitimate steady-state answer (e.g. "no default row configured"): it caches the None via a sentinel too, so a missing row costs one DB read per request/task instead of one per call site. Safe only when the getter is signal-invalidated on the row's create/save. * test: correct ZAP import query counts after dev merge (CI-verified) The additive merge estimate for EXPECTED_ZAP_IMPORT_V2/V3 was off by one on the import path; set to the values reported by CI (294/318). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent db27f46 commit 75590a9

34 files changed

Lines changed: 800 additions & 337 deletions

docker-compose.override.unit_tests.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ services:
2222
DD_DATABASE_USER: ${DD_DATABASE_USER:-defectdojo}
2323
DD_DATABASE_PASSWORD: ${DD_DATABASE_PASSWORD:-defectdojo}
2424
DD_CELERY_BROKER_URL: 'sqla+sqlite:///dojo.celerydb.sqlite'
25+
# No Redis/valkey in unit tests -> default django cache is LocMemCache.
26+
DD_CACHE_URL: ''
27+
# In-process singleton cache (dojo/caching.py) stays ON for deterministic
28+
# assertNumQueries counts; reset per request (middleware) and per test.
29+
DD_SETTINGS_CACHE_L1_TTL: '30'
2530
DD_JIRA_EXTRA_ISSUE_TYPES: 'Vulnerability' # Shouldn't trigger a migration error
2631
celerybeat: !reset
2732
celeryworker: !reset

docker-compose.override.unit_tests_cicd.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ services:
2323
DD_CELERY_BROKER_URL: 'sqla+sqlite:///dojo.celerydb.sqlite'
2424
DD_JIRA_EXTRA_ISSUE_TYPES: 'Vulnerability' # Shouldn't trigger a migration error
2525
DD_V3_FEATURE_LOCATIONS: ${DD_V3_FEATURE_LOCATIONS:-False}
26+
# No Redis/valkey in unit tests -> default django cache is LocMemCache.
27+
DD_CACHE_URL: ''
28+
# In-process singleton cache (dojo/caching.py) stays ON: a singleton is read
29+
# once per request/test (deterministic assertNumQueries), reset per request
30+
# (middleware) and per test (dojo_test_case setUp).
31+
DD_SETTINGS_CACHE_L1_TTL: '30'
2632
celerybeat: !reset
2733
celeryworker: !reset
2834
initializer: !reset

docker-compose.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ services:
5050
DD_ALLOWED_HOSTS: "${DD_ALLOWED_HOSTS:-*}"
5151
DD_DATABASE_URL: ${DD_DATABASE_URL:-postgresql://defectdojo:defectdojo@postgres:5432/defectdojo}
5252
DD_CELERY_BROKER_URL: ${DD_CELERY_BROKER_URL:-redis://valkey:6379/0}
53+
DD_CACHE_URL: ${DD_CACHE_URL:-redis://valkey:6379/1}
5354
DD_SECRET_KEY: "${DD_SECRET_KEY:-hhZCp@D28z!n@NED*yB!ROMt+WzsY*iq}"
5455
DD_CREDENTIAL_AES_256_KEY: "${DD_CREDENTIAL_AES_256_KEY:-&91a*agLqesc*0DJ+2*bAbsUZfR*4nLw}"
5556
DD_DATABASE_READINESS_TIMEOUT: "${DD_DATABASE_READINESS_TIMEOUT:-30}"
@@ -71,6 +72,7 @@ services:
7172
environment:
7273
DD_DATABASE_URL: ${DD_DATABASE_URL:-postgresql://defectdojo:defectdojo@postgres:5432/defectdojo}
7374
DD_CELERY_BROKER_URL: ${DD_CELERY_BROKER_URL:-redis://valkey:6379/0}
75+
DD_CACHE_URL: ${DD_CACHE_URL:-redis://valkey:6379/1}
7476
DD_SECRET_KEY: "${DD_SECRET_KEY:-hhZCp@D28z!n@NED*yB!ROMt+WzsY*iq}"
7577
DD_CREDENTIAL_AES_256_KEY: "${DD_CREDENTIAL_AES_256_KEY:-&91a*agLqesc*0DJ+2*bAbsUZfR*4nLw}"
7678
DD_DATABASE_READINESS_TIMEOUT: "${DD_DATABASE_READINESS_TIMEOUT:-30}"
@@ -91,6 +93,7 @@ services:
9193
environment:
9294
DD_DATABASE_URL: ${DD_DATABASE_URL:-postgresql://defectdojo:defectdojo@postgres:5432/defectdojo}
9395
DD_CELERY_BROKER_URL: ${DD_CELERY_BROKER_URL:-redis://valkey:6379/0}
96+
DD_CACHE_URL: ${DD_CACHE_URL:-redis://valkey:6379/1}
9497
DD_SECRET_KEY: "${DD_SECRET_KEY:-hhZCp@D28z!n@NED*yB!ROMt+WzsY*iq}"
9598
DD_CREDENTIAL_AES_256_KEY: "${DD_CREDENTIAL_AES_256_KEY:-&91a*agLqesc*0DJ+2*bAbsUZfR*4nLw}"
9699
DD_DATABASE_READINESS_TIMEOUT: "${DD_DATABASE_READINESS_TIMEOUT:-30}"

dojo/authorization/query_registrations.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
Tool_Product_Settings,
4141
Vulnerability_Id,
4242
)
43-
from dojo.request_cache import cache_for_request
43+
from dojo.request_cache import cache_for_request_or_task
4444

4545

4646
def _resolve_user(user):
@@ -81,7 +81,7 @@ def _authorized_product_type_ids(user):
8181
return Product_Type.objects.filter(authorized_users=user).values("id")
8282

8383

84-
@cache_for_request
84+
@cache_for_request_or_task
8585
def authorized_product_id_set(user_pk):
8686
"""
8787
Frozen set of product ids the user can access via authorized_users
@@ -101,7 +101,7 @@ def authorized_product_id_set(user_pk):
101101
)
102102

103103

104-
@cache_for_request
104+
@cache_for_request_or_task
105105
def authorized_product_type_id_set(user_pk):
106106
"""
107107
Frozen set of product_type ids the user is a direct member of via

dojo/authorization/template_filters.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from dojo.authorization.authorization import user_has_configuration_permission as configuration_permission
44
from dojo.authorization.authorization import user_has_global_permission, user_has_permission
5-
from dojo.request_cache import cache_for_request
5+
from dojo.request_cache import cache_for_request_or_task
66

77

88
def has_object_permission(obj, permission):
@@ -21,7 +21,7 @@ def has_configuration_permission(permission, request):
2121
return configuration_permission(user, permission)
2222

2323

24-
@cache_for_request
24+
@cache_for_request_or_task
2525
def get_user_permissions(user):
2626
return user.user_permissions.all()
2727

@@ -31,7 +31,7 @@ def user_has_configuration_permission_without_group(user, codename):
3131
return any(permission.codename == codename for permission in permissions)
3232

3333

34-
@cache_for_request
34+
@cache_for_request_or_task
3535
def get_group_permissions(group):
3636
return group.permissions.all()
3737

dojo/caching.py

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
"""
2+
In-process read-through cache for global, low-cardinality singleton config.
3+
4+
A single per-thread **L1** tier resolves a getter L1 → DB: a hit wins; a ``None``
5+
result means "not cached, compute it" and is never stored (unless the getter opts
6+
in with ``cache_none=True``, for getters where ``None`` is a legitimate answer).
7+
This is deliberately simple — it is only for global, user-INDEPENDENT,
8+
signal-invalidated singletons (feature flags, system settings, and the like),
9+
never per-user or per-object data.
10+
11+
There is intentionally **no shared/cross-process (L2) tier**: freshness is provided
12+
by resetting L1 at every request and task boundary (middleware + the Celery task
13+
base), so each request/task reads the singleton from the DB at most once and never
14+
serves a value cached during a prior request/task (e.g. a since-changed
15+
``System_Settings``). This keeps the design free of a Redis dependency, pickled
16+
model graphs, and cross-process invalidation — at the cost of one DB read per
17+
singleton per request/task. (The default ``django.core.cache`` backend may still be
18+
Redis for other uses; this module no longer reads or writes it.)
19+
20+
Values are stored as plain dicts/scalars (see ``model_to_cache_dict`` /
21+
``cache_dict_to_model``), never pickled model instances.
22+
23+
Configuration (Django setting, wired from env in ``settings.dist.py``):
24+
25+
* ``SETTINGS_CACHE_L1_TTL`` — per-thread in-process freshness budget in seconds
26+
(``-1`` disables the cache, making the decorator a pass-through). Keep it short.
27+
L1 is reset at each request/task boundary, so it is effectively request/task
28+
scoped.
29+
"""
30+
31+
import threading
32+
import time
33+
from functools import wraps
34+
35+
from django.conf import settings
36+
37+
38+
class _L1Store:
39+
40+
"""
41+
Per-thread in-process store, TTL-stamped. ``get`` returns the value or ``None``
42+
(absent, expired, or L1 disabled via ``SETTINGS_CACHE_L1_TTL`` < 0).
43+
44+
Per-thread (not shared across threads) so it needs no locking, and is reset at
45+
each request/task boundary (see ``reset``) — making it effectively request/task
46+
scoped on a reused worker/uwsgi thread.
47+
"""
48+
49+
def __init__(self):
50+
self._local = threading.local()
51+
52+
def _bucket(self):
53+
bucket = getattr(self._local, "b", None)
54+
if bucket is None:
55+
bucket = self._local.b = {}
56+
return bucket
57+
58+
def get(self, key):
59+
if getattr(settings, "SETTINGS_CACHE_L1_TTL", 30) < 0:
60+
return None
61+
entry = self._bucket().get(key)
62+
if entry is None:
63+
return None
64+
value, expiry = entry
65+
if time.monotonic() >= expiry:
66+
self._bucket().pop(key, None)
67+
return None
68+
return value
69+
70+
def set(self, key, value):
71+
ttl = getattr(settings, "SETTINGS_CACHE_L1_TTL", 30)
72+
if ttl < 0:
73+
return
74+
self._bucket()[key] = (value, time.monotonic() + ttl)
75+
76+
def invalidate(self, key):
77+
# Only this thread; other threads/processes self-heal within the L1 TTL
78+
# (and reset at their next request/task boundary).
79+
self._bucket().pop(key, None)
80+
81+
def reset(self):
82+
# Clear THIS thread's L1 bucket. Called at request/task boundaries so a
83+
# reused worker/uwsgi thread never serves a value cached during a prior
84+
# request or task (e.g. a since-changed System_Settings).
85+
self._bucket().clear()
86+
87+
def clear(self):
88+
# Test helper: drop this thread's entries.
89+
self._local = threading.local()
90+
91+
92+
_L1_STORE = _L1Store()
93+
94+
# Sentinel stored in L1 to represent a cached ``None`` result, so a legitimately
95+
# ``None`` value (e.g. "no default configured") is distinguishable from "absent".
96+
# Only used by getters that opt in via ``cache_none=True``.
97+
_CACHED_NONE = object()
98+
99+
100+
def dojo_settings_cache(*, key: str, cache_none: bool = False):
101+
"""
102+
Read-through in-process (L1) cache for a fixed-key singleton getter.
103+
104+
Resolves L1 → wrapped function. Becomes a pass-through when L1 is disabled
105+
(``SETTINGS_CACHE_L1_TTL=-1``). Freshness across processes comes from resetting
106+
L1 each request/task (see ``reset_l1_cache``), not a shared tier.
107+
108+
By default a ``None`` result is treated as "no value" and is not cached (so the
109+
next call retries) — right for singletons that always exist and where ``None``
110+
means "not yet computed / transient error".
111+
112+
Pass ``cache_none=True`` for getters where ``None`` is a legitimate steady-state
113+
answer (e.g. "no default row configured"). It caches the ``None`` too, so a
114+
missing row costs one DB read per request/task instead of one per call site.
115+
Safe only when the getter is signal-invalidated on the row's create/save (so the
116+
cached ``None`` is dropped the moment a value appears).
117+
"""
118+
119+
def decorator(fn):
120+
@wraps(fn)
121+
def wrapper(*args, **kwargs):
122+
value = _L1_STORE.get(key) # ---- L1 ----
123+
if value is _CACHED_NONE: # cached "no value" (cache_none=True)
124+
return None
125+
if value is not None:
126+
return value
127+
128+
value = fn(*args, **kwargs) # ---- miss: compute ----
129+
if value is not None:
130+
_L1_STORE.set(key, value)
131+
elif cache_none:
132+
_L1_STORE.set(key, _CACHED_NONE)
133+
return value
134+
135+
return wrapper
136+
137+
return decorator
138+
139+
140+
def invalidate_dojo_settings_cache(key: str) -> None:
141+
"""
142+
Drop a cached singleton from L1 (this thread).
143+
144+
With no shared tier, other threads/processes self-heal at their next
145+
request/task boundary (L1 reset), so there is nothing cross-process to drop.
146+
"""
147+
_L1_STORE.invalidate(key)
148+
149+
150+
def reset_l1_cache() -> None:
151+
"""
152+
Reset the current thread's L1 tier.
153+
154+
Call at request/task boundaries (reused worker/uwsgi threads) so the
155+
in-process L1 is effectively request/task-scoped and never serves a value
156+
cached during a prior request or task.
157+
"""
158+
_L1_STORE.reset()
159+
160+
161+
def model_to_cache_dict(instance) -> dict:
162+
"""
163+
Flatten a model instance to a plain dict of concrete field values.
164+
165+
Keyed by ``attname`` (so a relation ``foo`` becomes ``foo_id``) and includes
166+
the primary key. M2M and reverse relations are skipped. Storing this instead
167+
of the model instance keeps the cache free of pickled model graphs; rebuild a
168+
live instance with ``cache_dict_to_model``.
169+
"""
170+
return {f.attname: f.value_from_object(instance) for f in instance._meta.concrete_fields}
171+
172+
173+
# Attribute set on instances rebuilt for read-only cache use (``read_only=True``).
174+
# A guarded model's ``save()`` checks it and refuses to persist a cache-derived
175+
# snapshot (see ``System_Settings.save``). Instance-level, so it never leaks to a
176+
# freshly-fetched (``no_cache=True``) instance, which is a distinct object.
177+
READ_ONLY_CACHE_MARKER = "_dd_read_only_cache_snapshot"
178+
179+
180+
class ReadOnlyCachedInstanceError(RuntimeError):
181+
182+
"""
183+
Raised when code tries to save a model instance rebuilt from the read-through
184+
cache. Such an instance is a point-in-time snapshot; persisting it can clobber
185+
concurrent changes with stale field values. Fetch a fresh DB instance
186+
(e.g. ``System_Settings.objects.get(no_cache=True)``) before saving.
187+
"""
188+
189+
190+
def cache_dict_to_model(model_cls, data: dict, *, read_only: bool = False):
191+
"""
192+
Rebuild an in-memory model instance from a ``model_to_cache_dict`` dict.
193+
194+
For read-only use; callers that persist changes must fetch a fresh DB instance
195+
rather than saving a cache-derived one. Pass ``read_only=True`` to tag the
196+
instance with ``READ_ONLY_CACHE_MARKER`` so a guarded model's ``save()`` fails
197+
loudly instead of silently writing a stale snapshot.
198+
"""
199+
instance = model_cls(**data)
200+
if read_only:
201+
setattr(instance, READ_ONLY_CACHE_MARKER, True)
202+
return instance

dojo/celery.py

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ def __call__(self, *args, **kwargs):
2828
"""
2929
Restore user context in the celery worker via crum.impersonate.
3030
31+
Also resets the request/task-scoped L1 settings cache at the start of every
32+
task (including eager): a prefork worker reuses its thread across tasks, so an
33+
L1 value cached during a prior task (e.g. System_Settings) would otherwise be
34+
served stale even after another process changed and saved it. There is no
35+
shared tier, so a reset task re-reads each singleton once from the DB.
36+
3137
The apply_async method injects ``async_user_id`` into kwargs when a task
3238
is dispatched. Here we pop it, resolve to a user instance, and set it
3339
as the current user in thread-local storage so that all downstream
@@ -39,17 +45,27 @@ def __call__(self, *args, **kwargs):
3945
intact so that callers who already set a user (e.g. via
4046
crum.impersonate in tests or request middleware) are not disrupted.
4147
"""
42-
if "async_user_id" not in kwargs:
43-
return super().__call__(*args, **kwargs)
44-
45-
import crum # noqa: PLC0415
46-
47-
from dojo.models import Dojo_User # noqa: PLC0415 circular import
48-
49-
user_id = kwargs.pop("async_user_id")
50-
user = Dojo_User.objects.filter(pk=user_id).first() if user_id else None
51-
with crum.impersonate(user):
52-
return super().__call__(*args, **kwargs)
48+
from dojo.caching import reset_l1_cache # noqa: PLC0415
49+
from dojo.request_cache import begin_task_cache, end_task_cache # noqa: PLC0415
50+
reset_l1_cache()
51+
# Install a fresh task-scoped request-cache (begin) and drop it afterwards
52+
# (end), so cache_for_request_or_task can memoize within this task without a
53+
# value leaking to the next task on this reused worker thread.
54+
begin_task_cache()
55+
try:
56+
if "async_user_id" not in kwargs:
57+
return super().__call__(*args, **kwargs)
58+
59+
import crum # noqa: PLC0415
60+
61+
from dojo.models import Dojo_User # noqa: PLC0415 circular import
62+
63+
user_id = kwargs.pop("async_user_id")
64+
user = Dojo_User.objects.filter(pk=user_id).first() if user_id else None
65+
with crum.impersonate(user):
66+
return super().__call__(*args, **kwargs)
67+
finally:
68+
end_task_cache()
5369

5470
def apply_async(self, args=None, kwargs=None, **options):
5571
"""Override apply_async to inject user context and track tasks."""

dojo/endpoint/queries.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ def get_auth_filter(key): return None
77
Endpoint,
88
Endpoint_Status,
99
)
10-
from dojo.request_cache import cache_for_request
10+
from dojo.request_cache import cache_for_request_or_task
1111

1212

1313
# Cached: all parameters are hashable, no dynamic queryset filtering
14-
@cache_for_request
14+
@cache_for_request_or_task
1515
def get_authorized_endpoints(permission, user=None):
1616
impl = get_auth_filter("endpoint.get_authorized_endpoints")
1717
if impl:
@@ -27,7 +27,7 @@ def get_authorized_endpoints_for_queryset(permission, queryset, user=None):
2727

2828

2929
# Cached: all parameters are hashable, no dynamic queryset filtering
30-
@cache_for_request
30+
@cache_for_request_or_task
3131
def get_authorized_endpoint_status(permission, user=None):
3232
impl = get_auth_filter("endpoint.get_authorized_endpoint_status")
3333
if impl:

dojo/engagement/queries.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
def get_auth_filter(key): return None
55

66
from dojo.models import Engagement
7-
from dojo.request_cache import cache_for_request
7+
from dojo.request_cache import cache_for_request_or_task
88

99

1010
# Cached: all parameters are hashable, no dynamic queryset filtering
11-
@cache_for_request
11+
@cache_for_request_or_task
1212
def get_authorized_engagements(permission):
1313
impl = get_auth_filter("engagement.get_authorized_engagements")
1414
if impl:

0 commit comments

Comments
 (0)