Begin cache restructure so it can be tested

src/cache.h

@@ -1,17 +0,0 @@
-#pragma once
-
-#include <optional.h>
-
-#include <memory>
-#include <string>
-
-struct Config;
-struct IndexFile;
-
-std::unique_ptr<IndexFile> LoadCachedIndex(Config* config,
-                                           const std::string& filename);
-
-optional<std::string> LoadCachedFileContents(Config* config,
-                                             const std::string& filename);
-
-void WriteToCache(Config* config, IndexFile& file);

src/cache_loader.cc

@@ -1,36 +0,0 @@
-#include "cache_loader.h"
-
-#include "cache.h"
-#include "indexer.h"
-
-CacheLoader::CacheLoader(Config* config) : config_(config) {}
-
-IndexFile* CacheLoader::TryLoad(const std::string& path) {
-  auto it = caches.find(path);
-  if (it != caches.end())
-    return it->second.get();
-
-  std::unique_ptr<IndexFile> cache = LoadCachedIndex(config_, path);
-  if (!cache)
-    return nullptr;
-
-  caches[path] = std::move(cache);
-  return caches[path].get();
-}
-
-std::unique_ptr<IndexFile> CacheLoader::TryTakeOrLoad(const std::string& path) {
-  auto it = caches.find(path);
-  if (it != caches.end()) {
-    auto result = std::move(it->second);
-    caches.erase(it);
-    return result;
-  }
-
-  return LoadCachedIndex(config_, path);
-}
-
-std::unique_ptr<IndexFile> CacheLoader::TakeOrLoad(const std::string& path) {
-  auto result = TryTakeOrLoad(path);
-  assert(result);
-  return result;
-}

src/cache_loader.h

@@ -1,23 +0,0 @@
-#pragma once
-
-#include "config.h"
-
-#include <unordered_map>
-
-// Manages loading caches from file paths for the indexer process.
-struct CacheLoader {
-  explicit CacheLoader(Config* config);
-
-  IndexFile* TryLoad(const std::string& path);
-
-  // Takes the existing cache or loads the cache at |path|. May return nullptr
-  // if the cache does not exist.
-  std::unique_ptr<IndexFile> TryTakeOrLoad(const std::string& path);
-
-  // Takes the existing cache or loads the cache at |path|. Asserts the cache
-  // exists.
-  std::unique_ptr<IndexFile> TakeOrLoad(const std::string& path);
-
-  std::unordered_map<std::string, std::unique_ptr<IndexFile>> caches;
-  Config* config_;
-};

src/cache_manager.cc

@@ -1,5 +1,6 @@
-#include "cache.h"
+#include "cache_manager.h"
 
+#include "config.h"
 #include "indexer.h"
 #include "language_server_api.h"
 #include "platform.h"
@@ -7,6 +8,78 @@
 #include <loguru/loguru.hpp>
 
 #include <algorithm>
+#include <unordered_map>
+
+namespace {
+
+// Manages loading caches from file paths for the indexer process.
+struct RealCacheManager : ICacheManager {
+  explicit RealCacheManager(Config* config) : config_(config) {}
+  ~RealCacheManager() override = default;
+
+  IndexFile* TryLoad(const std::string& path) override {
+    auto it = caches.find(path);
+    if (it != caches.end())
+      return it->second.get();
+
+    std::unique_ptr<IndexFile> cache = LoadCachedIndex(config_, path);
+    if (!cache)
+      return nullptr;
+
+    caches[path] = std::move(cache);
+    return caches[path].get();
+  }
+
+  std::unique_ptr<IndexFile> TryTakeOrLoad(const std::string& path) override {
+    auto it = caches.find(path);
+    if (it != caches.end()) {
+      auto result = std::move(it->second);
+      caches.erase(it);
+      return result;
+    }
+
+    return LoadCachedIndex(config_, path);
+  }
+
+  void IterateLoadedCaches(std::function<void(IndexFile*)> fn) override {
+    for (const auto& it : caches) {
+      assert(it.second);
+      fn(it.second.get());
+    }
+  }
+
+  std::unordered_map<std::string, std::unique_ptr<IndexFile>> caches;
+  Config* config_;
+};
+
+// struct FakeCacheManager : ICacheManager {
+//   explicit FakeCacheManager(const std::vector<FakeCacheEntry>& entries) {
+//     assert(false && "TODO");
+//   }
+// };
+
+}  // namespace
+
+// static
+std::unique_ptr<ICacheManager> ICacheManager::Make(Config* config) {
+  return MakeUnique<RealCacheManager>(config);
+}
+
+// static
+std::unique_ptr<ICacheManager> ICacheManager::MakeFake(
+    const std::vector<FakeCacheEntry>& entries) {
+  // return MakeUnique<FakeCacheManager>(entries);
+  assert(false && "TODO");
+  return nullptr;
+}
+
+ICacheManager::~ICacheManager() = default;
+
+std::unique_ptr<IndexFile> ICacheManager::TakeOrLoad(const std::string& path) {
+  auto result = TryTakeOrLoad(path);
+  assert(result);
+  return result;
+}
 
 namespace {
 

src/cache_manager.h

@@ -0,0 +1,46 @@
+#pragma once
+
+#include <optional.h>
+
+#include <functional>
+#include <memory>
+#include <string>
+
+struct Config;
+struct IndexFile;
+
+struct ICacheManager {
+  struct FakeCacheEntry {
+    std::string path;
+    std::string content;
+    std::string json;
+  };
+
+  static std::unique_ptr<ICacheManager> Make(Config* config);
+  static std::unique_ptr<ICacheManager> MakeFake(
+      const std::vector<FakeCacheEntry>& entries);
+
+  virtual ~ICacheManager();
+
+  // Tries to load a cache for |path|, returning null if there is none. The
+  // cache loader still owns the cache.
+  virtual IndexFile* TryLoad(const std::string& path) = 0;
+
+  // Takes the existing cache or loads the cache at |path|. May return null if
+  // the cache does not exist.
+  virtual std::unique_ptr<IndexFile> TryTakeOrLoad(const std::string& path) = 0;
+
+  // Takes the existing cache or loads the cache at |path|. Asserts the cache
+  // exists.
+  std::unique_ptr<IndexFile> TakeOrLoad(const std::string& path);
+
+  // Iterate over all loaded caches.
+  virtual void IterateLoadedCaches(std::function<void(IndexFile*)> fn) = 0;
+};
+
+// FIXME: only use ICacheLoader, not these functions.
+std::unique_ptr<IndexFile> LoadCachedIndex(Config* config,
+                                           const std::string& filename);
+optional<std::string> LoadCachedFileContents(Config* config,
+                                             const std::string& filename);
+void WriteToCache(Config* config, IndexFile& file);

src/command_line.cc

@@ -1,6 +1,5 @@
 // TODO: cleanup includes
-#include "cache.h"
+#include "cache_manager.h"
-#include "cache_loader.h"
 #include "clang_complete.h"
 #include "code_complete_cache.h"
 #include "file_consumer.h"

src/import_pipeline.cc

@@ -1,7 +1,6 @@
 #include "import_pipeline.h"
 
-#include "cache.h"
+#include "cache_manager.h"
-#include "cache_loader.h"
 #include "config.h"
 #include "import_manager.h"
 #include "language_server_api.h"
@@ -21,6 +20,7 @@
 #include <vector>
 
 namespace {
+
 // Send indexing progress to client if reporting is enabled.
 void EmitProgress(Config* config) {
   if (config->enableProgressReports) {
@@ -45,7 +45,7 @@ std::vector<Index_DoIdMap> DoParseFile(
     FileConsumerSharedState* file_consumer_shared,
     TimestampManager* timestamp_manager,
     ImportManager* import_manager,
-    CacheLoader* cache_loader,
+    ICacheManager* cache_manager,
     bool is_interactive,
     const std::string& path,
     const std::vector<std::string>& args,
@@ -54,7 +54,7 @@ std::vector<Index_DoIdMap> DoParseFile(
 
   // Always run this block, even if we are interactive, so we can check
   // dependencies and reset files in |file_consumer_shared|.
-  IndexFile* previous_index = cache_loader->TryLoad(path);
+  IndexFile* previous_index = cache_manager->TryLoad(path);
   if (previous_index) {
     // If none of the dependencies have changed and the index is not
     // interactive (ie, requested by a file save), skip parsing and just load
@@ -78,7 +78,7 @@ std::vector<Index_DoIdMap> DoParseFile(
         return FileParseQuery::NoSuchFile;
 
       optional<int64_t> last_cached_modification =
-          timestamp_manager->GetLastCachedModificationTime(cache_loader, path);
+          timestamp_manager->GetLastCachedModificationTime(cache_manager, path);
 
       // File has been changed.
       if (!last_cached_modification ||
@@ -126,7 +126,7 @@ std::vector<Index_DoIdMap> DoParseFile(
 
       // TODO/FIXME: real perf
       PerformanceImportFile perf;
-      result.push_back(Index_DoIdMap(cache_loader->TakeOrLoad(path), perf,
+      result.push_back(Index_DoIdMap(cache_manager->TakeOrLoad(path), perf,
                                      is_interactive, false /*write_to_disk*/));
       for (const std::string& dependency : previous_index->dependencies) {
         // Only load a dependency if it is not already loaded.
@@ -140,7 +140,7 @@ std::vector<Index_DoIdMap> DoParseFile(
                     << previous_index->path << ")";
 
         std::unique_ptr<IndexFile> dependency_index =
-            cache_loader->TryTakeOrLoad(dependency);
+            cache_manager->TryTakeOrLoad(dependency);
 
         // |dependency_index| may be null if there is no cache for it but
         // another file has already started importing it.
@@ -174,18 +174,18 @@ std::vector<Index_DoIdMap> DoParseFile(
     loaded_primary = loaded_primary || contents->path == path;
     file_contents.push_back(*contents);
   }
-  for (const auto& it : cache_loader->caches) {
+  cache_manager->IterateLoadedCaches([&](IndexFile* index) {
-    const std::unique_ptr<IndexFile>& index = it.second;
+    // FIXME: ReadContent should go through |cache_manager|.
-    assert(index);
     optional<std::string> index_content = ReadContent(index->path);
     if (!index_content) {
       LOG_S(ERROR) << "Failed to preload index content for " << index->path;
-      continue;
+      return;
     }
+
     file_contents.push_back(FileContents(index->path, *index_content));
 
     loaded_primary = loaded_primary || index->path == path;
-  }
+  });
   if (!loaded_primary) {
     optional<std::string> content = ReadContent(path);
     if (!content) {
@@ -231,15 +231,15 @@ std::vector<Index_DoIdMap> ParseFile(
   if (contents)
     file_contents = FileContents(entry.filename, *contents);
 
-  CacheLoader cache_loader(config);
+  std::unique_ptr<ICacheManager> cache_manager = ICacheManager::Make(config);
 
   // Try to determine the original import file by loading the file from cache.
   // This lets the user request an index on a header file, which clang will
   // complain about if indexed by itself.
-  IndexFile* entry_cache = cache_loader.TryLoad(entry.filename);
+  IndexFile* entry_cache = cache_manager->TryLoad(entry.filename);
   std::string tu_path = entry_cache ? entry_cache->import_file : entry.filename;
   return DoParseFile(config, working_files, index, file_consumer_shared,
-                     timestamp_manager, import_manager, &cache_loader,
+                     timestamp_manager, import_manager, cache_manager.get(),
                      is_interactive, tu_path, entry.args, file_contents);
 }
 
@@ -586,3 +586,32 @@ bool QueryDb_ImportMain(Config* config,
 
   return did_work;
 }
+
+#if false
+TEST_SUITE("ImportPipeline") {
+  TEST_CASE("hello") {
+    MultiQueueWaiter waiter;
+    QueueManager::CreateInstance(&waiter);
+    auto* queue = QueueManager::instance();
+
+    std::string path = "foo.cc";
+    std::vector<std::string> args = {};
+    bool is_interactive = false;
+    optional<std::string> contents = std::string("void foo();");
+    queue->index_request.Enqueue(
+        Index_Request(path, args, is_interactive, contents));
+
+    Config config;
+    WorkingFiles working_files;
+    FileConsumerSharedState file_consumer_shared;
+    TimestampManager timestamp_manager;
+    ImportManager import_manager;
+    ClangIndex index;
+    IndexMain_DoParse(&config, &working_files, &file_consumer_shared,
+                      &timestamp_manager, &import_manager, &index);
+
+    REQUIRE(queue->index_request.Size() == 0);
+    REQUIRE(queue->do_id_map.Size() == 1);
+  }
+}
+#endif

src/messages/cquery_freshen_index.cc

@@ -1,4 +1,4 @@
-#include "cache_loader.h"
+#include "cache_manager.h"
 #include "message_handler.h"
 #include "platform.h"
 #include "project.h"
@@ -25,7 +25,7 @@ struct CqueryFreshenIndexHandler : MessageHandler {
     // TODO: think about this flow and test it more.
 
     // Unmark all files whose timestamp has changed.
-    CacheLoader cache_loader(config);
+    std::unique_ptr<ICacheManager> cache_manager = ICacheManager::Make(config);
     for (const auto& file : db->files) {
       if (!file.def)
         continue;
@@ -36,7 +36,7 @@ struct CqueryFreshenIndexHandler : MessageHandler {
         continue;
 
       optional<int64_t> cached_modification =
-          timestamp_manager->GetLastCachedModificationTime(&cache_loader,
+          timestamp_manager->GetLastCachedModificationTime(cache_manager.get(),
                                                            file.def->path);
       if (modification_timestamp != cached_modification)
         file_consumer_shared->Reset(file.def->path);

src/messages/text_document_did_open.cc

@@ -1,4 +1,4 @@
-#include "cache.h"
+#include "cache_manager.h"
 #include "clang_complete.h"
 #include "include_complete.h"
 #include "message_handler.h"

src/timestamp_manager.cc

@@ -1,9 +1,10 @@
 #include "timestamp_manager.h"
 
+#include "cache_manager.h"
 #include "indexer.h"
 
 optional<int64_t> TimestampManager::GetLastCachedModificationTime(
-    CacheLoader* cache_loader,
+    ICacheManager* cache_manager,
     const std::string& path) {
   {
     std::lock_guard<std::mutex> guard(mutex_);
@@ -11,7 +12,7 @@ optional<int64_t> TimestampManager::GetLastCachedModificationTime(
     if (it != timestamps_.end())
       return it->second;
   }
-  IndexFile* file = cache_loader->TryLoad(path);
+  IndexFile* file = cache_manager->TryLoad(path);
   if (!file)
     return nullopt;
 

src/timestamp_manager.h

@@ -1,17 +1,17 @@
 #pragma once
 
-#include "cache_loader.h"
-
 #include <optional.h>
 
 #include <mutex>
 #include <unordered_map>
 
+struct ICacheManager;
+
 // Caches timestamps of cc files so we can avoid a filesystem reads. This is
 // important for import perf, as during dependency checking the same files are
 // checked over and over again if they are common headers.
 struct TimestampManager {
-  optional<int64_t> GetLastCachedModificationTime(CacheLoader* cache_loader,
+  optional<int64_t> GetLastCachedModificationTime(ICacheManager* cache_manager,
                                                   const std::string& path);
 
   void UpdateCachedModificationTime(const std::string& path, int64_t timestamp);