From ee84acff486664426f441b9e949b2b519e59d63f Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 26 Feb 2025 11:50:13 +0900 Subject: [PATCH 1/5] minimal impl --- .../client/AbstractWebClientBuilder.java | 10 +++ .../armeria/client/ClientBuilder.java | 6 +- .../com/linecorp/armeria/client/Clients.java | 44 ++++++++++ .../armeria/client/DefaultWebClient.java | 17 ++-- .../linecorp/armeria/client/RestClient.java | 41 ++++++++++ .../linecorp/armeria/client/WebClient.java | 41 ++++++++++ .../armeria/client/WebClientBuilder.java | 10 +++ .../client/websocket/WebSocketClient.java | 33 ++++++++ .../websocket/WebSocketClientBuilder.java | 2 + .../internal/client/SessionProtocolUtil.java | 43 ++++++++++ .../internal/common/ArmeriaHttpUtil.java | 5 ++ .../internal/common/DefaultRequestTarget.java | 8 +- .../armeria/client/ClientBuilderTest.java | 27 +++++++ .../client/HttpClientRequestPathTest.java | 2 +- .../armeria/client/RestClientBuilderTest.java | 51 ++++++++++++ .../WebClientAdditionalAuthorityTest.java | 2 +- .../armeria/client/WebClientBuilderTest.java | 56 +++++++++++++ .../websocket/WebSocketClientBuilderTest.java | 26 ++++++ .../client/eureka/EurekaEndpointGroup.java | 17 ++++ .../server/eureka/EurekaUpdatingListener.java | 17 ++++ .../client/grpc/GrpcClientBuilder.java | 6 +- .../armeria/client/grpc/GrpcClients.java | 48 +++++++++++ .../client/grpc/GrpcClientBuilderTest.java | 28 +++++++ .../client/retrofit2/ArmeriaRetrofit.java | 34 ++++++++ .../retrofit2/ArmeriaRetrofitBuilderTest.java | 23 ++++++ .../client/thrift/ThriftClientBuilder.java | 6 +- .../armeria/client/thrift/ThriftClients.java | 80 +++++++++++++++++++ .../thrift/ThriftClientBuilderTest.java | 28 +++++++ .../thrift/ThriftOverHttpClientTest.java | 2 +- 29 files changed, 695 insertions(+), 18 deletions(-) create mode 100644 core/src/main/java/com/linecorp/armeria/internal/client/SessionProtocolUtil.java create mode 100644 core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java diff --git a/core/src/main/java/com/linecorp/armeria/client/AbstractWebClientBuilder.java b/core/src/main/java/com/linecorp/armeria/client/AbstractWebClientBuilder.java index f75ff6ec13b..570473cefae 100644 --- a/core/src/main/java/com/linecorp/armeria/client/AbstractWebClientBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/AbstractWebClientBuilder.java @@ -18,6 +18,8 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.linecorp.armeria.common.SessionProtocol.httpAndHttpsValues; import static com.linecorp.armeria.internal.client.ClientUtil.UNDEFINED_URI; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.maybeApplyDefaultProtocol; import static java.util.Objects.requireNonNull; import java.net.URI; @@ -60,6 +62,13 @@ protected AbstractWebClientBuilder(URI uri) { this(validateUri(uri), null, null, null); } + /** + * Creates a new instance. + */ + protected AbstractWebClientBuilder(EndpointGroup endpointGroup, @Nullable String path) { + this(defaultSessionProtocol(), requireNonNull(endpointGroup, "endpointGroup"), path); + } + /** * Creates a new instance. * @@ -87,6 +96,7 @@ protected AbstractWebClientBuilder(@Nullable URI uri, @Nullable Scheme scheme, private static URI validateUri(URI uri) { requireNonNull(uri, "uri"); + uri = maybeApplyDefaultProtocol(uri); if (Clients.isUndefinedUri(uri)) { return uri; } diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientBuilder.java index f621681a9ec..647d6706d57 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientBuilder.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client; import static com.google.common.base.Preconditions.checkArgument; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.maybeApplyDefaultProtocol; import static java.util.Objects.requireNonNull; import java.net.URI; @@ -76,12 +77,11 @@ public final class ClientBuilder extends AbstractClientOptionsBuilder { private final Scheme scheme; ClientBuilder(URI uri) { - checkArgument(uri.getScheme() != null, "uri must have scheme: %s", uri); checkArgument(uri.getRawAuthority() != null, "uri must have authority: %s", uri); - this.uri = uri; + this.uri = maybeApplyDefaultProtocol(uri); endpointGroup = null; path = null; - scheme = Scheme.parse(uri.getScheme()); + scheme = Scheme.parse(this.uri.getScheme()); } ClientBuilder(Scheme scheme, EndpointGroup endpointGroup, @Nullable String path) { diff --git a/core/src/main/java/com/linecorp/armeria/client/Clients.java b/core/src/main/java/com/linecorp/armeria/client/Clients.java index 490ad660606..13341573806 100644 --- a/core/src/main/java/com/linecorp/armeria/client/Clients.java +++ b/core/src/main/java/com/linecorp/armeria/client/Clients.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client; import static com.linecorp.armeria.internal.client.ClientUtil.UNDEFINED_URI; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static java.util.Objects.requireNonNull; import java.net.URI; @@ -72,6 +73,17 @@ public static T newClient(URI uri, Class clientType) { return builder(uri).build(clientType); } + /** + * Creates a new client that connects to the specified {@link EndpointGroup} with + * the default {@link SessionProtocol} and {@link ClientFactory}. + * + * @param endpointGroup the server {@link EndpointGroup} + * @param clientType the type of the new client + */ + public static T newClient(EndpointGroup endpointGroup, Class clientType) { + return builder(endpointGroup).build(clientType); + } + /** * Creates a new client that connects to the specified {@link EndpointGroup} with the specified * {@code scheme} using the default {@link ClientFactory}. @@ -88,6 +100,18 @@ public static T newClient(String scheme, EndpointGroup endpointGroup, Class< return builder(scheme, endpointGroup).build(clientType); } + /** + * Creates a new client that connects to the specified {@link EndpointGroup} and + * {@code path} using the default {@link SessionProtocol} and {@link ClientFactory}. + * + * @param endpointGroup the server {@link EndpointGroup} + * @param path the path to the endpoint + * @param clientType the type of the new client + */ + public static T newClient(EndpointGroup endpointGroup, String path, Class clientType) { + return builder(endpointGroup, path).build(clientType); + } + /** * Creates a new client that connects to the specified {@link EndpointGroup} with the specified * {@code scheme} and {@code path} using the default {@link ClientFactory}. @@ -192,6 +216,16 @@ public static ClientBuilder builder(URI uri) { return new ClientBuilder(requireNonNull(uri, "uri")); } + /** + * Returns a new {@link ClientBuilder} that builds the client that connects to the specified + * {@link EndpointGroup} using the default {@link SessionProtocol}. + * + * @throws IllegalArgumentException if the {@code scheme} is invalid. + */ + public static ClientBuilder builder(EndpointGroup endpointGroup) { + return builder(Scheme.of(SerializationFormat.NONE, defaultSessionProtocol()), endpointGroup); + } + /** * Returns a new {@link ClientBuilder} that builds the client that connects to the specified * {@link EndpointGroup} with the specified {@code scheme}. @@ -202,6 +236,16 @@ public static ClientBuilder builder(String scheme, EndpointGroup endpointGroup) return builder(Scheme.parse(requireNonNull(scheme, "scheme")), endpointGroup); } + /** + * Returns a new {@link ClientBuilder} that builds the client that connects to the specified + * {@link EndpointGroup} and {@code path} using the default {@link SessionProtocol}. + * + * @throws IllegalArgumentException if the {@code scheme} is invalid. + */ + public static ClientBuilder builder(EndpointGroup endpointGroup, String path) { + return builder(Scheme.of(SerializationFormat.NONE, defaultSessionProtocol()), endpointGroup, path); + } + /** * Returns a new {@link ClientBuilder} that builds the client that connects to the specified * {@link EndpointGroup} with the specified {@code scheme} and {@code path}. diff --git a/core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java b/core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java index ad847f1c528..7e849468473 100644 --- a/core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java +++ b/core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static java.util.Objects.requireNonNull; import com.google.common.base.Strings; @@ -86,17 +87,16 @@ public HttpResponse execute(HttpRequest req, RequestOptions requestOptions) { scheme = req.scheme(); authority = req.authority(); - if (scheme == null || authority == null) { + if (authority == null) { return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException( - "Scheme and authority must be specified in \":path\" or " + - "in \":scheme\" and \":authority\". :path=" + - originalPath + ", :scheme=" + req.scheme() + ", :authority=" + req.authority())); + "Authority must be specified in \":path\" or " + + "in \":authority\". :path=" + originalPath + ", :authority=" + req.authority())); } } endpointGroup = Endpoint.parse(authority); try { - protocol = Scheme.parse(scheme).sessionProtocol(); + protocol = computeSessionProtocol(scheme); } catch (Exception e) { return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException( "Failed to parse a scheme: " + reqTarget.scheme(), e)); @@ -126,6 +126,13 @@ public HttpResponse execute(HttpRequest req, RequestOptions requestOptions) { return ClientUtil.executeWithFallback(preClient, ctx, newReq, errorResponseFactory()); } + private static SessionProtocol computeSessionProtocol(@Nullable String scheme) { + if (scheme == null) { + return defaultSessionProtocol(); + } + return Scheme.parse(scheme).sessionProtocol(); + } + private static HttpResponse abortRequestAndReturnFailureResponse( HttpRequest req, IllegalArgumentException cause) { req.abort(cause); diff --git a/core/src/main/java/com/linecorp/armeria/client/RestClient.java b/core/src/main/java/com/linecorp/armeria/client/RestClient.java index caf0c3bef31..a012c696f93 100644 --- a/core/src/main/java/com/linecorp/armeria/client/RestClient.java +++ b/core/src/main/java/com/linecorp/armeria/client/RestClient.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static java.util.Objects.requireNonNull; import java.net.URI; @@ -78,6 +79,16 @@ static RestClient of(URI uri) { return builder(uri).build(); } + /** + * Returns a new {@link RestClient} that connects to the specified {@link EndpointGroup} with + * the default {@link SessionProtocol}, {@link ClientFactory} and {@link ClientOptions}. + * + * @param endpointGroup the server {@link EndpointGroup} + */ + static RestClient of(EndpointGroup endpointGroup) { + return builder(endpointGroup).build(); + } + /** * Returns a new {@link RestClient} that connects to the specified {@link EndpointGroup} with * the specified {@code protocol} using the default {@link ClientFactory} and the default @@ -110,6 +121,18 @@ static RestClient of(SessionProtocol protocol, EndpointGroup endpointGroup) { return builder(protocol, endpointGroup).build(); } + /** + * Returns a new {@link RestClient} that connects to the specified {@link EndpointGroup} and + * {@code path} using the default {@link SessionProtocol}, {@link ClientFactory} and + * the {@link ClientOptions}. + * + * @param endpointGroup the server {@link EndpointGroup} + * @param path the path to the endpoint + */ + static RestClient of(EndpointGroup endpointGroup, String path) { + return builder(endpointGroup, path).build(); + } + /** * Returns a new {@link RestClient} that connects to the specified {@link EndpointGroup} with * the specified {@code protocol} and {@code path} using the default {@link ClientFactory} and @@ -173,6 +196,15 @@ static RestClientBuilder builder(URI uri) { return new RestClientBuilder(uri); } + /** + * Returns a new {@link RestClientBuilder} created with the default {@link SessionProtocol} + * and base {@link EndpointGroup}. + */ + static RestClientBuilder builder(EndpointGroup endpointGroup) { + requireNonNull(endpointGroup, "endpointGroup"); + return builder(defaultSessionProtocol(), endpointGroup); + } + /** * Returns a new {@link RestClientBuilder} created with the specified {@code protocol} * and base {@link EndpointGroup}. @@ -199,6 +231,15 @@ static RestClientBuilder builder(SessionProtocol protocol, EndpointGroup endpoin return new RestClientBuilder(protocol, endpointGroup, null); } + /** + * Returns a new {@link RestClientBuilder} created with the base {@link EndpointGroup} and path. + */ + static RestClientBuilder builder(EndpointGroup endpointGroup, String path) { + requireNonNull(endpointGroup, "endpointGroup"); + requireNonNull(path, "path"); + return builder(defaultSessionProtocol(), endpointGroup, path); + } + /** * Returns a new {@link RestClientBuilder} created with the specified {@code protocol}. * base {@link EndpointGroup} and path. diff --git a/core/src/main/java/com/linecorp/armeria/client/WebClient.java b/core/src/main/java/com/linecorp/armeria/client/WebClient.java index c7a3ca8df9d..1f5383aea0c 100644 --- a/core/src/main/java/com/linecorp/armeria/client/WebClient.java +++ b/core/src/main/java/com/linecorp/armeria/client/WebClient.java @@ -79,6 +79,16 @@ static WebClient of(URI uri) { return builder(uri).build(); } + /** + * Returns a new {@link WebClient} that connects to the specified {@link EndpointGroup} + * using the default {@link SessionProtocol}, {@link ClientFactory} and {@link ClientOptions}. + * + * @param endpointGroup the server {@link EndpointGroup} + */ + static WebClient of(EndpointGroup endpointGroup) { + return builder(endpointGroup).build(); + } + /** * Returns a new {@link WebClient} that connects to the specified {@link EndpointGroup} with * the specified {@code protocol} using the default {@link ClientFactory} and the default @@ -111,6 +121,18 @@ static WebClient of(SessionProtocol protocol, EndpointGroup endpointGroup) { return builder(protocol, endpointGroup).build(); } + /** + * Returns a new {@link WebClient} that connects to the specified {@link EndpointGroup} + * and {@code path} using the default {@link SessionProtocol}, {@link ClientFactory} and + * {@link ClientOptions}. + * + * @param endpointGroup the server {@link EndpointGroup} + * @param path the path to the endpoint + */ + static WebClient of(EndpointGroup endpointGroup, String path) { + return builder(endpointGroup, path).build(); + } + /** * Returns a new {@link WebClient} that connects to the specified {@link EndpointGroup} with * the specified {@code protocol} and {@code path} using the default {@link ClientFactory} and @@ -174,6 +196,15 @@ static WebClientBuilder builder(URI uri) { return new WebClientBuilder(uri); } + /** + * Returns a new {@link WebClientBuilder} created with the default {@link SessionProtocol} + * and base {@link EndpointGroup}. + */ + static WebClientBuilder builder(EndpointGroup endpointGroup) { + requireNonNull(endpointGroup, "endpointGroup"); + return new WebClientBuilder(endpointGroup, null); + } + /** * Returns a new {@link WebClientBuilder} created with the specified {@code protocol} * and base {@link EndpointGroup}. @@ -200,6 +231,16 @@ static WebClientBuilder builder(SessionProtocol protocol, EndpointGroup endpoint return new WebClientBuilder(protocol, endpointGroup, null); } + /** + * Returns a new {@link WebClientBuilder} created with the default {@link SessionProtocol}, + * base {@link EndpointGroup} and path. + */ + static WebClientBuilder builder(EndpointGroup endpointGroup, String path) { + requireNonNull(endpointGroup, "endpointGroup"); + requireNonNull(path, "path"); + return new WebClientBuilder(endpointGroup, path); + } + /** * Returns a new {@link WebClientBuilder} created with the specified {@code protocol}. * base {@link EndpointGroup} and path. diff --git a/core/src/main/java/com/linecorp/armeria/client/WebClientBuilder.java b/core/src/main/java/com/linecorp/armeria/client/WebClientBuilder.java index 1e9ec4ff4a8..f51f77d4f6f 100644 --- a/core/src/main/java/com/linecorp/armeria/client/WebClientBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/WebClientBuilder.java @@ -56,6 +56,16 @@ public final class WebClientBuilder extends AbstractWebClientBuilder { super(uri); } + /** + * Creates a new instance. + * + * @throws IllegalArgumentException if the {@code sessionProtocol} is not one of the fields + * in {@link SessionProtocol} + */ + WebClientBuilder(EndpointGroup endpointGroup, @Nullable String path) { + super(endpointGroup, path); + } + /** * Creates a new instance. * diff --git a/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClient.java b/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClient.java index dd810c2417f..4e7e861c1b3 100644 --- a/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClient.java +++ b/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClient.java @@ -17,6 +17,7 @@ package com.linecorp.armeria.client.websocket; import static com.linecorp.armeria.internal.client.ClientUtil.UNDEFINED_URI; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static java.util.Objects.requireNonNull; import java.net.URI; @@ -97,6 +98,14 @@ static WebSocketClient of(URI uri) { return builder(uri).build(); } + /** + * Returns a new {@link WebSocketClient} that connects to the specified {@link EndpointGroup} using + * the default {@link Scheme} and {@link ClientOptions}. + */ + static WebSocketClient of(EndpointGroup endpointGroup) { + return builder(endpointGroup).build(); + } + /** * Returns a new {@link WebSocketClient} that connects to the specified {@link EndpointGroup} with * the specified {@code scheme} using the default {@link ClientOptions}. @@ -121,6 +130,14 @@ static WebSocketClient of(SessionProtocol protocol, EndpointGroup endpointGroup) return builder(protocol, endpointGroup).build(); } + /** + * Returns a new {@link WebSocketClient} that connects to the specified {@link EndpointGroup} with + * and {@code path} using the default {@link Scheme} and {@link ClientOptions}. + */ + static WebSocketClient of(EndpointGroup endpointGroup, String path) { + return builder(endpointGroup, path).build(); + } + /** * Returns a new {@link WebSocketClient} that connects to the specified {@link EndpointGroup} with * the specified {@code scheme} and {@code path} using the default {@link ClientOptions}. @@ -166,6 +183,14 @@ static WebSocketClientBuilder builder(URI uri) { return new WebSocketClientBuilder(requireNonNull(uri, "uri")); } + /** + * Returns a new {@link WebSocketClientBuilder} created with the specified {@link EndpointGroup} + * and the default {@link Scheme}. + */ + static WebSocketClientBuilder builder(EndpointGroup endpointGroup) { + return builder(Scheme.of(SerializationFormat.WS, defaultSessionProtocol()), endpointGroup); + } + /** * Returns a new {@link WebSocketClientBuilder} created with the specified {@code scheme} * and the {@link EndpointGroup}. @@ -194,6 +219,14 @@ static WebSocketClientBuilder builder(SessionProtocol protocol, EndpointGroup en return builder(Scheme.of(SerializationFormat.WS, protocol), endpointGroup); } + /** + * Returns a new {@link WebSocketClientBuilder} created with the specified + * the {@link EndpointGroup} and the {@code path}, using the default {@link Scheme}. + */ + static WebSocketClientBuilder builder(EndpointGroup endpointGroup, String path) { + return builder(Scheme.of(SerializationFormat.WS, defaultSessionProtocol()), endpointGroup, path); + } + /** * Returns a new {@link WebSocketClientBuilder} created with the specified {@code scheme}, * the {@link EndpointGroup}, and the {@code path}. diff --git a/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilder.java b/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilder.java index 840169c0439..b54efda166d 100644 --- a/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilder.java @@ -65,6 +65,7 @@ import com.linecorp.armeria.common.auth.OAuth1aToken; import com.linecorp.armeria.common.auth.OAuth2Token; import com.linecorp.armeria.common.websocket.WebSocketFrameType; +import com.linecorp.armeria.internal.client.SessionProtocolUtil; /** * Builds a {@link WebSocketClient}. @@ -100,6 +101,7 @@ private static URI validateUri(URI uri) { if (Clients.isUndefinedUri(uri)) { return uri; } + uri = SessionProtocolUtil.maybeApplyDefaultProtocol(uri); final String givenScheme = requireNonNull(uri, "uri").getScheme(); final Scheme scheme = validateScheme(givenScheme); if (scheme.uriText().equals(givenScheme)) { diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/SessionProtocolUtil.java b/core/src/main/java/com/linecorp/armeria/internal/client/SessionProtocolUtil.java new file mode 100644 index 00000000000..75a4dce89d9 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/internal/client/SessionProtocolUtil.java @@ -0,0 +1,43 @@ +/* + * Copyright 2025 LINE Corporation + * + * LINE Corporation licenses this file to you 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: + * + * https://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 com.linecorp.armeria.internal.client; + +import java.net.URI; +import java.net.URISyntaxException; + +import com.linecorp.armeria.common.SessionProtocol; + +public final class SessionProtocolUtil { + + public static URI maybeApplyDefaultProtocol(URI uri) { + if (uri.getScheme() != null) { + return uri; + } + try { + return new URI(defaultSessionProtocol().uriText(), uri.getUserInfo(), uri.getAuthority(), + uri.getPort(), uri.getPath(), uri.getRawQuery(), uri.getFragment()); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + } + + public static SessionProtocol defaultSessionProtocol() { + return SessionProtocol.HTTP; + } + + private SessionProtocolUtil() {} +} diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java index e768ba4a9fe..a8c929ab044 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java @@ -279,6 +279,11 @@ public static String schemeValidateAndNormalize(String scheme) { * Returns {@code -1} otherwise. */ public static int findAuthority(String reqTarget) { + // A scheme-relative uri denoted as "//" authority path-abempty + // https://datatracker.ietf.org/doc/html/rfc3986#section-4.2 + if (reqTarget.length() > 2 && reqTarget.charAt(0) == '/' && reqTarget.charAt(1) == '/') { + return 2; + } final int firstColonIdx = reqTarget.indexOf(':'); if (firstColonIdx <= 0 || reqTarget.length() <= firstColonIdx + 3) { return -1; diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java index 75cb29a3da1..0aacbdf1a9b 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java @@ -27,6 +27,7 @@ import com.linecorp.armeria.common.RequestTargetForm; import com.linecorp.armeria.common.Scheme; import com.linecorp.armeria.common.annotation.Nullable; +import com.linecorp.armeria.internal.client.SessionProtocolUtil; import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals; import it.unimi.dsi.fastutil.Arrays; @@ -501,7 +502,12 @@ public static String removeMatrixVariables(String path) { private static RequestTarget slowAbsoluteFormForClient(String reqTarget, int authorityPos) { // Extract scheme and authority while looking for path. final SchemeAndAuthority schemeAndAuthority; - final String scheme = reqTarget.substring(0, authorityPos - 3); + final String scheme; + if (authorityPos <= 3) { + scheme = SessionProtocolUtil.defaultSessionProtocol().uriText(); + } else { + scheme = reqTarget.substring(0, authorityPos - 3); + } final int nextPos = findNextComponent(reqTarget, authorityPos); final String authority; if (nextPos < 0) { diff --git a/core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java index ec037f74eed..6ad2f669d84 100644 --- a/core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java @@ -16,9 +16,16 @@ package com.linecorp.armeria.client; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static org.assertj.core.api.Assertions.assertThat; +import java.net.URI; +import java.util.stream.Stream; + import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; /** * Test for {@link ClientBuilder}. @@ -50,4 +57,24 @@ void endpointWithPath() { .build(WebClient.class); assertThat(client.uri().toString()).isEqualTo("http://127.0.0.1/foo"); } + + public static Stream withoutScheme_args() throws Exception { + return Stream.of( + Arguments.of(Clients.newClient("//google.com", WebClient.class)), + Arguments.of(Clients.builder("//google.com").build(WebClient.class)), + Arguments.of(Clients.builder(new URI(null, "google.com", null, null)) + .build(WebClient.class)), + Arguments.of(Clients.newClient(Endpoint.of("google.com"), WebClient.class)), + Arguments.of(Clients.newClient(Endpoint.of("google.com"), "/", WebClient.class)), + Arguments.of(Clients.builder(Endpoint.of("google.com")).build(WebClient.class)), + Arguments.of(Clients.builder(Endpoint.of("google.com"), "/").build(WebClient.class)) + ); + } + + @ParameterizedTest + @MethodSource("withoutScheme_args") + void withoutScheme(WebClient client) { + assertThat(client.scheme().sessionProtocol()).isEqualTo(defaultSessionProtocol()); + assertThat(client.uri().toString()).isEqualTo("http://google.com/"); + } } diff --git a/core/src/test/java/com/linecorp/armeria/client/HttpClientRequestPathTest.java b/core/src/test/java/com/linecorp/armeria/client/HttpClientRequestPathTest.java index 7ba83d52755..dfa7a919d33 100644 --- a/core/src/test/java/com/linecorp/armeria/client/HttpClientRequestPathTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/HttpClientRequestPathTest.java @@ -124,7 +124,7 @@ void default_withRelativePath() { final HttpResponse response = WebClient.of().execute(request); assertThatThrownBy(() -> response.aggregate().join()) .hasCauseInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Scheme and authority must be specified"); + .hasMessageContaining("Authority must be specified"); } @ParameterizedTest diff --git a/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java new file mode 100644 index 00000000000..3e87df919cb --- /dev/null +++ b/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java @@ -0,0 +1,51 @@ +/* + * Copyright 2025 LINE Corporation + * + * LINE Corporation licenses this file to you 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: + * + * https://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 com.linecorp.armeria.client; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; +import java.util.stream.Stream; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import com.linecorp.armeria.common.SessionProtocol; + +class RestClientBuilderTest { + + public static Stream withoutScheme_args() throws Exception { + return Stream.of( + Arguments.of(RestClient.of("//google.com")), + Arguments.of(RestClient.builder("//google.com").build()), + Arguments.of(RestClient.of(new URI(null, "google.com", null, null))), + Arguments.of(RestClient.builder(new URI(null, "google.com", null, null)).build()), + Arguments.of(RestClient.of(Endpoint.of("google.com"))), + Arguments.of(RestClient.of(Endpoint.of("google.com"), "/")), + Arguments.of(RestClient.builder(Endpoint.of("google.com")).build()), + Arguments.of(RestClient.builder(Endpoint.of("google.com"), "/").build()) + ); + } + + @ParameterizedTest + @MethodSource("withoutScheme_args") + void withoutScheme(RestClient client) { + assertThat(client.scheme().sessionProtocol()).isEqualTo(SessionProtocol.HTTP); + assertThat(client.uri().toString()).isEqualTo("http://google.com/"); + } +} diff --git a/core/src/test/java/com/linecorp/armeria/client/WebClientAdditionalAuthorityTest.java b/core/src/test/java/com/linecorp/armeria/client/WebClientAdditionalAuthorityTest.java index 07ab1ac55f1..f138fe177c1 100644 --- a/core/src/test/java/com/linecorp/armeria/client/WebClientAdditionalAuthorityTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/WebClientAdditionalAuthorityTest.java @@ -284,7 +284,7 @@ void noAuthority() { final HttpRequest request = HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/")); assertThatThrownBy(() -> client.execute(request)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Scheme and authority must be specified in"); + .hasMessageContaining("Authority must be specified in"); } @Test diff --git a/core/src/test/java/com/linecorp/armeria/client/WebClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/WebClientBuilderTest.java index df61361f9ca..d848641aaec 100644 --- a/core/src/test/java/com/linecorp/armeria/client/WebClientBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/WebClientBuilderTest.java @@ -16,23 +16,33 @@ package com.linecorp.armeria.client; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.net.URI; +import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import com.linecorp.armeria.common.AggregatedHttpRequest; import com.linecorp.armeria.common.AggregatedHttpResponse; import com.linecorp.armeria.common.HttpHeaderNames; import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.RequestHeadersBuilder; +import com.linecorp.armeria.common.SessionProtocol; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.testing.junit5.server.ServerExtension; @@ -50,6 +60,7 @@ protected void configure(ServerBuilder sb) { return HttpResponse.of(pathAndQuery); }); sb.service("/head", (ctx, req) -> HttpResponse.of("Hello Armeria")); + sb.service("/hello", (ctx, req) -> HttpResponse.of("world")); } }; @@ -76,6 +87,51 @@ void uriWithoutNone() { assertThat(client.uri().toString()).isEqualTo("https://google.com/"); } + public static Stream withoutScheme_args() throws Exception { + return Stream.of( + Arguments.of(WebClient.of("//google.com")), + Arguments.of(WebClient.builder("//google.com").build()), + Arguments.of(WebClient.of(new URI(null, "google.com", null, null))), + Arguments.of(WebClient.builder(new URI(null, "google.com", null, null)).build()), + Arguments.of(WebClient.of(Endpoint.of("google.com"))), + Arguments.of(WebClient.of(Endpoint.of("google.com"), "/")), + Arguments.of(WebClient.builder(Endpoint.of("google.com")).build()), + Arguments.of(WebClient.builder(Endpoint.of("google.com"), "/").build()) + ); + } + + @ParameterizedTest + @MethodSource("withoutScheme_args") + void withoutScheme(WebClient client) { + assertThat(client.scheme().sessionProtocol()).isEqualTo(defaultSessionProtocol()); + assertThat(client.uri().toString()).isEqualTo("http://google.com/"); + } + + public static Stream defaultWithoutScheme_args() throws Exception { + final String authority = server.httpUri().getAuthority(); + return Stream.of( + Arguments.of(HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", + HttpHeaderNames.AUTHORITY, authority))), + Arguments.of(HttpRequest.of(HttpMethod.GET, "//" + authority + "/hello")) + ); + } + + @ParameterizedTest + @MethodSource("defaultWithoutScheme_args") + void defaultWithoutScheme(HttpRequest req) { + assertThat(WebClient.of().blocking().execute(req).contentUtf8()).isEqualTo("world"); + } + + @Test + void endpointGroupWithSchemeRelativeUri() { + final WebClient webClient = WebClient.of(SessionProtocol.HTTP, server.endpoint(SessionProtocol.HTTP)); + assertThatThrownBy(() -> webClient.get("//relative-uri").aggregate().join()) + .isInstanceOf(CompletionException.class) + .cause() + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot send a request with a \":path\" header"); + } + @Test void endpointWithoutPath() { final WebClient client = WebClient.builder("http", Endpoint.of("127.0.0.1")) diff --git a/core/src/test/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilderTest.java index df22c6d51c3..ed92ab51944 100644 --- a/core/src/test/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilderTest.java @@ -16,11 +16,17 @@ package com.linecorp.armeria.client.websocket; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static org.assertj.core.api.Assertions.assertThat; +import java.net.URI; +import java.util.stream.Stream; + import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.MethodSource; import com.linecorp.armeria.client.ClientOptions; import com.linecorp.armeria.client.Endpoint; @@ -108,4 +114,24 @@ void webSocketClientDefaultOptions() { assertThat(client.options().get(ClientOptions.REQUEST_AUTO_ABORT_DELAY_MILLIS)).isEqualTo(5000); assertThat(client.options().get(ClientOptions.AUTO_FILL_ORIGIN_HEADER)).isTrue(); } + + public static Stream withoutScheme_args() throws Exception { + return Stream.of( + Arguments.of(WebSocketClient.of("//google.com")), + Arguments.of(WebSocketClient.builder("//google.com").build()), + Arguments.of(WebSocketClient.of(new URI(null, "google.com", null, null))), + Arguments.of(WebSocketClient.builder(new URI(null, "google.com", null, null)).build()), + Arguments.of(WebSocketClient.of(Endpoint.of("google.com"))), + Arguments.of(WebSocketClient.of(Endpoint.of("google.com"), "/")), + Arguments.of(WebSocketClient.builder(Endpoint.of("google.com")).build()), + Arguments.of(WebSocketClient.builder(Endpoint.of("google.com"), "/").build()) + ); + } + + @ParameterizedTest + @MethodSource("withoutScheme_args") + void withoutScheme(WebSocketClient client) { + assertThat(client.scheme().sessionProtocol()).isEqualTo(defaultSessionProtocol()); + assertThat(client.uri().toString()).isEqualTo("ws+http://google.com/"); + } } diff --git a/eureka/src/main/java/com/linecorp/armeria/client/eureka/EurekaEndpointGroup.java b/eureka/src/main/java/com/linecorp/armeria/client/eureka/EurekaEndpointGroup.java index 631817fd1a7..50af685ceae 100644 --- a/eureka/src/main/java/com/linecorp/armeria/client/eureka/EurekaEndpointGroup.java +++ b/eureka/src/main/java/com/linecorp/armeria/client/eureka/EurekaEndpointGroup.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client.eureka; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static java.util.Objects.requireNonNull; import java.io.IOException; @@ -105,6 +106,14 @@ public static EurekaEndpointGroup of(URI eurekaUri) { return new EurekaEndpointGroupBuilder(eurekaUri).build(); } + /** + * Returns a new {@link EurekaEndpointGroup} that retrieves the {@link Endpoint} list from the specified + * {@link EndpointGroup} using the default {@link SessionProtocol}. + */ + public static EurekaEndpointGroup of(EndpointGroup endpointGroup) { + return new EurekaEndpointGroupBuilder(defaultSessionProtocol(), endpointGroup, null).build(); + } + /** * Returns a new {@link EurekaEndpointGroup} that retrieves the {@link Endpoint} list from the specified * {@link EndpointGroup}. @@ -138,6 +147,14 @@ public static EurekaEndpointGroupBuilder builder(URI eurekaUri) { return new EurekaEndpointGroupBuilder(eurekaUri); } + /** + * Returns a new {@link EurekaEndpointGroupBuilder} created with the specified + * {@link EndpointGroup} using the default {@link SessionProtocol}. + */ + public static EurekaEndpointGroupBuilder builder(EndpointGroup endpointGroup) { + return new EurekaEndpointGroupBuilder(defaultSessionProtocol(), endpointGroup, null); + } + /** * Returns a new {@link EurekaEndpointGroupBuilder} created with the specified {@link SessionProtocol} and * {@link EndpointGroup}. diff --git a/eureka/src/main/java/com/linecorp/armeria/server/eureka/EurekaUpdatingListener.java b/eureka/src/main/java/com/linecorp/armeria/server/eureka/EurekaUpdatingListener.java index d5f1a93f13b..c4ccad1f7b8 100644 --- a/eureka/src/main/java/com/linecorp/armeria/server/eureka/EurekaUpdatingListener.java +++ b/eureka/src/main/java/com/linecorp/armeria/server/eureka/EurekaUpdatingListener.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.server.eureka; import static com.google.common.base.Strings.isNullOrEmpty; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static com.linecorp.armeria.server.eureka.InstanceInfoBuilder.disabledPort; import static java.util.Objects.requireNonNull; @@ -80,6 +81,14 @@ public static EurekaUpdatingListener of(URI eurekaUri) { return new EurekaUpdatingListenerBuilder(eurekaUri).build(); } + /** + * Returns a new {@link EurekaUpdatingListener} which registers the current {@link Server} to + * the specified {@link EndpointGroup} using the default {@link SessionProtocol}. + */ + public static EurekaUpdatingListener of(EndpointGroup endpointGroup) { + return new EurekaUpdatingListenerBuilder(defaultSessionProtocol(), endpointGroup, null).build(); + } + /** * Returns a new {@link EurekaUpdatingListener} which registers the current {@link Server} to * the specified {@link EndpointGroup}. @@ -113,6 +122,14 @@ public static EurekaUpdatingListenerBuilder builder(URI eurekaUri) { return new EurekaUpdatingListenerBuilder(eurekaUri); } + /** + * Returns a new {@link EurekaUpdatingListenerBuilder} created with the specified + * {@link EndpointGroup} using the default {@link SessionProtocol}. + */ + public static EurekaUpdatingListenerBuilder builder(EndpointGroup endpointGroup) { + return new EurekaUpdatingListenerBuilder(defaultSessionProtocol(), endpointGroup, null); + } + /** * Returns a new {@link EurekaUpdatingListenerBuilder} created with the specified {@link SessionProtocol} * and {@link EndpointGroup}. diff --git a/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java b/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java index dedcd2f49fc..e2cb3ce4ecb 100644 --- a/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java +++ b/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java @@ -78,6 +78,7 @@ import com.linecorp.armeria.common.grpc.GrpcSerializationFormats; import com.linecorp.armeria.common.grpc.protocol.ArmeriaMessageDeframer; import com.linecorp.armeria.common.grpc.protocol.ArmeriaMessageFramer; +import com.linecorp.armeria.internal.client.SessionProtocolUtil; import com.linecorp.armeria.unsafe.grpc.GrpcUnsafeBufferUtil; import io.grpc.CallCredentials; @@ -110,10 +111,9 @@ public final class GrpcClientBuilder extends AbstractClientOptionsBuilder { GrpcClientBuilder(URI uri) { requireNonNull(uri, "uri"); - checkArgument(uri.getScheme() != null, "uri must have scheme: %s", uri); endpointGroup = null; - this.uri = uri; - scheme = Scheme.parse(uri.getScheme()); + this.uri = SessionProtocolUtil.maybeApplyDefaultProtocol(uri); + scheme = Scheme.parse(this.uri.getScheme()); validateOrSetSerializationFormat(); } diff --git a/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClients.java b/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClients.java index a6f1543eca5..626c3495df5 100644 --- a/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClients.java +++ b/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClients.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client.grpc; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static java.util.Objects.requireNonNull; import java.net.URI; @@ -68,6 +69,19 @@ public static T newClient(URI uri, Class clientType) { return builder(uri).build(clientType); } + /** + * Creates a new gRPC client that connects to the specified {@link EndpointGroup} + * using the default {@link ClientFactory} and {@link SessionProtocol}. + * + * @param endpointGroup the server {@link EndpointGroup} + * @param clientType the type of the new gRPC client + * + * @throws IllegalArgumentException if the specified {@code clientType} is an unsupported gRPC client stub. + */ + public static T newClient(EndpointGroup endpointGroup, Class clientType) { + return builder(endpointGroup).build(clientType); + } + /** * Creates a new gRPC client that connects to the specified {@link EndpointGroup} with the specified * {@code scheme} using the default {@link ClientFactory}. @@ -119,6 +133,22 @@ public static T newClient(SessionProtocol protocol, EndpointGroup endpointGr return builder(protocol, endpointGroup).build(clientType); } + /** + * Creates a new gRPC client that connects to the specified {@link EndpointGroup} with + * the specified {@link SerializationFormat} using the default {@link SessionProtocol} and + * {@link ClientFactory}. + * + * @param serializationFormat the {@link SerializationFormat} + * @param endpointGroup the server {@link EndpointGroup} + * @param clientType the type of the new gRPC client + * + * @throws IllegalArgumentException if the {@code clientType} is an unsupported gRPC client stub. + */ + public static T newClient(SerializationFormat serializationFormat, EndpointGroup endpointGroup, + Class clientType) { + return builder(serializationFormat, endpointGroup).build(clientType); + } + /** * Returns a new {@link GrpcClientBuilder} that builds the client that connects to the specified * {@code uri}. @@ -147,6 +177,14 @@ public static GrpcClientBuilder builder(URI uri) { return new GrpcClientBuilder(requireNonNull(uri, "uri")); } + /** + * Returns a new {@link GrpcClientBuilder} that builds the client that connects to the specified + * {@link EndpointGroup} with the default {@link Scheme}. + */ + public static GrpcClientBuilder builder(EndpointGroup endpointGroup) { + return builder(Scheme.of(GrpcSerializationFormats.PROTO, defaultSessionProtocol()), endpointGroup); + } + /** * Returns a new {@link GrpcClientBuilder} that builds the client that connects to the specified * {@link EndpointGroup} with the specified {@code scheme}. @@ -170,6 +208,16 @@ public static GrpcClientBuilder builder(SessionProtocol protocol, EndpointGroup endpointGroup); } + /** + * Returns a new {@link GrpcClientBuilder} that builds the gRPC client that connects + * to the specified {@link EndpointGroup} with the specified {@link SerializationFormat} + * and the default {@link SessionProtocol}. + */ + public static GrpcClientBuilder builder(SerializationFormat serializationFormat, + EndpointGroup endpointGroup) { + return builder(Scheme.of(serializationFormat, defaultSessionProtocol()), endpointGroup); + } + /** * Returns a new {@link GrpcClientBuilder} that builds the client that connects to the specified * {@link EndpointGroup} with the specified {@link Scheme}. diff --git a/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java b/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java index 56f144f9377..82bab5e3cc8 100644 --- a/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java @@ -16,22 +16,28 @@ package com.linecorp.armeria.client.grpc; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static testing.grpc.Messages.PayloadType.COMPRESSABLE; import java.io.InputStream; +import java.net.URI; +import java.util.stream.Stream; import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.MethodSource; import com.google.protobuf.ByteString; import com.linecorp.armeria.client.ClientBuilderParams; import com.linecorp.armeria.client.Clients; +import com.linecorp.armeria.client.Endpoint; import com.linecorp.armeria.client.endpoint.EndpointGroup; import com.linecorp.armeria.common.CommonPools; import com.linecorp.armeria.common.ContentTooLargeException; @@ -307,4 +313,26 @@ void useDefaultGrpcExceptionHandlerFunctionAsFallback() { .extracting(Status::getCode) .isEqualTo(Code.RESOURCE_EXHAUSTED); } + + public static Stream withoutScheme_args() throws Exception { + return Stream.of( + Arguments.of(GrpcClients.newClient("//google.com", TestServiceBlockingStub.class)), + Arguments.of(GrpcClients.builder("//google.com").build(TestServiceBlockingStub.class)), + Arguments.of(GrpcClients.builder(new URI(null, "google.com", null, null)) + .build(TestServiceBlockingStub.class)), + Arguments.of(GrpcClients.newClient(new URI(null, "google.com", null, null), + TestServiceBlockingStub.class)), + Arguments.of(GrpcClients.newClient(Endpoint.of("google.com"), TestServiceBlockingStub.class)), + Arguments.of(GrpcClients.builder(Endpoint.of("google.com")) + .build(TestServiceBlockingStub.class)) + ); + } + + @ParameterizedTest + @MethodSource("withoutScheme_args") + void withoutScheme(TestServiceBlockingStub client) throws Exception { + final ClientBuilderParams params = Clients.unwrap(client, ClientBuilderParams.class); + assertThat(params.scheme().sessionProtocol()).isEqualTo(defaultSessionProtocol()); + assertThat(params.uri().toString()).isEqualTo("gproto+http://google.com/"); + } } diff --git a/retrofit2/src/main/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java b/retrofit2/src/main/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java index 3bed31904c0..c349fc05503 100644 --- a/retrofit2/src/main/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java +++ b/retrofit2/src/main/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client.retrofit2; import static com.google.common.base.Preconditions.checkArgument; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static java.util.Objects.requireNonNull; import java.net.URI; @@ -24,6 +25,7 @@ import com.linecorp.armeria.client.Endpoint; import com.linecorp.armeria.client.WebClient; import com.linecorp.armeria.client.endpoint.EndpointGroup; +import com.linecorp.armeria.common.Scheme; import com.linecorp.armeria.common.SessionProtocol; import retrofit2.Retrofit; @@ -55,6 +57,14 @@ public static Retrofit of(URI baseUrl) { return builder(baseUrl).build(); } + /** + * Returns a new {@link Retrofit} which sends requests to the specified {@link Endpoint} using + * the default {@link SessionProtocol}. + */ + public static Retrofit of(EndpointGroup endpointGroup) { + return builder(defaultSessionProtocol(), endpointGroup).build(); + } + /** * Returns a new {@link Retrofit} which sends requests to the specified {@link Endpoint} using * the specified {@code protocol}. @@ -79,6 +89,14 @@ public static Retrofit of(SessionProtocol protocol, EndpointGroup endpointGroup) return builder(protocol, endpointGroup).build(); } + /** + * Returns a new {@link Retrofit} which sends requests to the specified {@link Endpoint} and {@code path} + * using the default {@link Scheme}. + */ + public static Retrofit of(EndpointGroup endpointGroup, String path) { + return builder(endpointGroup, path).build(); + } + /** * Returns a new {@link Retrofit} which sends requests to the specified {@link Endpoint} using * the specified {@code protocol} and {@code path}. @@ -134,6 +152,14 @@ public static ArmeriaRetrofitBuilder builder(URI baseUrl) { return builder(WebClient.of(baseUrl)); } + /** + * Returns a new {@link ArmeriaRetrofitBuilder} that builds a client that sends requests to + * the specified {@link EndpointGroup} using the default {@link SessionProtocol}. + */ + public static ArmeriaRetrofitBuilder builder(EndpointGroup endpointGroup) { + return builder(defaultSessionProtocol(), endpointGroup); + } + /** * Returns a new {@link ArmeriaRetrofitBuilder} that builds a client that sends requests to * the specified {@link EndpointGroup} using the specified {@code protocol}. @@ -160,6 +186,14 @@ public static ArmeriaRetrofitBuilder builder(SessionProtocol protocol, EndpointG return builder(WebClient.of(protocol, endpointGroup)); } + /** + * Returns a new {@link ArmeriaRetrofitBuilder} that builds a client that sends requests to + * the specified {@link EndpointGroup} and {@code path} using the default {@link SessionProtocol} . + */ + public static ArmeriaRetrofitBuilder builder(EndpointGroup endpointGroup, String path) { + return builder(defaultSessionProtocol(), endpointGroup, path); + } + /** * Returns a new {@link ArmeriaRetrofitBuilder} that builds a client that sends requests to * the specified {@link EndpointGroup} using the specified {@link SessionProtocol} and {@code path}. diff --git a/retrofit2/src/test/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofitBuilderTest.java b/retrofit2/src/test/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofitBuilderTest.java index d0e10e36dcb..8bfc56c662e 100644 --- a/retrofit2/src/test/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofitBuilderTest.java +++ b/retrofit2/src/test/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofitBuilderTest.java @@ -20,12 +20,17 @@ import static org.awaitility.Awaitility.await; import java.io.IOException; +import java.net.URI; import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import com.fasterxml.jackson.databind.ObjectMapper; @@ -174,6 +179,24 @@ void streamingResponseIsCompleteWhenVoidReturnType() { await().until(future::isDone); } + public static Stream withoutScheme_args() throws Exception { + return Stream.of( + Arguments.of(ArmeriaRetrofit.of("//google.com")), + Arguments.of(ArmeriaRetrofit.builder("//google.com").build()), + Arguments.of(ArmeriaRetrofit.builder(new URI(null, "google.com", null, null)).build()), + Arguments.of(ArmeriaRetrofit.of(Endpoint.of("google.com"))), + Arguments.of(ArmeriaRetrofit.of(Endpoint.of("google.com"), "/")), + Arguments.of(ArmeriaRetrofit.builder(Endpoint.of("google.com")).build()), + Arguments.of(ArmeriaRetrofit.builder(Endpoint.of("google.com"), "/").build()) + ); + } + + @ParameterizedTest + @MethodSource("withoutScheme_args") + void withoutScheme(Retrofit retrofit) throws Exception { + assertThat(retrofit.baseUrl().toString()).isEqualTo("http://google.com/"); + } + interface Service { @GET("/secret") CompletableFuture secret(); diff --git a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClientBuilder.java b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClientBuilder.java index 17f31b6c8d8..5fa0464ae4d 100644 --- a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClientBuilder.java +++ b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClientBuilder.java @@ -59,6 +59,7 @@ import com.linecorp.armeria.common.auth.OAuth1aToken; import com.linecorp.armeria.common.auth.OAuth2Token; import com.linecorp.armeria.common.thrift.ThriftSerializationFormats; +import com.linecorp.armeria.internal.client.SessionProtocolUtil; /** * Creates a new Thrift client that connects to the specified {@link URI} using the builder pattern. @@ -78,10 +79,9 @@ public final class ThriftClientBuilder extends AbstractClientOptionsBuilder { ThriftClientBuilder(URI uri) { requireNonNull(uri, "uri"); - checkArgument(uri.getScheme() != null, "uri must have scheme: %s", uri); endpointGroup = null; - this.uri = uri; - scheme = Scheme.parse(uri.getScheme()); + this.uri = SessionProtocolUtil.maybeApplyDefaultProtocol(uri); + scheme = Scheme.parse(this.uri.getScheme()); validateOrSetSerializationFormat(); } diff --git a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClients.java b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClients.java index bf3c1915d90..7aaf8c21a8a 100644 --- a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClients.java +++ b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClients.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client.thrift; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static java.util.Objects.requireNonNull; import java.net.URI; @@ -68,6 +69,33 @@ public static T newClient(URI uri, Class clientType) { return builder(uri).build(clientType); } + /** + * Creates a new Thrift client that connects to the specified {@link EndpointGroup} + * using the default {@link Scheme} and {@link ClientFactory}. + * + * @param endpointGroup the server {@link EndpointGroup} + * @param clientType the type of the new Thrift client + */ + public static T newClient(EndpointGroup endpointGroup, Class clientType) { + return builder(endpointGroup).build(clientType); + } + + /** + * Creates a new Thrift client that connects to the specified {@link EndpointGroup} with the specified + * {@link SerializationFormat} using the default {@link SessionProtocol} and {@link ClientFactory}. + * + * @param serializationFormat the {@link SerializationFormat} + * @param endpointGroup the server {@link EndpointGroup} + * @param clientType the type of the new Thrift client + * + * @throws IllegalArgumentException if the specified {@link SerializationFormat} is invalid or the + * specified {@code clientType} is an unsupported Thrift client stub. + */ + public static T newClient(SerializationFormat serializationFormat, EndpointGroup endpointGroup, + String path, Class clientType) { + return builder(serializationFormat, endpointGroup).path(path).build(clientType); + } + /** * Creates a new Thrift client that connects to the specified {@link EndpointGroup} with the specified * {@code scheme} using the default {@link ClientFactory}. @@ -139,6 +167,39 @@ public static T newClient(SessionProtocol protocol, EndpointGroup endpointGr return builder(protocol, endpointGroup).build(clientType); } + /** + * Creates a new Thrift client that connects to the specified {@link EndpointGroup} and + * {@link SerializationFormat} using the default {@link SessionProtocol} and + * {@link ClientFactory}. + * + * @param serializationFormat the {@link SerializationFormat} + * @param endpointGroup the server {@link EndpointGroup} + * @param clientType the type of the new Thrift client + * + * @throws IllegalArgumentException if the specified {@link SerializationFormat} is invalid or the + * specified {@code clientType} is an unsupported Thrift client stub. + */ + public static T newClient(SerializationFormat serializationFormat, + EndpointGroup endpointGroup, Class clientType) { + return builder(serializationFormat, endpointGroup).build(clientType); + } + + /** + * Creates a new Thrift client that connects to the specified {@link EndpointGroup} and + * {@code path} using the default {@link Scheme} and {@link ClientFactory}. + * + * @param endpointGroup the server {@link EndpointGroup} + * @param path the path to the endpoint + * @param clientType the type of the new client + * + * @throws IllegalArgumentException if the specified {@code clientType} is an unsupported Thrift client + * stub. + */ + public static T newClient(EndpointGroup endpointGroup, String path, Class clientType) { + return builder(Scheme.of(ThriftSerializationFormats.BINARY, defaultSessionProtocol()), + endpointGroup).path(path).build(clientType); + } + /** * Creates a new client that connects to the specified {@link EndpointGroup} with * the specified {@link SessionProtocol}, {@code path} and {@link ThriftSerializationFormats#BINARY} @@ -184,6 +245,14 @@ public static ThriftClientBuilder builder(URI uri) { return new ThriftClientBuilder(requireNonNull(uri, "uri")); } + /** + * Returns a new {@link ThriftClientBuilder} that builds the client that connects to the specified + * {@link EndpointGroup} with the default {@link Scheme}. + */ + public static ThriftClientBuilder builder(EndpointGroup endpointGroup) { + return builder(Scheme.of(ThriftSerializationFormats.BINARY, defaultSessionProtocol()), endpointGroup); + } + /** * Returns a new {@link ThriftClientBuilder} that builds the client that connects to the specified * {@link EndpointGroup} with the specified {@code scheme}. @@ -197,6 +266,17 @@ public static ThriftClientBuilder builder(String scheme, EndpointGroup endpointG return builder(Scheme.parse(requireNonNull(scheme, "scheme")), endpointGroup); } + /** + * Returns a new {@link ThriftClientBuilder} that builds the client that connects to the specified + * {@link EndpointGroup} with the specified {@link SerializationFormat}. + * + * @throws IllegalArgumentException if the {@code serializationFormat} is invalid. + */ + public static ThriftClientBuilder builder(SerializationFormat serializationFormat, + EndpointGroup endpointGroup) { + return builder(Scheme.of(serializationFormat, defaultSessionProtocol()), endpointGroup); + } + /** * Returns a new {@link ThriftClientBuilder} that builds the Thrift client that connects * to the specified {@link EndpointGroup} with the specified {@link SessionProtocol} and diff --git a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java index a5ca1f4a0f0..3fbf80ef297 100644 --- a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java +++ b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java @@ -15,17 +15,23 @@ */ package com.linecorp.armeria.client.thrift; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.net.URI; import java.util.Arrays; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.stream.Stream; import org.apache.thrift.transport.TTransportException; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.reflections.ReflectionUtils; import com.linecorp.armeria.client.AbstractClientOptionsBuilder; @@ -166,4 +172,26 @@ void apiConsistency() { }).hasSize(1); } } + + public static Stream withoutScheme_args() throws Exception { + return Stream.of( + Arguments.of(ThriftClients.newClient("//google.com", HelloService.Iface.class)), + Arguments.of(ThriftClients.builder("//google.com").build(HelloService.Iface.class)), + Arguments.of(ThriftClients.builder(new URI(null, "google.com", null, null)) + .build(HelloService.Iface.class)), + Arguments.of(ThriftClients.newClient(Endpoint.of("google.com"), HelloService.Iface.class)), + Arguments.of(ThriftClients.newClient(Endpoint.of("google.com"), "/", HelloService.Iface.class)), + Arguments.of(ThriftClients.builder(Endpoint.of("google.com")).build(HelloService.Iface.class)), + Arguments.of(ThriftClients.builder(Endpoint.of("google.com")).path("/") + .build(HelloService.Iface.class)) + ); + } + + @ParameterizedTest + @MethodSource("withoutScheme_args") + void withoutScheme(HelloService.Iface client) throws Exception { + final ClientBuilderParams params = Clients.unwrap(client, ClientBuilderParams.class); + assertThat(params.scheme().sessionProtocol()).isEqualTo(defaultSessionProtocol()); + assertThat(params.uri().toString()).isEqualTo("tbinary+http://google.com/"); + } } diff --git a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftOverHttpClientTest.java b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftOverHttpClientTest.java index a2755945f29..12248efd0da 100644 --- a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftOverHttpClientTest.java +++ b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftOverHttpClientTest.java @@ -753,7 +753,7 @@ void endpointMapping( : server.httpPort()); final HelloService.Iface client = ThriftClients.builder(format.uriText() + '+' + protocol.uriText() + - "://my-group/" + Handlers.HELLO.path(format)) + "://my-group" + Handlers.HELLO.path(format)) .options(clientOptions) .endpointRemapper(endpoint -> { if ("my-group".equals(endpoint.host())) { From 37978c636be7c109f6ed27ebd8994e7743b62f1d Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 26 Feb 2025 15:57:53 +0900 Subject: [PATCH 2/5] cleanup --- .../internal/common/ArmeriaHttpUtil.java | 5 +-- .../common/DefaultRequestTargetTest.java | 36 ++++++++++++++++--- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java index a8c929ab044..c8144634566 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java @@ -281,7 +281,8 @@ public static String schemeValidateAndNormalize(String scheme) { public static int findAuthority(String reqTarget) { // A scheme-relative uri denoted as "//" authority path-abempty // https://datatracker.ietf.org/doc/html/rfc3986#section-4.2 - if (reqTarget.length() > 2 && reqTarget.charAt(0) == '/' && reqTarget.charAt(1) == '/') { + if (reqTarget.length() >= 3 && reqTarget.charAt(0) == '/' && + reqTarget.charAt(1) == '/' && reqTarget.charAt(2) != '/') { return 2; } final int firstColonIdx = reqTarget.indexOf(':'); @@ -775,7 +776,7 @@ public static void toArmeria(io.netty.handler.codec.http.HttpHeaders inHeaders, * for HTTP1 en/decoders. */ private static void purgeHttp1OnlyHeaders(io.netty.handler.codec.http.HttpHeaders inHeaders, - HttpHeadersBuilder out) { + HttpHeadersBuilder out) { //TODO(minwoox): dedup the logic between these method and toArmeria maybeSetTeHeader(inHeaders, out); maybeRemoveConnectionHeaders(inHeaders, out); diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java index 62251179c7e..00699709606 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/common/DefaultRequestTargetTest.java @@ -38,6 +38,7 @@ import com.linecorp.armeria.common.QueryParams; import com.linecorp.armeria.common.RequestTarget; +import com.linecorp.armeria.common.RequestTargetForm; import com.linecorp.armeria.common.annotation.Nullable; class DefaultRequestTargetTest { @@ -310,6 +311,31 @@ void clientShouldRetainConsecutiveSlashesInFragment() { assertAccepted(forClient("/#/////"), "/", null, "/////"); } + @Test + void notSchemeRelative() { + assertThat(RequestTarget.forClient("//").form()).isEqualTo(RequestTargetForm.ORIGIN); + assertThat(RequestTarget.forClient("//").path()).isEqualTo("//"); + + assertThat(RequestTarget.forClient("a//").form()).isEqualTo(RequestTargetForm.ORIGIN); + assertThat(RequestTarget.forClient("a//").path()).isEqualTo("/a//"); + + assertThat(RequestTarget.forClient("a//", "//").form()).isEqualTo(RequestTargetForm.ORIGIN); + assertThat(RequestTarget.forClient("a//", "//").path()).isEqualTo("//a//"); + + assertThat(RequestTarget.forClient("///").form()).isEqualTo(RequestTargetForm.ORIGIN); + assertThat(RequestTarget.forClient("///").path()).isEqualTo("///"); + + assertThat(RequestTarget.forClient("///", "//").form()).isEqualTo(RequestTargetForm.ORIGIN); + assertThat(RequestTarget.forClient("///", "//").path()).isEqualTo("////"); + } + + @Test + void schemeRelative() { + assertThat(RequestTarget.forClient("//a").form()).isEqualTo(RequestTargetForm.ABSOLUTE); + assertThat(RequestTarget.forClient("//a").authority()).isEqualTo("a"); + assertThat(RequestTarget.forClient("//a").path()).isEqualTo("/"); + } + @ParameterizedTest @EnumSource(Mode.class) void shouldAcceptColon(Mode mode) { @@ -622,9 +648,9 @@ private static class RequestTargetWithRawPath { @Override public String toString() { return "RequestTargetWithRawPath{" + - "rawPath='" + rawPath + '\'' + - ", requestTarget=" + requestTarget + - '}'; + "rawPath='" + rawPath + '\'' + + ", requestTarget=" + requestTarget + + '}'; } } @@ -653,8 +679,8 @@ private static RequestTargetWithRawPath forClient(String rawPath, @Nullable Stri final RequestTarget target = DefaultRequestTarget.forClient(rawPath, prefix); if (target != null) { logger.info("forClient({}, {}) => path: {}, query: {}, fragment: {}", - rawPath, prefix, target.path(), - target.query(), target.fragment()); + rawPath, prefix, target.path(), + target.query(), target.fragment()); } else { logger.info("forClient({}, {}) => null", rawPath, prefix); } From 93727c5a16b5ef5b0a326a4731a3b7b63deb6df8 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 26 Feb 2025 16:12:31 +0900 Subject: [PATCH 3/5] tmp --- .../main/java/com/linecorp/armeria/client/ClientBuilder.java | 5 +++-- core/src/main/java/com/linecorp/armeria/client/Clients.java | 4 ---- .../java/com/linecorp/armeria/client/WebClientBuilder.java | 3 --- .../linecorp/armeria/client/websocket/WebSocketClient.java | 2 +- .../com/linecorp/armeria/client/RestClientBuilderTest.java | 5 ++--- .../com/linecorp/armeria/client/grpc/GrpcClientBuilder.java | 5 +++-- .../linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java | 3 +-- .../linecorp/armeria/client/thrift/ThriftClientBuilder.java | 5 +++-- 8 files changed, 13 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/client/ClientBuilder.java b/core/src/main/java/com/linecorp/armeria/client/ClientBuilder.java index 647d6706d57..8821e448608 100644 --- a/core/src/main/java/com/linecorp/armeria/client/ClientBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/ClientBuilder.java @@ -77,11 +77,12 @@ public final class ClientBuilder extends AbstractClientOptionsBuilder { private final Scheme scheme; ClientBuilder(URI uri) { + uri = maybeApplyDefaultProtocol(uri); checkArgument(uri.getRawAuthority() != null, "uri must have authority: %s", uri); - this.uri = maybeApplyDefaultProtocol(uri); + this.uri = uri; endpointGroup = null; path = null; - scheme = Scheme.parse(this.uri.getScheme()); + scheme = Scheme.parse(uri.getScheme()); } ClientBuilder(Scheme scheme, EndpointGroup endpointGroup, @Nullable String path) { diff --git a/core/src/main/java/com/linecorp/armeria/client/Clients.java b/core/src/main/java/com/linecorp/armeria/client/Clients.java index 13341573806..bacad00cf98 100644 --- a/core/src/main/java/com/linecorp/armeria/client/Clients.java +++ b/core/src/main/java/com/linecorp/armeria/client/Clients.java @@ -219,8 +219,6 @@ public static ClientBuilder builder(URI uri) { /** * Returns a new {@link ClientBuilder} that builds the client that connects to the specified * {@link EndpointGroup} using the default {@link SessionProtocol}. - * - * @throws IllegalArgumentException if the {@code scheme} is invalid. */ public static ClientBuilder builder(EndpointGroup endpointGroup) { return builder(Scheme.of(SerializationFormat.NONE, defaultSessionProtocol()), endpointGroup); @@ -239,8 +237,6 @@ public static ClientBuilder builder(String scheme, EndpointGroup endpointGroup) /** * Returns a new {@link ClientBuilder} that builds the client that connects to the specified * {@link EndpointGroup} and {@code path} using the default {@link SessionProtocol}. - * - * @throws IllegalArgumentException if the {@code scheme} is invalid. */ public static ClientBuilder builder(EndpointGroup endpointGroup, String path) { return builder(Scheme.of(SerializationFormat.NONE, defaultSessionProtocol()), endpointGroup, path); diff --git a/core/src/main/java/com/linecorp/armeria/client/WebClientBuilder.java b/core/src/main/java/com/linecorp/armeria/client/WebClientBuilder.java index f51f77d4f6f..aa6417b47e8 100644 --- a/core/src/main/java/com/linecorp/armeria/client/WebClientBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/client/WebClientBuilder.java @@ -58,9 +58,6 @@ public final class WebClientBuilder extends AbstractWebClientBuilder { /** * Creates a new instance. - * - * @throws IllegalArgumentException if the {@code sessionProtocol} is not one of the fields - * in {@link SessionProtocol} */ WebClientBuilder(EndpointGroup endpointGroup, @Nullable String path) { super(endpointGroup, path); diff --git a/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClient.java b/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClient.java index 4e7e861c1b3..48122e6880c 100644 --- a/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClient.java +++ b/core/src/main/java/com/linecorp/armeria/client/websocket/WebSocketClient.java @@ -221,7 +221,7 @@ static WebSocketClientBuilder builder(SessionProtocol protocol, EndpointGroup en /** * Returns a new {@link WebSocketClientBuilder} created with the specified - * the {@link EndpointGroup} and the {@code path}, using the default {@link Scheme}. + * {@link EndpointGroup}, {@code path} and the default {@link Scheme}. */ static WebSocketClientBuilder builder(EndpointGroup endpointGroup, String path) { return builder(Scheme.of(SerializationFormat.WS, defaultSessionProtocol()), endpointGroup, path); diff --git a/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java index 3e87df919cb..feb64b51191 100644 --- a/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java @@ -16,6 +16,7 @@ package com.linecorp.armeria.client; +import static com.linecorp.armeria.internal.client.SessionProtocolUtil.defaultSessionProtocol; import static org.assertj.core.api.Assertions.assertThat; import java.net.URI; @@ -25,8 +26,6 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import com.linecorp.armeria.common.SessionProtocol; - class RestClientBuilderTest { public static Stream withoutScheme_args() throws Exception { @@ -45,7 +44,7 @@ public static Stream withoutScheme_args() throws Exception { @ParameterizedTest @MethodSource("withoutScheme_args") void withoutScheme(RestClient client) { - assertThat(client.scheme().sessionProtocol()).isEqualTo(SessionProtocol.HTTP); + assertThat(client.scheme().sessionProtocol()).isEqualTo(defaultSessionProtocol()); assertThat(client.uri().toString()).isEqualTo("http://google.com/"); } } diff --git a/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java b/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java index e2cb3ce4ecb..807c38fe79c 100644 --- a/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java +++ b/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java @@ -112,8 +112,9 @@ public final class GrpcClientBuilder extends AbstractClientOptionsBuilder { GrpcClientBuilder(URI uri) { requireNonNull(uri, "uri"); endpointGroup = null; - this.uri = SessionProtocolUtil.maybeApplyDefaultProtocol(uri); - scheme = Scheme.parse(this.uri.getScheme()); + uri = SessionProtocolUtil.maybeApplyDefaultProtocol(uri); + this.uri = uri; + scheme = Scheme.parse(uri.getScheme()); validateOrSetSerializationFormat(); } diff --git a/retrofit2/src/main/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java b/retrofit2/src/main/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java index c349fc05503..4ad854ec5cf 100644 --- a/retrofit2/src/main/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java +++ b/retrofit2/src/main/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofit.java @@ -25,7 +25,6 @@ import com.linecorp.armeria.client.Endpoint; import com.linecorp.armeria.client.WebClient; import com.linecorp.armeria.client.endpoint.EndpointGroup; -import com.linecorp.armeria.common.Scheme; import com.linecorp.armeria.common.SessionProtocol; import retrofit2.Retrofit; @@ -91,7 +90,7 @@ public static Retrofit of(SessionProtocol protocol, EndpointGroup endpointGroup) /** * Returns a new {@link Retrofit} which sends requests to the specified {@link Endpoint} and {@code path} - * using the default {@link Scheme}. + * using the default {@link SessionProtocol}. */ public static Retrofit of(EndpointGroup endpointGroup, String path) { return builder(endpointGroup, path).build(); diff --git a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClientBuilder.java b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClientBuilder.java index 5fa0464ae4d..faddd5000c0 100644 --- a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClientBuilder.java +++ b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/client/thrift/ThriftClientBuilder.java @@ -80,8 +80,9 @@ public final class ThriftClientBuilder extends AbstractClientOptionsBuilder { ThriftClientBuilder(URI uri) { requireNonNull(uri, "uri"); endpointGroup = null; - this.uri = SessionProtocolUtil.maybeApplyDefaultProtocol(uri); - scheme = Scheme.parse(this.uri.getScheme()); + uri = SessionProtocolUtil.maybeApplyDefaultProtocol(uri); + this.uri = uri; + scheme = Scheme.parse(uri.getScheme()); validateOrSetSerializationFormat(); } From 4686e2b8b1e93a3a7b93f392b275bda411600d03 Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Wed, 26 Feb 2025 17:52:43 +0900 Subject: [PATCH 4/5] introduce client path parsing --- .../common/AbstractRequestContextBuilder.java | 2 +- .../client/DefaultClientRequestContext.java | 5 +++-- .../internal/common/DefaultRequestTarget.java | 14 ++++++++++++- .../internal/common/RequestTargetCache.java | 21 +++++++++++++++++-- .../ClientRequestContextBuilderTest.java | 6 ++++++ .../client/ClientRequestContextTest.java | 19 +++++++++++++---- .../internal/client/grpc/ArmeriaChannel.java | 5 +++-- .../client/thrift/DefaultTHttpClient.java | 5 +++-- .../thrift/ThriftClientBuilderTest.java | 17 +++++++++++++++ .../thrift/ThriftOverHttpClientTest.java | 2 +- 10 files changed, 81 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java b/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java index 89945e7869b..82cc9a8d58f 100644 --- a/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/AbstractRequestContextBuilder.java @@ -107,7 +107,7 @@ protected AbstractRequestContextBuilder(boolean server, HttpRequest req) { final String rawPath = req.headers().path(); final RequestTarget reqTarget = server ? RequestTarget.forServer(rawPath) - : RequestTarget.forClient(rawPath); + : DefaultRequestTarget.forClientPath(rawPath); checkArgument(reqTarget != null, "request.path is not valid: %s", rawPath); checkArgument(reqTarget.form() != RequestTargetForm.ABSOLUTE, "request.path must not contain scheme or authority: %s", rawPath); diff --git a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java index 3bb0da0b334..cd2bd50373d 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java +++ b/core/src/main/java/com/linecorp/armeria/internal/client/DefaultClientRequestContext.java @@ -78,6 +78,7 @@ import com.linecorp.armeria.common.util.UnmodifiableFuture; import com.linecorp.armeria.internal.common.CancellationScheduler; import com.linecorp.armeria.internal.common.CancellationScheduler.CancellationTask; +import com.linecorp.armeria.internal.common.DefaultRequestTarget; import com.linecorp.armeria.internal.common.HeaderOverridingHttpRequest; import com.linecorp.armeria.internal.common.NonWrappingRequestContext; import com.linecorp.armeria.internal.common.RequestContextExtension; @@ -675,7 +676,7 @@ public ClientRequestContext newDerivedContext(RequestId id, final String oldPath = requestTarget().pathAndQuery(); final String newPath = newHeaders.path(); if (!oldPath.equals(newPath)) { - // path is changed. + // path is changed final RequestTarget reqTarget = RequestTarget.forClient(newPath); checkArgument(reqTarget != null, "invalid path: %s", newPath); @@ -709,7 +710,7 @@ public ClientRequestContext newDerivedContext(RequestId id, protected RequestTarget validateHeaders(RequestHeaders headers) { // no need to validate since internal headers will contain // the default host and session protocol headers set by endpoints. - return RequestTarget.forClient(headers.path()); + return DefaultRequestTarget.forClientPath(headers.path()); } @Override diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java index 0aacbdf1a9b..fbe628589aa 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/DefaultRequestTarget.java @@ -201,6 +201,18 @@ public static RequestTarget forClient(String reqTarget, @Nullable String prefix) return slowAbsoluteFormForClient(reqTarget, authorityPos); } + return forClientPath(reqTarget, prefix); + } + + @Nullable + public static RequestTarget forClientPath(String reqTarget) { + return forClientPath(reqTarget, null); + } + + @Nullable + public static RequestTarget forClientPath(String reqTarget, @Nullable String prefix) { + requireNonNull(reqTarget, "reqTarget"); + // Concatenate `prefix` and `reqTarget` if necessary. final String actualReqTarget; if (prefix == null || "*".equals(reqTarget)) { @@ -210,7 +222,7 @@ public static RequestTarget forClient(String reqTarget, @Nullable String prefix) actualReqTarget = ArmeriaHttpUtil.concatPaths(prefix, reqTarget); } - final RequestTarget cached = RequestTargetCache.getForClient(actualReqTarget); + final RequestTarget cached = RequestTargetCache.getForClientPath(actualReqTarget); if (cached != null) { return cached; } diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/RequestTargetCache.java b/core/src/main/java/com/linecorp/armeria/internal/common/RequestTargetCache.java index 62fb40ecbf4..746d83b2946 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/RequestTargetCache.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/RequestTargetCache.java @@ -41,6 +41,10 @@ public final class RequestTargetCache { private static final Cache CLIENT_CACHE = Flags.parsedPathCacheSpec() != null ? buildCache(Flags.parsedPathCacheSpec()) : null; + @Nullable + private static final Cache CLIENT_PATH_CACHE = + Flags.parsedPathCacheSpec() != null ? buildCache(Flags.parsedPathCacheSpec()) : null; + private static Cache buildCache(String spec) { return Caffeine.from(spec).build(); } @@ -55,6 +59,10 @@ public static void registerClientMetrics(MeterRegistry registry) { if (CLIENT_CACHE != null) { CaffeineMetricSupport.setup(registry, METER_ID_PREFIX.withTags("type", "client"), CLIENT_CACHE); } + if (CLIENT_PATH_CACHE != null) { + CaffeineMetricSupport.setup(registry, METER_ID_PREFIX.withTags("type", "client.path"), + CLIENT_PATH_CACHE); + } } @Nullable @@ -67,6 +75,11 @@ public static RequestTarget getForClient(String reqTarget) { return get(reqTarget, CLIENT_CACHE); } + @Nullable + public static RequestTarget getForClientPath(String reqTarget) { + return get(reqTarget, CLIENT_PATH_CACHE); + } + @Nullable private static RequestTarget get(String reqTarget, @Nullable Cache cache) { if (cache != null) { @@ -84,6 +97,10 @@ public static void putForClient(String reqTarget, RequestTarget normalized) { put(reqTarget, normalized, CLIENT_CACHE); } + public static void putForClientPath(String reqTarget, RequestTarget normalized) { + put(reqTarget, normalized, CLIENT_PATH_CACHE); + } + private static void put(String reqTarget, RequestTarget normalized, @Nullable Cache cache) { assert reqTarget != null; @@ -123,8 +140,8 @@ public static Set cachedServerPaths() { */ @VisibleForTesting public static Set cachedClientPaths() { - assert CLIENT_CACHE != null : "CLIENT_CACHE"; - return CLIENT_CACHE.asMap().keySet(); + assert CLIENT_PATH_CACHE != null : "CLIENT_PATH_CACHE"; + return CLIENT_PATH_CACHE.asMap().keySet(); } private RequestTargetCache() {} diff --git a/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextBuilderTest.java index e695208ac0c..f77e89ecb9a 100644 --- a/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextBuilderTest.java @@ -50,4 +50,10 @@ void testEndPointGroup() { assertThat(ctx2.endpoint()).isEqualTo(EndpointGroup.of( Endpoint.of("127.0.0.1", 1))); } + + @Test + void testSchemeRelativeUri() { + final ClientRequestContext ctx = ClientRequestContext.of(HttpRequest.of(HttpMethod.GET, "//abc")); + assertThat(ctx.path()).isEqualTo("//abc"); + } } diff --git a/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextTest.java b/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextTest.java index d549701c386..555e7dc2507 100644 --- a/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/ClientRequestContextTest.java @@ -269,19 +269,30 @@ void hasOwnAttr() { } } - @ParameterizedTest - @ValueSource(strings = {"%", "http:///", "http://foo.com/bar"}) - void updateRequestWithInvalidPath(String path) { + @Test + void updateRequestWithInvalidPath() { final ClientRequestContext clientRequestContext = clientRequestContext(); assertThat(clientRequestContext.path()).isEqualTo("/"); final HttpRequest request = - HttpRequest.of(RequestHeaders.of(HttpMethod.GET, path)); + HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "%")); assertThatThrownBy(() -> clientRequestContext.updateRequest(request)) .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("invalid path"); } + @ParameterizedTest + @ValueSource(strings = {"http:///", "http://foo.com/bar"}) + void updateRequestWithAbsolutePath(String path) { + final ClientRequestContext clientRequestContext = clientRequestContext(); + assertThat(clientRequestContext.path()).isEqualTo("/"); + final HttpRequest request = + HttpRequest.of(RequestHeaders.of(HttpMethod.GET, path)); + + clientRequestContext.updateRequest(request); + assertThat(clientRequestContext.path()).isEqualTo('/' + path); + } + @ParameterizedTest @ArgumentsSource(TimedOutExceptionProvider.class) void isTimedOut_true(Throwable cause) { diff --git a/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java b/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java index 9150cca90f9..f5447f6d68c 100644 --- a/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java +++ b/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java @@ -54,6 +54,7 @@ import com.linecorp.armeria.common.util.Unwrappable; import com.linecorp.armeria.internal.client.DefaultClientRequestContext; import com.linecorp.armeria.internal.client.TailPreClient; +import com.linecorp.armeria.internal.common.DefaultRequestTarget; import com.linecorp.armeria.internal.common.RequestTargetCache; import com.linecorp.armeria.internal.common.grpc.InternalGrpcExceptionHandler; import com.linecorp.armeria.internal.common.grpc.StatusAndMetadata; @@ -255,9 +256,9 @@ public T as(Class type) { private DefaultClientRequestContext newContext(HttpMethod method, HttpRequest req, MethodDescriptor methodDescriptor) { final String path = req.path(); - final RequestTarget reqTarget = RequestTarget.forClient(path); + final RequestTarget reqTarget = DefaultRequestTarget.forClientPath(path); assert reqTarget != null : path; - RequestTargetCache.putForClient(path, reqTarget); + RequestTargetCache.putForClientPath(path, reqTarget); final RequestOptions requestOptions = REQUEST_OPTIONS_MAP.get(methodDescriptor.getType()); assert requestOptions != null; diff --git a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/client/thrift/DefaultTHttpClient.java b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/client/thrift/DefaultTHttpClient.java index 2445fa0ef0a..56f6fbed65d 100644 --- a/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/client/thrift/DefaultTHttpClient.java +++ b/thrift/thrift0.13/src/main/java/com/linecorp/armeria/internal/client/thrift/DefaultTHttpClient.java @@ -36,6 +36,7 @@ import com.linecorp.armeria.internal.client.ClientUtil; import com.linecorp.armeria.internal.client.DefaultClientRequestContext; import com.linecorp.armeria.internal.client.TailPreClient; +import com.linecorp.armeria.internal.common.DefaultRequestTarget; import com.linecorp.armeria.internal.common.RequestTargetCache; import io.micrometer.core.instrument.MeterRegistry; @@ -76,14 +77,14 @@ private RpcResponse execute0( path = path + '#' + serviceName; } - final RequestTarget reqTarget = RequestTarget.forClient(path, uri().getRawPath()); + final RequestTarget reqTarget = DefaultRequestTarget.forClientPath(path, uri().getRawPath()); if (reqTarget == null) { return RpcResponse.ofFailure(new TTransportException( new IllegalArgumentException("invalid path: " + path))); } // A thrift path is always good to cache as it cannot have non-fixed parameters. - RequestTargetCache.putForClient(path, reqTarget); + RequestTargetCache.putForClientPath(path, reqTarget); final RpcRequest call = RpcRequest.of(serviceType, method, args); final DefaultClientRequestContext ctx = new DefaultClientRequestContext( diff --git a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java index 3fbf80ef297..2e49c89677a 100644 --- a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java +++ b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java @@ -36,9 +36,12 @@ import com.linecorp.armeria.client.AbstractClientOptionsBuilder; import com.linecorp.armeria.client.ClientBuilderParams; +import com.linecorp.armeria.client.ClientRequestContext; +import com.linecorp.armeria.client.ClientRequestContextCaptor; import com.linecorp.armeria.client.Clients; import com.linecorp.armeria.client.Endpoint; import com.linecorp.armeria.common.HttpRequest; +import com.linecorp.armeria.common.RpcResponse; import com.linecorp.armeria.common.SerializationFormat; import com.linecorp.armeria.common.stream.AbortedStreamException; import com.linecorp.armeria.common.thrift.ThriftSerializationFormats; @@ -194,4 +197,18 @@ void withoutScheme(HelloService.Iface client) throws Exception { assertThat(params.scheme().sessionProtocol()).isEqualTo(defaultSessionProtocol()); assertThat(params.uri().toString()).isEqualTo("tbinary+http://google.com/"); } + + @Test + void doubleSlashPath() throws Exception { + try (ClientRequestContextCaptor captor = Clients.newContextCaptor()) { + final HelloService.Iface iface = + ThriftClients.builder(URI.create("http://1.2.3.4//my-path")) + .rpcDecorator((delegate, ctx, req) -> RpcResponse.of("world")) + .build(HelloService.Iface.class); + assertThat(iface.hello("hello")).isEqualTo("world"); + final ClientRequestContext ctx = captor.get(); + assertThat(ctx.authority()).isEqualTo("1.2.3.4"); + assertThat(ctx.path()).isEqualTo("//my-path"); + } + } } diff --git a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftOverHttpClientTest.java b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftOverHttpClientTest.java index 12248efd0da..a2755945f29 100644 --- a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftOverHttpClientTest.java +++ b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftOverHttpClientTest.java @@ -753,7 +753,7 @@ void endpointMapping( : server.httpPort()); final HelloService.Iface client = ThriftClients.builder(format.uriText() + '+' + protocol.uriText() + - "://my-group" + Handlers.HELLO.path(format)) + "://my-group/" + Handlers.HELLO.path(format)) .options(clientOptions) .endpointRemapper(endpoint -> { if ("my-group".equals(endpoint.host())) { From f9cb6bd77cf21c3ff4c516b1d26336122230e89a Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Thu, 27 Feb 2025 11:55:22 +0900 Subject: [PATCH 5/5] minor fixes --- .../armeria/client/ClientBuilderTest.java | 2 +- .../armeria/client/RedirectingClientTest.java | 44 ++++++++++++--- .../armeria/client/RestClientBuilderTest.java | 2 +- .../armeria/client/WebClientBuilderTest.java | 20 +++++-- .../client/retry/RetryingClientTest.java | 53 ++++++++++++++----- .../websocket/WebSocketClientBuilderTest.java | 2 +- .../client/grpc/GrpcClientBuilderTest.java | 2 +- .../retrofit2/ArmeriaRetrofitBuilderTest.java | 2 +- .../thrift/ThriftClientBuilderTest.java | 2 +- 9 files changed, 98 insertions(+), 31 deletions(-) diff --git a/core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java index 6ad2f669d84..6d0e6cf994e 100644 --- a/core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/ClientBuilderTest.java @@ -58,7 +58,7 @@ void endpointWithPath() { assertThat(client.uri().toString()).isEqualTo("http://127.0.0.1/foo"); } - public static Stream withoutScheme_args() throws Exception { + private static Stream withoutScheme_args() throws Exception { return Stream.of( Arguments.of(Clients.newClient("//google.com", WebClient.class)), Arguments.of(Clients.builder("//google.com").build(WebClient.class)), diff --git a/core/src/test/java/com/linecorp/armeria/client/RedirectingClientTest.java b/core/src/test/java/com/linecorp/armeria/client/RedirectingClientTest.java index fe8ab6c86d5..fe0af4aed8f 100644 --- a/core/src/test/java/com/linecorp/armeria/client/RedirectingClientTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/RedirectingClientTest.java @@ -107,9 +107,9 @@ protected void configure(ServerBuilder sb) throws Exception { "http://foo.com:" + server.httpPort() + "/anotherDomainRedirect")); sb.virtualHost("foo.com") .service("/anotherDomainRedirect", (ctx, req) -> { - assertThat(req.authority()).isEqualTo("foo.com:" + server.server().activeLocalPort()); - return HttpResponse.of(200); - }); + assertThat(req.authority()).isEqualTo("foo.com:" + server.server().activeLocalPort()); + return HttpResponse.of(200); + }); sb.service("/removeDotSegments/foo", (ctx, req) -> HttpResponse.ofRedirect("./bar")) .service("/removeDotSegments/bar", (ctx, req) -> HttpResponse.of(200)) @@ -137,9 +137,9 @@ protected void configure(ServerBuilder sb) throws Exception { "http://domain1.com:" + server.httpPort() + "/authorityLoop")); sb.service("/queryLoop1", (ctx, req) -> { - final String queryParams = ctx.query(); - final String redirectUrl = "/queryLoop2" + (queryParams == null ? "" : '?' + queryParams); - return HttpResponse.ofRedirect(redirectUrl); + final String queryParams = ctx.query(); + final String redirectUrl = "/queryLoop2" + (queryParams == null ? "" : '?' + queryParams); + return HttpResponse.ofRedirect(redirectUrl); }) .service("/queryLoop2", (ctx, req) -> { final String queryParams = ctx.query(); @@ -165,6 +165,12 @@ protected void configure(ServerBuilder sb) throws Exception { return HttpResponse.of(400); } }); + + sb.service("/bar", (ctx, req) -> + HttpResponse.ofRedirect("//barAuthority:" + server.httpPort() + "/barRedirect1")) + .service("/barRedirect1", (ctx, req) -> + HttpResponse.ofRedirect("//barAuthority:" + server.httpPort() + "/barRedirect2")) + .service("/barRedirect2", (ctx, req) -> HttpResponse.of(200)); } private int otherHttpPort(ServiceRequestContext ctx) { @@ -200,7 +206,7 @@ void redirect_failExceedingTotalAttempts() { private static AggregatedHttpResponse sendRequest(int maxRedirects) { final WebClient client = WebClient.builder(server.httpUri()) - .decorator(LoggingClient.newDecorator()) + .decorator(LoggingClient.newDecorator()) .followRedirects(RedirectConfig.builder() .maxRedirects(maxRedirects) .build()) @@ -337,7 +343,7 @@ void cyclicRedirectsException(String originalPath, List expectedPathRege final String message = exception.getMessage(); // All URIs have a port number. expectedPathRegexPatterns.forEach(pattern -> - assertThat(message).containsPattern(pattern)); + assertThat(message).containsPattern(pattern)); }); } } @@ -416,6 +422,28 @@ void unencodedLocation() { assertThat(log2.context().uri().toString()).endsWith("/unencodedLocation/foo%20bar?value=$%7BP%7D"); } + @Test + void redirectRelativeScheme() { + try (MockAddressResolverGroup group = MockAddressResolverGroup.of(hostname -> { + if ("barAuthority".equalsIgnoreCase(hostname)) { + return server.httpSocketAddress().getAddress(); + } + throw new RuntimeException(); + }); + ClientFactory cf = ClientFactory.builder() + .addressResolverGroupFactory(ignored -> group) + .build()) { + final BlockingWebClient client = WebClient.builder(server.httpUri()) + .factory(cf) + .decorator(LoggingClient.newDecorator()) + .followRedirects(RedirectConfig.builder() + .allowAllDomains() + .build()) + .build().blocking(); + assertThat(client.get("/bar").status().code()).isEqualTo(200); + } + } + @Test void testResolveLocation() { // Absolute paths and URIs should supersede the original path. diff --git a/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java index feb64b51191..3607a77c4cb 100644 --- a/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/RestClientBuilderTest.java @@ -28,7 +28,7 @@ class RestClientBuilderTest { - public static Stream withoutScheme_args() throws Exception { + private static Stream withoutScheme_args() throws Exception { return Stream.of( Arguments.of(RestClient.of("//google.com")), Arguments.of(RestClient.builder("//google.com").build()), diff --git a/core/src/test/java/com/linecorp/armeria/client/WebClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/WebClientBuilderTest.java index d848641aaec..a08a48d51d5 100644 --- a/core/src/test/java/com/linecorp/armeria/client/WebClientBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/WebClientBuilderTest.java @@ -87,7 +87,7 @@ void uriWithoutNone() { assertThat(client.uri().toString()).isEqualTo("https://google.com/"); } - public static Stream withoutScheme_args() throws Exception { + private static Stream withoutScheme_args() throws Exception { return Stream.of( Arguments.of(WebClient.of("//google.com")), Arguments.of(WebClient.builder("//google.com").build()), @@ -107,9 +107,11 @@ void withoutScheme(WebClient client) { assertThat(client.uri().toString()).isEqualTo("http://google.com/"); } - public static Stream defaultWithoutScheme_args() throws Exception { + private static Stream defaultWithoutScheme_args() throws Exception { final String authority = server.httpUri().getAuthority(); return Stream.of( + Arguments.of(HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", + HttpHeaderNames.AUTHORITY, authority))), Arguments.of(HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "/hello", HttpHeaderNames.AUTHORITY, authority))), Arguments.of(HttpRequest.of(HttpMethod.GET, "//" + authority + "/hello")) @@ -123,13 +125,25 @@ void defaultWithoutScheme(HttpRequest req) { } @Test - void endpointGroupWithSchemeRelativeUri() { + void schemeRelativeUriForPath() { + // https://datatracker.ietf.org/doc/html/rfc3986#section-3.3 + // path-absolute ; begins with "/" but not "//" + final WebClient webClient = WebClient.of(SessionProtocol.HTTP, server.endpoint(SessionProtocol.HTTP)); assertThatThrownBy(() -> webClient.get("//relative-uri").aggregate().join()) .isInstanceOf(CompletionException.class) .cause() .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("Cannot send a request with a \":path\" header"); + + final HttpRequest req = + HttpRequest.of(RequestHeaders.of(HttpMethod.GET, "//relative-uri", + HttpHeaderNames.AUTHORITY, server.httpUri().getAuthority())); + assertThatThrownBy(() -> webClient.execute(req).aggregate().join()) + .isInstanceOf(CompletionException.class) + .cause() + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Cannot send a request with a \":path\" header"); } @Test diff --git a/core/src/test/java/com/linecorp/armeria/client/retry/RetryingClientTest.java b/core/src/test/java/com/linecorp/armeria/client/retry/RetryingClientTest.java index 38dc858d780..d33bfafcb91 100644 --- a/core/src/test/java/com/linecorp/armeria/client/retry/RetryingClientTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/retry/RetryingClientTest.java @@ -53,6 +53,7 @@ import com.google.common.base.Stopwatch; +import com.linecorp.armeria.client.BlockingWebClient; import com.linecorp.armeria.client.ClientFactory; import com.linecorp.armeria.client.ClientRequestContext; import com.linecorp.armeria.client.HttpClient; @@ -102,16 +103,16 @@ static void afterAll() { clientFactory.closeAsync(); } - private final AtomicInteger responseAbortServiceCallCounter = new AtomicInteger(); + private static final AtomicInteger responseAbortServiceCallCounter = new AtomicInteger(); - private final AtomicInteger requestAbortServiceCallCounter = new AtomicInteger(); + private static final AtomicInteger requestAbortServiceCallCounter = new AtomicInteger(); - private final AtomicInteger subscriberCancelServiceCallCounter = new AtomicInteger(); + private static final AtomicInteger subscriberCancelServiceCallCounter = new AtomicInteger(); - private AtomicInteger reqCount; + private static AtomicInteger reqCount; @RegisterExtension - final ServerExtension server = new ServerExtension() { + static final ServerExtension server = new ServerExtension() { @Override protected boolean runForEachTest() { return true; @@ -311,12 +312,27 @@ protected HttpResponse doGet(ServiceRequestContext ctx, HttpRequest req) } } }); + + sb.service("/relative-uri", new AbstractHttpService() { + @Override + protected HttpResponse doGet(ServiceRequestContext ctx, HttpRequest req) + throws Exception { + if (reqCount.getAndIncrement() < 2) { + return HttpResponse.of(HttpStatus.SERVICE_UNAVAILABLE); + } else { + return HttpResponse.of("Succeeded after retry"); + } + } + }); } }; @BeforeEach void setUp() { reqCount = new AtomicInteger(); + requestAbortServiceCallCounter.set(0); + responseAbortServiceCallCounter.set(0); + subscriberCancelServiceCallCounter.set(0); } @Test @@ -526,15 +542,15 @@ void honorRetryMapping() { void evaluatesMappingOnce() { final AtomicInteger evaluations = new AtomicInteger(0); final RetryConfigMapping mapping = - (ctx, req) -> { - evaluations.incrementAndGet(); - return RetryConfig - .builder0(RetryRule.builder() - .onStatus(HttpStatus.valueOf(500)) - .thenBackoff()) - .maxTotalAttempts(2) - .build(); - }; + (ctx, req) -> { + evaluations.incrementAndGet(); + return RetryConfig + .builder0(RetryRule.builder() + .onStatus(HttpStatus.valueOf(500)) + .thenBackoff()) + .maxTotalAttempts(2) + .build(); + }; final WebClient client = client(mapping); @@ -835,6 +851,15 @@ void useSameEventLoopWhenAggregate() throws InterruptedException { latch.await(); } + @Test + void retryWithSchemeRelativeUri() { + final RetryRule rule = RetryRule.builder().onServerErrorStatus().onException().thenBackoff(); + final BlockingWebClient client = WebClient.builder() + .decorator(RetryingClient.newDecorator(rule)) + .build().blocking(); + assertThat(client.get(server.httpUri() + "//relative-uri").status().code()).isEqualTo(200); + } + private WebClient client(RetryRule retryRule) { return client(retryRule, 10000, 0, 100); } diff --git a/core/src/test/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilderTest.java b/core/src/test/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilderTest.java index ed92ab51944..2372d9c5606 100644 --- a/core/src/test/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilderTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/websocket/WebSocketClientBuilderTest.java @@ -115,7 +115,7 @@ void webSocketClientDefaultOptions() { assertThat(client.options().get(ClientOptions.AUTO_FILL_ORIGIN_HEADER)).isTrue(); } - public static Stream withoutScheme_args() throws Exception { + private static Stream withoutScheme_args() throws Exception { return Stream.of( Arguments.of(WebSocketClient.of("//google.com")), Arguments.of(WebSocketClient.builder("//google.com").build()), diff --git a/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java b/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java index 82bab5e3cc8..936b5ecc8ce 100644 --- a/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java @@ -314,7 +314,7 @@ void useDefaultGrpcExceptionHandlerFunctionAsFallback() { .isEqualTo(Code.RESOURCE_EXHAUSTED); } - public static Stream withoutScheme_args() throws Exception { + private static Stream withoutScheme_args() throws Exception { return Stream.of( Arguments.of(GrpcClients.newClient("//google.com", TestServiceBlockingStub.class)), Arguments.of(GrpcClients.builder("//google.com").build(TestServiceBlockingStub.class)), diff --git a/retrofit2/src/test/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofitBuilderTest.java b/retrofit2/src/test/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofitBuilderTest.java index 8bfc56c662e..3b49e5dc674 100644 --- a/retrofit2/src/test/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofitBuilderTest.java +++ b/retrofit2/src/test/java/com/linecorp/armeria/client/retrofit2/ArmeriaRetrofitBuilderTest.java @@ -179,7 +179,7 @@ void streamingResponseIsCompleteWhenVoidReturnType() { await().until(future::isDone); } - public static Stream withoutScheme_args() throws Exception { + private static Stream withoutScheme_args() throws Exception { return Stream.of( Arguments.of(ArmeriaRetrofit.of("//google.com")), Arguments.of(ArmeriaRetrofit.builder("//google.com").build()), diff --git a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java index 2e49c89677a..a8bbf278907 100644 --- a/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java +++ b/thrift/thrift0.13/src/test/java/com/linecorp/armeria/client/thrift/ThriftClientBuilderTest.java @@ -176,7 +176,7 @@ void apiConsistency() { } } - public static Stream withoutScheme_args() throws Exception { + private static Stream withoutScheme_args() throws Exception { return Stream.of( Arguments.of(ThriftClients.newClient("//google.com", HelloService.Iface.class)), Arguments.of(ThriftClients.builder("//google.com").build(HelloService.Iface.class)),