refactoring mostly complete, some TODO items remain. And a final chec…

…k over all changed code should happen as well.

Now compiling, passing all tests and rebased on main

include/config/backend/yaml_file.h

@@ -14,10 +14,7 @@ class YamlFile : public ConfigBackend {
    std::string error_context;
    std::unordered_map<ConfigKeyPath, YAML::Node> yaml_fields;
 
-   YamlFile(
-      std::string error_context,
-      std::unordered_map<ConfigKeyPath, YAML::Node> yaml_fields
-   )
+   YamlFile(std::string error_context, std::unordered_map<ConfigKeyPath, YAML::Node> yaml_fields)
        : error_context(std::move(error_context)),
          yaml_fields(std::move(yaml_fields)) {}
 

include/config/config_interface.h

@@ -16,17 +16,23 @@
 
 namespace silo::config {
 
-/// For config structs (containing help and possibly config
-/// file paths):
+/// For config structs (containing help and possibly config file paths):
+/// We use a concept instead of virtual method overrides.
+/// This is because we want to call the virtual method overwriteFrom for the default values when
+/// constructing a Config. Instead, making the constructor private and instead creating a factory
+/// method would also be possible. Here, a concept works great, because the only usage of the
+/// interface uses a template anyways, due to the different return types (RuntimeConfig vs.
+/// PreprocessingConfig), whose easily accessible structure should remain.
 template <typename C>
 concept Config = requires(C c, const C cc, const VerifiedConfigSource& config_source) {
+   /// The specification which
+   { C::getConfigSpecification() } -> std::same_as<ConfigSpecification>;
 
    /// Whether the user gave the --help option or environment
    /// variable equivalent.
    /// bool asksForHelp() const = 0;
    { cc.asksForHelp() } -> std::same_as<bool>;
 
-
    /// Optional config file that the user gave (or that is provided
    /// by the type via its defaults) that should be loaded.
    /// std::optional<std::filesystem::path> configPath() const = 0;
@@ -51,12 +57,9 @@ concept Config = requires(C c, const C cc, const VerifiedConfigSource& config_so
 /// pass to exit(): 0 if the user gave --help, 1 in case of erroneous
 /// usage (the error is already printed in that case).
 template <Config C>
-std::variant<C, int32_t> getConfig(
-   std::span<const std::string> cmd
-) {
+std::variant<C, int32_t> getConfig(std::span<const std::string> cmd) {
    const auto config_specification = C::getConfigSpecification();
    try {
-
       auto env_source =
          EnvironmentVariables::decodeEnvironmentVariables().verify(config_specification);
       auto cmd_source = CommandLineArguments{cmd}.verify(config_specification);

include/config/config_key_path.h

@@ -16,10 +16,11 @@ namespace silo::config {
 /// This is easy to handle internally and also easy for transformation
 /// into CLI argument string and environment variable string
 class ConfigKeyPath {
-   ConfigKeyPath(std::vector<std::vector<std::string>> path) : path(path) {}
+   ConfigKeyPath(std::vector<std::vector<std::string>> path)
+       : path(path) {}
 
   public:
-   std::vector<std::vector<std::string>> path; // TODO make private and get()?
+   std::vector<std::vector<std::string>> path;  // TODO make private and get()?
 
    ConfigKeyPath() = default;
 

include/silo/common/fmt_formatters.h

@@ -1,12 +1,12 @@
 #pragma once
 
+#include <chrono>
 #include <filesystem>
 #include <optional>
-#include <chrono>
 
 #include <fmt/format.h>
-#include <nlohmann/json.hpp>
 #include <yaml-cpp/yaml.h>
+#include <nlohmann/json.hpp>
 
 #include "silo/common/panic.h"
 
@@ -27,7 +27,7 @@ struct [[maybe_unused]] fmt::formatter<std::unordered_map<T, U>> : fmt::formatte
       -> decltype(ctx.out()) {
       auto out = ctx.out();
       fmt::format_to(out, "{{\n");
-      for(const auto& [key, value] : val){
+      for (const auto& [key, value] : val) {
          fmt::format_to(out, "  {}: {},\n", key, value);
       }
       fmt::format_to(out, "}}");

include/silo/config/preprocessing_config.h

@@ -25,7 +25,7 @@ class PreprocessingConfig {
    std::filesystem::path output_directory = DEFAULT_OUTPUT_DIRECTORY;                        // TODO
    std::filesystem::path intermediate_results_directory = std::filesystem::path{"./temp/"};  // TODO
    std::optional<std::filesystem::path> preprocessing_database_location;
-   uint32_t duckdb_memory_limit_in_g = 10; // TODO remove
+   std::optional<uint32_t> duckdb_memory_limit_in_g = 10;  // TODO remove
    std::optional<std::filesystem::path> lineage_definitions_file;
    std::optional<std::filesystem::path> ndjson_input_filename;
    std::filesystem::path database_config_file = "database_config.yaml";  // TODO

include/silo/config/runtime_config.h

@@ -32,6 +32,8 @@ struct RuntimeConfig {
    ApiOptions api_options;
    QueryOptions query_options;
 
+   RuntimeConfig();
+
    static ConfigSpecification getConfigSpecification();
 
    void validate() const {};
@@ -41,7 +43,6 @@ struct RuntimeConfig {
    [[nodiscard]] std::optional<std::filesystem::path> configPath() const;
 
    void overwriteFrom(const VerifiedConfigSource& config_source);
-
 };
 
 }  // namespace silo::config

include/silo_api/api.h

@@ -6,5 +6,5 @@
 
 class SiloServer : public Poco::Util::ServerApplication {
   public:
-   int start(const silo::config::RuntimeConfig& runtime_config);
+   int runApi(const silo::config::RuntimeConfig& runtime_config);
 };

src/config/backend/yaml_file.cpp

@@ -1,8 +1,8 @@
 #include "config/backend/yaml_file.h"
 
-#include <unordered_set>
-#include <istream>
 #include <fstream>
+#include <istream>
+#include <unordered_set>
 
 #include <fmt/format.h>
 #include <boost/algorithm/string/join.hpp>
@@ -147,41 +147,32 @@ YamlFile YamlFile::fromYAML(const std::string& error_context, const std::string&
 
       // Collect all paths present
       std::unordered_map<ConfigKeyPath, YAML::Node> paths;
-      yamlToPaths(
-         error_context,
-         node,
-         ConsList<std::vector<std::string>>{},
-         paths
-      );
+      yamlToPaths(error_context, node, ConsList<std::vector<std::string>>{}, paths);
 
       return YamlFile{error_context, paths};
    } catch (const YAML::ParserException& parser_exception) {
-      throw std::runtime_error(fmt::format(
-         "{} does not contain valid YAML: {}", error_context, parser_exception.what())
+      throw std::runtime_error(
+         fmt::format("{} does not contain valid YAML: {}", error_context, parser_exception.what())
       );
    }
 }
 
 YamlFile YamlFile::readFile(const std::filesystem::path& path) {
    const std::ifstream file(path, std::ios::in | std::ios::binary);
    if (file.fail()) {
-      throw std::runtime_error(fmt::format(
-         "Could not open the YAML file: '{}'", path
-      ));
+      throw std::runtime_error(fmt::format("Could not open the YAML file: '{}'", path));
    }
 
    std::ostringstream contents;
    contents << file.rdbuf();
-   if(contents.fail()){
-      throw std::runtime_error(fmt::format(
-         "Error when reading the YAML file: '{}'", path
-      ));
+   if (contents.fail()) {
+      throw std::runtime_error(fmt::format("Error when reading the YAML file: '{}'", path));
    }
 
    return fromYAML(fmt::format("file: '{}'", path.string()), contents.str());
 }
 
-std::string YamlFile::errorContext() const { // TODO naming consistent?
+std::string YamlFile::errorContext() const {  // TODO naming consistent?
    return fmt::format("YAML file '{}'", error_context);
 }
 

src/config/command_line_arguments.test.cpp

@@ -8,13 +8,19 @@ using silo::config::ConfigKeyPath;
 TEST(CommandLineArguments, correctUnixOptionString) {
    ASSERT_EQ(CommandLineArguments::configKeyPathToString(ConfigKeyPath::from({{"a"}})), "--a");
    ASSERT_EQ(CommandLineArguments::configKeyPathToString(ConfigKeyPath::from({{"abc"}})), "--abc");
-   ASSERT_EQ(CommandLineArguments::configKeyPathToString(ConfigKeyPath::from({{"some", "camel", "case"}})), "--some-camel-case");
    ASSERT_EQ(
-      CommandLineArguments::configKeyPathToString(ConfigKeyPath::from({{"some"}, {"subsectioned", "sequence"}})),
+      CommandLineArguments::configKeyPathToString(ConfigKeyPath::from({{"some", "camel", "case"}})),
+      "--some-camel-case"
+   );
+   ASSERT_EQ(
+      CommandLineArguments::configKeyPathToString(
+         ConfigKeyPath::from({{"some"}, {"subsectioned", "sequence"}})
+      ),
       "--some-subsectioned-sequence"
    );
    ASSERT_EQ(
-      CommandLineArguments::configKeyPathToString(ConfigKeyPath::from({{"some", "more", "sections"}})),
+      CommandLineArguments::configKeyPathToString(ConfigKeyPath::from({{"some", "more", "sections"}}
+      )),
       "--some-more-sections"
    );
 }

src/config/config_key_path.cpp

@@ -6,21 +6,26 @@
 #include "config/backend/yaml_file.h"
 
 namespace {
-bool isLowerCaseOrNumeric(char c){
+bool isLowerCaseOrNumeric(char c) {
    return (std::islower(c) || std::isdigit(c));
 }
-}
+}  // namespace
 
 namespace silo::config {
 
 ConfigKeyPath ConfigKeyPath::from(std::vector<std::vector<std::string>> paths) {
-   for(const auto& sublevel : paths){
-      for(const std::string& string : sublevel){
-         if(string.empty()){
-            throw std::runtime_error("Internal Error: tried to create ConfigKeyPath with an empty part.");
+   for (const auto& sublevel : paths) {
+      for (const std::string& string : sublevel) {
+         if (string.empty()) {
+            throw std::runtime_error(
+               "Internal Error: tried to create ConfigKeyPath with an empty part."
+            );
          }
-         if(!std::ranges::all_of(string, isLowerCaseOrNumeric)){
-            throw std::runtime_error("Internal Error: tried to create ConfigKeyPath of a value that is not lower-case or numberic.");
+         if (!std::ranges::all_of(string, isLowerCaseOrNumeric)) {
+            throw std::runtime_error(
+               "Internal Error: tried to create ConfigKeyPath of a value that is not lower-case or "
+               "numberic."
+            );
          }
       }
    }

src/config/environment_variables.test.cpp

@@ -8,16 +8,23 @@ using silo::config::EnvironmentVariables;
 
 TEST(EnvironmentVariables, correctPrefixedUppercase) {
    ASSERT_EQ(EnvironmentVariables::configKeyPathToString(ConfigKeyPath::from({{"a"}})), "SILO_A");
-   ASSERT_EQ(EnvironmentVariables::configKeyPathToString(ConfigKeyPath::from({{"abc"}})), "SILO_ABC");
    ASSERT_EQ(
-      EnvironmentVariables::configKeyPathToString(ConfigKeyPath::from({{"some", "snake", "case"}})), "SILO_SOME_SNAKE_CASE"
+      EnvironmentVariables::configKeyPathToString(ConfigKeyPath::from({{"abc"}})), "SILO_ABC"
    );
    ASSERT_EQ(
-      EnvironmentVariables::configKeyPathToString(ConfigKeyPath::from({{"some"}, {"subsectioned", "sequence"}})),
+      EnvironmentVariables::configKeyPathToString(ConfigKeyPath::from({{"some", "snake", "case"}})),
+      "SILO_SOME_SNAKE_CASE"
+   );
+   ASSERT_EQ(
+      EnvironmentVariables::configKeyPathToString(
+         ConfigKeyPath::from({{"some"}, {"subsectioned", "sequence"}})
+      ),
       "SILO_SOME_SUBSECTIONED_SEQUENCE"
    );
    ASSERT_EQ(
-      EnvironmentVariables::configKeyPathToString(ConfigKeyPath::from({{"some"}, {"more"}, {"sections"}})),
+      EnvironmentVariables::configKeyPathToString(
+         ConfigKeyPath::from({{"some"}, {"more"}, {"sections"}})
+      ),
       "SILO_SOME_MORE_SECTIONS"
    );
 }

src/main.cpp

@@ -10,14 +10,11 @@
 #include "silo/config/util/config_repository.h"
 #include "silo/database.h"
 #include "silo/preprocessing/preprocessor.h"
-#include "silo_api/logging.h"
 #include "silo_api/api.h"
-
+#include "silo_api/logging.h"
 
 /// Does not throw exceptions
-static int runPreprocessor(
-   const silo::config::PreprocessingConfig& preprocessing_config
-) {
+static int runPreprocessor(const silo::config::PreprocessingConfig& preprocessing_config) {
    auto database_config = silo::config::ConfigRepository().getValidatedConfig(
       preprocessing_config.getDatabaseConfigFilename()
    );
@@ -29,12 +26,11 @@ static int runPreprocessor(
    silo::common::LineageTreeAndIdMap lineage_definitions;
    if (auto lineage_file_name = preprocessing_config.getLineageDefinitionsFilename()) {
       SPDLOG_INFO(
-         "preprocessing - read and verify the lineage tree '{}'",
-         lineage_file_name.value().string()
-      );
-      lineage_definitions = silo::common::LineageTreeAndIdMap::fromLineageDefinitionFilePath(
-         lineage_file_name.value()
+         "preprocessing - read and verify the lineage tree '{}'", lineage_file_name.value().string()
       );
+      lineage_definitions =
+         silo::common::LineageTreeAndIdMap::fromLineageDefinitionFilePath(lineage_file_name.value()
+         );
    }
 
    auto preprocessor = silo::preprocessing::Preprocessor(
@@ -46,6 +42,11 @@ static int runPreprocessor(
    return 0;
 }
 
+static int runApi(const silo::config::RuntimeConfig& runtime_config) {
+   SiloServer server;
+   return server.runApi(runtime_config);
+}
+
 enum class ExecutionMode { PREPROCESSING, API };
 
 int main(int argc, char** argv) {
@@ -100,7 +101,7 @@ int main(int argc, char** argv) {
             overloaded{
                [&](const silo::config::RuntimeConfig& runtime_config) {
                   SPDLOG_TRACE("runtime_config = {}", runtime_config);
-                  return SiloServer{}.start(runtime_config);
+                  return runApi(runtime_config);
                },
                [&](int32_t exit_code) { return exit_code; }
             },

src/silo/config/runtime_config.cpp

@@ -73,6 +73,10 @@ ConfigSpecification RuntimeConfig::getConfigSpecification() {
    };
 }
 
+RuntimeConfig::RuntimeConfig() {
+   overwriteFrom(getConfigSpecification());
+}
+
 bool RuntimeConfig::asksForHelp() const {
    return help;
 }