Return MultiParams from request.post_vars() to handle multi-valued keys#2755
Open
aicayzer wants to merge 1 commit into
Open
Return MultiParams from request.post_vars() to handle multi-valued keys#2755aicayzer wants to merge 1 commit into
aicayzer wants to merge 1 commit into
Conversation
…form fields Form submissions with multiple values for the same key (e.g. ``<select multiple>``) previously had every value but the last silently discarded, because ``post_vars()`` called ``dict(parse_qsl(...))``. Switch the implementation to return a ``MultiParams`` object built from ``parse_qs(...)``, mirroring the existing ``request.args`` shape so both sides of the GET/POST surface behave consistently. ``MultiParams`` gains an ``items()`` method that yields (key, first_value) pairs, matching ``__getitem__`` semantics, so it works in patterns such as ``dict(post_vars())`` and the existing ``_coerce_execute_write_payload`` dict comprehension. Internal callers of ``_json_or_form_payload`` (``execute_write`` and ``stored_queries``) had ``isinstance(data, dict)`` guards intended to validate JSON shape but also rejected the new ``MultiParams`` return on the form path. Gate those guards on ``is_json`` so the form path passes through unchanged. The bundled ``my_plugin`` test fixture wraps ``post_vars()`` in ``dict()`` before passing to ``Response.json``, demonstrating the migration path for plugins that need a JSON-serialisable mapping. Closes simonw#2425
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2425.
Problem
request.post_vars()is implemented asdict(parse_qsl(body, keep_blank_values=True)), which silently collapses any duplicate keys to the last value. Forms that submit multiple values for the same field — e.g.<select multiple>, multi-checkbox groups,?_facet=...&_facet=...style POSTs — lose data with no error.request.argsalready handles this correctly viaMultiParams.Fix
Return a
MultiParamsbuilt fromparse_qs(...)instead of adict, matching the shape ofrequest.args.MultiParamswas already imported inasgi.py, so no new imports.Added an
items()method toMultiParamsthat yields(key, first_value)pairs (matching__getitem__semantics), soMultiParamscan be passed directly intodict(), dict-comprehension patterns and similar.The internal
_json_or_form_payloadhelper feeds two callers —_coerce_execute_write_payloadand theQueryStoreView/QueryUpdateViewPOST handlers — that both hadisinstance(data, dict)guards intended to validate JSON shape. Those guards spuriously rejected the newMultiParamson the form path, so they have been gated onis_json. The fall-through logic that re-renders the HTML form on validation errors uses.get()onquery_dataand works against eitherdictorMultiParams.Behavioural notes
post_vars[key]andpost_vars.get(key)continue to work but now return the first submitted value rather than the last. This matchesrequest.argssemantics and is documented in the changelog.post_vars.getlist(key)now returns the full list of submitted values — the new capability the issue asked for.Response.json(await request.post_vars())should wrap indict(...)becauseMultiParamsis not directly JSON-serialisable. The bundledmy_plugintest fixture has been updated to demonstrate this.Test plan
test_request_post_varsto assertMultiParamsbehaviour against the previously-existing single-value payload.test_request_post_vars_multimirrorstest_request_args, covering[],get,getlist,in,keys,lenandKeyErrorfor missing keys.pytest --ignore=tests/test_multipart.py -n auto: 1853 passed).tests/test_multipart.pyhas a pre-existingModuleNotFoundError: multipart_form_data_conformanceunrelated to this change.Files touched
datasette/utils/asgi.py—post_vars()returnsMultiParams.datasette/utils/__init__.py—MultiParams.items().datasette/views/query_helpers.py— gateisinstance(data, dict)onis_jsonin_coerce_execute_write_payload.datasette/views/stored_queries.py— gateisinstanceguards onis_jsoninQueryStoreView.postandQueryUpdateView.post; drop the now-redundantisinstance(query_data, dict)checks in the error fall-through branches.tests/test_internals_request.py— updated and added tests.tests/plugins/my_plugin.py— wrappost_vars()indict()for JSON serialisation.docs/internals.rstanddocs/changelog.rst— documentation updates.