diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/SelfCheckAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/application/SelfCheckAlg.scala index 826d6f5a10..462cdad32e 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/application/SelfCheckAlg.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/application/SelfCheckAlg.scala @@ -81,9 +81,9 @@ final class SelfCheckAlg[F[_]](config: Config)(implicit private def checkUrlChecker: F[Unit] = config.urlCheckerTestUrls.traverse_ { url => - urlChecker.exists(url).flatMap { urlExists => + urlChecker.validate(url).flatMap { res => val msg = s"Self check of UrlChecker failed: checking that $url exists failed" - F.whenA(!urlExists)(logger.warn(msg)) + F.whenA(res.notExists)(logger.warn(msg)) } } } diff --git a/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala index 9ab3c02d08..dc58b5d4f1 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala @@ -206,7 +206,7 @@ final class NurtureAlg[F[_]](config: ForgeCfg)(implicit .traverse { case (_, dependency) => coursierAlg .getMetadata(dependency, resolvers) - .flatMap(_.filterUrls(urlChecker.exists)) + .flatMap(_.filterUrls(uri => urlChecker.validate(uri).map(_.exists))) .tupleLeft(dependency) } .map(_.toMap) diff --git a/modules/core/src/main/scala/org/scalasteward/core/nurture/UpdateInfoUrl.scala b/modules/core/src/main/scala/org/scalasteward/core/nurture/UpdateInfoUrl.scala index fae0fedbef..b7c9aa38f2 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/nurture/UpdateInfoUrl.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/nurture/UpdateInfoUrl.scala @@ -22,13 +22,23 @@ import org.http4s.Uri /** A URL of a resource that provides additional information for an update. */ sealed trait UpdateInfoUrl { def url: Uri + + def withUrl(url: Uri): UpdateInfoUrl } object UpdateInfoUrl { - final case class CustomChangelog(url: Uri) extends UpdateInfoUrl - final case class CustomReleaseNotes(url: Uri) extends UpdateInfoUrl - final case class GitHubReleaseNotes(url: Uri) extends UpdateInfoUrl - final case class VersionDiff(url: Uri) extends UpdateInfoUrl + final case class CustomChangelog(url: Uri) extends UpdateInfoUrl { + override def withUrl(url: Uri): UpdateInfoUrl = CustomChangelog(url) + } + final case class CustomReleaseNotes(url: Uri) extends UpdateInfoUrl { + override def withUrl(url: Uri): UpdateInfoUrl = CustomReleaseNotes(url) + } + final case class GitHubReleaseNotes(url: Uri) extends UpdateInfoUrl { + override def withUrl(url: Uri): UpdateInfoUrl = GitHubReleaseNotes(url) + } + final case class VersionDiff(url: Uri) extends UpdateInfoUrl { + override def withUrl(url: Uri): UpdateInfoUrl = VersionDiff(url) + } implicit val updateInfoUrlOrder: Order[UpdateInfoUrl] = Order.by { diff --git a/modules/core/src/main/scala/org/scalasteward/core/nurture/UpdateInfoUrlFinder.scala b/modules/core/src/main/scala/org/scalasteward/core/nurture/UpdateInfoUrlFinder.scala index 5341a6aeec..8077051dd1 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/nurture/UpdateInfoUrlFinder.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/nurture/UpdateInfoUrlFinder.scala @@ -16,7 +16,7 @@ package org.scalasteward.core.nurture -import cats.Monad +import cats.{Functor, Monad, Parallel} import cats.syntax.all.* import org.http4s.Uri import org.scalasteward.core.application.Config.ForgeCfg @@ -31,22 +31,37 @@ import org.scalasteward.core.util.UrlChecker final class UpdateInfoUrlFinder[F[_]](implicit config: ForgeCfg, urlChecker: UrlChecker[F], + parallel: Parallel[F], F: Monad[F] ) { def findUpdateInfoUrls( dependency: DependencyMetadata, versionUpdate: Version.Update ): F[List[UpdateInfoUrl]] = { - val updateInfoUrls: List[UpdateInfoUrl] = - dependency.releaseNotesUrl.toList.map(CustomReleaseNotes.apply) ++ - dependency.forgeRepo.toSeq.flatMap(forgeRepo => - possibleUpdateInfoUrls(forgeRepo, versionUpdate) - ) + val updateInfoUrls: F[List[UpdateInfoUrl]] = + dependency.forgeRepo.toList + .flatTraverse(forgeRepo => possibleUpdateInfoUrls(urlChecker)(forgeRepo, versionUpdate)) + .map(dependency.releaseNotesUrl.toList.map(CustomReleaseNotes.apply) ++ _) - updateInfoUrls - .sorted(UpdateInfoUrl.updateInfoUrlOrder.toOrdering) - .distinctBy(_.url) - .filterA(updateInfoUrl => urlChecker.exists(updateInfoUrl.url)) + for { + urls <- updateInfoUrls + .map( + _.sorted(UpdateInfoUrl.updateInfoUrlOrder.toOrdering) + .distinctBy(_.url) + ) + checkedUrls <- + urls.parFlatTraverse(updateInfoUrl => + urlChecker + .validate(updateInfoUrl.url) + .map( + _.fold( + exists = List(updateInfoUrl), + notExists = List.empty, + redirectTo = u => List(updateInfoUrl.withUrl(u)) + ) + ) + ) + } yield checkedUrls } } @@ -81,17 +96,31 @@ object UpdateInfoUrlFinder { repoForge.diffUrlFor(tagName(update.currentVersion), tagName(update.nextVersion)) ) - private[nurture] def possibleUpdateInfoUrls( + private[nurture] def possibleUpdateInfoUrls[F[_]: Functor](urlChecker: UrlChecker[F])( forgeRepo: ForgeRepo, update: Version.Update - ): List[UpdateInfoUrl] = { + ): F[List[UpdateInfoUrl]] = { def customUrls(wrap: Uri => UpdateInfoUrl, fileNames: List[String]): List[UpdateInfoUrl] = fileNames.map(f => wrap(forgeRepo.fileUrlFor(f))) - gitHubReleaseNotesFor(forgeRepo, update.nextVersion) ++ - customUrls(CustomReleaseNotes.apply, possibleReleaseNotesFilenames) ++ - customUrls(CustomChangelog.apply, possibleChangelogFilenames) ++ - possibleVersionDiffs(forgeRepo, update) + for { + forgeRepoMaybe <- urlChecker + .validate(forgeRepo.repoUrl) + .map( + _.fold( + exists = forgeRepo.some, + notExists = none, + redirectTo = u => forgeRepo.copy(repoUrl = u).some + ) + ) + urls = + forgeRepoMaybe.toList.flatMap(forgeRepoChecked => + gitHubReleaseNotesFor(forgeRepoChecked, update.nextVersion) ++ + customUrls(CustomReleaseNotes.apply, possibleReleaseNotesFilenames) ++ + customUrls(CustomChangelog.apply, possibleChangelogFilenames) ++ + possibleVersionDiffs(forgeRepoChecked, update) + ) + } yield urls } private def gitHubReleaseNotesFor( diff --git a/modules/core/src/main/scala/org/scalasteward/core/util/UrlChecker.scala b/modules/core/src/main/scala/org/scalasteward/core/util/UrlChecker.scala index 92f2a7d536..06edd1a713 100644 --- a/modules/core/src/main/scala/org/scalasteward/core/util/UrlChecker.scala +++ b/modules/core/src/main/scala/org/scalasteward/core/util/UrlChecker.scala @@ -20,6 +20,7 @@ import cats.effect.Sync import cats.syntax.all.* import com.github.benmanes.caffeine.cache.Caffeine import org.http4s.client.Client +import org.http4s.headers.Location import org.http4s.{Method, Request, Status, Uri} import org.scalasteward.core.application.Config import org.typelevel.log4cats.Logger @@ -27,21 +28,43 @@ import scalacache.Entry import scalacache.caffeine.CaffeineCache trait UrlChecker[F[_]] { - def exists(url: Uri): F[Boolean] + def validate(url: Uri): F[UrlChecker.UrlValidationResult] } final case class UrlCheckerClient[F[_]](client: Client[F]) extends AnyVal object UrlChecker { + + sealed abstract class UrlValidationResult extends Product with Serializable { + def exists: Boolean = + fold(exists = true, notExists = false, redirectTo = _ => false) + + def notExists: Boolean = + fold(exists = false, notExists = true, redirectTo = _ => false) + + def fold[A](exists: => A, notExists: => A, redirectTo: Uri => A): A = + this match { + case UrlValidationResult.Exists => exists + case UrlValidationResult.NotExists => notExists + case UrlValidationResult.RedirectTo(url) => redirectTo(url) + } + } + + object UrlValidationResult { + case object Exists extends UrlValidationResult + case object NotExists extends UrlValidationResult + final case class RedirectTo(url: Uri) extends UrlValidationResult + } + private def buildCache[F[_]](config: Config)(implicit F: Sync[F] - ): F[CaffeineCache[F, String, Status]] = + ): F[CaffeineCache[F, String, UrlValidationResult]] = F.delay { val cache = Caffeine .newBuilder() .maximumSize(16384L) .expireAfterWrite(config.cacheTtl.length, config.cacheTtl.unit) - .build[String, Entry[Status]]() + .build[String, Entry[UrlValidationResult]]() CaffeineCache(cache) } @@ -52,16 +75,45 @@ object UrlChecker { ): F[UrlChecker[F]] = buildCache(config).map { statusCache => new UrlChecker[F] { - override def exists(url: Uri): F[Boolean] = - status(url).map(_ === Status.Ok).handleErrorWith { throwable => - logger.debug(throwable)(s"Failed to check if $url exists").as(false) - } + override def validate(url: Uri): F[UrlValidationResult] = + check(url).handleErrorWith(th => + logger + .debug(th)(s"Failed to check if $url exists") + .as(UrlValidationResult.NotExists) + ) + + /** While checking for the [[Uri]]s presence, we perform up to 3 recursive lookups when + * receiving a `MovedPermanently` response. + */ + private def check(url: Uri): F[UrlValidationResult] = { + def lookup(url: Uri, maxDepth: Int): F[Option[Uri]] = + Option(maxDepth).filter(_ > 0).flatTraverse { depth => + val req = Request[F](method = Method.HEAD, uri = url) + + modify(req).flatMap(r => + urlCheckerClient.client.run(r).use { + case resp if resp.status === Status.Ok => + F.pure(url.some) + + case resp if resp.status === Status.MovedPermanently => + resp.headers + .get[Location] + .flatTraverse(location => lookup(location.uri, depth - 1)) + + case _ => + F.pure(none) + } + ) + } - private def status(url: Uri): F[Status] = statusCache.cachingF(url.renderString)(None) { - val req = Request[F](method = Method.HEAD, uri = url) - modify(req).flatMap(urlCheckerClient.client.status) + lookup(url, maxDepth = 3).map { + case Some(u) if u === url => UrlValidationResult.Exists + case Some(u) => UrlValidationResult.RedirectTo(u) + case None => UrlValidationResult.NotExists + } } + } } } } diff --git a/modules/core/src/test/scala/org/scalasteward/core/nurture/UpdateInfoUrlFinderTest.scala b/modules/core/src/test/scala/org/scalasteward/core/nurture/UpdateInfoUrlFinderTest.scala index 8765032eb3..e78217ed4a 100644 --- a/modules/core/src/test/scala/org/scalasteward/core/nurture/UpdateInfoUrlFinderTest.scala +++ b/modules/core/src/test/scala/org/scalasteward/core/nurture/UpdateInfoUrlFinderTest.scala @@ -2,8 +2,9 @@ package org.scalasteward.core.nurture import cats.syntax.all.* import munit.CatsEffectSuite -import org.http4s.HttpApp +import org.http4s.{Headers, HttpApp, Response, Status} import org.http4s.dsl.Http4sDsl +import org.http4s.headers.Location import org.http4s.syntax.literals.* import org.scalasteward.core.application.Config.ForgeCfg import org.scalasteward.core.coursier.DependencyMetadata @@ -18,12 +19,24 @@ import org.scalasteward.core.nurture.UpdateInfoUrlFinder.* class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] { private val httpApp = HttpApp[MockEff] { + case HEAD -> Root / "foo" / "foo" => Ok() + case HEAD -> Root / "foo" / "bar" => Ok() + case HEAD -> Root / "foo" / "bar2" => Ok() + case req @ HEAD -> Root / "foo" / "bar3" => + val redirectUrl = req.uri.withPath(Path.empty / "foo" / "bar3-redirected") + + Response[MockEff]( + status = Status.MovedPermanently, + headers = Headers(Location(redirectUrl)) + ).pure[MockEff] + case HEAD -> Root / "foo" / "bar3-redirected" => Ok() case HEAD -> Root / "foo" / "bar" / "README.md" => Ok() case HEAD -> Root / "foo" / "bar" / "compare" / "v0.1.0...v0.2.0" => Ok() case HEAD -> Root / "foo" / "bar1" / "blob" / "master" / "RELEASES.md" => Ok() case HEAD -> Root / "foo" / "buz" / "compare" / "v0.1.0...v0.2.0" => PermanentRedirect() case HEAD -> Root / "foo" / "bar2" / "releases" / "tag" / "v0.2.0" => Ok() - case _ => NotFound() + case HEAD -> Root / "foo" / "bar3-redirected" / "releases" / "tag" / "v0.2.0" => Ok() + case _ => NotFound() } private val authApp = GitHubAuth.api(List(Repository("foo/bar"))) private val state = MockState.empty.copy(clientResponses = authApp <+> httpApp) @@ -68,6 +81,18 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] { assertIO(obtained, expected) } + test( + "findUpdateInfoUrls: GitHubReleaseNotes and CustomReleaseNotes with the same URL after redirection" + ) { + val metadata = DependencyMetadata.empty.copy( + scmUrl = uri"https://github.com/foo/bar3".some, + releaseNotesUrl = uri"https://github.com/foo/bar3-redirected/releases/tag/v0.2.0".some + ) + val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state) + val expected = List(GitHubReleaseNotes(metadata.releaseNotesUrl.get)) + assertIO(obtained, expected) + } + test("findUpdateInfoUrls: releaseNotesUrl is in possibleReleaseRelatedUrls") { val metadata = DependencyMetadata.empty.copy( scmUrl = uri"https://github.com/foo/bar1".some, @@ -92,8 +117,8 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] { addLabels = false ) private val onPremUpdateUrlFinder = new UpdateInfoUrlFinder[MockEff] - private val gitHubFooBarRepo = ForgeRepo(GitHub, uri"https://github.com/foo/bar/") - private val bitbucketFooBarRepo = ForgeRepo(Bitbucket, uri"https://bitbucket.org/foo/bar/") + private val gitHubFooBarRepo = ForgeRepo(GitHub, uri"https://github.com/foo/bar") + private val bitbucketFooBarRepo = ForgeRepo(Bitbucket, uri"https://bitbucket.org/foo/bar") private val gitLabFooBarRepo = ForgeRepo(GitLab, uri"https://gitlab.com/foo/bar") test("findUpdateInfoUrls: on-prem, repoUrl not found") { @@ -185,10 +210,10 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] { } test("possibleUpdateInfoUrls: github.com") { - val obtained = possibleUpdateInfoUrls( + val obtained = possibleUpdateInfoUrls(urlChecker)( gitHubFooBarRepo, versionUpdate - ).map(_.url.renderString) + ).map(_.map(_.url.renderString)).runA(state) val expected = List( s"https://github.com/foo/bar/releases/tag/v$v2", s"https://github.com/foo/bar/releases/tag/$v2", @@ -221,14 +246,14 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] { s"https://github.com/foo/bar/compare/$v1...$v2", s"https://github.com/foo/bar/compare/release-$v1...release-$v2" ) - assertEquals(obtained, expected) + assertIO(obtained, expected) } test("possibleUpdateInfoUrls: gitlab.com") { - val obtained = possibleUpdateInfoUrls( + val obtained = possibleUpdateInfoUrls(urlChecker)( gitLabFooBarRepo, versionUpdate - ).map(_.url.renderString) + ).map(_.map(_.url.renderString)).runA(state) val expected = possibleReleaseNotesFilenames.map(name => s"https://gitlab.com/foo/bar/blob/master/$name") ++ possibleChangelogFilenames.map(name => s"https://gitlab.com/foo/bar/blob/master/$name") ++ @@ -237,14 +262,14 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] { s"https://gitlab.com/foo/bar/compare/$v1...$v2", s"https://gitlab.com/foo/bar/compare/release-$v1...release-$v2" ) - assertEquals(obtained, expected) + assertIO(obtained, expected) } test("possibleUpdateInfoUrls: on-prem gitlab") { - val obtained = possibleUpdateInfoUrls( + val obtained = possibleUpdateInfoUrls(urlChecker)( ForgeRepo(GitLab, uri"https://gitlab.on-prem.net/foo/bar"), versionUpdate - ).map(_.url.renderString) + ).map(_.map(_.url.renderString)).runA(state) val expected = possibleReleaseNotesFilenames.map(name => s"https://gitlab.on-prem.net/foo/bar/blob/master/$name" ) ++ possibleChangelogFilenames.map(name => @@ -254,14 +279,14 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] { s"https://gitlab.on-prem.net/foo/bar/compare/$v1...$v2", s"https://gitlab.on-prem.net/foo/bar/compare/release-$v1...release-$v2" ) - assertEquals(obtained, expected) + assertIO(obtained, expected) } test("possibleUpdateInfoUrls: bitbucket.org") { - val obtained = possibleUpdateInfoUrls( + val obtained = possibleUpdateInfoUrls(urlChecker)( bitbucketFooBarRepo, versionUpdate - ).map(_.url.renderString) + ).map(_.map(_.url.renderString)).runA(state) val expected = possibleReleaseNotesFilenames.map(name => s"https://bitbucket.org/foo/bar/src/master/$name" @@ -272,14 +297,16 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] { s"https://bitbucket.org/foo/bar/compare/$v2..$v1#diff", s"https://bitbucket.org/foo/bar/compare/release-$v2..release-$v1#diff" ) - assertEquals(obtained, expected) + assertIO(obtained, expected) } test("possibleUpdateInfoUrls: on-prem Bitbucket Server") { val repoUrl = uri"https://bitbucket-server.on-prem.com" / "foo" / "bar" val obtained = - possibleUpdateInfoUrls(ForgeRepo(BitbucketServer, repoUrl), versionUpdate).map(_.url) + possibleUpdateInfoUrls(urlChecker)(ForgeRepo(BitbucketServer, repoUrl), versionUpdate) + .map(_.map(_.url)) + .runA(state) val expected = repoUrl / "browse" / "ReleaseNotes.md" - assert(clue(obtained).contains(expected)) + assertIOBoolean(obtained.map(_.contains(expected))) } } diff --git a/modules/core/src/test/scala/org/scalasteward/core/util/UrlCheckerTest.scala b/modules/core/src/test/scala/org/scalasteward/core/util/UrlCheckerTest.scala new file mode 100644 index 0000000000..3d0d673adb --- /dev/null +++ b/modules/core/src/test/scala/org/scalasteward/core/util/UrlCheckerTest.scala @@ -0,0 +1,81 @@ +package org.scalasteward.core.util + +import cats.syntax.all.* +import munit.CatsEffectSuite +import org.http4s.{Headers, HttpApp, Response, Status, Uri} +import org.http4s.dsl.Http4sDsl +import org.http4s.headers.Location +import org.http4s.syntax.all.* +import org.scalasteward.core.mock.* +import org.scalasteward.core.mock.MockContext.context.* +import org.scalasteward.core.mock.{MockEff, MockState} +import org.scalasteward.core.util.UrlChecker.UrlValidationResult + +class UrlCheckerTest extends CatsEffectSuite with Http4sDsl[MockEff] { + private val baseUrl = uri"https://github.com/scala-steward-org" + private val finishedRedirectUrl = baseUrl / "finished-redirect" + private val infiniteRedirectUrl = baseUrl / "infinite-redirect" + private val secondRedirectUrl = baseUrl / "second-redirect" + + private def movedPermanentlyResponse(uri: Uri): MockEff[Response[MockEff]] = + Response[MockEff]( + status = Status.MovedPermanently, + headers = Headers(Location(uri)) + ).pure[MockEff] + + private val state = + MockState.empty.copy(clientResponses = GitHubAuth.api(List.empty) <+> HttpApp { + case HEAD -> Root / "scala-steward-org" => Ok() + case HEAD -> Root / "scala-steward-org" / "wrong-uri" => NotFound() + case HEAD -> Root / "scala-steward-org" / "single-redirect" => + movedPermanentlyResponse(finishedRedirectUrl) + case HEAD -> Root / "scala-steward-org" / "first-redirect" => + movedPermanentlyResponse(secondRedirectUrl) + case HEAD -> Root / "scala-steward-org" / "second-redirect" => + movedPermanentlyResponse(finishedRedirectUrl) + case HEAD -> Root / "scala-steward-org" / "finished-redirect" => Ok() + case HEAD -> Root / "scala-steward-org" / "infinite-redirect" => + movedPermanentlyResponse(infiniteRedirectUrl) + case _ => + ServiceUnavailable() + }) + + test("An URL exists") { + val url = baseUrl + val check = urlChecker.validate(url).runA(state) + + assertIOBoolean(check.map(_.exists)) + } + + test("An URL does not exist") { + val url = baseUrl / "wrong-uri" + val check = urlChecker.validate(url).runA(state) + + assertIOBoolean(check.map(_.notExists)) + } + + test("An URL redirects to another existing URL") { + val httpUrl = uri"https://github.com/scala-steward-org/single-redirect" + val check = urlChecker.validate(httpUrl).runA(state) + val anticipatedResult = UrlValidationResult.RedirectTo(finishedRedirectUrl) + + assertIO(check, anticipatedResult) + } + + test("An URL redirects to another redirecting URL (two redirects)") { + val httpUrl = uri"https://github.com/scala-steward-org/first-redirect" + val check = urlChecker.validate(httpUrl).runA(state) + val anticipatedResult = UrlValidationResult.RedirectTo(finishedRedirectUrl) + + assertIO(check, anticipatedResult) + } + + // basically, we prohibit the infinite loop of traversing redirect URLs + test("An URL redirects to another redirecting URL (more than two redirects)") { + val httpUrl = uri"https://github.com/scala-steward-org/infinite-redirect" + val check = urlChecker.validate(httpUrl).runA(state) + val anticipatedResult = UrlValidationResult.NotExists + + assertIO(check, anticipatedResult) + } +}