Skip to content

Commit bec9060

Browse files
bishibooshbishiboosh
committed
fix: Avoid rate limiting issues with GitHub
1 parent c9f0fe7 commit bec9060

File tree

10 files changed

+526
-193
lines changed

10 files changed

+526
-193
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
### Removed
1212

1313
### Fixed
14+
- Avoid GitHub API rate limiting by avoiding concurrent requests when quertying GitHub releases. As
15+
a consequence, GitHub release checks will be slower, so the check for releases notes is now disabled
16+
by default, and will have to be enabled explicitly in the configuration or the command-line options.
1417

1518
### Security
1619

‎cli/src/commonMain/kotlin/com/deezer/caupain/cli/CaupainCLI.kt‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,15 @@ import com.github.ajalt.clikt.parameters.options.convert
6868
import com.github.ajalt.clikt.parameters.options.default
6969
import com.github.ajalt.clikt.parameters.options.flag
7070
import com.github.ajalt.clikt.parameters.options.multiple
71+
import com.github.ajalt.clikt.parameters.options.nullableFlag
7172
import com.github.ajalt.clikt.parameters.options.option
7273
import com.github.ajalt.clikt.parameters.options.required
7374
import com.github.ajalt.clikt.parameters.options.unique
7475
import com.github.ajalt.clikt.parameters.options.versionOption
7576
import com.github.ajalt.clikt.parameters.types.choice
7677
import com.github.ajalt.mordant.animation.coroutines.CoroutineProgressTaskAnimator
7778
import com.github.ajalt.mordant.animation.coroutines.animateInCoroutine
79+
import com.github.ajalt.mordant.platform.MultiplatformSystem
7880
import com.github.ajalt.mordant.widgets.progress.marquee
7981
import com.github.ajalt.mordant.widgets.progress.percentage
8082
import com.github.ajalt.mordant.widgets.progress.progressBar
@@ -457,10 +459,12 @@ class CaupainCLI(
457459
?: parsedConfiguration?.gradleStabilityLevel
458460
?: GradleStabilityLevel.STABLE,
459461
checkIgnored = parsedConfiguration?.checkIgnored == true,
460-
githubToken = releaseNoteOptions?.githubToken ?: parsedConfiguration?.githubToken,
461462
searchReleaseNote = releaseNoteOptions?.searchReleaseNote
462-
?: parsedConfiguration?.let { it.searchReleaseNote ?: (it.githubToken != null) }
463+
?: parsedConfiguration?.searchReleaseNote
463464
?: false,
465+
githubToken = releaseNoteOptions?.githubToken
466+
?: parsedConfiguration?.githubToken
467+
?: currentContext.readEnvvar("CAUPAIN_GITHUB_TOKEN"),
464468
verifyExistence = verifyExistence || parsedConfiguration?.verifyExistence == true
465469
)
466470
}
@@ -661,16 +665,15 @@ class CaupainCLI(
661665
}
662666

663667
private class ReleaseNoteOptions : OptionGroup() {
664-
val githubToken by option(
665-
"--github-token",
666-
help = "GitHub token for searching release notes",
667-
envvar = "CAUPAIN_GITHUB_TOKEN"
668-
).required()
669-
670668
val searchReleaseNote by option(
671669
"--search-release-notes",
672670
help = "Search for release notes for updated versions on GitHub",
673-
).flag(default = true, defaultForHelp = "true if GitHub token is provided")
671+
).nullableFlag().required()
672+
673+
val githubToken by option(
674+
"--github-token",
675+
help = "GitHub token for searching release notes. Taken from CAUPAIN_GITHUB_TOKEN environment variable if not set",
676+
)
674677
}
675678

676679
private class MultiplePathOptions(fileSystem: FileSystem) : OptionGroup() {

‎cli/src/jvmTest/kotlin/com/deezer/caupain/cli/CaupainCLIConfigTest.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ class CaupainCLIConfigTest {
198198
versionCatalog = null,
199199
versionCatalogInfo = null,
200200
)
201-
createCli(
201+
val result = createCli(
202202
expectedGradleVersion = when {
203203
hasOptionsOverride -> gradleVersionCli
204204
hasConf -> gradleVersionConf
@@ -327,6 +327,7 @@ class CaupainCLIConfigTest {
327327
"outputBaseNameCli",
328328
"--gradle-wrapper-properties",
329329
gradleWrapperPathCli.toString(),
330+
"--search-release-notes",
330331
"--github-token",
331332
"githubTokenCli",
332333
"--cache-dir",
@@ -342,6 +343,7 @@ class CaupainCLIConfigTest {
342343
else -> emptyList()
343344
}
344345
)
346+
assertEquals(expected = 0, actual = result.statusCode, "CLI exited with non-zero code, error is ${result.stderr}")
345347
if (hasOptionsOverride) {
346348
assertTrue(fileSystem.exists("outputDirCli/outputBaseNameCli.md".toPath()))
347349
} else if (hasConf) {
@@ -414,4 +416,4 @@ private object TestAppDirs : AppDirs {
414416
override fun getUserDataDir(roaming: Boolean): String = ""
415417

416418
override fun getUserLogDir(): String = ""
417-
}
419+
}

‎core/src/commonMain/kotlin/com/deezer/caupain/DependencyUpdateChecker.kt‎

Lines changed: 62 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -473,54 +473,84 @@ internal class DefaultDependencyUpdateChecker(
473473

474474
private suspend fun checkUpdateInfo(updatedVersions: List<DependencyUpdateResult>): Map<UpdateInfo.Type, List<UpdateInfo>> {
475475
val nbUpdatedVersions = updatedVersions.size
476-
val updateInfosMutex = Mutex()
476+
val nbSteps = if (configuration.searchReleaseNote) {
477+
nbUpdatedVersions * 2
478+
} else {
479+
nbUpdatedVersions
480+
}
481+
// Search Maven infos
482+
val mavenInfos = getMavenInfos(updatedVersions) {
483+
val percentage = ++completed * 50 / nbSteps + 50
484+
progressFlow.value = DependencyUpdateChecker.Progress.Determinate(
485+
taskName = GATHERING_INFO_TASK,
486+
percentage = percentage
487+
)
488+
}
489+
// Search release notes and build final infos
477490
val updatesInfos = mutableMapOf<UpdateInfo.Type, MutableList<UpdateInfo>>()
491+
for (result in updatedVersions) {
492+
logger.info("Finding release note info for ${result.dependency.moduleId}")
493+
val type = when (result.dependency) {
494+
is Dependency.Library -> UpdateInfo.Type.LIBRARY
495+
is Dependency.Plugin -> UpdateInfo.Type.PLUGIN
496+
}
497+
val mavenInfo = mavenInfos[result.dependencyKey]
498+
val releaseNoteUrl = if (configuration.searchReleaseNote) {
499+
releaseNoteResolver.getReleaseNoteUrl(
500+
mavenInfo = mavenInfo,
501+
updatedVersion = result.updatedVersion
502+
)
503+
} else {
504+
null
505+
}
506+
val updateInfo = toUpdateInfo(
507+
key = result.dependencyKey,
508+
dependency = result.dependency,
509+
currentVersion = result.currentVersion,
510+
updatedVersion = result.updatedVersion,
511+
mavenInfo = mavenInfo,
512+
releaseNoteUrl = releaseNoteUrl
513+
)
514+
val infosForType = updatesInfos[type]
515+
?: mutableListOf<UpdateInfo>().also { updatesInfos[type] = it }
516+
infosForType.add(updateInfo)
517+
val percentage = ++completed * 50 / nbSteps + 50
518+
progressFlow.value = DependencyUpdateChecker.Progress.Determinate(
519+
taskName = GATHERING_INFO_TASK,
520+
percentage = percentage
521+
)
522+
}
523+
progressFlow.value = DependencyUpdateChecker.Progress.Determinate(DONE, 100)
524+
return updatesInfos
525+
}
526+
527+
private suspend inline fun getMavenInfos(
528+
updatedVersions: List<DependencyUpdateResult>,
529+
crossinline onStepFinished: () -> Unit,
530+
): Map<String, MavenInfo> {
531+
val mavenInfoMutex = Mutex()
532+
val mavenInfos = mutableMapOf<String, MavenInfo>()
478533
coroutineScope {
479534
updatedVersions
480535
.map { result ->
481536
async {
482537
logger.info("Finding Maven info for ${result.dependency.moduleId}")
483-
val type = when (result.dependency) {
484-
is Dependency.Library -> UpdateInfo.Type.LIBRARY
485-
is Dependency.Plugin -> UpdateInfo.Type.PLUGIN
486-
}
487538
val mavenInfo = infoResolver.getMavenInfo(
488539
dependency = result.dependency,
489540
repository = result.repository,
490541
updatedVersion = result.updatedVersion
491542
)
492-
val releaseNoteUrl = if (configuration.searchReleaseNote) {
493-
releaseNoteResolver.getReleaseNoteUrl(
494-
mavenInfo = mavenInfo,
495-
updatedVersion = result.updatedVersion
496-
)
497-
} else {
498-
null
499-
}
500-
val updateInfo = toUpdateInfo(
501-
key = result.dependencyKey,
502-
dependency = result.dependency,
503-
currentVersion = result.currentVersion,
504-
updatedVersion = result.updatedVersion,
505-
mavenInfo = mavenInfo,
506-
releaseNoteUrl = releaseNoteUrl
507-
)
508-
updateInfosMutex.withLock {
509-
val infosForType = updatesInfos[type]
510-
?: mutableListOf<UpdateInfo>().also { updatesInfos[type] = it }
511-
infosForType.add(updateInfo)
543+
if (mavenInfo != null) {
544+
mavenInfoMutex.withLock {
545+
mavenInfos[result.dependencyKey] = mavenInfo
546+
}
512547
}
513-
val percentage = ++completed * 50 / nbUpdatedVersions + 50
514-
progressFlow.value = DependencyUpdateChecker.Progress.Determinate(
515-
taskName = GATHERING_INFO_TASK,
516-
percentage = percentage
517-
)
548+
onStepFinished()
518549
}
519550
}
520551
.awaitAll()
521-
progressFlow.value = DependencyUpdateChecker.Progress.Determinate(DONE, 100)
522552
}
523-
return updatesInfos
553+
return mavenInfos
524554
}
525555

526556
private fun toUpdateInfo(

‎core/src/commonMain/kotlin/com/deezer/caupain/model/github/Repository.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package com.deezer.caupain.model.github
2+
3+
import kotlinx.serialization.SerialName
4+
import kotlinx.serialization.Serializable
5+
6+
@Serializable
7+
internal data class Repository(
8+
val id: String,
9+
@SerialName("default_branch") val defaultBranch: String? = null,
10+
)

‎core/src/commonMain/kotlin/com/deezer/caupain/model/github/SearchResult.kt‎

Lines changed: 0 additions & 40 deletions
This file was deleted.

‎core/src/commonMain/kotlin/com/deezer/caupain/model/github/Tree.kt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.deezer.caupain.model.github
2+
3+
import kotlinx.serialization.SerialName
4+
import kotlinx.serialization.Serializable
5+
6+
@Serializable
7+
internal data class Tree(
8+
val sha: String,
9+
@SerialName("tree") val content: List<TreeContent>
10+
)
11+
12+
@Serializable
13+
internal data class TreeContent(
14+
val path: String,
15+
val type: String,
16+
)

‎core/src/commonMain/kotlin/com/deezer/caupain/resolver/GithubReleaseNoteResolver.kt‎

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ import com.deezer.caupain.internal.processRequest
2828
import com.deezer.caupain.model.GradleDependencyVersion
2929
import com.deezer.caupain.model.Logger
3030
import com.deezer.caupain.model.github.Release
31-
import com.deezer.caupain.model.github.SearchResults
31+
import com.deezer.caupain.model.github.Repository
32+
import com.deezer.caupain.model.github.Tree
3233
import com.deezer.caupain.model.maven.MavenInfo
3334
import io.ktor.client.HttpClient
3435
import io.ktor.client.request.get
@@ -59,6 +60,7 @@ internal class GithubReleaseNoteResolver(
5960
?.url
6061
?.takeUnless { it.isBlank() }
6162
?.replace("git@", "https://")
63+
?.removeSuffix(".git")
6264
?.let { url ->
6365
try {
6466
Url(url)
@@ -95,28 +97,58 @@ internal class GithubReleaseNoteResolver(
9597
logger.error("Failed to fetch releases for $owner/$repository", error)
9698
}
9799
) {
98-
getGithubResource("repos", owner, repository, "releases")
100+
getGithubResource(REPOS_PATH, owner, repository, "releases")
99101
}
100102
}
101103
val releaseNote = releaseNotes.firstOrNull { it.matches(version) }
102104
if (releaseNote != null) return releaseNote.url
103105
// If not present, try to find the changelog file in the repository
104-
val searchResults = withContext(ioDispatcher) {
105-
httpClient.processRequest<SearchResults?>(
106+
return getChangelogUrl(owner, repository)
107+
}
108+
109+
private suspend fun getChangelogUrl(
110+
owner: String,
111+
repository: String,
112+
): String? {
113+
// First get the repository info to find the default branch
114+
val defaultBranch = withContext(ioDispatcher) {
115+
httpClient.processRequest<Repository, String?>(
106116
default = null,
117+
transform = { it.defaultBranch },
107118
onRecoverableError = { error ->
108-
logger.error("Failed to search for changelog in $owner/$repository", error)
119+
logger.error("Failed to find default branch for $owner/$repository", error)
109120
}
110121
) {
111-
getGithubResource("search", "code") {
112-
parameters["q"] = "$version repo:$owner/$repository filename:CHANGELOG.md"
122+
getGithubResource(REPOS_PATH, owner, repository)
123+
}
124+
} ?: return null
125+
// Then check if a CHANGELOG.md file exists in the default branch
126+
val changelogPath = withContext(ioDispatcher) {
127+
httpClient.processRequest<Tree, String?>(
128+
default = null,
129+
transform = { tree ->
130+
tree
131+
.content
132+
.firstNotNullOfOrNull { file ->
133+
if (file.path == "CHANGELOG.md" && file.type == "blob") {
134+
file.path
135+
} else {
136+
null
137+
}
138+
}
139+
},
140+
onRecoverableError = { error ->
141+
logger.error("Failed to find contents for $owner/$repository", error)
113142
}
143+
) {
144+
getGithubResource(REPOS_PATH, owner, repository, "git", "trees", defaultBranch)
114145
}
115146
}
116-
if (searchResults != null && searchResults.totalCount > 0) {
117-
return searchResults.items.firstOrNull()?.url
147+
return if (changelogPath == null) {
148+
null
149+
} else {
150+
"https://github.com/$owner/$repository/blob/$defaultBranch/$changelogPath"
118151
}
119-
return null
120152
}
121153

122154
private fun checkToken(): Boolean {
@@ -130,7 +162,7 @@ internal class GithubReleaseNoteResolver(
130162
@Suppress("SuspendFunWithCoroutineScopeReceiver") // We need to use HttpClient as receiver
131163
private suspend fun HttpClient.getGithubResource(
132164
vararg pathSegments: String,
133-
urlBuilder: URLBuilder.() -> Unit = {}
165+
urlBuilder: URLBuilder.() -> Unit = {},
134166
): HttpResponse {
135167
return get {
136168
url {
@@ -147,5 +179,6 @@ internal class GithubReleaseNoteResolver(
147179
companion object {
148180
private val BASE_API_URL = Url("https://api.github.com")
149181
private const val GITHUB_HOST = "github.com"
182+
private const val REPOS_PATH = "repos"
150183
}
151-
}
184+
}

0 commit comments

Comments
 (0)