Repository: scala-steward-org/scala-steward
Pull request #3560 (2025-01-21 15:42:13)

Rewrite `RepoCacheAlg.checkCache` as for-comprehension
This is a small refactoring that rewrites `checkCache` as for-comprehension which I think is a little bit easier to read. It also removes the second parameter from `skipIfFailedRecently` which was a needless complication.


Patch:
diff --git a/modules/core/src/main/scala/org/scalasteward/core/repocache/RefreshErrorAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/repocache/RefreshErrorAlg.scala
index 36638d10..6062edf5 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/repocache/RefreshErrorAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/repocache/RefreshErrorAlg.scala
@@ -35,12 +35,12 @@ final class RefreshErrorAlg[F[_]](
     dateTimeAlg: DateTimeAlg[F],
     F: MonadThrow[F]
 ) {
-  def skipIfFailedRecently[A](repo: Repo)(fa: F[A]): F[A] =
+  def throwIfFailedRecently(repo: Repo): F[Unit] =
     failedRecently(repo).flatMap {
-      case None => fa
+      case None => F.unit
       case Some(fd) =>
         val msg = s"Skipping due to previous error for ${showDuration(fd)}"
-        F.raiseError[A](new Throwable(msg) with NoStackTrace)
+        F.raiseError(new Throwable(msg) with NoStackTrace)
     }
 
   def persistError[A](repo: Repo)(fa: F[A]): F[A] =
diff --git a/modules/core/src/main/scala/org/scalasteward/core/repocache/RepoCacheAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/repocache/RepoCacheAlg.scala
index 7e055dd6..cf760bce 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/repocache/RepoCacheAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/repocache/RepoCacheAlg.scala
@@ -40,19 +40,17 @@ final class RepoCacheAlg[F[_]](config: Config)(implicit
     F: MonadThrow[F]
 ) {
   def checkCache(repo: Repo): F[(RepoData, RepoOut)] =
-    logger.info(s"Check cache of ${repo.show}") >>
-      refreshErrorAlg.skipIfFailedRecently(repo) {
-        (
-          forgeApiAlg.createForkOrGetRepoWithBranch(repo, config.forgeCfg.doNotFork),
-          repoCacheRepository.findCache(repo)
-        ).parTupled.flatMap { case ((repoOut, branchOut), maybeCache) =>
-          val latestSha1 = branchOut.commit.sha
-          maybeCache
-            .filter(_.sha1 === latestSha1)
-            .fold(cloneAndRefreshCache(repo, repoOut))(supplementCache(repo, _).pure[F])
-            .map(data => (data, repoOut))
-        }
-      }
+    for {
+      _ <- logger.info(s"Check cache of ${repo.show}")
+      _ <- refreshErrorAlg.throwIfFailedRecently(repo)
+      getRepoAndBranch = forgeApiAlg.createForkOrGetRepoWithBranch(repo, config.forgeCfg.doNotFork)
+      tuple <- (getRepoAndBranch, repoCacheRepository.findCache(repo)).parTupled
+      ((repoOut, branchOut), maybeCache) = tuple
+      latestSha1 = branchOut.commit.sha
+      data <- maybeCache
+        .filter(_.sha1 === latestSha1)
+        .fold(cloneAndRefreshCache(repo, repoOut))(supplementCache(repo, _).pure[F])
+    } yield (data, repoOut)
 
   private def supplementCache(repo: Repo, cache: RepoCache): RepoData =
     RepoData(repo, cache, repoConfigAlg.mergeWithGlobal(cache.maybeRepoConfig))


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/repocache/RefreshErrorAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/repocache/RefreshErrorAlgTest.scala
index 3dca6d94..39eb0d37 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/repocache/RefreshErrorAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/repocache/RefreshErrorAlgTest.scala
@@ -7,23 +7,26 @@ import org.scalasteward.core.mock.MockContext.context.refreshErrorAlg
 import org.scalasteward.core.mock.{MockEff, MockEffOps, MockState}
 
 class RefreshErrorAlgTest extends CatsEffectSuite {
-  test("skipIfFailedRecently: not failed") {
+  test("throwIfFailedRecently: not failed") {
     val repo = Repo("refresh-error", "test-1")
-    val expected = 42
-    refreshErrorAlg.skipIfFailedRecently(repo)(MockEff.pure(expected)).runA(MockState.empty).map {
-      obtained => assertEquals(obtained, expected)
+    val p = refreshErrorAlg.throwIfFailedRecently(repo)
+
+    val expected = "()"
+    p.runA(MockState.empty).attempt.map { obtained =>
+      val obtainedStr = obtained.fold(_.getMessage, _.toString).take(expected.length)
+      assertEquals(obtainedStr, expected)
     }
   }
 
-  test("skipIfFailedRecently: failed") {
+  test("throwIfFailedRecently: failed") {
     val repo = Repo("refresh-error", "test-2")
     val p = refreshErrorAlg.persistError(repo)(MockEff.raiseError(new Throwable())).attempt >>
-      refreshErrorAlg.skipIfFailedRecently(repo)(MockEff.unit)
+      refreshErrorAlg.throwIfFailedRecently(repo)
 
     val expected = "Skipping due to previous error"
     p.runA(MockState.empty).attempt.map { obtained =>
-      val message = obtained.fold(_.getMessage, _.toString).take(expected.length)
-      assertEquals(message, expected)
+      val obtainedStr = obtained.fold(_.getMessage, _.toString).take(expected.length)
+      assertEquals(obtainedStr, expected)
     }
   }
 }