Fixed Sonar violations in configuration

Simplified code, added unit tests.

Change-Id: I0a35daf8a2d4cfd0979e30363c9fad30e7c0570b
Issue-ID: SDC-1867
Signed-off-by: vempo <vitaliy.emporopulo@amdocs.com>

3 files changed, 209 insertions, 64 deletions

diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/NonConfigResource.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/NonConfigResource.java
index 752f202d3a..459c8b476b 100644
--- a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/NonConfigResource.java
+++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/NonConfigResource.java
@@ -17,77 +17,129 @@
 package org.onap.config;
 
 import java.io.File;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Collection;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
 import java.util.Set;
-import java.util.function.Predicate;
+import java.util.function.Function;
 
 public class NonConfigResource {
 
-    private static final Set<URL> urls = Collections.synchronizedSet(new HashSet<>());
-    private static final Set<File> files = Collections.synchronizedSet(new HashSet<>());
+    static final String NODE_CONFIG_LOCATION = "node.config.location";
+    static final String CONFIG_LOCATION = "config.location";
 
-    public static void add(URL url) {
+    private final List<Function<String, Path>> lookupFunctions =
+            Arrays.asList(this::getFromFile, this::findInFiles, this::getForNode, this::getGlobal, this::findInUrls);
+
+    private final Set<URL> urls = Collections.synchronizedSet(new HashSet<>());
+    private final Set<File> files = Collections.synchronizedSet(new HashSet<>());
+
+    private final Function<String, String> propertyGetter;
+
+    NonConfigResource(Function<String, String> propertyGetter) {
+        this.propertyGetter = propertyGetter;
+    }
+
+    public NonConfigResource() {
+        this(System::getProperty);
+    }
+
+    public void add(URL url) {
         urls.add(url);
     }
 
-    public static Path locate(String resource) {
+    public void add(File file) {
+        files.add(file);
+    }
+
+    public Path locate(String resource) {
+
+        if (resource == null) {
+            return null;
+        }
+
         try {
-            if (resource != null) {
-                File file = new File(resource);
-                if (file.exists()) {
-                    return Paths.get(resource);
-                }
-                for (File availableFile : files) {
-                    if (availableFile.getAbsolutePath().endsWith(resource) && availableFile.exists()) {
-                        return Paths.get(availableFile.getAbsolutePath());
-                    }
-                }
-                if (System.getProperty("node.config.location") != null) {
-                    Path path = locate(new File(System.getProperty("node.config.location")), resource);
-                    if (path != null) {
-                        return path;
-                    }
-                }
-                if (System.getProperty("config.location") != null) {
-                    Path path = locate(new File(System.getProperty("config.location")), resource);
-                    if (path != null) {
-                        return path;
-                    }
-                }
-                for (URL url : urls) {
-                    if (url.getFile().endsWith(resource)) {
-                        return Paths.get(url.toURI());
-                    }
-                }
-            }
+
+            return lookupFunctions.stream()
+                           .map(f -> f.apply(resource))
+                           .filter(Objects::nonNull)
+                           .findFirst().orElse(null);
+
         } catch (Exception exception) {
             exception.printStackTrace();
+            return null;
         }
+    }
+
+    private Path locate(File root, String resource) {
+
+        if (!root.exists()) {
+            return null;
+        }
+
+        return ConfigurationUtils.getAllFiles(root, true, false)
+                       .stream()
+                       .filter(f -> !ConfigurationUtils.isConfig(f))
+                       .peek(this::add).filter(f -> f.getAbsolutePath().endsWith(resource))
+                       .findFirst()
+                       .map(file -> Paths.get(file.getAbsolutePath())).orElse(null);
+    }
+
+    private Path getFromFile(String resource) {
+        return new File(resource).exists() ? Paths.get(resource) : null;
+    }
+
+    private Path findInUrls(String resource) {
+
+        for (URL url : urls) {
+
+            if (url.getFile().endsWith(resource)) {
+                return Paths.get(toUri(url));
+            }
+        }
+
         return null;
     }
 
-    private static Path locate(File root, String resource) {
-        if (root.exists()) {
-            Collection<File> filesystemResources = ConfigurationUtils.getAllFiles(root, true, false);
-            Predicate<File> f1 = ConfigurationUtils::isConfig;
-            for (File file : filesystemResources) {
-                if (!f1.test(file)) {
-                    add(file);
-                    if (file.getAbsolutePath().endsWith(resource)) {
-                        return Paths.get(file.getAbsolutePath());
-                    }
-                }
+    private Path findInFiles(String resource) {
+
+        for (File availableFile : files) {
+
+            String absolutePath = availableFile.getAbsolutePath();
+            if (absolutePath.endsWith(resource) && availableFile.exists()) {
+                return Paths.get(absolutePath);
             }
         }
+
         return null;
     }
 
-    public static void add(File file) {
-        files.add(file);
+    private Path getForNode(String resource) {
+        return getFromProperty(NODE_CONFIG_LOCATION, resource);
+    }
+
+    private Path getGlobal(String resource) {
+        return getFromProperty(CONFIG_LOCATION, resource);
+    }
+
+    private Path getFromProperty(String property, String resource) {
+        String value = propertyGetter.apply(property);
+        return (value == null) ? null : locate(new File(value), resource);
+    }
+
+    private static URI toUri(URL url) {
+
+        try {
+            return url.toURI();
+        } catch (URISyntaxException e) {
+            throw new IllegalStateException("Unexpected URL syntax: " + url.toString(), e);
+        }
     }
 }
diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationImpl.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationImpl.java
index 8f8a39d82c..c51c8621a9 100644
--- a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationImpl.java
+++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationImpl.java
@@ -21,6 +21,7 @@ import static org.onap.config.ConfigurationUtils.isBlank;
 import java.io.File;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Modifier;
 import java.net.URL;
 import java.util.ArrayList;
@@ -32,6 +33,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Predicate;
+import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.onap.config.ConfigurationUtils;
 import org.onap.config.Constants;
 import org.onap.config.NonConfigResource;
@@ -42,22 +44,18 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
 
     private static final String KEY_CANNOT_BE_NULL = "Key can't be null.";
 
-    private static final Object LOCK = new Object();
+    private static final NonConfigResource nonConfigResource = new NonConfigResource();
 
-    private static boolean instantiated = false;
+    static {
 
-    public ConfigurationImpl() throws Exception {
-
-        synchronized (LOCK) {
+        try {
             init();
+        } catch (ConfigurationException e) {
+            throw new IllegalStateException("Failed to initialize configuration");
         }
     }
 
-    private void init() throws Exception {
-
-        if (instantiated) {
-            return;
-        }
+    private static void init() throws ConfigurationException {
 
         Map<String, AggregateConfiguration> moduleConfigStore = new HashMap<>();
         List<URL> classpathResources = ConfigurationUtils.getAllClassPathResources();
@@ -72,7 +70,7 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
                 }
                 moduleConfig.addConfig(url);
             } else {
-                NonConfigResource.add(url);
+                nonConfigResource.add(url);
             }
         }
         String configLocation = System.getProperty("config.location");
@@ -90,7 +88,7 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
                     }
                     moduleConfig.addConfig(file);
                 } else {
-                    NonConfigResource.add(file);
+                    nonConfigResource.add(file);
                 }
             }
         }
@@ -135,11 +133,9 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
                 }
             }
         }
-
-        instantiated = true;
     }
 
-    private void populateFinalConfigurationIncrementally(Map<String, AggregateConfiguration> configs) {
+    private static void populateFinalConfigurationIncrementally(Map<String, AggregateConfiguration> configs) {
 
         if (configs.get(Constants.DEFAULT_TENANT + Constants.KEY_ELEMENTS_DELIMITER + Constants.DB_NAMESPACE) != null) {
             ConfigurationRepository.lookup().populateConfiguration(
@@ -362,7 +358,8 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
     }
 
     private <T> T read(String tenant, String namespace, Class<T> clazz, String keyPrefix, Hint... hints)
-            throws Exception {
+            throws NoSuchMethodException, IllegalAccessException, InvocationTargetException, InstantiationException {
+
         Config confAnnotation = clazz.getAnnotation(Config.class);
         if (confAnnotation != null && confAnnotation.key().length() > 0 && !keyPrefix.endsWith(".")) {
             keyPrefix += (confAnnotation.key() + ".");
@@ -455,7 +452,7 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
         if (String.class.equals(clazz)) {
             if (obj.toString().startsWith("@") && ConfigurationUtils.isExternalLookup(processingHint)) {
                 String contents = ConfigurationUtils.getFileContents(
-                        NonConfigResource.locate(obj.toString().substring(1).trim()));
+                        nonConfigResource.locate(obj.toString().substring(1).trim()));
                 if (contents == null) {
                     contents = ConfigurationUtils.getFileContents(obj.toString().substring(1).trim());
                 }
diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/NonConfigResourceTest.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/NonConfigResourceTest.java
new file mode 100644
index 0000000000..7566a29749
--- /dev/null
+++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/NonConfigResourceTest.java
@@ -0,0 +1,96 @@
+/*
+ * Copyright © 2018 Nokia
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onap.config;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.onap.config.NonConfigResource.CONFIG_LOCATION;
+import static org.onap.config.NonConfigResource.NODE_CONFIG_LOCATION;
+
+import java.io.File;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.Map;
+import org.junit.Test;
+
+public class NonConfigResourceTest {
+
+    private static final String RESOURCE_NAME = NonConfigResourceTest.class.getSimpleName() + ".class";
+    private final URL sampleUrlResource = NonConfigResourceTest.class.getResource(RESOURCE_NAME);
+    private final String sampleResourcePath = sampleUrlResource.getPath();
+    private final File sampleResourceFile = new File(sampleResourcePath);
+
+    @Test
+    public void testShouldLocateResourceWhenAbsPathProvided2() {
+        Map<String, String> properties = Collections.emptyMap();
+        Path actualResourcePath = new NonConfigResource(properties::get).locate(sampleResourceFile.toString());
+        assertEquals(0, actualResourcePath.compareTo(sampleResourceFile.toPath()));
+    }
+
+    @Test
+    public void testShouldLocateResourceWhenPresentInFiles2() {
+        Map<String, String> properties = Collections.emptyMap();
+        NonConfigResource testedObject = new NonConfigResource(properties::get);
+        testedObject.add(sampleResourceFile);
+        Path thisFilePath = testedObject.locate(RESOURCE_NAME);
+        assertEquals(0, thisFilePath.compareTo(sampleResourceFile.toPath()));
+    }
+
+    @Test
+    public void testShouldLocateResourceWhenNodeConfigPropertyIsSet2() {
+
+        Map<String, String> properties = Collections.singletonMap(
+                NODE_CONFIG_LOCATION, new File(sampleResourcePath).getParentFile().getPath());
+
+        NonConfigResource testedNonConfigResource = new NonConfigResource(properties::get);
+        System.getProperties().setProperty(NODE_CONFIG_LOCATION,
+                new File(sampleResourcePath).getParentFile().getPath());
+        Path thisFilePath = testedNonConfigResource.locate(RESOURCE_NAME);
+        assertEquals(0, thisFilePath.compareTo(new File(sampleResourcePath).toPath()));
+    }
+
+    @Test
+    public void testShouldLocateResourceWhenConfigPropertyIsSet2() {
+
+        Map<String, String> properties = Collections.singletonMap(
+                CONFIG_LOCATION, new File(sampleResourcePath).getParentFile().getPath());
+
+        NonConfigResource testedNonConfigResource = new NonConfigResource(properties::get);
+        Path thisFilePath = testedNonConfigResource.locate(RESOURCE_NAME);
+        assertEquals(0, thisFilePath.compareTo(new File(sampleResourcePath).toPath()));
+    }
+
+    @Test
+    public void testShouldLocatePathWhenResourcePresentInUrls2() throws URISyntaxException {
+        Map<String, String> properties = Collections.emptyMap();
+        NonConfigResource testedObject = new NonConfigResource(properties::get);
+        testedObject.add(sampleUrlResource);
+        Path thisFilePath = testedObject.locate(RESOURCE_NAME);
+        assertEquals(0, thisFilePath.compareTo(Paths.get(sampleUrlResource.toURI())));
+    }
+
+    @Test
+    public void testShouldNotLocateResource2() {
+        Map<String, String> properties = Collections.emptyMap();
+        NonConfigResource testedObject = new NonConfigResource(properties::get);
+        Path thisFilePath = testedObject.locate("nonexistingresource");
+        assertNull(thisFilePath);
+    }
+}
\ No newline at end of file