You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by xb...@apache.org on 2024/05/23 17:32:34 UTC
(pinot) branch master updated: Refactored CommonsConfigurationUtils for loading properties configuration. (#13201)
This is an automated email from the ASF dual-hosted git repository.
xbli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 0dec8a9ab9 Refactored CommonsConfigurationUtils for loading properties configuration. (#13201)
0dec8a9ab9 is described below
commit 0dec8a9ab9183eb91b9abee50edebdfcb5969103
Author: Abhishek Sharma <ab...@spothero.com>
AuthorDate: Thu May 23 13:32:29 2024 -0400
Refactored CommonsConfigurationUtils for loading properties configuration. (#13201)
* Delete unnecessary setFile call.
* Refactored CommonsConfigurationUtils for properties configuration creation and added UTs.
---
.../pinot/spi/env/CommonsConfigurationUtils.java | 29 ++++++++------
.../spi/env/CommonsConfigurationUtilsTest.java | 44 ++++++++++++++++++++++
2 files changed, 61 insertions(+), 12 deletions(-)
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java b/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
index 1b09d25014..0613230e09 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
@@ -84,11 +84,15 @@ public class CommonsConfigurationUtils {
* @param setDefaultDelimiter representing to set the default list delimiter.
* @return a {@link PropertiesConfiguration} instance.
*/
- public static PropertiesConfiguration fromPath(String path, boolean setIOFactory, boolean setDefaultDelimiter)
+ public static PropertiesConfiguration fromPath(@Nullable String path, boolean setIOFactory,
+ boolean setDefaultDelimiter)
throws ConfigurationException {
PropertiesConfiguration config = createPropertiesConfiguration(setIOFactory, setDefaultDelimiter);
- FileHandler fileHandler = new FileHandler(config);
- fileHandler.load(path);
+ // if provided path is non-empty, load the existing properties from provided file path
+ if (StringUtils.isNotEmpty(path)) {
+ FileHandler fileHandler = new FileHandler(config);
+ fileHandler.load(path);
+ }
return config;
}
@@ -99,12 +103,15 @@ public class CommonsConfigurationUtils {
* @param setDefaultDelimiter representing to set the default list delimiter.
* @return a {@link PropertiesConfiguration} instance.
*/
- public static PropertiesConfiguration fromInputStream(InputStream stream, boolean setIOFactory,
+ public static PropertiesConfiguration fromInputStream(@Nullable InputStream stream, boolean setIOFactory,
boolean setDefaultDelimiter)
throws ConfigurationException {
PropertiesConfiguration config = createPropertiesConfiguration(setIOFactory, setDefaultDelimiter);
- FileHandler fileHandler = new FileHandler(config);
- fileHandler.load(stream);
+ // if provided stream is not null, load the existing properties from provided input stream.
+ if (stream != null) {
+ FileHandler fileHandler = new FileHandler(config);
+ fileHandler.load(stream);
+ }
return config;
}
@@ -115,15 +122,13 @@ public class CommonsConfigurationUtils {
* @param setDefaultDelimiter representing to set the default list delimiter.
* @return a {@link PropertiesConfiguration} instance.
*/
- public static PropertiesConfiguration fromFile(File file, boolean setIOFactory, boolean setDefaultDelimiter)
+ public static PropertiesConfiguration fromFile(@Nullable File file, boolean setIOFactory, boolean setDefaultDelimiter)
throws ConfigurationException {
PropertiesConfiguration config = createPropertiesConfiguration(setIOFactory, setDefaultDelimiter);
- FileHandler fileHandler = new FileHandler(config);
- // check if file exists, load the properties otherwise set the file.
- if (file.exists()) {
+ // check if file exists, load the existing properties.
+ if (file != null && file.exists()) {
+ FileHandler fileHandler = new FileHandler(config);
fileHandler.load(file);
- } else {
- fileHandler.setFile(file);
}
return config;
}
diff --git a/pinot-spi/src/test/java/org/apache/pinot/spi/env/CommonsConfigurationUtilsTest.java b/pinot-spi/src/test/java/org/apache/pinot/spi/env/CommonsConfigurationUtilsTest.java
index af40022794..06174ed212 100644
--- a/pinot-spi/src/test/java/org/apache/pinot/spi/env/CommonsConfigurationUtilsTest.java
+++ b/pinot-spi/src/test/java/org/apache/pinot/spi/env/CommonsConfigurationUtilsTest.java
@@ -19,8 +19,11 @@
package org.apache.pinot.spi.env;
import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
import java.io.IOException;
import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.StringUtils;
@@ -29,6 +32,7 @@ import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
@@ -119,4 +123,44 @@ public class CommonsConfigurationUtilsTest {
(String) configuration.getProperty(PROPERTY_KEY));
assertEquals(recoveredValue, value);
}
+
+ @Test
+ public void testPropertiesConfigurationFromFile()
+ throws ConfigurationException {
+ PropertiesConfiguration configuration = CommonsConfigurationUtils.fromFile(null, false, true);
+ assertNotNull(configuration);
+ configuration.setProperty("Test Key", "Test Value");
+ CommonsConfigurationUtils.saveToFile(configuration, CONFIG_FILE);
+
+ configuration = CommonsConfigurationUtils.fromFile(CONFIG_FILE, false, true);
+ assertNotNull(configuration);
+ assertEquals(configuration.getProperty("Test Key"), "Test Value");
+ }
+
+ @Test
+ public void testPropertiesConfigurationFromPath()
+ throws ConfigurationException {
+ PropertiesConfiguration configuration = CommonsConfigurationUtils.fromPath(null, false, true);
+ assertNotNull(configuration);
+ configuration.setProperty("Test Key", "Test Value");
+ CommonsConfigurationUtils.saveToFile(configuration, CONFIG_FILE);
+
+ configuration = CommonsConfigurationUtils.fromPath(CONFIG_FILE.getPath(), false, true);
+ assertNotNull(configuration);
+ assertEquals(configuration.getProperty("Test Key"), "Test Value");
+ }
+
+ @Test
+ public void testPropertiesConfigurationFromInputStream()
+ throws ConfigurationException, FileNotFoundException {
+ PropertiesConfiguration configuration = CommonsConfigurationUtils.fromInputStream(null, false, true);
+ assertNotNull(configuration);
+ configuration.setProperty("Test Key", "Test Value");
+ CommonsConfigurationUtils.saveToFile(configuration, CONFIG_FILE);
+
+ FileInputStream inputStream = new FileInputStream(CONFIG_FILE);
+ configuration = CommonsConfigurationUtils.fromInputStream(inputStream, false, true);
+ assertNotNull(configuration);
+ assertEquals(configuration.getProperty("Test Key"), "Test Value");
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org