Included connect point port number in definition of UiLinkId.
Added dumpString() to ModelCache / UiTopology.
Added more unit tests for ModelCache.

Change-Id: I842bb418b25cc901bd12bc28c6660c836f7235bc
diff --git a/web/gui/src/main/java/org/onosproject/ui/impl/topo/model/ModelCache.java b/web/gui/src/main/java/org/onosproject/ui/impl/topo/model/ModelCache.java
index a9981e7..19ab015 100644
--- a/web/gui/src/main/java/org/onosproject/ui/impl/topo/model/ModelCache.java
+++ b/web/gui/src/main/java/org/onosproject/ui/impl/topo/model/ModelCache.java
@@ -197,6 +197,11 @@
         postEvent(REGION_ADDED_OR_UPDATED, uiRegion);
     }
 
+    // package private for unit test access
+    UiRegion accessRegion(RegionId id) {
+        return uiTopology.findRegion(id);
+    }
+
     // invoked from UiSharedTopologyModel region listener
     void removeRegion(Region region) {
         RegionId id = region.id();
@@ -241,6 +246,11 @@
         postEvent(DEVICE_ADDED_OR_UPDATED, uiDevice);
     }
 
+    // package private for unit test access
+    UiDevice accessDevice(DeviceId id) {
+        return uiTopology.findDevice(id);
+    }
+
     // invoked from UiSharedTopologyModel device listener
     void removeDevice(Device device) {
         DeviceId id = device.id();
@@ -290,6 +300,11 @@
         postEvent(LINK_ADDED_OR_UPDATED, uiLink);
     }
 
+    // package private for unit test access
+    UiLink accessLink(UiLinkId id) {
+        return uiTopology.findLink(id);
+    }
+
     // invoked from UiSharedTopologyModel link listener
     void removeLink(Link link) {
         UiLinkId id = uiLinkId(link);
@@ -310,36 +325,55 @@
 
     // === HOSTS
 
+    private EdgeLink synthesizeLink(Host h) {
+        return createEdgeLink(h, true);
+    }
+
     private UiHost addNewHost(Host h) {
         UiHost host = new UiHost(uiTopology, h);
         uiTopology.add(host);
 
-        UiLink edgeLink = addNewEdgeLink(host);
-        host.setEdgeLinkId(edgeLink.id());
+        EdgeLink elink = synthesizeLink(h);
+        UiLinkId elinkId = uiLinkId(elink);
+        host.setEdgeLinkId(elinkId);
+
+        // add synthesized edge link to the topology
+        UiLink edgeLink = addNewLink(elinkId);
+        edgeLink.attachEdgeLink(elink);
 
         return host;
     }
 
-    private void removeOldEdgeLink(UiHost uiHost) {
-        UiLink old = uiTopology.findLink(uiHost.edgeLinkId());
-        if (old != null) {
-            uiTopology.remove(old);
-        }
-    }
+    private void insertNewUiLink(UiLinkId id, EdgeLink e) {
+        UiLink newEdgeLink = addNewLink(id);
+        newEdgeLink.attachEdgeLink(e);
 
-    private UiLink addNewEdgeLink(UiHost uiHost) {
-        EdgeLink elink = createEdgeLink(uiHost.backingHost(), true);
-        UiLinkId elinkId = UiLinkId.uiLinkId(elink);
-        UiLink uiLink = addNewLink(elinkId);
-        uiLink.attachEdgeLink(elink);
-        return uiLink;
     }
 
     private void updateHost(UiHost uiHost, Host h) {
-        removeOldEdgeLink(uiHost);
+        UiLink existing = uiTopology.findLink(uiHost.edgeLinkId());
+
+        EdgeLink currentElink = synthesizeLink(h);
+        UiLinkId currentElinkId = uiLinkId(currentElink);
+
+        if (existing != null) {
+            if (!currentElinkId.equals(existing.id())) {
+                // edge link has changed
+                insertNewUiLink(currentElinkId, currentElink);
+                uiHost.setEdgeLinkId(currentElinkId);
+
+                uiTopology.remove(existing);
+            }
+
+        } else {
+            // no previously existing edge link
+            insertNewUiLink(currentElinkId, currentElink);
+            uiHost.setEdgeLinkId(currentElinkId);
+
+        }
+
         HostLocation hloc = h.location();
         uiHost.setLocation(hloc.deviceId(), hloc.port());
-        addNewEdgeLink(uiHost);
     }
 
     private void loadHosts() {
@@ -364,9 +398,17 @@
     // invoked from UiSharedTopologyModel host listener
     void moveHost(Host host, Host prevHost) {
         UiHost uiHost = uiTopology.findHost(prevHost.id());
-        updateHost(uiHost, host);
+        if (uiHost != null) {
+            updateHost(uiHost, host);
+            postEvent(HOST_MOVED, uiHost);
+        } else {
+            log.warn(E_NO_ELEMENT, "host", prevHost.id());
+        }
+    }
 
-        postEvent(HOST_MOVED, uiHost);
+    // package private for unit test access
+    UiHost accessHost(HostId id) {
+        return uiTopology.findHost(id);
     }
 
     // invoked from UiSharedTopologyModel host listener
@@ -374,8 +416,9 @@
         HostId id = host.id();
         UiHost uiHost = uiTopology.findHost(id);
         if (uiHost != null) {
+            UiLink edgeLink = uiTopology.findLink(uiHost.edgeLinkId());
+            uiTopology.remove(edgeLink);
             uiTopology.remove(uiHost);
-            removeOldEdgeLink(uiHost);
             postEvent(HOST_REMOVED, uiHost);
         } else {
             log.warn(E_NO_ELEMENT, "host", id);
@@ -386,6 +429,15 @@
     // === CACHE STATISTICS
 
     /**
+     * Returns a detailed (multi-line) string showing the contents of the cache.
+     *
+     * @return detailed string
+     */
+    public String dumpString() {
+        return uiTopology.dumpString();
+    }
+
+    /**
      * Returns the number of members in the cluster.
      *
      * @return number of cluster members
@@ -402,4 +454,31 @@
     public int regionCount() {
         return uiTopology.regionCount();
     }
+
+    /**
+     * Returns the number of devices in the topology.
+     *
+     * @return number of devices
+     */
+    public int deviceCount() {
+        return uiTopology.deviceCount();
+    }
+
+    /**
+     * Returns the number of links in the topology.
+     *
+     * @return number of links
+     */
+    public int linkCount() {
+        return uiTopology.linkCount();
+    }
+
+    /**
+     * Returns the number of hosts in the topology.
+     *
+     * @return number of hosts
+     */
+    public int hostCount() {
+        return uiTopology.hostCount();
+    }
 }
diff --git a/web/gui/src/test/java/org/onosproject/ui/impl/topo/model/AbstractTopoModelTest.java b/web/gui/src/test/java/org/onosproject/ui/impl/topo/model/AbstractTopoModelTest.java
index 98fc07d..9fad51c 100644
--- a/web/gui/src/test/java/org/onosproject/ui/impl/topo/model/AbstractTopoModelTest.java
+++ b/web/gui/src/test/java/org/onosproject/ui/impl/topo/model/AbstractTopoModelTest.java
@@ -136,15 +136,15 @@
     protected static final Set<Region> REGION_SET =
             ImmutableSet.of(REGION_1, REGION_2, REGION_3);
 
-    protected static final String D1 = "D1";
-    protected static final String D2 = "D2";
-    protected static final String D3 = "D3";
-    protected static final String D4 = "D4";
-    protected static final String D5 = "D5";
-    protected static final String D6 = "D6";
-    protected static final String D7 = "D7";
-    protected static final String D8 = "D8";
-    protected static final String D9 = "D9";
+    protected static final String D1 = "d1";
+    protected static final String D2 = "d2";
+    protected static final String D3 = "d3";
+    protected static final String D4 = "d4";
+    protected static final String D5 = "d5";
+    protected static final String D6 = "d6";
+    protected static final String D7 = "d7";
+    protected static final String D8 = "d8";
+    protected static final String D9 = "d9";
 
     protected static final String MFR = "Mfr";
     protected static final String HW = "h/w";
@@ -303,7 +303,6 @@
     private static final HostService MOCK_HOST = new MockHostService();
 
 
-
     private static class MockClusterService extends ClusterServiceAdapter {
         private final Map<NodeId, ControllerNode> nodes = new HashMap<>();
         private final Map<NodeId, ControllerNode.State> states = new HashMap<>();
@@ -470,38 +469,46 @@
 
     }
 
+    /**
+     * Synthesizes a pair of unidirectional links between two devices. The
+     * string array should be of the form:
+     * <pre>
+     *     { "device-A-id", "device-A-port", "device-B-id", "device-B-port" }
+     * </pre>
+     *
+     * @param linkPairData device ids and ports
+     * @return pair of synthesized links
+     */
+    protected static List<Link> makeLinkPair(String[] linkPairData) {
+        DeviceId devA = deviceId(linkPairData[0]);
+        PortNumber portA = portNumber(Long.valueOf(linkPairData[1]));
+        DeviceId devB = deviceId(linkPairData[2]);
+        PortNumber portB = portNumber(Long.valueOf(linkPairData[3]));
+
+        Link linkA = DefaultLink.builder()
+                .providerId(ProviderId.NONE)
+                .type(Link.Type.DIRECT)
+                .src(new ConnectPoint(devA, portA))
+                .dst(new ConnectPoint(devB, portB))
+                .build();
+
+        Link linkB = DefaultLink.builder()
+                .providerId(ProviderId.NONE)
+                .type(Link.Type.DIRECT)
+                .src(new ConnectPoint(devB, portB))
+                .dst(new ConnectPoint(devA, portA))
+                .build();
+
+        return ImmutableList.of(linkA, linkB);
+    }
 
     private static class MockLinkService extends LinkServiceAdapter {
         private final Set<Link> links = new HashSet<>();
 
         MockLinkService() {
             for (String[] linkPair : LINK_CONNECT_DATA) {
-                links.addAll(makeLinks(linkPair));
+                links.addAll(makeLinkPair(linkPair));
             }
-
-        }
-
-        private Set<Link> makeLinks(String[] linkPair) {
-            DeviceId devA = deviceId(linkPair[0]);
-            PortNumber portA = portNumber(Long.valueOf(linkPair[1]));
-            DeviceId devB = deviceId(linkPair[2]);
-            PortNumber portB = portNumber(Long.valueOf(linkPair[3]));
-
-            Link linkA = DefaultLink.builder()
-                    .providerId(ProviderId.NONE)
-                    .type(Link.Type.DIRECT)
-                    .src(new ConnectPoint(devA, portA))
-                    .dst(new ConnectPoint(devB, portB))
-                    .build();
-
-            Link linkB = DefaultLink.builder()
-                    .providerId(ProviderId.NONE)
-                    .type(Link.Type.DIRECT)
-                    .src(new ConnectPoint(devB, portB))
-                    .dst(new ConnectPoint(devA, portA))
-                    .build();
-
-            return ImmutableSet.of(linkA, linkB);
         }
 
         @Override
diff --git a/web/gui/src/test/java/org/onosproject/ui/impl/topo/model/ModelCacheTest.java b/web/gui/src/test/java/org/onosproject/ui/impl/topo/model/ModelCacheTest.java
index 752f28b..5ab7343 100644
--- a/web/gui/src/test/java/org/onosproject/ui/impl/topo/model/ModelCacheTest.java
+++ b/web/gui/src/test/java/org/onosproject/ui/impl/topo/model/ModelCacheTest.java
@@ -20,16 +20,27 @@
 import org.junit.Test;
 import org.onosproject.event.Event;
 import org.onosproject.event.EventDispatcher;
+import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.Link;
+import org.onosproject.net.region.Region;
 import org.onosproject.ui.impl.topo.model.UiModelEvent.Type;
 import org.onosproject.ui.model.topo.UiClusterMember;
+import org.onosproject.ui.model.topo.UiDevice;
 import org.onosproject.ui.model.topo.UiElement;
+import org.onosproject.ui.model.topo.UiLink;
+import org.onosproject.ui.model.topo.UiLinkId;
+import org.onosproject.ui.model.topo.UiRegion;
+
+import java.util.Collection;
+import java.util.Iterator;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.onosproject.cluster.NodeId.nodeId;
+import static org.onosproject.ui.model.topo.UiLinkId.uiLinkId;
 
 /**
  * Unit tests for {@link ModelCache}.
@@ -64,6 +75,12 @@
 
     private ModelCache cache;
 
+    private void assertContains(String msg, Collection<?> coll, Object... things) {
+        for (Object o : things) {
+            assertTrue(msg, coll.contains(o));
+        }
+    }
+
     @Before
     public void setUp() {
         cache = new ModelCache(MOCK_SERVICES, dispatcher);
@@ -98,6 +115,20 @@
     }
 
     @Test
+    public void nonExistentClusterMember() {
+        title("nonExistentClusterMember");
+        cache.addOrUpdateClusterMember(CNODE_1);
+        print(cache);
+        assertEquals("unex # members", 1, cache.clusterMemberCount());
+        dispatcher.assertEventCount(1);
+        dispatcher.assertLast(Type.CLUSTER_MEMBER_ADDED_OR_UPDATED, C1);
+
+        cache.removeClusterMember(CNODE_2);
+        assertEquals("unex # members", 1, cache.clusterMemberCount());
+        dispatcher.assertEventCount(1);
+    }
+
+    @Test
     public void createThreeNodeCluster() {
         title("createThreeNodeCluster");
         cache.addOrUpdateClusterMember(CNODE_1);
@@ -146,23 +177,145 @@
     public void addNodeAndDevices() {
         title("addNodeAndDevices");
         cache.addOrUpdateClusterMember(CNODE_1);
+        dispatcher.assertLast(Type.CLUSTER_MEMBER_ADDED_OR_UPDATED, C1);
         cache.addOrUpdateDevice(DEV_1);
+        dispatcher.assertLast(Type.DEVICE_ADDED_OR_UPDATED, D1);
         cache.addOrUpdateDevice(DEV_2);
+        dispatcher.assertLast(Type.DEVICE_ADDED_OR_UPDATED, D2);
         cache.addOrUpdateDevice(DEV_3);
+        dispatcher.assertLast(Type.DEVICE_ADDED_OR_UPDATED, D3);
+        dispatcher.assertEventCount(4);
         print(cache);
+
+        assertEquals("unex # nodes", 1, cache.clusterMemberCount());
+        assertEquals("unex # devices", 3, cache.deviceCount());
+        cache.removeDevice(DEV_4);
+        assertEquals("unex # devices", 3, cache.deviceCount());
+        dispatcher.assertEventCount(4);
+
+        cache.removeDevice(DEV_2);
+        dispatcher.assertLast(Type.DEVICE_REMOVED, D2);
+        dispatcher.assertEventCount(5);
+
+        // check out details of device
+        UiDevice dev = cache.accessDevice(DEVID_1);
+        assertEquals("wrong id", D1, dev.idAsString());
+        assertEquals("wrong region", R1, dev.regionId().toString());
+        Device d = dev.backingDevice();
+        assertEquals("wrong serial", SERIAL, d.serialNumber());
     }
 
     @Test
     public void addRegions() {
         title("addRegions");
         cache.addOrUpdateRegion(REGION_1);
+        dispatcher.assertLast(Type.REGION_ADDED_OR_UPDATED, R1);
+        dispatcher.assertEventCount(1);
+        assertEquals("unex # regions", 1, cache.regionCount());
+
+        cache.addOrUpdateRegion(REGION_2);
+        dispatcher.assertLast(Type.REGION_ADDED_OR_UPDATED, R2);
+        dispatcher.assertEventCount(2);
+        assertEquals("unex # regions", 2, cache.regionCount());
+
         print(cache);
+
+        cache.removeRegion(REGION_3);
+        dispatcher.assertEventCount(2);
+        assertEquals("unex # regions", 2, cache.regionCount());
+
+        cache.removeRegion(REGION_1);
+        dispatcher.assertLast(Type.REGION_REMOVED, R1);
+        dispatcher.assertEventCount(3);
+        assertEquals("unex # regions", 1, cache.regionCount());
+
+        print(cache);
+
+        UiRegion region = cache.accessRegion(REGION_2.id());
+        assertEquals("wrong id", REGION_2.id(), region.id());
+        assertEquals("unex # device IDs", 3, region.deviceIds().size());
+        assertContains("missing ID", region.deviceIds(), DEVID_4, DEVID_5, DEVID_6);
+        Region r = region.backingRegion();
+        print(r);
+        assertEquals("wrong region name", "Region-R2", r.name());
+    }
+
+    private static final String[] LINKS_2_7 = {D2, "27", D7, "72"};
+
+    @Test
+    public void addLinks() {
+        title("addLinks");
+
+        Iterator<Link> iter = makeLinkPair(LINKS_2_7).iterator();
+        Link link1 = iter.next();
+        Link link2 = iter.next();
+        print(link1);
+        print(link2);
+
+        UiLinkId idA2B = uiLinkId(link1);
+        UiLinkId idB2A = uiLinkId(link2);
+        // remember, link IDs are canonicalized
+        assertEquals("not same link ID", idA2B, idB2A);
+
+        // we've established that the ID is the same for both
+        UiLinkId linkId = idA2B;
+
+        cache.addOrUpdateLink(link1);
+        dispatcher.assertLast(Type.LINK_ADDED_OR_UPDATED, linkId.toString());
+        dispatcher.assertEventCount(1);
+        assertEquals("unex # links", 1, cache.linkCount());
+
+        UiLink link = cache.accessLink(linkId);
+        assertEquals("dev A not d2", DEVID_2, link.deviceA());
+        assertEquals("dev B not d7", DEVID_7, link.deviceB());
+        assertEquals("wrong backing link A-B", link1, link.linkAtoB());
+        assertEquals("backing link B-A?", null, link.linkBtoA());
+
+        cache.addOrUpdateLink(link2);
+        dispatcher.assertLast(Type.LINK_ADDED_OR_UPDATED, linkId.toString());
+        dispatcher.assertEventCount(2);
+        // NOTE: yes! expect 1 UiLink
+        assertEquals("unex # links", 1, cache.linkCount());
+
+        link = cache.accessLink(linkId);
+        assertEquals("dev A not d2", DEVID_2, link.deviceA());
+        assertEquals("dev B not d7", DEVID_7, link.deviceB());
+        assertEquals("wrong backing link A-B", link1, link.linkAtoB());
+        assertEquals("wrong backing link B-A", link2, link.linkBtoA());
+
+        // now remove links one at a time
+        cache.removeLink(link1);
+        // NOTE: yes! ADD_OR_UPDATE, since the link was updated
+        dispatcher.assertLast(Type.LINK_ADDED_OR_UPDATED, linkId.toString());
+        dispatcher.assertEventCount(3);
+        // NOTE: yes! expect 1 UiLink (still)
+        assertEquals("unex # links", 1, cache.linkCount());
+
+        link = cache.accessLink(linkId);
+        assertEquals("dev A not d2", DEVID_2, link.deviceA());
+        assertEquals("dev B not d7", DEVID_7, link.deviceB());
+        assertEquals("backing link A-B?", null, link.linkAtoB());
+        assertEquals("wrong backing link B-A", link2, link.linkBtoA());
+
+        // remove final link
+        cache.removeLink(link2);
+        dispatcher.assertLast(Type.LINK_REMOVED, linkId.toString());
+        dispatcher.assertEventCount(4);
+        // NOTE: finally link should be removed from cache
+        assertEquals("unex # links", 0, cache.linkCount());
     }
 
     @Test
     public void load() {
         title("load");
         cache.load();
-        print(cache);
+        print(cache.dumpString());
+
+        // See mock service bundle for expected values (AbstractTopoModelTest)
+        assertEquals("unex # cnodes", 3, cache.clusterMemberCount());
+        assertEquals("unex # regions", 3, cache.regionCount());
+        assertEquals("unex # devices", 9, cache.deviceCount());
+        assertEquals("unex # hosts", 18, cache.hostCount());
+        assertEquals("unex # hosts", 26, cache.linkCount());
     }
 }