From 9571aa1c68989c2860d9eb8f54af4b7f0a9e14a3 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 19 Jul 2023 18:05:41 +0300 Subject: [PATCH] Stop using Constants utility in IsolationLevelDataSourceAdapter See gh-30851 --- .../IsolationLevelDataSourceAdapter.java | 37 +++++--- .../IsolationLevelDataSourceAdapterTests.java | 95 +++++++++++++++++++ 2 files changed, 118 insertions(+), 14 deletions(-) create mode 100644 spring-jdbc/src/test/java/org/springframework/jdbc/datasource/IsolationLevelDataSourceAdapterTests.java diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/IsolationLevelDataSourceAdapter.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/IsolationLevelDataSourceAdapter.java index 48c9d8e288..9b8a33d5ba 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/IsolationLevelDataSourceAdapter.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/IsolationLevelDataSourceAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2023 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. @@ -18,12 +18,12 @@ package org.springframework.jdbc.datasource; import java.sql.Connection; import java.sql.SQLException; +import java.util.Map; -import org.springframework.core.Constants; import org.springframework.lang.Nullable; import org.springframework.transaction.TransactionDefinition; -import org.springframework.transaction.support.DefaultTransactionDefinition; import org.springframework.transaction.support.TransactionSynchronizationManager; +import org.springframework.util.Assert; /** * An adapter for a target {@link javax.sql.DataSource}, applying the current @@ -47,6 +47,7 @@ import org.springframework.transaction.support.TransactionSynchronizationManager * Make sure that the target DataSource properly cleans up such transaction state. * * @author Juergen Hoeller + * @author Sam Brannen * @since 2.0.3 * @see #setIsolationLevel * @see #setIsolationLevelName @@ -55,8 +56,18 @@ import org.springframework.transaction.support.TransactionSynchronizationManager */ public class IsolationLevelDataSourceAdapter extends UserCredentialsDataSourceAdapter { - /** Constants instance for TransactionDefinition. */ - private static final Constants constants = new Constants(TransactionDefinition.class); + /** + * Map of constant names to constant values for the isolation constants + * defined in {@link TransactionDefinition}. + */ + static final Map constants = Map.of( + "ISOLATION_DEFAULT", TransactionDefinition.ISOLATION_DEFAULT, + "ISOLATION_READ_UNCOMMITTED", TransactionDefinition.ISOLATION_READ_UNCOMMITTED, + "ISOLATION_READ_COMMITTED", TransactionDefinition.ISOLATION_READ_COMMITTED, + "ISOLATION_REPEATABLE_READ", TransactionDefinition.ISOLATION_REPEATABLE_READ, + "ISOLATION_SERIALIZABLE", TransactionDefinition.ISOLATION_SERIALIZABLE + ); + @Nullable private Integer isolationLevel; @@ -64,8 +75,8 @@ public class IsolationLevelDataSourceAdapter extends UserCredentialsDataSourceAd /** * Set the default isolation level by the name of the corresponding constant - * in {@link org.springframework.transaction.TransactionDefinition}, e.g. - * "ISOLATION_SERIALIZABLE". + * in {@link org.springframework.transaction.TransactionDefinition} — + * for example, {@code "ISOLATION_SERIALIZABLE"}. *

If not specified, the target DataSource's default will be used. * Note that a transaction-specific isolation value will always override * any isolation setting specified at the DataSource level. @@ -77,10 +88,10 @@ public class IsolationLevelDataSourceAdapter extends UserCredentialsDataSourceAd * @see #setIsolationLevel */ public final void setIsolationLevelName(String constantName) throws IllegalArgumentException { - if (!constantName.startsWith(DefaultTransactionDefinition.PREFIX_ISOLATION)) { - throw new IllegalArgumentException("Only isolation constants allowed"); - } - setIsolationLevel(constants.asNumber(constantName).intValue()); + Assert.hasText(constantName, "'constantName' must not be null or blank"); + Integer isolationLevel = constants.get(constantName); + Assert.notNull(isolationLevel, "Only isolation constants allowed"); + setIsolationLevel(isolationLevel); } /** @@ -103,9 +114,7 @@ public class IsolationLevelDataSourceAdapter extends UserCredentialsDataSourceAd * @see org.springframework.transaction.support.TransactionSynchronizationManager#getCurrentTransactionIsolationLevel() */ public void setIsolationLevel(int isolationLevel) { - if (!constants.getValues(DefaultTransactionDefinition.PREFIX_ISOLATION).contains(isolationLevel)) { - throw new IllegalArgumentException("Only values of isolation constants allowed"); - } + Assert.isTrue(constants.containsValue(isolationLevel), "Only values of isolation constants allowed"); this.isolationLevel = (isolationLevel != TransactionDefinition.ISOLATION_DEFAULT ? isolationLevel : null); } diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/IsolationLevelDataSourceAdapterTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/IsolationLevelDataSourceAdapterTests.java new file mode 100644 index 0000000000..20fac2c532 --- /dev/null +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/IsolationLevelDataSourceAdapterTests.java @@ -0,0 +1,95 @@ +/* + * Copyright 2002-2023 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.jdbc.datasource; + +import java.lang.reflect.Field; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; + +import org.springframework.transaction.TransactionDefinition; +import org.springframework.util.ReflectionUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.springframework.transaction.TransactionDefinition.ISOLATION_DEFAULT; +import static org.springframework.transaction.TransactionDefinition.ISOLATION_READ_COMMITTED; + +/** + * Tests for {@link IsolationLevelDataSourceAdapter}. + * + * @author Sam Brannen + * @since 6.1 + */ +class IsolationLevelDataSourceAdapterTests { + + private final IsolationLevelDataSourceAdapter adapter = new IsolationLevelDataSourceAdapter(); + + + @Test + void setIsolationLevelNameToUnsupportedValues() { + assertThatIllegalArgumentException().isThrownBy(() -> adapter.setIsolationLevelName(null)); + assertThatIllegalArgumentException().isThrownBy(() -> adapter.setIsolationLevelName(" ")); + assertThatIllegalArgumentException().isThrownBy(() -> adapter.setIsolationLevelName("bogus")); + } + + /** + * Verify that the internal 'constants' map is properly configured for all + * ISOLATION_ constants defined in {@link TransactionDefinition}. + */ + @Test + void setIsolationLevelNameToAllSupportedValues() { + Set uniqueValues = new HashSet<>(); + streamIsolationConstants() + .forEach(name -> { + adapter.setIsolationLevelName(name); + Integer isolationLevel = adapter.getIsolationLevel(); + if ("ISOLATION_DEFAULT".equals(name)) { + assertThat(isolationLevel).isNull(); + } + else { + Integer expected = IsolationLevelDataSourceAdapter.constants.get(name); + assertThat(isolationLevel).isEqualTo(expected); + } + uniqueValues.add(isolationLevel); + }); + assertThat(uniqueValues).hasSize(IsolationLevelDataSourceAdapter.constants.size()); + } + + @Test + void setIsolationLevel() { + assertThatIllegalArgumentException().isThrownBy(() -> adapter.setIsolationLevel(999)); + + adapter.setIsolationLevel(ISOLATION_DEFAULT); + assertThat(adapter.getIsolationLevel()).isNull(); + + adapter.setIsolationLevel(ISOLATION_READ_COMMITTED); + assertThat(adapter.getIsolationLevel()).isEqualTo(ISOLATION_READ_COMMITTED); + } + + + private static Stream streamIsolationConstants() { + return Arrays.stream(TransactionDefinition.class.getFields()) + .filter(ReflectionUtils::isPublicStaticFinal) + .map(Field::getName) + .filter(name -> name.startsWith("ISOLATION_")); + } + +}