mirror of
https://github.com/jhd3197/ServerKit.git
synced 2026-11-09 05:24:49 +09:00
refactor(services): one ConfigStore for the JSON-file config mechanics
Five services shipped byte-identical save_config copies and as many the same get_config skeleton — the next fix (an atomic write, a better error) would have had to land in every one. load_json_config / save_json_config own the mechanics; the path, the default, and any behaviour around the write stay per-service. Byte-compatible with the copies: plain open(..., 'w'), no locking, no atomic rename — adding those here would be a behaviour change smuggled in with a consolidation; if they're ever wanted they now land once. Divergent services (template cache invalidation, storage secret merging, email_relay's DB backing, build_service's leaner return dict) keep their own methods. monitoring_service converts separately — it is mid-edit on another branch of this plan.
This commit is contained in:
1 parent
d5dd6a5dd2
commit
a3fe9bf59d
7 files changed
+141
-71
No files matched your search
@@ -15,6 +15,7 @@ import schedule
|
||||
|
||||
from app import paths
|
||||
from app.utils import backup_crypto
|
||||
from app.utils.config_store import load_json_config, save_json_config
|
||||
from app.utils.formatting import format_bytes
|
||||
from app.utils.system import is_command_available
|
||||
from app.services.telemetry_service import TelemetryService, generate_correlation_id
|
||||
@@ -53,14 +54,7 @@ class BackupService:
|
||||
@classmethod
|
||||
def get_config(cls) -> Dict:
|
||||
"""Get backup configuration."""
|
||||
if os.path.exists(cls.BACKUP_CONFIG):
|
||||
try:
|
||||
with open(cls.BACKUP_CONFIG, 'r') as f:
|
||||
return json.load(f)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return {
|
||||
return load_json_config(cls.BACKUP_CONFIG, {
|
||||
'enabled': False,
|
||||
'retention_days': 30,
|
||||
'encrypt_backups': False,
|
||||
@@ -70,18 +64,12 @@ class BackupService:
|
||||
'on_failure': True,
|
||||
'email': ''
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
@classmethod
|
||||
def save_config(cls, config: Dict) -> Dict:
|
||||
"""Save backup configuration."""
|
||||
try:
|
||||
os.makedirs(cls.CONFIG_DIR, exist_ok=True)
|
||||
with open(cls.BACKUP_CONFIG, 'w') as f:
|
||||
json.dump(config, f, indent=2)
|
||||
return {'success': True, 'message': 'Configuration saved'}
|
||||
except Exception as e:
|
||||
return {'success': False, 'error': str(e)}
|
||||
return save_json_config(cls.BACKUP_CONFIG, config)
|
||||
|
||||
@classmethod
|
||||
def backup_application(cls, app_name: str, app_path: str,
|
||||
|
||||
@@ -22,6 +22,7 @@ from typing import Dict, List, Optional, Callable, Generator
|
||||
from pathlib import Path
|
||||
|
||||
from app import paths
|
||||
from app.utils.config_store import load_json_config
|
||||
|
||||
|
||||
class BuildService:
|
||||
@@ -68,13 +69,7 @@ class BuildService:
|
||||
@classmethod
|
||||
def get_config(cls) -> Dict:
|
||||
"""Get build configuration."""
|
||||
if os.path.exists(cls.BUILD_CONFIG):
|
||||
try:
|
||||
with open(cls.BUILD_CONFIG, 'r') as f:
|
||||
return json.load(f)
|
||||
except Exception:
|
||||
pass
|
||||
return {'apps': {}}
|
||||
return load_json_config(cls.BUILD_CONFIG, {'apps': {}})
|
||||
|
||||
@classmethod
|
||||
def save_config(cls, config: Dict) -> Dict:
|
||||
|
||||
@@ -9,6 +9,7 @@ from typing import Dict, List, Optional
|
||||
from pathlib import Path
|
||||
|
||||
from app import paths
|
||||
from app.utils.config_store import load_json_config, save_json_config
|
||||
from app.utils.git_security import (
|
||||
git_argv,
|
||||
git_env,
|
||||
@@ -28,28 +29,15 @@ class GitService:
|
||||
@classmethod
|
||||
def get_config(cls) -> Dict:
|
||||
"""Get deployment configuration."""
|
||||
if os.path.exists(cls.DEPLOY_CONFIG):
|
||||
try:
|
||||
with open(cls.DEPLOY_CONFIG, 'r') as f:
|
||||
return json.load(f)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return {
|
||||
return load_json_config(cls.DEPLOY_CONFIG, {
|
||||
'apps': {}, # app_id -> deployment config
|
||||
'webhook_secret': cls._generate_secret()
|
||||
}
|
||||
})
|
||||
|
||||
@classmethod
|
||||
def save_config(cls, config: Dict) -> Dict:
|
||||
"""Save deployment configuration."""
|
||||
try:
|
||||
os.makedirs(cls.CONFIG_DIR, exist_ok=True)
|
||||
with open(cls.DEPLOY_CONFIG, 'w') as f:
|
||||
json.dump(config, f, indent=2)
|
||||
return {'success': True, 'message': 'Configuration saved'}
|
||||
except Exception as e:
|
||||
return {'success': False, 'error': str(e)}
|
||||
return save_json_config(cls.DEPLOY_CONFIG, config)
|
||||
|
||||
@staticmethod
|
||||
def _generate_secret() -> str:
|
||||
|
||||
@@ -20,6 +20,7 @@ from typing import Dict, List, Optional
|
||||
from pathlib import Path
|
||||
|
||||
from app import paths
|
||||
from app.utils.config_store import load_json_config, save_json_config
|
||||
|
||||
|
||||
class NotificationService:
|
||||
@@ -40,14 +41,7 @@ class NotificationService:
|
||||
@classmethod
|
||||
def get_config(cls) -> Dict:
|
||||
"""Get notification configuration."""
|
||||
if os.path.exists(cls.NOTIFICATIONS_CONFIG):
|
||||
try:
|
||||
with open(cls.NOTIFICATIONS_CONFIG, 'r') as f:
|
||||
return json.load(f)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return {
|
||||
return load_json_config(cls.NOTIFICATIONS_CONFIG, {
|
||||
'discord': {
|
||||
'enabled': False,
|
||||
'webhook_url': '',
|
||||
@@ -87,18 +81,12 @@ class NotificationService:
|
||||
'headers': {},
|
||||
'notify_on': ['critical', 'warning']
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
@classmethod
|
||||
def save_config(cls, config: Dict) -> Dict:
|
||||
"""Save notification configuration."""
|
||||
try:
|
||||
os.makedirs(cls.CONFIG_DIR, exist_ok=True)
|
||||
with open(cls.NOTIFICATIONS_CONFIG, 'w') as f:
|
||||
json.dump(config, f, indent=2)
|
||||
return {'success': True, 'message': 'Configuration saved'}
|
||||
except Exception as e:
|
||||
return {'success': False, 'error': str(e)}
|
||||
return save_json_config(cls.NOTIFICATIONS_CONFIG, config)
|
||||
|
||||
@classmethod
|
||||
def update_channel_config(cls, channel: str, settings: Dict) -> Dict:
|
||||
|
||||
@@ -20,6 +20,7 @@ from pathlib import Path
|
||||
|
||||
from .notification_service import NotificationService
|
||||
from app import paths
|
||||
from app.utils.config_store import load_json_config, save_json_config
|
||||
from app.utils.system import (
|
||||
PackageManager,
|
||||
ServiceControl,
|
||||
@@ -44,14 +45,7 @@ class SecurityService:
|
||||
@classmethod
|
||||
def get_config(cls) -> Dict:
|
||||
"""Get security configuration."""
|
||||
if os.path.exists(cls.SECURITY_CONFIG):
|
||||
try:
|
||||
with open(cls.SECURITY_CONFIG, 'r') as f:
|
||||
return json.load(f)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
return {
|
||||
return load_json_config(cls.SECURITY_CONFIG, {
|
||||
'clamav': {
|
||||
'enabled': True,
|
||||
'scan_paths': ['/var/www', '/home'],
|
||||
@@ -84,18 +78,12 @@ class SecurityService:
|
||||
'on_suspicious_activity': True,
|
||||
'severity': 'critical'
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
@classmethod
|
||||
def save_config(cls, config: Dict) -> Dict:
|
||||
"""Save security configuration."""
|
||||
try:
|
||||
os.makedirs(cls.CONFIG_DIR, exist_ok=True)
|
||||
with open(cls.SECURITY_CONFIG, 'w') as f:
|
||||
json.dump(config, f, indent=2)
|
||||
return {'success': True, 'message': 'Configuration saved'}
|
||||
except Exception as e:
|
||||
return {'success': False, 'error': str(e)}
|
||||
return save_json_config(cls.SECURITY_CONFIG, config)
|
||||
|
||||
# ==========================================
|
||||
# CLAMAV INTEGRATION
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
"""JSON-file config persistence — one door for the load-or-default /
|
||||
dump-or-error mechanics (plan 75 §F4).
|
||||
|
||||
Five services shipped byte-identical ``save_config`` copies and seven more the
|
||||
same ``get_config`` skeleton, each a place a fix (an atomic write, a better
|
||||
error) would have had to land N times. These two helpers own the mechanics;
|
||||
the per-service parts stay per-service: the path, the default, and any extra
|
||||
behaviour around the write (cache invalidation, secret merging — those keep
|
||||
their own methods and may call these helpers inside).
|
||||
|
||||
Deliberate parity with the copies they replace: plain ``open(..., 'w')``, no
|
||||
locking, no atomic rename — none of the originals had them, and adding them
|
||||
here would be a behaviour change smuggled in with a consolidation. If atomic
|
||||
writes are ever wanted, they land here, once.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
|
||||
|
||||
def load_json_config(path, default):
|
||||
"""JSON file → dict; *default* on missing or corrupt.
|
||||
|
||||
A corrupt config reads as the default, never raises — the same contract
|
||||
the hand-rolled copies had, and the one callers rely on at boot.
|
||||
"""
|
||||
if os.path.exists(path):
|
||||
try:
|
||||
with open(path, 'r') as f:
|
||||
return json.load(f)
|
||||
except Exception: # noqa: BLE001
|
||||
pass
|
||||
return default
|
||||
|
||||
|
||||
def save_json_config(path, config):
|
||||
"""dict → JSON file (indented), creating the parent dir. Returns the
|
||||
service-dict shape every caller already returns."""
|
||||
try:
|
||||
os.makedirs(os.path.dirname(path) or '.', exist_ok=True)
|
||||
with open(path, 'w') as f:
|
||||
json.dump(config, f, indent=2)
|
||||
return {'success': True, 'message': 'Configuration saved'}
|
||||
except Exception as e: # noqa: BLE001
|
||||
return {'success': False, 'error': str(e)}
|
||||
@@ -0,0 +1,79 @@
|
||||
"""The ConfigStore contract (plan 75 §F4).
|
||||
|
||||
Five services shipped byte-identical save_config copies; the helpers in
|
||||
``app.utils.config_store`` are now the one place those mechanics live. These
|
||||
tests pin the behaviour the copies had — byte-for-byte compatibility is the
|
||||
point of the consolidation: indented JSON, parent dir created, corrupt file
|
||||
reads as the default, and the service-dict return shape.
|
||||
"""
|
||||
import json
|
||||
import os
|
||||
|
||||
from app.utils.config_store import load_json_config, save_json_config
|
||||
|
||||
|
||||
def test_missing_file_reads_as_the_default(tmp_path):
|
||||
default = {'enabled': False}
|
||||
assert load_json_config(str(tmp_path / 'cfg.json'), default) == default
|
||||
|
||||
|
||||
def test_the_default_is_not_shared_between_calls(tmp_path):
|
||||
"""Callers mutate what they get back; a shared default would leak."""
|
||||
default = {'items': []}
|
||||
first = load_json_config(str(tmp_path / 'cfg.json'), default)
|
||||
first['items'].append('x')
|
||||
assert load_json_config(str(tmp_path / 'cfg.json'), default) == default
|
||||
|
||||
|
||||
def test_corrupt_file_reads_as_the_default_never_raises(tmp_path):
|
||||
path = tmp_path / 'cfg.json'
|
||||
path.write_text('{"truncated":')
|
||||
assert load_json_config(str(path), {'a': 1}) == {'a': 1}
|
||||
|
||||
|
||||
def test_save_creates_the_parent_dir_and_writes_indented_json(tmp_path):
|
||||
path = tmp_path / 'nested' / 'cfg.json'
|
||||
result = save_json_config(str(path), {'b': 2})
|
||||
|
||||
assert result == {'success': True, 'message': 'Configuration saved'}
|
||||
assert json.loads(path.read_text()) == {'b': 2}
|
||||
assert path.read_text() == json.dumps({'b': 2}, indent=2)
|
||||
|
||||
|
||||
def test_save_failure_returns_the_error_shape(tmp_path):
|
||||
blocked = tmp_path / 'a-file'
|
||||
blocked.write_text('x')
|
||||
result = save_json_config(str(blocked / 'cfg.json'), {'b': 2})
|
||||
|
||||
assert result['success'] is False
|
||||
assert result['error']
|
||||
|
||||
|
||||
def test_round_trip(tmp_path):
|
||||
path = str(tmp_path / 'cfg.json')
|
||||
save_json_config(path, {'retention_days': 30})
|
||||
assert load_json_config(path, {}) == {'retention_days': 30}
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# The converted services actually go through the one door
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
||||
def test_converted_services_use_the_store(tmp_path, monkeypatch):
|
||||
"""Spot-check two of the converted services end to end: their get/save
|
||||
config must round-trip through the store with unchanged semantics."""
|
||||
from app.services.backup_service import BackupService
|
||||
from app.services.git_service import GitService
|
||||
|
||||
backup_path = str(tmp_path / 'backup.json')
|
||||
monkeypatch.setattr(BackupService, 'BACKUP_CONFIG', backup_path)
|
||||
assert BackupService.get_config()['retention_days'] == 30
|
||||
assert BackupService.save_config({'retention_days': 7})['success'] is True
|
||||
assert BackupService.get_config() == {'retention_days': 7}
|
||||
|
||||
git_path = str(tmp_path / 'deployments.json')
|
||||
monkeypatch.setattr(GitService, 'DEPLOY_CONFIG', git_path)
|
||||
default = GitService.get_config()
|
||||
assert 'webhook_secret' in default and default['apps'] == {}
|
||||
assert GitService.save_config({'apps': {'1': {}}})['success'] is True
|
||||
assert GitService.get_config() == {'apps': {'1': {}}}
|
||||
Reference in new issue
Block a user