changeset 172:6250883b81f0

core: improve save module, closes #2507 - Databases can now be opened in read-only or in read-write modes, - Module is fully reentrant, - SQL commands are in separate files, - Improve tests, - API to get/set property is using a dedicated structure.
author David Demelier <markand@malikania.fr>
date Wed, 21 Oct 2020 12:43:25 +0200
parents d841cff1017c
children cd69c8791dac
files libcore/CMakeLists.txt libcore/core/save.c libcore/core/save.h tests/test-save.c
diffstat 4 files changed, 330 insertions(+), 142 deletions(-) [+]
line wrap: on
line diff
--- a/libcore/CMakeLists.txt	Wed Oct 21 11:47:18 2020 +0200
+++ b/libcore/CMakeLists.txt	Wed Oct 21 12:43:25 2020 +0200
@@ -19,6 +19,14 @@
 project(libcore)
 
 set(
+	SQL_ASSETS
+	${libcore_SOURCE_DIR}/core/assets/sql/init.sql
+	${libcore_SOURCE_DIR}/core/assets/sql/property-get.sql
+	${libcore_SOURCE_DIR}/core/assets/sql/property-remove.sql
+	${libcore_SOURCE_DIR}/core/assets/sql/property-set.sql
+)
+
+set(
 	SOURCES
 	${libcore_SOURCE_DIR}/core/action.c
 	${libcore_SOURCE_DIR}/core/action.h
@@ -80,7 +88,8 @@
 
 molko_define_library(
 	TARGET libcore
-	SOURCES ${SOURCES}
+	SOURCES ${SOURCES} ${SQL_ASSETS}
+	ASSETS ${SQL_ASSETS}
 	LIBRARIES
 		libsqlite
 		SDL2::SDL2
@@ -98,3 +107,4 @@
 )
 
 source_group(core FILES ${SOURCES})
+source_group(core/assets/sql FILES ${SQL_ASSETS})
--- a/libcore/core/save.c	Wed Oct 21 11:47:18 2020 +0200
+++ b/libcore/core/save.c	Wed Oct 21 12:43:25 2020 +0200
@@ -18,146 +18,158 @@
 
 #include <assert.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include <sqlite3.h>
 
+#include <core/assets/sql/init.h>
+#include <core/assets/sql/property-get.h>
+#include <core/assets/sql/property-remove.h>
+#include <core/assets/sql/property-set.h>
+
 #include "error.h"
 #include "save.h"
 #include "sys.h"
-
-static sqlite3 *db;
+#include "util.h"
 
-static const char *sinit =
-	"BEGIN EXCLUSIVE TRANSACTION;"
-	""
-	"CREATE TABLE IF NOT EXISTS property("
-	"  id INTEGER PRIMARY KEY AUTOINCREMENT,"
-	"  key TEXT NOT NULL UNIQUE,"
-	"  value TEXT NOT NULL"
-	");"
-	""
-	"COMMIT"
-;
-
-static const char *sbegin =
-	"BEGIN EXCLUSIVE TRANSACTION"
-;
-
-static const char *scommit =
-	"COMMIT"
-;
+#define SQL_BEGIN       "BEGIN EXCLUSIVE TRANSACTION"
+#define SQL_COMMIT      "COMMIT"
+#define SQL_ROLLBACK    "ROLLBACK"
 
-static const char *srollback =
-	"ROLLBACK"
-;
+static bool
+exec(struct save *db, const char *sql)
+{
+	if (sqlite3_exec(db->handle, sql, NULL, NULL, NULL) != SQLITE_OK)
+		return errorf("%s", sqlite3_errmsg(db->handle));
 
-static const char *sset_property =
-	"INSERT OR REPLACE INTO property("
-	"  key,"
-	"  value"
-	")"
-	"VALUES("
-	"  ?,"
-	"  ?"
-	");"
-;
-
-static const char *sget_property =
-	"SELECT value"
-	"  FROM property"
-	" WHERE key = ?"
-;
-
-static const char *sremove_property =
-	"DELETE"
-	"  FROM property"
-	" WHERE key = ?"
-;
+	return true;
+}
 
 static bool
-exec(const char *sql)
+execu(struct save *db, const unsigned char *sql)
+{
+	return exec(db, (const char *)sql);
+}
+
+bool
+save_open(struct save *db, unsigned int idx, enum save_mode mode)
+{
+	assert(db);
+
+	return save_open_path(db, sys_savepath(idx), mode);
+}
+
+bool
+verify(struct save *db)
 {
-	if (sqlite3_exec(db, sql, NULL, NULL, NULL) != SQLITE_OK)
-		return errorf("%s", sqlite3_errmsg(db));
+	struct {
+		time_t *date;
+		struct save_property prop;
+	} table[] = {
+		{ .date = &db->created, { .key = "molko.create-date" } },
+		{ .date = &db->updated, { .key = "molko.update-date" } },
+	};
+
+	/* Ensure create and update dates are present. */
+	for (size_t i = 0; i < NELEM(table); ++i) {
+		if (!save_get_property(db, &table[i].prop)) {
+			sqlite3_close(db->handle);
+			return errorf("database not initialized correctly");
+		}
+
+		*table[i].date = strtoull(table[i].prop.value, NULL, 10);
+	}
 
 	return true;
 }
 
 bool
-save_open(unsigned int idx)
+save_open_path(struct save *db, const char *path, enum save_mode mode)
 {
-	return save_open_path(sys_savepath(idx));
+	assert(db);
+	assert(path);
+
+	int flags = 0;
+
+	switch (mode) {
+	case SAVE_MODE_WRITE:
+		flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
+		break;
+	default:
+		flags = SQLITE_OPEN_READONLY;
+		break;
+	}
+
+	if (sqlite3_open_v2(path, (sqlite3**)&db->handle, flags, NULL) != SQLITE_OK)
+		return errorf("%s", sqlite3_errmsg(db->handle));
+
+	if (mode == SAVE_MODE_WRITE) {
+		if (!execu(db, sql_init)) {
+			sqlite3_close(db->handle);
+			return false;
+		}
+	}
+
+	return verify(db);
 }
 
 bool
-save_open_path(const char *path)
+save_set_property(struct save *db, const struct save_property *prop)
 {
-	assert(path);
-
-	if (sqlite3_open(path, &db) != SQLITE_OK)
-		return errorf("database open error: %s", sqlite3_errmsg(db));
-	if (sqlite3_exec(db, sinit, NULL, NULL, NULL) != SQLITE_OK)
-		return errorf("database init error: %s", sqlite3_errmsg(db));
-
-	return true;
-}
-
-bool
-save_set_property(const char *key, const char *value)
-{
-	assert(key);
-	assert(value && strlen(value) <= SAVE_PROPERTY_VALUE_MAX);
+	assert(db);
+	assert(prop);
 
 	sqlite3_stmt *stmt = NULL;
 
-	if (!exec(sbegin))
+	if (!exec(db, SQL_BEGIN))
 		return false;
-	if (sqlite3_prepare(db, sset_property, -1, &stmt, NULL) != SQLITE_OK)
+	if (sqlite3_prepare(db->handle, (const char *)sql_property_set, -1, &stmt, NULL) != SQLITE_OK)
 		goto sqlite3_err;
-	if (sqlite3_bind_text(stmt, 1, key, -1, NULL) != SQLITE_OK ||
-	    sqlite3_bind_text(stmt, 2, value, -1, NULL) != SQLITE_OK)
+	if (sqlite3_bind_text(stmt, 1, prop->key, -1, NULL) != SQLITE_OK ||
+	    sqlite3_bind_text(stmt, 2, prop->value, -1, NULL) != SQLITE_OK)
 		goto sqlite3_err;
 	if (sqlite3_step(stmt) != SQLITE_DONE)
 		goto sqlite3_err;
 
 	sqlite3_finalize(stmt);
 
-	return exec(scommit);
+	return exec(db, SQL_COMMIT);
 
 sqlite3_err:
-	if (stmt) {
+	errorf("%s", sqlite3_errmsg(db->handle));
+
+	if (stmt)
 		sqlite3_finalize(stmt);
-		exec(srollback);
-	}
 
-	return errorf("%s", sqlite3_errmsg(db));
+	exec(db, SQL_ROLLBACK);
+
+	return false;
 }
 
-const char *
-save_get_property(const char *key)
+bool
+save_get_property(struct save *db, struct save_property *prop)
 {
-	assert(key);
+	assert(db);
+	assert(prop);
 
-	static char value[SAVE_PROPERTY_VALUE_MAX + 1];
-	const char *ret = value;
 	sqlite3_stmt *stmt = NULL;
+	bool ret = true;
 
-	memset(value, 0, sizeof (value));
-
-	if (sqlite3_prepare(db, sget_property, -1, &stmt, NULL) != SQLITE_OK)
+	if (sqlite3_prepare(db->handle, (const char *)sql_property_get,
+	    sizeof (sql_property_get), &stmt, NULL) != SQLITE_OK)
 		goto sqlite3_err;
-	if (sqlite3_bind_text(stmt, 1, key, -1, NULL) != SQLITE_OK)
+	if (sqlite3_bind_text(stmt, 1, prop->key, -1, NULL) != SQLITE_OK)
 		goto sqlite3_err;
 
 	switch (sqlite3_step(stmt)) {
 	case SQLITE_DONE:
 		/* Not found. */
-		ret = NULL;
+		ret = errorf("property '%s' was not found", prop->key);
 		break;
 	case SQLITE_ROW:
 		/* Found. */
-		snprintf(value, sizeof (value), "%s", sqlite3_column_text(stmt, 0));
+		snprintf(prop->value, sizeof (prop->value), "%s", sqlite3_column_text(stmt, 0));
 		break;
 	default:
 		/* Error. */
@@ -169,46 +181,54 @@
 	return ret;
 
 sqlite3_err:
+	errorf("%s", sqlite3_errmsg(db->handle));
+
 	if (stmt)
 		sqlite3_finalize(stmt);
 
-	errorf("%s", sqlite3_errmsg(db));
-
-	return NULL;
+	return false;
 }
 
 bool
-save_remove_property(const char *key)
+save_remove_property(struct save *db, const struct save_property *prop)
 {
-	assert(key);
+	assert(db);
+	assert(prop);
 
 	sqlite3_stmt *stmt = NULL;
 
-	if (!exec(sbegin))
+	if (!exec(db, SQL_BEGIN))
 		return false;
-	if (sqlite3_prepare(db, sremove_property, -1, &stmt, NULL) != SQLITE_OK)
+	if (sqlite3_prepare(db->handle, (const char *)sql_property_remove,
+	    sizeof (sql_property_remove), &stmt, NULL) != SQLITE_OK)
 		goto sqlite3_err;
-	if (sqlite3_bind_text(stmt, 1, key, -1, NULL) != SQLITE_OK)
+	if (sqlite3_bind_text(stmt, 1, prop->key, -1, NULL) != SQLITE_OK)
 		goto sqlite3_err;
 	if (sqlite3_step(stmt) != SQLITE_DONE)
 		goto sqlite3_err;
 
 	sqlite3_finalize(stmt);
 
-	return exec(scommit);
+	return exec(db, SQL_COMMIT);
 
 sqlite3_err:
+	errorf("%s", sqlite3_errmsg(db->handle));
+
 	if (stmt)
 		sqlite3_finalize(stmt);
 
-	errorf("%s", sqlite3_errmsg(db));
+	exec(db, SQL_ROLLBACK);
 
 	return false;
 }
 
 void
-save_finish(void)
+save_finish(struct save *db)
 {
-	if (db)
-		sqlite3_close(db);
+	assert(db);
+
+	if (db->handle)
+		sqlite3_close(db->handle);
+
+	memset(db, 0, sizeof (*db));
 }
--- a/libcore/core/save.h	Wed Oct 21 11:47:18 2020 +0200
+++ b/libcore/core/save.h	Wed Oct 21 12:43:25 2020 +0200
@@ -22,14 +22,60 @@
 /**
  * \file save.h
  * \brief Save functions.
+ *
+ * This module provides several functions to save the game data into a database.
+ *
+ * Database can be opened in read only mode (\ref SAVE_MODE_READ) which will
+ * return an error if not present or write mode (\ref SAVE_MODE_WRITE) which
+ * will create and initialize a database if not present on disk.
  */
 
 #include <stdbool.h>
+#include <time.h>
+
+#include "plat.h"
+
+/**
+ * \brief Max property key.
+ */
+#define SAVE_PROPERTY_KEY_MAX   (64)
 
 /**
  * \brief Max property value.
  */
-#define SAVE_PROPERTY_VALUE_MAX 1024
+#define SAVE_PROPERTY_VALUE_MAX (1024)
+
+/**
+ * \brief Save database handle.
+ */
+struct save {
+	time_t created; /*!< (-) Date when the save was created. */
+	time_t updated; /*!< (-) Date when it was last updated. */
+	void *handle;   /*!< (*) Private handle. */
+};
+
+/**
+ * \brief Open mode.
+ */
+enum save_mode {
+	SAVE_MODE_READ, /*!< Only try to read (no creation). */
+	SAVE_MODE_WRITE /*!< Open for both reading and writing */
+};
+
+/**
+ * \brief Mapping for the property table.
+ */
+struct save_property {
+	/**
+	 * (+) Property key to set.
+	 */
+	char key[SAVE_PROPERTY_KEY_MAX + 1];
+
+	/**
+	 * (+) Property value to set.
+	 */
+	char value[SAVE_PROPERTY_VALUE_MAX + 1];
+};
 
 /**
  * Open a database by index.
@@ -37,59 +83,74 @@
  * This function use the preferred path to store local files under the user
  * home directory. The parameter idx specifies the save slot to use.
  *
+ * \pre db != NULL
+ * \param db the database to initialize
  * \param idx the save slot
- * \return false on error
+ * \param mode the mode
+ * \return False on error.
  */
 bool
-save_open(unsigned int idx);
+save_open(struct save *db, unsigned int idx, enum save_mode mode) PLAT_NODISCARD;
 
 /**
  * Open the save slot specified by path.
  *
+ * \pre db != NULL
  * \pre path != NULL
+ * \param db the database to initialize
  * \param path the path to the save slot
- * \return false on error
+ * \param mode the mode
+ * \return False on error.
  */
 bool
-save_open_path(const char *path);
+save_open_path(struct save *db, const char *path, enum save_mode mode) PLAT_NODISCARD;
 
 /**
  * Sets an arbitrary property.
  *
  * If the property already exists, replace it.
  *
- * \pre key != NULL
- * \pre value != NULL && strlen(value) <= SAVE_PROPERTY_VALUE_MAX
- * \param key the property key
- * \param value the property value
+ * \pre db != NULL
+ * \pre prop != NULL
+ * \param db the database
+ * \param prop the property to set
+ * \return False on error.
  */
 bool
-save_set_property(const char *key, const char *value);
+save_set_property(struct save *db, const struct save_property *prop);
 
 /**
  * Get a property.
  *
- * \pre key != NULL
- * \param key the property key
- * \return the key or NULL if not found
+ * Call this function by setting prop->key to the desired key to get.
+ *
+ * \pre db != NULL
+ * \param db the database
+ * \param prop the property to retrieve
+ * \return False on error and prop->value is left untouched
  */
-const char *
-save_get_property(const char *key);
+bool
+save_get_property(struct save *db, struct save_property *prop) PLAT_NODISCARD;
 
 /**
  * Remove a property.
  *
- * \pre key != NULL
- * \param key the property key
+ * \pre db != NULL
+ * \pre prop != NULL
+ * \param db the database
+ * \param prop the property to remove
  * \return false on error
  */
 bool
-save_remove_property(const char *key);
+save_remove_property(struct save *db, const struct save_property *prop);
 
 /**
  * Close the save slot.
+ *
+ * \pre db != NULL
+ * \param db the database to close
  */
 void
-save_finish(void);
+save_finish(struct save *db);
 
 #endif /* !MOLKO_SAVE_H */
--- a/tests/test-save.c	Wed Oct 21 11:47:18 2020 +0200
+++ b/tests/test-save.c	Wed Oct 21 12:43:25 2020 +0200
@@ -18,44 +18,140 @@
 
 #include <stdio.h>
 
+#define GREATEST_USE_ABBREVS 0
 #include <greatest.h>
 
-#include "core/save.h"
-
-#define NAME "test.db"
+#include <core/save.h>
 
 static void
 clean(void *data)
 {
 	(void)data;
 
-	save_finish();
-	remove(NAME);
+	remove("1.db");
+	remove("2.db");
+}
+
+GREATEST_TEST
+open_read(void)
+{
+	struct save db;
+
+	/* Non-existent should return false. */
+	GREATEST_ASSERT(!save_open_path(&db, "1.db", SAVE_MODE_READ));
+
+	save_finish(&db);
+
+	GREATEST_PASS();
+}
+
+GREATEST_TEST
+open_write(void)
+{
+	struct save db[2] = {0};
+
+	/* Write should work on both non-existent and existent database. */
+	GREATEST_ASSERT(save_open_path(&db[0], "1.db", SAVE_MODE_WRITE));
+	GREATEST_ASSERT(save_open_path(&db[1], "2.db", SAVE_MODE_WRITE));
+
+	/* Update and create date must not be 0. */
+	GREATEST_ASSERT(db[0].created > 0);
+	GREATEST_ASSERT(db[0].updated > 0);
+	GREATEST_ASSERT(db[1].created > 0);
+	GREATEST_ASSERT(db[1].updated > 0);
+
+	save_finish(&db[0]);
+	save_finish(&db[1]);
+
+	/* Should work again. */
+	GREATEST_ASSERT(save_open_path(&db[0], "1.db", SAVE_MODE_WRITE));
+	GREATEST_ASSERT(save_open_path(&db[1], "2.db", SAVE_MODE_WRITE));
+	GREATEST_ASSERT(db[0].created > 0);
+	GREATEST_ASSERT(db[0].updated > 0);
+	GREATEST_ASSERT(db[1].created > 0);
+	GREATEST_ASSERT(db[1].updated > 0);
+
+	save_finish(&db[0]);
+	save_finish(&db[1]);
+
+	GREATEST_PASS();
 }
 
-TEST
-properties_simple(void)
+GREATEST_SUITE(open_suite)
+{
+	GREATEST_SET_SETUP_CB(clean, NULL);
+	GREATEST_SET_TEARDOWN_CB(clean, NULL);
+	GREATEST_RUN_TEST(open_read);
+	GREATEST_RUN_TEST(open_write);
+}
+
+GREATEST_TEST
+properties_set(void)
 {
-	ASSERT(save_open_path(NAME));
+	struct save db;
+	struct save_property prop;
+
+	GREATEST_ASSERT(save_open_path(&db, "1.db", SAVE_MODE_WRITE));
+
+	/* Insert a new property 'state'. */
+	prop = (struct save_property){.key = "state", .value = "intro"};
+	GREATEST_ASSERT(save_set_property(&db, &prop));
+	prop = (struct save_property){.key = "state"};
+	GREATEST_ASSERT(save_get_property(&db, &prop));
+	GREATEST_ASSERT_STR_EQ(prop.value, "intro");
+
+	/* Now we replace the value. */
+	prop = (struct save_property){.key = "state", .value = "map"};
+	GREATEST_ASSERT(save_set_property(&db, &prop));
+	prop = (struct save_property){.key = "state"};
+	GREATEST_ASSERT(save_get_property(&db, &prop));
+	GREATEST_ASSERT_STR_EQ(prop.value, "map");
+
+	save_finish(&db);
+
+	GREATEST_PASS();
+}
+
+GREATEST_TEST
+properties_notfound(void)
+{
+	struct save db;
+	struct save_property prop = {.key = "state"};
+
+	GREATEST_ASSERT(save_open_path(&db, "1.db", SAVE_MODE_WRITE));
+	GREATEST_ASSERT(!save_get_property(&db, &prop));
+	GREATEST_ASSERT_STR_EQ(prop.value, "");
+
+	GREATEST_PASS();
+}
+
+GREATEST_TEST
+properties_remove(void)
+{
+	struct save db;
+	struct save_property prop;
+
+	GREATEST_ASSERT(save_open_path(&db, "1.db", SAVE_MODE_WRITE));
 
 	/* Insert a new property 'initialized'. */
-	ASSERT(save_set_property("initialized", "1"));
-	ASSERT_STR_EQ(save_get_property("initialized"), "1");
+	prop = (struct save_property){.key = "state", .value = "intro"};
+	GREATEST_ASSERT(save_set_property(&db, &prop));
+	prop = (struct save_property){.key = "state"};
+	GREATEST_ASSERT(save_remove_property(&db, &prop));
+	prop = (struct save_property){.key = "state"};
+	GREATEST_ASSERT(!save_get_property(&db, &prop));
+	GREATEST_ASSERT_STR_EQ(prop.value, "");
 
-	/* This must replace the existing value. */
-	ASSERT(save_set_property("initialized", "2"));
-	ASSERT_STR_EQ(save_get_property("initialized"), "2");
-
-	ASSERT(save_remove_property("initialized"));
-	ASSERT(!save_get_property("initialized"));
-
-	PASS();
+	GREATEST_PASS();
 }
 
-SUITE(properties)
+GREATEST_SUITE(properties_suite)
 {
-	SET_TEARDOWN(clean, NULL);
-	RUN_TEST(properties_simple);
+	GREATEST_SET_SETUP_CB(clean, NULL);
+	GREATEST_SET_TEARDOWN_CB(clean, NULL);
+	GREATEST_RUN_TEST(properties_set);
+	GREATEST_RUN_TEST(properties_notfound);
+	GREATEST_RUN_TEST(properties_remove);
 }
 
 GREATEST_MAIN_DEFS();
@@ -64,6 +160,7 @@
 main(int argc, char **argv)
 {
 	GREATEST_MAIN_BEGIN();
-	RUN_SUITE(properties);
+	GREATEST_RUN_SUITE(open_suite);
+	GREATEST_RUN_SUITE(properties_suite);
 	GREATEST_MAIN_END();
 }