fix(filesystem): make the walker over a document or archive steerable

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
 

src/odr/internal/common/filesystem.cpp

@@ -7,9 +7,11 @@
 #include <odr/internal/util/file_util.hpp>
 #include <odr/internal/util/stream_util.hpp>
 
+#include <algorithm>
 #include <filesystem>
 #include <fstream>
 #include <map>
+#include <ranges>
 #include <system_error>
 
 namespace odr::internal {
@@ -149,9 +151,10 @@ class VirtualFileWalker final : public abstract::FileWalker {
 public:
   using Files = std::map<AbsPath, std::shared_ptr<abstract::File>>;
 
-  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::uint32_t>(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<AbsPath, std::shared_ptr<abstract::File>,
+               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<bool>(file_it->second);
   }
-  return !static_cast<bool>(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<abstract::FileWalker>
@@ -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<abstract::File>
 VirtualFilesystem::copy(std::shared_ptr<abstract::File> from,
                         const AbsPath &to) {
-  if (exists(to)) {
+  if (m_files.contains(to)) {
     return {};
   }
   m_files[to] = from;

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"

test/src/internal/common/filesystem_test.cpp

@@ -0,0 +1,160 @@
+#include <odr/internal/common/filesystem.hpp>
+
+#include <odr/internal/abstract/filesystem.hpp>
+#include <odr/internal/common/file.hpp>
+#include <odr/internal/common/path.hpp>
+
+#include <gtest/gtest.h>
+
+#include <memory>
+#include <string>
+#include <vector>
+
+using namespace odr::internal;
+
+namespace {
+
+/// The paths a walk over `paths` visits, in order.
+std::vector<std::string> walk(const abstract::ReadableFilesystem &filesystem,
+                              const AbsPath &root) {
+  std::vector<std::string> 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<std::string> &files,
+              const std::vector<std::string> &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<MemoryFile>(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<std::string>{"/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<MemoryFile>(std::string()),
+                  AbsPath("/a/b.txt"));
+  EXPECT_TRUE(filesystem.create_directory(AbsPath("/a")));
+
+  EXPECT_EQ(walk(filesystem, AbsPath("/")),
+            (std::vector<std::string>{"/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")));
+}