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

Liquibase: Incorrect creation of new foreign key relationships during schema updates #27598

Closed
1 task done
OmarHawk opened this issue Oct 16, 2024 · 4 comments · Fixed by #27608
Closed
1 task done
Labels
area: bug 🐛 $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ theme: liquibase $100 https://www.jhipster.tech/bug-bounties/

Comments

@OmarHawk
Copy link
Contributor

Overview of the issue

When adding a new Foreign Key relationship, the generated changelogs fail to be applied, especially, if the other entity side has an id column not named "id", but e.g. "uuid". In our case, we even had that foreign key already and tried to re-create that with some more options (like CASCADE). The generator successfully detected the need to recreate the foreign keys, but the new ones as part of the updated_entity_constraints produces wrong column name references (and maybe more).

Motivation for or Use Case

Should be possible to apply updates to schema involving changes in foreign keys.

Reproduce the error
  1. have an initial jdl and entities on the other side have an id field named "uuid" and generate changelog (and more) via jhipster jdl
  2. change jdl to have a new foreign key and run jhipster jdl
  3. try to apply liquibase changelog / start up application
  4. Failure during startup

Here are some stripped sample jdls.

Step 1:

entity D {
   status String
   @Id uuid UUID
}

entity L {
   comment String
   status String
   date Instant
}

relationship ManyToOne {
   L{data}
      to
   D{log}
}

Step 2:

entity D {
   status String
   @Id uuid UUID
}

entity L {
   comment String
   status String
   date Instant
}

relationship ManyToOne {
   L{data}
      to
   @OnDelete("CASCADE") @OnUpdate("CASCADE") 
   D{log}
}
Related issues
Suggest a Fix

This is because the logic in the update changelog files to construct the attributes for the addForeignKeyContraint works differently than in the add changelog for the initial changelog.
Not working:

const constraintName = this.getFKConstraintName(entityTableName, relationship.columnName, prodDatabaseType);
let baseColumnName = relationship.columnName + '_id';
if (relationship.relationshipType === 'one-to-one' && relationship.id === true) {
baseColumnName = 'id';
}

--> hardcoded "_id", no matter, if the actual other entity has a different column name as id, therefore having a different column name in the current entity

Working:

const constraintName = this.getFKConstraintName(entity.entityTableName, relationshipName, prodDatabaseType);
let baseColumnNames;
let referencedColumnNames;
if (relationshipType === 'one-to-one' && ownerSide && relationship.id === true) {
baseColumnNames = relationship.otherEntity.primaryKey.fields.map(field => field.columnName).join(',');
referencedColumnNames = relationship.otherEntity.primaryKey.fields.map(field => field.columnName).join(',');
} else if (relationship.otherEntity) {
baseColumnNames = relationship.otherEntity.primaryKey.fields.map(field => relationship.columnName + '_' + field.columnName).join(',');
referencedColumnNames = relationship.otherEntity.primaryKey.fields.map(field => field.columnName).join(',');
} %>

Especially line 38-41 being relevant, but I guess the rest may also be important....

JHipster Version(s)

8.7.1

JHipster configuration

To provide all information we need, you should run jhipster info in the project root folder (or workspaces root for microservices), and
copy/paste the result here.
jhipster info removes sentitive information like rememberKey, jwtSecretKey. Double check if there is any other sensitive info.

As alternative you can add a JDL wrapped in below structure

JDL definitions
     JDL content here
  

The information is mandatory for bug reports. This will allow us to use automated tests and genarate the broken sample using jhipster from-issue command.

Browsers and Operating System
  • Checking this box is mandatory (this is just to show you read everything)
@OmarHawk
Copy link
Contributor Author

Ideally, these attributes should be calculated somewhere centrally. An easy fix would be to copy&paste the part of code from the add to the update template

@mraible
Copy link
Contributor

mraible commented Oct 16, 2024 via email

@OmarHawk
Copy link
Contributor Author

Yes, I will :)

@DanielFran DanielFran added $100 https://www.jhipster.tech/bug-bounties/ $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ area: bug 🐛 theme: liquibase and removed area: triage theme: undefined labels Oct 16, 2024
OmarHawk added a commit to OmarHawk/generator-jhipster that referenced this issue Oct 17, 2024
@OmarHawk
Copy link
Contributor Author

OmarHawk commented Oct 17, 2024

Can you please create a PR to test if your theory works?

See my PR #27608 @mraible

https://github.com/jhipster/generator-jhipster/pull/27608/files#diff-4d1e23473a996d309372a8a4caadba2f4d0f8f40218d149dfee1a9f8b802a9c0R596-R599
-> this is the existing part of the tests that shows, the column there is actually supposed to be named another_another_id

https://github.com/jhipster/generator-jhipster/pull/27608/files#diff-4d1e23473a996d309372a8a4caadba2f4d0f8f40218d149dfee1a9f8b802a9c0R602-R612
-> this one is the new test that tests the reference is actually being created to that very column - and with the current state, it fails

OmarHawk added a commit to OmarHawk/generator-jhipster that referenced this issue Oct 18, 2024
OmarHawk added a commit to OmarHawk/generator-jhipster that referenced this issue Oct 18, 2024
OmarHawk added a commit to OmarHawk/generator-jhipster that referenced this issue Oct 21, 2024
OmarHawk added a commit to OmarHawk/generator-jhipster that referenced this issue Oct 21, 2024
OmarHawk added a commit to OmarHawk/generator-jhipster that referenced this issue Oct 21, 2024
OmarHawk added a commit to OmarHawk/generator-jhipster that referenced this issue Oct 21, 2024
@mshima mshima closed this as completed in 2081ef1 Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: bug 🐛 $$ bug-bounty $$ https://www.jhipster.tech/bug-bounties/ theme: liquibase $100 https://www.jhipster.tech/bug-bounties/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants