From 5cc6eed50a639ba82a8db0090226b302639a43f4 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Wed, 20 Nov 2024 12:51:52 +0100 Subject: [PATCH 1/2] correctly implement the documented semantics of beginTransaction() and clarify its semantics in the jdoc --- .../java/org/hibernate/SharedSessionContract.java | 11 +++++++++++ .../internal/AbstractSharedSessionContract.java | 10 +++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/SharedSessionContract.java b/hibernate-core/src/main/java/org/hibernate/SharedSessionContract.java index 109d37d56a3d..754764f8905b 100644 --- a/hibernate-core/src/main/java/org/hibernate/SharedSessionContract.java +++ b/hibernate-core/src/main/java/org/hibernate/SharedSessionContract.java @@ -68,6 +68,17 @@ public interface SharedSessionContract extends QueryProducer, AutoCloseable, Ser * Begin a unit of work and return the associated {@link Transaction} object. * If a new underlying transaction is required, begin the transaction. Otherwise, * continue the new work in the context of the existing underlying transaction. + *

+ * The JPA-standard way to begin a new transaction is by calling + * {@link #getTransaction getTransaction().begin()}. When + * {@linkplain org.hibernate.jpa.spi.JpaCompliance#isJpaTransactionComplianceEnabled + * strict JPA transaction compliance} is enabled via, for example, setting + * {@value org.hibernate.cfg.JpaComplianceSettings#JPA_TRANSACTION_COMPLIANCE}, + * or when resource-local transactions are used, the call to {@code begin()} + * fails if the transaction is already {@linkplain Transaction#isActive active}. + * On the other hand, this method does not fail when a transaction is already + * active, and simply returns the {@link Transaction} object representing the + * active transaction. * * @return an instance of {@link Transaction} representing the new transaction * 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 9a98a01aa60b..af1bead64cab 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java @@ -606,9 +606,13 @@ public CacheTransactionSynchronization getCacheTransactionSynchronization() { @Override public Transaction beginTransaction() { checkOpen(); - final Transaction result = getTransaction(); - result.begin(); - return result; + final Transaction transaction = getTransaction(); + // only need to begin a transaction if it was not + // already active (this is the documented semantics) + if ( !transaction.isActive() ) { + transaction.begin(); + } + return transaction; } protected void checkTransactionSynchStatus() { From eec7b8ab11c0b6a2798e05d918bb1069c5567cb7 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Wed, 20 Nov 2024 23:26:00 +0100 Subject: [PATCH 2/2] fix tests which were asserting stuff about JPA compliance for a non-JPA method The JPA spec does not have anything to say about our beginTransaction() method --- .../jta/JpaComplianceAlreadyStartedTransactionTest.java | 3 ++- .../jta/NonJpaComplianceAlreadyStartedTransactionTest.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/resource/transaction/jta/JpaComplianceAlreadyStartedTransactionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/resource/transaction/jta/JpaComplianceAlreadyStartedTransactionTest.java index f0ca7aabf91f..8e7bc38d9dc5 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/resource/transaction/jta/JpaComplianceAlreadyStartedTransactionTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/resource/transaction/jta/JpaComplianceAlreadyStartedTransactionTest.java @@ -45,7 +45,8 @@ public void anIllegalStateExceptionShouldBeThrownWhenBeginTxIsCalledWithAnAlread Transaction tx = null; try { // A call to begin() with an active Tx should cause an IllegalStateException - tx = s.beginTransaction(); + tx = s.getTransaction(); + tx.begin(); } catch (Exception e) { if ( tx != null && tx.isActive() ) { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/resource/transaction/jta/NonJpaComplianceAlreadyStartedTransactionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/resource/transaction/jta/NonJpaComplianceAlreadyStartedTransactionTest.java index a0e51a4368b8..5b27adf226d6 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/resource/transaction/jta/NonJpaComplianceAlreadyStartedTransactionTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/resource/transaction/jta/NonJpaComplianceAlreadyStartedTransactionTest.java @@ -50,7 +50,8 @@ public void setUp() { public void noIllegalStateExceptionShouldBeThrownWhenBeginTxIsCalledWithAnAlreadyActiveTx() throws Exception { tm.begin(); try (Session s = openSession()) { - Transaction tx = s.beginTransaction(); + Transaction tx = s.getTransaction(); + tx.begin(); try { s.persist( new TestEntity( "ABC" ) ); tx.commit();