Add optional 'name' field in BGP speaker config

Change-Id: If6f4567cd1a7c29b0424cc84bef050efe392544c

@@ -20,9 +20,10 @@ import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.collect.Sets;
 import org.onlab.packet.IpAddress;
 import org.onosproject.core.ApplicationId;
-import org.onosproject.net.config.Config;
 import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.config.Config;
 
+import java.util.Optional;
 import java.util.Set;
 
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -34,6 +35,7 @@ public class BgpConfig extends Config<ApplicationId> {
 
     public static final String SPEAKERS = "bgpSpeakers";
     public static final String CONNECT_POINT = "connectPoint";
+    public static final String NAME = "name";
     public static final String PEERS = "peers";
 
     // TODO add methods for updating config
@@ -52,7 +54,15 @@ public class BgpConfig extends Config<ApplicationId> {
             jsonNode.path(PEERS).forEach(addressNode ->
                             listenAddresses.add(IpAddress.valueOf(addressNode.asText()))
             );
-            speakers.add(new BgpSpeakerConfig(
+
+            Optional<String> name;
+            if (jsonNode.get(NAME) == null) {
+                name = Optional.empty();
+            } else {
+                name = Optional.of(jsonNode.get(NAME).asText());
+            }
+
+            speakers.add(new BgpSpeakerConfig(name,
                     ConnectPoint.deviceConnectPoint(jsonNode.path(CONNECT_POINT).asText()),
                     listenAddresses));
         });
@@ -65,14 +75,21 @@ public class BgpConfig extends Config<ApplicationId> {
      */
     public static class BgpSpeakerConfig {
 
+        private Optional<String> name;
         private ConnectPoint connectPoint;
         private Set<IpAddress> peers;
 
-        public BgpSpeakerConfig(ConnectPoint connectPoint, Set<IpAddress> peers) {
+        public BgpSpeakerConfig(Optional<String> name, ConnectPoint connectPoint,
+                                Set<IpAddress> peers) {
+            this.name = checkNotNull(name);
             this.connectPoint = checkNotNull(connectPoint);
             this.peers = checkNotNull(peers);
         }
 
+        public Optional<String> name() {
+            return name;
+        }
+
         public ConnectPoint connectPoint() {
             return connectPoint;
         }

@@ -16,14 +16,20 @@
 
 package org.onosproject.routing.cli;
 
+import com.google.common.collect.Lists;
 import org.apache.karaf.shell.commands.Command;
 import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.cli.Comparators;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
 import org.onosproject.net.config.NetworkConfigService;
 import org.onosproject.routing.RoutingService;
 import org.onosproject.routing.config.BgpConfig;
 
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
 /**
  * Lists the BGP speakers configured in the system.
  */
@@ -31,7 +37,11 @@ import org.onosproject.routing.config.BgpConfig;
         description = "Lists all BGP speakers")
 public class BgpSpeakersListCommand extends AbstractShellCommand {
 
-    private static final String FORMAT = "%s : %s";
+    private static final String FORMAT = "port=%s/%s, peers=%s";
+    private static final String NAME_FORMAT = "%s: " + FORMAT;
+
+    private static final Comparator<BgpConfig.BgpSpeakerConfig> SPEAKERS_COMPARATOR = (s1, s2) ->
+            Comparators.CONNECT_POINT_COMPARATOR.compare(s1.connectPoint(), s2.connectPoint());
 
     @Override
     protected void execute() {
@@ -39,15 +49,26 @@ public class BgpSpeakersListCommand extends AbstractShellCommand {
         CoreService coreService = get(CoreService.class);
         ApplicationId appId = coreService.getAppId(RoutingService.ROUTER_APP_ID);
 
-        print(appId.toString());
-
         BgpConfig config = configService.getConfig(appId, BgpConfig.class);
 
+        List<BgpConfig.BgpSpeakerConfig> bgpSpeakers =
+                Lists.newArrayList(config.bgpSpeakers());
+
+        Collections.sort(bgpSpeakers, SPEAKERS_COMPARATOR);
+
         if (config == null || config.bgpSpeakers().isEmpty()) {
             print("No speakers configured");
         } else {
-            config.bgpSpeakers().forEach(
-                    s -> print(FORMAT, s.connectPoint(), s.peers()));
+            bgpSpeakers.forEach(
+                s -> {
+                    if (s.name().isPresent()) {
+                        print(NAME_FORMAT, s.name().get(), s.connectPoint().deviceId(),
+                                s.connectPoint().port(), s.peers());
+                    } else {
+                        print(FORMAT, s.connectPoint().deviceId(),
+                                s.connectPoint().port(), s.peers());
+                    }
+                });
         }
     }
 }

@@ -56,6 +56,7 @@ import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 
 import static org.easymock.EasyMock.createMock;
@@ -149,9 +150,11 @@ public class PeerConnectivityManagerTest extends AbstractIntentTest {
     private Set<BgpConfig.BgpSpeakerConfig> setUpBgpSpeakers() {
 
         BgpConfig.BgpSpeakerConfig speaker1 = new BgpConfig.BgpSpeakerConfig(
+                Optional.empty(),
                 s1Eth100, Collections.singleton(IpAddress.valueOf("192.168.10.1")));
 
         BgpConfig.BgpSpeakerConfig speaker2 = new BgpConfig.BgpSpeakerConfig(
+                Optional.empty(),
                 s1Eth100, Sets.newHashSet(IpAddress.valueOf("192.168.20.1"),
                 IpAddress.valueOf("192.168.30.1")));
 

@@ -96,7 +96,7 @@ public abstract class Config<S> {
      * @return JSON node backing the configuration
      */
     public JsonNode node() {
-        return object;
+        return node;
     }
 
     /**