hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeffrey Dever <jsde...@sympatico.ca>
Subject Re: [PATCH] PostMethod & PutMethod revision (take 2)
Date Fri, 31 Jan 2003 03:28:57 GMT
1) I'm sure stuff like this is unintentional:
- * @author Ortwin Glück
- * @author <a href="mailto:mbowler@GargoyleSoftware.com">Mike Bowler</a>
+ * @author Ortwin Glück

2) I thought we had deprecated the "use disk" methods in GetMethod, but 
I don't see it that way in the source.  Didn't we agree that those 
methods were buggy unnecessisary but we can't remove them yet so we 
decided to deprecate them?  It would have an impact on your 

3) There are a couple of formatting violations, but we can pick that up 
with checkstyle

4) Magic number to set the buffer size         byte[] tmp = new byte[4096];

5) the refactoring into the super class looks great

6) can you make any new methods/classes @since 2.0beta1

7) About Mikes idea for having two different classes for stream posting 
and parameters posting.  I don't really like what that does to the 
public interface.  Its already fat when it comes to parameters, and 
conceptually it is just a POST method.  

Here is another idea.  Using streams is the general case, and the 
parameters is the special case.  Treat the parameters as a stream.

Right now, there is a shitload of methods for dealing with those stupid 
addParameter(string, string)
removeParameter(string, string)
setParameter(string, string) deprecated

The client really does not need so much access to the parameters and 
this is classic interface bloat.  All the client needs is:
setRequestBody(String stringBody)
setRequestBody(InputStream streamBody)
setRequestBody(NameValuePair[] parametersBody)

They are mutually exclusive right?  You can't have more than one body, 
and parameters are url encoded  to make the body.  They should all have 
the same method name, so it is clear that they all set the same thing, 
in different ways.

Right now, PostMethod is very anal.  If you call setRequestBody, and 
then addParameters, it throws an IllegalStateException.  Thats lame. 
 Its awkward, there is no obvious relationship between setRequestBody 
and addParameters but they are still mutually exclusive.

Internally we should just have one "body" datamember, which would be an 
InputStream.  When setting the request body, if the streamBody() form is 
used, its trivial.  When the stringBody() is used, create a 
StringInputStream which is easy.  When the parametersBody() form is 
used, url encode it to a string, and use that as the input stream, also 

The Content-Length could be calculated directly in the stringBody and 
parametersBody cases.   So you could chunk or not chunk with no problem. 
 The streamBody form is tricker, but no more so than what we currently 
face.  We could add another form of the stream body form that explicitly 
takes the content length and make that problem go away.
setRequestBody(InputStream streamBody, int contentLength)

What do you say to that?  We would get a simpler Post method, only one 
post method, and have a nice core streaming facility without any other 
datamembers getting in the way.  It would be a little awkward to 
deprecate appropriately, but it wont be as bad because of the current 
aformentioned anality of PostMethod.


Kalnichevski, Oleg wrote:

>Bug fixes:
>- Abstract EntityEnclosingMethod class has been introduced to encapsulate common behaviour
of all entity enclosing methods 
>- Expect: 100-continue" 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
>- More robust (or so I'd like to hope) request content buffering logic
>- PostMethod inherited from EntityEnclosingMethod class
>- PutMethod inherited from EntityEnclosingMethod class
>To be done next:
>Feedback, critique would be REALLY appreciated. I really would like to get this patch
committed by Sunday as I may be unable to work on HttpClient from Feb. 2nd to Feb. 28th. Please
let me know if you see a more elegant solution than HttpConnection.waitForResponse(long) of
>Jeff, Odi, 
>if you are around, please give me your opinion on Mike's suggestion about providing two
more specialized version of PostMethod

View raw message