Skip to content

Commit

Permalink
isDocumentPath no longer treats directories as documents
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 committed Feb 25, 2025
1 parent 5af8f1e commit f809fcf
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 f809fcf

Please sign in to comment.