Files
ersatztv/ErsatzTV.Application/MediaCollections/Commands/UpdateMultiCollectionHandler.cs
T
ccef0ca88a fix(70): test the weight gate; correct rationale my own clamp made false
Re-review of the fix commit returned MERGEABLE-WITH-NITS. It verified the gate is
now complete by enumerating the writers itself (no fourth persisting writer) and
proved B2's fix works by writing throwaway handler tests — which was also its
point: the fix shipped with none.

B2 was create and update silently DISAGREEING on the same input, and the fix
re-established agreement with nothing pinning it. Both paths are now driven from
one shared case list, plus an explicit test that create and update agree on every
case — the per-path tests would both have passed while the two diverged, which is
how the bug existed in the first place. Non-vacuity proven: inverting only the
update path's validation fails 10 of 20 on a clean build (0 errors, so not a
stale-dll pass), and the agreement test is among the failures.

The rest is my own prose contradicting my own code. The commit that added
EffectiveWeight removed the weight filter, then left four statements asserting a
0-weight source "is filtered out" — two of them authored by that same commit,
including the stated justification for Minimum=1 in MultiCollectionItemWeight.
A future agent could have read that and deleted the clamp or the floor as
redundant; they are belt-and-braces and neither is. Corrected to describe what the
code now does: the gate refuses input that means nothing on a share-of-airtime
scale, the clamp protects rows predating the gate.

Also corrected the writer count in the very bullet whose lesson is "grep every
writer of the field": ReplaceBlockItems writes BlockItem.PlaybackOrder, not
PlaylistItem.PlaybackOrder. There are TWO persisting writers of PlaylistItem's,
and the correction itself had miscounted by conflating the two fields — so the
lesson now says to grep each field separately.

Core.Tests 565 passed, ErsatzTV.Tests 1673 passed, 0 failed.

Refs #70

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-17 17:00:46 +00:00

205 lines
9.2 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 -- they silently disagreed before #402:
// EF's HasDefaultValue substitutes 1 for a 0 on INSERT (0 reads as "not set"), but an UPDATE writes the 0
// through, so the same input landed differently depending on the verb. The enumerator clamps out-of-range
// weights, so a 0 no longer removes the source; this gate is about refusing input that has no meaning on a
// share-of-airtime scale, and about keeping create and update honest with each other. 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);
}
}