Merge p9-w7-cardinality-invariant into dev (9.7 per-medium cardinality invariant)
This commit is contained in:
@@ -295,6 +295,15 @@ public class TrackController : ControllerBase
|
||||
{
|
||||
var error = result.Messages.FirstOrDefault()?.Message ?? "Failed to process and store audio";
|
||||
_logger.LogWarning("UploadTrack: UnifiedTrackService failed for {TrackName}: {Error}", trackName, error);
|
||||
|
||||
// A cardinality rejection is a well-formed request that violates a domain rule, so it
|
||||
// is 409 Conflict — distinct from the 500 used for processing failure. The marker is
|
||||
// stripped so the client sees only the human-readable detail.
|
||||
if (error.StartsWith(UnifiedTrackService.CardinalityViolationMarker, StringComparison.Ordinal))
|
||||
{
|
||||
return Conflict(error[UnifiedTrackService.CardinalityViolationMarker.Length..]);
|
||||
}
|
||||
|
||||
return StatusCode(500, error);
|
||||
}
|
||||
|
||||
|
||||
@@ -17,6 +17,14 @@ namespace DeepDrftAPI.Services;
|
||||
public class UnifiedTrackService
|
||||
{
|
||||
internal const string TrackNotFoundMessage = "Track not found.";
|
||||
|
||||
/// <summary>
|
||||
/// Stable marker prefixed onto a cardinality-rejection message so the controller can map this
|
||||
/// specific failure to 409 Conflict (a well-formed request that violates a domain rule),
|
||||
/// distinct from the 400 (malformed) and 500 (processing) paths. The human-readable detail
|
||||
/// follows the marker and is what the CMS surfaces to the admin.
|
||||
/// </summary>
|
||||
internal const string CardinalityViolationMarker = "CARDINALITY_VIOLATION: ";
|
||||
private readonly TrackContentService _contentTrackContentService;
|
||||
private readonly ITrackService _sqlTrackService;
|
||||
private readonly FileDb _fileDatabase;
|
||||
@@ -57,6 +65,34 @@ public class UnifiedTrackService
|
||||
int trackNumber,
|
||||
CancellationToken ct)
|
||||
{
|
||||
// Cardinality pre-check — BEFORE the vault write so a rejected over-limit add never orphans
|
||||
// audio in the tracks vault. This is a READ-only peek (no release is created for an upload we
|
||||
// may reject); the real FindOrCreateRelease still runs below for the accepted path. Only the
|
||||
// find path can violate: a release that does not yet exist has zero tracks and admits its
|
||||
// first. The guard is the general form `(liveCount + 1) > Max`, not Session/Mix-hardcoded, so
|
||||
// a future bounded medium is covered by the same line.
|
||||
if (!string.IsNullOrWhiteSpace(album))
|
||||
{
|
||||
var peek = await _sqlTrackService.GetReleaseByTitleAndArtist(album, artist, ct);
|
||||
if (!peek.Success)
|
||||
{
|
||||
var error = peek.Messages.FirstOrDefault()?.Message ?? "Unknown error";
|
||||
_logger.LogError("UploadAsync: release peek failed for ({Album}, {Artist}): {Error}", album, artist, error);
|
||||
return ResultContainer<TrackDto>.CreateFailResult($"Could not verify the release: {error}");
|
||||
}
|
||||
|
||||
if (peek.Value is { } existing)
|
||||
{
|
||||
var cardinality = MediumRules.CardinalityOf(existing.Medium);
|
||||
if (existing.TrackCount + 1 > cardinality.Max)
|
||||
{
|
||||
return ResultContainer<TrackDto>.CreateFailResult(
|
||||
$"{CardinalityViolationMarker}A {existing.Medium} release holds a single track; " +
|
||||
$"'{existing.Title}' already has one — edit the existing track or choose a different release.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var unpersisted = await _contentTrackContentService.AddTrackAsync(
|
||||
tempFilePath, trackName, artist, album, genre, releaseDate, originalFileName: originalFileName);
|
||||
|
||||
|
||||
@@ -36,6 +36,15 @@ public interface ITrackService
|
||||
Task<ResultContainer<ReleaseDto>> FindOrCreateRelease(
|
||||
string title, string artist, ReleaseDto releaseData, CancellationToken cancellationToken = default);
|
||||
|
||||
/// <summary>
|
||||
/// Read-only peek for an existing release by its natural key, or null when none exists — a find
|
||||
/// with no create side-effect. Backs the upload cardinality pre-check, which must read a release's
|
||||
/// medium and live-track count before deciding whether to admit an upload, without creating a
|
||||
/// release for an upload it may reject. The returned DTO carries TrackCount.
|
||||
/// </summary>
|
||||
Task<ResultContainer<ReleaseDto?>> GetReleaseByTitleAndArtist(
|
||||
string title, string artist, CancellationToken cancellationToken = default);
|
||||
|
||||
Task<ResultContainer<TrackDto>> Create(TrackDto newTrack);
|
||||
Task<ResultContainer<TrackDto>> Update(TrackDto track);
|
||||
Task<Result> Delete(long id);
|
||||
|
||||
@@ -201,6 +201,25 @@ public class TrackManager
|
||||
}
|
||||
}
|
||||
|
||||
public async Task<ResultContainer<ReleaseDto?>> GetReleaseByTitleAndArtist(
|
||||
string title, string artist, CancellationToken cancellationToken = default)
|
||||
{
|
||||
try
|
||||
{
|
||||
var existing = await Repository.GetReleaseByTitleAndArtistAsync(title, artist, cancellationToken);
|
||||
if (existing is null)
|
||||
return ResultContainer<ReleaseDto?>.CreatePassResult(null);
|
||||
|
||||
var dto = TrackConverter.Convert(existing);
|
||||
dto.TrackCount = await Repository.CountLiveTracksByReleaseAsync(existing.Id, cancellationToken);
|
||||
return ResultContainer<ReleaseDto?>.CreatePassResult(dto);
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
return ResultContainer<ReleaseDto?>.CreateFailResult(e.Message);
|
||||
}
|
||||
}
|
||||
|
||||
public async Task<ResultContainer<List<GenreSummaryDto>>> GetDistinctGenres(CancellationToken cancellationToken = default)
|
||||
{
|
||||
try
|
||||
|
||||
@@ -136,14 +136,15 @@
|
||||
// UploadTrackAsync call below passes _medium, and PUT api/track/meta resets ReleaseType to its
|
||||
// default server-side for a non-Cut medium.
|
||||
//
|
||||
// Switching to a single-track medium (Session/Mix) collapses any multi-track list to the first row
|
||||
// so the single-track invariant (§9.3) holds before save — the same collapse BatchUpload.OnMediumChanged
|
||||
// performs. Dropping rows 2..n is an in-memory trim only; existing tracks are not deleted server-side
|
||||
// (RemoveRow owns deletion), so the hidden rows simply fall out of this edit session.
|
||||
// Switching to a single-track medium collapses any multi-track list to the first row so the
|
||||
// single-track invariant (§9.3) holds before save — the same collapse BatchUpload.OnMediumChanged
|
||||
// performs, reading the same MediumRules cardinality the upload service enforces. Dropping rows
|
||||
// 2..n is an in-memory trim only; existing tracks are not deleted server-side (RemoveRow owns
|
||||
// deletion), so the hidden rows simply fall out of this edit session.
|
||||
private void OnMediumChanged(ReleaseMedium medium)
|
||||
{
|
||||
_medium = medium;
|
||||
if (medium != ReleaseMedium.Cut && _tracks.Count > 1)
|
||||
if (MediumRules.CardinalityOf(medium).IsSingleTrack && _tracks.Count > 1)
|
||||
{
|
||||
_tracks.RemoveRange(1, _tracks.Count - 1);
|
||||
_selectedIndex = _tracks.Count > 0 ? 0 : -1;
|
||||
@@ -194,7 +195,9 @@
|
||||
Status = BatchRowStatus.Queued
|
||||
}).ToList();
|
||||
|
||||
if (_medium != ReleaseMedium.Cut && _tracks.Count > 1)
|
||||
// Same single-track collapse on the load path, via the shared MediumRules declaration: a
|
||||
// release whose stored medium is single-track surfaces only its first row for editing.
|
||||
if (MediumRules.CardinalityOf(_medium).IsSingleTrack && _tracks.Count > 1)
|
||||
{
|
||||
_tracks.RemoveRange(1, _tracks.Count - 1);
|
||||
}
|
||||
|
||||
@@ -118,12 +118,13 @@
|
||||
private ReleaseType _releaseType = ReleaseType.Single;
|
||||
private ReleaseMedium _medium = ReleaseMedium.Cut;
|
||||
|
||||
// Switching to a single-track medium (Session/Mix) collapses any multi-track selection to the
|
||||
// first row so the single-track invariant holds before submit. Switching back to Cut keeps it.
|
||||
// Switching to a single-track medium collapses any multi-track selection to the first row so the
|
||||
// single-track invariant holds before submit. The predicate reads the same MediumRules cardinality
|
||||
// declaration the upload service enforces, so the form and the domain cannot drift.
|
||||
private void OnMediumChanged(ReleaseMedium medium)
|
||||
{
|
||||
_medium = medium;
|
||||
if (medium != ReleaseMedium.Cut && _tracks.Count > 1)
|
||||
if (MediumRules.CardinalityOf(medium).IsSingleTrack && _tracks.Count > 1)
|
||||
{
|
||||
_tracks.RemoveRange(1, _tracks.Count - 1);
|
||||
_selectedIndex = _tracks.Count > 0 ? 0 : -1;
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
namespace DeepDrftModels.Enums;
|
||||
|
||||
/// <summary>
|
||||
/// The allowed track-count range for a <see cref="ReleaseMedium"/>, expressed as an inclusive
|
||||
/// [Min, Max] band. <c>Max == int.MaxValue</c> denotes an unbounded (many-track) medium.
|
||||
/// </summary>
|
||||
public readonly record struct MediumCardinality(int Min, int Max)
|
||||
{
|
||||
/// <summary>True when <paramref name="trackCount"/> falls within the inclusive band.</summary>
|
||||
public bool Allows(int trackCount) => trackCount >= Min && trackCount <= Max;
|
||||
|
||||
/// <summary>True when the medium permits exactly one track (Max capped at 1).</summary>
|
||||
public bool IsSingleTrack => Max == 1;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Single source of truth for per-medium structural rules. Today it declares track cardinality;
|
||||
/// the same table is read by the upload service (to reject over-limit track-adds) and the CMS
|
||||
/// batch forms (to decide whether to collapse the master list to one row), so the two cannot
|
||||
/// drift. A future medium declares its cardinality here — and only here — and every consumer
|
||||
/// honours it automatically. Pure declaration, no dependencies.
|
||||
/// </summary>
|
||||
public static class MediumRules
|
||||
{
|
||||
private static readonly IReadOnlyDictionary<ReleaseMedium, MediumCardinality> Cardinalities =
|
||||
new Dictionary<ReleaseMedium, MediumCardinality>
|
||||
{
|
||||
[ReleaseMedium.Cut] = new(1, int.MaxValue),
|
||||
[ReleaseMedium.Session] = new(1, 1),
|
||||
[ReleaseMedium.Mix] = new(1, 1),
|
||||
};
|
||||
|
||||
/// <summary>The declared track-count band for <paramref name="medium"/>.</summary>
|
||||
public static MediumCardinality CardinalityOf(ReleaseMedium medium) => Cardinalities[medium];
|
||||
}
|
||||
@@ -213,4 +213,144 @@ public class MediumWritePathTests
|
||||
Assert.That(new TrackFilter { ReleaseId = 5 }.IsEmpty, Is.False);
|
||||
Assert.That(new TrackFilter().IsEmpty, Is.True);
|
||||
}
|
||||
|
||||
// 9.7 — the cardinality declaration is the single source of truth read by both the upload service
|
||||
// and the CMS form collapse. Guard the declared ranges so a drift in MediumRules is caught here.
|
||||
[Test]
|
||||
public void MediumRules_CardinalityOf_DeclaresExpectedRanges()
|
||||
{
|
||||
var cut = MediumRules.CardinalityOf(ReleaseMedium.Cut);
|
||||
Assert.That(cut.Min, Is.EqualTo(1));
|
||||
Assert.That(cut.Max, Is.EqualTo(int.MaxValue));
|
||||
Assert.That(cut.IsSingleTrack, Is.False);
|
||||
|
||||
var session = MediumRules.CardinalityOf(ReleaseMedium.Session);
|
||||
Assert.That(session.Min, Is.EqualTo(1));
|
||||
Assert.That(session.Max, Is.EqualTo(1));
|
||||
Assert.That(session.IsSingleTrack, Is.True);
|
||||
|
||||
var mix = MediumRules.CardinalityOf(ReleaseMedium.Mix);
|
||||
Assert.That(mix.Min, Is.EqualTo(1));
|
||||
Assert.That(mix.Max, Is.EqualTo(1));
|
||||
Assert.That(mix.IsSingleTrack, Is.True);
|
||||
}
|
||||
|
||||
// 9.7 — Allows() bands. The first track always fits; a single-track medium rejects the second; an
|
||||
// unbounded medium accepts any positive count.
|
||||
[Test]
|
||||
public void MediumCardinality_Allows_HonoursTheBand()
|
||||
{
|
||||
var single = MediumRules.CardinalityOf(ReleaseMedium.Session);
|
||||
Assert.That(single.Allows(1), Is.True);
|
||||
Assert.That(single.Allows(2), Is.False);
|
||||
|
||||
var many = MediumRules.CardinalityOf(ReleaseMedium.Cut);
|
||||
Assert.That(many.Allows(1), Is.True);
|
||||
Assert.That(many.Allows(50), Is.True);
|
||||
}
|
||||
|
||||
// 9.7 — GetReleaseByTitleAndArtist is the read-only peek the upload pre-check reads. It surfaces
|
||||
// the stored medium and the live-track count without creating a release. Null on miss.
|
||||
[Test]
|
||||
public async Task GetReleaseByTitleAndArtist_ExistingRelease_ReturnsMediumAndLiveCount()
|
||||
{
|
||||
var repo = CreateRepository();
|
||||
ITrackService manager = CreateManager(repo);
|
||||
|
||||
var release = new ReleaseEntity { Title = "Live at the Vault", Artist = "Artist A", Medium = ReleaseMedium.Session };
|
||||
_context.Tracks.Add(new TrackEntity { EntryKey = "ek-1", TrackName = "Track One", Release = release });
|
||||
await _context.SaveChangesAsync();
|
||||
|
||||
var peek = await manager.GetReleaseByTitleAndArtist("Live at the Vault", "Artist A");
|
||||
|
||||
Assert.That(peek.Success, Is.True);
|
||||
Assert.That(peek.Value, Is.Not.Null);
|
||||
Assert.That(peek.Value!.Medium, Is.EqualTo(ReleaseMedium.Session));
|
||||
Assert.That(peek.Value.TrackCount, Is.EqualTo(1));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task GetReleaseByTitleAndArtist_NoSuchRelease_ReturnsNullWithoutCreating()
|
||||
{
|
||||
ITrackService manager = CreateManager(CreateRepository());
|
||||
|
||||
var peek = await manager.GetReleaseByTitleAndArtist("Nothing Here", "Nobody");
|
||||
|
||||
Assert.That(peek.Success, Is.True);
|
||||
Assert.That(peek.Value, Is.Null);
|
||||
|
||||
// The peek must not have created a release for a non-existent (title, artist).
|
||||
var releases = (await manager.GetReleases()).Value!;
|
||||
Assert.That(releases, Is.Empty);
|
||||
}
|
||||
|
||||
// 9.7 — the cardinality decision the orchestrator makes, asserted over the SQL-layer seam it reads.
|
||||
// A Session release that already holds its single track REJECTS a second track-add: the peek's
|
||||
// live count + 1 exceeds the medium's Max. (The orchestrator's vault write spans the FileDatabase,
|
||||
// not reachable from this in-memory fixture — see the orphan-avoidance note in the handoff.)
|
||||
[Test]
|
||||
public async Task CardinalityDecision_SessionWithOneTrack_RejectsSecondAdd()
|
||||
{
|
||||
var repo = CreateRepository();
|
||||
ITrackService manager = CreateManager(repo);
|
||||
|
||||
var release = new ReleaseEntity { Title = "Live at the Vault", Artist = "Artist A", Medium = ReleaseMedium.Session };
|
||||
_context.Tracks.Add(new TrackEntity { EntryKey = "ek-1", TrackName = "Track One", Release = release });
|
||||
await _context.SaveChangesAsync();
|
||||
|
||||
var peek = (await manager.GetReleaseByTitleAndArtist("Live at the Vault", "Artist A")).Value!;
|
||||
var max = MediumRules.CardinalityOf(peek.Medium).Max;
|
||||
|
||||
Assert.That(peek.TrackCount + 1 > max, Is.True, "second track-add to a single-track Session is over-limit");
|
||||
}
|
||||
|
||||
// 9.7 — Mix mirrors Session: a Mix holding its one track rejects a second add.
|
||||
[Test]
|
||||
public async Task CardinalityDecision_MixWithOneTrack_RejectsSecondAdd()
|
||||
{
|
||||
var repo = CreateRepository();
|
||||
ITrackService manager = CreateManager(repo);
|
||||
|
||||
var release = new ReleaseEntity { Title = "Sunset Set", Artist = "DJ B", Medium = ReleaseMedium.Mix };
|
||||
_context.Tracks.Add(new TrackEntity { EntryKey = "ek-1", TrackName = "The Set", Release = release });
|
||||
await _context.SaveChangesAsync();
|
||||
|
||||
var peek = (await manager.GetReleaseByTitleAndArtist("Sunset Set", "DJ B")).Value!;
|
||||
var max = MediumRules.CardinalityOf(peek.Medium).Max;
|
||||
|
||||
Assert.That(peek.TrackCount + 1 > max, Is.True);
|
||||
}
|
||||
|
||||
// 9.7 — a Cut release accepts the 2nd and Nth track-add: the unbounded Max is never exceeded.
|
||||
[Test]
|
||||
public async Task CardinalityDecision_CutWithManyTracks_AcceptsFurtherAdds()
|
||||
{
|
||||
var repo = CreateRepository();
|
||||
ITrackService manager = CreateManager(repo);
|
||||
|
||||
var release = new ReleaseEntity { Title = "Studio Album", Artist = "Artist C", Medium = ReleaseMedium.Cut };
|
||||
_context.Tracks.AddRange(
|
||||
new TrackEntity { EntryKey = "c1", TrackName = "One", Release = release },
|
||||
new TrackEntity { EntryKey = "c2", TrackName = "Two", Release = release },
|
||||
new TrackEntity { EntryKey = "c3", TrackName = "Three", Release = release });
|
||||
await _context.SaveChangesAsync();
|
||||
|
||||
var peek = (await manager.GetReleaseByTitleAndArtist("Studio Album", "Artist C")).Value!;
|
||||
var max = MediumRules.CardinalityOf(peek.Medium).Max;
|
||||
|
||||
Assert.That(peek.TrackCount, Is.EqualTo(3));
|
||||
Assert.That(peek.TrackCount + 1 > max, Is.False, "Cut is unbounded — a 4th track is admitted");
|
||||
}
|
||||
|
||||
// 9.7 — the first track on a new Session/Mix succeeds: with no existing release the peek returns
|
||||
// null, so the pre-check never fires and the create path admits the 0→1 add (within 1..1).
|
||||
[Test]
|
||||
public async Task CardinalityDecision_FirstTrackOnNewSession_IsAdmitted()
|
||||
{
|
||||
ITrackService manager = CreateManager(CreateRepository());
|
||||
|
||||
// No release exists yet for this (title, artist).
|
||||
var peek = await manager.GetReleaseByTitleAndArtist("Brand New Session", "Artist D");
|
||||
Assert.That(peek.Value, Is.Null, "no release means the cardinality pre-check is skipped — the create path admits the first track");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user