diff --git a/src/accessiweather/app.py b/src/accessiweather/app.py index 4a2b4168a..df7069d28 100644 --- a/src/accessiweather/app.py +++ b/src/accessiweather/app.py @@ -20,7 +20,7 @@ from . import app_timer_manager from .models import WeatherData -from .paths import Paths +from .paths import Paths, RuntimeStoragePaths, resolve_runtime_storage from .single_instance import SingleInstanceManager if TYPE_CHECKING: @@ -103,6 +103,11 @@ def __init__( # Set up paths (similar to Toga's paths API) self.paths = Paths() + self.runtime_paths: RuntimeStoragePaths = resolve_runtime_storage( + self.paths, + config_dir=self._config_dir, + portable_mode=self._portable_mode, + ) # Core components (initialized in OnInit) self.config_manager: ConfigManager | None = None @@ -162,7 +167,9 @@ def OnInit(self) -> bool: try: # Check for single instance - self.single_instance_manager = SingleInstanceManager(self) + self.single_instance_manager = SingleInstanceManager( + self, runtime_paths=self.runtime_paths + ) if not self.single_instance_manager.try_acquire_lock(): if self._updated: # After an update restart the old lock file is stale; force-acquire it diff --git a/src/accessiweather/app_initialization.py b/src/accessiweather/app_initialization.py index 6fb5921e8..8ef941a17 100644 --- a/src/accessiweather/app_initialization.py +++ b/src/accessiweather/app_initialization.py @@ -3,7 +3,6 @@ from __future__ import annotations import logging -from pathlib import Path from typing import TYPE_CHECKING import wx @@ -23,12 +22,13 @@ def initialize_components(app: AccessiWeatherApp) -> None: app.config_manager = ConfigManager( app, + runtime_paths=app.runtime_paths, config_dir=getattr(app, "_config_dir", None), portable_mode=getattr(app, "_portable_mode", False), ) from .runtime_state import RuntimeStateManager - app.runtime_state_manager = RuntimeStateManager(app.config_manager.config_dir) + app.runtime_state_manager = RuntimeStateManager(app.runtime_paths.config_root) config = app.config_manager.load_config() # Defer update service initialization to background (using wx.CallLater) @@ -41,25 +41,10 @@ def initialize_components(app: AccessiWeatherApp) -> None: # keyring access until first use. We pass it directly to WeatherClient which # will access the value lazily when the VisualCrossing client is needed. lazy_api_key = config.settings.visual_crossing_api_key if config.settings else "" - config_dir_value = getattr(app.config_manager, "config_dir", None) - cache_root: Path | None = None - if config_dir_value is not None: - try: - cache_root = Path(config_dir_value) - except (TypeError, ValueError): # pragma: no cover - defensive logging - cache_root = None - if cache_root is None: - fallback_dir = getattr(app.paths, "config", None) - try: - cache_root = Path(fallback_dir) if fallback_dir is not None else Path.cwd() - except (TypeError, ValueError): # pragma: no cover - defensive logging - cache_root = Path.cwd() - cache_dir = cache_root / "weather_cache" - # Lazy import WeatherDataCache from .cache import WeatherDataCache - offline_cache = WeatherDataCache(cache_dir) + offline_cache = WeatherDataCache(app.runtime_paths.cache_dir) debug_enabled = bool(getattr(app, "debug_mode", False)) log_level = logging.DEBUG if debug_enabled else logging.INFO root_logger = logging.getLogger() diff --git a/src/accessiweather/config/config_manager.py b/src/accessiweather/config/config_manager.py index 83f1c283d..b56304974 100644 --- a/src/accessiweather/config/config_manager.py +++ b/src/accessiweather/config/config_manager.py @@ -20,6 +20,7 @@ if TYPE_CHECKING: from accessiweather.app import AccessiWeatherApp +from ..paths import RuntimeStoragePaths from .file_permissions import set_secure_file_permissions from .github_config import GitHubConfigOperations from .import_export import ImportExportOperations @@ -36,6 +37,7 @@ class ConfigManager: def __init__( self, app: AccessiWeatherApp, + runtime_paths: RuntimeStoragePaths | None = None, config_dir: str | Path | None = None, portable_mode: bool = False, ): @@ -44,13 +46,18 @@ def __init__( Args: app: Toga application instance + runtime_paths: Resolved canonical runtime storage layout config_dir: Custom configuration directory (overrides default) portable_mode: If True, use app directory for config instead of user directory """ self.app = app + self.runtime_paths = runtime_paths + # Determine config directory based on parameters + # Explicit config_dir and portable_mode take precedence; runtime_paths.config_root + # is the resolved canonical root and acts as the default when neither is given. if config_dir: self.config_dir = Path(config_dir) elif portable_mode: diff --git a/src/accessiweather/noaa_radio/preferences.py b/src/accessiweather/noaa_radio/preferences.py index b2697ffb8..fc05741bc 100644 --- a/src/accessiweather/noaa_radio/preferences.py +++ b/src/accessiweather/noaa_radio/preferences.py @@ -12,10 +12,16 @@ class RadioPreferences: """Stores per-station preferred stream URLs in a JSON file.""" - def __init__(self, config_dir: Path | str | None = None) -> None: - """Initialize with optional config directory for persistence.""" + def __init__( + self, + config_dir: Path | str | None = None, + path: Path | str | None = None, + ) -> None: + """Initialize with an optional canonical preferences file path.""" self._prefs: dict[str, str] = {} - if config_dir is not None: + if path is not None: + self._path = Path(path) + elif config_dir is not None: self._path = Path(config_dir) / "noaa_radio_prefs.json" else: self._path: Path | None = None diff --git a/src/accessiweather/paths.py b/src/accessiweather/paths.py index a694402b7..42188da93 100644 --- a/src/accessiweather/paths.py +++ b/src/accessiweather/paths.py @@ -9,6 +9,7 @@ import os import sys +from dataclasses import dataclass from pathlib import Path @@ -111,3 +112,57 @@ def logs(self) -> Path: path = self._get_base_path() / "Logs" path.mkdir(parents=True, exist_ok=True) return path + + +@dataclass(frozen=True) +class RuntimeStoragePaths: + """Canonical writable runtime storage layout resolved once at startup.""" + + config_root: Path + portable_mode: bool = False + custom_config_dir: bool = False + + @property + def config_file(self) -> Path: + return self.config_root / "accessiweather.json" + + @property + def state_dir(self) -> Path: + return self.config_root / "state" + + @property + def runtime_state_file(self) -> Path: + return self.state_dir / "runtime_state.json" + + @property + def cache_dir(self) -> Path: + return self.config_root / "weather_cache" + + @property + def noaa_radio_preferences_file(self) -> Path: + return self.config_root / "noaa_radio_prefs.json" + + @property + def lock_file(self) -> Path: + return self.state_dir / "accessiweather.lock" + + +def resolve_runtime_storage( + app_paths: Paths, + *, + config_dir: str | Path | None = None, + portable_mode: bool = False, +) -> RuntimeStoragePaths: + """Resolve the canonical writable runtime storage layout.""" + if config_dir is not None: + return RuntimeStoragePaths( + config_root=Path(config_dir), + portable_mode=portable_mode, + custom_config_dir=True, + ) + + if portable_mode: + app_dir = Path(sys.executable).parent if getattr(sys, "frozen", False) else Path.cwd() + return RuntimeStoragePaths(config_root=app_dir / "config", portable_mode=True) + + return RuntimeStoragePaths(config_root=Path(app_paths.config)) diff --git a/src/accessiweather/single_instance.py b/src/accessiweather/single_instance.py index 5dc5a1a51..951e18ad2 100644 --- a/src/accessiweather/single_instance.py +++ b/src/accessiweather/single_instance.py @@ -14,6 +14,8 @@ import time from pathlib import Path +from .paths import RuntimeStoragePaths + logger = logging.getLogger(__name__) # Global reference for cleanup handlers (needed for atexit and signal handlers) @@ -23,22 +25,37 @@ class SingleInstanceManager: """Manages single instance functionality using a lock file approach.""" - def __init__(self, app, lock_filename: str = "accessiweather.lock"): + def __init__( + self, + app, + lock_filename: str = "accessiweather.lock", + runtime_paths: RuntimeStoragePaths | None = None, + ): """ Initialize the single instance manager. Args: ---- - app: The application instance (must have a paths.data attribute) + app: The application instance lock_filename: Name of the lock file to create + runtime_paths: Resolved canonical runtime storage layout """ self.app = app self.lock_filename = lock_filename + self.runtime_paths = runtime_paths or getattr(app, "runtime_paths", None) self.lock_file_path: Path | None = None self._lock_acquired = False self._cleanup_registered = False + def _get_lock_file_path(self) -> Path: + """Return the canonical lock file path for the current runtime layout.""" + if self.runtime_paths is not None: + return self.runtime_paths.lock_file + + lock_dir = self.app.paths.data + return lock_dir / self.lock_filename + def try_acquire_lock(self) -> bool: """ Try to acquire the single instance lock. @@ -49,10 +66,8 @@ def try_acquire_lock(self) -> bool: """ try: - # Use app.paths.data for the lock file location (cross-platform data directory) - lock_dir = self.app.paths.data - lock_dir.mkdir(parents=True, exist_ok=True) - self.lock_file_path = lock_dir / self.lock_filename + self.lock_file_path = self._get_lock_file_path() + self.lock_file_path.parent.mkdir(parents=True, exist_ok=True) logger.info(f"Checking for existing instance using lock file: {self.lock_file_path}") @@ -299,10 +314,8 @@ def force_remove_lock(self) -> bool: """ try: if self.lock_file_path is None: - # Initialize lock file path if not already done - lock_dir = self.app.paths.data - lock_dir.mkdir(parents=True, exist_ok=True) - self.lock_file_path = lock_dir / self.lock_filename + self.lock_file_path = self._get_lock_file_path() + self.lock_file_path.parent.mkdir(parents=True, exist_ok=True) if self.lock_file_path.exists(): self.lock_file_path.unlink() diff --git a/src/accessiweather/ui/dialogs/noaa_radio_dialog.py b/src/accessiweather/ui/dialogs/noaa_radio_dialog.py index 60739ef3c..609380b78 100644 --- a/src/accessiweather/ui/dialogs/noaa_radio_dialog.py +++ b/src/accessiweather/ui/dialogs/noaa_radio_dialog.py @@ -11,7 +11,6 @@ from accessiweather.noaa_radio.player import RadioPlayer from accessiweather.noaa_radio.preferences import RadioPreferences from accessiweather.noaa_radio.wxradio_client import WxRadioClient -from accessiweather.paths import Paths if TYPE_CHECKING: pass @@ -70,7 +69,13 @@ def __init__(self, parent: wx.Window, lat: float, lon: float) -> None: on_reconnecting=self._on_reconnecting, ) self._url_provider = StreamURLProvider(use_fallback=False, wxradio_client=WxRadioClient()) - self._prefs = RadioPreferences(config_dir=Paths().data) + app = wx.GetApp() + prefs_path = ( + getattr(getattr(app, "runtime_paths", None), "noaa_radio_preferences_file", None) + if app is not None + else None + ) + self._prefs = RadioPreferences(path=prefs_path) self._current_urls: list[str] = [] self._current_url_index: int = 0 self._health_timer = wx.Timer(self) diff --git a/tests/test_noaa_radio.py b/tests/test_noaa_radio.py index b208fff3c..5767d2187 100644 --- a/tests/test_noaa_radio.py +++ b/tests/test_noaa_radio.py @@ -241,3 +241,11 @@ def test_save_creates_parent_dirs(self, tmp_path): prefs = RadioPreferences(config_dir=nested) prefs.set_preferred_url("TEST", "http://x.com") assert (nested / "noaa_radio_prefs.json").exists() + + def test_path_argument_wins_over_config_dir(self, tmp_path): + prefs_file = tmp_path / "prefs" / "custom.json" + prefs = RadioPreferences(config_dir=tmp_path / "ignored", path=prefs_file) + + prefs.set_preferred_url("TEST", "http://x.com") + + assert prefs_file.exists() diff --git a/tests/test_noaa_radio_dialog.py b/tests/test_noaa_radio_dialog.py index f6fb4cf7c..c412101b3 100644 --- a/tests/test_noaa_radio_dialog.py +++ b/tests/test_noaa_radio_dialog.py @@ -4,6 +4,7 @@ import importlib import sys +from pathlib import Path from unittest.mock import MagicMock, patch import pytest @@ -170,6 +171,25 @@ def test_dialog_inherits_from_wx_dialog(self, noaa_dialog_module): assert issubclass(noaa_dialog_module.NOAARadioDialog, wx.Dialog) + def test_dialog_uses_app_runtime_prefs_path(self, noaa_dialog_module): + fake_app = MagicMock() + fake_app.runtime_paths.noaa_radio_preferences_file = Path( + "/tmp/config/noaa_radio_prefs.json" + ) + + with ( + patch.object(noaa_dialog_module.wx, "GetApp", return_value=fake_app), + patch.object(noaa_dialog_module, "RadioPlayer"), + patch.object(noaa_dialog_module, "StreamURLProvider"), + patch.object(noaa_dialog_module, "WxRadioClient"), + patch.object(noaa_dialog_module, "RadioPreferences") as mock_prefs, + patch.object(noaa_dialog_module.NOAARadioDialog, "_init_ui"), + patch.object(noaa_dialog_module.NOAARadioDialog, "_load_stations"), + ): + noaa_dialog_module.NOAARadioDialog(MagicMock(), 40.7, -74.0) + + mock_prefs.assert_called_once_with(path=fake_app.runtime_paths.noaa_radio_preferences_file) + class TestGetSelectedStation: """Tests for station selection logic.""" diff --git a/tests/test_paths.py b/tests/test_paths.py index 0d48a94ca..b399b96d6 100644 --- a/tests/test_paths.py +++ b/tests/test_paths.py @@ -6,7 +6,7 @@ from pathlib import Path from unittest.mock import patch -from accessiweather.paths import Paths +from accessiweather.paths import Paths, RuntimeStoragePaths, resolve_runtime_storage class TestPathsInit: @@ -148,6 +148,42 @@ def test_frozen_with_force_portable_uses_exe_dir(self, tmp_path): result = p._get_base_path() assert result == tmp_path + +class TestRuntimeStorageResolution: + """Test canonical runtime storage resolution.""" + + def test_default_layout_uses_app_config_root(self, tmp_path): + paths = Paths() + paths._base_path = tmp_path + + runtime = resolve_runtime_storage(paths) + + assert runtime == RuntimeStoragePaths(config_root=tmp_path / "Config") + assert runtime.config_file == tmp_path / "Config" / "accessiweather.json" + assert runtime.runtime_state_file == tmp_path / "Config" / "state" / "runtime_state.json" + assert runtime.cache_dir == tmp_path / "Config" / "weather_cache" + assert runtime.noaa_radio_preferences_file == tmp_path / "Config" / "noaa_radio_prefs.json" + assert runtime.lock_file == tmp_path / "Config" / "state" / "accessiweather.lock" + + def test_custom_config_dir_wins(self, tmp_path): + paths = Paths() + custom = tmp_path / "custom-root" + + runtime = resolve_runtime_storage(paths, config_dir=custom) + + assert runtime.config_root == custom + assert runtime.custom_config_dir is True + assert runtime.portable_mode is False + + def test_portable_layout_uses_cwd_config_when_not_frozen(self, tmp_path): + paths = Paths() + + with patch("accessiweather.paths.Path.cwd", return_value=tmp_path): + runtime = resolve_runtime_storage(paths, portable_mode=True) + + assert runtime.config_root == tmp_path / "config" + assert runtime.portable_mode is True + def test_frozen_with_portable_marker_uses_exe_dir(self, tmp_path): exe = tmp_path / "AccessiWeather.exe" exe.write_text("x") diff --git a/tests/test_runtime_storage_integration.py b/tests/test_runtime_storage_integration.py new file mode 100644 index 000000000..f6446887f --- /dev/null +++ b/tests/test_runtime_storage_integration.py @@ -0,0 +1,49 @@ +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from accessiweather.app_initialization import initialize_components +from accessiweather.paths import RuntimeStoragePaths + + +def test_initialize_components_uses_runtime_cache_dir(tmp_path): + runtime_paths = RuntimeStoragePaths(config_root=tmp_path / "config") + settings = SimpleNamespace( + data_source="auto", + visual_crossing_api_key="", + to_alert_settings=lambda: object(), + sound_enabled=True, + sound_pack="default", + muted_sound_events=["data_updated"], + ai_cache_ttl=300, + weather_history_enabled=False, + ) + config = SimpleNamespace(settings=settings) + config_manager = MagicMock() + config_manager.config_dir = runtime_paths.config_root + config_manager.load_config.return_value = config + config_manager.get_config.return_value = config + + app = SimpleNamespace( + runtime_paths=runtime_paths, + _config_dir=None, + _portable_mode=False, + debug_mode=False, + ) + + with ( + patch("accessiweather.app_initialization.ConfigManager", return_value=config_manager), + patch("accessiweather.app_initialization.wx.CallLater", create=True), + patch("accessiweather.cache.WeatherDataCache") as mock_weather_cache, + patch("accessiweather.weather_client.WeatherClient"), + patch("accessiweather.location_manager.LocationManager"), + patch("accessiweather.display.WeatherPresenter"), + patch("accessiweather.notifications.toast_notifier.SafeDesktopNotifier"), + patch("accessiweather.cache.Cache"), + patch("accessiweather.alert_manager.AlertManager"), + patch("accessiweather.alert_notification_system.AlertNotificationSystem"), + ): + initialize_components(app) + + mock_weather_cache.assert_called_once_with(runtime_paths.cache_dir) diff --git a/tests/test_single_instance.py b/tests/test_single_instance.py new file mode 100644 index 000000000..2ec42c06c --- /dev/null +++ b/tests/test_single_instance.py @@ -0,0 +1,31 @@ +from __future__ import annotations + +from types import SimpleNamespace + +from accessiweather.paths import RuntimeStoragePaths +from accessiweather.single_instance import SingleInstanceManager + + +def test_lock_file_uses_runtime_storage_root(tmp_path): + runtime_paths = RuntimeStoragePaths(config_root=tmp_path / "config") + app = SimpleNamespace(runtime_paths=runtime_paths, formal_name="AccessiWeather") + manager = SingleInstanceManager(app, runtime_paths=runtime_paths) + + assert manager.try_acquire_lock() is True + assert manager.lock_file_path == tmp_path / "config" / "state" / "accessiweather.lock" + assert manager.lock_file_path.exists() + + manager.release_lock() + assert not manager.lock_file_path.exists() + + +def test_force_remove_lock_uses_runtime_storage_root(tmp_path): + runtime_paths = RuntimeStoragePaths(config_root=tmp_path / "config") + app = SimpleNamespace(runtime_paths=runtime_paths, formal_name="AccessiWeather") + manager = SingleInstanceManager(app, runtime_paths=runtime_paths) + + runtime_paths.lock_file.parent.mkdir(parents=True, exist_ok=True) + runtime_paths.lock_file.write_text("stale", encoding="utf-8") + + assert manager.force_remove_lock() is True + assert not runtime_paths.lock_file.exists()