4.x: Helidon connector configuration and redirects (helidon-io#7169)

* - Support for entity in redirects. Helidon connector needs to do some buffering. - Support for configuration of WebClient created by Helidon connector as in previous versions. * - Ensure that default property values only override config if a property is available - New test for Config - Re-enabled some tests * Update ConfigTest.java * Update ClientRequestImpl.java Add `@Override` --------- Co-authored-by: Romain Grecourt <romain.grecourt@oracle.com>
tvallin · Jul 11, 2023 · 4559600 · 4559600
1 parent aa16934
commit 4559600
Show file tree

Hide file tree

Showing 9 changed files with 233 additions and 18 deletions.
diff --git a/jersey/connector/pom.xml b/jersey/connector/pom.xml
@@ -70,6 +70,11 @@
             <artifactId>hamcrest-all</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.config</groupId>
+            <artifactId>helidon-config-yaml</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

diff --git a/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java b/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java
@@ -16,19 +16,25 @@
 
 package io.helidon.jersey.connector;
 
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.net.URI;
 import java.time.Duration;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.logging.Logger;
 
 import javax.net.ssl.SSLContext;
 
 import io.helidon.common.LazyValue;
 import io.helidon.common.Version;
 import io.helidon.common.http.Http;
 import io.helidon.common.uri.UriQueryWriteable;
+import io.helidon.config.Config;
 import io.helidon.nima.common.tls.Tls;
 import io.helidon.nima.http.media.ReadableEntity;
 import io.helidon.nima.webclient.WebClient;
@@ -37,17 +43,27 @@
 import io.helidon.nima.webclient.http1.Http1ClientRequest;
 import io.helidon.nima.webclient.http1.Http1ClientResponse;
 
+import jakarta.ws.rs.ProcessingException;
 import jakarta.ws.rs.client.Client;
 import jakarta.ws.rs.core.Configuration;
 import jakarta.ws.rs.core.Response;
-import org.glassfish.jersey.client.ClientProperties;
 import org.glassfish.jersey.client.ClientRequest;
 import org.glassfish.jersey.client.ClientResponse;
 import org.glassfish.jersey.client.spi.AsyncConnectorCallback;
 import org.glassfish.jersey.client.spi.Connector;
 import org.glassfish.jersey.internal.util.PropertiesHelper;
 
+import static org.glassfish.jersey.client.ClientProperties.CONNECT_TIMEOUT;
+import static org.glassfish.jersey.client.ClientProperties.FOLLOW_REDIRECTS;
+import static org.glassfish.jersey.client.ClientProperties.OUTBOUND_CONTENT_LENGTH_BUFFER;
+import static org.glassfish.jersey.client.ClientProperties.READ_TIMEOUT;
+import static org.glassfish.jersey.client.ClientProperties.getValue;
+
 class HelidonConnector implements Connector {
+    static final Logger LOGGER = Logger.getLogger(HelidonConnector.class.getName());
+
+    private static final int DEFAULT_TIMEOUT = 10000;
+
     private static final String HELIDON_VERSION = "Helidon/" + Version.VERSION + " (java "
             + PropertiesHelper.getSystemProperty("java.runtime.version") + ")";
 
@@ -60,15 +76,25 @@ class HelidonConnector implements Connector {
 
     HelidonConnector(Client client, Configuration config) {
         this.client = client;
+
+        // create underlying HTTP client
         Map<String, Object> properties = config.getProperties();
-        httpClient = WebClient.builder(Http1.PROTOCOL)
-                .connectTimeout(Duration.ofMillis(
-                        ClientProperties.getValue(properties, ClientProperties.CONNECT_TIMEOUT, 10000)))
-                .readTimeout(Duration.ofMillis(
-                        ClientProperties.getValue(properties, ClientProperties.READ_TIMEOUT, 10000)))
-                .followRedirect(
-                        ClientProperties.getValue(properties, ClientProperties.FOLLOW_REDIRECTS, true))
-                .build();
+        Http1Client.Http1ClientBuilder builder = WebClient.builder(Http1.PROTOCOL);
+
+        // use config for client
+        builder.config(helidonConfig(config).orElse(Config.empty()));
+
+        // possibly override config with properties
+        if (properties.containsKey(CONNECT_TIMEOUT)) {
+            builder.connectTimeout(Duration.ofMillis(getValue(properties, CONNECT_TIMEOUT, DEFAULT_TIMEOUT)));
+        }
+        if (properties.containsKey(READ_TIMEOUT)) {
+            builder.readTimeout(Duration.ofMillis(getValue(properties, READ_TIMEOUT, DEFAULT_TIMEOUT)));
+        }
+        if (properties.containsKey(FOLLOW_REDIRECTS)) {
+            builder.followRedirect(getValue(properties, FOLLOW_REDIRECTS, true));
+        }
+        httpClient = builder.build();
     }
 
     /**
@@ -104,8 +130,13 @@ private Http1ClientRequest mapRequest(ClientRequest request) {
         SSLContext sslContext = client.getSslContext();
         httpRequest.tls(Tls.builder().sslContext(sslContext).build());
 
-        // redirects
-        httpRequest.followRedirects(request.resolveProperty(ClientProperties.FOLLOW_REDIRECTS, true));
+        // request config
+        if (request.hasProperty(FOLLOW_REDIRECTS)) {
+            httpRequest.followRedirects(request.resolveProperty(FOLLOW_REDIRECTS, true));
+        }
+        if (request.hasProperty(READ_TIMEOUT)) {
+            httpRequest.readTimeout(Duration.ofMillis(request.resolveProperty(READ_TIMEOUT, DEFAULT_TIMEOUT)));
+        }
 
         // copy properties
         for (String name : request.getConfiguration().getPropertyNames()) {
@@ -179,10 +210,22 @@ public ClientResponse apply(ClientRequest request) {
         Http1ClientRequest httpRequest = mapRequest(request);
 
         if (request.hasEntity()) {
-            httpResponse = httpRequest.outputStream(os -> {
-                request.setStreamProvider(length -> os);
-                request.writeEntity();      // ask Jersey to write entity to WebClient stream
-            });
+            // if following redirects we need to buffer entity for WebClient
+            if (httpRequest.followRedirects()) {
+                int bufferSize = request.resolveProperty(OUTBOUND_CONTENT_LENGTH_BUFFER, 8 * 1024);
+                try (ByteArrayOutputStream baos = new ByteArrayOutputStream(bufferSize)) {
+                    request.setStreamProvider(contentLength -> baos);
+                    ((ProcessingRunnable) request::writeEntity).run();
+                    httpResponse = httpRequest.submit(baos.toByteArray());
+                } catch (IOException e) {
+                    throw new UncheckedIOException(e);
+                }
+            } else {
+                httpResponse = httpRequest.outputStream(os -> {
+                    request.setStreamProvider(length -> os);
+                    request.writeEntity();
+                });
+            }
         } else {
             httpResponse = httpRequest.request();
         }
@@ -216,4 +259,41 @@ public String getName() {
     @Override
     public void close() {
     }
+
+    Http1Client client() {
+        return httpClient;
+    }
+
+    /**
+     * Returns the Helidon Connector configuration, if available.
+     *
+     * @param configuration from Jakarta REST
+     * @return an optional config
+     */
+    static Optional<Config> helidonConfig(Configuration configuration) {
+        Object helidonConfig = configuration.getProperty(HelidonProperties.CONFIG);
+        if (helidonConfig != null) {
+            if (!(helidonConfig instanceof Config)) {
+                LOGGER.warning(String.format("Ignoring Helidon Connector config at '%s'",
+                        HelidonProperties.CONFIG));
+            } else {
+                return Optional.of((Config) helidonConfig);
+            }
+        }
+        return Optional.empty();
+    }
+
+    @FunctionalInterface
+    private interface ProcessingRunnable extends Runnable {
+        void runOrThrow() throws IOException;
+
+        @Override
+        default void run() {
+            try {
+                runOrThrow();
+            } catch (IOException e) {
+                throw new ProcessingException("Error writing entity:", e);
+            }
+        }
+    }
 }
diff --git a/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonProperties.java b/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonProperties.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2023 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.jersey.connector;
+
+import io.helidon.config.Config;
+import io.helidon.nima.webclient.WebClient;
+
+/**
+ * Configuration options specific to the Client API that utilizes {@link HelidonConnector}.
+ */
+public final class HelidonProperties {
+
+    private HelidonProperties() {
+    }
+
+    /**
+     * A Helidon {@link Config} instance used to create the corresponding {@link WebClient}.
+     * This property is settable on {@link jakarta.ws.rs.core.Configurable#property(String, Object)}.
+     */
+    public static final String CONFIG = "jersey.connector.helidon.config";
+}
diff --git a/jersey/connector/src/main/java/module-info.java b/jersey/connector/src/main/java/module-info.java
@@ -27,6 +27,7 @@
     requires jersey.client;
     requires jersey.common;
     requires io.helidon.nima.webclient;
+    requires io.helidon.config;
 
     exports io.helidon.jersey.connector;
     provides org.glassfish.jersey.client.spi.ConnectorProvider with HelidonConnectorProvider;

diff --git a/jersey/connector/src/test/java/io/helidon/jersey/connector/ConfigTest.java b/jersey/connector/src/test/java/io/helidon/jersey/connector/ConfigTest.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2023 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.jersey.connector;
+
+import io.helidon.config.Config;
+import io.helidon.config.ConfigSources;
+import io.helidon.nima.webclient.http1.Http1ClientRequest;
+import jakarta.ws.rs.client.Client;
+import jakarta.ws.rs.client.ClientBuilder;
+import org.glassfish.jersey.client.ClientProperties;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.CoreMatchers.is;
+
+/**
+ * Tests {@link HelidonConnector} configuration.
+ */
+class ConfigTest {
+
+    private static Config config;
+
+    @BeforeAll
+    static void init() {
+        config = Config.builder(
+                        () -> ConfigSources.classpath("application.yaml").build())
+                .disableEnvironmentVariablesSource()
+                .disableSystemPropertiesSource()
+                .build();
+    }
+
+    @Test
+    void testConfig() {
+        Client client = ClientBuilder.newBuilder()
+                .property(HelidonProperties.CONFIG, config.get("client"))
+                .build();
+        HelidonConnector connector = new HelidonConnector(client, client.getConfiguration());
+        Http1ClientRequest request = connector.client().get();
+        assertThat(request.followRedirects(), is(true));
+    }
+
+    @Test
+    void testConfigPropertyOverride() {
+        Client client = ClientBuilder.newBuilder()
+                .property(HelidonProperties.CONFIG, config.get("client"))
+                .property(ClientProperties.FOLLOW_REDIRECTS, false)     // override
+                .build();
+        HelidonConnector connector = new HelidonConnector(client, client.getConfiguration());
+        Http1ClientRequest request = connector.client().get();
+        assertThat(request.followRedirects(), is(false));
+    }
+}
diff --git a/jersey/connector/src/test/java/io/helidon/jersey/connector/TimeoutTest.java b/jersey/connector/src/test/java/io/helidon/jersey/connector/TimeoutTest.java
@@ -78,15 +78,13 @@ public static void setup() {
     }
 
     @Test
-    @Disabled
     public void testFast() {
         Response r = target("test").request().get();
         assertThat(r.getStatus(), is(200));
         assertThat(r.readEntity(String.class), is("GET"));
     }
 
     @Test
-    @Disabled
     public void testSlow() {
         try {
             target("test/timeout").property(ClientProperties.READ_TIMEOUT, 1_000).request().get();
@@ -97,7 +95,6 @@ public void testSlow() {
     }
 
     @Test
-    @Disabled
     public void testTimeoutInRequest() {
         try {
             target("test/timeout").request().property(ClientProperties.READ_TIMEOUT, 1_000).get();

diff --git a/jersey/connector/src/test/resources/application.yaml b/jersey/connector/src/test/resources/application.yaml
@@ -0,0 +1,18 @@
+#
+# Copyright (c) 2023 Oracle and/or its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+client:
+  follow-redirects: true
diff --git a/.../webclient/webclient/src/main/java/io/helidon/nima/webclient/http1/ClientRequestImpl.java b/.../webclient/webclient/src/main/java/io/helidon/nima/webclient/http1/ClientRequestImpl.java
@@ -283,6 +283,11 @@ boolean keepAlive() {
         return keepAlive;
     }
 
+    @Override
+    public boolean followRedirects() {
+        return followRedirects;
+    }
+
     @Override
     public ClientRequestHeaders headers() {
         return ClientRequestHeaders.create(explicitHeaders);

diff --git a/...webclient/webclient/src/main/java/io/helidon/nima/webclient/http1/Http1ClientRequest.java b/...webclient/webclient/src/main/java/io/helidon/nima/webclient/http1/Http1ClientRequest.java
@@ -56,4 +56,11 @@ public interface Http1ClientRequest extends ClientRequest<Http1ClientRequest, Ht
      * @return {@link UriQuery}
      */
     UriQuery uriQuery();
+
+    /**
+     * Whether to follow redirects or not.
+     *
+     * @return {@code true} if redirects are followed
+     */
+    boolean followRedirects();
 }