diff --git a/java-checks-test-sources/default/pom.xml b/java-checks-test-sources/default/pom.xml index 730e6460a35..fafa2d486f5 100644 --- a/java-checks-test-sources/default/pom.xml +++ b/java-checks-test-sources/default/pom.xml @@ -371,7 +371,7 @@ org.springframework spring-tx - 5.0.6.RELEASE + 5.3.21 provided diff --git a/java-checks-test-sources/default/src/main/java/checks/spring/SpringIncompatibleTransactionalCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/spring/SpringIncompatibleTransactionalCheckSample.java index 2a707080999..895d097f7bb 100644 --- a/java-checks-test-sources/default/src/main/java/checks/spring/SpringIncompatibleTransactionalCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/spring/SpringIncompatibleTransactionalCheckSample.java @@ -3,6 +3,9 @@ import javax.transaction.Transactional.TxType; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionTemplate; + +import java.util.UUID; import static javax.transaction.Transactional.TxType.REQUIRED; @@ -342,3 +345,35 @@ public void methodB() { } } + +class SpringIncompatibleTransactionalCheckSampleSupportTransactionTemplate { + + private TransactionTemplate transactionTemplate; + + @Transactional + public void retry(UUID id) { + } + + @Transactional(propagation = Propagation.REQUIRES_NEW) + public void retryNew(UUID id) { + } + + public void retryBatch(UUID id) { + // Possible FNs: calls inside a TransactionTemplate callback are suppressed because a transaction is guaranteed + // to be active at the call site. However, if the template's propagation is customized (e.g. NEVER or NOT_SUPPORTED), + // self-invocations that require a transaction would still be problematic. Resolving the template's propagation + // statically requires data-flow / symbolic-execution techniques and is out of scope for this rule. + transactionTemplate.executeWithoutResult(status -> { + retry(id); // compliant - inside a TransactionTemplate callback: a transaction is already active + retryNew(id); // compliant - self-invocation reports are suppressed inside TransactionTemplate callbacks + }); + transactionTemplate.execute(status -> { + retry(id); + retryNew(id); + return id.toString(); + }); + + retry(id); // Noncompliant {{"retry's" @Transactional requirement is incompatible with the one for this method.}} [[secondary=354]] + retryNew(id); // Noncompliant {{"retryNew's" @Transactional requirement is incompatible with the one for this method.}} [[secondary=358]] + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringIncompatibleTransactionalCheck.java b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringIncompatibleTransactionalCheck.java index 7d3427c465f..25cf0461ae6 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/spring/SpringIncompatibleTransactionalCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/spring/SpringIncompatibleTransactionalCheck.java @@ -31,6 +31,7 @@ import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.Symbol; import org.sonar.plugins.java.api.semantic.SymbolMetadata; import org.sonar.plugins.java.api.semantic.SymbolMetadata.AnnotationValue; @@ -59,6 +60,12 @@ public class SpringIncompatibleTransactionalCheck extends IssuableSubscriptionVi // Made name to represent no annotation private static final String NOT_TRANSACTIONAL = "SONAR_NOT_TRANSACTIONAL"; + private static final MethodMatchers TRANSACTION_TEMPLATE_EXECUTE = MethodMatchers.create() + .ofTypes("org.springframework.transaction.support.TransactionTemplate") + .names("execute", "executeWithoutResult") + .withAnyParameters() + .build(); + private static final Map> INCOMPATIBLE_PROPAGATION_MAP = buildIncompatiblePropagationMap(); private static Map> buildIncompatiblePropagationMap() { @@ -96,14 +103,21 @@ private void checkMethodInvocations(MethodTree method, @Nullable String callerPr return; } methodBody.accept(new BaseTreeVisitor() { + private boolean insideTransactionTemplateCallback = false; + @Override public void visitMethodInvocation(MethodInvocationTree methodInvocation) { + boolean previousState = insideTransactionTemplateCallback; + if (TRANSACTION_TEMPLATE_EXECUTE.matches(methodInvocation)) { + insideTransactionTemplateCallback = true; + } super.visitMethodInvocation(methodInvocation); + insideTransactionTemplateCallback = previousState; Symbol calleeMethodSymbol = methodInvocation.methodSymbol(); if (calleeMethodSymbol.isUnknown()) { return; } - if (methodsPropagationMap.containsKey(calleeMethodSymbol) && methodInvocationOnThisInstance(methodInvocation)) { + if (!insideTransactionTemplateCallback && methodsPropagationMap.containsKey(calleeMethodSymbol) && methodInvocationOnThisInstance(methodInvocation)) { String calleePropagation = methodsPropagationMap.get(calleeMethodSymbol); checkIncompatiblePropagation(methodInvocation, callerPropagation, calleeMethodSymbol, calleePropagation); }