You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refactor(api): IssueAsync commits, so every caller must wrap it in a transaction #195
Split out of #172, which fixed the lost-write bug in AdminUserService and deliberately left this alone: it is interface depth, not a correctness defect, and folding an auth-flow refactor into a two-line transaction fix would have been the wrong trade.
The seam
TokenService.IssueAsync stages a refresh token and commits it (src/Anichron.API/Services/TokenService.cs:43):
tokens.Add(newRefreshToken{ ...});awaitunitOfWork.SaveChangesAsync(ct);// the callee owns the savereturnnewAuthTokens(jwtFactory.Create(user),rawToken);
Because the callee commits, every caller that has its own pending changes must open a transaction purely to make the two saves atomic. AuthService does this twice — src/Anichron.API/Services/AuthService.cs:106-110 and :160-164:
returnAuthResult.Ok(awaitunitOfWork.ExecuteInTransactionAsync(async()=>{awaitunitOfWork.SaveChangesAsync(ct);// save #1 — the caller's changesreturnawaittokenService.IssueAsync(user,ct);// saves again inside},ct));
Two round trips and an explicit transaction, to work around a callee that commits.
Why it is worth changing
The interface currently reads "issuance also commits, so wrap me". It should read "issuance stages a token" — leaving the caller to save once, as the unit-of-work pattern intends. SaveChangesAsync is the caller's decision everywhere else in the API.
⚠️ The current shape is not a bug. The transactions are present and correct; this is about the interface making them necessary.
Sketch
Drop the SaveChangesAsync from IssueAsync, so it only stages.
Callers save once. AuthService's two sites collapse from transaction-plus-two-saves to a single SaveChangesAsync.
Check every other caller of IssueAsync first — any that relied on it committing will silently stop persisting the token, which is the obvious way to get this wrong.
Done when
IssueAsync stages without committing
AuthService:106 and :160 no longer need a transaction solely to pair two saves
A test pins that a token issued but not saved is not persisted, so the new contract is enforced rather than assumed
Every caller audited, not just the two in AuthService
Line numbers verified against master at the time of writing.
Activity
fix(api): Transact admin user writes, and stop the per-request Argon2…
fix(api): A Live Photo shows up once in On This Day; sign-in issuance…