From b272981baa26c48a26ea857ca8a2153388b5c9f2 Mon Sep 17 00:00:00 2001 From: Santiago Pericasgeertsen Date: Fri, 15 Sep 2023 11:28:13 -0400 Subject: [PATCH] - New test for protocol upgrade without TLS - Fixes problem in connector when accessing request properties - Renames test classes for clarity --- .../jersey/connector/HelidonConnector.java | 30 +++++++---- .../jersey/connector/HelidonProperties.java | 28 +++++++---- ...yConnectorBase.java => ConnectorBase.java} | 18 ++++--- ...Http1Test.java => ConnectorHttp1Test.java} | 4 +- ...nTest.java => ConnectorHttp2AlpnTest.java} | 8 ++- ...Test.java => ConnectorHttp2PriorTest.java} | 14 ++---- .../connector/ConnectorHttp2UpgradeTest.java | 50 +++++++++++++++++++ 7 files changed, 108 insertions(+), 44 deletions(-) rename jersey/tests/connector/src/test/java/io/helidon/jersey/connector/{JerseyConnectorBase.java => ConnectorBase.java} (88%) rename jersey/tests/connector/src/test/java/io/helidon/jersey/connector/{JerseyConnectorHttp1Test.java => ConnectorHttp1Test.java} (91%) rename jersey/tests/connector/src/test/java/io/helidon/jersey/connector/{JerseyConnectorHttp2AlpnTest.java => ConnectorHttp2AlpnTest.java} (92%) rename jersey/tests/connector/src/test/java/io/helidon/jersey/connector/{JerseyConnectorHttp2PriorTest.java => ConnectorHttp2PriorTest.java} (81%) create mode 100644 jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2UpgradeTest.java 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 index f97e17e2fa7..61f60263272 100644 --- a/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java +++ b/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonConnector.java @@ -51,8 +51,9 @@ import org.glassfish.jersey.client.spi.Connector; import org.glassfish.jersey.internal.util.PropertiesHelper; -import static io.helidon.jersey.connector.HelidonProperties.PROTOCOL_CONFIG; -import static io.helidon.jersey.connector.HelidonProperties.PROTOCOL_PREFERENCE; +import static io.helidon.jersey.connector.HelidonProperties.DEFAULT_HEADERS; +import static io.helidon.jersey.connector.HelidonProperties.PROTOCOL_CONFIGS; +import static io.helidon.jersey.connector.HelidonProperties.PROTOCOL_ID; import static io.helidon.jersey.connector.HelidonProperties.TLS; import static org.glassfish.jersey.client.ClientProperties.CONNECT_TIMEOUT; import static org.glassfish.jersey.client.ClientProperties.FOLLOW_REDIRECTS; @@ -63,6 +64,7 @@ class HelidonConnector implements Connector { static final Logger LOGGER = Logger.getLogger(HelidonConnector.class.getName()); private static final int DEFAULT_TIMEOUT = 10000; + private static final Map EMPTY_MAP_LIST = Map.of("", ""); private static final String HELIDON_VERSION = "Helidon/" + Version.VERSION + " (java " + PropertiesHelper.getSystemProperty("java.runtime.version") + ")"; @@ -103,16 +105,16 @@ class HelidonConnector implements Connector { builder.tls(getValue(properties, TLS, Tls.class)); hasTls = true; } - if (properties.containsKey(PROTOCOL_PREFERENCE)) { - builder.addProtocolPreference(getValue(properties, PROTOCOL_PREFERENCE, List.of(""))); - } - if (properties.containsKey(PROTOCOL_CONFIG)) { + if (properties.containsKey(PROTOCOL_CONFIGS)) { List protocolConfigs = - (List) properties.get(PROTOCOL_CONFIG); + (List) properties.get(PROTOCOL_CONFIGS); if (protocolConfigs != null) { builder.addProtocolConfigs(protocolConfigs); } } + if (properties.containsKey(DEFAULT_HEADERS)) { + builder.defaultHeadersMap(getValue(properties, DEFAULT_HEADERS, EMPTY_MAP_LIST)); + } webClient = builder.build(); } @@ -157,11 +159,17 @@ private HttpClientRequest mapRequest(ClientRequest request) { } // request config - if (request.hasProperty(FOLLOW_REDIRECTS)) { - httpRequest.followRedirects(request.resolveProperty(FOLLOW_REDIRECTS, true)); + Boolean followRedirects = request.resolveProperty(FOLLOW_REDIRECTS, Boolean.class); + if (followRedirects != null) { + httpRequest.followRedirects(followRedirects); + } + Duration readTimeout = request.resolveProperty(READ_TIMEOUT, Duration.class); + if (readTimeout != null) { + httpRequest.readTimeout(readTimeout); } - if (request.hasProperty(READ_TIMEOUT)) { - httpRequest.readTimeout(Duration.ofMillis(request.resolveProperty(READ_TIMEOUT, DEFAULT_TIMEOUT))); + String protocolId = request.resolveProperty(PROTOCOL_ID, String.class); + if (protocolId != null) { + httpRequest.protocolId(protocolId); } // copy properties 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 index d8faa928a9d..ef6eec4c86b 100644 --- a/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonProperties.java +++ b/jersey/connector/src/main/java/io/helidon/jersey/connector/HelidonProperties.java @@ -17,6 +17,7 @@ package io.helidon.jersey.connector; import java.util.List; +import java.util.Map; import io.helidon.common.tls.Tls; import io.helidon.config.Config; @@ -46,15 +47,6 @@ private HelidonProperties() { */ public static final String TLS = "jersey.connector.helidon.tls"; - /** - * Property name to set a {@code List} instance with a list of protocol preferences - * to be used by underlying {@link WebClient}. - * This property is settable on {@link jakarta.ws.rs.core.Configurable#property(String, Object)}. - * - * @see io.helidon.webclient.api.WebClientConfig.Builder#addProtocolPreference(List) - */ - public static final String PROTOCOL_PREFERENCE = "jersey.connector.helidon.protocolPreference"; - /** * Property name to set a {@code List} instance with a list of * protocol configs to be used by underlying {@link WebClient}. @@ -62,6 +54,22 @@ private HelidonProperties() { * * @see io.helidon.webclient.api.WebClientConfig.Builder#protocolConfigs(List) */ - public static final String PROTOCOL_CONFIG = "jersey.connector.helidon.protocolConfig"; + public static final String PROTOCOL_CONFIGS = "jersey.connector.helidon.protocolConfigs"; + /** + * Property name to set a {@code Map} instance with a list of + * default headers to be used by underlying {@link WebClient}. + * This property is settable on {@link jakarta.ws.rs.core.Configurable#property(String, Object)}. + * + * @see io.helidon.webclient.api.WebClientConfig.Builder#defaultHeadersMap(Map) + */ + public static final String DEFAULT_HEADERS = "jersey.connector.helidon.defaultHeaders"; + + /** + * Property name to set a protocol ID for each request. You can use this property + * to request an HTTP/2 upgrade from HTTP/1.1 by setting its value to {@code "h2"}. + * + * @see io.helidon.webclient.api.HttpClientRequest#protocolId(String) + */ + public static final String PROTOCOL_ID = "jersey.connector.helidon.protocolId"; } diff --git a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorBase.java b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java similarity index 88% rename from jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorBase.java rename to jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java index 8df4d80a081..0281ae8204f 100644 --- a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorBase.java +++ b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorBase.java @@ -19,6 +19,7 @@ import java.util.Arrays; import io.helidon.http.Http; +import io.helidon.webclient.http2.Http2Client; import io.helidon.webserver.http.HttpRules; import io.helidon.webserver.http.ServerRequest; import io.helidon.webserver.http.ServerResponse; @@ -36,21 +37,26 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasKey; -class JerseyConnectorBase { +class ConnectorBase { protected String baseURI; protected Client client; + protected String protocolId; @SetUpRoute static void routing(HttpRules rules) { - rules.get("/basic/get", JerseyConnectorBase::basicGet) - .post("/basic/post", JerseyConnectorBase::basicPost) - .get("/basic/getquery", JerseyConnectorBase::basicGetQuery) - .get("/basic/headers", JerseyConnectorBase::basicHeaders); + rules.get("/basic/get", ConnectorBase::basicGet) + .post("/basic/post", ConnectorBase::basicPost) + .get("/basic/getquery", ConnectorBase::basicGetQuery) + .get("/basic/headers", ConnectorBase::basicHeaders); } private WebTarget target(String uri) { - return client.target(baseURI).path(uri); + WebTarget webTarget = client.target(baseURI).path(uri); + if (protocolId != null) { + webTarget.property(HelidonProperties.PROTOCOL_ID, Http2Client.PROTOCOL_ID); + } + return webTarget; } static void basicGet(ServerRequest request, ServerResponse response) { diff --git a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp1Test.java b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp1Test.java similarity index 91% rename from jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp1Test.java rename to jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp1Test.java index 025ceb8e8dc..26686bac4f5 100644 --- a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp1Test.java +++ b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp1Test.java @@ -26,9 +26,9 @@ * WebClient to execute HTTP requests. */ @ServerTest -class JerseyConnectorHttp1Test extends JerseyConnectorBase { +class ConnectorHttp1Test extends ConnectorBase { - JerseyConnectorHttp1Test(WebServer webServer) { + ConnectorHttp1Test(WebServer webServer) { baseURI = "http://localhost:" + webServer.port(); ClientConfig config = new ClientConfig(); config.connectorProvider(new HelidonConnectorProvider()); // use Helidon's provider diff --git a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp2AlpnTest.java b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2AlpnTest.java similarity index 92% rename from jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp2AlpnTest.java rename to jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2AlpnTest.java index 4ef80d88ae2..cc52c41b90a 100644 --- a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp2AlpnTest.java +++ b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2AlpnTest.java @@ -28,14 +28,12 @@ import jakarta.ws.rs.client.ClientBuilder; import org.glassfish.jersey.client.ClientConfig; -import static io.helidon.jersey.connector.HelidonProperties.TLS; - /** * Tests HTTP/2 integration of Jakarta REST client with the Helidon connector that uses * WebClient to execute HTTP requests. Uses ALPN extension for negotiation. */ @ServerTest -class JerseyConnectorHttp2AlpnTest extends JerseyConnectorBase { +class ConnectorHttp2AlpnTest extends ConnectorBase { @SetUpServer static void setUpServer(WebServerConfig.Builder serverBuilder) { @@ -54,7 +52,7 @@ static void setUpServer(WebServerConfig.Builder serverBuilder) { serverBuilder.addProtocol(Http2Config.create()); // h2 support ALPN } - JerseyConnectorHttp2AlpnTest(WebServer server) { + ConnectorHttp2AlpnTest(WebServer server) { int port = server.port("https"); Tls tls = Tls.builder() @@ -65,7 +63,7 @@ static void setUpServer(WebServerConfig.Builder serverBuilder) { ClientConfig config = new ClientConfig(); config.connectorProvider(new HelidonConnectorProvider()); // use Helidon's provider - config.property(TLS, tls); + config.property(HelidonProperties.TLS, tls); client = ClientBuilder.newClient(config); baseURI = "https://localhost:" + port; diff --git a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp2PriorTest.java b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2PriorTest.java similarity index 81% rename from jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp2PriorTest.java rename to jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2PriorTest.java index d73383e641f..0690edfcd68 100644 --- a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/JerseyConnectorHttp2PriorTest.java +++ b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2PriorTest.java @@ -21,7 +21,6 @@ import io.helidon.common.configurable.Resource; import io.helidon.common.pki.Keys; import io.helidon.common.tls.Tls; -import io.helidon.webclient.http2.Http2Client; import io.helidon.webclient.http2.Http2ClientProtocolConfig; import io.helidon.webserver.WebServer; import io.helidon.webserver.WebServerConfig; @@ -31,16 +30,12 @@ import jakarta.ws.rs.client.ClientBuilder; import org.glassfish.jersey.client.ClientConfig; -import static io.helidon.jersey.connector.HelidonProperties.PROTOCOL_CONFIG; -import static io.helidon.jersey.connector.HelidonProperties.PROTOCOL_PREFERENCE; -import static io.helidon.jersey.connector.HelidonProperties.TLS; - /** * Tests HTTP/2 integration of Jakarta REST client with the Helidon connector that uses * WebClient to execute HTTP requests. Assumes prior knowledge. */ @ServerTest -class JerseyConnectorHttp2PriorTest extends JerseyConnectorBase { +class ConnectorHttp2PriorTest extends ConnectorBase { @SetUpServer static void setUpServer(WebServerConfig.Builder serverBuilder) { @@ -59,7 +54,7 @@ static void setUpServer(WebServerConfig.Builder serverBuilder) { serverBuilder.addProtocol(Http2Config.create()); } - JerseyConnectorHttp2PriorTest(WebServer server) { + ConnectorHttp2PriorTest(WebServer server) { int port = server.port("https"); Tls tls = Tls.builder() @@ -69,9 +64,8 @@ static void setUpServer(WebServerConfig.Builder serverBuilder) { ClientConfig config = new ClientConfig(); config.connectorProvider(new HelidonConnectorProvider()); // use Helidon's provider - config.property(TLS, tls); - config.property(PROTOCOL_PREFERENCE, List.of(Http2Client.PROTOCOL_ID)); - config.property(PROTOCOL_CONFIG, List.of(Http2ClientProtocolConfig.builder() + config.property(HelidonProperties.TLS, tls); + config.property(HelidonProperties.PROTOCOL_CONFIGS, List.of(Http2ClientProtocolConfig.builder() .priorKnowledge(true) .build())); client = ClientBuilder.newClient(config); diff --git a/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2UpgradeTest.java b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2UpgradeTest.java new file mode 100644 index 00000000000..ff11bffd2ee --- /dev/null +++ b/jersey/tests/connector/src/test/java/io/helidon/jersey/connector/ConnectorHttp2UpgradeTest.java @@ -0,0 +1,50 @@ +/* + * 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.webclient.http2.Http2Client; +import io.helidon.webserver.WebServer; +import io.helidon.webserver.WebServerConfig; +import io.helidon.webserver.http2.Http2Config; +import io.helidon.webserver.testing.junit5.ServerTest; +import io.helidon.webserver.testing.junit5.SetUpServer; +import jakarta.ws.rs.client.ClientBuilder; +import org.glassfish.jersey.client.ClientConfig; + +/** + * Tests HTTP/2 integration of Jakarta REST client with the Helidon connector that uses + * WebClient to execute HTTP requests. Upgrades connection from HTTP/1.1 to HTTP/2. + */ +@ServerTest +class ConnectorHttp2UpgradeTest extends ConnectorBase { + + @SetUpServer + static void setUpServer(WebServerConfig.Builder serverBuilder) { + serverBuilder.addProtocol(Http2Config.create()); + } + + ConnectorHttp2UpgradeTest(WebServer server) { + int port = server.port(); + + ClientConfig config = new ClientConfig(); + config.connectorProvider(new HelidonConnectorProvider()); // use Helidon's provider + + client = ClientBuilder.newClient(config); + baseURI = "http://localhost:" + port; + protocolId = Http2Client.PROTOCOL_ID; // HTTP/2 negotiation from 1.1 + } +}