Handled not thread-safe fields in configuration 65/71365/2
authorvempo <vitaliy.emporopulo@amdocs.com>
Sun, 28 Oct 2018 17:04:06 +0000 (19:04 +0200)
committerVitaly Emporopulo <Vitaliy.Emporopulo@amdocs.com>
Sun, 28 Oct 2018 19:05:11 +0000 (19:05 +0000)
Replaced not thread-safe fields with synchornized versions,
removed duplicate code, deleted class that was accessing DB,
made surefire plugin to pick up all available unit tests
(instead of hand-picked).

Change-Id: Idff3ac333dc87ebfd3ecf50438ba0179556eb9c9
Issue-ID: SDC-1867
Signed-off-by: vempo <vitaliy.emporopulo@amdocs.com>
common/onap-common-configuration-management/onap-configuration-management-core/pom.xml
common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/ConfigurationUtils.java
common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/NonConfigResource.java
common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AgglomerateConfiguration.java [deleted file]
common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AggregateConfiguration.java
common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationImpl.java
common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationRepository.java
common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/TestCMSuite.java [deleted file]

index c615270..09275a8 100755 (executable)
                 <version>${mvn.surefire.version}</version>
                 <configuration>
                     <systemPropertyVariables>
-                        <config.location>${project.basedir}/src/test/resources</config.location>
                         <node.config.location>${user.home}/TestResources</node.config.location>
                     </systemPropertyVariables>
-                    <includes>
-                        <include>**/TestCMSuite.java</include>
-                    </includes>
                 </configuration>
             </plugin>
         </plugins>
index dcdf17a..ee1d7b1 100644 (file)
@@ -26,6 +26,7 @@ import java.io.File;
 import java.lang.reflect.Field;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -71,7 +72,6 @@ import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.commons.io.IOUtils;
 import org.onap.config.api.Config;
 import org.onap.config.api.ConfigurationManager;
-import org.onap.config.impl.AgglomerateConfiguration;
 import org.onap.config.impl.ConfigurationRepository;
 import org.onap.config.impl.YamlConfiguration;
 import org.onap.config.type.ConfigurationMode;
@@ -163,29 +163,36 @@ public class ConfigurationUtils {
     }
 
     public static Optional<FileBasedConfiguration> getConfiguration(URL url) {
-        FileBasedConfiguration builder = null;
+
         try {
+
             ConfigurationType configType = ConfigurationUtils.getConfigType(url);
             switch (configType) {
                 case PROPERTIES:
-                    builder = new Configurations().fileBased(PropertiesConfiguration.class, url);
-                    break;
+                    return Optional.of(new Configurations().fileBased(PropertiesConfiguration.class, url));
                 case XML:
-                    builder = new Configurations().fileBased(XMLConfiguration.class, url);
-                    break;
+                    return Optional.of(new Configurations().fileBased(XMLConfiguration.class, url));
                 case JSON:
-                    builder = new Configurations().fileBased(JsonConfiguration.class, url);
-                    break;
+                    return Optional.of(new Configurations().fileBased(JsonConfiguration.class, url));
                 case YAML:
-                    builder = new Configurations().fileBased(YamlConfiguration.class, url);
-                    break;
+                    return Optional.of(new Configurations().fileBased(YamlConfiguration.class, url));
                 default:
                     throw new ConfigurationException(CONFIGURATION_TYPE_NOT_SUPPORTED + configType);
             }
         } catch (ConfigurationException exception) {
             exception.printStackTrace();
         }
-        return ofNullable(builder);
+
+        return Optional.empty();
+    }
+
+    public static Optional<FileBasedConfiguration> getConfiguration(File file) {
+
+        try {
+            return getConfiguration(file.getAbsoluteFile().toURI().toURL());
+        } catch (MalformedURLException e) {
+            throw new IllegalStateException("Malformed URL: " + file.getAbsolutePath());
+        }
     }
 
     public static ConfigurationMode getMergeStrategy(String file) {
@@ -242,32 +249,6 @@ public class ConfigurationUtils {
         return configurationMode.orElseGet(() -> getMergeStrategy(file.getName().toUpperCase()));
     }
 
-    public static Optional<FileBasedConfiguration> getConfiguration(File file) {
-        FileBasedConfiguration builder = null;
-        try {
-            ConfigurationType configType = ConfigurationUtils.getConfigType(file);
-            switch (configType) {
-                case PROPERTIES:
-                    builder = new Configurations().fileBased(PropertiesConfiguration.class, file);
-                    break;
-                case XML:
-                    builder = new Configurations().fileBased(XMLConfiguration.class, file);
-                    break;
-                case JSON:
-                    builder = new Configurations().fileBased(JsonConfiguration.class, file);
-                    break;
-                case YAML:
-                    builder = new Configurations().fileBased(YamlConfiguration.class, file);
-                    break;
-                default:
-                    throw new ConfigurationException(CONFIGURATION_TYPE_NOT_SUPPORTED + configType);
-            }
-        } catch (ConfigurationException exception) {
-            exception.printStackTrace();
-        }
-        return ofNullable(builder);
-    }
-
     public static ConfigurationType getConfigType(File file) {
         return Enum.valueOf(ConfigurationType.class,
                 file.getAbsolutePath().substring(file.getAbsolutePath().lastIndexOf('.') + 1).toUpperCase());
@@ -651,15 +632,15 @@ public class ConfigurationUtils {
     }
 
     public static Object getProperty(Configuration config, String key, int processingHints) {
+
         if (!isDirectLookup(processingHints)) {
-            if (config instanceof AgglomerateConfiguration) {
-                return ((AgglomerateConfiguration) config).getPropertyValue(key);
-            } else if (config instanceof CompositeConfiguration) {
+
+            if (config instanceof CompositeConfiguration) {
+
                 CompositeConfiguration conf = (CompositeConfiguration) config;
                 for (int i = 0; i < conf.getNumberOfConfigurations(); i++) {
-                    if (conf.getConfiguration(i) instanceof AgglomerateConfiguration) {
-                        return ((AgglomerateConfiguration) conf.getConfiguration(i)).getPropertyValue(key);
-                    } else if (isNodeSpecific(processingHints)) {
+
+                    if (isNodeSpecific(processingHints)) {
                         Object obj = conf.getConfiguration(i).getProperty(key);
                         if (obj != null) {
                             return obj;
index e7128c7..752f202 100644 (file)
@@ -21,14 +21,15 @@ import java.net.URL;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.function.Predicate;
 
 public class NonConfigResource {
 
-    private static final Set<URL> urls = new HashSet<>();
-    private static final Set<File> files = new HashSet<>();
+    private static final Set<URL> urls = Collections.synchronizedSet(new HashSet<>());
+    private static final Set<File> files = Collections.synchronizedSet(new HashSet<>());
 
     public static void add(URL url) {
         urls.add(url);
diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AgglomerateConfiguration.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AgglomerateConfiguration.java
deleted file mode 100644 (file)
index ed2511d..0000000
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Copyright © 2016-2018 European Support Limited
- *
- * 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.impl;
-
-import java.util.Collections;
-import java.util.Map;
-import java.util.WeakHashMap;
-import org.apache.commons.configuration2.DatabaseConfiguration;
-
-public class AgglomerateConfiguration extends DatabaseConfiguration {
-
-    private final Map<String, Object> store = Collections.synchronizedMap(new WeakHashMap<>());
-
-    public Object getPropertyValue(String key) {
-
-        Object objToReturn = store.get(key);
-        if (objToReturn == null && !store.containsKey(key)) {
-            objToReturn = super.getProperty(key);
-            store.put(key, objToReturn);
-        }
-
-        return objToReturn;
-    }
-
-}
index 32a1902..e81d82b 100644 (file)
@@ -35,10 +35,10 @@ import org.onap.config.type.ConfigurationMode;
 
 public final class AggregateConfiguration {
 
-    private final Map<String, Configuration> rootConfig = new HashMap<>();
-    private final Map<String, Configuration> unionConfig = new HashMap<>();
-    private final Map<String, Configuration> mergeConfig = new HashMap<>();
-    private final Map<String, Configuration> overrideConfig = new LinkedHashMap<>();
+    private final Map<String, Configuration> rootConfig = Collections.synchronizedMap(new HashMap<>());
+    private final Map<String, Configuration> unionConfig = Collections.synchronizedMap(new HashMap<>());
+    private final Map<String, Configuration> mergeConfig = Collections.synchronizedMap(new HashMap<>());
+    private final Map<String, Configuration> overrideConfig = Collections.synchronizedMap(new LinkedHashMap<>());
 
     public void addConfig(File file) throws Exception {
         addConfig(file.getAbsolutePath().toUpperCase(), ConfigurationUtils.getMergeStrategy(file),
index af5ae04..9a93801 100644 (file)
@@ -39,14 +39,26 @@ import org.onap.config.api.Hint;
 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 ThreadLocal<String> tenant = ThreadLocal.withInitial(() -> Constants.DEFAULT_TENANT);
+
+    private static final ThreadLocal<String> TENANT = ThreadLocal.withInitial(() -> Constants.DEFAULT_TENANT);
+
+    private static final Object LOCK = new Object();
 
     private static boolean instantiated = false;
 
     public ConfigurationImpl() throws Exception {
+
+        synchronized (LOCK) {
+            init();
+        }
+    }
+
+    private void init() throws Exception {
+
         if (instantiated || !CliConfigurationImpl.class.isAssignableFrom(this.getClass())) {
             throw new RuntimeException("Illegal access to configuration.");
         }
+
         Map<String, AggregateConfiguration> moduleConfigStore = new HashMap<>();
         List<URL> classpathResources = ConfigurationUtils.getAllClassPathResources();
         Predicate<URL> predicate = ConfigurationUtils::isConfig;
@@ -107,6 +119,7 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
                 }
             }
         }
+
         populateFinalConfigurationIncrementally(moduleConfigStore);
         String nodeConfigLocation = System.getProperty("node.config.location");
         if (nodeConfigLocation != null && nodeConfigLocation.trim().length() > 0) {
@@ -122,6 +135,7 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
                 }
             }
         }
+
         instantiated = true;
     }
 
@@ -186,8 +200,9 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
 
     @Override
     public <T> Map<String, T> populateMap(String tenantId, String namespace, String key, Class<T> clazz) {
+
         if (tenantId == null || tenantId.trim().length() == 0) {
-            tenantId = tenant.get();
+            tenantId = TENANT.get();
         } else {
             tenantId = tenantId.toUpperCase();
         }
@@ -218,8 +233,9 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
 
     @Override
     public Map generateMap(String tenantId, String namespace, String key) {
+
         if (tenantId == null || tenantId.trim().length() == 0) {
-            tenantId = tenant.get();
+            tenantId = TENANT.get();
         } else {
             tenantId = tenantId.toUpperCase();
         }
@@ -277,7 +293,7 @@ public class ConfigurationImpl implements org.onap.config.api.Configuration {
         }
 
         if (tenant == null || tenant.trim().length() == 0) {
-            tenant = ConfigurationImpl.tenant.get();
+            tenant = TENANT.get();
         } else {
             tenant = tenant.toUpperCase();
         }
index 132043d..5b950d1 100644 (file)
@@ -20,6 +20,7 @@ import java.io.File;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -38,10 +39,14 @@ public final class ConfigurationRepository {
 
     private static final ConfigurationRepository repo = new ConfigurationRepository();
 
-    private final Set<String> tenants = new HashSet<>();
-    private final Set<String> namespaces = new HashSet<>();
-    private final LinkedHashMap<String, ConfigurationHolder> store =
+    private final Set<String> tenants = Collections.synchronizedSet(new HashSet<>());
+
+    private final Set<String> namespaces = Collections.synchronizedSet(new HashSet<>());
+
+    private final Map<String, ConfigurationHolder> store = Collections.synchronizedMap(
+
             new LinkedHashMap<String, ConfigurationHolder>(16, 0.75f, true) {
+
                 @Override
                 protected boolean removeEldestEntry(Map.Entry eldest) {
                     try {
@@ -51,7 +56,8 @@ public final class ConfigurationRepository {
                         return false;
                     }
                 }
-            };
+            });
+
 
     private ConfigurationRepository() {
         tenants.add(Constants.DEFAULT_TENANT);
@@ -80,13 +86,8 @@ public final class ConfigurationRepository {
     }
 
     public Configuration getConfigurationFor(String tenant, String namespace) throws Exception {
-        ConfigurationHolder config;
         String module = tenant + Constants.KEY_ELEMENTS_DELIMITER + namespace;
-        config = store.get(module);
-        if (config == null) {
-            config = new ConfigurationHolder(new BasicConfigurationBuilder<>(AgglomerateConfiguration.class));
-            store.put(module, config);
-        }
+        ConfigurationHolder config = store.get(module);
         return config.getConfiguration(tenant + Constants.KEY_ELEMENTS_DELIMITER + namespace);
     }
 
diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/TestCMSuite.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/TestCMSuite.java
deleted file mode 100755 (executable)
index 6c46016..0000000
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * Copyright © 2016-2018 European Support Limited
- *
- * 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 org.junit.runner.RunWith;
-import org.junit.runners.Suite;
-import org.onap.config.test.CliFallbackAndLookupTest;
-import org.onap.config.test.CliTest;
-import org.onap.config.test.ConfigSourceLocationTest;
-import org.onap.config.test.FallbackConfigTest;
-import org.onap.config.test.FallbackToGlobalNamespaceTest;
-import org.onap.config.test.GlobalAndNamespaceConfigTest;
-import org.onap.config.test.JavaPropertiesConfigTest;
-import org.onap.config.test.JsonConfigTest;
-import org.onap.config.test.LoadOrderMergeAndOverrideTest;
-import org.onap.config.test.ModeAsConfigPropTest;
-import org.onap.config.test.MultiTenancyConfigTest;
-import org.onap.config.test.NodeSpecificCliTest;
-import org.onap.config.test.ValidateDefaultModeTest;
-import org.onap.config.test.XmlConfigTest;
-import org.onap.config.test.YamlConfigTest;
-
-/**
- * Created by sheetalm on 10/25/2016.
- */
-@RunWith(Suite.class)
-@Suite.SuiteClasses(
-        {ConfigurationUtilsTest.class, JavaPropertiesConfigTest.class, JsonConfigTest.class, XmlConfigTest.class,
-                YamlConfigTest.class, CliFallbackAndLookupTest.class, CliTest.class, ConfigSourceLocationTest.class,
-                FallbackConfigTest.class, FallbackToGlobalNamespaceTest.class, GlobalAndNamespaceConfigTest.class,
-                ModeAsConfigPropTest.class, MultiTenancyConfigTest.class, NodeSpecificCliTest.class,
-                ValidateDefaultModeTest.class, LoadOrderMergeAndOverrideTest.class})
-public class TestCMSuite extends junit.framework.TestSuite {
-
-    private TestCMSuite() {
-        // prevent instantiation
-    }
-}