From 02e1594ffee2231523d0486a5d8b590ff09581df Mon Sep 17 00:00:00 2001 From: Gaurav Agrawal Date: Tue, 19 Sep 2017 17:40:54 +0530 Subject: 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 --- .../ccsdk/sli/plugins/restapicall/JsonParser.java | 85 ++++++++++++---------- .../ccsdk/sli/plugins/restapicall/XmlJsonUtil.java | 14 ++-- .../ccsdk/sli/plugins/restapicall/XmlParser.java | 62 +++++++++------- .../sli/plugins/restapicall/TestJsonParser.java | 20 +++-- .../sli/plugins/restapicall/TestXmlParser.java | 10 +-- .../provider/src/test/resources/test.json | 5 +- 6 files changed, 114 insertions(+), 82 deletions(-) (limited to 'restapi-call-node/provider/src') 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 f2867f5a3..4a1dfef50 100644 --- 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 convertToProperties(String s) throws JSONException { - JSONObject json = new JSONObject(s); - - Map wm = new HashMap<>(); - Iterator ii = json.keys(); - while (ii.hasNext()) { - String key1 = ii.next(); - wm.put(key1, json.get(key1)); - } - - Map 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 convertToProperties(String s) + throws SvcLogicException { + + checkNotNull(s, "Input should not be null."); + + try { + JSONObject json = new JSONObject(s); + Map wm = new HashMap<>(); + Iterator 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 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 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 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 e80ca5081..b94f0a638 100644 --- 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 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 61b0e31d4..7ef776da1 100644 --- 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 convertToProperties(String s, Set listNameList) { + private XmlParser() { + // Preventing instantiation of the same. + } + + public static Map convertToProperties(String s, Set 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 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 dbca5ad7b..5526be81f 100644 --- 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 mm = JsonParser.convertToProperties(ss); + Map 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 mm) { List 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 544d259e0..e8567d591 100644 --- 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 listNameList = new HashSet(); listNameList.add("project.dependencies.dependency"); @@ -57,10 +57,8 @@ public class TestXmlParser { listNameList.add("project.build.pluginManagement." + "plugins.plugin.configuration.lifecycleMappingMetadata.pluginExecutions.pluginExecution"); - Map mm = XmlParser.convertToProperties(ss, listNameList); - + Map 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 a34f7e2a0..b48eb6b43 100644 --- 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": { -- cgit 1.2.3-korg