Inherit Event Handlers #34
@ -221,6 +221,15 @@ The same applies when an exception event handler throws an exception.
|
||||
|
||||
To avoid this, system events never cause system events and instead just issue a warning to the logger.
|
||||
|
||||
## Inheritance
|
||||
|
||||
When a superclass or an interface of an event listener defines event handlers, they will be detected and registered by Event Bus, even if they are `private`.
|
||||
If an event handler is overridden by the listener, the `@Event` annotation of the overridden method is automatically considered present on the overriding method.
|
||||
delvh marked this conversation as resolved
|
||||
If the overridden method already contains an implementation in the superclass, the superclass implementation is ignored as expected.
|
||||
kske marked this conversation as resolved
Outdated
delvh
commented
If the overridden method already contains an implementation in the superclass, the superclass implementation is ignored as expected. If the overridden method already contains an implementation in the superclass, the superclass implementation is ignored as expected.
delvh
commented
Or do I understand that wrong? Or do I understand that wrong?
kske
commented
You understood that correctly. There is a difference between the overridden method and the overriding method. One is in the superclass, the other in the subclass. You understood that correctly. There is a difference between the overridden method and the overriding method. One is in the superclass, the other in the subclass.
delvh
commented
The topmost comment was intended as a suggestion for the README. The topmost comment was intended as a suggestion for the README.
|
||||
|
||||
The `@Priority` and `@Polymorphic` annotations are inherited both on a class and on a method level.
|
||||
If the priority or polymorphism has to be redefined on an inherited handler, the `@Event` annotation has to be added explicitly.
|
||||
|
||||
## Debugging
|
||||
|
||||
In more complex setups, taking a look at the event handler execution order can be helpful for debugging.
|
||||
|
@ -2,7 +2,8 @@ package dev.kske.eventbus.core;
|
||||
|
||||
import java.lang.System.Logger;
|
||||
import java.lang.System.Logger.Level;
|
||||
import java.lang.reflect.InvocationTargetException;
|
||||
import java.lang.annotation.Annotation;
|
||||
import java.lang.reflect.*;
|
||||
import java.util.*;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.function.Consumer;
|
||||
@ -14,9 +15,8 @@ import dev.kske.eventbus.core.handler.*;
|
||||
* <p>
|
||||
* A singleton instance of this class can be lazily created and acquired using the
|
||||
* {@link EventBus#getInstance()} method.
|
||||
* <p>
|
||||
* This is a thread-safe implementation.
|
||||
*
|
||||
* @implNote This is a thread-safe implementation.
|
||||
* @author Kai S. K. Engelbart
|
||||
* @since 0.0.1
|
||||
* @see Event
|
||||
@ -237,7 +237,7 @@ public final class EventBus {
|
||||
priority = listener.getClass().getAnnotation(Priority.class).value();
|
||||
|
||||
registeredListeners.add(listener);
|
||||
for (var method : listener.getClass().getDeclaredMethods()) {
|
||||
for (var method : getHandlerMethods(listener.getClass())) {
|
||||
Event annotation = method.getAnnotation(Event.class);
|
||||
|
||||
// Skip methods without annotations
|
||||
@ -257,6 +257,49 @@ public final class EventBus {
|
||||
listener.getClass().getName());
|
||||
}
|
||||
|
||||
/**
|
||||
* Searches for event handling methods declared inside the inheritance hierarchy of an event
|
||||
* listener.
|
||||
*
|
||||
* @param listenerClass the class to inspect
|
||||
* @return all event handling methods defined for the given listener
|
||||
* @since 1.3.0
|
||||
*/
|
||||
private Set<Method> getHandlerMethods(Class<?> listenerClass) {
|
||||
|
||||
// Get methods declared by the listener
|
||||
Set<Method> methods = getMethodsAnnotatedWith(listenerClass, Event.class);
|
||||
|
||||
// Recursively add superclass handlers
|
||||
Class<?> superClass = listenerClass.getSuperclass();
|
||||
if (superClass != null && superClass != Object.class)
|
||||
kske marked this conversation as resolved
Outdated
delvh
commented
```java
Class<?> superClass = listenerClass.getSuperclass();
if (superClass != null && superClass != Object.class)
methods.addAll(getHandlerMethods(superClass));
```
|
||||
methods.addAll(getHandlerMethods(superClass));
|
||||
|
||||
// Recursively add interface handlers
|
||||
for (Class<?> iClass : listenerClass.getInterfaces())
|
||||
methods.addAll(getHandlerMethods(iClass));
|
||||
|
||||
return methods;
|
||||
}
|
||||
|
||||
/**
|
||||
* Searches for declared methods with a specific annotation inside a class.
|
||||
*
|
||||
* @param enclosingClass the class to inspect
|
||||
* @param annotationClass the annotation to look for
|
||||
* @return all methods matching the search criteria
|
||||
* @since 1.3.0
|
||||
*/
|
||||
private Set<Method> getMethodsAnnotatedWith(Class<?> enclosingClass,
|
||||
Class<? extends Annotation> annotationClass) {
|
||||
var methods = new HashSet<Method>();
|
||||
for (var method : enclosingClass.getDeclaredMethods())
|
||||
if (method.isAnnotationPresent(annotationClass))
|
||||
kske marked this conversation as resolved
Outdated
delvh
commented
```java
var set = new HashSet<Method>();
for(Method method : enclosingClass.getDeclaredMethods())
if(method.isAnnotationPresent(annotationClass))
set.add(method);
return set;
```
|
||||
methods.add(method);
|
||||
|
||||
return methods;
|
||||
}
|
||||
|
||||
/**
|
||||
* Registers a callback listener, which is a consumer that is invoked when an event occurs. The
|
||||
* listener is not polymorphic and has the {@link #DEFAULT_PRIORITY}.
|
||||
|
@ -18,6 +18,7 @@ import java.lang.annotation.*;
|
||||
* @see Event
|
||||
*/
|
||||
@Documented
|
||||
@Inherited
|
||||
@Retention(RUNTIME)
|
||||
@Target({ METHOD, TYPE })
|
||||
public @interface Polymorphic {
|
||||
|
@ -21,6 +21,7 @@ import java.lang.annotation.*;
|
||||
* @see Event
|
||||
*/
|
||||
@Documented
|
||||
@Inherited
|
||||
@Retention(RUNTIME)
|
||||
@Target({ METHOD, TYPE })
|
||||
public @interface Priority {
|
||||
|
@ -0,0 +1,42 @@
|
||||
package dev.kske.eventbus.core;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertSame;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
/**
|
||||
* Tests whether event handlers correctly work in the context of an inheritance hierarchy. The
|
||||
* effect of handler priorities is also accounted for.
|
||||
*
|
||||
* @author Kai S. K. Engelbart
|
||||
* @since 1.3.0
|
||||
*/
|
||||
class InheritanceTest extends SimpleEventListenerBase implements SimpleEventListenerInterface {
|
||||
|
||||
EventBus bus = new EventBus();
|
||||
|
||||
@Test
|
||||
void test() {
|
||||
bus.registerListener(this);
|
||||
var event = new SimpleEvent();
|
||||
|
||||
bus.dispatch(event);
|
||||
assertSame(3, event.getCounter());
|
||||
}
|
||||
|
||||
@Override
|
||||
void onSimpleEventAbstractHandler(SimpleEvent event) {
|
||||
assertSame(1, event.getCounter());
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onSimpleEventInterfaceHandler(SimpleEvent event) {
|
||||
event.increment();
|
||||
}
|
||||
|
||||
kske marked this conversation as resolved
delvh
commented
If you now even use priorities you can test whether the priority is always correct. If you now even use priorities you can test whether the priority is always correct.
Also, I think it would be good to explicitly override one of the superclass methods **not** to do anything.
|
||||
@Event
|
||||
private void onSimpleEventPrivate(SimpleEvent event) {
|
||||
assertSame(0, event.getCounter());
|
||||
event.increment();
|
||||
}
|
||||
}
|
@ -1,9 +1,31 @@
|
||||
package dev.kske.eventbus.core;
|
||||
|
||||
/**
|
||||
* A simple event for testing purposes.
|
||||
* A simple event for testing purposes. The event contains a counter that is supposed to be
|
||||
* incremented when the event is processed by a handler. That way it is possible to test whether all
|
||||
* handlers that were supposed to be invoked were in fact invoked.
|
||||
*
|
||||
* @author Kai S. K. Engelbart
|
||||
* @since 0.0.1
|
||||
*/
|
||||
class SimpleEvent {}
|
||||
class SimpleEvent {
|
||||
|
||||
private int counter;
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return String.format("SimpleEvent[%d]", counter);
|
||||
}
|
||||
|
||||
void increment() {
|
||||
++counter;
|
||||
}
|
||||
|
||||
int getCounter() {
|
||||
return counter;
|
||||
}
|
||||
|
||||
void reset() {
|
||||
counter = 0;
|
||||
}
|
||||
}
|
||||
|
@ -0,0 +1,25 @@
|
||||
package dev.kske.eventbus.core;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
|
||||
/**
|
||||
* An abstract class defining a package-private and a private handler for {@link SimpleEvent}.
|
||||
*
|
||||
* @author Kai S. K. Engelbart
|
||||
* @since 1.3.0
|
||||
*/
|
||||
@Priority(200)
|
||||
abstract class SimpleEventListenerBase {
|
||||
|
||||
@Event
|
||||
void onSimpleEventAbstractHandler(SimpleEvent event) {
|
||||
fail("This handler should not be invoked");
|
||||
}
|
||||
|
||||
@Priority(150)
|
||||
@Event
|
||||
private void onSimpleEventPrivate(SimpleEvent event) {
|
||||
assertSame(1, event.getCounter());
|
||||
event.increment();
|
||||
}
|
||||
}
|
@ -0,0 +1,14 @@
|
||||
package dev.kske.eventbus.core;
|
||||
|
||||
/**
|
||||
* An interface defining a single handler for {@link SimpleEvent}.
|
||||
*
|
||||
* @author Kai S. K. Engelbart
|
||||
* @since 1.3.0
|
||||
*/
|
||||
interface SimpleEventListenerInterface {
|
||||
|
||||
@Priority(120)
|
||||
@Event
|
||||
delvh marked this conversation as resolved
delvh
commented
Will an interface-private method annotated with Will an interface-private method annotated with `@Event` be registered?
Or should we explicitly disallow that?
kske
commented
There is no reason why it shouldn't be. There is no reason why it shouldn't be.
delvh
commented
Yes, and that's exactly what I find so scary. Yes, and that's exactly what I find so scary.
In a class, private methods are expected.
In an interface however, no one suspects that there is a private method that is responsible for changing the state.
kske
commented
Well, that would be a very rare case, as the event handler would only work when some class implements the interface and registers itself as an event listener. If such a situation actually arises, it should be made clear how that interface is supposed to be used. Well, that would be a very rare case, as the event handler would only work when some class implements the interface and registers itself as an event listener. If such a situation actually arises, it should be made clear how that interface is supposed to be used.
|
||||
void onSimpleEventInterfaceHandler(SimpleEvent event);
|
||||
}
|
Perhaps a new annotation
@ExcludeListener
should be added that instructs EventBus to ignore this method if present. This would allow to override behavior of superclasses that is in some rare cases counter-productive.(But if at all, that is beyond the scope of this PR)
That would be rather difficult to implement given the edge cases. If such a need arises, I will try.