From 48fc1a426387abfcba42951505e33eaea7b5420a Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 21 Jun 2023 12:54:06 +0900 Subject: [PATCH 1/6] Fix a bug where builders of `LogFormatter` return internal classes Motivation: `TextLogFormatterBuilder.build()` and `JsonLogFormatterBuilder.build()` return package-private classes that cause compliation 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()`. --- .../armeria/common/logging/JsonLogFormatter.java | 2 +- .../common/logging/JsonLogFormatterBuilder.java | 10 +++------- .../common/logging/LoggingDecoratorBuilder.java | 2 +- .../armeria/common/logging/TextLogFormatter.java | 2 +- .../common/logging/TextLogFormatterBuilder.java | 4 ++-- .../armeria/server/kotlin/Jsr305StrictTest.kt | 11 +++++++++++ 6 files changed, 19 insertions(+), 12 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 index da9fb8f208a..e9d3590d89b 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java @@ -43,7 +43,7 @@ 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 requestHeadersSanitizer; 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 index c01e14eb997..7124c12b530 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java @@ -26,7 +26,6 @@ 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; import com.linecorp.armeria.internal.common.JacksonUtil; @@ -36,8 +35,7 @@ @UnstableApi public final class JsonLogFormatterBuilder extends AbstractLogFormatterBuilder { - @Nullable - private ObjectMapper objectMapper; + private ObjectMapper objectMapper = JacksonUtil.newDefaultObjectMapper(); JsonLogFormatterBuilder() {} @@ -102,11 +100,9 @@ public JsonLogFormatterBuilder 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() { - final ObjectMapper objectMapper = this.objectMapper != null ? - this.objectMapper : JacksonUtil.newDefaultObjectMapper(); + public LogFormatter build() { final BiFunction defaultHeadersSanitizer = defaultSanitizer(objectMapper); final BiFunction defaultContentSanitizer = 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 index 77fdc12dc04..a5f40681053 100644 --- 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 index f7eab809b19..4b07b8b1b56 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java @@ -36,7 +36,7 @@ @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 requestHeadersSanitizer; 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 index 2a7692fed03..f33cdf57504 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java @@ -96,9 +96,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 index 3ee9bcbf83d..7832108262b 100644 --- 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,8 @@ package com.linecorp.armeria.server.kotlin +import com.linecorp.armeria.common.logging.LogFormatter +import com.linecorp.armeria.common.logging.LogWriter import com.linecorp.armeria.server.logging.LoggingService import org.junit.jupiter.api.Test @@ -35,5 +37,14 @@ 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 } } } From 84511aeb32860326dd88871e25f1ed1b0ab6772c Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 21 Jun 2023 13:01:55 +0900 Subject: [PATCH 2/6] Revert --- .../armeria/common/logging/JsonLogFormatterBuilder.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 index 7124c12b530..6e29e9cbbf2 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java @@ -35,7 +35,8 @@ @UnstableApi public final class JsonLogFormatterBuilder extends AbstractLogFormatterBuilder { - private ObjectMapper objectMapper = JacksonUtil.newDefaultObjectMapper(); + @Nullable + private ObjectMapper objectMapper; JsonLogFormatterBuilder() {} @@ -103,6 +104,8 @@ public JsonLogFormatterBuilder contentSanitizer( * Returns a newly-created JSON {@link LogFormatter} based on the properties of this builder. */ public LogFormatter build() { + final ObjectMapper objectMapper = this.objectMapper != null ? + this.objectMapper : JacksonUtil.newDefaultObjectMapper(); final BiFunction defaultHeadersSanitizer = defaultSanitizer(objectMapper); final BiFunction defaultContentSanitizer = From 48ac4165259dec3d6c9e1282f57778faf3556511 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 21 Jun 2023 14:28:37 +0900 Subject: [PATCH 3/6] Add @Nullable --- .../linecorp/armeria/common/logging/JsonLogFormatterBuilder.java | 1 + 1 file changed, 1 insertion(+) 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 index 6e29e9cbbf2..93954b5bfa1 100644 --- a/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java +++ b/core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java @@ -26,6 +26,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; import com.linecorp.armeria.internal.common.JacksonUtil; From 2267414b12c665aeff391796a1fd9cffaa43021d Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 21 Jun 2023 14:45:29 +0900 Subject: [PATCH 4/6] Fix Kotlin jsr305=strict compatibility issue --- .../common/logging/TextLogFormatter.java | 44 ++++++++++++------- .../logging/TextLogFormatterBuilder.java | 25 ++++++----- 2 files changed, 41 insertions(+), 28 deletions(-) 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 index 4b07b8b1b56..00939ce4336 100644 --- 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; @@ -38,30 +39,39 @@ final class TextLogFormatter implements LogFormatter { static final LogFormatter DEFAULT_INSTANCE = new TextLogFormatterBuilder().build(); - private final BiFunction - requestHeadersSanitizer; + private final BiFunction requestHeadersSanitizer; - private final BiFunction - responseHeadersSanitizer; + private final BiFunction responseHeadersSanitizer; - private final BiFunction - requestTrailersSanitizer; + private final BiFunction requestTrailersSanitizer; - private final BiFunction - responseTrailersSanitizer; + private final BiFunction responseTrailersSanitizer; - private final BiFunction requestContentSanitizer; + private final BiFunction requestContentSanitizer; + + private final BiFunction responseContentSanitizer; - private final BiFunction responseContentSanitizer; private final boolean includeContext; TextLogFormatter( - BiFunction requestHeadersSanitizer, - BiFunction responseHeadersSanitizer, - BiFunction requestTrailersSanitizer, - BiFunction responseTrailersSanitizer, - BiFunction requestContentSanitizer, - BiFunction responseContentSanitizer, + BiFunction requestHeadersSanitizer, + BiFunction responseHeadersSanitizer, + BiFunction requestTrailersSanitizer, + BiFunction responseTrailersSanitizer, + BiFunction requestContentSanitizer, + BiFunction responseContentSanitizer, boolean includeContext) { this.requestHeadersSanitizer = requestHeadersSanitizer; this.responseHeadersSanitizer = responseHeadersSanitizer; @@ -282,7 +292,7 @@ public String formatResponse(RequestLog log) { buf.append('}'); final List 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 index f33cdf57504..1ffef054772 100644 --- 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 requestHeadersSanitizer) { + BiFunction requestHeadersSanitizer) { return (TextLogFormatterBuilder) super.requestHeadersSanitizer(requestHeadersSanitizer); } @Override public TextLogFormatterBuilder responseHeadersSanitizer( - BiFunction - responseHeadersSanitizer) { + BiFunction responseHeadersSanitizer) { return (TextLogFormatterBuilder) super.responseHeadersSanitizer(responseHeadersSanitizer); } @Override public TextLogFormatterBuilder requestTrailersSanitizer( - BiFunction - requestTrailersSanitizer) { + BiFunction requestTrailersSanitizer) { return (TextLogFormatterBuilder) super.requestTrailersSanitizer(requestTrailersSanitizer); } @Override public TextLogFormatterBuilder responseTrailersSanitizer( - BiFunction - responseTrailersSanitizer) { + BiFunction responseTrailersSanitizer) { return (TextLogFormatterBuilder) super.responseTrailersSanitizer(responseTrailersSanitizer); } @Override public TextLogFormatterBuilder headersSanitizer( - BiFunction headersSanitizer) { + BiFunction headersSanitizer) { return (TextLogFormatterBuilder) super.headersSanitizer(headersSanitizer); } @Override public TextLogFormatterBuilder requestContentSanitizer( - BiFunction requestContentSanitizer) { + BiFunction requestContentSanitizer) { return (TextLogFormatterBuilder) super.requestContentSanitizer(requestContentSanitizer); } @Override public TextLogFormatterBuilder responseContentSanitizer( - BiFunction responseContentSanitizer) { + BiFunction responseContentSanitizer) { return (TextLogFormatterBuilder) super.responseContentSanitizer(responseContentSanitizer); } @Override public TextLogFormatterBuilder contentSanitizer( - BiFunction contentSanitizer) { + BiFunction contentSanitizer) { return (TextLogFormatterBuilder) super.contentSanitizer(contentSanitizer); } From f9ccc454ebbe08581a5d0461ea1ade75d307409e Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 21 Jun 2023 14:57:23 +0900 Subject: [PATCH 5/6] lint --- .../com/linecorp/armeria/server/kotlin/Jsr305StrictTest.kt | 1 - 1 file changed, 1 deletion(-) 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 index 7832108262b..59bf504bcc0 100644 --- a/kotlin/src/test/kotlin/com/linecorp/armeria/server/kotlin/Jsr305StrictTest.kt +++ b/kotlin/src/test/kotlin/com/linecorp/armeria/server/kotlin/Jsr305StrictTest.kt @@ -17,7 +17,6 @@ package com.linecorp.armeria.server.kotlin import com.linecorp.armeria.common.logging.LogFormatter -import com.linecorp.armeria.common.logging.LogWriter import com.linecorp.armeria.server.logging.LoggingService import org.junit.jupiter.api.Test From 8c9dd7ced788df6445b37ef92c8bd945d0b4f7f7 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 21 Jun 2023 15:40:49 +0900 Subject: [PATCH 6/6] Fix Kotlin jsr305=strict compatibility for JsonLogFormatterBuilder --- .../common/logging/JsonLogFormatter.java | 34 +++++++++++-------- .../logging/JsonLogFormatterBuilder.java | 17 +++++----- .../armeria/server/kotlin/Jsr305StrictTest.kt | 11 ++++++ 3 files changed, 39 insertions(+), 23 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 index e9d3590d89b..11c6de2a2a1 100644 --- 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; @@ -45,34 +46,37 @@ final class JsonLogFormatter implements LogFormatter { static final LogFormatter DEFAULT_INSTANCE = new JsonLogFormatterBuilder().build(); - private final BiFunction + private final BiFunction requestHeadersSanitizer; - private final BiFunction + private final BiFunction responseHeadersSanitizer; - private final BiFunction + private final BiFunction requestTrailersSanitizer; - private final BiFunction + private final BiFunction responseTrailersSanitizer; - private final BiFunction requestContentSanitizer; + private final BiFunction + requestContentSanitizer; - private final BiFunction responseContentSanitizer; + private final BiFunction + responseContentSanitizer; private final ObjectMapper objectMapper; JsonLogFormatter( - BiFunction requestHeadersSanitizer, - BiFunction - responseHeadersSanitizer, - BiFunction - requestTrailersSanitizer, - BiFunction - responseTrailersSanitizer, - BiFunction requestContentSanitizer, - BiFunction responseContentSanitizer, + BiFunction requestHeadersSanitizer, + BiFunction responseHeadersSanitizer, + BiFunction requestTrailersSanitizer, + BiFunction responseTrailersSanitizer, + BiFunction requestContentSanitizer, + BiFunction 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 index 93954b5bfa1..5c1a177bea9 100644 --- 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,53 +51,54 @@ public JsonLogFormatterBuilder objectMapper(ObjectMapper objectMapper) { @Override public JsonLogFormatterBuilder requestHeadersSanitizer( - BiFunction + BiFunction requestHeadersSanitizer) { return (JsonLogFormatterBuilder) super.requestHeadersSanitizer(requestHeadersSanitizer); } @Override public JsonLogFormatterBuilder responseHeadersSanitizer( - BiFunction + BiFunction responseHeadersSanitizer) { return (JsonLogFormatterBuilder) super.responseHeadersSanitizer(responseHeadersSanitizer); } @Override public JsonLogFormatterBuilder requestTrailersSanitizer( - BiFunction + BiFunction requestTrailersSanitizer) { return (JsonLogFormatterBuilder) super.requestTrailersSanitizer(requestTrailersSanitizer); } @Override public JsonLogFormatterBuilder responseTrailersSanitizer( - BiFunction + BiFunction responseTrailersSanitizer) { return (JsonLogFormatterBuilder) super.responseTrailersSanitizer(responseTrailersSanitizer); } @Override public JsonLogFormatterBuilder headersSanitizer( - BiFunction headersSanitizer) { + BiFunction + headersSanitizer) { return (JsonLogFormatterBuilder) super.headersSanitizer(headersSanitizer); } @Override public JsonLogFormatterBuilder requestContentSanitizer( - BiFunction requestContentSanitizer) { + BiFunction requestContentSanitizer) { return (JsonLogFormatterBuilder) super.requestContentSanitizer(requestContentSanitizer); } @Override public JsonLogFormatterBuilder responseContentSanitizer( - BiFunction responseContentSanitizer) { + BiFunction responseContentSanitizer) { return (JsonLogFormatterBuilder) super.responseContentSanitizer(responseContentSanitizer); } @Override public JsonLogFormatterBuilder contentSanitizer( - BiFunction contentSanitizer) { + BiFunction contentSanitizer) { return (JsonLogFormatterBuilder) super.contentSanitizer(contentSanitizer); } 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 index 59bf504bcc0..256bea1ea81 100644 --- a/kotlin/src/test/kotlin/com/linecorp/armeria/server/kotlin/Jsr305StrictTest.kt +++ b/kotlin/src/test/kotlin/com/linecorp/armeria/server/kotlin/Jsr305StrictTest.kt @@ -36,6 +36,7 @@ class Jsr305StrictTest { .responseContentSanitizer { _, _ -> null } .contentSanitizer { _, _ -> null } .responseCauseSanitizer { _, _ -> null } + LogFormatter.builderForText() .requestHeadersSanitizer { _, _ -> null } .responseHeadersSanitizer { _, _ -> null } @@ -45,5 +46,15 @@ class Jsr305StrictTest { .requestContentSanitizer { _, _ -> null } .responseContentSanitizer { _, _ -> null } .contentSanitizer { _, _ -> null } + + LogFormatter.builderForJson() + .requestHeadersSanitizer { _, _ -> null } + .responseHeadersSanitizer { _, _ -> null } + .requestTrailersSanitizer { _, _ -> null } + .responseTrailersSanitizer { _, _ -> null } + .headersSanitizer { _, _ -> null } + .requestContentSanitizer { _, _ -> null } + .responseContentSanitizer { _, _ -> null } + .contentSanitizer { _, _ -> null } } }