Improve reporting for key values

With path normalization (and even before path normalization), it was possible for the reported path to not match the input value. For example, with environment variables, the environment variable may have been `FOO__BAR`, but the report showed `FOO.BAR`, which makes it harder for users to trace the report back to the original source. We now add a `sourceKey` attribute to `Node` and report on it, to show user's the key value as it was originally present in the source.
sksamuel · Apr 4, 2024 · eac8a0b · eac8a0b
1 parent 373c382
commit eac8a0b
Show file tree

Hide file tree

Showing 10 changed files with 148 additions and 109 deletions.
diff --git a/hoplite-aws/src/main/kotlin/com/sksamuel/hoplite/aws/ParameterStorePathPropertySource.kt b/hoplite-aws/src/main/kotlin/com/sksamuel/hoplite/aws/ParameterStorePathPropertySource.kt
@@ -10,7 +10,6 @@ import com.sksamuel.hoplite.PropertySource
 import com.sksamuel.hoplite.PropertySourceContext
 import com.sksamuel.hoplite.decoder.toValidated
 import com.sksamuel.hoplite.parsers.toNode
-import java.util.Properties
 
 /**
  * Provides all keys under a prefix path as config values.
@@ -47,12 +46,9 @@ class ParameterStorePathPropertySource(
 
   override fun node(context: PropertySourceContext): ConfigResult<Node> {
     return fetchParameterStoreValues().map { params ->
-      val props = Properties()
-      params.forEach {
-        val name = if (stripPath) it.name.removePrefix(prefix) else it.name
-        props[name.removePrefix("/")] = it.value
+      params.associate { it.name to it.value }.toNode("aws_parameter_store at $prefix", "/") {
+        (if (stripPath) it.removePrefix(prefix) else it).removePrefix("/")
       }
-      props.toNode("aws_parameter_store at $prefix", "/")
     }.toValidated {
       ConfigFailure.PropertySourceFailure("Could not fetch data from AWS parameter store: ${it.message}", it)
     }

diff --git a/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/nodes.kt b/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/nodes.kt
@@ -14,6 +14,12 @@ sealed interface Node {
    */
   val path: DotPath
 
+  /**
+   * The original source key of this node without any normalization.
+   * Useful for reporting.
+   */
+  val sourceKey: String?
+
   /**
    * Returns the [PrimitiveNode] at the given key.
    * If this node is not a [MapNode] or the node contained at the
@@ -126,7 +132,8 @@ data class MapNode(
   override val pos: Pos,
   override val path: DotPath,
   val value: Node = Undefined,
-  override val meta: Map<String, Any?> = emptyMap()
+  override val meta: Map<String, Any?> = emptyMap(),
+  override val sourceKey: String? = if (path == DotPath.root) null else path.flatten(),
 ) : ContainerNode() {
   override val simpleName: String = "Map"
   override fun atKey(key: String): Node = map[key] ?: Undefined
@@ -138,7 +145,8 @@ data class ArrayNode(
   val elements: List<Node>,
   override val pos: Pos,
   override val path: DotPath,
-  override val meta: Map<String, Any?> = emptyMap()
+  override val meta: Map<String, Any?> = emptyMap(),
+  override val sourceKey: String? = if (path == DotPath.root) null else path.flatten(),
 ) : ContainerNode() {
   override val simpleName: String = "List"
   override fun atKey(key: String): Node = Undefined
@@ -157,7 +165,8 @@ data class StringNode(
   override val value: String,
   override val pos: Pos,
   override val path: DotPath,
-  override val meta: Map<String, Any?> = emptyMap()
+  override val meta: Map<String, Any?> = emptyMap(),
+  override val sourceKey: String? = if (path == DotPath.root) null else path.flatten(),
 ) : PrimitiveNode() {
   override val simpleName: String = "String"
 }
@@ -166,7 +175,8 @@ data class BooleanNode(
   override val value: Boolean,
   override val pos: Pos,
   override val path: DotPath,
-  override val meta: Map<String, Any?> = emptyMap()
+  override val meta: Map<String, Any?> = emptyMap(),
+  override val sourceKey: String? = if (path == DotPath.root) null else path.flatten(),
 ) : PrimitiveNode() {
   override val simpleName: String = "Boolean"
 }
@@ -177,7 +187,8 @@ data class LongNode(
   override val value: Long,
   override val pos: Pos,
   override val path: DotPath,
-  override val meta: Map<String, Any?> = emptyMap()
+  override val meta: Map<String, Any?> = emptyMap(),
+  override val sourceKey: String? = if (path == DotPath.root) null else path.flatten(),
 ) : NumberNode() {
   override val simpleName: String = "Long"
 }
@@ -186,15 +197,17 @@ data class DoubleNode(
   override val value: Double,
   override val pos: Pos,
   override val path: DotPath,
-  override val meta: Map<String, Any?> = emptyMap()
+  override val meta: Map<String, Any?> = emptyMap(),
+  override val sourceKey: String? = if (path == DotPath.root) null else path.flatten(),
 ) : NumberNode() {
   override val simpleName: String = "Double"
 }
 
 data class NullNode(
   override val pos: Pos,
   override val path: DotPath,
-  override val meta: Map<String, Any?> = emptyMap()
+  override val meta: Map<String, Any?> = emptyMap(),
+  override val sourceKey: String? = if (path == DotPath.root) null else path.flatten(),
 ) : PrimitiveNode() {
   override val simpleName: String = "null"
   override val value: Any? = null
@@ -204,6 +217,7 @@ object Undefined : Node {
   override val simpleName: String = "Undefined"
   override val pos: Pos = Pos.NoPos
   override val path = DotPath.root
+  override val sourceKey: String? = null
   override fun atKey(key: String): Node = this
   override fun atIndex(index: Int): Node = this
   override val size: Int = 0

diff --git a/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/parsers/loadProps.kt b/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/parsers/loadProps.kt
@@ -9,75 +9,95 @@ import com.sksamuel.hoplite.Undefined
 import com.sksamuel.hoplite.decoder.DotPath
 import java.util.Properties
 
-fun Properties.toNode(source: String, delimiter: String = ".") = asIterable().toNode(
+fun Properties.toNode(
+  source: String,
+  delimiter: String = ".",
+  keyExtractor: (Any) -> String = { it.toString() },
+) = asIterable().toNode(
   source = source,
-  keyExtractor = { it.key.toString() },
+  sourceKeyExtractor = { it.key },
+  keyExtractor = keyExtractor,
   valueExtractor = { it.value },
   delimiter = delimiter
 )
 
-fun <T : Any> Map<String, T?>.toNode(source: String, delimiter: String = ".") = entries.toNode(
+fun <T : Any> Map<String, T?>.toNode(
+  source: String,
+  delimiter: String = ".",
+  keyExtractor: (String) -> String = { it },
+) = entries.toNode(
   source = source,
-  keyExtractor = { it.key },
+  sourceKeyExtractor = { it.key },
+  keyExtractor = keyExtractor,
   valueExtractor = { it.value },
   delimiter = delimiter
 )
 
 data class Element(
   val values: MutableMap<String, Element> = hashMapOf(),
-  var value: Any? = null
+  var value: Any? = null,
+  var sourceKey: String? = null,
 )
 
-private fun <T> Iterable<T>.toNode(
+private fun <T, K> Iterable<T>.toNode(
   source: String,
-  keyExtractor: (T) -> String,
+  sourceKeyExtractor: (T) -> K,
+  keyExtractor: (K) -> String,
   valueExtractor: (T) -> Any?,
   delimiter: String = "."
 ): Node {
   val map = Element()
 
   forEach { item ->
-    val key = keyExtractor(item)
+    val sourceKey = sourceKeyExtractor(item)
+    val key = keyExtractor(sourceKey)
     val value = valueExtractor(item)
     val segments = key.split(delimiter)
 
     segments.foldIndexed(map) { index, element, segment ->
       element.values.getOrPut(segment) { Element() }.also {
-        if (index == segments.size - 1) it.value = value
+        if (index == segments.size - 1) {
+          it.value = value
+          it.sourceKey = sourceKey.toString()
+        }
       }
     }
   }
 
   val pos = Pos.SourcePos(source)
 
-  fun Any.transform(path: DotPath): Node = when (this) {
+  fun Any.transform(path: DotPath, parentSourceKey: String? = null): Node = when (this) {
     is Element -> when {
-      value != null && values.isEmpty() -> value?.transform(path) ?: Undefined
+      value != null && values.isEmpty() -> value?.transform(path, sourceKey) ?: Undefined
       else -> MapNode(
-        map = values.takeUnless { it.isEmpty() }?.mapValues { it.value.transform(path.with(it.key)) }.orEmpty(),
+        map = values.takeUnless { it.isEmpty() }?.mapValues { it.value.transform(path.with(it.key), sourceKey) }.orEmpty(),
         pos = pos,
         path = path,
-        value = value?.transform(path) ?: Undefined
+        value = value?.transform(path, sourceKey) ?: Undefined,
+        sourceKey = this.sourceKey,
       )
     }
     is Array<*> -> ArrayNode(
-      elements = mapNotNull { it?.transform(path) },
+      elements = mapNotNull { it?.transform(path, parentSourceKey) },
       pos = pos,
-      path = path
+      path = path,
+      sourceKey = parentSourceKey,
     )
     is Collection<*> -> ArrayNode(
-      elements = mapNotNull { it?.transform(path) },
+      elements = mapNotNull { it?.transform(path, parentSourceKey) },
       pos = pos,
-      path = path
+      path = path,
+      sourceKey = parentSourceKey,
     )
     is Map<*, *> -> MapNode(
       map = takeUnless { it.isEmpty() }?.mapNotNull { entry ->
-        entry.value?.let { entry.key.toString() to it.transform(path.with(entry.key.toString())) }
+        entry.value?.let { entry.key.toString() to it.transform(path.with(entry.key.toString()), parentSourceKey) }
       }?.toMap().orEmpty(),
       pos = pos,
-      path = path
+      path = path,
+      sourceKey = parentSourceKey,
     )
-    else -> StringNode(this.toString(), pos, path = path, emptyMap())
+    else -> StringNode(this.toString(), pos, path = path, emptyMap(), parentSourceKey)
   }
 
   return map.transform(DotPath.root)

diff --git a/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/report/Reporter.kt b/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/report/Reporter.kt
@@ -44,6 +44,7 @@ class Reporter(
   object Titles {
     const val Key = "Key"
     const val Source = "Source"
+    const val SourceKey = "Source Key"
     const val Value = "Value"
   }
 
@@ -107,19 +108,22 @@ class Reporter(
           value = value ?: "<null>",
           pos = state.node.pos,
           path = state.node.path,
-          meta = state.node.meta
+          meta = state.node.meta,
+          sourceKey = state.node.sourceKey,
         )
       )
     }
 
     val keyPadded = max(Titles.Key.length, nodes.maxOf { it.node.path.flatten().length })
     val sourcePadded = nodes.maxOf { max(it.node.pos.source()?.length ?: 0, Titles.Source.length) }
+    val sourceKeyPadded = max(Titles.SourceKey.length, nodes.maxOf { it.node.sourceKey.orEmpty().length })
     val valuePadded = max(Titles.Value.length, obfuscated.maxOf { (it.node as StringNode).value.length })
 
     val rows = obfuscated.map {
       listOfNotNull(
         it.node.path.flatten().padEnd(keyPadded, ' '),
         (it.node.pos.source() ?: "").padEnd(sourcePadded, ' '),
+        it.node.sourceKey.orEmpty().padEnd(sourceKeyPadded, ' '),
         (it.node as StringNode).value.padEnd(valuePadded, ' ')
       ).joinToString(" | ", "| ", " |")
     }
@@ -129,12 +133,14 @@ class Reporter(
     val bar = listOfNotNull(
       "".padEnd(keyPadded + 2, '-'),
       "".padEnd(sourcePadded + 2, '-'),
+      "".padEnd(sourceKeyPadded + 2, '-'),
       "".padEnd(valuePadded + 2, '-')
     ).joinToString("+", "+", "+")
 
     val titles = listOfNotNull(
       Titles.Key.padEnd(keyPadded, ' '),
       Titles.Source.padEnd(sourcePadded, ' '),
+      Titles.SourceKey.padEnd(sourceKeyPadded, ' '),
       Titles.Value.padEnd(valuePadded, ' ')
     ).joinToString(" | ", "| ", " |")
 

diff --git a/...e-core/src/main/kotlin/com/sksamuel/hoplite/sources/EnvironmentVariablesPropertySource.kt b/...e-core/src/main/kotlin/com/sksamuel/hoplite/sources/EnvironmentVariablesPropertySource.kt
@@ -6,7 +6,6 @@ import com.sksamuel.hoplite.PropertySource
 import com.sksamuel.hoplite.PropertySourceContext
 import com.sksamuel.hoplite.fp.valid
 import com.sksamuel.hoplite.parsers.toNode
-import java.util.Properties
 
 class EnvironmentVariablesPropertySource(
   private val useUnderscoresAsSeparator: Boolean,
@@ -18,29 +17,28 @@ class EnvironmentVariablesPropertySource(
   override fun source(): String = "Env Var"
 
   override fun node(context: PropertySourceContext): ConfigResult<Node> {
-    val props = Properties()
-    environmentVariableMap()
+    val map = environmentVariableMap()
       .mapKeys { if (prefix == null) it.key else it.key.removePrefix(prefix) }
-      .forEach {
-        val key = it.key
-          .let { key -> if (useUnderscoresAsSeparator) key.replace("__", ".") else key }
-          .let { key ->
-            if (allowUppercaseNames && Character.isUpperCase(key.codePointAt(0))) {
-              key.split(".").joinToString(separator = ".") { value ->
-                value.fold("") { acc, char ->
-                  when {
-                    acc.isEmpty() -> acc + char.lowercaseChar()
-                    acc.last() == '_' -> acc.dropLast(1) + char.uppercaseChar()
-                    else -> acc + char.lowercaseChar()
-                  }
+
+    return map.toNode("env") { key ->
+      key
+        .let { if (prefix == null) it else it.removePrefix(prefix) }
+        .let { if (useUnderscoresAsSeparator) it.replace("__", ".") else it }
+        .let {
+          if (allowUppercaseNames && Character.isUpperCase(it.codePointAt(0))) {
+            it.split(".").joinToString(separator = ".") { value ->
+              value.fold("") { acc, char ->
+                when {
+                  acc.isEmpty() -> acc + char.lowercaseChar()
+                  acc.last() == '_' -> acc.dropLast(1) + char.uppercaseChar()
+                  else -> acc + char.lowercaseChar()
                 }
               }
-            } else {
-              key
             }
+          } else {
+            it
           }
-        props[key] = it.value
-      }
-    return props.toNode("env").valid()
+        }
+    }.valid()
   }
 }
diff --git a/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/sources/SystemPropertiesPropertySource.kt b/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/sources/SystemPropertiesPropertySource.kt
@@ -23,13 +23,10 @@ open class SystemPropertiesPropertySource(
   override fun source(): String = "System Properties"
 
   override fun node(context: PropertySourceContext): ConfigResult<Node> {
-    val props = Properties()
-    systemPropertiesMap().let { systemPropertiesMap ->
-      systemPropertiesMap.keys
-        .filter { it.startsWith(prefix) }
-        .forEach { props[it.removePrefix(prefix)] = systemPropertiesMap[it] }
-    }
-    return if (props.isEmpty) Undefined.valid() else props.toNode("sysprops").valid()
+    val map = systemPropertiesMap().filter { it.key.startsWith(prefix) }
+    return if (map.isEmpty()) Undefined.valid() else map.toNode("sysprops") {
+      it.removePrefix(prefix)
+    }.valid()
   }
 
   companion object : SystemPropertiesPropertySource() {

diff --git a/hoplite-core/src/test/kotlin/com/sksamuel/hoplite/EnvironmentVariablesPropertySourceTest.kt b/hoplite-core/src/test/kotlin/com/sksamuel/hoplite/EnvironmentVariablesPropertySourceTest.kt
@@ -17,12 +17,13 @@ class EnvironmentVariablesPropertySourceTest : FunSpec({
     ).getUnsafe() shouldBe MapNode(
       mapOf(
         "a" to MapNode(
-          value = StringNode("foo", Pos.env, DotPath("a")),
-          map = mapOf("b" to StringNode("bar", Pos.env, DotPath("a", "b"))),
+          value = StringNode("foo", Pos.env, DotPath("a"), sourceKey = "a"),
+          map = mapOf("b" to StringNode("bar", Pos.env, DotPath("a", "b"), sourceKey = "a.b")),
           pos = Pos.SourcePos("env"),
-          path = DotPath("a")
+          path = DotPath("a"),
+          sourceKey = "a"
         ),
-        "c" to StringNode("baz", Pos.env, DotPath("c"))
+        "c" to StringNode("baz", Pos.env, DotPath("c"), sourceKey = "c"),
       ),
       pos = Pos.env,
       DotPath.root