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

Move DTOs from the service to the rest layer #21821

Open
1 task
mshima opened this issue Apr 15, 2023 · 9 comments
Open
1 task

Move DTOs from the service to the rest layer #21821

mshima opened this issue Apr 15, 2023 · 9 comments
Labels
area: enhancement 🔧 $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ theme: java $100 https://www.jhipster.tech/bug-bounties/

Comments

@mshima
Copy link
Member

mshima commented Apr 15, 2023

Overview of the feature request

JHipster DTOs are the rest api definitions and are probably best fit in the rest layer.

The main benefit is to simplify the templates by moving all dto operations to the controller.

Motivation for or Use Case
Related issues or PR
  • Checking this box is mandatory (this is just to show you read everything)
@gzsombor
Copy link
Member

That wont work, whenever you start to deal with lazily initialized JPA entities, because you need to that in a transaction.

@mshima
Copy link
Member Author

mshima commented Apr 16, 2023

We are using optimized eager loading in the repository layer.
And AFAIK we support DTOs without services. In this case DTO looks orphan without the corresponding service.

@Tcharl
Copy link
Contributor

Tcharl commented Apr 19, 2023

Nice idea, dto at service level cause issues for complex custom service relying on others: there are many cases where you get the DTO, retransform to entity to get a field that is used by the other service, ...
Just to think about this custom (and generated) services and lazyloadingexceptions (note that I'm not a fan of the current eager fetch neither, would have preferred a projected view but it's another topic)

@deepu105 deepu105 added $100 https://www.jhipster.tech/bug-bounties/ $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ labels Aug 15, 2023
@deepu105
Copy link
Member

In that case should we remove the VO objects on the Rest layer as well as they are just a mirror of DTOs. We created them to avoid leaking transactional layers to cleint side AFAIK

@mshima mshima mentioned this issue Sep 8, 2023
19 tasks
@pascalgrimaud
Copy link
Member

I agree with @gzsombor

It will probably work for all simple generated projects.
It won't work for real and existing projects.

DTOs support without service has always been a bad idea.
As @deepu105 said, the VO objects were created for that reason.

It's a really major breaking changes and we should collect more feedbacks before doing this change.
Just my opinion :-)

cc @jhipster/developers

@jdubois
Copy link
Member

jdubois commented Sep 13, 2023

Indeed, we have optional DTOs from the repository to service layers, for those who don't like to have their JPA entities crossing all layers (which isn't my case, I love to do this!!). And then the VOs, for "view objects", are from the service to Web layers - they are similar to the view objects we also have in the front end.
The DTOs are for the business layer, and the VOs are for the view layer.

@deepu105
Copy link
Member

In that case should we remove the VO objects on the Rest layer as well as they are just a mirror of DTOs. We created them to avoid leaking transactional layers to cleint side AFAIK

Also I thought we disallowed DTO without service. Atleast thats how I remember it, or maybe its only on the JDL side, i need to check the code to be sure

@mshima
Copy link
Member Author

mshima commented Sep 13, 2023

We have VO related to login and security only.
https://github.com/jhipster/generator-jhipster/tree/main/generators/server/templates/src/main/java/package/web/rest/vm

DTOs mapping happens between service and view layers or output of view layer.

If we remove service layer, generated DTOs are actually VO but they are generated at the non existing service layer package/service/dto:

application {
  config {
    baseName jhipster
    devDatabaseType h2Disk
  }
  entities *
}

entity DtoController {
  name String
}

dto * with mapstruct
service DtoController with no

DTO implementation filters related entity fields keeping only key and visual field (select label) behaving like a VO.

JHipster's DTO motivation is quite confusing to said the least.
I would say our current DTO implementation motivation is VO which makes sense since jhipster generates a view layer which most of the time makes sense to be filtered. Business layer DTOs can be much more complex.

@mshima
Copy link
Member Author

mshima commented Sep 13, 2023

I suppose this needs more discussion so I will remove from v8 tasks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: enhancement 🔧 $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ theme: java $100 https://www.jhipster.tech/bug-bounties/
Projects
None yet
Development

No branches or pull requests

6 participants