hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: Code Review Http-Core
Date Sun, 01 Jan 2006 19:59:19 GMT
Ortwin Gl├╝ck wrote:
> Hi Oleg,
> 
> As promised I have taken time and reviewed the http-core code. Here are 
> my thoughts.
> 

Hi Odi,

Many thanks for taking a look at the code. See my comments inline

> _________________________________________
> HttpClientConnection/HttpServerConnection
> 
> Currently the two interfaces look quite different: while the client 
> interface offers fine grained methods to send headers and body 
> separately, the server interface does not provide similar functionality. 
> This should be harmonized.
> 
> The client interface competes in functionality with the 
> HttpRequestExcecutor.
> 
> I conclude the following:
> 
> The sendRequest method should be removed from the HttpClientConnection. 
> Sending a whole request is the job of the executor which already handles 
> entity enclosing requests. It should also take care of requests without 
> an entity.
> 
> Add a sendRequestLine method instead. sendRequestHeader should then 
> accept a HttpRequest instead of a HttpEntityEnclosingRequest.
> 
> Likewise the sendResponse method in HttpServerConnection should be 
> broken apart into sendResponseHeaders and sendResponseBody, so an 
> executor has more control over the details. Currently the default 
> implementation does not use an executor.
> 

Agreed. That seems like a better approach

> __________
> HttpEntity
> 
> getContentType and getContentEncoding should not return Headers but 
> rather Strings.
> 

I made this change just very recently while optimizing the memory 
footprint of the HttpCore code. This interface allows for (1) lazy 
parsing of the said headers and (2) their efficient tokenization into an 
array of header elements using the original char buffer.

> _____________________________
> Request/Response interceptors
> 
> The order of multiple interceptors may be important (I am thinking of 
> transparent compression or encryption, or would you rather implement 
> that in the entity generator?). But currently they are stored in a 
> (unordered) hash set in AbstractHttpProcessor.
> 

This was already pointed out by Roland. I'll fix it shorty.

> ______________________
> DefaultEntityGenerator
> 
> It's a simple implementation but I find it too monolithic. An 
> interceptor pattern (message interceptor) similar to the 
> request/response interceptors may give us more flexibility here.
> 

I personally believe the existing request/response interceptor framework 
already gives us enough flexibility to pre-/post-process HTTP entities.

Besides I find very improbable that someone would ever want to heavily 
customize the way HTTP entities are generated. And If some one did, a 
custom implementation of the EntityGenerator would be a fair price to 
pay for that. Let us not build an abstraction for something that might 
never be used in the real world apps

> 
> Happy new year
> 

Happy new year and stuff

Oleg

> Odi
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: httpclient-dev-help@jakarta.apache.org
> 
> 


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


Mime
View raw message