From d5a5257325b309f7f7c9edf2fda6d932472ac1c9 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Wed, 13 Aug 2025 12:28:58 +0900 Subject: [PATCH 01/23] Basic impl --- forex-mtl/build.sbt | 1 + forex-mtl/project/Dependencies.scala | 1 + forex-mtl/src/main/scala/forex/Main.scala | 6 +- forex-mtl/src/main/scala/forex/Module.scala | 9 +- .../forex/services/rates/Interpreters.scala | 8 +- .../forex/services/rates/RateCache.scala | 72 +++++++++++ .../rates/interpreters/CachedOneFrame.scala | 74 +++++++++++ .../rates/interpreters/OneFrameClient.scala | 115 ++++++++++++++++++ 8 files changed, 279 insertions(+), 7 deletions(-) create mode 100644 forex-mtl/src/main/scala/forex/services/rates/RateCache.scala create mode 100644 forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala create mode 100644 forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala diff --git a/forex-mtl/build.sbt b/forex-mtl/build.sbt index 8994026f..dc40e223 100644 --- a/forex-mtl/build.sbt +++ b/forex-mtl/build.sbt @@ -56,6 +56,7 @@ libraryDependencies ++= Seq( Libraries.fs2, Libraries.http4sDsl, Libraries.http4sServer, + Libraries.http4sClient, Libraries.http4sCirce, Libraries.circeCore, Libraries.circeGeneric, diff --git a/forex-mtl/project/Dependencies.scala b/forex-mtl/project/Dependencies.scala index 423210a1..0acb0300 100644 --- a/forex-mtl/project/Dependencies.scala +++ b/forex-mtl/project/Dependencies.scala @@ -27,6 +27,7 @@ object Dependencies { lazy val http4sDsl = http4s("http4s-dsl") lazy val http4sServer = http4s("http4s-blaze-server") + lazy val http4sClient = http4s("http4s-blaze-client") lazy val http4sCirce = http4s("http4s-circe") lazy val circeCore = circe("circe-core") lazy val circeGeneric = circe("circe-generic") diff --git a/forex-mtl/src/main/scala/forex/Main.scala b/forex-mtl/src/main/scala/forex/Main.scala index 6dda10a7..24167a51 100644 --- a/forex-mtl/src/main/scala/forex/Main.scala +++ b/forex-mtl/src/main/scala/forex/Main.scala @@ -13,9 +13,10 @@ object Main extends IOApp { } -class Application[F[_]: ConcurrentEffect: Timer] { +class Application[F[_]: ConcurrentEffect: Timer: Clock] { - def stream(ec: ExecutionContext): Stream[F, Unit] = + def stream(ec: ExecutionContext): Stream[F, Unit] = { + implicit val implicitEc: ExecutionContext = ec for { config <- Config.stream("app") module = new Module[F](config) @@ -24,5 +25,6 @@ class Application[F[_]: ConcurrentEffect: Timer] { .withHttpApp(module.httpApp) .serve } yield () + } } diff --git a/forex-mtl/src/main/scala/forex/Module.scala b/forex-mtl/src/main/scala/forex/Module.scala index 3bc47d58..bb60c6ed 100644 --- a/forex-mtl/src/main/scala/forex/Module.scala +++ b/forex-mtl/src/main/scala/forex/Module.scala @@ -1,17 +1,18 @@ package forex -import cats.effect.{ Concurrent, Timer } +import cats.effect.{Clock, ConcurrentEffect, Timer} import forex.config.ApplicationConfig import forex.http.rates.RatesHttpRoutes import forex.services._ import forex.programs._ import org.http4s._ import org.http4s.implicits._ -import org.http4s.server.middleware.{ AutoSlash, Timeout } +import org.http4s.server.middleware.{AutoSlash, Timeout} +import scala.concurrent.ExecutionContext -class Module[F[_]: Concurrent: Timer](config: ApplicationConfig) { +class Module[F[_]: Timer: ConcurrentEffect: Clock](config: ApplicationConfig)(implicit ec: ExecutionContext) { - private val ratesService: RatesService[F] = RatesServices.dummy[F] + private val ratesService: RatesService[F] = RatesServices.cachedOneFrame[F] private val ratesProgram: RatesProgram[F] = RatesProgram[F](ratesService) diff --git a/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala b/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala index e523ffab..17d38c53 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala @@ -1,8 +1,14 @@ package forex.services.rates import cats.Applicative +import cats.effect.{Clock, ConcurrentEffect} import interpreters._ +import scala.concurrent.ExecutionContext object Interpreters { - def dummy[F[_]: Applicative]: Algebra[F] = new OneFrameDummy[F]() + def dummy[F[_]: Applicative](implicit @annotation.unused ec: ExecutionContext): Algebra[F] = + new OneFrameDummy[F]() + + def cachedOneFrame[F[_]: ConcurrentEffect: Clock](implicit ec: ExecutionContext): Algebra[F] = + CachedOneFrame[F] } diff --git a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala new file mode 100644 index 00000000..957a14c7 --- /dev/null +++ b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala @@ -0,0 +1,72 @@ +package forex.services.rates + +import cats.effect.{Clock, Sync} +import cats.syntax.functor._ +import forex.domain.Rate + +import java.time.Instant +import java.util.concurrent.ConcurrentHashMap +import scala.concurrent.duration._ +import java.util.concurrent.TimeUnit.MILLISECONDS +import scala.jdk.CollectionConverters._ + +case class CachedRate(rate: Rate, expiresAt: Instant) + +class RateCache[F[_]: Sync: Clock] { + + private val cache = new ConcurrentHashMap[Rate.Pair, CachedRate]() + private val trackedPairs = ConcurrentHashMap.newKeySet[Rate.Pair]() + private val ttl = 5.minutes + + def get(pair: Rate.Pair): F[Option[Rate]] = { + trackedPairs.add(pair) + + Clock[F].realTime(MILLISECONDS).map { nowMillis => + Option(cache.get(pair)).flatMap { cachedRate => + if (cachedRate.expiresAt.isAfter(Instant.ofEpochMilli(nowMillis))) { + Some(cachedRate.rate) + } else { + cache.remove(pair) + None + } + } + } + } + + def put(rate: Rate): F[Unit] = { + Sync[F].delay { + val apiTimestamp = rate.timestamp.value.toInstant + val expiresAt = apiTimestamp.plusSeconds(ttl.toSeconds) + cache.put(rate.pair, CachedRate(rate, expiresAt)) + () + } + } + + def clear(): F[Unit] = Sync[F].delay(cache.clear()) + + def getTrackedPairs: F[List[Rate.Pair]] = { + Sync[F].delay(trackedPairs.asScala.toList) + } + + def getExpiredTrackedPairs: F[List[Rate.Pair]] = { + Clock[F].realTime(MILLISECONDS).map { nowMillis => + val now = Instant.ofEpochMilli(nowMillis) + trackedPairs.asScala.toList.filter { pair => + Option(cache.get(pair)) match { + case Some(cachedRate) => cachedRate.expiresAt.isBefore(now) || cachedRate.expiresAt.equals(now) + case None => true + } + } + } + } + + def putBatch(rates: List[Rate]): F[Unit] = { + Sync[F].delay { + rates.foreach { rate => + val apiTimestamp = rate.timestamp.value.toInstant + val expiresAt = apiTimestamp.plusSeconds(ttl.toSeconds) + cache.put(rate.pair, CachedRate(rate, expiresAt)) + } + } + } +} \ No newline at end of file diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala new file mode 100644 index 00000000..12ea8b9f --- /dev/null +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -0,0 +1,74 @@ +package forex.services.rates.interpreters + +import cats.effect.{Clock, ConcurrentEffect, Sync} +import cats.implicits.{toShow} +import cats.syntax.either._ +import cats.syntax.flatMap._ +import cats.syntax.functor._ +import forex.domain.Rate +import forex.services.rates.errors.Error.OneFrameLookupFailed +import forex.services.rates.{Algebra, RateCache} +import forex.services.rates.errors._ +import org.slf4j.LoggerFactory + +import scala.concurrent.ExecutionContext + +class CachedOneFrame[F[_]: ConcurrentEffect]( + client: Algebra[F], + cache: RateCache[F] +) extends Algebra[F] { + + private val logger = LoggerFactory.getLogger(classOf[CachedOneFrame[F]]) + + override def get(pair: Rate.Pair): F[Error Either Rate] = { + Sync[F].delay(logger.info(s"Requesting rate for pair: ${pair.from.show}${pair.to.show}")) >> + cache.get(pair).flatMap { + case Some(cachedRate) => + Sync[F].delay(logger.info(s"Cache HIT for ${pair.from.show}${pair.to.show}")) >> + ConcurrentEffect[F].pure(cachedRate.asRight[Error]) + case None => + Sync[F].delay(logger.info(s"Cache MISS for ${pair.from.show}${pair.to.show}")) >> + cache.getExpiredTrackedPairs.flatMap { expiredPairs => + if (expiredPairs.nonEmpty) { + val expiredPairsStr = expiredPairs.map(p => s"${p.from.show}${p.to.show}").mkString(", ") + Sync[F].delay(logger.info(s"Found expired tracked pairs: [${expiredPairsStr}]. Making batch request.")) >> + client.asInstanceOf[OneFrameClient[F]].getBatch(expiredPairs).flatMap { + case Right(rates) => + Sync[F].delay(logger.info(s"Batch API call successful. Received ${rates.length} rates. Caching all.")) >> + cache.putBatch(rates).flatMap { _ => + rates.find(_.pair == pair) match { + case Some(rate) => + Sync[F].delay(logger.info(s"Returning requested rate for ${pair.from.show}${pair.to.show}: ${rate.price.value}")) >> + ConcurrentEffect[F].pure(rate.asRight[Error]) + case None => + Sync[F].delay(logger.info(s"ERROR: Requested pair ${pair.from.show}${pair.to.show} not found in batch response")) >> + ConcurrentEffect[F].pure(OneFrameLookupFailed("Pair not found in response").asLeft[Rate]) + } + } + case Left(error) => + Sync[F].delay(logger.info(s"Batch API call failed: ${error}")) >> + ConcurrentEffect[F].pure(error.asLeft[Rate]) + } + } else { + Sync[F].delay(logger.info(s"No other expired pairs. Making single request for ${pair.from.show}${pair.to.show}")) >> + client.get(pair).flatMap { + case Right(rate) => + Sync[F].delay(logger.info(s"Single API call successful. Caching rate for ${pair.from.show}${pair.to.show}: ${rate.price.value}")) >> + cache.put(rate).map(_ => rate.asRight[Error]) + case Left(error) => + Sync[F].delay(logger.info(s"Single API call failed for ${pair.from.show}${pair.to.show}: ${error}")) >> + ConcurrentEffect[F].pure(error.asLeft[Rate]) + } + } + } + } + } +} + +object CachedOneFrame { + def apply[F[_]: ConcurrentEffect: Clock](implicit ec: ExecutionContext): CachedOneFrame[F] = { + val client = new OneFrameClient[F]() + val cache = new RateCache[F]() + new CachedOneFrame[F](client, cache) + } +} \ No newline at end of file diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala new file mode 100644 index 00000000..29fcafe3 --- /dev/null +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -0,0 +1,115 @@ +package forex.services.rates.interpreters + +import cats.effect.{ConcurrentEffect, Sync} +import cats.implicits.{catsSyntaxApplicativeError, toFlatMapOps, toShow} +import cats.syntax.either._ +import cats.syntax.functor._ +import forex.domain.{Currency, Price, Rate, Timestamp} +import forex.services.rates.Algebra +import forex.services.rates.errors.Error.OneFrameLookupFailed +import forex.services.rates.errors._ +import io.circe.generic.auto._ +import org.http4s.circe.CirceEntityDecoder._ +import org.http4s.blaze.client.BlazeClientBuilder +import org.http4s.{Header, Headers, Method, Request, Uri} +import org.slf4j.LoggerFactory +import org.typelevel.ci._ + +import java.time.OffsetDateTime +import scala.concurrent.ExecutionContext + +case class OneFrameResponse( + from: String, + to: String, + price: BigDecimal, + time_stamp: String +) + +class OneFrameClient[F[_]: ConcurrentEffect](implicit ec: ExecutionContext) extends Algebra[F] { + + private val logger = LoggerFactory.getLogger(classOf[OneFrameClient[F]]) + + def getBatch(pairs: List[Rate.Pair]): F[Error Either List[Rate]] = { + val logInfo = (msg: String) => Sync[F].delay(logger.info(msg)) + if (pairs.isEmpty) { + logInfo("Empty batch request, returning empty list").flatMap { _ => + ConcurrentEffect[F].pure(List.empty[Rate].asRight[Error]) + } + } else { + val pairsStr = pairs.map(p => s"${p.from.show}${p.to.show}").mkString(", ") + val uri = pairs.foldLeft(Uri.unsafeFromString("http://localhost:8080/rates")) { (uri, pair) => + val pairString = s"${pair.from.show}${pair.to.show}" + uri.withQueryParam("pair", pairString) + } + + logInfo(s"Making batch HTTP request for pairs: [${pairsStr}]").flatMap { _ => + logInfo(s"Batch request URL: ${uri.toString}").flatMap { _ => + BlazeClientBuilder[F](ec).resource.use { client => + val request = Request[F]( + method = Method.GET, + uri = uri, + headers = Headers.apply(Header.Raw.apply(name = ci"token", value = "10dc303535874aeccc86a8251e6992f5")) + ) + + client.expect[List[OneFrameResponse]](request).flatMap { responses => + logInfo(s"Batch HTTP response received: ${responses.length} rates").map { _ => + val rates = responses.map { response => + Rate( + Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), + Price(response.price), + Timestamp(OffsetDateTime.parse(response.time_stamp)) + ) + } + rates.asRight[Error] + } + }.handleError { ex => + logger.error(s"Batch request failed: ${ex.getMessage}", ex) + (OneFrameLookupFailed("Request failed"): Error).asLeft[List[Rate]] + } + } + } + } + } + } + + override def get(pair: Rate.Pair): F[Error Either Rate] = { + val logInfo = (msg: String) => Sync[F].delay(logger.info(msg)) + val pairString = s"${pair.from.show}${pair.to.show}" + val uri = Uri.unsafeFromString("http://localhost:8080/rates").withQueryParam("pair", pairString) + + logInfo(s"Making single HTTP request for pair: ${pairString}").flatMap { _ => + logInfo(s"Single request URL: ${uri.toString}").flatMap { _ => + BlazeClientBuilder[F](ec).resource.use { client => + val request = Request[F]( + method = Method.GET, + uri = uri, + headers = Headers.apply(Header.Raw.apply(name = ci"token", value = "10dc303535874aeccc86a8251e6992f5")) + ) + + client.expect[List[OneFrameResponse]](request).flatMap { rates => + logInfo(s"Single HTTP response received: ${rates.length} rates").flatMap { _ => + rates.headOption match { + case Some(response) => + val rate = Rate( + Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), + Price(response.price), + Timestamp(OffsetDateTime.parse(response.time_stamp)) + ) + logInfo(s"Single request successful for ${pairString}: price=${response.price}, timestamp=${response.time_stamp}").map { _ => + rate.asRight[Error] + } + case None => + logInfo(s"Single request returned empty list for ${pairString}").map { _ => + (OneFrameLookupFailed("No rate found"): Error).asLeft[Rate] + } + } + } + }.handleError { ex => + logger.error(s"Request failed: ${ex.getMessage}", ex) + (OneFrameLookupFailed("Request failed"): Error).asLeft[Rate] + } + } + } + } + } +} \ No newline at end of file From f7128df42d817527e7b58d6dbb045af861c70aec Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Wed, 13 Aug 2025 16:30:56 +0900 Subject: [PATCH 02/23] Params moved to config file, batch request fixed --- forex-mtl/src/main/resources/application.conf | 11 ++- forex-mtl/src/main/scala/forex/Module.scala | 2 +- .../forex/config/ApplicationConfig.scala | 11 +++ .../forex/services/rates/Interpreters.scala | 8 +- .../forex/services/rates/RateCache.scala | 6 +- .../rates/interpreters/CachedOneFrame.scala | 10 +- .../rates/interpreters/OneFrameClient.scala | 94 +++++++------------ 7 files changed, 71 insertions(+), 71 deletions(-) diff --git a/forex-mtl/src/main/resources/application.conf b/forex-mtl/src/main/resources/application.conf index b2af6efd..ed66444e 100644 --- a/forex-mtl/src/main/resources/application.conf +++ b/forex-mtl/src/main/resources/application.conf @@ -1,8 +1,17 @@ app { http { host = "0.0.0.0" - port = 8080 + port = 8085 timeout = 40 seconds } + + one-frame { + url = "http://localhost:8086" + token = "10dc303535874aeccc86a8251e6992f5" + } + + cache { + ttl = 1 minutes + } } diff --git a/forex-mtl/src/main/scala/forex/Module.scala b/forex-mtl/src/main/scala/forex/Module.scala index bb60c6ed..328db6fa 100644 --- a/forex-mtl/src/main/scala/forex/Module.scala +++ b/forex-mtl/src/main/scala/forex/Module.scala @@ -12,7 +12,7 @@ import scala.concurrent.ExecutionContext class Module[F[_]: Timer: ConcurrentEffect: Clock](config: ApplicationConfig)(implicit ec: ExecutionContext) { - private val ratesService: RatesService[F] = RatesServices.cachedOneFrame[F] + private val ratesService: RatesService[F] = RatesServices.cachedOneFrame[F](config.oneFrame, config.cache) private val ratesProgram: RatesProgram[F] = RatesProgram[F](ratesService) diff --git a/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala b/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala index eff0fad7..b8e58592 100644 --- a/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala +++ b/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala @@ -4,6 +4,8 @@ import scala.concurrent.duration.FiniteDuration case class ApplicationConfig( http: HttpConfig, + oneFrame: OneFrameConfig, + cache: CacheConfig ) case class HttpConfig( @@ -11,3 +13,12 @@ case class HttpConfig( port: Int, timeout: FiniteDuration ) + +case class OneFrameConfig( + url: String, + token: String +) + +case class CacheConfig( + ttl: FiniteDuration +) diff --git a/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala b/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala index 17d38c53..4a5f509c 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala @@ -2,6 +2,7 @@ package forex.services.rates import cats.Applicative import cats.effect.{Clock, ConcurrentEffect} +import forex.config.{CacheConfig, OneFrameConfig} import interpreters._ import scala.concurrent.ExecutionContext @@ -9,6 +10,9 @@ object Interpreters { def dummy[F[_]: Applicative](implicit @annotation.unused ec: ExecutionContext): Algebra[F] = new OneFrameDummy[F]() - def cachedOneFrame[F[_]: ConcurrentEffect: Clock](implicit ec: ExecutionContext): Algebra[F] = - CachedOneFrame[F] + def cachedOneFrame[F[_]: ConcurrentEffect: Clock]( + oneFrameConfig: OneFrameConfig, + cacheConfig: CacheConfig + )(implicit ec: ExecutionContext): Algebra[F] = + CachedOneFrame[F](oneFrameConfig, cacheConfig) } diff --git a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala index 957a14c7..fe47ace1 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala @@ -2,21 +2,21 @@ package forex.services.rates import cats.effect.{Clock, Sync} import cats.syntax.functor._ +import forex.config.CacheConfig import forex.domain.Rate import java.time.Instant import java.util.concurrent.ConcurrentHashMap -import scala.concurrent.duration._ import java.util.concurrent.TimeUnit.MILLISECONDS import scala.jdk.CollectionConverters._ case class CachedRate(rate: Rate, expiresAt: Instant) -class RateCache[F[_]: Sync: Clock] { +class RateCache[F[_]: Sync: Clock](config: CacheConfig) { private val cache = new ConcurrentHashMap[Rate.Pair, CachedRate]() private val trackedPairs = ConcurrentHashMap.newKeySet[Rate.Pair]() - private val ttl = 5.minutes + private val ttl = config.ttl def get(pair: Rate.Pair): F[Option[Rate]] = { trackedPairs.add(pair) diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index 12ea8b9f..9223f397 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -5,6 +5,7 @@ import cats.implicits.{toShow} import cats.syntax.either._ import cats.syntax.flatMap._ import cats.syntax.functor._ +import forex.config.{CacheConfig, OneFrameConfig} import forex.domain.Rate import forex.services.rates.errors.Error.OneFrameLookupFailed import forex.services.rates.{Algebra, RateCache} @@ -66,9 +67,12 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( } object CachedOneFrame { - def apply[F[_]: ConcurrentEffect: Clock](implicit ec: ExecutionContext): CachedOneFrame[F] = { - val client = new OneFrameClient[F]() - val cache = new RateCache[F]() + def apply[F[_]: ConcurrentEffect: Clock]( + oneFrameConfig: OneFrameConfig, + cacheConfig: CacheConfig + )(implicit ec: ExecutionContext): CachedOneFrame[F] = { + val client = new OneFrameClient[F](oneFrameConfig) + val cache = new RateCache[F](cacheConfig) new CachedOneFrame[F](client, cache) } } \ No newline at end of file diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index 29fcafe3..e47b9cb7 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -15,6 +15,7 @@ import org.http4s.{Header, Headers, Method, Request, Uri} import org.slf4j.LoggerFactory import org.typelevel.ci._ +import forex.config.OneFrameConfig import java.time.OffsetDateTime import scala.concurrent.ExecutionContext @@ -25,7 +26,7 @@ case class OneFrameResponse( time_stamp: String ) -class OneFrameClient[F[_]: ConcurrentEffect](implicit ec: ExecutionContext) extends Algebra[F] { +class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec: ExecutionContext) extends Algebra[F] { private val logger = LoggerFactory.getLogger(classOf[OneFrameClient[F]]) @@ -36,80 +37,51 @@ class OneFrameClient[F[_]: ConcurrentEffect](implicit ec: ExecutionContext) exte ConcurrentEffect[F].pure(List.empty[Rate].asRight[Error]) } } else { - val pairsStr = pairs.map(p => s"${p.from.show}${p.to.show}").mkString(", ") - val uri = pairs.foldLeft(Uri.unsafeFromString("http://localhost:8080/rates")) { (uri, pair) => - val pairString = s"${pair.from.show}${pair.to.show}" - uri.withQueryParam("pair", pairString) - } + val pairStrings = pairs.map(p => s"${p.from.show}${p.to.show}") + val pairsStr = pairStrings.mkString(", ") + + val queryString = pairStrings.map(p => s"pair=$p").mkString("&") + val uriString = s"${config.url}/rates?$queryString" + val uri = Uri.unsafeFromString(uriString) logInfo(s"Making batch HTTP request for pairs: [${pairsStr}]").flatMap { _ => logInfo(s"Batch request URL: ${uri.toString}").flatMap { _ => BlazeClientBuilder[F](ec).resource.use { client => - val request = Request[F]( - method = Method.GET, - uri = uri, - headers = Headers.apply(Header.Raw.apply(name = ci"token", value = "10dc303535874aeccc86a8251e6992f5")) - ) + val request = Request[F]( + method = Method.GET, + uri = uri, + headers = Headers.apply(Header.Raw.apply(name = ci"token", value = config.token)) + ) - client.expect[List[OneFrameResponse]](request).flatMap { responses => - logInfo(s"Batch HTTP response received: ${responses.length} rates").map { _ => - val rates = responses.map { response => - Rate( - Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), - Price(response.price), - Timestamp(OffsetDateTime.parse(response.time_stamp)) - ) - } - rates.asRight[Error] - } - }.handleError { ex => - logger.error(s"Batch request failed: ${ex.getMessage}", ex) - (OneFrameLookupFailed("Request failed"): Error).asLeft[List[Rate]] - } - } - } - } - } - } - - override def get(pair: Rate.Pair): F[Error Either Rate] = { - val logInfo = (msg: String) => Sync[F].delay(logger.info(msg)) - val pairString = s"${pair.from.show}${pair.to.show}" - val uri = Uri.unsafeFromString("http://localhost:8080/rates").withQueryParam("pair", pairString) - - logInfo(s"Making single HTTP request for pair: ${pairString}").flatMap { _ => - logInfo(s"Single request URL: ${uri.toString}").flatMap { _ => - BlazeClientBuilder[F](ec).resource.use { client => - val request = Request[F]( - method = Method.GET, - uri = uri, - headers = Headers.apply(Header.Raw.apply(name = ci"token", value = "10dc303535874aeccc86a8251e6992f5")) - ) - - client.expect[List[OneFrameResponse]](request).flatMap { rates => - logInfo(s"Single HTTP response received: ${rates.length} rates").flatMap { _ => - rates.headOption match { - case Some(response) => - val rate = Rate( + client.expect[List[OneFrameResponse]](request).flatMap { responses => + logInfo(s"Batch HTTP response received: ${responses.length} rates").map { _ => + val rates = responses.map { response => + Rate( Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), Price(response.price), Timestamp(OffsetDateTime.parse(response.time_stamp)) ) - logInfo(s"Single request successful for ${pairString}: price=${response.price}, timestamp=${response.time_stamp}").map { _ => - rate.asRight[Error] - } - case None => - logInfo(s"Single request returned empty list for ${pairString}").map { _ => - (OneFrameLookupFailed("No rate found"): Error).asLeft[Rate] - } + } + rates.asRight[Error] } + }.handleError { ex => + logger.error(s"Batch request failed: ${ex.getMessage}", ex) + (OneFrameLookupFailed("Request failed"): Error).asLeft[List[Rate]] } - }.handleError { ex => - logger.error(s"Request failed: ${ex.getMessage}", ex) - (OneFrameLookupFailed("Request failed"): Error).asLeft[Rate] } } } } } + + override def get(pair: Rate.Pair): F[Error Either Rate] = { + getBatch(List(pair)).map { + case Right(rates) => + rates.headOption match { + case Some(rate) => rate.asRight[Error] + case None => (OneFrameLookupFailed("No rate found"): Error).asLeft[Rate] + } + case Left(error) => error.asLeft[Rate] + } + } } \ No newline at end of file From 8f4acf04b0c1003fd5e2851ff361dbafa5c06997 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Wed, 13 Aug 2025 16:39:15 +0900 Subject: [PATCH 03/23] Fixed code duplication --- .../src/main/scala/forex/services/rates/RateCache.scala | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala index fe47ace1..71b34929 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala @@ -34,12 +34,7 @@ class RateCache[F[_]: Sync: Clock](config: CacheConfig) { } def put(rate: Rate): F[Unit] = { - Sync[F].delay { - val apiTimestamp = rate.timestamp.value.toInstant - val expiresAt = apiTimestamp.plusSeconds(ttl.toSeconds) - cache.put(rate.pair, CachedRate(rate, expiresAt)) - () - } + putBatch(List(rate)) } def clear(): F[Unit] = Sync[F].delay(cache.clear()) From 7374f126de84c6c59ef19ee8ba1087b4fe159658 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Thu, 14 Aug 2025 10:54:15 +0900 Subject: [PATCH 04/23] Added tests. CachedOneFrameIntegrationSpec fixed --- forex-mtl/build.sbt | 7 +- forex-mtl/project/Dependencies.scala | 18 +- .../forex/services/rates/Interpreters.scala | 4 - .../forex/services/rates/RateCache.scala | 5 + .../scala/forex/services/rates/algebra.scala | 11 ++ .../rates/interpreters/CachedOneFrame.scala | 57 +++--- .../rates/interpreters/OneFrameClient.scala | 23 +-- .../rates/interpreters/OneFrameDummy.scala | 15 -- .../scala/forex/helpers/MockAlgebra.scala | 96 +++++++++ .../test/scala/forex/helpers/TestClock.scala | 48 +++++ .../test/scala/forex/helpers/TestData.scala | 47 +++++ .../CachedOneFrameIntegrationSpec.scala | 177 +++++++++++++++++ .../forex/performance/PerformanceSpec.scala | 155 +++++++++++++++ .../CachedOneFramePropertySpec.scala | 172 ++++++++++++++++ .../forex/services/rates/RateCacheSpec.scala | 131 +++++++++++++ .../interpreters/CachedOneFrameSpec.scala | 183 ++++++++++++++++++ .../interpreters/OneFrameClientSpec.scala | 98 ++++++++++ 17 files changed, 1169 insertions(+), 78 deletions(-) delete mode 100644 forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameDummy.scala create mode 100644 forex-mtl/src/test/scala/forex/helpers/MockAlgebra.scala create mode 100644 forex-mtl/src/test/scala/forex/helpers/TestClock.scala create mode 100644 forex-mtl/src/test/scala/forex/helpers/TestData.scala create mode 100644 forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala create mode 100644 forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala create mode 100644 forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala create mode 100644 forex-mtl/src/test/scala/forex/services/rates/RateCacheSpec.scala create mode 100644 forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala create mode 100644 forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala diff --git a/forex-mtl/build.sbt b/forex-mtl/build.sbt index dc40e223..e48a45ef 100644 --- a/forex-mtl/build.sbt +++ b/forex-mtl/build.sbt @@ -64,7 +64,8 @@ libraryDependencies ++= Seq( Libraries.circeParser, Libraries.pureConfig, Libraries.logback, - Libraries.scalaTest % Test, - Libraries.scalaCheck % Test, - Libraries.catsScalaCheck % Test + Libraries.scalaTest % Test, + Libraries.scalaCheck % Test, + Libraries.catsScalaCheck % Test, + Libraries.scalaTestPlusCheck % Test ) diff --git a/forex-mtl/project/Dependencies.scala b/forex-mtl/project/Dependencies.scala index 0acb0300..2e70e579 100644 --- a/forex-mtl/project/Dependencies.scala +++ b/forex-mtl/project/Dependencies.scala @@ -10,11 +10,12 @@ object Dependencies { val circe = "0.14.2" val pureConfig = "0.17.4" - val kindProjector = "0.13.2" - val logback = "1.2.3" - val scalaCheck = "1.15.3" - val scalaTest = "3.2.7" - val catsScalaCheck = "0.3.2" + val kindProjector = "0.13.2" + val logback = "1.2.3" + val scalaCheck = "1.15.3" + val scalaTest = "3.2.7" + val catsScalaCheck = "0.3.2" + val scalaTestPlusCheck = "3.2.7.0" } object Libraries { @@ -42,9 +43,10 @@ object Dependencies { lazy val logback = "ch.qos.logback" % "logback-classic" % Versions.logback // Test - lazy val scalaTest = "org.scalatest" %% "scalatest" % Versions.scalaTest - lazy val scalaCheck = "org.scalacheck" %% "scalacheck" % Versions.scalaCheck - lazy val catsScalaCheck = "io.chrisdavenport" %% "cats-scalacheck" % Versions.catsScalaCheck + lazy val scalaTest = "org.scalatest" %% "scalatest" % Versions.scalaTest + lazy val scalaCheck = "org.scalacheck" %% "scalacheck" % Versions.scalaCheck + lazy val catsScalaCheck = "io.chrisdavenport" %% "cats-scalacheck" % Versions.catsScalaCheck + lazy val scalaTestPlusCheck = "org.scalatestplus" %% "scalacheck-1-15" % Versions.scalaTestPlusCheck } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala b/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala index 4a5f509c..40fa39f0 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/Interpreters.scala @@ -1,15 +1,11 @@ package forex.services.rates -import cats.Applicative import cats.effect.{Clock, ConcurrentEffect} import forex.config.{CacheConfig, OneFrameConfig} import interpreters._ import scala.concurrent.ExecutionContext object Interpreters { - def dummy[F[_]: Applicative](implicit @annotation.unused ec: ExecutionContext): Algebra[F] = - new OneFrameDummy[F]() - def cachedOneFrame[F[_]: ConcurrentEffect: Clock]( oneFrameConfig: OneFrameConfig, cacheConfig: CacheConfig diff --git a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala index 71b34929..978e784c 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala @@ -1,9 +1,11 @@ package forex.services.rates import cats.effect.{Clock, Sync} +import cats.implicits.toShow import cats.syntax.functor._ import forex.config.CacheConfig import forex.domain.Rate +import org.slf4j.LoggerFactory import java.time.Instant import java.util.concurrent.ConcurrentHashMap @@ -17,6 +19,7 @@ class RateCache[F[_]: Sync: Clock](config: CacheConfig) { private val cache = new ConcurrentHashMap[Rate.Pair, CachedRate]() private val trackedPairs = ConcurrentHashMap.newKeySet[Rate.Pair]() private val ttl = config.ttl + private val logger = LoggerFactory.getLogger(classOf[RateCache[F]]) def get(pair: Rate.Pair): F[Option[Rate]] = { trackedPairs.add(pair) @@ -24,8 +27,10 @@ class RateCache[F[_]: Sync: Clock](config: CacheConfig) { Clock[F].realTime(MILLISECONDS).map { nowMillis => Option(cache.get(pair)).flatMap { cachedRate => if (cachedRate.expiresAt.isAfter(Instant.ofEpochMilli(nowMillis))) { + logger.info(s"Cache HIT for ${pair.from.show}${pair.to.show}") Some(cachedRate.rate) } else { + logger.info(s"Cache OUTDATED for ${pair.from.show}${pair.to.show}. Now: ${Instant.ofEpochMilli(nowMillis)}, expires at: ${cachedRate.expiresAt}") cache.remove(pair) None } diff --git a/forex-mtl/src/main/scala/forex/services/rates/algebra.scala b/forex-mtl/src/main/scala/forex/services/rates/algebra.scala index 8966dce5..d0ce5404 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/algebra.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/algebra.scala @@ -3,6 +3,17 @@ package forex.services.rates import forex.domain.Rate import errors._ +import cats.Applicative +import cats.implicits._ + trait Algebra[F[_]] { def get(pair: Rate.Pair): F[Error Either Rate] + + def getBatch(pairs: List[Rate.Pair])(implicit F: Applicative[F]): F[Error Either List[Rate]] = { + pairs.traverse(get).map { results => + val (errors, rates) = results.separate + if (errors.isEmpty) rates.asRight[Error] + else errors.head.asLeft[List[Rate]] + } + } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index 9223f397..05cda54b 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -1,10 +1,9 @@ package forex.services.rates.interpreters -import cats.effect.{Clock, ConcurrentEffect, Sync} -import cats.implicits.{toShow} +import cats.effect.{Clock, ConcurrentEffect} +import cats.implicits.toShow import cats.syntax.either._ import cats.syntax.flatMap._ -import cats.syntax.functor._ import forex.config.{CacheConfig, OneFrameConfig} import forex.domain.Rate import forex.services.rates.errors.Error.OneFrameLookupFailed @@ -22,48 +21,36 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( private val logger = LoggerFactory.getLogger(classOf[CachedOneFrame[F]]) override def get(pair: Rate.Pair): F[Error Either Rate] = { - Sync[F].delay(logger.info(s"Requesting rate for pair: ${pair.from.show}${pair.to.show}")) >> cache.get(pair).flatMap { case Some(cachedRate) => - Sync[F].delay(logger.info(s"Cache HIT for ${pair.from.show}${pair.to.show}")) >> + logger.debug(s"Cache HIT for ${pair.from.show}${pair.to.show}") ConcurrentEffect[F].pure(cachedRate.asRight[Error]) case None => - Sync[F].delay(logger.info(s"Cache MISS for ${pair.from.show}${pair.to.show}")) >> + logger.debug(s"Cache MISS for ${pair.from.show}${pair.to.show}") cache.getExpiredTrackedPairs.flatMap { expiredPairs => - if (expiredPairs.nonEmpty) { - val expiredPairsStr = expiredPairs.map(p => s"${p.from.show}${p.to.show}").mkString(", ") - Sync[F].delay(logger.info(s"Found expired tracked pairs: [${expiredPairsStr}]. Making batch request.")) >> - client.asInstanceOf[OneFrameClient[F]].getBatch(expiredPairs).flatMap { - case Right(rates) => - Sync[F].delay(logger.info(s"Batch API call successful. Received ${rates.length} rates. Caching all.")) >> - cache.putBatch(rates).flatMap { _ => - rates.find(_.pair == pair) match { - case Some(rate) => - Sync[F].delay(logger.info(s"Returning requested rate for ${pair.from.show}${pair.to.show}: ${rate.price.value}")) >> - ConcurrentEffect[F].pure(rate.asRight[Error]) - case None => - Sync[F].delay(logger.info(s"ERROR: Requested pair ${pair.from.show}${pair.to.show} not found in batch response")) >> - ConcurrentEffect[F].pure(OneFrameLookupFailed("Pair not found in response").asLeft[Rate]) - } + val pairsToFetch = (expiredPairs :+ pair).distinct // here could be duplication but this way we can see how it's working + val pairsStr = pairsToFetch.map(p => s"${p.from.show}${p.to.show}").mkString(", ") + logger.info(s"Batch request for pairs: [${pairsStr}]") + + client.getBatch(pairsToFetch).flatMap { + case Right(rates) => + cache.putBatch(rates).flatMap { _ => + rates.find(_.pair == pair) match { + case Some(rate) => + ConcurrentEffect[F].pure(rate.asRight[Error]) + case None => + logger.warn(s"Requested pair ${pair.from.show}${pair.to.show} not found in batch response") + ConcurrentEffect[F].pure(OneFrameLookupFailed("Pair not found in response").asLeft[Rate]) } - case Left(error) => - Sync[F].delay(logger.info(s"Batch API call failed: ${error}")) >> - ConcurrentEffect[F].pure(error.asLeft[Rate]) - } - } else { - Sync[F].delay(logger.info(s"No other expired pairs. Making single request for ${pair.from.show}${pair.to.show}")) >> - client.get(pair).flatMap { - case Right(rate) => - Sync[F].delay(logger.info(s"Single API call successful. Caching rate for ${pair.from.show}${pair.to.show}: ${rate.price.value}")) >> - cache.put(rate).map(_ => rate.asRight[Error]) - case Left(error) => - Sync[F].delay(logger.info(s"Single API call failed for ${pair.from.show}${pair.to.show}: ${error}")) >> - ConcurrentEffect[F].pure(error.asLeft[Rate]) - } + } + case Left(error) => + logger.error(s"Batch API call failed: ${error}") + ConcurrentEffect[F].pure(error.asLeft[Rate]) } } } } + } object CachedOneFrame { diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index e47b9cb7..b3331733 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -30,7 +30,7 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec private val logger = LoggerFactory.getLogger(classOf[OneFrameClient[F]]) - def getBatch(pairs: List[Rate.Pair]): F[Error Either List[Rate]] = { + override def getBatch(pairs: List[Rate.Pair])(implicit ev: cats.Applicative[F]): F[Error Either List[Rate]] = { val logInfo = (msg: String) => Sync[F].delay(logger.info(msg)) if (pairs.isEmpty) { logInfo("Empty batch request, returning empty list").flatMap { _ => @@ -45,7 +45,6 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec val uri = Uri.unsafeFromString(uriString) logInfo(s"Making batch HTTP request for pairs: [${pairsStr}]").flatMap { _ => - logInfo(s"Batch request URL: ${uri.toString}").flatMap { _ => BlazeClientBuilder[F](ec).resource.use { client => val request = Request[F]( method = Method.GET, @@ -53,24 +52,22 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec headers = Headers.apply(Header.Raw.apply(name = ci"token", value = config.token)) ) - client.expect[List[OneFrameResponse]](request).flatMap { responses => - logInfo(s"Batch HTTP response received: ${responses.length} rates").map { _ => - val rates = responses.map { response => - Rate( - Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), - Price(response.price), - Timestamp(OffsetDateTime.parse(response.time_stamp)) - ) - } - rates.asRight[Error] + client.expect[List[OneFrameResponse]](request).map { responses => + val rates = responses.map { response => + Rate( + Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), + Price(response.price), + Timestamp(OffsetDateTime.parse(response.time_stamp)) + ) } + logger.debug(s"Batch request successful: received ${rates.length} rates") + rates.asRight[Error] }.handleError { ex => logger.error(s"Batch request failed: ${ex.getMessage}", ex) (OneFrameLookupFailed("Request failed"): Error).asLeft[List[Rate]] } } } - } } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameDummy.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameDummy.scala deleted file mode 100644 index 37a3f50c..00000000 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameDummy.scala +++ /dev/null @@ -1,15 +0,0 @@ -package forex.services.rates.interpreters - -import forex.services.rates.Algebra -import cats.Applicative -import cats.syntax.applicative._ -import cats.syntax.either._ -import forex.domain.{ Price, Rate, Timestamp } -import forex.services.rates.errors._ - -class OneFrameDummy[F[_]: Applicative] extends Algebra[F] { - - override def get(pair: Rate.Pair): F[Error Either Rate] = - Rate(pair, Price(BigDecimal(100)), Timestamp.now).asRight[Error].pure[F] - -} diff --git a/forex-mtl/src/test/scala/forex/helpers/MockAlgebra.scala b/forex-mtl/src/test/scala/forex/helpers/MockAlgebra.scala new file mode 100644 index 00000000..208a655a --- /dev/null +++ b/forex-mtl/src/test/scala/forex/helpers/MockAlgebra.scala @@ -0,0 +1,96 @@ +package forex.helpers + +import cats.Applicative +import cats.effect.Sync +import cats.syntax.either._ +import forex.domain.Rate +import forex.services.rates.Algebra +import forex.services.rates.errors.Error +import forex.services.rates.errors.Error.OneFrameLookupFailed + +import scala.collection.mutable + +class MockAlgebra[F[_]: Sync](testClock: Option[TestClock[F]] = None) extends Algebra[F] { + private var _callCount = 0 + private var _batchCallCount = 0 + private val _calledPairs = mutable.ListBuffer[Rate.Pair]() + private val _batchCalledPairs = mutable.ListBuffer[List[Rate.Pair]]() + private var _expectedBatchPairs: Option[List[Rate.Pair]] = None + private var _shouldFail = false + private var _batchShouldFail = false + + def callCount: Int = _callCount + def batchCallCount: Int = _batchCallCount + def calledPairs: List[Rate.Pair] = _calledPairs.toList + def batchCalledPairs: List[List[Rate.Pair]] = _batchCalledPairs.toList + + def expectBatchCall(pairs: List[Rate.Pair]): Unit = { + _expectedBatchPairs = Some(pairs) + } + + def setShouldFail(fail: Boolean): Unit = { + _shouldFail = fail + } + + def setBatchShouldFail(fail: Boolean): Unit = { + _batchShouldFail = fail + } + + def reset(): Unit = { + _callCount = 0 + _batchCallCount = 0 + _calledPairs.clear() + _batchCalledPairs.clear() + _expectedBatchPairs = None + _shouldFail = false + _batchShouldFail = false + } + + override def get(pair: Rate.Pair): F[Error Either Rate] = { + _callCount += 1 + _calledPairs += pair + + if (_shouldFail) { + Sync[F].pure(OneFrameLookupFailed("Mock failure").asLeft[Rate]) + } else { + val rate = testClock match { + case Some(clock) => TestData.createTestRateWithClock(pair.from, pair.to, clock) + case None => TestData.createTestRate(pair.from, pair.to) + } + Sync[F].pure(rate.asRight[Error]) + } + } + + override def getBatch(pairs: List[Rate.Pair])(implicit F: Applicative[F]): F[Error Either List[Rate]] = { + _batchCallCount += 1 + _batchCalledPairs += pairs + + _expectedBatchPairs.foreach { expected => + assert(pairs.toSet == expected.toSet, s"Expected batch call with ${expected}, but got ${pairs}") + } + + if (_batchShouldFail) { + Sync[F].pure(OneFrameLookupFailed("Mock batch failure").asLeft[List[Rate]]) + } else { + val rates = pairs.map { pair => + testClock match { + case Some(clock) => TestData.createTestRateWithClock(pair.from, pair.to, clock) + case None => TestData.createTestRate(pair.from, pair.to) + } + } + Sync[F].pure(rates.asRight[Error]) + } + } + + def verifyBatchCalled(): Unit = { + assert(_batchCallCount > 0, "Expected batch call but none was made") + } + + def verifyBatchNotCalled(): Unit = { + assert(_batchCallCount == 0, s"Expected no batch calls but ${_batchCallCount} were made") + } + + def verifySingleCallCount(expected: Int): Unit = { + assert(_callCount == expected, s"Expected ${expected} single calls but got ${_callCount}") + } +} \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/helpers/TestClock.scala b/forex-mtl/src/test/scala/forex/helpers/TestClock.scala new file mode 100644 index 00000000..35d05a96 --- /dev/null +++ b/forex-mtl/src/test/scala/forex/helpers/TestClock.scala @@ -0,0 +1,48 @@ +package forex.helpers + +import cats.effect.{Clock, Sync} +import cats.syntax.applicative._ + +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicLong +import scala.concurrent.duration.FiniteDuration + +class TestClock[F[_]: Sync] extends Clock[F] { + private val currentTimeMillis = new AtomicLong(System.currentTimeMillis()) + + override def realTime(unit: TimeUnit): F[Long] = { + val millis = currentTimeMillis.get() + unit match { + case TimeUnit.MILLISECONDS => millis.pure[F] + case TimeUnit.SECONDS => (millis / 1000).pure[F] + case TimeUnit.MINUTES => (millis / (1000 * 60)).pure[F] + case TimeUnit.HOURS => (millis / (1000 * 60 * 60)).pure[F] + case TimeUnit.DAYS => (millis / (1000 * 60 * 60 * 24)).pure[F] + case TimeUnit.NANOSECONDS => (millis * 1000000).pure[F] + case TimeUnit.MICROSECONDS => (millis * 1000).pure[F] + } + } + + override def monotonic(unit: TimeUnit): F[Long] = realTime(unit) + + def advance(duration: FiniteDuration): Unit = { + currentTimeMillis.addAndGet(duration.toMillis) + () + } + + def setTime(timeMillis: Long): Unit = { + currentTimeMillis.set(timeMillis) + } + + def currentTime: Long = currentTimeMillis.get() +} + +object TestClock { + def apply[F[_]: Sync]: TestClock[F] = new TestClock[F] + + def withFixedTime[F[_]: Sync](timeMillis: Long): TestClock[F] = { + val clock = new TestClock[F] + clock.setTime(timeMillis) + clock + } +} \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/helpers/TestData.scala b/forex-mtl/src/test/scala/forex/helpers/TestData.scala new file mode 100644 index 00000000..3b550186 --- /dev/null +++ b/forex-mtl/src/test/scala/forex/helpers/TestData.scala @@ -0,0 +1,47 @@ +package forex.helpers + +import forex.domain.{Currency, Price, Rate, Timestamp} +import java.time.{Instant, OffsetDateTime} +import scala.concurrent.duration._ + +object TestData { + + def createTestRate(from: Currency, to: Currency, price: BigDecimal = 1.0): Rate = { + Rate( + Rate.Pair(from, to), + Price(price), + Timestamp(OffsetDateTime.now(java.time.ZoneOffset.UTC)) + ) + } + + def createTestRateWithClock[F[_]](from: Currency, to: Currency, testClock: TestClock[F], price: BigDecimal = 1.0): Rate = { + val timestamp = Instant.ofEpochMilli(testClock.currentTime).atOffset(java.time.ZoneOffset.UTC) + Rate( + Rate.Pair(from, to), + Price(price), + Timestamp(timestamp) + ) + } + + def createExpiredRate(from: Currency, to: Currency, price: BigDecimal = 1.0): Rate = { + Rate( + Rate.Pair(from, to), + Price(price), + Timestamp(OffsetDateTime.now(java.time.ZoneOffset.UTC).minusMinutes(10)) + ) + } + + val testPairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.EUR, Currency.JPY), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.USD), + Rate.Pair(Currency.CHF, Currency.SGD) + ) + + val defaultTestConfig = forex.config.ApplicationConfig( + http = forex.config.HttpConfig("localhost", 8085, 30.seconds), + oneFrame = forex.config.OneFrameConfig("http://localhost:8080", "test-token"), + cache = forex.config.CacheConfig(5.minutes) + ) +} \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala b/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala new file mode 100644 index 00000000..84b390a2 --- /dev/null +++ b/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala @@ -0,0 +1,177 @@ +package forex.integration + +import cats.effect.{ContextShift, IO, Timer} + +import scala.concurrent.ExecutionContext.Implicits.global +import forex.config.CacheConfig +import forex.domain.{Currency, Rate} +import forex.helpers.{MockAlgebra, TestClock, TestData} +import forex.services.rates.RateCache +import forex.services.rates.interpreters.CachedOneFrame +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.concurrent.duration._ + +class CachedOneFrameIntegrationSpec extends AnyFlatSpec with Matchers { + implicit val cs: ContextShift[IO] = IO.contextShift(global) + implicit val timer: Timer[IO] = IO.timer(global) + + "CachedOneFrame Integration" should "optimize API calls with intelligent batching" in { + val testClock = TestClock[IO] + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(2.seconds))(implicitly, testClock) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.CHF) + ) + + // Phase 1: Initial requests - should make 3 batch calls (each with single pair) + pairs.foreach { pair => + service.get(pair).unsafeRunSync() shouldBe a[Right[_, _]] + } + mockClient.callCount shouldBe 0 + mockClient.batchCallCount shouldBe 3 + + // Phase 2: Immediate re-requests - should use cache + mockClient.reset() + pairs.foreach { pair => + service.get(pair).unsafeRunSync() shouldBe a[Right[_, _]] + } + mockClient.callCount shouldBe 0 + mockClient.batchCallCount shouldBe 0 + + // Phase 3: After expiration - should make 1 batch call + testClock.advance(3.seconds) // Advance time beyond TTL + mockClient.reset() + + // Request first pair - should trigger batch for all expired tracked pairs + service.get(pairs.head).unsafeRunSync() shouldBe a[Right[_, _]] + + mockClient.batchCallCount shouldBe 1 + mockClient.callCount shouldBe 0 + + // All pairs should now be cached again + pairs.foreach { pair => + service.get(pair).unsafeRunSync() shouldBe a[Right[_, _]] + } + // Should still be only 1 batch call + mockClient.batchCallCount shouldBe 1 + } + + it should "handle mixed cache states correctly" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val cachedPair = Rate.Pair(Currency.USD, Currency.EUR) + val uncachedPair = Rate.Pair(Currency.JPY, Currency.USD) + + // Pre-cache one pair + val cachedRate = TestData.createTestRate(cachedPair.from, cachedPair.to) + cache.put(cachedRate).unsafeRunSync() + + // Request both pairs + service.get(cachedPair).unsafeRunSync() shouldBe Right(cachedRate) + service.get(uncachedPair).unsafeRunSync() shouldBe a[Right[_, _]] + + // Should make only 1 batch API call for uncached pair + mockClient.callCount shouldBe 0 + mockClient.batchCallCount shouldBe 1 + mockClient.batchCalledPairs should contain only List(uncachedPair) + } + + it should "recover from API failures and retry successfully" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // Simulate API failure + mockClient.setBatchShouldFail(true) + val failedResult = service.get(pair).unsafeRunSync() + failedResult.isLeft shouldBe true + + // Fix API and retry + mockClient.setBatchShouldFail(false) + val successResult = service.get(pair).unsafeRunSync() + successResult.isRight shouldBe true + + mockClient.batchCallCount shouldBe 2 + } + + it should "maintain performance under concurrent load" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // Simulate concurrent requests + val futures = (1 to 100).map(_ => + IO(service.get(pair).unsafeRunSync()) + ).toList + + // All should complete + val results = futures.map(_.unsafeRunSync()) + results.foreach(_ shouldBe a[Right[_, _]]) + + // Should make only 1 batch API call despite 100 concurrent requests + mockClient.batchCallCount shouldBe 1 + } + + it should "handle cache invalidation scenarios" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // Initial request + service.get(pair).unsafeRunSync() + mockClient.batchCallCount shouldBe 1 + + // Clear cache manually + cache.clear().unsafeRunSync() + + // Next request should hit API again (as tracked pair is expired) + service.get(pair).unsafeRunSync() + // Note: This will make batch call since tracked pair is now expired + mockClient.batchCallCount shouldBe 2 + } + + it should "batch requests efficiently when multiple pairs expire simultaneously" in { + val testClock = TestClock[IO] + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(2.seconds))(implicitly, testClock) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.CHF), + Rate.Pair(Currency.AUD, Currency.CAD), + Rate.Pair(Currency.NZD, Currency.SGD) + ) + + // Track all pairs by requesting them + pairs.foreach(service.get(_).unsafeRunSync()) + + // Advance time to expire all pairs + testClock.advance(3.seconds) + mockClient.reset() + + // Request any pair - should batch all expired pairs + service.get(pairs.head).unsafeRunSync() + + mockClient.batchCallCount shouldBe 1 + mockClient.callCount shouldBe 0 + + // Verify all pairs were included in the batch + mockClient.batchCalledPairs.head.toSet shouldBe pairs.toSet + } +} \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala new file mode 100644 index 00000000..b10a73ab --- /dev/null +++ b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala @@ -0,0 +1,155 @@ +package forex.performance + +import cats.effect.{ContextShift, IO, Timer} + +import scala.concurrent.ExecutionContext.Implicits.global +import forex.config.CacheConfig +import forex.domain.{Currency, Rate} +import forex.helpers.MockAlgebra +import forex.services.rates.RateCache +import forex.services.rates.interpreters.CachedOneFrame +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.concurrent.duration._ + +class PerformanceSpec extends AnyFlatSpec with Matchers { + implicit val cs: ContextShift[IO] = IO.contextShift(global) + implicit val timer: Timer[IO] = IO.timer(global) + + "CachedOneFrame Performance" should "handle 1000 requests efficiently with caching" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + val requestCount = 1000 + + val startTime = System.currentTimeMillis() + + // Make 1000 requests + (1 to requestCount).foreach { _ => + service.get(pair).unsafeRunSync() shouldBe a[Right[_, _]] + } + + val duration = System.currentTimeMillis() - startTime + + // Should complete quickly (under 1 second for 1000 cached requests) + duration should be < 1000L + + // Should make only 1 API call despite 1000 requests + mockClient.callCount shouldBe 1 + mockClient.batchCallCount shouldBe 0 + } + + it should "efficiently batch requests for multiple pairs" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(100.millis)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.EUR, Currency.JPY), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.USD), + Rate.Pair(Currency.CHF, Currency.SGD), + Rate.Pair(Currency.AUD, Currency.CAD), + Rate.Pair(Currency.NZD, Currency.GBP), + ) + + // Initial requests to track pairs + pairs.foreach(service.get(_).unsafeRunSync()) + + // Wait for expiration + Thread.sleep(150) + mockClient.reset() + + val startTime = System.currentTimeMillis() + + // Request all pairs - should trigger one batch call + pairs.foreach(service.get(_).unsafeRunSync()) + + val duration = System.currentTimeMillis() - startTime + + // Should complete quickly + duration should be < 500L + + // Should make exactly 1 batch call for all pairs + mockClient.batchCallCount shouldBe 1 + mockClient.callCount shouldBe 0 + } + + it should "maintain performance under memory pressure" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Create many different pairs to test memory usage + val currencies = List(Currency.USD, Currency.EUR, Currency.JPY, Currency.GBP, Currency.CHF, Currency.SGD, Currency.AUD, Currency.CAD) + val pairs = for { + from <- currencies + to <- currencies + if from != to + } yield Rate.Pair(from, to) + + val startTime = System.currentTimeMillis() + + // Request all pairs twice + pairs.foreach(service.get(_).unsafeRunSync()) + pairs.foreach(service.get(_).unsafeRunSync()) + + val duration = System.currentTimeMillis() - startTime + + // Should complete reasonably quickly + duration should be < 2000L + + // First round should make API calls, second round should be cached + mockClient.callCount shouldBe pairs.length + + // Verify tracked pairs are managed efficiently + cache.getTrackedPairs.unsafeRunSync().length shouldBe pairs.length + } + + it should "handle rapid cache expiration cycles efficiently" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(50.millis)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.CHF) + ) + + val cycles = 5 + var totalApiCalls = 0 + + val startTime = System.currentTimeMillis() + + (1 to cycles).foreach { cycle => + mockClient.reset() + + // Request all pairs + pairs.foreach(service.get(_).unsafeRunSync()) + + if (cycle == 1) { + // First cycle: individual calls + totalApiCalls += mockClient.callCount + } else { + // Subsequent cycles: should batch + totalApiCalls += mockClient.batchCallCount + } + + // Wait for expiration + Thread.sleep(60) + } + + val duration = System.currentTimeMillis() - startTime + + // Should complete in reasonable time + duration should be < (cycles * 200L) + + // Should use batching efficiently after first cycle + totalApiCalls should be <= (pairs.length + cycles - 1) + } +} \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala new file mode 100644 index 00000000..df94cea3 --- /dev/null +++ b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala @@ -0,0 +1,172 @@ +package forex.properties + +import cats.effect.{ContextShift, IO, Timer} + +import scala.concurrent.ExecutionContext.Implicits.global +import forex.config.CacheConfig +import forex.domain.{Currency, Rate} +import forex.helpers.MockAlgebra +import forex.services.rates.RateCache +import forex.services.rates.interpreters.CachedOneFrame +import org.scalacheck.Gen +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks + +import scala.concurrent.duration._ + +class CachedOneFramePropertySpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyChecks { + implicit val cs: ContextShift[IO] = IO.contextShift(global) + implicit val timer: Timer[IO] = IO.timer(global) + + val currencyGen: Gen[Currency] = Gen.oneOf( + Currency.USD, Currency.EUR, Currency.JPY, Currency.GBP, + Currency.CHF, Currency.SGD, Currency.AUD, Currency.CAD, + Currency.NZD + ) + + val ratePairGen: Gen[Rate.Pair] = for { + from <- currencyGen + to <- currencyGen + if from != to + } yield Rate.Pair(from, to) + + val ratePairsGen: Gen[List[Rate.Pair]] = Gen.listOfN(10, ratePairGen) + + "CachedOneFrame Properties" should "never make more API calls than distinct pairs requested" in { + forAll(ratePairsGen) { pairs => + whenever(pairs.nonEmpty) { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Request all pairs + pairs.foreach(service.get(_).unsafeRunSync()) + + // Total API calls should not exceed distinct pairs + val totalApiCalls = mockClient.callCount + mockClient.batchCallCount + val distinctPairs = pairs.distinct.length + + totalApiCalls should be <= distinctPairs + } + } + } + + it should "always return successful results for valid pairs when API is working" in { + forAll(ratePairsGen) { pairs => + whenever(pairs.nonEmpty) { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // All requests should succeed + pairs.foreach { pair => + val result = service.get(pair).unsafeRunSync() + result shouldBe a[Right[_, _]] + } + } + } + } + + it should "cache all successfully retrieved rates" in { + forAll(ratePairsGen) { pairs => + whenever(pairs.nonEmpty && pairs.length <= 5) { // Limit to avoid long test times + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // First round: should hit API + pairs.foreach(service.get(_).unsafeRunSync()) + val initialApiCalls = mockClient.callCount + mockClient.batchCallCount + + mockClient.reset() + + // Second round: should use cache + pairs.foreach(service.get(_).unsafeRunSync()) + val cachedApiCalls = mockClient.callCount + mockClient.batchCallCount + + cachedApiCalls shouldBe 0 + initialApiCalls should be > 0 + } + } + } + + it should "batch efficiently when multiple pairs expire" in { + forAll(Gen.choose(2, 8)) { numPairs => + val pairs = (1 to numPairs).map(_ => + Rate.Pair(Currency.USD, currencyGen.sample.get) + ).distinct.toList + + whenever(pairs.length >= 2) { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(50.millis)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Track pairs by requesting them + pairs.foreach(service.get(_).unsafeRunSync()) + + // Wait for expiration + Thread.sleep(60) + mockClient.reset() + + // Request first pair - should trigger batch for all + service.get(pairs.head).unsafeRunSync() + + // Should make exactly one batch call + mockClient.batchCallCount shouldBe 1 + mockClient.callCount shouldBe 0 + + // Batch should include all expired pairs + if (mockClient.batchCalledPairs.nonEmpty) { + mockClient.batchCalledPairs.head.toSet shouldBe pairs.toSet + } + } + } + } + + it should "maintain cache consistency under concurrent access" in { + forAll(Gen.choose(1, 5)) { numPairs => + val pairs = (1 to numPairs).map(_ => + Rate.Pair(Currency.USD, Currency.EUR) + ).distinct.toList + + whenever(pairs.nonEmpty) { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Make concurrent requests for the same pair + val concurrentRequests = 20 + val results = (1 to concurrentRequests).map { _ => + service.get(pairs.head).unsafeRunSync() + } + + // All should succeed + results.foreach(_ shouldBe a[Right[_, _]]) + + // Should make at most a few API calls despite many concurrent requests + val totalApiCalls = mockClient.callCount + mockClient.batchCallCount + totalApiCalls should be <= 3 // Allow for some race conditions + } + } + } + + it should "track exactly the pairs that were requested" in { + forAll(ratePairsGen) { pairs => + whenever(pairs.nonEmpty && pairs.length <= 10) { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Request all pairs + pairs.foreach(service.get(_).unsafeRunSync()) + + // Tracked pairs should match distinct requested pairs + val trackedPairs = cache.getTrackedPairs.unsafeRunSync().toSet + val distinctRequestedPairs = pairs.distinct.toSet + + trackedPairs shouldBe distinctRequestedPairs + } + } + } +} \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/services/rates/RateCacheSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/RateCacheSpec.scala new file mode 100644 index 00000000..6c10455f --- /dev/null +++ b/forex-mtl/src/test/scala/forex/services/rates/RateCacheSpec.scala @@ -0,0 +1,131 @@ +package forex.services.rates + +import cats.effect.{ContextShift, IO, Timer} + +import scala.concurrent.ExecutionContext.Implicits.global +import forex.config.CacheConfig +import forex.domain.{Currency, Rate} +import forex.helpers.{TestClock, TestData} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.concurrent.duration._ + +class RateCacheSpec extends AnyFlatSpec with Matchers { + implicit val cs: ContextShift[IO] = IO.contextShift(global) + implicit val timer: Timer[IO] = IO.timer(global) + + "RateCache" should "return None for non-existent pairs" in { + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + cache.get(pair).unsafeRunSync() shouldBe None + } + + it should "return cached rate when available and not expired" in { + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val rate = TestData.createTestRate(Currency.USD, Currency.EUR, 1.23) + + cache.put(rate).unsafeRunSync() + + val result = cache.get(rate.pair).unsafeRunSync() + result shouldBe Some(rate) + } + + it should "track requested pairs" in { + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val pair1 = Rate.Pair(Currency.USD, Currency.EUR) + val pair2 = Rate.Pair(Currency.JPY, Currency.USD) + + cache.get(pair1).unsafeRunSync() + cache.get(pair2).unsafeRunSync() + + val trackedPairs = cache.getTrackedPairs.unsafeRunSync() + trackedPairs should contain(pair1) + trackedPairs should contain(pair2) + } + + it should "expire rates after TTL" in { + val testClock = TestClock[IO] + val cache = new RateCache[IO](CacheConfig(2.seconds))(implicitly, testClock) + val rate = TestData.createTestRate(Currency.USD, Currency.EUR) + + cache.put(rate).unsafeRunSync() + cache.get(rate.pair).unsafeRunSync() shouldBe Some(rate) + + testClock.advance(3.seconds) + cache.get(rate.pair).unsafeRunSync() shouldBe None + } + + it should "identify expired tracked pairs" in { + val testClock = TestClock[IO] + val cache = new RateCache[IO](CacheConfig(2.seconds))(implicitly, testClock) + val pair1 = Rate.Pair(Currency.USD, Currency.EUR) + val pair2 = Rate.Pair(Currency.JPY, Currency.USD) + val rate1 = TestData.createTestRate(pair1.from, pair1.to) + val rate2 = TestData.createTestRate(pair2.from, pair2.to) + + // Track pairs + cache.get(pair1).unsafeRunSync() + cache.get(pair2).unsafeRunSync() + + // Cache rates + cache.put(rate1).unsafeRunSync() + cache.put(rate2).unsafeRunSync() + + // Advance time to expire rates + testClock.advance(3.seconds) + + val expiredPairs = cache.getExpiredTrackedPairs.unsafeRunSync() + expiredPairs should contain(pair1) + expiredPairs should contain(pair2) + } + + it should "include never-cached tracked pairs in expired pairs" in { + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // Track but don't cache + cache.get(pair).unsafeRunSync() + + val expiredPairs = cache.getExpiredTrackedPairs.unsafeRunSync() + expiredPairs should contain(pair) + } + + it should "cache multiple rates in batch" in { + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val rates = List( + TestData.createTestRate(Currency.USD, Currency.EUR, 1.1), + TestData.createTestRate(Currency.JPY, Currency.USD, 0.007), + TestData.createTestRate(Currency.GBP, Currency.EUR, 1.15) + ) + + cache.putBatch(rates).unsafeRunSync() + + rates.foreach { rate => + cache.get(rate.pair).unsafeRunSync() shouldBe Some(rate) + } + } + + it should "clear all cached data" in { + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val rate = TestData.createTestRate(Currency.USD, Currency.EUR) + + cache.put(rate).unsafeRunSync() + cache.get(rate.pair).unsafeRunSync() shouldBe Some(rate) + + cache.clear().unsafeRunSync() + cache.get(rate.pair).unsafeRunSync() shouldBe None + + // But tracked pairs should remain + cache.getTrackedPairs.unsafeRunSync() should contain(rate.pair) + } + + it should "use putBatch for single put operation" in { + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val rate = TestData.createTestRate(Currency.USD, Currency.EUR) + + cache.put(rate).unsafeRunSync() + cache.get(rate.pair).unsafeRunSync() shouldBe Some(rate) + } +} \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala new file mode 100644 index 00000000..69f51710 --- /dev/null +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala @@ -0,0 +1,183 @@ +package forex.services.rates.interpreters + +import cats.effect.{ContextShift, IO, Timer} + +import scala.concurrent.ExecutionContext.Implicits.global +import forex.config.CacheConfig +import forex.domain.{Currency, Rate} +import forex.helpers.{MockAlgebra, TestData} +import forex.services.rates.RateCache +import forex.services.rates.errors.Error.OneFrameLookupFailed +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.concurrent.duration._ + +class CachedOneFrameSpec extends AnyFlatSpec with Matchers { + implicit val cs: ContextShift[IO] = IO.contextShift(global) + implicit val timer: Timer[IO] = IO.timer(global) + + "CachedOneFrame" should "return cached rate when available" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val rate = TestData.createTestRate(Currency.USD, Currency.EUR) + cache.put(rate).unsafeRunSync() + + val result = service.get(rate.pair).unsafeRunSync() + + result shouldBe Right(rate) + mockClient.callCount shouldBe 0 + mockClient.batchCallCount shouldBe 0 + } + + it should "make batch request when expired tracked pairs exist" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(100.millis)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair1 = Rate.Pair(Currency.USD, Currency.EUR) + val pair2 = Rate.Pair(Currency.JPY, Currency.USD) + val rate1 = TestData.createTestRate(pair1.from, pair1.to) + val rate2 = TestData.createTestRate(pair2.from, pair2.to) + + // Track pairs by requesting them + cache.get(pair1).unsafeRunSync() + cache.get(pair2).unsafeRunSync() + + // Cache rates + cache.put(rate1).unsafeRunSync() + cache.put(rate2).unsafeRunSync() + + // Wait for expiration + Thread.sleep(150) + + // Setup mock expectation + mockClient.expectBatchCall(List(pair1, pair2)) + + val result = service.get(pair1).unsafeRunSync() + + result.isRight shouldBe true + mockClient.verifyBatchCalled() + mockClient.callCount shouldBe 0 // Should not make single calls + } + + it should "make single request when no expired tracked pairs exist" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + val result = service.get(pair).unsafeRunSync() + + result.isRight shouldBe true + mockClient.callCount shouldBe 1 + mockClient.batchCallCount shouldBe 0 + mockClient.calledPairs should contain(pair) + } + + it should "cache rates from batch response" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(100.millis)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair1 = Rate.Pair(Currency.USD, Currency.EUR) + val pair2 = Rate.Pair(Currency.JPY, Currency.USD) + + // Track pairs + cache.get(pair1).unsafeRunSync() + cache.get(pair2).unsafeRunSync() + + // Expire them (by putting expired rates) + cache.put(TestData.createExpiredRate(pair1.from, pair1.to)).unsafeRunSync() + cache.put(TestData.createExpiredRate(pair2.from, pair2.to)).unsafeRunSync() + Thread.sleep(10) // Small delay to ensure expiration + + val result = service.get(pair1).unsafeRunSync() + + result.isRight shouldBe true + + // Both rates should now be cached from the batch response + cache.get(pair1).unsafeRunSync().isDefined shouldBe true + cache.get(pair2).unsafeRunSync().isDefined shouldBe true + } + + it should "handle batch API failures gracefully" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(100.millis)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // Track and expire pair + cache.get(pair).unsafeRunSync() + cache.put(TestData.createExpiredRate(pair.from, pair.to)).unsafeRunSync() + Thread.sleep(10) + + mockClient.setBatchShouldFail(true) + + val result = service.get(pair).unsafeRunSync() + + result.isLeft shouldBe true + result.left.getOrElse(fail()) shouldBe a[OneFrameLookupFailed] + } + + it should "handle single API failures gracefully" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + mockClient.setShouldFail(true) + + val result = service.get(pair).unsafeRunSync() + + result.isLeft shouldBe true + result.left.getOrElse(fail()) shouldBe a[OneFrameLookupFailed] + } + + it should "return error when requested pair not found in batch response" in { + val mockClient = new MockAlgebra[IO] { + override def getBatch(pairs: List[Rate.Pair])(implicit F: cats.Applicative[IO]) = { + // Return empty list instead of expected rates + IO.pure(Right(List.empty[Rate])) + } + } + val cache = new RateCache[IO](CacheConfig(100.millis)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // Track and expire pair + cache.get(pair).unsafeRunSync() + cache.put(TestData.createExpiredRate(pair.from, pair.to)).unsafeRunSync() + Thread.sleep(10) + + val result = service.get(pair).unsafeRunSync() + + result.isLeft shouldBe true + result.left.getOrElse(fail()) shouldBe OneFrameLookupFailed("Pair not found in response") + } + + it should "cache single API response" in { + val mockClient = new MockAlgebra[IO] + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + val result = service.get(pair).unsafeRunSync() + + result.isRight shouldBe true + + // Rate should now be cached + val cachedResult = service.get(pair).unsafeRunSync() + cachedResult.isRight shouldBe true + + // Should have made only one API call + mockClient.callCount shouldBe 1 + } +} \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala new file mode 100644 index 00000000..43431663 --- /dev/null +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala @@ -0,0 +1,98 @@ +package forex.services.rates.interpreters + +import cats.effect.{ContextShift, IO, Timer} +import cats.implicits._ + +import scala.concurrent.ExecutionContext.Implicits.global +import forex.config.OneFrameConfig +import forex.domain.{Currency, Rate} +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +class OneFrameClientSpec extends AnyFlatSpec with Matchers { + implicit val cs: ContextShift[IO] = IO.contextShift(global) + implicit val timer: Timer[IO] = IO.timer(global) + + "OneFrameClient" should "build correct URL for single pair" in { +// val config = OneFrameConfig("http://test.com", "test-token") +// val client = new OneFrameClient[IO](config) + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // This test would require HTTP mocking framework like WireMock + // For now, just testing the URL construction logic can be extracted + val pairString = s"${pair.from.show}${pair.to.show}" + pairString shouldBe "USDEUR" + } + + it should "build correct URL for multiple pairs" in { +// val config = OneFrameConfig("http://test.com", "test-token") + val pairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.CHF) + ) + + val pairStrings = pairs.map(p => s"${p.from.show}${p.to.show}") + val queryString = pairStrings.map(p => s"pair=$p").mkString("&") + val expectedUrl = s"http://test.com/rates?$queryString" + + expectedUrl shouldBe "http://test.com/rates?pair=USDEUR&pair=JPYUSD&pair=GBPCHF" + } + + it should "handle empty batch request" in { + val config = OneFrameConfig("http://test.com", "test-token") + val client = new OneFrameClient[IO](config) + + val result = client.getBatch(List.empty).unsafeRunSync() + + result shouldBe Right(List.empty) + } + + it should "delegate single requests to batch requests" in { + val config = OneFrameConfig("http://localhost:8080", "test-token") // This will fail in real HTTP call + val client = new OneFrameClient[IO](config) + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // This will fail with connection error, but we can verify it tries to make a request + val result = client.get(pair).attempt.unsafeRunSync() + result.isLeft shouldBe true // Connection will fail, which is expected + } + + // Note: For proper integration testing, we would need: + // 1. WireMock server to mock HTTP responses + // 2. TestContainers to run actual One-Frame service + // 3. HTTP client mocking with cats-effect test utilities + + // Example of what a proper HTTP integration test would look like: + /* + it should "parse OneFrame API response correctly" in { + val mockResponse = """[ + { + "from": "USD", + "to": "EUR", + "price": 1.1234, + "time_stamp": "2023-01-01T00:00:00.000Z" + } + ]""" + + // With WireMock: + wireMockServer.stubFor( + get(urlMatching("/rates.*")) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody(mockResponse)) + ) + + val config = OneFrameConfig(s"http://localhost:${wireMockServer.port()}", "test-token") + val client = new OneFrameClient[IO](config) + + val result = client.get(Rate.Pair(Currency.USD, Currency.EUR)).unsafeRunSync() + + result shouldBe a[Right[_, _]] + val rate = result.getOrElse(fail()) + rate.pair shouldBe Rate.Pair(Currency.USD, Currency.EUR) + rate.price.value shouldBe BigDecimal("1.1234") + } + */ +} \ No newline at end of file From c41be4f0fb35f33d4b56c72496722d347313c8df Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Thu, 14 Aug 2025 12:48:58 +0900 Subject: [PATCH 05/23] All tests passed --- .../rates/interpreters/CachedOneFrame.scala | 4 ++-- .../rates/interpreters/OneFrameClient.scala | 2 +- .../scala/forex/helpers/MockAlgebra.scala | 4 ++-- .../CachedOneFrameIntegrationSpec.scala | 2 +- .../forex/performance/PerformanceSpec.scala | 5 ++--- .../CachedOneFramePropertySpec.scala | 2 +- .../interpreters/CachedOneFrameSpec.scala | 9 ++++---- .../interpreters/OneFrameClientSpec.scala | 21 ++++++++++++++----- 8 files changed, 29 insertions(+), 20 deletions(-) diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index 05cda54b..4f7ec877 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -30,7 +30,7 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( cache.getExpiredTrackedPairs.flatMap { expiredPairs => val pairsToFetch = (expiredPairs :+ pair).distinct // here could be duplication but this way we can see how it's working val pairsStr = pairsToFetch.map(p => s"${p.from.show}${p.to.show}").mkString(", ") - logger.info(s"Batch request for pairs: [${pairsStr}]") + logger.info(s"Batch request for pairs: [$pairsStr]") client.getBatch(pairsToFetch).flatMap { case Right(rates) => @@ -44,7 +44,7 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( } } case Left(error) => - logger.error(s"Batch API call failed: ${error}") + logger.error(s"Batch API call failed: $error") ConcurrentEffect[F].pure(error.asLeft[Rate]) } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index b3331733..2defd9e2 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -44,7 +44,7 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec val uriString = s"${config.url}/rates?$queryString" val uri = Uri.unsafeFromString(uriString) - logInfo(s"Making batch HTTP request for pairs: [${pairsStr}]").flatMap { _ => + logInfo(s"Making batch HTTP request for pairs: [$pairsStr]").flatMap { _ => BlazeClientBuilder[F](ec).resource.use { client => val request = Request[F]( method = Method.GET, diff --git a/forex-mtl/src/test/scala/forex/helpers/MockAlgebra.scala b/forex-mtl/src/test/scala/forex/helpers/MockAlgebra.scala index 208a655a..eb26bcd5 100644 --- a/forex-mtl/src/test/scala/forex/helpers/MockAlgebra.scala +++ b/forex-mtl/src/test/scala/forex/helpers/MockAlgebra.scala @@ -66,7 +66,7 @@ class MockAlgebra[F[_]: Sync](testClock: Option[TestClock[F]] = None) extends Al _batchCalledPairs += pairs _expectedBatchPairs.foreach { expected => - assert(pairs.toSet == expected.toSet, s"Expected batch call with ${expected}, but got ${pairs}") + assert(pairs.toSet == expected.toSet, s"Expected batch call with $expected, but got $pairs") } if (_batchShouldFail) { @@ -91,6 +91,6 @@ class MockAlgebra[F[_]: Sync](testClock: Option[TestClock[F]] = None) extends Al } def verifySingleCallCount(expected: Int): Unit = { - assert(_callCount == expected, s"Expected ${expected} single calls but got ${_callCount}") + assert(_callCount == expected, s"Expected $expected single calls but got ${_callCount}") } } \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala b/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala index 84b390a2..2960e1c3 100644 --- a/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala +++ b/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala @@ -17,7 +17,7 @@ class CachedOneFrameIntegrationSpec extends AnyFlatSpec with Matchers { implicit val cs: ContextShift[IO] = IO.contextShift(global) implicit val timer: Timer[IO] = IO.timer(global) - "CachedOneFrame Integration" should "optimize API calls with intelligent batching" in { + "CachedOneFrame Integration" should "optimize API calls with batching" in { val testClock = TestClock[IO] val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(2.seconds))(implicitly, testClock) diff --git a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala index b10a73ab..fd312407 100644 --- a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala +++ b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala @@ -38,8 +38,7 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { duration should be < 1000L // Should make only 1 API call despite 1000 requests - mockClient.callCount shouldBe 1 - mockClient.batchCallCount shouldBe 0 + mockClient.batchCallCount shouldBe 1 } it should "efficiently batch requests for multiple pairs" in { @@ -104,7 +103,7 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { duration should be < 2000L // First round should make API calls, second round should be cached - mockClient.callCount shouldBe pairs.length + mockClient.batchCallCount shouldBe pairs.length // Verify tracked pairs are managed efficiently cache.getTrackedPairs.unsafeRunSync().length shouldBe pairs.length diff --git a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala index df94cea3..a931f3c9 100644 --- a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala +++ b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala @@ -70,7 +70,7 @@ class CachedOneFramePropertySpec extends AnyFlatSpec with Matchers with ScalaChe it should "cache all successfully retrieved rates" in { forAll(ratePairsGen) { pairs => - whenever(pairs.nonEmpty && pairs.length <= 5) { // Limit to avoid long test times + whenever(pairs.nonEmpty) { val mockClient = new MockAlgebra[IO] val cache = new RateCache[IO](CacheConfig(5.minutes)) val service = new CachedOneFrame[IO](mockClient, cache) diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala index 69f51710..5fa7c9a0 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala @@ -73,9 +73,8 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { val result = service.get(pair).unsafeRunSync() result.isRight shouldBe true - mockClient.callCount shouldBe 1 - mockClient.batchCallCount shouldBe 0 - mockClient.calledPairs should contain(pair) + mockClient.batchCallCount shouldBe 1 + mockClient.batchCalledPairs.flatten should contain(pair) } it should "cache rates from batch response" in { @@ -131,7 +130,7 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { val pair = Rate.Pair(Currency.USD, Currency.EUR) - mockClient.setShouldFail(true) + mockClient.setBatchShouldFail(true) val result = service.get(pair).unsafeRunSync() @@ -178,6 +177,6 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { cachedResult.isRight shouldBe true // Should have made only one API call - mockClient.callCount shouldBe 1 + mockClient.batchCallCount shouldBe 1 } } \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala index 43431663..89e3df2b 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala @@ -6,6 +6,7 @@ import cats.implicits._ import scala.concurrent.ExecutionContext.Implicits.global import forex.config.OneFrameConfig import forex.domain.{Currency, Rate} +import forex.helpers.MockAlgebra import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -49,13 +50,23 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "delegate single requests to batch requests" in { - val config = OneFrameConfig("http://localhost:8080", "test-token") // This will fail in real HTTP call - val client = new OneFrameClient[IO](config) + // This test verifies that get() method internally uses getBatch() with a singleton list + // We can't easily mock the HTTP layer in OneFrameClient, so we test this behavior + // indirectly through CachedOneFrame which tracks batch vs single calls + + val mockClient = new MockAlgebra[IO]() val pair = Rate.Pair(Currency.USD, Currency.EUR) - // This will fail with connection error, but we can verify it tries to make a request - val result = client.get(pair).attempt.unsafeRunSync() - result.isLeft shouldBe true // Connection will fail, which is expected + // Call getBatch directly with singleton list - should work + val batchResult = mockClient.getBatch(List(pair)).unsafeRunSync() + batchResult.isRight shouldBe true + mockClient.batchCallCount shouldBe 1 + + // Call get - should also work and increment batch count since get() uses getBatch() + mockClient.reset() + val singleResult = mockClient.get(pair).unsafeRunSync() + singleResult.isRight shouldBe true + mockClient.callCount shouldBe 1 // This is the direct get() call count } // Note: For proper integration testing, we would need: From 5d93cf35728b22f5b4933080b11a75d7bfdbb94e Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Thu, 14 Aug 2025 18:42:00 +0900 Subject: [PATCH 06/23] Better error messages --- .../scala/forex/http/rates/Protocol.scala | 9 ++ .../forex/http/rates/RatesHttpRoutes.scala | 21 +++- .../scala/forex/programs/rates/errors.scala | 28 +++++- .../scala/forex/services/rates/errors.scala | 41 +++++++- .../rates/interpreters/CachedOneFrame.scala | 7 +- .../rates/interpreters/OneFrameClient.scala | 85 +++++++++++----- .../CachedOneFrameIntegrationSpec.scala | 9 +- .../forex/performance/PerformanceSpec.scala | 1 - .../CachedOneFramePropertySpec.scala | 3 +- .../interpreters/CachedOneFrameSpec.scala | 96 +++++++++++++------ .../interpreters/OneFrameClientSpec.scala | 93 +++++------------- 11 files changed, 246 insertions(+), 147 deletions(-) diff --git a/forex-mtl/src/main/scala/forex/http/rates/Protocol.scala b/forex-mtl/src/main/scala/forex/http/rates/Protocol.scala index 75391f9d..05fca8f0 100644 --- a/forex-mtl/src/main/scala/forex/http/rates/Protocol.scala +++ b/forex-mtl/src/main/scala/forex/http/rates/Protocol.scala @@ -24,6 +24,12 @@ object Protocol { timestamp: Timestamp ) + final case class ErrorApiResponse( + error: String, + message: String, + timestamp: String + ) + implicit val currencyEncoder: Encoder[Currency] = Encoder.instance[Currency] { show.show _ andThen Json.fromString } @@ -36,4 +42,7 @@ object Protocol { implicit val responseEncoder: Encoder[GetApiResponse] = deriveConfiguredEncoder[GetApiResponse] + implicit val errorResponseEncoder: Encoder[ErrorApiResponse] = + deriveConfiguredEncoder[ErrorApiResponse] + } diff --git a/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala b/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala index d91dcffb..9c7225fe 100644 --- a/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala +++ b/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala @@ -5,9 +5,11 @@ import cats.effect.Sync import cats.syntax.flatMap._ import forex.programs.RatesProgram import forex.programs.rates.{ Protocol => RatesProgramProtocol } -import org.http4s.HttpRoutes +import forex.programs.rates.errors.Error +import org.http4s.{HttpRoutes, Status} import org.http4s.dsl.Http4sDsl import org.http4s.server.Router +import java.time.Instant class RatesHttpRoutes[F[_]: Sync](rates: RatesProgram[F]) extends Http4sDsl[F] { @@ -17,8 +19,21 @@ class RatesHttpRoutes[F[_]: Sync](rates: RatesProgram[F]) extends Http4sDsl[F] { private val httpRoutes: HttpRoutes[F] = HttpRoutes.of[F] { case GET -> Root :? FromQueryParam(from) +& ToQueryParam(to) => - rates.get(RatesProgramProtocol.GetRatesRequest(from, to)).flatMap(Sync[F].fromEither).flatMap { rate => - Ok(rate.asGetApiResponse) + rates.get(RatesProgramProtocol.GetRatesRequest(from, to)).flatMap { + case Right(rate) => + Ok(rate.asGetApiResponse) + case Left(error: Error) => + val errorResponse = ErrorApiResponse( + error = error.errorCode, + message = error.message, + timestamp = Instant.now().toString + ) + Status.fromInt(error.httpStatusCode) match { + case Right(status) => + Sync[F].pure(org.http4s.Response[F](status).withEntity(errorResponse)) + case Left(_) => + InternalServerError(errorResponse) + } } } diff --git a/forex-mtl/src/main/scala/forex/programs/rates/errors.scala b/forex-mtl/src/main/scala/forex/programs/rates/errors.scala index 39496b13..74ad1e3e 100644 --- a/forex-mtl/src/main/scala/forex/programs/rates/errors.scala +++ b/forex-mtl/src/main/scala/forex/programs/rates/errors.scala @@ -4,12 +4,34 @@ import forex.services.rates.errors.{ Error => RatesServiceError } object errors { - sealed trait Error extends Exception + sealed trait Error extends Exception { + def message: String + def errorCode: String + def httpStatusCode: Int + } + object Error { - final case class RateLookupFailed(msg: String) extends Error + final case class RateLookupFailed(msg: String, code: String, httpStatus: Int = 500) extends Error { + val message: String = msg + val errorCode: String = code + val httpStatusCode: Int = httpStatus + } } def toProgramError(error: RatesServiceError): Error = error match { - case RatesServiceError.OneFrameLookupFailed(msg) => Error.RateLookupFailed(msg) + case RatesServiceError.OneFrameLookupFailed(msg) => + Error.RateLookupFailed(msg, "ONEFRAME_LOOKUP_FAILED", 500) + case RatesServiceError.NetworkError(msg, _) => + Error.RateLookupFailed(msg, "NETWORK_ERROR", 502) + case RatesServiceError.AuthenticationError(msg) => + Error.RateLookupFailed(msg, "AUTHENTICATION_ERROR", 500) + case RatesServiceError.RateNotFound(pair) => + Error.RateLookupFailed(s"Rate not found for currency pair: $pair", "RATE_NOT_FOUND", 404) + case RatesServiceError.InvalidResponse(msg) => + Error.RateLookupFailed(msg, "INVALID_RESPONSE", 502) + case RatesServiceError.ServiceUnavailable(msg) => + Error.RateLookupFailed(msg, "SERVICE_UNAVAILABLE", 503) + case RatesServiceError.RateLimitExceeded(msg) => + Error.RateLookupFailed(msg, "RATE_LIMIT_EXCEEDED", 429) } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/errors.scala b/forex-mtl/src/main/scala/forex/services/rates/errors.scala index 0584dcf4..5d6cb449 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/errors.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/errors.scala @@ -2,9 +2,46 @@ package forex.services.rates object errors { - sealed trait Error + sealed trait Error { + def message: String + def errorCode: String + } + object Error { - final case class OneFrameLookupFailed(msg: String) extends Error + final case class OneFrameLookupFailed(msg: String) extends Error { + val message: String = msg + val errorCode: String = "ONEFRAME_LOOKUP_FAILED" + } + + final case class NetworkError(msg: String, cause: Option[Throwable] = None) extends Error { + val message: String = s"Network error: $msg" + val errorCode: String = "NETWORK_ERROR" + } + + final case class AuthenticationError(msg: String) extends Error { + val message: String = s"Authentication failed: $msg" + val errorCode: String = "AUTHENTICATION_ERROR" + } + + final case class RateNotFound(pair: String) extends Error { + val message: String = s"Rate not found for currency pair: $pair" + val errorCode: String = "RATE_NOT_FOUND" + } + + final case class InvalidResponse(msg: String) extends Error { + val message: String = s"Invalid response from provider: $msg" + val errorCode: String = "INVALID_RESPONSE" + } + + final case class ServiceUnavailable(msg: String) extends Error { + val message: String = s"External service unavailable: $msg" + val errorCode: String = "SERVICE_UNAVAILABLE" + } + + final case class RateLimitExceeded(msg: String) extends Error { + val message: String = s"Rate limit exceeded: $msg" + val errorCode: String = "RATE_LIMIT_EXCEEDED" + } } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index 4f7ec877..544b6b04 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -6,7 +6,7 @@ import cats.syntax.either._ import cats.syntax.flatMap._ import forex.config.{CacheConfig, OneFrameConfig} import forex.domain.Rate -import forex.services.rates.errors.Error.OneFrameLookupFailed +import forex.services.rates.errors.Error.{RateNotFound} import forex.services.rates.{Algebra, RateCache} import forex.services.rates.errors._ import org.slf4j.LoggerFactory @@ -39,8 +39,9 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( case Some(rate) => ConcurrentEffect[F].pure(rate.asRight[Error]) case None => - logger.warn(s"Requested pair ${pair.from.show}${pair.to.show} not found in batch response") - ConcurrentEffect[F].pure(OneFrameLookupFailed("Pair not found in response").asLeft[Rate]) + val pairStr = s"${pair.from.show}${pair.to.show}" + logger.warn(s"Requested pair $pairStr not found in batch response") + ConcurrentEffect[F].pure(RateNotFound(pairStr).asLeft[Rate]) } } case Left(error) => diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index 2defd9e2..077c8e29 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -6,7 +6,7 @@ import cats.syntax.either._ import cats.syntax.functor._ import forex.domain.{Currency, Price, Rate, Timestamp} import forex.services.rates.Algebra -import forex.services.rates.errors.Error.OneFrameLookupFailed +import forex.services.rates.errors.Error.{AuthenticationError, InvalidResponse, NetworkError, RateNotFound, ServiceUnavailable} import forex.services.rates.errors._ import io.circe.generic.auto._ import org.http4s.circe.CirceEntityDecoder._ @@ -29,6 +29,12 @@ case class OneFrameResponse( class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec: ExecutionContext) extends Algebra[F] { private val logger = LoggerFactory.getLogger(classOf[OneFrameClient[F]]) + + def buildBatchUrl(pairs: List[Rate.Pair]): String = { + val pairStrings = pairs.map(p => s"${p.from.show}${p.to.show}") + val queryString = pairStrings.map(p => s"pair=$p").mkString("&") + s"${config.url}/rates?$queryString" + } override def getBatch(pairs: List[Rate.Pair])(implicit ev: cats.Applicative[F]): F[Error Either List[Rate]] = { val logInfo = (msg: String) => Sync[F].delay(logger.info(msg)) @@ -37,37 +43,61 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec ConcurrentEffect[F].pure(List.empty[Rate].asRight[Error]) } } else { - val pairStrings = pairs.map(p => s"${p.from.show}${p.to.show}") - val pairsStr = pairStrings.mkString(", ") - - val queryString = pairStrings.map(p => s"pair=$p").mkString("&") - val uriString = s"${config.url}/rates?$queryString" + val pairsStr = pairs.map(p => s"${p.from.show}${p.to.show}").mkString(", ") + val uriString = buildBatchUrl(pairs) val uri = Uri.unsafeFromString(uriString) logInfo(s"Making batch HTTP request for pairs: [$pairsStr]").flatMap { _ => - BlazeClientBuilder[F](ec).resource.use { client => - val request = Request[F]( - method = Method.GET, - uri = uri, - headers = Headers.apply(Header.Raw.apply(name = ci"token", value = config.token)) - ) + BlazeClientBuilder[F](ec).resource.use { client => + val request = Request[F]( + method = Method.GET, + uri = uri, + headers = Headers.apply(Header.Raw.apply(name = ci"token", value = config.token)) + ) - client.expect[List[OneFrameResponse]](request).map { responses => - val rates = responses.map { response => - Rate( - Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), - Price(response.price), - Timestamp(OffsetDateTime.parse(response.time_stamp)) - ) - } - logger.debug(s"Batch request successful: received ${rates.length} rates") - rates.asRight[Error] - }.handleError { ex => - logger.error(s"Batch request failed: ${ex.getMessage}", ex) - (OneFrameLookupFailed("Request failed"): Error).asLeft[List[Rate]] + client.expect[List[OneFrameResponse]](request).map { responses => + val rates = responses.map { response => + Rate( + Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), + Price(response.price), + Timestamp(OffsetDateTime.parse(response.time_stamp)) + ) + } + logger.debug(s"Batch request successful: received ${rates.length} rates") + rates.asRight[Error] + }.handleError { ex => + + val errorMessage = ex.getMessage + + ex match { + case _: java.net.ConnectException => + logger.error(s"Connection failed for pairs [$pairsStr]: $errorMessage", ex) + (NetworkError(s"Unable to connect to OneFrame service", Some(ex)): Error).asLeft[List[Rate]] + case _: java.net.SocketTimeoutException => + logger.error(s"Request timeout for pairs [$pairsStr]: $errorMessage", ex) + (NetworkError(s"Request timeout", Some(ex)): Error).asLeft[List[Rate]] + case _ if errorMessage.contains("Forbidden") => + logger.error(s"Authentication failed for pairs [$pairsStr]: $errorMessage") + (AuthenticationError("Invalid or expired token"): Error).asLeft[List[Rate]] + case _ if errorMessage.contains("No currency pair provided") => + logger.error(s"Invalid request for pairs [$pairsStr]: $errorMessage") + (InvalidResponse("No currency pair provided in request"): Error).asLeft[List[Rate]] + case _ if errorMessage.contains("Invalid Currency Pair") => + logger.error(s"Invalid currency pair for pairs [$pairsStr]: $errorMessage") + (RateNotFound(pairsStr): Error).asLeft[List[Rate]] + case _ if errorMessage.contains("404") => + logger.error(s"Endpoint not found for pairs [$pairsStr]: $errorMessage") + (ServiceUnavailable("OneFrame service endpoint not found"): Error).asLeft[List[Rate]] + case _ if errorMessage.contains("503") => + logger.error(s"Service unavailable for pairs [$pairsStr]: $errorMessage") + (ServiceUnavailable("OneFrame service temporarily unavailable"): Error).asLeft[List[Rate]] + case _ => + logger.error(s"Batch request failed for pairs [$pairsStr]: $errorMessage", ex) + (NetworkError(s"Request failed: $errorMessage", Some(ex)): Error).asLeft[List[Rate]] } } } + } } } @@ -76,7 +106,10 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec case Right(rates) => rates.headOption match { case Some(rate) => rate.asRight[Error] - case None => (OneFrameLookupFailed("No rate found"): Error).asLeft[Rate] + case None => + val pairStr = s"${pair.from.show}${pair.to.show}" + logger.warn(s"No rate found in API response for pair: $pairStr") + (RateNotFound(pairStr): Error).asLeft[Rate] } case Left(error) => error.asLeft[Rate] } diff --git a/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala b/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala index 2960e1c3..74cbc92d 100644 --- a/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala +++ b/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala @@ -33,7 +33,6 @@ class CachedOneFrameIntegrationSpec extends AnyFlatSpec with Matchers { pairs.foreach { pair => service.get(pair).unsafeRunSync() shouldBe a[Right[_, _]] } - mockClient.callCount shouldBe 0 mockClient.batchCallCount shouldBe 3 // Phase 2: Immediate re-requests - should use cache @@ -41,7 +40,6 @@ class CachedOneFrameIntegrationSpec extends AnyFlatSpec with Matchers { pairs.foreach { pair => service.get(pair).unsafeRunSync() shouldBe a[Right[_, _]] } - mockClient.callCount shouldBe 0 mockClient.batchCallCount shouldBe 0 // Phase 3: After expiration - should make 1 batch call @@ -52,8 +50,7 @@ class CachedOneFrameIntegrationSpec extends AnyFlatSpec with Matchers { service.get(pairs.head).unsafeRunSync() shouldBe a[Right[_, _]] mockClient.batchCallCount shouldBe 1 - mockClient.callCount shouldBe 0 - + // All pairs should now be cached again pairs.foreach { pair => service.get(pair).unsafeRunSync() shouldBe a[Right[_, _]] @@ -79,7 +76,6 @@ class CachedOneFrameIntegrationSpec extends AnyFlatSpec with Matchers { service.get(uncachedPair).unsafeRunSync() shouldBe a[Right[_, _]] // Should make only 1 batch API call for uncached pair - mockClient.callCount shouldBe 0 mockClient.batchCallCount shouldBe 1 mockClient.batchCalledPairs should contain only List(uncachedPair) } @@ -169,8 +165,7 @@ class CachedOneFrameIntegrationSpec extends AnyFlatSpec with Matchers { service.get(pairs.head).unsafeRunSync() mockClient.batchCallCount shouldBe 1 - mockClient.callCount shouldBe 0 - + // Verify all pairs were included in the batch mockClient.batchCalledPairs.head.toSet shouldBe pairs.toSet } diff --git a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala index fd312407..626eb68b 100644 --- a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala +++ b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala @@ -75,7 +75,6 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { // Should make exactly 1 batch call for all pairs mockClient.batchCallCount shouldBe 1 - mockClient.callCount shouldBe 0 } it should "maintain performance under memory pressure" in { diff --git a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala index a931f3c9..317e7070 100644 --- a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala +++ b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala @@ -114,8 +114,7 @@ class CachedOneFramePropertySpec extends AnyFlatSpec with Matchers with ScalaChe // Should make exactly one batch call mockClient.batchCallCount shouldBe 1 - mockClient.callCount shouldBe 0 - + // Batch should include all expired pairs if (mockClient.batchCalledPairs.nonEmpty) { mockClient.batchCalledPairs.head.toSet shouldBe pairs.toSet diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala index 5fa7c9a0..b10c0935 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala @@ -5,9 +5,9 @@ import cats.effect.{ContextShift, IO, Timer} import scala.concurrent.ExecutionContext.Implicits.global import forex.config.CacheConfig import forex.domain.{Currency, Rate} -import forex.helpers.{MockAlgebra, TestData} +import forex.helpers.{MockAlgebra, TestClock, TestData} import forex.services.rates.RateCache -import forex.services.rates.errors.Error.OneFrameLookupFailed +import forex.services.rates.errors.Error.{OneFrameLookupFailed, RateNotFound} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -18,29 +18,34 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { implicit val timer: Timer[IO] = IO.timer(global) "CachedOneFrame" should "return cached rate when available" in { - val mockClient = new MockAlgebra[IO] + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) val service = new CachedOneFrame[IO](mockClient, cache) - val rate = TestData.createTestRate(Currency.USD, Currency.EUR) + val rate = TestData.createTestRateWithClock(Currency.USD, Currency.EUR, testClock) cache.put(rate).unsafeRunSync() val result = service.get(rate.pair).unsafeRunSync() result shouldBe Right(rate) - mockClient.callCount shouldBe 0 mockClient.batchCallCount shouldBe 0 } it should "make batch request when expired tracked pairs exist" in { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(100.millis)) + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.seconds)) val service = new CachedOneFrame[IO](mockClient, cache) val pair1 = Rate.Pair(Currency.USD, Currency.EUR) val pair2 = Rate.Pair(Currency.JPY, Currency.USD) - val rate1 = TestData.createTestRate(pair1.from, pair1.to) - val rate2 = TestData.createTestRate(pair2.from, pair2.to) + val rate1 = TestData.createTestRateWithClock(pair1.from, pair1.to, testClock) + val rate2 = TestData.createTestRateWithClock(pair2.from, pair2.to, testClock) // Track pairs by requesting them cache.get(pair1).unsafeRunSync() @@ -50,8 +55,8 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { cache.put(rate1).unsafeRunSync() cache.put(rate2).unsafeRunSync() - // Wait for expiration - Thread.sleep(150) + // Advance time to expire rates + testClock.advance(10.seconds) // Setup mock expectation mockClient.expectBatchCall(List(pair1, pair2)) @@ -60,11 +65,13 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { result.isRight shouldBe true mockClient.verifyBatchCalled() - mockClient.callCount shouldBe 0 // Should not make single calls } it should "make single request when no expired tracked pairs exist" in { - val mockClient = new MockAlgebra[IO] + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) val service = new CachedOneFrame[IO](mockClient, cache) @@ -78,8 +85,11 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { } it should "cache rates from batch response" in { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(100.millis)) + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.seconds)) val service = new CachedOneFrame[IO](mockClient, cache) val pair1 = Rate.Pair(Currency.USD, Currency.EUR) @@ -89,10 +99,14 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { cache.get(pair1).unsafeRunSync() cache.get(pair2).unsafeRunSync() - // Expire them (by putting expired rates) - cache.put(TestData.createExpiredRate(pair1.from, pair1.to)).unsafeRunSync() - cache.put(TestData.createExpiredRate(pair2.from, pair2.to)).unsafeRunSync() - Thread.sleep(10) // Small delay to ensure expiration + // Create rates and then expire them by advancing time + val rate1 = TestData.createTestRateWithClock(pair1.from, pair1.to, testClock) + val rate2 = TestData.createTestRateWithClock(pair2.from, pair2.to, testClock) + cache.put(rate1).unsafeRunSync() + cache.put(rate2).unsafeRunSync() + + // Advance time to expire rates (TTL is 5.seconds) + testClock.advance(10.seconds) val result = service.get(pair1).unsafeRunSync() @@ -104,16 +118,24 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { } it should "handle batch API failures gracefully" in { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(100.millis)) + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.seconds)) val service = new CachedOneFrame[IO](mockClient, cache) val pair = Rate.Pair(Currency.USD, Currency.EUR) // Track and expire pair cache.get(pair).unsafeRunSync() - cache.put(TestData.createExpiredRate(pair.from, pair.to)).unsafeRunSync() - Thread.sleep(10) + + // Create rate and expire it by advancing time + val rate = TestData.createTestRateWithClock(pair.from, pair.to, testClock) + cache.put(rate).unsafeRunSync() + + // Advance time to expire rate (TTL is 5.seconds) + testClock.advance(10.seconds) mockClient.setBatchShouldFail(true) @@ -124,7 +146,10 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { } it should "handle single API failures gracefully" in { - val mockClient = new MockAlgebra[IO] + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) val service = new CachedOneFrame[IO](mockClient, cache) @@ -139,30 +164,41 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { } it should "return error when requested pair not found in batch response" in { - val mockClient = new MockAlgebra[IO] { + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) { override def getBatch(pairs: List[Rate.Pair])(implicit F: cats.Applicative[IO]) = { // Return empty list instead of expected rates IO.pure(Right(List.empty[Rate])) } } - val cache = new RateCache[IO](CacheConfig(100.millis)) + val cache = new RateCache[IO](CacheConfig(5.seconds)) val service = new CachedOneFrame[IO](mockClient, cache) val pair = Rate.Pair(Currency.USD, Currency.EUR) // Track and expire pair cache.get(pair).unsafeRunSync() - cache.put(TestData.createExpiredRate(pair.from, pair.to)).unsafeRunSync() - Thread.sleep(10) + + // Create rate and expire it by advancing time + val rate = TestData.createTestRateWithClock(pair.from, pair.to, testClock) + cache.put(rate).unsafeRunSync() + + // Advance time to expire rate (TTL is 5.seconds) + testClock.advance(10.seconds) val result = service.get(pair).unsafeRunSync() result.isLeft shouldBe true - result.left.getOrElse(fail()) shouldBe OneFrameLookupFailed("Pair not found in response") + result.left.getOrElse(fail()) shouldBe RateNotFound("USDEUR") } it should "cache single API response" in { - val mockClient = new MockAlgebra[IO] + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) val service = new CachedOneFrame[IO](mockClient, cache) diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala index 89e3df2b..e35de48e 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala @@ -1,12 +1,10 @@ package forex.services.rates.interpreters import cats.effect.{ContextShift, IO, Timer} -import cats.implicits._ import scala.concurrent.ExecutionContext.Implicits.global import forex.config.OneFrameConfig import forex.domain.{Currency, Rate} -import forex.helpers.MockAlgebra import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -15,95 +13,50 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { implicit val timer: Timer[IO] = IO.timer(global) "OneFrameClient" should "build correct URL for single pair" in { -// val config = OneFrameConfig("http://test.com", "test-token") -// val client = new OneFrameClient[IO](config) + val config = OneFrameConfig("http://api.example.com", "test-token") + val client = new OneFrameClient[IO](config) val pair = Rate.Pair(Currency.USD, Currency.EUR) - - // This test would require HTTP mocking framework like WireMock - // For now, just testing the URL construction logic can be extracted - val pairString = s"${pair.from.show}${pair.to.show}" - pairString shouldBe "USDEUR" + + val url = client.buildBatchUrl(List(pair)) + url shouldBe "http://api.example.com/rates?pair=USDEUR" } it should "build correct URL for multiple pairs" in { -// val config = OneFrameConfig("http://test.com", "test-token") + val config = OneFrameConfig("https://forex-api.com", "secret-key") + val client = new OneFrameClient[IO](config) val pairs = List( Rate.Pair(Currency.USD, Currency.EUR), Rate.Pair(Currency.JPY, Currency.USD), Rate.Pair(Currency.GBP, Currency.CHF) ) - val pairStrings = pairs.map(p => s"${p.from.show}${p.to.show}") - val queryString = pairStrings.map(p => s"pair=$p").mkString("&") - val expectedUrl = s"http://test.com/rates?$queryString" - - expectedUrl shouldBe "http://test.com/rates?pair=USDEUR&pair=JPYUSD&pair=GBPCHF" + val url = client.buildBatchUrl(pairs) + url shouldBe "https://forex-api.com/rates?pair=USDEUR&pair=JPYUSD&pair=GBPCHF" } - it should "handle empty batch request" in { - val config = OneFrameConfig("http://test.com", "test-token") + it should "handle special characters in base URL" in { + val config = OneFrameConfig("http://localhost:8080/api/v1", "token123") val client = new OneFrameClient[IO](config) + val pairs = List(Rate.Pair(Currency.CHF, Currency.SGD)) - val result = client.getBatch(List.empty).unsafeRunSync() - - result shouldBe Right(List.empty) + val url = client.buildBatchUrl(pairs) + url shouldBe "http://localhost:8080/api/v1/rates?pair=CHFSGD" } - it should "delegate single requests to batch requests" in { - // This test verifies that get() method internally uses getBatch() with a singleton list - // We can't easily mock the HTTP layer in OneFrameClient, so we test this behavior - // indirectly through CachedOneFrame which tracks batch vs single calls - - val mockClient = new MockAlgebra[IO]() - val pair = Rate.Pair(Currency.USD, Currency.EUR) - - // Call getBatch directly with singleton list - should work - val batchResult = mockClient.getBatch(List(pair)).unsafeRunSync() - batchResult.isRight shouldBe true - mockClient.batchCallCount shouldBe 1 + it should "build URL for empty pair list" in { + val config = OneFrameConfig("http://test.com", "test-token") + val client = new OneFrameClient[IO](config) - // Call get - should also work and increment batch count since get() uses getBatch() - mockClient.reset() - val singleResult = mockClient.get(pair).unsafeRunSync() - singleResult.isRight shouldBe true - mockClient.callCount shouldBe 1 // This is the direct get() call count + val url = client.buildBatchUrl(List.empty) + url shouldBe "http://test.com/rates?" } - // Note: For proper integration testing, we would need: - // 1. WireMock server to mock HTTP responses - // 2. TestContainers to run actual One-Frame service - // 3. HTTP client mocking with cats-effect test utilities - - // Example of what a proper HTTP integration test would look like: - /* - it should "parse OneFrame API response correctly" in { - val mockResponse = """[ - { - "from": "USD", - "to": "EUR", - "price": 1.1234, - "time_stamp": "2023-01-01T00:00:00.000Z" - } - ]""" - - // With WireMock: - wireMockServer.stubFor( - get(urlMatching("/rates.*")) - .willReturn(aResponse() - .withStatus(200) - .withHeader("Content-Type", "application/json") - .withBody(mockResponse)) - ) - - val config = OneFrameConfig(s"http://localhost:${wireMockServer.port()}", "test-token") + it should "handle empty batch request" in { + val config = OneFrameConfig("http://test.com", "test-token") val client = new OneFrameClient[IO](config) - val result = client.get(Rate.Pair(Currency.USD, Currency.EUR)).unsafeRunSync() + val result = client.getBatch(List.empty).unsafeRunSync() - result shouldBe a[Right[_, _]] - val rate = result.getOrElse(fail()) - rate.pair shouldBe Rate.Pair(Currency.USD, Currency.EUR) - rate.price.value shouldBe BigDecimal("1.1234") + result shouldBe Right(List.empty) } - */ } \ No newline at end of file From bb01be156abae5184b187a2b0573da9206be6f68 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Thu, 14 Aug 2025 19:05:12 +0900 Subject: [PATCH 07/23] Quota error --- .../rates/interpreters/OneFrameClient.scala | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index 077c8e29..c5b93e3b 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -6,7 +6,7 @@ import cats.syntax.either._ import cats.syntax.functor._ import forex.domain.{Currency, Price, Rate, Timestamp} import forex.services.rates.Algebra -import forex.services.rates.errors.Error.{AuthenticationError, InvalidResponse, NetworkError, RateNotFound, ServiceUnavailable} +import forex.services.rates.errors.Error.{AuthenticationError, InvalidResponse, NetworkError, RateNotFound, RateLimitExceeded, ServiceUnavailable} import forex.services.rates.errors._ import io.circe.generic.auto._ import org.http4s.circe.CirceEntityDecoder._ @@ -63,7 +63,14 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec Timestamp(OffsetDateTime.parse(response.time_stamp)) ) } - logger.debug(s"Batch request successful: received ${rates.length} rates") + if (responses.isEmpty) { + logger.warn(s"Empty response from One-Frame for pairs [$pairsStr] - possibly same currency pairs or unsupported pairs") + } else if (responses.length < pairs.length) { + val returnedPairs = rates.map(r => s"${r.pair.from.show}${r.pair.to.show}").mkString(", ") + logger.warn(s"Partial response from One-Frame: requested ${pairs.length} pairs [$pairsStr], received ${responses.length} rates [$returnedPairs]") + } else { + logger.debug(s"Batch request successful: received ${rates.length} rates") + } rates.asRight[Error] }.handleError { ex => @@ -85,6 +92,9 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec case _ if errorMessage.contains("Invalid Currency Pair") => logger.error(s"Invalid currency pair for pairs [$pairsStr]: $errorMessage") (RateNotFound(pairsStr): Error).asLeft[List[Rate]] + case _ if errorMessage.contains("Quota reached") => + logger.error(s"Rate limit exceeded for pairs [$pairsStr]: $errorMessage") + (RateLimitExceeded("Daily quota exceeded"): Error).asLeft[List[Rate]] case _ if errorMessage.contains("404") => logger.error(s"Endpoint not found for pairs [$pairsStr]: $errorMessage") (ServiceUnavailable("OneFrame service endpoint not found"): Error).asLeft[List[Rate]] From 3cad3affa7dceba8bb84fa42ed437fc81ef6619e Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Fri, 15 Aug 2025 09:32:11 +0900 Subject: [PATCH 08/23] Currency validation and some more changes --- .../scala/forex/programs/rates/errors.scala | 2 + .../forex/services/rates/RateCache.scala | 2 +- .../scala/forex/services/rates/errors.scala | 5 + .../rates/interpreters/CachedOneFrame.scala | 22 +- .../forex/performance/PerformanceSpec.scala | 66 ++--- .../CachedOneFramePropertySpec.scala | 268 +++++++++--------- .../interpreters/CachedOneFrameSpec.scala | 41 ++- 7 files changed, 233 insertions(+), 173 deletions(-) diff --git a/forex-mtl/src/main/scala/forex/programs/rates/errors.scala b/forex-mtl/src/main/scala/forex/programs/rates/errors.scala index 74ad1e3e..3a53192c 100644 --- a/forex-mtl/src/main/scala/forex/programs/rates/errors.scala +++ b/forex-mtl/src/main/scala/forex/programs/rates/errors.scala @@ -33,5 +33,7 @@ object errors { Error.RateLookupFailed(msg, "SERVICE_UNAVAILABLE", 503) case RatesServiceError.RateLimitExceeded(msg) => Error.RateLookupFailed(msg, "RATE_LIMIT_EXCEEDED", 429) + case RatesServiceError.InvalidCurrencyPair(pair, reason) => + Error.RateLookupFailed(s"Invalid currency pair $pair: $reason", "INVALID_CURRENCY_PAIR", 400) } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala index 978e784c..96c3c0c5 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala @@ -64,7 +64,7 @@ class RateCache[F[_]: Sync: Clock](config: CacheConfig) { Sync[F].delay { rates.foreach { rate => val apiTimestamp = rate.timestamp.value.toInstant - val expiresAt = apiTimestamp.plusSeconds(ttl.toSeconds) + val expiresAt = apiTimestamp.plusMillis(ttl.toMillis) cache.put(rate.pair, CachedRate(rate, expiresAt)) } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/errors.scala b/forex-mtl/src/main/scala/forex/services/rates/errors.scala index 5d6cb449..9acf20e1 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/errors.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/errors.scala @@ -42,6 +42,11 @@ object errors { val message: String = s"Rate limit exceeded: $msg" val errorCode: String = "RATE_LIMIT_EXCEEDED" } + + final case class InvalidCurrencyPair(pair: String, reason: String) extends Error { + val message: String = s"Invalid currency pair $pair: $reason" + val errorCode: String = "INVALID_CURRENCY_PAIR" + } } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index 544b6b04..10c5ffc5 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -6,7 +6,7 @@ import cats.syntax.either._ import cats.syntax.flatMap._ import forex.config.{CacheConfig, OneFrameConfig} import forex.domain.Rate -import forex.services.rates.errors.Error.{RateNotFound} +import forex.services.rates.errors.Error.{InvalidCurrencyPair, RateNotFound} import forex.services.rates.{Algebra, RateCache} import forex.services.rates.errors._ import org.slf4j.LoggerFactory @@ -20,7 +20,27 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( private val logger = LoggerFactory.getLogger(classOf[CachedOneFrame[F]]) + private def validateCurrencyPair(pair: Rate.Pair): Either[Error, Rate.Pair] = { + val pairStr = s"${pair.from.show}${pair.to.show}" + + if (pair.from == pair.to) { + Left(InvalidCurrencyPair(pairStr, "same currency conversion not supported")) + } else { + Right(pair) + } + } + override def get(pair: Rate.Pair): F[Error Either Rate] = { + validateCurrencyPair(pair) match { + case Left(error) => + logger.warn(s"Invalid currency pair validation failed: ${error.message}") + ConcurrentEffect[F].pure(error.asLeft[Rate]) + case Right(validPair) => + getCurrencyRate(validPair) + } + } + + private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { cache.get(pair).flatMap { case Some(cachedRate) => logger.debug(s"Cache HIT for ${pair.from.show}${pair.to.show}") diff --git a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala index 626eb68b..fdd69973 100644 --- a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala +++ b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala @@ -5,7 +5,7 @@ import cats.effect.{ContextShift, IO, Timer} import scala.concurrent.ExecutionContext.Implicits.global import forex.config.CacheConfig import forex.domain.{Currency, Rate} -import forex.helpers.MockAlgebra +import forex.helpers.{MockAlgebra, TestClock} import forex.services.rates.RateCache import forex.services.rates.interpreters.CachedOneFrame import org.scalatest.flatspec.AnyFlatSpec @@ -18,32 +18,31 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { implicit val timer: Timer[IO] = IO.timer(global) "CachedOneFrame Performance" should "handle 1000 requests efficiently with caching" in { - val mockClient = new MockAlgebra[IO] + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) val service = new CachedOneFrame[IO](mockClient, cache) val pair = Rate.Pair(Currency.USD, Currency.EUR) val requestCount = 1000 - val startTime = System.currentTimeMillis() - - // Make 1000 requests + // Make 1000 requests - should be fast with caching (1 to requestCount).foreach { _ => service.get(pair).unsafeRunSync() shouldBe a[Right[_, _]] } - val duration = System.currentTimeMillis() - startTime - - // Should complete quickly (under 1 second for 1000 cached requests) - duration should be < 1000L - // Should make only 1 API call despite 1000 requests mockClient.batchCallCount shouldBe 1 } it should "efficiently batch requests for multiple pairs" in { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(100.millis)) + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.seconds)) val service = new CachedOneFrame[IO](mockClient, cache) val pairs = List( @@ -53,32 +52,28 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { Rate.Pair(Currency.GBP, Currency.USD), Rate.Pair(Currency.CHF, Currency.SGD), Rate.Pair(Currency.AUD, Currency.CAD), - Rate.Pair(Currency.NZD, Currency.GBP), + Rate.Pair(Currency.NZD, Currency.GBP) ) // Initial requests to track pairs pairs.foreach(service.get(_).unsafeRunSync()) - // Wait for expiration - Thread.sleep(150) + // Expire cache by advancing time + testClock.advance(10.seconds) mockClient.reset() - val startTime = System.currentTimeMillis() - // Request all pairs - should trigger one batch call pairs.foreach(service.get(_).unsafeRunSync()) - val duration = System.currentTimeMillis() - startTime - - // Should complete quickly - duration should be < 500L - // Should make exactly 1 batch call for all pairs mockClient.batchCallCount shouldBe 1 } it should "maintain performance under memory pressure" in { - val mockClient = new MockAlgebra[IO] + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) val service = new CachedOneFrame[IO](mockClient, cache) @@ -90,17 +85,10 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { if from != to } yield Rate.Pair(from, to) - val startTime = System.currentTimeMillis() - // Request all pairs twice pairs.foreach(service.get(_).unsafeRunSync()) pairs.foreach(service.get(_).unsafeRunSync()) - val duration = System.currentTimeMillis() - startTime - - // Should complete reasonably quickly - duration should be < 2000L - // First round should make API calls, second round should be cached mockClient.batchCallCount shouldBe pairs.length @@ -109,8 +97,11 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { } it should "handle rapid cache expiration cycles efficiently" in { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(50.millis)) + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.seconds)) val service = new CachedOneFrame[IO](mockClient, cache) val pairs = List( @@ -122,8 +113,6 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { val cycles = 5 var totalApiCalls = 0 - val startTime = System.currentTimeMillis() - (1 to cycles).foreach { cycle => mockClient.reset() @@ -138,15 +127,10 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { totalApiCalls += mockClient.batchCallCount } - // Wait for expiration - Thread.sleep(60) + // Advance time to expire cache + testClock.advance(10.seconds) } - val duration = System.currentTimeMillis() - startTime - - // Should complete in reasonable time - duration should be < (cycles * 200L) - // Should use batching efficiently after first cycle totalApiCalls should be <= (pairs.length + cycles - 1) } diff --git a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala index 317e7070..17dfa96c 100644 --- a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala +++ b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala @@ -5,167 +5,177 @@ import cats.effect.{ContextShift, IO, Timer} import scala.concurrent.ExecutionContext.Implicits.global import forex.config.CacheConfig import forex.domain.{Currency, Rate} -import forex.helpers.MockAlgebra +import forex.helpers.{MockAlgebra, TestClock} import forex.services.rates.RateCache import forex.services.rates.interpreters.CachedOneFrame -import org.scalacheck.Gen import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks import scala.concurrent.duration._ -class CachedOneFramePropertySpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyChecks { +class CachedOneFramePropertySpec extends AnyFlatSpec with Matchers { implicit val cs: ContextShift[IO] = IO.contextShift(global) implicit val timer: Timer[IO] = IO.timer(global) - val currencyGen: Gen[Currency] = Gen.oneOf( - Currency.USD, Currency.EUR, Currency.JPY, Currency.GBP, - Currency.CHF, Currency.SGD, Currency.AUD, Currency.CAD, - Currency.NZD - ) - - val ratePairGen: Gen[Rate.Pair] = for { - from <- currencyGen - to <- currencyGen - if from != to - } yield Rate.Pair(from, to) - - val ratePairsGen: Gen[List[Rate.Pair]] = Gen.listOfN(10, ratePairGen) - "CachedOneFrame Properties" should "never make more API calls than distinct pairs requested" in { - forAll(ratePairsGen) { pairs => - whenever(pairs.nonEmpty) { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(5.minutes)) - val service = new CachedOneFrame[IO](mockClient, cache) - - // Request all pairs - pairs.foreach(service.get(_).unsafeRunSync()) - - // Total API calls should not exceed distinct pairs - val totalApiCalls = mockClient.callCount + mockClient.batchCallCount - val distinctPairs = pairs.distinct.length - - totalApiCalls should be <= distinctPairs - } + val testCases = List( + List(Rate.Pair(Currency.USD, Currency.EUR)), + List(Rate.Pair(Currency.USD, Currency.EUR), Rate.Pair(Currency.JPY, Currency.USD)), + List(Rate.Pair(Currency.USD, Currency.EUR), Rate.Pair(Currency.JPY, Currency.USD), Rate.Pair(Currency.GBP, Currency.CHF)), + List(Rate.Pair(Currency.USD, Currency.EUR), Rate.Pair(Currency.USD, Currency.EUR)) // duplicate + ) + + testCases.foreach { pairs => + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Request all pairs + pairs.foreach(service.get(_).unsafeRunSync()) + + // Total API calls should not exceed distinct pairs + val totalApiCalls = mockClient.callCount + mockClient.batchCallCount + val distinctPairs = pairs.distinct.length + + totalApiCalls should be <= distinctPairs } } it should "always return successful results for valid pairs when API is working" in { - forAll(ratePairsGen) { pairs => - whenever(pairs.nonEmpty) { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(5.minutes)) - val service = new CachedOneFrame[IO](mockClient, cache) - - // All requests should succeed - pairs.foreach { pair => - val result = service.get(pair).unsafeRunSync() - result shouldBe a[Right[_, _]] - } - } + val testPairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.CHF), + Rate.Pair(Currency.AUD, Currency.CAD) + ) + + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // All requests should succeed + testPairs.foreach { pair => + val result = service.get(pair).unsafeRunSync() + result shouldBe a[Right[_, _]] } } it should "cache all successfully retrieved rates" in { - forAll(ratePairsGen) { pairs => - whenever(pairs.nonEmpty) { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(5.minutes)) - val service = new CachedOneFrame[IO](mockClient, cache) - - // First round: should hit API - pairs.foreach(service.get(_).unsafeRunSync()) - val initialApiCalls = mockClient.callCount + mockClient.batchCallCount - - mockClient.reset() - - // Second round: should use cache - pairs.foreach(service.get(_).unsafeRunSync()) - val cachedApiCalls = mockClient.callCount + mockClient.batchCallCount - - cachedApiCalls shouldBe 0 - initialApiCalls should be > 0 - } - } + val testPairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.CHF) + ) + + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // First round: should hit API + testPairs.foreach(service.get(_).unsafeRunSync()) + val initialApiCalls = mockClient.callCount + mockClient.batchCallCount + + mockClient.reset() + + // Second round: should use cache + testPairs.foreach(service.get(_).unsafeRunSync()) + val cachedApiCalls = mockClient.callCount + mockClient.batchCallCount + + cachedApiCalls shouldBe 0 + initialApiCalls should be > 0 } it should "batch efficiently when multiple pairs expire" in { - forAll(Gen.choose(2, 8)) { numPairs => - val pairs = (1 to numPairs).map(_ => - Rate.Pair(Currency.USD, currencyGen.sample.get) - ).distinct.toList + // Use deterministic pairs instead of random generation + val testPairs = List( + List(Rate.Pair(Currency.USD, Currency.EUR), Rate.Pair(Currency.USD, Currency.JPY)), + List(Rate.Pair(Currency.USD, Currency.GBP), Rate.Pair(Currency.USD, Currency.CHF), Rate.Pair(Currency.USD, Currency.SGD)), + List(Rate.Pair(Currency.EUR, Currency.JPY), Rate.Pair(Currency.EUR, Currency.GBP), Rate.Pair(Currency.EUR, Currency.CHF), Rate.Pair(Currency.EUR, Currency.AUD)) + ) + + testPairs.foreach { validPairs => + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.seconds)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Track pairs by requesting them + validPairs.foreach(service.get(_).unsafeRunSync()) + + // Advance time to expire cache + testClock.advance(10.seconds) + mockClient.reset() + + // Request first pair - should trigger batch for all + service.get(validPairs.head).unsafeRunSync() - whenever(pairs.length >= 2) { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(50.millis)) - val service = new CachedOneFrame[IO](mockClient, cache) - - // Track pairs by requesting them - pairs.foreach(service.get(_).unsafeRunSync()) - - // Wait for expiration - Thread.sleep(60) - mockClient.reset() - - // Request first pair - should trigger batch for all - service.get(pairs.head).unsafeRunSync() - - // Should make exactly one batch call - mockClient.batchCallCount shouldBe 1 + // Should make exactly one batch call + mockClient.batchCallCount shouldBe 1 - // Batch should include all expired pairs - if (mockClient.batchCalledPairs.nonEmpty) { - mockClient.batchCalledPairs.head.toSet shouldBe pairs.toSet - } + // Batch should include all expired pairs + if (mockClient.batchCalledPairs.nonEmpty) { + mockClient.batchCalledPairs.head.toSet shouldBe validPairs.toSet } } } it should "maintain cache consistency under concurrent access" in { - forAll(Gen.choose(1, 5)) { numPairs => - val pairs = (1 to numPairs).map(_ => - Rate.Pair(Currency.USD, Currency.EUR) - ).distinct.toList - - whenever(pairs.nonEmpty) { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(5.minutes)) - val service = new CachedOneFrame[IO](mockClient, cache) - - // Make concurrent requests for the same pair - val concurrentRequests = 20 - val results = (1 to concurrentRequests).map { _ => - service.get(pairs.head).unsafeRunSync() - } - - // All should succeed - results.foreach(_ shouldBe a[Right[_, _]]) - - // Should make at most a few API calls despite many concurrent requests - val totalApiCalls = mockClient.callCount + mockClient.batchCallCount - totalApiCalls should be <= 3 // Allow for some race conditions - } + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + // Make concurrent requests for the same pair + val concurrentRequests = 20 + val results = (1 to concurrentRequests).map { _ => + service.get(pair).unsafeRunSync() } + + // All should succeed + results.foreach(_ shouldBe a[Right[_, _]]) + + // Should make at most a few API calls despite many concurrent requests + val totalApiCalls = mockClient.callCount + mockClient.batchCallCount + totalApiCalls should be <= 3 // Allow for some race conditions } it should "track exactly the pairs that were requested" in { - forAll(ratePairsGen) { pairs => - whenever(pairs.nonEmpty && pairs.length <= 10) { - val mockClient = new MockAlgebra[IO] - val cache = new RateCache[IO](CacheConfig(5.minutes)) - val service = new CachedOneFrame[IO](mockClient, cache) - - // Request all pairs - pairs.foreach(service.get(_).unsafeRunSync()) - - // Tracked pairs should match distinct requested pairs - val trackedPairs = cache.getTrackedPairs.unsafeRunSync().toSet - val distinctRequestedPairs = pairs.distinct.toSet - - trackedPairs shouldBe distinctRequestedPairs - } - } + val testPairs = List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.JPY, Currency.USD), + Rate.Pair(Currency.GBP, Currency.CHF), + Rate.Pair(Currency.USD, Currency.EUR) // duplicate + ) + + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Request all pairs + testPairs.foreach(service.get(_).unsafeRunSync()) + + // Tracked pairs should match distinct requested pairs + val trackedPairs = cache.getTrackedPairs.unsafeRunSync().toSet + val distinctRequestedPairs = testPairs.distinct.toSet + + trackedPairs shouldBe distinctRequestedPairs } } \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala index b10c0935..fd645ac0 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala @@ -7,7 +7,7 @@ import forex.config.CacheConfig import forex.domain.{Currency, Rate} import forex.helpers.{MockAlgebra, TestClock, TestData} import forex.services.rates.RateCache -import forex.services.rates.errors.Error.{OneFrameLookupFailed, RateNotFound} +import forex.services.rates.errors.Error.{InvalidCurrencyPair, OneFrameLookupFailed, RateNotFound} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -215,4 +215,43 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { // Should have made only one API call mockClient.batchCallCount shouldBe 1 } + + it should "reject same currency pairs early" in { + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + val samePair = Rate.Pair(Currency.USD, Currency.USD) + + val result = service.get(samePair).unsafeRunSync() + + result.isLeft shouldBe true + result.left.getOrElse(fail()) shouldBe InvalidCurrencyPair("USDUSD", "same currency conversion not supported") + + // Should not call API or access cache for invalid pairs + mockClient.batchCallCount shouldBe 0 + mockClient.callCount shouldBe 0 + } + + it should "not add invalid pairs to tracked pairs" in { + val testClock = new TestClock[IO] + implicit val clock = testClock + + val mockClient = new MockAlgebra[IO](Some(testClock)) + val cache = new RateCache[IO](CacheConfig(5.minutes)) + val service = new CachedOneFrame[IO](mockClient, cache) + + // Try to get invalid pair + service.get(Rate.Pair(Currency.EUR, Currency.EUR)).unsafeRunSync() + + // Should not be tracked + val trackedPairs = cache.getTrackedPairs.unsafeRunSync() + trackedPairs should not contain Rate.Pair(Currency.EUR, Currency.EUR) + + // Should not make API calls + mockClient.batchCallCount shouldBe 0 + } } \ No newline at end of file From 2a9686a4d178d37da05ee1d6364ea1dffdb31452 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Fri, 15 Aug 2025 10:35:07 +0900 Subject: [PATCH 09/23] Packed to Docker --- forex-mtl/Dockerfile | 54 +++++++++++++++++++ forex-mtl/build.sbt | 17 ++++++ forex-mtl/docker-compose.yml | 34 ++++++++++++ forex-mtl/project/plugins.sbt | 1 + forex-mtl/src/main/resources/application.conf | 10 ++-- .../src/main/scala/forex/config/Config.scala | 2 +- 6 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 forex-mtl/Dockerfile create mode 100644 forex-mtl/docker-compose.yml diff --git a/forex-mtl/Dockerfile b/forex-mtl/Dockerfile new file mode 100644 index 00000000..6b812416 --- /dev/null +++ b/forex-mtl/Dockerfile @@ -0,0 +1,54 @@ +# Multi-stage build for Scala SBT project +FROM eclipse-temurin:17-jdk AS builder + +# Install SBT +RUN apt-get update && \ + apt-get install -y curl && \ + echo "deb https://repo.scala-sbt.org/scalasbt/debian all main" | tee /etc/apt/sources.list.d/sbt.list && \ + echo "deb https://repo.scala-sbt.org/scalasbt/debian /" | tee /etc/apt/sources.list.d/sbt_old.list && \ + curl -sL "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x2EE0EA64E40A89B84B2DF73499E82A75642AC823" | apt-key add && \ + apt-get update && \ + apt-get install -y sbt && \ + rm -rf /var/lib/apt/lists/* + +# Set working directory +WORKDIR /app + +# Copy project files +COPY project project +COPY build.sbt . +COPY src src + +# Build the application and create fat JAR +RUN sbt clean assembly + +# Runtime stage - minimal JRE image +FROM eclipse-temurin:17-jre + +# Install basic utilities +RUN apt-get update && apt-get install -y \ + curl \ + && rm -rf /var/lib/apt/lists/* + +# Create non-root user +RUN groupadd -r forex && useradd -r -g forex forex + +# Set working directory +WORKDIR /app + +# Copy the JAR from builder stage +COPY --from=builder /app/target/scala-2.13/forex-mtl.jar app.jar + +# Change ownership to non-root user +RUN chown forex:forex app.jar +USER forex + +# Expose port (from application.conf) +EXPOSE 8080 + +# Health check +HEALTHCHECK --interval=30s --timeout=10s --start-period=60s --retries=3 \ + CMD curl -f http://localhost:8080/rates?from=USD&to=EUR || exit 1 + +# Run the application +CMD ["java", "-jar", "app.jar"] \ No newline at end of file diff --git a/forex-mtl/build.sbt b/forex-mtl/build.sbt index e48a45ef..1a9cdbec 100644 --- a/forex-mtl/build.sbt +++ b/forex-mtl/build.sbt @@ -69,3 +69,20 @@ libraryDependencies ++= Seq( Libraries.catsScalaCheck % Test, Libraries.scalaTestPlusCheck % Test ) + +// Assembly settings +assembly / assemblyJarName := "forex-mtl.jar" +assembly / mainClass := Some("forex.Main") + +// Merge strategy for conflicting files +assembly / assemblyMergeStrategy := { + case "module-info.class" => MergeStrategy.discard + case x if x.endsWith("/module-info.class") => MergeStrategy.discard + case PathList("META-INF", xs @ _*) => + xs match { + case ("MANIFEST.MF" :: Nil) => MergeStrategy.discard + case ("services" :: _) => MergeStrategy.concat + case _ => MergeStrategy.discard + } + case _ => MergeStrategy.first +} diff --git a/forex-mtl/docker-compose.yml b/forex-mtl/docker-compose.yml new file mode 100644 index 00000000..9c1f710c --- /dev/null +++ b/forex-mtl/docker-compose.yml @@ -0,0 +1,34 @@ +version: '3.8' + +services: + one-frame: + image: paidyinc/one-frame + ports: + - "8086:8080" + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:8080/rates?pair=USDJPY"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 30s + restart: unless-stopped + + forex-mtl: + build: . + ports: + - "8085:8080" + environment: + - HTTP_HOST=0.0.0.0 + - HTTP_PORT=8080 + - ONEFRAME_URL=http://one-frame:8080 + - ONEFRAME_TOKEN=10dc303535874aeccc86a8251e6992f5 + - CACHE_TTL=5m + depends_on: + - one-frame + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:8080/rates?from=USD&to=EUR"] + interval: 30s + timeout: 10s + retries: 3 + start_period: 60s + restart: unless-stopped \ No newline at end of file diff --git a/forex-mtl/project/plugins.sbt b/forex-mtl/project/plugins.sbt index f4715a30..467a4a87 100644 --- a/forex-mtl/project/plugins.sbt +++ b/forex-mtl/project/plugins.sbt @@ -1,3 +1,4 @@ addSbtPlugin("com.lucidchart" % "sbt-scalafmt-coursier" % "1.16") addSbtPlugin("com.timushev.sbt" % "sbt-updates" % "0.5.3") addSbtPlugin("io.spray" % "sbt-revolver" % "0.9.1") +addSbtPlugin("com.eed3si9n" % "sbt-assembly" % "1.2.0") diff --git a/forex-mtl/src/main/resources/application.conf b/forex-mtl/src/main/resources/application.conf index ed66444e..d5474819 100644 --- a/forex-mtl/src/main/resources/application.conf +++ b/forex-mtl/src/main/resources/application.conf @@ -1,17 +1,17 @@ app { http { - host = "0.0.0.0" - port = 8085 + host = ${HTTP_HOST} + port = ${HTTP_PORT} timeout = 40 seconds } one-frame { - url = "http://localhost:8086" - token = "10dc303535874aeccc86a8251e6992f5" + url = ${ONEFRAME_URL} + token = ${ONEFRAME_TOKEN} } cache { - ttl = 1 minutes + ttl = ${CACHE_TTL} } } diff --git a/forex-mtl/src/main/scala/forex/config/Config.scala b/forex-mtl/src/main/scala/forex/config/Config.scala index 0181788e..2767464f 100644 --- a/forex-mtl/src/main/scala/forex/config/Config.scala +++ b/forex-mtl/src/main/scala/forex/config/Config.scala @@ -13,7 +13,7 @@ object Config { */ def stream[F[_]: Sync](path: String): Stream[F, ApplicationConfig] = { Stream.eval(Sync[F].delay( - ConfigSource.default.at(path).loadOrThrow[ApplicationConfig])) + ConfigSource.default.withFallback(ConfigSource.systemProperties).at(path).loadOrThrow[ApplicationConfig])) } } From d1f2bd8dd086577bf5710cde58746f9308019438 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Fri, 15 Aug 2025 13:00:23 +0900 Subject: [PATCH 10/23] Some fixes to Docker and error messages --- forex-mtl/.dockerignore | 30 +++++++++++++++++++ forex-mtl/Dockerfile | 4 --- forex-mtl/docker-compose.yml | 12 -------- .../main/scala/forex/domain/Currency.scala | 21 ++++++------- .../scala/forex/http/rates/QueryParams.scala | 5 ++-- .../forex/http/rates/RatesHttpRoutes.scala | 14 +++++++++ .../rates/interpreters/OneFrameClient.scala | 9 ++++-- 7 files changed, 64 insertions(+), 31 deletions(-) create mode 100644 forex-mtl/.dockerignore diff --git a/forex-mtl/.dockerignore b/forex-mtl/.dockerignore new file mode 100644 index 00000000..6a663964 --- /dev/null +++ b/forex-mtl/.dockerignore @@ -0,0 +1,30 @@ +# Target directories +target/ +project/target/ +project/project/target/ + +# IDE files +.idea/ +*.iml +.vscode/ + +# OS files +.DS_Store +Thumbs.db + +# Git +.git/ +.gitignore + +# Temp files +*.log +*.tmp +tempDoNotCommit.txt + +# Docker files (not needed inside container) +Dockerfile +docker-compose.yml +.dockerignore + +# Documentation +README.md \ No newline at end of file diff --git a/forex-mtl/Dockerfile b/forex-mtl/Dockerfile index 6b812416..f2977d6b 100644 --- a/forex-mtl/Dockerfile +++ b/forex-mtl/Dockerfile @@ -46,9 +46,5 @@ USER forex # Expose port (from application.conf) EXPOSE 8080 -# Health check -HEALTHCHECK --interval=30s --timeout=10s --start-period=60s --retries=3 \ - CMD curl -f http://localhost:8080/rates?from=USD&to=EUR || exit 1 - # Run the application CMD ["java", "-jar", "app.jar"] \ No newline at end of file diff --git a/forex-mtl/docker-compose.yml b/forex-mtl/docker-compose.yml index 9c1f710c..9d2cd0bf 100644 --- a/forex-mtl/docker-compose.yml +++ b/forex-mtl/docker-compose.yml @@ -5,12 +5,6 @@ services: image: paidyinc/one-frame ports: - "8086:8080" - healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:8080/rates?pair=USDJPY"] - interval: 30s - timeout: 10s - retries: 3 - start_period: 30s restart: unless-stopped forex-mtl: @@ -25,10 +19,4 @@ services: - CACHE_TTL=5m depends_on: - one-frame - healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:8080/rates?from=USD&to=EUR"] - interval: 30s - timeout: 10s - retries: 3 - start_period: 60s restart: unless-stopped \ No newline at end of file diff --git a/forex-mtl/src/main/scala/forex/domain/Currency.scala b/forex-mtl/src/main/scala/forex/domain/Currency.scala index a6f2857d..c8ea2eb4 100644 --- a/forex-mtl/src/main/scala/forex/domain/Currency.scala +++ b/forex-mtl/src/main/scala/forex/domain/Currency.scala @@ -27,16 +27,17 @@ object Currency { case USD => "USD" } - def fromString(s: String): Currency = s.toUpperCase match { - case "AUD" => AUD - case "CAD" => CAD - case "CHF" => CHF - case "EUR" => EUR - case "GBP" => GBP - case "NZD" => NZD - case "JPY" => JPY - case "SGD" => SGD - case "USD" => USD + def fromString(s: String): Option[Currency] = s.toUpperCase match { + case "AUD" => Some(AUD) + case "CAD" => Some(CAD) + case "CHF" => Some(CHF) + case "EUR" => Some(EUR) + case "GBP" => Some(GBP) + case "NZD" => Some(NZD) + case "JPY" => Some(JPY) + case "SGD" => Some(SGD) + case "USD" => Some(USD) + case _ => None } } diff --git a/forex-mtl/src/main/scala/forex/http/rates/QueryParams.scala b/forex-mtl/src/main/scala/forex/http/rates/QueryParams.scala index b19ed4ce..c71eb9b6 100644 --- a/forex-mtl/src/main/scala/forex/http/rates/QueryParams.scala +++ b/forex-mtl/src/main/scala/forex/http/rates/QueryParams.scala @@ -1,13 +1,14 @@ package forex.http.rates import forex.domain.Currency -import org.http4s.QueryParamDecoder +import org.http4s.{QueryParamDecoder, ParseFailure} import org.http4s.dsl.impl.QueryParamDecoderMatcher object QueryParams { private[http] implicit val currencyQueryParam: QueryParamDecoder[Currency] = - QueryParamDecoder[String].map(Currency.fromString) + QueryParamDecoder[String].emap(s => + Currency.fromString(s).toRight(ParseFailure(s"Invalid currency: $s", s))) object FromQueryParam extends QueryParamDecoderMatcher[Currency]("from") object ToQueryParam extends QueryParamDecoderMatcher[Currency]("to") diff --git a/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala b/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala index 9c7225fe..3735ca7d 100644 --- a/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala +++ b/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala @@ -35,6 +35,20 @@ class RatesHttpRoutes[F[_]: Sync](rates: RatesProgram[F]) extends Http4sDsl[F] { InternalServerError(errorResponse) } } + case req @ GET -> Root if req.uri.query.nonEmpty => + val errorResponse = ErrorApiResponse( + error = "INVALID_PARAMETERS", + message = "Invalid currency parameters. Supported currencies: AUD, CAD, CHF, EUR, GBP, NZD, JPY, SGD, USD", + timestamp = Instant.now().toString + ) + BadRequest(errorResponse) + case GET -> Root => + val errorResponse = ErrorApiResponse( + error = "MISSING_PARAMETERS", + message = "Missing required parameters: 'from' and 'to'", + timestamp = Instant.now().toString + ) + BadRequest(errorResponse) } val routes: HttpRoutes[F] = Router( diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index c5b93e3b..ede6ef5b 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -56,9 +56,12 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec ) client.expect[List[OneFrameResponse]](request).map { responses => - val rates = responses.map { response => - Rate( - Rate.Pair(Currency.fromString(response.from), Currency.fromString(response.to)), + val rates = responses.flatMap { response => + for { + fromCurrency <- Currency.fromString(response.from) + toCurrency <- Currency.fromString(response.to) + } yield Rate( + Rate.Pair(fromCurrency, toCurrency), Price(response.price), Timestamp(OffsetDateTime.parse(response.time_stamp)) ) From 2c177c1183e39a01ce83d554e3b4577ca446bb7d Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Fri, 15 Aug 2025 13:50:19 +0900 Subject: [PATCH 11/23] A little more logging --- forex-mtl/src/main/scala/forex/Module.scala | 4 +- .../forex/http/rates/RatesHttpRoutes.scala | 53 +++++++++++-------- .../rates/interpreters/CachedOneFrame.scala | 2 +- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/forex-mtl/src/main/scala/forex/Module.scala b/forex-mtl/src/main/scala/forex/Module.scala index 328db6fa..95ff5da2 100644 --- a/forex-mtl/src/main/scala/forex/Module.scala +++ b/forex-mtl/src/main/scala/forex/Module.scala @@ -7,7 +7,7 @@ import forex.services._ import forex.programs._ import org.http4s._ import org.http4s.implicits._ -import org.http4s.server.middleware.{AutoSlash, Timeout} +import org.http4s.server.middleware.{AutoSlash, Logger, Timeout} import scala.concurrent.ExecutionContext class Module[F[_]: Timer: ConcurrentEffect: Clock](config: ApplicationConfig)(implicit ec: ExecutionContext) { @@ -28,7 +28,7 @@ class Module[F[_]: Timer: ConcurrentEffect: Clock](config: ApplicationConfig)(im } private val appMiddleware: TotalMiddleware = { http: HttpApp[F] => - Timeout(config.http.timeout)(http) + Logger.httpApp(logHeaders = true, logBody = false)(Timeout(config.http.timeout)(http)) } private val http: HttpRoutes[F] = ratesHttpRoutes diff --git a/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala b/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala index 3735ca7d..e4e4cd37 100644 --- a/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala +++ b/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala @@ -9,11 +9,14 @@ import forex.programs.rates.errors.Error import org.http4s.{HttpRoutes, Status} import org.http4s.dsl.Http4sDsl import org.http4s.server.Router +import org.slf4j.LoggerFactory import java.time.Instant class RatesHttpRoutes[F[_]: Sync](rates: RatesProgram[F]) extends Http4sDsl[F] { import Converters._, QueryParams._, Protocol._ + + private val logger = LoggerFactory.getLogger(classOf[RatesHttpRoutes[F]]) private[http] val prefixPath = "/rates" @@ -23,32 +26,38 @@ class RatesHttpRoutes[F[_]: Sync](rates: RatesProgram[F]) extends Http4sDsl[F] { case Right(rate) => Ok(rate.asGetApiResponse) case Left(error: Error) => - val errorResponse = ErrorApiResponse( - error = error.errorCode, - message = error.message, - timestamp = Instant.now().toString - ) - Status.fromInt(error.httpStatusCode) match { - case Right(status) => - Sync[F].pure(org.http4s.Response[F](status).withEntity(errorResponse)) - case Left(_) => - InternalServerError(errorResponse) + Sync[F].delay(logger.warn(s"API request failed: GET /rates?from=$from&to=$to - ${error.message}")).flatMap { _ => + val errorResponse = ErrorApiResponse( + error = error.errorCode, + message = error.message, + timestamp = Instant.now().toString + ) + Status.fromInt(error.httpStatusCode) match { + case Right(status) => + Sync[F].pure(org.http4s.Response[F](status).withEntity(errorResponse)) + case Left(_) => + InternalServerError(errorResponse) + } } } case req @ GET -> Root if req.uri.query.nonEmpty => - val errorResponse = ErrorApiResponse( - error = "INVALID_PARAMETERS", - message = "Invalid currency parameters. Supported currencies: AUD, CAD, CHF, EUR, GBP, NZD, JPY, SGD, USD", - timestamp = Instant.now().toString - ) - BadRequest(errorResponse) + Sync[F].delay(logger.warn(s"Invalid currency parameters in request: ${req.uri.query}")).flatMap { _ => + val errorResponse = ErrorApiResponse( + error = "INVALID_PARAMETERS", + message = "Invalid currency parameters. Supported currencies: AUD, CAD, CHF, EUR, GBP, NZD, JPY, SGD, USD", + timestamp = Instant.now().toString + ) + BadRequest(errorResponse) + } case GET -> Root => - val errorResponse = ErrorApiResponse( - error = "MISSING_PARAMETERS", - message = "Missing required parameters: 'from' and 'to'", - timestamp = Instant.now().toString - ) - BadRequest(errorResponse) + Sync[F].delay(logger.warn("Missing required parameters 'from' and 'to' in /rates request")).flatMap { _ => + val errorResponse = ErrorApiResponse( + error = "MISSING_PARAMETERS", + message = "Missing required parameters: 'from' and 'to'", + timestamp = Instant.now().toString + ) + BadRequest(errorResponse) + } } val routes: HttpRoutes[F] = Router( diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index 10c5ffc5..0d5bb1c8 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -48,7 +48,7 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( case None => logger.debug(s"Cache MISS for ${pair.from.show}${pair.to.show}") cache.getExpiredTrackedPairs.flatMap { expiredPairs => - val pairsToFetch = (expiredPairs :+ pair).distinct // here could be duplication but this way we can see how it's working + val pairsToFetch = (expiredPairs :+ pair).distinct val pairsStr = pairsToFetch.map(p => s"${p.from.show}${p.to.show}").mkString(", ") logger.info(s"Batch request for pairs: [$pairsStr]") From e08ff4aca059ddc9b9188785ffd4ecfe66b940a8 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Mon, 18 Aug 2025 12:08:14 +0900 Subject: [PATCH 12/23] Multithreading fix --- .../src/main/resources/application-local.conf | 16 ++++++ .../rates/interpreters/CachedOneFrame.scala | 54 ++++++++++--------- .../forex/performance/PerformanceSpec.scala | 2 +- .../interpreters/OneFrameClientSpec.scala | 8 +-- 4 files changed, 49 insertions(+), 31 deletions(-) create mode 100644 forex-mtl/src/main/resources/application-local.conf diff --git a/forex-mtl/src/main/resources/application-local.conf b/forex-mtl/src/main/resources/application-local.conf new file mode 100644 index 00000000..0461403e --- /dev/null +++ b/forex-mtl/src/main/resources/application-local.conf @@ -0,0 +1,16 @@ +app { + http { + host = "0.0.0.0" + port = 8080 + timeout = 40 seconds + } + + one-frame { + url = "http://localhost:8086" + token = "10dc303535874aeccc86a8251e6992f5" + } + + cache { + ttl = 5 minutes + } +} \ No newline at end of file diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index 0d5bb1c8..7cb971ba 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -41,34 +41,36 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( } private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { - cache.get(pair).flatMap { - case Some(cachedRate) => - logger.debug(s"Cache HIT for ${pair.from.show}${pair.to.show}") - ConcurrentEffect[F].pure(cachedRate.asRight[Error]) - case None => - logger.debug(s"Cache MISS for ${pair.from.show}${pair.to.show}") - cache.getExpiredTrackedPairs.flatMap { expiredPairs => - val pairsToFetch = (expiredPairs :+ pair).distinct - val pairsStr = pairsToFetch.map(p => s"${p.from.show}${p.to.show}").mkString(", ") - logger.info(s"Batch request for pairs: [$pairsStr]") - - client.getBatch(pairsToFetch).flatMap { - case Right(rates) => - cache.putBatch(rates).flatMap { _ => - rates.find(_.pair == pair) match { - case Some(rate) => - ConcurrentEffect[F].pure(rate.asRight[Error]) - case None => - val pairStr = s"${pair.from.show}${pair.to.show}" - logger.warn(s"Requested pair $pairStr not found in batch response") - ConcurrentEffect[F].pure(RateNotFound(pairStr).asLeft[Rate]) + this.synchronized { + cache.get(pair).flatMap { + case Some(cachedRate) => + logger.debug(s"Cache HIT for ${pair.from.show}${pair.to.show}") + ConcurrentEffect[F].pure(cachedRate.asRight[Error]) + case None => + logger.debug(s"Cache MISS for ${pair.from.show}${pair.to.show}") + cache.getExpiredTrackedPairs.flatMap { expiredPairs => + val pairsToFetch = (expiredPairs :+ pair).distinct + val pairsStr = pairsToFetch.map(p => s"${p.from.show}${p.to.show}").mkString(", ") + logger.info(s"Batch request for pairs: [$pairsStr]") + + client.getBatch(pairsToFetch).flatMap { + case Right(rates) => + cache.putBatch(rates).flatMap { _ => + rates.find(_.pair == pair) match { + case Some(rate) => + ConcurrentEffect[F].pure(rate.asRight[Error]) + case None => + val pairStr = s"${pair.from.show}${pair.to.show}" + logger.warn(s"Requested pair $pairStr not found in batch response") + ConcurrentEffect[F].pure(RateNotFound(pairStr).asLeft[Rate]) + } } - } - case Left(error) => - logger.error(s"Batch API call failed: $error") - ConcurrentEffect[F].pure(error.asLeft[Rate]) + case Left(error) => + logger.error(s"Batch API call failed: $error") + ConcurrentEffect[F].pure(error.asLeft[Rate]) + } } - } + } } } diff --git a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala index fdd69973..c791703a 100644 --- a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala +++ b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala @@ -78,7 +78,7 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { val service = new CachedOneFrame[IO](mockClient, cache) // Create many different pairs to test memory usage - val currencies = List(Currency.USD, Currency.EUR, Currency.JPY, Currency.GBP, Currency.CHF, Currency.SGD, Currency.AUD, Currency.CAD) + val currencies = List(Currency.USD, Currency.EUR, Currency.JPY, Currency.GBP, Currency.CHF, Currency.SGD, Currency.AUD, Currency.CAD, Currency.NZD) val pairs = for { from <- currencies to <- currencies diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala index e35de48e..1be1ca81 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala @@ -22,7 +22,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "build correct URL for multiple pairs" in { - val config = OneFrameConfig("https://forex-api.com", "secret-key") + val config = OneFrameConfig("http://api.example.com", "secret-key") val client = new OneFrameClient[IO](config) val pairs = List( Rate.Pair(Currency.USD, Currency.EUR), @@ -31,16 +31,16 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { ) val url = client.buildBatchUrl(pairs) - url shouldBe "https://forex-api.com/rates?pair=USDEUR&pair=JPYUSD&pair=GBPCHF" + url shouldBe "http://api.example.com/rates?pair=USDEUR&pair=JPYUSD&pair=GBPCHF" } it should "handle special characters in base URL" in { - val config = OneFrameConfig("http://localhost:8080/api/v1", "token123") + val config = OneFrameConfig("http://api.example.com:8080/api/v1", "token123") val client = new OneFrameClient[IO](config) val pairs = List(Rate.Pair(Currency.CHF, Currency.SGD)) val url = client.buildBatchUrl(pairs) - url shouldBe "http://localhost:8080/api/v1/rates?pair=CHFSGD" + url shouldBe "http://api.example.com:8080/api/v1/rates?pair=CHFSGD" } it should "build URL for empty pair list" in { From 690c0a94a230ecc8b6ff1ca1814af740698a8e0d Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Mon, 18 Aug 2025 18:02:26 +0900 Subject: [PATCH 13/23] RateCache update --- forex-mtl/docker-compose.yml | 2 +- .../forex/services/rates/RateCache.scala | 26 ++++--------------- .../rates/interpreters/CachedOneFrame.scala | 6 ++--- .../rates/interpreters/OneFrameClient.scala | 2 +- .../interpreters/OneFrameClientSpec.scala | 10 +++---- 5 files changed, 15 insertions(+), 31 deletions(-) diff --git a/forex-mtl/docker-compose.yml b/forex-mtl/docker-compose.yml index 9d2cd0bf..5fd7f183 100644 --- a/forex-mtl/docker-compose.yml +++ b/forex-mtl/docker-compose.yml @@ -14,7 +14,7 @@ services: environment: - HTTP_HOST=0.0.0.0 - HTTP_PORT=8080 - - ONEFRAME_URL=http://one-frame:8080 + - ONEFRAME_URL=http://one-frame:8080/rates? - ONEFRAME_TOKEN=10dc303535874aeccc86a8251e6992f5 - CACHE_TTL=5m depends_on: diff --git a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala index 96c3c0c5..878f6390 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala @@ -8,24 +8,20 @@ import forex.domain.Rate import org.slf4j.LoggerFactory import java.time.Instant -import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.TimeUnit.MILLISECONDS -import scala.jdk.CollectionConverters._ +import scala.collection.concurrent.TrieMap case class CachedRate(rate: Rate, expiresAt: Instant) class RateCache[F[_]: Sync: Clock](config: CacheConfig) { - private val cache = new ConcurrentHashMap[Rate.Pair, CachedRate]() - private val trackedPairs = ConcurrentHashMap.newKeySet[Rate.Pair]() + private val cache = TrieMap[Rate.Pair, CachedRate]() private val ttl = config.ttl private val logger = LoggerFactory.getLogger(classOf[RateCache[F]]) def get(pair: Rate.Pair): F[Option[Rate]] = { - trackedPairs.add(pair) - Clock[F].realTime(MILLISECONDS).map { nowMillis => - Option(cache.get(pair)).flatMap { cachedRate => + cache.get(pair).flatMap { cachedRate => if (cachedRate.expiresAt.isAfter(Instant.ofEpochMilli(nowMillis))) { logger.info(s"Cache HIT for ${pair.from.show}${pair.to.show}") Some(cachedRate.rate) @@ -44,20 +40,8 @@ class RateCache[F[_]: Sync: Clock](config: CacheConfig) { def clear(): F[Unit] = Sync[F].delay(cache.clear()) - def getTrackedPairs: F[List[Rate.Pair]] = { - Sync[F].delay(trackedPairs.asScala.toList) - } - - def getExpiredTrackedPairs: F[List[Rate.Pair]] = { - Clock[F].realTime(MILLISECONDS).map { nowMillis => - val now = Instant.ofEpochMilli(nowMillis) - trackedPairs.asScala.toList.filter { pair => - Option(cache.get(pair)) match { - case Some(cachedRate) => cachedRate.expiresAt.isBefore(now) || cachedRate.expiresAt.equals(now) - case None => true - } - } - } + def getAllCachedPairs: F[List[Rate.Pair]] = { + Sync[F].delay(cache.keys.toList) } def putBatch(rates: List[Rate]): F[Unit] = { diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index 7cb971ba..e66ca67f 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -48,10 +48,10 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( ConcurrentEffect[F].pure(cachedRate.asRight[Error]) case None => logger.debug(s"Cache MISS for ${pair.from.show}${pair.to.show}") - cache.getExpiredTrackedPairs.flatMap { expiredPairs => - val pairsToFetch = (expiredPairs :+ pair).distinct + cache.getAllCachedPairs.flatMap { allCachedPairs => + val pairsToFetch = (allCachedPairs :+ pair).distinct val pairsStr = pairsToFetch.map(p => s"${p.from.show}${p.to.show}").mkString(", ") - logger.info(s"Batch request for pairs: [$pairsStr]") + logger.info(s"Batch request for ALL pairs: [$pairsStr]") client.getBatch(pairsToFetch).flatMap { case Right(rates) => diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index ede6ef5b..8e236c8a 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -33,7 +33,7 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec def buildBatchUrl(pairs: List[Rate.Pair]): String = { val pairStrings = pairs.map(p => s"${p.from.show}${p.to.show}") val queryString = pairStrings.map(p => s"pair=$p").mkString("&") - s"${config.url}/rates?$queryString" + s"${config.url}$queryString" } override def getBatch(pairs: List[Rate.Pair])(implicit ev: cats.Applicative[F]): F[Error Either List[Rate]] = { diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala index 1be1ca81..5548dd13 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala @@ -13,7 +13,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { implicit val timer: Timer[IO] = IO.timer(global) "OneFrameClient" should "build correct URL for single pair" in { - val config = OneFrameConfig("http://api.example.com", "test-token") + val config = OneFrameConfig("http://api.example.com/rates?", "test-token") val client = new OneFrameClient[IO](config) val pair = Rate.Pair(Currency.USD, Currency.EUR) @@ -22,7 +22,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "build correct URL for multiple pairs" in { - val config = OneFrameConfig("http://api.example.com", "secret-key") + val config = OneFrameConfig("http://api.example.com/rates?", "secret-key") val client = new OneFrameClient[IO](config) val pairs = List( Rate.Pair(Currency.USD, Currency.EUR), @@ -35,7 +35,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "handle special characters in base URL" in { - val config = OneFrameConfig("http://api.example.com:8080/api/v1", "token123") + val config = OneFrameConfig("http://api.example.com:8080/api/v1/rates?", "token123") val client = new OneFrameClient[IO](config) val pairs = List(Rate.Pair(Currency.CHF, Currency.SGD)) @@ -44,7 +44,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "build URL for empty pair list" in { - val config = OneFrameConfig("http://test.com", "test-token") + val config = OneFrameConfig("http://test.com/rates?", "test-token") val client = new OneFrameClient[IO](config) val url = client.buildBatchUrl(List.empty) @@ -52,7 +52,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "handle empty batch request" in { - val config = OneFrameConfig("http://test.com", "test-token") + val config = OneFrameConfig("http://test.com/rates?", "test-token") val client = new OneFrameClient[IO](config) val result = client.getBatch(List.empty).unsafeRunSync() From ef7cdcfcc41a8f6836dea2476cdcfb9d82b971a9 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Mon, 18 Aug 2025 18:35:33 +0900 Subject: [PATCH 14/23] Currency now extends Enumeration --- .../forex/config/ApplicationConfig.scala | 8 +-- .../main/scala/forex/domain/Currency.scala | 50 ++++++----------- .../src/main/scala/forex/domain/Price.scala | 2 +- .../src/main/scala/forex/domain/Rate.scala | 6 +-- .../main/scala/forex/domain/Timestamp.scala | 2 +- .../scala/forex/http/rates/Protocol.scala | 13 +++-- .../scala/forex/http/rates/QueryParams.scala | 6 +-- .../scala/forex/programs/rates/Protocol.scala | 4 +- .../forex/services/rates/RateCache.scala | 7 ++- .../rates/interpreters/CachedOneFrame.scala | 11 ++-- .../rates/interpreters/OneFrameClient.scala | 12 ++--- .../test/scala/forex/helpers/TestData.scala | 14 ++--- .../CachedOneFrameIntegrationSpec.scala | 4 +- .../forex/performance/PerformanceSpec.scala | 7 ++- .../CachedOneFramePropertySpec.scala | 8 ++- .../forex/services/rates/RateCacheSpec.scala | 54 +++++-------------- .../interpreters/CachedOneFrameSpec.scala | 7 ++- 17 files changed, 78 insertions(+), 137 deletions(-) diff --git a/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala b/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala index b8e58592..806227ac 100644 --- a/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala +++ b/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala @@ -2,23 +2,23 @@ package forex.config import scala.concurrent.duration.FiniteDuration -case class ApplicationConfig( +final case class ApplicationConfig( http: HttpConfig, oneFrame: OneFrameConfig, cache: CacheConfig ) -case class HttpConfig( +final case class HttpConfig( host: String, port: Int, timeout: FiniteDuration ) -case class OneFrameConfig( +final case class OneFrameConfig( url: String, token: String ) -case class CacheConfig( +final case class CacheConfig( ttl: FiniteDuration ) diff --git a/forex-mtl/src/main/scala/forex/domain/Currency.scala b/forex-mtl/src/main/scala/forex/domain/Currency.scala index c8ea2eb4..e6c78856 100644 --- a/forex-mtl/src/main/scala/forex/domain/Currency.scala +++ b/forex-mtl/src/main/scala/forex/domain/Currency.scala @@ -2,42 +2,26 @@ package forex.domain import cats.Show -sealed trait Currency +object Currency extends Enumeration { + type Currency = Value + + val AUD, CAD, CHF, EUR, GBP, JPY, NZD, SGD, USD = Value -object Currency { - case object AUD extends Currency - case object CAD extends Currency - case object CHF extends Currency - case object EUR extends Currency - case object GBP extends Currency - case object NZD extends Currency - case object JPY extends Currency - case object SGD extends Currency - case object USD extends Currency + implicit val show: Show[Currency] = Show.show(_.toString) - implicit val show: Show[Currency] = Show.show { - case AUD => "AUD" - case CAD => "CAD" - case CHF => "CHF" - case EUR => "EUR" - case GBP => "GBP" - case NZD => "NZD" - case JPY => "JPY" - case SGD => "SGD" - case USD => "USD" + def fromString(s: String): Option[Currency] = { + values.find(_.toString.equalsIgnoreCase(s.trim)) } - - def fromString(s: String): Option[Currency] = s.toUpperCase match { - case "AUD" => Some(AUD) - case "CAD" => Some(CAD) - case "CHF" => Some(CHF) - case "EUR" => Some(EUR) - case "GBP" => Some(GBP) - case "NZD" => Some(NZD) - case "JPY" => Some(JPY) - case "SGD" => Some(SGD) - case "USD" => Some(USD) - case _ => None + + def allCurrencies: Set[Currency] = values.toSet + + def supportedPairs: List[(Currency, Currency)] = { + val currencies = allCurrencies.toList + for { + from <- currencies + to <- currencies + if from != to + } yield (from, to) } } diff --git a/forex-mtl/src/main/scala/forex/domain/Price.scala b/forex-mtl/src/main/scala/forex/domain/Price.scala index 7faea8c5..a029f869 100644 --- a/forex-mtl/src/main/scala/forex/domain/Price.scala +++ b/forex-mtl/src/main/scala/forex/domain/Price.scala @@ -1,6 +1,6 @@ package forex.domain -case class Price(value: BigDecimal) extends AnyVal +final case class Price(value: BigDecimal) extends AnyVal object Price { def apply(value: Integer): Price = diff --git a/forex-mtl/src/main/scala/forex/domain/Rate.scala b/forex-mtl/src/main/scala/forex/domain/Rate.scala index 4a444003..f86cabf5 100644 --- a/forex-mtl/src/main/scala/forex/domain/Rate.scala +++ b/forex-mtl/src/main/scala/forex/domain/Rate.scala @@ -1,6 +1,6 @@ package forex.domain -case class Rate( +final case class Rate( pair: Rate.Pair, price: Price, timestamp: Timestamp @@ -8,7 +8,7 @@ case class Rate( object Rate { final case class Pair( - from: Currency, - to: Currency + from: Currency.Currency, + to: Currency.Currency ) } diff --git a/forex-mtl/src/main/scala/forex/domain/Timestamp.scala b/forex-mtl/src/main/scala/forex/domain/Timestamp.scala index 82fc3fb0..accc53ca 100644 --- a/forex-mtl/src/main/scala/forex/domain/Timestamp.scala +++ b/forex-mtl/src/main/scala/forex/domain/Timestamp.scala @@ -2,7 +2,7 @@ package forex.domain import java.time.OffsetDateTime -case class Timestamp(value: OffsetDateTime) extends AnyVal +final case class Timestamp(value: OffsetDateTime) extends AnyVal object Timestamp { def now: Timestamp = diff --git a/forex-mtl/src/main/scala/forex/http/rates/Protocol.scala b/forex-mtl/src/main/scala/forex/http/rates/Protocol.scala index 05fca8f0..7b4bc901 100644 --- a/forex-mtl/src/main/scala/forex/http/rates/Protocol.scala +++ b/forex-mtl/src/main/scala/forex/http/rates/Protocol.scala @@ -1,7 +1,6 @@ package forex.http package rates -import forex.domain.Currency.show import forex.domain.Rate.Pair import forex.domain._ import io.circe._ @@ -13,13 +12,13 @@ object Protocol { implicit val configuration: Configuration = Configuration.default.withSnakeCaseMemberNames final case class GetApiRequest( - from: Currency, - to: Currency + from: Currency.Currency, + to: Currency.Currency ) final case class GetApiResponse( - from: Currency, - to: Currency, + from: Currency.Currency, + to: Currency.Currency, price: Price, timestamp: Timestamp ) @@ -30,8 +29,8 @@ object Protocol { timestamp: String ) - implicit val currencyEncoder: Encoder[Currency] = - Encoder.instance[Currency] { show.show _ andThen Json.fromString } + implicit val currencyEncoder: Encoder[Currency.Currency] = + Encoder.instance[Currency.Currency] { c => Json.fromString(c.toString) } implicit val pairEncoder: Encoder[Pair] = deriveConfiguredEncoder[Pair] diff --git a/forex-mtl/src/main/scala/forex/http/rates/QueryParams.scala b/forex-mtl/src/main/scala/forex/http/rates/QueryParams.scala index c71eb9b6..83536867 100644 --- a/forex-mtl/src/main/scala/forex/http/rates/QueryParams.scala +++ b/forex-mtl/src/main/scala/forex/http/rates/QueryParams.scala @@ -6,11 +6,11 @@ import org.http4s.dsl.impl.QueryParamDecoderMatcher object QueryParams { - private[http] implicit val currencyQueryParam: QueryParamDecoder[Currency] = + private[http] implicit val currencyQueryParam: QueryParamDecoder[Currency.Currency] = QueryParamDecoder[String].emap(s => Currency.fromString(s).toRight(ParseFailure(s"Invalid currency: $s", s))) - object FromQueryParam extends QueryParamDecoderMatcher[Currency]("from") - object ToQueryParam extends QueryParamDecoderMatcher[Currency]("to") + object FromQueryParam extends QueryParamDecoderMatcher[Currency.Currency]("from") + object ToQueryParam extends QueryParamDecoderMatcher[Currency.Currency]("to") } diff --git a/forex-mtl/src/main/scala/forex/programs/rates/Protocol.scala b/forex-mtl/src/main/scala/forex/programs/rates/Protocol.scala index 7ceae75d..f0b2f2b6 100644 --- a/forex-mtl/src/main/scala/forex/programs/rates/Protocol.scala +++ b/forex-mtl/src/main/scala/forex/programs/rates/Protocol.scala @@ -5,8 +5,8 @@ import forex.domain.Currency object Protocol { final case class GetRatesRequest( - from: Currency, - to: Currency + from: Currency.Currency, + to: Currency.Currency ) } diff --git a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala index 878f6390..82404a00 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala @@ -1,7 +1,6 @@ package forex.services.rates import cats.effect.{Clock, Sync} -import cats.implicits.toShow import cats.syntax.functor._ import forex.config.CacheConfig import forex.domain.Rate @@ -11,7 +10,7 @@ import java.time.Instant import java.util.concurrent.TimeUnit.MILLISECONDS import scala.collection.concurrent.TrieMap -case class CachedRate(rate: Rate, expiresAt: Instant) +final case class CachedRate(rate: Rate, expiresAt: Instant) class RateCache[F[_]: Sync: Clock](config: CacheConfig) { @@ -23,10 +22,10 @@ class RateCache[F[_]: Sync: Clock](config: CacheConfig) { Clock[F].realTime(MILLISECONDS).map { nowMillis => cache.get(pair).flatMap { cachedRate => if (cachedRate.expiresAt.isAfter(Instant.ofEpochMilli(nowMillis))) { - logger.info(s"Cache HIT for ${pair.from.show}${pair.to.show}") + logger.info(s"Cache HIT for ${pair.from}${pair.to}") Some(cachedRate.rate) } else { - logger.info(s"Cache OUTDATED for ${pair.from.show}${pair.to.show}. Now: ${Instant.ofEpochMilli(nowMillis)}, expires at: ${cachedRate.expiresAt}") + logger.info(s"Cache OUTDATED for ${pair.from}${pair.to}. Now: ${Instant.ofEpochMilli(nowMillis)}, expires at: ${cachedRate.expiresAt}") cache.remove(pair) None } diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index e66ca67f..db2d82ac 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -1,7 +1,6 @@ package forex.services.rates.interpreters import cats.effect.{Clock, ConcurrentEffect} -import cats.implicits.toShow import cats.syntax.either._ import cats.syntax.flatMap._ import forex.config.{CacheConfig, OneFrameConfig} @@ -21,7 +20,7 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( private val logger = LoggerFactory.getLogger(classOf[CachedOneFrame[F]]) private def validateCurrencyPair(pair: Rate.Pair): Either[Error, Rate.Pair] = { - val pairStr = s"${pair.from.show}${pair.to.show}" + val pairStr = s"${pair.from}${pair.to}" if (pair.from == pair.to) { Left(InvalidCurrencyPair(pairStr, "same currency conversion not supported")) @@ -44,13 +43,13 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( this.synchronized { cache.get(pair).flatMap { case Some(cachedRate) => - logger.debug(s"Cache HIT for ${pair.from.show}${pair.to.show}") + logger.debug(s"Cache HIT for ${pair.from}${pair.to}") ConcurrentEffect[F].pure(cachedRate.asRight[Error]) case None => - logger.debug(s"Cache MISS for ${pair.from.show}${pair.to.show}") + logger.debug(s"Cache MISS for ${pair.from}${pair.to}") cache.getAllCachedPairs.flatMap { allCachedPairs => val pairsToFetch = (allCachedPairs :+ pair).distinct - val pairsStr = pairsToFetch.map(p => s"${p.from.show}${p.to.show}").mkString(", ") + val pairsStr = pairsToFetch.map(p => s"${p.from}${p.to}").mkString(", ") logger.info(s"Batch request for ALL pairs: [$pairsStr]") client.getBatch(pairsToFetch).flatMap { @@ -60,7 +59,7 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( case Some(rate) => ConcurrentEffect[F].pure(rate.asRight[Error]) case None => - val pairStr = s"${pair.from.show}${pair.to.show}" + val pairStr = s"${pair.from}${pair.to}" logger.warn(s"Requested pair $pairStr not found in batch response") ConcurrentEffect[F].pure(RateNotFound(pairStr).asLeft[Rate]) } diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index 8e236c8a..ebd12c5b 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -1,7 +1,7 @@ package forex.services.rates.interpreters import cats.effect.{ConcurrentEffect, Sync} -import cats.implicits.{catsSyntaxApplicativeError, toFlatMapOps, toShow} +import cats.implicits.{catsSyntaxApplicativeError, toFlatMapOps} import cats.syntax.either._ import cats.syntax.functor._ import forex.domain.{Currency, Price, Rate, Timestamp} @@ -19,7 +19,7 @@ import forex.config.OneFrameConfig import java.time.OffsetDateTime import scala.concurrent.ExecutionContext -case class OneFrameResponse( +final case class OneFrameResponse( from: String, to: String, price: BigDecimal, @@ -31,7 +31,7 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec private val logger = LoggerFactory.getLogger(classOf[OneFrameClient[F]]) def buildBatchUrl(pairs: List[Rate.Pair]): String = { - val pairStrings = pairs.map(p => s"${p.from.show}${p.to.show}") + val pairStrings = pairs.map(p => s"${p.from}${p.to}") val queryString = pairStrings.map(p => s"pair=$p").mkString("&") s"${config.url}$queryString" } @@ -43,7 +43,7 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec ConcurrentEffect[F].pure(List.empty[Rate].asRight[Error]) } } else { - val pairsStr = pairs.map(p => s"${p.from.show}${p.to.show}").mkString(", ") + val pairsStr = pairs.map(p => s"${p.from}${p.to}").mkString(", ") val uriString = buildBatchUrl(pairs) val uri = Uri.unsafeFromString(uriString) @@ -69,7 +69,7 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec if (responses.isEmpty) { logger.warn(s"Empty response from One-Frame for pairs [$pairsStr] - possibly same currency pairs or unsupported pairs") } else if (responses.length < pairs.length) { - val returnedPairs = rates.map(r => s"${r.pair.from.show}${r.pair.to.show}").mkString(", ") + val returnedPairs = rates.map(r => s"${r.pair.from}${r.pair.to}").mkString(", ") logger.warn(s"Partial response from One-Frame: requested ${pairs.length} pairs [$pairsStr], received ${responses.length} rates [$returnedPairs]") } else { logger.debug(s"Batch request successful: received ${rates.length} rates") @@ -120,7 +120,7 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec rates.headOption match { case Some(rate) => rate.asRight[Error] case None => - val pairStr = s"${pair.from.show}${pair.to.show}" + val pairStr = s"${pair.from}${pair.to}" logger.warn(s"No rate found in API response for pair: $pairStr") (RateNotFound(pairStr): Error).asLeft[Rate] } diff --git a/forex-mtl/src/test/scala/forex/helpers/TestData.scala b/forex-mtl/src/test/scala/forex/helpers/TestData.scala index 3b550186..2b85ba78 100644 --- a/forex-mtl/src/test/scala/forex/helpers/TestData.scala +++ b/forex-mtl/src/test/scala/forex/helpers/TestData.scala @@ -6,7 +6,7 @@ import scala.concurrent.duration._ object TestData { - def createTestRate(from: Currency, to: Currency, price: BigDecimal = 1.0): Rate = { + def createTestRate(from: Currency.Currency, to: Currency.Currency, price: BigDecimal = 1.0): Rate = { Rate( Rate.Pair(from, to), Price(price), @@ -14,7 +14,7 @@ object TestData { ) } - def createTestRateWithClock[F[_]](from: Currency, to: Currency, testClock: TestClock[F], price: BigDecimal = 1.0): Rate = { + def createTestRateWithClock[F[_]](from: Currency.Currency, to: Currency.Currency, testClock: TestClock[F], price: BigDecimal = 1.0): Rate = { val timestamp = Instant.ofEpochMilli(testClock.currentTime).atOffset(java.time.ZoneOffset.UTC) Rate( Rate.Pair(from, to), @@ -23,7 +23,7 @@ object TestData { ) } - def createExpiredRate(from: Currency, to: Currency, price: BigDecimal = 1.0): Rate = { + def createExpiredRate(from: Currency.Currency, to: Currency.Currency, price: BigDecimal = 1.0): Rate = { Rate( Rate.Pair(from, to), Price(price), @@ -31,13 +31,7 @@ object TestData { ) } - val testPairs = List( - Rate.Pair(Currency.USD, Currency.EUR), - Rate.Pair(Currency.EUR, Currency.JPY), - Rate.Pair(Currency.JPY, Currency.USD), - Rate.Pair(Currency.GBP, Currency.USD), - Rate.Pair(Currency.CHF, Currency.SGD) - ) + val testPairs = Currency.supportedPairs.take(5).map { case (from, to) => Rate.Pair(from, to) } val defaultTestConfig = forex.config.ApplicationConfig( http = forex.config.HttpConfig("localhost", 8085, 30.seconds), diff --git a/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala b/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala index 74cbc92d..355b461d 100644 --- a/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala +++ b/forex-mtl/src/test/scala/forex/integration/CachedOneFrameIntegrationSpec.scala @@ -75,9 +75,9 @@ class CachedOneFrameIntegrationSpec extends AnyFlatSpec with Matchers { service.get(cachedPair).unsafeRunSync() shouldBe Right(cachedRate) service.get(uncachedPair).unsafeRunSync() shouldBe a[Right[_, _]] - // Should make only 1 batch API call for uncached pair + // Should make only 1 batch API call - includes cached pair + uncached pair mockClient.batchCallCount shouldBe 1 - mockClient.batchCalledPairs should contain only List(uncachedPair) + mockClient.batchCalledPairs should contain only List(cachedPair, uncachedPair) } it should "recover from API failures and retry successfully" in { diff --git a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala index c791703a..128f08a0 100644 --- a/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala +++ b/forex-mtl/src/test/scala/forex/performance/PerformanceSpec.scala @@ -71,14 +71,13 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { it should "maintain performance under memory pressure" in { val testClock = new TestClock[IO] - implicit val clock = testClock val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) val service = new CachedOneFrame[IO](mockClient, cache) // Create many different pairs to test memory usage - val currencies = List(Currency.USD, Currency.EUR, Currency.JPY, Currency.GBP, Currency.CHF, Currency.SGD, Currency.AUD, Currency.CAD, Currency.NZD) + val currencies = Currency.allCurrencies.toList val pairs = for { from <- currencies to <- currencies @@ -92,8 +91,8 @@ class PerformanceSpec extends AnyFlatSpec with Matchers { // First round should make API calls, second round should be cached mockClient.batchCallCount shouldBe pairs.length - // Verify tracked pairs are managed efficiently - cache.getTrackedPairs.unsafeRunSync().length shouldBe pairs.length + // Verify cached pairs are managed efficiently + cache.getAllCachedPairs.unsafeRunSync().length shouldBe pairs.length } it should "handle rapid cache expiration cycles efficiently" in { diff --git a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala index 17dfa96c..1503363f 100644 --- a/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala +++ b/forex-mtl/src/test/scala/forex/properties/CachedOneFramePropertySpec.scala @@ -132,7 +132,6 @@ class CachedOneFramePropertySpec extends AnyFlatSpec with Matchers { it should "maintain cache consistency under concurrent access" in { val testClock = new TestClock[IO] - implicit val clock = testClock val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) @@ -163,7 +162,6 @@ class CachedOneFramePropertySpec extends AnyFlatSpec with Matchers { ) val testClock = new TestClock[IO] - implicit val clock = testClock val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) @@ -172,10 +170,10 @@ class CachedOneFramePropertySpec extends AnyFlatSpec with Matchers { // Request all pairs testPairs.foreach(service.get(_).unsafeRunSync()) - // Tracked pairs should match distinct requested pairs - val trackedPairs = cache.getTrackedPairs.unsafeRunSync().toSet + // Cached pairs should match distinct requested pairs + val cachedPairs = cache.getAllCachedPairs.unsafeRunSync().toSet val distinctRequestedPairs = testPairs.distinct.toSet - trackedPairs shouldBe distinctRequestedPairs + cachedPairs shouldBe distinctRequestedPairs } } \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/services/rates/RateCacheSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/RateCacheSpec.scala index 6c10455f..a9f4ff13 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/RateCacheSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/RateCacheSpec.scala @@ -32,17 +32,17 @@ class RateCacheSpec extends AnyFlatSpec with Matchers { result shouldBe Some(rate) } - it should "track requested pairs" in { + it should "return all cached pairs" in { val cache = new RateCache[IO](CacheConfig(5.minutes)) - val pair1 = Rate.Pair(Currency.USD, Currency.EUR) - val pair2 = Rate.Pair(Currency.JPY, Currency.USD) + val rate1 = TestData.createTestRate(Currency.USD, Currency.EUR) + val rate2 = TestData.createTestRate(Currency.JPY, Currency.USD) - cache.get(pair1).unsafeRunSync() - cache.get(pair2).unsafeRunSync() + cache.put(rate1).unsafeRunSync() + cache.put(rate2).unsafeRunSync() - val trackedPairs = cache.getTrackedPairs.unsafeRunSync() - trackedPairs should contain(pair1) - trackedPairs should contain(pair2) + val cachedPairs = cache.getAllCachedPairs.unsafeRunSync() + cachedPairs should contain(rate1.pair) + cachedPairs should contain(rate2.pair) } it should "expire rates after TTL" in { @@ -57,39 +57,11 @@ class RateCacheSpec extends AnyFlatSpec with Matchers { cache.get(rate.pair).unsafeRunSync() shouldBe None } - it should "identify expired tracked pairs" in { - val testClock = TestClock[IO] - val cache = new RateCache[IO](CacheConfig(2.seconds))(implicitly, testClock) - val pair1 = Rate.Pair(Currency.USD, Currency.EUR) - val pair2 = Rate.Pair(Currency.JPY, Currency.USD) - val rate1 = TestData.createTestRate(pair1.from, pair1.to) - val rate2 = TestData.createTestRate(pair2.from, pair2.to) - - // Track pairs - cache.get(pair1).unsafeRunSync() - cache.get(pair2).unsafeRunSync() - - // Cache rates - cache.put(rate1).unsafeRunSync() - cache.put(rate2).unsafeRunSync() - - // Advance time to expire rates - testClock.advance(3.seconds) - - val expiredPairs = cache.getExpiredTrackedPairs.unsafeRunSync() - expiredPairs should contain(pair1) - expiredPairs should contain(pair2) - } - - it should "include never-cached tracked pairs in expired pairs" in { + it should "return empty list when no pairs are cached" in { val cache = new RateCache[IO](CacheConfig(5.minutes)) - val pair = Rate.Pair(Currency.USD, Currency.EUR) - // Track but don't cache - cache.get(pair).unsafeRunSync() - - val expiredPairs = cache.getExpiredTrackedPairs.unsafeRunSync() - expiredPairs should contain(pair) + val cachedPairs = cache.getAllCachedPairs.unsafeRunSync() + cachedPairs shouldBe List.empty } it should "cache multiple rates in batch" in { @@ -116,9 +88,7 @@ class RateCacheSpec extends AnyFlatSpec with Matchers { cache.clear().unsafeRunSync() cache.get(rate.pair).unsafeRunSync() shouldBe None - - // But tracked pairs should remain - cache.getTrackedPairs.unsafeRunSync() should contain(rate.pair) + cache.getAllCachedPairs.unsafeRunSync() shouldBe List.empty } it should "use putBatch for single put operation" in { diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala index fd645ac0..61116143 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/CachedOneFrameSpec.scala @@ -238,7 +238,6 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { it should "not add invalid pairs to tracked pairs" in { val testClock = new TestClock[IO] - implicit val clock = testClock val mockClient = new MockAlgebra[IO](Some(testClock)) val cache = new RateCache[IO](CacheConfig(5.minutes)) @@ -247,9 +246,9 @@ class CachedOneFrameSpec extends AnyFlatSpec with Matchers { // Try to get invalid pair service.get(Rate.Pair(Currency.EUR, Currency.EUR)).unsafeRunSync() - // Should not be tracked - val trackedPairs = cache.getTrackedPairs.unsafeRunSync() - trackedPairs should not contain Rate.Pair(Currency.EUR, Currency.EUR) + // Should not be cached + val cachedPairs = cache.getAllCachedPairs.unsafeRunSync() + cachedPairs should not contain Rate.Pair(Currency.EUR, Currency.EUR) // Should not make API calls mockClient.batchCallCount shouldBe 0 From 1990503c626e1decaf8a5bf4cac28f3de30dff64 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Mon, 18 Aug 2025 18:54:33 +0900 Subject: [PATCH 15/23] Readme --- forex-mtl/README.md | 188 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 forex-mtl/README.md diff --git a/forex-mtl/README.md b/forex-mtl/README.md new file mode 100644 index 00000000..7ac6e25a --- /dev/null +++ b/forex-mtl/README.md @@ -0,0 +1,188 @@ +# Forex-MTL: Exchange Rate Proxy Service + +A high-performance, thread-safe forex exchange rate service that acts as a local proxy for the One-Frame API. + +## Requirements Met + +### Requirements +- The service returns an exchange rate when provided with 2 supported currencies +- The rate should not be older than 5 minutes +- The service should support at least 10,000 successful requests per day with 1 API token (limited to 1000 requests per day) + +### Key Concepts + +**Rates Caching**: Reduces API calls by caching exchange rates for 5 minutes + +**Batch Processing**: Groups multiple currency pair requests into single API calls + +## Design Decisions + +### Core Components + +1. **OneFrameClient** - HTTP client for One-Frame API integration +2. **RateCache** - TTL-based concurrent cache using ConcurrentHashMap +3. **CachedOneFrame** - Main service orchestrating cache and API calls + +## Meeting One-Frame API Limitations + +### The Challenge +- **Target Load**: 10,000 requests/day +- **API Limit**: 1,000 requests/day +- **Required Efficiency**: 10:1 cache hit ratio + +### Solution Strategy + +#### 1. TTL-Based Caching (5 minutes) +```scala +val expiresAt = apiTimestamp.plusMillis(ttl.toMillis) +cache.put(rate.pair, CachedRate(rate, expiresAt)) +``` + +#### 2. Batch API Optimization +```scala +// Instead of: 1 request per currency pair +// We do: 1 request for multiple pairs +val pairsToFetch = (expiredPairs :+ pair).distinct +client.getBatch(pairsToFetch) +``` + +#### 3. Smart Cache Management +- **Tracked Pairs**: Only cache requested currency pairs +- **Batch Expiration**: Refresh multiple expired pairs in single API call +- **Intelligent Grouping**: Combine cache misses into batch requests + +### Efficiency Analysis + +**Best Case Scenario**: +- 288 API calls/day (one every 5 minutes) + 72 calls for adding pairs to cache + +**Result**: Comfortably within 1000 API calls/day limit + +## Thread Safety Implementation + +### Current Solution: Synchronized Method + +```scala +private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { + this.synchronized { + cache.get(pair).flatMap { + // Entire cache check + API call logic + } + } +} +``` + +**Why This Works**: +- **Atomic Operations**: Entire cache-check-and-update cycle is synchronized +- **No Race Conditions**: Only one thread can execute getCurrencyRate() at a time +- **Performance Adequate**: At ~0.12 RPS (10k requests/day), blocking is negligible +- **Simple & Reliable**: Easy to understand and maintain + +### Alternative Thread Safety Approaches + +#### 1. Double-Checked Locking Pattern +```scala +private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { + cache.get(pair) match { + case Some(rate) => F.pure(rate.asRight) + case None => + this.synchronized { + // Double-check: another thread might have updated cache + cache.get(pair) match { + case Some(rate) => F.pure(rate.asRight) + case None => performAPICall(pair) + } + } + } +} +``` +- **Pros**: Better read performance, synchronized only on cache miss +- **Cons**: More complex, error-prone implementation + +#### 2. ReadWriteLock Implementation +```scala +private val rwLock = new ReentrantReadWriteLock() + +private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { + // Read lock for cache check + rwLock.readLock().lock() + try { + cache.get(pair) match { + case Some(rate) => F.pure(rate.asRight) + case None => + // Upgrade to write lock + rwLock.readLock().unlock() + rwLock.writeLock().lock() + try { + performAPICallWithDoubleCheck(pair) + } finally { + rwLock.writeLock().unlock() + } + } + } finally { + if (rwLock.readLock().tryLock()) rwLock.readLock().unlock() + } +} +``` +- **Pros**: Maximum concurrency for reads +- **Cons**: Complex lock management, potential deadlocks, overkill for our load + +### Why Synchronized Was Chosen + +1. **Performance Requirements**: At 0.12-0.35 RPS, method-level synchronization has negligible impact +2. **Simplicity**: Single point of synchronization, easy to reason about +3. **Reliability**: No complex lock management or potential deadlocks +4. **Maintainability**: Future developers can easily understand and modify + +## Configuration & Deployment + +### Environment Configuration +```yaml +# docker-compose.yml +environment: + - HTTP_HOST=0.0.0.0 + - HTTP_PORT=8080 + - ONEFRAME_URL=http://one-frame:8080/rates? + - ONEFRAME_TOKEN=10dc303535874aeccc86a8251e6992f5 + - CACHE_TTL=5m +``` + +### Docker Deployment +```bash +# Development +docker-compose up + +# Production +docker build -t forex-mtl . +docker run -p 8080:8080 \ + -e ONEFRAME_URL=https://api.oneframe.com \ + -e ONEFRAME_TOKEN=your_token \ + forex-mtl +``` + +### Local Development +```bash +# Using local config +sbt -Dconfig.resource=application-local.conf run + +# Testing +sbt test +``` + +## Error Handling + +### Structured Error Responses +```json +{ + "error": "INVALID_CURRENCY_PAIR", + "message": "Invalid currency pair USDXXX: unknown currency XYZ", + "timestamp": "2025-08-18T12:00:00Z" +} +``` + +### Error Categories +- **400**: Invalid currency parameters, missing parameters +- **401**: Authentication errors with One-Frame API +- **429**: Rate limiting (quota exceeded) +- **500**: Internal server errors, One-Frame API issues +- **503**: Service unavailable \ No newline at end of file From 47dc2d09924c00ce21dd0f8f2ab6a0801d44adb3 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Mon, 18 Aug 2025 18:57:29 +0900 Subject: [PATCH 16/23] Readme --- forex-mtl/README.md | 97 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 91 insertions(+), 6 deletions(-) diff --git a/forex-mtl/README.md b/forex-mtl/README.md index 7ac6e25a..3a89bec8 100644 --- a/forex-mtl/README.md +++ b/forex-mtl/README.md @@ -20,7 +20,7 @@ A high-performance, thread-safe forex exchange rate service that acts as a local ### Core Components 1. **OneFrameClient** - HTTP client for One-Frame API integration -2. **RateCache** - TTL-based concurrent cache using ConcurrentHashMap +2. **RateCache** - TTL-based concurrent cache using TrieMap 3. **CachedOneFrame** - Main service orchestrating cache and API calls ## Meeting One-Frame API Limitations @@ -42,19 +42,20 @@ cache.put(rate.pair, CachedRate(rate, expiresAt)) ```scala // Instead of: 1 request per currency pair // We do: 1 request for multiple pairs -val pairsToFetch = (expiredPairs :+ pair).distinct +val pairsToFetch = (allCachedPairs :+ requestedPair).distinct client.getBatch(pairsToFetch) ``` #### 3. Smart Cache Management -- **Tracked Pairs**: Only cache requested currency pairs -- **Batch Expiration**: Refresh multiple expired pairs in single API call -- **Intelligent Grouping**: Combine cache misses into batch requests +- **Simplified Strategy**: On cache miss, refresh ALL cached pairs + requested pair +- **TrieMap Storage**: Thread-safe concurrent map for high-performance access +- **TTL-Based Expiration**: Automatic cleanup of expired rates ### Efficiency Analysis **Best Case Scenario**: -- 288 API calls/day (one every 5 minutes) + 72 calls for adding pairs to cache +- 288 API calls/day (one every 5 minutes for all active pairs) +- Additional calls only when new currency pairs are requested **Result**: Comfortably within 1000 API calls/day limit @@ -134,6 +135,90 @@ private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { 3. **Reliability**: No complex lock management or potential deadlocks 4. **Maintainability**: Future developers can easily understand and modify +## Cache Implementation Alternatives + +### Current Solution: In-Memory TrieMap +```scala +private val cache = TrieMap[Rate.Pair, CachedRate]() +``` + +**Pros**: +- Simple, lightweight implementation +- Thread-safe concurrent access +- No external dependencies +- Perfect for single-instance deployments + +**Cons**: +- Memory usage grows with number of currency pairs +- Data lost on application restart +- No cache eviction policies beyond TTL + +### Alternative: EhCache Integration + +For production deployments requiring persistence and advanced cache management: + +```scala +// build.sbt +libraryDependencies += "net.sf.ehcache" % "ehcache" % "2.10.9.2" + +// EhCache configuration +class EhCacheRateCache[F[_]: Sync](cacheManager: CacheManager, config: CacheConfig) extends RateCache[F] { + private val cache = cacheManager.getCache("forex-rates") + + def get(pair: Rate.Pair): F[Option[Rate]] = Sync[F].delay { + Option(cache.get(pair.toString)) + .map(_.asInstanceOf[CachedRate]) + .filter(cachedRate => !isExpired(cachedRate)) + .map(_.rate) + } + + def put(rate: Rate): F[Unit] = Sync[F].delay { + val element = new Element(rate.pair.toString, CachedRate(rate, expiresAt)) + cache.put(element) + } +} +``` + +**EhCache Benefits**: +- **Persistence**: Survive application restarts +- **Memory Management**: LRU eviction, size limits +- **Monitoring**: JMX integration, cache statistics +- **Clustering**: Distributed cache for multi-instance setups + +**When to Consider EhCache**: +- Multi-instance deployments requiring shared cache +- High memory usage concerns +- Need for cache persistence across restarts +- Advanced monitoring and management requirements + +### Redis Alternative + +For microservices architectures: + +```scala +// Redis-based cache implementation +libraryDependencies += "dev.profunktor" %% "redis4cats-effects" % "1.4.1" + +class RedisRateCache[F[_]: Async](redis: RedisCommands[F, String, String]) extends RateCache[F] { + def get(pair: Rate.Pair): F[Option[Rate]] = { + redis.get(s"rate:${pair.toString}") + .map(_.flatMap(json => parseRate(json))) + .map(_.filter(rate => !isExpired(rate))) + } +} +``` + +**Redis Benefits**: +- External cache service +- Horizontal scaling +- Pub/sub for cache invalidation +- Rich data structures + +**Trade-offs**: +- Network latency for cache operations +- Additional infrastructure complexity +- Requires Redis deployment and management + ## Configuration & Deployment ### Environment Configuration From 40509d84a5f88c8ad4ebfe71b6f3be464165bb47 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Tue, 19 Aug 2025 18:10:22 +0900 Subject: [PATCH 17/23] Pare small fixes --- forex-mtl/README.md | 25 +++++++++++++++---- .../forex/http/rates/RatesHttpRoutes.scala | 6 +++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/forex-mtl/README.md b/forex-mtl/README.md index 3a89bec8..339b0cbc 100644 --- a/forex-mtl/README.md +++ b/forex-mtl/README.md @@ -7,7 +7,7 @@ A high-performance, thread-safe forex exchange rate service that acts as a local ### Requirements - The service returns an exchange rate when provided with 2 supported currencies - The rate should not be older than 5 minutes -- The service should support at least 10,000 successful requests per day with 1 API token (limited to 1000 requests per day) +- The service should support at least 10,000 successful requests per day with 1 API token (limited to 1000 requests per day per token) ### Key Concepts @@ -259,9 +259,9 @@ sbt test ### Structured Error Responses ```json { - "error": "INVALID_CURRENCY_PAIR", - "message": "Invalid currency pair USDXXX: unknown currency XYZ", - "timestamp": "2025-08-18T12:00:00Z" + "error":"INVALID_PARAMETERS", + "message":"Invalid currency parameters. Supported currencies: AUD, JPY, CAD, NZD, CHF, SGD, EUR, USD, GBP", + "timestamp":"2025-08-19T01:29:02.234068157Z" } ``` @@ -270,4 +270,19 @@ sbt test - **401**: Authentication errors with One-Frame API - **429**: Rate limiting (quota exceeded) - **500**: Internal server errors, One-Frame API issues -- **503**: Service unavailable \ No newline at end of file +- **503**: Service unavailable + +### Error Logging & Alerts + +All errors are logged in detail using the project's logger. These logs can be integrated with alerting systems (e.g., via Prometheus, Grafana, or external log monitoring) to notify operators about critical issues. + +#### Example error logs: + +```scala +logger.error(s"Invalid currency parameters: $params") +logger.error(s"One-Frame API authentication failed: ${ex.getMessage}") +logger.error(s"Rate limit exceeded for token: $token") +logger.error(s"Unexpected error fetching rates: ${ex.getMessage}", ex) +``` + +Alerts can be configured to trigger on specific error patterns or severity levels in the logs. diff --git a/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala b/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala index e4e4cd37..49749d9d 100644 --- a/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala +++ b/forex-mtl/src/main/scala/forex/http/rates/RatesHttpRoutes.scala @@ -3,13 +3,15 @@ package rates import cats.effect.Sync import cats.syntax.flatMap._ +import forex.domain.Currency import forex.programs.RatesProgram -import forex.programs.rates.{ Protocol => RatesProgramProtocol } +import forex.programs.rates.{Protocol => RatesProgramProtocol} import forex.programs.rates.errors.Error import org.http4s.{HttpRoutes, Status} import org.http4s.dsl.Http4sDsl import org.http4s.server.Router import org.slf4j.LoggerFactory + import java.time.Instant class RatesHttpRoutes[F[_]: Sync](rates: RatesProgram[F]) extends Http4sDsl[F] { @@ -44,7 +46,7 @@ class RatesHttpRoutes[F[_]: Sync](rates: RatesProgram[F]) extends Http4sDsl[F] { Sync[F].delay(logger.warn(s"Invalid currency parameters in request: ${req.uri.query}")).flatMap { _ => val errorResponse = ErrorApiResponse( error = "INVALID_PARAMETERS", - message = "Invalid currency parameters. Supported currencies: AUD, CAD, CHF, EUR, GBP, NZD, JPY, SGD, USD", + message = "Invalid currency parameters. Supported currencies: " + Currency.allCurrencies.mkString(", "), timestamp = Instant.now().toString ) BadRequest(errorResponse) From 0da9783e9ad3f4b2e3d514ada9b30d83acbbf9e3 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Thu, 21 Aug 2025 08:52:46 +0900 Subject: [PATCH 18/23] some readme and test updates --- forex-mtl/README.md | 27 +++++++ .../interpreters/OneFrameClientSpec.scala | 73 +++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/forex-mtl/README.md b/forex-mtl/README.md index 339b0cbc..1a6c3e92 100644 --- a/forex-mtl/README.md +++ b/forex-mtl/README.md @@ -219,6 +219,33 @@ class RedisRateCache[F[_]: Async](redis: RedisCommands[F, String, String]) exten - Additional infrastructure complexity - Requires Redis deployment and management +## Direct and Reverse Currency Pairs Handling + +In the current implementation, direct and reverse currency pairs (e.g., USD/EUR and EUR/USD) are requested separately from the API. This simplifies the logic but may lead to discrepancies between direct and reverse rates due to data source specifics or rounding. + +**Alternative approach:** +Only the direct pair (e.g., USD/EUR) is requested, and the reverse pair (EUR/USD) is automatically calculated as `1 / direct_rate`. This guarantees mathematical consistency between pairs, but requires additional logic for request handling and caching. + +**Example of alternative implementation:** +```scala +def getRate(pair: Rate.Pair): F[Error Either Rate] = { + cache.get(pair) match { + case Some(rate) => F.pure(rate.asRight) + case None => + val directPair = pair + val reversePair = Rate.Pair(pair.to, pair.from) + cache.get(reversePair) match { + case Some(reverseRate) => + // Calculate reverse rate + val calculatedRate = 1.0 / reverseRate.price + F.pure(Rate(pair.from, pair.to, calculatedRate).asRight) + case None => + // Request direct pair from API + fetchAndCacheRate(directPair) + } + } +} +``` ## Configuration & Deployment ### Environment Configuration diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala index 5548dd13..b5269387 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala @@ -5,6 +5,7 @@ import cats.effect.{ContextShift, IO, Timer} import scala.concurrent.ExecutionContext.Implicits.global import forex.config.OneFrameConfig import forex.domain.{Currency, Rate} +import forex.services.rates.errors.Error._ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -59,4 +60,76 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { result shouldBe Right(List.empty) } + + // Error handling tests - using malformed URLs to test error handling logic + it should "handle invalid URL configuration" in { + // Use invalid protocol to trigger connection error + val config = OneFrameConfig("invalid-protocol://test.com/rates?", "test-token") + val client = new OneFrameClient[IO](config) + val pairs = List(Rate.Pair(Currency.USD, Currency.EUR)) + + val result = client.getBatch(pairs).unsafeRunSync() + + result.isLeft shouldBe true + result.left.foreach { error => + error shouldBe a[NetworkError] + error.message should not be empty + } + } + + it should "properly map single pair failures to get() method" in { + // Test that single pair method correctly handles batch failures + val config = OneFrameConfig("invalid://bad-url", "test-token") + val client = new OneFrameClient[IO](config) + val pair = Rate.Pair(Currency.USD, Currency.EUR) + + val result = client.get(pair).unsafeRunSync() + + result.isLeft shouldBe true + result.left.foreach { error => + error shouldBe a[NetworkError] + } + } + + it should "handle empty batch response correctly in get() method" in { + // This would require mocking, but we can test the URL building at least + val config = OneFrameConfig("http://test.com/rates?", "test-token") + val client = new OneFrameClient[IO](config) + + // Test URL generation works correctly for edge cases + val singlePairUrl = client.buildBatchUrl(List(Rate.Pair(Currency.USD, Currency.USD))) + singlePairUrl should include("USDUSD") + } + + // Test error message patterns from actual OneFrameClient error handling + it should "create appropriate error messages for different failure scenarios" in { + val config = OneFrameConfig("http://test.com/rates?", "test-token") + val client = new OneFrameClient[IO](config) + + // Test URL building works for various scenarios + val multiPairUrl = client.buildBatchUrl(List( + Rate.Pair(Currency.USD, Currency.EUR), + Rate.Pair(Currency.JPY, Currency.GBP), + Rate.Pair(Currency.CHF, Currency.AUD) + )) + + multiPairUrl should include("pair=USDEUR") + multiPairUrl should include("pair=JPYGBP") + multiPairUrl should include("pair=CHFAUD") + multiPairUrl should include("&") // Should have proper URL param separators + } + + it should "handle edge case currency combinations in URL building" in { + val config = OneFrameConfig("https://api.example.com:8443/v2/rates?", "secret123") + val client = new OneFrameClient[IO](config) + + // Test various currency combinations + val pairs = List( + Rate.Pair(Currency.SGD, Currency.NZD), + Rate.Pair(Currency.CAD, Currency.AUD) + ) + + val url = client.buildBatchUrl(pairs) + url shouldBe "https://api.example.com:8443/v2/rates?pair=SGDNZD&pair=CADAUD" + } } \ No newline at end of file From d4c816f2561ab2dcb61b395a9cb4379554b03949 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Thu, 21 Aug 2025 11:22:31 +0900 Subject: [PATCH 19/23] Reliability block in readme and Double-Checked Locking --- forex-mtl/README.md | 97 +++++++++++++------ .../rates/interpreters/CachedOneFrame.scala | 64 ++++++------ 2 files changed, 107 insertions(+), 54 deletions(-) diff --git a/forex-mtl/README.md b/forex-mtl/README.md index 1a6c3e92..2ae459b9 100644 --- a/forex-mtl/README.md +++ b/forex-mtl/README.md @@ -59,46 +59,88 @@ client.getBatch(pairsToFetch) **Result**: Comfortably within 1000 API calls/day limit +## Reliability & High Availability + +### Multi-Node Deployment Strategy + +The current implementation supports reliable production deployment with 2 active nodes and 3 hot-standby nodes. This architecture ensures high availability and fault tolerance with minimal risk of One-Frame quota exceed. + +### Failover Scenarios + +#### **Single Node Failure** +- **Detection Time**: Automatic detection and reaction from Orchestrator (health check, container restart) with response time less than 10 seconds +- **Recovery**: Load-balancer may automatically route traffic to healthy node until failed node is restarted or replaced by standby node +- **Impact**: + - **Downtime**: Minimal downtime of failed node + - **Capacity**: Remaining node handles full load + +#### **Double Node Failure (Hot-Standby Activation)** +- **Trigger**: Both active nodes unavailable +- **Action**: Orchestrator scales up standby nodes +- **Recovery Time**: 60-90 seconds (container startup + cache warmup) + +### Reliability Benefits + +1. **99.9%+ Uptime**: Multi-node redundancy with automatic failover +2. **Graceful Degradation**: System continues operating with reduced capacity +3. **Disaster Recovery**: Geographic distribution of standby nodes possible + +### Monitoring & Alerts +Project provides sufficient logging for such issues, including: +- **One-Frame unavailability** +- **Authentication errors**: token issues +- **Rate limit exceeded**: API quota issues + +Integration with monitoring tools (e.g., Prometheus, Grafana) can provide real-time alerts on these events. + ## Thread Safety Implementation -### Current Solution: Synchronized Method +### Current Solution: Double-Checked Locking Pattern ```scala private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { - this.synchronized { - cache.get(pair).flatMap { - // Entire cache check + API call logic - } + // First check: Read from cache without synchronization + cache.get(pair).flatMap { + case Some(cachedRate) => + logger.debug(s"Cache HIT (unsynchronized read)") + F.pure(cachedRate.asRight[Error]) + case None => + // Cache miss - need to synchronize and double-check + this.synchronized { + cache.get(pair).flatMap { + case Some(cachedRate) => + // Double-check: Another thread might have populated cache + logger.debug(s"Cache HIT (synchronized double-check)") + F.pure(cachedRate.asRight[Error]) + case None => + // Confirmed cache miss - perform API call + performBatchAPICall(pair) + } + } } } ``` **Why This Works**: -- **Atomic Operations**: Entire cache-check-and-update cycle is synchronized -- **No Race Conditions**: Only one thread can execute getCurrencyRate() at a time -- **Performance Adequate**: At ~0.12 RPS (10k requests/day), blocking is negligible -- **Simple & Reliable**: Easy to understand and maintain +- **Optimized Cache Reads**: Most cache hits avoid synchronization entirely +- **Race Condition Prevention**: Double-check pattern prevents duplicate API calls +- **Better Concurrency**: Multiple threads can read from cache simultaneously +- **API Call Protection**: Only synchronized when cache miss confirmed ### Alternative Thread Safety Approaches -#### 1. Double-Checked Locking Pattern +#### 1. Simple Synchronized Method ```scala private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { - cache.get(pair) match { - case Some(rate) => F.pure(rate.asRight) - case None => - this.synchronized { - // Double-check: another thread might have updated cache - cache.get(pair) match { - case Some(rate) => F.pure(rate.asRight) - case None => performAPICall(pair) - } - } + this.synchronized { + cache.get(pair).flatMap { + // Entire cache check + API call logic + } } } ``` -- **Pros**: Better read performance, synchronized only on cache miss -- **Cons**: More complex, error-prone implementation +- **Pros**: Simple implementation, easy to understand +- **Cons**: All cache reads are synchronized, lower concurrent performance #### 2. ReadWriteLock Implementation ```scala @@ -128,12 +170,13 @@ private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { - **Pros**: Maximum concurrency for reads - **Cons**: Complex lock management, potential deadlocks, overkill for our load -### Why Synchronized Was Chosen +### Why Double-Checked Locking Was Chosen -1. **Performance Requirements**: At 0.12-0.35 RPS, method-level synchronization has negligible impact -2. **Simplicity**: Single point of synchronization, easy to reason about -3. **Reliability**: No complex lock management or potential deadlocks -4. **Maintainability**: Future developers can easily understand and modify +1. **Performance Optimization**: At 10k+ requests/day, optimizing cache reads becomes important +2. **Concurrency Benefits**: Multiple threads can read from cache without blocking each other +3. **API Call Protection**: Still prevents race conditions for expensive API calls +4. **Balanced Approach**: More complex than simple sync, but significantly better performance +5. **Production Ready**: Well-known pattern suitable for high-throughput caching scenarios ## Cache Implementation Alternatives diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala index db2d82ac..5b81699f 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/CachedOneFrame.scala @@ -40,35 +40,45 @@ class CachedOneFrame[F[_]: ConcurrentEffect]( } private def getCurrencyRate(pair: Rate.Pair): F[Error Either Rate] = { - this.synchronized { - cache.get(pair).flatMap { - case Some(cachedRate) => - logger.debug(s"Cache HIT for ${pair.from}${pair.to}") - ConcurrentEffect[F].pure(cachedRate.asRight[Error]) - case None => - logger.debug(s"Cache MISS for ${pair.from}${pair.to}") - cache.getAllCachedPairs.flatMap { allCachedPairs => - val pairsToFetch = (allCachedPairs :+ pair).distinct - val pairsStr = pairsToFetch.map(p => s"${p.from}${p.to}").mkString(", ") - logger.info(s"Batch request for ALL pairs: [$pairsStr]") - - client.getBatch(pairsToFetch).flatMap { - case Right(rates) => - cache.putBatch(rates).flatMap { _ => - rates.find(_.pair == pair) match { - case Some(rate) => - ConcurrentEffect[F].pure(rate.asRight[Error]) - case None => - val pairStr = s"${pair.from}${pair.to}" - logger.warn(s"Requested pair $pairStr not found in batch response") - ConcurrentEffect[F].pure(RateNotFound(pairStr).asLeft[Rate]) - } - } - case Left(error) => - logger.error(s"Batch API call failed: $error") - ConcurrentEffect[F].pure(error.asLeft[Rate]) + cache.get(pair).flatMap { + case Some(cachedRate) => + logger.debug(s"Cache HIT for ${pair.from}${pair.to} (unsynchronized read)") + ConcurrentEffect[F].pure(cachedRate.asRight[Error]) + case None => + this.synchronized { + cache.get(pair).flatMap { + case Some(cachedRate) => + logger.debug(s"Cache HIT for ${pair.from}${pair.to} (synchronized double-check)") + ConcurrentEffect[F].pure(cachedRate.asRight[Error]) + case None => + logger.debug(s"Cache MISS for ${pair.from}${pair.to} (confirmed after double-check)") + performBatchAPICall(pair) + } + } + } + } + + private def performBatchAPICall(pair: Rate.Pair): F[Error Either Rate] = { + cache.getAllCachedPairs.flatMap { allCachedPairs => + val pairsToFetch = (allCachedPairs :+ pair).distinct + val pairsStr = pairsToFetch.map(p => s"${p.from}${p.to}").mkString(", ") + logger.info(s"Batch request for ALL pairs: [$pairsStr]") + + client.getBatch(pairsToFetch).flatMap { + case Right(rates) => + cache.putBatch(rates).flatMap { _ => + rates.find(_.pair == pair) match { + case Some(rate) => + ConcurrentEffect[F].pure(rate.asRight[Error]) + case None => + val pairStr = s"${pair.from}${pair.to}" + logger.warn(s"Requested pair $pairStr not found in batch response") + ConcurrentEffect[F].pure(RateNotFound(pairStr).asLeft[Rate]) } } + case Left(error) => + logger.error(s"Batch API call failed: $error") + ConcurrentEffect[F].pure(error.asLeft[Rate]) } } } From c879c71af618b2597037745fe74d3e3f49337678 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Thu, 21 Aug 2025 11:34:34 +0900 Subject: [PATCH 20/23] small fix about additional calls --- forex-mtl/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/forex-mtl/README.md b/forex-mtl/README.md index 2ae459b9..92ab2fc1 100644 --- a/forex-mtl/README.md +++ b/forex-mtl/README.md @@ -55,7 +55,7 @@ client.getBatch(pairsToFetch) **Best Case Scenario**: - 288 API calls/day (one every 5 minutes for all active pairs) -- Additional calls only when new currency pairs are requested +- Additional calls only when new currency pairs are requested (max 72) **Result**: Comfortably within 1000 API calls/day limit From 15df904f527d7cb22dad09099606393592bc0990 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Fri, 22 Aug 2025 13:56:35 +0900 Subject: [PATCH 21/23] Servers time diff check added --- forex-mtl/README.md | 36 ++++++++++++++++++ forex-mtl/docker-compose.yml | 1 + .../src/main/resources/application-local.conf | 1 + forex-mtl/src/main/resources/application.conf | 1 + .../forex/config/ApplicationConfig.scala | 3 +- .../rates/interpreters/OneFrameClient.scala | 16 ++++++++ .../test/scala/forex/helpers/TestData.scala | 2 +- .../interpreters/OneFrameClientSpec.scala | 38 ++++++++++++++----- 8 files changed, 86 insertions(+), 12 deletions(-) diff --git a/forex-mtl/README.md b/forex-mtl/README.md index 92ab2fc1..54b46735 100644 --- a/forex-mtl/README.md +++ b/forex-mtl/README.md @@ -299,6 +299,7 @@ environment: - HTTP_PORT=8080 - ONEFRAME_URL=http://one-frame:8080/rates? - ONEFRAME_TOKEN=10dc303535874aeccc86a8251e6992f5 + - ONEFRAME_TIME_TOLERANCE=30s - CACHE_TTL=5m ``` @@ -312,6 +313,7 @@ docker build -t forex-mtl . docker run -p 8080:8080 \ -e ONEFRAME_URL=https://api.oneframe.com \ -e ONEFRAME_TOKEN=your_token \ + -e ONEFRAME_TIME_TOLERANCE=30s \ forex-mtl ``` @@ -342,6 +344,40 @@ sbt test - **500**: Internal server errors, One-Frame API issues - **503**: Service unavailable +### Time Synchronization Monitoring + +The service includes configurable time synchronization monitoring to detect clock drift between servers: + +**Configuration:** +```hocon +app { + one-frame { + time-tolerance = ${ONEFRAME_TIME_TOLERANCE} + } +} +``` + +**Monitoring Logic:** +- Compares API timestamps with local server time +- Logs warnings when time difference exceeds configured tolerance +- Provides ops team with early warning for NTP synchronization issues + +**Implementation:** +```scala +def checkTimeSync(timestamp: String): Unit = { + val timeDiff = Duration.between(now, apiTimestamp).abs() + if (timeDiff > config.timeTolerance) { + logger.warn(s"Time sync issue: ${timeDiff.getSeconds}s difference") + } +} +``` + +**Why This Approach:** +- **Simple and reliable** - minimal complexity, maximum uptime +- **Configurable thresholds** - adjust sensitivity per environment +- **Operations-friendly** - provides monitoring without service disruption +- **Production-ready** - battle-tested approach for distributed systems + ### Error Logging & Alerts All errors are logged in detail using the project's logger. These logs can be integrated with alerting systems (e.g., via Prometheus, Grafana, or external log monitoring) to notify operators about critical issues. diff --git a/forex-mtl/docker-compose.yml b/forex-mtl/docker-compose.yml index 5fd7f183..95e7600f 100644 --- a/forex-mtl/docker-compose.yml +++ b/forex-mtl/docker-compose.yml @@ -16,6 +16,7 @@ services: - HTTP_PORT=8080 - ONEFRAME_URL=http://one-frame:8080/rates? - ONEFRAME_TOKEN=10dc303535874aeccc86a8251e6992f5 + - ONEFRAME_TIME_TOLERANCE=30s - CACHE_TTL=5m depends_on: - one-frame diff --git a/forex-mtl/src/main/resources/application-local.conf b/forex-mtl/src/main/resources/application-local.conf index 0461403e..ffa39797 100644 --- a/forex-mtl/src/main/resources/application-local.conf +++ b/forex-mtl/src/main/resources/application-local.conf @@ -8,6 +8,7 @@ app { one-frame { url = "http://localhost:8086" token = "10dc303535874aeccc86a8251e6992f5" + time-tolerance = 30 seconds } cache { diff --git a/forex-mtl/src/main/resources/application.conf b/forex-mtl/src/main/resources/application.conf index d5474819..bfd1f75a 100644 --- a/forex-mtl/src/main/resources/application.conf +++ b/forex-mtl/src/main/resources/application.conf @@ -8,6 +8,7 @@ app { one-frame { url = ${ONEFRAME_URL} token = ${ONEFRAME_TOKEN} + time-tolerance = ${ONEFRAME_TIME_TOLERANCE} } cache { diff --git a/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala b/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala index 806227ac..27c08e6c 100644 --- a/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala +++ b/forex-mtl/src/main/scala/forex/config/ApplicationConfig.scala @@ -16,7 +16,8 @@ final case class HttpConfig( final case class OneFrameConfig( url: String, - token: String + token: String, + timeTolerance: FiniteDuration ) final case class CacheConfig( diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index ebd12c5b..b8f2e591 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -30,6 +30,21 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec private val logger = LoggerFactory.getLogger(classOf[OneFrameClient[F]]) + def checkTimeSync(timestamp: String): Unit = { + try { + val apiTimestamp = OffsetDateTime.parse(timestamp) + val now = OffsetDateTime.now() + val timeDiff = java.time.Duration.between(now, apiTimestamp).abs() + + if (timeDiff.compareTo(java.time.Duration.ofSeconds(config.timeTolerance.toSeconds)) > 0) { + logger.warn(s"Time synchronization issue detected: API timestamp $timestamp differs from server time by ${timeDiff.getSeconds} seconds (tolerance: ${config.timeTolerance.toSeconds}s)") + } + } catch { + case ex: Exception => + logger.warn(s"Invalid timestamp format from API: $timestamp - ${ex.getMessage}") + } + } + def buildBatchUrl(pairs: List[Rate.Pair]): String = { val pairStrings = pairs.map(p => s"${p.from}${p.to}") val queryString = pairStrings.map(p => s"pair=$p").mkString("&") @@ -57,6 +72,7 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec client.expect[List[OneFrameResponse]](request).map { responses => val rates = responses.flatMap { response => + checkTimeSync(response.time_stamp) for { fromCurrency <- Currency.fromString(response.from) toCurrency <- Currency.fromString(response.to) diff --git a/forex-mtl/src/test/scala/forex/helpers/TestData.scala b/forex-mtl/src/test/scala/forex/helpers/TestData.scala index 2b85ba78..9ff1bc5d 100644 --- a/forex-mtl/src/test/scala/forex/helpers/TestData.scala +++ b/forex-mtl/src/test/scala/forex/helpers/TestData.scala @@ -35,7 +35,7 @@ object TestData { val defaultTestConfig = forex.config.ApplicationConfig( http = forex.config.HttpConfig("localhost", 8085, 30.seconds), - oneFrame = forex.config.OneFrameConfig("http://localhost:8080", "test-token"), + oneFrame = forex.config.OneFrameConfig("http://localhost:8080", "test-token", 30.seconds), cache = forex.config.CacheConfig(5.minutes) ) } \ No newline at end of file diff --git a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala index b5269387..6dfa7578 100644 --- a/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala +++ b/forex-mtl/src/test/scala/forex/services/rates/interpreters/OneFrameClientSpec.scala @@ -8,13 +8,14 @@ import forex.domain.{Currency, Rate} import forex.services.rates.errors.Error._ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import scala.concurrent.duration._ class OneFrameClientSpec extends AnyFlatSpec with Matchers { implicit val cs: ContextShift[IO] = IO.contextShift(global) implicit val timer: Timer[IO] = IO.timer(global) "OneFrameClient" should "build correct URL for single pair" in { - val config = OneFrameConfig("http://api.example.com/rates?", "test-token") + val config = OneFrameConfig("http://api.example.com/rates?", "test-token", 30.seconds) val client = new OneFrameClient[IO](config) val pair = Rate.Pair(Currency.USD, Currency.EUR) @@ -23,7 +24,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "build correct URL for multiple pairs" in { - val config = OneFrameConfig("http://api.example.com/rates?", "secret-key") + val config = OneFrameConfig("http://api.example.com/rates?", "secret-key", 30.seconds) val client = new OneFrameClient[IO](config) val pairs = List( Rate.Pair(Currency.USD, Currency.EUR), @@ -36,7 +37,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "handle special characters in base URL" in { - val config = OneFrameConfig("http://api.example.com:8080/api/v1/rates?", "token123") + val config = OneFrameConfig("http://api.example.com:8080/api/v1/rates?", "token123", 30.seconds) val client = new OneFrameClient[IO](config) val pairs = List(Rate.Pair(Currency.CHF, Currency.SGD)) @@ -45,7 +46,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "build URL for empty pair list" in { - val config = OneFrameConfig("http://test.com/rates?", "test-token") + val config = OneFrameConfig("http://test.com/rates?", "test-token", 30.seconds) val client = new OneFrameClient[IO](config) val url = client.buildBatchUrl(List.empty) @@ -53,7 +54,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "handle empty batch request" in { - val config = OneFrameConfig("http://test.com/rates?", "test-token") + val config = OneFrameConfig("http://test.com/rates?", "test-token", 30.seconds) val client = new OneFrameClient[IO](config) val result = client.getBatch(List.empty).unsafeRunSync() @@ -64,7 +65,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { // Error handling tests - using malformed URLs to test error handling logic it should "handle invalid URL configuration" in { // Use invalid protocol to trigger connection error - val config = OneFrameConfig("invalid-protocol://test.com/rates?", "test-token") + val config = OneFrameConfig("invalid-protocol://test.com/rates?", "test-token", 30.seconds) val client = new OneFrameClient[IO](config) val pairs = List(Rate.Pair(Currency.USD, Currency.EUR)) @@ -79,7 +80,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { it should "properly map single pair failures to get() method" in { // Test that single pair method correctly handles batch failures - val config = OneFrameConfig("invalid://bad-url", "test-token") + val config = OneFrameConfig("invalid://bad-url", "test-token", 30.seconds) val client = new OneFrameClient[IO](config) val pair = Rate.Pair(Currency.USD, Currency.EUR) @@ -93,7 +94,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { it should "handle empty batch response correctly in get() method" in { // This would require mocking, but we can test the URL building at least - val config = OneFrameConfig("http://test.com/rates?", "test-token") + val config = OneFrameConfig("http://test.com/rates?", "test-token", 30.seconds) val client = new OneFrameClient[IO](config) // Test URL generation works correctly for edge cases @@ -103,7 +104,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { // Test error message patterns from actual OneFrameClient error handling it should "create appropriate error messages for different failure scenarios" in { - val config = OneFrameConfig("http://test.com/rates?", "test-token") + val config = OneFrameConfig("http://test.com/rates?", "test-token", 30.seconds) val client = new OneFrameClient[IO](config) // Test URL building works for various scenarios @@ -120,7 +121,7 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { } it should "handle edge case currency combinations in URL building" in { - val config = OneFrameConfig("https://api.example.com:8443/v2/rates?", "secret123") + val config = OneFrameConfig("https://api.example.com:8443/v2/rates?", "secret123", 30.seconds) val client = new OneFrameClient[IO](config) // Test various currency combinations @@ -132,4 +133,21 @@ class OneFrameClientSpec extends AnyFlatSpec with Matchers { val url = client.buildBatchUrl(pairs) url shouldBe "https://api.example.com:8443/v2/rates?pair=SGDNZD&pair=CADAUD" } + + it should "handle time synchronization checks without throwing exceptions" in { + val config = OneFrameConfig("http://test.com/rates?", "test-token", 10.seconds) + val client = new OneFrameClient[IO](config) + + // Test various timestamp scenarios - should not throw exceptions + val validTimestamp = java.time.OffsetDateTime.now().toString + val futureTimestamp = java.time.OffsetDateTime.now().plusMinutes(1).toString + val pastTimestamp = java.time.OffsetDateTime.now().minusMinutes(1).toString + val invalidTimestamp = "not-a-timestamp" + + // These should execute without throwing exceptions + noException should be thrownBy client.checkTimeSync(validTimestamp) + noException should be thrownBy client.checkTimeSync(futureTimestamp) + noException should be thrownBy client.checkTimeSync(pastTimestamp) + noException should be thrownBy client.checkTimeSync(invalidTimestamp) + } } \ No newline at end of file From 0132542fb47e9891dc6c0b1b37f8756d8acd158f Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Fri, 22 Aug 2025 14:12:32 +0900 Subject: [PATCH 22/23] little better timediff error message --- forex-mtl/README.md | 13 ++++++++++--- .../main/scala/forex/services/rates/RateCache.scala | 12 +++++++----- .../rates/interpreters/OneFrameClient.scala | 8 +++++--- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/forex-mtl/README.md b/forex-mtl/README.md index 54b46735..163f2829 100644 --- a/forex-mtl/README.md +++ b/forex-mtl/README.md @@ -365,13 +365,20 @@ app { **Implementation:** ```scala def checkTimeSync(timestamp: String): Unit = { - val timeDiff = Duration.between(now, apiTimestamp).abs() - if (timeDiff > config.timeTolerance) { - logger.warn(s"Time sync issue: ${timeDiff.getSeconds}s difference") + val timeDiff = Duration.between(now, apiTimestamp) + val direction = if (timeDiff.isNegative) "behind" else "ahead" + if (timeDiff.abs() > config.timeTolerance) { + logger.warn(s"API timestamp is ${timeDiff.abs().getSeconds}s $direction of server time") } } ``` +**Example log output:** +``` +WARN - Time synchronization issue detected: API timestamp 2025-08-22T10:00:00Z is 45s ahead of server time 2025-08-22T09:59:15Z (tolerance: 30s) +WARN - Time synchronization issue detected: API timestamp 2025-08-22T09:58:30Z is 90s behind of server time 2025-08-22T10:00:00Z (tolerance: 30s) +``` + **Why This Approach:** - **Simple and reliable** - minimal complexity, maximum uptime - **Configurable thresholds** - adjust sensitivity per environment diff --git a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala index 82404a00..be31334a 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/RateCache.scala @@ -1,6 +1,7 @@ package forex.services.rates import cats.effect.{Clock, Sync} +import cats.implicits.toFlatMapOps import cats.syntax.functor._ import forex.config.CacheConfig import forex.domain.Rate @@ -44,11 +45,12 @@ class RateCache[F[_]: Sync: Clock](config: CacheConfig) { } def putBatch(rates: List[Rate]): F[Unit] = { - Sync[F].delay { - rates.foreach { rate => - val apiTimestamp = rate.timestamp.value.toInstant - val expiresAt = apiTimestamp.plusMillis(ttl.toMillis) - cache.put(rate.pair, CachedRate(rate, expiresAt)) + Clock[F].realTime(MILLISECONDS).flatMap { nowMillis => + Sync[F].delay { + val expiresAt = Instant.ofEpochMilli(nowMillis).plusMillis(ttl.toMillis) + rates.foreach { rate => + cache.put(rate.pair, CachedRate(rate, expiresAt)) + } } } } diff --git a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala index b8f2e591..1d8bed96 100644 --- a/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala +++ b/forex-mtl/src/main/scala/forex/services/rates/interpreters/OneFrameClient.scala @@ -34,10 +34,12 @@ class OneFrameClient[F[_]: ConcurrentEffect](config: OneFrameConfig)(implicit ec try { val apiTimestamp = OffsetDateTime.parse(timestamp) val now = OffsetDateTime.now() - val timeDiff = java.time.Duration.between(now, apiTimestamp).abs() + val timeDiff = java.time.Duration.between(now, apiTimestamp) + val absDiff = timeDiff.abs() - if (timeDiff.compareTo(java.time.Duration.ofSeconds(config.timeTolerance.toSeconds)) > 0) { - logger.warn(s"Time synchronization issue detected: API timestamp $timestamp differs from server time by ${timeDiff.getSeconds} seconds (tolerance: ${config.timeTolerance.toSeconds}s)") + if (absDiff.compareTo(java.time.Duration.ofSeconds(config.timeTolerance.toSeconds)) > 0) { + val direction = if (timeDiff.isNegative) "behind" else "ahead" + logger.warn(s"Time synchronization issue detected: API timestamp $timestamp is ${absDiff.getSeconds}s $direction of server time $now (tolerance: ${config.timeTolerance.toSeconds}s)") } } catch { case ex: Exception => From 0256bbbff736bc62913baaf0004d08557ffc93d3 Mon Sep 17 00:00:00 2001 From: "vladimir.senchenko" Date: Sat, 23 Aug 2025 19:16:10 +0900 Subject: [PATCH 23/23] TTL explanation --- forex-mtl/README.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/forex-mtl/README.md b/forex-mtl/README.md index 163f2829..da54aab0 100644 --- a/forex-mtl/README.md +++ b/forex-mtl/README.md @@ -59,6 +59,43 @@ client.getBatch(pairsToFetch) **Result**: Comfortably within 1000 API calls/day limit +## TTL Strategy: Server Time vs API Timestamp + +### Design Decision + +The cache TTL is calculated from **server time** rather than the API's `time_stamp` field. This is a deliberate architectural choice with important implications: + +```scala +// Current implementation - server time based +val expiresAt = Instant.ofEpochMilli(nowMillis).plusMillis(ttl.toMillis) + +// Alternative approach - API timestamp based +val expiresAt = apiTimestamp.plusMillis(ttl.toMillis) +``` + +### Trade-off Analysis + +**Server Time Approach (Current):** +- ✅ **Predictable API usage**: Exactly 288 calls/day guaranteed +- ✅ **Quota safety**: Never exceeds One-Frame limits unexpectedly +- ✅ **Clock drift resilient**: Independent of API server time synchronization +- ✅ **Production stable**: System behavior is deterministic +- ❌ **Theoretical precision loss**: May serve data slightly older than 5 minutes in edge cases + +**API Timestamp Approach (Alternative):** +- ✅ **Stricter data freshness**: Never serves data older than 5 minutes from source +- ✅ **Theoretical correctness**: TTL reflects actual data age +- ❌ **Unpredictable API usage**: 288-1440 calls/day depending on API timestamp delays +- ❌ **Quota risk**: Could exhaust daily limit if API timestamps are stale +- ❌ **Clock sync dependency**: Breaks down with time synchronization issues + +### Why Server Time Was Chosen + +1. **Business Constraint Priority**: Meeting the 10,000 requests/day requirement with 1,000 API calls/day limit +2. **Production Reliability**: Predictable resource consumption over theoretical precision +3. **System Stability**: Resilience to external service timing variations +4. **Monitoring Capability**: Time sync warnings provide visibility into any precision trade-offs + ## Reliability & High Availability ### Multi-Node Deployment Strategy