Reduce complexity of HealthCheckUtils

* Split bulk conditionals
* Split health checks to separate methods
* Fix minor issues

Issue-ID: SO-353
Change-Id: Ib6298bc488a94aa4fbb253e3894532708547533d
Signed-off-by: kurczews <krzysztof.kurczewski@nokia.com>

2 files changed, 98 insertions, 117 deletions

diff --git a/status-control/src/main/java/org/openecomp/mso/HealthCheckUtils.java b/status-control/src/main/java/org/openecomp/mso/HealthCheckUtils.java
index e7c4e0d3de..b64f5bf232 100644
--- a/status-control/src/main/java/org/openecomp/mso/HealthCheckUtils.java
+++ b/status-control/src/main/java/org/openecomp/mso/HealthCheckUtils.java
@@ -55,7 +55,7 @@ public class HealthCheckUtils {
             .entity (NOT_FOUND)
             .build ();
 
-    public enum NodeType {APIH, RA, BPMN};
+    public enum NodeType {APIH, RA, BPMN}
 
     public boolean catalogDBCheck (MsoLogger subMsoLogger, long startTime) {
         try(CatalogDatabase catalogDB = CatalogDatabase.getInstance()) {
@@ -79,7 +79,7 @@ public class HealthCheckUtils {
         return true;
     }
 
-    public boolean siteStatusCheck (MsoLogger subMsoLogger, long startTime) {
+    public boolean siteStatusCheck(MsoLogger subMsoLogger) {
         // Check the Site Status value in DB first, if set to false, return NOK
         String site = getProperty("site-name");
 
@@ -180,57 +180,26 @@ public class HealthCheckUtils {
         if (null == topologyProp) {
             return false;
         }
-        String port = topologyProp.getProperty("server-port", null);
-        String ip = System.getProperty("jboss.qualified.host.name");
-        String sslEnabled = topologyProp.getProperty("ssl-enable", null);
 
-        if (null == port || null == ip || ip.isEmpty() || port.isEmpty()) {
-            msoLogger.error (MessageEnum.GENERAL_EXCEPTION_ARG, "Not able to get the IP or the Port value. IP:" + ip + "; Port:" + port, "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, "Not able to get the IP or the Port value. IP:" + ip + "; Port:" + port);
-            return false;
+        checkHealthForProperty(topologyProp, "", requestId);
+
+        boolean healthCheck = false;
+        switch (type) {
+            case APIH:
+                healthCheck = checkHealthForProperty(topologyProp, "apih-healthcheck-urn", requestId);
+                break;
+            case RA:
+                healthCheck = checkHealthForProperty(topologyProp, "jra-healthcheck-urn", requestId);
+                break;
+            case BPMN:
+                healthCheck = checkHealthForProperty(topologyProp, "camunda-healthcheck-urn", requestId);
+                break;
+            default:
+                msoLogger.error(MessageEnum.GENERAL_EXCEPTION_ARG, "Unknown NodeType:" + type, "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, "Unknown NodeType:" + type);
+                break;
         }
 
-        String[] apis;
-        if (NodeType.APIH.equals (type)) {
-            String apiList = topologyProp.getProperty("apih-healthcheck-urn", null);
-            if (null == apiList) {
-                String errorDescription = "Not able to get apih-healthcheck-urn parameter";
-                msoLogger.error (MessageEnum.GENERAL_EXCEPTION_ARG, errorDescription, "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, errorDescription);
-                return false;
-            }
-            apis = apiList.split(",");
-        } else if (NodeType.RA.equals (type)){
-            String apiList = topologyProp.getProperty("jra-healthcheck-urn", null);
-            if (null == apiList) {
-                String errorDescription = "Not able to get jra-healthcheck-urn parameter";
-                msoLogger.error (MessageEnum.GENERAL_EXCEPTION_ARG, errorDescription, "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, errorDescription);
-                return false;
-            }
-            apis = apiList.split(",");
-        } else if (NodeType.BPMN.equals (type)){
-            String apiList = topologyProp.getProperty("camunda-healthcheck-urn", null);
-            if (null == apiList) {
-                String errorDescription = "Not able to get camunda-healthcheck-urn parameter";
-                msoLogger.error (MessageEnum.GENERAL_EXCEPTION_ARG, errorDescription, "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, errorDescription);
-                return false;
-            }
-            apis = apiList.split(",");
-        } else {
-            msoLogger.error (MessageEnum.GENERAL_EXCEPTION_ARG, "Unknown NodeType:" + type, "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, "Unknown NodeType:" + type);
-            return false;
-        }
-
-        // Verify health check on APIH servers
-        for (String url : apis) {
-            // if any of the parameters is null or empty, no need to establish the health check request, just go to the next iteration
-            if ((url == null)  || url.isEmpty()) {
-                continue;
-            }
-            // Exit the loop if local health check returns false from any of the sub component
-            if (!this.verifyLocalHealth(ip, port, url, sslEnabled, requestId)) {
-                return false;
-            }
-        }
-        return true;
+        return healthCheck;
     }
 
     public boolean verifyGlobalHealthCheck(boolean verifyBpmn, String requestId) {
@@ -241,56 +210,99 @@ public class HealthCheckUtils {
             return false;
         }
 
-        String apihLB = topologyProp.getProperty("apih-load-balancer", null);
-        String apihApi = topologyProp.getProperty("apih-nodehealthcheck-urn", null);
-        String bpmnLB= topologyProp.getProperty("camunda-load-balancer", null);
-        String bpmnApi = topologyProp.getProperty("camunda-nodehealthcheck-urn", null);
+        return verifyApihServersHealthCheck(topologyProp, requestId) &&
+                verifyCamundaServersHealthCheck(topologyProp, requestId, verifyBpmn) &&
+                verifyRaServersHealthCheck(topologyProp, requestId);
+    }
+
+    public String getProperty (String name) {
+        MsoJavaProperties prop = this.loadTopologyProperties();
+
+        return prop.getProperty(name, null);
+    }
+
+    protected String getFinalUrl (String ip, String port, String url, String sslEnabled) {
+        if (null == port && null == sslEnabled) {
+            int length = ip.length();
+            if ("/".equals(ip.substring(length - 1))) {
+                ip = ip.substring(0, length - 1);
+            }
+            return ip + url;
+        } else if ("true".equalsIgnoreCase(sslEnabled)) {
+            return "https://" + ip + ":" + port + url;
+        } else {
+            return "http://" + ip + ":" + port + url;
+        }
+    }
+
+    private boolean verifyRaServersHealthCheck(MsoJavaProperties topologyProp, String requestId) {
         String jraLB = topologyProp.getProperty("jra-load-balancer", null);
         String jraApi = topologyProp.getProperty("jra-nodehealthcheck-urn", null);
 
-        if (null == apihLB || null == apihApi || null == bpmnLB || null == bpmnApi || null == jraLB || null == jraApi
-                || apihLB.isEmpty () || apihApi.isEmpty () || bpmnLB.isEmpty () || bpmnApi.isEmpty () || jraLB.isEmpty () || jraApi.isEmpty () ) {
-            msoLogger.error (MessageEnum.GENERAL_EXCEPTION_ARG, "Key parameters are missing from the topology file", "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, "Key parameters are missing from the topology file");
+        if (null == jraLB || null == jraApi || jraLB.isEmpty() || jraApi.isEmpty()) {
+            msoLogger.error(MessageEnum.GENERAL_EXCEPTION_ARG, "Key parameters are missing from the topology file", "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, "Key parameters are missing from the topology file");
             return false;
         }
 
-        // Verify health check on APIH servers
-        if (!this.verifyLocalHealth (apihLB, null, apihApi, null, requestId)) {
+        return verifyLocalHealth(jraLB, null, jraApi, null, requestId);
+    }
+
+    private boolean verifyCamundaServersHealthCheck(MsoJavaProperties topologyProp, String requestId, boolean verifyBpmn) {
+        String bpmnLB = topologyProp.getProperty("camunda-load-balancer", null);
+        String bpmnApi = topologyProp.getProperty("camunda-nodehealthcheck-urn", null);
+
+        if (null == bpmnLB || null == bpmnApi || bpmnLB.isEmpty() || bpmnApi.isEmpty()) {
+            msoLogger.error(MessageEnum.GENERAL_EXCEPTION_ARG, "Key parameters are missing from the topology file", "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, "Key parameters are missing from the topology file");
             return false;
         }
 
-        // Verify health check on Camunda servers
-        if (verifyBpmn) {
-            if (!this.verifyLocalHealth (bpmnLB, null, bpmnApi, null, requestId)) {
-                return false;
-            }
-        }
+        return !verifyBpmn || verifyLocalHealth(bpmnLB, null, bpmnApi, null, requestId);
+    }
 
-        // Verify health check on RA servers
-        if (!verifyLocalHealth (jraLB, null, jraApi, null, requestId)) {
+    private boolean verifyApihServersHealthCheck(MsoJavaProperties topologyProp, String requestId) {
+        String apihLB = topologyProp.getProperty("apih-load-balancer", null);
+        String apihApi = topologyProp.getProperty("apih-nodehealthcheck-urn", null);
+
+        if (null == apihLB || null == apihApi || apihLB.isEmpty() || apihApi.isEmpty()) {
+            msoLogger.error(MessageEnum.GENERAL_EXCEPTION_ARG, "Key parameters are missing from the topology file", "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, "Key parameters are missing from the topology file");
             return false;
         }
 
-        return true;
+        return verifyLocalHealth(apihLB, null, apihApi, null, requestId);
     }
 
-    public String getProperty (String name) {
-        MsoJavaProperties prop = this.loadTopologyProperties();
-
-        return prop.getProperty(name, null);
+    private boolean checkHealthForProperty(MsoJavaProperties topologyProp, String property, String requestId) {
+        String apiList = topologyProp.getProperty(property, null);
+        if (apiList == null) {
+            String errorDescription = "Not able to get " + property + " parameter";
+            msoLogger.error(MessageEnum.GENERAL_EXCEPTION_ARG, errorDescription, "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, errorDescription);
+            return false;
+        }
+        String[] apis = apiList.split(",");
+        return checkHealthForEachApi(topologyProp, apis, requestId);
     }
 
-    protected String getFinalUrl (String ip, String port, String url, String sslEnabled) {
-        if (null == port && null == sslEnabled) {
-            int length = ip.length();
-            if (ip.substring(length - 1).equals ("/")) {
-                ip = ip.substring (0, length - 1);
+    private boolean checkHealthForEachApi(MsoJavaProperties topologyProp, String[] apis, String requestId) {
+
+        String port = topologyProp.getProperty("server-port", null);
+        String ip = System.getProperty("jboss.qualified.host.name");
+        String sslEnabled = topologyProp.getProperty("ssl-enable", null);
+
+        if (null == port || null == ip || ip.isEmpty() || port.isEmpty()) {
+            msoLogger.error(MessageEnum.GENERAL_EXCEPTION_ARG, "Not able to get the IP or the Port value. IP:" + ip + "; Port:" + port, "", HEALTH_CHECK, MsoLogger.ErrorCode.DataError, "Not able to get the IP or the Port value. IP:" + ip + "; Port:" + port);
+            return false;
+        }
+
+        for (String url : apis) {
+            // if any of the parameters is null or empty, no need to establish the health check request, just go to the next iteration
+            if ((url == null) || url.isEmpty()) {
+                continue;
+            }
+            // Exit the loop if local health check returns false from any of the sub component
+            if (!this.verifyLocalHealth(ip, port, url, sslEnabled, requestId)) {
+                return false;
             }
-            return ip + url;
-        } else if ("true".equalsIgnoreCase(sslEnabled)) {
-            return "https://" + ip + ":" + port + url;
-        } else {
-            return "http://" + ip + ":" + port + url;
         }
+        return true;
     }
 }
diff --git a/status-control/src/test/java/org/openecomp/mso/HealthCheckUtilsTest.java b/status-control/src/test/java/org/openecomp/mso/HealthCheckUtilsTest.java
index cdb5695f2c..91016d2475 100644
--- a/status-control/src/test/java/org/openecomp/mso/HealthCheckUtilsTest.java
+++ b/status-control/src/test/java/org/openecomp/mso/HealthCheckUtilsTest.java
@@ -21,22 +21,16 @@
 package org.openecomp.mso;
 
 
-import org.openecomp.mso.logger.MsoLogger;
-import org.openecomp.mso.properties.MsoJavaProperties;
-import org.openecomp.mso.requestsdb.RequestsDatabase;
 import org.apache.http.HttpStatus;
 import org.apache.http.HttpVersion;
 import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.message.BasicStatusLine;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.mockito.Mockito;
-
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
+import org.openecomp.mso.properties.MsoJavaProperties;
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -63,7 +57,7 @@ public class HealthCheckUtilsTest {
     private static CloseableHttpResponse nokRes, okRes;
 
     @BeforeClass
-    public static final void prepareMockvalues() {
+    public static void prepareMockvalues() {
         utils = Mockito.mock(HealthCheckUtils.class);
         client = Mockito.mock(CloseableHttpClient.class);
         nokRes = Mockito.mock(CloseableHttpResponse.class);
@@ -207,11 +201,11 @@ public class HealthCheckUtilsTest {
 
         try {
             Mockito.when (client.execute (any(HttpUriRequest.class))).thenReturn (okRes);
-            Boolean res1 = (Boolean)invokeProtectedMethod(tempUtil, "verifyLocalHealth", ip1, port, apihUrl1, sslEnable, null);
+            boolean res1 = tempUtil.verifyLocalHealth(ip1, port, apihUrl1, sslEnable, null);
             assertTrue(res1);
 
             Mockito.when (client.execute (any(HttpUriRequest.class))).thenReturn (nokRes);
-            Boolean res2 = (Boolean)invokeProtectedMethod(tempUtil, "verifyLocalHealth", ip1, port, apihUrl1, sslEnable, null);
+            boolean res2 = tempUtil.verifyLocalHealth(ip1, port, apihUrl1, sslEnable, null);
             assertFalse(res2);
 
         } catch (Exception e) {
@@ -270,29 +264,4 @@ public class HealthCheckUtilsTest {
         Mockito.when(utils.loadTopologyProperties()).thenReturn(properties);
     }
 
-    // User reflection to invoke to avoid change the publicity of the method
-    private static Object invokeProtectedMethod (HealthCheckUtils tempUtil, String methodName, String arg1, String arg2, String arg3, String arg4, String arg5) {
-        Method method;
-        try {
-            method = HealthCheckUtils.class.getDeclaredMethod(methodName, String.class, String.class, String.class, String.class, String.class);
-            method.setAccessible(true);
-            return  method.invoke(tempUtil, arg1, arg2, arg3, arg4, arg5);
-        } catch (NoSuchMethodException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
-        } catch (SecurityException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
-        } catch (IllegalAccessException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
-        } catch (IllegalArgumentException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
-        } catch (InvocationTargetException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
-        }
-        return null;
-    }
 }
\ No newline at end of file