Merge branch 'vfs_normalized_path_2' into 'master'

Use normalized path for file archives indices

See merge request OpenMW/openmw!3830

apps/openmw_test_suite/CMakeLists.txt

@@ -97,6 +97,8 @@ file(GLOB UNITTEST_SRC_FILES
     esmterrain/testgridsampling.cpp
 
     resource/testobjectcache.cpp
+
+    vfs/testpathutil.cpp
 )
 
 source_group(apps\\openmw_test_suite FILES openmw_test_suite.cpp ${UNITTEST_SRC_FILES})

apps/openmw_test_suite/testing_util.hpp

@@ -51,25 +51,21 @@ namespace TestingOpenMW
 
     struct VFSTestData : public VFS::Archive
     {
-        std::map<std::string, VFS::File*, VFS::Path::PathLess> mFiles;
+        VFS::FileMap mFiles;
 
-        VFSTestData(std::map<std::string, VFS::File*, VFS::Path::PathLess> files)
+        explicit VFSTestData(VFS::FileMap&& files)
             : mFiles(std::move(files))
         {
         }
 
-        void listResources(VFS::FileMap& out) override
-        {
-            for (const auto& [key, value] : mFiles)
-                out.emplace(key, value);
-        }
+        void listResources(VFS::FileMap& out) override { out = mFiles; }
 
-        bool contains(std::string_view file) const override { return mFiles.contains(file); }
+        bool contains(VFS::Path::NormalizedView file) const override { return mFiles.contains(file); }
 
         std::string getDescription() const override { return "TestData"; }
     };
 
-    inline std::unique_ptr<VFS::Manager> createTestVFS(std::map<std::string, VFS::File*, VFS::Path::PathLess> files)
+    inline std::unique_ptr<VFS::Manager> createTestVFS(VFS::FileMap&& files)
     {
         auto vfs = std::make_unique<VFS::Manager>();
         vfs->addArchive(std::make_unique<VFSTestData>(std::move(files)));

apps/openmw_test_suite/vfs/testpathutil.cpp

@@ -0,0 +1,139 @@
+#include <components/vfs/pathutil.hpp>
+
+#include <gtest/gtest.h>
+
+#include <sstream>
+
+namespace VFS::Path
+{
+    namespace
+    {
+        using namespace testing;
+
+        TEST(NormalizedTest, shouldSupportDefaultConstructor)
+        {
+            const Normalized value;
+            EXPECT_EQ(value.value(), "");
+        }
+
+        TEST(NormalizedTest, shouldSupportConstructorFromString)
+        {
+            const std::string string("Foo\\Bar/baz");
+            const Normalized value(string);
+            EXPECT_EQ(value.value(), "foo/bar/baz");
+        }
+
+        TEST(NormalizedTest, shouldSupportConstructorFromConstCharPtr)
+        {
+            const char* const ptr = "Foo\\Bar/baz";
+            const Normalized value(ptr);
+            EXPECT_EQ(value.value(), "foo/bar/baz");
+        }
+
+        TEST(NormalizedTest, shouldSupportConstructorFromStringView)
+        {
+            const std::string_view view = "Foo\\Bar/baz";
+            const Normalized value(view);
+            EXPECT_EQ(value.view(), "foo/bar/baz");
+        }
+
+        TEST(NormalizedTest, shouldSupportConstructorFromNormalizedView)
+        {
+            const NormalizedView view = "foo/bar/baz";
+            const Normalized value(view);
+            EXPECT_EQ(value.view(), "foo/bar/baz");
+        }
+
+        TEST(NormalizedTest, supportMovingValueOut)
+        {
+            Normalized value("Foo\\Bar/baz");
+            EXPECT_EQ(std::move(value).value(), "foo/bar/baz");
+            EXPECT_EQ(value.value(), "");
+        }
+
+        TEST(NormalizedTest, isNotEqualToNotNormalized)
+        {
+            const Normalized value("Foo\\Bar/baz");
+            EXPECT_NE(value.value(), "Foo\\Bar/baz");
+        }
+
+        TEST(NormalizedTest, shouldSupportOperatorLeftShiftToOStream)
+        {
+            const Normalized value("Foo\\Bar/baz");
+            std::stringstream stream;
+            stream << value;
+            EXPECT_EQ(stream.str(), "foo/bar/baz");
+        }
+
+        template <class T>
+        struct NormalizedOperatorsTest : Test
+        {
+        };
+
+        TYPED_TEST_SUITE_P(NormalizedOperatorsTest);
+
+        TYPED_TEST_P(NormalizedOperatorsTest, supportsEqual)
+        {
+            using Type0 = typename TypeParam::Type0;
+            using Type1 = typename TypeParam::Type1;
+            const Type0 normalized{ "a/foo/bar/baz" };
+            const Type1 otherEqual{ "a/foo/bar/baz" };
+            const Type1 otherNotEqual{ "b/foo/bar/baz" };
+            EXPECT_EQ(normalized, otherEqual);
+            EXPECT_EQ(otherEqual, normalized);
+            EXPECT_NE(normalized, otherNotEqual);
+            EXPECT_NE(otherNotEqual, normalized);
+        }
+
+        TYPED_TEST_P(NormalizedOperatorsTest, supportsLess)
+        {
+            using Type0 = typename TypeParam::Type0;
+            using Type1 = typename TypeParam::Type1;
+            const Type0 normalized{ "b/foo/bar/baz" };
+            const Type1 otherEqual{ "b/foo/bar/baz" };
+            const Type1 otherLess{ "a/foo/bar/baz" };
+            const Type1 otherGreater{ "c/foo/bar/baz" };
+            EXPECT_FALSE(normalized < otherEqual);
+            EXPECT_FALSE(otherEqual < normalized);
+            EXPECT_LT(otherLess, normalized);
+            EXPECT_FALSE(normalized < otherLess);
+            EXPECT_LT(normalized, otherGreater);
+            EXPECT_FALSE(otherGreater < normalized);
+        }
+
+        REGISTER_TYPED_TEST_SUITE_P(NormalizedOperatorsTest, supportsEqual, supportsLess);
+
+        template <class T0, class T1>
+        struct TypePair
+        {
+            using Type0 = T0;
+            using Type1 = T1;
+        };
+
+        using TypePairs = Types<TypePair<Normalized, Normalized>, TypePair<Normalized, const char*>,
+            TypePair<Normalized, std::string>, TypePair<Normalized, std::string_view>,
+            TypePair<Normalized, NormalizedView>, TypePair<NormalizedView, Normalized>,
+            TypePair<NormalizedView, const char*>, TypePair<NormalizedView, std::string>,
+            TypePair<NormalizedView, std::string_view>, TypePair<NormalizedView, NormalizedView>>;
+
+        INSTANTIATE_TYPED_TEST_SUITE_P(Typed, NormalizedOperatorsTest, TypePairs);
+
+        TEST(NormalizedViewTest, shouldSupportConstructorFromNormalized)
+        {
+            const Normalized value("Foo\\Bar/baz");
+            const NormalizedView view(value);
+            EXPECT_EQ(view.value(), "foo/bar/baz");
+        }
+
+        TEST(NormalizedViewTest, shouldSupportConstexprConstructorFromNormalizedStringLiteral)
+        {
+            constexpr NormalizedView view("foo/bar/baz");
+            EXPECT_EQ(view.value(), "foo/bar/baz");
+        }
+
+        TEST(NormalizedViewTest, constructorShouldThrowExceptionOnNotNormalized)
+        {
+            EXPECT_THROW([] { NormalizedView("Foo\\Bar/baz"); }(), std::invalid_argument);
+        }
+    }
+}

components/vfs/archive.hpp

@@ -2,9 +2,9 @@
 #define OPENMW_COMPONENTS_VFS_ARCHIVE_H
 
 #include <string>
-#include <string_view>
 
 #include "filemap.hpp"
+#include "pathutil.hpp"
 
 namespace VFS
 {
@@ -17,7 +17,7 @@ namespace VFS
         virtual void listResources(FileMap& out) = 0;
 
         /// True if this archive contains the provided normalized file.
-        virtual bool contains(std::string_view file) const = 0;
+        virtual bool contains(Path::NormalizedView file) const = 0;
 
         virtual std::string getDescription() const = 0;
     };

components/vfs/bsaarchive.hpp

@@ -52,19 +52,14 @@ namespace VFS
         void listResources(FileMap& out) override
         {
             for (auto& resource : mResources)
-            {
-                std::string ent = resource.mInfo->name();
-                Path::normalizeFilenameInPlace(ent);
-
-                out[ent] = &resource;
-            }
+                out[VFS::Path::Normalized(resource.mInfo->name())] = &resource;
         }
 
-        bool contains(std::string_view file) const override
+        bool contains(Path::NormalizedView file) const override
         {
             for (const auto& it : mResources)
             {
-                if (Path::pathEqual(file, it.mInfo->name()))
+                if (Path::pathEqual(file.value(), it.mInfo->name()))
                     return true;
             }
             return false;

components/vfs/filesystemarchive.cpp

@@ -37,9 +37,9 @@ namespace VFS
 
                 FileSystemArchiveFile file(path);
 
-                std::string searchable = Path::normalizeFilename(std::string_view{ proper }.substr(prefix));
+                VFS::Path::Normalized searchable(std::string_view{ proper }.substr(prefix));
 
-                const auto inserted = mIndex.emplace(searchable, file);
+                const auto inserted = mIndex.emplace(std::move(searchable), std::move(file));
                 if (!inserted.second)
                     Log(Debug::Warning)
                         << "Warning: found duplicate file for '" << proper
@@ -56,7 +56,7 @@ namespace VFS
         }
     }
 
-    bool FileSystemArchive::contains(std::string_view file) const
+    bool FileSystemArchive::contains(Path::NormalizedView file) const
     {
         return mIndex.find(file) != mIndex.end();
     }

components/vfs/filesystemarchive.hpp

@@ -30,12 +30,12 @@ namespace VFS
 
         void listResources(FileMap& out) override;
 
-        bool contains(std::string_view file) const override;
+        bool contains(Path::NormalizedView file) const override;
 
         std::string getDescription() const override;
 
     private:
-        std::map<std::string, FileSystemArchiveFile, std::less<>> mIndex;
+        std::map<VFS::Path::Normalized, FileSystemArchiveFile, std::less<>> mIndex;
         bool mBuiltIndex;
         std::filesystem::path mPath;
     };

components/vfs/pathutil.hpp

@@ -5,6 +5,7 @@
 
 #include <algorithm>
 #include <ostream>
+#include <stdexcept>
 #include <string>
 #include <string_view>
 
@@ -58,6 +59,59 @@ namespace VFS::Path
         bool operator()(std::string_view left, std::string_view right) const { return pathLess(left, right); }
     };
 
+    class Normalized;
+
+    class NormalizedView
+    {
+    public:
+        constexpr NormalizedView() noexcept = default;
+
+        constexpr NormalizedView(const char* value)
+            : mValue(value)
+        {
+            if (!isNormalized(mValue))
+                throw std::invalid_argument("NormalizedView value is not normalized: \"" + std::string(mValue) + "\"");
+        }
+
+        NormalizedView(const Normalized& value) noexcept;
+
+        constexpr std::string_view value() const noexcept { return mValue; }
+
+        friend constexpr bool operator==(const NormalizedView& lhs, const NormalizedView& rhs) = default;
+
+        friend constexpr bool operator==(const NormalizedView& lhs, const auto& rhs) { return lhs.mValue == rhs; }
+
+#if defined(_MSC_VER) && _MSC_VER <= 1935
+        friend constexpr bool operator==(const auto& lhs, const NormalizedView& rhs)
+        {
+            return lhs == rhs.mValue;
+        }
+#endif
+
+        friend constexpr bool operator<(const NormalizedView& lhs, const NormalizedView& rhs)
+        {
+            return lhs.mValue < rhs.mValue;
+        }
+
+        friend constexpr bool operator<(const NormalizedView& lhs, const auto& rhs)
+        {
+            return lhs.mValue < rhs;
+        }
+
+        friend constexpr bool operator<(const auto& lhs, const NormalizedView& rhs)
+        {
+            return lhs < rhs.mValue;
+        }
+
+        friend std::ostream& operator<<(std::ostream& stream, const NormalizedView& value)
+        {
+            return stream << value.mValue;
+        }
+
+    private:
+        std::string_view mValue;
+    };
+
     class Normalized
     {
     public:
@@ -84,6 +138,11 @@ namespace VFS::Path
             normalizeFilenameInPlace(mValue);
         }
 
+        explicit Normalized(NormalizedView value)
+            : mValue(value.value())
+        {
+        }
+
         const std::string& value() const& { return mValue; }
 
         std::string value() && { return std::move(mValue); }
@@ -96,26 +155,45 @@ namespace VFS::Path
 
         friend bool operator==(const Normalized& lhs, const Normalized& rhs) = default;
 
-        template <class T>
-        friend bool operator==(const Normalized& lhs, const T& rhs)
+        friend bool operator==(const Normalized& lhs, const auto& rhs) { return lhs.mValue == rhs; }
+
+#if defined(_MSC_VER) && _MSC_VER <= 1935
+        friend bool operator==(const auto& lhs, const Normalized& rhs)
         {
-            return lhs.mValue == rhs;
+            return lhs == rhs.mValue;
+        }
+#endif
+
+        friend bool operator==(const Normalized& lhs, const NormalizedView& rhs)
+        {
+            return lhs.mValue == rhs.value();
         }
 
-        friend bool operator<(const Normalized& lhs, const Normalized& rhs) { return lhs.mValue < rhs.mValue; }
+        friend bool operator<(const Normalized& lhs, const Normalized& rhs)
+        {
+            return lhs.mValue < rhs.mValue;
+        }
 
-        template <class T>
-        friend bool operator<(const Normalized& lhs, const T& rhs)
+        friend bool operator<(const Normalized& lhs, const auto& rhs)
         {
             return lhs.mValue < rhs;
         }
 
-        template <class T>
-        friend bool operator<(const T& lhs, const Normalized& rhs)
+        friend bool operator<(const auto& lhs, const Normalized& rhs)
         {
             return lhs < rhs.mValue;
         }
 
+        friend bool operator<(const Normalized& lhs, const NormalizedView& rhs)
+        {
+            return lhs.mValue < rhs.value();
+        }
+
+        friend bool operator<(const NormalizedView& lhs, const Normalized& rhs)
+        {
+            return lhs.value() < rhs.mValue;
+        }
+
         friend std::ostream& operator<<(std::ostream& stream, const Normalized& value)
         {
             return stream << value.mValue;
@@ -124,6 +202,11 @@ namespace VFS::Path
     private:
         std::string mValue;
     };
+
+    inline NormalizedView::NormalizedView(const Normalized& value) noexcept
+        : mValue(value.view())
+    {
+    }
 }
 
 #endif