spring-projects / spring-data-jpa

Simplifies the development of creating a JPA-based data access layer.
https://spring.io/projects/spring-data-jpa/
Apache License 2.0
2.98k stars 1.41k forks source link

3.2.x Auditing behavior fails with composite keys using @IdClass but succeeds with @Embeddable #3464

Closed imuhdork closed 4 months ago

imuhdork commented 4 months ago

Using @CreatedDate and @CreatedBy along with AuditingEntityListener and an AuditorAware Bean fails with appropriately annotated entities, composite keys, and repository interfaces when you using @IdClass pattern but succeeds when using the @Embeddable/@EmbeddedId pattern. Both patterns was working in spring data 3.1.x but stopped working in 3.2.x.

I attached a 3.1.6 version and a 3.2.5 version of a small maven style project recreating the issue. In the attached there are two key tests. Both tests pass in the 3.1.6 version of the attached project. The test using an @Embeddable composite key with an @EmbeddedId property of an entity passes in the 3.2.5 version without exception and the @CreatedBy and @CreatedDate are correctly persisted in the in-memory db. The test using an @IdClass annotated entity with a composite key class fails in the 3.2.5 version with the following exception:

com.imuhdork.spring.data.IdclassVsEmbeddableBugApplicationTests.testIdClassFooRepository -- Time elapsed: 0.136 s <<< FAILURE!
org.opentest4j.AssertionFailedError: Unexpected exception thrown: org.springframework.orm.jpa.JpaSystemException: identifier of an instance of com.imuhdork.spring.data.IdClassFoo was altered from IdClassFooCompositeKey(idClassFooId=0, createdBy=ANONYMOUS, createdDate=2024-05-07T15:10:37.356646) to IdClassFooCompositeKey(idClassFooId=0, createdBy=null, createdDate=null)
    at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
    at org.junit.jupiter.api.AssertDoesNotThrow.createAssertionFailedError(AssertDoesNotThrow.java:84)
    at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:75)
    at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:58)
    at org.junit.jupiter.api.Assertions.assertDoesNotThrow(Assertions.java:3228)
    at com.imuhdork.spring.data.IdclassVsEmbeddableBugApplicationTests.testIdClassFooRepository(IdclassVsEmbeddableBugApplicationTests.java:24)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: org.springframework.orm.jpa.JpaSystemException: identifier of an instance of com.imuhdork.spring.data.IdClassFoo was altered from IdClassFooCompositeKey(idClassFooId=0, createdBy=ANONYMOUS, createdDate=2024-05-07T15:10:37.356646) to IdClassFooCompositeKey(idClassFooId=0, createdBy=null, createdDate=null)
    at org.springframework.orm.jpa.vendor.HibernateJpaDialect.convertHibernateAccessException(HibernateJpaDialect.java:341)
    at org.springframework.orm.jpa.vendor.HibernateJpaDialect.translateExceptionIfPossible(HibernateJpaDialect.java:241)
    at org.springframework.orm.jpa.AbstractEntityManagerFactoryBean.translateExceptionIfPossible(AbstractEntityManagerFactoryBean.java:550)
    at org.springframework.dao.support.ChainedPersistenceExceptionTranslator.translateExceptionIfPossible(ChainedPersistenceExceptionTranslator.java:61)
    at org.springframework.dao.support.DataAccessUtils.translateIfNecessary(DataAccessUtils.java:335)
    at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:152)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
    at org.springframework.data.jpa.repository.support.CrudMethodMetadataPostProcessor$CrudMethodMetadataPopulatingMethodInterceptor.invoke(CrudMethodMetadataPostProcessor.java:164)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
    at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:223)
    at com.imuhdork.spring.data.$Proxy113.saveAndFlush(Unknown Source)
    at com.imuhdork.spring.data.IdclassVsEmbeddableBugApplicationTests.lambda$testIdClassFooRepository$0(IdclassVsEmbeddableBugApplicationTests.java:24)
    at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:71)
    ... 6 more
Caused by: org.hibernate.HibernateException: identifier of an instance of com.imuhdork.spring.data.IdClassFoo was altered from IdClassFooCompositeKey(idClassFooId=0, createdBy=ANONYMOUS, createdDate=2024-05-07T15:10:37.356646) to IdClassFooCompositeKey(idClassFooId=0, createdBy=null, createdDate=null)
    at org.hibernate.event.internal.DefaultFlushEntityEventListener.checkId(DefaultFlushEntityEventListener.java:95)
    at org.hibernate.event.internal.DefaultFlushEntityEventListener.getValues(DefaultFlushEntityEventListener.java:179)
    at org.hibernate.event.internal.DefaultFlushEntityEventListener.onFlushEntity(DefaultFlushEntityEventListener.java:138)
    at org.hibernate.event.service.internal.EventListenerGroupImpl.fireEventOnEachListener(EventListenerGroupImpl.java:127)
    at org.hibernate.event.internal.AbstractFlushingEventListener.flushEntities(AbstractFlushingEventListener.java:226)
    at org.hibernate.event.internal.AbstractFlushingEventListener.flushEverythingToExecutions(AbstractFlushingEventListener.java:90)
    at org.hibernate.event.internal.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:40)
    at org.hibernate.event.service.internal.EventListenerGroupImpl.fireEventOnEachListener(EventListenerGroupImpl.java:127)
    at org.hibernate.internal.SessionImpl.doFlush(SessionImpl.java:1403)
    at org.hibernate.internal.SessionImpl.flush(SessionImpl.java:1389)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.springframework.orm.jpa.SharedEntityManagerCreator$SharedEntityManagerInvocationHandler.invoke(SharedEntityManagerCreator.java:319)
    at jdk.proxy2/jdk.proxy2.$Proxy108.flush(Unknown Source)
    at org.springframework.data.jpa.repository.support.SimpleJpaRepository.flush(SimpleJpaRepository.java:663)
    at org.springframework.data.jpa.repository.support.SimpleJpaRepository.saveAndFlush(SimpleJpaRepository.java:630)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:354)
    at org.springframework.data.repository.core.support.RepositoryMethodInvoker$RepositoryFragmentMethodInvoker.lambda$new$0(RepositoryMethodInvoker.java:277)
    at org.springframework.data.repository.core.support.RepositoryMethodInvoker.doInvoke(RepositoryMethodInvoker.java:170)
    at org.springframework.data.repository.core.support.RepositoryMethodInvoker.invoke(RepositoryMethodInvoker.java:158)
    at org.springframework.data.repository.core.support.RepositoryComposition$RepositoryFragments.invoke(RepositoryComposition.java:516)
    at org.springframework.data.repository.core.support.RepositoryComposition.invoke(RepositoryComposition.java:285)
    at org.springframework.data.repository.core.support.RepositoryFactorySupport$ImplementationMethodExecutionInterceptor.invoke(RepositoryFactorySupport.java:628)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
    at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.doInvoke(QueryExecutorMethodInterceptor.java:168)
    at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.invoke(QueryExecutorMethodInterceptor.java:143)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
    at org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.invoke(DefaultMethodInvokingMethodInterceptor.java:70)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
    at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:123)
    at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:392)
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:119)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
    at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:137)
    ... 15 more

I am not 100% sure where exactly the issue is occurring but I think it is in spring data handling of audited fields as I would expect both of these styles to behave similarly with respect to auditing? Additionally, the fact that both tests pass in 3.1.6 one test fails in 3.2.5 confirms that auditing behavior worked at one point regardless of the style used to configure the composite key.

mp911de commented 4 months ago

Spring Data JPA uses a @PrePersist/@PreUpdate lifecycle hook via AuditingEntityListener. While I wasn't immediately able to find any JPA rules that would prevent updating the identifier in a lifecycle method, I can well imagine why updating an identifier isn't a great idea.

Spring Data JPA has upgraded with 3.2.x to Hibernate 6.4 whereas Spring Data 3.1.x used Hibernate 6.2.

Embeddables aren't necessarily identifiers and the check that raises the exception relies on the implementation of equals/hashCode. The exception comes from within Hibernate so I suggest that you create a reproducer using purely Hibernate API for @PrePersist updating an identifier (id-class) and report it against Hibernate.

imuhdork commented 4 months ago

Putting this here in case I get nowhere with hibernate. I was able to reproduce the issue removing auditing completely and just using @PrePersist. The @PrePersist method and tests work fine in 6.2.21.Final and the @IdClass test case fails in 6.2.22.Final.

It appears that between 6.2.21.Final and 6.2.22.Final the composite key @Id values set in a @PrePersist method on an @Entity get overwritten by hibernate. In 6.2.21.Final the tests pass, in 6.2.21.Final the @IdClass test fails because the @Id values are null even though the are set in the @PrePersist method. It is only when we get to 6.3.1.Final when the HibernateException is thrown.

Going even further, not only are the @Id properties overwritten but it appears that @PrePersist gets called twice in 6.2.22.Final and beyond in the @IdClass test case and only once in the @Embeddable test case.

imuhdork commented 4 months ago

Created hibernate defect and test case template compliant repo.