Files
ersatztv/ErsatzTV.Application/Scheduling/Commands/ReplaceBlockItemsHandler.cs
T
timothyandClaude Opus 4.8 216130b4d7 fix(#172): API hardening — null-name 500s, duplicate template items, unreachable 404
Clears the still-live findings from #172 (verified against main; #2/#4/#7 and the
auth/search/Trakt tail were already deliberate-documented or fixed since 2026-07-07).

- Null/empty Name → 500 (10 create/replace handlers). Block/Template/DecoTemplate/Deco
  Create+Replace/Update + UpdateFFmpegProfile did `request.Name.Length > 50` on a
  client-nullable string → unhandled NullReferenceException → HTTP 500 (no global
  exception filter). Now `string.IsNullOrWhiteSpace(request.Name) || .Length > 50` →
  422; also rejects empty/whitespace names, matching the group-create handlers'
  NotEmpty behavior. CreatePlaylist coalesces null→"" at the DTO so it was an
  empty-name persist, not a 500; guarded the same way.
- ReplaceTemplateItems overlap validation iterated with an `item == otherItem`
  record value-equality skip, so two exact-duplicate items were value-equal and
  bypassed the intersection check (both persisted). Now index-based (i != j) so
  duplicates register as a self-intersection and are rejected 422.
- Trimmed the unreachable 404 ProducesResponseType from POST /api/blocks/groups and
  POST /api/templates/groups (a create has no parent lookup that can 404); v1.json
  regenerated.
- Regression tests: all 10 name-guard paths + the duplicate-items path (19 cases).
- Docs: decisions.md entry + api-conventions.md §3b null-safe-validation bullet.

fixes #172

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-13 01:16:52 +02:00

221 lines
8.9 KiB
C#

using ErsatzTV.Core;
using ErsatzTV.Core.Domain;
using ErsatzTV.Core.Domain.Scheduling;
using ErsatzTV.Infrastructure.Data;
using ErsatzTV.Infrastructure.Extensions;
using Microsoft.EntityFrameworkCore;
namespace ErsatzTV.Application.Scheduling;
public class ReplaceBlockItemsHandler(IDbContextFactory<TvContext> dbContextFactory)
: IRequestHandler<ReplaceBlockItems, Either<BaseError, Unit>>
{
public async Task<Either<BaseError, Unit>> Handle(
ReplaceBlockItems request,
CancellationToken cancellationToken)
{
await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken);
Validation<BaseError, Block> validation = await Validate(dbContext, request, cancellationToken);
// Introduce the optimistic-concurrency check as a standalone Either AFTER the validation
// pipeline (never via Apply) so a PreconditionFailedError survives to a 412 and is not
// flattened to a generic 422 by Join() (issue #253 / api-conventions §7a).
// LanguageExtensions.ToEither joins the Seq<BaseError> to a single BaseError (the native
// Validation.ToEither() would keep Seq and is called explicitly here to avoid that shadow).
Either<BaseError, Block> validated = LanguageExtensions.ToEither(validation)
.Bind(block => block.CheckVersion(request.ExpectedVersions));
return await validated.Match(
Right: block => Persist(dbContext, request, block, cancellationToken),
Left: error => Task.FromResult<Either<BaseError, Unit>>(error));
}
private static async Task<Either<BaseError, Unit>> Persist(
TvContext dbContext,
ReplaceBlockItems request,
Block block,
CancellationToken cancellationToken)
{
block.Name = request.Name;
block.Minutes = request.Minutes;
block.StopScheduling = request.StopScheduling;
block.DateUpdated = DateTime.UtcNow;
dbContext.RemoveRange(block.Items);
block.Items = request.Items.Map(i => BuildItem(block, i.Index, i)).ToList();
// Unconditional bump: EF emits the root UPDATE only when a scalar actually differs, so a
// same-value/no-op save would otherwise write no root row and neither fire the concurrency
// token nor rotate other clients' ETags. Bumping guarantees both on every save (issue #253).
block.Version++;
// TODO: refresh any playouts that use this schedule
// foreach (Playout playout in programSchedule.Playouts)
// {
// await _channel.WriteAsync(new BuildPlayout(playout.Id, PlayoutBuildMode.Refresh));
// }
// Save through the guard so an EF concurrency failure (a racing writer won between our load
// and save) maps to 412 rather than surfacing as a 500.
return await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken);
}
private static BlockItem BuildItem(Block block, int index, ReplaceBlockItem item)
{
var result = new BlockItem
{
BlockId = block.Id,
Index = index,
CollectionType = item.CollectionType,
CollectionId = item.CollectionId,
MultiCollectionId = item.MultiCollectionId,
SmartCollectionId = item.SmartCollectionId,
MediaItemId = item.MediaItemId,
SearchTitle = item.SearchTitle,
SearchQuery = item.SearchQuery,
PlaybackOrder = item.PlaybackOrder,
IncludeInProgramGuide = item.IncludeInProgramGuide,
DisableWatermarks = item.DisableWatermarks,
BlockItemWatermarks = [],
BlockItemGraphicsElements = []
};
foreach (int watermarkId in item.WatermarkIds)
{
result.BlockItemWatermarks ??= [];
result.BlockItemWatermarks.Add(
new BlockItemWatermark
{
BlockItem = result,
WatermarkId = watermarkId
});
}
foreach (int graphicsElementId in item.GraphicsElementIds)
{
result.BlockItemGraphicsElements ??= [];
result.BlockItemGraphicsElements.Add(
new BlockItemGraphicsElement
{
BlockItem = result,
GraphicsElementId = graphicsElementId
});
}
return result;
}
private static Task<Validation<BaseError, Block>> Validate(
TvContext dbContext,
ReplaceBlockItems request,
CancellationToken cancellationToken) =>
BlockMustExist(dbContext, request.BlockId, cancellationToken)
.BindT(block => MinutesMustBeValid(request, block))
.BindT(block => BlockNameMustBeValid(dbContext, block, request))
.BindT(block => CollectionTypesMustBeValid(request, block));
private static Task<Validation<BaseError, Block>> BlockMustExist(
TvContext dbContext,
int blockId,
CancellationToken cancellationToken) =>
dbContext.Blocks
.Include(b => b.Items)
.ThenInclude(i => i.BlockItemWatermarks)
.ThenInclude(wm => wm.Watermark)
.Include(b => b.Items)
.ThenInclude(i => i.BlockItemGraphicsElements)
.ThenInclude(ge => ge.GraphicsElement)
.SelectOneAsync(b => b.Id, b => b.Id == blockId, cancellationToken)
.Map(o => o.ToValidation<BaseError>("[BlockId] does not exist."));
private static Validation<BaseError, Block> MinutesMustBeValid(ReplaceBlockItems request, Block block) =>
Optional(block)
.Filter(_ => request.Minutes > 0 && request.Minutes % 5 == 0 && request.Minutes <= 24 * 60)
.ToValidation<BaseError>("Block duration must be between 5 minutes and 24 hours");
private static Validation<BaseError, Block> CollectionTypesMustBeValid(ReplaceBlockItems request, Block block) =>
request.Items.Map(item => CollectionTypeMustBeValid(item, block)).Sequence().Map(_ => block);
private static Validation<BaseError, Block> CollectionTypeMustBeValid(ReplaceBlockItem item, Block block)
{
switch (item.CollectionType)
{
case CollectionType.Collection:
if (item.CollectionId is null)
{
return BaseError.New("[Collection] is required for collection type 'Collection'");
}
break;
case CollectionType.TelevisionShow:
if (item.MediaItemId is null)
{
return BaseError.New("[MediaItem] is required for collection type 'TelevisionShow'");
}
break;
case CollectionType.TelevisionSeason:
if (item.MediaItemId is null)
{
return BaseError.New("[MediaItem] is required for collection type 'TelevisionSeason'");
}
break;
case CollectionType.Artist:
if (item.MediaItemId is null)
{
return BaseError.New("[MediaItem] is required for collection type 'Artist'");
}
break;
case CollectionType.MultiCollection:
if (item.MultiCollectionId is null)
{
return BaseError.New("[MultiCollection] is required for collection type 'MultiCollection'");
}
break;
case CollectionType.SmartCollection:
if (item.SmartCollectionId is null)
{
return BaseError.New("[SmartCollection] is required for collection type 'SmartCollection'");
}
break;
case CollectionType.SearchQuery:
if (string.IsNullOrWhiteSpace(item.SearchQuery))
{
return BaseError.New("[SearchQuery] is required for collection type 'SearchQuery'");
}
break;
case CollectionType.FakeCollection:
default:
return BaseError.New("[CollectionType] is invalid");
}
return block;
}
private static async Task<Validation<BaseError, Block>> BlockNameMustBeValid(
TvContext dbContext,
Block block,
ReplaceBlockItems request)
{
if (string.IsNullOrWhiteSpace(request.Name) || request.Name.Length > 50)
{
return BaseError.New($"Block name \"{request.Name}\" is invalid");
}
Option<Block> maybeExisting = await dbContext.Blocks
.AsNoTracking()
.FirstOrDefaultAsync(d =>
d.Id != request.BlockId && d.BlockGroupId == request.BlockGroupId && d.Name == request.Name)
.Map(Optional);
return maybeExisting.IsSome
? BaseError.New($"A block named \"{request.Name}\" already exists in that block group")
: Success<BaseError, Block>(block);
}
}