From ca90302f217886fa0090b929187e7aeb8ae82156 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 18 Jun 2026 13:11:59 -0400 Subject: [PATCH] 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. --- DeepDrftContent/TrackContentService.cs | 54 ++++++++++++++----- .../Components/Pages/Tracks/BatchEdit.razor | 3 ++ DeepDrftManager/Services/CmsTrackService.cs | 4 +- DeepDrftTests/TrackReplaceAudioTests.cs | 3 +- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/DeepDrftContent/TrackContentService.cs b/DeepDrftContent/TrackContentService.cs index 92d3ea9..1f0b778 100644 --- a/DeepDrftContent/TrackContentService.cs +++ b/DeepDrftContent/TrackContentService.cs @@ -104,16 +104,25 @@ public class TrackContentService /// Swaps the audio bytes for an existing track in place: processes a new audio file and /// re-registers it under the SAME in the tracks vault. The track's /// 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 - /// extension differs from the original (e.g. .wav → .flac) does not strand the old file on disk - /// under its former filename. Returns the freshly stored on success - /// (so the caller can regenerate waveform data from the same bytes), or null on processing or - /// vault failure — matching the FileDatabase swallow-and-return-null contract. + /// untouched; only the binary changes. The new audio is written first; only on confirmed success + /// is a stale old backing file cleaned up. A cross-format replacement (e.g. .wav → .flac) leaves + /// the old file on disk under its former filename once the index is updated; the post-success + /// cleanup removes it. For a same-extension overwrite the register alone suffices — the file is + /// 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 on success + /// (so the caller can regenerate waveform data from the same bytes) — matching the FileDatabase + /// swallow-and-return-null contract. /// public async Task ReplaceTrackAudioAsync(string entryKey, string audioFilePath) { 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(VaultConstants.Tracks, entryKey); + var oldExtension = existing?.Extension; + var audioBinary = await _audioProcessorRouter.ProcessAudioFileAsync(audioFilePath); if (audioBinary == null) { @@ -126,19 +135,40 @@ public class TrackContentService await _fileDatabase.CreateVaultAsync(VaultConstants.Tracks, MediaVaultType.Audio); } - // Drop the old entry first. The backing file is keyed by entryKey + its *stored* - // extension, so a register alone would leave a stale file when the new format differs. - // 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); - + // Register the new audio. This upserts the index entry (new extension recorded) and + // writes the new file to disk. If this fails the original entry and file are untouched. var success = await _fileDatabase.RegisterResourceAsync(VaultConstants.Tracks, entryKey, audioBinary); 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; } + // 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; } catch (Exception ex) when (ex is not OperationCanceledException) diff --git a/DeepDrftManager/Components/Pages/Tracks/BatchEdit.razor b/DeepDrftManager/Components/Pages/Tracks/BatchEdit.razor index 40802e8..f6d4442 100644 --- a/DeepDrftManager/Components/Pages/Tracks/BatchEdit.razor +++ b/DeepDrftManager/Components/Pages/Tracks/BatchEdit.razor @@ -59,6 +59,9 @@ single-track medium, mirroring BatchUpload's same-named collapse. Cut keeps the full list. *@ + @* 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. *@