Refactor SubnetConfig 60/1160/3
authorAlessandro Boch <aboch@cisco.com>
Wed, 11 Sep 2013 23:34:41 +0000 (16:34 -0700)
committerAlessandro Boch <aboch@cisco.com>
Thu, 12 Sep 2013 16:01:57 +0000 (09:01 -0700)
- Change Subent Config to represent the switch ports with the NodeConnector notation
- Added validity check on zero subnet configuration
- Change device js to correctly set the value in the ports selects, JQuery changes from Andrew (andrekim@cisco.com)
- While uniform Subnet northbound to other NB modules, removed the node-ports endpoints. Have the subnet as end resource,
  we already have the POST for it.
- Added ISwitchManager.modifySubnet(SubnetConfig s)
- Added SubnetConfigTest.java for junit
- Updated subnet northbound junit

Change-Id: I7777d0dc3b60f6532836a5115d8fc2dd48c18768
Signed-off-by: Alessandro Boch <aboch@cisco.com>
20 files changed:
opendaylight/northbound/integrationtest/src/test/java/org/opendaylight/controller/northbound/integrationtest/NorthboundIT.java
opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetConfigs.java
opendaylight/northbound/subnets/src/main/java/org/opendaylight/controller/subnets/northbound/SubnetsNorthbound.java
opendaylight/northbound/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/northbound/SwitchNorthbound.java
opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/core/NodeConnector.java
opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/ISwitchManager.java
opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Subnet.java
opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/SubnetConfig.java
opendaylight/switchmanager/api/src/main/java/org/opendaylight/controller/switchmanager/Switch.java
opendaylight/switchmanager/api/src/test/java/org/opendaylight/controller/switchmanager/SubnetConfigTest.java [new file with mode: 0644]
opendaylight/switchmanager/api/src/test/java/org/opendaylight/controller/switchmanager/SubnetTest.java
opendaylight/switchmanager/api/src/test/java/org/opendaylight/controller/switchmanager/SwitchTest.java
opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/Activator.java
opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java
opendaylight/switchmanager/implementation/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerTest.java
opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/Devices.java
opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/NodeJsonBean.java
opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/PortJsonBean.java [new file with mode: 0644]
opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/SubnetGatewayPortBean.java
opendaylight/web/devices/src/main/resources/js/page.js

index 38336c1..eb158a5 100644 (file)
@@ -88,10 +88,10 @@ public class NorthboundIT {
         assertNotNull(bc);
         boolean debugit = false;
         Bundle b[] = bc.getBundles();
-        for (int i = 0; i < b.length; i++) {
-            int state = b[i].getState();
+        for (Bundle element : b) {
+            int state = element.getState();
             if (state != Bundle.ACTIVE && state != Bundle.RESOLVED) {
-                log.debug("Bundle:" + b[i].getSymbolicName() + " state:" + stateToString(state));
+                log.debug("Bundle:" + element.getSymbolicName() + " state:" + stateToString(state));
                 debugit = true;
             }
         }
@@ -143,8 +143,9 @@ public class NorthboundIT {
 
         if (debugMsg) {
             System.out.println("HTTP method: " + method + " url: " + restUrl.toString());
-            if (body != null)
+            if (body != null) {
                 System.out.println("body: " + body);
+            }
         }
 
         try {
@@ -172,8 +173,9 @@ public class NorthboundIT {
 
             // Response code for success should be 2xx
             httpResponseCode = connection.getResponseCode();
-            if (httpResponseCode > 299)
+            if (httpResponseCode > 299) {
                 return httpResponseCode.toString();
+            }
 
             if (debugMsg) {
                 System.out.println("HTTP response code: " + connection.getResponseCode());
@@ -291,7 +293,7 @@ public class NorthboundIT {
     }
 
     @Test
-    public void testSubnetsNorthbound() throws JSONException {
+    public void testSubnetsNorthbound() throws JSONException, ConstructionException {
         System.out.println("Starting Subnets JAXB client.");
         String baseURL = "http://127.0.0.1:8080/controller/nb/v2/subnetservice/";
 
@@ -300,17 +302,34 @@ public class NorthboundIT {
 
         String name2 = "testSubnet2";
         String subnet2 = "2.2.2.2/24";
-        String[] nodePorts2 = {"2/1", "2/2", "2/3", "2/4"};
-        StringBuilder nodePortsJson2 = new StringBuilder();
-        nodePortsJson2.append(nodePorts2[0] + "," + nodePorts2[1]  + "," + nodePorts2[2] + "," + nodePorts2[3]);
 
         String name3 = "testSubnet3";
         String subnet3 = "3.3.3.3/24";
-        String[] nodePorts3 = {"3/1", "3/2", "3/3"};
-        StringBuilder nodePortsJson3 = new StringBuilder();
-        nodePortsJson3.append(nodePorts3[0] + "," + nodePorts3[1]  + "," + nodePorts3[2]);
-        StringBuilder nodePortsJson3_1 = new StringBuilder();
-        nodePortsJson3_1.append(nodePortsJson3).append(",").append(nodePortsJson2);
+
+        /*
+         * Create the node connector string list for the two subnets as:
+         * portList2 = {"OF|1@OF|00:00:00:00:00:00:00:02", "OF|2@OF|00:00:00:00:00:00:00:02", "OF|3@OF|00:00:00:00:00:00:00:02", "OF|4@OF|00:00:00:00:00:00:00:02"};
+         * portList3 = {"OF|1@OF|00:00:00:00:00:00:00:03", "OF|2@OF|00:00:00:00:00:00:00:03", "OF|3@OF|00:00:00:00:00:00:00:03"};
+         */
+        Node node2 = new Node(Node.NodeIDType.OPENFLOW, 2L);
+        List<String> portList2 = new ArrayList<String>();
+        NodeConnector nc21 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)1, node2);
+        NodeConnector nc22 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)2, node2);
+        NodeConnector nc23 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)3, node2);
+        NodeConnector nc24 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)3, node2);
+        portList2.add(nc21.toString());
+        portList2.add(nc22.toString());
+        portList2.add(nc23.toString());
+        portList2.add(nc24.toString());
+
+        List<String> portList3 = new ArrayList<String>();
+        Node node3 = new Node(Node.NodeIDType.OPENFLOW, 3L);
+        NodeConnector nc31 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)1, node3);
+        NodeConnector nc32 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)2, node3);
+        NodeConnector nc33 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)3, node3);
+        portList3.add(nc31.toString());
+        portList3.add(nc32.toString());
+        portList3.add(nc33.toString());
 
         // Test GET subnets in default container
         String result = getJsonResult(baseURL + "default/subnets");
@@ -337,30 +356,20 @@ public class NorthboundIT {
         Assert.assertEquals(name1, json.getString("name"));
         Assert.assertEquals(subnet1, json.getString("subnet"));
 
-        // Test POST subnet2
-        JSONObject jo2 = new JSONObject().put("name", name2).put("subnet", subnet2);
+        // Test PUT subnet2
+        JSONObject jo2 = new JSONObject().put("name", name2).put("subnet", subnet2).put("nodeConnectors", portList2);
         // execute HTTP request and verify response code
         result = getJsonResult(baseURL + "default/subnet/" + name2, "PUT", jo2.toString());
         Assert.assertEquals(201, httpResponseCode.intValue());
-        // Test POST nodePorts
-        jo2.append("nodePorts", nodePortsJson2);
-        // execute HTTP request and verify response code
-        result = getJsonResult(baseURL + "default/subnet/" + name2 + "/nodePorts", "PUT", jo2.toString());
-        Assert.assertEquals(200, httpResponseCode.intValue());
-        // Test POST subnet3
+        // Test PUT subnet3
         JSONObject jo3 = new JSONObject().put("name", name3).put("subnet", subnet3);
         // execute HTTP request and verify response code
         result = getJsonResult(baseURL + "default/subnet/" + name3, "PUT", jo3.toString());
         Assert.assertEquals(201, httpResponseCode.intValue());
-        // Test POST nodePorts
-        jo3.append("nodePorts", nodePortsJson3);
+        // Test POST subnet3 (modify port list: add)
+        JSONObject jo3New = new JSONObject().put("name", name3).put("subnet", subnet3).put("nodeConnectors", portList3);
         // execute HTTP request and verify response code
-        result = getJsonResult(baseURL + "default/subnet/" + name3 + "/nodePorts", "PUT", jo3.toString());
-        Assert.assertEquals(200, httpResponseCode.intValue());
-        // Test PUT nodePorts
-        jo3.remove("nodePorts");
-        jo3.append("nodePorts", nodePortsJson3_1);
-        result = getJsonResult(baseURL + "default/subnet/" + name3 + "/nodePorts", "POST", jo3.toString());
+        result = getJsonResult(baseURL + "default/subnet/" + name3, "POST", jo3New.toString());
         Assert.assertEquals(200, httpResponseCode.intValue());
 
         // Test GET all subnets in default container
@@ -376,27 +385,42 @@ public class NorthboundIT {
                 Assert.assertEquals(subnet1, subnetConfig.getString("subnet"));
             } else if (subnetConfig.getString("name").equals(name2)) {
                 Assert.assertEquals(subnet2, subnetConfig.getString("subnet"));
-                String[] nodePortsGet2 = subnetConfig.getJSONArray("nodePorts").getString(0).split(",");
-                Assert.assertEquals(nodePorts2[0], nodePortsGet2[0]);
-                Assert.assertEquals(nodePorts2[1], nodePortsGet2[1]);
-                Assert.assertEquals(nodePorts2[2], nodePortsGet2[2]);
-                Assert.assertEquals(nodePorts2[3], nodePortsGet2[3]);
+                JSONArray portListGet = subnetConfig.getJSONArray("nodeConnectors");
+                Assert.assertEquals(portList2.get(0), portListGet.get(0));
+                Assert.assertEquals(portList2.get(1), portListGet.get(1));
+                Assert.assertEquals(portList2.get(2), portListGet.get(2));
+                Assert.assertEquals(portList2.get(3), portListGet.get(3));
             } else if (subnetConfig.getString("name").equals(name3)) {
                 Assert.assertEquals(subnet3, subnetConfig.getString("subnet"));
-                String[] nodePortsGet = subnetConfig.getJSONArray("nodePorts").getString(0).split(",");
-                Assert.assertEquals(nodePorts3[0], nodePortsGet[0]);
-                Assert.assertEquals(nodePorts3[1], nodePortsGet[1]);
-                Assert.assertEquals(nodePorts3[2], nodePortsGet[2]);
-                Assert.assertEquals(nodePorts2[0], nodePortsGet[3]);
-                Assert.assertEquals(nodePorts2[1], nodePortsGet[4]);
-                Assert.assertEquals(nodePorts2[2], nodePortsGet[5]);
-                Assert.assertEquals(nodePorts2[3], nodePortsGet[6]);
+                JSONArray portListGet = subnetConfig.getJSONArray("nodeConnectors");
+                Assert.assertEquals(portList3.get(0), portListGet.get(0));
+                Assert.assertEquals(portList3.get(1), portListGet.get(1));
+                Assert.assertEquals(portList3.get(2), portListGet.get(2));
             } else {
                 // Unexpected config name
                 Assert.assertTrue(false);
             }
         }
 
+        // Test POST subnet2 (modify port list: remove one port only)
+        List<String> newPortList2 = new ArrayList<String>(portList2);
+        newPortList2.remove(3);
+        JSONObject jo2New = new JSONObject().put("name", name2).put("subnet", subnet2).put("nodeConnectors", newPortList2);
+        // execute HTTP request and verify response code
+        result = getJsonResult(baseURL + "default/subnet/" + name2, "POST", jo2New.toString());
+        Assert.assertEquals(200, httpResponseCode.intValue());
+
+        // Test GET subnet2: verify contains only the first three ports
+        result = getJsonResult(baseURL + "default/subnet/" + name2);
+        jt = new JSONTokener(result);
+        subnetConfig = new JSONObject(jt);
+        Assert.assertEquals(200, httpResponseCode.intValue());
+        JSONArray portListGet2 = subnetConfig.getJSONArray("nodeConnectors");
+        Assert.assertEquals(portList2.get(0), portListGet2.get(0));
+        Assert.assertEquals(portList2.get(1), portListGet2.get(1));
+        Assert.assertEquals(portList2.get(2), portListGet2.get(2));
+        Assert.assertTrue(portListGet2.length() == 3);
+
         // Test DELETE subnet1
         result = getJsonResult(baseURL + "default/subnet/" + name1, "DELETE");
         Assert.assertEquals(204, httpResponseCode.intValue());
@@ -797,19 +821,25 @@ public class NorthboundIT {
             dstBytes[4] = Byte.parseByte(dst.substring(8, 10));
             Assert.assertTrue(Arrays.equals(dstBytes, dstMatch));
         }
-        if (act.getString("type").equals("SET_DL_TYPE"))
+        if (act.getString("type").equals("SET_DL_TYPE")) {
             Assert.assertTrue(act.getInt("dlType") == 10);
-        if (act.getString("type").equals("SET_VLAN_ID"))
+        }
+        if (act.getString("type").equals("SET_VLAN_ID")) {
             Assert.assertTrue(act.getInt("vlanId") == 2);
-        if (act.getString("type").equals("SET_VLAN_PCP"))
+        }
+        if (act.getString("type").equals("SET_VLAN_PCP")) {
             Assert.assertTrue(act.getInt("pcp") == 3);
-        if (act.getString("type").equals("SET_VLAN_CFI"))
+        }
+        if (act.getString("type").equals("SET_VLAN_CFI")) {
             Assert.assertTrue(act.getInt("cfi") == 1);
+        }
 
-        if (act.getString("type").equals("SET_NW_SRC"))
+        if (act.getString("type").equals("SET_NW_SRC")) {
             Assert.assertTrue(act.getString("address").equals("2.2.2.2"));
-        if (act.getString("type").equals("SET_NW_DST"))
+        }
+        if (act.getString("type").equals("SET_NW_DST")) {
             Assert.assertTrue(act.getString("address").equals("1.1.1.1"));
+        }
 
         if (act.getString("type").equals("PUSH_VLAN")) {
             int head = act.getInt("VlanHeader");
@@ -823,12 +853,15 @@ public class NorthboundIT {
             Assert.assertTrue(pcp == 1);
             Assert.assertTrue(tag == 0x8100);
         }
-        if (act.getString("type").equals("SET_NW_TOS"))
+        if (act.getString("type").equals("SET_NW_TOS")) {
             Assert.assertTrue(act.getInt("tos") == 16);
-        if (act.getString("type").equals("SET_TP_SRC"))
+        }
+        if (act.getString("type").equals("SET_TP_SRC")) {
             Assert.assertTrue(act.getInt("port") == 4201);
-        if (act.getString("type").equals("SET_TP_DST"))
+        }
+        if (act.getString("type").equals("SET_TP_DST")) {
             Assert.assertTrue(act.getInt("port") == 8080);
+        }
     }
 
     @Test
@@ -911,14 +944,16 @@ public class NorthboundIT {
             for (int i = 0; i < json_array.length(); i++) {
                 result = json_array.getJSONObject(i);
                 Integer nid = result.getJSONObject("node").getInt("id");
-                if (nid.equals(nodeId))
+                if (nid.equals(nodeId)) {
                     break;
+                }
             }
         } else {
             result = json.getJSONObject(array_name);
             Integer nid = result.getJSONObject("node").getInt("id");
-            if (!nid.equals(nodeId))
+            if (!nid.equals(nodeId)) {
                 result = null;
+            }
         }
         return result;
     }
@@ -1001,7 +1036,7 @@ public class NorthboundIT {
 
         // define variables for decoding returned strings
         String networkAddress;
-        JSONObject host_jo, dl_jo, nc_jo, node_jo;
+        JSONObject host_jo;
 
         // the two hosts should be in inactive host DB
         // test GET method: getInactiveHosts()
@@ -1116,8 +1151,9 @@ public class NorthboundIT {
             JSONArray ja = json.getJSONArray("hostConfig");
             for (int i = 0; i < ja.length(); i++) {
                 String na = ja.getJSONObject(i).getString("networkAddress");
-                if (na.equalsIgnoreCase(hostIp))
+                if (na.equalsIgnoreCase(hostIp)) {
                     return true;
+                }
             }
             return false;
         } else {
@@ -1198,9 +1234,15 @@ public class NorthboundIT {
             for (int j = 0; j < propsArray.length(); j++) {
                 JSONObject props = propsArray.getJSONObject(j);
                 String propName = props.getString("name");
-                if (propName.equals("bandwidth")) bandw = props;
-                if (propName.equals("state")) stt = props;
-                if (propName.equals("latency")) ltc = props;
+                if (propName.equals("bandwidth")) {
+                    bandw = props;
+                }
+                if (propName.equals("state")) {
+                    stt = props;
+                }
+                if (propName.equals("latency")) {
+                    ltc = props;
+                }
             }
 
             if (headNC.getInt("id") == headNC1_nodeConnId) {
@@ -1268,8 +1310,9 @@ public class NorthboundIT {
             int i;
             for (i = 0; i < ja.length(); i++) {
                 userlink = ja.getJSONObject(i);
-                if (userlink.getString("name").equalsIgnoreCase("userLink_1"))
+                if (userlink.getString("name").equalsIgnoreCase("userLink_1")) {
                     break;
+                }
             }
             Assert.assertFalse(i == ja.length());
         } else {
index 224bb14..31dd76f 100644 (file)
@@ -24,6 +24,7 @@ public class SubnetConfigs {
         @XmlElement
         List<SubnetConfig> subnetConfig;
         //To satisfy JAXB
+        @SuppressWarnings("unused")
         private SubnetConfigs() {
 
         }
index b16c8f9..18d8e53 100644 (file)
@@ -7,11 +7,7 @@
  */
 package org.opendaylight.controller.subnets.northbound;
 
-import java.util.HashSet;
-
 import java.util.List;
-import java.util.Set;
-
 import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
 import javax.ws.rs.GET;
@@ -24,7 +20,7 @@ import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.SecurityContext;
-import javax.xml.bind.JAXBElement;
+import javax.ws.rs.core.UriInfo;
 
 import org.codehaus.enunciate.jaxrs.ResponseCode;
 import org.codehaus.enunciate.jaxrs.StatusCodes;
@@ -32,7 +28,6 @@ import org.codehaus.enunciate.jaxrs.TypeHint;
 import org.opendaylight.controller.containermanager.IContainerManager;
 import org.opendaylight.controller.northbound.commons.RestMessages;
 import org.opendaylight.controller.northbound.commons.exception.BadRequestException;
-import org.opendaylight.controller.northbound.commons.exception.InternalServerErrorException;
 import org.opendaylight.controller.northbound.commons.exception.ResourceConflictException;
 import org.opendaylight.controller.northbound.commons.exception.ResourceNotFoundException;
 import org.opendaylight.controller.northbound.commons.exception.ServiceUnavailableException;
@@ -67,7 +62,9 @@ public class SubnetsNorthbound {
 
     @Context
     public void setSecurityContext(SecurityContext context) {
-        if (context != null && context.getUserPrincipal() != null) username = context.getUserPrincipal().getName();
+        if (context != null && context.getUserPrincipal() != null) {
+            username = context.getUserPrincipal().getName();
+        }
     }
 
     protected String getUserName() {
@@ -118,30 +115,25 @@ public class SubnetsNorthbound {
      *
      * Response in XML:
      * &lt;subnetConfig&gt;
-     *    &lt;name&gt;subnet1&lt;/name&gt;
-     *    &lt;subnet&gt;30.0.0.1/24&lt;/subnet&gt;
-     *    &lt;nodePorts&gt;1/1&gt;/nodePorts&gt;
-     *    &lt;nodePorts&gt;1/2&gt;/nodePorts&gt;
-     *    &lt;nodePorts&gt;1/3&gt;/nodePorts&gt;
+     *    &lt;name&gt;marketingdepartment&lt;/name&gt;
+     *    &lt;subnet&gt;30.31.54.254/24&lt;/subnet&gt;
      * &lt;/subnetConfig&gt;
      * &lt;subnetConfig&gt;
-     *    &lt;name&gt;subnet2&lt;/name&gt;
-     *    &lt;subnet&gt;20.0.0.1/24&lt;/subnet&gt;
-     *    &lt;nodePorts&gt;2/1&gt;/nodePorts&gt;
-     *    &lt;nodePorts&gt;2/2&gt;/nodePorts&gt;
-     *    &lt;nodePorts&gt;2/3&gt;/nodePorts&gt;
+     *    &lt;name&gt;salesdepartment&lt;/name&gt;
+     *    &lt;subnet&gt;20.18.1.254/16&lt;/subnet&gt;
+     *    &lt;nodeConnectors&gt;0F|11@OF|00:00:00:aa:bb:cc:dd:ee&gt;/nodeConnectors&gt;
+     *    &lt;nodeConnectors&gt;0F|13@OF|00:00:00:aa:bb:cc:dd:ee&gt;/nodeConnectors&gt;
      * &lt;/subnetConfig&gt;
      *
      * Response in JSON:
      * {
-     *  "name":"subnet1",
-     *  "subnet":"30.0.0.1/24",
-     *  "nodePorts":["1/1","1/2","1/3"]
+     *  "name":"marketingdepartment",
+     *  "subnet":"30.31.54.254/24",
      * }
      * {
-     *  "name":"subnet2",
-     *  "subnet":"20.0.0.1/24",
-     *  "nodePorts":["2/1","2/2","2/3"]
+     *  "name":"salesdepartment",
+     *  "subnet":"20.18.1.254/16",
+     *  "nodeConnectors":["0F|11@OF|00:00:00:aa:bb:cc:dd:ee", "0F|13@OF|00:00:00:aa:bb:cc:dd:ee"]
      * }
      * </pre>
      */
@@ -179,22 +171,21 @@ public class SubnetsNorthbound {
      *         <pre>
      * Example:
      *
-     * Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/subnet1
+     * Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/marketingdepartment
      *
      * Response in XML:
      * &lt;subnetConfig&gt;
-     *    &lt;name&gt;subnet1&lt;/name&gt;
+     *    &lt;name&gt;marketingdepartment&lt;/name&gt;
      *    &lt;subnet&gt;30.0.0.1/24&lt;/subnet&gt;
-     *    &lt;nodePorts&gt;1/1&gt;/nodePorts&gt;
-     *    &lt;nodePorts&gt;1/2&gt;/nodePorts&gt;
-     *    &lt;nodePorts&gt;1/3&gt;/nodePorts&gt;
+     *    &lt;nodeConnectors&gt;0F|1@OF|00:00:11:22:33:44:55:66&gt;/nodePorts&gt;
+     *    &lt;nodeConnectors&gt;0F|3@OF|00:00:11:22:33:44:55:66&gt;/nodePorts&gt;
      * &lt;/subnetConfig&gt;
      *
      * Response in JSON:
      * {
-     *  "name":"subnet1",
+     *  "name":"marketingdepartment",
      *  "subnet":"30.0.0.1/24",
-     *  "nodePorts":["1/1","1/2","1/3"]
+     *  "nodeConnectors":["0F|1@OF|00:00:11:22:33:44:55:66", "0F|3@OF|00:00:11:22:33:44:55:66"]
      * }
      * </pre>
      */
@@ -221,17 +212,15 @@ public class SubnetsNorthbound {
         SubnetConfig res = switchManager.getSubnetConfig(subnetName);
         if (res == null) {
             throw new ResourceNotFoundException(RestMessages.NOSUBNET.toString());
-        } else {
-            return res;
         }
+        return res;
     }
 
     /**
-     * Add a subnet to a container. If a subnet by the given name already exists
-     * this method will return a non-successful response.
+     * Add a subnet into the specified container context, node connectors are optional
      *
      * @param containerName
-     *            name of the container to which subnet needs to be added
+     *            name of the container context in which the subnet needs to be added
      * @param subnetName
      *            name of new subnet to be added
      * @param subnetConfigData
@@ -242,18 +231,21 @@ public class SubnetsNorthbound {
      *         <pre>
      * Example:
      *
-     * Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/subnet1
+     * Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/salesdepartment
      *
      * Request XML:
      *  &lt;subnetConfig&gt;
-     *      &lt;name&gt;subnet1&lt;/name&gt;
-     *      &lt;subnet&gt;30.0.0.1/24&lt;/subnet&gt;
-     * &lt;/subnetConfig&gt;
+     *      &lt;name&gt;salesdepartment&lt;/name&gt;
+     *      &lt;subnet&gt;172.173.174.254/24&lt;/subnet&gt;
+     *      &lt;nodeConnectors&gt;0F|22@OF|00:00:11:22:33:44:55:66&gt;/nodeConnectors&gt;
+     *      &lt;nodeConnectors&gt;0F|39@OF|00:00:ab:cd:33:44:55:66&gt;/nodeConnectors&gt;
+     *  &lt;/subnetConfig&gt;
      *
      * Request in JSON:
      * {
-     *  "name":"subnet1",
-     *  "subnet":"30.0.0.1/24"
+     *  "name":"salesdepartment",
+     *  "subnet":"172.173.174.254/24"
+     *  "nodeConnectors":["0F|22@OF|00:00:11:22:33:44:55:66", "0F|39@OF|00:00:ab:cd:33:44:55:66"]
      * }
      * </pre>
      */
@@ -284,16 +276,7 @@ public class SubnetsNorthbound {
         if (switchManager == null) {
             throw new ServiceUnavailableException("SwitchManager " + RestMessages.SERVICEUNAVAILABLE.toString());
         }
-        Set<String> ports = cfgObject.getNodePorts();
-        SubnetConfig subnetCfg = null;
-        if (ports == null) {
-            subnetCfg = new SubnetConfig(cfgObject.getName(), cfgObject.getSubnet(), new HashSet<String>(0));
-        } else {
-            subnetCfg = cfgObject;
-        }
-
-        Status status = switchManager.addSubnet(subnetCfg);
-
+        Status status = switchManager.addSubnet(cfgObject);
         if (status.isSuccess()) {
             NorthboundUtils.auditlog("Subnet Gateway", username, "added", subnetName, containerName);
             return Response.status(Response.Status.CREATED).build();
@@ -302,17 +285,17 @@ public class SubnetsNorthbound {
     }
 
     /**
-     * Delete a subnet from a container
+     * Delete a subnet from the specified container context
      *
      * @param containerName
-     *            name of the container from which subnet needs to be removed
+     *            name of the container in which subnet needs to be removed
      * @param subnetName
      *            name of new subnet to be deleted
      * @return Response as dictated by the HTTP Response Status code
      *
      *         <pre>
      * Example:
-     *            Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/subnet1
+     *            Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/engdepartment
      *
      * </pre>
      */
@@ -346,164 +329,51 @@ public class SubnetsNorthbound {
     }
 
     /**
-     * Modify a subnet. For now only changing the port list is allowed.
+     * Modify a subnet. Replace the existing subnet with the new specified one.
+     * For now only port list modification is allowed. If the respective subnet
+     * configuration does not exist this call is equivalent to a subnet
+     * creation.
      *
      * @param containerName
-     *            Name of the Container
+     *            Name of the Container context
      * @param subnetName
-     *            Name of the SubnetConfig to be modified
+     *            Name of the subnet to be modified
      * @param subnetConfigData
-     *            the {@link SubnetConfig} structure in request body
-     *            parameter
-     * @return If the operation is successful or not
+     *            the {@link SubnetConfig} structure in request body parameter
+     * @return Response as dictated by the HTTP Response Status code
      *
      *         <pre>
      * Example:
      *
-     * Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/subnet1/nodePorts
+     * Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/salesdepartment
      *
      *  Request in XML:
      *  &lt;subnetConfig&gt;
-     *      &lt;name&gt;subnet1&lt;/name&gt;
-     *      &lt;subnet&gt;30.0.0.1/24&lt;/subnet&gt;
-     *      &lt;nodePorts&gt;1/1&lt;/nodePorts&gt;
-     *      &lt;nodePorts&gt;1/2&lt;/nodePorts&gt;
-     *      &lt;nodePorts&gt;1/3&lt;/nodePorts&gt;
-     * &lt;/subnetConfig&gt;
+     *      &lt;name&gt;salesdepartment&lt;/name&gt;
+     *      &lt;subnet&gt;172.173.174.254/24&lt;/subnet&gt;
+     *      &lt;nodeConnectors&gt;0F|22@OF|00:00:11:22:33:44:55:66&gt;/nodeConnectors&gt;
+     *      &lt;nodeConnectors&gt;0F|39@OF|00:00:ab:cd:33:44:55:66&gt;/nodeConnectors&gt;
+     *  &lt;/subnetConfig&gt;
      *
      * Request in JSON:
      * {
-     *  "name":"subnet1",
-     *  "subnet":"30.0.0.1/24",
-     *  "nodePorts":["1/1","1/2","1/3"]
+     *  "name":"salesdepartment",
+     *  "subnet":"172.173.174.254/24"
+     *  "nodeConnectors":["0F|22@OF|00:00:11:22:33:44:55:66", "0F|39@OF|00:00:ab:cd:33:44:55:66"]
      * }
      * </pre>
      */
-    @Path("/{containerName}/subnet/{subnetName}/nodePorts")
+    @Path("/{containerName}/subnet/{subnetName}")
     @POST
     @Consumes({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML })
-    @StatusCodes({
-            @ResponseCode(code = 200, condition = "Ports replaced successfully"),
-            @ResponseCode(code = 400, condition = "Invalid request to change subnet name or invalid node ports passed"),
+    @StatusCodes({ @ResponseCode(code = 200, condition = "Configuration replaced successfully"),
             @ResponseCode(code = 401, condition = "User not authorized to perform this operation"),
             @ResponseCode(code = 409, condition = "Subnet name in url conflicts with name in request body"),
             @ResponseCode(code = 404, condition = "The containerName or subnetName is not found"),
-            @ResponseCode(code = 500, condition = "Internal server error: Modify ports failed"),
-            @ResponseCode(code = 503, condition = "Service unavailable") })
-    public Response modifySubnet(@PathParam("containerName") String containerName,
-            @PathParam("subnetName") String subnetName,
-            @TypeHint(SubnetConfig.class) SubnetConfig subnetConfigData) {
-
-        handleContainerDoesNotExist(containerName);
-
-        if (!NorthboundUtils.isAuthorized(getUserName(), containerName, Privilege.WRITE, this)) {
-            throw new UnauthorizedException("User is not authorized to perform this operation on container "
-                    + containerName);
-        }
-        handleNameMismatch(subnetConfigData.getName(), subnetName);
-
-        ISwitchManager switchManager = (ISwitchManager) ServiceHelper.getInstance(ISwitchManager.class, containerName,
-                this);
-        if (switchManager == null) {
-            throw new ServiceUnavailableException("SwitchManager " + RestMessages.SERVICEUNAVAILABLE.toString());
-        }
-
-        SubnetConfig subnetConf = subnetConfigData;
-        SubnetConfig existingConf = switchManager.getSubnetConfig(subnetName);
-
-        boolean successful = true;
-
-        // make sure that the name matches an existing subnet and we're not
-        // changing the name or subnet IP/mask
-        if (existingConf == null) {
-            // don't have a subnet by that name
-            return Response.status(Response.Status.NOT_FOUND).build();
-
-        } else if (!existingConf.getName().equals(subnetConf.getName())
-                || !existingConf.getSubnet().equals(subnetConf.getSubnet())) {
-            // can't change the name of a subnet
-            Response.status(Response.Status.BAD_REQUEST).build();
-        } else {
-            // create a set for fast lookups
-            Set<String> newPorts = new HashSet<String>(subnetConf.getNodePorts());
-
-            // go through the current ports and (1) remove ports that aren't
-            // there anymore and (2) remove ports that are still there from the
-            // set of ports to add
-            for (String s : existingConf.getNodePorts()) {
-                if (newPorts.contains(s)) {
-                    newPorts.remove(s);
-                } else {
-                    Status st = switchManager.removePortsFromSubnet(subnetName, s);
-                    successful = successful && st.isSuccess();
-                }
-            }
-
-            // add any remaining ports
-            for (String s : newPorts) {
-                Status st = switchManager.addPortsToSubnet(subnetName, s);
-                successful = successful && st.isSuccess();
-                if (successful) {
-                    NorthboundUtils.auditlog("Subnet Gateway", username, "added", s + " to " + subnetName,
-                            containerName);
-                }
-            }
-        }
-
-        if (successful) {
-            return Response.status(Response.Status.OK).build();
-        }
-        throw new InternalServerErrorException(RestMessages.INTERNALERROR.toString());
-    }
-
-    /**
-     * Add ports to a subnet in the container.
-     *
-     * @param containerName
-     *            name of the container that has the subnet to which node ports
-     *            need to be added
-     * @param subnetName
-     *            name of subnet to which node ports need to be added
-     * @param SubnetConfig
-     *            the {@link SubnetConfig} structure in request body
-     * @return Response as dictated by the HTTP Response Status code
-     *
-     *         <pre>
-     * Example:
-     *            Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/subnet1/nodePorts
-     *
-     * Request XML:
-     *  &lt;subnetConfig&gt;
-     *      &lt;name&gt;subnet1&lt;/name&gt;
-     *      &lt;subnet&gt;30.0.0.1/24&lt;/subnet&gt;
-     *      &lt;nodePorts&gt;1/1&lt;/nodePorts&gt;
-     *      &lt;nodePorts&gt;1/2&lt;/nodePorts&gt;
-     *      &lt;nodePorts&gt;1/3&lt;/nodePorts&gt;
-     * &lt;/subnetConfig&gt;
-     *
-     * Request in JSON:
-     * {
-     *  "name":"subnet1",
-     *  "subnet":"30.0.0.1/24",
-     *  "nodePorts":["1/1","1/2","1/3"]
-     * }
-     *
-     * </pre>
-     */
-    @Path("/{containerName}/subnet/{subnetName}/nodePorts")
-    @PUT
-    @Consumes({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML })
-    @StatusCodes({
-            @ResponseCode(code = 200, condition = "Added node ports to subnet successfully"),
-            @ResponseCode(code = 400, condition = "Invalid request to change subnet name or invalid node ports passed"),
-            @ResponseCode(code = 401, condition = "User not authorized to perform this operation"),
-            @ResponseCode(code = 404, condition = "The containerName or subnet is not found"),
-            @ResponseCode(code = 409, condition = "Subnet name in url conflicts with name in request body"),
-            @ResponseCode(code = 500, condition = "Internal server error : Port add failed"),
+            @ResponseCode(code = 500, condition = "Internal server error: Modify subnet failed"),
             @ResponseCode(code = 503, condition = "Service unavailable") })
-    public Response addNodePorts(@PathParam("containerName") String containerName,
-            @PathParam("subnetName") String subnetName,
-            @TypeHint(SubnetConfig.class) SubnetConfig subnetConfigData) {
+    public Response modifySubnet(@Context UriInfo uriInfo, @PathParam("containerName") String containerName,
+            @PathParam("subnetName") String subnetName, @TypeHint(SubnetConfig.class) SubnetConfig subnetConfigData) {
 
         handleContainerDoesNotExist(containerName);
 
@@ -513,145 +383,25 @@ public class SubnetsNorthbound {
         }
         handleNameMismatch(subnetConfigData.getName(), subnetName);
 
-        SubnetConfig subnetConf = subnetConfigData;
-
         ISwitchManager switchManager = (ISwitchManager) ServiceHelper.getInstance(ISwitchManager.class, containerName,
                 this);
         if (switchManager == null) {
             throw new ServiceUnavailableException("SwitchManager " + RestMessages.SERVICEUNAVAILABLE.toString());
         }
 
+        // Need to check this until Status does not return a CREATED status code
         SubnetConfig existingConf = switchManager.getSubnetConfig(subnetName);
 
-        // make sure that the name matches an existing subnet and we're not
-        // changing the name or subnet IP/mask
-        if (existingConf == null) {
-            // don't have a subnet by that name
-            return Response.status(Response.Status.NOT_FOUND).build();
-        } else if (!existingConf.getName().equals(subnetConf.getName())
-                || !existingConf.getSubnet().equals(subnetConf.getSubnet())) {
-            // can't change the name of a subnet
-            return Response.status(Response.Status.BAD_REQUEST).build();
-        }
-        Status st;
-        boolean successful = true;
-        Set<String> ports = subnetConf.getNodePorts();
-
-        if (ports == null || ports.isEmpty()) {
-            throw new BadRequestException(RestMessages.INVALIDDATA.toString());
-        }
-
-        // add new ports only
-        if (existingConf.getNodePorts() != null) {
-            ports.removeAll(existingConf.getNodePorts());
-        }
-        for (String port : ports) {
-            st = switchManager.addPortsToSubnet(subnetName, port);
-            successful = successful && st.isSuccess();
-            if (successful) {
-                NorthboundUtils.auditlog("Subnet Gateway", username, "added", st + " to " + subnetName, containerName);
-            }
-        }
-        if (successful) {
-            return Response.status(Response.Status.OK).build();
-        }
-        throw new InternalServerErrorException(RestMessages.INTERNALERROR.toString());
-    }
-
-    /**
-     * Delete ports from a subnet in the container
-     *
-     * @param containerName
-     *            name of the container that has the subnet from which node
-     *            ports need to be deleted
-     * @param subnetName
-     *            name of subnet from which node ports need to be deleted
-     * @param subnetConfigData
-     *            SubnetConfig object to be deleted
-     * @return Response as dictated by the HTTP Response Status code
-     *
-     *         <pre>
-     * Example:
-     *            Request URL: http://localhost:8080/controller/nb/v2/subnetservice/default/subnet/subnet1/nodePorts
-     *
-     * Request XML:
-     *  &lt;subnetConfig&gt;
-     *      &lt;name&gt;subnet3&lt;/name&gt;
-     *      &lt;subnet&gt;30.0.0.1/24&lt;/subnet&gt;
-     *      &lt;nodePorts&gt;1/1,1/2,1/3&lt;/nodePorts&gt;
-     * &lt;/subnetConfig&gt;
-     *
-     * Request in JSON:
-     * { "name" : "subnet1",
-     *   "subnet" : "30.0.0.1/24",
-     *    nodePorts : ["1/1,1/2,1/3"]}
-     *
-     * </pre>
-     */
-    @Path("/{containerName}/subnet/{subnetName}/nodePorts")
-    @DELETE
-    @Consumes({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML })
-    @StatusCodes({
-            @ResponseCode(code = 204, condition = "No content"),
-            @ResponseCode(code = 400, condition = "Invalid request to change subnet name or invalid node ports passed"),
-            @ResponseCode(code = 401, condition = "User not authorized to perform this operation"),
-            @ResponseCode(code = 404, condition = "The containerName or subnet is not found"),
-            @ResponseCode(code = 409, condition = "Subnet name in url conflicts with name in request body"),
-            @ResponseCode(code = 500, condition = "Internal server error : Delete node ports failed"),
-            @ResponseCode(code = 503, condition = "Service unavailable") })
-    public Response deleteNodePorts(@PathParam("containerName") String containerName,
-            @PathParam("subnetName") String subnetName,
-            @TypeHint(SubnetConfig.class) SubnetConfig subnetConfigData) {
-
-        handleContainerDoesNotExist(containerName);
-
-        if (!NorthboundUtils.isAuthorized(getUserName(), containerName, Privilege.WRITE, this)) {
-            throw new UnauthorizedException("User is not authorized to perform this operation on container "
-                    + containerName);
-        }
-        handleNameMismatch(subnetConfigData.getName(), subnetName);
-
-        SubnetConfig subnetConf = subnetConfigData;
-
-        if (subnetConf.getNodePorts() == null || subnetConf.getNodePorts().isEmpty()) {
-            throw new BadRequestException(RestMessages.INVALIDDATA.toString() + " : invalid node ports");
-        }
-
-        ISwitchManager switchManager = (ISwitchManager) ServiceHelper.getInstance(ISwitchManager.class, containerName,
-                this);
-        if (switchManager == null) {
-            throw new ServiceUnavailableException("SwitchManager " + RestMessages.SERVICEUNAVAILABLE.toString());
-        }
-
-        SubnetConfig existingConf = switchManager.getSubnetConfig(subnetName);
+        Status status = switchManager.modifySubnet(subnetConfigData);
 
-        // make sure that the name matches an existing subnet and we're not
-        // changing the name or subnet IP/mask
-        if (existingConf == null) {
-            // don't have a subnet by that name
-            return Response.status(Response.Status.NOT_FOUND).build();
-        } else if (!existingConf.getName().equals(subnetConf.getName())
-                || !existingConf.getSubnet().equals(subnetConf.getSubnet())) {
-            // can't change the name of a subnet
-            return Response.status(Response.Status.BAD_REQUEST).build();
-        }
-        Status st;
-        boolean successful = true;
-        Set<String> ports = subnetConf.getNodePorts();
-
-        // delete existing ports
-        ports.retainAll(existingConf.getNodePorts());
-        for (String port : ports) {
-            st = switchManager.removePortsFromSubnet(subnetName, port);
-            successful = successful && st.isSuccess();
-            if (successful) {
-                NorthboundUtils.auditlog("Subnet Gateway", username, "removed", st + " from " + subnetName,
-                        containerName);
+        if (status.isSuccess()) {
+            if (existingConf == null) {
+                NorthboundUtils.auditlog("Subnet Gateway", username, "created", subnetName, containerName);
+                return Response.created(uriInfo.getRequestUri()).build();
+            } else {
+                NorthboundUtils.auditlog("Subnet Gateway", username, "modified", subnetName, containerName);
             }
         }
-        if (successful) {
-            return Response.noContent().build();
-        }
-        throw new InternalServerErrorException(RestMessages.INTERNALERROR.toString());
+        return NorthboundUtils.getResponse(status);
     }
 }
index f4d302b..d88f9ef 100644 (file)
@@ -63,7 +63,9 @@ public class SwitchNorthbound {
 
     @Context
     public void setSecurityContext(SecurityContext context) {
-        if (context != null && context.getUserPrincipal() != null) username = context.getUserPrincipal().getName();
+        if (context != null && context.getUserPrincipal() != null) {
+            username = context.getUserPrincipal().getName();
+        }
     }
 
     protected String getUserName() {
@@ -222,19 +224,19 @@ public class SwitchNorthbound {
      *            Type of the node being programmed (Eg. 'OF')
      * @param nodeId
      *            Node Identifier as specified by
-     *            {@link org.opendaylight.controller.sal.core.Node}
-     *            (Eg. '00:00:00:00:00:00:00:03')
+     *            {@link org.opendaylight.controller.sal.core.Node} (Eg.
+     *            '00:00:00:00:00:00:00:03')
      * @param propertyName
-     *            Name of the Property. Properties that can be
-     *            configured are: description, forwarding(only for default
-     *            container) and tier
+     *            Name of the Property. Properties that can be configured are:
+     *            description, forwarding(only for default container) and tier
      * @param propertyValue
-     *            Value of the Property. Description can be any string (Eg. 'Node1'),
-     *            valid values for tier are 0, 1 and 2, and valid values for forwarding are 0 for
-     *            reactive and 1 for proactive forwarding.
+     *            Value of the Property. Description can be any string (Eg.
+     *            'Node1'), valid values for tier are non negative numbers, and
+     *            valid values for forwarding are 0 for reactive and 1 for
+     *            proactive forwarding.
      * @return Response as dictated by the HTTP Response Status code
      *
-     * <pre>
+     *         <pre>
      *
      * Example:
      *
index 85a6c22..50ccf69 100644 (file)
 package org.opendaylight.controller.sal.core;
 
 import java.io.Serializable;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
-import javax.xml.bind.annotation.XmlAttribute;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.opendaylight.controller.sal.utils.INodeConnectorFactory;
-import org.opendaylight.controller.sal.utils.INodeFactory;
 import org.opendaylight.controller.sal.utils.ServiceHelper;
 
 /**
@@ -408,28 +409,37 @@ public class NodeConnector implements Serializable {
 
     @Override
     public boolean equals(Object obj) {
-        if (this == obj)
+        if (this == obj) {
             return true;
-        if (obj == null)
+        }
+        if (obj == null) {
             return false;
-        if (getClass() != obj.getClass())
+        }
+        if (getClass() != obj.getClass()) {
             return false;
+        }
         NodeConnector other = (NodeConnector) obj;
         if (nodeConnectorID == null) {
-            if (other.nodeConnectorID != null)
+            if (other.nodeConnectorID != null) {
                 return false;
-        } else if (!nodeConnectorID.equals(other.nodeConnectorID))
+            }
+        } else if (!nodeConnectorID.equals(other.nodeConnectorID)) {
             return false;
+        }
         if (nodeConnectorNode == null) {
-            if (other.nodeConnectorNode != null)
+            if (other.nodeConnectorNode != null) {
                 return false;
-        } else if (!nodeConnectorNode.equals(other.nodeConnectorNode))
+            }
+        } else if (!nodeConnectorNode.equals(other.nodeConnectorNode)) {
             return false;
+        }
         if (nodeConnectorType == null) {
-            if (other.nodeConnectorType != null)
+            if (other.nodeConnectorType != null) {
                 return false;
-        } else if (!nodeConnectorType.equals(other.nodeConnectorType))
+            }
+        } else if (!nodeConnectorType.equals(other.nodeConnectorType)) {
             return false;
+        }
         return true;
     }
 
@@ -484,6 +494,27 @@ public class NodeConnector implements Serializable {
         return fromStringNoNode(parts[0], n);
     }
 
+    /**
+     * return a set of NodeConnector from a collection of string
+     *
+     * @param stringCollection Collection of String object to be parsed as a NodeConnector
+     *
+     * @return the Set of unique NodeConnector objects if parse is successful, empty Set otherwise
+     */
+    public static Set<NodeConnector> fromString(Collection<String> stringCollection) {
+        Set<NodeConnector> set = new HashSet<NodeConnector>();
+        if (stringCollection != null) {
+
+            for (String str : stringCollection) {
+                NodeConnector nodeConnector = NodeConnector.fromString(str);
+                if (nodeConnector != null) {
+                    set.add(nodeConnector);
+                }
+            }
+        }
+        return set;
+    }
+
     /**
      * return a NodeConnector from a string not containing explicitly
      * the Node portion which has to be supplied as parameter
@@ -609,8 +640,9 @@ public class NodeConnector implements Serializable {
             //The protocol plugin being used depends on typeStr.
             INodeConnectorFactory f = (INodeConnectorFactory) ServiceHelper
                     .getGlobalInstance(INodeConnectorFactory.class, new NodeConnector(), "(protocolName="+typeStr+")");
-            if(f==null)
+            if(f==null) {
                 return null;
+            }
             return f.fromStringNoNode(typeStr, IDStr, n);
         }
     }
index ee2c2c2..1af6771 100644 (file)
@@ -31,7 +31,7 @@ public interface ISwitchManager {
      * Add a subnet configuration
      *
      * @param  configObject refer to {@link Open Declaration org.opendaylight.controller.switchmanager.SubnetConfig}
-     * @return "Success" or failure reason
+     * @return the Status object representing the result of the request
      */
     public Status addSubnet(SubnetConfig configObject);
 
@@ -39,10 +39,18 @@ public interface ISwitchManager {
      * Remove a subnet configuration
      *
      * @param  configObject refer to {@link Open Declaration org.opendaylight.controller.switchmanager.SubnetConfig}
-     * @return "Success" or failure reason
+     * @return the Status object representing the result of the request
      */
     public Status removeSubnet(SubnetConfig configObject);
 
+    /**
+     * Modify a subnet configuration
+     *
+     * @param  configObject refer to {@link Open Declaration org.opendaylight.controller.switchmanager.SubnetConfig}
+     * @return the Status object representing the result of the request
+     */
+    public Status modifySubnet(SubnetConfig configObject);
+
     /**
      * Remove a subnet configuration given the name
      *
@@ -163,20 +171,20 @@ public interface ISwitchManager {
     /**
      * Add node connectors to a subnet
      *
-     * @param name The subnet config name
-     * @param nodeConnectors nodePorts string specified by {@link Open Declaration org.opendaylight.controller.switchmanager.SubnetConfig}
-     * @return "Success" or failure reason
+     * @param name The configured subnet name
+     * @param nodeConnectors list of string each representing a node connector as specified by {@link Open Declaration org.opendaylight.controller.sal.core.NodeConnector}
+     * @return The Status object indicating the result of this request
      */
-    public Status addPortsToSubnet(String name, String nodeConnectors);
+    public Status addPortsToSubnet(String name, List<String> nodeConnectors);
 
     /**
      * Remove node connectors from a subnet
      *
-     * @param name              the subnet config name
-     * @param nodeConnectors    nodePorts string specified by {@link Open Declaration org.opendaylight.controller.switchmanager.SubnetConfig}
-     * @return "Success" or failure reason
+     * @param name              the configured subnet name
+     * @param nodeConnectors    list of string each representing a node connector as specified by {@link Open Declaration org.opendaylight.controller.sal.core.NodeConnector}
+     * @return The Status object indicating the result of this request
      */
-    public Status removePortsFromSubnet(String name, String nodeConnectors);
+    public Status removePortsFromSubnet(String name, List<String> nodeConnectors);
 
     /**
      * Return the set of all the nodes
index 1deda7c..f33fa18 100644 (file)
@@ -38,9 +38,9 @@ public class Subnet implements Cloneable, Serializable {
     }
 
     public Subnet(SubnetConfig conf) {
-        networkAddress = conf.getIPnum();
+        networkAddress = conf.getIPAddress();
         subnetMaskLength = conf.getIPMaskLen();
-        nodeConnectors = conf.getSubnetNodeConnectors();
+        nodeConnectors = conf.getNodeConnectors();
     }
 
     public Subnet(Subnet subnet) {
@@ -196,27 +196,36 @@ public class Subnet implements Cloneable, Serializable {
 
     @Override
     public boolean equals(Object obj) {
-        if (this == obj)
+        if (this == obj) {
             return true;
-        if (obj == null)
+        }
+        if (obj == null) {
             return false;
-        if (getClass() != obj.getClass())
+        }
+        if (getClass() != obj.getClass()) {
             return false;
+        }
         Subnet other = (Subnet) obj;
         if (networkAddress == null) {
-            if (other.networkAddress != null)
+            if (other.networkAddress != null) {
                 return false;
-        } else if (!networkAddress.equals(other.networkAddress))
+            }
+        } else if (!networkAddress.equals(other.networkAddress)) {
             return false;
+        }
         if (nodeConnectors == null) {
-            if (other.nodeConnectors != null)
+            if (other.nodeConnectors != null) {
                 return false;
-        } else if (!nodeConnectors.equals(other.nodeConnectors))
+            }
+        } else if (!nodeConnectors.equals(other.nodeConnectors)) {
             return false;
-        if (subnetMaskLength != other.subnetMaskLength)
+        }
+        if (subnetMaskLength != other.subnetMaskLength) {
             return false;
-        if (vlan != other.vlan)
+        }
+        if (vlan != other.vlan) {
             return false;
+        }
         return true;
     }
 
@@ -243,8 +252,9 @@ public class Subnet implements Cloneable, Serializable {
 
     public boolean isMutualExclusive(Subnet otherSubnet) {
         if (this.networkAddress.getClass() != otherSubnet.networkAddress
-                .getClass())
+                .getClass()) {
             return true;
+        }
         if (this.isSubnetOf(otherSubnet.getNetworkAddress())) {
             return false;
         }
index 36ab101..b0d5861 100644 (file)
@@ -13,7 +13,6 @@ import java.io.Serializable;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -22,11 +21,12 @@ import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 
-import org.opendaylight.controller.sal.core.Node;
 import org.opendaylight.controller.sal.core.NodeConnector;
+import org.opendaylight.controller.sal.packet.BitBufferHelper;
 import org.opendaylight.controller.sal.utils.GUIField;
 import org.opendaylight.controller.sal.utils.NetUtils;
-import org.opendaylight.controller.sal.utils.NodeConnectorCreator;
+import org.opendaylight.controller.sal.utils.Status;
+import org.opendaylight.controller.sal.utils.StatusCode;
 
 /**
  * The class represents a subnet configuration.
@@ -34,13 +34,10 @@ import org.opendaylight.controller.sal.utils.NodeConnectorCreator;
 @XmlRootElement
 @XmlAccessorType(XmlAccessType.NONE)
 public class SubnetConfig implements Cloneable, Serializable {
-    //static fields are by default excluded by Gson parser
     private static final long serialVersionUID = 1L;
     private static final String prettyFields[] = { GUIField.NAME.toString(),
             GUIField.GATEWAYIP.toString(), GUIField.NODEPORTS.toString() };
 
-    // Order matters: JSP file expects following fields in the
-    // following order
     /**
      * Name of the subnet
      */
@@ -53,40 +50,40 @@ public class SubnetConfig implements Cloneable, Serializable {
     @XmlElement
     private String subnet;
     /**
-     * node ID/port list:
-     *<node-id>/a,b,c-m,r-t,y
+     * Set of node connectors in the format:
+     * Port Type|Port Id@Node Type|Node Id
      */
     @XmlElement
-    private Set<String> nodePorts;
+    private List<String> nodeConnectors;
 
     public SubnetConfig() {
     }
 
-    public SubnetConfig(String desc, String sub, Set<String> sp) {
-        name = desc;
-        subnet = sub;
-        nodePorts = sp;
+    public SubnetConfig(String name, String subnet, List<String> nodeConnectors) {
+        this.name = name;
+        this.subnet = subnet;
+        this.nodeConnectors = nodeConnectors;
     }
 
     public SubnetConfig(SubnetConfig subnetConfig) {
         name = subnetConfig.name;
         subnet = subnetConfig.subnet;
-        nodePorts = new HashSet<String>(subnetConfig.nodePorts);
+        nodeConnectors = (subnetConfig.nodeConnectors == null) ? null : new ArrayList<String>(subnetConfig.nodeConnectors);
     }
 
     public String getName() {
         return name;
     }
 
-    public Set<String> getNodePorts() {
-        return nodePorts;
+    public List<String> getNodePorts() {
+        return (nodeConnectors == null) ? new ArrayList<String>(0) : new ArrayList<String>(nodeConnectors);
     }
 
     public String getSubnet() {
         return subnet;
     }
 
-    public InetAddress getIPnum() {
+    public InetAddress getIPAddress() {
         InetAddress ip = null;
         try {
             ip = InetAddress.getByName(subnet.split("/")[0]);
@@ -98,87 +95,41 @@ public class SubnetConfig implements Cloneable, Serializable {
 
     public Short getIPMaskLen() {
         Short maskLen = 0;
-        if (hasValidIP()) {
-            String[] s = subnet.split("/");
-            maskLen = (s.length == 2) ? Short.valueOf(s[1]) : 32;
-        }
+        String[] s = subnet.split("/");
+        maskLen = (s.length == 2) ? Short.valueOf(s[1]) : 32;
         return maskLen;
     }
 
-    private Set<Short> getPortList(String ports) {
-        /*
-         * example:
-         *     ports = "1,3,5-12"
-         *     elemArray = ["1" "3" "5-12"]
-         *     elem[2] = "5-12" --> limits = ["5" "12"]
-         *     portList = [1 3 5 6 7 8 9 10 11 12]
-         */
-        Set<Short> portList = new HashSet<Short>();
-        String[] elemArray = ports.split(",");
-        for (String elem : elemArray) {
-            if (elem.contains("-")) {
-                String[] limits = elem.split("-");
-                for (short j = Short.valueOf(limits[0]); j <= Short
-                        .valueOf(limits[1]); j++) {
-                    portList.add(Short.valueOf(j));
-                }
-            } else {
-                portList.add(Short.valueOf(elem));
-            }
+    private Status validateSubnetAddress() {
+        if (!NetUtils.isIPAddressValid(subnet)) {
+            return new Status(StatusCode.BADREQUEST, String.format("Invalid Subnet configuration: Invalid address: %s", subnet));
         }
-        return portList;
-    }
-
-    private boolean hasValidIP() {
-        if (subnet != null) {
-            if (NetUtils.isIPv4AddressValid(subnet)) {
-                return true;
-            } else if (NetUtils.isIPv6AddressValid(subnet)) {
-                return true;
-            }
+        byte[] bytePrefix = NetUtils.getSubnetPrefix(this.getIPAddress(), this.getIPMaskLen()).getAddress();
+        long prefix = BitBufferHelper.getLong(bytePrefix);
+        if (prefix == 0) {
+            return new Status(StatusCode.BADREQUEST, "Invalid network source address: subnet zero");
         }
-        return false;
+        return new Status(StatusCode.SUCCESS);
     }
 
-    private boolean hasValidPorts() {
-        for (String portSet : nodePorts) {
-            if (!portSet.contains("/")) {
-                return false;
+    public static Status validatePorts(List<String> nodeConnectors) {
+        if (nodeConnectors != null) {
+            for (String port : nodeConnectors) {
+                if (null == NodeConnector.fromString(port)) {
+                    return new Status(StatusCode.BADREQUEST,
+                            "Invalid Subnet configuration: Not parsable node connector: " + port);
+                }
             }
         }
-        return true;
-    }
-
-    public boolean isValidSwitchPort(String sp) {
-        return sp.contains("/");
-    }
-
-    public boolean isValidConfig() {
-        return hasValidIP() && hasValidPorts();
-    }
-
-    @Override
-    public int hashCode() {
-        return name.hashCode();
+        return new Status(StatusCode.SUCCESS);
     }
 
-    @Override
-    public boolean equals(Object obj) {
-        /*
-         * Configuration will be stored in collection only if it is valid
-         * Hence we don't check here for uninitialized fields
-         */
-        if (this == obj)
-            return true;
-        if (obj == null)
-            return false;
-        if (getClass() != obj.getClass())
-            return false;
-        SubnetConfig that = (SubnetConfig) obj;
-        if (this.subnet.equals(that.subnet)) {
-            return true;
+    public Status validate() {
+        Status status = validateSubnetAddress();
+        if (status.isSuccess()) {
+            status = validatePorts(this.nodeConnectors);
         }
-        return false;
+        return status;
     }
 
     public static List<String> getGuiFieldsNames() {
@@ -189,66 +140,35 @@ public class SubnetConfig implements Cloneable, Serializable {
         return fieldList;
     }
 
-    //Utility method useful for adding to a passed Set all the
-    //NodeConnectors learnt from a string
-    private void getNodeConnectorsFromString(String codedNodeConnectors,
-            Set<NodeConnector> sp) {
-        if (codedNodeConnectors == null || codedNodeConnectors.isEmpty()) {
-            return;
-        }
-        if (sp == null) {
-            return;
-        }
-        // codedNodeConnectors = nodeId/a,b,c-m,r-t,y
-        String pieces[] = codedNodeConnectors.split("/");
-        for (Short port : getPortList(pieces[1])) {
-            Node n = Node.fromString(pieces[0]);
-            if (n == null) {
-                continue;
-            }
-            NodeConnector p = NodeConnectorCreator.createOFNodeConnector(port,
-                    n);
-            if (p == null) {
-                continue;
-            }
-            sp.add(p);
-        }
-    }
-
-    public Set<NodeConnector> getSubnetNodeConnectors() {
-        Set<NodeConnector> sp = new HashSet<NodeConnector>();
-        if (isGlobal())
-            return sp;
-        for (String str : nodePorts) {
-            getNodeConnectorsFromString(str, sp);
-        }
-        return sp;
-    }
-
-    public Set<NodeConnector> getNodeConnectors(String codedNodeConnectors) {
-        // codedNodeConnectors = nodeId/a,b,c-m,r-t,y
-        Set<NodeConnector> sp = new HashSet<NodeConnector>();
-        getNodeConnectorsFromString(codedNodeConnectors, sp);
-        return sp;
+    public Set<NodeConnector> getNodeConnectors() {
+        return NodeConnector.fromString(this.nodeConnectors);
     }
 
     public boolean isGlobal() {
         // If no ports are specified to be part of the domain, then it's a global domain IP
-        return (nodePorts == null || nodePorts.isEmpty());
+        return (nodeConnectors == null || nodeConnectors.isEmpty());
     }
 
-    public void addNodeConnectors(String sp) {
-        nodePorts.add(sp);
+    public void addNodeConnectors(List<String> nc) {
+        if (nc != null) {
+            if (nodeConnectors == null) {
+                nodeConnectors = new ArrayList<String>(nc);
+            } else {
+                nodeConnectors.addAll(nc);
+            }
+        }
     }
 
-    public void removeNodeConnectors(String sp) {
-        nodePorts.remove(sp);
+    public void removeNodeConnectors(List<String> nc) {
+        if (nc != null && nodeConnectors != null) {
+            nodeConnectors.removeAll(nc);
+        }
     }
 
     @Override
     public String toString() {
-        return ("SubnetConfig [Description=" + name + ", Subnet=" + subnet
-                + ", NodeConnectors=" + nodePorts + "]");
+        return ("SubnetConfig [Name=" + name + ", Subnet=" + subnet
+                + ", NodeConnectors=" + nodeConnectors + "]");
     }
 
     /**
@@ -259,4 +179,49 @@ public class SubnetConfig implements Cloneable, Serializable {
         return new SubnetConfig(this);
     }
 
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((name == null) ? 0 : name.hashCode());
+        result = prime * result + ((nodeConnectors == null) ? 0 : nodeConnectors.hashCode());
+        result = prime * result + ((subnet == null) ? 0 : subnet.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null) {
+            return false;
+        }
+        if (getClass() != obj.getClass()) {
+            return false;
+        }
+        SubnetConfig other = (SubnetConfig) obj;
+        if (name == null) {
+            if (other.name != null) {
+                return false;
+            }
+        } else if (!name.equals(other.name)) {
+            return false;
+        }
+        if (nodeConnectors == null) {
+            if (other.nodeConnectors != null) {
+                return false;
+            }
+        } else if (!nodeConnectors.equals(other.nodeConnectors)) {
+            return false;
+        }
+        if (subnet == null) {
+            if (other.subnet != null) {
+                return false;
+            }
+        } else if (!subnet.equals(other.subnet)) {
+            return false;
+        }
+        return true;
+    }
 }
index a665096..e0c120a 100644 (file)
@@ -66,7 +66,7 @@ public class Switch implements Serializable {
      * @return the nodeConnectors
      */
     public Set<NodeConnector> getNodeConnectors() {
-        return nodeConnectors;
+        return new HashSet<NodeConnector>(nodeConnectors);
     }
 
     /**
@@ -87,7 +87,7 @@ public class Switch implements Serializable {
     }
 
     public List<NodeConnector> getSpanPorts() {
-        return spanPorts;
+        return new ArrayList<NodeConnector>(this.spanPorts);
     }
 
     public Node getNode() {
diff --git a/opendaylight/switchmanager/api/src/test/java/org/opendaylight/controller/switchmanager/SubnetConfigTest.java b/opendaylight/switchmanager/api/src/test/java/org/opendaylight/controller/switchmanager/SubnetConfigTest.java
new file mode 100644 (file)
index 0000000..b410063
--- /dev/null
@@ -0,0 +1,55 @@
+package org.opendaylight.controller.switchmanager;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.Assert;
+import org.junit.Test;
+import org.opendaylight.controller.sal.core.ConstructionException;
+import org.opendaylight.controller.sal.core.Node;
+import org.opendaylight.controller.sal.core.NodeConnector;
+import org.opendaylight.controller.sal.utils.Status;
+
+public class SubnetConfigTest {
+
+    @Test
+    public void configuration() throws ConstructionException {
+        // Create the node connector string list
+        Node node1 = new Node(Node.NodeIDType.OPENFLOW, 1L);
+        Node node2 = new Node(Node.NodeIDType.OPENFLOW, 2L);
+        Node node3 = new Node(Node.NodeIDType.OPENFLOW, 3L);
+        NodeConnector nc1 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)1, node1);
+        NodeConnector nc2 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)2, node2);
+        NodeConnector nc3 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)3, node3);
+        List<String> portList = new ArrayList<String>();
+        portList.add(nc1.toString());
+        portList.add(nc2.toString());
+        portList.add(nc3.toString());
+
+        // Full subnet creation
+        SubnetConfig config = new SubnetConfig("eng", "11.1.1.254/16", portList);
+        Status status = config.validate();
+        Assert.assertTrue(status.isSuccess());
+
+        // No port set specified
+        config = new SubnetConfig("eng", "11.1.1.254/16", null);
+        status = config.validate();
+        Assert.assertTrue(status.isSuccess());
+
+        // Empty port set
+        config = new SubnetConfig("eng", "11.1.1.254/16", new ArrayList<String>(0));
+        status = config.validate();
+        Assert.assertTrue(status.isSuccess());
+
+        // Zero subnet
+        config = new SubnetConfig("eng", "1.2.3.254/1", null);
+        status = config.validate();
+        Assert.assertFalse(status.isSuccess());
+
+        // Port set with invalid port notation
+        List<String> badPortList = new ArrayList<String>();
+        badPortList.add("1/1");
+        config = new SubnetConfig("eng", "1.2.3.254/1", badPortList);
+        status = config.validate();
+        Assert.assertFalse(status.isSuccess());
+    }
+}
index 2f49895..64b57eb 100644 (file)
@@ -24,10 +24,8 @@ import org.opendaylight.controller.sal.core.Node;
 import org.opendaylight.controller.sal.core.NodeConnector;
 import org.opendaylight.controller.sal.core.Property;
 import org.opendaylight.controller.sal.core.Tier;
-import org.opendaylight.controller.sal.utils.GlobalConstants;
 import org.opendaylight.controller.sal.utils.NodeConnectorCreator;
 import org.opendaylight.controller.sal.utils.NodeCreator;
-import org.opendaylight.controller.sal.utils.ServiceHelper;
 
 public class SwitchTest {
 
@@ -88,13 +86,15 @@ public class SwitchTest {
                 .getSpanPorts();
 
         Assert.assertEquals(node, resultNode);
-        for (int i = 0; i < dlAddress.length; i++)
+        for (int i = 0; i < dlAddress.length; i++) {
             Assert.assertEquals(dlAddress[i], resultdlAddress[i]);
+        }
 
         Assert.assertTrue(ncSet.equals(resultncSet));
 
-        for (int i = 0; i < portList.size(); i++)
+        for (int i = 0; i < portList.size(); i++) {
             Assert.assertEquals(portList.get(i), resultSpanPort.get(i));
+        }
     }
 
     @Test
index e81b4cd..f177d10 100644 (file)
@@ -9,11 +9,6 @@
 
 package org.opendaylight.controller.switchmanager.internal;
 
-import java.util.Dictionary;
-import java.util.HashSet;
-import java.util.Hashtable;
-import java.util.Set;
-
 import org.apache.felix.dm.Component;
 import org.opendaylight.controller.clustering.services.IClusterContainerServices;
 import org.opendaylight.controller.configuration.IConfigurationContainerAware;
index d82c996..49cdabf 100644 (file)
@@ -193,7 +193,6 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
     public void shutDown() {
     }
 
-    @SuppressWarnings("deprecation")
     private void allocateCaches() {
         if (this.clusterContainerService == null) {
             this.nonClusterObjectCreate();
@@ -211,9 +210,6 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
                     EnumSet.of(IClusterServices.cacheMode.TRANSACTIONAL));
             clusterContainerService.createCache("switchmanager.subnets",
                     EnumSet.of(IClusterServices.cacheMode.TRANSACTIONAL));
-            clusterContainerService.createCache(
-                    "switchmanager.configSaveEvent",
-                    EnumSet.of(IClusterServices.cacheMode.TRANSACTIONAL));
             clusterContainerService.createCache("switchmanager.nodeProps",
                     EnumSet.of(IClusterServices.cacheMode.TRANSACTIONAL));
             clusterContainerService.createCache(
@@ -232,7 +228,7 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
         }
     }
 
-    @SuppressWarnings({ "unchecked", "deprecation" })
+    @SuppressWarnings({ "unchecked" })
     private void retrieveCaches() {
         if (this.clusterContainerService == null) {
             log.info("un-initialized clusterContainerService, can't create cache");
@@ -378,7 +374,7 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
 
     private Status updateDatabase(SubnetConfig conf, boolean add) {
         if (add) {
-            Subnet subnetCurr = subnets.get(conf.getIPnum());
+            Subnet subnetCurr = subnets.get(conf.getIPAddress());
             Subnet subnet;
             if (subnetCurr == null) {
                 subnet = new Subnet(conf);
@@ -388,25 +384,24 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
             // In case of API3 call we may receive the ports along with the
             // subnet creation
             if (!conf.isGlobal()) {
-                Set<NodeConnector> sp = conf.getSubnetNodeConnectors();
-                subnet.addNodeConnectors(sp);
+                subnet.addNodeConnectors(conf.getNodeConnectors());
             }
             boolean putNewSubnet = false;
             if(subnetCurr == null) {
-                if(subnets.putIfAbsent(conf.getIPnum(), subnet) == null) {
+                if(subnets.putIfAbsent(conf.getIPAddress(), subnet) == null) {
                     putNewSubnet = true;
                 }
             } else {
-                putNewSubnet = subnets.replace(conf.getIPnum(), subnetCurr, subnet);
+                putNewSubnet = subnets.replace(conf.getIPAddress(), subnetCurr, subnet);
             }
             if(!putNewSubnet) {
-                String msg = "Cluster conflict: Conflict while adding the subnet " + conf.getIPnum();
+                String msg = "Cluster conflict: Conflict while adding the subnet " + conf.getIPAddress();
                 return new Status(StatusCode.CONFLICT, msg);
             }
 
         // Subnet removal case
         } else {
-            subnets.remove(conf.getIPnum());
+            subnets.remove(conf.getIPAddress());
         }
         return new Status(StatusCode.SUCCESS);
     }
@@ -428,11 +423,11 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
     }
 
     private Status addRemoveSubnet(SubnetConfig conf, boolean isAdding) {
-        // Valid config check
-        if (!conf.isValidConfig()) {
-            String msg = "Invalid Subnet configuration";
-            log.warn(msg);
-            return new Status(StatusCode.BADREQUEST, msg);
+        // Valid configuration check
+        Status status = conf.validate();
+        if (!status.isSuccess()) {
+            log.warn(status.getDescription());
+            return status;
         }
 
         if (isAdding) {
@@ -442,24 +437,24 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
                         "Subnet with the specified name already configured.");
             }
             // Semantic check
-            Status rc = semanticCheck(conf);
-            if (!rc.isSuccess()) {
-                return rc;
+            status = semanticCheck(conf);
+            if (!status.isSuccess()) {
+                return status;
             }
         }
 
         // Update Database
-        Status rc = updateDatabase(conf, isAdding);
+        status = updateDatabase(conf, isAdding);
 
-        if (rc.isSuccess()) {
+        if (status.isSuccess()) {
             // Update Configuration
-            rc = updateConfig(conf, isAdding);
-            if(!rc.isSuccess()) {
+            status = updateConfig(conf, isAdding);
+            if(!status.isSuccess()) {
                 updateDatabase(conf, (!isAdding));
             }
         }
 
-        return rc;
+        return status;
     }
 
     /**
@@ -485,26 +480,100 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
     }
 
     @Override
-    public Status addPortsToSubnet(String name, String switchPorts) {
+    public Status modifySubnet(SubnetConfig conf) {
+        // Sanity check
+        if (conf == null) {
+            return new Status(StatusCode.BADREQUEST, "Invalid Subnet configuration: null");
+        }
+
+        // Valid configuration check
+        Status status = conf.validate();
+        if (!status.isSuccess()) {
+            log.warn(status.getDescription());
+            return status;
+        }
+
+        // If a subnet configuration with this name does not exist, consider this is a creation
+        SubnetConfig target = subnetsConfigList.get(conf.getName());
+        if (target == null) {
+            return this.addSubnet(conf);
+        }
+
+        // No change
+        if (target.equals(conf)) {
+            return new Status(StatusCode.SUCCESS);
+        }
+
+        // Check not allowed modifications
+        if (!target.getSubnet().equals(conf.getSubnet())) {
+            return new Status(StatusCode.BADREQUEST, "IP address change is not allowed");
+        }
+
+        // Derive the set of node connectors that are being removed
+        Set<NodeConnector> toRemove = target.getNodeConnectors();
+        toRemove.removeAll(conf.getNodeConnectors());
+        List<String> nodeConnectorStrings = null;
+        if (!toRemove.isEmpty()) {
+            nodeConnectorStrings = new ArrayList<String>();
+            for (NodeConnector nc : toRemove) {
+                nodeConnectorStrings.add(nc.toString());
+            }
+            status = this.removePortsFromSubnet(conf.getName(), nodeConnectorStrings);
+            if (!status.isSuccess()) {
+                return status;
+            }
+        }
+
+        // Derive the set of node connectors that are being added
+        Set<NodeConnector> toAdd = conf.getNodeConnectors();
+        toAdd.removeAll(target.getNodeConnectors());
+        if (!toAdd.isEmpty()) {
+            List<String> nodeConnectorStringRemoved = nodeConnectorStrings;
+            nodeConnectorStrings = new ArrayList<String>();
+            for (NodeConnector nc : toAdd) {
+                nodeConnectorStrings.add(nc.toString());
+            }
+            status = this.addPortsToSubnet(conf.getName(), nodeConnectorStrings);
+            if (!status.isSuccess()) {
+                // If any port was removed, add it back as a best recovery effort
+                if (!toRemove.isEmpty()) {
+                    this.addPortsToSubnet(conf.getName(), nodeConnectorStringRemoved);
+                }
+                return status;
+            }
+        }
+
+        // Update Configuration
+        subnetsConfigList.put(conf.getName(), conf);
+
+        return new Status(StatusCode.SUCCESS);
+    }
+
+    @Override
+    public Status addPortsToSubnet(String name, List<String> switchPorts) {
+        if (name == null) {
+            return new Status(StatusCode.BADREQUEST, "Null subnet name");
+        }
         SubnetConfig confCurr = subnetsConfigList.get(name);
         if (confCurr == null) {
             return new Status(StatusCode.NOTFOUND, "Subnet does not exist");
         }
-        if (!confCurr.isValidSwitchPort(switchPorts)) {
-            return new Status(StatusCode.BADREQUEST, "Invalid switchports");
+
+        if (switchPorts == null || switchPorts.isEmpty()) {
+            return new Status(StatusCode.BADREQUEST, "Null or empty port set");
         }
 
-        Subnet subCurr = subnets.get(confCurr.getIPnum());
+        Subnet subCurr = subnets.get(confCurr.getIPAddress());
         if (subCurr == null) {
-            log.debug("Cluster conflict: Subnet entry {} is not present in the subnets cache.", confCurr.getIPnum());
+            log.debug("Cluster conflict: Subnet entry {} is not present in the subnets cache.", confCurr.getIPAddress());
             return new Status(StatusCode.NOTFOUND, "Subnet does not exist");
         }
 
         // Update Database
         Subnet sub = subCurr.clone();
-        Set<NodeConnector> sp = confCurr.getNodeConnectors(switchPorts);
+        Set<NodeConnector> sp = NodeConnector.fromString(switchPorts);
         sub.addNodeConnectors(sp);
-        boolean subnetsReplaced = subnets.replace(confCurr.getIPnum(), subCurr, sub);
+        boolean subnetsReplaced = subnets.replace(confCurr.getIPAddress(), subCurr, sub);
         if (!subnetsReplaced) {
             String msg = "Cluster conflict: Conflict while adding ports to the subnet " + name;
             return new Status(StatusCode.CONFLICT, msg);
@@ -524,23 +593,35 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
     }
 
     @Override
-    public Status removePortsFromSubnet(String name, String switchPorts) {
+    public Status removePortsFromSubnet(String name, List<String> switchPorts) {
+        if (name == null) {
+            return new Status(StatusCode.BADREQUEST, "Null subnet name");
+        }
         SubnetConfig confCurr = subnetsConfigList.get(name);
         if (confCurr == null) {
             return new Status(StatusCode.NOTFOUND, "Subnet does not exist");
         }
 
-        Subnet subCurr = subnets.get(confCurr.getIPnum());
+        if (switchPorts == null || switchPorts.isEmpty()) {
+            return new Status(StatusCode.BADREQUEST, "Null or empty port set");
+        }
+
+        Subnet subCurr = subnets.get(confCurr.getIPAddress());
         if (subCurr == null) {
-            log.debug("Cluster conflict: Subnet entry {} is not present in the subnets cache.", confCurr.getIPnum());
+            log.debug("Cluster conflict: Subnet entry {} is not present in the subnets cache.", confCurr.getIPAddress());
             return new Status(StatusCode.NOTFOUND, "Subnet does not exist");
         }
 
+        // Validation check
+        Status status = SubnetConfig.validatePorts(switchPorts);
+        if (!status.isSuccess()) {
+            return status;
+        }
         // Update Database
         Subnet sub = subCurr.clone();
-        Set<NodeConnector> sp = confCurr.getNodeConnectors(switchPorts);
+        Set<NodeConnector> sp = NodeConnector.fromString(switchPorts);
         sub.deleteNodeConnectors(sp);
-        boolean subnetsReplace = subnets.replace(confCurr.getIPnum(), subCurr, sub);
+        boolean subnetsReplace = subnets.replace(confCurr.getIPAddress(), subCurr, sub);
         if (!subnetsReplace) {
             String msg = "Cluster conflict: Conflict while removing ports from the subnet " + name;
             return new Status(StatusCode.CONFLICT, msg);
@@ -632,6 +713,7 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
         }
     }
 
+    @SuppressWarnings("deprecation")
     @Override
     public void updateSwitchConfig(SwitchConfig cfgObject) {
         // update default container only
@@ -1816,9 +1898,7 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
         Switch sw = getSwitchByNode(node);
 
         ci.println("          NodeConnector                        Name");
-        if (sw == null) {
-            return;
-        }
+
         Set<NodeConnector> nodeConnectorSet = sw.getNodeConnectors();
         String nodeConnectorName;
         if (nodeConnectorSet != null && nodeConnectorSet.size() > 0) {
@@ -2056,6 +2136,7 @@ public class SwitchManager implements ISwitchManager, IConfigurationContainerAwa
         return null;
     }
 
+    @SuppressWarnings("deprecation")
     @Override
     public String getNodeDescription(Node node) {
         // Check first if user configured a name
index d74c3d3..97747e9 100644 (file)
@@ -8,12 +8,15 @@
 
 package org.opendaylight.controller.switchmanager.internal;
 
+import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 import org.junit.Assert;
 import org.junit.Test;
 import org.opendaylight.controller.sal.core.Bandwidth;
+import org.opendaylight.controller.sal.core.ConstructionException;
 import org.opendaylight.controller.sal.core.Latency;
 import org.opendaylight.controller.sal.core.Node;
 import org.opendaylight.controller.sal.core.NodeConnector;
@@ -28,28 +31,75 @@ import org.opendaylight.controller.switchmanager.SubnetConfig;
 public class SwitchManagerTest {
 
     @Test
-    public void testSwitchManagerAddRemoveSubnet() {
+    public void testSwitchManagerAddRemoveSubnet() throws ConstructionException {
         SwitchManager switchmgr = new SwitchManager();
         switchmgr.startUp();
 
-        Set<String> portList = new HashSet<String>();
-        portList.add("1/1");
-        portList.add("1/2");
-        portList.add("1/3");
+        // Create the node connector string list
+        Node node1 = new Node(Node.NodeIDType.OPENFLOW, 1L);
+        Node node2 = new Node(Node.NodeIDType.OPENFLOW, 2L);
+        NodeConnector nc1 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)1, node1);
+        NodeConnector nc2 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)2, node2);
+        NodeConnector nc3 = new NodeConnector(NodeConnector.NodeConnectorIDType.OPENFLOW, (short)3, node1);
+        List<String> portList = new ArrayList<String>();
+        portList.add(nc1.toString());
+        portList.add(nc2.toString());
+        portList.add(nc3.toString());
 
-        SubnetConfig subnet = new SubnetConfig("subnet", "10.0.0.254/16",
-                portList);
-        // System.out.println("*" + switchmgr.addSubnet(subnet) + "*");
+
+        SubnetConfig subnet = new SubnetConfig("subnet", "10.0.0.254/16", portList);
         Status addResult = (switchmgr.addSubnet(subnet));
         Assert.assertTrue(addResult.isSuccess());
 
         Status removeResult = (switchmgr.removeSubnet(subnet.getName()));
         Assert.assertTrue(removeResult.isSuccess());
 
-        SubnetConfig subnetConfigResult = switchmgr.getSubnetConfig(subnet
-                .getName());
-        Assert.assertTrue(subnetConfigResult == null);
+        SubnetConfig subnetConfigResult = switchmgr.getSubnetConfig(subnet.getName());
+        Assert.assertNull(subnetConfigResult);
+
+        subnet = new SubnetConfig("hr", "0.0.0.0", portList);
+        Status status = switchmgr.addSubnet(subnet);
+        Assert.assertFalse(status.isSuccess());
+
+        subnet = new SubnetConfig("hr", "12.12.12.254/16", null);
+        status = switchmgr.addSubnet(subnet);
+        Assert.assertTrue(status.isSuccess());
+
+    }
+
+    @Test
+    public void testSwitchManagerAddRemovePortsToSubnet() {
+        SwitchManager switchmgr = new SwitchManager();
+        switchmgr.startUp();
+
+        List<String> portList = new ArrayList<String>();
+        portList.add("OF|1@OF|1");
+        portList.add("OF|2@OF|00:00:00:00:00:00:00:02");
+        portList.add("OF|3@OF|00:00:00:00:00:00:00:01");
+
+        SubnetConfig subnet = new SubnetConfig("eng", "11.1.1.254/16", portList);
+        Status status = (switchmgr.addSubnet(subnet));
+        Assert.assertTrue(status.isSuccess());
+
+
+        // Empty port set
+        List<String> badPortSet = new ArrayList<String>();
+        status = switchmgr.addPortsToSubnet("eng", badPortSet);
+        Assert.assertFalse(status.isSuccess());
+
+        // Non existant subnet
+        status = switchmgr.removePortsFromSubnet("hr", badPortSet);
+        Assert.assertFalse(status.isSuccess());
+
+        // Port set containing non conventional but parsable port
+        badPortSet.add("1/1");
+        status = switchmgr.addPortsToSubnet("eng", badPortSet);
+        Assert.assertTrue(status.isSuccess());
 
+        // Port set containing non parsable port
+        badPortSet.add("OF1/1");
+        status = switchmgr.addPortsToSubnet("eng", badPortSet);
+        Assert.assertTrue(status.isSuccess());
     }
 
     @Test
index f670477..4e08f99 100644 (file)
@@ -10,7 +10,6 @@ package org.opendaylight.controller.devices.web;
 
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -60,9 +59,9 @@ import com.google.gson.Gson;
 @RequestMapping("/")
 public class Devices implements IDaylightWeb {
     private static final UserLevel AUTH_LEVEL = UserLevel.CONTAINERUSER;
-    private final String WEB_NAME = "Devices";
-    private final String WEB_ID = "devices";
-    private final short WEB_ORDER = 1;
+    private static final String WEB_NAME = "Devices";
+    private static final String WEB_ID = "devices";
+    private static final short WEB_ORDER = 1;
 
     public Devices() {
         ServiceHelper.registerGlobalService(IDaylightWeb.class, this, null);
@@ -400,35 +399,32 @@ public class Devices implements IDaylightWeb {
             @RequestParam(required = false) String container) {
         Gson gson = new Gson();
         List<Map<String, String>> subnets = new ArrayList<Map<String, String>>();
-        String containerName = (container == null) ? GlobalConstants.DEFAULT
-                .toString() : container;
+        String containerName = (container == null) ? GlobalConstants.DEFAULT.toString() : container;
 
         // Derive the privilege this user has on the current container
         String userName = request.getUserPrincipal().getName();
-        Privilege privilege = DaylightWebUtil.getContainerPrivilege(
-                userName, containerName, this);
+        Privilege privilege = DaylightWebUtil.getContainerPrivilege(userName, containerName, this);
 
         if (privilege != Privilege.NONE) {
-            ISwitchManager switchManager = (ISwitchManager) ServiceHelper
-                    .getInstance(ISwitchManager.class, containerName, this);
+            ISwitchManager switchManager = (ISwitchManager) ServiceHelper.getInstance(ISwitchManager.class,
+                    containerName, this);
             if (switchManager != null) {
                 for (SubnetConfig conf : switchManager.getSubnetsConfigList()) {
                     Map<String, String> subnet = new HashMap<String, String>();
                     subnet.put("name", conf.getName());
                     subnet.put("subnet", conf.getSubnet());
                     List<SubnetGatewayPortBean> portsList = new ArrayList<SubnetGatewayPortBean>();
-                    Iterator<NodeConnector> itor = conf.getSubnetNodeConnectors().iterator();
+                    Iterator<NodeConnector> itor = conf.getNodeConnectors().iterator();
                     while(itor.hasNext()) {
                         SubnetGatewayPortBean bean = new SubnetGatewayPortBean();
                         NodeConnector nodeConnector = itor.next();
                         String nodeName = getNodeDesc(nodeConnector.getNode().toString(), containerName);
                         Name ncName = ((Name) switchManager.getNodeConnectorProp(nodeConnector, Name.NamePropName));
                         String nodeConnectorName = (ncName != null) ? ncName.getValue() : "";
-                        nodeConnectorName += " (" + nodeConnector.getID() + ")";
                         bean.setNodeName(nodeName);
                         bean.setNodePortName(nodeConnectorName);
                         bean.setNodeId(nodeConnector.getNode().toString());
-                        bean.setNodePortId(nodeConnector.getID().toString());
+                        bean.setNodePortId(nodeConnector.toString());
                         portsList.add(bean);
                     }
                     subnet.put("nodePorts", gson.toJson(portsList));
@@ -464,7 +460,7 @@ public class Devices implements IDaylightWeb {
             ISwitchManager switchManager = (ISwitchManager) ServiceHelper
                     .getInstance(ISwitchManager.class, containerName, this);
             SubnetConfig cfgObject = new SubnetConfig(gatewayName,
-                    gatewayIPAddress, new HashSet<String>());
+                    gatewayIPAddress, new ArrayList<String>());
             Status result = switchManager.addSubnet(cfgObject);
             if (result.isSuccess()) {
                 resultBean.setStatus(true);
@@ -539,14 +535,17 @@ public class Devices implements IDaylightWeb {
         try {
             ISwitchManager switchManager = (ISwitchManager) ServiceHelper
                     .getInstance(ISwitchManager.class, containerName, this);
-            Status result = switchManager.addPortsToSubnet(portsName, nodeId
-                    + "/" + ports);
+            List<String> toAdd = new ArrayList<String>();
+            for (String port : ports.split(",")) {
+                toAdd.add(port);
+            }
+            Status result = switchManager.addPortsToSubnet(portsName, toAdd);
 
             if (result.isSuccess()) {
                 resultBean.setStatus(true);
                 resultBean
                         .setMessage("Added ports to subnet gateway address successfully");
-                DaylightWebUtil.auditlog("Ports to Subnet Gateway", userName, "added",nodeId+"/"+ ports, containerName);
+                DaylightWebUtil.auditlog("Ports to Subnet Gateway", userName, "added", ports, containerName);
             } else {
                 resultBean.setStatus(false);
                 resultBean.setMessage(result.getDescription());
@@ -578,13 +577,15 @@ public class Devices implements IDaylightWeb {
         try {
             ISwitchManager switchManager = (ISwitchManager) ServiceHelper
                     .getInstance(ISwitchManager.class, containerName, this);
-            Status result = switchManager.removePortsFromSubnet(gatewayName,
-                    nodePort);
+            List<String> toRemove = new ArrayList<String>();
+            for (String port : nodePort.split(",")) {
+                toRemove.add(port);
+            }
+            Status result = switchManager.removePortsFromSubnet(gatewayName, toRemove);
 
             if (result.isSuccess()) {
                 resultBean.setStatus(true);
-                resultBean
-                        .setMessage("Deleted port from subnet gateway address successfully");
+                resultBean.setMessage("Deleted port from subnet gateway address successfully");
                 DaylightWebUtil.auditlog("Ports from Subnet Gateway", userName, "removed", nodePort, containerName);
             } else {
                 resultBean.setStatus(false);
@@ -630,7 +631,7 @@ public class Devices implements IDaylightWeb {
                     Map<String, String> config = new HashMap<String, String>();
                     for (String name : config_data.keySet()) {
                         config.put(name, config_data.get(name));
-                        // Add switch name value (non-configuration field)
+                        // Add switch portName value (non-configuration field)
                         config.put("nodeName",
                                 getNodeDesc(config_data.get("nodeId"), containerName));
                     }
@@ -651,10 +652,8 @@ public class Devices implements IDaylightWeb {
 
     @RequestMapping(value = "/nodeports")
     @ResponseBody
-    public List<NodeJsonBean> getNodePorts(HttpServletRequest request,
-            @RequestParam(required = false) String container) {
-        String containerName = (container == null) ? GlobalConstants.DEFAULT
-                .toString() : container;
+    public String getNodePorts(HttpServletRequest request, @RequestParam(required = false) String container) {
+        String containerName = (container == null) ? GlobalConstants.DEFAULT.toString() : container;
 
         // Derive the privilege this user has on the current container
         String userName = request.getUserPrincipal().getName();
@@ -671,16 +670,13 @@ public class Devices implements IDaylightWeb {
 
         for (Switch node : switchManager.getNetworkDevices()) {
             NodeJsonBean nodeJsonBean = new NodeJsonBean();
-            List<String> port = new ArrayList<String>();
+            List<PortJsonBean> port = new ArrayList<PortJsonBean>();
             Set<NodeConnector> nodeConnectorSet = node.getNodeConnectors();
-
             if (nodeConnectorSet != null) {
                 for (NodeConnector nodeConnector : nodeConnectorSet) {
                     String nodeConnectorName = ((Name) switchManager
-                            .getNodeConnectorProp(nodeConnector,
-                                    Name.NamePropName)).getValue();
-                    port.add(nodeConnectorName
-                            + "(" + nodeConnector.getID() + ")");
+                            .getNodeConnectorProp(nodeConnector, Name.NamePropName)).getValue();
+                    port.add(new PortJsonBean(nodeConnector.getID().toString(), nodeConnectorName, nodeConnector.toString()));
                 }
             }
             nodeJsonBean.setNodeId(node.getNode().toString());
@@ -689,7 +685,7 @@ public class Devices implements IDaylightWeb {
             nodeJsonBeans.add(nodeJsonBean);
         }
 
-        return nodeJsonBeans;
+        return new Gson().toJson(nodeJsonBeans);
     }
 
     @RequestMapping(value = "/spanPorts/add", method = RequestMethod.GET)
index 0fd933a..61d4f2f 100644 (file)
@@ -13,7 +13,7 @@ import java.util.List;
 public class NodeJsonBean {
     String nodeId;
     String nodeName;
-    List<String> nodePorts;
+    List<PortJsonBean> nodePorts;
 
     public String getNodeId() {
         return nodeId;
@@ -23,7 +23,7 @@ public class NodeJsonBean {
         return nodeName;
     }
 
-    public List<String> getNodePorts() {
+    public List<PortJsonBean> getNodePorts() {
         return nodePorts;
     }
 
@@ -35,7 +35,7 @@ public class NodeJsonBean {
         this.nodeName = nodeName;
     }
 
-    public void setNodePorts(List<String> port) {
+    public void setNodePorts(List<PortJsonBean> port) {
         this.nodePorts = port;
     }
 
diff --git a/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/PortJsonBean.java b/opendaylight/web/devices/src/main/java/org/opendaylight/controller/devices/web/PortJsonBean.java
new file mode 100644 (file)
index 0000000..09b90d0
--- /dev/null
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2013 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.opendaylight.controller.devices.web;
+
+public class PortJsonBean {
+    String portId;
+    String portName;
+    String internalPortName;
+
+    public PortJsonBean() {
+        this.portId = null;
+        this.portName = null;
+        this.internalPortName = null;
+    }
+
+    public PortJsonBean(String id, String name, String internalName) {
+        this.portId = id;
+        this.portName = (name == null) ? internalName : name;
+        this.internalPortName = internalName;
+    }
+}
index 4124630..ba0d2a2 100644 (file)
@@ -10,6 +10,7 @@ package org.opendaylight.controller.devices.web;
 public class SubnetGatewayPortBean {
     private String nodeName;
     private String nodeId;
+    private PortJsonBean port;
     private String nodePortId;
     private String nodePortName;
 
index 3fd292b..813df23 100644 (file)
@@ -680,7 +680,11 @@ one.f.switchmanager.subnetGatewayConfig = {
                     one.f.switchmanager.subnetGatewayConfig.registry['currentNode'] = node;
                     var $ports = $('#' + one.f.switchmanager.subnetGatewayConfig.id.modal.form.ports);
                     var ports = nodeports[node];
-                    one.lib.form.select.inject($ports, ports);
+                    var options = {};
+                    $(ports).each(function(idx, val) {
+                        options[val.internalPortName] = val.portName+' ('+val.portId+')'; 
+                    });
+                    one.lib.form.select.inject($ports, options);
                     one.lib.form.select.prepend($ports, { '' : 'Please Select a Port' });
                     $ports.val($ports.find("option:first").val());
                 });
@@ -797,7 +801,7 @@ one.f.switchmanager.subnetGatewayConfig = {
                         },
                         {
                             property: 'nodePorts',
-                            label: 'Node/Ports',
+                            label: 'Ports',
                             sortable: false
                         }
                     ],
@@ -810,10 +814,9 @@ one.f.switchmanager.subnetGatewayConfig = {
                             var nodePorts = JSON.parse(json);
                             var nodePortHtml = "<div>";
                             $.each(nodePorts, function(index, nodePort) {
-                                var nodePortID = nodePort["nodeId"] + "/" + nodePort["nodePortId"]; 
-                                nodePortHtml += nodePort["nodeName"] + " / " + nodePort["nodePortName"];
+                                nodePortHtml += nodePort["nodePortName"] + " @ " + nodePort["nodeName"];
                                 nodePortHtml += "&nbsp;";
-                                nodePortHtml += '<a href="#" id=' + encodeURIComponent(nodePortID) + 
+                                nodePortHtml += '<a href="#" id=' + encodeURIComponent(nodePort["nodePortId"]) + 
                                     ' gatewayName=' + tableRow["name"] + 
                                     ' onclick="javascript:one.f.switchmanager.subnetGatewayConfig.actions.deleteNodePort(this);">Remove</a>';
                                 nodePortHtml += "<br/>";
@@ -1327,7 +1330,11 @@ one.f.switchmanager.spanPortConfig = {
                 one.f.switchmanager.spanPortConfig.registry['currentNode'] = nodeId;
                 var $ports = $('#' + one.f.switchmanager.spanPortConfig.id.modal.form.port);
                 var ports = one.f.switchmanager.spanPortConfig.registry['nodePorts'][nodeId]
-                one.lib.form.select.inject($ports, ports); 
+                var options = {};
+                $(ports).each(function(idx, val) {
+                    options[val.internalPortName] = val.portName+' ('+val.portId+')'; 
+                });
+                one.lib.form.select.inject($ports, options); 
             });
 
             $fieldset.append($label).append($select);