Fix unprocessed NPE

Process NPE safe way.

Add no NPE failures tests.

Add caught exceptions logging.

Issue-ID: SDC-2694
Signed-off-by: Dmitry Puzikov <d.puzikov2@partner.samsung.com>
Change-Id: Ic1d6e7f9b4f210ac0ca289dc6f44d7c6e15ef126

diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/CliConfigurationImpl.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/CliConfigurationImpl.java
index a2440b9..491382a 100644 (file)
--- a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/CliConfigurationImpl.java
+++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/CliConfigurationImpl.java
@@ -32,13 +32,17 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+
 import org.onap.config.ConfigurationUtils;
 import org.onap.config.api.ConfigurationManager;
 import org.onap.config.api.Hint;
 import org.onap.config.type.ConfigurationQuery;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public final class CliConfigurationImpl extends ConfigurationImpl implements ConfigurationManager {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(CliConfigurationImpl.class);
     private static final List<String> KEYS_TO_FILTER = Arrays.asList(NAMESPACE_KEY, MODE_KEY, LOAD_ORDER_KEY);
 
     public CliConfigurationImpl() {
@@ -50,55 +54,49 @@ public final class CliConfigurationImpl extends ConfigurationImpl implements Con
     }
 
     private String getConfigurationValue(ConfigurationQuery queryData) {
-
-        try {
-
-            Hint[] hints = getHints(queryData);
-
-            String[] value;
-            if (queryData.isFallback()) {
-                value = get(queryData.getTenant(), queryData.getNamespace(), queryData.getKey(), String[].class, hints);
-            } else {
-                value = getInternal(queryData.getTenant(), queryData.getNamespace(), queryData.getKey(), String[].class,
-                                hints);
+        if (queryData != null) {
+            try {
+                Hint[] hints = getHints(queryData);
+                String[] value;
+                if (queryData.isFallback()) {
+                    value = get(queryData.getTenant(), queryData.getNamespace(), queryData.getKey(), String[].class, hints);
+                } else {
+                    value = getInternal(queryData.getTenant(), queryData.getNamespace(), queryData.getKey(), String[].class,
+                            hints);
+                }
+                return ConfigurationUtils.getCommaSeparatedList(value);
+            } catch (Exception exception) {
+                LOGGER.warn("Error occurred while processing query: {}", queryData, exception);
             }
-
-            return ConfigurationUtils.getCommaSeparatedList(value);
-
-        } catch (Exception exception) {
-            exception.printStackTrace();
         }
-
         return null;
     }
 
     private Hint[] getHints(ConfigurationQuery query) {
         List<Hint> hints = new ArrayList<>(Hint.values().length);
-        hints.add(query.isLatest() ? Hint.LATEST_LOOKUP : Hint.DEFAULT);
-        hints.add(query.isExternalLookup() ? Hint.EXTERNAL_LOOKUP : Hint.DEFAULT);
-        hints.add(query.isNodeSpecific() ? Hint.NODE_SPECIFIC : Hint.DEFAULT);
+        if (query != null) {
+            hints.add(query.isLatest() ? Hint.LATEST_LOOKUP : Hint.DEFAULT);
+            hints.add(query.isExternalLookup() ? Hint.EXTERNAL_LOOKUP : Hint.DEFAULT);
+            hints.add(query.isNodeSpecific() ? Hint.NODE_SPECIFIC : Hint.DEFAULT);
+        }
         return hints.toArray(new Hint[0]);
     }
 
     private Object getInput(Map<String, Object> input) {
-
         Object toReturn = null;
-
-        try {
-
-            toReturn = Class.forName(input.get("ImplClass").toString()).newInstance();
-
-            Method[] methods = toReturn.getClass().getMethods();
-            for (Method method : methods) {
-                if (input.containsKey(method.getName())) {
-                    method.invoke(toReturn, input.get(method.getName()));
+        if (input != null) {
+            try {
+                toReturn = Class.forName(input.get("ImplClass").toString()).newInstance();
+                Method[] methods = toReturn.getClass().getMethods();
+                for (Method method : methods) {
+                    if (input.containsKey(method.getName())) {
+                        method.invoke(toReturn, input.get(method.getName()));
+                    }
                 }
+            } catch (Exception exception) {
+                LOGGER.warn("Error occurred while processing input: {}", input, exception);
             }
-
-        } catch (Exception exception) {
-            exception.printStackTrace();
         }
-
         return toReturn;
     }
 
@@ -107,43 +105,37 @@ public final class CliConfigurationImpl extends ConfigurationImpl implements Con
     }
 
     private Map<String, String> listConfiguration(ConfigurationQuery query) {
-
         Map<String, String> map = new HashMap<>();
-
-        try {
-
-            Collection<String> keys = getKeys(query.getTenant(), query.getNamespace());
-            for (String key : keys) {
-                map.put(key, getConfigurationValue(query.key(key)));
+        if (query != null) {
+            try {
+                Collection<String> keys = getKeys(query.getTenant(), query.getNamespace());
+                for (String key : keys) {
+                    map.put(key, getConfigurationValue(query.key(key)));
+                }
+            } catch (Exception exception) {
+                LOGGER.warn("Error occurred while processing query: {}", query, exception);
             }
-
-        } catch (Exception exception) {
-            exception.printStackTrace();
-            return null;
         }
-
         return map;
     }
 
     private ArrayList<String> getInMemoryKeys(String tenant, String namespace) {
-
         ArrayList<String> keys = new ArrayList<>();
-
         try {
-
             Iterator<String> iter = ConfigurationRepository.lookup().getConfigurationFor(tenant, namespace).getKeys();
             while (iter.hasNext()) {
-
                 String key = iter.next();
                 if (!KEYS_TO_FILTER.contains(key)) {
                     keys.add(key);
                 }
             }
-
         } catch (Exception exception) {
-            //do nothing
+            LOGGER.warn("Error occurred while searching for in-memory keys for namespace: '{}' and tenant: '{}'",
+                    namespace,
+                    tenant,
+                    exception
+            );
         }
-
         return keys;
     }
 

diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/type/ConfigurationQuery.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/type/ConfigurationQuery.java
index faefdb0..ce06572 100644 (file)
--- a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/type/ConfigurationQuery.java
+++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/type/ConfigurationQuery.java
@@ -95,4 +95,17 @@ public class ConfigurationQuery {
     public boolean isLatest() {
         return latest;
     }
+
+    @Override
+    public String toString() {
+        return "ConfigurationQuery{" +
+                "tenant='" + tenant + '\'' +
+                ", namespace='" + namespace + '\'' +
+                ", key='" + key + '\'' +
+                ", fallback=" + fallback +
+                ", externalLookup=" + externalLookup +
+                ", latest=" + latest +
+                ", nodeSpecific=" + nodeSpecific +
+                '}';
+    }
 }

diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/CliConfigurationImpTest.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/CliConfigurationImpTest.java
index 3d12eac..897583f 100644 (file)
--- a/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/CliConfigurationImpTest.java
+++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/CliConfigurationImpTest.java
@@ -19,8 +19,10 @@ package org.onap.config;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.util.HashMap;
 import java.util.Map;
 
+import org.junit.Assert;
 import org.junit.Test;
 import org.onap.config.api.ConfigurationManager;
 import org.onap.config.impl.CliConfigurationImpl;
@@ -51,6 +53,30 @@ public class CliConfigurationImpTest {
         assertEquals("org.junit.Test", testServiceImpl.getImplementationClass());
     }
 
+    @Test
+    public void listConfigurationNullQueryShouldntFailWithNPETest() {
+        ConfigurationManager conf = new CliConfigurationImpl();
+        Assert.assertEquals(0, conf.listConfiguration(null).size());
+    }
+
+    @Test
+    public void listConfigurationEmptyQueryTest() {
+        ConfigurationManager conf = new CliConfigurationImpl();
+        Assert.assertEquals(0, conf.listConfiguration(new HashMap<>()).size());
+    }
+
+    @Test
+    public void getConfigurationValueNullQueryShouldntFailWithNPETest() {
+        ConfigurationManager conf = new CliConfigurationImpl();
+        Assert.assertNull (conf.getConfigurationValue(null));
+    }
+
+    @Test
+    public void getConfigurationValueEmptyQueryTest() {
+        ConfigurationManager conf = new CliConfigurationImpl();
+        Assert.assertNull (conf.getConfigurationValue(new HashMap<>()));
+    }
+
     private void validateCliMapConfig(Map outputMap) {
         assertEquals("appc",
             outputMap.get(withoutArtifactPrefix(ConfigTestConstant.ARTIFACT_CONSUMER)));