Javadoc and code cleanup for SDN-IP

Change-Id: I4b2cd853cb2c91ace2c710b215a7b2dca9301c28

@@ -15,6 +15,8 @@
  */
 package org.onosproject.sdnip;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.LinkedList;
@@ -27,6 +29,7 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.Semaphore;
 
 import org.apache.commons.lang3.tuple.Pair;
+import org.onlab.packet.Ip4Prefix;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.flow.criteria.Criteria.IPCriterion;
 import org.onosproject.net.flow.criteria.Criterion;
@@ -36,14 +39,15 @@ import org.onosproject.net.intent.IntentService;
 import org.onosproject.net.intent.IntentState;
 import org.onosproject.net.intent.MultiPointToSinglePointIntent;
 import org.onosproject.net.intent.PointToPointIntent;
-import org.onlab.packet.Ip4Prefix;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
+/**
+ * Synchronizes intents between the in-memory intent store and the
+ * IntentService.
+ */
 public class IntentSynchronizer {
     private static final Logger log =
         LoggerFactory.getLogger(IntentSynchronizer.class);
@@ -148,6 +152,11 @@ public class IntentSynchronizer {
         }
     }
 
+    /**
+     * Signals the synchronizer that the SDN-IP leadership has changed.
+     *
+     * @param isLeader true if this instance is now the leader, otherwise false
+     */
     public void leaderChanged(boolean isLeader) {
         log.debug("SDN-IP Leader changed: {}", isLeader);
 

@@ -19,6 +19,10 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
+import org.onlab.packet.Ethernet;
+import org.onlab.packet.IPv4;
+import org.onlab.packet.IpAddress;
+import org.onlab.packet.IpPrefix;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.flow.DefaultTrafficSelector;
@@ -32,10 +36,6 @@ import org.onosproject.sdnip.config.BgpSpeaker;
 import org.onosproject.sdnip.config.Interface;
 import org.onosproject.sdnip.config.InterfaceAddress;
 import org.onosproject.sdnip.config.SdnIpConfigurationService;
-import org.onlab.packet.Ethernet;
-import org.onlab.packet.IPv4;
-import org.onlab.packet.IpAddress;
-import org.onlab.packet.IpPrefix;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -56,10 +56,10 @@ public class PeerConnectivityManager {
     /**
      * Creates a new PeerConnectivityManager.
      *
-     * @param appId             the application ID
+     * @param appId              the application ID
      * @param intentSynchronizer the intent synchronizer
-     * @param configService     the SDN-IP config service
-     * @param interfaceService  the interface service
+     * @param configService      the SDN-IP config service
+     * @param interfaceService   the interface service
      */
     public PeerConnectivityManager(ApplicationId appId,
                                    IntentSynchronizer intentSynchronizer,
@@ -75,20 +75,12 @@ public class PeerConnectivityManager {
      * Starts the peer connectivity manager.
      */
     public void start() {
-        // TODO are any of these errors?
         if (interfaceService.getInterfaces().isEmpty()) {
-
-            log.warn("The interface in configuration file is empty. "
-                             + "Thus, the SDN-IP application can not be started.");
+            log.warn("No interfaces found in configuration file");
         } else if (configService.getBgpPeers().isEmpty()) {
-
-            log.warn("The BGP peer in configuration file is empty."
-                             + "Thus, the SDN-IP application can not be started.");
-        } else if (configService.getBgpSpeakers() == null) {
-
-            log.error("The BGP speaker in configuration file is empty. "
-                              + "Thus, the SDN-IP application can not be started.");
-            return;
+            log.warn("No BGP peers found in configuration file");
+        } else if (configService.getBgpSpeakers().isEmpty()) {
+            log.error("No BGP speakers found in configuration file");
         }
 
         setUpConnectivity();
@@ -174,8 +166,7 @@ public class PeerConnectivityManager {
 
         TrafficSelector selector;
 
-        // install intent for BGP path from BGPd to BGP peer matching
-        // destination TCP port 179
+        // Path from BGP speaker to BGP peer matching destination TCP port 179
         selector = buildSelector(IPv4.PROTOCOL_TCP,
                                  bgpdAddress,
                                  bgpdPeerAddress,
@@ -185,8 +176,7 @@ public class PeerConnectivityManager {
         intents.add(new PointToPointIntent(appId, selector, treatment,
                                bgpdConnectPoint, bgpdPeerConnectPoint));
 
-        // install intent for BGP path from BGPd to BGP peer matching
-        // source TCP port 179
+        // Path from BGP speaker to BGP peer matching source TCP port 179
         selector = buildSelector(IPv4.PROTOCOL_TCP,
                                  bgpdAddress,
                                  bgpdPeerAddress,
@@ -196,8 +186,7 @@ public class PeerConnectivityManager {
         intents.add(new PointToPointIntent(appId, selector, treatment,
                                bgpdConnectPoint, bgpdPeerConnectPoint));
 
-        // install intent for reversed BGP path from BGP peer to BGPd
-        // matching destination TCP port 179
+        // Path from BGP peer to BGP speaker matching destination TCP port 179
         selector = buildSelector(IPv4.PROTOCOL_TCP,
                                  bgpdPeerAddress,
                                  bgpdAddress,
@@ -207,8 +196,7 @@ public class PeerConnectivityManager {
         intents.add(new PointToPointIntent(appId, selector, treatment,
                                bgpdPeerConnectPoint, bgpdConnectPoint));
 
-        // install intent for reversed BGP path from BGP peer to BGPd
-        // matching source TCP port 179
+        // Path from BGP peer to BGP speaker matching source TCP port 179
         selector = buildSelector(IPv4.PROTOCOL_TCP,
                                  bgpdPeerAddress,
                                  bgpdAddress,
@@ -218,7 +206,7 @@ public class PeerConnectivityManager {
         intents.add(new PointToPointIntent(appId, selector, treatment,
                                bgpdPeerConnectPoint, bgpdConnectPoint));
 
-        // install intent for ICMP path from BGPd to BGP peer
+        // ICMP path from BGP speaker to BGP peer
         selector = buildSelector(IPv4.PROTOCOL_ICMP,
                                  bgpdAddress,
                                  bgpdPeerAddress,
@@ -228,7 +216,7 @@ public class PeerConnectivityManager {
         intents.add(new PointToPointIntent(appId, selector, treatment,
                                bgpdConnectPoint, bgpdPeerConnectPoint));
 
-        // install intent for reversed ICMP path from BGP peer to BGPd
+        // ICMP path from BGP peer to BGP speaker
         selector = buildSelector(IPv4.PROTOCOL_ICMP,
                                  bgpdPeerAddress,
                                  bgpdAddress,

@@ -180,6 +180,13 @@ public class SdnIp implements SdnIpService {
         intentSynchronizer.leaderChanged(isPrimary);
     }
 
+    /**
+     * Converts DPIDs of the form xx:xx:xx:xx:xx:xx:xx to OpenFlow provider
+     * device URIs.
+     *
+     * @param dpid the DPID string to convert
+     * @return the URI string for this device
+     */
     static String dpidToUri(String dpid) {
         return "of:" + dpid.replace(":", "");
     }

@@ -20,8 +20,7 @@ import java.util.Map;
 import org.onlab.packet.IpAddress;
 
 /**
- * Provides information about the layer 3 properties of the network.
- * This is based on IP addresses configured on ports in the network.
+ * Provides information about the BGP elements configured in the network.
  */
 public interface SdnIpConfigurationService {
 

@@ -15,12 +15,29 @@
  */
 package org.onosproject.sdnip;
 
-import com.google.common.collect.Sets;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.reset;
+import static org.easymock.EasyMock.verify;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.onlab.junit.TestUtils;
 import org.onlab.junit.TestUtils.TestUtilsException;
+import org.onlab.packet.Ethernet;
+import org.onlab.packet.IPv4;
+import org.onlab.packet.IpAddress;
+import org.onlab.packet.IpPrefix;
+import org.onlab.packet.MacAddress;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
@@ -41,23 +58,11 @@ import org.onosproject.sdnip.config.BgpSpeaker;
 import org.onosproject.sdnip.config.Interface;
 import org.onosproject.sdnip.config.InterfaceAddress;
 import org.onosproject.sdnip.config.SdnIpConfigurationService;
-import org.onlab.packet.Ethernet;
-import org.onlab.packet.IPv4;
-import org.onlab.packet.IpAddress;
-import org.onlab.packet.IpPrefix;
-import org.onlab.packet.MacAddress;
-
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
 
-import static org.easymock.EasyMock.*;
+import com.google.common.collect.Sets;
 
 /**
- * Unit tests for PeerConnectivityManager interface.
+ * Unit tests for PeerConnectivityManager.
  */
 public class PeerConnectivityManagerTest extends AbstractIntentTest {
 
@@ -647,10 +652,12 @@ public class PeerConnectivityManagerTest extends AbstractIntentTest {
         expect(configInfoService.getBgpPeers()).andReturn(
                 peers).anyTimes();
         expect(configInfoService.getBgpSpeakers()).andReturn(
-                null).anyTimes();
+                Collections.emptyMap()).anyTimes();
         replay(configInfoService);
 
         reset(intentService);
+        IntentOperations.Builder builder = IntentOperations.builder(APPID);
+        intentService.execute(builder.build());
         replay(intentService);
         peerConnectivityManager.start();
         verify(intentService);

@@ -35,6 +35,13 @@ import org.junit.Before;
 import org.junit.Test;
 import org.onlab.junit.TestUtils;
 import org.onlab.junit.TestUtils.TestUtilsException;
+import org.onlab.packet.Ethernet;
+import org.onlab.packet.Ip4Address;
+import org.onlab.packet.Ip4Prefix;
+import org.onlab.packet.IpAddress;
+import org.onlab.packet.IpPrefix;
+import org.onlab.packet.MacAddress;
+import org.onlab.packet.VlanId;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DefaultHost;
@@ -50,23 +57,16 @@ import org.onosproject.net.flow.TrafficTreatment;
 import org.onosproject.net.host.HostListener;
 import org.onosproject.net.host.HostService;
 import org.onosproject.net.host.InterfaceIpAddress;
+import org.onosproject.net.intent.AbstractIntentTest;
 import org.onosproject.net.intent.Intent;
 import org.onosproject.net.intent.IntentOperations;
 import org.onosproject.net.intent.IntentService;
 import org.onosproject.net.intent.MultiPointToSinglePointIntent;
-import org.onosproject.net.intent.AbstractIntentTest;
 import org.onosproject.net.provider.ProviderId;
 import org.onosproject.sdnip.IntentSynchronizer.IntentKey;
 import org.onosproject.sdnip.config.BgpPeer;
 import org.onosproject.sdnip.config.Interface;
 import org.onosproject.sdnip.config.SdnIpConfigurationService;
-import org.onlab.packet.Ethernet;
-import org.onlab.packet.IpAddress;
-import org.onlab.packet.Ip4Address;
-import org.onlab.packet.IpPrefix;
-import org.onlab.packet.Ip4Prefix;
-import org.onlab.packet.MacAddress;
-import org.onlab.packet.VlanId;
 
 import com.google.common.collect.Sets;
 
@@ -302,11 +302,6 @@ public class RouterTest extends AbstractIntentTest {
         Intent addedIntent =
             intentSynchronizer.getRouteIntents().iterator().next();
 
-        // Construct the existing route entry
-        RouteEntry routeEntry = new RouteEntry(
-                Ip4Prefix.valueOf("1.1.1.0/24"),
-                Ip4Address.valueOf("192.168.10.1"));
-
         // Start to construct a new route entry and new intent
         RouteEntry routeEntryUpdate = new RouteEntry(
                 Ip4Prefix.valueOf("1.1.1.0/24"),