Skip to content

Commit

Permalink
Check rate limits for each request to GitHub (#208)
Browse files Browse the repository at this point in the history
* Modify GitHubApi to throw an IllegalStateException if the rate limit
is exceeded (X-RateLimit-Remaining is 0). I chose to fail-fast here
instead of assuming callers will always check the result. This is fine
in this small application.
* Move the rate limit check logic out of the GitHubResponse "from"
factory method and into its own method, sendRequestAndCheckRateLimit.
This keeps the "from" method simple and free of cross-cutting concerns.
* Add several methods to GitHubResponse to get the reset time, the time
until reset from a given date, and methods to check if the rate limit is
exceeded.
* Add rateLimitResource to GitHubResponse, so that it can be included in
log and exception messages, which will be most useful if the rate limit
is exceeded.
* Add GitHubResponseTest as dedicated test of GitHubResponse

Misc:

* Remove unused (except in tests) takeRequestWith1MilliTimeout from
MockWebServerExtensions.kt
* Add firstValueOrThrow to HttpHeadersExtensions.kt
* Add top-level and extension functions in DateTimeExtensions.kt

Closes #173
  • Loading branch information
sleberknight authored Jul 22, 2024
1 parent 6cce24c commit f78baab
Show file tree
Hide file tree
Showing 8 changed files with 332 additions and 85 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.kiwiproject.changelog.extension

import java.time.Instant
import java.time.ZoneOffset
import java.time.ZonedDateTime
import java.time.temporal.ChronoUnit

fun nowUtcTruncatedToSeconds(): ZonedDateTime =
nowUtc().truncatedToSeconds()

fun nowUtc(): ZonedDateTime =
ZonedDateTime.now(ZoneOffset.UTC)

fun ZonedDateTime.truncatedToSeconds(): ZonedDateTime =
truncatedTo(ChronoUnit.SECONDS)

fun utcZonedDateTimeFromEpochSeconds(epochSeconds: Long): ZonedDateTime =
Instant.ofEpochSecond(epochSeconds).atZone(ZoneOffset.UTC)
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@ import java.net.http.HttpHeaders

fun HttpHeaders.firstValueOrNull(name: String): String? = firstValue(name).orElse(null)

fun HttpHeaders.firstValueOrThrow(name: String): String =
firstValue(name).orElseThrow { IllegalStateException("$name header is required") }

fun HttpHeaders.firstValueAsLongOrThrow(name: String): Long =
firstValueAsLong(name).orElseThrow { IllegalStateException("$name header is required")}
125 changes: 84 additions & 41 deletions src/main/kotlin/org/kiwiproject/changelog/github/GitHubApi.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,59 +5,65 @@ import io.github.oshai.kotlinlogging.KotlinLogging
import io.github.oshai.kotlinlogging.Level
import org.kiwiproject.changelog.extension.firstValueAsLongOrThrow
import org.kiwiproject.changelog.extension.firstValueOrNull
import org.kiwiproject.changelog.extension.firstValueOrThrow
import org.kiwiproject.changelog.extension.nowUtcTruncatedToSeconds
import org.kiwiproject.changelog.extension.utcZonedDateTimeFromEpochSeconds
import org.kiwiproject.time.KiwiDurationFormatters
import java.net.URI
import java.net.http.HttpClient
import java.net.http.HttpHeaders
import java.net.http.HttpRequest
import java.net.http.HttpRequest.BodyPublishers
import java.net.http.HttpResponse
import java.net.http.HttpResponse.BodyHandlers
import java.time.Duration
import java.time.Instant
import java.time.ZoneId
import java.time.ZoneOffset
import java.time.ZonedDateTime
import java.time.format.DateTimeFormatter
import java.time.temporal.ChronoUnit

private val LOG = KotlinLogging.logger {}

internal const val RATE_LIMIT_REMAINING_WARNING_THRESHOLD = 5L

class GitHubApi(
private val githubToken: String,
private val httpClient: HttpClient = HttpClient.newHttpClient()
) {

/**
* Generic method to make a GET request to any GitHub REST API endpoint.
*
* Throws [IllegalStateException] if the GitHub rate limit is exceeded.
*/
fun get(url: String): GitHubResponse {
LOG.debug { "GET: $url" }

val httpRequest = newRequestBuilder(url).GET().build()
return sendRequest(httpRequest)
return sendRequestAndCheckRateLimit(httpRequest)
}

/**
* Generic method to make a POST request to any GitHub REST API endpoint.
*
* Throws [IllegalStateException] if the GitHub rate limit is exceeded.
*/
fun post(url: String, bodyJson: String): GitHubResponse {
LOG.debug { "POST: $url" }

val bodyPublisher = BodyPublishers.ofString(bodyJson)
val httpRequest = newRequestBuilder(url).POST(bodyPublisher).build()
return sendRequest(httpRequest)
return sendRequestAndCheckRateLimit(httpRequest)
}

/**
* Generic method to make a PATCH request to any GitHub REST API endpoint.
*
* Throws [IllegalStateException] if the GitHub rate limit is exceeded.
*/
fun patch(url: String, bodyJson: String): GitHubResponse {
LOG.debug { "PATCH: $url" }

val bodyPublisher = BodyPublishers.ofString(bodyJson)
val httpRequest = newRequestBuilder(url).method("PATCH", bodyPublisher).build()
return sendRequest(httpRequest)
return sendRequestAndCheckRateLimit(httpRequest)
}

private fun newRequestBuilder(url: String): HttpRequest.Builder =
Expand All @@ -66,8 +72,38 @@ class GitHubApi(
.header("Content-Type", "application/vnd.github+json")
.header("Authorization", "token $githubToken")

private fun sendRequestAndCheckRateLimit(httpRequest: HttpRequest): GitHubResponse {
val response = sendRequest(httpRequest)

val now = nowUtcTruncatedToSeconds()
val timeUntilReset = response.timeUntilRateLimitResetsFrom(now)
val humanTimeUntilReset = humanTimeUntilReset(timeUntilReset, response.rateLimitRemaining)

val currentDateTime = DateTimeFormatter.ISO_ZONED_DATE_TIME.format(now)
val rateLimitReset = response.resetAt()
val rateLimitLogMessage = "GitHub API rate info => Limit : ${response.rateLimitLimit}," +
" Remaining : ${response.rateLimitRemaining}," +
" Current time: ${currentDateTime}," +
" Reset at: $rateLimitReset, ${humanTimeUntilReset.message}," +
" Resource: ${response.rateLimitResource}"
LOG.at(humanTimeUntilReset.logLevel) { this.message = rateLimitLogMessage }

check(response.belowRateLimit()) {
IllegalStateException(
"Rate limit exceeded for resource: ${response.rateLimitResource}." +
" No more requests can be made to that resource until $rateLimitReset (${humanTimeUntilReset.message})"
)
}

return response
}

private fun sendRequest(httpRequest: HttpRequest): GitHubResponse {
val httpResponse = httpClient.send(httpRequest, BodyHandlers.ofString())

val link = httpResponse.headers().firstValueOrNull("Link")
LOG.debug { "GitHub 'Link' header: $link" }

return GitHubResponse.from(httpResponse)
}

Expand All @@ -81,9 +117,30 @@ class GitHubApi(
val linkHeader: String?,
val rateLimitLimit: Long,
val rateLimitRemaining: Long,
val rateLimitResetAt: Long
val rateLimitResetAt: Long,
val rateLimitResource: String
) {

/**
* The UTC date/time when the rate limit resets.
*/
fun resetAt(): ZonedDateTime = utcZonedDateTimeFromEpochSeconds(rateLimitResetAt)

/**
* The duration until the rate limit resets.
*/
fun timeUntilRateLimitResetsFrom(from: ZonedDateTime): Duration = Duration.between(from, resetAt())

/**
* There are requests remaining before the rate limit resets.
*/
fun belowRateLimit(): Boolean = !exceededRateLimit()

/**
* There are no more requests remaining before the rate limit resets.
*/
fun exceededRateLimit(): Boolean = rateLimitRemaining == 0L

companion object {

/**
Expand All @@ -94,20 +151,8 @@ class GitHubApi(
val rateLimitLimit = responseHeaders.firstValueAsLongOrThrow("X-RateLimit-Limit")
val rateLimitRemaining = responseHeaders.firstValueAsLongOrThrow("X-RateLimit-Remaining")
val rateLimitResetEpochSeconds = responseHeaders.firstValueAsLongOrThrow("X-RateLimit-Reset")

val now = ZonedDateTime.now(ZoneOffset.UTC).truncatedTo(ChronoUnit.SECONDS)
val resetAt = Instant.ofEpochSecond(rateLimitResetEpochSeconds).atZone(ZoneId.of("UTC"))
val timeUntilReset = Duration.between(now, resetAt)
val humanTimeUntilReset = humanTimeUntilReset(timeUntilReset)

val currentDateTime = DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(now)
val rateLimitReset = epochSecondsAsIsoFormatted(rateLimitResetEpochSeconds)
val rateLimitLogMessage =
"GitHub API rate info => Limit : $rateLimitLimit, Remaining : $rateLimitRemaining, Current time: $currentDateTime, Reset at: $rateLimitReset, ${humanTimeUntilReset.message}"
LOG.at(humanTimeUntilReset.logLevel) { this.message = rateLimitLogMessage }

val rateLimitResource = responseHeaders.firstValueOrThrow("X-RateLimit-Resource")
val link = responseHeaders.firstValueOrNull("Link")
LOG.debug { "GitHub 'Link' header: $link" }

return GitHubResponse(
httpResponse.statusCode(),
Expand All @@ -116,30 +161,28 @@ class GitHubApi(
link,
rateLimitLimit,
rateLimitRemaining,
rateLimitResetEpochSeconds
rateLimitResetEpochSeconds,
rateLimitResource
)
}

@VisibleForTesting
internal fun humanTimeUntilReset(timeUntilReset: Duration): TimeUntilReset =
when {
timeUntilReset.isNegative -> TimeUntilReset("Time until reset is negative! ($timeUntilReset)", true, Level.WARN)
else -> TimeUntilReset("Time until reset: ${KiwiDurationFormatters.formatDurationWords(timeUntilReset)}", false, Level.DEBUG)
}

data class TimeUntilReset(val message: String, val isNegative: Boolean, val logLevel: Level)
}
}
}

@VisibleForTesting
fun resetLimitAsIsoFormatted(responseHeaders: HttpHeaders): String {
val rateLimitReset = responseHeaders.firstValueAsLongOrThrow("X-RateLimit-Reset")
return epochSecondsAsIsoFormatted(rateLimitReset)
}
internal fun humanTimeUntilReset(timeUntilReset: Duration, rateLimitRemaining: Long): TimeUntilReset =
when {
timeUntilReset.isNegative ->
TimeUntilReset("Time until reset is negative! ($timeUntilReset)", true, Level.WARN)

else -> TimeUntilReset(
"Time until reset: ${KiwiDurationFormatters.formatDurationWords(timeUntilReset)}",
false,
logLevelForRateLimitRemaining(rateLimitRemaining)
)
}

private fun epochSecondsAsIsoFormatted(epochSeconds: Long): String {
return DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(
Instant.ofEpochSecond(epochSeconds).atZone(ZoneId.of("UTC"))
)
}
private fun logLevelForRateLimitRemaining(remaining: Long) =
if (remaining > RATE_LIMIT_REMAINING_WARNING_THRESHOLD) Level.DEBUG else Level.WARN

internal data class TimeUntilReset(val message: String, val isNegative: Boolean, val logLevel: Level)
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package org.kiwiproject.changelog.extension

import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.within
import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.api.RepeatedTest
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertAll
import java.time.Instant
import java.time.ZoneId
import java.time.ZoneOffset
import java.time.ZonedDateTime
import java.time.temporal.ChronoUnit
import kotlin.random.Random

@DisplayName("DateTimeExtensions")
class DateTimeExtensionsTest {

@Test
fun shouldGetCurrentTimeAtUTCTruncatedToSeconds() {
val now = nowUtcTruncatedToSeconds()

assertAll(
{ assertThat(now.offset).isEqualTo(ZoneOffset.UTC) },
{ assertThat(now.nano).isZero() }
)
}

@Test
fun shouldGetCurrentTimeAtUTC() {
val now = nowUtc()

assertAll(
{ assertThat(now.offset).isEqualTo(ZoneOffset.UTC) },
{ assertThat(now).isCloseTo(ZonedDateTime.now(ZoneOffset.UTC), within(100, ChronoUnit.MILLIS)) }
)
}

@RepeatedTest(10)
fun shouldTruncateZonedDateTimeToSeconds() {
val randomMinutes = Random.nextLong(1, 100)
val offset = Random.nextInt(10)
val now = ZonedDateTime.now(ZoneOffset.ofHours(offset)).plusMinutes(randomMinutes)
val nowWithSecondPrecision = now.truncatedToSeconds()

assertAll(
{ assertThat(nowWithSecondPrecision.offset).isEqualTo(now.offset) },
{ assertThat(nowWithSecondPrecision.nano).isZero() }
)
}

@RepeatedTest(10)
fun shouldCreateZonedDateTimeAtUTCFromEpochSeconds() {
val originalZonedDateTime = ZonedDateTime
.now(ZoneOffset.ofHours(Random.nextInt(10)))
.plusMinutes(Random.nextLong(0, 60))
val epochSeconds = originalZonedDateTime.toEpochSecond()

val utcZonedDateTime = utcZonedDateTimeFromEpochSeconds(epochSeconds)

assertThat(utcZonedDateTime)
.isEqualTo(Instant.ofEpochSecond(epochSeconds).atZone(ZoneId.of("UTC")))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ fun MockWebServer.urlWithoutTrailingSlashAsString(path: String): String =
fun MockWebServer.takeRequestWith1SecTimeout(): RecordedRequest =
this.takeRequest(1, TimeUnit.SECONDS)!!

/**
* Calls [MockWebServer.takeRequest] method with a 5-millisecond timeout.
*
* Use this when you don't expect there to be any more requests, and
* verify it by ensuring the returned `RecordedRequest` is `null`.
*/
fun MockWebServer.takeRequestWith1MilliTimeout() : RecordedRequest? =
this.takeRequest(1, TimeUnit.MILLISECONDS)

/**
* Asserts that there are no more recorded requests for a [MockWebServer].
*/
Expand All @@ -65,17 +56,28 @@ fun MockResponse.addJsonContentTypeHeader() : MockResponse {
}

/**
* Adds the GitHub
* Decrements `rateLimitRemaining`, and adds the GitHub
* [rate limit headers](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#checking-the-status-of-your-rate-limit).
*
* The rate limit reset time is calculated as "now" plus 42 minutes (naturally).
*/
fun MockResponse.addGitHubRateLimitHeaders(): MockResponse {
rateLimitRemaining--
return addGitHubRateLimitHeaders(rateLimitRemaining)
}

/**
* Adds the GitHub
* [rate limit headers](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#checking-the-status-of-your-rate-limit).
*
* The rate limit reset time is calculated as "now" plus 42 minutes (naturally).
*/
fun MockResponse.addGitHubRateLimitHeaders(rateLimitRemaining: Long): MockResponse {
val rateLimitResetAt = Instant.now().plus(42, ChronoUnit.MINUTES).epochSecond

addHeader("X-RateLimit-Limit", rateLimitLimit)
addHeader("X-RateLimit-Remaining", rateLimitRemaining)
addHeader("X-RateLimit-Reset", rateLimitResetAt)
addHeader("X-RateLimit-Resource", "core")
return this
}
Loading

0 comments on commit f78baab

Please sign in to comment.