From 9465fd744cc2117190bafc1a3e2da9f10ca29bf9 Mon Sep 17 00:00:00 2001 From: Roland Reichwein Date: Sat, 31 Dec 2022 22:00:11 +0100 Subject: Storage via SQLite, Added tests (WIP) --- Makefile | 12 +- common.mk | 4 + config.cpp | 9 +- config.h | 2 + debian/rules | 1 - debian/whiteboard.whiteboard-cleanup.service | 10 -- debian/whiteboard.whiteboard-cleanup.timer | 10 -- main.cpp | 7 ++ storage.cpp | 129 ++++++++++++++++++- storage.h | 22 +++- tests/Makefile | 9 +- tests/test-config.cpp | 61 +++++++++ tests/test-storage.cpp | 178 +++++++++++++++++++++++++++ tests/unittests.cpp | 94 -------------- whiteboard-cleanup | 24 ---- whiteboard.cpp | 27 +++- whiteboard.cron | 2 - whiteboard.h | 6 + 18 files changed, 443 insertions(+), 164 deletions(-) delete mode 100644 debian/whiteboard.whiteboard-cleanup.service delete mode 100644 debian/whiteboard.whiteboard-cleanup.timer create mode 100644 main.cpp create mode 100644 tests/test-config.cpp create mode 100644 tests/test-storage.cpp delete mode 100644 tests/unittests.cpp delete mode 100755 whiteboard-cleanup delete mode 100644 whiteboard.cron create mode 100644 whiteboard.h diff --git a/Makefile b/Makefile index a35cb9d..85a5ed4 100755 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ DISTROS=base debian11 ubuntu2204 VERSION=$(shell dpkg-parsechangelog --show-field Version) INCLUDES=-I. -HEADERS=file.h config.h qrcode.h storage.h +HEADERS=file.h config.h qrcode.h storage.h whiteboard.h SOURCES=$(HEADERS:.h=.cpp) OBJECTS=$(HEADERS:.h=.o) TARGETS=whiteboard.fcgi @@ -33,17 +33,9 @@ install: mkdir -p $(DESTDIR)/etc cp whiteboard.conf $(DESTDIR)/etc - - mkdir -p $(DESTDIR)/usr/bin - cp whiteboard-cleanup $(DESTDIR)/usr/bin/ - - mkdir -p $(DESTDIR)/etc/cron.d - cp whiteboard.cron $(DESTDIR)/etc/cron.d/whiteboard - -whiteboard.fcgi: $(OBJECTS) # link -%.fcgi: %.o +whiteboard.fcgi: $(OBJECTS) main.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) $(LIBS) -o $@ # .cpp -> .o diff --git a/common.mk b/common.mk index 7f34a96..9da602e 100644 --- a/common.mk +++ b/common.mk @@ -1,4 +1,8 @@ +CXX=clang++-14 + +ifeq ($(shell which $(CXX)),) CXX=clang++-13 +endif ifeq ($(shell which $(CXX)),) CXX=clang++-10 diff --git a/config.cpp b/config.cpp index 0255023..7474a1d 100644 --- a/config.cpp +++ b/config.cpp @@ -9,9 +9,10 @@ namespace pt = boost::property_tree; namespace { const std::string default_datapath {"/var/lib/whiteboard"}; + const uint64_t default_maxage{0}; // timeout in seconds; 0 = no timeout } -Config::Config(const std::string& config_filename): m_dataPath{default_datapath} +Config::Config(const std::string& config_filename): m_dataPath{default_datapath}, m_maxage{default_maxage} { try { @@ -20,6 +21,7 @@ Config::Config(const std::string& config_filename): m_dataPath{default_datapath} pt::read_xml(config_filename, tree, pt::xml_parser::no_comments | pt::xml_parser::trim_whitespace); m_dataPath = tree.get("config.datapath", default_datapath); + m_maxage = tree.get("config.maxage", default_maxage); } catch (const std::exception& ex) { std::cerr << "Error reading config file " << config_filename << ". Using " << default_datapath << "." << std::endl; } @@ -29,3 +31,8 @@ std::string Config::getDataPath() const { return m_dataPath; } + +uint64_t Config::getMaxage() const +{ + return m_maxage; +} diff --git a/config.h b/config.h index ab95dd5..4f589ff 100644 --- a/config.h +++ b/config.h @@ -8,8 +8,10 @@ class Config { private: std::string m_dataPath; + uint64_t m_maxage; public: Config(const std::string& config_filename = default_config_filename); std::string getDataPath() const; + uint64_t getMaxage() const; }; diff --git a/debian/rules b/debian/rules index 2ee2604..72c5b8c 100755 --- a/debian/rules +++ b/debian/rules @@ -11,4 +11,3 @@ override_dh_fixperms: override_dh_auto_install: dh_auto_install dh_installsystemd --name whiteboard - dh_installsystemd --name whiteboard-cleanup diff --git a/debian/whiteboard.whiteboard-cleanup.service b/debian/whiteboard.whiteboard-cleanup.service deleted file mode 100644 index 339c143..0000000 --- a/debian/whiteboard.whiteboard-cleanup.service +++ /dev/null @@ -1,10 +0,0 @@ -[Unit] -Description=Cleanup whiteboard data -Wants=whiteboard-cleanup.timer - -[Service] -Type=oneshot -ExecStart=/usr/bin/whiteboard-cleanup - -[Install] -WantedBy=multi-user.target diff --git a/debian/whiteboard.whiteboard-cleanup.timer b/debian/whiteboard.whiteboard-cleanup.timer deleted file mode 100644 index e2c93f9..0000000 --- a/debian/whiteboard.whiteboard-cleanup.timer +++ /dev/null @@ -1,10 +0,0 @@ -[Unit] -Description=Clean up whiteboard data -Requires=whiteboard-cleanup.service - -[Timer] -Unit=whiteboard-cleanup.service -OnCalendar=*-*-* 00:00:00 - -[Install] -WantedBy=timers.target diff --git a/main.cpp b/main.cpp new file mode 100644 index 0000000..c46b53f --- /dev/null +++ b/main.cpp @@ -0,0 +1,7 @@ +#include "whiteboard.h" + +int main(int argc, char* argv[]) +{ + return whiteboard(argc, argv); +} + diff --git a/storage.cpp b/storage.cpp index 9dc7615..545ba04 100644 --- a/storage.cpp +++ b/storage.cpp @@ -2,16 +2,135 @@ #include "config.h" +#include + #include -Storage::Storage(const Config& config): m_config(config) +using namespace std::string_literals; + +Storage::Storage(const Config& config): + m_db(config.getDataPath() + "/whiteboard.db3", SQLite::OPEN_READWRITE | SQLite::OPEN_CREATE), + m_maxage(config.getMaxage()) +{ + m_db.exec("CREATE TABLE IF NOT EXISTS documents (id VARCHAR(16) PRIMARY KEY, value BLOB, rev INTEGER, cursorpos INTEGER, timestamp BIGINT)"); +} + +uint64_t Storage::getNumberOfDocuments() +{ + SQLite::Statement query(m_db, "SELECT COUNT(*) FROM documents"); + if (!query.executeStep()) + throw std::runtime_error("Count not possible"); + + return static_cast(query.getColumn(0)); +} + +void Storage::cleanup() { - SQLite::Database db(m_config.getDataPath() + "/whiteboard.db3", SQLite::OPEN_READWRITE|SQLite::OPEN_CREATE); + if (m_maxage == 0) + return; + + SQLite::Statement query(m_db, "DELETE FROM documents WHERE timestamp + ? < unixepoch()"); + query.bind(1, static_cast(m_maxage)); - db.exec("CREATE TABLE IF NOT EXISTS documents (id INTEGER PRIMARY KEY, value TEXT)"); + query.exec(); } -std::string Storage::getDocument() +bool Storage::exists(const std::string& id) { - return ""; + SQLite::Statement query(m_db, "SELECT id FROM documents WHERE id = ?"); + query.bind(1, id); + + return query.executeStep(); } + +std::string Storage::getDocument(const std::string& id) +{ + SQLite::Statement query(m_db, "SELECT value FROM documents WHERE id = ?"); + query.bind(1, id); + + if (!query.executeStep()) + throw std::runtime_error("id "s + id + " not found"s); + + return query.getColumn(0); +} + +int Storage::getRevision(const std::string& id) +{ + SQLite::Statement query(m_db, "SELECT rev FROM documents WHERE id = ?"); + query.bind(1, id); + + if (!query.executeStep()) + throw std::runtime_error("id "s + id + " not found"s); + + return query.getColumn(0); +} + +int Storage::getCursorPos(const std::string& id) +{ + SQLite::Statement query(m_db, "SELECT cursorpos FROM documents WHERE id = ?"); + query.bind(1, id); + + if (!query.executeStep()) + throw std::runtime_error("id "s + id + " not found"s); + + return query.getColumn(0); +} + +std::tuple Storage::getRow(const std::string& id) +{ + SQLite::Statement query(m_db, "SELECT value, rev, cursorpos FROM documents WHERE id = ?"); + query.bind(1, id); + + if (!query.executeStep()) + throw std::runtime_error("id "s + id + " not found"s); + + return {query.getColumn(0), query.getColumn(1), query.getColumn(2)}; +} + +void Storage::setDocument(const std::string& id, const std::string& document) +{ + SQLite::Statement query(m_db, "UPDATE documents SET value = ? WHERE id = ?"); + query.bind(1, document); + query.bind(2, id); + + if (!query.exec()) { + SQLite::Statement query(m_db, "INSERT INTO documents (id, value, rev, cursorpos, timestamp) values (?, ?, ?, ?, unixepoch())"); + query.bind(1, id); + query.bind(2, document); + query.bind(3, 0); + query.bind(4, 0); + query.exec(); + } +} + +void Storage::setRevision(const std::string& id, int rev) +{ + SQLite::Statement query(m_db, "UPDATE documents SET rev = ? WHERE id = ?"); + query.bind(1, rev); + query.bind(2, id); + + if (!query.exec()) + throw std::runtime_error("Unable to insert row with id "s + id); +} + +void Storage::setCursorPos(const std::string& id, int cursorPos) +{ + SQLite::Statement query(m_db, "UPDATE documents SET cursorpos = ? WHERE id = ?"); + query.bind(1, cursorPos); + query.bind(2, id); + + if (!query.exec()) + throw std::runtime_error("Unable to insert row with id "s + id); +} + +void Storage::setRow(const std::string& id, const std::string& document, int rev, int cursorPos) +{ + SQLite::Statement query(m_db, "INSERT OR REPLACE INTO documents (id, value, rev, cursorpos, timestamp) values (?, ?, ?, ?, unixepoch())"); + query.bind(1, id); + query.bind(2, document); + query.bind(3, rev); + query.bind(4, cursorPos); + if (!query.exec()) + throw std::runtime_error("Unable to insert row with id "s + id); +} + diff --git a/storage.h b/storage.h index 068fad0..dc4e216 100644 --- a/storage.h +++ b/storage.h @@ -1,6 +1,9 @@ #pragma once #include +#include + +#include #include "config.h" @@ -8,9 +11,24 @@ class Storage { public: Storage(const Config& config); - std::string getDocument(); + + uint64_t getNumberOfDocuments(); + bool exists(const std::string& id); + + std::string getDocument(const std::string& id); + int getRevision(const std::string& id); + int getCursorPos(const std::string& id); + std::tuple getRow(const std::string& id); + + void setDocument(const std::string& id, const std::string& document); + void setRevision(const std::string& id, int rev); + void setCursorPos(const std::string& id, int cursorPos); + void setRow(const std::string& id, const std::string& document, int rev, int cursorPos); + + void cleanup(); private: - const Config& m_config; + SQLite::Database m_db; + uint64_t m_maxage; }; diff --git a/tests/Makefile b/tests/Makefile index 78097ce..1f912c3 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -1,5 +1,8 @@ include ../common.mk +UNITTESTS=test-config.cpp \ + test-storage.cpp + CXXFLAGS+=\ -I/usr/src/googletest/googletest/include \ -I/usr/src/googletest/googlemock/include \ @@ -10,11 +13,11 @@ CXXFLAGS+=\ test: unittests ./unittests -unittests: libgmock.a unittests.o ../config.o ../file.o ../storage.o +unittests: libgmock.a $(UNITTESTS:.cpp=.o) ../config.o ../file.o ../storage.o $(CXX) $(LDFLAGS) $^ $(LDLIBS) $(LIBS) -o $@ -unittests.o: unittests.cpp - $(CXX) $(CXXFLAGS) -o $@ -c unittests.cpp +%.o: %.cpp + $(CXX) $(CXXFLAGS) -o $@ -c $< libgmock.a: $(CXX) $(CXXFLAGS) -c /usr/src/googletest/googletest/src/gtest-all.cc diff --git a/tests/test-config.cpp b/tests/test-config.cpp new file mode 100644 index 0000000..065dedf --- /dev/null +++ b/tests/test-config.cpp @@ -0,0 +1,61 @@ +#include + +#include +#include +#include + +#include "config.h" +#include "file.h" + +namespace fs = std::filesystem; + +namespace { + const std::string testConfigFilename{"./test.conf"}; + const std::string testDbFilename{"./whiteboard.db3"}; +} + +class ConfigTest: public ::testing::Test +{ +protected: + ConfigTest(){ + } + + ~ConfigTest(){ + } +}; + +TEST_F(ConfigTest, defaultData) +{ + std::string filename{testConfigFilename + "doesntexist"}; + std::error_code ec; + fs::remove(filename, ec); + ASSERT_TRUE(!fs::exists(filename)); + { + Config config{filename}; + EXPECT_EQ(config.getDataPath(), "/var/lib/whiteboard"); + EXPECT_EQ(config.getMaxage(), 0); + ASSERT_TRUE(!fs::exists(filename)); + } + + ASSERT_TRUE(!fs::exists(filename)); +} + +TEST_F(ConfigTest, testData) +{ + File::setFile(testConfigFilename, R"CONFIG( + + /some/other/location + 2592000 + +)CONFIG"); + + { + Config config{testConfigFilename}; + EXPECT_EQ(config.getDataPath(), "/some/other/location"); + EXPECT_EQ(config.getMaxage(), 2592000); + } + + std::error_code ec; + fs::remove(testConfigFilename, ec); +} + diff --git a/tests/test-storage.cpp b/tests/test-storage.cpp new file mode 100644 index 0000000..67d7236 --- /dev/null +++ b/tests/test-storage.cpp @@ -0,0 +1,178 @@ +#include + +#include +#include +#include + +#include "config.h" +#include "file.h" +#include "storage.h" + +namespace fs = std::filesystem; + +namespace { + const std::string testConfigFilename{"./test.conf"}; + const std::string testDbFilename{"./whiteboard.db3"}; +} + +class StorageTest: public ::testing::Test +{ +protected: + StorageTest(){ + File::setFile(testConfigFilename, R"CONFIG( + + . + 2592000 + +)CONFIG"); + std::error_code ec; + fs::remove(testDbFilename, ec); + + m_config = Config{testConfigFilename}; + } + + ~StorageTest(){ + std::error_code ec; + fs::remove(testDbFilename, ec); + fs::remove(testConfigFilename, ec); + } + + Config m_config; +}; + +TEST_F(StorageTest, create) +{ + ASSERT_TRUE(!fs::exists(testDbFilename)); + + { + ASSERT_EQ(m_config.getDataPath(), "."); + ASSERT_TRUE(!fs::exists(testDbFilename)); + Storage storage(m_config); + } + + ASSERT_TRUE(fs::exists(testDbFilename)); +} + +TEST_F(StorageTest, getNumberOfDocuments) +{ + Storage storage(m_config); + EXPECT_EQ(storage.getNumberOfDocuments(), 0); + storage.setDocument("123", "abc"); + EXPECT_EQ(storage.getNumberOfDocuments(), 1); + storage.setDocument("def", "xyz"); + EXPECT_EQ(storage.getNumberOfDocuments(), 2); +} + +TEST_F(StorageTest, cleanup_empty) +{ + Storage storage(m_config); + EXPECT_EQ(storage.getNumberOfDocuments(), 0); + storage.cleanup(); + EXPECT_EQ(storage.getNumberOfDocuments(), 0); +} + +TEST_F(StorageTest, cleanup) +{ + Storage storage(m_config); + EXPECT_EQ(storage.getNumberOfDocuments(), 0); + storage.setDocument("123", "abc"); + EXPECT_EQ(storage.getNumberOfDocuments(), 1); + storage.cleanup(); + EXPECT_EQ(storage.getNumberOfDocuments(), 1); +} + +TEST_F(StorageTest, exists) +{ + Storage storage(m_config); + EXPECT_EQ(storage.exists(""), false); + EXPECT_EQ(storage.exists("0"), false); + EXPECT_EQ(storage.exists("123"), false); + EXPECT_EQ(storage.exists("abcdz"), false); + + storage.setDocument("", "abc"); + EXPECT_EQ(storage.exists(""), true); + storage.setDocument("0", "abc"); + EXPECT_EQ(storage.exists("0"), true); + storage.setDocument("123", "abc"); + EXPECT_EQ(storage.exists("123"), true); + storage.setDocument("abcdz", "abc"); + EXPECT_EQ(storage.exists("abcdz"), true); +} + +TEST_F(StorageTest, setDocument) +{ + Storage storage(m_config); + storage.setDocument("0", "abc"); + EXPECT_EQ(storage.getNumberOfDocuments(), 1); + EXPECT_EQ(storage.getDocument("0"), "abc"); +} + +TEST_F(StorageTest, setRevision) +{ + Storage storage(m_config); + storage.setDocument("0", "abc"); + storage.setRevision("0", 123); + + EXPECT_EQ(storage.getNumberOfDocuments(), 1); + EXPECT_EQ(storage.getRevision("0"), 123); +} + +TEST_F(StorageTest, setCursorPos) +{ + Storage storage(m_config); + storage.setDocument("0", "abc"); + storage.setCursorPos("0", 1234); + + EXPECT_EQ(storage.getNumberOfDocuments(), 1); + EXPECT_EQ(storage.getCursorPos("0"), 1234); +} + +TEST_F(StorageTest, setRow) +{ + Storage storage(m_config); + storage.setRow("0", "abc", 56, 67); + + EXPECT_EQ(storage.getNumberOfDocuments(), 1); + EXPECT_EQ(storage.getDocument("0"), "abc"); + EXPECT_EQ(storage.getRevision("0"), 56); + EXPECT_EQ(storage.getCursorPos("0"), 67); +} + +TEST_F(StorageTest, getDocument) +{ + Storage storage(m_config); + storage.setDocument("0", "xyz"); + storage.setDocument("0bc", "xyz2"); + storage.setDocument("iabc", "xyz3"); + storage.setDocument("zxy", "xyz4"); + + EXPECT_EQ(storage.getDocument("0"), "xyz"); +} + +TEST_F(StorageTest, getRevision) +{ + Storage storage(m_config); + storage.setRow("0", "abc", 123, 456); + + EXPECT_EQ(storage.getRevision("0"), 123); +} + +TEST_F(StorageTest, getCursorPos) +{ + Storage storage(m_config); + storage.setRow("0", "abc", 123, 456); + + EXPECT_EQ(storage.getCursorPos("0"), 456); +} + +TEST_F(StorageTest, getRow) +{ + Storage storage(m_config); + storage.setRow("0", "abc", 123, 456); + + auto row{storage.getRow("0")}; + EXPECT_EQ(std::get<0>(row), "abc"); + EXPECT_EQ(std::get<1>(row), 123); + EXPECT_EQ(std::get<2>(row), 456); +} + diff --git a/tests/unittests.cpp b/tests/unittests.cpp deleted file mode 100644 index 3b24f83..0000000 --- a/tests/unittests.cpp +++ /dev/null @@ -1,94 +0,0 @@ -#include - -#include -#include -#include - -#include "config.h" -#include "file.h" -#include "storage.h" - -namespace fs = std::filesystem; - -namespace { - const std::string testConfigFilename{"./test.conf"}; - const std::string testDbFilename{"./whiteboard.db3"}; -} - -class ConfigTest: public ::testing::Test -{ -protected: - ConfigTest(){ - } - - ~ConfigTest(){ - } -}; - -TEST_F(ConfigTest, defaultData) -{ - std::string filename{testConfigFilename + "doesntexist"}; - std::error_code ec; - fs::remove(filename, ec); - ASSERT_TRUE(!fs::exists(filename)); - { - Config config{filename}; - EXPECT_EQ(config.getDataPath(), "/var/lib/whiteboard"); - ASSERT_TRUE(!fs::exists(filename)); - } - - ASSERT_TRUE(!fs::exists(filename)); -} - -TEST_F(ConfigTest, testData) -{ - File::setFile(testConfigFilename, R"CONFIG( - - /some/other/location - 2592000 - -)CONFIG"); - - { - Config config{testConfigFilename}; - EXPECT_EQ(config.getDataPath(), "/some/other/location"); - } - - std::error_code ec; - fs::remove(testConfigFilename, ec); -} - -class StorageTest: public ::testing::Test -{ -protected: - StorageTest(){ - File::setFile(testConfigFilename, R"CONFIG( - - . - 2592000 - -)CONFIG"); - std::error_code ec; - fs::remove(testDbFilename, ec); - } - - ~StorageTest(){ - std::error_code ec; - fs::remove(testDbFilename, ec); - fs::remove(testConfigFilename, ec); - } -}; - -TEST_F(StorageTest, create) -{ - ASSERT_TRUE(!fs::exists(testDbFilename)); - - { - Config config(testConfigFilename); - ASSERT_EQ(config.getDataPath(), "."); - Storage storage(config); - } - - ASSERT_TRUE(fs::exists(testDbFilename)); -} - diff --git a/whiteboard-cleanup b/whiteboard-cleanup deleted file mode 100755 index 7a987e0..0000000 --- a/whiteboard-cleanup +++ /dev/null @@ -1,24 +0,0 @@ -#!/bin/bash -# -# Cleanup global whiteboard files -# -# To be called by cron -# - -set -e - -# look up config file -CONFIGFILE=/etc/whiteboard.conf -XMLLINTOPTS="--nonet --nocdata --nocatalogs" -DATAPATH=`xmllint --xpath "/config/datapath/text()" $XMLLINTOPTS $CONFIGFILE` -MAXAGE=`xmllint --xpath "/config/maxage/text()" $XMLLINTOPTS $CONFIGFILE` - -cd $DATAPATH - -ls -1 | while read i ; do - AGE=$((`date +"%s"` - `stat -c "%Y" $i`)) - if [[ "$AGE" -gt "$MAXAGE" ]] ; then - echo "Deleting entry $i ..." - rm $i - fi -done diff --git a/whiteboard.cpp b/whiteboard.cpp index 45434df..6d07576 100644 --- a/whiteboard.cpp +++ b/whiteboard.cpp @@ -93,15 +93,38 @@ namespace { return File::getFile(path); } -} -int main(void) + void usage() { + std::cout << + "Usage: \n" + " whiteboard [-c]\n" + "\n" + "Options:\n" + " -c : Cleanup database according to timeout rules (config: maxage)\n" + "\n" + "Without options, whiteboard will be started as FCGI application" + << std::endl; + } +} // namespace + +// the actual main() for testability +int whiteboard(int argc, char* argv[]) { Config config; data_path = config.getDataPath(); Storage storage(config); + if (argc == 2) { + if (argv[1] == "-h"s || argv[1] == "-?"s) { + usage(); + exit(0); + } else if (argv[1] == "-c"s) { + storage.cleanup(); + exit(0); + } + } + Magick::InitializeMagick(NULL); // for qrcode.cpp int result = FCGX_Init(); diff --git a/whiteboard.cron b/whiteboard.cron deleted file mode 100644 index 705c2eb..0000000 --- a/whiteboard.cron +++ /dev/null @@ -1,2 +0,0 @@ -# Cleanup whiteboard data once every day -02,31 * * * * root [ -x /usr/bin/whiteboard-cleanup ] && if [ ! -d /run/systemd/system ]; then /usr/bin/whiteboard-cleanup ; fi diff --git a/whiteboard.h b/whiteboard.h new file mode 100644 index 0000000..39eeb66 --- /dev/null +++ b/whiteboard.h @@ -0,0 +1,6 @@ +// pseudo main() - for testability + +#pragma once + +int whiteboard(int argc, char* argv[]); + -- cgit v1.2.3