spray / spray-json

A lightweight, clean and simple JSON implementation in Scala
Apache License 2.0
974 stars 190 forks source link

Can't convertTo[Seq[Foo]] with a RootJsonReader[Foo] #116

Open chris-martin opened 10 years ago

chris-martin commented 10 years ago

If I define a JsonReader[Foo], I can convertTo[Foo], but I can't convertTo[Seq[Foo]].

Project fully demonstrating the problem: https://github.com/chris-martin/spray-convertTo-Seq

Code:

import spray.json.DefaultJsonProtocol._
import spray.json._

case class Foo(a: Int, b: Int)

object FooJsonProtocol extends DefaultJsonProtocol {

  implicit object FooJsonReader extends RootJsonReader[Foo] {

    override def read(json: JsValue): Foo =
      json.asJsObject.getFields("a", "b") match {
        case Seq(JsNumber(a), JsNumber(b)) => Foo(a.toIntExact, b.toIntExact)
        case _ => throw deserializationError(s"Foo expected")
      }
  }
}

import FooJsonProtocol._

print("""{"a": 1, "b": 2}""".parseJson.convertTo[Foo])

print("""[{"a": 1, "b": 2}, {"a": 1, "b": 2}]""".parseJson.convertTo[Seq[Foo]])

Compiler error:

Cannot find JsonReader or JsonFormat type class for Seq[Foo]
     print("""[{"a": 1, "b": 2}, {"a": 1, "b": 2}]""".parseJson.convertTo[Seq[Foo]])
                                                                         ^
chris-martin commented 10 years ago

If I define a RootJsonFormat[Foo], then this does work. But that shouldn't be necessary, since all I'm doing is reading.

chris-martin commented 10 years ago

scala version: 2.11.1 spray-json version: 1.2.6

sirthias commented 10 years ago

spray-json 's type class infrastructure is build around the (Root)JsonFormat type, not the (Root)JsonReader. So you'll indeed have to provide a "Format" even if you are just reading.

chris-martin commented 10 years ago

I see. Is this a can't-fix, won't-fix, or patches-welcome kind of issue?

sirthias commented 10 years ago

It's a "cant/wont fix" as a fix would have to touch spray-jsons' core architecture which is not something that we currently would like to do.

liff commented 10 years ago

How about writing separate instances of JsonReader and JsonWriter for collections?

Something like:

https://github.com/liff/spray-json/blob/issue-116/src/main/scala/spray/json/CollectionFormats.scala#L65 https://github.com/liff/spray-json/blob/issue-116/src/test/scala/spray/json/CustomFormatSpec.scala#L72

The code is obviously incomplete and I'm sure there's a nicer or shorter way of writing it, do you think this trick could work?

erwan commented 9 years ago

liff: That would definitely work, actually that's how it's done in Play Json: https://github.com/playframework/playframework/blob/master/framework/src/play-json/src/main/scala/play/api/libs/json/Reads.scala

I think that it's a pretty common use-case. When you're working with jsonFormatX it's fine, but when you need to write a custom reader you don't always need to write a Write. Being forced to provide a Write, that maybe just throws an exception seems counter-productive.

Also, if you have to provide a Format all the time, what's the point of the Reads and Writes traits?