Merge branch 'master' into file

core/src/main/scala/pureconfig/DurationConvert.scala

@@ -5,7 +5,9 @@ package pureconfig
 
 import pureconfig.error.{ CannotConvert, ConfigReaderFailure, ConfigValueLocation }
 
-import scala.concurrent.duration.{ Duration, FiniteDuration }
+import scala.concurrent.duration.Duration.{ Inf, MinusInf }
+import scala.concurrent.duration.{ DAYS, Duration, FiniteDuration, HOURS, MICROSECONDS, MILLISECONDS, MINUTES, NANOSECONDS, SECONDS, TimeUnit }
+import scala.util.Try
 
 /**
  * Utility functions for converting a Duration to a String and vice versa.
@@ -17,14 +19,61 @@ private[pureconfig] object DurationConvert {
   val fromString: String => Option[ConfigValueLocation] => Either[ConfigReaderFailure, Duration] = { string => location =>
     if (string == UndefinedDuration) Right(Duration.Undefined)
     else try {
-      Right(Duration(addZeroUnit(justAMinute(itsGreekToMe(string)))))
+      Right(parseDuration(addZeroUnit(justAMinute(itsGreekToMe(string)))))
     } catch {
       case ex: NumberFormatException =>
         val err = s"${ex.getMessage}. (try a number followed by any of ns, us, ms, s, m, h, d)"
         Left(CannotConvert(string, "Duration", err, location, None))
     }
   }
 
+  ////////////////////////////////////
+  // This is a copy of Duration(str: String) that fixes the bug on precision
+  //
+
+  // "ms milli millisecond" -> List("ms", "milli", "millis", "millisecond", "milliseconds")
+  private[this] def words(s: String) = (s.trim split "\\s+").toList
+  private[this] def expandLabels(labels: String): List[String] = {
+    val hd :: rest = words(labels)
+    hd :: rest.flatMap(s => List(s, s + "s"))
+  }
+
+  private[this] val timeUnitLabels = List(
+    DAYS -> "d day",
+    HOURS -> "h hour",
+    MINUTES -> "min minute",
+    SECONDS -> "s sec second",
+    MILLISECONDS -> "ms milli millisecond",
+    MICROSECONDS -> "µs micro microsecond",
+    NANOSECONDS -> "ns nano nanosecond")
+
+  // Label => TimeUnit
+  protected[pureconfig] val timeUnit: Map[String, TimeUnit] =
+    timeUnitLabels.flatMap { case (unit, names) => expandLabels(names) map (_ -> unit) }.toMap
+
+  private[pureconfig] def parseDuration(s: String): Duration = {
+    val s1: String = s filterNot (_.isWhitespace)
+    s1 match {
+      case "Inf" | "PlusInf" | "+Inf" => Inf
+      case "MinusInf" | "-Inf" => MinusInf
+      case _ =>
+        val unitName = s1.reverse.takeWhile(_.isLetter).reverse
+        timeUnit get unitName match {
+          case Some(unit) =>
+            val valueStr = s1 dropRight unitName.length
+            // Reading Long first avoids losing precision unnecessarily
+            Try(Duration(java.lang.Long.parseLong(valueStr), unit)).getOrElse {
+              // But if the value is a fractional number, then we have to parse it 
+              // as a Double, which will lose precision and possibly change the units. 
+              Duration(java.lang.Double.parseDouble(valueStr), unit)
+            }
+          case _ => throw new NumberFormatException("format error " + s)
+        }
+    }
+  }
+
+  ////////////////////////////////////
+
   private val zeroRegex = "\\s*[+-]?0+\\s*$".r
   private val fauxMuRegex = "([0-9])(\\s*)us(\\s*)$".r
   private val shortMinuteRegex = "([0-9])(\\s*)m(\\s*)$".r

core/src/test/scala/pureconfig/BasicConvertersSuite.scala

@@ -17,6 +17,8 @@ import scala.concurrent.duration.{ Duration, FiniteDuration }
 
 class BasicConvertersSuite extends BaseSuite {
 
+  implicit override val generatorDrivenConfig = PropertyCheckConfiguration(minSuccessful = 100)
+
   behavior of "ConfigConvert"
 
   checkArbitrary[Duration]

core/src/test/scala/pureconfig/DurationConvertSuite.scala

@@ -7,10 +7,10 @@ import org.scalatest.{ EitherValues, FlatSpec, Matchers }
 import org.scalatest.Inspectors._
 import pureconfig.error.CannotConvert
 
-import scala.concurrent.duration.Duration
+import scala.concurrent.duration._
+import DurationConvertSuite._
 
 class DurationConvertSuite extends FlatSpec with Matchers with EitherValues {
-  import DurationConvert.{ fromDuration => fromD }
   "Converting a Duration to a String" should "pick an appropriate unit when dealing with whole units less than the next step up" in {
     fromD(Duration(14, TimeUnit.DAYS)) shouldBe "14d"
     fromD(Duration(16, TimeUnit.HOURS)) shouldBe "16h"
@@ -40,7 +40,6 @@ class DurationConvertSuite extends FlatSpec with Matchers with EitherValues {
     fromS(fromD(expected)).right.value shouldBe expected
   }
 
-  val fromS = DurationConvert.fromString(_: String)(None)
   "Converting a String to a Duration" should "succeed for known units" in {
     fromS("1d") shouldBe Right(Duration(1, TimeUnit.DAYS))
     fromS("47h") shouldBe Right(Duration(47, TimeUnit.HOURS))
@@ -91,4 +90,36 @@ class DurationConvertSuite extends FlatSpec with Matchers with EitherValues {
   it should "correctly round trip when converting Duration.Inf" in {
     fromS(fromD(Duration.Inf)) shouldEqual Right(Duration.Inf)
   }
+  it should "convert a value larger than 2^52" in {
+    fromS("8092048641075763 ns").right.value shouldBe Duration(8092048641075763L, NANOSECONDS)
+  }
+  it should "round trip a value which is greater than 2^52" in {
+    val expected = Duration(781251341142500992L, NANOSECONDS)
+    fromS(fromD(expected)).right.value shouldBe expected
+  }
+  it should "round trip a value < 2^52 which is > 2^52 when converted to milliseconds" in {
+    val expected = Duration(781251341142501L, MICROSECONDS)
+    fromS(fromD(expected)).right.value shouldBe expected
+  }
+  it should "freak when given a value larger than 2^64" in {
+    val result = dcc.from(scc.to("12345678901234567890 ns"))
+    result shouldBe a[Left[_, _]]
+    val ex = result.left.value.head.asInstanceOf[error.ThrowableFailure].throwable
+    ex.getMessage should include regex "trying to construct too large duration"
+  }
+  it should "parse a fractional value" in {
+    val expected = 1.5.minutes
+    fromS(fromD(expected)).right.value shouldBe expected
+  }
+  it should "change the units on a fractional value, failing to round trip in the config representation" in {
+    val duration = dcc.from(scc.to("1.5 minutes")).right.value
+    val rep = scc.from(dcc.to(duration)).right.value shouldBe "90s" // Not "1.5 minutes" as we might hope
+  }
+}
+
+object DurationConvertSuite {
+  val scc = implicitly[ConfigConvert[String]]
+  val dcc = implicitly[ConfigConvert[Duration]]
+  val fromD = DurationConvert.fromDuration(_: Duration)
+  val fromS = DurationConvert.fromString(_: String)(None)
 }

core/src/test/scala/pureconfig/gen/package.scala

@@ -21,6 +21,11 @@ package object gen {
   val genJavaDuration: Gen[JavaDuration] = for {
     seconds <- Gen.choose(Long.MinValue + 1, Long.MaxValue)
     nanoseconds <- Gen.choose(0L, MaximumNanoseconds)
+    // JDK Bug: when seconds % 60 == -1 and nanoseconds > 0, Duration.toString produces
+    // a strange value for the seconds with is -0. followed by 1000_000_000 - nanoseconds
+    // e.g. Duration.ofSeconds(-1, 1).toString returns PT-0.999999999S
+    // Duration.parse loads this value as PT0.999999999S instead of the original value
+    if nanoseconds == 0 || seconds % 60 != -1
   } yield JavaDuration.ofSeconds(seconds, nanoseconds)
 
   val genDuration: Gen[Duration] =