Rodney Waldhoff wrote:

> I've been poking through the ConnectionInterceptor and StreamInterceptor
> interfaces in Http Client, and I'm confused by a couple of things.
>
> I was unable to find an example implemenation of either interceptor in
> Slide, although Slide exposes setters that may be used to set the
> interceptor reference, so maybe somebody's using them somewhere and can
> comment on their expectations.

WebDavSession implements ConnectionInterceptor

I haven't designed them, better wait until Remy is back.

> StreamInterceptor:
>
> Although it's not documented, I think it is pretty clear from the code that
> StreamInterceptor provides a mechansim for an "external" class to be
> notified of the (body but not head) bytes read from or written to the
> server.
>
> * Is that correct?

I would also include the headers and method/status line

> * Is it reasonable to assert (that is, restrict by contract) that the byte
> data sent to StreamInterceptor is read only, or is the intention to allow
> the interceptor to somehow change the data being exchange?

Good enough for me.

> * When precisely should the bytesRead and bytesWritten methods be called?
> Just before the data is sent to the server? Just after the data is read back
> (but before the data is processed)?

As close to the actual read/write as possible.

> * Also, I note that there is no reference to or notice of the specific
> HttpMethod being invoked, or when we switch from one request/response pair
> to another.  I can't tell, for example, if a given request body is a PUT or
> a POST request by the methods in StreamInterceptor alone. If that's the
> case, it's not clear to me what the utility of StreamInterceptor really is.

If you add the read/write of the method/status line and headers, this would be
the perfect way to replace those setDebug() methods. The interceptor would then
be used to record the communication between server and client.

> ConnectionInterceptor:
>
> This one is more vexing.  I *think* I understand the general intent, but if
> I do, the kind of information supplied to ConnectionInterceptor isn't quite
> adequate for that purpose.  Also, it would seem that the current
> implementation doesn't adhere to the apparent contract very uniformly.  Some
> specific questions/issues:
>
> * ConnectionInterceptor.retry(int) returns a boolean, and the JavaDoc
> comment says "return true if a retry should be attempted".  Yet:
>
> a) This method is not uniformly applied (i.e., it is not invoked every time
> a request is to be retried)
>
> b) The returned boolean value is ignored (in fact, it is not even
> collected.)
>
> * ConnectionInterceptor.info(int,Hashtable) also returns a boolean, with the
> same comment, which isn't quite clear in this case. Keep going I assume?
> But again, this returned value is ignored by HttpClient. Also, it isn't
> clear to me when this method will be/should be invoked. Only when we get the
> intermediate response codes (100 Continue, etc.)?
>
> * Same with error(int,Exception)
>
> * When precisely should the remaining methods be invoked?
>
> * As with StreamInterceptor, there is no reference to or notice of the
> specific HttpMethod being invoked, to which server we are
> connecting/disconnecting, the way in which we are authenticating, the realm
> to which we are authentciating, the credentials that are being posted, etc.

I suspect the methods without a return value are just a notification to a client
user interface.
The ones with a return value can then be used to change the behaviour of
HttpClient
(do not retry on timeout or do retry on service unavailable)

Something like:
void requiredAuthentication()    -> ask user for password and setCredentials
boolean error()                         -> ask for other password and return if
retry is needed

> Can someone clarify how the Interceptors are to be used?  Are they being
> used anywhere?  If not, maybe we should simply remove these classes for a
> 1.0 release, and wait until the role and contract details can be worked out.

It would be nice to be able to use the StreamInterceptor as a stream logger and
the ConnectionInterceptor will also be appriciated by gui programmers.
But as you say, a different interface is needed to be able to support multiple
credentials and if you don't have the time to correct the interface better
remove it.


Dirk

Reply via email to