From 98f7b1bdebbde689a21be2236778453f4121187a Mon Sep 17 00:00:00 2001 From: David Schlosnagle Date: Mon, 6 Nov 2017 13:03:02 -0500 Subject: [PATCH] Fix additional backward compatibility breaks (#56) Restore types on LoggingInvocationEventHandler constants to com.palantir.tritium.api.functions.LongPredicate Add explicit backward compatibility bridge constructors to AbstractInvocationEventHandler and LoggingInvocationEventHandler --- .../event/AbstractInvocationEventHandler.java | 17 ++++++---- .../log/LoggingInvocationEventHandler.java | 32 ++++++++++++------- .../LoggingInvocationEventHandlerTest.java | 31 ++++++++++-------- 3 files changed, 49 insertions(+), 31 deletions(-) diff --git a/tritium-core/src/main/java/com/palantir/tritium/event/AbstractInvocationEventHandler.java b/tritium-core/src/main/java/com/palantir/tritium/event/AbstractInvocationEventHandler.java index 01b00d3c2..bf291038c 100644 --- a/tritium-core/src/main/java/com/palantir/tritium/event/AbstractInvocationEventHandler.java +++ b/tritium-core/src/main/java/com/palantir/tritium/event/AbstractInvocationEventHandler.java @@ -18,21 +18,19 @@ import static com.google.common.base.Preconditions.checkNotNull; -import java.util.function.BooleanSupplier; import javax.annotation.Nullable; /** * Abstract invocation event handler implementation. * - * @param - * invocation context + * @param invocation context */ public abstract class AbstractInvocationEventHandler implements InvocationEventHandler { private static final Object[] NO_ARGS = {}; - private final BooleanSupplier isEnabledSupplier; + private final java.util.function.BooleanSupplier isEnabledSupplier; /** * Always enabled instrumentation handler. @@ -41,7 +39,14 @@ protected AbstractInvocationEventHandler() { this(com.palantir.tritium.api.functions.BooleanSupplier.TRUE); } - protected AbstractInvocationEventHandler(BooleanSupplier isEnabledSupplier) { + /** + * Bridge for backward compatibility. + */ + protected AbstractInvocationEventHandler(com.palantir.tritium.api.functions.BooleanSupplier isEnabledSupplier) { + this((java.util.function.BooleanSupplier) isEnabledSupplier); + } + + protected AbstractInvocationEventHandler(java.util.function.BooleanSupplier isEnabledSupplier) { this.isEnabledSupplier = checkNotNull(isEnabledSupplier, "isEnabledSupplier"); } @@ -60,7 +65,7 @@ public final boolean isEnabled() { * instrumentation handler class * @return false if "instrument.fully.qualified.class.Name" is set to "false", otherwise true */ - protected static BooleanSupplier getSystemPropertySupplier( + protected static com.palantir.tritium.api.functions.BooleanSupplier getSystemPropertySupplier( Class> clazz) { checkNotNull(clazz, "clazz"); return InstrumentationProperties.getSystemPropertySupplier(clazz.getName()); diff --git a/tritium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java b/tritium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java index 103bfb975..cc9359e37 100644 --- a/tritium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java +++ b/tritium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.palantir.logsafe.SafeArg; +import com.palantir.tritium.api.functions.BooleanSupplier; import com.palantir.tritium.event.AbstractInvocationEventHandler; import com.palantir.tritium.event.DefaultInvocationContext; import com.palantir.tritium.event.InvocationContext; @@ -29,8 +30,6 @@ import java.util.Collection; import java.util.List; import java.util.concurrent.TimeUnit; -import java.util.function.BooleanSupplier; -import java.util.function.LongPredicate; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.slf4j.Logger; @@ -45,25 +44,36 @@ public class LoggingInvocationEventHandler extends AbstractInvocationEventHandle private static final Logger CLASS_LOGGER = LoggerFactory.getLogger(LoggingInvocationEventHandler.class); private static final List MESSAGE_PATTERNS = generateMessagePatterns(10); - public static final LongPredicate LOG_ALL_DURATIONS = com.palantir.tritium.api.functions.LongPredicate.TRUE; + public static final com.palantir.tritium.api.functions.LongPredicate LOG_ALL_DURATIONS = + com.palantir.tritium.api.functions.LongPredicate.TRUE; - public static final LongPredicate LOG_DURATIONS_GREATER_THAN_1_MICROSECOND = nanos -> - TimeUnit.MICROSECONDS.convert(nanos, TimeUnit.NANOSECONDS) > 1; + public static final com.palantir.tritium.api.functions.LongPredicate LOG_DURATIONS_GREATER_THAN_1_MICROSECOND = + nanos -> TimeUnit.MICROSECONDS.convert(nanos, TimeUnit.NANOSECONDS) > 1; - public static final LongPredicate LOG_DURATIONS_GREATER_THAN_0_MILLIS = nanos -> - TimeUnit.MILLISECONDS.convert(nanos, TimeUnit.NANOSECONDS) > 0; + public static final com.palantir.tritium.api.functions.LongPredicate LOG_DURATIONS_GREATER_THAN_0_MILLIS = + nanos -> TimeUnit.MILLISECONDS.convert(nanos, TimeUnit.NANOSECONDS) > 0; - public static final LongPredicate NEVER_LOG = com.palantir.tritium.api.functions.LongPredicate.FALSE; + public static final com.palantir.tritium.api.functions.LongPredicate NEVER_LOG = + com.palantir.tritium.api.functions.LongPredicate.FALSE; private final Logger logger; private final LoggingLevel level; - private final LongPredicate durationPredicate; + private final java.util.function.LongPredicate durationPredicate; public LoggingInvocationEventHandler(Logger logger, LoggingLevel level) { this(logger, level, LOG_ALL_DURATIONS); } - public LoggingInvocationEventHandler(Logger logger, LoggingLevel level, LongPredicate durationPredicate) { + /** + * Bridge for backward compatibility. + */ + public LoggingInvocationEventHandler(Logger logger, LoggingLevel level, + com.palantir.tritium.api.functions.LongPredicate durationPredicate) { + this(logger, level, (java.util.function.LongPredicate) durationPredicate); + } + + public LoggingInvocationEventHandler(Logger logger, LoggingLevel level, + java.util.function.LongPredicate durationPredicate) { super(createEnabledSupplier(logger, level)); this.logger = checkNotNull(logger, "logger"); this.level = checkNotNull(level, "level"); @@ -155,7 +165,7 @@ private static BooleanSupplier createEnabledSupplier(final Logger logger, final if (getSystemPropertySupplier(LoggingInvocationEventHandler.class).getAsBoolean()) { return () -> isEnabled(logger, level); } else { - return com.palantir.tritium.api.functions.BooleanSupplier.FALSE; + return BooleanSupplier.FALSE; } } diff --git a/tritium-slf4j/src/test/java/com/palantir/tritium/event/log/LoggingInvocationEventHandlerTest.java b/tritium-slf4j/src/test/java/com/palantir/tritium/event/log/LoggingInvocationEventHandlerTest.java index e7bb8dbc3..b060657cd 100644 --- a/tritium-slf4j/src/test/java/com/palantir/tritium/event/log/LoggingInvocationEventHandlerTest.java +++ b/tritium-slf4j/src/test/java/com/palantir/tritium/event/log/LoggingInvocationEventHandlerTest.java @@ -16,10 +16,7 @@ package com.palantir.tritium.event.log; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.startsWith; -import static org.junit.Assert.assertThat; +import static org.assertj.core.api.Assertions.assertThat; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -116,10 +113,10 @@ public void testNullLevel() { @Test public void testGenerateMessagePattern() { - assertThat(LoggingInvocationEventHandler.generateMessagePattern(0), equalTo("{}.{}() took {}ms")); - assertThat(LoggingInvocationEventHandler.generateMessagePattern(1), equalTo("{}.{}({}) took {}ms")); - assertThat(LoggingInvocationEventHandler.generateMessagePattern(2), equalTo("{}.{}({}, {}) took {}ms")); - assertThat(LoggingInvocationEventHandler.generateMessagePattern(3), equalTo("{}.{}({}, {}, {}) took {}ms")); + assertThat(LoggingInvocationEventHandler.generateMessagePattern(0)).isEqualTo("{}.{}() took {}ms"); + assertThat(LoggingInvocationEventHandler.generateMessagePattern(1)).isEqualTo("{}.{}({}) took {}ms"); + assertThat(LoggingInvocationEventHandler.generateMessagePattern(2)).isEqualTo("{}.{}({}, {}) took {}ms"); + assertThat(LoggingInvocationEventHandler.generateMessagePattern(3)).isEqualTo("{}.{}({}, {}, {}) took {}ms"); } @Test @@ -138,8 +135,7 @@ public void testGenerateMessage() throws Throwable { Object[] logParams = LoggingInvocationEventHandler.getLogParams(method, args, durationNanoseconds, level); String logMessage = MessageFormatter.arrayFormat(messagePattern, logParams).getMessage(); - assertThat(logMessage, - startsWith("TestInterface.multiArgumentMethod(String, int, Collection[3]) took 1.235ms")); + assertThat(logMessage).startsWith("TestInterface.multiArgumentMethod(String, int, Collection[3]) took 1.235ms"); } @Test @@ -154,16 +150,23 @@ public void testGenerateCollectionsMessage() throws Throwable { args[0] = ImmutableSet.of("a", "b"); Object[] logParams = LoggingInvocationEventHandler.getLogParams(method, args, durationNanoseconds, level); String logMessage = MessageFormatter.arrayFormat(messagePattern, logParams).getMessage(); - assertThat(logMessage, - startsWith("TestInterface.bulk(Set[2]) took 1.235ms")); + assertThat(logMessage).startsWith("TestInterface.bulk(Set[2]) took 1.235ms"); } @Test public void testGetMessagePattern() { for (int i = 0; i < 20; i++) { - assertThat(LoggingInvocationEventHandler.getMessagePattern(new Object[i]), - containsString("{}")); + assertThat(LoggingInvocationEventHandler.getMessagePattern(new Object[i])).contains("{}"); } } + @Test + public void testBackwardCompatibility() { + assertThat(LoggingInvocationEventHandler.LOG_DURATIONS_GREATER_THAN_1_MICROSECOND) + .isInstanceOf(com.palantir.tritium.api.functions.LongPredicate.class); + + com.palantir.tritium.api.functions.LongPredicate legacyPredicate = input -> false; + assertThat(new LoggingInvocationEventHandler(getLogger(), LoggingLevel.TRACE, legacyPredicate)).isNotNull(); + } + }