From 9e92587db9f0b2667f7a9519c4ea0cc83074289a Mon Sep 17 00:00:00 2001 From: Krzysztof Nirski Date: Tue, 29 Nov 2022 16:57:51 +0100 Subject: [PATCH 1/5] Extend Cache interface with Option-specific use case --- .../main/scala/scalacache/AbstractCache.scala | 54 ++++++++++++------- .../src/main/scala/scalacache/Cache.scala | 30 +++++++++++ 2 files changed, 65 insertions(+), 19 deletions(-) diff --git a/modules/core/src/main/scala/scalacache/AbstractCache.scala b/modules/core/src/main/scala/scalacache/AbstractCache.scala index a54165e5..40458725 100644 --- a/modules/core/src/main/scala/scalacache/AbstractCache.scala +++ b/modules/core/src/main/scala/scalacache/AbstractCache.scala @@ -96,26 +96,42 @@ trait AbstractCache[F[_], K, V] extends Cache[F, K, V] with LoggingSupport[F, K] key: K )(ttl: Option[Duration] = None)(f: => V)(implicit flags: Flags): F[V] = cachingF(key)(ttl)(Sync[F].delay(f)) + final override def cachingOption( + key: K + )(ttl: Option[Duration] = None)(f: => Option[V])(implicit flags: Flags): F[Option[V]] = + cachingFOption(key)(ttl)(Sync[F].delay(f)) + override def cachingF( key: K - )(ttl: Option[Duration] = None)(f: F[V])(implicit flags: Flags): F[V] = { - checkFlagsAndGet(key) - .handleErrorWith { e => - logger - .ifWarnEnabled(logger.warn(s"Failed to read from cache. Key = $key", e)) - .as(None) - } - .flatMap { - case Some(valueFromCache) => F.pure(valueFromCache) - case None => - f.flatTap { calculatedValue => - checkFlagsAndPut(key, calculatedValue, ttl) - .handleErrorWith { e => - logger.ifWarnEnabled { - logger.warn(s"Failed to write to cache. Key = $key", e) - }.void - } - } + )(ttl: Option[Duration] = None)(f: F[V])(implicit flags: Flags): F[V] = + read(key).flatMap { + case Some(valueFromCache) => F.pure(valueFromCache) + case None => f.flatTap(write(key, _, ttl)) + } + + override def cachingFOption( + key: K + )(ttl: Option[Duration] = None)(f: F[Option[V]])(implicit flags: Flags): F[Option[V]] = + read(key).flatMap { + case Some(valueFromCache) => F.pure(Some(valueFromCache)) + case None => f.flatTap { + case Some(calculatedValue) => write(key, calculatedValue, ttl) + case None => logger.ifDebugEnabled(logger.debug("Calculated value was empty, not writing into cache")).void } - } + } + + private def read(key: K) = + checkFlagsAndGet(key).handleErrorWith { e => + logger + .ifWarnEnabled(logger.warn(s"Failed to read from cache. Key = $key", e)) + .as(None) + } + + private def write(key: K, value: V, ttl: Option[Duration] = None)(implicit flags: Flags) = + checkFlagsAndPut(key, value, ttl).handleErrorWith { e => + logger.ifWarnEnabled { + logger.warn(s"Failed to write to cache. Key = $key", e) + }.void + } + } diff --git a/modules/core/src/main/scala/scalacache/Cache.scala b/modules/core/src/main/scala/scalacache/Cache.scala index aa9a497e..94f45044 100644 --- a/modules/core/src/main/scala/scalacache/Cache.scala +++ b/modules/core/src/main/scala/scalacache/Cache.scala @@ -80,6 +80,21 @@ trait Cache[F[_], K, V] { */ def caching(key: K)(ttl: Option[Duration])(f: => V)(implicit flags: Flags): F[V] + /** Get a value from the cache if it exists. Otherwise compute it, insert it into the cache (only when it isn't None), and return it. + * + * @param key + * The cache key + * @param ttl + * The time-to-live to use when inserting into the cache. The cache entry will expire after this time has elapsed. + * @param f + * A block that computes the value + * @param flags + * Flags used to conditionally alter the behaviour of ScalaCache + * @return + * The value, either retrieved from the cache or computed + */ + def cachingOption(key: K)(ttl: Option[Duration])(f: => Option[V])(implicit flags: Flags): F[Option[V]] + /** Get a value from the cache if it exists. Otherwise compute it, insert it into the cache, and return it. * * @param key @@ -95,6 +110,21 @@ trait Cache[F[_], K, V] { */ def cachingF(key: K)(ttl: Option[Duration])(f: F[V])(implicit flags: Flags): F[V] + /** Get a value from the cache if it exists. Otherwise compute it, insert it into the cache (only when it isn't None), and return it. + * + * @param key + * The cache key + * @param ttl + * The time-to-live to use when inserting into the cache. The cache entry will expire after this time has elapsed. + * @param f + * A block that computes the value + * @param flags + * Flags used to conditionally alter the behaviour of ScalaCache + * @return + * The value, either retrieved from the cache or computed + */ + def cachingFOption(key: K)(ttl: Option[Duration])(f: F[Option[V]])(implicit flags: Flags): F[Option[V]] + /** You should call this when you have finished using this Cache. (e.g. when your application shuts down) * * It will take care of gracefully shutting down the underlying cache client. From d737d749d7020916d136c5b2adbc3e7cf0438429 Mon Sep 17 00:00:00 2001 From: Krzysztof Nirski Date: Tue, 29 Nov 2022 20:51:15 +0100 Subject: [PATCH 2/5] Add missing flags parameter --- modules/core/src/main/scala/scalacache/AbstractCache.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/core/src/main/scala/scalacache/AbstractCache.scala b/modules/core/src/main/scala/scalacache/AbstractCache.scala index 40458725..3cd97d58 100644 --- a/modules/core/src/main/scala/scalacache/AbstractCache.scala +++ b/modules/core/src/main/scala/scalacache/AbstractCache.scala @@ -120,14 +120,14 @@ trait AbstractCache[F[_], K, V] extends Cache[F, K, V] with LoggingSupport[F, K] } } - private def read(key: K) = + private def read(key: K)(implicit flags: Flags) = checkFlagsAndGet(key).handleErrorWith { e => logger .ifWarnEnabled(logger.warn(s"Failed to read from cache. Key = $key", e)) .as(None) } - private def write(key: K, value: V, ttl: Option[Duration] = None)(implicit flags: Flags) = + private def write(key: K, value: V, ttl: Option[Duration])(implicit flags: Flags) = checkFlagsAndPut(key, value, ttl).handleErrorWith { e => logger.ifWarnEnabled { logger.warn(s"Failed to write to cache. Key = $key", e) From 74fa1b70b98db95fc8d9b4b99b0e8b1054b78c3b Mon Sep 17 00:00:00 2001 From: Krzysztof Nirski Date: Tue, 29 Nov 2022 21:10:49 +0100 Subject: [PATCH 3/5] Add some tests --- .../src/main/scala/scalacache/Cache.scala | 4 +- .../scala/scalacache/AbstractCacheSpec.scala | 64 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/modules/core/src/main/scala/scalacache/Cache.scala b/modules/core/src/main/scala/scalacache/Cache.scala index 94f45044..cff331f9 100644 --- a/modules/core/src/main/scala/scalacache/Cache.scala +++ b/modules/core/src/main/scala/scalacache/Cache.scala @@ -87,7 +87,7 @@ trait Cache[F[_], K, V] { * @param ttl * The time-to-live to use when inserting into the cache. The cache entry will expire after this time has elapsed. * @param f - * A block that computes the value + * A block that computes the (optional) value * @param flags * Flags used to conditionally alter the behaviour of ScalaCache * @return @@ -117,7 +117,7 @@ trait Cache[F[_], K, V] { * @param ttl * The time-to-live to use when inserting into the cache. The cache entry will expire after this time has elapsed. * @param f - * A block that computes the value + * A block that computes the (optional) value * @param flags * Flags used to conditionally alter the behaviour of ScalaCache * @return diff --git a/modules/core/src/test/scala/scalacache/AbstractCacheSpec.scala b/modules/core/src/test/scala/scalacache/AbstractCacheSpec.scala index 75b7ca3e..60ce03f7 100644 --- a/modules/core/src/test/scala/scalacache/AbstractCacheSpec.scala +++ b/modules/core/src/test/scala/scalacache/AbstractCacheSpec.scala @@ -131,6 +131,36 @@ class AbstractCacheSpec extends AnyFlatSpec with Matchers with BeforeAndAfter { result should be("value from cache") } + it should "run the Option block and cache resulting Some value" in { + var called = false + val result = cache + .cachingOption("myKey")(None) { + called = true + Some("result of block") + } + .unsafeRunSync() + + cache.getCalledWithArgs(0) should be("myKey") + cache.putCalledWithArgs(0) should be(("myKey", "result of block", None)) + called should be(true) + result should be(Some("result of block")) + } + + it should "run the Option block and not cache resulting None value" in { + var called = false + val result = cache + .cachingOption("myKey")(None) { + called = true + None + } + .unsafeRunSync() + + cache.getCalledWithArgs(0) should be("myKey") + cache.putCalledWithArgs should be(empty) + called should be(true) + result should be(None) + } + behavior of "#cachingF (Scala Try mode)" it should "run the block and cache its result with no TTL if the value is not found in the cache" in { @@ -170,6 +200,40 @@ class AbstractCacheSpec extends AnyFlatSpec with Matchers with BeforeAndAfter { tResult should be("value from cache") } + it should "run the Option block and cache resulting Some value" in { + var called = false + val result = cache + .cachingFOption("myKey")(None) { + SyncIO { + called = true + Some("result of block") + } + } + .unsafeRunSync() + + cache.getCalledWithArgs(0) should be("myKey") + cache.putCalledWithArgs(0) should be(("myKey", "result of block", None)) + called should be(true) + result should be(Some("result of block")) + } + + it should "run the Option block and not cache resulting None value" in { + var called = false + val result = cache + .cachingFOption("myKey")(None) { + SyncIO { + called = true + None + } + } + .unsafeRunSync() + + cache.getCalledWithArgs(0) should be("myKey") + cache.putCalledWithArgs should be(empty) + called should be(true) + result should be(None) + } + behavior of "#caching (sync mode)" it should "run the block and cache its result if the value is not found in the cache" in { From f636ec7ce860a208bffc17a699665c936bd4e2a6 Mon Sep 17 00:00:00 2001 From: Krzysztof Nirski Date: Tue, 29 Nov 2022 21:25:23 +0100 Subject: [PATCH 4/5] Fix formatting --- .../core/src/main/scala/scalacache/AbstractCache.scala | 9 +++++---- modules/core/src/main/scala/scalacache/Cache.scala | 6 ++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/modules/core/src/main/scala/scalacache/AbstractCache.scala b/modules/core/src/main/scala/scalacache/AbstractCache.scala index 3cd97d58..75cf7aac 100644 --- a/modules/core/src/main/scala/scalacache/AbstractCache.scala +++ b/modules/core/src/main/scala/scalacache/AbstractCache.scala @@ -114,10 +114,11 @@ trait AbstractCache[F[_], K, V] extends Cache[F, K, V] with LoggingSupport[F, K] )(ttl: Option[Duration] = None)(f: F[Option[V]])(implicit flags: Flags): F[Option[V]] = read(key).flatMap { case Some(valueFromCache) => F.pure(Some(valueFromCache)) - case None => f.flatTap { - case Some(calculatedValue) => write(key, calculatedValue, ttl) - case None => logger.ifDebugEnabled(logger.debug("Calculated value was empty, not writing into cache")).void - } + case None => + f.flatTap { + case Some(calculatedValue) => write(key, calculatedValue, ttl) + case None => logger.ifDebugEnabled(logger.debug("Calculated value was empty, not writing into cache")).void + } } private def read(key: K)(implicit flags: Flags) = diff --git a/modules/core/src/main/scala/scalacache/Cache.scala b/modules/core/src/main/scala/scalacache/Cache.scala index cff331f9..72f9b16e 100644 --- a/modules/core/src/main/scala/scalacache/Cache.scala +++ b/modules/core/src/main/scala/scalacache/Cache.scala @@ -80,7 +80,8 @@ trait Cache[F[_], K, V] { */ def caching(key: K)(ttl: Option[Duration])(f: => V)(implicit flags: Flags): F[V] - /** Get a value from the cache if it exists. Otherwise compute it, insert it into the cache (only when it isn't None), and return it. + /** Get a value from the cache if it exists. Otherwise compute it, insert it into the cache (only when it isn't None), + * and return it. * * @param key * The cache key @@ -110,7 +111,8 @@ trait Cache[F[_], K, V] { */ def cachingF(key: K)(ttl: Option[Duration])(f: F[V])(implicit flags: Flags): F[V] - /** Get a value from the cache if it exists. Otherwise compute it, insert it into the cache (only when it isn't None), and return it. + /** Get a value from the cache if it exists. Otherwise compute it, insert it into the cache (only when it isn't None), + * and return it. * * @param key * The cache key From 7f37ff5c8fdb27f9a890b7b3f73f1325279488c7 Mon Sep 17 00:00:00 2001 From: Krzysztof Nirski Date: Tue, 29 Nov 2022 21:28:17 +0100 Subject: [PATCH 5/5] Add type annotations to new private defs in AbstractCache --- modules/core/src/main/scala/scalacache/AbstractCache.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/core/src/main/scala/scalacache/AbstractCache.scala b/modules/core/src/main/scala/scalacache/AbstractCache.scala index 75cf7aac..7aa39a57 100644 --- a/modules/core/src/main/scala/scalacache/AbstractCache.scala +++ b/modules/core/src/main/scala/scalacache/AbstractCache.scala @@ -121,14 +121,14 @@ trait AbstractCache[F[_], K, V] extends Cache[F, K, V] with LoggingSupport[F, K] } } - private def read(key: K)(implicit flags: Flags) = + private def read(key: K)(implicit flags: Flags): F[Option[V]] = checkFlagsAndGet(key).handleErrorWith { e => logger .ifWarnEnabled(logger.warn(s"Failed to read from cache. Key = $key", e)) .as(None) } - private def write(key: K, value: V, ttl: Option[Duration])(implicit flags: Flags) = + private def write(key: K, value: V, ttl: Option[Duration])(implicit flags: Flags): F[Unit] = checkFlagsAndPut(key, value, ttl).handleErrorWith { e => logger.ifWarnEnabled { logger.warn(s"Failed to write to cache. Key = $key", e)