Skip to content

Commit

Permalink
Fix additional backward compatibility breaks (#56)
Browse files Browse the repository at this point in the history 
Restore types on LoggingInvocationEventHandler constants to
com.palantir.tritium.api.functions.LongPredicate

Add explicit backward compatibility bridge constructors to
AbstractInvocationEventHandler and LoggingInvocationEventHandler
  • Loading branch information
@schlosna
schlosna authored Nov 6, 2017
1 parent de776f9 commit 98f7b1b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 31 deletions.
17 changes: 11 additions & 6 deletions tritium-core/src/main/java/com/palantir/tritium/event/AbstractInvocationEventHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <C>
* invocation context
* @param <C> invocation context
*/
public abstract class AbstractInvocationEventHandler<C extends InvocationContext>
implements InvocationEventHandler<C> {

private static final Object[] NO_ARGS = {};

private final BooleanSupplier isEnabledSupplier;
private final java.util.function.BooleanSupplier isEnabledSupplier;

/**
* Always enabled instrumentation handler.
Expand All @@ -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");
}

Expand All @@ -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<? extends InvocationEventHandler<InvocationContext>> clazz) {
checkNotNull(clazz, "clazz");
return InstrumentationProperties.getSystemPropertySupplier(clazz.getName());
Expand Down
32 changes: 21 additions & 11 deletions ...ium-slf4j/src/main/java/com/palantir/tritium/event/log/LoggingInvocationEventHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -45,25 +44,36 @@ public class LoggingInvocationEventHandler extends AbstractInvocationEventHandle
private static final Logger CLASS_LOGGER = LoggerFactory.getLogger(LoggingInvocationEventHandler.class);
private static final List<String> 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");
Expand Down Expand Up @@ -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;
}
}

Expand Down
31 changes: 17 additions & 14 deletions ...slf4j/src/test/java/com/palantir/tritium/event/log/LoggingInvocationEventHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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();
}

}

0 comments on commit 98f7b1b

Please sign in to comment.