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.56k stars 4.02k forks source link

Manage uni-directional one-to-many relationships #1569

Closed jdubois closed 9 years ago

jdubois commented 9 years ago

To do the uni-directional mapping manually:

This is explained here

This wouldn't be hard to implement, just generate automatically the join table with Liquibase.

jdubois commented 9 years ago

After much thinking about this: I feel this is nearly always a bad solution. It's much better to have a bi-directional relationship in this case, as you don't have a mapping table to add (which will have a performance impact, and make everything more complex). So I don't think we should support this.

FaustXVI commented 9 years ago

What about using the @JoinColumn annotation to avoid the mapping table ? Let's use the JHipster documentation's exemple :

Owner (1) -----> (*) Car

What we want is the Car table to have the owner's id, so let's assume we have it. You can map the Car in the Owner like this :

@OneToMany
@JoinColumn(name = "owner_id")
private Set<Car> records = new HashSet<>();

What do you think ?

jdubois commented 9 years ago

I'm feeling stupid here! Yes, let's re-open this!

jdubois commented 9 years ago

Reading http://stackoverflow.com/questions/11938253/jpa-joincolumn-vs-mappedby I'm not convinced it's such a good idea. We would also need to ask one more question during generation, as the code is different between uni-directional and bi-direction.

Nobody has commented on this -> I'm closing this, but I would be happy to have more insight and comments on this

vritant commented 9 years ago

I think there is nothing inherently wrong in having a unidirectional @OneToOne or @OneToMany,and it should be left to the discretion of the developer to choose performance over design, or otherwise. so to not support it seems a little harsh. For example, A Bat can be of various Sizes, and an Apple can be of various Sizes, but to have a reference of an Apple and a Bat in the Size entity does not make sense.

coolduebtn commented 7 years ago

Hi guys, @jdubois @deepu105 there are many cases where you require a unidirectional @OneToMany and bidirectional doesnt make any sense.. for eg I am creating a model with the following entities. Entity Country , Entity City , Entity Attraction, Entity Image. Now the 3 entities Country , City and Attraction have @OneToMany relationship with Image. It makes sense to have only unidirectional mapping from each of the 3 entities to Image but not the inverse. Infact to have multiple Manytoone mapping from Image to other 3 entities is impractical..Also the idea is to just upload images independently without having the need to tag them with the 3 other entities. I really believe you should provide the option of unidirectonal @OneToMany mapping by using @JoinColumn

agoncal commented 7 years ago

I'm having an interesting use case where unidirectional one to many is needed. Let's say I have a conference, and I want to configure the tracks allowed for this conference. I want this to be unidirectional (basically, I'm configuring the conference, and this won't change much) :

entity Conference {
    name String,
}
entity Track {
    name String,
}

relationship OneToMany {
    Conference{allowedTracks} to Track
}

Now, each time I create a new proposal for this conference, I set the track based on the configuration.

So I don't want to have Track <--> Conference, that doesn't make sense in my use case. I want Track <-- Conference

vladmihalcea commented 7 years ago

Don't use unidirectional @OneToMany. Even with @JoinColumn, they are less efficient than bidirectional ones. This article demonstrates why you shouldn't be using unidirectional one-to-many associations.

agoncal commented 7 years ago

@vladmihalcea In my usecase it's not about efficiency. You configure a conference once in its life time (maybe twice if you did a mistake). The Track entity cannot reference the Conference Configuration, it doesn't make sense.

Modeling is not always about performance or efficiency, it's also about modeling a business domain and requirement (which can be more or less complex).

vladmihalcea commented 7 years ago

The business logic will never fall apart just because the association is bidirectional. Just because you don't see the child-to-parent association, it does not mean it does not exists because that's exactly what a FK is all about.

When you query a track row in the DB, you always get the conference_id FK, so, if it makes sense in the DB, chances are it makes sense in the Business Logic as well. Otherwise, you might just use a JSON store which is a much more realistic way of getting a nested collection. In fact, nested collections are associated with Document stores (e.g. MongoDB), not RDBMS.

As for efficiency, it's just a matter of time. Sooner or later, the data access design decision will matter.

FaustXVI commented 7 years ago

This is the main difference between a design that work and a nice design. A nice design must express the intent. So being able to express a unidirectional relationship is mandatory for that purpose. To me, the need of a onetomany of Antonio is totally legit. Performance should harm your design only when necessary, not before. Please don't prematurely optimise other's code.

Xavier Detant

On 21 Jun 2017 14:08, "Vlad Mihalcea" notifications@github.com wrote:

The business logic will never fall apart just because the association is bidirectional. Just because you don't see the child-to-parent association, it does not mean it does not exists because that's exactly what a FK is all about.

When you query a track row in the DB, you always get the conference_id FK, so, if it makes sense in the DB, chances are it makes sense in the Business Logic as well. Otherwise, you might just use a JSON store which is a much more realistic way of getting a nested collection.

As for efficiency, it's just a matter of time. Sooner or later, the data access design decision will matter.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/1569#issuecomment-310058839, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-EH4gePSBmwdGqReygF9gvXfHYijo7ks5sGQfSgaJpZM4E4I9n .

vladmihalcea commented 7 years ago

Let me quote Viktor Klang:

Optimization is by necessity. Performance is by design.

Actually, the same argument (Performance should harm your design only when necessary, not before) is why you the Open Session in View Anti-Pattern is the default option in Spring Boot.

You will never realize these data access issues when you start a project or on your development machine. You'll find out you've made a bad choice in production, once the data starts accumulating and while concurrency keeps on increasing as well.

deepu105 commented 7 years ago

This is getting interesting :smiling_imp:

jdubois commented 7 years ago

And by the way you can see here that JHipster does have Open Session in View set to false by default. People often underestimate how much care I had when configuring all this :-)

agoncal commented 7 years ago

@vladmihalcea let's say we have the following diagram (I've simplified the real one) :

One conference has several possible tracks. One conference has several proposals. One proposal has only one track.

Conference ------*-> Track
   |                    |
   +-----*> Proposal ---+

This means you can have this kind of data

Devoxx FR only has 3 possible tracks (Java, Web and Android), while Devoxx UK only allows 2 (Java and Agile). The Angular 4 proposal is in the single track "web"

Conference : Devoxx FR
  Tracks : Java, Web, Android     // read this as "allowed tracks"
  Proposal : Angular 4
     Track: Web
  Proposal : Java 8 streams
     Track : Java

Conference: Devoxx UK
  Tracks : Java, Agile
  Proposal : TDD
    Track: Agile
  Proposal : Jigsaw
    Track : Java

There is no way I want a track to be linked to a conference. It has no sense. The bidirectional is between Proposal and Track, but not between Conference and Track (as I said, it's configured one).

vladmihalcea commented 7 years ago

@agoncal Let's discuss on DB terms since it's wiser to map the entities to the DB schema than the other way around.

From a DB perspective, there are just 3 relationships:

If the track is not a child of conference, it means you can't have a one-to-many association so you need a many-to-many associations instead This way, you can have one track referencing many conferences and one conference can have multiple tracks.

So, we need a conference_track join table with 2 FK:

Now, this is what you called allowed tracks. We need to map proposal as well, but a proposal is associated with both a conference and a track, meaning you need to map it to conference_track.

So, the proposal table can have either:

The DB mapping makes it easy to see how the JPA entities will be mapped. The ConferenceTrack becomes an entity of it's own, with a @ManyToOne association to a Conference and a a @ManyToOne association to a Track. The Proposal will just need a @ManyToOne association to ConferenceTrack.

Therefore, you don't need any unidirectional association for this mapping.

agoncal commented 7 years ago

@vladmihalcea think of the conference to be the tenant id: there is no way I want the data of Devoxx FR to have access to data of Devoxx UK.

The JPA spec mentions several times unidirectional one-to-many mapping without too much warnings :

In JPA 2.9 Entity Relationships In addition, this specification also requires support for the following alternative mapping strategies: • The mapping of unidirectional one-to-many relationships by means of foreign key mappings. The JoinColumn annotation

3.2.4 Synchronization to the Database In the case of unidirectional one-to-one and one-to-many relationships, it is the developer’s responsibility to insure that the semantics of the relationships are adhered to.

At the end of the day, I would like JHipster to give me choices. With JDL it would be just a matter of supporting :

relationship OneToMany {
  Owner{car} to Car{owner}   // bi-directional
  Owner{car} to Car          // uni-directional
}
vladmihalcea commented 7 years ago

@agoncal Even if it's a standard, it does not mean that the JPA spec is free of flaws, like:

Related to unidirectional @OneToMany, just try to do the database schema design first and then it's almost impossible to find where the unidirectional @OneToMany fits naturally because, no matter how you look at the database schema, the FK should be mapped as @ManyToOne.

think of the conference to be the tenant id: there is no way I want the data of Devoxx FR to have access to data of Devoxx UK.

If the conference is the tenant_id, then why the track is not a child of conference then? If conference is the root entity, then you can have the track join via a simple track_id FK.

FaustXVI commented 7 years ago

Let's just put the technical details aside for a moment (since clearly we disagree and it will be a very long debate). Let's think product. Jhipster is a scalefolding tool. If the user really wants to do it, he can de a onetomany. But then, if he regenerate the project, all his work is lost. It's simply a poor user experience.

Xavier Detant

On 22 Jun 2017 08:14, "Vlad Mihalcea" notifications@github.com wrote:

@agoncal https://github.com/agoncal Even if it's a standard, it does not mean that the JPA spec is free of flaws, like:

Related to unidirectional @onetomany https://github.com/onetomany, just try to do the database schema design first and then it's almost impossible to find where the unidirectional @onetomany https://github.com/onetomany fits naturally because, no matter how your look at the database schema, the FK should be mapped as @manytoone https://github.com/manytoone.

think of the conference to be the tenant id: there is no way I want the data of Devoxx FR to have access to data of Devoxx UK.

If the conference is the tenant_id, then why the track is not a child of conference then? If conference is the root entity, then you can have the track join via a simple track_id FK.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/1569#issuecomment-310285842, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-EH-lTijH0d54Zz0RR9NQU7iJejE3eks5sGgZJgaJpZM4E4I9n .

jdubois commented 7 years ago

You have good points @FaustXVI @agoncal , now let me comment:

-> As there's a lot of work (and mostly testing!!) here, we should only do it if we also add those other "developer experience" features, like proposing a list of available entities or a list of existing relationships. We clearly don't have the time to do this now (I speak for the core team), as we focus on Angular 4, and then we have JHipster React (React support) and JHipster Webflux (Spring 5 Reactive support), etc etc. But contributions are welcome here, and I could help explain in more details.

agoncal commented 6 years ago

@jdubois @FaustXVI @vladmihalcea it appears that I'm now working on a kind of accounting module. We deal with Quotations, Invoices and Credit Invoices. All these 3 components have in common a OneToMany towards lines:

Because of being bi-directional, JHipster adds a @JSonIgnore on the parent (eg. Quotation) to avoid Jackson infinite recursion. This means that, even if I do a left join, there is no way I can get an invoice with its invoice lines (which is 100% of my usecases). So, out of the box, JHipster doesn't work in these use cases because of bidirectional.

There are several ways to change that (one of them being improving the JSon serialization on bidirectional relationships), but it's not out of the box in JHipster.

Having the choice of going uni-directional would be a plus.

LetsMakeUnidirectionalGreatAgain

vladmihalcea commented 6 years ago

@agoncal To get it clear, your problem is related to how you get the data serialized as JSON, right?

Having a bidirectional @OneToMany association is good for performance (SQL statement wise), but if you need to have the List of child entities instead of having the child referencing the parent in the JSON representation, I think it should be just a matter of allowing you to specify that in JHipster (ideally at the entity level via some annotation or something like that).

agoncal commented 6 years ago

@vladmihalcea you talk a lot about performance, but an invoice has between 1 to 5 invoice lines. My use case here, is more to be able to easily get an invoice with its lines, rather than performance. Unidirectional would make my day.

co5dt commented 6 years ago

I am facing similar issues. I have the three entities OrganizationalUnit, Employee and Contact. All of them relate to another entity ContactInfo like this:

    relationship OneToMany {
        OrganizationalUnit{contactInfo} to ContactInfo{organizationalUnit}
        Employee{contactInfo} to ContactInfo{employee}
        Contact{contactInfo} to ContactInfo{contactInfo}
    }

My solution is simple: Create a OrgaUnitContactInfo, EmployeeContactInfo, ContactContactInfo entity in JDL, wich are all just copy-paste clones of ContactInfo. What I then do:

Now the issue: How to deal with re-running the generator? I know question belongs somewhat to a stackexchange site but maybe I am lucky enough to get a hint on how to deal with this over here.

DareUrDream commented 4 years ago

We are also facing the same issue as we have like 40 entities in one project and there are a few such projects. So based on the limitations of Jhipster we are currently doing things the old fashioned way. Once this feature is available we will revisit it.

phanikiran-wtt-egov commented 3 years ago

We are facing same issue. we have entities A,B,C A one to many to B A one to many to C

and am using the mapStruct for DTO. Now the ADTO does not have bList and CList. B and C DTO has A. how do i get list of B and C DTO's in ADTO. I can override the code and do it. but i have to make a lot of change.

jgdvarun60 commented 7 months ago

enum Gender { MALE, FEMALE }

entity Person { name String gender Gender } entity Family {

}

relationship OneToOne { Family{father(name)} to Person{family1} Family{mother(name)} to Person{family2} }

ERROR! ERROR! You have duplicate properties in entity Person: family