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

Use json4s #29

Open
hmil opened this issue Aug 6, 2016 · 9 comments
Open

Use json4s #29

hmil opened this issue Aug 6, 2016 · 9 comments

Comments

@hmil
Copy link
Owner

hmil commented Aug 6, 2016

Having our own JSON ast implementation is redundant and for the sake of preserving human sanity we should use json4s instead.

@easel
Copy link
Contributor

easel commented Aug 17, 2016

Are you sure you want to bless a specific JSON api and bring in an additional dependency? What about simply providing an interface for pluggable encoders and decoders via type classes? What benefits are provided by handling json encoding/decoding in this layer vs. higher up?

@hmil
Copy link
Owner Author

hmil commented Aug 21, 2016

json4s is meant to be the go to way to represent JSON data in Scala. If all libraries use this, it will ease interoperability and reduce the number of different JSON AST implementations in a given application. Time will tell if this is going to happen or if this library is going to end up being "yet another json ast". The least I can do to support this effort is to allow RösHTTP to handle json4s ASTs.
I don't get your point, could you elaborate?

@hmil hmil modified the milestone: v2.0.0 Aug 23, 2016
@hmil
Copy link
Owner Author

hmil commented Sep 7, 2016

Nevermind, json4s v4 is scheduled for december so this will have to wait.

@hmil hmil removed this from the v2.0.0 milestone Sep 7, 2016
@a1russell
Copy link

a1russell commented Jan 5, 2017

Why not µPickle? This is already used in places that need JS and JVM side to work. Who knows how long you could be waiting on json4s?

It looks like json4s is still quite a ways off, looking at json4s/json4s#256.

@antonkulaga
Copy link
Contributor

Why not Circe?

@shogowada
Copy link

I'd like to use other third party JSON serializer too, but I agree that we don't want this library to depend on some specific JSON serializer. I think I have a solution.

I think it makes sense to let JSONBody take JSON string instead of specific type (JSONValue). Then each of us can easily create our own JSONBody using our favorite JSON serialization library.

This is current implementation:

class JSONBody private(value: JSONValue) extends BulkBodyPart {
  override def contentType: String = s"application/json; charset=utf-8"
  override def contentData: ByteBuffer = ByteBuffer.wrap(value.toString.getBytes("utf-8"))
}

object JSONBody {
  def apply(value: JSONValue): JSONBody = new JSONBody(value)
  // ...
}

It can be like below:

class JSONBody private(value: String) extends BulkBodyPart { // Note that it's taking String now
  override def contentType: String = s"application/json; charset=utf-8"
  override def contentData: ByteBuffer = ByteBuffer.wrap(value.getBytes("utf-8"))
}

object JSONBody {
  def apply(value: JSONValue): JSONBody = new JSONBody(value.toString) // We can still keep backward compatibility
  def apply(value: String): JSONBody = new JSONBody(value) // But we can now pass raw JSON
  // ...
}

class MyJSONBody[T](value: T) extends BulkBodyPart {
  private val jsonBody = JSONBody(MyFavoriteJSONSerializer.serialize(value))  // Then we all can use our favorite JSON serializer
  override def contentType: String = jsonBody.contentType
  override def contentData: ByteBuffer = jsonBody.contentData
}

Thoughts?

@a1russell
Copy link

I agree with @shogowada here. Perhaps it's best to just let people use their favorite libraries. The biggest tradeoff I can think of with this approach is that taking a String isn't "safe". So that apply would have to either be able to throw an exception or return an Option[JSONBody].

@antonkulaga I love Circe, too, but I think they are aiming for something more lightweight. Directly depending on Circe means depending on Cats, which is quite large.

@hmil
Copy link
Owner Author

hmil commented Apr 10, 2017

The JSONBody provided with RösHTTP is just a helper in case you want to create a JSON payload with RösHTTP API. I realize most of the time you are going to have another library handling the JSON in which case you'll want to use a lower-level body type or a custom body.

As it is right now, you would either define a body type like in @shogowada 's example or you would use ByteBufferBody and specify "application/json; charset=utf-8" as content-type.

A simple userland solution to avoid repetition of the content-type parameter would be to define a function like:

object MyJSONBody {
  def apply(value: string): BodyPart = ByteBufferBody(ByteBuffer.wrap(value.getBytes("utf-8")), "application/json; charset=utf-8");
}

(not 100% sure about the syntax, just writing this as I go)

What I think is really missing is a BodyPart subclass taking just a string and a content-type as arguments (and maybe an encoding...)

Could be something like:

class UTF8StringBody(value: String, override val contentType: string) { // ...

// or

class StringBody(value: String, override val contentType: string, encoding: string) { // ...

But then I wouldn't bundle the "string-to-json-body" adapter with the lib and I'd rather leave this as the users responsibility because it is a specific use case that is trivial to write on top of such a UTF8StringBody.

If someone feels like writing that (UTF8)StringBody class, PRs are welcome ;)

@emanresusername
Copy link
Contributor

i've actually pointed the rapture folks here for inspiration/use with their http module
i guess it's come full circle as i'd point to their json module as it seems like a good reference here :).
It works on both the jvm and scala.js and doesn't bless any particular json library, in line with this thread so far.

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

No branches or pull requests

6 participants