Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AspectJ aspect for @javax.transaction.Transactional is not initialised by default [SPR-16987] #21525

Closed
spring-issuemaster opened this issue Jun 29, 2018 · 5 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Jun 29, 2018

Mike B opened SPR-16987 and commented

If I

a) use Spring transaction management in the AspectJ mode
( @EnableTransactionManagement(mode = AdviceMode.ASPECTJ) )

b) use the javax.transaction.Transactional annotation instead of org.springframework.transaction.annotation.Transactional

then

a) it doesn't work out of the box - transactions are not created around annotated methods.

b) I get "Skipping transactional joinpoint [] because no transaction manager has been configured" in the logs (if I enable spring debug logging).

This seems to be because the default configuration in AspectJTransactionManagementConfiguration only initialises AnnotationTransactionAspect but not JtaAnnotationTransactionAspect. So the JtaAnnotationTransactionAspect gets woven in by aspectJ compiler and executed but because it is not initialised with the transaction manager it cannot create transactions.

As a workaround, I can initialise it myself but declaring

@Bean
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
public JtaAnnotationTransactionAspect jtaTransactionAspect() {
return JtaAnnotationTransactionAspect.aspectOf();
}

But could you please add it to the default configuration so that it works out of the box.


Affects: 5.0.7

Issue Links:

  • #16423 Support standard javax.transaction.Transactional in AspectJ

Referenced from: commits 5b24040

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 29, 2018

Juergen Hoeller commented

Good point, we relied on an explicit bean definition for the JtaAnnotationTransactionAspect there... which isn't really obvious and certainly not convenient. I've rolled this into 5.1 now, for @EnableTransactionManagement as well as <tx:annotation-driven> in aspectj mode.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 29, 2018

Juergen Hoeller commented

This is in master now, becoming available in the upcoming 5.1.0.BUILD-SNAPSHOT (https://repo.spring.io/snapshot). Feel free to give it an early try...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 2, 2018

Mike B commented

Thank you for the (very) quick response.

Just one comment.

Actually, in our project we happen to use both "javax.transaction.Transactional" and "org.springframework.transaction.annotation.Transactional". (I know it's not good but it's a large team so hard to enforce consistency.)

Looking at your change, in the xml config, if "javax.transaction.Transactional" is present in the classpath, then both annotations are enabled. But in java config, only one is enabled.

Could you do the same in java config, ie if "javax.transaction.Transactional" is present in the classpath, then enable support for both annotations.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 2, 2018

Juergen Hoeller commented

This should work fine with Java config already since AspectJJtaTransactionManagementConfiguration extends AspectJTransactionManagementConfiguration, so the declaration of the Spring annotation aspect is inherited with a different bean name... next to the locally declared JTA transaction aspect in that config class.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 2, 2018

Mike B commented

you are right, sorry, missed the inheritance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.