Skip to content

Commit ce26b5e

Browse files
committed
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. (cherry picked from commit 8b363bb)
1 parent 370ab1c commit ce26b5e

File tree

6 files changed

+61
-10
lines changed

6 files changed

+61
-10
lines changed
Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
1-
package com.gabrielfeo.gradle.enterprise.api.internal
1+
package com.gabrielfeo.gradle.enterprise.api
22

3-
import com.gabrielfeo.gradle.enterprise.api.Config
4-
import com.gabrielfeo.gradle.enterprise.api.GradleEnterpriseApi
3+
import com.gabrielfeo.gradle.enterprise.api.internal.*
54
import kotlinx.coroutines.test.runTest
6-
import okhttp3.HttpUrl.Companion.toHttpUrl
75
import org.junit.jupiter.api.assertDoesNotThrow
8-
import kotlin.test.assertEquals
9-
import kotlin.test.Test
6+
import kotlin.test.*
107

118
class GradleEnterpriseApiIntegrationTest {
129

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package com.gabrielfeo.gradle.enterprise.api.extension
2+
3+
import com.gabrielfeo.gradle.enterprise.api.*
4+
import com.gabrielfeo.gradle.enterprise.api.internal.*
5+
import kotlinx.coroutines.flow.collect
6+
import kotlinx.coroutines.flow.take
7+
import kotlinx.coroutines.test.runTest
8+
import org.junit.jupiter.api.*
9+
import org.junit.jupiter.api.Assertions.*
10+
11+
class BuildsApiExtensionsTest {
12+
13+
@Test
14+
fun getBuildsFlowUsesQueryInAllRequests() = runTest {
15+
env = RealEnv
16+
keychain = RealKeychain(RealSystemProperties)
17+
val recorder = RequestRecorder()
18+
val api = buildApi(recorder)
19+
api.buildsApi.getBuildsFlow(since = 0, query = "user:*").take(2000).collect()
20+
recorder.requests.forEachIndexed { i, request ->
21+
assertEquals("user:*", request.url.queryParameter("query"), "[$i]")
22+
}
23+
api.shutdown()
24+
}
25+
26+
private fun buildApi(recorder: RequestRecorder) =
27+
GradleEnterpriseApi.newInstance(
28+
config = Config(
29+
clientBuilder = recorder.clientBuilder(),
30+
cacheConfig = Config.CacheConfig(cacheEnabled = false),
31+
)
32+
)
33+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package com.gabrielfeo.gradle.enterprise.api.extension
2+
3+
import okhttp3.OkHttpClient
4+
import okhttp3.Request
5+
import java.util.*
6+
7+
class RequestRecorder {
8+
9+
val requests = LinkedList<Request>()
10+
11+
fun clientBuilder() = OkHttpClient.Builder()
12+
.addNetworkInterceptor {
13+
requests += it.request()
14+
it.proceed(it.request())
15+
}
16+
}

library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/extension/BuildsApiExtensions.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ fun BuildsApi.getBuildsFlow(
4646
maxWaitSecs = maxWaitSecs,
4747
maxBuilds = buildsPerPage,
4848
)
49-
val pagedBuilds = firstBuilds.asFlow().pagedUntilLastBuild(api, buildsPerPage)
49+
val pagedBuilds = firstBuilds.asFlow().pagedUntilLastBuild(api, query, buildsPerPage)
5050
emitAll(pagedBuilds)
5151
}
5252
}

library/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/pagedUntilLastBuild.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import kotlinx.coroutines.flow.*
1010
*/
1111
internal fun Flow<Build>.pagedUntilLastBuild(
1212
api: BuildsApi,
13+
query: String?,
1314
buildsPerPage: Int,
1415
): Flow<Build> {
1516
val firstBuilds = this
@@ -22,7 +23,11 @@ internal fun Flow<Build>.pagedUntilLastBuild(
2223
if (lastBuildId.isEmpty()) {
2324
return@flow
2425
} else while (true) {
25-
val builds = api.getBuilds(fromBuild = lastBuildId, maxBuilds = buildsPerPage)
26+
val builds = api.getBuilds(
27+
fromBuild = lastBuildId,
28+
query = query,
29+
maxBuilds = buildsPerPage,
30+
)
2631
emitAll(builds.asFlow())
2732
when {
2833
builds.isEmpty() || builds.size < buildsPerPage -> break

library/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/operator/PagedUntilLastBuildTest.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class PagedUntilLastBuildTest {
3030
fun `Pages and stops once API sends less than maxBuilds`() = runTest {
3131
val channel = Channel<Build>(Channel.RENDEZVOUS)
3232
flowOf(api.builds[0], api.builds[1])
33-
.pagedUntilLastBuild(api, buildsPerPage = 4)
33+
.pagedUntilLastBuild(api, query = null, buildsPerPage = 4)
3434
.onEach { channel.send(it) }
3535
.launchIn(this)
3636
assertEquals(api.builds[0], channel.receive())
@@ -51,7 +51,7 @@ class PagedUntilLastBuildTest {
5151
fun `Pages and stops once API sends empty list`() = runTest {
5252
val channel = Channel<Build>(Channel.RENDEZVOUS)
5353
flowOf(api.builds[0])
54-
.pagedUntilLastBuild(api, buildsPerPage = 2)
54+
.pagedUntilLastBuild(api, query = null, buildsPerPage = 2)
5555
.onEach { channel.send(it) }
5656
.launchIn(this)
5757
// should wait until original Flow is collected before requesting more builds

0 commit comments

Comments
 (0)