Skip to content

Commit

Permalink
Fix paging requests when using advanced query (#134)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
gabrielfeo committed Dec 16, 2023
1 parent 370ab1c commit ce26b5e
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -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 {

Expand Down
Original file line number Diff line number Diff line change
@@ -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),
)
)
}
Original file line number Diff line number Diff line change
@@ -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<Request>()

fun clientBuilder() = OkHttpClient.Builder()
.addNetworkInterceptor {
requests += it.request()
it.proceed(it.request())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import kotlinx.coroutines.flow.*
*/
internal fun Flow<Build>.pagedUntilLastBuild(
api: BuildsApi,
query: String?,
buildsPerPage: Int,
): Flow<Build> {
val firstBuilds = this
Expand All @@ -22,7 +23,11 @@ internal fun Flow<Build>.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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class PagedUntilLastBuildTest {
fun `Pages and stops once API sends less than maxBuilds`() = runTest {
val channel = Channel<Build>(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())
Expand All @@ -51,7 +51,7 @@ class PagedUntilLastBuildTest {
fun `Pages and stops once API sends empty list`() = runTest {
val channel = Channel<Build>(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
Expand Down

0 comments on commit ce26b5e

Please sign in to comment.