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
[WFLY-2954] Use TransactionSynchronizationRegistry in JMSContext #5921
[WFLY-2954] Use TransactionSynchronizationRegistry in JMSContext #5921
Conversation
Build 2685 is now running using a merge of b9642d0 |
Build 2685 outcome was SUCCESS using a merge of b9642d0 |
|
||
public static TransactionSynchronizationRegistry getTransactionSynchronizationRegistry() { | ||
ServiceController<TransactionSynchronizationRegistry> service = (ServiceController<TransactionSynchronizationRegistry>) container.getService(TxnServices.JBOSS_TXN_SYNCHRONIZATION_REGISTRY); | ||
return service == null ? null : service.getValue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there proper service dependencies around this? Same question for the existing getTransactionManager() method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct, I used the existing getTransactionManager() method but it could be done more cleanly.
I've amended my code to simply lookup the TransactionSynchronizationRegistry from JNDI.
Build 2785 is now running using a merge of 2b4028e |
Build 2785 outcome was SUCCESS using a merge of 2b4028e |
try { | ||
final Transaction transaction = getCurrentTransaction(); | ||
ctx = new InitialContext(); | ||
TransactionSynchronizationRegistry registry = (TransactionSynchronizationRegistry) ctx.lookup(TRANSACTION_SYNCHRONIZATION_REGISTRY_LOOKUP); | ||
if (delegate == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L197 and 198 should be inside the "if (delegate == null) {" block or a lookup will be performed for each getDelegate() call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Register the sync object using the TransactionSynchronizationRegistry instead of registering it directly on the Transaction. Fix setup of InjectedJMSContextTestCase that can not use a @Inject-ed JMSContext JIRA: https://issues.jboss.org/browse/WFLY-2954
Build 2804 is now running using a merge of f0d1fab |
Build 2804 outcome was SUCCESS using a merge of f0d1fab |
Merged. |
Register the sync object using the TransactionSynchronizationRegistry
instead of registering it directly on the Transaction.
JIRA: https://issues.jboss.org/browse/WFLY-2954