From a38983dc0c72aae90acf57f763bda34e90255ec6 Mon Sep 17 00:00:00 2001
From: Valtteri Koskivuori <vkoskiv@gmail.com>
Date: Thu, 14 Oct 2021 21:37:11 +0300
Subject: [PATCH] ConfigServer: Sync config files to disk automatically

At the risk of introducing premature optimization, it only syncs to disk
5 seconds after the latest valid configuration update, to handle cases
where programs might send frequent updates that would go to disk every
time otherwise.
Also syncs to disk when the client connection closes.

Co-authored-by: Timothy Flynn <trflynn@pm.me>
---
 .../ConfigServer/ClientConnection.cpp         | 33 ++++++++++++++++++-
 .../Services/ConfigServer/ClientConnection.h  |  5 +++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Userland/Services/ConfigServer/ClientConnection.cpp b/Userland/Services/ConfigServer/ClientConnection.cpp
index 4f5e7b02f4..11bcd1c0b0 100644
--- a/Userland/Services/ConfigServer/ClientConnection.cpp
+++ b/Userland/Services/ConfigServer/ClientConnection.cpp
@@ -8,6 +8,7 @@
 #include <ConfigServer/ConfigClientEndpoint.h>
 #include <LibCore/ConfigFile.h>
 #include <LibCore/FileWatcher.h>
+#include <LibCore/Timer.h>
 
 namespace ConfigServer {
 
@@ -20,6 +21,7 @@ struct CachedDomain {
 };
 
 static HashMap<String, NonnullOwnPtr<CachedDomain>> s_cache;
+static constexpr int s_disk_sync_delay_ms = 5'000;
 
 static Core::ConfigFile& ensure_domain_config(String const& domain)
 {
@@ -34,7 +36,7 @@ static Core::ConfigFile& ensure_domain_config(String const& domain)
     auto result = watcher_or_error.value()->add_watch(config->filename(), Core::FileWatcherEvent::Type::ContentModified);
     VERIFY(!result.is_error());
     watcher_or_error.value()->on_change = [config, domain](auto&) {
-        auto new_config = Core::ConfigFile::open(config->filename());
+        auto new_config = Core::ConfigFile::open(config->filename(), Core::ConfigFile::AllowWriting::Yes);
         // FIXME: Detect removed keys.
         // FIXME: Detect type of keys.
         for (auto& group : new_config->groups()) {
@@ -60,6 +62,7 @@ static Core::ConfigFile& ensure_domain_config(String const& domain)
 
 ClientConnection::ClientConnection(NonnullRefPtr<Core::LocalSocket> client_socket, int client_id)
     : IPC::ClientConnection<ConfigClientEndpoint, ConfigServerEndpoint>(*this, move(client_socket), client_id)
+    , m_sync_timer(Core::Timer::create_single_shot(s_disk_sync_delay_ms, [this]() { sync_dirty_domains_to_disk(); }))
 {
     s_connections.set(client_id, *this);
 }
@@ -71,6 +74,8 @@ ClientConnection::~ClientConnection()
 void ClientConnection::die()
 {
     s_connections.remove(client_id());
+    m_sync_timer->stop();
+    sync_dirty_domains_to_disk();
 }
 
 void ClientConnection::pledge_domains(Vector<String> const& domains)
@@ -104,6 +109,18 @@ bool ClientConnection::validate_access(String const& domain, String const& group
     return false;
 }
 
+void ClientConnection::sync_dirty_domains_to_disk()
+{
+    if (m_dirty_domains.is_empty())
+        return;
+    auto dirty_domains = move(m_dirty_domains);
+    dbgln("Syncing {} dirty domains to disk", dirty_domains.size());
+    for (auto domain : dirty_domains) {
+        auto& config = ensure_domain_config(domain);
+        config.sync();
+    }
+}
+
 Messages::ConfigServer::ReadStringValueResponse ClientConnection::read_string_value(String const& domain, String const& group, String const& key)
 {
     if (!validate_access(domain, group, key))
@@ -137,6 +154,14 @@ Messages::ConfigServer::ReadBoolValueResponse ClientConnection::read_bool_value(
     return Optional<bool> { config.read_bool_entry(group, key) };
 }
 
+void ClientConnection::start_or_restart_sync_timer()
+{
+    if (m_sync_timer->is_active())
+        m_sync_timer->restart();
+    else
+        m_sync_timer->start();
+}
+
 void ClientConnection::write_string_value(String const& domain, String const& group, String const& key, String const& value)
 {
     if (!validate_access(domain, group, key))
@@ -148,6 +173,8 @@ void ClientConnection::write_string_value(String const& domain, String const& gr
         return;
 
     config.write_entry(group, key, value);
+    m_dirty_domains.set(domain);
+    start_or_restart_sync_timer();
 
     for (auto& it : s_connections) {
         if (it.value != this && it.value->m_monitored_domains.contains(domain))
@@ -166,6 +193,8 @@ void ClientConnection::write_i32_value(String const& domain, String const& group
         return;
 
     config.write_num_entry(group, key, value);
+    m_dirty_domains.set(domain);
+    start_or_restart_sync_timer();
 
     for (auto& it : s_connections) {
         if (it.value != this && it.value->m_monitored_domains.contains(domain))
@@ -184,6 +213,8 @@ void ClientConnection::write_bool_value(String const& domain, String const& grou
         return;
 
     config.write_bool_entry(group, key, value);
+    m_dirty_domains.set(domain);
+    start_or_restart_sync_timer();
 
     for (auto& it : s_connections) {
         if (it.value != this && it.value->m_monitored_domains.contains(domain))
diff --git a/Userland/Services/ConfigServer/ClientConnection.h b/Userland/Services/ConfigServer/ClientConnection.h
index 026f17ff81..e673bd0b6a 100644
--- a/Userland/Services/ConfigServer/ClientConnection.h
+++ b/Userland/Services/ConfigServer/ClientConnection.h
@@ -35,11 +35,16 @@ private:
     virtual void write_bool_value([[maybe_unused]] String const& domain, [[maybe_unused]] String const& group, [[maybe_unused]] String const& key, [[maybe_unused]] bool value) override;
 
     bool validate_access(String const& domain, String const& group, String const& key);
+    void sync_dirty_domains_to_disk();
+    void start_or_restart_sync_timer();
 
     bool m_has_pledged { false };
     HashTable<String> m_pledged_domains;
 
     HashTable<String> m_monitored_domains;
+
+    NonnullRefPtr<Core::Timer> m_sync_timer;
+    HashTable<String> m_dirty_domains;
 };
 
 }