From 5e5f0af68476eccff5d1f3b2dc766e6ea088b515 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 17:54:21 +0200 Subject: [PATCH 1/8] fix(235-A): normalize async-op error contracts on Maintenance + Troubleshoot controllers (#235) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice A of the async-op API contract normalization. MaintenanceController: - EmptyTrash error path: was 500 text/plain (error.ToString()); now maps the BaseError Left through ApiResults.ToErrorResult() -> 404 (NotFoundError) / 422 ProblemDetails. Success stays 200 OkResult. Added ProducesResponseType 200 + 422. - CleanArtwork: fire-and-forget enqueue of DeleteOrphanedArtwork was a silent 200; now returns 202 Accepted (AcceptedResult) since it queues background work. Added ProducesResponseType 202. (Controller does not derive from ControllerBase, so results are built directly as before.) TroubleshootController.TroubleshootPlayback (GET|HEAD /api/troubleshoot/playback.m3u8): - Two bare body-less NotFound() call sites conflated "not found" with "prepare/ playback failure". Both now return a ProblemDetails body: * prepare-failure (result.IsLeft): mapped through error.ToErrorResult() -> 404 for NotFoundError (unknown media item/channel) else 422 for a validation BaseError. * terminal fall-through (prepare ok but no playable output): kept 404 with a distinguishing ApiResults.NotFoundProblem(...) detail. - Added ProducesResponseType 404 + 422 (409 already present). Consumer check: the SPA (PlaybackTroubleshootingScreen) feeds the playback.m3u8 URL straight to hls.js via HlsPlayer, which never inspects the HTTP status code — playback state is surfaced via the separate /api/troubleshoot/playback/status poll. So the 404->422 split for the validation subcase is safe; no player code branches on the status code. Tests: MaintenanceControllerTests (200/422/202 + enqueue assertion), TroubleshootControllerTests (prepare 404 NotFoundError, 422 validation). All green; Api error-metadata/contract/security scans still pass. Note: OpenAPI artifacts (v1.json / v1.d.ts) intentionally NOT regenerated here — the orchestrator regenerates once after all #235 slices merge. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Controllers/MaintenanceControllerTests.cs | 67 +++++++++++++++++++ .../TroubleshootControllerTests.cs | 50 ++++++++++++++ .../Controllers/Api/MaintenanceController.cs | 13 ++-- .../Controllers/Api/TroubleshootController.cs | 19 +++++- 4 files changed, 139 insertions(+), 10 deletions(-) create mode 100644 ErsatzTV.Tests/Controllers/MaintenanceControllerTests.cs 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/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/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/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")] -- 2.47.3 From d02f953922e8d30d3265e040e7404107456e62ac Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 17:57:41 +0200 Subject: [PATCH 2/8] fix(235-F7): release leaked Trakt lock on worker shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global Trakt lock is acquired by SchedulerService.RefreshTraktLists / MatchTraktLists (and TraktController) and released only when the *terminal* message of a batch — the one carrying Unlock: true (list == traktLists.Last()) — is processed by WorkerService, whose handler (AddTraktListHandler / MatchTraktListItemsHandler) calls IEntityLocker.UnlockTrakt() in a finally. WorkerService.ExecuteAsync breaks out of the read loop on stoppingToken.IsCancellationRequested (and exits on channel completion / reader cancellation) BEFORE processing the next message. If shutdown lands after a batch is enqueued but before its terminal Unlock: true message is handled, UnlockTrakt() never runs and the in-memory Trakt lock leaks for the rest of the process lifetime (subsequent Trakt operations 409 forever). Fix (option a): make the batch-release loss-tolerant with a compensating release in a finally around the read loop — if the Trakt lock is still held when the worker stops, release it. Chosen over tracking pending ownership (b) because the lock is a global singleton and WorkerService is its sole batch-release site, so "held at shutdown" unambiguously means "the terminal release was lost"; covers all three exit paths (break / channel completion / cancellation) in one place. Same lock-lifecycle class as #231/#233/#234. Regression test: WorkerServiceTests gates the first (non-terminal) batch message on the stopping token, then StopAsync-cancels so the worker breaks before the terminal Unlock: true message — asserts the lock is released and the terminal message was never processed. Proven non-vacuous: inverting the finally condition fails the test. Backend-only; no controller/DTO/SPA/OpenAPI impact. Co-Authored-By: Claude Opus 4.8 (1M context) --- ErsatzTV.Tests/Services/WorkerServiceTests.cs | 101 ++++++++++++++++++ ErsatzTV/Services/WorkerService.cs | 18 ++++ 2 files changed, 119 insertions(+) create mode 100644 ErsatzTV.Tests/Services/WorkerServiceTests.cs 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/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(); + } + } } } -- 2.47.3 From 9b73b62527c111c3987503f2bb945d1012ce8932 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 17:58:15 +0200 Subject: [PATCH 3/8] =?UTF-8?q?feat(235):=20async-op=20API=20contract=20no?= =?UTF-8?q?rmalization=20=E2=80=94=20playouts=20slice=20C=20(#235)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice C of the async-op contract normalization: - channel reset (POST /api/channels/{channelNumber}/playout/reset) now returns 202 Accepted (was 200 Ok) — it only queues a background rebuild - reset-all (POST /api/playouts/reset-all) still 202 but now returns a ResetAllPlayoutsResponseModel body reporting QueuedPlayoutIds / SkippedLocked / SkippedUnsupported instead of silently swallowing skips; handler returns a new ResetAllPlayoutsResult record - single-playout GET (GET /api/playouts/{id}) now exposes IsLocked on PlayoutResponseModel, set from IEntityLocker.IsPlayoutLocked mirroring the list projection — gives a polling client the lock flag Tests: channel reset asserts 202; reset-all asserts 202 + skipped-body shape; single GET asserts IsLocked; new ResetAllPlayoutsHandlerTests (in-memory SQLite) asserts locked/ExternalJson/None land in skipped lists and eligible playouts in queued. docs/api-conventions.md §3a updated. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Playouts/Commands/ResetAllPlayouts.cs | 2 +- .../Commands/ResetAllPlayoutsHandler.cs | 27 +++++- .../Playouts/ResetAllPlayoutsResult.cs | 6 ++ .../Api/Playouts/PlayoutResponseModel.cs | 9 +- .../Playouts/ResetAllPlayoutsResponseModel.cs | 7 ++ .../Playouts/ResetAllPlayoutsHandlerTests.cs | 82 +++++++++++++++++++ .../Controllers/ChannelControllerTests.cs | 4 +- .../Controllers/PlayoutControllerTests.cs | 30 ++++++- ErsatzTV/Controllers/Api/ChannelController.cs | 4 +- ErsatzTV/Controllers/Api/PlayoutController.cs | 29 ++++--- docs/api-conventions.md | 17 +++- 11 files changed, 188 insertions(+), 29 deletions(-) create mode 100644 ErsatzTV.Application/Playouts/ResetAllPlayoutsResult.cs create mode 100644 ErsatzTV.Core/Api/Playouts/ResetAllPlayoutsResponseModel.cs create mode 100644 ErsatzTV.Tests/Application/Playouts/ResetAllPlayoutsHandlerTests.cs 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.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/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/Controllers/ChannelControllerTests.cs b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs index 67b84c74e..5e1deb4e3 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/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index 69b5171ef..0fc9e3bc5 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -527,6 +527,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() { @@ -693,9 +708,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()); } @@ -1286,7 +1309,7 @@ public class PlayoutControllerTests null, null); - private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm) => + private static PlayoutResponseModel ToResponse(PlayoutNameViewModel vm, bool isLocked = false) => PlayoutResponseModel.From( vm.PlayoutId, vm.ScheduleKind, @@ -1303,7 +1326,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/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/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index a0bff515f..c023135d7 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()); } @@ -550,14 +553,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")] @@ -728,7 +736,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, @@ -740,7 +748,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/docs/api-conventions.md b/docs/api-conventions.md index 38a944ce1..14861a905 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` -- 2.47.3 From 628c9d722818ecb0fd603d182549fcada4613356 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:02:17 +0200 Subject: [PATCH 4/8] =?UTF-8?q?feat(235):=20F9=20API=20parity=20=E2=80=94?= =?UTF-8?q?=20library=20deep-scan,=20external-collections=20scan,=20scan-s?= =?UTF-8?q?how=20outcome=20enum=20(#235=20slice=20B)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the two F9 Libraries.razor parity gaps and normalizes scan-show error mapping to ProblemDetails. TASK 1 — library-wide deep scan: - QueueLibraryScanByLibraryId gains optional `bool DeepScan = false`; handler threads it into ForceSynchronize{Plex,Jellyfin,Emby}LibraryById. - POST /api/libraries/{id}/scan?deep=false binds it via [FromQuery]. TASK 2 — external-collections scan (new endpoints): - POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=false acquires the per-source collections lock (§3b: lock IS the running scan → 409), enqueues Synchronize{X}Collections(id, ForceScan:true, deep) to the scanner channel, returns 202; compensating-unlock on enqueue throw. TASK 3 — scan-show normalization: - New QueueShowScanResult enum; handler returns it instead of bool. - POST /api/libraries/{id}/scan-show now maps 202/404/409/422 (all errors ProblemDetails) instead of 200/404/400-anonymous-object. - Updated the lone Blazor caller (TelevisionSeasonList.razor). Tests: LibrariesController (scan deep=true, scan-show enum→status), the three media-source controllers (scan-collections route/404/409/202/compensating-unlock), and handler tests for both changed handlers (deep threading + show-scan outcomes). Docs: api-conventions §3b exemplar + blazor-route-parity §5 F9 gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Commands/QueueLibraryScanByLibraryId.cs | 2 +- .../QueueLibraryScanByLibraryIdHandler.cs | 6 +- .../Commands/QueueShowScanByLibraryId.cs | 13 +- .../QueueShowScanByLibraryIdHandler.cs | 26 ++- ...QueueLibraryScanByLibraryIdHandlerTests.cs | 32 ++- .../QueueShowScanByLibraryIdHandlerTests.cs | 214 ++++++++++++++++++ .../EmbyMediaSourcesControllerTests.cs | 46 ++++ .../JellyfinMediaSourcesControllerTests.cs | 46 ++++ .../Controllers/LibrariesControllerTests.cs | 91 +++++++- .../PlexMediaSourcesControllerTests.cs | 62 +++++ .../Api/EmbyMediaSourcesController.cs | 47 ++++ .../Api/JellyfinMediaSourcesController.cs | 47 ++++ .../Controllers/Api/LibrariesController.cs | 49 +++- .../Api/PlexMediaSourcesController.cs | 45 ++++ ErsatzTV/Pages/TelevisionSeasonList.razor | 5 +- docs/api-conventions.md | 9 + docs/blazor-route-parity.md | 6 +- 17 files changed, 704 insertions(+), 42 deletions(-) create mode 100644 ErsatzTV.Tests/Application/Libraries/QueueShowScanByLibraryIdHandlerTests.cs 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.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/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/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/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..d75a11a5e 100644 --- a/ErsatzTV/Controllers/Api/LibrariesController.cs +++ b/ErsatzTV/Controllers/Api/LibrariesController.cs @@ -22,13 +22,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 +54,47 @@ 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." + }), + _ => ApiResults.NotFoundProblem($"Library {id} does not exist.") + }; } return ApiResults.NotFoundProblem($"Show {request.ShowId} does not exist in library {id}."); 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/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/docs/api-conventions.md b/docs/api-conventions.md index 38a944ce1..639868c98 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -135,6 +135,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. diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index 6ae11a304..1addc1409 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -270,8 +270,10 @@ 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 +**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). (#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: -- 2.47.3 From a27cfc475e0453dd75fb2cb0710ffd33630c5f54 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:04:33 +0200 Subject: [PATCH 5/8] chore(235): regenerate OpenAPI artifacts + endpoint index after slice merge Co-Authored-By: Claude Opus 4.8 (1M context) --- ErsatzTV/wwwroot/openapi/v1.json | 423 ++++++++++++++++++++++++++++++- docs/endpoint-index.md | 5 +- web/src/api/generated/v1.d.ts | 6 + 3 files changed, 423 insertions(+), 11 deletions(-) diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 21666f098..01ed64660 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -1822,8 +1822,8 @@ } ], "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" }, "404": { "description": "Not Found", @@ -4798,6 +4798,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": [ @@ -6576,6 +6650,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": [ @@ -6662,6 +6810,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", @@ -6671,6 +6820,14 @@ "type": "integer", "format": "int32" } + }, + { + "name": "deep", + "in": "query", + "schema": { + "type": "boolean", + "default": false + } } ], "responses": { @@ -6783,8 +6940,8 @@ "required": true }, "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" }, "404": { "description": "Not Found", @@ -6806,8 +6963,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": { @@ -7548,6 +7725,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" + } + } + } } } } @@ -7559,8 +7756,8 @@ ], "summary": "Clean artwork cache", "responses": { - "200": { - "description": "OK" + "202": { + "description": "Accepted" } } } @@ -10458,7 +10655,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" + } + } + } } } } @@ -11275,6 +11489,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": [ @@ -15846,6 +16134,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": { @@ -15865,6 +16173,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" + } + } + } } } }, @@ -15960,6 +16288,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": { @@ -15979,6 +16327,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" + } + } + } } } } @@ -22533,7 +22901,8 @@ "dailyRebuildTime", "buildStatus", "decoId", - "decoName" + "decoName", + "isLocked" ], "type": "object", "properties": { @@ -22591,6 +22960,9 @@ "null", "string" ] + }, + "isLocked": { + "type": "boolean" } } }, @@ -23422,6 +23794,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/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/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index b03c5e51d..d4ee00744 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -1093,6 +1093,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": { @@ -1243,6 +1244,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; -- 2.47.3 From d32ca976f72f881efe5d0426cd83977b7be4d56a Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:10:02 +0200 Subject: [PATCH 6/8] feat(235): SPA clients for deep/collections scan + typed reset-all; docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - libraries.ts: scanLibrary(id, deep), new scanCollections(source, id, deep), corrected stale scanShow status-code comment (400 -> 202/404/409/422) - playouts.ts: resetAllPlayouts returns typed ResetAllPlayoutsResponseModel body - libraries.test.ts: deep-scan + scanCollections client tests - decisions.md: #235 async-op contract + F9 endpoints + accepted-by-design channels note - blazor-route-parity.md §5: F9 API gate closed; SPA deep/collections buttons = removal-PR work Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/blazor-route-parity.md | 6 +++- docs/decisions.md | 55 +++++++++++++++++++++++++++++++++++ web/src/api/libraries.test.ts | 21 ++++++++++++- web/src/api/libraries.ts | 28 ++++++++++++++---- web/src/api/playouts.ts | 9 ++++-- 5 files changed, 110 insertions(+), 9 deletions(-) diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index 1addc1409..ed95b8e5e 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -273,7 +273,11 @@ The removal PR is **gated** — it starts only after these clear: ~~#202 (media- **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). (#204's id-carrying pattern +proceed, and the **mandatory cold adversarial pass** (#91 comment 2026-07-09). **Remaining SPA affordance for +the removal PR** (API + thin `web/src/api/libraries.ts` clients — `scanLibrary(id, deep)`, `scanCollections` — +already shipped by #235): `LibrariesScreen` currently exposes only quick-scan; the removal PR must add the +**deep-scan** and **external-collections-scan** buttons (wiring the shipped clients) before deleting +`Libraries.razor`, or that capability is lost. (#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: diff --git a/docs/decisions.md b/docs/decisions.md index ce6d41b94..0d7a1d738 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -649,3 +649,58 @@ action of the Step 2 deletion PR merge** (not before — `main` moves until then cut. 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. 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..97b66ad98 100644 --- a/web/src/api/libraries.ts +++ b/web/src/api/libraries.ts @@ -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 }, diff --git a/web/src/api/playouts.ts b/web/src/api/playouts.ts index 839281c3d..529800061 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 { -- 2.47.3 From 787058d18c049cd3dde0ba8ae3b1c5e6c29e1cb0 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:37:35 +0200 Subject: [PATCH 7/8] fix(235): scheduler-safe collections lock ownership (Codex High / Fable reconciliation) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections endpoints acquire a per-provider collections lock (409 if held) and hand the single release to the ScannerService finally. But SchedulerService's periodic collection scans were enqueued WITHOUT the lock, and ScannerService's finally released the collections lock whenever held with no ownership check. A scheduler-queued scan running while an API request held the lock cross-released the API's lock (#250 bug class), letting a second API request get a spurious 202 instead of 409. Fix (mirrors the SynchronizePlexLibraryByIdIfNeeded(Unlock: !networksFollow) library-scan precedent): - Add `bool Unlock = true` (4th positional param) to the three Synchronize{Plex,Jellyfin,Emby}Collections records; default keeps the controller + Libraries.razor call sites compiling and releasing on run. - ScannerService: the three collection finallys now honor `request.Unlock` (the concrete typed request is in scope in each method) so a batch member with Unlock:false never releases a lock it doesn't own. - SchedulerService: replace the unlocked per-source enqueue with a lock-once per-provider batch — LockX Collections() once, enqueue each source with Unlock:isLast (last message owns the release), compensating unlock in catch, and SKIP the whole provider loop if the lock is already held. A naive "lock-per-source, skip if held" would deterministically starve the 2nd+ source; lock-once-batch does not. Tests (ErsatzTV.Tests/Services/): ScannerServiceCollectionLockTests drives the real ScannerService read loop + real EntityLocker and asserts Unlock:false leaves a held lock intact while Unlock:true releases (all three providers); SchedulerServiceCollectionLockTests reflect-invokes ScanPlexMediaSources and asserts it locks once + skips the enqueue when held, and hands the release to the last message when acquired. Proven non-vacuous: reverting the Plex fix fails exactly the three Plex tests. No OpenAPI/v1.json change (internal channel-message record, not a DTO). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Commands/SynchronizeEmbyCollections.cs | 2 +- .../SynchronizeJellyfinCollections.cs | 2 +- .../Commands/SynchronizePlexCollections.cs | 2 +- .../ScannerServiceCollectionLockTests.cs | 160 ++++++++++++++++++ .../SchedulerServiceCollectionLockTests.cs | 150 ++++++++++++++++ ErsatzTV/Services/ScannerService.cs | 14 +- ErsatzTV/Services/SchedulerService.cs | 76 +++++++-- 7 files changed, 388 insertions(+), 18 deletions(-) create mode 100644 ErsatzTV.Tests/Services/ScannerServiceCollectionLockTests.cs create mode 100644 ErsatzTV.Tests/Services/SchedulerServiceCollectionLockTests.cs 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/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.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/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; + } } } -- 2.47.3 From 589c35d3573a9e720a8263578f396588843268af Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 18:41:33 +0200 Subject: [PATCH 8/8] fix(235): make QueueShowScanResult switch total (explicit NotFound + throwing fallback) Codex-review Low: NotFound previously reached the catch-all by coincidence; a future enum value would silently 404. Explicit arm + UnreachableException fallback so an unmapped outcome fails loudly rather than mis-mapping to 404. Co-Authored-By: Claude Opus 4.8 (1M context) --- ErsatzTV/Controllers/Api/LibrariesController.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ErsatzTV/Controllers/Api/LibrariesController.cs b/ErsatzTV/Controllers/Api/LibrariesController.cs index d75a11a5e..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; @@ -93,7 +94,8 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe Title = "Unable to scan show", Detail = $"The scan for show {request.ShowId} in library {id} could not be completed." }), - _ => ApiResults.NotFoundProblem($"Library {id} does not exist.") + QueueShowScanResult.NotFound => ApiResults.NotFoundProblem($"Library {id} does not exist."), + _ => throw new UnreachableException($"Unmapped QueueShowScanResult: {result}") }; } -- 2.47.3