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

Remove String conversion in decoders. #671

Merged
merged 1 commit into from
Nov 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions argonaut/src/main/scala/io/finch/argonaut/Decoders.scala
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package io.finch.argonaut

import argonaut.{CursorHistory, DecodeJson, Json}
import com.twitter.finagle.netty3.ChannelBufferBuf
import com.twitter.util.{Return, Throw, Try}
import io.finch._
import io.finch.internal.BufText
import java.nio.charset.StandardCharsets
import jawn.Parser
import jawn.support.argonaut.Parser._
import scala.util.{Failure, Success}
Expand All @@ -16,12 +18,12 @@ trait Decoders {
implicit def decodeArgonaut[A](implicit d: DecodeJson[A]): Decode.Json[A] =
Decode.json { (b, cs) =>
val err: (String, CursorHistory) => Try[A] = { (str, hist) => Throw[A](Error(str)) }

// TODO: Eliminate toString conversion
// See https://github.com/finagle/finch/issues/511
// Jawn can parse from ByteBuffer's if they represent UTF-8 strings.
// We can check the charset here and do parsing w/o extra to-string conversion.
Parser.parseFromString[Json](BufText.extract(b, cs))(facade) match {
val attemptJson = cs match {
case StandardCharsets.UTF_8 =>
Parser.parseFromByteBuffer[Json](ChannelBufferBuf.Owned.extract(b).toByteBuffer())(facade)
case _ => Parser.parseFromString[Json](BufText.extract(b, cs))(facade)
}
attemptJson match {
case Success(value) => d.decodeJson(value).fold[Try[A]](err, Return(_))
case Failure(error) => Throw[A](Error(error.getMessage))
}
Expand Down
23 changes: 11 additions & 12 deletions circe/src/main/scala/io/finch/circe/Decoders.scala
Original file line number Diff line number Diff line change
@@ -1,26 +1,25 @@
package io.finch.circe

import cats.syntax.show._
import com.twitter.finagle.netty3.ChannelBufferBuf
import com.twitter.util.{Return, Throw, Try}
import io.circe.Decoder
import io.circe.jawn.decode
import io.circe.jawn._
import io.finch.{Decode, Error}
import io.finch.internal.BufText
import java.nio.charset.StandardCharsets

trait Decoders {

/**
* Maps a Circe's [[Decoder]] to Finch's [[Decode]].
*/
implicit def decodeCirce[A: Decoder]: Decode.Json[A] = Decode.json((b, cs) =>

// TODO: Eliminate toString conversion
// See https://github.com/finagle/finch/issues/511
// Jawn can parse from ByteBuffer's if they represent UTF-8 strings.
// We can check the charset here and do parsing w/o extra to-string conversion.
decode[A](BufText.extract(b, cs)).fold[Try[A]](
error => Throw[A](Error(error.show)),
value => Return(value)
)
)
implicit def decodeCirce[A: Decoder]: Decode.Json[A] = Decode.json { (b, cs) =>
val attemptJson = cs match {
case StandardCharsets.UTF_8 =>
parseByteBuffer(ChannelBufferBuf.Owned.extract(b).toByteBuffer()).right.flatMap(_.as[A])
case _ => decode[A](BufText.extract(b, cs))
}
attemptJson.fold[Try[A]](error => Throw[A](Error(error.show)), value => Return(value))
}
}
18 changes: 18 additions & 0 deletions core/src/main/scala/io/finch/internal/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.twitter.finagle.http.Message
import com.twitter.io.{Buf, Charsets}
import java.nio.CharBuffer
import java.nio.charset.{Charset, StandardCharsets}
import org.jboss.netty.buffer.ChannelBuffer

/**
* This package contains an internal-use only type-classes and utilities that power Finch's API.
Expand Down Expand Up @@ -60,6 +61,23 @@ package object internal {
}
// scalastyle:on return

/** Extract a byte array from a ChannelBuffer that is backed by an array.
* Precondition: buf.hasArray == true
*
* @param buf The ChannelBuffer to extract the array from
* @return An array of bytes
*/
def extractBytesFromArrayBackedBuf(buf: ChannelBuffer): Array[Byte] = {
val rawArray = buf.array
val size = buf.readableBytes()
if (rawArray.length == size) rawArray
else {
val dst = new Array[Byte](size)
System.arraycopy(buf.array(), 0, dst, 0, size)
dst
}
}

/**
* Enriches any string with fast `tooX` conversions.
*/
Expand Down
15 changes: 7 additions & 8 deletions jackson/src/main/scala/io/finch/jackson/package.scala
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
package io.finch

import scala.reflect.ClassTag

import com.fasterxml.jackson.databind.ObjectMapper
import com.twitter.finagle.netty3.ChannelBufferBuf
import com.twitter.util.Try
import io.finch.internal.BufText
import scala.reflect.ClassTag

package object jackson {

implicit def decodeJackson[A](implicit
mapper: ObjectMapper, ct: ClassTag[A]
): Decode.Json[A] = Decode.json((b, cs) =>
// TODO: Eliminate toString conversion
// See https://github.com/finagle/finch/issues/511
// Jackson can parse from byte[] automatically detecting the encoding.
Try(mapper.readValue(BufText.extract(b, cs), ct.runtimeClass.asInstanceOf[Class[A]]))
)
): Decode.Json[A] = Decode.json { (b, cs) =>
val buf = ChannelBufferBuf.Owned.extract(b)
if (buf.hasArray) Try(mapper.readValue(buf.array(), 0, buf.readableBytes(), ct.runtimeClass.asInstanceOf[Class[A]]))
else Try(mapper.readValue(buf.toString(cs), ct.runtimeClass.asInstanceOf[Class[A]]))
}

implicit def encodeJackson[A](implicit mapper: ObjectMapper): Encode.Json[A] =
Encode.json((a, cs) => BufText(mapper.writeValueAsString(a), cs))
Expand Down
12 changes: 7 additions & 5 deletions playjson/src/main/scala/io/finch/playjson/package.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package io.finch

import com.twitter.finagle.netty3.ChannelBufferBuf
import com.twitter.util.Try
import io.finch.internal.BufText
import io.finch.internal.{extractBytesFromArrayBackedBuf, BufText}
import play.api.libs.json._

package object playjson {
Expand All @@ -11,10 +12,11 @@ package object playjson {
* @tparam A the type of the data to decode into
*/
implicit def decodePlayJson[A](implicit reads: Reads[A]): Decode.Json[A] =
// TODO: Eliminate toString conversion
// See https://github.com/finagle/finch/issues/511
// PlayJson can parse from byte[] automatically detecting the charset.
Decode.json((b, cs) => Try(Json.parse(BufText.extract(b, cs)).as[A]))
Decode.json { (b, cs) =>
val buf = ChannelBufferBuf.Owned.extract(b)
if (buf.hasArray) Try(Json.parse(extractBytesFromArrayBackedBuf(buf)).as[A])
else Try(Json.parse(buf.toString(cs)).as[A])
}

/**
* @param writes Play JSON `Writes` to use for encoding
Expand Down
19 changes: 13 additions & 6 deletions sprayjson/src/main/scala/io/finch/sprayjson/package.scala
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
package io.finch

import com.twitter.finagle.netty3.ChannelBufferBuf
import com.twitter.util.Try
import io.finch.internal.BufText
import io.finch.internal.{extractBytesFromArrayBackedBuf, BufText}
import java.nio.charset.StandardCharsets
import spray.json._

package object sprayjson{
package object sprayjson {

/**
* @param format spray-json support for convert JSON val to specific type object
* @tparam A the type of the data to decode into
*/
implicit def decodeSpray[A](implicit format: JsonFormat[A]): Decode.Json[A] =
// TODO: Eliminate toString conversion
// See https://github.com/finagle/finch/issues/511
// SprayJson can parse from byte[] if it represents a UTF-8 string.
Decode.json((b, cs) => Try(BufText.extract(b, cs).parseJson.convertTo[A]))
Decode.json { (b, cs) =>
cs match {
case StandardCharsets.UTF_8 =>
val buf = ChannelBufferBuf.Owned.extract(b)
if (buf.hasArray) Try(JsonParser(extractBytesFromArrayBackedBuf(buf)).convertTo[A])
else Try(JsonParser(buf.toString(cs)).convertTo[A])
case _ => Try(BufText.extract(b, cs).parseJson.convertTo[A])
}
}

/**
* @param format spray-json support for convert JSON val to specific type object
Expand Down