camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Babak Vahdat <babak.vah...@swissonline.ch>
Subject Re: [2/3] git commit: CAMEL-7856 camel-cxf producer HttpAPI should not send the message body when using DELETE method
Date Sat, 27 Sep 2014 21:51:21 GMT
Hi Willem,

I guess the naming of the setter method is wrong (doesn’t adhere the Java
bean style) which’s why we do currently fail to invoke it through our
IntrospectionSupport utility when it’s given through an URI option:
https://github.com/apache/camel/commit/53792cc1cf4c45f3fae6d8cc84a3b15fb12cc05a#diff-154901a934ad9ebad992c3fa1e8bbcbaR409

This is exactly why this setter doesn’t get invoked at all although you set
it as an URI option below
https://github.com/apache/camel/commit/53792cc1cf4c45f3fae6d8cc84a3b15fb12cc05a#diff-cca41fa34076a43d0940e4366508432dR67

Also the ticket is marked as fixed but I didn't find this option being
enlisted
http://camel.apache.org/cxfrs.html

Babak

Willem.Jiang wrote
> I can add an option to cxfrs endpoint to avoid sending the payload in
> DELETE method. In this way, we could keep old behaviour of CxfProducer
> unchanged by default.
> 
> Any thoughts? 
> 
> 
> --  
> Willem Jiang
> 
> Red Hat, Inc.
> Web: http://www.redhat.com
> Blog: http://willemjiang.blogspot.com (English)
> http://jnn.iteye.com (Chinese)
> Twitter: willemjiang  
> Weibo: 姜宁willem
> 
> 
> 
> On September 24, 2014 at 9:47:58 PM, Sergey Beryozkin (

> sberyozkin@

> ) wrote:
>> Hi Dan, Willem
>> On 24/09/14 14:37, Daniel Kulp wrote:
>> >
>> > That’s likely a different issue. Nt sure the parameter should be added
>> as part of the  
>> body in this case as part of the consumer. I’d defer that to Sergey, but
>> certainly the CxfProducer  
>> needs to be able to send a DELETE with a payload.
>> >
>> It is indeed the case of the client proxy unintentionally picking up a
>> payload and using it in DELETE, so indeed it is about the optional
>> payload exclusion as opposed to completely blocking the client to avoid
>> sending payloads with DELETE
>>  
>> Willem, IMHO this should be optional
>>  
>> Thanks, Sergey
>>  
>> > Dan
>> >
>> >
>> >
>> > On Sep 24, 2014, at 9:24 AM, Willem Jiang wrote:
>> >
>> >> Actually, this patch is try to avoid sending the message body with the
>> DELETE method  
>> if camel proxy the request just like this
>> >>
>> >>  
>> >>  
>> >>  
>> >>
>> >> As camel-cxfrs consumer put the invocation parameters into the camel
>> message body,  
>> the proxy just add the message body 124 like this
>> >>
>> >> 2014-09-24 21:00:19,378 [qtp841166421-29] INFO LoggingInInterceptor -
>> Inbound  
>> Message
>> >> ----------------------------
>> >> ID: 11
>> >> Address:
>> http://localhost:1101/CxfRsRouterTest/route/customerservice/customers/124/  
>> >> Http-Method: DELETE
>> >> Content-Type:
>> >> Headers: {accept-encoding=[gzip,deflate], connection=[keep-alive],
>> Content-Type=[null],  
>> Host=[localhost:1101], User-Agent=[Apache-HttpClient/4.3.3 (java 1.5)]}
>> >> --------------------------------------
>> >> 2014-09-24 21:00:19,380 [qtp841166421-29] INFO body -
>> Exchange[ExchangePattern:  
>> InOut, BodyType: org.apache.cxf.message.MessageContentsList, Body: 124]
>> >> 2014-09-24 21:00:19,385 [qtp841166421-29] INFO LoggingOutInterceptor -
>> Outbound  
>> Message
>> >> ---------------------------
>> >> ID: 12
>> >> Address:
>> http://localhost:1102/CxfRsRouterTest/rest/customerservice/customers/124/  
>> >> Http-Method: DELETE
>> >> Content-Type: application/xml
>> >> Headers: {Host=[localhost:1101],
>> org.apache.cxf.request.method=[DELETE],  
>> accept-encoding=[gzip,deflate], connection=[keep-alive],
>> org.apache.cxf.message.Message.PATH_INFO=[/customerservice/customers/124/],  
>> breadcrumbId=[ID-localhost-51196-1411563616702-4-3], Accept=[*/*],
>> org.apache.cxf.request.uri=[/CxfRsRouterTest/route/customerservice/customers/124/],
 
>> User-Agent=[Apache-HttpClient/4.3.3 (java 1.5)],
>> Content-Type=[application/xml]}  
>> >> Payload: 124
>> >>
>> >> If we remove the message body in the application level (in the camel
>> route), the camel  
>> route could be more complex as it need to know the underlay http request
>> method.
>> >>
>> >> That is the reason that I create this JIRA to apply the best practise
>> here.
>> >>
>> >>
>> >>
>> >> --
>> >> Willem Jiang
>> >>
>> >> Red Hat, Inc.
>> >> Web: http://www.redhat.com
>> >> Blog: http://willemjiang.blogspot.com (English)
>> >> http://jnn.iteye.com (Chinese)
>> >> Twitter: willemjiang
>> >> Weibo: 姜宁willem
>> >>
>> >>
>> >>
>> >> On September 24, 2014 at 7:49:26 PM, Daniel Kulp (

> dkulp@

> ) wrote:
>> >>>
>> >>> Willem,
>> >>>
>> >>> My reading of the discussion is that a DELETE MAY contain a message
>> body, but the semantics  
>> >>> are somewhat undefined. A proxy SHOULD forward that body on. This
>> commit seems to  
>> make
>> >>> sure it doesn’t forward it along. Thus, I’m kind of against this
>> commit.
>> >>>
>> >>> We also had a discussion about this on the CXF list a while ago where
>> the body was needed:  
>> >>>
>> >>>
>> http://cxf.547215.n5.nabble.com/Sending-body-in-DELETE-HTTP-Requests-with-JAXRSClientFactoryBean-tt5735129.html
 
>> >>>
>> >>>
>> >>>
>> >>> Dan
>> >>>
>> >>>
>> >>>
>> >>> On Sep 24, 2014, at 1:44 AM, 

> ningjiang@

>  wrote:
>> >>>
>> >>>> CAMEL-7856 camel-cxf producer HttpAPI should not send the message
>> body when using  
>> >>> DELETE method
>> >>>>
>> >>>>
>> >>>> Project: http://git-wip-us.apache.org/repos/asf/camel/repo
>> >>>> Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/5b4abb61
 
>> >>>> Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/5b4abb61
>> >>>> Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/5b4abb61
>> >>>>
>> >>>> Branch: refs/heads/camel-2.13.x
>> >>>> Commit: 5b4abb619b808d317da99037b734e78fc4398d29
>> >>>> Parents: 2b1296e
>> >>>> Author: Willem Jiang
>> >>>> Authored: Wed Sep 24 13:41:41 2014 +0800
>> >>>> Committer: Willem Jiang
>> >>>> Committed: Wed Sep 24 13:43:43 2014 +0800
>> >>>>
>> >>>>
>> ----------------------------------------------------------------------  
>> >>>> .../java/org/apache/camel/component/cxf/jaxrs/CxfRsProducer.java
| 4
>> ++--  
>> >>>> .../org/apache/camel/component/cxf/jaxrs/CxfRsSpringRouter.xml |
1 +
>> >>>> 2 files changed, 3 insertions(+), 2 deletions(-)
>> >>>>
>> ----------------------------------------------------------------------  
>> >>>>
>> >>>>
>> >>>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/5b4abb61/components/camel-cxf/src/main/java/org/apache/camel/component/cxf/jaxrs/CxfRsProducer.java
 
>> >>>>
>> ----------------------------------------------------------------------  
>> >>>> diff --git
>> a/components/camel-cxf/src/main/java/org/apache/camel/component/cxf/jaxrs/CxfRsProducer.java
 
>> >>>
>> b/components/camel-cxf/src/main/java/org/apache/camel/component/cxf/jaxrs/CxfRsProducer.java
 
>> >>>> index 572e991..c580445 100644
>> >>>> ---
>> a/components/camel-cxf/src/main/java/org/apache/camel/component/cxf/jaxrs/CxfRsProducer.java
 
>> >>>> +++
>> b/components/camel-cxf/src/main/java/org/apache/camel/component/cxf/jaxrs/CxfRsProducer.java
 
>> >>>> @@ -158,8 +158,8 @@ public class CxfRsProducer extends
>> DefaultProducer {
>> >>>>
>> >>>> // set the body
>> >>>> Object body = null;
>> >>>> - if (!"GET".equals(httpMethod)) {
>> >>>> - // need to check the request object.
>> >>>> + if (!"GET".equals(httpMethod) && !"DELETE".equals(httpMethod))
{
>> >>>> + // need to check the request object if the http Method is not
GET
>> or DELETE
>> >>>> body = binding.bindCamelMessageBodyToRequestBody(inMessage,
>> exchange);  
>> >>>> if (LOG.isTraceEnabled()) {
>> >>>> LOG.trace("Request body = " + body);
>> >>>>
>> >>>>
>> http://git-wip-us.apache.org/repos/asf/camel/blob/5b4abb61/components/camel-cxf/src/test/resources/org/apache/camel/component/cxf/jaxrs/CxfRsSpringRouter.xml
 
>> >>>>
>> ----------------------------------------------------------------------  
>> >>>> diff --git
>> a/components/camel-cxf/src/test/resources/org/apache/camel/component/cxf/jaxrs/CxfRsSpringRouter.xml
 
>> >>>
>> b/components/camel-cxf/src/test/resources/org/apache/camel/component/cxf/jaxrs/CxfRsSpringRouter.xml
 
>> >>>> index 154b572..9f51dd6 100644
>> >>>> ---
>> a/components/camel-cxf/src/test/resources/org/apache/camel/component/cxf/jaxrs/CxfRsSpringRouter.xml
 
>> >>>> +++
>> b/components/camel-cxf/src/test/resources/org/apache/camel/component/cxf/jaxrs/CxfRsSpringRouter.xml
 
>> >>>> @@ -64,6 +64,7 @@
>> >>>>
>> >>>>
>> >>>>
>> >>>> +
>> >>>>
>> >>>>
>> >>>> True
>> >>>>
>> >>>
>> >>> --
>> >>> Daniel Kulp
>> >>> 

> dkulp@

>  - http://dankulp.com/blog
>> >>> Talend Community Coder - http://coders.talend.com
>> >
>>  
>>





--
View this message in context: http://camel.465427.n5.nabble.com/Re-2-3-git-commit-CAMEL-7856-camel-cxf-producer-HttpAPI-should-not-send-the-message-body-when-using-d-tp5756960p5757113.html
Sent from the Camel Development mailing list archive at Nabble.com.

Mime
View raw message