give some love to org.hibernate.Transaction

also clean up some stuff in AbstractPersistentCollection

Unfortunately TransactionStatus is an SPI type, which makes
its use in the API of Transaction a bit wrong. Also, it's
weird to see dependence on an interface defined by JPA.
(However these are relatively harmless aesthetic points, so
I decided not to deprecate anything.) Instead I've added
alternative "convenience" operations.

Note on terminology: a transaction which has been "marked for
rollback only" is still an active transaction! You can still
do work in it, and that work will still be atomically rolled
back when the transaction completes. Apparently there was
some confusion on this at some point, which was later fixed,
but some evidence of the confusion was left behind in code.

Author: gavinking

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

@@ -10,6 +10,10 @@
 
 import org.hibernate.resource.transaction.spi.TransactionStatus;
 
+import java.util.function.Consumer;
+
+import static org.hibernate.resource.transaction.backend.jta.internal.StatusTranslator.translate;
+
 /**
  * Represents a resource-local transaction, where <em>resource-local</em> is interpreted
  * by Hibernate to mean any transaction under the control of Hibernate. That is to say,
@@ -44,31 +48,172 @@
 public interface Transaction extends EntityTransaction {
 	/**
 	 * Get the current {@linkplain TransactionStatus status} of this transaction.
+	 *
+	 * @apiNote {@link TransactionStatus} belongs to an SPI package, and so this
+	 *          operation is a (fairly harmless) layer-breaker. Prefer the use
+	 *          of {@link #isActive}, {@link #isComplete}, {@link #wasStarted},
+	 *          {@link #wasSuccessful}, {@link #wasFailure}, or
+	 *          {@link #isInCompletionProcess} according to need.
+	 *
 	 */
 	TransactionStatus getStatus();
 
 	/**
-	 * Register a user {@link Synchronization synchronization callback} for this transaction.
+	 * Is this transaction still active?
+	 * <p>
+	 * A transaction which has been {@linkplain #markRollbackOnly marked for rollback}
+	 * is still considered active, and is still able to perform work. To determine if
+	 * a transaction has been marked for rollback, call {@link #getRollbackOnly()}.
+	 *
+	 * @return {@code true} if the {@linkplain #getStatus status}
+	 *         is {@link TransactionStatus#ACTIVE} or
+	 *         {@link TransactionStatus#MARKED_ROLLBACK}
+	 *
+	 * @since 7.0
+	 */
+	default boolean isActive() {
+		return switch (getStatus()) {
+			case ACTIVE, MARKED_ROLLBACK -> true;
+			default -> false;
+		};
+	}
+
+	/**
+	 * Is this transaction currently in the completion process?
+	 *
+	 * @return {@code true} if the {@linkplain #getStatus status}
+	 *         is {@link TransactionStatus#COMMITTING} or
+	 *         {@link TransactionStatus#ROLLING_BACK}
+	 *
+	 * @since 7.0
+	 */
+	default boolean isInCompletionProcess() {
+		return switch (getStatus()) {
+			case COMMITTING, ROLLING_BACK -> true;
+			default -> false;
+		};
+	}
+
+	/**
+	 * Is this transaction complete?
+	 *
+	 * @return {@code true} if the {@linkplain #getStatus status}
+	 *         is {@link TransactionStatus#COMMITTED},
+	 *         {@link TransactionStatus#ROLLED_BACK},
+	 *         {@link TransactionStatus#FAILED_COMMIT}, or
+	 *         {@link TransactionStatus#FAILED_ROLLBACK}
+	 *
+	 * @since 7.0
+	 */
+	default boolean isComplete() {
+		return switch (getStatus()) {
+			case COMMITTED, ROLLED_BACK, FAILED_COMMIT, FAILED_ROLLBACK -> true;
+			default -> false;
+		};
+	}
+
+	/**
+	 * Was this transaction already started?
+	 *
+	 * @return {@code true} if the {@linkplain #getStatus status} is
+	 *         anything other than {@link TransactionStatus#NOT_ACTIVE}
+	 *
+	 * @since 7.0
+	 */
+	default boolean wasStarted() {
+		return getStatus() != TransactionStatus.NOT_ACTIVE;
+	}
+
+	/**
+	 * Was this transaction already successfully committed?
+	 *
+	 * @return {@code true} if the {@linkplain #getStatus status}
+	 *         is {@link TransactionStatus#COMMITTED}
+	 *
+	 * @since 7.0
+	 */
+	default boolean wasSuccessful() {
+		return getStatus() == TransactionStatus.COMMITTED;
+	}
+
+	/**
+	 * Was this transaction a failure? Here we consider a successful rollback,
+	 * a failed commit, or a failed rollback to amount to transaction failure.
+	 *
+	 * @return {@code true} if the {@linkplain #getStatus status}
+	 *         is {@link TransactionStatus#ROLLED_BACK},
+	 *         {@link TransactionStatus#FAILED_COMMIT},
+	 *         {@link TransactionStatus#FAILED_ROLLBACK}
+	 *
+	 * @since 7.0
+	 */
+	 default boolean wasFailure() {
+		 return switch (getStatus()) {
+			 case ROLLED_BACK, FAILED_COMMIT, FAILED_ROLLBACK -> true;
+			 default -> false;
+		 };
+	 }
+
+	/**
+	 * Register an action which will be called during the "before completion" phase.
 	 *
-	 * @param synchronization The {@link Synchronization} callback to register.
+	 * @since 7.0
+	 */
+	default void runBeforeCompletion(Runnable action) {
+		 registerSynchronization( new Synchronization() {
+			 @Override
+			 public void beforeCompletion() {
+				 action.run();
+			 }
+			 @Override
+			 public void afterCompletion(int status) {
+			 }
+		 } );
+	 }
+
+	/**
+	 * Register an action which will be called during the "after completion" phase.
 	 *
-	 * @throws HibernateException Indicates a problem registering the synchronization.
+	 * @since 7.0
+	 */
+	default void runAfterCompletion(Consumer<TransactionStatus> action) {
+		registerSynchronization( new Synchronization() {
+			@Override
+			public void beforeCompletion() {
+			}
+			@Override
+			public void afterCompletion(int status) {
+				action.accept( translate( status ) );
+			}
+		} );
+	}
+
+	/**
+	 * Register a {@linkplain Synchronization synchronization callback} for this transaction.
+	 *
+	 * @param synchronization The {@link Synchronization} callback to register
+	 *
+	 * @apiNote {@link Synchronization} is a type defined by JTA, but this operation does
+	 *          not depend on the use of JTA for transaction management. Prefer the use of
+	 *          the methods {@link #runBeforeCompletion} and {@link #runAfterCompletion}
+	 *          for convenience.
 	 */
 	void registerSynchronization(Synchronization synchronization);
 
 	/**
-	 * Set the transaction timeout for any transaction started by any subsequent call to
-	 * {@link #begin} on this instance.
+	 * Set the transaction timeout for any transaction started by a subsequent call to
+	 * {@link #begin} on this instance of {@code Transaction}.
 	 *
-	 * @param seconds The number of seconds before a timeout.
+	 * @param seconds The number of seconds before a timeout
 	 */
 	void setTimeout(int seconds);
 
 	/**
-	 * Retrieve the transaction timeout set for this instance. A negative integer indicates
-	 * that no timeout has been set.
+	 * Retrieve the transaction timeout set for this instance.
+	 * <p>
+	 * A {@code null} return value indicates that no timeout has been set.
 	 *
-	 * @return The timeout, in seconds.
+	 * @return the timeout, in seconds, or {@code null}
 	 */
 	@Nullable Integer getTimeout();
 

hibernate-core/src/main/java/org/hibernate/cfg/TransactionSettings.java

@@ -157,8 +157,10 @@ public interface TransactionSettings {
 	 *
 	 * @settingDefault {@code false} (disabled)
 	 *
-	 * @apiNote Generally speaking, all access to transactional data should be done in a transaction.
+	 * @apiNote Generally speaking, all access to transactional data should be
+	 *          done in a transaction. Use of this setting is discouraged.
 	 *
+	 * @see org.hibernate.boot.spi.SessionFactoryOptions#isInitializeLazyStateOutsideTransactionsEnabled
 	 * @see org.hibernate.boot.SessionFactoryBuilder#applyLazyInitializationOutsideTransaction(boolean)
 	 */
 	@Unsafe
@@ -177,9 +179,11 @@ public interface TransactionSettings {
 	 *
 	 * @settingDefault {@code false} (disabled)
 	 *
-	 * @apiNote Generally speaking, all access to transactional data should be done in a transaction.
-	 * Combining this with second-level caching, e.g., will cause problems.
+	 * @apiNote Generally speaking, all access to transactional data should be
+	 *          done in a transaction. Combining this with second-level caching
+	 *          is not safe. Use of this setting is discouraged.
 	 *
+	 * @see org.hibernate.boot.spi.SessionFactoryOptions#isAllowOutOfTransactionUpdateOperations
 	 * @see org.hibernate.boot.SessionFactoryBuilder#allowOutOfTransactionUpdateOperations(boolean)
 	 *
 	 * @since 5.2