jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.55k stars 4.02k forks source link

Move DTOs from the service to the rest layer #21821

Open mshima opened 1 year ago

mshima commented 1 year ago
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
gzsombor commented 1 year ago

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

mshima commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

pascalgrimaud commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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