Fix bug when overriding configurations

This commit fixes (and adds a test for) a bug
where overriding a value in our configuration
reset the rest of the configuration to the
default values.
This commit is contained in:
Torkel Rogstad 2019-06-05 18:35:37 +02:00
parent e02e1a0fe6
commit 7a4c3c6b71
5 changed files with 66 additions and 13 deletions

View file

@ -12,12 +12,12 @@ import scala.concurrent.Promise
import scala.util.Success import scala.util.Success
import scala.util.Failure import scala.util.Failure
case class ChainAppConfig(val confs: Config*) extends AppConfig { case class ChainAppConfig(private val confs: Config*) extends AppConfig {
override protected val configOverrides: List[Config] = confs.toList override protected val configOverrides: List[Config] = confs.toList
override protected val moduleName: String = "chain" override protected val moduleName: String = "chain"
override protected type ConfigType = ChainAppConfig override protected type ConfigType = ChainAppConfig
override protected def newConfigOfType( override protected def newConfigOfType(configs: Seq[Config]): ChainAppConfig =
configs: List[Config]): ChainAppConfig = ChainAppConfig(configs: _*) ChainAppConfig(configs: _*)
/** /**
* Checks whether or not the chain project is initialized by * Checks whether or not the chain project is initialized by

View file

@ -54,7 +54,7 @@ abstract class AppConfig extends BitcoinSLogger {
protected type ConfigType <: AppConfig protected type ConfigType <: AppConfig
/** Constructor to make a new instance of this config type */ /** Constructor to make a new instance of this config type */
protected def newConfigOfType(configOverrides: List[Config]): ConfigType protected def newConfigOfType(configOverrides: Seq[Config]): ConfigType
/** List of user-provided configs that should /** List of user-provided configs that should
* override defaults * override defaults
@ -88,9 +88,23 @@ abstract class AppConfig extends BitcoinSLogger {
logger.debug(oldConfStr) logger.debug(oldConfStr)
} }
val newConf = newConfigOfType( val configOverrides = firstOverride +: configs
configOverrides = List(firstOverride) ++ configs if (logger.isTraceEnabled()) {
) configOverrides.zipWithIndex.foreach {
case (c, idx) => logger.trace(s"Override no. $idx: ${c.asReadableJson}")
}
}
val newConf = {
// the idea here is that after resolving the configuration,
// we extract the value under the 'bitcoin-s' key and use
// that as our config. here we have to do the reverse, to
// get the keys to resolve correctly
val reconstructedStr = s"""
bitcoin-s: ${this.config.asReadableJson}
"""
val reconstructed = ConfigFactory.parseString(reconstructedStr)
newConfigOfType(reconstructed +: configOverrides)
}
// to avoid non-necessary lazy load // to avoid non-necessary lazy load
if (logger.isDebugEnabled()) { if (logger.isDebugEnabled()) {
@ -229,7 +243,8 @@ abstract class AppConfig extends BitcoinSLogger {
.reduce(_.withFallback(_)) .reduce(_.withFallback(_))
val interestingOverrides = overrides.getConfig("bitcoin-s") val interestingOverrides = overrides.getConfig("bitcoin-s")
logger.trace(s"User-overrides for bitcoin-s config:") logger.trace(
s"${configOverrides.length} user-overrides for bitcoin-s config:")
logger.trace(interestingOverrides.asReadableJson) logger.trace(interestingOverrides.asReadableJson)
// to make the overrides actually override // to make the overrides actually override
@ -282,7 +297,7 @@ object AppConfig extends BitcoinSLogger {
*/ */
private[bitcoins] def throwIfDefaultDatadir(config: AppConfig): Unit = { private[bitcoins] def throwIfDefaultDatadir(config: AppConfig): Unit = {
val datadirStr = config.datadir.toString() val datadirStr = config.datadir.toString()
AppConfig.defaultDatadirRegex.findFirstMatchIn(datadirStr) match { defaultDatadirRegex.findFirstMatchIn(datadirStr) match {
case None => () // pass case None => () // pass
case Some(_) => case Some(_) =>
val errMsg = val errMsg =
@ -291,6 +306,8 @@ object AppConfig extends BitcoinSLogger {
s"Your data directory is $datadirStr. This would cause tests to potentially", s"Your data directory is $datadirStr. This would cause tests to potentially",
"overwrite your existing data, which you probably don't want." "overwrite your existing data, which you probably don't want."
).mkString(" ") ).mkString(" ")
logger.error(errMsg)
logger.error(s"Configuration: ${config.config.asReadableJson}")
throw new RuntimeException(errMsg) throw new RuntimeException(errMsg)
} }
} }

View file

@ -8,11 +8,11 @@ import org.bitcoins.node.db.NodeDbManagement
import scala.util.Failure import scala.util.Failure
import scala.util.Success import scala.util.Success
case class NodeAppConfig(confs: Config*) extends AppConfig { case class NodeAppConfig(private val confs: Config*) extends AppConfig {
override val configOverrides: List[Config] = confs.toList override val configOverrides: List[Config] = confs.toList
override protected def moduleName: String = "node" override protected def moduleName: String = "node"
override protected type ConfigType = NodeAppConfig override protected type ConfigType = NodeAppConfig
override protected def newConfigOfType(configs: List[Config]): NodeAppConfig = override protected def newConfigOfType(configs: Seq[Config]): NodeAppConfig =
NodeAppConfig(configs: _*) NodeAppConfig(configs: _*)
/** /**

View file

@ -8,6 +8,8 @@ import com.typesafe.config.ConfigFactory
import org.bitcoins.core.config.RegTest import org.bitcoins.core.config.RegTest
import org.bitcoins.core.config.MainNet import org.bitcoins.core.config.MainNet
import org.bitcoins.wallet.config.WalletAppConfig import org.bitcoins.wallet.config.WalletAppConfig
import java.nio.file.Paths
import org.bitcoins.core.hd.HDPurposes
class WalletAppConfigTest extends BitcoinSUnitTest { class WalletAppConfigTest extends BitcoinSUnitTest {
val config = WalletAppConfig() val config = WalletAppConfig()
@ -24,6 +26,40 @@ class WalletAppConfigTest extends BitcoinSUnitTest {
assert(mainnet.network == MainNet) assert(mainnet.network == MainNet)
} }
it should "not matter how the overrides are passed in" in {
val dir = Paths.get("/", "bar", "biz")
val overrider = ConfigFactory.parseString(s"""
|bitcoin-s {
| datadir = $dir
| network = mainnet
|}
|""".stripMargin)
val throughConstuctor = WalletAppConfig(overrider)
val throughWithOverrides = config.withOverrides(overrider)
assert(throughWithOverrides.network == MainNet)
assert(throughWithOverrides.network == throughConstuctor.network)
assert(throughWithOverrides.datadir.startsWith(dir))
assert(throughWithOverrides.datadir == throughConstuctor.datadir)
}
it must "be overridable without screwing up other options" in {
val dir = Paths.get("/", "foo", "bar")
val otherConf = ConfigFactory.parseString(s"bitcoin-s.datadir = $dir")
val thirdConf = ConfigFactory.parseString(
s"bitcoin-s.wallet.defaultAccountType = nested-segwit")
val overriden = config.withOverrides(otherConf)
val twiceOverriden = overriden.withOverrides(thirdConf)
assert(overriden.datadir.startsWith(dir))
assert(twiceOverriden.datadir.startsWith(dir))
assert(twiceOverriden.defaultAccountKind == HDPurposes.NestedSegWit)
}
it must "be overridable with multiple levels" in { it must "be overridable with multiple levels" in {
val testnet = ConfigFactory.parseString("bitcoin-s.network = testnet3") val testnet = ConfigFactory.parseString("bitcoin-s.network = testnet3")
val mainnet = ConfigFactory.parseString("bitcoin-s.network = mainnet") val mainnet = ConfigFactory.parseString("bitcoin-s.network = mainnet")

View file

@ -10,11 +10,11 @@ import java.nio.file.Files
import org.bitcoins.core.hd.HDPurpose import org.bitcoins.core.hd.HDPurpose
import org.bitcoins.core.hd.HDPurposes import org.bitcoins.core.hd.HDPurposes
case class WalletAppConfig(conf: Config*) extends AppConfig { case class WalletAppConfig(private val conf: Config*) extends AppConfig {
override val configOverrides: List[Config] = conf.toList override val configOverrides: List[Config] = conf.toList
override def moduleName: String = "wallet" override def moduleName: String = "wallet"
override type ConfigType = WalletAppConfig override type ConfigType = WalletAppConfig
override def newConfigOfType(configs: List[Config]): WalletAppConfig = override def newConfigOfType(configs: Seq[Config]): WalletAppConfig =
WalletAppConfig(configs: _*) WalletAppConfig(configs: _*)
lazy val defaultAccountKind: HDPurpose = lazy val defaultAccountKind: HDPurpose =