From fb3a32d49e6e14859c4aa6a146e270693bc216e3 Mon Sep 17 00:00:00 2001 From: BlackSpirits Date: Sun, 13 Sep 2026 15:45:05 +0200 Subject: [PATCH 1/3] Make TTS voice rename rollback-safe --- .../TextToSpeech/Engines/VoiceFileRename.cs | 130 +++++++++++++++--- 1 file changed, 108 insertions(+), 22 deletions(-) diff --git a/src/ui/Features/Video/TextToSpeech/Engines/VoiceFileRename.cs b/src/ui/Features/Video/TextToSpeech/Engines/VoiceFileRename.cs index a88fa79194..dc8dc02b78 100644 --- a/src/ui/Features/Video/TextToSpeech/Engines/VoiceFileRename.cs +++ b/src/ui/Features/Video/TextToSpeech/Engines/VoiceFileRename.cs @@ -1,6 +1,7 @@ using Nikse.SubtitleEdit.Features.Video.TextToSpeech.Voices; using Nikse.SubtitleEdit.Logic.Config; using System; +using System.Collections.Generic; using System.IO; using System.Linq; @@ -16,6 +17,13 @@ namespace Nikse.SubtitleEdit.Features.Video.TextToSpeech.Engines; /// public static class VoiceFileRename { + private sealed class RenameMove(string source, string target, string temp) + { + public string Source { get; } = source; + public string Target { get; } = target; + public string Temp { get; } = temp; + } + /// /// The reference recording clones from, or null when the voice is /// not a renamable file-backed clone: an engine preset, the "Default" speaker, the per-line @@ -136,46 +144,124 @@ public static bool Delete(Voice voice, out string error) return oldFileName; } - var sameFileDifferentCase = string.Equals(oldBaseName, newBaseName, StringComparison.OrdinalIgnoreCase); - if (!sameFileDifferentCase && File.Exists(newFileName)) + var sourceFiles = Directory.GetFiles(folder, oldBaseName + ".*") + .Where(file => string.Equals( + Path.GetFileNameWithoutExtension(file), + oldBaseName, + StringComparison.OrdinalIgnoreCase)) + .ToList(); + + if (!sourceFiles.Any(file => string.Equals(file, oldFileName, StringComparison.OrdinalIgnoreCase))) { - error = $"A voice named '{newName}' already exists"; - return null; + sourceFiles.Add(oldFileName); } + var moves = sourceFiles + .Select(source => new RenameMove( + source, + Path.Combine(folder, newBaseName + Path.GetExtension(source)), + Path.Combine(folder, $".se-voice-rename-{Guid.NewGuid():N}.tmp"))) + .ToList(); + + var staged = new List(); + var published = new List(); try { - // Sidecars first (a rename that fails half-way is still a usable voice); WAV last. - foreach (var sidecar in Directory.GetFiles(folder, oldBaseName + ".*")) + // Stage every source first. Besides making rollback possible, this makes a case-only + // rename portable: on a case-insensitive file system the destination stops existing + // once the source has been staged, while on a case-sensitive file system a distinct + // destination with the other casing remains and is detected below. + foreach (var move in moves) { - if (string.Equals(sidecar, oldFileName, StringComparison.OrdinalIgnoreCase) || - !string.Equals(Path.GetFileNameWithoutExtension(sidecar), oldBaseName, StringComparison.OrdinalIgnoreCase)) + File.Move(move.Source, move.Temp); + staged.Add(move); + } + + foreach (var move in moves) + { + if (File.Exists(move.Target)) { - continue; + throw new IOException($"A voice file named '{Path.GetFileName(move.Target)}' already exists"); } - - var target = Path.Combine(folder, newBaseName + Path.GetExtension(sidecar)); - File.Move(sidecar, target, overwrite: sameFileDifferentCase); } - File.Move(oldFileName, newFileName, overwrite: sameFileDifferentCase); - - // The prepared copy is keyed on the reference's file name; the next synthesis makes - // a fresh one under the new name, so the old one would only be an orphan. - var prepared = CloneReferenceTail.GetPreparedFileName(oldFileName); - foreach (var stale in new[] { prepared, prepared + ".stamp" }.Where(File.Exists)) + foreach (var move in moves) { - File.Delete(stale); + File.Move(move.Temp, move.Target); + published.Add(move); } - - Se.WriteToolsLog($"TTS voice renamed: '{oldFileName}' -> '{newFileName}'"); - return newFileName; } catch (Exception ex) { + RollbackRename(staged, published); Se.LogError(ex, $"Renaming TTS voice '{oldFileName}' to '{newFileName}' failed"); error = ex.Message; return null; } + + // The prepared copy is keyed on the reference's file name; the next synthesis makes a + // fresh one under the new name. Cache cleanup is best-effort and must not roll back an + // otherwise successful rename. + var prepared = CloneReferenceTail.GetPreparedFileName(oldFileName); + foreach (var stale in new[] { prepared, prepared + ".stamp" }.Where(File.Exists)) + { + try + { + File.Delete(stale); + } + catch (Exception ex) + { + Se.LogError(ex, $"Removing stale TTS voice cache '{stale}' failed"); + } + } + + Se.WriteToolsLog($"TTS voice renamed: '{oldFileName}' -> '{newFileName}'"); + return newFileName; + } + + private static void RollbackRename(List staged, List published) + { + // Published case-only paths can alias their original source on case-insensitive file + // systems. Move them through a unique temporary path so restoring the original casing is + // reliable without ever overwriting another voice. + for (var i = published.Count - 1; i >= 0; i--) + { + var move = published[i]; + if (!File.Exists(move.Target)) + { + continue; + } + + var rollbackTemp = Path.Combine( + Path.GetDirectoryName(move.Source) ?? string.Empty, + $".se-voice-rename-rollback-{Guid.NewGuid():N}.tmp"); + try + { + File.Move(move.Target, rollbackTemp); + File.Move(rollbackTemp, move.Source); + } + catch (Exception ex) + { + Se.LogError(ex, $"Rolling back TTS voice rename '{move.Target}' -> '{move.Source}' failed"); + } + } + + for (var i = staged.Count - 1; i >= 0; i--) + { + var move = staged[i]; + if (!File.Exists(move.Temp)) + { + continue; + } + + try + { + File.Move(move.Temp, move.Source); + } + catch (Exception ex) + { + Se.LogError(ex, $"Restoring staged TTS voice file '{move.Source}' failed"); + } + } } } From f33a8ea99d4f0278bc6306d4ead91986a5d4a3bd Mon Sep 17 00:00:00 2001 From: BlackSpirits Date: Sun, 13 Sep 2026 15:45:51 +0200 Subject: [PATCH 2/3] Add rollback-safe voice rename regressions --- .../Engines/VoiceFileRenameTests.cs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/UI/Features/Video/TextToSpeech/Engines/VoiceFileRenameTests.cs b/tests/UI/Features/Video/TextToSpeech/Engines/VoiceFileRenameTests.cs index cd33ea92ca..f203b502c6 100644 --- a/tests/UI/Features/Video/TextToSpeech/Engines/VoiceFileRenameTests.cs +++ b/tests/UI/Features/Video/TextToSpeech/Engines/VoiceFileRenameTests.cs @@ -72,6 +72,68 @@ public void Rename_RefusesExistingNameAndLeavesFilesAlone() Assert.True(File.Exists(Path.Combine(_folder, "B.wav"))); } + [Fact] + public void Rename_SidecarCollision_RollsBackAllStagedFiles() + { + var voice = MakeVoice("A"); + File.WriteAllText(Path.Combine(_folder, "A.txt"), "transcript"); + File.WriteAllText(Path.Combine(_folder, "A.json"), "metadata"); + File.WriteAllText(Path.Combine(_folder, "B.json"), "existing"); + + var result = VoiceFileRename.Rename(voice, "B", out var error); + + Assert.Null(result); + Assert.NotEqual(string.Empty, error); + Assert.True(File.Exists(Path.Combine(_folder, "A.wav"))); + Assert.Equal("transcript", File.ReadAllText(Path.Combine(_folder, "A.txt"))); + Assert.Equal("metadata", File.ReadAllText(Path.Combine(_folder, "A.json"))); + Assert.Equal("existing", File.ReadAllText(Path.Combine(_folder, "B.json"))); + Assert.False(File.Exists(Path.Combine(_folder, "B.wav"))); + Assert.False(File.Exists(Path.Combine(_folder, "B.txt"))); + Assert.Empty(Directory.GetFiles(_folder, ".se-voice-rename-*.tmp")); + } + + [Fact] + public void Rename_CaseOnlyRename_SucceedsWithoutOverwrite() + { + var voice = MakeVoice("CaseVoice"); + File.WriteAllText(Path.Combine(_folder, "CaseVoice.txt"), "transcript"); + + var result = VoiceFileRename.Rename(voice, "caseVoice", out var error); + + Assert.Equal(string.Empty, error); + Assert.Equal(Path.Combine(_folder, "caseVoice.wav"), result); + Assert.Contains( + Directory.GetFiles(_folder), + file => string.Equals(Path.GetFileName(file), "caseVoice.wav", StringComparison.Ordinal)); + Assert.Contains( + Directory.GetFiles(_folder), + file => string.Equals(Path.GetFileName(file), "caseVoice.txt", StringComparison.Ordinal)); + } + + [Fact] + public void Rename_CaseOnlyCollision_OnCaseSensitiveFileSystem_PreservesBothVoices() + { + var upper = MakeVoice("CaseVoice"); + MakeVoice("caseVoice"); + + if (Directory.GetFiles(_folder, "*.wav").Length < 2) + { + Assert.Skip("The current file system is case-insensitive."); + } + + File.WriteAllBytes(Path.Combine(_folder, "CaseVoice.wav"), new byte[] { 1, 2, 3 }); + File.WriteAllBytes(Path.Combine(_folder, "caseVoice.wav"), new byte[] { 4, 5, 6 }); + + var result = VoiceFileRename.Rename(upper, "caseVoice", out var error); + + Assert.Null(result); + Assert.NotEqual(string.Empty, error); + Assert.Equal(new byte[] { 1, 2, 3 }, File.ReadAllBytes(Path.Combine(_folder, "CaseVoice.wav"))); + Assert.Equal(new byte[] { 4, 5, 6 }, File.ReadAllBytes(Path.Combine(_folder, "caseVoice.wav"))); + Assert.Empty(Directory.GetFiles(_folder, ".se-voice-rename-*.tmp")); + } + [Fact] public void Rename_RefusesEmptyAndInvalidNames() { From a3f93dc9dd3007159370dca559340cdff74109aa Mon Sep 17 00:00:00 2001 From: BlackSpirits Date: Sun, 13 Sep 2026 15:47:50 +0200 Subject: [PATCH 3/3] Keep voice rename failures inside the existing error contract --- .../TextToSpeech/Engines/VoiceFileRename.cs | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/ui/Features/Video/TextToSpeech/Engines/VoiceFileRename.cs b/src/ui/Features/Video/TextToSpeech/Engines/VoiceFileRename.cs index dc8dc02b78..9d3589e29d 100644 --- a/src/ui/Features/Video/TextToSpeech/Engines/VoiceFileRename.cs +++ b/src/ui/Features/Video/TextToSpeech/Engines/VoiceFileRename.cs @@ -144,29 +144,29 @@ public static bool Delete(Voice voice, out string error) return oldFileName; } - var sourceFiles = Directory.GetFiles(folder, oldBaseName + ".*") - .Where(file => string.Equals( - Path.GetFileNameWithoutExtension(file), - oldBaseName, - StringComparison.OrdinalIgnoreCase)) - .ToList(); - - if (!sourceFiles.Any(file => string.Equals(file, oldFileName, StringComparison.OrdinalIgnoreCase))) - { - sourceFiles.Add(oldFileName); - } - - var moves = sourceFiles - .Select(source => new RenameMove( - source, - Path.Combine(folder, newBaseName + Path.GetExtension(source)), - Path.Combine(folder, $".se-voice-rename-{Guid.NewGuid():N}.tmp"))) - .ToList(); - var staged = new List(); var published = new List(); try { + var sourceFiles = Directory.GetFiles(folder, oldBaseName + ".*") + .Where(file => string.Equals( + Path.GetFileNameWithoutExtension(file), + oldBaseName, + StringComparison.OrdinalIgnoreCase)) + .ToList(); + + if (!sourceFiles.Any(file => string.Equals(file, oldFileName, StringComparison.OrdinalIgnoreCase))) + { + sourceFiles.Add(oldFileName); + } + + var moves = sourceFiles + .Select(source => new RenameMove( + source, + Path.Combine(folder, newBaseName + Path.GetExtension(source)), + Path.Combine(folder, $".se-voice-rename-{Guid.NewGuid():N}.tmp"))) + .ToList(); + // Stage every source first. Besides making rollback possible, this makes a case-only // rename portable: on a case-insensitive file system the destination stops existing // once the source has been staged, while on a case-sensitive file system a distinct