fix reducing results from multiple naming stragegies for optional att…

…ributes
kxbmap · Dec 28, 2020 · b8ada03 · b8ada03
1 parent 85f5f94
commit b8ada03
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 17 deletions.
diff --git a/core/src/main/scala/configs/ConfigKeyNaming.scala b/core/src/main/scala/configs/ConfigKeyNaming.scala
@@ -29,7 +29,7 @@ trait ConfigKeyNaming[A] { self =>
     field => self.apply(field).flatMap(f)
 
   def or(f: String => Seq[String]): ConfigKeyNaming[A] =
-    field => self.apply(field) ++ f(field)
+    field => (self.apply(field) ++ f(field)).distinct
 }
 
 object ConfigKeyNaming {

diff --git a/core/src/main/scala/configs/ConfigReader.scala b/core/src/main/scala/configs/ConfigReader.scala
@@ -29,17 +29,19 @@ trait ConfigReader[A] {
 
   protected def get(config: Config, path: String): Result[A]
 
-  final def read(config: Config, path: String): Result[A] = {
+  final def read(config: Config, path: String): Result[A] =
     get(config, path).pushPath(path)
-  }
 
-  final def read(config: Config, path: Seq[String]): Result[A] = {
-    // take first success, if all failed the last failure
-    path.zipWithIndex.iterator
-      .map { case (p,i) => (read(config, p),i)}
-      .find { case (p,i) => p.isSuccess || i == path.size - 1 }
-      .get._1
-  }
+  /**
+   * reduce results for multiple naming strategies
+   */
+  protected def reduce(results: Seq[Result[A]]): Result[A] =
+  // default is: take first success, if all failed the last failure
+    results.find( r => r.isSuccess ).getOrElse(results.last)
+
+  final def read(config: Config, path: Seq[String]): Result[A] =
+    reduce(path.map(read(config, _)))
+
 
   final def extract(config: Config, key: String = "extract"): Result[A] =
     get(config.atKey(key), key)
@@ -202,16 +204,24 @@ sealed abstract class ConfigReaderInstances extends ConfigReaderInstances0 {
   implicit def cbfJMapConfigReader[M[_, _], A, B](implicit C: ConfigReader[ju.Map[A, B]], F: Factory[(A, B), M[A, B]]): ConfigReader[M[A, B]] =
     C.map(c => F.fromSpecific(c.asScala))
 
-
-  implicit def optionConfigReader[A](implicit A: ConfigReader[A]): ConfigReader[Option[A]] =
-    ConfigReader.from { (c, p) =>
+  /**
+   * Reader for Option[A] must consider how to combine empty results for different naming strategies
+   */
+  implicit def optionConfigReader[A](implicit A: ConfigReader[A]): ConfigReader[Option[A]] = new ConfigReader[Option[A]] {
+    override def get(c: Config, p: String): Result[Option[A]] = {
       if (c.hasPathOrNull(p))
         A.read(c, p).map(Some(_)).handle {
           case ConfigError(ConfigError.NullValue(_, `p` :: Nil), es) if es.isEmpty => None
         }.popPath
       else
         Result.successful(None)
     }
+    override def reduce(results: Seq[Result[Option[A]]]): Result[Option[A]] =
+      // take first failure, then first success option that is defined, else first success option that is empty
+      results.find( r => r.isFailure)
+        .orElse(results.find( r => r.isSuccess && r.value.isDefined))
+        .getOrElse(results.last)
+  }
 
   implicit def javaOptionalConfigReader[A: ConfigReader]: ConfigReader[ju.Optional[A]] =
     optionConfigReader[A].map(_.fold(ju.Optional.empty[A]())(ju.Optional.of))

diff --git a/core/src/test/scala/configs/instance/CaseClassTypesTest.scala b/core/src/test/scala/configs/instance/CaseClassTypesTest.scala
@@ -5,9 +5,13 @@ import configs.{ConfigKeyNaming, ConfigReader}
 import scalaprops.Property.forAll
 import scalaprops.Scalaprops
 
-object CaseClassTypesTest extends Scalaprops {
+case class TestClass(myAttr1: String, myAttr2: String, testSeal: Option[TestSeal])
+case class ComplexClass(complexAttr: TestClass, myAttr3: String, myAttr4: String)
+sealed trait TestSeal
+case class Seal1(myAttr1: Option[String] = None, myAttr2: String) extends TestSeal
+case class Seal0() extends TestSeal
 
-  case class TestClass(myAttr1: String, myAttr2: String)
+object CaseClassTypesTest extends Scalaprops {
 
   val caseClassMultiNaming = {
     implicit val naming = ConfigKeyNaming.lowerCamelCase[TestClass].or(ConfigKeyNaming.hyphenSeparated[TestClass].apply)
@@ -19,8 +23,36 @@ object CaseClassTypesTest extends Scalaprops {
       """
       val config = ConfigFactory.parseString(configStr)
       val d = reader.extract(config)
-      println(d)
-      d.isSuccess
+      d.isSuccess &&
+        d.value.myAttr1.contains("test") &&
+        d.value.myAttr2.contains("test")
+    }
+  }
+
+  val caseClassComplexMultiNaming = {
+    // generic default naming
+    implicit def myDefaultNaming[A]: ConfigKeyNaming[A] =
+      ConfigKeyNaming.hyphenSeparated[A].or(ConfigKeyNaming.lowerCamelCase[A].apply)
+    forAll {
+      val reader = ConfigReader.derive[ComplexClass]
+      val configStr = """
+          complexAttr = {
+            my-attr-1 = test
+            myAttr2 = test
+            test-seal = {
+              type = Seal1
+              myAttr1 = test
+              my-attr-2 = test
+            }
+          }
+          my-attr-3 = test
+          myAttr4 = test
+      """
+      val config = ConfigFactory.parseString(configStr)
+      val d = reader.extract(config)
+      d.isSuccess &&
+        d.value.complexAttr.testSeal.get.asInstanceOf[Seal1].myAttr1.contains("test") &&
+        d.value.complexAttr.testSeal.get.asInstanceOf[Seal1].myAttr2.contains("test")
     }
   }
 }