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 <abhishek.sha...@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-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to