diff --git a/sickdays/README.md b/sickdays/README.md index 2d9abc8..500041d 100644 --- a/sickdays/README.md +++ b/sickdays/README.md @@ -82,7 +82,7 @@ KeepPanel(parent, day, *, on_saved, on_cancel=None, paths=None).pack() Normal day: five switches, all off (off = stands down), "Mark sick"; sick day: its switches, "Save", "Clear sick day". Past/free/uncheckable days show -why and offer no write; a refused write shows in the panel, never raised. +why and offer no write; a refusal (or a click mid-sync) shows in the panel. `on_saved(SickDay | None)` fires after the write and sync start: the entry on Mark sick / a changed Save (`is_sick=True`) or Clear (`is_sick=False`), `None` on an unchanged Save (nothing written). The panel never destroys itself. diff --git a/sickdays/sickdays/gui/_panel.py b/sickdays/sickdays/gui/_panel.py index e56720a..428152c 100644 --- a/sickdays/sickdays/gui/_panel.py +++ b/sickdays/sickdays/gui/_panel.py @@ -39,6 +39,18 @@ if TYPE_CHECKING: from sickdays._paths import Paths +#: Held by every background sync, and tried by every write. ``sync()`` reads +#: the pool, waits on the network, then writes its merged copy back, so a +#: write landing in between would be silently overwritten. A write that finds +#: a sync running is refused ("try again") rather than lost. +_SYNC_LOCK = threading.Lock() + + +def _serialised_sync(paths: Paths) -> None: + with _SYNC_LOCK: + sync_quietly(paths=paths) + + def start_sync(paths: Paths) -> threading.Thread: """Push the write in the background so the phone sees it; never raises. @@ -46,8 +58,8 @@ def start_sync(paths: Paths) -> threading.Thread: push half-way. The thread touches no widget. """ thread = threading.Thread( - target=sync_quietly, - kwargs={"paths": paths}, + target=_serialised_sync, + args=(paths,), name="sickdays-sync", daemon=False, ) @@ -66,8 +78,9 @@ class KeepPanel(tk.Frame): * "Save" with nothing changed -> ``None``: nothing written, no sync; * "Clear sick day" -> the cleared entry (``is_sick=False``). - A refused write (:class:`~sickdays.SickDayError`) is shown in the panel - and ``on_saved`` is not called. ``on_cancel``, when given, adds a + A refused write (:class:`~sickdays.SickDayError`, or a click while the + previous write's sync is still running) is shown in the panel and + ``on_saved`` is not called. ``on_cancel``, when given, adds a "Cancel" button that calls it. The panel never destroys itself; the callbacks decide what happens to it. """ @@ -171,11 +184,18 @@ class KeepPanel(tk.Frame): self._write(lambda: remove_sick_day(self._day, paths=self._paths)) def _write(self, action: Callable[[], SickDay]) -> None: + # Non-blocking on purpose (no ``with``): waiting would freeze Tk for + # the whole network round trip. + if not _SYNC_LOCK.acquire(blocking=False): # pylint: disable=consider-using-with + self._status.configure(text=_text.BUSY_NOTE, fg=tok.DANGER) + return try: entry = action() except SickDayError as exc: self._status.configure(text=str(exc), fg=tok.DANGER) return + finally: + _SYNC_LOCK.release() self.sync_thread = start_sync(self._paths) # Last: the callback may destroy this panel. self._on_saved(entry) diff --git a/sickdays/sickdays/gui/_text.py b/sickdays/sickdays/gui/_text.py index 079ba12..9c14657 100644 --- a/sickdays/sickdays/gui/_text.py +++ b/sickdays/sickdays/gui/_text.py @@ -53,6 +53,7 @@ INTRO_DETAIL: Final = ( ) PAST_NOTE: Final = "This day has passed; it can no longer be changed." NONE_AHEAD: Final = "None ahead." +BUSY_NOTE: Final = "Syncing \u2014 try again in a moment." WEEKDAY_INITIALS: Final = ("M", "T", "W", "T", "F", "S", "S") _WEEKDAYS: Final = ("Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun") diff --git a/sickdays/sickdays/tests/test_gui_panel.py b/sickdays/sickdays/tests/test_gui_panel.py index e74cdc9..e6425d9 100644 --- a/sickdays/sickdays/tests/test_gui_panel.py +++ b/sickdays/sickdays/tests/test_gui_panel.py @@ -4,16 +4,18 @@ from __future__ import annotations from datetime import timedelta +import threading import tkinter as tk from typing import TYPE_CHECKING import pytest -from sickdays._api import add_sick_day, is_sick_day +from sickdays._api import add_sick_day, is_sick_day, load from sickdays._day import today from sickdays._errors import ConflictCheckUnavailableError from sickdays._keep_api import kept_lockers from sickdays._paths import Paths +from sickdays._store import write_pool from sickdays.gui import KeepPanel, _text from sickdays.gui import _tokens as tok @@ -203,3 +205,41 @@ def test_explicit_paths_are_used_for_every_read_write_and_sync( assert not is_sick_day(FUTURE, log_path=other.log) assert synced == [other] assert not (tmp_path / "state" / "sick_days.json").exists() + + +def test_a_write_during_a_sync_is_refused_not_lost( + tk_root: tk.Tk, monkeypatch: pytest.MonkeyPatch +) -> None: + """sync() reads the pool, waits on the network, then writes its merged + copy back: a write landing in that window would be silently overwritten. + """ + loaded = threading.Event() + release = threading.Event() + calls: list[int] = [] + + def slow_sync(*, paths: Paths | None = None) -> bool: + calls.append(1) + if len(calls) > 1 or paths is None: + return False + snapshot = load(paths.log) + loaded.set() + release.wait(timeout=5) + write_pool(paths.log, snapshot) + return True + + monkeypatch.setattr("sickdays.gui._panel.sync_quietly", slow_sync) + saved: list[SickDay | None] = [] + first = KeepPanel(tk_root, FUTURE, on_saved=saved.append) + _button(first, "mark").invoke() + assert loaded.wait(timeout=5) + later = FUTURE + timedelta(days=1) + second = KeepPanel(tk_root, later, on_saved=saved.append) + _button(second, "mark").invoke() + release.set() + _join(first) + if second.sync_thread is not None: + second.sync_thread.join(timeout=5) + for entry in saved: + assert entry is not None + assert is_sick_day(entry.day), f"{entry.day} was written, then lost" + assert _status(second) == _text.BUSY_NOTE