Adversarial review of PR #402 returned BLOCKED. It could not break the WRR math or the stateless-restore claim (it probed restore across wraps at indices 12/13/20/37 — all held, and the clamp preserves a 1000:1 ratio exactly). What it broke was the perimeter. B1 — the validation gate had a hole, so the silent-drop bug shipped. CreateChannelFromLineup is a THIRD writer of PlaylistItem.PlaybackOrder; its own guard only covered MultiCollection entries, so a 2+ entry lineup of plain collections persisted WeightedShuffle straight through to PlaylistEnumerator's null-drop. My decisions.md claim that "the silent sites never see it" was false as written — corrected in place, with the lesson recorded: grep every writer of the field, the non-obvious composite handler is the one that gets missed. The Add*ToPlaylist handlers are safe only because they hardcode their order. B2 — Weight had no validation at all, and create/update disagreed on the same input. EF's HasDefaultValue(1) substitutes 1 for a 0 on INSERT (0 reads as "not set") but an UPDATE writes the 0 through — and a 0-weight source was filtered out of the rotation, deleting it from the channel silently. Exactly the failure this order is careful to avoid everywhere else. Now bounded 1..1000 by a shared MultiCollectionItemWeight used by both paths so they cannot drift, and clamped again in the enumerator for rows that predate the gate. B3 — Sum(weights) is checked arithmetic, so two int.MaxValue weights threw OverflowException from inside a playout build. Reachable through the API precisely because of B2. The ceiling fixes both; the sum also widens to long. M1 the lineup mirror now allows WeightedShuffle for multi collections, matching the PlayoutModeMustBeValid change it claims to mirror. M3 ScheduleAsGroup is documented as deliberately unread by this order. L1 MinimumDuration is computed over every source instead of the current rotation — under the clamp a rotation is a strict subset and is rebuilt each wrap, so caching over it went stale. L2 the retry guard keys off the rotation, not the raw collection count. N1 the tautological default test is gone: it built entities in C#, so it asserted the property initializer, not the migration — it could not have failed. Replaced with clamp, overflow, and cross-wrap restore cases (the property the review proved but found unpinned). H1 the two follow-ups the PR body claimed were "filed" did not exist. Now filed: #403 (silent dispatch-fallback hardening) and #404 (SPA weight UI, blocked-by #388). Core.Tests 565 passed, ErsatzTV.Tests 1643 passed, 0 failed. Refs #70 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
204 lines
9.0 KiB
C#
204 lines
9.0 KiB
C#
using System.Threading.Channels;
|
|
using ErsatzTV.Application.Playouts;
|
|
using ErsatzTV.Core;
|
|
using ErsatzTV.Core.Domain;
|
|
using ErsatzTV.Core.Interfaces.Repositories;
|
|
using ErsatzTV.Core.Interfaces.Search;
|
|
using ErsatzTV.Core.Scheduling;
|
|
using ErsatzTV.Infrastructure.Data;
|
|
using ErsatzTV.Infrastructure.Extensions;
|
|
using Microsoft.EntityFrameworkCore;
|
|
|
|
namespace ErsatzTV.Application.MediaCollections;
|
|
|
|
public class UpdateMultiCollectionHandler : IRequestHandler<UpdateMultiCollection, Either<BaseError, Unit>>
|
|
{
|
|
private readonly ChannelWriter<IBackgroundServiceRequest> _channel;
|
|
private readonly IDbContextFactory<TvContext> _dbContextFactory;
|
|
private readonly IMediaCollectionRepository _mediaCollectionRepository;
|
|
private readonly ISearchTargets _searchTargets;
|
|
|
|
public UpdateMultiCollectionHandler(
|
|
IDbContextFactory<TvContext> dbContextFactory,
|
|
IMediaCollectionRepository mediaCollectionRepository,
|
|
ChannelWriter<IBackgroundServiceRequest> channel,
|
|
ISearchTargets searchTargets)
|
|
{
|
|
_dbContextFactory = dbContextFactory;
|
|
_mediaCollectionRepository = mediaCollectionRepository;
|
|
_channel = channel;
|
|
_searchTargets = searchTargets;
|
|
}
|
|
|
|
public async Task<Either<BaseError, Unit>> Handle(
|
|
UpdateMultiCollection request,
|
|
CancellationToken cancellationToken)
|
|
{
|
|
await using TvContext dbContext = await _dbContextFactory.CreateDbContextAsync(cancellationToken);
|
|
Validation<BaseError, MultiCollection> validation = await Validate(dbContext, request, cancellationToken);
|
|
|
|
// Optimistic-concurrency check as a standalone Either AFTER validation, never via Apply (which
|
|
// Join()s the error Seq and would flatten PreconditionFailedError to a 422) — issue #253 §7a.
|
|
Either<BaseError, MultiCollection> validated = LanguageExtensions.ToEither(validation)
|
|
.Bind(c => c.CheckVersion(request.ExpectedVersions));
|
|
|
|
return await validated.Match(
|
|
Right: c => ApplyUpdateRequest(dbContext, c, request, cancellationToken),
|
|
Left: error => Task.FromResult<Either<BaseError, Unit>>(error));
|
|
}
|
|
|
|
private async Task<Either<BaseError, Unit>> ApplyUpdateRequest(
|
|
TvContext dbContext,
|
|
MultiCollection c,
|
|
UpdateMultiCollection request,
|
|
CancellationToken cancellationToken)
|
|
{
|
|
c.Name = request.Name;
|
|
|
|
// Bump the version on this first save (issue #253 §7a): it rotates other clients' ETags and,
|
|
// via IsConcurrencyToken, closes the load→save race — a stale writer's WHERE Version=@orig hits
|
|
// 0 rows → PreconditionFailedError (412). Saving the name first also keeps a name-only change
|
|
// from triggering a playout rebuild (the item save below stays gated on real item changes).
|
|
c.Version++;
|
|
Either<BaseError, Unit> nameSave = await dbContext.SaveChangesWithConcurrencyGuard(cancellationToken);
|
|
if (nameSave.IsLeft)
|
|
{
|
|
return nameSave;
|
|
}
|
|
|
|
var toAdd = request.Items
|
|
.Filter(i => i.CollectionId.HasValue)
|
|
// ReSharper disable once PossibleInvalidOperationException
|
|
.Filter(i => c.MultiCollectionItems.All(i2 => i2.CollectionId != i.CollectionId.Value))
|
|
.Map(i => new MultiCollectionItem
|
|
{
|
|
// ReSharper disable once PossibleInvalidOperationException
|
|
CollectionId = i.CollectionId.Value,
|
|
MultiCollectionId = c.Id,
|
|
ScheduleAsGroup = i.ScheduleAsGroup,
|
|
PlaybackOrder = i.PlaybackOrder,
|
|
Weight = i.Weight
|
|
})
|
|
.ToList();
|
|
var toRemove = c.MultiCollectionItems
|
|
.Filter(i => request.Items.All(i2 => i2.CollectionId != i.CollectionId))
|
|
.ToList();
|
|
|
|
// remove items that are no longer present
|
|
c.MultiCollectionItems.RemoveAll(toRemove.Contains);
|
|
|
|
// update existing items
|
|
foreach (MultiCollectionItem item in c.MultiCollectionItems)
|
|
{
|
|
foreach (UpdateMultiCollectionItem incoming in
|
|
request.Items.Filter(i => i.CollectionId == item.CollectionId))
|
|
{
|
|
item.ScheduleAsGroup = incoming.ScheduleAsGroup;
|
|
item.PlaybackOrder = incoming.PlaybackOrder;
|
|
item.Weight = incoming.Weight;
|
|
}
|
|
}
|
|
|
|
// add new items
|
|
c.MultiCollectionItems.AddRange(toAdd);
|
|
|
|
var toAddSmart = request.Items
|
|
.Filter(i => i.SmartCollectionId.HasValue)
|
|
// ReSharper disable once PossibleInvalidOperationException
|
|
.Filter(i => c.MultiCollectionSmartItems.All(i2 => i2.SmartCollectionId != i.SmartCollectionId.Value))
|
|
.Map(i => new MultiCollectionSmartItem
|
|
{
|
|
// ReSharper disable once PossibleInvalidOperationException
|
|
SmartCollectionId = i.SmartCollectionId.Value,
|
|
MultiCollectionId = c.Id,
|
|
ScheduleAsGroup = i.ScheduleAsGroup,
|
|
PlaybackOrder = i.PlaybackOrder,
|
|
Weight = i.Weight
|
|
})
|
|
.ToList();
|
|
var toRemoveSmart = c.MultiCollectionSmartItems
|
|
.Filter(i => request.Items.All(i2 => i2.SmartCollectionId != i.SmartCollectionId))
|
|
.ToList();
|
|
|
|
// remove items that are no longer present
|
|
c.MultiCollectionSmartItems.RemoveAll(toRemoveSmart.Contains);
|
|
|
|
// update existing items
|
|
foreach (MultiCollectionSmartItem item in c.MultiCollectionSmartItems)
|
|
{
|
|
foreach (UpdateMultiCollectionItem incoming in request.Items.Filter(i =>
|
|
i.SmartCollectionId == item.SmartCollectionId))
|
|
{
|
|
item.ScheduleAsGroup = incoming.ScheduleAsGroup;
|
|
item.PlaybackOrder = incoming.PlaybackOrder;
|
|
item.Weight = incoming.Weight;
|
|
}
|
|
}
|
|
|
|
// add new items
|
|
c.MultiCollectionSmartItems.AddRange(toAddSmart);
|
|
|
|
// rebuild playouts
|
|
if (await dbContext.SaveChangesAsync(cancellationToken) > 0)
|
|
{
|
|
_searchTargets.SearchTargetsChanged();
|
|
|
|
// refresh all playouts that use this collection
|
|
// post-commit side effect runs on CancellationToken.None so a late request cancellation
|
|
// can't abort it after the commit landed (#254)
|
|
foreach (int playoutId in await _mediaCollectionRepository.PlayoutIdsUsingMultiCollection(
|
|
request.MultiCollectionId))
|
|
{
|
|
await _channel.WriteAsync(new BuildPlayout(playoutId, PlayoutBuildMode.Refresh), CancellationToken.None);
|
|
}
|
|
}
|
|
|
|
return Unit.Default;
|
|
}
|
|
|
|
private static async Task<Validation<BaseError, MultiCollection>> Validate(
|
|
TvContext dbContext,
|
|
UpdateMultiCollection request,
|
|
CancellationToken cancellationToken) =>
|
|
(await MultiCollectionMustExist(dbContext, request, cancellationToken),
|
|
await ValidateName(dbContext, request),
|
|
ValidateWeights(request))
|
|
.Apply((collectionToUpdate, _, _) => collectionToUpdate);
|
|
|
|
// Bounds are shared with the create path so the two cannot drift. This gate matters more here than on
|
|
// create: EF's HasDefaultValue substitutes 1 for a 0 on INSERT (0 reads as "not set"), but an UPDATE
|
|
// writes the 0 through -- and a 0-weight source is filtered out of the rotation, deleting it from the
|
|
// channel with nothing reported. See #70.
|
|
private static Validation<BaseError, Unit> ValidateWeights(UpdateMultiCollection request) =>
|
|
request.Items.All(i => MultiCollectionItemWeight.IsValid(i.Weight))
|
|
? Unit.Default
|
|
: BaseError.New(MultiCollectionItemWeight.ValidationMessage);
|
|
|
|
private static Task<Validation<BaseError, MultiCollection>> MultiCollectionMustExist(
|
|
TvContext dbContext,
|
|
UpdateMultiCollection updateCollection,
|
|
CancellationToken cancellationToken) =>
|
|
dbContext.MultiCollections
|
|
.Include(mc => mc.MultiCollectionItems)
|
|
.Include(mc => mc.MultiCollectionSmartItems)
|
|
.SelectOneAsync(c => c.Id, c => c.Id == updateCollection.MultiCollectionId, cancellationToken)
|
|
.Map(o => o.ToValidation<BaseError>("MultiCollection does not exist."));
|
|
|
|
private static async Task<Validation<BaseError, string>> ValidateName(
|
|
TvContext dbContext,
|
|
UpdateMultiCollection updateMultiCollection)
|
|
{
|
|
Validation<BaseError, string> result1 = updateMultiCollection.NotEmpty(c => c.Name)
|
|
.Bind(_ => updateMultiCollection.NotLongerThan(50)(c => c.Name));
|
|
|
|
bool duplicateName = await dbContext.MultiCollections
|
|
.AnyAsync(c => c.Id != updateMultiCollection.MultiCollectionId && c.Name == updateMultiCollection.Name);
|
|
|
|
Validation<BaseError, Unit> result2 = duplicateName
|
|
? Fail<BaseError, Unit>("MultiCollection name must be unique")
|
|
: Success<BaseError, Unit>(Unit.Default);
|
|
|
|
return (result1, result2).Apply((_, _) => updateMultiCollection.Name);
|
|
}
|
|
}
|