Generator-jhipster: Composite primary key support

[yelhouti]

Overview of the issue

Hi guys, I'm working on the support of composite primary keys in jhipster. the goal is to allow use cases where we for complex table relationships...
This thread will help follow advancement of the work and discuss design choices that need to be made.
I'll try to put different questions here, and discuss them in the comments:

1. How to specify primary keys in the entity.json
2. How to handle urls: GET /id1/id2/id3, vs /id1-id2-id3 ...
3. How to handle Create vs Update (the keys is always specified since there is no AUTOINCREMENT...)

Motivation for or Use Case

When we want a many to many relationship with additional fields or more than two tables we need a lot of boiler plate code.

Reproduce the error

Related issues

Suggest a Fix

JHipster Version(s)

Environment and Tools

java version "1.8.0_161"
Java(TM) SE Runtime Environment (build 1.8.0_161-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.161-b12, mixed mode)

git version 2.14.3 (Apple Git-98)

node: v6.10.3

npm: 5.6.0

bower: 1.7.7

yeoman: 2.0.0

Docker version 17.12.0-ce, build c97c6d6

docker-compose version 1.18.0, build 8dd22a9

4.14

JHipster configuration

JHipster configuration, a .yo-rc.json file generated in the root folder

.yo-rc.json file

{
  "generator-jhipster": {
    "promptValues": {
      "packageName": "com.xxx.yyy.zzz",
      "nativeLanguage": "en"
    },
    "jhipsterVersion": "4.14.0",
    "baseName": "zzz",
    "packageName": "com.xxx.yyy.zzz",
    "packageFolder": "com/xxx/yyy/zzz",
    "serverPort": "8081",
    "authenticationType": "oauth2",
    "cacheProvider": "hazelcast",
    "enableHibernateCache": true,
    "websocket": false,
    "databaseType": "sql",
    "devDatabaseType": "mysql",
    "prodDatabaseType": "mysql",
    "searchEngine": false,
    "messageBroker": false,
    "serviceDiscoveryType": "eureka",
    "buildTool": "gradle",
    "enableSocialSignIn": false,
    "enableSwaggerCodegen": false,
    "jwtSecretKey": "replaced-by-jhipster-info",
    "enableTranslation": true,
    "applicationType": "microservice",
    "testFrameworks": [],
    "jhiPrefix": "jhi",
    "nativeLanguage": "en",
    "languages": [
      "en",
      "fr"
    ],
    "clientPackageManager": "npm",
    "skipClient": true,
    "skipUserManagement": true
  }
}

Entity configuration(s) entityName.json files generated in the .jhipster directory

I propose to add something like this in normal json file, I will also modify the example as the discution go further:

{
    "fluentMethods": true,
    "relationships": [
        {
            "relationshipType": "many-to-one",
            "relationshipValidateRules": "required",
            "relationshipName": "business",
            "otherEntityName": "business",
            "otherEntityField": "name"
        },
        {
            "relationshipType": "many-to-one",
            "relationshipValidateRules": "required",
            "relationshipName": "basicIndex",
            "otherEntityName": "basicIndex",
            "otherEntityField": "name"
        }
    ],
    "fields": [
        {
            "fieldName": "year",
            "fieldType": "Integer",
            "fieldValidateRules": [
                "required"
            ]
        },
        {
            "fieldName": "month",
            "fieldType": "Integer",
            "fieldValidateRules": [
                "min",
                "max"
            ],
            "fieldValidateRulesMin": 1,
            "fieldValidateRulesMax": 12
        },
        {
            "fieldName": "value",
            "fieldType": "Integer",
            "fieldValidateRules": [
                "required",
                "min"
            ],
            "fieldValidateRulesMin": 0
        }
    ],
    "id": [
        {
            "attributeName": "businessId",
            "columnName": "business_id"
        },
        {
            "attributeName": "basicIndexId",
            "columnName": "basic_index_id"
        },
        {
            "attributeName": "year",
            "columnName": "year"
        }
    ],
    "changelogDate": "20180210195726",
    "entityTableName": "business_basic_index",
    "dto": "mapstruct",
    "pagination": "no",
    "service": "serviceImpl",
    "jpaMetamodelFiltering": true,
    "microserviceName": "businessman"
}

This is clearly related to JDL, but JDL support will be added later and will depend on the new entity.json format

Browsers and Operating System

OSx and not relevant

• [x] Checking this box is mandatory (this is just to show you read everything)

Example repo for what the result should be, all suggestions are welcome:
https://github.com/yelhouti/jhipster-composite-key

[yelhouti]

1. I went with what is in the example, but I soon had to guess what was the type of the primary fields from each relationship, (since now we can have complex relationships ex: A<->AB<->B then AB<-> ABC <->C) then it's not possible to know the types of the primary keys of AB in ABC and having ABC.json independent from the other files without define the types of A and B primary keys in this file.
   I also suggest to put A, and B in different lines in the id field of the .json file (one entry per table instead of 1 per relashionship), and define the type in the id entry.
   EDIT: current relashionship entries has an "otherEntityName" feild which doesn't specify what the ID is (how many feilds or of which type they are). I circumvented that by specifying this information in the ID field I added, but now I need that information when building EntityClass to know the column used for the Join.
   I'm thinking about:
2. removing the ID field I added.
3. class fields can have property (primaryKey: true)
4. adding otherEntityId fields for each relationship if needed (!= id) ()
5. adding primaryKey in relationship (like in class field)
6. I went for /id1/id2/id3, because it will avoid parsing problems (ex: id contains "-" or the other separator)
7. we can query the DB to check if we are doing an update or a create, I would appreciate suggestions on this matter. (2 db querys is not the best way to go in my opinion)

[jdubois]

I agree this is a feature many people would like, at least to reverse-engineer an existing database.
However, we have far too many opened issues at the moment (42!!!), and we are working on JHipster 5 - there is no way we have enough time to review or merge your code at the moment, I prefer to warn you now.

[yelhouti]

I understand, thanks for the heads up. all I need is help for the spec, the merge will come when it will.

[gmarziou]

I'm not a big fan of /id1/id2/id3 URL, it is not very RESTful, usually sub routes are used for nested resources or variants.
',' separator seems a better choice, see https://stackoverflow.com/questions/21663635/url-to-rest-resource-with-a-composite-id

Still I have no idea how springfox will produce the swagger JSON file, have you tried ?

[yelhouti]

@gmarziou since they are in the same controller the are groupped:

[gmarziou]

From JPA perspective, do you plan to use @Embeddable and @Embedded? This is rather common usage for this case.

[gmarziou]

This works in swagger because you use nested routes but id is not materialized this way.

[yelhouti]

@gmarziou yes I did use @Embeddable and @Embedded, it kind of conflicts with @ManyToOne ...
I solved it like this: https://stackoverflow.com/questions/29952386/embedded-id-and-repeated-column-in-mapping-for-entity-exception
(when using DTO's there is no conflict), but I don't know how it will behave when it's not the case.

[gmarziou]

So if you materialize the id an an embedded class, the default "otherEntityName" field could be the toString() method of the embedded, this way developer could easily customize it, not sure how to expose this to frontend though.

[DanielFran]

I agree with @gmarziou about the URL, you should use URL mapping with matrix variables.

http://www.baeldung.com/spring-mvc-matrix-variables

https://books.google.pt/books?id=kcSNCgAAQBAJ&pg=PA114&lpg=PA114&dq=spring+rest+url+with+semi-colon&source=bl&ots=HrRU85ch4L&sig=v38eeIMD_JS01gv_y4hO1LlSbvw&sa=X&ved=0ahUKEwiUopLswKPZAhVL8RQKHWreBloQ6AEIbTAH#v=onepage&q=spring%20rest%20url%20with%20semi-colon&f=false

https://books.google.pt/books?id=GqDcDgAAQBAJ&pg=PA746&lpg=PA746&dq=spring+rest+url+with+semi-colon&source=bl&ots=4ytff1j9R0&sig=p4iwdIl2aZCxyT2rp5Miv4YyEOk&sa=X&ved=0ahUKEwiUopLswKPZAhVL8RQKHWreBloQ6AEIcjAI#v=onepage&q=spring%20rest%20url%20with%20semi-colon&f=false

[DanielFran]

About QueryService, when implementing specification for fields that are part of the composite key, you will need to use for example:

~java
if (criteria.getBusinessId() != null) {
specification = specification.and(buildReferringEntitySpecification(criteria.getBusinessId(),
BusinessBasicIndex _.id, BusinessBasicIndexId_.businessId));
}
~

[DanielFran]

Looking are your code example:

~java
@EmbeddedId
@AttributeOverrides({
@AttributeOverride(name = "businessId", column = @Column(name = "business_id", nullable = false)),
@AttributeOverride(name = "basicIndexId", column = @Column(name = "basic_index_id", nullable = false)),
@AttributeOverride(name = "year", column = @Column(name = "jhi_year", nullable = false))
})
private BusinessBasicIndexId id;
~

The use of @AttributeOverrides is not needed since the column names are the same defined in the class.

[deepu105]

@jhipster/developers anyone in favor of supporting this feature request? IMO we already have too much stuff to take care of and I don't like to see @yelhouti spending effort to do this and that end up being stale PR. @yelhouti maybe you should consider doing it as a module so that you don't have to depend on us to merge it and you have more freedom

This feature is more suitable to be built as a JHipster module so that it can be evaluated first. If the module ends up being very popular we could consider integrating it into the main project here. Please refer the documentation on how to build modules and look at some of the existing modules for inspiration. Reach out to us if you need any help like clarification on how the module system works, adding/exposing new methods for the API etc.
You can use the JHipster module generator to scaffold a module.

[deepu105]

@yelhouti i'm closing the issue as we wont be doing anything on this, we can still continue to discuss here

[yelhouti]

Well, IMHO this id the kind of features that jhipster can't afford not having, all projects using SQL at some point use non Autogenerated Ids, and composite keys... also I'm alawya on gitter and many poeple have requested this.
I'll keep working on it, and discuss it with other members, but if you don't think it's worth it feel free to not participte...

[jdubois]

Yes we need this, but we have far too much work at the moment, and we have more than 40 opened tickets... We mostly do this on our free time, and we can't do everything.
For me composite keys are useful for people who already have a database (otherwise they are always a bad choice), and this is not our core focus as JHipster is supposed to be an "application starter", so it's for new applications - even if I understand some people want to do this, of course!

[gmarziou]

I think it's a good feature but I also think that any change dealing with entity relationships is always painful to implement as it impacts many things in the generator. I have discussed on gitter with @yelhouti and we think the best for him is to expose his ideas through a sample app to collect feedback before working on the generator or a module.

[DanielFran]

If you see in stackoverflow, lot of people use jhipster to create the project + entities and then adapt the code to use entities with composite keys! This is indeed what I do.... And i think this is definitly a missing part of the core of jhipster-generator, at least I see that this functionality will permite adoption to a new community to continue growing up using jhipster.

[deepu105]

Well, that's a bit exaggerated. I have worked with a lot of SQL projects and have audited a lot more and have seen composite keys being used in less than 10% of them and if you have a designed a proper entity model then generated primaries keys are more than enough and IMO are even better. But of course if you prefer to use composite keys that's up to you and your use case but I wouldn't go on to say that its the holy grail for JHipster. I agree with what @gmarziou have suggested.

[yelhouti]

never said composite keys are the "holy grail for JHipster", jhipster is much more than this feature, but I still think it's must have, I also agree with @gmarziou so we will follow this route I guess, and @deepu105 thank you for the feedback :) and all your great work on the project

[yelhouti]

for Information, here is a repo with the proposed changes:
https://github.com/yelhouti/jhipster-composite-key

[DanielFran]

I take a look and seems OK to me, but found small issues and I will push the fixes!

Note:
I spend some time to implement a version with Matrix Variables, but now I conclude it is not a good solution because we would have URL with someting like:
@GetMapping("/business-basic-indices/{businessId}")
@Timed
public ResponseEntity getBusinessBasicIndex(@PathVariable Long businessId, @MatrixVariable Long basicIndexId, @MatrixVariable Integer year) {

Since it is mandatory to have Matrix Variables (basicIndexId and year) dependent on a Path (businessId), seems not good looking!

@yelhouti , @gmarziou , what do you think?

[gmarziou]

First, I haven't implemented such thing so I can't provide any real experience feedback.

My intuition is that we should avoid using '/' in URL because it has a very special meaning in server (REST nested resources) and in client (Angular routes, maybe same for React).

I haven't played a lot with MatrixVariable but to me the controller methods should use the embedded type as argument and this could be done with custom argument resolvers.

~java
@GetMapping("/business-basic-indices/{businessId}")
public ResponseEntity getBusinessBasicIndex(@PathVariable BusinessBasicIndexId id) {
~

It would be the job of the argument resolver to extract the composite key from request path maybe using methods in DTO Mapper or in BusinessBasicIndexId.

Few things are still not clear to me: JSON structure in REST API and impact on the client model. Should the id in JSON be flattened or an embedded object?

[yelhouti]

@DanielFran I emplemented something with the id joined by ',' so: id1,id2,id3 I didn't push it yet, it also makes urls shorter so I kind of prefer it this way. what do you think? I also like the idea of using the custom argument resolver, for now I just o it like this:

~java
@GetMapping("/business-basic-indices/{businessId},{basicIndexId},{year},{month}")
@Timed
public ResponseEntity getBusinessBasicIndex(@PathVariable Long businessId, @PathVariable Long basicIndexId, @PathVariable Integer year, @PathVariable Integer month) {
BusinessBasicIndexId id = new BusinessBasicIndexId(businessId, basicIndexId, year, month);
log.debug("REST request to get BusinessBasicIndex : {}", id);
BusinessBasicIndexDTO businessBasicIndexDTO = businessBasicIndexService.findOne(id);
return ResponseUtil.wrapOrNotFound(Optional.ofNullable(businessBasicIndexDTO));
}
~

The code this way is easily generated
PS: in this case the month is also part of the Id

[DanielFran]

@gmarziou I agree with you that we should not use "/" in the url!

@yelhouti This seems good to me!
I have some doubt if it should be used ',' or ';' instead. If we compare with Matrix Variables, ';' is used to separate different variables and ',' is used for a list of variable values.

Note: Since you are using here DTOs, you should not use BusinessBasicIndexId in the resource.
You should use the existing BusinessBasicIndexDTO or create a specific BusinessBasicIndexIdDTO.

[yelhouti]

@DanielFran since I don't use the variable name, I decided to use ',' we can change of course.
For the Id part, I used the Id, beacuse later in the JPA:
JpaRepository<BusinessBasicIndex, BusinessBasicIndexId>
You need to specify an Id, and all methods like findOne expect an Id
Also there is no need to have a DTO for the Id, since Id is a POJO...

[DanielFran]

@yelhouti Here what I implemented:

Resource:

~java
@GetMapping("/business-basic-indices/{businessId}/{basicIndexId}/{year}")
@Timed
public ResponseEntity getBusinessBasicIndex(@PathVariable Long businessId, @PathVariable Long basicIndexId, @PathVariable Integer year) {
log.debug("REST request to get BusinessBasicIndex : {}", businessId, basicIndexId, year);
BusinessBasicIndexDTO businessBasicIndexDTO = businessBasicIndexService.findOne(businessId, basicIndexId, year);
return ResponseUtil.wrapOrNotFound(Optional.ofNullable(businessBasicIndexDTO));
}
~

Service implementation:

~java
@Override
@Transactional(readOnly = true)
public BusinessBasicIndexDTO findOne(Long businessId, Long basicIndexId, Integer year) {
log.debug("Request to get BusinessBasicIndex : {}", businessId, basicIndexId, year);
BusinessBasicIndexId id = new BusinessBasicIndexId(businessId, basicIndexId, year);
BusinessBasicIndex businessBasicIndex = businessBasicIndexRepository.findOne(id);
return businessBasicIndexMapper.toDto(businessBasicIndex);
}
~

I believe that if we use DTO we should not use domain classes in resource.

About the validation in create or update entity in resource, I validate if the entity exists implementing the exists functionality in service that call the exists crud function.

I also reimplemented the tests without using the elastic search...

[yelhouti]

@DanielFran there was a problem with teh copy paste, could you try again, also, could we use gitter, for this broder discutuons to avoid poluting the thread, and keep here only general info when we agree?

[DanielFran]

@yelhouti Sorry, my copy&paste is strange!

Ok to use gitter!

[gmarziou]

I just edited your post, enclose your code within ~~~java and ~~~

[yelhouti]

@gmarziou thanks for the tip, @DanielFran I see what you did there, prsonnaly I see no added value, in my opinion it will only comlicate the generators code, if more poeple agree with you we can rediscuss this :)

[DanielFran]

@yelhouti I pushed some changes!

https://github.com/yelhouti/jhipster-composite-key/pull/1

[yelhouti]

@DanielFran could you try again without reformating/reordering the code, so I can check the real code changes? would you like to that before or after I push all my recent changes?

[DanielFran]

@yelhouti I added comments to my code...

[brunnels]

looks like customization to the .jhipster entity json and blueprint is the way forward with this. I'm working on something if anyone wants to help.

[deepu105]

@brunnels yes that would be a great start

[yelhouti]

@brunnels I can help, but first I think we should work on repo like, this: https://github.com/yelhouti/jhipster-composite-key
to show how we expect the final result to be.
Once we agree we code the blueprint.
Also, I think that at the same time we could work on Ids, with different type (ex: String...)
You can contact me on gitter to be more efficient.

[MathieuAA]

Such an interesting feature, good luck guys! I'll be sure to update the JDL once this is done. One question though, why a blueprint? 

[yelhouti]

@MathieuAA I agree, I would prefer having this in the main project.

[MathieuAA]

Unless I'm wrong about blueprints (which I may be), this should be on the main project as this is a "small thing". Not to minimize the impact of the work, but I don't see how this can't be in the main project... Or a module at least.Le 19 juil. 2018 14:17, yelhouti [e-mail] a écrit :@MathieuAA I agree, I would prefer having this in the main project.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

[yelhouti]

If we want end to end support (JDL -> .json -> code) support, we indeed must have this in the main project, or else we would have to modify json manually...

[deepu105]

@MathieuAA the problem is this is a PITA to maintain as it will complicate the templates, especially the angular and react ones a lot and I think its better to start as a blueprint. If the blueprint becomes popular we can always integrate it into generator. But for now we have enough complexity IMO

[MathieuAA]

Exactly. I have an idea for this. Le 19 juil. 2018 14:42, yelhouti [e-mail] a écrit :If we want end to end support (JDL -> .json -> code) support, we indeed must have this in the main project, or else we would have to modify json manually...

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

[yelhouti]

@deepu105 I understand your hesitation, the problem is that if we don't have JDL support it would a PITA for all the users to use it. Also I don't think the fornt end will get complicated as it's equivalent of DTOs...

[yelhouti]

@MathieuAA could you check the gitter example please: https://gitter.im/jhipster/generator-jhipster

[MathieuAA]

Almost good enough for me... But if it's a blueprint, I need to find a way to support what the blueprint enables.For instance, if a user doesn't use the blueprint, how can I know that the being-parsed JDL is valid and doesn't use something only logical and accepted for a blueprint?I have an idea about that, but this needs testing.

[DanielFran]

@brunnels @yelhouti I can help also if needed.

[yelhouti]

@DanielFran I'll invite ou on gitter

[jiangzhixiao]

i think this is great feature

[yelhouti]

A working and tested example is finally ready here.
I will start working the generator if it's ok with everyone, the code of the generator will (for now) be in this branch of my fork.
any help is welcome, but please before starting say on which part you will work so we don't step on each others toes.
Thanks in advance.

[pascalgrimaud]

As already discussed, we can't integrate it to the main generator-jhipster. So you shouldn't work on a branch of your fork.

Why not creating a module or blueprint ?

[yelhouti]

Hi @pascalgrimaud , as discussed with @MathieuAA it would be hard to do that with blueprints (let alone modules) as JDL doesn't support it (and without JDL composite keys are pretty useless).
I also feel that this issue is importent enough to merge, and if doesn't I can always put the modified files and use them in a blueprint.
Also development and debugging is much easier without the blueprint, And contributors will be able to help faster this way.

[pascalgrimaud]

No offence @yelhouti I know you try to help the project, but this feature is really complex IMO.

The problem will be to maintain it, in 6 months and more.

We have 50 tickets opened, and not enough contributors to help us.

I know some features we accepted, which don't work since months...

That's why I'm reluctant and you can see in previous comments that I'm not alone. But I'm not the only one who decide.

I just want to notice you, if you decide to start the work before the official "go" from the team.

[jdubois]

Please also note I originally coded this like it is modelled in Ruby on Rails: having an ID primary key is much easier and cleaner, both for us and for our users. And like RoR, I think we shouldn't have composite PK in the main generator. That's too much work and issues, for something which isn't our core goal. It's great if we can help people with existing databases, but we shouldn't waste our efforts on this.

[yelhouti]

@pascalgrimaud I totally understand your point, but since I wasted so much time rewriting composite keys for my projects, I don't my doing the work even if I'm the only one using it.
@jdubois I'm not doing this for existing databases, I'm doing for complexe new projects with business constraints best described as primary keys, and the ability to query them directly with rest endpoints.
I also would like to encourage you guys, when the code is done to have a vote/survey on this.

[jdubois]

Oh yes of course @yelhouti I'll have a look at it - I understand many people want this

[deepu105]

@yelhouti I'm also reluctant at this moment as I pointed earlier, so if you are doing a PR please keep in mind that we will merge it only if we feel that it's not a huge maintenance burden for us. If you can find some smart ways to do it without complicating existing templates then we will consider it

[yelhouti]

@deepu105 I totally get your point, I'll do my best :)

[yelhouti]

almost done with the server side blueprint, still working on "generated" tests.
https://github.com/elhoutico/jhipster-composite-key-server-blueprint
If anyone wants to help, I'll take it

[pascalgrimaud]

@yelhouti : good job but you should use twitter to have better visibility :-) just mention @java_hipster

[yelhouti]

@pascalgrimaud I'll do that as soon as I fix the tests, and as soon as jhipster-core merges the custom annotations PR, for now it kind of works only for me XD

[demetrio812]

Hello guys, anything planned on this? I like jhipster but I think that composite keys give us a better and cleaner (also to read) database design

[yelhouti]

@demetrio812 you can use the following blueprints to support composite keys:
server side: https://www.npmjs.com/package/generator-jhipster-composite-key-server
client side: https://www.npmjs.com/package/generator-jhipster-primeng-blueprint
Do no hesitate to ask questions or fill issues if you have any problem with these.

[demetrio812]

Thanks @yelhouti I will check them!

[Andrea-Giordano]

Helli @yelhouti, thanks for your work!
I am a jhipster noob trying to deal with composite keys: I have installed the blueprint but I don't understand how I can generate a jdl with composite keys to be imported into my project.

[yelhouti]

@Andrea-Giordano don't hesitate to PR the readme for better doc: https://github.com/elhoutico/jhipster-composite-key-server-blueprint/blob/master/test/templates/composite-key-blueprint/jhipster.jh
The composite id key is created automatically based on all the fields and relationships that contain the @id annotation. wherever you add @id this will be (part of) the id
The jdl mentioned generates the project in https://github.com/elhoutico/jhipster-composite-key-server-blueprint/tree/master/test/samples/composite-key-blueprint

[Andrea-Giordano]

@yelhouti when I run

jhipster --blueprints composite-key-server

I get the following error:

ERROR! Error: The installed generator-jhipster-composite-key-server blueprint targets JHipster v>=5.4.0 and is not compatible with this JHipster version. Either update the blueprint or JHipster. You can also disable this check using --skip-checks at your own risk

But my jhipster version is 6.7.1

[yelhouti]

--skip-checks

[yelhouti]

I would like to reopen this issue, now that we have a blueprint (as @deepu105 has suggested) that have been used for months by many users, https://github.com/elhoutico/jhipster-composite-key-server-blueprint/ I would like to merge the main logic to avoid upgrading the blueprint each few weeks for the latest features. what do you guys think?

[jdubois]

@yelhouti you've had more than 1,000 installs for your blueprint (see http://www.npm-stats.com/~packages/generator-jhipster-composite-key-server ) which is good for something this specific.

I'm really not a big fan of composite keys, but of course it's better for us to cover more use cases -> I'm mostly concerned about maintenance, is that something very complex to implement and maintain?

[yelhouti]

@jdubois thanks for considering, I really appreciate it. once implemented it should be very easy to maintain. For implementing, Everything is ready, the blueprint is kind of a fork a this repo (maintained up to the previous major version). I would still make some changes that i could do in the blueprint to have a cleaner code. just give me a go I will create a branch an move all my changes here.
EDIT: Some change could break other blueprints:

• id: is a field and not added in the template (if the id field is missing it is added automatically with the correct type).
• The way .json file are loaded today is one at a time, when using composite-keys this doesn't work a change I would make is load all at once in memory, and link one entity to an other to read composite key types... (+ and optionally once loaded allow blueprints to modify the jsons with a callback. @MathieuAA what do you think)

[jdubois]

Ids are used a little bit everywhere, for example in the front-ends, so indeed I'm afraid you might break a lot of things, and it's going to be very hard to test.
It can only be in the main generator if that's really stable, as people we see that option and will trust it.
If you really want to give it a try, we can review the PR and see if that seems OK, but it really seems risky to me.

[yelhouti]

Another thing I wanted to propose for tests that i do for my blueprints:
Have JDL with scenario options... the test, generate the app, runs it, deployes it on a server, and runs the e2e tests. I figure you do something similar that should catch this, don't you?

[jdubois]

Yes we do this on the main generator, but it's so complex it's not possible to test all scenarios. This is why touching on something so critical can lead to some issues that we won't catch, as we don't test everything.

[mshima]

I hardly recommend to postpone this feature for jhipster 7.1.0.
I think there will be a major refactoring and this feature will make much more complicated.
It will be much easier to implement this feature on jhipster 7.

[yelhouti]

@mshima willing to wait, waited for months XD. any ETA on 7.X?
EDIT: I think it's a bas idea for me to add Breaking changes on a 7.1...
Maybe you could add all your major changes to master, and add my changes after you. and we ship everything into 7.0.0. what do you think?

[mshima]

Sure, 7.1.0 or late 7.0.0.
I just saying that starting creating a major feature before a major refactoring can be frustrating and lots of merging.

[yelhouti]

@mshima totally agree with you, this is why I am asking you here to please let me know when done with the big refactoring, so I can hope in before the 7.X release

[yelhouti]

I might need some help with the react part If anyone has some time to spare, I would be very grateful!

[yelhouti]

@mshima @jdubois please add this to 7.x roadmap add let me know when I can start working on it

[pascalgrimaud]

@yelhouti : v7 has already started :) @avdev4j is the lead of the roadmap

[avdev4j]

Hi @yelhouti master is now the v7 branch. You can start on it when you want.

We will add this ticket to the github project, could you test to add it? (in project tab)

[yelhouti]

@avdev4j I'm afraid I can't, not enough privileges I think. @pascalgrimaud I was told by @mshima to wait until some big changes are done, are they?

[mshima]

@yelhouti I have plans to refactor the generators configs.
It is much harder to implement now, than it will be (if the patches get merged).

[yelhouti]

@mshima any ETA? When do you think I can start working on this and how much will I have before the release?

[mshima]

@yelhouti hard to tell.

@avdev4j how are planning to do with jhipster 7 jhipster-internal PRs?
Can I assign to you? Should I merge them directly after some period?
First PR https://github.com/jhipster/generator-jhipster/pull/12022

Others tasks:

• Remove most of configOptions usages in favor of this.config to improve inter generator synchronisation.
• Change entity to use Storage instead of just loading the json to improve inter generator synchronisation.
• Refactoring fields and relationships loading, it is too confusing right now.
• Run generators in parallel.

They are all tasks related to the refactoring I am taking about.

[avdev4j]

@mshima I don't want to be the single point of failure for this v7 ;). We should keep doing just like we used to do. The only thing we have to do, is to track issues and/or pr in the GitHub project. So, if a pr is ready feel free to merge it.

I wonder If you can add and manage tickets. Because you are in the core team you should can.

[avdev4j]

oh and yes, we need to update the v7 roadmap too https://github.com/jhipster/generator-jhipster/issues/10958

[yelhouti]

@mshima what you are working on looks really great for what I want to do I was waiting for this changes for ages. Some of the concerns I have:

1. In the current version each json is loaded just before using it, can we have them loaded all at once before any entity generation (really useful to handle relationships specially for my case composite keys)
2. Can I have a hook/callback or step in my blueprint, just after all json loading but before the writing phase, to populate the fields for blueprints (link the relationships...)? (this can also be done by loading all entities in the initialization phase)
   Thanks in advance.

[mshima]

@avdev4j ok then, I will merge it after some period of time.

@yelhouti

1. In the current version each json is loaded just before using it, can we have them loaded all at once before any entity generation (really useful to handle relationships specially for my case composite keys)

Actually the json is loaded at generator constructor, but is loaded as plain object and every change after it is just ignored.
My plan is to use yeoman-generator's Storage, it uses mem-fs and read/write every time a config is used.
I implemented a cache for yeoman-generator 4.11.0 that reloads the config every time the file is written.

2. Can I have a hook/callback or step in my blueprint, just after all json loading but before the writing phase, to populate the fields for blueprints (link the relationships...)? (this can also be done by loading all entities in the initialization phase)

Not sure the use case you are thinking about, but I don't see the need for hooks.
You can do whatever you want in configure step. You can even create a pre-conflicts (post-writing) step for example.
I do add relationships between entities in my blueprint, but I do in the app generator before calling entity generator, because there are too many problems with current entity workflow.

[yelhouti]

@mshima in my case, I don't want my changes to the json to be written to disk (they are cyclic due to relationships...), i want to be able to enrich them in a configure step, after loading but before any entity starts. can we discuss this on call (skype, slack, whatsapp or whatever)?

[mshima]

@mshima in my case, I don't want my changes to the json to be written to disk (they are cyclic due to relationships...), i want to be able to enrich them in a configure step, after loading but before any entity starts. can we discuss this on call (skype, slack, whatsapp or whatever)?

Sure, do you have gitter? Ping me there I can give my whatsapp there.
If you don't want to persist, you will need something like this: https://github.com/jhipster/generator-jhipster/pull/11833, otherwise you will have to set every data for the relationship.

[pascalgrimaud]

@yelhouti : I need you, to tell us if this feature should be in V7 plz. Should I put it in our roadmap ?

[yelhouti]

@pascalgrimaud yes please, just tell me when I can start working on it.

[pascalgrimaud]

@yelhouti : when you want ! In fact, the current master branch is for v7. And FYI, jhipster core has been merged here too.
So everything can be done here.

[yelhouti]

@pascalgrimaud great thanks, I will starting working on it ASAP, @mshima can you confirm that you finished what you asked me to wait for? thanks

[mshima]

@yelhouti not finished https://github.com/mshima/generator-jhipster/pull/27.
The PR is against my own branch to keep track of generated code changes, so that at least for the tests projects are generated correctly.

[mshima]

@yelhouti I am just changing the generator, feel free to start from the templates, or be prepared to rebase the generator.

[mshima]

@yelhouti done for now.

[pascalgrimaud]

starting with this bounty. If it's a lot of work, I'll increase it

[yelhouti]

@pascalgrimaud thanks :+1:

[pascalgrimaud]

After thinking a little bit about this feature and discussing with @MathieuAA, to achieve this feature, I think we need to provide another feature before:

• allow to choose the type + name of ID

Currently, all ID for SQL database are:

• Long id

Maybe, allow the possibility to chose the type: Integer, UUID, String, etc... and the name too.

Tell me if I'm wrong @yelhouti ?

[yelhouti]

@pascalgrimaud my blueprints (that I'm migrating into the main generator) already allows to specify the type of the primary key/keys. you do it like this:

entity A {
    @id key String required
}

only caveat is that it doesn't work when the name of the variable is id since the main generator overrides my type, but this is a very small code change I'm planning to do at the end.

[DanielFran]

@yelhouti do you need help migrating https://github.com/elhoutico/jhipster-composite-key-server-blueprint to generator?

[yelhouti]

@DanielFran Yes much needed, Thanks for asking ! I would gladly talk to any one who would like to help, about what to migrate, what not, what can should be improved while migrating... I will hopefully have a bit more time in two months to the code my self if no one can help.

[mshima]

IMO this should be implemented in 3 tasks:

• Entities generator https://github.com/jhipster/generator-jhipster/issues/12696
• Configurable single key.
• Composite key.

[yelhouti]

@mshima all are implemented here:
https://www.npmjs.com/package/generator-jhipster-composite-key-server
I just need to find few days to merge and clean

[yelhouti]

Update: I finally managed to get some work done, I think it's the hardest part, the rest is simple migration and renaming. I hope to finish by the end of next week

[pascalgrimaud]

@yelhouti : I'm increasing the bounty as I'm pretty sure it's a lot of work.
And it will be a nice feature to have for v7

[mshima]

@yelhouti I have a branch implementing configurable single key.
https://github.com/mshima/generator-jhipster/tree/skip_ci_id.

The missing feature is to allow to change id field name.

[pascalgrimaud]

@mshima : seriously, I'd like to know how you do to find so much time to bring all these features so quickly ?

[yelhouti]

@pascalgrimaud thanks a lot.
@mshima please dont merge that, I'm working on a big PR I can push the code to my repo so you can see the latest version, I would really appreciate your help on that

[mshima]

@pascalgrimaud I find it easier to change here than in all entities from my project.
I have many more changes in my internal branch, but they are not suitable for main inclusion or needs many more work to propose to implement here.
Structural changes like this needs to be implemented upstream otherwise there will be too many conflicts.

[mshima]

@yelhouti you should rebase against https://github.com/jhipster/generator-jhipster/pull/12703 at least.

[yelhouti]

I'd rather not for the moment, I have simit code (sharedEntities) to allow accing one from another for relationships...
I'm also cleaning the code for dtos... so the order of fields make more sense when the relationship is (part of) the id.

[mshima]

I'd rather not for the moment, I have simit code (sharedEntities) to allow accing one from another for relationships...

I think what you are talking about is already implemented in main branch.
https://github.com/jhipster/generator-jhipster/blob/2a54b065fa53b24ab27168a5571cb2a5ee9b3c6a/generators/entity/index.js#L487-L490

So relationship.otherEntity is the other entity that you are probably talking about.
https://github.com/jhipster/generator-jhipster/pull/12703 makes sure otherEntity is available even when executing jhipster entity foo and jhipster jdl --skip-application (it is currently like jhipster entity foo && jhipster entity bar && etc.

I'm also cleaning the code for dtos... so the order of fields make more sense when the relationship is (part of) the id.

Would be nice to take https://github.com/jhipster/generator-jhipster/issues/12707 into account for dtos.

[yelhouti]

@mshima I think avoiding flattening in DTO is a really good idea, however I wouldn't use a static inner Class, I would just reference another DTO, and in the mapper make sure to only fill the wanted fields (using @Named).

[mshima]

My example was with inner class because:

• it's a data leak safeguard
• each relationship can have a different field as value so cannot be 1 keyValue DTO.

But stop flattening is good enough IMO.
I don't know how @Named works.

[yelhouti]

Here an example of @Named:
EmployeeMapper.java

    @Named("default")
    @BeanMapping(ignoreByDefault = true)
    @Mapping(target = "username", source = "username")
    EmployeeDTO toDtoUsername(Employee entity);

    @Named("fullname")
    @BeanMapping(ignoreByDefault = true)
    @Mapping(target = "username", source = "username")
    @Mapping(target = "fullname", source = "fullname")
    EmployeeDTO toDtoFullname(Employee entity);

EmployeeSkillMapper.java

    @Mapping(source = "employee", target = "employee", qualifiedByName="fullname")
    EmployeeSkillDTO toDto(EmployeeSkill employeeSkill);

[yelhouti]

@mshima I will implement this two if no one minds

[mshima]

If you are talking about DTO, go for it.

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity.
Our core developers tend to be more verbose on denying. If there is no negative comment, possibly this feature will be accepted.
We are accepting PRs :smiley:.
Comment or this will be closed in 7 days

[yelhouti]

This is almost done, we have a working version compatible with previous tests... we are doing a big refactoring as suggested by mshima. There also some edge cases that we know are not working perfectly that would happen on very complicated schémas, we will add tests for them and fix them once we have time or once the community manifests interest for them.
Hopefully, once all this is merges, I will be able to contribute a bit more to the project since for now I was mainly working on our blueprints.