Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a bug where builders of LogFormatter return internal classes #4975

Merged
merged 6 commits into from
Jun 21, 2023
Merged

Fix a bug where builders of LogFormatter return internal classes #4975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
48fc1a4
Fix a bug where builders of `LogFormatter` return internal classes
ikhoon Jun 21, 2023
84511ae
Revert
ikhoon Jun 21, 2023
48ac416
Add @Nullable
ikhoon Jun 21, 2023
2267414
Fix Kotlin jsr305=strict compatibility issue
ikhoon Jun 21, 2023
f9ccc45
lint
ikhoon Jun 21, 2023
8c9dd7c
Fix Kotlin jsr305=strict compatibility for JsonLogFormatterBuilder
ikhoon Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
21 changes: 11 additions & 10 deletions core/src/main/java/com/linecorp/armeria/common/logging/JsonLogFormatterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/linecorp/armeria/common/logging/LoggingDecoratorBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
46 changes: 28 additions & 18 deletions core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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=");
Expand Down
29 changes: 16 additions & 13 deletions core/src/main/java/com/linecorp/armeria/common/logging/TextLogFormatterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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);
}

Expand All @@ -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()),
Expand Down
21 changes: 21 additions & 0 deletions kotlin/src/test/kotlin/com/linecorp/armeria/server/kotlin/Jsr305StrictTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 }
}
}