From b118cd814ea0a230486c4179d0bcdc27d39d92b1 Mon Sep 17 00:00:00 2001 From: vempo Date: Sun, 22 Oct 2017 19:03:46 +0300 Subject: Additinal fixes for resources not being released More fixes for input streams not being properly closed, with unit tests and other minor improvements (code cleanup and coding conventions). Change-Id: I6751f924a1469d49b996e4f1d6c61371af6714b1 Issue-ID: SDC-291 Signed-off-by: vempo --- .../core/nosqldb/util/ConfigurationManager.java | 77 ++++++++++++---------- .../nosqldb/util/ConfigurationManagerTest.java | 73 ++++++++++++++++++++ .../openecomp/core/utilities/file/FileUtils.java | 12 +++- .../core/utilities/file/FileUtilsTest.java | 48 ++++++++++++++ .../core/utilities/file/test-resource.txt | 1 + .../services/heattotosca/HeatToToscaUtil.java | 9 ++- .../sdc/translator/utils/ResourceWalker.java | 3 +- .../sdc/validation/util/ValidationTestUtil.java | 13 ++-- 8 files changed, 187 insertions(+), 49 deletions(-) create mode 100644 openecomp-be/lib/openecomp-core-lib/openecomp-nosqldb-lib/openecomp-nosqldb-core/src/test/java/org/openecomp/core/nosqldb/util/ConfigurationManagerTest.java create mode 100644 openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/test/java/org/openecomp/core/utilities/file/FileUtilsTest.java create mode 100644 openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/test/resources/org/openecomp/core/utilities/file/test-resource.txt (limited to 'openecomp-be') diff --git a/openecomp-be/lib/openecomp-core-lib/openecomp-nosqldb-lib/openecomp-nosqldb-core/src/main/java/org/openecomp/core/nosqldb/util/ConfigurationManager.java b/openecomp-be/lib/openecomp-core-lib/openecomp-nosqldb-lib/openecomp-nosqldb-core/src/main/java/org/openecomp/core/nosqldb/util/ConfigurationManager.java index 1f5e20fd0b..bde9d06ae4 100644 --- a/openecomp-be/lib/openecomp-core-lib/openecomp-nosqldb-lib/openecomp-nosqldb-core/src/main/java/org/openecomp/core/nosqldb/util/ConfigurationManager.java +++ b/openecomp-be/lib/openecomp-core-lib/openecomp-nosqldb-lib/openecomp-nosqldb-core/src/main/java/org/openecomp/core/nosqldb/util/ConfigurationManager.java @@ -20,25 +20,28 @@ package org.openecomp.core.nosqldb.util; +import org.openecomp.core.utilities.file.FileUtils; import org.openecomp.sdc.logging.api.Logger; import org.openecomp.sdc.logging.api.LoggerFactory; import org.openecomp.sdc.tosca.services.YamlUtil; import java.io.FileInputStream; -import java.io.FileNotFoundException; +import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.function.Function; /** * The type Configuration manager. */ public class ConfigurationManager { - private static final String CONFIGURATION_YAML_FILE = "configuration.yaml"; - private static final String cassandraKey = "cassandraConfig"; + static final String CONFIGURATION_YAML_FILE = "configuration.yaml"; + + private static final String CASSANDRA_KEY = "cassandraConfig"; private static final String DEFAULT_KEYSPACE_NAME = "dox"; private static final String CASSANDRA_ADDRESSES = "cassandra.addresses"; private static final String CASSANDRA_DOX_KEY_STORE = "cassandra.dox.keystore"; @@ -49,14 +52,14 @@ public class ConfigurationManager { private static final String CASSANDRA_SSL = "cassandra.ssl"; private static final String CASSANDRA_TRUSTSTORE = "cassandra.Truststore"; private static final String CASSANDRA_TRUSTSTORE_PASSWORD = "cassandra.TruststorePassword"; - private static final String cassandraHostsKey = "cassandraHosts"; - private static final String cassandraPortKey = "port"; - private static final String cassandraUsernameKey = "username"; - private static final String cassandraPasswordKey = "password"; - private static final String cassandraAuthenticateKey = "authenticate"; - private static final String cassandraSSLKey = "ssl"; - private static final String cassandraTruststorePathKey = "truststorePath"; - private static final String cassandraTruststorePasswordKey = "truststorePassword"; + private static final String CASSANDRA_HOSTS_KEY = "cassandraHosts"; + private static final String CASSANDRA_PORT_KEY = "port"; + private static final String CASSANDRA_USERNAME_KEY = "username"; + private static final String CASSANDRA_PASSWORD_KEY = "password"; + private static final String CASSANDRA_AUTHENTICATE_KEY = "authenticate"; + private static final String CASSANDRA_SSL_KEY = "ssl"; + private static final String CASSANDRA_TRUSTSTORE_PATH_KEY = "truststorePath"; + private static final String CASSANDRA_TRUSTSTORE_PASSWORD_KEY = "truststorePassword"; private static ConfigurationManager instance = null; private final LinkedHashMap cassandraConfiguration; @@ -64,18 +67,24 @@ public class ConfigurationManager { private ConfigurationManager() { - YamlUtil yamlUtil = new YamlUtil(); + String configurationYamlFile = System.getProperty(CONFIGURATION_YAML_FILE); - InputStream yamlAsString; - if (configurationYamlFile != null) { - yamlAsString = getConfigFileIs(configurationYamlFile); - } else { - //Load from resources - yamlAsString = yamlUtil.loadYamlFileIs("/" + CONFIGURATION_YAML_FILE); - } - Map> configurationMap = yamlUtil.yamlToMap(yamlAsString); - cassandraConfiguration = configurationMap.get(cassandraKey); + Function>> reader = (is) -> { + YamlUtil yamlUtil = new YamlUtil(); + return yamlUtil.yamlToMap(is); + }; + + try { + + Map> configurationMap = configurationYamlFile != null + ? readFromFile(configurationYamlFile, reader) // load from file + : FileUtils.readViaInputStream(CONFIGURATION_YAML_FILE, reader); // or from resource + cassandraConfiguration = configurationMap.get(CASSANDRA_KEY); + + } catch (IOException e) { + throw new RuntimeException("Failed to read configuration", e); + } } /** @@ -101,7 +110,7 @@ public class ConfigurationManager { if (addresses != null) { return addresses.split(","); } - List lsAddresses = (ArrayList) cassandraConfiguration.get(cassandraHostsKey); + List lsAddresses = (ArrayList) cassandraConfiguration.get(CASSANDRA_HOSTS_KEY); String[] addressesArray; addressesArray = (String[]) lsAddresses.toArray(new String[lsAddresses.size()]); return addressesArray; @@ -131,7 +140,7 @@ public class ConfigurationManager { public String getUsername() { String username = System.getProperty(CASSANDRA_USER); if (username == null) { - username = (String) cassandraConfiguration.get(cassandraUsernameKey); + username = (String) cassandraConfiguration.get(CASSANDRA_USERNAME_KEY); } return username; } @@ -144,7 +153,7 @@ public class ConfigurationManager { public String getPassword() { String password = System.getProperty(CASSANDRA_PASSWORD); if (password == null) { - password = (String) cassandraConfiguration.get(cassandraPasswordKey); + password = (String) cassandraConfiguration.get(CASSANDRA_PASSWORD_KEY); } return password; } @@ -157,7 +166,7 @@ public class ConfigurationManager { public String getTruststorePath() { String truststorePath = System.getProperty(CASSANDRA_TRUSTSTORE); if (truststorePath == null) { - truststorePath = (String) cassandraConfiguration.get(cassandraTruststorePathKey); + truststorePath = (String) cassandraConfiguration.get(CASSANDRA_TRUSTSTORE_PATH_KEY); } return truststorePath; } @@ -170,7 +179,7 @@ public class ConfigurationManager { public String getTruststorePassword() { String truststorePassword = System.getProperty(CASSANDRA_TRUSTSTORE_PASSWORD); if (truststorePassword == null) { - truststorePassword = (String) cassandraConfiguration.get(cassandraTruststorePasswordKey); + truststorePassword = (String) cassandraConfiguration.get(CASSANDRA_TRUSTSTORE_PASSWORD_KEY); } return truststorePassword; } @@ -184,7 +193,7 @@ public class ConfigurationManager { int port; String sslPort = System.getProperty(CASSANDRA_PORT); if (sslPort == null) { - sslPort = (String) cassandraConfiguration.get(cassandraPortKey); + sslPort = (String) cassandraConfiguration.get(CASSANDRA_PORT_KEY); if (sslPort == null) { sslPort = "0"; } @@ -200,7 +209,7 @@ public class ConfigurationManager { * @return the boolean */ public boolean isSsl() { - return getBooleanResult(CASSANDRA_SSL, cassandraSSLKey); + return getBooleanResult(CASSANDRA_SSL, CASSANDRA_SSL_KEY); } /** @@ -209,7 +218,7 @@ public class ConfigurationManager { * @return the boolean */ public boolean isAuthenticate() { - return getBooleanResult(CASSANDRA_AUTHENTICATE, cassandraAuthenticateKey); + return getBooleanResult(CASSANDRA_AUTHENTICATE, CASSANDRA_AUTHENTICATE_KEY); } private Boolean getBooleanResult(String property, String key) { @@ -226,13 +235,9 @@ public class ConfigurationManager { return res; } - private InputStream getConfigFileIs(String file) { - InputStream is = null; - try { - is = new FileInputStream(file); - } catch (FileNotFoundException exception) { - log.debug("",exception); + private T readFromFile(String file, Function reader) throws IOException { + try (InputStream is = new FileInputStream(file)) { + return reader.apply(is); } - return is; } } diff --git a/openecomp-be/lib/openecomp-core-lib/openecomp-nosqldb-lib/openecomp-nosqldb-core/src/test/java/org/openecomp/core/nosqldb/util/ConfigurationManagerTest.java b/openecomp-be/lib/openecomp-core-lib/openecomp-nosqldb-lib/openecomp-nosqldb-core/src/test/java/org/openecomp/core/nosqldb/util/ConfigurationManagerTest.java new file mode 100644 index 0000000000..f8d5d2dce7 --- /dev/null +++ b/openecomp-be/lib/openecomp-core-lib/openecomp-nosqldb-lib/openecomp-nosqldb-core/src/test/java/org/openecomp/core/nosqldb/util/ConfigurationManagerTest.java @@ -0,0 +1,73 @@ +package org.openecomp.core.nosqldb.util; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.io.Closeable; +import java.io.IOException; +import java.lang.reflect.Field; + +import static org.testng.Assert.*; + +/** + * @author EVITALIY + * @since 22 Oct 17 + */ +public class ConfigurationManagerTest { + + private static final String NON_EXISTENT = "unexistentfile"; + + @BeforeMethod + public void resetInstance() throws NoSuchFieldException, IllegalAccessException { + Field field = ConfigurationManager.class.getDeclaredField("instance"); + field.setAccessible(true); + field.set(null, null); + } + + @Test(expectedExceptions = IOException.class, + expectedExceptionsMessageRegExp = ".*" + NON_EXISTENT + ".*") + public void testGetInstanceSystemProperty() throws Throwable { + + try (ConfigurationSystemPropertyUpdater updater = new ConfigurationSystemPropertyUpdater(NON_EXISTENT)) { + ConfigurationManager.getInstance(); + } catch (RuntimeException e) { + Throwable cause = e.getCause(); + throw cause == null ? e : cause; + } + } + + @Test() + public void testGetInstanceDefault() throws Exception { + + try (ConfigurationSystemPropertyUpdater property = new ConfigurationSystemPropertyUpdater()) { + ConfigurationManager manager = ConfigurationManager.getInstance(); + assertNotNull(manager.getUsername()); + } + } + + + private static class ConfigurationSystemPropertyUpdater implements Closeable { + + private final String oldValue; + + private ConfigurationSystemPropertyUpdater(String value) { + this.oldValue = System.getProperty(ConfigurationManager.CONFIGURATION_YAML_FILE); + System.setProperty(ConfigurationManager.CONFIGURATION_YAML_FILE, value); + } + + private ConfigurationSystemPropertyUpdater() { + this.oldValue = System.getProperty(ConfigurationManager.CONFIGURATION_YAML_FILE); + System.clearProperty(ConfigurationManager.CONFIGURATION_YAML_FILE); + } + + @Override + public void close() throws IOException { + + if (oldValue == null) { + System.clearProperty(ConfigurationManager.CONFIGURATION_YAML_FILE); + } else { + System.setProperty(ConfigurationManager.CONFIGURATION_YAML_FILE, oldValue); + } + } + } +} \ No newline at end of file diff --git a/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/main/java/org/openecomp/core/utilities/file/FileUtils.java b/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/main/java/org/openecomp/core/utilities/file/FileUtils.java index 6ff213c34c..6ab3f8b049 100644 --- a/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/main/java/org/openecomp/core/utilities/file/FileUtils.java +++ b/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/main/java/org/openecomp/core/utilities/file/FileUtils.java @@ -28,6 +28,7 @@ import org.openecomp.sdc.logging.api.LoggerFactory; import org.openecomp.sdc.tosca.services.YamlUtil; import java.io.ByteArrayInputStream; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.net.URL; @@ -54,7 +55,16 @@ public class FileUtils { * @param function logic to be applied to the input stream */ public static T readViaInputStream(String fileName, Function function) { - return readViaInputStream(FileUtils.class.getClassLoader().getResource(fileName), function); + + Objects.requireNonNull(fileName); + + // the leading slash doesn't make sense and doesn't work when used with a class loader + URL resource = FileUtils.class.getClassLoader().getResource(fileName.startsWith("/") ? fileName.substring(1) : fileName); + if (resource == null) { + throw new IllegalArgumentException("Resource not found: " + fileName); + } + + return readViaInputStream(resource, function); } /** diff --git a/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/test/java/org/openecomp/core/utilities/file/FileUtilsTest.java b/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/test/java/org/openecomp/core/utilities/file/FileUtilsTest.java new file mode 100644 index 0000000000..e32aa39904 --- /dev/null +++ b/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/test/java/org/openecomp/core/utilities/file/FileUtilsTest.java @@ -0,0 +1,48 @@ +package org.openecomp.core.utilities.file; + +import org.testng.annotations.Test; + +import java.io.IOException; +import java.io.InputStream; +import java.util.function.Function; + +import static org.testng.Assert.assertTrue; + +/** + * @author EVITALIY + * @since 22 Oct 17 + */ +public class FileUtilsTest { + + private static final String TEST_RESOURCE = FileUtilsTest.class.getPackage().getName() + .replace('.', '/') + "/test-resource.txt"; + + private static final Function TEST_FUNCTION = (s) -> { + + try { + return s.available(); + } catch (IOException e) { + throw new RuntimeException(e); + } + }; + + @Test + public void testReadViaInputStreamWithSlash() throws Exception { + assertTrue(FileUtils.readViaInputStream(TEST_RESOURCE, TEST_FUNCTION) > 0); + } + + @Test + public void testReadViaInputStreamWithoutSlash() throws Exception { + assertTrue(FileUtils.readViaInputStream(TEST_RESOURCE, TEST_FUNCTION) > 0); + } + + @Test(expectedExceptions = NullPointerException.class) + public void testReadViaInputStreamNull() throws Exception { + FileUtils.readViaInputStream((String) null, TEST_FUNCTION); + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testReadViaInputStreamNotFound() throws Exception { + FileUtils.readViaInputStream("notfound.txt", TEST_FUNCTION); + } +} \ No newline at end of file diff --git a/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/test/resources/org/openecomp/core/utilities/file/test-resource.txt b/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/test/resources/org/openecomp/core/utilities/file/test-resource.txt new file mode 100644 index 0000000000..dce400c8a5 --- /dev/null +++ b/openecomp-be/lib/openecomp-core-lib/openecomp-utilities-lib/src/test/resources/org/openecomp/core/utilities/file/test-resource.txt @@ -0,0 +1 @@ +CLASSPATH RESOURCE \ No newline at end of file diff --git a/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/services/heattotosca/HeatToToscaUtil.java b/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/services/heattotosca/HeatToToscaUtil.java index c1f69cf3bf..4e1a60fff8 100644 --- a/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/services/heattotosca/HeatToToscaUtil.java +++ b/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/services/heattotosca/HeatToToscaUtil.java @@ -119,9 +119,12 @@ public class HeatToToscaUtil { mdcDataDebugMessage.debugEntryMessage(null, null); HeatToToscaTranslator heatToToscaTranslator = HeatToToscaTranslatorFactory.getInstance().createInterface(); - InputStream fileContent = fileNameContentMap.getFileContent(SdcCommon.MANIFEST_NAME); - heatToToscaTranslator.addManifest(SdcCommon.MANIFEST_NAME, FileUtils.toByteArray(fileContent)); + try (InputStream fileContent = fileNameContentMap.getFileContent(SdcCommon.MANIFEST_NAME)) { + heatToToscaTranslator.addManifest(SdcCommon.MANIFEST_NAME, FileUtils.toByteArray(fileContent)); + } catch (IOException e) { + throw new RuntimeException("Failed to read manifest", e); + } fileNameContentMap.getFileList().stream() .filter(fileName -> !(fileName.equals(SdcCommon.MANIFEST_NAME))).forEach( @@ -143,7 +146,7 @@ public class HeatToToscaUtil { return heatToToscaTranslator.translate(); } catch (IOException e) { // rethrow as a RuntimeException to keep the signature backward compatible - throw new RuntimeException(e); + throw new RuntimeException("Failed to read Heat template tree", e); } } diff --git a/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/utils/ResourceWalker.java b/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/utils/ResourceWalker.java index 353b791a1f..9ff1ff9992 100644 --- a/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/utils/ResourceWalker.java +++ b/openecomp-be/lib/openecomp-sdc-translator-lib/openecomp-sdc-translator-core/src/main/java/org/openecomp/sdc/translator/utils/ResourceWalker.java @@ -40,8 +40,7 @@ public class ResourceWalker { Exception { Map filesContent = new HashMap<>(); traverse(resourceDirectoryToStart, (fileName, stream) -> { - BufferedReader reader = new BufferedReader(new InputStreamReader(stream)); - try { + try (BufferedReader reader = new BufferedReader(new InputStreamReader(stream))) { filesContent.put(fileName, IOUtils.toString(reader)); } catch (IOException exception) { MdcDataErrorMessage.createErrorMessageAndUpdateMdc(LoggerConstants.TARGET_ENTITY_API, diff --git a/openecomp-be/lib/openecomp-sdc-validation-lib/openecomp-sdc-validation-sdk/src/main/java/org/openecomp/sdc/validation/util/ValidationTestUtil.java b/openecomp-be/lib/openecomp-sdc-validation-lib/openecomp-sdc-validation-sdk/src/main/java/org/openecomp/sdc/validation/util/ValidationTestUtil.java index fa1946f987..2b58a30e50 100644 --- a/openecomp-be/lib/openecomp-sdc-validation-lib/openecomp-sdc-validation-sdk/src/main/java/org/openecomp/sdc/validation/util/ValidationTestUtil.java +++ b/openecomp-be/lib/openecomp-sdc-validation-lib/openecomp-sdc-validation-sdk/src/main/java/org/openecomp/sdc/validation/util/ValidationTestUtil.java @@ -25,6 +25,7 @@ import org.testng.Assert; import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.StringWriter; import java.net.URL; @@ -55,8 +56,6 @@ public class ValidationTestUtil { private static Map getContentMapByPath(String path) { Map contentMap = new HashMap<>(); - byte[] fileContent; - FileInputStream fis; URL url = ValidationTestUtil.class.getResource(path); File pathFile = new File(url.getFile()); File[] files; @@ -71,13 +70,13 @@ public class ValidationTestUtil { } for (File file : files) { - try { - fis = new FileInputStream(file); - fileContent = FileUtils.toByteArray(fis); - contentMap.put(file.getName(), fileContent); + + try (FileInputStream fis = new FileInputStream(file)) { + contentMap.put(file.getName(), FileUtils.toByteArray(fis)); } catch (IOException e) { - log.debug("",e); + throw new RuntimeException("Failed to read file: " + file, e); } + } return contentMap; } -- cgit 1.2.3-korg