diff --git a/ErsatzTV.Application/Emby/Commands/SynchronizeEmbyCollections.cs b/ErsatzTV.Application/Emby/Commands/SynchronizeEmbyCollections.cs index b6511c5d1..f9b9c8ce4 100644 --- a/ErsatzTV.Application/Emby/Commands/SynchronizeEmbyCollections.cs +++ b/ErsatzTV.Application/Emby/Commands/SynchronizeEmbyCollections.cs @@ -2,6 +2,6 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Emby; -public record SynchronizeEmbyCollections(int EmbyMediaSourceId, bool ForceScan, bool DeepScan) +public record SynchronizeEmbyCollections(int EmbyMediaSourceId, bool ForceScan, bool DeepScan, bool Unlock = true) : IRequest>, IScannerBackgroundServiceRequest; diff --git a/ErsatzTV.Application/Jellyfin/Commands/SynchronizeJellyfinCollections.cs b/ErsatzTV.Application/Jellyfin/Commands/SynchronizeJellyfinCollections.cs index 705bd8c03..fc38efe68 100644 --- a/ErsatzTV.Application/Jellyfin/Commands/SynchronizeJellyfinCollections.cs +++ b/ErsatzTV.Application/Jellyfin/Commands/SynchronizeJellyfinCollections.cs @@ -2,6 +2,6 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Jellyfin; -public record SynchronizeJellyfinCollections(int JellyfinMediaSourceId, bool ForceScan, bool DeepScan) : +public record SynchronizeJellyfinCollections(int JellyfinMediaSourceId, bool ForceScan, bool DeepScan, bool Unlock = true) : IRequest>, IScannerBackgroundServiceRequest; diff --git a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs index 024b47be3..4b64fd73b 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs @@ -8,4 +8,4 @@ public enum QueueLibraryScanResult AlreadyScanning } -public record QueueLibraryScanByLibraryId(int LibraryId) : IRequest; +public record QueueLibraryScanByLibraryId(int LibraryId, bool DeepScan = false) : IRequest; diff --git a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs index 6bb6a5f4b..8782e6631 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs @@ -66,7 +66,7 @@ public class QueueLibraryScanByLibraryIdHandler( new SynchronizePlexLibraries(library.MediaSourceId), cancellationToken); await scannerWorker.WriteAsync( - new ForceSynchronizePlexLibraryById(library.Id, false), + new ForceSynchronizePlexLibraryById(library.Id, request.DeepScan), cancellationToken); break; case JellyfinLibrary: @@ -74,7 +74,7 @@ public class QueueLibraryScanByLibraryIdHandler( new SynchronizeJellyfinLibraries(library.MediaSourceId), cancellationToken); await scannerWorker.WriteAsync( - new ForceSynchronizeJellyfinLibraryById(library.Id, false), + new ForceSynchronizeJellyfinLibraryById(library.Id, request.DeepScan), cancellationToken); break; case EmbyLibrary: @@ -82,7 +82,7 @@ public class QueueLibraryScanByLibraryIdHandler( new SynchronizeEmbyLibraries(library.MediaSourceId), cancellationToken); await scannerWorker.WriteAsync( - new ForceSynchronizeEmbyLibraryById(library.Id, false), + new ForceSynchronizeEmbyLibraryById(library.Id, request.DeepScan), cancellationToken); break; } diff --git a/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryId.cs b/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryId.cs index e4b1bf194..886a76d19 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryId.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryId.cs @@ -1,3 +1,14 @@ namespace ErsatzTV.Application.Libraries; -public record QueueShowScanByLibraryId(int LibraryId, int ShowId, string ShowTitle, bool DeepScan) : IRequest; +public enum QueueShowScanResult +{ + Queued, + NotFound, + Unsupported, + SyncDisabled, + AlreadyScanning, + ScanFailed +} + +public record QueueShowScanByLibraryId(int LibraryId, int ShowId, string ShowTitle, bool DeepScan) + : IRequest; diff --git a/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryIdHandler.cs b/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryIdHandler.cs index 281606797..5d3c93f3c 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryIdHandler.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueShowScanByLibraryIdHandler.cs @@ -19,9 +19,9 @@ public class QueueShowScanByLibraryIdHandler( IMediator mediator, ChannelWriter workerChannel, ILogger logger) - : IRequestHandler + : IRequestHandler { - public async Task Handle(QueueShowScanByLibraryId request, CancellationToken cancellationToken) + public async Task Handle(QueueShowScanByLibraryId request, CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); @@ -42,14 +42,14 @@ public class QueueShowScanByLibraryIdHandler( if (!shouldSyncItems) { logger.LogWarning("Library sync is disabled for library id {Id}", library.Id); - return false; + return QueueShowScanResult.SyncDisabled; } - // Check if library is already being scanned - return false if locked + // A false from LockLibrary means a scan is already in progress; we own no release. if (!locker.LockLibrary(library.Id)) { logger.LogWarning("Library {Id} is already being scanned, cannot scan individual show", library.Id); - return false; + return QueueShowScanResult.AlreadyScanning; } logger.LogDebug( @@ -60,41 +60,43 @@ public class QueueShowScanByLibraryIdHandler( try { - var success = false; + QueueShowScanResult outcome; switch (library) { case PlexLibrary: Either plexResult = await mediator.Send( new SynchronizePlexShowById(library.Id, request.ShowId, request.DeepScan), cancellationToken); - success = plexResult.IsRight; + outcome = plexResult.IsRight ? QueueShowScanResult.Queued : QueueShowScanResult.ScanFailed; break; case JellyfinLibrary: Either jellyfinResult = await mediator.Send( new SynchronizeJellyfinShowById(library.Id, request.ShowId, request.DeepScan), cancellationToken); - success = jellyfinResult.IsRight; + outcome = jellyfinResult.IsRight ? QueueShowScanResult.Queued : QueueShowScanResult.ScanFailed; break; case EmbyLibrary: Either embyResult = await mediator.Send( new SynchronizeEmbyShowById(library.Id, request.ShowId, request.DeepScan), cancellationToken); - success = embyResult.IsRight; + outcome = embyResult.IsRight ? QueueShowScanResult.Queued : QueueShowScanResult.ScanFailed; break; case LocalLibrary: logger.LogWarning("Single show scanning is not supported for local libraries"); + outcome = QueueShowScanResult.Unsupported; break; default: logger.LogWarning("Unknown library type for library {Id}", library.Id); + outcome = QueueShowScanResult.Unsupported; break; } - if (success && request.DeepScan) + if (outcome == QueueShowScanResult.Queued && request.DeepScan) { await workerChannel.WriteAsync(new ExtractEmbeddedShowSubtitles(request.ShowId), cancellationToken); } - return success; + return outcome; } finally { @@ -103,6 +105,6 @@ public class QueueShowScanByLibraryIdHandler( } } - return false; + return QueueShowScanResult.NotFound; } } diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddEpisodeToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddEpisodeToPlaylistHandler.cs index 43c0cac87..05cad00de 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddEpisodeToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddEpisodeToPlaylistHandler.cs @@ -32,6 +32,10 @@ public class AddEpisodeToPlaylistHandler(IDbContextFactory dbContextF }; parameters.Playlist.Items.Add(playlistItem); + + // Mutates the playlist's editor-visible item list, so bump the concurrency token (issue #253). + parameters.Playlist.Version++; + await dbContext.SaveChangesAsync(); return Unit.Default; } diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs index fd8586c21..f6ab29b8b 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs @@ -69,6 +69,9 @@ public class AddItemsToPlaylistHandler : IRequestHandler dbContextFac }; parameters.Playlist.Items.Add(playlistItem); + + // Mutates the playlist's editor-visible item list, so bump the concurrency token (issue #253). + parameters.Playlist.Version++; + await dbContext.SaveChangesAsync(); return Unit.Default; } diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddSeasonToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddSeasonToPlaylistHandler.cs index 40519bf6c..e9873bc46 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddSeasonToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddSeasonToPlaylistHandler.cs @@ -32,6 +32,10 @@ public class AddSeasonToPlaylistHandler(IDbContextFactory dbContextFa }; parameters.Playlist.Items.Add(playlistItem); + + // Mutates the playlist's editor-visible item list, so bump the concurrency token (issue #253). + parameters.Playlist.Version++; + await dbContext.SaveChangesAsync(); return Unit.Default; } diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddShowToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddShowToPlaylistHandler.cs index 2795fd711..3159988ff 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddShowToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddShowToPlaylistHandler.cs @@ -32,6 +32,10 @@ public class AddShowToPlaylistHandler(IDbContextFactory dbContextFact }; parameters.Playlist.Items.Add(playlistItem); + + // Mutates the playlist's editor-visible item list, so bump the concurrency token (issue #253). + parameters.Playlist.Version++; + await dbContext.SaveChangesAsync(); return Unit.Default; } diff --git a/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItems.cs b/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItems.cs index 74efb5729..caffd1928 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItems.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItems.cs @@ -2,5 +2,9 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.MediaCollections; -public record ReplacePlaylistItems(int PlaylistId, string Name, List Items) +public record ReplacePlaylistItems( + int PlaylistId, + string Name, + List Items, + Option ExpectedVersion = default) : IRequest>>; diff --git a/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItemsHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItemsHandler.cs index b55436918..85d26a50b 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItemsHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/ReplacePlaylistItemsHandler.cs @@ -15,10 +15,21 @@ public class ReplacePlaylistItemsHandler(IDbContextFactory dbContextF { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); Validation validation = await Validate(dbContext, request, cancellationToken); - return await validation.Apply(ps => Persist(dbContext, request, ps, cancellationToken)); + + // Introduce the optimistic-concurrency check as a standalone Either AFTER the validation + // pipeline (never via Apply) so a PreconditionFailedError survives to a 412 and is not + // flattened to a generic 422 by Join() (issue #253 / api-conventions §7a). + // LanguageExtensions.ToEither joins the Seq to a single BaseError (the native + // Validation.ToEither() would keep Seq and is called explicitly here to avoid that shadow). + Either validated = LanguageExtensions.ToEither(validation) + .Bind(playlist => playlist.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: playlist => Persist(dbContext, request, playlist, cancellationToken), + Left: error => Task.FromResult>>(error)); } - private static async Task> Persist( + private static async Task>> Persist( TvContext dbContext, ReplacePlaylistItems request, Playlist playlist, @@ -30,9 +41,15 @@ public class ReplacePlaylistItemsHandler(IDbContextFactory dbContextF dbContext.RemoveRange(playlist.Items); playlist.Items = request.Items.Map(i => BuildItem(playlist, i.Index, i)).ToList(); - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump: EF emits the root UPDATE only when a scalar actually differs, so a + // same-value/no-op save would otherwise write no root row and neither fire the concurrency + // token nor rotate other clients' ETags. Bumping guarantees both on every save (issue #253). + playlist.Version++; - return playlist.Items.Map(Mapper.ProjectToViewModel).ToList(); + // Save through the guard so an EF concurrency failure (a racing writer won between our load + // and save) maps to 412 rather than surfacing as a 500. + Either saved = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + return saved.Map(_ => playlist.Items.Map(Mapper.ProjectToViewModel).ToList()); } private static PlaylistItem BuildItem(Playlist playlist, int index, ReplacePlaylistItem item) => diff --git a/ErsatzTV.Application/MediaCollections/Mapper.cs b/ErsatzTV.Application/MediaCollections/Mapper.cs index 930864e67..c724bcde4 100644 --- a/ErsatzTV.Application/MediaCollections/Mapper.cs +++ b/ErsatzTV.Application/MediaCollections/Mapper.cs @@ -92,7 +92,7 @@ internal static class Mapper new(playlistGroup.Id, playlistGroup.Name, playlistGroup.Playlists.Count, playlistGroup.IsSystem); internal static PlaylistViewModel ProjectToViewModel(Playlist playlist) => - new(playlist.Id, playlist.PlaylistGroupId, playlist.Name, playlist.IsSystem); + new(playlist.Id, playlist.PlaylistGroupId, playlist.Name, playlist.IsSystem, playlist.Version); internal static PlaylistItemViewModel ProjectToViewModel(PlaylistItem playlistItem) => new( diff --git a/ErsatzTV.Application/MediaCollections/PlaylistViewModel.cs b/ErsatzTV.Application/MediaCollections/PlaylistViewModel.cs index 0b57c33ae..febb7e254 100644 --- a/ErsatzTV.Application/MediaCollections/PlaylistViewModel.cs +++ b/ErsatzTV.Application/MediaCollections/PlaylistViewModel.cs @@ -1,3 +1,3 @@ namespace ErsatzTV.Application.MediaCollections; -public record PlaylistViewModel(int Id, int PlaylistGroupId, string Name, bool IsSystem); +public record PlaylistViewModel(int Id, int PlaylistGroupId, string Name, bool IsSystem, int Version); diff --git a/ErsatzTV.Application/Playouts/Commands/ResetAllPlayouts.cs b/ErsatzTV.Application/Playouts/Commands/ResetAllPlayouts.cs index dbf41139d..79967ab48 100644 --- a/ErsatzTV.Application/Playouts/Commands/ResetAllPlayouts.cs +++ b/ErsatzTV.Application/Playouts/Commands/ResetAllPlayouts.cs @@ -1,3 +1,3 @@ namespace ErsatzTV.Application.Playouts; -public record ResetAllPlayouts : IRequest; +public record ResetAllPlayouts : IRequest; diff --git a/ErsatzTV.Application/Playouts/Commands/ResetAllPlayoutsHandler.cs b/ErsatzTV.Application/Playouts/Commands/ResetAllPlayoutsHandler.cs index c99a127f6..e997bedb7 100644 --- a/ErsatzTV.Application/Playouts/Commands/ResetAllPlayoutsHandler.cs +++ b/ErsatzTV.Application/Playouts/Commands/ResetAllPlayoutsHandler.cs @@ -11,33 +11,49 @@ public class ResetAllPlayoutsHandler( IEntityLocker locker, ChannelWriter channel, IDbContextFactory dbContextFactory) - : IRequestHandler + : IRequestHandler { - public async Task Handle(ResetAllPlayouts request, CancellationToken cancellationToken) + public async Task Handle( + ResetAllPlayouts request, + CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + var queued = new List(); + var skippedLocked = new List(); + var skippedUnsupported = new List(); + foreach (Playout playout in await dbContext.Playouts.ToListAsync(cancellationToken)) { switch (playout.ScheduleKind) { case PlayoutScheduleKind.Classic: - if (!locker.IsPlayoutLocked(playout.Id)) + if (locker.IsPlayoutLocked(playout.Id)) + { + skippedLocked.Add(playout.Id); + } + else { await channel.WriteAsync( new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh), cancellationToken); + queued.Add(playout.Id); } break; case PlayoutScheduleKind.Block: case PlayoutScheduleKind.Sequential: case PlayoutScheduleKind.Scripted: - if (!locker.IsPlayoutLocked(playout.Id)) + if (locker.IsPlayoutLocked(playout.Id)) + { + skippedLocked.Add(playout.Id); + } + else { await channel.WriteAsync( new BuildPlayout(playout.Id, PlayoutBuildMode.Reset), cancellationToken); + queued.Add(playout.Id); } break; @@ -45,8 +61,11 @@ public class ResetAllPlayoutsHandler( case PlayoutScheduleKind.None: default: // external json cannot be reset + skippedUnsupported.Add(playout.Id); continue; } } + + return new ResetAllPlayoutsResult(queued, skippedLocked, skippedUnsupported); } } diff --git a/ErsatzTV.Application/Playouts/ResetAllPlayoutsResult.cs b/ErsatzTV.Application/Playouts/ResetAllPlayoutsResult.cs new file mode 100644 index 000000000..0d5fd9633 --- /dev/null +++ b/ErsatzTV.Application/Playouts/ResetAllPlayoutsResult.cs @@ -0,0 +1,6 @@ +namespace ErsatzTV.Application.Playouts; + +public record ResetAllPlayoutsResult( + List QueuedPlayoutIds, + List SkippedLocked, + List SkippedUnsupported); diff --git a/ErsatzTV.Application/Plex/Commands/SynchronizePlexCollections.cs b/ErsatzTV.Application/Plex/Commands/SynchronizePlexCollections.cs index b081770c9..ddefbcb63 100644 --- a/ErsatzTV.Application/Plex/Commands/SynchronizePlexCollections.cs +++ b/ErsatzTV.Application/Plex/Commands/SynchronizePlexCollections.cs @@ -2,5 +2,5 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Plex; -public record SynchronizePlexCollections(int PlexMediaSourceId, bool ForceScan, bool DeepScan) +public record SynchronizePlexCollections(int PlexMediaSourceId, bool ForceScan, bool DeepScan, bool Unlock = true) : IRequest>, IScannerBackgroundServiceRequest; diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs index b8656406a..93f2862b6 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/AddProgramScheduleItemHandler.cs @@ -52,6 +52,9 @@ public class AddProgramScheduleItemHandler : ProgramScheduleItemCommandBase, ProgramScheduleItem item = BuildItem(programSchedule, nextIndex, request); programSchedule.Items.Add(item); + // bump the optimistic-concurrency token so this config edit rotates other clients' ETags (#253) + programSchedule.Version++; + await dbContext.SaveChangesAsync(cancellationToken); // refresh any playouts that use this schedule diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs index 017f64ac5..5a610c97f 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/DeleteProgramScheduleItemHandler.cs @@ -34,6 +34,10 @@ public class DeleteProgramScheduleItemHandler( List playouts = item.ProgramSchedule.Playouts; dbContext.ProgramScheduleItems.Remove(item); + + // bump the optimistic-concurrency token so this config edit rotates other clients' ETags (#253) + item.ProgramSchedule.Version++; + await dbContext.SaveChangesAsync(cancellationToken); // post-commit side effect runs on CancellationToken.None so a late request cancellation diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItems.cs b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItems.cs index 0c3c0b8ec..9a9d4568c 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItems.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItems.cs @@ -44,5 +44,8 @@ public record ReplaceProgramScheduleItem( string PreferredSubtitleLanguageCode, ChannelSubtitleMode? SubtitleMode) : IProgramScheduleItemRequest; -public record ReplaceProgramScheduleItems(int ProgramScheduleId, List Items) : IRequest< +public record ReplaceProgramScheduleItems( + int ProgramScheduleId, + List Items, + Option ExpectedVersion = default) : IRequest< Either>>; diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs index f68ac5e73..43a626177 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/ReplaceProgramScheduleItemsHandler.cs @@ -26,13 +26,23 @@ public class ReplaceProgramScheduleItemsHandler( Some: async programSchedule => { Validation validation = await Validate(dbContext, request, programSchedule); - return await validation.Apply(ps => PersistItems(dbContext, request, ps, cancellationToken)); + + // Introduce the optimistic-concurrency check as a standalone Either AFTER the validation + // pipeline (never via Apply) so a PreconditionFailedError survives to a 412 and is not + // flattened to a generic 422 by Join() (issue #253 / api-conventions §7a). + Either validated = LanguageExtensions.ToEither(validation) + .Bind(ps => ps.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: ps => PersistItems(dbContext, request, ps, cancellationToken), + Left: error => + Task.FromResult>>(error)); }, None: () => Task.FromResult>>( new NotFoundError("[ProgramScheduleId] does not exist."))); } - private async Task> PersistItems( + private async Task>> PersistItems( TvContext dbContext, ReplaceProgramScheduleItems request, ProgramSchedule programSchedule, @@ -92,7 +102,20 @@ public class ReplaceProgramScheduleItemsHandler( programSchedule.Items.Add(BuildItem(programSchedule, i, orderedItems[i])); } - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump: this handler frequently saves with only CHILD changes and no root-scalar + // change, so without an explicit bump EF would emit no root UPDATE and the concurrency token + // would never fire (nor rotate other clients' ETags). Bumping guarantees both on every save, + // including a no-op same-items PUT-back (issue #253 / api-conventions §7a). + programSchedule.Version++; + + // Save through the guard so an EF concurrency failure (a racing writer won between our load and + // save) maps to 412 rather than surfacing as a 500. On failure, propagate the error WITHOUT + // running the post-save reload/enqueue below. + Either saved = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + if (saved.IsLeft) + { + return saved.Map(_ => (IEnumerable)[]); + } // refresh any playouts that use this schedule // post-commit side effect runs on CancellationToken.None so a late request cancellation diff --git a/ErsatzTV.Application/ProgramSchedules/Commands/UpdateProgramScheduleHandler.cs b/ErsatzTV.Application/ProgramSchedules/Commands/UpdateProgramScheduleHandler.cs index 9198db1a3..6500e9b5a 100644 --- a/ErsatzTV.Application/ProgramSchedules/Commands/UpdateProgramScheduleHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Commands/UpdateProgramScheduleHandler.cs @@ -54,6 +54,9 @@ public class UpdateProgramScheduleHandler( programSchedule.RandomStartPoint = request.RandomStartPoint; programSchedule.FixedStartTimeBehavior = request.FixedStartTimeBehavior; + // bump the optimistic-concurrency token so this config edit rotates other clients' ETags (#253) + programSchedule.Version++; + await dbContext.SaveChangesAsync(); if (needToRefreshPlayout) diff --git a/ErsatzTV.Application/ProgramSchedules/Mapper.cs b/ErsatzTV.Application/ProgramSchedules/Mapper.cs index 43460b45b..21f1821d0 100644 --- a/ErsatzTV.Application/ProgramSchedules/Mapper.cs +++ b/ErsatzTV.Application/ProgramSchedules/Mapper.cs @@ -12,7 +12,8 @@ internal static class Mapper programSchedule.TreatCollectionsAsShows, programSchedule.ShuffleScheduleItems, programSchedule.RandomStartPoint, - programSchedule.FixedStartTimeBehavior); + programSchedule.FixedStartTimeBehavior, + programSchedule.Version); internal static ProgramScheduleItemViewModel ProjectToViewModel(ProgramScheduleItem programScheduleItem) => programScheduleItem switch diff --git a/ErsatzTV.Application/ProgramSchedules/ProgramScheduleViewModel.cs b/ErsatzTV.Application/ProgramSchedules/ProgramScheduleViewModel.cs index c8750ad4e..1eee27acf 100644 --- a/ErsatzTV.Application/ProgramSchedules/ProgramScheduleViewModel.cs +++ b/ErsatzTV.Application/ProgramSchedules/ProgramScheduleViewModel.cs @@ -9,4 +9,5 @@ public record ProgramScheduleViewModel( bool TreatCollectionsAsShows, bool ShuffleScheduleItems, bool RandomStartPoint, - FixedStartTimeBehavior FixedStartTimeBehavior); + FixedStartTimeBehavior FixedStartTimeBehavior, + int Version); diff --git a/ErsatzTV.Application/ProgramSchedules/Queries/GetAllProgramSchedulesHandler.cs b/ErsatzTV.Application/ProgramSchedules/Queries/GetAllProgramSchedulesHandler.cs index c13f160eb..1abe7626f 100644 --- a/ErsatzTV.Application/ProgramSchedules/Queries/GetAllProgramSchedulesHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Queries/GetAllProgramSchedulesHandler.cs @@ -19,7 +19,8 @@ public class GetAllProgramSchedulesHandler(IDbContextFactory dbContex ps.TreatCollectionsAsShows, ps.ShuffleScheduleItems, ps.RandomStartPoint, - ps.FixedStartTimeBehavior)) + ps.FixedStartTimeBehavior, + ps.Version)) .ToListAsync(cancellationToken); } } diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItems.cs b/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItems.cs index 9669e5223..85c178b2d 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItems.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItems.cs @@ -6,5 +6,6 @@ public record ReplaceDecoTemplateItems( int DecoTemplateId, int DecoTemplateGroupId, string Name, - List Items) + List Items, + Option ExpectedVersion = default) : IRequest>>; diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItemsHandler.cs b/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItemsHandler.cs index 52fccc29d..d765ac468 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItemsHandler.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplaceDecoTemplateItemsHandler.cs @@ -20,10 +20,19 @@ public class ReplaceDecoTemplateItemsHandler( { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); Validation validation = await Validate(dbContext, request, cancellationToken); - return await validation.Apply(ps => Persist(dbContext, request, ps, cancellationToken)); + + // Introduce the optimistic-concurrency check as a standalone Either AFTER the validation + // pipeline (never via Apply) so a PreconditionFailedError survives to a 412 and is not + // flattened to a generic 422 by Join() (issue #253 / api-conventions §7a). + Either validated = LanguageExtensions.ToEither(validation) + .Bind(decoTemplate => decoTemplate.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: decoTemplate => Persist(dbContext, request, decoTemplate, cancellationToken), + Left: error => Task.FromResult>>(error)); } - private async Task> Persist( + private async Task>> Persist( TvContext dbContext, ReplaceDecoTemplateItems request, DecoTemplate decoTemplate, @@ -36,33 +45,49 @@ public class ReplaceDecoTemplateItemsHandler( decoTemplate.Items = request.Items.Map(i => BuildItem(decoTemplate, i)).ToList(); - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump: EF emits the root UPDATE only when a scalar actually differs, so a + // same-value/no-op save would otherwise write no root row and neither fire the concurrency + // token nor rotate other clients' ETags. Bumping guarantees both on every save (issue #253). + decoTemplate.Version++; - // Deco/break/default-filler content is only (re)applied during a Reset build (a Continue keeps the - // frozen DecoDefault filler items), and BlockKey change-detection has no deco dimension, so nothing - // self-heals a deco-template edit — the editor returned 200 but built filler stayed stale until a - // manual Reset (#251). Enqueue a Reset for every playout that references this deco template. This - // whole post-commit invalidation runs with CancellationToken.None (audit #22 policy): once the edit - // is committed, a late request cancellation must not be able to abort the affected-playout query OR - // the enqueue and leave content stale. - List playoutIds = await dbContext.PlayoutTemplates - .Where(pt => pt.DecoTemplateId == decoTemplate.Id) - .Select(pt => pt.PlayoutId) - .Distinct() - .ToListAsync(CancellationToken.None); + // Save through the guard so an EF concurrency failure (a racing writer won between our load + // and save) maps to 412 rather than surfacing as a 500. + Either saved = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + return await saved.Match( + Right: async _ => + { + // Deco/break/default-filler content is only (re)applied during a Reset build (a Continue keeps + // the frozen DecoDefault filler items), and BlockKey change-detection has no deco dimension, so + // nothing self-heals a deco-template edit — the editor returned 200 but built filler stayed + // stale until a manual Reset (#251). Enqueue a Reset for every playout that references this + // deco template. This whole post-commit invalidation runs with CancellationToken.None (audit + // #22 policy): once the edit is committed, a late request cancellation must not be able to + // abort the affected-playout query OR the enqueue and leave content stale. Only runs after a + // successful save (issue #253) — a 412/422 must not enqueue a Reset for content that was never + // persisted. + List playoutIds = await dbContext.PlayoutTemplates + .Where(pt => pt.DecoTemplateId == decoTemplate.Id) + .Select(pt => pt.PlayoutId) + .Distinct() + .ToListAsync(CancellationToken.None); - foreach (int playoutId in playoutIds) - { - await channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Reset), CancellationToken.None); - } + foreach (int playoutId in playoutIds) + { + await channel.WriteAsync( + new BuildPlayout(playoutId, PlayoutBuildMode.Reset), + CancellationToken.None); + } - await dbContext.Entry(decoTemplate) - .Collection(t => t.Items) - .Query() - .Include(i => i.Deco) - .LoadAsync(cancellationToken); + await dbContext.Entry(decoTemplate) + .Collection(t => t.Items) + .Query() + .Include(i => i.Deco) + .LoadAsync(cancellationToken); - return decoTemplate.Items.Map(Mapper.ProjectToViewModel).ToList(); + return Right>( + decoTemplate.Items.Map(Mapper.ProjectToViewModel).ToList()); + }, + Left: error => Task.FromResult(Left>(error))); } private static DecoTemplateItem BuildItem(DecoTemplate decoTemplate, ReplaceDecoTemplateItem item) => diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItems.cs b/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItems.cs index e00cfef5f..f8ff2c1a8 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItems.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItems.cs @@ -2,5 +2,10 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Scheduling; -public record ReplaceTemplateItems(int TemplateGroupId, int TemplateId, string Name, List Items) +public record ReplaceTemplateItems( + int TemplateGroupId, + int TemplateId, + string Name, + List Items, + Option ExpectedVersion = default) : IRequest>>; diff --git a/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItemsHandler.cs b/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItemsHandler.cs index 055f3c0e4..bc67427e2 100644 --- a/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItemsHandler.cs +++ b/ErsatzTV.Application/Scheduling/Commands/ReplaceTemplateItemsHandler.cs @@ -15,10 +15,19 @@ public class ReplaceTemplateItemsHandler(IDbContextFactory dbContextF { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); Validation validation = await Validate(dbContext, request, cancellationToken); - return await validation.Apply(ps => Persist(dbContext, request, ps, cancellationToken)); + + // Introduce the optimistic-concurrency check as a standalone Either AFTER the validation + // pipeline (never via Apply) so a PreconditionFailedError survives to a 412 and is not + // flattened to a generic 422 by Join() (issue #253 / api-conventions §7a). + Either validated = LanguageExtensions.ToEither(validation) + .Bind(template => template.CheckVersion(request.ExpectedVersion)); + + return await validated.Match( + Right: template => Persist(dbContext, request, template, cancellationToken), + Left: error => Task.FromResult>>(error)); } - private static async Task> Persist( + private static async Task>> Persist( TvContext dbContext, ReplaceTemplateItems request, Template template, @@ -30,7 +39,10 @@ public class ReplaceTemplateItemsHandler(IDbContextFactory dbContextF dbContext.RemoveRange(template.Items); template.Items = request.Items.Map(i => BuildItem(template, i)).ToList(); - await dbContext.SaveChangesAsync(cancellationToken); + // Unconditional bump: EF emits the root UPDATE only when a scalar actually differs, so a + // same-value/no-op save would otherwise write no root row and neither fire the concurrency + // token nor rotate other clients' ETags. Bumping guarantees both on every save (issue #253). + template.Version++; // TODO: refresh any playouts that use this schedule // foreach (Playout playout in programSchedule.Playouts) @@ -38,13 +50,22 @@ public class ReplaceTemplateItemsHandler(IDbContextFactory dbContextF // await _channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh)); // } - await dbContext.Entry(template) - .Collection(t => t.Items) - .Query() - .Include(i => i.Block) - .LoadAsync(cancellationToken); + // Save through the guard so an EF concurrency failure (a racing writer won between our load + // and save) maps to 412 rather than surfacing as a 500. + Either saved = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken); + return await saved.Match( + Right: async _ => + { + await dbContext.Entry(template) + .Collection(t => t.Items) + .Query() + .Include(i => i.Block) + .LoadAsync(cancellationToken); - return template.Items.Map(Mapper.ProjectToViewModel).ToList(); + return Right>( + template.Items.Map(Mapper.ProjectToViewModel).ToList()); + }, + Left: error => Task.FromResult(Left>(error))); } private static TemplateItem BuildItem(Template template, ReplaceTemplateItem item) => diff --git a/ErsatzTV.Application/Scheduling/DecoTemplateViewModel.cs b/ErsatzTV.Application/Scheduling/DecoTemplateViewModel.cs index c9543d63c..d1517d1c2 100644 --- a/ErsatzTV.Application/Scheduling/DecoTemplateViewModel.cs +++ b/ErsatzTV.Application/Scheduling/DecoTemplateViewModel.cs @@ -1,3 +1,3 @@ namespace ErsatzTV.Application.Scheduling; -public record DecoTemplateViewModel(int Id, int DecoTemplateGroupId, string GroupName, string Name); +public record DecoTemplateViewModel(int Id, int DecoTemplateGroupId, string GroupName, string Name, int Version); diff --git a/ErsatzTV.Application/Scheduling/Mapper.cs b/ErsatzTV.Application/Scheduling/Mapper.cs index d82d9325f..91e6356b5 100644 --- a/ErsatzTV.Application/Scheduling/Mapper.cs +++ b/ErsatzTV.Application/Scheduling/Mapper.cs @@ -65,7 +65,7 @@ internal static class Mapper new(templateGroup.Id, templateGroup.Name, templateGroup.Templates.Count); internal static TemplateViewModel ProjectToViewModel(Template template) => - new(template.Id, template.TemplateGroupId, template.TemplateGroup.Name, template.Name); + new(template.Id, template.TemplateGroupId, template.TemplateGroup.Name, template.Name, template.Version); internal static TemplateItemViewModel ProjectToViewModel(TemplateItem templateItem) { @@ -168,7 +168,8 @@ internal static class Mapper decoTemplate.Id, decoTemplate.DecoTemplateGroupId, decoTemplate.DecoTemplateGroup.Name, - decoTemplate.Name); + decoTemplate.Name, + decoTemplate.Version); } internal static DecoTemplateItemViewModel ProjectToViewModel(DecoTemplateItem decoTemplateItem) diff --git a/ErsatzTV.Application/Scheduling/TemplateViewModel.cs b/ErsatzTV.Application/Scheduling/TemplateViewModel.cs index cef2789bf..1e72952b0 100644 --- a/ErsatzTV.Application/Scheduling/TemplateViewModel.cs +++ b/ErsatzTV.Application/Scheduling/TemplateViewModel.cs @@ -1,3 +1,3 @@ namespace ErsatzTV.Application.Scheduling; -public record TemplateViewModel(int Id, int TemplateGroupId, string GroupName, string Name); +public record TemplateViewModel(int Id, int TemplateGroupId, string GroupName, string Name, int Version); diff --git a/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs b/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs index 63b08b96b..3439da4f9 100644 --- a/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs +++ b/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs @@ -14,7 +14,8 @@ public record PlayoutResponseModel( TimeSpan? DailyRebuildTime, PlayoutBuildStatusResponseModel? BuildStatus, int? DecoId, - string? DecoName) + string? DecoName, + bool IsLocked) { public static PlayoutResponseModel From( int id, @@ -27,7 +28,8 @@ public record PlayoutResponseModel( TimeSpan? dailyRebuildTime, PlayoutBuildStatusResponseModel? buildStatus, int? decoId, - string? decoName) => + string? decoName, + bool isLocked) => new( id, scheduleKind, @@ -39,5 +41,6 @@ public record PlayoutResponseModel( dailyRebuildTime, buildStatus, decoId, - decoName); + decoName, + isLocked); } diff --git a/ErsatzTV.Core/Api/Playouts/ResetAllPlayoutsResponseModel.cs b/ErsatzTV.Core/Api/Playouts/ResetAllPlayoutsResponseModel.cs new file mode 100644 index 000000000..830b2bb15 --- /dev/null +++ b/ErsatzTV.Core/Api/Playouts/ResetAllPlayoutsResponseModel.cs @@ -0,0 +1,7 @@ +#nullable enable +namespace ErsatzTV.Core.Api.Playouts; + +public record ResetAllPlayoutsResponseModel( + List QueuedPlayoutIds, + List SkippedLocked, + List SkippedUnsupported); diff --git a/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs b/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs index c25cde87b..61fd248ef 100644 --- a/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs +++ b/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs @@ -164,6 +164,30 @@ public class QueueLibraryScanByLibraryIdHandlerTests locker.Received(1).UnlockLibrary(libraryId); } + [Test] + public async Task Handle_Should_Thread_DeepScan_Into_Plex_ForceSynchronize() + { + int libraryId = await SeedSyncEnabledPlexLibrary(); + + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + Channel channel = ThreadingChannel.CreateUnbounded(); + + QueueLibraryScanByLibraryIdHandler handler = CreateHandler(locker, channel.Writer); + + QueueLibraryScanResult result = await handler.Handle( + new QueueLibraryScanByLibraryId(libraryId, DeepScan: true), + CancellationToken.None); + + result.ShouldBe(QueueLibraryScanResult.Queued); + + // first message refreshes the library list, second is the deep force-sync carrying DeepScan == true + channel.Reader.TryRead(out IScannerBackgroundServiceRequest? first).ShouldBeTrue(); + first.ShouldBeOfType(); + channel.Reader.TryRead(out IScannerBackgroundServiceRequest? second).ShouldBeTrue(); + second.ShouldBeOfType().DeepScan.ShouldBeTrue(); + } + private QueueLibraryScanByLibraryIdHandler CreateHandler( IEntityLocker locker, ChannelWriter writer) => @@ -193,7 +217,11 @@ public class QueueLibraryScanByLibraryIdHandlerTests return source.Libraries[0].Id; } - private async Task SeedSyncDisabledPlexLibrary() + private async Task SeedSyncDisabledPlexLibrary() => await SeedPlexLibrary(shouldSyncItems: false); + + private async Task SeedSyncEnabledPlexLibrary() => await SeedPlexLibrary(shouldSyncItems: true); + + private async Task SeedPlexLibrary(bool shouldSyncItems) { await using TvContext context = _db.CreateContext(); var source = new PlexMediaSource @@ -212,7 +240,7 @@ public class QueueLibraryScanByLibraryIdHandlerTests Name = "Plex Movies", MediaKind = LibraryMediaKind.Movies, Key = "1", - ShouldSyncItems = false, + ShouldSyncItems = shouldSyncItems, Paths = [] } ] diff --git a/ErsatzTV.Tests/Application/Libraries/QueueShowScanByLibraryIdHandlerTests.cs b/ErsatzTV.Tests/Application/Libraries/QueueShowScanByLibraryIdHandlerTests.cs new file mode 100644 index 000000000..e9aba9722 --- /dev/null +++ b/ErsatzTV.Tests/Application/Libraries/QueueShowScanByLibraryIdHandlerTests.cs @@ -0,0 +1,214 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Libraries; +using ErsatzTV.Application.Plex; +using ErsatzTV.Application.Subtitles; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using MediatR; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; +using ThreadingChannel = System.Threading.Channels.Channel; + +namespace ErsatzTV.Tests.Application.Libraries; + +[TestFixture] +public class QueueShowScanByLibraryIdHandlerTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + [Test] + public async Task Handle_Should_Return_NotFound_When_Library_Missing() + { + var locker = Substitute.For(); + var mediator = Substitute.For(); + + QueueShowScanByLibraryIdHandler handler = CreateHandler(locker, mediator, out _); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(9999, 1, "Show", false), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.NotFound); + locker.DidNotReceive().LockLibrary(Arg.Any()); + } + + [Test] + public async Task Handle_Should_Return_SyncDisabled_When_Item_Sync_Off() + { + int libraryId = await SeedPlexLibrary(shouldSyncItems: false); + var locker = Substitute.For(); + var mediator = Substitute.For(); + + QueueShowScanByLibraryIdHandler handler = CreateHandler(locker, mediator, out _); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 1, "Show", false), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.SyncDisabled); + locker.DidNotReceive().LockLibrary(Arg.Any()); + } + + [Test] + public async Task Handle_Should_Return_AlreadyScanning_When_Lock_Not_Acquired() + { + int libraryId = await SeedPlexLibrary(shouldSyncItems: true); + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(false); + var mediator = Substitute.For(); + + QueueShowScanByLibraryIdHandler handler = CreateHandler(locker, mediator, out _); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 1, "Show", false), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.AlreadyScanning); + await mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Handle_Should_Return_Unsupported_For_Local_Library() + { + int libraryId = await SeedLocalLibrary(); + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + var mediator = Substitute.For(); + + QueueShowScanByLibraryIdHandler handler = CreateHandler(locker, mediator, out _); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 1, "Show", false), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.Unsupported); + locker.Received(1).UnlockLibrary(libraryId); + } + + [Test] + public async Task Handle_Should_Return_Queued_And_Extract_Subtitles_On_Deep_Success() + { + int libraryId = await SeedPlexLibrary(shouldSyncItems: true); + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + var mediator = Substitute.For(); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right("ok")); + + QueueShowScanByLibraryIdHandler handler = CreateHandler( + locker, + mediator, + out Channel worker); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 42, "Show", DeepScan: true), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.Queued); + locker.Received(1).UnlockLibrary(libraryId); + worker.Reader.TryRead(out IBackgroundServiceRequest? request).ShouldBeTrue(); + request.ShouldBeOfType().ShowId.ShouldBe(42); + } + + [Test] + public async Task Handle_Should_Return_ScanFailed_When_SubScan_Left() + { + int libraryId = await SeedPlexLibrary(shouldSyncItems: true); + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + var mediator = Substitute.For(); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left(BaseError.New("scan error"))); + + QueueShowScanByLibraryIdHandler handler = CreateHandler( + locker, + mediator, + out Channel worker); + + QueueShowScanResult result = await handler.Handle( + new QueueShowScanByLibraryId(libraryId, 42, "Show", DeepScan: true), + CancellationToken.None); + + result.ShouldBe(QueueShowScanResult.ScanFailed); + locker.Received(1).UnlockLibrary(libraryId); + // no subtitle extraction on a failed scan + worker.Reader.TryRead(out _).ShouldBeFalse(); + } + + private QueueShowScanByLibraryIdHandler CreateHandler( + IEntityLocker locker, + IMediator mediator, + out Channel worker) + { + worker = ThreadingChannel.CreateUnbounded(); + return new QueueShowScanByLibraryIdHandler( + _db.Factory, + locker, + mediator, + worker.Writer, + NullLogger.Instance); + } + + private async Task SeedLocalLibrary() + { + await using TvContext context = _db.CreateContext(); + var source = new LocalMediaSource + { + Libraries = + [ + new LocalLibrary + { + Name = "Local Movies", + MediaKind = LibraryMediaKind.Movies, + Paths = [] + } + ] + }; + await context.LocalMediaSources.AddAsync(source); + await context.SaveChangesAsync(); + return source.Libraries[0].Id; + } + + private async Task SeedPlexLibrary(bool shouldSyncItems) + { + await using TvContext context = _db.CreateContext(); + var source = new PlexMediaSource + { + ServerName = "Plex Server", + ProductVersion = "1", + Platform = "Linux", + PlatformVersion = "1", + ClientIdentifier = "plex", + Connections = [], + PathReplacements = [], + Libraries = + [ + new PlexLibrary + { + Name = "Plex Shows", + MediaKind = LibraryMediaKind.Shows, + Key = "1", + ShouldSyncItems = shouldSyncItems, + Paths = [] + } + ] + }; + await context.PlexMediaSources.AddAsync(source); + await context.SaveChangesAsync(); + return source.Libraries[0].Id; + } +} diff --git a/ErsatzTV.Tests/Application/MediaCollections/ReplacePlaylistItemsHandlerConcurrencyTests.cs b/ErsatzTV.Tests/Application/MediaCollections/ReplacePlaylistItemsHandlerConcurrencyTests.cs new file mode 100644 index 000000000..d1ae0037e --- /dev/null +++ b/ErsatzTV.Tests/Application/MediaCollections/ReplacePlaylistItemsHandlerConcurrencyTests.cs @@ -0,0 +1,155 @@ +using ErsatzTV.Application; +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Microsoft.EntityFrameworkCore; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Application.MediaCollections; + +/// +/// Contract tests for the #253 optimistic-concurrency mechanic on the Playlist aggregate (mirrors +/// ReplaceBlockItemsHandlerConcurrencyTests, the Block reference implementation): the handler +/// pre-check (stale If-Match → 412), the force-write path (no If-Match), the unconditional Version +/// bump on every save, and the EF concurrency-token backstop that catches a writer that lost the +/// load→save race. The backstop test is non-vacuous by construction — remove the +/// IsConcurrencyToken() config on Playlist and the losing save silently succeeds instead of +/// mapping to a . +/// +[TestFixture] +public class ReplacePlaylistItemsHandlerConcurrencyTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private async Task SeedPlaylistAsync(int version) + { + await using TvContext ctx = _db.CreateContext(); + ctx.Playlists.Add( + new Playlist + { + Id = 1, + PlaylistGroupId = 1, + Name = "Kids", + IsSystem = false, + Version = version, + Items = new List() + }); + await ctx.SaveChangesAsync(); + } + + private static ReplacePlaylistItems Command(Option expectedVersion) => + new( + 1, + "Kids", + new List + { + new(0, CollectionType.Movie, null, null, null, 55, PlaybackOrder.Shuffle, null, false, true) + }, + expectedVersion); + + private async Task ReadVersionAsync() + { + await using TvContext ctx = _db.CreateContext(); + return await ctx.Playlists.Where(p => p.Id == 1).Select(p => p.Version).SingleAsync(); + } + + private static BaseError? LeftOrNull(Either result) => + result.Match(Right: _ => null, Left: e => e); + + [Test] + public async Task Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedPlaylistAsync(version: 2); + var handler = new ReplacePlaylistItemsHandler(_db.Factory); + + Either> result = + await handler.Handle(Command(Some(1)), CancellationToken.None); + + LeftOrNull(result).ShouldBeOfType(); + + // The pre-check runs before any mutation: version unchanged, no items written. + (await ReadVersionAsync()).ShouldBe(2); + await using TvContext ctx = _db.CreateContext(); + (await ctx.PlaylistItems.CountAsync(i => i.PlaylistId == 1)).ShouldBe(0); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump_Version() + { + await SeedPlaylistAsync(version: 2); + var handler = new ReplacePlaylistItemsHandler(_db.Factory); + + Either> result = + await handler.Handle(Command(Some(2)), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Absent_If_Match_Should_Force_Write_And_Bump_Version() + { + await SeedPlaylistAsync(version: 2); + var handler = new ReplacePlaylistItemsHandler(_db.Factory); + + // None expected version = Phase-1 force-write regardless of the stored version. + Either> result = + await handler.Handle(Command(None), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Save_Should_Bump_Version_Even_When_Content_Is_Unchanged() + { + await SeedPlaylistAsync(version: 5); + var handler = new ReplacePlaylistItemsHandler(_db.Factory); + + // Same content twice: the unconditional bump (M1) must still rotate the version each time, + // otherwise a no-op PUT-back would not fire the token or rotate other clients' ETags. + (await handler.Handle(Command(Some(5)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(6); + (await handler.Handle(Command(Some(6)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(7); + } + + [Test] + public async Task Racing_Save_Should_Map_Concurrency_Failure_To_412() + { + await SeedPlaylistAsync(version: 1); + + // Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on + // Playlist makes the second UPDATE key on the original version; it matches zero rows and throws + // DbUpdateConcurrencyException, which the shared save helper maps to a PreconditionFailedError. + await using TvContext ctxWinner = _db.CreateContext(); + await using TvContext ctxLoser = _db.CreateContext(); + + Playlist winner = await ctxWinner.Playlists.SingleAsync(p => p.Id == 1); + Playlist loser = await ctxLoser.Playlists.SingleAsync(p => p.Id == 1); + + winner.Version++; + Either winnerResult = await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None); + winnerResult.IsRight.ShouldBeTrue(); + + loser.Version++; + Either loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None); + + LeftOrNull(loserResult).ShouldBeOfType(); + + // Non-vacuous: the winner's write stuck at exactly its +1; the loser did not overwrite it. + (await ReadVersionAsync()).ShouldBe(2); + } +} diff --git a/ErsatzTV.Tests/Application/Playouts/ResetAllPlayoutsHandlerTests.cs b/ErsatzTV.Tests/Application/Playouts/ResetAllPlayoutsHandlerTests.cs new file mode 100644 index 000000000..e7b6ffab1 --- /dev/null +++ b/ErsatzTV.Tests/Application/Playouts/ResetAllPlayoutsHandlerTests.cs @@ -0,0 +1,82 @@ +using System.Collections.Generic; +using System.Threading; +using System.Threading.Channels; +using System.Threading.Tasks; +using ErsatzTV.Application; +using ErsatzTV.Application.Playouts; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Core.Scheduling; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using Channel = System.Threading.Channels.Channel; + +namespace ErsatzTV.Tests.Application.Playouts; + +[TestFixture] +public class ResetAllPlayoutsHandlerTests +{ + private InMemoryTvContext _db = null!; + private Channel _worker = null!; + private IEntityLocker _entityLocker = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _worker = Channel.CreateUnbounded(); + _entityLocker = Substitute.For(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private ResetAllPlayoutsHandler CreateHandler() => + new(_entityLocker, _worker.Writer, _db.Factory); + + private async Task SeedPlayout(PlayoutScheduleKind kind) + { + await using TvContext context = _db.CreateContext(); + var playout = new Playout { ChannelId = 0, ScheduleKind = kind }; + context.Playouts.Add(playout); + await context.SaveChangesAsync(); + return playout.Id; + } + + [Test] + public async Task Handle_Should_Queue_Eligible_And_Report_Skipped() + { + int classicId = await SeedPlayout(PlayoutScheduleKind.Classic); + int blockId = await SeedPlayout(PlayoutScheduleKind.Block); + int lockedId = await SeedPlayout(PlayoutScheduleKind.Sequential); + int externalJsonId = await SeedPlayout(PlayoutScheduleKind.ExternalJson); + int noneId = await SeedPlayout(PlayoutScheduleKind.None); + + _entityLocker.IsPlayoutLocked(lockedId).Returns(true); + + ResetAllPlayoutsResult result = + await CreateHandler().Handle(new ResetAllPlayouts(), CancellationToken.None); + + // eligible, unlocked playouts are queued + result.QueuedPlayoutIds.ShouldBe(new List { classicId, blockId }, ignoreOrder: true); + + // locked playout lands in SkippedLocked, not queued + result.SkippedLocked.ShouldBe(new List { lockedId }); + + // ExternalJson + None land in SkippedUnsupported + result.SkippedUnsupported.ShouldBe(new List { externalJsonId, noneId }, ignoreOrder: true); + + // exactly one BuildPlayout message per queued playout was enqueued + var enqueued = new List(); + while (_worker.Reader.TryRead(out IBackgroundServiceRequest? request)) + { + var build = request.ShouldBeOfType(); + enqueued.Add(build.PlayoutId); + } + + enqueued.ShouldBe(new List { classicId, blockId }, ignoreOrder: true); + } +} diff --git a/ErsatzTV.Tests/Application/ProgramSchedules/ReplaceProgramScheduleItemsHandlerConcurrencyTests.cs b/ErsatzTV.Tests/Application/ProgramSchedules/ReplaceProgramScheduleItemsHandlerConcurrencyTests.cs new file mode 100644 index 000000000..d665cdf3b --- /dev/null +++ b/ErsatzTV.Tests/Application/ProgramSchedules/ReplaceProgramScheduleItemsHandlerConcurrencyTests.cs @@ -0,0 +1,231 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.ProgramSchedules; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Scheduling; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Microsoft.EntityFrameworkCore; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Application.ProgramSchedules; + +/// +/// Contract tests for the #253 optimistic-concurrency mechanic on the ProgramSchedule / schedule-items +/// aggregate: the handler pre-check (stale If-Match → 412, before the positional reconcile runs — so the +/// item rows AND persisted fill-group/shuffle state are left untouched), the force-write path (no +/// If-Match), the unconditional Version bump on every save (including a no-op same-items PUT-back where +/// only child rows change), and the EF concurrency-token backstop that catches a writer that lost the +/// load→save race. The backstop test is non-vacuous by construction — remove the +/// IsConcurrencyToken() config on ProgramSchedule and the losing save silently succeeds instead +/// of mapping to a . +/// +[TestFixture] +public class ReplaceProgramScheduleItemsHandlerConcurrencyTests +{ + private InMemoryTvContext _db = null!; + private ChannelWriter _worker = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _worker = System.Threading.Channels.Channel.CreateUnbounded().Writer; + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + // Seeds a schedule (Id=1) with a single One/SearchQuery item (Id=1) and a persisted fill-group + // enumerator state pointing at that item, so the stale-If-Match test can prove the reconcile never ran. + private async Task SeedScheduleAsync(int version) + { + await using TvContext ctx = _db.CreateContext(); + ctx.ProgramSchedules.Add( + new ProgramSchedule + { + Id = 1, + Name = "Concurrency", + Version = version, + Items = new List + { + new ProgramScheduleItemOne + { + Id = 1, + Index = 0, + CollectionType = CollectionType.SearchQuery, + SearchTitle = "a", + SearchQuery = "a", + PlaybackOrder = PlaybackOrder.Shuffle, + GuideMode = GuideMode.Normal + } + }, + Playouts = [], + ProgramScheduleAlternates = [] + }); + await ctx.SaveChangesAsync(); + + ctx.Add(new PlayoutScheduleItemFillGroupIndex + { + PlayoutId = 1, + ProgramScheduleItemId = 1, + EnumeratorState = new CollectionEnumeratorState { Seed = 12345, Index = 7 } + }); + await ctx.SaveChangesAsync(); + } + + private static ReplaceProgramScheduleItems Command( + Option expectedVersion, + List? items = null) => + new(1, items ?? [MakeItem(0, PlayoutMode.One, "a")], expectedVersion); + + private async Task ReadVersionAsync() + { + await using TvContext ctx = _db.CreateContext(); + return await ctx.ProgramSchedules.Where(s => s.Id == 1).Select(s => s.Version).SingleAsync(); + } + + private static BaseError? LeftOrNull(Either> result) => + result.Match(Right: _ => null, Left: e => e); + + [Test] + public async Task Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedScheduleAsync(version: 2); + var handler = new ReplaceProgramScheduleItemsHandler(_db.Factory, _worker); + + // An empty item list WOULD delete the existing item (and cascade its fill-group state) if the + // reconcile ran. A stale If-Match must reject before that, leaving everything untouched. + Either> result = + await handler.Handle(Command(Some(1), []), CancellationToken.None); + + LeftOrNull(result).ShouldBeOfType(); + + (await ReadVersionAsync()).ShouldBe(2); + await using TvContext ctx = _db.CreateContext(); + (await ctx.ProgramScheduleItems.CountAsync(i => i.ProgramScheduleId == 1)).ShouldBe(1); + + // The reconcile never ran: the fill-group enumerator state is exactly as seeded. + PlayoutScheduleItemFillGroupIndex fillGroup = await ctx.Set() + .Include(x => x.EnumeratorState) + .SingleAsync(); + fillGroup.ProgramScheduleItemId.ShouldBe(1); + fillGroup.EnumeratorState.Seed.ShouldBe(12345); + fillGroup.EnumeratorState.Index.ShouldBe(7); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump_Version() + { + await SeedScheduleAsync(version: 2); + var handler = new ReplaceProgramScheduleItemsHandler(_db.Factory, _worker); + + Either> result = + await handler.Handle(Command(Some(2)), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Absent_If_Match_Should_Force_Write_And_Bump_Version() + { + await SeedScheduleAsync(version: 2); + var handler = new ReplaceProgramScheduleItemsHandler(_db.Factory, _worker); + + // None expected version = Phase-1 force-write regardless of the stored version. + Either> result = + await handler.Handle(Command(None), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task NoOp_Save_Should_Bump_Version_Even_With_Only_Child_Changes() + { + await SeedScheduleAsync(version: 5); + var handler = new ReplaceProgramScheduleItemsHandler(_db.Factory, _worker); + + // Same content twice: this handler saves with only CHILD changes and no root-scalar change, so the + // unconditional bump (M1) must still rotate the version each time, otherwise a no-op PUT-back would + // neither fire the token nor rotate other clients' ETags. + (await handler.Handle(Command(Some(5)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(6); + (await handler.Handle(Command(Some(6)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(7); + } + + [Test] + public async Task Racing_Save_Should_Map_Concurrency_Failure_To_412() + { + await SeedScheduleAsync(version: 1); + + // Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on + // ProgramSchedule makes the second UPDATE key on the original version; it matches zero rows and + // throws DbUpdateConcurrencyException, which the shared save helper maps to a PreconditionFailedError. + await using TvContext ctxWinner = _db.CreateContext(); + await using TvContext ctxLoser = _db.CreateContext(); + + ProgramSchedule winner = await ctxWinner.ProgramSchedules.SingleAsync(s => s.Id == 1); + ProgramSchedule loser = await ctxLoser.ProgramSchedules.SingleAsync(s => s.Id == 1); + + winner.Version++; + Either winnerResult = await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None); + winnerResult.IsRight.ShouldBeTrue(); + + loser.Version++; + Either loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None); + + loserResult.Match(Right: _ => null, Left: e => e).ShouldBeOfType(); + + // Non-vacuous: the winner's write stuck at exactly its +1; the loser did not overwrite it. + (await ReadVersionAsync()).ShouldBe(2); + } + + private static ReplaceProgramScheduleItem MakeItem(int index, PlayoutMode mode, string searchQuery) => + new( + index, + StartType.Dynamic, + StartTime: null, + FixedStartTimeBehavior: null, + mode, + CollectionType.SearchQuery, + CollectionId: null, + MultiCollectionId: null, + SmartCollectionId: null, + RerunCollectionId: null, + MediaItemId: null, + PlaylistId: null, + SearchTitle: searchQuery, + SearchQuery: searchQuery, + PlaybackOrder.Shuffle, + MarathonGroupBy.None, + MarathonShuffleGroups: false, + MarathonShuffleItems: false, + MarathonBatchSize: null, + FillWithGroupMode.None, + MultipleMode.Count, + MultipleCount: "1", + PlayoutDuration: null, + TailMode.None, + DiscardToFillAttempts: null, + CustomTitle: null, + GuideMode.Normal, + PreRollFillerId: null, + MidRollFillerId: null, + PostRollFillerId: null, + TailFillerId: null, + FallbackFillerId: null, + WatermarkIds: [], + GraphicsElementIds: [], + PreferredAudioLanguageCode: null, + PreferredAudioTitle: null, + PreferredSubtitleLanguageCode: null, + SubtitleMode: null); +} diff --git a/ErsatzTV.Tests/Application/Scheduling/ReplaceDecoTemplateItemsHandlerConcurrencyTests.cs b/ErsatzTV.Tests/Application/Scheduling/ReplaceDecoTemplateItemsHandlerConcurrencyTests.cs new file mode 100644 index 000000000..3d265d0c8 --- /dev/null +++ b/ErsatzTV.Tests/Application/Scheduling/ReplaceDecoTemplateItemsHandlerConcurrencyTests.cs @@ -0,0 +1,164 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Playouts; +using ErsatzTV.Application.Scheduling; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Domain.Scheduling; +using ErsatzTV.Core.Errors; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Microsoft.EntityFrameworkCore; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Application.Scheduling; + +/// +/// Contract tests for the #253 optimistic-concurrency mechanic on the DecoTemplate aggregate, +/// mirroring (the Block reference +/// implementation): the handler pre-check (stale If-Match → 412), the force-write path (no +/// If-Match), the unconditional Version bump on every save, and the EF concurrency-token backstop +/// that catches a writer that lost the load→save race. The backstop test is non-vacuous by +/// construction — remove the IsConcurrencyToken() config on DecoTemplate and the losing save +/// silently succeeds instead of mapping to a . +/// +[TestFixture] +public class ReplaceDecoTemplateItemsHandlerConcurrencyTests +{ + private InMemoryTvContext _db = null!; + private Channel _channel = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _channel = System.Threading.Channels.Channel.CreateUnbounded(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private async Task SeedDecoTemplateAsync(int version) + { + await using TvContext ctx = _db.CreateContext(); + ctx.Decos.Add(new Deco { Id = 10, DecoGroupId = 1, Name = "D" }); + ctx.DecoTemplates.Add( + new DecoTemplate + { + Id = 1, + DecoTemplateGroupId = 1, + Name = "Weekday", + Version = version, + Items = new List() + }); + await ctx.SaveChangesAsync(); + } + + private ReplaceDecoTemplateItemsHandler CreateHandler() => new(_db.Factory, _channel.Writer); + + private static ReplaceDecoTemplateItems Command(Option expectedVersion) => + new( + DecoTemplateId: 1, + DecoTemplateGroupId: 1, + Name: "Weekday", + Items: [new ReplaceDecoTemplateItem(DecoId: 10, StartTime: TimeSpan.Zero, EndTime: TimeSpan.FromHours(1))], + ExpectedVersion: expectedVersion); + + private async Task ReadVersionAsync() + { + await using TvContext ctx = _db.CreateContext(); + return await ctx.DecoTemplates.Where(t => t.Id == 1).Select(t => t.Version).SingleAsync(); + } + + private static BaseError? LeftOrNull(Either result) => + result.Match(Right: _ => null, Left: e => e); + + [Test] + public async Task Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedDecoTemplateAsync(version: 2); + ReplaceDecoTemplateItemsHandler handler = CreateHandler(); + + Either> result = + await handler.Handle(Command(Some(1)), CancellationToken.None); + + LeftOrNull(result).ShouldBeOfType(); + + // The pre-check runs before any mutation: version unchanged, no items written. + (await ReadVersionAsync()).ShouldBe(2); + await using TvContext ctx = _db.CreateContext(); + (await ctx.DecoTemplateItems.CountAsync(i => i.DecoTemplateId == 1)).ShouldBe(0); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump_Version() + { + await SeedDecoTemplateAsync(version: 2); + ReplaceDecoTemplateItemsHandler handler = CreateHandler(); + + Either> result = + await handler.Handle(Command(Some(2)), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Absent_If_Match_Should_Force_Write_And_Bump_Version() + { + await SeedDecoTemplateAsync(version: 2); + ReplaceDecoTemplateItemsHandler handler = CreateHandler(); + + // None expected version = Phase-1 force-write regardless of the stored version. + Either> result = + await handler.Handle(Command(None), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Save_Should_Bump_Version_Even_When_Content_Is_Unchanged() + { + await SeedDecoTemplateAsync(version: 5); + ReplaceDecoTemplateItemsHandler handler = CreateHandler(); + + // Same content twice: the unconditional bump (M1) must still rotate the version each time, + // otherwise a no-op PUT-back would not fire the token or rotate other clients' ETags. + (await handler.Handle(Command(Some(5)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(6); + (await handler.Handle(Command(Some(6)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(7); + } + + [Test] + public async Task Racing_Save_Should_Map_Concurrency_Failure_To_412() + { + await SeedDecoTemplateAsync(version: 1); + + // Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on + // DecoTemplate makes the second UPDATE key on the original version; it matches zero rows and + // throws DbUpdateConcurrencyException, which the shared save helper maps to a + // PreconditionFailedError. + await using TvContext ctxWinner = _db.CreateContext(); + await using TvContext ctxLoser = _db.CreateContext(); + + DecoTemplate winner = await ctxWinner.DecoTemplates.SingleAsync(t => t.Id == 1); + DecoTemplate loser = await ctxLoser.DecoTemplates.SingleAsync(t => t.Id == 1); + + winner.Version++; + Either winnerResult = await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None); + winnerResult.IsRight.ShouldBeTrue(); + + loser.Version++; + Either loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None); + + LeftOrNull(loserResult).ShouldBeOfType(); + + // Non-vacuous: the winner's write stuck at exactly its +1; the loser did not overwrite it. + (await ReadVersionAsync()).ShouldBe(2); + } +} diff --git a/ErsatzTV.Tests/Application/Scheduling/ReplaceTemplateItemsHandlerConcurrencyTests.cs b/ErsatzTV.Tests/Application/Scheduling/ReplaceTemplateItemsHandlerConcurrencyTests.cs new file mode 100644 index 000000000..4e71c0a91 --- /dev/null +++ b/ErsatzTV.Tests/Application/Scheduling/ReplaceTemplateItemsHandlerConcurrencyTests.cs @@ -0,0 +1,163 @@ +using ErsatzTV.Application; +using ErsatzTV.Application.Scheduling; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain.Scheduling; +using ErsatzTV.Core.Errors; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using Microsoft.EntityFrameworkCore; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Application.Scheduling; + +/// +/// Contract tests for the #253 optimistic-concurrency mechanic on the Template aggregate, +/// mirroring (the Block reference +/// implementation): the handler pre-check (stale If-Match → 412), the force-write path (no +/// If-Match), the unconditional Version bump on every save, and the EF concurrency-token backstop +/// that catches a writer that lost the load→save race. The backstop test is non-vacuous by +/// construction — remove the IsConcurrencyToken() config on Template and the losing save +/// silently succeeds instead of mapping to a . +/// +[TestFixture] +public class ReplaceTemplateItemsHandlerConcurrencyTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private async Task SeedTemplateAsync(int version) + { + await using TvContext ctx = _db.CreateContext(); + ctx.Blocks.Add( + new Block + { + Id = 10, + BlockGroupId = 1, + Name = "Morning", + Minutes = 30, + StopScheduling = BlockStopScheduling.AfterDurationEnd, + Items = new List() + }); + ctx.Templates.Add( + new Template + { + Id = 1, + TemplateGroupId = 1, + Name = "Weekday", + Version = version, + Items = new List() + }); + await ctx.SaveChangesAsync(); + } + + private static ReplaceTemplateItems Command(Option expectedVersion) => + new( + 1, + 1, + "Weekday", + new List { new(10, TimeSpan.Zero) }, + expectedVersion); + + private async Task ReadVersionAsync() + { + await using TvContext ctx = _db.CreateContext(); + return await ctx.Templates.Where(t => t.Id == 1).Select(t => t.Version).SingleAsync(); + } + + private static BaseError? LeftOrNull(Either result) => + result.Match(Right: _ => null, Left: e => e); + + [Test] + public async Task Stale_If_Match_Should_Fail_Precondition_And_Not_Mutate() + { + await SeedTemplateAsync(version: 2); + var handler = new ReplaceTemplateItemsHandler(_db.Factory); + + Either> result = + await handler.Handle(Command(Some(1)), CancellationToken.None); + + LeftOrNull(result).ShouldBeOfType(); + + // The pre-check runs before any mutation: version unchanged, no items written. + (await ReadVersionAsync()).ShouldBe(2); + await using TvContext ctx = _db.CreateContext(); + (await ctx.TemplateItems.CountAsync(i => i.TemplateId == 1)).ShouldBe(0); + } + + [Test] + public async Task Matching_If_Match_Should_Succeed_And_Bump_Version() + { + await SeedTemplateAsync(version: 2); + var handler = new ReplaceTemplateItemsHandler(_db.Factory); + + Either> result = + await handler.Handle(Command(Some(2)), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Absent_If_Match_Should_Force_Write_And_Bump_Version() + { + await SeedTemplateAsync(version: 2); + var handler = new ReplaceTemplateItemsHandler(_db.Factory); + + // None expected version = Phase-1 force-write regardless of the stored version. + Either> result = + await handler.Handle(Command(None), CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(3); + } + + [Test] + public async Task Save_Should_Bump_Version_Even_When_Content_Is_Unchanged() + { + await SeedTemplateAsync(version: 5); + var handler = new ReplaceTemplateItemsHandler(_db.Factory); + + // Same content twice: the unconditional bump (M1) must still rotate the version each time, + // otherwise a no-op PUT-back would not fire the token or rotate other clients' ETags. + (await handler.Handle(Command(Some(5)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(6); + (await handler.Handle(Command(Some(6)), CancellationToken.None)).IsRight.ShouldBeTrue(); + (await ReadVersionAsync()).ShouldBe(7); + } + + [Test] + public async Task Racing_Save_Should_Map_Concurrency_Failure_To_412() + { + await SeedTemplateAsync(version: 1); + + // Two writers load the same version, then both bump-and-save. The IsConcurrencyToken guard on + // Template makes the second UPDATE key on the original version; it matches zero rows and + // throws DbUpdateConcurrencyException, which the shared save helper maps to a + // PreconditionFailedError. + await using TvContext ctxWinner = _db.CreateContext(); + await using TvContext ctxLoser = _db.CreateContext(); + + Template winner = await ctxWinner.Templates.SingleAsync(t => t.Id == 1); + Template loser = await ctxLoser.Templates.SingleAsync(t => t.Id == 1); + + winner.Version++; + Either winnerResult = await ctxWinner.SaveChangesWithConcurrencyGuard(CancellationToken.None); + winnerResult.IsRight.ShouldBeTrue(); + + loser.Version++; + Either loserResult = await ctxLoser.SaveChangesWithConcurrencyGuard(CancellationToken.None); + + LeftOrNull(loserResult).ShouldBeOfType(); + + // Non-vacuous: the winner's write stuck at exactly its +1; the loser did not overwrite it. + (await ReadVersionAsync()).ShouldBe(2); + } +} diff --git a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs index 3e300c819..f1f12b89f 100644 --- a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs @@ -407,7 +407,7 @@ public class ChannelControllerTests IActionResult result = await _controller.ResetPlayout("5", mode: null, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(202); _workerChannel.Reader.TryRead(out IBackgroundServiceRequest? request).ShouldBeTrue(); var buildPlayout = request.ShouldBeOfType(); buildPlayout.PlayoutId.ShouldBe(9); @@ -436,7 +436,7 @@ public class ChannelControllerTests IActionResult result = await _controller.ResetPlayout("5", PlayoutBuildMode.Continue, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeOfType().StatusCode.ShouldBe(202); _workerChannel.Reader.TryRead(out IBackgroundServiceRequest? request).ShouldBeTrue(); request.ShouldBeOfType().Mode.ShouldBe(PlayoutBuildMode.Continue); await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); diff --git a/ErsatzTV.Tests/Controllers/DecoControllerTests.cs b/ErsatzTV.Tests/Controllers/DecoControllerTests.cs index 38068d4ac..e96eae117 100644 --- a/ErsatzTV.Tests/Controllers/DecoControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/DecoControllerTests.cs @@ -152,7 +152,7 @@ public class DecoControllerTests null, null, null, - new ErsatzTV.Application.MediaCollections.PlaylistViewModel(9, 3, "Idents", false), + new ErsatzTV.Application.MediaCollections.PlaylistViewModel(9, 3, "Idents", false, 1), DecoBreakPlacement.BlockStart) ]); _mediator.Send(Arg.Any(), Arg.Any()) diff --git a/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs b/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs index 6ebfe6754..e5ed3b39c 100644 --- a/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/DecoTemplateControllerTests.cs @@ -4,8 +4,10 @@ using ErsatzTV.Controllers.Api; using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Scheduling; +using ErsatzTV.Core.Errors; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -25,7 +27,12 @@ public class DecoTemplateControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new DecoTemplateController(_mediator); + _controller = new DecoTemplateController(_mediator) + { + // Provide a real HttpContext so the ETag/If-Match concurrency headers (#253) can be + // read from Request and written to Response. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } [Test] @@ -259,12 +266,10 @@ public class DecoTemplateControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning"))); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Right>([])); + .Returns(Right>( + [MakeItem(10, "Morning Bumper", TimeSpan.FromHours(6), TimeSpan.FromHours(7))])); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(new List - { - MakeItem(10, "Morning Bumper", TimeSpan.FromHours(6), TimeSpan.FromHours(7)) - }); + .Returns([MakeItem(10, "Morning Bumper", TimeSpan.FromHours(6), TimeSpan.FromHours(7))]); IActionResult result = await _controller.Replace( 4, @@ -318,11 +323,100 @@ public class DecoTemplateControllerTests CancellationToken.None); result.ShouldBeOfType(); - await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); } - private static DecoTemplateViewModel MakeDecoTemplate(int id, int groupId, string name) => - new(id, groupId, "Group", name); + [Test] + public async Task GetItems_Should_Set_ETag_From_DecoTemplate_Version() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeDecoTemplate(4, 2, "Morning", version: 9))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.GetItems(4, CancellationToken.None); + + _controller.Response.Headers.ETag.ToString().ShouldBe("\"9\""); + } + + [Test] + public async Task Replace_Should_Return_400_On_Malformed_If_Match() + { + _controller.Request.Headers.IfMatch = "not-an-etag"; + + IActionResult result = await _controller.Replace( + 4, + new ReplaceDecoTemplateRequest("Morning", []), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Replace_Should_Thread_If_Match_Version_Into_Command() + { + _controller.Request.Headers.IfMatch = "\"3\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning", version: 4))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + IActionResult result = await _controller.Replace( + 4, + new ReplaceDecoTemplateRequest("Morning", []), + CancellationToken.None); + + result.ShouldBeOfType(); + // On success the response carries the refreshed deco template's ETag. + _controller.Response.Headers.ETag.ToString().ShouldBe("\"4\""); + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.Some(3)), + Arg.Any()); + } + + [Test] + public async Task Replace_Without_If_Match_Should_Force_Write() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning"))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.Replace( + 4, + new ReplaceDecoTemplateRequest("Morning", []), + CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.None), + Arg.Any()); + } + + [Test] + public async Task Replace_Should_Return_412_On_Precondition_Failed() + { + _controller.Request.Headers.IfMatch = "\"2\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeDecoTemplate(4, 7, "Morning", version: 5))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left>(new PreconditionFailedError("stale"))); + + IActionResult result = await _controller.Replace( + 4, + new ReplaceDecoTemplateRequest("Morning", []), + CancellationToken.None); + + var objectResult = result.ShouldBeOfType(); + objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed); + } + + private static DecoTemplateViewModel MakeDecoTemplate(int id, int groupId, string name, int version = 1) => + new(id, groupId, "Group", name, version); private static DecoTemplateItemViewModel MakeItem(int decoId, string decoName, TimeSpan startTime, TimeSpan endTime) { diff --git a/ErsatzTV.Tests/Controllers/EmbyMediaSourcesControllerTests.cs b/ErsatzTV.Tests/Controllers/EmbyMediaSourcesControllerTests.cs index d5a672677..cd838dcd8 100644 --- a/ErsatzTV.Tests/Controllers/EmbyMediaSourcesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/EmbyMediaSourcesControllerTests.cs @@ -75,6 +75,10 @@ public class EmbyMediaSourcesControllerTests nameof(EmbyMediaSourcesController.RefreshLibraries), "POST", "/api/media-sources/emby/{id:int}/refresh-libraries"); + ShouldHaveActionRoute( + nameof(EmbyMediaSourcesController.ScanCollections), + "POST", + "/api/media-sources/emby/{id:int}/scan-collections"); } [Test] @@ -414,6 +418,48 @@ public class EmbyMediaSourcesControllerTests request.ShouldBeOfType().EmbyMediaSourceId.ShouldBe(1); } + [Test] + public async Task ScanCollections_Should_Return_404_When_Source_Missing() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.None); + + IActionResult result = await _controller.ScanCollections(99, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType(); + _entityLocker.DidNotReceive().LockEmbyCollections(); + } + + [Test] + public async Task ScanCollections_Should_Return_409_When_Collections_Locked() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new EmbyMediaSourceViewModel(1, "s", "a"))); + _entityLocker.LockEmbyCollections().Returns(false); + + IActionResult result = await _controller.ScanCollections(1, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + _scannerChannel.Reader.TryRead(out _).ShouldBeFalse(); + } + + [Test] + public async Task ScanCollections_Should_Enqueue_And_Return_202() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new EmbyMediaSourceViewModel(1, "s", "a"))); + _entityLocker.LockEmbyCollections().Returns(true); + + IActionResult result = await _controller.ScanCollections(1, deep: true, CancellationToken.None); + + result.ShouldBeOfType(); + _scannerChannel.Reader.TryRead(out IScannerBackgroundServiceRequest? request).ShouldBeTrue(); + var command = request.ShouldBeOfType(); + command.EmbyMediaSourceId.ShouldBe(1); + command.ForceScan.ShouldBeTrue(); + command.DeepScan.ShouldBeTrue(); + } + private static void ShouldHaveActionRoute(string actionName, string httpMethod, string route) { MethodInfo action = typeof(EmbyMediaSourcesController).GetMethod(actionName) diff --git a/ErsatzTV.Tests/Controllers/JellyfinMediaSourcesControllerTests.cs b/ErsatzTV.Tests/Controllers/JellyfinMediaSourcesControllerTests.cs index 5623b5dfa..a891570bf 100644 --- a/ErsatzTV.Tests/Controllers/JellyfinMediaSourcesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/JellyfinMediaSourcesControllerTests.cs @@ -75,6 +75,10 @@ public class JellyfinMediaSourcesControllerTests nameof(JellyfinMediaSourcesController.RefreshLibraries), "POST", "/api/media-sources/jellyfin/{id:int}/refresh-libraries"); + ShouldHaveActionRoute( + nameof(JellyfinMediaSourcesController.ScanCollections), + "POST", + "/api/media-sources/jellyfin/{id:int}/scan-collections"); } [Test] @@ -414,6 +418,48 @@ public class JellyfinMediaSourcesControllerTests request.ShouldBeOfType().JellyfinMediaSourceId.ShouldBe(1); } + [Test] + public async Task ScanCollections_Should_Return_404_When_Source_Missing() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.None); + + IActionResult result = await _controller.ScanCollections(99, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType(); + _entityLocker.DidNotReceive().LockJellyfinCollections(); + } + + [Test] + public async Task ScanCollections_Should_Return_409_When_Collections_Locked() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new JellyfinMediaSourceViewModel(1, "s", "a"))); + _entityLocker.LockJellyfinCollections().Returns(false); + + IActionResult result = await _controller.ScanCollections(1, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + _scannerChannel.Reader.TryRead(out _).ShouldBeFalse(); + } + + [Test] + public async Task ScanCollections_Should_Enqueue_And_Return_202() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new JellyfinMediaSourceViewModel(1, "s", "a"))); + _entityLocker.LockJellyfinCollections().Returns(true); + + IActionResult result = await _controller.ScanCollections(1, deep: true, CancellationToken.None); + + result.ShouldBeOfType(); + _scannerChannel.Reader.TryRead(out IScannerBackgroundServiceRequest? request).ShouldBeTrue(); + var command = request.ShouldBeOfType(); + command.JellyfinMediaSourceId.ShouldBe(1); + command.ForceScan.ShouldBeTrue(); + command.DeepScan.ShouldBeTrue(); + } + private static void ShouldHaveActionRoute(string actionName, string httpMethod, string route) { MethodInfo action = typeof(JellyfinMediaSourcesController).GetMethod(actionName) diff --git a/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs b/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs index 95ebbdf6b..476758752 100644 --- a/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs @@ -71,14 +71,15 @@ public class LibrariesControllerTests } [Test] - public async Task ScanShow_Should_Queue_Scan_By_Show_Id_When_Show_Belongs_To_Library() + public async Task ScanShow_Should_Return_202_And_Queue_Scan_By_Show_Id_When_Show_Belongs_To_Library() { _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); - _mediator.Send(Arg.Any(), Arg.Any()).Returns(true); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.Queued); IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42, DeepScan: true)); - result.ShouldBeOfType(); + result.ShouldBeOfType(); await _mediator.Received(1).Send( Arg.Is(r => r.LibraryId == 3 && r.ShowId == 42 && r.ShowTitle == "The Office" && r.DeepScan), @@ -86,14 +87,68 @@ public class LibrariesControllerTests } [Test] - public async Task ScanShow_Should_Return_BadRequest_When_Mediator_Fails_To_Queue() + public async Task ScanShow_Should_Return_409_When_AlreadyScanning() { _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); - _mediator.Send(Arg.Any(), Arg.Any()).Returns(false); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.AlreadyScanning); IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); - result.ShouldBeOfType(); + var conflict = result.ShouldBeOfType(); + conflict.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status409Conflict); + } + + [Test] + public async Task ScanShow_Should_Return_422_When_SyncDisabled() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.SyncDisabled); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); + } + + [Test] + public async Task ScanShow_Should_Return_422_When_Unsupported() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.Unsupported); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); + } + + [Test] + public async Task ScanShow_Should_Return_422_When_ScanFailed() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.ScanFailed); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); + } + + [Test] + public async Task ScanShow_Should_Return_404_When_Handler_Reports_NotFound() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueShowScanResult.NotFound); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + var notFound = result.ShouldBeOfType(); + notFound.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status404NotFound); } [Test] @@ -102,11 +157,25 @@ public class LibrariesControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(QueueLibraryScanResult.Queued); - IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + IActionResult result = await _controller.ScanLibrary(7, cancellationToken: CancellationToken.None); result.ShouldBeOfType(); await _mediator.Received(1).Send( - Arg.Is(r => r.LibraryId == 7), + Arg.Is(r => r.LibraryId == 7 && !r.DeepScan), + Arg.Any()); + } + + [Test] + public async Task ScanLibrary_Should_Pass_DeepScan_When_Deep_Query_True() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueLibraryScanResult.Queued); + + IActionResult result = await _controller.ScanLibrary(7, deep: true, CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send( + Arg.Is(r => r.LibraryId == 7 && r.DeepScan), Arg.Any()); } @@ -116,7 +185,7 @@ public class LibrariesControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(QueueLibraryScanResult.NotFound); - IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + IActionResult result = await _controller.ScanLibrary(7, cancellationToken: CancellationToken.None); var notFound = result.ShouldBeOfType(); notFound.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status404NotFound); @@ -128,7 +197,7 @@ public class LibrariesControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(QueueLibraryScanResult.AlreadyScanning); - IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + IActionResult result = await _controller.ScanLibrary(7, cancellationToken: CancellationToken.None); var conflict = result.ShouldBeOfType(); conflict.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status409Conflict); @@ -140,7 +209,7 @@ public class LibrariesControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(QueueLibraryScanResult.SyncDisabled); - IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + IActionResult result = await _controller.ScanLibrary(7, cancellationToken: CancellationToken.None); var unprocessable = result.ShouldBeOfType(); unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); diff --git a/ErsatzTV.Tests/Controllers/MaintenanceControllerTests.cs b/ErsatzTV.Tests/Controllers/MaintenanceControllerTests.cs new file mode 100644 index 000000000..35c8416d1 --- /dev/null +++ b/ErsatzTV.Tests/Controllers/MaintenanceControllerTests.cs @@ -0,0 +1,67 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Maintenance; +using ErsatzTV.Controllers.Api; +using ErsatzTV.Core; +using LanguageExt; +using MediatR; +using Microsoft.AspNetCore.Mvc; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using static LanguageExt.Prelude; + +namespace ErsatzTV.Tests.Controllers; + +[TestFixture] +public class MaintenanceControllerTests +{ + private MaintenanceController _controller = null!; + private IMediator _mediator = null!; + private Channel _workerChannel = null!; + + [SetUp] + public void SetUp() + { + _mediator = Substitute.For(); + _workerChannel = Channel.CreateUnbounded(); + _controller = new MaintenanceController(_mediator, _workerChannel.Writer); + } + + [Test] + public async Task EmptyTrash_Should_Return_200_On_Success() + { + _mediator.Send(Arg.Any()) + .Returns(Right(unit)); + + IActionResult result = await _controller.EmptyTrash(); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send(Arg.Any()); + } + + [Test] + public async Task EmptyTrash_Should_Return_422_ProblemDetails_On_Error() + { + _mediator.Send(Arg.Any()) + .Returns(Left(BaseError.New("Failed to empty trash"))); + + IActionResult result = await _controller.EmptyTrash(); + + var unprocessable = result.ShouldBeOfType(); + var problem = unprocessable.Value.ShouldBeOfType(); + problem.Status.ShouldBe(422); + problem.Detail.ShouldBe("Failed to empty trash"); + } + + [Test] + public async Task CleanArtwork_Should_Return_202_And_Enqueue_DeleteOrphanedArtwork() + { + IActionResult result = await _controller.CleanArtwork(CancellationToken.None); + + result.ShouldBeOfType(); + + _workerChannel.Reader.TryRead(out IBackgroundServiceRequest? enqueued).ShouldBeTrue(); + enqueued.ShouldBeOfType(); + } +} diff --git a/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs b/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs index 07dc7a086..be24d0f38 100644 --- a/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlaylistControllerTests.cs @@ -7,8 +7,10 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.MediaCollections; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -26,7 +28,12 @@ public class PlaylistControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new PlaylistController(_mediator); + _controller = new PlaylistController(_mediator) + { + // Provide a real HttpContext so the ETag/If-Match concurrency headers (#253) can be + // read from Request and written to Response. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } private PlaylistController _controller = null!; @@ -204,7 +211,7 @@ public class PlaylistControllerTests public async Task GetById_Should_Return_200_For_Some() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); IActionResult result = await _controller.GetById(4, CancellationToken.None); @@ -226,7 +233,7 @@ public class PlaylistControllerTests public async Task GetItems_Should_Return_200_And_Flatten_Names() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(new List { @@ -280,11 +287,24 @@ public class PlaylistControllerTests await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); } + [Test] + public async Task GetItems_Should_Set_ETag_From_Playlist_Version() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 9))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.GetItems(4, CancellationToken.None); + + _controller.Response.Headers.ETag.ToString().ShouldBe("\"9\""); + } + [Test] public async Task Create_Should_Return_201_And_Map_Request() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Right(new PlaylistViewModel(9, 1, "Kids", false))); + .Returns(Right(new PlaylistViewModel(9, 1, "Kids", false, 1))); IActionResult result = await _controller.Create( new CreatePlaylistRequest(1, "Kids"), @@ -313,8 +333,12 @@ public class PlaylistControllerTests [Test] public async Task Update_Should_Return_200_With_Items_And_Map_Request_By_Array_Order() { + // Existence pre-check reads version 1; the post-save re-query reads the bumped version 2 — + // the response ETag must carry the refreshed value (issue #253). _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns( + Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1)), + Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 2))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Right>(new List { @@ -331,6 +355,22 @@ public class PlaylistControllerTests false, true) })); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List + { + new( + 100, + 0, + CollectionType.Movie, + null, + null, + null, + new NamedMediaItemViewModel(55, "The Movie"), + PlaybackOrder.Shuffle, + null, + false, + true) + }); IActionResult result = await _controller.Update( 4, @@ -346,6 +386,8 @@ public class PlaylistControllerTests result.ShouldBeOfType().Value.ShouldBeOfType>().Count .ShouldBe(1); + // On success the response carries the refreshed playlist's ETag. + _controller.Response.Headers.ETag.ToString().ShouldBe("\"2\""); await _mediator.Received(1).Send( Arg.Is(c => c.PlaylistId == 4 && @@ -358,6 +400,81 @@ public class PlaylistControllerTests Arg.Any()); } + [Test] + public async Task Update_Should_Return_400_On_Malformed_If_Match() + { + _controller.Request.Headers.IfMatch = "not-an-etag"; + + IActionResult result = await _controller.Update( + 4, + new ReplacePlaylistRequest("Kids", new List()), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Update_Should_Thread_If_Match_Version_Into_Command() + { + _controller.Request.Headers.IfMatch = "\"3\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 3))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>(new List())); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + IActionResult result = await _controller.Update( + 4, + new ReplacePlaylistRequest("Kids", new List()), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.Some(3)), + Arg.Any()); + } + + [Test] + public async Task Update_Without_If_Match_Should_Force_Write() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>(new List())); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.Update( + 4, + new ReplacePlaylistRequest("Kids", new List()), + CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.None), + Arg.Any()); + } + + [Test] + public async Task Update_Should_Return_412_On_Precondition_Failed() + { + _controller.Request.Headers.IfMatch = "\"2\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 5))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left>(new PreconditionFailedError("stale"))); + + IActionResult result = await _controller.Update( + 4, + new ReplacePlaylistRequest("Kids", new List()), + CancellationToken.None); + + var objectResult = result.ShouldBeOfType(); + objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed); + } + [Test] public async Task Update_Should_Return_404_When_Playlist_Missing() { @@ -377,7 +494,7 @@ public class PlaylistControllerTests public async Task Update_Should_Return_422_On_Validation_Error() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Left>(BaseError.New("bad item"))); @@ -393,7 +510,7 @@ public class PlaylistControllerTests public async Task Update_Should_Return_422_On_System_Playlist_And_Not_Replace_Items() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true, 1))); IActionResult result = await _controller.Update( 4, @@ -410,7 +527,7 @@ public class PlaylistControllerTests public async Task Delete_Should_Return_204_On_Success() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.None); @@ -438,7 +555,7 @@ public class PlaylistControllerTests public async Task Delete_Should_Return_422_On_System_Playlist() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.Some(BaseError.New("Cannot delete system (generated) playlist"))); @@ -451,7 +568,7 @@ public class PlaylistControllerTests public async Task AddItems_Should_Return_204_And_Map_Request() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Right(Unit.Default)); @@ -488,7 +605,7 @@ public class PlaylistControllerTests public async Task AddItems_Should_Return_422_On_System_Playlist() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "System", true, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Left(BaseError.New("Cannot add items to system (generated) playlist"))); @@ -504,7 +621,7 @@ public class PlaylistControllerTests public async Task AddItems_Should_Return_422_On_Validation_Error() { _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false))); + .Returns(Option.Some(new PlaylistViewModel(4, 1, "Kids", false, 1))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Left(BaseError.New("Movie does not exist"))); diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index 9f99c254d..7eb4e769d 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -532,6 +532,21 @@ public class PlayoutControllerTests result.ShouldBeOfType().Value.ShouldBe(ToResponse(vm)); } + [Test] + public async Task GetById_Should_Expose_IsLocked_From_EntityLocker() + { + PlayoutNameViewModel vm = MakePlayout(9); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(vm)); + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.GetById(9, CancellationToken.None); + + var body = result.ShouldBeOfType().Value.ShouldBeOfType(); + body.IsLocked.ShouldBeTrue(); + body.ShouldBe(ToResponse(vm, isLocked: true)); + } + [Test] public async Task GetById_Should_Return_404_For_None_With_ProblemDetails() { @@ -698,9 +713,17 @@ public class PlayoutControllerTests [Test] public async Task ResetAll_Should_Return_202_And_Send_Command() { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new ResetAllPlayoutsResult([1, 2], [3], [4])); + IActionResult result = await _controller.ResetAll(CancellationToken.None); - result.ShouldBeOfType(); + var accepted = result.ShouldBeOfType(); + accepted.StatusCode.ShouldBe(202); + var body = accepted.Value.ShouldBeOfType(); + body.QueuedPlayoutIds.ShouldBe(new List { 1, 2 }); + body.SkippedLocked.ShouldBe(new List { 3 }); + body.SkippedUnsupported.ShouldBe(new List { 4 }); await _mediator.Received(1).Send(Arg.Any(), Arg.Any()); } @@ -1113,7 +1136,7 @@ public class PlayoutControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns([MakeTemplateViewModel(7), MakeTemplateViewModel(8)]); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Option.Some(new DecoTemplateViewModel(5, 1, "G", "DT"))); + .Returns(Option.Some(new DecoTemplateViewModel(5, 1, "G", "DT", 0))); _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.None); _mediator.Send(Arg.Any(), Arg.Any()) @@ -1252,16 +1275,16 @@ public class PlayoutControllerTests new(0, programScheduleId, [], [], [], false, 1, 1, null, 12, 31, null); private static ProgramScheduleViewModel MakeScheduleVm(int id) => - new(id, $"Schedule {id}", false, false, false, false, FixedStartTimeBehavior.Strict); + new(id, $"Schedule {id}", false, false, false, false, FixedStartTimeBehavior.Strict, 0); private static TemplateViewModel MakeTemplateViewModel(int id) => - new(id, 1, "Group", $"Template {id}"); + new(id, 1, "Group", $"Template {id}", 0); private static PlayoutTemplateViewModel MakeTemplateVm(int id, int index, int templateId, int? decoTemplateId) => new( id, - new TemplateViewModel(templateId, 1, "Group", $"Template {templateId}"), - decoTemplateId is { } dtId ? new DecoTemplateViewModel(dtId, 1, "DGroup", $"Deco {dtId}") : null, + new TemplateViewModel(templateId, 1, "Group", $"Template {templateId}", 0), + decoTemplateId is { } dtId ? new DecoTemplateViewModel(dtId, 1, "DGroup", $"Deco {dtId}", 0) : null, index, [], [], @@ -1292,7 +1315,7 @@ public class PlayoutControllerTests null, 0); - private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm) => + private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm, bool isLocked = false) => PlayoutResponseModel.From( vm.PlayoutId, vm.ScheduleKind, @@ -1309,7 +1332,8 @@ public class PlayoutControllerTests vm.BuildStatus.Success, vm.BuildStatus.Message), vm.DecoId, - vm.DecoName); + vm.DecoName, + isLocked); private static void ShouldHaveActionRoute(string actionName, string httpMethod, string route) { diff --git a/ErsatzTV.Tests/Controllers/PlexMediaSourcesControllerTests.cs b/ErsatzTV.Tests/Controllers/PlexMediaSourcesControllerTests.cs index 4c66dacd1..a2a9a4ec2 100644 --- a/ErsatzTV.Tests/Controllers/PlexMediaSourcesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlexMediaSourcesControllerTests.cs @@ -70,6 +70,10 @@ public class PlexMediaSourcesControllerTests nameof(PlexMediaSourcesController.RefreshLibraries), "POST", "/api/media-sources/plex/{id:int}/refresh-libraries"); + ShouldHaveActionRoute( + nameof(PlexMediaSourcesController.ScanCollections), + "POST", + "/api/media-sources/plex/{id:int}/scan-collections"); } // ----- P1 GetState ----- @@ -452,6 +456,64 @@ public class PlexMediaSourcesControllerTests Arg.Any()); } + // ----- P9 ScanCollections ----- + + [Test] + public async Task ScanCollections_Should_Return_404_When_Source_Missing() + { + SourceExists(false); + + IActionResult result = await _controller.ScanCollections(9, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType(); + _entityLocker.DidNotReceive().LockPlexCollections(); + await _channel.DidNotReceive().WriteAsync( + Arg.Any(), + Arg.Any()); + } + + [Test] + public async Task ScanCollections_Should_Return_409_When_Collections_Locked() + { + SourceExists(true); + _entityLocker.LockPlexCollections().Returns(false); + + IActionResult result = await _controller.ScanCollections(3, cancellationToken: CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + await _channel.DidNotReceive().WriteAsync( + Arg.Any(), + Arg.Any()); + } + + [Test] + public async Task ScanCollections_Should_Return_202_And_Enqueue() + { + SourceExists(true); + _entityLocker.LockPlexCollections().Returns(true); + + IActionResult result = await _controller.ScanCollections(3, deep: true, CancellationToken.None); + + result.ShouldBeOfType(); + await _channel.Received(1).WriteAsync( + Arg.Is(s => s.PlexMediaSourceId == 3 && s.ForceScan && s.DeepScan), + Arg.Any()); + } + + [Test] + public async Task ScanCollections_Should_Compensate_Unlock_When_Enqueue_Throws() + { + SourceExists(true); + _entityLocker.LockPlexCollections().Returns(true); + _channel.WriteAsync(Arg.Any(), Arg.Any()) + .Returns(_ => throw new InvalidOperationException("channel closed")); + + await Should.ThrowAsync( + () => _controller.ScanCollections(3, cancellationToken: CancellationToken.None)); + + _entityLocker.Received(1).UnlockPlexCollections(); + } + private void SourceExists(bool exists) => _mediator.Send(Arg.Any(), Arg.Any()) .Returns(exists diff --git a/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs b/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs index e09792cc3..a0494db99 100644 --- a/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs @@ -10,6 +10,7 @@ using ErsatzTV.Core.Errors; using ErsatzTV.Core.Scheduling; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -30,7 +31,12 @@ public class ScheduleControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new ScheduleController(_mediator); + _controller = new ScheduleController(_mediator) + { + // Provide a real HttpContext so the ETag/If-Match concurrency headers (#253) can be + // read from Request and written to Response. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } [Test] @@ -253,6 +259,10 @@ public class ScheduleControllerTests List items = [MakeOneItem(21), MakeOneItem(22)]; _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Right>(items)); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeSchedule(4, "Daily"))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(items); IActionResult result = await _controller.ReplaceItems( 4, @@ -271,6 +281,94 @@ public class ScheduleControllerTests Arg.Any()); } + [Test] + public async Task GetItems_Should_Set_ETag_From_Schedule_Version() + { + var response = new ProgramScheduleItemsWithDurationViewModel([], null); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeSchedule(4, "Daily", version: 9))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(response); + + await _controller.GetItems(4, CancellationToken.None); + + _controller.Response.Headers.ETag.ToString().ShouldBe("\"9\""); + } + + [Test] + public async Task ReplaceItems_Should_Return_400_On_Malformed_If_Match() + { + _controller.Request.Headers.IfMatch = "not-an-etag"; + + IActionResult result = await _controller.ReplaceItems( + 4, + new ReplaceScheduleItemsRequest([MakeItemRequest(PlayoutMode.One)]), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task ReplaceItems_Should_Thread_If_Match_Version_Into_Command_And_Set_New_ETag() + { + _controller.Request.Headers.IfMatch = "\"3\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([MakeOneItem(21)])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeSchedule(4, "Daily", version: 4))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns([MakeOneItem(21)]); + + IActionResult result = await _controller.ReplaceItems( + 4, + new ReplaceScheduleItemsRequest([MakeItemRequest(PlayoutMode.One)]), + CancellationToken.None); + + result.ShouldBeOfType(); + // On success the response carries the refreshed schedule's ETag. + _controller.Response.Headers.ETag.ToString().ShouldBe("\"4\""); + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.Some(3)), + Arg.Any()); + } + + [Test] + public async Task ReplaceItems_Without_If_Match_Should_Force_Write() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([MakeOneItem(21)])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeSchedule(4, "Daily", version: 1))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns([MakeOneItem(21)]); + + await _controller.ReplaceItems( + 4, + new ReplaceScheduleItemsRequest([MakeItemRequest(PlayoutMode.One)]), + CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.None), + Arg.Any()); + } + + [Test] + public async Task ReplaceItems_Should_Return_412_On_Precondition_Failed() + { + _controller.Request.Headers.IfMatch = "\"2\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left>(new PreconditionFailedError("stale"))); + + IActionResult result = await _controller.ReplaceItems( + 4, + new ReplaceScheduleItemsRequest([MakeItemRequest(PlayoutMode.One)]), + CancellationToken.None); + + var objectResult = result.ShouldBeOfType(); + objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed); + } + [Test] public async Task DeleteItem_Should_Return_204_And_Map_Route_Ids() { @@ -343,8 +441,8 @@ public class ScheduleControllerTests PreferredSubtitleLanguageCode: null, SubtitleMode: null); - private static ProgramScheduleViewModel MakeSchedule(int id, string name) => - new(id, name, true, true, false, false, FixedStartTimeBehavior.Flexible); + private static ProgramScheduleViewModel MakeSchedule(int id, string name, int version = 0) => + new(id, name, true, true, false, false, FixedStartTimeBehavior.Flexible, version); private static ProgramScheduleItemOneViewModel MakeOneItem(int id) => new( diff --git a/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs b/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs index a56f47017..ffe27ace4 100644 --- a/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/TemplateControllerTests.cs @@ -4,8 +4,10 @@ using ErsatzTV.Controllers.Api; using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Scheduling; +using ErsatzTV.Core.Errors; using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; @@ -25,7 +27,12 @@ public class TemplateControllerTests public void SetUp() { _mediator = Substitute.For(); - _controller = new TemplateController(_mediator); + _controller = new TemplateController(_mediator) + { + // Provide a real HttpContext so the ETag/If-Match concurrency headers (#253) can be + // read from Request and written to Response. + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; } [Test] @@ -248,12 +255,10 @@ public class TemplateControllerTests _mediator.Send(Arg.Any(), Arg.Any()) .Returns(Option.Some(MakeTemplate(4, 7, "Morning"))); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(Right>([])); + .Returns(Right>( + [MakeItem(10, "Cartoons", TimeSpan.FromHours(6), 60)])); _mediator.Send(Arg.Any(), Arg.Any()) - .Returns(new List - { - MakeItem(10, "Cartoons", TimeSpan.FromHours(6), 60) - }); + .Returns([MakeItem(10, "Cartoons", TimeSpan.FromHours(6), 60)]); IActionResult result = await _controller.Replace( 4, @@ -305,7 +310,96 @@ public class TemplateControllerTests CancellationToken.None); result.ShouldBeOfType(); - await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task GetItems_Should_Set_ETag_From_Template_Version() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeTemplate(4, 2, "Morning", version: 9))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.GetItems(4, CancellationToken.None); + + _controller.Response.Headers.ETag.ToString().ShouldBe("\"9\""); + } + + [Test] + public async Task Replace_Should_Return_400_On_Malformed_If_Match() + { + _controller.Request.Headers.IfMatch = "not-an-etag"; + + IActionResult result = await _controller.Replace( + 4, + new ReplaceTemplateRequest("Morning", []), + CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Replace_Should_Thread_If_Match_Version_Into_Command() + { + _controller.Request.Headers.IfMatch = "\"3\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeTemplate(4, 7, "Morning", version: 4))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + IActionResult result = await _controller.Replace( + 4, + new ReplaceTemplateRequest("Morning", []), + CancellationToken.None); + + result.ShouldBeOfType(); + // On success the response carries the refreshed template's ETag. + _controller.Response.Headers.ETag.ToString().ShouldBe("\"4\""); + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.Some(3)), + Arg.Any()); + } + + [Test] + public async Task Replace_Without_If_Match_Should_Force_Write() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeTemplate(4, 7, "Morning"))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Right>([])); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new List()); + + await _controller.Replace( + 4, + new ReplaceTemplateRequest("Morning", []), + CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(c => c.ExpectedVersion == Option.None), + Arg.Any()); + } + + [Test] + public async Task Replace_Should_Return_412_On_Precondition_Failed() + { + _controller.Request.Headers.IfMatch = "\"2\""; + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakeTemplate(4, 7, "Morning", version: 5))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Left>(new PreconditionFailedError("stale"))); + + IActionResult result = await _controller.Replace( + 4, + new ReplaceTemplateRequest("Morning", []), + CancellationToken.None); + + var objectResult = result.ShouldBeOfType(); + objectResult.StatusCode.ShouldBe(StatusCodes.Status412PreconditionFailed); } [Test] @@ -359,8 +453,8 @@ public class TemplateControllerTests result.ShouldBeOfType(); } - private static TemplateViewModel MakeTemplate(int id, int groupId, string name) => - new(id, groupId, "Group", name); + private static TemplateViewModel MakeTemplate(int id, int groupId, string name, int version = 1) => + new(id, groupId, "Group", name, version); private static TemplateItemViewModel MakeItem(int blockId, string blockName, TimeSpan startTime, int minutes) { diff --git a/ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs b/ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs index e04ac59cf..31d470256 100644 --- a/ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs @@ -12,6 +12,7 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Troubleshooting; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.FFmpeg; using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Interfaces.Repositories; using ErsatzTV.Core.Interfaces.Troubleshooting; @@ -274,6 +275,55 @@ public class TroubleshootControllerTests .Send(Arg.Any(), Arg.Any()); } + [Test] + public async Task TroubleshootPlayback_Should_Return_404_ProblemDetails_When_Prepare_Not_Found() + { + _entityLocker.IsTroubleshootingPlaybackLocked().Returns(false); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Either.Left( + new ErsatzTV.Core.Errors.NotFoundError("no such media item"))); + + IActionResult result = await _controller.TroubleshootPlayback( + mediaItem: 999, + channel: 0, + ffmpegProfile: 1, + StreamingMode.HttpLiveStreamingSegmenter, + watermark: [], + graphicsElement: [], + streamSelector: string.Empty, + subtitleId: null, + seekSeconds: 0, + start: null, + CancellationToken.None); + + var notFound = result.ShouldBeOfType(); + notFound.Value.ShouldBeOfType().Status.ShouldBe(404); + } + + [Test] + public async Task TroubleshootPlayback_Should_Return_422_ProblemDetails_When_Prepare_Fails_Validation() + { + _entityLocker.IsTroubleshootingPlaybackLocked().Returns(false); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Either.Left(BaseError.New("unable to prepare"))); + + IActionResult result = await _controller.TroubleshootPlayback( + mediaItem: 1, + channel: 0, + ffmpegProfile: 1, + StreamingMode.HttpLiveStreamingSegmenter, + watermark: [], + graphicsElement: [], + streamSelector: string.Empty, + subtitleId: null, + seekSeconds: 0, + start: null, + CancellationToken.None); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(422); + } + [Test] public async Task GetPlaybackStatus_Should_Report_Idle_When_No_Result_And_Unlocked() { diff --git a/ErsatzTV.Tests/Services/ScannerServiceCollectionLockTests.cs b/ErsatzTV.Tests/Services/ScannerServiceCollectionLockTests.cs new file mode 100644 index 000000000..0bbb04539 --- /dev/null +++ b/ErsatzTV.Tests/Services/ScannerServiceCollectionLockTests.cs @@ -0,0 +1,160 @@ +using System.Reflection; +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Emby; +using ErsatzTV.Application.Jellyfin; +using ErsatzTV.Application.Plex; +using ErsatzTV.Core; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Infrastructure.Locking; +using ErsatzTV.Services; +using LanguageExt; +using MediatR; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Services; + +/// +/// Regression net for the #250 cross-release bug on the per-provider collections lock (#235): +/// a scheduler-enqueued collection scan running with Unlock: false must NOT release a +/// collections lock owned by another party (an in-flight API scan), while Unlock: true +/// (an API request or the last message of a scheduler batch) must release on completion. +/// +[TestFixture] +public class ScannerServiceCollectionLockTests +{ + [Test] + public async Task Plex_UnlockFalse_DoesNotReleaseHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizePlexCollections(1, false, false, Unlock: false)); + + // another party (an API scan) owns the collections lock + locker.LockPlexCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.ArePlexCollectionsLocked().ShouldBeTrue(); + } + + [Test] + public async Task Plex_UnlockTrue_ReleasesHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizePlexCollections(1, false, false, Unlock: true)); + + locker.LockPlexCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.ArePlexCollectionsLocked().ShouldBeFalse(); + } + + [Test] + public async Task Jellyfin_UnlockFalse_DoesNotReleaseHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizeJellyfinCollections(1, false, false, Unlock: false)); + + locker.LockJellyfinCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.AreJellyfinCollectionsLocked().ShouldBeTrue(); + } + + [Test] + public async Task Jellyfin_UnlockTrue_ReleasesHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizeJellyfinCollections(1, false, false, Unlock: true)); + + locker.LockJellyfinCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.AreJellyfinCollectionsLocked().ShouldBeFalse(); + } + + [Test] + public async Task Emby_UnlockFalse_DoesNotReleaseHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizeEmbyCollections(1, false, false, Unlock: false)); + + locker.LockEmbyCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.AreEmbyCollectionsLocked().ShouldBeTrue(); + } + + [Test] + public async Task Emby_UnlockTrue_ReleasesHeldLock() + { + (ScannerService service, EntityLocker locker) = BuildService( + new SynchronizeEmbyCollections(1, false, false, Unlock: true)); + + locker.LockEmbyCollections().ShouldBeTrue(); + + await RunToCompletion(service); + + locker.AreEmbyCollectionsLocked().ShouldBeFalse(); + } + + private static (ScannerService, EntityLocker) BuildService(IScannerBackgroundServiceRequest request) + { + var mediator = Substitute.For(); + // every collection sync request returns success; the finally is what we're exercising + Either ok = LanguageExt.Unit.Default; + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(ok); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(ok); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(ok); + + var locker = new EntityLocker(mediator, NullLogger.Instance); + + var provider = Substitute.For(); + provider.GetService(typeof(IMediator)).Returns(mediator); + provider.GetService(typeof(IEntityLocker)).Returns(locker); + + var scope = Substitute.For(); + scope.ServiceProvider.Returns(provider); + + var scopeFactory = Substitute.For(); + scopeFactory.CreateScope().Returns(scope); + + var channel = Channel.CreateUnbounded(); + channel.Writer.TryWrite(request).ShouldBeTrue(); + channel.Writer.Complete(); + + var startup = new SystemStartup(); + startup.DatabaseIsReady(); + startup.SearchIndexIsReady(); + + var service = new ScannerService( + channel.Reader, + scopeFactory, + startup, + NullLogger.Instance); + + return (service, locker); + } + + private static async Task RunToCompletion(ScannerService service) + { + MethodInfo executeAsync = typeof(ScannerService) + .GetMethod("ExecuteAsync", BindingFlags.NonPublic | BindingFlags.Instance)!; + + // the channel writer is already completed, so the read loop drains the one queued + // request, runs its finally, then ExecuteAsync returns + var task = (Task)executeAsync.Invoke(service, [CancellationToken.None])!; + await task.WaitAsync(TimeSpan.FromSeconds(10)); + } +} diff --git a/ErsatzTV.Tests/Services/SchedulerServiceCollectionLockTests.cs b/ErsatzTV.Tests/Services/SchedulerServiceCollectionLockTests.cs new file mode 100644 index 000000000..68c229d88 --- /dev/null +++ b/ErsatzTV.Tests/Services/SchedulerServiceCollectionLockTests.cs @@ -0,0 +1,150 @@ +using System.Reflection; +using System.Threading.Channels; +using SysChannels = System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Plex; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Services; +using ErsatzTV.Tests.Support; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Services; + +/// +/// Verifies the scheduler's lock-once-per-provider collection batch (#235): it acquires +/// LockPlexCollections() exactly once, and when the slot is already held (an API scan or a +/// prior tick) it SKIPS the collection enqueue entirely rather than enqueuing an unlocked scan +/// that would cross-release the holder's lock (#250). When it does acquire, only the LAST message +/// carries the release (Unlock: true). +/// +[TestFixture] +public class SchedulerServiceCollectionLockTests +{ + private InMemoryTvContext _db; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + [Test] + public async Task Plex_SkipsCollectionEnqueue_WhenCollectionsLockAlreadyHeld() + { + await SeedPlexLibrary(id: 1, mediaSourceId: 7); + + var locker = Substitute.For(); + // isolate the collection path: the library loop enqueues nothing but still records the source id + locker.LockLibrary(Arg.Any()).Returns(false); + // the collections slot is already held by another party + locker.LockPlexCollections().Returns(false); + + (SchedulerService service, SysChannels.Channel scannerChannel) = BuildService(locker); + + await InvokeScanPlex(service); + List writes = await Drain(scannerChannel); + + locker.Received(1).LockPlexCollections(); + writes.OfType().ShouldBeEmpty(); + locker.DidNotReceive().UnlockPlexCollections(); + } + + [Test] + public async Task Plex_EnqueuesBatch_LastMessageOwnsRelease_WhenLockAcquired() + { + await SeedPlexLibrary(id: 1, mediaSourceId: 7); + await SeedPlexLibrary(id: 2, mediaSourceId: 8); + + var locker = Substitute.For(); + locker.LockLibrary(Arg.Any()).Returns(false); + locker.LockPlexCollections().Returns(true); + + (SchedulerService service, SysChannels.Channel scannerChannel) = BuildService(locker); + + await InvokeScanPlex(service); + List writes = await Drain(scannerChannel); + + locker.Received(1).LockPlexCollections(); + + List collectionScans = writes.OfType().ToList(); + collectionScans.Count.ShouldBe(2); + // exactly one message owns the single release, and it is the last one written + collectionScans.Count(m => m.Unlock).ShouldBe(1); + collectionScans.Last().Unlock.ShouldBeTrue(); + collectionScans.First().Unlock.ShouldBeFalse(); + + // release is handed off to the last message; no compensating unlock on the happy path + locker.DidNotReceive().UnlockPlexCollections(); + } + + private async Task SeedPlexLibrary(int id, int mediaSourceId) + { + await using TvContext context = _db.CreateContext(); + context.PlexLibraries.Add( + new PlexLibrary + { + Id = id, + Name = $"Plex {id}", + MediaKind = LibraryMediaKind.Movies, + MediaSourceId = mediaSourceId, + ShouldSyncItems = true, + Key = $"key-{id}" + }); + await context.SaveChangesAsync(); + } + + private (SchedulerService, SysChannels.Channel) BuildService(IEntityLocker locker) + { + var provider = Substitute.For(); + provider.GetService(typeof(TvContext)).Returns(_ => _db.CreateContext()); + + var scope = Substitute.For(); + scope.ServiceProvider.Returns(provider); + + var scopeFactory = Substitute.For(); + scopeFactory.CreateScope().Returns(scope); + + var workerChannel = SysChannels.Channel.CreateUnbounded(); + var scannerChannel = SysChannels.Channel.CreateUnbounded(); + + var startup = new SystemStartup(); + + var service = new SchedulerService( + scopeFactory, + workerChannel.Writer, + scannerChannel.Writer, + locker, + startup, + NullLogger.Instance); + + return (service, scannerChannel); + } + + private static async Task> Drain( + SysChannels.Channel channel) + { + channel.Writer.Complete(); + var results = new List(); + await foreach (IScannerBackgroundServiceRequest request in channel.Reader.ReadAllAsync()) + { + results.Add(request); + } + + return results; + } + + private static async Task InvokeScanPlex(SchedulerService service) + { + MethodInfo method = typeof(SchedulerService) + .GetMethod("ScanPlexMediaSources", BindingFlags.NonPublic | BindingFlags.Instance)!; + var task = (Task)method.Invoke(service, [CancellationToken.None])!; + await task.WaitAsync(TimeSpan.FromSeconds(10)); + } +} diff --git a/ErsatzTV.Tests/Services/WorkerServiceTests.cs b/ErsatzTV.Tests/Services/WorkerServiceTests.cs new file mode 100644 index 000000000..96c6cd404 --- /dev/null +++ b/ErsatzTV.Tests/Services/WorkerServiceTests.cs @@ -0,0 +1,101 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Core; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Services; +using LanguageExt; +using MediatR; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using Unit = LanguageExt.Unit; + +namespace ErsatzTV.Tests.Services; + +[TestFixture] +public class WorkerServiceTests +{ + // F7 regression (issue #235): the global Trakt lock is acquired by SchedulerService and released + // only when the *terminal* (Unlock: true) message of a batch is processed by WorkerService. If the + // worker stops before reaching that terminal message (shutdown break / channel completion / + // cancellation), the release never fires and the in-memory Trakt lock leaks for the life of the + // process. WorkerService must release a held Trakt lock as a compensating action on shutdown. + [Test] + public async Task Should_Release_Held_Trakt_Lock_When_Worker_Stops_Before_Terminal_Message() + { + var channel = Channel.CreateUnbounded(); + + // A Trakt batch: only the terminal message carries Unlock: true. If the worker never processes + // it, the handler-side UnlockTrakt() never runs. + AddTraktList nonTerminal = AddTraktList.Existing("user", "list-1", false); + AddTraktList terminal = AddTraktList.Existing("user", "list-2", true); + await channel.Writer.WriteAsync(nonTerminal); + await channel.Writer.WriteAsync(terminal); + + // Stateful stand-in for the singleton EntityLocker: the lock starts held (a batch acquired it). + var traktLocked = 1; + var locker = Substitute.For(); + locker.IsTraktLocked().Returns(_ => Volatile.Read(ref traktLocked) == 1); + locker.UnlockTrakt().Returns(_ => Interlocked.Exchange(ref traktLocked, 0) == 1); + + // Gate: processing the first (non-terminal) message parks on the stopping token, guaranteeing + // the worker never reaches the terminal (Unlock: true) message before it is stopped. + var firstSeen = new TaskCompletionSource(); + var processed = new List(); + var mediator = Substitute.For(); + mediator.Send(Arg.Any(), Arg.Any()) + .Returns(async call => + { + lock (processed) + { + processed.Add(call.Arg()); + } + + firstSeen.TrySetResult(); + + // Block on the stopping token so the loop parks here until StopAsync cancels it. + await Task.Delay(Timeout.Infinite, call.Arg()); + return (Either)Unit.Default; + }); + + var provider = Substitute.For(); + provider.GetService(typeof(IMediator)).Returns(mediator); + var scope = Substitute.For(); + scope.ServiceProvider.Returns(provider); + var scopeFactory = Substitute.For(); + scopeFactory.CreateScope().Returns(scope); + + var worker = new WorkerService( + channel.Reader, + scopeFactory, + locker, + NullLogger.Instance); + + await worker.StartAsync(CancellationToken.None); + + // Wait until the first message is actively being processed (parked on the stopping token). + await firstSeen.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + // Shut the worker down: cancels the stopping token -> parked Delay throws -> loop breaks + // before the terminal message is ever processed. + await worker.StopAsync(CancellationToken.None); + + // The compensating release must have fired even though the terminal message was never handled. + locker.IsTraktLocked().ShouldBeFalse(); + locker.Received(1).UnlockTrakt(); + + // Prove the leak scenario is genuine: we stopped after the non-terminal message but before the + // terminal (Unlock: true) one, so the handler-side release could not have run. + List seen; + lock (processed) + { + seen = processed.ToList(); + } + + seen.ShouldContain(nonTerminal); + seen.ShouldNotContain(terminal); + } +} diff --git a/ErsatzTV/Controllers/Api/ChannelController.cs b/ErsatzTV/Controllers/Api/ChannelController.cs index e70f1a833..d2357e9e7 100644 --- a/ErsatzTV/Controllers/Api/ChannelController.cs +++ b/ErsatzTV/Controllers/Api/ChannelController.cs @@ -211,7 +211,7 @@ public class ChannelController( "progress) and all other playout kinds use Reset (rebuild from scratch), matching the Blazor UI. " + "Pass mode to force a specific PlayoutBuildMode.")] [EndpointGroupName("general")] - [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status202Accepted)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] public async Task ResetPlayout( @@ -233,7 +233,7 @@ public class ChannelController( PlayoutBuildMode buildMode = mode ?? await DefaultResetMode(playoutId, cancellationToken); await workerChannel.WriteAsync(new BuildPlayout(playoutId, buildMode), cancellationToken); - return new OkResult(); + return new AcceptedResult(); } return ApiResults.NotFoundProblem(); diff --git a/ErsatzTV/Controllers/Api/DecoTemplateController.cs b/ErsatzTV/Controllers/Api/DecoTemplateController.cs index 82a316239..53c8e122e 100644 --- a/ErsatzTV/Controllers/Api/DecoTemplateController.cs +++ b/ErsatzTV/Controllers/Api/DecoTemplateController.cs @@ -143,6 +143,9 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase [HttpGet("/api/deco-templates/{id:int}/items")] [Tags("DecoTemplates")] [EndpointSummary("Get deco template items")] + [EndpointDescription( + "Returns the deco template's items and a strong ETag of the deco template's version. Pass that ETag " + + "back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] @@ -154,6 +157,9 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // The items GET returns children, not the root, so read the deco template's version for the ETag. + ConcurrencyHeaders.SetETag(Response, decoTemplate.Map(t => t.Version).IfNone(0)); + List items = await mediator.Send(new GetDecoTemplateItems(id), cancellationToken); return new OkObjectResult( items.OrderBy(i => i.StartTime.TimeOfDay).Map(ProjectToResponseModel).ToList()); @@ -164,16 +170,32 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase [EndpointSummary("Replace a deco template and its items")] [EndpointDescription( "Replaces the deco template's name and its full item list. Each item assigns a deco to a start/end time " + - "of day; an end time of 00:00:00 means the item runs to the end of the day. Items must not overlap.")] + "of day; an end time of 00:00:00 means the item runs to the end of the day. Items must not overlap. " + + "Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); " + + "a successful response carries the new ETag.")] [EndpointGroupName("general")] [ProducesResponseType(typeof(DecoTemplateWithItemsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Replace( int id, [Required] [FromBody] ReplaceDecoTemplateRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return new BadRequestObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status400BadRequest, + Title = "Invalid If-Match header", + Detail = "If-Match must be a strong ETag of the resource version (e.g. \"3\") or \"*\"." + }); + } + Option maybeDecoTemplate = await mediator.Send(new GetDecoTemplateById(id), cancellationToken); if (maybeDecoTemplate.IsNone) @@ -184,18 +206,27 @@ public class DecoTemplateController(IMediator mediator) : ControllerBase int decoTemplateGroupId = maybeDecoTemplate.Map(t => t.DecoTemplateGroupId).IfNone(0); Either> result = - await mediator.Send(request.ToCommand(decoTemplateGroupId, id), cancellationToken); + await mediator.Send( + request.ToCommand(decoTemplateGroupId, id, ifMatch.ExpectedVersion), + cancellationToken); return await result.Match( Left: error => Task.FromResult(error.ToErrorResult()), 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 refreshed = await mediator.Send(new GetDecoTemplateById(id), cancellationToken); List items = await mediator.Send(new GetDecoTemplateItems(id), cancellationToken); return refreshed.Match( - Some: vm => (IActionResult)new OkObjectResult(ProjectToWithItemsResponseModel(vm, items)), + 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(ProjectToWithItemsResponseModel(vm, items)); + }, None: () => ApiResults.NotFoundProblem()); }); } diff --git a/ErsatzTV/Controllers/Api/EmbyMediaSourcesController.cs b/ErsatzTV/Controllers/Api/EmbyMediaSourcesController.cs index 9780e20f4..a63450fda 100644 --- a/ErsatzTV/Controllers/Api/EmbyMediaSourcesController.cs +++ b/ErsatzTV/Controllers/Api/EmbyMediaSourcesController.cs @@ -278,6 +278,53 @@ public class EmbyMediaSourcesController( return new AcceptedResult(); } + [HttpPost("/api/media-sources/emby/{id:int}/scan-collections", Name = "ScanEmbyCollections")] + [Tags("Emby")] + [EndpointSummary("Scan an Emby source's collections")] + [EndpointDescription( + "Queues a synchronization of the source's collections (fire-and-forget). Pass ?deep=true for a deep " + + "scan. Returns 409 while an Emby collections scan is already in progress.")] + [EndpointGroupName("general")] + [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + public async Task ScanCollections( + int id, + [FromQuery] bool deep = false, + CancellationToken cancellationToken = default) + { + Option maybeSource = + await mediator.Send(new GetEmbyMediaSourceById(id), cancellationToken); + if (maybeSource.IsNone) + { + return ApiResults.NotFoundProblem(); + } + + // The collections lock IS the running scan (§3b): fail to acquire = a scan is already active → 409. + if (!entityLocker.LockEmbyCollections()) + { + return ApiResults.ConflictProblem( + "Emby collections scan in progress", + "An Emby collections scan is already in progress; try again once it completes."); + } + + try + { + await scannerWorkerChannel.WriteAsync( + new SynchronizeEmbyCollections(id, true, deep), + cancellationToken); + } + catch + { + // the scanner releases the lock when it processes the message; if the enqueue throws after we + // acquired the lock, release it here (EnqueueWithTraktLock compensating-unlock, §3b) + entityLocker.UnlockEmbyCollections(); + throw; + } + + return new AcceptedResult(); + } + // §C7: LockLibrary then enqueue the sync pair; a throw from the enqueue compensates by unlocking // (EnqueueWithTraktLock pattern) — a locked library is silently skipped (Blazor parity). private async Task EnqueueLibrarySync(int sourceId, int libraryId, CancellationToken cancellationToken) diff --git a/ErsatzTV/Controllers/Api/JellyfinMediaSourcesController.cs b/ErsatzTV/Controllers/Api/JellyfinMediaSourcesController.cs index aa767ed14..d3a96d296 100644 --- a/ErsatzTV/Controllers/Api/JellyfinMediaSourcesController.cs +++ b/ErsatzTV/Controllers/Api/JellyfinMediaSourcesController.cs @@ -278,6 +278,53 @@ public class JellyfinMediaSourcesController( return new AcceptedResult(); } + [HttpPost("/api/media-sources/jellyfin/{id:int}/scan-collections", Name = "ScanJellyfinCollections")] + [Tags("Jellyfin")] + [EndpointSummary("Scan a Jellyfin source's collections")] + [EndpointDescription( + "Queues a synchronization of the source's collections (fire-and-forget). Pass ?deep=true for a deep " + + "scan. Returns 409 while a Jellyfin collections scan is already in progress.")] + [EndpointGroupName("general")] + [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + public async Task ScanCollections( + int id, + [FromQuery] bool deep = false, + CancellationToken cancellationToken = default) + { + Option maybeSource = + await mediator.Send(new GetJellyfinMediaSourceById(id), cancellationToken); + if (maybeSource.IsNone) + { + return ApiResults.NotFoundProblem(); + } + + // The collections lock IS the running scan (§3b): fail to acquire = a scan is already active → 409. + if (!entityLocker.LockJellyfinCollections()) + { + return ApiResults.ConflictProblem( + "Jellyfin collections scan in progress", + "A Jellyfin collections scan is already in progress; try again once it completes."); + } + + try + { + await scannerWorkerChannel.WriteAsync( + new SynchronizeJellyfinCollections(id, true, deep), + cancellationToken); + } + catch + { + // the scanner releases the lock when it processes the message; if the enqueue throws after we + // acquired the lock, release it here (EnqueueWithTraktLock compensating-unlock, §3b) + entityLocker.UnlockJellyfinCollections(); + throw; + } + + return new AcceptedResult(); + } + // §C7: LockLibrary then enqueue the sync pair; a throw from the enqueue compensates by unlocking // (EnqueueWithTraktLock pattern) — a locked library is silently skipped (Blazor parity). private async Task EnqueueLibrarySync(int sourceId, int libraryId, CancellationToken cancellationToken) diff --git a/ErsatzTV/Controllers/Api/LibrariesController.cs b/ErsatzTV/Controllers/Api/LibrariesController.cs index d8525a52d..bd37c0bca 100644 --- a/ErsatzTV/Controllers/Api/LibrariesController.cs +++ b/ErsatzTV/Controllers/Api/LibrariesController.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using ErsatzTV.Application.Libraries; using ErsatzTV.Core.Api.Libraries; using ErsatzTV.Core.Interfaces.Repositories; @@ -22,13 +23,18 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe [HttpPost("/api/libraries/{id:int}/scan")] [Tags("Libraries")] [EndpointSummary("Scan library")] + [EndpointDescription("Queues a scan of the whole library. Pass ?deep=true for a deep (metadata-refresh) scan.")] [ProducesResponseType(StatusCodes.Status202Accepted)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] - public async Task ScanLibrary(int id, CancellationToken cancellationToken) + public async Task ScanLibrary( + int id, + [FromQuery] bool deep = false, + CancellationToken cancellationToken = default) { - QueueLibraryScanResult result = await mediator.Send(new QueueLibraryScanByLibraryId(id), cancellationToken); + QueueLibraryScanResult result = + await mediator.Send(new QueueLibraryScanByLibraryId(id, deep), cancellationToken); return result switch { QueueLibraryScanResult.Queued => new AcceptedResult(), @@ -49,19 +55,48 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe [HttpPost("/api/libraries/{id:int}/scan-show")] [Tags("Libraries")] [EndpointSummary("Scan show")] - [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status202Accepted)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ScanShow(int id, [FromBody] ScanShowRequest request) { Option maybeTitle = await televisionRepository.GetShowTitle(id, request.ShowId); foreach (string title in maybeTitle) { - bool result = await mediator.Send(new QueueShowScanByLibraryId(id, request.ShowId, title, request.DeepScan)); + QueueShowScanResult result = + await mediator.Send(new QueueShowScanByLibraryId(id, request.ShowId, title, request.DeepScan)); - return result - ? new OkResult() - : new BadRequestObjectResult(new { error = "Unable to queue show scan. Library may not exist, may not support single show scanning, or may already be scanning." }); + return result switch + { + QueueShowScanResult.Queued => new AcceptedResult(), + QueueShowScanResult.AlreadyScanning => ApiResults.ConflictProblem( + "Library scan in progress", + $"A scan for library {id} is already in progress; cannot scan an individual show."), + QueueShowScanResult.SyncDisabled => new UnprocessableEntityObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status422UnprocessableEntity, + Title = "Library sync is disabled", + Detail = $"Item sync is disabled for library {id}." + }), + QueueShowScanResult.Unsupported => new UnprocessableEntityObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status422UnprocessableEntity, + Title = "Single show scanning is not supported", + Detail = $"Library {id} does not support scanning an individual show." + }), + QueueShowScanResult.ScanFailed => new UnprocessableEntityObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status422UnprocessableEntity, + Title = "Unable to scan show", + Detail = $"The scan for show {request.ShowId} in library {id} could not be completed." + }), + QueueShowScanResult.NotFound => ApiResults.NotFoundProblem($"Library {id} does not exist."), + _ => throw new UnreachableException($"Unmapped QueueShowScanResult: {result}") + }; } return ApiResults.NotFoundProblem($"Show {request.ShowId} does not exist in library {id}."); diff --git a/ErsatzTV/Controllers/Api/MaintenanceController.cs b/ErsatzTV/Controllers/Api/MaintenanceController.cs index b6009e32a..2093207b7 100644 --- a/ErsatzTV/Controllers/Api/MaintenanceController.cs +++ b/ErsatzTV/Controllers/Api/MaintenanceController.cs @@ -2,6 +2,7 @@ using System.Threading.Channels; using ErsatzTV.Application; using ErsatzTV.Application.Maintenance; using ErsatzTV.Core; +using ErsatzTV.Extensions; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -23,17 +24,14 @@ public class MaintenanceController(IMediator mediator, ChannelWriter EmptyTrash() { Either result = await mediator.Send(new EmptyTrash()); foreach (BaseError error in result.LeftToSeq()) { - return new ContentResult - { - StatusCode = StatusCodes.Status500InternalServerError, - Content = error.ToString(), - ContentType = "text/plain" - }; + return error.ToErrorResult(); } return new OkResult(); @@ -42,9 +40,10 @@ public class MaintenanceController(IMediator mediator, ChannelWriter CleanArtwork(CancellationToken cancellationToken) { await workerChannel.WriteAsync(new DeleteOrphanedArtwork(), cancellationToken); - return new OkResult(); + return new AcceptedResult(); } } diff --git a/ErsatzTV/Controllers/Api/PlaylistController.cs b/ErsatzTV/Controllers/Api/PlaylistController.cs index 85055617d..be9e152fd 100644 --- a/ErsatzTV/Controllers/Api/PlaylistController.cs +++ b/ErsatzTV/Controllers/Api/PlaylistController.cs @@ -134,6 +134,9 @@ public class PlaylistController(IMediator mediator) : ControllerBase [HttpGet("/api/playlists/{id:int}/items", Name = "GetPlaylistItems")] [Tags("Playlists")] [EndpointSummary("Get the items in a playlist")] + [EndpointDescription( + "Returns the playlist's items and a strong ETag of the playlist's version. Pass that ETag back " + + "as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] @@ -145,6 +148,9 @@ public class PlaylistController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // The items GET returns children, not the root, so read the playlist's version for the ETag. + ConcurrencyHeaders.SetETag(Response, maybePlaylist.Map(p => p.Version).IfNone(0)); + List items = await mediator.Send(new GetPlaylistItems(id), cancellationToken); return new OkObjectResult(items.Map(ProjectToItemResponse).ToList()); } @@ -168,15 +174,33 @@ public class PlaylistController(IMediator mediator) : ControllerBase [HttpPut("/api/playlists/{id:int}", Name = "UpdatePlaylist")] [Tags("Playlists")] [EndpointSummary("Update a playlist (rename and replace its items)")] + [EndpointDescription( + "Replaces the playlist's name and its full item list. Item indexes are assigned from the array " + + "order. Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 " + + "(issue #253); a successful response carries the new ETag.")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Update( int id, [Required] [FromBody] ReplacePlaylistRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return new BadRequestObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status400BadRequest, + Title = "Invalid If-Match header", + Detail = "If-Match must be a strong ETag of the resource version (e.g. \"3\") or \"*\"." + }); + } + Option maybePlaylist = await mediator.Send(new GetPlaylistById(id), cancellationToken); if (maybePlaylist.IsNone) { @@ -195,10 +219,25 @@ public class PlaylistController(IMediator mediator) : ControllerBase } Either> result = - await mediator.Send(request.ToCommand(id), cancellationToken); - return result.Match( - Left: error => error.ToErrorResult(), - Right: items => (IActionResult)new OkObjectResult(items.Map(ProjectToItemResponse).ToList())); + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); + return await result.Match( + Left: error => Task.FromResult(error.ToErrorResult()), + 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 refreshed = + await mediator.Send(new GetPlaylistById(id), cancellationToken); + List 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()); + }); } [HttpDelete("/api/playlists/{id:int}", Name = "DeletePlaylist")] diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index 0e144e342..42eb40043 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -69,7 +69,7 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : public async Task GetById(int id, CancellationToken cancellationToken) { Option result = await mediator.Send(new GetPlayoutById(id), cancellationToken); - return result.Map(ToResponse).ToGetResult(); + return result.Map(vm => ToResponse(vm, entityLocker.IsPlayoutLocked(id))).ToGetResult(); } [HttpGet("/api/playouts/{id:int}/items", Name = "GetPlayoutItems")] @@ -128,7 +128,9 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : Option playout = await mediator.Send(new GetPlayoutById(created.PlayoutId), cancellationToken); return playout.Match( - Some: vm => (IActionResult)new CreatedResult($"/api/playouts/{vm.PlayoutId}", ToResponse(vm)), + Some: vm => (IActionResult)new CreatedResult( + $"/api/playouts/{vm.PlayoutId}", + ToResponse(vm, entityLocker.IsPlayoutLocked(vm.PlayoutId))), None: () => ApiResults.NotFoundProblem()); }); }); @@ -194,7 +196,8 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : return result.Match( Left: error => error.ToErrorResult(), - Right: playout => (IActionResult)new OkObjectResult(ToResponse(playout))); + Right: playout => (IActionResult)new OkObjectResult( + ToResponse(playout, entityLocker.IsPlayoutLocked(id)))); } private async Task> UpdateScheduleFile( @@ -260,7 +263,7 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : Option refreshed = await mediator.Send(new GetPlayoutById(id), cancellationToken); return refreshed.Match( - Some: vm => (IActionResult)new OkObjectResult(ToResponse(vm)), + Some: vm => (IActionResult)new OkObjectResult(ToResponse(vm, entityLocker.IsPlayoutLocked(id))), None: () => ApiResults.NotFoundProblem()); } @@ -583,14 +586,19 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : [Tags("Playouts")] [EndpointSummary("Reset all playouts")] [EndpointGroupName("general")] - [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ResetAllPlayoutsResponseModel), StatusCodes.Status202Accepted)] // No 409 lock guard here (unlike the id-keyed mutations): ResetAllPlayoutsHandler already // skips any playout whose build lock is held, matching Blazor. It is a fire-and-forget - // bulk enqueue, so it always accepts. See docs/decisions.md 2026-07-10. + // bulk enqueue, so it always accepts — the 202 body reports which playouts were queued and + // which were skipped (locked, or an unsupported ExternalJson/None kind). See docs/decisions.md 2026-07-10. public async Task ResetAll(CancellationToken cancellationToken) { - await mediator.Send(new ResetAllPlayouts(), cancellationToken); - return Accepted(); + ResetAllPlayoutsResult result = await mediator.Send(new ResetAllPlayouts(), cancellationToken); + var body = new ResetAllPlayoutsResponseModel( + result.QueuedPlayoutIds, + result.SkippedLocked, + result.SkippedUnsupported); + return new AcceptedResult((string)null, body); } [HttpPost("/api/playouts/{id:int}/erase-items", Name = "ErasePlayoutItems")] @@ -761,7 +769,7 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : private static PlayoutHistoryDetailsResponseModel ToDetailsResponse(PlayoutHistoryDetailsViewModel vm) => new(vm.PlaybackOrder, vm.CollectionType, vm.Name, vm.MediaItemType, vm.MediaItemTitle); - private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm) => + private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm, bool isLocked) => PlayoutResponseModel.From( vm.PlayoutId, vm.ScheduleKind, @@ -773,7 +781,8 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : vm.DbDailyRebuildTime, ToBuildStatus(vm.BuildStatus), vm.DecoId, - vm.DecoName); + vm.DecoName, + isLocked); private static PlayoutAlternateScheduleResponseModel ToResponse(PlayoutAlternateScheduleViewModel vm) => new( diff --git a/ErsatzTV/Controllers/Api/PlexMediaSourcesController.cs b/ErsatzTV/Controllers/Api/PlexMediaSourcesController.cs index e1cc451d6..ff6a48098 100644 --- a/ErsatzTV/Controllers/Api/PlexMediaSourcesController.cs +++ b/ErsatzTV/Controllers/Api/PlexMediaSourcesController.cs @@ -277,6 +277,51 @@ public class PlexMediaSourcesController( return new AcceptedResult(); } + [HttpPost("/api/media-sources/plex/{id:int}/scan-collections", Name = "ScanPlexCollections")] + [Tags("Plex")] + [EndpointSummary("Scan a Plex server's collections")] + [EndpointDescription( + "Queues a synchronization of the server's collections (fire-and-forget). Pass ?deep=true for a deep " + + "scan. Returns 409 while a Plex collections scan is already in progress.")] + [EndpointGroupName("general")] + [ProducesResponseType(StatusCodes.Status202Accepted)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + public async Task ScanCollections( + int id, + [FromQuery] bool deep = false, + CancellationToken cancellationToken = default) + { + if (!await PlexSourceExists(id, cancellationToken)) + { + return ApiResults.NotFoundProblem(); + } + + // The collections lock IS the running scan (§3b): fail to acquire = a scan is already active → 409. + if (!entityLocker.LockPlexCollections()) + { + return ApiResults.ConflictProblem( + "Plex collections scan in progress", + "A Plex collections scan is already in progress; try again once it completes."); + } + + try + { + await scannerWorkerChannel.WriteAsync( + new SynchronizePlexCollections(id, true, deep), + cancellationToken); + } + catch + { + // the scanner releases the lock when it processes the message; if the enqueue throws after we + // acquired the lock, release it here (EnqueueWithTraktLock compensating-unlock, §3b) + entityLocker.UnlockPlexCollections(); + throw; + } + + return new AcceptedResult(); + } + private async Task PlexSourceExists(int id, CancellationToken cancellationToken) => (await mediator.Send(new GetPlexMediaSourceById(id), cancellationToken)).IsSome; diff --git a/ErsatzTV/Controllers/Api/Requests/ReplaceDecoTemplateRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplaceDecoTemplateRequest.cs index e649a6a1e..734655928 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplaceDecoTemplateRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplaceDecoTemplateRequest.cs @@ -4,10 +4,14 @@ namespace ErsatzTV.Controllers.Api.Requests; public record ReplaceDecoTemplateRequest(string Name, List Items) { - public ReplaceDecoTemplateItems ToCommand(int decoTemplateGroupId, int decoTemplateId) => + public ReplaceDecoTemplateItems ToCommand( + int decoTemplateGroupId, + int decoTemplateId, + Option expectedVersion = default) => new( decoTemplateId, decoTemplateGroupId, Name, - (Items ?? []).Select(item => item.ToReplaceItem()).ToList()); + (Items ?? []).Select(item => item.ToReplaceItem()).ToList(), + expectedVersion); } diff --git a/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs index 2e1ad4ef4..6f5bdc5f6 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplacePlaylistRequest.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using ErsatzTV.Application.MediaCollections; using ErsatzTV.Core.Domain; +using LanguageExt; namespace ErsatzTV.Controllers.Api.Requests; @@ -56,8 +57,8 @@ public record PlaylistItemRequest( public record ReplacePlaylistRequest(string? Name, List? Items) { - public ReplacePlaylistItems ToCommand(int id) => - new(id, Name ?? string.Empty, BuildItems()); + public ReplacePlaylistItems ToCommand(int id, Option expectedVersion = default) => + new(id, Name ?? string.Empty, BuildItems(), expectedVersion); // Preview operates on the posted draft, so there is no persisted playlist id (0). public ReplacePlaylistItems ToReplaceCommand() => diff --git a/ErsatzTV/Controllers/Api/Requests/ReplaceScheduleItemsRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplaceScheduleItemsRequest.cs index 40a8f6864..fd17ec5df 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplaceScheduleItemsRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplaceScheduleItemsRequest.cs @@ -4,8 +4,9 @@ namespace ErsatzTV.Controllers.Api.Requests; public record ReplaceScheduleItemsRequest(List Items) { - public ReplaceProgramScheduleItems ToCommand(int scheduleId) => + public ReplaceProgramScheduleItems ToCommand(int scheduleId, Option expectedVersion = default) => new( scheduleId, - (Items ?? []).Select((item, index) => item.ToReplaceCommand(index)).ToList()); + (Items ?? []).Select((item, index) => item.ToReplaceCommand(index)).ToList(), + expectedVersion); } diff --git a/ErsatzTV/Controllers/Api/Requests/ReplaceTemplateRequest.cs b/ErsatzTV/Controllers/Api/Requests/ReplaceTemplateRequest.cs index f2c5f20e0..330a76176 100644 --- a/ErsatzTV/Controllers/Api/Requests/ReplaceTemplateRequest.cs +++ b/ErsatzTV/Controllers/Api/Requests/ReplaceTemplateRequest.cs @@ -4,10 +4,11 @@ namespace ErsatzTV.Controllers.Api.Requests; public record ReplaceTemplateRequest(string Name, List Items) { - public ReplaceTemplateItems ToCommand(int templateGroupId, int templateId) => + public ReplaceTemplateItems ToCommand(int templateGroupId, int templateId, Option expectedVersion = default) => new( templateGroupId, templateId, Name, - (Items ?? []).Select(item => item.ToReplaceItem()).ToList()); + (Items ?? []).Select(item => item.ToReplaceItem()).ToList(), + expectedVersion); } diff --git a/ErsatzTV/Controllers/Api/ScheduleController.cs b/ErsatzTV/Controllers/Api/ScheduleController.cs index 0a46e4e16..c325f7824 100644 --- a/ErsatzTV/Controllers/Api/ScheduleController.cs +++ b/ErsatzTV/Controllers/Api/ScheduleController.cs @@ -104,7 +104,9 @@ public class ScheduleController(IMediator mediator) : ControllerBase [EndpointDescription( "Returns the schedule's items plus a computed, best-effort runtime estimate: each item carries a " + "nullable durationEstimate and the envelope carries a nullable totalDurationEstimate. Estimates are " + - "derived from referenced collection/media runtimes and are null when unbounded or unknown.")] + "derived from referenced collection/media runtimes and are null when unbounded or unknown. The " + + "response also carries a strong ETag of the schedule's version; pass that ETag back as If-Match on " + + "the replace (PUT) to detect a concurrent edit (issue #253).")] [EndpointGroupName("general")] [ProducesResponseType(typeof(ScheduleItemsResponseModel), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] @@ -116,6 +118,9 @@ public class ScheduleController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // The items GET returns children, not the root, so read the schedule's version for the ETag. + ConcurrencyHeaders.SetETag(Response, schedule.Map(s => s.Version).IfNone(0)); + ProgramScheduleItemsWithDurationViewModel items = await mediator.Send(new GetProgramScheduleItemsWithDurations(id), cancellationToken); return new OkObjectResult(ScheduleItemResponseMapper.ProjectToResponseModel(items)); @@ -143,20 +148,57 @@ public class ScheduleController(IMediator mediator) : ControllerBase [HttpPut("/api/schedules/{id:int}/items")] [Tags("Schedules")] [EndpointSummary("Replace schedule items")] + [EndpointDescription( + "Replaces the schedule's full item list; item indexes are assigned from the array order. Send the " + + "ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); a successful " + + "response carries the new ETag.")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ReplaceItems( int id, [Required] [FromBody] ReplaceScheduleItemsRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return new BadRequestObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status400BadRequest, + Title = "Invalid If-Match header", + Detail = "If-Match must be a strong ETag of the resource version (e.g. \"3\") or \"*\"." + }); + } + Either> result = - await mediator.Send(request.ToCommand(id), cancellationToken); - return result - .Map(items => items.Select(ScheduleItemResponseMapper.ProjectToResponseModel).ToList()) - .ToUpdatedResult(); + await mediator.Send(request.ToCommand(id, ifMatch.ExpectedVersion), cancellationToken); + + return await result.Match( + Left: error => Task.FromResult(error.ToErrorResult()), + 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 refreshed = + await mediator.Send(new GetProgramScheduleById(id), cancellationToken); + List 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()); + }); } [HttpDelete("/api/schedules/{id:int}/items/{itemId:int}")] diff --git a/ErsatzTV/Controllers/Api/TemplateController.cs b/ErsatzTV/Controllers/Api/TemplateController.cs index e02f87209..5bf3a11f8 100644 --- a/ErsatzTV/Controllers/Api/TemplateController.cs +++ b/ErsatzTV/Controllers/Api/TemplateController.cs @@ -134,6 +134,9 @@ public class TemplateController(IMediator mediator) : ControllerBase [HttpGet("/api/templates/{id:int}/items")] [Tags("Templates")] [EndpointSummary("Get template items")] + [EndpointDescription( + "Returns the template's items and a strong ETag of the template's version. Pass that ETag back as " + + "If-Match on the replace (PUT) to detect a concurrent edit (issue #253).")] [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] @@ -145,6 +148,9 @@ public class TemplateController(IMediator mediator) : ControllerBase return ApiResults.NotFoundProblem(); } + // The items GET returns children, not the root, so read the template's version for the ETag. + ConcurrencyHeaders.SetETag(Response, template.Map(t => t.Version).IfNone(0)); + List items = await mediator.Send(new GetTemplateItems(id), cancellationToken); return new OkObjectResult( items.OrderBy(i => i.StartTime.TimeOfDay).Map(ProjectToResponseModel).ToList()); @@ -155,16 +161,32 @@ public class TemplateController(IMediator mediator) : ControllerBase [EndpointSummary("Replace a template and its items")] [EndpointDescription( "Replaces the template's name and its full item list. Each item assigns a block to a start time of day; " + - "items must not overlap (an item's end time is its start time plus the assigned block's duration).")] + "items must not overlap (an item's end time is its start time plus the assigned block's duration). " + + "Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); " + + "a successful response carries the new ETag.")] [EndpointGroupName("general")] [ProducesResponseType(typeof(TemplateWithItemsResponseModel), StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status412PreconditionFailed)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Replace( int id, [Required] [FromBody] ReplaceTemplateRequest request, CancellationToken cancellationToken) { + IfMatchCondition ifMatch = ConcurrencyHeaders.ParseIfMatch(Request); + if (ifMatch.Kind is IfMatchKind.Malformed) + { + return new BadRequestObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status400BadRequest, + Title = "Invalid If-Match header", + Detail = "If-Match must be a strong ETag of the resource version (e.g. \"3\") or \"*\"." + }); + } + Option maybeTemplate = await mediator.Send(new GetTemplateById(id), cancellationToken); if (maybeTemplate.IsNone) { @@ -174,16 +196,25 @@ public class TemplateController(IMediator mediator) : ControllerBase int templateGroupId = maybeTemplate.Map(t => t.TemplateGroupId).IfNone(0); Either> result = - await mediator.Send(request.ToCommand(templateGroupId, id), cancellationToken); + await mediator.Send(request.ToCommand(templateGroupId, id, ifMatch.ExpectedVersion), cancellationToken); return await result.Match( Left: error => Task.FromResult(error.ToErrorResult()), 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 refreshed = await mediator.Send(new GetTemplateById(id), cancellationToken); List items = await mediator.Send(new GetTemplateItems(id), cancellationToken); return refreshed.Match( - Some: vm => (IActionResult)new OkObjectResult(ProjectToWithItemsResponseModel(vm, items)), + 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(ProjectToWithItemsResponseModel(vm, items)); + }, None: () => ApiResults.NotFoundProblem()); }); } diff --git a/ErsatzTV/Controllers/Api/TroubleshootController.cs b/ErsatzTV/Controllers/Api/TroubleshootController.cs index 129c077f5..7a523f3f7 100644 --- a/ErsatzTV/Controllers/Api/TroubleshootController.cs +++ b/ErsatzTV/Controllers/Api/TroubleshootController.cs @@ -114,7 +114,9 @@ public class TroubleshootController( [Tags("Troubleshooting")] [EndpointSummary("Start a troubleshooting playback session")] [EndpointGroupName("general")] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task TroubleshootPlayback( [FromQuery] int mediaItem, @@ -174,9 +176,15 @@ public class TroubleshootController( Optional(start)), cancellationToken); - if (result.IsLeft) + // Distinguish "prepare failed" from the later "no playable output" fall-through: map the + // handler's BaseError through the standard helper (404 for NotFoundError — e.g. an unknown + // media item/channel — else 422 for a validation failure) with a ProblemDetails body, + // instead of a bare body-less 404. The SPA feeds this URL straight to hls.js (HlsPlayer) + // and never inspects the status code — failures surface via the /status poll — so the + // 404→422 split for validation errors is safe. + foreach (BaseError error in result.LeftToSeq()) { - return NotFound(); + return error.ToErrorResult(); } // Prepare returned a process, so the handler holds the troubleshooting lock now @@ -273,7 +281,12 @@ public class TroubleshootController( } } - return NotFound(); + // Terminal fall-through: Prepare succeeded but no playable output was produced (playback + // failed to start, was cancelled, or the segmenter never wrote segments). Keep the 404 status + // the SPA player already tolerates, but attach a distinguishing ProblemDetails body rather + // than a bare NotFound() so the response is self-describing. + return ApiResults.NotFoundProblem( + "Troubleshooting playback did not produce any output. It may have failed to start or been cancelled."); } [HttpHead("api/troubleshoot/playback/archive")] diff --git a/ErsatzTV/Pages/TelevisionSeasonList.razor b/ErsatzTV/Pages/TelevisionSeasonList.razor index 40016caf6..af9e0f4c7 100644 --- a/ErsatzTV/Pages/TelevisionSeasonList.razor +++ b/ErsatzTV/Pages/TelevisionSeasonList.razor @@ -307,8 +307,9 @@ private async Task ScanShow(bool deepScan) { - bool result = await Mediator.Send(new QueueShowScanByLibraryId(_show.LibraryId, _show.Id, _show.Title, deepScan)); - if (!result) + QueueShowScanResult result = + await Mediator.Send(new QueueShowScanByLibraryId(_show.LibraryId, _show.Id, _show.Title, deepScan)); + if (result != QueueShowScanResult.Queued) { Snackbar.Add($"Unable to scan show {_show.Title}", Severity.Error); } diff --git a/ErsatzTV/Services/ScannerService.cs b/ErsatzTV/Services/ScannerService.cs index cafd5380b..89271681a 100644 --- a/ErsatzTV/Services/ScannerService.cs +++ b/ErsatzTV/Services/ScannerService.cs @@ -221,7 +221,11 @@ public class ScannerService : BackgroundService } finally { - if (entityLocker.ArePlexCollectionsLocked()) + // request.Unlock is false when this collections scan is part of a scheduler-enqueued + // per-provider batch and a later message owns the single release (see #250 / #235). + // request.Unlock is false when this collections scan is part of a scheduler-enqueued + // per-provider batch and a later message owns the single release (see #250 / #235). + if (request.Unlock && entityLocker.ArePlexCollectionsLocked()) { entityLocker.UnlockPlexCollections(); } @@ -351,7 +355,9 @@ public class ScannerService : BackgroundService } finally { - if (entityLocker.AreJellyfinCollectionsLocked()) + // request.Unlock is false when this collections scan is part of a scheduler-enqueued + // per-provider batch and a later message owns the single release (see #250 / #235). + if (request.Unlock && entityLocker.AreJellyfinCollectionsLocked()) { entityLocker.UnlockJellyfinCollections(); } @@ -438,7 +444,9 @@ public class ScannerService : BackgroundService } finally { - if (entityLocker.AreEmbyCollectionsLocked()) + // request.Unlock is false when this collections scan is part of a scheduler-enqueued + // per-provider batch and a later message owns the single release (see #250 / #235). + if (request.Unlock && entityLocker.AreEmbyCollectionsLocked()) { entityLocker.UnlockEmbyCollections(); } diff --git a/ErsatzTV/Services/SchedulerService.cs b/ErsatzTV/Services/SchedulerService.cs index 0845bf6fa..de81a92a4 100644 --- a/ErsatzTV/Services/SchedulerService.cs +++ b/ErsatzTV/Services/SchedulerService.cs @@ -265,11 +265,35 @@ public class SchedulerService : BackgroundService } } - foreach (int mediaSourceId in mediaSourceIds) + // lock the collections slot once for the whole provider batch: the LAST message owns the + // single release (Unlock: true), the rest run Unlock: false. If the slot is already held + // (an API scan or a prior tick's batch is running/queued), skip enqueuing entirely - never + // enqueue unlocked, which would cross-release the holder's lock (#250). Skipping starves + // nothing: the next tick retries. (see #235) + // lock the collections slot once for the whole provider batch: the LAST message owns the + // single release (Unlock: true), the rest run Unlock: false. If the slot is already held + // (an API scan or a prior tick's batch is running/queued), skip enqueuing entirely - never + // enqueue unlocked, which would cross-release the holder's lock (#250). Skipping starves + // nothing: the next tick retries. (see #235) + var plexSourceIds = mediaSourceIds.ToList(); + if (plexSourceIds.Count > 0 && _entityLocker.LockPlexCollections()) { - await _scannerWorkerChannel.WriteAsync( - new SynchronizePlexCollections(mediaSourceId, false, false), - cancellationToken); + try + { + for (var i = 0; i < plexSourceIds.Count; i++) + { + bool isLast = i == plexSourceIds.Count - 1; + await _scannerWorkerChannel.WriteAsync( + new SynchronizePlexCollections(plexSourceIds[i], false, false, Unlock: isLast), + cancellationToken); + } + } + catch + { + // an enqueue threw before the last (releasing) message went out - compensate here + _entityLocker.UnlockPlexCollections(); + throw; + } } } @@ -300,11 +324,25 @@ public class SchedulerService : BackgroundService } } - foreach (int mediaSourceId in mediaSourceIds) + // lock-once per provider batch; last message owns the release; skip entirely if held (#235/#250) + var jellyfinSourceIds = mediaSourceIds.ToList(); + if (jellyfinSourceIds.Count > 0 && _entityLocker.LockJellyfinCollections()) { - await _scannerWorkerChannel.WriteAsync( - new SynchronizeJellyfinCollections(mediaSourceId, false, false), - cancellationToken); + try + { + for (var i = 0; i < jellyfinSourceIds.Count; i++) + { + bool isLast = i == jellyfinSourceIds.Count - 1; + await _scannerWorkerChannel.WriteAsync( + new SynchronizeJellyfinCollections(jellyfinSourceIds[i], false, false, Unlock: isLast), + cancellationToken); + } + } + catch + { + _entityLocker.UnlockJellyfinCollections(); + throw; + } } } @@ -335,11 +373,25 @@ public class SchedulerService : BackgroundService } } - foreach (int mediaSourceId in mediaSourceIds) + // lock-once per provider batch; last message owns the release; skip entirely if held (#235/#250) + var embySourceIds = mediaSourceIds.ToList(); + if (embySourceIds.Count > 0 && _entityLocker.LockEmbyCollections()) { - await _scannerWorkerChannel.WriteAsync( - new SynchronizeEmbyCollections(mediaSourceId, false, false), - cancellationToken); + try + { + for (var i = 0; i < embySourceIds.Count; i++) + { + bool isLast = i == embySourceIds.Count - 1; + await _scannerWorkerChannel.WriteAsync( + new SynchronizeEmbyCollections(embySourceIds[i], false, false, Unlock: isLast), + cancellationToken); + } + } + catch + { + _entityLocker.UnlockEmbyCollections(); + throw; + } } } diff --git a/ErsatzTV/Services/WorkerService.cs b/ErsatzTV/Services/WorkerService.cs index 5e3ebfd00..1cefb8eb2 100644 --- a/ErsatzTV/Services/WorkerService.cs +++ b/ErsatzTV/Services/WorkerService.cs @@ -9,6 +9,7 @@ using ErsatzTV.Application.MediaCollections; using ErsatzTV.Application.Playouts; using ErsatzTV.Application.Subtitles; using ErsatzTV.Core; +using ErsatzTV.Core.Interfaces.Locking; using MediatR; namespace ErsatzTV.Services; @@ -16,16 +17,19 @@ namespace ErsatzTV.Services; public class WorkerService : BackgroundService { private readonly ChannelReader _channel; + private readonly IEntityLocker _entityLocker; private readonly ILogger _logger; private readonly IServiceScopeFactory _serviceScopeFactory; public WorkerService( ChannelReader channel, IServiceScopeFactory serviceScopeFactory, + IEntityLocker entityLocker, ILogger logger) { _channel = channel; _serviceScopeFactory = serviceScopeFactory; + _entityLocker = entityLocker; _logger = logger; } @@ -143,5 +147,19 @@ public class WorkerService : BackgroundService { _logger.LogInformation("Worker service shutting down"); } + finally + { + // The global Trakt lock is acquired by SchedulerService/TraktController and released only + // when the *terminal* (Unlock: true) message of a batch is processed here. If this loop + // stops before reaching that message - shutdown break above, channel completion, or the + // reader throwing on cancellation - the release never fires and the in-memory Trakt lock + // leaks for the remaining life of the process (subsequent Trakt operations 409 forever). + // Make the batch-release loss-tolerant with a compensating release on worker shutdown. + if (_entityLocker.IsTraktLocked()) + { + _logger.LogDebug("Releasing held Trakt lock during worker shutdown"); + _entityLocker.UnlockTrakt(); + } + } } } diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 5e75fce0d..89b9f5d21 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -1863,8 +1863,8 @@ } ], "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" }, "404": { "description": "Not Found", @@ -4079,7 +4079,7 @@ "DecoTemplates" ], "summary": "Replace a deco template and its items", - "description": "Replaces the deco template's name and its full item list. Each item assigns a deco to a start/end time of day; an end time of 00:00:00 means the item runs to the end of the day. Items must not overlap.", + "description": "Replaces the deco template's name and its full item list. Each item assigns a deco to a start/end time of day; an end time of 00:00:00 means the item runs to the end of the day. Items must not overlap. Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); a successful response carries the new ETag.", "parameters": [ { "name": "id", @@ -4137,6 +4137,26 @@ } } }, + "400": { + "description": "Bad Request", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "404": { "description": "Not Found", "content": { @@ -4157,6 +4177,26 @@ } } }, + "412": { + "description": "Precondition Failed", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -4186,6 +4226,7 @@ "DecoTemplates" ], "summary": "Get deco template items", + "description": "Returns the deco template's items and a strong ETag of the deco template's version. Pass that ETag back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).", "parameters": [ { "name": "id", @@ -4879,6 +4920,80 @@ } } }, + "/api/media-sources/emby/{id}/scan-collections": { + "post": { + "tags": [ + "Emby" + ], + "summary": "Scan an Emby source's collections", + "description": "Queues a synchronization of the source's collections (fire-and-forget). Pass ?deep=true for a deep scan. Returns 409 while an Emby collections scan is already in progress.", + "operationId": "ScanEmbyCollections", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } + } + ], + "responses": { + "202": { + "description": "Accepted" + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/ffmpeg/profiles": { "get": { "tags": [ @@ -6657,6 +6772,80 @@ } } }, + "/api/media-sources/jellyfin/{id}/scan-collections": { + "post": { + "tags": [ + "Jellyfin" + ], + "summary": "Scan a Jellyfin source's collections", + "description": "Queues a synchronization of the source's collections (fire-and-forget). Pass ?deep=true for a deep scan. Returns 409 while a Jellyfin collections scan is already in progress.", + "operationId": "ScanJellyfinCollections", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } + } + ], + "responses": { + "202": { + "description": "Accepted" + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/languages": { "get": { "tags": [ @@ -6743,6 +6932,7 @@ "Libraries" ], "summary": "Scan library", + "description": "Queues a scan of the whole library. Pass ?deep=true for a deep (metadata-refresh) scan.", "parameters": [ { "name": "id", @@ -6752,6 +6942,14 @@ "type": "integer", "format": "int32" } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } } ], "responses": { @@ -6864,8 +7062,8 @@ "required": true }, "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" }, "404": { "description": "Not Found", @@ -6887,8 +7085,28 @@ } } }, - "400": { - "description": "Bad Request", + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "422": { + "description": "Unprocessable Entity", "content": { "text/plain": { "schema": { @@ -7629,6 +7847,26 @@ "responses": { "200": { "description": "OK" + }, + "422": { + "description": "Unprocessable Entity", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -7640,8 +7878,8 @@ ], "summary": "Clean artwork cache", "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" } } } @@ -8784,6 +9022,7 @@ "Playlists" ], "summary": "Update a playlist (rename and replace its items)", + "description": "Replaces the playlist's name and its full item list. Item indexes are assigned from the array order. Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); a successful response carries the new ETag.", "operationId": "UpdatePlaylist", "parameters": [ { @@ -8851,6 +9090,26 @@ } } }, + "400": { + "description": "Bad Request", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "404": { "description": "Not Found", "content": { @@ -8871,6 +9130,26 @@ } } }, + "412": { + "description": "Precondition Failed", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -8963,6 +9242,7 @@ "Playlists" ], "summary": "Get the items in a playlist", + "description": "Returns the playlist's items and a strong ETag of the playlist's version. Pass that ETag back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).", "operationId": "GetPlaylistItems", "parameters": [ { @@ -10659,7 +10939,24 @@ "operationId": "ResetAllPlayouts", "responses": { "202": { - "description": "Accepted" + "description": "Accepted", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ResetAllPlayoutsResponseModel" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ResetAllPlayoutsResponseModel" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ResetAllPlayoutsResponseModel" + } + } + } } } } @@ -11476,6 +11773,80 @@ } } }, + "/api/media-sources/plex/{id}/scan-collections": { + "post": { + "tags": [ + "Plex" + ], + "summary": "Scan a Plex server's collections", + "description": "Queues a synchronization of the server's collections (fire-and-forget). Pass ?deep=true for a deep scan. Returns 409 while a Plex collections scan is already in progress.", + "operationId": "ScanPlexCollections", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } + } + ], + "responses": { + "202": { + "description": "Accepted" + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } + } + }, "/api/rerun-collections": { "get": { "tags": [ @@ -12521,7 +12892,7 @@ "Schedules" ], "summary": "Get schedule items", - "description": "Returns the schedule's items plus a computed, best-effort runtime estimate: each item carries a nullable durationEstimate and the envelope carries a nullable totalDurationEstimate. Estimates are derived from referenced collection/media runtimes and are null when unbounded or unknown.", + "description": "Returns the schedule's items plus a computed, best-effort runtime estimate: each item carries a nullable durationEstimate and the envelope carries a nullable totalDurationEstimate. Estimates are derived from referenced collection/media runtimes and are null when unbounded or unknown. The response also carries a strong ETag of the schedule's version; pass that ETag back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).", "parameters": [ { "name": "id", @@ -12685,6 +13056,7 @@ "Schedules" ], "summary": "Replace schedule items", + "description": "Replaces the schedule's full item list; item indexes are assigned from the array order. Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); a successful response carries the new ETag.", "parameters": [ { "name": "id", @@ -12751,6 +13123,26 @@ } } }, + "400": { + "description": "Bad Request", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "404": { "description": "Not Found", "content": { @@ -12771,6 +13163,26 @@ } } }, + "412": { + "description": "Precondition Failed", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -15156,7 +15568,7 @@ "Templates" ], "summary": "Replace a template and its items", - "description": "Replaces the template's name and its full item list. Each item assigns a block to a start time of day; items must not overlap (an item's end time is its start time plus the assigned block's duration).", + "description": "Replaces the template's name and its full item list. Each item assigns a block to a start time of day; items must not overlap (an item's end time is its start time plus the assigned block's duration). Send the ETag from the items GET as If-Match to reject a stale overwrite with 412 (issue #253); a successful response carries the new ETag.", "parameters": [ { "name": "id", @@ -15214,6 +15626,26 @@ } } }, + "400": { + "description": "Bad Request", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "404": { "description": "Not Found", "content": { @@ -15234,6 +15666,26 @@ } } }, + "412": { + "description": "Precondition Failed", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -15263,6 +15715,7 @@ "Templates" ], "summary": "Get template items", + "description": "Returns the template's items and a strong ETag of the template's version. Pass that ETag back as If-Match on the replace (PUT) to detect a concurrent edit (issue #253).", "parameters": [ { "name": "id", @@ -16087,6 +16540,26 @@ } ], "responses": { + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "409": { "description": "Conflict", "content": { @@ -16106,6 +16579,26 @@ } } } + }, + "422": { + "description": "Unprocessable Entity", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } }, @@ -16201,6 +16694,26 @@ } ], "responses": { + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "409": { "description": "Conflict", "content": { @@ -16220,6 +16733,26 @@ } } } + }, + "422": { + "description": "Unprocessable Entity", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -22779,7 +23312,8 @@ "dailyRebuildTime", "buildStatus", "decoId", - "decoName" + "decoName", + "isLocked" ], "type": "object", "properties": { @@ -22837,6 +23371,9 @@ "null", "string" ] + }, + "isLocked": { + "type": "boolean" } } }, @@ -23142,7 +23679,8 @@ "treatCollectionsAsShows", "shuffleScheduleItems", "randomStartPoint", - "fixedStartTimeBehavior" + "fixedStartTimeBehavior", + "version" ], "type": "object", "properties": { @@ -23170,6 +23708,10 @@ }, "fixedStartTimeBehavior": { "$ref": "#/components/schemas/FixedStartTimeBehavior" + }, + "version": { + "type": "integer", + "format": "int32" } } }, @@ -23668,6 +24210,37 @@ } } }, + "ResetAllPlayoutsResponseModel": { + "required": [ + "queuedPlayoutIds", + "skippedLocked", + "skippedUnsupported" + ], + "type": "object", + "properties": { + "queuedPlayoutIds": { + "type": "array", + "items": { + "type": "integer", + "format": "int32" + } + }, + "skippedLocked": { + "type": "array", + "items": { + "type": "integer", + "format": "int32" + } + }, + "skippedUnsupported": { + "type": "array", + "items": { + "type": "integer", + "format": "int32" + } + } + } + }, "ResolutionResponseModel": { "required": [ "id", diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 81d93284e..3ed6b2514 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -112,12 +112,21 @@ Established by issue #215 (`PlayoutController` + `ChannelController.ResetPlayout `[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)]` to each guarded action. Precedent for the 409 shape: `TraktController` (its private `ConflictProblem()`). Two nuances: - **Fire-and-forget bulk operations don't 409** — `POST /api/playouts/reset-all` stays 202; its - handler (`ResetAllPlayoutsHandler`) already *skips* locked playouts, matching Blazor + the handler - semantics. Only per-id mutations 409. + handler (`ResetAllPlayoutsHandler`) *skips* locked playouts, matching Blazor + the handler + semantics. Only per-id mutations 409. As of #235 the handler returns a `ResetAllPlayoutsResult` + (`QueuedPlayoutIds` / `SkippedLocked` / `SkippedUnsupported`) and the controller returns the 202 + **with a `ResetAllPlayoutsResponseModel` body** reporting what was queued vs. skipped (locked, or + an unsupported `ExternalJson`/`None` kind) — a fire-and-forget bulk op still reports its outcome + rather than silently swallowing skips. - **Surface the lock state to clients** so they can pre-disable the buttons: stamp an `IsLocked` boolean onto the list DTO (`PlayoutListItemResponseModel`, set from `IsPlayoutLocked` in the - controller's list projection) rather than adding a push channel. The SPA reads it and, on a 409, - refreshes the list to pick up the flag. + controller's list projection) rather than adding a push channel — and (as of #235) onto the + single-playout GET DTO (`PlayoutResponseModel.IsLocked`, set the same way in every action that + maps it) so a client polling one playout has the same flag. The SPA reads it and, on a 409, + refreshes to pick up the flag. +- **Async-op success is 202, not 200** — an endpoint whose success path only *queues* a background + rebuild returns **202 Accepted**, not 200 (#235: `POST /api/channels/{channelNumber}/playout/reset` + queues a `BuildPlayout` → `AcceptedResult`). Reserve 200 for a synchronous durable result. ### 3b. Map a "queue a background job" outcome to status codes with an enum, not a `bool` @@ -135,6 +144,15 @@ operation itself, not a mutation racing it). Add `[ProducesResponseType]` for 20 `EnqueueWithTraktLock` compensating-unlock pattern (`TraktController`): if a `WriteAsync` throws after a successful `Lock*`, `Unlock*` in a `catch` and rethrow — one lock ⇄ exactly one release. +A second exemplar (issue #235 slice B), where the lock lives on the **controller** rather than in a +handler: `POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=` acquires the +per-source collections lock (`entityLocker.LockPlexCollections()` etc.) — the lock IS the running +collections scan, so a `false` = 409 — then `WriteAsync`es `Synchronize{X}Collections(id, ForceScan: +true, deep)` to the scanner channel and returns **202**. `ScannerService` releases that lock in a +`finally` when it processes the message; the controller compensating-unlocks in a `catch` if the +enqueue throws. `POST /api/libraries/{id}/scan?deep=` similarly threads an optional `[FromQuery] bool +deep` into `QueueLibraryScanByLibraryId(id, DeepScan)`. + `NotFoundError : BaseError` lives in `ErsatzTV.Core/Errors/NotFoundError.cs` — return it from a handler's validation when a lookup fails, so the controller-side mapping falls out for free. @@ -293,8 +311,21 @@ GET handlers that feed an ordered list must also `.OrderBy(i => i.Index)` — id The replace-all aggregate PUTs (blocks, templates, schedules items, playlists, collections, playouts, etc.) carry an **optimistic-concurrency contract** so a stale second tab can't silently overwrite a -fresher edit (issue #253). The Block endpoints are the reference implementation; PRs 2–4 fan the same -recipe across the other aggregates. +fresher edit (issue #253). The Block endpoints are the reference implementation; **PR2 fanned the same +recipe onto Template, DecoTemplate, Playlist, and schedule-items** (`PUT /api/templates/{id}`, +`/api/deco-templates/{id}`, `/api/playlists/{id}`, `/api/schedules/{id}/items`); PR3 covers the +Diff/Scalar aggregates (Collection, Playout ×2, MultiCollection, RerunCollection) and PR4 is the Phase-2 +428 flip. + +Each replace PUT keeps its **own** existing 200 body shape (Template/DecoTemplate return a +`…WithItemsResponseModel`, Block likewise; Playlist and schedule-items return the item array) and adds the +ETag as a **header only** — except schedules: `ScheduleController` returns the Application-layer +`ProgramScheduleViewModel` *directly* (no `ProgramScheduleResponseModel` exists), so the added +`int Version` also surfaces as a redundant `version` field in the `GET /api/schedules[/{id}]` bodies. That +is intentional and harmless (the ETag remains the authority); introducing a ResponseModel purely to hide +one field was judged disproportionate. Every replace PUT's *sibling config writers* bump `Version` too +(Playlist: the five `Add*ToPlaylist` handlers; schedule: `AddProgramScheduleItem` / `DeleteProgramScheduleItem` +/ `UpdateProgramSchedule`; Template/DecoTemplate have none). **Token.** Each versioned root implements `IVersionedAggregate` (`int Version`, EF-mapped with `.IsConcurrencyToken()` in its `IEntityTypeConfiguration`). A single dual-provider migration diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index 6ae11a304..046e2ea52 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -270,11 +270,23 @@ intentional exit ramp until phase (b) removes Blazor entirely. ## Section 5 — Removal execution runbook (#91 phase b, Step 2/3) The removal PR is **gated** — it starts only after these clear: ~~#202 (media-source write API + SPA)~~ -**DONE 2026-07-11**, #235 F9 (deep-scan + external-collections-scan API — no API today, so `Libraries.razor` -can't be deleted yet), and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). (#204's id-carrying pattern -redirects landed 2026-07-11; its catch-all fallback that replaces `MapFallbackToPage("/_Host")` is folded -into Step 2 below, since it can only ship when `_Host` is deleted.) When those close, the removal-PR routine -executes, in order: +**DONE 2026-07-11**, ~~#235 F9 (deep-scan + external-collections-scan API)~~ **API DONE (#235 slice B)**: +`POST /api/libraries/{id}/scan?deep=` now threads deep-scan, and `POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=` +covers external-collections scan (the two `Libraries.razor` parity gaps) — the SPA `Libraries.razor` port can now +proceed, and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). ~~**Remaining SPA affordance for +the removal PR**: `LibrariesScreen` exposes only quick-scan; add the **deep-scan** and **external-collections-scan** +buttons before deleting `Libraries.razor`.~~ **SPA affordance DONE 2026-07-11** (landed ahead of the removal PR +so the deletion diff stays pure): `LibrariesScreen` now wires the shipped `scanLibrary(id, deep)` + +`scanCollections(family, id, deep)` clients — a **Deep Scan Library** button on each remote library row and an +**External Collections** section (quick + deep per remote source), matching all four `Libraries.razor` scan actions. +The External Collections rows derive client-side from `getMediaSources()` (no new endpoint): the media-sources API +handler already filters each source's `libraries` to sync-enabled entries, so a remote source with a non-empty +`libraries` list is exactly `GetExternalCollections`'s `Libraries.Any(ShouldSyncItems)` filter. Collections scans +have no scan-status poll surface, so their button pending state is optimistic + timeout-bounded (follow-up #271: a +collections scan-status endpoint would let it reconcile like library scans). (#204's id-carrying +pattern redirects landed 2026-07-11; its catch-all fallback that replaces `MapFallbackToPage("/_Host")` is folded +into Step 2 below, since it can only ship when `_Host` is deleted.) With the SPA parity done, the removal PR's +remaining pre-work is just the **mandatory cold adversarial pass**; the runbook then executes, in order: 1. **Cut the rollback tag `blazor-final`** on the pre-deletion `main` commit — the *first* action, before deleting anything. Exact command + restore path: `docs/decisions.md` 2026-07-11 "Pre-removal Blazor diff --git a/docs/decisions.md b/docs/decisions.md index 2c5c25191..3c9d9f60c 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -650,6 +650,60 @@ action of the Step 2 deletion PR merge** (not before — `main` moves until then Not cut this session — `main` still carries Blazor and will advance before the removal PR. +## 2026-07-11 — Async-op API contract normalization + playout build observability + F9 scan endpoints (#235) + +Reviewer#20 F7/F8/F9. Normalizes the queue-triggering `/api/*` endpoints onto one contract, closes the two +F9 `Libraries.razor` parity gaps, and hardens the Trakt batch-lock lifecycle. Much of the F8 surface was +**already normalized** by #232 (library scan → `QueueLibraryScanResult` 202/404/409/422) and #215 (per-id +playout mutations + reset → 409 lock guard) — this issue finished the remaining outliers. + +**Normalized async-op contract** (queue-triggering endpoints): **202 Accepted** = work queued; **404 +ProblemDetails** = entity missing (controller pre-check); **409 ProblemDetails** = lock held (the running +job, or a mutation racing it — §3a/§3b); **422 ProblemDetails** = domain precondition (sync disabled / +unsupported / start failed). Trakt was the reference implementation. Changes made: +- `MaintenanceController.EmptyTrash` — error path **500 text/plain → 404/422 ProblemDetails** (`ToErrorResult`). +- `MaintenanceController.CleanArtwork` — silent **200 → 202** (fire-and-forget enqueue). No SPA consumer. +- `LibrariesController.ScanShow` — conflated **400 `{error}` → 202/404/409/422** via a new + `QueueShowScanResult` enum (6 outcomes incl. an honest `ScanFailed`→422, distinct from `Unsupported`). +- `ChannelController.ResetPlayout` — **200 → 202** (queue-triggering; 404/409 unchanged). +- `PlayoutController.ResetAll` — **202 (no body) → 202 + `ResetAllPlayoutsResponseModel`** reporting + `queuedPlayoutIds` / `skippedLocked` / `skippedUnsupported` (replaces the silent skip; still 202, still + skips locked/ExternalJson by design per §3a — now it *reports* what it skipped). +- `TroubleshootController.TroubleshootPlayback` — bare body-less `NotFound()` → **404/422 ProblemDetails** + with distinguishing detail. **Status codes the SPA HLS player depends on were preserved** — verified + `HlsPlayer.tsx` never branches on this endpoint's status (playback state comes from the separate + `/api/troubleshoot/playback/status` poll); only the error *body* was enriched. + +**Playout build observability**: the list endpoint (`GET /api/playouts`) already stamped `isLocked` + +`BuildStatus` on `PlayoutListItemResponseModel` (#215); this issue adds **`isLocked` to the single-playout +`GET /api/playouts/{id}`** (`PlayoutResponseModel`), so the detail poll surface carries the §3a lock flag +too. No dedicated `GET /api/playouts/{id}/status` push channel was added — the flag on the existing GETs is +the HTTP-observable substitute for Blazor's live lock event, matching the `GET /api/trakt/status` precedent. + +**F9 parity endpoints** (the `Libraries.razor` deletion gate — #202 did NOT close these): +- **Deep scan**: `POST /api/libraries/{id}/scan` gains `?deep=false`, threaded through + `QueueLibraryScanByLibraryId(LibraryId, DeepScan=false)` into `ForceSynchronize{Plex,Jellyfin,Emby}LibraryById(id, deep)` + (was hardcoded `false`). Non-breaking: existing callers omit it. +- **External-collections scan**: new `POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=false` + on the three #202 media-source controllers, dispatching `Synchronize{X}Collections(id, ForceScan:true, deep)`. + Each pre-checks source existence (404), acquires the per-source **collections** lock (`Lock{X}Collections()` — + the lock *is* the running scan, so a false = **409**), then enqueues and returns 202; the controller + compensating-unlocks in a `catch` if the enqueue throws (§3b), and `ScannerService` releases in its `finally`. + Thin SPA clients shipped (`scanLibrary(id, deep)`, `scanCollections`); **the SPA deep-scan / collections + buttons are the removal PR's remaining parity work** (parity doc §5). + +**F7 Trakt batch-lock leak fix**: the global Trakt lock was released only when the *terminal* batch message +(`Unlock: true`) was processed; a `WorkerService` shutdown/cancellation before that message leaked the lock +permanently (subsequent Trakt ops 409 until restart — same class as #231/#233/#234). Fix: `WorkerService` +now releases the Trakt lock in a `finally` on read-loop exit if still held. Non-vacuous regression test proven +against an inverted-condition control. + +**Accepted-by-design** (per the issue's decision-record ask): the worker's channels are **unbounded** and +there is **no shutdown drain** — messages still queued at process exit are dropped. This is acceptable because +the entity locks are **in-memory singletons that die with the process**, so a dropped message can't strand a +lock across restarts (the F7 `finally` covers the *within-process* shutdown-break leak, which is the only way +a lock outlives its batch while the process keeps running). Adding a bounded-channel backpressure / graceful +drain is out of scope and would not fix a correctness bug. ## 2026-07-11 — Optimistic-concurrency contract for replace-all PUTs (#253 PR1: infra + Block reference) Replace-all aggregate PUTs had **no** optimistic concurrency — a stale second tab silently overwrote a @@ -758,3 +812,36 @@ done with the #197 contract work. Tracked as a follow-up issue. (`MediaCollectionViewModel`, `MultiCollectionViewModel`, `RerunCollectionViewModel`) carry `int Version = 0` (defaulted — 0 for the selection-placeholder constructions, real value from the Mapper projection). Header-only via ETag, never echoed in a response body (the Block precedent). + +**Post-merge addendum (PR3 review, #269).** Activating the `Version` token means EF guards *every* root +UPDATE, so non-participating root-scalar writers that use plain `SaveChangesAsync` (playout settings / +schedule-file / on-demand-checkpoint, collection name) would 500 on a concurrent bump. The realistic +UPDATE writers were fixed in-PR with `ConcurrencyExtensions.SaveChangesForcingVersion` (Phase-1 +force-write on conflict: adopt the stored token, retry, never revert the concurrent bump). The deferral +above is re-scoped to the DELETE handlers + repository `Add*` writers only (→ #269). + +## 2026-07-11 — External Collections in the SPA derives from `getMediaSources()`; collections scans are optimistically bounded (#91b Libraries.razor parity) + +**Context.** The last SPA pre-work before deleting Blazor `Libraries.razor` was wiring the shipped +`scanLibrary(id, deep)` + `scanCollections(family, id, deep)` clients (F9 API, #235) into `LibrariesScreen`, +matching the page's four scan actions: quick-library (already had it), deep-library, quick-collections, +deep-collections. Landed as its own pure-SPA PR ahead of the removal PR so the deletion diff stays surgical. + +**Decision 1 — derive the External Collections rows client-side, no new endpoint.** Blazor's second table +comes from the `GetExternalCollections` MediatR query (one row per Plex/Jellyfin/Emby source with +`Libraries.Any(ShouldSyncItems)`), which has no REST wrapper. Rather than add one, the SPA derives the rows +from the existing `GET /api/media-sources`: `GetAllMediaSourcesForApiHandler` already filters each source's +`libraries` to sync-enabled entries (`ShouldIncludeLibrary` = `ShouldSyncItems` for remote families), so a +remote source with a **non-empty `libraries` list** is *exactly* the `Libraries.Any(ShouldSyncItems)` filter. +No extra endpoint, no extra fetch, single source of truth. (If `MediaSourceLibraryResponseModel` ever stops +filtering to sync-enabled libraries, this equivalence breaks — a test asserting the row set guards it.) + +**Decision 2 — collections-scan buttons use optimistic, timeout-bounded pending (no poll reconcile).** Library +scans reconcile their optimistic "pending" flag against `GET /api/libraries/scan-status`. Collections scans +have **no** such surface — that endpoint is library-keyed, and Blazor only ever observed collections locks via +in-process `IEntityLocker` `Are{X}CollectionsLocked` events with no HTTP mirror. So the `useCollectionsScan` +hook disables a button optimistically on click and clears it after a fixed `COLLECTIONS_PENDING_TIMEOUT_MS` +(a 409 "already scanning" is benign and keeps it disabled until the timeout; a 404/network clears it and +surfaces the error). This is the honest ceiling of what the current API exposes. **Follow-up (backlog):** a +collections scan-status endpoint would let the SPA reconcile collections pending against a live active set the +way library scans do; filed as **#271** so the timeout isn't mistaken for the intended end state. diff --git a/docs/endpoint-index.md b/docs/endpoint-index.md index d1b1ad3e5..7f833e36d 100644 --- a/docs/endpoint-index.md +++ b/docs/endpoint-index.md @@ -2,7 +2,7 @@ *Generated by `scripts/generate-endpoint-index.py` from `ErsatzTV/wwwroot/openapi/v1.json`. Do not edit by hand -- regenerated by `scripts/update-openapi.sh`.* -155 endpoints, 240 operations. +158 endpoints, 243 operations. ## Artists @@ -122,6 +122,7 @@ | GET | `/api/media-sources/emby/{id}/path-replacements` | GetEmbyPathReplacements | Get an Emby source's path replacements | | PUT | `/api/media-sources/emby/{id}/path-replacements` | ReplaceEmbyPathReplacements | Replace an Emby source's path replacements | | POST | `/api/media-sources/emby/{id}/refresh-libraries` | RefreshEmbyLibraries | Refresh an Emby source's libraries | +| POST | `/api/media-sources/emby/{id}/scan-collections` | ScanEmbyCollections | Scan an Emby source's collections | ## FFmpeg Profiles @@ -176,6 +177,7 @@ | GET | `/api/media-sources/jellyfin/{id}/path-replacements` | GetJellyfinPathReplacements | Get a Jellyfin source's path replacements | | PUT | `/api/media-sources/jellyfin/{id}/path-replacements` | ReplaceJellyfinPathReplacements | Replace a Jellyfin source's path replacements | | POST | `/api/media-sources/jellyfin/{id}/refresh-libraries` | RefreshJellyfinLibraries | Refresh a Jellyfin source's libraries | +| POST | `/api/media-sources/jellyfin/{id}/scan-collections` | ScanJellyfinCollections | Scan a Jellyfin source's collections | ## Languages @@ -295,6 +297,7 @@ | GET | `/api/media-sources/plex/{id}/path-replacements` | GetPlexPathReplacements | Get a Plex server's path replacements | | PUT | `/api/media-sources/plex/{id}/path-replacements` | ReplacePlexPathReplacements | Replace a Plex server's path replacements | | POST | `/api/media-sources/plex/{id}/refresh-libraries` | RefreshPlexLibraries | Refresh a Plex server's libraries | +| POST | `/api/media-sources/plex/{id}/scan-collections` | ScanPlexCollections | Scan a Plex server's collections | ## Rerun Collections diff --git a/docs/handoffs/chicorytv-issue-queue.md b/docs/handoffs/chicorytv-issue-queue.md index 596aa1692..409c11258 100644 --- a/docs/handoffs/chicorytv-issue-queue.md +++ b/docs/handoffs/chicorytv-issue-queue.md @@ -213,3 +213,19 @@ HARD CONSTRAINTS: on #N", status tables), not just resolve the conflict markers — git fast-forwards stale *facts* silently. The hand-written-docs sibling of the "regenerate generated artifacts after merging main" lesson. Also: docs-only PRs still trip main's `Build & test` branch protection, so a rebase = a fresh full-CI cycle before merge. +- **A lock/channel-ownership "no cross-release" verdict must enumerate EVERY producer of that message + type, not just the enqueuers visible in the PR diff** (2026-07-11, #235/#267). The cold fork cleared the + new `scan-collections` per-provider lock as sound — "only the acquirer enqueues, so only its message + triggers the scanner unlock" — because it checked the three new controllers but never grepped for ALL + `Synchronize{X}Collections(` senders. Codex did: `SchedulerService` also enqueues them *periodically and + unlocked*, and `ScannerService`'s `finally` released the lock whenever held → a scheduled scan + cross-releases an API scan's lock (#250 class). Rule: before trusting any "single owner / no double + release / no cross-release" claim, `grep` the whole host project for every writer of that channel message + (or acquirer of that lock) — the background scheduler/worker is the usual missing producer. This is why the + independent Codex pass earns its keep alongside the fork even when the fork returns MERGEABLE. +- **Fork-vs-Codex disagreement on a gate PR → Fable, and Fable often improves the fix, not just the verdict** + (2026-07-11, #235): the reconciliation isn't only "who's right" — Fable ratified Codex's High AND caught + that the orchestrator's proposed *fix* (per-source lock-or-skip) would deterministically starve the 2nd+ + media source (fixed loop order), prescribing the shipped library-scan §C7 pattern instead (lock once per + provider, batch-enqueue, release on the `Unlock: isLast` message). Escalate the FIX approach too, not just + the finding. diff --git a/web/src/api/decoTemplates.ts b/web/src/api/decoTemplates.ts index 3e88a15aa..09d01739a 100644 --- a/web/src/api/decoTemplates.ts +++ b/web/src/api/decoTemplates.ts @@ -1,4 +1,4 @@ -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; export type DecoTemplateGroup = components['schemas']['DecoTemplateGroupResponseModel']; @@ -46,8 +46,25 @@ export function getDecoTemplateItems(id: number): Promise { return request(`/api/deco-templates/${id}/items`); } -export function replaceDecoTemplate(id: number, body: ReplaceDecoTemplateRequest): Promise { - return request(`/api/deco-templates/${id}`, { body, method: 'PUT' }); +/** Load deco template items together with the deco template's concurrency ETag (issue #253). */ +export function getDecoTemplateItemsWithMeta(id: number): Promise> { + return requestWithMeta(`/api/deco-templates/${id}/items`); +} + +/** + * Replace a deco template. Pass the last-seen ETag as `If-Match` to reject a stale overwrite with + * 412; the resolved value carries the new ETag for a subsequent save (issue #253). + */ +export function replaceDecoTemplate( + id: number, + body: ReplaceDecoTemplateRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/deco-templates/${id}`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function messageFromDecoTemplateError(error: unknown, fallback = 'Unable to load deco templates'): string { diff --git a/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index ed3c8192a..2a1ed580b 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -1094,6 +1094,7 @@ export interface components { "buildStatus": null | components["schemas"]["PlayoutBuildStatusResponseModel"]; "decoId": null | number; "decoName": null | string; + "isLocked": boolean; }; "PlayoutScheduleKind": "None" | "Classic" | "Block" | "Sequential" | "Scripted" | "ExternalJson"; "PlayoutSettingsResponseModel": { @@ -1154,6 +1155,7 @@ export interface components { "shuffleScheduleItems": boolean; "randomStartPoint": boolean; "fixedStartTimeBehavior": components["schemas"]["FixedStartTimeBehavior"]; + "version": number; }; "RemoteConnectionResponseModel": { "address": null | string; @@ -1244,6 +1246,11 @@ export interface components { "selectedName": null | string; "firstRunPlaybackOrder": components["schemas"]["PlaybackOrder"]; "rerunPlaybackOrder": components["schemas"]["PlaybackOrder"]; + }; + "ResetAllPlayoutsResponseModel": { + "queuedPlayoutIds": Array; + "skippedLocked": Array; + "skippedUnsupported": Array; }; "ResolutionResponseModel": { "id": number; diff --git a/web/src/api/libraries.test.ts b/web/src/api/libraries.test.ts index 349f26bb0..2cb3745cd 100644 --- a/web/src/api/libraries.test.ts +++ b/web/src/api/libraries.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { scanLibrary, scanShow } from './libraries'; +import { scanCollections, scanLibrary, scanShow } from './libraries'; function noContent(): Response { return new Response(null, { status: 200 }); @@ -22,6 +22,25 @@ describe('libraries api client', () => { expect(fetchMock).toHaveBeenCalledWith('/api/libraries/4/scan', expect.objectContaining({ method: 'POST' })); }); + it('scanLibrary appends ?deep=true for a deep scan', async () => { + const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); + await scanLibrary(4, true); + expect(fetchMock).toHaveBeenCalledWith('/api/libraries/4/scan?deep=true', expect.objectContaining({ method: 'POST' })); + }); + + it('scanCollections POSTs to the media-source scan-collections endpoint', async () => { + const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); + await scanCollections('jellyfin', 7); + expect(fetchMock).toHaveBeenCalledWith('/api/media-sources/jellyfin/7/scan-collections', expect.objectContaining({ method: 'POST' })); + }); + + it('scanCollections appends ?deep=true for a deep scan', async () => { + const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); + await scanCollections('plex', 3, true); + const { url } = lastCall(fetchMock); + expect(url).toBe('/api/media-sources/plex/3/scan-collections?deep=true'); + }); + it('scanShow POSTs the show id and deepScan flag', async () => { const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); await scanShow(4, { deepScan: true, showId: 42 }); diff --git a/web/src/api/libraries.ts b/web/src/api/libraries.ts index 66361eae9..c9263f8ad 100644 --- a/web/src/api/libraries.ts +++ b/web/src/api/libraries.ts @@ -16,7 +16,7 @@ export type LibrariesScreenQueryState = data: LibrariesScreenData; error: string | null; refresh: () => void; - scanLibrary: (libraryId: number) => Promise; + scanLibrary: (libraryId: number, deep?: boolean) => Promise; scanningLibraryIds: Set; status: 'success'; } @@ -42,8 +42,24 @@ export function getLibraryScanStatus(): Promise { ); } -export function scanLibrary(libraryId: number): Promise { - return request(`/api/libraries/${libraryId}/scan`, { method: 'POST' }); +// Queues a library scan. `deep` requests a deep (force-metadata) scan — the API equivalent of the +// legacy Blazor "Deep Scan Library" button (#235 F9); omit or pass false for a quick scan. Returns +// 202 when queued, 404 (missing), 409 (already scanning), 422 (sync disabled) — see §3b. +export function scanLibrary(libraryId: number, deep = false): Promise { + const query = deep ? '?deep=true' : ''; + return request(`/api/libraries/${libraryId}/scan${query}`, { method: 'POST' }); +} + +// Media-source families that support an external-collections scan (#235 F9). Matches the three +// media-source controllers #202 introduced. +export type CollectionsScanSource = 'emby' | 'jellyfin' | 'plex'; + +// Queues an external-collections scan for a Plex/Jellyfin/Emby media source — the API equivalent of +// the legacy Blazor "Scan Collections" button (#235 F9). `deep` requests a deep scan. Returns 202 +// when queued, 404 (source missing), 409 (a collections scan is already running for that source). +export function scanCollections(source: CollectionsScanSource, sourceId: number, deep = false): Promise { + const query = deep ? '?deep=true' : ''; + return request(`/api/media-sources/${source}/${sourceId}/scan-collections${query}`, { method: 'POST' }); } export interface ScanShowParams { @@ -51,9 +67,11 @@ export interface ScanShowParams { deepScan?: boolean; } -// Queues a scan of a single show (by id) within a library. Returns 200 on success, 404 when the -// show id doesn't exist in the library, 400 when the library doesn't support single-show -// scanning. Body keys are `showId` and `deepScan` (see LibrariesController.ScanShowRequest). +// Queues a scan of a single show (by id) within a library. Returns 202 when queued, 404 when the +// show id doesn't exist in the library, 409 when a scan is already running, and 422 when the +// library doesn't support single-show scanning / sync is disabled / the scan failed to start (all +// error bodies are ProblemDetails; #235 normalized the old conflated 400). Body keys are `showId` +// and `deepScan` (see LibrariesController.ScanShowRequest). export function scanShow(libraryId: number, params: ScanShowParams): Promise { return request(`/api/libraries/${libraryId}/scan-show`, { body: { deepScan: params.deepScan ?? false, showId: params.showId }, @@ -297,9 +315,10 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta }); }, []); - const triggerScan = useCallback((libraryId: number): Promise => { + const triggerScan = useCallback((libraryId: number, deep = false): Promise => { if (pendingIdsRef.current.has(libraryId) || activeIdsRef.current.has(libraryId)) { - // Already pending or active - ignore the duplicate submission. + // Already pending or active - ignore the duplicate submission. Quick and deep scans share the + // same per-library lock, so both affordances gate on (and reconcile through) this one id set. return Promise.resolve(); } @@ -320,7 +339,7 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta }; }); - return scanLibrary(libraryId) + return scanLibrary(libraryId, deep) .then(() => { // 202 Accepted - a scan is genuinely queued. Poll scan-status; the pending flag is // promoted to "active" once the scan appears there. @@ -381,6 +400,107 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta return { data: null, error: null, refresh, status: 'loading' }; } +// --- External collections scan (Plex/Jellyfin/Emby) --- + +// A pending collections scan re-enables its button after this bound. Unlike library scans, an +// external-collections scan has NO authoritative "in progress" REST surface: /api/libraries/scan-status +// is library-keyed, and Blazor only ever observed collections locks through in-process IEntityLocker +// events (Are{X}CollectionsLocked) that have no HTTP mirror. So we can't reconcile "pending" against a +// live active set the way library scans do - we optimistically disable, then give up after this bound +// so a button can't wedge disabled forever. (A collections scan-status endpoint would let us reconcile +// properly - see the #91b follow-up note.) +const COLLECTIONS_PENDING_TIMEOUT_MS = 30000; + +export interface CollectionsScanState { + error: string | null; + scan: (source: CollectionsScanSource, sourceId: number, deep: boolean) => Promise; + scanningKeys: Set; +} + +// Stable key for a per-(family, source) collections scan, used by both the hook and the screen so the +// "is this row scanning?" lookup has exactly one definition. +export function collectionsScanKey(source: CollectionsScanSource, sourceId: number): string { + return `${source}:${sourceId}`; +} + +export function useCollectionsScan(): CollectionsScanState { + const [scanningKeys, setScanningKeys] = useState>(new Set()); + const [error, setError] = useState(null); + // Mirror kept in sync synchronously so scan() can guard double-submits without waiting for a render. + const scanningKeysRef = useRef>(new Set()); + const timeoutsRef = useRef>(new Map()); + const activeRef = useRef(true); + + useEffect(() => { + activeRef.current = true; + // Identity is stable across the hook's life (we only .set/.delete entries, never reassign the + // Map), so capturing it here is the same instance the cleanup clears at unmount. + const timeouts = timeoutsRef.current; + + return () => { + activeRef.current = false; + timeouts.forEach((timeoutId) => window.clearTimeout(timeoutId)); + timeouts.clear(); + }; + }, []); + + const clearKey = useCallback((key: string) => { + const timeoutId = timeoutsRef.current.get(key); + if (timeoutId !== undefined) { + window.clearTimeout(timeoutId); + timeoutsRef.current.delete(key); + } + + const next = new Set(scanningKeysRef.current); + next.delete(key); + scanningKeysRef.current = next; + + if (activeRef.current) { + setScanningKeys(next); + } + }, []); + + const scan = useCallback( + (source: CollectionsScanSource, sourceId: number, deep: boolean): Promise => { + const key = collectionsScanKey(source, sourceId); + if (scanningKeysRef.current.has(key)) { + // Already pending - ignore the duplicate submission (quick and deep share the source lock). + return Promise.resolve(); + } + + const pending = new Set(scanningKeysRef.current).add(key); + scanningKeysRef.current = pending; + setScanningKeys(pending); + setError(null); + + const timeoutId = window.setTimeout(() => clearKey(key), COLLECTIONS_PENDING_TIMEOUT_MS); + timeoutsRef.current.set(key, timeoutId); + + return scanCollections(source, sourceId, deep) + .then(() => { + // 202 Accepted - a scan is genuinely queued. Keep the button disabled until the bounded + // timeout expires (there is no completion signal to reconcile against). + }) + .catch((err: unknown) => { + if (err instanceof ApiError && err.status === 409) { + // 409 Conflict - a collections scan is already running for this source. Benign: keep the + // optimistic pending flag (button stays disabled) with no error; it clears on the timeout. + return; + } + + // 404 (source missing) / network error - nothing was queued, so re-enable and surface it. + clearKey(key); + if (activeRef.current) { + setError(messageFromLibrariesError(err, 'Unable to scan collections')); + } + }); + }, + [clearKey] + ); + + return { error, scan, scanningKeys }; +} + function messageFromLibrariesError(error: unknown, fallback = 'Unable to load libraries'): string { if (error instanceof ApiError) { return error.detail ?? error.message; diff --git a/web/src/api/playlists.ts b/web/src/api/playlists.ts index bc6b4af49..c6535ab70 100644 --- a/web/src/api/playlists.ts +++ b/web/src/api/playlists.ts @@ -1,4 +1,4 @@ -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; export type AddItemsToPlaylistRequest = components['schemas']['AddItemsToPlaylistRequest']; @@ -41,13 +41,28 @@ export function getPlaylistItems(id: number): Promise { return request(`/api/playlists/${id}/items`); } +/** Load playlist items together with the playlist's concurrency ETag (issue #253). */ +export function getPlaylistItemsWithMeta(id: number): Promise> { + return requestWithMeta(`/api/playlists/${id}/items`); +} + export function createPlaylist(body: CreatePlaylistRequest): Promise { return request('/api/playlists', { body, method: 'POST' }); } -// PUT = rename + replace the full item list; returns the persisted (re-indexed) items. -export function updatePlaylist(id: number, body: ReplacePlaylistRequest): Promise { - return request(`/api/playlists/${id}`, { body, method: 'PUT' }); +// PUT = rename + replace the full item list; returns the persisted (re-indexed) items. Pass the +// last-seen ETag as `If-Match` to reject a stale overwrite with 412; the resolved value carries +// the new ETag for a subsequent save (issue #253). +export function updatePlaylist( + id: number, + body: ReplacePlaylistRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/playlists/${id}`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function deletePlaylist(id: number): Promise { diff --git a/web/src/api/playouts.ts b/web/src/api/playouts.ts index 7aa49967f..5d5e08aab 100644 --- a/web/src/api/playouts.ts +++ b/web/src/api/playouts.ts @@ -108,8 +108,13 @@ export function getPlayoutChannelStates(): Promise { return request('/api/channels/state'); } -export function resetAllPlayouts(): Promise { - return request('/api/playouts/reset-all', { method: 'POST' }); +export type ResetAllPlayoutsResult = components['schemas']['ResetAllPlayoutsResponseModel']; + +// Queues a reset of every eligible playout. Returns 202 with a body reporting which playouts were +// queued vs skipped (locked, or an unsupported ExternalJson/None schedule kind) — #235 replaced the +// old silent skip. The caller may surface `skipped*` to explain why some playouts didn't reset. +export function resetAllPlayouts(): Promise { + return request('/api/playouts/reset-all', { method: 'POST' }); } export function deletePlayout(playoutId: number): Promise { diff --git a/web/src/api/schedules.ts b/web/src/api/schedules.ts index 4aac218a7..b87ba5b28 100644 --- a/web/src/api/schedules.ts +++ b/web/src/api/schedules.ts @@ -1,4 +1,4 @@ -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; // FillerPreset is exported by ./pickers — import (don't re-export) to avoid a duplicate `export *` // name in api/index.ts. LanguageCode/getLanguages moved to ./languages (also re-exported via @@ -46,17 +46,31 @@ export function getScheduleItems(scheduleId: number): Promise(`/api/schedules/${scheduleId}/items`); } +/** Load schedule items together with the schedule's concurrency ETag (issue #253). */ +export function getScheduleItemsWithMeta( + scheduleId: number +): Promise> { + return requestWithMeta(`/api/schedules/${scheduleId}/items`); +} + export function addScheduleItem(scheduleId: number, body: ScheduleItemRequest): Promise { return request(`/api/schedules/${scheduleId}/items`, { body, method: 'POST' }); } -// Destructive replace: the server deletes+recreates every item row (new ids) and triggers playout -// rebuilds. The editor batches all local draft edits into this single call. See docs/decisions.md. +// Positional in-place reconcile: the server reuses same-typed item rows (keeping fill-group state) and +// triggers playout rebuilds. The editor batches all local draft edits into this single call. Pass the +// last-seen ETag as `If-Match` to reject a stale overwrite with 412; the resolved value carries the new +// ETag for a subsequent save (issue #253). See docs/decisions.md. export function replaceScheduleItems( scheduleId: number, - body: ReplaceScheduleItemsRequest -): Promise { - return request(`/api/schedules/${scheduleId}/items`, { body, method: 'PUT' }); + body: ReplaceScheduleItemsRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/schedules/${scheduleId}/items`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function deleteScheduleItem(scheduleId: number, itemId: number): Promise { diff --git a/web/src/api/templates.ts b/web/src/api/templates.ts index d4788d03f..c91c28e9d 100644 --- a/web/src/api/templates.ts +++ b/web/src/api/templates.ts @@ -1,4 +1,4 @@ -import { ApiError, request } from './client'; +import { ApiError, request, requestWithMeta, type ResponseWithMeta } from './client'; import type { components } from './generated/v1'; export type TemplateGroup = components['schemas']['TemplateGroupResponseModel']; @@ -47,8 +47,25 @@ export function getTemplateItems(id: number): Promise { return request(`/api/templates/${id}/items`); } -export function replaceTemplate(id: number, body: ReplaceTemplateRequest): Promise { - return request(`/api/templates/${id}`, { body, method: 'PUT' }); +/** Load template items together with the template's concurrency ETag (issue #253). */ +export function getTemplateItemsWithMeta(id: number): Promise> { + return requestWithMeta(`/api/templates/${id}/items`); +} + +/** + * Replace a template. Pass the last-seen ETag as `If-Match` to reject a stale overwrite with 412; + * the resolved value carries the new ETag for a subsequent save (issue #253). + */ +export function replaceTemplate( + id: number, + body: ReplaceTemplateRequest, + ifMatch?: string | null +): Promise> { + return requestWithMeta(`/api/templates/${id}`, { + body, + method: 'PUT', + headers: ifMatch ? { 'If-Match': ifMatch } : undefined + }); } export function copyTemplate(id: number, body: CopyTemplateRequest): Promise