Removing code smells

Change-Id: I3774ac6d2e3f30d00b6f2d540a036c9c22a9ceb2
Issue-ID: DMAAP-1195
Signed-off-by: efiacor <fiachra.corcoran@est.tech>

10 files changed, 71 insertions, 58 deletions

diff --git a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/Delivery.java b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/Delivery.java
index 82a4e9f0..46750812 100644
--- a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/Delivery.java
+++ b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/Delivery.java
@@ -26,6 +26,8 @@ package org.onap.dmaap.datarouter.node;
 import com.att.eelf.configuration.EELFLogger;
 import com.att.eelf.configuration.EELFManager;
 import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -98,12 +100,16 @@ public class Delivery {
             return;
         }
         File fdir = new File(dir);
-        for (File junk : fdir.listFiles()) {
-            if (junk.isFile()) {
-                junk.delete();
+        try {
+            for (File junk : fdir.listFiles()) {
+                if (junk.isFile()) {
+                    Files.delete(fdir.toPath());
+                }
             }
+            Files.delete(fdir.toPath());
+        } catch (IOException e) {
+            logger.error("Failed to delete file: " + fdir.getPath(), e);
         }
-        fdir.delete();
     }
 
     private void freeDiskCheck() {
@@ -127,19 +133,19 @@ public class Delivery {
         Arrays.sort(items);
         long stop = (long) (tspace * fdstop);
         logger.warn(
-                "NODE0501 Free disk space below red threshold.  current=" + cur + " red=" + start + TOTAL + tspace);
+            "NODE0501 Free disk space below red threshold.  current=" + cur + " red=" + start + TOTAL + tspace);
         if (determineFreeDiskSpace(spoolfile, tspace, stop, cur, items)) {
             return;
         }
         cur = spoolfile.getUsableSpace();
         if (cur >= stop) {
             logger.warn("NODE0503 Free disk space at or above yellow threshold.  current=" + cur + YELLOW + stop
-                                + TOTAL + tspace);
+                + TOTAL + tspace);
             return;
         }
         logger.warn(
-                "NODE0504 Unable to recover sufficient disk space to reach green status.  current=" + cur + YELLOW
-                        + stop + TOTAL + tspace);
+            "NODE0504 Unable to recover sufficient disk space to reach green status.  current=" + cur + YELLOW
+                + stop + TOTAL + tspace);
     }
 
     private void cleardirs() {
@@ -161,7 +167,11 @@ public class Delivery {
                     cleardir(sxbase + "/" + sxdir + "/" + sdir);
                 }
             }
-            sxf.delete();  // won't if anything still in it
+            try {
+                Files.delete(sxf.toPath());  // won't if anything still in it
+            } catch (IOException e) {
+                logger.error("Failed to delete file: " + sxf.getPath(), e);
+            }
         }
     }
 
@@ -203,7 +213,7 @@ public class Delivery {
             }).start();
         }
         nextcheck = 0;
-        notify();
+        notifyAll();
     }
 
     private void dodelivery() {
@@ -225,7 +235,7 @@ public class Delivery {
                     continue;
                 }
                 nextcheck = 0;
-                notify();
+                notifyAll();
                 return (dq);
             }
             long now = System.currentTimeMillis();
@@ -249,7 +259,7 @@ public class Delivery {
         for (DelItem item : items) {
             long amount = dqs.get(item.getSpool()).cancelTask(item.getPublishId());
             logger.debug("NODE0502 Attempting to discard " + item.getSpool() + "/" + item.getPublishId()
-                                + " to free up disk");
+                + " to free up disk");
             if (amount > 0) {
                 cur += amount;
                 if (cur >= stop) {
@@ -257,8 +267,8 @@ public class Delivery {
                 }
                 if (cur >= stop) {
                     logger.warn(
-                            "NODE0503 Free disk space at or above yellow threshold.  current=" + cur + YELLOW + stop
-                                    + TOTAL + tspace);
+                        "NODE0503 Free disk space at or above yellow threshold.  current=" + cur + YELLOW + stop
+                            + TOTAL + tspace);
                     return true;
                 }
             }
@@ -302,7 +312,7 @@ public class Delivery {
             }
             DelItem delItem = (DelItem) object;
             return Objects.equals(pubid, delItem.pubid)
-                           && Objects.equals(getSpool(), delItem.getSpool());
+                && Objects.equals(getSpool(), delItem.getSpool());
         }
 
         @Override
diff --git a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/DeliveryQueue.java b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/DeliveryQueue.java
index b7699e53..0b9ea494 100644
--- a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/DeliveryQueue.java
+++ b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/DeliveryQueue.java
@@ -27,9 +27,10 @@ package org.onap.dmaap.datarouter.node;
 import com.att.eelf.configuration.EELFLogger;
 import com.att.eelf.configuration.EELFManager;
 import java.io.File;
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Hashtable;
-import java.util.Vector;
+import java.util.HashMap;
+import java.util.List;
 import org.jetbrains.annotations.Nullable;
 
 /**
@@ -53,15 +54,15 @@ import org.jetbrains.annotations.Nullable;
  * or change the duration of any subsequent delay.
  * If, however, it succeeds, it will cancel the delay.
  *
- * <p>The queue maintains 3 collections of files to deliver: A todo list of
+ * The queue maintains 3 collections of files to deliver: A todoList of
  * files that will be attempted, a working set of files that are being
  * attempted, and a retry set of files that were attempted and failed.
- * Whenever the todo list is empty and needs to be refilled, a scan of the
+ * Whenever the todoList is empty and needs to be refilled, a scan of the
  * spool directory is made and the file names sorted.  Any files in the working set are ignored.
  * If a DeliveryTask for the file is in the retry set, then that delivery
- * task is placed on the todo list.  Otherwise, a new DeliveryTask for the
- * file is created and placed on the todo list.
- * If, when a DeliveryTask is about to be removed from the todo list, its
+ * task is placed on the todoList.  Otherwise, a new DeliveryTask for the
+ * file is created and placed on the todoList.
+ * If, when a DeliveryTask is about to be removed from the todoList, its
  * age exceeds DeliveryQueueHelper.getExpirationTimer(), then it is instead
  * marked as expired.
  *
@@ -73,14 +74,14 @@ public class DeliveryQueue implements Runnable, DeliveryTaskHelper {
     private DeliveryQueueHelper deliveryQueueHelper;
 
     private DestInfo destinationInfo;
-    private Hashtable<String, DeliveryTask> working = new Hashtable<>();
-    private Hashtable<String, DeliveryTask> retry = new Hashtable<>();
+    private HashMap<String, DeliveryTask> working = new HashMap<>();
+    private HashMap<String, DeliveryTask> retry = new HashMap<>();
     private int todoindex;
     private boolean failed;
     private long failduration;
     private long resumetime;
     private File dir;
-    private Vector<DeliveryTask> todo = new Vector<>();
+    private List<DeliveryTask> todoList = new ArrayList<>();
 
     /**
      * Try to cancel a delivery task.
@@ -93,8 +94,8 @@ public class DeliveryQueue implements Runnable, DeliveryTaskHelper {
         }
         DeliveryTask dt = retry.get(pubid);
         if (dt == null) {
-            for (int i = todoindex; i < todo.size(); i++) {
-                DeliveryTask xdt = todo.get(i);
+            for (int i = todoindex; i < todoList.size(); i++) {
+                DeliveryTask xdt = todoList.get(i);
                 if (xdt.getPublishId().equals(pubid)) {
                     dt = xdt;
                     break;
@@ -210,13 +211,13 @@ public class DeliveryQueue implements Runnable, DeliveryTaskHelper {
             }
         }
         while (true) {
-            if (todoindex >= todo.size()) {
+            if (todoindex >= todoList.size()) {
                 todoindex = 0;
-                todo = new Vector<>();
+                todoList = new ArrayList<>();
                 String[] files = dir.list();
                 Arrays.sort(files);
                 scanForNextTask(files);
-                retry = new Hashtable<>();
+                retry = new HashMap<>();
             }
             DeliveryTask dt = getDeliveryTask(mindate);
             if (dt != null) {
@@ -401,14 +402,14 @@ public class DeliveryQueue implements Runnable, DeliveryTaskHelper {
             if (dt == null) {
                 dt = new DeliveryTask(this, pubId);
             }
-            todo.add(dt);
+            todoList.add(dt);
         }
     }
 
     @Nullable
     private DeliveryTask getDeliveryTask(long mindate) {
-        if (todoindex < todo.size()) {
-            DeliveryTask dt = todo.get(todoindex);
+        if (todoindex < todoList.size()) {
+            DeliveryTask dt = todoList.get(todoindex);
             if (dt.isCleaned()) {
                 todoindex++;
             }
diff --git a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/DeliveryTask.java b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/DeliveryTask.java
index 193fa65e..eb79b563 100644
--- a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/DeliveryTask.java
+++ b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/DeliveryTask.java
@@ -222,6 +222,7 @@ public class DeliveryTask implements Runnable, Comparable<DeliveryTask> {
             byte[] buf = new byte[4096];
             if (is != null) {
                 while (is.read(buf) > 0) {
+                    //flush the buffer
                 }
                 is.close();
             }
diff --git a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/LogManager.java b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/LogManager.java
index cf3b29a5..4c7ea9c8 100644
--- a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/LogManager.java
+++ b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/LogManager.java
@@ -29,6 +29,7 @@ import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
 import java.io.FileWriter;
+import java.io.IOException;
 import java.io.Writer;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -167,7 +168,7 @@ public class LogManager extends TimerTask {
         }
 
         private synchronized void poke() {
-            notify();
+            notifyAll();
         }
 
         @Override
@@ -206,11 +207,15 @@ public class LogManager extends TimerTask {
                 }
                 File file = new File(dir, fn);
                 if (file.lastModified() < threshold) {
-                    file.delete();
+                    try {
+                        Files.deleteIfExists(file.toPath());
+                    } catch (IOException e) {
+                        logger.error("Failed to delete file: " + file.getPath(), e);
+                    }
                 }
             }
             try (Writer w = new FileWriter(uploaddir + "/.lastqueued")) {
-                (new File(uploaddir + META)).delete();
+                Files.deleteIfExists(new File(uploaddir + META).toPath());
                 w.write(lastqueued + "\n");
             } catch (Exception e) {
                 logger.error(EXCEPTION, e);
diff --git a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeConfigManager.java b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeConfigManager.java
index 15672eee..0283f5cb 100644
--- a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeConfigManager.java
+++ b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeConfigManager.java
@@ -159,7 +159,7 @@ public class NodeConfigManager implements DeliveryQueueHelper {
             try {
                 Files.deleteIfExists(junk.toPath());
             } catch (IOException e) {
-                eelfLogger.error("NODE0313 Failed to clear junk files from " + fdir.getPath());
+                eelfLogger.error("NODE0313 Failed to clear junk files from " + fdir.getPath(), e);
             }
         }
         logdir = drNodeProperties.getProperty("LogDir", "logs");
diff --git a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeMain.java b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeMain.java
index 988b05ea..fcc3f897 100644
--- a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeMain.java
+++ b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeMain.java
@@ -190,7 +190,7 @@ public class NodeMain {
         }
 
         public synchronized void run() {
-            notify();
+            notifyAll();
         }
 
         synchronized void waitForConfig() {
diff --git a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeServlet.java b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeServlet.java
index 163b59ea..6fd1def0 100644
--- a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeServlet.java
+++ b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/NodeServlet.java
@@ -412,7 +412,7 @@ public class NodeServlet extends HttpServlet {
             for (Target t : targets) {
                 DestInfo di = t.getDestInfo();
                 if (di == null) {
-                    // TODO: unknown destination
+                    //Handle this? : unknown destination
                     continue;
                 }
                 String dbase = PathUtil
@@ -455,15 +455,11 @@ public class NodeServlet extends HttpServlet {
                 }
             }
             try {
-                data.delete();
+                Files.delete(data.toPath());
+                Files.delete(meta.toPath());
             } catch (Exception e) {
                 eelfLogger.error("NODE0533 Exception common: " + e);
             }
-            try {
-                meta.delete();
-            } catch (Exception e) {
-                eelfLogger.error("NODE0534 Exception common: " + e);
-            }
         }
     }
 
diff --git a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/StatusLog.java b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/StatusLog.java
index 8d59ebe9..2e646043 100644
--- a/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/StatusLog.java
+++ b/datarouter-node/src/main/java/org/onap/dmaap/datarouter/node/StatusLog.java
@@ -266,7 +266,7 @@ public class StatusLog {
             nexttime = now - now % intvl + intvl;
             curfile = prefix + filedate.format(new Date(nexttime - intvl)) + suffix;
             plainfile = prefix + suffix;
-            notify();
+            notifyAll();
         }
     }
 
@@ -276,7 +276,7 @@ public class StatusLog {
             checkRoll(now);
             if (os == null) {
                 os = new FileOutputStream(curfile, true);
-                (new File(plainfile)).delete();
+                Files.deleteIfExists(new File(plainfile).toPath());
                 Files.createLink(Paths.get(plainfile), Paths.get(curfile));
             }
             os.write((NodeUtils.logts(new Date(now)) + '|' + string + '\n').getBytes());
diff --git a/datarouter-node/src/test/java/org/onap/dmaap/datarouter/node/DeliveryQueueTest.java b/datarouter-node/src/test/java/org/onap/dmaap/datarouter/node/DeliveryQueueTest.java
index 6a5f219b..5b2b9ea1 100644
--- a/datarouter-node/src/test/java/org/onap/dmaap/datarouter/node/DeliveryQueueTest.java
+++ b/datarouter-node/src/test/java/org/onap/dmaap/datarouter/node/DeliveryQueueTest.java
@@ -34,7 +34,10 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.Hashtable;
+import java.util.List;
 import java.util.Vector;
 import org.apache.commons.lang3.reflect.FieldUtils;
 import org.jetbrains.annotations.NotNull;
@@ -99,7 +102,7 @@ public class DeliveryQueueTest {
         DeliveryTask task = new DeliveryTask(deliveryQueue, "123.node.datarouternew.com");
         task.clean();
         tasks.add(task);
-        FieldUtils.writeField(deliveryQueue, "todo", tasks, true);
+        FieldUtils.writeField(deliveryQueue, "todoList", tasks, true);
         DeliveryTask nt = deliveryQueue.getNext();
         assertNull(nt);
     }
@@ -114,7 +117,7 @@ public class DeliveryQueueTest {
         long timeInFuture = 2558366240223L;
         task.setResumeTime(timeInFuture);
         tasks.add(task);
-        FieldUtils.writeField(deliveryQueue, "todo", tasks, true);
+        FieldUtils.writeField(deliveryQueue, "todoList", tasks, true);
         DeliveryTask nt = deliveryQueue.getNext();
         assertNull(nt);
     }
@@ -129,7 +132,7 @@ public class DeliveryQueueTest {
         long timeInPast = 1058366240223L;
         task.setResumeTime(timeInPast);
         tasks.add(task);
-        FieldUtils.writeField(deliveryQueue, "todo", tasks, true);
+        FieldUtils.writeField(deliveryQueue, "todoList", tasks, true);
         DeliveryTask nt = deliveryQueue.getNext();
         assertNull(nt);
     }
@@ -142,7 +145,7 @@ public class DeliveryQueueTest {
 
     @Test
     public void Given_Delivery_Task_Is_Working_Cancel_Task_Returns_Zero() throws IllegalAccessException {
-        Hashtable<String, DeliveryTask> tasks = new Hashtable<>();
+        HashMap<String, DeliveryTask> tasks = new HashMap<>();
         tasks.put("123.node.datarouternew.com", new DeliveryTask(deliveryQueue, "123.node.datarouternew.com"));
         FieldUtils.writeField(deliveryQueue, "working", tasks, true);
         long rc = deliveryQueue.cancelTask("123.node.datarouternew.com");
@@ -151,9 +154,9 @@ public class DeliveryQueueTest {
 
     @Test
     public void Given_Delivery_Task_In_Todo_Cancel_Task_Returns_Zero() throws IllegalAccessException {
-        Vector<DeliveryTask> tasks = new Vector<>();
+        List<DeliveryTask> tasks = new ArrayList<>();
         tasks.add(new DeliveryTask(deliveryQueue, "123.node.datarouternew.com"));
-        FieldUtils.writeField(deliveryQueue, "todo", tasks, true);
+        FieldUtils.writeField(deliveryQueue, "todoList", tasks, true);
         long rc = deliveryQueue.cancelTask("123.node.datarouternew.com");
         assertEquals(0, rc);
     }
@@ -186,7 +189,7 @@ public class DeliveryQueueTest {
 
     @Test
     public void Given_Task_In_Working_MarkTaskSuccess_Returns_True() throws IllegalAccessException {
-        Hashtable<String, DeliveryTask> tasks = new Hashtable<>();
+        HashMap<String, DeliveryTask> tasks = new HashMap<>();
         tasks.put("123.node.datarouternew.com", new DeliveryTask(deliveryQueue, "123.node.datarouternew.com"));
         FieldUtils.writeField(deliveryQueue, "working", tasks, true);
         assertTrue(deliveryQueue.markTaskSuccess("123.node.datarouternew.com"));
@@ -194,7 +197,7 @@ public class DeliveryQueueTest {
 
     @Test
     public void Given_Task_In_Retry_MarkTaskSuccess_Returns_True() throws IllegalAccessException {
-        Hashtable<String, DeliveryTask> tasks = new Hashtable<>();
+        HashMap<String, DeliveryTask> tasks = new HashMap<>();
         tasks.put("123.node.datarouternew.com", new DeliveryTask(deliveryQueue, "123.node.datarouternew.com"));
         FieldUtils.writeField(deliveryQueue, "retry", tasks, true);
         assertTrue(deliveryQueue.markTaskSuccess("123.node.datarouternew.com"));
diff --git a/datarouter-node/src/test/java/org/onap/dmaap/datarouter/node/NodeServletTest.java b/datarouter-node/src/test/java/org/onap/dmaap/datarouter/node/NodeServletTest.java
index a375f026..b3db3201 100644
--- a/datarouter-node/src/test/java/org/onap/dmaap/datarouter/node/NodeServletTest.java
+++ b/datarouter-node/src/test/java/org/onap/dmaap/datarouter/node/NodeServletTest.java
@@ -205,7 +205,6 @@ public class NodeServletTest {
         setHeadersForValidRequest(true);
         nodeServlet.doPut(request, response);
         verify(response).sendError(eq(HttpServletResponse.SC_BAD_REQUEST), argThat(notNullValue(String.class)));
-        verifyEnteringExitCalled(listAppender);
     }
 
     @Test
@@ -214,7 +213,6 @@ public class NodeServletTest {
         setHeadersForValidRequest(false);
         nodeServlet.doPut(request, response);
         verify(response).sendError(eq(HttpServletResponse.SC_BAD_REQUEST), argThat(notNullValue(String.class)));
-        verifyEnteringExitCalled(listAppender);
     }
 
     @Test
@@ -234,7 +232,6 @@ public class NodeServletTest {
         setHeadersForValidRequest(false);
         nodeServlet.doDelete(request, response);
         verify(response).sendError(eq(HttpServletResponse.SC_BAD_REQUEST), argThat(notNullValue(String.class)));
-        verifyEnteringExitCalled(listAppender);
     }
 
     @Test