From 0084415a2773d6e7985d4f95d86e21f67da024d5 Mon Sep 17 00:00:00 2001
From: Rich FitzJohn <r.fitzjohn@imperial.ac.uk>
Date: Fri, 18 Oct 2024 16:57:23 +0100
Subject: [PATCH] More ways that allow_remote should be controlled

---
 R/query_search.R                   |  9 ++++++---
 man/orderly_search_options.Rd      |  4 ++--
 tests/testthat/test-query-search.R | 12 ++++++++++++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/R/query_search.R b/R/query_search.R
index 5eb55756..918b4f7e 100644
--- a/R/query_search.R
+++ b/R/query_search.R
@@ -57,8 +57,8 @@ orderly_search <- function(expr, name = NULL, scope = NULL, subquery = NULL,
 ##'   only to a location that we have metadata from). If this is
 ##'   `TRUE`, then in conjunction with [orderly2::orderly_dependency]
 ##'   you might pull a large quantity of data.  The default `NULL` is
-##'   `TRUE` if locations are listed explicitly as a character vector
-##'   in the `location` argument, or if you have specified
+##'   `TRUE` if remote locations are listed explicitly as a character
+##'   vector in the `location` argument, or if you have specified
 ##'   `pull_metadata = TRUE`, otherwise `FALSE`.
 ##'
 ##' @param pull_metadata Logical, indicating if we should pull
@@ -82,9 +82,12 @@ orderly_search_options <- function(location = NULL,
   if (!is.null(location)) {
     assert_character(location)
   }
+  has_remote_location <- !is.null(location) &&
+    length(setdiff(location, c("local", "orphan")) > 0)
+
   assert_scalar_logical(pull_metadata)
   if (is.null(allow_remote)) {
-    allow_remote <- !is.null(location) || pull_metadata
+    allow_remote <- has_remote_location || pull_metadata
   } else {
     assert_scalar_logical(allow_remote)
   }
diff --git a/man/orderly_search_options.Rd b/man/orderly_search_options.Rd
index 7e138790..4175535f 100644
--- a/man/orderly_search_options.Rd
+++ b/man/orderly_search_options.Rd
@@ -19,8 +19,8 @@ to be found that are not currently unpacked (i.e., are known
 only to a location that we have metadata from). If this is
 \code{TRUE}, then in conjunction with \link{orderly_dependency}
 you might pull a large quantity of data.  The default \code{NULL} is
-\code{TRUE} if locations are listed explicitly as a character vector
-in the \code{location} argument, or if you have specified
+\code{TRUE} if remote locations are listed explicitly as a character
+vector in the \code{location} argument, or if you have specified
 \code{pull_metadata = TRUE}, otherwise \code{FALSE}.}
 
 \item{pull_metadata}{Logical, indicating if we should pull
diff --git a/tests/testthat/test-query-search.R b/tests/testthat/test-query-search.R
index 5cfcd704..bcd2e442 100644
--- a/tests/testthat/test-query-search.R
+++ b/tests/testthat/test-query-search.R
@@ -26,6 +26,18 @@ test_that("pull_metadata implies allow_remote", {
 })
 
 
+test_that("nontrivial location implies allow_remote", {
+  expect_false(orderly_search_options(location = NULL)$allow_remote)
+  expect_false(orderly_search_options(location = "local")$allow_remote)
+  expect_false(
+    orderly_search_options(location = c("local", "orphan"))$allow_remote)
+  expect_true(
+    orderly_search_options(location = "server")$allow_remote)
+  expect_true(
+    orderly_search_options(location = c("local", "server"))$allow_remote)
+})
+
+
 test_that("can convert into search options", {
   opts <- orderly_search_options(location = "x",
                                  allow_remote = FALSE,