Fix a bug where builders of LogFormatter return internal classes (#…

…4975) Motivation: `TextLogFormatterBuilder.build()` and `JsonLogFormatterBuilder.build()` return package-private classes that cause compilation errors when using them in different packages. Modifications: - Make `TextLogFormatterBuilder.build()` and `JsonLogFormatterBuilder.build()` return `LogFormatter` instead of the internal classes. Result: You can no longer see compilation errors when using `TextLogFormatterBuilder.build()` and `JsonLogFormatterBuilder.build()`.
line · Jun 21, 2023 · e1cabd7 · e1cabd7
1 parent b7ef194
commit e1cabd7
Show file tree

Hide file tree

Showing 6 changed files with 97 additions and 58 deletions.
diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java
@@ -32,6 +32,7 @@
 import com.linecorp.armeria.common.HttpHeaders;
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.SerializationFormat;
+import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.annotation.UnstableApi;
 import com.linecorp.armeria.common.util.TextFormatter;
 
@@ -43,36 +44,39 @@ final class JsonLogFormatter implements LogFormatter {
 
     private static final Logger logger = LoggerFactory.getLogger(JsonLogFormatter.class);
 
-    static final JsonLogFormatter DEFAULT_INSTANCE = new JsonLogFormatterBuilder().build();
+    static final LogFormatter DEFAULT_INSTANCE = new JsonLogFormatterBuilder().build();
 
-    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
+    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
             requestHeadersSanitizer;
 
-    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
+    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
             responseHeadersSanitizer;
 
-    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
+    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
             requestTrailersSanitizer;
 
-    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
+    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
             responseTrailersSanitizer;
 
-    private final BiFunction<? super RequestContext, Object, ? extends JsonNode> requestContentSanitizer;
+    private final BiFunction<? super RequestContext, Object, ? extends @Nullable JsonNode>
+            requestContentSanitizer;
 
-    private final BiFunction<? super RequestContext, Object, ? extends JsonNode> responseContentSanitizer;
+    private final BiFunction<? super RequestContext, Object, ? extends @Nullable JsonNode>
+            responseContentSanitizer;
 
     private final ObjectMapper objectMapper;
 
     JsonLogFormatter(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode> requestHeadersSanitizer,
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
-                    responseHeadersSanitizer,
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
-                    requestTrailersSanitizer,
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
-                    responseTrailersSanitizer,
-            BiFunction<? super RequestContext, Object, ? extends JsonNode> requestContentSanitizer,
-            BiFunction<? super RequestContext, Object, ? extends JsonNode> responseContentSanitizer,
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable JsonNode> requestHeadersSanitizer,
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable JsonNode> responseHeadersSanitizer,
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable JsonNode> requestTrailersSanitizer,
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable JsonNode> responseTrailersSanitizer,
+            BiFunction<? super RequestContext, Object, ? extends @Nullable JsonNode> requestContentSanitizer,
+            BiFunction<? super RequestContext, Object, ? extends @Nullable JsonNode> responseContentSanitizer,
             ObjectMapper objectMapper) {
         this.requestHeadersSanitizer = requestHeadersSanitizer;
         this.responseHeadersSanitizer = responseHeadersSanitizer;

diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java
@@ -51,60 +51,61 @@ public JsonLogFormatterBuilder objectMapper(ObjectMapper objectMapper) {
 
     @Override
     public JsonLogFormatterBuilder requestHeadersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
+            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
                     requestHeadersSanitizer) {
         return (JsonLogFormatterBuilder) super.requestHeadersSanitizer(requestHeadersSanitizer);
     }
 
     @Override
     public JsonLogFormatterBuilder responseHeadersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
+            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
                     responseHeadersSanitizer) {
         return (JsonLogFormatterBuilder) super.responseHeadersSanitizer(responseHeadersSanitizer);
     }
 
     @Override
     public JsonLogFormatterBuilder requestTrailersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
+            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
                     requestTrailersSanitizer) {
         return (JsonLogFormatterBuilder) super.requestTrailersSanitizer(requestTrailersSanitizer);
     }
 
     @Override
     public JsonLogFormatterBuilder responseTrailersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode>
+            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
                     responseTrailersSanitizer) {
         return (JsonLogFormatterBuilder) super.responseTrailersSanitizer(responseTrailersSanitizer);
     }
 
     @Override
     public JsonLogFormatterBuilder headersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends JsonNode> headersSanitizer) {
+            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends @Nullable JsonNode>
+                    headersSanitizer) {
         return (JsonLogFormatterBuilder) super.headersSanitizer(headersSanitizer);
     }
 
     @Override
     public JsonLogFormatterBuilder requestContentSanitizer(
-            BiFunction<? super RequestContext, Object, ? extends JsonNode> requestContentSanitizer) {
+            BiFunction<? super RequestContext, Object, ? extends @Nullable JsonNode> requestContentSanitizer) {
         return (JsonLogFormatterBuilder) super.requestContentSanitizer(requestContentSanitizer);
     }
 
     @Override
     public JsonLogFormatterBuilder responseContentSanitizer(
-            BiFunction<? super RequestContext, Object, ? extends JsonNode> responseContentSanitizer) {
+            BiFunction<? super RequestContext, Object, ? extends @Nullable JsonNode> responseContentSanitizer) {
         return (JsonLogFormatterBuilder) super.responseContentSanitizer(responseContentSanitizer);
     }
 
     @Override
     public JsonLogFormatterBuilder contentSanitizer(
-            BiFunction<? super RequestContext, Object, ? extends JsonNode> contentSanitizer) {
+            BiFunction<? super RequestContext, Object, ? extends @Nullable JsonNode> contentSanitizer) {
         return (JsonLogFormatterBuilder) super.contentSanitizer(contentSanitizer);
     }
 
     /**
-     * Returns a newly-created {@link LogFormatter} based on the properties of this builder.
+     * Returns a newly-created JSON {@link LogFormatter} based on the properties of this builder.
      */
-    public JsonLogFormatter build() {
+    public LogFormatter build() {
         final ObjectMapper objectMapper = this.objectMapper != null ?
                                           this.objectMapper : JacksonUtil.newDefaultObjectMapper();
         final BiFunction<? super RequestContext, HttpHeaders, JsonNode> defaultHeadersSanitizer =

diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilder.java
@@ -546,7 +546,7 @@ protected final LogWriter logWriter() {
         if (!buildLogWriter) {
             return LogWriter.of();
         }
-        final TextLogFormatter logFormatter =
+        final LogFormatter logFormatter =
                 LogFormatter.builderForText()
                             .requestHeadersSanitizer(convertToStringSanitizer(requestHeadersSanitizer))
                             .responseHeadersSanitizer(convertToStringSanitizer(responseHeadersSanitizer))

diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java
@@ -26,6 +26,7 @@
 import com.linecorp.armeria.common.HttpHeaders;
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.SerializationFormat;
+import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.annotation.UnstableApi;
 import com.linecorp.armeria.common.util.TextFormatter;
 import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals;
@@ -36,32 +37,41 @@
 @UnstableApi
 final class TextLogFormatter implements LogFormatter {
 
-    static final TextLogFormatter DEFAULT_INSTANCE = new TextLogFormatterBuilder().build();
+    static final LogFormatter DEFAULT_INSTANCE = new TextLogFormatterBuilder().build();
 
-    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String>
-            requestHeadersSanitizer;
+    private final BiFunction<? super RequestContext, ? super HttpHeaders,
+            ? extends @Nullable String> requestHeadersSanitizer;
 
-    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String>
-            responseHeadersSanitizer;
+    private final BiFunction<? super RequestContext, ? super HttpHeaders,
+            ? extends @Nullable String> responseHeadersSanitizer;
 
-    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String>
-            requestTrailersSanitizer;
+    private final BiFunction<? super RequestContext, ? super HttpHeaders,
+            ? extends @Nullable String> requestTrailersSanitizer;
 
-    private final BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String>
-            responseTrailersSanitizer;
+    private final BiFunction<? super RequestContext, ? super HttpHeaders,
+            ? extends @Nullable String> responseTrailersSanitizer;
 
-    private final BiFunction<? super RequestContext, Object, ? extends String> requestContentSanitizer;
+    private final BiFunction<? super RequestContext, Object,
+            ? extends @Nullable String> requestContentSanitizer;
+
+    private final BiFunction<? super RequestContext, Object,
+            ? extends @Nullable String> responseContentSanitizer;
 
-    private final BiFunction<? super RequestContext, Object, ? extends String> responseContentSanitizer;
     private final boolean includeContext;
 
     TextLogFormatter(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String> requestHeadersSanitizer,
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String> responseHeadersSanitizer,
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String> requestTrailersSanitizer,
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String> responseTrailersSanitizer,
-            BiFunction<? super RequestContext, Object, ? extends String> requestContentSanitizer,
-            BiFunction<? super RequestContext, Object, ? extends String> responseContentSanitizer,
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> requestHeadersSanitizer,
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> responseHeadersSanitizer,
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> requestTrailersSanitizer,
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> responseTrailersSanitizer,
+            BiFunction<? super RequestContext, Object,
+                    ? extends @Nullable String> requestContentSanitizer,
+            BiFunction<? super RequestContext, Object,
+                    ? extends @Nullable String> responseContentSanitizer,
             boolean includeContext) {
         this.requestHeadersSanitizer = requestHeadersSanitizer;
         this.responseHeadersSanitizer = responseHeadersSanitizer;
@@ -282,7 +292,7 @@ public String formatResponse(RequestLog log) {
             buf.append('}');
 
             final List<RequestLogAccess> children = log.children();
-            final int numChildren = children != null ? children.size() : 0;
+            final int numChildren = children.size();
             if (numChildren > 1) {
                 // Append only when there were retries which the numChildren is greater than 1.
                 buf.append(", {totalAttempts=");

diff --git a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java
@@ -22,6 +22,7 @@
 
 import com.linecorp.armeria.common.HttpHeaders;
 import com.linecorp.armeria.common.RequestContext;
+import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.annotation.UnstableApi;
 
 /**
@@ -36,52 +37,54 @@ public final class TextLogFormatterBuilder extends AbstractLogFormatterBuilder<S
 
     @Override
     public TextLogFormatterBuilder requestHeadersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String> requestHeadersSanitizer) {
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> requestHeadersSanitizer) {
         return (TextLogFormatterBuilder) super.requestHeadersSanitizer(requestHeadersSanitizer);
     }
 
     @Override
     public TextLogFormatterBuilder responseHeadersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String>
-                    responseHeadersSanitizer) {
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> responseHeadersSanitizer) {
         return (TextLogFormatterBuilder) super.responseHeadersSanitizer(responseHeadersSanitizer);
     }
 
     @Override
     public TextLogFormatterBuilder requestTrailersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String>
-                    requestTrailersSanitizer) {
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> requestTrailersSanitizer) {
         return (TextLogFormatterBuilder) super.requestTrailersSanitizer(requestTrailersSanitizer);
     }
 
     @Override
     public TextLogFormatterBuilder responseTrailersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String>
-                    responseTrailersSanitizer) {
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> responseTrailersSanitizer) {
         return (TextLogFormatterBuilder) super.responseTrailersSanitizer(responseTrailersSanitizer);
     }
 
     @Override
     public TextLogFormatterBuilder headersSanitizer(
-            BiFunction<? super RequestContext, ? super HttpHeaders, ? extends String> headersSanitizer) {
+            BiFunction<? super RequestContext, ? super HttpHeaders,
+                    ? extends @Nullable String> headersSanitizer) {
         return (TextLogFormatterBuilder) super.headersSanitizer(headersSanitizer);
     }
 
     @Override
     public TextLogFormatterBuilder requestContentSanitizer(
-            BiFunction<? super RequestContext, Object, ? extends String> requestContentSanitizer) {
+            BiFunction<? super RequestContext, Object, ? extends @Nullable String> requestContentSanitizer) {
         return (TextLogFormatterBuilder) super.requestContentSanitizer(requestContentSanitizer);
     }
 
     @Override
     public TextLogFormatterBuilder responseContentSanitizer(
-            BiFunction<? super RequestContext, Object, ? extends String> responseContentSanitizer) {
+            BiFunction<? super RequestContext, Object, ? extends @Nullable String> responseContentSanitizer) {
         return (TextLogFormatterBuilder) super.responseContentSanitizer(responseContentSanitizer);
     }
 
     @Override
     public TextLogFormatterBuilder contentSanitizer(
-            BiFunction<? super RequestContext, Object, ? extends String> contentSanitizer) {
+            BiFunction<? super RequestContext, Object, ? extends @Nullable String> contentSanitizer) {
         return (TextLogFormatterBuilder) super.contentSanitizer(contentSanitizer);
     }
 
@@ -96,9 +99,9 @@ public TextLogFormatterBuilder includeContext(boolean includeContext) {
     }
 
     /**
-     * Returns a newly-created {@link TextLogFormatter} based on the properties of this builder.
+     * Returns a newly-created text {@link LogFormatter} based on the properties of this builder.
      */
-    public TextLogFormatter build() {
+    public LogFormatter build() {
         return new TextLogFormatter(
                 firstNonNull(requestHeadersSanitizer(), defaultSanitizer()),
                 firstNonNull(responseHeadersSanitizer(), defaultSanitizer()),

diff --git a/kotlin/src/test/kotlin/com/linecorp/armeria/server/kotlin/Jsr305StrictTest.kt b/kotlin/src/test/kotlin/com/linecorp/armeria/server/kotlin/Jsr305StrictTest.kt
@@ -16,6 +16,7 @@
 
 package com.linecorp.armeria.server.kotlin
 
+import com.linecorp.armeria.common.logging.LogFormatter
 import com.linecorp.armeria.server.logging.LoggingService
 import org.junit.jupiter.api.Test
 
@@ -35,5 +36,25 @@ class Jsr305StrictTest {
             .responseContentSanitizer { _, _ -> null }
             .contentSanitizer { _, _ -> null }
             .responseCauseSanitizer { _, _ -> null }
+
+        LogFormatter.builderForText()
+            .requestHeadersSanitizer { _, _ -> null }
+            .responseHeadersSanitizer { _, _ -> null }
+            .requestTrailersSanitizer { _, _ -> null }
+            .responseTrailersSanitizer { _, _ -> null }
+            .headersSanitizer { _, _ -> null }
+            .requestContentSanitizer { _, _ -> null }
+            .responseContentSanitizer { _, _ -> null }
+            .contentSanitizer { _, _ -> null }
+
+        LogFormatter.builderForJson()
+            .requestHeadersSanitizer { _, _ -> null }
+            .responseHeadersSanitizer { _, _ -> null }
+            .requestTrailersSanitizer { _, _ -> null }
+            .responseTrailersSanitizer { _, _ -> null }
+            .headersSanitizer { _, _ -> null }
+            .requestContentSanitizer { _, _ -> null }
+            .responseContentSanitizer { _, _ -> null }
+            .contentSanitizer { _, _ -> null }
     }
 }