Skip to content

Commit

Permalink
Logging cached requests (#706)
Browse files Browse the repository at this point in the history 
* Logging cached requests

* Fix scalafmt

---------

Co-authored-by: Fede Fernández <720923+fedefernandez@users.noreply.github.com>
  • Loading branch information
@a-khakimov @fedefernandez
a-khakimov and fedefernandez authored Mar 28, 2023
1 parent b1afbac commit fae0ac8
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 41 deletions.
12 changes: 6 additions & 6 deletions fetch-debug/src/main/scala/debug.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ object debug {

def showRequest(r: Request): Document =
r.request match {
case FetchOne(id, d) =>
Document.text(s"[Fetch one] From `${d.name}` with id $id") :-: showDuration(r.duration)
case Batch(ids, d) =>
Document.text(s"[Batch] From `${d.name}` with ids ${ids.toList}") :-: showDuration(
r.duration
)
case FetchOne(id, d, cached) =>
Document.text(s"[Fetch one] From `${d.name}` with id $id cached $cached") :-:
showDuration(r.duration)
case Batch(ids, d, cached) =>
Document.text(s"[Batch] From `${d.name}` with ids ${ids.toList} cached $cached") :-:
showDuration(r.duration)
}

def showMissing(d: Data[_, _], ids: List[_]): Document =
Expand Down
12 changes: 8 additions & 4 deletions fetch-examples/src/test/scala/GraphQLExample.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,21 @@ class GraphQLExample extends AnyWordSpec with Matchers {

def countFetches(r: Request): Int =
r.request match {
case FetchOne(_, _) => 1
case Batch(ids, _) => ids.toList.size
case FetchOne(_, _, false) => 1
case Batch(ids, _, false) => ids.toList.size
case FetchOne(_, _, true) => 0
case Batch(_, _, true) => 0
}

def totalFetched(rs: Seq[Round]): Int =
rs.map((round: Round) => round.queries.map(countFetches).sum).toList.sum

def countBatches(r: Request): Int =
r.request match {
case FetchOne(_, _) => 0
case Batch(_, _) => 1
case FetchOne(_, _, false) => 0
case Batch(_, _, false) => 1
case FetchOne(_, _, true) => 0
case Batch(_, _, true) => 0
}

def totalBatches(rs: Seq[Round]): Int =
Expand Down
40 changes: 26 additions & 14 deletions fetch/src/main/scala/fetch.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,29 @@ import cats.effect._
import cats.effect.implicits._
import cats.syntax.all._

import scala.collection.immutable.Map
import scala.util.control.NoStackTrace

object `package` {
private[fetch] sealed trait FetchRequest extends Product with Serializable
private[fetch] sealed trait FetchRequest extends Product with Serializable {
def cached: Boolean
}

private[fetch] sealed trait FetchQuery[I, A] extends FetchRequest {
def data: Data[I, A]
def identities: Set[I]
}
private[fetch] final case class FetchOne[I, A](id: I, data: Data[I, A]) extends FetchQuery[I, A] {
private[fetch] final case class FetchOne[I, A](
id: I,
data: Data[I, A],
cached: Boolean = false
) extends FetchQuery[I, A] {
override def identities: Set[I] = Set(id)
}
private[fetch] final case class Batch[I, A](ids: NonEmptyList[I], data: Data[I, A])
extends FetchQuery[I, A] {
private[fetch] final case class Batch[I, A](
ids: NonEmptyList[I],
data: Data[I, A],
cached: Boolean = false
) extends FetchQuery[I, A] {
override def identities: Set[I] = ids.toList.toSet
}

Expand Down Expand Up @@ -657,18 +665,21 @@ object `package` {
T: Clock[F]
): F[List[Request]] =
for {
startTime <- T.monotonic.map(_.toMillis)
c <- cache.get
maybeCached <- c.lookup(q.id, q.data)
result <- maybeCached match {
// Cached
case Some(v) => runCombinationResult(putResult, FetchDone(v)).as(Nil)
case Some(v) =>
runCombinationResult(putResult, FetchDone(v)).as(
List(Request(FetchOne(q.id, q.data, cached = true), startTime, startTime))
)

// Not cached, must fetch
case None =>
for {
startTime <- T.monotonic.map(_.toMillis)
o <- ds.fetch(q.id)
endTime <- T.monotonic.map(_.toMillis)
o <- ds.fetch(q.id)
endTime <- T.monotonic.map(_.toMillis)
result <- o match {
// Fetched
case Some(a) =>
Expand Down Expand Up @@ -711,18 +722,19 @@ object `package` {
result.tupleLeft(i).toRight(i)
}
cachedResults = cached.toMap
startTime <- T.monotonic.map(_.toMillis)
cachedRequests = cached.toNel.map(x =>
Request(Batch(x.map(_._1), q.data, cached = true), startTime, startTime)
)
result <- uncachedIds.toNel match {
// All cached
case None =>
runCombinationResult(putResult, FetchDone[Map[Any, Any]](cachedResults)).as(Nil)

// Some uncached
case Some(uncached) =>
val request = Batch[Any, Any](uncached, q.data)
for {
startTime <- T.monotonic.map(_.toMillis)

request = Batch[Any, Any](uncached, q.data)

batchedRequest <- ds.maxBatchSize match {
// Unbatched
case None =>
Expand All @@ -743,7 +755,7 @@ object `package` {

} yield batchedRequest.batches.map(Request(_, startTime, endTime))
}
} yield result
} yield result ++ cachedRequests.toList

private def runBatchedRequest[F[_]](
q: Batch[Any, Any],
Expand Down
2 changes: 1 addition & 1 deletion fetch/src/test/scala/FetchReportingTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class FetchReportingTests extends FetchSpec {
val io = Fetch.runLog[IO](fetch)

io.map { case (log, result) =>
log.rounds.size shouldEqual 2
log.rounds.size shouldEqual 3
totalBatches(log.rounds) shouldEqual 1
totalFetched(log.rounds) shouldEqual 3 + 1
}.unsafeToFuture()
Expand Down
12 changes: 8 additions & 4 deletions fetch/src/test/scala/FetchSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,21 @@ class FetchSpec extends AsyncFreeSpec with Matchers {

def countFetches(r: Request): Int =
r.request match {
case FetchOne(_, _) => 1
case Batch(ids, _) => ids.toList.size
case FetchOne(_, _, false) => 1
case Batch(ids, _, false) => ids.toList.size
case FetchOne(_, _, true) => 0
case Batch(_, _, true) => 0
}

def totalFetched(rs: Seq[Round]): Int =
rs.map((round: Round) => round.queries.map(countFetches).sum).toList.sum

def countBatches(r: Request): Int =
r.request match {
case FetchOne(_, _) => 0
case Batch(_, _) => 1
case FetchOne(_, _, false) => 0
case Batch(_, _, false) => 1
case FetchOne(_, _, true) => 0
case Batch(_, _, true) => 0
}

def totalBatches(rs: Seq[Round]): Int =
Expand Down
21 changes: 9 additions & 12 deletions fetch/src/test/scala/FetchTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,10 @@

package fetch

import scala.concurrent._
import java.util.concurrent._
import scala.concurrent.duration._

import cats._
import cats.data.NonEmptyList
import cats.effect._
import cats.instances.list._
import cats.instances.option._
import cats.data.NonEmptyList
import cats.syntax.all._
import fetch.syntax._

Expand Down Expand Up @@ -196,7 +191,7 @@ class FetchTests extends FetchSpec {
log.rounds.size shouldEqual 1
log.rounds.head.queries.size shouldEqual 1
log.rounds.head.queries.head.request should matchPattern {
case Batch(NonEmptyList(1, List(2)), _) =>
case Batch(NonEmptyList(1, List(2)), _, _) =>
}
}.unsafeToFuture()
}
Expand Down Expand Up @@ -284,7 +279,8 @@ class FetchTests extends FetchSpec {

io.map { case (log, result) =>
result shouldEqual ((1, 2), 3)
log.rounds.size shouldEqual 2
log.rounds.count(_.queries.forall(_.request.cached)) shouldEqual 1 // cached rounds
log.rounds.size shouldEqual 3 // all rounds
totalBatches(log.rounds) shouldEqual 2
totalFetched(log.rounds) shouldEqual 5
}.unsafeToFuture()
Expand All @@ -310,7 +306,8 @@ class FetchTests extends FetchSpec {

io.map { case (log, result) =>
result shouldEqual (1, 2)
log.rounds.size shouldEqual 2
log.rounds.count(_.queries.forall(_.request.cached)) shouldEqual 1 // cached rounds
log.rounds.size shouldEqual 3 // all rounds
totalBatches(log.rounds) shouldEqual 2
totalFetched(log.rounds) shouldEqual 4
}.unsafeToFuture()
Expand Down Expand Up @@ -530,8 +527,8 @@ class FetchTests extends FetchSpec {
val io = Fetch.runLog[IO](fetch)

io.map { case (log, result) =>
result shouldEqual 2
log.rounds.size shouldEqual 1
log.rounds.count(_.queries.forall(_.request.cached)) shouldEqual 7 // cached rounds
log.rounds.size shouldEqual 8 // all rounds
totalFetched(log.rounds) shouldEqual 3
}.unsafeToFuture()
}
Expand Down Expand Up @@ -561,7 +558,7 @@ class FetchTests extends FetchSpec {
io.map { case (log, result) =>
result shouldEqual 2
totalFetched(log.rounds) shouldEqual 0
log.rounds.size shouldEqual 0
log.rounds.size shouldEqual 8
}.unsafeToFuture()
}

Expand Down

0 comments on commit fae0ac8

Please sign in to comment.