Improve http client instance per request handling

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
seqeralabs · Sep 28, 2023 · 616c107 · 616c107
1 parent 2baf6cd
commit 616c107
Show file tree

Hide file tree

Showing 9 changed files with 37 additions and 60 deletions.
diff --git a/src/main/groovy/io/seqera/wave/auth/RegistryAuthServiceImpl.groovy b/src/main/groovy/io/seqera/wave/auth/RegistryAuthServiceImpl.groovy
@@ -18,7 +18,7 @@
 
 package io.seqera.wave.auth
 
-import java.net.http.HttpClient
+
 import java.net.http.HttpRequest
 import java.net.http.HttpResponse
 import java.util.concurrent.ExecutionException
@@ -34,10 +34,10 @@ import groovy.transform.CompileStatic
 import groovy.transform.ToString
 import groovy.util.logging.Slf4j
 import io.seqera.wave.configuration.HttpClientConfig
+import io.seqera.wave.http.HttpClientFactory
 import io.seqera.wave.util.Retryable
 import io.seqera.wave.util.StringUtils
 import jakarta.inject.Inject
-import jakarta.inject.Named
 import jakarta.inject.Singleton
 import static io.seqera.wave.WaveDefault.DOCKER_IO
 /**
@@ -76,10 +76,6 @@ class RegistryAuthServiceImpl implements RegistryAuthService {
                     .expireAfterAccess(1, TimeUnit.HOURS)
                     .build(loader)
 
-    @Inject
-    @Named("follow-redirects")
-    private HttpClient httpClient
-
     @Inject
     private RegistryLookupService lookupService
 
@@ -95,6 +91,7 @@ class RegistryAuthServiceImpl implements RegistryAuthService {
      * @return {@code true} if the login was successful or {@code false} otherwise
      */
     boolean login(String registryName, String username, String password) {
+        final httpClient = HttpClientFactory.followRedirectsHttpClient()
         // 0. default to 'docker.io' when the registry name is empty
         if( !registryName )
             registryName = DOCKER_IO
@@ -193,6 +190,7 @@ class RegistryAuthServiceImpl implements RegistryAuthService {
      * @return The resulting bearer token to authorise a pull request
      */
     protected String getToken0(CacheKey key) {
+        final httpClient = HttpClientFactory.followRedirectsHttpClient()
         final login = buildLoginUrl(key.auth.realm, key.image, key.auth.service)
         final req = makeRequest(login, key.creds)
         log.trace "Token request=$req"

diff --git a/src/main/groovy/io/seqera/wave/auth/RegistryLookupServiceImpl.groovy b/src/main/groovy/io/seqera/wave/auth/RegistryLookupServiceImpl.groovy
@@ -18,7 +18,7 @@
 
 package io.seqera.wave.auth
 
-import java.net.http.HttpClient
+
 import java.net.http.HttpRequest
 import java.net.http.HttpResponse
 import java.util.concurrent.TimeUnit
@@ -29,9 +29,9 @@ import com.google.common.cache.LoadingCache
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
 import io.seqera.wave.configuration.HttpClientConfig
+import io.seqera.wave.http.HttpClientFactory
 import io.seqera.wave.util.Retryable
 import jakarta.inject.Inject
-import jakarta.inject.Named
 import jakarta.inject.Singleton
 import static io.seqera.wave.WaveDefault.DOCKER_IO
 import static io.seqera.wave.WaveDefault.DOCKER_REGISTRY_1
@@ -50,9 +50,6 @@ class RegistryLookupServiceImpl implements RegistryLookupService {
     @Inject
     private HttpClientConfig httpConfig
 
-    @Inject
-    @Named("follow-redirects")
-    private HttpClient httpClient
 
     private CacheLoader<URI, RegistryAuth> loader = new CacheLoader<URI, RegistryAuth>() {
         @Override
@@ -71,6 +68,7 @@ class RegistryLookupServiceImpl implements RegistryLookupService {
 
 
     protected RegistryAuth lookup0(URI endpoint) {
+        final httpClient = HttpClientFactory.followRedirectsHttpClient()
         final request = HttpRequest.newBuilder() .uri(endpoint) .GET() .build()
         // retry strategy
         final retryable = Retryable

diff --git a/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy b/src/main/groovy/io/seqera/wave/core/RegistryProxyService.groovy
@@ -18,7 +18,6 @@
 
 package io.seqera.wave.core
 
-import java.net.http.HttpClient
 
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
@@ -31,14 +30,14 @@ import io.seqera.wave.auth.RegistryAuthService
 import io.seqera.wave.auth.RegistryCredentials
 import io.seqera.wave.auth.RegistryCredentialsProvider
 import io.seqera.wave.auth.RegistryLookupService
+import io.seqera.wave.http.HttpClientFactory
 import io.seqera.wave.model.ContainerCoordinates
 import io.seqera.wave.proxy.ProxyClient
 import io.seqera.wave.service.CredentialsService
 import io.seqera.wave.service.persistence.PersistenceService
 import io.seqera.wave.storage.DigestStore
 import io.seqera.wave.storage.Storage
 import jakarta.inject.Inject
-import jakarta.inject.Named
 import jakarta.inject.Singleton
 import static io.seqera.wave.WaveDefault.HTTP_REDIRECT_CODES
 /**
@@ -73,10 +72,6 @@ class RegistryProxyService {
     @Inject
     private RegistryAuthService loginService
 
-    @Inject
-    @Named("never-redirects")
-    private HttpClient httpClient
-
     @Inject
     private PersistenceService persistenceService
 
@@ -87,6 +82,7 @@ class RegistryProxyService {
     }
 
     private ProxyClient client(RoutePath route) {
+        final httpClient = HttpClientFactory.neverRedirectsHttpClient()
         final registry = registryLookup.lookup(route.registry)
         final creds = getCredentials(route)
         new ProxyClient(httpClient)

diff --git a/src/main/groovy/io/seqera/wave/http/HttpClientFactory.groovy b/src/main/groovy/io/seqera/wave/http/HttpClientFactory.groovy
@@ -19,59 +19,45 @@
 package io.seqera.wave.http
 
 import java.net.http.HttpClient
+import java.time.Duration
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.Executors
 
 import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
-import io.micronaut.context.annotation.Bean
-import io.micronaut.context.annotation.Factory
-import io.seqera.wave.configuration.HttpClientConfig
-import jakarta.inject.Inject
-import jakarta.inject.Named
 /**
  * Java HttpClient factory
  *
  * @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
  */
-@Factory
 @Slf4j
 @CompileStatic
 class HttpClientFactory {
 
     static private ExecutorService virtualThreadsExecutor = Executors.newVirtualThreadPerTaskExecutor()
 
-    static private HttpClient INSTANCE
+    static private Duration timeout = Duration.ofSeconds(20)
 
-    static private final Integer hold = Integer.valueOf(0)
-
-    @Inject
-    HttpClientConfig httpConfig
-
-    @Bean
-    @Named("follow-redirects")
-    HttpClient followRedirectsHttpClient() {
+    static HttpClient followRedirectsHttpClient() {
         return HttpClient.newBuilder()
                 .version(HttpClient.Version.HTTP_1_1)
                 .followRedirects(HttpClient.Redirect.NORMAL)
-                .connectTimeout(httpConfig.connectTimeout)
+                .connectTimeout(timeout)
                 .executor(virtualThreadsExecutor)
                 .build()
     }
 
-    @Bean
-    @Named("never-redirects")
-    HttpClient neverRedirectsHttpClient() {
+    static HttpClient neverRedirectsHttpClient() {
         return HttpClient.newBuilder()
                 .version(HttpClient.Version.HTTP_1_1)
                 .followRedirects(HttpClient.Redirect.NEVER)
-                .connectTimeout(httpConfig.connectTimeout)
+                .connectTimeout(timeout)
                 .executor(virtualThreadsExecutor)
                 .build()
     }
 
     static HttpClient newHttpClient() {
-        return INSTANCE = HttpClient.newBuilder()
+        return HttpClient.newBuilder()
                 .version(HttpClient.Version.HTTP_1_1)
                 .followRedirects(HttpClient.Redirect.NORMAL)
                 .executor(virtualThreadsExecutor)

diff --git a/src/main/groovy/io/seqera/wave/service/inspect/ContainerInspectServiceImpl.groovy b/src/main/groovy/io/seqera/wave/service/inspect/ContainerInspectServiceImpl.groovy
@@ -18,7 +18,6 @@
 
 package io.seqera.wave.service.inspect
 
-import java.net.http.HttpClient
 
 import groovy.transform.Canonical
 import groovy.transform.CompileStatic
@@ -34,11 +33,11 @@ import io.seqera.wave.core.ContainerAugmenter
 import io.seqera.wave.core.ContainerPath
 import io.seqera.wave.core.RegistryProxyService
 import io.seqera.wave.core.spec.ManifestSpec
+import io.seqera.wave.http.HttpClientFactory
 import io.seqera.wave.model.ContainerCoordinates
 import io.seqera.wave.proxy.ProxyClient
 import io.seqera.wave.util.RegHelper
 import jakarta.inject.Inject
-import jakarta.inject.Named
 import jakarta.inject.Singleton
 /**
  * Implements containers inspect service
@@ -73,9 +72,6 @@ class ContainerInspectServiceImpl implements ContainerInspectService {
     @Inject
     private RegistryProxyService proxyService
 
-    @Inject
-    @Named("never-redirects")
-    private HttpClient httpClient
 
     @Inject
     private RegistryAuthService loginService
@@ -197,6 +193,7 @@ class ContainerInspectServiceImpl implements ContainerInspectService {
 
     private ProxyClient client0(ContainerPath route, RegistryCredentials creds) {
         final registry = lookupService.lookup(route.registry)
+        final httpClient = HttpClientFactory.neverRedirectsHttpClient()
         new ProxyClient(httpClient)
                 .withRoute(route)
                 .withImage(route.image)

diff --git a/src/main/groovy/io/seqera/wave/tower/client/connector/HttpTowerConnector.groovy b/src/main/groovy/io/seqera/wave/tower/client/connector/HttpTowerConnector.groovy
@@ -27,10 +27,10 @@ import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
 import io.micronaut.http.HttpMethod
 import io.seqera.wave.configuration.HttpClientConfig
+import io.seqera.wave.http.HttpClientFactory
 import io.seqera.wave.service.pairing.socket.msg.ProxyHttpRequest
 import io.seqera.wave.service.pairing.socket.msg.ProxyHttpResponse
 import jakarta.inject.Inject
-import jakarta.inject.Named
 import jakarta.inject.Singleton
 /**
  * Implements a Tower client using a plain HTTP client
@@ -46,10 +46,12 @@ class HttpTowerConnector extends TowerConnector {
     @Inject
     private HttpClientConfig config
 
-    @Inject
-    @Named("never-redirects")
     private HttpClient client
 
+    {
+        this.client = HttpClientFactory.neverRedirectsHttpClient()
+    }
+
     @Override
     CompletableFuture<ProxyHttpResponse> sendAsync(String endpoint, ProxyHttpRequest request) {
         client

diff --git a/src/test/groovy/io/seqera/wave/ProxyClientTest.groovy b/src/test/groovy/io/seqera/wave/ProxyClientTest.groovy
@@ -22,18 +22,15 @@ import spock.lang.Requires
 import spock.lang.Shared
 import spock.lang.Specification
 
-import java.net.http.HttpClient
-
 import io.micronaut.context.ApplicationContext
 import io.micronaut.test.extensions.spock.annotation.MicronautTest
 import io.seqera.wave.auth.RegistryAuth
 import io.seqera.wave.auth.RegistryAuthService
 import io.seqera.wave.auth.RegistryCredentialsProvider
 import io.seqera.wave.auth.RegistryLookupService
+import io.seqera.wave.http.HttpClientFactory
 import io.seqera.wave.proxy.ProxyClient
 import jakarta.inject.Inject
-import jakarta.inject.Named
-
 /**
  *
  * @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@@ -48,14 +45,14 @@ class ProxyClientTest extends Specification {
     @Inject RegistryLookupService lookupService
     @Inject RegistryAuthService loginService
     @Inject RegistryCredentialsProvider credentialsProvider
-    @Inject @Named("never-redirects") HttpClient httpClient
 
     def 'should call target manifests on docker.io' () {
         given:
         def REG = 'docker.io'
         def IMAGE = 'library/hello-world'
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
+        def httpClient = HttpClientFactory.neverRedirectsHttpClient()
         and:
         def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
@@ -75,6 +72,7 @@ class ProxyClientTest extends Specification {
         def REG = 'docker.io'
         def IMAGE = 'library/hello-world'
         def registry = lookupService.lookup(REG)
+        def httpClient = HttpClientFactory.neverRedirectsHttpClient()
         and:
         def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
@@ -94,6 +92,7 @@ class ProxyClientTest extends Specification {
         def IMAGE = 'biocontainers/fastqc'
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
+        def httpClient = HttpClientFactory.neverRedirectsHttpClient()
         and:
         def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
@@ -114,6 +113,7 @@ class ProxyClientTest extends Specification {
         def IMAGE = 'biocontainers/fastqc'
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
+        def httpClient = HttpClientFactory.neverRedirectsHttpClient()
         and:
         def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
@@ -147,6 +147,7 @@ class ProxyClientTest extends Specification {
         def REG = '195996028523.dkr.ecr.eu-west-1.amazonaws.com'
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
+        def httpClient = HttpClientFactory.neverRedirectsHttpClient()
         and:
         def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
@@ -167,6 +168,7 @@ class ProxyClientTest extends Specification {
         def REG = 'public.ecr.aws'
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
+        def httpClient = HttpClientFactory.neverRedirectsHttpClient()
         and:
         def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
@@ -228,6 +230,7 @@ class ProxyClientTest extends Specification {
         def REG = 'europe-southwest1-docker.pkg.dev'
         def registry = lookupService.lookup(REG)
         def creds = credentialsProvider.getDefaultCredentials(REG)
+        def httpClient = HttpClientFactory.neverRedirectsHttpClient()
         and:
         def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)

diff --git a/src/test/groovy/io/seqera/wave/ProxyClientWithLocalRegistryTest.groovy b/src/test/groovy/io/seqera/wave/ProxyClientWithLocalRegistryTest.groovy
@@ -21,18 +21,15 @@ package io.seqera.wave
 import spock.lang.Shared
 import spock.lang.Specification
 
-import java.net.http.HttpClient
-
 import io.micronaut.context.ApplicationContext
 import io.micronaut.test.extensions.spock.annotation.MicronautTest
 import io.seqera.wave.auth.RegistryAuthService
 import io.seqera.wave.auth.RegistryCredentialsProvider
 import io.seqera.wave.auth.RegistryLookupService
+import io.seqera.wave.http.HttpClientFactory
 import io.seqera.wave.proxy.ProxyClient
 import io.seqera.wave.test.DockerRegistryContainer
 import jakarta.inject.Inject
-import jakarta.inject.Named
-
 /**
  *
  * @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@@ -47,7 +44,6 @@ class ProxyClientWithLocalRegistryTest extends Specification implements DockerRe
     @Inject RegistryLookupService lookupService
     @Inject RegistryAuthService loginService
     @Inject RegistryCredentialsProvider credentialsProvider
-    @Inject @Named("never-redirects") HttpClient httpClient
 
     def setupSpec() {
         initRegistryContainer(applicationContext)
@@ -57,6 +53,7 @@ class ProxyClientWithLocalRegistryTest extends Specification implements DockerRe
         given:
         def IMAGE = 'library/hello-world'
         and:
+        def httpClient = HttpClientFactory.neverRedirectsHttpClient()
         def proxy = new ProxyClient(httpClient)
                 .withImage(IMAGE)
                 .withRegistry(getLocalTestRegistryInfo())