fix test when nested jpaDerivedIdentifier is used #18067

emilpaw · 2022-03-08T22:56:33Z

when a relationship is used with the jpaDerivedIdentifier option and the
related entity also has a relationship with that option set the
updateEntityMapsIdAssociationWithNewId test fails. This
is caused by the createUpdatedEntity reusing the needed parent
entity. This can't work because a parent entity, which already has a child,
can't be used as the parent for a new entity. The reason being
jpaDerivedIdentifier enforces that parent and child share the same id,
which can't be fulfilled with a reused parent entity.
Remove the reuse of the parent entity in createUpdatedEntity when the
entity uses jpaDerivedIdentifier to fix the test.

Fixes #17906

Please make sure the below checklist is followed for Pull Requests.

All continuous integration tests are green
Tests are added where necessary
The JDL part is updated if necessary
jhipster-online is updated if necessary
Documentation is added/updated where necessary
Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

emilpaw · 2022-03-09T17:58:40Z

The failing test react-gradle-mysql-es-noi18n-mapsid has nothing to do with my change. I wonder why this test hasn't failed before since I did not introduce the issue.

Either way, I'd like to add a test for the changes I made but I haven't figured out where exactly it should be placed. Could someone help me with that?

mraible · 2022-03-13T06:23:38Z

Tests pass. Is this good enough?

emilpaw · 2022-03-13T10:08:45Z

@mraible The existing tests never caught the issue this PR fixes. As a measure to avoid regressions, in my opinion, we should add one JDL integration test. I am just still not sure how to go about that.

The JDL I want to test should look something like that:

entity Animal
entity Mamal
entity Cat

relationship OneToOne {
    Mamal to Animal with jpaDerivedIdentifier
    Cat to Mamal with jpaDerivedIdentifier
}

Where should I add such a test case?

mshima · 2022-03-13T10:42:48Z

It's possible to add an MapsIdGrandChildEntity*DTO json referring to https://github.com/jhipster/generator-jhipster/blob/2aa6f6d6581f98a8edb08379b252bc20e724b212/test-integration/samples/.jhipster/MapsIdChildEntityWithoutDTO.json

and add here

generator-jhipster/test-integration/scripts/11-generate-entities.sh

Line 127 in 2aa6f6d

moveEntity MapsIdChildEntityWithoutDTO

Or add to one of customId samples:

generator-jhipster/test-integration/samples/jdl-entities/custom-id.jdl

Line 19 in 2aa6f6d

EntityUuidIdMapsId to @Id EntityUuidId

generator-jhipster/test-integration/samples/jdl-entities/custom-id.jdl

Line 52 in 2aa6f6d

EntityUuidIdDTOMapsId to @Id EntityUuidIdDTO

generator-jhipster/test-integration/samples/jdl-entities/custom-id.jdl

Line 87 in 2aa6f6d

EntityCustomIdMapsId to @Id EntityCustomId

generator-jhipster/test-integration/samples/jdl-entities/custom-id.jdl

Line 120 in 2aa6f6d

EntityCustomIdDTOMapsId to @Id EntityCustomIdDTO

emilpaw · 2022-03-13T23:04:47Z

@mshima Thank you.

I added the test cases.

mshima · 2022-03-15T16:56:49Z

@emilpaw is this PR ready?

emilpaw · 2022-03-15T20:30:47Z

@mshima Yes, now that the E2E tests are working again it's ready. Thank you for the commit.

mshima

Simplify the condition.

generators/entity-server/templates/src/test/java/package/web/rest/EntityResourceIT.java.ejs

mshima

Remove back reference, it's not there

mshima · 2022-03-15T22:40:44Z

test-integration/samples/.jhipster/MapsIdGrandchildEntityWithoutDTO.json

+      "id": true,
+      "otherEntityField": "id",
+      "otherEntityName": "mapsIdChildEntityWithDTO",
+      "otherEntityRelationshipName": "mapsIdGrandchildEntityWithDTO",


Suggested change

"otherEntityRelationshipName": "mapsIdGrandchildEntityWithDTO",

"otherEntityRelationshipName" is needed even if the relationship is unidirectional, right?

This is a bug.
otherEntityRelationshipName should exist only in bidirectional.

test-integration/samples/.jhipster/MapsIdGrandchildEntityWithDTO.json

generators/entity-server/templates/src/test/java/package/web/rest/EntityResourceIT.java.ejs

test-integration/samples/.jhipster/MapsIdGrandchildEntityWithoutDTO.json

when a relationship is used with the jpaDerivedIdentifier option and the related entity also has a relationship with that option set the `updateEntityMapsIdAssociationWithNewId` test fails. This is caused by the `createUpdatedEntity` reusing the needed parent entity. This can't work because a parent entity, which already has a child, can't be used as the parent for a new entity. The reason being jpaDerivedIdentifier enforces that parent and child share the same id, which can't be fulfilled with a reused parent entity. Remove the reuse of the parent entity in `createUpdatedEntity` when the entity uses jpaDerivedIdentifier to fix the test. Fixes jhipster#17906

…d primary key.

emilpaw · 2022-03-16T11:27:17Z

I squashed the commits and now it's ready to merge.

mshima

We need investigation if one-to-one works without otherEntity filtering (removes already used ids). But not really related to this PR.

For now, if otherEntityRelationshipName is set, otherEntity is not set and the relationship is id, we should throw. Filtering will not work as expected.

mshima · 2022-03-18T16:03:07Z

@emilpaw can you run prettier and add relationships back-reference for the GrandChild?

emilpaw · 2022-03-18T20:44:41Z

@mshima Done.

mshima

LGTM

mshima · 2022-03-18T21:27:50Z

Thanks @emilpaw

emilpaw · 2022-12-22T15:29:32Z

Bug bounty claimed: https://opencollective.com/generator-jhipster/expenses/115030 Thanks :)

DanielFran · 2022-12-22T17:27:07Z

@emilpaw approved

github-actions bot added the theme: java label Mar 8, 2022

emilpaw changed the title ~~fix test when nested derived jpaDerivedIdentifier is used~~ fix test when nested jpaDerivedIdentifier is used Mar 8, 2022

emilpaw force-pushed the nested-derived-identifier-failing-tests branch from 6cc8d17 to 08ee70c Compare March 9, 2022 10:16

emilpaw force-pushed the nested-derived-identifier-failing-tests branch from 08ee70c to fa67538 Compare March 10, 2022 20:31

emilpaw force-pushed the nested-derived-identifier-failing-tests branch from 66763f2 to 8ce1162 Compare March 13, 2022 23:14

emilpaw marked this pull request as ready for review March 13, 2022 23:14

emilpaw marked this pull request as draft March 15, 2022 08:24

github-actions bot added the theme: front label Mar 15, 2022

emilpaw marked this pull request as ready for review March 15, 2022 20:27

mshima reviewed Mar 15, 2022

View reviewed changes

generators/entity-server/templates/src/test/java/package/web/rest/EntityResourceIT.java.ejs Outdated Show resolved Hide resolved

generators/entity-server/templates/src/test/java/package/web/rest/EntityResourceIT.java.ejs Outdated Show resolved Hide resolved

mshima previously approved these changes Mar 15, 2022

View reviewed changes

mshima reviewed Mar 15, 2022

View reviewed changes

mshima reviewed Mar 16, 2022

View reviewed changes

generators/entity-server/templates/src/test/java/package/web/rest/EntityResourceIT.java.ejs Outdated Show resolved Hide resolved

emilpaw commented Mar 16, 2022

View reviewed changes

test-integration/samples/.jhipster/MapsIdGrandchildEntityWithoutDTO.json Outdated Show resolved Hide resolved

emilpaw force-pushed the nested-derived-identifier-failing-tests branch from b699af4 to a813573 Compare March 16, 2022 10:52

github-actions bot added theme: angular theme: dependencies Pull requests that update a dependency file theme: gradle theme: react theme: vue labels Mar 16, 2022

emilpaw added 3 commits March 16, 2022 11:53

add integration test case which uses nested maps id

e5acc0c

unify entityTableName naming in MapsId integration tests

fb2a563

Skip cypress test if entity has a required relationship with a derive…

08e87bd

…d primary key.

emilpaw force-pushed the nested-derived-identifier-failing-tests branch from a813573 to 08e87bd Compare March 16, 2022 10:53

github-actions bot removed theme: react theme: vue theme: dependencies Pull requests that update a dependency file theme: angular theme: gradle labels Mar 16, 2022

Tcharl dismissed mshima’s stale review via 08e87bd March 18, 2022 06:37

DanielFran requested a review from mshima March 18, 2022 12:17

mshima reviewed Mar 18, 2022

View reviewed changes

mshima and others added 2 commits March 18, 2022 20:44

add warning on missing back-reference

caf1758

add back reference from child to grandchild

bb8e267

emilpaw force-pushed the nested-derived-identifier-failing-tests branch from df0b4a4 to bb8e267 Compare March 18, 2022 19:52

mshima approved these changes Mar 18, 2022

View reviewed changes

mshima merged commit 21a538e into jhipster:main Mar 18, 2022

emilpaw deleted the nested-derived-identifier-failing-tests branch March 18, 2022 21:16

pascalgrimaud added this to the 7.8.0 milestone Mar 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix test when nested jpaDerivedIdentifier is used #18067

fix test when nested jpaDerivedIdentifier is used #18067

emilpaw commented Mar 8, 2022 •

edited

Loading

emilpaw commented Mar 9, 2022

mraible commented Mar 13, 2022

emilpaw commented Mar 13, 2022

mshima commented Mar 13, 2022 •

edited

Loading

emilpaw commented Mar 13, 2022

mshima commented Mar 15, 2022

emilpaw commented Mar 15, 2022

mshima left a comment

mshima left a comment

mshima Mar 15, 2022

emilpaw Mar 16, 2022

mshima Mar 16, 2022

emilpaw commented Mar 16, 2022

mshima left a comment

mshima commented Mar 18, 2022

emilpaw commented Mar 18, 2022

mshima left a comment

mshima commented Mar 18, 2022

emilpaw commented Dec 22, 2022

DanielFran commented Dec 22, 2022

fix test when nested jpaDerivedIdentifier is used #18067

fix test when nested jpaDerivedIdentifier is used #18067

Conversation

emilpaw commented Mar 8, 2022 • edited Loading

emilpaw commented Mar 9, 2022

mraible commented Mar 13, 2022

emilpaw commented Mar 13, 2022

mshima commented Mar 13, 2022 • edited Loading

emilpaw commented Mar 13, 2022

mshima commented Mar 15, 2022

emilpaw commented Mar 15, 2022

mshima left a comment

Choose a reason for hiding this comment

mshima left a comment

Choose a reason for hiding this comment

mshima Mar 15, 2022

Choose a reason for hiding this comment

emilpaw Mar 16, 2022

Choose a reason for hiding this comment

mshima Mar 16, 2022

Choose a reason for hiding this comment

emilpaw commented Mar 16, 2022

mshima left a comment

Choose a reason for hiding this comment

mshima commented Mar 18, 2022

emilpaw commented Mar 18, 2022

mshima left a comment

Choose a reason for hiding this comment

mshima commented Mar 18, 2022

emilpaw commented Dec 22, 2022

DanielFran commented Dec 22, 2022

emilpaw commented Mar 8, 2022 •

edited

Loading

mshima commented Mar 13, 2022 •

edited

Loading