Endpoint processing more scalable

 - no processing was triggered after removing location
   augmentation from an endpoint
 - processing of events made more scalable
 - added test cases for removing/adding location
   augmentation from/to an endpoint

Change-Id: I5a9295a3e8b16d60921359887e9361302e926f55
Signed-off-by: Tomas Cechvala <tcechval@cisco.com>

diff --git a/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/endpoint/EndpointManager.java b/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/endpoint/EndpointManager.java
index fa7dcc70ccb9c0c4a2da3cfae2d0645e1c412bb1..d2cbbfbec370f23dee6673e197f1106ad6ba6f57 100644 (file)
--- a/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/endpoint/EndpointManager.java
+++ b/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/endpoint/EndpointManager.java
@@ -307,62 +307,67 @@ public class EndpointManager implements AutoCloseable {
         boolean notifyOldEg = false;
         boolean notifyNewEg = false;
 
-        // maintain external endpoints map
-        if (newLoc == null && oldLoc == null) {
-            if (newEp != null && newEpKey != null) {
+        // create endpoint
+        if (oldEp == null && newEp != null) {
+            if (newLoc != null) {
+                createEndpoint(newLoc, newEpKey, newEpgIds, tenantId);
+                endpoints.put(newEpKey, newEp);
+                notifyEndpointUpdated(newEpKey);
+                notifyNewLoc = true;
+                notifyNewEg = true;
+            } else {
                 externalEndpointsWithoutLocation.put(newEpKey, newEp);
             }
-            if (oldEp != null && oldEpKey != null) {
-                externalEndpointsWithoutLocation.remove(oldEpKey);
-            }
-            return;
-        }
-
-        // maintain endpoint maps
-        // new endpoint
-        if (newEp != null && newEpKey != null) {
-            endpoints.put(newEpKey, newEp);
-            // TODO This and the create endpoint / update endpoint section will cause switchUpdate
-            // to be called twice. Consider removing
-            notifyEndpointUpdated(newEpKey);
-        }
-        // odl endpoint
-        else if (oldEpKey != null) {
-            endpoints.remove(oldEpKey);
-            // TODO This and the create endpoint / update endpoint section will cause switchUpdate
-            // to be called twice. Consider removing
-            notifyEndpointUpdated(oldEpKey);
-        }
-
-        // create endpoint
-        if (oldEp == null && newEp != null && newLoc != null) {
-            createEndpoint(newLoc, newEpKey, newEpgIds, tenantId);
-            notifyNewLoc = true;
-            notifyNewEg = true;
         }
 
         // update endpoint
-        if (oldEp != null && newEp != null && oldEpKey != null && newEpKey != null && newLoc != null
-                && (oldEpKey.toString().equals(newEpKey.toString()))) {
-
-            // endpoint was moved, new location exists but is different from old one
-            if (oldLoc != null && !(oldLoc.getValue().equals(newLoc.getValue()))) {
-                // remove old endpoint
+        else if (oldEp != null && newEp != null && oldEpKey != null && newEpKey != null) {
+            // endpoint is not external anymore
+            if (newLoc != null && oldLoc == null) {
+                createEndpoint(newLoc, newEpKey, newEpgIds, tenantId);
+                externalEndpointsWithoutLocation.remove(oldEpKey);
+                endpoints.put(newEpKey, newEp);
+                notifyEndpointUpdated(newEpKey);
+                notifyNewLoc = true;
+                notifyNewEg = true;
+            }
+            // endpoint changed to external
+            else if (newLoc == null && oldLoc != null) {
                 removeEndpoint(oldEp, oldLoc, oldEpKey, oldEpgIds);
+                externalEndpointsWithoutLocation.put(newEpKey, newEp);
+                endpoints.remove(oldEpKey);
+                notifyEndpointUpdated(oldEpKey);
                 notifyOldLoc = true;
                 notifyOldEg = true;
+            // endpoint might have changed location, EPGs or it's properties
+            } else if (newLoc != null && oldLoc != null) {
+                    // endpoit changed location
+                    if (!(oldLoc.getValue().equals(newLoc.getValue()))) {
+                        notifyOldLoc = true;
+                        notifyNewLoc = true;
+                    }
+                    // endpoint changed EPGs
+                    if (!oldEpgIds.equals(newEpgIds)) {
+                        notifyOldEg = true;
+                        notifyNewEg = true;
+                    }
+                    removeEndpoint(oldEp, oldLoc, oldEpKey, oldEpgIds);
+                    createEndpoint(newLoc, newEpKey, newEpgIds, tenantId);
+                    notifyEndpointUpdated(newEpKey);
             }
-            // add moved endpoint
-            createEndpoint(newLoc, newEpKey, newEpgIds, tenantId);
-            notifyNewLoc = true;
-            notifyNewEg = true;
         }
 
         // remove endpoint
-        if (oldEp != null && newEp == null) {
-            removeEndpoint(oldEp, oldLoc, oldEpKey, oldEpgIds);
-            notifyOldLoc = true;
-            notifyOldEg = true;
+        else if (oldEp != null && newEp == null) {
+            if (oldLoc != null) {
+                removeEndpoint(oldEp, oldLoc, oldEpKey, oldEpgIds);
+                endpoints.remove(oldEpKey);
+                notifyEndpointUpdated(oldEpKey);
+                notifyOldLoc = true;
+                notifyOldEg = true;
+            } else {
+                externalEndpointsWithoutLocation.remove(oldEpKey);
+            }
         }
 
         // notifications

diff --git a/renderers/ofoverlay/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/endpoint/EndpointManagerTest.java b/renderers/ofoverlay/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/endpoint/EndpointManagerTest.java
index 70af322c795ec9e754927c33c614927fb8c6a0fb..a6d96dcc2077834e98304f41a12104cf08860e8f 100644 (file)
--- a/renderers/ofoverlay/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/endpoint/EndpointManagerTest.java
+++ b/renderers/ofoverlay/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/endpoint/EndpointManagerTest.java
@@ -541,16 +541,95 @@ public class EndpointManagerTest {
     public void updateEndpointTestNewLocNullOldLocNullExternalRemove() {
         when(context1.getNodeId()).thenReturn(null);
         when(context1.getLocationType()).thenReturn(LocationType.External);
+        manager.processEndpoint(null, endpoint1);
 
         manager.processEndpoint(endpoint1, null);
         verify(endpointListener, never()).endpointUpdated(any(EpKey.class));
         verify(endpointListener, never()).nodeEndpointUpdated(any(NodeId.class), any(EpKey.class));
     }
 
+    /**
+     * Endpoint changes it's location
+     */
+    @Test
+    public void updateEndpointLocationTestUpdate() {
+        Collection<Endpoint> collection;
+        manager.processEndpoint(null, endpoint1);
+
+        manager.processEndpoint(endpoint1, endpoint2);
+        verify(endpointListener, times(2)).endpointUpdated(any(EpKey.class));
+        // create: node1, update: node1 -> node2
+        verify(endpointListener, times(3)).nodeEndpointUpdated(any(NodeId.class), any(EpKey.class));
+        collection = manager.getEndpointsForGroup(new EgKey(tenantId, endpointGroupId));
+        Assert.assertFalse(collection.isEmpty());
+    }
+
+    /**
+     * Endpoint changes it's EPG
+     */
+    @Test
+    public void updateEndpointGroupTestUpdate() {
+        Collection<Endpoint> collection;
+        EndpointGroupId otherEndpointGroupId = mock(EndpointGroupId.class);
+        when(endpoint2.getEndpointGroup()).thenReturn(otherEndpointGroupId);
+        when(endpoint2.getAugmentation(OfOverlayContext.class)).thenReturn(context1);
+        manager.processEndpoint(null, endpoint1);
+
+        manager.processEndpoint(endpoint1, endpoint2);
+        verify(endpointListener, times(2)).endpointUpdated(any(EpKey.class));
+        verify(endpointListener, times(1)).nodeEndpointUpdated(any(NodeId.class), any(EpKey.class));
+        collection = manager.getEndpointsForGroup(new EgKey(tenantId, endpointGroupId));
+        Assert.assertTrue(collection.isEmpty());
+        collection = manager.getEndpointsForGroup(new EgKey(tenantId, otherEndpointGroupId));
+        Assert.assertFalse(collection.isEmpty());
+    }
+
+    /**
+     * Endpoint changes it's location and EPGs
+     */
     @Test
-    public void updateEndpointTestUpdate() {
+    public void updateEndpointLocationAndGroupTestUpdate() {
         Collection<Endpoint> collection;
-        when(nodeId2.getValue()).thenReturn("nodeValue1");
+        EndpointGroupId otherEndpointGroupId = mock(EndpointGroupId.class);
+        when(endpoint2.getEndpointGroup()).thenReturn(otherEndpointGroupId);
+        manager.processEndpoint(null, endpoint1);
+
+        manager.processEndpoint(endpoint1, endpoint2);
+        verify(endpointListener, times(2)).endpointUpdated(any(EpKey.class));
+        // create: node1, update: node1 -> node2
+        verify(endpointListener, times(3)).nodeEndpointUpdated(any(NodeId.class), any(EpKey.class));
+        collection = manager.getEndpointsForGroup(new EgKey(tenantId, endpointGroupId));
+        Assert.assertTrue(collection.isEmpty());
+        collection = manager.getEndpointsForGroup(new EgKey(tenantId, otherEndpointGroupId));
+        Assert.assertFalse(collection.isEmpty());
+    }
+
+    /**
+     * Endpoint becomes external when removing it's location augmentation.
+     * This might happen when an endpoint is removed from a device.
+     */
+    @Test
+    public void updateEndpointLocationRemovedTestUpdate() {
+        Collection<Endpoint> collection;
+        when(endpoint2.getAugmentation(OfOverlayContext.class)).thenReturn(null);
+        manager.processEndpoint(null, endpoint1);
+
+        manager.processEndpoint(endpoint1, endpoint2);
+        verify(endpointListener, times(2)).endpointUpdated(any(EpKey.class));
+        verify(endpointListener, times(2)).nodeEndpointUpdated(any(NodeId.class), any(EpKey.class));
+        collection = manager.getEndpointsForGroup(new EgKey(tenantId, endpointGroupId));
+        Assert.assertTrue(collection.isEmpty());
+    }
+
+    /**
+     * Endpoint is created when adding location augmentation.
+     * Endpoint is not external anymore.
+     */
+    @Test
+    public void updateEndpointLocationAddedTestUpdate() {
+        Collection<Endpoint> collection;
+        when(endpoint1.getAugmentation(OfOverlayContext.class)).thenReturn(null);
+        manager.processEndpoint(null, endpoint1);
 
         manager.processEndpoint(endpoint1, endpoint2);
         verify(endpointListener).endpointUpdated(any(EpKey.class));