Skip to content

Commit

Permalink
Improved the HandlerComparator to ensure it is transitive
Browse files Browse the repository at this point in the history
Depending on the order in which the methods were found, it was possible that a more generic handler could end up before a more specific one.

This fixes issue AxonFramework#248
  • Loading branch information
abuijze committed Jan 22, 2017
1 parent 88b150f commit a4d4e9d
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,20 @@ private static int compareHierarchy(Class<?> o1, Class<?> o2) {
} else if (o2.isAssignableFrom(o1)) {
return -1;
}
return 0;
return Integer.compare(depthOf(o2), depthOf(o1));
}

private static int depthOf(Class<?> o1) {
int depth = 0;
Class<?> type = o1;
while (type != null && !Object.class.equals(type)) {
depth++;
type = type.getSuperclass();
}
if (o1.isAnnotation()) {
depth += 1000;
}
return depth;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ private void testCollectionIsModifiedDuringIterationInRecursiveHierarchy(Supplie
public void testEventIsPublishedThroughoutHierarchy() throws Exception {
AggregateModel<SomeSubclass> inspector = ModelInspector.inspectAggregate(SomeSubclass.class);

CommandMessage<?> message = asCommandMessage("sub");
AtomicLong payload = new AtomicLong();

inspector.publish(new GenericEventMessage<>(payload), new SomeSubclass());
Expand All @@ -195,13 +194,6 @@ public void testExpectCommandToBeForwardedToEntity() throws Exception {
SomeSubclass target = new SomeSubclass();
MessageHandlingMember<? super SomeSubclass> handler = inspector.commandHandler(message.getCommandName());
assertEquals("1", handler.handle(message, target));

long startTime = System.currentTimeMillis();
for (int i = 0; i < 1_000_000; i++) {
inspector.commandHandler(message.getCommandName()).handle(message, target);
}
long endTime = System.currentTimeMillis();
System.out.println("Time taken: " + (endTime - startTime) + "ms");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,16 @@
import org.junit.Test;

import java.lang.annotation.Annotation;
import java.util.Comparator;
import java.util.Map;
import java.util.Optional;
import java.util.*;

import static org.junit.Assert.*;

public class HandlerComparatorTest {

private MessageHandlingMember stringHandler;
private MessageHandlingMember objectHandler;
private MessageHandlingMember longHandler;
private MessageHandlingMember numberHandler;
private MessageHandlingMember<?> stringHandler;
private MessageHandlingMember<?> objectHandler;
private MessageHandlingMember<?> longHandler;
private MessageHandlingMember<?> numberHandler;

private Comparator<MessageHandlingMember<?>> testSubject;

Expand Down Expand Up @@ -75,6 +73,15 @@ public void testHandlersIsEqualWithItself() throws Exception {
assertNotEquals(0, testSubject.compare(objectHandler, numberHandler));
}

@Test
public void testHandlersSortedCorrectly() {
List<MessageHandlingMember<?>> members = new ArrayList<>(Arrays.asList(objectHandler, numberHandler, stringHandler, longHandler));

members.sort(this.testSubject);
assertTrue(members.indexOf(longHandler) < members.indexOf(numberHandler));
assertEquals(3, members.indexOf(objectHandler));
}

@Test
public void testNotInSameHierarchyUsesPriorityBasedEvaluation() throws Exception {
assertTrue("Number should appear before String based on priority", testSubject.compare(numberHandler, stringHandler) < 0);
Expand Down

0 comments on commit a4d4e9d

Please sign in to comment.