You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by ff...@apache.org on 2008/08/20 16:43:11 UTC

svn commit: r687359 - /cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java

Author: ffang
Date: Wed Aug 20 07:43:11 2008
New Revision: 687359

URL: http://svn.apache.org/viewvc?rev=687359&view=rev
Log:
[CXF-1755]JMSConduit need copy protocol headers to response in Message

Modified:
    cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java

Modified: cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
URL: http://svn.apache.org/viewvc/cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java?rev=687359&r1=687358&r2=687359&view=diff
==============================================================================
--- cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java (original)
+++ cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java Wed Aug 20 07:43:11 2008
@@ -374,6 +374,7 @@
             inMessage.put(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS, 
                           outMessage.get(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS));
 
+            inMessage.put(Message.PROTOCOL_HEADERS, outMessage.get(Message.PROTOCOL_HEADERS));
             getLogger().log(Level.FINE, "The Response Message is : [" + response + "]");
             
             // setup the inMessage response stream



Re: svn commit: r687359 - /cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java

Posted by Freeman Fang <fr...@gmail.com>.
I just commit fix, copy headers to inMessage from response jms message 
directly without changing the outMessage.

Cheers
Freeman

Daniel Kulp wrote:
> Ah.  Yes.   That looks like an issue.   receive(...) probably should take the 
> inMessage and populate it instead of modifying the outMessage.   Very 
> interesting.   receive needs the inMessage to query the timeouts and stuff 
> from it, but it shouldn't modify it.   
>
> The result of the current code is that the headers are a "merged" set of 
> headers that went out and headers that came in.   Probably not what it should 
> be.
>
> Dan
>
>
> On Wednesday 20 August 2008 11:11:45 am Freeman Fang wrote:
>   
>> Hi Dan,
>>
>> My comment inline.
>>
>> Daniel Kulp wrote:
>>     
>>> Freeman,
>>>
>>> Why would you copy the headers from the outgoing message to the incoming
>>> message?    That seems very bizzare to me.   What if you DON'T send with
>>> MTOM, but the server responds with MTOM.     This doesn't seem like the
>>> right fix.
>>>
>>> To me, you need to get the headers copied into the JMS Message when
>>> sending, and the copied out of the JMS message when receiving.
>>>       
>> Agree, actually the code do what you want.
>> Look at the populateIncomingContext method of JMSTransportBase class,
>> this method copy headers out of the jms message to cxf message when
>> receiving the response, but the cxf message used in this method is the
>> outgoing message(
>> Please see  response = receive(pooledSession, outMessage); in
>> JBIConduit.handleResponse(), this invoke populateIncomingContext()
>> I'm not sure why here use the outgoing message, should be incoming
>> message instead?), so I just copy the property from the outgoing message
>> to incoming message.
>>
>> Freeman
>>
>>     
>>> Dan
>>>
>>> On Wednesday 20 August 2008 10:43:11 am ffang@apache.org wrote:
>>>       
>>>> Author: ffang
>>>> Date: Wed Aug 20 07:43:11 2008
>>>> New Revision: 687359
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=687359&view=rev
>>>> Log:
>>>> [CXF-1755]JMSConduit need copy protocol headers to response in Message
>>>>
>>>> Modified:
>>>>
>>>> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/
>>>> tra nsport/jms/JMSConduit.java
>>>>
>>>> Modified:
>>>> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/
>>>> tra nsport/jms/JMSConduit.java URL:
>>>> http://svn.apache.org/viewvc/cxf/branches/2.0.x-fixes/rt/transports/jms/
>>>> src
>>>> /main/java/org/apache/cxf/transport/jms/JMSConduit.java?rev=687359&r1=68
>>>> 7358 &r2=687359&view=diff
>>>> ========================================================================
>>>> === === ---
>>>> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/
>>>> tra nsport/jms/JMSConduit.java (original) +++
>>>> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/
>>>> tra nsport/jms/JMSConduit.java Wed Aug 20 07:43:11 2008 @@ -374,6 +374,7
>>>> @@ inMessage.put(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS,
>>>>
>>>> outMessage.get(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS));
>>>>
>>>> +            inMessage.put(Message.PROTOCOL_HEADERS,
>>>> outMessage.get(Message.PROTOCOL_HEADERS)); getLogger().log(Level.FINE,
>>>> "The Response Message is : [" + response + "]");
>>>>
>>>>              // setup the inMessage response stream
>>>>         
>
>
>
>   


Re: svn commit: r687359 - /cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java

Posted by Daniel Kulp <dk...@apache.org>.
Ah.  Yes.   That looks like an issue.   receive(...) probably should take the 
inMessage and populate it instead of modifying the outMessage.   Very 
interesting.   receive needs the inMessage to query the timeouts and stuff 
from it, but it shouldn't modify it.   

The result of the current code is that the headers are a "merged" set of 
headers that went out and headers that came in.   Probably not what it should 
be.

Dan


On Wednesday 20 August 2008 11:11:45 am Freeman Fang wrote:
> Hi Dan,
>
> My comment inline.
>
> Daniel Kulp wrote:
> > Freeman,
> >
> > Why would you copy the headers from the outgoing message to the incoming
> > message?    That seems very bizzare to me.   What if you DON'T send with
> > MTOM, but the server responds with MTOM.     This doesn't seem like the
> > right fix.
> >
> > To me, you need to get the headers copied into the JMS Message when
> > sending, and the copied out of the JMS message when receiving.
>
> Agree, actually the code do what you want.
> Look at the populateIncomingContext method of JMSTransportBase class,
> this method copy headers out of the jms message to cxf message when
> receiving the response, but the cxf message used in this method is the
> outgoing message(
> Please see  response = receive(pooledSession, outMessage); in
> JBIConduit.handleResponse(), this invoke populateIncomingContext()
> I'm not sure why here use the outgoing message, should be incoming
> message instead?), so I just copy the property from the outgoing message
> to incoming message.
>
> Freeman
>
> > Dan
> >
> > On Wednesday 20 August 2008 10:43:11 am ffang@apache.org wrote:
> >> Author: ffang
> >> Date: Wed Aug 20 07:43:11 2008
> >> New Revision: 687359
> >>
> >> URL: http://svn.apache.org/viewvc?rev=687359&view=rev
> >> Log:
> >> [CXF-1755]JMSConduit need copy protocol headers to response in Message
> >>
> >> Modified:
> >>
> >> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/
> >>tra nsport/jms/JMSConduit.java
> >>
> >> Modified:
> >> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/
> >>tra nsport/jms/JMSConduit.java URL:
> >> http://svn.apache.org/viewvc/cxf/branches/2.0.x-fixes/rt/transports/jms/
> >>src
> >> /main/java/org/apache/cxf/transport/jms/JMSConduit.java?rev=687359&r1=68
> >>7358 &r2=687359&view=diff
> >> ========================================================================
> >>=== === ---
> >> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/
> >>tra nsport/jms/JMSConduit.java (original) +++
> >> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/
> >>tra nsport/jms/JMSConduit.java Wed Aug 20 07:43:11 2008 @@ -374,6 +374,7
> >> @@ inMessage.put(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS,
> >>
> >> outMessage.get(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS));
> >>
> >> +            inMessage.put(Message.PROTOCOL_HEADERS,
> >> outMessage.get(Message.PROTOCOL_HEADERS)); getLogger().log(Level.FINE,
> >> "The Response Message is : [" + response + "]");
> >>
> >>              // setup the inMessage response stream



-- 
Daniel Kulp
dkulp@apache.org
http://www.dankulp.com/blog

Re: svn commit: r687359 - /cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java

Posted by Freeman Fang <fr...@gmail.com>.
Hi Dan,

My comment inline.

Daniel Kulp wrote:
> Freeman,
>
> Why would you copy the headers from the outgoing message to the incoming 
> message?    That seems very bizzare to me.   What if you DON'T send with 
> MTOM, but the server responds with MTOM.     This doesn't seem like the right 
> fix.
>
> To me, you need to get the headers copied into the JMS Message when sending, 
> and the copied out of the JMS message when receiving.
>   
Agree, actually the code do what you want.
Look at the populateIncomingContext method of JMSTransportBase class, 
this method copy headers out of the jms message to cxf message when 
receiving the response, but the cxf message used in this method is the 
outgoing message(
Please see  response = receive(pooledSession, outMessage); in 
JBIConduit.handleResponse(), this invoke populateIncomingContext()
I'm not sure why here use the outgoing message, should be incoming 
message instead?), so I just copy the property from the outgoing message 
to incoming message.

Freeman
> Dan
>
>
> On Wednesday 20 August 2008 10:43:11 am ffang@apache.org wrote:
>   
>> Author: ffang
>> Date: Wed Aug 20 07:43:11 2008
>> New Revision: 687359
>>
>> URL: http://svn.apache.org/viewvc?rev=687359&view=rev
>> Log:
>> [CXF-1755]JMSConduit need copy protocol headers to response in Message
>>
>> Modified:
>>    
>> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/tra
>> nsport/jms/JMSConduit.java
>>
>> Modified:
>> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/tra
>> nsport/jms/JMSConduit.java URL:
>> http://svn.apache.org/viewvc/cxf/branches/2.0.x-fixes/rt/transports/jms/src
>> /main/java/org/apache/cxf/transport/jms/JMSConduit.java?rev=687359&r1=687358
>> &r2=687359&view=diff
>> ===========================================================================
>> === ---
>> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/tra
>> nsport/jms/JMSConduit.java (original) +++
>> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/tra
>> nsport/jms/JMSConduit.java Wed Aug 20 07:43:11 2008 @@ -374,6 +374,7 @@
>>              inMessage.put(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS,
>>                           
>> outMessage.get(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS));
>>
>> +            inMessage.put(Message.PROTOCOL_HEADERS,
>> outMessage.get(Message.PROTOCOL_HEADERS)); getLogger().log(Level.FINE, "The
>> Response Message is : [" + response + "]");
>>
>>              // setup the inMessage response stream
>>     
>
>
>
>   


Re: svn commit: r687359 - /cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java

Posted by Daniel Kulp <dk...@apache.org>.
Note: copying the headers into the JMS Message can also solve CXF-1749 as the 
charset would be available.

Dan


On Wednesday 20 August 2008 10:59:33 am Daniel Kulp wrote:
> Freeman,
>
> Why would you copy the headers from the outgoing message to the incoming
> message?    That seems very bizzare to me.   What if you DON'T send with
> MTOM, but the server responds with MTOM.     This doesn't seem like the
> right fix.
>
> To me, you need to get the headers copied into the JMS Message when
> sending, and the copied out of the JMS message when receiving.
>
> Dan
>
> On Wednesday 20 August 2008 10:43:11 am ffang@apache.org wrote:
> > Author: ffang
> > Date: Wed Aug 20 07:43:11 2008
> > New Revision: 687359
> >
> > URL: http://svn.apache.org/viewvc?rev=687359&view=rev
> > Log:
> > [CXF-1755]JMSConduit need copy protocol headers to response in Message
> >
> > Modified:
> >
> > cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/t
> >ra nsport/jms/JMSConduit.java
> >
> > Modified:
> > cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/t
> >ra nsport/jms/JMSConduit.java URL:
> > http://svn.apache.org/viewvc/cxf/branches/2.0.x-fixes/rt/transports/jms/s
> >rc
> > /main/java/org/apache/cxf/transport/jms/JMSConduit.java?rev=687359&r1=687
> >358 &r2=687359&view=diff
> > =========================================================================
> >== === ---
> > cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/t
> >ra nsport/jms/JMSConduit.java (original) +++
> > cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/t
> >ra nsport/jms/JMSConduit.java Wed Aug 20 07:43:11 2008 @@ -374,6 +374,7 @@
> > inMessage.put(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS,
> >
> > outMessage.get(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS));
> >
> > +            inMessage.put(Message.PROTOCOL_HEADERS,
> > outMessage.get(Message.PROTOCOL_HEADERS)); getLogger().log(Level.FINE,
> > "The Response Message is : [" + response + "]");
> >
> >              // setup the inMessage response stream



-- 
Daniel Kulp
dkulp@apache.org
http://www.dankulp.com/blog

Re: svn commit: r687359 - /cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java

Posted by Daniel Kulp <dk...@apache.org>.
Freeman,

Why would you copy the headers from the outgoing message to the incoming 
message?    That seems very bizzare to me.   What if you DON'T send with 
MTOM, but the server responds with MTOM.     This doesn't seem like the right 
fix.

To me, you need to get the headers copied into the JMS Message when sending, 
and the copied out of the JMS message when receiving.

Dan


On Wednesday 20 August 2008 10:43:11 am ffang@apache.org wrote:
> Author: ffang
> Date: Wed Aug 20 07:43:11 2008
> New Revision: 687359
>
> URL: http://svn.apache.org/viewvc?rev=687359&view=rev
> Log:
> [CXF-1755]JMSConduit need copy protocol headers to response in Message
>
> Modified:
>    
> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/tra
>nsport/jms/JMSConduit.java
>
> Modified:
> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/tra
>nsport/jms/JMSConduit.java URL:
> http://svn.apache.org/viewvc/cxf/branches/2.0.x-fixes/rt/transports/jms/src
>/main/java/org/apache/cxf/transport/jms/JMSConduit.java?rev=687359&r1=687358
>&r2=687359&view=diff
> ===========================================================================
>=== ---
> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/tra
>nsport/jms/JMSConduit.java (original) +++
> cxf/branches/2.0.x-fixes/rt/transports/jms/src/main/java/org/apache/cxf/tra
>nsport/jms/JMSConduit.java Wed Aug 20 07:43:11 2008 @@ -374,6 +374,7 @@
>              inMessage.put(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS,
>                           
> outMessage.get(JMSConstants.JMS_CLIENT_RESPONSE_HEADERS));
>
> +            inMessage.put(Message.PROTOCOL_HEADERS,
> outMessage.get(Message.PROTOCOL_HEADERS)); getLogger().log(Level.FINE, "The
> Response Message is : [" + response + "]");
>
>              // setup the inMessage response stream



-- 
Daniel Kulp
dkulp@apache.org
http://www.dankulp.com/blog