Merge branch 'main' into pr-5580-festinalente91-feature/enrich-connec…

…tion-pool-listener-event-interface

.github/workflows/actions_build.yml

@@ -2,7 +2,7 @@ name: CI
 on:
   push:
     branches:
-      - main
+      - "**"
     tags-ignore:
       # The release versions will be verified by 'publish-release.yml'
       - armeria-*
@@ -140,7 +140,9 @@ jobs:
           -PbuildJdkVersion=${{ env.BUILD_JDK_VERSION }} \
           -PtestJavaVersion=${{ matrix.java }} \
           ${{ matrix.min-java && format('-PminimumJavaVersion={0}', matrix.min-java) || '' }} \
-          -Porg.gradle.java.installations.paths=${{ steps.setup-build-jdk.outputs.path }},${{ steps.setup-jdk.outputs.path }}
+          -Porg.gradle.java.installations.paths=${{ steps.setup-build-jdk.outputs.path }},${{ steps.setup-jdk.outputs.path }} \
+          -PpreferShadedTests=${{ github.ref_name != 'main' }}
+          # Unshaded tests are skipped for PRs to avoid running the same tests twice.
         shell: bash
         env:
           COMMIT_SHA: ${{ github.event.pull_request.head.sha }}

annotation-processor/src/main/java/com/linecorp/armeria/server/annotation/processor/DocumentationProcessor.java

@@ -38,6 +38,7 @@
 import javax.lang.model.SourceVersion;
 import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.ExecutableElement;
+import javax.lang.model.element.QualifiedNameable;
 import javax.lang.model.element.TypeElement;
 import javax.lang.model.element.VariableElement;
 import javax.tools.Diagnostic.Kind;
@@ -148,7 +149,9 @@ private void processAnnotation(TypeElement annotationElement, RoundEnvironment r
     }
 
     private void processMethod(ExecutableElement method) throws IOException {
-        final String className = ((TypeElement) method.getEnclosingElement()).getQualifiedName().toString();
+        final QualifiedNameable enclosingElement = (QualifiedNameable) method.getEnclosingElement();
+        assert enclosingElement != null;
+        final String className = enclosingElement.getQualifiedName().toString();
         final Properties properties = readProperties(className);
         final String docComment = processingEnv.getElementUtils().getDocComment(method);
         if (docComment == null || !docComment.contains("@param")) {

benchmarks/jmh/src/jmh/java/com/linecorp/armeria/server/RoutersBenchmark.java

@@ -78,7 +78,7 @@ private static ServiceConfig newServiceConfig(Route route) {
         final Path multipartUploadsLocation = Flags.defaultMultipartUploadsLocation();
         final ServiceErrorHandler serviceErrorHandler = ServerErrorHandler.ofDefault().asServiceErrorHandler();
         return new ServiceConfig(route, route,
-                                 SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
+                                 SERVICE, defaultServiceName, defaultServiceNaming, defaultLogName, 0, 0,
                                  false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
                                  SuccessFunction.always(), 0, multipartUploadsLocation,
                                  MultipartRemovalStrategy.ON_RESPONSE_COMPLETION,

brave/brave6/src/main/java/com/linecorp/armeria/common/brave/RequestContextCurrentTraceContext.java

@@ -174,7 +174,7 @@ public Scope newScope(@Nullable TraceContext currentSpan) {
 
     @UnstableApi
     @Override
-    public Scope decorateScope(TraceContext context, Scope scope) {
+    public Scope decorateScope(@Nullable TraceContext context, Scope scope) {
         // If a `Scope` is decorated, `ScopeDecorator`s populate some contexts as such as MDC, which are stored
         // to a thread-local. The activated contexts will be removed when `decoratedScope.close()` is called.
         // If `Scope.NOOP` is specified, CurrentTraceContext.decorateScope() performs nothing.

brave/brave6/src/main/java/com/linecorp/armeria/internal/common/brave/TraceContextUtil.java

@@ -39,7 +39,7 @@ public static TraceContext traceContext(RequestContext ctx) {
         return ctx.attr(TRACE_CONTEXT_KEY);
     }
 
-    public static void setTraceContext(RequestContext ctx, TraceContext traceContext) {
+    public static void setTraceContext(RequestContext ctx, @Nullable TraceContext traceContext) {
         ctx.setAttr(TRACE_CONTEXT_KEY, traceContext);
     }
 

build.gradle

@@ -23,6 +23,7 @@ plugins {
     alias libs.plugins.kotlin apply false
     alias libs.plugins.ktlint apply false
     alias libs.plugins.errorprone apply false
+    alias libs.plugins.nullaway apply false
 }
 
 allprojects {
@@ -171,13 +172,31 @@ configure(projectsWithFlags('java')) {
     // Error Prone compiler
     if (!rootProject.hasProperty('noLint')) {
         apply plugin: 'net.ltgt.errorprone'
+        apply plugin: 'net.ltgt.nullaway'
 
         dependencies {
             errorprone libs.errorprone.core
+            errorprone libs.nullaway
+        }
+
+        nullaway {
+            annotatedPackages.add("com.linecorp.armeria")
         }
 
         tasks.withType(JavaCompile) {
             options.errorprone.excludedPaths = '.*/gen-src/.*'
+            options.errorprone.nullaway {
+                if (name.contains("Test") || name.contains("Jmh")) {
+                    // Disable NullAway for tests and benchmarks for now.
+                    disable()
+                } else if (name.matches(/compileJava[0-9]+.*/)) {
+                    // Disable MR-JAR classes which seem to confuse NullAway and break the build.
+                    disable()
+                } else {
+                    error()
+                    assertsEnabled = true
+                }
+            }
         }
     }
 

consul/src/main/java/com/linecorp/armeria/internal/consul/HealthClient.java

@@ -110,6 +110,16 @@ CompletableFuture<List<Endpoint>> healthyEndpoints(String serviceName, @Nullable
 
     @Nullable
     private static Endpoint toEndpoint(HealthService healthService) {
+        if (healthService.service == null) {
+            return null;
+        }
+        if (healthService.node == null) {
+            return null;
+        }
+
+        assert healthService.service != null;
+        assert healthService.node != null;
+
         if (!Strings.isNullOrEmpty(healthService.service.address)) {
             return Endpoint.of(healthService.service.address, healthService.service.port);
         } else if (!Strings.isNullOrEmpty(healthService.node.address)) {
@@ -122,59 +132,74 @@ private static Endpoint toEndpoint(HealthService healthService) {
     @JsonIgnoreProperties(ignoreUnknown = true)
     @JsonInclude(Include.NON_NULL)
     private static final class HealthService {
+        @Nullable
         @JsonProperty("Node")
         Node node;
 
+        @Nullable
         @JsonProperty("Service")
         Service service;
     }
 
     @JsonIgnoreProperties(ignoreUnknown = true)
     @JsonInclude(Include.NON_NULL)
     private static final class Node {
+        @Nullable
         @JsonProperty("ID")
         String id;
 
+        @Nullable
         @JsonProperty("Node")
         String node;
 
+        @Nullable
         @JsonProperty("Address")
         String address;
 
+        @Nullable
         @JsonProperty("Datacenter")
         String datacenter;
 
+        @Nullable
         @JsonProperty("TaggedAddresses")
         Object taggedAddresses;
 
+        @Nullable
         @JsonProperty("Meta")
         Map<String, Object> meta;
     }
 
     @JsonIgnoreProperties(ignoreUnknown = true)
     @JsonInclude(Include.NON_NULL)
     public static final class Service {
+        @Nullable
         @JsonProperty("ID")
         String id;
 
+        @Nullable
         @JsonProperty("Service")
         String service;
 
+        @Nullable
         @JsonProperty("Tags")
         String[] tags;
 
+        @Nullable
         @JsonProperty("Address")
         String address;
 
+        @Nullable
         @JsonProperty("TaggedAddresses")
         Object taggedAddresses;
 
+        @Nullable
         @JsonProperty("Meta")
         Map<String, Object> meta;
 
         @JsonProperty("Port")
         int port;
 
+        @Nullable
         @JsonProperty("Weights")
         Map<String, Object> weights;
     }

core/build.gradle

@@ -227,6 +227,9 @@ if (tasks.findByName('trimShadedJar')) {
         keep "class com.linecorp.armeria.internal.shaded.bouncycastle.jcajce.provider.asymmetric.ec.** { *; }"
         keep "class com.linecorp.armeria.internal.shaded.bouncycastle.jcajce.provider.asymmetric.rsa.** { *; }"
         keep "class com.linecorp.armeria.internal.shaded.bouncycastle.jcajce.provider.asymmetric.x509.** { *; }"
+        // Keep the Guava classes accessed during testing.
+        keep "class com.linecorp.armeria.internal.shaded.guava.net.HttpHeaders { *; }"
+        keep "class com.linecorp.armeria.internal.shaded.guava.net.MediaType { *; }"
         dontnote
     }
 }

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

@@ -54,7 +54,6 @@
 import io.netty.channel.ChannelPromise;
 import io.netty.handler.codec.http.HttpHeaderValues;
 import io.netty.handler.codec.http2.Http2Error;
-import io.netty.handler.proxy.ProxyConnectException;
 
 abstract class AbstractHttpRequestHandler implements ChannelFutureListener {
 
@@ -216,6 +215,7 @@ RequestHeaders mergedRequestHeaders(RequestHeaders headers) {
      * {@link Channel#flush()} when each write unit is done.
      */
     final void writeHeaders(RequestHeaders headers, boolean needs100Continue) {
+        assert session != null;
         final SessionProtocol protocol = session.protocol();
         assert protocol != null;
         if (needs100Continue) {
@@ -377,8 +377,7 @@ final void failAndReset(Throwable cause) {
             session.markUnacquirable();
         }
 
-        if (cause instanceof ProxyConnectException || cause instanceof ResponseCompleteException) {
-            // - ProxyConnectException is handled by HttpSessionHandler.exceptionCaught().
+        if (cause instanceof ResponseCompleteException) {
             // - ResponseCompleteException means the response is successfully received.
             state = State.DONE;
             cancel();

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

@@ -24,6 +24,7 @@
 import com.linecorp.armeria.common.Response;
 import com.linecorp.armeria.common.RpcRequest;
 import com.linecorp.armeria.common.RpcResponse;
+import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.util.Unwrappable;
 
 /**
@@ -71,6 +72,7 @@ public interface Client<I extends Request, O extends Response> extends Unwrappab
      * @see ClientFactory#unwrap(Object, Class)
      * @see Unwrappable
      */
+    @Nullable
     @Override
     default <T> T as(Class<T> type) {
         requireNonNull(type, "type");

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

@@ -52,26 +52,31 @@ public ClientRequestContext newDerivedContext(RequestId id, @Nullable HttpReques
         return unwrap().newDerivedContext(id, req, rpcReq, endpoint);
     }
 
+    @Nullable
     @Override
     public EndpointGroup endpointGroup() {
         return unwrap().endpointGroup();
     }
 
+    @Nullable
     @Override
     public Endpoint endpoint() {
         return unwrap().endpoint();
     }
 
+    @Nullable
     @Override
     public String fragment() {
         return unwrap().fragment();
     }
 
+    @Nullable
     @Override
     public String authority() {
         return unwrap().authority();
     }
 
+    @Nullable
     @Override
     public String host() {
         return unwrap().host();

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

@@ -108,11 +108,13 @@ public Object newClient(ClientBuilderParams params) {
         return unwrap().newClient(params);
     }
 
+    @Nullable
     @Override
     public <T> ClientBuilderParams clientBuilderParams(T client) {
         return unwrap().clientBuilderParams(client);
     }
 
+    @Nullable
     @Override
     public <T> T unwrap(Object client, Class<T> type) {
         return unwrap().unwrap(client, type);

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

@@ -184,6 +184,7 @@ public Object newClient(ClientBuilderParams params) {
                 "No ClientFactory for scheme: " + scheme + " matched clientType: " + clientType);
     }
 
+    @Nullable
     @Override
     public <T> T unwrap(Object client, Class<T> type) {
         final T params = ClientFactory.super.unwrap(client, type);

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

@@ -147,6 +147,7 @@ public void cache(DnsQuestion question, UnknownHostException cause) {
         }
     }
 
+    @Nullable
     @Override
     public List<DnsRecord> get(DnsQuestion question) throws UnknownHostException {
         requireNonNull(question, "question");

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

@@ -68,6 +68,7 @@ public long maxResponseLength() {
         return maxResponseLength;
     }
 
+    @Nullable
     @Override
     public Long requestAutoAbortDelayMillis() {
         return requestAutoAbortDelayMillis;
@@ -78,6 +79,7 @@ public Map<AttributeKey<?>, Object> attrs() {
         return attributeMap;
     }
 
+    @Nullable
     @Override
     public ExchangeType exchangeType() {
         return exchangeType;

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

@@ -37,6 +37,8 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.function.Consumer;
 
+import javax.annotation.Nonnull;
+
 import com.github.benmanes.caffeine.cache.Cache;
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.google.common.annotations.VisibleForTesting;
@@ -323,6 +325,7 @@ public EndpointSelectionStrategy selectionStrategy() {
         return EndpointSelectionStrategy.weightedRoundRobin();
     }
 
+    @Nonnull
     @Override
     public Endpoint selectNow(ClientRequestContext ctx) {
         return this;
@@ -693,16 +696,40 @@ public <T> Endpoint withAttr(AttributeKey<T> key, @Nullable T value) {
             if (value == null) {
                 return this;
             }
-            return withAttrs(Attributes.of(key, value));
+            return replaceAttrs(Attributes.of(key, value));
         }
 
         if (attributes.attr(key) == value) {
             return this;
         } else {
             final AttributesBuilder attributesBuilder = attributes.toBuilder();
             attributesBuilder.set(key, value);
-            return withAttrs(attributesBuilder.build());
+            return replaceAttrs(attributesBuilder.build());
+        }
+    }
+
+    /**
+     * Returns a new {@link Endpoint} with the specified {@link Attributes}.
+     * Note that the {@link #attrs()} of this {@link Endpoint} is merged with the specified
+     * {@link Attributes}. For attributes with the same {@link AttributeKey}, the attribute
+     * in {@param newAttributes} has higher precedence.
+     */
+    @UnstableApi
+    @SuppressWarnings("unchecked")
+    public Endpoint withAttrs(Attributes newAttributes) {
+        requireNonNull(newAttributes, "newAttributes");
+        if (newAttributes.isEmpty()) {
+            return this;
         }
+        if (attrs().isEmpty()) {
+            return replaceAttrs(newAttributes);
+        }
+        final AttributesBuilder builder = attrs().toBuilder();
+        newAttributes.attrs().forEachRemaining(entry -> {
+            final AttributeKey<Object> key = (AttributeKey<Object>) entry.getKey();
+            builder.set(key, entry.getValue());
+        });
+        return new Endpoint(type, host, ipAddr, port, weight, builder.build());
     }
 
     /**
@@ -711,7 +738,7 @@ public <T> Endpoint withAttr(AttributeKey<T> key, @Nullable T value) {
      * {@link Attributes}.
      */
     @UnstableApi
-    public Endpoint withAttrs(Attributes newAttributes) {
+    public Endpoint replaceAttrs(Attributes newAttributes) {
         requireNonNull(newAttributes, "newAttributes");
         if (attrs().isEmpty() && newAttributes.isEmpty()) {
             return this;