Polish "Add option to allow Spring Batch custom isolation levels"

See gh-28859

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BasicBatchConfigurer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -26,6 +26,7 @@ import org.springframework.batch.core.launch.support.SimpleJobLauncher;
 import org.springframework.batch.core.repository.JobRepository;
 import org.springframework.batch.core.repository.support.JobRepositoryFactoryBean;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.boot.autoconfigure.batch.BatchProperties.Isolation;
 import org.springframework.boot.autoconfigure.transaction.TransactionManagerCustomizers;
 import org.springframework.boot.context.properties.PropertyMapper;
 import org.springframework.jdbc.datasource.DataSourceTransactionManager;
@@ -139,7 +140,8 @@ public class BasicBatchConfigurer implements BatchConfigurer, InitializingBean {
 	 * @return the isolation level or {@code null} to use the default
 	 */
 	protected String determineIsolationLevel() {
-		return this.properties.getJdbc().getIsolationLevelForCreate();
+		Isolation isolation = this.properties.getJdbc().getIsolationLevelForCreate();
+		return (isolation != null) ? isolation.toIsolationName() : null;
 	}
 
 	protected PlatformTransactionManager createTransactionManager() {

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/BatchProperties.java

@@ -66,6 +66,12 @@ public class BatchProperties {
 		private static final String DEFAULT_SCHEMA_LOCATION = "classpath:org/springframework/"
 				+ "batch/core/schema-@@platform@@.sql";
 
+		/**
+		 * Transaction isolation level to use when creating job meta-data for new jobs.
+		 * Auto-detected based on whether JPA is being used or not.
+		 */
+		private Isolation isolationLevelForCreate;
+
 		/**
 		 * Path to the SQL file to use to initialize the database schema.
 		 */
@@ -87,10 +93,13 @@ public class BatchProperties {
 		 */
 		private DatabaseInitializationMode initializeSchema = DatabaseInitializationMode.EMBEDDED;
 
-		/**
-		 * Transaction isolation level to use when creating job meta-data for new jobs.
-		 */
-		private String isolationLevelForCreate;
+		public Isolation getIsolationLevelForCreate() {
+			return this.isolationLevelForCreate;
+		}
+
+		public void setIsolationLevelForCreate(Isolation isolationLevelForCreate) {
+			this.isolationLevelForCreate = isolationLevelForCreate;
+		}
 
 		public String getSchema() {
 			return this.schema;
@@ -124,12 +133,45 @@ public class BatchProperties {
 			this.initializeSchema = initializeSchema;
 		}
 
-		public String getIsolationLevelForCreate() {
-			return this.isolationLevelForCreate;
-		}
+	}
 
-		public void setIsolationLevelForCreate(String isolationLevelForCreate) {
-			this.isolationLevelForCreate = isolationLevelForCreate;
+	/**
+	 * Available transaction isolation levels.
+	 */
+	public enum Isolation {
+
+		/**
+		 * Use the default isolation level of the underlying datastore.
+		 */
+		DEFAULT,
+
+		/**
+		 * Indicates that dirty reads, non-repeatable reads and phantom reads can occur.
+		 */
+		READ_UNCOMMITTED,
+
+		/**
+		 * Indicates that dirty reads are prevented; non-repeatable reads and phantom
+		 * reads can occur.
+		 */
+		READ_COMMITTED,
+
+		/**
+		 * Indicates that dirty reads and non-repeatable reads are prevented; phantom
+		 * reads can occur.
+		 */
+		REPEATABLE_READ,
+
+		/**
+		 * Indicate that dirty reads, non-repeatable reads and phantom reads are
+		 * prevented.
+		 */
+		SERIALIZABLE;
+
+		private static final String PREFIX = "ISOLATION_";
+
+		String toIsolationName() {
+			return PREFIX + name();
 		}
 
 	}

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/JpaBatchConfigurer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -22,6 +22,7 @@ import javax.sql.DataSource;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.boot.autoconfigure.batch.BatchProperties.Isolation;
 import org.springframework.boot.autoconfigure.transaction.TransactionManagerCustomizers;
 import org.springframework.orm.jpa.JpaTransactionManager;
 import org.springframework.transaction.PlatformTransactionManager;
@@ -38,8 +39,6 @@ public class JpaBatchConfigurer extends BasicBatchConfigurer {
 
 	private final EntityManagerFactory entityManagerFactory;
 
-	private final String isolationLevelForCreate;
-
 	/**
 	 * Create a new {@link BasicBatchConfigurer} instance.
 	 * @param properties the batch properties
@@ -52,18 +51,19 @@ public class JpaBatchConfigurer extends BasicBatchConfigurer {
 			TransactionManagerCustomizers transactionManagerCustomizers, EntityManagerFactory entityManagerFactory) {
 		super(properties, dataSource, transactionManagerCustomizers);
 		this.entityManagerFactory = entityManagerFactory;
-		this.isolationLevelForCreate = properties.getJdbc().getIsolationLevelForCreate();
 	}
 
 	@Override
 	protected String determineIsolationLevel() {
-		if (this.isolationLevelForCreate == null) {
-			logger.warn(
-					"JPA does not support custom isolation levels, so locks may not be taken when launching Jobs. Define spring.batch.jdbc.isolation-level-for-create property to force a custom isolation level.");
-			return "ISOLATION_DEFAULT";
+		String name = super.determineIsolationLevel();
+		if (name != null) {
+			return name;
+		}
+		else {
+			logger.warn("JPA does not support custom isolation levels, so locks may not be taken when launching Jobs. "
+					+ "To silence this warning, set 'spring.batch.jdbc.isolation-level-for-create' to 'default'.");
+			return Isolation.DEFAULT.toIsolationName();
 		}
-
-		return this.isolationLevelForCreate;
 	}
 
 	@Override

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java

@@ -23,6 +23,7 @@ import javax.persistence.EntityManagerFactory;
 import javax.sql.DataSource;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 
 import org.springframework.batch.core.BatchStatus;
 import org.springframework.batch.core.Job;
@@ -57,6 +58,8 @@ import org.springframework.boot.jdbc.init.DataSourceScriptDatabaseInitializer;
 import org.springframework.boot.sql.init.DatabaseInitializationMode;
 import org.springframework.boot.sql.init.DatabaseInitializationSettings;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.boot.test.system.CapturedOutput;
+import org.springframework.boot.test.system.OutputCaptureExtension;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Primary;
@@ -78,6 +81,7 @@ import static org.mockito.Mockito.mock;
  * @author Vedran Pavic
  * @author Kazuki Shimizu
  */
+@ExtendWith(OutputCaptureExtension.class)
 class BatchAutoConfigurationTests {
 
 	private final ApplicationContextRunner contextRunner = new ApplicationContextRunner()
@@ -208,8 +212,30 @@ class BatchAutoConfigurationTests {
 					// level)
 					assertThat(context.getBean(JobRepository.class).getLastJobExecution("job", new JobParameters()))
 							.isNull();
-					assertThat(context.getBean(JobRepository.class))
-							.satisfies(JobRepositoryTestingSupport.isolationLevelRequirements("ISOLATION_DEFAULT"));
+				});
+	}
+
+	@Test
+	void testDefaultIsolationLevelWithJpaLogsWarning(CapturedOutput output) {
+		this.contextRunner.withUserConfiguration(TestConfiguration.class, EmbeddedDataSourceConfiguration.class,
+				HibernateJpaAutoConfiguration.class).run((context) -> {
+					assertThat(context.getBean(BasicBatchConfigurer.class).determineIsolationLevel())
+							.isEqualTo("ISOLATION_DEFAULT");
+					assertThat(output).contains("JPA does not support custom isolation levels")
+							.contains("set 'spring.batch.jdbc.isolation-level-for-create' to 'default'");
+				});
+	}
+
+	@Test
+	void testCustomIsolationLevelWithJpaDoesNotLogWarning(CapturedOutput output) {
+		this.contextRunner.withPropertyValues("spring.batch.jdbc.isolation-level-for-create=default")
+				.withUserConfiguration(TestConfiguration.class, EmbeddedDataSourceConfiguration.class,
+						HibernateJpaAutoConfiguration.class)
+				.run((context) -> {
+					assertThat(context.getBean(BasicBatchConfigurer.class).determineIsolationLevel())
+							.isEqualTo("ISOLATION_DEFAULT");
+					assertThat(output).doesNotContain("JPA does not support custom isolation levels")
+							.doesNotContain("set 'spring.batch.jdbc.isolation-level-for-create' to 'default'");
 				});
 	}
 
@@ -234,16 +260,6 @@ class BatchAutoConfigurationTests {
 				});
 	}
 
-	@Test
-	void testCustomIsolationLevelForCreate() {
-		this.contextRunner
-				.withUserConfiguration(TestConfiguration.class, EmbeddedDataSourceConfiguration.class,
-						HibernateJpaAutoConfiguration.class)
-				.withPropertyValues("spring.batch.jdbc.isolation-level-for-create:ISOLATION_READ_COMMITTED")
-				.run((context) -> assertThat(context.getBean(JobRepository.class))
-						.satisfies(JobRepositoryTestingSupport.isolationLevelRequirements("ISOLATION_READ_COMMITTED")));
-	}
-
 	@Test
 	void testCustomizeJpaTransactionManagerUsingProperties() {
 		this.contextRunner

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationWithoutJpaTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -27,7 +27,6 @@ import org.springframework.batch.core.launch.JobLauncher;
 import org.springframework.batch.core.repository.JobRepository;
 import org.springframework.boot.autoconfigure.AutoConfigurations;
 import org.springframework.boot.autoconfigure.TestAutoConfigurationPackage;
-import org.springframework.boot.autoconfigure.batch.BatchProperties.Jdbc;
 import org.springframework.boot.autoconfigure.jdbc.EmbeddedDataSourceConfiguration;
 import org.springframework.boot.autoconfigure.orm.jpa.test.City;
 import org.springframework.boot.autoconfigure.transaction.TransactionAutoConfiguration;
@@ -38,7 +37,6 @@ import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.transaction.PlatformTransactionManager;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.from;
 
 /**
  * Tests for {@link BatchAutoConfiguration} when JPA is not on the classpath.
@@ -61,24 +59,19 @@ class BatchAutoConfigurationWithoutJpaTests {
 					assertThat(context).hasSingleBean(PlatformTransactionManager.class);
 					assertThat(context.getBean(PlatformTransactionManager.class).toString())
 							.contains("DataSourceTransactionManager");
-					assertThat(context.getBean(BatchProperties.class).getJdbc())
-							.returns("classpath:org/springframework/batch/core/schema-@@platform@@.sql",
-									from(Jdbc::getSchema))
-							.returns(DatabaseInitializationMode.EMBEDDED, from(Jdbc::getInitializeSchema))
-							.returns(null, from(Jdbc::getIsolationLevelForCreate));
+					assertThat(context.getBean(BatchProperties.class).getJdbc().getInitializeSchema())
+							.isEqualTo(DatabaseInitializationMode.EMBEDDED);
+					assertThat(context.getBean(BasicBatchConfigurer.class).determineIsolationLevel()).isNull();
 					assertThat(new JdbcTemplate(context.getBean(DataSource.class))
 							.queryForList("select * from BATCH_JOB_EXECUTION")).isEmpty();
 					assertThat(context.getBean(JobExplorer.class).findRunningJobExecutions("test")).isEmpty();
 					assertThat(context.getBean(JobRepository.class).getLastJobExecution("test", new JobParameters()))
 							.isNull();
-
-					assertThat(context.getBean(JobRepository.class)).satisfies(
-							JobRepositoryTestingSupport.isolationLevelRequirements("ISOLATION_SERIALIZABLE"));
 				});
 	}
 
 	@Test
-	void jdbcWithCustomSettings() {
+	void jdbcWithCustomPrefix() {
 		this.contextRunner.withUserConfiguration(DefaultConfiguration.class, EmbeddedDataSourceConfiguration.class)
 				.withPropertyValues("spring.datasource.generate-unique-name=true",
 						"spring.batch.jdbc.schema:classpath:batch/custom-schema-hsql.sql",
@@ -92,6 +85,15 @@ class BatchAutoConfigurationWithoutJpaTests {
 				});
 	}
 
+	@Test
+	void jdbcWithCustomIsolationLevel() {
+		this.contextRunner.withUserConfiguration(DefaultConfiguration.class, EmbeddedDataSourceConfiguration.class)
+				.withPropertyValues("spring.datasource.generate-unique-name=true",
+						"spring.batch.jdbc.isolation-level-for-create=read_committed")
+				.run((context) -> assertThat(context.getBean(BasicBatchConfigurer.class).determineIsolationLevel())
+						.isEqualTo("ISOLATION_READ_COMMITTED"));
+	}
+
 	@EnableBatchProcessing
 	@TestAutoConfigurationPackage(City.class)
 	static class DefaultConfiguration {

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/JobRepositoryTestingSupport.java

@@ -1,76 +0,0 @@
-/*
- * Copyright 2012-2021 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.
- * You may obtain a copy of the License at
- *
- *      https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.boot.autoconfigure.batch;
-
-import java.util.Arrays;
-import java.util.function.Consumer;
-import java.util.stream.Stream;
-import java.util.stream.Stream.Builder;
-
-import org.aopalliance.aop.Advice;
-import org.assertj.core.api.InstanceOfAssertFactories;
-
-import org.springframework.aop.Advisor;
-import org.springframework.aop.framework.Advised;
-import org.springframework.aop.support.AopUtils;
-import org.springframework.batch.core.repository.JobRepository;
-import org.springframework.transaction.interceptor.TransactionAspectSupport;
-
-import static org.assertj.core.api.Assertions.as;
-import static org.assertj.core.api.Assertions.assertThat;
-
-final class JobRepositoryTestingSupport {
-
-	private JobRepositoryTestingSupport() {
-
-	}
-
-	static Consumer<JobRepository> isolationLevelRequirements(String isolationLevel) {
-		return (jobRepository) ->
-		// jobRepository is proxied twice, the inner proxy has the transaction advice.
-		// This logic does not assume anything about proxy hierarchy, but it does about
-		// the advice itself.
-		assertThat(getTransactionAdvices(jobRepository))
-				.anySatisfy((advice) -> assertThat(advice).extracting("transactionAttributeSource")
-						.extracting(Object::toString, as(InstanceOfAssertFactories.STRING))
-						.contains("create*=PROPAGATION_REQUIRES_NEW," + isolationLevel)
-						.contains("getLastJobExecution*=PROPAGATION_REQUIRES_NEW," + isolationLevel));
-	}
-
-	private static Stream<Advice> getTransactionAdvices(Object candidate) {
-		Builder<Advice> builder = Stream.builder();
-		getTransactionAdvices(candidate, builder);
-		return builder.build();
-	}
-
-	private static void getTransactionAdvices(Object candidate, Builder<Advice> builder) {
-		try {
-			if (AopUtils.isAopProxy(candidate) && candidate instanceof Advised) {
-				Arrays.stream(((Advised) candidate).getAdvisors()).map(Advisor::getAdvice)
-						.filter(TransactionAspectSupport.class::isInstance).forEach(builder::add);
-				Object target = ((Advised) candidate).getTargetSource().getTarget();
-				if (target != null) {
-					getTransactionAdvices(target, builder);
-				}
-			}
-		}
-		catch (Exception ex) {
-			throw new IllegalStateException("Failed to unwrap proxied object", ex);
-		}
-	}
-
-}