amnaredo / test

0 stars 0 forks source link

Reading unquoted numbers as longs #110

Open amnaredo opened 3 years ago

amnaredo commented 3 years ago

It seems counterintuitive that a number that fits in an Int (and thus also a Long) could be read as an Int, but not a Long unless the number is expressed as a string (i.e. quoted). I know that the uPickle document states that Longs are written as strings, but it seems that while reading it should work even with unquoted numbers as long as they fit. This will allow uPickle to consume data produced by other serializers (such as Jackson) that (by default) write numbers unquoted.

scala> import upickle._
import upickle._

scala> case class IntWrapper(value: Int)
defined class IntWrapper

scala> read[IntWrapper]("""{"value" : 1422937970}""")
res0: IntWrapper = IntWrapper(1422937970)

scala> case class LongWrapper(value: Long)
defined class LongWrapper

scala> read[LongWrapper]("""{"value" : 1422937970}""")
upickle.Invalid$Data: data: Num(1.42293797E9) msg: Number
  at upickle.Implicits$$anonfun$validate$1.applyOrElse(Implicits.scala:16)
  at upickle.Implicits$$anonfun$validate$1.applyOrElse(Implicits.scala:16)
  at scala.runtime.AbstractPartialFunction.apply(AbstractPartialFunction.scala:36)
  at upickle.Implicits$$anonfun$upickle$Implicits$$numericStringReaderFunc$1.applyOrElse(Implicits.scala:45)
  at upickle.Implicits$$anonfun$upickle$Implicits$$numericStringReaderFunc$1.applyOrElse(Implicits.scala:45)
...

// quoted number
scala> read[LongWrapper]("""{"value" : "1422937970"}""")
res2: LongWrapper = LongWrapper(1422937970)

ID: 66 Original Author: ramnivas

amnaredo commented 3 years ago

Seems reasonable. The quotes are because literal longs don't work in Javascript, which is half of uPickle's purpose in life. I don't mind making reading flexible.

Original Author: lihaoyi

amnaredo commented 3 years ago

Actually, we have a problem. If we want to read in literal numbers as Longs, we no longer can use JSON.parse, and have to fall back to a much slower, manual parsing. That isn't great

Original Author: lihaoyi

amnaredo commented 3 years ago

@lihaoyi can you please explain why we would need to replace JSON.parse? The side effect of rounding the number is to be expected from the JS runtime, IMO this is not reason to discourage from using Long numbers. There are always other boxed solutions one can use. The result of this bug is that 3rd party serializers can't interop with upickle (because they optimistically transform Long to Number)

Original Author: romansky

amnaredo commented 3 years ago

I'm gonna close this. You can define your own Reader[Long] and Writer[Long] if you want this behavior. Corrupting everyone's data on Scala.js isn't really an option

Original Author: lihaoyi