Two holes the review round found in the previous fix, both of the same shape: a guard that names its own fields instead of deriving them. The deco validators short-circuited the entire Validators.IdsMustExist call when the DecoMode does not consume the ids, which took the 512-item raw-count cap with it -- an arbitrarily large array under Inherit/Disable parsed and materialized with nothing bounding it. Only the EXISTENCE half is the apply path's business, so the mode predicate is now a required argument of the shared validator and gates that half alone; the cap runs under every mode. The channel recovery path rechecked GraphicsElementIdsMustExist alone, so a watermark deleted between validation and SaveChangesAsync still surfaced as the unhandled 500 the fix exists to remove -- WatermarkId, FFmpegProfileId, FallbackFillerId and MirrorSourceChannelId are all written by the same save and lose the same race. Both handlers now re-ask the whole of Validate on DbUpdateException, so a validator added later is covered without editing the recovery path. The API-site outside-folder discriminator test seeded an Image row, so the Kind conjunct rejected it whatever the path comparison did: a composite revert to Path.GetFileName(path) == filename && kind == Text passed every API test. It now carries the seeded Kind, mirroring the seeder-site twin, so only the path half can reject it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV
417 lines
17 KiB
C#
417 lines
17 KiB
C#
using System.Globalization;
|
|
using System.Threading.Channels;
|
|
using ErsatzTV.Application;
|
|
using ErsatzTV.Application.Scheduling;
|
|
using ErsatzTV.Core;
|
|
using ErsatzTV.Core.Domain;
|
|
using ErsatzTV.Core.Domain.Scheduling;
|
|
using ErsatzTV.Infrastructure.Data;
|
|
using ErsatzTV.Tests.Support;
|
|
using LanguageExt;
|
|
using Microsoft.EntityFrameworkCore;
|
|
using NSubstitute;
|
|
using NUnit.Framework;
|
|
using Shouldly;
|
|
|
|
namespace ErsatzTV.Tests.Application.Scheduling;
|
|
|
|
/// <summary>
|
|
/// #568: the same full-replace-DTO FK hardening applied to UpdateChannelHandler's
|
|
/// graphicsElementIds also closes the identical twin defect in UpdateDecoHandler -- both
|
|
/// graphicsElementIds and watermarkIds are top-level ReplaceDecoRequest fields (not the "deep FK
|
|
/// ids nested inside item-list request bodies" carve-out in api-conventions.md), and the
|
|
/// reconcile in ApplyUpdateRequest blindly Adds a join row for every incoming id, so an unknown
|
|
/// id used to hit the FK constraint at SaveChangesAsync and surface as an unhandled 500.
|
|
/// </summary>
|
|
[TestFixture]
|
|
public class UpdateDecoGraphicsElementsTests
|
|
{
|
|
private InMemoryTvContext _db = null!;
|
|
private ChannelWriter<IBackgroundServiceRequest> _channel = null!;
|
|
|
|
[SetUp]
|
|
public async Task SetUp()
|
|
{
|
|
_db = await InMemoryTvContext.CreateAsync();
|
|
_channel = Substitute.For<ChannelWriter<IBackgroundServiceRequest>>();
|
|
}
|
|
|
|
[TearDown]
|
|
public async Task TearDown() => await _db.DisposeAsync();
|
|
|
|
private static bool IsLeft<T>(Either<BaseError, T> result) => result.Match(Right: _ => false, Left: _ => true);
|
|
|
|
private async Task SeedDeco()
|
|
{
|
|
await using TvContext context = _db.CreateContext();
|
|
context.Decos.Add(
|
|
new Deco
|
|
{
|
|
Id = 1,
|
|
DecoGroupId = 1,
|
|
Name = "D",
|
|
BreakContent = [],
|
|
DecoWatermarks = [],
|
|
DecoGraphicsElements = []
|
|
});
|
|
await context.SaveChangesAsync();
|
|
}
|
|
|
|
private static UpdateDeco MakeUpdate(
|
|
List<int> graphicsElementIds = null,
|
|
List<int> watermarkIds = null,
|
|
DecoMode? graphicsElementsMode = null,
|
|
DecoMode? watermarkMode = null) =>
|
|
new(
|
|
1,
|
|
1,
|
|
"D",
|
|
watermarkMode ?? DecoMode.Inherit,
|
|
watermarkIds ?? [],
|
|
false,
|
|
graphicsElementsMode ?? (graphicsElementIds is null ? DecoMode.Inherit : DecoMode.Override),
|
|
graphicsElementIds ?? [],
|
|
false,
|
|
DecoMode.Inherit,
|
|
[],
|
|
DecoMode.Inherit,
|
|
CollectionType.Collection,
|
|
null,
|
|
null,
|
|
null,
|
|
null,
|
|
false,
|
|
DecoMode.Inherit,
|
|
CollectionType.Collection,
|
|
null,
|
|
null,
|
|
null,
|
|
null);
|
|
|
|
private async Task<int> SeedGraphicsElement()
|
|
{
|
|
await using TvContext context = _db.CreateContext();
|
|
var element = new GraphicsElement { Path = "element-a.yml" };
|
|
context.GraphicsElements.Add(element);
|
|
await context.SaveChangesAsync();
|
|
return element.Id;
|
|
}
|
|
|
|
private async Task<List<int>> SeedGraphicsElements(int count)
|
|
{
|
|
await using TvContext context = _db.CreateContext();
|
|
List<GraphicsElement> elements = Enumerable.Range(0, count)
|
|
.Select(i => new GraphicsElement { Path = $"element-{i}.yml" })
|
|
.ToList();
|
|
context.GraphicsElements.AddRange(elements);
|
|
await context.SaveChangesAsync();
|
|
return elements.Select(e => e.Id).ToList();
|
|
}
|
|
|
|
private async Task<List<int>> SeedWatermarks(int count)
|
|
{
|
|
await using TvContext context = _db.CreateContext();
|
|
List<ChannelWatermark> watermarks = Enumerable.Range(0, count)
|
|
.Select(i => new ChannelWatermark { Name = $"W{i}" })
|
|
.ToList();
|
|
context.ChannelWatermarks.AddRange(watermarks);
|
|
await context.SaveChangesAsync();
|
|
return watermarks.Select(w => w.Id).ToList();
|
|
}
|
|
|
|
private async Task<int> SeedWatermark()
|
|
{
|
|
await using TvContext context = _db.CreateContext();
|
|
var watermark = new ChannelWatermark { Name = "W" };
|
|
context.ChannelWatermarks.Add(watermark);
|
|
await context.SaveChangesAsync();
|
|
return watermark.Id;
|
|
}
|
|
|
|
private async Task AttachWatermark(int watermarkId)
|
|
{
|
|
await using TvContext context = _db.CreateContext();
|
|
Deco deco = await context.Decos.Include(d => d.DecoWatermarks).SingleAsync(d => d.Id == 1);
|
|
deco.WatermarkMode = DecoMode.Override;
|
|
deco.DecoWatermarks.Add(new DecoWatermark { DecoId = 1, WatermarkId = watermarkId });
|
|
await context.SaveChangesAsync();
|
|
}
|
|
|
|
private async Task AttachGraphicsElement(int elementId)
|
|
{
|
|
await using TvContext context = _db.CreateContext();
|
|
Deco deco = await context.Decos.Include(d => d.DecoGraphicsElements).SingleAsync(d => d.Id == 1);
|
|
deco.GraphicsElementsMode = DecoMode.Override;
|
|
deco.DecoGraphicsElements.Add(new DecoGraphicsElement { DecoId = 1, GraphicsElementId = elementId });
|
|
await context.SaveChangesAsync();
|
|
}
|
|
|
|
// Removing UpdateDecoHandler.GraphicsElementIdsMustExist alone from Validate is row 36 of the
|
|
// mutation table in docs/graphics-elements.md, measured against the whole ErsatzTV.Tests project.
|
|
[Test]
|
|
public async Task Should_Reject_Unknown_GraphicsElementId_With_A_Validation_Error_Not_A_Throw()
|
|
{
|
|
await SeedDeco();
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(graphicsElementIds: [999]),
|
|
CancellationToken.None);
|
|
|
|
IsLeft(result).ShouldBeTrue();
|
|
BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
|
|
error.Value.ShouldContain("[GraphicsElementIds]");
|
|
error.Value.ShouldContain("999");
|
|
|
|
// no partial write: the deco keeps no graphics element association
|
|
await using TvContext context = _db.CreateContext();
|
|
Deco reloaded = await context.Decos.Include(d => d.DecoGraphicsElements).SingleAsync(d => d.Id == 1);
|
|
reloaded.DecoGraphicsElements.ShouldBeEmpty();
|
|
}
|
|
|
|
// Removing UpdateDecoHandler.WatermarkIdsMustExist alone from Validate is row 37 of the
|
|
// mutation table in docs/graphics-elements.md.
|
|
[Test]
|
|
public async Task Should_Reject_Unknown_WatermarkId_With_A_Validation_Error_Not_A_Throw()
|
|
{
|
|
await SeedDeco();
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
new UpdateDeco(
|
|
1,
|
|
1,
|
|
"D",
|
|
DecoMode.Override,
|
|
[999],
|
|
false,
|
|
DecoMode.Inherit,
|
|
[],
|
|
false,
|
|
DecoMode.Inherit,
|
|
[],
|
|
DecoMode.Inherit,
|
|
CollectionType.Collection,
|
|
null,
|
|
null,
|
|
null,
|
|
null,
|
|
false,
|
|
DecoMode.Inherit,
|
|
CollectionType.Collection,
|
|
null,
|
|
null,
|
|
null,
|
|
null),
|
|
CancellationToken.None);
|
|
|
|
IsLeft(result).ShouldBeTrue();
|
|
BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
|
|
error.Value.ShouldContain("[WatermarkIds]");
|
|
error.Value.ShouldContain("999");
|
|
}
|
|
|
|
// The mode, not the id list, decides whether an id is data. ApplyUpdateRequest reconciles the
|
|
// join table only under Override/Merge and Clear()s it otherwise, so validating unconditionally
|
|
// would reject a save the apply path was going to discard. Removing the ConsumesGraphicsElementIds
|
|
// guard alone from UpdateDecoHandler.GraphicsElementIdsMustExist is row 38 of the mutation table
|
|
// in docs/graphics-elements.md.
|
|
[Test]
|
|
public async Task Should_Ignore_An_Unknown_GraphicsElementId_When_The_Mode_Does_Not_Consume_It()
|
|
{
|
|
await SeedDeco();
|
|
int elementId = await SeedGraphicsElement();
|
|
await AttachGraphicsElement(elementId);
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(graphicsElementsMode: DecoMode.Inherit, graphicsElementIds: [999]),
|
|
CancellationToken.None);
|
|
|
|
result.IsRight.ShouldBeTrue();
|
|
|
|
// the apply path discards the ids under Inherit, and the existing attachment with them
|
|
await using TvContext reload = _db.CreateContext();
|
|
Deco reloaded = await reload.Decos.Include(d => d.DecoGraphicsElements).SingleAsync(d => d.Id == 1);
|
|
reloaded.GraphicsElementsMode.ShouldBe(DecoMode.Inherit);
|
|
reloaded.DecoGraphicsElements.ShouldBeEmpty();
|
|
}
|
|
|
|
// Twin of the above for the watermark half; removing the ConsumesWatermarkIds guard alone from
|
|
// UpdateDecoHandler.WatermarkIdsMustExist is row 39 of the mutation table in
|
|
// docs/graphics-elements.md.
|
|
[Test]
|
|
public async Task Should_Ignore_An_Unknown_WatermarkId_When_The_Mode_Does_Not_Consume_It()
|
|
{
|
|
await SeedDeco();
|
|
int watermarkId = await SeedWatermark();
|
|
await AttachWatermark(watermarkId);
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(watermarkMode: DecoMode.Disable, watermarkIds: [999]),
|
|
CancellationToken.None);
|
|
|
|
result.IsRight.ShouldBeTrue();
|
|
|
|
await using TvContext reload = _db.CreateContext();
|
|
Deco reloaded = await reload.Decos.Include(d => d.DecoWatermarks).SingleAsync(d => d.Id == 1);
|
|
reloaded.WatermarkMode.ShouldBe(DecoMode.Disable);
|
|
reloaded.DecoWatermarks.ShouldBeEmpty();
|
|
}
|
|
|
|
[Test]
|
|
public async Task Should_Accept_A_Known_GraphicsElementId()
|
|
{
|
|
await SeedDeco();
|
|
|
|
int elementId = await SeedGraphicsElement();
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(graphicsElementIds: [elementId]),
|
|
CancellationToken.None);
|
|
|
|
result.IsRight.ShouldBeTrue();
|
|
|
|
await using TvContext reload = _db.CreateContext();
|
|
Deco reloaded = await reload.Decos.Include(d => d.DecoGraphicsElements).SingleAsync(d => d.Id == 1);
|
|
reloaded.DecoGraphicsElements.Select(x => x.GraphicsElementId).ShouldBe(new[] { elementId });
|
|
}
|
|
|
|
// Both deco id lists go through the same Validators.IdsMustExist as the channel's, so both
|
|
// inherit the same raw-count cap; the channel fixture pins its edges, these two pin that each
|
|
// deco field is actually behind it and names itself when it rejects.
|
|
[Test]
|
|
public async Task Should_Reject_More_Than_The_Maximum_Number_Of_GraphicsElementIds()
|
|
{
|
|
await SeedDeco();
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(graphicsElementIds: Enumerable.Range(1, Validators.MaximumIdListCount + 1).ToList()),
|
|
CancellationToken.None);
|
|
|
|
IsLeft(result).ShouldBeTrue();
|
|
BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
|
|
error.Value.ShouldContain("[GraphicsElementIds]");
|
|
error.Value.ShouldContain(Validators.MaximumIdListCount.ToString(CultureInfo.InvariantCulture));
|
|
}
|
|
|
|
[Test]
|
|
public async Task Should_Reject_More_Than_The_Maximum_Number_Of_WatermarkIds()
|
|
{
|
|
await SeedDeco();
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(
|
|
watermarkMode: DecoMode.Override,
|
|
watermarkIds: Enumerable.Range(1, Validators.MaximumIdListCount + 1).ToList()),
|
|
CancellationToken.None);
|
|
|
|
IsLeft(result).ShouldBeTrue();
|
|
BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
|
|
error.Value.ShouldContain("[WatermarkIds]");
|
|
error.Value.ShouldContain(Validators.MaximumIdListCount.ToString(CultureInfo.InvariantCulture));
|
|
}
|
|
|
|
// The mode gate is handed to Validators.IdsMustExist rather than short-circuiting the call,
|
|
// because only the EXISTENCE half is the apply path's business: a list the reconcile discards
|
|
// was still parsed and materialized out of the request body. These two pin that the cap holds
|
|
// under a mode that consumes nothing -- row 47 of the mutation table in
|
|
// docs/graphics-elements.md. Note the ids all EXIST here, so nothing but the cap can reject
|
|
// them: a rejection is the cap's, not a smuggled existence check.
|
|
[Test]
|
|
public async Task Should_Reject_Too_Many_GraphicsElementIds_Even_Under_A_Mode_That_Does_Not_Consume_Them()
|
|
{
|
|
await SeedDeco();
|
|
List<int> ids = await SeedGraphicsElements(Validators.MaximumIdListCount + 1);
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(graphicsElementsMode: DecoMode.Inherit, graphicsElementIds: ids),
|
|
CancellationToken.None);
|
|
|
|
IsLeft(result).ShouldBeTrue();
|
|
BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
|
|
error.Value.ShouldContain("[GraphicsElementIds]");
|
|
error.Value.ShouldContain(Validators.MaximumIdListCount.ToString(CultureInfo.InvariantCulture));
|
|
}
|
|
|
|
[Test]
|
|
public async Task Should_Reject_Too_Many_WatermarkIds_Even_Under_A_Mode_That_Does_Not_Consume_Them()
|
|
{
|
|
await SeedDeco();
|
|
List<int> ids = await SeedWatermarks(Validators.MaximumIdListCount + 1);
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(watermarkMode: DecoMode.Disable, watermarkIds: ids),
|
|
CancellationToken.None);
|
|
|
|
IsLeft(result).ShouldBeTrue();
|
|
BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
|
|
error.Value.ShouldContain("[WatermarkIds]");
|
|
error.Value.ShouldContain(Validators.MaximumIdListCount.ToString(CultureInfo.InvariantCulture));
|
|
}
|
|
|
|
// The deco twin of the channel handler's lost-race translation: an element deleted between
|
|
// Validate and SaveChangesAsync must come back as the validator's own 422, not the FK
|
|
// exception. Removing the DbUpdateException catch from
|
|
// UpdateDecoHandler.ApplyUpdateRequestTranslatingLostRace is row 44 of the mutation table in
|
|
// docs/graphics-elements.md.
|
|
[Test]
|
|
public async Task Should_Translate_A_Deco_Element_Deleted_Between_Validation_And_Save_Into_The_Same_422()
|
|
{
|
|
var interceptor = new ArmedSaveFailureInterceptor();
|
|
await _db.DisposeAsync();
|
|
_db = await InMemoryTvContext.CreateAsync(interceptor);
|
|
|
|
await SeedDeco();
|
|
int elementId = await SeedGraphicsElement();
|
|
|
|
interceptor.SqlBeforeFailing =
|
|
$"DELETE FROM GraphicsElement WHERE Id = {elementId.ToString(CultureInfo.InvariantCulture)}";
|
|
interceptor.Armed = true;
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(graphicsElementIds: [elementId]),
|
|
CancellationToken.None);
|
|
|
|
IsLeft(result).ShouldBeTrue();
|
|
BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
|
|
error.Value.ShouldContain("[GraphicsElementIds]");
|
|
error.Value.ShouldContain(elementId.ToString(CultureInfo.InvariantCulture));
|
|
}
|
|
|
|
// The watermark half of the same recovery. Without it, removing the watermark question from the
|
|
// recheck would redden nothing -- and the recheck re-asks the whole of Validate exactly so that
|
|
// neither id list is the only one covered. Row 46 of the mutation table.
|
|
[Test]
|
|
public async Task Should_Translate_A_Deco_Watermark_Deleted_Between_Validation_And_Save_Into_The_Same_422()
|
|
{
|
|
var interceptor = new ArmedSaveFailureInterceptor();
|
|
await _db.DisposeAsync();
|
|
_db = await InMemoryTvContext.CreateAsync(interceptor);
|
|
|
|
await SeedDeco();
|
|
int watermarkId = await SeedWatermark();
|
|
|
|
interceptor.SqlBeforeFailing =
|
|
$"DELETE FROM ChannelWatermark WHERE Id = {watermarkId.ToString(CultureInfo.InvariantCulture)}";
|
|
interceptor.Armed = true;
|
|
|
|
var handler = new UpdateDecoHandler(_db.Factory, _channel);
|
|
Either<BaseError, Unit> result = await handler.Handle(
|
|
MakeUpdate(watermarkMode: DecoMode.Override, watermarkIds: [watermarkId]),
|
|
CancellationToken.None);
|
|
|
|
IsLeft(result).ShouldBeTrue();
|
|
BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
|
|
error.Value.ShouldContain("[WatermarkIds]");
|
|
error.Value.ShouldContain(watermarkId.ToString(CultureInfo.InvariantCulture));
|
|
}
|
|
}
|