New CloseableHttpClient instance created for each single call

[sabirove]

Target artefact:

  <groupId>net.avalara.avatax</groupId>
  <artifactId>avatax-rest-v2-api-java_2.11</artifactId>
  <version>21.1.2</version>

Subject: net.avalara.avatax.rest.client.RestCall net.avalara.avatax.rest.client.RestCallFactory net.avalara.avatax.rest.client.AvaTaxClient

Problem:

1. A new CloseableHttpClient is created for each call with a complete set of underlying resources (factories, connection pools, caches etc). This is extremely inefficient and wasteful. Furthermore, within RestCall::call it is used with one-shot semantics and is not closed after the call is done which can easily cause resource leaks.
2. AvaTaxClient instance has no way to close it (e.g. shutting down the executor) when we're done with it. This is required to allow for component lifecycle control (e.g. for applications that run in the managed lifecycle environments (containers) like Spring container).

Suggestion:

1. Use single CloseableHttpClient as constructor parameter (use default if none provided)
2. Make AvaTaxClient implement Closeable to allow for resource deallocation (ExecutorServcie + CloseableHttpClient)

[sabirove]

Suggested workaround for the time being (this is in kotlin, Java version can be easily inferred):

class SameInstanceHttpClientBuilder(private val client: CloseableHttpClient) : HttpClientBuilder() {
    override fun build(): CloseableHttpClient = client
}

class FixedAvaTaxClient(
    appName: String,
    appVersion: String,
    machineName: String,
    environment: AvaTaxEnvironment,
    private val threadPool: ExecutorService = Executors.newFixedThreadPool(3),
    private val client: CloseableHttpClient = HttpClients.createDefault()
) : AvaTaxClient(
    appName,
    appVersion,
    machineName,
    environment,
    threadPool,
    SameInstanceHttpClientBuilder(client)
), Closeable {

    override fun close() {
        var ex: Throwable? = null
        try {
            threadPool.shutdownNow()
        } catch (e: Throwable) {
            ex = e
        }
        try {
            client.close()
        } catch (e: Throwable) {
            if (ex == null) ex = e 
            else ex.addSuppressed(e)
        }
        if (ex != null) throw ex
    }
}

[svc-developer]

Added to 24.8.2. Please give it a try and let us know any feedback. Thank you