Skip to content

Commit

Permalink
Merge pull request #1462 from cashapp/jwilson.1113.caching
Browse files Browse the repository at this point in the history
Don't let clients cache the dev server's responses
  • Loading branch information
squarejesse authored Nov 14, 2024
2 parents 2f1b935 + 6d040f0 commit 75e9827
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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<Websocket>()
private var timer: Timer? = null
private var server: Http4kServer? = null
Expand All @@ -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),
),
),
)

Expand Down
Original file line number Diff line number Diff line change
@@ -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")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,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
Expand All @@ -44,7 +45,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)
}
Expand Down

0 comments on commit 75e9827

Please sign in to comment.