fix: Delete content from failed sys-deps and Quarto deploys (0.8.10)

DESCRIPTION

@@ -1,7 +1,7 @@
 Type: Package
 Package: audit.connect
 Title: Posit Connect Health Check
-Version: 0.8.9
+Version: 0.8.10
 Authors@R:
     person("Jumping", "Rivers", , "info@jumpingrivers.com", role = c("aut", "cre"))
 Description: Posit Connect Health Check. Deploys various content types to

NEWS.md

@@ -1,3 +1,10 @@
+# audit.connect 0.8.10
+
+## Bug fixes and minor improvements
+
+* A failed build of the system-libraries endpoint no longer leaves it on Connect (#67).
+* A Quarto deploy that fails before Quarto records it is deleted by title, and its temporary folder is always removed (#67).
+
 # audit.connect 0.8.9
 
 ## Bug fixes and minor improvements

R/check_sys_deps.R

@@ -6,6 +6,7 @@
 #' @noRd
 check_sys_deps = function(debug_level = 0:2) {
   debug_level = get_debug_level(debug_level)
+  suppress = get_suppress(debug_level)
   .connect$timezone = NULL
 
   app_dir = file.path(tempdir(), "pkg")
@@ -21,7 +22,12 @@ check_sys_deps = function(debug_level = 0:2) {
     setup_plumber_sys_deps_endpoint(app_dir, debug_level),
     silent = TRUE
   )
-  if ("try-error" %in% class(content)) {
+  # Poll here, so a failed build still leaves content for cleanup_plumber() to delete
+  built = content
+  if (!inherits(content, "try-error")) {
+    built = try(suppress(connectapi::poll_task(content)), silent = TRUE)
+  }
+  if (inherits(built, "try-error")) {
     cli::cli_alert_info("Can't deploy plumber end point - odd?")
     cli::cli_alert_info("Check the logs on {get_server()}")
     cli::cli_alert_info(
@@ -51,9 +57,7 @@ setup_plumber_sys_deps_endpoint = function(app_dir, debug_level) {
   ))
   bundle = suppress(connectapi::bundle_dir(app_dir))
   name = paste("UAT_check_package-", Sys.Date(), sep = "_")
-  content = suppress(connectapi::deploy(client, bundle, name = name))
-  suppress(connectapi::poll_task(content))
-  content
+  suppress(connectapi::deploy(client, bundle, name = name))
 }
 
 # Extracting the URL differs on first deployment and subsequent deployments

R/deploy_python.R

@@ -61,25 +61,30 @@ cleanup_python = function(tmp_dir, debug_level, title) {
   }
   suppress = get_suppress(debug_level)
   fs::dir_delete(tmp_dir)
-  # map in case we make multiple mistakes
   # fmt: skip
   con = suppress( # nolint
     connectapi::connect(
       server = get_server(),
       api_key = get_token()
     )
   )
+  delete_content_by_title(con, title, suppress)
+  invisible(NULL)
+}
+
+# For deploys that fail before they say what they created
+delete_content_by_title = function(con, title, suppress) {
   # Only the auditor's content, so nobody else's item with the same title is deleted
   content = suppress(connectapi::get_content(con, owner_guid = con$me()$guid))
   guid = content$guid[content$title %in% title]
+  # map in case we make multiple mistakes
   purrr::map(
     guid,
     ~ {
       item = connectapi::content_item(con, .x)
       suppress(connectapi::content_delete(item, force = TRUE))
     }
   )
-
   invisible(NULL)
 }
 

R/deploy_quarto.R

@@ -6,10 +6,10 @@ deploy_quarto = function(
   suppress = get_suppress(debug_level)
 
   tmp_dir = file.path(tempdir(), "quarto", basename(quarto_dir))
-  on.exit(cleanup_quarto(tmp_dir, debug_level))
+  title = paste("UAT: Quarto", Sys.Date())
+  on.exit(cleanup_quarto(tmp_dir, debug_level, title = title))
   dir.create(tmp_dir, showWarnings = FALSE)
   fs::dir_copy(quarto_dir, tmp_dir, overwrite = TRUE)
-  title = paste("UAT: Quarto", Sys.Date())
   # quarto uses rsconnect::accounts. So, looks up the server in list of accounts
   if (is.null(account)) {
     account = get_account()
@@ -27,21 +27,25 @@ deploy_quarto = function(
   invisible(has_deployed)
 }
 
-cleanup_quarto = function(tmp_dir, debug_level) {
+cleanup_quarto = function(tmp_dir, debug_level, title) {
   if (debug_level == 2) {
     return(NULL)
   }
+  on.exit(fs::dir_delete(tmp_dir))
   suppress = get_suppress(debug_level)
+  con = suppress(connectapi::connect(
+    server = get_server(),
+    api_key = get_token()
+  ))
   fnames = list.files(tmp_dir, recursive = TRUE, full.names = TRUE)
   dcf = fnames[stringr::str_detect(fnames, "(.*)\\.dcf$")]
+  # quarto_publish_doc() writes the deployment record only once the deploy succeeds
+  if (length(dcf) == 0L) {
+    return(delete_content_by_title(con, title, suppress))
+  }
   dcf_contents = read.dcf(dcf)
   url = dcf_contents[1, "url"]
   guid = stringr::str_match_all(url, "content/(.*)/")[[1]][, 2]
-  con = suppress(connectapi::connect(
-    server = get_server(),
-    api_key = get_token()
-  ))
   item = connectapi::content_item(con, guid = guid)
   suppress(connectapi::content_delete(item, force = TRUE))
-  fs::dir_delete(tmp_dir)
 }

tests/testthat/test-cleanup.R

@@ -31,3 +31,44 @@ test_that("cleanup_plumber() skips content that was never deployed", {
   expect_false(calls$deleted)
   expect_false(dir.exists(bundle_dir))
 })
+
+test_that("A failed sys-deps build still deletes the endpoint", {
+  calls = new.env()
+  calls$deleted = FALSE
+  mock_content_delete(calls)
+  testthat::local_mocked_bindings(
+    get_server = function(...) "https://connect.example.com/",
+    setup_plumber_sys_deps_endpoint = function(...) {
+      structure(list(), class = c("ContentTask", "Content", "R6"))
+    }
+  )
+  testthat::local_mocked_bindings(
+    poll_task = function(...) stop("build failed"),
+    .package = "connectapi"
+  )
+
+  expect_identical(check_sys_deps(debug_level = 0), NA)
+  expect_true(calls$deleted)
+})
+
+test_that("cleanup_quarto() deletes by title when the deploy left no record", {
+  calls = new.env()
+  calls$deleted = character(0)
+  title = paste("UAT: Quarto", Sys.Date())
+  testthat::local_mocked_bindings(
+    get_server = function(...) "https://connect.example.com/",
+    get_token = function() "key"
+  )
+  testthat::local_mocked_bindings(
+    connect = function(...) "client",
+    get_content = function(...) tibble::tibble(guid = c("a", "b"), title = c(title, "Other")),
+    content_item = function(con, guid) guid,
+    content_delete = function(item, ...) calls$deleted = c(calls$deleted, item),
+    .package = "connectapi"
+  )
+  tmp_dir = withr::local_tempdir()
+
+  cleanup_quarto(tmp_dir, debug_level = 0, title = title)
+  expect_identical(calls$deleted, "a")
+  expect_false(dir.exists(tmp_dir))
+})