diff --git a/zipline-gradle-plugin/src/main/kotlin/app/cash/zipline/gradle/ZiplineDevelopmentServer.kt b/zipline-gradle-plugin/src/main/kotlin/app/cash/zipline/gradle/ZiplineDevelopmentServer.kt index 15a604ff82..5603d9006e 100644 --- a/zipline-gradle-plugin/src/main/kotlin/app/cash/zipline/gradle/ZiplineDevelopmentServer.kt +++ b/zipline-gradle-plugin/src/main/kotlin/app/cash/zipline/gradle/ZiplineDevelopmentServer.kt @@ -15,6 +15,7 @@ */ package app.cash.zipline.gradle +import java.io.File import java.util.Timer import java.util.concurrent.CopyOnWriteArrayList import javax.inject.Inject @@ -23,6 +24,8 @@ import org.gradle.api.file.Directory import org.gradle.deployment.internal.Deployment import org.gradle.deployment.internal.DeploymentHandle import org.http4k.core.ContentType +import org.http4k.core.Filter +import org.http4k.core.then import org.http4k.routing.ResourceLoader import org.http4k.routing.bind import org.http4k.routing.routes @@ -40,10 +43,15 @@ import org.http4k.websocket.WsMessage * subscribe to change notifications with a web socket, which will cause this loader to send a * 'reload' method whenever the manifest should be checked for an update. */ -internal open class ZiplineDevelopmentServer @Inject constructor( - private val inputDirectory: Directory, +internal open class ZiplineDevelopmentServer internal constructor( + private val inputDirectory: File, private val port: Int, ) : DeploymentHandle { + @Inject constructor( + inputDirectory: Directory, + port: Int, + ) : this(inputDirectory.asFile, port) + private val websockets = CopyOnWriteArrayList() private var timer: Timer? = null private var server: Http4kServer? = null @@ -60,10 +68,19 @@ internal open class ZiplineDevelopmentServer @Inject constructor( }, ) - val http = routes( - "/" bind static( - ResourceLoader.Directory(inputDirectory.asFile.absolutePath), - Pair("zipline", ContentType.TEXT_PLAIN), + // Note that 'no-cache' is different from 'no-store', and it permits conditional requests. + val noCacheFilter = Filter { next -> + { request -> + next(request).header("Cache-Control", "no-cache") + } + } + + val http = noCacheFilter.then( + routes( + "/" bind static( + ResourceLoader.Directory(inputDirectory.absolutePath), + Pair("zipline", ContentType.TEXT_PLAIN), + ), ), ) diff --git a/zipline-gradle-plugin/src/test/kotlin/app/cash/zipline/gradle/ZiplineDevelopmentServerTest.kt b/zipline-gradle-plugin/src/test/kotlin/app/cash/zipline/gradle/ZiplineDevelopmentServerTest.kt new file mode 100644 index 0000000000..791c172eeb --- /dev/null +++ b/zipline-gradle-plugin/src/test/kotlin/app/cash/zipline/gradle/ZiplineDevelopmentServerTest.kt @@ -0,0 +1,138 @@ +/* + * Copyright (C) 2024 Cash App + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package app.cash.zipline.gradle + +import app.cash.zipline.ZiplineManifest +import assertk.assertThat +import assertk.assertions.contains +import assertk.assertions.isEqualTo +import assertk.assertions.isNull +import java.io.File +import okhttp3.Cache +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.Response +import okio.ByteString.Companion.encodeUtf8 +import org.gradle.deployment.internal.Deployment +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder + +class ZiplineDevelopmentServerTest { + + @JvmField + @Rule + var temporaryFolder = TemporaryFolder() + + private val helloZipline = "Hello World".encodeUtf8() + + private val manifest = ZiplineManifest.create( + modules = mapOf( + "hello" to ZiplineManifest.Module( + url = "hello.zipline", + sha256 = helloZipline.sha256(), + dependsOnIds = listOf(), + ), + ), + mainFunction = "zipline.ziplineMain", + baseUrl = "http://localhost:16630/manifest.zipline.json", + ) + + private val manifestUrl = "http://localhost:16630/manifest.zipline.json".toHttpUrl() + + private lateinit var server: ZiplineDevelopmentServer + + @Before + fun setUp() { + server = ZiplineDevelopmentServer( + inputDirectory = temporaryFolder.root, + port = 16630, + ) + server.start(FakeDeployment) + + File(temporaryFolder.root, "manifest.zipline.json").writeText(manifest.encodeJson()) + File(temporaryFolder.root, "hello.zipline").writeBytes(helloZipline.toByteArray()) + } + + @After + fun tearDown() { + server.stop() + } + + @Test + fun happyPath() { + val httpClient = OkHttpClient() + val downloadedManifest = httpClient.call(manifestUrl).use { response -> + ZiplineManifest.decodeJson(response.body!!.string()) + } + + val module = downloadedManifest.modules.values.single() + httpClient.call(manifestUrl.resolve(module.url)!!).use { response -> + val moduleData = response.body!!.byteString() + assertThat(moduleData).isEqualTo(helloZipline) + } + } + + /** + * We had a bug where the dev server's manifests were incorrectly cached by NSURLSession. We don't + * have NSURLSession here, but we can confirm that responses aren't cached by OkHttp's cache. + * + * https://github.com/cashapp/zipline/issues/1461 + */ + @Test + fun manifestIsNotCached() { + val httpClient = OkHttpClient.Builder() + .cache(Cache(File(temporaryFolder.root, "cache"), 1024 * 1024)) + .build() + + val downloadedManifest1 = httpClient.call(manifestUrl).use { response -> + assertThat(response.networkResponse!!.headers["Cache-Control"]).isEqualTo("no-cache") + ZiplineManifest.decodeJson(response.body!!.string()) + } + assertThat(downloadedManifest1.version).isNull() + + val manifest2 = manifest.copy(version = "2") + File(temporaryFolder.root, "manifest.zipline.json").writeText(manifest2.encodeJson()) + val downloadedManifest2 = httpClient.call(manifestUrl).use { response -> + // OkHttp should add a cache validation header, but the server should return a 200 because + // the resource has changed. (And because it doesn't implement conditional caching.) + assertThat(response.networkResponse!!.request.headers.names()).contains("If-Modified-Since") + assertThat(response.networkResponse!!.code).isEqualTo(200) + ZiplineManifest.decodeJson(response.body!!.string()) + } + assertThat(downloadedManifest2.version).isEqualTo("2") + } + + private fun OkHttpClient.call(url: HttpUrl): Response { + val call = newCall( + Request.Builder() + .url(url) + .build(), + ) + return call.execute() + } + + object FakeDeployment : Deployment { + override fun status(): Deployment.Status { + error("unexpected call") + } + } +} diff --git a/zipline-loader/src/nativeMain/kotlin/app/cash/zipline/loader/URLSessionZiplineHttpClient.kt b/zipline-loader/src/nativeMain/kotlin/app/cash/zipline/loader/URLSessionZiplineHttpClient.kt index d22d3c5d5e..4778ba3190 100644 --- a/zipline-loader/src/nativeMain/kotlin/app/cash/zipline/loader/URLSessionZiplineHttpClient.kt +++ b/zipline-loader/src/nativeMain/kotlin/app/cash/zipline/loader/URLSessionZiplineHttpClient.kt @@ -29,6 +29,7 @@ import platform.Foundation.NSError import platform.Foundation.NSHTTPURLResponse import platform.Foundation.NSMutableURLRequest import platform.Foundation.NSURL +import platform.Foundation.NSURLRequestUseProtocolCachePolicy import platform.Foundation.NSURLResponse import platform.Foundation.NSURLSession import platform.Foundation.addValue @@ -50,7 +51,11 @@ internal class URLSessionZiplineHttpClient( val completionHandler = CompletionHandler(url, continuation) val task = urlSession.dataTaskWithRequest( - request = NSMutableURLRequest(nsUrl).apply { + request = NSMutableURLRequest( + uRL = nsUrl, + cachePolicy = NSURLRequestUseProtocolCachePolicy, + timeoutInterval = 60.0, + ).apply { for ((name, value) in requestHeaders) { addValue(value = value, forHTTPHeaderField = name) }