Skip to content

Commit

Permalink
Refactor: introduce Version.Update as a parameter object
Browse files Browse the repository at this point in the history
The `currentVersion` & `nextVersion` parameters are paired together
relatively often, and because they are of the same type (`Version`)
they are susceptible to being accidentally swapped over without any
compiler error being raised. Encapsulating them into a single
concept (that of a `Version.Update`) makes code a little clearer,
and reduces the number of individual parameters being passed on
method calls.
  • Loading branch information
rtyley committed Aug 19, 2023
1 parent d52d95e commit 13380eb
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ final case class Version(value: String) {
}

object Version {
case class Update(currentVersion: Version, nextVersion: Version)

val tagNames: List[Version => String] = List("v" + _, _.value, "release-" + _)

implicit val versionCodec: Codec[Version] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ final class NurtureAlg[F[_]](config: ForgeCfg)(implicit
case (currentVersion, dependency) =>
dependencyToMetadata.get(dependency).toList.traverse { metadata =>
updateInfoUrlFinder
.findUpdateInfoUrls(metadata, currentVersion, dependency.version)
.findUpdateInfoUrls(metadata, Version.Update(currentVersion, dependency.version))
.tupleLeft(dependency.artifactId.name)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,13 @@ final class UpdateInfoUrlFinder[F[_]](implicit
F: Monad[F]
) {
def findUpdateInfoUrls(
metadata: DependencyMetadata,
currentVersion: Version,
nextVersion: Version
dependency: DependencyMetadata,
versionUpdate: Version.Update
): F[List[UpdateInfoUrl]] = {
val updateInfoUrls: List[UpdateInfoUrl] =
metadata.releaseNotesUrl.toList.map(CustomReleaseNotes.apply) ++
metadata.forgeRepo.toSeq.flatMap(forgeRepo =>
possibleUpdateInfoUrls(forgeRepo, currentVersion, nextVersion)
dependency.releaseNotesUrl.toList.map(CustomReleaseNotes.apply) ++
dependency.forgeRepo.toSeq.flatMap(forgeRepo =>
possibleUpdateInfoUrls(forgeRepo, versionUpdate)
)

updateInfoUrls
Expand Down Expand Up @@ -75,26 +74,24 @@ object UpdateInfoUrlFinder {

private[nurture] def possibleVersionDiffs(
repoForge: ForgeRepo,
currentVersion: Version,
nextVersion: Version
update: Version.Update
): List[VersionDiff] = for {
tagName <- Version.tagNames
} yield VersionDiff(
repoForge.diffUrlFor(tagName(currentVersion), tagName(nextVersion))
repoForge.diffUrlFor(tagName(update.currentVersion), tagName(update.nextVersion))
)

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

gitHubReleaseNotesFor(forgeRepo, nextVersion) ++
gitHubReleaseNotesFor(forgeRepo, update.nextVersion) ++
customUrls(CustomReleaseNotes, possibleReleaseNotesFilenames) ++
customUrls(CustomChangelog, possibleChangelogFilenames) ++
possibleVersionDiffs(forgeRepo, currentVersion, nextVersion)
possibleVersionDiffs(forgeRepo, update)
}

private def gitHubReleaseNotesFor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {

private val v1 = Version("0.1.0")
private val v2 = Version("0.2.0")
private val versionUpdate = Version.Update(v1, v2)

test("findUpdateInfoUrls: repoUrl not found") {
val metadata = DependencyMetadata.empty.copy(scmUrl = uri"https://github.com/foo/foo".some)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
assertIO(obtained, List.empty)
}

test("findUpdateInfoUrls: repoUrl ok") {
val metadata = DependencyMetadata.empty.copy(scmUrl = uri"https://github.com/foo/bar".some)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
val expected = List(VersionDiff(uri"https://github.com/foo/bar/compare/v0.1.0...v0.2.0"))
assertIO(obtained, expected)
}
Expand All @@ -46,7 +47,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
scmUrl = uri"https://github.com/foo/bar".some,
releaseNotesUrl = uri"https://github.com/foo/bar/README.md#changelog".some
)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
val expected = List(
CustomReleaseNotes(metadata.releaseNotesUrl.get),
VersionDiff(uri"https://github.com/foo/bar/compare/v0.1.0...v0.2.0")
Expand All @@ -59,7 +60,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
scmUrl = uri"https://github.com/foo/bar2".some,
releaseNotesUrl = uri"https://github.com/foo/bar2/releases/tag/v0.2.0".some
)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
val expected = List(GitHubReleaseNotes(metadata.releaseNotesUrl.get))
assertIO(obtained, expected)
}
Expand All @@ -69,14 +70,14 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
scmUrl = uri"https://github.com/foo/bar1".some,
releaseNotesUrl = uri"https://github.com/foo/bar1/blob/master/RELEASES.md".some
)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
val expected = List(CustomReleaseNotes(metadata.releaseNotesUrl.get))
assertIO(obtained, expected)
}

test("findUpdateInfoUrls: repoUrl permanent redirect") {
val metadata = DependencyMetadata.empty.copy(scmUrl = uri"https://github.com/foo/buz".some)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = updateInfoUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
assertIO(obtained, List.empty)
}

Expand All @@ -95,14 +96,14 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
test("findUpdateInfoUrls: on-prem, repoUrl not found") {
val metadata =
DependencyMetadata.empty.copy(scmUrl = uri"https://github.on-prem.com/foo/foo".some)
val obtained = onPremUpdateUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = onPremUpdateUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
assertIO(obtained, List.empty)
}

test("findUpdateInfoUrls: on-prem, repoUrl ok") {
val metadata =
DependencyMetadata.empty.copy(scmUrl = uri"https://github.on-prem.com/foo/bar".some)
val obtained = onPremUpdateUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = onPremUpdateUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
val expected =
List(VersionDiff(uri"https://github.on-prem.com/foo/bar/compare/v0.1.0...v0.2.0"))
assertIO(obtained, expected)
Expand All @@ -111,15 +112,15 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
test("findUpdateInfoUrls: on-prem, repoUrl permanent redirect") {
val metadata =
DependencyMetadata.empty.copy(scmUrl = uri"https://github.on-prem.com/foo/buz".some)
val obtained = onPremUpdateUrlFinder.findUpdateInfoUrls(metadata, v1, v2).runA(state)
val obtained = onPremUpdateUrlFinder.findUpdateInfoUrls(metadata, versionUpdate).runA(state)
assertIO(obtained, List.empty)
}

test("possibleVersionDiffs") {
val onPremForgeUrl = uri"https://github.onprem.io/"

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

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

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

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

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

assertEquals(
possibleVersionDiffs(ForgeRepo(AzureRepos, onPremForgeUrl.addPath("foo/bar")), v1, v2)
possibleVersionDiffs(ForgeRepo(AzureRepos, onPremForgeUrl.addPath("foo/bar")), versionUpdate)
.map(_.url.renderString),
List(
s"${onPremForgeUrl}foo/bar/branchCompare?baseVersion=GTv$v1&targetVersion=GTv$v2",
Expand All @@ -183,8 +184,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
test("possibleUpdateInfoUrls: github.com") {
val obtained = possibleUpdateInfoUrls(
gitHubFooBarRepo,
v1,
v2
versionUpdate
).map(_.url.renderString)
val expected = List(
s"https://github.com/foo/bar/releases/tag/v$v2",
Expand Down Expand Up @@ -224,8 +224,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
test("possibleUpdateInfoUrls: gitlab.com") {
val obtained = possibleUpdateInfoUrls(
gitLabFooBarRepo,
v1,
v2
versionUpdate
).map(_.url.renderString)
val expected =
possibleReleaseNotesFilenames.map(name => s"https://gitlab.com/foo/bar/blob/master/$name") ++
Expand All @@ -241,8 +240,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
test("possibleUpdateInfoUrls: on-prem gitlab") {
val obtained = possibleUpdateInfoUrls(
ForgeRepo(GitLab, uri"https://gitlab.on-prem.net/foo/bar"),
v1,
v2
versionUpdate
).map(_.url.renderString)
val expected = possibleReleaseNotesFilenames.map(name =>
s"https://gitlab.on-prem.net/foo/bar/blob/master/$name"
Expand All @@ -259,8 +257,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
test("possibleUpdateInfoUrls: bitbucket.org") {
val obtained = possibleUpdateInfoUrls(
bitbucketFooBarRepo,
v1,
v2
versionUpdate
).map(_.url.renderString)
val expected =
possibleReleaseNotesFilenames.map(name =>
Expand All @@ -278,7 +275,7 @@ class UpdateInfoUrlFinderTest extends CatsEffectSuite with Http4sDsl[MockEff] {
test("possibleUpdateInfoUrls: on-prem Bitbucket Server") {
val repoUrl = uri"https://bitbucket-server.on-prem.com" / "foo" / "bar"
val obtained =
possibleUpdateInfoUrls(ForgeRepo(BitbucketServer, repoUrl), v1, v2).map(_.url)
possibleUpdateInfoUrls(ForgeRepo(BitbucketServer, repoUrl), versionUpdate).map(_.url)
val expected = repoUrl / "browse" / "ReleaseNotes.md"
assert(clue(obtained).contains(expected))
}
Expand Down

0 comments on commit 13380eb

Please sign in to comment.