From 6c78b3e6a0a67c73f931337356a172cc69cee0e8 Mon Sep 17 00:00:00 2001 From: efiacor Date: Thu, 8 Aug 2019 10:55:33 +0000 Subject: Removing code smells Change-Id: I3774ac6d2e3f30d00b6f2d540a036c9c22a9ceb2 Issue-ID: DMAAP-1195 Signed-off-by: efiacor --- .../org/onap/dmaap/datarouter/node/Delivery.java | 40 ++++++++++++++-------- .../onap/dmaap/datarouter/node/DeliveryQueue.java | 37 ++++++++++---------- .../onap/dmaap/datarouter/node/DeliveryTask.java | 1 + .../org/onap/dmaap/datarouter/node/LogManager.java | 11 ++++-- .../dmaap/datarouter/node/NodeConfigManager.java | 2 +- .../org/onap/dmaap/datarouter/node/NodeMain.java | 2 +- .../onap/dmaap/datarouter/node/NodeServlet.java | 10 ++---- .../org/onap/dmaap/datarouter/node/StatusLog.java | 4 +-- .../dmaap/datarouter/node/DeliveryQueueTest.java | 19 +++++----- .../dmaap/datarouter/node/NodeServletTest.java | 3 -- 10 files changed, 71 insertions(+), 58 deletions(-) (limited to 'datarouter-node/src') 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. * - *

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 working = new Hashtable<>(); - private Hashtable retry = new Hashtable<>(); + private HashMap working = new HashMap<>(); + private HashMap retry = new HashMap<>(); private int todoindex; private boolean failed; private long failduration; private long resumetime; private File dir; - private Vector todo = new Vector<>(); + private List 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 { 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 tasks = new Hashtable<>(); + HashMap 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 tasks = new Vector<>(); + List 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 tasks = new Hashtable<>(); + HashMap 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 tasks = new Hashtable<>(); + HashMap 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 -- cgit 1.2.3-korg