Skip to content

Commit

Permalink
Handle a fragment in a client-side URI
Browse files Browse the repository at this point in the history 
WIP
  • Loading branch information
@trustin
trustin committed Mar 30, 2023
1 parent 787467f commit 123224c
Show file tree
Hide file tree
Showing 47 changed files with 1,307 additions and 683 deletions.
26 changes: 14 additions & 12 deletions benchmarks/jmh/src/jmh/java/com/linecorp/armeria/internal/common/PathParsingBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.infra.Blackhole;

import com.linecorp.armeria.common.RequestTarget;

/**
* Microbenchmarks for the {@link PathAndQuery#parse(String)} method.
* Microbenchmarks for the {@link RequestTarget#forServer(String)} method.
*/
@State(Scope.Thread)
public class PathParsingBenchmark {
Expand All @@ -42,37 +44,37 @@ public void setUp() {
}

@Benchmark
public PathAndQuery normal() {
public RequestTarget normal() {
return doNormal();
}

@Benchmark
@Fork(jvmArgsAppend = "-Dcom.linecorp.armeria.parsedPathCache=off")
public PathAndQuery normal_cacheDisabled() {
public RequestTarget normal_cacheDisabled() {
return doNormal();
}

private PathAndQuery doNormal() {
final PathAndQuery parsed = PathAndQuery.parse(path1);
parsed.storeInCache(path1);
private RequestTarget doNormal() {
final RequestTarget parsed = RequestTarget.forServer(path1);
RequestTargetCache.putForServer(path1, parsed);
return parsed;
}

@Benchmark
public PathAndQuery cachedAndNotCached(Blackhole bh) {
public RequestTarget cachedAndNotCached(Blackhole bh) {
return doCachedAndNotCached(bh);
}

@Benchmark
@Fork(jvmArgsAppend = "-Dcom.linecorp.armeria.parsedPathCache=off")
public PathAndQuery cachedAndNotCached_cacheDisabled(Blackhole bh) {
public RequestTarget cachedAndNotCached_cacheDisabled(Blackhole bh) {
return doCachedAndNotCached(bh);
}

private PathAndQuery doCachedAndNotCached(Blackhole bh) {
final PathAndQuery parsed = PathAndQuery.parse(path1);
parsed.storeInCache(path1);
final PathAndQuery parsed2 = PathAndQuery.parse(path2);
private RequestTarget doCachedAndNotCached(Blackhole bh) {
final RequestTarget parsed = RequestTarget.forServer(path1);
RequestTargetCache.putForServer(path1, parsed);
final RequestTarget parsed2 = RequestTarget.forServer(path2);
bh.consume(parsed2);
return parsed;
}
Expand Down
20 changes: 12 additions & 8 deletions benchmarks/jmh/src/jmh/java/com/linecorp/armeria/server/RoutersBenchmark.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.common.RequestHeaders;
import com.linecorp.armeria.common.RequestId;
import com.linecorp.armeria.common.RequestTarget;
import com.linecorp.armeria.common.SuccessFunction;
import com.linecorp.armeria.server.logging.AccessLogWriter;

Expand All @@ -48,6 +49,9 @@ public class RoutersBenchmark {
private static final RequestHeaders METHOD1_HEADERS =
RequestHeaders.of(HttpMethod.POST, "/grpc.package.Service/Method1");

private static final RequestTarget METHOD1_REQ_TARGET =
RequestTarget.forServer(METHOD1_HEADERS.path());

static {
final String defaultLogName = null;
final String defaultServiceName = null;
Expand All @@ -60,32 +64,32 @@ public class RoutersBenchmark {
SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
SuccessFunction.always(), multipartUploadsLocation, ImmutableList.of(),
HttpHeaders.of(), (ctx) -> RequestId.random()),
HttpHeaders.of(), ctx -> RequestId.random()),
new ServiceConfig(route2, route2,
SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
SuccessFunction.always(), multipartUploadsLocation, ImmutableList.of(),
HttpHeaders.of(), (ctx) -> RequestId.random())
HttpHeaders.of(), ctx -> RequestId.random())
);
FALLBACK_SERVICE = new ServiceConfig(Route.ofCatchAll(), Route.ofCatchAll(), SERVICE,
defaultLogName, defaultServiceName,
defaultServiceNaming, 0, 0, false, AccessLogWriter.disabled(),
CommonPools.blockingTaskExecutor(),
SuccessFunction.always(), multipartUploadsLocation,
ImmutableList.of(), HttpHeaders.of(), (ctx) -> RequestId.random());
ImmutableList.of(), HttpHeaders.of(), ctx -> RequestId.random());
HOST = new VirtualHost(
"localhost", "localhost", 0, null, SERVICES, FALLBACK_SERVICE, RejectedRouteHandler.DISABLED,
unused -> NOPLogger.NOP_LOGGER, defaultServiceNaming, 0, 0, false,
AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(), multipartUploadsLocation,
ImmutableList.of(),
(ctx) -> RequestId.random());
ctx -> RequestId.random());
ROUTER = Routers.ofVirtualHost(HOST, SERVICES, RejectedRouteHandler.DISABLED);
}

@Benchmark
public Routed<ServiceConfig> exactMatch() {
final RoutingContext ctx = DefaultRoutingContext.of(HOST, "localhost", METHOD1_HEADERS.path(),
null, METHOD1_HEADERS, RoutingStatus.OK);
final RoutingContext ctx = DefaultRoutingContext.of(HOST, "localhost", METHOD1_REQ_TARGET,
METHOD1_HEADERS, RoutingStatus.OK);
final Routed<ServiceConfig> routed = ROUTER.find(ctx);
if (routed.value() != SERVICES.get(0)) {
throw new IllegalStateException("Routing error");
Expand All @@ -96,8 +100,8 @@ public Routed<ServiceConfig> exactMatch() {
@Benchmark
public Routed<ServiceConfig> exactMatch_wrapped() {
final RoutingContext ctx = new RoutingContextWrapper(
DefaultRoutingContext.of(HOST, "localhost", METHOD1_HEADERS.path(),
null, METHOD1_HEADERS, RoutingStatus.OK));
DefaultRoutingContext.of(HOST, "localhost", METHOD1_REQ_TARGET,
METHOD1_HEADERS, RoutingStatus.OK));
final Routed<ServiceConfig> routed = ROUTER.find(ctx);
if (routed.value() != SERVICES.get(0)) {
throw new IllegalStateException("Routing error");
Expand Down
6 changes: 1 addition & 5 deletions core/src/main/java/com/linecorp/armeria/client/ClientRequestContextBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ public void run(Throwable cause) { /* no-op */ }
noopResponseCancellationScheduler.finishNow();
}

@Nullable
private final String fragment;
@Nullable
private Endpoint endpoint;
private ClientOptions options = ClientOptions.of();
Expand All @@ -81,12 +79,10 @@ public void run(Throwable cause) { /* no-op */ }

ClientRequestContextBuilder(HttpRequest request) {
super(false, request);
fragment = null;
}

ClientRequestContextBuilder(RpcRequest request, URI uri) {
super(false, request, uri);
fragment = uri.getRawFragment();
}

@Override
Expand Down Expand Up @@ -157,7 +153,7 @@ public ClientRequestContext build() {

final DefaultClientRequestContext ctx = new DefaultClientRequestContext(
eventLoop(), meterRegistry(), sessionProtocol(),
id(), method(), path(), query(), fragment, options, request(), rpcRequest(),
id(), method(), requestTarget(), options, request(), rpcRequest(),
requestOptions, responseCancellationScheduler,
isRequestStartTimeSet() ? requestStartTimeNanos() : System.nanoTime(),
isRequestStartTimeSet() ? requestStartTimeMicros() : SystemInfo.currentTimeMicros());
Expand Down
99 changes: 49 additions & 50 deletions core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,16 @@

package com.linecorp.armeria.client;

import static com.linecorp.armeria.internal.client.ClientUtil.pathWithQuery;
import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.concatPaths;
import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.isAbsoluteUri;
import static java.util.Objects.requireNonNull;

import java.net.URI;

import com.linecorp.armeria.client.endpoint.EndpointGroup;
import com.linecorp.armeria.common.ExchangeType;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.RequestTarget;
import com.linecorp.armeria.common.RequestTargetForm;
import com.linecorp.armeria.common.Scheme;
import com.linecorp.armeria.common.SessionProtocol;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.internal.common.PathAndQuery;

import io.micrometer.core.instrument.MeterRegistry;

Expand Down Expand Up @@ -58,69 +53,73 @@ public HttpResponse execute(HttpRequest req, RequestOptions requestOptions) {
requireNonNull(req, "req");
requireNonNull(requestOptions, "requestOptions");

final String originalPath = req.path();
final RequestTarget reqTarget = RequestTarget.forClient(originalPath, uri().getRawPath());
if (reqTarget == null) {
return abortRequestAndReturnFailureResponse(
req, new IllegalArgumentException("Invalid path: " + originalPath));
}

if (Clients.isUndefinedUri(uri())) {
final URI uri;
if (isAbsoluteUri(req.path())) {
try {
uri = URI.create(req.path());
} catch (Exception ex) {
final String scheme;
final String authority;
if (reqTarget.form() == RequestTargetForm.ABSOLUTE) {
scheme = reqTarget.scheme();
authority = reqTarget.authority();
assert scheme != null;
assert authority != null;
} else {
scheme = req.scheme();
authority = req.authority();

if (scheme == null || authority == null) {
return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException(
"Failed to create a URI: " + req.path(), ex));
"Scheme and authority must be specified in \":path\" or " +
"in \":scheme\" and \":authority\". :path=" +
originalPath + ", :scheme=" + req.scheme() + ", :authority=" + req.authority()));
}
} else if (req.scheme() != null && req.authority() != null) {
uri = req.uri();
} else {
return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException(
"Scheme and authority must be specified in \":path\" or " +
"in \":scheme\" and \":authority\". :path=" +
req.path() + ", :scheme=" + req.scheme() + ", :authority=" + req.authority()));
}

final SessionProtocol protocol;
try {
protocol = Scheme.parse(uri.getScheme()).sessionProtocol();
protocol = Scheme.parse(scheme).sessionProtocol();
} catch (Exception e) {
return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException(
"Failed to parse a scheme: " + uri.getScheme(), e));
"Failed to parse a scheme: " + reqTarget.scheme(), e));
}

final Endpoint endpoint = Endpoint.parse(uri.getAuthority());
final String query = uri.getRawQuery();
final String path = pathWithQuery(uri, query);
final HttpRequest newReq = req.withHeaders(req.headers().toBuilder().path(path));
return execute(endpoint, newReq, protocol, requestOptions);
final Endpoint endpoint = Endpoint.parse(authority);
final HttpRequest newReq = req.withHeaders(req.headers()
.toBuilder()
.path(reqTarget.pathAndQuery()));
return execute(protocol,
endpoint,
newReq.method(),
reqTarget,
newReq,
requestOptions);
}

if (isAbsoluteUri(req.path())) {
if (reqTarget.form() == RequestTargetForm.ABSOLUTE) {
return abortRequestAndReturnFailureResponse(req, new IllegalArgumentException(
"Cannot send a request with a \":path\" header that contains a URI with the authority, " +
"because the client was created with a base URI. path: " + req.path()));
"because the client was created with a base URI. path: " + originalPath));
}

final String originalPath = req.path();
final String newPath = concatPaths(uri().getRawPath(), originalPath);
final String newPath = reqTarget.pathAndQuery();
final HttpRequest newReq;
// newPath and originalPath should be the same reference if uri().getRawPath() can be ignorable
if (newPath != originalPath) {
newReq = req.withHeaders(req.headers().toBuilder().path(newPath));
} else {
if (newPath.equals(originalPath)) {
newReq = req;
} else {
newReq = req.withHeaders(req.headers().toBuilder().path(newPath));
}
return execute(endpointGroup(), newReq, scheme().sessionProtocol(), requestOptions);
}

private HttpResponse execute(EndpointGroup endpointGroup, HttpRequest req, SessionProtocol protocol,
RequestOptions requestOptions) {
final PathAndQuery pathAndQuery = PathAndQuery.parse(req.path());
if (pathAndQuery == null) {
final IllegalArgumentException cause = new IllegalArgumentException("invalid path: " + req.path());
return abortRequestAndReturnFailureResponse(req, cause);
}
final String newPath = pathAndQuery.toString();
if (!newPath.equals(req.path())) {
req = req.withHeaders(req.headers().toBuilder().path(newPath));
}
return execute(protocol, endpointGroup, req.method(),
pathAndQuery.path(), pathAndQuery.query(), null, req, requestOptions);
return execute(scheme().sessionProtocol(),
endpointGroup(),
newReq.method(),
reqTarget,
newReq,
requestOptions);
}

private static HttpResponse abortRequestAndReturnFailureResponse(
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.linecorp.armeria.client.proxy.ProxyType;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.RequestTarget;
import com.linecorp.armeria.common.SessionProtocol;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.logging.ClientConnectionTimings;
Expand All @@ -38,7 +39,6 @@
import com.linecorp.armeria.internal.client.DecodedHttpResponse;
import com.linecorp.armeria.internal.client.HttpSession;
import com.linecorp.armeria.internal.client.PooledChannel;
import com.linecorp.armeria.internal.common.PathAndQuery;
import com.linecorp.armeria.internal.common.RequestContextUtil;
import com.linecorp.armeria.server.ProxiedAddresses;
import com.linecorp.armeria.server.ServiceRequestContext;
Expand Down Expand Up @@ -222,7 +222,7 @@ private static void logSession(ClientRequestContext ctx, @Nullable PooledChannel
}

private static boolean isValidPath(HttpRequest req) {
return PathAndQuery.parse(req.path()) != null;
return RequestTarget.forClient(req.path()) != null;
}

private static HttpResponse earlyFailedResponse(Throwable t, ClientRequestContext ctx, HttpRequest req) {
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/com/linecorp/armeria/client/HttpClientFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import com.linecorp.armeria.common.util.ReleasableHolder;
import com.linecorp.armeria.common.util.ShutdownHooks;
import com.linecorp.armeria.common.util.TransportType;
import com.linecorp.armeria.internal.common.RequestTargetCache;
import com.linecorp.armeria.internal.common.util.SslContextUtil;

import io.micrometer.core.instrument.MeterRegistry;
Expand Down Expand Up @@ -166,6 +167,7 @@ final class HttpClientFactory implements ClientFactory {
this.options = options;

clientDelegate = new HttpClientDelegate(this, addressResolverGroup);
RequestTargetCache.registerClientMetrics(meterRegistry);
}

/**
Expand Down
Loading

0 comments on commit 123224c

Please sign in to comment.