Loading PDF files from URL in REST API and command line - #97
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #97 +/- ##
==========================================
- Coverage 98.68% 96.16% -2.53%
==========================================
Files 13 13
Lines 533 599 +66
==========================================
+ Hits 526 576 +50
- Misses 7 23 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Replaces synchronous `urllib.request` with `httpx.AsyncClient` in `extract_url`. This allows for non-blocking HTTP requests, improving performance when downloading multiple files concurrently. The refactoring handles file downloading, content type checking, and error management asynchronously, ensuring robust handling of network and file system operations.
Refactors the `extract_url` endpoint to accept a single `HttpUrl` instead of a list of URLs. This simplifies the API contract and logic, focusing on processing one file at a time. The implementation is updated to streamline the file download and validation process, ensuring that the input URL points to a PDF file and handling HTTP errors gracefully. It also introduces proper error handling for missing projects during backend retrieval.
…a extraction Refactors the `extract_url` function to first retrieve the project's backend using `registry.get_backend`. This ensures that the metadata extraction process is handled by the configured backend, improving modularity and error handling. The logic is updated to: 1. Fail fast if the project or configuration is missing. 2. Stream the PDF content into a temporary file. 3. Pass the temporary file path to the backend for metadata extraction. This change centralizes the extraction logic and improves robustness against configuration and project errors.
…reaming Replaces the use of `httpx.AsyncClient.stream` with a direct `client.get()` call in `extract_url`. This simplifies the asynchronous HTTP request handling logic. The corresponding tests in `test_api_routes.py` are updated to mock the new `client.get()` behavior instead of the previous streaming interface, ensuring compatibility with the refactored function.
|
|
||
| try: | ||
| async with httpx.AsyncClient() as client: | ||
| response = await client.get(url_str) |
Refactors the mocking logic in `test_schemathesis.py` to correctly simulate asynchronous HTTP responses when testing API endpoints. This ensures that the test environment accurately reflects how `httpx.AsyncClient` handles streaming responses, leading to more reliable and accurate test coverage.
Introduces support for routing all external URL downloads through a configurable proxy. This change adds `BIBRA_URL_PROXY` environment variable support, allowing users to specify a proxy URL. The proxy is integrated into: - `bibra/config.py` via `get_url_proxy()` - `bibra/api/v0/routes.py` (async client) - `bibra/cli.py` (synchronous stream) This feature helps mitigate Server-Side Request Forgery (SSRF) risks by controlling outbound network traffic. Corresponding tests are added to verify proxy usage.
| # Re-import to get fresh import | ||
| import importlib | ||
|
|
||
| import bibra.config |
There was a problem hiding this comment.
Pull request overview
This PR adds an “extract from URL” workflow so PDFs can be fetched server-side (avoiding browser CORS) and then processed via the existing extraction backend, exposed through both the REST API and the CLI.
Changes:
- Added
POST /v0/projects/{project_id}/extract-urlthat downloads a PDF from a submitted URL and extracts metadata. - Added
bibra extract-url <project_id> <url>CLI command that downloads a PDF to a temp file and extracts metadata. - Added/updated tests for API routes, Schemathesis-based API validation, and CLI behavior; introduced
BIBRA_URL_PROXYsupport.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
bibra/api/v0/routes.py |
Adds the extract-url API endpoint and URL-download logic (with proxy env support). |
bibra/cli.py |
Adds extract-url CLI command to download a URL to a temp file and extract metadata. |
bibra/config.py |
Adds get_url_proxy() helper for reading BIBRA_URL_PROXY. |
tests/test_api_routes.py |
Adds route presence + behavior tests for the new API endpoint and proxy passing. |
tests/test_schemathesis.py |
Updates schema-driven API test to handle the new endpoint and mock network calls. |
tests/test_cli.py |
Adds unit tests for the new CLI command with mocked registry/backend and httpx streaming. |
tests/test_config.py |
Adds unit tests for get_url_proxy(). |
.env.example |
Documents the BIBRA_URL_PROXY environment variable. |
uv.lock |
Updates lockfile options metadata. |
Suppressed comments (1)
tests/test_schemathesis.py:75
- The
httpx.AsyncClientmocking here is inconsistent (double-patching the same target and using non-async__aenter__/__aexit__). This will likely fail when the app executesasync with httpx.AsyncClient(...). Mock the returned client as an async context manager and patchhttpx.AsyncClientonce.
with patch("httpx.AsyncClient") as mock_client:
mock_client.__aenter__.return_value = mock_client
mock_client.__aexit__.return_value = False
mock_client.stream = MagicMock(return_value=mock_response)
mock_client.get = async_get
with patch("httpx.AsyncClient", return_value=mock_client):
case.call_and_validate()
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| proxy = get_url_proxy() | ||
| async with httpx.AsyncClient(proxy=proxy) as client: | ||
| response = await client.get(url_str) | ||
|
|
||
| content_type = response.headers.get("content-type", "") | ||
| if content_type != "application/pdf": | ||
| expected = "application/pdf" | ||
| detail = ( | ||
| f"'{url}' does not point to a PDF file. " | ||
| f"Expected '{expected}', got '{content_type}'." | ||
| ) | ||
| raise HTTPException(status_code=400, detail=detail) | ||
|
|
||
| status_code = response.status_code | ||
| if status_code >= 400: | ||
| raise HTTPException( | ||
| status_code=status_code, | ||
| detail=str(response.reason_phrase), | ||
| ) | ||
|
|
||
| with tempfile.NamedTemporaryFile(suffix=".pdf") as tmp: | ||
| async for chunk in response.aiter_bytes(chunk_size=1024 * 1024): | ||
| tmp.write(chunk) | ||
| tmp.flush() | ||
| return await backend.extract([tmp.name]) |
| async def extract_url( | ||
| project_id: str, | ||
| registry: Annotated[ProjectRegistry, Depends(get_registry)], | ||
| url: HttpUrl = Form(...), # noqa: B008 | ||
| ) -> PublicationMetadata: |
| """ | ||
| Extract publication metadata from a PDF or image file at a given URL for a | ||
| specific project. | ||
|
|
||
| Args: | ||
| project_id: The ID of the project to extract metadata for |
| assert "Invalid config syntax" in result.output | ||
|
|
||
|
|
||
| """Tests for the extract-url command.""" |
| @@ -1,3 +1,5 @@ | |||
| from unittest.mock import MagicMock, patch | |||
| assert len(extract_url_routes) >= 1 | ||
| # Check that the route uses POST method | ||
| route = extract_url_routes[0] | ||
| assert isinstance(route, APIRoute) | ||
|
|
| # URL Download Proxy (for SSRF mitigation) | ||
| # All URL downloads will be routed through this proxy. | ||
| #BIBRA_URL_PROXY=http://proxy.example.com:8080 |
Reasons for creating this PR
It is not currently possible to load PDF files from all URLs in web UI due to CORS restrictions. This PR adds functionality for loading PDFs to REST API and cli.
Link to relevant issue(s), if any
Description of the changes in this PR
extract-urlenpoint to REST APIextract-urlmethod to cliInstructions how to test this PR
REST API method can be called with
Cli method can be called with
Known problems or uncertainties in this PR
I'm not sure the way mocking is used in the unit tests is always appropriate, especially in
test_cli.pyChecklist
Disclosure of AI Tool Usage
Please indicate AI use by choosing the most suitable TLP:AI category below and removing the irrelevant categories from the list. AI:ORANGE is the minimum level for merging.
Describe the AI tool(s) you used: Claude Sonnet 5 in chat mode for unit tests and Gemini-2.5-flash for cli and REST code.