Skip to content

Commit

Permalink
Merge pull request #20 from JetBrains/fabrizio.scarponi/update-api-cl…
Browse files Browse the repository at this point in the history
…ient-for-cloudfront-pourpose

Change the endpoint for get package by Id or IdHash
updater kotlinx.document.store due an issue
  • Loading branch information
fscarponi authored Sep 20, 2024
2 parents 62fe96f + 6e12f96 commit 1301d23
Show file tree
Hide file tree
Showing 14 changed files with 29,302 additions and 14,051 deletions.
1 change: 1 addition & 0 deletions http/client/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ kotlin {
dependencies {
api(kotlin("test-junit5"))
api(packageSearchApiModelsVersions.junit.jupiter.engine)
api(packageSearchApiModelsVersions.ktor.client.java)
api(kotlinxDocumentStore.mvstore)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import io.ktor.client.request.get
import io.ktor.client.request.request
import io.ktor.client.request.setBody
import io.ktor.client.statement.*
import io.ktor.client.utils.EmptyContent
import io.ktor.http.*
import io.ktor.serialization.kotlinx.json.*
import io.ktor.serialization.kotlinx.protobuf.*
Expand All @@ -30,7 +31,6 @@ import org.jetbrains.packagesearch.api.v3.ApiPackage
import org.jetbrains.packagesearch.api.v3.ApiProject
import org.jetbrains.packagesearch.api.v3.ApiRepository
import org.jetbrains.packagesearch.packageversionutils.normalization.NormalizedVersion
import kotlin.collections.map
import kotlin.time.Duration
import kotlin.time.Duration.Companion.minutes
import kotlin.time.Duration.Companion.seconds
Expand Down Expand Up @@ -279,12 +279,9 @@ public class PackageSearchApiClient(
.associateBy { if (useHashes) it.idHash else it.id }
}

val onlineResults = defaultRequest<_, List<ApiPackage>>(
method = HttpMethod.Post,
url = endpoints.packageInfoByIdHashes,
body = GetPackageInfoRequest(unresolvedIdentifiers),
requestBuilder = requestBuilder,
).associateBy { it.idHash }
val onlineResults =
fetchOnlinePackageInfoByIdHashes(unresolvedIdentifiers, requestBuilder)


val notFoundPackages = idHashes - cachedResults.map { it.key }.toSet() - onlineResults.keys

Expand Down Expand Up @@ -312,12 +309,30 @@ public class PackageSearchApiClient(
onlineResultsMappedKeys + cachedResultMap
}

private suspend fun fetchOnlinePackageInfoByIdHashes(
unresolvedIdentifiers: Set<String>,
requestBuilder: (HttpRequestBuilder.() -> Unit)?
) = coroutineScope {
unresolvedIdentifiers.map { idHash ->
async {
defaultRawRequest(
method = HttpMethod.Get,
url = endpoints.packageInfoByIdHash,
body = EmptyContent,
requestBuilder = {
url { parameters.append("idHash", idHash) }
requestBuilder?.invoke(this)
}
).takeIf { it.status != HttpStatusCode.NoContent }?.body<ApiPackage>()
}
}.awaitAll().filterNotNull().associateBy({ it.idHash }, { it })
}

public suspend fun searchPackages(
request: SearchPackagesRequest,
requestBuilder: (HttpRequestBuilder.() -> Unit)? = null,
): List<ApiPackage> {


val searchCache = searchCacheCollection.await()

searchCache.find(selector = SearchCacheEntry::key.name, value = request)
Expand Down Expand Up @@ -457,7 +472,6 @@ public class PackageSearchApiClient(
return cachedResults.mapNotNull { it.value }
}


val onlineResults =
defaultRequest<_, List<ApiPackage>>(
method = HttpMethod.Post,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,5 @@ public data class SerializableHttpRequest(

public fun HttpRequest.toSerializable(): SerializableHttpRequest =
SerializableHttpRequest(url.toString(), method.value, headers.toMap())

public typealias EmptyBody = String
109 changes: 109 additions & 0 deletions http/client/src/jvmTest/kotlin/ApiClientPerformanceTests.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import io.ktor.client.HttpClient
import io.ktor.client.engine.java.Java
import io.ktor.client.plugins.HttpRequestRetry
import io.ktor.client.plugins.HttpTimeout
import io.ktor.client.plugins.compression.ContentEncoding
import io.ktor.client.plugins.contentnegotiation.ContentNegotiation
import io.ktor.client.plugins.defaultRequest
import io.ktor.client.plugins.logging.LogLevel
import io.ktor.client.plugins.logging.Logging
import io.ktor.client.request.header
import io.ktor.http.HttpStatusCode
import io.ktor.serialization.kotlinx.json.json
import kotlinx.coroutines.test.runTest
import kotlinx.document.database.mvstore.asDataStore
import org.h2.mvstore.MVStore
import org.jetbrains.packagesearch.api.v3.http.PackageSearchApiClient
import org.jetbrains.packagesearch.api.v3.http.PackageSearchEndpoints
import org.jetbrains.packagesearch.api.v3.http.requestTimeout
import org.junit.jupiter.api.AfterAll
import org.junit.jupiter.api.Test
import kotlin.time.Duration.Companion.seconds
import kotlin.time.measureTimedValue


class ApiClientPerformanceTests {

companion object {
internal val timingMap = mutableMapOf<String, Long>()

@Suppress("RedundantUnitReturnType")
@JvmStatic
@AfterAll
fun collectTimings(): Unit {
println("Timings:")
timingMap.forEach { (key, value) ->
println("$key: $value ms")
}
}
}

private val apiClient = PackageSearchApiClient(
dataStore = MVStore.open(null).asDataStore(),
endpoints = PackageSearchEndpoints.PROD,
httpClient = HttpClient(Java) {
defaultRequest {
header("JB-Plugin-Version", "241.0.12")
}
install(ContentNegotiation) {
json()
}
install(ContentEncoding) {
gzip()
}
install(HttpRequestRetry) {
maxRetries = 3
exponentialDelay()
retryIf(3) { _, httpResponse ->
httpResponse.status.value in 500..599 || httpResponse.status == HttpStatusCode.RequestTimeout
}
}
install(HttpTimeout) {
requestTimeout = 30.seconds
}
install(Logging) {
level = LogLevel.ALL
}
},

)

private val sampleIdHashes = setOf(
"afa0f79b67522a855aa1343ed55939170e5910020c7e39f081c59f38f132c0cf", //ktor
"e0644eb2501825154b2fc68467537bd8d4b460f2df6aab7b7b2051af793232db", //google guava bom
"ad25fb431b508c516a20bf717e144c96b494492f1fceaf7b82778bc1f24b39cc", //amazon kinesis client
"828bfa41ee3e7021583716feaa714647e4ba0c850342e298abe71245f6f2dd4a" //docker-java
)


@Test
fun `search single package `() = runTest {
val (apiPackages, duration) = measureTimedValue {
apiClient.getPackageInfoByIdHashes(idHashes = setOf(sampleIdHashes.first()))
}
assert(apiPackages.isNotEmpty())
timingMap["single package CF"] = duration.inWholeMilliseconds

}

@Test
fun `search multiple package cloudFront`() = runTest {

val (apiPackages, duration) = measureTimedValue {
apiClient.getPackageInfoByIdHashes(idHashes = sampleIdHashes)
}
assert(apiPackages.keys.size == sampleIdHashes.size)
assert(apiPackages.entries.isNotEmpty())
timingMap["multiple packages (${sampleIdHashes.size}) CF"] = duration.inWholeMilliseconds
}

@Test
fun `search single package on cloudFront that does not exist`() = runTest {
val (apiPackages, duration) = measureTimedValue {
apiClient.getPackageInfoByIdHashes(idHashes = setOf("invalid-hash"))
}
assert(apiPackages.isEmpty())
timingMap["not found package CF"] = duration.inWholeMilliseconds
}

}
44 changes: 20 additions & 24 deletions http/client/src/jvmTest/kotlin/CacheTests.kt
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ class CacheTests {
fun `PackageByID test Cache Hit`() = runTest(timeout = 30.seconds) {

val (apiClient, mockEngine, mockResponse) =
setupTestEnv<List<ApiPackage>>(resourceFilename = "package-info-by-ids-ktor.json")
setupTestEnv<ApiPackage>(resourceFilename = "package-info-by-ids-ktor.json")

val packageId = mockResponse.first().id
val packageId = mockResponse.id
// First request (populates cache)
val response1 = apiClient.getPackageInfoByIds(setOf(packageId))
assert(response1.values.firstOrNull()!!.id == packageId)
Expand All @@ -39,8 +39,7 @@ class CacheTests {
assert(response2.values.firstOrNull()!!.id == packageId)

// Ensure the mock engine wasn't called a second time.
val endpointsCalls =
mockEngine.getRequestCount(PackageSearchEndpoints.DEV.packageInfoByIdHashes)
val endpointsCalls = mockEngine.requestCountForGetIdHash(mockResponse.idHash)

mockEngine.close()
assertEquals(1, endpointsCalls)
Expand All @@ -51,11 +50,11 @@ class CacheTests {
fun `PackageByID test Cache Expired`() = runTest(timeout = 30.seconds) {

val testEnv =
setupTestEnv<List<ApiPackage>>(resourceFilename = "package-info-by-ids-ktor.json")
setupTestEnv<ApiPackage>(resourceFilename = "package-info-by-ids-ktor.json")

suspend fun getDBEntries() = testEnv.apiClient.packagesCacheCollection.await().iterateAll().toList()

val apiPackage = testEnv.mockResponse.first()
val apiPackage = testEnv.mockResponse

val expiredDBEntry = PackageCacheEntry(apiPackage.idHash, apiPackage, Clock.System.now().minus(7.days))

Expand All @@ -76,7 +75,7 @@ class CacheTests {
assert(elementInsideDB.size == 1)

// Ensure data has been retrieved from BE.
assertEquals(1, testEnv.mockEngine.getRequestCount(PackageSearchEndpoints.DEV.packageInfoByIdHashes))
assertEquals(1, testEnv.mockEngine.requestCountForGetIdHash(apiPackage.idHash))


// Second request (should hit cache)
Expand All @@ -86,16 +85,16 @@ class CacheTests {
assert(elementInsideDB.size == 1)

testEnv.mockEngine.close()
assertEquals(1, testEnv.mockEngine.getRequestCount(PackageSearchEndpoints.DEV.packageInfoByIdHashes))
assertEquals(1, testEnv.mockEngine.requestCountForGetIdHash(apiPackage.idHash))

}

@Test
fun `PackageByHash test Cache Hit`() = runTest(timeout = 30.seconds) {
val (apiClient, mockEngine, mockResponse) =
setupTestEnv<List<ApiPackage>>(resourceFilename = "package-info-by-ids-ktor.json")
setupTestEnv<ApiPackage>(resourceFilename = "package-info-by-ids-ktor.json")

val packageIdHash = mockResponse.first().idHash
val packageIdHash = mockResponse.idHash
// First request (populates cache)
val response1 = apiClient.getPackageInfoByIdHashes(setOf(packageIdHash))
assert(response1.values.firstOrNull()!!.idHash == packageIdHash)
Expand All @@ -106,7 +105,7 @@ class CacheTests {

// Ensure the mock engine wasn't called a second time.
val endpointsCalls =
mockEngine.getRequestCount(PackageSearchEndpoints.DEV.packageInfoByIdHashes)
mockEngine.requestCountForGetIdHash(idHash = packageIdHash)

mockEngine.close()
assertEquals(1, endpointsCalls)
Expand All @@ -115,14 +114,14 @@ class CacheTests {
@Test
fun `PackageById test cache not exhaustive`() = runTest(timeout = 30.seconds) {
val (apiClient, mockEngine, mockResponse) =
setupTestEnv<List<ApiPackage>>(resourceFilename = "package-info-by-ids-ktor.json")
setupTestEnv<ApiPackage>(resourceFilename = "package-info-by-ids-ktor.json")
//create cache entry
apiClient.getPackageInfoByIds(mockResponse.map { it.id }.toSet())
apiClient.getPackageInfoByIds(setOf(mockResponse.id))

apiClient.getPackageInfoByIds(mockResponse.map { it.id }.toSet() + "androidx.compose.runtime:runtime")
apiClient.getPackageInfoByIds(setOf(mockResponse.id) + "androidx.compose.runtime:runtime")

val endpointsCalls =
mockEngine.geRequestsFor(PackageSearchEndpoints.DEV.packageInfoByIdHashes)
mockEngine.geRequestsFor(PackageSearchEndpoints.DEV.packageInfoByIdHash)

mockEngine.close()
assertEquals(2, endpointsCalls.size)
Expand Down Expand Up @@ -151,7 +150,7 @@ class CacheTests {

@Test
fun `searchPackages test Cache Hit`() = runTest(timeout = 30.seconds) {
val (apiClient, mockEngine) = setupTestEnv<List<ApiPackage>>("package-info-by-ids-ktor.json")
val (apiClient, mockEngine) = setupTestEnv<List<ApiPackage>>("list-package-info-by-ids-ktor.json")
val queryString = "whatever"
repeat(3) {
apiClient.searchPackages(
Expand All @@ -171,7 +170,7 @@ class CacheTests {

@Test
fun `searchPackages non exhaustive cache`() = runTest(timeout = 30.seconds) {
val (apiClient, mockEngine) = setupTestEnv<List<ApiPackage>>("package-info-by-ids-ktor.json")
val (apiClient, mockEngine) = setupTestEnv<List<ApiPackage>>("list-package-info-by-ids-ktor.json")
val queryString = "whatever"
apiClient.searchPackages(
SearchPackagesRequest(
Expand All @@ -196,14 +195,11 @@ class CacheTests {

@Test
fun `refreshPackagesInfo smart cache`() = runTest(timeout = 30.seconds) {
val (apiClient, mockEngine, mockResponse) = setupTestEnv<List<ApiPackage>>("package-info-by-ids-ktor.json")
val (apiClient, mockEngine, mockResponse) =
setupTestEnv<List<ApiPackage>>("list-package-info-by-ids-ktor.json")

val refreshRequest = RefreshPackagesInfoRequest(
listOf(
RefreshPackagesInfoRequest.CacheRequest(
mockResponse.first().idHash
)
)
packages = listOf(RefreshPackagesInfoRequest.CacheRequest(mockResponse.first().idHash))
)

//ask to refresh package info for ktor hashID -> a new cache entry should be created or updated
Expand All @@ -216,7 +212,7 @@ class CacheTests {
apiClient.getPackageInfoByIds(setOf(mockResponse.first().id))

val endpointsCalls =
mockEngine.getRequestCount(PackageSearchEndpoints.DEV.packageInfoByIdHashes)
mockEngine.getRequestCount(PackageSearchEndpoints.DEV.packageInfoByIdHash)

mockEngine.close()
assertEquals(0, endpointsCalls)
Expand Down
Loading

0 comments on commit 1301d23

Please sign in to comment.