CORD-352 Refactoring SegmentRoutingConfig

- Add Javadoc and fix function name convention
- Add setAdjancencySids method
- Change return value of getAdjacencySids from List to ImmutableSet
- Validate config value
- Add unit test for SegmentRoutingConfig

Change-Id: Ic43ac31a49da8a9d62131d7803930280cf9994d2

@@ -89,6 +89,11 @@
             <groupId>org.osgi</groupId>
             <groupId>org.osgi</groupId>
             <artifactId>org.osgi.core</artifactId>
             <artifactId>org.osgi.core</artifactId>
         </dependency>
         </dependency>
+        <dependency>
+            <groupId>org.onosproject</groupId>
+            <artifactId>onlab-junit</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
     </dependencies>
 
 
     <build>
     <build>

@@ -158,7 +158,7 @@ public class TunnelHandler {
 
 
     private int createGroupsForTunnel(Tunnel tunnel) {
     private int createGroupsForTunnel(Tunnel tunnel) {
 
 
-        List<Integer> portNumbers;
+        Set<Integer> portNumbers;
         final int groupError = -1;
         final int groupError = -1;
 
 
         DeviceId deviceId = config.getDeviceId(tunnel.labelIds().get(0));
         DeviceId deviceId = config.getDeviceId(tunnel.labelIds().get(0));

@@ -16,7 +16,6 @@
 package org.onosproject.segmentrouting.config;
 package org.onosproject.segmentrouting.config;
 
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
 import org.onlab.packet.Ip4Address;
 import org.onlab.packet.Ip4Address;
 import org.onlab.packet.Ip4Prefix;
 import org.onlab.packet.Ip4Prefix;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MacAddress;
@@ -26,7 +25,6 @@ import org.onosproject.incubator.net.intf.Interface;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.config.NetworkConfigRegistry;
 import org.onosproject.net.config.NetworkConfigRegistry;
 import org.onosproject.net.host.InterfaceIpAddress;
 import org.onosproject.net.host.InterfaceIpAddress;
-import org.onosproject.segmentrouting.config.SegmentRoutingConfig.AdjacencySid;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.PortNumber;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
@@ -34,6 +32,7 @@ import org.slf4j.LoggerFactory;
 
 
 import java.util.ArrayList;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.List;
 import java.util.Map;
 import java.util.Map;
 import java.util.Set;
 import java.util.Set;
@@ -60,7 +59,7 @@ public class DeviceConfiguration implements DeviceProperties {
         boolean isEdge;
         boolean isEdge;
         HashMap<PortNumber, Ip4Address> gatewayIps;
         HashMap<PortNumber, Ip4Address> gatewayIps;
         HashMap<PortNumber, Ip4Prefix> subnets;
         HashMap<PortNumber, Ip4Prefix> subnets;
-        List<AdjacencySid> adjacencySids;
+        Map<Integer, Set<Integer>> adjacencySids;
 
 
         public SegmentRouterInfo() {
         public SegmentRouterInfo() {
             this.gatewayIps = new HashMap<>();
             this.gatewayIps = new HashMap<>();
@@ -83,11 +82,11 @@ public class DeviceConfiguration implements DeviceProperties {
                 cfgService.getConfig(subject, SegmentRoutingConfig.class);
                 cfgService.getConfig(subject, SegmentRoutingConfig.class);
             SegmentRouterInfo info = new SegmentRouterInfo();
             SegmentRouterInfo info = new SegmentRouterInfo();
             info.deviceId = subject;
             info.deviceId = subject;
-            info.nodeSid = config.getSid();
-            info.ip = config.getIp();
-            info.mac = config.getMac();
+            info.nodeSid = config.nodeSid();
+            info.ip = config.routerIp();
+            info.mac = config.routerMac();
             info.isEdge = config.isEdgeRouter();
             info.isEdge = config.isEdgeRouter();
-            info.adjacencySids = config.getAdjacencySids();
+            info.adjacencySids = config.adjacencySids();
 
 
             this.deviceConfigMap.put(info.deviceId, info);
             this.deviceConfigMap.put(info.deviceId, info);
             this.allSegmentIds.add(info.nodeSid);
             this.allSegmentIds.add(info.nodeSid);
@@ -410,19 +409,13 @@ public class DeviceConfiguration implements DeviceProperties {
      *
      *
      * @param deviceId device identification of the router
      * @param deviceId device identification of the router
      * @param sid adjacency Sid
      * @param sid adjacency Sid
-     * @return list of port numbers
+     * @return set of port numbers
      */
      */
-    public List<Integer> getPortsForAdjacencySid(DeviceId deviceId, int sid) {
+    public Set<Integer> getPortsForAdjacencySid(DeviceId deviceId, int sid) {
         SegmentRouterInfo srinfo = deviceConfigMap.get(deviceId);
         SegmentRouterInfo srinfo = deviceConfigMap.get(deviceId);
-        if (srinfo != null) {
-            for (AdjacencySid asid : srinfo.adjacencySids) {
-                if (asid.getAsid() == sid) {
-                    return asid.getPorts();
-                }
-            }
-        }
-
-        return Lists.newArrayList();
+        return srinfo != null ?
+                ImmutableSet.copyOf(srinfo.adjacencySids.get(sid)) :
+                ImmutableSet.copyOf(new HashSet<>());
     }
     }
 
 
     /**
     /**
@@ -435,20 +428,6 @@ public class DeviceConfiguration implements DeviceProperties {
      */
      */
     public boolean isAdjacencySid(DeviceId deviceId, int sid) {
     public boolean isAdjacencySid(DeviceId deviceId, int sid) {
         SegmentRouterInfo srinfo = deviceConfigMap.get(deviceId);
         SegmentRouterInfo srinfo = deviceConfigMap.get(deviceId);
-        if (srinfo != null) {
-            if (srinfo.adjacencySids.isEmpty()) {
-                return false;
-            } else {
-                for (AdjacencySid asid:
-                        srinfo.adjacencySids) {
-                    if (asid.getAsid() == sid) {
-                        return true;
-                    }
-                }
-                return false;
-            }
-        }
-
-        return false;
+        return srinfo != null && srinfo.adjacencySids.containsKey(sid);
     }
     }
 }
 }
\ No newline at end of file

@@ -16,113 +16,210 @@
 
 
 package org.onosproject.segmentrouting.config;
 package org.onosproject.segmentrouting.config;
 
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.ImmutableMap;
 import org.onlab.packet.Ip4Address;
 import org.onlab.packet.Ip4Address;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MacAddress;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.config.Config;
 import org.onosproject.net.config.Config;
-import org.onosproject.net.config.basics.BasicElementConfig;
 
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Optional;
+import java.util.Set;
 
 
 /**
 /**
  * Configuration object for Segment Routing Application.
  * Configuration object for Segment Routing Application.
  */
  */
 public class SegmentRoutingConfig extends Config<DeviceId> {
 public class SegmentRoutingConfig extends Config<DeviceId> {
-    private static final String NAME = "name";
-    private static final String IP = "routerIp";
-    private static final String MAC = "routerMac";
-    private static final String SID = "nodeSid";
-    private static final String EDGE = "isEdgeRouter";
-    private static final String ADJSID = "adjacencySids";
-
-    public Optional<String> getName() {
+    public static final String NAME = "name";
+    public static final String IP = "routerIp";
+    public static final String MAC = "routerMac";
+    public static final String SID = "nodeSid";
+    public static final String EDGE = "isEdgeRouter";
+    public static final String ADJSIDS = "adjacencySids";
+    public static final String ADJSID = "adjSid";
+    public static final String PORTS = "ports";
+
+    @Override
+    public boolean isValid() {
+        return hasOnlyFields(NAME, IP, MAC, SID, EDGE, ADJSIDS, ADJSID, PORTS) &&
+                this.name() != null &&
+                this.routerIp() != null &&
+                this.routerMac() != null &&
+                this.nodeSid() != -1 &&
+                this.isEdgeRouter() != null &&
+                this.adjacencySids() != null;
+    }
+
+    /**
+     * Gets the name of the router.
+     *
+     * @return Optional name of the router. May be empty if not configured.
+     */
+    public Optional<String> name() {
         String name = get(NAME, null);
         String name = get(NAME, null);
         return name != null ? Optional.of(name) : Optional.empty();
         return name != null ? Optional.of(name) : Optional.empty();
     }
     }
 
 
-    public BasicElementConfig setName(String name) {
-        return (BasicElementConfig) setOrClear(NAME, name);
+    /**
+     * Sets the name of the router.
+     *
+     * @param name name of the router.
+     * @return the config of the router.
+     */
+    public SegmentRoutingConfig setName(String name) {
+        return (SegmentRoutingConfig) setOrClear(NAME, name);
     }
     }
 
 
-    public Ip4Address getIp() {
+    /**
+     * Gets the IP address of the router.
+     *
+     * @return IP address of the router. Or null if not configured.
+     */
+    public Ip4Address routerIp() {
         String ip = get(IP, null);
         String ip = get(IP, null);
         return ip != null ? Ip4Address.valueOf(ip) : null;
         return ip != null ? Ip4Address.valueOf(ip) : null;
     }
     }
 
 
-    public BasicElementConfig setIp(String ip) {
-        return (BasicElementConfig) setOrClear(IP, ip);
+    /**
+     * Sets the IP address of the router.
+     *
+     * @param ip IP address of the router.
+     * @return the config of the router.
+     */
+    public SegmentRoutingConfig setRouterIp(String ip) {
+        return (SegmentRoutingConfig) setOrClear(IP, ip);
     }
     }
 
 
-    public MacAddress getMac() {
+    /**
+     * Gets the MAC address of the router.
+     *
+     * @return MAC address of the router. Or null if not configured.
+     */
+    public MacAddress routerMac() {
         String mac = get(MAC, null);
         String mac = get(MAC, null);
         return mac != null ? MacAddress.valueOf(mac) : null;
         return mac != null ? MacAddress.valueOf(mac) : null;
     }
     }
 
 
-    public BasicElementConfig setMac(String mac) {
-        return (BasicElementConfig) setOrClear(MAC, mac);
+    /**
+     * Sets the MAC address of the router.
+     *
+     * @param mac MAC address of the router.
+     * @return the config of the router.
+     */
+    public SegmentRoutingConfig setRouterMac(String mac) {
+        return (SegmentRoutingConfig) setOrClear(MAC, mac);
     }
     }
 
 
-    public int getSid() {
+    /**
+     * Gets the node SID of the router.
+     *
+     * @return node SID of the router. Or -1 if not configured.
+     */
+    public int nodeSid() {
         return get(SID, -1);
         return get(SID, -1);
     }
     }
 
 
-    public BasicElementConfig setSid(int sid) {
-        return (BasicElementConfig) setOrClear(SID, sid);
+    /**
+     * Sets the node SID of the router.
+     *
+     * @param sid node SID of the router.
+     * @return the config of the router.
+     */
+    public SegmentRoutingConfig setNodeSid(int sid) {
+        return (SegmentRoutingConfig) setOrClear(SID, sid);
     }
     }
 
 
-    public boolean isEdgeRouter() {
-        return get(EDGE, false);
+    /**
+     * Checks if the router is an edge router.
+     *
+     * @return true if the router is an edge router.
+     *         false if the router is not an edge router.
+     *         null if the value is not configured.
+     */
+    public Boolean isEdgeRouter() {
+        String isEdgeRouter = get(EDGE, null);
+        return isEdgeRouter != null ?
+                Boolean.valueOf(isEdgeRouter) :
+                null;
     }
     }
 
 
-    public BasicElementConfig setEdgeRouter(boolean isEdgeRouter) {
-        return (BasicElementConfig) setOrClear(EDGE, isEdgeRouter);
+    /**
+     * Specifies if the router is an edge router.
+     *
+     * @param isEdgeRouter true if the router is an edge router.
+     * @return the config of the router.
+     */
+    public SegmentRoutingConfig setIsEdgeRouter(boolean isEdgeRouter) {
+        return (SegmentRoutingConfig) setOrClear(EDGE, isEdgeRouter);
     }
     }
 
 
-    public List<AdjacencySid> getAdjacencySids() {
-        ArrayList<AdjacencySid> adjacencySids = new ArrayList<>();
-
-        if (!object.has(ADJSID)) {
-            return adjacencySids;
+    /**
+     * Gets the adjacency SIDs of the router.
+     *
+     * @return adjacency SIDs of the router. Or null if not configured.
+     */
+    public Map<Integer, Set<Integer>> adjacencySids() {
+        if (!object.has(ADJSIDS)) {
+            return null;
         }
         }
 
 
-        ArrayNode adjacencySidNodes = (ArrayNode) object.path(ADJSID);
-        adjacencySidNodes.forEach(adjacencySidNode -> {
-            int asid = adjacencySidNode.path(AdjacencySid.ASID).asInt();
-
-            ArrayList<Integer> ports = new ArrayList<Integer>();
-            ArrayNode portsNodes = (ArrayNode) adjacencySidNode.path(AdjacencySid.PORTS);
-            portsNodes.forEach(portNode -> {
-                ports.add(portNode.asInt());
-            });
-
-            AdjacencySid adjacencySid = new AdjacencySid(asid, ports);
-            adjacencySids.add(adjacencySid);
-        });
+        Map<Integer, Set<Integer>> adjacencySids = new HashMap<>();
+        ArrayNode adjacencySidsNode = (ArrayNode) object.path(ADJSIDS);
+        for (JsonNode adjacencySidNode : adjacencySidsNode) {
+            int asid = adjacencySidNode.path(ADJSID).asInt(-1);
+            if (asid == -1) {
+                return null;
+            }
+
+            HashSet<Integer> ports = new HashSet<>();
+            ArrayNode portsNode = (ArrayNode) adjacencySidNode.path(PORTS);
+            for (JsonNode portNode : portsNode) {
+                int port = portNode.asInt(-1);
+                if (port == -1) {
+                    return null;
+                }
+                ports.add(port);
+            }
+            adjacencySids.put(asid, ports);
+        }
 
 
-        return adjacencySids;
+        return ImmutableMap.copyOf(adjacencySids);
     }
     }
 
 
-    public class AdjacencySid {
-        private static final String ASID = "adjSid";
-        private static final String PORTS = "ports";
-
-        int asid;
-        List<Integer> ports;
-
-        public AdjacencySid(int asid, List<Integer> ports) {
-            this.asid = asid;
-            this.ports = ports;
-        }
+    /**
+     * Sets the adjacency SIDs of the router.
+     *
+     * @param adjacencySids adjacency SIDs of the router.
+     * @return the config of the router.
+     */
+    public SegmentRoutingConfig setAdjacencySids(Map<Integer, Set<Integer>> adjacencySids) {
+        if (adjacencySids == null) {
+            object.remove(ADJSIDS);
+        } else {
+            ArrayNode adjacencySidsNode = mapper.createArrayNode();
+
+            adjacencySids.forEach((sid, ports) -> {
+                ObjectNode adjacencySidNode = mapper.createObjectNode();
+
+                adjacencySidNode.put(ADJSID, sid);
+
+                ArrayNode portsNode = mapper.createArrayNode();
+                ports.forEach(port -> {
+                    portsNode.add(port.toString());
+                });
+                adjacencySidNode.set(PORTS, portsNode);
+
+                adjacencySidsNode.add(adjacencySidNode);
+            });
 
 
-        public int getAsid() {
-            return asid;
+            object.set(ADJSIDS, adjacencySidsNode);
         }
         }
 
 
-        public List<Integer> getPorts() {
-            return ports;
-        }
+        return this;
     }
     }
 }
 }
\ No newline at end of file

+/*
+ * Copyright 2014-2015 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.segmentrouting.config;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.packet.IpAddress;
+import org.onlab.packet.MacAddress;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.config.Config;
+import org.onosproject.net.config.ConfigApplyDelegate;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.Assert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests for class {@link SegmentRoutingConfig}.
+ */
+public class SegmentRoutingConfigTest {
+    private SegmentRoutingConfig config;
+    private Map<Integer, Set<Integer>> adjacencySids1;
+    private Map<Integer, Set<Integer>> adjacencySids2;
+
+    @Before
+    public void setUp() throws Exception {
+        String jsonString = "{" +
+                "\"name\" : \"Leaf-R1\"," +
+                "\"nodeSid\" : 101," +
+                "\"routerIp\" : \"10.0.1.254\"," +
+                "\"routerMac\" : \"00:00:00:00:01:80\"," +
+                "\"isEdgeRouter\" : true," +
+                "\"adjacencySids\" : [" +
+                "    { \"adjSid\" : 100, \"ports\" : [2, 3] }," +
+                "    { \"adjSid\" : 200, \"ports\" : [4, 5] }" +
+                "]}";
+
+        adjacencySids1 = new HashMap<>();
+        Set<Integer> ports1 = new HashSet<>();
+        ports1.add(2);
+        ports1.add(3);
+        adjacencySids1.put(100, ports1);
+        Set<Integer> ports2 = new HashSet<>();
+        ports2.add(4);
+        ports2.add(5);
+        adjacencySids1.put(200, ports2);
+
+        adjacencySids2 = new HashMap<>();
+        Set<Integer> ports3 = new HashSet<>();
+        ports3.add(6);
+        adjacencySids2.put(300, ports3);
+
+        DeviceId subject = DeviceId.deviceId("of:0000000000000001");
+        String key = "org.onosproject.segmentrouting";
+        ObjectMapper mapper = new ObjectMapper();
+        JsonNode jsonNode = mapper.readTree(jsonString);
+        ConfigApplyDelegate delegate = new MockDelegate();
+
+        config = new SegmentRoutingConfig();
+        config.init(subject, key, jsonNode, mapper, delegate);
+    }
+
+    @Test
+    public void testName() throws Exception {
+        assertTrue(config.name().isPresent());
+        assertThat(config.name().get(), is("Leaf-R1"));
+    }
+
+    @Test
+    public void testSetName() throws Exception {
+        config.setName("Spine-R1");
+        assertTrue(config.name().isPresent());
+        assertThat(config.name().get(), is("Spine-R1"));
+    }
+
+    @Test
+    public void testRouterIp() throws Exception {
+        assertThat(config.routerIp(), is(IpAddress.valueOf("10.0.1.254")));
+    }
+
+    @Test
+    public void testSetRouterIp() throws Exception {
+        config.setRouterIp("10.0.2.254");
+        assertThat(config.routerIp(), is(IpAddress.valueOf("10.0.2.254")));
+    }
+
+    @Test
+    public void testRouterMac() throws Exception {
+        assertThat(config.routerMac(), is(MacAddress.valueOf("00:00:00:00:01:80")));
+    }
+
+    @Test
+    public void testSetRouterMac() throws Exception {
+        config.setRouterMac("00:00:00:00:02:80");
+        assertThat(config.routerMac(), is(MacAddress.valueOf("00:00:00:00:02:80")));
+    }
+
+    @Test
+    public void testNodeSid() throws Exception {
+        assertThat(config.nodeSid(), is(101));
+    }
+
+    @Test
+    public void testSetNodeSid() throws Exception {
+        config.setNodeSid(200);
+        assertThat(config.nodeSid(), is(200));
+    }
+
+    @Test
+    public void testIsEdgeRouter() throws Exception {
+        assertThat(config.isEdgeRouter(), is(true));
+    }
+
+    @Test
+    public void testSetIsEdgeRouter() throws Exception {
+        config.setIsEdgeRouter(false);
+        assertThat(config.isEdgeRouter(), is(false));
+    }
+
+    @Test
+    public void testAdjacencySids() throws Exception {
+        assertThat(config.adjacencySids(), is(adjacencySids1));
+    }
+
+    @Test
+    public void testSetAdjacencySids() throws Exception {
+        config.setAdjacencySids(adjacencySids2);
+        assertThat(config.adjacencySids(), is(adjacencySids2));
+    }
+
+    private class MockDelegate implements ConfigApplyDelegate {
+        @Override
+        public void onApply(Config config) {
+        }
+    }
+}
\ No newline at end of file

@@ -28,8 +28,8 @@
                 "routerMac" : "00:00:00:00:01:80",
                 "routerMac" : "00:00:00:00:01:80",
                 "isEdgeRouter" : true,
                 "isEdgeRouter" : true,
                 "adjacencySids" : [
                 "adjacencySids" : [
-                    { "sid" : 100, "ports" : [2, 3] },
-                    { "sid" : 200, "ports" : [4, 5] }
+                    { "adjSids" : 100, "ports" : [2, 3] },
+                    { "adjSids" : 200, "ports" : [4, 5] }
                 ]
                 ]
             }
             }
         },
         },