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.

Sign up for GitHub

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

Jump to bottom

Add Nacos Support #5409

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

Add Nacos Support #5409

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
785b353
Add NacosClient and NacosEndpointGroup
KonaEspresso94 Jan 21, 2024
a76d4fc
Add NacosUpdatingListener
KonaEspresso94 Jan 24, 2024
4caabaf
Test codes
KonaEspresso94 Jan 24, 2024
3af6f6c
Modified NacosClientUtil::queryParams to return QueryParamsBuilder in…
KonaEspresso94 Jan 27, 2024
d4332aa
Merge branch 'main' into nacos-support
KonaEspresso94 Jan 27, 2024
9530372
Make properties of the classes that define data rendering private and…
KonaEspresso94 Feb 9, 2024
ffc64d7
Make LoginClient an implementation of SimpleDecoratingHttpClient.
KonaEspresso94 Feb 10, 2024
68e1137
LoginClient : Cache and reuse QueryStrings that were unnecessarily re…
KonaEspresso94 Feb 10, 2024
0f71869
QueryInstancesClient : Cache and reuse request path that were unneces…
KonaEspresso94 Feb 10, 2024
46abd6d
Reduce the parameters of the RegisterInstanceClient APIS, by moving a…
KonaEspresso94 Feb 10, 2024
adb40fc
Revert "Modified NacosClientUtil::queryParams to return QueryParamsBu…
KonaEspresso94 Feb 10, 2024
0120fd7
To reuse access-token, instead of the Caffeine cache, use own impleme…
KonaEspresso94 Feb 11, 2024
12b2214
Merge branch 'main' into nacos-support
KonaEspresso94 Feb 11, 2024
23d8913
Merge branch 'main' into nacos-support
KonaEspresso94 May 8, 2024
0d0082d
Merge branch 'main' into nacos-support
KonaEspresso94 Jun 16, 2024
a0d7895
Revert "To reuse access-token, instead of the Caffeine cache, use own…
KonaEspresso94 Jun 16, 2024
a427074
Lint code. (omitted 'final', meaningless lambda expression, unnecessa…
KonaEspresso94 Jun 16, 2024
625535a
Lint code. (Remove meaningless property and use parameter directly)
KonaEspresso94 Jun 16, 2024
ed1d5ba
Lint code. (Optimize imports, according to :nacos:checkStyleMain)
KonaEspresso94 Jun 16, 2024
f94cf66
Copyright header: Line Corporation -> LY Corporation
KonaEspresso94 Jun 16, 2024
c10ebc8
TODO comment about AsyncLoadingCache
KonaEspresso94 Jun 16, 2024
5fe5ba2
Hide NacosEndpointGroup as package private.
KonaEspresso94 Jun 19, 2024
03150f2
Apply AbstractDynamicEndpointGroupBuilder's changes(SELF type parameter)
KonaEspresso94 Jun 19, 2024
88f6235
Use SELF type parameter for NacosConfigSetters.
KonaEspresso94 Jun 19, 2024
0a715cd
Lint code: Run auto-indent.
KonaEspresso94 Jun 28, 2024
7476ede
NacosEndpointGroup : Batch the tasks related to scheduledFuture into …
KonaEspresso94 Jun 28, 2024
4e29132
lint code.
KonaEspresso94 Jun 28, 2024
110ee22
Remove override functions of NacosEndpointGroupBuilder (Use default i…
KonaEspresso94 Jun 28, 2024
0b988ba
Methods: namespaceId, groupName, clusterName, and app also can be Nac…
KonaEspresso94 Jun 30, 2024
8933fcd
Merge branch 'main' into nacos-support
KonaEspresso94 Jul 1, 2024
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
1 change: 0 additions & 1 deletion nacos/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
dependencies {
implementation(libs.caffeine)
testImplementation(libs.testcontainers.junit.jupiter)
}
6 changes: 4 additions & 2 deletions nacos/src/main/java/com/linecorp/armeria/client/nacos/NacosEndpointGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,14 @@ private void update() {
return null;
}
if (cause != null) {
logger.warn("Unexpected exception while fetching the registry from: {}", nacosClient.uri(), cause);
logger.warn("Unexpected exception while fetching the registry from: {}",
nacosClient.uri(), cause);
} else {
setEndpoints(endpoints);
}

scheduledFuture = eventLoop.schedule(this::update, registryFetchIntervalMillis, TimeUnit.MILLISECONDS);
scheduledFuture = eventLoop.schedule(this::update,
registryFetchIntervalMillis, TimeUnit.MILLISECONDS);
return null;
});
}
Expand Down
96 changes: 49 additions & 47 deletions nacos/src/main/java/com/linecorp/armeria/internal/nacos/LoginClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,25 @@

import java.util.concurrent.CompletableFuture;
import java.util.function.Function;

import com.linecorp.armeria.client.ClientRequestContext;
import com.linecorp.armeria.client.HttpClient;
import com.linecorp.armeria.client.SimpleDecoratingHttpClient;
import com.linecorp.armeria.client.WebClient;
import com.linecorp.armeria.common.*;
import org.checkerframework.checker.nullness.qual.NonNull;
import java.util.function.Supplier;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.github.benmanes.caffeine.cache.AsyncLoadingCache;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.Expiry;
import com.google.common.base.MoreObjects;

import com.linecorp.armeria.client.ClientRequestContext;
import com.linecorp.armeria.client.HttpClient;
import com.linecorp.armeria.client.SimpleDecoratingHttpClient;
import com.linecorp.armeria.client.WebClient;
import com.linecorp.armeria.common.HttpEntity;
import com.linecorp.armeria.common.HttpHeaderNames;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.MediaType;
import com.linecorp.armeria.common.QueryParams;
import com.linecorp.armeria.common.annotation.Nullable;
import com.linecorp.armeria.common.util.UnmodifiableFuture;

/**
* A Nacos client that is responsible for
Expand All @@ -47,42 +49,14 @@ public static Function<? super HttpClient, LoginClient> newDecorator(WebClient w
return delegate -> new LoginClient(delegate, webClient, username, password);
}

private static final String NACOS_ACCESS_TOKEN_CACHE_KEY = "NACOS_ACCESS_TOKEN_CACHE_KEY";
private final AsyncLoadingCache<String, LoginResult> tokenCache = Caffeine.newBuilder()
.maximumSize(1)
.expireAfter(new Expiry<String, LoginResult>() {
@Override
public long expireAfterCreate(@NonNull String key, @NonNull LoginResult loginResult,
long currentTime) {
return loginResult.tokenTtl.longValue();
}

@Override
public long expireAfterUpdate(@NonNull String key, @NonNull LoginResult loginResult,
long currentTime, long currentDuration) {
return loginResult.tokenTtl.longValue();
}

@Override
public long expireAfterRead(@NonNull String key, @NonNull LoginResult loginResult,
long currentTime, long currentDuration) {
return currentDuration;
}
})
.buildAsync((key, executor) -> {
try {
return loginInternal();
} catch (Exception e) {
throw new RuntimeException(e);
}
});

private final HttpClient delegate;

private final WebClient webClient;

private final String queryParamsForLogin;

private final CachedLoginResult cachedLoginResult = new CachedLoginResult();

LoginClient(HttpClient delegate, WebClient webClient, String username, String password) {
super(delegate);

Expand All @@ -94,12 +68,7 @@ public long expireAfterRead(@NonNull String key, @NonNull LoginResult loginResul
.toQueryString();
}

private CompletableFuture<String> login() {
return tokenCache.get(NACOS_ACCESS_TOKEN_CACHE_KEY)
.thenApply(loginResult -> loginResult.accessToken);
}

private CompletableFuture<LoginResult> loginInternal() {
private CompletableFuture<LoginResult> login() {
return webClient.prepare().post("/v1/auth/login")
.content(MediaType.FORM_DATA, queryParamsForLogin)
.asJson(LoginResult.class)
Expand All @@ -109,7 +78,7 @@ private CompletableFuture<LoginResult> loginInternal() {

@Override
public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) {
CompletableFuture<HttpResponse> future = login().thenApply(accessToken -> {
final CompletableFuture<HttpResponse> future = cachedLoginResult.get().thenApply(accessToken -> {
try {
return delegate.execute(ctx, req.mapHeaders(headers -> headers.toBuilder()
.set(HttpHeaderNames.AUTHORIZATION, "Bearer " + accessToken)
Expand All @@ -122,6 +91,39 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) {
return HttpResponse.of(future);
}

private final class CachedLoginResult {
@Nullable
private volatile CacheEntry cacheEntry;

CachedLoginResult() { }

CompletableFuture<String> get() {
final CacheEntry cacheEntry = this.cacheEntry;
if (cacheEntry != null && !cacheEntry.isExpired()) {
return UnmodifiableFuture.completedFuture(cacheEntry.loginResult.accessToken);
}

return login().thenApply(loginResult -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may result in sending multiple login requests that are not what we really want.
If login() is being performed, other requests can piggyback on the returned future.

By the way, I think an AsyncLoader loading a value when it is expired or periodically would be useful in many places.
Created #5506

Copy link
Member

@trustin trustin Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we shouldn't let @KonaEspresso94 wait for too long for the PR to be merged but just use Caffeine internally and later replace the Caffeine usage with AsyncLoader?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know it would take this long. 😅 Let's leave that part as is. I will continue to review this PR tomorrow.

Copy link
Author

@KonaEspresso94 KonaEspresso94 Jun 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your consideration. Following your comments, I have reverted the commit to use Caffeine instead of my implementation.

this.cacheEntry = new CacheEntry(loginResult);
return loginResult.accessToken;
});
}

private class CacheEntry {
private final LoginResult loginResult;
private final long expirationTimeMillis;

CacheEntry(LoginResult loginResult) {
this.loginResult = loginResult;
this.expirationTimeMillis = System.currentTimeMillis() + loginResult.tokenTtl * 1000;
}

boolean isExpired() {
return System.currentTimeMillis() >= expirationTimeMillis;
}
}
}

@JsonIgnoreProperties(ignoreUnknown = true)
private static final class LoginResult {
private final String accessToken;
Expand Down
2 changes: 1 addition & 1 deletion nacos/src/main/java/com/linecorp/armeria/internal/nacos/NacosClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public final class NacosClientBuilder implements NacosConfigSetters {
@Nullable
private String password;

final private String serviceName;
private final String serviceName;

@Nullable
private String namespaceId;
Expand Down
11 changes: 7 additions & 4 deletions nacos/src/main/java/com/linecorp/armeria/internal/nacos/QueryInstancesClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ final class QueryInstancesClient {

static QueryInstancesClient of(NacosClient nacosClient, String nacosApiVersion, String serviceName,
@Nullable String namespaceId, @Nullable String groupName,
@Nullable String clusterName, @Nullable Boolean healthyOnly, @Nullable String app) {
return new QueryInstancesClient(nacosClient, nacosApiVersion, serviceName, namespaceId, groupName, clusterName,
healthyOnly, app);
@Nullable String clusterName, @Nullable Boolean healthyOnly,
@Nullable String app) {
return new QueryInstancesClient(nacosClient, nacosApiVersion, serviceName, namespaceId, groupName,
clusterName, healthyOnly, app);
}

private final WebClient webClient;
Expand All @@ -56,7 +57,9 @@ static QueryInstancesClient of(NacosClient nacosClient, String nacosApiVersion,
@Nullable String clusterName, @Nullable Boolean healthyOnly, @Nullable String app) {
webClient = nacosClient.nacosWebClient();

final StringBuilder pathBuilder = new StringBuilder("/").append(nacosApiVersion).append("/ns/instance/list?");
final StringBuilder pathBuilder = new StringBuilder("/")
.append(nacosApiVersion)
.append("/ns/instance/list?");
final QueryParams params = NacosClientUtil
.queryParams(namespaceId, groupName, requireNonNull(serviceName, "serviceName"),
clusterName, healthyOnly, app, null, null, null);
Expand Down
6 changes: 2 additions & 4 deletions nacos/src/test/java/com/linecorp/armeria/server/nacos/NacosUpdatingListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.core.JsonProcessingException;

import com.linecorp.armeria.client.Endpoint;
import com.linecorp.armeria.internal.nacos.NacosTestBase;
import com.linecorp.armeria.internal.testing.GenerateNativeImageTrace;
Expand All @@ -42,7 +40,7 @@ class NacosUpdatingListenerTest extends NacosTestBase {
private static volatile List<Endpoint> sampleEndpoints;

@BeforeAll
static void startServers() throws JsonProcessingException {
static void startServers() {

await().pollInSameThread().pollInterval(Duration.ofSeconds(1)).untilAsserted(() -> {
assertThatCode(() -> {
Expand All @@ -69,7 +67,7 @@ static void startServers() throws JsonProcessingException {
}

@AfterAll
static void stopServers() throws Exception {
static void stopServers() {
servers.forEach(Server::close);
servers.clear();
}
Expand Down