Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a way to unwrap RequestContexts #4308

Merged
merged 26 commits into from
Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
0ae3cc8
make requestcontext unwrappable
jrhee17 May 27, 2022
652a16f
modify unwrap to peel only a single layer
jrhee17 May 31, 2022
88faa43
also add a test for client unwrapping
jrhee17 Jun 22, 2022
0230f5f
introduce extension classes for request context classes
jrhee17 Jun 22, 2022
c6dccec
add test case for service as
jrhee17 Jun 22, 2022
c2a203d
modify inheritance hierarchy
jrhee17 Jun 22, 2022
0366c8e
fix compile error
jrhee17 Jun 22, 2022
a4da669
try moving DefaultClientRequestContext to internal
jrhee17 Jun 22, 2022
a5c51f9
move the other abstract implementations to internal
jrhee17 Jun 23, 2022
aafa8f4
minor cleanups
jrhee17 Jun 23, 2022
720a942
remove unnecessary unstableapi
jrhee17 Jun 24, 2022
7c31e56
Merge branch 'master' of github.com:line/armeria into feature/unwrapp…
jrhee17 Jul 1, 2022
0d867c8
clientExtension also extends extension
jrhee17 Jul 1, 2022
d623c2f
Merge branch 'master' of github.com:line/armeria into feature/unwrapp…
jrhee17 Jul 1, 2022
2dbabe9
Merge branch 'master' of github.com:line/armeria into feature/unwrapp…
jrhee17 Jul 5, 2022
e099f31
retrying client now unwraps instead of force casting
jrhee17 Jul 6, 2022
86a3871
Merge branch 'master' of github.com:line/armeria into feature/unwrapp…
jrhee17 Jul 6, 2022
259e64d
remove another instance of force cast
jrhee17 Jul 6, 2022
986662d
address comments by @ikhoon
jrhee17 Jul 6, 2022
50aa23d
address lint failures
jrhee17 Jul 6, 2022
48d4c27
Merge branch 'master' of github.com:line/armeria into feature/unwrapp…
jrhee17 Jul 13, 2022
061c46b
address comments by @minwoox
jrhee17 Jul 18, 2022
3e6c62b
Merge branch 'master' into pr-4308@jrhee17+feature/unwrappable-context
ikhoon Aug 4, 2022
2dc4433
override unwrapAll for RequestContextWrapper
jrhee17 Aug 4, 2022
8998f91
RequestContext also overrides unwrapAll
jrhee17 Aug 4, 2022
32d7692
override unwrapAll return types
jrhee17 Aug 4, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.slf4j.LoggerFactory;

import com.linecorp.armeria.client.ClientRequestContext;
import com.linecorp.armeria.client.DefaultClientRequestContext;
import com.linecorp.armeria.client.HttpClient;
import com.linecorp.armeria.client.SimpleDecoratingHttpClient;
import com.linecorp.armeria.common.HttpRequest;
Expand All @@ -34,6 +33,7 @@
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.brave.RequestContextCurrentTraceContext;
import com.linecorp.armeria.common.logging.ClientConnectionTimings;
import com.linecorp.armeria.internal.common.RequestContextExtension;
import com.linecorp.armeria.internal.common.brave.SpanTags;

import brave.Span;
Expand Down Expand Up @@ -121,10 +121,10 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Ex
req = req.withHeaders(newHeaders);
ctx.updateRequest(req);

if (currentTraceContext != null && !span.isNoop() && ctx instanceof DefaultClientRequestContext) {
final DefaultClientRequestContext defaultCtx = (DefaultClientRequestContext) ctx;
final RequestContextExtension ctxExtension = ctx.as(RequestContextExtension.class);
if (currentTraceContext != null && !span.isNoop() && ctxExtension != null) {
// Make the span the current span and run scope decorators when the ctx is pushed.
defaultCtx.hook(() -> currentTraceContext.newScope(span.context()));
ctxExtension.hook(() -> currentTraceContext.newScope(span.context()));
}

maybeAddTagsToSpan(ctx, braveReq, span);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.brave.RequestContextCurrentTraceContext;
import com.linecorp.armeria.internal.common.RequestContextExtension;
import com.linecorp.armeria.internal.common.brave.SpanTags;
import com.linecorp.armeria.server.DefaultServiceRequestContext;
import com.linecorp.armeria.server.HttpService;
import com.linecorp.armeria.server.ServiceRequestContext;
import com.linecorp.armeria.server.SimpleDecoratingHttpService;
Expand Down Expand Up @@ -100,11 +100,10 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc
final HttpServerRequest braveReq = ServiceRequestContextAdapter.asHttpServerRequest(ctx);
final Span span = handler.handleReceive(braveReq);

if (currentTraceContext.scopeDecoratorAdded() && !span.isNoop() &&
ctx instanceof DefaultServiceRequestContext) {
final DefaultServiceRequestContext defaultCtx = (DefaultServiceRequestContext) ctx;
final RequestContextExtension ctxExtension = ctx.as(RequestContextExtension.class);
if (currentTraceContext.scopeDecoratorAdded() && !span.isNoop() && ctxExtension != null) {
// Run the scope decorators when the ctx is pushed to the thread local.
defaultCtx.hook(() -> currentTraceContext.decorateScope(span.context(),
ctxExtension.hook(() -> currentTraceContext.decorateScope(span.context(),
SERVICE_REQUEST_DECORATING_SCOPE));
minwoox marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.logging.ClientConnectionTimings;
import com.linecorp.armeria.common.util.SystemInfo;
import com.linecorp.armeria.internal.client.DefaultClientRequestContext;
import com.linecorp.armeria.internal.common.CancellationScheduler;
import com.linecorp.armeria.internal.common.CancellationScheduler.CancellationTask;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.util.SafeCloseable;
import com.linecorp.armeria.common.util.Unwrappable;
import com.linecorp.armeria.internal.client.ClientThreadLocalState;

/**
* Creates a new client that connects to a specified {@link URI}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@

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.google.common.base.Strings;

import com.linecorp.armeria.client.endpoint.EndpointGroup;
import com.linecorp.armeria.common.ExchangeType;
import com.linecorp.armeria.common.HttpRequest;
Expand Down Expand Up @@ -150,14 +149,4 @@ public RestClient asRestClient() {
public HttpClient unwrap() {
return (HttpClient) super.unwrap();
}

static String pathWithQuery(URI uri, @Nullable String query) {
String path = uri.getRawPath();
if (Strings.isNullOrEmpty(path)) {
path = query == null ? "/" : "/?" + query;
} else if (query != null) {
path = path + '?' + query;
}
return path;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.linecorp.armeria.common.stream.CancelledSubscriptionException;
import com.linecorp.armeria.common.stream.StreamWriter;
import com.linecorp.armeria.common.util.Exceptions;
import com.linecorp.armeria.internal.client.ClientRequestContextExtension;
import com.linecorp.armeria.internal.common.CancellationScheduler;
import com.linecorp.armeria.internal.common.CancellationScheduler.CancellationTask;
import com.linecorp.armeria.internal.common.InboundTrafficController;
Expand Down Expand Up @@ -354,9 +355,11 @@ private void cancelAction(@Nullable Throwable cause) {
private void cancelTimeoutOrLog(@Nullable Throwable cause, boolean cancel) {

CancellationScheduler responseCancellationScheduler = null;
if (ctx instanceof DefaultClientRequestContext) {
responseCancellationScheduler =
((DefaultClientRequestContext) ctx).responseCancellationScheduler();
if (ctx != null) {
final ClientRequestContextExtension ctxExtension = ctx.as(ClientRequestContextExtension.class);
if (ctxExtension != null) {
responseCancellationScheduler = ctxExtension.responseCancellationScheduler();
}
}

if (responseCancellationScheduler == null || !responseCancellationScheduler.isFinished()) {
Expand Down Expand Up @@ -398,9 +401,13 @@ private void cancelTimeoutOrLog(@Nullable Throwable cause, boolean cancel) {
}

void initTimeout() {
if (ctx instanceof DefaultClientRequestContext) {
if (ctx == null) {
return;
}
final ClientRequestContextExtension ctxExtension = ctx.as(ClientRequestContextExtension.class);
if (ctxExtension != null) {
final CancellationScheduler responseCancellationScheduler =
((DefaultClientRequestContext) ctx).responseCancellationScheduler();
ctxExtension.responseCancellationScheduler();
responseCancellationScheduler.init(
ctx.eventLoop(), newCancellationTask(),
TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis), /* server */ false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.util.AbstractUnwrappable;
import com.linecorp.armeria.common.util.SystemInfo;
import com.linecorp.armeria.internal.client.DefaultClientRequestContext;

import io.micrometer.core.instrument.MeterRegistry;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import com.linecorp.armeria.common.logging.RequestLogAccess;
import com.linecorp.armeria.common.logging.RequestLogBuilder;
import com.linecorp.armeria.common.util.SafeCloseable;
import com.linecorp.armeria.common.util.Unwrappable;
import com.linecorp.armeria.internal.common.JavaVersionSpecific;
import com.linecorp.armeria.internal.common.RequestContextUtil;
import com.linecorp.armeria.server.ServiceRequestContext;
Expand All @@ -61,7 +62,7 @@
* A server-side {@link Request} has a {@link ServiceRequestContext} and
* a client-side {@link Request} has a {@link ClientRequestContext}.
*/
public interface RequestContext {
public interface RequestContext extends Unwrappable {
jrhee17 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Returns the context of the {@link Request} that is being handled in the current thread.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.logging.RequestLogAccess;
import com.linecorp.armeria.common.logging.RequestLogBuilder;
import com.linecorp.armeria.common.util.AbstractUnwrappable;
import com.linecorp.armeria.server.ServiceRequestContext;

import io.micrometer.core.instrument.MeterRegistry;
Expand All @@ -38,180 +39,178 @@
*
* @param <T> the self type
*/
public abstract class RequestContextWrapper<T extends RequestContext> implements RequestContext {

private final T delegate;

public abstract class RequestContextWrapper<T extends RequestContext>
extends AbstractUnwrappable<T> implements RequestContext {
/**
* Creates a new instance.
*/
protected RequestContextWrapper(T delegate) {
this.delegate = requireNonNull(delegate, "delegate");
super(requireNonNull(delegate, "delegate"));
}

/**
* Returns the delegate context.
*/
protected final T delegate() {
jrhee17 marked this conversation as resolved.
Show resolved Hide resolved
return delegate;
return unwrap();
}

@Nullable
@Override
public ServiceRequestContext root() {
return delegate().root();
return unwrap().root();
}

@Nullable
@Override
public <V> V attr(AttributeKey<V> key) {
return delegate().attr(key);
return unwrap().attr(key);
}

@Nullable
@Override
public <V> V ownAttr(AttributeKey<V> key) {
return delegate().ownAttr(key);
return unwrap().ownAttr(key);
}

@Override
public boolean hasAttr(AttributeKey<?> key) {
return delegate().hasAttr(key);
return unwrap().hasAttr(key);
}

@Override
public boolean hasOwnAttr(AttributeKey<?> key) {
return delegate().hasOwnAttr(key);
return unwrap().hasOwnAttr(key);
}

@Override
public Iterator<Entry<AttributeKey<?>, Object>> attrs() {
return delegate().attrs();
return unwrap().attrs();
}

@Override
public Iterator<Entry<AttributeKey<?>, Object>> ownAttrs() {
return delegate().ownAttrs();
return unwrap().ownAttrs();
}

@Override
public <V> V setAttr(AttributeKey<V> key, @Nullable V value) {
return delegate().setAttr(key, value);
return unwrap().setAttr(key, value);
}

@Override
public HttpRequest request() {
return delegate().request();
return unwrap().request();
}

@Nullable
@Override
public RpcRequest rpcRequest() {
return delegate().rpcRequest();
return unwrap().rpcRequest();
}

@Override
public void updateRequest(HttpRequest req) {
delegate().updateRequest(req);
unwrap().updateRequest(req);
}

@Override
public void updateRpcRequest(RpcRequest rpcReq) {
delegate().updateRpcRequest(rpcReq);
unwrap().updateRpcRequest(rpcReq);
}

@Override
public SessionProtocol sessionProtocol() {
return delegate().sessionProtocol();
return unwrap().sessionProtocol();
}

@Nullable
@Override
public <A extends SocketAddress> A remoteAddress() {
return delegate().remoteAddress();
return unwrap().remoteAddress();
}

@Nullable
@Override
public <A extends SocketAddress> A localAddress() {
return delegate().localAddress();
return unwrap().localAddress();
}

@Nullable
@Override
public SSLSession sslSession() {
return delegate().sslSession();
return unwrap().sslSession();
}

@Override
public RequestId id() {
return delegate().id();
return unwrap().id();
}

@Override
public HttpMethod method() {
return delegate().method();
return unwrap().method();
}

@Override
public String path() {
return delegate().path();
return unwrap().path();
}

@Override
public String decodedPath() {
return delegate().decodedPath();
return unwrap().decodedPath();
}

@Override
public String query() {
return delegate().query();
return unwrap().query();
}

@Override
public RequestLogAccess log() {
return delegate().log();
return unwrap().log();
}

@Override
public RequestLogBuilder logBuilder() {
return delegate().logBuilder();
return unwrap().logBuilder();
}

@Override
public MeterRegistry meterRegistry() {
return delegate().meterRegistry();
return unwrap().meterRegistry();
}

@Override
public void cancel(Throwable cause) {
delegate().cancel(cause);
unwrap().cancel(cause);
}

@Override
public void cancel() {
delegate().cancel();
unwrap().cancel();
}

@Override
public void timeoutNow() {
delegate().timeoutNow();
unwrap().timeoutNow();
}

@Override
@Nullable
public Throwable cancellationCause() {
return delegate().cancellationCause();
return unwrap().cancellationCause();
}

@Override
public ContextAwareEventLoop eventLoop() {
return delegate().eventLoop();
return unwrap().eventLoop();
}

@Override
public ByteBufAllocator alloc() {
return delegate().alloc();
return unwrap().alloc();
}

@Override
Expand All @@ -221,6 +220,6 @@ public ExchangeType exchangeType() {

@Override
public String toString() {
return delegate().toString();
return unwrap().toString();
}
}
Loading