WIP: use Event object on NetworkGraphImpl

Change-Id: I3a1d80bb23057aa8f22f4f53d33d7eb40fd89a2e
diff --git a/src/main/java/net/onrc/onos/ofcontroller/floodlightlistener/RCNetworkGraphPublisher.java b/src/main/java/net/onrc/onos/ofcontroller/floodlightlistener/RCNetworkGraphPublisher.java
index 1ae616d..167583f 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/floodlightlistener/RCNetworkGraphPublisher.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/floodlightlistener/RCNetworkGraphPublisher.java
@@ -42,7 +42,7 @@
 												ILinkDiscoveryListener,
 												IFloodlightModule {
 	private static final Logger log = LoggerFactory.getLogger(RCNetworkGraphPublisher.class);
-	
+
 	private IFloodlightProviderService floodlightProvider;
 	private ILinkDiscoveryService linkDiscovery;
 	private IControllerRegistryService registryService;
@@ -78,7 +78,7 @@
 
 		switch (update.getOperation()) {
 		case LINK_ADDED:
-			southboundNetworkGraph.addLink(link);
+//			southboundNetworkGraph.addLink(link);
 			/*
 			TopologyElement topologyElement =
 					new TopologyElement(update.getSrc(),
@@ -93,7 +93,7 @@
 			// We never use it.
 			break;
 		case LINK_REMOVED:
-			southboundNetworkGraph.removeLink(link);
+//			southboundNetworkGraph.removeLink(link);
 			/*
 			TopologyElement topologyElement =
 					new TopologyElement(update.getSrc(),
@@ -128,7 +128,7 @@
 		}
 
 		Switch onosSwitch = FloodlightToOnosMappers.map(networkGraph, sw);
-		southboundNetworkGraph.addSwitch(onosSwitch);
+//		southboundNetworkGraph.addSwitch(onosSwitch);
 
 		/*
 		// TODO publish ADD_SWITCH event here
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphDatastore.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphDatastore.java
index 288b1f4..52a6ab7 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphDatastore.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphDatastore.java
@@ -5,8 +5,6 @@
 
 import net.onrc.onos.datastore.RCObject;
 import net.onrc.onos.datastore.RCObject.WriteOp;
-import net.onrc.onos.datastore.topology.RCLink;
-import net.onrc.onos.datastore.topology.RCPort;
 import net.onrc.onos.datastore.topology.RCSwitch;
 
 import org.slf4j.Logger;
@@ -36,14 +34,14 @@
 	private static final Logger log = LoggerFactory.getLogger(NetworkGraphDatastore.class);
 
 	private static final int NUM_RETRIES = 10;
-	
+
 	private final NetworkGraphImpl graph;
 
 	public NetworkGraphDatastore(NetworkGraphImpl graph) {
 		this.graph = graph;
 	}
 
-	public void addSwitch(Switch sw) {
+	public void addSwitch(SwitchEvent sw) {
 		log.debug("Adding switch {}", sw);
 		ArrayList<WriteOp> groupOp = new ArrayList<>();
 
@@ -55,13 +53,13 @@
 		// to assure that DPID is unique cluster-wide, etc.
 		groupOp.add(WriteOp.ForceCreate(rcSwitch));
 
-		for (Port port : sw.getPorts()) {
-			RCPort rcPort = new RCPort(sw.getDpid(), (long)port.getNumber());
-			rcPort.setStatus(RCPort.STATUS.ACTIVE);
-			rcSwitch.addPortId(rcPort.getId());
-
-			groupOp.add(WriteOp.ForceCreate(rcPort));
-		}
+//		for (Port port : sw.getPorts()) {
+//			RCPort rcPort = new RCPort(sw.getDpid(), (long)port.getNumber());
+//			rcPort.setStatus(RCPort.STATUS.ACTIVE);
+//			rcSwitch.addPortId(rcPort.getId());
+//
+//			groupOp.add(WriteOp.ForceCreate(rcPort));
+//		}
 
 		boolean failed = RCObject.multiWrite( groupOp );
 
@@ -76,12 +74,12 @@
 		}
 		else {
 			// Publish event to the in-memory cache
-			graph.addSwitch(sw);
+//			graph.addSwitch(sw);
 		}
 
 	}
 
-	public void deactivateSwitch(Switch sw) {
+	public void deactivateSwitch(SwitchEvent sw) {
 		log.debug("Deactivating switch {}", sw);
 		RCSwitch rcSwitch = new RCSwitch(sw.getDpid());
 
@@ -93,12 +91,12 @@
 				rcSwitch.setStatus(RCSwitch.STATUS.INACTIVE);
 				objectsToDeactive.add(rcSwitch);
 
-				for (Port p : sw.getPorts()) {
-					RCPort rcPort = new RCPort(sw.getDpid(), (long)p.getNumber());
-					rcPort.read();
-					rcPort.setStatus(RCPort.STATUS.INACTIVE);
-					objectsToDeactive.add(rcPort);
-				}
+//				for (Port p : sw.getPorts()) {
+//					RCPort rcPort = new RCPort(sw.getDpid(), (long)p.getNumber());
+//					rcPort.read();
+//					rcPort.setStatus(RCPort.STATUS.INACTIVE);
+//					objectsToDeactive.add(rcPort);
+//				}
 			} catch (ObjectDoesntExistException e) {
 				log.warn("Trying to deactivate an object that doesn't exist", e);
 				// We don't care to much if the object wasn't there, it's
@@ -120,132 +118,132 @@
 		}
 	}
 
-	public void addPort(Switch sw, Port port) {
+	public void addPort(PortEvent port) {
 		log.debug("Adding port {}", port);
-		RCSwitch rcSwitch = new RCSwitch(sw.getDpid());
-
-		try {
-			rcSwitch.read();
-		} catch (ObjectDoesntExistException e) {
-			log.warn("Add port failed because switch {} doesn't exist", sw.getDpid(), e);
-			return;
-		}
-
-		RCPort rcPort = new RCPort(port.getSwitch().getDpid(), (long)port.getNumber());
-		rcPort.setStatus(RCPort.STATUS.ACTIVE);
-		// TODO add description into RCPort
-		//rcPort.setDescription(port.getDescription());
-		rcSwitch.addPortId(rcPort.getId());
-
-		writeObject(rcPort);
-		writeObject(rcSwitch);
+//		RCSwitch rcSwitch = new RCSwitch(sw.getDpid());
+//
+//		try {
+//			rcSwitch.read();
+//		} catch (ObjectDoesntExistException e) {
+//			log.warn("Add port failed because switch {} doesn't exist", sw.getDpid(), e);
+//			return;
+//		}
+//
+//		RCPort rcPort = new RCPort(port.getSwitch().getDpid(), (long)port.getNumber());
+//		rcPort.setStatus(RCPort.STATUS.ACTIVE);
+//		// TODO add description into RCPort
+//		//rcPort.setDescription(port.getDescription());
+//		rcSwitch.addPortId(rcPort.getId());
+//
+//		writeObject(rcPort);
+//		writeObject(rcSwitch);
 	}
 
-	public void deactivatePort(Port port) {
+	public void deactivatePort(PortEvent port) {
 		log.debug("Deactivating port {}", port);
-		RCPort rcPort = new RCPort(port.getSwitch().getDpid(), (long)port.getNumber());
-
-		for (int i = 0; i < NUM_RETRIES; i++) {
-			try {
-				rcPort.read();
-			} catch (ObjectDoesntExistException e) {
-				// oh well, we were deactivating anyway
-				log.warn("Trying to deactivate a port that doesn't exist: {}", port);
-				return;
-			}
-
-			rcPort.setStatus(RCPort.STATUS.INACTIVE);
-
-			try {
-				rcPort.update();
-				break;
-			} catch (ObjectDoesntExistException | WrongVersionException e) {
-				// retry
-			}
-		}
+//		RCPort rcPort = new RCPort(port.getSwitch().getDpid(), (long)port.getNumber());
+//
+//		for (int i = 0; i < NUM_RETRIES; i++) {
+//			try {
+//				rcPort.read();
+//			} catch (ObjectDoesntExistException e) {
+//				// oh well, we were deactivating anyway
+//				log.warn("Trying to deactivate a port that doesn't exist: {}", port);
+//				return;
+//			}
+//
+//			rcPort.setStatus(RCPort.STATUS.INACTIVE);
+//
+//			try {
+//				rcPort.update();
+//				break;
+//			} catch (ObjectDoesntExistException | WrongVersionException e) {
+//				// retry
+//			}
+//		}
 	}
 
-	public void addLink(Link link) {
+	public void addLink(LinkEvent link) {
 		log.debug("Adding link {}", link);
-		RCLink rcLink = new RCLink(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber(),
-				link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
-
-		RCPort rcSrcPort = new RCPort(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber());
-		RCPort rcDstPort = new RCPort(link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
-
-		for (int i = 0; i < NUM_RETRIES; i++) {
-			try {
-				rcSrcPort.read();
-				rcDstPort.read();
-				rcLink.create();
-			} catch (ObjectDoesntExistException e) {
-				// port doesn't exist
-				log.error("Add link failed {}", link, e);
-				return;
-			} catch (ObjectExistsException e) {
-				log.debug("Link already exists {}", link);
-				return;
-			}
-
-			rcSrcPort.addLinkId(rcLink.getId());
-			rcDstPort.addLinkId(rcLink.getId());
-
-			rcLink.setStatus(RCLink.STATUS.ACTIVE);
-
-			try {
-				rcLink.update();
-				rcSrcPort.update();
-				rcDstPort.update();
-				break;
-			} catch (ObjectDoesntExistException | WrongVersionException e) {
-				log.debug(" ", e);
-				// retry
-			}
-		}
-		
-		// Publish event to in-memory cache
-		graph.addLink(link);
+//		RCLink rcLink = new RCLink(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber(),
+//				link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
+//
+//		RCPort rcSrcPort = new RCPort(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber());
+//		RCPort rcDstPort = new RCPort(link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
+//
+//		for (int i = 0; i < NUM_RETRIES; i++) {
+//			try {
+//				rcSrcPort.read();
+//				rcDstPort.read();
+//				rcLink.create();
+//			} catch (ObjectDoesntExistException e) {
+//				// port doesn't exist
+//				log.error("Add link failed {}", link, e);
+//				return;
+//			} catch (ObjectExistsException e) {
+//				log.debug("Link already exists {}", link);
+//				return;
+//			}
+//
+//			rcSrcPort.addLinkId(rcLink.getId());
+//			rcDstPort.addLinkId(rcLink.getId());
+//
+//			rcLink.setStatus(RCLink.STATUS.ACTIVE);
+//
+//			try {
+//				rcLink.update();
+//				rcSrcPort.update();
+//				rcDstPort.update();
+//				break;
+//			} catch (ObjectDoesntExistException | WrongVersionException e) {
+//				log.debug(" ", e);
+//				// retry
+//			}
+//		}
+//
+//		// Publish event to in-memory cache
+//		graph.putLink(link);
 	}
 
-	public void removeLink(Link link) {
+	public void removeLink(LinkEvent link) {
 		log.debug("Removing link {}", link);
-		RCLink rcLink = new RCLink(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber(),
-				link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
-
-		RCPort rcSrcPort = new RCPort(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber());
-		RCPort rcDstPort = new RCPort(link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
-
-		for (int i = 0; i < NUM_RETRIES; i++) {
-			try {
-				rcSrcPort.read();
-				rcDstPort.read();
-				rcLink.read();
-			} catch (ObjectDoesntExistException e) {
-				log.error("Remove link failed {}", link, e);
-				return;
-			}
-
-			rcSrcPort.removeLinkId(rcLink.getId());
-			rcDstPort.removeLinkId(rcLink.getId());
-
-			try {
-				rcSrcPort.update();
-				rcDstPort.update();
-				rcLink.delete();
-			} catch (ObjectDoesntExistException e) {
-				log.error("Remove link failed {}", link, e);
-				return;
-			} catch (WrongVersionException e) {
-				// retry
-			}
-		}
+//		RCLink rcLink = new RCLink(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber(),
+//				link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
+//
+//		RCPort rcSrcPort = new RCPort(link.getSourceSwitchDpid(), (long)link.getSourcePortNumber());
+//		RCPort rcDstPort = new RCPort(link.getDestinationSwitchDpid(), (long)link.getDestinationPortNumber());
+//
+//		for (int i = 0; i < NUM_RETRIES; i++) {
+//			try {
+//				rcSrcPort.read();
+//				rcDstPort.read();
+//				rcLink.read();
+//			} catch (ObjectDoesntExistException e) {
+//				log.error("Remove link failed {}", link, e);
+//				return;
+//			}
+//
+//			rcSrcPort.removeLinkId(rcLink.getId());
+//			rcDstPort.removeLinkId(rcLink.getId());
+//
+//			try {
+//				rcSrcPort.update();
+//				rcDstPort.update();
+//				rcLink.delete();
+//			} catch (ObjectDoesntExistException e) {
+//				log.error("Remove link failed {}", link, e);
+//				return;
+//			} catch (WrongVersionException e) {
+//				// retry
+//			}
+//		}
 	}
 
-	public void updateDevice(Device device) {
+	public void updateDevice(DeviceEvent device) {
 		// TODO implement
 	}
 
-	public void removeDevice(Device device) {
+	public void removeDevice(DeviceEvent device) {
 		// TODO implement
 	}
 
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java
index e3a3a39..14dfd64 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java
@@ -1,6 +1,7 @@
 package net.onrc.onos.ofcontroller.networkgraph;
 
-import java.util.NoSuchElementException;
+import java.net.InetAddress;
+import java.util.Set;
 
 import net.onrc.onos.datastore.topology.RCLink;
 import net.onrc.onos.datastore.topology.RCPort;
@@ -19,8 +20,13 @@
  *
  * TODO To be synchronized based on TopologyEvent Notification.
  *
+ * TODO TBD: Caller is expected to maintain parent/child calling order. Parent
+ * Object must exist before adding sub component(Add Switch -> Port). Child
+ * Object need to be removed before removing parent (Delete Port->Switch)
+ *
  * TODO TBD: This class may delay the requested change to handle event
  * re-ordering. e.g.) Link Add came in, but Switch was not there.
+ *
  */
 public class NetworkGraphImpl extends AbstractNetworkGraph {
 
@@ -32,211 +38,364 @@
     }
 
     /**
-     * Add Switch to Topology.
+     * put Switch
      *
-     * Fails with an Exception if a switch with same DPID already exist in
-     * Topology.
+     * XXX Internal Invariant-maintenance method. Will not write to DB. Will not
+     * fire Notification.
      *
-     * @param sw
+     * @param swEvt
      */
-    void addSwitch(Switch sw) {
-	if (sw == null) {
+    void putSwitch(SwitchEvent swEvt) {
+	if (swEvt == null) {
 	    throw new IllegalArgumentException("Switch cannot be null");
 	}
-	Switch oldSw = switches.putIfAbsent(sw.getDpid(), sw);
-	if (oldSw != null) {
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException("Switch already exists");
+	Switch sw = switches.get(swEvt.getDpid());
+
+	if (sw == null) {
+	    sw = new SwitchImpl(this, swEvt.getDpid());
+	    Switch existing = switches.putIfAbsent(swEvt.getDpid(), sw);
+	    if (existing != null) {
+		log.warn(
+			"Concurrent putSwitch not expected. Continuing updating {}",
+			existing);
+		sw = existing;
+	    }
+	}
+
+	// Add upate when more attributes are added to Event object
+	// no attribute to update for now
+    }
+
+    /**
+     * remove Switch.
+     *
+     * XXX Internal Invariant-maintenance method. Will not write to DB. Will not
+     * fire Notification.
+     *
+     * @param swEvt
+     */
+    void removeSwitch(SwitchEvent swEvt) {
+	if (swEvt == null) {
+	    throw new IllegalArgumentException("Switch cannot be null");
+	}
+
+	Switch sw = switches.get(swEvt.getDpid());
+
+	if (sw == null) {
+	    log.warn("Switch {} already removed, ignoreing", swEvt);
+	    return;
+	}
+
+	// Sanity check
+	if (!sw.getPorts().isEmpty()) {
+	    log.warn(
+		    "Ports on Switch {} should be removed prior to removing Switch. Removing Switch anyways",
+		    swEvt);
+	    // XXX Should we remove Port?
+	}
+	if (!sw.getDevices().isEmpty()) {
+	    log.warn(
+		    "Devices on Switch {} should be removed prior to removing Switch. Removing Switch anyways",
+		    swEvt);
+	    // XXX Should we remove Device to Switch relation?
+	}
+	if (!sw.getIncomingLinks().iterator().hasNext()) {
+	    log.warn(
+		    "IncomingLinks on Switch {} should be removed prior to removing Switch. Removing Switch anyways",
+		    swEvt);
+	    // XXX Should we remove Link?
+	}
+	if (!sw.getOutgoingLinks().iterator().hasNext()) {
+	    log.warn(
+		    "OutgoingLinks on Switch {} should be removed prior to removing Switch. Removing Switch anyways",
+		    swEvt);
+	    // XXX Should we remove Link?
+	}
+
+	boolean removed = switches.remove(swEvt.getDpid(), sw);
+	if (removed) {
+	    log.warn(
+		    "Switch instance was replaced concurrently while removing {}. Something is not right.",
+		    sw);
 	}
     }
 
     /**
-     * Deactivate and remove Switch.
+     * put Port
      *
-     * XXX Should it deactivate or delete its Ports also?
+     * XXX Internal Invariant-maintenance method. Will not write to DB. Will not
+     * fire Notification.
      *
-     * @param sw
+     * @param portEvt
      */
-    void deactivateSwitch(Switch sw) {
-	if (sw == null) {
-	    throw new IllegalArgumentException("Switch cannot be null");
-	}
-
-	if( !isSwitchInstanceInTopology(sw) ){
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format(
-			    "Switch with dpid %s did not exist or different instance registered.",
-			    new Dpid(sw.getDpid())));
-	}
-
-	// XXX Are we sure we want to deactivate Ports also?
-	for (Port p : sw.getPorts()) {
-	    deactivatePort(p);
-	}
-
-	// TODO Deactivate Switch: What to do simply remove?
-	for (Link l : sw.getIncomingLinks()) {
-	    removeLink(l);
-	}
-
-	for (Link l : sw.getOutgoingLinks()) {
-	    removeLink(l);
-	}
-
-	if (!switches.containsKey(sw.getDpid())) {
-	    throw new NoSuchElementException(String.format(
-		    "Switch with dpid %s not found.", new Dpid(sw.getDpid())));
-	}
-	boolean removed = switches.remove(sw.getDpid(), sw);
-
-	if (!removed) {
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format(
-			    "Switch with dpid %s did not exist or different instance registered.",
-			    new Dpid(sw.getDpid())));
-	}
-    }
-
-    /**
-     * Add Port to Topology.
-     *
-     * @param port
-     */
-    void addPort(Port port) {
-	if (port == null) {
+    void putPort(PortEvent portEvt) {
+	if (portEvt == null) {
 	    throw new IllegalArgumentException("Port cannot be null");
 	}
-	Switch sw = port.getSwitch();
-
-	if( !isSwitchInstanceInTopology(sw) ){
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format(
-			    "Switch with dpid %s did not exist or different instance registered.",
-			    new Dpid(sw.getDpid())));
+	Switch sw = switches.get(portEvt.getDpid());
+	if (sw == null) {
+	    throw new BrokenInvariantException(String.format(
+		    "Switch with dpid %s did not exist.",
+		    new Dpid(portEvt.getDpid())));
 	}
+	Port p = sw.getPort(portEvt.getNumber());
+	PortImpl port = null;
+	if (p != null) {
+	    port = getPortImpl(p);
+	}
+
+	if (port == null) {
+	    port = new PortImpl(this, sw, portEvt.getNumber());
+	}
+
+	// TODO update attributes
 
 	SwitchImpl s = getSwitchImpl(sw);
-
 	s.addPort(port);
-	// XXX Check If port already exist, if so then what? deactivate old?
     }
 
     /**
-     * Deactivate and remove Ports.
+     * remove Port
      *
-     * @param port
+     * XXX Internal Invariant-maintenance method. Will not write to DB. Will not
+     * fire Notification.
+     *
+     * @param portEvt
      */
-    void deactivatePort(Port port) {
-	if (port == null) {
+    void removePort(PortEvent portEvt) {
+	if (portEvt == null) {
 	    throw new IllegalArgumentException("Port cannot be null");
 	}
 
-	Switch sw = port.getSwitch();
-
-	if( !isSwitchInstanceInTopology(sw) ){
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format(
-			    "Switch with dpid %s did not exist or different instance registered.",
-			    new Dpid(sw.getDpid())));
+	Switch sw = switches.get(portEvt.getDpid());
+	if (sw == null) {
+	    log.warn("Parent Switch for Port {} already removed, ignoreing", portEvt);
+	    return;
 	}
 
+	Port p = sw.getPort(portEvt.getNumber());
+	if (p == null) {
+	    log.warn("Port {} already removed, ignoreing", portEvt);
+	    return;
+	}
+	// null check
 
-	// remove Link
-	removeLink(port.getIncomingLink());
-	removeLink(port.getOutgoingLink());
-
-	// remove Device
-	for(Device d: port.getDevices()) {
-	    removeDevice(d);
+	if (!p.getDevices().iterator().hasNext()) {
+	    log.warn(
+		    "Devices on Port {} should be removed prior to removing Port. Removing Port anyways",
+		    portEvt);
+	    // XXX Should we remove Device to Port relation?
+	}
+	if (p.getIncomingLink() != null) {
+	    log.warn(
+		    "IncomingLinks on Port {} should be removed prior to removing Port. Removing Port anyways",
+		    portEvt);
+	    // XXX Should we remove Link?
+	}
+	if (p.getOutgoingLink() != null) {
+	    log.warn(
+		    "OutgoingLinks on Port {} should be removed prior to removing Port. Removing Port anyways",
+		    portEvt);
+	    // XXX Should we remove Link?
 	}
 
 	// remove Port from Switch
-	SwitchImpl s = getSwitchImpl(sw);
-	s.removePort(port);
+	 SwitchImpl s = getSwitchImpl(sw);
+	 s.removePort(p);
     }
 
-    void addLink(Link link) {
+    /**
+     * put Link
+     *
+     * XXX Internal Invariant-maintenance method. Will not write to DB. Will not
+     * fire Notification.
+     *
+     * @param linkEvt
+     */
+    void putLink(LinkEvent linkEvt) {
+	if (linkEvt == null) {
+	    throw new IllegalArgumentException("Link cannot be null");
+	}
+	// TODO Auto-generated method stub
+
+	Switch srcSw = switches.get(linkEvt.getSrc().dpid);
+	if (srcSw == null) {
+	    throw new BrokenInvariantException(
+		    String.format(
+			    "Switch with dpid %s did not exist.",
+			    new Dpid(linkEvt.getSrc().dpid)));
+	}
+
+	Switch dstSw = switches.get(linkEvt.getDst().dpid);
+	if (dstSw == null) {
+	    throw new BrokenInvariantException(
+		    String.format(
+			    "Switch with dpid %s did not exist.",
+			    new Dpid(linkEvt.getDst().dpid)));
+	}
+
+	Port srcPort = srcSw.getPort(linkEvt.getSrc().number);
+	if (srcPort == null) {
+	    throw new BrokenInvariantException(
+		    String.format(
+			    "Src Port %s of a Link did not exist.",
+			    linkEvt.getSrc() ));
+	}
+	Port dstPort = dstSw.getPort(linkEvt.getDst().number);
+	if (dstPort == null) {
+	    throw new BrokenInvariantException(
+		    String.format(
+			    "Dst Port %s of a Link did not exist.",
+			    linkEvt.getDst() ));
+	}
+
+	Link l = dstPort.getIncomingLink();
+	LinkImpl link = null;
+	assert( l == srcPort.getOutgoingLink() );
+	if (l != null) {
+//	    link = getLink
+	}
+
+	// TODO update Switch
+	// TODO update Link
+
+
+	// XXX check Existing Link first?
+//	srcPort.setOutgoingLink(link);
+//	dstPort.setIncomingLink(link);
+    }
+
+    /**
+     * removeLink
+     *
+     * XXX Internal Invariant-maintenance method. Will not write to DB. Will not
+     * fire Notification.
+     *
+     * @param link
+     */
+    void removeLink(LinkEvent link) {
 	if (link == null) {
 	    throw new IllegalArgumentException("Link cannot be null");
 	}
+	// TODO Auto-generated method stub
 
-	Switch srcSw = link.getSourceSwitch();
-	if( !isSwitchInstanceInTopology(srcSw) ){
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format(
-			    "Switch with dpid %s did not exist or different instance registered.",
-			    new Dpid(srcSw.getDpid())));
-	}
-
-	Switch dstSw = link.getDestinationSwitch();
-	if( !isSwitchInstanceInTopology(dstSw) ){
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format(
-			    "Switch with dpid %s did not exist or different instance registered.",
-			    new Dpid(dstSw.getDpid())));
-	}
-
-	PortImpl srcPort = getPortImpl( link.getSourcePort() );
-	PortImpl dstPort = getPortImpl( link.getDestinationPort() );
-
-	// XXX check Existing Link first?
-	srcPort.setOutgoingLink(link);
-	dstPort.setIncomingLink(link);
+	// Switch srcSw = link.getSourceSwitch();
+	// if (!isSwitchInstanceInTopology(srcSw)) {
+	// // XXX Define or choose more appropriate Exception.
+	// throw new RuntimeException(
+	// String.format(
+	// "Switch with dpid %s did not exist or different instance registered.",
+	// new Dpid(srcSw.getDpid())));
+	// }
+	//
+	// Switch dstSw = link.getDestinationSwitch();
+	// if (!isSwitchInstanceInTopology(dstSw)) {
+	// // XXX Define or choose more appropriate Exception.
+	// throw new RuntimeException(
+	// String.format(
+	// "Switch with dpid %s did not exist or different instance registered.",
+	// new Dpid(dstSw.getDpid())));
+	// }
+	//
+	// PortImpl srcPort = getPortImpl(link.getSourcePort());
+	// PortImpl dstPort = getPortImpl(link.getDestinationPort());
+	//
+	// // XXX check Existing Link first?
+	// if (srcPort.getOutgoingLink() != link
+	// || dstPort.getIncomingLink() != link) {
+	// // XXX Define or choose more appropriate Exception.
+	// throw new RuntimeException(String.format(
+	// "Link %s did not belong to Topology", link.toString()));
+	// }
+	// // remove Link
+	// srcPort.setOutgoingLink(null);
+	// dstPort.setIncomingLink(null);
     }
 
-    void removeLink(Link link) {
-	if (link == null) {
-	    throw new IllegalArgumentException("Link cannot be null");
-	}
-
-	Switch srcSw = link.getSourceSwitch();
-	if( !isSwitchInstanceInTopology(srcSw) ){
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format(
-			    "Switch with dpid %s did not exist or different instance registered.",
-			    new Dpid(srcSw.getDpid())));
-	}
-
-	Switch dstSw = link.getDestinationSwitch();
-	if( !isSwitchInstanceInTopology(dstSw) ){
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format(
-			    "Switch with dpid %s did not exist or different instance registered.",
-			    new Dpid(dstSw.getDpid())));
-	}
-
-	PortImpl srcPort = getPortImpl( link.getSourcePort() );
-	PortImpl dstPort = getPortImpl( link.getDestinationPort() );
-
-	// XXX check Existing Link first?
-	if( srcPort.getOutgoingLink() != link || dstPort.getIncomingLink() != link) {
-	    // XXX Define or choose more appropriate Exception.
-	    throw new RuntimeException(
-		    String.format("Link %s did not belong to Topology", link.toString())
-		    );
-	}
-	// remove Link
-	srcPort.setOutgoingLink(null);
-	dstPort.setIncomingLink(null);
-    }
-
-    void updateDevice(Device device) {
-	if (device == null) {
+    // XXX Need to rework Device related
+    /**
+     * Add new device to DB
+     *
+     * @param device
+     */
+    void updateDevice(DeviceEvent deviceToUpdate,
+	    Set<InetAddress> updatedIpAddrs, Set<Port> updatedAttachmentPoints) {
+	if (deviceToUpdate == null) {
 	    throw new IllegalArgumentException("Device cannot be null");
 	}
 	// TODO Auto-generated method stub
 
+	// Device existingDevice =
+	// getDeviceByMac(deviceToUpdate.getMacAddress());
+	// if (existingDevice != deviceToUpdate) {
+	// throw new IllegalArgumentException(
+	// "Must supply Device Object in this NetworkGraph");
+	// }
+	//
+	// DeviceImpl device = getDeviceImpl(deviceToUpdate);
+	//
+	// // Update IP Addr
+	// // uniq
+	// Set<InetAddress> prevAddrs = new HashSet<>(
+	// deviceToUpdate.getIpAddress());
+	// Set<InetAddress> newAddrs = updatedIpAddrs;
+	//
+	// // delta
+	// @SuppressWarnings("unchecked")
+	// Collection<InetAddress> delAddr = CollectionUtils.subtract(newAddrs,
+	// prevAddrs);
+	// @SuppressWarnings("unchecked")
+	// Collection<InetAddress> addAddr = CollectionUtils.subtract(prevAddrs,
+	// newAddrs);
+	//
+	// for (InetAddress addr : delAddr) {
+	// Set<Device> devices = addr2Device.get(addr);
+	// if (devices == null) {
+	// continue;
+	// }
+	// devices.remove(device);
+	// device.removeIpAddress(addr);
+	// }
+	// for (InetAddress addr : addAddr) {
+	// Set<Device> devices = addr2Device.get(addr);
+	// if (devices == null) {
+	// devices = new HashSet<>();
+	// addr2Device.put(addr, devices);
+	// }
+	// devices.add(device);
+	// device.addIpAddress(addr);
+	// }
+	//
+	// // Update Attachment Point
+	// // uniq
+	// Set<Port> prevPorts = new HashSet<>();
+	// CollectionUtils.addAll(prevAddrs,
+	// deviceToUpdate.getAttachmentPoints()
+	// .iterator());
+	// Set<Port> newPorts = updatedAttachmentPoints;
+	// // delta
+	// @SuppressWarnings("unchecked")
+	// Collection<Port> delPorts = CollectionUtils.subtract(newPorts,
+	// prevPorts);
+	// @SuppressWarnings("unchecked")
+	// Collection<Port> addPorts = CollectionUtils.subtract(prevPorts,
+	// newPorts);
+	//
+	// for (Port p : delPorts) {
+	// device.removeAttachmentPoint(p);
+	// getPortImpl(p).removeDevice(device);
+	// }
+	//
+	// for (Port p : addPorts) {
+	// device.addAttachmentPoint(p);
+	// getPortImpl(p).addDevice(device);
+	// }
+
+	// TODO Auto-generated method stub
+
     }
 
-    void removeDevice(Device device) {
+    void removeDevice(DeviceEvent device) {
 	if (device == null) {
 	    throw new IllegalArgumentException("Device cannot be null");
 	}
@@ -266,11 +425,11 @@
     }
 
     public boolean isSwitchInstanceInTopology(Switch sw) {
-        // check if the sw instance is valid in Topology
-        if (sw != switches.get(sw.getDpid())) {
-            return false;
-        }
-        return true;
+	// check if the sw instance is valid in Topology
+	if (sw != switches.get(sw.getDpid())) {
+	    return false;
+	}
+	return true;
     }
 
     public void loadWholeTopologyFromDB() {
@@ -279,11 +438,9 @@
 	for (RCSwitch sw : RCSwitch.getAllSwitches()) {
 	    try {
 		sw.read();
-		// TODO SwitchImpl probably should have a constructor from
-		// RCSwitch
-		SwitchImpl memSw = new SwitchImpl(this, sw.getDpid());
-
-		addSwitch(memSw);
+		// TODO if there is going to be inactive Switch in DB, skip
+		// TODO update other attributes if there exist any
+		putSwitch(new SwitchEvent(sw.getDpid()));
 	    } catch (ObjectDoesntExistException e) {
 		log.error("Read Switch Failed, skipping", e);
 	    }
@@ -293,16 +450,15 @@
 	    try {
 		p.read();
 
-		// TODO PortImpl probably should have a constructor from RCPort
 		Switch sw = this.getSwitch(p.getDpid());
 		if (sw == null) {
 		    log.error("Switch {} missing when adding Port {}",
 			    new Dpid(p.getDpid()), p);
 		    continue;
 		}
-		PortImpl memPort = new PortImpl(this, sw, p.getNumber());
-
-		addPort(memPort);
+		PortEvent portEvent = new PortEvent(p.getDpid(), p.getNumber());
+		// TODO update other attributes if there exist any
+		putPort(portEvent);
 	    } catch (ObjectDoesntExistException e) {
 		log.error("Read Port Failed, skipping", e);
 	    }
@@ -336,14 +492,30 @@
 		    continue;
 		}
 
-		LinkImpl memLink = new LinkImpl(this,
-			srcSw.getPort(l.getSrc().number), dstSw.getPort(l
-				.getDst().number));
-
-		addLink(memLink);
+		LinkEvent linkEvent = new LinkEvent(l.getSrc().dpid,
+			l.getSrc().number, l.getDst().dpid, l.getDst().number);
+		// TODO update other attributes if there exist any
+		putLink(linkEvent);
 	    } catch (ObjectDoesntExistException e) {
 		log.debug("Delete Link Failed", e);
 	    }
 	}
     }
+
+    /**
+     * Exception to be thrown when Modification to the Network Graph cannot be continued due to broken invariant.
+     *
+     * XXX Should this be checked exception or RuntimeException
+     */
+    public static class BrokenInvariantException extends RuntimeException {
+	private static final long serialVersionUID = 1L;
+
+	public BrokenInvariantException() {
+	    super();
+	}
+
+	public BrokenInvariantException(String message) {
+	    super(message);
+	}
+    }
 }
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Switch.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Switch.java
index 717d241..b1dce75 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Switch.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Switch.java
@@ -25,5 +25,6 @@
 
 	public Link getLinkToNeighbor(Long dpid);
 
+	// XXX Iterable or Collection?
 	public Collection<Device> getDevices();
 }
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/SwitchImpl.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/SwitchImpl.java
index 4e8c661..6d98740 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/SwitchImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/SwitchImpl.java
@@ -53,7 +53,7 @@
 	@Override
 	public Link getLinkToNeighbor(Long neighborDpid) {
 		for (Link link : graph.getOutgoingLinksFromSwitch(dpid)) {
-			if (link.getDestinationSwitch().getDpid() == neighborDpid) {
+			if (link.getDestinationSwitch().getDpid().equals(neighborDpid) ) {
 				return link;
 			}
 		}