diff --git a/api/src/main/java/com/google/common/flogger/context/ScopedLoggingContext.java b/api/src/main/java/com/google/common/flogger/context/ScopedLoggingContext.java index 37fe63b..243b60d 100644 --- a/api/src/main/java/com/google/common/flogger/context/ScopedLoggingContext.java +++ b/api/src/main/java/com/google/common/flogger/context/ScopedLoggingContext.java @@ -137,25 +137,25 @@ public abstract static class Builder { private Tags tags = null; private ContextMetadata.Builder metadata = null; private LogLevelMap logLevelMap = null; + private boolean hasLogLevelMap = false; protected Builder() {} /** * Sets the tags to be used with the context. This method can be called at most once per - * builder. Calling with a null value does nothing. + * builder. */ @CanIgnoreReturnValue - public final Builder withTags(@NullableDecl Tags tags) { + public final Builder withTags(Tags tags) { checkState(this.tags == null, "tags already set"); - if (tags != null) { - this.tags = tags; - } + checkNotNull(tags, "tags"); + this.tags = tags; return this; } /** * Adds a single metadata key/value pair to the context. This method can be called multiple - * times on a builder. Calling with a null value does nothing. + * times on a builder. Calling with a null value does not add metadata. */ @CanIgnoreReturnValue public final Builder withMetadata(MetadataKey key, @NullableDecl T value) { @@ -171,14 +171,13 @@ public final Builder withMetadata(MetadataKey key, @NullableDecl T value) /** * Sets the log level map to be used with the context being built. This method can be called at - * most once per builder. Calling with a null value does nothing. + * most once per builder. Calling with a null value does not set a log level map. */ @CanIgnoreReturnValue public final Builder withLogLevelMap(@NullableDecl LogLevelMap logLevelMap) { - checkState(this.logLevelMap == null, "log level map already set"); - if (logLevelMap != null) { - this.logLevelMap = logLevelMap; - } + checkState(!hasLogLevelMap, "log level map already set"); + hasLogLevelMap = true; + this.logLevelMap = logLevelMap; return this; } diff --git a/api/src/test/java/com/google/common/flogger/testing/AbstractScopedLoggingContextTest.java b/api/src/test/java/com/google/common/flogger/testing/AbstractScopedLoggingContextTest.java index b780abf..5e68a65 100644 --- a/api/src/test/java/com/google/common/flogger/testing/AbstractScopedLoggingContextTest.java +++ b/api/src/test/java/com/google/common/flogger/testing/AbstractScopedLoggingContextTest.java @@ -20,6 +20,7 @@ import static com.google.common.flogger.testing.MetadataSubject.assertThat; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.fail; import com.google.common.collect.ImmutableMap; import com.google.common.flogger.LoggingScope; @@ -129,11 +130,11 @@ public void testLoggedTags_areMerged() { } @Test - public void testNewContext_withNullTags_ignored() { + public void testNewContext_withNoOpTags_ignored() { assertThat(getTagMap()).isEmpty(); context .newContext() - .withTags(null) + .withTags(Tags.empty()) .run( () -> { assertThat(getTagMap()).isEmpty(); @@ -143,6 +144,17 @@ public void testNewContext_withNullTags_ignored() { checkDone(); } + @Test + public void testNewContext_withTagsTwice_rejected() { + try { + context.newContext().withTags(Tags.empty()).withTags(Tags.of("foo", "bar")).run(() -> {}); + fail("Expected IllegalStateException"); + } catch (IllegalStateException e) { + markTestAsDone(); + } + checkDone(); + } + @Test public void testNewContext_withMetadata() { assertThat(getMetadata()).hasSize(0); @@ -205,6 +217,17 @@ public void testNewContext_withNullLogLevelMap_ignored() { checkDone(); } + @Test + public void testNewContext_withLogLevelMapTwice_rejected() { + try { + context.newContext().withLogLevelMap(null).withLogLevelMap(null).run(() -> {}); + fail("Expected IllegalStateException"); + } catch (IllegalStateException e) { + markTestAsDone(); + } + checkDone(); + } + @Test public void testNewContext_withMergedTags() { assertThat(getTagMap()).isEmpty();