diff --git a/otto/src/main/java/com/squareup/otto/AnnotatedHandlerFinder.java b/otto/src/main/java/com/squareup/otto/AnnotatedHandlerFinder.java index c7fb155..cf4c9d4 100644 --- a/otto/src/main/java/com/squareup/otto/AnnotatedHandlerFinder.java +++ b/otto/src/main/java/com/squareup/otto/AnnotatedHandlerFinder.java @@ -19,6 +19,8 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -49,74 +51,107 @@ private static void loadAnnotatedMethods(Class listenerClass) { Map, Set> subscriberMethods = new HashMap, Set>(); Map, Method> producerMethods = new HashMap, Method>(); - for (Method method : listenerClass.getDeclaredMethods()) { - // The compiler sometimes creates synthetic bridge methods as part of the - // type erasure process. As of JDK8 these methods now include the same - // annotations as the original declarations. They should be ignored for - // subscribe/produce. - if (method.isBridge()) { - continue; - } - if (method.isAnnotationPresent(Subscribe.class)) { - Class[] parameterTypes = method.getParameterTypes(); - if (parameterTypes.length != 1) { - throw new IllegalArgumentException("Method " + method + " has @Subscribe annotation but requires " - + parameterTypes.length + " arguments. Methods must require a single argument."); - } - - Class eventType = parameterTypes[0]; - if (eventType.isInterface()) { - throw new IllegalArgumentException("Method " + method + " has @Subscribe annotation on " + eventType - + " which is an interface. Subscription must be on a concrete class type."); - } - - if ((method.getModifiers() & Modifier.PUBLIC) == 0) { - throw new IllegalArgumentException("Method " + method + " has @Subscribe annotation on " + eventType - + " but is not 'public'."); - } - - Set methods = subscriberMethods.get(eventType); - if (methods == null) { - methods = new HashSet(); - subscriberMethods.put(eventType, methods); - } - methods.add(method); - } else if (method.isAnnotationPresent(Produce.class)) { - Class[] parameterTypes = method.getParameterTypes(); - if (parameterTypes.length != 0) { - throw new IllegalArgumentException("Method " + method + "has @Produce annotation but requires " - + parameterTypes.length + " arguments. Methods must require zero arguments."); - } - if (method.getReturnType() == Void.class) { - throw new IllegalArgumentException("Method " + method - + " has a return type of void. Must declare a non-void type."); + Class currentClass = listenerClass; + while (shouldCheck(currentClass)) { + for (Method method : currentClass.getDeclaredMethods()) { + // The compiler sometimes creates synthetic bridge methods as part of the + // type erasure process. As of JDK8 these methods now include the same + // annotations as the original declarations. They should be ignored for + // subscribe/produce. + if (method.isBridge()) { + continue; } - Class eventType = method.getReturnType(); - if (eventType.isInterface()) { - throw new IllegalArgumentException("Method " + method + " has @Produce annotation on " + eventType - + " which is an interface. Producers must return a concrete class type."); - } - if (eventType.equals(Void.TYPE)) { - throw new IllegalArgumentException("Method " + method + " has @Produce annotation but has no return type."); + boolean containsMethod = false; + for (Set methods : subscriberMethods.values()) { + if (containsMethod(method, methods)) { + containsMethod = true; + break; + } } - if ((method.getModifiers() & Modifier.PUBLIC) == 0) { - throw new IllegalArgumentException("Method " + method + " has @Produce annotation on " + eventType - + " but is not 'public'."); + //if this method has been overridden in a subclass, skip it + if (containsMethod || containsMethod(method, producerMethods.values())) { + continue; } - if (producerMethods.containsKey(eventType)) { - throw new IllegalArgumentException("Producer for type " + eventType + " has already been registered."); + if (method.isAnnotationPresent(Subscribe.class)) { + Class[] parameterTypes = method.getParameterTypes(); + if (parameterTypes.length != 1) { + throw new IllegalArgumentException("Method " + method + " has @Subscribe annotation but requires " + + parameterTypes.length + " arguments. Methods must require a single argument."); + } + + Class eventType = parameterTypes[0]; + if (eventType.isInterface()) { + throw new IllegalArgumentException("Method " + method + " has @Subscribe annotation on " + eventType + + " which is an interface. Subscription must be on a concrete class type."); + } + + if ((method.getModifiers() & Modifier.PUBLIC) == 0) { + throw new IllegalArgumentException("Method " + method + " has @Subscribe annotation on " + eventType + + " but is not 'public'."); + } + + Set methods = subscriberMethods.get(eventType); + if (methods == null) { + methods = new HashSet(); + subscriberMethods.put(eventType, methods); + } + methods.add(method); + } else if (method.isAnnotationPresent(Produce.class)) { + Class[] parameterTypes = method.getParameterTypes(); + if (parameterTypes.length != 0) { + throw new IllegalArgumentException("Method " + method + "has @Produce annotation but requires " + + parameterTypes.length + " arguments. Methods must require zero arguments."); + } + if (method.getReturnType() == Void.class) { + throw new IllegalArgumentException("Method " + method + + " has a return type of void. Must declare a non-void type."); + } + + Class eventType = method.getReturnType(); + if (eventType.isInterface()) { + throw new IllegalArgumentException("Method " + method + " has @Produce annotation on " + eventType + + " which is an interface. Producers must return a concrete class type."); + } + if (eventType.equals(Void.TYPE)) { + throw new IllegalArgumentException("Method " + method + " has @Produce annotation but has no return type."); + } + + if ((method.getModifiers() & Modifier.PUBLIC) == 0) { + throw new IllegalArgumentException("Method " + method + " has @Produce annotation on " + eventType + + " but is not 'public'."); + } + + if (producerMethods.containsKey(eventType)) { + throw new IllegalArgumentException("Producer for type " + eventType + " has already been registered."); + } + producerMethods.put(eventType, method); } - producerMethods.put(eventType, method); } + currentClass = currentClass.getSuperclass(); } PRODUCERS_CACHE.put(listenerClass, producerMethods); SUBSCRIBERS_CACHE.put(listenerClass, subscriberMethods); } + static boolean shouldCheck(Class clazz) { + String name = clazz.getName(); + return !name.startsWith("java.") && !name.startsWith("android."); + } + + static boolean containsMethod(Method check, Collection methods) { + for (Method method : methods) { + if (check.getName().equals(method.getName()) + && Arrays.equals(check.getParameterTypes(), method.getParameterTypes())) { + return true; + } + } + return false; + } + /** This implementation finds all methods marked with a {@link Produce} annotation. */ static Map, EventProducer> findAllProducers(Object listener) { final Class listenerClass = listener.getClass(); diff --git a/otto/src/test/java/com/squareup/otto/outside/AnnotatedHandlerFinderTest.java b/otto/src/test/java/com/squareup/otto/outside/AnnotatedHandlerFinderTest.java index 66194eb..3479e7b 100644 --- a/otto/src/test/java/com/squareup/otto/outside/AnnotatedHandlerFinderTest.java +++ b/otto/src/test/java/com/squareup/otto/outside/AnnotatedHandlerFinderTest.java @@ -57,10 +57,12 @@ H getHandler() { return handler; } + protected Bus bus; + @Before public void setUp() throws Exception { handler = createHandler(); - Bus bus = new Bus(ThreadEnforcer.ANY); + bus = new Bus(ThreadEnforcer.ANY); bus.register(handler); bus.post(EVENT); } @@ -139,6 +141,66 @@ public void overriddenAndAnnotatedInSubclass(Object o) { } } + public static class AnnotatedInSuperclassTest + extends AbstractEventBusTest { + + static class SuperClass { + final List annotatedInSuperclass = new ArrayList(); + final List overriddenInSubclass = new ArrayList(); + final List differentParametersInSubclass = new ArrayList(); + + @Subscribe + public void annotatedInSuperclass(Object o){ + annotatedInSuperclass.add(o); + } + + @Subscribe + public void overriddenInSubclass(Object o){ + overriddenInSubclass.add(o); + } + + @Subscribe + public void differentSignatureInSubclass(Object o){ + differentParametersInSubclass.add(o); + } + } + + static class SubClass extends SuperClass { + final List overriddenInSubclass2 = new ArrayList(); + final List differentParametersInSubclass2 = new ArrayList(); + + @Subscribe + public void overriddenInSubclass(Object o){ + overriddenInSubclass2.add(o); + } + + @Subscribe + public void differentSignatureInSubclass(String o){ + differentParametersInSubclass2.add(o); + } + } + + @Test public void annotatedInSuperclass() { + assertThat(getHandler().annotatedInSuperclass).containsExactly(EVENT); + } + + @Test public void overriddenInSubclass() { + assertThat(getHandler().overriddenInSubclass).isEmpty(); + assertThat(getHandler().overriddenInSubclass2).containsExactly(EVENT); + } + + @Test public void differentSignatureInSubclass() { + String stringEvent = "event"; + bus.post(stringEvent); + assertThat(getHandler().differentParametersInSubclass).containsExactly(EVENT, stringEvent); + assertThat(getHandler().differentParametersInSubclass2).containsExactly(stringEvent); + } + + @Override SubClass createHandler() { + return new SubClass(); + } + } + public static class NeitherAbstractNorAnnotatedInSuperclassTest extends AbstractEventBusTest { static class SuperClass { diff --git a/otto/src/test/java/com/squareup/otto/outside/AnnotatedProducerFinderTest.java b/otto/src/test/java/com/squareup/otto/outside/AnnotatedProducerFinderTest.java index 37b9295..71e8562 100644 --- a/otto/src/test/java/com/squareup/otto/outside/AnnotatedProducerFinderTest.java +++ b/otto/src/test/java/com/squareup/otto/outside/AnnotatedProducerFinderTest.java @@ -80,4 +80,24 @@ static class SimpleProducer { bus.register(new Subscriber()); assertThat(producer.produceCalled).isEqualTo(2); } + + + public static class ExtendedProducerTest { + + static class ExtendedProducer extends SimpleProducer { + } + + @Test public void subclassedProducer() { + Bus bus = new Bus(ThreadEnforcer.ANY); + Subscriber subscriber = new Subscriber(); + ExtendedProducer producer = new ExtendedProducer(); + + bus.register(producer); + assertThat(producer.produceCalled).isEqualTo(0); + bus.register(subscriber); + assertThat(producer.produceCalled).isEqualTo(1); + assertEquals(Arrays.asList(SimpleProducer.VALUE), subscriber.events); + } + } + }