Cosmetic code cleanup in registry (line lengths and comment formats)

Change-Id: Icbfff8924eeda3f2fedfdb7e41d484c00da60750
diff --git a/src/main/java/net/onrc/onos/core/registry/RegistryException.java b/src/main/java/net/onrc/onos/core/registry/RegistryException.java
index f05aef8..8137d19 100644
--- a/src/main/java/net/onrc/onos/core/registry/RegistryException.java
+++ b/src/main/java/net/onrc/onos/core/registry/RegistryException.java
@@ -4,17 +4,6 @@
 
     private static final long serialVersionUID = -8276300722010217913L;
 
-    /*
-    public RegistryException() {
-        // TODO Auto-generated constructor stub
-    }
-
-    public RegistryException(Throwable cause) {
-        super(cause);
-        // TODO Auto-generated constructor stub
-    }
-    */
-
     public RegistryException(String message) {
         super(message);
     }
diff --git a/src/main/java/net/onrc/onos/core/registry/StandaloneRegistry.java b/src/main/java/net/onrc/onos/core/registry/StandaloneRegistry.java
index de07adf..0b552f1 100644
--- a/src/main/java/net/onrc/onos/core/registry/StandaloneRegistry.java
+++ b/src/main/java/net/onrc/onos/core/registry/StandaloneRegistry.java
@@ -2,6 +2,7 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -34,6 +35,9 @@
     private String registeredControllerId;
     private Map<String, ControlChangeCallback> switchCallbacks;
 
+    private long blockTop;
+    private static final long BLOCK_SIZE = 0x1000000L;
+
     //
     // Unique ID generation state
     //
@@ -95,14 +99,16 @@
 
     @Override
     public Collection<String> getAllControllers() throws RegistryException {
-        List<String> l = new ArrayList<String>();
-        l.add(registeredControllerId);
-        return l;
+        //List<String> l = new ArrayList<String>();
+        //l.add(registeredControllerId);
+        //return l;
+        return Collections.singletonList(registeredControllerId);
     }
 
     @Override
     public String getControllerForSwitch(long dpid) throws RegistryException {
-        return (switchCallbacks.get(HexString.toHexString(dpid)) == null) ? null : registeredControllerId;
+        return (switchCallbacks.get(HexString.toHexString(dpid)) == null)
+                ? null : registeredControllerId;
     }
 
     @Override
@@ -112,7 +118,8 @@
 
         for (String strSwitch : switchCallbacks.keySet()) {
             log.debug("Switch _{}", strSwitch);
-            List<ControllerRegistryEntry> list = new ArrayList<ControllerRegistryEntry>();
+            List<ControllerRegistryEntry> list =
+                    new ArrayList<ControllerRegistryEntry>();
             list.add(new ControllerRegistryEntry(registeredControllerId, 0));
 
             switches.put(strSwitch, list);
@@ -127,12 +134,10 @@
         throw new NotImplementedException("Not yet implemented");
     }
 
-    private long blockTop;
-    private static final long BLOCK_SIZE = 0x1000000L;
-
     /**
      * Returns a block of IDs which are unique and unused.
-     * Range of IDs is fixed size and is assigned incrementally as this method called.
+     * Range of IDs is fixed size and is assigned incrementally as this method
+     * called.
      *
      * @return an IdBlock containing a set of unique IDs
      */
diff --git a/src/main/java/net/onrc/onos/core/registry/ZookeeperRegistry.java b/src/main/java/net/onrc/onos/core/registry/ZookeeperRegistry.java
index 2f1c4c7..35ce962 100644
--- a/src/main/java/net/onrc/onos/core/registry/ZookeeperRegistry.java
+++ b/src/main/java/net/onrc/onos/core/registry/ZookeeperRegistry.java
@@ -55,7 +55,8 @@
  *
  * @author jono
  */
-public class ZookeeperRegistry implements IFloodlightModule, IControllerRegistryService {
+public class ZookeeperRegistry implements IFloodlightModule,
+                                          IControllerRegistryService {
 
     private static final Logger log = LoggerFactory.getLogger(ZookeeperRegistry.class);
 
@@ -63,14 +64,15 @@
 
     private IRestApiService restApi;
 
-    //This is the default, it's overwritten by the connectionString configuration parameter
+    // This is the default. It is overwritten by the connectionString
+    // configuration parameter
     private String connectionString = "localhost:2181";
 
     private static final String NAMESPACE = "onos";
     private static final String SWITCH_LATCHES_PATH = "/switches";
     private static final String CLUSTER_LEADER_PATH = "/cluster/leader";
 
-    private static final String SERVICES_PATH = "/"; //i.e. the root of our namespace
+    private static final String SERVICES_PATH = "/"; // i.e. the root of our namespace
     private static final String CONTROLLER_SERVICE_NAME = "controllers";
 
     private CuratorFramework curatorFrameworkClient;
@@ -135,14 +137,13 @@
         }
     }
 
-    /*
-     * Dispatcher thread for leadership change events coming from Curator.
-     */
+    // Dispatcher thread for leadership change events coming from Curator
     private void dispatchEvents() {
         while (!Thread.currentThread().isInterrupted()) {
             try {
                 SwitchLeaderEvent event = switchLeadershipEvents.take();
-                SwitchLeadershipData swData = switches.get(HexString.toHexString(event.getDpid()));
+                SwitchLeadershipData swData =
+                        switches.get(HexString.toHexString(event.getDpid()));
                 if (swData == null) {
                     log.debug("Leadership data {} not found", event.getDpid());
                     continue;
@@ -169,14 +170,16 @@
         public void isLeader() {
             log.debug("Became leader for {}", dpid);
 
-            switchLeadershipEvents.add(new SwitchLeaderEvent(HexString.toLong(dpid), true));
+            switchLeadershipEvents.add(
+                    new SwitchLeaderEvent(HexString.toLong(dpid), true));
         }
 
         @Override
         public void notLeader() {
             log.debug("Lost leadership for {}", dpid);
 
-            switchLeadershipEvents.add(new SwitchLeaderEvent(HexString.toLong(dpid), false));
+            switchLeadershipEvents.add(
+                    new SwitchLeaderEvent(HexString.toLong(dpid), false));
         }
     }
 
@@ -194,7 +197,8 @@
             switch (event.getType()) {
                 case CHILD_ADDED:
                 case CHILD_UPDATED:
-                    //Check we have a PathChildrenCache for this child, add one if not
+                    // Check we have a PathChildrenCache for this child
+                    // and add one if not
                     synchronized (switchPathCaches) {
                         if (switchPathCaches.get(strSwitch) == null) {
                             PathChildrenCache pc = new PathChildrenCache(client,
@@ -205,7 +209,7 @@
                     }
                     break;
                 case CHILD_REMOVED:
-                    //Remove our PathChildrenCache for this child
+                    // Remove our PathChildrenCache for this child
                     PathChildrenCache pc = null;
                     synchronized (switchPathCaches) {
                         pc = switchPathCaches.remove(strSwitch);
@@ -215,8 +219,9 @@
                     }
                     break;
                 default:
-                    //All other switchLeadershipEvents are connection status switchLeadershipEvents. We don't need to
-                    //do anything as the path cache handles these on its own.
+                    // All other switchLeadershipEvents are connection status
+                    // switchLeadershipEvents. We don't need to do anything as
+                    // the path cache handles these on its own.
                     break;
             }
 
@@ -241,11 +246,13 @@
     }
 
     @Override
-    public void requestControl(long dpid, ControlChangeCallback cb) throws RegistryException {
+    public void requestControl(long dpid, ControlChangeCallback cb)
+            throws RegistryException {
         log.info("Requesting control for {}", HexString.toHexString(dpid));
 
         if (controllerId == null) {
-            throw new IllegalStateException("Must register a controller before calling requestControl");
+            throw new IllegalStateException("Must register a controller before"
+                    + " calling requestControl");
         }
 
         String dpidStr = HexString.toHexString(dpid);
@@ -257,7 +264,8 @@
 
         String latchPath = SWITCH_LATCHES_PATH + "/" + dpidStr;
 
-        LeaderLatch latch = new LeaderLatch(curatorFrameworkClient, latchPath, controllerId);
+        LeaderLatch latch =
+                new LeaderLatch(curatorFrameworkClient, latchPath, controllerId);
         SwitchLeaderListener listener = new SwitchLeaderListener(dpidStr);
         latch.addListener(listener);
 
@@ -266,24 +274,25 @@
         SwitchLeadershipData oldData = switches.putIfAbsent(dpidStr, swData);
 
         if (oldData != null) {
-            //There was already data for that key in the map
-            //i.e. someone else got here first so we can't succeed
+            // There was already data for that key in the map
+            // i.e. someone else got here first so we can't succeed
             log.debug("Already requested control for {}", dpidStr);
             throw new RegistryException("Already requested control for " + dpidStr);
         }
 
-        //Now that we know we were able to add our latch to the collection,
-        //we can start the leader election in Zookeeper. However I don't know
-        //how to handle if the start fails - the latch is already in our
-        //switches list.
-        //TODO seems like there's a Curator bug when latch.start is called when
-        //there's no Zookeeper connection which causes two znodes to be put in
-        //Zookeeper at the latch path when we reconnect to Zookeeper.
+        // Now that we know we were able to add our latch to the collection,
+        // we can start the leader election in Zookeeper. However I don't know
+        // how to handle if the start fails - the latch is already in our
+        // switches list.
+        // TODO seems like there's a Curator bug when latch.start is called when
+        // there's no Zookeeper connection which causes two znodes to be put in
+        // Zookeeper at the latch path when we reconnect to Zookeeper.
         try {
             latch.start();
         } catch (Exception e) {
             log.warn("Error starting leader latch: {}", e.getMessage());
-            throw new RegistryException("Error starting leader latch for " + dpidStr, e);
+            throw new RegistryException("Error starting leader latch for "
+                    + dpidStr, e);
         }
 
     }
@@ -308,8 +317,8 @@
         try {
             latch.close();
         } catch (IOException e) {
-            //I think it's OK not to do anything here. Either the node got
-            //deleted correctly, or the connection went down and the node got deleted.
+            // I think it's OK not to do anything here. Either the node got
+            // deleted correctly, or the connection went down and the node got deleted.
             log.debug("releaseControl: caught IOException {}", dpidStr);
         }
     }
@@ -388,16 +397,19 @@
         }
 
         try {
-            //We've seen issues with these caches get stuck out of date, so we'll have to
-            //force them to refresh before each read. This slows down the method as it
-            //blocks on a Zookeeper query, however at the moment only the cleanup thread
-            //uses this and that isn't particularly time-sensitive.
+            // We've seen issues with these caches get stuck out of date, so
+            // we'll have to force them to refresh before each read. This slows
+            // down the method as it blocks on a Zookeeper query, however at
+            // the moment only the cleanup thread uses this and that isn't
+            // particularly time-sensitive.
+            // TODO verify if it is still the case that caches can be out of date
             switchCache.rebuild();
         } catch (Exception e) {
             log.error("Exception rebuilding the switch cache:", e);
         }
 
-        List<ChildData> sortedData = new ArrayList<ChildData>(switchCache.getCurrentData());
+        List<ChildData> sortedData =
+                new ArrayList<ChildData>(switchCache.getCurrentData());
 
         Collections.sort(
                 sortedData,
@@ -423,14 +435,14 @@
 
     @Override
     public Collection<Long> getSwitchesControlledByController(String controller) {
-        //TODO remove this if not needed
+        // TODO remove this if not needed
         throw new NotImplementedException("Not yet implemented");
     }
 
 
-    //TODO what should happen when there's no ZK connection? Currently we just return
-    //the cache but this may lead to false impressions - i.e. we don't actually know
-    //what's in ZK so we shouldn't say we do
+    // TODO what should happen when there's no ZK connection? Currently we just
+    // return the cache but this may lead to false impressions - i.e. we don't
+    // actually know what's in ZK so we shouldn't say we do
     @Override
     public Map<String, List<ControllerRegistryEntry>> getAllSwitches() {
         Map<String, List<ControllerRegistryEntry>> data =
@@ -441,8 +453,7 @@
                     new ArrayList<ControllerRegistryEntry>();
 
             if (entry.getValue().getCurrentData().size() < 1) {
-                //TODO prevent even having the PathChildrenCache in this case
-                //log.info("Switch entry with no leader elections: {}", entry.getKey());
+                // TODO prevent even having the PathChildrenCache in this case
                 continue;
             }
 
@@ -453,7 +464,8 @@
                 String[] splitted = d.getPath().split("-");
                 int sequenceNumber = Integer.parseInt(splitted[splitted.length - 1]);
 
-                contendingControllers.add(new ControllerRegistryEntry(childsControllerId, sequenceNumber));
+                contendingControllers.add(new ControllerRegistryEntry(
+                        childsControllerId, sequenceNumber));
             }
 
             Collections.sort(contendingControllers);
@@ -544,13 +556,12 @@
         return l;
     }
 
-    //TODO currently blocks startup when it can't get a Zookeeper connection.
-    //Do we support starting up with no Zookeeper connection?
+    // TODO currently blocks startup when it can't get a Zookeeper connection.
+    // Do we support starting up with no Zookeeper connection?
     @Override
-    public void init(FloodlightModuleContext context) throws FloodlightModuleException {
-        log.info("Initialising the Zookeeper Registry - Zookeeper connection required");
-
-        //Read the Zookeeper connection string from the config
+    public void init(FloodlightModuleContext context)
+            throws FloodlightModuleException {
+        // Read the Zookeeper connection string from the config
         Map<String, String> configParams = context.getConfigParams(this);
         String connectionStringParam = configParams.get("connectionString");
         if (connectionStringParam != null) {
@@ -561,11 +572,11 @@
         restApi = context.getServiceImpl(IRestApiService.class);
 
         switches = new ConcurrentHashMap<String, SwitchLeadershipData>();
-        //switchPathCaches = new HashMap<String, PathChildrenCache>();
         switchPathCaches = new ConcurrentHashMap<String, PathChildrenCache>();
 
         RetryPolicy retryPolicy = new ExponentialBackoffRetry(1000, 3);
-        curatorFrameworkClient = CuratorFrameworkFactory.newClient(this.connectionString,
+        curatorFrameworkClient =
+                CuratorFrameworkFactory.newClient(this.connectionString,
                 SESSION_TIMEOUT, CONNECTION_TIMEOUT, retryPolicy);
 
         curatorFrameworkClient.start();
@@ -576,24 +587,25 @@
                 ID_COUNTER_PATH,
                 new RetryOneTime(100));
 
-        rootSwitchCache = new PathChildrenCache(curatorFrameworkClient, SWITCH_LATCHES_PATH, true);
+        rootSwitchCache = new PathChildrenCache(
+                curatorFrameworkClient, SWITCH_LATCHES_PATH, true);
         rootSwitchCache.getListenable().addListener(switchPathCacheListener);
 
-        //Build the service discovery object
+        // Build the service discovery object
         serviceDiscovery = ServiceDiscoveryBuilder.builder(ControllerService.class)
                 .client(curatorFrameworkClient).basePath(SERVICES_PATH).build();
 
-        //We read the list of services very frequently (GUI periodically queries them)
-        //so we'll cache them to cut down on Zookeeper queries.
+        // We read the list of services very frequently (GUI periodically
+        // queries them) so we'll cache them to cut down on Zookeeper queries.
         serviceCache = serviceDiscovery.serviceCacheBuilder()
                 .name(CONTROLLER_SERVICE_NAME).build();
 
-
         try {
             serviceDiscovery.start();
             serviceCache.start();
 
-            //Don't prime the cache, we want a notification for each child node in the path
+            // Don't prime the cache, we want a notification for each child
+            // node in the path
             rootSwitchCache.start(StartMode.NORMAL);
         } catch (Exception e) {
             throw new FloodlightModuleException(
@@ -629,7 +641,7 @@
         try {
             clusterLeaderLatch.start();
         } catch (Exception e) {
-            log.error("Error on startup starting the cluster leader election: {}", e.getMessage());
+            log.error("Error starting the cluster leader election: ", e);
         }
 
         // Keep trying until there is a cluster leader
@@ -641,7 +653,7 @@
                 }
                 Thread.sleep(CLUSTER_LEADER_ELECTION_RETRY_MS);
             } catch (Exception e) {
-                log.error("Error on startup waiting for cluster leader election: {}", e.getMessage());
+                log.error("Error waiting for cluster leader election:", e);
             }
         } while (true);