fix mybooks routing bug

[RayBB]

Closes #12770

Fix: Split lists catch-all route to prevent shadowing reading log endpoint

Problem

Requests to /people/{username}/books/{key}.json (the reading log API) were returning 422 Unprocessable Entity instead of the expected reading log data.

Root Cause

The FastAPI lists module originally defined a catch-all route:

@router.get("/people/{username}/{category}/{list_id}.json")

where category was typed as Literal["lists", "series"] and list_id had pattern OL\d+L.

In FastAPI/Starlette, route matching happens in two stages:

1. URL pattern matching — the path template /people/{username}/{category}/{list_id}.json matches any 4-segment /people/*/*/*.json URL at the string level
2. Pydantic validation — AFTER the route is matched, path parameters are validated against their type annotations

The reading log route (/people/{username}/books/{key}.json) overlaps with this catch-all pattern at the URL level. Since the lists router was registered before the reading log router in asgi_app.py, a request to /people/mekBot/books/want-to-read.json would:

1. Match the lists route at the URL level (username=mekBot, category=books, list_id=want-to-read)
2. Fail Pydantic validation because "books" is not in Literal["lists", "series"]
3. Return 422 — never reaching the reading log handler

Why This Worked in web.py

The legacy web.py framework uses compiled regex patterns for routes. The reading log route was defined as:

path = r"/people/([^/]+)/books/(want-to-read|currently-reading|already-read)"

This regex is specific enough that it only matches the three valid reading log keys. Even if another route was registered earlier, it wouldn't accidentally consume the URL because web.py matches routes sequentially and the reading log regex wouldn't match a route like /people/{username}/lists/{list_id}/'s regex.

The Fix

Split the catch-all route into two explicit routes with no parameterized overlap:

@router.get("/people/{username}/lists/{list_id}.json")
@router.get("/people/{username}/series/{list_id}.json")

This eliminates the route overlap entirely — there is no longer a general {category} segment that could accidentally consume reading log URLs.

Testing

Added a simple integration test (openlibrary/tests/fastapi/test_public_my_books.py) that uses the fastapi_client fixture — this fixture imports ALL routers in the same order as the production app. The test verifies that reading log URLs return 200/403/404 instead of 422 by checking just a few states (public, private, user not found).

Confirmed working on testing:
https://testing.openlibrary.org/people/mekBot/books/already-read.json

All 214 FastAPI tests pass.

How This Test Helps Going Forward

This test catches this specific issue because it exercises the real route registration. Whenever someone introduces a new route that could shadow an existing one, this test — and others like it — will immediately flag it with a 422 failure. For any new endpoints with potentially intersecting path patterns, adding a small test like this is a lightweight way to confirm the right route is being matched.

An Alternative Approach (and why we didn't take it)

Another fix would have been to simply reorder the router registrations in asgi_app.py so that public_my_books_router is included before lists_router. This would work, but it's fragile — the right order isn't obvious and could easily be broken again by someone adding or reordering imports later. Splitting the catch-all into explicit routes is the more robust fix.

Preventing Similar Issues

Future route definitions should avoid using parameterized path segments that could overlap with more specific routes. When a path segment has a fixed set of valid values (like "lists" or "series"), define explicit routes rather than a catch-all with Literal validation.

[Copilot]

Pull request overview

Fixes a FastAPI route-matching conflict where the lists “catch-all” people route could match reading log URLs first and then fail validation, returning 422 instead of reaching /people/{username}/books/{key}.json.

Changes:

• Split the ambiguous lists people route into two explicit routes (/people/{username}/lists/{list_id}.json and /people/{username}/series/{list_id}.json) to eliminate overlap with reading log endpoints.
• Add a FastAPI integration regression test that exercises the full app router registration order and asserts reading log endpoints return 200/403/404 (never 422).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
openlibrary/fastapi/lists.py	Replaces a parameterized {category} path segment with explicit lists and series routes to prevent shadowing the reading log route.
openlibrary/tests/fastapi/test_public_my_books.py	Adds regression coverage to ensure reading log URLs are routed to the correct handler under production router inclusion order.

[cdrini]

Lgtm, good catch! Pushing up a few small fixes now -- namely the people-specific series endpoint shouldn't exist! Tested locally and the endpoints work.

[cdrini]

Unfortunately one of the import lines was changed in a recent PR since the last deploy, so I can't patch this out ; it's causing conflicts :/ Creating a minimal PR based on the last deploy... https://github.com/cdrini/openlibrary/commit/ce81afa2b.diff

[cdrini]

That fixed the 422... But these are now sporadically reporting internal server error :/

[cdrini]

Whatever that issue is, it seems unlikely to be related to the changes here which are just removing endpoints. Merging this piece.