Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Using plugin results in DTO given to dto parameter not being used for return #3460

Open
1 of 4 tasks
ryanolf-tb opened this issue May 2, 2024 · 2 comments
Open
1 of 4 tasks
Labels
Bug 🐛 This is something that is not working as expected

Comments

@ryanolf-tb
Copy link

ryanolf-tb commented May 2, 2024

Description

Based on the docs, I expect a DTO given to a handler via the dto parameter to apply to the return value when no return_dto is specified. However, when the SQLALchemyPlugin is used this is not the case.

More generally, it looks like the return_dto resolution order in the BaseRouteHandler puts generated DTOs from serialization plugins ahead of DTOs from explicit dto parameters on the handler. For me this is not the expected behavior based on the documentation, though I can see that one may want to opt-in to the return_dto provided by the plugin while still providing a data DTO, which perhaps is the reason for the current behavior.

It may be that the docs just need an update to clarify? An alternative might be to specify a particular value that can be passed to the return_dto parameter to maintain the current behavior, but to otherwise resolve to dto if it's given.

URL to code causing the issue

No response

MCVE

from typing import Annotated

from advanced_alchemy.extensions.litestar.plugins.init.config.engine import EngineConfig
from litestar import Litestar, get
from litestar.contrib.sqlalchemy.dto import SQLAlchemyDTO
from litestar.contrib.sqlalchemy.plugins import (
    SQLAlchemyPlugin,
    SQLAlchemySyncConfig,
)
from litestar.dto import DTOConfig
from litestar.testing import TestClient
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column


class Base(DeclarativeBase):
    pass


class Address(Base):
    __tablename__ = "addresses"

    id: Mapped[int] = mapped_column(primary_key=True)
    street: Mapped[str]
    city: Mapped[str]
    state: Mapped[str]
    zip: Mapped[str]


db_config = SQLAlchemySyncConfig(
    connection_string="sqlite://",
    engine_config=EngineConfig(echo=False),
    create_all=True,
)


AddressDTO = SQLAlchemyDTO[
    Annotated[
        Address,
        DTOConfig(exclude={"zip"}),
    ]
]


@get("/address", dto=AddressDTO, sync_to_thread=False)
def get_address() -> Address:
    return Address(id=1, street="123 Main St", city="Anytown", state="NY", zip="12345")


app_with_plugin = Litestar(
    route_handlers=[get_address],
    plugins=[SQLAlchemyPlugin(config=db_config)],
)

app_without_plugin = Litestar(
    route_handlers=[get_address],
)

with TestClient(app=app_without_plugin) as client:
    if "zip" not in client.get("/address").json():
        print("'zip' is properly excluded without the plugin")

with TestClient(app=app_with_plugin) as client:
    if "zip" in client.get("/address").json():
        print("'zip' is not excluded with the plugin")

Steps to reproduce

1. Run the MCVE
2. Note that 'zip' is not properly excluded from the response JSON when the `SQLALchemyPlugin` is used.

Screenshots

"![SCREENSHOT_DESCRIPTION](SCREENSHOT_LINK.png)"

Logs

'zip' is properly excluded without the plugin
INFO - 2024-05-02 11:27:19,957 - httpx - _client - HTTP Request: GET http://testserver.local/address "HTTP/1.1 200 OK"
'zip' is not excluded with the plugin
INFO - 2024-05-02 11:27:19,968 - httpx - _client - HTTP Request: GET http://testserver.local/address "HTTP/1.1 200 OK"

Litestar Version

2.8.2

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)

Note

While we are open for sponsoring on GitHub Sponsors and
OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.
Fund with Polar
@ryanolf-tb ryanolf-tb added the Bug 🐛 This is something that is not working as expected label May 2, 2024
@Alc-Alc
Copy link
Contributor

Alc-Alc commented May 2, 2024

Not dismissing the behavior, but just pointing this out zip: Mapped[str] = mapped_column(info=dto_field('private')) can be used to ignore it via the plugin

dto_field is litestar.dto.dto_field

@ryanolf-tb
Copy link
Author

ryanolf-tb commented May 2, 2024

I think in the typical CRUD use case one would use dto_field. In my example, I can just use return_dto instead of dto. This may not have been brought up before because it is not encountered much in practice, and it's easy to work around if one understands the behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 This is something that is not working as expected
Projects
None yet
Development

No branches or pull requests

2 participants