From ce122b6f177512e5a9c839dba971b82f6a3e40c7 Mon Sep 17 00:00:00 2001 From: Taka Cho 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 --- .../org/onap/policy/drools/serverpool/Bucket.java | 35 ++++----- .../org/onap/policy/drools/serverpool/Server.java | 82 +++++++++++++--------- 2 files changed, 67 insertions(+), 50 deletions(-) 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 { // 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 { 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 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 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 adjustedTestServers = - new TreeSet(); + 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 { // 'pingHosts' error static final String PINGHOSTS_ERROR = "Server.pingHosts error"; + // a string for print + static final String PRINTOUT_DASHES = "-------"; + /*==============================*/ /* Comparable interface */ /*==============================*/ @@ -963,13 +966,11 @@ public class Server implements Comparable { 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 { sendThreadPool = new ThreadPoolExecutor(corePoolSize, maximumPoolSize, keepAliveTime, TimeUnit.MILLISECONDS, - new LinkedTransferQueue()); + new LinkedTransferQueue<>()); sendThreadPool.allowCoreThreadTimeOut(true); } return sendThreadPool; @@ -1170,31 +1171,7 @@ public class Server implements Comparable { 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 { } } + /** + * 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 entity, + final PrintStream out, + final Collection 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 { "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