Cleaned-up some javadocs and added some tests.

@@ -74,13 +74,12 @@ public class ReactiveForwarding {
 
 
         @Override
         @Override
         public void process(PacketContext context) {
         public void process(PacketContext context) {
-            /*
-             *  stop processing if the packet has been handled,
-             *  we can't do any more to it
-             */
+            // Stop processing if the packet has been handled, since we
+            // can't do any more to it.
             if (context.isHandled()) {
             if (context.isHandled()) {
                 return;
                 return;
             }
             }
+
             InboundPacket pkt = context.inPacket();
             InboundPacket pkt = context.inPacket();
             HostId id = HostId.hostId(pkt.parsed().getDestinationMAC());
             HostId id = HostId.hostId(pkt.parsed().getDestinationMAC());
 
 
@@ -100,10 +99,9 @@ public class ReactiveForwarding {
 
 
             // Otherwise, get a set of paths that lead from here to the
             // Otherwise, get a set of paths that lead from here to the
             // destination edge switch.
             // destination edge switch.
-
             Set<Path> paths = topologyService.getPaths(topologyService.currentTopology(),
             Set<Path> paths = topologyService.getPaths(topologyService.currentTopology(),
-                    context.inPacket().receivedFrom().deviceId(),
-                    dst.location().deviceId());
+                                                       context.inPacket().receivedFrom().deviceId(),
+                                                       dst.location().deviceId());
             if (paths.isEmpty()) {
             if (paths.isEmpty()) {
                 // If there are no paths, flood and bail.
                 // If there are no paths, flood and bail.
                 flood(context);
                 flood(context);
@@ -137,9 +135,8 @@ public class ReactiveForwarding {
 
 
     // Floods the specified packet.
     // Floods the specified packet.
     private void flood(PacketContext context) {
     private void flood(PacketContext context) {
-        boolean canBcast = topologyService.isBroadcastPoint(topologyService.currentTopology(),
-                context.inPacket().receivedFrom());
-        if (canBcast) {
+        if (topologyService.isBroadcastPoint(topologyService.currentTopology(),
+                                             context.inPacket().receivedFrom())) {
             packetOutFlood(context);
             packetOutFlood(context);
         } else {
         } else {
             context.block();
             context.block();
@@ -154,27 +151,25 @@ public class ReactiveForwarding {
 
 
     // Install a rule forwarding the packet to the specified port.
     // Install a rule forwarding the packet to the specified port.
     private void installRule(PacketContext context, PortNumber portNumber) {
     private void installRule(PacketContext context, PortNumber portNumber) {
-        // we don't yet support bufferids in the flowservice so packet out and
-        // then install a flowmod.
-        context.treatmentBuilder().add(Instructions.createOutput(portNumber));
-        context.send();
-
-
         Ethernet inPkt = context.inPacket().parsed();
         Ethernet inPkt = context.inPacket().parsed();
         TrafficSelector.Builder builder = new DefaultTrafficSelector.Builder();
         TrafficSelector.Builder builder = new DefaultTrafficSelector.Builder();
         builder.add(Criteria.matchEthType(inPkt.getEtherType()))
         builder.add(Criteria.matchEthType(inPkt.getEtherType()))
-        .add(Criteria.matchEthSrc(inPkt.getSourceMAC()))
-        .add(Criteria.matchEthDst(inPkt.getDestinationMAC()))
-        .add(Criteria.matchInPort(context.inPacket().receivedFrom().port()));
+                .add(Criteria.matchEthSrc(inPkt.getSourceMAC()))
+                .add(Criteria.matchEthDst(inPkt.getDestinationMAC()))
+                .add(Criteria.matchInPort(context.inPacket().receivedFrom().port()));
 
 
         TrafficTreatment.Builder treat = new DefaultTrafficTreatment.Builder();
         TrafficTreatment.Builder treat = new DefaultTrafficTreatment.Builder();
         treat.add(Instructions.createOutput(portNumber));
         treat.add(Instructions.createOutput(portNumber));
 
 
         FlowRule f = new DefaultFlowRule(context.inPacket().receivedFrom().deviceId(),
         FlowRule f = new DefaultFlowRule(context.inPacket().receivedFrom().deviceId(),
-                builder.build(), treat.build());
+                                         builder.build(), treat.build());
 
 
         flowRuleService.applyFlowRules(f);
         flowRuleService.applyFlowRules(f);
 
 
+        // we don't yet support bufferids in the flowservice so packet out and
+        // then install a flowmod.
+        context.treatmentBuilder().add(Instructions.createOutput(portNumber));
+        context.send();
     }
     }
 
 
 }
 }

 /**
 /**
- * Core subsystem for tracking infrastructure devices.
+ * Core subsystem for tracking global inventory of infrastructure devices.
  */
  */
 package org.onlab.onos.net.trivial.device.impl;
 package org.onlab.onos.net.trivial.device.impl;

+/**
+ * Core subsystem for tracking and manipulating global flow state.
+ */
+package org.onlab.onos.net.trivial.flow.impl;

 /**
 /**
- * Core subsystem for tracking edn-station hosts.
+ * Core subsystem for tracking global inventory of end-station hosts.
  */
  */
 package org.onlab.onos.net.trivial.host.impl;
 package org.onlab.onos.net.trivial.host.impl;

 /**
 /**
- * Core subsystem for tracking infrastructure links.
+ * Core subsystem for tracking global inventory of infrastructure links.
  */
  */
 package org.onlab.onos.net.trivial.link.impl;
 package org.onlab.onos.net.trivial.link.impl;

 /**
 /**
  * Core subsystem for processing inbound packets and emitting outbound packets.
  * Core subsystem for processing inbound packets and emitting outbound packets.
+ * Processing of inbound packets is always in the local context only, but
+ * emitting outbound packets allows for cluster-wide operation.
  */
  */
 package org.onlab.onos.net.trivial.packet.impl;
 package org.onlab.onos.net.trivial.packet.impl;

@@ -142,6 +142,15 @@ public class DefaultTopology extends AbstractModel implements Topology {
         return clusters.get(clusterId);
         return clusters.get(clusterId);
     }
     }
 
 
+    /**
+     * Returns the topology cluster that contains the given device.
+     *
+     * @param deviceId device identifier
+     * @return topology cluster
+     */
+    TopologyCluster getCluster(DeviceId deviceId) {
+        return clustersByDevice.get(deviceId);
+    }
 
 
     /**
     /**
      * Returns the set of cluster devices.
      * Returns the set of cluster devices.
@@ -174,13 +183,13 @@ public class DefaultTopology extends AbstractModel implements Topology {
     }
     }
 
 
     /**
     /**
-     * Indicates whether the given point is part of a broadcast tree.
+     * Indicates whether the given point is part of a broadcast set.
      *
      *
      * @param connectPoint connection point
      * @param connectPoint connection point
-     * @return true if in broadcast tree
+     * @return true if in broadcast set
      */
      */
-    boolean isInBroadcastTree(ConnectPoint connectPoint) {
-        // Any non-infrastructure, i.e. edge points are assumed to be OK
+    boolean isBroadcastPoint(ConnectPoint connectPoint) {
+        // Any non-infrastructure, i.e. edge points are assumed to be OK.
         if (!isInfrastructure(connectPoint)) {
         if (!isInfrastructure(connectPoint)) {
             return true;
             return true;
         }
         }
@@ -191,13 +200,23 @@ public class DefaultTopology extends AbstractModel implements Topology {
             throw new IllegalArgumentException("No cluster found for device " + connectPoint.deviceId());
             throw new IllegalArgumentException("No cluster found for device " + connectPoint.deviceId());
         }
         }
 
 
-        // If the broadcast tree is null or empty, or if the point explicitly
-        // belongs to the broadcast tree points, return true;
+        // If the broadcast set is null or empty, or if the point explicitly
+        // belongs to it, return true;
         Set<ConnectPoint> points = broadcastSets.get(cluster.id());
         Set<ConnectPoint> points = broadcastSets.get(cluster.id());
         return points == null || points.isEmpty() || points.contains(connectPoint);
         return points == null || points.isEmpty() || points.contains(connectPoint);
     }
     }
 
 
     /**
     /**
+     * Returns the size of the cluster broadcast set.
+     *
+     * @param clusterId cluster identifier
+     * @return size of the cluster broadcast set
+     */
+    int broadcastSetSize(ClusterId clusterId) {
+        return broadcastSets.get(clusterId).size();
+    }
+
+    /**
      * Returns the set of pre-computed shortest paths between source and
      * Returns the set of pre-computed shortest paths between source and
      * destination devices.
      * destination devices.
      *
      *

@@ -144,7 +144,7 @@ class SimpleTopologyStore {
      * @return true if broadcast allowed; false otherwise
      * @return true if broadcast allowed; false otherwise
      */
      */
     boolean isBroadcastPoint(DefaultTopology topology, ConnectPoint connectPoint) {
     boolean isBroadcastPoint(DefaultTopology topology, ConnectPoint connectPoint) {
-        return topology.isInBroadcastTree(connectPoint);
+        return topology.isBroadcastPoint(connectPoint);
     }
     }
 
 
     /**
     /**

 /**
 /**
- * Core subsystem for tracking consistent topology graph views.
+ * Core subsystem for tracking global & consistent topology graph views.
  */
  */
 package org.onlab.onos.net.trivial.topology.impl;
 package org.onlab.onos.net.trivial.topology.impl;

+package org.onlab.onos.net.trivial.topology.impl;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.onos.net.ConnectPoint;
+import org.onlab.onos.net.Device;
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.Link;
+import org.onlab.onos.net.Path;
+import org.onlab.onos.net.PortNumber;
+import org.onlab.onos.net.provider.ProviderId;
+import org.onlab.onos.net.topology.ClusterId;
+import org.onlab.onos.net.topology.GraphDescription;
+import org.onlab.onos.net.topology.LinkWeight;
+import org.onlab.onos.net.topology.TopologyCluster;
+import org.onlab.onos.net.topology.TopologyEdge;
+
+import java.util.Set;
+
+import static com.google.common.collect.ImmutableSet.of;
+import static org.junit.Assert.*;
+import static org.onlab.onos.net.DeviceId.deviceId;
+import static org.onlab.onos.net.PortNumber.portNumber;
+import static org.onlab.onos.net.trivial.topology.impl.SimpleTopologyManagerTest.device;
+import static org.onlab.onos.net.trivial.topology.impl.SimpleTopologyManagerTest.link;
+
+/**
+ * Test of the default topology implementation.
+ */
+public class DefaultTopologyTest {
+
+    public static final ProviderId PID = new ProviderId("foo.bar");
+
+    public static final DeviceId D1 = deviceId("of:1");
+    public static final DeviceId D2 = deviceId("of:2");
+    public static final DeviceId D3 = deviceId("of:3");
+    public static final DeviceId D4 = deviceId("of:4");
+    public static final DeviceId D5 = deviceId("of:5");
+
+    public static final PortNumber P1 = portNumber(1);
+    public static final PortNumber P2 = portNumber(2);
+
+    public static final LinkWeight WEIGHT = new LinkWeight() {
+        @Override
+        public double weight(TopologyEdge edge) {
+            return edge.src().deviceId().equals(D4) ||
+                    edge.dst().deviceId().equals(D4) ? 2.0 : 1.0;
+        }
+    };
+
+    private DefaultTopology dt;
+
+    @Before
+    public void setUp() {
+        long now = System.currentTimeMillis();
+        Set<Device> devices = of(device("1"), device("2"),
+                                 device("3"), device("4"),
+                                 device("5"));
+        Set<Link> links = of(link("1", 1, "2", 1), link("2", 1, "1", 1),
+                             link("3", 2, "2", 2), link("2", 2, "3", 2),
+                             link("1", 3, "4", 3), link("4", 3, "1", 3),
+                             link("3", 4, "4", 4), link("4", 4, "3", 4));
+        GraphDescription graphDescription =
+                new DefaultGraphDescription(now, devices, links);
+
+        dt = new DefaultTopology(PID, graphDescription);
+        assertEquals("incorrect supplier", PID, dt.providerId());
+        assertEquals("incorrect time", now, dt.time());
+        assertEquals("incorrect device count", 5, dt.deviceCount());
+        assertEquals("incorrect link count", 8, dt.linkCount());
+        assertEquals("incorrect cluster count", 2, dt.clusterCount());
+        assertEquals("incorrect broadcast set size", 6,
+                     dt.broadcastSetSize(ClusterId.clusterId(0)));
+    }
+
+    @Test
+    public void pathRelated() {
+        Set<Path> paths = dt.getPaths(D1, D2);
+        assertEquals("incorrect path count", 1, paths.size());
+
+        paths = dt.getPaths(D1, D3);
+        assertEquals("incorrect path count", 2, paths.size());
+
+        paths = dt.getPaths(D1, D5);
+        assertTrue("no paths expected", paths.isEmpty());
+
+        paths = dt.getPaths(D1, D3, WEIGHT);
+        assertEquals("incorrect path count", 1, paths.size());
+    }
+
+    @Test
+    public void pointRelated() {
+        assertTrue("should be infrastructure point",
+                   dt.isInfrastructure(new ConnectPoint(D1, P1)));
+        assertFalse("should not be infrastructure point",
+                    dt.isInfrastructure(new ConnectPoint(D1, P2)));
+    }
+
+    @Test
+    public void clusterRelated() {
+        Set<TopologyCluster> clusters = dt.getClusters();
+        assertEquals("incorrect cluster count", 2, clusters.size());
+
+        TopologyCluster c = dt.getCluster(D1);
+        Set<DeviceId> devs = dt.getClusterDevices(c);
+        assertEquals("incorrect cluster device count", 4, devs.size());
+        assertTrue("cluster should contain D2", devs.contains(D2));
+        assertFalse("cluster should not contain D5", devs.contains(D5));
+    }
+
+}

@@ -101,7 +101,6 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
         for (int i = 0; i < flowRules.length; i++) {
         for (int i = 0; i < flowRules.length; i++) {
             applyRule(flowRules[i]);
             applyRule(flowRules[i]);
         }
         }
-
     }
     }
 
 
     private void applyRule(FlowRule flowRule) {
     private void applyRule(FlowRule flowRule) {
@@ -120,9 +119,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
                 .setHardTimeout(10)
                 .setHardTimeout(10)
                 .setPriority(flowRule.priority())
                 .setPriority(flowRule.priority())
                 .build();
                 .build();
-
         sw.sendMsg(fm);
         sw.sendMsg(fm);
-
     }
     }
 
 
     private List<OFAction> buildActions(List<Instruction> instructions, OFFactory factory) {
     private List<OFAction> buildActions(List<Instruction> instructions, OFFactory factory) {