Fix findbugs violations in utils

- Naked notify
- An apparent infinite recursive loop
- Field isn't final but should be
- Boxing/unboxing to parse a primitive
- Method concatenates strings using + in a loop
- Method ignores exceptional return value
- Reliance on default encoding
- Should be a static inner clas
- Could be refactored into a static inner class
- Result of integer multiplication cast to long

Change-Id: Ie10a638f81490f9ddc1c27f651c4471168c50bd6
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>

diff --git a/utils/mdsal-utils/src/main/java/org/opendaylight/ovsdb/utils/mdsal/utils/NotifyingDataChangeListener.java b/utils/mdsal-utils/src/main/java/org/opendaylight/ovsdb/utils/mdsal/utils/NotifyingDataChangeListener.java
index 683e75cf4737da69260336c53ac2fd12335d568e..853ffee9f1b1f0201b8c1f0335f34abc3d3740e6 100644 (file)
--- a/utils/mdsal-utils/src/main/java/org/opendaylight/ovsdb/utils/mdsal/utils/NotifyingDataChangeListener.java
+++ b/utils/mdsal-utils/src/main/java/org/opendaylight/ovsdb/utils/mdsal/utils/NotifyingDataChangeListener.java
@@ -7,6 +7,7 @@
  */
 package org.opendaylight.ovsdb.utils.mdsal.utils;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
@@ -103,6 +104,7 @@ public class NotifyingDataChangeListener implements AutoCloseable, DataTreeChang
     }
 
     @Override
+    @SuppressFBWarnings("NN_NAKED_NOTIFY")
     public void onDataTreeChanged(Collection<DataTreeModification<DataObject>> changes) {
         if (!listen) {
             return;

diff --git a/utils/ovsdb-it-utils/src/main/java/org/opendaylight/ovsdb/utils/ovsdb/it/utils/DockerOvs.java b/utils/ovsdb-it-utils/src/main/java/org/opendaylight/ovsdb/utils/ovsdb/it/utils/DockerOvs.java
index 72b56e2ea3eff525babdf8a815cde6b7bd2b74f8..d524e947e71d524de9b9c3b4d96ba2fe431cd8d6 100644 (file)
--- a/utils/ovsdb-it-utils/src/main/java/org/opendaylight/ovsdb/utils/ovsdb/it/utils/DockerOvs.java
+++ b/utils/ovsdb-it-utils/src/main/java/org/opendaylight/ovsdb/utils/ovsdb/it/utils/DockerOvs.java
@@ -13,12 +13,14 @@ import static org.ops4j.pax.exam.CoreOptions.propagateSystemProperties;
 import com.esotericsoftware.yamlbeans.YamlException;
 import com.esotericsoftware.yamlbeans.YamlReader;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.FileNotFoundException;
-import java.io.FileReader;
-import java.io.FileWriter;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.io.OutputStreamWriter;
 import java.io.Reader;
+import java.io.Writer;
 import java.net.InetSocketAddress;
 import java.net.URL;
 import java.nio.ByteBuffer;
@@ -26,6 +28,7 @@ import java.nio.channels.ClosedByInterruptException;
 import java.nio.channels.SocketChannel;
 import java.nio.charset.Charset;
 import java.nio.charset.CharsetDecoder;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -139,9 +142,9 @@ public class DockerOvs implements AutoCloseable {
     private boolean runDocker;
     private boolean createOdlNetwork;
 
-    class DockerComposeServiceInfo {
-        public String name;
-        public String port;
+    private static class DockerComposeServiceInfo {
+        String name;
+        String port;
     }
 
     private final Map<String, DockerComposeServiceInfo> dockerComposeServices = new HashMap<>();
@@ -201,7 +204,7 @@ public class DockerOvs implements AutoCloseable {
         ProcUtils.runProcess(60000, upCmd);
         isRunning = true;
         int waitSeconds = Integer.parseInt(envDockerWaitForPing);
-        waitForOvsdbServers(waitSeconds * 1000);
+        waitForOvsdbServers(waitSeconds * 1000L);
     }
 
     private void setupEnvForDockerCompose(String venvWs) {
@@ -419,7 +422,8 @@ public class DockerOvs implements AutoCloseable {
         YamlReader yamlReader = null;
         Map root = null;
         try {
-            yamlReader = new YamlReader(new FileReader(tmpDockerComposeFile));
+            yamlReader = new YamlReader(new InputStreamReader(new FileInputStream(tmpDockerComposeFile),
+                    StandardCharsets.UTF_8));
             root = (Map) yamlReader.read();
         } catch (FileNotFoundException e) {
             LOG.warn("DockerOvs.parseDockerComposeYaml error reading yaml file", e);
@@ -476,7 +480,9 @@ public class DockerOvs implements AutoCloseable {
             isRunning = false;
         }
 
-        tmpDockerComposeFile.delete();
+        if (!tmpDockerComposeFile.delete()) {
+            LOG.warn("Failed to delete {}", tmpDockerComposeFile);
+        }
     }
 
     /**
@@ -485,8 +491,7 @@ public class DockerOvs implements AutoCloseable {
      * checked to make sure the Open_Vswitch DB is present. Note that this thread will
      * run until it succeeds unless its interrupt() method is called.
      */
-    private class OvsdbPing extends Thread {
-
+    private static class OvsdbPing extends Thread {
         private final String host;
         private final int port;
         private final AtomicInteger result;
@@ -498,12 +503,12 @@ public class DockerOvs implements AutoCloseable {
          * @param ovsNumber which OVS is this?
          * @param result an AtomicInteger that is incremented upon a successful "ping"
          */
-        OvsdbPing(int ovsNumber, AtomicInteger result) {
-            this.host = getOvsdbAddress(ovsNumber);
-            this.port = Integer.parseInt(getOvsdbPort(ovsNumber));
+        OvsdbPing(AtomicInteger result, String host, int port) {
+            this.host = host;
+            this.port = port;
             this.result = result;
             listDbsRequest = ByteBuffer.wrap(
-                    ("{\"method\": \"list_dbs\", \"params\": [], \"id\": " + port + "}").getBytes());
+                ("{\"method\": \"list_dbs\", \"params\": [], \"id\": " + port + "}").getBytes(StandardCharsets.UTF_8));
             listDbsRequest.mark();
         }
 
@@ -565,11 +570,12 @@ public class DockerOvs implements AutoCloseable {
 
         OvsdbPing[] pingers = new OvsdbPing[numOvs];
         if (numOvs == 1) {
-            pingers[0] = new OvsdbPing(0, numRunningOvs);
+            pingers[0] = new OvsdbPing(numRunningOvs, getOvsdbAddress(0), Integer.parseInt(getOvsdbPort(0)));
             pingers[0].start();
         } else {
             for (int i = 0; i < numOvs; i++) {
-                pingers[i] = new OvsdbPing(i + 1, numRunningOvs);
+                pingers[i] = new OvsdbPing(numRunningOvs, getOvsdbAddress(i + 1),
+                        Integer.parseInt(getOvsdbPort(i + 1)));
                 pingers[i].start();
             }
         }
@@ -605,8 +611,8 @@ public class DockerOvs implements AutoCloseable {
         try {
             tmpFile = File.createTempFile("ovsdb-it-tmp-", null);
 
-            try (Reader in = new InputStreamReader(url.openStream());
-                                FileWriter out = new FileWriter(tmpFile)) {
+            try (Reader in = new InputStreamReader(url.openStream(), StandardCharsets.UTF_8);
+                    Writer out = new OutputStreamWriter(new FileOutputStream(tmpFile), StandardCharsets.UTF_8)) {
                 char[] buf = new char[1024];
                 int read;
                 while (-1 != (read = in.read(buf))) {

diff --git a/utils/ovsdb-it-utils/src/main/java/org/opendaylight/ovsdb/utils/ovsdb/it/utils/ProcUtils.java b/utils/ovsdb-it-utils/src/main/java/org/opendaylight/ovsdb/utils/ovsdb/it/utils/ProcUtils.java
index 1fa708f82e5513210dd080e2bdbca0f1ae1f18f1..9c2d57f6caf6134eed0d78a07809a4bb50f9a52d 100644 (file)
--- a/utils/ovsdb-it-utils/src/main/java/org/opendaylight/ovsdb/utils/ovsdb/it/utils/ProcUtils.java
+++ b/utils/ovsdb-it-utils/src/main/java/org/opendaylight/ovsdb/utils/ovsdb/it/utils/ProcUtils.java
@@ -11,6 +11,7 @@ package org.opendaylight.ovsdb.utils.ovsdb.it.utils;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
 import org.junit.Assert;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -89,8 +90,10 @@ public final class ProcUtils {
         int exitValue = -1;
 
         // Use a try block to guarantee stdout and stderr are closed
-        try (BufferedReader stdout = new BufferedReader(new InputStreamReader(proc.getInputStream()));
-             BufferedReader stderr = new BufferedReader(new InputStreamReader(proc.getErrorStream()))) {
+        try (BufferedReader stdout = new BufferedReader(new InputStreamReader(proc.getInputStream(),
+                StandardCharsets.UTF_8));
+             BufferedReader stderr = new BufferedReader(new InputStreamReader(proc.getErrorStream(),
+                StandardCharsets.UTF_8))) {
 
             exitValue = waitForExitValue(waitFor, proc);
 

diff --git a/utils/southbound-utils/src/main/java/org/opendaylight/ovsdb/utils/southbound/utils/SouthboundUtils.java b/utils/southbound-utils/src/main/java/org/opendaylight/ovsdb/utils/southbound/utils/SouthboundUtils.java
index 4d2c3b7edd02cadbf4d022856a8d5d002fe53208..b30c62623ee8ef8e83d7535bb37e22f1a2de5233 100644 (file)
--- a/utils/southbound-utils/src/main/java/org/opendaylight/ovsdb/utils/southbound/utils/SouthboundUtils.java
+++ b/utils/southbound-utils/src/main/java/org/opendaylight/ovsdb/utils/southbound/utils/SouthboundUtils.java
@@ -25,6 +25,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.ovsdb.utils.config.ConfigProperties;
 import org.opendaylight.ovsdb.utils.mdsal.utils.MdsalUtils;
@@ -114,7 +115,7 @@ public class SouthboundUtils {
     private final MdsalUtils mdsalUtils;
     public static final String OPENFLOW_CONNECTION_PROTOCOL = "tcp";
     public static final String OPENFLOW_SECURE_PROTOCOL = "ssl";
-    public static short OPENFLOW_PORT = 6653;
+    public static final short OPENFLOW_PORT = 6653;
     public static final String OVSDB_URI_PREFIX = "ovsdb";
     public static final String BRIDGE_URI_PREFIX = "bridge";
     private static final String DISABLE_IN_BAND = "disable-in-band";
@@ -505,18 +506,6 @@ public class SouthboundUtils {
         return protocolList;
     }
 
-    public boolean addBridge(final ConnectionInfo connectionInfo, InstanceIdentifier<Node> bridgeIid,
-                             final String bridgeName, NodeId bridgeNodeId, final boolean setProtocolEntries,
-                             final Class<? extends OvsdbFailModeBase> failMode, final boolean setManagedBy,
-                             final Class<? extends DatapathTypeBase> dpType,
-                             final List<BridgeExternalIds> externalIds,
-                             final List<ControllerEntry> controllerEntries,
-                             final List<BridgeOtherConfigs> otherConfigs,
-                             final String dpid) throws InterruptedException {
-        return addBridge(connectionInfo, bridgeIid, bridgeName, bridgeNodeId, setProtocolEntries, failMode,
-                setManagedBy, dpType, externalIds, controllerEntries, otherConfigs, dpid);
-    }
-
     /*
      * base method for adding test bridges.  Other helper methods used to create bridges should utilize this method.
      *
@@ -936,11 +925,8 @@ public class SouthboundUtils {
         if (controllersStr.isEmpty()) {
             LOG.warn("Failed to determine OpenFlow controller ip address");
         } else if (LOG.isDebugEnabled()) {
-            controllerIpStr = "";
-            for (String currControllerIpStr : controllersStr) {
-                controllerIpStr += " " + currControllerIpStr;
-            }
-            LOG.debug("Found {} OpenFlow Controller(s) :{}", controllersStr.size(), controllerIpStr);
+            LOG.debug("Found {} OpenFlow Controller(s) :{}", controllersStr.size(),
+                    controllersStr.stream().collect(Collectors.joining(" ")));
         }
 
         return controllersStr;
@@ -1245,37 +1231,36 @@ public class SouthboundUtils {
             return false;
         }
 
-        if (dbVersion != null && !dbVersion.isEmpty() && minVersion != null
-                && !minVersion.isEmpty()) {
-            if (Integer.valueOf(dbVersionMatcher.group(1)).equals(Integer.valueOf(minVersionMatcher.group(1)))
-                   && Integer.valueOf(dbVersionMatcher.group(2)).equals(Integer.valueOf(minVersionMatcher.group(2)))
-                   && Integer.valueOf(dbVersionMatcher.group(3)).equals(Integer.valueOf(minVersionMatcher.group(3)))) {
+        if (dbVersion != null && !dbVersion.isEmpty() && minVersion != null && !minVersion.isEmpty()) {
+            final int dbVersionMatch1 = Integer.parseInt(dbVersionMatcher.group(1));
+            final int dbVersionMatch2 = Integer.parseInt(dbVersionMatcher.group(2));
+            final int dbVersionMatch3 = Integer.parseInt(dbVersionMatcher.group(3));
+            final int minVersionMatch1 = Integer.parseInt(minVersionMatcher.group(1));
+            final int minVersionMatch2 = Integer.parseInt(minVersionMatcher.group(2));
+            final int minVersionMatch3 = Integer.parseInt(minVersionMatcher.group(3));
+            if (dbVersionMatch1 == minVersionMatch1 && dbVersionMatch2 == minVersionMatch2
+                    && dbVersionMatch3 == minVersionMatch3) {
                 return true;
             }
 
-            if (Integer.valueOf(dbVersionMatcher.group(1)).intValue()
-                    > Integer.valueOf(minVersionMatcher.group(1)).intValue()) {
+            if (dbVersionMatch1 > minVersionMatch1) {
                 return true;
             }
 
-            if (Integer.valueOf(dbVersionMatcher.group(1)).intValue()
-                    < Integer.valueOf(minVersionMatcher.group(1)).intValue()) {
+            if (dbVersionMatch1 < minVersionMatch1) {
                 return false;
             }
 
             // major version is equal
-            if (Integer.valueOf(dbVersionMatcher.group(2)).intValue()
-                    > Integer.valueOf(minVersionMatcher.group(2)).intValue()) {
+            if (dbVersionMatch2 > minVersionMatch2) {
                 return true;
             }
 
-            if (Integer.valueOf(dbVersionMatcher.group(2)).intValue()
-                    < Integer.valueOf(minVersionMatcher.group(2)).intValue()) {
+            if (dbVersionMatch2 < minVersionMatch2) {
                 return false;
             }
 
-            if (Integer.valueOf(dbVersionMatcher.group(3)).intValue()
-                   > Integer.valueOf(minVersionMatcher.group(3)).intValue()) {
+            if (dbVersionMatch3 > minVersionMatch3) {
                 return true;
             }
         }