litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.64k stars 382 forks source link

Enhancement: Add more types to the default `Response` serializer. #458

Closed cofin closed 2 years ago

cofin commented 2 years ago

It looks like most of the project templates we have floating around implement a custom serializer for Responses. We should consider enhancing the built in to reduce the need for this.

For instance, here is the current Response.serializer:

    @staticmethod
    def serializer(value: Any) -> Dict[str, Any]:
        """Serializer hook for orjson to handle pydantic models.

        This method can be overridden to extend json serialization.

        Args:
            value: The value to be serialized

        Returns:
            A string keyed dictionary of json compatible values
        """
        if isinstance(value, BaseModel):
            return value.dict()
        raise TypeError  # pragma: no cover

and here is one that's used on another project:

@staticmethod
    def serializer(value: Any) -> Dict[str, Any]:
        """Serializer hook for orjson to handle pydantic models.

        Args:
            value: The value to be serialized

        Returns:
            A string keyed dictionary of json compatible values
        """
        if isinstance(value, Enum):
            return value.value
        if isinstance(value, EnumMeta):
            return None
        if isinstance(value, SecretStr):
            return value.get_secret_value()
        if isinstance(value, UUID):
            return str(value)
        return starlite.Response.serializer(value)

Thoughts?

Goldziher commented 2 years ago

@cofin - This is a good idea. Please make sure to add tests and verify that all of these are in fact needed. I would assume that UUID is json serializable because it implements __str__, and thus it wouldnt be required. I am also not sure about Enum in the above.

I would suggest we move the default serializer out of the response class and into a serializers package or into file in util. It can be reused in other places

cofin commented 2 years ago

Excellent. Are there others that we should add in?

Goldziher commented 2 years ago

I would assume that all non-json serializable builtins not handled by orjson. You can checkout this list of types handled by pydantic-factories and supported by pydantic as a reference: https://github.com/Goldziher/pydantic-factories/blob/main/pydantic_factories/factory.py#L242

Another note-> please make sure to put the pydantic serializer as the first if clause -> it will ensure performance is pretty good.