Revert "Fix statistics race condition on big flows"

[openflowplugin.git] / openflowplugin-impl / src / test / java / org / opendaylight / openflowplugin / impl / services / MultipartRequestOnTheFlyCallbackTest.java

diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallbackTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallbackTest.java
index a085ce1e4cfb591c670a9b60aced993eef958abf..5d15eff5cd7e86192ff42eeafd5da300ab46c6b5 100644 (file)
--- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallbackTest.java
+++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/services/MultipartRequestOnTheFlyCallbackTest.java
@@ -12,6 +12,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -40,6 +41,7 @@ import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo;
 import org.opendaylight.openflowplugin.api.openflow.device.DeviceState;
 import org.opendaylight.openflowplugin.api.openflow.device.RequestContext;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.DeviceFlowRegistry;
+import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowDescriptor;
 import org.opendaylight.openflowplugin.api.openflow.registry.flow.FlowRegistryKey;
 import org.opendaylight.openflowplugin.api.openflow.statistics.ofpspecific.EventIdentifier;
 import org.opendaylight.openflowplugin.impl.rpc.AbstractRequestContext;
@@ -105,6 +107,8 @@ public class MultipartRequestOnTheFlyCallbackTest {
     @Mock
     private DeviceFlowRegistry mockedFlowRegistry;
     @Mock
+    private FlowDescriptor mockedFlowDescriptor;
+    @Mock
     private ReadOnlyTransaction mockedReadOnlyTx;
 
     private AbstractRequestContext<List<MultipartReply>> dummyRequestContext;
@@ -113,7 +117,7 @@ public class MultipartRequestOnTheFlyCallbackTest {
     private final short tableId = 0;
 
     @Before
-    public void initialization() {
+    public void initialization() throws Exception{
         when(mockedDeviceContext.getMessageSpy()).thenReturn(new MessageIntelligenceAgencyImpl());
         when(mockedNodeId.toString()).thenReturn(DUMMY_NODE_ID);
         when(mockedPrimaryConnection.getNodeId()).thenReturn(mockedNodeId);
@@ -127,7 +131,6 @@ public class MultipartRequestOnTheFlyCallbackTest {
 
         when(mockedDeviceContext.getPrimaryConnectionContext()).thenReturn(mockedPrimaryConnection);
         when(mockedDeviceInfo.getNodeInstanceIdentifier()).thenReturn(NODE_PATH);
-        when(mockedDeviceState.deviceSynchronized()).thenReturn(true);
         when(mockedDeviceInfo.getNodeId()).thenReturn(mockedNodeId);
         when(mockedDeviceInfo.getDatapathId()).thenReturn(BigInteger.TEN);
         when(mockedDeviceInfo.getVersion()).thenReturn(OFConstants.OFP_VERSION_1_3);
@@ -135,13 +138,17 @@ public class MultipartRequestOnTheFlyCallbackTest {
         when(mockedDeviceContext.getDeviceState()).thenReturn(mockedDeviceState);
         when(mockedDeviceContext.getDeviceInfo()).thenReturn(mockedDeviceInfo);
         when(mockedDeviceContext.getDeviceFlowRegistry()).thenReturn(mockedFlowRegistry);
+        when(mockedFlowRegistry.retrieveIdForFlow(Matchers.any(FlowRegistryKey.class))).thenReturn(mockedFlowDescriptor);
 
         final InstanceIdentifier<FlowCapableNode> nodePath = mockedDeviceInfo.getNodeInstanceIdentifier().augmentation(FlowCapableNode.class);
         final FlowCapableNodeBuilder flowNodeBuilder = new FlowCapableNodeBuilder();
         flowNodeBuilder.setTable(Collections.<Table> emptyList());
         final Optional<FlowCapableNode> flowNodeOpt = Optional.of(flowNodeBuilder.build());
+
         final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> flowNodeFuture = Futures.immediateCheckedFuture(flowNodeOpt);
-        when(mockedReadOnlyTx.read(LogicalDatastoreType.OPERATIONAL, nodePath)).thenReturn(flowNodeFuture);
+        final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> flowNodeFutureSpy = spy(flowNodeFuture);
+        when(mockedReadOnlyTx.read(LogicalDatastoreType.OPERATIONAL, nodePath)).thenReturn(flowNodeFutureSpy);
+        when(flowNodeFutureSpy.get()).thenReturn(flowNodeOpt);
         when(mockedDeviceContext.getReadTransaction()).thenReturn(mockedReadOnlyTx);
 
         dummyRequestContext = new AbstractRequestContext<List<MultipartReply>>(DUMMY_XID) {
@@ -203,7 +210,7 @@ public class MultipartRequestOnTheFlyCallbackTest {
         final MatchBuilder matchBuilder = new MatchBuilder()
                 .setMatchEntry(Collections.<MatchEntry>emptyList());
         final FlowStatsBuilder flowStatsBuilder = new FlowStatsBuilder()
-.setTableId(tableId)
+                .setTableId(tableId)
                 .setPriority(2)
                 .setCookie(BigInteger.ZERO)
                 .setByteCount(BigInteger.TEN)
@@ -231,7 +238,10 @@ public class MultipartRequestOnTheFlyCallbackTest {
         final Optional<FlowCapableNode> flowNodeOpt = Optional.of(flowNodeBuilder.build());
         final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> flowNodeFuture = Futures
                 .immediateCheckedFuture(flowNodeOpt);
-        when(mockedReadOnlyTx.read(LogicalDatastoreType.OPERATIONAL, nodePath)).thenReturn(flowNodeFuture);
+        final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> flowNodeFutureSpy = spy(flowNodeFuture);
+
+        when(mockedReadOnlyTx.read(LogicalDatastoreType.OPERATIONAL, nodePath)).thenReturn(flowNodeFutureSpy);
+        when(flowNodeFutureSpy.get()).thenReturn(flowNodeOpt);
         when(mockedDeviceContext.getReadTransaction()).thenReturn(mockedReadOnlyTx);
 
         multipartRequestOnTheFlyCallback.onSuccess(mpReplyMessage.build());
@@ -239,7 +249,6 @@ public class MultipartRequestOnTheFlyCallbackTest {
 
         verify(mockedReadOnlyTx, times(1)).read(LogicalDatastoreType.OPERATIONAL, nodePath);
         verify(mockedReadOnlyTx, times(1)).close();
-        verify(mockedFlowRegistry).storeIfNecessary(Matchers.<FlowRegistryKey> any());
         verify(mockedDeviceContext, times(1)).writeToTransaction(eq(LogicalDatastoreType.OPERATIONAL),
                 eq(tableIdent), Matchers.<Table> any());
         /*
@@ -258,20 +267,55 @@ public class MultipartRequestOnTheFlyCallbackTest {
      */
     @Test
     public void testOnSuccessWithValidMultipart2() throws Exception {
+        final MatchBuilder matchBuilder = new MatchBuilder()
+                .setMatchEntry(Collections.<MatchEntry>emptyList());
+        final FlowStatsBuilder flowStatsBuilder = new FlowStatsBuilder()
+                .setTableId(tableId)
+                .setPriority(2)
+                .setCookie(BigInteger.ZERO)
+                .setByteCount(BigInteger.TEN)
+                .setPacketCount(BigInteger.ONE)
+                .setDurationSec(11L)
+                .setDurationNsec(12L)
+                .setMatch(matchBuilder.build())
+                .setFlags(new FlowModFlags(true, false, false, false, false));
+        final MultipartReplyFlowBuilder multipartReplyFlowBuilder = new MultipartReplyFlowBuilder()
+                .setFlowStats(Collections.singletonList(flowStatsBuilder.build()));
+        final MultipartReplyFlowCaseBuilder multipartReplyFlowCaseBuilder = new MultipartReplyFlowCaseBuilder()
+                .setMultipartReplyFlow(multipartReplyFlowBuilder.build());
         final MultipartReplyMessageBuilder mpReplyMessage = new MultipartReplyMessageBuilder()
-                .setType(MultipartType.OFPMPDESC)
-                .setFlags(new MultipartRequestFlags(false));
+                .setType(MultipartType.OFPMPFLOW)
+                .setFlags(new MultipartRequestFlags(false))
+                .setMultipartReplyBody(multipartReplyFlowCaseBuilder.build())
+                .setXid(21L);
 
-        multipartRequestOnTheFlyCallback.onSuccess(mpReplyMessage.build());
+        final InstanceIdentifier<FlowCapableNode> nodePath = mockedDeviceInfo.getNodeInstanceIdentifier()
+                .augmentation(FlowCapableNode.class);
+        final FlowCapableNodeBuilder flowNodeBuilder = new FlowCapableNodeBuilder();
+        final TableBuilder tableDataBld = new TableBuilder();
+        tableDataBld.setId(tableId);
+        flowNodeBuilder.setTable(Collections.singletonList(tableDataBld.build()));
+        final Optional<FlowCapableNode> flowNodeOpt = Optional.of(flowNodeBuilder.build());
+        final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> flowNodeFuture = Futures
+                .immediateCheckedFuture(flowNodeOpt);
+        final CheckedFuture<Optional<FlowCapableNode>, ReadFailedException> flowNodeFutureSpy = spy(flowNodeFuture);
 
-        final RpcResult<List<MultipartReply>> actualResult = dummyRequestContext.getFuture().get();
-        assertNotNull(actualResult.getErrors());
-        assertTrue(actualResult.getErrors().isEmpty());
-        assertNotNull(actualResult.getResult());
-        assertTrue(actualResult.getResult().isEmpty());
+        when(mockedReadOnlyTx.read(LogicalDatastoreType.OPERATIONAL, nodePath)).thenReturn(flowNodeFutureSpy);
+        when(flowNodeFutureSpy.get()).thenReturn(flowNodeOpt);
+        when(mockedDeviceContext.getReadTransaction()).thenReturn(mockedReadOnlyTx);
 
-        Mockito.verify(mockedFlowRegistry, Mockito.never()).storeIfNecessary(Matchers.any());
-        Mockito.verify(mockedDeviceContext, Mockito.never()).writeToTransaction(Matchers.eq(LogicalDatastoreType.OPERATIONAL),
-                Matchers.<InstanceIdentifier>any(), Matchers.<DataObject>any());
+        multipartRequestOnTheFlyCallback.onSuccess(mpReplyMessage.build());
+        final InstanceIdentifier<Table> tableIdent = nodePath.child(Table.class, new TableKey(tableId));
+
+        verify(mockedReadOnlyTx, times(1)).read(LogicalDatastoreType.OPERATIONAL, nodePath);
+        verify(mockedReadOnlyTx, times(1)).close();
+        verify(mockedDeviceContext, times(1)).writeToTransaction(eq(LogicalDatastoreType.OPERATIONAL),
+                eq(tableIdent), Matchers.<Table> any());
+        /*
+         * One call for Table one call for Flow
+         * we are not able to create Flow InstanceIdentifier because we are missing FlowId
+         */
+        verify(mockedDeviceContext, times(2)).writeToTransaction(eq(LogicalDatastoreType.OPERATIONAL),
+                Matchers.<InstanceIdentifier> any(), Matchers.<DataObject> any());
     }
 }