We're updating the issue view to help you get more done. 

EntityManager.find() should only check for roll-back-only condition if there is an active JTA transaction, otherwise ORM should throw convert( e, lockOptions )

Description

added a check for roll-back-only condition, however, we should first check for active JTA transaction. The reason is that when Hibernate ORM calls accessTransaction().getRollbackOnly(), an ISE exception is immediately thrown by org.hibernate.engine.transaction.internal.TransactionImpl.getRollbackOnly, which is confusing to users, as they called EntityManager.find(), which shouldn't throw "IllegalStateException: JPA compliance dictates throwing IllegalStateException when #getRollbackOnly is called on non-active transaction", as the application didn't call #getRollbackOnly.

I suggest a change from:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 public <T> T find(Class<T> entityClass, Object primaryKey, LockModeType lockModeType, Map<String, Object> properties) { checkOpen(); LockOptions lockOptions = null; try { getLoadQueryInfluencers().getEffectiveEntityGraph().applyConfiguredGraph( properties ); final IdentifierLoadAccess<T> loadAccess = byId( entityClass ); loadAccess.with( determineAppropriateLocalCacheMode( properties ) ); if ( lockModeType != null ) { if ( !LockModeType.NONE.equals( lockModeType) ) { checkTransactionNeededForUpdateOperation(); } lockOptions = buildLockOptions( lockModeType, properties ); loadAccess.with( lockOptions ); } return loadAccess.load( (Serializable) primaryKey ); } catch ( EntityNotFoundException ignored ) { // DefaultLoadEventListener.returnNarrowedProxy may throw ENFE (see HHH-7861 for details), // which find() should not throw. Find() should return null if the entity was not found. if ( log.isDebugEnabled() ) { String entityName = entityClass != null ? entityClass.getName(): null; String identifierValue = primaryKey != null ? primaryKey.toString() : null ; log.ignoringEntityNotFound( entityName, identifierValue ); } return null; } catch ( ObjectDeletedException e ) { //the spec is silent about people doing remove() find() on the same PC return null; } catch ( ObjectNotFoundException e ) { //should not happen on the entity itself with get throw new IllegalArgumentException( e.getMessage(), e ); } catch ( MappingException | TypeMismatchException | ClassCastException e ) { throw exceptionConverter.convert( new IllegalArgumentException( e.getMessage(), e ) ); } catch ( JDBCException e ) { if ( accessTransaction().getRollbackOnly() ) { // assume this is the similar to the WildFly / IronJacamar "feature" described under HHH-12472 return null; } else { throw exceptionConverter.convert( e, lockOptions ); } } catch ( RuntimeException e ) { throw exceptionConverter.convert( e, lockOptions ); } finally { getLoadQueryInfluencers().getEffectiveEntityGraph().clear(); } }

Change instead to:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 public <T> T find(Class<T> entityClass, Object primaryKey, LockModeType lockModeType, Map<String, Object> properties) { checkOpen(); LockOptions lockOptions = null; try { getLoadQueryInfluencers().getEffectiveEntityGraph().applyConfiguredGraph( properties ); final IdentifierLoadAccess<T> loadAccess = byId( entityClass ); loadAccess.with( determineAppropriateLocalCacheMode( properties ) ); if ( lockModeType != null ) { if ( !LockModeType.NONE.equals( lockModeType) ) { checkTransactionNeededForUpdateOperation(); } lockOptions = buildLockOptions( lockModeType, properties ); loadAccess.with( lockOptions ); } return loadAccess.load( (Serializable) primaryKey ); } catch ( EntityNotFoundException ignored ) { // DefaultLoadEventListener.returnNarrowedProxy may throw ENFE (see HHH-7861 for details), // which find() should not throw. Find() should return null if the entity was not found. if ( log.isDebugEnabled() ) { String entityName = entityClass != null ? entityClass.getName(): null; String identifierValue = primaryKey != null ? primaryKey.toString() : null ; log.ignoringEntityNotFound( entityName, identifierValue ); } return null; } catch ( ObjectDeletedException e ) { //the spec is silent about people doing remove() find() on the same PC return null; } catch ( ObjectNotFoundException e ) { //should not happen on the entity itself with get throw new IllegalArgumentException( e.getMessage(), e ); } catch ( MappingException | TypeMismatchException | ClassCastException e ) { throw exceptionConverter.convert( new IllegalArgumentException( e.getMessage(), e ) ); } catch ( JDBCException e ) { // Note one line change is below. if ( accessTransaction().isActive() && accessTransaction().getRollbackOnly() && accessTransaction().getRollbackOnly() ) { // assume this is the similar to the WildFly / IronJacamar "feature" described under HHH-12472 return null; } else { throw exceptionConverter.convert( e, lockOptions ); } } catch ( RuntimeException e ) { throw exceptionConverter.convert( e, lockOptions ); } finally { getLoadQueryInfluencers().getEffectiveEntityGraph().clear(); } }

I'll try the above change locally and check if the current ORM testsuite still passes (on ORM master branch).

Environment

None

Status

Assignee

Scott Marlow

Reporter

Scott Marlow

Fix versions

Labels

None

backPortable

None

Suitable for new contributors

None

Requires Release Note

None

Pull Request

None

backportDecision

None

Affects versions

5.3.10
5.3.9

Priority

Minor