diff --git a/build.sbt b/build.sbt index ad03664..3a9327b 100644 --- a/build.sbt +++ b/build.sbt @@ -1,4 +1,4 @@ -lazy val tscfgVersion = setVersion("0.9.94") +lazy val tscfgVersion = setVersion("0.9.95") organization := "com.github.carueda" name := "tscfg" diff --git a/changelog.md b/changelog.md index dc0c080..25f4540 100644 --- a/changelog.md +++ b/changelog.md @@ -1,3 +1,9 @@ +2019-10-07 - 0.9.95 + +- fix #54 "Shared config objects" + https://github.com/carueda/tscfg/issues/54#issuecomment-539096913. + In short, the `@define`s are traversed first in ModelBuilder. + 2019-09-14 - 0.9.94 - resolve #54 "Shared config objects" diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf index f6ba53c..49cdc21 100644 --- a/src/main/resources/application.conf +++ b/src/main/resources/application.conf @@ -1 +1 @@ -tscfg.version = 0.9.94 +tscfg.version = 0.9.95 diff --git a/src/main/scala/tscfg/ModelBuilder.scala b/src/main/scala/tscfg/ModelBuilder.scala index 72c0c90..c38bee6 100644 --- a/src/main/scala/tscfg/ModelBuilder.scala +++ b/src/main/scala/tscfg/ModelBuilder.scala @@ -2,8 +2,8 @@ package tscfg import com.typesafe.config._ import tscfg.generators.tsConfigUtil -import tscfg.model.{AnnType, DURATION, ObjectType, SIZE} import tscfg.model.durations.ms +import tscfg.model.{DURATION, SIZE} import scala.collection.JavaConverters._ @@ -42,13 +42,18 @@ class ModelBuilder(assumeAllRequired: Boolean = false) { private val warns = collection.mutable.ArrayBuffer[Warning]() private def fromConfig(namespace: Namespace, conf: Config): model.ObjectType = { - // do two passes as lightbend config does not necessarily preserve member order: - val ot = fromConfig1(namespace, conf) - fromConfig2(namespace, ot) - } - - private def fromConfig1(namespace: Namespace, conf: Config): model.ObjectType = { val memberStructs = getMemberStructs(conf) + // have the `@define`s be traversed first: + .sortWith { case (childStruct, s2) ⇒ + if (childStruct.isLeaf) false + else if (s2.isLeaf) true + else { + val cv = conf.getValue(childStruct.name) + val comments = cv.origin().comments().asScala.toList + comments.exists(_.trim.startsWith("@define")) + } + } + val members: immutable.Map[String, model.AnnType] = memberStructs.map { childStruct ⇒ val name = childStruct.name val cv = conf.getValue(name) @@ -121,35 +126,6 @@ class ModelBuilder(assumeAllRequired: Boolean = false) { model.ObjectType(members) } - private def fromConfig2(namespace: Namespace, ot: model.ObjectType): model.ObjectType = { - val resolvedMembers = ot.members.map { case (name, annType) ⇒ - val modAnnType = annType.t match { - - case _:model.STRING.type ⇒ - annType.default match { - case Some(strValue) ⇒ - namespace.resolveDefine(strValue) match { - case Some(ort) ⇒ AnnType(ort) - case _ ⇒ annType - } - - case None ⇒ annType - } - - //// the following would be part of changes to allow recursive type - //case ot:ObjectType ⇒ - // val ot2 = fromConfig2(namespace, ot) - // AnnType(ot2) - - case _ ⇒ annType - } - - name → modAnnType - } - - model.ObjectType(resolvedMembers) - } - private case class Struct(name: String, members: mutable.HashMap[String, Struct] = mutable.HashMap.empty) { def isLeaf: Boolean = members.isEmpty // $COVERAGE-OFF$ diff --git a/src/main/scala/tscfg/Namespace.scala b/src/main/scala/tscfg/Namespace.scala index 4a557ea..2d53ae9 100644 --- a/src/main/scala/tscfg/Namespace.scala +++ b/src/main/scala/tscfg/Namespace.scala @@ -3,21 +3,15 @@ package tscfg import tscfg.model.{ObjectRefType, Type} object Namespace { - val root: Namespace = new Namespace("", None) - - def getAllDefines: Map[String, Type] = { - allDefines.toMap - } - - private def addToAllDefines(defineFullPath: String, t: Type): Unit = { - allDefines.update(defineFullPath, t) - } - - private val allDefines = collection.mutable.HashMap[String, Type]() + /** Returns a new, empty root namespace. */ + def root: Namespace = new Namespace("", None, + collection.mutable.HashMap[String, Type]()) } -class Namespace private(simpleName: String, parent: Option[Namespace]) { - import Namespace.addToAllDefines +class Namespace private(simpleName: String, parent: Option[Namespace], + allDefines: collection.mutable.HashMap[String, Type]) { + + def getAllDefines: Map[String, Type] = allDefines.toMap def getPath: Seq[String] = parent match { case None ⇒ Seq.empty @@ -26,7 +20,7 @@ class Namespace private(simpleName: String, parent: Option[Namespace]) { def getPathString: String = getPath.mkString(".") - def extend(simpleName: String): Namespace = new Namespace(simpleName, Some(this)) + def extend(simpleName: String): Namespace = new Namespace(simpleName, Some(this), allDefines) private val defineNames = collection.mutable.HashSet[String]() @@ -35,13 +29,14 @@ class Namespace private(simpleName: String, parent: Option[Namespace]) { assert(simpleName.nonEmpty) if (defineNames.contains(simpleName)) { - println(s"WARN: duplicate @define '$simpleName' in namespace $getPathString. Ignoring previous entry") + val ns = if (getPath.nonEmpty) s"'$getPathString'" else "(root)" + println(s"WARN: duplicate @define '$simpleName' in namespace $ns. Ignoring previous entry") // TODO include in build warnings } defineNames.add(simpleName) - addToAllDefines(resolvedFullPath(simpleName), t) + allDefines.update(resolvedFullPath(simpleName), t) } private def resolvedFullPath(simpleName: String): String = parent match { diff --git a/src/main/tscfg/example/issue54exampleD.spec.conf b/src/main/tscfg/example/issue54exampleD.spec.conf new file mode 100644 index 0000000..f8e9074 --- /dev/null +++ b/src/main/tscfg/example/issue54exampleD.spec.conf @@ -0,0 +1,8 @@ +#@define +Struct = { + a = int +} + +exampleD { + test = Struct +} diff --git a/src/main/tscfg/example/issue54exampleE.spec.conf b/src/main/tscfg/example/issue54exampleE.spec.conf new file mode 100644 index 0000000..f527c6c --- /dev/null +++ b/src/main/tscfg/example/issue54exampleE.spec.conf @@ -0,0 +1,8 @@ +#@define +Struct = { + a = int +} + +exampleE { + test = Struct +} diff --git a/src/test/scala/tscfg/NamespaceSpec.scala b/src/test/scala/tscfg/NamespaceSpec.scala index d4f1005..714b95b 100644 --- a/src/test/scala/tscfg/NamespaceSpec.scala +++ b/src/test/scala/tscfg/NamespaceSpec.scala @@ -22,7 +22,7 @@ class NamespaceSpec extends Specification { "add and resolve define" in { root.addDefine("RootDef1", objectType) root.resolveDefine("RootDef1") must beSome - Namespace.getAllDefines.keys must_== Set("RootDef1") + root.getAllDefines.keys must_== Set("RootDef1") } } @@ -35,7 +35,7 @@ class NamespaceSpec extends Specification { ns00.addDefine("n00def1", objectType) ns00.resolveDefine("n00def1") must beSome - Namespace.getAllDefines.keys must_== Set("RootDef1", "ns00.n00def1") + root.getAllDefines.keys must_== Set("RootDef1", "ns00.n00def1") } "resolve define in parent namespace" in { @@ -52,7 +52,7 @@ class NamespaceSpec extends Specification { ns000.addDefine("n000def1", objectType) ns000.resolveDefine("n000def1") must beSome - Namespace.getAllDefines.keys must_== Set( + root.getAllDefines.keys must_== Set( "RootDef1", "ns00.n00def1", "ns00.ns000.n000def1" ) } @@ -77,7 +77,7 @@ class NamespaceSpec extends Specification { "all defines" should { "resolve" in { - val all = Namespace.getAllDefines + val all = root.getAllDefines all.size must_== 3 all.get("RootDef1") must beSome(objectType) all.get("ns00.n00def1") must beSome(objectType) diff --git a/src/test/scala/tscfg/generators/java/JavaMainSpec.scala b/src/test/scala/tscfg/generators/java/JavaMainSpec.scala index d4ce60d..300d096 100644 --- a/src/test/scala/tscfg/generators/java/JavaMainSpec.scala +++ b/src/test/scala/tscfg/generators/java/JavaMainSpec.scala @@ -732,6 +732,36 @@ class JavaMainSpec extends Specification { } } + "issue 54 - shared config - exampleD" should { + "be handled" in { + val c = new JavaIssue54exampleDCfg(ConfigFactory.parseString( + """ + |exampleD { + | test { + | a = 1 + | } + |} + |""".stripMargin)) + + c.exampleD.test.a === 1 + } + } + + "issue 54 - shared config - exampleE" should { + "be handled" in { + val c = new JavaIssue54exampleECfg(ConfigFactory.parseString( + """ + |exampleE { + | test { + | a = 1 + | } + |} + |""".stripMargin)) + + c.exampleE.test.a === 1 + } + } + /* SKIP due to weird travis-ci issue (with the scala version, not the java one) "issue 54 - shared config - example2" should { "be handled" in { diff --git a/src/test/scala/tscfg/generators/scala/ScalaMainSpec.scala b/src/test/scala/tscfg/generators/scala/ScalaMainSpec.scala index 6d08fa1..974865c 100644 --- a/src/test/scala/tscfg/generators/scala/ScalaMainSpec.scala +++ b/src/test/scala/tscfg/generators/scala/ScalaMainSpec.scala @@ -641,6 +641,36 @@ class ScalaMainSpec extends Specification { } } + "issue 54 - shared config - exampleD" should { + "be handled" in { + val c = ScalaIssue54exampleDCfg(ConfigFactory.parseString( + """ + |exampleD { + | test { + | a = 1 + | } + |} + |""".stripMargin)) + + c.exampleD.test.a === 1 + } + } + + "issue 54 - shared config - exampleE" should { + "be handled" in { + val c = ScalaIssue54exampleECfg(ConfigFactory.parseString( + """ + |exampleE { + | test { + | a = 1 + | } + |} + |""".stripMargin)) + + c.exampleE.test.a === 1 + } + } + // for some strange reason ScalaIssue54bCfg keeps of getting wrongly generated on travis-ci: // [error] /home/travis/build/carueda/tscfg/src/test/scala/tscfg/example/ScalaIssue54bCfg.scala:8: not found: type Shared // [error] e : Shared,