fix(api): resolve per-show scan by exact show id, not substring title (fixes #219)
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 7m11s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 12s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m49s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 7m11s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 12s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 9m49s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
POST /api/libraries/{id}/scan-show resolved the target show via
GetShowIdByTitle, an EF.Functions.Like "%title%" substring match with
no OrderBy - non-deterministic under duplicate/overlapping titles and
capable of scanning the wrong show. The Blazor UI never had this bug
(it always passed the exact show id); this endpoint shipped days ago
in PR #216 with no external consumers, so the contract break is safe.
BREAKING CHANGE: ScanShowRequest now takes `showId: int` instead of
`showTitle: string`. Replaced ITelevisionRepository.GetShowIdByTitle
with GetShowTitle(libraryId, showId), which also enforces the show
belongs to the given library. LibrariesController.ScanShow now returns
a genuine 404 ProblemDetails (via ApiResults.NotFoundProblem, the
established pre-check pattern from TemplateController.DeleteGroup)
when the show id doesn't exist in that library, then queues
QueueShowScanByLibraryId with the DB-resolved title.
SPA: libraries.ts ScanShowParams.showId replaces showTitle;
MediaDetailScreen.tsx passes show.id. Extended
ApiErrorResponseMetadataTests and OpenApiErrorResponseContractTests
with the new 404 contract for ScanShow. Regenerated v1.json / v1.d.ts
via scripts/update-openapi.sh + npm run generate:api.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -14,7 +14,7 @@ public class FakeTelevisionRepository : ITelevisionRepository
|
||||
public Task<List<Show>> GetAllShows() => throw new NotSupportedException();
|
||||
|
||||
public Task<Option<Show>> GetShow(int showId, CancellationToken cancellationToken) => throw new NotSupportedException();
|
||||
public Task<Option<int>> GetShowIdByTitle(int libraryId, string title) => throw new NotSupportedException();
|
||||
public Task<Option<string>> GetShowTitle(int libraryId, int showId) => throw new NotSupportedException();
|
||||
|
||||
public Task<List<Episode>> GetShowItems(int showId) => throw new NotSupportedException();
|
||||
public Task<List<int>> GetEpisodeIdsForShow(int showId) => throw new NotSupportedException();
|
||||
|
||||
@@ -10,7 +10,7 @@ public interface ITelevisionRepository
|
||||
Task<bool> AllEpisodesExist(List<int> episodeIds);
|
||||
Task<List<Show>> GetAllShows();
|
||||
Task<Option<Show>> GetShow(int showId, CancellationToken cancellationToken);
|
||||
Task<Option<int>> GetShowIdByTitle(int libraryId, string title);
|
||||
Task<Option<string>> GetShowTitle(int libraryId, int showId);
|
||||
Task<List<Episode>> GetShowItems(int showId);
|
||||
Task<List<int>> GetEpisodeIdsForShow(int showId);
|
||||
Task<List<Season>> GetAllSeasons();
|
||||
|
||||
@@ -80,16 +80,16 @@ public class TelevisionRepository : ITelevisionRepository
|
||||
.SelectOneAsync(s => s.Id, s => s.Id == showId, cancellationToken);
|
||||
}
|
||||
|
||||
public async Task<Option<int>> GetShowIdByTitle(int libraryId, string title)
|
||||
public async Task<Option<string>> GetShowTitle(int libraryId, int showId)
|
||||
{
|
||||
await using TvContext dbContext = await _dbContextFactory.CreateDbContextAsync();
|
||||
return await dbContext.ShowMetadata
|
||||
.AsNoTracking()
|
||||
.Where(sm => sm.ShowId == showId)
|
||||
.Where(sm => sm.Show.LibraryPath.LibraryId == libraryId)
|
||||
.Where(sm => EF.Functions.Like(sm.Title, $"%{title}%"))
|
||||
.Map(sm => sm.ShowId)
|
||||
.Map(sm => sm.Title)
|
||||
.FirstOrDefaultAsync()
|
||||
.Map(showId => showId > 0 ? Option<int>.Some(showId) : Option<int>.None);
|
||||
.Map(Optional);
|
||||
}
|
||||
|
||||
public async Task<List<int>> GetEpisodeIdsForShow(int showId)
|
||||
|
||||
@@ -139,6 +139,7 @@ public class ApiErrorResponseMetadataTests
|
||||
[TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.DeleteProfileAsync), StatusCodes.Status404NotFound)]
|
||||
[TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.DeleteProfileAsync), StatusCodes.Status401Unauthorized)]
|
||||
[TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.DeleteProfileAsync), StatusCodes.Status422UnprocessableEntity)]
|
||||
[TestCase(typeof(LibrariesController), nameof(LibrariesController.ScanShow), StatusCodes.Status404NotFound)]
|
||||
public void Api_Error_Response_Metadata_Should_Document_ProblemDetails(
|
||||
Type controllerType,
|
||||
string actionName,
|
||||
|
||||
@@ -3,7 +3,10 @@ using ErsatzTV.Application.Libraries;
|
||||
using ErsatzTV.Controllers.Api;
|
||||
using ErsatzTV.Core.Api.Libraries;
|
||||
using ErsatzTV.Core.Interfaces.Repositories;
|
||||
using LanguageExt;
|
||||
using MediatR;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.AspNetCore.Mvc.Routing;
|
||||
using NSubstitute;
|
||||
using NUnit.Framework;
|
||||
@@ -16,12 +19,14 @@ public class LibrariesControllerTests
|
||||
{
|
||||
private LibrariesController _controller = null!;
|
||||
private IMediator _mediator = null!;
|
||||
private ITelevisionRepository _televisionRepository = null!;
|
||||
|
||||
[SetUp]
|
||||
public void SetUp()
|
||||
{
|
||||
_mediator = Substitute.For<IMediator>();
|
||||
_controller = new LibrariesController(Substitute.For<ITelevisionRepository>(), _mediator);
|
||||
_televisionRepository = Substitute.For<ITelevisionRepository>();
|
||||
_controller = new LibrariesController(_televisionRepository, _mediator);
|
||||
}
|
||||
|
||||
[Test]
|
||||
@@ -52,4 +57,42 @@ public class LibrariesControllerTests
|
||||
|
||||
result.ShouldBe(expected);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task ScanShow_Should_Return_NotFoundProblem_When_Show_Not_In_Library()
|
||||
{
|
||||
_televisionRepository.GetShowTitle(3, 999).Returns(Option<string>.None);
|
||||
|
||||
IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(999));
|
||||
|
||||
var notFound = result.ShouldBeOfType<NotFoundObjectResult>();
|
||||
notFound.Value.ShouldBeOfType<ProblemDetails>().Status.ShouldBe(StatusCodes.Status404NotFound);
|
||||
await _mediator.DidNotReceive().Send(Arg.Any<QueueShowScanByLibraryId>(), Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task ScanShow_Should_Queue_Scan_By_Show_Id_When_Show_Belongs_To_Library()
|
||||
{
|
||||
_televisionRepository.GetShowTitle(3, 42).Returns(Option<string>.Some("The Office"));
|
||||
_mediator.Send(Arg.Any<QueueShowScanByLibraryId>(), Arg.Any<CancellationToken>()).Returns(true);
|
||||
|
||||
IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42, DeepScan: true));
|
||||
|
||||
result.ShouldBeOfType<OkResult>();
|
||||
await _mediator.Received(1).Send(
|
||||
Arg.Is<QueueShowScanByLibraryId>(r =>
|
||||
r.LibraryId == 3 && r.ShowId == 42 && r.ShowTitle == "The Office" && r.DeepScan),
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task ScanShow_Should_Return_BadRequest_When_Mediator_Fails_To_Queue()
|
||||
{
|
||||
_televisionRepository.GetShowTitle(3, 42).Returns(Option<string>.Some("The Office"));
|
||||
_mediator.Send(Arg.Any<QueueShowScanByLibraryId>(), Arg.Any<CancellationToken>()).Returns(false);
|
||||
|
||||
IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42));
|
||||
|
||||
result.ShouldBeOfType<BadRequestObjectResult>();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -260,6 +260,7 @@ public class OpenApiErrorResponseContractTests
|
||||
[TestCase("/api/trakt/lists/{id}", "put", "404")]
|
||||
[TestCase("/api/trakt/lists/{id}", "put", "422")]
|
||||
[TestCase("/api/troubleshoot/playback/subtitles/{mediaItemId}", "get", "404")]
|
||||
[TestCase("/api/libraries/{id}/scan-show", "post", "404")]
|
||||
public void Static_OpenApi_Should_Document_ProblemDetails_For_Api_Error_Responses(
|
||||
string path,
|
||||
string method,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
using ErsatzTV.Application.Libraries;
|
||||
using ErsatzTV.Core.Api.Libraries;
|
||||
using ErsatzTV.Core.Interfaces.Repositories;
|
||||
using ErsatzTV.Extensions;
|
||||
using MediatR;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
@@ -29,27 +30,23 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe
|
||||
[HttpPost("/api/libraries/{id:int}/scan-show")]
|
||||
[Tags("Libraries")]
|
||||
[EndpointSummary("Scan show")]
|
||||
[ProducesResponseType(StatusCodes.Status200OK)]
|
||||
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]
|
||||
[ProducesResponseType(StatusCodes.Status400BadRequest)]
|
||||
public async Task<IActionResult> ScanShow(int id, [FromBody] ScanShowRequest request)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(request.ShowTitle))
|
||||
Option<string> maybeTitle = await televisionRepository.GetShowTitle(id, request.ShowId);
|
||||
foreach (string title in maybeTitle)
|
||||
{
|
||||
return new BadRequestObjectResult(new { error = "ShowTitle is required" });
|
||||
}
|
||||
|
||||
string trimmedTitle = request.ShowTitle.Trim();
|
||||
Option<int> maybeShowId = await televisionRepository.GetShowIdByTitle(id, trimmedTitle);
|
||||
foreach (int showId in maybeShowId)
|
||||
{
|
||||
bool result = await mediator.Send(new QueueShowScanByLibraryId(id, showId, trimmedTitle, request.DeepScan));
|
||||
bool 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 new BadRequestObjectResult(
|
||||
new { error = $"Unable to locate show with title {request.ShowTitle} in library {id}" });
|
||||
return ApiResults.NotFoundProblem($"Show {request.ShowId} does not exist in library {id}.");
|
||||
}
|
||||
}
|
||||
|
||||
public record ScanShowRequest(string ShowTitle, bool DeepScan = false);
|
||||
public record ScanShowRequest(int ShowId, bool DeepScan = false);
|
||||
|
||||
@@ -5317,6 +5317,46 @@
|
||||
"responses": {
|
||||
"200": {
|
||||
"description": "OK"
|
||||
},
|
||||
"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"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"400": {
|
||||
"description": "Bad Request",
|
||||
"content": {
|
||||
"text/plain": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/ProblemDetails"
|
||||
}
|
||||
},
|
||||
"application/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/ProblemDetails"
|
||||
}
|
||||
},
|
||||
"text/json": {
|
||||
"schema": {
|
||||
"$ref": "#/components/schemas/ProblemDetails"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -20790,15 +20830,13 @@
|
||||
},
|
||||
"ScanShowRequest": {
|
||||
"required": [
|
||||
"showTitle"
|
||||
"showId"
|
||||
],
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"showTitle": {
|
||||
"type": [
|
||||
"null",
|
||||
"string"
|
||||
]
|
||||
"showId": {
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
"deepScan": {
|
||||
"type": "boolean",
|
||||
|
||||
Vendored
+1
-1
@@ -1280,7 +1280,7 @@ export interface components {
|
||||
"libraryRefreshInterval": number;
|
||||
};
|
||||
"ScanShowRequest": {
|
||||
"showTitle": null | string;
|
||||
"showId": number;
|
||||
"deepScan"?: boolean;
|
||||
};
|
||||
"ScheduleItemRequest": {
|
||||
|
||||
@@ -22,19 +22,19 @@ describe('libraries api client', () => {
|
||||
expect(fetchMock).toHaveBeenCalledWith('/api/libraries/4/scan', expect.objectContaining({ method: 'POST' }));
|
||||
});
|
||||
|
||||
it('scanShow POSTs the show title and deepScan flag', async () => {
|
||||
it('scanShow POSTs the show id and deepScan flag', async () => {
|
||||
const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent());
|
||||
await scanShow(4, { deepScan: true, showTitle: 'The Office' });
|
||||
await scanShow(4, { deepScan: true, showId: 42 });
|
||||
const { init, url } = lastCall(fetchMock);
|
||||
expect(url).toBe('/api/libraries/4/scan-show');
|
||||
expect(init?.method).toBe('POST');
|
||||
expect(JSON.parse(String(init?.body))).toEqual({ deepScan: true, showTitle: 'The Office' });
|
||||
expect(JSON.parse(String(init?.body))).toEqual({ deepScan: true, showId: 42 });
|
||||
});
|
||||
|
||||
it('scanShow defaults deepScan to false when omitted', async () => {
|
||||
const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent());
|
||||
await scanShow(9, { showTitle: 'Firefly' });
|
||||
await scanShow(9, { showId: 17 });
|
||||
const { init } = lastCall(fetchMock);
|
||||
expect(JSON.parse(String(init?.body))).toEqual({ deepScan: false, showTitle: 'Firefly' });
|
||||
expect(JSON.parse(String(init?.body))).toEqual({ deepScan: false, showId: 17 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -52,16 +52,16 @@ export function scanLibrary(libraryId: number): Promise<void> {
|
||||
}
|
||||
|
||||
export interface ScanShowParams {
|
||||
showTitle: string;
|
||||
showId: number;
|
||||
deepScan?: boolean;
|
||||
}
|
||||
|
||||
// Queues a scan of a single show (by title) within a library. Returns 200 on success, 400 when
|
||||
// the title can't be resolved / the library doesn't support single-show scanning. Body keys are
|
||||
// `showTitle` and `deepScan` (see LibrariesController.ScanShowRequest).
|
||||
// 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).
|
||||
export function scanShow(libraryId: number, params: ScanShowParams): Promise<void> {
|
||||
return request<void>(`/api/libraries/${libraryId}/scan-show`, {
|
||||
body: { deepScan: params.deepScan ?? false, showTitle: params.showTitle },
|
||||
body: { deepScan: params.deepScan ?? false, showId: params.showId },
|
||||
method: 'POST'
|
||||
});
|
||||
}
|
||||
|
||||
@@ -154,7 +154,7 @@ describe('media detail screens', () => {
|
||||
const scanCall = fetchSpy.mock.calls.find(([url]) => String(url) === '/api/libraries/3/scan-show');
|
||||
expect(scanCall).toBeTruthy();
|
||||
const body = JSON.parse(String((scanCall![1] as RequestInit).body));
|
||||
expect(body).toMatchObject({ deepScan: true, showTitle: 'The Show' });
|
||||
expect(body).toMatchObject({ deepScan: true, showId: 42 });
|
||||
});
|
||||
await waitFor(() => expect(screen.getByText('Scan queued')).toBeInTheDocument());
|
||||
});
|
||||
|
||||
@@ -584,7 +584,7 @@ function ShowScanControls({ show }: { show: ShowDetail }) {
|
||||
const runScan = (deepScan: boolean) => {
|
||||
setScanning(deepScan ? 'deep' : 'quick');
|
||||
setMessage(null);
|
||||
scanShow(show.libraryId, { deepScan, showTitle: show.title })
|
||||
scanShow(show.libraryId, { deepScan, showId: show.id })
|
||||
.then(() => {
|
||||
if (activeRef.current) {
|
||||
setScanning(false);
|
||||
|
||||
Reference in New Issue
Block a user