From 8b363bb5ff019af21be7086af5b500c339e18edf Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Sat, 16 Dec 2023 01:44:36 +0000 Subject: [PATCH] Fix paging requests when using advanced query (#134) When using `query` in `BuildsApi.getBuildsFlow`, paging responses would be incorrect because requests after the 1st would be missing the `query` parameter. For example: ``` ?since=0&query="user:gabriel.feo"&maxBuilds=1000 ?fromBuild=X&maxBuilds=1000 ... ``` Also move `GradleEnterpriseApiIntegrationTest` to correct package. --- .../GradleEnterpriseApiIntegrationTest.kt | 9 ++--- .../api/extension/BuildsApiExtensionsTest.kt | 33 +++++++++++++++++++ .../api/extension/RequestRecorder.kt | 16 +++++++++ .../api/extension/BuildsApiExtensions.kt | 2 +- .../internal/operator/pagedUntilLastBuild.kt | 7 +++- .../operator/PagedUntilLastBuildTest.kt | 4 +-- 6 files changed, 61 insertions(+), 10 deletions(-) rename library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/{internal => }/GradleEnterpriseApiIntegrationTest.kt (77%) create mode 100644 library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensionsTest.kt create mode 100644 library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/RequestRecorder.kt diff --git a/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/GradleEnterpriseApiIntegrationTest.kt b/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/GradleEnterpriseApiIntegrationTest.kt similarity index 77% rename from library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/GradleEnterpriseApiIntegrationTest.kt rename to library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/GradleEnterpriseApiIntegrationTest.kt index 3e5f5daa..f9e52a41 100644 --- a/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/GradleEnterpriseApiIntegrationTest.kt +++ b/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/GradleEnterpriseApiIntegrationTest.kt @@ -1,12 +1,9 @@ -package com.gabrielfeo.gradle.enterprise.api.internal +package com.gabrielfeo.gradle.enterprise.api -import com.gabrielfeo.gradle.enterprise.api.Config -import com.gabrielfeo.gradle.enterprise.api.GradleEnterpriseApi +import com.gabrielfeo.gradle.enterprise.api.internal.* import kotlinx.coroutines.test.runTest -import okhttp3.HttpUrl.Companion.toHttpUrl import org.junit.jupiter.api.assertDoesNotThrow -import kotlin.test.assertEquals -import kotlin.test.Test +import kotlin.test.* class GradleEnterpriseApiIntegrationTest { diff --git a/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensionsTest.kt b/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensionsTest.kt new file mode 100644 index 00000000..c02d3ca4 --- /dev/null +++ b/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensionsTest.kt @@ -0,0 +1,33 @@ +package com.gabrielfeo.gradle.enterprise.api.extension + +import com.gabrielfeo.gradle.enterprise.api.* +import com.gabrielfeo.gradle.enterprise.api.internal.* +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.take +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.* +import org.junit.jupiter.api.Assertions.* + +class BuildsApiExtensionsTest { + + @Test + fun getBuildsFlowUsesQueryInAllRequests() = runTest { + env = RealEnv + keychain = RealKeychain(RealSystemProperties) + val recorder = RequestRecorder() + val api = buildApi(recorder) + api.buildsApi.getBuildsFlow(since = 0, query = "user:*").take(2000).collect() + recorder.requests.forEachIndexed { i, request -> + assertEquals("user:*", request.url.queryParameter("query"), "[$i]") + } + api.shutdown() + } + + private fun buildApi(recorder: RequestRecorder) = + GradleEnterpriseApi.newInstance( + config = Config( + clientBuilder = recorder.clientBuilder(), + cacheConfig = Config.CacheConfig(cacheEnabled = false), + ) + ) +} \ No newline at end of file diff --git a/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/RequestRecorder.kt b/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/RequestRecorder.kt new file mode 100644 index 00000000..a0679cfa --- /dev/null +++ b/library/src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/RequestRecorder.kt @@ -0,0 +1,16 @@ +package com.gabrielfeo.gradle.enterprise.api.extension + +import okhttp3.OkHttpClient +import okhttp3.Request +import java.util.* + +class RequestRecorder { + + val requests = LinkedList() + + fun clientBuilder() = OkHttpClient.Builder() + .addNetworkInterceptor { + requests += it.request() + it.proceed(it.request()) + } +} \ No newline at end of file diff --git a/library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensions.kt b/library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensions.kt index ff58d9cd..fc2e9389 100644 --- a/library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensions.kt +++ b/library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensions.kt @@ -46,7 +46,7 @@ fun BuildsApi.getBuildsFlow( maxWaitSecs = maxWaitSecs, maxBuilds = buildsPerPage, ) - val pagedBuilds = firstBuilds.asFlow().pagedUntilLastBuild(api, buildsPerPage) + val pagedBuilds = firstBuilds.asFlow().pagedUntilLastBuild(api, query, buildsPerPage) emitAll(pagedBuilds) } } diff --git a/library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/pagedUntilLastBuild.kt b/library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/pagedUntilLastBuild.kt index a4b534f7..e13ac6cd 100644 --- a/library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/pagedUntilLastBuild.kt +++ b/library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/pagedUntilLastBuild.kt @@ -10,6 +10,7 @@ import kotlinx.coroutines.flow.* */ internal fun Flow.pagedUntilLastBuild( api: BuildsApi, + query: String?, buildsPerPage: Int, ): Flow { val firstBuilds = this @@ -22,7 +23,11 @@ internal fun Flow.pagedUntilLastBuild( if (lastBuildId.isEmpty()) { return@flow } else while (true) { - val builds = api.getBuilds(fromBuild = lastBuildId, maxBuilds = buildsPerPage) + val builds = api.getBuilds( + fromBuild = lastBuildId, + query = query, + maxBuilds = buildsPerPage, + ) emitAll(builds.asFlow()) when { builds.isEmpty() || builds.size < buildsPerPage -> break diff --git a/library/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/PagedUntilLastBuildTest.kt b/library/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/PagedUntilLastBuildTest.kt index 410b40dc..471fed52 100644 --- a/library/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/PagedUntilLastBuildTest.kt +++ b/library/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/PagedUntilLastBuildTest.kt @@ -30,7 +30,7 @@ class PagedUntilLastBuildTest { fun `Pages and stops once API sends less than maxBuilds`() = runTest { val channel = Channel(Channel.RENDEZVOUS) flowOf(api.builds[0], api.builds[1]) - .pagedUntilLastBuild(api, buildsPerPage = 4) + .pagedUntilLastBuild(api, query = null, buildsPerPage = 4) .onEach { channel.send(it) } .launchIn(this) assertEquals(api.builds[0], channel.receive()) @@ -51,7 +51,7 @@ class PagedUntilLastBuildTest { fun `Pages and stops once API sends empty list`() = runTest { val channel = Channel(Channel.RENDEZVOUS) flowOf(api.builds[0]) - .pagedUntilLastBuild(api, buildsPerPage = 2) + .pagedUntilLastBuild(api, query = null, buildsPerPage = 2) .onEach { channel.send(it) } .launchIn(this) // should wait until original Flow is collected before requesting more builds