Review issues 2

webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java

@@ -24,6 +24,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
@@ -56,6 +57,7 @@ public abstract class ClientRequestBase<T extends ClientRequest<T>, R extends Ht
     public static final Http.Header USER_AGENT_HEADER = Http.Headers.create(Http.HeaderNames.USER_AGENT,
                                                                             "Helidon " + Version.VERSION);
     private static final Map<String, AtomicLong> COUNTERS = new ConcurrentHashMap<>();
+    private static final Set<String> SUPPORTED_SCHEMES = Set.of("https", "http");
 
     private final Map<String, String> pathParams = new HashMap<>();
     private final HttpClientConfig clientConfig;
@@ -145,7 +147,7 @@ public T uri(String uri) {
     @Override
     public ClientUri resolvedUri() {
         // we do not want to update our own URI, as this method may be called multiple times
-        return resolveUri(this.clientUri.copy());
+        return resolveUri(ClientUri.create(this.clientUri));
     }
 
     @Override
@@ -249,7 +251,7 @@ public T proxy(Proxy proxy) {
     @Override
     public R request() {
         headers.setIfAbsent(USER_AGENT_HEADER);
-        return doSubmit(BufferData.EMPTY_BYTES);
+        return validateAndSubmit(BufferData.EMPTY_BYTES);
     }
 
     @Override
@@ -258,7 +260,7 @@ public final R submit(Object entity) {
             rejectHeadWithEntity();
         }
         headers.setIfAbsent(USER_AGENT_HEADER);
-        return doSubmit(entity);
+        return validateAndSubmit(entity);
     }
 
     @Override
@@ -435,6 +437,18 @@ private static String nextRequestId(String protocolId) {
         return "client-" + protocolId + "-" + Long.toHexString(counter.getAndIncrement());
     }
 
+    private R validateAndSubmit(Object entity) {
+        if (!SUPPORTED_SCHEMES.contains(uri().scheme())) {
+            throw new IllegalArgumentException(
+                    String.format("Not supported scheme %s, client supported schemes are: %s",
+                                  uri().scheme(),
+                                  String.join(", ", SUPPORTED_SCHEMES)
+                    )
+            );
+        }
+        return doSubmit(entity);
+    }
+
     private String resolvePathParams(String path) {
         String result = path;
         for (Map.Entry<String, String> entry : pathParams.entrySet()) {

webclient/api/src/main/java/io/helidon/webclient/api/ClientUri.java

@@ -17,7 +17,6 @@
 package io.helidon.webclient.api;
 
 import java.net.URI;
-import java.util.Set;
 
 import io.helidon.common.uri.UriFragment;
 import io.helidon.common.uri.UriInfo;
@@ -29,34 +28,30 @@
  * URI abstraction for WebClient.
  */
 public class ClientUri implements UriInfo {
-    private static final Set<String> DEFAULT_SUPPORTED_SCHEMES = Set.of("http", "https");
     private final UriInfo base;
     private final UriQueryWriteable query;
 
     private UriInfo.Builder uriBuilder;
     private boolean skipUriEncoding = false;
 
-    protected ClientUri() {
+    private ClientUri() {
         this.base = null;
         this.query = UriQueryWriteable.create();
         this.uriBuilder = UriInfo.builder();
     }
 
-    protected ClientUri(ClientUri baseUri) {
-        validateScheme(baseUri.scheme());
+    private ClientUri(ClientUri baseUri) {
         this.base = baseUri;
         this.uriBuilder = UriInfo.builder(base);
         this.skipUriEncoding = baseUri.skipUriEncoding;
         this.query = UriQueryWriteable.create().from(baseUri.query());
-        validateScheme(baseUri.scheme());
     }
 
-    protected ClientUri(UriInfo baseUri) {
+    private ClientUri(UriInfo baseUri) {
         this.base = baseUri;
         this.uriBuilder = UriInfo.builder(baseUri);
         this.skipUriEncoding = false;
         this.query = UriQueryWriteable.create().from(baseUri.query());
-        validateScheme(baseUri.scheme());
     }
 
     /**
@@ -98,15 +93,6 @@ public static ClientUri create(URI baseUri) {
         return create().resolve(baseUri);
     }
 
-    /**
-     * Create copy of this client uri.
-     *
-     * @return copy of this client uri
-     */
-    public ClientUri copy() {
-        return ClientUri.create(this);
-    }
-
     @Override
     public String toString() {
         UriInfo info = uriBuilder.query(this.query).build();
@@ -135,7 +121,6 @@ public URI toUri() {
      * @return updated instance
      */
     public ClientUri scheme(String scheme) {
-        validateScheme(scheme);
         uriBuilder.scheme(scheme);
         return this;
     }
@@ -197,7 +182,6 @@ public ClientUri resolve(URI uri) {
         }
 
         if (uri.getScheme() != null) {
-            validateScheme(uri.getScheme());
             uriBuilder.scheme(uri.getScheme());
         }
         if (uri.getHost() != null) {
@@ -337,10 +321,6 @@ public String pathWithQueryAndFragment() {
         return path;
     }
 
-    protected Set<String> supportedSchemes() {
-        return DEFAULT_SUPPORTED_SCHEMES;
-    }
-
     private String extractQuery(String path) {
         if (path != null) {
             int i = path.indexOf('?');
@@ -379,15 +359,4 @@ private String resolvePath(String path, String resolvePath) {
                 + "/"
                 + resolvePath;
     }
-
-    private void validateScheme(String scheme) {
-        if (!supportedSchemes().contains(scheme)) {
-            throw new IllegalArgumentException(
-                    String.format("Not supported scheme %s, client supported schemes are: %s",
-                                  scheme,
-                                  String.join(", ", supportedSchemes())
-                    )
-            );
-        }
-    }
 }

webclient/api/src/test/java/io/helidon/webclient/api/ClientUriTest.java

@@ -24,7 +24,6 @@
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class ClientUriTest {
     @Test
@@ -105,12 +104,4 @@ void testResolveAll() {
         assertThat(helper.port(), is(80));
         assertThat(helper.scheme(), is("https"));
     }
-
-    @Test
-    void testSchemeValidation() {
-        ClientUri validClientUri = ClientUri.create(URI.create("http://localhost"));
-        assertThrows(IllegalArgumentException.class, () -> ClientUri.create(URI.create("ftp://localhost")));
-        assertThrows(IllegalArgumentException.class, () -> validClientUri.scheme("git"));
-        assertThrows(IllegalArgumentException.class, () -> validClientUri.resolve(URI.create("ldap://localhost")));
-    }
 }

webclient/http1/src/main/java/io/helidon/webclient/http1/Http1ClientImpl.java

@@ -43,7 +43,7 @@ class Http1ClientImpl implements Http1Client, HttpClientSpi {
     @Override
     public Http1ClientRequest method(Http.Method method) {
         ClientUri clientUri = clientConfig.baseUri()
-                .map(ClientUri::copy) // create from base config
+                .map(ClientUri::create) // create from base config
                 .orElseGet(ClientUri::create); // create as empty
 
         clientConfig.baseFragment().ifPresent(clientUri::fragment);

webclient/tests/http1/src/test/java/io/helidon/webclient/http1/Http1ClientTest.java

@@ -36,6 +36,7 @@
 import io.helidon.http.media.EntityWriter;
 import io.helidon.http.media.MediaContext;
 import io.helidon.http.media.MediaContextConfig;
+import io.helidon.webclient.http2.Http2Client;
 import io.helidon.webserver.testing.junit5.ServerTest;
 import io.helidon.webserver.testing.junit5.SetUpRoute;
 import io.helidon.webclient.api.ClientConnection;
@@ -56,9 +57,11 @@
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.IsNot.not;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /*
 This class uses package local API to validate connection cache, and at the same time benefits from @ServerTest
@@ -76,9 +79,11 @@ class Http1ClientTest {
 
     private final String baseURI;
     private final WebClient injectedHttp1client;
+    private final int plainPort;
 
     Http1ClientTest(WebServer webServer, WebClient client) {
         baseURI = "http://localhost:" + webServer.port();
+        this.plainPort = webServer.port();
         injectedHttp1client = client;
     }
 
@@ -376,6 +381,22 @@ void testReadTimeoutPerRequest() {
         assertThat(ste.getCause(), instanceOf(SocketTimeoutException.class));
     }
 
+    @Test
+    void testSchemeValidation() {
+        try (var r = Http1Client.builder()
+                .baseUri("test://localhost:" + plainPort + "/")
+                .shareConnectionCache(false)
+                .build()
+                .get("/")
+                .request()) {
+
+            fail("Should have failed because of invalid scheme.");
+        } catch (IllegalArgumentException e) {
+            assertThat(e.getMessage(), startsWith("Not supported scheme test"));
+        }
+    }
+
+
     private static void validateSuccessfulResponse(Http1Client client) {
         String requestEntity = "Sending Something";
         Http1ClientRequest request = client.put("/test");

webclient/websocket/src/main/java/io/helidon/webclient/websocket/WsClientConfigBlueprint.java

@@ -29,7 +29,6 @@
  *         io.helidon.webclient.spi.ProtocolConfig)
  */
 @Prototype.Blueprint
-@Prototype.CustomMethods(WsClientConfigSupport.WsCustomMethods.class)
 @Configured
 interface WsClientConfigBlueprint extends HttpClientConfig, Prototype.Factory<WsClient> {
     /**

webclient/websocket/src/main/java/io/helidon/webclient/websocket/WsClientImpl.java

@@ -23,6 +23,7 @@
 import java.security.SecureRandom;
 import java.util.Base64;
 import java.util.Random;
+import java.util.Set;
 
 import io.helidon.common.LazyValue;
 import io.helidon.common.socket.SocketContext;
@@ -31,6 +32,7 @@
 import io.helidon.http.ClientResponseHeaders;
 import io.helidon.http.Http;
 import io.helidon.webclient.api.ClientConnection;
+import io.helidon.webclient.api.ClientUri;
 import io.helidon.webclient.api.HttpClientResponse;
 import io.helidon.webclient.api.WebClient;
 import io.helidon.webclient.http1.Http1Client;
@@ -56,7 +58,7 @@ class WsClientImpl implements WsClient {
     private static final byte[] KEY_SUFFIX = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11".getBytes(StandardCharsets.US_ASCII);
     private static final int KEY_SUFFIX_LENGTH = KEY_SUFFIX.length;
     private static final Base64.Encoder B64_ENCODER = Base64.getEncoder();
-
+    private static final Set<String> SUPPORTED_SCHEMES = Set.of("wss", "ws", "https", "http");
     private final ClientRequestHeaders headers;
     private final WebClient webClient;
     private final Http1Client http1Client;
@@ -93,10 +95,20 @@ public void connect(URI uri, WsListener listener) {
                 .uri(uri);
         UriInfo resolvedUri = upgradeRequest.resolvedUri();
         String scheme = resolvedUri.scheme();
+
+        if (!SUPPORTED_SCHEMES.contains(scheme)) {
+            throw new IllegalArgumentException(
+                    String.format("Not supported scheme %s, web socket client supported schemes are: %s",
+                                  scheme,
+                                  String.join(", ", SUPPORTED_SCHEMES)
+                    )
+            );
+        }
+
         if ("ws".equals(scheme)) {
-            upgradeRequest.uri(WsClientUri.create(resolvedUri).scheme("http"));
+            upgradeRequest.uri(ClientUri.create(resolvedUri).scheme("http"));
         } else if ("wss".equals(scheme)) {
-            upgradeRequest.uri(WsClientUri.create(resolvedUri).scheme("https"));
+            upgradeRequest.uri(ClientUri.create(resolvedUri).scheme("https"));
         }
 
         upgradeRequest.headers(headers -> headers.setIfAbsent(Http.Headers.create(Http.HeaderNames.HOST, resolvedUri
@@ -166,4 +178,5 @@ protected String hash(SocketContext ctx, String wsKey) {
             throw new IllegalStateException("SHA-1 not provided", e);
         }
     }
+
 }