Skip to content

Commit

Permalink
decrease allocation in request building and response handling
Browse files Browse the repository at this point in the history
  • Loading branch information
agourlay committed Nov 4, 2023
1 parent d88dd2f commit b2dbaac
Show file tree
Hide file tree
Showing 10 changed files with 159 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ trait HttpDsl extends HttpDslOps with HttpRequestsDsl {
def WithHeaders(headers: (String, String)*): BodyElementCollector[Step, Seq[Step]] =
BodyElementCollector[Step, Seq[Step]] { steps =>
// the surrounding steps are hidden from the logs
val saveStep = save((withHeadersKey, encodeSessionHeaders(headers)), show = false)
val saveStep = save((withHeadersKey, encodeSessionHeaders(headers.toVector)), show = false)
val rollbackStep = rollback(withHeadersKey, show = false)
saveStep +: steps :+ rollbackStep
}
Expand All @@ -188,7 +188,7 @@ trait HttpDslOps {
else if (keep.isEmpty)
s.removeKey(withHeadersKey).asRight
else
s.addValue(withHeadersKey, encodeSessionHeaders(keep))
s.addValue(withHeadersKey, encodeSessionHeaders(keep.toVector))
}
}
}
Expand All @@ -199,7 +199,7 @@ object HttpDsl {

def save_many_from_session_json(fromKey: String)(args: Seq[FromSessionSetter[Json]]): Step =
CEffectStep.fromSyncE(
s"${args.iterator.map(_.title).mkString(" and ")}",
title = args.iterator.map(_.title).mkString(" and "),
sc => {
val session = sc.session
for {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,19 @@ import scala.concurrent.duration.FiniteDuration

sealed trait HttpError extends CornichonError

case class TimeoutErrorAfter[A: Show](request: A, after: FiniteDuration) extends HttpError {
case class TimeoutErrorAfter(request: String, after: FiniteDuration) extends HttpError {
lazy val baseErrorMessage =
s"""|${request.show}
s"""|$request
|connection timed out error after ${after.toMillis} ms""".trim.stripMargin
}

case class RequestError[A: Show](request: A, e: Throwable) extends HttpError {
case class RequestError(request: String, e: Throwable) extends HttpError {
lazy val baseErrorMessage =
s"""|${request.show}
s"""|request
|encountered the following error:
|${CornichonError.genStacktrace(e)}""".trim.stripMargin
}

case class UnmarshallingResponseError(e: Throwable, response: String) extends HttpError {
lazy val baseErrorMessage = s"""HTTP response '$response' generated error:
|${CornichonError.genStacktrace(e)}""".trim.stripMargin
}

case class SseError(e: Throwable) extends HttpError {
lazy val baseErrorMessage =
s"""expected SSE connection but got error:
|${CornichonError.genStacktrace(e)}""".trim.stripMargin
}

case class StatusNonExpected[A: Show](expectedStatus: A, actualStatus: A, headers: Seq[(String, String)], rawBody: String, requestDescription: String) extends HttpError {
lazy val baseErrorMessage = {
// TODO do not assume that body is JSON - use content-type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ trait BaseRequest {
def params: Seq[(String, String)]
def headers: Seq[(String, String)]

// used for step title log display
def compactDescription: String

def paramsTitle: String = if (params.isEmpty) "" else s" with query parameters ${printArrowPairs(params)}"
def headersTitle: String = if (headers.isEmpty) "" else s" with headers ${printArrowPairs(headers)}"
// used for request session storage
def detailedDescription: String
}

case class HttpRequest[A: Show: Resolvable: Encoder](method: HttpMethod, url: String, body: Option[A], params: Seq[(String, String)], headers: Seq[(String, String)])
Expand All @@ -44,10 +45,65 @@ case class HttpRequest[A: Show: Resolvable: Encoder](method: HttpMethod, url: St

def withBody[B: Show: Resolvable: Encoder](body: B) = copy(body = Some(body))

lazy val compactDescription: String = {
val base = s"${method.name} $url"
val payloadTitle = body.fold("")(p => s" with body\n${p.show}")
base + payloadTitle + paramsTitle + headersTitle
def compactDescription: String = {
val builder = new StringBuilder()
builder.append(method.name)
builder.append(" ")
builder.append(url)
body.foreach { p =>
builder.append(" with body\n")
builder.append(p.show)
}
if (params.nonEmpty) {
builder.append(" with query parameters ")
printArrowPairsBuilder(params, builder)
}

if (headers.nonEmpty) {
builder.append(" with headers ")
printArrowPairsBuilder(headers, builder)
}
builder.toString()
}

def detailedDescription: String = {
val builder = new StringBuilder()
builder.append("HTTP ")

// method
builder.append(method.name)

// URL
builder.append(" request to ")
builder.append(url)
builder.append("\n")

// parameters
if (params.isEmpty)
builder.append("without parameters")
else {
builder.append("with parameters ")
printArrowPairsBuilder(params, builder)
}
builder.append("\n")

// headers
if (headers.isEmpty)
builder.append("without headers")
else {
builder.append("with headers ")
printArrowPairsBuilder(headers, builder)
}
builder.append("\n")

// body
body match {
case Some(b) =>
builder.append("with body\n")
builder.append(b.show)
case None => builder.append("without body")
}
builder.result()
}
}

Expand All @@ -63,20 +119,6 @@ trait HttpRequestsDsl {
def patch(url: String): HttpRequest[String] = HttpRequest[String](PATCH, url, None, Nil, Nil)
}

object HttpRequest extends HttpRequestsDsl {

implicit def showRequest[A: Show]: Show[HttpRequest[A]] = new Show[HttpRequest[A]] {
def show(r: HttpRequest[A]): String = {
val body = r.body.fold("without body")(b => s"with body\n${b.show}")
val params = if (r.params.isEmpty) "without parameters" else s"with parameters ${printArrowPairs(r.params)}"
val headers = if (r.headers.isEmpty) "without headers" else s"with headers ${printArrowPairs(r.headers)}"

s"HTTP ${r.method.name} request to ${r.url}\n$params\n$headers\n$body"
}
}

}

case class HttpStream(name: String) extends AnyVal

object HttpStreams {
Expand All @@ -93,19 +135,48 @@ case class HttpStreamedRequest(stream: HttpStream, url: String, takeWithin: Fini
def withHeaders(headers: (String, String)*) = copy(headers = headers)
def addHeaders(headers: (String, String)*) = copy(headers = this.headers ++ headers)

lazy val compactDescription: String = {
val base = s"open ${stream.name} to $url"
base + paramsTitle + headersTitle
def compactDescription: String = {
val builder = new StringBuilder()
builder.append("open ")
builder.append(stream.name)
builder.append(" to ")
builder.append(url)
if (params.nonEmpty) {
builder.append(" with query parameters ")
printArrowPairsBuilder(params, builder)
}
if (headers.nonEmpty) {
builder.append(" with headers ")
printArrowPairsBuilder(headers, builder)
}
builder.result()
}
}

object HttpStreamedRequest {

implicit val showStreamedRequest: Show[HttpStreamedRequest] = (r: HttpStreamedRequest) => {
val params = if (r.params.isEmpty) "without parameters" else s"with parameters ${printArrowPairs(r.params)}"
val headers = if (r.headers.isEmpty) "without headers" else s"with headers ${printArrowPairs(r.headers)}"
def detailedDescription: String = {
val builder = new StringBuilder()
builder.append(stream.name)
builder.append(" request to ")
builder.append(url)
builder.append("\n")

// parameters
if (params.isEmpty)
builder.append("without parameters")
else {
builder.append("with parameters ")
printArrowPairsBuilder(params, builder)
}
builder.append("\n")

// headers
if (headers.isEmpty)
builder.append("without headers")
else {
builder.append("with headers ")
printArrowPairsBuilder(headers, builder)
}
builder.append("\n")

s"${r.stream.name} request to ${r.url}\n$params\n$headers"
builder.result()
}

}
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package com.github.agourlay.cornichon.http

case class HttpResponse(status: Int, headers: Seq[(String, String)] = Nil, body: String)
case class HttpResponse(status: Int, headers: Vector[(String, String)], body: String)
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package com.github.agourlay.cornichon.http

import cats.Show
import cats.data.EitherT
import cats.syntax.show._
import cats.effect.IO
import cats.effect.unsafe.IORuntime
import org.http4s.circe._
Expand All @@ -13,7 +12,6 @@ import com.github.agourlay.cornichon.json.CornichonJson._
import com.github.agourlay.cornichon.http.HttpStreams._
import com.github.agourlay.cornichon.resolver.Resolvable
import com.github.agourlay.cornichon.http.HttpService._
import com.github.agourlay.cornichon.http.HttpRequest._
import com.github.agourlay.cornichon.util.TraverseUtils.traverseIL
import io.circe.{ Encoder, Json }
import org.http4s.EntityEncoder
Expand Down Expand Up @@ -62,15 +60,15 @@ class HttpService(
resolvedRequest = HttpRequest(r.method, url, jsonBody, params, headers)
configuredRequest = configureRequest(resolvedRequest, config)
resp <- client.runRequest(configuredRequest, requestTimeout)(circeJsonEncoder, Json.showJson) // TODO remove implicits when removing Scala 2.12
newSession <- EitherT.fromEither[IO](handleResponse(resp, configuredRequest.show, expectedStatus, extractor)(scenarioContext.session))
newSession <- EitherT.fromEither[IO](handleResponse(resp, configuredRequest.detailedDescription, expectedStatus, extractor)(scenarioContext.session))
} yield newSession

private def runStreamRequest(r: HttpStreamedRequest, expectedStatus: Option[Int], extractor: ResponseExtractor)(scenarioContext: ScenarioContext) =
for {
(url, _, params, headers) <- EitherT.fromEither[IO](resolveRequestParts[String](r.url, None, r.params, r.headers, SelectNone)(scenarioContext))
resolvedRequest = HttpStreamedRequest(r.stream, url, r.takeWithin, params, headers)
resp <- EitherT(client.openStream(resolvedRequest, requestTimeout))
newSession <- EitherT.fromEither[IO](handleResponse(resp, resolvedRequest.show, expectedStatus, extractor)(scenarioContext.session))
newSession <- EitherT.fromEither[IO](handleResponse(resp, resolvedRequest.detailedDescription, expectedStatus, extractor)(scenarioContext.session))
} yield newSession

private def withBaseUrl(input: String) = {
Expand Down Expand Up @@ -162,12 +160,16 @@ object HttpService {
def encodeSessionHeader(name: String, value: String) =
s"$name$headersKeyValueDelim$value"

def encodeSessionHeaders(headers: Seq[(String, String)]): String = {
def encodeSessionHeaders(headers: Vector[(String, String)]): String = {
val len = headers.length
val builder = new StringBuilder(len * 10)
var i = 0
for ((name, value) <- headers) {
builder.append(encodeSessionHeader(name, value))
while (i < len) {
val (name, value) = headers(i)
// unroll `encodeSessionHeader` to avoid creating intermediate strings
builder.append(name);
builder.append(headersKeyValueDelim);
builder.append(value)
if (i < len - 1)
builder.append(interHeadersValueDelim)
i += 1
Expand All @@ -184,13 +186,13 @@ object HttpService {
Right(elms(0) -> elms(1))
}

private def configureRequest[A: Show](req: HttpRequest[A], config: Config): HttpRequest[A] = {
private def configureRequest[A](req: HttpRequest[A], config: Config): HttpRequest[A] = {
if (config.traceRequests)
println(DebugLogInstruction(req.show, 1).colorized)
println(DebugLogInstruction(req.detailedDescription, 1).colorized)
if (config.warnOnDuplicateHeaders && req.headers.groupBy(_._1).exists(_._2.size > 1))
println(WarningLogInstruction(s"\n**Warning**\nduplicate headers detected in request:\n${req.show}", 1).colorized)
println(WarningLogInstruction(s"\n**Warning**\nduplicate headers detected in request:\n${req.detailedDescription}", 1).colorized)
if (config.failOnDuplicateHeaders && req.headers.groupBy(_._1).exists(_._2.size > 1))
throw BasicError(s"duplicate headers detected in request:\n${req.show}").toException
throw BasicError(s"duplicate headers detected in request:\n${req.detailedDescription}").toException
else
req
}
Expand All @@ -202,17 +204,7 @@ object HttpService {
case ByNames(names) => headers.filterNot { case (n, _) => names.contains(n) }
}

private def expectStatusCode(httpResponse: HttpResponse, expected: Option[Int], requestDescription: String): Either[CornichonError, HttpResponse] =
expected match {
case None =>
Right(httpResponse)
case Some(expectedStatus) if httpResponse.status == expectedStatus =>
Right(httpResponse)
case Some(expectedStatus) =>
Left(StatusNonExpected(expectedStatus, httpResponse.status, httpResponse.headers, httpResponse.body, requestDescription))
}

def fillInSessionWithResponse(session: Session, extractor: ResponseExtractor, requestDescription: String)(response: HttpResponse): Either[CornichonError, Session] = {
def fillInSessionWithResponse(response: HttpResponse, session: Session, extractor: ResponseExtractor, requestDescription: String): Either[CornichonError, Session] = {
val updatedSession = session
.addValueInternal(lastResponseStatusKey, statusToString(response.status))
.addValueInternal(lastResponseBodyKey, response.body)
Expand All @@ -231,8 +223,11 @@ object HttpService {
}

private def handleResponse(resp: HttpResponse, requestDescription: String, expectedStatus: Option[Int], extractor: ResponseExtractor)(session: Session): Either[CornichonError, Session] =
expectStatusCode(resp, expectedStatus, requestDescription)
.flatMap(fillInSessionWithResponse(session, extractor, requestDescription))
expectedStatus match {
case Some(expectedStatus) if resp.status != expectedStatus =>
Left(StatusNonExpected(expectedStatus, resp.status, resp.headers, resp.body, requestDescription))
case _ => fillInSessionWithResponse(resp, session, extractor, requestDescription)
}

// Avoid reallocating known strings
private def statusToString(status: Int): String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ class Http4sClient(
}
}

private def fromHttp4sHeaders(headers: Headers): Seq[(String, String)] =
headers.headers.map(h => (h.name.toString, h.value))
private def fromHttp4sHeaders(headers: Headers): Vector[(String, String)] =
headers.headers.iterator.map(h => (h.name.toString, h.value)).toVector

def addQueryParams(uri: Uri, moreParams: Seq[(String, String)]): Uri =
if (moreParams.isEmpty)
Expand Down Expand Up @@ -127,11 +127,11 @@ class Http4sClient(
}
}

val timeout = IO.delay(TimeoutErrorAfter(cReq, t).asLeft).delayBy(t)
val timeout = IO.delay(TimeoutErrorAfter(cReq.detailedDescription, t).asLeft).delayBy(t)

IO.race(cornichonResponse, timeout)
.map(_.fold(identity, identity))
.handleError { t => RequestError(cReq, t).asLeft }
.handleError { t => RequestError(cReq.detailedDescription, t).asLeft }
}
)

Expand Down Expand Up @@ -162,11 +162,11 @@ class Http4sClient(
}
}

val timeout = IO.delay(TimeoutErrorAfter(streamReq, t).asLeft).delayBy(t)
val timeout = IO.delay(TimeoutErrorAfter(streamReq.detailedDescription, t).asLeft).delayBy(t)

IO.race(cornichonResponse, timeout)
.map(_.fold(identity, identity))
.handleError { t => RequestError(streamReq, t).asLeft }
.handleError { t => RequestError(streamReq.detailedDescription, t).asLeft }
}
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import scala.concurrent.duration.FiniteDuration
class NoOpHttpClient extends HttpClient {

def runRequest[A](cReq: HttpRequest[A], t: FiniteDuration)(implicit ee: EntityEncoder[IO, A], sh: Show[A]) =
EitherT.apply(IO.pure(HttpResponse(200, ("dummy" -> "value") :: Nil, "NoOpBody").asRight))
EitherT.apply(IO.pure(HttpResponse(200, Vector("dummy" -> "value"), "NoOpBody").asRight))

def openStream(req: HttpStreamedRequest, t: FiniteDuration) =
IO.pure(HttpResponse(200, Nil, "NoOpBody").asRight)
IO.pure(HttpResponse(200, Vector.empty, "NoOpBody").asRight)

def shutdown() =
Done.ioDone
Expand Down
Loading

0 comments on commit b2dbaac

Please sign in to comment.