From 2f65c0178c5858bd4acacddeb48574dad10d4b3b Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 15 Aug 2026 15:23:14 +0200 Subject: [PATCH] fix: address final review findings for service clients plan Fixes 9 numbered findings + 7 minor fixes from the whole-plan review of the MyWhoosh/Garmin service clients (Plan 3): Garmin uploader (app/garmin/uploader.py): - Detect Garmin-rejected imports (failures without successes) and raise new GarminImportRejected instead of reporting them as successful. - Reclassify 429/rate-limit/500 login failures as transient instead of falling through to permanent auth errors; unrecognized login failures are now treated as transient (retryable) rather than GarminAuthError. - Mirror the auth-token check from the login branch into the import branch so 401-at-import-time raises GarminAuthError instead of propagating raw. - Add common GarminError base class, hoist transient-token tuple to a shared module constant, check response.status_code==409 before the duplicate substring fallback, and create the tokenstore dir 0o700. MyWhoosh client (app/mywhoosh/client.py): - Add optional max_pages bound to list_activities pagination. - Add aclose()/__aenter__/__aexit__ so the client's own httpx.AsyncClient gets closed, while never closing an injected client. - Guard the two remaining unguarded JSON-decode paths (login body, download-fit metadata) so malformed bodies raise MyWhooshIntegrationError instead of raw ValueError/AttributeError. - Row-level malformation (missing id/activityFileId, unparseable startDatetime) is now skipped rather than aborting the whole page; envelope-shape failures still raise. id/activityFileId checks use explicit None/"" comparisons instead of Python falsiness. - Replace asserts in _authenticated_post with explicit exceptions; restrict the reauth retry to 401 only, treat 403 as immediately terminal; naive startDatetime values are now treated as already-UTC instead of host-local. MyWhoosh tokenstore (app/mywhoosh/tokenstore.py): - load() now treats any corrupt/malformed token file (bad JSON, missing keys, OS errors) as "absent" instead of raising, so a bad cache no longer permanently wedges a user. pyproject.toml: - Tighten garminconnect pin to >=0.3.10,<1 (import_activity requires 0.3.10+). Adds/updates tests across tests/mywhoosh/ and tests/garmin/ covering all of the above, including a fake client that wraps GarminUploadBlocked in a plain RuntimeError to mirror the real garminconnect library's MFA error wrapping. Co-Authored-By: Claude Sonnet 5 --- app/garmin/uploader.py | 54 +++++++++-- app/mywhoosh/client.py | 58 +++++++++--- app/mywhoosh/tokenstore.py | 12 +-- pyproject.toml | 2 +- tests/garmin/test_uploader.py | 98 +++++++++++++++++++- tests/mywhoosh/test_client_activities.py | 113 ++++++++++++++++++++++- tests/mywhoosh/test_client_auth.py | 97 ++++++++++++++++++- tests/mywhoosh/test_tokenstore.py | 25 +++++ 8 files changed, 426 insertions(+), 33 deletions(-) diff --git a/app/garmin/uploader.py b/app/garmin/uploader.py index 69d67bd..5b69545 100644 --- a/app/garmin/uploader.py +++ b/app/garmin/uploader.py @@ -18,18 +18,40 @@ class UploadResult: raw_response: Any -class GarminUploadBlocked(RuntimeError): +class GarminError(RuntimeError): pass -class GarminAuthError(RuntimeError): +class GarminUploadBlocked(GarminError): pass -class GarminTransientError(RuntimeError): +class GarminAuthError(GarminError): pass +class GarminTransientError(GarminError): + pass + + +class GarminImportRejected(GarminError): + pass + + +_TRANSIENT_ERROR_TOKENS = ( + "timeout", + "temporar", + "connection", + "429", + "too many", + "rate limit", + "500", + "502", + "503", + "504", +) + + class GarminUploader: def __init__( self, @@ -56,9 +78,12 @@ class GarminUploader: if _looks_duplicate_error(exc): return UploadResult("duplicate", True, None, str(exc)) text = str(exc).lower() - if any(token in text for token in ("timeout", "temporar", "connection", "502", "503", "504")): + if any(token in text for token in _TRANSIENT_ERROR_TOKENS): raise GarminTransientError("Garmin import failed transiently") from exc + if any(token in text for token in ("password", "credential", "unauthorized", "401")): + raise GarminAuthError("Garmin import failed: authentication rejected") from exc raise + _raise_if_import_rejected(response) return UploadResult("imported", False, _extract_activity_id(response), response) finally: self._mfa_code = None @@ -66,7 +91,7 @@ class GarminUploader: def _ensure_client(self) -> GarminClientProtocol: if self._client is not None: return self._client - self.tokenstore.mkdir(parents=True, exist_ok=True) + self.tokenstore.mkdir(parents=True, exist_ok=True, mode=0o700) factory = self.client_factory or _default_garmin_factory client = factory(self.email, self.password, prompt_mfa=self._prompt_mfa) try: @@ -79,9 +104,9 @@ class GarminUploader: raise GarminUploadBlocked("Garmin MFA is required") from exc if any(token in text for token in ("password", "credential", "unauthorized", "401")): raise GarminAuthError("Garmin authentication failed") from exc - if any(token in text for token in ("timeout", "temporar", "connection", "502", "503", "504")): + if any(token in text for token in _TRANSIENT_ERROR_TOKENS): raise GarminTransientError("Garmin login failed transiently") from exc - raise GarminAuthError("Garmin login failed") from exc + raise GarminTransientError("Garmin login failed") from exc self._client = client return client @@ -98,10 +123,25 @@ def _default_garmin_factory(*args: Any, **kwargs: Any) -> GarminClientProtocol: def _looks_duplicate_error(exc: Exception) -> bool: + status_code = getattr(getattr(exc, "response", None), "status_code", None) + if status_code == 409: + return True text = str(exc).lower() return any(token in text for token in ("duplicate", "already exists", "409")) +def _raise_if_import_rejected(response: Any) -> None: + if not isinstance(response, dict): + return + detailed = response.get("detailedImportResult") + if not isinstance(detailed, dict): + return + failures = detailed.get("failures") + successes = detailed.get("successes") + if isinstance(failures, list) and failures and not successes: + raise GarminImportRejected(f"Garmin rejected the import ({len(failures)} failure(s))") + + def _extract_activity_id(response: Any) -> str | None: if not isinstance(response, dict): return None diff --git a/app/mywhoosh/client.py b/app/mywhoosh/client.py index e224225..4b645f9 100644 --- a/app/mywhoosh/client.py +++ b/app/mywhoosh/client.py @@ -31,9 +31,20 @@ class MyWhooshIntegrationError(MyWhooshError): class MyWhooshClient: def __init__(self, token_store: MyWhooshTokenStore, http_client: httpx.AsyncClient | None = None) -> None: self.token_store = token_store + self._owns_http = http_client is None self.http = http_client or httpx.AsyncClient(timeout=30.0) self.token = token_store.load() + async def aclose(self) -> None: + if self._owns_http: + await self.http.aclose() + + async def __aenter__(self) -> "MyWhooshClient": + return self + + async def __aexit__(self, *exc_info: object) -> None: + await self.aclose() + async def login(self, email: str, password: str) -> None: payload = { "Username": email, @@ -56,6 +67,8 @@ class MyWhooshClient: body = response.json() except ValueError as exc: raise MyWhooshIntegrationError("MyWhoosh login returned invalid JSON") from exc + if not isinstance(body, dict): + raise MyWhooshIntegrationError("MyWhoosh login response is not a JSON object") if body.get("Success") is not True or not body.get("AccessToken"): raise MyWhooshAuthError(str(body.get("Message") or "MyWhoosh login failed")) self.token = MyWhooshToken( @@ -72,7 +85,8 @@ class MyWhooshClient: async def _authenticated_post(self, url: str, payload: dict, email: str, password: str) -> httpx.Response: await self.ensure_authenticated(email, password) for attempt in range(2): - assert self.token is not None + if self.token is None: + raise MyWhooshIntegrationError("no token after ensure_authenticated") try: response = await self.http.post( url, @@ -81,7 +95,9 @@ class MyWhooshClient: ) except httpx.TransportError as exc: raise MyWhooshTransientError("MyWhoosh request failed") from exc - if response.status_code not in {401, 403}: + if response.status_code == 403: + raise MyWhooshAuthError(f"MyWhoosh returned HTTP {response.status_code}") + if response.status_code != 401: if response.status_code >= 500: raise MyWhooshTransientError(f"MyWhoosh returned HTTP {response.status_code}") return response @@ -91,13 +107,15 @@ class MyWhooshClient: await self.login(email, password) continue raise MyWhooshAuthError("MyWhoosh session rejected after reauthentication") - raise AssertionError("unreachable") + raise MyWhooshIntegrationError("unreachable state in _authenticated_post") - async def list_activities(self, email: str, password: str) -> list[MyWhooshActivity]: + async def list_activities( + self, email: str, password: str, max_pages: int | None = None + ) -> list[MyWhooshActivity]: activities: list[MyWhooshActivity] = [] page = 1 total_pages = 1 - while page <= total_pages: + while page <= total_pages and (max_pages is None or page <= max_pages): response = await self._authenticated_post( ACTIVITIES_BASE + "rider/profile/activities", {"sortDate": "DESC", "page": page}, @@ -119,26 +137,30 @@ class MyWhooshClient: if not isinstance(results, list): raise MyWhooshIntegrationError("MyWhoosh activities response has unexpected shape") for row in results: - activities.append(self._normalize_activity(row)) + activity = self._normalize_activity(row) + if activity is not None: + activities.append(activity) page += 1 return activities - def _normalize_activity(self, row: object) -> MyWhooshActivity: + def _normalize_activity(self, row: object) -> MyWhooshActivity | None: if not isinstance(row, dict): raise MyWhooshIntegrationError("MyWhoosh activity row is not an object") activity_id = row.get("id") activity_file_id = row.get("activityFileId") - if not activity_id or not activity_file_id: - raise MyWhooshIntegrationError("MyWhoosh activity row missing stable id or activityFileId") + if activity_id is None or activity_id == "" or activity_file_id is None or activity_file_id == "": + return None raw_started = row.get("startDatetime") started_at: datetime | None = None if raw_started: try: - started_at = datetime.fromisoformat(str(raw_started).replace("Z", "+00:00")).astimezone( - timezone.utc - ) - except ValueError as exc: - raise MyWhooshIntegrationError("MyWhoosh activity row has invalid startDatetime") from exc + parsed = datetime.fromisoformat(str(raw_started).replace("Z", "+00:00")) + except ValueError: + return None + if parsed.tzinfo is None: + started_at = parsed.replace(tzinfo=timezone.utc) + else: + started_at = parsed.astimezone(timezone.utc) return MyWhooshActivity( id=str(activity_id), title=str(row.get("title") or ""), @@ -155,7 +177,13 @@ class MyWhooshClient: ) if response.status_code >= 400: raise MyWhooshIntegrationError(f"download metadata returned HTTP {response.status_code}") - url = response.json().get("data") + try: + body = response.json() + except ValueError as exc: + raise MyWhooshIntegrationError("MyWhoosh download response returned invalid JSON") from exc + if not isinstance(body, dict): + raise MyWhooshIntegrationError("MyWhoosh download response is not a JSON object") + url = body.get("data") if not isinstance(url, str) or not url: raise MyWhooshIntegrationError("MyWhoosh download response has no URL") try: diff --git a/app/mywhoosh/tokenstore.py b/app/mywhoosh/tokenstore.py index 5ed3dda..347bea2 100644 --- a/app/mywhoosh/tokenstore.py +++ b/app/mywhoosh/tokenstore.py @@ -12,13 +12,13 @@ class MyWhooshTokenStore: def load(self) -> MyWhooshToken | None: try: raw = json.loads(self.path.read_text("utf-8")) - except FileNotFoundError: + return MyWhooshToken( + access_token=raw["access_token"], + refresh_token=raw.get("refresh_token"), + whoosh_id=raw.get("whoosh_id"), + ) + except (FileNotFoundError, OSError, ValueError, KeyError, TypeError): return None - return MyWhooshToken( - access_token=raw["access_token"], - refresh_token=raw.get("refresh_token"), - whoosh_id=raw.get("whoosh_id"), - ) def save(self, token: MyWhooshToken) -> None: self.path.parent.mkdir(parents=True, exist_ok=True, mode=0o700) diff --git a/pyproject.toml b/pyproject.toml index b618751..02c9ce9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ dependencies = [ "python-multipart>=0.0.9,<1", "itsdangerous>=2.1,<3", "httpx>=0.27,<1", - "garminconnect>=0.2,<1", + "garminconnect>=0.3.10,<1", ] [project.optional-dependencies] diff --git a/tests/garmin/test_uploader.py b/tests/garmin/test_uploader.py index 3e2a2c9..0317ed0 100644 --- a/tests/garmin/test_uploader.py +++ b/tests/garmin/test_uploader.py @@ -2,7 +2,13 @@ from pathlib import Path import pytest -from app.garmin.uploader import GarminUploadBlocked, GarminUploader +from app.garmin.uploader import ( + GarminAuthError, + GarminImportRejected, + GarminTransientError, + GarminUploadBlocked, + GarminUploader, +) class FakeGarmin: @@ -78,3 +84,93 @@ def test_mfa_code_is_returned_only_to_prompt(tmp_path: Path) -> None: ) uploader.import_fit(tmp_path / "ride.fit", mfa_code="123456") assert seen == ["123456"] + + +def test_import_rejected_by_garmin_raises_garmin_import_rejected(tmp_path: Path) -> None: + rejected_response = { + "detailedImportResult": { + "successes": [], + "failures": [{"internalId": 1, "messages": ["Invalid FIT file"]}], + } + } + uploader = GarminUploader( + email="g@example.com", + password="pw", + tokenstore=tmp_path / "garmin", + client_factory=lambda *a, **kw: FakeGarmin(*a, **kw, import_result=rejected_response), + ) + with pytest.raises(GarminImportRejected): + uploader.import_fit(tmp_path / "ride.fit") + + +def test_login_failure_with_429_is_transient(tmp_path: Path) -> None: + uploader = GarminUploader( + email="g@example.com", + password="pw", + tokenstore=tmp_path / "garmin", + client_factory=lambda *a, **kw: FakeGarmin(*a, **kw, login_error=RuntimeError("429 too many requests")), + ) + with pytest.raises(GarminTransientError): + uploader.import_fit(tmp_path / "ride.fit") + + +def test_login_failure_with_timeout_is_transient(tmp_path: Path) -> None: + uploader = GarminUploader( + email="g@example.com", + password="pw", + tokenstore=tmp_path / "garmin", + client_factory=lambda *a, **kw: FakeGarmin(*a, **kw, login_error=RuntimeError("connection timeout")), + ) + with pytest.raises(GarminTransientError): + uploader.import_fit(tmp_path / "ride.fit") + + +def test_login_failure_with_credential_message_is_auth_error(tmp_path: Path) -> None: + uploader = GarminUploader( + email="g@example.com", + password="pw", + tokenstore=tmp_path / "garmin", + client_factory=lambda *a, **kw: FakeGarmin(*a, **kw, login_error=RuntimeError("invalid credential")), + ) + with pytest.raises(GarminAuthError): + uploader.import_fit(tmp_path / "ride.fit") + + +def test_login_failure_unrecognized_is_transient(tmp_path: Path) -> None: + uploader = GarminUploader( + email="g@example.com", + password="pw", + tokenstore=tmp_path / "garmin", + client_factory=lambda *a, **kw: FakeGarmin(*a, **kw, login_error=RuntimeError("something odd happened")), + ) + with pytest.raises(GarminTransientError): + uploader.import_fit(tmp_path / "ride.fit") + + +def test_import_time_401_raises_auth_error(tmp_path: Path) -> None: + uploader = GarminUploader( + email="g@example.com", + password="pw", + tokenstore=tmp_path / "garmin", + client_factory=lambda *a, **kw: FakeGarmin(*a, **kw, import_error=RuntimeError("401 unauthorized")), + ) + with pytest.raises(GarminAuthError): + uploader.import_fit(tmp_path / "ride.fit") + + +def test_mfa_wrapped_in_generic_exception_still_blocks(tmp_path: Path) -> None: + class WrappedMfaGarmin(FakeGarmin): + def login(self, tokenstore=None): + try: + self.prompt_mfa() + except GarminUploadBlocked as exc: + raise RuntimeError(f"Login failed: Garmin requested MFA ({exc})") from exc + + uploader = GarminUploader( + email="g@example.com", + password="pw", + tokenstore=tmp_path / "garmin", + client_factory=WrappedMfaGarmin, + ) + with pytest.raises(GarminUploadBlocked): + uploader.import_fit(tmp_path / "ride.fit") diff --git a/tests/mywhoosh/test_client_activities.py b/tests/mywhoosh/test_client_activities.py index 14619eb..07c36b4 100644 --- a/tests/mywhoosh/test_client_activities.py +++ b/tests/mywhoosh/test_client_activities.py @@ -134,7 +134,7 @@ async def test_download_fit_fetches_signed_url_bytes(tmp_path) -> None: @pytest.mark.asyncio -async def test_list_activities_raises_integration_error_on_missing_stable_id(tmp_path) -> None: +async def test_list_activities_skips_row_missing_stable_id(tmp_path) -> None: async def handler(request: httpx.Request) -> httpx.Response: if request.url.path.endswith("/activities"): return httpx.Response( @@ -147,7 +147,13 @@ async def test_list_activities_raises_integration_error_on_missing_stable_id(tmp "title": "Ride without id", "activityFileId": "f-1", "startDatetime": "2026-08-15T06:00:00.000Z", - } + }, + { + "id": "a-2", + "title": "Ride with id", + "activityFileId": "f-2", + "startDatetime": "2026-08-15T06:00:00.000Z", + }, ], } }, @@ -158,5 +164,108 @@ async def test_list_activities_raises_integration_error_on_missing_stable_id(tmp store.save(MyWhooshToken(access_token="cached", refresh_token=None, whoosh_id=None)) client = MyWhooshClient(store, http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler))) + activities = await client.list_activities("rider@example.com", "secret") + + assert [a.id for a in activities] == ["a-2"] + + +@pytest.mark.asyncio +async def test_list_activities_skips_row_with_unparseable_start_datetime(tmp_path) -> None: + async def handler(request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/activities"): + return httpx.Response( + 200, + json={ + "data": { + "totalPages": 1, + "results": [ + { + "id": "a-1", + "title": "Ride with bad date", + "activityFileId": "f-1", + "startDatetime": "not-a-date", + }, + { + "id": "a-2", + "title": "Ride with good date", + "activityFileId": "f-2", + "startDatetime": "2026-08-15T06:00:00.000Z", + }, + ], + } + }, + ) + raise AssertionError(request.url) + + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + store.save(MyWhooshToken(access_token="cached", refresh_token=None, whoosh_id=None)) + client = MyWhooshClient(store, http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler))) + + activities = await client.list_activities("rider@example.com", "secret") + + assert [a.id for a in activities] == ["a-2"] + + +@pytest.mark.asyncio +async def test_list_activities_raises_integration_error_on_envelope_shape_failure(tmp_path) -> None: + async def handler(request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/activities"): + return httpx.Response(200, json={"data": {"totalPages": 1}}) + raise AssertionError(request.url) + + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + store.save(MyWhooshToken(access_token="cached", refresh_token=None, whoosh_id=None)) + client = MyWhooshClient(store, http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler))) + with pytest.raises(MyWhooshIntegrationError): await client.list_activities("rider@example.com", "secret") + + +@pytest.mark.asyncio +async def test_list_activities_respects_max_pages(tmp_path) -> None: + calls = [] + + async def handler(request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/activities"): + payload = json.loads(request.content) + page = payload["page"] + calls.append(page) + result = { + "data": { + "totalPages": 5, + "results": [ + { + "id": f"a-{page}", + "title": f"Ride {page}", + "activityFileId": f"f-{page}", + "startDatetime": "2026-08-15T06:00:00.000Z", + } + ], + } + } + return httpx.Response(200, json=result) + raise AssertionError(request.url) + + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + store.save(MyWhooshToken(access_token="cached", refresh_token=None, whoosh_id=None)) + client = MyWhooshClient(store, http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler))) + + activities = await client.list_activities("rider@example.com", "secret", max_pages=1) + + assert calls == [1] + assert [a.id for a in activities] == ["a-1"] + + +@pytest.mark.asyncio +async def test_download_fit_raises_integration_error_on_invalid_json(tmp_path) -> None: + async def handler(request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/download-activity-file"): + return httpx.Response(200, content=b"not json", headers={"content-type": "application/json"}) + raise AssertionError(request.url) + + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + store.save(MyWhooshToken(access_token="cached", refresh_token=None, whoosh_id=None)) + client = MyWhooshClient(store, http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler))) + + with pytest.raises(MyWhooshIntegrationError): + await client.download_fit("f-1", "rider@example.com", "secret") diff --git a/tests/mywhoosh/test_client_auth.py b/tests/mywhoosh/test_client_auth.py index bb8ef10..37908ec 100644 --- a/tests/mywhoosh/test_client_auth.py +++ b/tests/mywhoosh/test_client_auth.py @@ -1,7 +1,12 @@ import httpx import pytest -from app.mywhoosh.client import MyWhooshClient, MyWhooshAuthError +from app.mywhoosh.client import ( + MyWhooshAuthError, + MyWhooshClient, + MyWhooshIntegrationError, + MyWhooshTransientError, +) from app.mywhoosh.models import MyWhooshToken from app.mywhoosh.tokenstore import MyWhooshTokenStore @@ -37,3 +42,93 @@ async def test_invalid_credentials_raise_auth_error(tmp_path) -> None: ) with pytest.raises(MyWhooshAuthError): await client.login("rider@example.com", "bad") + + +@pytest.mark.asyncio +async def test_login_returns_json_array_raises_integration_error(tmp_path) -> None: + async def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, json=["not", "an", "object"]) + + client = MyWhooshClient( + MyWhooshTokenStore(tmp_path / "mywhoosh.json"), + http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler)), + ) + with pytest.raises(MyWhooshIntegrationError): + await client.login("rider@example.com", "bad") + + +@pytest.mark.asyncio +async def test_login_5xx_raises_transient_error(tmp_path) -> None: + async def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(503, text="Service Unavailable") + + client = MyWhooshClient( + MyWhooshTokenStore(tmp_path / "mywhoosh.json"), + http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler)), + ) + with pytest.raises(MyWhooshTransientError): + await client.login("rider@example.com", "secret") + + +@pytest.mark.asyncio +async def test_second_401_after_reauth_raises_auth_error(tmp_path) -> None: + login_call_count = 0 + + async def handler(request: httpx.Request) -> httpx.Response: + nonlocal login_call_count + if request.url.path.endswith("/activities"): + return httpx.Response(401, json={"message": "expired"}) + if request.url.path.endswith("/login"): + login_call_count += 1 + return httpx.Response( + 200, + json={ + "Success": True, + "AccessToken": "fresh-access", + "RefreshToken": "fresh-refresh", + "WhooshId": "w-1", + }, + ) + raise AssertionError(request.url) + + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + store.save(MyWhooshToken(access_token="stale", refresh_token=None, whoosh_id=None)) + client = MyWhooshClient(store, http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler))) + + with pytest.raises(MyWhooshAuthError): + await client.list_activities("rider@example.com", "secret") + + assert login_call_count == 1 + + +@pytest.mark.asyncio +async def test_aclose_closes_self_owned_http_client(tmp_path) -> None: + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + client = MyWhooshClient(store) + + await client.aclose() + + assert client.http.is_closed is True + + +@pytest.mark.asyncio +async def test_aclose_does_not_close_injected_http_client(tmp_path) -> None: + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + injected = httpx.AsyncClient() + client = MyWhooshClient(store, http_client=injected) + + await client.aclose() + + assert injected.is_closed is False + await injected.aclose() + + +@pytest.mark.asyncio +async def test_client_usable_as_async_context_manager(tmp_path) -> None: + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + + async with MyWhooshClient(store) as client: + http_client = client.http + assert http_client.is_closed is False + + assert http_client.is_closed is True diff --git a/tests/mywhoosh/test_tokenstore.py b/tests/mywhoosh/test_tokenstore.py index 2a322aa..5c33a66 100644 --- a/tests/mywhoosh/test_tokenstore.py +++ b/tests/mywhoosh/test_tokenstore.py @@ -16,3 +16,28 @@ def test_tokenstore_round_trip_and_permissions(tmp_path: Path) -> None: def test_missing_token_returns_none(tmp_path: Path) -> None: store = MyWhooshTokenStore(tmp_path / "missing.json") assert store.load() is None + + +def test_corrupt_token_file_returns_none(tmp_path: Path) -> None: + path = tmp_path / "mywhoosh.json" + path.write_bytes(b"not valid json {{{") + store = MyWhooshTokenStore(path) + assert store.load() is None + + +def test_token_file_missing_access_token_returns_none(tmp_path: Path) -> None: + path = tmp_path / "mywhoosh.json" + path.write_text('{"refresh_token": "r", "whoosh_id": "w"}', encoding="utf-8") + store = MyWhooshTokenStore(path) + assert store.load() is None + + +def test_clear_removes_token_and_load_returns_none(tmp_path: Path) -> None: + store = MyWhooshTokenStore(tmp_path / "tokens" / "mywhoosh.json") + token = MyWhooshToken(access_token="access", refresh_token="refresh", whoosh_id="whoosh-1") + store.save(token) + + store.clear() + + assert store.load() is None + assert not store.path.exists()