From fd93c89d9540e63058ea7c2ffc8f35865c93e712 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Tue, 12 Feb 2019 10:48:27 +0000 Subject: [PATCH 1/3] HHH-13262 - Add test for issue --- .../flush/NonTransactionalDataAccessTest.java | 73 ++++++++++--------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/test/flush/NonTransactionalDataAccessTest.java b/hibernate-core/src/test/java/org/hibernate/test/flush/NonTransactionalDataAccessTest.java index dc2592c981d4..9ecb834156e2 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/flush/NonTransactionalDataAccessTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/flush/NonTransactionalDataAccessTest.java @@ -8,20 +8,18 @@ import javax.persistence.Entity; import javax.persistence.GeneratedValue; -import javax.persistence.GenerationType; import javax.persistence.Id; +import javax.persistence.Table; import javax.persistence.TransactionRequiredException; -import java.io.Serializable; import org.hibernate.Session; import org.hibernate.cfg.AvailableSettings; import org.hibernate.cfg.Configuration; -import org.hibernate.resource.transaction.spi.TransactionStatus; - -import org.junit.Test; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.After; +import org.junit.Test; import static org.hamcrest.core.IsNot.not; import static org.hamcrest.core.IsNull.nullValue; @@ -50,21 +48,21 @@ protected void configure(Configuration configuration) { @Override protected void prepareTest() throws Exception { - try (Session s = openSession()) { - final MyEntity entity = new MyEntity( "entity" ); - s.getTransaction().begin(); - try { - s.save( entity ); - - s.getTransaction().commit(); - } - catch (Exception e) { - if ( s.getTransaction().getStatus() == TransactionStatus.ACTIVE ) { - s.getTransaction().rollback(); + final MyEntity entity = new MyEntity( "entity" ); + inTransaction( + session -> { + session.save( entity ); } - throw e; - } - } + ); + } + + @After + public void tearDown() { + inTransaction( + session -> { + session.createQuery( "delete from MyEntity" ).executeUpdate(); + } + ); } @Test @@ -82,6 +80,26 @@ public void testFlushAllowingOutOfTransactionUpdateOperations() throws Exception } } + @Test + public void testNativeQueryAllowingOutOfTransactionUpdateOperations() throws Exception { + allowUpdateOperationOutsideTransaction = "true"; + rebuildSessionFactory(); + prepareTest(); + try (Session s = openSession()) { + s.createSQLQuery( "delete from MY_ENTITY" ).executeUpdate(); + } + } + + @Test(expected = TransactionRequiredException.class) + public void testNativeQueryDisallowingOutOfTransactionUpdateOperations() throws Exception { + allowUpdateOperationOutsideTransaction = "false"; + rebuildSessionFactory(); + prepareTest(); + try (Session s = openSession()) { + s.createSQLQuery( "delete from MY_ENTITY" ).executeUpdate(); + } + } + @Test(expected = TransactionRequiredException.class) public void testFlushDisallowingOutOfTransactionUpdateOperations() throws Exception { allowUpdateOperationOutsideTransaction = "false"; @@ -113,6 +131,7 @@ public void testFlushOutOfTransaction() throws Exception { } @Entity(name = "MyEntity") + @Table(name = "MY_ENTITY") public static class MyEntity { @Id @GeneratedValue @@ -135,20 +154,4 @@ public void setName(String name) { this.name = name; } } - - public final Serializable doInsideTransaction(Session s, java.util.function.Supplier sp) { - s.getTransaction().begin(); - try { - final Serializable result = sp.get(); - - s.getTransaction().commit(); - return result; - } - catch (Exception e) { - if ( s.getTransaction().getStatus() == TransactionStatus.ACTIVE ) { - s.getTransaction().rollback(); - } - throw e; - } - } } From 9d04140fc69042553e9fe2b01f33b81b09372803 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Tue, 12 Feb 2019 11:22:34 +0000 Subject: [PATCH 2/3] HHH-13262 - javax.persistence.TransactionRequiredException: Executing an update/delete query --- .../engine/spi/SessionDelegatorBaseImpl.java | 5 +++++ .../spi/SharedSessionContractImplementor.java | 13 +++++++++++++ .../AbstractSharedSessionContract.java | 12 ++++++++++++ .../org/hibernate/internal/SessionImpl.java | 18 +++++++----------- .../procedure/internal/ProcedureCallImpl.java | 6 ++---- .../query/internal/AbstractProducedQuery.java | 9 ++------- 6 files changed, 41 insertions(+), 22 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java index 4ec317d20c59..38762d1debd4 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/SessionDelegatorBaseImpl.java @@ -156,6 +156,11 @@ public boolean isTransactionInProgress() { return delegate.isTransactionInProgress(); } + @Override + public void checkTransactionNeededForUpdateOperation(String exceptionMessage) { + delegate.checkTransactionNeededForUpdateOperation( exceptionMessage ); + } + @Override public LockOptions getLockRequest(LockModeType lockModeType, Map properties) { return delegate.getLockRequest( lockModeType, properties ); diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java index 0c3fe2ea2ceb..cb227d0c20e3 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java @@ -12,6 +12,7 @@ import java.util.List; import java.util.UUID; import javax.persistence.FlushModeType; +import javax.persistence.TransactionRequiredException; import org.hibernate.CacheMode; import org.hibernate.Criteria; @@ -180,6 +181,18 @@ default long getTimestamp() { */ boolean isTransactionInProgress(); + /** + * Check if an active Transaction is necessary for the update operation to be executed. + * If an active Transaction is necessary but it is not then a TransactionRequiredException is raised. + * + * @param exceptionMessage, the message to use for the TransactionRequiredException + */ + default void checkTransactionNeededForUpdateOperation(String exceptionMessage) { + if ( !isTransactionInProgress() ) { + throw getExceptionConverter().convert( new TransactionRequiredException( exceptionMessage ) ); + } + } + /** * Provides access to the underlying transaction or creates a new transaction if * one does not already exist or is active. This is primarily for internal or diff --git a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java index ac3d5da8acf5..a4b6a5c1f114 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java @@ -15,6 +15,7 @@ import java.util.TimeZone; import java.util.UUID; import javax.persistence.FlushModeType; +import javax.persistence.TransactionRequiredException; import javax.persistence.Tuple; import org.hibernate.AssertionFailure; @@ -128,6 +129,7 @@ public abstract class AbstractSharedSessionContract implements SharedSessionCont protected boolean closed; protected boolean waitingForAutoClose; + private transient boolean disallowOutOfTransactionUpdateOperations; // transient & non-final for Serialization purposes - ugh private transient SessionEventListenerManagerImpl sessionEventsManager = new SessionEventListenerManagerImpl(); @@ -141,6 +143,7 @@ public abstract class AbstractSharedSessionContract implements SharedSessionCont public AbstractSharedSessionContract(SessionFactoryImpl factory, SessionCreationOptions options) { this.factory = factory; this.cacheTransactionSync = factory.getCache().getRegionFactory().createTransactionContext( this ); + this.disallowOutOfTransactionUpdateOperations = !factory.getSessionFactoryOptions().isAllowOutOfTransactionUpdateOperations(); this.flushMode = options.getInitialSessionFlushMode(); @@ -389,6 +392,13 @@ public boolean isTransactionInProgress() { return !isClosed() && transactionCoordinator.isTransactionActive(); } + @Override + public void checkTransactionNeededForUpdateOperation(String exceptionMessage) { + if ( disallowOutOfTransactionUpdateOperations && !isTransactionInProgress() ) { + throw getExceptionConverter().convert( new TransactionRequiredException( exceptionMessage ) ); + } + } + @Override public Transaction getTransaction() throws HibernateException { if ( getFactory().getSessionFactoryOptions().getJpaCompliance().isJpaTransactionComplianceEnabled() ) { @@ -1133,5 +1143,7 @@ private void readObject(ObjectInputStream ois) throws IOException, ClassNotFound entityNameResolver = new CoordinatingEntityNameResolver( factory, interceptor ); exceptionConverter = new ExceptionConverterImpl( this ); + this.disallowOutOfTransactionUpdateOperations = !getFactory().getSessionFactoryOptions().isAllowOutOfTransactionUpdateOperations(); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java index de441c2b044b..9a5b59b601ca 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -238,7 +238,6 @@ public final class SessionImpl private transient int dontFlushFromFind; - private transient boolean disallowOutOfTransactionUpdateOperations; private transient ExceptionMapper exceptionMapper; private transient ManagedFlushChecker managedFlushChecker; @@ -262,7 +261,7 @@ public SessionImpl(SessionFactoryImpl factory, SessionCreationOptions options) { this.autoClear = options.shouldAutoClear(); this.autoClose = options.shouldAutoClose(); this.queryParametersValidationEnabled = options.isQueryParametersValidationEnabled(); - this.disallowOutOfTransactionUpdateOperations = !factory.getSessionFactoryOptions().isAllowOutOfTransactionUpdateOperations(); + this.discardOnClose = getFactory().getSessionFactoryOptions().isReleaseResourcesOnCloseEnabled(); if ( options instanceof SharedSessionCreationOptions && ( (SharedSessionCreationOptions) options ).isTransactionCoordinatorShared() ) { @@ -1441,7 +1440,7 @@ public void flush() throws HibernateException { } private void doFlush() { - checkTransactionNeeded(); + checkTransactionNeededForUpdateOperation(); checkTransactionSynchStatus(); try { @@ -3474,7 +3473,7 @@ public T find(Class entityClass, Object primaryKey, LockModeType lockMode if ( lockModeType != null ) { if ( !LockModeType.NONE.equals( lockModeType) ) { - checkTransactionNeeded(); + checkTransactionNeededForUpdateOperation(); } lockOptions = buildLockOptions( lockModeType, properties ); loadAccess.with( lockOptions ); @@ -3547,10 +3546,8 @@ private CacheStoreMode determineCacheStoreMode(Map settings) { return ( CacheStoreMode ) settings.get( JPA_SHARED_CACHE_STORE_MODE ); } - private void checkTransactionNeeded() { - if ( disallowOutOfTransactionUpdateOperations && !isTransactionInProgress() ) { - throw new TransactionRequiredException( "no transaction is in progress" ); - } + private void checkTransactionNeededForUpdateOperation() { + checkTransactionNeededForUpdateOperation( "no transaction is in progress" ); } @Override @@ -3576,7 +3573,7 @@ public void lock(Object entity, LockModeType lockModeType) { @Override public void lock(Object entity, LockModeType lockModeType, Map properties) { checkOpen(); - checkTransactionNeeded(); + checkTransactionNeededForUpdateOperation(); if ( !contains( entity ) ) { throw new IllegalArgumentException( "entity not in the persistence context" ); @@ -3618,7 +3615,7 @@ public void refresh(Object entity, LockModeType lockModeType, Map R uniqueElement(List list) throws NonUniqueResultException @Override public int executeUpdate() throws HibernateException { - if ( ! getProducer().isTransactionInProgress() ) { - throw getProducer().getExceptionConverter().convert( - new TransactionRequiredException( - "Executing an update/delete query" - ) - ); - } + getProducer().checkTransactionNeededForUpdateOperation( "Executing an update/delete query" ); + beforeQuery(); try { return doExecuteUpdate(); From a3bd66c0a43d5ea3eaa1551579ea482adf201e17 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Sun, 24 Feb 2019 17:01:05 -0800 Subject: [PATCH 3/3] HHH-13262 - javax.persistence.TransactionRequiredException: Executing an update/delete query --- .../hibernate/engine/spi/SharedSessionContractImplementor.java | 2 +- .../org/hibernate/internal/AbstractSharedSessionContract.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java index cb227d0c20e3..d0db9689f6e0 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/SharedSessionContractImplementor.java @@ -189,7 +189,7 @@ default long getTimestamp() { */ default void checkTransactionNeededForUpdateOperation(String exceptionMessage) { if ( !isTransactionInProgress() ) { - throw getExceptionConverter().convert( new TransactionRequiredException( exceptionMessage ) ); + throw new TransactionRequiredException( exceptionMessage ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java index a4b6a5c1f114..ee523b802dea 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java @@ -395,7 +395,7 @@ public boolean isTransactionInProgress() { @Override public void checkTransactionNeededForUpdateOperation(String exceptionMessage) { if ( disallowOutOfTransactionUpdateOperations && !isTransactionInProgress() ) { - throw getExceptionConverter().convert( new TransactionRequiredException( exceptionMessage ) ); + throw new TransactionRequiredException( exceptionMessage ); } }