From b22280264b29481793df0a904b2e3b5a75faab77 Mon Sep 17 00:00:00 2001 From: sksamuel Date: Tue, 16 Nov 2021 20:58:30 -0600 Subject: [PATCH] Fixed empty constructor bug #236 --- changelog.md | 4 ++ .../hoplite/decoder/DataClassDecoder.kt | 64 +++++++++++-------- .../com/sksamuel/hoplite/fp/Validated.kt | 20 +++--- .../com/sksamuel/hoplite/yaml/Issue236Test.kt | 26 ++++++++ 4 files changed, 77 insertions(+), 37 deletions(-) create mode 100644 hoplite-yaml/src/test/kotlin/com/sksamuel/hoplite/yaml/Issue236Test.kt diff --git a/changelog.md b/changelog.md index b48e6729..a9ec6e8c 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,9 @@ # Changelog +### 1.4.14 + +* Fixed regression with multiple constructors introduced in 1.4.10 + ### 1.4.13 * Fixed regression with ConfigSouce::fromPath #234 diff --git a/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/decoder/DataClassDecoder.kt b/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/decoder/DataClassDecoder.kt index b2581a08..b8ea9f82 100644 --- a/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/decoder/DataClassDecoder.kt +++ b/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/decoder/DataClassDecoder.kt @@ -1,7 +1,5 @@ package com.sksamuel.hoplite.decoder -import com.sksamuel.hoplite.fp.invalid -import com.sksamuel.hoplite.fp.valid import com.sksamuel.hoplite.ConfigFailure import com.sksamuel.hoplite.ConfigResult import com.sksamuel.hoplite.DecodeMode @@ -12,13 +10,15 @@ import com.sksamuel.hoplite.ParameterMapper import com.sksamuel.hoplite.PrimitiveNode import com.sksamuel.hoplite.StringNode import com.sksamuel.hoplite.Undefined +import com.sksamuel.hoplite.fp.NonEmptyList import com.sksamuel.hoplite.fp.Validated import com.sksamuel.hoplite.fp.ValidatedNel import com.sksamuel.hoplite.fp.flatMap +import com.sksamuel.hoplite.fp.invalid import com.sksamuel.hoplite.fp.sequence +import com.sksamuel.hoplite.fp.valid import com.sksamuel.hoplite.isDefined import com.sksamuel.hoplite.simpleName -import java.lang.IllegalArgumentException import java.lang.reflect.InvocationTargetException import kotlin.reflect.KClass import kotlin.reflect.KFunction @@ -53,12 +53,18 @@ class DataClassDecoder : NullHandlingDecoder { return ConfigFailure.DataClassWithoutConstructor(klass).invalid() } - data class Arg(val constructor: KFunction, - val parameter: KParameter, - val configName: String, // the config value name that was used - val value: Any?) + data class Arg( + val parameter: KParameter, + val configName: String, // the config value name that was used + val value: Any?, + ) + + data class Constructor( + val constructor: KFunction, + val args: List, + ) - val argsList = klass.constructors.map { constructor -> + val constructors = klass.constructors.map { constructor -> // try for the value type // we have a special case, which is a data class with a single field with the name 'value'. @@ -91,32 +97,36 @@ class DataClassDecoder : NullHandlingDecoder { param.isOptional && n is Undefined -> null else -> context.decoder(param) .flatMap { it.decode(n, param.type, context) } - .map { Arg(constructor, param, name, it) } + .map { Arg(param, name, it) } .mapInvalid { ConfigFailure.ParamFailure(param, it) } } }.sequence() - args + + args.map { Constructor(constructor, it) } } - val firstValidOrLastInvalidArgs = argsList.firstOrNull{ it is Validated.Valid } ?: - argsList.last { it is Validated.Invalid } - return when (firstValidOrLastInvalidArgs) { - // in invalid we wrap in an error containing each individual error - is Validated.Invalid -> ConfigFailure.DataClassFieldErrors( - firstValidOrLastInvalidArgs.error, type, node.pos).invalid() - is Validated.Valid -> { - - // in strict mode we throw an error if not all config values were used for the class - if (node is MapNode) { - if (context.mode == DecodeMode.Strict && firstValidOrLastInvalidArgs.value.size != node.size) { - val unusedValues = node.map.keys.minus(firstValidOrLastInvalidArgs.value.map { it.configName }) - return ConfigFailure.UnusedConfigValues(unusedValues.toList()).invalid() - } - } - return construct(type, firstValidOrLastInvalidArgs.value.first().constructor, - firstValidOrLastInvalidArgs.value.map { it.parameter to it.value }.toMap()) + // see if one of the constructors worked + val firstValidOrLastInvalidArgs: Validated, Constructor> = constructors + .find { it is Validated.Valid } ?: constructors.last { it is Validated.Invalid } + + return firstValidOrLastInvalidArgs.fold( + // if invalid, we wrap in an error containing each individual error + { ConfigFailure.DataClassFieldErrors(it, type, node.pos).invalid() }, + { constructor -> + + // in strict mode we throw an error if not all config values were used for the class + if (node is MapNode && context.mode == DecodeMode.Strict && constructor.args.size != node.size) { + val unusedValues = node.map.keys.minus(constructor.args.map { it.configName }.toSet()) + ConfigFailure.UnusedConfigValues(unusedValues.toList()).invalid() + } else { + construct( + type = type, + constructor = constructor.constructor, + args = constructor.args.associate { it.parameter to it.value }, + ) } } + ) } private fun construct( diff --git a/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/fp/Validated.kt b/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/fp/Validated.kt index f6c9dd82..9277f9a2 100644 --- a/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/fp/Validated.kt +++ b/hoplite-core/src/main/kotlin/com/sksamuel/hoplite/fp/Validated.kt @@ -13,24 +13,24 @@ sealed class Validated { } fun map(f: (A) -> B): Validated = when (this) { - is Validated.Valid -> f(this.value).valid() - is Validated.Invalid -> this + is Valid -> f(this.value).valid() + is Invalid -> this } fun mapInvalid(f: (E) -> F): Validated = when (this) { - is Validated.Invalid -> f(this.error).invalid() - is Validated.Valid -> this + is Invalid -> f(this.error).invalid() + is Valid -> this } fun toValidatedNel(): Validated, A> = when (this) { - is Validated.Valid -> value.valid() - is Validated.Invalid -> NonEmptyList.of(error).invalid() + is Valid -> value.valid() + is Invalid -> NonEmptyList.of(error).invalid() } - fun fold(ifInvalid: (E) -> Unit, ifValid: (A) -> Unit) { - when (this) { - is Validated.Valid -> ifValid(this.value) - is Validated.Invalid -> ifInvalid(this.error) + fun fold(ifInvalid: (E) -> T, ifValid: (A) -> T): T { + return when (this) { + is Valid -> ifValid(this.value) + is Invalid -> ifInvalid(this.error) } } diff --git a/hoplite-yaml/src/test/kotlin/com/sksamuel/hoplite/yaml/Issue236Test.kt b/hoplite-yaml/src/test/kotlin/com/sksamuel/hoplite/yaml/Issue236Test.kt new file mode 100644 index 00000000..b73b9946 --- /dev/null +++ b/hoplite-yaml/src/test/kotlin/com/sksamuel/hoplite/yaml/Issue236Test.kt @@ -0,0 +1,26 @@ +package com.sksamuel.hoplite.yaml + +import com.sksamuel.hoplite.ConfigLoader +import io.kotest.core.spec.style.FreeSpec + +data class A( + val foo: Fooby = Fooby() +) + +data class Fooby( + val x: String = "x" +) + +class Issue236Test : FreeSpec({ + + val config = "{" + + "}" + + "Bug Reproduce" { + ConfigLoader.Builder() + .addSource(YamlPropertySource(config)) + .build() + .loadConfigOrThrow() + } + +})