From 8acb01dfb8a8fb2f6f77f154382933283b269566 Mon Sep 17 00:00:00 2001 From: Dmitry Puzikov Date: Thu, 28 Nov 2019 09:24:00 +0100 Subject: [PATCH] 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 Change-Id: Ic1d6e7f9b4f210ac0ca289dc6f44d7c6e15ef126 --- .../org/onap/config/impl/CliConfigurationImpl.java | 98 ++++++++++------------ .../org/onap/config/type/ConfigurationQuery.java | 13 +++ .../org/onap/config/CliConfigurationImpTest.java | 26 ++++++ 3 files changed, 84 insertions(+), 53 deletions(-) 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 a2440b9e94..491382ac5b 100644 --- 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 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 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 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 listConfiguration(ConfigurationQuery query) { - Map map = new HashMap<>(); - - try { - - Collection keys = getKeys(query.getTenant(), query.getNamespace()); - for (String key : keys) { - map.put(key, getConfigurationValue(query.key(key))); + if (query != null) { + try { + Collection 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 getInMemoryKeys(String tenant, String namespace) { - ArrayList keys = new ArrayList<>(); - try { - Iterator 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 faefdb0eb3..ce0657240a 100644 --- 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 3d12eac13d..897583f605 100644 --- 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))); -- 2.16.6