Refinements to RestApiCall plugin

Changes includes:
1) Check for null in JsonParser.convertToProperties() which can otherwise result in null pointer exception
2) Use logger built-in string formatting rather then string concatenation
3) Use StringBuilder for multiple string concatenations
4) Making utility classes final and defines private constructor for them
5) Added testcases/testpoints
https://sonar.onap.org/component_issues/index?id=org.onap.ccsdk.sli.plugins%3Accsdk-sli-plugins#resolved=false|severities=CRITICAL

Change-Id: Ic047b6d0369827a38a98c52e8365f1fe7266840f
Issue-Id: CCSDK-67
Signed-off-by: Gaurav Agrawal <gaurav.agrawal@huawei.com>

diff --git a/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/JsonParser.java b/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/JsonParser.java
index f2867f5..4a1dfef 100644 (file)
--- a/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/JsonParser.java
+++ b/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/JsonParser.java
@@ -21,6 +21,8 @@
 
 package org.onap.ccsdk.sli.plugins.restapicall;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -29,57 +31,64 @@ import java.util.Map;
 import org.codehaus.jettison.json.JSONArray;
 import org.codehaus.jettison.json.JSONException;
 import org.codehaus.jettison.json.JSONObject;
+import org.onap.ccsdk.sli.core.sli.SvcLogicException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class JsonParser {
+public final class JsonParser {
 
     private static final Logger log = LoggerFactory.getLogger(JsonParser.class);
 
-    @SuppressWarnings("unchecked")
-    public static Map<String, String> convertToProperties(String s) throws JSONException {
-        JSONObject json = new JSONObject(s);
-
-        Map<String, Object> wm = new HashMap<>();
-        Iterator<String> ii = json.keys();
-        while (ii.hasNext()) {
-            String key1 = ii.next();
-            wm.put(key1, json.get(key1));
-        }
-
-        Map<String, String> mm = new HashMap<>();
+    private JsonParser() {
+        // Preventing instantiation of the same.
+    }
 
-        while (!wm.isEmpty())
-            for (String key : new ArrayList<>(wm.keySet())) {
-                Object o = wm.get(key);
-                wm.remove(key);
+    @SuppressWarnings("unchecked")
+    public static Map<String, String> convertToProperties(String s)
+        throws SvcLogicException {
+
+        checkNotNull(s, "Input should not be null.");
+
+        try {
+            JSONObject json = new JSONObject(s);
+            Map<String, Object> wm = new HashMap<>();
+            Iterator<String> ii = json.keys();
+            while (ii.hasNext()) {
+                String key1 = ii.next();
+                wm.put(key1, json.get(key1));
+            }
 
-                if (o instanceof Boolean || o instanceof Number || o instanceof String) {
-                    mm.put(key, o.toString());
+            Map<String, String> mm = new HashMap<>();
 
-                    log.info("Added property: " + key + ": " + o.toString());
-                }
+            while (!wm.isEmpty())
+                for (String key : new ArrayList<>(wm.keySet())) {
+                    Object o = wm.get(key);
+                    wm.remove(key);
 
-                else if (o instanceof JSONObject) {
-                    JSONObject jo = (JSONObject) o;
-                    Iterator<String> i = jo.keys();
-                    while (i.hasNext()) {
-                        String key1 = i.next();
-                        wm.put(key + "." + key1, jo.get(key1));
-                    }
-                }
+                    if (o instanceof Boolean || o instanceof Number || o instanceof String) {
+                        mm.put(key, o.toString());
 
-                else if (o instanceof JSONArray) {
-                    JSONArray ja = (JSONArray) o;
-                    mm.put(key + "_length", String.valueOf(ja.length()));
+                        log.info("Added property: {} : {}", key, o.toString());
+                    } else if (o instanceof JSONObject) {
+                        JSONObject jo = (JSONObject) o;
+                        Iterator<String> i = jo.keys();
+                        while (i.hasNext()) {
+                            String key1 = i.next();
+                            wm.put(key + "." + key1, jo.get(key1));
+                        }
+                    } else if (o instanceof JSONArray) {
+                        JSONArray ja = (JSONArray) o;
+                        mm.put(key + "_length", String.valueOf(ja.length()));
 
-                    log.info("Added property: " + key + "_length" + ": " + String.valueOf(ja.length()));
+                        log.info("Added property: {}_length: {}", key, String.valueOf(ja.length()));
 
-                    for (int i = 0; i < ja.length(); i++)
-                        wm.put(key + '[' + i + ']', ja.get(i));
+                        for (int i = 0; i < ja.length(); i++)
+                            wm.put(key + '[' + i + ']', ja.get(i));
+                    }
                 }
-            }
-
-        return mm;
+            return mm;
+        } catch (JSONException e) {
+            throw new SvcLogicException("Unable to convert JSON to properties" + e.getLocalizedMessage(), e);
+        }
     }
 }

diff --git a/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/XmlJsonUtil.java b/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/XmlJsonUtil.java
index e80ca50..b94f0a6 100644 (file)
--- a/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/XmlJsonUtil.java
+++ b/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/XmlJsonUtil.java
@@ -29,10 +29,14 @@ import java.util.Map;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class XmlJsonUtil {
+public final class XmlJsonUtil {
 
     private static final Logger log = LoggerFactory.getLogger(XmlJsonUtil.class);
 
+    private XmlJsonUtil() {
+        // Preventing instantiation of the same.
+    }
+
     public static String getXml(Map<String, String> varmap, String var) {
         boolean escape = true;
         if (var.startsWith("'")) {
@@ -99,7 +103,7 @@ public class XmlJsonUtil {
                 try {
                     length = Integer.parseInt(lengthStr);
                 } catch (Exception e) {
-                    log.warn("Invalid number for " + var + "_length:" + lengthStr);
+                    log.warn("Invalid number for {}_length:{}", var, lengthStr);
                 }
             }
 
@@ -364,9 +368,9 @@ public class XmlJsonUtil {
     }
 
     private static String pad(int n) {
-        String s = "";
+        StringBuilder s = new StringBuilder();
         for (int i = 0; i < n; i++)
-            s += Character.toString('\t');
-        return s;
+            s.append(Character.toString('\t'));
+        return s.toString();
     }
 }

diff --git a/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/XmlParser.java b/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/XmlParser.java
index 61b0e31..7ef776d 100644 (file)
--- a/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/XmlParser.java
+++ b/restapi-call-node/provider/src/main/java/org/onap/ccsdk/sli/plugins/restapicall/XmlParser.java
@@ -21,37 +21,49 @@
 
 package org.onap.ccsdk.sli.plugins.restapicall;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
+import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
 
+import org.onap.ccsdk.sli.core.sli.SvcLogicException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xml.sax.Attributes;
 import org.xml.sax.SAXException;
 import org.xml.sax.helpers.DefaultHandler;
 
-public class XmlParser {
+public final class XmlParser {
 
     private static final Logger log = LoggerFactory.getLogger(XmlParser.class);
 
-    public static Map<String, String> convertToProperties(String s, Set<String> listNameList) {
+    private XmlParser() {
+        // Preventing instantiation of the same.
+    }
+
+    public static Map<String, String> convertToProperties(String s, Set<String> listNameList)
+        throws SvcLogicException {
+
+        checkNotNull(s, "Input should not be null.");
+
         Handler handler = new Handler(listNameList);
         try {
             SAXParserFactory factory = SAXParserFactory.newInstance();
             SAXParser saxParser = factory.newSAXParser();
             InputStream in = new ByteArrayInputStream(s.getBytes());
             saxParser.parse(in, handler);
-        } catch (Exception e) {
-            e.printStackTrace();
+        } catch (ParserConfigurationException | IOException | SAXException e) {
+            throw new SvcLogicException("Unable to convert XML to properties" + e.getLocalizedMessage(), e);
         }
-
         return handler.getProperties();
     }
 
@@ -72,8 +84,8 @@ public class XmlParser {
                 this.listNameList = new HashSet<>();
         }
 
-        String currentName = "";
-        String currentValue = "";
+        StringBuilder currentName = new StringBuilder();
+        StringBuilder currentValue = new StringBuilder();
 
         @Override
         public void startElement(String uri, String localName, String qName, Attributes attributes)
@@ -88,15 +100,16 @@ public class XmlParser {
                 name = name.substring(i2 + 1);
 
             if (currentName.length() > 0)
-                currentName += Character.toString('.');
-            currentName += name;
+                currentName.append(Character.toString('.'));
+            currentName.append(name);
 
-            String listName = removeIndexes(currentName);
+            String listName = removeIndexes(currentName.toString());
 
             if (listNameList.contains(listName)) {
-                int len = getInt(properties, currentName + "_length");
-                properties.put(currentName + "_length", String.valueOf(len + 1));
-                currentName += "[" + len + "]";
+                String n = currentName.toString() + "_length";
+                int len = getInt(properties, n);
+                properties.put(n, String.valueOf(len + 1));
+                currentName.append("[").append(len).append("]");
             }
         }
 
@@ -111,20 +124,19 @@ public class XmlParser {
             if (i2 >= 0)
                 name = name.substring(i2 + 1);
 
-            if (currentValue.trim().length() > 0) {
-                currentValue = currentValue.trim();
-                properties.put(currentName, currentValue);
-
-                log.info("Added property: " + currentName + ": " + currentValue);
+            String s = currentValue.toString().trim();
+            if (s.length() > 0) {
+                properties.put(currentName.toString(), s);
 
-                currentValue = "";
+                log.info("Added property: {} : {}", currentName, s);
+                currentValue = new StringBuilder();
             }
 
             int i1 = currentName.lastIndexOf("." + name);
             if (i1 <= 0)
-                currentName = "";
+                currentName = new StringBuilder();
             else
-                currentName = currentName.substring(0, i1);
+                currentName = new StringBuilder(currentName.substring(0, i1));
         }
 
         @Override
@@ -132,7 +144,7 @@ public class XmlParser {
             super.characters(ch, start, length);
 
             String value = new String(ch, start, length);
-            currentValue += value;
+            currentValue.append(value);
         }
 
         private static int getInt(Map<String, String> mm, String name) {
@@ -143,7 +155,7 @@ public class XmlParser {
         }
 
         private String removeIndexes(String currentName) {
-            String s = "";
+            StringBuilder b = new StringBuilder();
             boolean add = true;
             for (int i = 0; i < currentName.length(); i++) {
                 char c = currentName.charAt(i);
@@ -152,9 +164,9 @@ public class XmlParser {
                 else if (c == ']')
                     add = true;
                 else if (add)
-                    s += Character.toString(c);
+                    b.append(Character.toString(c));
             }
-            return s;
+            return b.toString();
         }
     }
 }

diff --git a/restapi-call-node/provider/src/test/java/jtest/org/onap/ccsdk/sli/plugins/restapicall/TestJsonParser.java b/restapi-call-node/provider/src/test/java/jtest/org/onap/ccsdk/sli/plugins/restapicall/TestJsonParser.java
index dbca5ad..5526be8 100644 (file)
--- a/restapi-call-node/provider/src/test/java/jtest/org/onap/ccsdk/sli/plugins/restapicall/TestJsonParser.java
+++ b/restapi-call-node/provider/src/test/java/jtest/org/onap/ccsdk/sli/plugins/restapicall/TestJsonParser.java
@@ -22,6 +22,7 @@
 package jtest.org.onap.ccsdk.sli.plugins.restapicall;
 
 import java.io.BufferedReader;
+import java.io.IOException;
 import java.io.InputStreamReader;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -29,6 +30,7 @@ import java.util.List;
 import java.util.Map;
 
 import org.junit.Test;
+import org.onap.ccsdk.sli.core.sli.SvcLogicException;
 import org.onap.ccsdk.sli.plugins.restapicall.JsonParser;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -38,30 +40,34 @@ public class TestJsonParser {
     private static final Logger log = LoggerFactory.getLogger(TestJsonParser.class);
 
     @Test
-    public void test() throws Exception {
+    public void test() throws SvcLogicException, IOException {
         BufferedReader in = new BufferedReader(
                 new InputStreamReader(ClassLoader.getSystemResourceAsStream("test.json"))
         );
-        String ss = "";
-        String line = null;
+        StringBuilder b = new StringBuilder();
+        String line;
         while ((line = in.readLine()) != null)
-            ss += line + '\n';
+            b.append(line).append('\n');
 
-        Map<String, String> mm = JsonParser.convertToProperties(ss);
+        Map<String, String> mm = JsonParser.convertToProperties(b.toString());
 
         logProperties(mm);
 
         in.close();
     }
 
+    @Test(expected = NullPointerException.class)
+    public void testNullString() throws SvcLogicException {
+        JsonParser.convertToProperties(null);
+    }
+
     private void logProperties(Map<String, String> mm) {
         List<String> ll = new ArrayList<>();
         for (Object o : mm.keySet())
             ll.add((String) o);
         Collections.sort(ll);
-
         log.info("Properties:");
         for (String name : ll)
-            log.info("--- " + name + ": " + mm.get(name));
+            log.info("--- {}: {}", name, mm.get(name));
     }
 }

diff --git a/restapi-call-node/provider/src/test/java/jtest/org/onap/ccsdk/sli/plugins/restapicall/TestXmlParser.java b/restapi-call-node/provider/src/test/java/jtest/org/onap/ccsdk/sli/plugins/restapicall/TestXmlParser.java
index 544d259..e8567d5 100644 (file)
--- a/restapi-call-node/provider/src/test/java/jtest/org/onap/ccsdk/sli/plugins/restapicall/TestXmlParser.java
+++ b/restapi-call-node/provider/src/test/java/jtest/org/onap/ccsdk/sli/plugins/restapicall/TestXmlParser.java
@@ -44,10 +44,10 @@ public class TestXmlParser {
         BufferedReader in = new BufferedReader(
                 new InputStreamReader(ClassLoader.getSystemResourceAsStream("test3.xml"))
         );
-        String ss = "";
-        String line = null;
+        StringBuilder b = new StringBuilder();
+        String line;
         while ((line = in.readLine()) != null)
-            ss += line + '\n';
+            b.append(line).append('\n');
 
         Set<String> listNameList = new HashSet<String>();
         listNameList.add("project.dependencies.dependency");
@@ -57,10 +57,8 @@ public class TestXmlParser {
         listNameList.add("project.build.pluginManagement." +
                         "plugins.plugin.configuration.lifecycleMappingMetadata.pluginExecutions.pluginExecution");
 
-        Map<String, String> mm = XmlParser.convertToProperties(ss, listNameList);
-
+        Map<String, String> mm = XmlParser.convertToProperties(b.toString(), listNameList);
         logProperties(mm);
-
         in.close();
     }
 

diff --git a/restapi-call-node/provider/src/test/resources/test.json b/restapi-call-node/provider/src/test/resources/test.json
index a34f7e2..b48eb6b 100644 (file)
--- a/restapi-call-node/provider/src/test/resources/test.json
+++ b/restapi-call-node/provider/src/test/resources/test.json
@@ -27,7 +27,10 @@
             "number-primary-servers": "2",
             "equipment-id": "Server1",
             "server-model": "Unknown",
-            "server-id": "Server1"
+            "server-id": "Server1",
+            "test-node" : {
+                "test-inner-node" : "Test-Value"
+            }
         }
     ],
     "resource-state": {