You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Kalnichevski, Oleg" <ol...@bearingpoint.com> on 2003/01/31 19:58:35 UTC
[PATCH] PostMethod & PutMethod revision (take 3)
Bug fixes:
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11095
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11653
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14731
Changelog:
- Abstract EntityEnclosingMethod class has been introduced to encapsulate common behaviour of all entity enclosing methods
- "Expect: 100-continue" header support
- HttpClient does not hang indefinitely if the server of origin does not return status code 100 when expected. HttpClient resumes sending request body after 3 seconds if no response is sent
- Support for chunk encoded requests in all entity enclosing methods
- Entity enclosing methods do not allow automatic redirection
- More robust (or so I'd like to hope) request content buffering logic
- PostMethod inherited from EntityEnclosingMethod class
- PutMethod inherited from EntityEnclosingMethod class
- Older methods of PostMethod dealing with url-encoded parameters deprecated in favour of new setRequestBody(NameValuePair[]) method
Feedback, critique appreciated, as always.
Jeff, Mike
I decided to not touch request body buffering as yet. The patch is getting really unwieldy. I would like to have it committed, if you confirm it does not break test cases on your systems. We can deal with smaller issues and improvements incrementally once the patch has been checked in
Cheers
Oleg
PS: The patch appears to have exceeded the allowed maximum. It has just been thrown back to me ;-)
I am afraid I'll have to compress the file
Re: [PATCH] PostMethod & PutMethod revision (take 3)
Posted by Jeffrey Dever <js...@sympatico.ca>.
The local and webapp tests are fine here (except that one timeout issue
that has been around for a while).
There are a few things that could be tweaked, but please submit the
patch so we can all benefit from it. Very high quality, great work!
Jandalf
Kalnichevski, Oleg wrote:
>Bug fixes:
>
>http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11095
>http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11653
>http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14731
>
>Changelog:
>
>- Abstract EntityEnclosingMethod class has been introduced to encapsulate common behaviour of all entity enclosing methods
>- "Expect: 100-continue" header support
>- HttpClient does not hang indefinitely if the server of origin does not return status code 100 when expected. HttpClient resumes sending request body after 3 seconds if no response is sent
>- Support for chunk encoded requests in all entity enclosing methods
>- Entity enclosing methods do not allow automatic redirection
>- More robust (or so I'd like to hope) request content buffering logic
>- PostMethod inherited from EntityEnclosingMethod class
>- PutMethod inherited from EntityEnclosingMethod class
>- Older methods of PostMethod dealing with url-encoded parameters deprecated in favour of new setRequestBody(NameValuePair[]) method
>
>Feedback, critique appreciated, as always.
>
>Jeff, Mike
>I decided to not touch request body buffering as yet. The patch is getting really unwieldy. I would like to have it committed, if you confirm it does not break test cases on your systems. We can deal with smaller issues and improvements incrementally once the patch has been checked in
>
>Cheers
>
>Oleg
>
>PS: The patch appears to have exceeded the allowed maximum. It has just been thrown back to me ;-)
>I am afraid I'll have to compress the file
>
>
>------------------------------------------------------------------------
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
>
Re: [PATCH] PostMethod & PutMethod revision (take 3)
Posted by Jeffrey Dever <js...@sympatico.ca>.
>
>
>I'd say that those Eclipse folks might find it an interesting idea to
>toy with. It falls more into their area expertise. Any buddies involved
>in Eclipse?
>
>
Naw, I'm just a user :-)
Re: [PATCH] PostMethod & PutMethod revision (take 3)
Posted by Oleg Kalnichevski <o....@dplanet.ch>.
> But such is with coding style, everyone has their own variation. There
> are such things as Jalopy that will reformat code so you can format in
> your own style and convert later. While this was helpful, I spend more
> time looking at code than writing code, which made it harder to
> comprehend afterwards. Come to think of it, it woulld be cool if the
> editor would show the code in your personal style and save it in the
> project style transparently ... or if code itself was in xml ... I
> wonder if there is a jakarta project for that ...
>
> Jandalf.
>
I'd say that those Eclipse folks might find it an interesting idea to
toy with. It falls more into their area expertise. Any buddies involved
in Eclipse?
Cheers
Olegolas
Re: [PATCH] PostMethod & PutMethod revision (take 3)
Posted by Jeffrey Dever <js...@sympatico.ca>.
>
>
> I have almost started moving code of
>PostMethod.generateRequestBody(NameValuePair[]) to URIUtils class, but
>there's one point that made me reconsider. Currently URIUtils does not
>know any HttpClient specific classes and that sort of makes sense. I am
>not sure if we should make URIUtils be tightly coupled with
>NameValuePair class
>
Good point. I think that is a goal of Sung-Gu. But it makes it less
useful if we can't pass in and get out fundamental objects of
HttpClient. Its too bad Java does not have a Pair class, such as the
excellent pair template in the STL.
>>2) EntityEnclosingMethod
>>There is a useExpectHeader variable, with a setter and a getter, but it
>>is not used for anything.
>>
>>
>Odi once expressed an opinion that there should be an option to disable
>sending of "Expect: 100-continue" header. In that vein addRequestHeaders
>method add the header only in case of useExpectHeader variable being
>true
>
I get it. Cool.
>All the changes you have made look fine to me. I only regret that you
>had to go through this laborious process for me. I have to say I have
>rather non-conventional coding style. I like tons of blanks everywhere.
>I like curly braces on a separate line. I am always in pains when I
>program for HttpClient as I have to adapt my coding style. Sometimes
>style violations do slip into my code
>
>void doStuff( String param1, int param2 )
>{
> try
> {
> }
> catch( Exception e )
> {
> }
>}
>
>
And I am of the type:
void doStuff(String param1, int param2)
{
try {
}
catch (Exception e) {
}
}
But such is with coding style, everyone has their own variation. There
are such things as Jalopy that will reformat code so you can format in
your own style and convert later. While this was helpful, I spend more
time looking at code than writing code, which made it harder to
comprehend afterwards. Come to think of it, it woulld be cool if the
editor would show the code in your personal style and save it in the
project style transparently ... or if code itself was in xml ... I
wonder if there is a jakarta project for that ...
Jandalf.
Re: [PATCH] PostMethod & PutMethod revision (take 3)
Posted by Oleg Kalnichevski <o....@dplanet.ch>.
Jandalf
> 1) PostMethod.generateRequestBody()
> This method just url encodes the parameters. In the 5000 lines of URI
> handling code, isn't there a method that url encodes an array of name
> value pairs? (I looked, but couldn't find one)
>
There are tons of REALLY bizarre stuff in URIUtils (like those
toUsingCharset methods) but nothing of a like of a method encode to
encode name/value pairs. I have almost started moving code of
PostMethod.generateRequestBody(NameValuePair[]) to URIUtils class, but
there's one point that made me reconsider. Currently URIUtils does not
know any HttpClient specific classes and that sort of makes sense. I am
not sure if we should make URIUtils be tightly coupled with
NameValuePair class
I have to say I know little about URI classes. So, I leave this decision
to Sung-Gu
> 2) EntityEnclosingMethod
> There is a useExpectHeader variable, with a setter and a getter, but it
> is not used for anything.
Odi once expressed an opinion that there should be an option to disable
sending of "Expect: 100-continue" header. In that vein addRequestHeaders
method add the header only in case of useExpectHeader variable being
true
protected void addRequestHeaders(HttpState state, HttpConnection conn)
throws IOException, HttpException {
...
super.addRequestHeaders(state, conn);
...
if (isHttp11() && getUseExpectHeader()
&& (this.requestBodyStream != null)) {
if (getRequestHeader("Expect") == null) {
setRequestHeader("Expect", "100-continue");
}
}
}
All the changes you have made look fine to me. I only regret that you
had to go through this laborious process for me. I have to say I have
rather non-conventional coding style. I like tons of blanks everywhere.
I like curly braces on a separate line. I am always in pains when I
program for HttpClient as I have to adapt my coding style. Sometimes
style violations do slip into my code
void doStuff( String param1, int param2 )
{
try
{
}
catch( Exception e )
{
}
}
Enjoy your weekend
Oleg
Re: [PATCH] PostMethod & PutMethod revision (take 3)
Posted by Jeffrey Dever <js...@sympatico.ca>.
Hey Oleg. I'm Playing with PostMethod writing some tests. It looks
really good in there, and will be even better after get rid the
deprecated stuff. I had a few changes, and a few questions:
1) PostMethod.generateRequestBody()
This method just url encodes the parameters. In the 5000 lines of URI
handling code, isn't there a method that url encodes an array of name
value pairs? (I looked, but couldn't find one)
2) EntityEnclosingMethod
There is a useExpectHeader variable, with a setter and a getter, but it
is not used for anything.
3) PostMethod.deprecated_parameters
I made this private because we might as well. Fine about the underscore
in the name, makes it stand out as somthing we *want* to get rid of.
4) PostMethod.URL_ENCODED_CONTENT_TYPE
Made this public. In the case where a user is using
setRequestBody(string) where the string is URL encoded they need a way
to add the right header. I'm using this to simplify the tests I added too.
I made some other minor changes (mostly formatting) which are attached
as a patch. Hope that its OK I committed this. I was going to consult
with you first, but I commited the new test file before I realized that
I was relying on one of the changes, so I had to go for the gusto.
Jandalf.
Kalnichevski, Oleg wrote:
>Bug fixes:
>
>http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11095
>http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11653
>http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14731
>
>Changelog:
>
>- Abstract EntityEnclosingMethod class has been introduced to encapsulate common behaviour of all entity enclosing methods
>- "Expect: 100-continue" header support
>- HttpClient does not hang indefinitely if the server of origin does not return status code 100 when expected. HttpClient resumes sending request body after 3 seconds if no response is sent
>- Support for chunk encoded requests in all entity enclosing methods
>- Entity enclosing methods do not allow automatic redirection
>- More robust (or so I'd like to hope) request content buffering logic
>- PostMethod inherited from EntityEnclosingMethod class
>- PutMethod inherited from EntityEnclosingMethod class
>- Older methods of PostMethod dealing with url-encoded parameters deprecated in favour of new setRequestBody(NameValuePair[]) method
>
>Feedback, critique appreciated, as always.
>
>Jeff, Mike
>I decided to not touch request body buffering as yet. The patch is getting really unwieldy. I would like to have it committed, if you confirm it does not break test cases on your systems. We can deal with smaller issues and improvements incrementally once the patch has been checked in
>
>Cheers
>
>Oleg
>
>PS: The patch appears to have exceeded the allowed maximum. It has just been thrown back to me ;-)
>I am afraid I'll have to compress the file
>
>
>------------------------------------------------------------------------
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
>
Re: [PATCH] PostMethod & PutMethod revision (take 3)
Posted by Oleg Kalnichevski <o....@dplanet.ch>.
Thanks, Mike
Good points. I'll make corrections before committing the patch
Enjoy your weekend
Cheers
Oleg
On Fri, 2003-01-31 at 22:43, Michael Becke wrote:
> Looks good, nice work. I only noticed a few things, mostly in regard to
> formatting, comments.
>
> - in EntityEnclosingMethod there are a few places when there is an extra
> carriage return between the Javadoc comment and the method.
> - PostMethod and EntityEnclosingMethod have some unused imports
> - the @deprecated message for PostMethod.getParameters() points to
> setRequestBody(), but I'm guessing you wanted getRequestBody().
> - EntityEnclosingMethod.setFollowsRedirects() always sets to false.
> perhaps a warning should be printed until this can be remedied.
>
> All tests passed for me.
>
> Mike
>
> Kalnichevski, Oleg wrote:
> > Bug fixes:
> >
> > http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11095
> > http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11653
> > http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14731
> >
> > Changelog:
> >
> > - Abstract EntityEnclosingMethod class has been introduced to encapsulate common behaviour of all entity enclosing methods
> > - "Expect: 100-continue" header support
> > - HttpClient does not hang indefinitely if the server of origin does not return status code 100 when expected. HttpClient resumes sending request body after 3 seconds if no response is sent
> > - Support for chunk encoded requests in all entity enclosing methods
> > - Entity enclosing methods do not allow automatic redirection
> > - More robust (or so I'd like to hope) request content buffering logic
> > - PostMethod inherited from EntityEnclosingMethod class
> > - PutMethod inherited from EntityEnclosingMethod class
> > - Older methods of PostMethod dealing with url-encoded parameters deprecated in favour of new setRequestBody(NameValuePair[]) method
> >
> > Feedback, critique appreciated, as always.
> >
> > Jeff, Mike
> > I decided to not touch request body buffering as yet. The patch is getting really unwieldy. I would like to have it committed, if you confirm it does not break test cases on your systems. We can deal with smaller issues and improvements incrementally once the patch has been checked in
> >
> > Cheers
> >
> > Oleg
> >
> > PS: The patch appears to have exceeded the allowed maximum. It has just been thrown back to me ;-)
> > I am afraid I'll have to compress the file
> >
> >
> > ------------------------------------------------------------------------
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
--
Oleg Kalnichevski <o....@dplanet.ch>
Re: [PATCH] PostMethod & PutMethod revision (take 3)
Posted by Michael Becke <be...@u.washington.edu>.
Looks good, nice work. I only noticed a few things, mostly in regard to
formatting, comments.
- in EntityEnclosingMethod there are a few places when there is an extra
carriage return between the Javadoc comment and the method.
- PostMethod and EntityEnclosingMethod have some unused imports
- the @deprecated message for PostMethod.getParameters() points to
setRequestBody(), but I'm guessing you wanted getRequestBody().
- EntityEnclosingMethod.setFollowsRedirects() always sets to false.
perhaps a warning should be printed until this can be remedied.
All tests passed for me.
Mike
Kalnichevski, Oleg wrote:
> Bug fixes:
>
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11095
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=11653
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14731
>
> Changelog:
>
> - Abstract EntityEnclosingMethod class has been introduced to encapsulate common behaviour of all entity enclosing methods
> - "Expect: 100-continue" header support
> - HttpClient does not hang indefinitely if the server of origin does not return status code 100 when expected. HttpClient resumes sending request body after 3 seconds if no response is sent
> - Support for chunk encoded requests in all entity enclosing methods
> - Entity enclosing methods do not allow automatic redirection
> - More robust (or so I'd like to hope) request content buffering logic
> - PostMethod inherited from EntityEnclosingMethod class
> - PutMethod inherited from EntityEnclosingMethod class
> - Older methods of PostMethod dealing with url-encoded parameters deprecated in favour of new setRequestBody(NameValuePair[]) method
>
> Feedback, critique appreciated, as always.
>
> Jeff, Mike
> I decided to not touch request body buffering as yet. The patch is getting really unwieldy. I would like to have it committed, if you confirm it does not break test cases on your systems. We can deal with smaller issues and improvements incrementally once the patch has been checked in
>
> Cheers
>
> Oleg
>
> PS: The patch appears to have exceeded the allowed maximum. It has just been thrown back to me ;-)
> I am afraid I'll have to compress the file
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org