From 5de21a4ebac6ed2e2d1e1249ca3c986b7f1938b8 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Mon, 2 Dec 2024 15:41:24 +0100 Subject: [PATCH] Make Credentials thread-safe during initial loading of credentials Also add JavaDoc and add some more tests --- .../dstadler/commons/util/Credentials.java | 50 ++++++++++++++-- .../commons/util/CredentialsTest.java | 57 +++++++++++++++++++ 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/dstadler/commons/util/Credentials.java b/src/main/java/org/dstadler/commons/util/Credentials.java index 7bfa0466..b1cec605 100644 --- a/src/main/java/org/dstadler/commons/util/Credentials.java +++ b/src/main/java/org/dstadler/commons/util/Credentials.java @@ -9,12 +9,35 @@ import java.util.Properties; import java.util.logging.Logger; +/** + * Helper class to fetch credentials from a file "credentials.properties". + * + * The file can be located in the current directory or in the parent directory (..) + * or in a directory "resources/". + */ public class Credentials { private final static Logger log = LoggerFactory.make(); private static Properties properties; - public static Properties loadCredentials() { + /** + * Loads properties from a file "credentials.properties". + * + * Ensures data is replaced in a thread-safe way + * + * Failures to load the file are caught and reported + * as severe logs, no IOException is thrown. + * + * An empty set of properties is returned if no credentials file can be found + * + * @return The properties loaded from the file or empty when no credentials are found.. + */ + public synchronized static Properties loadCredentials() { + // are properties already initialized? + if (properties != null) { + return properties; + } + properties = new Properties(); File file = new File("credentials.properties"); @@ -30,6 +53,14 @@ public static Properties loadCredentials() { return properties; } + /** + * Load properties from the given file. + * + * Failures to load the file are caught and reported + * as severe logs, no IOException is thrown. + * + * @param file The text-file to read credentials. + */ protected static void loadProperties(File file) { try (FileInputStream fileStream = new FileInputStream(file)) { properties.load(fileStream); @@ -43,10 +74,14 @@ protected static void loadProperties(File file) { } } + /** + * Get the given credential or return null. + * + * @param key The credential to read. + * @return The value found in the credentials or null if not found. + */ public static String getCredentialOrNull(String key) { - if (properties == null) { - loadCredentials(); - } + loadCredentials(); String result = properties.getProperty(key, null); if (result == null && @@ -57,6 +92,13 @@ public static String getCredentialOrNull(String key) { return result; } + /** + * Get the given credential or throw an exception. + * + * @param key The credential to read. + * @return The value found in the credentials. + * @throws java.lang.IllegalStateException If no value is set for the given credential + */ public static String getCredentialOrFail(String key) { String result = getCredentialOrNull(key); if(result == null) { diff --git a/src/test/java/org/dstadler/commons/util/CredentialsTest.java b/src/test/java/org/dstadler/commons/util/CredentialsTest.java index b5528c88..c15609c4 100644 --- a/src/test/java/org/dstadler/commons/util/CredentialsTest.java +++ b/src/test/java/org/dstadler/commons/util/CredentialsTest.java @@ -3,13 +3,17 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.atomic.AtomicReference; import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.stream.IntStreams; import org.junit.Test; public class CredentialsTest { @@ -29,8 +33,16 @@ public void testLoadProperties() throws IOException { assertTrue(tempDir.delete()); assertTrue(tempDir.mkdirs()); try { + assertNull(Credentials.getCredentialOrNull("user")); + assertThrows(IllegalStateException.class, + () -> Credentials.getCredentialOrFail("user")); + Credentials.loadProperties(tempFile); + assertNull(Credentials.getCredentialOrNull("user")); + assertThrows(IllegalStateException.class, + () -> Credentials.getCredentialOrFail("user")); + FileUtils.writeStringToFile(tempFile, "user=abcd", "UTF-8"); Credentials.loadProperties(tempFile); @@ -40,6 +52,15 @@ public void testLoadProperties() throws IOException { assertNull(Credentials.getCredentialOrNull("pwd")); Credentials.loadProperties(tempDir); + + assertEquals("abcd", Credentials.getCredentialOrFail("user")); + assertEquals("abcd", Credentials.getCredentialOrNull("user")); + + FileUtils.writeStringToFile(tempFile, "user=efgh", "UTF-8"); + Credentials.loadProperties(tempFile); + + assertEquals("efgh", Credentials.getCredentialOrFail("user")); + assertEquals("efgh", Credentials.getCredentialOrNull("user")); } finally { FileUtils.deleteQuietly(tempFile); FileUtils.deleteQuietly(tempDir); @@ -78,4 +99,40 @@ public void testLoadInvalidFile() { public void testPrivateConstructor() throws Exception { org.dstadler.commons.testing.PrivateConstructorCoverage.executePrivateConstructor(Credentials.class); } + + @Test + public void testParallel() throws Throwable { + AtomicReference exc = new AtomicReference<>(); + + // load credentials in multiple streams to verify multi-threading + IntStreams.range(100).asLongStream(). + parallel(). + forEach(value -> { + // populate the file at some point + if (value == 20) { + try { + File file = File.createTempFile("CredentialsTest", ".properties"); + try { + FileUtils.writeStringToFile(file, """ + user=ab + password=cdr + """, + StandardCharsets.UTF_8); + Credentials.loadProperties(file); + } finally { + assertTrue("Could not delete file " + file, + !file.exists() || file.delete()); + } + } catch (Throwable e) { + exc.set(e); + } + } + Credentials.getCredentialOrNull("user"); + Credentials.getCredentialOrNull("password"); + }); + + if (exc.get() != null) { + throw exc.get(); + } + } }