diff --git a/brave/brave6/src/test/java/com/linecorp/armeria/client/brave/BraveClientIntegrationTest.java b/brave/brave6/src/test/java/com/linecorp/armeria/client/brave/BraveClientIntegrationTest.java index 6470fb97ea7..a62137c9b0c 100644 --- a/brave/brave6/src/test/java/com/linecorp/armeria/client/brave/BraveClientIntegrationTest.java +++ b/brave/brave6/src/test/java/com/linecorp/armeria/client/brave/BraveClientIntegrationTest.java @@ -19,7 +19,6 @@ import java.util.List; import java.util.function.BiConsumer; -import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; @@ -27,7 +26,6 @@ import com.google.common.collect.ImmutableList; -import com.linecorp.armeria.client.ClientFactory; import com.linecorp.armeria.client.WebClient; import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpRequest; @@ -43,16 +41,6 @@ abstract class BraveClientIntegrationTest extends ITHttpAsyncClient { - /** - * OkHttp's MockWebServer does not support H2C with HTTP/1 upgrade request. - */ - private static final ClientFactory clientFactoryWithoutUpgradeRequest = - ClientFactory.builder().useHttp2Preface(true).build(); - - @AfterAll static void closeClientFactory() { - clientFactoryWithoutUpgradeRequest.closeAsync(); - } - private final List protocols; private final SessionProtocol sessionProtocol; @@ -81,7 +69,6 @@ protected CurrentTraceContext.Builder currentTraceContextBuilder() { @Override protected WebClient newClient(int port) { return WebClient.builder(sessionProtocol.uriText() + "://127.0.0.1:" + port) - .factory(clientFactoryWithoutUpgradeRequest) .decorator(BraveClient.newDecorator(httpTracing)) .build(); } diff --git a/consul/src/main/java/com/linecorp/armeria/internal/consul/ConsulClient.java b/consul/src/main/java/com/linecorp/armeria/internal/consul/ConsulClient.java index bf571931037..a08dadaac7f 100644 --- a/consul/src/main/java/com/linecorp/armeria/internal/consul/ConsulClient.java +++ b/consul/src/main/java/com/linecorp/armeria/internal/consul/ConsulClient.java @@ -75,14 +75,15 @@ ObjectMapper getObjectMapper() { } /** - * Registers a service to Consul Agent with service ID. + * Registers a service to Consul Agent. * * @param serviceId a service ID that identifying a service * @param serviceName a service name to register * @param endpoint an endpoint of service to register * @param check a check for the service * @param tags tags for the service - * @return a {@link CompletableFuture} that will be completed with the registered service ID + * + * @return an HttpResponse representing the HTTP response from Consul */ public HttpResponse register(String serviceId, String serviceName, Endpoint endpoint, @Nullable Check check, List tags) { @@ -90,23 +91,36 @@ public HttpResponse register(String serviceId, String serviceName, Endpoint endp } /** - * De-registers a service to Consul Agent. + * De-registers a service from Consul Agent. * * @param serviceId a service ID that identifying a service + * + * @return an HttpResponse representing the HTTP response from Consul */ public HttpResponse deregister(String serviceId) { return agentClient.deregister(serviceId); } /** - * Get registered endpoints with service name from Consul agent. + * Retrieves the list of registered endpoints for the specified service name from the Consul agent. + * + * @param serviceName the name of the service whose endpoints are to be retrieved + * + * @return a {@link CompletableFuture} which provides a list of {@link Endpoint}s */ public CompletableFuture> endpoints(String serviceName) { return endpoints(serviceName, null, null); } /** - * Get registered endpoints with service name in datacenter from Consul agent. + * Retrieves the list of registered endpoints for the specified service name and datacenter + * from the Consul agent, optionally applying a filter. + * + * @param serviceName the name of the service whose endpoints are to be retrieved + * @param datacenter the datacenter to query; if {@code null}, the default datacenter is used + * @param filter a filter expression to apply; if {@code null}, no filtering is performed + * + * @return a {@link CompletableFuture} which provides a list of {@link Endpoint}s */ public CompletableFuture> endpoints(String serviceName, @Nullable String datacenter, @Nullable String filter) { @@ -114,14 +128,25 @@ public CompletableFuture> endpoints(String serviceName, @Nullable } /** - * Returns the registered endpoints with the specified service name from Consul agent. + * Retrieves the list of healthy endpoints for the specified service name from the Consul agent. + * + * @param serviceName the name of the service whose healthy endpoints are to be retrieved + * + * @return a {@link CompletableFuture} which provides a list of healthy {@link Endpoint}s */ public CompletableFuture> healthyEndpoints(String serviceName) { return healthyEndpoints(serviceName, null, null); } /** - * Returns the registered endpoints with the specified service name in datacenter from Consul agent. + * Retrieves the list of healthy endpoints for the specified service name and datacenter + * from the Consul agent, optionally applying a filter. + * + * @param serviceName the name of the service whose healthy endpoints are to be retrieved + * @param datacenter the datacenter to query; if {@code null}, the default datacenter is used + * @param filter a filter expression to apply; if {@code null}, no filtering is performed + * + * @return a {@link CompletableFuture} which provides a list of healthy {@link Endpoint}s */ public CompletableFuture> healthyEndpoints(String serviceName, @Nullable String datacenter, @Nullable String filter) { diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java index f3d60aea270..818795f4a3f 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpClientDelegate.java @@ -15,6 +15,7 @@ */ package com.linecorp.armeria.client; +import static com.linecorp.armeria.internal.common.util.IpAddrUtil.isCreatedWithIpAddressOnly; import static java.util.Objects.requireNonNull; import java.net.InetAddress; @@ -88,24 +89,36 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Ex } final SessionProtocol protocol = ctx.sessionProtocol(); - final ProxyConfig proxyConfig; + + final Endpoint endpointWithPort = endpoint.withDefaultPort(ctx.sessionProtocol()); + final EventLoop eventLoop = ctx.eventLoop().withoutContext(); + // TODO(ikhoon) Use ctx.exchangeType() to create an optimized HttpResponse for non-streaming response. + final DecodedHttpResponse res = new DecodedHttpResponse(eventLoop); + updateCancellationTask(ctx, req, res); + try { - proxyConfig = getProxyConfig(protocol, endpoint); + resolveProxyConfig(protocol, endpoint, ctx, (proxyConfig, thrown) -> { + if (thrown != null) { + earlyFailedResponse(thrown, ctx, res); + } else { + assert proxyConfig != null; + execute0(ctx, endpointWithPort, req, res, proxyConfig); + } + }); } catch (Throwable t) { return earlyFailedResponse(t, ctx); } + return res; + } + private void execute0(ClientRequestContext ctx, Endpoint endpointWithPort, HttpRequest req, + DecodedHttpResponse res, ProxyConfig proxyConfig) { final Throwable cancellationCause = ctx.cancellationCause(); if (cancellationCause != null) { - return earlyFailedResponse(cancellationCause, ctx); + earlyFailedResponse(cancellationCause, ctx, res); + return; } - final Endpoint endpointWithPort = endpoint.withDefaultPort(ctx.sessionProtocol()); - final EventLoop eventLoop = ctx.eventLoop().withoutContext(); - // TODO(ikhoon) Use ctx.exchangeType() to create an optimized HttpResponse for non-streaming response. - final DecodedHttpResponse res = new DecodedHttpResponse(eventLoop); - updateCancellationTask(ctx, req, res); - final ClientConnectionTimingsBuilder timingsBuilder = ClientConnectionTimings.builder(); if (endpointWithPort.hasIpAddr() || @@ -125,8 +138,6 @@ public HttpResponse execute(ClientRequestContext ctx, HttpRequest req) throws Ex } }); } - - return res; } private static void updateCancellationTask(ClientRequestContext ctx, HttpRequest req, @@ -215,24 +226,52 @@ private void acquireConnectionAndExecute0(ClientRequestContext ctx, Endpoint end } } - private ProxyConfig getProxyConfig(SessionProtocol protocol, Endpoint endpoint) { - final ProxyConfig proxyConfig = factory.proxyConfigSelector().select(protocol, endpoint); - requireNonNull(proxyConfig, "proxyConfig"); + private void resolveProxyConfig(SessionProtocol protocol, Endpoint endpoint, ClientRequestContext ctx, + BiConsumer<@Nullable ProxyConfig, @Nullable Throwable> onComplete) { + final ProxyConfig unresolvedProxyConfig = factory.proxyConfigSelector().select(protocol, endpoint); + requireNonNull(unresolvedProxyConfig, "unresolvedProxyConfig"); + final ProxyConfig proxyConfig = maybeSetHAProxySourceAddress(unresolvedProxyConfig); - // special behavior for haproxy when sourceAddress is null - if (proxyConfig.proxyType() == ProxyType.HAPROXY && - ((HAProxyConfig) proxyConfig).sourceAddress() == null) { - final InetSocketAddress proxyAddress = proxyConfig.proxyAddress(); + final InetSocketAddress proxyAddress = proxyConfig.proxyAddress(); + final boolean needsDnsResolution = proxyAddress != null && !isCreatedWithIpAddressOnly(proxyAddress); + if (needsDnsResolution) { assert proxyAddress != null; + final Future resolveFuture = addressResolverGroup + .getResolver(ctx.eventLoop().withoutContext()) + .resolve(createUnresolvedAddressForRefreshing(proxyAddress)); - // use proxy information in context if available - final ServiceRequestContext serviceCtx = ServiceRequestContext.currentOrNull(); - if (serviceCtx != null) { - final ProxiedAddresses proxiedAddresses = serviceCtx.proxiedAddresses(); - return ProxyConfig.haproxy(proxyAddress, proxiedAddresses.sourceAddress()); - } + resolveFuture.addListener(future -> { + if (future.isSuccess()) { + final InetSocketAddress resolvedAddress = (InetSocketAddress) future.getNow(); + final ProxyConfig newProxyConfig = proxyConfig.withProxyAddress(resolvedAddress); + onComplete.accept(newProxyConfig, null); + } else { + final Throwable cause = future.cause(); + onComplete.accept(null, cause); + } + }); + } else { + onComplete.accept(proxyConfig, null); } + } + private static ProxyConfig maybeSetHAProxySourceAddress(ProxyConfig proxyConfig) { + if (proxyConfig.proxyType() != ProxyType.HAPROXY) { + return proxyConfig; + } + if (((HAProxyConfig) proxyConfig).sourceAddress() != null) { + return proxyConfig; + } + + final ServiceRequestContext sctx = ServiceRequestContext.currentOrNull(); + final ProxiedAddresses serviceProxiedAddresses = sctx == null ? null : sctx.proxiedAddresses(); + if (serviceProxiedAddresses != null) { + // A special behavior for haproxy when sourceAddress is null. + // Use proxy information in the service context if available. + final InetSocketAddress proxyAddress = proxyConfig.proxyAddress(); + assert proxyAddress != null; + return ProxyConfig.haproxy(proxyAddress, serviceProxiedAddresses.sourceAddress()); + } return proxyConfig; } @@ -253,6 +292,15 @@ private static HttpResponse earlyFailedResponse(Throwable t, ClientRequestContex return HttpResponse.ofFailure(cause); } + private static HttpResponse earlyFailedResponse(Throwable t, + ClientRequestContext ctx, + DecodedHttpResponse response) { + final UnprocessedRequestException cause = UnprocessedRequestException.of(t); + ctx.cancel(cause); + response.close(cause); + return response; + } + private static void doExecute(PooledChannel pooledChannel, ClientRequestContext ctx, HttpRequest req, DecodedHttpResponse res) { final Channel channel = pooledChannel.get(); @@ -260,4 +308,8 @@ private static void doExecute(PooledChannel pooledChannel, ClientRequestContext res.init(session.inboundTrafficController()); session.invoke(pooledChannel, ctx, req, res); } + + private static InetSocketAddress createUnresolvedAddressForRefreshing(InetSocketAddress previousAddress) { + return InetSocketAddress.createUnresolved(previousAddress.getHostString(), previousAddress.getPort()); + } } diff --git a/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java b/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java index dfd3a91fb6a..695557a68cc 100644 --- a/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java +++ b/core/src/main/java/com/linecorp/armeria/client/HttpResponseWrapper.java @@ -25,6 +25,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.annotations.VisibleForTesting; + import com.linecorp.armeria.common.HttpData; import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.HttpObject; @@ -58,6 +60,8 @@ class HttpResponseWrapper implements StreamWriter { private final EventLoop eventLoop; private final ClientRequestContext ctx; private final long maxContentLength; + @VisibleForTesting + static final String UNEXPECTED_EXCEPTION_MSG = "Unexpected exception while closing a request"; private boolean responseStarted; private long contentLengthHeaderValue = -1; @@ -232,14 +236,16 @@ void close(@Nullable Throwable cause, boolean cancel) { requestAutoAbortDelayMillis, TimeUnit.MILLISECONDS); } - private void closeAction(@Nullable Throwable cause) { + private boolean closeAction(@Nullable Throwable cause) { + final boolean closed; if (cause != null) { - delegate.close(cause); + closed = delegate.tryClose(cause); ctx.logBuilder().endResponse(cause); } else { - delegate.close(); + closed = delegate.tryClose(); ctx.logBuilder().endResponse(); } + return closed; } private void cancelAction(@Nullable Throwable cause) { @@ -262,8 +268,10 @@ private void cancelTimeoutAndLog(@Nullable Throwable cause, boolean cancel) { cancelAction(cause); return; } - if (delegate.isOpen()) { - closeAction(cause); + + // don't log if the cause will be exposed via the response/log + if (delegate.isOpen() && closeAction(cause)) { + return; } // the context has been cancelled either by timeout or by user invocation @@ -275,7 +283,7 @@ private void cancelTimeoutAndLog(@Nullable Throwable cause, boolean cancel) { return; } - final StringBuilder logMsg = new StringBuilder("Unexpected exception while closing a request"); + final StringBuilder logMsg = new StringBuilder(UNEXPECTED_EXCEPTION_MSG); final HttpRequest request = ctx.request(); assert request != null; final String authority = request.authority(); diff --git a/core/src/main/java/com/linecorp/armeria/client/RefreshingAddressResolver.java b/core/src/main/java/com/linecorp/armeria/client/RefreshingAddressResolver.java index 3a2e6c61a6a..1b31102eb1f 100644 --- a/core/src/main/java/com/linecorp/armeria/client/RefreshingAddressResolver.java +++ b/core/src/main/java/com/linecorp/armeria/client/RefreshingAddressResolver.java @@ -161,7 +161,7 @@ private CompletableFuture sendQueries(List questions, S return resolver.resolve(questions, hostname).handle((records, cause) -> { if (cause != null) { cause = Exceptions.peel(cause); - return new CacheEntry(hostname, null, questions, creationTimeNanos, cause); + return new CacheEntry(executor(), hostname, null, questions, creationTimeNanos, cause); } InetAddress inetAddress = null; @@ -175,17 +175,18 @@ private CompletableFuture sendQueries(List questions, S break; } catch (UnknownHostException e) { // Should never reach here because we already validated it in extractAddressBytes. - return new CacheEntry(hostname, null, questions, creationTimeNanos, + return new CacheEntry(executor(), hostname, null, questions, creationTimeNanos, new IllegalArgumentException("Invalid address: " + hostname, e)); } } if (inetAddress == null) { - return new CacheEntry(hostname, null, questions, creationTimeNanos, new UnknownHostException( - "failed to receive DNS records for " + hostname)); + return new CacheEntry(executor(), hostname, null, questions, creationTimeNanos, + new UnknownHostException( + "failed to receive DNS records for " + hostname)); } - return new CacheEntry(hostname, inetAddress, questions, creationTimeNanos, null); + return new CacheEntry(executor(), hostname, inetAddress, questions, creationTimeNanos, null); }); } @@ -220,8 +221,7 @@ public void onRemoval(DnsQuestion question, @Nullable List records, final CacheEntry entry = addressResolverCache.getIfPresent(hostname); if (entry != null) { if (entry.refreshable()) { - // onRemoval is invoked by the executor of 'dnsResolverCache'. - executor().execute(entry::refresh); + entry.refresh(); } else { // Remove the old CacheEntry. addressResolverCache.invalidate(hostname); @@ -263,6 +263,7 @@ public void close() { final class CacheEntry { + private final EventExecutor executor; private final String hostname; @Nullable private final InetAddress address; @@ -280,8 +281,10 @@ final class CacheEntry { private ScheduledFuture retryFuture; private int numAttemptsSoFar = 1; - CacheEntry(String hostname, @Nullable InetAddress address, List questions, - @Nullable Long originalCreationTimeNanos, @Nullable Throwable cause) { + CacheEntry(EventExecutor executor, String hostname, @Nullable InetAddress address, + List questions, @Nullable Long originalCreationTimeNanos, + @Nullable Throwable cause) { + this.executor = executor; this.hostname = hostname; this.address = address; this.questions = questions; @@ -302,8 +305,8 @@ final class CacheEntry { cacheable = !DnsUtil.isDnsQueryTimedOut(unknownHostException.getCause()); if (cacheable) { - negativeCacheFuture = executor().schedule(() -> addressResolverCache.invalidate(hostname), - negativeTtl, TimeUnit.SECONDS); + negativeCacheFuture = executor.schedule(() -> addressResolverCache.invalidate(hostname), + negativeTtl, TimeUnit.SECONDS); } } this.cacheable = cacheable; @@ -330,7 +333,7 @@ Throwable cause() { } void clear() { - executor().execute(() -> { + executor.execute(() -> { if (retryFuture != null) { retryFuture.cancel(false); } @@ -341,6 +344,14 @@ void clear() { } void refresh() { + if (executor.inEventLoop()) { + refresh0(); + } else { + executor.execute(this::refresh0); + } + } + + void refresh0() { if (resolverClosed) { return; } @@ -354,10 +365,10 @@ void refresh() { final String hostname = address.getHostName(); // 'sendQueries()' always successfully completes. sendQueries(questions, hostname, originalCreationTimeNanos).thenAccept(entry -> { - if (executor().inEventLoop()) { + if (executor.inEventLoop()) { maybeUpdate(hostname, entry); } else { - executor().execute(() -> maybeUpdate(hostname, entry)); + executor.execute(() -> maybeUpdate(hostname, entry)); } }); } @@ -381,7 +392,7 @@ private Object maybeUpdate(String hostname, CacheEntry entry) { if (retryFuture != null) { retryFuture.cancel(false); } - this.retryFuture = executor().schedule(() -> { + this.retryFuture = executor.schedule(() -> { if (refreshable()) { refresh(); } else { @@ -440,6 +451,7 @@ long originalCreationTimeNanos() { @Override public String toString() { return MoreObjects.toStringHelper(this).omitNullValues() + .add("executor", executor) .add("hostname", hostname) .add("address", address) .add("questions", questions) diff --git a/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroup.java b/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroup.java index 637da21fe30..ea56d7b719e 100644 --- a/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroup.java +++ b/core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroup.java @@ -194,7 +194,7 @@ private void setCandidates(List endpoints) { } initialized = true; destroyOldContexts(contextGroup); - setEndpoints(allHealthyEndpoints()); + setEndpoints0(allHealthyEndpoints()); return null; }); } finally { @@ -291,6 +291,12 @@ private void destroyOldContexts(HealthCheckContextGroup contextGroup) { } private void updateHealth(Endpoint endpoint, boolean health) { + if (isClosing()) { + logger.debug("HealthCheckedEndpointGroup is closed. Ignoring health update for: {}. (healthy: {})", + endpoint, health); + return; + } + final boolean updated; // A healthy endpoint should be a valid checker context. if (health && findContext(endpoint) != null) { @@ -303,8 +309,15 @@ private void updateHealth(Endpoint endpoint, boolean health) { // Each new health status will be updated after initialization of the first context group. if (updated && initialized) { - setEndpoints(allHealthyEndpoints()); + setEndpoints0(allHealthyEndpoints()); + } + } + + private void setEndpoints0(List endpoints) { + if (isClosing()) { + return; } + setEndpoints(endpoints); } @Override diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java index 085609d2d41..7cb08d5dda2 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ConnectProxyConfig.java @@ -90,6 +90,12 @@ public ProxyType proxyType() { return ProxyType.CONNECT; } + @Override + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { + return new ConnectProxyConfig(newProxyAddress, this.username, + this.password, this.headers, this.useTls); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java index 3a08c219e24..7eb33e83ee0 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/DirectProxyConfig.java @@ -40,6 +40,12 @@ public InetSocketAddress proxyAddress() { return null; } + @Override + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { + throw new UnsupportedOperationException( + "A proxy address can't be set to DirectProxyConfig."); + } + @Override public String toString() { return "DirectProxyConfig{proxyType=DIRECT}"; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java index f300e524f80..2633755e043 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/HAProxyConfig.java @@ -17,6 +17,7 @@ package com.linecorp.armeria.client.proxy; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; import java.net.InetSocketAddress; import java.util.Objects; @@ -42,8 +43,11 @@ public final class HAProxyConfig extends ProxyConfig { } HAProxyConfig(InetSocketAddress proxyAddress, InetSocketAddress sourceAddress) { - checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), - "sourceAddress and proxyAddress should be the same type"); + // If proxyAddress is unresolved, getAddress() will return null. + if (proxyAddress.getAddress() != null) { + checkArgument(sourceAddress.getAddress().getClass() == proxyAddress.getAddress().getClass(), + "sourceAddress and proxyAddress should be the same type"); + } this.proxyAddress = proxyAddress; this.sourceAddress = sourceAddress; } @@ -67,6 +71,13 @@ public InetSocketAddress sourceAddress() { return sourceAddress; } + @Override + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { + requireNonNull(newProxyAddress, "newProxyAddress"); + return this.sourceAddress == null ? new HAProxyConfig(newProxyAddress) + : new HAProxyConfig(newProxyAddress, this.sourceAddress); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java index 736124b8c91..e26c2590b01 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/ProxyConfig.java @@ -40,7 +40,6 @@ public abstract class ProxyConfig { */ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new Socks4ProxyConfig(proxyAddress, null); } @@ -52,7 +51,6 @@ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress) { */ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, String username) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new Socks4ProxyConfig(proxyAddress, requireNonNull(username, "username")); } @@ -63,7 +61,6 @@ public static Socks4ProxyConfig socks4(InetSocketAddress proxyAddress, String us */ public static Socks5ProxyConfig socks5(InetSocketAddress proxyAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new Socks5ProxyConfig(proxyAddress, null, null); } @@ -77,7 +74,6 @@ public static Socks5ProxyConfig socks5(InetSocketAddress proxyAddress) { public static Socks5ProxyConfig socks5( InetSocketAddress proxyAddress, String username, String password) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new Socks5ProxyConfig(proxyAddress, requireNonNull(username, "username"), requireNonNull(password, "password")); } @@ -89,7 +85,6 @@ public static Socks5ProxyConfig socks5( */ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), false); } @@ -101,7 +96,6 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress) { */ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, boolean useTls) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new ConnectProxyConfig(proxyAddress, null, null, HttpHeaders.of(), useTls); } @@ -129,7 +123,6 @@ public static ConnectProxyConfig connect( public static ConnectProxyConfig connect( InetSocketAddress proxyAddress, HttpHeaders headers, boolean useTls) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new ConnectProxyConfig(proxyAddress, null, null, headers, useTls); } @@ -146,7 +139,6 @@ public static ConnectProxyConfig connect( public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, String username, String password, HttpHeaders headers, boolean useTls) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); requireNonNull(username, "username"); requireNonNull(password, "password"); requireNonNull(headers, "headers"); @@ -162,7 +154,6 @@ public static ConnectProxyConfig connect(InetSocketAddress proxyAddress, String public static HAProxyConfig haproxy( InetSocketAddress proxyAddress, InetSocketAddress sourceAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); requireNonNull(sourceAddress, "sourceAddress"); checkArgument(!sourceAddress.isUnresolved(), "sourceAddress must be resolved"); return new HAProxyConfig(proxyAddress, sourceAddress); @@ -176,7 +167,6 @@ public static HAProxyConfig haproxy( */ public static ProxyConfig haproxy(InetSocketAddress proxyAddress) { requireNonNull(proxyAddress, "proxyAddress"); - checkArgument(!proxyAddress.isUnresolved(), "proxyAddress must be resolved"); return new HAProxyConfig(proxyAddress); } @@ -201,6 +191,12 @@ public static ProxyConfig direct() { @Nullable public abstract InetSocketAddress proxyAddress(); + /** + * Returns a new proxy address instance that respects DNS TTL. + * @param newProxyAddress the inet socket address + */ + public abstract ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress); + @Nullable static String maskPassword(@Nullable String username, @Nullable String password) { return username != null ? "****" : null; diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java index 49adc940602..d8f76ef6fa5 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks4ProxyConfig.java @@ -56,6 +56,11 @@ public ProxyType proxyType() { return ProxyType.SOCKS4; } + @Override + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { + return new Socks4ProxyConfig(newProxyAddress, this.username); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java index 8e541b8c041..b100ff90439 100644 --- a/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java +++ b/core/src/main/java/com/linecorp/armeria/client/proxy/Socks5ProxyConfig.java @@ -69,6 +69,11 @@ public ProxyType proxyType() { return ProxyType.SOCKS5; } + @Override + public ProxyConfig withProxyAddress(InetSocketAddress newProxyAddress) { + return new Socks5ProxyConfig(newProxyAddress, this.username, this.password); + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { diff --git a/core/src/main/java/com/linecorp/armeria/client/retry/AbstractBackoffBuilder.java b/core/src/main/java/com/linecorp/armeria/client/retry/AbstractBackoffBuilder.java new file mode 100644 index 00000000000..c2c194f1d05 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/client/retry/AbstractBackoffBuilder.java @@ -0,0 +1,92 @@ +/* + * Copyright 2024 LY Corporation + * + * LY Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.client.retry; + +import static java.util.Objects.requireNonNull; + +import java.util.Random; +import java.util.concurrent.ThreadLocalRandom; +import java.util.function.Supplier; + +import com.linecorp.armeria.common.annotation.Nullable; + +/** + * A skeletal builder implementation for {@link Backoff}. + */ +abstract class AbstractBackoffBuilder> { + @Nullable + private Double minJitterRate; + @Nullable + private Double maxJitterRate; + @Nullable + private Integer maxAttempts; + @Nullable + private Supplier randomSupplier; + + @SuppressWarnings("unchecked") + private SELF self() { + return (SELF) this; + } + + /** + * Sets the minimum and maximum jitter rates to apply to the delay. + */ + public final SELF jitter(double minJitterRate, double maxJitterRate) { + this.minJitterRate = minJitterRate; + this.maxJitterRate = maxJitterRate; + return self(); + } + + /** + * Sets the minimum and maximum jitter rates to apply to the delay, as well as a + * custom {@link Random} supplier for generating the jitter. + */ + public final SELF jitter(double minJitterRate, double maxJitterRate, Supplier randomSupplier) { + requireNonNull(randomSupplier, "randomSupplier"); + this.minJitterRate = minJitterRate; + this.maxJitterRate = maxJitterRate; + this.randomSupplier = randomSupplier; + return self(); + } + + /** + * Sets the maximum number of attempts. + */ + public final SELF maxAttempts(int maxAttempts) { + this.maxAttempts = maxAttempts; + return self(); + } + + abstract Backoff doBuild(); + + /** + * Builds and returns {@link Backoff} instance with configured properties. + */ + public final Backoff build() { + Backoff backoff = doBuild(); + if (minJitterRate != null && maxJitterRate != null) { + Supplier randomSupplier = this.randomSupplier; + if (randomSupplier == null) { + randomSupplier = ThreadLocalRandom::current; + } + backoff = new JitterAddingBackoff(backoff, minJitterRate, maxJitterRate, randomSupplier); + } + if (maxAttempts != null) { + backoff = new AttemptLimitingBackoff(backoff, maxAttempts); + } + return backoff; + } +} diff --git a/core/src/main/java/com/linecorp/armeria/client/retry/Backoff.java b/core/src/main/java/com/linecorp/armeria/client/retry/Backoff.java index 6ec01aa13f1..5b8a289fd33 100644 --- a/core/src/main/java/com/linecorp/armeria/client/retry/Backoff.java +++ b/core/src/main/java/com/linecorp/armeria/client/retry/Backoff.java @@ -25,6 +25,7 @@ import com.linecorp.armeria.common.Flags; import com.linecorp.armeria.common.annotation.Nullable; +import com.linecorp.armeria.common.annotation.UnstableApi; import com.linecorp.armeria.common.util.Unwrappable; /** @@ -131,6 +132,60 @@ static Backoff of(String specification) { return BackoffSpec.parse(specification).build(); } + /** + * Returns an {@link ExponentialBackoffBuilder} that provides methods to configure + * backoff delay which is exponentially-increasing. The default values are as follows: + *
    + *
  • {@code initialDelayMillis}: {@value ExponentialBackoffBuilder#DEFAULT_INITIAL_DELAY_MILLIS}
    • + *
  • {@code maxDelayMillis}: {@value ExponentialBackoffBuilder#DEFAULT_MAX_DELAY_MILLIS}
    • + *
  • {@code multiplier}: {@value ExponentialBackoffBuilder#DEFAULT_MULTIPLIER}
    • + *
+ */ + @UnstableApi + static ExponentialBackoffBuilder builderForExponential() { + return new ExponentialBackoffBuilder(); + } + + /** + * Returns a {@link FibonacciBackoffBuilder} that provides methods to configure + * backoff delay which follows fibonacci sequence. + * f(n) = f(n-1) + f(n-2) where f(0) = f(1) = {@code initialDelayMillis} + * The default values are as follows: + *
    + *
  • {@code initialDelayMillis}: {@value FibonacciBackoffBuilder#DEFAULT_INITIAL_DELAY_MILLIS}
    • + *
  • {@code maxDelayMillis}: {@value FibonacciBackoffBuilder#DEFAULT_MAX_DELAY_MILLIS}
    • + *
+ */ + @UnstableApi + static FibonacciBackoffBuilder builderForFibonacci() { + return new FibonacciBackoffBuilder(); + } + + /** + * Returns a {@link FixedBackoffBuilder} that provides methods to configure + * backoff delay which is a fixed value. The default values are as follows: + *
    + *
  • {@code delayMillis}: {@value FixedBackoffBuilder#DEFAULT_DELAY_MILLIS}
    • + *
+ */ + @UnstableApi + static FixedBackoffBuilder builderForFixed() { + return new FixedBackoffBuilder(); + } + + /** + * Returns a {@link RandomBackoffBuilder} that provides methods to configure + * backoff delay which is a random value. The default values are as follows: + *
    + *
  • {@code minDelayMillis}: {@value RandomBackoffBuilder#DEFAULT_MIN_DELAY_MILLIS}
    • + *
  • {@code maxDelayMillis}: {@value RandomBackoffBuilder#DEFAULT_MAX_DELAY_MILLIS}
    • + *
+ */ + @UnstableApi + static RandomBackoffBuilder builderForRandom() { + return new RandomBackoffBuilder(); + } + /** * Returns the number of milliseconds to wait for before attempting a retry. * diff --git a/core/src/main/java/com/linecorp/armeria/client/retry/ExponentialBackoffBuilder.java b/core/src/main/java/com/linecorp/armeria/client/retry/ExponentialBackoffBuilder.java new file mode 100644 index 00000000000..5d1b4d1469e --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/client/retry/ExponentialBackoffBuilder.java @@ -0,0 +1,103 @@ +/* + * Copyright 2024 LY Corporation + * + * LY Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.client.retry; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.linecorp.armeria.common.annotation.UnstableApi; + +/** + * A builder for creating instances of Exponential {@link Backoff}. + * + *

This builder allows you to configure an exponential backoff strategy by specifying + * the initial delay, the maximum delay, and a multiplier. The exponential backoff + * increases the delay between retries exponentially, starting from the initial delay and + * multiplying the delay by the specified multiplier after each retry, up to the maximum delay.

+ * + *

Example usage:

+ * + * 
+ * {@code
+ * Backoff backoff = Backoff.builderForExponential()
+ *     .initialDelayMillis(200)
+ *     .maxDelayMillis(10000)
+ *     .multiplier(2.0)
+ *     .build();
+ * }
+ *
+ */ +@UnstableApi +public final class ExponentialBackoffBuilder extends AbstractBackoffBuilder { + + static final long DEFAULT_INITIAL_DELAY_MILLIS = 200; + static final long DEFAULT_MAX_DELAY_MILLIS = 10000; + static final double DEFAULT_MULTIPLIER = 2.0; + + private long initialDelayMillis = DEFAULT_INITIAL_DELAY_MILLIS; + private long maxDelayMillis = DEFAULT_MAX_DELAY_MILLIS; + private double multiplier = DEFAULT_MULTIPLIER; + + ExponentialBackoffBuilder() {} + + /** + * Sets the initial delay in milliseconds for the Exponential {@link Backoff}. + * + *

The initial delay is the starting value for the exponential backoff, determining + * the delay before the first retry. Subsequent delays will increase exponentially + * based on the multiplier.

+ * + * @param initialDelayMillis the initial delay in milliseconds + */ + public ExponentialBackoffBuilder initialDelayMillis(long initialDelayMillis) { + checkArgument(initialDelayMillis >= 0, "initialDelayMillis: %s (expected: >= 0)", initialDelayMillis); + this.initialDelayMillis = initialDelayMillis; + return this; + } + + /** + * Sets the maximum delay in milliseconds for the Exponential {@link Backoff}. + * + *

The maximum delay is the upper limit for the backoff delay. Once the delay reaches + * this value, it will not increase further, even if the multiplier would result in a higher value.

+ * + * @param maxDelayMillis the maximum delay in milliseconds + */ + public ExponentialBackoffBuilder maxDelayMillis(long maxDelayMillis) { + checkArgument(maxDelayMillis >= 0, "maxDelayMillis: %s (expected: >= 0)", maxDelayMillis); + this.maxDelayMillis = maxDelayMillis; + return this; + } + + /** + * Sets the multiplier for the Exponential {@link Backoff}. + * + *

The multiplier controls how much the delay increases after each retry. + * The delay for each retry is determined by multiplying the previous delay by this value, + * until the maximum delay is reached.

+ * + * @param multiplier the multiplier for the exponential backoff + */ + public ExponentialBackoffBuilder multiplier(double multiplier) { + checkArgument(multiplier > 1.0, "multiplier: %s (expected: > 1.0)", multiplier); + this.multiplier = multiplier; + return this; + } + + @Override + Backoff doBuild() { + return new ExponentialBackoff(initialDelayMillis, maxDelayMillis, multiplier); + } +} diff --git a/core/src/main/java/com/linecorp/armeria/client/retry/FibonacciBackoffBuilder.java b/core/src/main/java/com/linecorp/armeria/client/retry/FibonacciBackoffBuilder.java new file mode 100644 index 00000000000..7fa0feaf1fa --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/client/retry/FibonacciBackoffBuilder.java @@ -0,0 +1,87 @@ +/* + * Copyright 2024 LY Corporation + * + * LY Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.client.retry; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.linecorp.armeria.common.annotation.UnstableApi; + +/** + * A builder for creating instances of Fibonacci {@link Backoff}. + * + *

This builder allows you to configure a Fibonacci backoff strategy by specifying + * an initial delay and a maximum delay in milliseconds. The Fibonacci backoff strategy + * increases the delay between retries according to the Fibonacci sequence, while respecting + * the configured maximum delay.

+ * + *

Example usage:

+ * + * 
+ * {@code
+ * Backoff backoff = Backoff.builderForFibonacci()
+ *     .initialDelayMillis(200)
+ *     .maxDelayMillis(10000)
+ *     .build();
+ * }
+ *
+ */ +@UnstableApi +public final class FibonacciBackoffBuilder extends AbstractBackoffBuilder { + + static final long DEFAULT_INITIAL_DELAY_MILLIS = 200; + static final long DEFAULT_MAX_DELAY_MILLIS = 10000; + + private long initialDelayMillis = 200; + private long maxDelayMillis = 10000; + + FibonacciBackoffBuilder() {} + + /** + * Sets the initial delay in milliseconds for the Fibonacci {@link Backoff}. + * + *

The initial delay is the base value from which the Fibonacci sequence will start, + * and it determines the delay before the first retry.

+ * + * @param initialDelayMillis the initial delay in milliseconds + */ + public FibonacciBackoffBuilder initialDelayMillis(long initialDelayMillis) { + checkArgument(initialDelayMillis >= 0, + "initialDelayMillis: %s (expected: >= 0)", initialDelayMillis); + + this.initialDelayMillis = initialDelayMillis; + return this; + } + + /** + * Sets the maximum delay in milliseconds for the Fibonacci {@link Backoff}. + * + *

The maximum delay sets an upper limit to the delays generated by the Fibonacci + * sequence. Once the delays reach this value, they will not increase further.

+ * + * @param maxDelayMillis the maximum delay in milliseconds + */ + public FibonacciBackoffBuilder maxDelayMillis(long maxDelayMillis) { + checkArgument(maxDelayMillis >= 0, + "maxDelayMillis: %s (expected: >= 0)", maxDelayMillis); + this.maxDelayMillis = maxDelayMillis; + return this; + } + + @Override + Backoff doBuild() { + return new FibonacciBackoff(initialDelayMillis, maxDelayMillis); + } +} diff --git a/core/src/main/java/com/linecorp/armeria/client/retry/FixedBackoffBuilder.java b/core/src/main/java/com/linecorp/armeria/client/retry/FixedBackoffBuilder.java new file mode 100644 index 00000000000..4893fc85b10 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/client/retry/FixedBackoffBuilder.java @@ -0,0 +1,65 @@ +/* + * Copyright 2024 LY Corporation + * + * LY Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.client.retry; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.linecorp.armeria.common.annotation.UnstableApi; + +/** + * A builder for creating instances of fixed {@link Backoff}. + * + *

This builder allows you to configure the delay duration for a fixed backoff strategy. + * You can specify the delay in milliseconds and then create a Fixed {@link Backoff} instance + * with the configured delay.

+ * + *

Example usage:

+ * + * 
+ * {@code
+ * Backoff backoff = Backoff.builderForFixed()
+ *     .delayMillis(500)
+ *     .build();
+ * }
+ *
+ */ +@UnstableApi +public final class FixedBackoffBuilder extends AbstractBackoffBuilder { + static final long DEFAULT_DELAY_MILLIS = 500; + + private long delayMillis = 500; + + FixedBackoffBuilder() {} + + /** + * Sets the delay duration in milliseconds for the Fixed {@link Backoff}. + * + *

This value determines the fixed amount of time the backoff will delay + * before retrying an operation.

+ * + * @param delayMillis the delay in milliseconds + */ + public FixedBackoffBuilder delayMillis(long delayMillis) { + checkArgument(delayMillis >= 0, "delayMillis: %s (expected: >= 0)", delayMillis); + this.delayMillis = delayMillis; + return this; + } + + @Override + Backoff doBuild() { + return new FixedBackoff(delayMillis); + } +} diff --git a/core/src/main/java/com/linecorp/armeria/client/retry/RandomBackoffBuilder.java b/core/src/main/java/com/linecorp/armeria/client/retry/RandomBackoffBuilder.java new file mode 100644 index 00000000000..404b2482812 --- /dev/null +++ b/core/src/main/java/com/linecorp/armeria/client/retry/RandomBackoffBuilder.java @@ -0,0 +1,101 @@ +/* + * Copyright 2024 LY Corporation + * + * LY Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.client.retry; + +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + +import java.util.Random; +import java.util.function.Supplier; + +import com.linecorp.armeria.common.annotation.UnstableApi; + +/** + * A builder for creating instances of Random {@link Backoff}. + * + *

This builder allows you to configure a random backoff strategy by specifying + * a minimum delay, maximum delay in milliseconds and random supplier. + *

Example usage:

+ * + * 
+ * {@code
+ * BackOff backoff = Backoff.builderForRandom()
+ *     .minDelayMillis(200)
+ *     .maxDelayMillis(10000)
+ *     .build();
+ * }
+ *
+ */ +@UnstableApi +public final class RandomBackoffBuilder extends AbstractBackoffBuilder { + + static final long DEFAULT_MIN_DELAY_MILLIS = 200; + static final long DEFAULT_MAX_DELAY_MILLIS = 10000; + + private long minDelayMillis = 200; + private long maxDelayMillis = 10000; + private Supplier randomSupplier = Random::new; + + RandomBackoffBuilder() {} + + /** + * Sets the minimum delay, in milliseconds, for the Random {@link Backoff}. + * + *

This value represents the minimum time the backoff will delay before retrying an operation.

+ * + * @param minDelayMillis the minimum delay in milliseconds + * @return this {@code RandomBackoffBuilder} instance for method chaining + */ + public RandomBackoffBuilder minDelayMillis(long minDelayMillis) { + checkArgument(minDelayMillis >= 0, "minDelayMillis: %s (expected: >= 0)", minDelayMillis); + this.minDelayMillis = minDelayMillis; + return this; + } + + /** + * Sets the maximum delay, in milliseconds, for the Random {@link Backoff}. + * + *

This value represents the maximum time the backoff will delay before retrying an operation.

+ * + * @param maxDelayMillis the maximum delay in milliseconds + * @return this {@code RandomBackoffBuilder} instance for method chaining + */ + public RandomBackoffBuilder maxDelayMillis(long maxDelayMillis) { + checkArgument(maxDelayMillis >= 0, "maxDelayMillis: %s (expected: >= 0)", maxDelayMillis); + this.maxDelayMillis = maxDelayMillis; + return this; + } + + /** + * Sets the {@link Supplier} that provides instances of {@link Random} for the Random {@link Backoff}. + * + *

This supplier will be used to generate random values when determining the + * backoff delay between retries.

+ * + * @param randomSupplier a {@link Supplier} that provides {@link Random} instances + * @return this {@code RandomBackoffBuilder} instance for method chaining + */ + public RandomBackoffBuilder randomSupplier(Supplier randomSupplier) { + requireNonNull(randomSupplier, "randomSupplier"); + this.randomSupplier = randomSupplier; + return this; + } + + @Override + Backoff doBuild() { + return new RandomBackoff(minDelayMillis, maxDelayMillis, randomSupplier); + } +} diff --git a/core/src/main/java/com/linecorp/armeria/common/stream/DefaultStreamMessage.java b/core/src/main/java/com/linecorp/armeria/common/stream/DefaultStreamMessage.java index 856e593c18c..58ae76a7d38 100644 --- a/core/src/main/java/com/linecorp/armeria/common/stream/DefaultStreamMessage.java +++ b/core/src/main/java/com/linecorp/armeria/common/stream/DefaultStreamMessage.java @@ -441,28 +441,41 @@ private void handleCloseEvent(SubscriptionImpl subscription, CloseEvent o) { @Override public void close() { - if (setState(State.OPEN, State.CLOSED)) { - addObjectOrEvent(SUCCESSFUL_CLOSE); - } + tryClose(); } @Override public final void close(Throwable cause) { requireNonNull(cause, "cause"); - if (cause instanceof CancelledSubscriptionException) { - throw new IllegalArgumentException("cause: " + cause + " (must use Subscription.cancel())"); - } tryClose(cause); } + /** + * Tries to close the stream. + * + * @return {@code true} if the stream has been closed by this method call. + * {@code false} if the stream has been closed already by another party. + */ + @UnstableApi + public final boolean tryClose() { + if (setState(State.OPEN, State.CLOSED)) { + addObjectOrEvent(SUCCESSFUL_CLOSE); + return true; + } + return false; + } + /** * Tries to close the stream with the specified {@code cause}. * * @return {@code true} if the stream has been closed by this method call. - * {@code false} if the stream has been closed already by other party. + * {@code false} if the stream has been closed already by another party. */ public final boolean tryClose(Throwable cause) { + if (cause instanceof CancelledSubscriptionException) { + throw new IllegalArgumentException("cause: " + cause + " (must use Subscription.cancel())"); + } if (setState(State.OPEN, State.CLOSED)) { addObjectOrEvent(new CloseEvent(cause)); return true; diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java index 3fd74fdfe5c..b9b5a063d4b 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java +++ b/core/src/main/java/com/linecorp/armeria/internal/common/util/IpAddrUtil.java @@ -15,6 +15,9 @@ */ package com.linecorp.armeria.internal.common.util; +import java.net.InetAddress; +import java.net.InetSocketAddress; + import com.linecorp.armeria.common.annotation.Nullable; import io.netty.util.NetUtil; @@ -36,5 +39,15 @@ public static String normalize(@Nullable String ipAddr) { return NetUtil.bytesToIpAddress(array); } + public static boolean isCreatedWithIpAddressOnly(InetSocketAddress socketAddress) { + if (socketAddress.isUnresolved()) { + return false; + } + + final InetAddress inetAddress = socketAddress.getAddress(); + // If hostname and host address are the same, it was created with an IP address + return socketAddress.getHostString().equals(inetAddress.getHostAddress()); + } + private IpAddrUtil() {} } diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java b/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java index 546c4f443a0..0e93c707a8e 100644 --- a/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java +++ b/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolver.java @@ -48,6 +48,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.ServiceLoader; @@ -458,6 +459,18 @@ private static AnnotatedValueResolver of(AnnotatedElement annotatedElement, final Param param = annotatedElement.getAnnotation(Param.class); if (param != null) { final String name = findName(typeElement, param.value()); + // If the parameter is of type Map and the @Param annotation does not specify a value, + // map all query parameters into the Map. + if (Map.class.isAssignableFrom(type)) { + if (DefaultValues.isSpecified(param.value())) { + throw new IllegalArgumentException( + String.format("Invalid @Param annotation on Map parameter: '%s'. " + + "The @Param annotation specifies a value ('%s'), " + + "which is not allowed. ", + annotatedElement, param.value())); + } + return ofQueryParamMap(name, annotatedElement, typeElement, type, description); + } if (type == File.class || type == Path.class || type == MultipartFile.class) { return ofFileParam(name, annotatedElement, typeElement, type, description); } @@ -585,6 +598,25 @@ private static AnnotatedValueResolver ofQueryParam(String name, .build(); } + private static AnnotatedValueResolver ofQueryParamMap(String name, + AnnotatedElement annotatedElement, + AnnotatedElement typeElement, Class type, + DescriptionInfo description) { + + return new Builder(annotatedElement, type, name) + .annotationType(Param.class) + .typeElement(typeElement) + .description(description) + .aggregation(AggregationStrategy.FOR_FORM_DATA) + .resolver((resolver, ctx) -> ctx.queryParams().stream() + .collect(toImmutableMap( + Entry::getKey, + Entry::getValue, + (existing, replacement) -> replacement + ))) + .build(); + } + private static AnnotatedValueResolver ofFileParam(String name, AnnotatedElement annotatedElement, AnnotatedElement typeElement, Class type, diff --git a/core/src/main/resources/com/linecorp/armeria/public_suffixes.txt b/core/src/main/resources/com/linecorp/armeria/public_suffixes.txt index d53cdf13f0c..678c269f160 100644 --- a/core/src/main/resources/com/linecorp/armeria/public_suffixes.txt +++ b/core/src/main/resources/com/linecorp/armeria/public_suffixes.txt @@ -92,6 +92,7 @@ *.lcl.dev *.lclstage.dev *.linodeobjects.com +*.localto.net *.magentosite.cloud *.me-central-1.airflow.amazonaws.com *.me-south-1.airflow.amazonaws.com @@ -125,6 +126,7 @@ *.quipelements.com *.r.appspot.com *.run.app +*.s.brave.app *.s.brave.io *.sa-east-1.airflow.amazonaws.com *.sapporo.jp @@ -459,6 +461,7 @@ alesund.no algard.no aliases121.com alibaba +alibabacloudcs.com alipay allfinanz allstate @@ -1085,6 +1088,8 @@ br.it bradesco brand.se brasilia.me +brave.app +brave.io bremanger.no brescia.it bridgestone @@ -1108,6 +1113,7 @@ bss.design bt bt.it bu.no +bubbleapps.io budejju.no build builders @@ -1285,6 +1291,7 @@ cci.fr cd cd.eu.org cdn-edges.net +cdn.bubble.io cdn.cloudflare.net cdn.cloudflareanycast.net cdn.cloudflarecn.net @@ -2237,6 +2244,7 @@ edu.tr edu.tt edu.tw edu.ua +edu.ug edu.uy edu.vc edu.ve @@ -3177,6 +3185,7 @@ gov.tr gov.tt gov.tw gov.ua +gov.ug gov.uk gov.vc gov.ve @@ -4618,6 +4627,7 @@ lanxess laocai.vn lapy.pl laquila.it +laravel.cloud lardal.no larvik.no lasalle @@ -4782,6 +4792,7 @@ loans localcert.net localhostcert.net localplayer.dev +localtonet.com locker locus lodi.it @@ -4821,6 +4832,8 @@ loseyourip.com loten.no lotte lotto +lovable.app +lovableproject.com love lovepop.jp lovesick.jp @@ -5134,6 +5147,7 @@ mil.tr mil.tt mil.tw mil.tz +mil.ug mil.uy mil.vc mil.ve @@ -6039,6 +6053,7 @@ obanazawa.yamagata.jp obi obihiro.hokkaido.jp obira.hokkaido.jp +obj.ag objects.lpg.cloudscale.ch objects.rma.cloudscale.ch obl.ong @@ -8668,6 +8683,7 @@ us.ngrok.io us.org us.platform.sh us.reclaim.cloud +us.ug usa.oita.jp user.aseinet.ne.jp user.party.eus diff --git a/core/src/test/java/com/linecorp/armeria/client/DNSResolverFacadeUtils.java b/core/src/test/java/com/linecorp/armeria/client/DNSResolverFacadeUtils.java new file mode 100644 index 00000000000..c0a0eaf2d0e --- /dev/null +++ b/core/src/test/java/com/linecorp/armeria/client/DNSResolverFacadeUtils.java @@ -0,0 +1,66 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.client; + +import static com.google.common.collect.ImmutableList.toImmutableList; + +import java.net.InetSocketAddress; +import java.util.function.Function; +import java.util.stream.Stream; + +import com.linecorp.armeria.client.endpoint.dns.TestDnsServer; +import com.linecorp.armeria.client.retry.Backoff; +import com.linecorp.armeria.common.prometheus.PrometheusMeterRegistries; + +import io.netty.channel.EventLoopGroup; +import io.netty.resolver.AddressResolverGroup; +import io.netty.resolver.ResolvedAddressTypes; +import io.netty.resolver.dns.DnsServerAddressStreamProvider; +import io.netty.resolver.dns.DnsServerAddresses; + +public final class DNSResolverFacadeUtils { + + private DNSResolverFacadeUtils() { } + + public static Function> getAddressResolverGroupForTest( + TestDnsServer dnsServer) { + return eventLoopGroup -> { + final DnsResolverGroupBuilder builder = builder(dnsServer); + builder.autoRefreshBackoff(Backoff.fixed(0L)); + return builder.build(eventLoopGroup); + }; + } + + private static DnsResolverGroupBuilder builder(TestDnsServer... servers) { + return builder(true, servers); + } + + private static DnsResolverGroupBuilder builder(boolean withCacheOption, TestDnsServer... servers) { + final DnsServerAddressStreamProvider dnsServerAddressStreamProvider = + hostname -> DnsServerAddresses.sequential( + Stream.of(servers).map(TestDnsServer::addr).collect(toImmutableList())).stream(); + final DnsResolverGroupBuilder builder = new DnsResolverGroupBuilder() + .serverAddressStreamProvider(dnsServerAddressStreamProvider) + .meterRegistry(PrometheusMeterRegistries.newRegistry()) + .resolvedAddressTypes(ResolvedAddressTypes.IPV4_ONLY) + .traceEnabled(false); + if (withCacheOption) { + builder.dnsCache(DnsCache.builder().build()); + } + return builder; + } +} diff --git a/core/src/test/java/com/linecorp/armeria/client/Http2GoAwayTest.java b/core/src/test/java/com/linecorp/armeria/client/Http2GoAwayTest.java index 287f505c1af..dad61f85650 100644 --- a/core/src/test/java/com/linecorp/armeria/client/Http2GoAwayTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/Http2GoAwayTest.java @@ -15,12 +15,13 @@ */ package com.linecorp.armeria.client; -import static io.netty.handler.codec.http2.Http2CodecUtil.connectionPrefaceBuf; +import static com.linecorp.armeria.internal.testing.Http2ByteUtil.handleInitialExchange; +import static com.linecorp.armeria.internal.testing.Http2ByteUtil.newClientFactory; +import static com.linecorp.armeria.internal.testing.Http2ByteUtil.readFrame; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.BufferedOutputStream; -import java.io.IOException; import java.io.InputStream; import java.net.ServerSocket; import java.net.Socket; @@ -31,14 +32,9 @@ import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.extension.RegisterExtension; -import com.google.common.io.ByteStreams; - import com.linecorp.armeria.common.AggregatedHttpResponse; import com.linecorp.armeria.testing.junit5.common.EventLoopExtension; -import io.netty.buffer.ByteBuf; -import io.netty.buffer.Unpooled; -import io.netty.handler.codec.http2.Http2CodecUtil; import io.netty.handler.codec.http2.Http2FrameTypes; @Timeout(10) @@ -53,7 +49,7 @@ class Http2GoAwayTest { @Test void streamEndsBeforeGoAway() throws Exception { try (ServerSocket ss = new ServerSocket(0); - ClientFactory clientFactory = newClientFactory()) { + ClientFactory clientFactory = newClientFactory(eventLoop.get())) { final int port = ss.getLocalPort(); @@ -101,7 +97,7 @@ void streamEndsBeforeGoAway() throws Exception { @Test void streamEndsAfterGoAway() throws Exception { try (ServerSocket ss = new ServerSocket(0); - ClientFactory clientFactory = newClientFactory()) { + ClientFactory clientFactory = newClientFactory(eventLoop.get())) { final int port = ss.getLocalPort(); @@ -150,7 +146,7 @@ void streamEndsAfterGoAway() throws Exception { @Test void streamGreaterThanLastStreamId() throws Exception { try (ServerSocket ss = new ServerSocket(0); - ClientFactory clientFactory = newClientFactory()) { + ClientFactory clientFactory = newClientFactory(eventLoop.get())) { final int port = ss.getLocalPort(); @@ -208,55 +204,4 @@ void streamGreaterThanLastStreamId() throws Exception { } } } - - private static ClientFactory newClientFactory() { - return ClientFactory.builder() - .useHttp2Preface(true) - // Set the window size to the HTTP/2 default values to simplify the traffic. - .http2InitialConnectionWindowSize(Http2CodecUtil.DEFAULT_WINDOW_SIZE) - .http2InitialStreamWindowSize(Http2CodecUtil.DEFAULT_WINDOW_SIZE) - .workerGroup(eventLoop.get(), false) - .build(); - } - - private static void handleInitialExchange(InputStream in, BufferedOutputStream out) throws IOException { - // Read the connection preface and discard it. - readBytes(in, connectionPrefaceBuf().readableBytes()); - - // Read a SETTINGS frame. - assertThat(readFrame(in).getByte(3)).isEqualTo(Http2FrameTypes.SETTINGS); - - // Send a SETTINGS frame and the ack for the received SETTINGS frame. - sendEmptySettingsAndAckFrame(out); - - // Read a SETTINGS ack frame. - assertThat(readFrame(in).getByte(3)).isEqualTo(Http2FrameTypes.SETTINGS); - } - - private static byte[] readBytes(InputStream in, int length) throws IOException { - final byte[] buf = new byte[length]; - ByteStreams.readFully(in, buf); - return buf; - } - - private static void sendEmptySettingsAndAckFrame(BufferedOutputStream bos) throws IOException { - // Send an empty SETTINGS frame. - bos.write(new byte[] { 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00 }); - // Send a SETTINGS_ACK frame. - bos.write(new byte[] { 0x00, 0x00, 0x00, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00 }); - bos.flush(); - } - - private static int payloadLength(byte[] buf) { - return (buf[0] & 0xff) << 16 | (buf[1] & 0xff) << 8 | (buf[2] & 0xff); - } - - private static ByteBuf readFrame(InputStream in) throws IOException { - final byte[] frameBuf = readBytes(in, 9); - final int payloadLength = payloadLength(frameBuf); - final ByteBuf buffer = Unpooled.buffer(9 + payloadLength); - buffer.writeBytes(frameBuf); - buffer.writeBytes(in, payloadLength); - return buffer; - } } diff --git a/core/src/test/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroupTest.java b/core/src/test/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroupTest.java index 2b0452c5434..f6ee0f93b2f 100644 --- a/core/src/test/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroupTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/endpoint/healthcheck/HealthCheckedEndpointGroupTest.java @@ -602,6 +602,38 @@ void cacheReflectsAttributeChanges() throws InterruptedException { } } + @Test + void shouldStopUpdatingEndpointsWhenClosing() throws InterruptedException { + final AtomicInteger counter = new AtomicInteger(); + final Function checkFactory = ctx -> { + counter.incrementAndGet(); + ctx.updateHealth(HEALTHY, null, null, null); + return AsyncCloseableSupport.of(); + }; + + final Endpoint candidate1 = Endpoint.of("candidate1"); + final Endpoint candidate2 = Endpoint.of("candidate2"); + + final MockEndpointGroup delegate = new MockEndpointGroup(); + delegate.set(candidate1, candidate2, candidate2); + + final HealthCheckedEndpointGroup endpointGroup = + new HealthCheckedEndpointGroup(delegate, true, + 10000, 10000, + SessionProtocol.HTTP, 80, + DEFAULT_HEALTH_CHECK_RETRY_BACKOFF, + ClientOptions.of(), checkFactory, + HealthCheckStrategy.all(), + DEFAULT_ENDPOINT_PREDICATE); + assertThat(counter.get()).isEqualTo(2); + final EndpointComparator comparator = new EndpointComparator(); + assertThat(endpointGroup.endpoints()).usingElementComparator(comparator) + .containsOnly(candidate1, candidate2); + endpointGroup.close(); + assertThat(endpointGroup.endpoints()).usingElementComparator(comparator) + .containsOnly(candidate1, candidate2); + } + static final class MockEndpointGroup extends DynamicEndpointGroup { MockEndpointGroup() {} diff --git a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java index f1fee902870..457020f1205 100644 --- a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyClientIntegrationTest.java @@ -15,6 +15,9 @@ */ package com.linecorp.armeria.client.proxy; +import static com.linecorp.armeria.client.endpoint.dns.TestDnsServer.newAddressRecord; +import static io.netty.handler.codec.dns.DnsRecordType.A; +import static io.netty.handler.codec.dns.DnsSection.ANSWER; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; @@ -51,12 +54,15 @@ import org.junit.jupiter.params.provider.MethodSource; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.linecorp.armeria.client.ClientFactory; +import com.linecorp.armeria.client.DNSResolverFacadeUtils; import com.linecorp.armeria.client.Endpoint; import com.linecorp.armeria.client.SessionProtocolNegotiationException; import com.linecorp.armeria.client.UnprocessedRequestException; import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.client.endpoint.dns.TestDnsServer; import com.linecorp.armeria.client.logging.LoggingClient; import com.linecorp.armeria.common.AggregatedHttpResponse; import com.linecorp.armeria.common.Flags; @@ -76,6 +82,8 @@ import io.netty.channel.Channel; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.handler.codec.dns.DefaultDnsQuestion; +import io.netty.handler.codec.dns.DefaultDnsResponse; import io.netty.handler.codec.http.DefaultFullHttpResponse; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.EmptyHttpHeaders; @@ -97,6 +105,7 @@ import io.netty.handler.ssl.SslContext; import io.netty.handler.ssl.SslContextBuilder; import io.netty.handler.traffic.ChannelTrafficShapingHandler; +import io.netty.resolver.dns.DnsErrorCauseException; import io.netty.util.ReferenceCountUtil; class ProxyClientIntegrationTest { @@ -426,7 +435,7 @@ public void connectFailed(SessionProtocol protocol, Endpoint endpoint, assertThatThrownBy(responseFuture::join).isInstanceOf(CompletionException.class) .hasCauseInstanceOf(UnprocessedRequestException.class) .hasRootCauseInstanceOf(NullPointerException.class) - .hasRootCauseMessage("proxyConfig"); + .hasRootCauseMessage("unresolvedProxyConfig"); } } @@ -847,6 +856,66 @@ public void connectFailed(SessionProtocol protocol, Endpoint endpoint, } } + @Test + void testProxyConfigShouldBeResolved() throws Exception { + try (TestDnsServer server = new TestDnsServer(ImmutableMap.of( + new DefaultDnsQuestion("a.com.", A), + new DefaultDnsResponse(0).addRecord(ANSWER, newAddressRecord("a.com.", "127.0.0.1")))) + ) { + final InetSocketAddress proxySocketAddress = new InetSocketAddress( + "a.com", httpProxyServer.address().getPort()); + try (ClientFactory clientFactory = + ClientFactory.builder() + .addressResolverGroupFactory( + DNSResolverFacadeUtils.getAddressResolverGroupForTest(server)) + .proxyConfig(ProxyConfig.connect(proxySocketAddress)) + .useHttp2Preface(true) + .build()) { + + final WebClient webClient = WebClient.builder(SessionProtocol.H1C, backendServer.httpEndpoint()) + .factory(clientFactory) + .decorator(LoggingClient.newDecorator()) + .build(); + final CompletableFuture responseFuture = + webClient.get(PROXY_PATH).aggregate(); + final AggregatedHttpResponse response = responseFuture.join(); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(response.contentUtf8()).isEqualTo(SUCCESS_RESPONSE); + assertThat(numSuccessfulProxyRequests).isEqualTo(1); + } + } + } + + @Test + void testProxyConfigShouldBeFailedToResolved() throws Exception { + try (TestDnsServer server = new TestDnsServer(ImmutableMap.of( + new DefaultDnsQuestion("b.com.", A), + new DefaultDnsResponse(0).addRecord(ANSWER, newAddressRecord("b.com.", "127.0.0.1")))) + ) { + final InetSocketAddress proxySocketAddress = new InetSocketAddress( + "a.com", httpProxyServer.address().getPort()); + try (ClientFactory clientFactory = + ClientFactory.builder() + .addressResolverGroupFactory( + DNSResolverFacadeUtils.getAddressResolverGroupForTest(server)) + .proxyConfig(ProxyConfig.connect(proxySocketAddress)) + .useHttp2Preface(true) + .build()) { + + final WebClient webClient = WebClient.builder(SessionProtocol.H1C, backendServer.httpEndpoint()) + .factory(clientFactory) + .decorator(LoggingClient.newDecorator()) + .build(); + final CompletableFuture responseFuture = + webClient.get(PROXY_PATH).aggregate(); + assertThatThrownBy(responseFuture::join).isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(UnprocessedRequestException.class) + .hasRootCauseInstanceOf(DnsErrorCauseException.class) + .hasRootCauseMessage("Query failed with NXDOMAIN"); + } + } + } + private static final class SleepHandler extends ChannelInboundHandlerAdapter { @Override diff --git a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java index 1c0b8c68bc7..295cf96c6b3 100644 --- a/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/proxy/ProxyConfigTest.java @@ -48,13 +48,19 @@ void testEqualityAndHashCode(ProxyConfig config) { void testUnresolvedProxyAddress() { final InetSocketAddress unresolved = InetSocketAddress.createUnresolved("unresolved", 0); final InetSocketAddress resolved = new InetSocketAddress("127.0.0.1", 80); - assertThatThrownBy(() -> ProxyConfig.socks4(unresolved)).isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> ProxyConfig.socks5(unresolved)).isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> ProxyConfig.connect(unresolved)).isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> ProxyConfig.haproxy(unresolved, resolved)) - .isInstanceOf(IllegalArgumentException.class); - assertThatThrownBy(() -> ProxyConfig.haproxy(resolved, unresolved)) - .isInstanceOf(IllegalArgumentException.class); + + assertThat(ProxyConfig.socks4(unresolved)).isInstanceOf(Socks4ProxyConfig.class); + assertThat(ProxyConfig.socks5(unresolved)).isInstanceOf(Socks5ProxyConfig.class); + assertThat(ProxyConfig.connect(unresolved)).isInstanceOf(ConnectProxyConfig.class); + + // for the HAProxy. + assertThat(ProxyConfig.haproxy(unresolved, resolved)).isInstanceOf(HAProxyConfig.class); + assertThat(ProxyConfig.haproxy(unresolved)).isInstanceOf(HAProxyConfig.class); + assertThat(ProxyConfig.haproxy(resolved)).isInstanceOf(HAProxyConfig.class); + assertThatThrownBy( + () -> ProxyConfig.haproxy(resolved, unresolved)).isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy( + () -> ProxyConfig.haproxy(unresolved, unresolved)).isInstanceOf(IllegalArgumentException.class); } @Test diff --git a/core/src/test/java/com/linecorp/armeria/client/retry/BackoffTest.java b/core/src/test/java/com/linecorp/armeria/client/retry/BackoffTest.java index 93b6ad1cf59..7c15a5d92fc 100644 --- a/core/src/test/java/com/linecorp/armeria/client/retry/BackoffTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/retry/BackoffTest.java @@ -74,6 +74,18 @@ void withJitter() throws Exception { assertThat(backoff.nextDelayMillis(3)).isEqualTo(803); } + @Test + void withJitterBuilder() throws Exception { + final Random random = new Random(1); + final Backoff backoff = Backoff.builderForFixed() + .delayMillis(1000) + .jitter(-0.3, 0.3, () -> random) + .build(); + assertThat(backoff.nextDelayMillis(1)).isEqualTo(1240); + assertThat(backoff.nextDelayMillis(2)).isEqualTo(771); + assertThat(backoff.nextDelayMillis(3)).isEqualTo(803); + } + @Test void withMaxAttempts() throws Exception { final Backoff backoff = Backoff.fixed(100).withMaxAttempts(2); @@ -82,6 +94,17 @@ void withMaxAttempts() throws Exception { assertThat(backoff.nextDelayMillis(3)).isEqualTo(-1); } + @Test + void withMaxAttemptsBuilder() throws Exception { + final Backoff backoff = Backoff.builderForFixed() + .delayMillis(100) + .maxAttempts(2) + .build(); + assertThat(backoff.nextDelayMillis(1)).isEqualTo(100); + assertThat(backoff.nextDelayMillis(2)).isEqualTo(-1); + assertThat(backoff.nextDelayMillis(3)).isEqualTo(-1); + } + @Test void unwrap() { final Backoff backoff = Backoff.fixed(100); diff --git a/core/src/test/java/com/linecorp/armeria/client/retry/ExponentialBackoffTest.java b/core/src/test/java/com/linecorp/armeria/client/retry/ExponentialBackoffTest.java index 4eed997b99d..c9bb116aa10 100644 --- a/core/src/test/java/com/linecorp/armeria/client/retry/ExponentialBackoffTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/retry/ExponentialBackoffTest.java @@ -31,6 +31,19 @@ void normal() { assertThat(backoff.nextDelayMillis(5)).isEqualTo(120); assertThat(backoff.nextDelayMillis(6)).isEqualTo(120); assertThat(backoff.nextDelayMillis(7)).isEqualTo(120); + + final Backoff backoff2 = Backoff.builderForExponential() + .initialDelayMillis(10) + .maxDelayMillis(120) + .multiplier(3.0) + .build(); + assertThat(backoff2.nextDelayMillis(1)).isEqualTo(10); + assertThat(backoff2.nextDelayMillis(2)).isEqualTo(30); + assertThat(backoff2.nextDelayMillis(3)).isEqualTo(90); + assertThat(backoff2.nextDelayMillis(4)).isEqualTo(120); + assertThat(backoff2.nextDelayMillis(5)).isEqualTo(120); + assertThat(backoff2.nextDelayMillis(6)).isEqualTo(120); + assertThat(backoff2.nextDelayMillis(7)).isEqualTo(120); } @Test @@ -42,6 +55,17 @@ void nonPrecomputed() { assertThat(backoff.nextDelayMillis(30)).isEqualTo(5368709120L); // Not precomputed, should fallback to computation and return a correct value. assertThat(backoff.nextDelayMillis(31)).isEqualTo(10737418240L); + + final Backoff backoff2 = Backoff.builderForExponential() + .initialDelayMillis(10) + .maxDelayMillis(Long.MAX_VALUE) + .multiplier(2.0) + .build(); + assertThat(backoff2.nextDelayMillis(1)).isEqualTo(10); + assertThat(backoff2.nextDelayMillis(2)).isEqualTo(20); + assertThat(backoff2.nextDelayMillis(3)).isEqualTo(40); + assertThat(backoff2.nextDelayMillis(30)).isEqualTo(5368709120L); + assertThat(backoff2.nextDelayMillis(31)).isEqualTo(10737418240L); } @Test @@ -51,5 +75,15 @@ void overflow() { assertThat(backoff.nextDelayMillis(2)).isEqualTo((long) (Long.MAX_VALUE / 3 * 2.0)); assertThat(backoff.nextDelayMillis(3)).isEqualTo(Long.MAX_VALUE); assertThat(backoff.nextDelayMillis(4)).isEqualTo(Long.MAX_VALUE); + + final Backoff backoff2 = Backoff.builderForExponential() + .initialDelayMillis(Long.MAX_VALUE / 3) + .maxDelayMillis(Long.MAX_VALUE) + .multiplier(2.0) + .build(); + assertThat(backoff2.nextDelayMillis(1)).isEqualTo(Long.MAX_VALUE / 3); + assertThat(backoff2.nextDelayMillis(2)).isEqualTo((long) (Long.MAX_VALUE / 3 * 2.0)); + assertThat(backoff2.nextDelayMillis(3)).isEqualTo(Long.MAX_VALUE); + assertThat(backoff2.nextDelayMillis(4)).isEqualTo(Long.MAX_VALUE); } } diff --git a/core/src/test/java/com/linecorp/armeria/client/retry/FibonacciBackoffTest.java b/core/src/test/java/com/linecorp/armeria/client/retry/FibonacciBackoffTest.java index 8a91981d990..1e9ae000061 100644 --- a/core/src/test/java/com/linecorp/armeria/client/retry/FibonacciBackoffTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/retry/FibonacciBackoffTest.java @@ -30,6 +30,16 @@ void testNextDelay() { assertThat(backoff.nextDelayMillis(3)).isEqualTo(20); assertThat(backoff.nextDelayMillis(4)).isEqualTo(30); assertThat(backoff.nextDelayMillis(7)).isEqualTo(120); + + final Backoff backoff2 = Backoff.builderForFibonacci() + .initialDelayMillis(10) + .maxDelayMillis(120) + .build(); + assertThat(backoff2.nextDelayMillis(1)).isEqualTo(10); + assertThat(backoff2.nextDelayMillis(2)).isEqualTo(10); + assertThat(backoff2.nextDelayMillis(3)).isEqualTo(20); + assertThat(backoff2.nextDelayMillis(4)).isEqualTo(30); + assertThat(backoff2.nextDelayMillis(7)).isEqualTo(120); } @Test @@ -38,6 +48,14 @@ void testLargeNumberOfRetries() { assertThat(backoff.nextDelayMillis(30)).isEqualTo(832040); assertThat(backoff.nextDelayMillis(31)).isEqualTo(1346269); assertThat(backoff.nextDelayMillis(32)).isEqualTo(2178309); + + final Backoff backoff2 = Backoff.builderForFibonacci() + .initialDelayMillis(1) + .maxDelayMillis(Long.MAX_VALUE) + .build(); + assertThat(backoff2.nextDelayMillis(30)).isEqualTo(832040); + assertThat(backoff2.nextDelayMillis(31)).isEqualTo(1346269); + assertThat(backoff2.nextDelayMillis(32)).isEqualTo(2178309); } @Test @@ -48,17 +66,37 @@ void testOverflow() { assertThat(backoff.nextDelayMillis(3)).isEqualTo(Long.MAX_VALUE / 3 * 2); assertThat(backoff.nextDelayMillis(4)).isEqualTo(Long.MAX_VALUE / 3 * 3); assertThat(backoff.nextDelayMillis(5)).isEqualTo(Long.MAX_VALUE); + + final Backoff backoff2 = Backoff.builderForFibonacci() + .initialDelayMillis(Long.MAX_VALUE / 3) + .maxDelayMillis(Long.MAX_VALUE) + .build(); + assertThat(backoff.nextDelayMillis(1)).isEqualTo(Long.MAX_VALUE / 3); + assertThat(backoff.nextDelayMillis(2)).isEqualTo(Long.MAX_VALUE / 3); + assertThat(backoff.nextDelayMillis(3)).isEqualTo(Long.MAX_VALUE / 3 * 2); + assertThat(backoff.nextDelayMillis(4)).isEqualTo(Long.MAX_VALUE / 3 * 3); + assertThat(backoff.nextDelayMillis(5)).isEqualTo(Long.MAX_VALUE); } @Test void testConstraintInitialDelay() { assertThatThrownBy(() -> new FibonacciBackoff(-5, 120)) .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> Backoff.builderForFibonacci() + .initialDelayMillis(-5) + .maxDelayMillis(120) + .build()) + .isInstanceOf(IllegalArgumentException.class); } @Test void testConstraintMaxDelay() { assertThatThrownBy(() -> new FibonacciBackoff(10, 0)) .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> Backoff.builderForFibonacci() + .initialDelayMillis(10) + .maxDelayMillis(0) + .build()) + .isInstanceOf(IllegalArgumentException.class); } } diff --git a/core/src/test/java/com/linecorp/armeria/client/retry/RandomBackoffTest.java b/core/src/test/java/com/linecorp/armeria/client/retry/RandomBackoffTest.java index 13a72cba6bb..cbe212b1b6c 100644 --- a/core/src/test/java/com/linecorp/armeria/client/retry/RandomBackoffTest.java +++ b/core/src/test/java/com/linecorp/armeria/client/retry/RandomBackoffTest.java @@ -33,6 +33,20 @@ public void nextDelayMillis() throws Exception { assertThat(backoff.nextDelayMillis(1)).isEqualTo(95); } + @Test + public void nextDelayMillisWithBuilder() throws Exception { + final Random r = new Random(1); + final Backoff backoff2 = Backoff.builderForRandom() + .minDelayMillis(10) + .maxDelayMillis(100) + .randomSupplier(() -> r) + .build(); + assertThat(backoff2.nextDelayMillis(1)).isEqualTo(18); + assertThat(backoff2.nextDelayMillis(1)).isEqualTo(93); + assertThat(backoff2.nextDelayMillis(1)).isEqualTo(12); + assertThat(backoff2.nextDelayMillis(1)).isEqualTo(95); + } + @Test public void validation() { // Negative minDelayMillis diff --git a/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java b/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java new file mode 100644 index 00000000000..8f670ab67e1 --- /dev/null +++ b/core/src/test/java/com/linecorp/armeria/internal/common/util/IpAddrUtilTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.internal.common.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.UnknownHostException; + +import org.junit.jupiter.api.Test; + +class IpAddrUtilTest { + + @Test + void testIsCreatedWithIpAddressOnly() throws UnknownHostException { + InetSocketAddress inetSocketAddress = new InetSocketAddress("foo.com", 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); + + inetSocketAddress = InetSocketAddress.createUnresolved("foo.com", 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); + + inetSocketAddress = new InetSocketAddress("1.2.3.4", 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + + InetAddress inetAddress = InetAddress.getByName("1.2.3.4"); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + + inetAddress = InetAddress.getByName("0.0.0.0"); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + + inetAddress = InetAddress.getByName("::"); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isTrue(); + + inetAddress = InetAddress.getByAddress("foo.com", new byte[] { 1, 2, 3, 4 }); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); + + inetAddress = InetAddress.getByName("foo.com"); + inetSocketAddress = new InetSocketAddress(inetAddress, 8080); + assertThat(IpAddrUtil.isCreatedWithIpAddressOnly(inetSocketAddress)).isFalse(); + } +} diff --git a/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java b/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java index bb5f68a8403..f32ff8afd5f 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceTest.java @@ -19,6 +19,7 @@ import static org.apache.hc.core5.http.HttpHeaders.CONTENT_TYPE; import static org.apache.hc.core5.http.HttpHeaders.IF_MATCH; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; import java.nio.charset.Charset; @@ -26,6 +27,7 @@ import java.util.ArrayList; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.TreeSet; @@ -75,6 +77,7 @@ import com.linecorp.armeria.internal.testing.AnticipatedException; import com.linecorp.armeria.internal.testing.GenerateNativeImageTrace; import com.linecorp.armeria.server.HttpStatusException; +import com.linecorp.armeria.server.Server; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.TestConverters.NaiveIntConverterFunction; @@ -628,6 +631,14 @@ public String paramPrecedence(RequestContext ctx, validateContext(ctx); return username + '/' + password; } + + @Get("/param/map") + public String map(RequestContext ctx, @Param Map map) { + validateContext(ctx); + return map.isEmpty() ? "empty" : map.entrySet().stream() + .map(entry -> entry.getKey() + '=' + entry.getValue()) + .collect(Collectors.joining(", ")); + } } @ResponseConverter(UnformattedStringConverterFunction.class) @@ -925,6 +936,13 @@ public ResponseEntity responseEntityHttpResponse(RequestContext ct } } + public static class MyAnnotationService16 { + @Get("/param/map-invalid") + public String invalidMapParam(@Param("param") Map param) { + return "Should not reach here"; + } + } + @Test void testAnnotatedService() throws Exception { try (CloseableHttpClient hc = HttpClients.createMinimal()) { @@ -1057,6 +1075,11 @@ void testParam() throws Exception { testStatusCode(hc, get("/7/param/default2"), 400); testBody(hc, get("/7/param/default_null"), "(null)"); + + // Case all query parameters test map + testBody(hc, get("/7/param/map?key1=value1&key2=value2"), + "key1=value1, key2=value2"); + testBody(hc, get("/7/param/map"), "empty"); } } @@ -1353,6 +1376,18 @@ void testResponseEntity() throws Exception { } } + @Test + void testInvalidParamAnnotationUsageOnMap() { + assertThatThrownBy(() -> + Server.builder() + .annotatedService() + .build(new MyAnnotationService16()) + .build() + ) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid @Param annotation on Map parameter"); + } + private enum UserLevel { LV1, LV2 diff --git a/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java b/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java index 7fd3fe0b472..d3acd4c0422 100644 --- a/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java +++ b/core/src/test/java/com/linecorp/armeria/internal/server/annotation/AnnotatedValueResolverTest.java @@ -112,6 +112,7 @@ class AnnotatedValueResolverTest { static final ServiceRequestContext context; static final HttpRequest request; static final RequestHeaders originalHeaders; + static final String QUERY_PARAM_MAP = "queryParamMap"; static Map> successExpectAttrKeys; static Map> failExpectAttrKeys; @@ -363,7 +364,11 @@ private static void testResolver(AnnotatedValueResolver resolver) { } } } else { - assertThat(resolver.defaultValue()).isNotNull(); + if (QUERY_PARAM_MAP.equals(resolver.httpElementName())) { + assertThat(resolver.defaultValue()).isNull(); + } else { + assertThat(resolver.defaultValue()).isNotNull(); + } if (resolver.hasContainer() && List.class.isAssignableFrom(resolver.containerType())) { assertThat((List) value).hasSize(1) .containsOnly(resolver.defaultValue()); @@ -371,6 +376,12 @@ private static void testResolver(AnnotatedValueResolver resolver) { .isEqualTo(resolver.elementType()); } else if (resolver.shouldWrapValueAsOptional()) { assertThat(value).isEqualTo(Optional.of(resolver.defaultValue())); + } else if (QUERY_PARAM_MAP.equals(resolver.httpElementName())) { + assertThat(value).isNotNull(); + assertThat(value).isInstanceOf(Map.class); + assertThat((Map) value).size() + .isEqualTo(existingHttpParameters.size() + + existingWithoutValueParameters.size()); } else { assertThat(value).isEqualTo(resolver.defaultValue()); } @@ -447,6 +458,7 @@ void method1(@Param String var1, @Param @Default Integer emptyParam2, @Param @Default List emptyParam3, @Param @Default List emptyParam4, + @Param Map queryParamMap, @Header List header1, @Header("header1") Optional> optionalHeader1, @Header String header2, diff --git a/gradle/scripts/README.md b/gradle/scripts/README.md index f8a4e54a6c4..ad31c81e9ed 100644 --- a/gradle/scripts/README.md +++ b/gradle/scripts/README.md @@ -695,8 +695,16 @@ and artifact ID. For example: If enabled, each project with `java` flag will have the `automaticModuleName` property. -You can override the automatic module name of a certain project via the `automaticModuleNameOverrides` -extension property: +You can override the automatic module name of a certain project via `automaticModuleNameOverride`: + + ```groovy + ext { + // Change the automatic module name of a project to 'com.example.fubar'. + automaticModuleNameOverride = 'com.example.fubar' + } + ``` + +Alternatively, you can also specify a mapping via the `automaticModuleNameOverrides` extension property: ```groovy ext { diff --git a/gradle/scripts/lib/common-info.gradle b/gradle/scripts/lib/common-info.gradle index 0f29c25407e..fd4b671c224 100644 --- a/gradle/scripts/lib/common-info.gradle +++ b/gradle/scripts/lib/common-info.gradle @@ -73,20 +73,35 @@ allprojects { return null } - // Use the overridden one if available. - def overriddenAutomaticModuleName = findOverridden('automaticModuleNameOverrides', project) - if (overriddenAutomaticModuleName != null) { - return overriddenAutomaticModuleName - } + return project.provider { + // per-project override + if (project.ext.has('automaticModuleNameOverride')) { + def override = project.ext.get('automaticModuleNameOverride') + if (!(override instanceof String)) { + throw new IllegalStateException("project.ext.automaticModuleNameOverride must be a String: ${override}") + } + return override + } + + // Use the overridden one if available. - // Generate from the groupId and artifactId otherwise. - def groupIdComponents = String.valueOf(rootProject.group).split("\\.").toList() - def artifactIdComponents = - String.valueOf(project.ext.artifactId).replace('-', '.').split("\\.").toList() - if (groupIdComponents.last() == artifactIdComponents.first()) { - return String.join('.', groupIdComponents + artifactIdComponents.drop(1)) - } else { - return String.join('.', groupIdComponents + artifactIdComponents) + def overriddenAutomaticModuleName = findOverridden('automaticModuleNameOverrides', project) + if (overriddenAutomaticModuleName != null) { + return overriddenAutomaticModuleName + } + + // Generate from the groupId and artifactId otherwise. + def groupIdComponents = String.valueOf(rootProject.group).split("\\.").toList() + def artifactIdComponents = + String.valueOf(project.ext.artifactId).replace('-', '.').split("\\.").toList() + def generatedName + if (groupIdComponents.last() == artifactIdComponents.first()) { + generatedName = String.join('.', groupIdComponents + artifactIdComponents.drop(1)) + } else { + generatedName = String.join('.', groupIdComponents + artifactIdComponents) + } + generatedName = generatedName.replaceAll("\\.(\\d)", "_\$1") + return generatedName } }.call() } diff --git a/gradle/scripts/lib/java-shade.gradle b/gradle/scripts/lib/java-shade.gradle index fd1ddd3ec55..04f23ba9154 100644 --- a/gradle/scripts/lib/java-shade.gradle +++ b/gradle/scripts/lib/java-shade.gradle @@ -48,8 +48,10 @@ configure(relocatedProjects) { // Set the 'Automatic-Module-Name' property in MANIFEST.MF. if (project.ext.automaticModuleName != null) { - manifest { - attributes('Automatic-Module-Name': project.ext.automaticModuleName) + doFirst { + manifest { + attributes('Automatic-Module-Name': project.ext.automaticModuleName.get()) + } } } } diff --git a/gradle/scripts/lib/java.gradle b/gradle/scripts/lib/java.gradle index a70498ac852..b20a6174f67 100644 --- a/gradle/scripts/lib/java.gradle +++ b/gradle/scripts/lib/java.gradle @@ -166,8 +166,10 @@ configure(projectsWithFlags('java')) { // Set the 'Automatic-Module-Name' property in 'MANIFEST.MF' if `automaticModuleName` is not null. if (project.ext.automaticModuleName != null) { tasks.named('jar') { - manifest { - attributes('Automatic-Module-Name': project.ext.automaticModuleName) + doFirst { + manifest { + attributes('Automatic-Module-Name': project.ext.automaticModuleName.get()) + } } } } diff --git a/it/internal-logging/src/test/java/com/linecorp/armeria/client/HttpResponseWrapperLogTest.java b/it/internal-logging/src/test/java/com/linecorp/armeria/client/HttpResponseWrapperLogTest.java new file mode 100644 index 00000000000..39ac9085805 --- /dev/null +++ b/it/internal-logging/src/test/java/com/linecorp/armeria/client/HttpResponseWrapperLogTest.java @@ -0,0 +1,119 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.armeria.client; + +import static com.linecorp.armeria.internal.testing.Http2ByteUtil.handleInitialExchange; +import static com.linecorp.armeria.internal.testing.Http2ByteUtil.newClientFactory; +import static com.linecorp.armeria.internal.testing.Http2ByteUtil.readFrame; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.awaitility.Awaitility.await; + +import java.io.BufferedOutputStream; +import java.io.InputStream; +import java.net.ServerSocket; +import java.net.Socket; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.slf4j.LoggerFactory; + +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.ClosedSessionException; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpRequest; +import com.linecorp.armeria.testing.junit5.common.EventLoopExtension; + +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import io.netty.handler.codec.http2.Http2FrameTypes; + +class HttpResponseWrapperLogTest { + + @RegisterExtension + static final EventLoopExtension eventLoop = new EventLoopExtension(); + + private static final LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); + private static final Logger logger = + (Logger) LoggerFactory.getLogger(HttpResponseWrapper.class); + private static final ListAppender appender = new ListAppender<>(); + + @BeforeEach + void beforeEach() { + appender.setContext(context); + appender.start(); + logger.addAppender(appender); + } + + @AfterEach + void afterEach() { + appender.stop(); + logger.detachAppender(appender); + } + + @Test + void goAwayNotLogged() throws Exception { + try (ServerSocket ss = new ServerSocket(0); + ClientFactory clientFactory = newClientFactory(eventLoop.get())) { + + final int port = ss.getLocalPort(); + + final WebClient client = WebClient.builder("h2c://127.0.0.1:" + port) + .factory(clientFactory) + .build(); + final HttpRequest req = HttpRequest.streaming(HttpMethod.GET, "/"); + final CompletableFuture resFuture = client.execute(req).aggregate(); + try (Socket s = ss.accept()) { + + final InputStream in = s.getInputStream(); + final BufferedOutputStream bos = new BufferedOutputStream(s.getOutputStream()); + handleInitialExchange(in, bos); + + // Read a HEADERS frame. + assertThat(readFrame(in).getByte(3)).isEqualTo(Http2FrameTypes.HEADERS); + + // Send a GOAWAY frame. + bos.write(new byte[] { + 0x00, 0x00, 0x08, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x03, // lastStreamId = 3 + 0x00, 0x00, 0x00, 0x00 // errorCode = 0 + }); + bos.flush(); + + // The second request should fail with UnprocessedRequestException + // which has a cause of GoAwayReceivedException. + await().untilAsserted(resFuture::isCompletedExceptionally); + assertThatThrownBy(resFuture::join).isInstanceOf(CompletionException.class) + .hasCauseInstanceOf(ClosedSessionException.class); + + // Read a GOAWAY frame. + assertThat(readFrame(in).getByte(3)).isEqualTo(Http2FrameTypes.GO_AWAY); + + assertThat(in.read()).isEqualTo(-1); + } + } + assertThat(appender.list).allSatisfy(event -> { + assertThat(event.getMessage()) + .doesNotContain(HttpResponseWrapper.UNEXPECTED_EXCEPTION_MSG); + }); + } +} diff --git a/settings.gradle b/settings.gradle index 64a00b3b1af..537258a8287 100644 --- a/settings.gradle +++ b/settings.gradle @@ -207,6 +207,7 @@ includeWithFlags ':it:builders', 'java' includeWithFlags ':it:context-storage', 'java' includeWithFlags ':it:dgs', 'java17' includeWithFlags ':it:flags-cyclic-dep', 'java' +includeWithFlags ':it:flags-provider', 'java', 'relocate' includeWithFlags ':it:graphql-multipart', 'java17' includeWithFlags ':it:grpcweb', 'java', 'akka-grpc_2.13' includeWithFlags ':it:grpc:java', 'java' @@ -214,7 +215,7 @@ includeWithFlags ':it:grpc:kotlin', 'java', 'relocate includeWithFlags ':it:grpc:kotlin-coroutine-context-provider', 'java', 'relocate', 'kotlin-grpc', 'kotlin' includeWithFlags ':it:grpc:scala', 'java', 'relocate', 'scala-grpc_2.13', 'scala_2.13' includeWithFlags ':it:grpc:reactor', 'java', 'relocate', 'reactor-grpc' -includeWithFlags ':it:flags-provider', 'java', 'relocate' +includeWithFlags ':it:internal-logging', 'java', 'relocate' includeWithFlags ':it:jackson-provider', 'java', 'relocate' includeWithFlags ':it:kotlin', 'java', 'relocate', 'kotlin' includeWithFlags ':it:kubernetes-chaos-tests', 'java', 'relocate' diff --git a/testing-internal/src/main/java/com/linecorp/armeria/internal/testing/Http2ByteUtil.java b/testing-internal/src/main/java/com/linecorp/armeria/internal/testing/Http2ByteUtil.java new file mode 100644 index 00000000000..22a3658ff23 --- /dev/null +++ b/testing-internal/src/main/java/com/linecorp/armeria/internal/testing/Http2ByteUtil.java @@ -0,0 +1,90 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package com.linecorp.armeria.internal.testing; + +import static io.netty.handler.codec.http2.Http2CodecUtil.connectionPrefaceBuf; +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.BufferedOutputStream; +import java.io.IOException; +import java.io.InputStream; + +import com.google.common.io.ByteStreams; + +import com.linecorp.armeria.client.ClientFactory; + +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; +import io.netty.channel.EventLoop; +import io.netty.handler.codec.http2.Http2CodecUtil; +import io.netty.handler.codec.http2.Http2FrameTypes; + +public final class Http2ByteUtil { + + public static ClientFactory newClientFactory(EventLoop eventLoop) { + return ClientFactory.builder() + .useHttp2Preface(true) + // Set the window size to the HTTP/2 default values to simplify the traffic. + .http2InitialConnectionWindowSize(Http2CodecUtil.DEFAULT_WINDOW_SIZE) + .http2InitialStreamWindowSize(Http2CodecUtil.DEFAULT_WINDOW_SIZE) + .workerGroup(eventLoop, false) + .build(); + } + + public static void handleInitialExchange(InputStream in, BufferedOutputStream out) throws IOException { + // Read the connection preface and discard it. + readBytes(in, connectionPrefaceBuf().readableBytes()); + + // Read a SETTINGS frame. + assertThat(readFrame(in).getByte(3)).isEqualTo(Http2FrameTypes.SETTINGS); + + // Send a SETTINGS frame and the ack for the received SETTINGS frame. + sendEmptySettingsAndAckFrame(out); + + // Read a SETTINGS ack frame. + assertThat(readFrame(in).getByte(3)).isEqualTo(Http2FrameTypes.SETTINGS); + } + + public static byte[] readBytes(InputStream in, int length) throws IOException { + final byte[] buf = new byte[length]; + ByteStreams.readFully(in, buf); + return buf; + } + + public static void sendEmptySettingsAndAckFrame(BufferedOutputStream bos) throws IOException { + // Send an empty SETTINGS frame. + bos.write(new byte[] { 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00 }); + // Send a SETTINGS_ACK frame. + bos.write(new byte[] { 0x00, 0x00, 0x00, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00 }); + bos.flush(); + } + + public static int payloadLength(byte[] buf) { + return (buf[0] & 0xff) << 16 | (buf[1] & 0xff) << 8 | (buf[2] & 0xff); + } + + public static ByteBuf readFrame(InputStream in) throws IOException { + final byte[] frameBuf = readBytes(in, 9); + final int payloadLength = payloadLength(frameBuf); + final ByteBuf buffer = Unpooled.buffer(9 + payloadLength); + buffer.writeBytes(frameBuf); + buffer.writeBytes(in, payloadLength); + return buffer; + } + + private Http2ByteUtil() {} +} diff --git a/thrift/thrift0.13/build.gradle b/thrift/thrift0.13/build.gradle index 7bfd1e1ffd6..79c398497c5 100644 --- a/thrift/thrift0.13/build.gradle +++ b/thrift/thrift0.13/build.gradle @@ -27,7 +27,7 @@ dependencies { // Use the old compiler. def thriftFullVersion = libs.thrift013.get().versionConstraint.requiredVersion ext { - thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.')); + thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.')) } // Keep the original Guava references in ThriftListenableFuture,