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.

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

Annotate MetadataKey.emit... methods to indicate that the value can be null. Fix implementations to account for null values (by ignoring nulls). #398

Merged
merged 1 commit into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions api/src/main/java/com/google/common/flogger/LogContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ private Key() {}
public static final MetadataKey<Object> LOG_SITE_GROUPING_KEY =
new MetadataKey<Object>("group_by", Object.class, true) {
@Override
public void emitRepeated(Iterator<Object> keys, KeyValueHandler out) {
public void emitRepeated(Iterator<?> keys, KeyValueHandler out) {
if (keys.hasNext()) {
Object first = keys.next();
if (!keys.hasNext()) {
Expand Down Expand Up @@ -175,7 +175,10 @@ public void emitRepeated(Iterator<Object> keys, KeyValueHandler out) {
public static final MetadataKey<Tags> TAGS =
new MetadataKey<Tags>("tags", Tags.class, false) {
@Override
public void emit(Tags tags, KeyValueHandler out) {
public void emit(@Nullable Tags tags, KeyValueHandler out) {
if (tags == null) {
return;
}
for (Map.Entry<String, ? extends Set<Object>> e : tags.asMap().entrySet()) {
Set<Object> values = e.getValue();
if (!values.isEmpty()) {
Expand Down
8 changes: 4 additions & 4 deletions api/src/main/java/com/google/common/flogger/MetadataKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public final boolean canRepeat() {
* Emits one or more key/value pairs for the given metadata value. Call this method in preference
* to using {@link #emit} directly to protect against unbounded reentrant logging.
*/
public final void safeEmit(T value, KeyValueHandler kvh) {
public final void safeEmit(@Nullable T value, KeyValueHandler kvh) {
if (isCustom && Platform.getCurrentRecursionDepth() > MAX_CUSTOM_METADATAKEY_RECURSION_DEPTH) {
// Recursive logging detected, possibly caused by custom metadata keys triggering reentrant
// logging. To halt recursion, emit the keys in the default non-custom format without invoking
Expand All @@ -187,7 +187,7 @@ public final void safeEmit(T value, KeyValueHandler kvh) {
* in preference to using {@link #emitRepeated} directly to protect against unbounded reentrant
* logging.
*/
public final void safeEmitRepeated(Iterator<T> values, KeyValueHandler kvh) {
public final void safeEmitRepeated(Iterator<? extends @Nullable T> values, KeyValueHandler kvh) {
checkState(canRepeat, "non repeating key");
if (isCustom && Platform.getCurrentRecursionDepth() > MAX_CUSTOM_METADATAKEY_RECURSION_DEPTH) {
// Recursive logging detected, possibly caused by custom metadata keys triggering reentrant
Expand Down Expand Up @@ -236,7 +236,7 @@ public final void safeEmitRepeated(Iterator<T> values, KeyValueHandler kvh) {
*
* <p>By default this method just calls {@code out.handle(getLabel(), value)}.
*/
protected void emit(T value, KeyValueHandler kvh) {
protected void emit(@Nullable T value, KeyValueHandler kvh) {
kvh.handle(getLabel(), value);
}

Expand All @@ -253,7 +253,7 @@ protected void emit(T value, KeyValueHandler kvh) {
* <p>See the {@link #emit(Object,KeyValueHandler)} method for additional caveats for custom
* implementations.
*/
protected void emitRepeated(Iterator<T> values, KeyValueHandler kvh) {
protected void emitRepeated(Iterator<? extends @Nullable T> values, KeyValueHandler kvh) {
while (values.hasNext()) {
emit(values.next(), kvh);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ protected void emit(Object value, KeyValueHandler kvh) {
}

@Override
protected void emitRepeated(Iterator<Object> values, KeyValueHandler kvh) {
protected void emitRepeated(Iterator<?> values, KeyValueHandler kvh) {
// Hack for test to preserve the given values past a single use. In normal logging there
// would be a new Metadata instance created for each of the reentrant logging calls.
ImmutableList<Object> copy = ImmutableList.copyOf(values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Iterator;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import org.jspecify.annotations.Nullable;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -42,7 +43,7 @@ public class SimpleLogRecordTest {
private static final MetadataKey<String> PATH_KEY =
new MetadataKey<String>("path", String.class, true) {
@Override
public void emitRepeated(Iterator<String> values, KeyValueHandler out) {
public void emitRepeated(Iterator<? extends @Nullable String> values, KeyValueHandler out) {
out.handle(getLabel(), Joiner.on('/').join(values));
}
};
Expand Down
Loading