From 21f1c4a849db86d402ee6d8cd83f299bc673ba24 Mon Sep 17 00:00:00 2001 From: Alexander Date: Thu, 19 Dec 2024 14:18:54 +0100 Subject: [PATCH] cleanup responses (#460) - Cleanup responses - Move transform speedup in mixin --- docs/en/docs/release-notes.md | 14 +++++ docs/en/docs/responses.md | 6 +- esmerald/responses/base.py | 101 ++++++++++++++------------------- esmerald/responses/encoders.py | 41 ++++++------- esmerald/responses/json.py | 21 +------ esmerald/responses/mixins.py | 30 ++++++++++ pyproject.toml | 4 +- tests/test_responses.py | 48 ++++++++++++++++ 8 files changed, 158 insertions(+), 107 deletions(-) create mode 100644 esmerald/responses/mixins.py diff --git a/docs/en/docs/release-notes.md b/docs/en/docs/release-notes.md index 728fca4d..fd1601e5 100644 --- a/docs/en/docs/release-notes.md +++ b/docs/en/docs/release-notes.md @@ -5,6 +5,20 @@ hide: # Release Notes +## 3.6.2 + +### Changed + +- Cleanup Response. +- Move `transform` method to lilya but provide speedup in a mixin. +- Esmerald `Response` behaves like `make_response` in lilya with a plain `Response`. +- Special handle None (nothing is returned) in `Response`. It shouldn't map to `null` so not all handlers have to return a value. + + +### Fixed + +- `bytes` won't be encoded as json when returned from a handler. This would unexpectly lead to a base64 encoding. + ## 3.6.1 ### Added diff --git a/docs/en/docs/responses.md b/docs/en/docs/responses.md index fc3cf91e..bddeadd0 100644 --- a/docs/en/docs/responses.md +++ b/docs/en/docs/responses.md @@ -39,9 +39,9 @@ This will allow you to use the [ORJSON](#orjson) and [UJSON](#ujson) as well as ### Response -Classic and generic `Response` that fits almost every single use case out there. It behaves like lilya `Response` by default. -It has a special mode for `media_type="application/json"` (used by handlers) in which it behaves like `make_response` (encodes content). -This special mode is required for encoding data like datetimes, ... +Classic and generic `Response` that fits almost every single use case out there. It behaves like lilya `make_response` with a plain `Response` by default. +It has a special mode for `media_type="application/json"` (used by handlers) in which it behaves like `make_response` with a plain `JSONResponse`. +The special mode has an exception for strings, so you can return in handlers a string object without having it jsonified. ```python {!> ../../../docs_src/responses/response.py !} diff --git a/esmerald/responses/base.py b/esmerald/responses/base.py index 042d2c28..30422553 100644 --- a/esmerald/responses/base.py +++ b/esmerald/responses/base.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from functools import partial from typing import ( TYPE_CHECKING, @@ -12,8 +14,10 @@ cast, ) +import orjson from lilya import status from lilya.responses import ( + RESPONSE_TRANSFORM_KWARGS, Error as Error, FileResponse as FileResponse, # noqa HTMLResponse as HTMLResponse, # noqa @@ -24,13 +28,14 @@ Response as LilyaResponse, # noqa StreamingResponse as StreamingResponse, # noqa ) -from orjson import OPT_OMIT_MICROSECONDS, OPT_SERIALIZE_NUMPY, dumps, loads from typing_extensions import Annotated, Doc -from esmerald.encoders import LILYA_ENCODER_TYPES, Encoder, json_encoder +from esmerald.encoders import Encoder from esmerald.enums import MediaType from esmerald.exceptions import ImproperlyConfigured +from .mixins import ORJSONTransformMixin + PlainTextResponse = PlainText if TYPE_CHECKING: # pragma: no cover @@ -40,7 +45,7 @@ T = TypeVar("T") -class Response(LilyaResponse, Generic[T]): +class Response(ORJSONTransformMixin, LilyaResponse, Generic[T]): """ Default `Response` object from Esmerald where it can be as the return annotation of a [handler](https://esmerald.dev/routing/handlers/). @@ -98,7 +103,7 @@ def __init__( ), ] = None, background: Annotated[ - Optional[Union["BackgroundTask", "BackgroundTasks"]], + Optional[Union[BackgroundTask, BackgroundTasks]], Doc( """ Any instance of a [BackgroundTask or BackgroundTasks](https://esmerald.dev/background-tasks/). @@ -114,7 +119,7 @@ def __init__( ), ] = None, cookies: Annotated[ - Optional["ResponseCookies"], + Optional[ResponseCookies], Doc( """ A sequence of `esmerald.datastructures.Cookie` objects. @@ -174,62 +179,42 @@ def __init__( ) self.cookies = cookies or [] - @staticmethod - def transform( - value: Any, *, encoders: Union[Sequence[Encoder], None] = None - ) -> Dict[str, Any]: - """ - The transformation of the data being returned. - - Supports all the default encoders from Lilya and custom from Esmerald. - """ - return cast( - Dict[str, Any], - json_encoder( - value, - json_encode_fn=partial(dumps, option=OPT_SERIALIZE_NUMPY | OPT_OMIT_MICROSECONDS), - post_transform_fn=loads, - with_encoders=encoders, - ), - ) - - def make_response(self, content: Any) -> Union[bytes, str]: - # here we need lilyas encoders not only esmerald encoders - # in case no extra encoders are defined use the default by passing None - encoders = ( - ( - ( - *self.encoders, - *LILYA_ENCODER_TYPES.get(), - ) + def make_response(self, content: Any) -> bytes | memoryview | str: + if ( + content is None + or content is NoReturn + and ( + self.status_code < 100 + or self.status_code in {status.HTTP_204_NO_CONTENT, status.HTTP_304_NOT_MODIFIED} ) - if self.encoders - else None + ): + return b"" + transform_kwargs = RESPONSE_TRANSFORM_KWARGS.get() + if transform_kwargs: + transform_kwargs = transform_kwargs.copy() + else: + transform_kwargs = {} + transform_kwargs.setdefault( + "json_encode_fn", + partial( + orjson.dumps, + option=orjson.OPT_SERIALIZE_NUMPY | orjson.OPT_OMIT_MICROSECONDS, + ), ) try: - if ( - content is None - or content is NoReturn - and ( - self.status_code < 100 - or self.status_code - in {status.HTTP_204_NO_CONTENT, status.HTTP_304_NOT_MODIFIED} - ) - ): - return b"" - # switch to a special mode for MediaType.JSON + # switch to a special mode for MediaType.JSON (default handlers) if self.media_type == MediaType.JSON: - return cast( - bytes, - json_encoder( - content, - json_encode_fn=partial( - dumps, option=OPT_SERIALIZE_NUMPY | OPT_OMIT_MICROSECONDS - ), - post_transform_fn=None, - with_encoders=encoders, - ), - ) - return super().make_response(content) + # "" should serialize to json + if content == "": + return b'""' + # keep it a serialized json object + transform_kwargs.setdefault("post_transform_fn", None) + else: + # strip '"' + transform_kwargs.setdefault("post_transform_fn", lambda x: x.strip(b'"')) + with self.with_transform_kwargs(transform_kwargs): + # if content is bytes it won't be transformed and + # if None or NoReturn, return b"", this differs from the dedicated JSONResponses. + return super().make_response(content) except (AttributeError, ValueError, TypeError) as e: # pragma: no cover raise ImproperlyConfigured("Unable to serialize response content") from e diff --git a/esmerald/responses/encoders.py b/esmerald/responses/encoders.py index 011ae03d..2928dd21 100644 --- a/esmerald/responses/encoders.py +++ b/esmerald/responses/encoders.py @@ -1,10 +1,11 @@ from functools import partial -from typing import Any, cast +from typing import Any import orjson +from lilya.responses import RESPONSE_TRANSFORM_KWARGS -from esmerald.encoders import LILYA_ENCODER_TYPES, json_encoder -from esmerald.responses.json import BaseJSONResponse +from .json import BaseJSONResponse +from .mixins import ORJSONTransformMixin try: import ujson @@ -12,7 +13,7 @@ ujson = None -class ORJSONResponse(BaseJSONResponse): +class ORJSONResponse(ORJSONTransformMixin, BaseJSONResponse): """ An alternative to `JSONResponse` and performance wise, faster. @@ -20,28 +21,20 @@ class ORJSONResponse(BaseJSONResponse): """ def make_response(self, content: Any) -> bytes: - # here we need lilyas encoders not only esmerald encoders - encoders = ( - ( - ( - *self.encoders, - *LILYA_ENCODER_TYPES.get(), - ) - ) - if self.encoders - else None - ) - return cast( - bytes, - json_encoder( - content, - json_encode_fn=partial( - orjson.dumps, option=orjson.OPT_SERIALIZE_NUMPY | orjson.OPT_OMIT_MICROSECONDS - ), - post_transform_fn=None, - with_encoders=encoders, + new_params = RESPONSE_TRANSFORM_KWARGS.get() + if new_params: + new_params = new_params.copy() + else: + new_params = {} + new_params.setdefault( + "json_encode_fn", + partial( + orjson.dumps, + option=orjson.OPT_SERIALIZE_NUMPY | orjson.OPT_OMIT_MICROSECONDS, ), ) + with self.with_transform_kwargs(new_params): + return super().make_response(content) class UJSONResponse(BaseJSONResponse): diff --git a/esmerald/responses/json.py b/esmerald/responses/json.py index 2b7a7df0..0eb661f0 100644 --- a/esmerald/responses/json.py +++ b/esmerald/responses/json.py @@ -1,22 +1,3 @@ -from typing import Any, Dict, cast - -from lilya.responses import JSONResponse - -from esmerald.encoders import json_encoder - - -class BaseJSONResponse(JSONResponse): - """ - Making sure it parses all the values from pydantic into dictionary. - """ - - @staticmethod - def transform(value: Any) -> Dict[str, Any]: # pragma: no cover - """ - Makes sure that every value is checked and if it's a pydantic model then parses into - a dict(). - """ - return cast(Dict[str, Any], json_encoder(value)) - +from lilya.responses import JSONResponse as BaseJSONResponse __all__ = ["BaseJSONResponse"] diff --git a/esmerald/responses/mixins.py b/esmerald/responses/mixins.py new file mode 100644 index 00000000..00ef856a --- /dev/null +++ b/esmerald/responses/mixins.py @@ -0,0 +1,30 @@ +from functools import partial +from typing import Any + +import orjson +from lilya.responses import RESPONSE_TRANSFORM_KWARGS + + +class ORJSONTransformMixin: + @classmethod + def transform(cls, value: Any) -> Any: + """ + The transformation of the data being returned (simplify operation). + + Supports all the default encoders from Lilya and custom from Esmerald. + """ + transform_kwargs = RESPONSE_TRANSFORM_KWARGS.get() + if transform_kwargs is None: + transform_kwargs = {} + else: + transform_kwargs.copy() + transform_kwargs.setdefault( + "json_encode_fn", + partial( + orjson.dumps, option=orjson.OPT_SERIALIZE_NUMPY | orjson.OPT_OMIT_MICROSECONDS + ), + ) + transform_kwargs.setdefault("post_transform_fn", orjson.loads) + + with cls.with_transform_kwargs(transform_kwargs): + return super().transform(value) # type: ignore diff --git a/pyproject.toml b/pyproject.toml index b1a61876..c03cddd4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,7 +41,7 @@ dependencies = [ "email-validator >=2.2.0,<3.0.0", "itsdangerous>=2.1.2,<3.0.0", "jinja2>=3.1.2,<4.0.0", - "lilya>=0.11.6", + "lilya>=0.11.8", "loguru>=0.7.0,<0.8.0", "pydantic>=2.10,<3.0.0", "pydantic-settings>=2.0.0,<3.0.0", @@ -103,7 +103,7 @@ testing = [ "ujson>=5.7.0,<6", "anyio[trio]>=3.6.2,<5.0.0", "brotli>=1.0.9,<2.0.0", - "edgy[postgres]>=0.23.3", + "edgy[postgres]>=0.24.1", "databasez>=0.9.7", "flask>=1.1.2,<4.0.0", "freezegun>=1.2.2,<2.0.0", diff --git a/tests/test_responses.py b/tests/test_responses.py index 6b7bc083..273cd454 100644 --- a/tests/test_responses.py +++ b/tests/test_responses.py @@ -34,6 +34,26 @@ def route_five() -> Response: return Response("Ok") +@get("/six") +def route_six() -> bytes: + return b"Ok" + + +@get("/seven") +def route_seven() -> str: + return "" + + +@get("/eight") +def route_eight() -> str: + return "hello" + + +@get("/nine") +def route_nine() -> None: + pass + + def test_ujson_response(test_client_factory): with create_client(routes=[Gateway(handler=route_one)]) as client: response = client.get("/one") @@ -72,6 +92,34 @@ def test_default_decorator(test_client_factory): assert response.status_code == status.HTTP_207_MULTI_STATUS +def test_implicit_bytes_returnal(test_client_factory): + with create_client(routes=[route_six]) as client: + response = client.get("/six") + + assert response.text == "Ok" + + +def test_implicit_empty_str_returnal(test_client_factory): + with create_client(routes=[route_seven]) as client: + response = client.get("/seven") + + assert response.text == '""' + + +def test_str_returnal(test_client_factory): + with create_client(routes=[route_eight]) as client: + response = client.get("/eight") + + assert response.text == '"hello"' + + +def test_implicit_none_returnal(test_client_factory): + with create_client(routes=[route_nine]) as client: + response = client.get("/nine") + + assert response.text == "" + + @get(status_code=status.HTTP_207_MULTI_STATUS) def multiple(name: Union[str, None]) -> Response: if name is None: