Skip to content

Commit

Permalink
Fix #393: Loosen InvocationEventHanndler context bound to object
Browse files Browse the repository at this point in the history
  • Loading branch information
@carterkozak
carterkozak committed Sep 15, 2019
1 parent 64742c3 commit 2b0e12d
Show file tree
Hide file tree
Showing 11 changed files with 150 additions and 189 deletions.
6 changes: 3 additions & 3 deletions tritium-api/src/main/java/com/palantir/tritium/event/InvocationEventHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
* @see java.lang.reflect.InvocationHandler
* @param <C> invocation context
*/
public interface InvocationEventHandler<C extends InvocationContext> {
public interface InvocationEventHandler<C> {

/**
* Returns true if this event handler instance is enabled, otherwise false.
Expand Down Expand Up @@ -57,7 +57,7 @@ public interface InvocationEventHandler<C extends InvocationContext> {
* @param context the current invocation context.
* @param result the return value from invocation, or null if {@link Void}.
*/
void onSuccess(@Nullable InvocationContext context, @Nullable Object result);
void onSuccess(@Nullable C context, @Nullable Object result);

/**
* Invoked when an invocation fails.
Expand All @@ -69,6 +69,6 @@ public interface InvocationEventHandler<C extends InvocationContext> {
* @param context the current invocation context.
* @param cause the throwable which caused the failure.
*/
void onFailure(@Nullable InvocationContext context, @Nonnull Throwable cause);
void onFailure(@Nullable C context, @Nonnull Throwable cause);

}
7 changes: 3 additions & 4 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 @@ -28,8 +28,7 @@
*
* @param <C> invocation context
*/
public abstract class AbstractInvocationEventHandler<C extends InvocationContext>
implements InvocationEventHandler<C> {
public abstract class AbstractInvocationEventHandler<C> implements InvocationEventHandler<C> {

private static final Logger logger = LoggerFactory.getLogger(AbstractInvocationEventHandler.class);

Expand Down Expand Up @@ -78,7 +77,7 @@ public final boolean isEnabled() {
*
* @param context invocation context
*/
protected final void debugIfNullContext(@Nullable InvocationContext context) {
protected final void debugIfNullContext(@Nullable C context) {
if (context == null) {
logger.debug(
"{} encountered null metric context, likely due to exception in preInvocation",
Expand All @@ -98,7 +97,7 @@ private static SafeArg<String> safeClassName(@Nullable Object obj) {
* @return false if "instrument.fully.qualified.class.Name" is set to "false", otherwise true
*/
protected static com.palantir.tritium.api.functions.BooleanSupplier getSystemPropertySupplier(
Class<? extends InvocationEventHandler<InvocationContext>> clazz) {
Class<? extends InvocationEventHandler<?>> clazz) {
checkNotNull(clazz, "clazz");
return InstrumentationProperties.getSystemPropertySupplier(clazz.getName());
}
Expand Down
78 changes: 30 additions & 48 deletions tritium-core/src/main/java/com/palantir/tritium/event/CompositeInvocationEventHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,20 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class CompositeInvocationEventHandler extends AbstractInvocationEventHandler<InvocationContext> {
public final class CompositeInvocationEventHandler extends AbstractInvocationEventHandler<Object[]> {

private static final Logger logger = LoggerFactory.getLogger(CompositeInvocationEventHandler.class);

private final InvocationEventHandler<InvocationContext>[] handlers;
private final InvocationEventHandler<?>[] handlers;

@SuppressWarnings("unchecked")
private CompositeInvocationEventHandler(List<InvocationEventHandler<InvocationContext>> handlers) {
private CompositeInvocationEventHandler(List<InvocationEventHandler<?>> handlers) {
this.handlers = checkNotNull(handlers, "handlers").toArray(new InvocationEventHandler[0]);
for (InvocationEventHandler<InvocationContext> handler : handlers) {
for (InvocationEventHandler<?> handler : handlers) {
checkNotNull(handler, "Null handlers are not allowed");
}
}

public static InvocationEventHandler<InvocationContext> of(
List<InvocationEventHandler<InvocationContext>> handlers) {
public static InvocationEventHandler<?> of(List<InvocationEventHandler<?>> handlers) {
if (handlers.isEmpty()) {
return NoOpInvocationEventHandler.INSTANCE;
} else if (handlers.size() == 1) {
Expand All @@ -54,56 +52,58 @@ public static InvocationEventHandler<InvocationContext> of(
}

@Nullable
private InvocationEventHandler<InvocationContext> tryGetEnabledHandler(int index) {
InvocationEventHandler<InvocationContext> handler = handlers[index];
private InvocationEventHandler<?> tryGetEnabledHandler(int index) {
InvocationEventHandler<?> handler = handlers[index];
if (handler.isEnabled()) {
return handler;
}
return null;
}

@Override
public InvocationContext preInvocation(@Nonnull Object instance, @Nonnull Method method, @Nonnull Object[] args) {
InvocationContext[] contexts = new InvocationContext[handlers.length];
public Object[] preInvocation(@Nonnull Object instance, @Nonnull Method method, @Nonnull Object[] args) {
Object[] contexts = new Object[handlers.length];

for (int i = 0; i < handlers.length; i++) {
contexts[i] = handlePreInvocation(tryGetEnabledHandler(i), instance, method, args);
}

return new CompositeInvocationContext(instance, method, args, contexts);
return contexts;
}

@Override
public void onSuccess(@Nullable InvocationContext context, @Nullable Object result) {
public void onSuccess(@Nullable Object[] context, @Nullable Object result) {
debugIfNullContext(context);
if (context != null) {
success(((CompositeInvocationContext) context).getContexts(), result);
success(context, result);
}
}

private void success(@Nonnull InvocationContext[] contexts, @Nullable Object result) {
@SuppressWarnings("unchecked")
private void success(@Nonnull Object[] contexts, @Nullable Object result) {
for (int i = contexts.length - 1; i > -1; i--) {
handleSuccess(handlers[i], contexts[i], result);
handleSuccess((InvocationEventHandler<Object>) handlers[i], contexts[i], result);
}
}

@Override
public void onFailure(@Nullable InvocationContext context, @Nonnull Throwable cause) {
public void onFailure(@Nullable Object[] context, @Nonnull Throwable cause) {
debugIfNullContext(context);
if (context != null) {
failure(((CompositeInvocationContext) context).getContexts(), cause);
failure(context, cause);
}
}

private void failure(InvocationContext[] contexts, @Nonnull Throwable cause) {
@SuppressWarnings("unchecked")
private void failure(Object[] contexts, @Nonnull Throwable cause) {
for (int i = contexts.length - 1; i > -1; i--) {
handleFailure(handlers[i], contexts[i], cause);
handleFailure((InvocationEventHandler<Object>) handlers[i], contexts[i], cause);
}
}

@Nullable
private static InvocationContext handlePreInvocation(
@Nullable InvocationEventHandler<? extends InvocationContext> handler,
private static Object handlePreInvocation(
@Nullable InvocationEventHandler<?> handler,
Object instance,
Method method,
Object[] args) {
Expand All @@ -123,7 +123,7 @@ public String toString() {
}

private static void preInvocationFailed(
@Nullable InvocationEventHandler<? extends InvocationContext> handler,
@Nullable InvocationEventHandler<?> handler,
@Nullable Object instance,
Method method,
@Nullable Exception exception) {
Expand All @@ -136,9 +136,9 @@ private static void preInvocationFailed(
exception);
}

private static void handleSuccess(
InvocationEventHandler<?> handler,
@Nullable InvocationContext context,
private static <T> void handleSuccess(
InvocationEventHandler<T> handler,
@Nullable T context,
@Nullable Object result) {
if (context != null) {
try {
Expand All @@ -149,9 +149,9 @@ private static void handleSuccess(
}
}

private static void handleFailure(
InvocationEventHandler<?> handler,
@Nullable InvocationContext context,
private static <T> void handleFailure(
InvocationEventHandler<T> handler,
@Nullable T context,
Throwable cause) {
if (context != null) {
try {
Expand All @@ -164,7 +164,7 @@ private static void handleFailure(

private static void eventFailed(
String event,
@Nullable InvocationContext context,
@Nullable Object context,
@Nullable Object result,
RuntimeException exception) {
logger.warn(
Expand All @@ -174,22 +174,4 @@ private static void eventFailed(
UnsafeArg.of("result", result),
exception);
}

static class CompositeInvocationContext extends DefaultInvocationContext {

private final InvocationContext[] contexts;

CompositeInvocationContext(
Object instance,
Method method,
@Nullable Object[] args,
InvocationContext[] contexts) {
super(System.nanoTime(), instance, method, args);
this.contexts = checkNotNull(contexts);
}

InvocationContext[] getContexts() {
return contexts;
}
}
}
Loading

0 comments on commit 2b0e12d

Please sign in to comment.