Check only directories name during cache init (#1484)

Instead of checking whole path check only top-level directories
names and produce error if there are some unexpected one.

Nested directories in the `lost` directory are allowed for now.

Relates-To: OLPEDGE-2868

Signed-off-by: Andrey Kashcheev <[email protected]>

Author: andrey-kashcheev

olp-cpp-sdk-core/include/olp/core/utils/Dir.h

@@ -148,11 +148,11 @@ class CORE_API Dir {
   static bool IsReadOnly(const std::string& path);
 
   /**
-   * @brief Iterates through all directories in the provided path and calls the
-   * provided callback function for each directory.
+   * @brief Iterates through top-level directories in the provided path and
+   * calls the provided callback function for each directory.
    *
    * @param path The path of the root directory.
-   * @param path_fn The callback function.
+   * @param path_fn The callback function with a relative path.
    */
   static void ForEachDirectory(const std::string& path, PathCallback path_fn);
 };

olp-cpp-sdk-core/src/cache/DiskCache.cpp

@@ -42,7 +42,7 @@ namespace cache {
 
 namespace {
 constexpr auto kLogTag = "DiskCache";
-constexpr auto kLevelDbLostFolder = "/lost";
+constexpr auto kLevelDbLostFolder = "lost";
 constexpr auto kMaxL0Files = 4;
 
 leveldb::Slice ToLeveldbSlice(const std::string& slice) {
@@ -54,11 +54,11 @@ static bool RepairCache(const std::string& data_path) {
   auto status = leveldb::RepairDB(data_path, leveldb::Options());
   if (status.ok()) {
     OLP_SDK_LOG_INFO(kLogTag, "RepairCache: repaired - " << data_path);
-    const auto lost_folder_path = data_path + kLevelDbLostFolder;
+    const auto lost_folder_path = data_path + '/' + kLevelDbLostFolder;
     if (utils::Dir::Exists(lost_folder_path)) {
       OLP_SDK_LOG_INFO_F(
           kLogTag, "RepairCache: some data may have been lost - deleting '%s'",
-          kLevelDbLostFolder);
+          lost_folder_path.c_str());
       utils::Dir::Remove(lost_folder_path);
     }
     return true;
@@ -205,14 +205,14 @@ OpenResult DiskCache::Open(const std::string& data_path,
   }
 
   // Check cache path for unexpected directories
-  const std::vector<std::string> expected_dirs = {disk_cache_path_ +
-                                                  kLevelDbLostFolder};
+  const std::vector<std::string> expected_dirs = {kLevelDbLostFolder};
   bool unexpected_dirs = false;
   utils::Dir::ForEachDirectory(disk_cache_path_, [&](const std::string& dir) {
     if (std::find(expected_dirs.begin(), expected_dirs.end(), dir) ==
         expected_dirs.end()) {
-      OLP_SDK_LOG_WARNING_F(
-          kLogTag, "Open: unexpected directory found, path='%s'", dir.c_str());
+      OLP_SDK_LOG_WARNING_F(kLogTag,
+                            "Open: unexpected directory found, path='%s/%s'",
+                            disk_cache_path_.c_str(), dir.c_str());
       unexpected_dirs = true;
     }
   });

olp-cpp-sdk-core/src/utils/Dir.cpp

@@ -546,9 +546,7 @@ void Dir::ForEachDirectory(const std::string& path, PathCallback path_fn) {
       }
 
       if (find_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
-        current_path = path + "\\" + find_data.cFileName;
-        path_fn(current_path);
-        ForEachDirectory(current_path, path_fn);
+        path_fn(find_data.cFileName);
       }
     } while (FindNextFileA(handle, &find_data) != 0);
 
@@ -557,50 +555,39 @@ void Dir::ForEachDirectory(const std::string& path, PathCallback path_fn) {
 
 #else
 
-  std::queue<std::string> recursive_queue;
-  recursive_queue.push(path);
-
-  auto iterate_directory = [&](const std::string& path) {
-    auto* dir = ::opendir(path.c_str());
-    if (dir == nullptr) {
-      return;
-    }
+  auto* dir = ::opendir(path.c_str());
+  if (dir == nullptr) {
+    return;
+  }
 
-    struct ::dirent* entry = nullptr;
-    while ((entry = ::readdir(dir)) != nullptr) {
-      const char* entry_name = entry->d_name;
+  struct ::dirent* entry = nullptr;
+  while ((entry = ::readdir(dir)) != nullptr) {
+    const char* entry_name = entry->d_name;
 
-      if (strcmp(entry_name, ".") == 0 || strcmp(entry_name, "..") == 0) {
-        continue;
-      }
+    if (strcmp(entry_name, ".") == 0 || strcmp(entry_name, "..") == 0) {
+      continue;
+    }
 
-      std::string full_path = path + "/" + entry_name;
+    std::string full_path = path + "/" + entry_name;
 
 #ifdef __APPLE__
-      struct ::stat path_stat;
-      if (::lstat(full_path.c_str(), &path_stat) == 0) {
+    struct ::stat path_stat;
+    if (::lstat(full_path.c_str(), &path_stat) == 0) {
 #else
-      struct ::stat64 path_stat;
-      if (::lstat64(full_path.c_str(), &path_stat) == 0) {
+    struct ::stat64 path_stat;
+    if (::lstat64(full_path.c_str(), &path_stat) == 0) {
 #endif
-        if (S_ISDIR(path_stat.st_mode)) {
-          path_fn(full_path);
-          recursive_queue.push(std::move(full_path));
-        }
-      } else if (errno != ENOENT) {
-        // Ignore ENOENT errors as its a common case, e.g. cache compaction.
-        break;
+      if (S_ISDIR(path_stat.st_mode)) {
+        path_fn(entry_name);
       }
+    } else if (errno != ENOENT) {
+      // Ignore ENOENT errors as its a common case, e.g. cache compaction.
+      break;
     }
-
-    ::closedir(dir);
-  };
-
-  while (!recursive_queue.empty()) {
-    iterate_directory(recursive_queue.front());
-    recursive_queue.pop();
   }
 
+  ::closedir(dir);
+
 #endif
 }
 

olp-cpp-sdk-core/tests/cache/DefaultCacheTest.cpp

@@ -686,20 +686,19 @@ TEST(DefaultCacheTest, OpenTypeCache) {
     ASSERT_FALSE(cache.Contains(key2));
   }
 
-  {
-    SCOPED_TRACE("Additional directories");
-
+  auto additional_dirs_test = [](const std::string& mutable_path,
+                                 const std::string& protected_path) {
     constexpr auto kExpectedDirResult =
         olp::cache::DefaultCache::StorageOpenResult::Success;
     constexpr auto kUnexpectedDirResult =
         olp::cache::DefaultCache::StorageOpenResult::OpenDiskPathFailure;
 
     auto scenarios = {
+        std::make_tuple("/lost/tmp", kExpectedDirResult),
         std::make_tuple("/lost", kExpectedDirResult),
-        std::make_tuple("/lost/tmp", kUnexpectedDirResult),
         std::make_tuple("/found", kUnexpectedDirResult),
-        std::make_tuple("/ARCHIVES", kUnexpectedDirResult),
-        std::make_tuple("/ARCHIVES/removed", kUnexpectedDirResult)};
+        std::make_tuple("/ARCHIVES/removed", kUnexpectedDirResult),
+        std::make_tuple("/ARCHIVES", kUnexpectedDirResult)};
 
     for (auto& scenario : scenarios) {
       {
@@ -732,6 +731,21 @@ TEST(DefaultCacheTest, OpenTypeCache) {
         olp::utils::Dir::Remove(dir_path);
       }
     }
+  };
+
+  {
+    SCOPED_TRACE("Additional directories");
+
+    additional_dirs_test(mutable_path, protected_path);
+  }
+
+  {
+    SCOPED_TRACE("Additional directories, relative paths");
+
+    std::string mutable_relative_path = mutable_path + "/../mutable_cache";
+    std::string protected_relative_path =
+        protected_path + "/../protected_cache";
+    additional_dirs_test(mutable_relative_path, protected_relative_path);
   }
 }