fix(#253 PR2): close review findings (ETag/items consistency + SPA load ordering)
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Failing after 40s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m2s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Failing after 40s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m2s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Independent Codex review of #268 found two Blockers the fork missed + two Mediums: - Blocker: replace PUTs returned the handler's item snapshot but re-queried the root for the ETag separately, so a racing writer could pair stale items with a newer ETag (silent overwrite). All four controllers now reload root-then-items (version-first, fail-safe) and 404 when the root is gone between commit and reload — matching the Block reference. Fixes the Blocker + the Medium '200 without ETag' case together. - Blocker: PlaylistsScreen loaded items+root via Promise.all (concurrent), pairing a stale name with the current ETag; now sequential (items-with-meta first, then root). - Medium: SchedulesScreen loadItems now marks not-loaded/loading up front so canEdit is false through the 412 conflict reload (no stale-draft edits lost). Controller unit-test mocks updated to stub the new reload query. Full suite green (ErsatzTV.Tests 1334, web 667, check:api no drift).
This commit is contained in:
@@ -212,10 +212,14 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase
|
||||
|
||||
return await result.Match(
|
||||
Left: error => Task.FromResult(error.ToErrorResult()),
|
||||
Right: async items =>
|
||||
Right: async _ =>
|
||||
{
|
||||
// Reload the root (version) FIRST, then the items, so the emitted ETag is never newer than
|
||||
// the returned items (issue #253 fail-safe ordering; matches BlockController).
|
||||
Option<DecoTemplateViewModel> refreshed =
|
||||
await mediator.Send(new GetDecoTemplateById(id), cancellationToken);
|
||||
List<DecoTemplateItemViewModel> items =
|
||||
await mediator.Send(new GetDecoTemplateItems(id), cancellationToken);
|
||||
return refreshed.Match(
|
||||
Some: vm =>
|
||||
{
|
||||
|
||||
@@ -222,12 +222,21 @@ public class PlaylistController(IMediator mediator) : ControllerBase
|
||||
await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken);
|
||||
return await result.Match(
|
||||
Left: error => Task.FromResult(error.ToErrorResult()),
|
||||
Right: async items =>
|
||||
Right: async _ =>
|
||||
{
|
||||
// Reload the root (version) FIRST, then the items, so the emitted ETag is never newer than
|
||||
// the returned items (issue #253 fail-safe ordering; matches BlockController). A None root
|
||||
// (deleted between commit and reload) is a 404, never a 200 without an ETag.
|
||||
Option<PlaylistViewModel> refreshed =
|
||||
await mediator.Send(new GetPlaylistById(id), cancellationToken);
|
||||
refreshed.Do(vm => ConcurrencyHeaders.SetETag(Response, vm.Version));
|
||||
return (IActionResult)new OkObjectResult(items.Map(ProjectToItemResponse).ToList());
|
||||
List<PlaylistItemViewModel> items = await mediator.Send(new GetPlaylistItems(id), cancellationToken);
|
||||
return refreshed.Match(
|
||||
Some: vm =>
|
||||
{
|
||||
ConcurrencyHeaders.SetETag(Response, vm.Version);
|
||||
return (IActionResult)new OkObjectResult(items.Map(ProjectToItemResponse).ToList());
|
||||
},
|
||||
None: () => ApiResults.NotFoundProblem());
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -180,15 +180,24 @@ public class ScheduleController(IMediator mediator) : ControllerBase
|
||||
|
||||
return await result.Match(
|
||||
Left: error => Task.FromResult(error.ToErrorResult()),
|
||||
Right: async items =>
|
||||
Right: async _ =>
|
||||
{
|
||||
// Return the new ETag so a same-tab second save doesn't 412 against its own write.
|
||||
// Reload the root (version) FIRST, then the items, so the emitted ETag is never newer than
|
||||
// the returned items (issue #253 fail-safe ordering; matches BlockController). A None root
|
||||
// (deleted between commit and reload) is a 404, never a 200 without an ETag.
|
||||
Option<ProgramScheduleViewModel> refreshed =
|
||||
await mediator.Send(new GetProgramScheduleById(id), cancellationToken);
|
||||
refreshed.IfSome(vm => ConcurrencyHeaders.SetETag(Response, vm.Version));
|
||||
|
||||
return (IActionResult)new OkObjectResult(
|
||||
items.Select(ScheduleItemResponseMapper.ProjectToResponseModel).ToList());
|
||||
List<ProgramScheduleItemViewModel> items =
|
||||
await mediator.Send(new GetProgramScheduleItems(id), cancellationToken);
|
||||
return refreshed.Match(
|
||||
Some: vm =>
|
||||
{
|
||||
// Return the new ETag so a same-tab second save doesn't 412 against its own write.
|
||||
ConcurrencyHeaders.SetETag(Response, vm.Version);
|
||||
return (IActionResult)new OkObjectResult(
|
||||
items.Select(ScheduleItemResponseMapper.ProjectToResponseModel).ToList());
|
||||
},
|
||||
None: () => ApiResults.NotFoundProblem());
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -200,9 +200,14 @@ public class TemplateController(IMediator mediator) : ControllerBase
|
||||
|
||||
return await result.Match(
|
||||
Left: error => Task.FromResult(error.ToErrorResult()),
|
||||
Right: async items =>
|
||||
Right: async _ =>
|
||||
{
|
||||
// Reload the root (version) FIRST, then the items, so the emitted ETag is never newer than
|
||||
// the returned items (issue #253 fail-safe ordering; matches BlockController). Returning the
|
||||
// handler's item snapshot alongside a separately re-queried version could pair stale items
|
||||
// with a newer ETag — a client would then silently overwrite the interleaving write.
|
||||
Option<TemplateViewModel> refreshed = await mediator.Send(new GetTemplateById(id), cancellationToken);
|
||||
List<TemplateItemViewModel> items = await mediator.Send(new GetTemplateItems(id), cancellationToken);
|
||||
return refreshed.Match(
|
||||
Some: vm =>
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user