#193 HapiContext.newClient can return same connection

[DiGiT]

It seems that calling HapiContext.newClient() with the same host:port combination returns the same Connection which conflicts with the Javadoc.

Steps to reproduce:
1. Have an HL7 listener running on port 5678
2. Call hapiContext.newClient("localhost", 5678, false)
3. Call hapiContext.newClient("localhost", 5678, false)

Expected Result:
Javadoc implies that newClient() should return a new Connection every time.

Actual Result:
Step 3 returns same Connection as step 2.

Thanks.

[DiGiT]

Or maybe it's intentional to return the same connection in which the Javadoc should be updated. And how would you open multiple connections to the same host:port in that case?

[Christian Ohr]

Yes, connection reuse is intended. I clarified the javadocs.

[DiGiT]

OK, thanks. The recommended way to force a new connection with HAPI 2.1 is still through directly constructing a new Connection?

[James Agnew]

I believe it should be possible to force a new connection by creating a new HapiContext instance. That's definitely the best way to do it, since Connection really ought to be a part of the internal API as opposed to something that developers create directly.

I've added that as a note to your already enhanced docs Christian.

I think though that it would make sense to add a configuration flag or an alternate method or something to force the creation of a new connection instead of reusing existing ones. Creating a new context instance is easy of course, but it can be inefficient since you lose all of the performance advantage that comes with the PipeParser's internal runtime structure cache.

I'm reopening this for your thoughts Christian. Feel free to close again if you disagree. :)

[James Agnew]

One more thought- Should getConnectionHub() be deprocated? The only use case I can see where it makes sense to call it would be to manually shut it down (and thereby close any pooled connections) but that is deprocated itself (as we recently had pointed out on the listserv).

Maybe the HapiContext should have a shutdown method itself which force-closes all clients and servers without touching the executor service?

[Christian Ohr]

+1 to have HapiContext implement Closeable, although we need to track server instances then, which is currently not done. Then we can also deprecate getConnectionHub, so ConnectionHub becomes completely invisible...

In this respect I would rather not allow open redundant connections through the HapiContext. I don't think it's needed very often, and there are workarounds (likely directly constructing a Connection or open a 2nd context).

[DiGiT]

Well, my use case is a test tool that must open multiple connections to simulate real client behavior. So I'm fine with directly constructing a Connection but please don't make me setup another HapiContext since it contains lots of other stuff that I want to share.

HapiContext.close() would be very useful; not just for Connections but the ExecutorService which can keep an app running.

[Christian Ohr]

Committed that HapiContext implements Closeable, but server instances are not yet stopped.