diff --git a/src/main/java/net/floodlightcontroller/core/util/ListenerDispatcher.java b/src/main/java/net/floodlightcontroller/core/util/ListenerDispatcher.java
index 70c80ff91801fcf82f26197725188c615bea59c3..aa675d220558af44cbe8dddfe9904efc08c3ad1e 100644
--- a/src/main/java/net/floodlightcontroller/core/util/ListenerDispatcher.java
+++ b/src/main/java/net/floodlightcontroller/core/util/ListenerDispatcher.java
@@ -18,8 +18,6 @@
package net.floodlightcontroller.core.util;
import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
@@ -38,6 +36,25 @@ public class ListenerDispatcher<U, T extends IListener<U>> {
protected static Logger logger = LoggerFactory.getLogger(ListenerDispatcher.class);
List<T> listeners = null;
+ private void visit(List<T> newlisteners, U type, HashSet<T> visited,
+ List<T> ordering, T listener) {
+ if (!visited.contains(listener)) {
+ visited.add(listener);
+
+ for (T i : newlisteners) {
+ if (ispre(type, i, listener)) {
+ visit(newlisteners, type, visited, ordering, i);
+ }
+ }
+ ordering.add(listener);
+ }
+ }
+
+ private boolean ispre(U type, T l1, T l2) {
+ return (l2.isCallbackOrderingPrereq(type, l1.getName()) ||
+ l1.isCallbackOrderingPostreq(type, l2.getName()));
+ }
+
/**
* Add a listener to the list of listeners
* @param listener
@@ -48,31 +65,36 @@ public class ListenerDispatcher<U, T extends IListener<U>> {
newlisteners.addAll(listeners);
newlisteners.add(listener);
-
- // compute transitive closure of dependencies
- HashSet<String> deps = new HashSet<String>();
- for (T k : newlisteners) {
- for (T i : newlisteners) {
- for (T j : newlisteners) {
- boolean ispre =
- (i.isCallbackOrderingPrereq(type, j.getName()) ||
- j.isCallbackOrderingPostreq(type, i.getName()));
- if (ispre ||
- (deps.contains(i.getName() + "|||" + k.getName()) &&
- deps.contains(k.getName() + "|||" + j.getName()))) {
- deps.add(i.getName() + "|||" + j.getName());
- // Check for dependency cycle
- if (deps.contains(j.getName() + "|||" + i.getName())) {
- logger.error("Cross dependency cycle between {} and {}",
- i.getName(), j.getName());
- }
- }
+ // Find nodes without outgoing edges
+ List<T> terminals = new ArrayList<T>();
+ for (T i : newlisteners) {
+ boolean isterm = true;
+ for (T j : newlisteners) {
+ if (ispre(type, i, j)) {
+ isterm = false;
+ break;
}
}
+ if (isterm) {
+ terminals.add(i);
+ }
}
- Collections.sort(newlisteners, new ListenerComparator(deps));
- listeners = newlisteners;
+ if (terminals.size() == 0) {
+ logger.error("No listener dependency solution: " +
+ "No listeners without incoming dependencies");
+ listeners = newlisteners;
+ return;
+ }
+
+ // visit depth-first traversing in the opposite order from
+ // the dependencies. Note we will not generally detect cycles
+ HashSet<T> visited = new HashSet<T>();
+ List<T> ordering = new ArrayList<T>();
+ for (T term : terminals) {
+ visit(newlisteners, type, visited, ordering, term);
+ }
+ listeners = ordering;
}
/**
@@ -102,27 +124,4 @@ public class ListenerDispatcher<U, T extends IListener<U>> {
public List<T> getOrderedListeners() {
return listeners;
}
-
- /**
- * Comparator for listeners to use with computed transitive dependencies
- *
- * @author readams
- *
- */
- protected class ListenerComparator implements Comparator<IListener<U>> {
- HashSet<String> deps;
-
- ListenerComparator(HashSet<String> deps) {
- this.deps = deps;
- }
-
- @Override
- public int compare(IListener<U> arg0, IListener<U> arg1) {
- if (deps.contains(arg0.getName() + "|||" + arg1.getName()))
- return 1;
- else if (deps.contains(arg1.getName() + "|||" + arg0.getName()))
- return -1;
- return 0; // strictly partial ordering
- }
- }
}
diff --git a/src/test/java/net/floodlightcontroller/core/util/MessageDispatcherTest.java b/src/test/java/net/floodlightcontroller/core/util/MessageDispatcherTest.java
index a253245a031a6a4f1ed679d6bb9ab2d47665d35d..0829aa447f4d211b0e207c45a0527b7a825906b9 100644
--- a/src/test/java/net/floodlightcontroller/core/util/MessageDispatcherTest.java
+++ b/src/test/java/net/floodlightcontroller/core/util/MessageDispatcherTest.java
@@ -22,6 +22,10 @@ import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+
import org.junit.Test;
import org.openflow.protocol.OFType;
@@ -30,57 +34,120 @@ import net.floodlightcontroller.test.FloodlightTestCase;
public class MessageDispatcherTest extends FloodlightTestCase {
- /**
- * Verify that our callbacks are ordered with respect to the order specified
- * @throws Exception
- */
- @Test
- public void testCallbackOrderingBase() throws Exception {
- testCallbackOrdering(new String[] {"topology"}, new String[] {"topology"});
- testCallbackOrdering(new String[] {"learningswitch","topology"}, new String[] {"learningswitch","topology"});
- testCallbackOrdering(new String[] {"topology","devicemanager","learningswitch"}, new String[] {"topology","devicemanager","learningswitch"});
- testCallbackOrdering(new String[] {"topology","devicemanager","routing","learningswitch"}, new String[] {"topology","devicemanager","routing","learningswitch"});
- testCallbackOrdering(new String[] {"devicemanager","topology","learningswitch","routing"}, new String[] {"topology","devicemanager","routing","learningswitch"});
+ IOFMessageListener createLMock(String name) {
+ IOFMessageListener mock = createNiceMock(IOFMessageListener.class);
+ expect(mock.getName()).andReturn(name).anyTimes();
+ return mock;
}
-
- protected void testCallbackOrdering(String[] addOrder, String[] verifyOrder) throws Exception {
+
+ void addPrereqs(IOFMessageListener mock, String... deps) {
+ for (String dep : deps) {
+ expect(mock.isCallbackOrderingPrereq(OFType.PACKET_IN, dep)).andReturn(true).anyTimes();
+ }
+ }
+
+ void testOrdering(ArrayList<IOFMessageListener> inputListeners) {
ListenerDispatcher<OFType, IOFMessageListener> ld =
- new ListenerDispatcher<OFType, IOFMessageListener>();
-
- IOFMessageListener topology = createNiceMock(IOFMessageListener.class);
- IOFMessageListener devicemanager = createNiceMock(IOFMessageListener.class);
- IOFMessageListener routing = createNiceMock(IOFMessageListener.class);
- IOFMessageListener learningswitch = createNiceMock(IOFMessageListener.class);
-
- expect(topology.getName()).andReturn("topology").anyTimes();
- expect(devicemanager.getName()).andReturn("devicemanager").anyTimes();
- expect(routing.getName()).andReturn("routing").anyTimes();
- expect(learningswitch.getName()).andReturn("learningswitch").anyTimes();
-
- expect(devicemanager.isCallbackOrderingPrereq(OFType.PACKET_IN, "topology")).andReturn(true).anyTimes();
- expect(routing.isCallbackOrderingPrereq(OFType.PACKET_IN, "topology")).andReturn(true).anyTimes();
- expect(routing.isCallbackOrderingPrereq(OFType.PACKET_IN, "devicemanager")).andReturn(true).anyTimes();
- expect(learningswitch.isCallbackOrderingPrereq(OFType.PACKET_IN, "routing")).andReturn(true).anyTimes();
- expect(learningswitch.isCallbackOrderingPrereq(OFType.PACKET_IN, "devicemanager")).andReturn(true).anyTimes();
+ new ListenerDispatcher<OFType, IOFMessageListener>();
+
+ for (IOFMessageListener l : inputListeners) {
+ ld.addListener(OFType.PACKET_IN, l);
+ }
+ for (IOFMessageListener l : inputListeners) {
+ verify(l);
+ }
+
+ List<IOFMessageListener> result = ld.getOrderedListeners();
+ System.out.print("Ordering: ");
+ for (IOFMessageListener l : result) {
+ System.out.print(l.getName());
+ System.out.print(",");
+ }
+ System.out.print("\n");
- replay(topology, devicemanager, routing, learningswitch);
- for (String o : addOrder) {
- if ("topology".equals(o)) {
- ld.addListener(OFType.PACKET_IN, topology);
- } else if ("devicemanager".equals(o)) {
- ld.addListener(OFType.PACKET_IN, devicemanager);
- } else if ("routing".equals(o)) {
- ld.addListener(OFType.PACKET_IN, routing);
- } else {
- ld.addListener(OFType.PACKET_IN, learningswitch);
+ for (int ind_i = 0; ind_i < result.size(); ind_i++) {
+ IOFMessageListener i = result.get(ind_i);
+ for (int ind_j = ind_i+1; ind_j < result.size(); ind_j++) {
+ IOFMessageListener j = result.get(ind_j);
+
+ boolean orderwrong =
+ (i.isCallbackOrderingPrereq(OFType.PACKET_IN, j.getName()) ||
+ j.isCallbackOrderingPostreq(OFType.PACKET_IN, i.getName()));
+ assertFalse("Invalid order: " +
+ ind_i + " (" + i.getName() + ") " +
+ ind_j + " (" + j.getName() + ") ", orderwrong);
}
}
+ }
+
+ void randomTestOrdering(ArrayList<IOFMessageListener> mocks) {
+ Random rand = new Random(0);
+ ArrayList<IOFMessageListener> random =
+ new ArrayList<IOFMessageListener>();
+ random.addAll(mocks);
+ for (int i = 0; i < 20; i++) {
+ for (int j = 0; j < random.size(); j++) {
+ int ind = rand.nextInt(mocks.size()-1);
+ IOFMessageListener tmp = random.get(j);
+ random.set(j, random.get(ind));
+ random.set(ind, tmp);
+ }
+ testOrdering(random);
+ }
+ }
+
+ @Test
+ public void testCallbackOrderingSimple() throws Exception {
+ ArrayList<IOFMessageListener> mocks =
+ new ArrayList<IOFMessageListener>();
+ for (int i = 0; i < 10; i++) {
+ mocks.add(createLMock(""+i));
+ }
+ for (int i = 1; i < 10; i++) {
+ addPrereqs(mocks.get(i), ""+(i-1));
+ }
+ for (IOFMessageListener l : mocks) {
+ replay(l);
+ }
+ randomTestOrdering(mocks);
+ }
- verify(topology, devicemanager, routing, learningswitch);
- for (int i = 0; i < verifyOrder.length; ++i) {
- String o = verifyOrder[i];
- assertEquals(o, ld.getOrderedListeners().get(i).getName());
+ @Test
+ public void testCallbackOrderingPartial() throws Exception {
+ ArrayList<IOFMessageListener> mocks =
+ new ArrayList<IOFMessageListener>();
+ for (int i = 0; i < 10; i++) {
+ mocks.add(createLMock(""+i));
+ }
+ for (int i = 1; i < 5; i++) {
+ addPrereqs(mocks.get(i), ""+(i-1));
+ }
+ for (int i = 6; i < 10; i++) {
+ addPrereqs(mocks.get(i), ""+(i-1));
}
+ for (IOFMessageListener l : mocks) {
+ replay(l);
+ }
+ randomTestOrdering(mocks);
}
+
+ @Test
+ public void testCallbackOrderingPartial2() throws Exception {
+ ArrayList<IOFMessageListener> mocks =
+ new ArrayList<IOFMessageListener>();
+ for (int i = 0; i < 10; i++) {
+ mocks.add(createLMock(""+i));
+ }
+ for (int i = 2; i < 5; i++) {
+ addPrereqs(mocks.get(i), ""+(i-1));
+ }
+ for (int i = 6; i < 9; i++) {
+ addPrereqs(mocks.get(i), ""+(i-1));
+ }
+ for (IOFMessageListener l : mocks) {
+ replay(l);
+ }
+ randomTestOrdering(mocks);
+ }
}