Skip to content

Commit

Permalink
Updates to get flaskparser tests to pass under ma3
Browse files Browse the repository at this point in the history
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.
- Reverse the decision on marshmallow-code#297 . After reading the referent material, I
  agree with the aiohttp maintainers in aio-libs/aiohttp#3302 . "" and
  "{}" should not be treated equivalently -- they are different inputs.
  Additionally, I foresee that supporting "" as "{}" will pose an issue
  if the proposal for "json_or_form" in marshmallow-code#419 is accepted.
  More detailed explanation is provided in the inline comment, and the
  test for this case is preserved with its expectations reversed

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

Minor fix in core: restore the cache clearing step. This is apparently
used by the flaskparser.
Possibly this only shows up under test usage? The main parse function
should always be cloning the parser before its cache could be
populated.
  • Loading branch information
sirosen committed Sep 4, 2019
1 parent dd58ab3 commit 849550b
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 84 deletions.
1 change: 1 addition & 0 deletions src/webargs/core.py
Expand Up @@ -207,6 +207,7 @@ def _clone(self):
that these methods always have separate caches.
"""
clone = copy(self)
clone._cache = {}
return clone

def parse(
Expand Down
18 changes: 9 additions & 9 deletions src/webargs/flaskparser.py
Expand Up @@ -54,12 +54,15 @@ class FlaskParser(core.Parser):
**core.Parser.__location_map__
)

def location_load_view_args(self, schema, req):
def location_load_view_args(self, req, schema):
"""Pull a the request's ``view_args``."""
return req.view_args or core.missing

def location_load_json(self, schema, req):
def location_load_json(self, req, schema):
"""Pull a json value from the request."""
if not is_json_request(req):
return core.missing

json_data = self._cache.get("json")
if json_data is None:
# We decode the json manually here instead of
Expand All @@ -73,23 +76,20 @@ def location_load_json(self, schema, req):
return core.missing
else:
return self.handle_invalid_json_error(e, req)

return json_data

def location_load_querystring(self, schema, req):
def location_load_querystring(self, req, schema):
"""Pull a querystring value from the request."""
return MultiDictProxy(req.args, schema)

def location_load_form(self, req, schema):
"""Pull a form value from the request."""
try:
return MultiDictProxy(req.form, schema)
except AttributeError:
pass
return core.missing
return MultiDictProxy(req.form, schema)

def location_load_headers(self, req, schema):
"""Pull a value from the header data."""
return req.headers
return MultiDictProxy(req.headers, schema)

def location_load_cookies(self, req, schema):
"""Pull a value from the cookiejar."""
Expand Down
1 change: 1 addition & 0 deletions src/webargs/multidictproxy.py
Expand Up @@ -11,6 +11,7 @@ class MultiDictProxy(Mapping):
In all other cases, __getitem__ proxies directly to the input multidict.
"""

def __init__(self, multidict, schema):
self.data = multidict
self.multiple_keys = self._list_multiple_keys(schema)
Expand Down
81 changes: 53 additions & 28 deletions src/webargs/testing.py
Expand Up @@ -40,48 +40,51 @@ def testapp(self):
def test_parse_querystring_args(self, testapp):
assert testapp.get("/echo?name=Fred").json == {"name": "Fred"}

def test_parse_querystring_with_query_location_specified(self, testapp):
assert testapp.get("/echo_query?name=Steve").json == {"name": "Steve"}

def test_parse_form(self, testapp):
assert testapp.post("/echo", {"name": "Joe"}).json == {"name": "Joe"}
assert testapp.post("/echo_form", {"name": "Joe"}).json == {"name": "Joe"}

def test_parse_json(self, testapp):
assert testapp.post_json("/echo", {"name": "Fred"}).json == {"name": "Fred"}
assert testapp.post_json("/echo_json", {"name": "Fred"}).json == {
"name": "Fred"
}

def test_parse_querystring_default(self, testapp):
assert testapp.get("/echo").json == {"name": "World"}

def test_parse_json_default(self, testapp):
assert testapp.post_json("/echo", {}).json == {"name": "World"}
assert testapp.post_json("/echo_json", {}).json == {"name": "World"}

def test_parse_json_with_charset(self, testapp):
res = testapp.post(
"/echo",
"/echo_json",
json.dumps({"name": "Steve"}),
content_type="application/json;charset=UTF-8",
)
assert res.json == {"name": "Steve"}

def test_parse_json_with_vendor_media_type(self, testapp):
res = testapp.post(
"/echo",
"/echo_json",
json.dumps({"name": "Steve"}),
content_type="application/vnd.api+json;charset=UTF-8",
)
assert res.json == {"name": "Steve"}

def test_parse_json_ignores_extra_data(self, testapp):
assert testapp.post_json("/echo", {"extra": "data"}).json == {"name": "World"}
assert testapp.post_json("/echo_json", {"extra": "data"}).json == {
"name": "World"
}

def test_parse_json_blank(self, testapp):
assert testapp.post_json("/echo", None).json == {"name": "World"}
def test_parse_json_empty(self, testapp):
assert testapp.post_json("/echo_json", {}).json == {"name": "World"}

def test_parse_json_ignore_unexpected_int(self, testapp):
assert testapp.post_json("/echo", 1).json == {"name": "World"}
def test_parse_json_error_unexpected_int(self, testapp):
res = testapp.post_json("/echo_json", 1, expect_errors=True)
assert res.status_code == 422

def test_parse_json_ignore_unexpected_list(self, testapp):
assert testapp.post_json("/echo", [{"extra": "data"}]).json == {"name": "World"}
def test_parse_json_error_unexpected_list(self, testapp):
res = testapp.post_json("/echo_json", [{"extra": "data"}], expect_errors=True)
assert res.status_code == 422

def test_parse_json_many_schema_invalid_input(self, testapp):
res = testapp.post_json(
Expand All @@ -93,11 +96,14 @@ def test_parse_json_many_schema(self, testapp):
res = testapp.post_json("/echo_many_schema", [{"name": "Steve"}]).json
assert res == [{"name": "Steve"}]

def test_parse_json_many_schema_ignore_malformed_data(self, testapp):
assert testapp.post_json("/echo_many_schema", {"extra": "data"}).json == []
def test_parse_json_many_schema_error_malformed_data(self, testapp):
res = testapp.post_json(
"/echo_many_schema", {"extra": "data"}, expect_errors=True
)
assert res.status_code == 422

def test_parsing_form_default(self, testapp):
assert testapp.post("/echo", {}).json == {"name": "World"}
assert testapp.post("/echo_form", {}).json == {"name": "World"}

def test_parse_querystring_multiple(self, testapp):
expected = {"name": ["steve", "Loria"]}
Expand All @@ -106,19 +112,28 @@ def test_parse_querystring_multiple(self, testapp):
def test_parse_form_multiple(self, testapp):
expected = {"name": ["steve", "Loria"]}
assert (
testapp.post("/echo_multi", {"name": ["steve", "Loria"]}).json == expected
testapp.post("/echo_multi_form", {"name": ["steve", "Loria"]}).json
== expected
)

def test_parse_json_list(self, testapp):
expected = {"name": ["Steve"]}
assert testapp.post_json("/echo_multi", {"name": "Steve"}).json == expected
assert (
testapp.post_json("/echo_multi_json", {"name": ["Steve"]}).json == expected
)

def test_parse_json_list_error_malformed_data(self, testapp):
res = testapp.post_json(
"/echo_multi_json", {"name": "Steve"}, expect_errors=True
)
assert res.status_code == 422

def test_parse_json_with_nonascii_chars(self, testapp):
text = u"øˆƒ£ºº∆ƒˆ∆"
assert testapp.post_json("/echo", {"name": text}).json == {"name": text}
assert testapp.post_json("/echo_json", {"name": text}).json == {"name": text}

def test_validation_error_returns_422_response(self, testapp):
res = testapp.post("/echo", {"name": "b"}, expect_errors=True)
res = testapp.post("/echo_json", {"name": "b"}, expect_errors=True)
assert res.status_code == 422

def test_user_validation_error_returns_422_response_by_default(self, testapp):
Expand Down Expand Up @@ -176,8 +191,8 @@ def test_parse_nested_many_missing(self, testapp):
assert res.json == {}

def test_parse_json_if_no_json(self, testapp):
res = testapp.post("/echo")
assert res.json == {"name": "World"}
res = testapp.post("/echo_json", expect_errors=True)
assert res.status_code == 422

def test_parse_files(self, testapp):
res = testapp.post(
Expand All @@ -186,19 +201,29 @@ def test_parse_files(self, testapp):
assert res.json == {"myfile": "data"}

# https://github.com/sloria/webargs/pull/297
#
# NOTE: although this was originally considered correct behavior for
# webargs under #297 , under the refactor for #419 it is being reconsidered
# as *incorrect*
# the reason is that it makes it impossible to disambiguate "" (not valid
# JSON) from "{}" (valid, empty JSON). This in turn has downstream negative
# impact when you try to build sophisticated nested loaders which traverse
# the various location_load_{locname} methods because you can't tell if the
# user submitted empty data or no data in a location. In particular, this
# would pose serious problems for implementing the "json_or_form" location
def test_empty_json(self, testapp):
res = testapp.post(
"/echo",
"/echo_json",
"",
headers={"Accept": "application/json", "Content-Type": "application/json"},
expect_errors=True,
)
assert res.status_code == 200
assert res.json == {"name": "World"}
assert res.status_code == 422

# https://github.com/sloria/webargs/issues/329
def test_invalid_json(self, testapp):
res = testapp.post(
"/echo",
"/echo_json",
'{"foo": "bar", }',
headers={"Accept": "application/json", "Content-Type": "application/json"},
expect_errors=True,
Expand Down
73 changes: 48 additions & 25 deletions tests/apps/flask_app.py
Expand Up @@ -3,77 +3,100 @@
from flask.views import MethodView

import marshmallow as ma
from webargs import fields
from webargs import fields, dict2schema
from webargs.flaskparser import parser, use_args, use_kwargs
from webargs.core import MARSHMALLOW_VERSION_INFO

if MARSHMALLOW_VERSION_INFO[0] < 3:
schema_kwargs = {"strict": True}
else:
schema_kwargs = {"unknown": ma.EXCLUDE}


class TestAppConfig:
TESTING = True


hello_args = {"name": fields.Str(missing="World", validate=lambda n: len(n) >= 3)}
hello_multiple = {"name": fields.List(fields.Str())}
hello_args = dict2schema(
{"name": fields.Str(missing="World", validate=lambda n: len(n) >= 3)}
)(**schema_kwargs)
hello_multiple = dict2schema({"name": fields.List(fields.Str())})(**schema_kwargs)


class HelloSchema(ma.Schema):
name = fields.Str(missing="World", validate=lambda n: len(n) >= 3)


strict_kwargs = {"strict": True} if MARSHMALLOW_VERSION_INFO[0] < 3 else {}
hello_many_schema = HelloSchema(many=True, **strict_kwargs)
hello_many_schema = HelloSchema(many=True, **schema_kwargs)

app = Flask(__name__)
app.config.from_object(TestAppConfig)


@app.route("/echo", methods=["GET", "POST"])
@app.route("/echo", methods=["GET"])
def echo():
return J(parser.parse(hello_args))
return J(parser.parse(hello_args, location="query"))


@app.route("/echo_form", methods=["POST"])
def echo_form():
return J(parser.parse(hello_args, location="form"))

@app.route("/echo_query")
def echo_query():
return J(parser.parse(hello_args, request, locations=("query",)))

@app.route("/echo_json", methods=["POST"])
def echo_json():
return J(parser.parse(hello_args))


@app.route("/echo_use_args", methods=["GET", "POST"])
@use_args(hello_args)
@app.route("/echo_use_args", methods=["GET"])
@use_args(hello_args, location="query")
def echo_use_args(args):
return J(args)


@app.route("/echo_use_args_validated", methods=["GET", "POST"])
@use_args({"value": fields.Int()}, validate=lambda args: args["value"] > 42)
@app.route("/echo_use_args_validated", methods=["POST"])
@use_args(
{"value": fields.Int()}, validate=lambda args: args["value"] > 42, location="form"
)
def echo_use_args_validated(args):
return J(args)


@app.route("/echo_use_kwargs", methods=["GET", "POST"])
@use_kwargs(hello_args)
@app.route("/echo_use_kwargs", methods=["GET"])
@use_kwargs(hello_args, location="query")
def echo_use_kwargs(name):
return J({"name": name})


@app.route("/echo_multi", methods=["GET", "POST"])
@app.route("/echo_multi", methods=["GET"])
def multi():
return J(parser.parse(hello_multiple, location="query"))


@app.route("/echo_multi_form", methods=["POST"])
def multi_form():
return J(parser.parse(hello_multiple, location="form"))


@app.route("/echo_multi_json", methods=["POST"])
def multi_json():
return J(parser.parse(hello_multiple))


@app.route("/echo_many_schema", methods=["GET", "POST"])
def many_nested():
arguments = parser.parse(hello_many_schema, locations=("json",))
arguments = parser.parse(hello_many_schema)
return Response(json.dumps(arguments), content_type="application/json")


@app.route("/echo_use_args_with_path_param/<name>")
@use_args({"value": fields.Int()})
@use_args({"value": fields.Int()}, location="query")
def echo_use_args_with_path(args, name):
return J(args)


@app.route("/echo_use_kwargs_with_path_param/<name>")
@use_kwargs({"value": fields.Int()})
@use_kwargs({"value": fields.Int()}, location="query")
def echo_use_kwargs_with_path(name, value):
return J({"value": value})

Expand All @@ -89,30 +112,30 @@ def always_fail(value):

@app.route("/echo_headers")
def echo_headers():
return J(parser.parse(hello_args, locations=("headers",)))
return J(parser.parse(hello_args, location="headers"))


@app.route("/echo_cookie")
def echo_cookie():
return J(parser.parse(hello_args, request, locations=("cookies",)))
return J(parser.parse(hello_args, request, location="cookies"))


@app.route("/echo_file", methods=["POST"])
def echo_file():
args = {"myfile": fields.Field()}
result = parser.parse(args, locations=("files",))
result = parser.parse(args, location="files")
fp = result["myfile"]
content = fp.read().decode("utf8")
return J({"myfile": content})


@app.route("/echo_view_arg/<view_arg>")
def echo_view_arg(view_arg):
return J(parser.parse({"view_arg": fields.Int()}, locations=("view_args",)))
return J(parser.parse({"view_arg": fields.Int()}, location="view_args"))


@app.route("/echo_view_arg_use_args/<view_arg>")
@use_args({"view_arg": fields.Int(location="view_args")})
@use_args({"view_arg": fields.Int()}, location="view_args")
def echo_view_arg_with_use_args(args, **kwargs):
return J(args)

Expand Down

0 comments on commit 849550b

Please sign in to comment.