Add DeadEvent #9

Merged
kske merged 2 commits from f/dead-event into develop 2021-02-21 09:16:33 +01:00
5 changed files with 115 additions and 17 deletions

View File

@ -0,0 +1,37 @@
package dev.kske.eventbus.core;
/**
* Wraps an event that was dispatched but for which no handler has been bound.
* <p>
* Handling dead events is useful as it can identify a poorly configured event distribution.
*
* @author Kai S. K. Engelbart
* @since 1.1.0
*/
public final class DeadEvent {
private final EventBus eventBus;
private final Object event;
DeadEvent(EventBus eventBus, Object event) {
this.eventBus = eventBus;
this.event = event;
}
@Override
public String toString() {
return String.format("DeadEvent[eventBus=%s, event=%s]", eventBus, event);
}
/**
* @return the event bus that dispatched this event
delvh marked this conversation as resolved Outdated
Outdated
Review
 *  that dispatched this event
```java * … that dispatched this event ```
Outdated
Review

I have ammended my original commit.

I have ammended my original commit.
* @since 1.1.0
*/
public EventBus getEventBus() { return eventBus; }
/**
* @return the event that could not be delivered
* @since 1.1.0
*/
public Object getEvent() { return event; }
}

View File

@ -73,14 +73,25 @@ public final class EventBus {
var state = dispatchState.get();
state.isDispatching = true;
for (var handler : getHandlersFor(event.getClass()))
if (state.isCancelled) {
logger.log(Level.INFO, "Cancelled dispatching event {0}", event);
state.isCancelled = false;
break;
} else {
handler.execute(event);
}
Iterator<EventHandler> handlers = getHandlersFor(event.getClass());
if (handlers.hasNext()) {
kske marked this conversation as resolved
Review

that if should be useless as the

while() {}

loop also checks in the first iteration,
which is exactly the difference to the

do{}while()

loop.

that **if** should be useless as the ```java while() {} ``` loop also checks in the first iteration, which is exactly the difference to the ```java do{}while() ``` loop.
Review

I need the else (if) case here as well, but unfortunately this is not Python.

I need the `else (if)` case here as well, but unfortunately this is not Python.
while (handlers.hasNext())
if (state.isCancelled) {
logger.log(Level.INFO, "Cancelled dispatching event {0}", event);
state.isCancelled = false;
break;
} else {
handlers.next().execute(event);
}
} else if (!(event instanceof DeadEvent)) {
kske marked this conversation as resolved
Review

this would be a little bit different then.

this would be a little bit different then.
Review

That's why I use the if statement. I could use a boolean flag instead, but the iterator logic should be more efficient and arguably more readable here.

That's why I use the if statement. I could use a boolean flag instead, but the iterator logic should be more efficient and arguably more readable here.
// Dispatch dead event
dispatch(new DeadEvent(this, event));
} else {
// Warn about the dead event not being handled
logger.log(Level.WARNING, "{0} not handled", event);
}
// Reset dispatch state
state.isDispatching = false;
@ -89,25 +100,26 @@ public final class EventBus {
}
/**
* Searches for the event handlers bound to an event class.
* Searches for the event handlers bound to an event class. This includes polymorphic handlers
* that are bound to a supertype of the event class.
*
* @param eventClass the event class to use for the search
* @return all event handlers registered for the event class
* @return an iterator over the applicable handlers in descending order of priority
* @since 0.0.1
*/
private List<EventHandler> getHandlersFor(Class<?> eventClass) {
private Iterator<EventHandler> getHandlersFor(Class<?> eventClass) {
// Get handlers defined for the event class
Set<EventHandler> handlers = bindings.getOrDefault(eventClass, new TreeSet<>());
TreeSet<EventHandler> handlers = bindings.getOrDefault(eventClass, new TreeSet<>());
delvh marked this conversation as resolved
Review

Why do you need that now?

Why do you need that now?
Review

I don't, but why unnecessarily obfuscate the type? The fact that this is a tree set is important here as it guarantees the event handler order, so the variable type might as well reflect this.

I don't, but why unnecessarily obfuscate the type? The fact that this is a tree set is important here as it guarantees the event handler order, so the variable type might as well reflect this.
// Get subtype handlers
// Get polymorphic handlers
for (var binding : bindings.entrySet())
if (binding.getKey().isAssignableFrom(eventClass))
for (var handler : binding.getValue())
if (handler.isPolymorphic())
handlers.add(handler);
return new ArrayList<>(handlers);
return handlers.iterator();
}
/**

View File

@ -68,7 +68,7 @@ final class EventHandler implements Comparable<EventHandler> {
* Compares this to another event handler based on priority. In case of equal priority a
* non-zero value based on hash codes is returned.
* <p>
* This is used to retrieve event handlers in order of descending priority from a tree set.
* This is used to retrieve event handlers in descending order of priority from a tree set.
*
* @since 0.0.1
*/

View File

@ -0,0 +1,49 @@
package dev.kske.eventbus.core;
import static org.junit.jupiter.api.Assertions.*;
import org.junit.jupiter.api.Test;
/**
* Tests the dispatching of a dead event if an event could not be delivered.
*
* @author Kai S. K. Engelbart
* @since 1.1.0
*/
class DeadTest {
EventBus bus = new EventBus();
String event = "This event has no handler";
boolean deadEventHandled;
/**
* Tests dead event delivery.
*
* @since 1.1.0
*/
@Test
void testDeadEvent() {
bus.registerListener(this);
bus.dispatch(event);
assertTrue(deadEventHandled);
bus.removeListener(this);
}
/**
* Tests how the event bus reacts to an unhandled dead event. This should not lead to an
* exception or endless recursion and instead be logged.
*
* @since 1.1.0
*/
@Test
void testUnhandledDeadEvent() {
bus.dispatch(event);
}
@Event
void onDeadEvent(DeadEvent deadEvent) {
assertEquals(bus, deadEvent.getEventBus());
assertEquals(event, deadEvent.getEvent());
deadEventHandled = true;
}
}

View File

@ -27,8 +27,8 @@ class DispatchTest {
}
/**
* Tests {@link EventBus#dispatch(Object)} with multiple handler priorities, a subtype handler
* and a static handler.
* Tests {@link EventBus#dispatch(Object)} with multiple handler priorities, a polymorphic
* handler and a static handler.
*
* @since 0.0.1
*/