Switch to using the Java HTTP client (#255)

* Switch to using the Java HTTP client

There are 2 reasons to make that change:

1. reduce the number of dependencies of the client, in order to
remove the dependency on Netty
2. the JDK HTTP client doesn't need to be closed

The 2d point deserves a bit more information. The test resources
client factory is called by the test resources client, and in
general, there should be at most a couple calls per build. In
practice, this isn't the case: in Micronaut Data for example,
there are many calls because tests spawn their own application
context.

One issue is that Micronaut Core will _not_ call `#close` on
a `PropertySourceLoader` once an application context is closed,
even if that loader implements `Closeable`. It is arguable if
it's a bug or not, but in practice, it means that there is no
way for Micronaut Test Resources to proactively close a client.

Switching to the JDK HTTP Client solves this problem because
there is no need to call the close method anymore.

This PR doesn't change the communication protocol between
the client and the server because core lacks some support
to make it easily possible. The consequence is that we're
still adding a dependency on `micronaut-json-core`, which
also means that if there's no JSON mapper on classpath
(either Jackson or Serde), then test resources won't work.
I am unsure but I think that was already the case before,
even with the dependency on HTTP Netty, unless that was
bringing a mapper on classpath transparently.

* Fix client String decoding

Author: melix

config/accepted-api-changes.json

@@ -3,5 +3,10 @@
     "type": "io.micronaut.testresources.redis.RedisTestResourceProvider",
     "member": "Field REDIS_PORT",
     "reason": "We're now using Redis URI"
+  },
+  {
+    "type": "io.micronaut.testresources.client.DefaultTestResourcesClient",
+    "member": "Constructor io.micronaut.testresources.client.DefaultTestResourcesClient(io.micronaut.http.client.HttpClient,java.lang.String)",
+    "reason": "This type is internal API"
   }
 ]

test-resources-client/build.gradle

@@ -9,7 +9,7 @@ and provide their value on demand.
 """
 
 dependencies {
-    api(mn.micronaut.http.client)
+    api(mn.micronaut.json.core)
     api(project(':micronaut-test-resources-core'))
 
     testRuntimeOnly(mn.micronaut.http.server.netty)

test-resources-client/src/main/java/io/micronaut/testresources/client/DefaultTestResourcesClient.java

@@ -15,46 +15,57 @@
  */
 package io.micronaut.testresources.client;
 
+import io.micronaut.core.annotation.Internal;
 import io.micronaut.core.annotation.Nullable;
 import io.micronaut.core.type.Argument;
-import io.micronaut.http.HttpRequest;
-import io.micronaut.http.MutableHttpRequest;
-import io.micronaut.http.client.BlockingHttpClient;
-import io.micronaut.http.client.HttpClient;
-import io.micronaut.http.uri.UriBuilder;
+import io.micronaut.json.JsonMapper;
 
+import java.io.IOException;
 import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.time.Duration;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-
-import static io.micronaut.http.HttpHeaders.ACCEPT;
-import static io.micronaut.http.HttpHeaders.USER_AGENT;
+import java.util.function.Consumer;
 
 /**
  * A simple implementation of the test resources client.
  */
 @SuppressWarnings("unchecked")
+@Internal
 public class DefaultTestResourcesClient implements TestResourcesClient {
     public static final String ACCESS_TOKEN = "Access-Token";
 
-    private static final URI RESOLVABLE_PROPERTIES_URI = UriBuilder.of("/").path("/list").build();
-    private static final URI REQUIRED_PROPERTIES_URI = UriBuilder.of("/").path("/requirements/expr").build();
-    private static final URI REQUIRED_PROPERTY_ENTRIES_URI = UriBuilder.of("/").path("/requirements/entries").build();
-    private static final URI CLOSE_ALL_URI = UriBuilder.of("/").path("/close/all").build();
-    private static final URI CLOSE_URI = UriBuilder.of("/").path("/close").build();
-    private static final URI RESOLVE_URI = UriBuilder.of("/").path("/resolve").build();
-    private static final Argument<List<String>> LIST_OF_STRING = Argument.listOf(String.class);
+    private static final String RESOLVABLE_PROPERTIES_URI = "/list";
+    private static final String REQUIRED_PROPERTIES_URI = "/requirements/expr";
+    private static final String REQUIRED_PROPERTY_ENTRIES_URI = "/requirements/entries";
+    private static final String CLOSE_ALL_URI = "/close/all";
+    private static final String CLOSE_URI = "/close";
+    private static final String RESOLVE_URI = "/resolve";
+    private static final Argument<List<String>> LIST_OF_STRING = Argument.LIST_OF_STRING;
+    private static final Argument<String> STRING = Argument.STRING;
+    private static final Argument<Boolean> BOOLEAN = Argument.BOOLEAN;
 
-    private final BlockingHttpClient client;
+    private final JsonMapper jsonMapper;
+    private final String baseUri;
+    private final HttpClient client;
 
     private final String accessToken;
+    private final Duration clientTimeout;
 
-    public DefaultTestResourcesClient(HttpClient client, String accessToken) {
-        this.client = client.toBlocking();
+    public DefaultTestResourcesClient(String baseUri, String accessToken, int clientReadTimeout) {
+        this.baseUri = baseUri;
+        clientTimeout = Duration.ofSeconds(clientReadTimeout);
+        this.client = HttpClient.newBuilder()
+            .build();
         this.accessToken = accessToken;
+        this.jsonMapper = JsonMapper.createDefault();
     }
 
     @Override
@@ -63,8 +74,9 @@ public List<String> getResolvableProperties(Map<String, Collection<String>> prop
         Map<String, Object> properties = new HashMap<>();
         properties.put("propertyEntries", propertyEntries);
         properties.put("testResourcesConfig", testResourcesConfig);
-        HttpRequest<?> req = configure(HttpRequest.POST(RESOLVABLE_PROPERTIES_URI, properties));
-        return client.retrieve(req, LIST_OF_STRING);
+        return request(RESOLVABLE_PROPERTIES_URI, LIST_OF_STRING,
+            r -> POST(r, properties)
+        );
     }
 
     @Override
@@ -73,45 +85,81 @@ public Optional<String> resolve(String name, Map<String, Object> properties, Map
         params.put("name", name);
         params.put("properties", properties);
         params.put("testResourcesConfig", testResourcesConfiguration);
-        HttpRequest<?> req = configure(HttpRequest.POST(RESOLVE_URI, params));
-        return Optional.ofNullable(client.retrieve(req));
+        return Optional.ofNullable(request(RESOLVE_URI, STRING, r -> POST(r, params)));
     }
 
     @Override
     public List<String> getRequiredProperties(String expression) {
-        return doGet(REQUIRED_PROPERTIES_URI, LIST_OF_STRING, expression);
+        return request(REQUIRED_PROPERTIES_URI + "/" + expression, LIST_OF_STRING, this::GET);
     }
 
     @Override
     public List<String> getRequiredPropertyEntries() {
-        return doGet(REQUIRED_PROPERTY_ENTRIES_URI, LIST_OF_STRING);
+        return request(REQUIRED_PROPERTY_ENTRIES_URI, LIST_OF_STRING, this::GET);
     }
 
     @Override
     public boolean closeAll() {
-        return doGet(CLOSE_ALL_URI, Argument.BOOLEAN);
+        return request(CLOSE_ALL_URI, BOOLEAN, this::GET);
     }
 
     @Override
     public boolean closeScope(@Nullable String id) {
-        return doGet(CLOSE_URI, Argument.BOOLEAN, id);
+        return request(CLOSE_URI + "/" + id, BOOLEAN, this::GET);
     }
 
-    private <T> T doGet(URI uri, Argument<T> clazz, String... pathElements) {
-        UriBuilder builder = UriBuilder.of(uri);
-        for (String param : pathElements) {
-            builder = builder.path(param);
-        }
-        MutableHttpRequest<?> req = configure(HttpRequest.GET(builder.build()));
-        return client.retrieve(req, clazz);
+    @SuppressWarnings({"java:S100", "checkstyle:MethodName"})
+    private void POST(HttpRequest.Builder request, Object o) {
+        request.POST(HttpRequest.BodyPublishers.ofByteArray(writeValueAsBytes(o)));
+    }
+
+    @SuppressWarnings({"java:S100", "checkstyle:MethodName"})
+    private void GET(HttpRequest.Builder request) {
+        request.GET();
     }
 
-    private <T> MutableHttpRequest<T> configure(MutableHttpRequest<T> request) {
-        MutableHttpRequest<T> result = request.header(USER_AGENT, "Micronaut HTTP Client")
-            .header(ACCEPT, "application/json");
+    private <T> T request(String path, Argument<T> type, Consumer<? super HttpRequest.Builder> config) {
+        var request = HttpRequest.newBuilder()
+            .uri(uri(path))
+            .timeout(clientTimeout);
+        request = request.header("User-Agent", "Micronaut Test Resources Client")
+            .header("Content-Type", "application/json")
+            .header("Accept", "application/json");
         if (accessToken != null) {
-            result = result.header(ACCESS_TOKEN, accessToken);
+            request = request.header(ACCESS_TOKEN, accessToken);
+        }
+        config.accept(request);
+        try {
+            var response = client.send(request.build(), HttpResponse.BodyHandlers.ofString());
+            var body = response.body();
+            if (response.statusCode() == 200) {
+                if (STRING.equalsType(type)) {
+                    return (T) body;
+                }
+                return jsonMapper.readValue(body, type);
+            }
+            return null;
+        } catch (IOException e) {
+            throw new TestResourcesException(e);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new TestResourcesException(e);
+        }
+    }
+
+    private URI uri(String path) {
+        try {
+            return new URI(baseUri + path);
+        } catch (URISyntaxException e) {
+            throw new TestResourcesException(e);
+        }
+    }
+
+    private byte[] writeValueAsBytes(Object o) {
+        try {
+            return jsonMapper.writeValueAsBytes(o);
+        } catch (IOException e) {
+            throw new TestResourcesException(e);
         }
-        return result;
     }
 }

test-resources-client/src/main/java/io/micronaut/testresources/client/TestResourcesClientFactory.java

@@ -16,16 +16,11 @@
 package io.micronaut.testresources.client;
 
 import io.micronaut.context.ApplicationContext;
-import io.micronaut.http.client.DefaultHttpClientConfiguration;
-import io.micronaut.http.client.HttpClient;
-import io.micronaut.http.client.HttpClientConfiguration;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.MalformedURLException;
+import java.lang.ref.WeakReference;
 import java.net.URL;
-import java.time.Duration;
-import java.time.temporal.ChronoUnit;
 import java.util.Optional;
 import java.util.Properties;
 
@@ -39,12 +34,15 @@
 public final class TestResourcesClientFactory {
     private static final String DEFAULT_TIMEOUT_SECONDS = "60";
 
+    private static WeakReference<TestResourcesClient> cachedClient;
+
     private TestResourcesClientFactory() {
 
     }
 
     /**
      * Creates a new test resources client configured via a properties file.
+     *
      * @param configFile the URL to the configuration properties file.
      * @return a new test resources client.
      */
@@ -55,44 +53,37 @@ public static TestResourcesClient configuredAt(URL configFile) {
         } catch (IOException e) {
             throw new TestResourcesException(e);
         }
-        try {
-            String serverUri = props.getProperty(TestResourcesClient.SERVER_URI);
-            String accessToken = props.getProperty(TestResourcesClient.ACCESS_TOKEN);
-            int clientReadTimeout = Integer.parseInt(props.getProperty(TestResourcesClient.CLIENT_READ_TIMEOUT, DEFAULT_TIMEOUT_SECONDS));
-            HttpClientConfiguration config = new DefaultHttpClientConfiguration();
-            config.setReadTimeout(Duration.of(clientReadTimeout, ChronoUnit.SECONDS));
-            HttpClient client = HttpClient.create(new URL(serverUri), config);
-            return new DefaultTestResourcesClient(client, accessToken);
-        } catch (MalformedURLException e) {
-            throw new TestResourcesException(e);
-        }
+        String serverUri = props.getProperty(TestResourcesClient.SERVER_URI);
+        String accessToken = props.getProperty(TestResourcesClient.ACCESS_TOKEN);
+        int clientReadTimeout = Integer.parseInt(props.getProperty(TestResourcesClient.CLIENT_READ_TIMEOUT, DEFAULT_TIMEOUT_SECONDS));
+        return new DefaultTestResourcesClient(serverUri, accessToken, clientReadTimeout);
     }
 
     /**
      * Creates a new test resources client configured via system properties.
+     *
      * @return a new test resources client, if system properties were found.
      */
     public static Optional<TestResourcesClient> fromSystemProperties() {
+        var client = cachedClient != null ? cachedClient.get() : null;
+        if (client != null) {
+            return Optional.of(client);
+        }
         String serverUri = System.getProperty(ConfigFinder.systemPropertyNameOf(TestResourcesClient.SERVER_URI));
         if (serverUri != null) {
             String accessToken = System.getProperty(ConfigFinder.systemPropertyNameOf(TestResourcesClient.ACCESS_TOKEN));
             String clientTimeoutString = System.getProperty(ConfigFinder.systemPropertyNameOf(TestResourcesClient.CLIENT_READ_TIMEOUT), DEFAULT_TIMEOUT_SECONDS);
             int clientReadTimeout = Integer.parseInt(clientTimeoutString);
-            HttpClientConfiguration config = new DefaultHttpClientConfiguration();
-            config.setReadTimeout(Duration.of(clientReadTimeout, ChronoUnit.SECONDS));
-            HttpClient client;
-            try {
-                client = HttpClient.create(new URL(serverUri), config);
-            } catch (MalformedURLException e) {
-                return Optional.empty();
-            }
-            return Optional.of(new DefaultTestResourcesClient(client, accessToken));
+            client = new DefaultTestResourcesClient(serverUri, accessToken, clientReadTimeout);
+            cachedClient = new WeakReference<>(client);
+            return Optional.of(client);
         }
         return Optional.empty();
     }
 
     /**
      * Extracts the {@link TestResourcesClient} from the given {@link ApplicationContext}.
+     *
      * @param context the application context
      * @return the test resources client
      */

test-resources-client/src/main/java/io/micronaut/testresources/client/TestResourcesClientPropertyExpressionResolver.java

@@ -20,7 +20,6 @@
 import io.micronaut.core.annotation.Nullable;
 import io.micronaut.core.convert.ConversionService;
 import io.micronaut.core.value.PropertyResolver;
-import io.micronaut.http.client.exceptions.HttpClientResponseException;
 import io.micronaut.testresources.core.LazyTestResourcesExpressionResolver;
 import io.micronaut.testresources.core.TestResourcesResolver;
 import org.slf4j.Logger;
@@ -116,12 +115,7 @@ public <T> Optional<T> resolve(PropertyResolver propertyResolver,
         }
 
         private static Optional<String> callClient(String expression, TestResourcesClient client, Map<String, Object> props, Map<String, Object> properties) {
-            try {
-                return client.resolve(expression, props, properties);
-            } catch (HttpClientResponseException ex) {
-                LOGGER.debug("Test resources client failed to resolve expression '{}'", expression, ex);
-                return Optional.empty();
-            }
+            return client.resolve(expression, props, properties);
         }
 
         @Override

test-resources-client/src/main/java/io/micronaut/testresources/client/TestResourcesException.java

@@ -22,4 +22,8 @@ public class TestResourcesException extends RuntimeException {
     public TestResourcesException(Throwable cause) {
         super(cause);
     }
+
+    public TestResourcesException(String message) {
+        super(message);
+    }
 }

test-resources-client/src/test/groovy/io/micronaut/testresources/client/TestServer.groovy

@@ -8,7 +8,7 @@ import io.micronaut.testresources.core.TestResourcesResolver
 
 @Controller("/")
 @Requires(property = 'server', notEquals = 'false')
- class TestServer implements TestResourcesResolver {
+class TestServer implements TestResourcesResolver {
 
     @Override
     @Post("/list")

test-resources-server/build.gradle

@@ -24,6 +24,7 @@ dependencies {
     runtimeOnly(mnSerde.micronaut.serde.jackson)
     runtimeOnly(mn.snakeyaml)
 
+    testImplementation(mn.micronaut.http.client)
     testImplementation(project(':micronaut-test-resources-client'))
     testRuntimeOnly(project(':micronaut-test-resources-kafka'))