On Sat, 2007-01-13 at 15:40 +0100, Roland Weber wrote: > Hi Oleg, > > > I like you have done so far and I think it is a good start. Now onto the > > details. > > Thanks. > > > My gripes are rather minor and mostly related to class / method naming. > > That is very important. > > > (1) I actually think ManagedClientConnection would be a better name for > > the interface, because it reflects the fact that the connection is not > > self-containing and requires an external entity to manage its state. > > It needs an external entity, but that entity is the application and not > a connection manager. My thoughts about connection management so far > lead me to believe that the prepare/open/update methods in this form > should not be available at all for connections obtained from a connection > manager. There is no way to distinguish an update() for "tunnel created" > from an update() for "TLS/SSL socket layered". > > > (2) Can we do without #prepare(Socket) method? Do we really need it? > > What is the point of giving the connection an unconnected socket? > > I'm afraid not. I thought the JavaDocs were clear about the reason? > > This allows the connection to close that socket if shutdown > is called before it is open. Closing the unconnected socket > will interrupt a thread that is blocked on the connect. > > See also https://issues.apache.org/jira/browse/HTTPCLIENT-475, the > original problem there was that connection.abort() does not close > the underlying socket on which a thread was blocked while connecting. > I'm open to better names than prepare(), but we need to tell the > connection which socket to close if it is aborted. >
True. But I somehow I can't help thinking #open() and #update() might suffice. Why not use #open() / #update() instead of #prepare / #open()? I'll try to put together a patch for your consideration. Oleg > >> - org.apache.http.conn.SocketConnectionOperator > > > > How about ClientConnectionOperaton? > > Fine by me. I don't expect to see any non-socket operators in > HttpConn or HttpClient, so we can drop the Socket from the name. > > >> a) Extended connection interface. > >> I can't develop the interface without rewriting the connection > >> manager to see what is actually needed. That's a major effort, > >> and I can't tell yet when I will have the time. > > > > We need something that works _now_, something we can run the existing > > test cases against, before we get carried too far away with all sorts of > > fancy ideas. I feel very strongly that at this point of time having > > something that works should take precedence over API clarity. Let us > > just merge the functionality of the HttpHostConnection into the > > UnmanagedClientConnection, mark it 'to be reviewed', and keep changes to > > the existing connection managers to the minimum. > > That's a fair request. My problem is the complexity of the MTHCM. > It's got several inner classes, including a wrapper for the connection. > And the original connection class has specific methods like tunnelCreated > which do not map to the new interface. As I wrote, it wasn't meant to be > for connections from a connection manager. > What needs to be done needs to be done, so I'll hack up a temporary > new interface for connection management. However, I'm not going to start > on this until I know that I've got more than just a few short hours on a > week-end. That thing is way too complex for a quick shot, in particular > since I'll have to rewrite the test cases too. It'll probably be February > until I find enough time, but I'll keep it at the top of my list. > > > >> b) Encapsulate tunnelling logic. > > > > Let us just port the method director to HttpCore and then see what needs > > to be refactored and what could be improved. > > There's no other way to go forward anyway. I just wanted to > put on record what I consider to be the long-term goal. > > >> c) Determine socket security dynamically. > > > > Give it a shot > > Ok. Not this week-end though. > > cheers, > Roland > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
