Monix removal #713

ghost · 2021-10-28T09:25:43Z

No description provided.

adamw · 2021-10-28T10:14:25Z

backend/src/main/scala/com/softwaremill/bootzooka/email/EmailService.scala

+    idGenerator
+      .nextId[Email]()
+      .map(id => emailModel.insert(Email(id, data)))
+      .unsafeRunSync()


we should ideally run .unsafeRunSync in one place (in main). That's why there was the .to[] construct, which allowed embedding an IO/Task into a ConnectionIO. For why this matters see: https://blog.softwaremill.com/from-transactional-to-type-safe-reasonable-transactions-a5019906245e

During my research of topics I found out that doing it is such way, using .to method is probably impossible.
Currently suggested way to use WeakAsync.liftK[IO, ConnectionIO] but it unfortunately uses unsafeToFuture and we want to preserve context - fiber local, so we cannot use any unsafe
All of this is caused mostly by updates in Cats Effect type type-class hierarchy.

adamw · 2021-10-28T10:15:15Z

backend/src/main/scala/com/softwaremill/bootzooka/http/EndpointsToRoutes.scala

+    val serverOptions: Http4sServerOptions[IO, IO] = Http4sServerOptions
+      .customInterceptors[IO, IO]
+      .decodeFailureHandler(decodeFailureHandler)
+      .serverLog(Log.defaultServerLog[IO])


do we need to specify this? I think it's the default

adamw · 2021-10-28T10:15:48Z

backend/src/main/scala/com/softwaremill/bootzooka/http/EndpointsToRoutes.scala

    val openapi = OpenAPIDocsInterpreter()
      .serverEndpointsToOpenAPI(es.toList, "Bootzooka", "1.0")
      .servers(List(Server(s"$apiContextPath", None)))
    val yaml = openapi.toYaml
-    new SwaggerHttp4s(yaml).routes[Task]
+    val value: List[ServerEndpoint[_, _, _, Any, IO]] = SwaggerUI[IO](yaml)


this needs a better name ;)

adamw · 2021-10-28T10:16:26Z

backend/src/main/scala/com/softwaremill/bootzooka/http/HttpApi.scala

          .bindHttp(config.port, config.host)
          .withHttpApp(app)
          .resource
      }
  }

-  private val staticFileBlocker = Blocker.liftExecutionContext(ExecutionContext.fromExecutorService(Executors.newFixedThreadPool(4)))
+  private def indexResponse(r: Request[IO]): IO[Response[IO]] =
+    StaticFile.fromResource(s"/webapp/index.html", Some(r)).getOrElseF(IO.pure(Response.notFound[IO]))


maybe we can use a tapir endpoint for that?

adamw · 2021-10-28T10:17:16Z

backend/src/main/scala/com/softwaremill/bootzooka/passwordreset/PasswordResetService.scala

+      val passwordResetCode: PasswordResetCode = PasswordResetCode(id, user.id, validUntil)
+      passwordResetCodeModel.insert(passwordResetCode).map(_ => passwordResetCode)
+    }
+    passwordResetCodeIO.unsafeRunSync()


again, we should use .to

adamw · 2021-10-28T10:17:31Z

backend/src/main/scala/com/softwaremill/bootzooka/security/ApiKeyService.scala

+      logger.debug(s"Creating a new api key for user $userId, valid until: $validUntil")
+      apiKeyModel.insert(apiKey).map(_ => apiKey)
+    }
+    apiKeyIO.unsafeRunSync()


here as well

adamw · 2021-10-28T10:18:04Z

backend/src/main/scala/com/softwaremill/bootzooka/security/Auth.scala

@@ -37,21 +35,21 @@ class Auth[T](
      case None =>
        logger.debug(s"Auth failed for: ${authTokenOps.tokenName} $id")
        // random sleep to prevent timing attacks
-        Timer[Task].sleep(random.nextInt(1000).millis) >> Task.raiseError(Fail.Unauthorized("Unauthorized"))
+        Temporal[IO].sleep(random.nextInt(1000).millis) >> IO.raiseError(Fail.Unauthorized("Unauthorized"))


IO.sleep simply?

From what I read in cats 3 migration guide this is the preferred way to replace Timer.

But in another place you're using IO.sleep

But there I am replacing Task.sleep not Timer.sleep

Is it any different?

adamw · 2021-10-28T10:18:20Z

backend/src/main/scala/com/softwaremill/bootzooka/user/UserService.scala

+          apiKey <- apiKeyService.create(user.id, config.defaultApiKeyValid)
+        } yield apiKey
+      }
+      apiKeyIO.unsafeRunSync()


adamw · 2021-10-28T10:20:52Z

backend/src/test/scala/com/softwaremill/bootzooka/passwordreset/PasswordResetApiTest.scala


-    override lazy val baseSttpBackend: SttpBackend[Task, Any] = SttpBackendStub(TaskMonadAsyncError)
+    override lazy val baseSttpBackend: SttpBackend[IO, Any] = SttpBackendStub(CatsMonadError.monadError)


maybe you can use AsyncHttpClientFs2Backend.stub isntead? and remove the custom CatsMonadError impl

adamw · 2021-10-28T10:22:27Z

docs/stack.md

@@ -26,7 +26,7 @@ React is one of the most popular JavaScript framework right now, developed and s

 ### Why Monix?

-Monix is a concurrency toolkit, thanks to which working with side-effects is concurrency-safe and type-safe. The basic datatype, `Task`, is a lazily evaluated description of side effects. It offers a number of combinators to sequence, handle errors, allocate resources and run effects asynchronously.
+Monix is a concurrency toolkit, thanks to which working with side-effects is concurrency-safe and type-safe. The basic datatype, `IO`, is a lazily evaluated description of side effects. It offers a number of combinators to sequence, handle errors, allocate resources and run effects asynchronously.


:D I think this needs more attention

adamw · 2021-10-28T10:24:01Z

backend/src/main/scala/com/softwaremill/bootzooka/util/CorrelationIdDecorator.scala

-
-/** Based on [[https://olegpy.com/better-logging-monix-1/]]. Makes the current correlation id available for logback loggers.
-  */
-class MonixMDCAdapter extends LogbackMDCAdapter {


hm what about correlation ids - do they work in the new setup?

this logback adapter needs to stay :)

- Renamed variable for swagger routes - Removed default log addition

adamw · 2021-12-20T13:28:57Z

backend/src/main/scala/com/softwaremill/bootzooka/email/EmailService.scala

@@ -40,19 +41,19 @@ class EmailService(emailModel: EmailModel, idGenerator: IdGenerator, emailSender
      emailModel.count().transact(xa).map(_.toDouble).map(Metrics.emailQueueGauge.set)
    }

-    Task.parZip2(sendProcess, monitoringProcess)
+    sendProcess.flatMap(r => monitoringProcess.flatMap(d => IO(r, d)))


that will work differenlty. Previously, we were runing send & monitor in parallel. Now, they would run sequentially - causing monitoring never to run, as send never finishes. So we need to change this back to parallel

adamw · 2021-12-20T13:29:31Z

backend/src/main/scala/com/softwaremill/bootzooka/http/EndpointsToRoutes.scala

    val openapi = OpenAPIDocsInterpreter()
      .serverEndpointsToOpenAPI(es.toList, "Bootzooka", "1.0")
      .servers(List(Server(s"$apiContextPath", None)))
-    val yaml = openapi.toYaml
-    new SwaggerHttp4s(yaml).routes[Task]
+    val swaggerRoutes: List[ServerEndpoint[_, _, _, Any, IO]] = SwaggerUI[IO](openapi.toYaml)


tapir needs updating to 0.19-final :)

also maybe we can use the bundle: https://tapir.softwaremill.com/en/latest/docs/openapi.html#using-swagger

adamw · 2021-12-20T14:51:29Z

There's quite a lot of compiler warnings - maybe we could fix them. Also, needs a rebase.

# Conflicts: # build.sbt

adamw · 2021-12-23T09:35:37Z

backend/src/main/scala/com/softwaremill/bootzooka/Main.scala

 import sttp.client3.SttpBackend

 object Main extends StrictLogging {
  def main(args: Array[String]): Unit = {
-    CorrelationId.init()


we need to migrate correlation id support as well:) probably implementing https://github.com/softwaremill/monix-correlation-id using fiber-locals inside bootzooka for now, we might extract it as a separate repository later

adamw · 2021-12-23T09:36:56Z

backend/src/main/scala/com/softwaremill/bootzooka/email/EmailService.scala

    for {
      emails <- emailModel.find(config.batchSize).transact(xa)
      _ = if (emails.nonEmpty) logger.info(s"Sending ${emails.size} emails")
-      _ <- Task.sequence(emails.map(_.data).map(emailSender.apply))
+      _ <- IO(emails.map(_.data).map(emailSender.apply))


what happened to .sequence? This won't work, we're constructing an IO[IO[Unit]] and ignoring the inner result

adamw · 2021-12-23T09:45:25Z

backend/src/main/scala/com/softwaremill/bootzooka/email/sender/DummyEmailSender.scala

@@ -14,7 +14,7 @@ object DummyEmailSender extends EmailSender with StrictLogging {
    sentEmails.clear()
  }

-  override def apply(email: EmailData): Task[Unit] = Task {
+  override def apply(email: EmailData): IO[Unit] = IO.pure {


This definitely isn't pure! Fixing

adamw · 2021-12-23T09:47:09Z

backend/src/test/scala/com/softwaremill/bootzooka/test/BaseTest.scala

 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers

 trait BaseTest extends AnyFlatSpec with Matchers {
-  CorrelationId.init()


same here, this should stay in the migrated form

adamw · 2021-12-23T09:47:51Z

backend/src/main/scala/com/softwaremill/bootzooka/user/UserService.scala

-    } yield ()
+    } yield {
+      val confirmationEmail = emailTemplates.passwordChangeNotification(user.login)
+      emailScheduler(EmailData(user.emailLowerCased, confirmationEmail))


This effect needs to be sequenced with the others - I'm surprised this worked (did tests pass)? Anyway, fixing

adamw · 2021-12-23T09:48:10Z

backend/src/main/scala/com/softwaremill/bootzooka/user/UserService.scala

      val confirmationEmail = emailTemplates.profileDetailsChangeNotification(user.login)
      emailScheduler(EmailData(user.emailLowerCased, confirmationEmail))
    }

-    doChange() flatMap { anyUpdate =>
+    doChange().map { anyUpdate =>


Here we need a .flatMap - to sequence the effects, and not ignore them. Fixing

adamw · 2022-01-03T11:01:37Z

Thanks! :) I added one follow-up: #769

bartekzylinski added 5 commits October 27, 2021 13:23

Removed depdencies to monix and updated others to works with cats3

3527ce9

Removed monix code and replaced it with cats effects

ebca2e7

Removed MonixMDCAdapter

e763fae

Fixed compilations errors in tests

26eac29

Fixed failing tests

fd1ca92

ghost requested a review from adamw October 28, 2021 09:25

ghost self-assigned this Oct 28, 2021

adamw reviewed Oct 28, 2021

View reviewed changes

bartekzylinski added 6 commits October 28, 2021 13:01

Minor review fixes

201710d

- Renamed variable for swagger routes - Removed default log addition

Repalce SttpBackendStub with AsyncHttpClient based stub

4d9e4f5

Replace http4s staticFile with tapirEndpoint

fd0ea12

Repalce Temporal.sleep with IO.sleep

d7f84c9

Removed monix info from docs

eab42d0

Removed unused EC from DB

2252c3b

Pask423 self-assigned this Nov 4, 2021

Attempt to remove runUsafe

e26c83a

adamw reviewed Dec 20, 2021

View reviewed changes

Simplify effect management

dfd9500

Pask added 9 commits December 22, 2021 21:38

Merge branch 'master' into monix-removal

3ea36cc

# Conflicts: # build.sbt

Optimized imports

9986956

Update dependencies and refactor swagger

eb69223

Removed deprecated CORS config

6346dcc

Fixed compiler warnings

d3c3198

Changed execution order of sendProcess and monitoringProcess

3d8d0f9

Optimized imports in tests

f929f85

Fixed failing tests

50d34e6

Removed unused variable

a559d43

adamw reviewed Dec 23, 2021

View reviewed changes

adamw and others added 8 commits December 23, 2021 11:08

Fix profile details changed notification

be5b82b

Fix change password notification

061c24b

Properly setup dummy email sender

5b0cb8f

Fix compile

1de5728

Add missing servers

3bcd3c5

Fix tests

ebae028

Fix swagger path

8f87937

Restored corelationIds

9e2d3ea

adamw merged commit cf67f69 into master Jan 3, 2022

Pask423 deleted the monix-removal branch January 28, 2022 17:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monix removal #713

Monix removal #713

ghost commented Oct 28, 2021

adamw Oct 28, 2021

Pask423 Nov 4, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

ghost Oct 28, 2021

adamw Oct 28, 2021

ghost Oct 28, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

adamw Oct 28, 2021

adamw Dec 23, 2021

adamw Dec 20, 2021

adamw Dec 20, 2021

adamw Dec 20, 2021

adamw commented Dec 20, 2021

adamw Dec 23, 2021

adamw Dec 23, 2021

adamw Dec 23, 2021

adamw Dec 23, 2021

adamw Dec 23, 2021

adamw Dec 23, 2021

adamw commented Jan 3, 2022


		override lazy val baseSttpBackend: SttpBackend[Task, Any] = SttpBackendStub(TaskMonadAsyncError)
		override lazy val baseSttpBackend: SttpBackend[IO, Any] = SttpBackendStub(CatsMonadError.monadError)

Monix removal #713

Monix removal #713

Conversation

ghost commented Oct 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamw commented Dec 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamw commented Jan 3, 2022