Handle more possible dir save extra data states

src/LibHac/FsSystem/DirectorySaveDataFileSystem.cs

@@ -728,6 +728,24 @@ public class DirectorySaveDataFileSystem : IFileSystem, ISaveDataExtraDataAccess
             (byte)'t', (byte)'a', (byte)'_'
         };
 
+    /// <summary>
+    /// Initializes the save data's extra data files.
+    /// </summary>
+    /// <returns></returns>
+    /// <remarks><para>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:</para>
+    /// <para>This is the state a properly committed save should be in.<br/>
+    /// Committed, Modified -> Use committed</para>
+    /// <para>This is the state the save will be in after an interrupted commit.<br/>
+    /// Working, Synchronizing -> Use modified</para>
+    /// <para>These states shouldn't normally happen. Use the committed file, ignoring the others.<br/>
+    /// Committed, Synchronizing -> Use committed<br/>
+    /// Committed, Modified, Synchronizing -> Use committed</para>
+    /// <para>If only one file exists then use that file.<br/>
+    /// Committed -> Use committed<br/>
+    /// Modified -> Use modified<br/>
+    /// Synchronizing -> Use synchronizing</para>
+    /// </remarks>
     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<SaveDataExtraData>());
             if (rc.IsFailure()) return rc;
 
             if (_isJournalingSupported)
             {
-                rc = _baseFs.CreateFile(in pathCommittedExtraData, Unsafe.SizeOf<SaveDataExtraData>());
+                rc = _baseFs.GetEntryType(out _, in pathCommittedExtraData);
-                if (rc.IsFailure() && !ResultFs.PathAlreadyExists.Includes(rc))
+
-                    return rc;
+                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<SaveDataExtraData>());
+                        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.
+                // The committed file doesn't exist. Try to recover from whatever invalid state we're in.
-                // 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;
 
-                rc = _baseFs.RenameFile(in pathSynchronizingExtraData, in pathCommittedExtraData);
+                // If the synchronizing file exists then the previous commit failed.
-                if (rc.IsFailure()) return rc;
+                // 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<SaveDataExtraData>());
+                    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
             {

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()
     {