From e140419323e1cbdb72db01b9e671535d485a39d8 Mon Sep 17 00:00:00 2001 From: Alex Barney Date: Sat, 5 Feb 2022 16:12:29 -0700 Subject: [PATCH] Handle more possible dir save extra data states --- .../FsSystem/DirectorySaveDataFileSystem.cs | 79 ++++++++++-- .../DirectorySaveDataFileSystemTests.cs | 114 +++++++++++++++++- 2 files changed, 178 insertions(+), 15 deletions(-) rename tests/LibHac.Tests/{Fs => FsSystem}/DirectorySaveDataFileSystemTests.cs (79%) diff --git a/src/LibHac/FsSystem/DirectorySaveDataFileSystem.cs b/src/LibHac/FsSystem/DirectorySaveDataFileSystem.cs index 61cd6549..ac1aca8d 100644 --- a/src/LibHac/FsSystem/DirectorySaveDataFileSystem.cs +++ b/src/LibHac/FsSystem/DirectorySaveDataFileSystem.cs @@ -728,6 +728,24 @@ public class DirectorySaveDataFileSystem : IFileSystem, ISaveDataExtraDataAccess (byte)'t', (byte)'a', (byte)'_' }; + /// + /// Initializes the save data's extra data files. + /// + /// + /// There's no telling what state users might leave the extra data files in, so we want + /// to be able to handle or recover from all possible states based on which files exist: + /// This is the state a properly committed save should be in.
+ /// Committed, Modified -> Use committed + /// This is the state the save will be in after an interrupted commit.
+ /// Working, Synchronizing -> Use modified + /// These states shouldn't normally happen. Use the committed file, ignoring the others.
+ /// Committed, Synchronizing -> Use committed
+ /// Committed, Modified, Synchronizing -> Use committed + /// If only one file exists then use that file.
+ /// Committed -> Use committed
+ /// Modified -> Use modified
+ /// Synchronizing -> Use synchronizing + /// private Result InitializeExtraData() { using var pathModifiedExtraData = new Path(); @@ -742,7 +760,8 @@ public class DirectorySaveDataFileSystem : IFileSystem, ISaveDataExtraDataAccess rc = PathFunctions.SetUpFixedPath(ref pathSynchronizingExtraData.Ref(), SynchronizingExtraDataName); if (rc.IsFailure()) return rc; - // Ensure the extra data files exist + // Ensure the extra data files exist. + // We don't currently handle the case where some of the extra data paths are directories instead of files. rc = _baseFs.GetEntryType(out _, in pathModifiedExtraData); if (rc.IsFailure()) @@ -750,14 +769,36 @@ public class DirectorySaveDataFileSystem : IFileSystem, ISaveDataExtraDataAccess if (!ResultFs.PathNotFound.Includes(rc)) return rc; + // The Modified file doesn't exist. Create it. rc = _baseFs.CreateFile(in pathModifiedExtraData, Unsafe.SizeOf()); if (rc.IsFailure()) return rc; if (_isJournalingSupported) { - rc = _baseFs.CreateFile(in pathCommittedExtraData, Unsafe.SizeOf()); - if (rc.IsFailure() && !ResultFs.PathAlreadyExists.Includes(rc)) - return rc; + rc = _baseFs.GetEntryType(out _, in pathCommittedExtraData); + + if (rc.IsFailure()) + { + if (!ResultFs.PathNotFound.Includes(rc)) + return rc; + + // Neither the modified or committed files existed. + // Check if the synchronizing file exists and use it if it does. + rc = _baseFs.GetEntryType(out _, in pathSynchronizingExtraData); + + if (rc.IsSuccess()) + { + rc = _baseFs.RenameFile(in pathSynchronizingExtraData, in pathCommittedExtraData); + if (rc.IsFailure()) return rc; + } + else + { + // The synchronizing file did not exist. Create an empty committed extra data file. + rc = _baseFs.CreateFile(in pathCommittedExtraData, Unsafe.SizeOf()); + if (rc.IsFailure() && !ResultFs.PathAlreadyExists.Includes(rc)) + return rc; + } + } } } else @@ -785,13 +826,31 @@ public class DirectorySaveDataFileSystem : IFileSystem, ISaveDataExtraDataAccess } else if (ResultFs.PathNotFound.Includes(rc)) { - // If a previous commit failed, the committed extra data may be missing. - // Finish that commit by copying the working extra data to the committed extra data - rc = SynchronizeExtraData(in pathSynchronizingExtraData, in pathModifiedExtraData); - if (rc.IsFailure()) return rc; + // The committed file doesn't exist. Try to recover from whatever invalid state we're in. - rc = _baseFs.RenameFile(in pathSynchronizingExtraData, in pathCommittedExtraData); - if (rc.IsFailure()) return rc; + // If the synchronizing file exists then the previous commit failed. + // Finish that commit by copying the working extra data to the committed extra data + if (_baseFs.GetEntryType(out _, in pathSynchronizingExtraData).IsSuccess()) + { + rc = SynchronizeExtraData(in pathSynchronizingExtraData, in pathModifiedExtraData); + if (rc.IsFailure()) return rc; + + rc = _baseFs.RenameFile(in pathSynchronizingExtraData, in pathCommittedExtraData); + if (rc.IsFailure()) return rc; + } + else + { + // The only existing file is the modified file. + // Copy the working extra data to the committed extra data. + rc = _baseFs.CreateFile(in pathSynchronizingExtraData, Unsafe.SizeOf()); + if (rc.IsFailure()) return rc; + + rc = SynchronizeExtraData(in pathSynchronizingExtraData, in pathModifiedExtraData); + if (rc.IsFailure()) return rc; + + rc = _baseFs.RenameFile(in pathSynchronizingExtraData, in pathCommittedExtraData); + if (rc.IsFailure()) return rc; + } } else { diff --git a/tests/LibHac.Tests/Fs/DirectorySaveDataFileSystemTests.cs b/tests/LibHac.Tests/FsSystem/DirectorySaveDataFileSystemTests.cs similarity index 79% rename from tests/LibHac.Tests/Fs/DirectorySaveDataFileSystemTests.cs rename to tests/LibHac.Tests/FsSystem/DirectorySaveDataFileSystemTests.cs index 9e2945f4..9f7f41fa 100644 --- a/tests/LibHac.Tests/Fs/DirectorySaveDataFileSystemTests.cs +++ b/tests/LibHac.Tests/FsSystem/DirectorySaveDataFileSystemTests.cs @@ -6,11 +6,12 @@ using LibHac.Fs; using LibHac.Fs.Fsa; using LibHac.FsSrv; using LibHac.FsSystem; +using LibHac.Tests.Fs; using LibHac.Tests.Fs.IFileSystemTestBase; using LibHac.Tools.Fs; using Xunit; -namespace LibHac.Tests.Fs; +namespace LibHac.Tests.FsSystem; public class DirectorySaveDataFileSystemTests : CommittableIFileSystemTests { @@ -79,7 +80,7 @@ public class DirectorySaveDataFileSystemTests : CommittableIFileSystemTests } [Fact] - public void CreateFile_CreatedInWorkingDirectory() + public void CreateFile_CreatedInModifiedDirectory() { (IFileSystem baseFs, IFileSystem saveFs) = CreateFileSystemInternal(); @@ -165,7 +166,7 @@ public class DirectorySaveDataFileSystemTests : CommittableIFileSystemTests } [Fact] - public void Initialize_InterruptedAfterCommitPart1_UsesWorkingData() + public void Initialize_InterruptedAfterCommitPart1_UsesModifiedData() { var baseFs = new InMemoryFileSystem(); @@ -183,7 +184,7 @@ public class DirectorySaveDataFileSystemTests : CommittableIFileSystemTests } [Fact] - public void Initialize_InterruptedDuringCommitPart2_UsesWorkingData() + public void Initialize_InterruptedDuringCommitPart2_UsesModifiedData() { var baseFs = new InMemoryFileSystem(); @@ -299,7 +300,7 @@ public class DirectorySaveDataFileSystemTests : CommittableIFileSystemTests } [Fact] - public void Initialize_InterruptedAfterCommitPart1_UsesWorkingExtraData() + public void Initialize_InterruptedAfterCommitPart1_UsesModifiedExtraData() { var baseFs = new InMemoryFileSystem(); @@ -313,6 +314,109 @@ public class DirectorySaveDataFileSystemTests : CommittableIFileSystemTests Assert.Equal(0x67890, extraData.DataSize); } + [Fact] + public void Initialize_OnlySynchronizingExtraDataExists_UsesSynchronizingExtraData() + { + var baseFs = new InMemoryFileSystem(); + + CreateExtraDataForTest(baseFs, "/ExtraData_", 0x67890).ThrowIfFailure(); + + CreateDirSaveFs(out DirectorySaveDataFileSystem saveFs, baseFs, true, true, true).ThrowIfFailure(); + + saveFs.ReadExtraData(out SaveDataExtraData extraData).ThrowIfFailure(); + + Assert.Equal(0x67890, extraData.DataSize); + } + + [Fact] + public void Initialize_OnlyCommittedExtraDataExists_UsesCommittedExtraData() + { + var baseFs = new InMemoryFileSystem(); + + CreateExtraDataForTest(baseFs, "/ExtraData0", 0x67890).ThrowIfFailure(); + + CreateDirSaveFs(out DirectorySaveDataFileSystem saveFs, baseFs, true, true, true).ThrowIfFailure(); + + saveFs.ReadExtraData(out SaveDataExtraData extraData).ThrowIfFailure(); + + Assert.Equal(0x67890, extraData.DataSize); + } + + [Fact] + public void Initialize_OnlyModifiedExtraDataExists_UsesModifiedExtraData() + { + var baseFs = new InMemoryFileSystem(); + + CreateExtraDataForTest(baseFs, "/ExtraData1", 0x67890).ThrowIfFailure(); + + CreateDirSaveFs(out DirectorySaveDataFileSystem saveFs, baseFs, true, true, true).ThrowIfFailure(); + + saveFs.ReadExtraData(out SaveDataExtraData extraData).ThrowIfFailure(); + + Assert.Equal(0x67890, extraData.DataSize); + } + + [Fact] + public void Initialize_SynchronizingAndCommittedExtraDataExist_UsesCommittedExtraData() + { + var baseFs = new InMemoryFileSystem(); + + CreateExtraDataForTest(baseFs, "/ExtraData_", 0x12345).ThrowIfFailure(); + CreateExtraDataForTest(baseFs, "/ExtraData0", 0x67890).ThrowIfFailure(); + + CreateDirSaveFs(out DirectorySaveDataFileSystem saveFs, baseFs, true, true, true).ThrowIfFailure(); + + saveFs.ReadExtraData(out SaveDataExtraData extraData).ThrowIfFailure(); + + Assert.Equal(0x67890, extraData.DataSize); + } + + [Fact] + public void Initialize_SynchronizingAndModifiedExtraDataExist_UsesModifiedExtraData() + { + var baseFs = new InMemoryFileSystem(); + + CreateExtraDataForTest(baseFs, "/ExtraData_", 0x12345).ThrowIfFailure(); + CreateExtraDataForTest(baseFs, "/ExtraData1", 0x67890).ThrowIfFailure(); + + CreateDirSaveFs(out DirectorySaveDataFileSystem saveFs, baseFs, true, true, true).ThrowIfFailure(); + + saveFs.ReadExtraData(out SaveDataExtraData extraData).ThrowIfFailure(); + + Assert.Equal(0x67890, extraData.DataSize); + } + + [Fact] + public void Initialize_CommittedAndModifiedExtraDataExist_UsesCommittedExtraData() + { + var baseFs = new InMemoryFileSystem(); + + CreateExtraDataForTest(baseFs, "/ExtraData0", 0x12345).ThrowIfFailure(); + CreateExtraDataForTest(baseFs, "/ExtraData1", 0x67890).ThrowIfFailure(); + + CreateDirSaveFs(out DirectorySaveDataFileSystem saveFs, baseFs, true, true, true).ThrowIfFailure(); + + saveFs.ReadExtraData(out SaveDataExtraData extraData).ThrowIfFailure(); + + Assert.Equal(0x12345, extraData.DataSize); + } + + [Fact] + public void Initialize_AllThreeExtraDataFilesExist_UsesCommittedExtraData() + { + var baseFs = new InMemoryFileSystem(); + + CreateExtraDataForTest(baseFs, "/ExtraData_", 0x12345).ThrowIfFailure(); + CreateExtraDataForTest(baseFs, "/ExtraData0", 0x54321).ThrowIfFailure(); + CreateExtraDataForTest(baseFs, "/ExtraData1", 0x67890).ThrowIfFailure(); + + CreateDirSaveFs(out DirectorySaveDataFileSystem saveFs, baseFs, true, true, true).ThrowIfFailure(); + + saveFs.ReadExtraData(out SaveDataExtraData extraData).ThrowIfFailure(); + + Assert.Equal(0x54321, extraData.DataSize); + } + [Fact] public void CommitSaveData_MultipleCommits_CommitIdIsUpdatedSkippingInvalidIds() {