Explicit notes on isolation level handling in participating transactions

Issue: SPR-16463

spring-tx/src/main/java/org/springframework/transaction/TransactionDefinition.java

@@ -205,21 +205,29 @@ public interface TransactionDefinition {
 
 	/**
 	 * Return the isolation level.
-	 * <p>Must return one of the {@code ISOLATION_XXX} constants
-	 * defined on {@link TransactionDefinition this interface}.
-	 * <p>Only makes sense in combination with {@link #PROPAGATION_REQUIRED}
-	 * or {@link #PROPAGATION_REQUIRES_NEW}.
+	 * <p>Must return one of the {@code ISOLATION_XXX} constants defined on
+	 * {@link TransactionDefinition this interface}. Those constants are designed
+	 * to match the values of the same constants on {@link java.sql.Connection}.
+	 * <p>Exclusively designed for use with {@link #PROPAGATION_REQUIRED} or
+	 * {@link #PROPAGATION_REQUIRES_NEW} since it only applies to newly started
+	 * transactions. Consider switching the "validateExistingTransactions" flag to
+	 * "true" on your transaction manager if you'd like isolation level declarations
+	 * to get rejected when participating in an existing transaction with a different
+	 * isolation level.
 	 * <p>Note that a transaction manager that does not support custom isolation levels
 	 * will throw an exception when given any other level than {@link #ISOLATION_DEFAULT}.
 	 * @return the isolation level
+	 * @see #ISOLATION_DEFAULT
+	 * @see org.springframework.transaction.support.AbstractPlatformTransactionManager#setValidateExistingTransaction
 	 */
 	int getIsolationLevel();
 
 	/**
 	 * Return the transaction timeout.
 	 * <p>Must return a number of seconds, or {@link #TIMEOUT_DEFAULT}.
-	 * <p>Only makes sense in combination with {@link #PROPAGATION_REQUIRED}
-	 * or {@link #PROPAGATION_REQUIRES_NEW}.
+	 * <p>Exclusively designed for use with {@link #PROPAGATION_REQUIRED} or
+	 * {@link #PROPAGATION_REQUIRES_NEW} since it only applies to newly started
+	 * transactions.
 	 * <p>Note that a transaction manager that does not support timeouts will throw
 	 * an exception when given any other timeout than {@link #TIMEOUT_DEFAULT}.
 	 * @return the transaction timeout
@@ -228,13 +236,12 @@ public interface TransactionDefinition {
 
 	/**
 	 * Return whether to optimize as a read-only transaction.
-	 * <p>The read-only flag applies to any transaction context, whether
-	 * backed by an actual resource transaction
-	 * ({@link #PROPAGATION_REQUIRED}/{@link #PROPAGATION_REQUIRES_NEW}) or
-	 * operating non-transactionally at the resource level
-	 * ({@link #PROPAGATION_SUPPORTS}). In the latter case, the flag will
-	 * only apply to managed resources within the application, such as a
-	 * Hibernate {@code Session}.
+	 * <p>The read-only flag applies to any transaction context, whether backed
+	 * by an actual resource transaction ({@link #PROPAGATION_REQUIRED}/
+	 * {@link #PROPAGATION_REQUIRES_NEW}) or operating non-transactionally at
+	 * the resource level ({@link #PROPAGATION_SUPPORTS}). In the latter case,
+	 * the flag will only apply to managed resources within the application,
+	 * such as a Hibernate {@code Session}.
 	 * <p>This just serves as a hint for the actual transaction subsystem;
 	 * it will <i>not necessarily</i> cause failure of write access attempts.
 	 * A transaction manager which cannot interpret the read-only hint will

spring-tx/src/main/java/org/springframework/transaction/annotation/Transactional.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -86,19 +86,30 @@ public @interface Transactional {
 	/**
 	 * The transaction isolation level.
 	 * <p>Defaults to {@link Isolation#DEFAULT}.
+	 * <p>Exclusively designed for use with {@link Propagation#REQUIRED} or
+	 * {@link Propagation#REQUIRES_NEW} since it only applies to newly started
+	 * transactions. Consider switching the "validateExistingTransactions" flag to
+	 * "true" on your transaction manager if you'd like isolation level declarations
+	 * to get rejected when participating in an existing transaction with a different
+	 * isolation level.
 	 * @see org.springframework.transaction.interceptor.TransactionAttribute#getIsolationLevel()
+	 * @see org.springframework.transaction.support.AbstractPlatformTransactionManager#setValidateExistingTransaction
 	 */
 	Isolation isolation() default Isolation.DEFAULT;
 
 	/**
 	 * The timeout for this transaction.
 	 * <p>Defaults to the default timeout of the underlying transaction system.
+	 * <p>Exclusively designed for use with {@link Propagation#REQUIRED} or
+	 * {@link Propagation#REQUIRES_NEW} since it only applies to newly started
+	 * transactions.
 	 * @see org.springframework.transaction.interceptor.TransactionAttribute#getTimeout()
 	 */
 	int timeout() default TransactionDefinition.TIMEOUT_DEFAULT;
 
 	/**
-	 * {@code true} if the transaction is read-only.
+	 * A boolean flag that can be set to {@code true} if the transaction is
+	 * effectively read-only, allowing for corresponding optimizations at runtime.
 	 * <p>Defaults to {@code false}.
 	 * <p>This just serves as a hint for the actual transaction subsystem;
 	 * it will <i>not necessarily</i> cause failure of write access attempts.
@@ -106,6 +117,7 @@ public @interface Transactional {
 	 * <i>not</i> throw an exception when asked for a read-only transaction
 	 * but rather silently ignore the hint.
 	 * @see org.springframework.transaction.interceptor.TransactionAttribute#isReadOnly()
+	 * @see org.springframework.transaction.support.TransactionSynchronizationManager#isCurrentTransactionReadOnly()
 	 */
 	boolean readOnly() default false;
 

spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -215,6 +215,7 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 	 * <p>Default is "false", leniently ignoring inner transaction settings,
 	 * simply overriding them with the outer transaction's characteristics.
 	 * Switch this flag to "true" in order to enforce strict validation.
+	 * @since 2.5.1
 	 */
 	public final void setValidateExistingTransaction(boolean validateExistingTransaction) {
 		this.validateExistingTransaction = validateExistingTransaction;
@@ -223,6 +224,7 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 	/**
 	 * Return whether existing transactions should be validated before participating
 	 * in them.
+	 * @since 2.5.1
 	 */
 	public final boolean isValidateExistingTransaction() {
 		return this.validateExistingTransaction;
@@ -285,6 +287,7 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 	 * boundary. This allows, for example, to continue unit tests even after an
 	 * operation failed and the transaction will never be completed. All transaction
 	 * managers will only fail earlier if this flag has explicitly been set to "true".
+	 * @since 2.0
 	 * @see org.springframework.transaction.UnexpectedRollbackException
 	 */
 	public final void setFailEarlyOnGlobalRollbackOnly(boolean failEarlyOnGlobalRollbackOnly) {
@@ -294,6 +297,7 @@ public abstract class AbstractPlatformTransactionManager implements PlatformTran
 	/**
 	 * Return whether to fail early in case of the transaction being globally marked
 	 * as rollback-only.
+	 * @since 2.0
 	 */
 	public final boolean isFailEarlyOnGlobalRollbackOnly() {
 		return this.failEarlyOnGlobalRollbackOnly;

spring-tx/src/main/java/org/springframework/transaction/support/DefaultTransactionDefinition.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -110,7 +110,7 @@ public class DefaultTransactionDefinition implements TransactionDefinition, Seri
 	 * Set the propagation behavior by the name of the corresponding constant in
 	 * TransactionDefinition, e.g. "PROPAGATION_REQUIRED".
 	 * @param constantName name of the constant
-	 * @exception IllegalArgumentException if the supplied value is not resolvable
+	 * @throws IllegalArgumentException if the supplied value is not resolvable
 	 * to one of the {@code PROPAGATION_} constants or is {@code null}
 	 * @see #setPropagationBehavior
 	 * @see #PROPAGATION_REQUIRED
@@ -125,8 +125,16 @@ public class DefaultTransactionDefinition implements TransactionDefinition, Seri
 	/**
 	 * Set the propagation behavior. Must be one of the propagation constants
 	 * in the TransactionDefinition interface. Default is PROPAGATION_REQUIRED.
-	 * @exception IllegalArgumentException if the supplied value is not
-	 * one of the {@code PROPAGATION_} constants
+	 * <p>Exclusively designed for use with {@link #PROPAGATION_REQUIRED} or
+	 * {@link #PROPAGATION_REQUIRES_NEW} since it only applies to newly started
+	 * transactions. Consider switching the "validateExistingTransactions" flag to
+	 * "true" on your transaction manager if you'd like isolation level declarations
+	 * to get rejected when participating in an existing transaction with a different
+	 * isolation level.
+	 * <p>Note that a transaction manager that does not support custom isolation levels
+	 * will throw an exception when given any other level than {@link #ISOLATION_DEFAULT}.
+	 * @throws IllegalArgumentException if the supplied value is not one of the
+	 * {@code PROPAGATION_} constants
 	 * @see #PROPAGATION_REQUIRED
 	 */
 	public final void setPropagationBehavior(int propagationBehavior) {
@@ -145,7 +153,7 @@ public class DefaultTransactionDefinition implements TransactionDefinition, Seri
 	 * Set the isolation level by the name of the corresponding constant in
 	 * TransactionDefinition, e.g. "ISOLATION_DEFAULT".
 	 * @param constantName name of the constant
-	 * @exception IllegalArgumentException if the supplied value is not resolvable
+	 * @throws IllegalArgumentException if the supplied value is not resolvable
 	 * to one of the {@code ISOLATION_} constants or is {@code null}
 	 * @see #setIsolationLevel
 	 * @see #ISOLATION_DEFAULT
@@ -160,8 +168,16 @@ public class DefaultTransactionDefinition implements TransactionDefinition, Seri
 	/**
 	 * Set the isolation level. Must be one of the isolation constants
 	 * in the TransactionDefinition interface. Default is ISOLATION_DEFAULT.
-	 * @exception IllegalArgumentException if the supplied value is not
-	 * one of the {@code ISOLATION_} constants
+	 * <p>Exclusively designed for use with {@link #PROPAGATION_REQUIRED} or
+	 * {@link #PROPAGATION_REQUIRES_NEW} since it only applies to newly started
+	 * transactions. Consider switching the "validateExistingTransactions" flag to
+	 * "true" on your transaction manager if you'd like isolation level declarations
+	 * to get rejected when participating in an existing transaction with a different
+	 * isolation level.
+	 * <p>Note that a transaction manager that does not support custom isolation levels
+	 * will throw an exception when given any other level than {@link #ISOLATION_DEFAULT}.
+	 * @throws IllegalArgumentException if the supplied value is not one of the
+	 * {@code ISOLATION_} constants
 	 * @see #ISOLATION_DEFAULT
 	 */
 	public final void setIsolationLevel(int isolationLevel) {
@@ -179,6 +195,11 @@ public class DefaultTransactionDefinition implements TransactionDefinition, Seri
 	/**
 	 * Set the timeout to apply, as number of seconds.
 	 * Default is TIMEOUT_DEFAULT (-1).
+	 * <p>Exclusively designed for use with {@link #PROPAGATION_REQUIRED} or
+	 * {@link #PROPAGATION_REQUIRES_NEW} since it only applies to newly started
+	 * transactions.
+	 * <p>Note that a transaction manager that does not support timeouts will throw
+	 * an exception when given any other timeout than {@link #TIMEOUT_DEFAULT}.
 	 * @see #TIMEOUT_DEFAULT
 	 */
 	public final void setTimeout(int timeout) {
@@ -196,6 +217,16 @@ public class DefaultTransactionDefinition implements TransactionDefinition, Seri
 	/**
 	 * Set whether to optimize as read-only transaction.
 	 * Default is "false".
+	 * <p>The read-only flag applies to any transaction context, whether backed
+	 * by an actual resource transaction ({@link #PROPAGATION_REQUIRED}/
+	 * {@link #PROPAGATION_REQUIRES_NEW}) or operating non-transactionally at
+	 * the resource level ({@link #PROPAGATION_SUPPORTS}). In the latter case,
+	 * the flag will only apply to managed resources within the application,
+	 * such as a Hibernate {@code Session}.
+	 * <p>This just serves as a hint for the actual transaction subsystem;
+	 * it will <i>not necessarily</i> cause failure of write access attempts.
+	 * A transaction manager which cannot interpret the read-only hint will
+	 * <i>not</i> throw an exception when asked for a read-only transaction.
 	 */
 	public final void setReadOnly(boolean readOnly) {
 		this.readOnly = readOnly;

src/docs/asciidoc/data-access.adoc

@@ -195,9 +195,6 @@ execution.
 
 The `TransactionDefinition` interface specifies:
 
-* __Isolation__: The degree to which this transaction is isolated from the work of other
-  transactions. For example, can this transaction see uncommitted writes from other
-  transactions?
 * __Propagation__: Typically, all code executed within a transaction scope will run in
   that transaction. However, you have the option of specifying the behavior in the event
   that a transactional method is executed when a transaction context already exists. For
@@ -205,6 +202,9 @@ The `TransactionDefinition` interface specifies:
   the existing transaction can be suspended and a new transaction created. __Spring
   offers all of the transaction propagation options familiar from EJB CMT__. To read
   about the semantics of transaction propagation in Spring, see <<tx-propagation>>.
+* __Isolation__: The degree to which this transaction is isolated from the work of other
+  transactions. For example, can this transaction see uncommitted writes from other
+  transactions?
 * __Timeout__: How long this transaction runs before timing out and being rolled back
   automatically by the underlying transaction infrastructure.
 * __Read-only status__: A read-only transaction can be used when your code reads but
@@ -1021,17 +1021,17 @@ that are nested within `<tx:advice/>` and `<tx:attributes/>` tags are summarized
 | `isolation`
 | No
 | DEFAULT
-| Transaction isolation level.
+| Transaction isolation level. Only applicable to propagation REQUIRED or REQUIRES_NEW.
 
 | `timeout`
 | No
 | -1
-| Transaction timeout value (in seconds).
+| Transaction timeout (seconds). Only applicable to propagation REQUIRED or REQUIRES_NEW.
 
 | `read-only`
 | No
 | false
-| Is this transaction read-only?
+| Read/write vs. read-only transaction. Only applicable to REQUIRED or REQUIRES_NEW.
 
 | `rollback-for`
 | No
@@ -1155,7 +1155,6 @@ transactional behavior.
 
 [TIP]
 ====
-
 Spring recommends that you only annotate concrete classes (and methods of concrete
 classes) with the `@Transactional` annotation, as opposed to annotating interfaces. You
 certainly can place the `@Transactional` annotation on an interface (or an interface
@@ -1301,7 +1300,7 @@ annotation are summarized in the following table:
 
 | <<tx-multiple-tx-mgrs-with-attransactional,value>>
 | String
-|  Optional qualifier specifying the transaction manager to be used.
+| Optional qualifier specifying the transaction manager to be used.
 
 | <<tx-propagation,propagation>>
 | enum: `Propagation`
@@ -1309,15 +1308,15 @@ annotation are summarized in the following table:
 
 | `isolation`
 | enum: `Isolation`
-| Optional isolation level.
-
-| `readOnly`
-| boolean
-| Read/write vs. read-only transaction
+| Optional isolation level. Only applicable to propagation REQUIRED or REQUIRES_NEW.
 
 | `timeout`
 | int (in seconds granularity)
-| Transaction timeout.
+| Optional transaction timeout. Only applicable to propagation REQUIRED or REQUIRES_NEW.
+
+| `readOnly`
+| boolean
+| Read/write vs. read-only transaction. Only applicable to REQUIRED or REQUIRES_NEW.
 
 | `rollbackFor`
 | Array of `Class` objects, which must be derived from `Throwable.`
@@ -1451,11 +1450,28 @@ image::images/tx_prop_required.png[]
 
 PROPAGATION_REQUIRED
 
+`PROPAGATION_REQUIRED` enforces a physical transaction: either locally for the current
+scope if no transaction exists yet, or participating in an existing 'outer' transaction
+defined for a larger scope. This is a fine default in common call stack arrangements
+within the same thread, e.g. a service facade delegating to several repository methods
+where all the underlying resources have to participate in the service-level transaction.
+
+[NOTE]
+====
+By default, a participating transaction will join the characteristics of the outer scope,
+silently ignoring the local isolation level, timeout value or read-only flag (if any).
+Consider switching the "validateExistingTransactions" flag to "true" on your transaction
+manager if you'd like isolation level declarations to get rejected when participating in
+an existing transaction with a different isolation level. This non-lenient mode will also
+reject read-only mismatches, i.e. an inner read-write transaction trying to participate
+in a read-only outer scope.
+====
+
 When the propagation setting is `PROPAGATION_REQUIRED`, a __logical__ transaction scope
 is created for each method upon which the setting is applied. Each such logical
 transaction scope can determine rollback-only status individually, with an outer
-transaction scope being logically independent from the inner transaction scope. Of
-course, in case of standard `PROPAGATION_REQUIRED` behavior, all these scopes will be
+transaction scope being logically independent from the inner transaction scope.
+Of course, in case of standard `PROPAGATION_REQUIRED` behavior, all these scopes will be
 mapped to the same physical transaction. So a rollback-only marker set in the inner
 transaction scope does affect the outer transaction's chance to actually commit (as you
 would expect it to).
@@ -1477,11 +1493,14 @@ image::images/tx_prop_requires_new.png[]
 
 PROPAGATION_REQUIRES_NEW
 
-`PROPAGATION_REQUIRES_NEW`, in contrast to `PROPAGATION_REQUIRED`, uses a __completely__
-independent transaction for each affected transaction scope. In that case, the
-underlying physical transactions are different and hence can commit or roll back
+`PROPAGATION_REQUIRES_NEW`, in contrast to `PROPAGATION_REQUIRED`, always uses an
+__independent__ physical transaction for each affected transaction scope, never
+participating in an existing transaction for an outer scope. In such an arrangement,
+the underlying resource transactions are different and hence can commit or roll back
 independently, with an outer transaction not affected by an inner transaction's rollback
-status.
+status, and with an inner transaction's locks released immediately after its completion.
+Such an independent inner transaction may also declare its own isolation level, timeout
+and read-only settings, never inheriting an outer transaction's characteristics.
 
 [[tx-propagation-nested]]
 ===== Nested