Hi Roland

> ifce EntityEnclosingMessage {
>   setOgEntity(HttpOgEntity)
>   HttpIcEntity getIcEntity(Class what)
> }

Unfortunately I do not think this solves the problem completely. It is
necessary to be able to set HttpIncomingEntity when assembling the
incoming request on the server. Likewise, it is necessary to be able to
have access to the HttpOutcomingEntity when streaming out the request
body on the client side. Effectively the interface would probably have
to be

ifce EntityEnclosingMessage {
  setOgEntity(HttpOgEntity)
  setIcEntity(HttpInEntity)
  HttpIcEntity getIcEntity(Class what)
  HttpOgEntity getOgEntity()
}

To me this is hardly any better than having to cast to generic interface
to a super interface. Not to mention that this interface looks kind of
schizophrenic.

I am also not in favor of forcing EntityEnclosingMessage impls to know
how to produce entities of different types. This coupling is unjustified
IMO. I believe this problem should be solved through a number of simple
entity consumer classes that "decorate" the Http(Incoming)Entity
interface. Something of this sort:

http://svn.apache.org/repos/asf/jakarta/httpclient/trunk/http-common/src/java/org/apache/http/entity/BufferedHttpEntity.java

> If we don't want a framework to instantiate different types of
> incoming
> entities, there really is no point in the interface. But then I'd go
> all
> the way and move getInputStream() into the HttpMessage rather than
> wrap
> it in a clumsy entity object.

I do want such a framework but I am not in favor of tightly coupling it
with the EntityEnclosingMessage, HttpResponse impl classes.

I can't help thinking we are trying to come up with a solution for a
non-existent problem. Design purism aside, the only real issue with the
current implementation I know of is that _some_ outgoing entities _may_
have _difficulties_ producing an input stream to represent its content.
In the extreme case I would rather prefer to see NotSupportedException
thrown instead of a whole bunch of otherwise unnecessary and/or ugly
interfaces.

cheers,

Oleg


On Thu, Apr 28, 2005 at 01:57:11PM +0200, Roland Weber wrote:
> Hi Oleg,
> 
> [I had an initial version of this mail almost completed
>  when my computer deadlocked. I really hate computers.]
> 
> > My main point is that HttpEntity that cannot produce its content is
> > COMPLETELY useless unless cast to a more specific interface. Having to
> > cast HttpEntity interface EVERY TIME it is used results in a quite ugly
> > and IMO irritating code.
> 
> I agree. I wrote HttpEntity into the high level design solely to combine
> the two trees of incoming and outgoing entities. I never expected it to
> be used in a method signature.
> 
> > Likewise if HttpEntityEnclosingRequest interface is made to produce
> > an HttpOutpoingEntity on the client side, this interface is no longer
> > useable on the server, because the server expects an HttpIncomingEntity
> > instead. Inevitably one needs two interfaces
> > HttpIncomingEntityEnclosingRequest and
> > HttpOutgoingEntityEnclosingRequest to solve the problem. The same story
> > goes for the HttpResponse interface. One would also need two super
> > interfaces HttpIncomingResponse and HttpOutgoingResponse. For all these
> > interfaces one would need HttpMutable* counterparts. The situation gets
> > completely ridiculous, at least in my opinion, and we end up with a
> > classical over-designed API. 
> 
> I agree. Here is an alternative solution to keeping the number of 
> interfaces
> reasonable: have separate methods for incoming and outgoing entities in 
> the
> same interface. (ic = incoming, og = outgoing)
> 
> ifce EntityEnclosingMessage {
>   setOgEntity(HttpOgEntity)
>   HttpIcEntity getIcEntity(Class what)
> }
> 
> ifce TwoWayEntity implements HttpIcEntity, HttpOcEntity
> 
> class BasicHttpMessage implements EntityEnclosingMessage {
>   boolean i_was_received
>   HttpOgEntity og_entity
>   HttpIcEntity ic_entity
> 
>   setOgEntity(HttpOgEntity oge) {
>     if i_was_received throw "are you kidding?"
>     og_entity := oge
>   }
> 
>   HttpIcEntity getIcEntity(Class what) {
>     if (i_was_received) {
>       if ic_entity is null {
>         ic_entity := (instantiate 'what' from InputStream)
>       } else {
>         if ic_entity not instance of 'what' -> fail
>         if ic_entity not repeatable -> fail
>       }
>       return ic_entity
>     } else { // i was not received
>       if og_entity instanceof (TwoWayEntity, what)
>         return og_entity
>     } else {
>       throw "entity not accessible or non-existant"
>     }
>   }
> } // class BasicHttpMessage
> 
> 
> Instead of having a getEntity() which requires a cast that may or may not
> work on the client or server side, you have to different methods to access
> either what's coming in or going out, which may or may not work on the 
> client
> or server side. This should solve the casting problem.
> 
> > We have long maintained a stance that HttpClient is a content agnostic
> > library. Anything that has to do with content processing should be built
> > on top of HttpClient and should not be a part of the library. We
> > certainly should not get ourselves into the MIME content business. I
> > personally would rather prefer a simple consumer pattern where various
> > HTTP content consumers simply retrieve the content from
> > HttpEntity#getContent as an InputStream, hence the motivation for moving
> > this functionality to the base interface
> 
> If we don't want a framework to instantiate different types of incoming
> entities, there really is no point in the interface. But then I'd go all
> the way and move getInputStream() into the HttpMessage rather than wrap
> it in a clumsy entity object.
> 
> HttpClient as it is supports different ways to provide a request body:
> byte array, string, input stream, and multipart-mime (which we would
> like to move to commons-codec). The idea of the Ic/OgEntity stuff was
> to maintain this capability, and to match it with the complementary
> conversion for incoming message bodies. As a framework, *without*
> providing significantly more than samples for non-trivial content types.
> I have no problem with dropping the idea, but then it should be dropped
> completely so that only HttpOutgoingEntity remains (under whatever name).
> 
> > All these drivels aside I'll be happy to see a patch that keeps
> > HttpEntity/HttpIncomingEntity/HttpOutcomingEntity paradigm (which is
> > quite beautiful from the conceptual standpoint) and addresses the above
> > mentioned shortcomings.
> 
> And I would love to provide one. Alas, as an IBMer I am not allowed to
> post executable code without going through approval hell, involving
> about 6 levels in the functional guidance hierarchy and two levels in
> each the general law and IP law hierarchy. And I don't even have the
> business case to get the approval process started. [I'm serious.]
> 
> > I just _personally_ do not see how this can be
> > done, at least at this point
> 
> The proposal above should deal with your immediate concerns, which were
> the typecasts. The fact that inappropriate methods become available in the
> interfaces - like setOutgoingEntity() on a response on the client side -
> merely exposes a problem that was hidden before by dealing with incoming 
> and
> outgoing entities through the same methods. I believe the problem stems
> from using the same request/response objects on the client and server 
> side,
> although their function (incoming/outcoming) is reversed. I don't think
> you can accurately address this problem by changing the entity interface
> hierarchy. Whether there should be a framework for instantiating different
> types of incoming entities at all is a separate point for discussion.
> 
> cheers,
>   Roland
>  

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to