Move indices to TopologyImpl.

Change-Id: I3efd73129fafb786e959810e8239ce14d26af6d7
diff --git a/src/main/java/net/onrc/onos/core/topology/PortImpl.java b/src/main/java/net/onrc/onos/core/topology/PortImpl.java
index 77bc942..792d9f1 100644
--- a/src/main/java/net/onrc/onos/core/topology/PortImpl.java
+++ b/src/main/java/net/onrc/onos/core/topology/PortImpl.java
@@ -1,10 +1,6 @@
 package net.onrc.onos.core.topology;
 
-import java.util.Collections;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
-
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import net.onrc.onos.core.util.Dpid;
 import net.onrc.onos.core.util.PortNumber;
@@ -25,15 +21,10 @@
 
     private final SwitchPort switchPort;
 
-    // These needs to be ConcurrentCollecton if allowing the topology to be
-    // accessed concurrently
-    protected Set<Device> devices;
-
     public PortImpl(Topology topology, Switch parentSwitch, PortNumber number) {
         super(topology);
         this.sw = parentSwitch;
         this.number = number;
-        this.devices = new HashSet<>();
 
         switchPort = new SwitchPort(parentSwitch.getDpid(),
                                     number);
@@ -92,27 +83,7 @@
 
     @Override
     public Iterable<Device> getDevices() {
-        return Collections.unmodifiableSet(this.devices);
-    }
-
-    /**
-     * @param d
-     * @return true if successfully added
-     */
-    public boolean addDevice(Device d) {
-        return this.devices.add(d);
-    }
-
-    /**
-     * @param d
-     * @return true if device existed and was removed
-     */
-    public boolean removeDevice(Device d) {
-        return this.devices.remove(d);
-    }
-
-    public void removeAllDevice() {
-        this.devices.clear();
+        return topology.getDevices(this.asSwitchPort());
     }
 
     @Override
diff --git a/src/main/java/net/onrc/onos/core/topology/SwitchImpl.java b/src/main/java/net/onrc/onos/core/topology/SwitchImpl.java
index 4952c6f..ad13a79 100644
--- a/src/main/java/net/onrc/onos/core/topology/SwitchImpl.java
+++ b/src/main/java/net/onrc/onos/core/topology/SwitchImpl.java
@@ -1,18 +1,15 @@
 package net.onrc.onos.core.topology;
 
-import net.onrc.onos.core.util.Dpid;
-import net.onrc.onos.core.util.PortNumber;
-
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import net.onrc.onos.core.util.Dpid;
+import net.onrc.onos.core.util.PortNumber;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 /**
@@ -24,9 +21,6 @@
 public class SwitchImpl extends TopologyObject implements Switch {
 
     private Dpid dpid;
-    // These needs to be ConcurrentCollecton if allowing the topology to be
-    // accessed concurrently
-    private final Map<PortNumber, Port> ports;
 
     public SwitchImpl(Topology topology, Long dpid) {
         this(topology, new Dpid(dpid));
@@ -35,7 +29,6 @@
     public SwitchImpl(Topology topology, Dpid dpid) {
         super(topology);
         this.dpid = dpid;
-        ports = new HashMap<>();
     }
 
     @Override
@@ -45,12 +38,12 @@
 
     @Override
     public Collection<Port> getPorts() {
-        return Collections.unmodifiableCollection(ports.values());
+        return topology.getPorts(getDpid());
     }
 
     @Override
     public Port getPort(PortNumber number) {
-        return ports.get(number);
+        return topology.getPort(getDpid(), number);
     }
 
     @Override
@@ -82,7 +75,7 @@
         // calculating this every time?
         List<Device> devices = new ArrayList<Device>();
 
-        for (Port port : ports.values()) {
+        for (Port port : getPorts()) {
             for (Device device : port.getDevices()) {
                 devices.add(device);
             }
@@ -91,25 +84,16 @@
         return devices;
     }
 
-    public void addPort(Port port) {
-        this.ports.put(port.getNumber(), port);
-    }
-
-    public Port removePort(Port port) {
-        Port p = this.ports.remove(port.getNumber());
-        return p;
-    }
-
     public Port addPort(Long portNumber) {
         PortImpl port = new PortImpl(topology, this, portNumber);
-        ports.put(port.getNumber(), port);
+        ((TopologyImpl) topology).putPort(port);
         return port;
     }
 
     @Override
     public Iterable<Link> getOutgoingLinks() {
         LinkedList<Link> links = new LinkedList<Link>();
-        for (Port port : ports.values()) {
+        for (Port port : getPorts()) {
             Link link = port.getOutgoingLink();
             if (link != null) {
                 links.add(link);
@@ -121,7 +105,7 @@
     @Override
     public Iterable<Link> getIncomingLinks() {
         LinkedList<Link> links = new LinkedList<Link>();
-        for (Port port : ports.values()) {
+        for (Port port : getPorts()) {
             Link link = port.getIncomingLink();
             if (link != null) {
                 links.add(link);
diff --git a/src/main/java/net/onrc/onos/core/topology/Topology.java b/src/main/java/net/onrc/onos/core/topology/Topology.java
index 3bef1e2..089d0e1 100644
--- a/src/main/java/net/onrc/onos/core/topology/Topology.java
+++ b/src/main/java/net/onrc/onos/core/topology/Topology.java
@@ -1,5 +1,7 @@
 package net.onrc.onos.core.topology;
 
+import java.util.Collection;
+
 import net.floodlightcontroller.util.MACAddress;
 import net.onrc.onos.core.topology.web.serializers.TopologySerializer;
 import net.onrc.onos.core.util.Dpid;
@@ -49,6 +51,14 @@
     public Port getPort(SwitchPort port);
 
     /**
+     * Gets all ports on a switch specified.
+     *
+     * @param dpid Switch dpid
+     * @return ports.
+     */
+    public Collection<Port> getPorts(Dpid dpid);
+
+    /**
      * Gets the outgoing link from a switch port.
      *
      * @param dpid   the switch DPID.
@@ -113,6 +123,21 @@
     public Device getDeviceByMac(MACAddress address);
 
     /**
+     * Gets all devices in the network.
+     *
+     * @return all devices in the network
+     */
+    public Iterable<Device> getDevices();
+
+    /**
+     * Gets all devices on specified port.
+     *
+     * @param port port which the device is attached
+     * @return all devices attached to the port.
+     */
+    public Collection<Device> getDevices(SwitchPort port);
+
+    /**
      * Acquire a read lock on the entire topology. The topology will not
      * change while readers have the lock. Must be released using
      * {@link #releaseReadLock()}. This method will block until a read lock is
@@ -124,6 +149,4 @@
      * Release the read lock on the topology.
      */
     public void releaseReadLock();
-
-    public Iterable<Device> getDevices();
 }
diff --git a/src/main/java/net/onrc/onos/core/topology/TopologyImpl.java b/src/main/java/net/onrc/onos/core/topology/TopologyImpl.java
index 2bba015..5ce3729 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyImpl.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyImpl.java
@@ -1,5 +1,6 @@
 package net.onrc.onos.core.topology;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -15,12 +16,21 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Multimaps;
+
 public class TopologyImpl implements Topology {
     @SuppressWarnings("unused")
     private static final Logger log = LoggerFactory.getLogger(TopologyImpl.class);
 
     // DPID -> Switch
     private final ConcurrentMap<Dpid, Switch> switches;
+    // XXX may need to be careful when shallow copying.
+    private final ConcurrentMap<Dpid, ConcurrentMap<PortNumber, Port>> ports;
+
+    // Index from Port to Device
+    private final Multimap<SwitchPort, Device> devices;
     private final ConcurrentMap<MACAddress, Device> mac2Device;
 
     private final ConcurrentMap<SwitchPort, Link> outgoingLinks;
@@ -34,6 +44,9 @@
     public TopologyImpl() {
         // TODO: Does these object need to be stored in Concurrent Collection?
         switches = new ConcurrentHashMap<>();
+        ports = new ConcurrentHashMap<>();
+        devices = Multimaps.synchronizedMultimap(
+                HashMultimap.<SwitchPort, Device>create());
         mac2Device = new ConcurrentHashMap<>();
         outgoingLinks = new ConcurrentHashMap<>();
         incomingLinks = new ConcurrentHashMap<>();
@@ -45,16 +58,43 @@
         return switches.get(dpid);
     }
 
+    // Only add switch.
     protected void putSwitch(Switch sw) {
         switches.put(sw.getDpid(), sw);
+        ports.putIfAbsent(sw.getDpid(), new ConcurrentHashMap<PortNumber, Port>());
     }
 
+    // TODO remove me when ready
     protected void removeSwitch(Long dpid) {
-        switches.remove(new Dpid(dpid));
+        removeSwitch(new Dpid(dpid));
     }
 
+    // XXX Will remove ports in snapshot as side-effect.
     protected void removeSwitch(Dpid dpid) {
         switches.remove(dpid);
+        ports.remove(dpid);
+    }
+
+    // This method is expected to be serialized by writeLock.
+    protected void putPort(Port port) {
+        ConcurrentMap<PortNumber, Port> portMap = ports.get(port.getDpid());
+        if (portMap == null) {
+            portMap = new ConcurrentHashMap<>();
+            ConcurrentMap<PortNumber, Port> existing =
+                    ports.putIfAbsent(port.getDpid(), portMap);
+            if (existing != null) {
+                // port map was added concurrently, using theirs
+                portMap = existing;
+            }
+        }
+        portMap.put(port.getNumber(), port);
+    }
+
+    protected void removePort(Port port) {
+        ConcurrentMap<PortNumber, Port> portMap = ports.get(port.getDpid());
+        if (portMap != null) {
+            portMap.remove(port.getNumber());
+        }
     }
 
     @Override
@@ -65,9 +105,9 @@
 
     @Override
     public Port getPort(Dpid dpid, PortNumber number) {
-        Switch sw = getSwitch(dpid);
-        if (sw != null) {
-            return sw.getPort(number);
+        ConcurrentMap<PortNumber, Port> portMap = ports.get(dpid);
+        if (portMap != null) {
+            return portMap.get(number);
         }
         return null;
     }
@@ -78,6 +118,15 @@
     }
 
     @Override
+    public Collection<Port> getPorts(Dpid dpid) {
+        ConcurrentMap<PortNumber, Port> portMap = ports.get(dpid);
+        if (portMap == null) {
+            return Collections.emptyList();
+        }
+        return Collections.unmodifiableCollection(portMap.values());
+    }
+
+    @Override
     public Link getOutgoingLink(Dpid dpid, PortNumber number) {
         return outgoingLinks.get(new SwitchPort(dpid, number));
     }
@@ -139,11 +188,34 @@
         return Collections.unmodifiableCollection(mac2Device.values());
     }
 
+    @Override
+    public Collection<Device> getDevices(SwitchPort port) {
+        return Collections.unmodifiableCollection(devices.get(port));
+    }
+
+    // This method is expected to be serialized by writeLock.
+    // XXX new or updated device
     protected void putDevice(Device device) {
+        // assuming Device is immutable
+        Device oldDevice = mac2Device.get(device.getMacAddress());
+        if (oldDevice != null) {
+            // remove old attachment point
+            removeDevice(oldDevice);
+        }
+        // add new attachment points
+        for (Port port : device.getAttachmentPoints()) {
+            // TODO Won't need remove() if we define Device equality to reflect
+            //      all of it's fields.
+            devices.remove(port.asSwitchPort(), device);
+            devices.put(port.asSwitchPort(), device);
+        }
         mac2Device.put(device.getMacAddress(), device);
     }
 
     protected void removeDevice(Device device) {
+        for (Port port : device.getAttachmentPoints()) {
+            devices.remove(port.asSwitchPort(), device);
+        }
         mac2Device.remove(device.getMacAddress());
     }
 
diff --git a/src/main/java/net/onrc/onos/core/topology/TopologyManager.java b/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
index 8302ff3..3cff46a 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
@@ -894,12 +894,11 @@
             reorderedAddedPortEvents.put(id, portEvent);
             return;
         }
-        SwitchImpl switchImpl = getSwitchImpl(sw);
 
         Port port = sw.getPort(portEvent.getPortNumber());
         if (port == null) {
             port = new PortImpl(topology, sw, portEvent.getPortNumber());
-            switchImpl.addPort(port);
+            topology.putPort(port);
         } else {
             // TODO: Update the port attributes
             log.debug("Update port attributes");
@@ -967,7 +966,7 @@
 
         // Remove the Port from the Switch
         SwitchImpl switchImpl = getSwitchImpl(sw);
-        switchImpl.removePort(port);
+        topology.removePort(port);
 
         apiRemovedPortEvents.add(portEvent);
     }
@@ -1116,7 +1115,6 @@
 
             // Add Device <-> Port attachment
             PortImpl portImpl = getPortImpl(port);
-            portImpl.addDevice(device);
             deviceImpl.addAttachmentPoint(port);
             attachmentFound = true;
         }
@@ -1140,30 +1138,13 @@
      */
     @GuardedBy("topology.writeLock")
     private void removeDevice(DeviceEvent deviceEvent) {
-        log.debug("Removing a device to the topology: mac {}", deviceEvent.getMac());
+        log.debug("Removing a device from the topology: mac {}", deviceEvent.getMac());
         Device device = topology.getDeviceByMac(deviceEvent.getMac());
         if (device == null) {
             log.warn("Device {} already removed, ignoring", deviceEvent);
             return;
         }
-        DeviceImpl deviceImpl = getDeviceImpl(device);
 
-        // Process each attachment point
-        for (SwitchPort swp : deviceEvent.getAttachmentPoints()) {
-            // Attached Ports must exist
-            Port port = topology.getPort(swp.getDpid(), swp.getPortNumber());
-            if (port == null) {
-                log.warn("Port for the attachment point {} did not exist. skipping attachment point mutation", swp);
-                continue;
-            }
-
-            // Remove Device <-> Port attachment
-            PortImpl portImpl = getPortImpl(port);
-            portImpl.removeDevice(device);
-            deviceImpl.removeAttachmentPoint(port);
-        }
-
-        log.debug("Removing the device info into the Topology: mac {}", deviceEvent.getMac());
         topology.removeDevice(device);
         apiRemovedDeviceEvents.add(deviceEvent);
     }