Remove outdated JpaExceptionTranslatorAspect [SPR-15047]

[spring-projects-issues]

pas filip opened SPR-15047 and commented

Hello,

I've recently upgraded to hibernate 5 and seem to have some issues receiving the correct translated exception when using spring's JpaExceptionTranslatorAspect.
Inside of the JpaExceptionTranslatorAspect the exception translation is immediately done by
EntityManagerFactoryUtils.convertJpaAccessExceptionIfPossible which doesn't seem to take into account the fix implemented in jira #19024.

@@ -43,13 +48,16 @@ public DataAccessException translateExceptionIfPossible(RuntimeException ex) {
 		if (ex instanceof HibernateException) {
 			return convertHibernateAccessException((HibernateException) ex);
 		}
-		return null;
+		if (ex instanceof PersistenceException && ex.getCause() instanceof HibernateException) {
+			return convertHibernateAccessException((HibernateException) ex.getCause());
+		}
+		return EntityManagerFactoryUtils.convertJpaAccessExceptionIfPossible(ex);
 	}

So instead of receiving a org.springframework.dao.DataIntegrityViolationException we receive a JpaSystemException which contains the correct exception however it's not being translated.

org.springframework.orm.jpa.JpaSystemException: org.hibernate.exception.ConstraintViolationException: could not execute batch; nested exception is javax.persistence.PersistenceException: org.hibernate.exception.ConstraintViolationException: could not execute batch

	at org.springframework.orm.jpa.EntityManagerFactoryUtils.convertJpaAccessExceptionIfPossible(EntityManagerFactoryUtils.java:418)
	at org.springframework.orm.jpa.aspectj.JpaExceptionTranslatorAspect.ajc$afterThrowing$org_springframework_orm_jpa_aspectj_JpaExceptionTranslatorAspect$1$18a1ac9(JpaExceptionTranslatorAspect.aj:37)
	..
	..
	at org.springframework.transaction.aspectj.AbstractTransactionAspect.ajc$around$org_springframework_transaction_aspectj_AbstractTransactionAspect$1$2a73e96cproceed(AbstractTransactionAspect.aj:66)
	at org.springframework.transaction.aspectj.AbstractTransactionAspect$AbstractTransactionAspect$1.proceedWithInvocation(AbstractTransactionAspect.aj:72)
	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:281)
	at org.springframework.transaction.aspectj.AbstractTransactionAspect.ajc$around$org_springframework_transaction_aspectj_AbstractTransactionAspect$1$2a73e96c(AbstractTransactionAspect.aj:70)
	...
Caused by: javax.persistence.PersistenceException: org.hibernate.exception.ConstraintViolationException: could not execute batch
	at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:147)
	at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:155)
	at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:162)
	at org.hibernate.internal.SessionImpl.doFlush(SessionImpl.java:1413)
	at org.hibernate.internal.SessionImpl.flush(SessionImpl.java:1393)
	...
	at org.springframework.orm.jpa.SharedEntityManagerCreator$SharedEntityManagerInvocationHandler.invoke(SharedEntityManagerCreator.java:298)
	at com.sun.proxy.$Proxy115.flush(Unknown Source)
	... 42 more
Caused by: org.hibernate.exception.ConstraintViolationException: could not execute batch
	at org.hibernate.exception.internal.SQLStateConversionDelegate.convert(SQLStateConversionDelegate.java:112)
	at org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:42)
	at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:111)
	at org.hibernate.engine.jdbc.batch.internal.BatchingBatch.performExecution(BatchingBatch.java:121)
	at org.hibernate.engine.jdbc.batch.internal.BatchingBatch.doExecuteBatch(BatchingBatch.java:97)
	at org.hibernate.engine.jdbc.batch.internal.AbstractBatchImpl.execute(AbstractBatchImpl.java:147)
	at org.hibernate.engine.jdbc.internal.JdbcCoordinatorImpl.getBatch(JdbcCoordinatorImpl.java:195)
	at org.hibernate.persister.entity.AbstractEntityPersister.insert(AbstractEntityPersister.java:2914)
	at org.hibernate.persister.entity.AbstractEntityPersister.insert(AbstractEntityPersister.java:3434)
	at org.hibernate.action.internal.EntityInsertAction.execute(EntityInsertAction.java:89)
	at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:582)
	at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:456)
	at org.hibernate.event.internal.AbstractFlushingEventListener.performExecutions(AbstractFlushingEventListener.java:337)
	at org.hibernate.event.internal.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:39)
	at org.hibernate.internal.SessionImpl.doFlush(SessionImpl.java:1407)
	... 49 more
Caused by: java.sql.BatchUpdateException: ORA-00001: unique constraint (PHOENIX_DEV.SYS_C0061089_CUST) violated

	at oracle.jdbc.driver.OraclePreparedStatement.executeBatch(OraclePreparedStatement.java:12296)
	at oracle.jdbc.driver.OracleStatementWrapper.executeBatch(OracleStatementWrapper.java:246)
	at com.zaxxer.hikari.pool.ProxyStatement.executeBatch(ProxyStatement.java:128)
	at com.zaxxer.hikari.pool.HikariProxyPreparedStatement.executeBatch(HikariProxyPreparedStatement.java)
	at org.hibernate.engine.jdbc.batch.internal.BatchingBatch.performExecution(BatchingBatch.java:111)
	... 60 more

Also would it be possible to perhaps adapt the JpaExceptionTranslatorAspect to delegate to a managed bean implementing org.springframework.dao.support.PersistenceExceptionTranslator
This could be a nice enhancement since the JpaExceptionTranslatorAspect is included with all the other spring aspects in the same jar and for weaving purposes simplifies things
as it should be possible to plug-in for example a translator that simply re-throws the exception if no translation is desired even though the aspect has been woven.

I can't entirely exclude that somehow there is an issue with my hibernate configuration but it seems the PersistenceExceptionTranslator implemented by the EntityManagerFactoryBean
isn't actually called at all which results in an incorrect translation.
Currently the only workaround I see, that seems to work, is to implement another aspect that implements the translation as specified in jira issue #19024.

Also seems related to jira issue #19026.

---

Issue Links:

• Flush exception translation not working anymore with Hibernate 5.2 [SPR-14457] #19026 Flush exception translation not working anymore with Hibernate 5.2
• Hibernate5 LocalSessionFactoryBean does not translate javax.persistence exceptions [SPR-14455] #19024 Hibernate5 LocalSessionFactoryBean does not translate javax.persistence exceptions

Referenced from: commits cff311b

[spring-projects-issues]

pas filip commented

Juergen,

I've noticed you changed the type of issue to an improvement however for me this is clearly a bug.
However the 2nd part of my request would qualify as an improvement....

Filip

[spring-projects-issues]

Juergen Hoeller commented

It isn't quite a traditional Spring bug since the actual regression is caused by Hibernate 5.2's refactoring of its exception hierarchy, and Hibernate 5.2 is very recent still. Anyway, I've changed the type back to bug.

[spring-projects-issues]

Juergen Hoeller commented

So previously, you received native Hibernate exceptions untranslated, and now you're receiving them within a JpaSystemException since that algorithm detects them as PersistenceException subclasses? Is this the actual regression when you're using Hibernate 5.2 there?

I wonder whether we should keep the JpaExceptionTranslatorAspect at all. From my perspective, it's an outdated class, and I wouldn't recommend the approach to begin with. I'm inclined to simply remove it in Spring Framework 5.0.

[spring-projects-issues]

pas filip commented

It seems I was a little too quick in creating this defect.
The jpa translating aspect is not the cause of the regression I experienced.
The code inside of the aspect hasn't been updated for a while it seems.
It seems to be related to some changes that were done in a test that we own.
(Originally test used transaction template which results in a correct translation of the exception, was then moved to a Transactional test and a flush was added and there the regression was introduced.)

Sorry for the trouble!

About eliminating this aspect seems like a good option for me to remove it since:

• I don't rely on it in my current project.
• The fact that it doesn't take into account any provider specific exceptions.
• Behaviour seems inconsistent depeding on wether the exception gets translated by the aspect as opposed to the persistence exception translator. (which is setup with the right dialect.)

[spring-projects-issues]

Juergen Hoeller commented

Alright, I'll simply remove the aspect in 5.0 for good then.