Skip to content

Commit 12d02a0

Browse files
authored
feat: DH-18588: Allow shortened Name.rootFIle config file specifier (#6621)
(cherry picked from commit a3b00c2)
1 parent 315ff3f commit 12d02a0

File tree

2 files changed

+70
-4
lines changed

2 files changed

+70
-4
lines changed

Configuration/src/main/java/io/deephaven/configuration/Configuration.java

+28-4
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,19 @@ private static class Named extends Configuration {
6969

7070
@Override
7171
protected String determinePropertyFile() {
72-
final String propFile = System.getProperty("Configuration." + name + ".rootFile");
73-
if (propFile == null) {
74-
throw new ConfigurationException("No property file defined for named configuration " + name);
72+
String propFile = System.getProperty("Configuration." + name + ".rootFile");
73+
if (propFile != null) {
74+
return propFile;
7575
}
7676

77-
return propFile;
77+
// If Configuration.name.rootFile doesn't exist allow a fallback onto `name.rootFile`
78+
propFile = System.getProperty(name + ".rootFile");
79+
if (propFile != null) {
80+
return propFile;
81+
}
82+
83+
throw new ConfigurationException("No property file defined for named configuration " + name +
84+
": Make sure the property `Configuration." + name + ".rootFile` or `" + name + ".rootFile` is set");
7885
}
7986
}
8087

@@ -98,6 +105,7 @@ public static Configuration getInstance() {
98105
* @throws ConfigurationException if the named configuration could not be loaded.
99106
*/
100107
public static Configuration getNamed(@NotNull final String name) throws ConfigurationException {
108+
validateConfigName(name);
101109
return NAMED_CONFIGURATIONS.computeIfAbsent(name, (k) -> {
102110
try {
103111
return new Named(name, DefaultConfigurationContext::new);
@@ -151,6 +159,7 @@ public static Configuration newStandaloneConfiguration(
151159
* @return a new Configuration instance, guaranteed to not be cached.
152160
*/
153161
public static Configuration newStandaloneConfiguration(@NotNull final String name) {
162+
validateConfigName(name);
154163
return newStandaloneConfiguration(name, DefaultConfigurationContext::new);
155164
}
156165

@@ -433,4 +442,19 @@ private static void writeLine(StringBuilder out, String sKey, String sLeftValue,
433442
static boolean isQuiet() {
434443
return Bootstrap.isQuiet() || System.getProperty(QUIET_PROPERTY) != null;
435444
}
445+
446+
/**
447+
* Check that the passed in config name is not null and is allowed.
448+
*
449+
* @param name the Configuration name.
450+
*/
451+
static void validateConfigName(final String name) {
452+
if (name == null) {
453+
throw new ConfigurationException("Configuration name must not be null");
454+
}
455+
456+
if ("Configuration".equals(name)) {
457+
throw new ConfigurationException("The name `Configuration` may not be used as a Configuration name");
458+
}
459+
}
436460
}

Configuration/src/test/java/io/deephaven/configuration/TestConfiguration.java

+42
Original file line numberDiff line numberDiff line change
@@ -560,4 +560,46 @@ public void testNamedConfiguration() {
560560
assertNull(c2.getStringWithDefault("test2", null));
561561
assertEquals("true", c3.getProperty("test2"));
562562
}
563+
564+
public void testNamedConfigurationSimple() {
565+
System.setProperty(FILENAME_PROPERTY, "resources/lib-tests.prop");
566+
System.clearProperty("Configuration.Test1.rootFile");
567+
System.setProperty("Test1.rootFile", "resources/test1.prop");
568+
569+
Configuration.reset();
570+
Configuration dc = Configuration.getInstance();
571+
Configuration c1 = Configuration.getNamed("Test1");
572+
573+
assertEquals("cache", dc.getProperty("cacheDir"));
574+
assertNull(c1.getStringWithDefault("cacheDir", null));
575+
576+
assertNull(dc.getStringWithDefault("test1", null));
577+
assertEquals("true", c1.getProperty("test1"));
578+
579+
assertNull(dc.getStringWithDefault("test3", null));
580+
// test1 imports test3, so expected.
581+
assertEquals("true", c1.getProperty("test3"));
582+
583+
// Make sure we can't try to load a Configuration with a null name, or `Configuration`
584+
try {
585+
final Configuration c = Configuration.getNamed(null);
586+
fail("Should have rejected a null name");
587+
} catch (ConfigurationException ignored) {
588+
589+
}
590+
591+
try {
592+
final Configuration c = Configuration.getNamed("Configuration");
593+
fail("Should have rejected the name `Configuration`");
594+
} catch (ConfigurationException ignored) {
595+
596+
}
597+
598+
try {
599+
final Configuration c = Configuration.getNamed("NonExistent");
600+
fail("Should have rejected a named config that doesnt exist");
601+
} catch (ConfigurationException ignored) {
602+
603+
}
604+
}
563605
}

0 commit comments

Comments
 (0)