Feature/147 consider transaction configuration if transactional method is called from same bean#148
Conversation
…s transaction propagation
…s regarding spring-transaction:TransactionChangingMethodMustNotBeInvokedFromSameClassOrSubclass
| OPTIONAL MATCH | ||
| (annotation)-[:HAS]->(propagation:Value {name: supportedAnnotationType.propagationAttributeName})-[:IS]->(propagationType:Java:Field) | ||
| OPTIONAL MATCH | ||
| (annotation)-[:HAS]->(configuration:Value) |
There was a problem hiding this comment.
A configuration is the set of all configuration parameters. We are dealing with a single (configuration) parameter here, not the entire configuration.
| sourceTxSemantics AS SourceTransactionalSemantics, | ||
| targetTxMethod as TargetTransactionalMethod, | ||
| targetTxSemantics AS TargetTransactionalSemantics, | ||
| targetTxMethod.additionalTransactionConfiguration AS TargetMethodSetsAdditionalConfigurationAttribute, |
There was a problem hiding this comment.
| targetTxMethod.additionalTransactionConfiguration AS TargetMethodSetsAdditionalConfigurationAttribute, | |
| targetTxMethod.additionalTransactionConfiguration AS TargetMethodSetsAdditionalConfiguration, |
Should be as short as possible
There was a problem hiding this comment.
| <description>Provides transactional methods as ":Spring:Transactional:Method" and its configuration.</description> |
| public class GenericTransactionalClass<T> { | ||
|
|
||
| @Transactional | ||
| public void methodWithRequiredSemantics(T parameter) { |
There was a problem hiding this comment.
Please consider renaming this method to make it clear, that the readOnly flag is overridden.
| private void privateSubClassMethod() { | ||
| } | ||
|
|
||
| // The rollback configuration is ignored if the method is called within the same bean. |
There was a problem hiding this comment.
| // The rollback configuration is ignored if the method is called within the same bean. | |
| // The readOnly flag is ignored if the method is called within the same bean. |
| <requiresConcept refId="spring-transaction:TransactionalMethod"/> | ||
| <requiresConcept refId="java:GeneratedType"/> | ||
| <description>Transactional methods must not be invoked from the same class or from a subclass within the inheritance hierarchy if the transactional semantics of the invoked method requires a change to the current transaction handling.</description> | ||
| <description>Transactional methods must not be invoked from the same class or from a subclass within the inheritance hierarchy if the transactional semantics of the invoked method requires a change to the current transaction handling or sets an additional configuration attribute.</description> |
There was a problem hiding this comment.
| <description>Transactional methods must not be invoked from the same class or from a subclass within the inheritance hierarchy if the transactional semantics of the invoked method requires a change to the current transaction handling or sets an additional configuration attribute.</description> | |
| <description>Transactional methods must not be invoked from the same class or from a subclass within the inheritance hierarchy if the transactional semantics of the invoked method requires a change to the current transaction handling or the invoked method sets an additional configuration parameter.</description> |
| <includeConstraint refId="spring-transaction:PrivateMethodMustNotBeAnnotatedWithTransactional"/> | ||
| </group> | ||
|
|
||
| <constraint id="spring-transaction:TransactionChangingMethodMustNotBeInvokedFromSameClassOrSubclass"> |
There was a problem hiding this comment.
The actual issue is that the transaction configuration is supposed to be changed by this call which is not possible within the same been. Following this simple / agnostic approach, violations should be reported if the invoked method or the calling method have additional configuration parameters. Since we do not analyze these configurations semantically, we have to presume, that they are incompatible.
Consequently (apart from the transaction propagation types), within the same bean only such method calls are allowed that use the default configuration, so we can be sure, that the configuration is not changed.
However, this PR is still an improvement to the current version, since the newly reported findings should be correct (if not complete). The change described above can therefore be deferred to a later development step. (An additional issue should be created in that case).
There was a problem hiding this comment.
If this change is implemented, we should consider separating this constraint from the one that checks the transaction propagation. Apart from technical considerations, it will be easier to interpret more specific findings.
There was a problem hiding this comment.
I agree, checking the invoked method as well as the calling method regarding additional configuration parameters might find further potential compatibility issues. However, as long as we don't evaluate every configuration parameter semantically, we still have the problem of False Positives if the assumption of incompatible configuration parameters is wrong.
2 participants
No description provided.