From 40b60339e5e8aeb07e782f8c34b5c2ef50da56ab Mon Sep 17 00:00:00 2001 From: "miroslav.macko" Date: Mon, 3 Oct 2016 18:07:15 +0200 Subject: [PATCH] Bug 6745 SimplifiedOperationalListener optimation - optimize processNodeModification method - update test Change-Id: I941a84bff4322c2b2b64f8404791ec6d00a9d200 Signed-off-by: miroslav.macko --- .../impl/SimplifiedOperationalListener.java | 66 ++++++++----------- .../SimplifiedOperationalListenerTest.java | 15 +++-- 2 files changed, 37 insertions(+), 44 deletions(-) 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 2e8b40cf4d..b633bb07b7 100644 --- 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 */ protected Optional> processNodeModification( final DataTreeModification modification) { + Optional> result; final NodeId nodeId = ModificationUtil.nodeId(modification); - updateCache(modification); + final DataObjectModification 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 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> skipModification(final DataTreeModification modification) { @@ -119,30 +114,27 @@ public class SimplifiedOperationalListener extends AbstractFrmSyncListener return Optional.absent(); } - private boolean isDelete(final DataTreeModification modification) { - return ModificationType.DELETE == modification.getRootNode().getModificationType(); + private boolean isDelete(final DataObjectModification nodeModification) { + return Objects.nonNull(nodeModification.getDataBefore()) && Objects.isNull(nodeModification.getDataAfter()); } /** * All connectors disappeared from operational store (logical delete). */ - private boolean isDeleteLogical(final DataTreeModification modification) { - final DataObjectModification rootNode = modification.getRootNode(); - return !safeConnectorsEmpty(rootNode.getDataBefore()) && safeConnectorsEmpty(rootNode.getDataAfter()); + private boolean isDeleteLogical(final DataObjectModification nodeModification) { + return !safeConnectorsEmpty(nodeModification.getDataBefore()) && safeConnectorsEmpty(nodeModification.getDataAfter()); } - private boolean isAdd(final DataTreeModification modification) { - final DataObjectModification rootNode = modification.getRootNode(); - return rootNode.getDataBefore() == null && rootNode.getDataAfter() != null; + private boolean isAdd(final DataObjectModification nodeModification) { + return Objects.isNull(nodeModification.getDataBefore()) && Objects.nonNull(nodeModification.getDataAfter()); } /** * All connectors appeared in operational store (logical add). */ - private boolean isAddLogical(final DataTreeModification modification) { - final DataObjectModification rootNode = modification.getRootNode(); - return safeConnectorsEmpty(rootNode.getDataBefore()) && !safeConnectorsEmpty(rootNode.getDataAfter()); + private boolean isAddLogical(final DataObjectModification 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 10441849da..32435aa8c1 100644 --- 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 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)); -- 2.36.6