Merge upload-duplicate-detection into dev (block duplicate-release uploads by title+artist)
This commit is contained in:
@@ -96,6 +96,37 @@ public class TrackController : ControllerBase
|
|||||||
return Ok(result.Value);
|
return Ok(result.Value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GET api/track/release/exists?title=...&artist=... ([ApiKeyAuthorize])
|
||||||
|
// Upload-form pre-flight: does a release with this exact (title, artist) already exist? Returns the
|
||||||
|
// matching ReleaseDto (so the caller can name it in the block message) or 404 when none exists. Uses
|
||||||
|
// the same GetReleaseByTitleAndArtist read the upload create-path duplicate guard uses, so the
|
||||||
|
// pre-flight and the server backstop agree on the match by construction (exact ordinal comparison,
|
||||||
|
// soft-deleted rows excluded). "release/exists" is a literal 2-segment route declared before the
|
||||||
|
// parameterized "{trackId}" route and distinct from "release/{id:long}" (different segment shape).
|
||||||
|
[ApiKeyAuthorize]
|
||||||
|
[HttpGet("release/exists")]
|
||||||
|
public async Task<ActionResult> ReleaseExists(
|
||||||
|
[FromQuery] string? title,
|
||||||
|
[FromQuery] string? artist,
|
||||||
|
CancellationToken ct = default)
|
||||||
|
{
|
||||||
|
if (string.IsNullOrWhiteSpace(title) || string.IsNullOrWhiteSpace(artist))
|
||||||
|
return BadRequest("title and artist are both required");
|
||||||
|
|
||||||
|
var result = await _sqlTrackService.GetReleaseByTitleAndArtist(title, artist, ct);
|
||||||
|
if (!result.Success)
|
||||||
|
{
|
||||||
|
var error = result.Messages.FirstOrDefault()?.Message ?? "Unknown error";
|
||||||
|
_logger.LogError("ReleaseExists failed for ({Title}, {Artist}): {Error}", title, artist, error);
|
||||||
|
return StatusCode(500, "Failed to check release");
|
||||||
|
}
|
||||||
|
|
||||||
|
if (result.Value is null)
|
||||||
|
return NotFound();
|
||||||
|
|
||||||
|
return Ok(result.Value);
|
||||||
|
}
|
||||||
|
|
||||||
// GET api/track/genres (unauthenticated)
|
// GET api/track/genres (unauthenticated)
|
||||||
// Distinct non-null genres with track counts. Public browse data, same posture as GET
|
// Distinct non-null genres with track counts. Public browse data, same posture as GET
|
||||||
// api/track/page. Literal segment, declared before the parameterized "{trackId}" route.
|
// api/track/page. Literal segment, declared before the parameterized "{trackId}" route.
|
||||||
@@ -220,6 +251,7 @@ public class TrackController : ControllerBase
|
|||||||
[FromForm] string? releaseType,
|
[FromForm] string? releaseType,
|
||||||
[FromForm] string? medium,
|
[FromForm] string? medium,
|
||||||
[FromForm] int? trackNumber,
|
[FromForm] int? trackNumber,
|
||||||
|
[FromForm] long? releaseId,
|
||||||
CancellationToken cancellationToken)
|
CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
_logger.LogInformation("UploadTrack called: trackName={TrackName}, artist={Artist}, fileName={FileName}, size={Size}",
|
_logger.LogInformation("UploadTrack called: trackName={TrackName}, artist={Artist}, fileName={FileName}, size={Size}",
|
||||||
@@ -315,6 +347,7 @@ public class TrackController : ControllerBase
|
|||||||
parsedReleaseType,
|
parsedReleaseType,
|
||||||
parsedMedium,
|
parsedMedium,
|
||||||
resolvedTrackNumber,
|
resolvedTrackNumber,
|
||||||
|
releaseId,
|
||||||
cancellationToken);
|
cancellationToken);
|
||||||
|
|
||||||
if (!result.Success || result.Value is null)
|
if (!result.Success || result.Value is null)
|
||||||
@@ -322,14 +355,19 @@ public class TrackController : ControllerBase
|
|||||||
var error = result.Messages.FirstOrDefault()?.Message ?? "Failed to process and store audio";
|
var error = result.Messages.FirstOrDefault()?.Message ?? "Failed to process and store audio";
|
||||||
_logger.LogWarning("UploadTrack: UnifiedTrackService failed for {TrackName}: {Error}", trackName, error);
|
_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
|
// A cardinality or duplicate-release rejection is a well-formed request that violates a
|
||||||
// is 409 Conflict — distinct from the 500 used for processing failure. The marker is
|
// domain rule, so it is 409 Conflict — distinct from the 500 used for processing failure.
|
||||||
// stripped so the client sees only the human-readable detail.
|
// The marker is stripped so the client sees only the human-readable detail.
|
||||||
if (error.StartsWith(UnifiedTrackService.CardinalityViolationMarker, StringComparison.Ordinal))
|
if (error.StartsWith(UnifiedTrackService.CardinalityViolationMarker, StringComparison.Ordinal))
|
||||||
{
|
{
|
||||||
return Conflict(error[UnifiedTrackService.CardinalityViolationMarker.Length..]);
|
return Conflict(error[UnifiedTrackService.CardinalityViolationMarker.Length..]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (error.StartsWith(UnifiedTrackService.DuplicateReleaseMarker, StringComparison.Ordinal))
|
||||||
|
{
|
||||||
|
return Conflict(error[UnifiedTrackService.DuplicateReleaseMarker.Length..]);
|
||||||
|
}
|
||||||
|
|
||||||
return StatusCode(500, error);
|
return StatusCode(500, error);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -18,6 +18,12 @@
|
|||||||
<PackageReference Include="Cerebellum.AuthBlocks" Version="10.3.33" />
|
<PackageReference Include="Cerebellum.AuthBlocks" Version="10.3.33" />
|
||||||
</ItemGroup>
|
</ItemGroup>
|
||||||
|
|
||||||
|
<ItemGroup>
|
||||||
|
<!-- Exposes the internal 409 markers (CardinalityViolationMarker / DuplicateReleaseMarker) to the
|
||||||
|
test suite so UploadDuplicateDetectionTests can assert the orchestrator's rejection contract. -->
|
||||||
|
<InternalsVisibleTo Include="DeepDrftTests" />
|
||||||
|
</ItemGroup>
|
||||||
|
|
||||||
<ItemGroup>
|
<ItemGroup>
|
||||||
<ProjectReference Include="..\DeepDrftModels\DeepDrftModels.csproj" />
|
<ProjectReference Include="..\DeepDrftModels\DeepDrftModels.csproj" />
|
||||||
<ProjectReference Include="..\DeepDrftContent\DeepDrftContent.csproj" />
|
<ProjectReference Include="..\DeepDrftContent\DeepDrftContent.csproj" />
|
||||||
|
|||||||
@@ -25,6 +25,16 @@ public class UnifiedTrackService
|
|||||||
/// follows the marker and is what the CMS surfaces to the admin.
|
/// follows the marker and is what the CMS surfaces to the admin.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
internal const string CardinalityViolationMarker = "CARDINALITY_VIOLATION: ";
|
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 TrackContentService _contentTrackContentService;
|
||||||
private readonly ITrackService _sqlTrackService;
|
private readonly ITrackService _sqlTrackService;
|
||||||
private readonly FileDb _fileDatabase;
|
private readonly FileDb _fileDatabase;
|
||||||
@@ -64,33 +74,66 @@ public class UnifiedTrackService
|
|||||||
ReleaseType releaseType,
|
ReleaseType releaseType,
|
||||||
ReleaseMedium medium,
|
ReleaseMedium medium,
|
||||||
int trackNumber,
|
int trackNumber,
|
||||||
|
long? releaseId,
|
||||||
CancellationToken ct)
|
CancellationToken ct)
|
||||||
{
|
{
|
||||||
// Cardinality pre-check — BEFORE the vault write so a rejected over-limit add never orphans
|
// Resolve which release this track lands on BEFORE the vault write, so a rejected upload never
|
||||||
// audio in the tracks vault. This is a READ-only peek (no release is created for an upload we
|
// orphans audio. Two paths:
|
||||||
// may reject); the real FindOrCreateRelease still runs below for the accepted path. Only the
|
// - releaseId is null → CREATE path: this is the first row of a submit. (title, artist) must
|
||||||
// find path can violate: a release that does not yet exist has zero tracks and admits its
|
// NOT already exist — the upload form creates new releases only. A pre-existing match is a
|
||||||
// first. The guard is the general form `(liveCount + 1) > Max`, not Session/Mix-hardcoded, so
|
// duplicate and is blocked (409).
|
||||||
// a future bounded medium is covered by the same line.
|
// - 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))
|
if (!string.IsNullOrWhiteSpace(album))
|
||||||
{
|
{
|
||||||
var peek = await _sqlTrackService.GetReleaseByTitleAndArtist(album, artist, ct);
|
if (releaseId is { } attachId)
|
||||||
if (!peek.Success)
|
|
||||||
{
|
{
|
||||||
var error = peek.Messages.FirstOrDefault()?.Message ?? "Unknown error";
|
var attachPeek = await _sqlTrackService.GetReleaseByTitleAndArtist(album, artist, ct);
|
||||||
_logger.LogError("UploadAsync: release peek failed for ({Album}, {Artist}): {Error}", album, artist, error);
|
if (!attachPeek.Success)
|
||||||
return ResultContainer<TrackDto>.CreateFailResult($"Could not verify the release: {error}");
|
{
|
||||||
}
|
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)
|
// 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.
|
||||||
var cardinality = MediumRules.CardinalityOf(existing.Medium);
|
if (attachPeek.Value is not { } target || target.Id != attachId)
|
||||||
if (existing.TrackCount + 1 > cardinality.Max)
|
|
||||||
{
|
{
|
||||||
return ResultContainer<TrackDto>.CreateFailResult(
|
return ResultContainer<TrackDto>.CreateFailResult(
|
||||||
$"{CardinalityViolationMarker}A {existing.Medium} release holds a single track; " +
|
$"{DuplicateReleaseMarker}The release this track should attach to could not be found. " +
|
||||||
$"'{existing.Title}' already has one — edit the existing track or choose a different release.");
|
"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
|
// 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)
|
// a null ReleaseId. Release-cardinal metadata (artist/genre/description/releaseDate/type/uploader)
|
||||||
// rides on the release, not the track.
|
// rides on the release, not the track.
|
||||||
long? releaseId = null;
|
long? resolvedReleaseId = resolved?.Id;
|
||||||
if (!string.IsNullOrWhiteSpace(album))
|
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
|
var releaseData = new ReleaseDto
|
||||||
{
|
{
|
||||||
Title = album,
|
Title = album,
|
||||||
@@ -124,13 +170,13 @@ public class UnifiedTrackService
|
|||||||
CreatedByUserId = createdByUserId,
|
CreatedByUserId = createdByUserId,
|
||||||
};
|
};
|
||||||
|
|
||||||
// Medium (like every other field in releaseData) applies only when this upload CREATES the
|
// FindOrCreateRelease either creates a fresh release (WasCreated = true) or returns the
|
||||||
// release. FindOrCreateRelease returns an existing (title, artist) row untouched — the first
|
// row the concurrent winner just inserted (WasCreated = false). In the CREATE path the
|
||||||
// upload's medium is authoritative. Do NOT "fix" this to overwrite the stored medium on a
|
// duplicate peek above already verified no pre-existing row exists — so WasCreated = false
|
||||||
// subsequent track add: medium is a release-level property, changed only via the edit path
|
// means we lost a concurrent-insert race. Treat that as the duplicate condition: reject
|
||||||
// (PUT api/track/meta), never silently flipped by adding a track to an existing release.
|
// rather than silently attaching, keeping the DB unique index as the final safety net.
|
||||||
var releaseResult = await _sqlTrackService.FindOrCreateRelease(album, artist, releaseData, ct);
|
var releaseResult = await _sqlTrackService.FindOrCreateRelease(album, artist, releaseData, ct);
|
||||||
if (!releaseResult.Success || releaseResult.Value is null)
|
if (!releaseResult.Success)
|
||||||
{
|
{
|
||||||
var error = releaseResult.Messages.FirstOrDefault()?.Message ?? "Unknown error";
|
var error = releaseResult.Messages.FirstOrDefault()?.Message ?? "Unknown error";
|
||||||
_logger.LogError(
|
_logger.LogError(
|
||||||
@@ -139,11 +185,21 @@ public class UnifiedTrackService
|
|||||||
return ResultContainer<TrackDto>.CreateFailResult($"Track was uploaded but could not be saved: {error}");
|
return ResultContainer<TrackDto>.CreateFailResult($"Track was uploaded but could not be saved: {error}");
|
||||||
}
|
}
|
||||||
|
|
||||||
releaseId = releaseResult.Value.Id;
|
var (resolvedRelease, wasCreated) = releaseResult.Value;
|
||||||
|
if (!wasCreated)
|
||||||
|
{
|
||||||
|
// The winning concurrent upload created this release between our peek and our insert.
|
||||||
|
// Reject with the same marker the pre-flight peek uses so the controller maps it to 409.
|
||||||
|
return ResultContainer<TrackDto>.CreateFailResult(
|
||||||
|
$"{DuplicateReleaseMarker}A release titled '{resolvedRelease.Title}' by {resolvedRelease.Artist} already " +
|
||||||
|
"exists. The upload form creates new releases only — use the edit tools to change an existing one.");
|
||||||
|
}
|
||||||
|
|
||||||
|
resolvedReleaseId = resolvedRelease.Id;
|
||||||
}
|
}
|
||||||
|
|
||||||
var trackDto = TrackConverter.Convert(unpersisted);
|
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.
|
trackDto.Release = null; // FK already resolved; Create must not re-resolve a detached graph.
|
||||||
|
|
||||||
var saveResult = await _sqlTrackService.Create(trackDto);
|
var saveResult = await _sqlTrackService.Create(trackDto);
|
||||||
@@ -166,6 +222,26 @@ public class UnifiedTrackService
|
|||||||
return saveResult;
|
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>
|
/// <summary>
|
||||||
/// Replace an existing track's audio in place: look up the SQL row, swap only the vault bytes
|
/// 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
|
/// keyed by its EntryKey, regenerate both waveform datums from the new audio, then write the
|
||||||
|
|||||||
@@ -59,8 +59,12 @@ public interface ITrackService
|
|||||||
/// Resolve the release matching <paramref name="title"/> + <paramref name="artist"/>, creating
|
/// Resolve the release matching <paramref name="title"/> + <paramref name="artist"/>, creating
|
||||||
/// one from <paramref name="releaseData"/> when none exists. Backs the upload flow's FK
|
/// one from <paramref name="releaseData"/> when none exists. Backs the upload flow's FK
|
||||||
/// resolution so a track lands on a shared release rather than duplicating release-cardinal data.
|
/// resolution so a track lands on a shared release rather than duplicating release-cardinal data.
|
||||||
|
/// The <c>WasCreated</c> flag in the result is <see langword="true"/> when a new row was inserted
|
||||||
|
/// and <see langword="false"/> when an existing row was found (including after a lost concurrent-insert
|
||||||
|
/// race). The CREATE path in <c>UnifiedTrackService.UploadAsync</c> uses this to turn a
|
||||||
|
/// "found existing" outcome into a duplicate rejection rather than a silent attach.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
Task<ResultContainer<ReleaseDto>> FindOrCreateRelease(
|
Task<ResultContainer<(ReleaseDto Release, bool WasCreated)>> FindOrCreateRelease(
|
||||||
string title, string artist, ReleaseDto releaseData, CancellationToken cancellationToken = default);
|
string title, string artist, ReleaseDto releaseData, CancellationToken cancellationToken = default);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
@@ -164,14 +164,14 @@ public class TrackManager
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public async Task<ResultContainer<ReleaseDto>> FindOrCreateRelease(
|
public async Task<ResultContainer<(ReleaseDto Release, bool WasCreated)>> FindOrCreateRelease(
|
||||||
string title, string artist, ReleaseDto releaseData, CancellationToken cancellationToken = default)
|
string title, string artist, ReleaseDto releaseData, CancellationToken cancellationToken = default)
|
||||||
{
|
{
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var existing = await Repository.GetReleaseByTitleAndArtistAsync(title, artist, cancellationToken);
|
var existing = await Repository.GetReleaseByTitleAndArtistAsync(title, artist, cancellationToken);
|
||||||
if (existing is not null)
|
if (existing is not null)
|
||||||
return ResultContainer<ReleaseDto>.CreatePassResult(TrackConverter.Convert(existing));
|
return ResultContainer<(ReleaseDto, bool)>.CreatePassResult((TrackConverter.Convert(existing), false));
|
||||||
|
|
||||||
// The natural key (title + artist) is authoritative — override whatever the caller put
|
// The natural key (title + artist) is authoritative — override whatever the caller put
|
||||||
// in releaseData so a typo upstream cannot create a release that won't be found again.
|
// in releaseData so a typo upstream cannot create a release that won't be found again.
|
||||||
@@ -186,21 +186,21 @@ public class TrackManager
|
|||||||
try
|
try
|
||||||
{
|
{
|
||||||
var added = await Repository.AddReleaseAsync(entity, cancellationToken);
|
var added = await Repository.AddReleaseAsync(entity, cancellationToken);
|
||||||
return ResultContainer<ReleaseDto>.CreatePassResult(TrackConverter.Convert(added));
|
return ResultContainer<(ReleaseDto, bool)>.CreatePassResult((TrackConverter.Convert(added), true));
|
||||||
}
|
}
|
||||||
catch (ClassifiedDbException ex) when (ex.Error.Category == DbErrorCategory.UniqueViolation)
|
catch (ClassifiedDbException ex) when (ex.Error.Category == DbErrorCategory.UniqueViolation)
|
||||||
{
|
{
|
||||||
// Concurrent upload inserted the same (title, artist) between our read and write.
|
// Concurrent upload inserted the same (title, artist) between our read and write.
|
||||||
// Re-query and return the winning row. Should not return null here since the
|
// Re-query and return the winning row as WasCreated=false so the caller (UploadAsync
|
||||||
// constraint just fired, but re-throw if it does so the caller sees an error.
|
// CREATE path) treats the lost race as a duplicate rather than silently attaching.
|
||||||
var race = await Repository.GetReleaseByTitleAndArtistAsync(title, artist, cancellationToken);
|
var race = await Repository.GetReleaseByTitleAndArtistAsync(title, artist, cancellationToken);
|
||||||
if (race is null) throw;
|
if (race is null) throw;
|
||||||
return ResultContainer<ReleaseDto>.CreatePassResult(TrackConverter.Convert(race));
|
return ResultContainer<(ReleaseDto, bool)>.CreatePassResult((TrackConverter.Convert(race), false));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
catch (Exception e)
|
catch (Exception e)
|
||||||
{
|
{
|
||||||
return ResultContainer<ReleaseDto>.CreateFailResult(e.Message);
|
return ResultContainer<(ReleaseDto, bool)>.CreateFailResult(e.Message);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -302,13 +302,13 @@ public class TrackManager
|
|||||||
if (newTrack.Release is { } release && !string.IsNullOrWhiteSpace(release.Title))
|
if (newTrack.Release is { } release && !string.IsNullOrWhiteSpace(release.Title))
|
||||||
{
|
{
|
||||||
var resolved = await FindOrCreateRelease(release.Title, release.Artist, release);
|
var resolved = await FindOrCreateRelease(release.Title, release.Artist, release);
|
||||||
if (!resolved.Success || resolved.Value is null)
|
if (!resolved.Success)
|
||||||
{
|
{
|
||||||
var error = resolved.Messages.FirstOrDefault()?.Message ?? "Failed to resolve release.";
|
var error = resolved.Messages.FirstOrDefault()?.Message ?? "Failed to resolve release.";
|
||||||
return ResultContainer<TrackDto>.CreateFailResult(error);
|
return ResultContainer<TrackDto>.CreateFailResult(error);
|
||||||
}
|
}
|
||||||
|
|
||||||
newTrack.ReleaseId = resolved.Value.Id;
|
newTrack.ReleaseId = resolved.Value.Release.Id;
|
||||||
}
|
}
|
||||||
|
|
||||||
var added = await Repository.AddAsync(TrackConverter.Convert(newTrack));
|
var added = await Repository.AddAsync(TrackConverter.Convert(newTrack));
|
||||||
|
|||||||
@@ -146,6 +146,9 @@
|
|||||||
private string _releaseDate = string.Empty;
|
private string _releaseDate = string.Empty;
|
||||||
private ReleaseType _releaseType = ReleaseType.Single;
|
private ReleaseType _releaseType = ReleaseType.Single;
|
||||||
private ReleaseMedium _medium = ReleaseMedium.Cut;
|
private ReleaseMedium _medium = ReleaseMedium.Cut;
|
||||||
|
// The id of the release being edited. New tracks added in this session attach to it via the upload
|
||||||
|
// service's releaseId (ATTACH) path, so they are not rejected as a pre-existing-(title,artist) duplicate.
|
||||||
|
private long? _releaseId;
|
||||||
|
|
||||||
// The medium selector drives ReleaseType visibility and is persisted on save: every UpdateAsync /
|
// The medium selector drives ReleaseType visibility and is persisted on save: every UpdateAsync /
|
||||||
// UploadTrackAsync call below passes _medium, and PUT api/track/meta resets ReleaseType to its
|
// UploadTrackAsync call below passes _medium, and PUT api/track/meta resets ReleaseType to its
|
||||||
@@ -214,6 +217,10 @@
|
|||||||
}
|
}
|
||||||
|
|
||||||
var release = tracks[0].Release;
|
var release = tracks[0].Release;
|
||||||
|
// The release being edited already exists, so any new track added here ATTACHES to it (the upload
|
||||||
|
// service's releaseId path) rather than taking the CREATE path, which would reject it as a
|
||||||
|
// duplicate (title, artist). Fall back to the track's own ReleaseId if the nav is not populated.
|
||||||
|
_releaseId = release?.Id ?? tracks[0].ReleaseId;
|
||||||
_albumName = albumName;
|
_albumName = albumName;
|
||||||
_artist = release?.Artist ?? string.Empty;
|
_artist = release?.Artist ?? string.Empty;
|
||||||
_genre = release?.Genre ?? string.Empty;
|
_genre = release?.Genre ?? string.Empty;
|
||||||
@@ -592,6 +599,7 @@
|
|||||||
_releaseType,
|
_releaseType,
|
||||||
trackNumber,
|
trackNumber,
|
||||||
_medium,
|
_medium,
|
||||||
|
_releaseId,
|
||||||
progress);
|
progress);
|
||||||
|
|
||||||
if (!uploadResult.Success || uploadResult.Value is null)
|
if (!uploadResult.Success || uploadResult.Value is null)
|
||||||
|
|||||||
@@ -298,6 +298,29 @@
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Pre-flight duplicate guard (primary block): the upload form creates new releases only, so a
|
||||||
|
// (title, artist) that already exists in the catalogue is refused BEFORE any bytes transfer —
|
||||||
|
// the admin is not surprised at the end of a long upload. The server backstops this on the
|
||||||
|
// create path, but checking here keeps the failure fast and visible. The values passed match
|
||||||
|
// exactly what the upload sends (untrimmed _albumName/_artist) so the pre-flight and the server
|
||||||
|
// agree on the match. A check failure (API unreachable) blocks rather than proceeding blind.
|
||||||
|
var duplicateCheck = await CmsTrackService.GetExistingReleaseAsync(_albumName, _artist);
|
||||||
|
if (!duplicateCheck.Success)
|
||||||
|
{
|
||||||
|
var checkError = duplicateCheck.Messages.FirstOrDefault()?.Message ?? "Unknown error";
|
||||||
|
_errorMessage = $"Could not verify the release name: {checkError}";
|
||||||
|
Snackbar.Add(_errorMessage, Severity.Error);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (duplicateCheck.Value is { } existing)
|
||||||
|
{
|
||||||
|
_errorMessage = $"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.";
|
||||||
|
Snackbar.Add(_errorMessage, Severity.Error);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// For single-track media (Session/Mix) the track name is derived from the Release Name —
|
// For single-track media (Session/Mix) the track name is derived from the Release Name —
|
||||||
// no separate Track Name input is shown. Sync here so the stored name always matches.
|
// no separate Track Name input is shown. Sync here so the stored name always matches.
|
||||||
if (MediumRules.CardinalityOf(_medium).IsSingleTrack && _tracks.Count > 0)
|
if (MediumRules.CardinalityOf(_medium).IsSingleTrack && _tracks.Count > 0)
|
||||||
@@ -327,6 +350,11 @@
|
|||||||
}
|
}
|
||||||
|
|
||||||
int succeeded = 0, failed = 0;
|
int succeeded = 0, failed = 0;
|
||||||
|
// Within-batch attach: row 1 creates the release (no releaseId → CREATE path); once it
|
||||||
|
// succeeds we carry its ReleaseId into rows 2..N so they ATTACH to the just-created release
|
||||||
|
// rather than tripping the server's pre-existing-duplicate block. Only a multi-track Cut
|
||||||
|
// reaches row 2 (single-track media collapse to one row).
|
||||||
|
long? batchReleaseId = null;
|
||||||
for (int i = 0; i < _tracks.Count; i++)
|
for (int i = 0; i < _tracks.Count; i++)
|
||||||
{
|
{
|
||||||
var row = _tracks[i];
|
var row = _tracks[i];
|
||||||
@@ -375,6 +403,7 @@
|
|||||||
_releaseType,
|
_releaseType,
|
||||||
trackNumber,
|
trackNumber,
|
||||||
_medium,
|
_medium,
|
||||||
|
batchReleaseId,
|
||||||
progress);
|
progress);
|
||||||
|
|
||||||
if (!result.Success || result.Value is null)
|
if (!result.Success || result.Value is null)
|
||||||
@@ -387,6 +416,15 @@
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
|
// Capture the release id created by the first successful row so subsequent rows
|
||||||
|
// attach to it (the within-batch multi-track Cut path). Only set once — later
|
||||||
|
// rows must not overwrite it. A null ReleaseId here (loose track) leaves it null,
|
||||||
|
// which is correct: a release-less upload has no within-batch release to attach to.
|
||||||
|
if (batchReleaseId is null && result.Value.ReleaseId is { } createdReleaseId)
|
||||||
|
{
|
||||||
|
batchReleaseId = createdReleaseId;
|
||||||
|
}
|
||||||
|
|
||||||
// The upload endpoint does not accept an imagePath, so link the cover art with
|
// The upload endpoint does not accept an imagePath, so link the cover art with
|
||||||
// a follow-up metadata update — same two-step pattern BatchEdit uses.
|
// a follow-up metadata update — same two-step pattern BatchEdit uses.
|
||||||
if (_imagePath is { } imgPath)
|
if (_imagePath is { } imgPath)
|
||||||
@@ -487,7 +525,13 @@
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
Snackbar.Add($"Uploaded {succeeded} track(s); {failed} failed — review errors below.", Severity.Warning);
|
// Surface the actual reason, not just counts — a server rejection (duplicate, cardinality)
|
||||||
|
// relays a human-readable message via row.ErrorMessage. Show the first failure's reason so
|
||||||
|
// the admin sees WHY without scanning the rows; the per-row errors remain as detail.
|
||||||
|
var firstError = _tracks.FirstOrDefault(t => t.Status == BatchRowStatus.Failed)?.ErrorMessage;
|
||||||
|
var reason = string.IsNullOrWhiteSpace(firstError) ? "review errors below" : firstError;
|
||||||
|
_errorMessage = succeeded == 0 ? reason : $"{succeeded} uploaded; {failed} failed: {reason}";
|
||||||
|
Snackbar.Add($"Uploaded {succeeded} track(s); {failed} failed — {reason}", Severity.Warning);
|
||||||
// Stay on page so the admin can see the failed rows.
|
// Stay on page so the admin can see the failed rows.
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -68,6 +68,7 @@ public class CmsTrackService : ICmsTrackService
|
|||||||
ReleaseType releaseType,
|
ReleaseType releaseType,
|
||||||
int trackNumber,
|
int trackNumber,
|
||||||
ReleaseMedium medium = ReleaseMedium.Cut,
|
ReleaseMedium medium = ReleaseMedium.Cut,
|
||||||
|
long? releaseId = null,
|
||||||
IProgress<long>? progress = null,
|
IProgress<long>? progress = null,
|
||||||
CancellationToken ct = default)
|
CancellationToken ct = default)
|
||||||
{
|
{
|
||||||
@@ -91,6 +92,9 @@ public class CmsTrackService : ICmsTrackService
|
|||||||
// The upload endpoint binds "medium" to the created release's ReleaseMedium (defaulting to Cut
|
// The upload endpoint binds "medium" to the created release's ReleaseMedium (defaulting to Cut
|
||||||
// for an unrecognised value). Authoritative only when this upload creates the release.
|
// for an unrecognised value). Authoritative only when this upload creates the release.
|
||||||
multipart.Add(new StringContent(medium.ToString()), "medium");
|
multipart.Add(new StringContent(medium.ToString()), "medium");
|
||||||
|
// releaseId present → ATTACH (rows 2..N of a within-batch Cut); absent → CREATE (server rejects a
|
||||||
|
// pre-existing (title, artist) as a duplicate). Only sent when set so the form omits it on row 1.
|
||||||
|
if (releaseId is { } rid) multipart.Add(new StringContent(rid.ToString()), "releaseId");
|
||||||
|
|
||||||
var send = await phase.SendAsync(UploadPath, multipart, $"upload of {trackName}");
|
var send = await phase.SendAsync(UploadPath, multipart, $"upload of {trackName}");
|
||||||
if (send.Response is not { } response)
|
if (send.Response is not { } response)
|
||||||
@@ -474,6 +478,53 @@ public class CmsTrackService : ICmsTrackService
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public async Task<ResultContainer<ReleaseDto?>> GetExistingReleaseAsync(
|
||||||
|
string title, string artist, CancellationToken ct = default)
|
||||||
|
{
|
||||||
|
var client = _httpClientFactory.CreateClient(ContentCmsClientName);
|
||||||
|
var query = $"api/track/release/exists?title={Uri.EscapeDataString(title)}&artist={Uri.EscapeDataString(artist)}";
|
||||||
|
|
||||||
|
HttpResponseMessage response;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
response = await client.GetAsync(query, ct);
|
||||||
|
}
|
||||||
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
_logger.LogError(ex, "Content API call failed for release existence check ({Title}, {Artist})", title, artist);
|
||||||
|
return ResultContainer<ReleaseDto?>.CreateFailResult("Content API is unreachable.");
|
||||||
|
}
|
||||||
|
|
||||||
|
using (response)
|
||||||
|
{
|
||||||
|
// 404 is the not-found (null) case, not a failure — no release matches this (title, artist).
|
||||||
|
if (response.StatusCode == HttpStatusCode.NotFound)
|
||||||
|
{
|
||||||
|
return ResultContainer<ReleaseDto?>.CreatePassResult(null);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!response.IsSuccessStatusCode)
|
||||||
|
{
|
||||||
|
_logger.LogError("Content API release existence check failed for ({Title}, {Artist}): {Status}",
|
||||||
|
title, artist, (int)response.StatusCode);
|
||||||
|
return ResultContainer<ReleaseDto?>.CreateFailResult("Failed to check for an existing release.");
|
||||||
|
}
|
||||||
|
|
||||||
|
ReleaseDto? release;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
release = await response.Content.ReadFromJsonAsync<ReleaseDto>(ct);
|
||||||
|
}
|
||||||
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
_logger.LogError(ex, "Failed to deserialize ReleaseDto from release existence check");
|
||||||
|
return ResultContainer<ReleaseDto?>.CreateFailResult("Content API returned an unexpected response.");
|
||||||
|
}
|
||||||
|
|
||||||
|
return ResultContainer<ReleaseDto?>.CreatePassResult(release);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private static readonly HashSet<string> KnownImageMimeTypes = new(StringComparer.OrdinalIgnoreCase)
|
private static readonly HashSet<string> KnownImageMimeTypes = new(StringComparer.OrdinalIgnoreCase)
|
||||||
{
|
{
|
||||||
"image/jpeg", "image/png", "image/gif", "image/webp", "image/svg+xml", "image/bmp"
|
"image/jpeg", "image/png", "image/gif", "image/webp", "image/svg+xml", "image/bmp"
|
||||||
|
|||||||
@@ -25,6 +25,10 @@ public interface ICmsTrackService
|
|||||||
/// sets Content-Length and is the denominator for <paramref name="progress"/>, which reports cumulative
|
/// sets Content-Length and is the denominator for <paramref name="progress"/>, which reports cumulative
|
||||||
/// bytes pushed to the wire. Each progress tick also resets the idle/heartbeat upload timeout, so a
|
/// bytes pushed to the wire. Each progress tick also resets the idle/heartbeat upload timeout, so a
|
||||||
/// stalled connection aborts without a fixed total-duration cap.
|
/// stalled connection aborts without a fixed total-duration cap.
|
||||||
|
/// <paramref name="releaseId"/> distinguishes the two rows of a within-batch multi-track Cut: null on
|
||||||
|
/// the first row (CREATE — the server rejects a pre-existing (title, artist) as a duplicate) and the
|
||||||
|
/// id returned by that first row on rows 2..N (ATTACH — the server skips the duplicate check and adds
|
||||||
|
/// the track to the release the batch just created).
|
||||||
/// </summary>
|
/// </summary>
|
||||||
Task<ResultContainer<TrackDto>> UploadTrackAsync(
|
Task<ResultContainer<TrackDto>> UploadTrackAsync(
|
||||||
Stream wavStream,
|
Stream wavStream,
|
||||||
@@ -42,9 +46,20 @@ public interface ICmsTrackService
|
|||||||
ReleaseType releaseType,
|
ReleaseType releaseType,
|
||||||
int trackNumber,
|
int trackNumber,
|
||||||
ReleaseMedium medium = ReleaseMedium.Cut,
|
ReleaseMedium medium = ReleaseMedium.Cut,
|
||||||
|
long? releaseId = null,
|
||||||
IProgress<long>? progress = null,
|
IProgress<long>? progress = null,
|
||||||
CancellationToken ct = default);
|
CancellationToken ct = default);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Upload-form pre-flight: returns the existing release whose exact (title, artist) matches, or null
|
||||||
|
/// when none exists. Backs the duplicate block the form runs BEFORE transferring bytes, so the admin
|
||||||
|
/// is not surprised at the end of a long upload. A 404 from the API is the not-found (null) case, not
|
||||||
|
/// a failure. The match semantics are the API's <c>GetReleaseByTitleAndArtist</c> — the same read the
|
||||||
|
/// server backstop uses — so the pre-flight and the backstop agree.
|
||||||
|
/// </summary>
|
||||||
|
Task<ResultContainer<ReleaseDto?>> GetExistingReleaseAsync(
|
||||||
|
string title, string artist, CancellationToken ct = default);
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Delete a track via the Content API, which removes the SQL row then the vault entry.
|
/// Delete a track via the Content API, which removes the SQL row then the vault entry.
|
||||||
/// Maps a 404 to a "Track not found." failure.
|
/// Maps a 404 to a "Track not found." failure.
|
||||||
|
|||||||
@@ -28,6 +28,9 @@
|
|||||||
</ItemGroup>
|
</ItemGroup>
|
||||||
|
|
||||||
<ItemGroup>
|
<ItemGroup>
|
||||||
|
<!-- Referenced for UnifiedTrackService — the dual-database upload orchestrator whose create-path
|
||||||
|
duplicate guard and within-batch attach path are exercised in UploadDuplicateDetectionTests. -->
|
||||||
|
<ProjectReference Include="..\DeepDrftAPI\DeepDrftAPI.csproj" />
|
||||||
<ProjectReference Include="..\DeepDrftContent\DeepDrftContent.csproj" />
|
<ProjectReference Include="..\DeepDrftContent\DeepDrftContent.csproj" />
|
||||||
<ProjectReference Include="..\DeepDrftData\DeepDrftData.csproj" />
|
<ProjectReference Include="..\DeepDrftData\DeepDrftData.csproj" />
|
||||||
<!-- Referenced for ProgressStreamContent (the upload progress/heartbeat HttpContent). It is plain
|
<!-- Referenced for ProgressStreamContent (the upload progress/heartbeat HttpContent). It is plain
|
||||||
|
|||||||
@@ -60,9 +60,9 @@ public class MediumWritePathTests
|
|||||||
"Live at the Vault", "Artist A", ReleaseData("Live at the Vault", "Artist A", ReleaseMedium.Session));
|
"Live at the Vault", "Artist A", ReleaseData("Live at the Vault", "Artist A", ReleaseMedium.Session));
|
||||||
|
|
||||||
Assert.That(result.Success, Is.True);
|
Assert.That(result.Success, Is.True);
|
||||||
Assert.That(result.Value!.Medium, Is.EqualTo(ReleaseMedium.Session));
|
Assert.That(result.Value.Release.Medium, Is.EqualTo(ReleaseMedium.Session));
|
||||||
|
|
||||||
var stored = await CreateRepository().GetReleaseByIdAsync(result.Value.Id);
|
var stored = await CreateRepository().GetReleaseByIdAsync(result.Value.Release.Id);
|
||||||
Assert.That(stored!.Medium, Is.EqualTo(ReleaseMedium.Session));
|
Assert.That(stored!.Medium, Is.EqualTo(ReleaseMedium.Session));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -75,7 +75,7 @@ public class MediumWritePathTests
|
|||||||
var result = await manager.FindOrCreateRelease(
|
var result = await manager.FindOrCreateRelease(
|
||||||
"Sunset Set", "DJ B", ReleaseData("Sunset Set", "DJ B", ReleaseMedium.Mix));
|
"Sunset Set", "DJ B", ReleaseData("Sunset Set", "DJ B", ReleaseMedium.Mix));
|
||||||
|
|
||||||
Assert.That(result.Value!.Medium, Is.EqualTo(ReleaseMedium.Mix));
|
Assert.That(result.Value.Release.Medium, Is.EqualTo(ReleaseMedium.Mix));
|
||||||
}
|
}
|
||||||
|
|
||||||
// 9.5.A — a Cut upload (the default) creates a release carrying Medium == Cut.
|
// 9.5.A — a Cut upload (the default) creates a release carrying Medium == Cut.
|
||||||
@@ -87,7 +87,7 @@ public class MediumWritePathTests
|
|||||||
var result = await manager.FindOrCreateRelease(
|
var result = await manager.FindOrCreateRelease(
|
||||||
"Studio Album", "Artist C", ReleaseData("Studio Album", "Artist C", ReleaseMedium.Cut));
|
"Studio Album", "Artist C", ReleaseData("Studio Album", "Artist C", ReleaseMedium.Cut));
|
||||||
|
|
||||||
Assert.That(result.Value!.Medium, Is.EqualTo(ReleaseMedium.Cut));
|
Assert.That(result.Value.Release.Medium, Is.EqualTo(ReleaseMedium.Cut));
|
||||||
}
|
}
|
||||||
|
|
||||||
// 9.5.A — a second upload to an existing release does NOT mutate the stored medium. The first
|
// 9.5.A — a second upload to an existing release does NOT mutate the stored medium. The first
|
||||||
@@ -105,10 +105,10 @@ public class MediumWritePathTests
|
|||||||
var found = await manager.FindOrCreateRelease(
|
var found = await manager.FindOrCreateRelease(
|
||||||
"Live at the Vault", "Artist A", ReleaseData("Live at the Vault", "Artist A", ReleaseMedium.Cut));
|
"Live at the Vault", "Artist A", ReleaseData("Live at the Vault", "Artist A", ReleaseMedium.Cut));
|
||||||
|
|
||||||
Assert.That(found.Value!.Id, Is.EqualTo(created.Value!.Id), "same release row is returned");
|
Assert.That(found.Value.Release.Id, Is.EqualTo(created.Value.Release.Id), "same release row is returned");
|
||||||
Assert.That(found.Value.Medium, Is.EqualTo(ReleaseMedium.Session), "medium stays as first set");
|
Assert.That(found.Value.Release.Medium, Is.EqualTo(ReleaseMedium.Session), "medium stays as first set");
|
||||||
|
|
||||||
var stored = await CreateRepository().GetReleaseByIdAsync(created.Value.Id);
|
var stored = await CreateRepository().GetReleaseByIdAsync(created.Value.Release.Id);
|
||||||
Assert.That(stored!.Medium, Is.EqualTo(ReleaseMedium.Session), "DB row unchanged");
|
Assert.That(stored!.Medium, Is.EqualTo(ReleaseMedium.Session), "DB row unchanged");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -207,9 +207,9 @@ public class MediumWritePathTests
|
|||||||
var result = await manager.FindOrCreateRelease("Studio Album", "Artist C", data);
|
var result = await manager.FindOrCreateRelease("Studio Album", "Artist C", data);
|
||||||
|
|
||||||
Assert.That(result.Success, Is.True);
|
Assert.That(result.Success, Is.True);
|
||||||
Assert.That(result.Value!.Description, Is.EqualTo(prose));
|
Assert.That(result.Value.Release.Description, Is.EqualTo(prose));
|
||||||
|
|
||||||
var stored = await CreateRepository().GetReleaseByIdAsync(result.Value.Id);
|
var stored = await CreateRepository().GetReleaseByIdAsync(result.Value.Release.Id);
|
||||||
Assert.That(stored!.Description, Is.EqualTo(prose));
|
Assert.That(stored!.Description, Is.EqualTo(prose));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,292 @@
|
|||||||
|
using System.Text;
|
||||||
|
using Data.Data.Repositories;
|
||||||
|
using Data.Managers;
|
||||||
|
using DeepDrftAPI.Services;
|
||||||
|
using DeepDrftContent;
|
||||||
|
using DeepDrftContent.Processors;
|
||||||
|
using DeepDrftData;
|
||||||
|
using DeepDrftData.Data;
|
||||||
|
using DeepDrftData.Repositories;
|
||||||
|
using DeepDrftModels.DTOs;
|
||||||
|
using DeepDrftModels.Entities;
|
||||||
|
using DeepDrftModels.Enums;
|
||||||
|
using Microsoft.EntityFrameworkCore;
|
||||||
|
using Microsoft.Extensions.Logging.Abstractions;
|
||||||
|
using Microsoft.Extensions.Options;
|
||||||
|
using NetBlocks.Models;
|
||||||
|
using FileDb = DeepDrftContent.FileDatabase.Services.FileDatabase;
|
||||||
|
|
||||||
|
namespace DeepDrftTests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Server-backstop coverage for upload duplicate detection. Drives the full
|
||||||
|
/// <see cref="UnifiedTrackService.UploadAsync"/> dual-database write over a real temp-isolated
|
||||||
|
/// <see cref="FileDb"/> vault and an EF in-memory <see cref="DeepDrftContext"/>, so the create-path
|
||||||
|
/// duplicate block, the within-batch attach path, and the existing single-track cardinality rule are
|
||||||
|
/// all asserted against the same orchestrator the controller calls.
|
||||||
|
///
|
||||||
|
/// The rule under test: a (title, artist) that pre-existed the submit is blocked on the CREATE path
|
||||||
|
/// (no releaseId), but the within-batch multi-track Cut still succeeds because rows 2..N pass the
|
||||||
|
/// release id row 1 created (ATTACH path) and so skip the duplicate lookup entirely.
|
||||||
|
/// </summary>
|
||||||
|
[TestFixture]
|
||||||
|
public class UploadDuplicateDetectionTests
|
||||||
|
{
|
||||||
|
private string _testDir = string.Empty;
|
||||||
|
private DeepDrftContext _context = null!;
|
||||||
|
|
||||||
|
[SetUp]
|
||||||
|
public void SetUp()
|
||||||
|
{
|
||||||
|
_testDir = Path.Combine(Path.GetTempPath(), "UploadDuplicateDetectionTests", Guid.NewGuid().ToString());
|
||||||
|
Directory.CreateDirectory(_testDir);
|
||||||
|
|
||||||
|
var options = new DbContextOptionsBuilder<DeepDrftContext>()
|
||||||
|
.UseInMemoryDatabase(databaseName: Guid.NewGuid().ToString())
|
||||||
|
.Options;
|
||||||
|
_context = new DeepDrftContext(options);
|
||||||
|
}
|
||||||
|
|
||||||
|
[TearDown]
|
||||||
|
public void TearDown()
|
||||||
|
{
|
||||||
|
_context.Dispose();
|
||||||
|
try { Directory.Delete(_testDir, recursive: true); }
|
||||||
|
catch { /* Best-effort cleanup — ignore failures */ }
|
||||||
|
}
|
||||||
|
|
||||||
|
private TrackManager CreateManager()
|
||||||
|
{
|
||||||
|
var repository = new TrackRepository(
|
||||||
|
_context, NullLogger<Repository<DeepDrftContext, TrackEntity>>.Instance);
|
||||||
|
return new TrackManager(
|
||||||
|
repository, NullLogger<Manager<TrackEntity, TrackDto, TrackRepository, TrackConverter>>.Instance);
|
||||||
|
}
|
||||||
|
|
||||||
|
private async Task<UnifiedTrackService> CreateUnifiedServiceAsync(ITrackService sqlTrackService)
|
||||||
|
{
|
||||||
|
var fileDatabase = await FileDb.FromAsync(_testDir);
|
||||||
|
Assert.That(fileDatabase, Is.Not.Null);
|
||||||
|
|
||||||
|
var content = new TrackContentService(
|
||||||
|
fileDatabase!, new AudioProcessorRouter(
|
||||||
|
new AudioProcessor(), new Mp3AudioProcessor(), new FlacAudioProcessor()));
|
||||||
|
var waveforms = new WaveformProfileService(
|
||||||
|
fileDatabase!, new AudioProcessor(), new RmsLoudnessAlgorithm(),
|
||||||
|
Options.Create(new WaveformProfileOptions()), NullLogger<WaveformProfileService>.Instance);
|
||||||
|
|
||||||
|
return new UnifiedTrackService(
|
||||||
|
content, sqlTrackService, fileDatabase!, waveforms,
|
||||||
|
NullLogger<UnifiedTrackService>.Instance);
|
||||||
|
}
|
||||||
|
|
||||||
|
private async Task<string> WriteWavAsync(double durationSeconds)
|
||||||
|
{
|
||||||
|
var path = Path.Combine(_testDir, Guid.NewGuid().ToString("N") + ".wav");
|
||||||
|
await File.WriteAllBytesAsync(path, BuildMinimalPcmWav(durationSeconds));
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
|
||||||
|
private Task<ResultContainer<TrackDto>> UploadAsync(
|
||||||
|
UnifiedTrackService service, string tempPath, string trackName, string artist,
|
||||||
|
string? album, ReleaseMedium medium, long? releaseId)
|
||||||
|
=> service.UploadAsync(
|
||||||
|
tempPath, trackName, artist, album,
|
||||||
|
genre: null, description: null, releaseDate: null, createdByUserId: 1,
|
||||||
|
originalFileName: null, releaseType: ReleaseType.Single, medium: medium,
|
||||||
|
trackNumber: 1, releaseId: releaseId, ct: default);
|
||||||
|
|
||||||
|
// CREATE path: a brand-new single-track Mix succeeds (no pre-existing (title, artist)).
|
||||||
|
[Test]
|
||||||
|
public async Task UploadAsync_NewSingleTrackRelease_Succeeds()
|
||||||
|
{
|
||||||
|
var service = await CreateUnifiedServiceAsync(CreateManager());
|
||||||
|
|
||||||
|
var result = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Sunset Set", "DJ B", "Sunset Set", ReleaseMedium.Mix, releaseId: null);
|
||||||
|
|
||||||
|
Assert.That(result.Success, Is.True, result.Messages.FirstOrDefault()?.Message);
|
||||||
|
Assert.That(result.Value!.ReleaseId, Is.Not.Null);
|
||||||
|
}
|
||||||
|
|
||||||
|
// CREATE path: uploading a (title, artist) that already exists is blocked with the duplicate marker
|
||||||
|
// (which the controller maps to 409), for ANY medium — here a Cut.
|
||||||
|
[Test]
|
||||||
|
public async Task UploadAsync_DuplicateTitleArtist_IsBlockedWithDuplicateMarker()
|
||||||
|
{
|
||||||
|
var service = await CreateUnifiedServiceAsync(CreateManager());
|
||||||
|
|
||||||
|
var first = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Studio Album", "Artist C", "Studio Album", ReleaseMedium.Cut, releaseId: null);
|
||||||
|
Assert.That(first.Success, Is.True, "the first create must succeed");
|
||||||
|
|
||||||
|
// Second submit, same (title, artist), no releaseId → CREATE path → duplicate block.
|
||||||
|
var duplicate = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Studio Album", "Artist C", "Studio Album", ReleaseMedium.Cut, releaseId: null);
|
||||||
|
|
||||||
|
Assert.That(duplicate.Success, Is.False);
|
||||||
|
var message = duplicate.Messages.FirstOrDefault()?.Message ?? string.Empty;
|
||||||
|
Assert.That(message, Does.StartWith(UnifiedTrackService.DuplicateReleaseMarker));
|
||||||
|
Assert.That(message, Does.Contain("Studio Album"), "the block message names the existing release");
|
||||||
|
}
|
||||||
|
|
||||||
|
// The crux regression guard: a within-batch multi-track Cut. Row 1 CREATEs the release; row 2 passes
|
||||||
|
// row 1's ReleaseId (ATTACH path) and must succeed — the within-batch release is NOT a pre-existing
|
||||||
|
// duplicate. Both tracks end up on the same release.
|
||||||
|
[Test]
|
||||||
|
public async Task UploadAsync_WithinBatchMultiTrackCut_AttachesAndSucceeds()
|
||||||
|
{
|
||||||
|
var manager = CreateManager();
|
||||||
|
var service = await CreateUnifiedServiceAsync(manager);
|
||||||
|
|
||||||
|
var row1 = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Track One", "Artist A", "Live at the Vault", ReleaseMedium.Cut, releaseId: null);
|
||||||
|
Assert.That(row1.Success, Is.True, "row 1 creates the release");
|
||||||
|
var releaseId = row1.Value!.ReleaseId;
|
||||||
|
Assert.That(releaseId, Is.Not.Null);
|
||||||
|
|
||||||
|
// Row 2 attaches to the just-created release — same (title, artist), but with the explicit id.
|
||||||
|
var row2 = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Track Two", "Artist A", "Live at the Vault", ReleaseMedium.Cut, releaseId);
|
||||||
|
|
||||||
|
Assert.That(row2.Success, Is.True, row2.Messages.FirstOrDefault()?.Message);
|
||||||
|
Assert.That(row2.Value!.ReleaseId, Is.EqualTo(releaseId), "row 2 lands on the same release row 1 created");
|
||||||
|
|
||||||
|
var peek = (await ((ITrackService)manager).GetReleaseByTitleAndArtist("Live at the Vault", "Artist A")).Value!;
|
||||||
|
Assert.That(peek.TrackCount, Is.EqualTo(2), "both within-batch tracks are on the one release");
|
||||||
|
}
|
||||||
|
|
||||||
|
// The existing single-track cardinality rule still fires on the attach path: a Session already
|
||||||
|
// holding its one track rejects a second add with the cardinality marker (controller → 409). This
|
||||||
|
// is reachable here only via an explicit releaseId, since a no-id second submit is the duplicate path.
|
||||||
|
[Test]
|
||||||
|
public async Task UploadAsync_SecondTrackOnSingleTrackRelease_IsBlockedWithCardinalityMarker()
|
||||||
|
{
|
||||||
|
var manager = CreateManager();
|
||||||
|
var service = await CreateUnifiedServiceAsync(manager);
|
||||||
|
|
||||||
|
var first = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Live Set", "DJ A", "Live Set", ReleaseMedium.Session, releaseId: null);
|
||||||
|
Assert.That(first.Success, Is.True);
|
||||||
|
var releaseId = first.Value!.ReleaseId;
|
||||||
|
|
||||||
|
// A second track aimed at the same single-track Session via its id → cardinality rejection.
|
||||||
|
var second = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Second Take", "DJ A", "Live Set", ReleaseMedium.Session, releaseId);
|
||||||
|
|
||||||
|
Assert.That(second.Success, Is.False);
|
||||||
|
var message = second.Messages.FirstOrDefault()?.Message ?? string.Empty;
|
||||||
|
Assert.That(message, Does.StartWith(UnifiedTrackService.CardinalityViolationMarker));
|
||||||
|
}
|
||||||
|
|
||||||
|
// ATTACH anti-forgery guard: when the caller supplies a releaseId that does NOT match the release
|
||||||
|
// the natural key (title, artist) resolves to, the upload is rejected. Guards against a stale or
|
||||||
|
// forged releaseId pointing at a different (title, artist) than this row carries.
|
||||||
|
[Test]
|
||||||
|
public async Task UploadAsync_AttachWithMismatchedReleaseId_IsRejectedWithDuplicateMarker()
|
||||||
|
{
|
||||||
|
var manager = CreateManager();
|
||||||
|
var service = await CreateUnifiedServiceAsync(manager);
|
||||||
|
|
||||||
|
// Create two separate releases so we have two distinct ids.
|
||||||
|
var releaseA = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Track One", "Artist A", "Release A", ReleaseMedium.Cut, releaseId: null);
|
||||||
|
Assert.That(releaseA.Success, Is.True, "release A must be created");
|
||||||
|
var idA = releaseA.Value!.ReleaseId!.Value;
|
||||||
|
|
||||||
|
var releaseB = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Track One", "Artist B", "Release B", ReleaseMedium.Cut, releaseId: null);
|
||||||
|
Assert.That(releaseB.Success, Is.True, "release B must be created");
|
||||||
|
|
||||||
|
// Try to ATTACH to release A while carrying release B's (title, artist). The natural-key lookup
|
||||||
|
// resolves to B — id A ≠ B.Id → anti-forgery guard fires.
|
||||||
|
var forged = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Track Two", "Artist B", "Release B", ReleaseMedium.Cut, releaseId: idA);
|
||||||
|
|
||||||
|
Assert.That(forged.Success, Is.False);
|
||||||
|
var message = forged.Messages.FirstOrDefault()?.Message ?? string.Empty;
|
||||||
|
Assert.That(message, Does.StartWith(UnifiedTrackService.DuplicateReleaseMarker));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Loose-track success: an upload with null/whitespace album stays release-less (ReleaseId null).
|
||||||
|
// Confirms the duplicate guard is correctly bypassed for tracks that carry no album.
|
||||||
|
[Test]
|
||||||
|
public async Task UploadAsync_NullAlbum_SucceedsAsLooseTrack()
|
||||||
|
{
|
||||||
|
var service = await CreateUnifiedServiceAsync(CreateManager());
|
||||||
|
|
||||||
|
var result = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Standalone Cut", "DJ Solo", album: null, ReleaseMedium.Cut, releaseId: null);
|
||||||
|
|
||||||
|
Assert.That(result.Success, Is.True, result.Messages.FirstOrDefault()?.Message);
|
||||||
|
Assert.That(result.Value!.ReleaseId, Is.Null, "a null-album track must stay a loose track with no release");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Case-sensitivity caveat: the assertion below verifies ordinal == equality as implemented by the
|
||||||
|
// EF in-memory provider (which evaluates LINQ predicates in-process). The deployed PostgreSQL
|
||||||
|
// instance may use a different column collation (e.g. case-insensitive) — production case-sensitivity
|
||||||
|
// depends on the collation of the `release` table's `title` and `artist` columns, not on this test.
|
||||||
|
// Matching semantics: GetReleaseByTitleAndArtist (the read both the pre-flight and the create-path
|
||||||
|
// duplicate guard use) is exact — a case difference is NOT a match, so it does not trip the block.
|
||||||
|
// This asserts the pre-flight and the create path agree by using the one shared read.
|
||||||
|
[Test]
|
||||||
|
public async Task UploadAsync_CaseDifferentTitle_IsNotADuplicateOnInMemoryProvider()
|
||||||
|
{
|
||||||
|
var service = await CreateUnifiedServiceAsync(CreateManager());
|
||||||
|
|
||||||
|
var first = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Studio Album", "Artist C", "Studio Album", ReleaseMedium.Cut, releaseId: null);
|
||||||
|
Assert.That(first.Success, Is.True);
|
||||||
|
|
||||||
|
// Different case → not the same natural key under the in-memory provider's ordinal == →
|
||||||
|
// admitted as a new release. Production outcome depends on PostgreSQL column collation.
|
||||||
|
var differentCase = await UploadAsync(
|
||||||
|
service, await WriteWavAsync(2.0), "Studio Album", "Artist C", "STUDIO ALBUM", ReleaseMedium.Cut, releaseId: null);
|
||||||
|
|
||||||
|
Assert.That(differentCase.Success, Is.True, differentCase.Messages.FirstOrDefault()?.Message);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Builds a standard-PCM mono 16-bit 44.1 kHz WAV of the requested duration with a full-scale square
|
||||||
|
// wave (non-silent so the loudness algorithm yields a real envelope). Same layout as
|
||||||
|
// TrackReplaceAudioTests / WaveformProfileServiceTests.
|
||||||
|
private static byte[] BuildMinimalPcmWav(double durationSeconds)
|
||||||
|
{
|
||||||
|
const int sampleRate = 44100;
|
||||||
|
const ushort channels = 1;
|
||||||
|
const ushort bitsPerSample = 16;
|
||||||
|
const ushort blockAlign = channels * (bitsPerSample / 8);
|
||||||
|
const uint byteRate = sampleRate * blockAlign;
|
||||||
|
|
||||||
|
var frames = (int)(sampleRate * durationSeconds);
|
||||||
|
var data = new byte[frames * blockAlign];
|
||||||
|
for (var i = 0; i < frames; i++)
|
||||||
|
{
|
||||||
|
var sample = (i % 2 == 0) ? short.MaxValue : short.MinValue;
|
||||||
|
data[i * 2] = (byte)(sample & 0xFF);
|
||||||
|
data[i * 2 + 1] = (byte)((sample >> 8) & 0xFF);
|
||||||
|
}
|
||||||
|
|
||||||
|
using var ms = new MemoryStream();
|
||||||
|
using var w = new BinaryWriter(ms, Encoding.ASCII, leaveOpen: true);
|
||||||
|
|
||||||
|
w.Write(Encoding.ASCII.GetBytes("RIFF"));
|
||||||
|
w.Write((uint)(36 + data.Length));
|
||||||
|
w.Write(Encoding.ASCII.GetBytes("WAVE"));
|
||||||
|
|
||||||
|
w.Write(Encoding.ASCII.GetBytes("fmt "));
|
||||||
|
w.Write(16u);
|
||||||
|
w.Write((ushort)1); // PCM
|
||||||
|
w.Write(channels);
|
||||||
|
w.Write((uint)sampleRate);
|
||||||
|
w.Write(byteRate);
|
||||||
|
w.Write(blockAlign);
|
||||||
|
w.Write(bitsPerSample);
|
||||||
|
|
||||||
|
w.Write(Encoding.ASCII.GetBytes("data"));
|
||||||
|
w.Write((uint)data.Length);
|
||||||
|
w.Write(data);
|
||||||
|
|
||||||
|
w.Flush();
|
||||||
|
return ms.ToArray();
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user