Fix #393: Loosen InvocationEventHanndler context bound to object

Author: carterkozak

tritium-api/src/main/java/com/palantir/tritium/event/InvocationEventHandler.java

@@ -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.
@@ -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.
@@ -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);
 
 }

tritium-core/src/main/java/com/palantir/tritium/event/AbstractInvocationEventHandler.java

@@ -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);
 
@@ -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",
@@ -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());
     }

tritium-core/src/main/java/com/palantir/tritium/event/CompositeInvocationEventHandler.java

@@ -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) {
@@ -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) {
@@ -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) {
@@ -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 {
@@ -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 {
@@ -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(
@@ -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;
-        }
-    }
 }