Can't parse JSON map to Map[String, T]

[amnaredo]

Hi Li,

Thanks for your work on this project, it's very nice! I've faced a problem when trying to parse the following JSON (output from CouchDb):

{  
   "_id":"481e7e36693c1ae3f1871be7e506aade",
   "_rev":"2-7bc08b6426dd0a165d9c7d1f9e08aad5",
   "name":"Alice",
   "age":25,
   "_attachments":{  
      "attachment":{  
         "content_type":"application/octet-stream",
         "revpos":2,
         "digest":"md5-boWfl561SDppW4MTLQhVyA==",
         "length":5,
         "stub":true
      }
   }
}

I have the following classes:

abstract class CouchDoc {
  val _id: String
  val _rev: String
  val _attachments: Map[String, CouchAttachment]
}

case class CouchAttachment(
  content_type: String,
  revpos: Int,
  digest: String,
  length: Int,
  stub: Boolean)

case class Person(
  name: String,
  age: Int,
  _id: String,
  _rev: String,
  _attachments: Map[String, CouchAttachment] = Map()) extends CouchDoc

When I'm trying to parse the JSON with something like:

upickle.read[Person](json)

I get the following exception:

data: Obj(ArrayBuffer((attachment,Obj(ArrayBuffer((content_type,Str(application/octet-stream)), (revpos,Num(2.0)), (digest,Str(md5-boWfl561SDppW4MTLQhVyA==)), (length,Num(5.0)), (stub,True)))))) msg: Array(n)
upickle.Invalid$Data: data: Obj(ArrayBuffer((attachment,Obj(ArrayBuffer((content_type,Str(application/octet-stream)), (revpos,Num(2.0)), (digest,Str(md5-boWfl561SDppW4MTLQhVyA==)), (length,Num(5.0)), (stub,True)))))) msg: Array(n)
    at upickle.Implicits$$anonfun$validate$1.applyOrElse(Implicits.scala:16)
    at upickle.Implicits$$anonfun$validate$1.applyOrElse(Implicits.scala:16)
    at upickle.Implicits$$anonfun$MapR$1.applyOrElse(Implicits.scala:109)
    at upickle.Implicits$$anonfun$MapR$1.applyOrElse(Implicits.scala:109)
    at upickle.Reader$$anonfun$read$1.applyOrElse(Types.scala:42)
    at upickle.Reader$$anonfun$read$1.applyOrElse(Types.scala:42)
    at upickle.Types$class.readJs(Types.scala:139)
    at upickle.package$.readJs(package.scala:10)
    at upickle.Generated$$anonfun$Tuple5R$1.applyOrElse(Generated.scala:49)
    at upickle.Generated$$anonfun$Tuple5R$1.applyOrElse(Generated.scala:49)
    at upickle.Reader$$anonfun$read$1.applyOrElse(Types.scala:42)
    at upickle.Reader$$anonfun$read$1.applyOrElse(Types.scala:42)
    at upickle.Types$class.readJs(Types.scala:139)
    at upickle.package$.readJs(package.scala:10)
    at upickle.Generated$InternalGenerated$$anonfun$Case5R$1.applyOrElse(Generated.scala:233)
    at upickle.Generated$InternalGenerated$$anonfun$Case5R$1.applyOrElse(Generated.scala:233)
    at upickle.GeneratedUtil$$anonfun$readerCaseFunction$1.applyOrElse(GeneratedUtil.scala:15)
    at upickle.GeneratedUtil$$anonfun$readerCaseFunction$1.applyOrElse(GeneratedUtil.scala:15)
    at upickle.Reader$$anonfun$read$1.applyOrElse(Types.scala:42)
    at upickle.Reader$$anonfun$read$1.applyOrElse(Types.scala:42)
    at upickle.Reader$$anonfun$read$1.applyOrElse(Types.scala:42)
    at upickle.Reader$$anonfun$read$1.applyOrElse(Types.scala:42)
    at upickle.Types$class.readJs(Types.scala:139)
    at upickle.package$.readJs(package.scala:10)
    at upickle.Types$class.read(Types.scala:135)
    at upickle.package$.read(package.scala:10)

As I understand, it's having problems with creating an instance of Map. Do you know any way to resolve this? Thanks!

ID: 39 Original Author: beloglazov

[amnaredo]

The basic problems is that Maps serialize to lists of tuples, so they can support non-string keys properly. Other people have requested being able to serialize them to JSON dicts. I haven't any concrete plans to make it possible right now, but I'd like to do so at some point.

Original Author: lihaoyi

[amnaredo]

You could probably shadow the default implementation for serializing Maps and make it drop down to json dicts if the keys are all strings. Kinda hacky but it'd work, and I suspect it'd work.

Original Author: lihaoyi

[amnaredo]

Thanks for your reply. I've added the following instances for Maps with String keys in my project:

  implicit def MapWithStringKeysW[V: Writer] = Writer[Map[String, V]] {
    obj => Js.Obj(obj.toSeq.map(x => (x._1, writeJs[V](x._2))): _*)
  }

  implicit def MapWithStringKeysR[V: Reader] = Reader[Map[String, V]] {
    case json: Js.Obj => json.value.map(x => (x._1, readJs[V](x._2))).toMap
  }

This works for me at the moment. Would you consider adding something like this to the library?

Original Author: beloglazov

[amnaredo]

Yep, I'd like to do that at some point. How does this play with existing non-string maps? Do those still work using []s instead of {}s?

On Mon, Nov 17, 2014 at 3:40 PM, Anton Beloglazov notifications@github.com wrote:

Thanks for your reply. I've added the following instances for Maps with String keys in my project:

implicit def MapWithStringKeysW[V: Writer] = Writer[Map[String, V]] { obj => Js.Obj(obj.toSeq.map(x => (x._1, writeJsV)): _*) }

implicit def MapWithStringKeysR[V: Reader] = Reader[Map[String, V]] { case json: Js.Obj => json.value.map(x => (x._1, readJsV)).toMap }

This works for me at the moment. Would you consider adding something like this to the library?

— Reply to this email directly or view it on GitHub https://github.com/lihaoyi/upickle/issues/39#issuecomment-63397874.

Original Author: lihaoyi

[amnaredo]

Here are some examples you've given in https://github.com/lihaoyi/upickle/issues/17#issuecomment-52853230:

println(upickle.write(Map("hello" -> (1, "world"))))
println(upickle.write(Map((1, "hello") -> "world")))
println(upickle.write(Map(Seq(1, 2, 3) -> "world")))
println(upickle.write(Map("hello" -> Seq(1, 2, 3))))

The output is:

{"hello":[1,"world"]}
[[[1,"hello"],"world"]]
[[[1,2,3],"world"]]
{"hello":[1,2,3]}

Would you like me to test anything else?

Original Author: beloglazov

[amnaredo]

That looks perfectly reasonable. What about

def func[T: Writer](t: T) = upickle.write(Map(t -> 123))

func("10")
upickle.write(Map("10" -> 123))
func(10)

Original Author: lihaoyi

[amnaredo]

In this case the output is:

[["10",123]]
{"10":123}
[[10,123]]

It didn't pull the correct implicit for the first case for some reason. Do you have any ideas of how to solve this?

Original Author: beloglazov

[amnaredo]

I don't know if it's solvable; it's doing exactly what we're asking it to do: get a Writer[String], and then combining it with a Writer[Map[T, V]], which happens to be the array-of-tuples writer. The problem is when we ask for Writer[Map[T, V]], we don't know that T =:= String until runtime, at which point it's too late.

For it to be consistent, it feels to me that the only solutions would be to

• Dynamically inspect the keys using .isInstanceOf[String] to check if they're all strings, converting it to a json dict if so, OR
• Serialize non-strings into strings, the way Jackson does

Both of these feel pretty sketchy, but I'm not sure which one is less sketchy =/ Having the shape of your JSON changing unpredictably under you is sketchy, but so is having to recursively JSON.parse your string-keys to deserialize what should be a simple nested datastructure.

Original Author: lihaoyi

[amnaredo]

I see, thanks for the explanation. I've added two more instances as you suggested:

implicit def MapW[K: Writer, V: Writer]: Writer[Map[K, V]] = Writer[Map[K, V]](
  x => {
    if (x.nonEmpty && x.head._1.isInstanceOf[String])
      Js.Obj(x.toSeq.map(entry => (entry._1.asInstanceOf[String], writeJs[V](entry._2))): _*)
    else
      Js.Arr(x.toSeq.map(writeJs[(K, V)]): _*)
  }
)

implicit def MapR[K: Reader, V: Reader]: Reader[Map[K, V]] = Reader[Map[K, V]] {
  case json: Js.Obj => json.value.map(x => (x._1.asInstanceOf[K], readJs[V](x._2))).toMap
  case json: Js.Arr => json.value.map(readJs[(K, V)]).toMap
}

The output is now correct for the case of String keys:

{"10":123}
{"10":123}
[[10,123]]

Original Author: beloglazov

[amnaredo]

Here's an interesting case: what if instead of checking the head of the map, we checked the implicit Writer[K] to see if it's the StringWriter? That way we'd have consistent behavior with e.g. write(Map[String, T].empty), which in your current setup will default to [] rather than {}. By checking the Writer, it no longer matters if the Map is empty, and writing a Map of the same K will always result in the same "shaped" JSON structure.

write(Map[String, T].empty) and write(Map[MyCaseClass, T].empty) will differ ({} and [] respectively) but that's probably OK

Original Author: lihaoyi

[amnaredo]

Sorry for taking long to reply. I agree, empty string-key maps should definitely be handled consistently with non-empty ones. However, I couldn't figure out how to check if Writer[K] is an instance of a string Writer. There is no StringWriter class and implicitly[Writer[K]].isInstanceOf[Writer[String]] fails because of the type erasure. Could you please clarify how you see doing that?

Original Author: beloglazov

[amnaredo]

I was more thinking

implicitly[Writer[T]] == upickle.StringRW

That should work right?

Original Author: lihaoyi

[amnaredo]

Naturally, it won't work for people who define their own Writer[String] but maybe that's ok.

Another less-hard-coded solution is to provide Reader[T] and Writer[T] with another pair of method

Writer[T].asStringKey(t: T): Option[String]
Reader[T].fromStringKey(s: String): Option[T]

That will allow other non-upickle.StringRW implicits to emulate this behavior. For example, we may consider it desirable to make Map[T, V] for T in {Int, Short, Byte, Long, Boolean, Double, Float} have a asStringKey property that doesn't return None. Then users who wanted their own type T behave similarly could do so.

Of course, the downside is that this doubles the API surface-area of Reader and Writer. But maybe it's OK, especially if we make asStringKey/fromStringKey have a default of None so most people won't have to care about it

Original Author: lihaoyi

[amnaredo]

Sorry, I didn't notice StringRW earlier. What do you think about the following version? It should probably handle custom Writer[String] as well:

implicit def MapW[K: Writer, V: Writer](implicit sw: Writer[String]): Writer[Map[K, V]] = Writer[Map[K, V]](
  x => {
    if (implicitly[Writer[K]] == sw)
      Js.Obj(x.toSeq.map(entry => (entry._1.asInstanceOf[String], writeJs[V](entry._2))): _*)
    else
      Js.Arr(x.toSeq.map(writeJs[(K, V)]): _*)
  }
)

You idea about asStringKey / fromStringKey sounds interesting as well. I guess it could be useful in some cases to be able to encode custom types into string keys of a json object.

Original Author: beloglazov

[amnaredo]

On further thought, I think it'd be best to limit the weirdness of JSON objects to the Map serializer and not have it infect the rest of the code through the Reader and Writer traits. I like your solution the most out of the ones we've seen so far. Do you want to send a PR with tests for all the examples we discussed?

Original Author: lihaoyi

[amnaredo]

Remember you'll need to update MapR too so that round-trip serialization would work right

Original Author: lihaoyi

[amnaredo]

Sure, I'll try to do that soon. Thanks!

Original Author: beloglazov

[amnaredo]

Thanks for making these changes! Sorry, I was busy with other work. Are you going to release a new version?

Original Author: beloglazov

[amnaredo]

At some point; thinking of waiting for Scala.js 0.6.0 to come out. Lazy to modify and publish all my libraries more than once...

Original Author: lihaoyi

[amnaredo]

Cool, I'm looking forward to that :)

Original Author: beloglazov

[amnaredo]

I think the error I'm receiving is related to this issue https://groups.google.com/forum/#!topic/couchdb-scala/ZpHGuPBoS0Q

Original Author: Nabegh

[amnaredo]

Thanks for your reply. I've added the following instances for Maps with String keys in my project:

  implicit def MapWithStringKeysW[V: Writer] = Writer[Map[String, V]] {
    obj => Js.Obj(obj.toSeq.map(x => (x._1, writeJs[V](x._2))): _*)
  }

  implicit def MapWithStringKeysR[V: Reader] = Reader[Map[String, V]] {
    case json: Js.Obj => json.value.map(x => (x._1, readJs[V](x._2))).toMap
  }

This works for me at the moment. Would you consider adding something like this to the library?

It seems the library has evolved meanwhile and the writer needs to be written differently. I think following should do the job with recent version:

implicit def rwMap[T: ReadWriter]: ReadWriter[Map[String, T]] = readwriter[ujson.Obj].bimap[Map[String, T]](
    obj => obj.toSeq.map(x => (x._1, writeJs[T](x._2)))),
    {case json: Js.Obj => json.value.map(x => (x._1, read[T](x._2))).toMap}
  )

Original Author: OndrejSpanel