Skip to content

Commit

Permalink
Stop using null to represent missing values in cursors (#791)
Browse files Browse the repository at this point in the history
  • Loading branch information
ruippeixotog committed Jul 12, 2020
1 parent 0fd3ad5 commit 8144f29
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 64 deletions.
27 changes: 14 additions & 13 deletions core/src/main/scala/pureconfig/BasicReaders.scala
Expand Up @@ -170,30 +170,31 @@ trait NumericReaders {
trait TypesafeConfigReaders {

implicit val configConfigReader: ConfigReader[Config] =
ConfigReader.fromCursor(_.asObjectCursor.right.map(_.value.toConfig))
ConfigReader.fromCursor(_.asObjectCursor.right.map(_.objValue.toConfig))

implicit val configObjectConfigReader: ConfigReader[ConfigObject] =
ConfigReader.fromCursor(_.asObjectCursor.right.map(_.value))
ConfigReader.fromCursor(_.asObjectCursor.right.map(_.objValue))

implicit val configValueConfigReader: ConfigReader[ConfigValue] =
ConfigReader.fromCursor(c => Right(c.value))
ConfigReader.fromCursor(_.asConfigValue)

implicit val configListConfigReader: ConfigReader[ConfigList] =
ConfigReader.fromCursor(_.asListCursor.right.map(_.value))
ConfigReader.fromCursor(_.asListCursor.right.map(_.listValue))

// TODO: improve error handling by copying the parsing logic from Typesafe and making it use PureConfig Results
// (see PeriodUtils and DurationUtils)
implicit val configMemorySizeReader: ConfigReader[ConfigMemorySize] =
ConfigReader
.fromCursor { cur =>
cur scopeFailure {
ConvertHelpers.tryToEither {
Try {
val wrapped = cur.value.atKey("_")
val bytes = wrapped.getBytes("_").longValue()
ConfigMemorySize ofBytes bytes
}
ConfigReader.fromCursor { cur =>
cur.scopeFailure {
ConvertHelpers.tryToEither {
Try {
val wrapped = cur.asConfigValue.right.get.atKey("_")
val bytes = wrapped.getBytes("_").longValue()
ConfigMemorySize.ofBytes(bytes)
}
}
}
}
}

/**
Expand Down
86 changes: 56 additions & 30 deletions core/src/main/scala/pureconfig/ConfigCursor.scala
Expand Up @@ -15,9 +15,9 @@ import pureconfig.error._
sealed trait ConfigCursor {

/**
* The `ConfigValue` to which this cursor points to.
* The optional `ConfigValue` which this cursor points to.
*/
def value: ConfigValue
def valueOpt: Option[ConfigValue]

/**
* The path in the config to which this cursor points as a list of keys in reverse order (deepest key first).
Expand All @@ -32,23 +32,36 @@ sealed trait ConfigCursor {
/**
* The file system location of the config to which this cursor points.
*/
def origin: Option[ConfigOrigin] = Option(value).map(_.origin())
def origin: Option[ConfigOrigin] = valueOpt.map(_.origin())

/**
* Returns whether this cursor points to an undefined value. A cursor can point to an undefined value when a missing
* config key is requested or when a `null` `ConfigValue` is provided, among other reasons.
*
* @return `true` if this cursor points to an undefined value, `false` otherwise.
*/
def isUndefined: Boolean = value == null
def isUndefined: Boolean = valueOpt.isEmpty

/**
* Returns whether this cursor points to a `null` config value. An explicit `null` value is different than a missing
* value - `isUndefined` can be used to check for the latter.
*
* @return `true` if this cursor points to a `null` value, `false` otherwise.
*/
def isNull: Boolean = value != null && value.unwrapped == null
def isNull: Boolean = valueOpt.exists(_.unwrapped == null)

/**
* Casts this cursor to a `ConfigValue`.
*
* @return a `Right` with the config value pointed to by this cursor if the value is defined, `Left` with a list of
* failures otherwise.
*/
def asConfigValue: ConfigReader.Result[ConfigValue] = {
valueOpt match {
case Some(value) => Right(value)
case None => failed(KeyNotFound.forKeys(path, Set()))
}
}

/**
* Casts this cursor to a string.
Expand Down Expand Up @@ -224,18 +237,15 @@ sealed trait ConfigCursor {
*/
@deprecated("Use `asListCursor` and/or `asObjectCursor` instead", "0.10.1")
def asCollectionCursor: ConfigReader.Result[Either[ConfigListCursor, ConfigObjectCursor]] = {
if (isUndefined) {
failed(KeyNotFound.forKeys(path, Set()))
} else {
asConfigValue.right.flatMap { value =>
val listAtLeft = asListCursor.right.map(Left.apply)
lazy val mapAtRight = asObjectCursor.right.map(Right.apply)
listAtLeft.left.flatMap(_ => mapAtRight).left.flatMap(_ => failed(WrongType(value.valueType, Set(LIST, OBJECT))))
}
}

def fluent: FluentConfigCursor =
if (isUndefined) FluentConfigCursor(failed(KeyNotFound.forKeys(path, Set())))
else FluentConfigCursor(Right(this))
FluentConfigCursor(asConfigValue.right.map(_ => this))

/**
* Returns a failed `ConfigReader` result resulting from scoping a `FailureReason` into the context of this cursor.
Expand Down Expand Up @@ -280,10 +290,9 @@ sealed trait ConfigCursor {
cast: ConfigValue => Either[FailureReason, A]
): ConfigReader.Result[A] = {

if (isUndefined)
failed(KeyNotFound.forKeys(path, Set()))
else
asConfigValue.right.flatMap { value =>
scopeFailure(ConfigCursor.transform(value, expectedType).right.flatMap(cast))
}
}
}

Expand All @@ -296,7 +305,16 @@ object ConfigCursor {
* @param pathElems the path of `value` in the config as a list of keys
* @return a `ConfigCursor` for `value` at the path given by `pathElems`.
*/
def apply(value: ConfigValue, pathElems: List[String]): ConfigCursor = SimpleConfigCursor(value, pathElems)
def apply(value: ConfigValue, pathElems: List[String]): ConfigCursor = SimpleConfigCursor(Option(value), pathElems)

/**
* Builds a `ConfigCursor` for an optional `ConfigValue` at a path.
*
* @param value the optional `ConfigValue` to which the cursor should point
* @param pathElems the path of `value` in the config as a list of keys
* @return a `ConfigCursor` for `value` at the path given by `pathElems`.
*/
def apply(value: Option[ConfigValue], pathElems: List[String]): ConfigCursor = SimpleConfigCursor(value, pathElems)

/**
* Handle automatic type conversions of `ConfigValue`s the way the
Expand Down Expand Up @@ -359,25 +377,29 @@ object ConfigCursor {
/**
* A simple `ConfigCursor` providing no extra operations.
*/
case class SimpleConfigCursor(value: ConfigValue, pathElems: List[String]) extends ConfigCursor
case class SimpleConfigCursor(valueOpt: Option[ConfigValue], pathElems: List[String]) extends ConfigCursor

/**
* A `ConfigCursor` pointing to a config list.
*/
case class ConfigListCursor(value: ConfigList, pathElems: List[String], offset: Int = 0) extends ConfigCursor {
case class ConfigListCursor(listValue: ConfigList, pathElems: List[String], offset: Int = 0) extends ConfigCursor {

@inline private[this] def validIndex(idx: Int) = idx >= 0 && idx < size
@inline private[this] def indexKey(idx: Int) = (offset + idx).toString

def valueOpt: Option[ConfigList] = Some(listValue)

override def asConfigValue: ConfigReader.Result[ConfigList] = Right(listValue)

/**
* Returns whether the config list pointed to by this cursor is empty.
*/
def isEmpty: Boolean = value.isEmpty
def isEmpty: Boolean = listValue.isEmpty

/**
* Returns the size of the config list pointed to by this cursor.
*/
def size: Int = value.size
def size: Int = listValue.size

/**
* Returns a cursor to the config at a given index.
Expand All @@ -400,19 +422,19 @@ case class ConfigListCursor(value: ConfigList, pathElems: List[String], offset:
* @return a cursor to the config at `idx` if such a config exists, a cursor to an undefined value otherwise.
*/
def atIndexOrUndefined(idx: Int): ConfigCursor =
ConfigCursor(if (validIndex(idx)) value.get(idx) else null, indexKey(idx) :: pathElems)
ConfigCursor(if (validIndex(idx)) Some(listValue.get(idx)) else None, indexKey(idx) :: pathElems)

/**
* Returns a cursor to the tail of the config list pointed to by this cursor if non-empty.
*
* @return a `Some` with the tail of the config list if the list is not empty, `None` otherwise.
*/
def tailOption: Option[ConfigListCursor] = {
if (value.isEmpty) None
if (listValue.isEmpty) None
else {
val newValue = ConfigValueFactory
.fromAnyRef(value.asScala.drop(1).asJava)
.withOrigin(value.origin)
.fromAnyRef(listValue.asScala.drop(1).asJava)
.withOrigin(listValue.origin)
.asInstanceOf[ConfigList]

Some(ConfigListCursor(newValue, pathElems, offset = offset + 1))
Expand All @@ -425,7 +447,7 @@ case class ConfigListCursor(value: ConfigList, pathElems: List[String], offset:
* @return a list of cursors to the elements of the config list pointed to by this cursor.
*/
def list: List[ConfigCursor] =
value.asScala.toList.drop(offset).zipWithIndex.map {
listValue.asScala.toList.drop(offset).zipWithIndex.map {
case (cv, idx) => ConfigCursor(cv, indexKey(idx) :: pathElems)
}

Expand All @@ -436,23 +458,27 @@ case class ConfigListCursor(value: ConfigList, pathElems: List[String], offset:
/**
* A `ConfigCursor` pointing to a config object.
*/
case class ConfigObjectCursor(value: ConfigObject, pathElems: List[String]) extends ConfigCursor {
case class ConfigObjectCursor(objValue: ConfigObject, pathElems: List[String]) extends ConfigCursor {

def valueOpt: Option[ConfigObject] = Some(objValue)

override def asConfigValue: ConfigReader.Result[ConfigObject] = Right(objValue)

/**
* Returns whether the config object pointed to by this cursor is empty.
*/
def isEmpty: Boolean = value.isEmpty
def isEmpty: Boolean = objValue.isEmpty

/**
* Returns the size of the config object pointed to by this cursor.
*/
def size: Int = value.size
def size: Int = objValue.size

/**
* Returns the list of keys of the config object pointed to by this cursor.
*/
def keys: Iterable[String] =
value.keySet.asScala
objValue.keySet.asScala

/**
* Returns a cursor to the config at a given key.
Expand All @@ -475,7 +501,7 @@ case class ConfigObjectCursor(value: ConfigObject, pathElems: List[String]) exte
* @return a cursor to the config at `key` if such a config exists, a cursor to an undefined value otherwise.
*/
def atKeyOrUndefined(key: String): ConfigCursor =
ConfigCursor(value.get(key), key :: pathElems)
ConfigCursor(objValue.get(key), key :: pathElems)

/**
* Returns a cursor to the object pointed to by this cursor without a given key.
Expand All @@ -484,13 +510,13 @@ case class ConfigObjectCursor(value: ConfigObject, pathElems: List[String]) exte
* @return a cursor to the object pointed to by this cursor without `key`.
*/
def withoutKey(key: String): ConfigObjectCursor =
ConfigObjectCursor(value.withoutKey(key), pathElems)
ConfigObjectCursor(objValue.withoutKey(key), pathElems)

/**
* Returns a map of cursors to the elements of the config object pointed to by this cursor.
*/
def map: Map[String, ConfigCursor] =
value.asScala.toMap.map { case (key, cv) => key -> ConfigCursor(cv, key :: pathElems) }
objValue.asScala.toMap.map { case (key, cv) => key -> ConfigCursor(cv, key :: pathElems) }

// Avoid unnecessary cast.
override def asObjectCursor: ConfigReader.Result[ConfigObjectCursor] = Right(this)
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/scala/pureconfig/ConfigReader.scala
Expand Up @@ -105,7 +105,7 @@ trait ConfigReader[A] {
* @return a `ConfigReader` returning the results of this reader when the input configs are mapped using `f`.
*/
def contramapConfig(f: ConfigValue => ConfigValue): ConfigReader[A] =
fromCursor[A] { cur => from(ConfigCursor(f(cur.value), cur.pathElems)) }
fromCursor[A] { cur => from(ConfigCursor(cur.valueOpt.map(f), cur.pathElems)) }

/**
* Applies a function to config cursors before passing them to this reader.
Expand Down Expand Up @@ -190,7 +190,7 @@ object ConfigReader extends BasicReaders with CollectionReaders with ProductRead
* @return a `ConfigReader` for reading objects of type `A` using `fromF`.
*/
def fromFunction[A](fromF: ConfigValue => ConfigReader.Result[A]) =
fromCursor(fromF.compose(_.value))
fromCursor(_.asConfigValue.right.flatMap(fromF))

def fromString[A](fromF: String => Either[FailureReason, A]): ConfigReader[A] =
ConfigReader.fromCursor(_.asString).emap(fromF)
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/scala/pureconfig/ConfigSource.scala
Expand Up @@ -321,7 +321,7 @@ object ConfigSource {
*/
private[pureconfig] def fromCursor(cur: ConfigCursor): ConfigSource =
new ConfigSource {
def value(): Result[ConfigValue] = Right(cur.value)
def value(): Result[ConfigValue] = cur.asConfigValue
override def cursor() = Right(cur)
}

Expand All @@ -333,7 +333,7 @@ object ConfigSource {
*/
private[pureconfig] def fromCursor(cur: FluentConfigCursor): ConfigSource =
new ConfigSource {
def value(): Result[ConfigValue] = cur.cursor.right.map(_.value)
def value(): Result[ConfigValue] = cur.cursor.right.flatMap(_.asConfigValue)
override def cursor() = cur.cursor
override def fluentCursor() = cur
}
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/docs/complex-types.md
Expand Up @@ -104,7 +104,7 @@ def extractByType(typ: String, objCur: ConfigObjectCursor): ConfigReader.Result[
case "class1" => class1Reader.from(objCur)
case "class2" => class2Reader.from(objCur)
case t =>
objCur.failed(CannotConvert(objCur.value.toString, "Identifiable",
objCur.failed(CannotConvert(objCur.objValue.toString, "Identifiable",
s"type has value $t instead of class1 or class2"))
}

Expand Down
Expand Up @@ -71,7 +71,9 @@ class FieldCoproductHint[A](key: String) extends CoproductHint[A] {
option <-
options
.find(valueStr == fieldValue(_))
.toRight(ConfigReaderFailures(valueCur.failureFor(UnexpectedValueForFieldCoproductHint(valueCur.value))))
.toRight(
ConfigReaderFailures(valueCur.failureFor(UnexpectedValueForFieldCoproductHint(valueCur.valueOpt.get)))
)
.right
} yield Use(objCur.withoutKey(key), option)
}
Expand Down Expand Up @@ -102,7 +104,9 @@ class FirstSuccessCoproductHint[A] extends CoproductHint[A] {
Attempt(
cursor,
options,
failures => ConfigReaderFailures(cursor.failureFor(NoValidCoproductOptionFound(cursor.value, failures)))
failures =>
cursor.asConfigValue
.fold(identity, v => ConfigReaderFailures(cursor.failureFor(NoValidCoproductOptionFound(v, failures))))
)
)

Expand Down
Expand Up @@ -31,7 +31,7 @@ class EnumCoproductHint[A] extends CoproductHint[A] {
cursor.asString.right.flatMap { str =>
options.find(str == fieldValue(_)) match {
case Some(opt) => Right(Use(cursor, opt))
case None => cursor.failed[CoproductHint.Action](NoValidCoproductOptionFound(cursor.value, Seq.empty))
case None => cursor.failed[CoproductHint.Action](NoValidCoproductOptionFound(cursor.valueOpt.get, Seq.empty))
}
}

Expand Down
Expand Up @@ -21,7 +21,8 @@ object EnumerationConfigReaderBuilder {
new EnumerationConfigReaderBuilder[CNil] {
def build(transformName: String => String): ConfigReader[CNil] =
new ConfigReader[CNil] {
def from(cur: ConfigCursor): Result[CNil] = cur.failed(NoValidCoproductOptionFound(cur.value, Seq.empty))
def from(cur: ConfigCursor): Result[CNil] =
cur.asConfigValue.right.flatMap(v => cur.failed(NoValidCoproductOptionFound(v, Seq.empty)))
}
}

Expand Down
Expand Up @@ -79,11 +79,13 @@ class ProductConvertersSuite extends BaseSuite {

implicit val defaultInt = new ConfigReader[Int] with ReadsMissingKeys {
def from(cur: ConfigCursor) =
if (cur.isUndefined) Right(42)
else {
val s = cur.value.render(ConfigRenderOptions.concise)
cur.scopeFailure(catchReadError(_.toInt)(implicitly)(s))
}
cur.asConfigValue.fold(
_ => Right(42),
v => {
val s = v.render(ConfigRenderOptions.concise)
cur.scopeFailure(catchReadError(_.toInt)(implicitly)(s))
}
)
}
ConfigReader[Conf].from(conf).right.value shouldBe Conf(1, 42)
}
Expand Down
Expand Up @@ -50,7 +50,7 @@ final class YamlConfigSource private (
* @return a config object source that produces YAML object documents read by this source
*/
def asObjectSource: ConfigObjectSource =
ConfigObjectSource(fluentCursor().asObjectCursor.right.map(_.value.toConfig))
ConfigObjectSource(fluentCursor().asObjectCursor.right.map(_.objValue.toConfig))

/**
* Returns a new source that produces a multi-document YAML read by this source as a config list.
Expand Down
2 changes: 1 addition & 1 deletion tests/src/test/scala/pureconfig/ConfigCursorSuite.scala
Expand Up @@ -194,7 +194,7 @@ class ConfigCursorSuite extends BaseSuite {
}

it should "handle in a safe way cursors to undefined values" in {
val cur = ConfigCursor(null, defaultPath)
val cur = ConfigCursor(None, defaultPath)
cur.path shouldBe defaultPathStr
cur.isUndefined shouldBe true
cur.isNull shouldBe false
Expand Down

0 comments on commit 8144f29

Please sign in to comment.