Merge pull request #125 from mrc-ide/use-files-from-disk

Use existing files on disk instead of pushing afresh

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: orderly2
 Title: Orderly Next Generation
-Version: 1.99.12
+Version: 1.99.13
 Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"),
                     email = "[email protected]"),
              person("Robert", "Ashton", role = "aut"),

R/location.R

@@ -590,7 +590,6 @@ location_build_pull_plan_packets <- function(packet_id, recursive, root, call) {
   index <- root$index$data()
   if (recursive) {
     full <- find_all_dependencies(packet_id, index$metadata)
-    n_extra <- length(full) - length(packet_id)
   } else {
     full <- packet_id
   }
@@ -801,10 +800,17 @@ location_pull_files <- function(files, root) {
     cli::cli_alert_info("Looking for suitable files already on disk")
     store <- temporary_filestore(root)
     cleanup <- function() store$destroy()
-    for (hash in files$hash) {
+    on_disk <- vlapply(files$hash, function(hash) {
+      exists <- FALSE
       if (!is.null(path <- find_file_by_hash(root, hash))) {
         store$put(path, hash)
+        exists <- TRUE
       }
+      exists
+    })
+    if (any(on_disk)) {
+      cli::cli_alert_success("Found {sum(on_disk)} file{?s} on disk")
+      files <- files[!on_disk, , drop = FALSE]
     }
   }
 

tests/testthat/test-location.R

@@ -927,6 +927,26 @@ test_that("skip files in the file store", {
 })
 
 
+test_that("skip files known elsewhere on disk", {
+  root <- list()
+  for (name in c("src", "dst")) {
+    root[[name]] <- create_temporary_root(use_file_store = FALSE)
+  }
+
+  id <- create_random_packet_chain(root$src, 3)
+  orderly_location_add("src", "path", list(path = root$src$path),
+                       root = root$dst)
+  orderly_location_pull_metadata(root = root$dst)
+  suppressMessages(orderly_location_pull_packet(id[[1]], root = root$dst))
+
+  res <- testthat::evaluate_promise(
+    orderly_location_pull_packet(id[[2]], root = root$dst))
+  expect_match(res$messages, "Found 1 file on disk", all = FALSE)
+  expect_match(res$messages, "Need to fetch 2 files.+from 1 location",
+               all = FALSE)
+})
+
+
 test_that("can prune orphans from tree", {
   root <- list()
   for (name in c("here", "there")) {