Skip to content

Commit

Permalink
Refactor: introduce ForgeRepo as a parameter object
Browse files Browse the repository at this point in the history
`forgeType` & `repoUrl` are two parameters that are passed together in
several method calls, suggesting that they belong together in a single
parameter object - a `ForgeRepo` where the type of forge indicated by
the repoUrl has been established::

https://refactoring.com/catalog/introduceParameterObject.html
https://refactoring.guru/introduce-parameter-object
https://docondev.com/blog/2020/6/2/refactoring-introduce-parameter-object

Those two pieces of information - `forgeType` & `repoUrl` - are all that
are required to generate urls for diffs or file display, meaning that
actually `ForgeRepo` can be more than a simple parameter object, and
provide convenient methods to generate those urls.

New tests for generated file & diff urls
========================================

This change also adds `ForgeRepoTest`, which gives clear examples of what
diff and file urls should be for different forge types. The urls included
in the test correspond to real live repos as much as possible, so it's
possible to click through on them and see that they do indeed correspond
to real live urls genuinely served by those different forge hosts.
  • Loading branch information
rtyley committed Aug 19, 2023
1 parent b42866b commit d52d95e
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package org.scalasteward.core.coursier
import cats.Monad
import cats.syntax.all._
import org.http4s.Uri
import org.scalasteward.core.application.Config.ForgeCfg
import org.scalasteward.core.forge.ForgeRepo
import org.scalasteward.core.util.uri

final case class DependencyMetadata(
Expand Down Expand Up @@ -46,6 +48,9 @@ final case class DependencyMetadata(
val urls = scmUrl.toList ++ homePage.toList
urls.find(_.scheme.exists(uri.httpSchemes)).orElse(urls.headOption)
}

def forgeRepo(implicit config: ForgeCfg): Option[ForgeRepo] =
repoUrl.flatMap(ForgeRepo.fromRepoUrl)
}

object DependencyMetadata {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2018-2023 Scala Steward contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.scalasteward.core.forge

import org.http4s.Uri
import org.scalasteward.core.application.Config.ForgeCfg

/** ForgeRepo encapsulates two concepts that are commonly considered together - the URI of a repo,
* and the 'type' of forge that url represents. Given a URI, once we know it's a GitHub or GitLab
* forge, etc, then we can know how to construct many of the urls for common resources existing at
* that repo host- for instance, the url to view a particular file, or to diff two commits.
*/
case class ForgeRepo(forgeType: ForgeType, repoUrl: Uri) {
def diffUrlFor(from: String, to: String): Uri = forgeType.diffs.forDiff(from, to)(repoUrl)

def fileUrlFor(fileName: String): Uri = forgeType.files.forFile(fileName)(repoUrl)
}

object ForgeRepo {
def fromRepoUrl(repoUrl: Uri)(implicit config: ForgeCfg): Option[ForgeRepo] = for {
repoForgeType <- ForgeType.fromRepoUrl(repoUrl)
} yield ForgeRepo(repoForgeType, repoUrl)
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import org.http4s.Uri
import org.scalasteward.core.application.Config.ForgeCfg
import org.scalasteward.core.coursier.DependencyMetadata
import org.scalasteward.core.data.Version
import org.scalasteward.core.forge.ForgeType
import org.scalasteward.core.forge.ForgeRepo
import org.scalasteward.core.forge.ForgeType._
import org.scalasteward.core.nurture.UpdateInfoUrl._
import org.scalasteward.core.nurture.UpdateInfoUrlFinder.possibleUpdateInfoUrls
Expand All @@ -38,13 +38,11 @@ final class UpdateInfoUrlFinder[F[_]](implicit
currentVersion: Version,
nextVersion: Version
): F[List[UpdateInfoUrl]] = {
val updateInfoUrls =
val updateInfoUrls: List[UpdateInfoUrl] =
metadata.releaseNotesUrl.toList.map(CustomReleaseNotes.apply) ++
metadata.repoUrl.toList.flatMap { repoUrl =>
ForgeType.fromRepoUrl(repoUrl).toSeq.flatMap { forgeType =>
possibleUpdateInfoUrls(forgeType, repoUrl, currentVersion, nextVersion)
}
}
metadata.forgeRepo.toSeq.flatMap(forgeRepo =>
possibleUpdateInfoUrls(forgeRepo, currentVersion, nextVersion)
)

updateInfoUrls
.sorted(UpdateInfoUrl.updateInfoUrlOrder.toOrdering)
Expand Down Expand Up @@ -76,40 +74,39 @@ object UpdateInfoUrlFinder {
}

private[nurture] def possibleVersionDiffs(
forgeType: ForgeType,
repoUrl: Uri,
repoForge: ForgeRepo,
currentVersion: Version,
nextVersion: Version
): List[VersionDiff] = for {
tagName <- Version.tagNames
} yield VersionDiff(
forgeType.diffs.forDiff(tagName(currentVersion), tagName(nextVersion))(repoUrl)
repoForge.diffUrlFor(tagName(currentVersion), tagName(nextVersion))
)

private[nurture] def possibleUpdateInfoUrls(
forgeType: ForgeType,
repoUrl: Uri,
forgeRepo: ForgeRepo,
currentVersion: Version,
nextVersion: Version
): List[UpdateInfoUrl] = {
def customUrls(wrap: Uri => UpdateInfoUrl, fileNames: List[String]): List[UpdateInfoUrl] =
fileNames.map(f => wrap(forgeType.files.forFile(f)(repoUrl)))
fileNames.map(f => wrap(forgeRepo.fileUrlFor(f)))

gitHubReleaseNotesFor(forgeType, repoUrl, nextVersion) ++
gitHubReleaseNotesFor(forgeRepo, nextVersion) ++
customUrls(CustomReleaseNotes, possibleReleaseNotesFilenames) ++
customUrls(CustomChangelog, possibleChangelogFilenames) ++
possibleVersionDiffs(forgeType, repoUrl, currentVersion, nextVersion)
possibleVersionDiffs(forgeRepo, currentVersion, nextVersion)
}

private def gitHubReleaseNotesFor(
forgeType: ForgeType,
repoUrl: Uri,
forgeRepo: ForgeRepo,
version: Version
): List[UpdateInfoUrl] =
forgeType match {
forgeRepo.forgeType match {
case GitHub =>
Version.tagNames
.map(tagName => GitHubReleaseNotes(repoUrl / "releases" / "tag" / tagName(version)))
.map(tagName =>
GitHubReleaseNotes(forgeRepo.repoUrl / "releases" / "tag" / tagName(version))
)
case _ => Nil
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package org.scalasteward.core.forge

import munit.FunSuite
import org.http4s.Uri
import org.http4s.implicits._
import org.scalasteward.core.forge.ForgeType._

/** As much as possible, uris in this test suite should aim to be real, clickable, uris that
* actually go to real pages, allowing developers working against this test suite to verify that
* the url patterns constructed here actually _are_ valid, and match what forges are currently
* using in the real world.
*/
class ForgeRepoTest extends FunSuite {

def check(
repo: ForgeRepo,
expectedReadme: Uri,
diffTags: (String, String),
expectedDiff: Uri
): Unit = {
assertEquals(repo.fileUrlFor("README.md"), expectedReadme)
assertEquals(repo.diffUrlFor(diffTags._1, diffTags._2), expectedDiff)
}

test("GitHub url patterns") {
check(
ForgeRepo(GitHub, uri"https://github.com/scala-steward-org/scala-steward-action"),
uri"https://github.com/scala-steward-org/scala-steward-action/blob/master/README.md",
"v2.55.0" -> "v2.56.0",
uri"https://github.com/scala-steward-org/scala-steward-action/compare/v2.55.0...v2.56.0"
)
}

test("GitLab url patterns") {
check(
ForgeRepo(GitLab, uri"https://gitlab.com/gitlab-org/gitlab"),
uri"https://gitlab.com/gitlab-org/gitlab/blob/master/README.md",
"v15.11.8-ee" -> "v15.11.9-ee",
uri"https://gitlab.com/gitlab-org/gitlab/compare/v15.11.8-ee...v15.11.9-ee"
)
}

test("Gitea url patterns") {
check(
ForgeRepo(Gitea, uri"https://gitea.com/lunny/levelqueue"),
uri"https://gitea.com/lunny/levelqueue/src/branch/master/README.md",
"v0.1.0" -> "v0.2.0",
uri"https://gitea.com/lunny/levelqueue/compare/v0.1.0...v0.2.0"
)
}

test("Azure url patterns") {
check(
ForgeRepo(
AzureRepos,
uri"https://dev.azure.com/rtyley/scala-steward-testing/_git/scala-steward-testing"
),
uri"https://dev.azure.com/rtyley/scala-steward-testing/_git/scala-steward-testing?path=README.md",
"v1.0.0" -> "v1.0.1",
uri"https://dev.azure.com/rtyley/scala-steward-testing/_git/scala-steward-testing/branchCompare?baseVersion=GTv1.0.0&targetVersion=GTv1.0.1"
)
}

test("BitBucket url patterns") {
check(
ForgeRepo(Bitbucket, uri"https://bitbucket.org/rtyley/scala-steward-test-repo"),
uri"https://bitbucket.org/rtyley/scala-steward-test-repo/src/master/README.md",
"v1.0.0" -> "v1.0.1",
uri"https://bitbucket.org/rtyley/scala-steward-test-repo/compare/v1.0.1..v1.0.0#diff"
)
}

test("BitBucket Server url patterns") {
check(
ForgeRepo(BitbucketServer, uri"https://bitbucket-server.on-prem.com/foo/bar"),
uri"https://bitbucket-server.on-prem.com/foo/bar/browse/README.md",
"v1.0.0" -> "v1.0.1",
uri"https://bitbucket-server.on-prem.com/foo/bar/compare/v1.0.1..v1.0.0#diff"
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import org.http4s.implicits._
import org.scalasteward.core.application.Config.ForgeCfg
import org.scalasteward.core.coursier.DependencyMetadata
import org.scalasteward.core.data.Version
import org.scalasteward.core.forge.ForgeType
import org.scalasteward.core.forge.ForgeType._
import org.scalasteward.core.forge.{ForgeRepo, ForgeType}
import org.scalasteward.core.mock.MockContext.context._
import org.scalasteward.core.mock.{MockEff, MockState}
import org.scalasteward.core.nurture.UpdateInfoUrl._
Expand Down Expand Up @@ -88,6 +88,9 @@ 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 gitLabFooBarRepo = ForgeRepo(GitLab, uri"https://gitlab.com/foo/bar")

test("findUpdateInfoUrls: on-prem, repoUrl not found") {
val metadata =
Expand Down Expand Up @@ -116,7 +119,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
val onPremForgeUrl = uri"https://github.onprem.io/"

assertEquals(
possibleVersionDiffs(GitHub, uri"https://github.com/foo/bar", v1, v2)
possibleVersionDiffs(gitHubFooBarRepo, v1, v2)
.map(_.url.renderString),
List(
s"https://github.com/foo/bar/compare/v$v1...v$v2",
Expand All @@ -127,7 +130,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {

// should canonicalize (drop last slash)
assertEquals(
possibleVersionDiffs(GitHub, uri"https://github.com/foo/bar/", v1, v2)
possibleVersionDiffs(gitHubFooBarRepo, v1, v2)
.map(_.url.renderString),
List(
s"https://github.com/foo/bar/compare/v$v1...v$v2",
Expand All @@ -137,7 +140,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
)

assertEquals(
possibleVersionDiffs(GitHub, uri"https://gitlab.com/foo/bar", v1, v2)
possibleVersionDiffs(gitLabFooBarRepo, v1, v2)
.map(_.url.renderString),
List(
s"https://gitlab.com/foo/bar/compare/v$v1...v$v2",
Expand All @@ -147,7 +150,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
)

assertEquals(
possibleVersionDiffs(Bitbucket, uri"https://bitbucket.org/foo/bar", v1, v2)
possibleVersionDiffs(bitbucketFooBarRepo, v1, v2)
.map(_.url.renderString),
List(
s"https://bitbucket.org/foo/bar/compare/v$v2..v$v1#diff",
Expand All @@ -157,7 +160,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
)

assertEquals(
possibleVersionDiffs(GitHub, onPremForgeUrl.addPath("foo/bar"), v1, v2)
possibleVersionDiffs(ForgeRepo(GitHub, onPremForgeUrl.addPath("foo/bar")), v1, v2)
.map(_.url.renderString),
List(
s"${onPremForgeUrl}foo/bar/compare/v$v1...v$v2",
Expand All @@ -167,7 +170,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
)

assertEquals(
possibleVersionDiffs(AzureRepos, onPremForgeUrl.addPath("foo/bar"), v1, v2)
possibleVersionDiffs(ForgeRepo(AzureRepos, onPremForgeUrl.addPath("foo/bar")), v1, v2)
.map(_.url.renderString),
List(
s"${onPremForgeUrl}foo/bar/branchCompare?baseVersion=GTv$v1&targetVersion=GTv$v2",
Expand All @@ -179,8 +182,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {

test("possibleUpdateInfoUrls: github.com") {
val obtained = possibleUpdateInfoUrls(
GitHub,
uri"https://github.com/foo/bar",
gitHubFooBarRepo,
v1,
v2
).map(_.url.renderString)
Expand Down Expand Up @@ -221,8 +223,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {

test("possibleUpdateInfoUrls: gitlab.com") {
val obtained = possibleUpdateInfoUrls(
GitLab,
uri"https://gitlab.com/foo/bar",
gitLabFooBarRepo,
v1,
v2
).map(_.url.renderString)
Expand All @@ -239,8 +240,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {

test("possibleUpdateInfoUrls: on-prem gitlab") {
val obtained = possibleUpdateInfoUrls(
GitLab,
uri"https://gitlab.on-prem.net/foo/bar",
ForgeRepo(GitLab, uri"https://gitlab.on-prem.net/foo/bar"),
v1,
v2
).map(_.url.renderString)
Expand All @@ -258,8 +258,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {

test("possibleUpdateInfoUrls: bitbucket.org") {
val obtained = possibleUpdateInfoUrls(
Bitbucket,
uri"https://bitbucket.org/foo/bar/",
bitbucketFooBarRepo,
v1,
v2
).map(_.url.renderString)
Expand All @@ -277,9 +276,9 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
}

test("possibleUpdateInfoUrls: on-prem Bitbucket Server") {
val forgeUrl = uri"https://bitbucket-server.on-prem.com"
val repoUrl = forgeUrl / "foo" / "bar"
val obtained = possibleUpdateInfoUrls(BitbucketServer, repoUrl, v1, v2).map(_.url)
val repoUrl = uri"https://bitbucket-server.on-prem.com" / "foo" / "bar"
val obtained =
possibleUpdateInfoUrls(ForgeRepo(BitbucketServer, repoUrl), v1, v2).map(_.url)
val expected = repoUrl / "browse" / "ReleaseNotes.md"
assert(clue(obtained).contains(expected))
}
Expand Down

0 comments on commit d52d95e

Please sign in to comment.