Merge pull request #1635 from scala-steward-org/topic/clean-vcsapialg

Clean up and remove duplication in VCSApiAlg

Author: fthomas

modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala

@@ -66,12 +66,12 @@ final class NurtureAlg[F[_]](implicit
   def cloneAndSync(repo: Repo): F[(Repo, Branch)] =
     for {
       _ <- logger.info(s"Clone and synchronize ${repo.show}")
-      repoOut <- vcsApiAlg.createForkOrGetRepo(config, repo)
+      repoOut <- vcsApiAlg.createForkOrGetRepo(repo, config.doNotFork)
       _ <-
         gitAlg
           .cloneExists(repo)
           .ifM(F.unit, vcsRepoAlg.clone(repo, repoOut) >> vcsRepoAlg.syncFork(repo, repoOut))
-      defaultBranch <- vcsRepoAlg.defaultBranch(repoOut)
+      defaultBranch <- vcsApiAlg.parentOrRepo(repoOut, config.doNotFork).map(_.default_branch)
     } yield (repoOut.repo, defaultBranch)
 
   def updateDependencies(

modules/core/src/main/scala/org/scalasteward/core/repocache/RepoCacheAlg.scala

@@ -48,7 +48,7 @@ final class RepoCacheAlg[F[_]](implicit
         logger.info(s"Skipping due to previous error"),
         for {
           ((repoOut, branchOut), cachedSha1) <- (
-            vcsApiAlg.createForkOrGetRepoWithDefaultBranch(config, repo),
+            vcsApiAlg.createForkOrGetRepoWithDefaultBranch(repo, config.doNotFork),
             repoCacheRepository.findSha1(repo)
           ).parTupled
           latestSha1 = branchOut.commit.sha

modules/core/src/main/scala/org/scalasteward/core/vcs/VCSApiAlg.scala

@@ -16,11 +16,9 @@
 
 package org.scalasteward.core.vcs
 
-import cats.Monad
 import cats.syntax.all._
-import org.scalasteward.core.application.Config
 import org.scalasteward.core.git.Branch
-import org.scalasteward.core.util.MonadThrowable
+import org.scalasteward.core.util.{ApplicativeThrowable, MonadThrowable}
 import org.scalasteward.core.vcs.data._
 
 trait VCSApiAlg[F[_]] {
@@ -34,33 +32,27 @@ trait VCSApiAlg[F[_]] {
 
   def listPullRequests(repo: Repo, head: String, base: Branch): F[List[PullRequestOut]]
 
-  final def createForkOrGetRepo(config: Config, repo: Repo): F[RepoOut] =
-    if (config.doNotFork) getRepo(repo)
-    else createFork(repo)
+  final def createForkOrGetRepo(repo: Repo, doNotFork: Boolean): F[RepoOut] =
+    if (doNotFork) getRepo(repo) else createFork(repo)
 
-  final def createForkOrGetRepoWithDefaultBranch(config: Config, repo: Repo)(implicit
+  final def createForkOrGetRepoWithDefaultBranch(repo: Repo, doNotFork: Boolean)(implicit
       F: MonadThrowable[F]
   ): F[(RepoOut, BranchOut)] =
-    if (config.doNotFork) getRepoWithDefaultBranch(repo)
-    else createForkWithDefaultBranch(repo)
+    for {
+      forkOrRepo <- createForkOrGetRepo(repo, doNotFork)
+      defaultBranch <- getDefaultBranchOfParentOrRepo(forkOrRepo, doNotFork)
+    } yield (forkOrRepo, defaultBranch)
 
-  final def createForkWithDefaultBranch(repo: Repo)(implicit
+  final def getDefaultBranchOfParentOrRepo(repoOut: RepoOut, doNotFork: Boolean)(implicit
       F: MonadThrowable[F]
-  ): F[(RepoOut, BranchOut)] =
-    for {
-      fork <- createFork(repo)
-      parent <- fork.parentOrRaise[F]
-      branchOut <- getDefaultBranch(parent)
-    } yield (fork, branchOut)
+  ): F[BranchOut] =
+    parentOrRepo(repoOut, doNotFork).flatMap(getDefaultBranch)
 
-  final def getRepoWithDefaultBranch(repo: Repo)(implicit
-      F: Monad[F]
-  ): F[(RepoOut, BranchOut)] =
-    for {
-      repoOut <- getRepo(repo)
-      branchOut <- getDefaultBranch(repoOut)
-    } yield (repoOut, branchOut)
+  final def parentOrRepo(repoOut: RepoOut, doNotFork: Boolean)(implicit
+      F: ApplicativeThrowable[F]
+  ): F[RepoOut] =
+    if (doNotFork) F.pure(repoOut) else repoOut.parentOrRaise[F]
 
-  final def getDefaultBranch(repoOut: RepoOut): F[BranchOut] =
+  private def getDefaultBranch(repoOut: RepoOut): F[BranchOut] =
     getBranch(repoOut.repo, repoOut.default_branch)
 }

modules/core/src/main/scala/org/scalasteward/core/vcs/VCSRepoAlg.scala

@@ -20,16 +20,14 @@ import cats.syntax.all._
 import org.http4s.Uri
 import org.http4s.Uri.UserInfo
 import org.scalasteward.core.application.Config
-import org.scalasteward.core.git.{Branch, GitAlg}
+import org.scalasteward.core.git.GitAlg
 import org.scalasteward.core.util
 import org.scalasteward.core.util.MonadThrowable
 import org.scalasteward.core.vcs.data.{Repo, RepoOut}
 
 trait VCSRepoAlg[F[_]] {
   def clone(repo: Repo, repoOut: RepoOut): F[Unit]
 
-  def defaultBranch(repoOut: RepoOut): F[Branch]
-
   def syncFork(repo: Repo, repoOut: RepoOut): F[Unit]
 }
 
@@ -42,10 +40,6 @@ object VCSRepoAlg {
           _ <- gitAlg.setAuthor(repo, config.gitAuthor)
         } yield ()
 
-      override def defaultBranch(repoOut: RepoOut): F[Branch] =
-        if (config.doNotFork) repoOut.default_branch.pure[F]
-        else repoOut.parentOrRaise[F].map(_.default_branch)
-
       override def syncFork(repo: Repo, repoOut: RepoOut): F[Unit] =
         if (config.doNotFork) ().pure[F]
         else

modules/core/src/test/scala/org/scalasteward/core/bitbucket/http4s/Http4sBitbucketApiAlgTest.scala

@@ -161,29 +161,25 @@ class Http4sBitbucketApiAlgTest extends AnyFunSuite with Matchers {
   val pullRequest = PullRequestOut(prUrl, PullRequestState.Open, "scala-steward-pr")
 
   test("createForkOrGetRepo") {
-    val repoOut =
-      bitbucketApiAlg.createForkOrGetRepo(config, repo).unsafeRunSync()
+    val repoOut = bitbucketApiAlg.createForkOrGetRepo(repo, doNotFork = false).unsafeRunSync()
     repoOut shouldBe fork
   }
 
   test("createForkOrGetRepo without forking") {
-    val repoOut =
-      bitbucketApiAlg.createForkOrGetRepo(config.copy(doNotFork = true), repo).unsafeRunSync()
+    val repoOut = bitbucketApiAlg.createForkOrGetRepo(repo, doNotFork = true).unsafeRunSync()
     repoOut shouldBe parent
   }
 
   test("createForkOrGetRepoWithDefaultBranch") {
     val (repoOut, branchOut) =
-      bitbucketApiAlg.createForkOrGetRepoWithDefaultBranch(config, repo).unsafeRunSync()
+      bitbucketApiAlg.createForkOrGetRepoWithDefaultBranch(repo, doNotFork = false).unsafeRunSync()
     repoOut shouldBe fork
     branchOut shouldBe defaultBranch
   }
 
   test("createForkOrGetRepoWithDefaultBranch without forking") {
     val (repoOut, branchOut) =
-      bitbucketApiAlg
-        .createForkOrGetRepoWithDefaultBranch(config.copy(doNotFork = true), repo)
-        .unsafeRunSync()
+      bitbucketApiAlg.createForkOrGetRepoWithDefaultBranch(repo, doNotFork = true).unsafeRunSync()
     repoOut shouldBe parent
     branchOut shouldBe defaultBranch
   }
@@ -193,7 +189,7 @@ class Http4sBitbucketApiAlgTest extends AnyFunSuite with Matchers {
       "scala-steward-pr",
       "body",
       "master",
-      Branch("master")
+      master
     )
     val pr = bitbucketApiAlg.createPullRequest(repo, data).unsafeRunSync()
     pr shouldBe pullRequest

modules/core/src/test/scala/org/scalasteward/core/github/http4s/Http4sGitHubApiAlgTest.scala

@@ -85,29 +85,25 @@ class Http4sGitHubApiAlgTest extends AnyFunSuite with Matchers {
   )
 
   test("createForkOrGetRepo") {
-    val repoOut =
-      gitHubApiAlg.createForkOrGetRepo(config, repo).unsafeRunSync()
+    val repoOut = gitHubApiAlg.createForkOrGetRepo(repo, doNotFork = false).unsafeRunSync()
     repoOut shouldBe fork
   }
 
   test("createForkOrGetRepo without forking") {
-    val repoOut =
-      gitHubApiAlg.createForkOrGetRepo(config.copy(doNotFork = true), repo).unsafeRunSync()
+    val repoOut = gitHubApiAlg.createForkOrGetRepo(repo, doNotFork = true).unsafeRunSync()
     repoOut shouldBe parent
   }
 
   test("createForkOrGetRepoWithDefaultBranch") {
     val (repoOut, branchOut) =
-      gitHubApiAlg.createForkOrGetRepoWithDefaultBranch(config, repo).unsafeRunSync()
+      gitHubApiAlg.createForkOrGetRepoWithDefaultBranch(repo, doNotFork = false).unsafeRunSync()
     repoOut shouldBe fork
     branchOut shouldBe defaultBranch
   }
 
   test("createForkOrGetRepoWithDefaultBranch without forking") {
     val (repoOut, branchOut) =
-      gitHubApiAlg
-        .createForkOrGetRepoWithDefaultBranch(config.copy(doNotFork = true), repo)
-        .unsafeRunSync()
+      gitHubApiAlg.createForkOrGetRepoWithDefaultBranch(repo, doNotFork = true).unsafeRunSync()
     repoOut shouldBe parent
     branchOut shouldBe defaultBranch
   }