From edf784340999b6e8f44241e04e6d6f8afc27be29 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 26 Aug 2021 19:05:50 +0200 Subject: [PATCH] ConfigServer+LibConfig: Add way for clients to listen for config changes This patch adds a Config::Listener abstract class that anyone can inherit from and receive notifications when configuration values change. We don't yet monitor file system changes, so these only work for changes made by ConfigServer itself. In order to receive these notifications, clients must monitor the domain by calling monitor_domain(). Only pledged domains can be monitored. Note that the client initiating the change does not get notified. --- Userland/Libraries/LibConfig/CMakeLists.txt | 1 + Userland/Libraries/LibConfig/Client.cpp | 27 ++++++ Userland/Libraries/LibConfig/Client.h | 10 +++ Userland/Libraries/LibConfig/Listener.cpp | 44 ++++++++++ Userland/Libraries/LibConfig/Listener.h | 27 ++++++ .../ConfigServer/ClientConnection.cpp | 86 +++++++++++++++---- .../Services/ConfigServer/ClientConnection.h | 3 + .../Services/ConfigServer/ConfigClient.ipc | 3 + .../Services/ConfigServer/ConfigServer.ipc | 2 + 9 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 Userland/Libraries/LibConfig/Listener.cpp create mode 100644 Userland/Libraries/LibConfig/Listener.h diff --git a/Userland/Libraries/LibConfig/CMakeLists.txt b/Userland/Libraries/LibConfig/CMakeLists.txt index 04527241bd..f4506dc610 100644 --- a/Userland/Libraries/LibConfig/CMakeLists.txt +++ b/Userland/Libraries/LibConfig/CMakeLists.txt @@ -1,5 +1,6 @@ set(SOURCES Client.cpp + Listener.cpp ) set(GENERATED_SOURCES diff --git a/Userland/Libraries/LibConfig/Client.cpp b/Userland/Libraries/LibConfig/Client.cpp index a3c3bef4b3..2a53ecee8f 100644 --- a/Userland/Libraries/LibConfig/Client.cpp +++ b/Userland/Libraries/LibConfig/Client.cpp @@ -5,6 +5,7 @@ */ #include +#include namespace Config { @@ -24,6 +25,11 @@ void Client::pledge_domains(Vector const& domains) async_pledge_domains(domains); } +void Client::monitor_domain(String const& domain) +{ + async_monitor_domain(domain); +} + String Client::read_string(StringView domain, StringView group, StringView key, StringView fallback) { return read_string_value(domain, group, key).value_or(fallback); @@ -54,4 +60,25 @@ void Client::write_bool(StringView domain, StringView group, StringView key, boo async_write_bool_value(domain, group, key, value); } +void Client::notify_changed_string_value(String const& domain, String const& group, String const& key, String const& value) +{ + Listener::for_each([&](auto& listener) { + listener.config_string_did_change(domain, group, key, value); + }); +} + +void Client::notify_changed_i32_value(String const& domain, String const& group, String const& key, i32 value) +{ + Listener::for_each([&](auto& listener) { + listener.config_i32_did_change(domain, group, key, value); + }); +} + +void Client::notify_changed_bool_value(String const& domain, String const& group, String const& key, bool value) +{ + Listener::for_each([&](auto& listener) { + listener.config_bool_did_change(domain, group, key, value); + }); +} + } diff --git a/Userland/Libraries/LibConfig/Client.h b/Userland/Libraries/LibConfig/Client.h index 395a3188fc..ce6efdbf3f 100644 --- a/Userland/Libraries/LibConfig/Client.h +++ b/Userland/Libraries/LibConfig/Client.h @@ -22,6 +22,7 @@ class Client final public: void pledge_domains(Vector const&); + void monitor_domain(String const&); String read_string(StringView domain, StringView group, StringView key, StringView fallback); i32 read_i32(StringView domain, StringView group, StringView key, i32 fallback); @@ -38,6 +39,10 @@ private: : IPC::ServerConnection(*this, "/tmp/portal/config") { } + + void notify_changed_string_value(String const& domain, String const& group, String const& key, String const& value) override; + void notify_changed_i32_value(String const& domain, String const& group, String const& key, i32 value) override; + void notify_changed_bool_value(String const& domain, String const& group, String const& key, bool value) override; }; inline String read_string(StringView domain, StringView group, StringView key, StringView fallback = {}) @@ -80,4 +85,9 @@ inline void pledge_domains(String const& domains) Client::the().pledge_domains({ domains }); } +inline void monitor_domain(String const& domain) +{ + Client::the().monitor_domain(domain); +} + } diff --git a/Userland/Libraries/LibConfig/Listener.cpp b/Userland/Libraries/LibConfig/Listener.cpp new file mode 100644 index 0000000000..dbf452cded --- /dev/null +++ b/Userland/Libraries/LibConfig/Listener.cpp @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2021, Andreas Kling + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include + +namespace Config { + +static HashTable s_listeners; + +Listener::Listener() +{ + s_listeners.set(this); +} + +Listener::~Listener() +{ + s_listeners.remove(this); +} + +void Listener::for_each(Function callback) +{ + for (auto* listener : s_listeners) + callback(*listener); +} + +void Listener::config_string_did_change(String const&, String const&, String const&, String const&) +{ +} + +void Listener::config_i32_did_change(String const&, String const&, String const&, i32) +{ +} + +void Listener::config_bool_did_change(String const&, String const&, String const&, bool) +{ +} + +} diff --git a/Userland/Libraries/LibConfig/Listener.h b/Userland/Libraries/LibConfig/Listener.h new file mode 100644 index 0000000000..239a7d1561 --- /dev/null +++ b/Userland/Libraries/LibConfig/Listener.h @@ -0,0 +1,27 @@ +/* + * Copyright (c) 2021, Andreas Kling + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace Config { + +class Listener { +public: + virtual ~Listener(); + + static void for_each(Function); + + virtual void config_string_did_change(String const& domain, String const& group, String const& key, String const& value); + virtual void config_i32_did_change(String const& domain, String const& group, String const& key, i32 value); + virtual void config_bool_did_change(String const& domain, String const& group, String const& key, bool value); + +protected: + Listener(); +}; + +} diff --git a/Userland/Services/ConfigServer/ClientConnection.cpp b/Userland/Services/ConfigServer/ClientConnection.cpp index 3551a1af5b..246a95e10f 100644 --- a/Userland/Services/ConfigServer/ClientConnection.cpp +++ b/Userland/Services/ConfigServer/ClientConnection.cpp @@ -12,6 +12,25 @@ namespace ConfigServer { static HashMap> s_connections; +struct CachedDomain { + String domain; + NonnullRefPtr config; +}; + +static HashMap> s_cache; + +static Core::ConfigFile& ensure_domain_config(String const& domain) +{ + auto it = s_cache.find(domain); + if (it != s_cache.end()) + return *it->value->config; + + auto config = Core::ConfigFile::open_for_app(domain, Core::ConfigFile::AllowWriting::Yes); + auto cache_entry = make(domain, config); + s_cache.set(domain, move(cache_entry)); + return *config; +} + ClientConnection::ClientConnection(NonnullRefPtr client_socket, int client_id) : IPC::ClientConnection(*this, move(client_socket), client_id) { @@ -38,6 +57,16 @@ void ClientConnection::pledge_domains(Vector const& domains) m_pledged_domains.set(domain); } +void ClientConnection::monitor_domain(String const& domain) +{ + if (m_has_pledged && !m_pledged_domains.contains(domain)) { + did_misbehave("Attempt to monitor non-pledged domain"); + return; + } + + m_monitored_domains.set(domain); +} + bool ClientConnection::validate_access(String const& domain, String const& group, String const& key) { if (!m_has_pledged) @@ -53,10 +82,10 @@ Messages::ConfigServer::ReadStringValueResponse ClientConnection::read_string_va if (!validate_access(domain, group, key)) return nullptr; - auto config = Core::ConfigFile::open_for_app(domain); - if (!config->has_key(group, key)) + auto& config = ensure_domain_config(domain); + if (!config.has_key(group, key)) return Optional {}; - return Optional { config->read_entry(group, key) }; + return Optional { config.read_entry(group, key) }; } Messages::ConfigServer::ReadI32ValueResponse ClientConnection::read_i32_value(String const& domain, String const& group, String const& key) @@ -64,10 +93,10 @@ Messages::ConfigServer::ReadI32ValueResponse ClientConnection::read_i32_value(St if (!validate_access(domain, group, key)) return nullptr; - auto config = Core::ConfigFile::open_for_app(domain); - if (!config->has_key(group, key)) + auto& config = ensure_domain_config(domain); + if (!config.has_key(group, key)) return Optional {}; - return Optional { config->read_num_entry(group, key) }; + return Optional { config.read_num_entry(group, key) }; } Messages::ConfigServer::ReadBoolValueResponse ClientConnection::read_bool_value(String const& domain, String const& group, String const& key) @@ -75,10 +104,10 @@ Messages::ConfigServer::ReadBoolValueResponse ClientConnection::read_bool_value( if (!validate_access(domain, group, key)) return nullptr; - auto config = Core::ConfigFile::open_for_app(domain); - if (!config->has_key(group, key)) + auto& config = ensure_domain_config(domain); + if (!config.has_key(group, key)) return Optional {}; - return Optional { config->read_bool_entry(group, key) }; + return Optional { config.read_bool_entry(group, key) }; } void ClientConnection::write_string_value(String const& domain, String const& group, String const& key, String const& value) @@ -86,8 +115,17 @@ void ClientConnection::write_string_value(String const& domain, String const& gr if (!validate_access(domain, group, key)) return; - auto config = Core::ConfigFile::open_for_app(domain, Core::ConfigFile::AllowWriting::Yes); - config->write_entry(group, key, value); + auto& config = ensure_domain_config(domain); + + if (config.has_key(group, key) && config.read_entry(group, key) == value) + return; + + config.write_entry(group, key, value); + + for (auto& it : s_connections) { + if (it.value != this && it.value->m_monitored_domains.contains(domain)) + it.value->async_notify_changed_string_value(domain, group, key, value); + } } void ClientConnection::write_i32_value(String const& domain, String const& group, String const& key, i32 value) @@ -95,8 +133,17 @@ void ClientConnection::write_i32_value(String const& domain, String const& group if (!validate_access(domain, group, key)) return; - auto config = Core::ConfigFile::open_for_app(domain, Core::ConfigFile::AllowWriting::Yes); - config->write_num_entry(group, key, value); + auto& config = ensure_domain_config(domain); + + if (config.has_key(group, key) && config.read_num_entry(group, key) == value) + return; + + config.write_num_entry(group, key, value); + + for (auto& it : s_connections) { + if (it.value != this && it.value->m_monitored_domains.contains(domain)) + it.value->async_notify_changed_i32_value(domain, group, key, value); + } } void ClientConnection::write_bool_value(String const& domain, String const& group, String const& key, bool value) @@ -104,8 +151,17 @@ void ClientConnection::write_bool_value(String const& domain, String const& grou if (!validate_access(domain, group, key)) return; - auto config = Core::ConfigFile::open_for_app(domain, Core::ConfigFile::AllowWriting::Yes); - config->write_bool_entry(group, key, value); + auto& config = ensure_domain_config(domain); + + if (config.has_key(group, key) && config.read_bool_entry(group, key) == value) + return; + + config.write_bool_entry(group, key, value); + + for (auto& it : s_connections) { + if (it.value != this && it.value->m_monitored_domains.contains(domain)) + it.value->async_notify_changed_bool_value(domain, group, key, value); + } } } diff --git a/Userland/Services/ConfigServer/ClientConnection.h b/Userland/Services/ConfigServer/ClientConnection.h index cdbc53ab64..55bd3ebb9c 100644 --- a/Userland/Services/ConfigServer/ClientConnection.h +++ b/Userland/Services/ConfigServer/ClientConnection.h @@ -24,6 +24,7 @@ private: explicit ClientConnection(NonnullRefPtr, int client_id); virtual void pledge_domains(Vector const&) override; + virtual void monitor_domain(String const&) override; virtual Messages::ConfigServer::ReadStringValueResponse read_string_value([[maybe_unused]] String const& domain, [[maybe_unused]] String const& group, [[maybe_unused]] String const& key) override; virtual Messages::ConfigServer::ReadI32ValueResponse read_i32_value([[maybe_unused]] String const& domain, [[maybe_unused]] String const& group, [[maybe_unused]] String const& key) override; virtual Messages::ConfigServer::ReadBoolValueResponse read_bool_value([[maybe_unused]] String const& domain, [[maybe_unused]] String const& group, [[maybe_unused]] String const& key) override; @@ -35,6 +36,8 @@ private: bool m_has_pledged { false }; HashTable m_pledged_domains; + + HashTable m_monitored_domains; }; } diff --git a/Userland/Services/ConfigServer/ConfigClient.ipc b/Userland/Services/ConfigServer/ConfigClient.ipc index bd0318b3ad..d8436b671d 100644 --- a/Userland/Services/ConfigServer/ConfigClient.ipc +++ b/Userland/Services/ConfigServer/ConfigClient.ipc @@ -1,3 +1,6 @@ endpoint ConfigClient { + notify_changed_string_value(String domain, String group, String key, String value) =| + notify_changed_i32_value(String domain, String group, String key, i32 value) =| + notify_changed_bool_value(String domain, String group, String key, bool value) =| } diff --git a/Userland/Services/ConfigServer/ConfigServer.ipc b/Userland/Services/ConfigServer/ConfigServer.ipc index 9ce1bbc70a..055e85241f 100644 --- a/Userland/Services/ConfigServer/ConfigServer.ipc +++ b/Userland/Services/ConfigServer/ConfigServer.ipc @@ -2,6 +2,8 @@ endpoint ConfigServer { pledge_domains(Vector domains) =| + monitor_domain(String domain) =| + read_string_value(String domain, String group, String key) => (Optional value) read_i32_value(String domain, String group, String key) => (Optional value) read_bool_value(String domain, String group, String key) => (Optional value)