Introduce a way to unwrap RequestContexts

[jrhee17]

Motivation:

Armeria has some core logic which assumes that the concrete default implementations of [Client|Service]RequestContext are Default[Client|Service]RequestContext.

e.g. If the ClientRequestContext isn't an instance of DefaultClientRequestContext, then the response timeout may not be respected.

armeria/core/src/main/java/com/linecorp/armeria/client/HttpResponseDecoder.java

Lines 401 to 407 in 5edaef0

	if (ctx instanceof DefaultClientRequestContext) {
	final CancellationScheduler responseCancellationScheduler =
	((DefaultClientRequestContext) ctx).responseCancellationScheduler();
	responseCancellationScheduler.init(
	ctx.eventLoop(), newCancellationTask(),
	TimeUnit.MILLISECONDS.toNanos(responseTimeoutMillis), /* server */ false);
	}

This usage pattern mostly stems from the fact that [Client|Service]RequestContext are public APIs that users are exposed to a lot (especially in decorators). However, Armeria developers are skeptical of adding public methods that are either 1) very specific and used only in core logic 2) have the potential to break behavior if used incorrectly.

Up to now, we have added such methods to the default implementation and checked with instanceof when invoking such methods.

With the introduction of #4232, we are now planning on introducing and extensively utilizing wrapper classes for [Client|Service]RequestContext. However, using wrapper classes means previous assumptions of instanceof Default[Client|Service]RequestContext will be invalid.

In this pull request, I propose that:

1. RequestContext now extends Unwrappable, allowing users to peel RequestContexts to the intended type.
2. We expose another interface [Client]RequestContextExtension, which contains "extension" methods for ClientRequestContext, RequestContext. In this interface, we can add methods that Armeria uses internally without worrying about public API exposure.
3. Default[Client|Service]RequestContext doesn't need to be exposed as a public API. We may simply expose the interface to users and hide the detailed implementation like we do for the Http[Response|Request] series.
   • Note: This move was handled in this PR mainly due to CancellationScheduler. While extracting ClientRequestContextExtension, CancellationScheduler which is an internal class was exposed as the return value of #responseCancellationScheduler. I had the choice of exposing CancellationScheduler, or hiding DefaultClientRequestContext. I chose the latter and also moved DefaultServiceRequestContext for consistency.

Modifications:

• RequestContext now extends Unwrappable. RequestContextWrapper now extends AbstractUnwrappable
• Convert usages of instanceof DefaultClientRequestContext to unwrap instead
  • e.g. ctx instanceof DefaultClientRequestContext -> ctx.as(ClientRequestContextExtension.class)
• RequestContextExtension, ClientRequestContextExtension has been added to an internal package
• DefaultClientRequestContext, DefaultServiceRequestContext, NonWrappingRequestContext have been moved to an internal package
  • pathWithQuery is moved to ClientUtil and made public
  • ClientThreadLocalState is moved to an internal package and made public
• Class hierarchy has been modified for RequestContext
  • Now there is an extra layer between ClientRequestContext and DefaultClientRequestContext:
    • ClientRequestContext -> ClientRequestContextExtension -> DefaultClientRequestContext
  • Now there is an extra layer between RequestContext and NonWrappingRequestContext
    • RequestContext -> RequestContextExtension -> NonWrappingRequestContext

Before:

After:

Result:

• Developers may add more utility functions to RequestContext without worrying about public API exposure
• Armeria tries to unwrap RequestContext instead of simply checking the type.

[codecov]

Codecov Report

Merging #4308 (32d7692) into master (dd31525) will increase coverage by 0.00%.
The diff coverage is 45.00%.

@@            Coverage Diff            @@
##             master    #4308   +/-   ##
=========================================
  Coverage     73.51%   73.51%           
- Complexity    17663    17673   +10     
=========================================
  Files          1504     1504           
  Lines         66100    66112   +12     
  Branches       8338     8341    +3     
=========================================
+ Hits          48594    48603    +9     
- Misses        13292    13294    +2     
- Partials       4214     4215    +1     

Impacted Files	Coverage Δ
...rp/armeria/client/ClientRequestContextBuilder.java	80.64% <ø> (ø)
...rp/armeria/client/ClientRequestContextWrapper.java	5.40% <0.00%> (+5.40%)	⬆️
...main/java/com/linecorp/armeria/client/Clients.java	62.50% <ø> (ø)
.../com/linecorp/armeria/client/DefaultWebClient.java	93.10% <ø> (+0.91%)	⬆️
...n/java/com/linecorp/armeria/client/UserClient.java	79.41% <ø> (ø)
...rmeria/internal/client/ClientThreadLocalState.java	80.70% <ø> (ø)
...ria/internal/common/NonWrappingRequestContext.java	79.41% <ø> (ø)
.../internal/server/DefaultServiceRequestContext.java	87.66% <ø> (ø)
...rp/armeria/server/AbstractHttpResponseHandler.java	92.00% <ø> (ø)
.../armeria/server/AggregatedHttpResponseHandler.java	73.68% <ø> (ø)
... and 46 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

[minwoox]

Looks great! 😄

[ikhoon]

Thanks, @jrhee17! 💯

[ikhoon]

master branch has been merged. Let's do the same work for unwrapAll(). 😉

[jrhee17]

Let's do the same work for unwrapAll()

Do you mean override the return type for unwrapAll?
I thought it's difficult to do this with the current constraints provided by Unwrappable 😅

ref: #4338 (comment)

[ikhoon]

Couldn't we safely cast the return type to RequestContext?

@Override
default RequestContext unwrapAll() {
    return (RequestContext) Unwrappable.super.unwrapAll();
}

[jrhee17]

Sorry, I guess I should've made my comment on the other PR clearer 😅

To be precise, as you said we can override the return type as RequestContext for objects that are "unwrappable".

However, I'm unsure how the API should be designed for objects that wrap an Unwrappable (i.e. via AbstractUnwrappable)

1. If AbstractUnwrappable#unwrapAll returns an Object:
   In this case AbstractUnwrappable#unwrapAll returns an Object whereas RequestContext#unwrapAll returns a RequestContext, which causes a return type collision.
   e.g.

'unwrapAll()' in 'com.linecorp.armeria.common.util.AbstractUnwrappable' clashes with 'unwrapAll()' in 'com.linecorp.armeria.common.RequestContext'; attempting to use incompatible return type

2. If AbstractUnwrappable#unwrapAll returns T:
   I'm not sure if there's type safe way to know the innermost type of a wrappable.

final Foo foo = new Foo();
final Bar<Foo> bar = new Bar<>(foo);
final Qux<Bar<Foo>> qux = new Qux<>(bar);

I guess it's not too late to modify to a type unsafe API though 😅
e.g.

<U extends Unwrappable> U unwrapAll()

ref: #4338 (comment)

Let me know if you have a better idea though 😅

[ikhoon]

'unwrapAll()' in 'com.linecorp.armeria.common.util.AbstractUnwrappable' clashes with 'unwrapAll()' in 'com.linecorp.armeria.common.RequestContext'; attempting to use incompatible return type

The error seems to happen in RequestContextWrapper. How about overriding the return type of unwrapAll() in RequestContextWrapper?
I don't see that we need to change the return type of AbstractUnwrappable#unwrapAll.
The subtype of AbstractUnwrappable or Unwrappable freely overrides the return type if necessary.

public abstract class RequestContextWrapper<T extends RequestContext>
        extends AbstractUnwrappable<T> implements RequestContext {

    @Override
    public RequestContext unwrapAll() {
        return (RequestContext) super.unwrapAll();
    }
}

[jrhee17]

Ah, I think you're right. I guess we can override the return type for wrapper types only. Done!

[ikhoon]

Thanks, @jrhee17! You paid off our technical debt. 😆
Let me merge this PR when CI builds finish.