mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-03 05:10:16 +03:00
ConnorYoh
b4a264239c
New SaaS accounts were landing with `team_id = null`. That state is unrecoverable: portal access derives from leading a team, and signup is the only place one is assigned. Five things had to be fixed, all on the signup path. Only the last is a behaviour change you'd notice. ### 1. Shared-PK entity was routed to `merge()` `SaasUserExtensions` pre-sets its `@MapsId` id in the constructor, so Spring Data's id-nullness check treated a brand-new row as existing and `save()` failed with `AssertionFailure: null identifier`. Now implements `Persistable` and decides on the creation timestamp — the idiom already used by `ProcessedFileEntity` and `SourceDocCountEntity`. This was the blocker. It threw on every signup, and because the failure was swallowed (see 3) every new account was stranded. ### 2. User and team were committed separately `createUser()` is annotated `@Transactional` but is called as `this.createUser(...)`, and self-invocation bypasses the proxy — so the annotation did nothing. `saveUser()` and `ensurePersonalTeam()` each committed in their own transaction, leaving a window where a **committed user was visible with `team_id = null`**. Parallel requests entering that window each provisioned a team, producing duplicates (observed: teams 160/161 and 162/163 for one user). Both writes now happen in one transaction via `SaasTeamService.saveUserWithPersonalTeam()`. The window is gone, so there is nothing left to race over. ### 3. A failed team create was swallowed The old code logged at WARN and committed the user anyway. It now propagates: the shared transaction rolls the user back, the request 401s, and a retry starts clean. Nothing half-built is committed. This is the deliberate trade — a transient failure now surfaces instead of silently producing an account that can never reach the portal. ### 4. Per-request healing removed `recoverMissingTeam` (added in #7180) ran on **every authenticated request** whose user had no team, with no mutual exclusion. Under a burst of parallel requests it was itself a source of concurrent provisioning. Provisioning belongs to signup alone. ### 5. Policy seeding could not run `@TransactionalEventListener(AFTER_COMMIT)` leaves the *completed* transaction bound to the thread, so `JpaPolicyStore.save`'s `@Transactional` joined it instead of opening a live one — and its `FOR UPDATE` lock threw `TransactionRequiredException`. Now seeded in `BEFORE_COMMIT`: the lock has a live transaction, rollback safety is unchanged (a rolled-back team still leaves no policy), and it stays on a single pooled connection. ## Verified `:saas:test` green, both spotless gates green, on top of current `main`. Manually on a live signup: **one** team per user, and the concurrent-signup race resolves correctly through the pre-existing unique-constraint catch (`users_supabase_auth_id_key` violation → refetch the winner). 12 filter tests needed updating. Two of them asserted behaviour this PR deliberately removes (`personalTeamFailureSwallowed`, `assignsTeamWhenMissing`), so they were rewritten to assert the new contract rather than re-stubbed into passing. ## Not in scope - **Existing stranded accounts** are not repaired — with the healer gone, nothing fixes them on the request path. They need a one-off backfill or deletion. - **A DB-level invariant.** A partial unique index (`UNIQUE(created_by_user_id) WHERE is_personal`) would make duplicate personal teams impossible rather than merely unreachable. Wanted, but it is a Supabase migration in the SaaS repo, so it is deliberately separate. - **Per-request auth cost.** The filter still does two remote-Postgres round-trips per authenticated request; a frontend request storm makes that expensive. Being handled separately.