Breaking environment variable handling

Single underscores as path separators, uppercase.

Always loaded by default in place of the override source.

The override source is now unnecessary and removed.

Improved path normalization disambiguation, both for classes with
case-ambiguous properties and maps.

README.md

@@ -181,33 +181,32 @@ The `PropertySource` interface is how Hoplite reads configuration values.
 
 Hoplite supports several built in property source implementations, and you can write your own if required.
 
-The `EnvironmentVariableOverridePropertySource`, `SystemPropertiesPropertySource` and `UserSettingsPropertySource` sources are automatically registered,
-with precedence in that order. Other property sources can be passed to the config loader builder as required.
+The `EnvironmentVariablesPropertySource`, `SystemPropertiesPropertySource`, `UserSettingsPropertySource`, and `XdgConfigPropertySource`
+sources are automatically registered, with precedence in that order. Other property sources can be passed to the config loader builder
+as required.
 
 
 
 ### EnvironmentVariablesPropertySource
 
-The `EnvironmentVariablesPropertySource` reads config from environment variables. It does not map cases. So, `HOSTNAME` does *not* provide a value for a field with the name `hostname`.
+The `EnvironmentVariablesPropertySource` reads config from environment variables.
+This property source maps environment variable names to config properties via idiomatic conventions for environment variables.
+Env vars are idiomatically UPPERCASE and [contain only](https://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html) letters (`A` to `Z`), digits (`0` to `9`), and the underscore (`_`) character.
 
-For nested config, use a period to separate keys, for example `topic.name` would override `name` located in a `topic` parent.
-Alternatively, in some environments a `.` is not supported in ENV names, so you can also use double underscore `__`. Eg `topic__name` would be translated to `topic.name`.
+Hoplite maps env vars as follows:
 
-Optionally you can also create a `EnvironmentVariablesPropertySource` with `allowUppercaseNames` set to `true` to allow for uppercase-only names.
+* Underscores are separators for nested config. For example `TOPIC_NAME` would override a property `name` located in a `topic` parent.
 
+* To bind env vars to maps, the key is part of the nested config e.g. `TOPIC_NAME_FOO` and `TOPIC_NAME_BAR` would set the "foo" and "bar"
+keys for the `name` map property. Note that keys are one exception to the idiomatic uppercase rule -- the env var name determines the
+case of the map key.
 
+If the optional (not specified by default) `prefix` setting is provided, then only env vars that begin with the prefix are considered,
+and the prefix is stripped from the env var before processing.
 
-### EnvironmentVariableOverridePropertySource
-
-The `EnvironmentVariableOverridePropertySource` reads config from environment variables like the `EnvironmentVariablesPropertySource`.
-However, unlike that latter source, it is registered by default _and_ only looks for env vars
-with a special `config.override.` prefix. This prefix is stripped from the variable before being applied. This can be useful to apply changes
-at runtime without requiring a build.
-
-For example, given a config key of `database.host`, if an env variable exists with the key `config.override.database.host`, then the
-value in the env var would override.
-
-In some environments a . is not supported in ENV names, so you can also use double underscore __. Eg `topic__name` would be translated to `topic.name`.
+As of Hoplite 3, the `EnvironmentVariablesPropertySource` is applied by default and may be used to override other config properties
+directly. There is no longer any built-in support for the `config.override.` prefix. However, the optional `prefix` setting can still
+be used for the same purpose.
 
 
 ### SystemPropertiesPropertySource

changelog.md

@@ -1,5 +1,13 @@
 # Changelog
 
+### 3.0.0
+
+* Breaking: the `EnvironmentVariablesPropertySource` now uses idiomatic environment variable format i.e. upper case (though lower
+case is still accepted), letters, and digits. **Single underscores** now separate path hierarchies.
+* Breaking: The `EnvironmentVariableOverridePropertySource` has been removed. The standard `EnvironmentVariablesPropertySource` is now
+loaded by default, and takes precedence over other default sources just like the `EnvironmentVariableOverridePropertySource`
+did. To maintain similar behavior, configure it with a filtering `prefix`.
+
 ### 2.7.5
 
 * Use daemon threads in `FileWatcher` to enable clean shutdown.

hoplite-core/src/main/kotlin/com/sksamuel/hoplite/ConfigLoaderBuilder.kt

@@ -27,7 +27,7 @@ import com.sksamuel.hoplite.secrets.AllStringNodesSecretsPolicy
 import com.sksamuel.hoplite.secrets.Obfuscator
 import com.sksamuel.hoplite.secrets.PrefixObfuscator
 import com.sksamuel.hoplite.secrets.SecretsPolicy
-import com.sksamuel.hoplite.sources.EnvironmentVariableOverridePropertySource
+import com.sksamuel.hoplite.sources.EnvironmentVariablesPropertySource
 import com.sksamuel.hoplite.sources.SystemPropertiesPropertySource
 import com.sksamuel.hoplite.sources.UserSettingsPropertySource
 import com.sksamuel.hoplite.sources.XdgConfigPropertySource
@@ -80,13 +80,27 @@ class ConfigLoaderBuilder private constructor() {
      * use [empty] to obtain an empty ConfigLoaderBuilder and call the various addDefault methods manually.
      */
     fun default(): ConfigLoaderBuilder {
+      return defaultWithoutPropertySources()
+        .addDefaultPropertySources()
+    }
+
+    /**
+     * Returns a [ConfigLoaderBuilder] with all defaults applied, except for [PropertySource]s.
+     *
+     * This means that the default [Decoder]s, [Preprocessor]s, [NodeTransformer]s, [ParameterMapper]s,
+     * and [Parser]s are all registered.
+     *
+     * If you wish to avoid adding defaults, for example to avoid certain decoders or sources, then
+     * use [empty] to obtain an empty ConfigLoaderBuilder and call the various addDefault methods manually.
+     */
+    fun defaultWithoutPropertySources(configure: ConfigLoaderBuilder.() -> Unit = { }): ConfigLoaderBuilder {
       return empty()
         .addDefaultDecoders()
         .addDefaultPreprocessors()
         .addDefaultNodeTransformers()
         .addDefaultParamMappers()
-        .addDefaultPropertySources()
         .addDefaultParsers()
+        .apply(configure)
     }
 
     /**
@@ -103,12 +117,29 @@ class ConfigLoaderBuilder private constructor() {
      */
     @ExperimentalHoplite
     fun newBuilder(): ConfigLoaderBuilder {
+      return newBuilderWithoutPropertySources().addDefaultPropertySources()
+    }
+
+    /**
+     * Returns a [ConfigLoaderBuilder] with all defaults applied, using resolvers in place of preprocessors,
+     * but without any [PropertySource]s.
+     *
+     * This means that the default [Decoder]s, [Resolver]s, [NodeTransformer]s, [ParameterMapper]s,
+     * and [Parser]s are all registered.
+     *
+     * If you wish to avoid adding defaults, for example to avoid certain decoders or sources, then
+     * use [empty] to obtain an empty ConfigLoaderBuilder and call the various addDefault methods manually.
+     *
+     * Note: This new builder is experimental and may require breaking changes to your config files.
+     * This builder will become the default in 3.0
+     */
+    @ExperimentalHoplite
+    fun newBuilderWithoutPropertySources(): ConfigLoaderBuilder {
       return empty()
         .addDefaultDecoders()
         .addDefaultResolvers()
         .addDefaultNodeTransformers()
         .addDefaultParamMappers()
-        .addDefaultPropertySources()
         .addDefaultParsers()
     }
 
@@ -412,7 +443,13 @@ class ConfigLoaderBuilder private constructor() {
 }
 
 fun defaultPropertySources(): List<PropertySource> = listOfNotNull(
-  EnvironmentVariableOverridePropertySource(true),
+  EnvironmentVariablesPropertySource(),
+  SystemPropertiesPropertySource,
+  UserSettingsPropertySource,
+  XdgConfigPropertySource,
+)
+
+fun emptyByDefaultPropertySources(): List<PropertySource> = listOfNotNull(
   SystemPropertiesPropertySource,
   UserSettingsPropertySource,
   XdgConfigPropertySource,

hoplite-core/src/main/kotlin/com/sksamuel/hoplite/ConfigLoaderBuilderExtensions.kt

@@ -121,35 +121,9 @@ fun ConfigLoaderBuilder.addCommandLineSource(
 
 /**
  * Adds a [PropertySource] that will read the environment settings.
- *
- * @param useUnderscoresAsSeparator if true, use double underscore instead of period to separate keys in nested config
- * @param allowUppercaseNames if true, allow uppercase-only names
- * @param useSingleUnderscoresAsSeparator if true, allows single underscores as separators, to conform with
- * idiomatic environment variable names
- */
-fun ConfigLoaderBuilder.addEnvironmentSource(
-  useUnderscoresAsSeparator: Boolean = true,
-  allowUppercaseNames: Boolean = true,
-  useSingleUnderscoresAsSeparator: Boolean = false,
-) = addPropertySource(
-  EnvironmentVariablesPropertySource(useUnderscoresAsSeparator, useSingleUnderscoresAsSeparator, allowUppercaseNames)
-)
-
-/**
- * Adds a [PropertySource] that will read the environment settings.
- *
- * With this source, environment variables are expected to be idiomatic i.e. uppercase, with underscores as
- * separators for path elements. Dashes are removed.
- *
- * Generally a [PathNormalizer] should be added to the [ConfigLoaderBuilder] to normalize paths when this source
- * is used.
  */
-fun ConfigLoaderBuilder.addIdiomaticEnvironmentSource() = addPropertySource(
-  EnvironmentVariablesPropertySource(
-    useUnderscoresAsSeparator = false,
-    useSingleUnderscoresAsSeparator = true,
-    allowUppercaseNames = false
-  )
+fun ConfigLoaderBuilder.addEnvironmentSource() = addPropertySource(
+  EnvironmentVariablesPropertySource()
 )
 
 /**

hoplite-core/src/main/kotlin/com/sksamuel/hoplite/PropertySource.kt

@@ -97,28 +97,8 @@ interface PropertySource {
      * parsing mechanism using double-underscore as a path separator, and converting uppercase names with
      * underscores to camel case.
      */
-    fun environment(
-      useUnderscoresAsSeparator: Boolean = true,
-      allowUppercaseNames: Boolean = true,
-      useSingleUnderscoresAsSeparator: Boolean = false,
-    ) =
-      EnvironmentVariablesPropertySource(
-        useUnderscoresAsSeparator,
-        useSingleUnderscoresAsSeparator,
-        allowUppercaseNames
-      )
-
-    /**
-     * Returns a [PropertySource] that will read the environment settings, supporting idiomatic environment
-     * names. Underscores are used as path separators, and "-" are removed/ignored. We recommend this be used
-     * along with a [PathNormalizer].
-     */
-    fun idiomaticEnvironment() =
-      EnvironmentVariablesPropertySource(
-        useUnderscoresAsSeparator = false,
-        useSingleUnderscoresAsSeparator = true,
-        allowUppercaseNames = false
-      )
+    fun environment() =
+      EnvironmentVariablesPropertySource()
 
     /**
      * Returns a [PropertySource] that will read from the specified string.

hoplite-core/src/main/kotlin/com/sksamuel/hoplite/internal/PropertySourceLoader.kt

@@ -32,7 +32,9 @@ class PropertySourceLoader(
     configSources: List<ConfigSource>,
     resourceOrFiles: List<String>
   ): ConfigResult<NonEmptyList<Node>> {
-    require(propertySources.isNotEmpty() || configSources.isNotEmpty() || resourceOrFiles.isNotEmpty())
+    require(propertySources.isNotEmpty() || configSources.isNotEmpty() || resourceOrFiles.isNotEmpty()) {
+      "There must be at least one property source, config source, or resource/file defined"
+    }
 
     return ConfigSource
       .fromResourcesOrFiles(resourceOrFiles.toList(), classpathResourceLoader)

hoplite-core/src/main/kotlin/com/sksamuel/hoplite/sources/EnvironmentVariablesPropertySource.kt

@@ -8,40 +8,21 @@ import com.sksamuel.hoplite.fp.valid
 import com.sksamuel.hoplite.parsers.toNode
 
 class EnvironmentVariablesPropertySource(
-  private val useUnderscoresAsSeparator: Boolean,
-  private val useSingleUnderscoresAsSeparator: Boolean,
-  private val allowUppercaseNames: Boolean,
   private val environmentVariableMap: () -> Map<String, String> = { System.getenv() },
-  private val prefix: String? = null, // optional prefix to strip from the vars
+  /** Optional prefix to limit env var selection. It is stripped before processing. */
+  private val prefix: String? = null,
 ) : PropertySource {
+  companion object {
+    const val DELIMITER = "_"
+  }
 
   override fun source(): String = "Env Var"
 
   override fun node(context: PropertySourceContext): ConfigResult<Node> {
     val map = environmentVariableMap()
+      .filterKeys { if (prefix == null) true else it.startsWith(prefix) }
       .mapKeys { if (prefix == null) it.key else it.key.removePrefix(prefix) }
 
-    // at the moment the delimiter is either `__` or `.` -- it can't be mixed
-    val delimiter = if (useUnderscoresAsSeparator) "__" else if (useSingleUnderscoresAsSeparator) "_" else "."
-
-    return map.toNode("env", delimiter) { key ->
-      key
-        .let { if (prefix == null) it else it.removePrefix(prefix) }
-        .let {
-          if (allowUppercaseNames && Character.isUpperCase(it.codePointAt(0))) {
-            it.split(delimiter).joinToString(separator = delimiter) { value ->
-              value.fold("") { acc, char ->
-                when {
-                  acc.isEmpty() -> acc + char.lowercaseChar()
-                  acc.last() == '_' -> acc.dropLast(1) + char.uppercaseChar()
-                  else -> acc + char.lowercaseChar()
-                }
-              }
-            }
-          } else {
-            it
-          }
-        }
-    }.valid()
+    return map.toNode("env", DELIMITER).valid()
   }
 }

hoplite-core/src/main/kotlin/com/sksamuel/hoplite/transformer/PathNormalizer.kt

@@ -36,7 +36,10 @@ object PathNormalizer : NodeTransformer {
       )
       when (normalizedPathNode){
         is MapNode -> normalizedPathNode.copy(map = normalizedPathNode.map.mapKeys { (key, _) ->
-          normalizePathElementExceptDiscriminator(key, sealedTypeDiscriminatorField)
+          val normalizedKey = normalizePathElementExceptDiscriminator(key, sealedTypeDiscriminatorField)
+          // if normalization would cause overwriting an existing key, then don't normalize it
+          // this can be relevant for writing config into Maps
+          if (normalizedPathNode.map.containsKey(normalizedKey)) key else normalizedKey
         })
         else -> normalizedPathNode
       }

hoplite-core/src/test/kotlin/com/sksamuel/hoplite/CascadingNormalizationTest.kt

@@ -2,11 +2,9 @@ package com.sksamuel.hoplite
 
 import com.sksamuel.hoplite.sources.EnvironmentVariablesPropertySource
 import com.sksamuel.hoplite.sources.MapPropertySource
-import com.sksamuel.hoplite.transformer.PathNormalizer
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.shouldBe
 
-@OptIn(ExperimentalHoplite::class)
 class CascadingNormalizationTest : FunSpec() {
   init {
     test("Parameter normalization works with cascading") {
@@ -16,14 +14,10 @@ class CascadingNormalizationTest : FunSpec() {
 
       val configInputs = mapOf("section" to mapOf("test" to 1, "sub-section" to mapOf("some-value" to 2)))
 
-      val config = ConfigLoaderBuilder.newBuilder()
-        .addNodeTransformer(PathNormalizer)
+      val config = ConfigLoaderBuilder.defaultWithoutPropertySources()
         .addPropertySource(
           EnvironmentVariablesPropertySource(
-            useUnderscoresAsSeparator = false,
-            allowUppercaseNames = false,
-            useSingleUnderscoresAsSeparator = false,
-            environmentVariableMap = { mapOf("section.subSection.someValue" to "3") }
+            environmentVariableMap = { mapOf("SECTION_SUBSECTION_SOMEVALUE" to "3") }
           )
         )
         .addPropertySource(MapPropertySource(configInputs))