diff --git a/DeepDrftAPI/Controllers/TrackController.cs b/DeepDrftAPI/Controllers/TrackController.cs index 379b1b8..fb0fde9 100644 --- a/DeepDrftAPI/Controllers/TrackController.cs +++ b/DeepDrftAPI/Controllers/TrackController.cs @@ -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); } diff --git a/DeepDrftAPI/Services/UnifiedTrackService.cs b/DeepDrftAPI/Services/UnifiedTrackService.cs index a568db8..d0718ef 100644 --- a/DeepDrftAPI/Services/UnifiedTrackService.cs +++ b/DeepDrftAPI/Services/UnifiedTrackService.cs @@ -17,6 +17,14 @@ namespace DeepDrftAPI.Services; public class UnifiedTrackService { internal const string TrackNotFoundMessage = "Track not found."; + + /// + /// 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. + /// + 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.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.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); diff --git a/DeepDrftData/ITrackService.cs b/DeepDrftData/ITrackService.cs index 007b512..9650f6d 100644 --- a/DeepDrftData/ITrackService.cs +++ b/DeepDrftData/ITrackService.cs @@ -36,6 +36,15 @@ public interface ITrackService Task> FindOrCreateRelease( string title, string artist, ReleaseDto releaseData, CancellationToken cancellationToken = default); + /// + /// 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. + /// + Task> GetReleaseByTitleAndArtist( + string title, string artist, CancellationToken cancellationToken = default); + Task> Create(TrackDto newTrack); Task> Update(TrackDto track); Task Delete(long id); diff --git a/DeepDrftData/TrackManager.cs b/DeepDrftData/TrackManager.cs index 986f73a..108e228 100644 --- a/DeepDrftData/TrackManager.cs +++ b/DeepDrftData/TrackManager.cs @@ -201,6 +201,25 @@ public class TrackManager } } + public async Task> GetReleaseByTitleAndArtist( + string title, string artist, CancellationToken cancellationToken = default) + { + try + { + var existing = await Repository.GetReleaseByTitleAndArtistAsync(title, artist, cancellationToken); + if (existing is null) + return ResultContainer.CreatePassResult(null); + + var dto = TrackConverter.Convert(existing); + dto.TrackCount = await Repository.CountLiveTracksByReleaseAsync(existing.Id, cancellationToken); + return ResultContainer.CreatePassResult(dto); + } + catch (Exception e) + { + return ResultContainer.CreateFailResult(e.Message); + } + } + public async Task>> GetDistinctGenres(CancellationToken cancellationToken = default) { try diff --git a/DeepDrftManager/Components/Pages/Tracks/BatchEdit.razor b/DeepDrftManager/Components/Pages/Tracks/BatchEdit.razor index e9bbdf2..38ac5ed 100644 --- a/DeepDrftManager/Components/Pages/Tracks/BatchEdit.razor +++ b/DeepDrftManager/Components/Pages/Tracks/BatchEdit.razor @@ -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); } diff --git a/DeepDrftManager/Components/Pages/Tracks/BatchUpload.razor b/DeepDrftManager/Components/Pages/Tracks/BatchUpload.razor index ab8e880..3611188 100644 --- a/DeepDrftManager/Components/Pages/Tracks/BatchUpload.razor +++ b/DeepDrftManager/Components/Pages/Tracks/BatchUpload.razor @@ -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; diff --git a/DeepDrftModels/Enums/MediumRules.cs b/DeepDrftModels/Enums/MediumRules.cs new file mode 100644 index 0000000..d623214 --- /dev/null +++ b/DeepDrftModels/Enums/MediumRules.cs @@ -0,0 +1,35 @@ +namespace DeepDrftModels.Enums; + +/// +/// The allowed track-count range for a , expressed as an inclusive +/// [Min, Max] band. Max == int.MaxValue denotes an unbounded (many-track) medium. +/// +public readonly record struct MediumCardinality(int Min, int Max) +{ + /// True when falls within the inclusive band. + public bool Allows(int trackCount) => trackCount >= Min && trackCount <= Max; + + /// True when the medium permits exactly one track (Max capped at 1). + public bool IsSingleTrack => Max == 1; +} + +/// +/// 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. +/// +public static class MediumRules +{ + private static readonly IReadOnlyDictionary Cardinalities = + new Dictionary + { + [ReleaseMedium.Cut] = new(1, int.MaxValue), + [ReleaseMedium.Session] = new(1, 1), + [ReleaseMedium.Mix] = new(1, 1), + }; + + /// The declared track-count band for . + public static MediumCardinality CardinalityOf(ReleaseMedium medium) => Cardinalities[medium]; +} diff --git a/DeepDrftTests/MediumWritePathTests.cs b/DeepDrftTests/MediumWritePathTests.cs index bb43897..a282c63 100644 --- a/DeepDrftTests/MediumWritePathTests.cs +++ b/DeepDrftTests/MediumWritePathTests.cs @@ -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"); + } }