Formalized TopologyStore in preparation for separating managers and stores.

+package org.onlab.onos.net.topology;
+
+import org.onlab.onos.event.Event;
+import org.onlab.onos.net.ConnectPoint;
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.Link;
+import org.onlab.onos.net.Path;
+import org.onlab.onos.net.provider.ProviderId;
+
+import java.util.List;
+import java.util.Set;
+
+/**
+ * Manages inventory of topology snapshots. It may do so using whatever
+ * means appropriate.
+ */
+public interface TopologyStore {
+
+    /**
+     * Returns the current topology snapshot.
+     *
+     * @return current topology descriptor
+     */
+    Topology currentTopology();
+
+    /**
+     * Indicates whether the topology is the latest.
+     *
+     * @param topology topology descriptor
+     * @return true if topology is the most recent one
+     */
+    boolean isLatest(Topology topology);
+
+    /**
+     * Returns the immutable graph view of the current topology.
+     *
+     * @param topology topology descriptor
+     * @return graph view
+     */
+    TopologyGraph getGraph(Topology topology);
+
+    /**
+     * Returns the set of topology SCC clusters.
+     *
+     * @param topology topology descriptor
+     * @return set of clusters
+     */
+    Set<TopologyCluster> getClusters(Topology topology);
+
+    /**
+     * Returns the cluster of the specified topology.
+     *
+     * @param topology  topology descriptor
+     * @param clusterId cluster identity
+     * @return topology cluster
+     */
+    TopologyCluster getCluster(Topology topology, ClusterId clusterId);
+
+    /**
+     * Returns the cluster of the specified topology.
+     *
+     * @param topology topology descriptor
+     * @param cluster  topology cluster
+     * @return set of cluster links
+     */
+    Set<DeviceId> getClusterDevices(Topology topology, TopologyCluster cluster);
+
+    /**
+     * Returns the cluster of the specified topology.
+     *
+     * @param topology topology descriptor
+     * @param cluster  topology cluster
+     * @return set of cluster links
+     */
+    Set<Link> getClusterLinks(Topology topology, TopologyCluster cluster);
+
+    /**
+     * Returns the set of pre-computed shortest paths between src and dest.
+     *
+     * @param topology topology descriptor
+     * @param src      source device
+     * @param dst      destination device
+     * @return set of shortest paths
+     */
+    Set<Path> getPaths(Topology topology, DeviceId src, DeviceId dst);
+
+    /**
+     * Computes and returns the set of shortest paths between src and dest.
+     *
+     * @param topology topology descriptor
+     * @param src      source device
+     * @param dst      destination device
+     * @param weight   link weight function
+     * @return set of shortest paths
+     */
+    Set<Path> getPaths(Topology topology, DeviceId src, DeviceId dst,
+                       LinkWeight weight);
+
+    /**
+     * Indicates whether the given connect point is part of the network fabric.
+     *
+     * @param topology     topology descriptor
+     * @param connectPoint connection point
+     * @return true if infrastructure; false otherwise
+     */
+    boolean isInfrastructure(Topology topology, ConnectPoint connectPoint);
+
+    /**
+     * Indicates whether broadcast is allowed for traffic received on the
+     * given connection point.
+     *
+     * @param topology     topology descriptor
+     * @param connectPoint connection point
+     * @return true if broadcast allowed; false otherwise
+     */
+    boolean isBroadcastPoint(Topology topology, ConnectPoint connectPoint);
+
+    /**
+     * Generates a new topology snapshot from the specified description.
+     *
+     * @param providerId       provider identification
+     * @param graphDescription topology graph description
+     * @param reasons          list of events that triggered the update
+     * @return topology update event or null if the description is old
+     */
+    TopologyEvent updateTopology(ProviderId providerId,
+                                 GraphDescription graphDescription,
+                                 List<Event> reasons);
+}

@@ -41,7 +41,7 @@ import static org.slf4j.LoggerFactory.getLogger;
  */
 @Component(immediate = true)
 @Service
-public class SimplePathManager implements PathService {
+public class PathManager implements PathService {
 
     private static final String ELEMENT_ID_NULL = "Element ID cannot be null";
 

 package org.onlab.onos.net.trivial.topology.impl;
 
+import org.apache.felix.scr.annotations.Activate;
+import org.apache.felix.scr.annotations.Component;
+import org.apache.felix.scr.annotations.Deactivate;
+import org.apache.felix.scr.annotations.Service;
 import org.onlab.onos.event.Event;
 import org.onlab.onos.net.ConnectPoint;
 import org.onlab.onos.net.DeviceId;
@@ -13,149 +17,94 @@ import org.onlab.onos.net.topology.Topology;
 import org.onlab.onos.net.topology.TopologyCluster;
 import org.onlab.onos.net.topology.TopologyEvent;
 import org.onlab.onos.net.topology.TopologyGraph;
+import org.onlab.onos.net.topology.TopologyStore;
+import org.slf4j.Logger;
 
 import java.util.List;
 import java.util.Set;
 
+import static org.slf4j.LoggerFactory.getLogger;
+
 /**
  * Manages inventory of topology snapshots using trivial in-memory
  * structures implementation.
  */
-class SimpleTopologyStore {
+@Component(immediate = true)
+@Service
+public class SimpleTopologyStore implements TopologyStore {
+
+    private final Logger log = getLogger(getClass());
 
     private volatile DefaultTopology current;
 
-    /**
-     * Returns the current topology snapshot.
-     *
-     * @return current topology descriptor
-     */
-    Topology currentTopology() {
+    @Activate
+    public void activate() {
+        log.info("Started");
+    }
+
+    @Deactivate
+    public void deactivate() {
+        log.info("Stopped");
+    }
+    @Override
+    public Topology currentTopology() {
         return current;
     }
 
-    /**
-     * Indicates whether the topology is the latest.
-     *
-     * @param topology topology descriptor
-     * @return true if topology is the most recent one
-     */
-    boolean isLatest(Topology topology) {
+    @Override
+    public boolean isLatest(Topology topology) {
         // Topology is current only if it is the same as our current topology
         return topology == current;
     }
 
-    /**
-     * Returns the immutable graph view of the current topology.
-     *
-     * @param topology topology descriptor
-     * @return graph view
-     */
-    TopologyGraph getGraph(DefaultTopology topology) {
-        return topology.getGraph();
+    @Override
+    public TopologyGraph getGraph(Topology topology) {
+        return defaultTopology(topology).getGraph();
     }
 
-    /**
-     * Returns the set of topology SCC clusters.
-     *
-     * @param topology topology descriptor
-     * @return set of clusters
-     */
-    Set<TopologyCluster> getClusters(DefaultTopology topology) {
-        return topology.getClusters();
+    @Override
+    public Set<TopologyCluster> getClusters(Topology topology) {
+        return defaultTopology(topology).getClusters();
     }
 
-    /**
-     * Returns the cluster of the specified topology.
-     *
-     * @param topology  topology descriptor
-     * @param clusterId cluster identity
-     * @return topology cluster
-     */
-    TopologyCluster getCluster(DefaultTopology topology, ClusterId clusterId) {
-        return topology.getCluster(clusterId);
+    @Override
+    public TopologyCluster getCluster(Topology topology, ClusterId clusterId) {
+        return defaultTopology(topology).getCluster(clusterId);
     }
 
-    /**
-     * Returns the cluster of the specified topology.
-     *
-     * @param topology  topology descriptor
-     * @param cluster topology cluster
-     * @return set of cluster links
-     */
-    Set<DeviceId> getClusterDevices(DefaultTopology topology, TopologyCluster cluster) {
-        return topology.getClusterDevices(cluster);
+    @Override
+    public Set<DeviceId> getClusterDevices(Topology topology, TopologyCluster cluster) {
+        return defaultTopology(topology).getClusterDevices(cluster);
     }
 
-    /**
-     * Returns the cluster of the specified topology.
-     *
-     * @param topology  topology descriptor
-     * @param cluster topology cluster
-     * @return set of cluster links
-     */
-    Set<Link> getClusterLinks(DefaultTopology topology, TopologyCluster cluster) {
-        return topology.getClusterLinks(cluster);
+    @Override
+    public Set<Link> getClusterLinks(Topology topology, TopologyCluster cluster) {
+        return defaultTopology(topology).getClusterLinks(cluster);
     }
 
-    /**
-     * Returns the set of pre-computed shortest paths between src and dest.
-     *
-     * @param topology topology descriptor
-     * @param src      source device
-     * @param dst      destination device
-     * @return set of shortest paths
-     */
-    Set<Path> getPaths(DefaultTopology topology, DeviceId src, DeviceId dst) {
-        return topology.getPaths(src, dst);
-    }
-
-    /**
-     * Computes and returns the set of shortest paths between src and dest.
-     *
-     * @param topology topology descriptor
-     * @param src      source device
-     * @param dst      destination device
-     * @param weight   link weight function
-     * @return set of shortest paths
-     */
-    Set<Path> getPaths(DefaultTopology topology, DeviceId src, DeviceId dst,
+    @Override
+    public Set<Path> getPaths(Topology topology, DeviceId src, DeviceId dst) {
+        return defaultTopology(topology).getPaths(src, dst);
+    }
+
+    @Override
+    public Set<Path> getPaths(Topology topology, DeviceId src, DeviceId dst,
                               LinkWeight weight) {
-        return topology.getPaths(src, dst, weight);
+        return defaultTopology(topology).getPaths(src, dst, weight);
     }
 
-    /**
-     * Indicates whether the given connect point is part of the network fabric.
-     *
-     * @param topology     topology descriptor
-     * @param connectPoint connection point
-     * @return true if infrastructure; false otherwise
-     */
-    boolean isInfrastructure(DefaultTopology topology, ConnectPoint connectPoint) {
-        return topology.isInfrastructure(connectPoint);
+    @Override
+    public boolean isInfrastructure(Topology topology, ConnectPoint connectPoint) {
+        return defaultTopology(topology).isInfrastructure(connectPoint);
     }
 
-    /**
-     * Indicates whether broadcast is allowed for traffic received on the
-     * given connection point.
-     *
-     * @param topology     topology descriptor
-     * @param connectPoint connection point
-     * @return true if broadcast allowed; false otherwise
-     */
-    boolean isBroadcastPoint(DefaultTopology topology, ConnectPoint connectPoint) {
-        return topology.isBroadcastPoint(connectPoint);
+    @Override
+    public boolean isBroadcastPoint(Topology topology, ConnectPoint connectPoint) {
+        return defaultTopology(topology).isBroadcastPoint(connectPoint);
     }
 
-    /**
-     * Generates a new topology snapshot from the specified description.
-     *
-     * @param providerId       provider identification
-     * @param graphDescription topology graph description
-     * @param reasons          list of events that triggered the update
-     * @return topology update event or null if the description is old
-     */
-    TopologyEvent updateTopology(ProviderId providerId,
+    @Override
+    public TopologyEvent updateTopology(ProviderId providerId,
                                         GraphDescription graphDescription,
                                         List<Event> reasons) {
         // First off, make sure that what we're given is indeed newer than
@@ -175,4 +124,13 @@ class SimpleTopologyStore {
         }
     }
 
+    // Validates the specified topology and returns it as a default
+    private DefaultTopology defaultTopology(Topology topology) {
+        if (topology instanceof DefaultTopology) {
+            return (DefaultTopology) topology;
+        }
+        throw new IllegalArgumentException("Topology class " + topology.getClass() +
+                                                   " not supported");
+    }
+
 }

@@ -27,6 +27,7 @@ import org.onlab.onos.net.topology.TopologyProvider;
 import org.onlab.onos.net.topology.TopologyProviderRegistry;
 import org.onlab.onos.net.topology.TopologyProviderService;
 import org.onlab.onos.net.topology.TopologyService;
+import org.onlab.onos.net.topology.TopologyStore;
 import org.slf4j.Logger;
 
 import java.util.List;
@@ -40,7 +41,7 @@ import static org.slf4j.LoggerFactory.getLogger;
  */
 @Component(immediate = true)
 @Service
-public class SimpleTopologyManager
+public class TopologyManager
         extends AbstractProviderRegistry<TopologyProvider, TopologyProviderService>
         implements TopologyService, TopologyProviderRegistry {
 
@@ -55,7 +56,8 @@ public class SimpleTopologyManager
     private final AbstractListenerRegistry<TopologyEvent, TopologyListener>
             listenerRegistry = new AbstractListenerRegistry<>();
 
-    private final SimpleTopologyStore store = new SimpleTopologyStore();
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected TopologyStore store;
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected EventDeliveryService eventDispatcher;
@@ -81,49 +83,40 @@ public class SimpleTopologyManager
     @Override
     public boolean isLatest(Topology topology) {
         checkNotNull(topology, TOPOLOGY_NULL);
-        return store.isLatest(defaultTopology(topology));
-    }
-
-    // Validates the specified topology and returns it as a default
-    private DefaultTopology defaultTopology(Topology topology) {
-        if (topology instanceof DefaultTopology) {
-            return (DefaultTopology) topology;
-        }
-        throw new IllegalArgumentException("Topology class " + topology.getClass() +
-                                                   " not supported");
+        return store.isLatest(topology);
     }
 
     @Override
     public Set<TopologyCluster> getClusters(Topology topology) {
         checkNotNull(topology, TOPOLOGY_NULL);
-        return store.getClusters(defaultTopology(topology));
+        return store.getClusters(topology);
     }
 
     @Override
     public TopologyCluster getCluster(Topology topology, ClusterId clusterId) {
         checkNotNull(topology, TOPOLOGY_NULL);
         checkNotNull(topology, CLUSTER_ID_NULL);
-        return store.getCluster(defaultTopology(topology), clusterId);
+        return store.getCluster(topology, clusterId);
     }
 
     @Override
     public Set<DeviceId> getClusterDevices(Topology topology, TopologyCluster cluster) {
         checkNotNull(topology, TOPOLOGY_NULL);
         checkNotNull(topology, CLUSTER_NULL);
-        return store.getClusterDevices(defaultTopology(topology), cluster);
+        return store.getClusterDevices(topology, cluster);
     }
 
     @Override
     public Set<Link> getClusterLinks(Topology topology, TopologyCluster cluster) {
         checkNotNull(topology, TOPOLOGY_NULL);
         checkNotNull(topology, CLUSTER_NULL);
-        return store.getClusterLinks(defaultTopology(topology), cluster);
+        return store.getClusterLinks(topology, cluster);
     }
 
     @Override
     public TopologyGraph getGraph(Topology topology) {
         checkNotNull(topology, TOPOLOGY_NULL);
-        return store.getGraph(defaultTopology(topology));
+        return store.getGraph(topology);
     }
 
     @Override
@@ -131,7 +124,7 @@ public class SimpleTopologyManager
         checkNotNull(topology, TOPOLOGY_NULL);
         checkNotNull(src, DEVICE_ID_NULL);
         checkNotNull(dst, DEVICE_ID_NULL);
-        return store.getPaths(defaultTopology(topology), src, dst);
+        return store.getPaths(topology, src, dst);
     }
 
     @Override
@@ -140,21 +133,21 @@ public class SimpleTopologyManager
         checkNotNull(src, DEVICE_ID_NULL);
         checkNotNull(dst, DEVICE_ID_NULL);
         checkNotNull(weight, "Link weight cannot be null");
-        return store.getPaths(defaultTopology(topology), src, dst, weight);
+        return store.getPaths(topology, src, dst, weight);
     }
 
     @Override
     public boolean isInfrastructure(Topology topology, ConnectPoint connectPoint) {
         checkNotNull(topology, TOPOLOGY_NULL);
         checkNotNull(connectPoint, CONNECTION_POINT_NULL);
-        return store.isInfrastructure(defaultTopology(topology), connectPoint);
+        return store.isInfrastructure(topology, connectPoint);
     }
 
     @Override
     public boolean isBroadcastPoint(Topology topology, ConnectPoint connectPoint) {
         checkNotNull(topology, TOPOLOGY_NULL);
         checkNotNull(connectPoint, CONNECTION_POINT_NULL);
-        return store.isBroadcastPoint(defaultTopology(topology), connectPoint);
+        return store.isBroadcastPoint(topology, connectPoint);
     }
 
     @Override

@@ -27,8 +27,8 @@ import static org.junit.Assert.assertNotNull;
 import static org.onlab.junit.TestTools.assertAfter;
 import static org.onlab.onos.net.device.DeviceEvent.Type.DEVICE_ADDED;
 import static org.onlab.onos.net.link.LinkEvent.Type.LINK_ADDED;
-import static org.onlab.onos.net.trivial.topology.impl.SimpleTopologyManagerTest.device;
-import static org.onlab.onos.net.trivial.topology.impl.SimpleTopologyManagerTest.link;
+import static org.onlab.onos.net.trivial.topology.impl.TopologyManagerTest.device;
+import static org.onlab.onos.net.trivial.topology.impl.TopologyManagerTest.link;
 
 /**
  * Test of the default topology provider implementation.

@@ -21,8 +21,8 @@ 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;
+import static org.onlab.onos.net.trivial.topology.impl.TopologyManagerTest.device;
+import static org.onlab.onos.net.trivial.topology.impl.TopologyManagerTest.link;
 
 /**
  * Test of the default topology implementation.

@@ -41,11 +41,11 @@ import static org.onlab.onos.net.topology.TopologyEvent.Type.TOPOLOGY_CHANGED;
 /**
  * Test of the topology subsystem.
  */
-public class SimpleTopologyManagerTest {
+public class TopologyManagerTest {
 
     private static final ProviderId PID = new ProviderId("of", "foo");
 
-    private SimpleTopologyManager mgr;
+    private TopologyManager mgr;
 
     protected TopologyService service;
     protected TopologyProviderRegistry registry;
@@ -55,10 +55,11 @@ public class SimpleTopologyManagerTest {
 
     @Before
     public void setUp() {
-        mgr = new SimpleTopologyManager();
+        mgr = new TopologyManager();
         service = mgr;
         registry = mgr;
 
+        mgr.store = new SimpleTopologyStore();
         mgr.eventDispatcher = new TestEventDispatcher();
         mgr.activate();