From 2028b41602b3abf7c9bf450744efde7b296707a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= <adam@uhlir.dev>
Date: Wed, 19 Apr 2023 11:54:48 +0200
Subject: [PATCH] feat: integrate env. variable support natively (#70)

---
 README.md                                 |   8 +-
 confutils.nim                             |  24 +++-
 confutils/envvar/envvar_serialization.nim |  50 -------
 confutils/envvar/reader.nim               |  86 -----------
 confutils/envvar/utils.nim                |  93 ------------
 confutils/envvar/writer.nim               |  54 -------
 tests/test_config_file.nim                |  23 +--
 tests/test_envvar.nim                     | 166 ++++++++--------------
 8 files changed, 88 insertions(+), 416 deletions(-)
 delete mode 100644 confutils/envvar/envvar_serialization.nim
 delete mode 100644 confutils/envvar/reader.nim
 delete mode 100644 confutils/envvar/utils.nim
 delete mode 100644 confutils/envvar/writer.nim

diff --git a/README.md b/README.md
index a1ce420..67f1f0d 100644
--- a/README.md
+++ b/README.md
@@ -425,8 +425,12 @@ variables. The default format is called `CmdLineFormat` and it uses the
 same `parseCmdArg` calls responsible for parsing the command-line.
 
 The names of the environment variables are prefixed by the name of the
-program by default. They are matched in case-insensitive fashion and
-certain characters such as `-` and `_` are ignored.
+program by default and joined with the name of command line option, which is 
+uppercased and characters `-` and spaces are replaced with underscore:
+
+```nim
+let env_variable_name = &"{prefix}_{key}".toUpperAscii.multiReplace(("-", "_"), (" ", "_"))
+```
 
 #### `configFileEnumerator`
 
diff --git a/confutils.nim b/confutils.nim
index 083d079..dcf6ec4 100644
--- a/confutils.nim
+++ b/confutils.nim
@@ -1,5 +1,6 @@
 import
-  std/[options, strutils, wordwrap],
+  os,
+  std/[options, strutils, wordwrap, strformat],
   stew/shims/macros,
   serialization,
   confutils/[defs, cli_parser, config_file]
@@ -862,6 +863,12 @@ proc addConfigFileContent*(secondarySources: auto,
   except IOError:
     raiseAssert "This should not be possible"
 
+func constructEnvKey*(prefix: string, key: string): string =
+  ## Generates env. variable names from keys and prefix following the
+  ## IEEE Open Group env. variable spec: https://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html
+
+  return (&"{prefix}_{key}").toUpperAscii.multiReplace(("-", "_"), (" ", "_"))
+
 proc loadImpl[C, SecondarySources](
     Configuration: typedesc[C],
     cmdLine = commandLineParams(),
@@ -871,7 +878,8 @@ proc loadImpl[C, SecondarySources](
     quitOnFailure = true,
     secondarySourcesRef: ref SecondarySources,
     secondarySources: proc (config: Configuration,
-                            sources: ref SecondarySources) = nil): Configuration =
+                            sources: ref SecondarySources) = nil,
+    envVarsPrefix = getAppFilename()): Configuration =
   ## Loads a program configuration by parsing command-line arguments
   ## and a standard set of config files that can specify:
   ##
@@ -1105,7 +1113,12 @@ proc loadImpl[C, SecondarySources](
   proc processMissingOpts(conf: var Configuration, cmd: CmdInfo) =
     for opt in cmd.opts:
       if fieldCounters[opt.idx] == 0:
-        if secondarySourcesRef.setters[opt.idx](conf, secondarySourcesRef):
+        let envKey = constructEnvKey(envVarsPrefix, opt.name)
+
+        if existsEnv(envKey):
+          let envContent = getEnv(envKey)
+          applySetter(opt.idx, envContent)
+        elif secondarySourcesRef.setters[opt.idx](conf, secondarySourcesRef):
           # all work is done in the config file setter,
           # there is nothing left to do here.
           discard
@@ -1124,13 +1137,14 @@ template load*(
     copyrightBanner = "",
     printUsage = true,
     quitOnFailure = true,
-    secondarySources: untyped = nil): untyped =
+    secondarySources: untyped = nil,
+    envVarsPrefix = getAppFilename()): untyped =
 
   block:
     var secondarySourcesRef = generateSecondarySources(Configuration)
     loadImpl(Configuration, cmdLine, version,
              copyrightBanner, printUsage, quitOnFailure,
-             secondarySourcesRef, secondarySources)
+             secondarySourcesRef, secondarySources, envVarsPrefix)
 
 func defaults*(Configuration: type): Configuration =
   load(Configuration, cmdLine = @[], printUsage = false, quitOnFailure = false)
diff --git a/confutils/envvar/envvar_serialization.nim b/confutils/envvar/envvar_serialization.nim
deleted file mode 100644
index 18f37e0..0000000
--- a/confutils/envvar/envvar_serialization.nim
+++ /dev/null
@@ -1,50 +0,0 @@
-import
-  stew/shims/macros,
-  serialization, ./reader, ./writer
-
-export
-  serialization, reader, writer
-
-serializationFormat Envvar
-
-Envvar.setReader EnvvarReader
-Envvar.setWriter EnvvarWriter, PreferredOutput = void
-
-template supports*(_: type Envvar, T: type): bool =
-  # The Envvar format should support every type
-  true
-
-template decode*(_: type Envvar,
-                 prefix: string,
-                 RecordType: distinct type,
-                 params: varargs[untyped]): auto =
-  mixin init, ReaderType
-
-  {.noSideEffect.}:
-    var reader = unpackArgs(init, [EnvvarReader, prefix, params])
-    reader.readValue(RecordType)
-
-template encode*(_: type Envvar,
-                 prefix: string,
-                 value: auto,
-                 params: varargs[untyped]) =
-  mixin init, WriterType, writeValue
-
-  {.noSideEffect.}:
-    var writer = unpackArgs(init, [EnvvarWriter, prefix, params])
-    writeValue writer, value
-
-template loadFile*(_: type Envvar,
-                   prefix: string,
-                   RecordType: distinct type,
-                   params: varargs[untyped]): auto =
-  mixin init, ReaderType, readValue
-
-  var reader = unpackArgs(init, [EnvvarReader, prefix, params])
-  reader.readValue(RecordType)
-
-template saveFile*(_: type Envvar, prefix: string, value: auto, params: varargs[untyped]) =
-  mixin init, WriterType, writeValue
-
-  var writer = unpackArgs(init, [EnvvarWriter, prefix, params])
-  writer.writeValue(value)
diff --git a/confutils/envvar/reader.nim b/confutils/envvar/reader.nim
deleted file mode 100644
index 2545625..0000000
--- a/confutils/envvar/reader.nim
+++ /dev/null
@@ -1,86 +0,0 @@
-import
-  tables, typetraits, options, os,
-  serialization/[object_serialization, errors],
-  ./utils
-
-type
-  EnvvarReader* = object
-    prefix: string
-    key: seq[string]
-
-  EnvvarError* = object of SerializationError
-
-  EnvvarReaderError* = object of EnvvarError
-
-  GenericEnvvarReaderError* = object of EnvvarReaderError
-    deserializedField*: string
-    innerException*: ref CatchableError
-
-proc handleReadException*(r: EnvvarReader,
-                          Record: type,
-                          fieldName: string,
-                          field: auto,
-                          err: ref CatchableError) =
-  var ex = new GenericEnvvarReaderError
-  ex.deserializedField = fieldName
-  ex.innerException = err
-  raise ex
-
-proc init*(T: type EnvvarReader, prefix: string): T =
-  result.prefix = prefix
-
-proc readValue*[T](r: var EnvvarReader, value: var T) =
-  mixin readValue
-  # TODO: reduce allocation
-
-  when T is string:
-    let key = constructKey(r.prefix, r.key)
-    value = os.getEnv(key)
-
-  elif T is (SomePrimitives or range):
-    let key = constructKey(r.prefix, r.key)
-    getValue(key, value)
-
-  elif T is Option:
-    template getUnderlyingType[T](_: Option[T]): untyped = T
-    let key = constructKey(r.prefix, r.key)
-    if existsEnv(key):
-      type uType = getUnderlyingType(value)
-      when uType is string:
-        value = some(os.getEnv(key))
-      else:
-        value = some(r.readValue(uType))
-
-  elif T is (seq or array):
-    when uTypeIsPrimitives(T):
-      let key = constructKey(r.prefix, r.key)
-      getValue(key, value)
-
-    else:
-      let key = r.key[^1]
-      for i in 0..<value.len:
-        r.key[^1] = key & $i
-        r.readValue(value[i])
-
-  elif T is (object or tuple):
-    type T = type(value)
-    when T.totalSerializedFields > 0:
-      let fields = T.fieldReadersTable(EnvvarReader)
-      var expectedFieldPos = 0
-      r.key.add ""
-      value.enumInstanceSerializedFields(fieldName, field):
-        when T is tuple:
-          r.key[^1] = $expectedFieldPos
-          var reader = fields[][expectedFieldPos].reader
-          expectedFieldPos += 1
-        else:
-          r.key[^1] = fieldName
-          var reader = findFieldReader(fields[], fieldName, expectedFieldPos)
-
-        if reader != nil:
-          reader(value, r)
-      discard r.key.pop()
-
-  else:
-    const typeName = typetraits.name(T)
-    {.fatal: "Failed to convert from Envvar an unsupported type: " & typeName.}
diff --git a/confutils/envvar/utils.nim b/confutils/envvar/utils.nim
deleted file mode 100644
index 4da98d1..0000000
--- a/confutils/envvar/utils.nim
+++ /dev/null
@@ -1,93 +0,0 @@
-import
-  os,
-  stew/byteutils, stew/ptrops
-
-type
-  SomePrimitives* = SomeInteger | enum | bool | SomeFloat | char
-
-proc setValue*[T: SomePrimitives](key: string, val: openArray[T]) =
-  os.putEnv(key, byteutils.toHex(makeOpenArray(baseAddr val, byte, val.len*sizeof(T))))
-
-proc setValue*(key: string, val: SomePrimitives) =
-  os.putEnv(key, byteutils.toHex(makeOpenArray(val.unsafeAddr, byte, sizeof(val))))
-
-proc decodePaddedHex(hex: string, res: ptr UncheckedArray[byte], outputLen: int) =
-  # make it an even length
-  let
-    inputLen = hex.len and not 0x01
-    numHex = inputLen div 2
-    maxLen = min(outputLen, numHex)
-
-  var
-    offI = hex.len - maxLen * 2
-    offO = outputLen - maxLen
-
-  for i in 0 ..< maxLen:
-    res[i + offO] = hex[2*i + offI].readHexChar shl 4 or hex[2*i + 1 + offI].readHexChar
-
-  # write single nibble from odd length hex
-  if (offO > 0) and (offI > 0):
-    res[offO-1] = hex[offI-1].readHexChar
-
-proc getValue*(key: string, outVal: var string) =
-  let hex = os.getEnv(key)
-  let size = (hex.len div 2) + (hex.len and 0x01)
-  outVal.setLen(size)
-  decodePaddedHex(hex, cast[ptr UncheckedArray[byte]](outVal[0].addr), size)
-
-proc getValue*[T: SomePrimitives](key: string, outVal: var seq[T]) =
-  let hex = os.getEnv(key)
-  let byteSize = (hex.len div 2) + (hex.len and 0x01)
-  let size = (byteSize + sizeof(T) - 1) div sizeof(T)
-  outVal.setLen(size)
-  decodePaddedHex(hex, cast[ptr UncheckedArray[byte]](outVal[0].addr), size * sizeof(T))
-
-proc getValue*[N, T: SomePrimitives](key: string, outVal: var array[N, T]) =
-  let hex = os.getEnv(key)
-  decodePaddedHex(hex, cast[ptr UncheckedArray[byte]](outVal[0].addr), sizeof(outVal))
-
-proc getValue*(key: string, outVal: var SomePrimitives) =
-  let hex = os.getEnv(key)
-  decodePaddedHex(hex, cast[ptr UncheckedArray[byte]](outVal.addr), sizeof(outVal))
-
-template uTypeIsPrimitives*[T](_: type seq[T]): bool =
-  when T is SomePrimitives:
-    true
-  else:
-    false
-
-template uTypeIsPrimitives*[N, T](_: type array[N, T]): bool =
-  when T is SomePrimitives:
-    true
-  else:
-    false
-
-template uTypeIsPrimitives*[T](_: type openArray[T]): bool =
-  when T is SomePrimitives:
-    true
-  else:
-    false
-
-template uTypeIsRecord*(_: typed): bool =
-  false
-
-template uTypeIsRecord*[T](_: type seq[T]): bool =
-  when T is (object or tuple):
-    true
-  else:
-    false
-
-template uTypeIsRecord*[N, T](_: type array[N, T]): bool =
-  when T is (object or tuple):
-    true
-  else:
-    false
-
-func constructKey*(prefix: string, keys: openArray[string]): string =
-  var size = prefix.len
-  for i in 0..<keys.len:
-    inc(size, keys[i].len)
-  result = newStringOfCap(size)
-  result.add prefix
-  for x in keys:
-    result.add x
diff --git a/confutils/envvar/writer.nim b/confutils/envvar/writer.nim
deleted file mode 100644
index 2b160e1..0000000
--- a/confutils/envvar/writer.nim
+++ /dev/null
@@ -1,54 +0,0 @@
-import
-  typetraits, options, tables, os,
-  serialization, ./utils
-
-type
-  EnvvarWriter* = object
-    prefix: string
-    key: seq[string]
-
-proc init*(T: type EnvvarWriter, prefix: string): T =
-  result.prefix = prefix
-
-proc writeValue*(w: var EnvvarWriter, value: auto) =
-  mixin enumInstanceSerializedFields, writeValue, writeFieldIMPL
-  # TODO: reduce allocation
-
-  when value is string:
-    let key = constructKey(w.prefix, w.key)
-    os.putEnv(key, value)
-
-  elif value is (SomePrimitives or range):
-    let key = constructKey(w.prefix, w.key)
-    setValue(key, value)
-
-  elif value is Option:
-    if value.isSome:
-      w.writeValue value.get
-
-  elif value is (seq or array or openArray):
-    when uTypeIsPrimitives(type value):
-      let key = constructKey(w.prefix, w.key)
-      setValue(key, value)
-
-    elif uTypeIsRecord(type value):
-      let key = w.key[^1]
-      for i in 0..<value.len:
-        w.key[^1] = key & $i
-        w.writeValue(value[i])
-
-    else:
-      const typeName = typetraits.name(value.type)
-      {.fatal: "Failed to convert to Envvar array an unsupported type: " & typeName.}
-
-  elif value is (object or tuple):
-    type RecordType = type value
-    w.key.add ""
-    value.enumInstanceSerializedFields(fieldName, field):
-      w.key[^1] = fieldName
-      w.writeFieldIMPL(FieldTag[RecordType, fieldName], field, value)
-    discard w.key.pop()
-
-  else:
-    const typeName = typetraits.name(value.type)
-    {.fatal: "Failed to convert to Envvar an unsupported type: " & typeName.}
diff --git a/tests/test_config_file.nim b/tests/test_config_file.nim
index 52d9666..c42575c 100644
--- a/tests/test_config_file.nim
+++ b/tests/test_config_file.nim
@@ -13,8 +13,7 @@ import
 
 import
   toml_serialization, json_serialization,
-  ../confutils/winreg/winreg_serialization,
-  ../confutils/envvar/envvar_serialization
+  ../confutils/winreg/winreg_serialization
 
 type
   ValidatorPrivKey = object
@@ -164,20 +163,6 @@ proc readValue(r: var TomlReader, value: var GraffitiBytes) =
   except ValueError as ex:
     raise newException(SerializationError, ex.msg)
 
-proc readValue(r: var EnvvarReader,
-  value: var (InputFile | InputDir | OutFile | OutDir | ValidatorKeyPath)) =
-  type T = type value
-  value = r.readValue(string).T
-
-proc readValue(r: var EnvvarReader, value: var ValidIpAddress) =
-  value = ValidIpAddress.init(r.readValue(string))
-
-proc readValue(r: var EnvvarReader, value: var Port) =
-  value = r.readValue(int).Port
-
-proc readValue(r: var EnvvarReader, value: var GraffitiBytes) =
-  value = hexToByteArray[value.len](r.readValue(string))
-
 proc readValue(r: var WinregReader,
   value: var (InputFile | InputDir | OutFile | OutDir | ValidatorKeyPath)) =
   type T = type value
@@ -194,12 +179,10 @@ proc readValue(r: var WinregReader, value: var GraffitiBytes) {.used.} =
 
 proc testConfigFile() =
   suite "config file test suite":
-    putEnv("prefixdata-dir", "ENV VAR DATADIR")
+    putEnv("PREFIX_DATA_DIR", "ENV VAR DATADIR")
 
     test "basic config file":
-      let conf = TestConf.load(secondarySources = proc (config: TestConf, sources: auto) =
-        sources.addConfigFile(Envvar, InputFile "prefix")
-
+      let conf = TestConf.load(envVarsPrefix="prefix", secondarySources = proc (config: TestConf, sources: auto) =
         if config.configFile.isSome:
           sources.addConfigFile(Toml, config.configFile.get)
         else:
diff --git a/tests/test_envvar.nim b/tests/test_envvar.nim
index 472811e..8f0eae1 100644
--- a/tests/test_envvar.nim
+++ b/tests/test_envvar.nim
@@ -1,117 +1,71 @@
+# nim-confutils
+# Copyright (c) 2023 Status Research & Development GmbH
+# Licensed and distributed under either of
+#   * MIT license: [LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT
+#   * Apache License, Version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0)
+# at your option. This file may not be copied, modified, or distributed except according to those terms.
+
 import
-  std/options,
+  std/[os, strutils],
   unittest2,
-  ../confutils/envvar/envvar_serialization,
-  ../confutils/envvar/utils
+  ../confutils
 
-const
-  commonPrefix = "Nimbus"
-
-template readWrite(key: string, val: typed) =
-  test key:
-    setValue(key, val)
-    var outVal: type val
-    getValue(key, outVal)
-    check outVal == val
-
-proc testUtils() =
-  type
-    Fruit = enum
-      Apple
-
-  suite "envvar utils test suite":
-    readWrite("some number", 123'u32)
-    readWrite("some number 64", 123'u64)
-    readWrite("some bytes", @[1.byte, 2.byte])
-    readWrite("some int list", @[4,5,6])
-    readWrite("some array", [1.byte, 2.byte, 4.byte])
-    readWrite("some string", "hello world")
-    readWrite("some enum", Apple)
-    readWrite("some boolean", true)
-    readWrite("some float32", 1.234'f32)
-    readWrite("some float64", 1.234'f64)
-
-proc testEncoder() =
-  type
-    Class = enum
-      Truck
-      MPV
-      SUV
-
-    Fuel = enum
-      Gasoline
-      Diesel
-
-    Engine = object
-      cylinder: int
-      valve: int16
-      fuel: Fuel
-
-    Suspension = object
-      dist: int
-      length: int
-
-    Vehicle = object
-      name: string
-      color: int
-      class: Class
-      engine: Engine
-      wheel: int
-      suspension: array[3, Suspension]
-      door: array[4, int]
-      antennae: Option[int]
-      bumper: Option[string]
-
-  suite "envvar encoder test suite":
-    test "basic encoder and decoder":
-      let v = Vehicle(
-        name: "buggy",
-        color: 213,
-        class: MPV,
-        engine: Engine(
-          cylinder: 3,
-          valve: 2,
-          fuel: Diesel
-        ),
-        wheel: 6,
-        door: [1,2,3,4],
-        suspension: [
-          Suspension(dist: 1, length: 5),
-          Suspension(dist: 2, length: 6),
-          Suspension(dist: 3, length: 7)
-        ],
-        bumper: some("Chromium")
-      )
-
-      Envvar.encode(commonPrefix, v)
-      let x = Envvar.decode(commonPrefix, Vehicle)
-      check x == v
-      check x.antennae.isNone
-      check x.bumper.get() == "Chromium"
+const EnvVarPrefix = "Nimbus"
 
 type
-  ValidIpAddress {.requiresInit.} = object
-    value: string
+  SomeObject = object
+    name: string
+    isNice: bool
 
-  TestObject = object
-    address: Option[ValidIpAddress]
+  TestConf* = object
+    logLevel* {.
+      defaultValue: "DEBUG"
+      desc: "Sets the log level."
+      name: "log-level" }: string
 
-proc readValue(r: var EnvvarReader, value: var ValidIpAddress) =
-  r.readValue(value.value)
+    somObject* {.
+      desc: "..."
+      defaultValue: SomeObject()
+      name: "object" }: SomeObject
 
-proc writeValue(w: var EnvvarWriter, value: ValidIpAddress) =
-  w.writeValue(value.value)
+    dataDir* {.
+      defaultValue: ""
+      desc: "The directory where nimbus will store all blockchain data"
+      abbr: "d"
+      name: "data-dir" }: OutDir
 
-proc testOptionalFields() =
-  suite "optional fields test suite":
-    test "optional field with requiresInit pragma":
+func defaultObject(conf: TestConf): SomeObject =
+  discard
 
-      var z = TestObject(address: some(ValidIpAddress(value: "1.2.3.4")))
-      Envvar.saveFile(commonPrefix, z)
-      var x = Envvar.loadFile(commonPrefix, TestObject)
-      check x.address.isSome
-      check x.address.get().value == "1.2.3.4"
 
-testUtils()
-testEncoder()
-testOptionalFields()
+func completeCmdArg(T: type SomeObject, val: string): seq[string] =
+  @[]
+
+func parseCmdArg(T: type SomeObject, p: string): T =
+  let parsedString = p.split('-')
+  SomeObject(name:parsedString[0], isNice: parseBool(parsedString[1]))
+
+
+proc testEnvvar() =
+  suite "env var support suite":
+
+    test "env vars are loaded":
+      putEnv("NIMBUS_DATA_DIR", "ENV VAR DATADIR")
+      let conf = TestConf.load(envVarsPrefix=EnvVarPrefix)
+      check conf.dataDir.string == "ENV VAR DATADIR"
+
+    test "env vars do not have priority over cli parameters":
+      putEnv("NIMBUS_DATA_DIR", "ENV VAR DATADIR")
+      putEnv("NIMBUS_LOG_LEVEL", "ERROR")
+
+      let conf = TestConf.load(@["--log-level=INFO"], envVarsPrefix=EnvVarPrefix)
+      check conf.dataDir.string == "ENV VAR DATADIR"
+      check conf.logLevel.string == "INFO"
+
+    test "env vars use parseCmdArg":
+      putEnv("NIMBUS_OBJECT", "helloObject-true")
+      let conf = TestConf.load(envVarsPrefix=EnvVarPrefix)
+      check conf.somObject.name.string == "helloObject"
+      check conf.somObject.isNice.bool == true
+
+testEnvvar()