search

jakartaee / persistence

https://jakartaee.github.io/persistence/
Other
186 stars 55 forks source link

Derived identity TCK tests rely on unspecified cascading behavior #632

Open beikov opened 1 month ago

beikov commented 1 month ago

The following tests rely on unspecified cascading behavior of the @MapsId annotation:

After fixing a Hibernate issue, we realized that the spec does not define this behavior and that assuming CascadeType.PERSIST for associations that are part of an entity id is not correct.

The mentioned tests essentially assume that persist cascading is applied. By reordering the persist calls in the test, this wrong assumption can be fixed. Interestingly, a few similar other tests e.g. ee.jakarta.tck.persistence.core.derivedid.ex4a and ee.jakarta.tck.persistence.core.derivedid.ex4b do not rely on such behavior, so maybe this behavior was assumed by accident?

gavinking commented 1 month ago

The issue here is that an entity in the persistent state must have a well-defined persistent identity. Yes, right from the very moment that persist() is called.

But in these tests, that's not the case: the dependent objects do not have a well-defined persistent identity when they are made persistent, their persistent identity can only be determined after the Employees are made persistent.

One supposes that other JPA implementations, like Hibernate, have hacks around these TCK tests, but those hacks are not correct.

scottmarlow commented 1 month ago

@lukasj do you expect that we will try to make further Persistence 3.2 test changes like https://github.com/jakartaee/platform-tck/pull/1309 which is for this challenge?

beikov commented 2 weeks ago

Please exclude the tests from the next 3.1 and 3.2 TCK builds.

scottmarlow commented 1 week ago

Impacted 3.1 tests (for EE 10 Platform TCK testing) would be:

com/sun/ts/tests/jpa/core/derivedid/ex1a/Client.java#DIDTest_from_appmanaged com/sun/ts/tests/jpa/core/derivedid/ex1a/Client.java#DIDTest_from_appmanagedNoTx com/sun/ts/tests/jpa/core/derivedid/ex1a/Client.java#DIDTest_from_pmservlet com/sun/ts/tests/jpa/core/derivedid/ex1a/Client.java#DIDTest_from_puservlet com/sun/ts/tests/jpa/core/derivedid/ex1a/Client.java#DIDTest_from_stateful3 com/sun/ts/tests/jpa/core/derivedid/ex1a/Client.java#DIDTest_from_stateless3 com/sun/ts/tests/jpa/core/derivedid/ex1b/Client.java#DIDTest_from_appmanaged com/sun/ts/tests/jpa/core/derivedid/ex1b/Client.java#DIDTest_from_appmanagedNoTx com/sun/ts/tests/jpa/core/derivedid/ex1b/Client.java#DIDTest_from_pmservlet com/sun/ts/tests/jpa/core/derivedid/ex1b/Client.java#DIDTest_from_puservlet com/sun/ts/tests/jpa/core/derivedid/ex1b/Client.java#DIDTest_from_stateful3 com/sun/ts/tests/jpa/core/derivedid/ex1b/Client.java#DIDTest_from_stateless3 com/sun/ts/tests/jpa/core/derivedid/ex2b/Client.java#DIDTest_from_appmanaged com/sun/ts/tests/jpa/core/derivedid/ex2b/Client.java#DIDTest_from_appmanagedNoTx com/sun/ts/tests/jpa/core/derivedid/ex2b/Client.java#DIDTest_from_pmservlet com/sun/ts/tests/jpa/core/derivedid/ex2b/Client.java#DIDTest_from_puservlet com/sun/ts/tests/jpa/core/derivedid/ex2b/Client.java#DIDTest_from_stateful3 com/sun/ts/tests/jpa/core/derivedid/ex2b/Client.java#DIDTest_from_stateless3 com/sun/ts/tests/jpa/core/derivedid/ex3a/Client.java#DIDTest_from_appmanaged com/sun/ts/tests/jpa/core/derivedid/ex3a/Client.java#DIDTest_from_appmanagedNoTx com/sun/ts/tests/jpa/core/derivedid/ex3a/Client.java#DIDTest_from_pmservlet com/sun/ts/tests/jpa/core/derivedid/ex3a/Client.java#DIDTest_from_puservlet com/sun/ts/tests/jpa/core/derivedid/ex3a/Client.java#DIDTest_from_stateful3 com/sun/ts/tests/jpa/core/derivedid/ex3a/Client.java#DIDTest_from_stateless3 com/sun/ts/tests/jpa/core/derivedid/ex3b/Client.java#DIDTest_from_appmanaged com/sun/ts/tests/jpa/core/derivedid/ex3b/Client.java#DIDTest_from_appmanagedNoTx com/sun/ts/tests/jpa/core/derivedid/ex3b/Client.java#DIDTest_from_pmservlet com/sun/ts/tests/jpa/core/derivedid/ex3b/Client.java#DIDTest_from_puservlet com/sun/ts/tests/jpa/core/derivedid/ex3b/Client.java#DIDTest_from_stateful3 com/sun/ts/tests/jpa/core/derivedid/ex3b/Client.java#DIDTest_from_stateless3

com/sun/ts/tests/jpa/core/annotations/mapsid/Client.java#persistMX1Test1_from_appmanaged com/sun/ts/tests/jpa/core/annotations/mapsid/Client.java#persistMX1Test1_from_appmanagedNoTx com/sun/ts/tests/jpa/core/annotations/mapsid/Client.java#persistMX1Test1_from_pmservlet com/sun/ts/tests/jpa/core/annotations/mapsid/Client.java#persistMX1Test1_from_puservlet com/sun/ts/tests/jpa/core/annotations/mapsid/Client.java#persistMX1Test1_from_stateful3 com/sun/ts/tests/jpa/core/annotations/mapsid/Client.java#persistMX1Test1_from_stateless3

scottmarlow commented 1 week ago

https://github.com/jakartaee/platform-tck/pull/1334 is for Persistence 3.1 + Jakarta EE 10 Platform TCK

scottmarlow commented 1 week ago

Any Persistence 3.2 test excludes should be done by updating the tckrefactor branch:

For reference see git commit 4eb2b298eaf2a954f3c7e38e1da2a8fb6c8f1b85

scottmarlow commented 4 days ago

The staged https://www.eclipse.org/downloads/download.php?file=/ee4j/jakartaee-tck/jakartaee10/staged/eftl/jakarta-persistence-tck-3.1.5.zip contains an update for this TCK challenge.