Move from rappdirs cache dir to tools::R_user_dir() (#800)

---------

Co-authored-by: Hadley Wickham <h.wickham@gmail.com>

Author: Bisaloo

DESCRIPTION

@@ -23,7 +23,6 @@ Imports:
     magrittr,
     openssl,
     R6,
-    rappdirs,
     rlang (>= 1.3.0),
     vctrs (>= 0.6.3),
     withr
@@ -44,6 +43,7 @@ Suggests:
     otelsdk (>= 0.2.0),
     paws.common (>= 0.8.0),
     promises,
+    rappdirs,
     rmarkdown,
     testthat (>= 3.1.8),
     tibble,

NEWS.md

@@ -2,6 +2,7 @@
 
 * Fixed OAuth token cache pruning so that it actually matches the encrypted `.rds.enc` files written to disk; previously the pruning pattern only matched an unencrypted `.rds` file that was never created, so cached tokens were never automatically deleted regardless of age.
 * httr2 now requires rlang >= 1.3.0, which changes the hash used to name files cached by `req_cache()` and on-disk OAuth token caches (e.g. from `req_oauth_auth_code(cache_disk = TRUE)`). Existing cached files won't match the new hash, so they'll be silently ignored (triggering a normal cache miss/re-authentication) and cleaned up over time by the usual pruning rules; you can also delete them manually.
+* `oauth_cache_path()` now defaults to a standard R cache directory (via `tools::R_user_dir()`). Because this release also changes the hash used for cache filenames, existing OAuth tokens will generally not be reused and you may need to authenticate once after upgrading. New tokens are written to the new location, and obsolete tokens in both the old and new locations are removed by the usual pruning rules. httr2 no longer requires the rappdirs package (#800).
 * `req_oauth_*()` gains an `expiry_margin` argument to control how early cached OAuth tokens are treated as expired; the default margin increases from 5 to 30 seconds (@zacdav-db, #860).
 
 # httr2 1.2.3

R/oauth.R

@@ -210,9 +210,12 @@ cache_disk <- function(client, key = NULL) {
 }
 
 # Update req_oauth_auth_code() docs if change default from 30
-cache_disk_prune <- function(days = 30, path = oauth_cache_path()) {
+cache_disk_prune <- function(
+  days = 30,
+  paths = c(oauth_cache_path(), oauth_cache_path_legacy())
+) {
   files <- dir(
-    path,
+    paths,
     recursive = TRUE,
     full.names = TRUE,
     pattern = "-token\\.rds\\.enc$"
@@ -232,13 +235,35 @@ cache_disk_prune <- function(days = 30, path = oauth_cache_path()) {
 #' @export
 oauth_cache_path <- function() {
   path <- Sys.getenv("HTTR2_OAUTH_CACHE")
-  if (path != "") {
-    return(path)
+  if (nzchar(path)) {
+    path
+  } else {
+    tools::R_user_dir("httr2", which = "cache")
   }
-
-  rappdirs::user_cache_dir("httr2")
 }
+# Equivalent to rappdirs::user_cache_dir("httr2"), inlined so httr2 doesn't
+# depend on rappdirs solely to find tokens cached by older versions. The
+# appname is nested twice and gains a "Cache" subdir on Windows because that's
+# what rappdirs did with its default `appauthor` and `opinion` arguments.
+oauth_cache_path_legacy <- function() {
+  base <- Sys.getenv("R_USER_CACHE_DIR")
+  if (nzchar(base)) {
+    if (.Platform$OS.type == "windows") {
+      return(file.path(base, "httr2", "httr2", "Cache"))
+    } else {
+      return(file.path(base, "httr2"))
+    }
+  }
 
+  if (.Platform$OS.type == "windows") {
+    base <- Sys.getenv("LOCALAPPDATA", Sys.getenv("APPDATA"))
+    file.path(base, "httr2", "httr2", "Cache")
+  } else if (Sys.info()[["sysname"]] == "Darwin") {
+    "~/Library/Caches/httr2"
+  } else {
+    file.path(Sys.getenv("XDG_CACHE_HOME", "~/.cache"), "httr2")
+  }
+}
 
 #' Clear OAuth cache
 #'

tests/testthat/test-oauth.R

@@ -216,9 +216,57 @@ test_that("can prune old files", {
   expect_equal(dir(path), "a-token.rds.enc")
 })
 
+test_that("prunes old files from both new and legacy locations", {
+  new_path <- withr::local_tempdir()
+  legacy_path <- withr::local_tempdir()
+  local_mocked_bindings(
+    oauth_cache_path = function() new_path,
+    oauth_cache_path_legacy = function() legacy_path
+  )
+
+  touch(file.path(new_path, "a-token.rds.enc"), Sys.time() - 86400 * 1)
+  touch(file.path(new_path, "b-token.rds.enc"), Sys.time() - 86400 * 2)
+  touch(file.path(legacy_path, "a-token.rds.enc"), Sys.time() - 86400 * 1)
+  touch(file.path(legacy_path, "b-token.rds.enc"), Sys.time() - 86400 * 2)
+
+  cache_disk_prune(2)
+
+  expect_equal(dir(new_path), "a-token.rds.enc")
+  expect_equal(dir(legacy_path), "a-token.rds.enc")
+})
+
 # cache_path --------------------------------------------------------------
 
 test_that("can override path with env var", {
   withr::local_envvar("HTTR2_OAUTH_CACHE" = "/tmp")
   expect_equal(oauth_cache_path(), "/tmp")
 })
+
+test_that("inlined legacy path matches rappdirs", {
+  path <- oauth_cache_path_legacy()
+  rappdirs_path <- rappdirs::user_cache_dir("httr2")
+
+  if (.Platform$OS.type == "windows") {
+    # rappdirs uses the CSIDL API, which can return an 8.3 short form of
+    # the user's home directory, while our env-var based path uses the
+    # long form. Both refer to the same directory, so convert to the
+    # (existing) directory's canonical short form before comparing.
+    dir.create(path, recursive = TRUE, showWarnings = FALSE)
+    withr::defer(unlink(path, recursive = TRUE))
+    path <- utils::shortPathName(path)
+    rappdirs_path <- utils::shortPathName(rappdirs_path)
+  }
+
+  expect_equal(
+    normalizePath(path, mustWork = FALSE),
+    normalizePath(rappdirs_path, mustWork = FALSE)
+  )
+})
+
+test_that("legacy path respects R_USER_CACHE_DIR", {
+  path <- withr::local_tempdir()
+  withr::local_envvar("R_USER_CACHE_DIR" = path)
+
+  expected <- rappdirs::user_cache_dir("httr2")
+  expect_equal(oauth_cache_path_legacy(), expected)
+})