diff --git a/src/config_manager.py b/src/config_manager.py index 8a426f4b..e4c5cc4b 100644 --- a/src/config_manager.py +++ b/src/config_manager.py @@ -56,6 +56,13 @@ class ConfigManager: self.secrets_path: str = secrets_path or "config/config_secrets.json" self.template_path: str = "config/config.template.json" self.config: Dict[str, Any] = {} + # (mtime_ns, size) signature of (config, secrets, template) at the + # last successful load. load_config() skips the full re-read (3 file + # parses + recursive template migration) when nothing changed — + # ~30 web request handlers call it, some 2-3x per request. Cross- + # process freshness is preserved: another process's save bumps the + # mtime, so the next load here re-reads. + self._loaded_sig: Optional[tuple] = None self.logger: logging.Logger = get_logger(__name__) # Initialize atomic config manager @@ -122,6 +129,14 @@ class ConfigManager: # Update in-memory config if save was successful if result.status == SaveResultStatus.SUCCESS: self.config = new_config_data + # In-memory config now matches what was just written; refresh + # the load signature so the fast path stays valid. NOTE: the + # in-memory copy includes merged secrets; the on-disk file has + # them stripped — the fast path returning self.config preserves + # exactly the pre-cache behavior (load-after-save also returned + # the secret-merged self.config only after re-reading secrets; + # here secrets file is unchanged, so contents are equivalent). + self._loaded_sig = self._files_signature() self.logger.info(f"Configuration successfully saved atomically to {os.path.abspath(self.config_path)}") elif result.status == SaveResultStatus.ROLLED_BACK: # Reload config from file after rollback @@ -179,13 +194,36 @@ class ConfigManager: atomic_mgr = self._get_atomic_manager() return atomic_mgr.validate_config_file(config_path) + def _files_signature(self) -> tuple: + """(mtime_ns, size) of config/secrets/template, None for missing — + cheap staleness probe (3 stats) for the load_config fast path.""" + sig = [] + for path in (self.config_path, self.secrets_path, self.template_path): + try: + st = os.stat(path) + sig.append((st.st_mtime_ns, st.st_size)) + except OSError: + sig.append(None) + return tuple(sig) + def load_config(self) -> Dict[str, Any]: - """Load configuration from JSON files.""" + """Load configuration from JSON files. + + Fast path: when config.json, config_secrets.json and the template + are all unchanged since the last successful load (mtime_ns + size), + the already-parsed self.config is returned without touching the + files — same aliasing semantics as the full path, which also + returns self.config. + """ try: + current_sig = self._files_signature() + if self.config and self._loaded_sig == current_sig: + return self.config + # Check if config file exists, if not create from template if not os.path.exists(self.config_path): self._create_config_from_template() - + # Load main config self.logger.info(f"Attempting to load config from: {os.path.abspath(self.config_path)}") with open(self.config_path, 'r') as f: @@ -205,7 +243,10 @@ class ConfigManager: self.logger.warning(f"Secrets file not readable ({self.secrets_path}): {e}. Continuing without secrets.") except (json.JSONDecodeError, OSError) as e: self.logger.warning(f"Error reading secrets file ({self.secrets_path}): {e}. Continuing without secrets.") - + + # Signature taken AFTER load + migration (migration may write the + # config back), so it reflects exactly what was read/written. + self._loaded_sig = self._files_signature() return self.config except FileNotFoundError as e: @@ -264,7 +305,8 @@ class ConfigManager: json.dump(config_to_write, f, indent=4) # Update the in-memory config to the new state (which includes secrets for runtime) - self.config = new_config_data + self.config = new_config_data + self._loaded_sig = self._files_signature() self.logger.info(f"Configuration successfully saved to {os.path.abspath(self.config_path)}") if secrets_content: self.logger.info("Secret values were preserved in memory and not written to the main config file.") diff --git a/test/test_config_load_cache.py b/test/test_config_load_cache.py new file mode 100644 index 00000000..e0d634fb --- /dev/null +++ b/test/test_config_load_cache.py @@ -0,0 +1,129 @@ +"""Tests for load_config's mtime fast path (src/config_manager.py). + +load_config used to re-read + re-parse config.json, the template (with a +recursive migration diff) and secrets on EVERY call — ~30 web request +handlers call it, some 2-3x per request. The fast path skips all of it +when the three files' (mtime_ns, size) signatures are unchanged. + +The invariant that matters most: cross-process freshness — a save from +the web process must be picked up by the display process's next load. +That's guaranteed because the signature is re-stat'd on every call. +""" + +import json +import os +import sys + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from src.config_manager import ConfigManager # noqa: E402 + + +@pytest.fixture +def mgr(tmp_path): + config = tmp_path / "config.json" + secrets = tmp_path / "secrets.json" + template = tmp_path / "template.json" + config.write_text(json.dumps({"display": {"brightness": 90}, "timezone": "UTC"})) + secrets.write_text(json.dumps({"weather": {"api_key": "sek"}})) + template.write_text(json.dumps({"display": {"brightness": 90}, "timezone": "UTC"})) + m = ConfigManager(config_path=str(config), secrets_path=str(secrets)) + m.template_path = str(template) + return m, config, secrets, template + + +def _count_opens(monkeypatch, mgr_paths): + """Count open() calls hitting the config files.""" + counts = {"n": 0} + real_open = open + + def counting_open(file, *args, **kwargs): + if str(file) in mgr_paths: + counts["n"] += 1 + return real_open(file, *args, **kwargs) + + import builtins + monkeypatch.setattr(builtins, "open", counting_open) + return counts + + +class TestFastPath: + def test_unchanged_files_are_not_reread(self, mgr, monkeypatch): + m, config, secrets, template = mgr + first = m.load_config() + assert first["weather"]["api_key"] == "sek" # secrets merged + counts = _count_opens(monkeypatch, {str(config), str(secrets), str(template)}) + for _ in range(10): + again = m.load_config() + assert counts["n"] == 0, "fast path must not re-open any config file" + assert again is first # same aliasing semantics as the full path + + def test_config_change_triggers_reload(self, mgr): + m, config, secrets, template = mgr + m.load_config() + data = json.loads(config.read_text()) + data["display"]["brightness"] = 55 + config.write_text(json.dumps(data)) + os.utime(config, (os.stat(config).st_atime, os.stat(config).st_mtime + 2)) + assert m.load_config()["display"]["brightness"] == 55 + + def test_secrets_change_triggers_reload(self, mgr): + m, config, secrets, template = mgr + m.load_config() + secrets.write_text(json.dumps({"weather": {"api_key": "NEW"}})) + os.utime(secrets, (os.stat(secrets).st_atime, os.stat(secrets).st_mtime + 2)) + assert m.load_config()["weather"]["api_key"] == "NEW" + + def test_template_change_triggers_reload_and_migration(self, mgr): + m, config, secrets, template = mgr + m.load_config() + template.write_text(json.dumps({ + "display": {"brightness": 90}, "timezone": "UTC", + "brand_new_key": {"added": True}})) + os.utime(template, (os.stat(template).st_atime, os.stat(template).st_mtime + 2)) + reloaded = m.load_config() + assert reloaded.get("brand_new_key") == {"added": True} + + def test_same_second_edit_detected_via_mtime_ns_or_size(self, mgr): + """Coarse-mtime same-second edits: size difference still busts it.""" + m, config, secrets, template = mgr + m.load_config() + st = os.stat(config) + data = json.loads(config.read_text()) + data["timezone"] = "America/New_York" # different byte length + config.write_text(json.dumps(data)) + os.utime(config, (st.st_atime, st.st_mtime)) # force same mtime + assert m.load_config()["timezone"] == "America/New_York" + + +class TestSaveCoherence: + def test_save_config_then_load_returns_saved_data(self, mgr, monkeypatch): + m, config, secrets, template = mgr + m.load_config() + new = {"display": {"brightness": 42}, "timezone": "UTC", + "weather": {"api_key": "sek"}} + m.save_config(new) + counts = _count_opens(monkeypatch, {str(config), str(secrets), str(template)}) + loaded = m.load_config() + assert loaded["display"]["brightness"] == 42 + assert loaded["weather"]["api_key"] == "sek" # secrets survive in memory + assert counts["n"] == 0 # signature refreshed by save; no re-read + + def test_cross_process_save_is_picked_up(self, mgr): + """Another process writing config.json (different mtime) must bust + this process's fast path — the core cross-process guarantee.""" + m, config, secrets, template = mgr + m.load_config() + other = ConfigManager(config_path=str(config), secrets_path=str(secrets)) + other.template_path = str(template) + other.load_config() + other.save_config({"display": {"brightness": 11}, "timezone": "UTC", + "weather": {"api_key": "sek"}}) + os.utime(config, (os.stat(config).st_atime, os.stat(config).st_mtime + 2)) + assert m.load_config()["display"]["brightness"] == 11 + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-v"]))