Reduce the frozen transaction baseline
The committed store (openaev-api/src/test/resources/archunit_store/, see its README) records the
existing violations of the background-transaction rules. Each line is a place where the code
believes it has a transaction or a tenant scope it does not have. This skill treats ONE entry (or
one class's entries) end to end: fix, prove, re-freeze.
Hard rules
- One class per PR. A store shrink must be reviewable at a glance; batching classes hides
regressions in the diff noise.
- Never hand-edit the store. The only legitimate change is a re-freeze run (below).
- The store diff must contain removals only. An added line in the same diff means the fix
introduced a new violation somewhere: stop and look.
- TDD when behavior changes. A self-invocation fix that makes a transaction REAL changes
rollback and scope semantics: write the test proving the new behavior first (red), then fix.
- Re-freeze in the same PR as the fix. The locked store makes a solved violation FAIL the
frozen run (
StoreUpdateFailedException: removing the solved line is a store write, and writes
are locked). The fix and the smaller store travel together.
Phase 1 - Triage the entry
Find the entry in the store file (via stored.rules) and classify it:
| Type |
Store rule |
What it means |
| A |
self-invocation |
a method calls a @Transactional method of its own class: the proxy is bypassed, no transaction, no scope, silently |
| B |
@Transactional on a job |
the job relies on the annotation; background code must open through the primitive |
| C |
raw plumbing in a job |
a hand-built TransactionTemplate / PlatformTransactionManager: a transaction with no tenant scope |
For type A, determine the flavor by reading the CALLER:
- A1, caller already transactional: the inner annotation never did anything on this path. The
behavior does not change; the fix is structural honesty.
- A2, caller not transactional (HTTP side): the callee silently ran without transaction or
scope. This is a live latent bug: fixing it CHANGES behavior (real atomicity, real scope).
- A3, background side: same as A2 but the fix goes through the primitive.
Phase 2 - Apply the sanctioned fix pattern
| Flavor |
Fix |
| A1 |
remove the inner @Transactional (dead) or restructure so the transactional boundary is the real entry point; document the boundary |
| A2 |
extract the transactional method to a collaborator bean and call it through injection (the proxy works again), or make the true entry point @Transactional with the TxCtx parameter |
| A3 |
open through TenantScopedTransaction.execute(ctx, ...); per-tenant work uses forEachTenant |
| B |
remove @Transactional from the job; wrap the work in the primitive with the right scope (forTenant, forEachTenant, or allTenants() for cross-tenant bulk reads and deletes by predicate) |
| C |
replace the raw template with the primitive; the scope choice is the same table as B |
Self-injection (@Autowired of the class into itself) is NOT a sanctioned fix: the rule flags it
deliberately. Extraction to a collaborator is the honest shape.
Background conversions inherit the primitive's rules: never catch-and-continue inside one
transaction (poisoning), recovery around executeNew or a forEachTenant iteration. See
docs/docs/development/tenant-isolation.md.
Phase 3 - Prove it
- A2/A3/B/C (behavior changes): red-first test for the new semantics; at minimum a rollback proof
(fail mid-way, assert nothing committed) and, if the class touches an isolated table, a scope
proof (models:
TenantScopedTransactionIntegrationTest, TenantScopeAllTenantsIntegrationTest).
- A1 (no behavior change): the class's existing test suite green is enough; say so in the PR.
Phase 4 - Re-freeze the store
# one-off: allow the store to shrink, run the frozen rules, re-lock by NOT keeping the flags
mvn -ntp -pl openaev-api test -Dtest='TenantBackgroundTransactionArchTest' \
-Darchunit.freeze.store.default.allowStoreCreation=true \
-Darchunit.freeze.store.default.allowStoreUpdate=true
# verify: the diff contains ONLY removals, then run locked to confirm green
git diff --stat openaev-api/src/test/resources/archunit_store/
mvn -ntp -pl openaev-api test -Dtest='TenantBackgroundTransactionArchTest'
Commit the smaller store WITH the fix. A reviewer must see, in one diff: the fix, its test, and
the exact baseline lines it retires.
Definition of Done
1---2name: reduce-tx-baseline3description: Fixes one entry of the frozen background-transaction baseline (openaev-api/src/test/resources/archunit_store) and shrinks the store in the same PR. Use when asked to fix a @Transactional self-invocation, convert a job away from @Transactional or raw TransactionTemplate, or generally to reduce the architecture debt recorded by TenantBackgroundTransactionArchTest. Covers the triage of the violation type, the sanctioned fix pattern for each, the tests, and the deliberate store re-freeze.4---56# Reduce the frozen transaction baseline78The committed store (`openaev-api/src/test/resources/archunit_store/`, see its README) records the9existing violations of the background-transaction rules. Each line is a place where the code10believes it has a transaction or a tenant scope it does not have. This skill treats ONE entry (or11one class's entries) end to end: fix, prove, re-freeze.1213## Hard rules14151. **One class per PR.** A store shrink must be reviewable at a glance; batching classes hides16 regressions in the diff noise.172. **Never hand-edit the store.** The only legitimate change is a re-freeze run (below).183. **The store diff must contain removals only.** An added line in the same diff means the fix19 introduced a new violation somewhere: stop and look.204. **TDD when behavior changes.** A self-invocation fix that makes a transaction REAL changes21 rollback and scope semantics: write the test proving the new behavior first (red), then fix.225. **Re-freeze in the same PR as the fix.** The locked store makes a solved violation FAIL the23 frozen run (`StoreUpdateFailedException`: removing the solved line is a store write, and writes24 are locked). The fix and the smaller store travel together.2526## Phase 1 - Triage the entry2728Find the entry in the store file (via `stored.rules`) and classify it:2930| Type | Store rule | What it means |31|---|---|---|32| A | self-invocation | a method calls a `@Transactional` method of its own class: the proxy is bypassed, no transaction, no scope, silently |33| B | `@Transactional` on a job | the job relies on the annotation; background code must open through the primitive |34| C | raw plumbing in a job | a hand-built `TransactionTemplate` / `PlatformTransactionManager`: a transaction with no tenant scope |3536For type A, determine the flavor by reading the CALLER:3738- **A1, caller already transactional**: the inner annotation never did anything on this path. The39 behavior does not change; the fix is structural honesty.40- **A2, caller not transactional (HTTP side)**: the callee silently ran without transaction or41 scope. This is a live latent bug: fixing it CHANGES behavior (real atomicity, real scope).42- **A3, background side**: same as A2 but the fix goes through the primitive.4344## Phase 2 - Apply the sanctioned fix pattern4546| Flavor | Fix |47|---|---|48| A1 | remove the inner `@Transactional` (dead) or restructure so the transactional boundary is the real entry point; document the boundary |49| A2 | extract the transactional method to a collaborator bean and call it through injection (the proxy works again), or make the true entry point `@Transactional` with the `TxCtx` parameter |50| A3 | open through `TenantScopedTransaction.execute(ctx, ...)`; per-tenant work uses `forEachTenant` |51| B | remove `@Transactional` from the job; wrap the work in the primitive with the right scope (`forTenant`, `forEachTenant`, or `allTenants()` for cross-tenant bulk reads and deletes by predicate) |52| C | replace the raw template with the primitive; the scope choice is the same table as B |5354Self-injection (`@Autowired` of the class into itself) is NOT a sanctioned fix: the rule flags it55deliberately. Extraction to a collaborator is the honest shape.5657Background conversions inherit the primitive's rules: never catch-and-continue inside one58transaction (poisoning), recovery around `executeNew` or a `forEachTenant` iteration. See59`docs/docs/development/tenant-isolation.md`.6061## Phase 3 - Prove it6263- A2/A3/B/C (behavior changes): red-first test for the new semantics; at minimum a rollback proof64 (fail mid-way, assert nothing committed) and, if the class touches an isolated table, a scope65 proof (models: `TenantScopedTransactionIntegrationTest`, `TenantScopeAllTenantsIntegrationTest`).66- A1 (no behavior change): the class's existing test suite green is enough; say so in the PR.6768## Phase 4 - Re-freeze the store6970```bash71# one-off: allow the store to shrink, run the frozen rules, re-lock by NOT keeping the flags72mvn -ntp -pl openaev-api test -Dtest='TenantBackgroundTransactionArchTest' \73 -Darchunit.freeze.store.default.allowStoreCreation=true \74 -Darchunit.freeze.store.default.allowStoreUpdate=true7576# verify: the diff contains ONLY removals, then run locked to confirm green77git diff --stat openaev-api/src/test/resources/archunit_store/78mvn -ntp -pl openaev-api test -Dtest='TenantBackgroundTransactionArchTest'79```8081Commit the smaller store WITH the fix. A reviewer must see, in one diff: the fix, its test, and82the exact baseline lines it retires.8384## Definition of Done8586- [ ] one class treated, its violation lines gone from the store, no line added anywhere87- [ ] behavior-changing fixes proven red-first (rollback, scope); A1 justified as no-op88- [ ] locked frozen run green after the re-freeze89- [ ] full test suite of the touched module green90- [ ] the PR states the count before/after (the burn-down is the point)
openaev-platform