From 12d02a0119c6a242cc9165d4a8a1cfb4bfdb4a02 Mon Sep 17 00:00:00 2001
From: Andrew <3199649+abaranec@users.noreply.github.com>
Date: Wed, 5 Feb 2025 10:53:45 -0500
Subject: [PATCH] feat: DH-18588: Allow shortened Name.rootFIle config file
 specifier (#6621)

(cherry picked from commit a3b00c27d5b223cf547564e0b54702a6e76de5c9)
---
 .../configuration/Configuration.java          | 32 ++++++++++++--
 .../configuration/TestConfiguration.java      | 42 +++++++++++++++++++
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/Configuration/src/main/java/io/deephaven/configuration/Configuration.java b/Configuration/src/main/java/io/deephaven/configuration/Configuration.java
index ffc6f4d541b..6d4e23e57f3 100644
--- a/Configuration/src/main/java/io/deephaven/configuration/Configuration.java
+++ b/Configuration/src/main/java/io/deephaven/configuration/Configuration.java
@@ -69,12 +69,19 @@ private static class Named extends Configuration {
 
         @Override
         protected String determinePropertyFile() {
-            final String propFile = System.getProperty("Configuration." + name + ".rootFile");
-            if (propFile == null) {
-                throw new ConfigurationException("No property file defined for named configuration " + name);
+            String propFile = System.getProperty("Configuration." + name + ".rootFile");
+            if (propFile != null) {
+                return propFile;
             }
 
-            return propFile;
+            // If Configuration.name.rootFile doesn't exist allow a fallback onto `name.rootFile`
+            propFile = System.getProperty(name + ".rootFile");
+            if (propFile != null) {
+                return propFile;
+            }
+
+            throw new ConfigurationException("No property file defined for named configuration " + name +
+                    ": Make sure the property `Configuration." + name + ".rootFile` or `" + name + ".rootFile` is set");
         }
     }
 
@@ -98,6 +105,7 @@ public static Configuration getInstance() {
      * @throws ConfigurationException if the named configuration could not be loaded.
      */
     public static Configuration getNamed(@NotNull final String name) throws ConfigurationException {
+        validateConfigName(name);
         return NAMED_CONFIGURATIONS.computeIfAbsent(name, (k) -> {
             try {
                 return new Named(name, DefaultConfigurationContext::new);
@@ -151,6 +159,7 @@ public static Configuration newStandaloneConfiguration(
      * @return a new Configuration instance, guaranteed to not be cached.
      */
     public static Configuration newStandaloneConfiguration(@NotNull final String name) {
+        validateConfigName(name);
         return newStandaloneConfiguration(name, DefaultConfigurationContext::new);
     }
 
@@ -433,4 +442,19 @@ private static void writeLine(StringBuilder out, String sKey, String sLeftValue,
     static boolean isQuiet() {
         return Bootstrap.isQuiet() || System.getProperty(QUIET_PROPERTY) != null;
     }
+
+    /**
+     * Check that the passed in config name is not null and is allowed.
+     *
+     * @param name the Configuration name.
+     */
+    static void validateConfigName(final String name) {
+        if (name == null) {
+            throw new ConfigurationException("Configuration name must not be null");
+        }
+
+        if ("Configuration".equals(name)) {
+            throw new ConfigurationException("The name `Configuration` may not be used as a Configuration name");
+        }
+    }
 }
diff --git a/Configuration/src/test/java/io/deephaven/configuration/TestConfiguration.java b/Configuration/src/test/java/io/deephaven/configuration/TestConfiguration.java
index 472956e726e..3a63acc134f 100644
--- a/Configuration/src/test/java/io/deephaven/configuration/TestConfiguration.java
+++ b/Configuration/src/test/java/io/deephaven/configuration/TestConfiguration.java
@@ -560,4 +560,46 @@ public void testNamedConfiguration() {
         assertNull(c2.getStringWithDefault("test2", null));
         assertEquals("true", c3.getProperty("test2"));
     }
+
+    public void testNamedConfigurationSimple() {
+        System.setProperty(FILENAME_PROPERTY, "resources/lib-tests.prop");
+        System.clearProperty("Configuration.Test1.rootFile");
+        System.setProperty("Test1.rootFile", "resources/test1.prop");
+
+        Configuration.reset();
+        Configuration dc = Configuration.getInstance();
+        Configuration c1 = Configuration.getNamed("Test1");
+
+        assertEquals("cache", dc.getProperty("cacheDir"));
+        assertNull(c1.getStringWithDefault("cacheDir", null));
+
+        assertNull(dc.getStringWithDefault("test1", null));
+        assertEquals("true", c1.getProperty("test1"));
+
+        assertNull(dc.getStringWithDefault("test3", null));
+        // test1 imports test3, so expected.
+        assertEquals("true", c1.getProperty("test3"));
+
+        // Make sure we can't try to load a Configuration with a null name, or `Configuration`
+        try {
+            final Configuration c = Configuration.getNamed(null);
+            fail("Should have rejected a null name");
+        } catch (ConfigurationException ignored) {
+
+        }
+
+        try {
+            final Configuration c = Configuration.getNamed("Configuration");
+            fail("Should have rejected the name `Configuration`");
+        } catch (ConfigurationException ignored) {
+
+        }
+
+        try {
+            final Configuration c = Configuration.getNamed("NonExistent");
+            fail("Should have rejected a named config that doesnt exist");
+        } catch (ConfigurationException ignored) {
+
+        }
+    }
 }