diff --git a/src/open_cups/application_state.py b/src/open_cups/application_state.py index 1d21dd6..cb0653e 100644 --- a/src/open_cups/application_state.py +++ b/src/open_cups/application_state.py @@ -15,6 +15,12 @@ def get_session_room(self, session_id: str) -> Room | None: return room return None + def is_session_live(self, session_id: str, timeout_seconds: float) -> bool: + room = self.get_session_room(session_id) + if room is None: + return False + return room.is_session_live(session_id, timeout_seconds) + def create_room(self, room_id: str, session_id: str) -> None: room = Room(room_id, session_id) self.rooms[room_id] = room diff --git a/src/open_cups/room.py b/src/open_cups/room.py index fb55ca5..f4054c6 100644 --- a/src/open_cups/room.py +++ b/src/open_cups/room.py @@ -47,6 +47,14 @@ def has_session(self, session_id: str) -> bool: return True return bool(self.is_host(session_id)) + def is_session_live(self, session_id: str, timeout_seconds: float) -> bool: + current_time = time.time() + if self.is_host(session_id): + return current_time - self._host_last_seen <= timeout_seconds + if session_id not in self._sessions: + return False + return current_time - self._sessions[session_id].last_seen <= timeout_seconds + def get_participants_by_activity( self, inactivity_timeout_seconds: float, diff --git a/src/open_cups/session_state.py b/src/open_cups/session_state.py index 626c159..3b9ad53 100644 --- a/src/open_cups/session_state.py +++ b/src/open_cups/session_state.py @@ -2,20 +2,40 @@ import streamlit as st +from open_cups.application_state import ApplicationState + +# A live session refreshes its last_seen every autorefresh tick (~2s), so this +# threshold tolerates a couple of missed ticks before treating it as stale. +SESSION_LIVENESS_TIMEOUT_SECONDS = 10 + class SessionState: """Per-user session state wrapper. - See https://docs.streamlit.io/develop/api-reference/caching-and-state/st.session_state - for more details. + The session id is kept in the URL so it survives a websocket drop (a phone + locking, a page reload). To stop a copy-pasted link from cloning identity, + a session id from the URL is only adopted when that session is currently + stale (a genuine reconnect); if it is still live, the visitor is treated as + a new user. See https://github.com/BayerC/open_cups/issues/165. """ - def __init__(self) -> None: + def __init__(self, application_state: ApplicationState) -> None: if "session_id" not in st.session_state: - existing = st.query_params.get("session_id") - st.session_state.session_id = existing or str(uuid.uuid4()) + st.session_state.session_id = _resolve_session_id(application_state) st.query_params["session_id"] = st.session_state.session_id @property def session_id(self) -> str: return str(st.session_state.session_id) + + +def _resolve_session_id(application_state: ApplicationState) -> str: + url_session_id = st.query_params.get("session_id") + if url_session_id is None: + return str(uuid.uuid4()) + if application_state.is_session_live( + url_session_id, + SESSION_LIVENESS_TIMEOUT_SECONDS, + ): + return str(uuid.uuid4()) + return url_session_id diff --git a/src/open_cups/state_provider.py b/src/open_cups/state_provider.py index d93b0f3..fb1f538 100644 --- a/src/open_cups/state_provider.py +++ b/src/open_cups/state_provider.py @@ -105,7 +105,7 @@ def cleanup_all(self) -> None: class Context: def __init__(self) -> None: self.application_state: ApplicationState = self._get_application_state() - self.session_state = SessionState() + self.session_state = SessionState(self.application_state) @staticmethod @st.cache_resource diff --git a/tests/unit/test_room.py b/tests/unit/test_room.py index 73b77e7..b194842 100644 --- a/tests/unit/test_room.py +++ b/tests/unit/test_room.py @@ -104,6 +104,41 @@ def test_integration_with_stats_tracker(monkeypatch: pytest.MonkeyPatch) -> None assert room.get_status_history() == [] +def test_is_session_live_true_for_recent_host(mock_time: MockTime) -> None: + mock_time.current_time = 0.0 + room = Room("room-id", "host-id") + + mock_time.current_time = 5.0 + + assert room.is_session_live("host-id", timeout_seconds=10) + + +def test_is_session_live_true_for_recent_client(mock_time: MockTime) -> None: + mock_time.current_time = 0.0 + room = Room("room-id", "host-id") + room.set_session_status("user", UserStatus.GREEN) + + mock_time.current_time = 5.0 + + assert room.is_session_live("user", timeout_seconds=10) + + +def test_is_session_live_false_for_stale_client(mock_time: MockTime) -> None: + mock_time.current_time = 0.0 + room = Room("room-id", "host-id") + room.set_session_status("user", UserStatus.GREEN) + + mock_time.current_time = 20.0 + + assert not room.is_session_live("user", timeout_seconds=10) + + +def test_is_session_live_false_for_unknown_session() -> None: + room = Room("room-id", "host-id") + + assert not room.is_session_live("unknown-id", timeout_seconds=10) + + def test_get_participants_by_activity_separates_active_and_inactive( mock_time: MockTime, ) -> None: diff --git a/tests/unit/test_session_state.py b/tests/unit/test_session_state.py new file mode 100644 index 0000000..fbd2afb --- /dev/null +++ b/tests/unit/test_session_state.py @@ -0,0 +1,40 @@ +from streamlit.testing.v1 import AppTest + +from tests.bdd.fixture import run_wrapper +from tests.bdd.test_helper import get_room_id + + +def _resolved_session_id(app: AppTest) -> str: + value = app.query_params["session_id"] + resolved = value[0] if isinstance(value, list) else value + return str(resolved) + + +def test_fresh_visitor_gets_a_session_id() -> None: + app = AppTest.from_function(run_wrapper) + app.run() + + assert _resolved_session_id(app) + + +def test_url_session_id_adopted_when_not_live() -> None: + app = AppTest.from_function(run_wrapper) + app.query_params["session_id"] = "stale-or-unknown-id" + app.run() + + assert _resolved_session_id(app) == "stale-or-unknown-id" + + +def test_copy_pasted_live_session_forks_to_new_user() -> None: + host = AppTest.from_function(run_wrapper) + host.run() + host.button(key="start_room").click().run() + host_session_id = _resolved_session_id(host) + room_id = get_room_id(host) + + visitor = AppTest.from_function(run_wrapper) + visitor.query_params["room_id"] = room_id + visitor.query_params["session_id"] = host_session_id + visitor.run() + + assert _resolved_session_id(visitor) != host_session_id