HHH-18867 correctly implement the documented semantics of beginTransaction()

hibernate-core/src/main/java/org/hibernate/SharedSessionContract.java

@@ -69,6 +69,29 @@ public interface SharedSessionContract extends QueryProducer, AutoCloseable, Ser
 	 * If a new underlying transaction is required, begin the transaction. Otherwise,
 	 * continue the new work in the context of the existing underlying transaction.
 	 *
+	 * @apiNote
+	 * The JPA-standard way to begin a new resource-local transaction is by calling
+	 * {@link #getTransaction getTransaction().begin()}. But it's not always safe to
+	 * execute this idiom.
+	 * <ul>
+	 * <li>JPA doesn't allow an {@link jakarta.persistence.EntityTransaction
+	 * EntityTransaction} to represent a JTA transaction context. Therefore, 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},
+	 * the call to {@code getTransaction()} fails if transactions are managed by JTA.
+	 * <p>
+	 * On the other hand, this method does not fail when JTA transaction management
+	 * is used, not even if strict JPA transaction compliance is enabled.
+	 * <li>Even when resource-local transactions are in use, and even when strict JPA
+	 * transaction compliance is <em>disabled</em>, the call to {@code begin()}
+	 * fails if a transaction is already {@linkplain Transaction#isActive active}.
+	 * <p>
+	 * This method never fails when a transaction is already active. Instead,
+	 * {@code beginTransaction()} simply returns the {@link Transaction} object
+	 * representing the active transaction.
+	 * </ul>
+	 *
 	 * @return an instance of {@link Transaction} representing the new transaction
 	 *
 	 * @see #getTransaction()
@@ -79,6 +102,20 @@ public interface SharedSessionContract extends QueryProducer, AutoCloseable, Ser
 	/**
 	 * Get the {@link Transaction} instance associated with this session.
 	 *
+	 * @apiNote
+	 * This method is the JPA-standard way to obtain an instance of
+	 * {@link jakarta.persistence.EntityTransaction EntityTransaction}
+	 * representing a resource-local transaction. But JPA doesn't allow an
+	 * {@code EntityTransaction} to represent a JTA transaction. Therefore, 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},
+	 * this method fails if transactions are managed by JTA.
+	 * <p>
+	 * On the other hand, when JTA transaction management is used, and when
+	 * strict JPA transaction compliance is <em>disabled</em>, this method happily
+	 * returns a {@link Transaction} representing the current JTA transaction context.
+	 *
 	 * @return an instance of {@link Transaction} representing the transaction
 	 *         associated with this session
 	 *

hibernate-core/src/main/java/org/hibernate/Transaction.java

@@ -17,7 +17,7 @@
  * depending on how Hibernate is configured.
  * <p>
  * Every resource-local transaction is associated with a {@link Session} and begins with
- * an explicit call to {@link Session#beginTransaction()}, or, equivalently, with
+ * an explicit call to {@link Session#beginTransaction()}, or, almost equivalently, with
  * {@code session.getTransaction().begin()}, and ends with a call to {@link #commit()}
  * or {@link #rollback()}.
  * <p>
@@ -31,6 +31,11 @@
  * <p>
  * A {@code Transaction} object is not threadsafe.
  *
+ * @apiNote JPA doesn't allow an {@link EntityTransaction} to represent a JTA transaction.
+ * But when {@linkplain org.hibernate.jpa.spi.JpaCompliance#isJpaTransactionComplianceEnabled
+ * strict JPA transaction compliance} is disabled, as it is by default, Hibernate allows an
+ * instance of this interface to represent the current JTA transaction context.
+ *
  * @author Anton van Straaten
  * @author Steve Ebersole
  *

hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java

@@ -610,9 +610,13 @@ public CacheTransactionSynchronization getCacheTransactionSynchronization() {
 	@Override
 	public Transaction beginTransaction() {
 		checkOpen();
-		final Transaction result = getTransaction();
-		result.begin();
-		return result;
+		final Transaction transaction = accessTransaction();
+		// 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() {

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() ) {

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();