Skip to content

Commit

Permalink
Switch to static builder() methods (line#2221)
Browse files Browse the repository at this point in the history
Related: line#1719
Motivation:

We decided to switch from `new *Builder()` to `*.builder()`.

Modifications:

- Switch to static `builder()` methods in:
  - `ClientFactory`
  - `ClientOptions`
  - `ClientDecoration`
  - `ClientConnectionTimings`
  - `HttpClient`
  - `LoggingClient`
  - `LoggingService`
  - `EndpointInfo`
  - `FieldInfo`
  - `ClientCacheControl`
  - `ServerCacheControl`
  - `ClientRequestContext`
  - `ServiceRequestContext`
  - `RequestContextCurrentTraceContext`
  - `RetrofitMeterIdPrefixFunction`
    - (Breaking) `RetrofitMeterIfPrefixFunctionBuilder` is now a top-level class without a public constructor.
- Miscellaneous:
  - Increase the test timeout for `ArmeriaSpringActuatorAutoConfigurationTest` for less flakiness
  - Maybe fixes the flakiness in `GracefulShutdownSupportTest`
  • Loading branch information
trustin committed Nov 5, 2019
1 parent 72ef8a7 commit ac784ab
Show file tree
Hide file tree
Showing 145 changed files with 1,585 additions and 1,203 deletions.
Expand Up @@ -20,7 +20,6 @@
import org.openjdk.jmh.annotations.State;

import com.linecorp.armeria.client.HttpClient;
import com.linecorp.armeria.client.HttpClientBuilder;
import com.linecorp.armeria.client.retry.RetryStrategyWithContent;
import com.linecorp.armeria.client.retry.RetryingHttpClient;
import com.linecorp.armeria.common.HttpResponse;
Expand All @@ -33,9 +32,9 @@ protected HttpClient newClient() {
final RetryStrategyWithContent<HttpResponse> retryStrategy =
(ctx, response) -> response.aggregate().handle((unused1, unused2) -> null);

return new HttpClientBuilder(baseUrl())
.decorator(RetryingHttpClient.builder(retryStrategy)
.newDecorator())
.build();
return HttpClient.builder(baseUrl())
.decorator(RetryingHttpClient.builder(retryStrategy)
.newDecorator())
.build();
}
}
Expand Up @@ -20,7 +20,6 @@
import org.openjdk.jmh.annotations.State;

import com.linecorp.armeria.client.HttpClient;
import com.linecorp.armeria.client.HttpClientBuilder;
import com.linecorp.armeria.client.retry.RetryStrategy;
import com.linecorp.armeria.client.retry.RetryingHttpClient;

Expand All @@ -29,8 +28,8 @@ public class WithoutDuplicator extends RetryingHttpClientBase {

@Override
protected HttpClient newClient() {
return new HttpClientBuilder(baseUrl())
.decorator(RetryingHttpClient.newDecorator(RetryStrategy.never()))
.build();
return HttpClient.builder(baseUrl())
.decorator(RetryingHttpClient.newDecorator(RetryStrategy.never()))
.build();
}
}
Expand Up @@ -36,7 +36,6 @@
import com.linecorp.armeria.grpc.shared.GithubApiService;
import com.linecorp.armeria.server.Service;
import com.linecorp.armeria.server.ServiceRequestContext;
import com.linecorp.armeria.server.ServiceRequestContextBuilder;
import com.linecorp.armeria.server.grpc.GrpcService;
import com.linecorp.armeria.unsafe.ByteBufHttpData;

Expand Down Expand Up @@ -77,9 +76,9 @@ public class GrpcServiceBenchmark {
public void initBuffers() {
req = HttpRequest.of(EMPTY_HEADERS,
HttpData.wrap(ByteBufAllocator.DEFAULT.buffer().writeBytes(FRAMED_EMPTY)));
ctx = ServiceRequestContextBuilder.of(req)
.service(SERVICE)
.build();
ctx = ServiceRequestContext.builder(req)
.service(SERVICE)
.build();
}

@TearDown(Level.Invocation)
Expand Down
Expand Up @@ -20,7 +20,6 @@
import org.openjdk.jmh.annotations.State;

import com.linecorp.armeria.client.ClientFactory;
import com.linecorp.armeria.client.ClientFactoryBuilder;
import com.linecorp.armeria.client.retrofit2.ArmeriaRetrofitBuilder;
import com.linecorp.armeria.retrofit2.shared.SimpleBenchmarkBase;
import com.linecorp.armeria.retrofit2.shared.SimpleBenchmarkClient;
Expand All @@ -34,9 +33,10 @@ public class DownstreamSimpleBenchmark extends SimpleBenchmarkBase {
@Override
protected SimpleBenchmarkClient newClient() {
final ClientFactory factory =
new ClientFactoryBuilder()
.sslContextCustomizer(ssl -> ssl.trustManager(InsecureTrustManagerFactory.INSTANCE))
.build();
ClientFactory.builder()
.sslContextCustomizer(
ssl -> ssl.trustManager(InsecureTrustManagerFactory.INSTANCE))
.build();
return new ArmeriaRetrofitBuilder(factory)
.baseUrl(baseUrl())
.addConverterFactory(JacksonConverterFactory.create())
Expand Down
Expand Up @@ -101,8 +101,7 @@ public static void setCurrentThreadNotRequestThread(boolean value) {
}
}

private static final RequestContextCurrentTraceContext DEFAULT =
new RequestContextCurrentTraceContextBuilder().build();
private static final RequestContextCurrentTraceContext DEFAULT = builder().build();

private static final Logger logger = LoggerFactory.getLogger(RequestContextCurrentTraceContext.class);

Expand Down
Expand Up @@ -31,6 +31,14 @@ public final class RequestContextCurrentTraceContextBuilder extends CurrentTrace

private final ImmutableList.Builder<Pattern> nonRequestThreadPatterns = ImmutableList.builder();

/**
* Creates a new instance.
*
* @deprecated Use {@link RequestContextCurrentTraceContext#builder()}.
*/
@Deprecated
public RequestContextCurrentTraceContextBuilder() {}

/**
* Sets a regular expression that matches names of threads that should be considered non-request
* threads, meaning they may have spans created for clients outside of the context of an Armeria
Expand Down
Expand Up @@ -31,7 +31,7 @@

import com.google.common.collect.ImmutableList;

import com.linecorp.armeria.client.ClientDecorationBuilder;
import com.linecorp.armeria.client.ClientDecoration;
import com.linecorp.armeria.client.ClientFactory;
import com.linecorp.armeria.client.ClientOption;
import com.linecorp.armeria.client.ClientOptions;
Expand Down Expand Up @@ -95,9 +95,9 @@ protected Builder tracingBuilder(Sampler sampler) {
protected HttpClient newClient(int port) {
return HttpClient.of(sessionProtocol.uriText() + "://127.0.0.1:" + port,
ClientOptions.of(ClientOption.DECORATION.newValue(
new ClientDecorationBuilder()
.add(BraveClient.newDecorator(httpTracing))
.build())));
ClientDecoration.builder()
.add(BraveClient.newDecorator(httpTracing))
.build())));
}

@Override
Expand Down Expand Up @@ -176,7 +176,7 @@ public RequestContext newDerivedContext(@Nullable HttpRequest req,

@Override
public EventLoop eventLoop() {
return ClientFactory.DEFAULT.eventLoopGroup().next();
return ClientFactory.ofDefault().eventLoopGroup().next();
}

@Nullable
Expand Down
Expand Up @@ -35,7 +35,6 @@

import com.linecorp.armeria.client.Client;
import com.linecorp.armeria.client.ClientRequestContext;
import com.linecorp.armeria.client.ClientRequestContextBuilder;
import com.linecorp.armeria.common.HttpHeaderNames;
import com.linecorp.armeria.common.HttpMethod;
import com.linecorp.armeria.common.HttpRequest;
Expand Down Expand Up @@ -213,7 +212,7 @@ private static RequestLog testRemoteInvocation(Tracing tracing, @Nullable String
final RpcRequest rpcReq = RpcRequest.of(HelloService.Iface.class, "hello", "Armeria");
final HttpResponse res = HttpResponse.of(HttpStatus.OK);
final RpcResponse rpcRes = RpcResponse.of("Hello, Armeria!");
final ClientRequestContext ctx = ClientRequestContextBuilder.of(req).build();
final ClientRequestContext ctx = ClientRequestContext.builder(req).build();
final HttpRequest actualReq = ctx.request();
assertThat(actualReq).isNotNull();

Expand Down
Expand Up @@ -47,7 +47,6 @@
import com.linecorp.armeria.common.util.SafeCloseable;
import com.linecorp.armeria.server.Service;
import com.linecorp.armeria.server.ServiceRequestContext;
import com.linecorp.armeria.server.ServiceRequestContextBuilder;

import brave.Tracing;
import brave.http.HttpTracing;
Expand Down Expand Up @@ -169,8 +168,7 @@ private static RequestLog testServiceInvocation(Reporter<Span> reporter,
final HttpRequest req = HttpRequest.of(RequestHeaders.of(HttpMethod.POST, "/hello/trustin",
HttpHeaderNames.SCHEME, "http",
HttpHeaderNames.AUTHORITY, "foo.com"));
final ServiceRequestContext ctx = ServiceRequestContextBuilder.of(req)
.build();
final ServiceRequestContext ctx = ServiceRequestContext.builder(req).build();
final RpcRequest rpcReq = RpcRequest.of(HelloService.Iface.class, "hello", "trustin");
final HttpResponse res = HttpResponse.of(HttpStatus.OK);
final RpcResponse rpcRes = RpcResponse.of("Hello, trustin!");
Expand Down
Expand Up @@ -41,18 +41,18 @@
class AbstractClientOptionsBuilder<B extends AbstractClientOptionsBuilder<B>> {

private final Map<ClientOption<?>, ClientOptionValue<?>> options = new LinkedHashMap<>();
private final ClientDecorationBuilder decoration = new ClientDecorationBuilder();
private final ClientDecorationBuilder decoration = ClientDecoration.builder();
private final HttpHeadersBuilder httpHeaders = HttpHeaders.builder();

/**
* Creates a new instance with the default options.
*/
protected AbstractClientOptionsBuilder() {}
AbstractClientOptionsBuilder() {}

/**
* Creates a new instance with the specified base options.
*/
protected AbstractClientOptionsBuilder(ClientOptions options) {
AbstractClientOptionsBuilder(ClientOptions options) {
requireNonNull(options, "options");
options(options);
}
Expand Down
Expand Up @@ -68,7 +68,7 @@ public final class ClientBuilder extends AbstractClientOptionsBuilder<ClientBuil

private SerializationFormat format = SerializationFormat.NONE;

private ClientFactory factory = ClientFactory.DEFAULT;
private ClientFactory factory = ClientFactory.ofDefault();

/**
* Creates a new {@link ClientBuilder} that builds the client that connects to the specified {@code uri}.
Expand Down Expand Up @@ -117,7 +117,7 @@ private ClientBuilder(@Nullable URI uri, @Nullable Scheme scheme, @Nullable Sess
}

/**
* Sets the {@link ClientFactory} of the client. The default is {@link ClientFactory#DEFAULT}.
* Sets the {@link ClientFactory} of the client. The default is {@link ClientFactory#ofDefault()}.
*/
public ClientBuilder factory(ClientFactory factory) {
this.factory = requireNonNull(factory, "factory");
Expand Down
Expand Up @@ -81,6 +81,13 @@ public static ClientConnectionTimings get(RequestLog log) {
return null;
}

/**
* Returns a newly created {@link ClientConnectionTimingsBuilder}.
*/
public static ClientConnectionTimingsBuilder builder() {
return new ClientConnectionTimingsBuilder();
}

ClientConnectionTimings(long connectionAcquisitionStartTimeMicros, long connectionAcquisitionDurationNanos,
long dnsResolutionStartTimeMicros, long dnsResolutionDurationNanos,
long socketConnectStartTimeMicros, long socketConnectDurationNanos,
Expand Down
Expand Up @@ -47,7 +47,10 @@ public final class ClientConnectionTimingsBuilder {

/**
* Creates a new instance.
*
* @deprecated Use {@link ClientConnectionTimings#builder()}.
*/
@Deprecated
public ClientConnectionTimingsBuilder() {
connectionAcquisitionStartTimeMicros = SystemInfo.currentTimeMicros();
connectionAcquisitionStartNanos = System.nanoTime();
Expand Down
Expand Up @@ -32,9 +32,19 @@ public final class ClientDecoration {

/**
* A {@link ClientDecoration} that decorates no {@link Client}.
*
* @deprecated Use {@link #of()}.
*/
@Deprecated
public static final ClientDecoration NONE = new ClientDecoration(Collections.emptyList());

/**
* Returns an empty {@link ClientDecoration} which does not decorate a {@link Client}.
*/
public static ClientDecoration of() {
return NONE;
}

/**
* Creates a new instance from a single decorator {@link Function}.
*
Expand All @@ -46,7 +56,14 @@ public final class ClientDecoration {
*/
public static <T extends Client<I, O>, R extends Client<I, O>, I extends Request, O extends Response>
ClientDecoration of(Class<I> requestType, Class<O> responseType, Function<T, R> decorator) {
return new ClientDecorationBuilder().add(requestType, responseType, decorator).build();
return builder().add(requestType, responseType, decorator).build();
}

/**
* Returns a newly created {@link ClientDecorationBuilder}.
*/
public static ClientDecorationBuilder builder() {
return new ClientDecorationBuilder();
}

private final List<Entry<?, ?>> entries;
Expand Down
Expand Up @@ -37,6 +37,14 @@ public final class ClientDecorationBuilder {

private final List<Entry<?, ?>> entries = new ArrayList<>();

/**
* Creates a new instance.
*
* @deprecated Use {@link ClientDecoration#builder()}.
*/
@Deprecated
public ClientDecorationBuilder() {}

/**
* Adds a new decorator {@link Function}.
*
Expand Down
31 changes: 24 additions & 7 deletions core/src/main/java/com/linecorp/armeria/client/ClientFactory.java
Expand Up @@ -43,10 +43,10 @@
*
* <h3>Life cycle of the default {@link ClientFactory}</h3>
* <p>
* {@link Clients} or {@link ClientBuilder} uses {@link #DEFAULT}, the default {@link ClientFactory},
* unless you specified a {@link ClientFactory} explicitly. Calling {@link #close()} on the default
* {@link ClientFactory} will neither terminate its I/O threads nor release other related resources unlike
* other {@link ClientFactory} to protect itself from accidental premature termination.
* {@link Clients} or {@link ClientBuilder} uses the default {@link ClientFactory} returned by
* {@link #ofDefault()}, unless you specified a {@link ClientFactory} explicitly. Calling {@link #close()}
* on the default {@link ClientFactory} will neither terminate its I/O threads nor release other related
* resources unlike other {@link ClientFactory} to protect itself from accidental premature termination.
* </p><p>
* Instead, when the current {@link ClassLoader} is {@linkplain ClassLoader#getSystemClassLoader() the system
* class loader}, a {@linkplain Runtime#addShutdownHook(Thread) shutdown hook} is registered so that they are
Expand All @@ -60,21 +60,38 @@ public interface ClientFactory extends AutoCloseable {

/**
* The default {@link ClientFactory} implementation.
*
* @deprecated Use {@link #ofDefault()}.
*/
ClientFactory DEFAULT = new ClientFactoryBuilder().build();
@Deprecated
ClientFactory DEFAULT = builder().build();

/**
* Returns the default {@link ClientFactory} implementation.
*/
static ClientFactory ofDefault() {
return DEFAULT;
}

/**
* Returns a newly created {@link ClientFactoryBuilder}.
*/
static ClientFactoryBuilder builder() {
return new ClientFactoryBuilder();
}

/**
* Closes the default {@link ClientFactory}.
*/
static void closeDefault() {
LoggerFactory.getLogger(ClientFactory.class).debug(
"Closing the default {}", ClientFactory.class.getSimpleName());
((DefaultClientFactory) DEFAULT).doClose();
((DefaultClientFactory) ofDefault()).doClose();
}

/**
* Disables the {@linkplain Runtime#addShutdownHook(Thread) shutdown hook} which closes
* {@linkplain #DEFAULT the default <code>ClientFactory</code>}. This method is useful when you need
* {@linkplain #ofDefault() the default <code>ClientFactory</code>}. This method is useful when you need
* full control over the life cycle of the default {@link ClientFactory}.
*/
static void disableShutdownHook() {
Expand Down
Expand Up @@ -118,7 +118,10 @@ public final class ClientFactoryBuilder {

/**
* Creates a new instance.
*
* @deprecated Use {@link ClientFactory#builder()}.
*/
@Deprecated
public ClientFactoryBuilder() {
connectTimeoutMillis(Flags.defaultConnectTimeoutMillis());
}
Expand Down

0 comments on commit ac784ab

Please sign in to comment.