From 99e254ad34aa2b8e02e3d66a160bb9bd7d5d7eef Mon Sep 17 00:00:00 2001 From: Gabriel Volpe Date: Mon, 4 May 2020 15:57:38 +0200 Subject: [PATCH] Documenting errors + removing HList reverse --- .../dev/profunktor/redis4cats/hlist.scala | 12 ----- .../dev/profunktor/redis4cats/HListSpec.scala | 6 --- .../redis4cats/interpreter/Redis.scala | 4 +- .../profunktor/redis4cats/transactions.scala | 13 +++--- .../redis4cats/RedisTransactionsDemo.scala | 4 +- .../profunktor/redis4cats/TestScenarios.scala | 19 +++++--- site/docs/transactions.md | 46 +++++++++++++------ 7 files changed, 56 insertions(+), 48 deletions(-) diff --git a/modules/core/src/main/scala/dev/profunktor/redis4cats/hlist.scala b/modules/core/src/main/scala/dev/profunktor/redis4cats/hlist.scala index 2dae94b8..3890459d 100644 --- a/modules/core/src/main/scala/dev/profunktor/redis4cats/hlist.scala +++ b/modules/core/src/main/scala/dev/profunktor/redis4cats/hlist.scala @@ -16,8 +16,6 @@ package dev.profunktor.redis4cats -import scala.annotation.tailrec - /** * An heterogeneous list, mainly used to operate on transactions. * @@ -30,16 +28,6 @@ object hlist { sealed trait HList { def ::[A](a: A): HCons[A, this.type] = HCons(a, this) - - def reverse: HList = { - @tailrec - def go(res: HList, ys: HList): HList = - ys match { - case HNil => res - case HCons(h, t) => go(h :: res, t) - } - go(HNil, this) - } } final case class HCons[+H, +Tail <: HList](head: H, tail: Tail) extends HList diff --git a/modules/core/src/test/scala/dev/profunktor/redis4cats/HListSpec.scala b/modules/core/src/test/scala/dev/profunktor/redis4cats/HListSpec.scala index a665210f..4f91669f 100644 --- a/modules/core/src/test/scala/dev/profunktor/redis4cats/HListSpec.scala +++ b/modules/core/src/test/scala/dev/profunktor/redis4cats/HListSpec.scala @@ -47,10 +47,4 @@ class HListSpec extends AnyFunSuite with Matchers { assert(n2.isInstanceOf[Int]) } - test("Reversing HList") { - val hl = "foo" :: 123 :: "bar" :: true :: 2.4 :: 'a' :: HNil - assert(hl.reverse === 'a' :: 2.4 :: true :: "bar" :: 123 :: "foo" :: HNil) - assert(hl.reverse.reverse == hl) - } - } diff --git a/modules/effects/src/main/scala/dev/profunktor/redis4cats/interpreter/Redis.scala b/modules/effects/src/main/scala/dev/profunktor/redis4cats/interpreter/Redis.scala index 35b8086d..3bfcec1c 100644 --- a/modules/effects/src/main/scala/dev/profunktor/redis4cats/interpreter/Redis.scala +++ b/modules/effects/src/main/scala/dev/profunktor/redis4cats/interpreter/Redis.scala @@ -26,6 +26,7 @@ import dev.profunktor.redis4cats.domain._ import dev.profunktor.redis4cats.effect.{ JRFuture, Log, RedisBlocker } import dev.profunktor.redis4cats.effect.JRFuture._ import dev.profunktor.redis4cats.effects._ +import dev.profunktor.redis4cats.transactions.TransactionDiscarded import io.lettuce.core.{ GeoArgs, GeoRadiusStoreArgs, @@ -43,9 +44,6 @@ import io.lettuce.core.cluster.api.async.RedisClusterAsyncCommands import io.lettuce.core.cluster.api.sync.{ RedisClusterCommands => RedisClusterSyncCommands } import java.util.concurrent.TimeUnit import scala.concurrent.duration._ -import scala.util.control.NoStackTrace - -case object TransactionDiscarded extends NoStackTrace object Redis { diff --git a/modules/effects/src/main/scala/dev/profunktor/redis4cats/transactions.scala b/modules/effects/src/main/scala/dev/profunktor/redis4cats/transactions.scala index 71fee0df..9e87a324 100644 --- a/modules/effects/src/main/scala/dev/profunktor/redis4cats/transactions.scala +++ b/modules/effects/src/main/scala/dev/profunktor/redis4cats/transactions.scala @@ -28,7 +28,9 @@ import scala.util.control.NoStackTrace object transactions { - case object TransactionAborted extends NoStackTrace + sealed trait TransactionError extends NoStackTrace + case object TransactionAborted extends TransactionError + case object TransactionDiscarded extends TransactionError case class RedisTransaction[F[_]: Concurrent: Log: Timer, K, V]( cmd: RedisCommands[F, K, V] @@ -67,18 +69,17 @@ object transactions { } def cancelFibers(fibs: HList, err: Throwable = TransactionAborted): F[Unit] = - joinOrCancel(fibs.reverse, HNil)(false).void >> promise.complete(err.asLeft) + joinOrCancel(fibs, HNil)(false).void >> promise.complete(err.asLeft) val tx = Resource.makeCase(cmd.multi >> runner(commands, HNil)) { case ((fibs: HList), ExitCase.Completed) => for { _ <- F.info("Transaction completed") - _ <- cmd.exec.handleErrorWith(e => cancelFibers(fibs.reverse, e) >> F.raiseError(e)) - tr <- joinOrCancel(fibs.reverse, HNil)(true) + _ <- cmd.exec.handleErrorWith(e => cancelFibers(fibs, e) >> F.raiseError(e)) + tr <- joinOrCancel(fibs, HNil)(true) // Casting here is fine since we have a `Witness` that proves this true - res = tr.asInstanceOf[HList].reverse.asInstanceOf[w.R] - _ <- promise.complete(res.asRight) + _ <- promise.complete(tr.asInstanceOf[w.R].asRight) } yield () case ((fibs: HList), ExitCase.Error(e)) => F.error(s"Transaction failed: ${e.getMessage}") >> diff --git a/modules/examples/src/main/scala/dev/profunktor/redis4cats/RedisTransactionsDemo.scala b/modules/examples/src/main/scala/dev/profunktor/redis4cats/RedisTransactionsDemo.scala index 7a626bb6..2822dbba 100644 --- a/modules/examples/src/main/scala/dev/profunktor/redis4cats/RedisTransactionsDemo.scala +++ b/modules/examples/src/main/scala/dev/profunktor/redis4cats/RedisTransactionsDemo.scala @@ -63,12 +63,14 @@ object RedisTransactionsDemo extends LoggerIOApp { val prog = tx.exec(operations) .flatMap { - case _ ~: _ ~: res1 ~: _ ~: _ ~: res2 => + case _ ~: _ ~: res1 ~: _ ~: _ ~: res2 ~: HNil => putStrLn(s"res1: $res1, res2: $res2") } .onError { case TransactionAborted => putStrLn("[Error] - Transaction Aborted") + case TransactionDiscarded => + putStrLn("[Error] - Transaction Discarded") case _: TimeoutException => putStrLn("[Error] - Timeout") } diff --git a/modules/tests/src/test/scala/dev/profunktor/redis4cats/TestScenarios.scala b/modules/tests/src/test/scala/dev/profunktor/redis4cats/TestScenarios.scala index 920d43ed..7ba82431 100644 --- a/modules/tests/src/test/scala/dev/profunktor/redis4cats/TestScenarios.scala +++ b/modules/tests/src/test/scala/dev/profunktor/redis4cats/TestScenarios.scala @@ -256,13 +256,18 @@ trait TestScenarios { val tx = RedisTransaction(cmd) - for { - _ <- tx.exec(cmd.set(key1, "foo") :: cmd.set(key2, "bar") :: HNil) - x <- cmd.get(key1) - _ <- IO(assert(x.contains("foo"))) - y <- cmd.get(key2) - _ <- IO(assert(y.contains("bar"))) - } yield () + val operations = + cmd.set(key1, "osx") :: cmd.set(key2, "windows") :: cmd.get(key1) :: cmd.sIsMember("foo", "bar") :: + cmd.set(key1, "nix") :: cmd.set(key2, "linux") :: cmd.get(key1) :: HNil + + tx.exec(operations).map { + case _ ~: _ ~: res1 ~: res2 ~: _ ~: _ ~: res3 ~: HNil => + assert(res1.contains("osx")) + assert(res2 === false) + assert(res3.contains("nix")) + case tr => + assert(false, s"Unexpected result: $tr") + } } diff --git a/site/docs/transactions.md b/site/docs/transactions.md index de6d395b..906a85c0 100644 --- a/site/docs/transactions.md +++ b/site/docs/transactions.md @@ -7,17 +7,20 @@ position: 3 # Transactions -Redis supports [transactions](https://redis.io/topics/transactions) via the `MULTI`, `EXEC` and `DISCARD` commands. `redis4cats` provides a `RedisTransaction` utility that models a transaction as a resource via the primitive `bracketCase`. +Redis supports [transactions](https://redis.io/topics/transactions) via the `MULTI`, `EXEC` and `DISCARD` commands. `redis4cats` provides a `RedisTransaction` utility that models a transaction as a `Resource`. -- `acquire`: begin transaction -- `use`: send transactional commands -- `release`: either commit on success or rollback on failure / cancellation. +Note that every command has to be forked (`.start`) because the commands need to be sent to the server asynchronously and no response will be received until either an `EXEC` or a `DISCARD` command is sent. Both forking and sending the final command is handled by `RedisTransaction`. + +These are internals, though. All you need to care about is what commands you want to run as part of a transaction +and handle the possible errors and retry logic. ### Working with transactions The most common way is to create a `RedisTransaction` once by passing the commands API as a parameter and invoke the `exec` function every time you want to run the given commands as part of a new transaction. -Note that every command has to be forked (`.start`) because the commands need to be sent to the server asynchronously and no response will be received until either an `EXEC` or a `DISCARD` command is sent. Both forking and sending the final command is handled by `RedisTransaction`. Every command has to be atomic and independent of previous results, so it wouldn't make sense to chain commands using `flatMap` (though, it would technically work). +Every command has to be atomic and independent of previous Redis results, so it is not recommended to chain commands using `flatMap`. + +Below you can find a first example of transactional commands. ```scala mdoc:invisible import cats.effect.{IO, Resource} @@ -66,18 +69,35 @@ commandsApi.use { cmd => // RedisCommands[IO, String, String] // the result type is inferred as well // Unit :: Option[String] :: Unit :: HNil - val setters = - tx.exec(commands).flatMap { - case _ ~: res1 ~: _ ~: HNil => - putStrLn(s"Key1 result: $res1") - } - - getters >> setters >> getters.void + val prog = + tx.exec(commands) + .flatMap { + case _ ~: res1 ~: _ ~: HNil => + putStrLn(s"Key1 result: $res1") + } + .onError { + case TransactionAborted => + putStrLn("[Error] - Transaction Aborted") + case TransactionDiscarded => + putStrLn("[Error] - Transaction Discarded") + case _: TimeoutException => + putStrLn("[Error] - Timeout") + } + + getters >> prog >> getters.void } ``` It should be exclusively used to run Redis commands as part of a transaction, not any other computations. Fail to do so, may result in unexpected behavior. +Transactional commands may be discarded if something went wrong in between. The possible errors you may get are: + +- `TransactionDiscarded`: The `EXEC` command failed and the transactional commands were discarded. +- `TransactionAborted`: The `DISCARD` command was triggered due to cancellation or other failure within the transaction. +- `TimeoutException`: The transaction timed out due to some unknown error. + +### How NOT to use transactions + For example, the following transaction will result in a dead-lock: ```scala mdoc:silent @@ -98,7 +118,7 @@ commandsApi.use { cmd => You should never pass a transactional command: `MULTI`, `EXEC` or `DISCARD`. -The following example will result in a successful transaction; the error will be swallowed: +The following example will result in a successful transaction on Redis. Yet, the operation will end up raising the error passed as a command. ```scala mdoc:silent commandsApi.use { cmd =>