Refactoring to eliminate duplicate DefaultTopology and DefaultTopologyGraph; eli…

…minating a few fixmes.

Change-Id: I4461b6f1c8ae60d39f5da909bf6995294cbfa84d

@@ -15,10 +15,8 @@
  */
  */
 package org.onosproject.net.topology;
 package org.onosproject.net.topology;
 
 
-import static org.slf4j.LoggerFactory.getLogger;
-
-import java.util.Map;
-
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
 import org.onosproject.net.AbstractDescription;
 import org.onosproject.net.AbstractDescription;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.Device;
 import org.onosproject.net.Device;
@@ -27,14 +25,15 @@ import org.onosproject.net.Link;
 import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.SparseAnnotations;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
 
 
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Maps;
+import java.util.Map;
+
+import static org.slf4j.LoggerFactory.getLogger;
 
 
 /**
 /**
  * Default implementation of an immutable topology graph data carrier.
  * Default implementation of an immutable topology graph data carrier.
  */
  */
 public class DefaultGraphDescription extends AbstractDescription
 public class DefaultGraphDescription extends AbstractDescription
-implements GraphDescription {
+        implements GraphDescription {
 
 
     private static final Logger log = getLogger(DefaultGraphDescription.class);
     private static final Logger log = getLogger(DefaultGraphDescription.class);
 
 
@@ -43,19 +42,15 @@ implements GraphDescription {
     private final ImmutableSet<TopologyVertex> vertexes;
     private final ImmutableSet<TopologyVertex> vertexes;
     private final ImmutableSet<TopologyEdge> edges;
     private final ImmutableSet<TopologyEdge> edges;
 
 
-    private final Map<DeviceId, TopologyVertex> vertexesById = Maps
-            .newHashMap();
+    private final Map<DeviceId, TopologyVertex> vertexesById = Maps.newHashMap();
 
 
     /**
     /**
      * Creates a minimal topology graph description to allow core to construct
      * Creates a minimal topology graph description to allow core to construct
      * and process the topology graph.
      * and process the topology graph.
      *
      *
      * @param nanos       time in nanos of when the topology description was created
      * @param nanos       time in nanos of when the topology description was created
-     *
      * @param devices     collection of infrastructure devices
      * @param devices     collection of infrastructure devices
-     *
      * @param links       collection of infrastructure links
      * @param links       collection of infrastructure links
-     *
      * @param annotations optional key/value annotations map
      * @param annotations optional key/value annotations map
      * @deprecated in Cardinal Release
      * @deprecated in Cardinal Release
      */
      */
@@ -71,15 +66,10 @@ implements GraphDescription {
      * and process the topology graph.
      * and process the topology graph.
      *
      *
      * @param nanos       time in nanos of when the topology description was created
      * @param nanos       time in nanos of when the topology description was created
-     *
      * @param millis      time in millis of when the topology description was created
      * @param millis      time in millis of when the topology description was created
-     *
      * @param devices     collection of infrastructure devices
      * @param devices     collection of infrastructure devices
-     *
      * @param links       collection of infrastructure links
      * @param links       collection of infrastructure links
-     *
      * @param annotations optional key/value annotations map
      * @param annotations optional key/value annotations map
-     *
      */
      */
     public DefaultGraphDescription(long nanos, long millis,
     public DefaultGraphDescription(long nanos, long millis,
                                    Iterable<Device> devices,
                                    Iterable<Device> devices,
@@ -114,8 +104,7 @@ implements GraphDescription {
     }
     }
 
 
     // Builds a set of topology vertexes from the specified list of devices
     // Builds a set of topology vertexes from the specified list of devices
-    private ImmutableSet<TopologyVertex>
-            buildVertexes(Iterable<Device> devices) {
+    private ImmutableSet<TopologyVertex> buildVertexes(Iterable<Device> devices) {
         ImmutableSet.Builder<TopologyVertex> vertexes = ImmutableSet.builder();
         ImmutableSet.Builder<TopologyVertex> vertexes = ImmutableSet.builder();
         for (Device device : devices) {
         for (Device device : devices) {
             TopologyVertex vertex = new DefaultTopologyVertex(device.id());
             TopologyVertex vertex = new DefaultTopologyVertex(device.id());

@@ -13,20 +13,14 @@
  * See the License for the specific language governing permissions and
  * See the License for the specific language governing permissions and
  * limitations under the License.
  * limitations under the License.
  */
  */
-package org.onosproject.store.topology.impl;
-
-import static com.google.common.base.MoreObjects.toStringHelper;
-import static org.onlab.graph.GraphPathSearch.ALL_PATHS;
-import static org.onosproject.core.CoreService.CORE_PROVIDER_ID;
-import static org.onosproject.net.Link.State.ACTIVE;
-import static org.onosproject.net.Link.State.INACTIVE;
-import static org.onosproject.net.Link.Type.INDIRECT;
-
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+package org.onosproject.common;
 
 
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.collect.ImmutableSetMultimap.Builder;
 import org.onlab.graph.DijkstraGraphSearch;
 import org.onlab.graph.DijkstraGraphSearch;
 import org.onlab.graph.GraphPathSearch;
 import org.onlab.graph.GraphPathSearch;
 import org.onlab.graph.GraphPathSearch.Result;
 import org.onlab.graph.GraphPathSearch.Result;
@@ -50,14 +44,18 @@ import org.onosproject.net.topology.TopologyEdge;
 import org.onosproject.net.topology.TopologyGraph;
 import org.onosproject.net.topology.TopologyGraph;
 import org.onosproject.net.topology.TopologyVertex;
 import org.onosproject.net.topology.TopologyVertex;
 
 
-import com.google.common.base.Supplier;
-import com.google.common.base.Suppliers;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSetMultimap;
-import com.google.common.collect.ImmutableSetMultimap.Builder;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static com.google.common.base.MoreObjects.toStringHelper;
+import static org.onlab.graph.GraphPathSearch.ALL_PATHS;
+import static org.onosproject.core.CoreService.CORE_PROVIDER_ID;
+import static org.onosproject.net.Link.State.ACTIVE;
+import static org.onosproject.net.Link.State.INACTIVE;
+import static org.onosproject.net.Link.Type.INDIRECT;
 
 
-// FIXME: Move to onos-core-common when ready
 /**
 /**
  * Default implementation of the topology descriptor. This carries the backing
  * Default implementation of the topology descriptor. This carries the backing
  * topology data.
  * topology data.
@@ -83,12 +81,10 @@ public class DefaultTopology extends AbstractModel implements Topology {
     /**
     /**
      * Creates a topology descriptor attributed to the specified provider.
      * Creates a topology descriptor attributed to the specified provider.
      *
      *
-     * @param providerId
-     *            identity of the provider
-     * @param description
-     *            data describing the new topology
+     * @param providerId  identity of the provider
+     * @param description data describing the new topology
      */
      */
-    DefaultTopology(ProviderId providerId, GraphDescription description) {
+    public DefaultTopology(ProviderId providerId, GraphDescription description) {
         super(providerId);
         super(providerId);
         this.time = description.timestamp();
         this.time = description.timestamp();
         this.creationTime = description.creationTime();
         this.creationTime = description.creationTime();
@@ -104,8 +100,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
 
 
         this.weight = new HopCountLinkWeight(graph.getVertexes().size());
         this.weight = new HopCountLinkWeight(graph.getVertexes().size());
         this.broadcastSets = Suppliers.memoize(() -> buildBroadcastSets());
         this.broadcastSets = Suppliers.memoize(() -> buildBroadcastSets());
-        this.infrastructurePoints = Suppliers
-                .memoize(() -> findInfrastructurePoints());
+        this.infrastructurePoints = Suppliers.memoize(() -> findInfrastructurePoints());
         this.computeCost = Math.max(0, System.nanoTime() - time);
         this.computeCost = Math.max(0, System.nanoTime() - time);
     }
     }
 
 
@@ -156,7 +151,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
      *
      *
      * @return topology graph
      * @return topology graph
      */
      */
-    TopologyGraph getGraph() {
+    public TopologyGraph getGraph() {
         return graph;
         return graph;
     }
     }
 
 
@@ -165,7 +160,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
      *
      *
      * @return set of clusters
      * @return set of clusters
      */
      */
-    Set<TopologyCluster> getClusters() {
+    public Set<TopologyCluster> getClusters() {
         return ImmutableSet.copyOf(clusters.get().values());
         return ImmutableSet.copyOf(clusters.get().values());
     }
     }
 
 
@@ -173,10 +168,9 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * Returns the specified topology cluster.
      * Returns the specified topology cluster.
      *
      *
      * @param clusterId cluster identifier
      * @param clusterId cluster identifier
-     *
      * @return topology cluster
      * @return topology cluster
      */
      */
-    TopologyCluster getCluster(ClusterId clusterId) {
+    public TopologyCluster getCluster(ClusterId clusterId) {
         return clusters.get().get(clusterId);
         return clusters.get().get(clusterId);
     }
     }
 
 
@@ -184,10 +178,9 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * Returns the topology cluster that contains the given device.
      * Returns the topology cluster that contains the given device.
      *
      *
      * @param deviceId device identifier
      * @param deviceId device identifier
-     *
      * @return topology cluster
      * @return topology cluster
      */
      */
-    TopologyCluster getCluster(DeviceId deviceId) {
+    public TopologyCluster getCluster(DeviceId deviceId) {
         return clustersByDevice().get(deviceId);
         return clustersByDevice().get(deviceId);
     }
     }
 
 
@@ -195,10 +188,9 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * Returns the set of cluster devices.
      * Returns the set of cluster devices.
      *
      *
      * @param cluster topology cluster
      * @param cluster topology cluster
-     *
      * @return cluster devices
      * @return cluster devices
      */
      */
-    Set<DeviceId> getClusterDevices(TopologyCluster cluster) {
+    public Set<DeviceId> getClusterDevices(TopologyCluster cluster) {
         return devicesByCluster().get(cluster);
         return devicesByCluster().get(cluster);
     }
     }
 
 
@@ -206,10 +198,9 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * Returns the set of cluster links.
      * Returns the set of cluster links.
      *
      *
      * @param cluster topology cluster
      * @param cluster topology cluster
-     *
      * @return cluster links
      * @return cluster links
      */
      */
-    Set<Link> getClusterLinks(TopologyCluster cluster) {
+    public Set<Link> getClusterLinks(TopologyCluster cluster) {
         return linksByCluster().get(cluster);
         return linksByCluster().get(cluster);
     }
     }
 
 
@@ -217,10 +208,9 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * Indicates whether the given point is an infrastructure link end-point.
      * Indicates whether the given point is an infrastructure link end-point.
      *
      *
      * @param connectPoint connection point
      * @param connectPoint connection point
-     *
      * @return true if infrastructure
      * @return true if infrastructure
      */
      */
-    boolean isInfrastructure(ConnectPoint connectPoint) {
+    public boolean isInfrastructure(ConnectPoint connectPoint) {
         return infrastructurePoints.get().contains(connectPoint);
         return infrastructurePoints.get().contains(connectPoint);
     }
     }
 
 
@@ -228,10 +218,9 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * Indicates whether the given point is part of a broadcast set.
      * Indicates whether the given point is part of a broadcast set.
      *
      *
      * @param connectPoint connection point
      * @param connectPoint connection point
-     *
      * @return true if in broadcast set
      * @return true if in broadcast set
      */
      */
-    boolean isBroadcastPoint(ConnectPoint connectPoint) {
+    public boolean isBroadcastPoint(ConnectPoint connectPoint) {
         // Any non-infrastructure, i.e. edge points are assumed to be OK.
         // Any non-infrastructure, i.e. edge points are assumed to be OK.
         if (!isInfrastructure(connectPoint)) {
         if (!isInfrastructure(connectPoint)) {
             return true;
             return true;
@@ -254,10 +243,9 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * Returns the size of the cluster broadcast set.
      * Returns the size of the cluster broadcast set.
      *
      *
      * @param clusterId cluster identifier
      * @param clusterId cluster identifier
-     *
      * @return size of the cluster broadcast set
      * @return size of the cluster broadcast set
      */
      */
-    int broadcastSetSize(ClusterId clusterId) {
+    public int broadcastSetSize(ClusterId clusterId) {
         return broadcastSets.get().get(clusterId).size();
         return broadcastSets.get().get(clusterId).size();
     }
     }
 
 
@@ -266,12 +254,10 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * destination devices.
      * destination devices.
      *
      *
      * @param src source device
      * @param src source device
-     *
      * @param dst destination device
      * @param dst destination device
-     *
      * @return set of shortest paths
      * @return set of shortest paths
      */
      */
-    Set<Path> getPaths(DeviceId src, DeviceId dst) {
+    public Set<Path> getPaths(DeviceId src, DeviceId dst) {
         return getPaths(src, dst, null);
         return getPaths(src, dst, null);
     }
     }
 
 
@@ -280,14 +266,11 @@ public class DefaultTopology extends AbstractModel implements Topology {
      * destination devices.
      * destination devices.
      *
      *
      * @param src    source device
      * @param src    source device
-     *
      * @param dst    destination device
      * @param dst    destination device
-     *
      * @param weight link weight function
      * @param weight link weight function
-     *
      * @return set of shortest paths
      * @return set of shortest paths
      */
      */
-    Set<Path> getPaths(DeviceId src, DeviceId dst, LinkWeight weight) {
+    public Set<Path> getPaths(DeviceId src, DeviceId dst, LinkWeight weight) {
         final DefaultTopologyVertex srcV = new DefaultTopologyVertex(src);
         final DefaultTopologyVertex srcV = new DefaultTopologyVertex(src);
         final DefaultTopologyVertex dstV = new DefaultTopologyVertex(dst);
         final DefaultTopologyVertex dstV = new DefaultTopologyVertex(dst);
         Set<TopologyVertex> vertices = graph.getVertexes();
         Set<TopologyVertex> vertices = graph.getVertexes();
@@ -470,8 +453,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
         final ImmutableSetMultimap<TopologyCluster, DeviceId> devicesByCluster;
         final ImmutableSetMultimap<TopologyCluster, DeviceId> devicesByCluster;
         final ImmutableSetMultimap<TopologyCluster, Link> linksByCluster;
         final ImmutableSetMultimap<TopologyCluster, Link> linksByCluster;
 
 
-        public ClusterIndexes(
-                ImmutableMap<DeviceId, TopologyCluster> clustersByDevice,
+        public ClusterIndexes(ImmutableMap<DeviceId, TopologyCluster> clustersByDevice,
                               ImmutableSetMultimap<TopologyCluster, DeviceId> devicesByCluster,
                               ImmutableSetMultimap<TopologyCluster, DeviceId> devicesByCluster,
                               ImmutableSetMultimap<TopologyCluster, Link> linksByCluster) {
                               ImmutableSetMultimap<TopologyCluster, Link> linksByCluster) {
             this.clustersByDevice = clustersByDevice;
             this.clustersByDevice = clustersByDevice;

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * See the License for the specific language governing permissions and
  * limitations under the License.
  * limitations under the License.
  */
  */
-package org.onosproject.store.trivial;
+package org.onosproject.common;
 
 
 import org.onlab.graph.AdjacencyListsGraph;
 import org.onlab.graph.AdjacencyListsGraph;
 import org.onosproject.net.topology.TopologyEdge;
 import org.onosproject.net.topology.TopologyEdge;

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * See the License for the specific language governing permissions and
  * limitations under the License.
  * limitations under the License.
  */
  */
-package org.onosproject.store.trivial;
+package org.onosproject.common;
 
 
 import org.junit.Before;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.Test;

@@ -19,6 +19,7 @@ import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Service;
 import org.apache.felix.scr.annotations.Service;
+import org.onosproject.common.DefaultTopology;
 import org.onosproject.event.Event;
 import org.onosproject.event.Event;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.DeviceId;

-/*
- * Copyright 2014 Open Networking Laboratory
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.onosproject.store.topology.impl;
-
-import org.onlab.graph.AdjacencyListsGraph;
-import org.onosproject.net.topology.TopologyEdge;
-import org.onosproject.net.topology.TopologyGraph;
-import org.onosproject.net.topology.TopologyVertex;
-
-import java.util.Set;
-
-/**
- * Default implementation of an immutable topology graph based on a generic
- * implementation of adjacency lists graph.
- */
-public class DefaultTopologyGraph
-        extends AdjacencyListsGraph<TopologyVertex, TopologyEdge>
-        implements TopologyGraph {
-
-    /**
-     * Creates a topology graph comprising of the specified vertexes and edges.
-     *
-     * @param vertexes set of graph vertexes
-     * @param edges    set of graph edges
-     */
-    public DefaultTopologyGraph(Set<TopologyVertex> vertexes, Set<TopologyEdge> edges) {
-        super(vertexes, edges);
-    }
-
-}

@@ -15,6 +15,8 @@
  */
  */
 package org.onosproject.store.topology.impl;
 package org.onosproject.store.topology.impl;
 
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static org.onosproject.net.topology.TopologyEvent.Type.TOPOLOGY_CHANGED;
 import static org.slf4j.LoggerFactory.getLogger;
 import static org.slf4j.LoggerFactory.getLogger;
 
 
 import java.util.Collections;
 import java.util.Collections;
@@ -25,6 +27,7 @@ import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Service;
 import org.apache.felix.scr.annotations.Service;
+import org.onosproject.common.DefaultTopology;
 import org.onosproject.event.Event;
 import org.onosproject.event.Event;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.Device;
 import org.onosproject.net.Device;
@@ -48,21 +51,21 @@ import org.slf4j.Logger;
 /**
 /**
  * Manages inventory of topology snapshots using trivial in-memory
  * Manages inventory of topology snapshots using trivial in-memory
  * structures implementation.
  * structures implementation.
- *
+ * <p>
  * Note: This component is not distributed per-se. It runs on every
  * Note: This component is not distributed per-se. It runs on every
  * instance and feeds off of other distributed stores.
  * instance and feeds off of other distributed stores.
  */
  */
 @Component(immediate = true)
 @Component(immediate = true)
 @Service
 @Service
 public class DistributedTopologyStore
 public class DistributedTopologyStore
-extends AbstractStore<TopologyEvent, TopologyStoreDelegate>
-implements TopologyStore {
+        extends AbstractStore<TopologyEvent, TopologyStoreDelegate>
+        implements TopologyStore {
 
 
     private final Logger log = getLogger(getClass());
     private final Logger log = getLogger(getClass());
 
 
     private volatile DefaultTopology current =
     private volatile DefaultTopology current =
             new DefaultTopology(ProviderId.NONE,
             new DefaultTopology(ProviderId.NONE,
-                    new DefaultGraphDescription(0L,
+                                new DefaultGraphDescription(0L, 0L,
                                                             Collections.<Device>emptyList(),
                                                             Collections.<Device>emptyList(),
                                                             Collections.<Link>emptyList()));
                                                             Collections.<Link>emptyList()));
 
 
@@ -75,6 +78,7 @@ implements TopologyStore {
     public void deactivate() {
     public void deactivate() {
         log.info("Stopped");
         log.info("Stopped");
     }
     }
+
     @Override
     @Override
     public Topology currentTopology() {
     public Topology currentTopology() {
         return current;
         return current;
@@ -149,18 +153,15 @@ implements TopologyStore {
         // Promote the new topology to current and return a ready-to-send event.
         // Promote the new topology to current and return a ready-to-send event.
         synchronized (this) {
         synchronized (this) {
             current = newTopology;
             current = newTopology;
-            return new TopologyEvent(TopologyEvent.Type.TOPOLOGY_CHANGED,
-                                     current, reasons);
+            return new TopologyEvent(TOPOLOGY_CHANGED, current, reasons);
         }
         }
     }
     }
 
 
     // Validates the specified topology and returns it as a default
     // Validates the specified topology and returns it as a default
     private DefaultTopology defaultTopology(Topology topology) {
     private DefaultTopology defaultTopology(Topology topology) {
-        if (topology instanceof DefaultTopology) {
+        checkArgument(topology instanceof DefaultTopology,
+                      "Topology class %s not supported", topology.getClass());
         return (DefaultTopology) topology;
         return (DefaultTopology) topology;
     }
     }
-        throw new IllegalArgumentException("Topology class " + topology.getClass() +
-                " not supported");
-    }
 
 
 }
 }