From 094a1fe5ed11c31d7f35f8f642862ab087860f15 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 15 Aug 2026 20:30:42 +0200 Subject: [PATCH] Split _workout_sync.py and _sick_tracker.py _workout_sync.py (332) -> _sync_client.py token, Firebase/GitHub client _sync_records.py payload predicates + log decoding _sick_tracker.py (317) -> _sick_budget.py SickHistory + rolling budget This one was not a pure move until the fixtures followed it. Three autouse test guards patch names by module, and all three pointed at _workout_sync: isolate_sync_token SYNC_TOKEN_FILE -> now also on _sync_client _no_real_firebase_config CONFIG_FILE -> now on _sync_client _firebase_config helper (7 call sites) -> now passed _sync_client With them stale, sync_client built a REAL client: the suite went from 14s to over 120s and test_workout_sync_part5 asserted against live workout data pulled from the actual backend. Fixed, the same tests run in 0.7s. Both modules keep SYNC_TOKEN_FILE so either patch point works, and every relocated guard target is asserted to exist. 977 tests pass at 100% branch coverage; pylint back to 8.70 after fixing the findings the split introduced (a dropped `return None` in _parse_iso and leftover dead imports). `screen_lock --status` unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EveB813BQ8abAP68VSqZ5Y --- screen_locker/_sick_budget.py | 125 ++++++++++++ screen_locker/_sick_tracker.py | 132 +++--------- screen_locker/_sync_client.py | 120 +++++++++++ screen_locker/_sync_records.py | 91 +++++++++ screen_locker/_workout_sync.py | 192 +++--------------- screen_locker/tests/conftest.py | 9 +- screen_locker/tests/test_sync_fixtures.py | 10 +- .../tests/test_workout_sync_part2.py | 22 +- .../tests/test_workout_sync_part3.py | 10 +- .../tests/test_workout_sync_part5.py | 28 +-- .../tests/test_workout_sync_part6.py | 12 +- 11 files changed, 441 insertions(+), 310 deletions(-) create mode 100644 screen_locker/_sick_budget.py create mode 100644 screen_locker/_sync_client.py create mode 100644 screen_locker/_sync_records.py diff --git a/screen_locker/_sick_budget.py b/screen_locker/_sick_budget.py new file mode 100644 index 0000000..212eb8f --- /dev/null +++ b/screen_locker/_sick_budget.py @@ -0,0 +1,125 @@ +"""The sick-day record and the rolling budget computed from it. + +Split out of :mod:`screen_locker._sick_tracker` to keep every file under the +250-line cap. Re-exported from there, so callers and their patch targets are +unchanged. + +Everything here is pure: it reads a :class:`SickHistory` already in memory. +Loading and saving that history stays in ``_sick_tracker``. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from datetime import datetime, timedelta, timezone +import logging +from typing import Any + +from screen_locker._constants import ( + SICK_BUDGET_PER_7_DAYS, + SICK_BUDGET_PER_30_DAYS, + SICK_BUDGET_PER_90_DAYS, + SICK_LOCKOUT_MULTIPLIER_PER_RECENT, + SICK_LOCKOUT_SECONDS, +) + +_logger = logging.getLogger(__name__) + + +@dataclass +class SickHistory: + """Persistent sick-day bookkeeping.""" + + sick_days: list[str] = field(default_factory=list) + debt: int = 0 + commitments: dict[str, bool] = field(default_factory=dict) + broken_commitments: list[str] = field(default_factory=list) + justifications: list[dict[str, Any]] = field(default_factory=list) + + +def _today_iso() -> str: + """Return today's date as ``YYYY-MM-DD`` (UTC).""" + return datetime.now(tz=timezone.utc).strftime("%Y-%m-%d") + + +def _parse_iso(date_str: str) -> datetime | None: + """Parse ``YYYY-MM-DD`` into a UTC datetime, or return None.""" + try: + return datetime.strptime(date_str, "%Y-%m-%d").replace(tzinfo=timezone.utc) + except ValueError as exc: + _logger.warning( + "Sick history holds an unparsable date %r (%s) — that entry is " + "IGNORED when counting sick days", + date_str, + exc, + ) + return None + + +def is_sick_day(history: SickHistory, *, today: str | None = None) -> bool: + """Return True if today is recorded as a sick day.""" + today_str = today or _today_iso() + return today_str in history.sick_days + + +def count_in_window( + history: SickHistory, + days: int, + *, + today: str | None = None, +) -> int: + """Return how many ``sick_days`` fall in the trailing ``days`` window.""" + today_str = today or _today_iso() + today_dt = _parse_iso(today_str) + if today_dt is None: + return 0 + cutoff = today_dt - timedelta(days=days) + count = 0 + for entry in history.sick_days: + parsed = _parse_iso(entry) + if parsed is None: + continue + if cutoff < parsed <= today_dt: + count += 1 + return count + + +def is_budget_exhausted( + history: SickHistory, + *, + today: str | None = None, +) -> bool: + """Return True if any rolling window has reached its sick budget.""" + return ( + count_in_window(history, 7, today=today) >= SICK_BUDGET_PER_7_DAYS + or count_in_window(history, 30, today=today) >= SICK_BUDGET_PER_30_DAYS + or count_in_window(history, 90, today=today) >= SICK_BUDGET_PER_90_DAYS + ) + + +def compute_lockout_seconds( + history: SickHistory, + *, + today: str | None = None, +) -> int: + """Escalating sick countdown: ``base * 2 ** recent_count_in_30d``.""" + recent = count_in_window(history, 30, today=today) + multiplier = SICK_LOCKOUT_MULTIPLIER_PER_RECENT**recent + return SICK_LOCKOUT_SECONDS * multiplier + + +def budget_summary( + history: SickHistory, + *, + today: str | None = None, +) -> str: + """One-line UI summary string for budget + debt.""" + week = count_in_window(history, 7, today=today) + month = count_in_window(history, 30, today=today) + quarter = count_in_window(history, 90, today=today) + return ( + f"Sick: {week}/{SICK_BUDGET_PER_7_DAYS}w · " + f"{month}/{SICK_BUDGET_PER_30_DAYS}m · " + f"{quarter}/{SICK_BUDGET_PER_90_DAYS}q · " + f"Debt: {history.debt}" + ) diff --git a/screen_locker/_sick_tracker.py b/screen_locker/_sick_tracker.py index 36ece92..cd265ea 100644 --- a/screen_locker/_sick_tracker.py +++ b/screen_locker/_sick_tracker.py @@ -6,7 +6,7 @@ state via :func:`save_history`. from __future__ import annotations -from dataclasses import dataclass, field +from dataclasses import dataclass from datetime import datetime, timedelta, timezone import json import logging @@ -15,50 +15,41 @@ from typing import Any from gatelock.log_integrity import compute_entry_hmac from screen_locker._constants import ( - SICK_BUDGET_PER_7_DAYS, - SICK_BUDGET_PER_30_DAYS, - SICK_BUDGET_PER_90_DAYS, SICK_COMMITMENT_PENALTY_DAYS, SICK_HISTORY_FILE, SICK_HISTORY_REVIEW_COUNT, SICK_JUSTIFICATION_MIN_CHARS, - SICK_LOCKOUT_MULTIPLIER_PER_RECENT, - SICK_LOCKOUT_SECONDS, +) +from screen_locker._sick_budget import ( + SickHistory, + _parse_iso, + _today_iso, + budget_summary, + compute_lockout_seconds, + count_in_window, + is_budget_exhausted, + is_sick_day, ) +__all__ = [ + "SickHistory", + "add_sick_day", + "budget_summary", + "clear_one_debt", + "compute_lockout_seconds", + "count_in_window", + "had_commitment_for_today", + "is_budget_exhausted", + "is_sick_day", + "load_history", + "mark_commitment_broken", + "record_commitment_for_tomorrow", + "save_history", +] + _logger = logging.getLogger(__name__) -@dataclass -class SickHistory: - """Persistent sick-day bookkeeping.""" - - sick_days: list[str] = field(default_factory=list) - debt: int = 0 - commitments: dict[str, bool] = field(default_factory=dict) - broken_commitments: list[str] = field(default_factory=list) - justifications: list[dict[str, Any]] = field(default_factory=list) - - -def _today_iso() -> str: - """Return today's date as ``YYYY-MM-DD`` (UTC).""" - return datetime.now(tz=timezone.utc).strftime("%Y-%m-%d") - - -def _parse_iso(date_str: str) -> datetime | None: - """Parse ``YYYY-MM-DD`` into a UTC datetime, or return None.""" - try: - return datetime.strptime(date_str, "%Y-%m-%d").replace(tzinfo=timezone.utc) - except ValueError as exc: - _logger.warning( - "Sick history holds an unparsable date %r (%s) — that entry is " - "IGNORED when counting sick days", - date_str, - exc, - ) - return None - - def load_history() -> SickHistory: """Read the persistent sick-day history file. @@ -99,75 +90,6 @@ def save_history(history: SickHistory) -> bool: return True -def is_sick_day(history: SickHistory, *, today: str | None = None) -> bool: - """Return True if today is recorded as a sick day.""" - today_str = today or _today_iso() - return today_str in history.sick_days - - -def count_in_window( - history: SickHistory, - days: int, - *, - today: str | None = None, -) -> int: - """Return how many ``sick_days`` fall in the trailing ``days`` window.""" - today_str = today or _today_iso() - today_dt = _parse_iso(today_str) - if today_dt is None: - return 0 - cutoff = today_dt - timedelta(days=days) - count = 0 - for entry in history.sick_days: - parsed = _parse_iso(entry) - if parsed is None: - continue - if cutoff < parsed <= today_dt: - count += 1 - return count - - -def is_budget_exhausted( - history: SickHistory, - *, - today: str | None = None, -) -> bool: - """Return True if any rolling window has reached its sick budget.""" - return ( - count_in_window(history, 7, today=today) >= SICK_BUDGET_PER_7_DAYS - or count_in_window(history, 30, today=today) >= SICK_BUDGET_PER_30_DAYS - or count_in_window(history, 90, today=today) >= SICK_BUDGET_PER_90_DAYS - ) - - -def compute_lockout_seconds( - history: SickHistory, - *, - today: str | None = None, -) -> int: - """Escalating sick countdown: ``base * 2 ** recent_count_in_30d``.""" - recent = count_in_window(history, 30, today=today) - multiplier = SICK_LOCKOUT_MULTIPLIER_PER_RECENT**recent - return SICK_LOCKOUT_SECONDS * multiplier - - -def budget_summary( - history: SickHistory, - *, - today: str | None = None, -) -> str: - """One-line UI summary string for budget + debt.""" - week = count_in_window(history, 7, today=today) - month = count_in_window(history, 30, today=today) - quarter = count_in_window(history, 90, today=today) - return ( - f"Sick: {week}/{SICK_BUDGET_PER_7_DAYS}w · " - f"{month}/{SICK_BUDGET_PER_30_DAYS}m · " - f"{quarter}/{SICK_BUDGET_PER_90_DAYS}q · " - f"Debt: {history.debt}" - ) - - def add_sick_day(history: SickHistory, *, today: str | None = None) -> int: """Append today's date and increment debt. Returns new debt. diff --git a/screen_locker/_sync_client.py b/screen_locker/_sync_client.py new file mode 100644 index 0000000..391427b --- /dev/null +++ b/screen_locker/_sync_client.py @@ -0,0 +1,120 @@ +"""Building the sync client: read the token, pick Firebase or the GitHub mirror. + +Split out of :mod:`screen_locker._workout_sync` to keep every file under the +250-line cap. Re-exported from there, so callers and their patch targets are +unchanged. + +``sync_client`` returning None means sync is OFF -- and says why in the log, +because a silent None here is exactly how the PC stopped syncing for weeks. +""" + +from __future__ import annotations + +import logging + +from crdt_sync import ( + CONFIG_FILE, + ConfigError, + FirebaseAuthError, + GitHubSyncClient, + RemoteStore, + RemoteSyncError, + firebase_client_for, + mirror_client_for, +) + +from screen_locker._constants import ( + SYNC_REPO_NAME, + SYNC_REPO_OWNER, + SYNC_TIMEOUT_SECONDS, + SYNC_TOKEN_FILE, +) + +_logger = logging.getLogger(__name__) + + +def read_sync_token() -> str | None: + """Return the saved sync PAT, or None if sync isn't configured. + + Unlike diet_guard's equivalent, an absent or empty token file is a + normal state here -- sync is an optional primary channel, not something + the app requires to function. + """ + if not SYNC_TOKEN_FILE.exists(): + return None + token = SYNC_TOKEN_FILE.read_text().strip() + return token or None + + +def remote_client(github: RemoteStore) -> RemoteStore: + """Return the backend to read the phone's workout log from. + + Firebase when ``~/.config/crdt-sync/`` is set up, with GitHub kept as a + mirror so a phone that has not moved yet is still seen; GitHub alone + otherwise. Both callers here are read-only pulls -- this side never pushes + -- and :class:`MirrorSyncClient` reads the union of both, so a workout + logged against either backend still counts. + + The config file is checked before constructing anything, so an + unconfigured machine never reaches the network. + + Rolling back is deleting this function and passing ``github`` straight + through: no data moves either way. + """ + if not CONFIG_FILE.is_file(): + return github + try: + return mirror_client_for("screen_locker", github) + except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc: + _logger.warning( + "Firebase unavailable, reading workouts via GitHub only: %s", exc + ) + return github + + +def sync_client() -> RemoteStore | None: + """Return the configured read client, or None if sync is set up nowhere. + + A GitHub token is no longer required: Firebase has been the primary backend + since eb4ff01, so a Firebase-only machine must still sync. Previously this + module returned early whenever the PAT was missing, reporting "sync is OFF" + on a machine whose sync was working perfectly -- a false negative that hid + a live backend behind a legacy credential check. + + GitHub is used alone when only the PAT exists, Firebase alone when only + ``~/.config/crdt-sync/`` exists, and the mirrored union when both do. + ``None`` means neither is configured, which stays a benign, expected state. + """ + token = read_sync_token() + github = ( + GitHubSyncClient( + SYNC_REPO_OWNER, + SYNC_REPO_NAME, + token, + timeout_seconds=SYNC_TIMEOUT_SECONDS, + ) + if token is not None + else None + ) + if github is not None: + return remote_client(github) + if not CONFIG_FILE.is_file(): + _logger.warning( + "Cannot pull synced workouts: no sync token at %s and no Firebase " + "config at %s — sync is OFF, so only ADB/HTTP can verify a phone " + "workout and phone-logged workouts will NOT count here", + SYNC_TOKEN_FILE, + CONFIG_FILE, + ) + return None + try: + return firebase_client_for("screen_locker") + except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc: + _logger.warning( + "Firebase is configured at %s but unusable, and there is no GitHub " + "token at %s to fall back to: %s — pulling NO synced workouts", + CONFIG_FILE, + SYNC_TOKEN_FILE, + exc, + ) + return None diff --git a/screen_locker/_sync_records.py b/screen_locker/_sync_records.py new file mode 100644 index 0000000..9ea1102 --- /dev/null +++ b/screen_locker/_sync_records.py @@ -0,0 +1,91 @@ +"""Decoding sync-log blobs into records, and telling record kinds apart. + +Split out of :mod:`screen_locker._workout_sync` to keep every file under the +250-line cap. Re-exported from there, so callers are unchanged. + +Everything here is pure: it parses JSON that a caller already fetched. Talking +to GitHub or Firebase stays in ``_workout_sync``. +""" + +from __future__ import annotations + +import json +import logging +from typing import TYPE_CHECKING + +from crdt_sync import Hlc, Record + +from screen_locker._manual_workout import MANUAL_WORKOUT_SYNC_KIND + +if TYPE_CHECKING: + from collections.abc import Callable + +_logger = logging.getLogger(__name__) + +_PAYLOAD_FIELD = "payload" + + +def _is_manual_payload(payload: object) -> bool: + """True if a decoded sync payload is a manual-workout record.""" + return isinstance(payload, dict) and payload.get("kind") == MANUAL_WORKOUT_SYNC_KIND + + +def _is_session_payload(payload: object) -> bool: + """True if a decoded sync payload is a completed StrongLifts session. + + Identified by SHAPE -- the presence of ``exercises`` -- not by a ``kind`` + field, because the phone app's ``WorkoutSession.toJson()`` emits no ``kind`` + at all. Keying session detection on ``kind`` is what made real sessions + invisible to the only reader that walks every device directory. + + Shape-matching also excludes the PC-origin ``runnerup_verified`` / + ``phone_verified`` records that share these device logs: they are non-manual + too, so "not manual" alone would hand a caller expecting ``exercises`` and + ``duration_seconds`` a record that has neither. + """ + return isinstance(payload, dict) and isinstance(payload.get("exercises"), list) + + +def _session_records(log_json: str) -> dict[str, tuple[dict, Hlc]]: + """Return ``{record_id: (payload, hlc)}`` for session records in a log blob. + + Raises the same decode errors as :func:`_manual_records`; callers treat a + corrupt device log as "no sessions from that device". + """ + return _records_matching(log_json, _is_session_payload) + + +def _records_matching( + log_json: str, predicate: Callable[[object], bool] +) -> dict[str, tuple[dict, Hlc]]: + """Return ``{record_id: (payload, hlc)}`` for payloads passing ``predicate``. + + Raises: + TypeError: If the top-level JSON isn't an object or a record's shape + doesn't match what :meth:`Record.from_dict` expects. + KeyError: If a record is missing an expected key. + ValueError: Via ``json.loads`` or :meth:`crdt_sync.Hlc.from_str`. + """ + raw = json.loads(log_json) + if not isinstance(raw, dict): + msg = f"top-level sync payload is not a JSON object: {raw!r}" + raise TypeError(msg) + result: dict[str, tuple[dict, Hlc]] = {} + for data in raw.values(): + record = Record.from_dict(data) + field = record.fields.get(_PAYLOAD_FIELD) + if field is None: + continue + payload, hlc = field + if predicate(payload): + result[record.id] = (payload, hlc) + return result + + +def _manual_records(log_json: str) -> dict[str, tuple[dict, Hlc]]: + """Return ``{record_id: (payload, hlc)}`` for manual records in a log blob. + + Raises the same decode errors as :func:`_records_matching`; callers treat a + corrupt device log as "no manual records from that device". + """ + return _records_matching(log_json, _is_manual_payload) diff --git a/screen_locker/_workout_sync.py b/screen_locker/_workout_sync.py index 19253f6..ddb89df 100644 --- a/screen_locker/_workout_sync.py +++ b/screen_locker/_workout_sync.py @@ -10,32 +10,53 @@ not an error -- unlike diet_guard, where sync is core to the app. from __future__ import annotations -import json import logging from typing import TYPE_CHECKING from crdt_sync import ( CONFIG_FILE, - ConfigError, - FirebaseAuthError, - GitHubSyncClient, Hlc, - Record, RemoteStore, RemoteSyncError, - firebase_client_for, - mirror_client_for, ) from screen_locker._constants import ( SYNC_REPO_NAME, SYNC_REPO_OWNER, - SYNC_TIMEOUT_SECONDS, SYNC_TOKEN_FILE, ) -from screen_locker._manual_workout import MANUAL_WORKOUT_SYNC_KIND +from screen_locker._sync_client import ( + read_sync_token, + remote_client, + sync_client, +) +from screen_locker._sync_records import ( + _is_manual_payload, + _is_session_payload, + _manual_records, + _records_matching, + _session_records, +) from screen_locker._sync_retry import with_sync_retry +# Re-exported for callers (_manual_push) and for the autouse isolate_sync_token +# fixture, which redirects SYNC_TOKEN_FILE on this module as well as on +# _sync_client -- without both, a real token on the host leaks into the tests. +__all__ = [ + "CONFIG_FILE", + "SYNC_TOKEN_FILE", + "_is_manual_payload", + "_is_session_payload", + "_manual_records", + "_records_matching", + "_session_records", + "pull_all_manual_records", + "pull_synced_workout", + "read_sync_token", + "remote_client", + "sync_client", +] + if TYPE_CHECKING: from collections.abc import Callable @@ -45,40 +66,6 @@ _DEVICES_PREFIX = "screen-locker-sync/devices" _PAYLOAD_FIELD = "payload" -def _is_manual_payload(payload: object) -> bool: - """True if a decoded sync payload is a manual-workout record.""" - return isinstance(payload, dict) and payload.get("kind") == MANUAL_WORKOUT_SYNC_KIND - - -def _is_session_payload(payload: object) -> bool: - """True if a decoded sync payload is a completed StrongLifts session. - - Identified by SHAPE -- the presence of ``exercises`` -- not by a ``kind`` - field, because the phone app's ``WorkoutSession.toJson()`` emits no ``kind`` - at all. Keying session detection on ``kind`` is what made real sessions - invisible to the only reader that walks every device directory. - - Shape-matching also excludes the PC-origin ``runnerup_verified`` / - ``phone_verified`` records that share these device logs: they are non-manual - too, so "not manual" alone would hand a caller expecting ``exercises`` and - ``duration_seconds`` a record that has neither. - """ - return isinstance(payload, dict) and isinstance(payload.get("exercises"), list) - - -def read_sync_token() -> str | None: - """Return the saved sync PAT, or None if sync isn't configured. - - Unlike diet_guard's equivalent, an absent or empty token file is a - normal state here -- sync is an optional primary channel, not something - the app requires to function. - """ - if not SYNC_TOKEN_FILE.exists(): - return None - token = SYNC_TOKEN_FILE.read_text().strip() - return token or None - - def _merge_device_records( client: RemoteStore, extract: Callable[[str], dict[str, tuple[dict, Hlc]]], @@ -156,80 +143,6 @@ def _merge_device_records( return merged -def remote_client(github: RemoteStore) -> RemoteStore: - """Return the backend to read the phone's workout log from. - - Firebase when ``~/.config/crdt-sync/`` is set up, with GitHub kept as a - mirror so a phone that has not moved yet is still seen; GitHub alone - otherwise. Both callers here are read-only pulls -- this side never pushes - -- and :class:`MirrorSyncClient` reads the union of both, so a workout - logged against either backend still counts. - - The config file is checked before constructing anything, so an - unconfigured machine never reaches the network. - - Rolling back is deleting this function and passing ``github`` straight - through: no data moves either way. - """ - if not CONFIG_FILE.is_file(): - return github - try: - return mirror_client_for("screen_locker", github) - except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc: - _logger.warning( - "Firebase unavailable, reading workouts via GitHub only: %s", exc - ) - return github - - -def sync_client() -> RemoteStore | None: - """Return the configured read client, or None if sync is set up nowhere. - - A GitHub token is no longer required: Firebase has been the primary backend - since eb4ff01, so a Firebase-only machine must still sync. Previously this - module returned early whenever the PAT was missing, reporting "sync is OFF" - on a machine whose sync was working perfectly -- a false negative that hid - a live backend behind a legacy credential check. - - GitHub is used alone when only the PAT exists, Firebase alone when only - ``~/.config/crdt-sync/`` exists, and the mirrored union when both do. - ``None`` means neither is configured, which stays a benign, expected state. - """ - token = read_sync_token() - github = ( - GitHubSyncClient( - SYNC_REPO_OWNER, - SYNC_REPO_NAME, - token, - timeout_seconds=SYNC_TIMEOUT_SECONDS, - ) - if token is not None - else None - ) - if github is not None: - return remote_client(github) - if not CONFIG_FILE.is_file(): - _logger.warning( - "Cannot pull synced workouts: no sync token at %s and no Firebase " - "config at %s — sync is OFF, so only ADB/HTTP can verify a phone " - "workout and phone-logged workouts will NOT count here", - SYNC_TOKEN_FILE, - CONFIG_FILE, - ) - return None - try: - return firebase_client_for("screen_locker") - except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc: - _logger.warning( - "Firebase is configured at %s but unusable, and there is no GitHub " - "token at %s to fall back to: %s — pulling NO synced workouts", - CONFIG_FILE, - SYNC_TOKEN_FILE, - exc, - ) - return None - - def pull_synced_workout() -> tuple[dict | None, str | None]: """Return ``(data, error)``: the last-synced StrongLifts session, if any. @@ -269,51 +182,6 @@ def pull_synced_workout() -> tuple[dict | None, str | None]: return payload, None -def _session_records(log_json: str) -> dict[str, tuple[dict, Hlc]]: - """Return ``{record_id: (payload, hlc)}`` for session records in a log blob. - - Raises the same decode errors as :func:`_manual_records`; callers treat a - corrupt device log as "no sessions from that device". - """ - return _records_matching(log_json, _is_session_payload) - - -def _records_matching( - log_json: str, predicate: Callable[[object], bool] -) -> dict[str, tuple[dict, Hlc]]: - """Return ``{record_id: (payload, hlc)}`` for payloads passing ``predicate``. - - Raises: - TypeError: If the top-level JSON isn't an object or a record's shape - doesn't match what :meth:`Record.from_dict` expects. - KeyError: If a record is missing an expected key. - ValueError: Via ``json.loads`` or :meth:`crdt_sync.Hlc.from_str`. - """ - raw = json.loads(log_json) - if not isinstance(raw, dict): - msg = f"top-level sync payload is not a JSON object: {raw!r}" - raise TypeError(msg) - result: dict[str, tuple[dict, Hlc]] = {} - for data in raw.values(): - record = Record.from_dict(data) - field = record.fields.get(_PAYLOAD_FIELD) - if field is None: - continue - payload, hlc = field - if predicate(payload): - result[record.id] = (payload, hlc) - return result - - -def _manual_records(log_json: str) -> dict[str, tuple[dict, Hlc]]: - """Return ``{record_id: (payload, hlc)}`` for manual records in a log blob. - - Raises the same decode errors as :func:`_records_matching`; callers treat a - corrupt device log as "no manual records from that device". - """ - return _records_matching(log_json, _is_manual_payload) - - def pull_all_manual_records() -> list[tuple[str, dict]]: """Return all synced manual-workout records across every device log. diff --git a/screen_locker/tests/conftest.py b/screen_locker/tests/conftest.py index efecb19..5607aa8 100644 --- a/screen_locker/tests/conftest.py +++ b/screen_locker/tests/conftest.py @@ -320,8 +320,9 @@ def _no_real_firebase_config(monkeypatch: pytest.MonkeyPatch) -> None: assert against live data instead of their own fakes. Tests that want the Firebase path point it back at a file they control. """ - from screen_locker import _workout_sync + # CONFIG_FILE is read by remote_client, which lives in _sync_client since + # _workout_sync was split. Patching the old module would leave the real + # config in play and the sync tests would assert against live data. + from screen_locker import _sync_client - monkeypatch.setattr( - _workout_sync, "CONFIG_FILE", Path("/nonexistent/firebase.json") - ) + monkeypatch.setattr(_sync_client, "CONFIG_FILE", Path("/nonexistent/firebase.json")) diff --git a/screen_locker/tests/test_sync_fixtures.py b/screen_locker/tests/test_sync_fixtures.py index ace5406..5a25aaa 100644 --- a/screen_locker/tests/test_sync_fixtures.py +++ b/screen_locker/tests/test_sync_fixtures.py @@ -28,9 +28,13 @@ def isolate_sync_token(tmp_path: Path) -> Iterator[None]: file makes ``read_sync_token()`` return None, the same benign "sync not configured" state every existing test already assumes. """ - with patch( - "screen_locker._workout_sync.SYNC_TOKEN_FILE", - tmp_path / "sync_token", + # read_sync_token moved to _sync_client when _workout_sync was split; + # _workout_sync still re-exports the constant, so both names must be + # redirected or a real token on the host leaks into the tests. + token = tmp_path / "sync_token" + with ( + patch("screen_locker._sync_client.SYNC_TOKEN_FILE", token), + patch("screen_locker._workout_sync.SYNC_TOKEN_FILE", token), ): yield diff --git a/screen_locker/tests/test_workout_sync_part2.py b/screen_locker/tests/test_workout_sync_part2.py index 7c3272a..4ee0edd 100644 --- a/screen_locker/tests/test_workout_sync_part2.py +++ b/screen_locker/tests/test_workout_sync_part2.py @@ -16,7 +16,7 @@ from crdt_sync import ( ) import pytest -from screen_locker import _workout_sync +from screen_locker import _sync_client, _workout_sync from screen_locker.tests._workout_sync_fixtures import ( _firebase_config, ) @@ -44,7 +44,7 @@ class TestSyncClient: """Uses github alone when only the token exists.""" _workout_sync.SYNC_TOKEN_FILE.write_text("tok") github = MagicMock() - with patch.object(_workout_sync, "GitHubSyncClient", return_value=github): + with patch.object(_sync_client, "GitHubSyncClient", return_value=github): assert _workout_sync.sync_client() is github def test_mirrors_when_both_the_token_and_firebase_exist( @@ -52,21 +52,21 @@ class TestSyncClient: ) -> None: """Mirrors when both the token and firebase exist.""" _workout_sync.SYNC_TOKEN_FILE.write_text("tok") - _firebase_config(_workout_sync, tmp_path, monkeypatch) + _firebase_config(_sync_client, tmp_path, monkeypatch) github = MagicMock() monkeypatch.setattr( - _workout_sync, "mirror_client_for", lambda _app, client: ("mirror", client) + _sync_client, "mirror_client_for", lambda _app, client: ("mirror", client) ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=github): + with patch.object(_sync_client, "GitHubSyncClient", return_value=github): assert _workout_sync.sync_client() == ("mirror", github) def test_uses_firebase_alone_when_only_the_config_exists( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """A Firebase-only machine must still sync -- no PAT required.""" - _firebase_config(_workout_sync, tmp_path, monkeypatch) + _firebase_config(_sync_client, tmp_path, monkeypatch) firebase = MagicMock() - monkeypatch.setattr(_workout_sync, "firebase_client_for", lambda _app: firebase) + monkeypatch.setattr(_sync_client, "firebase_client_for", lambda _app: firebase) assert _workout_sync.sync_client() is firebase @pytest.mark.parametrize( @@ -81,12 +81,12 @@ class TestSyncClient: self, error: Exception, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """No PAT to fall back to: report it loudly, do not raise.""" - _firebase_config(_workout_sync, tmp_path, monkeypatch) + _firebase_config(_sync_client, tmp_path, monkeypatch) def _boom(*_args: object, **_kwargs: object) -> None: raise error - monkeypatch.setattr(_workout_sync, "firebase_client_for", _boom) + monkeypatch.setattr(_sync_client, "firebase_client_for", _boom) assert _workout_sync.sync_client() is None def test_warns_when_firebase_only_is_unusable( @@ -96,13 +96,13 @@ class TestSyncClient: caplog: pytest.LogCaptureFixture, ) -> None: """Warns when firebase only is unusable.""" - _firebase_config(_workout_sync, tmp_path, monkeypatch) + _firebase_config(_sync_client, tmp_path, monkeypatch) def _boom(*_args: object, **_kwargs: object) -> None: message = "backend down" raise FirebaseSyncError(message) - monkeypatch.setattr(_workout_sync, "firebase_client_for", _boom) + monkeypatch.setattr(_sync_client, "firebase_client_for", _boom) with caplog.at_level("WARNING"): _workout_sync.sync_client() assert "backend down" in caplog.text diff --git a/screen_locker/tests/test_workout_sync_part3.py b/screen_locker/tests/test_workout_sync_part3.py index 369352e..81c93c3 100644 --- a/screen_locker/tests/test_workout_sync_part3.py +++ b/screen_locker/tests/test_workout_sync_part3.py @@ -11,7 +11,7 @@ from typing import TYPE_CHECKING from crdt_sync import ConfigError -from screen_locker import _workout_sync +from screen_locker import _sync_client, _workout_sync from screen_locker.tests._workout_sync_fixtures import _firebase_config if TYPE_CHECKING: @@ -36,9 +36,9 @@ class TestRemoteClient: self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """Configured: reads union both, so either backend's workout counts.""" - _firebase_config(_workout_sync, tmp_path, monkeypatch) + _firebase_config(_sync_client, tmp_path, monkeypatch) monkeypatch.setattr( - _workout_sync, + _sync_client, "mirror_client_for", lambda _app, client: ("mirror", client), ) @@ -50,13 +50,13 @@ class TestRemoteClient: self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """A broken Firebase must degrade to GitHub, never fail the pull.""" - _firebase_config(_workout_sync, tmp_path, monkeypatch) + _firebase_config(_sync_client, tmp_path, monkeypatch) def _boom(*_args: object, **_kwargs: object) -> None: message = "no password" raise ConfigError(message) - monkeypatch.setattr(_workout_sync, "mirror_client_for", _boom) + monkeypatch.setattr(_sync_client, "mirror_client_for", _boom) github = object() assert _workout_sync.remote_client(github) is github diff --git a/screen_locker/tests/test_workout_sync_part5.py b/screen_locker/tests/test_workout_sync_part5.py index f2ea67b..74681b9 100644 --- a/screen_locker/tests/test_workout_sync_part5.py +++ b/screen_locker/tests/test_workout_sync_part5.py @@ -15,7 +15,7 @@ from crdt_sync import ( GitHubSyncError, ) -from screen_locker import _workout_sync +from screen_locker import _sync_client, _workout_sync from screen_locker.tests._workout_sync_fixtures import ( _firebase_config, _manual_payload, @@ -36,7 +36,7 @@ class TestPullSyncedWorkout: def test_returns_none_none_when_no_token_is_configured(self) -> None: """Returns none none when no token is configured.""" - with patch.object(_workout_sync, "GitHubSyncClient") as client_cls: + with patch.object(_sync_client, "GitHubSyncClient") as client_cls: assert _workout_sync.pull_synced_workout() == (None, None) client_cls.assert_not_called() @@ -45,7 +45,7 @@ class TestPullSyncedWorkout: _workout_sync.SYNC_TOKEN_FILE.write_text("tok") client = MagicMock() client.list_directory.side_effect = GitHubSyncError("offline") - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): assert _workout_sync.pull_synced_workout() == (None, "offline") def test_does_not_propagate_a_firebase_sync_error(self) -> None: @@ -58,7 +58,7 @@ class TestPullSyncedWorkout: _workout_sync.SYNC_TOKEN_FILE.write_text("tok") client = MagicMock() client.list_directory.side_effect = FirebaseSyncError("firebase exploded") - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): data, error = _workout_sync.pull_synced_workout() assert data is None assert error == "firebase exploded" @@ -71,10 +71,10 @@ class TestPullSyncedWorkout: The configuration the locker actually died in, pinned in its own right so a future GitHub-only narrowing of the handler cannot pass. """ - _firebase_config(_workout_sync, tmp_path, monkeypatch) + _firebase_config(_sync_client, tmp_path, monkeypatch) client = MagicMock() client.list_directory.side_effect = FirebaseSyncError("firebase exploded") - monkeypatch.setattr(_workout_sync, "firebase_client_for", lambda _app: client) + monkeypatch.setattr(_sync_client, "firebase_client_for", lambda _app: client) data, error = _workout_sync.pull_synced_workout() assert data is None assert error == "firebase exploded" @@ -83,14 +83,14 @@ class TestPullSyncedWorkout: """Returns none none when nothing has been pushed yet.""" _workout_sync.SYNC_TOKEN_FILE.write_text("tok") client = _multi_device_client({"phone": None}) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): assert _workout_sync.pull_synced_workout() == (None, None) def test_returns_none_none_when_there_are_no_devices_at_all(self) -> None: """Returns none none when there are no devices at all.""" _workout_sync.SYNC_TOKEN_FILE.write_text("tok") client = _multi_device_client({}) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): assert _workout_sync.pull_synced_workout() == (None, None) def test_returns_the_payload_on_success(self) -> None: @@ -100,7 +100,7 @@ class TestPullSyncedWorkout: client = _multi_device_client( {"phone": json.dumps({"a": _session_record_dict("session:a", payload)})} ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): assert _workout_sync.pull_synced_workout() == (payload, None) def test_finds_a_session_in_a_uuid_named_device_directory(self) -> None: @@ -131,7 +131,7 @@ class TestPullSyncedWorkout: ), } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): assert _workout_sync.pull_synced_workout() == (fresh, None) def test_returns_the_highest_clock_session_across_devices(self) -> None: @@ -163,7 +163,7 @@ class TestPullSyncedWorkout: ), } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): data, error = _workout_sync.pull_synced_workout() assert error is None assert data == newest @@ -201,7 +201,7 @@ class TestPullSyncedWorkout: ) } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): data, error = _workout_sync.pull_synced_workout() assert error is None assert data == session @@ -216,12 +216,12 @@ class TestPullSyncedWorkout: "phone": json.dumps({"a": _session_record_dict("session:a", payload)}), } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): assert _workout_sync.pull_synced_workout() == (payload, None) def test_returns_none_none_when_every_device_log_is_corrupt(self) -> None: """Returns none none when every device log is corrupt.""" _workout_sync.SYNC_TOKEN_FILE.write_text("tok") client = _multi_device_client({"corrupt": "{not valid json"}) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): assert _workout_sync.pull_synced_workout() == (None, None) diff --git a/screen_locker/tests/test_workout_sync_part6.py b/screen_locker/tests/test_workout_sync_part6.py index 0cf0050..8ac856f 100644 --- a/screen_locker/tests/test_workout_sync_part6.py +++ b/screen_locker/tests/test_workout_sync_part6.py @@ -13,7 +13,7 @@ from crdt_sync import ( GitHubSyncError, ) -from screen_locker import _workout_sync +from screen_locker import _sync_client, _workout_sync from screen_locker.tests._workout_sync_fixtures import ( _manual_payload, _manual_record_dict, @@ -33,7 +33,7 @@ class TestPullAllManualRecords: _workout_sync.SYNC_TOKEN_FILE.write_text("tok") client = MagicMock() client.list_directory.side_effect = GitHubSyncError("offline") - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): assert _workout_sync.pull_all_manual_records() == [] def test_merges_manual_records_across_devices(self) -> None: @@ -49,7 +49,7 @@ class TestPullAllManualRecords: ), } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): result = _workout_sync.pull_all_manual_records() assert sorted(rid for rid, _ in result) == ["manual:a", "manual:b"] @@ -66,7 +66,7 @@ class TestPullAllManualRecords: "corrupt": "{not json", } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): result = _workout_sync.pull_all_manual_records() assert [rid for rid, _ in result] == ["manual:a"] @@ -95,7 +95,7 @@ class TestPullAllManualRecords: ), } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): result = _workout_sync.pull_all_manual_records() assert len(result) == 1 assert result[0][1]["cost"] == "NEW" @@ -125,7 +125,7 @@ class TestPullAllManualRecords: ), } ) - with patch.object(_workout_sync, "GitHubSyncClient", return_value=client): + with patch.object(_sync_client, "GitHubSyncClient", return_value=client): result = _workout_sync.pull_all_manual_records() assert len(result) == 1 assert result[0][1]["cost"] == "NEW"