Improve transaction management for @SQL scripts

Prior to this commit, the support for SQL script execution via @SQL
provided an algorithm for looking up a required
PlatformTransactionManager to use to drive transactions. However, a
transaction manager is not actually required for all testing scenarios.

This commit improves the transaction management support for @SQL so
that SQL scripts can be executed without a transaction if a transaction
manger is not present in the ApplicationContext. The updated algorithm
now supports the following use cases.

 - If a transaction manager and data source are both present (i.e.,
   explicitly specified via the transactionManager and dataSource
   attributes of @SqlConfig or implicitly discovered in the
   ApplicationContext based on conventions), both will be used.

 - If a transaction manager is not explicitly specified and not
   implicitly discovered based on conventions, SQL scripts will be
   executed without a transaction but requiring the presence of a data
   source. If a data source is not present, an exception will be thrown.

 - If a data source is not explicitly specified and not implicitly
   discovered based on conventions, an attempt will be made to retrieve
   it by using reflection to invoke a public method named
   getDataSource() on the transaction manager. If this attempt fails,
   an exception will be thrown.

 - If a data source can be retrieved from the resolved transaction
   manager using reflection, an exception will be thrown if the
   resolved data source is not the data source associated with the
   resolved transaction manager. This helps to avoid possibly
   unintended configuration errors.

 - If @SqlConfig.transactionMode is set to ISOLATED, an exception will
   be thrown if a transaction manager is not present.

Issue: SPR-11911

Author: sbrannen

spring-test/src/main/java/org/springframework/test/context/jdbc/Sql.java

@@ -22,9 +22,6 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 
-import org.springframework.jdbc.datasource.init.ScriptUtils;
-import org.springframework.util.ResourceUtils;
-
 import static java.lang.annotation.ElementType.*;
 import static java.lang.annotation.RetentionPolicy.*;
 
@@ -39,8 +36,8 @@
  *
  * <p>The configuration options provided by this annotation and
  * {@link SqlConfig @SqlConfig} are equivalent to those supported by
- * {@link ScriptUtils} and
- * {@link org.springframework.jdbc.datasource.init.ResourceDatabasePopulator}
+ * {@link org.springframework.jdbc.datasource.init.ScriptUtils ScriptUtils} and
+ * {@link org.springframework.jdbc.datasource.init.ResourceDatabasePopulator ResourceDatabasePopulator}
  * but are a superset of those provided by the {@code <jdbc:initialize-database/>}
  * XML namespace element. Consult the javadocs of individual attributes in this
  * annotation and {@link SqlConfig @SqlConfig} for details.
@@ -110,9 +107,9 @@ static enum ExecutionPhase {
 	 * <em>absolute</em> classpath resource, for example:
 	 * {@code "/org/example/schema.sql"}. A path which references a
 	 * URL (e.g., a path prefixed with
-	 * {@link ResourceUtils#CLASSPATH_URL_PREFIX classpath:},
-	 * {@link ResourceUtils#FILE_URL_PREFIX file:}, {@code http:}, etc.) will be
-	 * loaded using the specified resource protocol.
+	 * {@link org.springframework.util.ResourceUtils#CLASSPATH_URL_PREFIX classpath:},
+	 * {@link org.springframework.util.ResourceUtils#FILE_URL_PREFIX file:},
+	 * {@code http:}, etc.) will be loaded using the specified resource protocol.
 	 * <h3>Default Script Detection</h3>
 	 * <p>If no SQL scripts are specified, an attempt will be made to detect a
 	 * <em>default</em> script depending on where this annotation is declared.

spring-test/src/main/java/org/springframework/test/context/jdbc/SqlConfig.java

@@ -21,8 +21,6 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 
-import org.springframework.jdbc.datasource.init.ScriptUtils;
-
 import static java.lang.annotation.ElementType.*;
 import static java.lang.annotation.RetentionPolicy.*;
 
@@ -46,7 +44,7 @@
  * attribute. Thus, in order to support overrides of <em>inherited</em> global
  * configuration, {@code @SqlConfig} attributes have an <em>explicit</em>
  * {@code default} value of either {@code ""} for Strings or {@code DEFAULT} for
- * Enums. This approach allows local declarations {@code @SqlConfig} to
+ * Enums. This approach allows local declarations of {@code @SqlConfig} to
  * selectively override individual attributes from global declarations of
  * {@code @SqlConfig} by providing a value other than {@code ""} or {@code DEFAULT}.
  *
@@ -91,22 +89,50 @@ static enum TransactionMode {
 		DEFAULT,
 
 		/**
-		 * Indicates that the transaction mode to use when executing SQL scripts
-		 * should be <em>inferred</em> based on whether or not a Spring-managed
-		 * transaction is currently present.
-		 * <p>SQL scripts will be executed within the current transaction if present;
-		 * otherwise, scripts will be executed in a new transaction that will be
-		 * immediately committed.
-		 * <p>The <em>current</em> transaction will typically be managed by the
-		 * {@link org.springframework.test.context.transaction.TransactionalTestExecutionListener
-		 * TransactionalTestExecutionListener}.
+		 * Indicates that the transaction mode to use when executing SQL
+		 * scripts should be <em>inferred</em> using the rules listed below.
+		 * In the context of these rules, the term "<em>available</em>"
+		 * means that the bean for the data source or transaction manager
+		 * is either explicitly specified via a corresponding annotation
+		 * attribute in {@code @SqlConfig} or discoverable via conventions. See
+		 * {@link org.springframework.test.context.transaction.TestContextTransactionUtils TestContextTransactionUtils}
+		 * for details on the conventions used to discover such beans in
+		 * the {@code ApplicationContext}.
+		 *
+		 * <h4>Inference Rules</h4>
+		 * <ol>
+		 * <li>If neither a transaction manager nor a data source is
+		 * available, an exception will be thrown.
+		 * <li>If a transaction manager is not available but a data source
+		 * is available, SQL scripts will be executed directly against the
+		 * data source without a transaction.
+		 * <li>If a transaction manager is available:
+		 * <ul>
+		 * <li>If a data source is not available, an attempt will be made
+		 * to retrieve it from the transaction manager by using reflection
+		 * to invoke a public method named {@code getDataSource()} on the
+		 * transaction manager. If the attempt fails, an exception will be
+		 * thrown.
+		 * <li>Using the resolved transaction manager and data source, SQL
+		 * scripts will be executed within an existing transaction if
+		 * present; otherwise, scripts will be executed in a new transaction
+		 * that will be immediately committed. An <em>existing</em>
+		 * transaction will typically be managed by the
+		 * {@link org.springframework.test.context.transaction.TransactionalTestExecutionListener TransactionalTestExecutionListener}.
+		 * </ul>
+		 * </ol>
 		 * @see #ISOLATED
+		 * @see org.springframework.test.context.transaction.TestContextTransactionUtils#retrieveDataSource
+		 * @see org.springframework.test.context.transaction.TestContextTransactionUtils#retrieveTransactionManager
 		 */
 		INFERRED,
 
 		/**
 		 * Indicates that SQL scripts should always be executed in a new,
 		 * <em>isolated</em> transaction that will be immediately committed.
+		 * <p>In contrast to {@link #INFERRED}, this mode requires the
+		 * presence of a transaction manager <strong>and</strong> a data
+		 * source.
 		 */
 		ISOLATED
 	}
@@ -164,18 +190,24 @@ static enum ErrorMode {
 
 
 	/**
-	 * The bean name of the {@link javax.sql.DataSource} against which the scripts
-	 * should be executed.
-	 * <p>The name is only used if there is more than one bean of type
-	 * {@code DataSource} in the test's {@code ApplicationContext}. If there is
-	 * only one such bean, it is not necessary to specify a bean name.
+	 * The bean name of the {@link javax.sql.DataSource} against which the
+	 * scripts should be executed.
+	 * <p>The name is only required if there is more than one bean of type
+	 * {@code DataSource} in the test's {@code ApplicationContext}. If there
+	 * is only one such bean, it is not necessary to specify a bean name.
 	 * <p>Defaults to an empty string, requiring that one of the following is
 	 * true:
 	 * <ol>
+	 * <li>An explicit bean name is defined in a global declaration of
+	 * {@code @SqlConfig}.
+	 * <li>The data source can be retrieved from the transaction manager
+	 * by using reflection to invoke a public method named
+	 * {@code getDataSource()} on the transaction manager.
 	 * <li>There is only one bean of type {@code DataSource} in the test's
 	 * {@code ApplicationContext}.</li>
 	 * <li>The {@code DataSource} to use is named {@code "dataSource"}.</li>
 	 * </ol>
+	 * @see org.springframework.test.context.transaction.TestContextTransactionUtils#retrieveDataSource
 	 */
 	String dataSource() default "";
 
@@ -188,6 +220,8 @@ static enum ErrorMode {
 	 * <p>Defaults to an empty string, requiring that one of the following is
 	 * true:
 	 * <ol>
+	 * <li>An explicit bean name is defined in a global declaration of
+	 * {@code @SqlConfig}.
 	 * <li>There is only one bean of type {@code PlatformTransactionManager} in
 	 * the test's {@code ApplicationContext}.</li>
 	 * <li>{@link org.springframework.transaction.annotation.TransactionManagementConfigurer
@@ -197,6 +231,7 @@ static enum ErrorMode {
 	 * <li>The {@code PlatformTransactionManager} to use is named
 	 * {@code "transactionManager"}.</li>
 	 * </ol>
+	 * @see org.springframework.test.context.transaction.TestContextTransactionUtils#retrieveTransactionManager
 	 */
 	String transactionManager() default "";
 
@@ -223,32 +258,35 @@ static enum ErrorMode {
 	 * SQL scripts.
 	 * <p>Implicitly defaults to {@code ";"} if not specified and falls back to
 	 * {@code "\n"} as a last resort.
-	 * <p>May be set to {@link ScriptUtils#EOF_STATEMENT_SEPARATOR} to signal
-	 * that each script contains a single statement without a separator.
-	 * @see ScriptUtils#DEFAULT_STATEMENT_SEPARATOR
+	 * <p>May be set to
+	 * {@link org.springframework.jdbc.datasource.init.ScriptUtils#EOF_STATEMENT_SEPARATOR}
+	 * to signal that each script contains a single statement without a
+	 * separator.
+	 * @see org.springframework.jdbc.datasource.init.ScriptUtils#DEFAULT_STATEMENT_SEPARATOR
+	 * @see org.springframework.jdbc.datasource.init.ScriptUtils#EOF_STATEMENT_SEPARATOR
 	 */
 	String separator() default "";
 
 	/**
 	 * The prefix that identifies single-line comments within the SQL scripts.
 	 * <p>Implicitly defaults to {@code "--"}.
-	 * @see ScriptUtils#DEFAULT_COMMENT_PREFIX
+	 * @see org.springframework.jdbc.datasource.init.ScriptUtils#DEFAULT_COMMENT_PREFIX
 	 */
 	String commentPrefix() default "";
 
 	/**
 	 * The start delimiter that identifies block comments within the SQL scripts.
 	 * <p>Implicitly defaults to {@code "/*"}.
 	 * @see #blockCommentEndDelimiter
-	 * @see ScriptUtils#DEFAULT_BLOCK_COMMENT_START_DELIMITER
+	 * @see org.springframework.jdbc.datasource.init.ScriptUtils#DEFAULT_BLOCK_COMMENT_START_DELIMITER
 	 */
 	String blockCommentStartDelimiter() default "";
 
 	/**
 	 * The end delimiter that identifies block comments within the SQL scripts.
 	 * <p>Implicitly defaults to <code>"*&#47;"</code>.
 	 * @see #blockCommentStartDelimiter
-	 * @see ScriptUtils#DEFAULT_BLOCK_COMMENT_END_DELIMITER
+	 * @see org.springframework.jdbc.datasource.init.ScriptUtils#DEFAULT_BLOCK_COMMENT_END_DELIMITER
 	 */
 	String blockCommentEndDelimiter() default "";
 

spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java

@@ -43,6 +43,7 @@
 import org.springframework.transaction.support.TransactionTemplate;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
+import org.springframework.util.ReflectionUtils;
 import org.springframework.util.ResourceUtils;
 
 /**
@@ -165,24 +166,77 @@ private void executeSqlScripts(Sql sql, ExecutionPhase executionPhase, TestConte
 			logger.debug("Executing SQL scripts: " + ObjectUtils.nullSafeToString(scripts));
 		}
 
-		final DataSource dataSource = TestContextTransactionUtils.retrieveDataSource(testContext,
-			mergedSqlConfig.getDataSource());
+		String dsName = mergedSqlConfig.getDataSource();
+		String tmName = mergedSqlConfig.getTransactionManager();
+		DataSource dataSource = TestContextTransactionUtils.retrieveDataSource(testContext, dsName);
 		final PlatformTransactionManager transactionManager = TestContextTransactionUtils.retrieveTransactionManager(
-			testContext, mergedSqlConfig.getTransactionManager());
+			testContext, tmName);
+		final boolean newTxRequired = mergedSqlConfig.getTransactionMode() == TransactionMode.ISOLATED;
 
-		int propagation = (mergedSqlConfig.getTransactionMode() == TransactionMode.ISOLATED) ? TransactionDefinition.PROPAGATION_REQUIRES_NEW
-				: TransactionDefinition.PROPAGATION_REQUIRED;
+		if (transactionManager == null) {
+			if (newTxRequired) {
+				throw new IllegalStateException(String.format("Failed to execute SQL scripts for test context %s: "
+						+ "cannot execute SQL scripts using Transaction Mode "
+						+ "[%s] without a PlatformTransactionManager.", testContext, TransactionMode.ISOLATED));
+			}
+
+			if (dataSource == null) {
+				throw new IllegalStateException(String.format("Failed to execute SQL scripts for test context %s: "
+						+ "supply at least a DataSource or PlatformTransactionManager.", testContext));
+			}
 
-		TransactionAttribute transactionAttribute = TestContextTransactionUtils.createDelegatingTransactionAttribute(
-			testContext, new DefaultTransactionAttribute(propagation));
+			// Execute scripts directly against the DataSource
+			populator.execute(dataSource);
+		}
+		else {
+			DataSource dataSourceFromTxMgr = getDataSourceFromTransactionManager(transactionManager);
+
+			// Ensure user configured an appropriate DataSource/TransactionManager pair.
+			if ((dataSource != null) && (dataSourceFromTxMgr != null) && !dataSource.equals(dataSourceFromTxMgr)) {
+				throw new IllegalStateException(String.format("Failed to execute SQL scripts for test context %s: "
+						+ "the configured DataSource [%s] (named '%s') is not the one associated "
+						+ "with transaction manager [%s] (named '%s').", testContext, dataSource.getClass().getName(),
+					dsName, transactionManager.getClass().getName(), tmName));
+			}
+
+			if (dataSource == null) {
+				dataSource = dataSourceFromTxMgr;
+				if (dataSource == null) {
+					throw new IllegalStateException(String.format("Failed to execute SQL scripts for test context %s: "
+							+ "could not obtain DataSource from transaction manager [%s] (named '%s').", testContext,
+						transactionManager.getClass().getName(), tmName));
+				}
+			}
 
-		new TransactionTemplate(transactionManager, transactionAttribute).execute(new TransactionCallbackWithoutResult() {
+			final DataSource finalDataSource = dataSource;
+			int propagation = newTxRequired ? TransactionDefinition.PROPAGATION_REQUIRES_NEW
+					: TransactionDefinition.PROPAGATION_REQUIRED;
 
-			@Override
-			public void doInTransactionWithoutResult(TransactionStatus status) {
-				populator.execute(dataSource);
-			};
-		});
+			TransactionAttribute transactionAttribute = TestContextTransactionUtils.createDelegatingTransactionAttribute(
+				testContext, new DefaultTransactionAttribute(propagation));
+
+			new TransactionTemplate(transactionManager, transactionAttribute).execute(new TransactionCallbackWithoutResult() {
+
+				@Override
+				public void doInTransactionWithoutResult(TransactionStatus status) {
+					populator.execute(finalDataSource);
+				}
+			});
+		}
+	}
+
+	private DataSource getDataSourceFromTransactionManager(PlatformTransactionManager transactionManager) {
+		try {
+			Method getDataSourceMethod = transactionManager.getClass().getMethod("getDataSource");
+			Object obj = ReflectionUtils.invokeMethod(getDataSourceMethod, transactionManager);
+			if (obj instanceof DataSource) {
+				return (DataSource) obj;
+			}
+		}
+		catch (Exception e) {
+			/* ignore */
+		}
+		return null;
 	}
 
 	private String[] getScripts(Sql sql, TestContext testContext, boolean classLevel) {