Skip to content

Add request.form data migration support#359

Merged
zmievsa merged 7 commits intomainfrom
add-form-data-migration-support
Mar 22, 2026
Merged

Add request.form data migration support#359
zmievsa merged 7 commits intomainfrom
add-form-data-migration-support

Conversation

@zmievsa
Copy link
Copy Markdown
Owner

@zmievsa zmievsa commented Mar 21, 2026

Expose a mutable form property on RequestInfo so users can read and modify multipart/form-encoded fields (strings and file uploads) inside convert_request_to_next_version_for handlers. The property returns None for non-form requests, matching the existing pattern for missing bodies. Mutated form data is reconstructed into a new FormData before being passed to FastAPI's dependency solver.

Expose a mutable `form` property on `RequestInfo` so users can read and
modify multipart/form-encoded fields (strings and file uploads) inside
`convert_request_to_next_version_for` handlers. The property returns
`None` for non-form requests, matching the existing pattern for missing
bodies. Mutated form data is reconstructed into a new `FormData` before
being passed to FastAPI's dependency solver.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 96.84211% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.84%. Comparing base (62e15d1) to head (73ef8e7).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
tests/test_data_migrations.py 96.38% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #359      +/-   ##
==========================================
- Coverage   99.89%   99.84%   -0.06%     
==========================================
  Files          72       72              
  Lines        5553     5645      +92     
  Branches      356      358       +2     
==========================================
+ Hits         5547     5636      +89     
- Misses          6        9       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Remove redundant None guards in migrator functions that were always True,
causing partial branch coverage. The tests only make form requests so
form is always set; add type: ignore comments to keep type checkers happy.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zmievsa-semasoftware
Copy link
Copy Markdown

Code review

Found 2 issues:

  1. Multi-value form fields are silently truncated. dict(body.multi_items()) discards all but the last value for repeated keys (e.g. tag=a&tag=b{"tag": "b"}), and the reconstruction in versions.py compounds this with .items() on the lossy dict. Any endpoint using List[str] = Form(...) or repeated form fields will silently receive truncated data even when no migration is defined.

if isinstance(body, FormData):
self._form: Union[dict[str, Union[UploadFile, str]], None] = dict(body.multi_items())
else:
self._form = None

CURRENT_DEPENDENCY_SOLVER_VAR.set("cadwyn")
body_for_solving = (
FormData(list(request_info._form.items())) if request_info._form is not None else request_info.body
)

  1. _get_body has # pragma: no cover # This is from FastAPI on its definition, but the new form migration tests now call this function. The pragma excludes the entire function from coverage tracking, which is now inaccurate. Removing it may require adding coverage for any remaining uncovered branches inside the function.

async def _get_body(
request: FastapiRequest, body_field: Union[ModelField, None], exit_stack: AsyncExitStack
): # pragma: no cover # This is from FastAPI
is_body_form = body_field and isinstance(body_field.field_info, params.Form)
try:

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

zmievsa and others added 5 commits March 21, 2026 17:51
- Store form data as list[tuple] instead of dict to preserve duplicate
  keys (e.g. tags=a&tags=b no longer truncated to tags=b)
- Reconstruct FormData directly from the list, dropping the lossy
  .items() call in _migrate_request
- Remove function-level pragma: no cover from _get_body since the form
  migration tests now exercise it; retain targeted pragmas only on the
  two defensive exception handlers (HTTPException re-raise and generic
  Exception catch)
- Update existing form migration tests to use list operations and add
  tests for multi-value fields and all previously uncovered _get_body
  branches

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pport

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@zmievsa zmievsa linked an issue Mar 22, 2026 that may be closed by this pull request
@zmievsa zmievsa merged commit 5a7eea2 into main Mar 22, 2026
12 checks passed
@zmievsa zmievsa deleted the add-form-data-migration-support branch March 22, 2026 13:13
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.

Add request.form data migration support

2 participants