spray / spray-json

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

JsonReader for List[T] #267

Open kostrse opened 6 years ago

kostrse commented 6 years ago

spray-json has JsonFormat for a List[T] but doesn't have JsonReader.

https://github.com/spray/spray-json/blob/326a001a44bd818750f2baa3f65e68184f5a2d5a/src/main/scala/spray/json/CollectionFormats.scala#L22-L31

I'm writing a domain model and most of my entities have only JsonReaders defined.

I don't need to serialize to JSON (only parse) so I decided to not implement JsonWriters/JsonFormat.

import spray.json._

case class User(name: String)

case class ValueResponse[T](value: T)
case class ValueListResponse[T](values: List[T])

object ApiProtocol extends DefaultJsonProtocol {

  // Some of my entities have JsonFormat but some only JsonReader
  implicit val userReader: JsonReader[T] = ...

  implicit def valueReader[T : JsonReader] = new JsonReader[ValueResponse[T]] {
    def read(value: JsValue): ValueResponse[T] = {
      value.asJsObject.fields.get("value") match {
        case Some(value: JsObject) => ValueResponse(value.convertTo[T])
      }
    }
  }

  implicit def valueListReader[T : JsonReader] = new JsonReader[ValueListResponse[T]] {
    def read(value: JsValue): ValueListResponse[T] = {
      value.asJsObject.fields.get("values") match {
        case Some(values: JsArray) => ValueListResponse(values.convertTo[List[T]])
                                                                        ^
[error] Cannot find JsonReader or JsonFormat type class for List[T]
      }
    }
  }
}

Is this by design always assumed that that users should implement full JsonFormat rather than only JsonReader (or only JsonWriter)?

In some cases serialization expected to be used only in one way or another, so implementing JsonFormat may be unnecessary sometimes.

SO: https://stackoverflow.com/q/51331765/191683

jrudolph commented 6 years ago

Is this by design always assumed that that users should implement full JsonFormat rather than only JsonReader (or only JsonWriter)?

Yes, right now, it is like that. The reason is that the way the implicits are structured don't allow for an easy split up of specific JsonReader and JsonWriter so that JsonFormat is still working (at least that's how I remember it from some years ago). Changing the implicit setup right now is currently too dangerous because of compatibility constraints.

giftig commented 5 years ago

Is any change planned for this? If it's too dangerous right now then it always will be, I suppose? This behaviour is quite misleading and it makes JsonReaders relatively useless except in very simple cases, and requires having write implementations where they're not required at all, or else working around it by adding additional "standard" readers yourself. It's not the easiest thing to debug, either, as it can take a while to get from the compile error "Cannot find JsonReader or JsonFormat type class for List[T]" to figuring out that you really do have a JsonReader[T] in scope and the problem is a missing generic ListReader, which you have to dive into the library code to discover. I don't think I've seen anything warning about this aspect of the API, though granted I don't see anything especially obvious in your main docs encouraging using JsonReader. If it's unlikely to change it's probably worth making it very clear that JsonReader isn't really intended to be used.