Explicit EventHandler creation avoiding reflection

library/pom.xml

@@ -20,7 +20,7 @@
   <parent>
     <groupId>com.squareup</groupId>
     <artifactId>otto-parent</artifactId>
-    <version>1.3.5-SNAPSHOT</version>
+    <version>1.3.6-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

library/src/main/java/com/squareup/otto/Bus.java

@@ -239,6 +239,27 @@ public void register(Object object) {
     }
   }
 
+  /**
+   * Registers a new EventHandler for events of the provided type that will execute
+   * the provided callback whenever it is posted.
+   *
+   * @param type event class to subscribe to.
+   * @param callback Callback instance that will be executed when an event of the
+   *                 provided type is posted.
+   */
+  public void register(Class type, Callback callback) {
+    Set<EventHandler> handlers = handlersByType.get(type);
+    if (handlers == null) {
+      //concurrent put if absent
+      Set<EventHandler> handlersCreation = new CopyOnWriteArraySet<EventHandler>();
+      handlers = handlersByType.putIfAbsent(type, handlersCreation);
+      if (handlers == null) {
+        handlers = handlersCreation;
+      }
+    }
+    handlers.add(new EventHandler(callback));
+  }
+
   private void dispatchProducerResultToHandler(EventHandler handler, EventProducer producer) {
     Object event = null;
     try {
@@ -299,6 +320,31 @@ public void unregister(Object object) {
     }
   }
 
+  /**
+   * Unregisters a Callback for a particular event type.
+   *
+   * @param type event class to unsubscribe from.
+   * @param callback Callback instance that would be executed when an event of the
+   *                 provided type was posted.
+   */
+  public void unregister(Class type, Callback callback) {
+    Set<EventHandler> currentHandlers = getHandlersForEventType(type);
+    if (currentHandlers == null) {
+      throw new IllegalArgumentException("There are no EventHandlers for type " + type);
+    }
+    EventHandler eventHandler = null;
+    for (EventHandler candidate : currentHandlers) {
+      if (candidate.hasCallback(callback)) {
+        eventHandler = candidate;
+      }
+    }
+    if (eventHandler == null) {
+      throw new IllegalArgumentException("There is no EventHandler for type " + type + " with callback " + callback);
+    }
+    eventHandler.invalidate();
+    currentHandlers.remove(eventHandler);
+  }
+
   /**
    * Posts an event to all registered handlers.  This method will return successfully after the event has been posted to
    * all handlers, and regardless of any exceptions thrown by handlers.

library/src/main/java/com/squareup/otto/Callback.java

@@ -0,0 +1,19 @@
+package com.squareup.otto;
+
+import java.lang.reflect.InvocationTargetException;
+
+/**
+ * <p>This interface represents something that an EventHandler can call whenever it is posted an Event.</p>
+ *
+ * @author Guillermo Gutierrez
+ */
+public interface Callback<T> {
+    /**
+     * <p>This method will be called whenever an Event is posted to the EventHandler that has an implementation of this
+     * interface.</p>
+     *
+     * @param event event object that has been posted
+     * @throws InvocationTargetException
+     */
+    void call(T event) throws InvocationTargetException;
+}

library/src/main/java/com/squareup/otto/EventHandler.java

@@ -25,18 +25,16 @@
  *
  * <p>This class only verifies the suitability of the method and event type if something fails.  Callers are expected t
  * verify their uses of this class.
- *
+ * <p/>
  * <p>Two EventHandlers are equivalent when they refer to the same method on the same object (not class).   This
  * property is used to ensure that no handler method is registered more than once.
  *
  * @author Cliff Biffle
  */
 class EventHandler {
 
-  /** Object sporting the handler method. */
-  private final Object target;
-  /** Handler method. */
-  private final Method method;
+  /** Handler Callback. */
+  private final Callback callback;
   /** Object hash code. */
   private final int hashCode;
   /** Should this handler receive events? */
@@ -50,14 +48,23 @@ class EventHandler {
       throw new NullPointerException("EventHandler method cannot be null.");
     }
 
-    this.target = target;
-    this.method = method;
-    method.setAccessible(true);
+    // Compute hash code eagerly since we know it will be used frequently and we cannot estimate the runtime of the
+    // target's hashCode call.
+    final int prime = 31;
+    this.callback = new MethodInvokingCallback(target, method);
+    this.hashCode = (prime + callback.hashCode()) * prime + target.hashCode();
+  }
+
+  EventHandler(final Callback callback) {
+    if (callback == null) {
+      throw new NullPointerException("EventHandler callback cannot be null.");
+    }
 
     // Compute hash code eagerly since we know it will be used frequently and we cannot estimate the runtime of the
     // target's hashCode call.
     final int prime = 31;
-    hashCode = (prime + method.hashCode()) * prime + target.hashCode();
+    this.callback = callback;
+    this.hashCode = (prime + callback.hashCode()) * prime;
   }
 
   public boolean isValid() {
@@ -74,31 +81,24 @@ public void invalidate() {
   }
 
   /**
-   * Invokes the wrapped handler method to handle {@code event}.
+   * Invokes the wrapped handler callback to handle {@code event}.
    *
-   * @param event  event to handle
-   * @throws java.lang.IllegalStateException  if previously invalidated.
-   * @throws java.lang.reflect.InvocationTargetException  if the wrapped method throws any {@link Throwable} that is not
-   *     an {@link Error} ({@code Error}s are propagated as-is).
+   * @param event event to handle
+   * @throws java.lang.reflect.InvocationTargetException if the wrapped callback throws any
+   *                                                     {@link Throwable} that is not
+   *                                                     an {@link Error} ({@code Error}s
+   *                                                     are propagated as-is).
    */
+  @SuppressWarnings("unchecked")
   public void handleEvent(Object event) throws InvocationTargetException {
     if (!valid) {
       throw new IllegalStateException(toString() + " has been invalidated and can no longer handle events.");
     }
-    try {
-      method.invoke(target, event);
-    } catch (IllegalAccessException e) {
-      throw new AssertionError(e);
-    } catch (InvocationTargetException e) {
-      if (e.getCause() instanceof Error) {
-        throw (Error) e.getCause();
-      }
-      throw e;
-    }
+    callback.call(event);
   }
 
   @Override public String toString() {
-    return "[EventHandler " + method + "]";
+    return "[EventHandler " + callback.toString() + "]";
   }
 
   @Override public int hashCode() {
@@ -120,7 +120,11 @@ public void handleEvent(Object event) throws InvocationTargetException {
 
     final EventHandler other = (EventHandler) obj;
 
-    return method.equals(other.method) && target == other.target;
+    return callback.equals(other.callback);
   }
 
+
+  public boolean hasCallback(Callback callback) {
+    return this.callback.equals(callback);
+  }
 }

library/src/main/java/com/squareup/otto/MethodInvokingCallback.java

@@ -0,0 +1,85 @@
+package com.squareup.otto;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+/**
+ * <p>Extracted from EventHandler's original implementation</p>
+ * <p>Wraps a Method in a Callback interface in order to be called whenever the EventHandler is posted an Event</p>
+ * <p>This class only verifies the suitability of the method and event type if something fails. Callers are expected to
+ * verify their uses of this class.</p>
+ * <p>Two MethodInvokingCallback are equivalent when they refer to the same method on the same object (not class).
+ * This property is used to ensure that no handler method is registered more than once.</p>
+ *
+ * @author Guillermo Gutierrez
+ */
+public class MethodInvokingCallback implements Callback {
+  private final Object target;
+  private final Method method;
+  private final int hashCode;
+
+  public MethodInvokingCallback(Object target, Method method) {
+    method.setAccessible(true);
+    this.target = target;
+    this.method = method;
+    // Compute hash code eagerly since we know it will be used frequently and we cannot estimate the runtime of the
+    // target's hashCode call.
+    final int prime = 31;
+    hashCode = (prime + method.hashCode()) * prime + target.hashCode();
+  }
+
+  /**
+   * <p>This method will be called whenever an Event is posted to the EventHandler that has an implementation of this
+   * interface.</p>
+   *
+   * @param event event to handle
+   * @throws java.lang.reflect.InvocationTargetException if the wrapped method throws any {@link Throwable} that is not
+   *                                                     an {@link Error} ({@code Error}s are propagated as-is).
+   */
+  @Override
+  public void call(Object event) throws InvocationTargetException {
+    try {
+      method.invoke(target, event);
+    } catch (IllegalAccessException e) {
+      throw new AssertionError(e);
+    } catch (InvocationTargetException e) {
+      if (e.getCause() instanceof Error) {
+        throw (Error) e.getCause();
+      }
+      throw e;
+    }
+  }
+
+  public Method getMethod() {
+    return method;
+  }
+
+  @Override
+  public String toString() {
+    return method.toString();
+  }
+
+  @Override
+  public int hashCode() {
+    return hashCode;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) {
+      return true;
+    }
+
+    if (obj == null) {
+      return false;
+    }
+
+    if (getClass() != obj.getClass()) {
+      return false;
+    }
+
+    final MethodInvokingCallback other = (MethodInvokingCallback) obj;
+
+    return method.equals(other.getMethod()) && target == other.target;
+  }
+}

library/src/test/java/com/squareup/otto/BusTest.java

@@ -19,6 +19,7 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -305,6 +306,66 @@ class VoidProducer {
                  expectedEvents, catcher2.getEvents());
   }
 
+  @Test public void unregisterCallback() {
+    final StringCatcher catcher1 = new StringCatcher();
+    final StringCatcher catcher2 = new StringCatcher();
+    Callback catcher1Callback = new Callback<String>() {
+      @Override
+      public void call(String event) throws InvocationTargetException {
+        catcher1.hereHaveAString(event);
+      }
+    };
+    Callback catcher2Callback = new Callback<String>() {
+      @Override
+      public void call(String event) throws InvocationTargetException {
+        catcher2.hereHaveAString(event);
+      }
+    };
+    try {
+      bus.unregister(String.class, catcher1Callback);
+      fail("Attempting to unregister an unregistered object succeeded");
+    } catch (IllegalArgumentException expected) {
+      // OK.
+    }
+
+    bus.register(String.class, catcher1Callback);
+    bus.post(EVENT);
+    bus.register(String.class, catcher2Callback);
+    bus.post(EVENT);
+
+    List<String> expectedEvents = new ArrayList<String>();
+    expectedEvents.add(EVENT);
+    expectedEvents.add(EVENT);
+
+    assertEquals("Two correct events should be delivered.",
+        expectedEvents, catcher1.getEvents());
+
+    assertEquals("One correct event should be delivered.",
+        Arrays.asList(EVENT), catcher2.getEvents());
+
+    bus.unregister(String.class, catcher1Callback);
+    bus.post(EVENT);
+
+    assertEquals("Shouldn't catch any more events when unregistered.",
+        expectedEvents, catcher1.getEvents());
+    assertEquals("Two correct events should be delivered.",
+        expectedEvents, catcher2.getEvents());
+
+    try {
+      bus.unregister(String.class, catcher1Callback);
+      fail("Attempting to unregister an unregistered object succeeded");
+    } catch (IllegalArgumentException expected) {
+      // OK.
+    }
+
+    bus.unregister(String.class, catcher2Callback);
+    bus.post(EVENT);
+    assertEquals("Shouldn't catch any more events when unregistered.",
+        expectedEvents, catcher1.getEvents());
+    assertEquals("Shouldn't catch any more events when unregistered.",
+        expectedEvents, catcher2.getEvents());
+  }
+
   @Test public void producingNullIsInvalid() {
     try {
       bus.register(new NullProducer());

library/src/test/java/com/squareup/otto/EventHandlerTest.java

@@ -65,6 +65,16 @@ public class EventHandlerTest {
     }
   }
 
+  /** Checks that EventHandler's constructor disallows null callbacks. */
+  @Test public void rejectionOfNullCallbacks() {
+    try {
+      new EventHandler(null);
+      fail("EventHandler must immediately reject null methods.");
+    } catch (NullPointerException expected) {
+      // Hooray!
+    }
+  }
+
   /** Checks that EventHandler's constructor disallows null targets. */
   @Test public void rejectionOfNullTargets() throws NoSuchMethodException {
     Method method = getRecordingMethod();

pom.xml

@@ -25,7 +25,7 @@
 
   <groupId>com.squareup</groupId>
   <artifactId>otto-parent</artifactId>
-  <version>1.3.5-SNAPSHOT</version>
+  <version>1.3.6-SNAPSHOT</version>
   <packaging>pom</packaging>
 
   <name>Otto (Parent)</name>

sample/pom.xml

@@ -20,7 +20,7 @@
   <parent>
     <groupId>com.squareup</groupId>
     <artifactId>otto-parent</artifactId>
-    <version>1.3.5-SNAPSHOT</version>
+    <version>1.3.6-SNAPSHOT</version>
     <relativePath>../pom.xml</relativePath>
   </parent>