From 8b7a0405a97cacf765c0e9a1988af98cd91a9f67 Mon Sep 17 00:00:00 2001 From: vempo Date: Sun, 28 Oct 2018 19:04:06 +0200 Subject: [PATCH] Handled not thread-safe fields in configuration 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 --- .../onap-configuration-management-core/pom.xml | 4 -- .../java/org/onap/config/ConfigurationUtils.java | 67 ++++++++-------------- .../java/org/onap/config/NonConfigResource.java | 5 +- .../onap/config/impl/AgglomerateConfiguration.java | 39 ------------- .../onap/config/impl/AggregateConfiguration.java | 8 +-- .../org/onap/config/impl/ConfigurationImpl.java | 24 ++++++-- .../onap/config/impl/ConfigurationRepository.java | 21 +++---- .../src/test/java/org/onap/config/TestCMSuite.java | 52 ----------------- 8 files changed, 62 insertions(+), 158 deletions(-) delete mode 100644 common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AgglomerateConfiguration.java delete mode 100755 common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/TestCMSuite.java diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/pom.xml b/common/onap-common-configuration-management/onap-configuration-management-core/pom.xml index c615270ead..09275a8f7e 100755 --- a/common/onap-common-configuration-management/onap-configuration-management-core/pom.xml +++ b/common/onap-common-configuration-management/onap-configuration-management-core/pom.xml @@ -125,12 +125,8 @@ ${mvn.surefire.version} - ${project.basedir}/src/test/resources ${user.home}/TestResources - - **/TestCMSuite.java - diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/ConfigurationUtils.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/ConfigurationUtils.java index dcdf17a992..ee1d7b10aa 100644 --- a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/ConfigurationUtils.java +++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/ConfigurationUtils.java @@ -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 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 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 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; 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 e7128c746d..752f202d3a 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 @@ -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 urls = new HashSet<>(); - private static final Set files = new HashSet<>(); + private static final Set urls = Collections.synchronizedSet(new HashSet<>()); + private static final Set 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 index ed2511d722..0000000000 --- a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AgglomerateConfiguration.java +++ /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 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; - } - -} diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AggregateConfiguration.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AggregateConfiguration.java index 32a1902151..e81d82bf7d 100644 --- a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AggregateConfiguration.java +++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/AggregateConfiguration.java @@ -35,10 +35,10 @@ import org.onap.config.type.ConfigurationMode; public final class AggregateConfiguration { - private final Map rootConfig = new HashMap<>(); - private final Map unionConfig = new HashMap<>(); - private final Map mergeConfig = new HashMap<>(); - private final Map overrideConfig = new LinkedHashMap<>(); + private final Map rootConfig = Collections.synchronizedMap(new HashMap<>()); + private final Map unionConfig = Collections.synchronizedMap(new HashMap<>()); + private final Map mergeConfig = Collections.synchronizedMap(new HashMap<>()); + private final Map overrideConfig = Collections.synchronizedMap(new LinkedHashMap<>()); public void addConfig(File file) throws Exception { addConfig(file.getAbsolutePath().toUpperCase(), ConfigurationUtils.getMergeStrategy(file), 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 af5ae04104..9a93801144 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 @@ -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 tenant = ThreadLocal.withInitial(() -> Constants.DEFAULT_TENANT); + + private static final ThreadLocal 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 moduleConfigStore = new HashMap<>(); List classpathResources = ConfigurationUtils.getAllClassPathResources(); Predicate 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 Map populateMap(String tenantId, String namespace, String key, Class 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(); } diff --git a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationRepository.java b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationRepository.java index 132043d5b5..5b950d1af6 100644 --- a/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationRepository.java +++ b/common/onap-common-configuration-management/onap-configuration-management-core/src/main/java/org/onap/config/impl/ConfigurationRepository.java @@ -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 tenants = new HashSet<>(); - private final Set namespaces = new HashSet<>(); - private final LinkedHashMap store = + private final Set tenants = Collections.synchronizedSet(new HashSet<>()); + + private final Set namespaces = Collections.synchronizedSet(new HashSet<>()); + + private final Map store = Collections.synchronizedMap( + new LinkedHashMap(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 index 6c460160d0..0000000000 --- a/common/onap-common-configuration-management/onap-configuration-management-core/src/test/java/org/onap/config/TestCMSuite.java +++ /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 - } -} -- 2.16.6