Revisit the 5-minute Cache-Control TTL on GET /api/documents/density #709

New Issue

marcel · 2026-06-01T19:39:13+02:00

marcel commented

2026-06-01 19:39:13 +02:00

Context

The document timeline/density chart on the search dashboard is backed by GET /api/documents/density. The endpoint computes month-bucketed document counts and is fetched whenever the user changes filters (sender, receiver, tags, status, full-text search) on the dashboard.

The response carries an HTTP cache header set in DocumentController.java (~line 410):

return ResponseEntity.ok()
    .cacheControl(CacheControl.maxAge(5, TimeUnit.MINUTES).cachePrivate())
    .body(result);
// → Cache-Control: max-age=300, private

There is no server-side compute cache (no @Cacheable, no Caffeine), and no reverse-proxy/CDN cache in front (Caddy doesn't cache by default). The only cache is this private, max-age=300 header, which lets each user's browser serve repeated density requests from its own cache.

Problem

The 300s TTL is a "feels safe" default, not a value tuned to anything measurable:

The computation is cheap, so the cache is not buying much performance. DocumentService.getDensity() does an in-memory aggregation (load filtered documents → project to dates → group by month into a TreeMap). The archive is ≈5k documents and the stated target is <200ms p95. SQL GROUP BY migration is only planned past 50k docs (issue #481). The backend can comfortably serve these requests uncached at current scale.
The cost of the TTL is staleness, not performance. When a user uploads, edits, transcribes, re-dates, or re-tags a document, the density chart can show stale month counts to that same user for up to 5 minutes. Because the header is private, this only affects the individual browser (no shared/CDN staleness).

So the trade-off is currently tilted toward "avoid a cheap recompute" at the cost of "show stale counts after an edit," which is the wrong way round for a near-real-time editing workflow.

Decision (resolved 2026-06-01)

Remove the Cache-Control header from the density response entirely — always fresh.

Multi-persona review was unanimous toward freshness; DevOps confirmed there is no load reason to keep the cache at this scale (no CDN, no proxy cache, sub-200ms recompute, handful of concurrent users), and UX flagged that a stale chart violates "visibility of system status" — it's an interactive control (click/drag to select), so stale counts are actively misleading.
"Remove" = omit the header, not no-store. Plain omission is the simplest correct option; no-store adds no real protection here (Nora, Felix).
The recompute being imperceptible (<200ms p95) makes the burst-absorption argument for a short TTL not worth the staleness it buys.

Acceptance criteria

DocumentController.density() no longer sets a Cache-Control header — return ResponseEntity.ok(result) (header omitted, not no-store).
DocumentControllerTest.density_emitsPrivateCacheControlHeader updated red-first to assert the header is absent: header().doesNotExist("Cache-Control") (rename the test accordingly).
The getDensity() Javadoc in DocumentService.java (~L140) no longer claims Cache-Control: max-age=300 ... absorbs repeated browse loads — update the comment to match.
No change to DocumentService.getDensity() computation logic.

Out of scope

The in-memory → SQL aggregation migration (tracked separately in #481).
Any server-side compute cache (@Cacheable/Caffeine) — not warranted at current scale.
ETag/conditional-request (304) modeling — raised in review as the "more correct" primitive, but overkill for a sub-200ms recompute; parked.

Pointers

backend/.../document/DocumentController.java — density endpoint + cache header (~L397–412)
backend/.../document/DocumentService.java — getDensity() + scale notes (~L128–196)
backend/.../document/DocumentControllerTest.java — density_emitsPrivateCacheControlHeader (~L1391–1401)
frontend/src/lib/document/timeline.ts — fetchDensity() / buildDensityUrl() (relies on HTTP cache; no explicit frontend cache)

## Context The document timeline/density chart on the search dashboard is backed by `GET /api/documents/density`. The endpoint computes month-bucketed document counts and is fetched whenever the user changes filters (sender, receiver, tags, status, full-text search) on the dashboard. The response carries an HTTP cache header set in `DocumentController.java` (~line 410): ```java return ResponseEntity.ok() .cacheControl(CacheControl.maxAge(5, TimeUnit.MINUTES).cachePrivate()) .body(result); // → Cache-Control: max-age=300, private ``` There is **no** server-side compute cache (no `@Cacheable`, no Caffeine), and no reverse-proxy/CDN cache in front (Caddy doesn't cache by default). The only cache is this `private, max-age=300` header, which lets each user's **browser** serve repeated density requests from its own cache. ## Problem The 300s TTL is a "feels safe" default, not a value tuned to anything measurable: - **The computation is cheap, so the cache is not buying much performance.** `DocumentService.getDensity()` does an **in-memory** aggregation (load filtered documents → project to dates → group by month into a `TreeMap`). The archive is ≈5k documents and the stated target is **<200ms p95**. SQL `GROUP BY` migration is only planned past 50k docs (issue #481). The backend can comfortably serve these requests uncached at current scale. - **The cost of the TTL is staleness, not performance.** When a user uploads, edits, transcribes, re-dates, or re-tags a document, the density chart can show stale month counts to that same user for up to 5 minutes. Because the header is `private`, this only affects the individual browser (no shared/CDN staleness). So the trade-off is currently tilted toward "avoid a cheap recompute" at the cost of "show stale counts after an edit," which is the wrong way round for a near-real-time editing workflow. ## Decision (resolved 2026-06-01) **Remove the `Cache-Control` header from the density response entirely** — always fresh. - Multi-persona review was unanimous toward freshness; DevOps confirmed there is **no load reason** to keep the cache at this scale (no CDN, no proxy cache, sub-200ms recompute, handful of concurrent users), and UX flagged that a stale chart violates "visibility of system status" — it's an interactive control (click/drag to select), so stale counts are actively misleading. - **"Remove" = omit the header**, not `no-store`. Plain omission is the simplest correct option; `no-store` adds no real protection here (Nora, Felix). - The recompute being imperceptible (<200ms p95) makes the burst-absorption argument for a short TTL not worth the staleness it buys. ## Acceptance criteria - [ ] `DocumentController.density()` no longer sets a `Cache-Control` header — return `ResponseEntity.ok(result)` (header omitted, not `no-store`). - [ ] `DocumentControllerTest.density_emitsPrivateCacheControlHeader` updated **red-first** to assert the header is absent: `header().doesNotExist("Cache-Control")` (rename the test accordingly). - [ ] The `getDensity()` Javadoc in `DocumentService.java` (~L140) no longer claims `Cache-Control: max-age=300 ... absorbs repeated browse loads` — update the comment to match. - [ ] No change to `DocumentService.getDensity()` computation logic. ## Out of scope - The in-memory → SQL aggregation migration (tracked separately in #481). - Any server-side compute cache (`@Cacheable`/Caffeine) — not warranted at current scale. - ETag/conditional-request (304) modeling — raised in review as the "more correct" primitive, but overkill for a sub-200ms recompute; parked. ## Pointers - `backend/.../document/DocumentController.java` — density endpoint + cache header (~L397–412) - `backend/.../document/DocumentService.java` — `getDensity()` + scale notes (~L128–196) - `backend/.../document/DocumentControllerTest.java` — `density_emitsPrivateCacheControlHeader` (~L1391–1401) - `frontend/src/lib/document/timeline.ts` — `fetchDensity()` / `buildDensityUrl()` (relies on HTTP cache; no explicit frontend cache)

marcel added the P2-medium needs-discussion refactor labels 2026-06-01 19:39:18 +02:00

marcel commented

2026-06-01 19:40:36 +02:00

🏛️ Markus Keller — Application Architect

Observations

This is a presentation-layer tuning knob, not a structural change. The cache lives entirely in one line of DocumentController (CacheControl.maxAge(5, MINUTES).cachePrivate()), there is no @Cacheable/Caffeine layer, and there is no reverse-proxy cache in front (Caddy doesn't cache by default). So the only cache is the client's own browser. The issue frames this correctly.
The decision to aggregate in-memory and rely on a short HTTP TTL is already documented in the getDensity Javadoc (DocumentService.java:140-146) and deferred to #481 past 50k docs. That's good architectural hygiene — the why is recorded next to the code.
There's an existing precedent worth being consistent with: the file-serving endpoint uses private, max-age=31536000, immutable (Document.java:188). That's correct there because file bytes are immutable. Density is the opposite — its inputs mutate on every edit — so a long TTL is the wrong instinct copied into a context where it doesn't hold.

Recommendations

Lower it, don't remove it. A blunt max-age is the wrong primitive for "expensive enough to not recompute on every keystroke, but cheap enough that staleness is unacceptable." A short window (30s) keeps the burst-absorbing benefit during rapid filter-clicking while bounding staleness to something a user won't notice as "wrong." Removing it entirely is also defensible at this scale, but you lose the cheap protection against a user hammering filters.
This does not need an ADR. It's a parameter, not an architectural decision with lasting structural consequences. The Javadoc already carries the rationale; update that comment to match whatever value is chosen and that's sufficient. Don't manufacture ceremony for a one-line change.
Out of scope is correctly drawn. Keeping getDensity() untouched and not introducing a server-side cache is the right boundary — adding Caffeine here would be premature complexity for a sub-200ms in-memory aggregation over 5k rows.

Open Decisions

Blunt TTL vs. a conditional-request (ETag) model. A Cache-Control window is a time-based guess. The architecturally cleaner primitive for "cache until the data actually changes" is an ETag / If-None-Match 304 flow keyed off a cheap document-mutation signal. That eliminates staleness and the recompute on unchanged data — but it's more moving parts for a chart that already computes in <200ms. For this scale the simple short TTL is almost certainly the right call; I flag the ETag option only so the choice is deliberate rather than defaulted. (Cost: ETag = correctness + zero staleness, but more code and a mutation-version source; short TTL = trivial, but accepts bounded staleness.)

## 🏛️ Markus Keller — Application Architect ### Observations - This is a **presentation-layer tuning knob**, not a structural change. The cache lives entirely in one line of `DocumentController` (`CacheControl.maxAge(5, MINUTES).cachePrivate()`), there is no `@Cacheable`/Caffeine layer, and there is no reverse-proxy cache in front (Caddy doesn't cache by default). So the only cache is the client's own browser. The issue frames this correctly. - The decision to aggregate in-memory and rely on a short HTTP TTL is already documented in the `getDensity` Javadoc (`DocumentService.java:140-146`) and deferred to #481 past 50k docs. That's good architectural hygiene — the *why* is recorded next to the code. - There's an existing precedent worth being consistent with: the file-serving endpoint uses `private, max-age=31536000, immutable` (`Document.java:188`). That's correct there because file bytes are immutable. Density is the opposite — its inputs mutate on every edit — so a long TTL is the wrong instinct copied into a context where it doesn't hold. ### Recommendations - **Lower it, don't remove it.** A blunt `max-age` is the wrong primitive for "expensive enough to not recompute on every keystroke, but cheap enough that staleness is unacceptable." A short window (`30s`) keeps the burst-absorbing benefit during rapid filter-clicking while bounding staleness to something a user won't notice as "wrong." Removing it entirely is also defensible at this scale, but you lose the cheap protection against a user hammering filters. - **This does not need an ADR.** It's a parameter, not an architectural decision with lasting structural consequences. The Javadoc already carries the rationale; update that comment to match whatever value is chosen and that's sufficient. Don't manufacture ceremony for a one-line change. - **Out of scope is correctly drawn.** Keeping `getDensity()` untouched and not introducing a server-side cache is the right boundary — adding Caffeine here would be premature complexity for a sub-200ms in-memory aggregation over 5k rows. ### Open Decisions - **Blunt TTL vs. a conditional-request (ETag) model.** A `Cache-Control` window is a time-based guess. The architecturally cleaner primitive for "cache until the data actually changes" is an `ETag` / `If-None-Match` 304 flow keyed off a cheap document-mutation signal. That eliminates staleness *and* the recompute on unchanged data — but it's more moving parts for a chart that already computes in <200ms. For this scale the simple short TTL is almost certainly the right call; I flag the ETag option only so the choice is deliberate rather than defaulted. _(Cost: ETag = correctness + zero staleness, but more code and a mutation-version source; short TTL = trivial, but accepts bounded staleness.)_

marcel commented

2026-06-01 19:40:46 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

The change is genuinely tiny: one argument in DocumentController.density() (DocumentController.java:410) and one assertion in DocumentControllerTest.density_emitsPrivateCacheControlHeader. The AC already correctly scopes out getDensity() — no production logic moves.
The current test asserts containsString("max-age=300") and containsString("private"). That's the only thing pinning the value, which is exactly the red/green lever for this change.

Recommendations

Red first, even though it's one number. Change the test assertion to the new expected max-age (or, if removed, to assert absence), watch it go red against the current 300, then change the controller line to green. This is the whole TDD cycle for this issue — don't edit the controller first.
Replace the magic 5 / TimeUnit.MINUTES with a named constant. Right now the rationale lives in DocumentService's Javadoc but the value lives in DocumentController with no explanation. Extract it so the code self-documents:
```
// Browser-cache window: absorbs rapid filter-clicking without masking edits.
private static final Duration DENSITY_CACHE_TTL = Duration.ofSeconds(30);
// ...
.cacheControl(CacheControl.maxAge(DENSITY_CACHE_TTL).cachePrivate())
```
CacheControl.maxAge(Duration) is the non-deprecated overload — prefer it over the (long, TimeUnit) form.
If the decision is "remove entirely," assert it properly: header().doesNotExist("Cache-Control") rather than just dropping the assertion, so the absence is intentional and regression-protected. And update the getDensity Javadoc that currently claims max-age=300 ... absorbs repeated browse loads — a stale comment is worse than none.

Open Decisions (none)

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - The change is genuinely tiny: one argument in `DocumentController.density()` (`DocumentController.java:410`) and one assertion in `DocumentControllerTest.density_emitsPrivateCacheControlHeader`. The AC already correctly scopes out `getDensity()` — no production logic moves. - The current test asserts `containsString("max-age=300")` and `containsString("private")`. That's the only thing pinning the value, which is exactly the red/green lever for this change. ### Recommendations - **Red first, even though it's one number.** Change the test assertion to the new expected `max-age` (or, if removed, to assert absence), watch it go red against the current `300`, then change the controller line to green. This is the whole TDD cycle for this issue — don't edit the controller first. - **Replace the magic `5` / `TimeUnit.MINUTES` with a named constant.** Right now the rationale lives in `DocumentService`'s Javadoc but the value lives in `DocumentController` with no explanation. Extract it so the code self-documents: ```java // Browser-cache window: absorbs rapid filter-clicking without masking edits. private static final Duration DENSITY_CACHE_TTL = Duration.ofSeconds(30); // ... .cacheControl(CacheControl.maxAge(DENSITY_CACHE_TTL).cachePrivate()) ``` `CacheControl.maxAge(Duration)` is the non-deprecated overload — prefer it over the `(long, TimeUnit)` form. - **If the decision is "remove entirely,"** assert it properly: `header().doesNotExist("Cache-Control")` rather than just dropping the assertion, so the absence is intentional and regression-protected. And update the `getDensity` Javadoc that currently claims `max-age=300 ... absorbs repeated browse loads` — a stale comment is worse than none. ### Open Decisions _(none)_

marcel commented

2026-06-01 19:40:56 +02:00

🔒 Nora Steiner ("NullX") — Application Security Engineer

Observations

The cachePrivate() directive is doing real security work here, and it must stay. The density response is filterable per-user (sender, receiver, tag, status, full-text q) and reflects which documents exist that match a query. A public cache (browser shared cache, or any intermediary) could serve one user's filtered month-counts to another. private is the correct, deliberate scope — CWE-525 (sensitive info in browser cache) is bounded to the single authenticated browser.
I checked the endpoint: GET /api/documents/density has no @RequirePermission, which is correct — write-permission annotations are for mutating verbs. Authentication is enforced globally (anyRequest().authenticated() in SecurityConfig), so this is not an unauthenticated leak.

Recommendations

Whatever TTL is chosen, keep private. Lowering max-age or removing it has zero security downside — fresher data is strictly safer for a per-user response. The one change that would be a security regression is switching to cachePublic() or dropping private while keeping a cache. Don't.
If you remove the header entirely, prefer simply omitting it over adding no-store. Omission already means the browser won't reliably reuse a credentialed GET across sessions; no-store would be belt-and-suspenders but adds no real protection here and just noise.
Add the chosen-header assertion to the regression suite (the existing density_emitsPrivateCacheControlHeader test) so a future refactor can't silently flip private → public. That test is the security control here.

Open Decisions (none) — this is a freshness/UX tradeoff; from the security angle, every candidate value is acceptable as long as `private` is retained.

## 🔒 Nora Steiner ("NullX") — Application Security Engineer ### Observations - The `cachePrivate()` directive is doing real security work here, and it must stay. The density response is **filterable per-user** (sender, receiver, tag, status, full-text `q`) and reflects which documents exist that match a query. A `public` cache (browser shared cache, or any intermediary) could serve one user's filtered month-counts to another. `private` is the correct, deliberate scope — CWE-525 (sensitive info in browser cache) is bounded to the single authenticated browser. - I checked the endpoint: `GET /api/documents/density` has no `@RequirePermission`, which is correct — write-permission annotations are for mutating verbs. Authentication is enforced globally (`anyRequest().authenticated()` in `SecurityConfig`), so this is not an unauthenticated leak. ### Recommendations - **Whatever TTL is chosen, keep `private`.** Lowering `max-age` or removing it has **zero** security downside — fresher data is strictly safer for a per-user response. The one change that would be a security regression is switching to `cachePublic()` or dropping `private` while keeping a cache. Don't. - If you remove the header entirely, prefer simply omitting it over adding `no-store`. Omission already means the browser won't reliably reuse a credentialed `GET` across sessions; `no-store` would be belt-and-suspenders but adds no real protection here and just noise. - Add the chosen-header assertion to the regression suite (the existing `density_emitsPrivateCacheControlHeader` test) so a future refactor can't silently flip `private` → `public`. That test *is* the security control here. ### Open Decisions _(none)_ — this is a freshness/UX tradeoff; from the security angle, every candidate value is acceptable as long as `private` is retained.

marcel commented

2026-06-01 19:41:05 +02:00

🧪 Sara Holt — QA Engineer

Observations

Exactly one test pins this behavior: DocumentControllerTest.density_emitsPrivateCacheControlHeader, asserting containsString("max-age=300") + containsString("private"). It's a @WebMvcTest slice — fast, right layer for a header assertion. No integration test touches the TTL, nor should one; the cache header is a controller-contract concern, not a Postgres-behavior concern.
The 88% branch-coverage gate is not at risk — this changes a constant, not a branch.

Recommendations

Tighten the assertion to an exact value while you're in there. containsString("max-age=300") would also pass for max-age=3000. If the decision is a concrete TTL, assert the exact directive (Cache-Control: max-age=30, private) so a fat-fingered future edit can't slip through. One logical thing per test still holds — it's one header contract.
If the decision is "remove the header": assert header().doesNotExist("Cache-Control"). Don't just delete the test — an absent assertion is an untested behavior, and "no cache header" is itself the contract we're now promising.
No new freshness/E2E test. "Chart reflects an edit within Ns" is tempting to E2E, but it's inherently timing-based and would be flaky (you'd be asserting on browser cache eviction). The HTTP-header unit assertion is the correct and sufficient coverage; trust the browser to honor max-age. Keep this out of the Playwright suite.

Open Decisions (none)

## 🧪 Sara Holt — QA Engineer ### Observations - Exactly one test pins this behavior: `DocumentControllerTest.density_emitsPrivateCacheControlHeader`, asserting `containsString("max-age=300")` + `containsString("private")`. It's a `@WebMvcTest` slice — fast, right layer for a header assertion. No integration test touches the TTL, nor should one; the cache header is a controller-contract concern, not a Postgres-behavior concern. - The 88% branch-coverage gate is not at risk — this changes a constant, not a branch. ### Recommendations - **Tighten the assertion to an exact value while you're in there.** `containsString("max-age=300")` would also pass for `max-age=3000`. If the decision is a concrete TTL, assert the exact directive (`Cache-Control: max-age=30, private`) so a fat-fingered future edit can't slip through. One logical thing per test still holds — it's one header contract. - **If the decision is "remove the header":** assert `header().doesNotExist("Cache-Control")`. Don't just delete the test — an absent assertion is an untested behavior, and "no cache header" is itself the contract we're now promising. - **No new freshness/E2E test.** "Chart reflects an edit within Ns" is tempting to E2E, but it's inherently timing-based and would be flaky (you'd be asserting on browser cache eviction). The HTTP-header unit assertion is the correct and sufficient coverage; trust the browser to honor `max-age`. Keep this out of the Playwright suite. ### Open Decisions _(none)_

marcel commented

2026-06-01 19:41:15 +02:00

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

This maps directly to Nielsen's heuristic #1, Visibility of system status. The density chart sits above the document list as a filter surface. If a user edits a document's date, or transcribes/re-tags one, and the bars don't move for up to 5 minutes, the interface is silently lying about the current state of the archive. For the dual audience this matters differently: a 60+ researcher won't suspect a cache — they'll assume their edit didn't save and may redo it.
The chart is also a control, not just a readout (click-to-select month, drag-to-select range). A stale control is worse than a stale chart — the user acts on counts that no longer reflect reality.

Recommendations

Favor freshness. From a pure UX standpoint, remove the cache or set it as low as the team is comfortable with. A recompute the user can't perceive (sub-200ms) is strictly better than a visible-but-wrong chart. There is no UX upside to 5 minutes of staleness — the user never benefits from "the chart is slightly out of date but loaded from cache."
If a short TTL is kept for burst-absorption, 30s is the ceiling I'd accept — long enough to cover rapid filter-clicking within one interaction, short enough that it never survives across an edit-then-look-again flow.
No accessibility-specific concern here (no markup or contrast change). The only user-facing risk is the trust/status one above.

Open Decisions (none) — my recommendation is unambiguous: prioritize freshness.

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist ### Observations - This maps directly to Nielsen's heuristic #1, **Visibility of system status**. The density chart sits *above* the document list as a filter surface. If a user edits a document's date, or transcribes/re-tags one, and the bars don't move for up to 5 minutes, the interface is silently lying about the current state of the archive. For the dual audience this matters differently: a 60+ researcher won't suspect a cache — they'll assume their edit didn't save and may redo it. - The chart is also a *control*, not just a readout (click-to-select month, drag-to-select range). A stale control is worse than a stale chart — the user acts on counts that no longer reflect reality. ### Recommendations - **Favor freshness.** From a pure UX standpoint, remove the cache or set it as low as the team is comfortable with. A recompute the user can't perceive (sub-200ms) is strictly better than a visible-but-wrong chart. There is no UX upside to 5 minutes of staleness — the user never benefits from "the chart is slightly out of date but loaded from cache." - If a short TTL is kept for burst-absorption, **30s is the ceiling I'd accept** — long enough to cover rapid filter-clicking within one interaction, short enough that it never survives across an edit-then-look-again flow. - No accessibility-specific concern here (no markup or contrast change). The only user-facing risk is the trust/status one above. ### Open Decisions _(none)_ — my recommendation is unambiguous: prioritize freshness.

marcel commented

2026-06-01 19:41:26 +02:00

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

There is no shared cache to lose by lowering this. Caddy doesn't cache by default, and there's no CDN in front of a family-scale self-hosted app. The private directive means even if a cache existed it wouldn't store this. So the only thing the 300s buys is "the same browser doesn't re-request density while clicking filters." That's a micro-optimization, not a load-bearing one.
Concurrent user count here is a handful, not thousands. A sub-200ms in-memory aggregation over ~5k rows, hit a few extra times per browse session, is invisible on a CX32. Removing the cache will not move the needle on VPS load, DB connections, or p95.

Recommendations

The load argument for keeping the cache is effectively nil at current scale — decide this purely on the freshness/UX axis, not on infrastructure cost. I have no objection to removing it.
Make the decision data-driven instead of another "feels safe" guess. The endpoint is already trace-able via Actuator/Micrometer. Before and after the change, look at the http.server.requests timer for uri=/api/documents/density in Grafana (p50/p95, request rate). That turns "300s felt like a lot" into "density p95 is Xms at Y req/min, so the cache is/ isn't worth keeping." This is a 5-minute Grafana check, not new instrumentation.
Keep private (network-isolation/privacy reasons are Nora's call, but operationally it also keeps any future intermediary from caching per-user data — don't regress it).

Open Decisions (none)

## 🛠️ Tobias Wendt — DevOps & Platform Engineer ### Observations - There is **no shared cache to lose** by lowering this. Caddy doesn't cache by default, and there's no CDN in front of a family-scale self-hosted app. The `private` directive means even if a cache existed it wouldn't store this. So the only thing the 300s buys is "the same browser doesn't re-request density while clicking filters." That's a micro-optimization, not a load-bearing one. - Concurrent user count here is a handful, not thousands. A sub-200ms in-memory aggregation over ~5k rows, hit a few extra times per browse session, is invisible on a CX32. Removing the cache will not move the needle on VPS load, DB connections, or p95. ### Recommendations - **The load argument for keeping the cache is effectively nil at current scale** — decide this purely on the freshness/UX axis, not on infrastructure cost. I have no objection to removing it. - **Make the decision data-driven instead of another "feels safe" guess.** The endpoint is already trace-able via Actuator/Micrometer. Before *and* after the change, look at the `http.server.requests` timer for `uri=/api/documents/density` in Grafana (p50/p95, request rate). That turns "300s felt like a lot" into "density p95 is Xms at Y req/min, so the cache is/ isn't worth keeping." This is a 5-minute Grafana check, not new instrumentation. - Keep `private` (network-isolation/privacy reasons are Nora's call, but operationally it also keeps any future intermediary from caching per-user data — don't regress it). ### Open Decisions _(none)_

marcel commented

2026-06-01 19:41:38 +02:00

📋 Elicit — Requirements Engineer

Observations

The issue is well-structured (context, problem, candidate options, scoped ACs, pointers) — this is a healthy, refinement-ready ticket. The needs-discussion label is correctly applied: the central AC, "A decision is recorded on the target TTL," is a decision, not yet a requirement. Nothing is implementable until that number exists.
Underneath the TTL choice is an unstated non-functional requirement about data freshness that currently competes with an existing one. The code documents a performance NFR ("<200ms p95", DocumentService.java:140). There is no corresponding freshness NFR — and this whole issue exists because the two were never weighed against each other.

Recommendations

Make the implicit freshness NFR explicit and measurable before coding, in EARS form:

NFR-FRESH-001: When a user modifies a document's date, sender, receiver, tags, or status, the timeline density chart shall reflect the change within N seconds on the next view.
Pin N and the TTL falls out of it automatically (max-age ≈ N, or remove the header if N→0). This converts a subjective "5 minutes feels like a lot" into a testable target.
Watch the ambiguity in the word "remove." "Remove the cache" could mean (a) omit the Cache-Control header, or (b) emit no-store. These produce different browser behavior. The implementer needs the exact intended outcome — pin it in the decision, don't leave it to interpretation.
The ACs are otherwise testable and the out-of-scope section correctly fences off #481. No scope creep risk here — it's a one-knob change.

Open Decisions

What is the acceptable staleness window (N) for the density chart after an edit? This single answer resolves the TTL value. Options on the table: 0s (remove header — always fresh), ~30s (absorbs filter-click bursts, edit-staleness imperceptible), ~60s (more burst protection, slightly more staleness). The team has uniformly leaned toward freshness; this just needs the number recorded.
"Remove" semantics: omit the header vs. no-store — must be stated explicitly if removal is chosen.

## 📋 Elicit — Requirements Engineer ### Observations - The issue is well-structured (context, problem, candidate options, scoped ACs, pointers) — this is a healthy, refinement-ready ticket. The `needs-discussion` label is correctly applied: the central AC, *"A decision is recorded on the target TTL,"* is a **decision, not yet a requirement**. Nothing is implementable until that number exists. - Underneath the TTL choice is an unstated **non-functional requirement about data freshness** that currently competes with an existing one. The code documents a performance NFR (*"<200ms p95"*, `DocumentService.java:140`). There is no corresponding freshness NFR — and this whole issue exists because the two were never weighed against each other. ### Recommendations - Make the implicit freshness NFR explicit and measurable before coding, in EARS form: > **NFR-FRESH-001:** When a user modifies a document's date, sender, receiver, tags, or status, the timeline density chart shall reflect the change within **N seconds** on the next view. Pin `N` and the TTL falls out of it automatically (`max-age ≈ N`, or remove the header if `N→0`). This converts a subjective "5 minutes feels like a lot" into a testable target. - Watch the **ambiguity in the word "remove."** "Remove the cache" could mean (a) omit the `Cache-Control` header, or (b) emit `no-store`. These produce different browser behavior. The implementer needs the exact intended outcome — pin it in the decision, don't leave it to interpretation. - The ACs are otherwise testable and the out-of-scope section correctly fences off #481. No scope creep risk here — it's a one-knob change. ### Open Decisions - **What is the acceptable staleness window (N) for the density chart after an edit?** This single answer resolves the TTL value. Options on the table: `0s` (remove header — always fresh), `~30s` (absorbs filter-click bursts, edit-staleness imperceptible), `~60s` (more burst protection, slightly more staleness). _The team has uniformly leaned toward freshness; this just needs the number recorded._ - **"Remove" semantics:** omit the header vs. `no-store` — must be stated explicitly if removal is chosen.

marcel commented

2026-06-01 19:41:54 +02:00

🗳️ Decision Queue — Action Required

2 decisions need your input before implementation starts.

Product / Data freshness

What staleness window (N) is acceptable for the density chart after an edit? This single number resolves the TTL. The whole panel leaned toward freshness — DevOps confirmed there's no load reason to keep the cache (no CDN, no proxy cache, sub-200ms recompute, handful of users), and UX wants the chart to never visibly lie about archive state. Options:
- 0s — remove the header (always fresh). Simplest; recompute is imperceptible at this scale. (Leonie, Tobias comfortable; Markus's only caveat: you lose cheap protection against a user hammering filters.)
- ~30s — short browser-cache window. Absorbs rapid filter-clicking within one interaction; edit-staleness too short to notice. (Markus's and Leonie's preferred ceiling.)
- ~60s — slightly more burst protection, slightly more staleness.
  (Raised by: Elicit, Markus, Leonie, Tobias)

Implementation semantics

If you choose "remove," which removal? Omit the Cache-Control header entirely (recommended — simplest, no downside here) vs. emit no-store. These are different browser behaviors and must be pinned so the implementer isn't guessing. (Raised by: Elicit, Felix, Nora — Nora and Felix both recommend plain omission over no-store.)

Non-negotiable constraints all personas agreed on (not decisions — just don't break these):

Keep cachePrivate() whatever you choose (Nora: per-user filtered data — public would be a cross-user leak).
Update the now-stale getDensity Javadoc (DocumentService.java:140) that still cites max-age=300.
Update the existing density_emitsPrivateCacheControlHeader test red-first; assert the exact new value, or header().doesNotExist(...) if removed.

## 🗳️ Decision Queue — Action Required _2 decisions need your input before implementation starts._ ### Product / Data freshness - **What staleness window (N) is acceptable for the density chart after an edit?** This single number resolves the TTL. The whole panel leaned toward freshness — DevOps confirmed there's **no load reason** to keep the cache (no CDN, no proxy cache, sub-200ms recompute, handful of users), and UX wants the chart to never visibly lie about archive state. Options: - **`0s` — remove the header (always fresh).** Simplest; recompute is imperceptible at this scale. _(Leonie, Tobias comfortable; Markus's only caveat: you lose cheap protection against a user hammering filters.)_ - **`~30s` — short browser-cache window.** Absorbs rapid filter-clicking within one interaction; edit-staleness too short to notice. _(Markus's and Leonie's preferred ceiling.)_ - **`~60s`** — slightly more burst protection, slightly more staleness. _(Raised by: Elicit, Markus, Leonie, Tobias)_ ### Implementation semantics - **If you choose "remove," which removal?** Omit the `Cache-Control` header entirely (recommended — simplest, no downside here) **vs.** emit `no-store`. These are different browser behaviors and must be pinned so the implementer isn't guessing. _(Raised by: Elicit, Felix, Nora — Nora and Felix both recommend plain omission over `no-store`.)_ --- **Non-negotiable constraints all personas agreed on (not decisions — just don't break these):** - Keep `cachePrivate()` whatever you choose (Nora: per-user filtered data — `public` would be a cross-user leak). - Update the now-stale `getDensity` Javadoc (`DocumentService.java:140`) that still cites `max-age=300`. - Update the existing `density_emitsPrivateCacheControlHeader` test red-first; assert the **exact** new value, or `header().doesNotExist(...)` if removed.

marcel removed the needs-discussion label 2026-06-01 19:46:25 +02:00

marcel referenced this issue from a commit

2026-06-01 19:57:00 +02:00

refactor(document): drop the 5-minute Cache-Control TTL on /density (#709)

marcel referenced a pull request that will close this issue

2026-06-01 19:57:12 +02:00

refactor(document): drop the 5-minute Cache-Control TTL on /density (#709) #711

marcel commented

2026-06-01 19:57:32 +02:00

✅ Implemented — PR #711

Worked the resolved decision (remove the explicit header, always-fresh) via red/green TDD on branch feat/issue-709-remove-density-cache-ttl.

Commit: 50f5546 — refactor(document): drop the 5-minute Cache-Control TTL on /density (#709)

AC status

DocumentController.density() no longer sets a Cache-Control header → return ResponseEntity.ok(result) (unused CacheControl / TimeUnit imports removed).
Cache-header test updated red-first — density_emitsPrivateCacheControlHeader → density_isNeverBrowserCached.
getDensity() Javadoc no longer claims max-age=300 … absorbs repeated browse loads.
No change to getDensity() computation logic.

⚠️ One deviation from the literal AC (confirmed with the maintainer before proceeding)

The AC asked for header().doesNotExist("Cache-Control") ("omit, not no-store"). That turned out to be impossible: our explicit header was overriding Spring Security's HeaderWriterFilter, which writes Cache-Control: no-cache, no-store, max-age=0, must-revalidate on every secured response. Omitting our header lets that framework default apply.

This over-satisfies the goal:

The density response is now never browser-cached at all — stronger than plain omission.
no-store is strictly safer than private for per-user filtered data — no caching means no cross-user leak, so Nora's cachePrivate() constraint is satisfied for free.

The test now asserts the exact real contract: Cache-Control: no-cache, no-store, max-age=0, must-revalidate.

Verification

DocumentControllerTest — 105/105 green (red confirmed before the controller change).
./mvnw clean package -DskipTests — BUILD SUCCESS.

Out-of-scope (#481 SQL aggregation, server-side compute cache, ETag/304) untouched as specified.

## ✅ Implemented — PR #711 Worked the resolved decision (remove the explicit header, always-fresh) via red/green TDD on branch `feat/issue-709-remove-density-cache-ttl`. **Commit:** `50f5546` — `refactor(document): drop the 5-minute Cache-Control TTL on /density (#709)` ### AC status - [x] `DocumentController.density()` no longer sets a `Cache-Control` header → `return ResponseEntity.ok(result)` (unused `CacheControl` / `TimeUnit` imports removed). - [x] Cache-header test updated **red-first** — `density_emitsPrivateCacheControlHeader` → `density_isNeverBrowserCached`. - [x] `getDensity()` Javadoc no longer claims `max-age=300 … absorbs repeated browse loads`. - [x] No change to `getDensity()` computation logic. ### ⚠️ One deviation from the literal AC (confirmed with the maintainer before proceeding) The AC asked for `header().doesNotExist("Cache-Control")` ("omit, not `no-store`"). That turned out to be **impossible**: our explicit header was *overriding* Spring Security's `HeaderWriterFilter`, which writes `Cache-Control: no-cache, no-store, max-age=0, must-revalidate` on every secured response. Omitting our header lets that framework default apply. This **over-satisfies** the goal: - The density response is now **never browser-cached at all** — stronger than plain omission. - `no-store` is strictly safer than `private` for per-user filtered data — no caching means no cross-user leak, so Nora's `cachePrivate()` constraint is satisfied for free. The test now asserts the exact real contract: `Cache-Control: no-cache, no-store, max-age=0, must-revalidate`. ### Verification - `DocumentControllerTest` — 105/105 green (red confirmed before the controller change). - `./mvnw clean package -DskipTests` — BUILD SUCCESS. Out-of-scope (#481 SQL aggregation, server-side compute cache, ETag/304) untouched as specified.

marcel referenced this issue

2026-06-01 19:58:33 +02:00

refactor(document): drop the 5-minute Cache-Control TTL on /density (#709) #711

marcel referenced this issue

2026-06-01 19:58:59 +02:00

refactor(document): drop the 5-minute Cache-Control TTL on /density (#709) #711

marcel referenced this issue

2026-06-01 19:59:43 +02:00

refactor(document): drop the 5-minute Cache-Control TTL on /density (#709) #711

marcel closed this issue

2026-06-01 20:01:32 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#709

Revisit the 5-minute Cache-Control TTL on GET /api/documents/density #709

Context

Problem

Decision (resolved 2026-06-01)

Acceptance criteria

Out of scope

Pointers

🏛️ Markus Keller — Application Architect

Observations

Recommendations

Open Decisions

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

Recommendations

Open Decisions (none)

🔒 Nora Steiner ("NullX") — Application Security Engineer

Observations

Recommendations

Open Decisions (none) — this is a freshness/UX tradeoff; from the security angle, every candidate value is acceptable as long as private is retained.

🧪 Sara Holt — QA Engineer

Observations

Recommendations

Open Decisions (none)

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

Recommendations

Open Decisions (none) — my recommendation is unambiguous: prioritize freshness.

🛠️ Tobias Wendt — DevOps & Platform Engineer

Observations

Recommendations

Open Decisions (none)

📋 Elicit — Requirements Engineer

Observations

Recommendations

Open Decisions

🗳️ Decision Queue — Action Required

Product / Data freshness

Implementation semantics

✅ Implemented — PR #711

AC status

⚠️ One deviation from the literal AC (confirmed with the maintainer before proceeding)

Verification

Open Decisions (none) — this is a freshness/UX tradeoff; from the security angle, every candidate value is acceptable as long as `private` is retained.