diff --git a/crdt-sync/crdt_sync/tests/test_config.py b/crdt-sync/crdt_sync/tests/test_config.py index dc42ce9..b139e8b 100644 --- a/crdt-sync/crdt_sync/tests/test_config.py +++ b/crdt-sync/crdt_sync/tests/test_config.py @@ -21,11 +21,7 @@ import pytest from crdt_sync import ( ConfigError, FirebaseConfig, - FirebaseSyncClient, - credential_store_for, - mirror_client_for, ) -from crdt_sync._config import _DEFAULT_TIMEOUT_SECONDS, firebase_client_for if TYPE_CHECKING: from pathlib import Path @@ -141,215 +137,3 @@ def test_names_an_unfilled_placeholder(tmp_path: Path) -> None: with pytest.raises(ConfigError, match=r"placeholder for: apiKey"): FirebaseConfig.load(_write(tmp_path / "firebase.json", unfilled)) - - -def test_reads_a_password(tmp_path: Path) -> None: - """Reads a password.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - password = tmp_path / "password" - password.write_text("hunter2", encoding="utf-8") - - assert config.read_password(password) == "hunter2" - - -def test_strips_a_trailing_newline_from_the_password(tmp_path: Path) -> None: - """The likeliest hand-editing mistake, and otherwise invisible.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - password = tmp_path / "password" - password.write_text("hunter2\n", encoding="utf-8") - - assert config.read_password(password) == "hunter2" - - -def test_rejects_an_empty_password(tmp_path: Path) -> None: - """Rejects an empty password.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - password = tmp_path / "password" - password.write_text(" ", encoding="utf-8") - - with pytest.raises(ConfigError, match="empty or still holds"): - config.read_password(password) - - -def test_rejects_a_placeholder_password(tmp_path: Path) -> None: - """Rejects a placeholder password.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - password = tmp_path / "password" - password.write_text("PASTE_SYNC_ACCOUNT_PASSWORD_HERE", encoding="utf-8") - - with pytest.raises(ConfigError, match="placeholder"): - config.read_password(password) - - -def test_reports_an_unreadable_password(tmp_path: Path) -> None: - """Reports an unreadable password.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - (tmp_path / "password").mkdir() - - with pytest.raises(ConfigError, match="could not be read"): - config.read_password(tmp_path / "password") - - -def test_credential_store_is_per_app() -> None: - """Two apps must not share one cache file; concurrent writes corrupt it.""" - diet = credential_store_for("diet_guard") - alarm = credential_store_for("wake_alarm") - - assert diet._path != alarm._path - assert diet._path.name == "firebase_auth.json" - assert "diet_guard" in str(diet._path) - - -def test_client_signs_in_only_without_a_cached_session( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """The common path is a cached refresh token, costing no round trip.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - sign_ins: list[tuple[str, str]] = [] - - class _Auth: - def __init__(self, api_key: str, _store: object) -> None: - self.api_key = api_key - - def has_session(self) -> bool: - """Pretend a cached session exists.""" - return True - - def sign_in(self, email: str, password: str) -> None: - """Record the sign-in attempt.""" - sign_ins.append((email, password)) # pragma: no cover - must not run - - monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) - client = firebase_client_for("diet_guard", config=config) - - assert not sign_ins - assert client is not None - - -def test_client_signs_in_when_there_is_no_session( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Client signs in when there is no session.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - password = tmp_path / "password" - password.write_text("hunter2", encoding="utf-8") - monkeypatch.setattr("crdt_sync._config.PASSWORD_FILE", password) - sign_ins: list[tuple[str, str]] = [] - - class _Auth: - def __init__(self, api_key: str, _store: object) -> None: - self.api_key = api_key - - def has_session(self) -> bool: - """Pretend no cached session exists.""" - return False - - def sign_in(self, email: str, password_value: str) -> None: - """Record the credentials the caller signed in with.""" - sign_ins.append((email, password_value)) - - monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) - firebase_client_for("diet_guard", config=config) - - assert sign_ins == [("sync@example.com", "hunter2")] - - -def test_mirror_client_makes_firebase_primary( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Firebase authoritative, GitHub best-effort -- the cutover shape.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - github = _StubRemote() - - class _Auth: - def __init__(self, api_key: str, _store: object) -> None: - self.api_key = api_key - - def has_session(self) -> bool: - """Pretend a cached session exists.""" - return True - - def sign_in(self, email: str, password: str) -> None: - """Record the sign-in attempt.""" - raise AssertionError # pragma: no cover - must not run - - monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) - client = mirror_client_for("diet_guard", github, config=config) - - # Reaching into the wrapper is the point: which backend is authoritative - # is the one thing a rollback depends on getting right. - assert client.mirror is github - assert isinstance(client.primary, FirebaseSyncClient) - - -def test_client_loads_the_shared_config_when_none_is_given( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """The default path: apps name themselves and nothing else.""" - monkeypatch.setattr( - "crdt_sync._config.CONFIG_FILE", - _write(tmp_path / "firebase.json", _VALID), - ) - - class _Auth: - def __init__(self, api_key: str, _store: object) -> None: - self.api_key = api_key - - def has_session(self) -> bool: - """Pretend a cached session exists.""" - return True - - def sign_in(self, email: str, password: str) -> None: - """Record the sign-in attempt.""" - raise AssertionError # pragma: no cover - must not run - - monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) - - assert firebase_client_for("wake_alarm") is not None - - -def _client_with_stub_auth( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, - **kwargs: float, -) -> FirebaseSyncClient: - """Return a client built with stubbed auth, forwarding ``kwargs``.""" - config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) - - class _Auth: - def __init__(self, api_key: str, _store: object) -> None: - """Record the key; the credential store is irrelevant here.""" - self.api_key = api_key - - def has_session(self) -> bool: - """Pretend a cached session exists, so no sign-in is attempted.""" - return True - - monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) - return firebase_client_for("diet_guard", config=config, **kwargs) - - -def test_client_uses_the_requested_timeout( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """A caller's timeout must reach the client, not just the GitHub half.""" - # A keyword with a default adds no branch, so coverage alone cannot tell - # whether it is actually threaded through to the client. - client = _client_with_stub_auth(tmp_path, monkeypatch, timeout_seconds=3) - - assert client._timeout_seconds == 3 - - -def test_client_timeout_defaults_to_the_shared_value( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Omitting the keyword keeps the library-wide default.""" - client = _client_with_stub_auth(tmp_path, monkeypatch) - - assert client._timeout_seconds == _DEFAULT_TIMEOUT_SECONDS diff --git a/crdt-sync/crdt_sync/tests/test_config_client.py b/crdt-sync/crdt_sync/tests/test_config_client.py new file mode 100644 index 0000000..8283493 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_config_client.py @@ -0,0 +1,233 @@ +"""Tests for building a sync client from a config file. + +Split from ``test_config.py`` (250-line cap); +``test_config_credentials.py`` keeps reading and validating the password. +""" + +# These tests assert on private attributes on purpose: `_timeout_seconds` +# and `_path` are exactly what the fixes under test set, and there is no +# public accessor for either. +# pylint: disable=protected-access + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import pytest + +from crdt_sync import ( + FirebaseConfig, + FirebaseSyncClient, + credential_store_for, + mirror_client_for, +) +from crdt_sync._config import _DEFAULT_TIMEOUT_SECONDS, firebase_client_for + +if TYPE_CHECKING: + from pathlib import Path + +_VALID = { + "apiKey": "AIzaSyExample", + "databaseUrl": "https://kuhy-syncs-default-rtdb.europe-west1.firebasedatabase.app", + "projectId": "kuhy-syncs", + "uid": "OvA2REQyLIhAHOEjzwS1o877rgG3", + "email": "sync@example.com", +} + + +class _StubRemote: + """A do-nothing :class:`~crdt_sync.RemoteStore`, standing in for GitHub. + + A real implementation rather than a bare ``object()`` so the type checker + accepts it without a suppression. + """ + + def list_directory(self, _path: str) -> list[str]: + """Report an empty remote directory.""" + return [] + + def get_file_text(self, _path: str) -> str | None: + """Report the file as absent.""" + return None + + def put_file_text(self, path: str, text: str, *, message: str) -> None: + """Accept and discard a write.""" + + def delete_file(self, path: str, *, message: str = "") -> None: + """Accept and discard a delete.""" + + def can_access_remote(self) -> bool: + """Report the remote as reachable.""" + return True + + +def _write(path: Path, data: object) -> Path: + """Write ``data`` as JSON to ``path`` and return it.""" + path.write_text(json.dumps(data), encoding="utf-8") + return path + + +def test_credential_store_is_per_app() -> None: + """Two apps must not share one cache file; concurrent writes corrupt it.""" + diet = credential_store_for("diet_guard") + alarm = credential_store_for("wake_alarm") + + assert diet._path != alarm._path + assert diet._path.name == "firebase_auth.json" + assert "diet_guard" in str(diet._path) + + +def test_client_signs_in_only_without_a_cached_session( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The common path is a cached refresh token, costing no round trip.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + sign_ins: list[tuple[str, str]] = [] + + class _Auth: + def __init__(self, api_key: str, _store: object) -> None: + self.api_key = api_key + + def has_session(self) -> bool: + """Pretend a cached session exists.""" + return True + + def sign_in(self, email: str, password: str) -> None: + """Record the sign-in attempt.""" + sign_ins.append((email, password)) # pragma: no cover - must not run + + monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) + client = firebase_client_for("diet_guard", config=config) + + assert not sign_ins + assert client is not None + + +def test_client_signs_in_when_there_is_no_session( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Client signs in when there is no session.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + password = tmp_path / "password" + password.write_text("hunter2", encoding="utf-8") + monkeypatch.setattr("crdt_sync._config.PASSWORD_FILE", password) + sign_ins: list[tuple[str, str]] = [] + + class _Auth: + def __init__(self, api_key: str, _store: object) -> None: + self.api_key = api_key + + def has_session(self) -> bool: + """Pretend no cached session exists.""" + return False + + def sign_in(self, email: str, password_value: str) -> None: + """Record the credentials the caller signed in with.""" + sign_ins.append((email, password_value)) + + monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) + firebase_client_for("diet_guard", config=config) + + assert sign_ins == [("sync@example.com", "hunter2")] + + +def test_mirror_client_makes_firebase_primary( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Firebase authoritative, GitHub best-effort -- the cutover shape.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + github = _StubRemote() + + class _Auth: + def __init__(self, api_key: str, _store: object) -> None: + self.api_key = api_key + + def has_session(self) -> bool: + """Pretend a cached session exists.""" + return True + + def sign_in(self, email: str, password: str) -> None: + """Record the sign-in attempt.""" + raise AssertionError # pragma: no cover - must not run + + monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) + client = mirror_client_for("diet_guard", github, config=config) + + # Reaching into the wrapper is the point: which backend is authoritative + # is the one thing a rollback depends on getting right. + assert client.mirror is github + assert isinstance(client.primary, FirebaseSyncClient) + + +def test_client_loads_the_shared_config_when_none_is_given( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The default path: apps name themselves and nothing else.""" + monkeypatch.setattr( + "crdt_sync._config.CONFIG_FILE", + _write(tmp_path / "firebase.json", _VALID), + ) + + class _Auth: + def __init__(self, api_key: str, _store: object) -> None: + self.api_key = api_key + + def has_session(self) -> bool: + """Pretend a cached session exists.""" + return True + + def sign_in(self, email: str, password: str) -> None: + """Record the sign-in attempt.""" + raise AssertionError # pragma: no cover - must not run + + monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) + + assert firebase_client_for("wake_alarm") is not None + + +def _client_with_stub_auth( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + **kwargs: float, +) -> FirebaseSyncClient: + """Return a client built with stubbed auth, forwarding ``kwargs``.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + + class _Auth: + def __init__(self, api_key: str, _store: object) -> None: + """Record the key; the credential store is irrelevant here.""" + self.api_key = api_key + + def has_session(self) -> bool: + """Pretend a cached session exists, so no sign-in is attempted.""" + return True + + monkeypatch.setattr("crdt_sync._config.FirebaseTokenProvider", _Auth) + return firebase_client_for("diet_guard", config=config, **kwargs) + + +def test_client_uses_the_requested_timeout( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A caller's timeout must reach the client, not just the GitHub half.""" + # A keyword with a default adds no branch, so coverage alone cannot tell + # whether it is actually threaded through to the client. + client = _client_with_stub_auth(tmp_path, monkeypatch, timeout_seconds=3) + + assert client._timeout_seconds == 3 + + +def test_client_timeout_defaults_to_the_shared_value( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Omitting the keyword keeps the library-wide default.""" + client = _client_with_stub_auth(tmp_path, monkeypatch) + + assert client._timeout_seconds == _DEFAULT_TIMEOUT_SECONDS diff --git a/crdt-sync/crdt_sync/tests/test_config_credentials.py b/crdt-sync/crdt_sync/tests/test_config_credentials.py new file mode 100644 index 0000000..a6e11de --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_config_credentials.py @@ -0,0 +1,112 @@ +"""Tests for reading the sync password and building a client from config. + +Split from ``test_config.py`` (250-line cap), which keeps loading and +validating the config document itself. +""" + +# These tests assert on private attributes on purpose: `_timeout_seconds` +# and `_path` are exactly what the fixes under test set, and there is no +# public accessor for either. +# pylint: disable=protected-access + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import pytest + +from crdt_sync import ( + ConfigError, + FirebaseConfig, +) + +if TYPE_CHECKING: + from pathlib import Path + +_VALID = { + "apiKey": "AIzaSyExample", + "databaseUrl": "https://kuhy-syncs-default-rtdb.europe-west1.firebasedatabase.app", + "projectId": "kuhy-syncs", + "uid": "OvA2REQyLIhAHOEjzwS1o877rgG3", + "email": "sync@example.com", +} + + +class _StubRemote: + """A do-nothing :class:`~crdt_sync.RemoteStore`, standing in for GitHub. + + A real implementation rather than a bare ``object()`` so the type checker + accepts it without a suppression. + """ + + def list_directory(self, _path: str) -> list[str]: + """Report an empty remote directory.""" + return [] + + def get_file_text(self, _path: str) -> str | None: + """Report the file as absent.""" + return None + + def put_file_text(self, path: str, text: str, *, message: str) -> None: + """Accept and discard a write.""" + + def delete_file(self, path: str, *, message: str = "") -> None: + """Accept and discard a delete.""" + + def can_access_remote(self) -> bool: + """Report the remote as reachable.""" + return True + + +def _write(path: Path, data: object) -> Path: + """Write ``data`` as JSON to ``path`` and return it.""" + path.write_text(json.dumps(data), encoding="utf-8") + return path + + +def test_reads_a_password(tmp_path: Path) -> None: + """Reads a password.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + password = tmp_path / "password" + password.write_text("hunter2", encoding="utf-8") + + assert config.read_password(password) == "hunter2" + + +def test_strips_a_trailing_newline_from_the_password(tmp_path: Path) -> None: + """The likeliest hand-editing mistake, and otherwise invisible.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + password = tmp_path / "password" + password.write_text("hunter2\n", encoding="utf-8") + + assert config.read_password(password) == "hunter2" + + +def test_rejects_an_empty_password(tmp_path: Path) -> None: + """Rejects an empty password.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + password = tmp_path / "password" + password.write_text(" ", encoding="utf-8") + + with pytest.raises(ConfigError, match="empty or still holds"): + config.read_password(password) + + +def test_rejects_a_placeholder_password(tmp_path: Path) -> None: + """Rejects a placeholder password.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + password = tmp_path / "password" + password.write_text("PASTE_SYNC_ACCOUNT_PASSWORD_HERE", encoding="utf-8") + + with pytest.raises(ConfigError, match="placeholder"): + config.read_password(password) + + +def test_reports_an_unreadable_password(tmp_path: Path) -> None: + """Reports an unreadable password.""" + config = FirebaseConfig.load(_write(tmp_path / "firebase.json", _VALID)) + (tmp_path / "password").mkdir() + + with pytest.raises(ConfigError, match="could not be read"): + config.read_password(tmp_path / "password") diff --git a/crdt-sync/crdt_sync/tests/test_firebase.py b/crdt-sync/crdt_sync/tests/test_firebase.py index eb25b9b..599fd49 100644 --- a/crdt-sync/crdt_sync/tests/test_firebase.py +++ b/crdt-sync/crdt_sync/tests/test_firebase.py @@ -25,9 +25,6 @@ from crdt_sync import ( FirebaseSyncError, FirebaseTokenProvider, MemoryCredentialStore, - RemoteNotFoundError, - RemoteStore, - RemoteSyncError, ) from crdt_sync import _firebase as fb from crdt_sync._firebase import decode_key, encode_key, encode_path @@ -210,160 +207,3 @@ class TestListDirectory: pytest.raises(FirebaseSyncError), ): _client().list_directory("ns/devices") - - -class TestPutFileText: - """Put file text.""" - - def test_writes_the_text_as_a_json_string_leaf(self) -> None: - """Writes the text as a JSON string leaf.""" - with ( - patch.object(fb.requests, "get", return_value=_response(200, None)), - patch.object( - fb.requests, "put", return_value=_response(200, '{"a":1}') - ) as put_mock, - ): - _client().put_file_text( - "ns/devices/pc/log.json", '{"a":1}', message="ignored" - ) - assert put_mock.call_args.kwargs["json"] == '{"a":1}' - assert put_mock.call_args.args[0].endswith("/log~2Ejson.json") - - def test_raises_on_a_non_2xx_response(self) -> None: - """Raises on a non 2xx response.""" - with ( - patch.object( - fb.requests, "put", return_value=_response(400, {"error": "bad"}) - ), - pytest.raises(FirebaseSyncError), - ): - _client().put_file_text("ns/devices/pc/log.json", "{}", message="m") - - def test_turns_a_network_failure_into_a_sync_error(self) -> None: - """Turns a network failure into a sync error.""" - with ( - patch.object( - fb.requests, "put", side_effect=requests.ConnectionError("offline") - ), - pytest.raises(FirebaseSyncError, match="network error"), - ): - _client().put_file_text("ns/devices/pc/log.json", "{}", message="m") - - -class TestGetStringMap: - """Get string map.""" - - def test_returns_the_map_with_keys_decoded(self) -> None: - """Returns the map with keys decoded.""" - payload = {"pc": "sha-1", "phone": "sha-2"} - with patch.object(fb.requests, "get", return_value=_response(200, payload)): - assert _client().get_string_map("ns/revs") == payload - - def test_degrades_to_empty_when_absent(self) -> None: - """Degrades to empty when absent.""" - with patch.object(fb.requests, "get", return_value=_response(200, None)): - assert not _client().get_string_map("ns/revs") - - def test_skips_non_string_entries_rather_than_failing(self) -> None: - # The revs node is an optimisation; a corrupt one must degrade into - # "fetch everything", never into a failed tick. - """Skips non string entries rather than failing.""" - with patch.object( - fb.requests, "get", return_value=_response(200, {"pc": "sha-1", "b": 42}) - ): - assert _client().get_string_map("ns/revs") == {"pc": "sha-1"} - - def test_raises_on_a_non_2xx_response(self) -> None: - """Raises on a non 2xx response.""" - with ( - patch.object( - fb.requests, "get", return_value=_response(500, {"error": "boom"}) - ), - pytest.raises(FirebaseSyncError), - ): - _client().get_string_map("ns/revs") - - -class TestDeleteFile: - """Delete file.""" - - def test_sends_a_delete_to_the_escaped_path(self) -> None: - """Sends a delete to the escaped path.""" - with patch.object( - fb.requests, "delete", return_value=_response(200, None) - ) as delete_mock: - _client().delete_file("ns/devices/pc/log.json") - assert delete_mock.call_args.args[0].endswith("/log~2Ejson.json") - - def test_raises_on_a_non_2xx_response(self) -> None: - """Raises on a non 2xx response.""" - with ( - patch.object( - fb.requests, "delete", return_value=_response(500, {"error": "boom"}) - ), - pytest.raises(FirebaseSyncError), - ): - _client().delete_file("ns/devices/pc/log.json") - - def test_turns_a_network_failure_into_a_sync_error(self) -> None: - """Turns a network failure into a sync error.""" - with ( - patch.object( - fb.requests, "delete", side_effect=requests.ConnectionError("x") - ), - pytest.raises(FirebaseSyncError, match="network error"), - ): - _client().delete_file("ns/devices/pc/log.json") - - -class TestCanAccessRemote: - """Can access remote.""" - - def test_true_when_the_database_root_reads(self) -> None: - """True when the database root reads.""" - with patch.object(fb.requests, "get", return_value=_response(200, {"ns": 1})): - assert _client().can_access_remote() is True - - def test_false_when_the_rules_reject_the_token(self) -> None: - """False when the rules reject the token.""" - with patch.object( - fb.requests, "get", return_value=_response(401, {"error": "denied"}) - ): - assert _client().can_access_remote() is False - - def test_false_on_a_network_error(self) -> None: - """False on a network error.""" - with patch.object( - fb.requests, "get", side_effect=requests.ConnectionError("offline") - ): - assert _client().can_access_remote() is False - - def test_false_when_not_signed_in_rather_than_raising(self) -> None: - # "cannot get a token" is exactly "cannot access the remote", and a - # settings Test-connection button must not blow up. - """False when not signed in rather than raising.""" - assert _client(signed_in=False).can_access_remote() is False - - -class TestRemoteStoreContract: - """Remote store contract.""" - - def test_the_client_satisfies_remote_store(self) -> None: - """The client satisfies remote store.""" - assert isinstance(_client(), RemoteStore) - - def test_database_not_found_is_catchable_as_either_error(self) -> None: - """Database not found is catchable as either error.""" - error = DatabaseNotFoundError("gone") - assert isinstance(error, FirebaseSyncError) - assert isinstance(error, RemoteNotFoundError) - assert isinstance(error, RemoteSyncError) - - def test_a_trailing_slash_on_the_database_url_is_tolerated(self) -> None: - """A trailing slash on the database URL is tolerated.""" - client = FirebaseSyncClient(f"{_DATABASE_URL}/", _auth()) - with patch.object( - fb.requests, "get", return_value=_response(200, "ok") - ) as get_mock: - client.get_file_text("ns/x.json") - assert get_mock.call_args.args[0] == f"{_DATABASE_URL}/ns/x~2Ejson.json" diff --git a/crdt-sync/crdt_sync/tests/test_firebase_auth.py b/crdt-sync/crdt_sync/tests/test_firebase_auth.py index 936fcbb..9ce8e5b 100644 --- a/crdt-sync/crdt_sync/tests/test_firebase_auth.py +++ b/crdt-sync/crdt_sync/tests/test_firebase_auth.py @@ -9,14 +9,12 @@ from __future__ import annotations import datetime as dt import json -from typing import TYPE_CHECKING from unittest.mock import MagicMock, patch import pytest import requests from crdt_sync import ( - FileCredentialStore, FirebaseAuthError, FirebaseCredentials, FirebaseTokenProvider, @@ -24,9 +22,6 @@ from crdt_sync import ( ) from crdt_sync import _firebase_auth as fa -if TYPE_CHECKING: - from pathlib import Path - _NOW = dt.datetime(2026, 8, 3, 12, tzinfo=dt.timezone.utc) # Fixture values referenced through names ruff's hardcoded-credential checks @@ -173,267 +168,3 @@ class TestSignIn: pytest.raises(FirebaseAuthError, match="network error"), ): _provider().sign_in("a@b.c", "x") - - -class TestSignInWithGoogle: - """Sign in with google.""" - - @staticmethod - def _google_body(**overrides: object) -> dict[str, object]: - body: dict[str, object] = { - "idToken": _ID_1, - "refreshToken": _REFRESH_1, - "expiresIn": "3600", - "localId": _UID, - "email": "me@example.com", - } - body.update(overrides) - return body - - def test_stores_the_session(self) -> None: - """Stores the session.""" - store = MemoryCredentialStore() - with _patch_post(_response(200, self._google_body())): - _provider(store).sign_in_with_google("google-jwt") - saved = store.load() - assert saved is not None - assert saved.id_token == _ID_1 - assert saved.refresh_token == _REFRESH_1 - assert saved.expires_at == _NOW + dt.timedelta(hours=1) - - def test_returns_the_account_email(self) -> None: - """Returns the account email.""" - with _patch_post(_response(200, self._google_body())): - email = _provider().sign_in_with_google("google-jwt") - assert email == "me@example.com" - - def test_returns_an_empty_email_when_google_omits_one(self) -> None: - # Firebase leaves `email` out when the Google account has no verified - # address. That is not a failure -- the uid check is what matters -- - # so the caller gets "" rather than a KeyError. - """Returns an empty email when google omits one.""" - body = self._google_body() - del body["email"] - with _patch_post(_response(200, body)): - assert not _provider().sign_in_with_google("google-jwt") - - def test_sends_the_credential_form_encoded_in_post_body(self) -> None: - # identitytoolkit rejects the IdP credential as a JSON field with - # INVALID_IDP_RESPONSE, which reads like a bad token; pin the shape. - """Sends the credential form encoded in post body.""" - post = MagicMock(return_value=_response(200, self._google_body())) - with patch.object(fa.requests, "post", post): - _provider().sign_in_with_google("google-jwt") - sent = post.call_args.kwargs["json"] - assert sent["postBody"] == "id_token=google-jwt&providerId=google.com" - assert "signInWithIdp" in post.call_args.args[0] - - def test_accepts_the_uid_the_data_belongs_to(self) -> None: - """Accepts the uid the data belongs to.""" - store = MemoryCredentialStore() - with _patch_post(_response(200, self._google_body())): - _provider(store).sign_in_with_google("google-jwt", expected_uid=_UID) - assert store.load() is not None - - def test_rejects_and_stores_nothing_for_a_different_uid(self) -> None: - # The signed-up identity does exist server-side by now -- this call - # omits `idToken`, so an unlinked Google account is signed *up* before - # the uid is known. What this guarantees is that no such session is - # persisted locally, where it would be denied every read and write. - """Rejects and stores nothing for a different uid.""" - store = MemoryCredentialStore() - body = self._google_body(localId="someone-else") - with ( - _patch_post(_response(200, body)), - pytest.raises(FirebaseAuthError, match="wrong account"), - ): - _provider(store).sign_in_with_google("google-jwt", expected_uid=_UID) - assert store.load() is None - - def test_reports_googles_reason_for_a_rejected_token(self) -> None: - """Reports googles reason for a rejected token.""" - body = {"error": {"message": "INVALID_IDP_RESPONSE"}} - with ( - _patch_post(_response(400, body)), - pytest.raises(FirebaseAuthError, match="INVALID_IDP_RESPONSE"), - ): - _provider().sign_in_with_google("stale-jwt") - - -class TestIdToken: - """Id token.""" - - def test_fails_loudly_when_not_signed_in(self) -> None: - # Never returns None or a stale token: a sync that quietly stops - # syncing is the failure mode this design exists to prevent. - """Fails loudly when not signed in.""" - with pytest.raises(FirebaseAuthError, match="not signed in"): - _provider().id_token() - - def test_returns_the_stored_token_without_a_network_call(self) -> None: - """Returns the stored token without a network call.""" - provider = _provider(MemoryCredentialStore(_credentials())) - with patch.object(fa.requests, "post", side_effect=AssertionError("no HTTP")): - assert provider.id_token() == _ID_1 - - def test_refreshes_an_expired_token_and_keeps_the_rotated_one(self) -> None: - """Refreshes an expired token and keeps the rotated one.""" - store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) - body = { - "id_token": _ID_2, - "refresh_token": _REFRESH_2, - "expires_in": "3600", - } - with _patch_post(_response(200, body)): - assert _provider(store).id_token() == _ID_2 - saved = store.load() - assert saved is not None - assert saved.refresh_token == _REFRESH_2 - - def test_fails_loudly_when_the_refresh_token_was_revoked(self) -> None: - """Fails loudly when the refresh token was revoked.""" - store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) - with ( - _patch_post(_response(400, {"error": {"message": "TOKEN_EXPIRED"}})), - pytest.raises(FirebaseAuthError, match="TOKEN_EXPIRED"), - ): - _provider(store).id_token() - - def test_a_revoked_refresh_token_is_cleared(self) -> None: - """A dead session must stop reporting itself as connected. - - Regression: ``has_session`` is a presence check, so a revoked token - kept reading as "connected" while every sync failed with - TOKEN_EXPIRED. - """ - store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) - provider = _provider(store) - with ( - _patch_post(_response(400, {"error": {"message": "TOKEN_EXPIRED"}})), - pytest.raises(FirebaseAuthError), - ): - provider.id_token() - assert provider.has_session() is False - assert store.load() is None - - def test_a_network_error_keeps_the_session(self) -> None: - """Signing out on a transient failure would need a manual re-login.""" - store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) - provider = _provider(store) - with ( - patch.object( - fa.requests, - "post", - side_effect=requests.ConnectionError("connection reset"), - ), - pytest.raises(FirebaseAuthError), - ): - provider.id_token() - assert provider.has_session() is True - assert store.load() is not None - - def test_a_server_error_keeps_the_session(self) -> None: - """A server error keeps the session.""" - store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) - provider = _provider(store) - with ( - _patch_post(_response(503, {"error": {"message": "SERVICE_UNAVAILABLE"}})), - pytest.raises(FirebaseAuthError), - ): - provider.id_token() - assert provider.has_session() is True - - -class TestSessionLifecycle: - """Session lifecycle.""" - - def test_has_session_is_false_before_sign_in_and_true_after(self) -> None: - """Has session is false before sign in and true after.""" - store = MemoryCredentialStore() - provider = _provider(store) - assert provider.has_session() is False - body = {"idToken": _ID_1, "refreshToken": _REFRESH_1, "expiresIn": "3600"} - with _patch_post(_response(200, body)): - provider.sign_in("a@b.c", "x") - assert provider.has_session() is True - - def test_sign_out_clears_the_store(self) -> None: - """Sign out clears the store.""" - store = MemoryCredentialStore(_credentials()) - provider = _provider(store) - assert provider.id_token() == _ID_1 - provider.sign_out() - assert store.load() is None - with pytest.raises(FirebaseAuthError): - provider.id_token() - - def test_defaults_to_the_real_clock_when_none_is_injected(self) -> None: - """Defaults to the real clock when none is injected.""" - provider = FirebaseTokenProvider("k", MemoryCredentialStore()) - assert provider.has_session() is False - - -class TestFileCredentialStore: - """File credential store.""" - - def test_round_trips_through_a_file(self, tmp_path: Path) -> None: - """Round trips through a file.""" - store = FileCredentialStore(tmp_path / "nested" / "creds.json") - store.save(_credentials()) - assert store.load() == _credentials() - - def test_writes_the_file_readable_only_by_this_user(self, tmp_path: Path) -> None: - # The refresh token is the real secret; it must never be briefly - # world-readable between write and chmod. - """Writes the file readable only by this user.""" - path = tmp_path / "creds.json" - FileCredentialStore(path).save(_credentials()) - assert path.stat().st_mode & 0o777 == 0o600 - - def test_load_returns_none_when_absent(self, tmp_path: Path) -> None: - """Load returns none when absent.""" - assert FileCredentialStore(tmp_path / "missing.json").load() is None - - def test_load_returns_none_for_a_truncated_file(self, tmp_path: Path) -> None: - # An interrupted write reads as "not signed in"; the caller's next - # step is to sign in again, which repairs it. - """Load returns none for a truncated file.""" - path = tmp_path / "creds.json" - path.write_text('{"id_token": "a"', encoding="utf-8") - assert FileCredentialStore(path).load() is None - - def test_load_returns_none_when_fields_are_missing(self, tmp_path: Path) -> None: - """Load returns none when fields are missing.""" - path = tmp_path / "creds.json" - path.write_text('{"id_token": "a"}', encoding="utf-8") - assert FileCredentialStore(path).load() is None - - def test_load_returns_none_for_an_unparsable_expiry(self, tmp_path: Path) -> None: - """Load returns none for an unparsable expiry.""" - path = tmp_path / "creds.json" - path.write_text( - json.dumps( - {"id_token": "a", "refresh_token": "b", "expires_at": "not-a-date"} - ), - encoding="utf-8", - ) - assert FileCredentialStore(path).load() is None - - def test_clear_removes_the_file_and_is_idempotent(self, tmp_path: Path) -> None: - """Clear removes the file and is idempotent.""" - path = tmp_path / "creds.json" - store = FileCredentialStore(path) - store.save(_credentials()) - store.clear() - assert not path.exists() - store.clear() - - def test_overwrites_an_existing_file(self, tmp_path: Path) -> None: - """Overwrites an existing file.""" - path = tmp_path / "creds.json" - store = FileCredentialStore(path) - store.save(_credentials()) - store.save(_credentials(id_value=_ID_2)) - loaded = store.load() - assert loaded is not None - assert loaded.id_token == _ID_2 diff --git a/crdt-sync/crdt_sync/tests/test_firebase_auth_google.py b/crdt-sync/crdt_sync/tests/test_firebase_auth_google.py new file mode 100644 index 0000000..ced4a42 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_firebase_auth_google.py @@ -0,0 +1,159 @@ +"""Tests for the Google sign-in path. + +Split from ``test_firebase_auth.py`` (250-line cap), which keeps credential +handling and password sign-in. +""" + + +from __future__ import annotations + +import datetime as dt +import json +from unittest.mock import MagicMock, patch + +import pytest + +from crdt_sync import ( + FirebaseAuthError, + FirebaseCredentials, + FirebaseTokenProvider, + MemoryCredentialStore, +) +from crdt_sync import _firebase_auth as fa + +_NOW = dt.datetime(2026, 8, 3, 12, tzinfo=dt.timezone.utc) + +# Fixture values referenced through names ruff's hardcoded-credential checks +# (S105-S107) do not key off. Comparing an attribute called `id_token` to a +# bare string literal trips S105 no matter how obviously fake the value is. +_ID_1 = "id-1" +_ID_2 = "id-2" +_REFRESH_1 = "refresh-1" +_REFRESH_2 = "refresh-2" +_UID = "uid-the-data-belongs-to" + + +def _response(status_code: int = 200, json_data: object = None) -> MagicMock: + """Build a fake ``requests.Response`` with a status and JSON body.""" + response = MagicMock() + response.status_code = status_code + response.ok = 200 <= status_code < 300 + response.json = MagicMock(return_value=json_data if json_data is not None else {}) + response.text = json.dumps(json_data) if json_data is not None else "" + return response + + +def _provider( + store: object = None, + *, + now: dt.datetime = _NOW, +) -> FirebaseTokenProvider: + return FirebaseTokenProvider( + "fake-api-key", + store or MemoryCredentialStore(), + clock=lambda: now, + ) + + +# Parameters named without "token": ruff's hardcoded-credential check (S107) +# keys off the identifier, and these are obviously fake test fixtures. +def _credentials( + *, + id_value: str = _ID_1, + refresh_value: str = _REFRESH_1, + valid_for: dt.timedelta = dt.timedelta(hours=1), +) -> FirebaseCredentials: + return FirebaseCredentials( + id_token=id_value, + refresh_token=refresh_value, + expires_at=_NOW + valid_for, + ) + + +def _patch_post(*responses: MagicMock) -> object: + return patch.object(fa.requests, "post", side_effect=list(responses)) + + +class TestSignInWithGoogle: + """Sign in with google.""" + + @staticmethod + def _google_body(**overrides: object) -> dict[str, object]: + body: dict[str, object] = { + "idToken": _ID_1, + "refreshToken": _REFRESH_1, + "expiresIn": "3600", + "localId": _UID, + "email": "me@example.com", + } + body.update(overrides) + return body + + def test_stores_the_session(self) -> None: + """Stores the session.""" + store = MemoryCredentialStore() + with _patch_post(_response(200, self._google_body())): + _provider(store).sign_in_with_google("google-jwt") + saved = store.load() + assert saved is not None + assert saved.id_token == _ID_1 + assert saved.refresh_token == _REFRESH_1 + assert saved.expires_at == _NOW + dt.timedelta(hours=1) + + def test_returns_the_account_email(self) -> None: + """Returns the account email.""" + with _patch_post(_response(200, self._google_body())): + email = _provider().sign_in_with_google("google-jwt") + assert email == "me@example.com" + + def test_returns_an_empty_email_when_google_omits_one(self) -> None: + # Firebase leaves `email` out when the Google account has no verified + # address. That is not a failure -- the uid check is what matters -- + # so the caller gets "" rather than a KeyError. + """Returns an empty email when google omits one.""" + body = self._google_body() + del body["email"] + with _patch_post(_response(200, body)): + assert not _provider().sign_in_with_google("google-jwt") + + def test_sends_the_credential_form_encoded_in_post_body(self) -> None: + # identitytoolkit rejects the IdP credential as a JSON field with + # INVALID_IDP_RESPONSE, which reads like a bad token; pin the shape. + """Sends the credential form encoded in post body.""" + post = MagicMock(return_value=_response(200, self._google_body())) + with patch.object(fa.requests, "post", post): + _provider().sign_in_with_google("google-jwt") + sent = post.call_args.kwargs["json"] + assert sent["postBody"] == "id_token=google-jwt&providerId=google.com" + assert "signInWithIdp" in post.call_args.args[0] + + def test_accepts_the_uid_the_data_belongs_to(self) -> None: + """Accepts the uid the data belongs to.""" + store = MemoryCredentialStore() + with _patch_post(_response(200, self._google_body())): + _provider(store).sign_in_with_google("google-jwt", expected_uid=_UID) + assert store.load() is not None + + def test_rejects_and_stores_nothing_for_a_different_uid(self) -> None: + # The signed-up identity does exist server-side by now -- this call + # omits `idToken`, so an unlinked Google account is signed *up* before + # the uid is known. What this guarantees is that no such session is + # persisted locally, where it would be denied every read and write. + """Rejects and stores nothing for a different uid.""" + store = MemoryCredentialStore() + body = self._google_body(localId="someone-else") + with ( + _patch_post(_response(200, body)), + pytest.raises(FirebaseAuthError, match="wrong account"), + ): + _provider(store).sign_in_with_google("google-jwt", expected_uid=_UID) + assert store.load() is None + + def test_reports_googles_reason_for_a_rejected_token(self) -> None: + """Reports googles reason for a rejected token.""" + body = {"error": {"message": "INVALID_IDP_RESPONSE"}} + with ( + _patch_post(_response(400, body)), + pytest.raises(FirebaseAuthError, match="INVALID_IDP_RESPONSE"), + ): + _provider().sign_in_with_google("stale-jwt") diff --git a/crdt-sync/crdt_sync/tests/test_firebase_auth_session.py b/crdt-sync/crdt_sync/tests/test_firebase_auth_session.py new file mode 100644 index 0000000..8178a34 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_firebase_auth_session.py @@ -0,0 +1,175 @@ +"""Tests for the auth session lifecycle. + +Split from ``test_firebase_auth.py`` (250-line cap); +``test_firebase_auth_tokens.py`` keeps ID-token refresh. +""" + + + +from __future__ import annotations + +import datetime as dt +import json +from typing import TYPE_CHECKING +from unittest.mock import MagicMock, patch + +import pytest + +from crdt_sync import ( + FileCredentialStore, + FirebaseAuthError, + FirebaseCredentials, + FirebaseTokenProvider, + MemoryCredentialStore, +) +from crdt_sync import _firebase_auth as fa + +if TYPE_CHECKING: + from pathlib import Path + +_NOW = dt.datetime(2026, 8, 3, 12, tzinfo=dt.timezone.utc) + +# Fixture values referenced through names ruff's hardcoded-credential checks +# (S105-S107) do not key off. Comparing an attribute called `id_token` to a +# bare string literal trips S105 no matter how obviously fake the value is. +_ID_1 = "id-1" +_ID_2 = "id-2" +_REFRESH_1 = "refresh-1" +_REFRESH_2 = "refresh-2" +_UID = "uid-the-data-belongs-to" + + +def _response(status_code: int = 200, json_data: object = None) -> MagicMock: + """Build a fake ``requests.Response`` with a status and JSON body.""" + response = MagicMock() + response.status_code = status_code + response.ok = 200 <= status_code < 300 + response.json = MagicMock(return_value=json_data if json_data is not None else {}) + response.text = json.dumps(json_data) if json_data is not None else "" + return response + + +def _provider( + store: object = None, + *, + now: dt.datetime = _NOW, +) -> FirebaseTokenProvider: + return FirebaseTokenProvider( + "fake-api-key", + store or MemoryCredentialStore(), + clock=lambda: now, + ) + + +# Parameters named without "token": ruff's hardcoded-credential check (S107) +# keys off the identifier, and these are obviously fake test fixtures. +def _credentials( + *, + id_value: str = _ID_1, + refresh_value: str = _REFRESH_1, + valid_for: dt.timedelta = dt.timedelta(hours=1), +) -> FirebaseCredentials: + return FirebaseCredentials( + id_token=id_value, + refresh_token=refresh_value, + expires_at=_NOW + valid_for, + ) + + +def _patch_post(*responses: MagicMock) -> object: + return patch.object(fa.requests, "post", side_effect=list(responses)) + + +class TestSessionLifecycle: + """Session lifecycle.""" + + def test_has_session_is_false_before_sign_in_and_true_after(self) -> None: + """Has session is false before sign in and true after.""" + store = MemoryCredentialStore() + provider = _provider(store) + assert provider.has_session() is False + body = {"idToken": _ID_1, "refreshToken": _REFRESH_1, "expiresIn": "3600"} + with _patch_post(_response(200, body)): + provider.sign_in("a@b.c", "x") + assert provider.has_session() is True + + def test_sign_out_clears_the_store(self) -> None: + """Sign out clears the store.""" + store = MemoryCredentialStore(_credentials()) + provider = _provider(store) + assert provider.id_token() == _ID_1 + provider.sign_out() + assert store.load() is None + with pytest.raises(FirebaseAuthError): + provider.id_token() + + def test_defaults_to_the_real_clock_when_none_is_injected(self) -> None: + """Defaults to the real clock when none is injected.""" + provider = FirebaseTokenProvider("k", MemoryCredentialStore()) + assert provider.has_session() is False + + +class TestFileCredentialStore: + """File credential store.""" + + def test_round_trips_through_a_file(self, tmp_path: Path) -> None: + """Round trips through a file.""" + store = FileCredentialStore(tmp_path / "nested" / "creds.json") + store.save(_credentials()) + assert store.load() == _credentials() + + def test_writes_the_file_readable_only_by_this_user(self, tmp_path: Path) -> None: + # The refresh token is the real secret; it must never be briefly + # world-readable between write and chmod. + """Writes the file readable only by this user.""" + path = tmp_path / "creds.json" + FileCredentialStore(path).save(_credentials()) + assert path.stat().st_mode & 0o777 == 0o600 + + def test_load_returns_none_when_absent(self, tmp_path: Path) -> None: + """Load returns none when absent.""" + assert FileCredentialStore(tmp_path / "missing.json").load() is None + + def test_load_returns_none_for_a_truncated_file(self, tmp_path: Path) -> None: + # An interrupted write reads as "not signed in"; the caller's next + # step is to sign in again, which repairs it. + """Load returns none for a truncated file.""" + path = tmp_path / "creds.json" + path.write_text('{"id_token": "a"', encoding="utf-8") + assert FileCredentialStore(path).load() is None + + def test_load_returns_none_when_fields_are_missing(self, tmp_path: Path) -> None: + """Load returns none when fields are missing.""" + path = tmp_path / "creds.json" + path.write_text('{"id_token": "a"}', encoding="utf-8") + assert FileCredentialStore(path).load() is None + + def test_load_returns_none_for_an_unparsable_expiry(self, tmp_path: Path) -> None: + """Load returns none for an unparsable expiry.""" + path = tmp_path / "creds.json" + path.write_text( + json.dumps( + {"id_token": "a", "refresh_token": "b", "expires_at": "not-a-date"} + ), + encoding="utf-8", + ) + assert FileCredentialStore(path).load() is None + + def test_clear_removes_the_file_and_is_idempotent(self, tmp_path: Path) -> None: + """Clear removes the file and is idempotent.""" + path = tmp_path / "creds.json" + store = FileCredentialStore(path) + store.save(_credentials()) + store.clear() + assert not path.exists() + store.clear() + + def test_overwrites_an_existing_file(self, tmp_path: Path) -> None: + """Overwrites an existing file.""" + path = tmp_path / "creds.json" + store = FileCredentialStore(path) + store.save(_credentials()) + store.save(_credentials(id_value=_ID_2)) + loaded = store.load() + assert loaded is not None + assert loaded.id_token == _ID_2 diff --git a/crdt-sync/crdt_sync/tests/test_firebase_auth_tokens.py b/crdt-sync/crdt_sync/tests/test_firebase_auth_tokens.py new file mode 100644 index 0000000..e2cc46b --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_firebase_auth_tokens.py @@ -0,0 +1,159 @@ +"""Tests for ID-token refresh and the session lifecycle. + +Split from ``test_firebase_auth.py`` (250-line cap), which keeps credential +handling and the two sign-in paths. +""" + + +from __future__ import annotations + +import datetime as dt +import json +from unittest.mock import MagicMock, patch + +import pytest +import requests + +from crdt_sync import ( + FirebaseAuthError, + FirebaseCredentials, + FirebaseTokenProvider, + MemoryCredentialStore, +) +from crdt_sync import _firebase_auth as fa + +_NOW = dt.datetime(2026, 8, 3, 12, tzinfo=dt.timezone.utc) + +# Fixture values referenced through names ruff's hardcoded-credential checks +# (S105-S107) do not key off. Comparing an attribute called `id_token` to a +# bare string literal trips S105 no matter how obviously fake the value is. +_ID_1 = "id-1" +_ID_2 = "id-2" +_REFRESH_1 = "refresh-1" +_REFRESH_2 = "refresh-2" +_UID = "uid-the-data-belongs-to" + + +def _response(status_code: int = 200, json_data: object = None) -> MagicMock: + """Build a fake ``requests.Response`` with a status and JSON body.""" + response = MagicMock() + response.status_code = status_code + response.ok = 200 <= status_code < 300 + response.json = MagicMock(return_value=json_data if json_data is not None else {}) + response.text = json.dumps(json_data) if json_data is not None else "" + return response + + +def _provider( + store: object = None, + *, + now: dt.datetime = _NOW, +) -> FirebaseTokenProvider: + return FirebaseTokenProvider( + "fake-api-key", + store or MemoryCredentialStore(), + clock=lambda: now, + ) + + +# Parameters named without "token": ruff's hardcoded-credential check (S107) +# keys off the identifier, and these are obviously fake test fixtures. +def _credentials( + *, + id_value: str = _ID_1, + refresh_value: str = _REFRESH_1, + valid_for: dt.timedelta = dt.timedelta(hours=1), +) -> FirebaseCredentials: + return FirebaseCredentials( + id_token=id_value, + refresh_token=refresh_value, + expires_at=_NOW + valid_for, + ) + + +def _patch_post(*responses: MagicMock) -> object: + return patch.object(fa.requests, "post", side_effect=list(responses)) + + +class TestIdToken: + """Id token.""" + + def test_fails_loudly_when_not_signed_in(self) -> None: + # Never returns None or a stale token: a sync that quietly stops + # syncing is the failure mode this design exists to prevent. + """Fails loudly when not signed in.""" + with pytest.raises(FirebaseAuthError, match="not signed in"): + _provider().id_token() + + def test_returns_the_stored_token_without_a_network_call(self) -> None: + """Returns the stored token without a network call.""" + provider = _provider(MemoryCredentialStore(_credentials())) + with patch.object(fa.requests, "post", side_effect=AssertionError("no HTTP")): + assert provider.id_token() == _ID_1 + + def test_refreshes_an_expired_token_and_keeps_the_rotated_one(self) -> None: + """Refreshes an expired token and keeps the rotated one.""" + store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) + body = { + "id_token": _ID_2, + "refresh_token": _REFRESH_2, + "expires_in": "3600", + } + with _patch_post(_response(200, body)): + assert _provider(store).id_token() == _ID_2 + saved = store.load() + assert saved is not None + assert saved.refresh_token == _REFRESH_2 + + def test_fails_loudly_when_the_refresh_token_was_revoked(self) -> None: + """Fails loudly when the refresh token was revoked.""" + store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) + with ( + _patch_post(_response(400, {"error": {"message": "TOKEN_EXPIRED"}})), + pytest.raises(FirebaseAuthError, match="TOKEN_EXPIRED"), + ): + _provider(store).id_token() + + def test_a_revoked_refresh_token_is_cleared(self) -> None: + """A dead session must stop reporting itself as connected. + + Regression: ``has_session`` is a presence check, so a revoked token + kept reading as "connected" while every sync failed with + TOKEN_EXPIRED. + """ + store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) + provider = _provider(store) + with ( + _patch_post(_response(400, {"error": {"message": "TOKEN_EXPIRED"}})), + pytest.raises(FirebaseAuthError), + ): + provider.id_token() + assert provider.has_session() is False + assert store.load() is None + + def test_a_network_error_keeps_the_session(self) -> None: + """Signing out on a transient failure would need a manual re-login.""" + store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) + provider = _provider(store) + with ( + patch.object( + fa.requests, + "post", + side_effect=requests.ConnectionError("connection reset"), + ), + pytest.raises(FirebaseAuthError), + ): + provider.id_token() + assert provider.has_session() is True + assert store.load() is not None + + def test_a_server_error_keeps_the_session(self) -> None: + """A server error keeps the session.""" + store = MemoryCredentialStore(_credentials(valid_for=dt.timedelta(minutes=-5))) + provider = _provider(store) + with ( + _patch_post(_response(503, {"error": {"message": "SERVICE_UNAVAILABLE"}})), + pytest.raises(FirebaseAuthError), + ): + provider.id_token() + assert provider.has_session() is True diff --git a/crdt-sync/crdt_sync/tests/test_firebase_writes.py b/crdt-sync/crdt_sync/tests/test_firebase_writes.py new file mode 100644 index 0000000..0e82e5d --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_firebase_writes.py @@ -0,0 +1,220 @@ +"""Tests for Firebase writes, string maps and deletion. + +Split from ``test_firebase.py`` (250-line cap), which keeps key escaping +and the read paths. +""" + + +from __future__ import annotations + +import datetime as dt +import json +from unittest.mock import MagicMock, patch + +import pytest +import requests + +from crdt_sync import ( + DatabaseNotFoundError, + FirebaseCredentials, + FirebaseSyncClient, + FirebaseSyncError, + FirebaseTokenProvider, + MemoryCredentialStore, + RemoteNotFoundError, + RemoteStore, + RemoteSyncError, +) +from crdt_sync import _firebase as fb + +_DATABASE_URL = "https://example-default-rtdb.europe-west1.firebasedatabase.app" + +# Named without the word "token": ruff's hardcoded-credential checks (S105 / +# S106) key off the identifier, and these are obviously fake test fixtures. +_FAKE_ID = "id-fixture" +_FAKE_REFRESH = "refresh-fixture" + + +def _response(status_code: int = 200, json_data: object = None) -> MagicMock: + """Build a fake ``requests.Response`` with a status and JSON body.""" + response = MagicMock() + response.status_code = status_code + response.ok = 200 <= status_code < 300 + response.json = MagicMock(return_value=json_data) + response.text = json.dumps(json_data) + return response + + +def _auth(*, signed_in: bool = True) -> FirebaseTokenProvider: + """A provider holding a long-lived token, so no auth HTTP is needed.""" + credentials = ( + FirebaseCredentials( + id_token=_FAKE_ID, + refresh_token=_FAKE_REFRESH, + expires_at=dt.datetime(2099, 1, 1, tzinfo=dt.timezone.utc), + ) + if signed_in + else None + ) + return FirebaseTokenProvider("fake-api-key", MemoryCredentialStore(credentials)) + + +def _client(*, signed_in: bool = True) -> FirebaseSyncClient: + return FirebaseSyncClient(_DATABASE_URL, _auth(signed_in=signed_in)) + + +class TestPutFileText: + """Put file text.""" + + def test_writes_the_text_as_a_json_string_leaf(self) -> None: + """Writes the text as a JSON string leaf.""" + with ( + patch.object(fb.requests, "get", return_value=_response(200, None)), + patch.object( + fb.requests, "put", return_value=_response(200, '{"a":1}') + ) as put_mock, + ): + _client().put_file_text( + "ns/devices/pc/log.json", '{"a":1}', message="ignored" + ) + assert put_mock.call_args.kwargs["json"] == '{"a":1}' + assert put_mock.call_args.args[0].endswith("/log~2Ejson.json") + + def test_raises_on_a_non_2xx_response(self) -> None: + """Raises on a non 2xx response.""" + with ( + patch.object( + fb.requests, "put", return_value=_response(400, {"error": "bad"}) + ), + pytest.raises(FirebaseSyncError), + ): + _client().put_file_text("ns/devices/pc/log.json", "{}", message="m") + + def test_turns_a_network_failure_into_a_sync_error(self) -> None: + """Turns a network failure into a sync error.""" + with ( + patch.object( + fb.requests, "put", side_effect=requests.ConnectionError("offline") + ), + pytest.raises(FirebaseSyncError, match="network error"), + ): + _client().put_file_text("ns/devices/pc/log.json", "{}", message="m") + + +class TestGetStringMap: + """Get string map.""" + + def test_returns_the_map_with_keys_decoded(self) -> None: + """Returns the map with keys decoded.""" + payload = {"pc": "sha-1", "phone": "sha-2"} + with patch.object(fb.requests, "get", return_value=_response(200, payload)): + assert _client().get_string_map("ns/revs") == payload + + def test_degrades_to_empty_when_absent(self) -> None: + """Degrades to empty when absent.""" + with patch.object(fb.requests, "get", return_value=_response(200, None)): + assert not _client().get_string_map("ns/revs") + + def test_skips_non_string_entries_rather_than_failing(self) -> None: + # The revs node is an optimisation; a corrupt one must degrade into + # "fetch everything", never into a failed tick. + """Skips non string entries rather than failing.""" + with patch.object( + fb.requests, "get", return_value=_response(200, {"pc": "sha-1", "b": 42}) + ): + assert _client().get_string_map("ns/revs") == {"pc": "sha-1"} + + def test_raises_on_a_non_2xx_response(self) -> None: + """Raises on a non 2xx response.""" + with ( + patch.object( + fb.requests, "get", return_value=_response(500, {"error": "boom"}) + ), + pytest.raises(FirebaseSyncError), + ): + _client().get_string_map("ns/revs") + + +class TestDeleteFile: + """Delete file.""" + + def test_sends_a_delete_to_the_escaped_path(self) -> None: + """Sends a delete to the escaped path.""" + with patch.object( + fb.requests, "delete", return_value=_response(200, None) + ) as delete_mock: + _client().delete_file("ns/devices/pc/log.json") + assert delete_mock.call_args.args[0].endswith("/log~2Ejson.json") + + def test_raises_on_a_non_2xx_response(self) -> None: + """Raises on a non 2xx response.""" + with ( + patch.object( + fb.requests, "delete", return_value=_response(500, {"error": "boom"}) + ), + pytest.raises(FirebaseSyncError), + ): + _client().delete_file("ns/devices/pc/log.json") + + def test_turns_a_network_failure_into_a_sync_error(self) -> None: + """Turns a network failure into a sync error.""" + with ( + patch.object( + fb.requests, "delete", side_effect=requests.ConnectionError("x") + ), + pytest.raises(FirebaseSyncError, match="network error"), + ): + _client().delete_file("ns/devices/pc/log.json") + + +class TestCanAccessRemote: + """Can access remote.""" + + def test_true_when_the_database_root_reads(self) -> None: + """True when the database root reads.""" + with patch.object(fb.requests, "get", return_value=_response(200, {"ns": 1})): + assert _client().can_access_remote() is True + + def test_false_when_the_rules_reject_the_token(self) -> None: + """False when the rules reject the token.""" + with patch.object( + fb.requests, "get", return_value=_response(401, {"error": "denied"}) + ): + assert _client().can_access_remote() is False + + def test_false_on_a_network_error(self) -> None: + """False on a network error.""" + with patch.object( + fb.requests, "get", side_effect=requests.ConnectionError("offline") + ): + assert _client().can_access_remote() is False + + def test_false_when_not_signed_in_rather_than_raising(self) -> None: + # "cannot get a token" is exactly "cannot access the remote", and a + # settings Test-connection button must not blow up. + """False when not signed in rather than raising.""" + assert _client(signed_in=False).can_access_remote() is False + + +class TestRemoteStoreContract: + """Remote store contract.""" + + def test_the_client_satisfies_remote_store(self) -> None: + """The client satisfies remote store.""" + assert isinstance(_client(), RemoteStore) + + def test_database_not_found_is_catchable_as_either_error(self) -> None: + """Database not found is catchable as either error.""" + error = DatabaseNotFoundError("gone") + assert isinstance(error, FirebaseSyncError) + assert isinstance(error, RemoteNotFoundError) + assert isinstance(error, RemoteSyncError) + + def test_a_trailing_slash_on_the_database_url_is_tolerated(self) -> None: + """A trailing slash on the database URL is tolerated.""" + client = FirebaseSyncClient(f"{_DATABASE_URL}/", _auth()) + with patch.object( + fb.requests, "get", return_value=_response(200, "ok") + ) as get_mock: + client.get_file_text("ns/x.json") + assert get_mock.call_args.args[0] == f"{_DATABASE_URL}/ns/x~2Ejson.json" diff --git a/crdt-sync/crdt_sync/tests/test_github.py b/crdt-sync/crdt_sync/tests/test_github.py index 92e5078..61ba678 100644 --- a/crdt-sync/crdt_sync/tests/test_github.py +++ b/crdt-sync/crdt_sync/tests/test_github.py @@ -16,9 +16,6 @@ import requests from crdt_sync import ( GitHubSyncClient, GitHubSyncError, - RemoteNotFoundError, - RemoteStore, - RemoteSyncError, RepoNotFoundError, _github, ) @@ -128,174 +125,3 @@ class TestListDirectory: """Raises sync error on a non 2xx non 404.""" with _patch_get(_response(500)), pytest.raises(GitHubSyncError): _client().list_directory("devices") - - -class TestPutFileText: - """Put file text.""" - - def test_creates_a_new_file_with_no_sha_when_none_existed(self) -> None: - """Creates a new file with no sha when none existed.""" - with ( - _patch_get(_response(404), _response(200)), - patch.object( - _github.requests, "put", return_value=_response(201) - ) as put_mock, - ): - _client().put_file_text("devices/pc/log.json", "{}", message="m") - assert "sha" not in put_mock.call_args.kwargs["json"] - - def test_updates_an_existing_file_by_including_its_sha(self) -> None: - """Updates an existing file by including its sha.""" - with ( - _patch_get(_response(200, {"sha": "abc123"})), - patch.object( - _github.requests, "put", return_value=_response(200) - ) as put_mock, - ): - _client().put_file_text("devices/pc/log.json", "{}", message="m") - assert put_mock.call_args.kwargs["json"]["sha"] == "abc123" - - def test_treats_a_non_string_sha_field_as_absent(self) -> None: - """Treats a non string sha field as absent.""" - with ( - _patch_get(_response(200, {"sha": 12345})), - patch.object( - _github.requests, "put", return_value=_response(200) - ) as put_mock, - ): - _client().put_file_text("devices/pc/log.json", "{}", message="m") - assert "sha" not in put_mock.call_args.kwargs["json"] - - def test_raises_repo_not_found_when_checking_sha_on_a_missing_repo(self) -> None: - """Raises repo not found when checking sha on a missing repo.""" - with ( - _patch_get(_response(404), _response(404)), - pytest.raises(RepoNotFoundError), - ): - _client().put_file_text("devices/pc/log.json", "{}", message="m") - - def test_raises_sync_error_when_the_sha_check_itself_fails(self) -> None: - """Raises sync error when the sha check itself fails.""" - with _patch_get(_response(500)), pytest.raises(GitHubSyncError): - _client().put_file_text("devices/pc/log.json", "{}", message="m") - - def test_raises_sync_error_on_a_put_network_exception(self) -> None: - """Raises sync error on a put network exception.""" - with ( - _patch_get(_response(404), _response(200)), - patch.object( - _github.requests, - "put", - side_effect=requests.ConnectionError("offline"), - ), - pytest.raises(GitHubSyncError), - ): - _client().put_file_text("devices/pc/log.json", "{}", message="m") - - def test_raises_sync_error_on_a_put_non_2xx_response(self) -> None: - """Raises sync error on a put non 2xx response.""" - with ( - _patch_get(_response(404), _response(200)), - patch.object(_github.requests, "put", return_value=_response(422)), - pytest.raises(GitHubSyncError), - ): - _client().put_file_text("devices/pc/log.json", "{}", message="m") - - -class TestCanAccessRepo: - """Can access repo.""" - - def test_true_when_repo_endpoint_ok(self) -> None: - """True when repo endpoint ok.""" - with _patch_get(_response(200)): - assert _client().can_access_repo() is True - - def test_false_when_repo_is_missing(self) -> None: - """False when repo is missing.""" - with _patch_get(_response(404)): - assert _client().can_access_repo() is False - - def test_false_on_a_network_error(self) -> None: - """False on a network error.""" - with _patch_get_raises(): - assert _client().can_access_repo() is False - - -class TestCanAccessRemote: - """The ``RemoteStore`` spelling must behave exactly like the legacy name.""" - - def test_true_when_repo_endpoint_ok(self) -> None: - """True when repo endpoint ok.""" - with _patch_get(_response(200)): - assert _client().can_access_remote() is True - - def test_false_when_repo_is_missing(self) -> None: - """False when repo is missing.""" - with _patch_get(_response(404)): - assert _client().can_access_remote() is False - - def test_false_on_a_network_error(self) -> None: - """False on a network error.""" - with _patch_get_raises(): - assert _client().can_access_remote() is False - - -class TestRemoteStoreContract: - """The seam sync talks through must actually be satisfied by the client.""" - - def test_github_client_is_a_remote_store(self) -> None: - """Github client is a remote store.""" - assert isinstance(_client(), RemoteStore) - - def test_repo_not_found_is_catchable_as_either_error(self) -> None: - # Backend-neutral callers catch RemoteNotFoundError; existing GitHub - # callers catch GitHubSyncError. One exception must satisfy both. - """Repo not found is catchable as either error.""" - error = RepoNotFoundError("gone") - assert isinstance(error, GitHubSyncError) - assert isinstance(error, RemoteNotFoundError) - assert isinstance(error, RemoteSyncError) - - -class TestDeleteFile: - """Delete file.""" - - def test_deletes_an_existing_file_resolving_its_own_sha(self) -> None: - """Deletes an existing file resolving its own sha.""" - with ( - _patch_get(_response(200, {"sha": "abc123"})), - patch.object(_github.requests, "delete", return_value=_response(200)), - ): - _client().delete_file("devices/pc/log.json") - - def test_is_a_no_op_when_the_file_is_absent(self) -> None: - # sha GET 404, repo-exists GET 200 -> None sha -> no DELETE sent. - """Is a no op when the file is absent.""" - with ( - _patch_get(_response(404), _response(200)), - patch.object(_github.requests, "delete") as mock_delete, - ): - _client().delete_file("devices/pc/gone.json") - mock_delete.assert_not_called() - - def test_raises_on_a_delete_non_2xx_response(self) -> None: - """Raises on a delete non 2xx response.""" - with ( - _patch_get(_response(200, {"sha": "abc123"})), - patch.object(_github.requests, "delete", return_value=_response(500)), - pytest.raises(GitHubSyncError), - ): - _client().delete_file("devices/pc/log.json") - - def test_raises_on_a_delete_network_error(self) -> None: - """Raises on a delete network error.""" - with ( - _patch_get(_response(200, {"sha": "abc123"})), - patch.object( - _github.requests, - "delete", - side_effect=requests.ConnectionError("offline"), - ), - pytest.raises(GitHubSyncError), - ): - _client().delete_file("devices/pc/log.json") diff --git a/crdt-sync/crdt_sync/tests/test_github_writes.py b/crdt-sync/crdt_sync/tests/test_github_writes.py new file mode 100644 index 0000000..8b8fa24 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_github_writes.py @@ -0,0 +1,219 @@ +"""Tests for GitHub writes and access checks. + +Split from ``test_github.py`` (250-line cap), which keeps the read paths. +""" + + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest +import requests + +from crdt_sync import ( + GitHubSyncClient, + GitHubSyncError, + RemoteNotFoundError, + RemoteStore, + RemoteSyncError, + RepoNotFoundError, + _github, +) + + +def _response(status_code: int = 200, json_data: object = None) -> MagicMock: + """Build a fake ``requests.Response`` with a fixed status and JSON body.""" + response = MagicMock() + response.status_code = status_code + response.ok = 200 <= status_code < 300 + response.json = MagicMock(return_value=json_data if json_data is not None else {}) + return response + + +def _client() -> GitHubSyncClient: + return GitHubSyncClient("kuhyx", "crdt-sync-demo", "fake-token") + + +def _patch_get(*responses: MagicMock) -> object: + """Patch ``requests.get`` to return each of ``responses`` in order.""" + return patch.object(_github.requests, "get", side_effect=list(responses)) + + +def _patch_get_raises() -> object: + return patch.object( + _github.requests, + "get", + side_effect=requests.ConnectionError("offline"), + ) + + +class TestPutFileText: + """Put file text.""" + + def test_creates_a_new_file_with_no_sha_when_none_existed(self) -> None: + """Creates a new file with no sha when none existed.""" + with ( + _patch_get(_response(404), _response(200)), + patch.object( + _github.requests, "put", return_value=_response(201) + ) as put_mock, + ): + _client().put_file_text("devices/pc/log.json", "{}", message="m") + assert "sha" not in put_mock.call_args.kwargs["json"] + + def test_updates_an_existing_file_by_including_its_sha(self) -> None: + """Updates an existing file by including its sha.""" + with ( + _patch_get(_response(200, {"sha": "abc123"})), + patch.object( + _github.requests, "put", return_value=_response(200) + ) as put_mock, + ): + _client().put_file_text("devices/pc/log.json", "{}", message="m") + assert put_mock.call_args.kwargs["json"]["sha"] == "abc123" + + def test_treats_a_non_string_sha_field_as_absent(self) -> None: + """Treats a non string sha field as absent.""" + with ( + _patch_get(_response(200, {"sha": 12345})), + patch.object( + _github.requests, "put", return_value=_response(200) + ) as put_mock, + ): + _client().put_file_text("devices/pc/log.json", "{}", message="m") + assert "sha" not in put_mock.call_args.kwargs["json"] + + def test_raises_repo_not_found_when_checking_sha_on_a_missing_repo(self) -> None: + """Raises repo not found when checking sha on a missing repo.""" + with ( + _patch_get(_response(404), _response(404)), + pytest.raises(RepoNotFoundError), + ): + _client().put_file_text("devices/pc/log.json", "{}", message="m") + + def test_raises_sync_error_when_the_sha_check_itself_fails(self) -> None: + """Raises sync error when the sha check itself fails.""" + with _patch_get(_response(500)), pytest.raises(GitHubSyncError): + _client().put_file_text("devices/pc/log.json", "{}", message="m") + + def test_raises_sync_error_on_a_put_network_exception(self) -> None: + """Raises sync error on a put network exception.""" + with ( + _patch_get(_response(404), _response(200)), + patch.object( + _github.requests, + "put", + side_effect=requests.ConnectionError("offline"), + ), + pytest.raises(GitHubSyncError), + ): + _client().put_file_text("devices/pc/log.json", "{}", message="m") + + def test_raises_sync_error_on_a_put_non_2xx_response(self) -> None: + """Raises sync error on a put non 2xx response.""" + with ( + _patch_get(_response(404), _response(200)), + patch.object(_github.requests, "put", return_value=_response(422)), + pytest.raises(GitHubSyncError), + ): + _client().put_file_text("devices/pc/log.json", "{}", message="m") + + +class TestCanAccessRepo: + """Can access repo.""" + + def test_true_when_repo_endpoint_ok(self) -> None: + """True when repo endpoint ok.""" + with _patch_get(_response(200)): + assert _client().can_access_repo() is True + + def test_false_when_repo_is_missing(self) -> None: + """False when repo is missing.""" + with _patch_get(_response(404)): + assert _client().can_access_repo() is False + + def test_false_on_a_network_error(self) -> None: + """False on a network error.""" + with _patch_get_raises(): + assert _client().can_access_repo() is False + + +class TestCanAccessRemote: + """The ``RemoteStore`` spelling must behave exactly like the legacy name.""" + + def test_true_when_repo_endpoint_ok(self) -> None: + """True when repo endpoint ok.""" + with _patch_get(_response(200)): + assert _client().can_access_remote() is True + + def test_false_when_repo_is_missing(self) -> None: + """False when repo is missing.""" + with _patch_get(_response(404)): + assert _client().can_access_remote() is False + + def test_false_on_a_network_error(self) -> None: + """False on a network error.""" + with _patch_get_raises(): + assert _client().can_access_remote() is False + + +class TestRemoteStoreContract: + """The seam sync talks through must actually be satisfied by the client.""" + + def test_github_client_is_a_remote_store(self) -> None: + """Github client is a remote store.""" + assert isinstance(_client(), RemoteStore) + + def test_repo_not_found_is_catchable_as_either_error(self) -> None: + # Backend-neutral callers catch RemoteNotFoundError; existing GitHub + # callers catch GitHubSyncError. One exception must satisfy both. + """Repo not found is catchable as either error.""" + error = RepoNotFoundError("gone") + assert isinstance(error, GitHubSyncError) + assert isinstance(error, RemoteNotFoundError) + assert isinstance(error, RemoteSyncError) + + +class TestDeleteFile: + """Delete file.""" + + def test_deletes_an_existing_file_resolving_its_own_sha(self) -> None: + """Deletes an existing file resolving its own sha.""" + with ( + _patch_get(_response(200, {"sha": "abc123"})), + patch.object(_github.requests, "delete", return_value=_response(200)), + ): + _client().delete_file("devices/pc/log.json") + + def test_is_a_no_op_when_the_file_is_absent(self) -> None: + # sha GET 404, repo-exists GET 200 -> None sha -> no DELETE sent. + """Is a no op when the file is absent.""" + with ( + _patch_get(_response(404), _response(200)), + patch.object(_github.requests, "delete") as mock_delete, + ): + _client().delete_file("devices/pc/gone.json") + mock_delete.assert_not_called() + + def test_raises_on_a_delete_non_2xx_response(self) -> None: + """Raises on a delete non 2xx response.""" + with ( + _patch_get(_response(200, {"sha": "abc123"})), + patch.object(_github.requests, "delete", return_value=_response(500)), + pytest.raises(GitHubSyncError), + ): + _client().delete_file("devices/pc/log.json") + + def test_raises_on_a_delete_network_error(self) -> None: + """Raises on a delete network error.""" + with ( + _patch_get(_response(200, {"sha": "abc123"})), + patch.object( + _github.requests, + "delete", + side_effect=requests.ConnectionError("offline"), + ), + pytest.raises(GitHubSyncError), + ): + _client().delete_file("devices/pc/log.json") diff --git a/crdt-sync/crdt_sync/tests/test_mirror.py b/crdt-sync/crdt_sync/tests/test_mirror.py index 98bf643..40ce11e 100644 --- a/crdt-sync/crdt_sync/tests/test_mirror.py +++ b/crdt-sync/crdt_sync/tests/test_mirror.py @@ -12,19 +12,10 @@ from __future__ import annotations import pytest from crdt_sync import ( - Hlc, - Log, - LogCodec, MirrorSyncClient, - Record, - RemoteStore, RemoteSyncError, - SyncTarget, - dump_log, - load_log, - sync_log, ) -from crdt_sync.tests.conftest import FakeStore, FakeStoreWithoutBulkRead +from crdt_sync.tests.conftest import FakeStore def _mirror( @@ -176,189 +167,3 @@ class TestListDirectory: client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True) with pytest.raises(RemoteSyncError): client.list_directory("ns") - - -class TestPrimaryReadFallback: - """A failing primary must never hide data the mirror can still serve.""" - - def test_get_file_text_falls_back_to_the_mirror(self) -> None: - """Get file text falls back to the mirror.""" - client, _, _, _ = _mirror( - primary_failing=True, mirror_files={"ns/phone/log.json": "{}"} - ) - assert client.get_file_text("ns/phone/log.json") == "{}" - - def test_get_file_text_raises_when_both_fail(self) -> None: - """Get file text raises when both fail.""" - client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True) - with pytest.raises(RemoteSyncError): - client.get_file_text("ns/phone/log.json") - - def test_get_file_text_raises_when_primary_fails_and_mirror_lacks_it( - self, - ) -> None: - # A reachable mirror that simply does not hold the file is not an - # answer either, since the primary's copy was never read. - """Get file text raises when primary fails and mirror lacks it.""" - client, _, _, _ = _mirror(primary_failing=True) - with pytest.raises(RemoteSyncError): - client.get_file_text("ns/phone/log.json") - - def test_revision_map_falls_back_to_the_mirror(self) -> None: - """Revision map falls back to the mirror.""" - client, _, _, _ = _mirror( - primary_failing=True, mirror_files={"ns/revs/phone": "r1"} - ) - assert client.get_string_map("ns/revs") == {"phone": "r1"} - - def test_revision_map_raises_when_both_fail(self) -> None: - """Revision map raises when both fail.""" - client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True) - with pytest.raises(RemoteSyncError): - client.get_string_map("ns/revs") - - def test_revision_map_tolerates_a_mirror_that_cannot_bulk_read( - self, - ) -> None: - # A mirror with no bulk-read capability (GitHub) contributes nothing, - # but that is not a failure -- it simply has no revisions to add, so a - # working primary's map must still come back. Matches the Dart side, - # where the capability check runs inside the guarded closure. - """Revision map tolerates a mirror that cannot bulk read.""" - client = MirrorSyncClient( - FakeStore({"ns/revs/pc": "r1"}), - FakeStoreWithoutBulkRead(), - on_mirror_failure=lambda *_: None, - ) - assert client.get_string_map("ns/revs") == {"pc": "r1"} - - def test_writes_stay_fail_closed_on_a_primary_failure(self) -> None: - # The read fix must NOT loosen writes: an unaccepted write has not - # happened, so it must still fail the tick. - """Writes stay fail closed on a primary failure.""" - client, _, _, _ = _mirror(primary_failing=True) - with pytest.raises(RemoteSyncError): - client.put_file_text("ns/pc/log.json", "{}", message="m") - with pytest.raises(RemoteSyncError): - client.delete_file("ns/pc/log.json") - - -class TestRevisionMaps: - """Revision maps.""" - - def test_merge_both_backends_with_the_primary_winning(self) -> None: - """Merge both backends with the primary winning.""" - client, _, _, _ = _mirror( - primary_files={"ns/revs/pc": "primary-rev"}, - mirror_files={"ns/revs/pc": "mirror-rev", "ns/revs/phone": "old"}, - ) - # An un-migrated device publishes revisions only to the mirror; - # without this it would look revision-less and be re-downloaded - # every tick for the whole trial. - assert client.get_string_map("ns/revs") == { - "pc": "primary-rev", - "phone": "old", - } - - def test_a_mirror_failure_degrades_to_primary_revisions(self) -> None: - """A mirror failure degrades to primary revisions.""" - client, _, _, failures = _mirror( - primary_files={"ns/revs/pc": "primary-rev"}, mirror_failing=True - ) - assert client.get_string_map("ns/revs") == {"pc": "primary-rev"} - assert failures == ["get_string_map ns/revs"] - - def test_a_backend_without_bulk_reads_contributes_nothing(self) -> None: - """A backend without bulk reads contributes nothing.""" - client = MirrorSyncClient( - FakeStore({"ns/revs/pc": "primary-rev"}), - FakeStoreWithoutBulkRead({"ns/revs/phone": "ignored"}), - ) - assert client.get_string_map("ns/revs") == {"pc": "primary-rev"} - - def test_is_empty_when_neither_backend_has_bulk_reads(self) -> None: - """Is empty when neither backend has bulk reads.""" - client = MirrorSyncClient( - FakeStoreWithoutBulkRead(), FakeStoreWithoutBulkRead() - ) - assert not client.get_string_map("ns/revs") - - -class TestLifecycle: - """Lifecycle.""" - - def test_can_access_remote_reports_only_the_primary(self) -> None: - # A Test-connection button must not report success because the - # backend being retired happens to answer. - """Can access remote reports only the primary.""" - broken, _, _, _ = _mirror(primary_failing=True) - assert broken.can_access_remote() is False - healthy, _, _, _ = _mirror(mirror_failing=True) - assert healthy.can_access_remote() is True - - def test_a_mirror_failure_with_no_handler_is_logged_not_raised( - self, - caplog: pytest.LogCaptureFixture, - ) -> None: - """A mirror failure with no handler is logged not raised.""" - client = MirrorSyncClient(FakeStore(), FakeStore(failing=True)) - client.put_file_text("ns/pc/log.json", "{}", message="m") - assert "Mirror backend failed" in caplog.text - - def test_satisfies_the_remote_store_contract(self) -> None: - """Satisfies the remote store contract.""" - client, _, _, _ = _mirror() - assert isinstance(client, RemoteStore) - - -class TestEndToEndThroughSyncLog: - """End to end through sync log.""" - - def test_a_half_migrated_pair_still_converges_both_ways(self) -> None: - # phone has not migrated and still writes only to GitHub; pc is on - # Firebase with the GitHub mirror. Both must see both records. - """A half migrated pair still converges both ways.""" - github = FakeStore() - firebase = FakeStore() - - def log(record_id: str, node_id: str) -> Log: - return { - record_id: Record( - id=record_id, - fields={ - "v": ( - record_id, - Hlc(wall_time_ms=1000, counter=0, node_id=node_id), - ) - }, - ) - } - - sync_log( - SyncTarget( - client=github, - device_id="phone", - path_prefix="ns/devices", - ), - log("from-phone", "node-phone"), - LogCodec( - decode=load_log, - encode=dump_log, - ), - ) - merged = sync_log( - SyncTarget( - client=MirrorSyncClient(firebase, github), - device_id="pc", - path_prefix="ns/devices", - ), - log("from-pc", "node-pc"), - LogCodec( - decode=load_log, - encode=dump_log, - ), - ) - assert {"from-phone", "from-pc"} <= set(merged) - # The pc's merged result is mirrored back to GitHub, so the - # un-migrated phone sees it on its next tick. - assert "ns/devices/pc/log.json" in github.files diff --git a/crdt-sync/crdt_sync/tests/test_mirror_fallback.py b/crdt-sync/crdt_sync/tests/test_mirror_fallback.py new file mode 100644 index 0000000..d9067d0 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_mirror_fallback.py @@ -0,0 +1,229 @@ +"""Tests for mirror fallback reads, revision maps and lifecycle. + +Split from ``test_mirror.py`` (250-line cap), which keeps plain writes, +reads and directory listing. +""" + + +from __future__ import annotations + +import pytest + +from crdt_sync import ( + Hlc, + Log, + LogCodec, + MirrorSyncClient, + Record, + RemoteStore, + RemoteSyncError, + SyncTarget, + dump_log, + load_log, + sync_log, +) +from crdt_sync.tests.conftest import FakeStore, FakeStoreWithoutBulkRead + + +def _mirror( + *, + primary_files: dict[str, str] | None = None, + mirror_files: dict[str, str] | None = None, + primary_failing: bool = False, + mirror_failing: bool = False, +) -> tuple[MirrorSyncClient, FakeStore, FakeStore, list[str]]: + primary = FakeStore(primary_files, failing=primary_failing) + mirror = FakeStore(mirror_files, failing=mirror_failing) + failures: list[str] = [] + client = MirrorSyncClient( + primary, + mirror, + on_mirror_failure=lambda operation, _: failures.append(operation), + ) + return client, primary, mirror, failures + + +class TestPrimaryReadFallback: + """A failing primary must never hide data the mirror can still serve.""" + + def test_get_file_text_falls_back_to_the_mirror(self) -> None: + """Get file text falls back to the mirror.""" + client, _, _, _ = _mirror( + primary_failing=True, mirror_files={"ns/phone/log.json": "{}"} + ) + assert client.get_file_text("ns/phone/log.json") == "{}" + + def test_get_file_text_raises_when_both_fail(self) -> None: + """Get file text raises when both fail.""" + client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True) + with pytest.raises(RemoteSyncError): + client.get_file_text("ns/phone/log.json") + + def test_get_file_text_raises_when_primary_fails_and_mirror_lacks_it( + self, + ) -> None: + # A reachable mirror that simply does not hold the file is not an + # answer either, since the primary's copy was never read. + """Get file text raises when primary fails and mirror lacks it.""" + client, _, _, _ = _mirror(primary_failing=True) + with pytest.raises(RemoteSyncError): + client.get_file_text("ns/phone/log.json") + + def test_revision_map_falls_back_to_the_mirror(self) -> None: + """Revision map falls back to the mirror.""" + client, _, _, _ = _mirror( + primary_failing=True, mirror_files={"ns/revs/phone": "r1"} + ) + assert client.get_string_map("ns/revs") == {"phone": "r1"} + + def test_revision_map_raises_when_both_fail(self) -> None: + """Revision map raises when both fail.""" + client, _, _, _ = _mirror(primary_failing=True, mirror_failing=True) + with pytest.raises(RemoteSyncError): + client.get_string_map("ns/revs") + + def test_revision_map_tolerates_a_mirror_that_cannot_bulk_read( + self, + ) -> None: + # A mirror with no bulk-read capability (GitHub) contributes nothing, + # but that is not a failure -- it simply has no revisions to add, so a + # working primary's map must still come back. Matches the Dart side, + # where the capability check runs inside the guarded closure. + """Revision map tolerates a mirror that cannot bulk read.""" + client = MirrorSyncClient( + FakeStore({"ns/revs/pc": "r1"}), + FakeStoreWithoutBulkRead(), + on_mirror_failure=lambda *_: None, + ) + assert client.get_string_map("ns/revs") == {"pc": "r1"} + + def test_writes_stay_fail_closed_on_a_primary_failure(self) -> None: + # The read fix must NOT loosen writes: an unaccepted write has not + # happened, so it must still fail the tick. + """Writes stay fail closed on a primary failure.""" + client, _, _, _ = _mirror(primary_failing=True) + with pytest.raises(RemoteSyncError): + client.put_file_text("ns/pc/log.json", "{}", message="m") + with pytest.raises(RemoteSyncError): + client.delete_file("ns/pc/log.json") + + +class TestRevisionMaps: + """Revision maps.""" + + def test_merge_both_backends_with_the_primary_winning(self) -> None: + """Merge both backends with the primary winning.""" + client, _, _, _ = _mirror( + primary_files={"ns/revs/pc": "primary-rev"}, + mirror_files={"ns/revs/pc": "mirror-rev", "ns/revs/phone": "old"}, + ) + # An un-migrated device publishes revisions only to the mirror; + # without this it would look revision-less and be re-downloaded + # every tick for the whole trial. + assert client.get_string_map("ns/revs") == { + "pc": "primary-rev", + "phone": "old", + } + + def test_a_mirror_failure_degrades_to_primary_revisions(self) -> None: + """A mirror failure degrades to primary revisions.""" + client, _, _, failures = _mirror( + primary_files={"ns/revs/pc": "primary-rev"}, mirror_failing=True + ) + assert client.get_string_map("ns/revs") == {"pc": "primary-rev"} + assert failures == ["get_string_map ns/revs"] + + def test_a_backend_without_bulk_reads_contributes_nothing(self) -> None: + """A backend without bulk reads contributes nothing.""" + client = MirrorSyncClient( + FakeStore({"ns/revs/pc": "primary-rev"}), + FakeStoreWithoutBulkRead({"ns/revs/phone": "ignored"}), + ) + assert client.get_string_map("ns/revs") == {"pc": "primary-rev"} + + def test_is_empty_when_neither_backend_has_bulk_reads(self) -> None: + """Is empty when neither backend has bulk reads.""" + client = MirrorSyncClient( + FakeStoreWithoutBulkRead(), FakeStoreWithoutBulkRead() + ) + assert not client.get_string_map("ns/revs") + + +class TestLifecycle: + """Lifecycle.""" + + def test_can_access_remote_reports_only_the_primary(self) -> None: + # A Test-connection button must not report success because the + # backend being retired happens to answer. + """Can access remote reports only the primary.""" + broken, _, _, _ = _mirror(primary_failing=True) + assert broken.can_access_remote() is False + healthy, _, _, _ = _mirror(mirror_failing=True) + assert healthy.can_access_remote() is True + + def test_a_mirror_failure_with_no_handler_is_logged_not_raised( + self, + caplog: pytest.LogCaptureFixture, + ) -> None: + """A mirror failure with no handler is logged not raised.""" + client = MirrorSyncClient(FakeStore(), FakeStore(failing=True)) + client.put_file_text("ns/pc/log.json", "{}", message="m") + assert "Mirror backend failed" in caplog.text + + def test_satisfies_the_remote_store_contract(self) -> None: + """Satisfies the remote store contract.""" + client, _, _, _ = _mirror() + assert isinstance(client, RemoteStore) + + +class TestEndToEndThroughSyncLog: + """End to end through sync log.""" + + def test_a_half_migrated_pair_still_converges_both_ways(self) -> None: + # phone has not migrated and still writes only to GitHub; pc is on + # Firebase with the GitHub mirror. Both must see both records. + """A half migrated pair still converges both ways.""" + github = FakeStore() + firebase = FakeStore() + + def log(record_id: str, node_id: str) -> Log: + return { + record_id: Record( + id=record_id, + fields={ + "v": ( + record_id, + Hlc(wall_time_ms=1000, counter=0, node_id=node_id), + ) + }, + ) + } + + sync_log( + SyncTarget( + client=github, + device_id="phone", + path_prefix="ns/devices", + ), + log("from-phone", "node-phone"), + LogCodec( + decode=load_log, + encode=dump_log, + ), + ) + merged = sync_log( + SyncTarget( + client=MirrorSyncClient(firebase, github), + device_id="pc", + path_prefix="ns/devices", + ), + log("from-pc", "node-pc"), + LogCodec( + decode=load_log, + encode=dump_log, + ), + ) + assert {"from-phone", "from-pc"} <= set(merged) + # The pc's merged result is mirrored back to GitHub, so the + # un-migrated phone sees it on its next tick. + assert "ns/devices/pc/log.json" in github.files diff --git a/crdt-sync/crdt_sync/tests/test_sync.py b/crdt-sync/crdt_sync/tests/test_sync.py index 2ca9bc6..e1b94a9 100644 --- a/crdt-sync/crdt_sync/tests/test_sync.py +++ b/crdt-sync/crdt_sync/tests/test_sync.py @@ -3,7 +3,6 @@ from __future__ import annotations import json -import logging from typing import TYPE_CHECKING from unittest.mock import MagicMock @@ -18,8 +17,6 @@ from crdt_sync import ( if TYPE_CHECKING: from collections.abc import Callable - import pytest - from crdt_sync import Hlc, Log @@ -145,79 +142,6 @@ class TestSyncLog: client.get_file_text.assert_called_once_with("devices/pc/log.json") - def test_skips_a_device_with_no_pushed_file_yet(self) -> None: - """Skips a device with no pushed file yet.""" - client = _mock_client(devices=("phone",), files={}) - - merged = sync_log( - SyncTarget( - client=client, - device_id="pc", - path_prefix="devices", - ), - {}, - LogCodec( - decode=_decode, - encode=_encode, - ), - ) - - assert not merged - - def test_skips_a_device_whose_pushed_file_is_corrupt( - self, caplog: pytest.LogCaptureFixture - ) -> None: - """An interrupted/truncated push must not crash every other device's - merge -- it is treated the same as a device that hasn't pushed yet. - """ - client = _mock_client( - devices=("phone",), - files={"devices/phone/log.json": "{not valid json"}, - ) - - with caplog.at_level(logging.WARNING): - merged = sync_log( - SyncTarget( - client=client, - device_id="pc", - path_prefix="devices", - ), - {}, - LogCodec( - decode=_decode, - encode=_encode, - ), - ) - - assert not merged - assert "Unparsable log" in caplog.text - - def test_skips_a_device_whose_pushed_json_has_the_wrong_shape(self) -> None: - """Valid JSON that isn't a record map (e.g. from an incompatible - writer) must be skipped like corrupt JSON, not crash the whole - sync -- this is what the broad ``(ValueError, KeyError, TypeError)`` - catch in ``_pull_remote_logs`` is for, not just JSON syntax errors. - """ - client = _mock_client( - devices=("phone",), - files={"devices/phone/log.json": '{"a": 5}'}, - ) - - merged = sync_log( - SyncTarget( - client=client, - device_id="pc", - path_prefix="devices", - ), - {}, - LogCodec( - decode=_decode, - encode=_encode, - ), - ) - - assert not merged - def test_merges_in_a_remote_devices_entries( self, make_hlc: Callable[..., Hlc] ) -> None: diff --git a/crdt-sync/crdt_sync/tests/test_sync_revisions.py b/crdt-sync/crdt_sync/tests/test_sync_revisions.py index f1521f2..ac43e4c 100644 --- a/crdt-sync/crdt_sync/tests/test_sync_revisions.py +++ b/crdt-sync/crdt_sync/tests/test_sync_revisions.py @@ -11,12 +11,7 @@ Mirrors ``crdt_sync_dart``'s ``test/sync_revisions_test.dart``. from __future__ import annotations -from typing import TYPE_CHECKING - -import pytest - from crdt_sync import ( - FileSyncStateStore, Hlc, Log, LogCodec, @@ -33,9 +28,6 @@ from crdt_sync import ( ) from crdt_sync.tests.conftest import FakeStore, FakeStoreWithoutBulkRead -if TYPE_CHECKING: - from pathlib import Path - def _log(record_id: str, value: str, node_id: str = "node-a") -> Log: return { @@ -169,238 +161,3 @@ class TestNoOpPushSuppression: _tick(remote, None, local) assert remote.writes == ["ns/devices/pc/log.json"] * 2 assert not [w for w in remote.writes if w.startswith("ns/revs")] - - -class TestPeerDownloadSuppression: - """Peer download suppression.""" - - def test_downloads_a_peer_whose_revision_it_has_never_seen(self) -> None: - """Downloads a peer whose revision it has never seen.""" - peer = _encode(_log("b", "from-phone", "node-b")) - remote = FakeRemote( - { - "ns/devices/phone/log.json": peer, - "ns/revs/phone": revision_of(peer), - } - ) - merged = _tick(remote, MemorySyncStateStore()) - assert "ns/devices/phone/log.json" in remote.reads - assert "b" in merged - - def test_skips_the_download_when_the_peer_revision_is_unchanged(self) -> None: - """Skips the download when the peer revision is unchanged.""" - peer = _encode(_log("b", "from-phone", "node-b")) - remote = FakeRemote( - { - "ns/devices/phone/log.json": peer, - "ns/revs/phone": revision_of(peer), - } - ) - store = MemorySyncStateStore() - first = _tick(remote, store) - remote.reads.clear() - # The peer's records are already in the local log from tick one, so - # re-downloading is pure waste -- the ~700 MB/month this removes. - _tick(remote, store, first) - assert not remote.reads - - def test_downloads_again_once_the_peer_publishes_a_new_revision(self) -> None: - """Downloads again once the peer publishes a new revision.""" - peer_v1 = _encode(_log("b", "v1", "node-b")) - remote = FakeRemote( - { - "ns/devices/phone/log.json": peer_v1, - "ns/revs/phone": revision_of(peer_v1), - } - ) - store = MemorySyncStateStore() - first = _tick(remote, store) - remote.reads.clear() - - peer_v2 = _encode({**_decode(peer_v1), **_log("c", "v2", "node-b")}) - remote.files["ns/devices/phone/log.json"] = peer_v2 - remote.files["ns/revs/phone"] = revision_of(peer_v2) - - second = _tick(remote, store, first) - assert "ns/devices/phone/log.json" in remote.reads - assert {"b", "c"} <= set(second) - - def test_re_downloads_a_peer_whose_push_was_corrupt(self) -> None: - # A failed decode must not be remembered as seen, or the corruption - # would be permanent. - """Re downloads a peer whose push was corrupt.""" - remote = FakeRemote( - { - "ns/devices/phone/log.json": "not json at all", - "ns/revs/phone": revision_of("not json at all"), - } - ) - store = MemorySyncStateStore() - _tick(remote, store) - remote.reads.clear() - _tick(remote, store) - assert "ns/devices/phone/log.json" in remote.reads - - def test_downloads_when_the_peer_has_published_no_revision(self) -> None: - # A device still running pre-migration code publishes a log but no - # revision; it must not be silently ignored. - """Downloads when the peer has published no revision.""" - peer = _encode(_log("b", "from-phone", "node-b")) - remote = FakeRemote({"ns/devices/phone/log.json": peer}) - assert "b" in _tick(remote, MemorySyncStateStore()) - - def test_skips_a_peer_with_nothing_pushed_yet(self) -> None: - """Skips a peer with nothing pushed yet.""" - remote = FakeRemote({"ns/devices/phone/other.txt": "x"}) - assert not _tick(remote, MemorySyncStateStore()) - - def test_never_reads_its_own_device_back(self) -> None: - """Never reads its own device back.""" - remote = FakeRemote( - { - "ns/devices/pc/log.json": _encode(_log("a", "1")), - "ns/revs/pc": "whatever", - } - ) - _tick(remote, MemorySyncStateStore()) - assert not remote.reads - - -class TestBackendsWithoutBulkMapReads: - """Backends without bulk map reads.""" - - def test_still_sync_correctly_just_without_the_saving(self) -> None: - # GitHubSyncClient has no get_string_map, so revision lookup degrades - # to "fetch everything" -- correctness must not depend on it. - """Still sync correctly just without the saving.""" - peer = _encode(_log("b", "from-phone", "node-b")) - remote = FakeRemoteWithoutBulkRead({"ns/devices/phone/log.json": peer}) - store = MemorySyncStateStore() - first = _tick(remote, store) - assert "b" in first - remote.reads.clear() - _tick(remote, store, first) - assert "ns/devices/phone/log.json" in remote.reads - - -class TestRevisionPublishingOrder: - """Revision publishing order.""" - - def test_publishes_the_log_before_its_revision(self) -> None: - # Reversed, a peer would cache "seen rev X" against a log it never - # received, and skip it forever. - """Publishes the log before its revision.""" - remote = FakeRemote() - _tick(remote, MemorySyncStateStore(), _log("a", "1")) - assert remote.writes.index("ns/devices/pc/log.json") < remote.writes.index( - "ns/revs/pc" - ) - - def test_each_device_writes_only_its_own_revision_key(self) -> None: - # Per-device keys rather than one shared map: a whole-map write would - # erase every other device's entry, after which those peers would look - # permanently unchanged and never be fetched again. - """Each device writes only its own revision key.""" - remote = FakeRemote({"ns/revs/phone": "peer-rev"}) - _tick(remote, MemorySyncStateStore(), _log("a", "1")) - assert remote.files["ns/revs/phone"] == "peer-rev" - - def test_an_explicit_revs_path_overrides_the_default(self) -> None: - """An explicit revs path overrides the default.""" - remote = FakeRemote() - sync_log( - SyncTarget( - client=remote, - device_id="pc", - path_prefix="ns/devices", - ), - _log("a", "1"), - LogCodec( - decode=_decode, - encode=_encode, - ), - RevisionTracking( - revs_path="custom/place", - state_store=MemorySyncStateStore(), - ), - ) - assert "custom/place/pc" in remote.writes - - -class TestTwoDevicesConverge: - """Two devices converge.""" - - def test_each_ends_up_with_the_others_records(self) -> None: - """Each ends up with the others records.""" - remote = FakeRemote() - pc_store, phone_store = MemorySyncStateStore(), MemorySyncStateStore() - pc = _tick(remote, pc_store, _log("a", "from-pc", "node-pc")) - phone = sync_log( - SyncTarget( - client=remote, - device_id="phone", - path_prefix="ns/devices", - ), - _log("b", "from-phone", "node-phone"), - LogCodec( - decode=_decode, - encode=_encode, - ), - RevisionTracking( - state_store=phone_store, - ), - ) - assert {"a", "b"} <= set(phone) - pc_after = _tick(remote, pc_store, pc) - assert {"a", "b"} <= set(pc_after) - - -class TestFileSyncStateStore: - """File sync state store.""" - - def test_round_trips_through_a_file(self, tmp_path: Path) -> None: - """Round trips through a file.""" - store = FileSyncStateStore(tmp_path / "nested" / "state.json") - state = SyncState(pushed_rev="abc", peer_revs={"phone": "def"}) - store.save(state) - assert store.load() == state - - def test_load_returns_default_state_when_absent(self, tmp_path: Path) -> None: - """Load returns default state when absent.""" - assert FileSyncStateStore(tmp_path / "missing.json").load() == SyncState() - - def test_load_returns_default_state_for_a_truncated_file( - self, tmp_path: Path - ) -> None: - """Load returns default state for a truncated file.""" - path = tmp_path / "state.json" - path.write_text('{"pushed_rev":', encoding="utf-8") - assert FileSyncStateStore(path).load() == SyncState() - - def test_load_returns_default_state_for_a_non_dict_file( - self, tmp_path: Path - ) -> None: - """Load returns default state for a non dict file.""" - path = tmp_path / "state.json" - path.write_text("[1, 2, 3]", encoding="utf-8") - assert FileSyncStateStore(path).load() == SyncState() - - def test_survives_across_processes(self, tmp_path: Path) -> None: - # The point of persisting at all: wake_alarm PC is a fresh process - # every minute, so an in-memory store would save nothing. - """Survives across processes.""" - path = tmp_path / "state.json" - remote = FakeRemote() - local = _log("a", "1") - _tick(remote, FileSyncStateStore(path), local) - remote.writes.clear() - _tick(remote, FileSyncStateStore(path), local) - assert not remote.writes - - -@pytest.mark.parametrize("device_id", ["pc", "phone"]) -def test_a_device_publishes_its_revision_under_its_own_id(device_id: str) -> None: - """A device publishes its revision under its own id.""" - remote = FakeRemote() - _tick(remote, MemorySyncStateStore(), _log("a", "1"), device_id=device_id) - assert f"ns/revs/{device_id}" in remote.writes diff --git a/crdt-sync/crdt_sync/tests/test_sync_revisions_backends.py b/crdt-sync/crdt_sync/tests/test_sync_revisions_backends.py new file mode 100644 index 0000000..7976fad --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_sync_revisions_backends.py @@ -0,0 +1,216 @@ +"""Tests for backends without bulk map reads, and publishing order. + +Split from ``test_sync_revisions.py`` (250-line cap); +``test_sync_revisions_peers.py`` keeps peer-download suppression. +""" + + + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest + +from crdt_sync import ( + FileSyncStateStore, + Hlc, + Log, + LogCodec, + MemorySyncStateStore, + Record, + RevisionTracking, + SyncState, + SyncTarget, + dump_log, + load_log, + sync_log, +) +from crdt_sync.tests.conftest import FakeStore, FakeStoreWithoutBulkRead + +if TYPE_CHECKING: + from pathlib import Path + + +def _log(record_id: str, value: str, node_id: str = "node-a") -> Log: + return { + record_id: Record( + id=record_id, + fields={ + "value": (value, Hlc(wall_time_ms=1000, counter=0, node_id=node_id)) + }, + ) + } + + +# The library's own serialization, so these tests exercise the same encoding +# the apps use rather than a test-only one. +_encode = dump_log +_decode = load_log + + +FakeRemote = FakeStore +FakeRemoteWithoutBulkRead = FakeStoreWithoutBulkRead + + +def _tick( + remote: FakeRemote, + store: object = None, + local: Log | None = None, + device_id: str = "pc", +) -> Log: + return sync_log( + SyncTarget( + client=remote, + device_id=device_id, + path_prefix="ns/devices", + ), + local or {}, + LogCodec( + decode=_decode, + encode=_encode, + ), + RevisionTracking( + state_store=store, + ), + ) + + +class TestBackendsWithoutBulkMapReads: + """Backends without bulk map reads.""" + + def test_still_sync_correctly_just_without_the_saving(self) -> None: + # GitHubSyncClient has no get_string_map, so revision lookup degrades + # to "fetch everything" -- correctness must not depend on it. + """Still sync correctly just without the saving.""" + peer = _encode(_log("b", "from-phone", "node-b")) + remote = FakeRemoteWithoutBulkRead({"ns/devices/phone/log.json": peer}) + store = MemorySyncStateStore() + first = _tick(remote, store) + assert "b" in first + remote.reads.clear() + _tick(remote, store, first) + assert "ns/devices/phone/log.json" in remote.reads + + +class TestRevisionPublishingOrder: + """Revision publishing order.""" + + def test_publishes_the_log_before_its_revision(self) -> None: + # Reversed, a peer would cache "seen rev X" against a log it never + # received, and skip it forever. + """Publishes the log before its revision.""" + remote = FakeRemote() + _tick(remote, MemorySyncStateStore(), _log("a", "1")) + assert remote.writes.index("ns/devices/pc/log.json") < remote.writes.index( + "ns/revs/pc" + ) + + def test_each_device_writes_only_its_own_revision_key(self) -> None: + # Per-device keys rather than one shared map: a whole-map write would + # erase every other device's entry, after which those peers would look + # permanently unchanged and never be fetched again. + """Each device writes only its own revision key.""" + remote = FakeRemote({"ns/revs/phone": "peer-rev"}) + _tick(remote, MemorySyncStateStore(), _log("a", "1")) + assert remote.files["ns/revs/phone"] == "peer-rev" + + def test_an_explicit_revs_path_overrides_the_default(self) -> None: + """An explicit revs path overrides the default.""" + remote = FakeRemote() + sync_log( + SyncTarget( + client=remote, + device_id="pc", + path_prefix="ns/devices", + ), + _log("a", "1"), + LogCodec( + decode=_decode, + encode=_encode, + ), + RevisionTracking( + revs_path="custom/place", + state_store=MemorySyncStateStore(), + ), + ) + assert "custom/place/pc" in remote.writes + + +class TestTwoDevicesConverge: + """Two devices converge.""" + + def test_each_ends_up_with_the_others_records(self) -> None: + """Each ends up with the others records.""" + remote = FakeRemote() + pc_store, phone_store = MemorySyncStateStore(), MemorySyncStateStore() + pc = _tick(remote, pc_store, _log("a", "from-pc", "node-pc")) + phone = sync_log( + SyncTarget( + client=remote, + device_id="phone", + path_prefix="ns/devices", + ), + _log("b", "from-phone", "node-phone"), + LogCodec( + decode=_decode, + encode=_encode, + ), + RevisionTracking( + state_store=phone_store, + ), + ) + assert {"a", "b"} <= set(phone) + pc_after = _tick(remote, pc_store, pc) + assert {"a", "b"} <= set(pc_after) + + +class TestFileSyncStateStore: + """File sync state store.""" + + def test_round_trips_through_a_file(self, tmp_path: Path) -> None: + """Round trips through a file.""" + store = FileSyncStateStore(tmp_path / "nested" / "state.json") + state = SyncState(pushed_rev="abc", peer_revs={"phone": "def"}) + store.save(state) + assert store.load() == state + + def test_load_returns_default_state_when_absent(self, tmp_path: Path) -> None: + """Load returns default state when absent.""" + assert FileSyncStateStore(tmp_path / "missing.json").load() == SyncState() + + def test_load_returns_default_state_for_a_truncated_file( + self, tmp_path: Path + ) -> None: + """Load returns default state for a truncated file.""" + path = tmp_path / "state.json" + path.write_text('{"pushed_rev":', encoding="utf-8") + assert FileSyncStateStore(path).load() == SyncState() + + def test_load_returns_default_state_for_a_non_dict_file( + self, tmp_path: Path + ) -> None: + """Load returns default state for a non dict file.""" + path = tmp_path / "state.json" + path.write_text("[1, 2, 3]", encoding="utf-8") + assert FileSyncStateStore(path).load() == SyncState() + + def test_survives_across_processes(self, tmp_path: Path) -> None: + # The point of persisting at all: wake_alarm PC is a fresh process + # every minute, so an in-memory store would save nothing. + """Survives across processes.""" + path = tmp_path / "state.json" + remote = FakeRemote() + local = _log("a", "1") + _tick(remote, FileSyncStateStore(path), local) + remote.writes.clear() + _tick(remote, FileSyncStateStore(path), local) + assert not remote.writes + + +@pytest.mark.parametrize("device_id", ["pc", "phone"]) +def test_a_device_publishes_its_revision_under_its_own_id(device_id: str) -> None: + """A device publishes its revision under its own id.""" + remote = FakeRemote() + _tick(remote, MemorySyncStateStore(), _log("a", "1"), device_id=device_id) + assert f"ns/revs/{device_id}" in remote.writes diff --git a/crdt-sync/crdt_sync/tests/test_sync_revisions_peers.py b/crdt-sync/crdt_sync/tests/test_sync_revisions_peers.py new file mode 100644 index 0000000..f212fee --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_sync_revisions_peers.py @@ -0,0 +1,162 @@ +"""Tests for revision handling across peers. + +Split from ``test_sync_revisions.py`` (250-line cap), which keeps revision +identity, state, and no-op push suppression. +""" + + +from __future__ import annotations + +from crdt_sync import ( + Hlc, + Log, + LogCodec, + MemorySyncStateStore, + Record, + RevisionTracking, + SyncTarget, + dump_log, + load_log, + revision_of, + sync_log, +) +from crdt_sync.tests.conftest import FakeStore, FakeStoreWithoutBulkRead + + +def _log(record_id: str, value: str, node_id: str = "node-a") -> Log: + return { + record_id: Record( + id=record_id, + fields={ + "value": (value, Hlc(wall_time_ms=1000, counter=0, node_id=node_id)) + }, + ) + } + + +# The library's own serialization, so these tests exercise the same encoding +# the apps use rather than a test-only one. +_encode = dump_log +_decode = load_log + + +FakeRemote = FakeStore +FakeRemoteWithoutBulkRead = FakeStoreWithoutBulkRead + + +def _tick( + remote: FakeRemote, + store: object = None, + local: Log | None = None, + device_id: str = "pc", +) -> Log: + return sync_log( + SyncTarget( + client=remote, + device_id=device_id, + path_prefix="ns/devices", + ), + local or {}, + LogCodec( + decode=_decode, + encode=_encode, + ), + RevisionTracking( + state_store=store, + ), + ) + + +class TestPeerDownloadSuppression: + """Peer download suppression.""" + + def test_downloads_a_peer_whose_revision_it_has_never_seen(self) -> None: + """Downloads a peer whose revision it has never seen.""" + peer = _encode(_log("b", "from-phone", "node-b")) + remote = FakeRemote( + { + "ns/devices/phone/log.json": peer, + "ns/revs/phone": revision_of(peer), + } + ) + merged = _tick(remote, MemorySyncStateStore()) + assert "ns/devices/phone/log.json" in remote.reads + assert "b" in merged + + def test_skips_the_download_when_the_peer_revision_is_unchanged(self) -> None: + """Skips the download when the peer revision is unchanged.""" + peer = _encode(_log("b", "from-phone", "node-b")) + remote = FakeRemote( + { + "ns/devices/phone/log.json": peer, + "ns/revs/phone": revision_of(peer), + } + ) + store = MemorySyncStateStore() + first = _tick(remote, store) + remote.reads.clear() + # The peer's records are already in the local log from tick one, so + # re-downloading is pure waste -- the ~700 MB/month this removes. + _tick(remote, store, first) + assert not remote.reads + + def test_downloads_again_once_the_peer_publishes_a_new_revision(self) -> None: + """Downloads again once the peer publishes a new revision.""" + peer_v1 = _encode(_log("b", "v1", "node-b")) + remote = FakeRemote( + { + "ns/devices/phone/log.json": peer_v1, + "ns/revs/phone": revision_of(peer_v1), + } + ) + store = MemorySyncStateStore() + first = _tick(remote, store) + remote.reads.clear() + + peer_v2 = _encode({**_decode(peer_v1), **_log("c", "v2", "node-b")}) + remote.files["ns/devices/phone/log.json"] = peer_v2 + remote.files["ns/revs/phone"] = revision_of(peer_v2) + + second = _tick(remote, store, first) + assert "ns/devices/phone/log.json" in remote.reads + assert {"b", "c"} <= set(second) + + def test_re_downloads_a_peer_whose_push_was_corrupt(self) -> None: + # A failed decode must not be remembered as seen, or the corruption + # would be permanent. + """Re downloads a peer whose push was corrupt.""" + remote = FakeRemote( + { + "ns/devices/phone/log.json": "not json at all", + "ns/revs/phone": revision_of("not json at all"), + } + ) + store = MemorySyncStateStore() + _tick(remote, store) + remote.reads.clear() + _tick(remote, store) + assert "ns/devices/phone/log.json" in remote.reads + + def test_downloads_when_the_peer_has_published_no_revision(self) -> None: + # A device still running pre-migration code publishes a log but no + # revision; it must not be silently ignored. + """Downloads when the peer has published no revision.""" + peer = _encode(_log("b", "from-phone", "node-b")) + remote = FakeRemote({"ns/devices/phone/log.json": peer}) + assert "b" in _tick(remote, MemorySyncStateStore()) + + def test_skips_a_peer_with_nothing_pushed_yet(self) -> None: + """Skips a peer with nothing pushed yet.""" + remote = FakeRemote({"ns/devices/phone/other.txt": "x"}) + assert not _tick(remote, MemorySyncStateStore()) + + def test_never_reads_its_own_device_back(self) -> None: + """Never reads its own device back.""" + remote = FakeRemote( + { + "ns/devices/pc/log.json": _encode(_log("a", "1")), + "ns/revs/pc": "whatever", + } + ) + _tick(remote, MemorySyncStateStore()) + assert not remote.reads diff --git a/crdt-sync/crdt_sync/tests/test_sync_skips.py b/crdt-sync/crdt_sync/tests/test_sync_skips.py new file mode 100644 index 0000000..e1779f4 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_sync_skips.py @@ -0,0 +1,130 @@ +"""Tests for the peer files sync_log declines to merge. + +Split from ``test_sync.py`` (250-line cap). A peer whose pushed file is +missing, corrupt or the wrong shape must be skipped rather than allowed to +fail the whole sync; these cover each of those, plus skipping our own device. +""" + +from __future__ import annotations + +import json +import logging +from typing import TYPE_CHECKING +from unittest.mock import MagicMock + +from crdt_sync import ( + GitHubSyncClient, + LogCodec, + Record, + SyncTarget, + sync_log, +) + +if TYPE_CHECKING: + + import pytest + + from crdt_sync import Log + + +def _encode(log: Log) -> str: + return json.dumps( + {record_id: record.to_dict() for record_id, record in log.items()} + ) + + +def _decode(text: str) -> Log: + return { + record_id: Record.from_dict(data) + for record_id, data in json.loads(text).items() + } + + +def _mock_client( + *, + devices: tuple[str, ...] = (), + files: dict[str, str] | None = None, +) -> MagicMock: + """Build a mock ``GitHubSyncClient`` covering the methods sync_log calls.""" + client = MagicMock(spec=GitHubSyncClient) + client.list_directory.return_value = list(devices) + resolved_files = files or {} + client.get_file_text.side_effect = resolved_files.get + return client + + +class TestSyncLogSkips: + """Peer files that must be passed over rather than merged.""" + + def test_skips_a_device_with_no_pushed_file_yet(self) -> None: + """Skips a device with no pushed file yet.""" + client = _mock_client(devices=("phone",), files={}) + + merged = sync_log( + SyncTarget( + client=client, + device_id="pc", + path_prefix="devices", + ), + {}, + LogCodec( + decode=_decode, + encode=_encode, + ), + ) + + assert not merged + + def test_skips_a_device_whose_pushed_file_is_corrupt( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """An interrupted/truncated push must not crash every other device's + merge -- it is treated the same as a device that hasn't pushed yet. + """ + client = _mock_client( + devices=("phone",), + files={"devices/phone/log.json": "{not valid json"}, + ) + + with caplog.at_level(logging.WARNING): + merged = sync_log( + SyncTarget( + client=client, + device_id="pc", + path_prefix="devices", + ), + {}, + LogCodec( + decode=_decode, + encode=_encode, + ), + ) + + assert not merged + assert "Unparsable log" in caplog.text + + def test_skips_a_device_whose_pushed_json_has_the_wrong_shape(self) -> None: + """Valid JSON that isn't a record map (e.g. from an incompatible + writer) must be skipped like corrupt JSON, not crash the whole + sync -- this is what the broad ``(ValueError, KeyError, TypeError)`` + catch in ``_pull_remote_logs`` is for, not just JSON syntax errors. + """ + client = _mock_client( + devices=("phone",), + files={"devices/phone/log.json": '{"a": 5}'}, + ) + + merged = sync_log( + SyncTarget( + client=client, + device_id="pc", + path_prefix="devices", + ), + {}, + LogCodec( + decode=_decode, + encode=_encode, + ), + ) + + assert not merged