Code smell cleanup (#353)

* Code smell cleanup

* attempt to correct code coverage reporting for Sonar

* moved static code inside constructor for easier unit testing

Author: ddsharpe

imagetool/src/main/java/com/oracle/weblogic/imagetool/aru/AruUtil.java

@@ -37,21 +37,31 @@ public class AruUtil {
 
     private static final String BUG_SEARCH_URL = ARU_REST_URL + "/search?bug=%s";
 
-    private static int REST_RETRIES = 10;
-    private static int REST_INTERVAL = 500;
+    private int restRetries = 10;
+    private int restInterval = 500;
 
-    static {
-        // static block to parst environment variable overrides for REST call defaults
+    /**
+     * Get ARU HTTP helper instance.
+     * @return ARU helper.
+     */
+    public static AruUtil rest() {
+        if (instance == null) {
+            instance = new AruUtil();
+        }
+        return instance;
+    }
+
+    protected AruUtil() {
         final String retriesEnvVar = "WLSIMG_REST_RETRY_MAX";
         final String retriesString = System.getenv(retriesEnvVar);
         try {
             if (retriesString != null) {
-                REST_RETRIES = Integer.parseInt(retriesString);
-                if (REST_RETRIES < 1) {
-                    REST_RETRIES = 10;
-                    logger.severe("IMG-0109", retriesEnvVar, retriesString, 1, REST_RETRIES);
+                restRetries = Integer.parseInt(retriesString);
+                if (restRetries < 1) {
+                    restRetries = 10;
+                    logger.severe("IMG-0109", retriesEnvVar, retriesString, 1, restRetries);
                 }
-                logger.fine("Retry max set to {0}", REST_RETRIES);
+                logger.fine("Retry max set to {0}", restRetries);
             }
         } catch (NumberFormatException nfe) {
             logger.warning("IMG-0108", retriesEnvVar, retriesString);
@@ -61,33 +71,18 @@ public class AruUtil {
         final String waitString = System.getenv(waitEnvVar);
         try {
             if (waitString != null) {
-                REST_INTERVAL = Integer.parseInt(waitString);
-                if (REST_INTERVAL < 0) {
-                    REST_INTERVAL = 500;
-                    logger.severe("IMG-0109", waitEnvVar, waitString, 0, REST_INTERVAL);
+                restInterval = Integer.parseInt(waitString);
+                if (restInterval < 0) {
+                    restInterval = 500;
+                    logger.severe("IMG-0109", waitEnvVar, waitString, 0, restInterval);
                 }
-                logger.fine("Retry interval set to {0}", REST_INTERVAL);
+                logger.fine("Retry interval set to {0}", restInterval);
             }
         } catch (NumberFormatException nfe) {
             logger.warning("IMG-0108", waitEnvVar, waitString);
         }
     }
 
-    /**
-     * Get ARU HTTP helper instance.
-     * @return ARU helper.
-     */
-    public static AruUtil rest() {
-        if (instance == null) {
-            instance = new AruUtil();
-        }
-        return instance;
-    }
-
-    protected AruUtil() {
-        // hide constructor
-    }
-
     /**
      * Get list of PSU available for each of the ARU products for the given FMW install type.
      *
@@ -482,24 +477,44 @@ public String downloadAruPatch(AruPatch aruPatch, String targetDir, String usern
         return filename;
     }
 
+    /**
+     * The maximum number of retries that will be attempted when trying to reach the ARU REST API method.
+     * This value can be set by using the environment variable WLSIMG_REST_RETRY_MAX.
+     *
+     * @return The maximum number of retries to attempt.
+     */
+    public int getMaxRetries() {
+        return restRetries;
+    }
+
+    /**
+     * The time between each ARU REST retry.
+     * This value can be set by using the environment variable WLSIMG_REST_RETRY_INTERVAL.
+     *
+     * @return The time to wait between each ARU REST API attempt during the retry loop.
+     */
+    public int getRetryInterval() {
+        return restInterval;
+    }
+
     private interface CallToRetry {
         Document process() throws IOException, XPathExpressionException, AruException;
     }
 
     // create an environment variable that can override the tries count (undocumented)
     private static Document retry(CallToRetry call) throws AruException, RetryFailedException {
-        for (int i = 0; i < REST_RETRIES; i++) {
+        for (int i = 0; i < rest().getMaxRetries(); i++) {
             try {
                 return call.process();
             } catch (UnknownHostException e) {
                 throw new AruException(e.getLocalizedMessage(), e);
             } catch (IOException | XPathExpressionException e) {
-                logger.info("IMG-0106", e.getMessage(), (i + 1), REST_RETRIES);
+                logger.info("IMG-0106", e.getMessage(), (i + 1), rest().getMaxRetries());
             }
             try {
-                if (REST_INTERVAL > 0) {
-                    logger.finer("Waiting {0} ms before retry...", REST_INTERVAL);
-                    Thread.sleep(REST_INTERVAL);
+                if (rest().getRetryInterval() > 0) {
+                    logger.finer("Waiting {0} ms before retry...", rest().getRetryInterval());
+                    Thread.sleep(rest().getRetryInterval());
                 }
             } catch (InterruptedException wakeAndAbort) {
                 logger.warning("Process interrupted!");

imagetool/src/main/java/com/oracle/weblogic/imagetool/aru/InstalledPatch.java

@@ -32,18 +32,15 @@ public static List<InstalledPatch> getPatchList(String oraclePatches) {
             return result;
         }
         String[] tokens = oraclePatches.split(";");
+        // Each patch record is made up of 3 fields.
         if (tokens.length % 3 != 0) {
             logger.severe("IMG-0095", tokens.length);
         }
-        for (int i = 0; i < tokens.length; i++) {
+        for (int j = 0; j < tokens.length; j = j + 3) {
             InstalledPatch found = new InstalledPatch();
-            found.bugNumber = tokens[i];
-            if (tokens.length > i + 1) {
-                found.uniquePatchNumber = tokens[++i];
-            }
-            if (tokens.length > i + 1) {
-                found.patchDescription = tokens[++i].replaceAll("^\"|\"$", "");
-            }
+            found.bugNumber = tokens[j];
+            found.uniquePatchNumber = tokens[j + 1];
+            found.patchDescription = tokens[j + 2].replaceAll("(^\")|(\"$)", "");
             result.add(found);
         }
 

imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/menu/CommonOptions.java

@@ -167,7 +167,7 @@ void initializeOptions() throws InvalidCredentialException, IOException, Invalid
         handleChown();
         handleAdditionalBuildCommands();
 
-        if (kubernetesTarget == KubernetesTarget.OpenShift) {
+        if (kubernetesTarget == KubernetesTarget.OPENSHIFT) {
             dockerfileOptions.setDomainGroupAsUser(true);
             // if the user did not set the OS user:group, make the default oracle:root, instead of oracle:oracle
             if (!isOptionSet(osUserAndGroup)) {
@@ -373,7 +373,7 @@ public String buildId() {
         description = "Apply settings appropriate to the target environment.  Default: ${DEFAULT-VALUE}."
             + "  Supported values: ${COMPLETION-CANDIDATES}."
     )
-    KubernetesTarget kubernetesTarget = KubernetesTarget.Default;
+    KubernetesTarget kubernetesTarget = KubernetesTarget.DEFAULT;
 
     @SuppressWarnings("unused")
     @Unmatched

imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/menu/KubernetesTarget.java

@@ -4,6 +4,22 @@
 package com.oracle.weblogic.imagetool.cli.menu;
 
 public enum KubernetesTarget {
-    Default,
-    OpenShift
+    DEFAULT("Default"),
+    OPENSHIFT("OpenShift");
+
+    private String value;
+
+    /**
+     * Create an Enum value for the installer type.
+     * @param value value is the first part of the key for the installer in the cache, and the filename of the
+     *              response file
+     */
+    KubernetesTarget(String value) {
+        this.value = value;
+    }
+
+    @Override
+    public String toString() {
+        return value;
+    }
 }

pom.xml

@@ -33,6 +33,7 @@
     <properties>
         <sonar.organization>oracle</sonar.organization>
         <sonar.host.url>https://sonarcloud.io</sonar.host.url>
+        <sonar.coverage.jacoco.xmlReportPaths>tests/target/site/jacoco-aggregate/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
 
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <maven.compiler.source>1.8</maven.compiler.source>

tests/src/test/java/com/oracle/weblogic/imagetool/tests/ITImagetool.java

@@ -1119,7 +1119,7 @@ void createWlsImgWithOpenShiftSettings(TestInfo testInfo) throws Exception {
             .wdtDomainHome("/u01/domains/simple_domain")
             .wdtModel(WDT_MODEL, WDT_MODEL2)
             .wdtVariables(WDT_VARIABLES)
-            .target(KubernetesTarget.OpenShift)
+            .target(KubernetesTarget.OPENSHIFT)
             .build();
 
         try (PrintWriter out = getTestMethodWriter(testInfo)) {