Block duplicate-release uploads by (title, artist): pre-flight check + server 409 backstop, with within-batch Cut attach via releaseId
This commit is contained in:
@@ -25,6 +25,16 @@ public class UnifiedTrackService
|
||||
/// follows the marker and is what the CMS surfaces to the admin.
|
||||
/// </summary>
|
||||
internal const string CardinalityViolationMarker = "CARDINALITY_VIOLATION: ";
|
||||
|
||||
/// <summary>
|
||||
/// Stable marker prefixed onto a duplicate-release rejection so the controller can map it to 409
|
||||
/// Conflict, the same way <see cref="CardinalityViolationMarker"/> is mapped. Fires when an upload
|
||||
/// with no explicit releaseId would create a release whose (title, artist) already exists in the
|
||||
/// catalogue — the upload form is a create-new tool, never an edit/append path. The human-readable
|
||||
/// detail follows the marker and is what the CMS surfaces to the admin.
|
||||
/// </summary>
|
||||
internal const string DuplicateReleaseMarker = "DUPLICATE_RELEASE: ";
|
||||
|
||||
private readonly TrackContentService _contentTrackContentService;
|
||||
private readonly ITrackService _sqlTrackService;
|
||||
private readonly FileDb _fileDatabase;
|
||||
@@ -64,33 +74,66 @@ public class UnifiedTrackService
|
||||
ReleaseType releaseType,
|
||||
ReleaseMedium medium,
|
||||
int trackNumber,
|
||||
long? releaseId,
|
||||
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.
|
||||
// Resolve which release this track lands on BEFORE the vault write, so a rejected upload never
|
||||
// orphans audio. Two paths:
|
||||
// - releaseId is null → CREATE path: this is the first row of a submit. (title, artist) must
|
||||
// NOT already exist — the upload form creates new releases only. A pre-existing match is a
|
||||
// duplicate and is blocked (409).
|
||||
// - releaseId is set → ATTACH path: rows 2..N of a within-batch multi-track Cut, attaching
|
||||
// to the release row 1 just created. No (title, artist) lookup — the release id is
|
||||
// authoritative — so the within-batch build is never mistaken for a pre-existing duplicate.
|
||||
// Both paths run the cardinality guard `(liveCount + 1) > Max` (not Session/Mix-hardcoded, so a
|
||||
// future bounded medium is covered by the same line).
|
||||
ResolvedRelease? resolved = null;
|
||||
if (!string.IsNullOrWhiteSpace(album))
|
||||
{
|
||||
var peek = await _sqlTrackService.GetReleaseByTitleAndArtist(album, artist, ct);
|
||||
if (!peek.Success)
|
||||
if (releaseId is { } attachId)
|
||||
{
|
||||
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}");
|
||||
}
|
||||
var attachPeek = await _sqlTrackService.GetReleaseByTitleAndArtist(album, artist, ct);
|
||||
if (!attachPeek.Success)
|
||||
{
|
||||
var error = attachPeek.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)
|
||||
// The attach target must be the same release the natural key resolves to — a guard against
|
||||
// a stale/forged releaseId pointing at a different (title, artist) than this row carries.
|
||||
if (attachPeek.Value is not { } target || target.Id != attachId)
|
||||
{
|
||||
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.");
|
||||
$"{DuplicateReleaseMarker}The release this track should attach to could not be found. " +
|
||||
"Start the upload again.");
|
||||
}
|
||||
|
||||
var cardinalityCheck = CheckCardinality(target);
|
||||
if (cardinalityCheck is { } violation)
|
||||
return ResultContainer<TrackDto>.CreateFailResult(violation);
|
||||
|
||||
resolved = new ResolvedRelease(target.Id);
|
||||
}
|
||||
else
|
||||
{
|
||||
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}");
|
||||
}
|
||||
|
||||
// CREATE path: a pre-existing (title, artist) is a duplicate. Block it — the form never
|
||||
// edits or appends to an existing release.
|
||||
if (peek.Value is { } existing)
|
||||
{
|
||||
return ResultContainer<TrackDto>.CreateFailResult(
|
||||
$"{DuplicateReleaseMarker}A release titled '{existing.Title}' by {existing.Artist} already " +
|
||||
"exists. The upload form creates new releases only — use the edit tools to change an existing one.");
|
||||
}
|
||||
// resolved stays null → FindOrCreateRelease below creates the release.
|
||||
}
|
||||
}
|
||||
|
||||
@@ -109,9 +152,12 @@ public class UnifiedTrackService
|
||||
// shared release (created on first sighting); an upload without one stays a loose track with
|
||||
// a null ReleaseId. Release-cardinal metadata (artist/genre/description/releaseDate/type/uploader)
|
||||
// rides on the release, not the track.
|
||||
long? releaseId = null;
|
||||
if (!string.IsNullOrWhiteSpace(album))
|
||||
long? resolvedReleaseId = resolved?.Id;
|
||||
if (!string.IsNullOrWhiteSpace(album) && resolvedReleaseId is null)
|
||||
{
|
||||
// CREATE path only: the duplicate guard above proved no (title, artist) match exists, so this
|
||||
// mints the release. (The attach path already resolved the id from the pre-check above and
|
||||
// skips FindOrCreateRelease entirely, so a within-batch row never re-runs the natural-key find.)
|
||||
var releaseData = new ReleaseDto
|
||||
{
|
||||
Title = album,
|
||||
@@ -124,11 +170,9 @@ public class UnifiedTrackService
|
||||
CreatedByUserId = createdByUserId,
|
||||
};
|
||||
|
||||
// Medium (like every other field in releaseData) applies only when this upload CREATES the
|
||||
// release. FindOrCreateRelease returns an existing (title, artist) row untouched — the first
|
||||
// upload's medium is authoritative. Do NOT "fix" this to overwrite the stored medium on a
|
||||
// subsequent track add: medium is a release-level property, changed only via the edit path
|
||||
// (PUT api/track/meta), never silently flipped by adding a track to an existing release.
|
||||
// FindOrCreateRelease's find branch still backstops a concurrent insert of the same
|
||||
// (title, artist) between the duplicate peek and this call — it returns the winning row
|
||||
// rather than throwing. Medium and every other field apply only on the create it performs.
|
||||
var releaseResult = await _sqlTrackService.FindOrCreateRelease(album, artist, releaseData, ct);
|
||||
if (!releaseResult.Success || releaseResult.Value is null)
|
||||
{
|
||||
@@ -139,11 +183,11 @@ public class UnifiedTrackService
|
||||
return ResultContainer<TrackDto>.CreateFailResult($"Track was uploaded but could not be saved: {error}");
|
||||
}
|
||||
|
||||
releaseId = releaseResult.Value.Id;
|
||||
resolvedReleaseId = releaseResult.Value.Id;
|
||||
}
|
||||
|
||||
var trackDto = TrackConverter.Convert(unpersisted);
|
||||
trackDto.ReleaseId = releaseId;
|
||||
trackDto.ReleaseId = resolvedReleaseId;
|
||||
trackDto.Release = null; // FK already resolved; Create must not re-resolve a detached graph.
|
||||
|
||||
var saveResult = await _sqlTrackService.Create(trackDto);
|
||||
@@ -166,6 +210,26 @@ public class UnifiedTrackService
|
||||
return saveResult;
|
||||
}
|
||||
|
||||
// The release a track resolved onto before the vault write. A null Id is the create path (mint
|
||||
// below); a non-null Id is the attach path (a within-batch multi-track Cut row 2..N).
|
||||
private readonly record struct ResolvedRelease(long Id);
|
||||
|
||||
// The cardinality guard shared by the attach path and (historically) the create path: a release
|
||||
// already at its medium's Max rejects a further track. Returns the marker-prefixed rejection
|
||||
// message, or null when the add is within limits. The create path never trips this (a brand-new
|
||||
// release has zero tracks and admits its first), so only the attach path calls it today.
|
||||
private static string? CheckCardinality(ReleaseDto release)
|
||||
{
|
||||
var cardinality = MediumRules.CardinalityOf(release.Medium);
|
||||
if (release.TrackCount + 1 > cardinality.Max)
|
||||
{
|
||||
return $"{CardinalityViolationMarker}A {release.Medium} release holds a single track; " +
|
||||
$"'{release.Title}' already has one — edit the existing track or choose a different release.";
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Replace an existing track's audio in place: look up the SQL row, swap only the vault bytes
|
||||
/// keyed by its EntryKey, regenerate both waveform datums from the new audio, then write the
|
||||
|
||||
Reference in New Issue
Block a user