From 9ea168bd6a2f7766808333779a3a86f01668b6a2 Mon Sep 17 00:00:00 2001 From: Ultrasonic1209 <44583181+Ultrasonic1209@users.noreply.github.com> Date: Sun, 4 Sep 2022 10:50:58 +0000 Subject: [PATCH 1/5] Validate falsey values correctly --- sanic_ext/extras/validation/check.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sanic_ext/extras/validation/check.py b/sanic_ext/extras/validation/check.py index 42dc7bc..dacd672 100644 --- a/sanic_ext/extras/validation/check.py +++ b/sanic_ext/extras/validation/check.py @@ -3,6 +3,7 @@ from dataclasses import _HAS_DEFAULT_FACTORY # type: ignore from typing import Any, Literal, NamedTuple, Optional, Tuple, Union, get_args +from sanic.utils import str_to_bool from sanic_ext.utils.typing import UnionType, is_generic, is_optional MISSING: Tuple[Any, ...] = (_HAS_DEFAULT_FACTORY,) @@ -133,6 +134,8 @@ def coerce_type(self): coerce_type = self.hint if is_optional(coerce_type): coerce_type = get_args(self.hint)[0] + elif coerce_type is bool: + coerce_type = str_to_bool return coerce_type From 289d4f7cf9bcbb5756330a22c640d874b8179ef9 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 4 Sep 2022 14:21:45 +0300 Subject: [PATCH 2/5] Add test for boolean coercing --- tests/extra/test_validation_dataclass.py | 42 +++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/extra/test_validation_dataclass.py b/tests/extra/test_validation_dataclass.py index cd7778f..995ddc4 100644 --- a/tests/extra/test_validation_dataclass.py +++ b/tests/extra/test_validation_dataclass.py @@ -295,7 +295,7 @@ def test_modeling_union_type_ModelUnionTypeStrInt(): check_data(models.ModelUnionTypeStrInt, {"foo": 1.1}, schema) -def test_validate_decorator(app): +def test_validate_decorator_json(app): @dataclass class Pet: name: str @@ -318,3 +318,43 @@ async def post(self, _, body: Pet): _, response = app.test_client.post("/method", json={"name": "Snoopy"}) assert response.status == 200 assert response.json["is_pet"] + + +@pytest.mark.parametrize( + "query_value,expected", + ( + ("true", True), + ("True", True), + ("1", True), + ("Yes", True), + ("y", True), + ("false", False), + ("False", False), + ("0", False), + ("No", False), + ("n", False), + ), +) +def test_validate_decorator_query(app, query_value, expected): + @dataclass + class Query: + flag: bool + + @app.get("/function") + @validate(query=Query) + async def handler(_, query: Query): + return json({"is_flagged": query.flag}) + + class MethodView(HTTPMethodView, attach=app, uri="/method"): + decorators = [validate(query=Query)] + + async def get(self, _, query: Query): + return json({"is_flagged": query.flag}) + + _, response = app.test_client.get(f"/function?flag={query_value}") + assert response.status == 200 + assert response.json["is_flagged"] is expected + + _, response = app.test_client.get(f"/method?flag={query_value}") + assert response.status == 200 + assert response.json["is_flagged"] is expected From a148a64c22a6e9efaa882bf012ec381172134d67 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 4 Sep 2022 14:26:43 +0300 Subject: [PATCH 3/5] Make pretty --- sanic_ext/extras/validation/check.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sanic_ext/extras/validation/check.py b/sanic_ext/extras/validation/check.py index dacd672..e1d287c 100644 --- a/sanic_ext/extras/validation/check.py +++ b/sanic_ext/extras/validation/check.py @@ -4,6 +4,7 @@ from typing import Any, Literal, NamedTuple, Optional, Tuple, Union, get_args from sanic.utils import str_to_bool + from sanic_ext.utils.typing import UnionType, is_generic, is_optional MISSING: Tuple[Any, ...] = (_HAS_DEFAULT_FACTORY,) From 08a089dd31bce9a000318765c76661455f9c5bdc Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 4 Sep 2022 14:28:08 +0300 Subject: [PATCH 4/5] Allow for optional bool --- sanic_ext/extras/validation/check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sanic_ext/extras/validation/check.py b/sanic_ext/extras/validation/check.py index e1d287c..b2eaaaf 100644 --- a/sanic_ext/extras/validation/check.py +++ b/sanic_ext/extras/validation/check.py @@ -135,7 +135,7 @@ def coerce_type(self): coerce_type = self.hint if is_optional(coerce_type): coerce_type = get_args(self.hint)[0] - elif coerce_type is bool: + if coerce_type is bool: coerce_type = str_to_bool return coerce_type From e44badb0815fe30481c206b65caf59c339cb21a5 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 4 Sep 2022 15:45:08 +0300 Subject: [PATCH 5/5] Allow for optional bool --- sanic_ext/extras/validation/check.py | 60 ++++++++++++++++++++---- tests/extra/test_validation_dataclass.py | 25 ++++++++++ 2 files changed, 76 insertions(+), 9 deletions(-) diff --git a/sanic_ext/extras/validation/check.py b/sanic_ext/extras/validation/check.py index b2eaaaf..e35395a 100644 --- a/sanic_ext/extras/validation/check.py +++ b/sanic_ext/extras/validation/check.py @@ -68,6 +68,8 @@ def validate( else: raise e else: + if allow_coerce: + value = self.coerce(value, allow_multiple) value = _check_nullability( value, self.nullable, @@ -105,12 +107,9 @@ def validate( allow_coerce, ) - if allow_coerce: - value = self.coerce(value) - return value - def coerce(self, value): + def coerce(self, value, allow_multiple=False): if is_generic(self.coerce_type): args = get_args(self.coerce_type) if type(None) in args and value is None: @@ -118,16 +117,27 @@ def coerce(self, value): coerce_types = [arg for arg in args if not isinstance(None, arg)] else: coerce_types = [self.coerce_type] + + if self.nullable: + value = self._do_coerce(value, str_to_none) + if value is None or ( + isinstance(value, list) + and allow_multiple + and all(val is None for val in value) + ): + return value + for coerce_type in coerce_types: + if isinstance(value, coerce_type): + return value + coercer = self.get_coercer(coerce_type) try: - if isinstance(value, list): - value = [coerce_type(item) for item in value] - else: - value = coerce_type(value) + value = self._do_coerce(value, coercer, raise_exception=True) except (ValueError, TypeError): ... else: return value + return value @property @@ -135,11 +145,33 @@ def coerce_type(self): coerce_type = self.hint if is_optional(coerce_type): coerce_type = get_args(self.hint)[0] + return coerce_type + + @staticmethod + def _do_coerce(value, coercer, raise_exception=False): + try: + if isinstance(value, list): + value = [coercer(item) for item in value] + else: + value = coercer(value) + except (ValueError, TypeError): + if raise_exception: + raise + return value + + @staticmethod + def get_coercer(coerce_type): if coerce_type is bool: coerce_type = str_to_bool return coerce_type +def str_to_none(value: Optional[str]): + if value is None or value.lower() in ("null", "none", ""): + return None + raise ValueError(f"Could not convert {value} to None") + + def check_data(model, data, schema, allow_multiple=False, allow_coerce=False): if not isinstance(data, dict): raise TypeError(f"Value '{data}' is not a dict") @@ -186,7 +218,17 @@ def _check_nullability( ): if not nullable and value is None: raise ValueError("Value cannot be None") - if nullable and value is not None: + if ( + nullable + and (value is not None) + and ( + allow_multiple is False + or ( + isinstance(value, list) + and any(val is not None for val in value) + ) + ) + ): exc = None for hint in allowed: try: diff --git a/tests/extra/test_validation_dataclass.py b/tests/extra/test_validation_dataclass.py index 995ddc4..6adb289 100644 --- a/tests/extra/test_validation_dataclass.py +++ b/tests/extra/test_validation_dataclass.py @@ -358,3 +358,28 @@ async def get(self, _, query: Query): _, response = app.test_client.get(f"/method?flag={query_value}") assert response.status == 200 assert response.json["is_flagged"] is expected + + +@pytest.mark.parametrize( + "query,expected", + ( + ("?maybe=true", True), + ("?maybe=false", False), + ("?maybe=null", None), + ("?maybe=None", None), + ("?maybe=", None), + ("", None), + ), +) +def test_validate_decorator_query_optional_bool(app, query, expected): + @dataclass + class Query: + maybe: Optional[bool] = None + + @app.get("/") + @validate(query=Query) + async def handler(_, query: Query): + return json({"is_flagged": query.maybe}) + + _, response = app.test_client.get(f"/{query}") + assert response.json["is_flagged"] is expected