diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f71ef07..8224c2dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,10 @@ The release run heads these entries with the version and opens a fresh - A jni build without a JDK fails instead of shipping a package missing `odr-core-java.jar`. `ODR_JNI_JAR=OFF` is how the AAR build asks for the native half alone. +- A `FileWalker` over a document or an archive can be steered: `flat_next()`, + `pop()` and `depth()` do what they say instead of nothing. +- A directory a document or an archive only implies is one: `exists()` and + `is_directory()` answer for `/` and for a path that files sit under. ## v6.9.0 - 2026-08-18 diff --git a/src/odr/internal/common/filesystem.cpp b/src/odr/internal/common/filesystem.cpp index bb64dc8e..f28ef3b0 100644 --- a/src/odr/internal/common/filesystem.cpp +++ b/src/odr/internal/common/filesystem.cpp @@ -7,9 +7,11 @@ #include #include +#include #include #include #include +#include #include namespace odr::internal { @@ -149,9 +151,10 @@ class VirtualFileWalker final : public abstract::FileWalker { public: using Files = std::map>; - VirtualFileWalker(const AbsPath &root, const Files &files) { + VirtualFileWalker(AbsPath root, const Files &files) + : m_root{std::move(root)} { for (const auto &[path, file] : files) { - if (path.descendant_of(root)) { + if (path.descendant_of(m_root)) { m_files[path] = file; } } @@ -160,7 +163,8 @@ class VirtualFileWalker final : public abstract::FileWalker { } /// The iterator has to be re-seated into the copied map. - VirtualFileWalker(const VirtualFileWalker &other) : m_files{other.m_files} { + VirtualFileWalker(const VirtualFileWalker &other) + : m_root{other.m_root}, m_files{other.m_files} { m_iterator = other.m_iterator == std::end(other.m_files) ? std::end(m_files) : m_files.find(other.m_iterator->first); @@ -186,8 +190,10 @@ class VirtualFileWalker final : public abstract::FileWalker { return m_iterator == std::end(m_files); } + /// 0 directly in the walked root, as `recursive_directory_iterator`. [[nodiscard]] std::uint32_t depth() const override { - return 0; // TODO + const RelPath relative = m_iterator->first.rebase(m_root); + return static_cast(std::ranges::distance(relative)) - 1; } [[nodiscard]] AbsPath path() const override { return m_iterator->first; } @@ -198,24 +204,36 @@ class VirtualFileWalker final : public abstract::FileWalker { [[nodiscard]] bool is_directory() const override { return !is_file(); } - void pop() override { - // TODO - } + /// At depth 0 this ends the walk - the parent is the walked root. + void pop() override { skip_subtree_(m_iterator->first.parent()); } void next() override { ++m_iterator; } - void flat_next() override { - // TODO - } + /// The next entry that is not under the current one. + void flat_next() override { skip_subtree_(m_iterator->first); } private: - Files m_files; - Files::iterator m_iterator; + /// Ordered component-wise, so that a subtree is contiguous and can be + /// skipped by walking it. By string it is not: "/a" < "/a-b" < "/a/b". + using DepthFirstFiles = + std::map, + decltype(std::ranges::lexicographical_compare)>; + + AbsPath m_root; + DepthFirstFiles m_files; + DepthFirstFiles::iterator m_iterator; + + void skip_subtree_(const AbsPath &path) { + do { + ++m_iterator; + } while (m_iterator != std::end(m_files) && + m_iterator->first.descendant_of(path)); + } }; } // namespace bool VirtualFilesystem::exists(const AbsPath &path) const { - return m_files.contains(path); + return is_file(path) || is_directory(path); } bool VirtualFilesystem::is_file(const AbsPath &path) const { @@ -227,11 +245,18 @@ bool VirtualFilesystem::is_file(const AbsPath &path) const { } bool VirtualFilesystem::is_directory(const AbsPath &path) const { + if (path.root()) { + return true; + } const auto file_it = m_files.find(path); - if (file_it == std::end(m_files)) { - return false; + if (file_it != std::end(m_files)) { + return !static_cast(file_it->second); } - return !static_cast(file_it->second); + // An intermediate directory need not be an entry of its own - a zip may name + // only its files. + return std::ranges::any_of(m_files, [&path](const auto &entry) { + return entry.first.descendant_of(path); + }); } std::unique_ptr @@ -254,7 +279,9 @@ VirtualFilesystem::create_file(const AbsPath & /*path*/) { } bool VirtualFilesystem::create_directory(const AbsPath &path) { - if (exists(path)) { + // Not `exists()`: a merely implied directory still has to become an entry, + // or an archive naming `a/b.txt` before `a/` would lose `a/` from the walk. + if (m_files.contains(path)) { return false; } m_files[path] = nullptr; @@ -277,7 +304,7 @@ bool VirtualFilesystem::copy(const AbsPath &from, const AbsPath &to) { if (from_it == std::end(m_files)) { return false; } - if (exists(to)) { + if (m_files.contains(to)) { return false; } m_files[to] = from_it->second; @@ -293,7 +320,7 @@ VirtualFilesystem::copy(const abstract::File & /*from*/, std::shared_ptr VirtualFilesystem::copy(std::shared_ptr from, const AbsPath &to) { - if (exists(to)) { + if (m_files.contains(to)) { return {}; } m_files[to] = from; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ab6b6630..09157ede 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -46,6 +46,7 @@ add_executable(odr_test "src/internal/cfb/cfb_archive_test.cpp" + "src/internal/common/filesystem_test.cpp" "src/internal/common/list_numbering_test.cpp" "src/internal/common/path_test.cpp" "src/internal/common/table_cursor_test.cpp" diff --git a/test/src/internal/common/filesystem_test.cpp b/test/src/internal/common/filesystem_test.cpp new file mode 100644 index 00000000..9e910f99 --- /dev/null +++ b/test/src/internal/common/filesystem_test.cpp @@ -0,0 +1,160 @@ +#include + +#include +#include +#include + +#include + +#include +#include +#include + +using namespace odr::internal; + +namespace { + +/// The paths a walk over `paths` visits, in order. +std::vector walk(const abstract::ReadableFilesystem &filesystem, + const AbsPath &root) { + std::vector result; + for (const auto walker = filesystem.file_walker(root); !walker->end(); + walker->next()) { + result.push_back(walker->path().string()); + } + return result; +} + +VirtualFilesystem +filesystem_of(const std::vector &files, + const std::vector &directories = {}) { + VirtualFilesystem result; + for (const std::string &directory : directories) { + result.create_directory(AbsPath(directory)); + } + for (const std::string &file : files) { + result.copy(std::make_shared(std::string()), AbsPath(file)); + } + return result; +} + +} // namespace + +TEST(VirtualFilesystem, walker_reports_the_depth_of_every_entry) { + const VirtualFilesystem filesystem = + filesystem_of({"/mimetype", "/a/b.txt", "/a/c/d.txt"}); + + const auto walker = filesystem.file_walker(AbsPath("/")); + + EXPECT_EQ(walker->path().string(), "/a/b.txt"); + EXPECT_EQ(walker->depth(), 1); + walker->next(); + EXPECT_EQ(walker->path().string(), "/a/c/d.txt"); + EXPECT_EQ(walker->depth(), 2); + walker->next(); + EXPECT_EQ(walker->path().string(), "/mimetype"); + EXPECT_EQ(walker->depth(), 0); +} + +TEST(VirtualFilesystem, walker_depth_is_relative_to_the_walked_root) { + const VirtualFilesystem filesystem = filesystem_of({"/a/c/d.txt"}); + + const auto walker = filesystem.file_walker(AbsPath("/a")); + + EXPECT_EQ(walker->depth(), 1); +} + +TEST(VirtualFilesystem, flat_next_skips_the_subtree_and_terminates) { + const VirtualFilesystem filesystem = + filesystem_of({"/a/b.txt", "/a/c/d.txt", "/e.txt"}); + + const auto walker = filesystem.file_walker(AbsPath("/")); + + EXPECT_EQ(walker->path().string(), "/a/b.txt"); + walker->flat_next(); + EXPECT_EQ(walker->path().string(), "/a/c/d.txt"); + walker->flat_next(); + EXPECT_EQ(walker->path().string(), "/e.txt"); + walker->flat_next(); + EXPECT_TRUE(walker->end()); +} + +/// The loop flat_next() exists for; it never terminated before. +TEST(VirtualFilesystem, flat_next_over_a_directory_skips_what_is_under_it) { + const VirtualFilesystem filesystem = + filesystem_of({"/a/b.txt", "/a/c.txt", "/e.txt"}, {"/a"}); + + const auto walker = filesystem.file_walker(AbsPath("/")); + + EXPECT_EQ(walker->path().string(), "/a"); + EXPECT_TRUE(walker->is_directory()); + walker->flat_next(); + EXPECT_EQ(walker->path().string(), "/e.txt"); +} + +/// "/a-b" sorts between "/a" and "/a/b" by string. +TEST(VirtualFilesystem, flat_next_skips_a_subtree_a_sibling_sorts_into) { + const VirtualFilesystem filesystem = + filesystem_of({"/a/b.txt", "/a-b.txt", "/e.txt"}, {"/a"}); + + EXPECT_EQ(walk(filesystem, AbsPath("/")), + (std::vector{"/a", "/a/b.txt", "/a-b.txt", "/e.txt"})); + + const auto walker = filesystem.file_walker(AbsPath("/")); + walker->flat_next(); + EXPECT_EQ(walker->path().string(), "/a-b.txt"); +} + +TEST(VirtualFilesystem, pop_leaves_the_directory) { + const VirtualFilesystem filesystem = + filesystem_of({"/a/b.txt", "/a/c.txt", "/e.txt"}); + + const auto walker = filesystem.file_walker(AbsPath("/")); + + EXPECT_EQ(walker->path().string(), "/a/b.txt"); + walker->pop(); + EXPECT_EQ(walker->path().string(), "/e.txt"); +} + +TEST(VirtualFilesystem, pop_at_depth_zero_ends_the_walk) { + const VirtualFilesystem filesystem = filesystem_of({"/a.txt", "/b.txt"}); + + const auto walker = filesystem.file_walker(AbsPath("/")); + + EXPECT_EQ(walker->depth(), 0); + walker->pop(); + EXPECT_TRUE(walker->end()); +} + +/// An archive names its entries in its own order. A directory arriving after +/// what it holds is still an entry of its own, or `flat_next()` never sees it. +TEST(VirtualFilesystem, + a_directory_entry_survives_arriving_after_its_children) { + VirtualFilesystem filesystem; + filesystem.copy(std::make_shared(std::string()), + AbsPath("/a/b.txt")); + EXPECT_TRUE(filesystem.create_directory(AbsPath("/a"))); + + EXPECT_EQ(walk(filesystem, AbsPath("/")), + (std::vector{"/a", "/a/b.txt"})); + + const auto walker = filesystem.file_walker(AbsPath("/")); + EXPECT_TRUE(walker->is_directory()); + walker->flat_next(); + EXPECT_TRUE(walker->end()); +} + +TEST(VirtualFilesystem, an_intermediate_directory_is_a_directory) { + const VirtualFilesystem filesystem = filesystem_of({"/a/c/d.txt"}); + + EXPECT_TRUE(filesystem.exists(AbsPath("/"))); + EXPECT_TRUE(filesystem.is_directory(AbsPath("/"))); + EXPECT_TRUE(filesystem.exists(AbsPath("/a"))); + EXPECT_TRUE(filesystem.is_directory(AbsPath("/a"))); + EXPECT_TRUE(filesystem.exists(AbsPath("/a/c"))); + EXPECT_TRUE(filesystem.is_directory(AbsPath("/a/c"))); + + EXPECT_FALSE(filesystem.is_file(AbsPath("/a"))); + EXPECT_FALSE(filesystem.exists(AbsPath("/b"))); + EXPECT_FALSE(filesystem.is_directory(AbsPath("/a/c/d.txt"))); +}