Revise contribution

See gh-35380

spring-core/src/main/java/org/springframework/util/function/SingletonSupplier.java

@@ -48,6 +48,7 @@ public class SingletonSupplier<T extends @Nullable Object> implements Supplier<T
 	private volatile @Nullable T singletonInstance;
 
 	private volatile boolean initialized;
+
 	/**
 	 * Guards access to write operations on the {@code singletonInstance} field.
 	 */
@@ -97,16 +98,19 @@ public class SingletonSupplier<T extends @Nullable Object> implements Supplier<T
 	 */
 	@Override
 	public @Nullable T get() {
+		T instance = this.singletonInstance;
 		if (!this.initialized) {
 			this.writeLock.lock();
 			try {
+				instance = this.singletonInstance;
 				if (!this.initialized) {
 					if (this.instanceSupplier != null) {
-						this.singletonInstance = this.instanceSupplier.get();
+						instance = this.instanceSupplier.get();
 					}
-					if (this.singletonInstance == null && this.defaultSupplier != null) {
-						this.singletonInstance = this.defaultSupplier.get();
+					if (instance == null && this.defaultSupplier != null) {
+						instance = this.defaultSupplier.get();
 					}
+					this.singletonInstance = instance;
 					this.initialized = true;
 				}
 			}
@@ -114,7 +118,7 @@ public class SingletonSupplier<T extends @Nullable Object> implements Supplier<T
 				this.writeLock.unlock();
 			}
 		}
-		return this.singletonInstance;
+		return instance;
 	}
 
 	/**

spring-core/src/test/java/org/springframework/util/function/SingletonSupplierTests.java

@@ -17,9 +17,7 @@
 package org.springframework.util.function;
 
 import java.util.ArrayList;
-import java.util.List;
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -29,123 +27,129 @@ import org.junit.jupiter.api.RepeatedTest;
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 
 /**
  * Tests for {@link SingletonSupplier}.
  *
  * @author Dmytro Nosan
+ * @author Sam Brannen
+ * @since 7.0
  */
 class SingletonSupplierTests {
 
 	@Test
 	void shouldReturnDefaultWhenInstanceSupplierReturnsNull() {
-		SingletonSupplier<String> singletonSupplier = new SingletonSupplier<>(() -> null, () -> "Default");
+		var singletonSupplier = new SingletonSupplier<>(() -> null, () -> "Default");
+
 		assertThat(singletonSupplier.get()).isEqualTo("Default");
 	}
 
 	@Test
 	void shouldReturnNullForOfNullableWithNullInstance() {
-		SingletonSupplier<String> singletonSupplier = SingletonSupplier.ofNullable((String) null);
+		var singletonSupplier = SingletonSupplier.ofNullable((String) null);
+
 		assertThat(singletonSupplier).isNull();
 	}
 
 	@Test
 	void shouldReturnNullForOfNullableWithNullSupplier() {
-		SingletonSupplier<String> singletonSupplier = SingletonSupplier.ofNullable((Supplier<String>) null);
+		var singletonSupplier = SingletonSupplier.ofNullable((Supplier<String>) null);
+
 		assertThat(singletonSupplier).isNull();
 	}
 
 	@Test
 	void shouldReturnNullWhenAllSuppliersReturnNull() {
-		SingletonSupplier<String> singletonSupplier = new SingletonSupplier<>(() -> null, () -> null);
+		var singletonSupplier = new SingletonSupplier<>(() -> null, () -> null);
+
 		assertThat(singletonSupplier.get()).isNull();
 	}
 
 	@Test
 	void shouldReturnNullWhenNoInstanceOrDefaultSupplier() {
-		SingletonSupplier<String> singletonSupplier = new SingletonSupplier<>((String) null, null);
+		var singletonSupplier = new SingletonSupplier<>((String) null, null);
+
 		assertThat(singletonSupplier.get()).isNull();
 	}
 
 	@Test
 	void shouldReturnSingletonInstanceOnMultipleCalls() {
-		SingletonSupplier<String> singletonSupplier = SingletonSupplier.of("Hello");
+		var singletonSupplier = SingletonSupplier.of("Hello");
+
 		assertThat(singletonSupplier.get()).isEqualTo("Hello");
 		assertThat(singletonSupplier.get()).isEqualTo("Hello");
 	}
 
-
 	@Test
 	void shouldReturnSingletonInstanceOnMultipleSupplierCalls() {
-		SingletonSupplier<String> singletonSupplier = SingletonSupplier.of(new HelloStringSupplier());
+		var singletonSupplier = SingletonSupplier.of(new HelloStringSupplier());
+
 		assertThat(singletonSupplier.get()).isEqualTo("Hello 0");
 		assertThat(singletonSupplier.get()).isEqualTo("Hello 0");
 	}
 
 	@Test
 	void shouldReturnSupplierForOfNullableWithNonNullInstance() {
-		SingletonSupplier<String> singletonSupplier = SingletonSupplier.ofNullable("Hello");
+		var singletonSupplier = SingletonSupplier.ofNullable("Hello");
+
 		assertThat(singletonSupplier).isNotNull();
 		assertThat(singletonSupplier.get()).isEqualTo("Hello");
 	}
 
 	@Test
 	void shouldReturnSupplierForOfNullableWithNonNullSupplier() {
-		SingletonSupplier<String> singletonSupplier = SingletonSupplier.ofNullable(() -> "Hello");
+		var singletonSupplier = SingletonSupplier.ofNullable(() -> "Hello");
+
 		assertThat(singletonSupplier).isNotNull();
 		assertThat(singletonSupplier.get()).isEqualTo("Hello");
 	}
 
 	@Test
 	void shouldThrowWhenObtainCalledAndNoInstanceAvailable() {
-		SingletonSupplier<String> singletonSupplier = new SingletonSupplier<>((String) null, null);
-		assertThatThrownBy(singletonSupplier::obtain).isInstanceOf(IllegalStateException.class)
-				.hasMessage("No instance from Supplier");
+		var singletonSupplier = new SingletonSupplier<>((String) null, null);
+
+		assertThatIllegalStateException()
+				.isThrownBy(singletonSupplier::obtain)
+				.withMessage("No instance from Supplier");
 	}
 
 	@Test
 	void shouldUseDefaultSupplierWhenInstanceIsNull() {
-		SingletonSupplier<String> singletonSupplier = new SingletonSupplier<>((String) null, () -> "defaultSupplier");
+		var singletonSupplier = new SingletonSupplier<>((String) null, () -> "defaultSupplier");
+
 		assertThat(singletonSupplier.get()).isEqualTo("defaultSupplier");
 	}
 
 	@Test
 	void shouldUseDefaultSupplierWhenInstanceSupplierReturnsNull() {
-		SingletonSupplier<String> singletonSupplier = new SingletonSupplier<>((Supplier<String>) null, () -> "defaultSupplier");
+		var singletonSupplier = new SingletonSupplier<>((Supplier<String>) null, () -> "defaultSupplier");
+
 		assertThat(singletonSupplier.get()).isEqualTo("defaultSupplier");
 	}
 
 	@Test
 	void shouldUseInstanceSupplierWhenProvidedAndIgnoreDefaultSupplier() {
-		AtomicInteger defaultValue = new AtomicInteger();
-		SingletonSupplier<Integer> singletonSupplier = new SingletonSupplier<>(() -> -1, defaultValue::incrementAndGet);
+		var defaultValue = new AtomicInteger();
+		var singletonSupplier = new SingletonSupplier<>(() -> -1, defaultValue::incrementAndGet);
+
 		assertThat(singletonSupplier.get()).isEqualTo(-1);
-		assertThat(defaultValue.get()).isEqualTo(0);
+		assertThat(defaultValue.get()).isZero();
 	}
 
 	@Test
 	void shouldUseInstanceWhenProvidedAndIgnoreDefaultSupplier() {
-		AtomicInteger defaultValue = new AtomicInteger();
-		SingletonSupplier<Integer> singletonSupplier = new SingletonSupplier<>(-1, defaultValue::incrementAndGet);
+		var defaultValue = new AtomicInteger();
+		var singletonSupplier = new SingletonSupplier<>(-1, defaultValue::incrementAndGet);
+
 		assertThat(singletonSupplier.get()).isEqualTo(-1);
-		assertThat(defaultValue.get()).isEqualTo(0);
+		assertThat(defaultValue.get()).isZero();
 	}
 
 	@Test
 	void shouldReturnConsistentlyNullSingletonInstanceOnMultipleSupplierCalls() {
-		SingletonSupplier<String> singletonSupplier = SingletonSupplier.of(new Supplier<>() {
-
-			int count = 0;
-
-			@Override
-			public String get() {
-				if (this.count++ == 0) {
-					return null;
-				}
-				return "Hello";
-			}
-		});
+		var count = new AtomicInteger();
+		var singletonSupplier = SingletonSupplier.of(() -> (count.getAndIncrement() == 0 ? null : "Hello"));
 
 		assertThat(singletonSupplier.get()).isNull();
 		assertThat(singletonSupplier.get()).isNull();
@@ -154,11 +158,12 @@ class SingletonSupplierTests {
 	@RepeatedTest(100)
 	void shouldReturnSingletonInstanceOnMultipleConcurrentSupplierCalls() throws Exception {
 		int numberOfThreads = 4;
-		CountDownLatch ready = new CountDownLatch(numberOfThreads);
-		CountDownLatch start = new CountDownLatch(1);
-		List<Future<String>> futures = new ArrayList<>();
-		SingletonSupplier<String> singletonSupplier = SingletonSupplier.of(new HelloStringSupplier());
-		ExecutorService executorService = Executors.newFixedThreadPool(numberOfThreads);
+		var ready = new CountDownLatch(numberOfThreads);
+		var start = new CountDownLatch(1);
+		var futures = new ArrayList<Future<String>>();
+		var singletonSupplier = SingletonSupplier.of(new HelloStringSupplier());
+		var executorService = Executors.newFixedThreadPool(numberOfThreads);
+
 		try {
 			for (int i = 0; i < numberOfThreads; i++) {
 				futures.add(executorService.submit(() -> {
@@ -179,7 +184,7 @@ class SingletonSupplierTests {
 
 	private static final class HelloStringSupplier implements Supplier<String> {
 
-		private final AtomicInteger count = new AtomicInteger();
+		final AtomicInteger count = new AtomicInteger();
 
 		@Override
 		public String get() {

spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslatorTests.java

@@ -39,7 +39,6 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 
 /**
@@ -191,7 +190,7 @@ class SQLErrorCodeSQLExceptionTranslatorTests {
 	}
 
 	@Test
-	void dataSourceInitialization() throws Exception {
+	void dataSourceInitializationWhenConnectionCannotBeObtained() throws Exception {
 		SQLException connectionException = new SQLException();
 		SQLException duplicateKeyException = new SQLException("test", "", 1);
 
@@ -200,6 +199,11 @@ class SQLErrorCodeSQLExceptionTranslatorTests {
 
 		translator = new SQLErrorCodeSQLExceptionTranslator(dataSource);
 		assertThat(translator.translate("test", null, duplicateKeyException)).isNull();
+	}
+
+	@Test
+	void dataSourceInitialization() throws Exception {
+		SQLException duplicateKeyException = new SQLException("test", "", 1);
 
 		DatabaseMetaData databaseMetaData = mock();
 		given(databaseMetaData.getDatabaseProductName()).willReturn("Oracle");
@@ -207,8 +211,9 @@ class SQLErrorCodeSQLExceptionTranslatorTests {
 		Connection connection = mock();
 		given(connection.getMetaData()).willReturn(databaseMetaData);
 
-		reset(dataSource);
+		DataSource dataSource = mock();
 		given(dataSource.getConnection()).willReturn(connection);
+
 		translator = new SQLErrorCodeSQLExceptionTranslator(dataSource);
 		assertThat(translator.translate("test", null, duplicateKeyException))
 				.isInstanceOf(DuplicateKeyException.class);