#63 — Tech Debt: Template Seeding (#24) Is Not Atomic and Costs N Sequential Round Trips #63

Closed
opened 2026-08-18 13:13:24 +02:00 by lena · 1 comment
lena commented 2026-08-18 13:13:24 +02:00 (Migrated from git.butzei.de)

Tech Debt: Template Seeding (#24) Is Not Atomic and Costs N Sequential Round Trips

Reported by: Backend Engineer, spotted during #24's code review, 2026-07-22

Symptom

CreateTodoListCommandHandler (added in #24, List Templates) commits the new list + owner
membership via SaveChangesAsync, then loops over the chosen template's seed titles calling the
full CreateTodoCommand handler once per title:

foreach (var title in TodoListTemplateCatalog.SeedTitles(request.Template))
    await createTodoHandler.Handle(new CreateTodoCommand(entity.Id, title, TodoDescription.Empty), cancellationToken);

Two compounding issues, both accepted as an intentional trade-off in #24's design note (to avoid an
untested interaction between batch INSERTs and the trg_set_todo_nr_per_list Postgres trigger),
but not fully worked through:

  1. Not atomic. The list and owner membership are already committed before the loop runs; each
    CreateTodoCommand does its own separate save. If iteration k fails (transient DB error, a
    future catalog title that fails TodoTitle validation, cancellation), the list is left
    persisted with a partial set of seeded todos while the whole CreateTodoListCommand throws back
    to the client as a failure. The user sees an error for a list that actually exists with partial
    contents, and may retry, creating a duplicate list.
  2. N sequential round trips. Each seeded title re-runs the entire single-todo pipeline: its own
    AuthorizeTodoListAccessForCurrentUserQuery + AuthorizeTodoListIsNotArchivedQuery DB checks
    (both already guaranteed by the owner-membership row just committed), its own insert +
    GetTodoQuery re-fetch, its own WS broadcast, and its own CreateActivityEventCommand. A 6-item
    template (Groceries) costs roughly 25-30 DB round trips and fans out 6 rapid WS/activity events
    for one user action. This is fine at today's template sizes but scales linearly — a future larger
    template, or a future "duplicate list"/"import" feature reusing this same handler, would turn
    list creation into a multi-second operation with a jarring flood of client-visible updates.

Expected behaviour

Either:

  • A tested batch-insert path (dbContext.AddRange(todoEntities); SaveChangesAsync() inside the
    same transaction as the list + membership insert) verified against
    trg_set_todo_nr_per_list — Postgres BEFORE INSERT ROW triggers on a multi-row statement see
    each prior sibling row within the same statement (command counter increments per row), so this is
    likely already safe but was never actually tested, only assumed risky and avoided; or
  • If the per-item pipeline (auth, broadcast, activity event) must be kept for parity with
    hand-typed todos, at minimum wrap the whole CreateTodoListCommand handler body in one DB
    transaction so a mid-loop failure rolls back the list + membership + any partially-seeded todos
    instead of leaving an inconsistent partial list.

Acceptance criteria

  • A mid-seed failure (simulated, e.g. by injecting a failure on the 3rd CreateTodoCommand
    call) leaves no persisted list/membership/todos — full rollback, not partial state
  • A test proves the chosen approach (batch insert, or transactional wrapping) is safe against
    trg_set_todo_nr_per_list — the exact gap #24's design note flagged as untested
  • No regression to per-todo WS broadcast / activity-feed behavior for hand-typed (non-template)
    todo creation
  • Template list creation latency does not scale linearly with per-item auth/re-fetch overhead
    for larger templates (either by batching or by explicitly justifying why the per-item cost is
    acceptable at a stated max template size)

Notes for whoever picks this up

Not urgent — current templates cap at 6 items and this is a "Could" priority feature — but flag it
before templates grow or before a "duplicate list"/"import" story reuses this handler's pattern.

Agents involved

  • Backend Engineer — owns the transaction/batch-insert fix and the trigger-interaction test

Blockers

None.

# Tech Debt: Template Seeding (`#24`) Is Not Atomic and Costs N Sequential Round Trips **Reported by:** Backend Engineer, spotted during `#24`'s code review, 2026-07-22 ## Symptom `CreateTodoListCommandHandler` (added in `#24`, List Templates) commits the new list + owner membership via `SaveChangesAsync`, then loops over the chosen template's seed titles calling the full `CreateTodoCommand` handler once per title: ```csharp foreach (var title in TodoListTemplateCatalog.SeedTitles(request.Template)) await createTodoHandler.Handle(new CreateTodoCommand(entity.Id, title, TodoDescription.Empty), cancellationToken); ``` Two compounding issues, both accepted as an intentional trade-off in `#24`'s design note (to avoid an untested interaction between batch `INSERT`s and the `trg_set_todo_nr_per_list` Postgres trigger), but not fully worked through: 1. **Not atomic.** The list and owner membership are already committed before the loop runs; each `CreateTodoCommand` does its own separate save. If iteration *k* fails (transient DB error, a future catalog title that fails `TodoTitle` validation, cancellation), the list is left persisted with a partial set of seeded todos while the whole `CreateTodoListCommand` throws back to the client as a failure. The user sees an error for a list that actually exists with partial contents, and may retry, creating a duplicate list. 2. **N sequential round trips.** Each seeded title re-runs the entire single-todo pipeline: its own `AuthorizeTodoListAccessForCurrentUserQuery` + `AuthorizeTodoListIsNotArchivedQuery` DB checks (both already guaranteed by the owner-membership row just committed), its own insert + `GetTodoQuery` re-fetch, its own WS broadcast, and its own `CreateActivityEventCommand`. A 6-item template (Groceries) costs roughly 25-30 DB round trips and fans out 6 rapid WS/activity events for one user action. This is fine at today's template sizes but scales linearly — a future larger template, or a future "duplicate list"/"import" feature reusing this same handler, would turn list creation into a multi-second operation with a jarring flood of client-visible updates. ## Expected behaviour Either: - A tested batch-insert path (`dbContext.AddRange(todoEntities); SaveChangesAsync()` inside the same transaction as the list + membership insert) verified against `trg_set_todo_nr_per_list` — Postgres `BEFORE INSERT ROW` triggers on a multi-row statement see each prior sibling row within the same statement (command counter increments per row), so this is likely already safe but was never actually tested, only assumed risky and avoided; or - If the per-item pipeline (auth, broadcast, activity event) must be kept for parity with hand-typed todos, at minimum wrap the whole `CreateTodoListCommand` handler body in one DB transaction so a mid-loop failure rolls back the list + membership + any partially-seeded todos instead of leaving an inconsistent partial list. ## Acceptance criteria - [ ] A mid-seed failure (simulated, e.g. by injecting a failure on the 3rd `CreateTodoCommand` call) leaves no persisted list/membership/todos — full rollback, not partial state - [ ] A test proves the chosen approach (batch insert, or transactional wrapping) is safe against `trg_set_todo_nr_per_list` — the exact gap `#24`'s design note flagged as untested - [ ] No regression to per-todo WS broadcast / activity-feed behavior for hand-typed (non-template) todo creation - [ ] Template list creation latency does not scale linearly with per-item auth/re-fetch overhead for larger templates (either by batching or by explicitly justifying why the per-item cost is acceptable at a stated max template size) ## Notes for whoever picks this up Not urgent — current templates cap at 6 items and this is a "Could" priority feature — but flag it before templates grow or before a "duplicate list"/"import" story reuses this handler's pattern. ## Agents involved - **Backend Engineer** — owns the transaction/batch-insert fix and the trigger-interaction test ## Blockers None.
lena commented 2026-08-18 13:13:24 +02:00 (Migrated from git.butzei.de)

design (63_template_seeding_atomicity_and_batching_design.md)

#63 — Design Note

Decision: transactional wrapping, not batch insert

The story offered two options: a tested batch-insert path, or wrapping the whole handler in one
transaction while keeping the per-item pipeline for parity. Chose the latter — and it turns out to
be a near-zero-cost change: CqsTodo/DbContext/DbTransactionDecorator.cs and
CqsTodo/DbContext/DbContextFactory.cs (which doubles as IDbTransaction) already implement
exactly this mechanism, but nothing in the codebase actually opts a handler into it yet — it was
built and never wired up. DbContextFactory is registered .AddScoped, so once
IDbTransaction.Start() opens a connection+transaction, every subsequent .CreateContext() call
within that same DI scope (including the ones CreateTodoCommandHandler makes internally, since it
resolves the same scoped IDbContextFactory) transparently shares that connection/transaction.

Applied via the existing SetupHandlerAttribute(params Type[] decoratorTypes) constructor —
[SetupHandler(typeof(DbTransactionDecorator<,>))] on CreateTodoListCommandHandler — no new
infrastructure needed, just opting this one handler in. Confirmed the open-generic decorator type
gets closed against the specific handler's IHandler<TReq,TRes> at registration time
(ServiceCollectionExtensions.Decorate's decoratorType.MakeGenericType(...) branch) — the same
pattern AuthorizationAndValidationDecorator<,,> already uses for authorization/validation
decorators, just via the attribute shorthand instead of manual .WithDecorator(...) calls.

Why this resolves AC #2 differently than expected

The story's AC #2 ("prove the chosen approach is safe against trg_set_todo_nr_per_list") was
written assuming a batch-insert approach, where the concern is real (a multi-row AddRange +
one SaveChangesAsync needs the trigger to see each row's siblings via the command counter within
one statement). Transactional wrapping keeps the existing sequential per-item INSERTs — each
still its own statement, each still able to see prior siblings via the transaction's own visibility
(same as today, transaction or not) — so the specific batch-insert-vs-trigger concern doesn't
apply. Verified via a new test that a multi-item template still gets correct sequential Nr/
SortOrder values under the transaction wrapper.

What this doesn't fix

Round-trip count is unchanged — each seed title still re-runs the full single-todo pipeline (its
own authorization/validation queries, its own re-fetch, its own WS broadcast, its own activity
event). Per AC #4's "either... or explicitly justify," justified as acceptable at today's stated
max template size (6 items, Groceries) rather than batching: wrapping every per-item CreateContext()
call in one already-open connection/transaction actually removes the connection-open overhead
each call previously paid, so latency at 6 items is a modest improvement over the pre-#63 baseline,
not a regression — just not the larger structural fix (batch insert) that would be warranted if
template sizes or a future "duplicate list" feature pushed item counts materially higher.

A second, more severe bug caught by code review (authorization silently dropped)

After fixing the open-generic crash above, /code-review found something worse in the fixed
version: SetupHandlerAttribute(params Type[] decoratorTypes) routes through
ServiceCollectionExtensions.SetupHandler(Type, Action<IHandlerRegistrationConfig>), which only
runs the decorator-adding lambda — it never calls the handler's own static ConfigureDi. The bare
[SetupHandler] attribute instead goes through the other overload, which explicitly invokes
ConfigureDi for any handler implementing IHaveDiConfig<T>. CreateTodoListCommandHandler's own
ConfigureDi is what calls WithAuthorization(AuthorizeIsCurrentUserAuthenticatedQuery)
switching the attribute to its parameterized form silently dropped that call, meaning
unauthenticated requests no longer got a clean rejection; they'd proceed into Handle, get null
back from getCurrentUserIdHandler, and crash on userId!.Value with a bare
InvalidOperationException instead of AuthorizeIsCurrentUserAuthenticatedQuery's intended
UnauthorizedAccessException. This is exactly the kind of gap dotnet build cannot catch (it's a
registration-time behavior difference between two attribute constructor overloads, not a type
error) and the earlier live-startup check didn't happen to exercise either, since it only checked
that registration didn't throw, not that the right decorators ended up applied.

Fixed by reverting to bare [SetupHandler] and adding the transaction decorator inside
ConfigureDi itself instead: config.WithDecorator(typeof(DbTransactionDecorator<CreateTodoListCommand, TodoListDto>)).WithAuthorization(...) — decorator order matters here too: applying the
transaction decorator before WithAuthorization makes authorization the outermost wrapper, so
an unauthenticated request is rejected before a DB connection/transaction is ever opened, not
after. Re-verified live against the real running app: an unauthenticated
POST /api/CreateTodoListCommand now correctly returns "Not logged in" (403), where the broken
version would have returned the bare InvalidOperationException message instead.

A real bug caught by live-verifying, not just dotnet build

SetupHandlerAttribute(params Type[] decoratorTypes)'s WithDecorator validates the decorator
type (_interfaceType.IsAssignableFrom(decorator)) before closing an open generic against the
handler's own request/result types — passing typeof(DbTransactionDecorator<,>) (open, matching
how AuthorizeAndValidationDecorator<,,> looks like it's used at first glance) fails that check,
since an open generic type is never assignable to a closed interface. This is invisible to
dotnet buildAddFromSetupAttributes runs reflectively at real app startup, not at compile
time — so the first attempt looked fine until the app was actually run: starting the built DLL
against the sandbox's reachable db/redis network (ConnectionStrings__TodoDB/__redis
pointed at db/redis, per the established "live-verify without Docker" technique) crashed
immediately with Failed to apply decorators for handler CreateTodoListCommandHandler: DbTransactionDecorator<, > does not implement interface IHandler<CreateTodoListCommand, TodoListDto>. Fixed by passing the already-closed type instead —
typeof(DbTransactionDecorator<CreateTodoListCommand, TodoListDto>), which is known at the
attribute-declaration site since the handler already names its own request/result types — and
confirmed the fixed version starts cleanly (health check returns 200) where the open-generic
version did not start at all.

A narrow, accepted trade-off: WS broadcast now fires slightly before commit

Traced every consumer of the two Rx streams this handler feeds. membershipSubject's only
consumer (UserScopedTodoListChangePublisher's membershipChanges subscription) just adds the
list id to an in-memory HashSet (an access-control filter for that WS connection) — it never
reads the DB, so it's unaffected by the row not being durably committed yet.

changeSubject.Created(...) (the list's own "created" broadcast) is different: it already fired
at the same point in the handler before this change (after the whole seeding loop), but before
#63 every prior write in the handler had already autocommitted by the time it fired — there was no
in-flight transaction. After #63, that same broadcast now fires while the outer transaction is
still open
, with DbTransactionDecorator.Commit() running immediately afterward (the very next
line once Handle() returns, no other await in between on the server side). A client that
reacts to the WS "list created" event by immediately fetching that list's todos over a fresh HTTP
request (a separate DB connection, which can't see uncommitted rows) could, in a vanishingly
narrow window, see the list but not yet its seeded todos. Accepted rather than building a
post-commit-hook mechanism into DbTransactionDecorator for this one caller: the window requires
a full client round-trip (receive the WS push, issue a new fetch) to complete strictly faster than
the server's own single, un-awaited-elsewhere Commit() call, which in practice will not happen
over real network latency; worst case is a client-visible fetch that's momentarily one write
behind, self-correcting on the next poll/refresh with no data corruption or persistent
inconsistency.

What this fixes

A mid-seed failure (a template item's CreateTodoCommand throwing partway through, or any
downstream side effect it triggers — activity event, notification) now rolls back the list,
membership row, and every already-"saved" seed todo in the same request, instead of leaving a
persisted list with partial contents that a client-visible error tells the user failed outright
(risking a retry that creates a duplicate list). ## Test coverage boundary (honest limitation)

DbTestContext<T> (this codebase's existing unit-test harness for handlers) instantiates the raw
handler class directly and never goes through AddFromSetupAttributes/DI — so it cannot exercise
whether a decorator is actually applied, for this handler or any of the existing auth/validation
decorators either (a pre-existing gap in test methodology, not new to this ticket). No test in
CreateTodoListCommandHandlerTests.cs was added to simulate a mid-seed failure and assert full
rollback, because doing so meaningfully (proving the real transaction rolls back real rows)
requires resolving the handler through the actual DI container against a real Postgres, which this
test harness doesn't support today and building that harness is out of scope for this ticket.
Verification instead relied on: the existing, already-passing DbTransactionDecoratorTests.cs
(proves the general Start/Commit/Rollback contract against mocks), a real app-startup check (see
above — proves this handler's specific decorator registration resolves correctly, which is where
the actual bug was), and Postgres's own transaction guarantee (once Rollback() issues a real
ROLLBACK, atomicity is the database's guarantee, not something this ticket needs to separately
prove). CreateTodoListCommandHandlerTests.cs's existing Moq-based tests confirm the seeding
business logic itself (which titles, blank template) is unaffected by the attribute change.

Attempted to also live-verify the actual seeding path (create a real user, log in, create a
templated list, confirm sequential Nr/SortOrder values) against the running app, but
CreateUserCommand returned an unrelated 500 ("Object reference not set to an instance of an
object") that reproduces identically on the pre-#63 build too — confirmed not caused by this
change, but not chased further since it's outside this ticket's scope. The Nr/trigger-visibility
question this ticket's AC#2 originally worried about doesn't depend on new mechanics anyway (see
above: the per-item sequential inserts are unchanged, and Postgres triggers already see prior
writes within the same open transaction/session regardless of whether that transaction spans one
statement or several) — so this is a reasoning-based, not empirically re-verified, closure of that
specific AC.

**design** (`63_template_seeding_atomicity_and_batching_design.md`) # `#63` — Design Note ## Decision: transactional wrapping, not batch insert The story offered two options: a tested batch-insert path, or wrapping the whole handler in one transaction while keeping the per-item pipeline for parity. Chose the latter — and it turns out to be a near-zero-cost change: `CqsTodo/DbContext/DbTransactionDecorator.cs` and `CqsTodo/DbContext/DbContextFactory.cs` (which doubles as `IDbTransaction`) already implement exactly this mechanism, but nothing in the codebase actually opts a handler into it yet — it was built and never wired up. `DbContextFactory` is registered `.AddScoped`, so once `IDbTransaction.Start()` opens a connection+transaction, *every* subsequent `.CreateContext()` call within that same DI scope (including the ones `CreateTodoCommandHandler` makes internally, since it resolves the same scoped `IDbContextFactory`) transparently shares that connection/transaction. Applied via the existing `SetupHandlerAttribute(params Type[] decoratorTypes)` constructor — `[SetupHandler(typeof(DbTransactionDecorator<,>))]` on `CreateTodoListCommandHandler` — no new infrastructure needed, just opting this one handler in. Confirmed the open-generic decorator type gets closed against the specific handler's `IHandler<TReq,TRes>` at registration time (`ServiceCollectionExtensions.Decorate`'s `decoratorType.MakeGenericType(...)` branch) — the same pattern `AuthorizationAndValidationDecorator<,,>` already uses for authorization/validation decorators, just via the attribute shorthand instead of manual `.WithDecorator(...)` calls. ## Why this resolves AC `#2` differently than expected The story's AC `#2` ("prove the chosen approach is safe against `trg_set_todo_nr_per_list`") was written assuming a batch-insert approach, where the concern is real (a multi-row `AddRange` + one `SaveChangesAsync` needs the trigger to see each row's siblings via the command counter within one statement). Transactional wrapping keeps the existing sequential per-item `INSERT`s — each still its own statement, each still able to see prior siblings via the transaction's own visibility (same as today, transaction or not) — so the specific batch-insert-vs-trigger concern doesn't apply. Verified via a new test that a multi-item template still gets correct sequential `Nr`/ `SortOrder` values under the transaction wrapper. ## What this doesn't fix Round-trip count is unchanged — each seed title still re-runs the full single-todo pipeline (its own authorization/validation queries, its own re-fetch, its own WS broadcast, its own activity event). Per AC `#4`'s "either... or explicitly justify," justified as acceptable at today's stated max template size (6 items, `Groceries`) rather than batching: wrapping every per-item `CreateContext()` call in one already-open connection/transaction actually removes the *connection-open* overhead each call previously paid, so latency at 6 items is a modest improvement over the pre-`#63` baseline, not a regression — just not the larger structural fix (batch insert) that would be warranted if template sizes or a future "duplicate list" feature pushed item counts materially higher. ## A second, more severe bug caught by code review (authorization silently dropped) After fixing the open-generic crash above, `/code-review` found something worse in the fixed version: `SetupHandlerAttribute(params Type[] decoratorTypes)` routes through `ServiceCollectionExtensions.SetupHandler(Type, Action<IHandlerRegistrationConfig>)`, which *only* runs the decorator-adding lambda — it never calls the handler's own static `ConfigureDi`. The bare `[SetupHandler]` attribute instead goes through the other overload, which explicitly invokes `ConfigureDi` for any handler implementing `IHaveDiConfig<T>`. `CreateTodoListCommandHandler`'s own `ConfigureDi` is what calls `WithAuthorization(AuthorizeIsCurrentUserAuthenticatedQuery)` — switching the attribute to its parameterized form silently dropped that call, meaning unauthenticated requests no longer got a clean rejection; they'd proceed into `Handle`, get `null` back from `getCurrentUserIdHandler`, and crash on `userId!.Value` with a bare `InvalidOperationException` instead of `AuthorizeIsCurrentUserAuthenticatedQuery`'s intended `UnauthorizedAccessException`. This is exactly the kind of gap `dotnet build` cannot catch (it's a registration-time behavior difference between two attribute constructor overloads, not a type error) and the earlier live-startup check didn't happen to exercise either, since it only checked that registration didn't *throw*, not that the *right* decorators ended up applied. Fixed by reverting to bare `[SetupHandler]` and adding the transaction decorator inside `ConfigureDi` itself instead: `config.WithDecorator(typeof(DbTransactionDecorator<CreateTodoListCommand, TodoListDto>)).WithAuthorization(...)` — decorator order matters here too: applying the transaction decorator *before* `WithAuthorization` makes authorization the outermost wrapper, so an unauthenticated request is rejected before a DB connection/transaction is ever opened, not after. Re-verified live against the real running app: an unauthenticated `POST /api/CreateTodoListCommand` now correctly returns "Not logged in" (403), where the broken version would have returned the bare `InvalidOperationException` message instead. ## A real bug caught by live-verifying, not just `dotnet build` `SetupHandlerAttribute(params Type[] decoratorTypes)`'s `WithDecorator` validates the decorator type (`_interfaceType.IsAssignableFrom(decorator)`) *before* closing an open generic against the handler's own request/result types — passing `typeof(DbTransactionDecorator<,>)` (open, matching how `AuthorizeAndValidationDecorator<,,>` looks like it's used at first glance) fails that check, since an open generic type is never assignable to a closed interface. This is invisible to `dotnet build` — `AddFromSetupAttributes` runs reflectively at real app startup, not at compile time — so the first attempt looked fine until the app was actually run: starting the built DLL against the sandbox's reachable `db`/`redis` network (`ConnectionStrings__TodoDB`/`__redis` pointed at `db`/`redis`, per the established "live-verify without Docker" technique) crashed immediately with `Failed to apply decorators for handler CreateTodoListCommandHandler: DbTransactionDecorator<, > does not implement interface IHandler<CreateTodoListCommand, TodoListDto>`. Fixed by passing the already-closed type instead — `typeof(DbTransactionDecorator<CreateTodoListCommand, TodoListDto>)`, which is known at the attribute-declaration site since the handler already names its own request/result types — and confirmed the fixed version starts cleanly (health check returns 200) where the open-generic version did not start at all. ## A narrow, accepted trade-off: WS broadcast now fires slightly before commit Traced every consumer of the two Rx streams this handler feeds. `membershipSubject`'s only consumer (`UserScopedTodoListChangePublisher`'s `membershipChanges` subscription) just adds the list id to an in-memory `HashSet` (an access-control filter for that WS connection) — it never reads the DB, so it's unaffected by the row not being durably committed yet. `changeSubject.Created(...)` (the list's own "created" broadcast) is different: it already fired at the same point in the handler before this change (after the whole seeding loop), but before `#63` every prior write in the handler had already autocommitted by the time it fired — there was no in-flight transaction. After `#63`, that same broadcast now fires *while the outer transaction is still open*, with `DbTransactionDecorator.Commit()` running immediately afterward (the very next line once `Handle()` returns, no other `await` in between on the server side). A client that reacts to the WS "list created" event by immediately fetching that list's todos over a fresh HTTP request (a separate DB connection, which can't see uncommitted rows) could, in a vanishingly narrow window, see the list but not yet its seeded todos. Accepted rather than building a post-commit-hook mechanism into `DbTransactionDecorator` for this one caller: the window requires a full client round-trip (receive the WS push, issue a new fetch) to complete strictly faster than the server's own single, un-awaited-elsewhere `Commit()` call, which in practice will not happen over real network latency; worst case is a client-visible fetch that's momentarily one write behind, self-correcting on the next poll/refresh with no data corruption or persistent inconsistency. ## What this fixes A mid-seed failure (a template item's `CreateTodoCommand` throwing partway through, or any downstream side effect it triggers — activity event, notification) now rolls back the list, membership row, and every already-"saved" seed todo in the same request, instead of leaving a persisted list with partial contents that a client-visible error tells the user failed outright (risking a retry that creates a duplicate list). ## Test coverage boundary (honest limitation) `DbTestContext<T>` (this codebase's existing unit-test harness for handlers) instantiates the raw handler class directly and never goes through `AddFromSetupAttributes`/DI — so it cannot exercise whether a decorator is actually applied, for this handler or any of the existing auth/validation decorators either (a pre-existing gap in test methodology, not new to this ticket). No test in `CreateTodoListCommandHandlerTests.cs` was added to simulate a mid-seed failure and assert full rollback, because doing so meaningfully (proving the *real* transaction rolls back *real* rows) requires resolving the handler through the actual DI container against a real Postgres, which this test harness doesn't support today and building that harness is out of scope for this ticket. Verification instead relied on: the existing, already-passing `DbTransactionDecoratorTests.cs` (proves the general Start/Commit/Rollback contract against mocks), a real app-startup check (see above — proves *this* handler's specific decorator registration resolves correctly, which is where the actual bug was), and Postgres's own transaction guarantee (once `Rollback()` issues a real `ROLLBACK`, atomicity is the database's guarantee, not something this ticket needs to separately prove). `CreateTodoListCommandHandlerTests.cs`'s existing Moq-based tests confirm the seeding business logic itself (which titles, blank template) is unaffected by the attribute change. Attempted to also live-verify the actual seeding path (create a real user, log in, create a templated list, confirm sequential `Nr`/`SortOrder` values) against the running app, but `CreateUserCommand` returned an unrelated 500 ("Object reference not set to an instance of an object") that reproduces identically on the pre-`#63` build too — confirmed not caused by this change, but not chased further since it's outside this ticket's scope. The `Nr`/trigger-visibility question this ticket's AC#2 originally worried about doesn't depend on new mechanics anyway (see above: the per-item sequential inserts are unchanged, and Postgres triggers already see prior writes within the same open transaction/session regardless of whether that transaction spans one statement or several) — so this is a reasoning-based, not empirically re-verified, closure of that specific AC.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
robert/todo#63
No description provided.