From 71ba568df167ec43d6d3b3b9044c2ca8db8fc17f Mon Sep 17 00:00:00 2001 From: DesvoSoft Date: Thu, 15 Oct 2026 15:18:39 -0600 Subject: [PATCH 1/6] feat(api): UI-agnostic API seam over the core Add app/api (AppApi with servers, console, tunnel, players, backups, mods, java, settings and storage areas), event subscriptions through a caller-supplied dispatcher and task handles with progress and cancellation. Mod/plugin file operations and icon downloads move out of the Modrinth browser into app/services/mod_content.py and mod_icons.py, with a guard that refuses deletes outside the server's mods/plugins folders. The main window and the Modrinth browser now call the API. --- app/api/__init__.py | 16 + app/api/app_api.py | 86 ++++++ app/api/areas.py | 109 +++++++ app/api/events.py | 90 ++++++ app/api/mods.py | 101 ++++++ app/api/servers.py | 157 ++++++++++ app/api/storage.py | 64 ++++ app/api/tasks.py | 117 +++++++ app/services/mod_content.py | 195 ++++++++++++ app/services/mod_icons.py | 62 ++++ app/services/server_paths.py | 15 + app/ui/app_settings.py | 11 +- app/ui/main.py | 299 +++++++----------- app/ui/modrinth_browser.py | 249 ++++----------- app/ui/server_properties_editor.py | 15 +- tests/test_api_events_tasks.py | 174 +++++++++++ tests/test_api_facade.py | 478 +++++++++++++++++++++++++++++ tests/test_mod_content.py | 233 ++++++++++++++ tests/test_modrinth_browser.py | 5 +- 19 files changed, 2086 insertions(+), 390 deletions(-) create mode 100644 app/api/__init__.py create mode 100644 app/api/app_api.py create mode 100644 app/api/areas.py create mode 100644 app/api/events.py create mode 100644 app/api/mods.py create mode 100644 app/api/servers.py create mode 100644 app/api/storage.py create mode 100644 app/api/tasks.py create mode 100644 app/services/mod_content.py create mode 100644 app/services/mod_icons.py create mode 100644 app/services/server_paths.py create mode 100644 tests/test_api_events_tasks.py create mode 100644 tests/test_api_facade.py create mode 100644 tests/test_mod_content.py diff --git a/app/api/__init__.py b/app/api/__init__.py new file mode 100644 index 0000000..59b8109 --- /dev/null +++ b/app/api/__init__.py @@ -0,0 +1,16 @@ +"""UI-agnostic API over the core (docs/ARCHITECTURE.md, "API seam"). + +UIs import from here, never from app.core / app.services directly for +actions or state. This package never imports a GUI toolkit. +""" + +from app.api.app_api import AppApi +from app.api.events import Dispatcher, EventHub, Subscription, direct +from app.api.servers import MemorySample, NoServerJarError, ServerInfo, ServerRunningError +from app.api.tasks import Task +from app.core.server_events import ServerEvent + +__all__ = [ + "AppApi", "Dispatcher", "EventHub", "MemorySample", "NoServerJarError", "ServerEvent", + "ServerInfo", "ServerRunningError", "Subscription", "Task", "direct", +] diff --git a/app/api/app_api.py b/app/api/app_api.py new file mode 100644 index 0000000..66cad88 --- /dev/null +++ b/app/api/app_api.py @@ -0,0 +1,86 @@ +"""AppApi: the one object a UI talks to. + +Inputs and outputs are plain Python (str, dict, dataclasses, PIL images); +nothing here imports a GUI toolkit. A UI creates one AppApi, sets the event +dispatcher for its main loop, and calls the areas below. +""" + +import concurrent.futures +import logging +from typing import Optional + +from app.api.areas import BackupsApi, ConsoleApi, JavaApi, SettingsApi, TunnelApi +from app.api.events import Dispatcher, EventHub, direct +from app.api.mods import ModsApi +from app.api.servers import ServersApi +from app.api.storage import StorageApi +from app.api.tasks import TaskRunner +from app.core.server_events import EventBus + +logger = logging.getLogger(__name__) + + +class AppApi: + def __init__(self, bus: Optional[EventBus] = None, manager=None, + executor: Optional[concurrent.futures.Executor] = None, + dispatcher: Dispatcher = direct, mods: Optional[ModsApi] = None, + java: Optional[JavaApi] = None, settings: Optional[SettingsApi] = None): + self.bus = bus or EventBus() + if manager is None: + # Imported here: ZBBManager wires every service at construction, + # which tests of the API alone don't need. + from app.core.core import ZBBManager + manager = ZBBManager(self.bus) + self._manager = manager + self._executor = executor or concurrent.futures.ThreadPoolExecutor( + max_workers=4, thread_name_prefix="api-task") + self.events = EventHub(self.bus, dispatcher) + self.tasks = TaskRunner(self._executor, lambda: self.events.dispatcher) + + self.servers = ServersApi(manager, self.bus, self.tasks) + self.console = ConsoleApi(manager) + self.tunnel = TunnelApi(manager) + self.backups = BackupsApi(manager) + # PlayerOrchestrator already takes and returns plain values. + self.players = manager.players + self.mods = mods or ModsApi(create_snapshot=self.backups.create_snapshot) + self.java = java or JavaApi() + self.settings = settings or SettingsApi() + self.storage = StorageApi() + + @property + def legacy_manager(self): + """The ZBBManager, for the CustomTkinter screens not ported to this + API yet (players panel, settings, server properties). New code and + the Qt UI must not use it.""" + return self._manager + + def set_dispatcher(self, dispatcher: Dispatcher) -> None: + """Where event handlers, task progress and completion run from now on.""" + self.events.set_dispatcher(dispatcher) + + def start_background_services(self) -> None: + """Tick loop, version pre-warm, tunnel auto-start (once, at startup).""" + self._manager.bootstrap() + + def shutdown(self) -> None: + """Stop servers, tunnel and monitors; then kill any child process that + survived the graceful stop. Blocking (seconds): call off the UI thread.""" + self.tasks.cancel_all() + self.events.close() + self._manager.shutdown() + self._kill_orphan_processes() + + def _kill_orphan_processes(self) -> None: + # Last resort during exit: a failure here is unactionable (the Job + # Object reaps survivors when the app dies), so it is only logged. + for owner in (getattr(self._manager, "server_runner", None), + getattr(self._manager, "playit_manager", None)): + proc = getattr(owner, "process", None) if owner else None + if proc is None or proc.poll() is not None: + continue + try: + proc.kill() + proc.wait(timeout=2) + except Exception as exc: # noqa: BLE001 - see the comment above + logger.debug("Orphan process kill failed: %s", exc) diff --git a/app/api/areas.py b/app/api/areas.py new file mode 100644 index 0000000..40fe132 --- /dev/null +++ b/app/api/areas.py @@ -0,0 +1,109 @@ +"""The smaller areas of the API: console, tunnel, players, backups, Java, settings.""" + +import logging +from dataclasses import dataclass +from typing import Any, Optional + +from app.services.java_detector import JavaDetector +from app.services.settings_manager import SettingsManager + +logger = logging.getLogger(__name__) + + +class ConsoleApi: + def __init__(self, manager): + self._manager = manager + + def send(self, command: str) -> bool: + """Send a console command to the running server. False when none runs. + The safety check (and the blocked-command notice) is in the core.""" + if not command or not self._manager.is_running(): + return False + self._manager.send_command(command) + return True + + +class TunnelApi: + def __init__(self, manager): + self._manager = manager + + def start(self) -> None: + self._manager.start_tunnel() + + def stop(self) -> None: + self._manager.stop_tunnel() + + def reset(self, mode: str = "soft") -> None: + """Delete the tunnels, keep the agent linked ("soft"). Blocking.""" + self._manager.reset_tunnel(mode=mode) + + def link(self, setup_code: str) -> bool: + """Link the Playit agent with a setup code. Blocking.""" + return bool(self._manager.link_playit_manually(setup_code)) + + @property + def is_linked(self) -> bool: + playit = getattr(self._manager, "playit_manager", None) + return bool(playit is not None and playit.is_linked) + + @property + def is_running(self) -> bool: + playit = getattr(self._manager, "playit_manager", None) + return bool(playit is not None and getattr(playit, "running", False)) + + +class BackupsApi: + def __init__(self, manager): + self._manager = manager + + def create_snapshot(self, server_name: str) -> tuple: + """Pre-update snapshot: (path, None) or (None, error). Blocking.""" + return self._manager.create_pre_update_snapshot(server_name) + + def delete(self, server_name: str, backup_path: str) -> tuple: + return self._manager.delete_backup(server_name, backup_path) + + +@dataclass(frozen=True) +class JavaSummary: + major: int + source: str # "PORTABLE", "PATH", "REGISTRY", ... + version_string: str + + @property + def portable(self) -> bool: + return self.source == "PORTABLE" + + +class JavaApi: + def __init__(self, detector_factory=JavaDetector): + self._detector_factory = detector_factory + + def best(self) -> Optional[JavaSummary]: + """The preferred Java found on this machine, or None. Blocking.""" + installations = self._detector_factory().detect_all() + if not installations: + return None + best = installations[0] + return JavaSummary(best.major, best.source, best.version_string) + + +class SettingsApi: + """App-wide settings (config/zbb_settings.json).""" + + def __init__(self, settings: Optional[SettingsManager] = None): + self._settings = settings + + @property + def _store(self) -> SettingsManager: + return self._settings or SettingsManager() + + def get(self, key: str, default: Any = None) -> Any: + return self._store.get(key, default) + + def set(self, key: str, value: Any) -> None: + self._store.set(key, value) + + def save(self) -> None: + """Write pending changes now (they are otherwise debounced).""" + self._store.save() diff --git a/app/api/events.py b/app/api/events.py new file mode 100644 index 0000000..6614fa6 --- /dev/null +++ b/app/api/events.py @@ -0,0 +1,90 @@ +"""Event subscriptions for UIs, on top of the core EventBus. + +The core emits on whatever thread did the work. A UI may only touch its +widgets from its own thread, so every subscription goes through a +*dispatcher*: a callable that takes a zero-argument function and runs it +where the UI wants it. CustomTkinter passes ``lambda fn: root.after(0, fn)``, +a Qt UI a queued signal; tests and headless callers use ``direct``. +""" + +import logging +import threading +from typing import Any, Callable, Optional + +from app.core.server_events import EventBus + +logger = logging.getLogger(__name__) + +Dispatcher = Callable[[Callable[[], None]], None] + + +def direct(fn: Callable[[], None]) -> None: + """Run on the emitting thread (headless use, tests).""" + fn() + + +class Subscription: + """One handler on one event. ``cancel()`` stops delivery at once, also + for calls a dispatcher has queued but not run yet.""" + + def __init__(self, bus: EventBus, event: str, handler: Callable[[Any], None], dispatcher: Dispatcher): + self.event = event + self._bus = bus + self._handler = handler + self._dispatcher = dispatcher + self._active = True + + @property + def active(self) -> bool: + return self._active + + def _deliver(self, data: Any) -> None: + if not self._active: + return + + def run(): + if self._active: + self._handler(data) + + self._dispatcher(run) + + def cancel(self) -> None: + if not self._active: + return + self._active = False + self._bus.unsubscribe(self.event, self._deliver) + + +class EventHub: + """Subscribe to core events with a dispatcher (see the module docstring).""" + + def __init__(self, bus: EventBus, dispatcher: Dispatcher = direct): + self.bus = bus + self._dispatcher = dispatcher + self._subscriptions: list[Subscription] = [] + self._lock = threading.Lock() + + @property + def dispatcher(self) -> Dispatcher: + return self._dispatcher + + def set_dispatcher(self, dispatcher: Dispatcher) -> None: + """Default for subscriptions made from now on (the UI sets it once + its main loop object exists).""" + self._dispatcher = dispatcher + + def subscribe(self, event: str, handler: Callable[[Any], None], + dispatcher: Optional[Dispatcher] = None) -> Subscription: + sub = Subscription(self.bus, event, handler, dispatcher or self._dispatcher) + with self._lock: + self._subscriptions = [s for s in self._subscriptions if s.active] + self._subscriptions.append(sub) + self.bus.subscribe(event, sub._deliver) + return sub + + def close(self) -> None: + """Cancel every subscription made through this hub.""" + with self._lock: + subs, self._subscriptions = self._subscriptions, [] + for sub in subs: + sub.cancel() diff --git a/app/api/mods.py b/app/api/mods.py new file mode 100644 index 0000000..cc3f8f4 --- /dev/null +++ b/app/api/mods.py @@ -0,0 +1,101 @@ +"""Mods, plugins and modpacks: Modrinth search/install/update and the +server's content files. Every method blocks (network or disk): call it +off the UI thread.""" + +import threading +from typing import Callable, Optional + +from app.services import mod_content, mod_icons, mod_install_tracker +from app.services.mod_content import ( # noqa: F401 (re-exported for UIs) + DeleteResult, ModDownloadError, NoCompatibleVersionError, UnsafePathError, + filter_updates_for_selection, snapshot_then_apply, +) +from app.services.modrinth import ( # noqa: F401 (re-exported for UIs) + MOD_LOADERS, PLUGIN_LOADERS, ModrinthClient, ModrinthException, loader_display, +) +from app.services.mrpack_installer import MrpackCompatibilityError # noqa: F401 (re-exported) + + +class ModsApi: + def __init__(self, client_factory: Callable = ModrinthClient, + create_snapshot: Optional[Callable] = None): + self._client_factory = client_factory + self._client = None + self._client_lock = threading.Lock() + self._create_snapshot = create_snapshot + + @property + def client(self): + with self._client_lock: + if self._client is None: + self._client = self._client_factory() + return self._client + + # -- Modrinth -------------------------------------------------------- + def search(self, query: str, **filters) -> dict: + return self.client.search(query, **filters) + + def get_versions(self, slug: str, mc_version: Optional[str] = None, loader: Optional[str] = None) -> list: + return self.client.get_versions(slug, mc_version=mc_version, loader=loader) + + def required_dependencies(self, version: dict, mc_version: str, loader: str, installed_slugs) -> dict: + return self.client.get_required_dependencies(version, mc_version, loader, installed_slugs) + + def check_updates(self, server_name: str, mc_version: str, loader: str) -> list: + return self.client.check_updates(server_name, mc_version, loader) + + def fetch_icon(self, icon_url: str): + """PIL image (cached on disk) or None; raises on network errors.""" + return mod_icons.fetch_icon(icon_url, self.client.session) + + @staticmethod + def cached_icon(icon_url: str): + return mod_icons.cached_icon(icon_url) + + # -- installs -------------------------------------------------------- + def install_version(self, version: dict, server_name: str, loader: str, slug: str, + progress: Optional[Callable[[float], None]] = None) -> Optional[str]: + return mod_content.install_version(self.client, version, server_name, loader, slug, progress) + + def install_latest(self, slug: str, server_name: str, mc_version: str, loader: str) -> Optional[str]: + return mod_content.install_latest(self.client, slug, server_name, mc_version, loader) + + def install_modpack(self, version: dict, server_name: str, + on_download: Optional[Callable[[float], None]] = None, + on_status: Optional[Callable[[str], None]] = None) -> dict: + return mod_content.install_modpack(self.client, version, server_name, on_download, on_status) + + def apply_updates(self, server_name: str, loader: str, updates: list, + on_update: Optional[Callable[[dict], None]] = None) -> tuple: + """Pre-update snapshot, then each update. Returns (snapshot_error, + updated, failed); nothing is touched when the snapshot fails.""" + def apply_one(update): + if on_update: + on_update(update) + return self.client.apply_update(update, server_name, loader) + return snapshot_then_apply(self._create_snapshot, server_name, updates, apply_one) + + # -- the server's files ------------------------------------------------ + @staticmethod + def installed_files(server_name: str) -> list[str]: + return mod_content.installed_jar_files(server_name) + + @staticmethod + def installed_slugs(server_name: str) -> set: + return mod_install_tracker.get_installed_slugs(server_name) + + @staticmethod + def installed_filename(server_name: str, slug: str) -> str: + return mod_install_tracker.get_installed_filename(server_name, slug) + + @staticmethod + def uninstall(server_name: str, slug: str) -> Optional[str]: + return mod_content.uninstall(server_name, slug) + + @staticmethod + def delete_file(server_name: str, path: str) -> str: + return mod_content.delete_content_file(server_name, path) + + @staticmethod + def delete_files(server_name: str, paths) -> DeleteResult: + return mod_content.delete_content_files(server_name, paths) diff --git a/app/api/servers.py b/app/api/servers.py new file mode 100644 index 0000000..ea100af --- /dev/null +++ b/app/api/servers.py @@ -0,0 +1,157 @@ +"""Servers: list, inspect, create, start/stop, delete, import/export.""" + +import logging +import os +from dataclasses import dataclass +from typing import Optional + +from app.api.tasks import Task, TaskRunner +from app.core import logic +from app.core.constants import SERVERS_DIR +from app.core.provisioning import ProvisionResult +from app.core.server_events import ServerEvent +from app.services import migration +from app.services.modrinth import loader_for_server_type +from app.services.server_paths import check_server_name +from app.services.server_properties import load_server_properties + +logger = logging.getLogger(__name__) + +CREATE_TASK = "create_server" + + +class ServerRunningError(RuntimeError): + """The action needs the server stopped.""" + + +class NoServerJarError(ValueError): + """The folder chosen for import has no .jar to run.""" + + +@dataclass(frozen=True) +class ServerInfo: + name: str + engine: Optional[str] # metadata "type": Vanilla, Fabric, Paper... (None: unknown) + mc_version: Optional[str] # None when the metadata has none + loader: Optional[str] # Modrinth loader, None for vanilla + max_players: Optional[int] + + @property + def mod_context(self) -> tuple: + """(name, mc_version, loader), what the Mods tab works with.""" + return self.name, self.mc_version if self.mc_version is not None else "1.20.1", self.loader + + +@dataclass(frozen=True) +class MemorySample: + used_bytes: Optional[int] # None while nothing runs + limit_mb: Optional[int] # the server's RAM allocation + + +class ServersApi: + def __init__(self, manager, events, tasks: TaskRunner): + self._manager = manager + self._events = events + self._tasks = tasks + + # -- queries --------------------------------------------------------- + def list(self) -> list[str]: + """Server folder names (creates the servers folder on first use).""" + os.makedirs(SERVERS_DIR, exist_ok=True) + return [d for d in os.listdir(SERVERS_DIR) if os.path.isdir(os.path.join(SERVERS_DIR, d))] + + def exists(self, name: str) -> bool: + return os.path.isdir(os.path.join(SERVERS_DIR, check_server_name(name))) + + def info(self, name: str) -> ServerInfo: + meta = logic.get_server_meta(check_server_name(name)) or {} + engine = meta.get("type") + try: + max_players = int(load_server_properties(name).get("max-players", 20)) + except (TypeError, ValueError): + max_players = None + return ServerInfo(name=name, engine=engine, mc_version=meta.get("version"), + loader=loader_for_server_type(meta.get("type", "Vanilla")), max_players=max_players) + + def is_running(self) -> bool: + return self._manager.is_running() + + @property + def active(self) -> Optional[str]: + """The server the core acts on: the running one, or the last selected + while nothing runs.""" + return self._manager.current_server + + def running_server(self) -> Optional[str]: + return self._manager.current_server if self._manager.is_running() else None + + def memory(self) -> MemorySample: + used = self._manager.server_memory_usage() + server = self._manager.current_server + return MemorySample(used, logic.get_server_ram(server) if server else None) + + # -- commands -------------------------------------------------------- + def select(self, name: str) -> bool: + """Make ``name`` the active server, unless another one runs.""" + if self._manager.is_running(): + return False + self._manager.select_server(check_server_name(name)) + return True + + def start(self, name: Optional[str] = None) -> bool: + """Start ``name`` (default: the active server). Blocking; call off the + UI thread. False when a server already runs.""" + if self._manager.is_running(): + return False + if name and self._manager.current_server != name: + self._manager.select_server(check_server_name(name)) + return bool(self._manager.start_server()) + + def stop(self) -> None: + """Blocking (graceful wait, then kill): call off the UI thread.""" + self._manager.stop_server() + + def delete(self, name: str) -> None: + """Delete a server folder (only the link for an imported server).""" + check_server_name(name) + if self._manager.is_running() and self._manager.current_server == name: + raise ServerRunningError(name) + logic.delete_server(name) + if self._manager.current_server == name: + self._manager.current_server = None + + def export(self, name: str, dest_path: str) -> str: + return migration.export_server(check_server_name(name), dest_path) + + def import_pack(self, src_path: str, name: str) -> dict: + return migration.import_server(src_path, check_server_name(name)) + + def import_folder(self, folder: str) -> bool: + """Link an existing server folder. Raises NoServerJarError when it has + no .jar, ServerExistsError when the name is taken.""" + if not any(f.endswith(".jar") for f in os.listdir(folder)): + raise NoServerJarError(folder) + return bool(self._manager.load_server_manually(folder)) + + def update_meta(self, name: str, updates: dict) -> bool: + return logic.update_server_meta(check_server_name(name), updates) + + def request_mod_install(self, name: str, mod_ids: list) -> None: + """Ask the core to fetch missing mods after a dependency crash.""" + self._events.emit(ServerEvent.REQUEST_MOD_INSTALL, { + "missing_mod_ids": list(mod_ids), "server_name": name}) + + def create(self, config: dict) -> Task: + """Provision a server from wizard config. Progress: ``(fraction, text)``; + result: a ProvisionResult. Cancelling rolls the attempt back.""" + def job(task: Task): + try: + return self._manager.provision_server(config, task.report, task.cancel_event) + except Exception as exc: + # The UI waits on a result to close its progress dialog. + logger.exception("Provisioning %s crashed", config.get("name")) + return ProvisionResult(ok=False, name=config.get("name", ""), error=str(exc)) + return self._tasks.start(CREATE_TASK, job) + + def creations(self) -> list[Task]: + return self._tasks.active(CREATE_TASK) diff --git a/app/api/storage.py b/app/api/storage.py new file mode 100644 index 0000000..e3efa5a --- /dev/null +++ b/app/api/storage.py @@ -0,0 +1,64 @@ +"""Folders: where things live and opening them in the system file manager.""" + +import logging +import os +import subprocess +import sys +from pathlib import Path +from typing import Callable, Optional + +from app.core.constants import BASE_DIR, LOGS_DIR, SERVERS_DIR +from app.services.server_paths import check_server_name + +logger = logging.getLogger(__name__) + + +def open_in_file_manager(path, *, create: bool = False, platform: Optional[str] = None, + run: Callable = subprocess.run) -> bool: + """Show ``path`` in Explorer / Finder / the desktop's file manager. + + Returns False when the folder doesn't exist (and ``create`` is off) or + the file manager couldn't be launched; the caller tells the user. + """ + path = Path(path) + if create: + try: + path.mkdir(parents=True, exist_ok=True) + except OSError as exc: + logger.warning("Could not create %s: %s", path, exc) + return False + if not path.exists(): + return False + platform = platform or sys.platform + command = "explorer" if platform == "win32" else "open" if platform == "darwin" else "xdg-open" + try: + run([command, str(path)], check=False) + except OSError as exc: + logger.warning("Failed to open %s with %s: %s", path, command, exc) + return False + return True + + +class StorageApi: + @property + def data_dir(self) -> Path: + return Path(BASE_DIR) + + @property + def servers_dir(self) -> Path: + return Path(SERVERS_DIR) + + @property + def logs_dir(self) -> Path: + return Path(LOGS_DIR) + + def server_dir(self, server_name: str, subpath: str = "") -> Path: + path = Path(SERVERS_DIR) / check_server_name(server_name) + return path / subpath if subpath else path + + def ensure_servers_dir(self) -> Path: + os.makedirs(SERVERS_DIR, exist_ok=True) + return Path(SERVERS_DIR) + + def open_folder(self, path, create: bool = False) -> bool: + return open_in_file_manager(path, create=create) diff --git a/app/api/tasks.py b/app/api/tasks.py new file mode 100644 index 0000000..0d5d168 --- /dev/null +++ b/app/api/tasks.py @@ -0,0 +1,117 @@ +"""Handles for long operations: progress, cancellation and completion. + +A ``Task`` wraps one background job. The job receives the task and calls +``task.report(...)`` for progress and checks ``task.cancel_event`` (the same +``threading.Event`` the provisioning code already polls between steps and +download chunks). Progress and completion callbacks go through the task's +dispatcher, so a UI receives them on its own thread. +""" + +import concurrent.futures +import logging +import threading +from typing import Any, Callable, Optional + +from app.api.events import Dispatcher, direct + +logger = logging.getLogger(__name__) + + +class Task: + def __init__(self, name: str = "", dispatcher: Dispatcher = direct, cancellable: bool = True): + self.name = name + self.cancellable = cancellable + self.cancel_event = threading.Event() + self._dispatcher = dispatcher + self._future: concurrent.futures.Future = concurrent.futures.Future() + self._progress: list[Callable] = [] + self._done: list[Callable] = [] + self._lock = threading.Lock() + + # -- caller side --------------------------------------------------- + def on_progress(self, callback: Callable[..., None]) -> "Task": + with self._lock: + self._progress.append(callback) + return self + + def on_done(self, callback: Callable[["Task"], None]) -> "Task": + """``callback(task)`` once finished (at once if it already is).""" + with self._lock: + finished = self._future.done() + if not finished: + self._done.append(callback) + if finished: + self._dispatcher(lambda: callback(self)) + return self + + def cancel(self) -> bool: + """Ask the job to stop. False when this kind of task can't be cancelled.""" + if not self.cancellable: + return False + self.cancel_event.set() + return True + + @property + def cancel_requested(self) -> bool: + return self.cancel_event.is_set() + + def done(self) -> bool: + return self._future.done() + + def result(self, timeout: Optional[float] = None) -> Any: + return self._future.result(timeout) + + def exception(self, timeout: Optional[float] = None) -> Optional[BaseException]: + return self._future.exception(timeout) + + # -- job side -------------------------------------------------------- + def report(self, *args) -> None: + with self._lock: + listeners = list(self._progress) + for callback in listeners: + self._dispatcher(lambda cb=callback: cb(*args)) + + def _run(self, job: Callable[["Task"], Any]) -> None: + try: + self._future.set_result(job(self)) + except Exception as exc: + logger.exception("Task %s failed", self.name or job) + self._future.set_exception(exc) + with self._lock: + callbacks, self._done = self._done, [] + for callback in callbacks: + self._dispatcher(lambda cb=callback: cb(self)) + + +class TaskRunner: + """Starts tasks on an executor and remembers the unfinished ones, so the + app can cancel them all on exit.""" + + def __init__(self, executor: concurrent.futures.Executor, dispatcher_source: Callable[[], Dispatcher]): + self._executor = executor + self._dispatcher_source = dispatcher_source + self._active: set[Task] = set() + self._lock = threading.Lock() + + def start(self, name: str, job: Callable[[Task], Any], cancellable: bool = True) -> Task: + task = Task(name, self._dispatcher_source(), cancellable) + with self._lock: + self._active.add(task) + + def run(): + try: + task._run(job) + finally: + with self._lock: + self._active.discard(task) + + self._executor.submit(run) + return task + + def active(self, name: Optional[str] = None) -> list[Task]: + with self._lock: + return [t for t in self._active if name is None or t.name == name] + + def cancel_all(self) -> None: + for task in self.active(): + task.cancel() diff --git a/app/services/mod_content.py b/app/services/mod_content.py new file mode 100644 index 0000000..591fb31 --- /dev/null +++ b/app/services/mod_content.py @@ -0,0 +1,195 @@ +"""A server's mod and plugin files: list, install, delete, update, modpacks. + +Every delete goes through ``resolve_content_file``, which refuses anything +that isn't a file directly inside that server's ``mods/`` or ``plugins/`` +folder, so a stale or tampered ``installed_mods.json`` entry, or a path +handed over by a UI, can never remove something else. +""" + +import logging +import os +import shutil +import tempfile +from dataclasses import dataclass, field +from typing import Callable, Optional + +from app.core.constants import SERVERS_DIR +from app.core.logic import get_server_meta +from app.services import mod_install_tracker +from app.services.mrpack_installer import install_mrpack +from app.services.server_paths import UnsafePathError, check_server_name + +logger = logging.getLogger(__name__) + +CONTENT_DIRS = ("mods", "plugins") + + +class ModDownloadError(RuntimeError): + """A Modrinth file could not be downloaded.""" + + +class NoCompatibleVersionError(LookupError): + """Modrinth has no build of the project for this server.""" + + +def content_dirs(server_name: str) -> list[str]: + check_server_name(server_name) + return [os.path.join(SERVERS_DIR, server_name, d) for d in CONTENT_DIRS] + + +def installed_jar_files(server_name: str) -> list[str]: + """Absolute paths of the .jar files in mods/ and plugins/, sorted per folder.""" + files = [] + for d in content_dirs(server_name): + if os.path.isdir(d): + files.extend(os.path.join(d, f) for f in sorted(os.listdir(d)) if f.endswith(".jar")) + return files + + +def resolve_content_file(server_name: str, path: str) -> str: + """Return ``path`` if it names an entry directly inside the server's + mods/ or plugins/ folder; raise UnsafePathError otherwise. + + The parent folder is compared after resolving links (a server imported + from another folder is a junction), the entry itself is not: removing a + link only removes the link. + """ + allowed = {os.path.normcase(os.path.realpath(d)) for d in content_dirs(server_name)} + absolute = os.path.abspath(path) + parent = os.path.normcase(os.path.realpath(os.path.dirname(absolute))) + if parent not in allowed: + raise UnsafePathError(f"{path!r} is not in the mods or plugins folder of {server_name!r}") + if os.path.isdir(absolute) and not os.path.islink(absolute): + raise UnsafePathError(f"{path!r} is a folder") + return absolute + + +def delete_content_file(server_name: str, path: str) -> str: + """Delete one mod/plugin file and forget its Modrinth record. Returns the filename.""" + path = resolve_content_file(server_name, path) + fname = os.path.basename(path) + os.remove(path) + mod_install_tracker.remove_install_by_filename(server_name, fname) + logger.info("Deleted %s from %s", fname, server_name) + return fname + + +@dataclass +class DeleteResult: + deleted: list = field(default_factory=list) # filenames + failed: list = field(default_factory=list) # (path, error message) + removed_slugs: set = field(default_factory=set) + + +def delete_content_files(server_name: str, paths) -> DeleteResult: + """Delete several files; one failure doesn't stop the others.""" + filename_to_slug = {mod_install_tracker.get_installed_filename(server_name, slug): slug + for slug in mod_install_tracker.get_installed_slugs(server_name)} + result = DeleteResult() + for path in paths: + try: + fname = delete_content_file(server_name, path) + except (OSError, UnsafePathError) as exc: + logger.warning("Failed to delete %s: %s", path, exc) + result.failed.append((path, str(exc))) + continue + result.deleted.append(fname) + if fname in filename_to_slug: + result.removed_slugs.add(filename_to_slug[fname]) + return result + + +def uninstall(server_name: str, slug: str) -> Optional[str]: + """Remove a mod installed from Modrinth by its slug. Returns the deleted + filename, or None when the slug isn't recorded. A recorded file that is + already gone only drops the record.""" + fname = mod_install_tracker.get_installed_filename(server_name, slug) + if not fname: + return None + for d in content_dirs(server_name): + candidate = resolve_content_file(server_name, os.path.join(d, fname)) + if os.path.isfile(candidate): + os.remove(candidate) + logger.info("Uninstalled %s (%s) from %s", slug, fname, server_name) + break + mod_install_tracker.remove_install(server_name, slug) + return fname + + +def install_version(client, version: dict, server_name: str, loader: str, slug: str, + progress: Optional[Callable[[float], None]] = None) -> Optional[str]: + """Download one Modrinth version into mods/ or plugins/ and record its + slug. Returns the file path, or None when the download failed.""" + check_server_name(server_name) + path = client.download_version(version, server_name, loader, progress_callback=progress) + if path: + mod_install_tracker.record_install(server_name, slug, os.path.basename(path)) + return path + + +def install_latest(client, slug: str, server_name: str, mc_version: str, loader: str) -> Optional[str]: + """Install the newest compatible build of a project. Raises + NoCompatibleVersionError when there is none; None on a failed download.""" + check_server_name(server_name) + if not client.get_versions(slug, mc_version=mc_version, loader=loader): + raise NoCompatibleVersionError(slug) + path = client.download_mod(slug, server_name, mc_version, loader) + if path: + mod_install_tracker.record_install(server_name, slug, os.path.basename(path)) + return path + + +def install_modpack(client, version: dict, server_name: str, + on_download: Optional[Callable[[float], None]] = None, + on_status: Optional[Callable[[str], None]] = None) -> dict: + """Download a .mrpack version to a temporary folder and install it into + the server. Returns install_mrpack's summary; raises ModDownloadError + or MrpackCompatibilityError. The temporary folder is always removed.""" + check_server_name(server_name) + tmp_dir = tempfile.mkdtemp(prefix="zbb_modpack_") + try: + downloaded = client.download_version_to(version, tmp_dir, progress_callback=on_download) + if not downloaded: + raise ModDownloadError(version.get("id", "?")) + meta = get_server_meta(server_name) + return install_mrpack( + downloaded, server_name, + progress_callback=on_status, + server_type=meta.get("type"), + mc_version=meta.get("version"), + ) + finally: + shutil.rmtree(tmp_dir, ignore_errors=True) + + +def snapshot_then_apply(create_snapshot: Optional[Callable], server_name: str, + updates: list, apply_one: Callable[[dict], bool]) -> tuple[Optional[str], int, int]: + """Snapshot the server, then apply each update in order. + + Returns (snapshot_error, updated, failed). When the snapshot fails no + update is applied — updating without a rollback point is exactly the + false confidence this flow exists to prevent. + """ + if create_snapshot is None: + return "Snapshot service unavailable.", 0, 0 + path, error = create_snapshot(server_name) + if not path: + return error or "Unknown snapshot error.", 0, 0 + + updated, failed = 0, 0 + for update in updates: + try: + ok = apply_one(update) + except Exception as exc: + logger.warning("Update failed for %s: %s", update.get("filename"), exc) + ok = False + if ok: + updated += 1 + else: + failed += 1 + return None, updated, failed + + +def filter_updates_for_selection(updates: list, selected_filenames: set) -> list: + """The subset of check_updates() results matching the selected filenames.""" + return [u for u in updates if u.get("filename") in selected_filenames] diff --git a/app/services/mod_icons.py b/app/services/mod_icons.py new file mode 100644 index 0000000..a0ac8d3 --- /dev/null +++ b/app/services/mod_icons.py @@ -0,0 +1,62 @@ +"""Modrinth project icons: download, scale to a thumbnail, cache on disk. + +Returns PIL images; wrapping them for a toolkit (CTkImage, QPixmap) and any +in-memory cache of those wrappers belongs to the UI. +""" + +import hashlib +import io +import logging +from pathlib import Path +from typing import Optional + +from PIL import Image + +from app.core.constants import BASE_DIR + +logger = logging.getLogger(__name__) + +ICON_SIZE = 48 +ICON_TIMEOUT = 8 + + +def icon_cache_dir() -> Path: + return BASE_DIR / ".zbb_cache" / "modrinth_icons" + + +def icon_disk_path(icon_url: str) -> Path: + return icon_cache_dir() / (hashlib.md5(icon_url.encode()).hexdigest() + ".png") + + +def cached_icon(icon_url: str) -> Optional[Image.Image]: + """The icon from the disk cache, or None when missing/unreadable.""" + disk_path = icon_disk_path(icon_url) + if not disk_path.exists(): + return None + try: + img = Image.open(disk_path) + img.load() + except OSError as exc: + logger.debug("Unreadable cached icon %s: %s", disk_path, exc) + return None + return img + + +def fetch_icon(icon_url: str, session) -> Optional[Image.Image]: + """Cached icon, else download it with ``session`` (a requests.Session), + scale it to ICON_SIZE and cache it. None for non-http(s) URLs. Network + and decode errors propagate to the caller.""" + if not icon_url.startswith(("https://", "http://")): + return None + img = cached_icon(icon_url) + if img is not None: + return img + resp = session.get(icon_url, timeout=ICON_TIMEOUT) + resp.raise_for_status() + img = Image.open(io.BytesIO(resp.content)).resize((ICON_SIZE, ICON_SIZE), Image.LANCZOS) + try: + icon_cache_dir().mkdir(parents=True, exist_ok=True) + img.save(icon_disk_path(icon_url)) + except OSError as exc: + logger.debug("Icon disk cache write failed: %s", exc) + return img diff --git a/app/services/server_paths.py b/app/services/server_paths.py new file mode 100644 index 0000000..476389f --- /dev/null +++ b/app/services/server_paths.py @@ -0,0 +1,15 @@ +"""Validation of server names used to build paths under SERVERS_DIR.""" + +import os + + +class UnsafePathError(ValueError): + """A name or path that would reach outside the place it must stay in.""" + + +def check_server_name(server_name: str) -> str: + """A server name is one folder under SERVERS_DIR, never a path.""" + if (not isinstance(server_name, str) or not server_name.strip() or server_name in (".", "..") + or "/" in server_name or "\\" in server_name or os.sep in server_name or ":" in server_name): + raise UnsafePathError(f"Not a server name: {server_name!r}") + return server_name diff --git a/app/ui/app_settings.py b/app/ui/app_settings.py index bd5c933..699feb3 100644 --- a/app/ui/app_settings.py +++ b/app/ui/app_settings.py @@ -9,13 +9,13 @@ `self.after(0, ...)`. """ import logging -import subprocess import sys import threading from pathlib import Path import customtkinter as ctk +from app.api.storage import open_in_file_manager from app.core import i18n from app.core.app_config import AppConfig from app.core.bootstrap import remember_language @@ -776,14 +776,7 @@ def _open_logs_dir(self): @staticmethod def _open_dir(directory: Path): - directory.mkdir(parents=True, exist_ok=True) - path = str(directory) - if sys.platform == "win32": - subprocess.run(["explorer", path], check=False) - elif sys.platform == "darwin": - subprocess.run(["open", path], check=False) - else: - subprocess.run(["xdg-open", path], check=False) + open_in_file_manager(directory, create=True) def _storage_categories(self) -> list: """(label, size_bytes) pairs -- runs on a worker thread.""" diff --git a/app/ui/main.py b/app/ui/main.py index 43120aa..5cddf95 100644 --- a/app/ui/main.py +++ b/app/ui/main.py @@ -6,7 +6,6 @@ import concurrent.futures import webbrowser import time -import subprocess logger = logging.getLogger(__name__) @@ -25,19 +24,16 @@ from app.ui.win_effects import apply_rounded_corners from app.ui.icons import icon from app.ui.formatting import engine_name, format_duration, format_memory, memory_tooltip -from app.services.server_properties import load_server_properties -import app.core.logic as logic -from app.core.constants import SERVERS_DIR, ASSETS_DIR, LOGS_DIR +from app.api import AppApi, NoServerJarError, ServerEvent, ServerRunningError, direct +from app.core.constants import ASSETS_DIR, LOGS_DIR from app.core.logging_setup import configure_logging, log_tk_callback_exception from app.ui.server_wizard import ServerWizard from app.ui.server_properties_editor import ServerPropertiesEditor -from app.core.server_events import ServerEvent, EventBus from app.core.app_config import AppConfig from app.core.i18n import _, N_ from app.ui.modrinth_browser import ModrinthBrowser, _ICON_EXECUTOR as MODRINTH_ICON_EXECUTOR from app.ui.toast import Toast -from app.core.core import ZBBManager from app.core.playit_manager import TunnelStatus, tunnel_status_label _theme_path = ASSETS_DIR / "zbb_theme.json" @@ -168,23 +164,31 @@ def _init_state_variables(self): # be selected to browse and configure it. self.viewed_server = None - self.events = EventBus() - self.zbb_manager = ZBBManager(self.events) self.executor = concurrent.futures.ThreadPoolExecutor(max_workers=10, thread_name_prefix='UI_Worker') - - self.events.subscribe(ServerEvent.CONSOLE_LINE, self.update_console) - self.events.subscribe(ServerEvent.NOTIFICATION, self._handle_notification) - self.events.subscribe(ServerEvent.TUNNEL_CONSOLE_LINE, self.update_tunnel_console) - self.events.subscribe(ServerEvent.TUNNEL_STATUS, self.on_tunnel_status) - self.events.subscribe(ServerEvent.READY, self.on_server_ready) - self.events.subscribe(ServerEvent.STARTING, self.on_server_starting) - self.events.subscribe(ServerEvent.STOPPED, self.on_server_stopped) - self.events.subscribe(ServerEvent.PLAYER_COUNT, self.on_player_count_update) - # Toast notification for lag spikes - self.events.subscribe(ServerEvent.LAG_SPIKE, lambda d: self.after(0, lambda: ( - self.update_console("[Watchdog] Lag threshold exceeded. Consider reducing world size or adding more RAM."), - Toast.show(self, "Lag spike threshold exceeded", toast_type="warning"), - ))) + # Core events, task progress and task completion reach this window + # through the Tk main loop. + self.api = AppApi(executor=self.executor, dispatcher=lambda fn: self.after(0, fn)) + self.events = self.api.bus + # Screens not yet ported to the API (players, settings, properties) + # still take the manager; see docs/ROADMAP.md 8b. + self.zbb_manager = self.api.legacy_manager + + # Handlers that marshal on their own (they are also called from + # worker threads) take the events on the emitting thread. + subscribe = self.api.events.subscribe + subscribe(ServerEvent.CONSOLE_LINE, self._log_console_line) + subscribe(ServerEvent.NOTIFICATION, self._handle_notification) + subscribe(ServerEvent.TUNNEL_CONSOLE_LINE, self._log_tunnel_line) + subscribe(ServerEvent.TUNNEL_STATUS, self.on_tunnel_status, dispatcher=direct) + subscribe(ServerEvent.READY, self.on_server_ready, dispatcher=direct) + subscribe(ServerEvent.STARTING, self.on_server_starting, dispatcher=direct) + subscribe(ServerEvent.STOPPED, self.on_server_stopped, dispatcher=direct) + subscribe(ServerEvent.PLAYER_COUNT, self.on_player_count_update) + subscribe(ServerEvent.LAG_SPIKE, self._on_lag_spike) + + def _on_lag_spike(self, _data=None): + self.server_console.log("[Watchdog] Lag threshold exceeded. Consider reducing world size or adding more RAM.") + Toast.show(self, "Lag spike threshold exceeded", toast_type="warning") def _build_layout(self): self._build_sidebar() @@ -515,8 +519,7 @@ def _on_console_tab_changed(self): self._ensure_modrinth_browser() elif self.console_tabs.get() == "Players": self._ensure_players_panel() - from app.services.settings_manager import SettingsManager - SettingsManager().set("last_console_tab", self.console_tabs.get()) + self.api.settings.set("last_console_tab", self.console_tabs.get()) def _ensure_players_panel(self): if hasattr(self, "players_panel"): @@ -540,7 +543,7 @@ def _ensure_modrinth_browser(self): self.modrinth_browser = ModrinthBrowser( self.console_tabs.tab("Mods"), get_server_info=self._get_current_server_info, - create_snapshot=self.zbb_manager.create_pre_update_snapshot, + mods=self.api.mods, is_server_running=self._viewing_running_server, on_installed_count=lambda n: self._refresh_tab_labels(installed=n), ) @@ -553,7 +556,7 @@ def _ensure_modrinth_browser(self): def _init_background_services(self): self.check_java_startup() self.load_servers() - self.zbb_manager.bootstrap() + self.api.start_background_services() # Pre-warm now handled by ZBBManager.bootstrap() → core.py def _build_console_search_bar(self, parent, console_attr): @@ -599,7 +602,7 @@ def on_filter_change(choice): ToolTip(filter_menu, log_class.FILTER_HINT) def send_server_command(self, event=None): - if not self.zbb_manager.is_running(): + if not self.api.servers.is_running(): self.server_console.log("[UI] Server is not running.") return cmd = self.entry_console.get() @@ -607,7 +610,7 @@ def send_server_command(self, event=None): # Safety check lives in ServerOrchestrator.send_command -- it owns # the single source of truth for blocked-command feedback (console # line + toast via EventBus), so this stays a thin passthrough. - self.zbb_manager.send_command(cmd) + self.api.console.send(cmd) if not self._command_history or self._command_history[-1] != cmd: self._command_history.append(cmd) del self._command_history[:-self._COMMAND_HISTORY_MAX] @@ -632,8 +635,8 @@ def _set_console_input(self, running: bool): otherwise, with a placeholder that says why.""" if running: placeholder = "Type a command (Up/Down: history)" - elif self.zbb_manager.is_running() and self.zbb_manager.current_server != self.viewed_server: - placeholder = f"{self.zbb_manager.current_server} is running — select it to send commands" + elif self.api.servers.is_running() and self.api.servers.active != self.viewed_server: + placeholder = f"{self.api.servers.active} is running — select it to send commands" elif self.viewed_server: placeholder = "Start the server to send commands" else: @@ -651,12 +654,9 @@ def _set_console_input(self, running: bool): def check_java_startup(self): def _check(): - from app.services.java_detector import JavaDetector - detector = JavaDetector() - installations = detector.detect_all() - if installations: - best = installations[0] - source_badge = "Portable" if best.source == "PORTABLE" else "System" + best = self.api.java.best() + if best: + source_badge = "Portable" if best.portable else "System" label = f"Java {best.major} ({source_badge})" self.after(0, lambda: self.lbl_java_ver.configure(text=label, text_color=AppConfig.COLOR_BTN_SUCCESS)) self.after(0, lambda: setattr(self.status_tooltip, "text", f"Java: {label}")) @@ -670,9 +670,7 @@ def _check(): def load_servers(self): def _scan(): try: - if not os.path.exists(SERVERS_DIR): - os.makedirs(SERVERS_DIR, exist_ok=True) - servers = [d for d in os.listdir(SERVERS_DIR) if os.path.isdir(os.path.join(SERVERS_DIR, d))] + servers = self.api.servers.list() except OSError as e: logger.warning("Failed to scan servers: %s", e) servers = [] @@ -713,8 +711,7 @@ def _restore_last_selection(self, servers): if getattr(self, "_selection_restored", False): return self._selection_restored = True - from app.services.settings_manager import SettingsManager - settings = SettingsManager() + settings = self.api.settings last_server = settings.get("last_server") if self.viewed_server or last_server not in servers: return @@ -752,7 +749,7 @@ def _update_mods_tab_state(self): self.console_tabs.set("Console") def on_server_delete(self, server_name): - if self.zbb_manager.is_running() and self.zbb_manager.current_server == server_name: + if self.api.servers.running_server() == server_name: Toast.show(self, "Stop the server before deleting it", toast_type="warning") return confirmed = ZBBDialog.confirm( @@ -769,8 +766,8 @@ def on_server_delete(self, server_name): def _delete(): try: - logic.delete_server(server_name) - except OSError as e: + self.api.servers.delete(server_name) + except (OSError, ServerRunningError) as e: self.after(0, lambda e=e: Toast.show(self, f"Delete failed: {e}", toast_type="error")) return self.after(0, lambda: self._on_server_deleted(server_name)) @@ -778,8 +775,6 @@ def _delete(): threading.Thread(target=_delete, daemon=True).start() def _on_server_deleted(self, server_name): - if self.zbb_manager.current_server == server_name: - self.zbb_manager.current_server = None if self.viewed_server == server_name: self.viewed_server = None self.lbl_dash_title.configure(text="Select a server") @@ -794,7 +789,7 @@ def _on_server_deleted(self, server_name): def on_server_export(self, server_name): from tkinter import filedialog - from app.services.migration import export_server, MigrationError + from app.services.migration import MigrationError dest_path = filedialog.asksaveasfilename( defaultextension=".zbbpack", @@ -808,7 +803,7 @@ def on_server_export(self, server_name): def _export(): try: - export_server(server_name, dest_path) + self.api.servers.export(server_name, dest_path) except (MigrationError, OSError) as e: self.after(0, lambda e=e: Toast.show(self, f"Export failed: {e}", toast_type="error")) return @@ -831,7 +826,7 @@ def show_add_server_menu(self): def on_import_zbbpack(self): from tkinter import filedialog - from app.services.migration import import_server, MigrationError + from app.services.migration import MigrationError src_path = filedialog.askopenfilename( filetypes=[("ZeroBlockBridge Pack", "*.zbbpack")], @@ -850,7 +845,7 @@ def on_import_zbbpack(self): if entered is None: return entered = entered.strip() or suggested_name - if os.path.isdir(os.path.join(SERVERS_DIR, entered)): + if self.api.servers.exists(entered): Toast.show(self, f"A server named '{entered}' already exists", toast_type="error") initial = entered continue @@ -861,7 +856,7 @@ def on_import_zbbpack(self): def _import(): try: - import_server(src_path, new_name) + self.api.servers.import_pack(src_path, new_name) except (MigrationError, OSError) as e: self.after(0, lambda e=e: Toast.show(self, f"Import failed: {e}", toast_type="error")) return @@ -876,23 +871,18 @@ def on_server_select(self, server_name): # Any server can be viewed and configured; only the selection of the # server that *runs* is locked while one is running. self.viewed_server = server_name - if not self.zbb_manager.is_running(): - self.zbb_manager.select_server(server_name) - from app.services.settings_manager import SettingsManager - SettingsManager().set("last_server", server_name) + self.api.servers.select(server_name) + self.api.settings.set("last_server", server_name) self.lbl_dash_title.configure(text=f"{server_name}") for name, it in self.server_items.items(): it.set_selected(name == server_name) - meta = logic.get_server_meta(server_name) - server_type = engine_name((meta or {}).get("type")) - mc_version = (meta or {}).get("version", "") + info = self.api.servers.info(server_name) + server_type = engine_name(info.engine) + mc_version = info.mc_version or "" self.lbl_server_info.configure(text=f"{server_type} {mc_version}".strip(), text_color=AppConfig.COLOR_TEXT_PRIMARY) - try: - self._max_players = int(load_server_properties(server_name).get("max-players", 20)) - except (TypeError, ValueError): - self._max_players = None + self._max_players = info.max_players # Tab enabled state first: a new count on a still-disabled tab showed @@ -903,7 +893,7 @@ def on_server_select(self, server_name): if hasattr(self, "players_panel"): self.players_panel.refresh() if not hasattr(self, "modrinth_browser"): - self._refresh_tab_labels(installed=len(ModrinthBrowser._installed_jar_files(server_name))) + self._refresh_tab_labels(installed=len(self.api.mods.installed_files(server_name))) self.btn_config.configure(state="normal") self.btn_open_folder.configure(state="normal") @@ -911,17 +901,16 @@ def on_server_select(self, server_name): self.server_console.log(f"[UI] Selected server: {server_name}") def _viewing_running_server(self) -> bool: - return (self.viewed_server is not None and self.zbb_manager.is_running() - and self.zbb_manager.current_server == self.viewed_server) + return self.viewed_server is not None and self.api.servers.running_server() == self.viewed_server def _sync_server_header(self): """Header controls for the viewed server: live state when it is the one running, plain Offline (start disabled) when another server runs.""" viewing_running = self._viewing_running_server() - other_running = self.zbb_manager.is_running() and not viewing_running + other_running = self.api.servers.is_running() and not viewing_running self._show_run_stop(self.btn_start, self.btn_stop, running=viewing_running, enabled=bool(self.viewed_server) and not other_running, side="right") - self._start_tooltip.text = (f"Stop {self.zbb_manager.current_server} first — one server runs at a time" + self._start_tooltip.text = (f"Stop {self.api.servers.active} first — one server runs at a time" if other_running else "Start server") self._set_console_input(viewing_running) if viewing_running and self._server_phase is not None: @@ -932,42 +921,25 @@ def _sync_server_header(self): self._ram_tooltip.text = "" self._refresh_players_badge() - def _open_in_file_manager(self, path) -> None: - try: - if sys.platform == "win32": - subprocess.run(["explorer", str(path)], check=False) - elif sys.platform == "darwin": - subprocess.run(["open", str(path)], check=False) - else: - subprocess.run(["xdg-open", str(path)], check=False) - except Exception as e: - self.server_console.log(f"[Error] Failed to open folder: {e}") - def open_server_folder(self): - if not self.viewed_server: - self._open_in_file_manager(SERVERS_DIR) - return - server_path = os.path.join(SERVERS_DIR, self.viewed_server) - if os.path.exists(server_path): - self._open_in_file_manager(server_path) + storage = self.api.storage + path = storage.server_dir(self.viewed_server) if self.viewed_server else storage.servers_dir + if path.exists() and not storage.open_folder(path): + self.server_console.log(f"[Error] Failed to open folder: {path}") def _get_current_server_info(self): """(name, mc_version, loader) of the viewed server, for the Mods tab.""" if not self.viewed_server: return None - meta = logic.get_server_meta(self.viewed_server) - mc_version = meta.get("version", "1.20.1") - from app.services.modrinth import loader_for_server_type - loader = loader_for_server_type(meta.get("type", "Vanilla")) - - logger.debug("Server Info for Mod Search: %s | MC: %s | Loader: %s", self.viewed_server, mc_version, loader or 'any') - return (self.viewed_server, mc_version, loader) + name, mc_version, loader = self.api.servers.info(self.viewed_server).mod_context + logger.debug("Server Info for Mod Search: %s | MC: %s | Loader: %s", name, mc_version, loader or 'any') + return name, mc_version, loader def save_advanced_settings(self, *args): - if not self.zbb_manager.current_server: return + if not self.api.servers.active: return label = self.var_java_path.get() path = getattr(self, "_java_label_to_path", {}).get(label, "auto") - logic.update_server_meta(self.zbb_manager.current_server, { + self.api.servers.update_meta(self.api.servers.active, { "advanced_mode": self.var_advanced_mode.get(), "java_path": path, "use_aikars": self.var_use_aikars.get(), @@ -975,24 +947,24 @@ def save_advanced_settings(self, *args): def edit_server_properties(self): if not self.viewed_server: return - ServerPropertiesEditor(self, self.viewed_server, logic, self.zbb_manager) + ServerPropertiesEditor(self, self.viewed_server, zbb_manager=self.zbb_manager) - def update_console(self, text): + def _log_console_line(self, text): if isinstance(text, str): - self.after(0, lambda: self.server_console.log(text)) + self.server_console.log(text) def _handle_notification(self, data): if data and isinstance(data, dict): msg = data.get("msg", "") toast_type = Toast.resolve_type(data) duration = 6000 if toast_type == "error" else 4000 - self.after(0, lambda: Toast.show(self, msg, toast_type=toast_type, duration=duration)) + Toast.show(self, msg, toast_type=toast_type, duration=duration) if data.get("action") == "mod_dependency_fix": - missing_mod_ids = data.get("missing_mod_ids") or [] - self.after(0, lambda: self._offer_mod_auto_install(missing_mod_ids)) + self._offer_mod_auto_install(data.get("missing_mod_ids") or []) def _offer_mod_auto_install(self, missing_mod_ids): - if not missing_mod_ids or not self.zbb_manager.current_server: + server = self.api.servers.active + if not missing_mod_ids or not server: return preview = ", ".join(missing_mod_ids[:5]) + ("..." if len(missing_mod_ids) > 5 else "") msg = ( @@ -1000,23 +972,17 @@ def _offer_mod_auto_install(self, missing_mod_ids): "Attempt to auto-install these from Modrinth into the mods/ folder?" ) if ZBBDialog.confirm(self, "Missing Mod Dependencies", msg, confirm_text="Install"): - self.events.emit(ServerEvent.REQUEST_MOD_INSTALL, { - "missing_mod_ids": missing_mod_ids, - "server_name": self.zbb_manager.current_server, - }) + self.api.servers.request_mod_install(server, missing_mod_ids) - def update_tunnel_console(self, text): - self.after(0, lambda: self.tunnel_console.log(text)) + def _log_tunnel_line(self, text): + self.tunnel_console.log(text) def start_server_action(self): - if self.zbb_manager.is_running(): + if self.api.servers.is_running(): return - if self.viewed_server and self.zbb_manager.current_server != self.viewed_server: - self.zbb_manager.select_server(self.viewed_server) - - def _start(): - self.zbb_manager.start_server() - self.executor.submit(_start) + if self.viewed_server: + self.api.servers.select(self.viewed_server) + self.executor.submit(self.api.servers.start) def _show_run_stop(self, btn_run, btn_stop, running: bool, enabled: bool = True, side: str = "left"): """Show only one of run/stop at a time, matching current state.""" @@ -1033,7 +999,7 @@ def _show_run_stop(self, btn_run, btn_stop, running: bool, enabled: bool = True, fg_color=AppConfig.COLOR_BTN_SUCCESS if enabled else AppConfig.COLOR_BTN_GHOST) def _set_current_server_pill(self, status: str): - item = self.server_items.get(self.zbb_manager.current_server) + item = self.server_items.get(self.api.servers.active) if item: item.set_status(status) @@ -1053,10 +1019,8 @@ def on_server_ready(self, data=None): self.after(0, lambda: self._set_current_server_pill("online")) def on_player_count_update(self, count): - def _apply(): - self._player_count = count - self._refresh_players_badge() - self.after(0, _apply) + self._player_count = count + self._refresh_players_badge() # Tab keys are internal (compared, persisted as "last_console_tab"); # the button shows the translated label plus a live count. @@ -1169,9 +1133,8 @@ def _render_status_label(self): text_color=AppConfig.COLOR_STATUS_ONLINE) def _sample_server_memory(self): - used = self.zbb_manager.server_memory_usage() - server = self.zbb_manager.current_server - limit = logic.get_server_ram(server) if server else None + sample = self.api.servers.memory() + used, limit = sample.used_bytes, sample.limit_mb def _apply(): if self._server_phase is None or not self._viewing_running_server(): @@ -1197,7 +1160,7 @@ def on_server_stopped(self, data=None): def stop_server_action(self): # Off the Tk thread: stop() blocks up to ~15s (graceful wait + kill). def _stop(): - self.zbb_manager.stop_server() + self.api.servers.stop() self.on_server_stopped() self.executor.submit(_stop) @@ -1205,34 +1168,28 @@ def create_server_dialog(self): ServerWizard(self, on_complete_callback=self.on_wizard_complete) def on_wizard_complete(self, config): - # Cancel (or closing the app) sets this; provisioning stops at the - # next step/download chunk, kills a running installer and rolls back. - cancel_event = threading.Event() - self._active_provision_cancels = getattr(self, "_active_provision_cancels", set()) - self._active_provision_cancels.add(cancel_event) + # Cancel (or closing the app) stops provisioning at the next + # step/download chunk, kills a running installer and rolls back. + task = None dialog = DownloadProgressDialog(self, title=f"Installing {config['name']}...", - on_cancel=cancel_event.set) + on_cancel=lambda: task.cancel()) - def _provision(): - try: - result = self.zbb_manager.provision_server(config, dialog.update_progress, cancel_event) - finally: - self._active_provision_cancels.discard(cancel_event) + def _finished(task): + result = task.result() if result.cancelled: - self.after(0, lambda: self._on_provision_cancelled(dialog)) + self._on_provision_cancelled(dialog) elif result.ok: - self.after(0, lambda: self._on_download_complete( - dialog, result.name, config.get("start_after_creation", False), result.warnings)) + self._on_download_complete( + dialog, result.name, config.get("start_after_creation", False), result.warnings) else: - self.after(0, lambda: self._on_provision_failed(dialog, result)) + self._on_provision_failed(dialog, result) - self.executor.submit(_provision) + task = self.api.servers.create(config).on_progress(dialog.update_progress).on_done(_finished) def cancel_active_tasks(self): """Stop every cancellable background task (server creations); used when the app exits so none keeps the process alive.""" - for event in list(getattr(self, "_active_provision_cancels", ())): - event.set() + self.api.tasks.cancel_all() def _on_provision_cancelled(self, dialog): dialog.close() @@ -1267,10 +1224,10 @@ def _on_download_complete(self, dialog, name, start_after_creation=False, warnin def start_tunnel(self): self._show_run_stop(self.btn_tunnel_start, self.btn_tunnel_stop, running=True, enabled=False) - self.zbb_manager.start_tunnel() + self.api.tunnel.start() def stop_tunnel(self): - self.zbb_manager.stop_tunnel() + self.api.tunnel.stop() self._show_run_stop(self.btn_tunnel_start, self.btn_tunnel_stop, running=False) def reset_tunnel(self): @@ -1285,7 +1242,7 @@ def reset_tunnel(self): self.tunnel_console.log("[System] Clearing tunnels...") def _reset_task(): - self.zbb_manager.reset_tunnel(mode="soft") + self.api.tunnel.reset(mode="soft") self.after(0, lambda: self.on_tunnel_status({"status": TunnelStatus.OFFLINE, "skip_debounce": True})) self.after(0, lambda: self._show_run_stop(self.btn_tunnel_start, self.btn_tunnel_stop, running=False)) self.after(0, lambda: self.tunnel_console.log("[System] Tunnels cleared. Use Start Tunnel to create a new tunnel.")) @@ -1312,7 +1269,7 @@ def _update(): self.lbl_tunnel_status.configure(text=f"● {tunnel_status_label(status)}", text_color=color) - is_linked = self.zbb_manager.playit_manager.is_linked + is_linked = self.api.tunnel.is_linked # --- CRITICAL DNS DISPLAY LOGIC (DO NOT TOUCH!) --- # DO NOT MODIFY: this section is the final link in the DNS recovery chain. @@ -1403,7 +1360,7 @@ def _link_with_setup_code(self): Toast.show(self, "Verifying code with Playit...", toast_type="info") def _link_task(): - success = self.zbb_manager.link_playit_manually(code) + success = self.api.tunnel.link(code) self.after(0, lambda: self.btn_link_code.configure(state="normal")) if success: self.after(0, lambda: self.entry_setup_code.delete(0, 'end')) @@ -1417,17 +1374,12 @@ def load_existing_server_action(self): folder = ctk.filedialog.askdirectory(title="Select Existing Server Folder") if not folder: return - # Quick validation: check for any .jar try: - files = os.listdir(folder) - jars = [f for f in files if f.endswith(".jar")] - if not jars: - Toast.show(self, "No .jar file found in the selected folder.", toast_type="error") - return - - if self.zbb_manager.load_server_manually(folder): + if self.api.servers.import_folder(folder): self.load_servers() # Refresh list Toast.show(self, "Server loaded successfully!", toast_type="success") + except NoServerJarError: + Toast.show(self, "No .jar file found in the selected folder.", toast_type="error") except Exception as e: Toast.show(self, f"Failed to load server: {e}", toast_type="error") @@ -1436,10 +1388,9 @@ def load_existing_server_action(self): # hide to the system tray or exit; exit_app() really ends the process. # ------------------------------------------------------------------ def on_close(self): - from app.services.settings_manager import SettingsManager from app.ui.tray import CLOSE_ASK, CLOSE_TRAY, resolve_close_action, tray_supported - settings = SettingsManager() + settings = self.api.settings action = resolve_close_action(settings.get("close_action", CLOSE_ASK), tray_supported()) if action == CLOSE_ASK: from app.ui.close_dialog import CloseChoiceDialog @@ -1457,12 +1408,11 @@ def on_close(self): def _running_summary(self) -> list: """What exiting would stop, for the close dialog.""" running = [] - if self.zbb_manager.is_running(): - running.append(f"the server '{self.zbb_manager.current_server}'") - playit = getattr(self.zbb_manager, "playit_manager", None) - if playit is not None and getattr(playit, "running", False): + if self.api.servers.is_running(): + running.append(f"the server '{self.api.servers.active}'") + if self.api.tunnel.is_running: running.append("the Playit tunnel") - if getattr(self, "_active_provision_cancels", None): + if self.api.servers.creations(): running.append("the server being created") return running @@ -1507,8 +1457,7 @@ def exit_app(self): # SettingsManager debounces writes 500ms — a change right before # close (e.g. toggling a webhook event in Settings) could otherwise # never reach disk since nothing was waiting for that timer to fire. - from app.services.settings_manager import SettingsManager - SettingsManager().save() + self.api.settings.save() if self._status_tick_job is not None: self.after_cancel(self._status_tick_job) @@ -1522,8 +1471,7 @@ def exit_app(self): def _do_shutdown(): try: - self.zbb_manager.shutdown() - self._kill_orphan_processes() + self.api.shutdown() finally: if hasattr(self, '_instance_lock'): self._instance_lock.release() @@ -1532,29 +1480,6 @@ def _do_shutdown(): threading.Thread(target=_do_shutdown, daemon=True, name="AppShutdown").start() self._poll_shutdown(deadline=time.monotonic() + 8.0) - def _kill_orphan_processes(self): - """Force-kill any subprocesses still alive after graceful shutdown.""" - # Best-effort last resort during app exit: a kill failure here is - # unactionable (the Job Object reaps survivors when the app dies). - runner = getattr(self.zbb_manager, 'server_runner', None) - if runner: - proc = getattr(runner, 'process', None) - if proc and proc.poll() is None: - try: - proc.kill() - proc.wait(timeout=2) - except Exception: - pass - pm = getattr(self.zbb_manager, 'playit_manager', None) - if pm: - proc = getattr(pm, 'process', None) - if proc and proc.poll() is None: - try: - proc.kill() - proc.wait(timeout=2) - except Exception: - pass - def _poll_shutdown(self, deadline: float): if self._shutdown_event.is_set() or time.monotonic() >= deadline: self.destroy() diff --git a/app/ui/modrinth_browser.py b/app/ui/modrinth_browser.py index 242ac75..958f5d5 100644 --- a/app/ui/modrinth_browser.py +++ b/app/ui/modrinth_browser.py @@ -8,26 +8,18 @@ import customtkinter as ctk import hashlib -import io import logging import os -import shutil -import tempfile import threading from concurrent.futures import ThreadPoolExecutor from typing import Callable, Optional -from PIL import Image - +from app.api.mods import (MOD_LOADERS, PLUGIN_LOADERS, ModDownloadError, ModrinthException, ModsApi, + MrpackCompatibilityError, NoCompatibleVersionError, UnsafePathError, + filter_updates_for_selection, loader_display) from app.core.app_config import AppConfig from app.core import i18n from app.core.i18n import N_, format_count -from app.core.constants import BASE_DIR, SERVERS_DIR -from app.services.modrinth import (MOD_LOADERS, PLUGIN_LOADERS, ModrinthClient, ModrinthException, - loader_display) -from app.services.mrpack_installer import install_mrpack, MrpackCompatibilityError -from app.services import mod_install_tracker -from app.core.logic import get_server_meta from app.ui.toast import Toast from app.ui.ui_components import ( ToolTip, ZBBDialog, ZBBToplevel, dialog_buttons, dialog_header, ScrollableFrame, themed_menu, @@ -48,11 +40,6 @@ _ICONS_IN_FLIGHT: set[str] = set() _ICONS_LOCK = threading.Lock() _ICON_EXECUTOR = ThreadPoolExecutor(max_workers=4, thread_name_prefix="modrinth-icon") -_ICON_DISK_DIR = BASE_DIR / ".zbb_cache" / "modrinth_icons" - - -def _icon_disk_path(icon_url: str): - return _ICON_DISK_DIR / (hashlib.md5(icon_url.encode()).hexdigest() + ".png") def _store_icon(icon_url: str, ctk_img: "ctk.CTkImage") -> None: @@ -69,39 +56,6 @@ def _store_icon(icon_url: str, ctk_img: "ctk.CTkImage") -> None: _RENDER_BATCH_SIZE = 2 -def _filter_updates_for_selection(updates: list, selected_filenames: set) -> list: - """Return the subset of check_updates() results matching selected filenames.""" - return [u for u in updates if u.get("filename") in selected_filenames] - - -def _snapshot_then_apply(create_snapshot: Optional[Callable], server_name: str, - updates: list, apply_one: Callable[[dict], bool]) -> tuple[Optional[str], int, int]: - """Snapshot the server, then apply each update in order. - - Returns (snapshot_error, updated, failed). When the snapshot fails no - update is applied — updating without a rollback point is exactly the - false confidence this flow exists to prevent. - """ - if create_snapshot is None: - return "Snapshot service unavailable.", 0, 0 - path, error = create_snapshot(server_name) - if not path: - return error or "Unknown snapshot error.", 0, 0 - - updated, failed = 0, 0 - for update in updates: - try: - ok = apply_one(update) - except Exception as exc: - logger.warning("Update failed for %s: %s", update.get("filename"), exc) - ok = False - if ok: - updated += 1 - else: - failed += 1 - return None, updated, failed - - class ModrinthBrowser(ctk.CTkFrame): """ Self-contained Modrinth browsing panel. @@ -149,7 +103,7 @@ def bundle_summary(loader: str) -> str: ) def __init__(self, master, get_server_info: Callable = None, - create_snapshot: Callable = None, is_server_running: Callable = None, + mods: Optional[ModsApi] = None, is_server_running: Callable = None, on_installed_count: Callable = None, **kwargs): """ Args: @@ -157,9 +111,9 @@ def __init__(self, master, get_server_info: Callable = None, get_server_info: Callable returning (server_name, mc_version, loader) for the currently selected server. If None, search still works but install is disabled. - create_snapshot: Blocking callable (server_name) -> (path, error) - that takes a pre-update snapshot. If None, mod - updates are refused (no rollback point). + mods: The API's mods area (search, installs, file deletes, icons). + Its pre-update snapshot guards every update; a + ModsApi without one refuses updates. is_server_running: Callable () -> bool for the currently selected server. If None, running-server checks are skipped (matches prior behavior for any embedder that @@ -167,10 +121,9 @@ def __init__(self, master, get_server_info: Callable = None, """ super().__init__(master, fg_color="transparent", **kwargs) self.get_server_info = get_server_info - self.create_snapshot = create_snapshot + self.mods = mods or ModsApi() self.is_server_running = is_server_running self.on_installed_count = on_installed_count - self.client = ModrinthClient() # Search state self._search_query = "" @@ -750,7 +703,7 @@ def _do_search(self): def _search(): try: - results = self.client.search( + results = self.mods.search( self._search_query, mc_version=self._search_mc_version, loader=self._search_loader, @@ -868,7 +821,7 @@ def _load_popular_mods(self): def _do_load(): try: - results = self.client.search( + results = self.mods.search( "", mc_version=mc_version, loader=loader, project_type="mod", limit=_PAGE_SIZE, index="downloads", ) @@ -1006,7 +959,7 @@ def _render_results(self): ctx = self._resolve_server_context() self._installed_slugs_cache = ( - mod_install_tracker.get_installed_slugs(ctx[0]) if ctx else set() + self.mods.installed_slugs(ctx[0]) if ctx else set() ) self._render_cards_batch(list(enumerate(self._current_hits)), gen) @@ -1218,7 +1171,7 @@ def _update_from_card(self, update: dict): self._warn_server_running() return fname = update.get("filename", "") - fpath = next((f for f in self._installed_jar_files(ctx[0]) if os.path.basename(f) == fname), None) + fpath = next((f for f in self.mods.installed_files(ctx[0]) if os.path.basename(f) == fname), None) if fpath: self._apply_single_update(fpath, update) @@ -1273,24 +1226,16 @@ def _toggle_installed_view(self): # Installed view can have changed their state. ctx = self._resolve_server_context() self._installed_slugs_cache = ( - mod_install_tracker.get_installed_slugs(ctx[0]) if ctx else set() + self.mods.installed_slugs(ctx[0]) if ctx else set() ) self._refresh_installed_state() - @staticmethod - def _installed_jar_files(server_name: str) -> list: - files = [] - for d in (os.path.join(SERVERS_DIR, server_name, "mods"), os.path.join(SERVERS_DIR, server_name, "plugins")): - if os.path.isdir(d): - files.extend(os.path.join(d, f) for f in sorted(os.listdir(d)) if f.endswith(".jar")) - return files - def _refresh_installed_counter(self, check_updates: bool = False): """Installed (N) on the toggle button, plus an amber dot when the background check found updates. check_updates hits Modrinth, so it only runs when the target server changes.""" ctx = self._resolve_server_context() - self._installed_count = len(self._installed_jar_files(ctx[0])) if ctx else 0 + self._installed_count = len(self.mods.installed_files(ctx[0])) if ctx else 0 self._update_installed_button() if not (check_updates and ctx and ctx[2] and self._installed_count): return @@ -1298,7 +1243,7 @@ def _refresh_installed_counter(self, check_updates: bool = False): def _worker(): try: - updates = self.client.check_updates(ctx[0], ctx[1], ctx[2]) + updates = self.mods.check_updates(ctx[0], ctx[1], ctx[2]) except Exception as exc: logger.debug("Background update count failed: %s", exc) return @@ -1345,7 +1290,7 @@ def _render_installed(self, behind: bool = False): self.installed_frame._parent_canvas.yview_moveto(0) ctx = self._resolve_server_context() - files = self._installed_jar_files(ctx[0]) if ctx else [] + files = self.mods.installed_files(ctx[0]) if ctx else [] self._installed_files = files self._installed_count = len(files) has_files = bool(files) @@ -1391,7 +1336,7 @@ def _render_installed(self, behind: bool = False): def _check_updates_for_badges(self, server_name: str, mc_version: str, loader: str, gen: int): def _worker(): try: - updates = self.client.check_updates(server_name, mc_version, loader) + updates = self.mods.check_updates(server_name, mc_version, loader) except Exception as exc: logger.debug("Background update check failed: %s", exc) return @@ -1433,13 +1378,10 @@ def _apply_single_update(self, fpath: str, update: dict, badge=None): badge.configure(state="disabled", text="Updating…") self._set_status(f"Creating snapshot before updating {fname}…", busy=True) - def _apply(u): - self.after(0, lambda: self._set_status(f"Updating {fname}…", busy=True)) - return self.client.apply_update(u, server_name, loader) - def _worker(): - error, updated, _failed = _snapshot_then_apply( - self.create_snapshot, server_name, [update], _apply) + error, updated, _failed = self.mods.apply_updates( + server_name, loader, [update], + on_update=lambda _u: self.after(0, lambda: self._set_status(f"Updating {fname}…", busy=True))) if error: # Nothing was touched — leave _pending_updates intact so the # badge still reflects a real, retryable update offer. @@ -1548,26 +1490,17 @@ def _confirm_uninstall_mod(self, slug: str, title: str): self._warn_server_running() return server_name = ctx[0] - fname = mod_install_tracker.get_installed_filename(server_name, slug) - if not fname: + if not self.mods.installed_filename(server_name, slug): return - filepath = None - for subdir in ("mods", "plugins"): - candidate = os.path.join(SERVERS_DIR, server_name, subdir, fname) - if os.path.isfile(candidate): - filepath = candidate - break if not ZBBDialog.confirm(self.winfo_toplevel(), "Uninstall Mod", f"Uninstall '{title}'?", confirm_text="Uninstall", danger=True): return try: - if filepath: - os.remove(filepath) - mod_install_tracker.remove_install(server_name, slug) + self.mods.uninstall(server_name, slug) self._set_status(f"Uninstalled {title}", kind="success") self._installed_slugs_cache.discard(slug) self._refresh_installed_state() - except OSError as exc: + except (OSError, UnsafePathError) as exc: self._set_status(f"Failed to uninstall {title}: {exc}", kind="error") def _confirm_delete_mod(self, filepath: str): @@ -1578,11 +1511,14 @@ def _confirm_delete_mod(self, filepath: str): if not ZBBDialog.confirm(self.winfo_toplevel(), "Delete Mod", f"Delete '{fname}'?", confirm_text="Delete", danger=True): return + ctx = self._resolve_server_context() + if not ctx: + return try: - os.remove(filepath) + self.mods.delete_file(ctx[0], filepath) self._set_status(f"Deleted {fname}", kind="success") self._render_installed() # refresh inline - except OSError as exc: + except (OSError, UnsafePathError) as exc: self._set_status(f"Failed to delete {fname}: {exc}", kind="error") def _on_delete_selected(self): @@ -1599,26 +1535,11 @@ def _on_delete_selected(self): return ctx = self._resolve_server_context() - server_name = ctx[0] if ctx else None - filename_to_slug = {} - if server_name: - for slug in mod_install_tracker.get_installed_slugs(server_name): - filename_to_slug[mod_install_tracker.get_installed_filename(server_name, slug)] = slug - - deleted, failed = 0, 0 - for fpath in selected: - try: - fname = os.path.basename(fpath) - os.remove(fpath) - if server_name: - mod_install_tracker.remove_install_by_filename(server_name, fname) - slug = filename_to_slug.get(fname) - if slug: - self._installed_slugs_cache.discard(slug) - deleted += 1 - except OSError as exc: - logger.warning("Failed to delete %s: %s", fpath, exc) - failed += 1 + if not ctx: + return + result = self.mods.delete_files(ctx[0], selected) + self._installed_slugs_cache -= result.removed_slugs + deleted, failed = len(result.deleted), len(result.failed) self._set_status(f"Deleted {deleted} file(s)" + (f", {failed} failed" if failed else ""), kind="success") self._render_installed() @@ -1636,22 +1557,20 @@ def _on_update_selected(self): def _worker(): try: - updates = self.client.check_updates(server_name, mc_version, loader) + updates = self.mods.check_updates(server_name, mc_version, loader) except Exception as exc: self.after(0, lambda e=exc: self._set_status(f"Update check failed: {e}", kind="error")) return - matched = _filter_updates_for_selection(updates, selected_filenames) + matched = filter_updates_for_selection(updates, selected_filenames) if not matched: self.after(0, lambda: self._set_status("No updates available for the selected mods.")) return - def _apply(u): - self.after(0, lambda: self._set_status(f"Updating {u['filename']}…")) - return self.client.apply_update(u, server_name, loader) - self.after(0, lambda: self._set_status("Creating snapshot before updating…", busy=True)) - error, updated, failed = _snapshot_then_apply(self.create_snapshot, server_name, matched, _apply) + error, updated, failed = self.mods.apply_updates( + server_name, loader, matched, + on_update=lambda u: self.after(0, lambda: self._set_status(f"Updating {u['filename']}…"))) if error: self.after(0, lambda: self._set_status(f"Updates aborted — {error}", kind="error")) return @@ -1679,7 +1598,7 @@ def _on_install(self, hit: dict, batch: dict = None): def _fetch_versions(): try: - versions = self.client.get_versions(slug, mc_version=mc_version, loader=loader) + versions = self.mods.get_versions(slug, mc_version=mc_version, loader=loader) if not versions: self.after(0, lambda: self._set_status(f"No compatible versions of {title} found.", kind="error")) self.after(0, lambda: self._note_batch_result(batch, ok=False)) @@ -1706,18 +1625,16 @@ def _do_install_version(self, version, server_name, loader, title, slug, def _install_one(version, server_name, loader, title, slug) -> bool: """Download+record a single mod version. Returns True on success.""" - path = self.client.download_version( - version, server_name, loader, - progress_callback=lambda p: self.after( + path = self.mods.install_version( + version, server_name, loader, slug, + progress=lambda p: self.after( 0, lambda: self._set_status(f"Downloading {title}… {int(p * 100)}%") ), ) if not path: return False - fname = os.path.basename(path) - mod_install_tracker.record_install(server_name, slug, fname) - def _mark_installed(slug=slug, fname=fname): + def _mark_installed(slug=slug): self._installed_slugs_cache.add(slug) self.after(0, _mark_installed) @@ -1753,7 +1670,7 @@ def _resolve_and_install_dependencies(self, version, server_name, loader, mc_ver via an Event so the primary install's batch/status bookkeeping happens after dependency handling completes.""" try: - dep_result = self.client.get_required_dependencies( + dep_result = self.mods.required_dependencies( version, mc_version, loader, self._installed_slugs_cache) except Exception as exc: logger.debug("Dependency resolution failed for %s: %s", title, exc) @@ -1805,16 +1722,13 @@ def _ask(): dep_slug = dep_project.get("slug", dep_project.get("id", "")) dep_title = dep_project.get("title", dep_slug) try: - dep_path = self.client.download_version( - dep_version, server_name, loader, - progress_callback=lambda p, t=dep_title: self.after( + dep_path = self.mods.install_version( + dep_version, server_name, loader, dep_slug, + progress=lambda p, t=dep_title: self.after( 0, lambda: self._set_status(f"Downloading {t}… {int(p * 100)}%") ), ) if dep_path: - dep_fname = os.path.basename(dep_path) - mod_install_tracker.record_install(server_name, dep_slug, dep_fname) - def _mark(dep_slug=dep_slug): self._installed_slugs_cache.add(dep_slug) @@ -1836,7 +1750,7 @@ def _on_install_modpack(self, hit: dict, batch: dict = None): def _fetch_versions(): try: - versions = self.client.get_versions(slug, mc_version=mc_version, loader=loader) + versions = self.mods.get_versions(slug, mc_version=mc_version, loader=loader) if not versions: self.after(0, lambda: self._set_status(f"No compatible versions of {title} found.", kind="error")) self.after(0, lambda: self._note_batch_result(batch, ok=False)) @@ -1859,30 +1773,21 @@ def _do_install_modpack_version(self, version, server_name, title, batch: dict = self._set_status(f"Installing modpack {title}…", busy=True) def _install(): - tmp_dir = tempfile.mkdtemp(prefix="zbb_modpack_") try: - downloaded = self.client.download_version_to( - version, tmp_dir, - progress_callback=lambda p: self.after( + summary = self.mods.install_modpack( + version, server_name, + on_download=lambda p: self.after( 0, lambda: self._set_status(f"Downloading {title}… {int(p * 100)}%") ), - ) - if not downloaded: - self.after(0, lambda: self._set_status(f"Download failed for {title}.", kind="error")) - self.after(0, lambda: self._note_batch_result(batch, ok=False)) - return - - meta = get_server_meta(server_name) - summary = install_mrpack( - downloaded, server_name, - progress_callback=lambda msg: self.after(0, lambda m=msg: self._set_status(m)), - server_type=meta.get("type"), - mc_version=meta.get("version"), + on_status=lambda msg: self.after(0, lambda m=msg: self._set_status(m)), ) self.after(0, lambda: self._set_status( f"Modpack {title}: {self._format_mrpack_summary(summary)}", kind="success")) logger.info("Installed modpack %s to server %s", title, server_name) self.after(0, lambda: self._note_batch_result(batch, ok=True)) + except ModDownloadError: + self.after(0, lambda: self._set_status(f"Download failed for {title}.", kind="error")) + self.after(0, lambda: self._note_batch_result(batch, ok=False)) except MrpackCompatibilityError as exc: self.after(0, lambda e=exc: self._set_status(f"Incompatible: {e}", kind="error")) self.after(0, lambda e=exc: ZBBDialog.info( @@ -1891,8 +1796,6 @@ def _install(): except Exception as exc: self.after(0, lambda e=exc: self._set_status(f"Modpack install failed: {e}", kind="error")) self.after(0, lambda: self._note_batch_result(batch, ok=False)) - finally: - shutil.rmtree(tmp_dir, ignore_errors=True) threading.Thread(target=_install, daemon=True).start() @@ -1961,15 +1864,10 @@ def _run_opt(): for mod in bundle: self.after(0, lambda m=mod: self._set_status(f"Installing {m['name']}...")) try: - versions = self.client.get_versions(mod["slug"], mc_version=mc_version, loader=loader) - if not versions: - failed.append(f"{mod['name']} (no build for MC {mc_version} yet)") - continue - path = self.client.download_mod(mod["slug"], server_name, mc_version, loader) - if not path: + if not self.mods.install_latest(mod["slug"], server_name, mc_version, loader): failed.append(mod["name"]) - else: - mod_install_tracker.record_install(server_name, mod["slug"], os.path.basename(path)) + except NoCompatibleVersionError: + failed.append(f"{mod['name']} (no build for MC {mc_version} yet)") except Exception as exc: logger.error("Failed to install %s: %s", mod["name"], exc) failed.append(mod["name"]) @@ -1999,7 +1897,7 @@ def _on_check_updates(self): def _check(): try: - updates = self.client.check_updates(server_name, mc_version, loader) + updates = self.mods.check_updates(server_name, mc_version, loader) self.after(0, lambda: self._show_updates_dialog(updates)) except Exception as exc: self.after(0, lambda e=exc: self._set_status(f"Update check failed: {e}", kind="error")) @@ -2052,15 +1950,9 @@ def _queue_icon_fetch(self, icon_url: str, icon_frame, lbl_initial): _ICONS_IN_FLIGHT.add(icon_url) _ICON_EXECUTOR.submit(self._load_icon, icon_url, icon_frame, lbl_initial) - @staticmethod - def _icon_from_disk(icon_url: str) -> Optional[ctk.CTkImage]: - disk_path = _icon_disk_path(icon_url) - if not disk_path.exists(): - return None - try: - img = Image.open(disk_path) - img.load() - except OSError: + def _icon_from_disk(self, icon_url: str) -> Optional[ctk.CTkImage]: + img = self.mods.cached_icon(icon_url) + if img is None: return None ctk_img = ctk.CTkImage(img, size=(48, 48)) _store_icon(icon_url, ctk_img) @@ -2068,18 +1960,11 @@ def _icon_from_disk(icon_url: str) -> Optional[ctk.CTkImage]: def _load_icon(self, icon_url: str, icon_frame, lbl_initial): try: - ctk_img = self._icon_from_disk(icon_url) - if ctk_img is None: - resp = self.client.session.get(icon_url, timeout=8) - resp.raise_for_status() - img = Image.open(io.BytesIO(resp.content)).resize((48, 48), Image.LANCZOS) - try: - _ICON_DISK_DIR.mkdir(parents=True, exist_ok=True) - img.save(_icon_disk_path(icon_url)) - except OSError as exc: - logger.debug("Icon disk cache write failed: %s", exc) - ctk_img = ctk.CTkImage(img, size=(48, 48)) - _store_icon(icon_url, ctk_img) + img = self.mods.fetch_icon(icon_url) + if img is None: + return + ctk_img = ctk.CTkImage(img, size=(48, 48)) + _store_icon(icon_url, ctk_img) self.after(0, lambda: self._apply_icon(icon_frame, lbl_initial, ctk_img)) except Exception as exc: logger.debug("Image fetch error: %s", exc) diff --git a/app/ui/server_properties_editor.py b/app/ui/server_properties_editor.py index ae81011..1ef8657 100644 --- a/app/ui/server_properties_editor.py +++ b/app/ui/server_properties_editor.py @@ -2,9 +2,8 @@ from tkinter import filedialog import logging import os -import subprocess -import sys import threading +from app.api.storage import open_in_file_manager from app.core.app_config import AppConfig from app.core import i18n from app.core.constants import SERVERS_DIR @@ -17,6 +16,7 @@ from app.ui.icons import icon from app.services.backup_manager import BackupManager from app.services.server_properties import load_server_properties, save_server_properties, list_worlds +from app.core import logic from app.core.logic import BackupScheduler, get_server_meta, update_server_meta from app.services.mrpack_installer import install_mrpack, MrpackCompatibilityError @@ -160,7 +160,7 @@ def property_label(key: str) -> str: class ServerPropertiesEditor(ZBBToplevel): - def __init__(self, parent, server_name, logic_module, zbb_manager=None): + def __init__(self, parent, server_name, logic_module=None, zbb_manager=None): super().__init__(parent) self.title(f"Edit Properties - {server_name}") self.geometry("700x600") @@ -168,7 +168,7 @@ def __init__(self, parent, server_name, logic_module, zbb_manager=None): self.resizable(True, True) self.server_name = server_name - self.logic = logic_module + self.logic = logic_module or logic self.zbb_manager = zbb_manager self.properties = load_server_properties(server_name) @@ -1147,12 +1147,7 @@ def _open_path(self, server_path: str): "That folder does not exist yet - it is created the first time it is needed.", kind="info") return - if sys.platform == "win32": - subprocess.run(["explorer", server_path], check=False) - elif sys.platform == "darwin": - subprocess.run(["open", server_path], check=False) - else: - subprocess.run(["xdg-open", server_path], check=False) + open_in_file_manager(server_path) def save_launch_settings(self): meta_path = os.path.join(str(SERVERS_DIR), self.server_name, "metadata.json") diff --git a/tests/test_api_events_tasks.py b/tests/test_api_events_tasks.py new file mode 100644 index 0000000..1c6981b --- /dev/null +++ b/tests/test_api_events_tasks.py @@ -0,0 +1,174 @@ +"""Event delivery through a caller-supplied dispatcher, and task handles +(progress, completion, cancellation) of the API seam.""" + +import concurrent.futures +import threading + +import pytest + +from app.api.events import EventHub, direct +from app.api.tasks import Task, TaskRunner +from app.core.server_events import EventBus, ServerEvent + + +class QueueDispatcher: + """Stands in for a UI main loop: runs nothing until pumped.""" + + def __init__(self): + self.queue = [] + + def __call__(self, fn): + self.queue.append(fn) + + def pump(self): + while self.queue: + self.queue.pop(0)() + + +class TestEventHub: + def test_handler_runs_only_through_the_dispatcher(self): + bus, loop, got = EventBus(), QueueDispatcher(), [] + EventHub(bus, loop).subscribe(ServerEvent.CONSOLE_LINE, got.append) + bus.emit(ServerEvent.CONSOLE_LINE, "a") + bus.emit(ServerEvent.CONSOLE_LINE, "b") + assert got == [] + loop.pump() + assert got == ["a", "b"] + + def test_emitting_thread_never_runs_the_handler(self): + bus, loop, threads = EventBus(), QueueDispatcher(), [] + EventHub(bus, loop).subscribe(ServerEvent.READY, lambda _d: threads.append(threading.current_thread())) + worker = threading.Thread(target=bus.emit, args=(ServerEvent.READY, None)) + worker.start() + worker.join() + loop.pump() + assert threads == [threading.current_thread()] + + def test_cancel_drops_already_queued_calls(self): + bus, loop, got = EventBus(), QueueDispatcher(), [] + sub = EventHub(bus, loop).subscribe(ServerEvent.CONSOLE_LINE, got.append) + bus.emit(ServerEvent.CONSOLE_LINE, "queued") + sub.cancel() + bus.emit(ServerEvent.CONSOLE_LINE, "after") + loop.pump() + assert got == [] + assert not sub.active + sub.cancel() # idempotent + + def test_per_subscription_dispatcher_overrides_the_default(self): + bus, loop, got = EventBus(), QueueDispatcher(), [] + hub = EventHub(bus, loop) + hub.subscribe(ServerEvent.STOPPED, got.append, dispatcher=direct) + bus.emit(ServerEvent.STOPPED, 1) + assert got == [1] and loop.queue == [] + + def test_set_dispatcher_applies_to_later_subscriptions(self): + bus, loop, got = EventBus(), QueueDispatcher(), [] + hub = EventHub(bus) + assert hub.dispatcher is direct + hub.set_dispatcher(loop) + hub.subscribe(ServerEvent.PLAYER_COUNT, got.append) + bus.emit(ServerEvent.PLAYER_COUNT, 3) + assert got == [] + loop.pump() + assert got == [3] + + def test_close_cancels_every_subscription(self): + bus, got = EventBus(), [] + hub = EventHub(bus) + hub.subscribe(ServerEvent.READY, got.append) + hub.subscribe(ServerEvent.STOPPED, got.append) + hub.close() + bus.emit(ServerEvent.READY, 1) + bus.emit(ServerEvent.STOPPED, 2) + assert got == [] + + def test_handler_error_is_contained_by_the_bus(self): + bus, got = EventBus(), [] + hub = EventHub(bus) + hub.subscribe(ServerEvent.READY, lambda _d: 1 / 0) + hub.subscribe(ServerEvent.READY, got.append) + bus.emit(ServerEvent.READY, "ok") + assert got == ["ok"] + + +def _runner(loop=None): + executor = concurrent.futures.ThreadPoolExecutor(max_workers=2) + return TaskRunner(executor, lambda: loop or direct), executor + + +class TestTask: + def test_progress_and_done_go_through_the_dispatcher(self): + loop = QueueDispatcher() + runner, executor = _runner(loop) + progress, done = [], [] + + def job(task): + task.report(0.5, "half") + return "result" + + task = runner.start("job", job).on_progress(lambda *a: progress.append(a)).on_done(done.append) + executor.shutdown(wait=True) + assert progress == [] and done == [] + loop.pump() + assert done == [task] + assert task.result(timeout=1) == "result" + + def test_progress_listener_added_before_report(self): + task, got = Task(), [] + task.on_progress(lambda *a: got.append(a)) + task.report(0.25, "x") + assert got == [(0.25, "x")] + + def test_on_done_after_completion_fires_at_once(self): + task, got = Task(), [] + task._run(lambda t: 42) + task.on_done(got.append) + assert got == [task] and task.done() + + def test_cancel_sets_the_event_the_job_polls(self): + runner, executor = _runner() + started, finished = threading.Event(), [] + + def job(task): + started.set() + task.cancel_event.wait(timeout=5) + finished.append(task.cancel_requested) + return "stopped" + + task = runner.start("create_server", job) + started.wait(timeout=5) + assert runner.active("create_server") == [task] + assert task.cancel() is True + assert task.result(timeout=5) == "stopped" + executor.shutdown(wait=True) + assert finished == [True] + assert runner.active() == [] + + def test_cancel_all_reaches_every_running_task(self): + runner, executor = _runner() + gate = threading.Barrier(3) + + def job(task): + gate.wait(timeout=5) + return task.cancel_event.wait(timeout=5) + + tasks = [runner.start("a", job), runner.start("b", job)] + gate.wait(timeout=5) + runner.cancel_all() + assert [t.result(timeout=5) for t in tasks] == [True, True] + executor.shutdown(wait=True) + + def test_non_cancellable_task_refuses(self): + task = Task(cancellable=False) + assert task.cancel() is False + assert not task.cancel_requested + + def test_job_exception_is_kept_and_done_still_fires(self): + done = [] + task = Task().on_done(done.append) + task._run(lambda t: (_ for _ in ()).throw(RuntimeError("boom"))) + assert done == [task] + assert isinstance(task.exception(timeout=1), RuntimeError) + with pytest.raises(RuntimeError): + task.result(timeout=1) diff --git a/tests/test_api_facade.py b/tests/test_api_facade.py new file mode 100644 index 0000000..266622e --- /dev/null +++ b/tests/test_api_facade.py @@ -0,0 +1,478 @@ +"""AppApi and its areas with a fake manager: no Tk, no network, no server.""" + +import ast +import concurrent.futures +import io +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest +from PIL import Image + +from app.api import AppApi, NoServerJarError, ServerRunningError +from app.api.areas import ConsoleApi, JavaApi, SettingsApi, TunnelApi +from app.api.mods import ModsApi +from app.api.storage import StorageApi, open_in_file_manager +from app.core.provisioning import ProvisionResult +from app.core.server_events import EventBus, ServerEvent +from app.services import mod_icons +from app.services.server_paths import UnsafePathError + +ROOT = Path(__file__).resolve().parent.parent + + +class FakeManager: + def __init__(self, running=False, current=None): + self.running = running + self.current_server = current + self.players = object() + self.selected = [] + self.calls = [] + self.server_runner = None + self.playit_manager = None + + def is_running(self): + return self.running + + def select_server(self, name): + self.selected.append(name) + self.current_server = name + + def start_server(self): + self.calls.append(("start", self.current_server)) + return True + + def stop_server(self): + self.calls.append(("stop",)) + + def send_command(self, cmd): + self.calls.append(("cmd", cmd)) + + def server_memory_usage(self): + return 1024 if self.running else None + + def load_server_manually(self, folder): + self.calls.append(("import", folder)) + return True + + def provision_server(self, config, progress, cancel_event): + progress(0.5, "half") + return ProvisionResult(ok=True, name=config["name"]) + + def create_pre_update_snapshot(self, name): + return Path("snap.zip"), None + + def delete_backup(self, name, path): + return True, None + + def bootstrap(self): + self.calls.append(("bootstrap",)) + + def shutdown(self): + self.calls.append(("shutdown",)) + + +@pytest.fixture +def servers_dir(tmp_path): + root = tmp_path / "servers" + with patch("app.api.servers.SERVERS_DIR", str(root)), \ + patch("app.api.storage.SERVERS_DIR", str(root)), \ + patch("app.core.logic.SERVERS_DIR", str(root)): + yield root + + +def make_api(manager=None, **kw): + executor = concurrent.futures.ThreadPoolExecutor(max_workers=2) + api = AppApi(EventBus(), manager or FakeManager(), executor=executor, + mods=kw.pop("mods", MagicMock()), java=kw.pop("java", MagicMock()), + settings=kw.pop("settings", MagicMock()), **kw) + return api, executor + + +class TestServers: + def test_list_creates_the_folder_and_lists_only_folders(self, servers_dir): + api, _ = make_api() + assert api.servers.list() == [] + (servers_dir / "alpha").mkdir() + (servers_dir / "stray.txt").write_text("x", encoding="utf-8") + assert api.servers.list() == ["alpha"] + assert api.servers.exists("alpha") and not api.servers.exists("beta") + + def test_info_reads_metadata_and_properties(self, servers_dir): + api, _ = make_api() + with patch("app.core.logic.get_server_meta", return_value={"type": "Fabric", "version": "1.21"}), \ + patch("app.api.servers.load_server_properties", return_value={"max-players": "8"}): + info = api.servers.info("alpha") + assert (info.engine, info.mc_version, info.loader, info.max_players) == ("Fabric", "1.21", "fabric", 8) + assert info.mod_context == ("alpha", "1.21", "fabric") + + def test_info_defaults_match_the_old_screens(self, servers_dir): + api, _ = make_api() + with patch("app.core.logic.get_server_meta", return_value={}), \ + patch("app.api.servers.load_server_properties", return_value={"max-players": "lots"}): + info = api.servers.info("alpha") + assert info.engine is None and info.mc_version is None and info.max_players is None + assert info.mod_context == ("alpha", "1.20.1", None) # vanilla: no loader + + def test_select_and_start(self, servers_dir): + manager = FakeManager() + api, _ = make_api(manager) + assert api.servers.select("alpha") is True + assert api.servers.start("beta") is True + assert manager.calls == [("start", "beta")] + assert api.servers.active == "beta" + + def test_nothing_changes_while_a_server_runs(self, servers_dir): + manager = FakeManager(running=True, current="alpha") + api, _ = make_api(manager) + assert api.servers.select("beta") is False + assert api.servers.start("beta") is False + assert manager.selected == [] and manager.calls == [] + assert api.servers.running_server() == "alpha" + + def test_delete_refuses_the_running_server(self, servers_dir): + (servers_dir / "alpha").mkdir(parents=True) + api, _ = make_api(FakeManager(running=True, current="alpha")) + with pytest.raises(ServerRunningError): + api.servers.delete("alpha") + assert (servers_dir / "alpha").exists() + + def test_delete_removes_folder_and_clears_the_active_server(self, servers_dir): + (servers_dir / "alpha" / "world").mkdir(parents=True) + manager = FakeManager(current="alpha") + api, _ = make_api(manager) + api.servers.delete("alpha") + assert not (servers_dir / "alpha").exists() + assert manager.current_server is None + + @pytest.mark.parametrize("name", ["..", ".", "", "../servers", "a/b"]) + def test_delete_never_leaves_the_servers_folder(self, servers_dir, name): + (servers_dir / "keep").mkdir(parents=True) + api, _ = make_api() + with pytest.raises(UnsafePathError): + api.servers.delete(name) + assert (servers_dir / "keep").exists() + + def test_import_folder_needs_a_jar(self, servers_dir, tmp_path): + manager = FakeManager() + api, _ = make_api(manager) + empty = tmp_path / "empty" + empty.mkdir() + with pytest.raises(NoServerJarError): + api.servers.import_folder(str(empty)) + (empty / "server.jar").write_bytes(b"x") + assert api.servers.import_folder(str(empty)) is True + assert manager.calls == [("import", str(empty))] + + def test_export_and_import_pack_delegate(self, servers_dir): + api, _ = make_api() + with patch("app.api.servers.migration") as migration: + api.servers.export("alpha", "out.zbbpack") + api.servers.import_pack("in.zbbpack", "beta") + migration.export_server.assert_called_once_with("alpha", "out.zbbpack") + migration.import_server.assert_called_once_with("in.zbbpack", "beta") + with pytest.raises(UnsafePathError): + api.servers.import_pack("in.zbbpack", "../beta") + + def test_memory_sample(self, servers_dir): + api, _ = make_api(FakeManager(running=True, current="alpha")) + with patch("app.core.logic.get_server_ram", return_value=4096): + sample = api.servers.memory() + assert (sample.used_bytes, sample.limit_mb) == (1024, 4096) + api, _ = make_api(FakeManager()) + assert api.servers.memory().limit_mb is None + + def test_update_meta_and_mod_install_request(self, servers_dir): + api, _ = make_api() + got = [] + api.bus.subscribe(ServerEvent.REQUEST_MOD_INSTALL, got.append) + with patch("app.core.logic.update_server_meta", return_value=True) as update: + assert api.servers.update_meta("alpha", {"ram": 2048}) + update.assert_called_once_with("alpha", {"ram": 2048}) + api.servers.request_mod_install("alpha", ("fabric-api",)) + assert got == [{"missing_mod_ids": ["fabric-api"], "server_name": "alpha"}] + + def test_create_reports_progress_and_result(self, servers_dir): + api, executor = make_api() + progress = [] + task = api.servers.create({"name": "new"}).on_progress(lambda *a: progress.append(a)) + result = task.result(timeout=5) + executor.shutdown(wait=True) + assert result.ok and result.name == "new" + assert api.servers.creations() == [] + + def test_create_turns_a_crash_into_a_failed_result(self, servers_dir): + manager = FakeManager() + manager.provision_server = MagicMock(side_effect=RuntimeError("disk gone")) + api, executor = make_api(manager) + result = api.servers.create({"name": "new"}).result(timeout=5) + executor.shutdown(wait=True) + assert not result.ok and result.error == "disk gone" and result.name == "new" + + def test_create_passes_the_cancel_event(self, servers_dir): + manager = FakeManager() + seen = [] + + def provision(config, progress, cancel_event): + cancel_event.wait(timeout=5) + seen.append(cancel_event.is_set()) + return ProvisionResult(ok=False, name="new", cancelled=True) + manager.provision_server = provision + api, executor = make_api(manager) + task = api.servers.create({"name": "new"}) + api.tasks.cancel_all() + assert task.result(timeout=5).cancelled + executor.shutdown(wait=True) + assert seen == [True] + + +class TestSmallAreas: + def test_console_only_sends_to_a_running_server(self): + manager = FakeManager() + console = ConsoleApi(manager) + assert console.send("say hi") is False + manager.running = True + assert console.send("") is False + assert console.send("say hi") is True + assert manager.calls == [("cmd", "say hi")] + + def test_tunnel(self): + manager = MagicMock() + manager.link_playit_manually.return_value = 1 + manager.playit_manager = SimpleNamespace(is_linked=True, running=False) + tunnel = TunnelApi(manager) + tunnel.start() + tunnel.stop() + tunnel.reset() + assert tunnel.link("code") is True + assert tunnel.is_linked and not tunnel.is_running + manager.reset_tunnel.assert_called_once_with(mode="soft") + assert not TunnelApi(SimpleNamespace(playit_manager=None)).is_linked + assert not TunnelApi(SimpleNamespace(playit_manager=None)).is_running + + def test_java_best(self): + found = SimpleNamespace(major=21, source="PORTABLE", version_string="21.0.4") + java = JavaApi(lambda: SimpleNamespace(detect_all=lambda: [found])) + best = java.best() + assert (best.major, best.portable, best.version_string) == (21, True, "21.0.4") + assert JavaApi(lambda: SimpleNamespace(detect_all=lambda: [])).best() is None + + def test_settings(self): + store = MagicMock() + store.get.return_value = "Dark" + settings = SettingsApi(store) + assert settings.get("theme", "Light") == "Dark" + settings.set("theme", "Light") + settings.save() + store.set.assert_called_once_with("theme", "Light") + store.save.assert_called_once() + + def test_backups_delegate(self): + api, _ = make_api() + assert api.backups.create_snapshot("alpha") == (Path("snap.zip"), None) + assert api.backups.delete("alpha", "x.zip") == (True, None) + + +class TestStorage: + @pytest.mark.parametrize("platform, command", [ + ("win32", "explorer"), ("darwin", "open"), ("linux", "xdg-open")]) + def test_one_opener_per_platform(self, tmp_path, platform, command): + calls = [] + assert open_in_file_manager(tmp_path, platform=platform, run=lambda c, check: calls.append(c)) + assert calls == [[command, str(tmp_path)]] + + def test_missing_folder_is_not_opened_unless_created(self, tmp_path): + calls = [] + target = tmp_path / "new" + assert not open_in_file_manager(target, run=lambda c, check: calls.append(c)) + assert calls == [] + assert open_in_file_manager(target, create=True, run=lambda c, check: calls.append(c)) + assert target.is_dir() and len(calls) == 1 + + def test_launch_failure_is_reported(self, tmp_path): + def boom(cmd, check): + raise FileNotFoundError(cmd[0]) + assert open_in_file_manager(tmp_path, run=boom) is False + + def test_cannot_create(self, tmp_path): + blocker = tmp_path / "file" + blocker.write_text("x", encoding="utf-8") + assert open_in_file_manager(blocker / "sub", create=True, run=lambda c, check: None) is False + + def test_server_paths(self, servers_dir): + storage = StorageApi() + assert storage.server_dir("alpha") == servers_dir / "alpha" + assert storage.server_dir("alpha", "backups") == servers_dir / "alpha" / "backups" + assert storage.ensure_servers_dir() == servers_dir and servers_dir.is_dir() + assert storage.servers_dir == servers_dir + assert storage.data_dir and storage.logs_dir + with pytest.raises(UnsafePathError): + storage.server_dir("..") + with patch("app.api.storage.open_in_file_manager", return_value=True) as opener: + assert storage.open_folder(servers_dir) + opener.assert_called_once_with(servers_dir, create=False) + + +class TestAppApi: + def test_dispatcher_reaches_events_and_tasks(self): + api, executor = make_api() + queued = [] + api.set_dispatcher(queued.append) + got = [] + api.events.subscribe(ServerEvent.READY, got.append) + api.bus.emit(ServerEvent.READY, 1) + task = api.servers.create({"name": "x"}).on_progress(lambda *a: got.append(a)) + task.result(timeout=5) + executor.shutdown(wait=True) + assert got == [] + for fn in queued: + fn() + assert 1 in got + + def test_lifecycle_and_orphan_kill(self): + manager = FakeManager() + alive, dead, stubborn = MagicMock(), MagicMock(), MagicMock() + alive.poll.return_value = None + dead.poll.return_value = 0 + stubborn.poll.return_value = None + stubborn.kill.side_effect = OSError("access denied") + manager.server_runner = SimpleNamespace(process=alive) + manager.playit_manager = SimpleNamespace(process=stubborn) + api, _ = make_api(manager) + assert api.legacy_manager is manager and api.players is manager.players + api.start_background_services() + api.shutdown() # the stubborn process doesn't stop the shutdown + assert manager.calls == [("bootstrap",), ("shutdown",)] + alive.kill.assert_called_once() + dead.kill.assert_not_called() + + def test_default_construction_builds_the_real_manager(self): + with patch("app.core.core.ZBBManager") as manager_cls: + api = AppApi() + manager_cls.assert_called_once_with(api.bus) + assert isinstance(api.mods, ModsApi) + + +class TestModsApi: + def test_delegates_to_the_client(self): + client = MagicMock() + mods = ModsApi(client_factory=lambda: client) + mods.search("sodium", limit=5) + mods.get_versions("sodium", mc_version="1.21", loader="fabric") + mods.required_dependencies({"id": "v"}, "1.21", "fabric", {"x"}) + mods.check_updates("srv", "1.21", "fabric") + client.search.assert_called_once_with("sodium", limit=5) + client.get_versions.assert_called_once_with("sodium", mc_version="1.21", loader="fabric") + client.get_required_dependencies.assert_called_once_with({"id": "v"}, "1.21", "fabric", {"x"}) + client.check_updates.assert_called_once_with("srv", "1.21", "fabric") + assert mods.client is client # created once + + def test_apply_updates_snapshots_first(self): + client, order = MagicMock(), [] + client.apply_update.side_effect = lambda u, s, l: order.append(("apply", u["filename"])) or True + + def snapshot(name): + order.append(("snapshot", name)) + return Path("s.zip"), None + mods = ModsApi(client_factory=lambda: client, create_snapshot=snapshot) + seen = [] + assert mods.apply_updates("srv", "fabric", [{"filename": "a.jar"}], on_update=seen.append) == (None, 1, 0) + assert order == [("snapshot", "srv"), ("apply", "a.jar")] + assert seen == [{"filename": "a.jar"}] + + def test_without_a_snapshot_nothing_is_updated(self): + client = MagicMock() + error, updated, _ = ModsApi(client_factory=lambda: client).apply_updates("srv", "fabric", [{"filename": "a"}]) + assert error and updated == 0 + client.apply_update.assert_not_called() + + def test_file_operations_delegate_to_the_service(self): + mods = ModsApi(client_factory=MagicMock) + with patch("app.api.mods.mod_content") as content, patch("app.api.mods.mod_install_tracker") as tracker: + mods.installed_files("srv") + mods.installed_slugs("srv") + mods.installed_filename("srv", "x") + mods.uninstall("srv", "x") + mods.delete_file("srv", "p") + mods.delete_files("srv", ["p"]) + mods.install_version({"id": 1}, "srv", "fabric", "x") + mods.install_latest("x", "srv", "1.21", "fabric") + mods.install_modpack({"id": 1}, "srv") + content.installed_jar_files.assert_called_once_with("srv") + content.uninstall.assert_called_once_with("srv", "x") + content.delete_content_file.assert_called_once_with("srv", "p") + content.delete_content_files.assert_called_once_with("srv", ["p"]) + assert content.install_version.called and content.install_latest.called and content.install_modpack.called + tracker.get_installed_slugs.assert_called_once_with("srv") + tracker.get_installed_filename.assert_called_once_with("srv", "x") + + +def _png_bytes(): + buf = io.BytesIO() + Image.new("RGBA", (128, 128), (255, 0, 0, 255)).save(buf, "PNG") + return buf.getvalue() + + +class TestIcons: + def test_downloads_scales_and_caches(self, tmp_path): + session = MagicMock() + session.get.return_value = SimpleNamespace(content=_png_bytes(), raise_for_status=lambda: None) + with patch("app.services.mod_icons.BASE_DIR", tmp_path): + mods = ModsApi(client_factory=lambda: SimpleNamespace(session=session)) + img = mods.fetch_icon("https://cdn.modrinth.com/a.png") + assert img.size == (48, 48) + again = mods.fetch_icon("https://cdn.modrinth.com/a.png") + assert again.size == (48, 48) + assert ModsApi.cached_icon("https://cdn.modrinth.com/a.png") is not None + session.get.assert_called_once() # the second came from disk + + def test_only_web_urls_are_fetched(self): + session = MagicMock() + assert mod_icons.fetch_icon("file:///C:/secret.png", session) is None + session.get.assert_not_called() + + def test_unreadable_cache_entry_is_ignored(self, tmp_path): + with patch("app.services.mod_icons.BASE_DIR", tmp_path): + path = mod_icons.icon_disk_path("https://x/y.png") + path.parent.mkdir(parents=True) + path.write_bytes(b"not a png") + assert mod_icons.cached_icon("https://x/y.png") is None + + def test_cache_write_failure_still_returns_the_icon(self, tmp_path): + session = MagicMock() + session.get.return_value = SimpleNamespace(content=_png_bytes(), raise_for_status=lambda: None) + blocker = tmp_path / "file" + blocker.write_text("x", encoding="utf-8") + with patch("app.services.mod_icons.BASE_DIR", blocker): + assert mod_icons.fetch_icon("https://x/z.png", session).size == (48, 48) + + +def _imports(path: Path) -> set: + names = set() + for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))): + if isinstance(node, ast.Import): + names.update(a.name for a in node.names) + elif isinstance(node, ast.ImportFrom) and node.module: + names.add(node.module) + names.update(f"{node.module}.{a.name}" for a in node.names) + return names + + +class TestBoundaries: + def test_api_never_imports_a_gui_toolkit_or_the_ui(self): + for path in (ROOT / "app" / "api").glob("*.py"): + bad = {n for n in _imports(path) + if n.split(".")[0] in ("tkinter", "customtkinter", "PySide6", "pystray") + or n.startswith("app.ui")} + assert not bad, f"{path.name} imports {bad}" + + @pytest.mark.parametrize("screen", ["main.py", "modrinth_browser.py"]) + def test_ported_screens_reach_the_core_only_through_the_api(self, screen): + names = _imports(ROOT / "app" / "ui" / screen) + forbidden = {"app.core.logic", "app.core.core", "app.services.mod_install_tracker", + "app.services.mrpack_installer", "app.services.modrinth", "subprocess", "shutil", + "app.services.server_properties", "requests"} + assert not names & forbidden + source = (ROOT / "app" / "ui" / screen).read_text(encoding="utf-8") + assert "os.remove(" not in source and "os.makedirs(" not in source diff --git a/tests/test_mod_content.py b/tests/test_mod_content.py new file mode 100644 index 0000000..f7cc300 --- /dev/null +++ b/tests/test_mod_content.py @@ -0,0 +1,233 @@ +"""Mod/plugin file operations moved out of the Modrinth browser +(app/services/mod_content.py): the delete guard above all.""" + +import os +import subprocess +from unittest.mock import MagicMock, patch + +import pytest + +from app.core.fs_links import create_junction +from app.services import mod_content, mod_install_tracker +from app.services.mod_content import ModDownloadError, NoCompatibleVersionError, UnsafePathError +from app.services.server_paths import check_server_name + + +@pytest.fixture +def servers(tmp_path): + root = tmp_path / "servers" + root.mkdir() + with patch("app.services.mod_content.SERVERS_DIR", str(root)), \ + patch("app.services.mod_install_tracker.SERVERS_DIR", str(root)): + yield root + + +def _jar(folder, name, content=b"jar"): + folder.mkdir(parents=True, exist_ok=True) + path = folder / name + path.write_bytes(content) + return path + + +class TestServerName: + @pytest.mark.parametrize("name", ["", " ", ".", "..", "a/b", "a\\b", "../x", "C:x", None]) + def test_rejects_anything_but_one_folder_name(self, name): + with pytest.raises(UnsafePathError): + check_server_name(name) + + def test_accepts_plain_names(self): + assert check_server_name("My Server 1.20") == "My Server 1.20" + + +class TestResolveContentFile: + def test_file_in_mods_or_plugins_is_allowed(self, servers): + mod = _jar(servers / "srv" / "mods", "a.jar") + plugin = _jar(servers / "srv" / "plugins", "p.jar") + assert mod_content.resolve_content_file("srv", str(mod)) == os.path.abspath(mod) + assert mod_content.resolve_content_file("srv", str(plugin)) == os.path.abspath(plugin) + + @pytest.mark.parametrize("relative", [ + "server.properties", # the server folder itself + "world/level.dat", # anywhere else in it + "mods/../server.properties", # traversal out of mods/ + "mods/sub/a.jar", # nested below mods/ + ]) + def test_other_places_in_the_server_are_refused(self, servers, relative): + target = servers / "srv" / relative + _jar(target.parent, target.name) + (servers / "srv" / "mods").mkdir(exist_ok=True) + with pytest.raises(UnsafePathError): + mod_content.resolve_content_file("srv", str(servers / "srv" / relative)) + + def test_another_servers_mods_are_refused(self, servers): + other = _jar(servers / "other" / "mods", "a.jar") + (servers / "srv" / "mods").mkdir(parents=True) + with pytest.raises(UnsafePathError): + mod_content.resolve_content_file("srv", str(other)) + + def test_a_folder_inside_mods_is_refused(self, servers): + (servers / "srv" / "mods" / "config").mkdir(parents=True) + with pytest.raises(UnsafePathError): + mod_content.resolve_content_file("srv", str(servers / "srv" / "mods" / "config")) + + def test_server_reached_through_a_link_is_allowed(self, servers, tmp_path): + real = tmp_path / "elsewhere" + mod = _jar(real / "mods", "a.jar") + try: + create_junction(str(real), str(servers / "linked")) # how imported servers are linked + except (OSError, subprocess.SubprocessError) as exc: + pytest.skip(f"directory links not available: {exc}") + linked = servers / "linked" / "mods" / "a.jar" + assert mod_content.resolve_content_file("linked", str(linked)) + mod_content.delete_content_file("linked", str(linked)) + assert not mod.exists() + + +class TestDelete: + def test_delete_removes_file_and_record(self, servers): + mod = _jar(servers / "srv" / "mods", "sodium.jar") + mod_install_tracker.record_install("srv", "sodium", "sodium.jar") + assert mod_content.delete_content_file("srv", str(mod)) == "sodium.jar" + assert not mod.exists() + assert mod_install_tracker.get_installed_slugs("srv") == set() + + def test_delete_outside_never_touches_the_file(self, servers): + props = _jar(servers / "srv", "server.properties") + (servers / "srv" / "mods").mkdir() + with pytest.raises(UnsafePathError): + mod_content.delete_content_file("srv", str(props)) + assert props.exists() + + def test_bulk_delete_reports_each_outcome(self, servers): + a = _jar(servers / "srv" / "mods", "a.jar") + b = _jar(servers / "srv" / "plugins", "b.jar") + outside = _jar(servers / "srv", "eula.txt") + mod_install_tracker.record_install("srv", "alpha", "a.jar") + result = mod_content.delete_content_files( + "srv", [str(a), str(outside), str(servers / "srv" / "mods" / "gone.jar"), str(b)]) + assert result.deleted == ["a.jar", "b.jar"] + assert [p for p, _ in result.failed] == [str(outside), str(servers / "srv" / "mods" / "gone.jar")] + assert result.removed_slugs == {"alpha"} + assert outside.exists() + + +class TestUninstall: + def test_removes_recorded_file(self, servers): + mod = _jar(servers / "srv" / "plugins", "lp.jar") + mod_install_tracker.record_install("srv", "luckperms", "lp.jar") + assert mod_content.uninstall("srv", "luckperms") == "lp.jar" + assert not mod.exists() + assert mod_install_tracker.get_installed_slugs("srv") == set() + + def test_unknown_slug_is_a_no_op(self, servers): + assert mod_content.uninstall("srv", "nothing") is None + + def test_missing_file_only_drops_the_record(self, servers): + (servers / "srv" / "mods").mkdir(parents=True) + mod_install_tracker.record_install("srv", "sodium", "sodium.jar") + assert mod_content.uninstall("srv", "sodium") == "sodium.jar" + assert mod_install_tracker.get_installed_slugs("srv") == set() + + def test_tampered_record_cannot_delete_outside(self, servers): + props = _jar(servers / "srv", "server.properties") + (servers / "srv" / "mods").mkdir() + mod_install_tracker.record_install("srv", "evil", "../server.properties") + with pytest.raises(UnsafePathError): + mod_content.uninstall("srv", "evil") + assert props.exists() + + +class TestInstalledFiles: + def test_lists_jars_of_both_folders(self, servers): + _jar(servers / "srv" / "mods", "b.jar") + _jar(servers / "srv" / "mods", "a.jar") + _jar(servers / "srv" / "mods", "notes.txt") + _jar(servers / "srv" / "plugins", "p.jar") + files = [os.path.relpath(f, servers / "srv") for f in mod_content.installed_jar_files("srv")] + assert files == [os.path.join("mods", "a.jar"), os.path.join("mods", "b.jar"), os.path.join("plugins", "p.jar")] + + def test_no_folders_no_files(self, servers): + assert mod_content.installed_jar_files("srv") == [] + + +class TestInstalls: + def test_install_version_records_slug(self, servers): + client = MagicMock() + client.download_version.return_value = str(servers / "srv" / "mods" / "sodium-1.jar") + progress = MagicMock() + path = mod_content.install_version(client, {"id": "v"}, "srv", "fabric", "sodium", progress) + assert path.endswith("sodium-1.jar") + client.download_version.assert_called_once_with({"id": "v"}, "srv", "fabric", progress_callback=progress) + assert mod_install_tracker.get_installed_filename("srv", "sodium") == "sodium-1.jar" + + def test_failed_download_records_nothing(self, servers): + client = MagicMock() + client.download_version.return_value = None + assert mod_content.install_version(client, {}, "srv", "fabric", "sodium") is None + assert mod_install_tracker.get_installed_slugs("srv") == set() + + def test_install_version_rejects_bad_server_name(self, servers): + with pytest.raises(UnsafePathError): + mod_content.install_version(MagicMock(), {}, "..", "fabric", "x") + + def test_install_latest(self, servers): + client = MagicMock() + client.get_versions.return_value = [{"id": "1"}] + client.download_mod.return_value = "/x/lithium.jar" + assert mod_content.install_latest(client, "lithium", "srv", "1.21", "fabric") == "/x/lithium.jar" + assert mod_install_tracker.get_installed_filename("srv", "lithium") == "lithium.jar" + + def test_install_latest_without_a_build(self, servers): + client = MagicMock() + client.get_versions.return_value = [] + with pytest.raises(NoCompatibleVersionError): + mod_content.install_latest(client, "lithium", "srv", "1.21", "fabric") + client.download_mod.assert_not_called() + + def test_install_latest_failed_download(self, servers): + client = MagicMock() + client.get_versions.return_value = [{"id": "1"}] + client.download_mod.return_value = None + assert mod_content.install_latest(client, "lithium", "srv", "1.21", "fabric") is None + assert mod_install_tracker.get_installed_slugs("srv") == set() + + +class TestInstallModpack: + def _client(self, tmp_dirs, ok=True): + client = MagicMock() + + def download(version, dest, progress_callback=None): + tmp_dirs.append(dest) + if progress_callback: + progress_callback(1.0) + return os.path.join(dest, "pack.mrpack") if ok else None + client.download_version_to.side_effect = download + return client + + def test_installs_and_removes_temp_folder(self, servers): + tmp_dirs, statuses, progress = [], [], [] + client = self._client(tmp_dirs) + with patch("app.services.mod_content.get_server_meta", return_value={"type": "Fabric", "version": "1.21"}), \ + patch("app.services.mod_content.install_mrpack", return_value={"installed": 3}) as inst: + summary = mod_content.install_modpack(client, {"id": "v"}, "srv", progress.append, statuses.append) + assert summary == {"installed": 3} + assert progress == [1.0] + args, kwargs = inst.call_args + assert args == (os.path.join(tmp_dirs[0], "pack.mrpack"), "srv") + assert kwargs["server_type"] == "Fabric" and kwargs["mc_version"] == "1.21" + assert kwargs["progress_callback"] == statuses.append + assert not os.path.exists(tmp_dirs[0]) + + def test_failed_download_raises_and_cleans_up(self, servers): + tmp_dirs = [] + with pytest.raises(ModDownloadError): + mod_content.install_modpack(self._client(tmp_dirs, ok=False), {"id": "v"}, "srv") + assert not os.path.exists(tmp_dirs[0]) + + def test_install_error_still_cleans_up(self, servers): + tmp_dirs = [] + with patch("app.services.mod_content.get_server_meta", return_value={}), \ + patch("app.services.mod_content.install_mrpack", side_effect=ValueError("bad pack")): + with pytest.raises(ValueError): + mod_content.install_modpack(self._client(tmp_dirs), {"id": "v"}, "srv") + assert not os.path.exists(tmp_dirs[0]) diff --git a/tests/test_modrinth_browser.py b/tests/test_modrinth_browser.py index 4504c86..2afca4b 100644 --- a/tests/test_modrinth_browser.py +++ b/tests/test_modrinth_browser.py @@ -1,8 +1,9 @@ -"""Unit tests for pure-logic helpers in app/ui/modrinth_browser.py (F8).""" +"""Pre-update snapshot ordering and update selection (app/services/mod_content.py, F8).""" from pathlib import Path -from app.ui.modrinth_browser import _filter_updates_for_selection, _snapshot_then_apply +from app.services.mod_content import (filter_updates_for_selection as _filter_updates_for_selection, + snapshot_then_apply as _snapshot_then_apply) class TestSnapshotThenApply: From a0e7bf1575c3098288cd4c7a4e5af9b699effa1e Mon Sep 17 00:00:00 2001 From: DesvoSoft Date: Thu, 15 Oct 2026 15:22:17 -0600 Subject: [PATCH 2/6] docs: record the API seam design, inventory and 8b status --- docs/ARCHITECTURE.md | 97 ++++++++++++++++++++++++++++++++++++++------ docs/ROADMAP.md | 2 +- docs/STANDARDS.md | 10 ++--- docs/changelog.md | 2 + 4 files changed, 93 insertions(+), 18 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cd2d670..4d5bc7d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -10,6 +10,7 @@ This document covers the internal architecture, auto-healing system, technical d 1. [Event-Driven Architecture](#event-driven-architecture) 2. [Layer Separation](#layer-separation) + - [API Seam](#api-seam) 3. [Central Orchestrator: ZBBManager](#central-orchestrator-zbbmanager) 4. [Sub-Orchestrators](#sub-orchestrators) 5. [EventBus & ServerEvent](#eventbus--serverevent) @@ -35,19 +36,19 @@ This document covers the internal architecture, auto-healing system, technical d ## Event-Driven Architecture -The core and UI are **fully decoupled** via `EventBus`. `ZBBManager` is the single orchestrator; the UI subscribes to events and never performs blocking I/O or calls services directly. +The core and UI are **fully decoupled**: the UI calls the [API seam](#api-seam) (`app/api`) and receives core events through it; `ZBBManager` is the single orchestrator behind it. The UI never performs blocking I/O on its thread. ``` -UI emits event → ZBBManager receives → delegates to orchestrator/service - → service emits result → UI updates +UI → AppApi (commands/queries) → ZBBManager / services +UI ← dispatcher ← AppApi.events ← EventBus ← core/services emit ``` Rules: -- Use `events.subscribe()` — never `.on()`. -- UI updates only via `self.after(0, callback)` (Tkinter thread safety). -- No service or core module imports from `app/ui/` (one documented exception: `bootstrap.resolve_data_dir()` opens the setup dialogs before any window exists). -- Mutating flows (start/stop/backup/restart/snapshot) go through `ZBBManager`. When a UI widget needs one, it receives the manager method as an injected callback (e.g. `ModrinthBrowser(create_snapshot=zbb_manager.create_pre_update_snapshot)`) instead of importing the service. -- Pragmatic exception: UI may import services for read-only/stateless helpers (load properties, list templates, disk usage). +- Subscribe through `api.events.subscribe()` (core code: `events.subscribe()`) — never `.on()`. +- Widgets are touched only on the UI thread: the dispatcher given to `AppApi` marshals events and task callbacks there (`self.after(0, fn)` in CustomTkinter). +- No service or core module imports from `app/ui/` (one documented exception: `bootstrap.resolve_data_dir()` opens the setup dialogs before any window exists). `app/api` imports no GUI toolkit. +- Ported screens (main window, Modrinth browser) reach the core only through `AppApi`; the others use `AppApi.legacy_manager` until 8b part B. +- Pragmatic exception (unported screens): read-only/stateless helpers (load properties, list templates, disk usage). --- @@ -55,11 +56,71 @@ Rules: ``` app/ui/ → Presentation only. No business logic. No direct service calls. +app/api/ → UI-agnostic facade: commands, queries, events, tasks (plain Python). app/core/ → Orchestration, EventBus, business logic. app/services/ → Specialized services (auto-healing, API clients, utilities). ``` -Dependency direction is strictly one-way: `ui → core → services`. Never the reverse. +Dependency direction is strictly one-way: `ui → api → core → services`. Never the reverse. + +--- + +## API Seam + +**Package:** `app/api/` (`AppApi` in `app_api.py`; areas in `servers.py`, `mods.py`, `storage.py`, `areas.py`; `events.py`, `tasks.py`) + +The layer every UI talks to: the CustomTkinter screens today, the Qt screens of `docs/ROADMAP.md` 8d (and a pywebview fallback if the 8c spike fails) tomorrow. Inputs and outputs are plain Python — `str`, `dict`, dataclasses (`ServerInfo`, `MemorySample`, `JavaSummary`, `DeleteResult`), PIL images — and the package never imports a GUI toolkit or `app.ui` (`tests/test_api_facade.py::TestBoundaries` fails if it does). Everything is testable with pytest and fakes, without a display or network. + +**Why a package, not one more module in `app/core`:** it is a layer of its own (`ui → api → core → services`) with a boundary a test can check, and it keeps `app/core` free of presentation-shaped helpers ("open folder", icon thumbnails). `app/core/bootstrap.py` is untouched: the API is imported only after the data directory is resolved (it reads `constants`). + +### Areas + +| Area | Commands | Queries | +|------|----------|---------| +| `servers` | `select`, `start`, `stop`, `delete` (refuses the running server and any name that isn't one folder), `export`, `import_pack`, `import_folder` (needs a `.jar`), `update_meta`, `request_mod_install`, `create(config) → Task` | `list`, `exists`, `info(name) → ServerInfo` (engine, version, loader, max players), `is_running`, `active`, `running_server`, `memory() → MemorySample`, `creations()` | +| `console` | `send(cmd) → bool` (False when nothing runs; the sanitizer stays in `ServerOrchestrator`) | lines arrive as `CONSOLE_LINE` events | +| `tunnel` | `start`, `stop`, `reset(mode)`, `link(code)` | `is_linked`, `is_running`; status via `TUNNEL_STATUS` | +| `players` | `PlayerOrchestrator` as is (`roster`, `whitelist_add`, `op`, `ban`, `kick`…) — it already takes and returns plain values | | +| `backups` | `create_snapshot`, `delete` | | +| `mods` | `install_version`, `install_latest`, `install_modpack`, `apply_updates` (pre-update snapshot first, nothing touched if it fails), `uninstall`, `delete_file(s)` | `search`, `get_versions`, `required_dependencies`, `check_updates`, `installed_files/slugs/filename`, `fetch_icon` / `cached_icon` | +| `java` | | `best() → JavaSummary` | +| `settings` | `set`, `save` | `get` | +| `storage` | `open_folder(path, create=False)` — the only place that launches Explorer/Finder/xdg-open (`open_in_file_manager`) | `data_dir`, `servers_dir`, `logs_dir`, `server_dir(name, sub)` | + +Lifecycle: `start_background_services()` (tick loop, pre-warm), `shutdown()` (cancels tasks, closes subscriptions, `ZBBManager.shutdown()`, then kills a child process that survived — moved out of `main.py`). `mods` methods block on network/disk and are called from the UI's workers; `servers.start/stop` block too. + +### Events: the UI chooses the thread + +`api.events.subscribe(event, handler, dispatcher=None) → Subscription` sits on the existing `EventBus` (`ServerEvent` names unchanged). A *dispatcher* is `Callable[[Callable[[], None]], None]`: the core emits on whatever thread did the work, and the dispatcher decides where the handler runs. `AppApi(dispatcher=...)` / `set_dispatcher()` set the default; one subscription can pass its own. + +| UI | Dispatcher | +|----|-----------| +| CustomTkinter | `lambda fn: root.after(0, fn)` (`MCTunnelApp`) | +| Qt (8d) | emit a queued signal / `QMetaObject.invokeMethod(..., Qt.QueuedConnection)` | +| Tests, headless | `direct` (run on the emitting thread) | + +`Subscription.cancel()` also drops calls already queued in the dispatcher, so a closed screen never receives a late event. `EventHub.close()` cancels all of them (done by `shutdown()`). + +### Long operations: `Task` + +`servers.create(config)` returns a `Task` (`app/api/tasks.py`) instead of blocking: `on_progress(cb)` receives `(fraction, text)`, `on_done(cb)` the finished task (`result()` is a `ProvisionResult`; a crash becomes `ok=False` with the message, so a progress dialog always closes), `cancel()` sets the same `threading.Event` provisioning already polls between steps and download chunks (a running loader installer is killed, the attempt rolled back). Callbacks go through the dispatcher. `api.tasks.cancel_all()` is what the window calls on exit. + +### Inventory: what the screens called (2026-10-15, before the seam) + +| Screen | Direct core/service use | Now | +|--------|------------------------|-----| +| `main.py` | ~40 `zbb_manager.*` calls (start/stop/select/send_command/provision/tunnel/link/import/shutdown), `logic.get_server_meta`/`update_server_meta`/`get_server_ram`/`delete_server`, `load_server_properties`, `migration.export/import_server`, `JavaDetector`, `SettingsManager`, `os.makedirs(SERVERS_DIR)`, folder opening via `subprocess`, orphan-process kill, direct `EventBus` subscriptions with `self.after` inside each handler | API (part A) | +| `modrinth_browser.py` | `ModrinthClient` (search, versions, deps, downloads, updates, icon HTTP via its session), `install_mrpack` + temp folder `shutil.rmtree`, `mod_install_tracker`, `os.remove` of mod files (3 places), `get_server_meta`, PIL decode/resize/disk cache of icons, `SERVERS_DIR` path building | API (part A); icons are wrapped in `CTkImage` and memory-cached in the UI only | +| `server_properties_editor.py` | `BackupManager`, `logic` module (scheduler, RAM, icon), `save_server_properties`/`list_worlds`, `install_mrpack`, `JavaDetector`, ~61 property descriptions/impact levels as UI data | Part B (folder opening already via `open_in_file_manager`) | +| `app_settings.py` | `zbb_manager` (JDK purge, crash reports, webhook reload), `JavaDetector`, `DiscordWebhookService`, `dir_size`, `SettingsManager` | Part B (folder opening done) | +| `players_panel.py` | `zbb.players`, `fetch_head` | Part B | +| `server_wizard.py` | `VersionManager`, `JavaDetector`, `template_manager`, `content_import` checks | Part B | + +Until part B lands those screens receive `AppApi.legacy_manager` (the `ZBBManager`); new code and the Qt UI must not use it. + +### Mod files behind a guard + +`app/services/mod_content.py` holds what the Modrinth browser did itself: listing jars, installs that record the Modrinth slug, modpack install in a temporary folder that is always removed, the snapshot-then-update order (`snapshot_then_apply`), and deletes. Every delete goes through `resolve_content_file`: the entry must sit directly in that server's `mods/` or `plugins/` (parent compared after resolving links, so junction-imported servers work; the entry itself is not followed, so a link is removed, never its target). A tampered `installed_mods.json` filename like `../server.properties` is refused. Server names used to build paths are checked by `server_paths.check_server_name` (one folder name: no separators, `..` or drive). `app/services/mod_icons.py` downloads (http/https only), scales to 48 px and disk-caches icons as PIL images. --- @@ -240,9 +301,9 @@ Subscribes to `CRASHED` event. On each crash: ### Pre-Update Snapshots -**File:** `app/core/orchestrators.py` (`BackupOrchestrator.create_pre_update_snapshot`), `app/services/backup_manager.py`, `app/ui/modrinth_browser.py` +**File:** `app/core/orchestrators.py` (`BackupOrchestrator.create_pre_update_snapshot`), `app/services/backup_manager.py`, `app/services/mod_content.py`, `app/api/mods.py` -- Mod updates (single-badge and bulk "Update Selected") call `ZBBManager.create_pre_update_snapshot(server_name)` — injected into `ModrinthBrowser` as the `create_snapshot` callback, so the UI never touches `BackupManager` — before any file changes. Ordering lives in the pure helper `_snapshot_then_apply`: snapshot failure aborts the update rather than proceeding without a rollback point. +- Mod updates (single-badge and bulk "Update Selected") go through `api.mods.apply_updates`, which calls `ZBBManager.create_pre_update_snapshot(server_name)` (via `api.backups`) before any file changes. Ordering lives in `mod_content.snapshot_then_apply`: snapshot failure aborts the update rather than proceeding without a rollback point. - The orchestrator refuses a snapshot when the target is the active, running server (locked files), when another backup holds `_backup_in_progress`, and **discards** a snapshot that skipped locked files instead of reporting it as a usable rollback point. Emits `BACKUP_COMPLETED` / `BACKUP_FAILED` like scheduled backups. - Backup reasons: `manual` (untagged filename, "Create Backup"), `auto` (scheduled auto-backups and the optional backup before a scheduled restart — both honor the user's "keep last N"), `pre_update` (default cap: 5). Tagged backups (`{timestamp}__{reason}.zip`) are retention-scoped by `reason`, so no rotation ever deletes another reason's backups — in particular the auto-backup rotation never touches manual backups (it did before `auto` existed). - Rollback reuses the Backups tab restore flow (`server_properties_editor.py`): one row per backup with a reason chip (Manual / Auto / Pre-update), size chip, per-row **Restore** (blocked while the server runs) and **Delete** (via `ZBBManager.delete_backup`), and **Open Folder** for the backup directory. @@ -278,7 +339,7 @@ All auto-healing events surface via the **Toast** system (`app/ui/toast.py`): ## Threading Model - All background threads: `daemon=True`. -- UI updates: always via `self.after(0, callback)`. +- UI updates: always on the UI thread — event handlers and `Task` callbacks arrive through the dispatcher passed to `AppApi` (`self.after(0, fn)` in CustomTkinter); a worker that touches widgets itself still uses `self.after(0, callback)`. - `ServerRunner.running`: property backed by `_state_lock` (threading.Lock) — prevents TOCTOU race in Watchdog `_do_restart()`. - `ServerRunner.connected_players`: all join/leave mutations inside `_players_lock`; cleared in `start()` to prevent stale player data after restart. - `EventBus`: `threading.RLock` for subscribe/emit. @@ -372,6 +433,15 @@ ZeroBlockBridge/ ├── app/ │ ├── launcher.py # Entry point: resolve data dir, then start UI (~17 LOC) │ │ +│ ├── api/ # UI-agnostic facade (see "API Seam"); no GUI imports +│ │ ├── app_api.py # AppApi: areas, dispatcher, lifecycle (shutdown + orphan kill) +│ │ ├── servers.py # ServersApi, ServerInfo, MemorySample +│ │ ├── mods.py # ModsApi over Modrinth + mod_content + mod_icons +│ │ ├── areas.py # Console, Tunnel, Backups, Java, Settings +│ │ ├── storage.py # Paths + open_in_file_manager (the only folder opener) +│ │ ├── events.py # EventHub / Subscription / dispatchers +│ │ └── tasks.py # Task handles: progress, done, cancel +│ │ │ ├── ui/ # Presentation Layer — no business logic │ │ ├── main.py # MCTunnelApp: main window, sidebar, hero status bar, console tabs (~1201 LOC) │ │ ├── server_wizard.py # 6-step creation wizard (~826 LOC) @@ -439,6 +509,9 @@ ZeroBlockBridge/ │ ├── nbt.py # Minimal NBT reader for level.dat (size/depth capped) │ ├── mrpack_installer.py # Modrinth modpack (.mrpack) install (~215 LOC) │ ├── mod_install_tracker.py # Installed Modrinth slugs per server (~73 LOC) +│ ├── mod_content.py # A server's mod/plugin files: installs, guarded deletes, modpacks, update order +│ ├── mod_icons.py # Modrinth icon download + 48px disk cache (PIL) +│ ├── server_paths.py # Server-name validation for paths under servers/ │ ├── mod_id_resolver.py # Crash-log mod IDs → Modrinth slugs (~26 LOC) │ ├── sha1_validator.py # SHA1-verified download with retry (~113 LOC) │ ├── disk_usage.py # Folder size helpers for Settings → Storage/Java (~38 LOC) diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 647ba84..81aae57 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -34,7 +34,7 @@ CustomTkinter's rendering problems are structural (every widget is a Tk canvas; | # | Item | Notes | |---|------|-------| | 8a | i18n foundation and language at install | **Part 1 in review** (`feature/i18n-foundation`): i18n module, catalogs + tooling, OS language detection, picker on the first setup page, persistence (marker / portable settings / Settings → General), Spanish for the pre-app dialogs, EULA, close dialog, tray and single-instance message, locale-aware formatting. **Part 2 in review** (`feature/i18n-core-messages`): label/key separation (console and Settings tabs, theme, player filters, Modrinth sort, schedule mode, property impact, tunnel status, crash reasons, blocked-command reasons, Java dropdown, explicit console line kinds) and Spanish for the core/services messages (notifications, exceptions, progress). Scope: Dependency-free i18n module, gettext `.po` catalogs in `assets/locales`, Babel extraction, plural and locale-aware formatting helpers. Separate display labels from internal keys (console/settings tabs, theme, player filters, sort options, BlockedReason, crash reason codes, tunnel-log tagging). Language picker on the first setup page (default: Windows UI language), stored in `install.json` (portable: the data folder's settings) and in Settings → General. Spanish for setup/update/uninstall dialogs, core messages and notifications. | -| 8b | UI-agnostic API seam over the core | Commands, queries and events with tests; move logic out of screens (Modrinth browser file deletes/downloads, folder opening). | +| 8b | UI-agnostic API seam over the core | **Part A in review** (`feature/api-seam`): `app/api` facade (servers, console, tunnel, players, backups, mods, Java, settings, storage), event subscriptions through a UI-supplied dispatcher, task handles with progress and cancellation; mod file deletes/installs/modpacks/icons moved from the Modrinth browser into `app/services/mod_content.py` / `mod_icons.py` behind a mods/plugins-only delete guard; "open folder" in one place; main window and Modrinth browser switched to the API. **Part B** (next): server-properties metadata into a data module, and the players panel, settings dialog, wizard and properties editor on the API (then `AppApi.legacy_manager` goes away). Scope: Commands, queries and events with tests; move logic out of screens (Modrinth browser file deletes/downloads, folder opening). | | 8c | Qt spike (2 days) | Server list, live console, tray, one wizard dialog, dark/light, Spanish from the same catalog. Passes if: dialog opens < 100 ms with no flash, exe ≤ 60 MB, startup ≤ current + 1 s, correct at 150% scaling with Spanish text. Otherwise repeat with pywebview. | | 8d | Screen-by-screen migration behind `--ui=qt` | Main window/console/tray → server list/header → wizard → properties → settings and setup dialogs → players → Modrinth browser. One PR per group, Spanish included. Remove CustomTkinter at parity. | diff --git a/docs/STANDARDS.md b/docs/STANDARDS.md index 2ad1d8e..69977fd 100644 --- a/docs/STANDARDS.md +++ b/docs/STANDARDS.md @@ -13,9 +13,9 @@ All contributors (human or AI) must adhere to these rules. ZBB is built on strict **decoupled architecture**: -- **UI → Core → Services** (one-way). Never the reverse. -- All UI↔Core communication via `EventBus` or `ZBBManager` methods. Mutating flows (start/stop/backup/restart/snapshot) never call services from `app/ui/` — pass the manager method to the widget as a callback when needed (e.g. `ModrinthBrowser(create_snapshot=...)`). -- Pragmatic exception: UI may import services for read-only/stateless helpers (load properties, list templates, disk usage, file dialogs). +- **UI → API → Core → Services** (one-way). Never the reverse. +- All UI↔Core communication goes through `AppApi` (`app/api`, ARCHITECTURE "API Seam"): commands and queries with plain-Python inputs/outputs, events via `api.events.subscribe()` with the UI's dispatcher, long work as a `Task`. A ported screen never imports `app.core.logic`, `ZBBManager` or a service to act; new behavior goes into a service and is exposed on the API first. `app/api` never imports a GUI toolkit or `app.ui`. +- Screens not ported yet (players, settings, properties, wizard) still take `AppApi.legacy_manager` and may import services for read-only/stateless helpers (load properties, list templates, disk usage, file dialogs) until 8b part B. - `ZBBManager` is the single orchestrator for server lifecycle. No other component manages lifecycles. - **Headless-ready**: every feature must work without a GUI (preparatory for CLI/REST API). - Use `events.subscribe()` — `.on()` is banned. @@ -292,7 +292,7 @@ python -m py_compile app/core/core.py ### Testing UI code -UI modules are not driven through Tk in tests. Extract decision logic into module-level pure helpers or static methods and test those (e.g. `modrinth_browser._snapshot_then_apply`, `_filter_updates_for_selection`, `ConsoleWidget._line_tag`). +UI modules are not driven through Tk in tests. Put decisions and file/network work behind the API (tested with fakes: `tests/test_api_facade.py`, `test_mod_content.py`, `test_api_events_tasks.py`), and test what remains in a screen as module-level pure helpers or static methods (e.g. `ConsoleWidget._line_tag`). ### Test Standards @@ -364,7 +364,7 @@ pytest tests/ -q # 7. Visual pass: screenshot every window/tab in Dark AND Light, plus the main window at 900x580 ``` -**Current score (2026-09-14): 97/100** — F401 clean, no banned patterns, no color/radius literals outside documented exceptions. Known deduction: `_kill_orphan_processes` (`main.py`) swallows `Exception` without logging on app exit (commented as intentional, but still breaks §2.2). +**Current score (2026-09-14): 97/100** — F401 clean, no banned patterns, no color/radius literals outside documented exceptions. The former deduction (`main._kill_orphan_processes` swallowing exceptions) is gone: the kill moved to `AppApi.shutdown` and logs. --- diff --git a/docs/changelog.md b/docs/changelog.md index fcc6362..be34077 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -16,6 +16,8 @@ Format based on [Keep a Changelog](https://keepachangelog.com/). - Settings tabs and theme, console tabs, player filters, Modrinth sort order, the restart schedule mode and the property impact hints show in the chosen language. ### Changed +- Internal: a UI-independent API (`app/api`) now sits between the screens and the core, so the upcoming Qt interface and today's one share the same tested code. The main window and the Modrinth browser use it; nothing changes on screen. +- Deleting or uninstalling a mod/plugin only ever removes a file directly inside that server's `mods/` or `plugins/` folder: a damaged `installed_mods.json` entry can no longer point a delete elsewhere. Deleting a server refuses a name that isn't a plain folder name. - The mod auto-install result reads as full sentences ("2 mods were installed. 1 mod failed to install."). - The Tunnel Log colors ZBB's own error lines red because they are marked as errors, not because they contain the word "failed" or "Error", so they stay red in Spanish. From ff4b751d7d7e39cba84af14dbe3740fc2308e7bc Mon Sep 17 00:00:00 2001 From: DesvoSoft Date: Thu, 15 Oct 2026 15:28:06 -0600 Subject: [PATCH 3/6] fix(api): refuse server names Windows resolves to another folder Names like '...', 'x.' or '. ' passed the check but Windows strips trailing dots and spaces, so a delete of '...' would have targeted the servers folder itself and 'x.' the server 'x'. Device names and the characters Windows forbids are refused there too. --- app/services/server_paths.py | 33 ++++++++++++++++++++++++++++++--- tests/test_mod_content.py | 27 +++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/app/services/server_paths.py b/app/services/server_paths.py index 476389f..31002b1 100644 --- a/app/services/server_paths.py +++ b/app/services/server_paths.py @@ -1,15 +1,42 @@ """Validation of server names used to build paths under SERVERS_DIR.""" import os +import re +import sys class UnsafePathError(ValueError): """A name or path that would reach outside the place it must stay in.""" +# Windows strips trailing dots/spaces ("x." is "x", "..." is the parent +# folder itself) and maps device names to devices, whatever the extension. +_RESERVED = re.compile(r"^(con|prn|aux|nul|com[0-9]|lpt[0-9])(\..*)?$", re.IGNORECASE) +_CONTROL_CHARS = {chr(c) for c in range(32)} +# Only Windows forbids these in names; elsewhere a server folder may use them. +_WINDOWS_CHARS = set('<>:"|?*') + + +def _forbidden_chars() -> set: + chars = _CONTROL_CHARS | {"/", "\\"} + return chars | _WINDOWS_CHARS if sys.platform == "win32" else chars + + def check_server_name(server_name: str) -> str: - """A server name is one folder under SERVERS_DIR, never a path.""" - if (not isinstance(server_name, str) or not server_name.strip() or server_name in (".", "..") - or "/" in server_name or "\\" in server_name or os.sep in server_name or ":" in server_name): + """A server name is one folder directly under SERVERS_DIR, never a path, + on every platform: no separators or control characters (on Windows also + none of <>:"|?* and no device name like NUL), no trailing dot or space.""" + if (not isinstance(server_name, str) or not server_name.strip() + or server_name in (".", "..") or server_name.endswith((".", " ")) + or set(server_name) & _forbidden_chars() or os.sep in server_name + or (sys.platform == "win32" and _RESERVED.match(server_name))): raise UnsafePathError(f"Not a server name: {server_name!r}") return server_name + + +def is_valid_server_name(server_name: str) -> bool: + try: + check_server_name(server_name) + except UnsafePathError: + return False + return True diff --git a/tests/test_mod_content.py b/tests/test_mod_content.py index f7cc300..a50639a 100644 --- a/tests/test_mod_content.py +++ b/tests/test_mod_content.py @@ -10,7 +10,7 @@ from app.core.fs_links import create_junction from app.services import mod_content, mod_install_tracker from app.services.mod_content import ModDownloadError, NoCompatibleVersionError, UnsafePathError -from app.services.server_paths import check_server_name +from app.services.server_paths import check_server_name, is_valid_server_name @pytest.fixture @@ -30,13 +30,36 @@ def _jar(folder, name, content=b"jar"): class TestServerName: - @pytest.mark.parametrize("name", ["", " ", ".", "..", "a/b", "a\\b", "../x", "C:x", None]) + @pytest.mark.parametrize("name", [ + "", " ", ".", "..", "...", "....", ". ", ".. ", " ..", "x.", "x ", "a/b", "a\\b", "../x", + "a\nb", "a\x00b", None]) def test_rejects_anything_but_one_folder_name(self, name): with pytest.raises(UnsafePathError): check_server_name(name) + assert not is_valid_server_name(name) + + @pytest.mark.parametrize("name", ["C:x", "a|b", "what?", "NUL", "con.txt", "COM1"]) + def test_rejects_windows_only_names_on_windows(self, name, monkeypatch): + monkeypatch.setattr("app.services.server_paths.sys.platform", "win32") + with pytest.raises(UnsafePathError): + check_server_name(name) + + def test_windows_only_rules_do_not_apply_elsewhere(self, monkeypatch): + monkeypatch.setattr("app.services.server_paths.sys.platform", "linux") + assert check_server_name("a:b") == "a:b" + assert check_server_name("con") == "con" + + @pytest.mark.parametrize("name", ["...", "x.", ". "]) + def test_names_windows_resolves_elsewhere_never_reach_a_delete(self, servers, name): + (servers / "x").mkdir() + with pytest.raises(UnsafePathError): + mod_content.content_dirs(name) + assert (servers / "x").exists() def test_accepts_plain_names(self): assert check_server_name("My Server 1.20") == "My Server 1.20" + assert check_server_name(".hidden") == ".hidden" + assert is_valid_server_name("survival") class TestResolveContentFile: From e535f70a3813cebded11182eef1c013bb87bfb54 Mon Sep 17 00:00:00 2001 From: DesvoSoft Date: Thu, 15 Oct 2026 15:28:41 -0600 Subject: [PATCH 4/6] fix(mods): keep the installed state right after deleting one mod file Deleting a Modrinth-installed jar from the Installed view dropped its record but the browser kept the slug cached, so the card still showed Installed and Uninstall did nothing. ModsApi.delete_file now returns the forgotten slug. Also: an import name that can't be a folder asks again instead of failing silently, and a refused server delete shows a toast. --- app/api/mods.py | 11 +++++++++-- app/api/servers.py | 7 ++++++- app/ui/main.py | 6 +++++- app/ui/modrinth_browser.py | 3 ++- tests/test_api_facade.py | 29 +++++++++++++++++++++++++++-- 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/app/api/mods.py b/app/api/mods.py index cc3f8f4..176e6ea 100644 --- a/app/api/mods.py +++ b/app/api/mods.py @@ -2,6 +2,7 @@ server's content files. Every method blocks (network or disk): call it off the UI thread.""" +import os import threading from typing import Callable, Optional @@ -93,8 +94,14 @@ def uninstall(server_name: str, slug: str) -> Optional[str]: return mod_content.uninstall(server_name, slug) @staticmethod - def delete_file(server_name: str, path: str) -> str: - return mod_content.delete_content_file(server_name, path) + def delete_file(server_name: str, path: str) -> Optional[str]: + """Delete one mod/plugin file. Returns the Modrinth slug it was + installed as (now forgotten), or None. Raises OSError/UnsafePathError.""" + fname = os.path.basename(path) + slug = next((s for s in mod_install_tracker.get_installed_slugs(server_name) + if mod_install_tracker.get_installed_filename(server_name, s) == fname), None) + mod_content.delete_content_file(server_name, path) + return slug @staticmethod def delete_files(server_name: str, paths) -> DeleteResult: diff --git a/app/api/servers.py b/app/api/servers.py index ea100af..f0243c1 100644 --- a/app/api/servers.py +++ b/app/api/servers.py @@ -12,7 +12,7 @@ from app.core.server_events import ServerEvent from app.services import migration from app.services.modrinth import loader_for_server_type -from app.services.server_paths import check_server_name +from app.services.server_paths import check_server_name, is_valid_server_name from app.services.server_properties import load_server_properties logger = logging.getLogger(__name__) @@ -60,6 +60,11 @@ def list(self) -> list[str]: os.makedirs(SERVERS_DIR, exist_ok=True) return [d for d in os.listdir(SERVERS_DIR) if os.path.isdir(os.path.join(SERVERS_DIR, d))] + @staticmethod + def valid_name(name: str) -> bool: + """Whether ``name`` can be a server folder (ask before create/import).""" + return is_valid_server_name(name) + def exists(self, name: str) -> bool: return os.path.isdir(os.path.join(SERVERS_DIR, check_server_name(name))) diff --git a/app/ui/main.py b/app/ui/main.py index 5cddf95..f08f447 100644 --- a/app/ui/main.py +++ b/app/ui/main.py @@ -767,7 +767,7 @@ def on_server_delete(self, server_name): def _delete(): try: self.api.servers.delete(server_name) - except (OSError, ServerRunningError) as e: + except (OSError, ValueError, ServerRunningError) as e: self.after(0, lambda e=e: Toast.show(self, f"Delete failed: {e}", toast_type="error")) return self.after(0, lambda: self._on_server_deleted(server_name)) @@ -845,6 +845,10 @@ def on_import_zbbpack(self): if entered is None: return entered = entered.strip() or suggested_name + if not self.api.servers.valid_name(entered): + Toast.show(self, f"'{entered}' can't be used as a server name", toast_type="error") + initial = entered + continue if self.api.servers.exists(entered): Toast.show(self, f"A server named '{entered}' already exists", toast_type="error") initial = entered diff --git a/app/ui/modrinth_browser.py b/app/ui/modrinth_browser.py index 958f5d5..2059bd3 100644 --- a/app/ui/modrinth_browser.py +++ b/app/ui/modrinth_browser.py @@ -1515,7 +1515,8 @@ def _confirm_delete_mod(self, filepath: str): if not ctx: return try: - self.mods.delete_file(ctx[0], filepath) + removed_slug = self.mods.delete_file(ctx[0], filepath) + self._installed_slugs_cache.discard(removed_slug) self._set_status(f"Deleted {fname}", kind="success") self._render_installed() # refresh inline except (OSError, UnsafePathError) as exc: diff --git a/tests/test_api_facade.py b/tests/test_api_facade.py index 266622e..42bdc5c 100644 --- a/tests/test_api_facade.py +++ b/tests/test_api_facade.py @@ -91,6 +91,12 @@ def make_api(manager=None, **kw): class TestServers: + def test_valid_name_lets_the_ui_ask_again_instead_of_crashing(self): + api, _ = make_api() + assert api.servers.valid_name("Survival 2") + assert not api.servers.valid_name("a/b") + assert not api.servers.valid_name("x.") + def test_list_creates_the_folder_and_lists_only_folders(self, servers_dir): api, _ = make_api() assert api.servers.list() == [] @@ -394,20 +400,39 @@ def test_file_operations_delegate_to_the_service(self): mods.installed_slugs("srv") mods.installed_filename("srv", "x") mods.uninstall("srv", "x") - mods.delete_file("srv", "p") mods.delete_files("srv", ["p"]) mods.install_version({"id": 1}, "srv", "fabric", "x") mods.install_latest("x", "srv", "1.21", "fabric") mods.install_modpack({"id": 1}, "srv") content.installed_jar_files.assert_called_once_with("srv") content.uninstall.assert_called_once_with("srv", "x") - content.delete_content_file.assert_called_once_with("srv", "p") content.delete_content_files.assert_called_once_with("srv", ["p"]) assert content.install_version.called and content.install_latest.called and content.install_modpack.called tracker.get_installed_slugs.assert_called_once_with("srv") tracker.get_installed_filename.assert_called_once_with("srv", "x") +class TestModsApiDeleteFile: + """Regression: a single delete must report the slug it forgot, or the + browser keeps showing the mod as installed (and Uninstall does nothing).""" + + def test_returns_the_forgotten_slug(self, tmp_path): + root = tmp_path / "servers" + mods_dir = root / "srv" / "mods" + mods_dir.mkdir(parents=True) + (mods_dir / "sodium.jar").write_bytes(b"x") + (mods_dir / "manual.jar").write_bytes(b"x") + with patch("app.services.mod_content.SERVERS_DIR", str(root)), patch("app.services.mod_install_tracker.SERVERS_DIR", str(root)): + from app.services import mod_install_tracker + mod_install_tracker.record_install("srv", "sodium", "sodium.jar") + assert ModsApi.delete_file("srv", str(mods_dir / "sodium.jar")) == "sodium" + assert ModsApi.delete_file("srv", str(mods_dir / "manual.jar")) is None + assert mod_install_tracker.get_installed_slugs("srv") == set() + with pytest.raises(UnsafePathError): + ModsApi.delete_file("srv", str(root / "srv" / "server.properties")) + assert not list(mods_dir.iterdir()) + + def _png_bytes(): buf = io.BytesIO() Image.new("RGBA", (128, 128), (255, 0, 0, 255)).save(buf, "PNG") From b99a21b99301759266d47132f931f89253270ed3 Mon Sep 17 00:00:00 2001 From: DesvoSoft Date: Thu, 15 Oct 2026 15:29:25 -0600 Subject: [PATCH 5/6] fix(api): always finish a task, whatever happens to its job or UI A BaseException in the job, an executor that is already shut down or drops the queued job, and a dispatcher whose window was destroyed left the task unfinished (progress dialog open, counted as running) or aborted the job mid-rollback. Each case now ends the task and runs its done callbacks. --- app/api/tasks.py | 48 ++++++++++++++++++++++++++++----- docs/ARCHITECTURE.md | 4 +-- tests/test_api_events_tasks.py | 49 ++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 9 deletions(-) diff --git a/app/api/tasks.py b/app/api/tasks.py index 0d5d168..c399158 100644 --- a/app/api/tasks.py +++ b/app/api/tasks.py @@ -41,7 +41,7 @@ def on_done(self, callback: Callable[["Task"], None]) -> "Task": if not finished: self._done.append(callback) if finished: - self._dispatcher(lambda: callback(self)) + self._dispatch(lambda: callback(self)) return self def cancel(self) -> bool: @@ -69,18 +69,39 @@ def report(self, *args) -> None: with self._lock: listeners = list(self._progress) for callback in listeners: - self._dispatcher(lambda cb=callback: cb(*args)) + self._dispatch(lambda cb=callback: cb(*args)) + + def _dispatch(self, fn: Callable[[], None]) -> None: + # A dispatcher whose UI is gone (window destroyed during exit) must + # not abort the job that reports to it, e.g. a creation rolling back. + try: + self._dispatcher(fn) + except Exception as exc: + logger.debug("Task %s: callback dropped: %s", self.name, exc) def _run(self, job: Callable[["Task"], Any]) -> None: try: self._future.set_result(job(self)) - except Exception as exc: + except BaseException as exc: logger.exception("Task %s failed", self.name or job) self._future.set_exception(exc) + self._finish() + if not isinstance(exc, Exception): + raise + return + self._finish() + + def _fail(self, exc: BaseException) -> None: + """Finish without running (the executor refused or dropped the job).""" + if not self._future.done(): + self._future.set_exception(exc) + self._finish() + + def _finish(self) -> None: with self._lock: callbacks, self._done = self._done, [] for callback in callbacks: - self._dispatcher(lambda cb=callback: cb(self)) + self._dispatch(lambda cb=callback: cb(self)) class TaskRunner: @@ -102,12 +123,25 @@ def run(): try: task._run(job) finally: - with self._lock: - self._active.discard(task) + self._forget(task) + + def dropped(future): + # Cancelled while queued (executor shutdown with cancel_futures). + if future.cancelled(): + self._forget(task) + task._fail(concurrent.futures.CancelledError()) - self._executor.submit(run) + try: + self._executor.submit(run).add_done_callback(dropped) + except RuntimeError as exc: # executor already shut down + self._forget(task) + task._fail(exc) return task + def _forget(self, task: Task) -> None: + with self._lock: + self._active.discard(task) + def active(self, name: Optional[str] = None) -> list[Task]: with self._lock: return [t for t in self._active if name is None or t.name == name] diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4d5bc7d..516e35a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -77,7 +77,7 @@ The layer every UI talks to: the CustomTkinter screens today, the Qt screens of | Area | Commands | Queries | |------|----------|---------| -| `servers` | `select`, `start`, `stop`, `delete` (refuses the running server and any name that isn't one folder), `export`, `import_pack`, `import_folder` (needs a `.jar`), `update_meta`, `request_mod_install`, `create(config) → Task` | `list`, `exists`, `info(name) → ServerInfo` (engine, version, loader, max players), `is_running`, `active`, `running_server`, `memory() → MemorySample`, `creations()` | +| `servers` | `select`, `start`, `stop`, `delete` (refuses the running server and any name that isn't one folder), `export`, `import_pack`, `import_folder` (needs a `.jar`), `update_meta`, `request_mod_install`, `create(config) → Task` | `list`, `exists`, `valid_name`, `info(name) → ServerInfo` (engine, version, loader, max players), `is_running`, `active`, `running_server`, `memory() → MemorySample`, `creations()` | | `console` | `send(cmd) → bool` (False when nothing runs; the sanitizer stays in `ServerOrchestrator`) | lines arrive as `CONSOLE_LINE` events | | `tunnel` | `start`, `stop`, `reset(mode)`, `link(code)` | `is_linked`, `is_running`; status via `TUNNEL_STATUS` | | `players` | `PlayerOrchestrator` as is (`roster`, `whitelist_add`, `op`, `ban`, `kick`…) — it already takes and returns plain values | | @@ -103,7 +103,7 @@ Lifecycle: `start_background_services()` (tick loop, pre-warm), `shutdown()` (ca ### Long operations: `Task` -`servers.create(config)` returns a `Task` (`app/api/tasks.py`) instead of blocking: `on_progress(cb)` receives `(fraction, text)`, `on_done(cb)` the finished task (`result()` is a `ProvisionResult`; a crash becomes `ok=False` with the message, so a progress dialog always closes), `cancel()` sets the same `threading.Event` provisioning already polls between steps and download chunks (a running loader installer is killed, the attempt rolled back). Callbacks go through the dispatcher. `api.tasks.cancel_all()` is what the window calls on exit. +`servers.create(config)` returns a `Task` (`app/api/tasks.py`) instead of blocking: `on_progress(cb)` receives `(fraction, text)`, `on_done(cb)` the finished task (`result()` is a `ProvisionResult`; a crash becomes `ok=False` with the message, so a progress dialog always closes), `cancel()` sets the same `threading.Event` provisioning already polls between steps and download chunks (a running loader installer is killed, the attempt rolled back). Callbacks go through the dispatcher; a task always finishes — a job that raises, an executor that refuses or drops it, or a dispatcher whose window is gone (the job keeps running) all end it with `on_done`. `api.tasks.cancel_all()` is what the window calls on exit. ### Inventory: what the screens called (2026-10-15, before the seam) diff --git a/tests/test_api_events_tasks.py b/tests/test_api_events_tasks.py index 1c6981b..6414c36 100644 --- a/tests/test_api_events_tasks.py +++ b/tests/test_api_events_tasks.py @@ -172,3 +172,52 @@ def test_job_exception_is_kept_and_done_still_fires(self): assert isinstance(task.exception(timeout=1), RuntimeError) with pytest.raises(RuntimeError): task.result(timeout=1) + + +class TestTaskEndsAreAlwaysReported: + """Regressions: a task must finish (and say so) whatever happens to its + job, its executor or the UI it reports to — or a progress dialog stays + open and the task counts as running forever.""" + + def test_base_exception_still_completes_the_task(self): + done = [] + task = Task().on_done(done.append) + with pytest.raises(KeyboardInterrupt): + task._run(lambda t: (_ for _ in ()).throw(KeyboardInterrupt())) + assert done == [task] and task.done() + + def test_submit_to_a_shut_down_executor_fails_the_task(self): + executor = concurrent.futures.ThreadPoolExecutor(max_workers=1) + executor.shutdown() + runner = TaskRunner(executor, lambda: direct) + done = [] + task = runner.start("x", lambda t: 1).on_done(done.append) + assert done == [task] + assert isinstance(task.exception(timeout=1), RuntimeError) + assert runner.active() == [] + + def test_job_dropped_from_the_queue_fails_the_task(self): + executor = concurrent.futures.ThreadPoolExecutor(max_workers=1) + runner = TaskRunner(executor, lambda: direct) + release = threading.Event() + first = runner.start("busy", lambda t: release.wait(timeout=5)) + queued = runner.start("queued", lambda t: "never") + executor.shutdown(wait=False, cancel_futures=True) + release.set() + assert first.result(timeout=5) is True + with pytest.raises(concurrent.futures.CancelledError): + queued.result(timeout=5) + assert runner.active() == [] + + def test_dead_dispatcher_does_not_abort_the_job(self): + def gone(_fn): + raise RuntimeError("main thread is not in main loop") + + task = Task(dispatcher=gone) + task.on_progress(lambda *a: None) + + def job(t): + t.report(0.5, "rolling back") + return "rolled back" + task._run(job) + assert task.result(timeout=1) == "rolled back" From 5f359d99561af79438daa906bb1041e238af04f1 Mon Sep 17 00:00:00 2001 From: DesvoSoft Date: Thu, 15 Oct 2026 15:35:29 -0600 Subject: [PATCH 6/6] fix(api): evaluate annotations the same way on Python 3.11 and 3.14 On 3.11 (CI, release builds) ServersApi's -> list[Task] was evaluated at definition time inside the class, where list is the ServersApi.list method, and importing app.api failed. The 3.14 dev machine defers annotations, so it passed locally. The API modules and the new services now postpone annotations, and a test evaluates every annotation in them, which reproduces the 3.11 failure on 3.14. --- app/api/__init__.py | 2 ++ app/api/app_api.py | 2 ++ app/api/areas.py | 2 ++ app/api/events.py | 2 ++ app/api/mods.py | 2 ++ app/api/servers.py | 2 ++ app/api/storage.py | 2 ++ app/api/tasks.py | 2 ++ app/services/mod_content.py | 2 ++ app/services/mod_icons.py | 2 ++ app/services/server_paths.py | 2 ++ docs/STANDARDS.md | 1 + tests/test_api_annotations.py | 50 +++++++++++++++++++++++++++++++++++ 13 files changed, 73 insertions(+) create mode 100644 tests/test_api_annotations.py diff --git a/app/api/__init__.py b/app/api/__init__.py index 59b8109..f44ab9c 100644 --- a/app/api/__init__.py +++ b/app/api/__init__.py @@ -4,6 +4,8 @@ actions or state. This package never imports a GUI toolkit. """ +from __future__ import annotations + from app.api.app_api import AppApi from app.api.events import Dispatcher, EventHub, Subscription, direct from app.api.servers import MemorySample, NoServerJarError, ServerInfo, ServerRunningError diff --git a/app/api/app_api.py b/app/api/app_api.py index 66cad88..cfee539 100644 --- a/app/api/app_api.py +++ b/app/api/app_api.py @@ -5,6 +5,8 @@ dispatcher for its main loop, and calls the areas below. """ +from __future__ import annotations + import concurrent.futures import logging from typing import Optional diff --git a/app/api/areas.py b/app/api/areas.py index 40fe132..36d9d33 100644 --- a/app/api/areas.py +++ b/app/api/areas.py @@ -1,5 +1,7 @@ """The smaller areas of the API: console, tunnel, players, backups, Java, settings.""" +from __future__ import annotations + import logging from dataclasses import dataclass from typing import Any, Optional diff --git a/app/api/events.py b/app/api/events.py index 6614fa6..e4926da 100644 --- a/app/api/events.py +++ b/app/api/events.py @@ -7,6 +7,8 @@ a Qt UI a queued signal; tests and headless callers use ``direct``. """ +from __future__ import annotations + import logging import threading from typing import Any, Callable, Optional diff --git a/app/api/mods.py b/app/api/mods.py index 176e6ea..c02808a 100644 --- a/app/api/mods.py +++ b/app/api/mods.py @@ -2,6 +2,8 @@ server's content files. Every method blocks (network or disk): call it off the UI thread.""" +from __future__ import annotations + import os import threading from typing import Callable, Optional diff --git a/app/api/servers.py b/app/api/servers.py index f0243c1..41fefec 100644 --- a/app/api/servers.py +++ b/app/api/servers.py @@ -1,5 +1,7 @@ """Servers: list, inspect, create, start/stop, delete, import/export.""" +from __future__ import annotations + import logging import os from dataclasses import dataclass diff --git a/app/api/storage.py b/app/api/storage.py index e3efa5a..dce0b0b 100644 --- a/app/api/storage.py +++ b/app/api/storage.py @@ -1,5 +1,7 @@ """Folders: where things live and opening them in the system file manager.""" +from __future__ import annotations + import logging import os import subprocess diff --git a/app/api/tasks.py b/app/api/tasks.py index c399158..4e3b511 100644 --- a/app/api/tasks.py +++ b/app/api/tasks.py @@ -7,6 +7,8 @@ dispatcher, so a UI receives them on its own thread. """ +from __future__ import annotations + import concurrent.futures import logging import threading diff --git a/app/services/mod_content.py b/app/services/mod_content.py index 591fb31..38a391f 100644 --- a/app/services/mod_content.py +++ b/app/services/mod_content.py @@ -6,6 +6,8 @@ handed over by a UI, can never remove something else. """ +from __future__ import annotations + import logging import os import shutil diff --git a/app/services/mod_icons.py b/app/services/mod_icons.py index a0ac8d3..f2a7ff5 100644 --- a/app/services/mod_icons.py +++ b/app/services/mod_icons.py @@ -4,6 +4,8 @@ in-memory cache of those wrappers belongs to the UI. """ +from __future__ import annotations + import hashlib import io import logging diff --git a/app/services/server_paths.py b/app/services/server_paths.py index 31002b1..bdae297 100644 --- a/app/services/server_paths.py +++ b/app/services/server_paths.py @@ -1,5 +1,7 @@ """Validation of server names used to build paths under SERVERS_DIR.""" +from __future__ import annotations + import os import re import sys diff --git a/docs/STANDARDS.md b/docs/STANDARDS.md index 69977fd..a30f7da 100644 --- a/docs/STANDARDS.md +++ b/docs/STANDARDS.md @@ -42,6 +42,7 @@ ZBB is built on strict **decoupled architecture**: - All imports at **module top** (PEP 8). Inline imports only to break confirmed circular dependencies — document why with a comment. - No unused imports. `flake8 --select=F401` must return zero violations. - `from typing import Dict, List` is dead weight in Python 3.9+. Use `dict[str, ...]` and `list[...]`. +- CI and release builds run **Python 3.11**, which evaluates annotations when a function is defined; a 3.14 dev machine defers them (PEP 649) and hides mistakes such as a method named `list` breaking a later `-> list[...]` in the same class. Modules in `app/api` start with `from __future__ import annotations`, and `tests/test_api_annotations.py` evaluates every annotation there so the error shows up locally too. ### 2.2 Exceptions diff --git a/tests/test_api_annotations.py b/tests/test_api_annotations.py new file mode 100644 index 0000000..a6ebd4c --- /dev/null +++ b/tests/test_api_annotations.py @@ -0,0 +1,50 @@ +"""Annotations of the API and its new services must evaluate. + +CI and release builds run Python 3.11, which evaluates annotations when a +function is defined; the dev machine runs 3.14, which defers them (PEP 649) +until something reads them. A method named like a builtin (ServersApi.list) +then broke ``-> list[Task]`` on 3.11 only. Reading every annotation here +evaluates it on any version, so the mistake fails locally too. +""" + +import importlib +import inspect +import pkgutil +import typing + +import pytest + +import app.api + +MODULES = sorted( + [f"app.api.{m.name}" for m in pkgutil.iter_modules(app.api.__path__)] + + ["app.services.mod_content", "app.services.mod_icons", "app.services.server_paths"]) + + +def _functions(module): + for _, obj in inspect.getmembers(module): + if inspect.isclass(obj) and obj.__module__ == module.__name__: + for _, member in inspect.getmembers(obj): + fn = getattr(member, "fget", member) + if inspect.isfunction(fn): + yield fn + elif inspect.isfunction(obj) and obj.__module__ == module.__name__: + yield obj + + +@pytest.mark.parametrize("name", MODULES) +def test_every_annotation_evaluates(name): + module = importlib.import_module(name) + functions = list(_functions(module)) + assert functions + for fn in functions: + typing.get_type_hints(fn) + + +@pytest.mark.parametrize("name", MODULES) +def test_annotations_are_postponed(name): + """`from __future__ import annotations` keeps 3.11 from evaluating them + at definition time (the same behavior as the dev machine's 3.14).""" + module = importlib.import_module(name) + assert getattr(module, "annotations", None) is not None + assert module.annotations.compiler_flag