For being able to implement something like keep-alive timeouts without needing an API change, these are my thoughts..
ManagedClientConnection is OK -- the open(...) method takes an HttpContext, and that context is pretty safe to reuse over the lifetime of that connection. You could pretty easily define a contract that the user of the connection puts a value in the context, ManagedClientConnection could keep a handle on it from the open method and then look for the value whenever needed (like releaseConnection). ClientConnectionManager is a little more tricky. You could capture the HttpContext the same way as ManagedClientConnection could do it internally, except also expose it -- since ClientConnectionManager determines the ManagedClientConnection implementation, it can make sure it exposes the HttpContext however it needs it. (One example being setting parameters before calling connectionPool.freeEntry from ClientConnectionManager.releaseConnection.) AbstractConnPool doesn't really have any arbitrary map-like construct in its PoolEntries. It's OK though, because it's an abstract class, and methods can be added when needed (and method signatures overloaded when needed). The whole thing is similiar to the changes required for adding state to the connections. The data could be pulled directly from the HttpContext if wanted, so adding a get/set state to ManagedClientConnection is a useful addition, but not a strictly required one. Then to pass the information through to the PoolEntry so that the ClientConnectionManager could use it, there absolutely needed to be a get/set state on AbstractPoolEntry -- no other way around it. So, in summary... It definitely does seem like state (for auth tokens) and keep-alive timeouts could have been added without API changes, so everything looks to me. Still, I think it'd be useful to maybe change get/setState in AbstractPoolEntry to get/setAttribute(TSCCMContext.USER_TOKEN, ..). This works perfectly (with alloc entry nulling the whole context before closing it, and the check for the state being same [to reuse the connection] looks just as the user token attribute). But then again, since AbstractConnPool is abstract, that can be done whenever it's needed too. Sam On Thu, Jul 10, 2008 at 5:57 PM, Sam Berlin <[EMAIL PROTECTED]> wrote: > That's the question, really. I'll dig into it a bit deeper tonight > and try to come up with some more formalized thoughts -- I just > remember having this nagging feeling while doing the keep-alive that > it was only possible to add because the API wasn't frozen. Ideally, > it would have been possible (if a little less elegant) even if the API > was frozen. > > Sam > > On Thu, Jul 10, 2008 at 5:52 PM, Oleg Kalnichevski <[EMAIL PROTECTED]> wrote: >> On Thu, 2008-07-10 at 17:32 -0400, Sam Berlin wrote: >> >> ... >> >>> While adding keep-alive support, I noticed that code-changes that need >>> to provide the ClientConnectionManager with more information aren't >>> possible without API changes. In the rest of HttpClient's code, >>> there's usually an HttpContext being passed around that can hold >>> key/value pairs so things can easily be plugged in without requiring >>> full-blown API changes. I can't see any way to do this with >>> ClientConnectionManager currently. Maybe there should be some Context >>> that that uses? Or share the HttpContext itself? This will likely >>> become an issue after an API freeze if there's more data we find >>> ourselves suddenly wanting. >>> >>> Sam >> >> What methods of ClientConnectionManager should take a context as an >> additional parameter? >> >> Oleg >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [EMAIL PROTECTED] >> For additional commands, e-mail: [EMAIL PROTECTED] >> >> >
