Skip to content

Commit

Permalink
isDocumentPath no longer treats directories as documents (#1139)
Browse files Browse the repository at this point in the history
Additionally, remove the uncalled defaultAppName, which removes one use of isDocumentPath.

fixes #1138
  • Loading branch information
aronatkins authored Feb 25, 2025
1 parent 5af8f1e commit 01e6f39
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 46 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# rsconnect (development version)

* Content directories with a period in their name are no longer treated as a
document path when computing the location for deployment records. (#1138)

# rsconnect 1.3.4

* Use base64 encoded test data. Addresses CRAN test failures when run with
Expand Down
8 changes: 6 additions & 2 deletions R/config.R
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ deploymentConfigFiles <- function(recordPath) {
}

# Does the path point to an individual piece of content?
isDocumentPath <- function(path) {
tools::file_ext(path) != ""
isDocumentPath <- function(recordPath) {
# deployDoc and deployApp have already decided if the record path corresponds to a file
# (single-file document deploys) or a directory (all other content).
#
# The record path is a document path whenever it is not a directory.
!dir.exists(recordPath)
}
26 changes: 0 additions & 26 deletions R/deploymentTarget.R
Original file line number Diff line number Diff line change
Expand Up @@ -315,32 +315,6 @@ updateDeployment <- function(previous, appTitle = NULL, envVars = NULL) {
)
}

defaultAppName <- function(recordPath, server = NULL) {
if (isDocumentPath(recordPath)) {
name <- file_path_sans_ext(basename(recordPath))
if (name == "index") {
# parent directory will give more informative name
name <- basename(dirname(recordPath))
} else {
# deploying a document
}
} else {
# deploying a directory
name <- basename(recordPath)
}

if (isShinyappsServer(server)) {
# Replace non-alphanumerics with underscores, trim to length 64
name <- tolower(gsub("[^[:alnum:]_-]+", "_", name, perl = TRUE))
name <- gsub("_+", "_", name)
if (nchar(name) > 64) {
name <- substr(name, 1, 64)
}
}

name
}

shouldUpdateApp <- function(application,
uniqueName,
forceUpdate = FALSE,
Expand Down
6 changes: 5 additions & 1 deletion tests/testthat/helper.R
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ local_temp_app <- function(files = list(), env = caller_env()) {
if (!hier == ".") {
dir.create(file.path(dir, hier), recursive = TRUE)
}
writeLines(content, file.path(dir, name))
if (length(content) > 0) {
writeLines(content, file.path(dir, name))
} else {
file.create(file.path(dir, name))
}
}

dir
Expand Down
10 changes: 10 additions & 0 deletions tests/testthat/test-config.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,13 @@ test_that("account file containing pattern characters found with server name", {
dir <- accountConfigFile("[email protected]", server = "complex")
expect_equal(dir, expected)
})

test_that("isDocumentPath", {
stuff <- local_temp_app(list(
"shiny.app/app.R" = c(),
"doc/research.Rmd" = c()
))
expect_false(isDocumentPath(file.path(stuff, "shiny.app")))
expect_false(isDocumentPath(file.path(stuff, "doc")))
expect_true(isDocumentPath(file.path(stuff, "doc/research.Rmd")))
})
17 changes: 0 additions & 17 deletions tests/testthat/test-deploymentTarget.R
Original file line number Diff line number Diff line change
Expand Up @@ -435,23 +435,6 @@ test_that("can find existing application on shinyapps.io & not use it", {
confirm_existing_app_not_used("shinyapps.io")
})

# defaultAppName ----------------------------------------------------------

test_that("defaultAppName works with sites, documents, and directories", {
expect_equal(defaultAppName("foo/bar.Rmd"), "bar")
expect_equal(defaultAppName("foo/index.html"), "foo")
expect_equal(defaultAppName("foo/bar"), "bar")
})

test_that("defaultAppName reifies appNames for shinyApps", {
expect_equal(defaultAppName("a b c", "shinyapps.io"), "a_b_c")
expect_equal(defaultAppName("a!b!c", "shinyapps.io"), "a_b_c")
expect_equal(defaultAppName("a b c", "shinyapps.io"), "a_b_c")

long_name <- strrep("abcd", 64 / 4)
expect_equal(defaultAppName(paste(long_name, "..."), "shinyapps.io"), long_name)
})

# helpers -----------------------------------------------------------------

test_that("shouldUpdateApp errors when non-interactive", {
Expand Down

0 comments on commit 01e6f39

Please sign in to comment.