Skip to content

Commit 6a88783

Browse files
badGarnetclaude
andauthored
fix(etl-uvicorn): do not require a body when every input field is optional (#73)
## Problem `wrap_in_fastapi` chooses its `/invoke` signature on parameter **presence**: ```python if input_schema_model.model_fields: async def run_job(request: input_schema_model) # no default => body REQUIRED else: async def run_job() # no body accepted ``` A pydantic body parameter with no default is mandatory **even when every field inside the model is optional**. So a plugin whose parameters are all optional gets a required body that no caller has a reason to populate — and before it grew those parameters, that same plugin accepted no body at all. Adding an optional parameter therefore looks backward-compatible while silently flipping the HTTP contract. ## Fix Default the body when the generated input model has no required fields: ```python body_is_optional = input_schema_model.model_fields and not any( field.is_required() for field in input_schema_model.model_fields.values() ) ``` - **all fields optional** → `request: Optional[input_schema_model] = None`; an absent body resolves each field to its own default, which is exactly what the function signature already promises. - **any field required** → unchanged, body mandatory. A downloader invoked without `file_data` still fails validation rather than receiving `None`. - **no parameters** → unchanged, no body accepted. The handler body is extracted into `run_job_with_body` so the two model-bearing branches cannot drift. ## Testing `make check-version`, `make check` clean; **75 tests pass** (5 new). | plugin shape | bodyless | `{}` | populated | |---|---|---|---| | no params | 200 | 200 | — | | all params optional | **200** (was 422) | 200 | 200 | | any param required | 422 | 422 | 200 | New tests are in `test/api/test_api.py`. Verified they catch the regression by reverting the fix: `test_all_optional_params_accept_absent_or_empty_body[None]` fails. The other four are guards against the fix over-reaching — notably `test_required_param_still_rejects_an_absent_body`, since silently accepting an absent `file_data` would be worse than the bug being fixed. Also verified against the **real** playground indexer served through the patched generator, rather than only synthetic functions: - bodyless POST → 200, indexes from the settings-file fallback - populated wire `invocation_settings` → 200, and **takes precedence** over the file fallback That second case is the point: this restores the bodyless contract without reverting the wire-settings capability. Both planes work. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/Unstructured-IO/unstructured-platform-plugins/pull/73?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4aea423 commit 6a88783

4 files changed

Lines changed: 167 additions & 14 deletions

File tree

CHANGELOG.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
1+
## 0.0.45
2+
3+
* **`/invoke` no longer demands a body from a plugin whose parameters are all optional.** A pydantic
4+
body parameter with no default is mandatory even when every field inside the model is optional, so
5+
such a plugin required a body that no caller has a reason to populate — and before it grew those
6+
parameters the same plugin accepted no body at all, which made adding one look
7+
backward-compatible while silently flipping the HTTP contract to 422 for every bodyless caller.
8+
An absent body now resolves each field to its own default, which is what the signature already
9+
promised. Plugins with at least one required field are unchanged: a missing `file_data` still
10+
fails validation rather than arriving as `None`.
11+
* **An optional `file_data` no longer 500s when absent.** The wrapper converted `file_data` from its
12+
dict form unconditionally, so a plugin declaring it optional hit
13+
`AttributeError: 'NoneType' object has no attribute 'model_dump'` on any body that omitted it —
14+
previously reachable via `POST {}`, and via a bodyless request once the change above landed. `None`
15+
is now passed through untouched and only a real value is converted.
16+
117
## 0.0.44
218

319
* **Ignore SIGTERM in plugin uvicorn Servers**: plugin webservers now keep

test/api/test_api.py

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,3 +470,114 @@ def test_streaming_unstructured_ingest_error_with_none_status_code():
470470
"Async gen test UnstructuredIngestError with None status_code"
471471
in invoke_response.status_code_text
472472
)
473+
474+
475+
# --- optional-body contract -------------------------------------------------------------------
476+
#
477+
# A pydantic body parameter with no default is mandatory even when every field inside the model is
478+
# optional. A plugin whose parameters are ALL optional therefore used to demand a body that no
479+
# caller has a reason to populate: before it grew those parameters the same plugin accepted no body
480+
# at all, so adding one silently flipped its HTTP contract and every bodyless caller got a 422.
481+
482+
483+
class _Echo(BaseModel):
484+
settings: Optional[dict] = None
485+
context: Optional[dict] = None
486+
received: Optional[str] = None
487+
488+
489+
def _all_optional(
490+
invocation_settings: Optional[dict] = None, invocation_context: Optional[dict] = None
491+
) -> _Echo:
492+
return _Echo(settings=invocation_settings, context=invocation_context)
493+
494+
495+
def _no_params() -> _Echo:
496+
return _Echo(received="ok")
497+
498+
499+
def _has_required(element_dicts: str, invocation_context: Optional[dict] = None) -> _Echo:
500+
return _Echo(received=element_dicts)
501+
502+
503+
@pytest.mark.parametrize("body", [None, {}])
504+
def test_all_optional_params_accept_absent_or_empty_body(body):
505+
client = TestClient(wrap_in_fastapi(func=_all_optional, plugin_id="mock_plugin"))
506+
507+
kwargs = {} if body is None else {"json": body}
508+
resp = client.post("/invoke", **kwargs)
509+
510+
assert resp.status_code == 200
511+
invoke_response = InvokeResponse.model_validate(resp.json())
512+
invoke_response.generic_validation()
513+
# Each field resolves to its own default, which is what the signature already promised.
514+
assert invoke_response.output == {"settings": None, "context": None, "received": None}
515+
516+
517+
def test_all_optional_params_still_receive_a_populated_body():
518+
# The tolerance must not swallow a body that IS supplied, or the wire-settings plane silently
519+
# stops working while every request keeps returning 200.
520+
client = TestClient(wrap_in_fastapi(func=_all_optional, plugin_id="mock_plugin"))
521+
522+
resp = client.post("/invoke", json={"invocation_settings": {"k": "v"}})
523+
524+
assert resp.status_code == 200
525+
assert InvokeResponse.model_validate(resp.json()).output == {
526+
"settings": {"k": "v"},
527+
"context": None,
528+
"received": None,
529+
}
530+
531+
532+
def test_required_param_still_rejects_an_absent_body():
533+
# The tolerance must not leak into plugins that genuinely need input: a downloader invoked
534+
# without `file_data` has to fail loudly rather than receive None.
535+
client = TestClient(wrap_in_fastapi(func=_has_required, plugin_id="mock_plugin"))
536+
537+
assert client.post("/invoke").status_code == 422
538+
assert client.post("/invoke", json={}).status_code == 422
539+
assert client.post("/invoke", json={"element_dicts": "x"}).status_code == 200
540+
541+
542+
class _FileDataEcho(BaseModel):
543+
identifier: Optional[str] = None
544+
545+
546+
def _optional_file_data(
547+
file_data: Optional[FileData] = None, invocation_context: Optional[dict] = None
548+
) -> _FileDataEcho:
549+
return _FileDataEcho(identifier=None if file_data is None else file_data.identifier)
550+
551+
552+
@pytest.mark.parametrize("body", [None, {}])
553+
def test_optional_file_data_is_preserved_as_none(body):
554+
# `file_data` is converted from its dict form for the wrapped function, but it can legitimately
555+
# be absent. Calling `.model_dump()` on None raised before `wrap_fn` ran, turning the
556+
# optional-body contract into a 500 rather than a normal response.
557+
client = TestClient(wrap_in_fastapi(func=_optional_file_data, plugin_id="mock_plugin"))
558+
559+
kwargs = {} if body is None else {"json": body}
560+
resp = client.post("/invoke", **kwargs)
561+
562+
assert resp.status_code == 200
563+
invoke_response = InvokeResponse.model_validate(resp.json())
564+
invoke_response.generic_validation()
565+
assert invoke_response.output == {"identifier": None}
566+
567+
568+
def test_optional_file_data_is_still_converted_when_supplied():
569+
client = TestClient(wrap_in_fastapi(func=_optional_file_data, plugin_id="mock_plugin"))
570+
571+
resp = client.post("/invoke", json={"file_data": mock_file_data[0].model_dump()})
572+
573+
assert resp.status_code == 200
574+
assert InvokeResponse.model_validate(resp.json()).output == {"identifier": "mock file data"}
575+
576+
577+
def test_no_param_plugin_still_accepts_a_bodyless_post():
578+
client = TestClient(wrap_in_fastapi(func=_no_params, plugin_id="mock_plugin"))
579+
580+
resp = client.post("/invoke")
581+
582+
assert resp.status_code == 200
583+
assert InvokeResponse.model_validate(resp.json()).output["received"] == "ok"
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
__version__ = "0.0.44" # pragma: no cover
1+
__version__ = "0.0.45" # pragma: no cover

unstructured_platform_plugins/etl_uvicorn/api_generator.py

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -254,22 +254,48 @@ async def _stream_response():
254254
file_data=request_dict.get("file_data", None),
255255
)
256256

257-
if input_schema_model.model_fields:
257+
async def run_job_with_body(request: BaseModel) -> ResponseType:
258+
log_func_and_body(func=func, body=request.json())
259+
# Create dictionary from pydantic model while preserving underlying types
260+
request_dict = {f: getattr(request, f) for f in request.model_fields}
261+
# Make sure nested classes get instantiated correctly. `file_data` can legitimately be None
262+
# -- a plugin may declare it optional, and then an absent or partial body leaves it unset --
263+
# so convert only a real value. Calling `.model_dump()` on None would raise before `wrap_fn`
264+
# runs, turning the optional-body contract into a 500.
265+
file_data = request_dict.get("file_data")
266+
if file_data is not None:
267+
request_dict["file_data"] = file_data_from_dict(file_data.model_dump())
268+
map_inputs(func=func, raw_inputs=request_dict)
269+
if logger.level == LOG_LEVELS.get("trace", logging.NOTSET):
270+
logger.log(level=logger.level, msg=f"passing inputs to function: {request_dict}")
271+
return await wrap_fn(func=func, kwargs=request_dict)
272+
273+
# A pydantic body parameter with no default is mandatory even when every field inside the model
274+
# is optional. So a plugin whose parameters are ALL optional would demand a body that no caller
275+
# has a reason to populate -- and before it grew those parameters that same plugin accepted no
276+
# body at all, so adding one flips the HTTP contract while looking backward-compatible. Default
277+
# the body in that case: an absent body resolves each field to its own default, which is exactly
278+
# what the function signature already promises.
279+
#
280+
# A plugin with at least one required field keeps a mandatory body, so an indexer that needs
281+
# `file_data` still fails validation rather than silently receiving None.
282+
body_is_optional = input_schema_model.model_fields and not any(
283+
field.is_required() for field in input_schema_model.model_fields.values()
284+
)
285+
286+
if body_is_optional:
287+
288+
@fastapi_app.post("/invoke", response_model=InvokeResponse)
289+
async def run_job(request: Optional[input_schema_model] = None) -> ResponseType:
290+
return await run_job_with_body(
291+
request if request is not None else input_schema_model()
292+
)
293+
294+
elif input_schema_model.model_fields:
258295

259296
@fastapi_app.post("/invoke", response_model=InvokeResponse)
260297
async def run_job(request: input_schema_model) -> ResponseType:
261-
log_func_and_body(func=func, body=request.json())
262-
# Create dictionary from pydantic model while preserving underlying types
263-
request_dict = {f: getattr(request, f) for f in request.model_fields}
264-
# Make sure nested classes get instantiated correctly
265-
if "file_data" in request_dict:
266-
request_dict["file_data"] = file_data_from_dict(
267-
request_dict["file_data"].model_dump()
268-
)
269-
map_inputs(func=func, raw_inputs=request_dict)
270-
if logger.level == LOG_LEVELS.get("trace", logging.NOTSET):
271-
logger.log(level=logger.level, msg=f"passing inputs to function: {request_dict}")
272-
return await wrap_fn(func=func, kwargs=request_dict)
298+
return await run_job_with_body(request)
273299

274300
else:
275301

0 commit comments

Comments
 (0)