From ce122b6f177512e5a9c839dba971b82f6a3e40c7 Mon Sep 17 00:00:00 2001
From: Taka Cho <takamune.cho@att.com>
Date: Tue, 30 Jun 2020 16:54:50 -0400
Subject: sonar fix for feature server pool

- reduce cognitive complexity
- return a boolean value
- Replace this lambda with a method reference
- use lombok annotation @EqualsAndHashcode
- removed unnecessary exception

Issue-ID: POLICY-2616
Change-Id: Ia0c300c2909bbdd232f2a3c758a5709b70f06d1c
Signed-off-by: Taka Cho <takamune.cho@att.com>
---
 .../org/onap/policy/drools/serverpool/Bucket.java  | 35 ++++-----
 .../org/onap/policy/drools/serverpool/Server.java  | 82 +++++++++++++---------
 2 files changed, 67 insertions(+), 50 deletions(-)

(limited to 'feature-server-pool/src/main')

diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java
index c3b2ac82..b82f2e1d 100644
--- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java
+++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Bucket.java
@@ -66,6 +66,7 @@ import javax.ws.rs.client.Entity;
 import javax.ws.rs.client.WebTarget;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.Setter;
 import org.slf4j.Logger;
@@ -179,6 +180,7 @@ public class Bucket {
     private static final String QP_KEYWORD = "keyword";
     private static final String QP_DEST = "dest";
     private static final String QP_TTL = "ttl";
+    private static final String OWNED_STR = "Owned";
 
     // BACKUP data (only buckets for where we are the owner, or a backup)
 
@@ -579,7 +581,7 @@ public class Bucket {
      * @throws IOException when error occurred
      */
     public static void bucketMessage(
-        final PrintStream out, final String keyword, String message) throws IOException {
+        final PrintStream out, final String keyword, String message) {
 
         if (keyword == null) {
             out.println("'keyword' is mandatory");
@@ -1149,6 +1151,7 @@ public class Bucket {
      * on any server.
      * Each instance of this class corresponds to a 'Bucket' instance.
      */
+    @EqualsAndHashCode
     private static class TestBucket implements Comparable<TestBucket> {
         // bucket number
         int index;
@@ -1326,6 +1329,7 @@ public class Bucket {
      * around a 'TestServer' instance, as it would be if another specific
      * server failed.
      */
+    @EqualsAndHashCode
     private static class AdjustedTestServer
         implements Comparable<AdjustedTestServer> {
         TestServer server;
@@ -1529,15 +1533,12 @@ public class Bucket {
              * 'needBuckets' TreeSet: those with the fewest buckets allocated are
              * at the head of the list.
              */
-            Comparator<TestServer> bucketCount = new Comparator<>() {
-                @Override
-                public int compare(TestServer s1, TestServer s2) {
-                    int rval = s1.buckets.size() - s2.buckets.size();
-                    if (rval == 0) {
-                        rval = Util.uuidComparator.compare(s1.uuid, s2.uuid);
-                    }
-                    return rval;
+            Comparator<TestServer> bucketCount = (s1, s2) -> {
+                int rval = s1.buckets.size() - s2.buckets.size();
+                if (rval == 0) {
+                    rval = Util.uuidComparator.compare(s1.uuid, s2.uuid);
                 }
+                return rval;
             };
 
             // sort servers according to the order in which they can
@@ -1715,7 +1716,7 @@ public class Bucket {
                 // populate a 'TreeSet' of 'AdjustedTestServer' instances based
                 // the failure of 'failedServer'
                 TreeSet<AdjustedTestServer> adjustedTestServers =
-                    new TreeSet<AdjustedTestServer>();
+                    new TreeSet<>();
                 for (TestServer server : testServers.values()) {
                     if (server == failedServer
                             || Objects.equals(siteSocketAddress,
@@ -1872,11 +1873,11 @@ public class Bucket {
                 // dump out 'owned' bucket information
                 if (ts.buckets.isEmpty()) {
                     // no buckets owned by this server
-                    out.printf(format, ts.uuid, "Owned", 0, "");
+                    out.printf(format, ts.uuid, OWNED_STR, 0, "");
                 } else {
                     // dump out primary buckets information
                     totalOwner +=
-                        dumpBucketsSegment(out, format, ts.buckets, ts.uuid.toString(), "Owned");
+                        dumpBucketsSegment(out, format, ts.buckets, ts.uuid.toString(), OWNED_STR);
                 }
                 // optionally dump out primary buckets information
                 totalPrimary +=
@@ -1899,7 +1900,7 @@ public class Bucket {
 
                 // optionally dump out unassigned owned buckets information
                 if (dumpBucketsSegment(out, format, nullServer.buckets,
-                                       uuidField, "Owned") != 0) {
+                                       uuidField, OWNED_STR) != 0) {
                     uuidField = "";
                 }
                 // optionally dump out unassigned primary backup buckets information
@@ -2247,7 +2248,9 @@ public class Bucket {
                         && oldOwner.isActive()
                         && (delay = getTimeout()) > 0) {
                     // ignore return value -- 'data' will indicate the result
-                    dataAvailable.await(delay, TimeUnit.MILLISECONDS);
+                    if (!dataAvailable.await(delay, TimeUnit.MILLISECONDS)) {
+                        logger.error("CountDownLatch await time reached");
+                    }
                 }
                 if (lclData == null) {
                     // no data available -- log an error, and abort
@@ -2282,11 +2285,11 @@ public class Bucket {
             } catch (Exception e) {
                 logger.error("Exception in {}", this, e);
             } finally {
-                run_cleanup();
+                runCleanup();
             }
         }
 
-        private void run_cleanup() {
+        private void runCleanup() {
             /*
              * cleanly leave state -- we want to make sure that messages
              * are processed in order, so the queue needs to remain until
diff --git a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java
index c60683ef..634c15ec 100644
--- a/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java
+++ b/feature-server-pool/src/main/java/org/onap/policy/drools/serverpool/Server.java
@@ -203,6 +203,9 @@ public class Server implements Comparable<Server> {
     // 'pingHosts' error
     static final String PINGHOSTS_ERROR = "Server.pingHosts error";
 
+    // a string for print
+    static final String PRINTOUT_DASHES = "-------";
+
     /*==============================*/
     /* Comparable<Server> interface */
     /*==============================*/
@@ -963,13 +966,11 @@ public class Server implements Comparable<Server> {
                 if (responseCallback != null) {
                     responseCallback.exceptionResponse(e);
                 }
-                MainLoop.queueWork(() -> {
-                    // this runs in the 'MainLoop' thread
+                // this runs in the 'MainLoop' thread
 
-                    // the DNS cache may have been out-of-date when this server
-                    // was first contacted -- fix the problem, if needed
-                    checkServer();
-                });
+                // the DNS cache may have been out-of-date when this server
+                // was first contacted -- fix the problem, if needed
+                MainLoop.queueWork(this::checkServer);
             }
         });
     }
@@ -985,7 +986,7 @@ public class Server implements Comparable<Server> {
             sendThreadPool =
                 new ThreadPoolExecutor(corePoolSize, maximumPoolSize,
                                        keepAliveTime, TimeUnit.MILLISECONDS,
-                                       new LinkedTransferQueue<Runnable>());
+                                       new LinkedTransferQueue<>());
             sendThreadPool.allowCoreThreadTimeOut(true);
         }
         return sendThreadPool;
@@ -1170,31 +1171,7 @@ public class Server implements Comparable<Server> {
                     new String(Base64.getEncoder().encode(bos.toByteArray()),
                         StandardCharsets.UTF_8),
                     MediaType.APPLICATION_OCTET_STREAM_TYPE);
-
-                // loop through hosts
-                for (InetSocketAddress host : hosts) {
-                    HttpClient httpClient = null;
-
-                    try {
-                        httpClient = buildClient(host.toString(), host,
-                                                 socketAddressToName(host));
-                        getTarget(httpClient).path("admin").request().post(entity);
-                        httpClient.shutdown();
-                        httpClient = null;
-                    } catch (KeyManagementException | NoSuchAlgorithmException e) {
-                        out.println(host + ": Unable to create client connection");
-                        logger.error(PINGHOSTS_ERROR, e);
-                    } catch (NoSuchFieldException | IllegalAccessException e) {
-                        out.println(host + ": Unable to get link to target");
-                        logger.error(PINGHOSTS_ERROR, e);
-                    } catch (Exception e) {
-                        out.println(host + ": " + e);
-                        logger.error(PINGHOSTS_ERROR, e);
-                    }
-                    if (httpClient != null) {
-                        httpClient.shutdown();
-                    }
-                }
+                pingHostsLoop(entity, out, hosts);
             } catch (IOException e) {
                 out.println("Unable to generate 'ping' data: " + e);
                 logger.error(PINGHOSTS_ERROR, e);
@@ -1213,6 +1190,43 @@ public class Server implements Comparable<Server> {
         }
     }
 
+    /**
+     * This method is used for pingHosts method to reduce its Cognitive Complexity.
+     *
+     * @param entity for sending out to all hosts
+     * @param out the 'PrintStream' to use for displaying information
+     * @param hosts a collection of 'InetSocketAddress' instances, which are
+     *     the hosts to send the information to
+     */
+    static void pingHostsLoop(final Entity<String> entity,
+                              final PrintStream out,
+                              final Collection<InetSocketAddress> hosts) {
+        // loop through hosts
+        for (InetSocketAddress host : hosts) {
+            HttpClient httpClient = null;
+
+            try {
+                httpClient = buildClient(host.toString(), host,
+                                     socketAddressToName(host));
+                getTarget(httpClient).path("admin").request().post(entity);
+                httpClient.shutdown();
+                httpClient = null;
+            } catch (KeyManagementException | NoSuchAlgorithmException e) {
+                out.println(host + ": Unable to create client connection");
+                logger.error(PINGHOSTS_ERROR, e);
+            } catch (NoSuchFieldException | IllegalAccessException e) {
+                out.println(host + ": Unable to get link to target");
+                logger.error(PINGHOSTS_ERROR, e);
+            } catch (Exception e) {
+                out.println(host + ": " + e);
+                logger.error(PINGHOSTS_ERROR, e);
+            }
+            if (httpClient != null) {
+                httpClient.shutdown();
+            }
+        }
+    }
+
     /**
      * This method may be invoked from any thread:
      * Dump out the current 'servers' table in a human-readable table form.
@@ -1264,14 +1278,14 @@ public class Server implements Comparable<Server> {
                        "Count", "Update Time", "Elapsed", "Allowed");
             out.printf(format, "", "----", "----------", "----",
                        "---------------", "----",
-                       "-----", "-----------", "-------", "-------");
+                       "-----", "-----------", PRINTOUT_DASHES, PRINTOUT_DASHES);
             // @formatter:on
         } else {
             // @formatter:off
             out.printf(format, "", "UUID", "IP Address", "Port",
                        "Count", "Update Time", "Elapsed", "Allowed");
             out.printf(format, "", "----", "----------", "----",
-                       "-----", "-----------", "-------", "-------");
+                       "-----", "-----------", PRINTOUT_DASHES, PRINTOUT_DASHES);
             // @formatter:on
         }
 
-- 
cgit 1.2.3-korg