diff --git a/docker-compose.override.unit_tests.yml b/docker-compose.override.unit_tests.yml index 565ff78d955..ba243c56d60 100644 --- a/docker-compose.override.unit_tests.yml +++ b/docker-compose.override.unit_tests.yml @@ -22,6 +22,11 @@ services: DD_DATABASE_USER: ${DD_DATABASE_USER:-defectdojo} DD_DATABASE_PASSWORD: ${DD_DATABASE_PASSWORD:-defectdojo} DD_CELERY_BROKER_URL: 'sqla+sqlite:///dojo.celerydb.sqlite' + # No Redis/valkey in unit tests -> default django cache is LocMemCache. + DD_CACHE_URL: '' + # In-process singleton cache (dojo/caching.py) stays ON for deterministic + # assertNumQueries counts; reset per request (middleware) and per test. + DD_SETTINGS_CACHE_L1_TTL: '30' DD_JIRA_EXTRA_ISSUE_TYPES: 'Vulnerability' # Shouldn't trigger a migration error celerybeat: !reset celeryworker: !reset diff --git a/docker-compose.override.unit_tests_cicd.yml b/docker-compose.override.unit_tests_cicd.yml index 01be14baa27..511f76ebcdb 100644 --- a/docker-compose.override.unit_tests_cicd.yml +++ b/docker-compose.override.unit_tests_cicd.yml @@ -23,6 +23,12 @@ services: DD_CELERY_BROKER_URL: 'sqla+sqlite:///dojo.celerydb.sqlite' DD_JIRA_EXTRA_ISSUE_TYPES: 'Vulnerability' # Shouldn't trigger a migration error DD_V3_FEATURE_LOCATIONS: ${DD_V3_FEATURE_LOCATIONS:-False} + # No Redis/valkey in unit tests -> default django cache is LocMemCache. + DD_CACHE_URL: '' + # In-process singleton cache (dojo/caching.py) stays ON: a singleton is read + # once per request/test (deterministic assertNumQueries), reset per request + # (middleware) and per test (dojo_test_case setUp). + DD_SETTINGS_CACHE_L1_TTL: '30' celerybeat: !reset celeryworker: !reset initializer: !reset diff --git a/docker-compose.yml b/docker-compose.yml index bfd3ad50743..e749c671700 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -50,6 +50,7 @@ services: DD_ALLOWED_HOSTS: "${DD_ALLOWED_HOSTS:-*}" DD_DATABASE_URL: ${DD_DATABASE_URL:-postgresql://defectdojo:defectdojo@postgres:5432/defectdojo} DD_CELERY_BROKER_URL: ${DD_CELERY_BROKER_URL:-redis://valkey:6379/0} + DD_CACHE_URL: ${DD_CACHE_URL:-redis://valkey:6379/1} DD_SECRET_KEY: "${DD_SECRET_KEY:-hhZCp@D28z!n@NED*yB!ROMt+WzsY*iq}" DD_CREDENTIAL_AES_256_KEY: "${DD_CREDENTIAL_AES_256_KEY:-&91a*agLqesc*0DJ+2*bAbsUZfR*4nLw}" DD_DATABASE_READINESS_TIMEOUT: "${DD_DATABASE_READINESS_TIMEOUT:-30}" @@ -71,6 +72,7 @@ services: environment: DD_DATABASE_URL: ${DD_DATABASE_URL:-postgresql://defectdojo:defectdojo@postgres:5432/defectdojo} DD_CELERY_BROKER_URL: ${DD_CELERY_BROKER_URL:-redis://valkey:6379/0} + DD_CACHE_URL: ${DD_CACHE_URL:-redis://valkey:6379/1} DD_SECRET_KEY: "${DD_SECRET_KEY:-hhZCp@D28z!n@NED*yB!ROMt+WzsY*iq}" DD_CREDENTIAL_AES_256_KEY: "${DD_CREDENTIAL_AES_256_KEY:-&91a*agLqesc*0DJ+2*bAbsUZfR*4nLw}" DD_DATABASE_READINESS_TIMEOUT: "${DD_DATABASE_READINESS_TIMEOUT:-30}" @@ -91,6 +93,7 @@ services: environment: DD_DATABASE_URL: ${DD_DATABASE_URL:-postgresql://defectdojo:defectdojo@postgres:5432/defectdojo} DD_CELERY_BROKER_URL: ${DD_CELERY_BROKER_URL:-redis://valkey:6379/0} + DD_CACHE_URL: ${DD_CACHE_URL:-redis://valkey:6379/1} DD_SECRET_KEY: "${DD_SECRET_KEY:-hhZCp@D28z!n@NED*yB!ROMt+WzsY*iq}" DD_CREDENTIAL_AES_256_KEY: "${DD_CREDENTIAL_AES_256_KEY:-&91a*agLqesc*0DJ+2*bAbsUZfR*4nLw}" DD_DATABASE_READINESS_TIMEOUT: "${DD_DATABASE_READINESS_TIMEOUT:-30}" diff --git a/dojo/caching.py b/dojo/caching.py new file mode 100644 index 00000000000..7bdc2a9fee5 --- /dev/null +++ b/dojo/caching.py @@ -0,0 +1,160 @@ +""" +In-process read-through cache for global, low-cardinality singleton config. + +A single per-thread **L1** tier resolves a getter L1 → DB: a hit wins; a ``None`` +result means "not cached, compute it" and is never stored. This is deliberately +simple — it is only for global, user-INDEPENDENT, signal-invalidated singletons +(feature flags, system settings, and the like), never per-user or per-object data. + +There is intentionally **no shared/cross-process (L2) tier**: freshness is provided +by resetting L1 at every request and task boundary (middleware + the Celery task +base), so each request/task reads the singleton from the DB at most once and never +serves a value cached during a prior request/task (e.g. a since-changed +``System_Settings``). This keeps the design free of a Redis dependency, pickled +model graphs, and cross-process invalidation — at the cost of one DB read per +singleton per request/task. (The default ``django.core.cache`` backend may still be +Redis for other uses; this module no longer reads or writes it.) + +Values are stored as plain dicts/scalars (see ``model_to_cache_dict`` / +``cache_dict_to_model``), never pickled model instances. + +Configuration (Django setting, wired from env in ``settings.dist.py``): + +* ``SETTINGS_CACHE_L1_TTL`` — per-thread in-process freshness budget in seconds + (``-1`` disables the cache, making the decorator a pass-through). Keep it short. + L1 is reset at each request/task boundary, so it is effectively request/task + scoped. +""" + +import threading +import time +from functools import wraps + +from django.conf import settings + + +class _L1Store: + + """ + Per-thread in-process store, TTL-stamped. ``get`` returns the value or ``None`` + (absent, expired, or L1 disabled via ``SETTINGS_CACHE_L1_TTL`` < 0). + + Per-thread (not shared across threads) so it needs no locking, and is reset at + each request/task boundary (see ``reset``) — making it effectively request/task + scoped on a reused worker/uwsgi thread. + """ + + def __init__(self): + self._local = threading.local() + + def _bucket(self): + bucket = getattr(self._local, "b", None) + if bucket is None: + bucket = self._local.b = {} + return bucket + + def get(self, key): + if getattr(settings, "SETTINGS_CACHE_L1_TTL", 30) < 0: + return None + entry = self._bucket().get(key) + if entry is None: + return None + value, expiry = entry + if time.monotonic() >= expiry: + self._bucket().pop(key, None) + return None + return value + + def set(self, key, value): + ttl = getattr(settings, "SETTINGS_CACHE_L1_TTL", 30) + if ttl < 0: + return + self._bucket()[key] = (value, time.monotonic() + ttl) + + def invalidate(self, key): + # Only this thread; other threads/processes self-heal within the L1 TTL + # (and reset at their next request/task boundary). + self._bucket().pop(key, None) + + def reset(self): + # Clear THIS thread's L1 bucket. Called at request/task boundaries so a + # reused worker/uwsgi thread never serves a value cached during a prior + # request or task (e.g. a since-changed System_Settings). + self._bucket().clear() + + def clear(self): + # Test helper: drop this thread's entries. + self._local = threading.local() + + +_L1_STORE = _L1Store() + + +def dojo_settings_cache(*, key: str): + """ + Read-through in-process (L1) cache for a fixed-key singleton getter. + + Resolves L1 → wrapped function. A ``None`` result is treated as "no value" and + is not cached (so the next call retries). Becomes a pass-through when L1 is + disabled (``SETTINGS_CACHE_L1_TTL=-1``). Freshness across processes comes from + resetting L1 each request/task (see ``reset_l1_cache``), not a shared tier. + """ + + def decorator(fn): + @wraps(fn) + def wrapper(*args, **kwargs): + value = _L1_STORE.get(key) # ---- L1 ---- + if value is not None: + return value + + value = fn(*args, **kwargs) # ---- miss: compute ---- + if value is not None: + _L1_STORE.set(key, value) + return value + + return wrapper + + return decorator + + +def invalidate_dojo_settings_cache(key: str) -> None: + """ + Drop a cached singleton from L1 (this thread). + + With no shared tier, other threads/processes self-heal at their next + request/task boundary (L1 reset), so there is nothing cross-process to drop. + """ + _L1_STORE.invalidate(key) + + +def reset_l1_cache() -> None: + """ + Reset the current thread's L1 tier. + + Call at request/task boundaries (reused worker/uwsgi threads) so the + in-process L1 is effectively request/task-scoped and never serves a value + cached during a prior request or task. + """ + _L1_STORE.reset() + + +def model_to_cache_dict(instance) -> dict: + """ + Flatten a model instance to a plain dict of concrete field values. + + Keyed by ``attname`` (so a relation ``foo`` becomes ``foo_id``) and includes + the primary key. M2M and reverse relations are skipped. Storing this instead + of the model instance keeps the cache free of pickled model graphs; rebuild a + live instance with ``cache_dict_to_model``. + """ + return {f.attname: f.value_from_object(instance) for f in instance._meta.concrete_fields} + + +def cache_dict_to_model(model_cls, data: dict): + """ + Rebuild an in-memory model instance from a ``model_to_cache_dict`` dict. + + For read-only use; callers that persist changes must fetch a fresh DB instance + rather than saving a cache-derived one. + """ + return model_cls(**data) diff --git a/dojo/celery.py b/dojo/celery.py index 81dd44095d3..da9aa63619b 100644 --- a/dojo/celery.py +++ b/dojo/celery.py @@ -28,6 +28,12 @@ def __call__(self, *args, **kwargs): """ Restore user context in the celery worker via crum.impersonate. + Also resets the request/task-scoped L1 settings cache at the start of every + task (including eager): a prefork worker reuses its thread across tasks, so an + L1 value cached during a prior task (e.g. System_Settings) would otherwise be + served stale even after another process changed and saved it. The shared L2 + tier is left intact, so a reset task re-reads each singleton once from L2. + The apply_async method injects ``async_user_id`` into kwargs when a task is dispatched. Here we pop it, resolve to a user instance, and set it as the current user in thread-local storage so that all downstream @@ -39,6 +45,9 @@ def __call__(self, *args, **kwargs): intact so that callers who already set a user (e.g. via crum.impersonate in tests or request middleware) are not disrupted. """ + from dojo.caching import reset_l1_cache # noqa: PLC0415 + reset_l1_cache() + if "async_user_id" not in kwargs: return super().__call__(*args, **kwargs) diff --git a/dojo/middleware.py b/dojo/middleware.py index a576244312a..5516b672b18 100644 --- a/dojo/middleware.py +++ b/dojo/middleware.py @@ -8,16 +8,59 @@ import pghistory.middleware from django.conf import settings from django.db import models +from django.dispatch import receiver from django.http import HttpResponseRedirect from django.urls import reverse from watson.middleware import SearchContextMiddleware from watson.search import search_context_manager +from dojo.caching import ( + cache_dict_to_model, + dojo_settings_cache, + invalidate_dojo_settings_cache, + model_to_cache_dict, + reset_l1_cache, +) from dojo.models import Dojo_User from dojo.product_announcements import LongRunningRequestProductAnnouncement logger = logging.getLogger(__name__) +# Two-tier (in-process L1 + django.core.cache L2) read-through cache for the +# System_Settings singleton, via the shared dojo_settings_cache decorator. Stored +# as a plain dict (no pickled model graph) and rebuilt into a read-only instance +# per call. Write paths use ``objects.get(no_cache=True)`` and are unaffected. +SYSTEM_SETTINGS_CACHE_KEY = "dojo.system_settings.singleton" + + +@dojo_settings_cache(key=SYSTEM_SETTINGS_CACHE_KEY) +def _cached_system_settings_dict(): + from dojo.models import System_Settings # noqa: PLC0415 circular import + settings_obj = System_Settings.objects.get_from_db() + # ``get_from_db`` returns an unsaved defaults instance (pk None) when the row + # can't be read; returning None keeps that out of the cache so the next call + # retries the DB instead of serving stale defaults. + if settings_obj.pk is None: + return None + return model_to_cache_dict(settings_obj) + + +def get_cached_system_settings(): + from dojo.models import System_Settings # noqa: PLC0415 circular import + data = _cached_system_settings_dict() + if not isinstance(data, dict): + return System_Settings() + return cache_dict_to_model(System_Settings, data) + + +@receiver(models.signals.post_save, sender="dojo.System_Settings") +def _invalidate_system_settings_cache(*args, **kwargs): + # Connected at import time (string sender avoids the circular import) so the + # bust fires in requests, Celery, commands and tests -- not only when a + # middleware instance is constructed. + invalidate_dojo_settings_cache(SYSTEM_SETTINGS_CACHE_KEY) + + EXEMPT_URLS = [re.compile(settings.LOGIN_URL.lstrip("/"))] if hasattr(settings, "LOGIN_EXEMPT_URLS"): EXEMPT_URLS += [re.compile(expr) for expr in settings.LOGIN_EXEMPT_URLS] @@ -74,53 +117,31 @@ def __call__(self, request): return response -class DojoSytemSettingsMiddleware: +class DojoSettingsManagerMiddleware: + # Caching of the System_Settings singleton lives in dojo.caching (L1+L2). This + # middleware only (a) resets the request-scoped L1 tier and (b) surfaces a + # System_Settings DB-read error as a banner. The thread-local carries just that + # error message (set by System_Settings_Manager.get_from_db). _thread_local = local() def __init__(self, get_response): self.get_response = get_response - from dojo.models import System_Settings # noqa: PLC0415 circular import - # Use classmethod directly to avoid keeping reference to middleware instance - models.signals.post_save.connect(DojoSytemSettingsMiddleware.cleanup, sender=System_Settings) def __call__(self, request): - self.load() - try: - # Store error in request for context processor to display - # (We can't use messages here because MessageMiddleware runs after this middleware) - if hasattr(self._thread_local, "system_settings_error"): - request.system_settings_error = self._thread_local.system_settings_error - # Clear from thread-local after copying to request - delattr(self._thread_local, "system_settings_error") - return self.get_response(request) - finally: - # ensure cleanup happens even if an exception occurs - self.cleanup() - - def process_exception(self, request, exception): - self.cleanup() - - @classmethod - def get_system_settings(cls): - if hasattr(cls._thread_local, "system_settings"): - return cls._thread_local.system_settings - return None - - @classmethod - def cleanup(cls, *args, **kwargs): # noqa: ARG003 - if hasattr(cls._thread_local, "system_settings"): - del cls._thread_local.system_settings - if hasattr(cls._thread_local, "system_settings_error"): - delattr(cls._thread_local, "system_settings_error") - - @classmethod - def load(cls): - # cleanup any existing settings first to ensure fresh state - cls.cleanup() - from dojo.models import System_Settings # noqa: PLC0415 circular import - system_settings = System_Settings.objects.get(no_cache=True) - cls._thread_local.system_settings = system_settings - return system_settings + # uwsgi/gunicorn reuse threads across requests, so the in-process L1 cache + # (threading.local) would otherwise persist between requests. Reset it at + # the start of each request so cached singletons are request-scoped. + reset_l1_cache() + # Drop any error left on this reused thread by a previous request. + if hasattr(self._thread_local, "system_settings_error"): + delattr(self._thread_local, "system_settings_error") + # Warm the cache once; this also captures any DB-read error (via + # get_from_db) so the context processor can display it as a banner. + # (We can't use messages here because MessageMiddleware runs after this.) + get_cached_system_settings() + if hasattr(self._thread_local, "system_settings_error"): + request.system_settings_error = self._thread_local.system_settings_error + return self.get_response(request) class System_Settings_Manager(models.Manager): @@ -133,11 +154,11 @@ def get_from_db(self, *args, **kwargs): except Exception as e: # Store error message in thread-local for middleware to display error_msg = str(e) - if hasattr(DojoSytemSettingsMiddleware._thread_local, "system_settings_error"): + if hasattr(DojoSettingsManagerMiddleware._thread_local, "system_settings_error"): # Only store the first error to avoid duplicates pass else: - DojoSytemSettingsMiddleware._thread_local.system_settings_error = error_msg + DojoSettingsManagerMiddleware._thread_local.system_settings_error = error_msg # Return defaults so app can still start - error will be displayed as warning message # logger.debug('unable to get system_settings from database, returning defaults. Exception was:', exc_info=True) return System_Settings() @@ -145,16 +166,12 @@ def get_from_db(self, *args, **kwargs): def get(self, no_cache=False, *args, **kwargs): # noqa: FBT002 - this is bit hard to fix nice have this universally fixed if no_cache: - # logger.debug('no_cache specified or cached value found, loading system settings from db') + # logger.debug('no_cache specified, loading system settings from db') return self.get_from_db(*args, **kwargs) - - from_cache = DojoSytemSettingsMiddleware.get_system_settings() - - if not from_cache: - # logger.debug('no cached value found, loading system settings from db') - return self.get_from_db(*args, **kwargs) - - return from_cache + # Read through the shared L1/L2 cache (dojo.caching). L1 is request/task + # scoped (reset by the middleware and the Celery task base), so repeated + # reads within a request are served in-process. + return get_cached_system_settings() class APITrailingSlashMiddleware: diff --git a/dojo/settings/settings.dist.py b/dojo/settings/settings.dist.py index 2d92588ad70..ced429d3b46 100644 --- a/dojo/settings/settings.dist.py +++ b/dojo/settings/settings.dist.py @@ -266,6 +266,15 @@ DD_V3_FEATURE_LOCATIONS=(bool, True), # Dictates if v3 org/asset relabeling (+url routing) will be enabled (on by default as of 3.0.0; set to False to restore Product/Product Type labels and URLs) DD_ENABLE_V3_ORGANIZATION_ASSET_RELABEL=(bool, True), + # Shared cache backend (django.core.cache). When set, Django uses RedisCache + # (e.g. redis://valkey:6379/1); when empty it falls back to LocMemCache. Used + # by general framework caching; the singleton settings cache (dojo/caching.py) + # is in-process only and does not read or write this backend. + DD_CACHE_URL=(str, ""), + # In-process (L1) read-through cache for global singleton getters (see + # dojo/caching.py). Per-thread freshness budget in seconds; -1 disables it. + # Reset every request/task, so each request/task reads the singleton once. + DD_SETTINGS_CACHE_L1_TTL=(int, 30), # Notification env-vars (SLA notify, alert refresh/counter/cap, system-level trump). Defined in dojo.notifications.settings. **NOTIFICATIONS_ENV_DEFAULTS, ) @@ -314,6 +323,20 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param # Raises django's ImproperlyConfigured exception if SECRET_KEY not in os.environ SECRET_KEY = env("DD_SECRET_KEY") +# Default cache backend (django.core.cache). Redis when DD_CACHE_URL is set, +# else per-process LocMemCache. General framework caching only; the singleton +# settings cache (dojo/caching.py) is in-process and does not use this backend. +if env("DD_CACHE_URL"): + CACHES = { + "default": { + "BACKEND": "django.core.cache.backends.redis.RedisCache", + "LOCATION": env("DD_CACHE_URL"), + }, + } + +# In-process singleton cache (dojo/caching.py) +SETTINGS_CACHE_L1_TTL = env("DD_SETTINGS_CACHE_L1_TTL") + # Local time zone for this installation. Choices can be found here: # http://en.wikipedia.org/wiki/List_of_tz_zones_by_name # although not all choices may be available on all operating systems. @@ -791,7 +814,7 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param DJANGO_MIDDLEWARE_CLASSES = [ "django.middleware.common.CommonMiddleware", "dojo.middleware.APITrailingSlashMiddleware", - "dojo.middleware.DojoSytemSettingsMiddleware", + "dojo.middleware.DojoSettingsManagerMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "django.middleware.security.SecurityMiddleware", diff --git a/unittests/dojo_test_case.py b/unittests/dojo_test_case.py index 9f8cf55f89f..9e4db142715 100644 --- a/unittests/dojo_test_case.py +++ b/unittests/dojo_test_case.py @@ -16,12 +16,13 @@ from rest_framework.test import APIClient, APITestCase from vcr_unittest import VCRTestCase +from dojo.caching import invalidate_dojo_settings_cache from dojo.importers.location_manager import LocationManager from dojo.jira import helper as jira_helper from dojo.jira.views import get_custom_field from dojo.location.models import Location, LocationFindingReference from dojo.location.status import FindingLocationStatus -from dojo.middleware import DojoSytemSettingsMiddleware +from dojo.middleware import SYSTEM_SETTINGS_CACHE_KEY, get_cached_system_settings from dojo.models import ( SEVERITIES, DojoMeta, @@ -44,6 +45,17 @@ logger = logging.getLogger(__name__) +def refresh_system_settings_cache(): + """ + Make the next ``System_Settings.objects.get()`` reflect DB changes made in a + test. Tests mutate settings via ``.update()`` (which fires no post_save) so + the L1/L2 cache isn't auto-busted; drop it and re-warm so a subsequent cached + read is served in-process. Replaces the old middleware ``load()``. + """ + invalidate_dojo_settings_cache(SYSTEM_SETTINGS_CACHE_KEY) + get_cached_system_settings() + + def get_unit_tests_path(): return Path(__file__).parent @@ -61,14 +73,14 @@ def wrapper(*args, **kwargs): # Set the flag to the specified value System_Settings.objects.update(**{flag_name: value}) # Reinitialize middleware with updated settings as this doesn't happen automatically during django tests - DojoSytemSettingsMiddleware.load() + refresh_system_settings_cache() try: return test_func(*args, **kwargs) finally: # Reset the flag to its original state after the test System_Settings.objects.update(**{flag_name: not value}) # Reinitialize middleware with updated settings as this doesn't happen automatically during django tests - DojoSytemSettingsMiddleware.load() + refresh_system_settings_cache() return wrapper return decorator @@ -84,14 +96,14 @@ def wrapper(*args, **kwargs): # Set the flag to the specified value System_Settings.objects.update(**{field: value}) # Reinitialize middleware with updated settings as this doesn't happen automatically during django tests - DojoSytemSettingsMiddleware.load() + refresh_system_settings_cache() try: return test_func(*args, **kwargs) finally: # Reset the flag to its original state after the test System_Settings.objects.update(**{field: old_value}) # Reinitialize middleware with updated settings as this doesn't happen automatically during django tests - DojoSytemSettingsMiddleware.load() + refresh_system_settings_cache() return wrapper @@ -131,7 +143,7 @@ def system_settings(self, **kwargs): setattr(ss, key, value) ss.save() # Refresh the cache with the new settings - DojoSytemSettingsMiddleware.load() + refresh_system_settings_cache() def create_product_type(self, name, *args, description="dummy description", **kwargs): product_type = Product_Type(name=name, description=description) @@ -566,7 +578,7 @@ def __init__(self, *args, **kwargs): def setUp(self): super().setUp() # Initialize middleware with fresh settings from db - DojoSytemSettingsMiddleware.load() + refresh_system_settings_cache() def common_check_finding(self, finding): self.assertIn(finding.severity, SEVERITIES) diff --git a/unittests/test_caching.py b/unittests/test_caching.py new file mode 100644 index 00000000000..c39dec90fb4 --- /dev/null +++ b/unittests/test_caching.py @@ -0,0 +1,85 @@ +from django.test import TestCase, override_settings + +from dojo.caching import ( + _L1_STORE, # noqa: PLC2701 test needs the in-process store + cache_dict_to_model, + dojo_settings_cache, + invalidate_dojo_settings_cache, + model_to_cache_dict, +) +from dojo.models import System_Settings + +from .dojo_test_case import DojoTestCase + + +@override_settings(SETTINGS_CACHE_L1_TTL=30) +class DojoSettingsCacheTest(TestCase): + + """ + Unit tests for the in-process (L1) read-through decorator (dojo/caching.py). + + There is no shared/L2 tier: the decorator only memoizes in-process and relies + on L1 reset at request/task boundaries for cross-process freshness. + """ + + def setUp(self): + _L1_STORE.clear() + + def tearDown(self): + _L1_STORE.clear() + + def _build_getter(self, *, key="k", returns=1): + calls = {"n": 0} + + @dojo_settings_cache(key=key) + def getter(): + calls["n"] += 1 + return returns + + return getter, calls + + def test_l1_memoizes_in_process(self): + getter, calls = self._build_getter() + getter() + getter() + getter() + self.assertEqual(calls["n"], 1) # computed once, then served from L1 + + @override_settings(SETTINGS_CACHE_L1_TTL=-1) + def test_l1_disabled_is_passthrough(self): + getter, calls = self._build_getter() + getter() + getter() + self.assertEqual(calls["n"], 2) # recomputed every call when L1 off + + def test_reset_recomputes(self): + getter, calls = self._build_getter() + getter() + _L1_STORE.reset() + getter() + self.assertEqual(calls["n"], 2) # reset drops L1, so it recomputes + + def test_none_result_is_not_cached(self): + getter, calls = self._build_getter(returns=None) + self.assertIsNone(getter()) + self.assertIsNone(getter()) + self.assertEqual(calls["n"], 2) # None recomputed each call (never stored) + + def test_invalidate_recomputes(self): + getter, calls = self._build_getter(returns=7) + self.assertEqual(getter(), 7) + invalidate_dojo_settings_cache("k") + getter() + self.assertEqual(calls["n"], 2) # recomputed after invalidation + + +class ModelDictRoundTripTest(DojoTestCase): + + def test_round_trip_preserves_field_values(self): + settings_obj = System_Settings.objects.get(no_cache=True) + data = model_to_cache_dict(settings_obj) + self.assertIsInstance(data, dict) + self.assertEqual(data["id"], settings_obj.pk) + rebuilt = cache_dict_to_model(System_Settings, data) + self.assertEqual(rebuilt.pk, settings_obj.pk) + self.assertEqual(rebuilt.enable_deduplication, settings_obj.enable_deduplication) diff --git a/unittests/test_false_positive_history_logic.py b/unittests/test_false_positive_history_logic.py index 8748239bedd..9ca6b8a91c1 100644 --- a/unittests/test_false_positive_history_logic.py +++ b/unittests/test_false_positive_history_logic.py @@ -4,6 +4,7 @@ from crum import impersonate from django.conf import settings +from django.test import override_settings from dojo.finding.deduplication import do_false_positive_history_batch from dojo.finding.views import EditFinding @@ -126,6 +127,7 @@ @versioned_fixtures +@override_settings(SETTINGS_CACHE_L1_TTL=30, SETTINGS_CACHE_L2_TTL=-1) class TestFalsePositiveHistoryLogic(DojoTestCase): fixtures = ["dojo_testdata.json"] @@ -1684,7 +1686,7 @@ def test_fp_history_batch_issues_single_candidate_query(self): # 4 lazy-load chain: findings[0].test / .engagement / .product / .test_type # 1 candidates SELECT (with .only()) # 1 bulk UPDATE - with self.assertNumQueries(7): + with self.assertNumQueries(6): do_false_positive_history_batch(batch) # One candidate-fetch call for the whole batch — not one per finding. self.assertEqual(mock_fetch.call_count, 1, "Expected exactly one call to _fetch_fp_candidates_for_batch") @@ -1713,7 +1715,7 @@ def test_fp_history_batch_retroactive_marks_existing_active_fp(self): # 4 lazy-load chain: findings[0].test / .engagement / .product / .test_type # 1 candidates SELECT (with .only()) # 1 bulk UPDATE - with self.assertNumQueries(7): + with self.assertNumQueries(6): do_false_positive_history_batch(batch) # The pre-existing active finding must now be retroactively marked FP. @@ -1721,9 +1723,9 @@ def test_fp_history_batch_retroactive_marks_existing_active_fp(self): def test_fp_history_batch_query_count_does_not_grow_with_affected_findings(self): """ - Query count must stay flat (7) no matter how many findings are retroactively marked. + Query count must stay flat (6) no matter how many findings are retroactively marked. - With the old per-finding approach this would have been 7 + N queries where N is the + With the old per-finding approach this would have been 6 + N queries where N is the number of pre-existing findings that get marked as FP. With the batch approach it is always 7: System_Settings, 4 lazy-load chain, candidates SELECT, one bulk UPDATE. """ @@ -1748,7 +1750,7 @@ def test_fp_history_batch_query_count_does_not_grow_with_affected_findings(self) # 4 lazy-load chain: findings[0].test / .engagement / .product / .test_type # 1 candidates SELECT (with .only()) # 1 bulk UPDATE covering all retroactively marked findings - with self.assertNumQueries(7): + with self.assertNumQueries(6): do_false_positive_history_batch(batch) # All pre-existing findings must now be marked as FP. diff --git a/unittests/test_importers_performance.py b/unittests/test_importers_performance.py index 30ecfeb00f3..55686172bbd 100644 --- a/unittests/test_importers_performance.py +++ b/unittests/test_importers_performance.py @@ -275,7 +275,7 @@ def _import_reimport_performance( @tag("performance") -@override_settings(V3_FEATURE_LOCATIONS=False) +@override_settings(V3_FEATURE_LOCATIONS=False, SETTINGS_CACHE_L1_TTL=30, SETTINGS_CACHE_L2_TTL=-1) class TestDojoImporterPerformanceSmall(TestDojoImporterPerformanceBase): """Performance tests using small sample files (StackHawk, ~6 findings).""" @@ -349,7 +349,7 @@ def test_import_reimport_reimport_performance_pghistory_async(self): expected_num_async_tasks2=1, expected_num_queries3=28, expected_num_async_tasks3=1, - expected_num_queries4=99, + expected_num_queries4=105, expected_num_async_tasks4=0, ) @@ -367,13 +367,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._import_reimport_performance( - expected_num_queries1=170, + expected_num_queries1=172, expected_num_async_tasks1=2, - expected_num_queries2=129, + expected_num_queries2=130, expected_num_async_tasks2=1, - expected_num_queries3=36, + expected_num_queries3=37, expected_num_async_tasks3=1, - expected_num_queries4=99, + expected_num_queries4=105, expected_num_async_tasks4=0, ) @@ -392,13 +392,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async_with_product_gr self.system_settings(enable_product_grade=True) self._import_reimport_performance( - expected_num_queries1=180, + expected_num_queries1=184, expected_num_async_tasks1=4, - expected_num_queries2=139, + expected_num_queries2=142, expected_num_async_tasks2=3, - expected_num_queries3=43, + expected_num_queries3=46, expected_num_async_tasks3=3, - expected_num_queries4=108, + expected_num_queries4=116, expected_num_async_tasks4=2, ) @@ -545,15 +545,15 @@ def test_deduplication_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._deduplication_performance( - expected_num_queries1=106, + expected_num_queries1=108, expected_num_async_tasks1=2, - expected_num_queries2=87, + expected_num_queries2=89, expected_num_async_tasks2=2, ) @tag("performance") -@override_settings(V3_FEATURE_LOCATIONS=True) +@override_settings(V3_FEATURE_LOCATIONS=True, SETTINGS_CACHE_L1_TTL=30, SETTINGS_CACHE_L2_TTL=-1) class TestDojoImporterPerformanceSmallLocations(TestDojoImporterPerformanceBase): r""" @@ -639,7 +639,7 @@ def test_import_reimport_reimport_performance_pghistory_async(self): expected_num_async_tasks2=1, expected_num_queries3=36, expected_num_async_tasks3=1, - expected_num_queries4=100, + expected_num_queries4=106, expected_num_async_tasks4=0, ) @@ -657,13 +657,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._import_reimport_performance( - expected_num_queries1=179, + expected_num_queries1=181, expected_num_async_tasks1=2, - expected_num_queries2=140, + expected_num_queries2=141, expected_num_async_tasks2=1, - expected_num_queries3=46, + expected_num_queries3=47, expected_num_async_tasks3=1, - expected_num_queries4=100, + expected_num_queries4=106, expected_num_async_tasks4=0, ) @@ -682,13 +682,13 @@ def test_import_reimport_reimport_performance_pghistory_no_async_with_product_gr self.system_settings(enable_product_grade=True) self._import_reimport_performance( - expected_num_queries1=192, + expected_num_queries1=196, expected_num_async_tasks1=4, - expected_num_queries2=153, + expected_num_queries2=156, expected_num_async_tasks2=3, - expected_num_queries3=53, + expected_num_queries3=56, expected_num_async_tasks3=3, - expected_num_queries4=112, + expected_num_queries4=120, expected_num_async_tasks4=2, ) @@ -809,8 +809,8 @@ def test_deduplication_performance_pghistory_no_async(self): testuser.usercontactinfo.save() self._deduplication_performance( - expected_num_queries1=115, + expected_num_queries1=117, expected_num_async_tasks1=2, - expected_num_queries2=198, + expected_num_queries2=200, expected_num_async_tasks2=2, ) diff --git a/unittests/test_metrics_queries.py b/unittests/test_metrics_queries.py index 4a0cf3c87eb..94ec80f2035 100644 --- a/unittests/test_metrics_queries.py +++ b/unittests/test_metrics_queries.py @@ -4,7 +4,7 @@ from datetime import date, datetime from unittest.mock import patch -from django.test import RequestFactory +from django.test import RequestFactory, override_settings from django.urls import reverse from dojo.metrics import utils @@ -47,6 +47,7 @@ def add(*args, **kwargs): @versioned_fixtures +@override_settings(SETTINGS_CACHE_L1_TTL=30, SETTINGS_CACHE_L2_TTL=-1) class FindingQueriesTest(DojoTestCase): fixtures = ["dojo_testdata.json", "unit_metrics_additional_data.json"] @@ -90,7 +91,7 @@ def test_finding_queries(self, mock_timezone): # Queries over Finding (legacy auth: fewer auth-layer queries # than RBAC since per-action role-permission lookups are gone). - with self.assertNumQueries(27): + with self.assertNumQueries(25): product_types = [] finding_queries = utils.finding_queries( product_types, @@ -267,6 +268,7 @@ def test_closed_findings_filtered_by_mitigated_date(self, mock_timezone): # TODO: Delete this after the move to Locations @skip_unless_v2 +@override_settings(SETTINGS_CACHE_L1_TTL=30, SETTINGS_CACHE_L2_TTL=-1) class EndpointQueriesTest(DojoTestCase): fixtures = ["dojo_testdata.json"] @@ -302,7 +304,7 @@ def test_endpoint_queries(self, mock_now): # Queries over Finding and Endpoint_Status (legacy auth: fewer # auth-layer queries than RBAC). - with self.assertNumQueries(41): + with self.assertNumQueries(40): product_types = Product_Type.objects.all() endpoint_queries = utils.endpoint_queries( product_types, diff --git a/unittests/test_system_settings.py b/unittests/test_system_settings.py index e127bec8f59..4c37898d012 100644 --- a/unittests/test_system_settings.py +++ b/unittests/test_system_settings.py @@ -1,12 +1,12 @@ -from unittest.mock import Mock +from unittest import mock -from django.db import models from django.http import HttpResponse from django.test import RequestFactory, TestCase, override_settings from django.urls import reverse from django.utils.timezone import now -from dojo.middleware import DojoSytemSettingsMiddleware +from dojo.caching import invalidate_dojo_settings_cache, reset_l1_cache +from dojo.middleware import SYSTEM_SETTINGS_CACHE_KEY, DojoSettingsManagerMiddleware, get_cached_system_settings from dojo.models import ( Engagement, Finding, @@ -93,185 +93,72 @@ def test_post_request_initializes_form_with_finding_instance(self): self.assertIn(response.status_code, [200, 302]) +@override_settings(SETTINGS_CACHE_L1_TTL=30) class TestSystemSettingsMiddlewareIntegration(DojoTestCase): - """Integration tests for DojoSytemSettingsMiddleware using RequestFactory.""" + """ + Integration tests for DojoSettingsManagerMiddleware + System_Settings_Manager. + + Caching lives in dojo.caching (in-process L1 decorator); the middleware resets + the request-scoped L1 tier and surfaces a load error. These tests pin L1 on via + override_settings so they don't depend on the compose env. + """ def setUp(self): - """Set up test environment.""" super().setUp() self.factory = RequestFactory() - # Ensure signal is connected - models.signals.post_save.disconnect(DojoSytemSettingsMiddleware.cleanup, sender=System_Settings) - models.signals.post_save.connect(DojoSytemSettingsMiddleware.cleanup, sender=System_Settings) - - def test_middleware_loads_cache_on_request(self): - """Test that middleware loads settings into cache when processing a request.""" - # Ensure cache is empty - DojoSytemSettingsMiddleware.cleanup() - self.assertIsNone(DojoSytemSettingsMiddleware.get_system_settings()) - - # Create middleware with mock get_response - mock_response = HttpResponse("OK") - mock_get_response = Mock(return_value=mock_response) - middleware = DojoSytemSettingsMiddleware(mock_get_response) - - # Create a request - request = self.factory.get("/test/") - - # Process request through middleware - response = middleware(request) - - # Verify response is returned - self.assertEqual(response, mock_response) - mock_get_response.assert_called_once_with(request) - - # Verify cache was populated during request processing - # Note: cache should be cleaned up after request, but we can check during processing - # Since cleanup happens in finally block, cache should be empty after __call__ returns - self.assertIsNone(DojoSytemSettingsMiddleware.get_system_settings()) - - def test_middleware_cleans_up_cache_after_request(self): - """Test that middleware cleans up cache after request processing.""" - # Manually load cache first - DojoSytemSettingsMiddleware.load() - self.assertIsNotNone(DojoSytemSettingsMiddleware.get_system_settings()) - - # Create middleware - middleware = DojoSytemSettingsMiddleware(lambda _r: HttpResponse("OK")) - - # Process request - request = self.factory.get("/test/") - middleware(request) - - # Verify cache is cleaned up after request - self.assertIsNone(DojoSytemSettingsMiddleware.get_system_settings()) - - def test_middleware_cleans_up_cache_on_exception(self): - """Test that middleware cleans up cache even when exception occurs.""" - # Load cache first - DojoSytemSettingsMiddleware.load() - self.assertIsNotNone(DojoSytemSettingsMiddleware.get_system_settings()) - - # Create middleware that raises an exception - def failing_get_response(request): - msg = "Test exception" - raise ValueError(msg) - - middleware = DojoSytemSettingsMiddleware(failing_get_response) - - # Process request - should raise exception - request = self.factory.get("/test/") - with self.assertRaises(ValueError): - middleware(request) - - # Verify cache is cleaned up even after exception - self.assertIsNone(DojoSytemSettingsMiddleware.get_system_settings()) - - def test_middleware_process_exception_cleans_up_cache(self): - """Test that process_exception method cleans up cache.""" - # Load cache first - DojoSytemSettingsMiddleware.load() - self.assertIsNotNone(DojoSytemSettingsMiddleware.get_system_settings()) - - # Create middleware - middleware = DojoSytemSettingsMiddleware(lambda _r: HttpResponse("OK")) - # Call process_exception directly - request = self.factory.get("/test/") - exception = ValueError("Test exception") - middleware.process_exception(request, exception) + def test_no_cache_always_hits_db(self): + # no_cache bypasses both tiers: every read is a fresh query. + with self.assertNumQueries(2): + System_Settings.objects.get(no_cache=True) + System_Settings.objects.get(no_cache=True) - # Verify cache is cleaned up - self.assertIsNone(DojoSytemSettingsMiddleware.get_system_settings()) - - def test_middleware_cache_isolation_between_requests(self): - """Test that cache is isolated between requests (thread-local).""" - # Create middleware - middleware = DojoSytemSettingsMiddleware(lambda _r: HttpResponse("OK")) - - # First request - request1 = self.factory.get("/test1/") - middleware(request1) - self.assertIsNone(DojoSytemSettingsMiddleware.get_system_settings()) - - # Second request - cache should be empty at start - request2 = self.factory.get("/test2/") - middleware(request2) - self.assertIsNone(DojoSytemSettingsMiddleware.get_system_settings()) - - def test_middleware_cache_during_request_processing(self): - """Test that cache is available during request processing.""" - # Track if cache was available during request - cache_available_during_request = [] - - def get_response_with_cache_check(request): - # Check if cache is available during request processing - cached = DojoSytemSettingsMiddleware.get_system_settings() - cache_available_during_request.append(cached is not None) - return HttpResponse("OK") - - middleware = DojoSytemSettingsMiddleware(get_response_with_cache_check) - - # Process request - request = self.factory.get("/test/") - middleware(request) - - # Verify cache was available during request processing - self.assertTrue(cache_available_during_request[0], "Cache should be available during request processing") - - # But cleaned up after request - self.assertIsNone(DojoSytemSettingsMiddleware.get_system_settings()) - - def test_multiple_get_calls_use_cache(self): - """Test that multiple calls to System_Settings.objects.get() use cache instead of multiple DB queries.""" - # Ensure cache is empty - DojoSytemSettingsMiddleware.cleanup() - - # First call should hit DB (cache is empty) + def test_repeated_cached_get_served_from_l1(self): + # Cold start, warm once (1 query), then repeated cached reads do no queries. + reset_l1_cache() + invalidate_dojo_settings_cache(SYSTEM_SETTINGS_CACHE_KEY) with self.assertNumQueries(1): - settings1 = System_Settings.objects.get() - - # Load into cache via middleware - DojoSytemSettingsMiddleware.load() - - # Now multiple calls should use cache (no additional DB queries) + get_cached_system_settings() with self.assertNumQueries(0): - settings2 = System_Settings.objects.get() - settings3 = System_Settings.objects.get() - settings4 = System_Settings.objects.get() + s2 = System_Settings.objects.get() + s3 = System_Settings.objects.get() + # Rebuilt per call from the cached dict: equal data, not the same instance. + self.assertEqual(s2.pk, s3.pk) + + def test_middleware_resets_l1_each_request(self): + # Warm L1, change the row underneath via update() (no post_save signal, so + # L1 is not auto-busted), then a request must still see the new value + # because the middleware resets L1 at request start. + get_cached_system_settings() + System_Settings.objects.update(enable_deduplication=True) + seen = [] + + def view(_request): + seen.append(System_Settings.objects.get().enable_deduplication) + return HttpResponse("OK") - # All calls should return the same cached object instance - self.assertEqual(settings1.id, settings2.id) - self.assertEqual(settings2.id, settings3.id) - self.assertEqual(settings3.id, settings4.id) - # Verify they're the same object instance (same memory address) - self.assertIs(settings2, settings3) - self.assertIs(settings3, settings4) + middleware = DojoSettingsManagerMiddleware(view) + middleware(self.factory.get("/test/")) + self.assertEqual(seen, [True]) - def test_multiple_get_calls_within_request_use_cache(self): - """Test that multiple get() calls within a single request use cache.""" - retrieved_settings = [] + def test_middleware_surfaces_load_error(self): + # When the DB read fails, get_from_db stashes an error on the thread-local + # and returns defaults; the middleware copies the error onto the request + # for the banner context processor. + invalidate_dojo_settings_cache(SYSTEM_SETTINGS_CACHE_KEY) + reset_l1_cache() + captured = {} - def get_response_with_multiple_gets(request): - # Make multiple calls to get() during request processing - retrieved_settings.append(System_Settings.objects.get()) - retrieved_settings.append(System_Settings.objects.get()) - retrieved_settings.append(System_Settings.objects.get()) + def view(request): + captured["err"] = getattr(request, "system_settings_error", None) return HttpResponse("OK") - middleware = DojoSytemSettingsMiddleware(get_response_with_multiple_gets) - - # Process request - should only hit DB once (when loading cache) - # Then all subsequent get() calls should use cache - request = self.factory.get("/test/") - with self.assertNumQueries(1): # Only one query to load settings into cache - middleware(request) - - # Verify we got 3 settings objects - self.assertEqual(len(retrieved_settings), 3) + def failing_get_from_db(*args, **kwargs): + DojoSettingsManagerMiddleware._thread_local.system_settings_error = "boom" + return System_Settings() # defaults (pk None) -> not cached - # All should be the same cached instance - self.assertIs(retrieved_settings[0], retrieved_settings[1]) - self.assertIs(retrieved_settings[1], retrieved_settings[2]) - self.assertEqual(retrieved_settings[0].id, retrieved_settings[1].id) + middleware = DojoSettingsManagerMiddleware(view) + with mock.patch.object(System_Settings.objects, "get_from_db", side_effect=failing_get_from_db): + middleware(self.factory.get("/test/")) + self.assertEqual(captured["err"], "boom") diff --git a/unittests/test_tag_inheritance_perf.py b/unittests/test_tag_inheritance_perf.py index 698b1126a85..1dfaf83965b 100644 --- a/unittests/test_tag_inheritance_perf.py +++ b/unittests/test_tag_inheritance_perf.py @@ -99,6 +99,8 @@ def _make_locations(product: Product, n: int) -> None: @override_settings( CELERY_TASK_ALWAYS_EAGER=True, CELERY_TASK_EAGER_PROPAGATES=True, + SETTINGS_CACHE_L1_TTL=30, + SETTINGS_CACHE_L2_TTL=-1, ) class TagInheritancePerfBaselines(DojoTestCase): @@ -405,10 +407,10 @@ def test_baseline_product_tag_remove_propagates_to_100_locations_v3(self): EXPECTED_PRODUCT_TAG_REMOVE_100_V2 = 53 EXPECTED_PRODUCT_TAG_REMOVE_100_V3 = 53 - EXPECTED_CREATE_ONE_FINDING_V2 = 55 - EXPECTED_CREATE_ONE_FINDING_V3 = 55 - EXPECTED_CREATE_100_FINDINGS_V2 = 3124 - EXPECTED_CREATE_100_FINDINGS_V3 = 3124 + EXPECTED_CREATE_ONE_FINDING_V2 = 56 + EXPECTED_CREATE_ONE_FINDING_V3 = 56 + EXPECTED_CREATE_100_FINDINGS_V2 = 3224 + EXPECTED_CREATE_100_FINDINGS_V3 = 3224 EXPECTED_FINDING_ADD_USER_TAG_V2 = 17 EXPECTED_FINDING_ADD_USER_TAG_V3 = 17 @@ -433,6 +435,8 @@ def test_baseline_product_tag_remove_propagates_to_100_locations_v3(self): CELERY_TASK_ALWAYS_EAGER=True, CELERY_TASK_EAGER_PROPAGATES=True, SECURE_SSL_REDIRECT=False, + SETTINGS_CACHE_L1_TTL=30, + SETTINGS_CACHE_L2_TTL=-1, ) @versioned_fixtures class TagInheritanceImportPerfBaselines(DojoAPITestCase): @@ -590,9 +594,9 @@ def test_baseline_zap_scan_reimport_with_new_findings_v3(self): # the async watson indexer, executed inline under CELERY_TASK_ALWAYS_EAGER); # +5 reimport (no-change + with-new) queries from removal of # WATSON_ASYNC_INDEX_UPDATE_THRESHOLD making async dispatch unconditional. - EXPECTED_ZAP_IMPORT_V2 = 287 - EXPECTED_ZAP_IMPORT_V3 = 311 - EXPECTED_ZAP_REIMPORT_NO_CHANGE_V2 = 74 - EXPECTED_ZAP_REIMPORT_NO_CHANGE_V3 = 86 - EXPECTED_ZAP_REIMPORT_WITH_NEW_V2 = 148 - EXPECTED_ZAP_REIMPORT_WITH_NEW_V3 = 177 + EXPECTED_ZAP_IMPORT_V2 = 291 + EXPECTED_ZAP_IMPORT_V3 = 315 + EXPECTED_ZAP_REIMPORT_NO_CHANGE_V2 = 77 + EXPECTED_ZAP_REIMPORT_NO_CHANGE_V3 = 89 + EXPECTED_ZAP_REIMPORT_WITH_NEW_V2 = 151 + EXPECTED_ZAP_REIMPORT_WITH_NEW_V3 = 180