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

Change current DTO implementation #12707

Closed
1 task
mshima opened this issue Oct 10, 2020 · 12 comments · Fixed by #12947
Closed
1 task

Change current DTO implementation #12707

mshima opened this issue Oct 10, 2020 · 12 comments · Fixed by #12947

Comments

@mshima
Copy link
Member

mshima commented Oct 10, 2020

Overview of the feature request

Current DTO implementation flattens id and label from a child to its parent when relationship is one-to-one.
IMO we should keep hierarchical implementation for consistency.

Motivation for or Use Case

Flattening relationships makes templates more complicated.
I don't see benefits to flattened relationship.

Related issues or PR
  • Checking this box is mandatory (this is just to show you read everything)
@tillias
Copy link

tillias commented Oct 16, 2020

@mshima may I ask if what you want in this ticket is exactly the same I desperately need in my project?

Let's say I have following dto:

public class HouseDTO {
    private Long id;
    private String name;
    private String description;
    private String url;
    private Long ownerId;
}

private class OwnerDTO {
    private Long id;
    private String name;
}

If I need full entity in front-end (and I'm using Angular) then I can do the following:

public class HouseFullDTO {
    private Long id;
    private String name;
    private String description;
    private String url;
    private OwnerDTO owner;
}

In the Back-End it work just fine (you can even introduce HouseBaseDTO and move all except Owner to it). But Front-End is painful like a hell!

Waaaaay more cleaner solution will be delivering OwnerDTO which contains only ID filled:

public class HouseDTO {
    private Long id;
    private String name;
    private String description;
    private String url;
    private OwnerDTO owner; <--- here all fields are null except ID
}

This has so many benefits, e.g. very easy regeneration of entities, consistent handling in FE, consistent handling in BE, easy merging changes etc etc. If somebody is interested in it I can tell more about my abyss of pain I have with this freaking flattening approach. In fact regenerating entitis at the moment is not worth even trying, because the amount of manual work needed to adapt all custom DTOs, front-end entities and services is equal starting project from scratch without JHipster.

@mshima
Copy link
Member Author

mshima commented Oct 16, 2020

@tillias something like this but with reference dto:

public class HouseDTO {
    private Long id;
    private String name;
    private String description;
    private String url;
    private HouseOwnerReferenceDTO owner;

    public static HouseOwnerReferenceDTO {
        private Long id;
        private String name;
    }
}

private class OwnerDTO {
    private Long id;
    private String name;
    private Date birthday;
}

Related to #12374

@tillias
Copy link

tillias commented Oct 16, 2020

Will this also include front-end entities? What is name in RefDTO? Which field to take if there is no name field in the entity? Anyways it will be better then flattening

@mshima
Copy link
Member Author

mshima commented Oct 16, 2020

@tillias updated the example.
name is the value field (label) that jhipster allows to add to each relationship, it can be missing.

@mshima
Copy link
Member Author

mshima commented Oct 16, 2020

Will this also include front-end entities?

It must include, otherwise it won't work.

@tillias
Copy link

tillias commented Oct 16, 2020

Ok, then I don't get it how it will make life easier and we're talking about two completely different issues.

Let's say I have following component: https://github.com/tillias/microservice-catalog/blob/master/src/main/webapp/app/entities/microservice/microservice-dashboard/microservice-card/microservice-card.component.html

It receives IMicroservice entity which contains (Team, Status) custom attributes. With your approach it will be replaced with Ref object which delivers nothing but ID, so it will be exactly the same problem with templates.

With my suggestion you simply need to write custom mapper in BE and additional method in Service FE. That was it.

@mshima
Copy link
Member Author

mshima commented Oct 16, 2020

Ok, then I don't get it how it will make life easier and we're talking about two completely different issues.

If you need the complete object, just change owner to use OwnerDTO instead of HouseOwnerReferenceDTO
This approach:

  • Can be used with maptstruct, otherwise we should create our own mapper for each relationship.
  • Safeguard for data leak.
  • Api compatibility, just less data.
  • Cyclic relationship safe.

JHipster should provide the full object reference for Value Objects like embedded, cascade all relationships.

@tillias
Copy link

tillias commented Oct 16, 2020

Ok I see, so this means either support use-case when partial object is needed or use-case when full object is needed. But not both use-case simultaneously. That is indeed interesting approach, but this will still require manual adaption of all entities (both BE and FE). After this adaption you will still have to adapt UI templates as well. And after each import it will overwrite those custom changes. Whatever you decide on the triage I have created #12779 which won't need any mapper adaptions at all -> if user needs it then implement it as a custom method. Also it won't pollute domain model with technical details (at least in FE, cause what is called domain model in BE is data mapping layer)

@mshima
Copy link
Member Author

mshima commented Oct 16, 2020

#12779 seems @valueObject(cascade) I have in an internal implementation.

  • Parent needs to have reference to full Child implementation.
  • Guarantee Childs are eager loaded.
  • Add Cascade.ALL to the parent -> child relationship.

@tillias
Copy link

tillias commented Oct 16, 2020

Just to be on a safe side, no offence meant, I do understand and value what you're doing here guys, very much appreciated! My use-case was more related to overwriting custom logic (especially in "domain entities"). I wish there was https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/partial-classes-and-methods in Java :(

@gzsombor
Copy link
Member

@tillias : I don't understand, why the proposed approach works on the backend, but it's painful on the frontend? What makes the frontend so special?
In my experience, using inheritence in the DTO objects works neatly. I have :

  • IdDTO(Long id)
  • NamedDTO(String name) + extends IdDTO
  • HouseListDTO(couple of fields for listing) + extends NamedDTO
  • HouseDetailDTO(everything with references to other entities) + extends HouseListDTO
  • HouseUpdateDTO(fields which can be updated)
  • HouseCreationDTO( fields required for creation)

Yes, there are lot of different types, but the compiler is there, and there is no confusion, what fields are filled, and what not. And yes, there is no custom logic in the DTO, they are just for defining the protocol, the available fields.

@tillias
Copy link

tillias commented Oct 16, 2020

@gzsombor proposed approach doesn't work neither in BE nor in FE. Sorry for misunderstanding, I'll blogpost about it next week. Short anwer -- of course if you have tons of time for manual tuning of entities and merging stuff for hours after each import-jdl, then it works just fine. But then generator is not needed in a first place, at least part which produces DTOs. It is much more easier writing custom service layer + DTOs once and avoid merge completely.

Update:
I have just tested it with single entity:

entity House {
    name String required
    testField String required
    description TextBlob
}

I have added testField into JDL and executed jhipster import-jdl. As a result 31 files changed, overwriting:

  1. Data layer entity
  2. BE Service
  3. DTO
  4. BE Resource
  5. FE Model
  6. FE Service

So you move your logic into custom BE Service, custom DTO, custom BE Resource, custom FE Model Object, custom FE Service right, creating parallel hierarchy of entities and services to what is generated by JHipster?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants