hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: HttpEntity#consumeContent deprecated; was Re: HttpEntity#writeTo and #consumeContent
Date Wed, 21 Apr 2010 18:23:43 GMT
On Wed, 2010-04-21 at 10:01 -0400, James Leigh wrote:
> On Sat, 2010-04-10 at 17:43 +0200, Oleg Kalnichevski wrote:
> > On Sat, 2010-04-10 at 11:12 -0400, James Leigh wrote:
> > > On Fri, 2010-04-09 at 21:12 +0200, Oleg Kalnichevski wrote:
> > > > 
> > > > James et al
> > > > 
> > > > I deprecated HttpEntity#consumeContent in favor of a more standard
> > > > InputStream#close contract. Javadocs / tutorial have been updated to
> > > > describe the recommended way to ensure resource deallocation: 
> > > > 
> > > > http://svn.apache.org/viewvc?rev=932551&view=rev
> > > > 
> > > > Please review / comment / critique
> > > > 
> > > > Oleg
> > > > 
> > > 
> > > Thanks for doing this Oleg. However, it is not clear from the revised
> > > javadocs if getContent().close() must be called if the content is not
> > > read. 
> > 
> > The trouble is this very much depends on the connection management
> > logic, which is out of scope for HttpCore. If one does not intent to
> > re-use the underlying connection is going to close it in any way, there
> > is no point reading the content.
> > 
> > I intent to address this issue in the documentation for HttpClient once
> > HttpClient is upgraded to the next version of HttpCore.  
> 
> > > 
> > > While getContent().close() works there are now many ways to clean-up the
> > > same resources and the javadocs need to be very clear. Perhaps more
> > > information in needed in HttpMessage and its sub-interfaces?
> > > 
> > 
> > I am not much of a writer. I always tend to be too terse with my
> > writing. Please do feel free to improve the javadocs and submit the
> > changes as a patch.
> > 
> > Cheers
> > 
> > Oleg
> > 
> 
> How about the following javadoc additions?
> 
> For HttpMessage we could say:
> 
> Some instances of this require resources to be manually closed. If this
> instance is also an instance of HttpEntityEnclosingRequest or
> HttpResponse and the result of getEntity() is non-null, it must be
> manually closed. See HttpEntity for how to close the resource.
> 
> For HttpRequest we could say:
> 
> Some instances of this require resources to be manually closed. If this
> instance is also an instance of HttpEntityEnclosingRequest and the
> result of getEntity() is non-null, it must be manually closed. See
> HttpEntity for how to close the resource.
> 
> For HttpEntityEnclosingRequest we could say:
> 
> Instances of this interface may require resources to be manually closed.
> If the result of getEntity() is non-null, it must be manually closed.
> See HttpEntity for how to close the resource.
> 
> For HttpResponse we could say:
> 
> Instances of this interface may require resources to be manually closed.
> If the result of getEntity() is non-null, it must be manually closed.
> See HttpEntity for how to close the resource.
> 
> For HttpEntity we could say:
> 
> Instances of this interface require resources to be manually closed. To
> free up any used resources every instance of HttpEntity must either call
> #writeTo or InputStream#close() on the resust of #getContent() or if
> this is also an instance of ProducingNHttpEntity or ConsumingNHttpEntity
> #finish()
> 
> For ProducingNHttpEntity we could say:
> 
> See HttpEntity on how to close this resource.
> 
> For ConsumingNHttpEntity we could say:
> 
> See HttpEntity on how to close this resource.
> 
> James
> 

James,

It would be much easier for me if could submit these changes as a patch
in udiff format against SVN trunk. 

Evil Extortionist Oleg 

PS:

I have also been thinking about using generics in HttpEntity and related
interfaces to make it more apparent that some entities are backed by a
dynamic stream of data that needs to be properly closed out. Something
along these lines:

interface HttpEntity<T> {

  T getContent();

}

interface HttpResponse<T> {

  HttpEntity<T> getContent();

}

HttpResponse<InputStream> response = httpclient.execute(...);
HttpEntity<InputStream> entity = response.getEntity();




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


Mime
View raw message