SONARJAVA-4881 Update S2230 to only target Spring <= 5 #4969
Conversation
rules/S2230/java/rule.adoc
Outdated
| Nor does Spring make provision for the methods invoked by the method it called. | ||
|
|
||
| Therefore marking a private method, for instance, @Transactional can only result in a runtime error or exception if the method is annotated as @Transactional. | ||
| Therefore, marking a private method, for instance, @Transactional can only result in a runtime error or exception if the method is annotated as @Transactional. |
There was a problem hiding this comment.
I'm not sure if this is true actually. There just seems no way to invoke the private method through the proxy that is generated.
You can call this method just fine by calling it from a public method in the same class, but since it doesn't go through the generated proxy, it will not apply the effect of the @Transactional annotation.
void delegate() {
privateMethod();
}
@Transactional
private void privateMethod() {
jdbcClient.sql("INSERT INTO foo(id) VALUES (1)").update();
if (true) {
throw new RuntimeException("Always crashes");
}
jdbcClient.sql("INSERT INTO foo(id) VALUES (2)").update();
}
///
try {
testRepo.delegate();
} catch (Exception e) {
e.printStackTrace();
}
System.out.println("delegate " + testRepo.findAll());
System.out.println(testRepo.findAll());Will print delegate [1] even though the private transaction annotated method results in an exception and should have rolled back the insert.
There was a problem hiding this comment.
You are right. I've rephrased that part.
There was a problem hiding this comment.
Thank you, the new description aligns much better with the actual behavior
romainbrenguier
force-pushed
the
romain/spring-version-S2230
branch
from
59c0bfc to
0a44352
Compare
rules/S2230/java/rule.adoc
Outdated
| Therefore marking a private method, for instance, @Transactional can only result in a runtime error or exception if the method is annotated as @Transactional. | ||
| Therefore, marking a private method, for instance, @Transactional gives a false sense of security, and can lead to incorrect assumptions and potential bugs. | ||
|
|
||
| Note that the rule only targets Spring up to Spring 5, because Spring 6 takes into account all non-private methods. |
There was a problem hiding this comment.
Should we move this sentence into a new Exceptions section?
romainbrenguier
force-pushed
the
romain/spring-version-S2230
branch
from
0d5e60c to
ba13be4
Compare
romainbrenguier
force-pushed
the
romain/spring-version-S2230
branch
from
ba13be4 to
515ff23
Compare
Their is an associated PR on sonar-java to change the behaviour of the rule: SonarSource/sonar-java#5094
romainbrenguier
force-pushed
the
romain/spring-version-S2230
branch
from
515ff23 to
5513bcc
Compare
|
|
SONARJAVA-4881
Their is an associated PR on sonar-java to change the behaviour of the rule: #5094
Review
A dedicated reviewer checked the rule description successfully for: