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

Enhancement: Provide option to set DTOs for nested fields #3500

Open
aranvir opened this issue May 16, 2024 · 7 comments
Open

Enhancement: Provide option to set DTOs for nested fields #3500

aranvir opened this issue May 16, 2024 · 7 comments
Labels
Enhancement This is a new feature or request

Comments

@aranvir
Copy link
Contributor

aranvir commented May 16, 2024

Summary

I really enjoy working with DTOs as it simplifies my work greatly.

One weakness I've observed is when it comes to nested models, e.g., from including a relationship. I know it's possible to exclude fields of nested fields as explained in the docs, but I think it would be more convenient if one could provide "nested field DTOs". For example, this could be a dict where field names are mapped to DTOs to use for handling the model.

From user perspective this seems cleaner - but I don't know how easy it is to implement this.

Basic Example

Based on the example in the docs:

New

...

user_address_config = DTOConfig(
    exclude={
        "id",
        "street",
    }
)
UserAddressDTO = SQLAlchemyDTO[Annotated[Address, user_address_config]]


user_pets_config = DTOConfig(
    exclude={
        "id",
        "user_id",
    }
)
UserPetsDTO = SQLAlchemyDTO[Annotated[Pets, user_pets_config]]

UserDTO = SQLAlchemyDTO[User]
config = DTOConfig(
    exclude={
        "id",
    },
    field_dto={
        "address": UserAddressDTO,
        "pets": UserPetsDTO
    }
)
ReadUserDTO = SQLAlchemyDTO[Annotated[User, config]]
...

Currently

...
UserDTO = SQLAlchemyDTO[User]
config = DTOConfig(
    exclude={
        "id",
        "address.id",
        "address.street",
        "pets.0.id",
        "pets.0.user_id",
    }
)
ReadUserDTO = SQLAlchemyDTO[Annotated[User, config]]
...

Drawbacks and Impact

In this specific example, the current way is quicker to write and needs less lines. However, as an application becomes more complex there might be the use case where multiple models have a nested field (e.g., a user from an author reference) and would like to use the same nesting specification. Then, re-usability of a nested DTO would be preferably over managing the same exclude list over multiple DTOs.

Unresolved questions

No response


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
@aranvir aranvir added the Enhancement This is a new feature or request label May 16, 2024
@peterschutt
Copy link
Contributor

What about supplying config objects, instead of full-blown DTOs?

config = DTOConfig(
    exclude={
        "id",
    },
    field_dto={
        "address": user_address_config,
        "pets": user_pets_config,
    }
)
ReadUserDTO = SQLAlchemyDTO[Annotated[User, config]]

@peterschutt
Copy link
Contributor

peterschutt commented May 18, 2024

Here's some questions:

  • what should happen if the rename_strategy for the nested model is different to the parent model config?
  • what about max_nested_depth? If the parent config has max_nested_depth=1 and the nested model has max_nested_depth=1, should a nested field of the nested model be included? Or, should the parent nested depth win?
  • partial, would you expect a non-partial parent model should be allowed to contain a partial inner model?

@aranvir
Copy link
Contributor Author

aranvir commented May 18, 2024

That... are all good questions :D so, using configs instead of full DTO makes sense to me - in the end, the root DTO already takes care of processing any nested models it's just about sharing configs for nested models in an easy way.

For the other things it should be one general approach that is consistent - in my opinion: Parent overules child. The parent structure is used for handling communication, so it owns the interface description and should have last say in renaming and nesting and so on.

I'm not sure if I understand the last question: With partial you mean excluding fields? If so, then I'd say that is the point of having nested configs, no? To have a cleaner way of excluding fields per model instead of digging through all nested models from the parent DTO.

@peterschutt
Copy link
Contributor

For the other things it should be one general approach that is consistent - in my opinion: Parent overules child. The parent structure is used for handling communication, so it owns the interface description and should have last say in renaming and nesting and so on.

Yeah, so it would just read the exclude, include, and any explicit field renames from the nested config objects.

I'm not sure if I understand the last question: With partial you mean excluding fields? If so, then I'd say that is the point of having nested configs, no? To have a cleaner way of excluding fields per model instead of digging through all nested models from the parent DTO.

partial makes the set of all fields not required for a model, basically for PATCH route handling.

@aranvir
Copy link
Contributor Author

aranvir commented May 18, 2024

Ah okay, so yea then for partial I'd also say parent overrules child config.

@peterschutt
Copy link
Contributor

So I think how this would work then, is inside DTOConfig.__post_init__(), we'd take any of the nested config objects and build the dotted path include/exclude/renames into the parent configs include/exclude/rename fields. E.g., if parent had exclude={"foo"}, and the nested was {"bar": DTOConfig(exclude={"baz"})}, then the parent exclude would end up as {"foo", "bar.baz"} after __post_init__() has executed.

@abdulhaq-e
Copy link
Member

abdulhaq-e commented May 21, 2024

I have an implementation here #2465 that addresses this. I've tried it in production as well. My main use case is having a SQLAlchemy model in a dataclass.

The only thing missing in that implementation is writing tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement This is a new feature or request
Projects
None yet
Development

No branches or pull requests

3 participants