Skip to content

Commit

Permalink
Start to implement marshmallow-code#419 (flaskparser first)
Browse files Browse the repository at this point in the history
Stated goals in the form of CHANGELOG update

Gut significant chunks of webargs.core
- replace
    "parse_{location}" with "load_{location}"
    "locations=<collection>" with "location=<string>"
    "location_handler" with "location_loader"
- remove
    "parse_arg"
    "get_value"
- add
    webargs.multidictproxy which uses a Schema to handle MultiDict types
    and behave similarly to webargs.core.get_value

As a proving ground for this implementation/approach, get flaskparser
tests to pass under ma3

Fixes as the location loading code undergoes refactoring for the
flaskparser:
- Restore missing mimetype check for JSON payloads
- Treat form data as a multidict without extraneous AttributeError check
  (after testing, flask will return '{}' for form data when it is
  missing or empty -- just respect/allow this)
- req.headers is a multidict type as well
- fix some mistakes with new argument ordering

Changes to the base test app at src/webargs/testing.py of note:
- Because each route can only specify that it loads data from a single
  location (unless you pull extra tricks), several testing cases have
  had their routes modified, e.g. "/echo" -> "/echo_json"
- New test, test_parse_json_list_error_malformed_data added to check
  that webargs does not incorrectly treat non-list data as singleton
  lists when passing data to a schema with list fields. The previous
  behavior was questionable before, but with full data going to schemas
  it definitely becomes incorrect.
- Many tests which previously expected success with malformed data
  ignored now expect 422 errors. For example, sending a non-dict JSON
  value (e.g. `1`) should fail because it cannot be loaded by the
  schema. It should *not* be silently ignored because data was provided
  and failed to parse. This is a major behavior change for webargs.

Supporting modifications were made in tests/apps/flask_app.py ,
including the addition of `ma.EXCLUDE` when the detected marshmallow
version is >= 3.

After work on the relevant PR and trying things both ways, the
current handling of empty JSON bodies (interpreting them as {}) is being
kept externally, but changed internally. This relates to the decision
made in marshmallow-code#297 . Originally in the course of work on marshmallow-code#419, it seemed like
it might be necessary to change the behavior to make missing JSON data
behave as `missing` externally. However, making the change internal and
wrapping it to present the same external interface is best.

load_* methods are expected to return `missing` or {} when data is
missing for the given location. e.g. When a JSON body is "" it should be
`missing`. This is considered correct.
However, in terms of the external behavior of webargs, no data should be
interpreted as `{}`.

In order to achieve this, the wrapper which calls the various
"loader_func"s to load location data explicitly checks the result for
`missing` and converts it to `{}` if it is missing.

This makes it easy to use webargs to implement an API with optional JSON
data, but internally preserves the ability to detect a missing JSON
payload and distinguish it from a payload containing `{}`.
  • Loading branch information
sirosen committed Nov 24, 2019
1 parent 4099814 commit 5a58a23
Show file tree
Hide file tree
Showing 10 changed files with 523 additions and 464 deletions.
32 changes: 17 additions & 15 deletions CHANGELOG.rst
Expand Up @@ -16,35 +16,37 @@ Refactoring:

* *Backwards-incompatible*: Schema fields may not specify a location any
longer, and `Parser.use_args` and `Parser.use_kwargs` now accept `location`
(singular) instead of `locations` plural. Instead of using a single field or
(singular) instead of `locations` (plural). Instead of using a single field or
schema with multiple `locations`, users are recommended to make multiple
calls to `use_args` or `use_kwargs` with a distinct schema per location. For
example, code should be rewritten like this:

.. code-block:: python
# under webargs v5
class CompoundSchema:
q1 = ma.fields.Int(location="query")
q2 = ma.fields.Int(location="query")
h1 = ma.fields.Int(location="headers")
@parser.use_args(CompoundSchema(), locations=("query", "headers"))
@parser.use_args(
{
"q1": ma.fields.Int(location="query"),
"q2": ma.fields.Int(location="query"),
"h1": ma.fields.Int(location="headers"),
},
locations=("query", "headers"),
)
def foo(q1, q2, h1):
...
# should be split up like so under webargs v6
class QueryParamSchema:
q1 = ma.fields.Int()
q2 = ma.fields.Int()
class HeaderSchema:
h1 = ma.fields.Int()
@parser.use_args(QueryParamSchema(), location="query")
@parser.use_args(HeaderSchema(), location="headers")
@parser.use_args({"q1": ma.fields.Int(), "q2": ma.fields.Int()}, location="query")
@parser.use_args({"h1": ma.fields.Int()}, location="headers")
def foo(q1, q2, h1):
...
* The `location_handler` decorator has been removed and replaced with
`location_loader`. `location_loader` serves the same purpose (letting you
write custom hooks for loading data) but its expected method signature is
different. See the docs on `location_loader` for proper usage.

5.5.2 (2019-10-06)
******************

Expand Down
19 changes: 19 additions & 0 deletions src/webargs/asyncparser.py
Expand Up @@ -96,6 +96,25 @@ async def parse(
)
return data

async def _load_location_data(self, schema, req, location):
"""Return a dictionary-like object for the location on the given request.
Needs to have the schema in hand in order to correctly handle loading
lists from multidict objects and `many=True` schemas.
"""
loader_func = self._get_loader(location)
if asyncio.iscoroutinefunction(loader_func):
data = await loader_func(req, schema)
else:
data = loader_func(req, schema)

# when the desired location is empty (no data), provide an empty
# dict as the default so that optional arguments in a location
# (e.g. optional JSON body) work smoothly
if data is core.missing:
data = {}
return data

async def _on_validation_error(
self,
error: ValidationError,
Expand Down

0 comments on commit 5a58a23

Please sign in to comment.