From 8655d683e68fc335c74affd71f48d627bdb846dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 15 Oct 2019 14:29:25 +0200 Subject: [PATCH 1/5] HHH-13666 Remove some dead code in tests related to transactions --- .../test/java/org/hibernate/test/tm/AfterCompletionTest.java | 4 ---- .../org/hibernate/test/tm/BeforeCompletionFailureTest.java | 4 ---- 2 files changed, 8 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/test/tm/AfterCompletionTest.java b/hibernate-core/src/test/java/org/hibernate/test/tm/AfterCompletionTest.java index b604577021..465fb0913e 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/tm/AfterCompletionTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/tm/AfterCompletionTest.java @@ -98,10 +98,6 @@ public class AfterCompletionTest extends BaseNonConfigCoreFunctionalTestCase { } } - private void registerAfterCallbackCompletionHandler(Session session) { - ( (SessionImplementor) session ).getActionQueue().registerProcess( new AfterCallbackCompletionHandler() ); - } - public static class BeforeCallbackCompletionHandler implements BeforeTransactionCompletionProcess { @Override public void doBeforeTransactionCompletion(SessionImplementor session) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/tm/BeforeCompletionFailureTest.java b/hibernate-core/src/test/java/org/hibernate/test/tm/BeforeCompletionFailureTest.java index 4b2643e902..249d1a07b9 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/tm/BeforeCompletionFailureTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/tm/BeforeCompletionFailureTest.java @@ -118,10 +118,6 @@ public class BeforeCompletionFailureTest extends BaseNonConfigCoreFunctionalTest return new SimpleEntity( id, "key", "name" ); } - private void runTest() { - - } - @Entity(name = "SimpleEntity") public static class SimpleEntity { @Id From 250f56933925d8b1b41f1d41f4001762cbd785b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 15 Oct 2019 14:32:02 +0200 Subject: [PATCH 2/5] HHH-13666 Clarify that existing BeforeCompletion/AfterCompletion tests are about JTA only --- .../{AfterCompletionTest.java => JtaAfterCompletionTest.java} | 2 +- ...onFailureTest.java => JtaBeforeCompletionFailureTest.java} | 2 +- .../test/integration/jta/JtaTransactionAfterCallbackTest.java | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) rename hibernate-core/src/test/java/org/hibernate/test/tm/{AfterCompletionTest.java => JtaAfterCompletionTest.java} (98%) rename hibernate-core/src/test/java/org/hibernate/test/tm/{BeforeCompletionFailureTest.java => JtaBeforeCompletionFailureTest.java} (97%) diff --git a/hibernate-core/src/test/java/org/hibernate/test/tm/AfterCompletionTest.java b/hibernate-core/src/test/java/org/hibernate/test/tm/JtaAfterCompletionTest.java similarity index 98% rename from hibernate-core/src/test/java/org/hibernate/test/tm/AfterCompletionTest.java rename to hibernate-core/src/test/java/org/hibernate/test/tm/JtaAfterCompletionTest.java index 465fb0913e..8145a6df1d 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/tm/AfterCompletionTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/tm/JtaAfterCompletionTest.java @@ -37,7 +37,7 @@ import static org.junit.Assert.assertTrue; /** * @author Chris Cranford */ -public class AfterCompletionTest extends BaseNonConfigCoreFunctionalTestCase { +public class JtaAfterCompletionTest extends BaseNonConfigCoreFunctionalTestCase { @Override protected Class[] getAnnotatedClasses() { return new Class[] { SimpleEntity.class }; diff --git a/hibernate-core/src/test/java/org/hibernate/test/tm/BeforeCompletionFailureTest.java b/hibernate-core/src/test/java/org/hibernate/test/tm/JtaBeforeCompletionFailureTest.java similarity index 97% rename from hibernate-core/src/test/java/org/hibernate/test/tm/BeforeCompletionFailureTest.java rename to hibernate-core/src/test/java/org/hibernate/test/tm/JtaBeforeCompletionFailureTest.java index 249d1a07b9..30c256787f 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/tm/BeforeCompletionFailureTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/tm/JtaBeforeCompletionFailureTest.java @@ -34,7 +34,7 @@ import static org.junit.Assert.fail; /** * @author Steve Ebersole */ -public class BeforeCompletionFailureTest extends BaseNonConfigCoreFunctionalTestCase { +public class JtaBeforeCompletionFailureTest extends BaseNonConfigCoreFunctionalTestCase { @Override protected Class[] getAnnotatedClasses() { diff --git a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/jta/JtaTransactionAfterCallbackTest.java b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/jta/JtaTransactionAfterCallbackTest.java index 26a30771e7..4380fe4d05 100644 --- a/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/jta/JtaTransactionAfterCallbackTest.java +++ b/hibernate-envers/src/test/java/org/hibernate/envers/test/integration/jta/JtaTransactionAfterCallbackTest.java @@ -31,6 +31,8 @@ import org.hibernate.testing.TestForIssue; import org.hibernate.testing.jta.TestingJtaBootstrap; import org.hibernate.testing.jta.TestingJtaPlatformImpl; +import org.hibernate.test.tm.JtaAfterCompletionTest; + import static org.hibernate.testing.junit4.ExtraAssertions.assertTyping; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -39,7 +41,7 @@ import static org.junit.Assert.assertTrue; /** * An envers specific quest that verifies the {@link AuditProcessManager} gets flushed. * - * There is a similar test called {@link org.hibernate.test.tm.AfterCompletionTest} + * There is a similar test called {@link JtaAfterCompletionTest} * in hibernate-core which verifies that the callbacks fires. * * The premise behind this test is to verify that when a JTA transaction is aborted by From f9c149ab0db9215d2fc45837b67b227c0bb5b77b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 15 Oct 2019 15:04:13 +0200 Subject: [PATCH 3/5] HHH-13666 Throw a HibernateException with a more appropriate message upon beforeTransactionCompletion/afterTransactionCompletion failure --- .../src/main/java/org/hibernate/engine/spi/ActionQueue.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java index 8b7c3698f4..66bdaf380e 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java @@ -957,7 +957,7 @@ public class ActionQueue { throw he; } catch (Exception e) { - throw new AssertionFailure( "Unable to perform beforeTransactionCompletion callback", e ); + throw new HibernateException( "Unable to perform beforeTransactionCompletion callback", e ); } } } @@ -987,7 +987,7 @@ public class ActionQueue { // continue loop } catch (Exception e) { - throw new AssertionFailure( "Exception releasing cache locks", e ); + throw new HibernateException( "Unable to perform afterTransactionCompletion callback", e ); } } From 5c8169ba7cd81b5ab9ac282bd5c596f25c7d7f9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 15 Oct 2019 15:05:52 +0200 Subject: [PATCH 4/5] HHH-13666 Re-use the wrapped exception's message upon beforeTransactionCompletion/afterTransactionCompletion failure For convenience. --- .../src/main/java/org/hibernate/engine/spi/ActionQueue.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java index 66bdaf380e..6272656aff 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java @@ -957,7 +957,7 @@ public class ActionQueue { throw he; } catch (Exception e) { - throw new HibernateException( "Unable to perform beforeTransactionCompletion callback", e ); + throw new HibernateException( "Unable to perform beforeTransactionCompletion callback: " + e.getMessage(), e ); } } } @@ -987,7 +987,7 @@ public class ActionQueue { // continue loop } catch (Exception e) { - throw new HibernateException( "Unable to perform afterTransactionCompletion callback", e ); + throw new HibernateException( "Unable to perform afterTransactionCompletion callback: " + e.getMessage(), e ); } } From 6cdb0256d41e873b67a8a6ce40a6d1ee5a4b6599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 15 Oct 2019 15:06:48 +0200 Subject: [PATCH 5/5] HHH-13666 Test custom BeforeTransactionCompletionProcess/AfterTransactionCompletionProcess --- .../CustomAfterCompletionTest.java | 133 ++++++++++++++++++ .../CustomBeforeCompletionTest.java | 133 ++++++++++++++++++ 2 files changed, 266 insertions(+) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/actionqueue/CustomAfterCompletionTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/actionqueue/CustomBeforeCompletionTest.java diff --git a/hibernate-core/src/test/java/org/hibernate/test/actionqueue/CustomAfterCompletionTest.java b/hibernate-core/src/test/java/org/hibernate/test/actionqueue/CustomAfterCompletionTest.java new file mode 100644 index 0000000000..9806c82066 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/actionqueue/CustomAfterCompletionTest.java @@ -0,0 +1,133 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.actionqueue; + +import java.util.concurrent.atomic.AtomicBoolean; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + +import org.hibernate.HibernateException; +import org.hibernate.action.spi.AfterTransactionCompletionProcess; +import org.hibernate.engine.spi.SharedSessionContractImplementor; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.Assert; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + +public class CustomAfterCompletionTest extends BaseCoreFunctionalTestCase { + + @Test + @TestForIssue(jiraKey = "HHH-13666") + public void success() { + inSession( session -> { + AtomicBoolean called = new AtomicBoolean( false ); + session.getActionQueue().registerProcess( new AfterTransactionCompletionProcess() { + @Override + public void doAfterTransactionCompletion(boolean success, SharedSessionContractImplementor session) { + called.set( true ); + } + } ); + Assert.assertFalse( called.get() ); + inTransaction( session, theSession -> { + theSession.persist( new SimpleEntity( "jack" ) ); + } ); + Assert.assertTrue( called.get() ); + } ); + + // Check that the transaction was committed + inTransaction( session -> { + long count = session.createQuery( "select count(*) from SimpleEntity", Long.class ) + .uniqueResult(); + assertEquals( 1L, count ); + } ); + } + + @Test + @TestForIssue(jiraKey = "HHH-13666") + public void failure() { + try { + inSession( session -> { + session.getActionQueue().registerProcess( new AfterTransactionCompletionProcess() { + @Override + public void doAfterTransactionCompletion(boolean success, SharedSessionContractImplementor session) { + throw new RuntimeException( "My exception" ); + } + } ); + inTransaction( session, theSession -> { + theSession.persist( new SimpleEntity( "jack" ) ); + } ); + } ); + Assert.fail( "Expected exception to be thrown" ); + } + catch (Exception e) { + assertThat( e, instanceOf( HibernateException.class ) ); + assertThat( e.getMessage(), containsString( "Unable to perform afterTransactionCompletion callback" ) ); + Throwable cause = e.getCause(); + assertThat( cause, instanceOf( RuntimeException.class ) ); + assertThat( cause.getMessage(), containsString( "My exception" ) ); + + // Make sure that the original message is appended to the wrapping exception's message, for convenience + assertThat( e.getMessage(), containsString( "My exception" ) ); + } + + // Check that the transaction was committed + inTransaction( session -> { + long count = session.createQuery( "select count(*) from SimpleEntity", Long.class ) + .uniqueResult(); + assertEquals( 1L, count ); + } ); + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { SimpleEntity.class }; + } + + @Override + protected boolean isCleanupTestDataRequired() { + return true; + } + + @Entity(name = "SimpleEntity") + public static class SimpleEntity { + @Id + @GeneratedValue + private Integer id; + + private String name; + + SimpleEntity() { + } + + SimpleEntity(String name) { + this.name = name; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/actionqueue/CustomBeforeCompletionTest.java b/hibernate-core/src/test/java/org/hibernate/test/actionqueue/CustomBeforeCompletionTest.java new file mode 100644 index 0000000000..ef778b4157 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/actionqueue/CustomBeforeCompletionTest.java @@ -0,0 +1,133 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.actionqueue; + +import java.util.concurrent.atomic.AtomicBoolean; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; + +import org.hibernate.HibernateException; +import org.hibernate.action.spi.BeforeTransactionCompletionProcess; +import org.hibernate.engine.spi.SessionImplementor; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.Assert; +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + +public class CustomBeforeCompletionTest extends BaseCoreFunctionalTestCase { + + @Test + @TestForIssue(jiraKey = "HHH-13666") + public void success() { + inSession( session -> { + AtomicBoolean called = new AtomicBoolean( false ); + session.getActionQueue().registerProcess( new BeforeTransactionCompletionProcess() { + @Override + public void doBeforeTransactionCompletion(SessionImplementor session) { + called.set( true ); + } + } ); + Assert.assertFalse( called.get() ); + inTransaction( session, theSession -> { + theSession.persist( new SimpleEntity( "jack" ) ); + } ); + Assert.assertTrue( called.get() ); + } ); + + // Check that the transaction was committed + inTransaction( session -> { + long count = session.createQuery( "select count(*) from SimpleEntity", Long.class ) + .uniqueResult(); + assertEquals( 1L, count ); + } ); + } + + @Test + @TestForIssue(jiraKey = "HHH-13666") + public void failure() { + try { + inSession( session -> { + session.getActionQueue().registerProcess( new BeforeTransactionCompletionProcess() { + @Override + public void doBeforeTransactionCompletion(SessionImplementor session) { + throw new RuntimeException( "My exception" ); + } + } ); + inTransaction( session, theSession -> { + theSession.persist( new SimpleEntity( "jack" ) ); + } ); + } ); + Assert.fail( "Expected exception to be thrown" ); + } + catch (Exception e) { + assertThat( e, instanceOf( HibernateException.class ) ); + assertThat( e.getMessage(), containsString( "Unable to perform beforeTransactionCompletion callback" ) ); + Throwable cause = e.getCause(); + assertThat( cause, instanceOf( RuntimeException.class ) ); + assertThat( cause.getMessage(), containsString( "My exception" ) ); + + // Make sure that the original message is appended to the wrapping exception's message, for convenience + assertThat( e.getMessage(), containsString( "My exception" ) ); + } + + // Check that the transaction was rolled back + inTransaction( session -> { + long count = session.createQuery( "select count(*) from SimpleEntity", Long.class ) + .uniqueResult(); + assertEquals( 0L, count ); + } ); + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { SimpleEntity.class }; + } + + @Override + protected boolean isCleanupTestDataRequired() { + return true; + } + + @Entity(name = "SimpleEntity") + public static class SimpleEntity { + @Id + @GeneratedValue + private Integer id; + + private String name; + + SimpleEntity() { + } + + SimpleEntity(String name) { + this.name = name; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } +}