Hi Christian, I have added a test case to my github branch which demonstrates the session issue and fails as a result. The session in the session holder is the same as the session passed into the onMessage method.
In the jms systests: org.apache.cxf.systest.jms.tx.JMSTransactionClientServerSameSessionTest Currently when it executes, a soap fault is returned immediately. What actually should be happening is a rollback of the message occurs, and the GreeterImplWithTransaction then flips the boolean flag and returns a valid result. I have debugged this test and it fails to perform the rollback because of this test: boolean trans = resourceHolder == null || !resourceHolder.containsSession(session); The resourceHolder does contain the session. Where as for all other test cases (and my own which use both websphere mq and activemq), the sessions do not match. I need to understand exactly why the resource holder having the session means trans = false.... Thanks On Fri, Dec 5, 2014 at 2:42 PM, Jason Pell <[email protected]> wrote: > Would be interested in your feedback regarding the change. > > > https://github.com/pellcorp/apache-cxf/compare/apache:2.7.x-fixes...cxf6136_jms_ex_rollback?expand=1 > > > > On Fri, Dec 5, 2014 at 1:53 PM, Jason Pell <[email protected]> wrote: > >> Never mind, think I figured it out. >> >> Another question I have, is should the JMSDestination sendExchange be >> aborted if we are going to rollback anyway? As it stands, the message gets >> sent to the MOM and then immediately backed out, which seems kind of silly. >> >> >> >> On Fri, Dec 5, 2014 at 12:42 PM, Jason Pell <[email protected]> wrote: >> >>> Ok - in my local tests with both websphere mq and active mq, the trans >>> test appears to be doing the right thing, but I am unsure why. I don't >>> understand why the sessions are not the same. >>> >>> However when I am debugging the >>> org.apache.cxf.systest.jms.tx.JMSTransactionClientServerTest in >>> cxf-systests-transport-jm the trans test is false because the sessions >>> match. >>> >>> I know I must be missing something, and I will continue to debug it to >>> see if I can figure it out, but perhaps someone might have some insight to >>> assist me >>> >>> On Fri, Dec 5, 2014 at 11:20 AM, Jason Pell <[email protected]> wrote: >>> >>>> Hi Christian, >>>> >>>> While writing a test case for the transaction case for request / >>>> response I came across something else in the code I am confused about. >>>> >>>> The JMSDestination has a check of the following: >>>> >>>> boolean trans = resourceHolder == null || >>>> !resourceHolder.containsSession(session); >>>> >>>> So if the resourceHolder is not found or the resource holder does not >>>> have the session, then there is no transaction. Is this correct? >>>> >>>> I would have thought the opposite would have been true: >>>> >>>> boolean trans = resourceHolder != null && >>>> resourceHolder.containsSession(session); >>>> >>>> Can you comment on this? >>>> >>>> >>>> On Fri, Dec 5, 2014 at 9:26 AM, Jason Pell <[email protected]> wrote: >>>> >>>>> Very annoyed with myself that I did not check the transaction support >>>>> for request response till now. Was on my list of todos. Now I am going to >>>>> have to depend on 2.7.15 snapahot. >>>>> >>>>> I need guaranteed delivery even for request response as my client is >>>>> not expecting to have to retry and the message needs to survive a server >>>>> crash or db issue. >>>>> >>>>> I am not going to use xa transactions though. Will be enough to use >>>>> good old jms transaction manager support. >>>>> On 05/12/2014 9:13 AM, "Jason Pell" <[email protected]> wrote: >>>>> >>>>>> I check that the exception cause is instanceof Exception and not >>>>>> propogate. Otherwise the old functionality applies. >>>>>> On 05/12/2014 8:41 AM, "Christian Schneider" <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> I think one thing to consider is handling of java.lang.Error. These >>>>>>> are not of type Exception and not of type RuntimeException. As they are >>>>>>> not >>>>>>> expected a Rollback may be apropriate but I am not sure. >>>>>>> >>>>>>> As far as I can remember the reason why I did not implement a more >>>>>>> sophisticated behaviour till now is that I never actually used >>>>>>> transactions >>>>>>> for Request/Reply. >>>>>>> In one way messaging transactions are essential as the caller has to >>>>>>> be sure that messages do not get lost. >>>>>>> In request/reply there is always someone on the other side listening >>>>>>> for the reply. If no reply comes he can and probably will retry. So >>>>>>> there >>>>>>> is much less pressure to use transactions at all. >>>>>>> >>>>>>> Christian >>>>>>> >>>>>>> >>>>>>> On 04.12.2014 18:09, Jason Pell wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> I would like to change the existing behavior of JMS destination to >>>>>>>> NOT >>>>>>>> rollback the transaction if a checked exception is encountered. >>>>>>>> >>>>>>>> https://issues.apache.org/jira/browse/CXF-6136 >>>>>>>> >>>>>>>> Christian suggested I post an email to this list to give everyone an >>>>>>>> opportunity to agree or disagree with my proposed changes. >>>>>>>> >>>>>>>> Currently if a transaction manager is registered (even just a JMS >>>>>>>> transaction manager) and an exception is thrown by an impl method >>>>>>>> the JMS >>>>>>>> message will be rolled back. >>>>>>>> >>>>>>>> There is currently no distinction made. Even if I throw a business >>>>>>>> soap >>>>>>>> fault its still going to roll back the message. >>>>>>>> >>>>>>>> I would like to add code to JMS Destination to no longer propagate >>>>>>>> checked >>>>>>>> exceptions which will result in the delivery of a soap fault >>>>>>>> response to >>>>>>>> the JMS reply queue. >>>>>>>> >>>>>>>> Christian has suggested we could make this change without a >>>>>>>> backwards >>>>>>>> compatible config entry in JMSConfiguration. I am happy to add a >>>>>>>> config >>>>>>>> entry to maintain legacy behavior.. >>>>>>>> >>>>>>>> Thoughts? >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Christian Schneider >>>>>>> http://www.liquid-reality.de >>>>>>> >>>>>>> Open Source Architect >>>>>>> http://www.talend.com >>>>>>> >>>>>>> >>>> >>> >> >
