-
Notifications
You must be signed in to change notification settings - Fork 24
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
Comments
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? |
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. |
Nevermind, json4s v4 is scheduled for december so this will have to wait. |
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. |
Why not Circe? |
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 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? |
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 @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. |
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 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 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 If someone feels like writing that |
i've actually pointed the rapture folks here for inspiration/use with their http module |
Having our own JSON ast implementation is redundant and for the sake of preserving human sanity we should use json4s instead.
The text was updated successfully, but these errors were encountered: