fix: register-new-then-remove-old in ReplaceTrackAudioAsync; replace wording in timeout messages; doc comment on ExistingTrackCount

On partial failure the old path deleted the original audio before
confirming the new write succeeded. Now: load old extension, register
new audio first (original untouched on failure), then clean up stale
backing file only on success and only when extension changed.
This commit is contained in:
daniel-c-harvey
2026-06-18 13:11:59 -04:00
parent 16784b37f2
commit ca90302f21
4 changed files with 49 additions and 15 deletions
+42 -12
View File
@@ -104,16 +104,25 @@ public class TrackContentService
/// Swaps the audio bytes for an existing track in place: processes a new audio file and /// Swaps the audio bytes for an existing track in place: processes a new audio file and
/// re-registers it under the SAME <paramref name="entryKey"/> in the tracks vault. The track's /// re-registers it under the SAME <paramref name="entryKey"/> in the tracks vault. The track's
/// vault key — and therefore its SQL link, release membership, position, and metadata — is /// vault key — and therefore its SQL link, release membership, position, and metadata — is
/// untouched; only the binary changes. The prior entry is removed first so a replacement whose /// untouched; only the binary changes. The new audio is written first; only on confirmed success
/// extension differs from the original (e.g. .wav → .flac) does not strand the old file on disk /// is a stale old backing file cleaned up. A cross-format replacement (e.g. .wav → .flac) leaves
/// under its former filename. Returns the freshly stored <see cref="AudioBinary"/> on success /// the old file on disk under its former filename once the index is updated; the post-success
/// (so the caller can regenerate waveform data from the same bytes), or null on processing or /// cleanup removes it. For a same-extension overwrite the register alone suffices — the file is
/// vault failure — matching the FileDatabase swallow-and-return-null contract. /// written in place. If the register fails the original audio is left intact and null is returned,
/// so the track remains playable. Returns the freshly stored <see cref="AudioBinary"/> on success
/// (so the caller can regenerate waveform data from the same bytes) — matching the FileDatabase
/// swallow-and-return-null contract.
/// </summary> /// </summary>
public async Task<AudioBinary?> ReplaceTrackAudioAsync(string entryKey, string audioFilePath) public async Task<AudioBinary?> ReplaceTrackAudioAsync(string entryKey, string audioFilePath)
{ {
try try
{ {
// Capture the old extension before touching the vault. After register the index
// will point to the new extension, so we need the old value now to detect a
// cross-format swap and clean up the stale file post-success.
var existing = await _fileDatabase.LoadResourceAsync<AudioBinary>(VaultConstants.Tracks, entryKey);
var oldExtension = existing?.Extension;
var audioBinary = await _audioProcessorRouter.ProcessAudioFileAsync(audioFilePath); var audioBinary = await _audioProcessorRouter.ProcessAudioFileAsync(audioFilePath);
if (audioBinary == null) if (audioBinary == null)
{ {
@@ -126,19 +135,40 @@ public class TrackContentService
await _fileDatabase.CreateVaultAsync(VaultConstants.Tracks, MediaVaultType.Audio); await _fileDatabase.CreateVaultAsync(VaultConstants.Tracks, MediaVaultType.Audio);
} }
// Drop the old entry first. The backing file is keyed by entryKey + its *stored* // Register the new audio. This upserts the index entry (new extension recorded) and
// extension, so a register alone would leave a stale file when the new format differs. // writes the new file to disk. If this fails the original entry and file are untouched.
// A null/false removal is non-fatal (the entry may already be absent); the register
// below is the authoritative write.
await _fileDatabase.RemoveResourceAsync(VaultConstants.Tracks, entryKey);
var success = await _fileDatabase.RegisterResourceAsync(VaultConstants.Tracks, entryKey, audioBinary); var success = await _fileDatabase.RegisterResourceAsync(VaultConstants.Tracks, entryKey, audioBinary);
if (!success) if (!success)
{ {
Console.WriteLine($"TrackContentService.ReplaceTrackAudioAsync: vault write failed for {entryKey}"); Console.WriteLine($"TrackContentService.ReplaceTrackAudioAsync: vault write failed for {entryKey}; original audio preserved");
return null; return null;
} }
// Post-success stale-file cleanup for cross-format swaps. The register wrote the new
// file (e.g. .flac) and updated the index to the new extension, but the old backing
// file (e.g. .wav) is now unreferenced on disk. Delete it directly by constructing the
// old path — RemoveResourceAsync would now resolve to the new extension and delete the
// wrong file. Non-fatal: an orphaned old file is a disk-hygiene concern, not a
// playback issue (the index no longer references it).
if (oldExtension != null && oldExtension != audioBinary.Extension)
{
var vault = _fileDatabase.GetVault(VaultConstants.Tracks);
if (vault != null)
{
var sanitizedKey = System.Text.RegularExpressions.Regex.Replace(entryKey, @"[^a-zA-Z0-9]", "-");
var staleFilePath = Path.Combine(vault.RootPath, $"{sanitizedKey}{oldExtension}");
try
{
if (File.Exists(staleFilePath))
File.Delete(staleFilePath);
}
catch (Exception ex)
{
Console.WriteLine($"TrackContentService.ReplaceTrackAudioAsync: stale backing-file removal failed for {entryKey} ({staleFilePath}): {ex.Message} — new audio is live; orphaned file may remain on disk");
}
}
}
return audioBinary; return audioBinary;
} }
catch (Exception ex) when (ex is not OperationCanceledException) catch (Exception ex) when (ex is not OperationCanceledException)
@@ -59,6 +59,9 @@
single-track medium, mirroring BatchUpload's same-named collapse. Cut keeps the full list. *@ single-track medium, mirroring BatchUpload's same-named collapse. Cut keeps the full list. *@
<MudGrid> <MudGrid>
<MudItem xs="12" md="5"> <MudItem xs="12" md="5">
@* ExistingTrackCount counts edit-session persisted rows (Id.HasValue), not authoritative
live release count — acceptable because this gate only hides a UI control; the
TrySoftDeleteEmptyReleaseAsync backstop remains the authoritative guard. *@
<BatchTrackList Tracks="_tracks" <BatchTrackList Tracks="_tracks"
@bind-SelectedIndex="_selectedIndex" @bind-SelectedIndex="_selectedIndex"
Disabled="_saving" Disabled="_saving"
+2 -2
View File
@@ -272,12 +272,12 @@ public class CmsTrackService : ICmsTrackService
_owner._logger.LogWarning("{Operation} stalled — no progress for {IdleSeconds}s; aborting.", _owner._logger.LogWarning("{Operation} stalled — no progress for {IdleSeconds}s; aborting.",
operationLabel, _owner._uploadIdleTimeout.TotalSeconds); operationLabel, _owner._uploadIdleTimeout.TotalSeconds);
return LargeBodySendResult.Fail( return LargeBodySendResult.Fail(
$"Upload stalled — no data transferred for {_owner._uploadIdleTimeout.TotalSeconds:0}s. Please retry."); $"{operationLabel} stalled — no data transferred for {_owner._uploadIdleTimeout.TotalSeconds:0}s. Please retry.");
} }
_owner._logger.LogWarning("{Operation} timed out waiting for server response after {ResponseSeconds}s.", _owner._logger.LogWarning("{Operation} timed out waiting for server response after {ResponseSeconds}s.",
operationLabel, _owner._uploadResponseTimeout.TotalSeconds); operationLabel, _owner._uploadResponseTimeout.TotalSeconds);
return LargeBodySendResult.Fail( return LargeBodySendResult.Fail(
$"Upload timed out waiting for the server to respond after {_owner._uploadResponseTimeout.TotalSeconds:0}s. Please retry."); $"{operationLabel} timed out waiting for the server to respond after {_owner._uploadResponseTimeout.TotalSeconds:0}s. Please retry.");
} }
catch (Exception ex) catch (Exception ex)
{ {
+2 -1
View File
@@ -90,7 +90,8 @@ public class TrackReplaceAudioTests
var content = CreateContentService(fileDatabase!); var content = CreateContentService(fileDatabase!);
// A .wav original replaced by a .flac: the backing filename is keyed by extension, so a // A .wav original replaced by a .flac: the backing filename is keyed by extension, so a
// register-only swap would strand the old .wav. The replace removes the old entry first. // register-only swap would strand the old .wav. The replace writes the new entry first, then
// cleans up the stale old backing file (detected by comparing old vs. new extension).
var original = await WriteWavAsync(BuildMinimalPcmWav(2.0), ".wav"); var original = await WriteWavAsync(BuildMinimalPcmWav(2.0), ".wav");
var seeded = await content.AddTrackAsync(original, "Original", "Artist"); var seeded = await content.AddTrackAsync(original, "Original", "Artist");
var entryKey = seeded!.EntryKey; var entryKey = seeded!.EntryKey;