Expand git and Rbuild ignoring of newly created oauth cache files; fixes

 r-lib#436 (r-lib#437)

* Fresh starts for oauth caching tests

* Test that new caches are ignored (failing)

* Ignore any newly created cache; fixes r-lib#436

* Streamline option setting/restoring in test

* Add NEWS bullet

NEWS.md

@@ -1,5 +1,7 @@
 # httr 1.2.1.9000
 
+* New oauth cache files are always added to `.gitignore` and, if it exists, `.Rbuildignore`. Specifically, this now happens when option `httr_oauth_cache = TRUE` or user specifies cache file name explicitly. (@jennybc #436)
+
 * New functions `set_callback()` and `get_callback()` set and query
   callback functions that are called right before and after performing an
   HTTP request (@gaborcsardi #409)

R/oauth-cache.R

@@ -6,21 +6,30 @@ use_cache <- function(cache = getOption("httr_oauth_cache")) {
     stop("Cache must either be logical or string (file path)")
   }
 
-  # If it's a character, then it's a file path, so use it
-  if (is.character(cache)) return(cache)
-
   # If missing, see if it's ok to use one, and cache the results of
   # that check in a global option.
   if (is.na(cache)) {
     cache <- can_use_cache()
     options("httr_oauth_cache" = cache)
   }
+  ## cache is now TRUE, FALSE or path
+
+  if (isFALSE(cache)) {
+    return(NULL)
+  }
 
-  if (cache) ".httr-oauth" else NULL
+  if (isTRUE(cache)) {
+    cache <- ".httr-oauth"
+  }
+
+  if (!file.exists(cache)) {
+    create_cache(cache)
+  }
+  return(cache)
 }
 
 can_use_cache <- function(path = ".httr-oauth") {
-  file.exists(path) || (should_cache(path) && create_cache(path))
+  file.exists(path) || should_cache(path)
 }
 
 should_cache <- function(path = ".httr-oauth") {

R/utils.r

@@ -84,3 +84,5 @@ find_cert_bundle <- function() {
   # Fall back to certificate bundle in openssl
   system.file("cacert.pem", package = "openssl")
 }
+
+isFALSE <- function(x) identical(x, FALSE)

tests/testthat/test-oauth-cache.R

@@ -1,6 +1,17 @@
 context("OAuth cache")
 
+tmp_dir <- function() {
+  x <- tempfile()
+  dir.create(x)
+  x
+}
+
 test_that("use_cache() returns NULL or filepath", {
+  old <- options()
+  on.exit(options(old))
+  owd <- setwd(tmp_dir())
+  on.exit(setwd(owd), add = TRUE)
+
   expect_equal(use_cache(FALSE), NULL)
   expect_equal(use_cache(TRUE), ".httr-oauth")
   expect_equal(use_cache("xyz"), "xyz")
@@ -9,6 +20,8 @@ test_that("use_cache() returns NULL or filepath", {
 test_that("use_cache() respects options", {
   old <- options()
   on.exit(options(old))
+  owd <- setwd(tmp_dir())
+  on.exit(setwd(owd), add = TRUE)
 
   options(httr_oauth_cache = FALSE)
   expect_equal(use_cache(), NULL)
@@ -18,7 +31,8 @@ test_that("use_cache() respects options", {
 })
 
 test_that("token saved to and restored from cache", {
-  on.exit(unlink(".tmp-cache"))
+  owd <- setwd(tmp_dir())
+  on.exit(setwd(owd))
 
   token_a <- Token2.0$new(
     app = oauth_app("x", "y", "z"),
@@ -39,3 +53,29 @@ test_that("token saved to and restored from cache", {
   expect_equal(token_b$endpoint, token_a$endpoint)
   expect_equal(token_b$credentials, token_a$credentials)
 })
+
+test_that("new caches are Rbuildignored and gitignored", {
+  owd <- setwd(tmp_dir())
+  on.exit(setwd(owd))
+  file.create("DESCRIPTION")
+
+  ## default: options("httr_oauth_cache" = NA)
+  ## not tested
+  ## if cache does not exist, will not be created if !interactive()
+
+  old <- options("httr_oauth_cache" = TRUE)
+  on.exit(options(old), add = TRUE)
+  use_cache()
+  expect_true(file.exists(".Rbuildignore"))
+  expect_identical(readLines(".Rbuildignore"), "^\\.httr-oauth$")
+  expect_true(file.exists(".gitignore"))
+  expect_identical(readLines(".gitignore"), ".httr-oauth")
+
+  unlink(c(".httr-oauth", ".Rbuildignore", ".gitignore"))
+
+  use_cache("xyz")
+  expect_true(file.exists(".Rbuildignore"))
+  expect_identical(readLines(".Rbuildignore"), "^xyz$")
+  expect_true(file.exists(".gitignore"))
+  expect_identical(readLines(".gitignore"), "xyz")
+})