Switch transport to aiohttp; add session re-auth and order/product endpoints

[dvejsada]

Replaces the httpx transport with aiohttp so the package fits the Home Assistant ecosystem, and upstreams a few robustness improvements proven out in the HA-RohlikCZ integration.

Transport: httpx → aiohttp

• HttpClient is now built on aiohttp.ClientSession. HTTP/2 is dropped (aiohttp doesn't need it; HA mandates aiohttp).
• Bring-your-own session: RohlikAPI(..., session=...) accepts an externally managed aiohttp.ClientSession (e.g. HA's shared session via async_get_clientsession). An injected session is never closed by the client. The httpx-specific client property becomes session.
• A small buffered Response wrapper keeps the service layer synchronous (.json() / .raise_for_status()), decoupling services from aiohttp's streaming semantics.
• Query params are coerced to aiohttp-acceptable strings (bool → "true"/"false", None dropped, nested dicts/lists JSON-encoded) — matching what the integration already did inline for product search.
• Connection/timeout failures are caught via a shared HTTP_ERRORS tuple (aiohttp.ClientError plus TimeoutError, since aiohttp raises asyncio.TimeoutError on timeout, which is not a ClientError).

Transparent re-authentication on HTTP 401

• HttpClient invokes an on_unauthorized callback and retries the request once when it receives a 401, so a long-lived client recovers from an expired session instead of failing every subsequent call.
• RohlikAPI wires this to a new AuthManager.relogin(). Login/logout requests are exempt to avoid recursion; concurrent 401s are serialized so only one re-login fires.

New endpoints (from the integration)

• orders.get_detail(order_id) — full order including line items
• orders.get_all_delivered(page_size=50) — paginates the entire delivered-orders history
• products.get_detail(product_id) — raw product detail (brand, attributes)
• products.get_categories(product_id) — category hierarchy
• 404 → None convention for these optional-resource fetches, so a discontinued product reads as "not found" rather than an error.

Tests & docs

• 133 tests passing (was 120); added coverage for the 401 retry path (incl. login-endpoint exemption and no-handler passthrough), relogin, session injection, the new endpoints, and the 404 handling.
• ruff, black, mypy (strict) all green on Python 3.13.
• README updated: aiohttp + session-injection section, re-auth feature note, and the new service methods.

🤖 Generated with Claude Code

---

Generated by Claude Code

[dvejsada]

@claude please review this PR. Focus areas:

• The aiohttp HttpClient rewrite, especially the buffered Response wrapper and connection/release handling in _request.
• The 401 re-auth retry logic — correctness of the login/logout recursion exemption and the concurrent-401 serialization (_reauth_lock).
• Session injection semantics: an injected session must never be closed by the client.
• The new orders/products endpoints and the 404 → None convention.

---

Generated by Claude Code

[claude]

Claude finished @dvejsada's task in 3m 41s —— View job

---

PR Review: Switch transport to aiohttp; add session re-auth and order/product endpoints

Tasks:

• Gather context and read changed files
• Review HttpClient rewrite (buffered Response wrapper, connection/release handling)
• Review 401 re-auth retry logic (recursion exemption, _reauth_lock serialization)
• Review session injection semantics
• Review new orders/products endpoints and 404 → None convention
• Post final review feedback

---

The overall structure is clean and well thought out. A few issues, one of which could cause a real problem in HA.

---

1. _reauth_lock does not deduplicate — N concurrent 401s cause N sequential re-logins

http_client.py:211–214

async with self._reauth_lock:
    await self._on_unauthorized()
status, body, response = await _send()

The lock serializes concurrent 401 handlers but does not prevent each waiting coroutine from calling relogin() in turn. Trace for two coroutines A and B that both receive a 401:

1. A acquires lock → relogin() → _reset_session() → login() → success, _is_logged_in = True
2. A releases lock
3. B acquires lock → relogin() → _reset_session() (sets _is_logged_in = False again) → login() → fires a second HTTP auth request

For N simultaneous 401s (typical in HA with many sensors polling at once), you get N sequential logins instead of 1. The comment "Serialize re-auth so concurrent 401s trigger a single login" is therefore inaccurate.

Fix: use a generation counter so waiters skip re-auth if it already happened while they were queued:

# In __init__:
self._reauth_generation = 0

# In _request:
current_gen = self._reauth_generation
async with self._reauth_lock:
    if self._reauth_generation == current_gen:  # no one re-authed yet
        await self._on_unauthorized()
        self._reauth_generation += 1
status, body, response = await _send()

Fix this →

---

2. Connection leak if response.read() raises mid-response

http_client.py:188–202

async def _send() -> tuple[int, bytes, aiohttp.ClientResponse]:
    response = await self.session.request(...)
    body = await response.read()   # ← can throw ServerDisconnectedError, etc.
    return response.status, body, response

response.read() reads and releases the connection on success. But if it raises (e.g. aiohttp.ServerDisconnectedError, asyncio.TimeoutError), the connection is not explicitly released and will leak from the pool until the session is closed. The idiomatic fix is a context manager or explicit try/finally:

async with self.session.request(..., timeout=...) as response:
    body = await response.read()
    return response.status, body, response

aiohttp.ClientResponse.__aexit__ calls release() on any code path, including exceptions. release() is idempotent after a completed read(), so there is no double-release risk.

Fix this →

---

3. Injected closed session is silently replaced with a new owned session

http_client.py:109–114

@property
def session(self) -> aiohttp.ClientSession:
    if self._session is None or self._session.closed:
        self._session = aiohttp.ClientSession()
        self._owns_session = True
    return self._session

If HA's shared session closes (e.g. during teardown) and code still tries to make a request, this property silently creates a new owned session that bypasses HA's connection management. The caller gets no indication that the injected session was replaced, and the new session won't have HA's connector or SSL context.

Better to not re-create when the original session was injected and is now closed:

@property
def session(self) -> aiohttp.ClientSession:
    if self._owns_session and (self._session is None or self._session.closed):
        self._session = aiohttp.ClientSession()
    return self._session  # type: ignore[return-value]

Any subsequent request would then raise RuntimeError: Session is closed, which is the expected signal that the client should be torn down and rebuilt.

Fix this →

---

4. get_all_delivered silently returns partial results on mid-pagination failure

services/orders.py:56–79

while True:
    page = await self.get_delivered(limit=page_size, offset=offset)
    if not page:   # None (error) and [] (done) are indistinguishable here
        break
    all_orders.extend(page)
    ...

If page 2 fails (network error) after page 1 succeeds, the function returns only the first page without any indication that the result is incomplete. The docstring documents this for page-1 failures but is silent about mid-pagination failures. At minimum the docstring should note this, or the function could distinguish None from []:

page = await self.get_delivered(limit=page_size, offset=offset)
if page is None:     # request error — stop, return what we have
    break
if not page:         # empty list — genuinely done
    break

This at least makes the two termination paths explicitly different and would allow a warning log on page is None.

---

Minor: Response keeps a full aiohttp.ClientResponse reference for raise_for_status

http_client.py:39–52

__slots__ = ("status", "_body", "_response")

def raise_for_status(self) -> None:
    self._response.raise_for_status()

aiohttp.ClientResponse.raise_for_status() only needs status, reason, request_info, history, and headers. Holding the whole response object alive means headers, cookies, and the URL object are retained for the lifetime of Response. Not a problem at modest load, but worth noting if memory efficiency matters. A simpler alternative is to store only the status, body, and a bound method reference.

---

What looks good

• _prepare_params — correct coercion of bool, None, and nested objects for aiohttp.
• HTTP_ERRORS tuple — correctly includes TimeoutError separately since aiohttp's asyncio.TimeoutError isn't a ClientError.
• _NO_REAUTH_ENDPOINTS — the login/logout exemption is correct and tested.
• Session ownership flag _owns_session — close() correctly checks this before closing. The injected-session tests at test_http_client.py:134–155 validate both test_injected_session_is_used and test_close_does_not_close_injected_session.
• 404 → None in get_detail / get_categories — checking response.status == 404 before raise_for_status() correctly suppresses the _LOGGER.warning that _fetch_endpoint's error handler would otherwise emit for an expected not-found case.
• relogin() avoids the login() short-circuit — calling _reset_session() first ensures _is_logged_in = False, so login() always fires a fresh HTTP request rather than returning the stale cached response.

---

[dvejsada]

Thanks for the review — all four substantive issues are addressed in 7fc93b1:

1. Concurrent-401 dedup — added a _reauth_generation counter; coroutines that queue behind an in-flight re-auth see the bumped generation and skip the redundant login. New test test_concurrent_401s_trigger_single_reauth asserts a single handler call (and exactly 4 requests) for two simultaneous 401s.
2. Connection release — _send now buffers the body inside async with session.request(...), so the connection is released even if read() raises mid-response.
3. Closed injected session — the session property only (re)creates owned sessions; a closed injected session is returned as-is, so the next request raises aiohttp's Session is closed rather than silently spawning a session that bypasses the owner's connector/SSL. New test test_closed_injected_session_is_not_replaced.
4. get_all_delivered partial results — now distinguishes None (request error → warn, stop, possibly-incomplete) from [] (genuinely done), and the docstring documents the incomplete-result case.

On the minor point (Response retaining the full ClientResponse): I left it as-is. The suggested "store a bound method reference" doesn't actually reduce retention — a bound response.raise_for_status still keeps response alive via __self__. The only real alternative is to capture status/reason/request_info/history/headers and reconstruct ClientResponseError ourselves, which is more surface for negligible gain: a Response is created and consumed within a single service call and then dropped, so it isn't retained across requests. Happy to revisit if profiling ever shows it matters.

135 tests pass; ruff, black, and mypy (strict) all green on 3.13.

---

Generated by Claude Code

[dvejsada]

@claude please re-review the latest changes (commit 7fc93b1), which address the first review:

• The _reauth_generation dedup for concurrent 401s — is the generation capture/compare race-free?
• The async with session.request(...) change in _send — connection release on all paths.
• The session property no longer recreating a closed injected session.
• orders.get_all_delivered distinguishing None (error) from [] (done).

Also flag anything new the changes may have introduced.

---

Generated by Claude Code

[claude]

Claude finished @dvejsada's task in 3m 6s —— View job

---

Re-review: commit 7fc93b1

Tasks:

• Load tools
• Read http_client.py — generation dedup, connection release, session property
• Read services/orders.py — None vs [] distinction
• Read tests/test_http_client.py and tests/test_services.py
• Post final review

---

All four issues from the first review are correctly addressed. Detailed analysis of each fix follows.

---

1. _reauth_generation dedup — race-free ✓

http_client.py:225–230

seen_generation = self._reauth_generation
async with self._reauth_lock:
    if self._reauth_generation == seen_generation:
        await self._on_unauthorized()
        self._reauth_generation += 1

This is race-free in asyncio's cooperative model. The key invariant is that seen_generation = self._reauth_generation has no await in it, so no other coroutine can interleave between the read and the lock acquire. The dedup check inside the lock then determines whether re-auth has already happened:

Trace for two concurrent 401s (A and B):

1. A captures seen_generation = 0, acquires lock (uncontested, no yield), starts await _on_unauthorized(), yields
2. B captures seen_generation = 0 (generation still 0), blocks on lock
3. A's handler returns → _reauth_generation = 1 → releases lock
4. B acquires lock: 1 != 0 → skips handler. B still retries the request (retry is unconditional after the lock block)

The test at test_http_client.py:248–269 validates this correctly: _FakeResponse.__aenter__ yields via asyncio.sleep(0) to create the overlap, and the _CountingSession counter confirms exactly 4 requests (2 initial 401s + 2 retries) and 1 handler call. ✓

One edge case worth knowing: if _on_unauthorized() raises an exception, the generation is not incremented (the increment is after the await, which would be skipped). Lock waiters would then each see seen_generation == _reauth_generation and each call the handler in turn. This is probably the right behavior for a transient re-auth failure (let each caller escalate), but it is worth documenting if the expected failure behavior is "all fail fast."

---

2. async with session.request(...) — connection release ✓

http_client.py:202–212

async with self.session.request(...) as response:
    body = await response.read()
    return response.status, body, response

Connection is released via ClientResponse.__aexit__ → release() on every code path, including if read() raises. ✓

One clarification worth having in mind: the return statement exits the async with block, so __aexit__() is called before the return value is handed to the caller. The returned response is a released aiohttp.ClientResponse. The stored reference in Response._response is kept only for raise_for_status(), which accesses status, reason, request_info, history, headers — none requiring an active connection. This is safe and intentional.

---

3. Closed injected session not replaced ✓

http_client.py:120–124

if self._owns_session and (self._session is None or self._session.closed):
    self._session = aiohttp.ClientSession()
assert self._session is not None
return self._session

A closed injected session is returned as-is. The next request will raise aiohttp's RuntimeError: Session is closed, which is the correct signal to the owner. ✓

Minor nit (not a new bug): the assert on line 123 is disabled when Python is run with -O. The invariant it guards is sound by construction (an owned client always creates a session above; an injected client always sets _session in __init__), so this won't trigger in practice. If you want to be defensive, raise RuntimeError("no session") would survive -O.

---

4. get_all_delivered — None vs [] ✓

services/orders.py:72–86

if page is None:
    _LOGGER.warning("Stopped paginating delivered orders at offset %s; result may be incomplete", offset)
    break
if not page:
    break  # Empty page: genuinely no more orders.

The two termination paths are now distinct. None (transport error) logs a warning and returns partial results; [] (end of list) exits cleanly. The docstring documents the incomplete-result case. ✓

---

Nothing new introduced

Reviewed all changes in 7fc93b1 against origin/main:

• get_all_delivered additionally calls _ensure_logged_in() before its loop, even though get_delivered() (called in the loop) also calls it — harmless double-check, no issue.
• The _CountingSession.request() test helper is synchronous, which is intentional: the asyncio.sleep(0) in _FakeResponse.__aenter__ is the interleaving point, and that is the right place for it (it creates the race window on the lock, not inside request()).
• No changes to auth.py or other service files that look problematic.

---

Verdict: the four fixes are sound. This is ready to merge.

[dvejsada]

@claude final review please. Since the last approving review, commit f75fb6b adds Attributes: docstrings to all models, fixes RecipeDetail.duration (int → str, since the API returns a display string), corrects the RohlikAPI example to use the typed Cart, replaces the session property assert with an explicit raise (so it survives python -O), and documents the re-auth-callback failure behaviour. Please confirm there are no remaining issues; if it looks good, this is ready to merge.

---

Generated by Claude Code

[claude]

Claude finished @dvejsada's task in 1m 48s —— View job

---

Final review: commit f75fb6b

Tasks:

• Load tools
• Read http_client.py — raise vs assert fix, re-auth failure docs
• Read models.py — Attributes: docstrings, RecipeDetail.duration type fix
• Read client.py — RohlikAPI example Cart correction
• Verify tests cover duration as a string
• Post final verdict

---

All five changes in f75fb6b are correct. No new issues introduced.

Attributes: docstrings — complete ✓

All 21 dataclasses in models.py now have Attributes: sections. The descriptions are accurate: units are stated (CZK, kJ/kcal, grams), the str vs float price distinction is explained in the module docstring, ID cross-references point to the right methods (cart.add_items, recipes.get_ingredient_products, etc.), and the servings/sales/products_in_list raw-dict fields are flagged explicitly.

RecipeDetail.duration: int | None → str | None — correct ✓

The test fixture at tests/test_recipes.py:114 already passes "duration": "Do hodinky" (a string) and asserts result.duration == "Do hodinky" at line 157. The old int annotation was wrong; the fix brings the annotation in line with both the real API and the existing test.

RohlikAPI example uses typed Cart — correct ✓

Old: print(cart["total_price"]) — dict access on what is actually a Cart dataclass.
New: cart.total_price, cart.total_items, item.name/item.quantity/item.price — all valid attribute accesses on Cart and CartItem. The example now correctly reflects the service layer's return type.

session property raise RuntimeError — correct ✓

# pragma: no cover - unreachable by construction is appropriate: owned sessions are created in the if branch above, injected sessions are always non-None after __init__. The raise (as opposed to assert) survives python -O. The comment explains why the pattern was chosen.

_request callback-failure documentation — accurate ✓

"If the callback raises (re-auth itself failed), the error propagates to the caller and the request is not retried; a subsequent request will attempt re-auth again."

Tracing the code: if _on_unauthorized() raises, _reauth_generation is not incremented (the increment is on the line after the await, which is never reached), the lock is released by async with, and the next request starts with the same generation — so it will attempt re-auth again. The documentation correctly describes this behavior.

---

Verdict: this is ready to merge. All issues from both previous review rounds are addressed; f75fb6b introduces no new concerns.