Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JsonReader for List[T] #267

Open
kostrse opened this issue Jul 15, 2018 · 2 comments
Open

JsonReader for List[T] #267

kostrse opened this issue Jul 15, 2018 · 2 comments

Comments

@kostrse
Copy link

kostrse commented Jul 15, 2018

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

/**
* Supplies the JsonFormat for Lists.
*/
implicit def listFormat[T :JsonFormat] = new RootJsonFormat[List[T]] {
def write(list: List[T]) = JsArray(list.map(_.toJson).toVector)
def read(value: JsValue): List[T] = value match {
case JsArray(elements) => elements.map(_.convertTo[T])(collection.breakOut)
case x => deserializationError("Expected List as JsArray, but got " + x)
}
}

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
Copy link
Member

jrudolph commented Jul 26, 2018

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
Copy link

giftig commented Aug 19, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants