Skip to content

Commit

Permalink
address comments by @ikhoon and @minwoox
Browse files Browse the repository at this point in the history
  • Loading branch information
@jrhee17
jrhee17 committed Jan 21, 2025
1 parent 23a2685 commit 71a5785
Show file tree
Hide file tree
Showing 13 changed files with 158 additions and 104 deletions.
11 changes: 9 additions & 2 deletions core/src/main/java/com/linecorp/armeria/client/ClientBuilderParams.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.linecorp.armeria.client.endpoint.EndpointGroup;
import com.linecorp.armeria.common.Scheme;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.annotation.UnstableApi;

/**
* Provides the construction parameters of a client.
Expand All @@ -36,7 +37,9 @@ static ClientBuilderParams of(URI uri, Class<?> type, ClientOptions options) {
requireNonNull(uri, "uri");
requireNonNull(type, "type");
requireNonNull(options, "options");
return new DefaultClientBuilderParams(uri, type, options);
return new ClientBuilderParamsBuilder(uri).options(options)
.clientType(type)
.build();
}

/**
Expand All @@ -48,7 +51,10 @@ static ClientBuilderParams of(Scheme scheme, EndpointGroup endpointGroup,
requireNonNull(endpointGroup, "endpointGroup");
requireNonNull(type, "type");
requireNonNull(options, "options");
return new DefaultClientBuilderParams(scheme, endpointGroup, absolutePathRef, type, options);
return new ClientBuilderParamsBuilder(scheme, endpointGroup, absolutePathRef)
.clientType(type)
.options(options)
.build();
}

/**
Expand Down Expand Up @@ -85,6 +91,7 @@ static ClientBuilderParams of(Scheme scheme, EndpointGroup endpointGroup,
* Returns a {@link ClientBuilderParamsBuilder} which allows creation of a new
* {@link ClientBuilderParams} based on the current properties.
*/
@UnstableApi
default ClientBuilderParamsBuilder paramsBuilder() {
return new ClientBuilderParamsBuilder(this);
}
Expand Down
114 changes: 87 additions & 27 deletions core/src/main/java/com/linecorp/armeria/client/ClientBuilderParamsBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,102 +16,162 @@

package com.linecorp.armeria.client;

import static com.google.common.base.MoreObjects.firstNonNull;
import static com.linecorp.armeria.internal.client.ClientBuilderParamsUtil.nullOrEmptyToSlash;
import static java.util.Objects.requireNonNull;

import java.net.URI;
import java.net.URISyntaxException;

import com.linecorp.armeria.client.endpoint.EndpointGroup;
import com.linecorp.armeria.common.Scheme;
import com.linecorp.armeria.common.SerializationFormat;
import com.linecorp.armeria.common.SessionProtocol;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.annotation.UnstableApi;
import com.linecorp.armeria.internal.client.ClientBuilderParamsUtil;
import com.linecorp.armeria.internal.client.endpoint.FailingEndpointGroup;
import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals;

/**
* Allows creation of a new {@link ClientBuilderParams} based on a previous {@link ClientBuilderParams}.
*/
@UnstableApi
public final class ClientBuilderParamsBuilder {

private final ClientBuilderParams params;
private final URI uri;
private final EndpointGroup endpointGroup;
private final SessionProtocol sessionProtocol;

@Nullable
private SerializationFormat serializationFormat;
@Nullable
private String absolutePathRef;

@Nullable
private Class<?> type;
@Nullable
private ClientOptions options;

ClientBuilderParamsBuilder(ClientBuilderParams params) {
this.params = params;
uri = params.uri();
endpointGroup = params.endpointGroup();
sessionProtocol = params.scheme().sessionProtocol();

serializationFormat = params.scheme().serializationFormat();
absolutePathRef = params.absolutePathRef();
type = params.clientType();
options = params.options();
}

ClientBuilderParamsBuilder(URI uri) {
this.uri = uri;
final Scheme scheme = Scheme.parse(uri.getScheme());
final EndpointGroup endpointGroup;
if (ClientBuilderParamsUtil.isInternalUri(uri)) {
endpointGroup = FailingEndpointGroup.of();
} else {
endpointGroup = Endpoint.parse(uri.getRawAuthority());
}
final String absolutePathRef;
try (TemporaryThreadLocals tempThreadLocals = TemporaryThreadLocals.acquire()) {
final StringBuilder buf = tempThreadLocals.stringBuilder();
buf.append(nullOrEmptyToSlash(uri.getRawPath()));
if (uri.getRawQuery() != null) {
buf.append('?').append(uri.getRawQuery());
}
if (uri.getRawFragment() != null) {
buf.append('#').append(uri.getRawFragment());
}
absolutePathRef = buf.toString();
}
this.endpointGroup = endpointGroup;
serializationFormat = scheme.serializationFormat();
sessionProtocol = scheme.sessionProtocol();
this.absolutePathRef = absolutePathRef;
}

ClientBuilderParamsBuilder(Scheme scheme, EndpointGroup endpointGroup, @Nullable String absolutePathRef) {
this.endpointGroup = endpointGroup;
final String schemeStr;
if (scheme.serializationFormat() == SerializationFormat.NONE) {
schemeStr = scheme.sessionProtocol().uriText();
} else {
schemeStr = scheme.uriText();
}
final String normalizedAbsolutePathRef = nullOrEmptyToSlash(absolutePathRef);
final URI uri;
if (endpointGroup instanceof Endpoint) {
uri = URI.create(schemeStr + "://" + ((Endpoint) endpointGroup).authority() +
normalizedAbsolutePathRef);
} else {
// Create a valid URI which will never succeed.
uri = URI.create(schemeStr + "://" + ClientBuilderParamsUtil.ENDPOINTGROUP_PREFIX +
Integer.toHexString(System.identityHashCode(endpointGroup)) +
":1" + normalizedAbsolutePathRef);
}
this.uri = uri;
serializationFormat = scheme.serializationFormat();
sessionProtocol = scheme.sessionProtocol();
this.absolutePathRef = normalizedAbsolutePathRef;
}

/**
* Sets the {@link SerializationFormat} for this {@link ClientBuilderParams}.
*/
public ClientBuilderParamsBuilder serializationFormat(SerializationFormat serializationFormat) {
this.serializationFormat = serializationFormat;
this.serializationFormat = requireNonNull(serializationFormat, "serializationFormat");
return this;
}

/**
* Sets the {@param absolutePathRef} for this {@link ClientBuilderParams}.
*/
public ClientBuilderParamsBuilder absolutePathRef(String absolutePathRef) {
this.absolutePathRef = absolutePathRef;
this.absolutePathRef = requireNonNull(absolutePathRef, "absolutePathRef");
return this;
}

/**
* Sets the {@param type} for this {@link ClientBuilderParams}.
*/
public ClientBuilderParamsBuilder clientType(Class<?> type) {
this.type = type;
this.type = requireNonNull(type, "type");
return this;
}

/**
* Sets the {@link ClientOptions} for this {@link ClientBuilderParams}.
*/
public ClientBuilderParamsBuilder options(ClientOptions options) {
this.options = options;
this.options = requireNonNull(options, "options");
return this;
}

/**
* Builds a new {@link ClientBuilderParams} based on the configured properties.
*/
public ClientBuilderParams build() {
final Scheme scheme;
if (serializationFormat != null) {
scheme = Scheme.of(serializationFormat, params.scheme().sessionProtocol());
} else {
scheme = params.scheme();
}
final ClientOptions options = requireNonNull(this.options, "options");
final Class<?> type = requireNonNull(this.type, "type");

final SerializationFormat serializationFormat = this.serializationFormat;
final String absolutePathRef = this.absolutePathRef;
final ClientFactory factory = options.factory();
final Scheme scheme = factory.validateScheme(Scheme.of(serializationFormat, sessionProtocol));
final String schemeStr;
if (scheme.serializationFormat() == SerializationFormat.NONE) {
schemeStr = scheme.sessionProtocol().uriText();
} else {
schemeStr = scheme.uriText();
}

final String path;
if (absolutePathRef != null) {
path = nullOrEmptyToSlash(absolutePathRef);
} else {
path = params.absolutePathRef();
}
final String path = nullOrEmptyToSlash(absolutePathRef);

final URI prevUri = params.uri();
final URI uri;
try {
uri = new URI(schemeStr, prevUri.getRawAuthority(), path,
prevUri.getRawQuery(), prevUri.getRawFragment());
uri = new URI(schemeStr + "://" + this.uri.getRawAuthority() + path);
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
return new DefaultClientBuilderParams(scheme, params.endpointGroup(), uri.getRawPath(),
uri, firstNonNull(type, params.clientType()),
firstNonNull(options, params.options()));
return new DefaultClientBuilderParams(scheme, endpointGroup, path,
factory.validateUri(uri), type, options);
}
}
13 changes: 8 additions & 5 deletions core/src/main/java/com/linecorp/armeria/client/Clients.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.linecorp.armeria.common.SerializationFormat;
import com.linecorp.armeria.common.SessionProtocol;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.annotation.UnstableApi;
import com.linecorp.armeria.common.util.SafeCloseable;
import com.linecorp.armeria.common.util.Unwrappable;
import com.linecorp.armeria.internal.client.ClientBuilderParamsUtil;
Expand Down Expand Up @@ -179,12 +180,9 @@ public static <T> T newClient(SessionProtocol protocol, EndpointGroup endpointGr
*
* @param preprocessors the {@link ClientPreprocessors}
* @param clientType the type of the new client
*
* @throws IllegalArgumentException if the specified {@code clientType} is unsupported for
* the specified {@link SerializationFormat} or
* {@link ClientPreprocessors}
*/
public static <T> T newClient(ClientPreprocessors preprocessors, Class<T> clientType) {
@UnstableApi
public static <T> T newClient(ClientPreprocessors preprocessors, Class<T> clientType) {
return builder(preprocessors).build(clientType);
}

Expand All @@ -200,6 +198,7 @@ public static <T> T newClient(ClientPreprocessors preprocessors, Class<T> clien
* the specified {@link SerializationFormat} or
* {@link ClientPreprocessors}
*/
@UnstableApi
public static <T> T newClient(SerializationFormat serializationFormat, ClientPreprocessors preprocessors,
Class<T> clientType) {
return builder(serializationFormat, preprocessors).build(clientType);
Expand All @@ -218,6 +217,7 @@ public static <T> T newClient(SerializationFormat serializationFormat, ClientPre
* the specified {@link SerializationFormat} or
* {@link ClientPreprocessors}
*/
@UnstableApi
public static <T> T newClient(SerializationFormat serializationFormat, ClientPreprocessors preprocessors,
Class<T> clientType, String path) {
return builder(serializationFormat, preprocessors, path).build(clientType);
Expand Down Expand Up @@ -307,6 +307,7 @@ public static ClientBuilder builder(Scheme scheme, EndpointGroup endpointGroup,
* Returns a new {@link ClientBuilder} that builds the client that is configured with the specified
* {@link ClientPreprocessors}.
*/
@UnstableApi
public static ClientBuilder builder(ClientPreprocessors preprocessors) {
requireNonNull(preprocessors, "preprocessors");
return new ClientBuilder(SerializationFormat.NONE, preprocessors, null);
Expand All @@ -316,6 +317,7 @@ public static ClientBuilder builder(ClientPreprocessors preprocessors) {
* Returns a new {@link ClientBuilder} that builds the client that is configured with the specified
* {@link ClientPreprocessors}.
*/
@UnstableApi
public static ClientBuilder builder(SerializationFormat serializationFormat,
ClientPreprocessors preprocessors) {
requireNonNull(serializationFormat, "serializationFormat");
Expand All @@ -327,6 +329,7 @@ public static ClientBuilder builder(SerializationFormat serializationFormat,
* Returns a new {@link ClientBuilder} that builds the client that is configured with the specified
* {@link SerializationFormat}, {@link ClientPreprocessors} and {@param path}.
*/
@UnstableApi
public static ClientBuilder builder(SerializationFormat serializationFormat,
ClientPreprocessors preprocessors, String path) {
requireNonNull(serializationFormat, "serializationFormat");
Expand Down
69 changes: 0 additions & 69 deletions core/src/main/java/com/linecorp/armeria/client/DefaultClientBuilderParams.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,12 @@
*/
package com.linecorp.armeria.client;

import static com.linecorp.armeria.internal.client.ClientBuilderParamsUtil.nullOrEmptyToSlash;
import static java.util.Objects.requireNonNull;

import java.net.URI;

import com.google.common.base.MoreObjects;

import com.linecorp.armeria.client.endpoint.EndpointGroup;
import com.linecorp.armeria.common.Scheme;
import com.linecorp.armeria.common.SerializationFormat;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.internal.client.ClientBuilderParamsUtil;
import com.linecorp.armeria.internal.client.endpoint.FailingEndpointGroup;
import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals;

/**
* Default {@link ClientBuilderParams} implementation.
Expand All @@ -42,67 +34,6 @@ final class DefaultClientBuilderParams implements ClientBuilderParams {
private final Class<?> type;
private final ClientOptions options;

/**
* Creates a new instance.
*/
DefaultClientBuilderParams(URI uri, Class<?> type, ClientOptions options) {
final ClientFactory factory = requireNonNull(options, "options").factory();
this.uri = factory.validateUri(uri);
this.type = requireNonNull(type, "type");
this.options = options;

scheme = factory.validateScheme(Scheme.parse(uri.getScheme()));
if (ClientBuilderParamsUtil.isInternalUri(uri)) {
endpointGroup = FailingEndpointGroup.of();
} else {
endpointGroup = Endpoint.parse(uri.getRawAuthority());
}

try (TemporaryThreadLocals tempThreadLocals = TemporaryThreadLocals.acquire()) {
final StringBuilder buf = tempThreadLocals.stringBuilder();
buf.append(nullOrEmptyToSlash(uri.getRawPath()));
if (uri.getRawQuery() != null) {
buf.append('?').append(uri.getRawQuery());
}
if (uri.getRawFragment() != null) {
buf.append('#').append(uri.getRawFragment());
}
absolutePathRef = buf.toString();
}
}

DefaultClientBuilderParams(Scheme scheme, EndpointGroup endpointGroup,
@Nullable String absolutePathRef,
Class<?> type, ClientOptions options) {
final ClientFactory factory = requireNonNull(options, "options").factory();
this.scheme = factory.validateScheme(scheme);
this.endpointGroup = requireNonNull(endpointGroup, "endpointGroup");
this.type = requireNonNull(type, "type");
this.options = options;

final String schemeStr;
if (scheme.serializationFormat() == SerializationFormat.NONE) {
schemeStr = scheme.sessionProtocol().uriText();
} else {
schemeStr = scheme.uriText();
}

final String normalizedAbsolutePathRef = nullOrEmptyToSlash(absolutePathRef);
final URI uri;
if (endpointGroup instanceof Endpoint) {
uri = URI.create(schemeStr + "://" + ((Endpoint) endpointGroup).authority() +
normalizedAbsolutePathRef);
} else {
// Create a valid URI which will never succeed.
uri = URI.create(schemeStr + "://" + ClientBuilderParamsUtil.ENDPOINTGROUP_PREFIX +
Integer.toHexString(System.identityHashCode(endpointGroup)) +
":1" + normalizedAbsolutePathRef);
}

this.uri = factory.validateUri(uri);
this.absolutePathRef = normalizedAbsolutePathRef;
}

DefaultClientBuilderParams(Scheme scheme, EndpointGroup endpointGroup, String absolutePathRef,
URI uri, Class<?> type, ClientOptions options) {
this.scheme = options.factory().validateScheme(scheme);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/linecorp/armeria/client/DefaultWebClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public HttpResponse execute(HttpRequest req, RequestOptions requestOptions) {
assert scheme != null;
assert authority != null;
} else {
// the scheme and authority may be null if the client is preprocessor-based
// the scheme and authority may be null if the client has preprocessors
scheme = req.scheme();
authority = req.authority();
}
Expand Down
Loading

0 comments on commit 71a5785

Please sign in to comment.