Refactor to allow for initial values in DatabaseState

include/mostly_harmless/data/mostlyharmless_DatabaseState.h

@@ -26,7 +26,6 @@ namespace mostly_harmless::data {
             DoubleIndex = 5
         };
 
-
         template <DatabaseStorageType T>
         auto databaseQueryCallback(void* ud, int count, char** data, char** /*columns*/) -> int {
             auto* result = static_cast<std::optional<T>*>(ud);
@@ -48,6 +47,10 @@ namespace mostly_harmless::data {
         }
     } // namespace
 
+    /**
+     * \brief A std::variant containing all types satisfying the DatabaseStorageType concept.
+     */
+    using DatabaseValueVariant = std::variant<std::string, bool, int, float, double>;
     /**
      * \brief Represents a connection to a sqlite database.
      *
@@ -68,22 +71,68 @@ namespace mostly_harmless::data {
         /**
          * @private
          */
-        DatabaseState(Private, const std::filesystem::path& location) {
+        DatabaseState(Private, const std::filesystem::path& location, const std::vector<std::pair<std::string, DatabaseValueVariant>>& initialValues) {
             const auto checkResult = [](int response) -> void {
                 if (response != SQLITE_OK) {
                     throw std::exception{};
                 }
             };
-            auto resultCode = sqlite3_open(location.string().c_str(), &m_databaseHandle);
-            checkResult(resultCode);
-            const std::string enableWalCommand{ "PRAGMA journal_mode=WAL" };
-            resultCode = sqlite3_exec(m_databaseHandle, enableWalCommand.c_str(), nullptr, nullptr, nullptr);
-            checkResult(resultCode);
-            const std::string command{
-                "CREATE TABLE IF NOT EXISTS DATA (NAME text UNIQUE, TEXT_VALUE text, BOOL_VALUE bool, INT_VALUE int, FLOAT_VALUE float, DOUBLE_VALUE double);"
-            };
-            resultCode = sqlite3_exec(m_databaseHandle, command.c_str(), nullptr, nullptr, nullptr);
-            checkResult(resultCode);
+
+            // Try open existing
+            if (sqlite3_open_v2(location.string().c_str(), &m_databaseHandle, SQLITE_OPEN_READWRITE, nullptr) != SQLITE_OK) {
+                checkResult(sqlite3_open(location.string().c_str(), &m_databaseHandle)); // Didn't exist, try create
+            }
+            // Enable WAL
+            checkResult(sqlite3_exec(m_databaseHandle, "PRAGMA journal_mode=WAL", nullptr, nullptr, nullptr));
+            // Create Table if not present
+            checkResult(sqlite3_exec(m_databaseHandle,
+                                     "CREATE TABLE IF NOT EXISTS DATA (NAME text UNIQUE, TEXT_VALUE text, BOOL_VALUE bool, INT_VALUE int, FLOAT_VALUE float, DOUBLE_VALUE double);",
+                                     nullptr,
+                                     nullptr,
+                                     nullptr));
+            // Populate with initial values, if key is present already, skip set
+            for (const auto& [key, value] : initialValues) {
+                std::visit([this, &key](auto&& arg) {
+                    using T = std::decay_t<decltype(arg)>;
+                    if (get<T>(key)) {
+                        return;
+                    }
+                    set(key, std::forward<decltype(arg)>(arg));
+                },
+                           value);
+            }
+        }
+
+        /**
+         * Non Copyable, as the database connection pointer will be closed on destruction...
+         */
+        DatabaseState(const DatabaseState& /*other*/) = delete;
+
+        /**
+         * Moveable, nulls `other`'s connection pointer
+         * @param other The moved-from DatabaseState instance
+         */
+        DatabaseState(DatabaseState&& other) noexcept {
+            std::swap(m_databaseHandle, other.m_databaseHandle);
+        }
+
+        /**
+         *
+         * Non Copyable, as the database connection pointer will be closed on destruction...
+         *
+         */
+        DatabaseState& operator=(const DatabaseState& /*other*/) = delete;
+
+        /**
+         * Moveable, nulls `other`'s connection pointer
+         * @param other The moved-from DatabaseState instance
+         * @return *this
+         */
+        DatabaseState& operator=(DatabaseState&& other) noexcept {
+            if (this != &other) {
+                std::swap(m_databaseHandle, other.m_databaseHandle);
+            }
+            return *this;
         }
 
         /**
@@ -92,20 +141,22 @@ namespace mostly_harmless::data {
          * If it doesn't exist, creates the database, and a table to store user data in.
          *
          * \param location A path to the database to create or open.
+         * \param initialValues A vector containing the initial values to add to the database if it didn't exist. If the database DID exist, but any of the keys in the vector aren't present, they'll be added with the values specified
+         * and existing items will be skipped.
          * \return A DatabaseState instance on success, nullopt otherwise.
          */
-        [[nodiscard]] static auto try_create(const std::filesystem::path& location) -> std::optional<DatabaseState> {
+        [[nodiscard]] static auto try_create(const std::filesystem::path& location, const std::vector<std::pair<std::string, DatabaseValueVariant>>& initialValues) -> std::optional<DatabaseState> {
             try {
-                DatabaseState state{ {}, location };
-                return state;
+                DatabaseState state{ {}, location, initialValues };
+                return std::move(state);
             } catch (...) {
                 assert(false);
                 return {};
             }
         }
 
         /**
-         * @private
+         * The internal database handle is closed if not null.
          */
         ~DatabaseState() noexcept {
             if (!m_databaseHandle) return;
@@ -119,8 +170,8 @@ namespace mostly_harmless::data {
          * @param toSet The value to set.
          */
         template <DatabaseStorageType T>
-        auto set(std::string_view name, T&& toSet) -> void {
-            struct Properties {
+        auto set(std::string_view name, const T& toSet) -> void {
+            struct {
                 std::string textValue{};
                 bool boolValue{ false };
                 int intValue{ 0 };

tests/CMakeLists.txt

@@ -4,4 +4,5 @@ set(MOSTLYHARMLESS_TEST_SOURCE
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_TaskThreadTests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/utils/mostlyharmless_TimerTests.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/events/mostlyharmless_InputEventContextTests.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/data/mostlyharmless_DatabaseStateTests.cpp
         PARENT_SCOPE)

tests/data/mostlyharmless_DatabaseStateTests.cpp

@@ -0,0 +1,60 @@
+//
+// Created by Syl Morrison on 12/04/2025.
+//
+#include <mostly_harmless/utils/mostlyharmless_Directories.h>
+#include <mostly_harmless/data/mostlyharmless_DatabaseState.h>
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/matchers/catch_matchers_floating_point.hpp>
+
+namespace mostly_harmless::testing {
+    template <bool ShouldSucceed>
+    auto tryCreateDatabase(const std::filesystem::path& destination, const std::vector<std::pair<std::string, data::DatabaseValueVariant>>& initialValues) {
+        auto databaseOpt = data::DatabaseState::try_create(destination, initialValues);
+        REQUIRE(databaseOpt.has_value() == ShouldSucceed);
+        return std::move(databaseOpt);
+    }
+
+    TEST_CASE("Test DatabaseState") {
+        auto tempDir = utils::directories::getDirectory(utils::directories::DirectoryType::Temp);
+        if (!tempDir) {
+            REQUIRE(false);
+        }
+        auto dbFile = *tempDir / "moha_test_db.sqlite";
+        SECTION("Test Valid Location, with no initial values") {
+            {
+                auto databaseOpt = tryCreateDatabase<true>(dbFile, {});
+                auto& database = *databaseOpt;
+                REQUIRE_NOTHROW(database.set<std::string>("Hello", "World"));
+                const auto retrieved = database.get<std::string>("Hello");
+                REQUIRE(retrieved.has_value());
+                REQUIRE(*retrieved == "World");
+                REQUIRE(!database.get<int>("aaaaa"));
+            }
+            {
+                std::vector<std::pair<std::string, data::DatabaseValueVariant>> initialValues;
+                initialValues.emplace_back("IntTest", 10);
+                initialValues.emplace_back("DoubleTest", 15.0);
+                auto databaseOpt = tryCreateDatabase<true>(dbFile, initialValues);
+                auto& database = *databaseOpt;
+                auto retrievedDouble = database.get<double>("DoubleTest");
+                REQUIRE(retrievedDouble.has_value());
+                REQUIRE_THAT(retrievedDouble.value(), Catch::Matchers::WithinRel(15.0));
+                database.set<double>("DoubleTest", 20.0);
+                retrievedDouble = database.get<double>("DoubleTest");
+                REQUIRE(retrievedDouble.has_value());
+                REQUIRE_THAT(retrievedDouble.value(), Catch::Matchers::WithinRel(20.0));
+                auto database2Opt = tryCreateDatabase<true>(dbFile, initialValues);
+                auto& database2 = *database2Opt;
+                retrievedDouble = database2.get<double>("DoubleTest");
+                REQUIRE(retrievedDouble.has_value());
+                REQUIRE_THAT(retrievedDouble.value(), Catch::Matchers::WithinRel(20.0));
+            }
+
+            std::filesystem::remove(dbFile);
+        }
+        SECTION("Test Invalid Location") {
+            tryCreateDatabase<false>("/iamthelordofthebongo", {});
+        }
+    }
+
+} // namespace mostly_harmless::testing