Combined new and cached headers

R/req-cache.R

@@ -131,9 +131,11 @@ cache_post_fetch <- function(req, resp, path = NULL) {
     if (debug) cli::cli_text("Cached value still ok; retrieving body from cache")
 
     # Replace body with cached result
-    cache_body <- cache_body(req, path)
-    resp$body <- cache_body$body
-    resp$headers[["content-type"]] <- cache_body$`content-type`
+    resp$body <- cache_body(req, path)
+
+    # Combine headers
+    resp$headers <- cache_headers(req, resp)
+
     resp
   } else if (resp_is_cacheable(resp)) {
     if (debug) cli::cli_text("Saving response to cache {.val {hash(req$url)}}")
@@ -145,22 +147,24 @@ cache_post_fetch <- function(req, resp, path = NULL) {
 }
 
 cache_body <- function(req, path = NULL) {
-  cache_resp <- cache_get(req)
-  body <- cache_resp$body
-  content_type <- resp_header(cache_resp, "content-type")
-  out <- list(body = body, `content-type` = content_type)
+  body <- cache_get(req)$body
 
   if (is.null(path)) {
-    return(out)
+    return(body)
   }
 
   if (is_path(body)) {
     file.copy(body, path, overwrite = TRUE)
   } else {
     writeBin(body, path)
   }
-  out$body <- new_path(path)
-  out
+  new_path(path)
+}
+
+cache_headers <- function(req, resp) {
+  # https://www.rfc-editor.org/rfc/rfc7232#section-4.1
+  cached_headers <- cache_get(req)$headers
+  as_headers(modify_list(cached_headers, !!!resp$headers))
 }
 
 # Caching headers ---------------------------------------------------------

tests/testthat/test-req-cache.R

@@ -100,22 +100,40 @@ test_that("can get and set from cache", {
   req <- request("http://example.com") %>% req_cache(tempfile())
   resp <- response(
     200,
-    headers = list(Etag = "ABC", `content-type` = "application/json"),
+    headers = list(
+      Etag = "ABC",
+      `content-type` = "application/json",
+      other = "header"
+    ),
     body = charToRaw(jsonlite::toJSON(list(a = jsonlite::unbox(1))))
   )
 
+  cached_resp <- response(
+    304,
+    headers = list(
+      Etag = "DEF",
+      other = "new"
+    )
+  )
+
   expect_false(cache_exists(req))
   cache_set(req, resp)
   expect_true(cache_exists(req))
   expect_equal(cache_get(req), resp)
 
+  # Uses new headers if available, otherwise cached headers
+  out_headers <- cache_headers(req, cached_resp)
+  expect_equal(out_headers$`content-type`, "application/json")
+  expect_equal(out_headers$Etag, "DEF")
+  expect_equal(out_headers$other, "new")
+
   # If path is null can leave resp as is
-  expect_equal(cache_body(req, NULL), list(body = resp$body, `content-type` = "application/json"))
+  expect_equal(cache_body(req, NULL), resp$body)
   expect_equal(resp_body_json(cache_get(req)), list(a = 1L))
   # If path is set, need to save to path
   path <- tempfile()
   body <- cache_body(req, path)
-  expect_equal(body, list(body = new_path(path), `content-type` = "application/json"))
+  expect_equal(body, new_path(path))
   expect_equal(readLines(path, warn = FALSE), rawToChar(resp$body))
 })
 
@@ -133,11 +151,11 @@ test_that("handles responses with files", {
 
   # If path is null, just leave body as is, since req_body() already
   # papers over the differences
-  expect_equal(cache_body(req, NULL)$body, new_path(body_path))
+  expect_equal(cache_body(req, NULL), new_path(body_path))
 
   # If path is not null, copy to desired location, and update body
   path2 <- tempfile()
-  body <- cache_body(req, path2)$body
+  body <- cache_body(req, path2)
   expect_equal(readLines(body), "Hi there")
   expect_equal(body, new_path(path2))
 })