Don't return duplicate entries in LayeredFS. Add tests

src/LibHac/FsSystem/LayeredFileSystem.cs

@@ -1,13 +1,35 @@
 using System;
 using System.Collections.Generic;
+using LibHac.Common;
 using LibHac.Fs;
 
 namespace LibHac.FsSystem
 {
     public class LayeredFileSystem : FileSystemBase
     {
+        /// <summary>
+        /// List of source <see cref="IFileSystem"/>s.
+        /// Filesystems at the beginning of the list will take precedence over those later in the list.
+        /// </summary>
         private List<IFileSystem> Sources { get; } = new List<IFileSystem>();
 
+        /// <summary>
+        /// Creates a new <see cref="LayeredFileSystem"/> from the input <see cref="IFileSystem"/> objects.
+        /// </summary>
+        /// <param name="lowerFileSystem">The base <see cref="IFileSystem"/>.</param>
+        /// <param name="upperFileSystem">The <see cref="IFileSystem"/> to be layered on top of the <paramref name="lowerFileSystem"/>.</param>
+        public LayeredFileSystem(IFileSystem lowerFileSystem, IFileSystem upperFileSystem)
+        {
+            Sources.Add(upperFileSystem);
+            Sources.Add(lowerFileSystem);
+        }
+
+        /// <summary>
+        /// Creates a new <see cref="LayeredFileSystem"/> from the input <see cref="IFileSystem"/> objects.
+        /// </summary>
+        /// <param name="sourceFileSystems">An <see cref="IList{IFileSystem}"/> containing the <see cref="IFileSystem"/>s
+        /// used to create the <see cref="LayeredFileSystem"/>. Filesystems at the beginning of the list will take
+        /// precedence over those later in the list.</param>
         public LayeredFileSystem(IList<IFileSystem> sourceFileSystems)
         {
             Sources.AddRange(sourceFileSystems);
@@ -18,29 +40,71 @@ namespace LibHac.FsSystem
             directory = default;
             path = PathTools.Normalize(path);
 
-            var dirs = new List<IDirectory>();
+            // Open directories from all layers so they can be merged
+            // Only allocate the list for multiple sources if needed
+            List<IFileSystem> multipleSources = null;
+            IFileSystem singleSource = null;
 
             foreach (IFileSystem fs in Sources)
             {
                 Result rc = fs.GetEntryType(out DirectoryEntryType entryType, path);
-                if (rc.IsFailure()) return rc;
 
-                if (entryType == DirectoryEntryType.File && dirs.Count == 0)
+                if (rc.IsSuccess())
                 {
-                    ThrowHelper.ThrowResult(ResultFs.PathNotFound.Value);
+                    // There were no directories with this path in higher levels, so the entry is a file
+                    if (entryType == DirectoryEntryType.File && singleSource is null)
+                    {
+                        return ResultFs.PathNotFound.Log();
                     }
 
                     if (entryType == DirectoryEntryType.Directory)
                     {
-                    rc = fs.OpenDirectory(out IDirectory subDirectory, path, mode);
-                    if (rc.IsFailure()) return rc;
-
-                    dirs.Add(subDirectory);
+                        if (singleSource is null)
+                        {
+                            singleSource = fs;
+                        }
+                        else if (multipleSources is null)
+                        {
+                            multipleSources = new List<IFileSystem> { singleSource, fs };
+                        }
+                        else
+                        {
+                            multipleSources.Add(fs);
+                        }
+                    }
+                }
+                else if (!ResultFs.PathNotFound.Includes(rc))
+                {
+                    return rc;
                 }
             }
 
-            directory = new LayeredFileSystemDirectory(dirs);
-            return Result.Success;
+            if (!(multipleSources is null))
+            {
+                var dir = new MergedDirectory(multipleSources, path, mode);
+                Result rc = dir.Initialize();
+
+                if (rc.IsSuccess())
+                {
+                    directory = dir;
+                }
+
+                return rc;
+            }
+
+            if (!(singleSource is null))
+            {
+                Result rc = singleSource.OpenDirectory(out IDirectory dir, path, mode);
+
+                if (rc.IsSuccess())
+                {
+                    directory = dir;
+                }
+
+                return rc;
+            }
+
+            return ResultFs.PathNotFound.Log();
         }
 
         protected override Result OpenFileImpl(out IFile file, string path, OpenMode mode)
@@ -51,8 +115,9 @@ namespace LibHac.FsSystem
             foreach (IFileSystem fs in Sources)
             {
                 Result rc = fs.GetEntryType(out DirectoryEntryType type, path);
-                if (rc.IsFailure()) return rc;
 
+                if (rc.IsSuccess())
+                {
                     if (type == DirectoryEntryType.File)
                     {
                         return fs.OpenFile(out file, path, mode);
@@ -63,6 +128,11 @@ namespace LibHac.FsSystem
                         return ResultFs.PathNotFound.Log();
                     }
                 }
+                else if (!ResultFs.PathNotFound.Includes(rc))
+                {
+                    return rc;
+                }
+            }
 
             return ResultFs.PathNotFound.Log();
         }
@@ -92,7 +162,7 @@ namespace LibHac.FsSystem
 
             foreach (IFileSystem fs in Sources)
             {
-                Result getEntryResult = fs.GetEntryType(out DirectoryEntryType type, path);
+                Result getEntryResult = fs.GetEntryType(out _, path);
 
                 if (getEntryResult.IsSuccess())
                 {
@@ -110,7 +180,7 @@ namespace LibHac.FsSystem
 
             foreach (IFileSystem fs in Sources)
             {
-                Result getEntryResult = fs.GetEntryType(out DirectoryEntryType type, path);
+                Result getEntryResult = fs.GetEntryType(out _, path);
 
                 if (getEntryResult.IsSuccess())
                 {
@@ -134,5 +204,95 @@ namespace LibHac.FsSystem
         protected override Result DeleteFileImpl(string path) => ResultFs.UnsupportedOperation.Log();
         protected override Result RenameDirectoryImpl(string oldPath, string newPath) => ResultFs.UnsupportedOperation.Log();
         protected override Result RenameFileImpl(string oldPath, string newPath) => ResultFs.UnsupportedOperation.Log();
+
+        private class MergedDirectory : IDirectory
+        {
+            // Needed to open new directories for GetEntryCount
+            private List<IFileSystem> SourceFileSystems { get; }
+            private List<IDirectory> SourceDirs { get; }
+            private string Path { get; }
+            private OpenDirectoryMode Mode { get; }
+
+            // todo: Efficient way to remove duplicates
+            private HashSet<string> Names { get; } = new HashSet<string>();
+
+            public MergedDirectory(List<IFileSystem> sourceFileSystems, string path, OpenDirectoryMode mode)
+            {
+                SourceFileSystems = sourceFileSystems;
+                SourceDirs = new List<IDirectory>(sourceFileSystems.Count);
+                Path = path;
+                Mode = mode;
+            }
+
+            public Result Initialize()
+            {
+                foreach (IFileSystem fs in SourceFileSystems)
+                {
+                    Result rc = fs.OpenDirectory(out IDirectory dir, Path, Mode);
+                    if (rc.IsFailure()) return rc;
+
+                    SourceDirs.Add(dir);
+                }
+
+                return Result.Success;
+            }
+
+            public Result Read(out long entriesRead, Span<DirectoryEntry> entryBuffer)
+            {
+                entriesRead = 0;
+                int entryIndex = 0;
+
+                for (int i = 0; i < SourceDirs.Count && entryIndex < entryBuffer.Length; i++)
+                {
+                    long subEntriesRead;
+
+                    do
+                    {
+                        Result rs = SourceDirs[i].Read(out subEntriesRead, entryBuffer.Slice(entryIndex, 1));
+                        if (rs.IsFailure()) return rs;
+
+                        if (subEntriesRead == 1 && Names.Add(StringUtils.Utf8ZToString(entryBuffer[entryIndex].Name)))
+                        {
+                            entryIndex++;
+                        }
+                    } while (subEntriesRead != 0 && entryIndex < entryBuffer.Length);
+                }
+
+                entriesRead = entryIndex;
+                return Result.Success;
+            }
+
+            public Result GetEntryCount(out long entryCount)
+            {
+                entryCount = 0;
+                long totalEntryCount = 0;
+                var entry = new DirectoryEntry();
+
+                // todo: Efficient way to remove duplicates
+                var names = new HashSet<string>();
+
+                // Open new directories for each source because we need to remove duplicate entries
+                foreach (IFileSystem fs in SourceFileSystems)
+                {
+                    Result rc = fs.OpenDirectory(out IDirectory dir, Path, Mode);
+                    if (rc.IsFailure()) return rc;
+
+                    long entriesRead;
+                    do
+                    {
+                        dir.Read(out entriesRead, SpanHelpers.AsSpan(ref entry));
+                        if (rc.IsFailure()) return rc;
+
+                        if (entriesRead == 1 && names.Add(StringUtils.Utf8ZToString(entry.Name)))
+                        {
+                            totalEntryCount++;
+                        }
+                    } while (entriesRead != 0);
+                }
+
+                entryCount = totalEntryCount;
+                return Result.Success;
+            }
+        }
     }
 }

src/LibHac/FsSystem/LayeredFileSystemDirectory.cs

@@ -1,52 +0,0 @@
-using System;
-using System.Collections.Generic;
-using LibHac.Fs;
-
-namespace LibHac.FsSystem
-{
-    public class LayeredFileSystemDirectory : IDirectory
-    {
-        private List<IDirectory> Sources { get; }
-
-        public LayeredFileSystemDirectory(List<IDirectory> sources)
-        {
-            Sources = sources;
-        }
-
-        // Todo: Don't return duplicate entries
-        public Result Read(out long entriesRead, Span<DirectoryEntry> entryBuffer)
-        {
-            entriesRead = 0;
-            int entryIndex = 0;
-
-            for (int i = 0; i < Sources.Count && entryIndex < entryBuffer.Length; i++)
-            {
-                Result rs = Sources[i].Read(out long subEntriesRead, entryBuffer.Slice(entryIndex));
-                if (rs.IsFailure()) return rs;
-
-                entryIndex += (int)subEntriesRead;
-            }
-
-            entriesRead = entryIndex;
-            return Result.Success;
-        }
-
-        // Todo: Don't count duplicate entries
-        public Result GetEntryCount(out long entryCount)
-        {
-            entryCount = 0;
-            long totalEntryCount = 0;
-
-            foreach (IDirectory dir in Sources)
-            {
-                Result rc = dir.GetEntryCount(out long subEntryCount);
-                if (rc.IsFailure()) return rc;
-
-                totalEntryCount += subEntryCount;
-            }
-
-            entryCount = totalEntryCount;
-            return Result.Success;
-        }
-    }
-}

tests/LibHac.Tests/Fs/IFileSystemTestBase/IFileSystemTests.IDirectory.cs

@@ -7,6 +7,29 @@ namespace LibHac.Tests.Fs.IFileSystemTestBase
 {
     public abstract partial class IFileSystemTests
     {
+        [Fact]
+        public void IDirectoryRead_EmptyFs_NoEntriesAreRead()
+        {
+            IFileSystem fs = CreateFileSystem();
+            Span<DirectoryEntry> entries = stackalloc DirectoryEntry[1];
+
+            Assert.Success(fs.OpenDirectory(out IDirectory directory, "/", OpenDirectoryMode.All));
+
+            Assert.Success(directory.Read(out long entriesRead, entries));
+            Assert.Equal(0, entriesRead);
+        }
+
+        [Fact]
+        public void IDirectoryGetEntryCount_EmptyFs_EntryCountIsZero()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Success(fs.OpenDirectory(out IDirectory directory, "/", OpenDirectoryMode.All));
+
+            Assert.Success(directory.GetEntryCount(out long entryCount));
+            Assert.Equal(0, entryCount);
+        }
+
         [Fact]
         public void IDirectoryRead_AllEntriesAreReturned()
         {

tests/LibHac.Tests/Fs/LayeredFileSystemTests.cs

@@ -0,0 +1,195 @@
+using System;
+using LibHac.Common;
+using LibHac.Fs;
+using LibHac.FsSystem;
+using Xunit;
+
+namespace LibHac.Tests.Fs
+{
+    public class LayeredFileSystemTests
+    {
+        private IFileSystem CreateFileSystem()
+        {
+            var lowerLayerFs = new InMemoryFileSystem();
+            var upperLayerFs = new InMemoryFileSystem();
+
+            var layeredFs = new LayeredFileSystem(lowerLayerFs, upperLayerFs);
+
+            lowerLayerFs.CreateDirectory("/dir").ThrowIfFailure();
+            upperLayerFs.CreateDirectory("/dir").ThrowIfFailure();
+            lowerLayerFs.CreateDirectory("/dir2").ThrowIfFailure();
+            upperLayerFs.CreateDirectory("/dir2").ThrowIfFailure();
+            lowerLayerFs.CreateDirectory("/dir3").ThrowIfFailure();
+            upperLayerFs.CreateDirectory("/dir3").ThrowIfFailure();
+
+            lowerLayerFs.CreateDirectory("/lowerDir").ThrowIfFailure();
+            upperLayerFs.CreateDirectory("/upperDir").ThrowIfFailure();
+
+            lowerLayerFs.CreateFile("/dir/replacedFile", 1, CreateFileOptions.None).ThrowIfFailure();
+            upperLayerFs.CreateFile("/dir/replacedFile", 2, CreateFileOptions.None).ThrowIfFailure();
+
+            lowerLayerFs.CreateFile("/dir2/lowerFile", 0, CreateFileOptions.None).ThrowIfFailure();
+            upperLayerFs.CreateFile("/dir2/upperFile", 0, CreateFileOptions.None).ThrowIfFailure();
+
+            lowerLayerFs.CreateFile("/dir3/lowerFile", 0, CreateFileOptions.None).ThrowIfFailure();
+            upperLayerFs.CreateFile("/dir3/upperFile", 2, CreateFileOptions.None).ThrowIfFailure();
+            lowerLayerFs.CreateFile("/dir3/replacedFile", 1, CreateFileOptions.None).ThrowIfFailure();
+            upperLayerFs.CreateFile("/dir3/replacedFile", 2, CreateFileOptions.None).ThrowIfFailure();
+
+            lowerLayerFs.CreateFile("/replacedWithDir", 0, CreateFileOptions.None).ThrowIfFailure();
+            upperLayerFs.CreateDirectory("/replacedWithDir").ThrowIfFailure();
+            upperLayerFs.CreateFile("/replacedWithDir/subFile", 0, CreateFileOptions.None).ThrowIfFailure();
+
+            return layeredFs;
+        }
+
+        private IFileSystem CreateEmptyFileSystem()
+        {
+            var baseLayerFs = new InMemoryFileSystem();
+            var topLayerFs = new InMemoryFileSystem();
+
+            return new LayeredFileSystem(baseLayerFs, topLayerFs);
+        }
+
+        [Fact]
+        public void OpenFile_FileDoesNotExist_ReturnsPathNotFound()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Result(ResultFs.PathNotFound, fs.OpenFile(out _, "/fakefile", OpenMode.All));
+        }
+
+        [Fact]
+        public void OpenFile_FileIsInBothSources_ReturnsFileFromTopSource()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Success(fs.OpenFile(out IFile file, "/dir/replacedFile", OpenMode.All));
+            Assert.Success(file.GetSize(out long fileSize));
+
+            Assert.Equal(2, fileSize);
+        }
+
+        [Fact]
+        public void OpenFile_InsideMergedDirectory_CanOpenFilesFromBothSources()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Success(fs.OpenFile(out _, "/dir2/lowerFile", OpenMode.All));
+            Assert.Success(fs.OpenFile(out _, "/dir2/upperFile", OpenMode.All));
+        }
+
+        [Fact]
+        public void OpenDirectory_DirDoesNotExist_ReturnsPathNotFound()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Result(ResultFs.PathNotFound, fs.OpenDirectory(out _, "/fakedir", OpenDirectoryMode.All));
+        }
+
+        [Fact]
+        public void OpenDirectory_ExistsInSingleLayer_ReturnsNonMergedDirectory()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Success(fs.OpenDirectory(out IDirectory dir, "/lowerDir", OpenDirectoryMode.All));
+            Assert.Equal(typeof(InMemoryFileSystem), dir.GetType().DeclaringType);
+        }
+
+        [Fact]
+        public void OpenDirectory_ExistsInMultipleLayers_ReturnsMergedDirectory()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Success(fs.OpenDirectory(out IDirectory dir, "/dir", OpenDirectoryMode.All));
+            Assert.Equal(typeof(LayeredFileSystem), dir.GetType().DeclaringType);
+        }
+
+        [Fact]
+        public void GetEntryType_InsideMergedDirectory_CanGetEntryTypesFromBothSources()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Success(fs.GetEntryType(out _, "/dir2/lowerFile"));
+            Assert.Success(fs.GetEntryType(out _, "/dir2/upperFile"));
+        }
+
+        [Fact]
+        public void IDirectoryRead_DuplicatedEntriesAreReturnedOnlyOnce()
+        {
+            IFileSystem fs = CreateFileSystem();
+            Span<DirectoryEntry> entries = stackalloc DirectoryEntry[4];
+
+            Assert.Success(fs.OpenDirectory(out IDirectory directory, "/dir3", OpenDirectoryMode.All));
+
+            Assert.Success(directory.Read(out long entriesRead, entries));
+            Assert.Equal(3, entriesRead);
+        }
+
+        [Fact]
+        public void IDirectoryRead_DuplicatedEntryReturnsFromTopLayer()
+        {
+            IFileSystem fs = CreateFileSystem();
+            var entry = new DirectoryEntry();
+
+            Assert.Success(fs.OpenDirectory(out IDirectory directory, "/dir", OpenDirectoryMode.All));
+
+            Assert.Success(directory.Read(out _, SpanHelpers.AsSpan(ref entry)));
+            Assert.Equal("replacedFile", StringUtils.Utf8ZToString(entry.Name));
+            Assert.Equal(2, entry.Size);
+        }
+
+        [Fact]
+        public void IDirectoryRead_EmptyFs_NoEntriesAreRead()
+        {
+            IFileSystem fs = CreateEmptyFileSystem();
+            var entry = new DirectoryEntry();
+
+            Assert.Success(fs.OpenDirectory(out IDirectory directory, "/", OpenDirectoryMode.All));
+
+            Assert.Success(directory.Read(out long entriesRead, SpanHelpers.AsSpan(ref entry)));
+            Assert.Equal(0, entriesRead);
+        }
+
+        [Fact]
+        public void IDirectoryGetEntryCount_DuplicatedEntriesAreCountedOnlyOnce()
+        {
+            IFileSystem fs = CreateFileSystem();
+
+            Assert.Success(fs.OpenDirectory(out IDirectory directory, "/dir3", OpenDirectoryMode.All));
+
+            Assert.Success(directory.GetEntryCount(out long entryCount));
+            Assert.Equal(3, entryCount);
+        }
+
+        [Fact]
+        public void IDirectoryGetEntryCount_MergedDirectoryAfterRead_AllEntriesAreCounted()
+        {
+            IFileSystem fs = CreateFileSystem();
+            var entry = new DirectoryEntry();
+
+            Assert.Success(fs.OpenDirectory(out IDirectory directory, "/dir3", OpenDirectoryMode.All));
+
+            // Read all entries
+            long entriesRead;
+            do
+            {
+                Assert.Success(directory.Read(out entriesRead, SpanHelpers.AsSpan(ref entry)));
+            } while (entriesRead != 0);
+
+            Assert.Success(directory.GetEntryCount(out long entryCount));
+            Assert.Equal(3, entryCount);
+        }
+
+        [Fact]
+        public void IDirectoryGetEntryCount_EmptyFs_EntryCountIsZero()
+        {
+            IFileSystem fs = CreateEmptyFileSystem();
+
+            Assert.Success(fs.OpenDirectory(out IDirectory directory, "/", OpenDirectoryMode.All));
+
+            Assert.Success(directory.GetEntryCount(out long entryCount));
+            Assert.Equal(0, entryCount);
+        }
+    }
+}