diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs new file mode 100644 index 0000000000..4743f64242 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs @@ -0,0 +1,198 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +using static io.github.hatayama.UnityCliLoop.Infrastructure.ThirdPartyToolMigrationFileServiceConstants; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies batch migration writes commit atomically or roll back. + /// + public sealed class ThirdPartyToolMigrationFileWriterTests + { + [Test] + public void WriteBatch_WhenAllTargetsAreWritable_CommitsEveryFileAndLeavesNoSidecars() + { + // Verifies a mixed existing/new batch commits all targets and leaves no sidecar files. + string tempDirectory = CreateTempDirectory(); + try + { + string existingFile1 = Path.Combine(tempDirectory, "ExistingOne.cs"); + string existingFile2 = Path.Combine(tempDirectory, "ExistingTwo.cs"); + string newFile = Path.Combine(tempDirectory, "NewFile.cs"); + File.WriteAllText(existingFile1, "original-one"); + File.WriteAllText(existingFile2, "original-two"); + + List changes = new() + { + new MigrationFileChange(existingFile1, "migrated-one"), + new MigrationFileChange(existingFile2, "migrated-two"), + new MigrationFileChange(newFile, "migrated-new") + }; + + ThirdPartyToolMigrationFileWriter.WriteBatch(changes); + + Assert.That(File.ReadAllText(existingFile1), Is.EqualTo("migrated-one")); + Assert.That(File.ReadAllText(existingFile2), Is.EqualTo("migrated-two")); + Assert.That(File.ReadAllText(newFile), Is.EqualTo("migrated-new")); + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenPrepareFails_LeavesAllTargetFilesUntouched() + { + // Verifies a prepare failure leaves every target untouched and removes temp sidecars. + string tempDirectory = CreateTempDirectory(); + try + { + string existingFile = Path.Combine(tempDirectory, "Existing.cs"); + File.WriteAllText(existingFile, "original"); + string missingDirectoryFile = Path.Combine( + tempDirectory, + "missing-directory", + "Missing.cs"); + + List changes = new() + { + new MigrationFileChange(existingFile, "migrated"), + new MigrationFileChange(missingDirectoryFile, "never-written") + }; + + Assert.Throws( + () => ThirdPartyToolMigrationFileWriter.WriteBatch(changes)); + + Assert.That(File.ReadAllText(existingFile), Is.EqualTo("original")); + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenCommitFails_RestoresCommittedFilesFromBackups() + { + // Verifies a mid-batch commit failure restores already committed files from backups. + string tempDirectory = CreateTempDirectory(); + try + { + string file1 = Path.Combine(tempDirectory, "FileOne.cs"); + string file2 = Path.Combine(tempDirectory, "FileTwo.cs"); + string file3 = Path.Combine(tempDirectory, "FileThree.cs"); + File.WriteAllText(file1, "original-one"); + File.WriteAllText(file3, "original-three"); + Directory.CreateDirectory(file2); + + List changes = new() + { + new MigrationFileChange(file1, "migrated-one"), + new MigrationFileChange(file2, "migrated-two"), + new MigrationFileChange(file3, "migrated-three") + }; + + Assert.Throws( + () => ThirdPartyToolMigrationFileWriter.WriteBatch(changes)); + + Assert.That(File.ReadAllText(file1), Is.EqualTo("original-one")); + Assert.That(File.ReadAllText(file3), Is.EqualTo("original-three")); + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenCommitFails_DeletesNewlyCreatedFiles() + { + // Verifies rollback removes newly created targets that had no backup sidecar. + string tempDirectory = CreateTempDirectory(); + try + { + string newFile = Path.Combine(tempDirectory, "NewFile.cs"); + string failingTarget = Path.Combine(tempDirectory, "FailingTarget.cs"); + Directory.CreateDirectory(failingTarget); + + List changes = new() + { + new MigrationFileChange(newFile, "migrated-new"), + new MigrationFileChange(failingTarget, "never-committed") + }; + + Assert.Throws( + () => ThirdPartyToolMigrationFileWriter.WriteBatch(changes)); + + Assert.That(File.Exists(newFile), Is.False); + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public async Task WriteBatchAsync_WhenBatchExceedsYieldSize_CommitsEveryFile() + { + // Verifies the async prepare yield path still commits every file in a large batch. + string tempDirectory = CreateTempDirectory(); + try + { + List changes = new(); + for (int index = 0; index < PreviewYieldBatchSize + 8; index++) + { + string filePath = Path.Combine(tempDirectory, $"File{index:D2}.cs"); + File.WriteAllText(filePath, $"original-{index}"); + changes.Add(new MigrationFileChange(filePath, $"migrated-{index}")); + } + + await ThirdPartyToolMigrationFileWriter.WriteBatchAsync(changes); + + for (int index = 0; index < changes.Count; index++) + { + Assert.That( + File.ReadAllText(changes[index].FilePath), + Is.EqualTo($"migrated-{index}")); + } + + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + private static string CreateTempDirectory() + { + string tempDirectory = Path.Combine( + Path.GetTempPath(), + "UnityCliLoopMigrationWriterTests", + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(tempDirectory); + return tempDirectory; + } + + private static int CountSidecarFiles(string directory) + { + return Directory.EnumerateFiles(directory, "*", SearchOption.AllDirectories) + .Count(filePath => + filePath.EndsWith(".tmp", StringComparison.OrdinalIgnoreCase) || + filePath.EndsWith(".bak", StringComparison.OrdinalIgnoreCase)); + } + } +} diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs.meta b/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs.meta new file mode 100644 index 0000000000..e1c031c441 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 1e2826ae14e624b0d81d3bf76a647d5b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs index 582fb815ed..a9fc880b18 100644 --- a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs @@ -92,10 +92,7 @@ public ThirdPartyToolMigrationResult ApplyMigration(string projectRoot) string normalizedProjectRoot = NormalizeProjectRoot(projectRoot); MigrationPlan plan = GetCurrentMigrationPlan(normalizedProjectRoot); InvalidatePreviewCache(); - foreach (MigrationFileChange change in plan.Changes) - { - ThirdPartyToolMigrationFileWriter.Write(change.FilePath, change.Content); - } + ThirdPartyToolMigrationFileWriter.WriteBatch(plan.Changes); return new ThirdPartyToolMigrationResult( plan.ChangedFilePaths.Count, @@ -120,15 +117,7 @@ public async Task ApplyMigrationAsync( } InvalidatePreviewCache(); - for (int index = 0; index < plan.Changes.Count; index++) - { - MigrationFileChange change = plan.Changes[index]; - ThirdPartyToolMigrationFileWriter.Write(change.FilePath, change.Content); - if ((index + 1) % ThirdPartyToolMigrationFileServiceConstants.PreviewYieldBatchSize == 0) - { - await Task.Yield(); - } - } + await ThirdPartyToolMigrationFileWriter.WriteBatchAsync(plan.Changes); return new ThirdPartyToolMigrationResult( plan.ChangedFilePaths.Count, diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs index 9cd2875ecd..f121d7f3f3 100644 --- a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs @@ -2,30 +2,213 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Threading.Tasks; namespace io.github.hatayama.UnityCliLoop.Infrastructure { /// - /// Writes migrated files atomically through temporary sidecar files. + /// Writes a migration plan as one batch transaction: every temp sidecar is written first, + /// then all targets are committed via rename/replace while their backups are kept, and a + /// mid-batch commit failure rolls the already committed files back from those backups. /// internal static class ThirdPartyToolMigrationFileWriter { - internal static void Write(string filePath, string content) + private const string TempSidecarExtension = ".tmp"; + private const string BackupSidecarExtension = ".bak"; + + internal static void WriteBatch(IReadOnlyList changes) { - Debug.Assert(!string.IsNullOrEmpty(filePath), "filePath must not be null or empty"); - Debug.Assert(content != null, "content must not be null"); + Debug.Assert(changes != null, "changes must not be null"); + + List preparedWrites = PrepareAll(changes); + CommitAll(preparedWrites); + } + + internal static async Task WriteBatchAsync(IReadOnlyList changes) + { + Debug.Assert(changes != null, "changes must not be null"); + + List preparedWrites = await PrepareAllAsync(changes); + // Commit is rename-only and fast; it must run without yields so no editor callback + // (asset refresh, domain reload) can observe a half-committed batch. + CommitAll(preparedWrites); + } + + private static List PrepareAll(IReadOnlyList changes) + { + List preparedWrites = new(changes.Count); + bool prepared = false; + try + { + foreach (MigrationFileChange change in changes) + { + Prepare(preparedWrites, change); + } + + prepared = true; + return preparedWrites; + } + finally + { + if (!prepared) + { + // A prepare failure has not touched any target file yet; deleting the temp + // sidecars returns the project to its exact pre-apply state. + DeleteTempSidecars(preparedWrites, 0); + } + } + } + + private static async Task> PrepareAllAsync( + IReadOnlyList changes) + { + List preparedWrites = new(changes.Count); + bool prepared = false; + try + { + for (int index = 0; index < changes.Count; index++) + { + Prepare(preparedWrites, changes[index]); + if ((index + 1) % ThirdPartyToolMigrationFileServiceConstants.PreviewYieldBatchSize == 0) + { + await Task.Yield(); + } + } - string tempFilePath = CreateUniqueSidecarPath(filePath, ".tmp"); - ThirdPartyToolMigrationFileAccess.WriteAllText(tempFilePath, content); - if (!ThirdPartyToolMigrationFileAccess.Exists(filePath)) + prepared = true; + return preparedWrites; + } + finally { - ThirdPartyToolMigrationFileAccess.Move(tempFilePath, filePath); - return; + if (!prepared) + { + DeleteTempSidecars(preparedWrites, 0); + } } + } + + private static void Prepare(List preparedWrites, MigrationFileChange change) + { + string tempFilePath = CreateUniqueSidecarPath(change.FilePath, TempSidecarExtension); + // Register before WriteAllText so a mid-write IOException can still clean up a partial .tmp. + preparedWrites.Add(new PreparedWrite(change.FilePath, tempFilePath)); + ThirdPartyToolMigrationFileAccess.WriteAllText(tempFilePath, change.Content); + } + + private static void CommitAll(List preparedWrites) + { + List committedWrites = new(preparedWrites.Count); + bool committed = false; + try + { + foreach (PreparedWrite preparedWrite in preparedWrites) + { + committedWrites.Add(Commit(preparedWrite)); + } - string backupFilePath = CreateUniqueSidecarPath(filePath, ".bak"); - ThirdPartyToolMigrationFileAccess.Replace(tempFilePath, filePath, backupFilePath); - ThirdPartyToolMigrationFileAccess.Delete(backupFilePath); + committed = true; + } + finally + { + if (committed) + { + DeleteBackupSidecars(committedWrites); + } + else + { + // The commit exception is propagating right now; rollback and cleanup must be + // best-effort so they never replace that original exception. + RollbackCommitted(committedWrites); + DeleteTempSidecars(preparedWrites, committedWrites.Count); + } + } + } + + private static CommittedWrite Commit(PreparedWrite preparedWrite) + { + if (!ThirdPartyToolMigrationFileAccess.Exists(preparedWrite.TargetFilePath)) + { + ThirdPartyToolMigrationFileAccess.Move( + preparedWrite.TempFilePath, + preparedWrite.TargetFilePath); + return new CommittedWrite(preparedWrite.TargetFilePath, null); + } + + string backupFilePath = CreateUniqueSidecarPath( + preparedWrite.TargetFilePath, + BackupSidecarExtension); + ThirdPartyToolMigrationFileAccess.Replace( + preparedWrite.TempFilePath, + preparedWrite.TargetFilePath, + backupFilePath); + return new CommittedWrite(preparedWrite.TargetFilePath, backupFilePath); + } + + private static void RollbackCommitted(List committedWrites) + { + for (int index = committedWrites.Count - 1; index >= 0; index--) + { + CommittedWrite committedWrite = committedWrites[index]; + try + { + ThirdPartyToolMigrationFileAccess.Delete(committedWrite.TargetFilePath); + if (committedWrite.BackupFilePath != null) + { + ThirdPartyToolMigrationFileAccess.Move( + committedWrite.BackupFilePath, + committedWrite.TargetFilePath); + } + } + catch (Exception restoreException) + { + // Keep restoring the remaining files; a file that cannot be restored keeps + // its .bak on disk so the user can recover it manually. + UnityEngine.Debug.LogError( + $"[uloop] Migration rollback failed for '{committedWrite.TargetFilePath}': " + + $"{restoreException.Message}" + + (committedWrite.BackupFilePath == null + ? string.Empty + : $" Backup kept at '{committedWrite.BackupFilePath}'.")); + } + } + } + + private static void DeleteTempSidecars(List preparedWrites, int firstIndex) + { + for (int index = firstIndex; index < preparedWrites.Count; index++) + { + TryDeleteSidecar(preparedWrites[index].TempFilePath); + } + } + + private static void DeleteBackupSidecars(List committedWrites) + { + foreach (CommittedWrite committedWrite in committedWrites) + { + if (committedWrite.BackupFilePath != null) + { + TryDeleteSidecar(committedWrite.BackupFilePath); + } + } + } + + private static void TryDeleteSidecar(string sidecarFilePath) + { + try + { + if (ThirdPartyToolMigrationFileAccess.Exists(sidecarFilePath)) + { + ThirdPartyToolMigrationFileAccess.Delete(sidecarFilePath); + } + } + catch (Exception deleteException) + { + // Sidecar cleanup must never fail the migration itself; a stray sidecar is + // visible in the project window and harmless to compilation. + UnityEngine.Debug.LogError( + $"[uloop] Failed to delete migration sidecar '{sidecarFilePath}': " + + $"{deleteException.Message}"); + } } internal static string CreateUniqueSidecarPath(string filePath, string extension) @@ -43,6 +226,31 @@ internal static string CreateUniqueSidecarPath(string filePath, string extension return sidecarPath; } + + private readonly struct PreparedWrite + { + public PreparedWrite(string targetFilePath, string tempFilePath) + { + TargetFilePath = targetFilePath; + TempFilePath = tempFilePath; + } + + public string TargetFilePath { get; } + public string TempFilePath { get; } + } + + private readonly struct CommittedWrite + { + public CommittedWrite(string targetFilePath, string backupFilePath) + { + TargetFilePath = targetFilePath; + BackupFilePath = backupFilePath; + } + + public string TargetFilePath { get; } + // Null when the target file did not exist before the commit (plain move, no backup). + public string BackupFilePath { get; } + } } ///