fix: unit tests and mock fixtures

include/silo/common/data_version.h

@@ -20,7 +20,7 @@ class DataVersion {
    static std::optional<DataVersion> fromString(const std::string& string);
 
   private:
-   DataVersion(std::string data_version);
+   explicit DataVersion(std::string data_version);
    std::string data_version;
 };
 

include/silo/database.h

@@ -10,6 +10,7 @@
 
 #include "silo/common/data_version.h"
 #include "silo/config/database_config.h"
+#include "silo/query_engine/query_result.h"
 #include "silo/storage/aa_store.h"
 #include "silo/storage/column/date_column.h"
 #include "silo/storage/column/float_column.h"
@@ -63,9 +64,11 @@ class Database {
    void setDataVersion(const DataVersion& data_version);
    virtual DataVersion getDataVersion() const;
 
+   virtual query_engine::QueryResult executeQuery(const std::string& query) const;
+
   private:
    PangoLineageAliasLookup alias_key;
-   DataVersion data_version_ = {""};
+   DataVersion data_version_ = DataVersion{""};
 
    void build(
       const preprocessing::PreprocessingConfig& preprocessing_config,

include/silo_api/database_mutex.h

@@ -12,9 +12,9 @@ class FixedDatabase {
    std::shared_lock<std::shared_mutex> lock;
 
   public:
-   FixedDatabase(silo::Database& database, std::shared_mutex& mutex);
+   FixedDatabase(const silo::Database& database, std::shared_lock<std::shared_mutex>&& mutex);
 
-   silo::Database& database;
+   const silo::Database& database;
 };
 
 class DatabaseMutex {
@@ -26,9 +26,9 @@ class DatabaseMutex {
 
    explicit DatabaseMutex(silo::Database&& database);
 
-   void setDatabase(silo::Database&& db);
+   void setDatabase(silo::Database&& new_database);
 
-   const FixedDatabase getDatabase();
+   virtual FixedDatabase getDatabase();
 };
 }  // namespace silo_api
 

src/silo/common/data_version.cpp

@@ -1,5 +1,6 @@
 #include "silo/common/data_version.h"
 
+#include <algorithm>
 #include <chrono>
 #include <utility>
 
@@ -8,7 +9,7 @@ namespace silo {
 DataVersion DataVersion::mineDataVersion() {
    const auto now = std::chrono::system_clock::now();
    const auto now_as_time_t = std::chrono::system_clock::to_time_t(now);
-   return DataVersion(std::to_string(now_as_time_t));
+   return DataVersion{std::to_string(now_as_time_t)};
 }
 
 DataVersion::DataVersion(std::string data_version)
@@ -24,7 +25,9 @@ bool DataVersion::operator<(const DataVersion& other) const {
 }
 
 std::optional<DataVersion> DataVersion::fromString(const std::string& string) {
-   if (std::all_of(string.begin(), string.end(), [](char c) { return c >= '0' && c <= '9'; })) {
+   if (std::all_of(string.begin(), string.end(), [](char character) {
+          return character >= '0' && character <= '9';
+       })) {
       return DataVersion{string};
    }
    return std::nullopt;

src/silo/common/data_version.test.cpp

@@ -11,11 +11,22 @@ TEST(DataVersion, shouldMineDataVersionFromUnixTime) {
 }
 
 TEST(DataVersion, shouldConstructFromVersionString) {
-   const auto version = DataVersion("1234567890");
-   EXPECT_EQ(version.toString(), "1234567890");
+   const auto version = DataVersion::fromString("1234567890");
+   EXPECT_TRUE(version.has_value());
+   if (version.has_value()) {
+      EXPECT_EQ(version->toString(), "1234567890");
+   }
+}
+
+TEST(DataVersion, shouldRejectFalseVersionFromString) {
+   const auto version = DataVersion::fromString("3X123");
+   EXPECT_FALSE(version.has_value());
 }
 
 TEST(DataVersion, shouldConstructWithDefaultVersion) {
-   const auto version = DataVersion();
-   EXPECT_EQ(version.toString(), "");
+   const auto version = DataVersion::fromString("");
+   EXPECT_TRUE(version.has_value());
+   if (version.has_value()) {
+      EXPECT_EQ(version->toString(), "");
+   }
 }

src/silo/database.cpp

@@ -40,6 +40,7 @@
 #include "silo/preprocessing/pango_lineage_count.h"
 #include "silo/preprocessing/partition.h"
 #include "silo/preprocessing/preprocessing_config.h"
+#include "silo/query_engine/query_engine.h"
 #include "silo/storage/aa_store.h"
 #include "silo/storage/column/date_column.h"
 #include "silo/storage/column/float_column.h"
@@ -758,4 +759,10 @@ DataVersion Database::getDataVersion() const {
    return data_version_;
 }
 
+query_engine::QueryResult Database::executeQuery(const std::string& query) const {
+   const silo::query_engine::QueryEngine query_engine(*this);
+
+   return query_engine.executeQuery(query);
+}
+
 }  // namespace silo

src/silo/database.test.cpp

@@ -102,18 +102,21 @@ TEST(DatabaseTest, shouldReturnCorrectDatabaseInfo) {
 TEST(DatabaseTest, shouldSaveAndReloadDatabaseWithoutErrors) {
    auto first_database = buildTestDatabase();
 
-   const std::string directory = "output/test_serialized_state/";
+   const std::filesystem::path directory = "output/test_serialized_state/";
    if (std::filesystem::exists(directory)) {
       std::filesystem::remove_all(directory);
    }
    std::filesystem::create_directories(directory);
 
+   silo::DataVersion data_version = first_database.getDataVersion();
+
    first_database.saveDatabaseState(directory);
 
-   auto database = silo::Database::loadDatabaseState(directory);
+   auto database = silo::Database::loadDatabaseState(directory / data_version.toString());
 
    const auto simple_database_info = database.getDatabaseInfo();
 
    EXPECT_GT(simple_database_info.total_size, 0);
    EXPECT_EQ(simple_database_info.sequence_count, 100);
+   EXPECT_GT(simple_database_info.n_bitmaps_size, 0);
 }

src/silo/storage/database_partition.cpp

@@ -21,6 +21,7 @@ class StringColumnPartition;
 DatabasePartition::DatabasePartition(std::vector<silo::preprocessing::Chunk> chunks)
     : chunks(std::move(chunks)) {}
 
+// NOLINTNEXTLINE(readability-function-cognitive-complexity)
 void DatabasePartition::flipBitmaps() {
    for (auto& [_, seq_store] : nuc_sequences) {
       tbb::enumerable_thread_specific<decltype(seq_store.indexing_differences_to_reference_genome)>
@@ -37,7 +38,7 @@ void DatabasePartition::flipBitmaps() {
          }
       });
       for (const auto& local : flipped_bitmaps) {
-         for (auto& element : local) {
+         for (const auto& element : local) {
             seq_store.indexing_differences_to_reference_genome.emplace_back(element);
          }
       }
@@ -57,7 +58,7 @@ void DatabasePartition::flipBitmaps() {
          }
       });
       for (const auto& local : flipped_bitmaps) {
-         for (auto& element : local) {
+         for (const auto& element : local) {
             aa_store.indexing_differences_to_reference_sequence.emplace_back(element);
          }
       }

src/silo_api/api.cpp

@@ -183,7 +183,7 @@ class SiloServer : public Poco::Util::ServerApplication {
 
       const Poco::Net::ServerSocket server_socket(port);
 
-      silo_api::DatabaseWatcher watcher(data_directory, database_mutex);
+      const silo_api::DatabaseWatcher watcher(data_directory, database_mutex);
 
       Poco::Net::HTTPServer server(
          new silo_api::SiloRequestHandlerFactory(database_mutex),

src/silo_api/database_mutex.cpp

@@ -1,17 +1,21 @@
 #include "silo_api/database_mutex.h"
 
-silo_api::FixedDatabase::FixedDatabase(silo::Database& database, std::shared_mutex& mutex)
-    : lock(mutex),
+silo_api::FixedDatabase::FixedDatabase(
+   const silo::Database& database,
+   std::shared_lock<std::shared_mutex>&& mutex
+)
+    : lock(std::move(mutex)),
       database(database) {}
 
 silo_api::DatabaseMutex::DatabaseMutex(silo::Database&& database)
     : database(std::move(database)) {}
 
-void silo_api::DatabaseMutex::setDatabase(silo::Database&& db) {
+void silo_api::DatabaseMutex::setDatabase(silo::Database&& new_database) {
    const std::unique_lock lock(mutex);
-   database = std::move(db);
+   database = std::move(new_database);
 }
 
-const silo_api::FixedDatabase silo_api::DatabaseMutex::getDatabase() {
-   return FixedDatabase(database, mutex);
+silo_api::FixedDatabase silo_api::DatabaseMutex::getDatabase() {
+   std::shared_lock<std::shared_mutex> lock(mutex);
+   return {database, std::move(lock)};
 }

src/silo_api/database_watcher.cpp

@@ -1,11 +1,15 @@
 #include "silo_api/database_watcher.h"
 
+#include <ranges>
+
+#include <boost/algorithm/string/join.hpp>
+
 namespace {
 
 std::optional<silo::Database> findInitialDatabase(const std::filesystem::path& path) {
    SPDLOG_INFO("Scanning path {} for valid data", path.string());
    std::vector<std::pair<std::filesystem::path, silo::DataVersion>> all_found_data;
-   for (auto& directory_entry : std::filesystem::directory_iterator{path}) {
+   for (const auto& directory_entry : std::filesystem::directory_iterator{path}) {
       SPDLOG_INFO("Checking directory entry {}", directory_entry.path().string());
       if (!directory_entry.is_directory()) {
          SPDLOG_INFO("Skipping {} because it is not a directory", directory_entry.path().string());
@@ -14,7 +18,7 @@ std::optional<silo::Database> findInitialDatabase(const std::filesystem::path& p
       auto data_version = silo::DataVersion::fromString(directory_entry.path().filename().string());
       if (data_version == std::nullopt) {
          SPDLOG_INFO(
-            "Skipping {}. Its name is not a valid data version.", path.filename().string()
+            "Skipping {}. Its name is not a valid data version.", directory_entry.path().string()
          );
          continue;
       }
@@ -23,21 +27,21 @@ std::optional<silo::Database> findInitialDatabase(const std::filesystem::path& p
          directory_entry.path().string(),
          data_version->toString()
       );
-      all_found_data.emplace_back(directory_entry, *data_version);
+      all_found_data.emplace_back(directory_entry.path(), std::move(*data_version));
    }
    if (all_found_data.empty()) {
       SPDLOG_INFO("No previous data found, place data in {} for ingestion", path.string());
       return std::nullopt;
    }
    if (all_found_data.size() == 1) {
-      std::filesystem::path data_path = all_found_data.at(0).first;
+      const std::filesystem::path data_path = all_found_data.at(0).first;
       SPDLOG_INFO("Using previous data: {}", data_path.string());
       return silo::Database::loadDatabaseState(data_path);
    }
-   auto max_element = std::max(
+   auto max_element = std::max_element(
       all_found_data.begin(),
       all_found_data.end(),
-      [](const auto& element1, const auto& element2) { return element1->second < element2->second; }
+      [](const auto& element1, const auto& element2) { return element1.second < element2.second; }
    );
    SPDLOG_INFO(
       "Selected database with highest data-version {} in path {} for ingestion",
@@ -59,37 +63,39 @@ silo_api::DatabaseWatcher::DatabaseWatcher(const std::string& path, DatabaseMute
    }
 }
 
-void silo_api::DatabaseWatcher::onItemAdded(const Poco::DirectoryWatcher::DirectoryEvent& ev) {
-   SPDLOG_TRACE("Item {} was added to the folder {}", watcher.directory().path(), ev.item.path());
-   if (!ev.item.isDirectory()) {
-      SPDLOG_INFO("Ignoring item added event: {}. It is not a folder", ev.item.path());
+void silo_api::DatabaseWatcher::onItemAdded(const Poco::DirectoryWatcher::DirectoryEvent& event) {
+   SPDLOG_TRACE(
+      "Item {} was added to the folder {}", watcher.directory().path(), event.item.path()
+   );
+   if (!event.item.isDirectory()) {
+      SPDLOG_INFO("Ignoring item added event: {}. It is not a folder", event.item.path());
       return;
    }
 
-   std::filesystem::path path = ev.item.path();
+   const std::filesystem::path path = event.item.path();
    auto new_data_version = silo::DataVersion::fromString(path.filename().string());
    if (new_data_version == std::nullopt) {
       SPDLOG_INFO(
          "Ignoring item added event: {}. Its name {} is not a valid data version.",
-         ev.item.path(),
+         event.item.path(),
          path.filename().string()
       );
       return;
    }
    {
-      auto fixed_database = database_mutex.getDatabase();
+      const auto fixed_database = database_mutex.getDatabase();
       if (!(fixed_database.database.getDataVersion() < new_data_version.value())) {
          SPDLOG_INFO(
             "Ignoring item added event: {}. Its version is not newer than the current version",
-            ev.item.path()
+            event.item.path()
          );
          return;
       }
    }
 
-   SPDLOG_INFO("New data version detected: {}", ev.item.path());
-   const std::string newVersionPath = ev.item.path();
-   auto database = silo::Database::loadDatabaseState(newVersionPath);
+   SPDLOG_INFO("New data version detected: {}", event.item.path());
+   const std::string new_version_path = event.item.path();
+   auto database = silo::Database::loadDatabaseState(new_version_path);
 
-   database_mutex.setDatabase(silo::Database::loadDatabaseState(newVersionPath));
+   database_mutex.setDatabase(silo::Database::loadDatabaseState(new_version_path));
 }

src/silo_api/info_handler.cpp

@@ -89,7 +89,7 @@ void InfoHandler::get(
 ) {
    const auto request_parameter = getQueryParameter(request);
 
-   auto& fixed_database = database.getDatabase();
+   const auto fixed_database = database.getDatabase();
 
    response.set("data-version", fixed_database.database.getDataVersion().toString());
 

src/silo_api/query_handler.cpp

@@ -34,9 +34,7 @@ void QueryHandler::post(
    try {
       auto fixed_database = database.getDatabase();
 
-      silo::query_engine::QueryEngine query_engine(fixed_database.database);
-
-      const auto query_result = query_engine.executeQuery(query);
+      const auto query_result = fixed_database.database.executeQuery(query);
 
       response.set("data-version", fixed_database.database.getDataVersion().toString());