Update: Fix a bug for StatisticCollector.java causes by a change of OpenFlow protocols

src/main/java/net/floodlightcontroller/statistics/StatisticsCollector.java

@@ -128,10 +128,10 @@ public class StatisticsCollector implements IFloodlightModule, IStatisticsServic
 			IOFSwitch sw = switchService.getSwitch(npt.getNodeId());
 			long speed = 0;
 
-			// Fix Problem starts here
 			if(sw == null) return speed; /* could have disconnected; we'll assume zero-speed then */
 			if(sw.getPort(npt.getPortId()) == null) return speed;
 
+			/* getCurrSpeed() should handle different OpenFlow Version */
 			OFVersion detectedVersion = sw.getOFFactory().getVersion();
 			switch(detectedVersion){
 				case OF_10:
@@ -147,7 +147,7 @@ public class StatisticsCollector implements IFloodlightModule, IStatisticsServic
 				case OF_14:
 				case OF_15:
 					for(OFPortDescProp p : sw.getPort(npt.getPortId()).getProperties()){
-						if( p.getType() == 0 ){
+						if( p.getType() == 0 ){ /* OpenFlow 1.4 and OpenFlow 1.5 will return zero */
 							speed = ((OFPortDescPropEthernet) p).getCurrSpeed();
 						}
 					}

src/test/java/net/floodlightcontroller/statistics/StatisticsTest.java

@@ -9,40 +9,39 @@ import net.floodlightcontroller.core.types.NodePortTuple;
 import net.floodlightcontroller.test.FloodlightTestCase;
 import org.easymock.EasyMock;
 import org.projectfloodlight.openflow.protocol.*;
+import static org.projectfloodlight.openflow.protocol.OFVersion.*;
 import org.projectfloodlight.openflow.types.DatapathId;
 import org.projectfloodlight.openflow.types.OFPort;
 import org.projectfloodlight.openflow.protocol.OFFactories;
-import net.floodlightcontroller.restserver.IRestApiService;
 import net.floodlightcontroller.threadpool.IThreadPoolService;
-import org.junit.*;
-
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
 import static org.easymock.EasyMock.*;
 import static org.junit.Assert.*;
+import org.junit.*;
 
 /**
- * Created by qw on 5/18/17.
+ * Created by qing wang on 5/18/17.
  */
 
-
 public class StatisticsTest extends FloodlightTestCase {
 
     private FloodlightContext cntx;
     private FloodlightModuleContext fmc;
-//    private static IRestApiService restApiService;
-    private IThreadPoolService threadPoolService;
     private MockThreadPoolService threadpool;
     private IOFSwitchService switchService;
+
+    private static final OFFactory factory10 = OFFactories.getFactory(OFVersion.OF_10);
+    private static final OFFactory factory11 = OFFactories.getFactory(OFVersion.OF_11);
+    private static final OFFactory factory12 = OFFactories.getFactory(OFVersion.OF_12);
     private static final OFFactory factory13 = OFFactories.getFactory(OFVersion.OF_13);
     private static final OFFactory factory14 = OFFactories.getFactory(OFVersion.OF_14);
-
-    protected StatisticsCollector statsCollector;
-
-    private IOFSwitch sw_OF13, sw_OF14;
+    private static final OFFactory factory15 = OFFactories.getFactory(OFVersion.OF_15);
+    private StatisticsCollector statsCollector;
+    public static IOFSwitch sw_OF11, sw_OF12, sw_OF13, sw_OF14, sw_OF15;
+    private DatapathId switchDPID = DatapathId.of(1L);
 
     @Override
     public void setUp() throws Exception {
@@ -58,43 +57,105 @@ public class StatisticsTest extends FloodlightTestCase {
 
         fmc.addService(IThreadPoolService.class, threadpool);
         fmc.addService(IOFSwitchService.class, switchService);
-//        fmc.addService(IRestApiService.class, restApiService);
 
         threadpool.init(fmc);
         statsCollector.init(fmc);
         threadpool.startUp(fmc);
-//        statsCollector.startUp(fmc);
 
+        // Create OpenFlow11 Mock Switch
+        sw_OF11 = EasyMock.createMock(IOFSwitch.class);
+        sw_OF11 = getSwitchByOFVersion(factory11);
 
-        //TODO: Pull the common code from testGetCurrPortSpeedOF13/OF14, make the IOFSwitch OF13/OF14 as a parameter
+        // Create OpenFlow12 Mock Switch
+        sw_OF12 = EasyMock.createMock(IOFSwitch.class);
+        sw_OF12 = getSwitchByOFVersion(factory12);
 
+        // Create OpenFlow13 Mock Switch
+        sw_OF13 = EasyMock.createMock(IOFSwitch.class);
+        sw_OF13 = getSwitchByOFVersion(factory13);
+
+        // Create OpenFlow14 Mock Switch
+        sw_OF14 = EasyMock.createMock(IOFSwitch.class);
+        sw_OF14 = getSwitchByOFVersion(factory14);
+
+        // Create OpenFlow15 Mock Switch
+        sw_OF15 = EasyMock.createMock(IOFSwitch.class);
+        sw_OF15 = getSwitchByOFVersion(factory15);
+
+    }
+
+    private IOFSwitch getSwitchByOFVersion(OFFactory factory) {
+        IOFSwitch sw = EasyMock.createMock(IOFSwitch.class);
+
+        OFVersion openflowVer = factory.getVersion();
+        switch (openflowVer) {
+            case OF_11:
+            case OF_12:
+            case OF_13:
+                reset(sw);
+                expect(sw.getId()).andReturn(switchDPID).anyTimes();
+                expect(sw.getOFFactory()).andReturn(factory).anyTimes();
+                expect(sw.getPort(OFPort.of(1))).andReturn(factory.buildPortDesc()
+                        .setPortNo(OFPort.of(1))
+                        .setName("eth1")
+                        .setCurrSpeed(100L)
+                        .build()).anyTimes();
+                replay(sw);
+                break;
+
+            case OF_14:
+            case OF_15:
+                reset(sw);
+                expect(sw.getId()).andReturn(switchDPID).anyTimes();
+                expect(sw.getOFFactory()).andReturn(factory).anyTimes();
+                expect(sw.getPort(OFPort.of(1))).andReturn(factory.buildPortDesc()
+                        .setPortNo(OFPort.of(1))
+                        .setName("eth1")
+                        .setProperties(Collections.singletonList(factory.buildPortDescPropEthernet().setCurrSpeed(100L).build()))
+                        .build()).anyTimes();
+                replay(sw);
+                break;
+
+            default:
+                break;
+
+        }
+
+        return sw;
     }
 
+
     /**
+     * Test getSpeed() method can work with Openflow 1.1
      *
      * @throws Exception
      */
     @Test
-    public void testGetCurrentPortSpeedOF13() throws Exception{
-        // Create OpenFlow13 Mock Switch
-        sw_OF13 = EasyMock.createMock(IOFSwitch.class);
-        reset(sw_OF13);
-        expect(sw_OF13.getId()).andReturn(DatapathId.of(1L)).anyTimes();
-        expect(sw_OF13.getOFFactory()).andReturn(factory13).anyTimes();
+    public void testGetCurrentPortSpeedOF11() throws Exception {
+        NodePortTuple npt = new NodePortTuple(sw_OF11.getId(), OFPort.of(1));
+
+        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
+        switchMap.put(sw_OF11.getId(), sw_OF11);
+        getMockSwitchService().setSwitches(switchMap);
+
+        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
 
-        OFPortDesc portDesc13 = factory13.buildPortDesc()
-                .setPortNo(OFPort.of(1))
-                .setName("eth1")
-                .setCurrSpeed(100L)
-                .build();
+        long speed = statsSpeedCollector.getSpeed(npt);
+        assertEquals(speed, 100L);
 
-        expect(sw_OF13.getPort(OFPort.of(1))).andReturn(portDesc13).anyTimes();
-        replay(sw_OF13);
+    }
 
-        NodePortTuple npt = new NodePortTuple(DatapathId.of(1L), OFPort.of(1));
+    /**
+     * Test getSpeed() method can work with Openflow 1.2
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testGetCurrentPortSpeedOF12() throws Exception {
+        NodePortTuple npt = new NodePortTuple(sw_OF12.getId(), OFPort.of(1));
 
         Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
-        switchMap.put(sw_OF13.getId(), sw_OF13);
+        switchMap.put(sw_OF12.getId(), sw_OF12);
         getMockSwitchService().setSwitches(switchMap);
 
         StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
@@ -104,34 +165,57 @@ public class StatisticsTest extends FloodlightTestCase {
 
     }
 
-
     /**
+     * Test getSpeed() method can work with Openflow 1.3
      *
+     * @throws Exception
+     */
+    @Test
+    public void testGetCurrentPortSpeedOF13() throws Exception {
+        NodePortTuple npt = new NodePortTuple(sw_OF13.getId(), OFPort.of(1));
+
+        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
+        switchMap.put(sw_OF13.getId(), sw_OF13);
+        getMockSwitchService().setSwitches(switchMap);
+
+        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
+
+        long speed = statsSpeedCollector.getSpeed(npt);
+        assertEquals(speed, 100L);
+
+    }
+
+    /**
+     * Test getSpeed() method can work with Openflow 1.4
      *
      * @throws Exception
      */
     @Test
     public void testGetCurrentPortSpeedOF14() throws Exception {
-        // Create OpenFlow14 Mock Switch
-        sw_OF14 = EasyMock.createMock(IOFSwitch.class);
-        reset(sw_OF14);
-        expect(sw_OF14.getId()).andReturn(DatapathId.of(1L)).anyTimes();
-        expect(sw_OF14.getOFFactory()).andReturn(factory14).anyTimes();
+        NodePortTuple npt = new NodePortTuple(sw_OF14.getId(), OFPort.of(1));
+
+        Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
+        switchMap.put(sw_OF14.getId(), sw_OF14);
+        getMockSwitchService().setSwitches(switchMap);
 
+        StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();
 
-        OFPortDesc portDesc14 = factory14.buildPortDesc()
-                .setPortNo(OFPort.of(1))
-                .setName("eth1")
-                .setProperties(Collections.singletonList(factory14.buildPortDescPropEthernet().setCurrSpeed(100L).build()))
-                .build();
+        long speed = statsSpeedCollector.getSpeed(npt);
+        assertEquals(speed, 100L);
 
-        expect(sw_OF14.getPort(OFPort.of(1))).andReturn(portDesc14).anyTimes();
-        replay(sw_OF14);
+    }
 
-        NodePortTuple npt = new NodePortTuple(DatapathId.of(1L), OFPort.of(1));
+    /**
+     * Test getSpeed() method can work with Openflow 1.5
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testGetCurrentPortSpeedOF15() throws Exception {
+        NodePortTuple npt = new NodePortTuple(sw_OF15.getId(), OFPort.of(1));
 
         Map<DatapathId, IOFSwitch> switchMap = new HashMap<>();
-        switchMap.put(sw_OF14.getId(), sw_OF14);
+        switchMap.put(sw_OF15.getId(), sw_OF15);
         getMockSwitchService().setSwitches(switchMap);
 
         StatisticsCollector.PortStatsCollector statsSpeedCollector = statsCollector.new PortStatsCollector();