diff --git a/ErsatzTV.Application/Libraries/Commands/CreateLocalLibraryHandler.cs b/ErsatzTV.Application/Libraries/Commands/CreateLocalLibraryHandler.cs index 4a575af29..7dc41a2c1 100644 --- a/ErsatzTV.Application/Libraries/Commands/CreateLocalLibraryHandler.cs +++ b/ErsatzTV.Application/Libraries/Commands/CreateLocalLibraryHandler.cs @@ -44,7 +44,17 @@ public class CreateLocalLibraryHandler : LocalLibraryHandlerBase, if (_entityLocker.LockLibrary(localLibrary.Id)) { - await _scannerWorkerChannel.WriteAsync(new ForceScanLocalLibrary(localLibrary.Id)); + try + { + await _scannerWorkerChannel.WriteAsync(new ForceScanLocalLibrary(localLibrary.Id)); + } + catch + { + // the scanner only unlocks when it receives the message; if the enqueue fails + // after we acquired the lock, release it here or it is held forever. + _entityLocker.UnlockLibrary(localLibrary.Id); + throw; + } } return ProjectToViewModel(localLibrary); diff --git a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs index b01cf6db3..024b47be3 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryId.cs @@ -1,3 +1,11 @@ namespace ErsatzTV.Application.Libraries; -public record QueueLibraryScanByLibraryId(int LibraryId) : IRequest; +public enum QueueLibraryScanResult +{ + Queued, + NotFound, + SyncDisabled, + AlreadyScanning +} + +public record QueueLibraryScanByLibraryId(int LibraryId) : IRequest; diff --git a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs index 2b62826c7..6bb6a5f4b 100644 --- a/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs +++ b/ErsatzTV.Application/Libraries/Commands/QueueLibraryScanByLibraryIdHandler.cs @@ -17,9 +17,11 @@ public class QueueLibraryScanByLibraryIdHandler( IEntityLocker locker, ChannelWriter scannerWorker, ILogger logger) - : IRequestHandler + : IRequestHandler { - public async Task Handle(QueueLibraryScanByLibraryId request, CancellationToken cancellationToken) + public async Task Handle( + QueueLibraryScanByLibraryId request, + CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); @@ -40,10 +42,17 @@ public class QueueLibraryScanByLibraryIdHandler( if (!shouldSyncItems) { logger.LogWarning("Library sync is disabled for library id {Id}", library.Id); - return false; + return QueueLibraryScanResult.SyncDisabled; } - if (locker.LockLibrary(library.Id)) + // A true from LockLibrary confers ownership of exactly one release; a false means a scan + // is already in progress and we own no release. + if (!locker.LockLibrary(library.Id)) + { + return QueueLibraryScanResult.AlreadyScanning; + } + + try { logger.LogDebug("Queued library scan for library id {Id}", library.Id); @@ -78,10 +87,18 @@ public class QueueLibraryScanByLibraryIdHandler( break; } } + catch + { + // the scanner only unlocks when it receives the message; if enqueueing fails + // (e.g. request aborted / channel completed) after we acquired the lock, release + // it here or it is held forever (EnqueueWithTraktLock pattern). + locker.UnlockLibrary(library.Id); + throw; + } - return true; + return QueueLibraryScanResult.Queued; } - return false; + return QueueLibraryScanResult.NotFound; } } diff --git a/ErsatzTV.Application/Libraries/Commands/UpdateLocalLibraryHandler.cs b/ErsatzTV.Application/Libraries/Commands/UpdateLocalLibraryHandler.cs index 7914d667e..b20728c12 100644 --- a/ErsatzTV.Application/Libraries/Commands/UpdateLocalLibraryHandler.cs +++ b/ErsatzTV.Application/Libraries/Commands/UpdateLocalLibraryHandler.cs @@ -96,7 +96,17 @@ public class UpdateLocalLibraryHandler : LocalLibraryHandlerBase, if (_entityLocker.LockLibrary(existing.Id)) { - await _scannerWorkerChannel.WriteAsync(new ForceScanLocalLibrary(existing.Id)); + try + { + await _scannerWorkerChannel.WriteAsync(new ForceScanLocalLibrary(existing.Id)); + } + catch + { + // the scanner only unlocks when it receives the message; if the enqueue fails + // after we acquired the lock, release it here or it is held forever. + _entityLocker.UnlockLibrary(existing.Id); + throw; + } } } diff --git a/ErsatzTV.Application/MediaCollections/Commands/UpdateTraktListHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/UpdateTraktListHandler.cs index e7a486ce7..772d5ed62 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/UpdateTraktListHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/UpdateTraktListHandler.cs @@ -55,7 +55,17 @@ public class UpdateTraktListHandler( if (entityLocker.LockTrakt()) { - await workerChannel.WriteAsync(new MatchTraktListItems(traktList.Id), cancellationToken); + try + { + await workerChannel.WriteAsync(new MatchTraktListItems(traktList.Id), cancellationToken); + } + catch + { + // the background handler only unlocks when it receives the message; if the + // enqueue fails after we acquired the lock, release it here or it is held forever. + entityLocker.UnlockTrakt(); + throw; + } } } else if (traktList.PlaylistId is not null) diff --git a/ErsatzTV.Application/Plex/Commands/SynchronizePlexLibraryById.cs b/ErsatzTV.Application/Plex/Commands/SynchronizePlexLibraryById.cs index c9b704f36..67e21b5d9 100644 --- a/ErsatzTV.Application/Plex/Commands/SynchronizePlexLibraryById.cs +++ b/ErsatzTV.Application/Plex/Commands/SynchronizePlexLibraryById.cs @@ -7,15 +7,20 @@ public interface ISynchronizePlexLibraryById : IRequest false; public bool DeepScan => false; } -public record ForceSynchronizePlexLibraryById(int PlexLibraryId, bool DeepScan) : ISynchronizePlexLibraryById +public record ForceSynchronizePlexLibraryById(int PlexLibraryId, bool DeepScan, bool Unlock = true) + : ISynchronizePlexLibraryById { public bool ForceScan => true; } diff --git a/ErsatzTV.Application/Plex/Commands/SynchronizePlexNetworks.cs b/ErsatzTV.Application/Plex/Commands/SynchronizePlexNetworks.cs index f0730c1e1..ac2d150cc 100644 --- a/ErsatzTV.Application/Plex/Commands/SynchronizePlexNetworks.cs +++ b/ErsatzTV.Application/Plex/Commands/SynchronizePlexNetworks.cs @@ -2,5 +2,9 @@ using ErsatzTV.Core; namespace ErsatzTV.Application.Plex; -public record SynchronizePlexNetworks(int PlexLibraryId, bool ForceScan) : IRequest>, - IScannerBackgroundServiceRequest; +// Unlock defaults true so single-message callers release the library lock as before. In the Plex +// "Shows" scheduler batch this message runs LAST and carries the single release (the preceding +// library message runs with Unlock: false). +public record SynchronizePlexNetworks(int PlexLibraryId, bool ForceScan, bool Unlock = true) + : IRequest>, + IScannerBackgroundServiceRequest; diff --git a/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs b/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs new file mode 100644 index 000000000..c25cde87b --- /dev/null +++ b/ErsatzTV.Tests/Application/Libraries/QueueLibraryScanByLibraryIdHandlerTests.cs @@ -0,0 +1,224 @@ +using System.Threading.Channels; +using ErsatzTV.Application; +using ErsatzTV.Application.Libraries; +using ErsatzTV.Application.MediaSources; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using ThreadingChannel = System.Threading.Channels.Channel; + +namespace ErsatzTV.Tests.Application.Libraries; + +[TestFixture] +public class QueueLibraryScanByLibraryIdHandlerTests +{ + 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(); + Channel channel = ThreadingChannel.CreateUnbounded(); + + QueueLibraryScanByLibraryIdHandler handler = CreateHandler(locker, channel.Writer); + + QueueLibraryScanResult result = await handler.Handle( + new QueueLibraryScanByLibraryId(9999), + CancellationToken.None); + + result.ShouldBe(QueueLibraryScanResult.NotFound); + locker.DidNotReceive().LockLibrary(Arg.Any()); + } + + [Test] + public async Task Handle_Should_Return_SyncDisabled_When_Item_Sync_Off() + { + int libraryId = await SeedSyncDisabledPlexLibrary(); + + var locker = Substitute.For(); + Channel channel = ThreadingChannel.CreateUnbounded(); + + QueueLibraryScanByLibraryIdHandler handler = CreateHandler(locker, channel.Writer); + + QueueLibraryScanResult result = await handler.Handle( + new QueueLibraryScanByLibraryId(libraryId), + CancellationToken.None); + + result.ShouldBe(QueueLibraryScanResult.SyncDisabled); + locker.DidNotReceive().LockLibrary(Arg.Any()); + } + + [Test] + public async Task Handle_Should_Queue_And_Enqueue_When_Lock_Acquired() + { + int libraryId = await SeedLocalLibrary(); + + 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), + CancellationToken.None); + + result.ShouldBe(QueueLibraryScanResult.Queued); + locker.Received(1).LockLibrary(libraryId); + locker.DidNotReceive().UnlockLibrary(Arg.Any()); + channel.Reader.TryRead(out IScannerBackgroundServiceRequest? request).ShouldBeTrue(); + request.ShouldBeOfType(); + } + + [Test] + public async Task Handle_Should_Return_AlreadyScanning_When_Lock_Not_Acquired() + { + int libraryId = await SeedLocalLibrary(); + + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(false); + Channel channel = ThreadingChannel.CreateUnbounded(); + + QueueLibraryScanByLibraryIdHandler handler = CreateHandler(locker, channel.Writer); + + QueueLibraryScanResult result = await handler.Handle( + new QueueLibraryScanByLibraryId(libraryId), + CancellationToken.None); + + result.ShouldBe(QueueLibraryScanResult.AlreadyScanning); + channel.Reader.TryRead(out _).ShouldBeFalse(); + locker.DidNotReceive().UnlockLibrary(Arg.Any()); + } + + [Test] + public async Task Two_Concurrent_Scans_Should_Be_Queued_Then_AlreadyScanning() + { + int libraryId = await SeedLocalLibrary(); + + var locker = Substitute.For(); + // first acquire wins, second loses (a scan is already in progress) + locker.LockLibrary(libraryId).Returns(true, false); + Channel channel = ThreadingChannel.CreateUnbounded(); + + QueueLibraryScanByLibraryIdHandler handler = CreateHandler(locker, channel.Writer); + + QueueLibraryScanResult first = await handler.Handle( + new QueueLibraryScanByLibraryId(libraryId), + CancellationToken.None); + QueueLibraryScanResult second = await handler.Handle( + new QueueLibraryScanByLibraryId(libraryId), + CancellationToken.None); + + first.ShouldBe(QueueLibraryScanResult.Queued); + second.ShouldBe(QueueLibraryScanResult.AlreadyScanning); + } + + [Test] + public async Task Handle_Should_Unlock_And_Rethrow_When_Enqueue_Throws() + { + int libraryId = await SeedLocalLibrary(); + + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + Channel channel = ThreadingChannel.CreateUnbounded(); + // a completed writer makes WriteAsync throw ChannelClosedException, simulating a failed enqueue + channel.Writer.Complete(); + + QueueLibraryScanByLibraryIdHandler handler = CreateHandler(locker, channel.Writer); + + await Should.ThrowAsync(() => + handler.Handle(new QueueLibraryScanByLibraryId(libraryId), CancellationToken.None)); + + locker.Received(1).UnlockLibrary(libraryId); + } + + [Test] + public async Task Handle_Should_Unlock_When_Enqueue_Canceled() + { + int libraryId = await SeedLocalLibrary(); + + var locker = Substitute.For(); + locker.LockLibrary(libraryId).Returns(true); + + // a writer whose enqueue is aborted (e.g. request cancelled after the lock was taken) + var writer = Substitute.For>(); + writer.WriteAsync(Arg.Any(), Arg.Any()) + .Returns(ValueTask.FromException(new OperationCanceledException())); + + QueueLibraryScanByLibraryIdHandler handler = CreateHandler(locker, writer); + + await Should.ThrowAsync(() => + handler.Handle(new QueueLibraryScanByLibraryId(libraryId), CancellationToken.None)); + + locker.Received(1).UnlockLibrary(libraryId); + } + + private QueueLibraryScanByLibraryIdHandler CreateHandler( + IEntityLocker locker, + ChannelWriter writer) => + new( + _db.Factory, + locker, + 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 SeedSyncDisabledPlexLibrary() + { + 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 Movies", + MediaKind = LibraryMediaKind.Movies, + Key = "1", + ShouldSyncItems = false, + Paths = [] + } + ] + }; + await context.PlexMediaSources.AddAsync(source); + await context.SaveChangesAsync(); + return source.Libraries[0].Id; + } +} diff --git a/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs b/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs index 0971c747c..95ebbdf6b 100644 --- a/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs @@ -95,4 +95,54 @@ public class LibrariesControllerTests result.ShouldBeOfType(); } + + [Test] + public async Task ScanLibrary_Should_Return_202_When_Queued() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueLibraryScanResult.Queued); + + IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send( + Arg.Is(r => r.LibraryId == 7), + Arg.Any()); + } + + [Test] + public async Task ScanLibrary_Should_Return_404_When_NotFound() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueLibraryScanResult.NotFound); + + IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + + var notFound = result.ShouldBeOfType(); + notFound.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status404NotFound); + } + + [Test] + public async Task ScanLibrary_Should_Return_409_When_AlreadyScanning() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueLibraryScanResult.AlreadyScanning); + + IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + + var conflict = result.ShouldBeOfType(); + conflict.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status409Conflict); + } + + [Test] + public async Task ScanLibrary_Should_Return_422_When_SyncDisabled() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(QueueLibraryScanResult.SyncDisabled); + + IActionResult result = await _controller.ScanLibrary(7, CancellationToken.None); + + var unprocessable = result.ShouldBeOfType(); + unprocessable.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status422UnprocessableEntity); + } } diff --git a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs index 58b7358e1..f4de94901 100644 --- a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs +++ b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs @@ -271,6 +271,9 @@ public class OpenApiErrorResponseContractTests [TestCase("/api/trakt/lists/{id}", "put", "422")] [TestCase("/api/troubleshoot/playback/subtitles/{mediaItemId}", "get", "404")] [TestCase("/api/libraries/{id}/scan-show", "post", "404")] + [TestCase("/api/libraries/{id}/scan", "post", "404")] + [TestCase("/api/libraries/{id}/scan", "post", "409")] + [TestCase("/api/libraries/{id}/scan", "post", "422")] public void Static_OpenApi_Should_Document_ProblemDetails_For_Api_Error_Responses( string path, string method, diff --git a/ErsatzTV/Controllers/Api/LibrariesController.cs b/ErsatzTV/Controllers/Api/LibrariesController.cs index 844a73859..d8525a52d 100644 --- a/ErsatzTV/Controllers/Api/LibrariesController.cs +++ b/ErsatzTV/Controllers/Api/LibrariesController.cs @@ -22,10 +22,29 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe [HttpPost("/api/libraries/{id:int}/scan")] [Tags("Libraries")] [EndpointSummary("Scan library")] - public async Task ScanLibrary(int id) => - await mediator.Send(new QueueLibraryScanByLibraryId(id)) - ? new OkResult() - : new NotFoundResult(); + [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) + { + QueueLibraryScanResult result = await mediator.Send(new QueueLibraryScanByLibraryId(id), cancellationToken); + return result switch + { + QueueLibraryScanResult.Queued => new AcceptedResult(), + QueueLibraryScanResult.AlreadyScanning => ApiResults.ConflictProblem( + "Library scan in progress", + $"A scan for library {id} is already in progress."), + QueueLibraryScanResult.SyncDisabled => new UnprocessableEntityObjectResult( + new ProblemDetails + { + Status = StatusCodes.Status422UnprocessableEntity, + Title = "Library sync is disabled", + Detail = $"Item sync is disabled for library {id}." + }), + _ => ApiResults.NotFoundProblem($"Library {id} does not exist.") + }; + } [HttpPost("/api/libraries/{id:int}/scan-show")] [Tags("Libraries")] diff --git a/ErsatzTV/Services/ScannerService.cs b/ErsatzTV/Services/ScannerService.cs index a61ea5bd4..cafd5380b 100644 --- a/ErsatzTV/Services/ScannerService.cs +++ b/ErsatzTV/Services/ScannerService.cs @@ -104,31 +104,36 @@ public class ScannerService : BackgroundService IMediator mediator = scope.ServiceProvider.GetRequiredService(); IEntityLocker entityLocker = scope.ServiceProvider.GetRequiredService(); - Either scanResult = await mediator.Send(request, cancellationToken); - scanResult.BiIter( - name => _logger.LogDebug( - "Done scanning local library {Library}", - name), - error => - { - if (error is ScanIsNotRequired) - { - _logger.LogDebug( - "Scan is not required for local library {LibraryId} at this time", - request.LibraryId); - } - else - { - _logger.LogWarning( - "Unable to scan local library {LibraryId}: {Error}", - request.LibraryId, - error.Value); - } - }); - - if (entityLocker.IsLibraryLocked(request.LibraryId)) + try { - entityLocker.UnlockLibrary(request.LibraryId); + Either scanResult = await mediator.Send(request, cancellationToken); + scanResult.BiIter( + name => _logger.LogDebug( + "Done scanning local library {Library}", + name), + error => + { + if (error is ScanIsNotRequired) + { + _logger.LogDebug( + "Scan is not required for local library {LibraryId} at this time", + request.LibraryId); + } + else + { + _logger.LogWarning( + "Unable to scan local library {LibraryId}: {Error}", + request.LibraryId, + error.Value); + } + }); + } + finally + { + if (entityLocker.IsLibraryLocked(request.LibraryId)) + { + entityLocker.UnlockLibrary(request.LibraryId); + } } } @@ -156,29 +161,36 @@ public class ScannerService : BackgroundService IMediator mediator = scope.ServiceProvider.GetRequiredService(); IEntityLocker entityLocker = scope.ServiceProvider.GetRequiredService(); - Either result = await mediator.Send(request, cancellationToken); - result.BiIter( - name => _logger.LogDebug("Done synchronizing plex library {Name}", name), - error => - { - if (error is ScanIsNotRequired) - { - _logger.LogDebug( - "Scan is not required for plex library {LibraryId} at this time", - request.PlexLibraryId); - } - else - { - _logger.LogWarning( - "Unable to synchronize plex library {LibraryId}: {Error}", - request.PlexLibraryId, - error.Value); - } - }); - - if (entityLocker.IsLibraryLocked(request.PlexLibraryId)) + try { - entityLocker.UnlockLibrary(request.PlexLibraryId); + Either result = await mediator.Send(request, cancellationToken); + result.BiIter( + name => _logger.LogDebug("Done synchronizing plex library {Name}", name), + error => + { + if (error is ScanIsNotRequired) + { + _logger.LogDebug( + "Scan is not required for plex library {LibraryId} at this time", + request.PlexLibraryId); + } + else + { + _logger.LogWarning( + "Unable to synchronize plex library {LibraryId}: {Error}", + request.PlexLibraryId, + error.Value); + } + }); + } + finally + { + // request.Unlock is false when a later message in the same batch (SynchronizePlexNetworks) + // owns the single release for this library lock. + if (request.Unlock && entityLocker.IsLibraryLocked(request.PlexLibraryId)) + { + entityLocker.UnlockLibrary(request.PlexLibraryId); + } } } @@ -190,24 +202,29 @@ public class ScannerService : BackgroundService IMediator mediator = scope.ServiceProvider.GetRequiredService(); IEntityLocker entityLocker = scope.ServiceProvider.GetRequiredService(); - Either result = await mediator.Send(request, cancellationToken); - result.BiIter( - _ => _logger.LogDebug("Done synchronizing plex collections"), - error => - { - if (error is ScanIsNotRequired) - { - _logger.LogDebug("Scan is not required for plex collections at this time"); - } - else - { - _logger.LogWarning("Unable to synchronize plex collections: {Error}", error.Value); - } - }); - - if (entityLocker.ArePlexCollectionsLocked()) + try { - entityLocker.UnlockPlexCollections(); + Either result = await mediator.Send(request, cancellationToken); + result.BiIter( + _ => _logger.LogDebug("Done synchronizing plex collections"), + error => + { + if (error is ScanIsNotRequired) + { + _logger.LogDebug("Scan is not required for plex collections at this time"); + } + else + { + _logger.LogWarning("Unable to synchronize plex collections: {Error}", error.Value); + } + }); + } + finally + { + if (entityLocker.ArePlexCollectionsLocked()) + { + entityLocker.UnlockPlexCollections(); + } } } @@ -219,29 +236,36 @@ public class ScannerService : BackgroundService IMediator mediator = scope.ServiceProvider.GetRequiredService(); IEntityLocker entityLocker = scope.ServiceProvider.GetRequiredService(); - Either result = await mediator.Send(request, cancellationToken); - result.BiIter( - _ => _logger.LogDebug("Done synchronizing plex networks for library {LibraryId}", request.PlexLibraryId), - error => - { - if (error is ScanIsNotRequired) - { - _logger.LogDebug( - "Scan is not required for plex networks in library {LibraryId} at this time", - request.PlexLibraryId); - } - else - { - _logger.LogWarning( - "Unable to synchronize plex networks for library {LibraryId}: {Error}", - request.PlexLibraryId, - error.Value); - } - }); - - if (entityLocker.IsLibraryLocked(request.PlexLibraryId)) + try { - entityLocker.UnlockLibrary(request.PlexLibraryId); + Either result = await mediator.Send(request, cancellationToken); + result.BiIter( + _ => _logger.LogDebug( + "Done synchronizing plex networks for library {LibraryId}", + request.PlexLibraryId), + error => + { + if (error is ScanIsNotRequired) + { + _logger.LogDebug( + "Scan is not required for plex networks in library {LibraryId} at this time", + request.PlexLibraryId); + } + else + { + _logger.LogWarning( + "Unable to synchronize plex networks for library {LibraryId}: {Error}", + request.PlexLibraryId, + error.Value); + } + }); + } + finally + { + if (request.Unlock && entityLocker.IsLibraryLocked(request.PlexLibraryId)) + { + entityLocker.UnlockLibrary(request.PlexLibraryId); + } } } @@ -269,29 +293,34 @@ public class ScannerService : BackgroundService IMediator mediator = scope.ServiceProvider.GetRequiredService(); IEntityLocker entityLocker = scope.ServiceProvider.GetRequiredService(); - Either result = await mediator.Send(request, cancellationToken); - result.BiIter( - name => _logger.LogDebug("Done synchronizing jellyfin library {Name}", name), - error => - { - if (error is ScanIsNotRequired) - { - _logger.LogDebug( - "Scan is not required for jellyfin library {LibraryId} at this time", - request.JellyfinLibraryId); - } - else - { - _logger.LogWarning( - "Unable to synchronize jellyfin library {LibraryId}: {Error}", - request.JellyfinLibraryId, - error.Value); - } - }); - - if (entityLocker.IsLibraryLocked(request.JellyfinLibraryId)) + try { - entityLocker.UnlockLibrary(request.JellyfinLibraryId); + Either result = await mediator.Send(request, cancellationToken); + result.BiIter( + name => _logger.LogDebug("Done synchronizing jellyfin library {Name}", name), + error => + { + if (error is ScanIsNotRequired) + { + _logger.LogDebug( + "Scan is not required for jellyfin library {LibraryId} at this time", + request.JellyfinLibraryId); + } + else + { + _logger.LogWarning( + "Unable to synchronize jellyfin library {LibraryId}: {Error}", + request.JellyfinLibraryId, + error.Value); + } + }); + } + finally + { + if (entityLocker.IsLibraryLocked(request.JellyfinLibraryId)) + { + entityLocker.UnlockLibrary(request.JellyfinLibraryId); + } } } @@ -303,24 +332,29 @@ public class ScannerService : BackgroundService IMediator mediator = scope.ServiceProvider.GetRequiredService(); IEntityLocker entityLocker = scope.ServiceProvider.GetRequiredService(); - Either result = await mediator.Send(request, cancellationToken); - result.BiIter( - _ => _logger.LogDebug("Done synchronizing jellyfin collections"), - error => - { - if (error is ScanIsNotRequired) - { - _logger.LogDebug("Scan is not required for jellyfin collections at this time"); - } - else - { - _logger.LogWarning("Unable to synchronize jellyfin collections: {Error}", error.Value); - } - }); - - if (entityLocker.AreJellyfinCollectionsLocked()) + try { - entityLocker.UnlockJellyfinCollections(); + Either result = await mediator.Send(request, cancellationToken); + result.BiIter( + _ => _logger.LogDebug("Done synchronizing jellyfin collections"), + error => + { + if (error is ScanIsNotRequired) + { + _logger.LogDebug("Scan is not required for jellyfin collections at this time"); + } + else + { + _logger.LogWarning("Unable to synchronize jellyfin collections: {Error}", error.Value); + } + }); + } + finally + { + if (entityLocker.AreJellyfinCollectionsLocked()) + { + entityLocker.UnlockJellyfinCollections(); + } } } @@ -346,29 +380,34 @@ public class ScannerService : BackgroundService IMediator mediator = scope.ServiceProvider.GetRequiredService(); IEntityLocker entityLocker = scope.ServiceProvider.GetRequiredService(); - Either result = await mediator.Send(request, cancellationToken); - result.BiIter( - name => _logger.LogDebug("Done synchronizing emby library {Name}", name), - error => - { - if (error is ScanIsNotRequired) - { - _logger.LogDebug( - "Scan is not required for emby library {LibraryId} at this time", - request.EmbyLibraryId); - } - else - { - _logger.LogWarning( - "Unable to synchronize emby library {LibraryId}: {Error}", - request.EmbyLibraryId, - error.Value); - } - }); - - if (entityLocker.IsLibraryLocked(request.EmbyLibraryId)) + try { - entityLocker.UnlockLibrary(request.EmbyLibraryId); + Either result = await mediator.Send(request, cancellationToken); + result.BiIter( + name => _logger.LogDebug("Done synchronizing emby library {Name}", name), + error => + { + if (error is ScanIsNotRequired) + { + _logger.LogDebug( + "Scan is not required for emby library {LibraryId} at this time", + request.EmbyLibraryId); + } + else + { + _logger.LogWarning( + "Unable to synchronize emby library {LibraryId}: {Error}", + request.EmbyLibraryId, + error.Value); + } + }); + } + finally + { + if (entityLocker.IsLibraryLocked(request.EmbyLibraryId)) + { + entityLocker.UnlockLibrary(request.EmbyLibraryId); + } } } @@ -380,24 +419,29 @@ public class ScannerService : BackgroundService IMediator mediator = scope.ServiceProvider.GetRequiredService(); IEntityLocker entityLocker = scope.ServiceProvider.GetRequiredService(); - Either result = await mediator.Send(request, cancellationToken); - result.BiIter( - _ => _logger.LogDebug("Done synchronizing emby collections"), - error => - { - if (error is ScanIsNotRequired) - { - _logger.LogDebug("Scan is not required for emby collections at this time"); - } - else - { - _logger.LogWarning("Unable to synchronize emby collections: {Error}", error.Value); - } - }); - - if (entityLocker.AreEmbyCollectionsLocked()) + try { - entityLocker.UnlockEmbyCollections(); + Either result = await mediator.Send(request, cancellationToken); + result.BiIter( + _ => _logger.LogDebug("Done synchronizing emby collections"), + error => + { + if (error is ScanIsNotRequired) + { + _logger.LogDebug("Scan is not required for emby collections at this time"); + } + else + { + _logger.LogWarning("Unable to synchronize emby collections: {Error}", error.Value); + } + }); + } + finally + { + if (entityLocker.AreEmbyCollectionsLocked()) + { + entityLocker.UnlockEmbyCollections(); + } } } } diff --git a/ErsatzTV/Services/SchedulerService.cs b/ErsatzTV/Services/SchedulerService.cs index 42e792c36..072c74deb 100644 --- a/ErsatzTV/Services/SchedulerService.cs +++ b/ErsatzTV/Services/SchedulerService.cs @@ -229,11 +229,15 @@ public class SchedulerService : BackgroundService if (_entityLocker.LockLibrary(library.Id)) { + // one LockLibrary ⇄ one release: when a networks scan follows, it (the last message) + // carries the release; the library message runs with Unlock: false. + bool networksFollow = library.MediaKind is LibraryMediaKind.Shows; + await _scannerWorkerChannel.WriteAsync( - new SynchronizePlexLibraryByIdIfNeeded(library.Id), + new SynchronizePlexLibraryByIdIfNeeded(library.Id, Unlock: !networksFollow), cancellationToken); - if (library.MediaKind is LibraryMediaKind.Shows) + if (networksFollow) { await _scannerWorkerChannel.WriteAsync( new SynchronizePlexNetworks(library.Id, false), diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index b2366655f..6657b3bf5 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -5416,8 +5416,68 @@ } ], "responses": { - "200": { - "description": "OK" + "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" + } + } + } + }, + "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" + } + } + } } } } diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 52375bffa..69f99086c 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -119,6 +119,22 @@ action. Precedent for the 409 shape: `TraktController` (its private `ConflictPro 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. +### 3b. Map a "queue a background job" outcome to status codes with an enum, not a `bool` + +When an endpoint *starts* a background operation guarded by an `IEntityLocker` lock, return an +**outcome enum from the handler** and map it in the controller — don't collapse distinct outcomes +into a lying `bool`/200. Exemplar (issue #232): `QueueLibraryScanByLibraryId` → +`QueueLibraryScanResult { Queued | NotFound | SyncDisabled | AlreadyScanning }`, mapped by +`LibrariesController.ScanLibrary` to **202** (`AcceptedResult`, queued), **404** +(`ApiResults.NotFoundProblem`), **422** (`UnprocessableEntityObjectResult` + `ProblemDetails`, a +domain precondition such as sync-disabled), and **409** (`ApiResults.ConflictProblem`, the lock is +already held = already scanning). Here the acquired lock **is** the running job, so +`LockLibrary(id) == false` means "already scanning" → 409 (a variant of §3a where the lock is the +operation itself, not a mutation racing it). Add `[ProducesResponseType]` for 202/404/409/422 and +`typeof(ProblemDetails)` on the error ones. **Guard the lock→enqueue** with the +`EnqueueWithTraktLock` compensating-unlock pattern (`TraktController`): if a `WriteAsync` throws +after a successful `Lock*`, `Unlock*` in a `catch` and rethrow — one lock ⇄ exactly one release. + `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/web/src/App.test.tsx b/web/src/App.test.tsx index e9d6289d4..0cf8e2809 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -1249,24 +1249,21 @@ describe('ChicoryTV SPA scaffold', () => { expect(clearCount).toBeGreaterThan(0); }); - it('re-enables the scan button and stops polling when a queued scan never appears (grace window expiry)', async () => { - const intervalHandlers: Array<() => void> = []; - let clearCount = 0; - vi.spyOn(window, 'setInterval').mockImplementation((handler: TimerHandler, timeout?: number) => { - if (typeof handler === 'function' && (timeout ?? 0) >= 10000) { - intervalHandlers.push(handler as () => void); - } - return intervalHandlers.length; - }); - vi.spyOn(window, 'clearInterval').mockImplementation(() => { - clearCount += 1; - }); - + it('reconciles a 409 "already scanning" against scan-status without an error toast (#232)', async () => { + // POST /scan now returns 409 when a scan is already in progress. That is benign - the SPA + // must NOT show an error; it reconciles against scan-status (which reports the active scan) + // and keeps the button disabled while that scan runs. mockDashboardApi({ - libraryScanStatuses: [], + libraryScanStatusSequence: [ + [], // initial screen load: nothing active yet + [{ libraryId: 31, percent: 0.3 }] // post-409 reconcile fetch: the in-progress scan + ], mediaSources: [ mediaSource({ libraries: [library({ id: 31, itemCount: 10, mediaKind: 'Movies', name: 'Movies' })] }) - ] + ], + mutationFailures: { + '/api/libraries/31/scan': { status: 409 } + } }); render(); @@ -1279,42 +1276,27 @@ describe('ChicoryTV SPA scaffold', () => { await waitFor(() => { expect(window.fetch).toHaveBeenCalledWith('/api/libraries/31/scan', expect.objectContaining({ method: 'POST' })); }); - await waitFor(() => { - expect(fetchCount('/api/libraries/scan-status')).toBe(2); - }); - expect(screen.getByRole('button', { name: 'Scan Movies' })).toBeDisabled(); - expect(intervalHandlers.length).toBeGreaterThan(0); - // The id never shows up in scan-status. It survives a couple of ticks... - await runPollTick(intervalHandlers); - expect(fetchCount('/api/libraries/scan-status')).toBe(3); + // The reconcile fetch surfaces the already-running scan; the button stays disabled... + expect(await screen.findByText('30%')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Scan Movies' })).toBeDisabled(); - // ...but expires once the grace window runs out, freeing the button and the poll. - await runPollTick(intervalHandlers); - expect(screen.getByRole('button', { name: 'Scan Movies' })).not.toBeDisabled(); - expect(clearCount).toBeGreaterThan(0); + // ...and no error toast is shown for the benign conflict. + expect(screen.queryByText(/Request failed/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Unable to scan/)).not.toBeInTheDocument(); }); - it('re-enables the scan button and stops polling when scan-status errors repeatedly after a trigger (grace window on error)', async () => { - const intervalHandlers: Array<() => void> = []; - let clearCount = 0; - vi.spyOn(window, 'setInterval').mockImplementation((handler: TimerHandler, timeout?: number) => { - if (typeof handler === 'function' && (timeout ?? 0) >= 10000) { - intervalHandlers.push(handler as () => void); - } - return intervalHandlers.length; - }); - vi.spyOn(window, 'clearInterval').mockImplementation(() => { - clearCount += 1; - }); - + it('surfaces a 422 "sync disabled" scan rejection and leaves the button enabled (#232)', async () => { + // POST /scan now returns 422 when item sync is disabled for the library. Nothing was queued, + // so the button must not be stuck disabled and the error must be surfaced. mockDashboardApi({ libraryScanStatuses: [], - libraryScanStatusFailAfterTrigger: true, mediaSources: [ mediaSource({ libraries: [library({ id: 31, itemCount: 10, mediaKind: 'Movies', name: 'Movies' })] }) - ] + ], + mutationFailures: { + '/api/libraries/31/scan': { detail: 'Item sync is disabled for library 31.', status: 422 } + } }); render(); @@ -1324,29 +1306,8 @@ describe('ChicoryTV SPA scaffold', () => { fireEvent.click(screen.getByRole('button', { name: 'Scan Movies' })); - await waitFor(() => { - expect(window.fetch).toHaveBeenCalledWith('/api/libraries/31/scan', expect.objectContaining({ method: 'POST' })); - }); - - // The immediate post-trigger status fetch errors (first grace-tick burn), but the - // button must stay disabled and the poll must stay armed - a transient failure must - // not be indistinguishable from "give up immediately". - await waitFor(() => { - expect(fetchCount('/api/libraries/scan-status')).toBe(2); - }); - expect(screen.getByRole('button', { name: 'Scan Movies' })).toBeDisabled(); - expect(intervalHandlers.length).toBeGreaterThan(0); - - // scan-status keeps erroring on every poll tick... - await runPollTick(intervalHandlers); - expect(fetchCount('/api/libraries/scan-status')).toBe(3); - expect(screen.getByRole('button', { name: 'Scan Movies' })).toBeDisabled(); - - // ...but the same grace budget burns down on failures too, so persistent failure - // eventually frees the button and stops the interval instead of polling forever. - await runPollTick(intervalHandlers); + expect(await screen.findByText('Item sync is disabled for library 31.')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Scan Movies' })).not.toBeDisabled(); - expect(clearCount).toBeGreaterThan(0); }); it('shows Libraries API errors and retries', async () => { @@ -3910,7 +3871,8 @@ function mockDashboardApi({ scanStatusShouldFail = true; } - return Promise.resolve(new Response(null, { status: 200 })); + // Backend contract (#232): a successful queue is 202 Accepted, not a lying 200. + return Promise.resolve(new Response(null, { status: 202 })); } if (path === '/api/playouts') { diff --git a/web/src/api/libraries.ts b/web/src/api/libraries.ts index b2b1e0d01..c771f1352 100644 --- a/web/src/api/libraries.ts +++ b/web/src/api/libraries.ts @@ -28,11 +28,6 @@ type LibrariesScreenState = | { data: null; error: string; status: 'error' } | { data: null; error: null; status: 'loading' }; -// A trigger'd scan disappears from the button-disabled set once either: it has been -// observed at least once in scan-status (promoted to "active"), or this many poll -// ticks pass without ever appearing (the scanner never picked it up / it failed silently). -const PENDING_GRACE_TICKS = 3; - export function getMediaSources(): Promise { return request('/api/media-sources'); } @@ -66,35 +61,17 @@ export function scanShow(libraryId: number, params: ScanShowParams): Promise, - graceTicks: Map, - isSeenActive: (libraryId: number) => boolean -): Set { +// Drops a pending id that has been observed active in scan-status; scan-status is authoritative +// from that point. POST /scan now returns 202 only for a real (forced) scan, so every pending id +// eventually appears in scan-status - no "give up after N polls" grace heuristic is needed +// (the POST tells the client directly when nothing was queued: 409/404/422 instead of a lying 200). +function prunePromotedPending(pendingIds: Set, isSeenActive: (libraryId: number) => boolean): Set { const nextPending = new Set(); pendingIds.forEach((libraryId) => { - if (isSeenActive(libraryId)) { - // Seen active at least once - normal active/inactive pruning takes over. - graceTicks.delete(libraryId); - return; + if (!isSeenActive(libraryId)) { + nextPending.add(libraryId); } - - const ticksRemaining = (graceTicks.get(libraryId) ?? PENDING_GRACE_TICKS) - 1; - - if (ticksRemaining <= 0) { - // Grace window expired without ever appearing in scan-status - give up on it. - graceTicks.delete(libraryId); - return; - } - - graceTicks.set(libraryId, ticksRemaining); - nextPending.add(libraryId); }); return nextPending; @@ -112,7 +89,6 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta // guard against double-submits without waiting for a render. const pendingIdsRef = useRef>(new Set()); const activeIdsRef = useRef>(new Set()); - const pendingGraceTicksRef = useRef>(new Map()); useEffect(() => { activeRef.current = true; @@ -160,13 +136,10 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta const hadScanInProgress = hadScanInProgressRef.current; hadScanInProgressRef.current = scanStatuses.length > 0; - // Compute the pruned/promoted pending set - and write the grace-tick and pending - // refs - OUTSIDE the setState updater. Updaters must be pure: StrictMode double- - // invokes them (which would double-decrement grace ticks) and concurrent rendering - // may invoke-and-discard one. Mirrors how activeIdsRef is written above. - const nextPending = pruneGraceExpiredPending(pendingIdsRef.current, pendingGraceTicksRef.current, (libraryId) => - activeScanIds.has(libraryId) - ); + // Compute the promoted pending set - and write the pending ref - OUTSIDE the setState + // updater. Updaters must be pure (StrictMode double-invokes them and concurrent rendering + // may invoke-and-discard one). Mirrors how activeIdsRef is written above. + const nextPending = prunePromotedPending(pendingIdsRef.current, (libraryId) => activeScanIds.has(libraryId)); pendingIdsRef.current = nextPending; @@ -188,32 +161,10 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta } }) .catch(() => { - if (!activeRef.current) { - return; - } - - // A status-fetch failure never promotes a pending id to active, so it never - // survives via "seen active" - it just burns down the same grace budget as a - // successful poll that never saw it. This keeps a persistently-erroring endpoint - // from leaving the scan button disabled and the poll interval running forever. - // Active-scan state (activeIdsRef / scanStatuses) is left untouched: a transient - // failure must not kill an in-progress scan's UI. - const nextPending = pruneGraceExpiredPending(pendingIdsRef.current, pendingGraceTicksRef.current, () => false); - - pendingIdsRef.current = nextPending; - - setState((current) => { - if (current.status !== 'success') { - return current; - } - - return { - data: current.data, - error: current.error, - pendingLibraryIds: nextPending, - status: 'success' - }; - }); + // A transient status-fetch failure is left to resolve on the next poll: pending and + // active-scan state are untouched so an in-progress scan's UI is not disturbed. A 202 + // is only returned for a real scan, so pending ids reconcile against scan-status on the + // next successful poll rather than needing a give-up budget here. }); }, [loadSources]); @@ -227,7 +178,6 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta hadScanInProgressRef.current = scanStatuses.length > 0; pendingIdsRef.current = new Set(); activeIdsRef.current = new Set(scanStatuses.map((scan) => scan.libraryId)); - pendingGraceTicksRef.current = new Map(); setState({ data: { scanStatuses, sources }, error: null, @@ -266,14 +216,40 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta load(); }, [load]); + const clearPending = useCallback((libraryId: number) => { + const pendingIds = new Set(pendingIdsRef.current); + pendingIds.delete(libraryId); + pendingIdsRef.current = pendingIds; + + if (!activeRef.current) { + return; + } + + setState((current) => { + if (current.status !== 'success') { + return current; + } + + const pendingLibraryIds = new Set(current.pendingLibraryIds); + pendingLibraryIds.delete(libraryId); + + return { + data: current.data, + error: current.error, + pendingLibraryIds, + status: 'success' + }; + }); + }, []); + const triggerScan = useCallback((libraryId: number): Promise => { if (pendingIdsRef.current.has(libraryId) || activeIdsRef.current.has(libraryId)) { // Already pending or active - ignore the duplicate submission. return Promise.resolve(); } + // Optimistically mark pending; the POST response tells us how to reconcile. pendingIdsRef.current = new Set(pendingIdsRef.current).add(libraryId); - pendingGraceTicksRef.current.set(libraryId, PENDING_GRACE_TICKS); setState((current) => { if (current.status !== 'success') { @@ -289,12 +265,23 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta }); return scanLibrary(libraryId) - .then(() => loadScanStatuses()) + .then(() => { + // 202 Accepted - a scan is genuinely queued. Poll scan-status; the pending flag is + // promoted to "active" once the scan appears there. + void loadScanStatuses(); + }) .catch((error: unknown) => { - const pendingIds = new Set(pendingIdsRef.current); - pendingIds.delete(libraryId); - pendingIdsRef.current = pendingIds; - pendingGraceTicksRef.current.delete(libraryId); + if (error instanceof ApiError && error.status === 409) { + // 409 Conflict - the library is already scanning. Benign: drop the optimistic pending + // and reconcile against scan-status (an active scan keeps the button disabled). No toast. + clearPending(libraryId); + void loadScanStatuses(); + return; + } + + // 404 (no such library) / 422 (sync disabled) / network error - nothing was queued, so + // don't leave the button disabled; surface the error. + clearPending(libraryId); if (!activeRef.current) { return; @@ -305,18 +292,15 @@ export function useLibrariesScreenQuery(pollMs = 10000): LibrariesScreenQuerySta return current; } - const pendingLibraryIds = new Set(current.pendingLibraryIds); - pendingLibraryIds.delete(libraryId); - return { data: current.data, error: messageFromLibrariesError(error, 'Unable to scan library'), - pendingLibraryIds, + pendingLibraryIds: current.pendingLibraryIds, status: 'success' }; }); }); - }, [loadScanStatuses]); + }, [clearPending, loadScanStatuses]); if (state.status === 'success') { const activeLibraryIds = new Set(state.data.scanStatuses.map((scan) => scan.libraryId));