Skip to content

fix: resolve volumes multiget endpoint failing with .json extension#13093

Open
RsbhThakur wants to merge 1 commit into
internetarchive:masterfrom
RsbhThakur:13092/fix/volumes-json-multiget
Open

fix: resolve volumes multiget endpoint failing with .json extension#13093
RsbhThakur wants to merge 1 commit into
internetarchive:masterfrom
RsbhThakur:13092/fix/volumes-json-multiget

Conversation

@RsbhThakur

Copy link
Copy Markdown
Contributor

Closes #13092

fix

Technical

  • Removed the manual URL regex parsing code (MULTIGET_PATH_RE) in get_volumes_multiget (in openlibrary/fastapi/books.py). FastAPI/Starlette already parses the req parameter and automatically strips the .json extension from the URL.
  • Bypassed the custom regex block to directly pass the pre-parsed req parameter to readlinks.readlinks(req, {}).
  • Removed the unused import re and import urllib.parse at the top of books.py (cleaned up automatically via Ruff linter).
  • Updated all three unit test methods under TestGetVolumesMultiget in openlibrary/tests/fastapi/test_books.py to assert the exact arguments passed to the mocked readlinks function, eliminating mock testing gaps and preventing future regressions.

Testing

  1. Run the local development environment.
  2. Verify the multiget volumes API works with the .json extension:
    curl -i http://localhost:8080/api/volumes/brief/json/oclc:19683037.json
    Expected: Returns the full metadata JSON block for the OCLC record.
    Actual (Before): Returned {} empty response.
  3. Run unit tests inside the Docker container to ensure all mock assertions pass successfully:
    docker compose run --rm home pytest openlibrary/tests/fastapi/test_books.py

Screenshot

N/A (This is a backend API bug fix).

@openlibrary-bot

openlibrary-bot commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @RsbhThakur!

The linked issue hasn't been triaged yet — triage happens on Mondays and Fridays. There are currently 30 open non-draft PRs ahead of yours.

PR triage checklist (maintainers / Richy)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — checks are still running
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by PAM, Open Library's Project AI Manager. PAM provides status visibility, performs basic project management functions, and gives actionable feedback so contributors aren't left waiting.

— Richy, via PAM (Open Library's Project AI Manager)

@github-actions github-actions Bot added the Needs: Response Issues which require feedback from lead label Jul 3, 2026
@mekarpeles mekarpeles removed the Needs: Response Issues which require feedback from lead label Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Volumes multiget JSON API /api/volumes/.../json/{req}.json fails to return metadata due to regex parsing issue

3 participants