Skip to content

Commit

Permalink
Bug: ConfigWriter minimizer does not omit nested unrequired nodes. (#…
Browse files Browse the repository at this point in the history 
…2058)

ConfigWriter properly excludes unrequired parameters
  • Loading branch information
@Caparow
Caparow authored Feb 22, 2024
1 parent a876b43 commit 35366c8
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 69 deletions.
120 changes: 52 additions & 68 deletions distage/distage-framework/.jvm/src/main/scala/izumi/distage/roles/bundled/ConfigWriter.scala
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package izumi.distage.roles.bundled

import com.typesafe.config.{Config, ConfigFactory, ConfigRenderOptions}
import com.typesafe.config.{Config, ConfigObject, ConfigRenderOptions}
import distage.config.AppConfig
import distage.{BootstrapModuleDef, Plan}
import izumi.distage.config.codec.ConfigMeta
import izumi.distage.config.model.ConfTag
import izumi.distage.framework.services.{ConfigMerger, RoleAppPlanner}
import izumi.distage.model.definition.Id
import izumi.distage.model.plan.ExecutableOp
import izumi.distage.model.plan.operations.OperationOrigin
import izumi.distage.roles.bundled.ConfigWriter.{ConfigPath, ExtractConfigPath, WriteReference}
import izumi.distage.roles.bundled.ConfigWriter.{ConfigPath, WriteReference}
import izumi.distage.roles.model.meta.{RoleBinding, RolesInfo}
import izumi.distage.roles.model.{RoleDescriptor, RoleTask}
import izumi.functional.quasi.QuasiIO
Expand Down Expand Up @@ -76,6 +76,7 @@ final class ConfigWriter[F[_]](
val loaded = index(roleId)

// TODO: mergeFilter considers system properties, we might want to AVOID that in configwriter
subLogger.info(s"About to output configs...")
val mergedRoleConfig = configMerger.mergeFilter(appConfig.shared, List(loaded), _ => true, "configwriter")
writeConfig(options, fileNameFull, mergedRoleConfig, subLogger)

Expand Down Expand Up @@ -119,9 +120,38 @@ final class ConfigWriter[F[_]](
.reboot(bootstrapOverride, Some(correctedAppConfig))
.makePlan(Set(roleDIKey))

def getConfig(plan: Plan): Iterator[ConfigPath] = {
plan.stepsUnordered.iterator.collect {
case ExtractConfigPath(path) => path
def getConfig(plan: Plan): Seq[ConfigPath] = {
val configTags = plan.stepsUnordered.toSeq.flatMap {
op =>
op.origin.value match {
case defined: OperationOrigin.Defined =>
defined.binding.tags.collect {
case t: ConfTag =>
t
}
case _ =>
Seq.empty
}
}

val paths = configTags.flatMap(t => unpack(Seq(t.confPath), t.fieldsMeta))
paths
}

def unpack(path: Seq[String], meta: ConfigMeta): Seq[ConfigPath] = {
meta match {
case ConfigMeta.ConfigMetaCaseClass(fields) =>
fields.flatMap {
case (name, meta) =>
unpack(path :+ name, meta)
}
case ConfigMeta.ConfigMetaSealedTrait(branches) =>
branches.toSeq.flatMap {
case (name, meta) =>
unpack(path :+ name, meta)
}
case ConfigMeta.ConfigMetaEmpty() => Seq(ConfigPath(path.mkString(".")))
case ConfigMeta.ConfigMetaUnknown() => Seq(ConfigPath(path.mkString(".")))
}
}

Expand All @@ -136,49 +166,6 @@ final class ConfigWriter[F[_]](
}
}

// private[this] def buildConfig(config: WriteReference, cmp: ConfigurableComponent): Config = {
// val referenceConfig = s"${cmp.roleId}-reference.conf"
// logger.info(s"[${cmp.roleId}] Resolving $referenceConfig... with ${config.includeCommon -> "shared sections"}")
//
// val reference = Value(ConfigFactory.parseResourcesAnySyntax(referenceConfig))
// .mut(cmp.parent.filter(_ => config.includeCommon))(_.withFallback(_))
// .get
// .resolve()
//
// if (reference.isEmpty) {
// logger.warn(s"[${cmp.roleId}] Reference config is empty.")
// }
//
// val resolved = ConfigFactory
// .systemProperties()
// .withFallback(reference)
// .resolve()
//
// val filtered = cleanupEffectiveAppConfig(resolved, reference)
// filtered.checkValid(reference)
// filtered
// }
//
//
//
// // TODO: sdk?
// @nowarn("msg=Unused import")
// private[this] def cleanupEffectiveAppConfig(effectiveAppConfig: Config, reference: Config): Config = {
// import scala.collection.compat._
// import scala.jdk.CollectionConverters._
//
// ConfigFactory.parseMap(effectiveAppConfig.root().unwrapped().asScala.view.filterKeys(reference.hasPath).toMap.asJava)
// }
//
// private[this] def outputFileName(service: String, version: Option[ArtifactVersion], asJson: Boolean, suffix: Option[String]): String = {
// val extension = if (asJson) "json" else "conf"
// val vstr = version.map(_.version).getOrElse("0.0.0-UNKNOWN")
// val suffixStr = suffix.fold("")("-" + _)
//
// s"$service$suffixStr-$vstr.$extension"
// }
//

private[this] def writeConfig(options: WriteReference, fileName: String, typesafeConfig: Config, subLogger: IzLogger): Try[Unit] = {
val configRenderOptions = ConfigRenderOptions.defaults.setOriginComments(false).setComments(false)

Expand Down Expand Up @@ -243,32 +230,29 @@ object ConfigWriter extends RoleDescriptor {

@nowarn("msg=Unused import")
def minimized(requiredPaths: Set[ConfigPath], source: Config): Config = {
import scala.collection.compat.*
import scala.jdk.CollectionConverters.*

val paths = requiredPaths.map(_.toPath)

ConfigFactory.parseMap {
source
.root().unwrapped().asScala
.view
.filterKeys(key => paths.exists(_.startsWith(key)))
.toMap
.asJava
}
}

object ExtractConfigPath {
def unapply(op: ExecutableOp): Option[ConfigPath] = {
op.origin.value match {
case defined: OperationOrigin.Defined =>
defined.binding.tags.collectFirst {
case ConfTag(path) => ConfigPath(path)
def filter(path: Seq[String], config: ConfigObject): ConfigObject = {
config.entrySet().asScala.foldLeft(config) {
case (c, e) =>
val key = e.getKey
val npath = path :+ key
val pathKey = npath.mkString(".")
if (paths.contains(pathKey) || paths.exists(p => p.startsWith(pathKey + "."))) {
e.getValue match {
case configObject: ConfigObject => c.withValue(key, filter(npath, configObject))
case _ => c
}
} else {
c.withoutKey(key)
}
case _ =>
None
}
}

val filtered = filter(Seq.empty, source.root()).toConfig
filtered
}

final case class ConfigPath(parts: Seq[String]) {
Expand Down
3 changes: 3 additions & 0 deletions distage/distage-framework/.jvm/src/test/resources/testrole00-reference.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ testservice {

systemPropInt: 222
systemPropList: [1, 2, 3]
unrequiredEntry {
value: "this will be omitted"
}
}

integrationOnlyCfg {
Expand Down
4 changes: 3 additions & 1 deletion distage/distage-framework/.jvm/src/test/scala/izumi/distage/roles/test/RoleAppTest.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,13 @@ class RoleAppTest extends AnyWordSpec with WithProperties {
assert(role0CfgMin.exists(), s"$role0CfgMin exists")
assert(role0Cfg.length() > role0CfgMin.length())

val role0CfgMinParsed = ConfigFactory.parseString(new String(Files.readAllBytes(role0CfgMin.toPath), UTF_8))
val cfgContent = new String(Files.readAllBytes(role0CfgMin.toPath), UTF_8)
val role0CfgMinParsed = ConfigFactory.parseString(cfgContent)

assert(!role0CfgMinParsed.hasPath("unrequiredEntry"))
assert(!role0CfgMinParsed.hasPath("logger"))
assert(!role0CfgMinParsed.hasPath("listconf"))
assert(!role0CfgMinParsed.hasPath("testservice.unrequiredEntry"))

assert(role0CfgMinParsed.hasPath("integrationOnlyCfg"))
assert(role0CfgMinParsed.hasPath("integrationOnlyCfg2"))
Expand Down

0 comments on commit 35366c8

Please sign in to comment.