jowilf / starlette-admin

Fast, beautiful and extensible administrative interface framework for Starlette & FastApi applications
https://jowilf.github.io/starlette-admin/
MIT License
521 stars 55 forks source link

Bug: UUID pk gives JSON serialization error when excluded from the list #552

Open alg opened 1 week ago

alg commented 1 week ago

Describe the bug When I have a model with the pk (id) field of UUID type and include it in the list, it works fine. But if I exclude it, while leaving among the fields, it gives an error:

  File "/Users/alg/p/template-fastapi/.venv/lib/python3.11/site-packages/starlette/responses.py", line 187, in render
    return json.dumps(
           ^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/__init__.py", line 238, in dumps
    **kw).encode(obj)
          ^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 258, in iterencode
    return _iterencode(o, 0)
           ^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.9/Frameworks/Python.framework/Versions/3.11/lib/python3.11/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type UUID is not JSON serializable

To Reproduce

Environment (please complete the following information):

barbarrista commented 5 days ago

Hi! It's works for me

class UuidSerializableModelView(ModelView):
    async def get_pk_value(
        self,
        request: Request,
        obj: Any,  # noqa: ANN401
    ) -> Any:  # noqa: ANN401
        pk = await super().get_pk_value(request, obj)
        if isinstance(pk, UUID):
            return str(pk)
        return pk

class MyView(UuidSerializableModelView):

    fields: Sequence[BaseField] = [
        StringField("id", label="ID"),
  ]
alg commented 5 days ago

@barbarrista I took a different approach, which I think is safer since It looks like get_pk_value is supposed to return anything that is pk as is (not a string representation of UUID).

In my view subclass I override BaseView.serialize like this:

    @override
    async def serialize(
        self,
        obj: Any,
        request: Request,
        action: RequestAction,
        include_relationships: bool = True,
        include_select2: bool = False,
    ) -> dict[str, Any]:
        data = await super().serialize(obj, request, action, include_relationships, include_select2)

        # Fixing non-serializable PK
        if self.pk_attr and self.pk_attr in data:
            pk_value = data[self.pk_attr]
            if isinstance(pk_value, UUID):
                data[self.pk_attr] = str(pk_value)

        return data

It could be extracted in a class as you suggested. Just wondering if pk value should be passed through some method when forced in the original serialize method so its serialization easier to configure and extend.

jowilf commented 5 days ago

Both approaches are good workarounds but we can solve this by using the serialized pk value instead of the raw pk value in the serialize method. One way to achieve this is by adding another abstract method get_serialized_pk_value in the BaseView and then using self.pk_field.serialize_value for sqla implementation.

Feel free to submit a PR.

alg commented 5 days ago

@jowilf That's what I was thinking. I'm on it.

jowilf commented 4 days ago

@alg great, thanks