Bug 6745 SimplifiedOperationalListener optimation

	- optimize processNodeModification method
	- update test

Change-Id: I941a84bff4322c2b2b64f8404791ec6d00a9d200
Signed-off-by: miroslav.macko <miroslav.macko@pantheon.tech>
(cherry picked from commit 40b60339e5e8aeb07e782f8c34b5c2ef50da56ab)

diff --git a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java
index 2e8b40cf4dd36e7bf42ad62ce3f21034567813ee..b633bb07b71f929362e8928ab45f5c641d5944a1 100644 (file)
--- a/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java
+++ b/applications/forwardingrules-sync/src/main/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListener.java
@@ -15,9 +15,9 @@ import java.text.SimpleDateFormat;
 import java.util.Collection;
 import java.util.Date;
 import java.util.List;
+import java.util.Objects;
 import javax.annotation.Nonnull;
 import org.opendaylight.controller.md.sal.binding.api.DataObjectModification;
-import org.opendaylight.controller.md.sal.binding.api.DataObjectModification.ModificationType;
 import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.openflowplugin.applications.frsync.SyncReactor;
@@ -76,37 +76,32 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener<Node>
      */
     protected Optional<ListenableFuture<Boolean>> processNodeModification(
             final DataTreeModification<Node> modification) {
+        Optional<ListenableFuture<Boolean>> result;
         final NodeId nodeId = ModificationUtil.nodeId(modification);
-        updateCache(modification);
+        final DataObjectModification<Node> nodeModification = modification.getRootNode();
 
-        final boolean isAdd = isAdd(modification) || isAddLogical(modification);
+        if (isDelete(nodeModification) || isDeleteLogical(nodeModification)) {
+            operationalSnapshot.updateCache(nodeId, Optional.absent());
+            deviceMastershipManager.onDeviceDisconnected(nodeId);
+            result = skipModification(modification);
+        } else {
+            operationalSnapshot.updateCache(nodeId, Optional.fromNullable(ModificationUtil.flowCapableNodeAfter(modification)));
 
-        if (isAdd) {
-            deviceMastershipManager.onDeviceConnected(nodeId);
-        }
+            final boolean isAdd = isAdd(nodeModification) || isAddLogical(nodeModification);
 
-        // if node is registered for reconcile we need consistent data from operational DS (skip partial collections)
-        // but we can accept first modification since all statistics are intentionally collected in one step on startup
-        if (reconciliationRegistry.isRegistered(nodeId) && (isAdd || isConsistentForReconcile(modification))) {
-            return reconciliation(modification);
-        } else {
-            return skipModification(modification);
-        }
-    }
+            if (isAdd) {
+                deviceMastershipManager.onDeviceConnected(nodeId);
+            }
 
-    /**
-     * Remove if delete. Update only if FlowCapableNode Augmentation modified.
-     * Unregister for device mastership.
-     * @param modification Datastore modification
-     */
-    private void updateCache(final DataTreeModification<Node> modification) {
-        NodeId nodeId = ModificationUtil.nodeId(modification);
-        if (isDelete(modification) || isDeleteLogical(modification)) {
-            operationalSnapshot.updateCache(nodeId, Optional.absent());
-            deviceMastershipManager.onDeviceDisconnected(nodeId);
-            return;
+            // if node is registered for reconcile we need consistent data from operational DS (skip partial collections)
+            // but we can accept first modification since all statistics are intentionally collected in one step on startup
+            if (reconciliationRegistry.isRegistered(nodeId) && (isAdd || isConsistentForReconcile(modification))) {
+                result = reconciliation(modification);
+            } else {
+                result = skipModification(modification);
+            }
         }
-        operationalSnapshot.updateCache(nodeId, Optional.fromNullable(ModificationUtil.flowCapableNodeAfter(modification)));
+        return result;
     }
 
     private Optional<ListenableFuture<Boolean>> skipModification(final DataTreeModification<Node> modification) {
@@ -119,30 +114,27 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener<Node>
         return Optional.absent();
     }
 
-    private boolean isDelete(final DataTreeModification<Node> modification) {
-        return ModificationType.DELETE == modification.getRootNode().getModificationType();
+    private boolean isDelete(final DataObjectModification<Node> nodeModification) {
+        return Objects.nonNull(nodeModification.getDataBefore()) && Objects.isNull(nodeModification.getDataAfter());
     }
 
     /**
      * All connectors disappeared from operational store (logical delete).
      */
-    private boolean isDeleteLogical(final DataTreeModification<Node> modification) {
-        final DataObjectModification<Node> rootNode = modification.getRootNode();
-        return !safeConnectorsEmpty(rootNode.getDataBefore()) && safeConnectorsEmpty(rootNode.getDataAfter());
+    private boolean isDeleteLogical(final DataObjectModification<Node> nodeModification) {
+        return !safeConnectorsEmpty(nodeModification.getDataBefore()) && safeConnectorsEmpty(nodeModification.getDataAfter());
 
     }
 
-    private boolean isAdd(final DataTreeModification<Node> modification) {
-        final DataObjectModification<Node> rootNode = modification.getRootNode();
-        return rootNode.getDataBefore() == null && rootNode.getDataAfter() != null;
+    private boolean isAdd(final DataObjectModification<Node> nodeModification) {
+        return Objects.isNull(nodeModification.getDataBefore()) && Objects.nonNull(nodeModification.getDataAfter());
     }
 
     /**
      * All connectors appeared in operational store (logical add).
      */
-    private boolean isAddLogical(final DataTreeModification<Node> modification) {
-        final DataObjectModification<Node> rootNode = modification.getRootNode();
-        return safeConnectorsEmpty(rootNode.getDataBefore()) && !safeConnectorsEmpty(rootNode.getDataAfter());
+    private boolean isAddLogical(final DataObjectModification<Node> nodeModification) {
+        return safeConnectorsEmpty(nodeModification.getDataBefore()) && !safeConnectorsEmpty(nodeModification.getDataAfter());
     }
 
     /**

diff --git a/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListenerTest.java b/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListenerTest.java
index 10441849daf78a358bce9d39e62a7aca8583036b..32435aa8c1fb339715a1320839d3de3d5d8e4363 100644 (file)
--- a/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListenerTest.java
+++ b/applications/forwardingrules-sync/src/test/java/org/opendaylight/openflowplugin/applications/frsync/impl/SimplifiedOperationalListenerTest.java
@@ -24,7 +24,6 @@ import org.mockito.Mockito;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.DataObjectModification;
-import org.opendaylight.controller.md.sal.binding.api.DataObjectModification.ModificationType;
 import org.opendaylight.controller.md.sal.binding.api.DataTreeIdentifier;
 import org.opendaylight.controller.md.sal.binding.api.DataTreeModification;
 import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
@@ -43,7 +42,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.Fl
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.snapshot.gathering.status.grouping.SnapshotGatheringStatusEnd;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.node.NodeConnector;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
@@ -83,6 +81,10 @@ public class SimplifiedOperationalListenerTest {
     private ReconciliationRegistry reconciliationRegistry;
     @Mock
     private DeviceMastershipManager deviceMastershipManager;
+    @Mock
+    private List nodeConnector;
+    @Mock
+    private Node operationalNodeEmpty;
 
     @Before
     public void setUp() throws Exception {
@@ -123,8 +125,6 @@ public class SimplifiedOperationalListenerTest {
     public void testOnDataTreeChangedDeletePhysical() throws Exception {
         Mockito.when(operationalModification.getDataBefore()).thenReturn(operationalNode);
         Mockito.when(operationalModification.getDataAfter()).thenReturn(null);
-        Mockito.when(dataTreeModification.getRootNode().getModificationType()).thenReturn(ModificationType.DELETE);
-        Mockito.when(reconciliationRegistry.isRegistered(NODE_ID)).thenReturn(false);
 
         nodeListenerOperational.onDataTreeChanged(Collections.singleton(dataTreeModification));
 
@@ -135,9 +135,10 @@ public class SimplifiedOperationalListenerTest {
     @Test
     public void testOnDataTreeChangedDeleteLogical() {
         Mockito.when(operationalModification.getDataBefore()).thenReturn(operationalNode);
-        List<NodeConnector> nodeConnectorList = Mockito.mock(List.class);
-        Mockito.when(operationalNode.getNodeConnector()).thenReturn(nodeConnectorList);
-        Mockito.when(reconciliationRegistry.isRegistered(NODE_ID)).thenReturn(false);
+        Mockito.when(operationalNode.getNodeConnector()).thenReturn(nodeConnector);
+        Mockito.when(operationalNodeEmpty.getId()).thenReturn(NODE_ID);
+        Mockito.when(operationalModification.getDataAfter()).thenReturn(operationalNodeEmpty);
+        Mockito.when(operationalNodeEmpty.getNodeConnector()).thenReturn(null);
 
         nodeListenerOperational.onDataTreeChanged(Collections.singleton(dataTreeModification));