diff --git a/spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java b/spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java index 3721c8df3f00..266735a42a2b 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java +++ b/spring-tx/src/main/java/org/springframework/transaction/support/AbstractPlatformTransactionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 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. @@ -473,11 +473,10 @@ private TransactionStatus handleExistingTransaction( if (definition.getIsolationLevel() != TransactionDefinition.ISOLATION_DEFAULT) { Integer currentIsolationLevel = TransactionSynchronizationManager.getCurrentTransactionIsolationLevel(); if (currentIsolationLevel == null || currentIsolationLevel != definition.getIsolationLevel()) { - Constants isoConstants = DefaultTransactionDefinition.constants; throw new IllegalTransactionStateException("Participating transaction with definition [" + definition + "] specifies isolation level which is incompatible with existing transaction: " + (currentIsolationLevel != null ? - isoConstants.toCode(currentIsolationLevel, DefaultTransactionDefinition.PREFIX_ISOLATION) : + DefaultTransactionDefinition.getIsolationLevelName(currentIsolationLevel) : "(unknown)")); } } diff --git a/spring-tx/src/main/java/org/springframework/transaction/support/DefaultTransactionDefinition.java b/spring-tx/src/main/java/org/springframework/transaction/support/DefaultTransactionDefinition.java index c42a0f8208e4..cea1f3a4d62a 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/support/DefaultTransactionDefinition.java +++ b/spring-tx/src/main/java/org/springframework/transaction/support/DefaultTransactionDefinition.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. @@ -17,10 +17,11 @@ package org.springframework.transaction.support; import java.io.Serializable; +import java.util.Map; -import org.springframework.core.Constants; import org.springframework.lang.Nullable; import org.springframework.transaction.TransactionDefinition; +import org.springframework.util.Assert; /** * Default implementation of the {@link TransactionDefinition} interface, @@ -31,6 +32,7 @@ * {@link org.springframework.transaction.interceptor.DefaultTransactionAttribute}. * * @author Juergen Hoeller + * @author Sam Brannen * @since 08.05.2003 */ @SuppressWarnings("serial") @@ -49,8 +51,31 @@ public class DefaultTransactionDefinition implements TransactionDefinition, Seri public static final String READ_ONLY_MARKER = "readOnly"; - /** Constants instance for TransactionDefinition. */ - static final Constants constants = new Constants(TransactionDefinition.class); + /** + * Map of constant names to constant values for the propagation constants + * defined in {@link TransactionDefinition}. + */ + static final Map propagationConstants = Map.of( + "PROPAGATION_REQUIRED", TransactionDefinition.PROPAGATION_REQUIRED, + "PROPAGATION_SUPPORTS", TransactionDefinition.PROPAGATION_SUPPORTS, + "PROPAGATION_MANDATORY", TransactionDefinition.PROPAGATION_MANDATORY, + "PROPAGATION_REQUIRES_NEW", TransactionDefinition.PROPAGATION_REQUIRES_NEW, + "PROPAGATION_NOT_SUPPORTED", TransactionDefinition.PROPAGATION_NOT_SUPPORTED, + "PROPAGATION_NEVER", TransactionDefinition.PROPAGATION_NEVER, + "PROPAGATION_NESTED", TransactionDefinition.PROPAGATION_NESTED + ); + + /** + * Map of constant names to constant values for the isolation constants + * defined in {@link TransactionDefinition}. + */ + static final Map isolationConstants = 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 + ); private int propagationBehavior = PROPAGATION_REQUIRED; @@ -108,7 +133,7 @@ public DefaultTransactionDefinition(int propagationBehavior) { /** * Set the propagation behavior by the name of the corresponding constant in - * TransactionDefinition, e.g. "PROPAGATION_REQUIRED". + * {@link TransactionDefinition} — for example, {@code "PROPAGATION_REQUIRED"}. * @param constantName name of the constant * @throws IllegalArgumentException if the supplied value is not resolvable * to one of the {@code PROPAGATION_} constants or is {@code null} @@ -116,10 +141,10 @@ public DefaultTransactionDefinition(int propagationBehavior) { * @see #PROPAGATION_REQUIRED */ public final void setPropagationBehaviorName(String constantName) throws IllegalArgumentException { - if (!constantName.startsWith(PREFIX_PROPAGATION)) { - throw new IllegalArgumentException("Only propagation constants allowed"); - } - setPropagationBehavior(constants.asNumber(constantName).intValue()); + Assert.hasText(constantName, "'constantName' must not be null or blank"); + Integer propagationBehavior = propagationConstants.get(constantName); + Assert.notNull(propagationBehavior, "Only propagation behavior constants allowed"); + this.propagationBehavior = propagationBehavior; } /** @@ -138,9 +163,8 @@ public final void setPropagationBehaviorName(String constantName) throws Illegal * @see #PROPAGATION_REQUIRED */ public final void setPropagationBehavior(int propagationBehavior) { - if (!constants.getValues(PREFIX_PROPAGATION).contains(propagationBehavior)) { - throw new IllegalArgumentException("Only values of propagation constants allowed"); - } + Assert.isTrue(propagationConstants.containsValue(propagationBehavior), + "Only values of propagation constants allowed"); this.propagationBehavior = propagationBehavior; } @@ -151,7 +175,7 @@ public final int getPropagationBehavior() { /** * Set the isolation level by the name of the corresponding constant in - * TransactionDefinition, e.g. "ISOLATION_DEFAULT". + * {@link TransactionDefinition} — for example, {@code "ISOLATION_DEFAULT"}. * @param constantName name of the constant * @throws IllegalArgumentException if the supplied value is not resolvable * to one of the {@code ISOLATION_} constants or is {@code null} @@ -159,10 +183,10 @@ public final int getPropagationBehavior() { * @see #ISOLATION_DEFAULT */ public final void setIsolationLevelName(String constantName) throws IllegalArgumentException { - if (!constantName.startsWith(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 = isolationConstants.get(constantName); + Assert.notNull(isolationLevel, "Only isolation constants allowed"); + this.isolationLevel = isolationLevel; } /** @@ -181,9 +205,8 @@ public final void setIsolationLevelName(String constantName) throws IllegalArgum * @see #ISOLATION_DEFAULT */ public final void setIsolationLevel(int isolationLevel) { - if (!constants.getValues(PREFIX_ISOLATION).contains(isolationLevel)) { - throw new IllegalArgumentException("Only values of isolation constants allowed"); - } + Assert.isTrue(isolationConstants.containsValue(isolationLevel), + "Only values of isolation constants allowed"); this.isolationLevel = isolationLevel; } @@ -294,9 +317,9 @@ public String toString() { */ protected final StringBuilder getDefinitionDescription() { StringBuilder result = new StringBuilder(); - result.append(constants.toCode(this.propagationBehavior, PREFIX_PROPAGATION)); + result.append(getPropagationBehaviorName(this.propagationBehavior)); result.append(','); - result.append(constants.toCode(this.isolationLevel, PREFIX_ISOLATION)); + result.append(getIsolationLevelName(this.isolationLevel)); if (this.timeout != TIMEOUT_DEFAULT) { result.append(','); result.append(PREFIX_TIMEOUT).append(this.timeout); @@ -308,4 +331,22 @@ protected final StringBuilder getDefinitionDescription() { return result; } + private static String getPropagationBehaviorName(int propagationBehavior) { + for (Map.Entry entry : propagationConstants.entrySet()) { + if (entry.getValue().equals(propagationBehavior)) { + return entry.getKey(); + } + } + throw new IllegalArgumentException("Unsupported propagation behavior: " + propagationBehavior); + } + + static String getIsolationLevelName(int isolationLevel) { + for (Map.Entry entry : isolationConstants.entrySet()) { + if (entry.getValue().equals(isolationLevel)) { + return entry.getKey(); + } + } + throw new IllegalArgumentException("Unsupported isolation level: " + isolationLevel); + } + } diff --git a/spring-tx/src/test/java/org/springframework/transaction/TestTransactionManager.java b/spring-tx/src/test/java/org/springframework/transaction/support/TestTransactionManager.java similarity index 90% rename from spring-tx/src/test/java/org/springframework/transaction/TestTransactionManager.java rename to spring-tx/src/test/java/org/springframework/transaction/support/TestTransactionManager.java index 3087387681d7..2347dd1f37af 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/TestTransactionManager.java +++ b/spring-tx/src/test/java/org/springframework/transaction/support/TestTransactionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 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. @@ -14,10 +14,10 @@ * limitations under the License. */ -package org.springframework.transaction; +package org.springframework.transaction.support; -import org.springframework.transaction.support.AbstractPlatformTransactionManager; -import org.springframework.transaction.support.DefaultTransactionStatus; +import org.springframework.transaction.CannotCreateTransactionException; +import org.springframework.transaction.TransactionDefinition; /** * @author Juergen Hoeller diff --git a/spring-tx/src/test/java/org/springframework/transaction/TransactionSupportTests.java b/spring-tx/src/test/java/org/springframework/transaction/support/TransactionSupportTests.java similarity index 64% rename from spring-tx/src/test/java/org/springframework/transaction/TransactionSupportTests.java rename to spring-tx/src/test/java/org/springframework/transaction/support/TransactionSupportTests.java index f6f559a1b305..387ca4c5ef34 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/TransactionSupportTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/support/TransactionSupportTests.java @@ -14,22 +14,37 @@ * limitations under the License. */ -package org.springframework.transaction; +package org.springframework.transaction.support; + +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.AfterEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.springframework.transaction.support.DefaultTransactionDefinition; -import org.springframework.transaction.support.DefaultTransactionStatus; -import org.springframework.transaction.support.TransactionCallbackWithoutResult; -import org.springframework.transaction.support.TransactionSynchronizationManager; -import org.springframework.transaction.support.TransactionTemplate; +import org.springframework.transaction.IllegalTransactionStateException; +import org.springframework.transaction.MockCallbackPreferringTransactionManager; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.TransactionSystemException; +import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatRuntimeException; +import static org.springframework.transaction.TransactionDefinition.ISOLATION_REPEATABLE_READ; +import static org.springframework.transaction.TransactionDefinition.ISOLATION_SERIALIZABLE; +import static org.springframework.transaction.TransactionDefinition.PROPAGATION_MANDATORY; +import static org.springframework.transaction.TransactionDefinition.PROPAGATION_REQUIRED; +import static org.springframework.transaction.TransactionDefinition.PROPAGATION_SUPPORTS; +import static org.springframework.transaction.support.DefaultTransactionDefinition.PREFIX_ISOLATION; +import static org.springframework.transaction.support.DefaultTransactionDefinition.PREFIX_PROPAGATION; /** * @author Juergen Hoeller @@ -48,33 +63,33 @@ void postConditions() { void noExistingTransaction() { PlatformTransactionManager tm = new TestTransactionManager(false, true); DefaultTransactionStatus status1 = (DefaultTransactionStatus) - tm.getTransaction(new DefaultTransactionDefinition(TransactionDefinition.PROPAGATION_SUPPORTS)); + tm.getTransaction(new DefaultTransactionDefinition(PROPAGATION_SUPPORTS)); assertThat(status1.hasTransaction()).as("Must not have transaction").isFalse(); DefaultTransactionStatus status2 = (DefaultTransactionStatus) - tm.getTransaction(new DefaultTransactionDefinition(TransactionDefinition.PROPAGATION_REQUIRED)); + tm.getTransaction(new DefaultTransactionDefinition(PROPAGATION_REQUIRED)); assertThat(status2.hasTransaction()).as("Must have transaction").isTrue(); assertThat(status2.isNewTransaction()).as("Must be new transaction").isTrue(); assertThatExceptionOfType(IllegalTransactionStateException.class).isThrownBy(() -> - tm.getTransaction(new DefaultTransactionDefinition(TransactionDefinition.PROPAGATION_MANDATORY))); + tm.getTransaction(new DefaultTransactionDefinition(PROPAGATION_MANDATORY))); } @Test void existingTransaction() { PlatformTransactionManager tm = new TestTransactionManager(true, true); DefaultTransactionStatus status1 = (DefaultTransactionStatus) - tm.getTransaction(new DefaultTransactionDefinition(TransactionDefinition.PROPAGATION_SUPPORTS)); + tm.getTransaction(new DefaultTransactionDefinition(PROPAGATION_SUPPORTS)); assertThat(status1.getTransaction()).as("Must have transaction").isNotNull(); assertThat(status1.isNewTransaction()).as("Must not be new transaction").isFalse(); DefaultTransactionStatus status2 = (DefaultTransactionStatus) - tm.getTransaction(new DefaultTransactionDefinition(TransactionDefinition.PROPAGATION_REQUIRED)); + tm.getTransaction(new DefaultTransactionDefinition(PROPAGATION_REQUIRED)); assertThat(status2.getTransaction()).as("Must have transaction").isNotNull(); assertThat(status2.isNewTransaction()).as("Must not be new transaction").isFalse(); DefaultTransactionStatus status3 = (DefaultTransactionStatus) - tm.getTransaction(new DefaultTransactionDefinition(TransactionDefinition.PROPAGATION_MANDATORY)); + tm.getTransaction(new DefaultTransactionDefinition(PROPAGATION_MANDATORY)); assertThat(status3.getTransaction()).as("Must have transaction").isNotNull(); assertThat(status3.isNewTransaction()).as("Must not be new transaction").isFalse(); } @@ -263,35 +278,101 @@ void setTransactionManager() { } @Test - void setPropagationBehaviorName() { - assertThatIllegalArgumentException().isThrownBy(() -> template.setPropagationBehaviorName("TIMEOUT_DEFAULT")); + void setPropagationBehaviorNameToUnsupportedValues() { + assertThatIllegalArgumentException().isThrownBy(() -> template.setPropagationBehaviorName(null)); + assertThatIllegalArgumentException().isThrownBy(() -> template.setPropagationBehaviorName(" ")); + assertThatIllegalArgumentException().isThrownBy(() -> template.setPropagationBehaviorName("bogus")); + assertThatIllegalArgumentException().isThrownBy(() -> template.setPropagationBehaviorName("ISOLATION_SERIALIZABLE")); + } - template.setPropagationBehaviorName("PROPAGATION_SUPPORTS"); - assertThat(template.getPropagationBehavior()).isEqualTo(TransactionDefinition.PROPAGATION_SUPPORTS); + /** + * Verify that the internal 'propagationConstants' map is properly configured + * for all PROPAGATION_ constants defined in {@link TransactionDefinition}. + */ + @Test + void setPropagationBehaviorNameToAllSupportedValues() { + Set uniqueValues = new HashSet<>(); + streamPropagationConstants() + .forEach(name -> { + template.setPropagationBehaviorName(name); + int propagationBehavior = template.getPropagationBehavior(); + int expected = DefaultTransactionDefinition.propagationConstants.get(name); + assertThat(propagationBehavior).isEqualTo(expected); + uniqueValues.add(propagationBehavior); + }); + assertThat(uniqueValues).hasSize(DefaultTransactionDefinition.propagationConstants.size()); } @Test void setPropagationBehavior() { assertThatIllegalArgumentException().isThrownBy(() -> template.setPropagationBehavior(999)); + assertThatIllegalArgumentException().isThrownBy(() -> template.setPropagationBehavior(ISOLATION_SERIALIZABLE)); - template.setPropagationBehavior(TransactionDefinition.PROPAGATION_MANDATORY); - assertThat(template.getPropagationBehavior()).isEqualTo(TransactionDefinition.PROPAGATION_MANDATORY); + template.setPropagationBehavior(PROPAGATION_MANDATORY); + assertThat(template.getPropagationBehavior()).isEqualTo(PROPAGATION_MANDATORY); } @Test - void setIsolationLevelName() { - assertThatIllegalArgumentException().isThrownBy(() -> template.setIsolationLevelName("TIMEOUT_DEFAULT")); + void setIsolationLevelNameToUnsupportedValues() { + assertThatIllegalArgumentException().isThrownBy(() -> template.setIsolationLevelName(null)); + assertThatIllegalArgumentException().isThrownBy(() -> template.setIsolationLevelName(" ")); + assertThatIllegalArgumentException().isThrownBy(() -> template.setIsolationLevelName("bogus")); + assertThatIllegalArgumentException().isThrownBy(() -> template.setIsolationLevelName("PROPAGATION_MANDATORY")); + } - template.setIsolationLevelName("ISOLATION_SERIALIZABLE"); - assertThat(template.getIsolationLevel()).isEqualTo(TransactionDefinition.ISOLATION_SERIALIZABLE); + /** + * Verify that the internal 'isolationConstants' map is properly configured + * for all ISOLATION_ constants defined in {@link TransactionDefinition}. + */ + @Test + void setIsolationLevelNameToAllSupportedValues() { + Set uniqueValues = new HashSet<>(); + streamIsolationConstants() + .forEach(name -> { + template.setIsolationLevelName(name); + int isolationLevel = template.getIsolationLevel(); + int expected = DefaultTransactionDefinition.isolationConstants.get(name); + assertThat(isolationLevel).isEqualTo(expected); + uniqueValues.add(isolationLevel); + }); + assertThat(uniqueValues).hasSize(DefaultTransactionDefinition.isolationConstants.size()); } @Test void setIsolationLevel() { assertThatIllegalArgumentException().isThrownBy(() -> template.setIsolationLevel(999)); - template.setIsolationLevel(TransactionDefinition.ISOLATION_REPEATABLE_READ); - assertThat(template.getIsolationLevel()).isEqualTo(TransactionDefinition.ISOLATION_REPEATABLE_READ); + template.setIsolationLevel(ISOLATION_REPEATABLE_READ); + assertThat(template.getIsolationLevel()).isEqualTo(ISOLATION_REPEATABLE_READ); + } + + @Test + void getDefinitionDescription() { + assertThat(template.getDefinitionDescription()).asString() + .isEqualTo("PROPAGATION_REQUIRED,ISOLATION_DEFAULT"); + + template.setPropagationBehavior(PROPAGATION_MANDATORY); + template.setIsolationLevel(ISOLATION_REPEATABLE_READ); + template.setReadOnly(true); + template.setTimeout(42); + assertThat(template.getDefinitionDescription()).asString() + .isEqualTo("PROPAGATION_MANDATORY,ISOLATION_REPEATABLE_READ,timeout_42,readOnly"); + } + + private static Stream streamPropagationConstants() { + return streamTransactionDefinitionConstants() + .filter(name -> name.startsWith(PREFIX_PROPAGATION)); + } + + private static Stream streamIsolationConstants() { + return streamTransactionDefinitionConstants() + .filter(name -> name.startsWith(PREFIX_ISOLATION)); + } + + private static Stream streamTransactionDefinitionConstants() { + return Arrays.stream(TransactionDefinition.class.getFields()) + .filter(ReflectionUtils::isPublicStaticFinal) + .map(Field::getName); } }