HHH-10664 - Prep 6.0 feature branch - merge hibernate-entitymanager into hibernate-core (HEM tests - JPA requirements around access to Transaction delegate after EntityManager is closed; also its requirement that the same Transaction delegate be available across all calls to its commit/rollback methods)

hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java

@@ -476,8 +476,8 @@ public class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement
 	}
 
 	@Override
-	public void markAsJpaBootstrap(boolean jpaBootstrap) {
-		this.options.jpaBootstrap = jpaBootstrap;
+	public void markAsJpaBootstrap() {
+		this.options.jpaBootstrap = true;
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java

@@ -19,6 +19,7 @@ import org.hibernate.SessionFactoryObserver;
 import org.hibernate.boot.SchemaAutoTooling;
 import org.hibernate.boot.TempTableDdlTransactionHandling;
 import org.hibernate.boot.registry.StandardServiceRegistry;
+import org.hibernate.boot.spi.SessionFactoryOptions;
 import org.hibernate.cache.spi.QueryCacheFactory;
 import org.hibernate.cfg.BaselineSessionEventsListenerBuilder;
 import org.hibernate.context.spi.CurrentTenantIdentifierResolver;
@@ -38,6 +39,11 @@ import org.hibernate.tuple.entity.EntityTuplizerFactory;
 public interface SessionFactoryOptionsState {
 	StandardServiceRegistry getServiceRegistry();
 
+	/**
+	 * @deprecated (since 5.2) see {@link SessionFactoryOptions#isJpaBootstrap} for details
+	 * on deprecation and intention/use.
+	 */
+	@Deprecated
 	boolean isJpaBootstrap();
 
 	Object getBeanManagerReference();

hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryBuilderImplementor.java

@@ -15,6 +15,23 @@ import org.hibernate.boot.SessionFactoryBuilder;
  * @author Steve Ebersole
  */
 public interface SessionFactoryBuilderImplementor extends SessionFactoryBuilder {
-	void markAsJpaBootstrap(boolean jpaBootstrap);
+	/**
+	 * Indicates that the SessionFactory being built comes from JPA bootstrapping.
+	 * Internally {@code false} is the assumed value.  We only need to call this to
+	 * mark that as true.
+	 *
+	 * @deprecated (since 5.2) In fact added in 5.2 as part of consolidating JPA support
+	 * directly into Hibernate contracts (SessionFactory, Session); intended to provide
+	 * transition help in cases where we need to know the difference in JPA/native use for
+	 * various reasons.
+	 */
+	@Deprecated
+	void markAsJpaBootstrap();
+
+	/**
+	 * Build the SessionFactoryOptions that will ultimately be passed to SessionFactoryImpl constructor.
+	 *
+	 * @return The options.
+	 */
 	SessionFactoryOptions buildSessionFactoryOptions();
 }

hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java

@@ -43,12 +43,21 @@ public interface SessionFactoryOptions {
 	 */
 	StandardServiceRegistry getServiceRegistry();
 
-	boolean isJpaBootstrap();
-
 	Object getBeanManagerReference();
 
 	Object getValidatorFactoryReference();
 
+	/**
+	 * @deprecated (since 5.2) In fact added in 5.2 as part of consolidating JPA support
+	 * directly into Hibernate contracts (SessionFactory, Session); intended to provide
+	 * transition help in cases where we need to know the difference in JPA/native use for
+	 * various reasons.
+	 *
+	 * @see SessionFactoryBuilderImplementor#markAsJpaBootstrap
+	 */
+	@Deprecated
+	boolean isJpaBootstrap();
+
 	/**
 	 * The name to be used for the SessionFactory.  This is use both in:<ul>
 	 *     <li>in-VM serialization</li>

hibernate-core/src/main/java/org/hibernate/engine/transaction/internal/TransactionImpl.java

@@ -27,21 +27,21 @@ public class TransactionImpl implements TransactionImplementor {
 	private static final Logger LOG = CoreLogging.logger( TransactionImpl.class );
 
 	private final TransactionCoordinator transactionCoordinator;
-	private final TransactionDriver transactionDriverControl;
-
-	private boolean valid = true;
+	private TransactionDriver transactionDriverControl;
 
 	public TransactionImpl(TransactionCoordinator transactionCoordinator) {
 		this.transactionCoordinator = transactionCoordinator;
-		this.transactionDriverControl = transactionCoordinator.getTransactionDriverControl();
 	}
 
 	@Override
 	public void begin() {
-		if ( !valid ) {
-			throw new TransactionException( "Transaction instance is no longer valid" );
+		if ( !transactionCoordinator.isActive() ) {
+			throw new TransactionException( "Cannot begin Transaction on closed Session/EntityManager" );
 		}
 
+		if ( transactionDriverControl == null ) {
+			transactionDriverControl = transactionCoordinator.getTransactionDriverControl();
+		}
 		// per-JPA
 		if ( isActive() ) {
 			throw new IllegalStateException( "Transaction already active" );
@@ -61,13 +61,21 @@ public class TransactionImpl implements TransactionImplementor {
 		LOG.debug( "committing" );
 
 		try {
-			this.transactionDriverControl.commit();
+			internalGetTransactionDriverControl().commit();
 		}
 		finally {
-			invalidate();
+//			invalidate();
 		}
 	}
 
+	public TransactionDriver internalGetTransactionDriverControl() {
+		// NOTE here to help be a more descriptive NullPointerException
+		if ( this.transactionDriverControl == null ) {
+			throw new IllegalStateException( "Transaction was not properly begun/started" );
+		}
+		return this.transactionDriverControl;
+	}
+
 	@Override
 	public void rollback() {
 		// todo : may need a "JPA compliant" flag here
@@ -76,7 +84,7 @@ public class TransactionImpl implements TransactionImplementor {
 		if ( status == TransactionStatus.ROLLED_BACK || status == TransactionStatus.NOT_ACTIVE ) {
 			// Allow rollback() calls on completed transactions, just no-op.
 			LOG.debug( "rollback() called on an inactive transaction" );
-			invalidate();
+//			invalidate();
 			return;
 		}
 
@@ -87,7 +95,7 @@ public class TransactionImpl implements TransactionImplementor {
 		LOG.debug( "rolling back" );
 		if ( status != TransactionStatus.FAILED_COMMIT || allowFailedCommitToPhysicallyRollback() ) {
 			try {
-				this.transactionDriverControl.rollback();
+				internalGetTransactionDriverControl().rollback();
 			}
 			finally {
 				invalidate();
@@ -102,7 +110,10 @@ public class TransactionImpl implements TransactionImplementor {
 
 	@Override
 	public TransactionStatus getStatus() {
-		return transactionDriverControl.getStatus();
+		// Allow looking at STATUS on closed Session/Transaction
+		return transactionDriverControl == null
+				? TransactionStatus.NOT_ACTIVE
+				: transactionDriverControl.getStatus();
 	}
 
 	@Override
@@ -122,7 +133,7 @@ public class TransactionImpl implements TransactionImplementor {
 
 	@Override
 	public void setRollbackOnly() {
-		transactionDriverControl.markRollbackOnly();
+		internalGetTransactionDriverControl().markRollbackOnly();
 	}
 
 	@Override
@@ -130,10 +141,6 @@ public class TransactionImpl implements TransactionImplementor {
 		return getStatus() == TransactionStatus.MARKED_ROLLBACK;
 	}
 
-	public void invalidate() {
-		valid = false;
-	}
-
 	protected boolean allowFailedCommitToPhysicallyRollback() {
 		return false;
 	}

hibernate-core/src/main/java/org/hibernate/engine/transaction/spi/TransactionImplementor.java

@@ -18,6 +18,14 @@ public interface TransactionImplementor extends Transaction {
 	 *     <li>The transaction is rolled-back</li>
 	 *     <li>The session that owns the transaction is closed</li>
 	 * </ul>
+	 *
+	 * @deprecated (since 5.2) as part of effort to consolidate support for JPA and Hibernate SessionFactory, Session, etc
+	 * natively, support for local Transaction delegates to remain "valid" after they are committed or rolled-back (and to a
+	 * degree after the owning Session is closed) to more closely comply with the JPA spec natively in terms
+	 * of allowing that extended access after Session is closed.  Hibernate impls have all been changed to no-op here.
 	 */
-	void invalidate();
+	@Deprecated
+	default void invalidate() {
+		// no-op : see @deprecated note
+	}
 }

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

@@ -369,15 +369,34 @@ public abstract class AbstractSharedSessionContract implements SharedSessionCont
 
 	@Override
 	public Transaction getTransaction() throws HibernateException {
-		// See class-level JavaDocs for a discussion of the concurrent-access safety of this method
-		checkOpen();
+		if ( getFactory().getSessionFactoryOptions().isJpaBootstrap() ) {
+			// JPA requires that we throw IllegalStateException if this is called
+			// on a JTA EntityManager
+			//
+			// todo : ultimately add an option for allowing users to access the Transaction in JTA cases too like classic Hibernate
 
-		// todo : determine whether this is allowed for JPA scenarios based on PersistenceUnitTransactionType && "strictness"
-		if ( this.currentHibernateTransaction == null || this.currentHibernateTransaction.getStatus() != TransactionStatus.ACTIVE ) {
-			this.currentHibernateTransaction = new TransactionImpl( getTransactionCoordinator() );
+			if ( getTransactionCoordinator().getTransactionCoordinatorBuilder().isJta() ) {
+				throw new IllegalStateException( "A JTA EntityManager cannot use getTransaction()" );
+			}
+
+			if ( this.currentHibernateTransaction == null ) {
+				this.currentHibernateTransaction = new TransactionImpl( getTransactionCoordinator() );
+			}
+			if ( !isClosed() ) {
+				getTransactionCoordinator().pulse();
+			}
+			return currentHibernateTransaction;
+		}
+		else {
+			// Historically Hibernate would not allow access to the Transaction after the Session is closed
+			checkOpen();
+
+			if ( this.currentHibernateTransaction == null || this.currentHibernateTransaction.getStatus() != TransactionStatus.ACTIVE ) {
+				this.currentHibernateTransaction = new TransactionImpl( getTransactionCoordinator() );
+			}
+			getTransactionCoordinator().pulse();
+			return currentHibernateTransaction;
 		}
-		getTransactionCoordinator().pulse();
-		return currentHibernateTransaction;
 	}
 
 	@Override

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

@@ -158,7 +158,6 @@ public final class SessionFactoryImpl implements SessionFactoryImplementor {
 
 	private final String name;
 	private final String uuid;
-	private final boolean jpaBootstrap;
 	private transient boolean isClosed;
 
 	private final transient SessionFactoryObserverChain observer = new SessionFactoryObserverChain();
@@ -221,7 +220,6 @@ public final class SessionFactoryImpl implements SessionFactoryImplementor {
 		catch (Exception e) {
 			throw new AssertionFailure("Could not generate UUID");
 		}
-		this.jpaBootstrap = options.isJpaBootstrap();
 
 		final JdbcServices jdbcServices = serviceRegistry.getService( JdbcServices.class );
 

hibernate-core/src/main/java/org/hibernate/jpa/boot/internal/EntityManagerFactoryBuilderImpl.java

@@ -869,7 +869,7 @@ public class EntityManagerFactoryBuilderImpl implements EntityManagerFactoryBuil
 	}
 
 	protected void populate(SessionFactoryBuilder sfBuilder, StandardServiceRegistry ssr) {
-		( ( SessionFactoryBuilderImplementor) sfBuilder ).markAsJpaBootstrap( true );
+		( ( SessionFactoryBuilderImplementor) sfBuilder ).markAsJpaBootstrap();
 
 		final StrategySelector strategySelector = ssr.getService( StrategySelector.class );
 

hibernate-core/src/main/java/org/hibernate/query/internal/AbstractProducedQuery.java

@@ -56,14 +56,12 @@ import org.hibernate.jpa.internal.util.LockModeTypeHelper;
 import org.hibernate.property.access.spi.BuiltInPropertyAccessStrategies;
 import org.hibernate.property.access.spi.Getter;
 import org.hibernate.property.access.spi.PropertyAccess;
-import org.hibernate.proxy.HibernateProxyHelper;
 import org.hibernate.query.ParameterMetadata;
 import org.hibernate.query.QueryParameter;
 import org.hibernate.query.spi.QueryImplementor;
 import org.hibernate.query.spi.QueryParameterBinding;
 import org.hibernate.transform.ResultTransformer;
 import org.hibernate.type.SerializableType;
-import org.hibernate.type.StandardBasicTypes;
 import org.hibernate.type.Type;
 
 import static org.hibernate.jpa.QueryHints.HINT_CACHEABLE;
@@ -1132,6 +1130,20 @@ public abstract class AbstractProducedQuery<R> implements QueryImplementor<R> {
 		try {
 			return doExecuteUpdate();
 		}
+		catch ( QueryExecutionRequestException e) {
+			throw new IllegalStateException( e );
+		}
+		catch( TypeMismatchException e ) {
+			throw new IllegalArgumentException( e );
+		}
+		catch ( HibernateException e) {
+			if ( getProducer().getFactory().getSessionFactoryOptions().isJpaBootstrap() ) {
+				throw getProducer().convert( e );
+			}
+			else {
+				throw e;
+			}
+		}
 		finally {
 			afterQuery();
 		}

hibernate-core/src/main/java/org/hibernate/resource/transaction/backend/jdbc/internal/JdbcResourceLocalTransactionCoordinatorImpl.java

@@ -168,16 +168,6 @@ public class JdbcResourceLocalTransactionCoordinatorImpl implements TransactionC
 		for ( TransactionObserver observer : observers ) {
 			observer.afterCompletion( successful, false );
 		}
-		invalidateDelegate();
-	}
-
-	private void invalidateDelegate() {
-		if ( physicalTransactionDelegate == null ) {
-			throw new IllegalStateException( "Physical-transaction delegate not known on attempt to invalidate" );
-		}
-
-		physicalTransactionDelegate.invalidate();
-		physicalTransactionDelegate = null;
 	}
 
 	public void addObserver(TransactionObserver observer) {

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/BaseEntityManagerFunctionalTestCase.java

@@ -284,6 +284,10 @@ public abstract class BaseEntityManagerFunctionalTestCase extends BaseUnitTestCa
 		if ( em == null ) {
 			return;
 		}
+		if ( !em.isOpen() ) {
+			return;
+		}
+
 		if ( em.getTransaction().isActive() ) {
 			em.getTransaction().rollback();
             log.warn("You left an open transaction! Fix your test case. For now, we are closing it for you.");