From 4e0918c02d270b2ec8daa3bed0370c7b381334ca Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 28 Sep 2022 13:04:28 -0400 Subject: [PATCH 01/18] Move all docs mounts to the first mount --- R/plumber.R | 1 + R/ui.R | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/R/plumber.R b/R/plumber.R index 03cefd730..fe06a9b83 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -258,6 +258,7 @@ Plumber <- R6Class( if (isTRUE(docs_info$enabled)) { mount_docs( pr = self, + pr_private = private, host = host, port = port, docs_info = docs_info, diff --git a/R/ui.R b/R/ui.R index f6bcf11ee..e8ff60e78 100644 --- a/R/ui.R +++ b/R/ui.R @@ -1,8 +1,9 @@ #' @include globals.R +docs_root <- paste0("/__docs__/") # Mount OpenAPI and Docs #' @noRd -mount_docs <- function(pr, host, port, docs_info, callback, quiet = FALSE) { +mount_docs <- function(pr, pr_private, host, port, docs_info, callback, quiet = FALSE) { # return early if not enabled if (!isTRUE(docs_info$enabled)) { @@ -34,7 +35,17 @@ mount_docs <- function(pr, host, port, docs_info, callback, quiet = FALSE) { if (is_docs_available(docs_info$docs)) { docs_mount <- .globals$docs[[docs_info$docs]]$mount + current_mnt_names <- names(pr_private$mnts) docs_url <- do.call(docs_mount, c(list(pr, api_url), docs_info$args)) + # Mount order matters + # Move new & ordered docs mounts to the front to be processed first + post_mnt_names <- names(pr_private$mnts) + doc_paths <- setdiff(post_mnt_names, current_mnt_names) + pr_private$mnts <- c( + pr_private$mnts[doc_paths], + pr_private$mnts[setdiff(post_mnt_names, doc_paths)] + ) + if (!isTRUE(quiet)) { message("Running ", docs_info$docs, " Docs at ", docs_url, sep = "") } @@ -184,7 +195,6 @@ register_docs <- function(name, index, static = NULL) { stopifnot(is.function(index)) if (!is.null(static)) stopifnot(is.function(static)) - docs_root <- paste0("/__docs__/") docs_paths <- c("/index.html", "/") mount_docs_func <- function(pr, api_url, ...) { # Save initial extra argument values From d7c4461045be366dcb9498eaaf46664413933da2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 28 Sep 2022 13:06:20 -0400 Subject: [PATCH 02/18] Restore the working directory last; Unmount docs via exit hook --- R/plumber.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/R/plumber.R b/R/plumber.R index fe06a9b83..b67b063f1 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -250,9 +250,9 @@ Plumber <- R6Class( } # Set and restore the wd to make it appear that the proc is running local to the file's definition. + old_wd <- NULL if (!is.null(private$filename)) { old_wd <- setwd(dirname(private$filename)) - on.exit({setwd(old_wd)}, add = TRUE) } if (isTRUE(docs_info$enabled)) { @@ -266,9 +266,11 @@ Plumber <- R6Class( quiet = quiet ) on.exit(unmount_docs(self, docs_info), add = TRUE) + on.exit({ + }, add = TRUE) } - on.exit(private$runHooks("exit"), add = TRUE) + # Restore the prior working directory after exit hooks have run httpuv::runServer(host, port, self) }, From 9fd1d785c9affedc504c1b5ea13ac5ecfbb8717b Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 28 Sep 2022 13:07:09 -0400 Subject: [PATCH 03/18] Test that the docs mount is first --- tests/testthat/helper-interrupt.R | 6 ++++++ tests/testthat/test-plumber-run.R | 6 ------ tests/testthat/test-ui.R | 26 ++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 tests/testthat/helper-interrupt.R create mode 100644 tests/testthat/test-ui.R diff --git a/tests/testthat/helper-interrupt.R b/tests/testthat/helper-interrupt.R new file mode 100644 index 000000000..74f7ceeb9 --- /dev/null +++ b/tests/testthat/helper-interrupt.R @@ -0,0 +1,6 @@ + +with_interrupt <- function(expr, delay = 0) { + # Causes pr_run() to immediately exit + later::later(httpuv::interrupt, delay = delay) + force(expr) +} diff --git a/tests/testthat/test-plumber-run.R b/tests/testthat/test-plumber-run.R index ffdf829bf..b70c59295 100644 --- a/tests/testthat/test-plumber-run.R +++ b/tests/testthat/test-plumber-run.R @@ -1,10 +1,4 @@ -with_interrupt <- function(expr) { - # Causes pr_run() to immediately exit - later::later(httpuv::interrupt) - force(expr) -} - test_that("quiet=TRUE suppresses startup messages", { with_interrupt({ expect_message(pr() %>% pr_run(quiet = TRUE), NA) diff --git a/tests/testthat/test-ui.R b/tests/testthat/test-ui.R new file mode 100644 index 000000000..a166111e4 --- /dev/null +++ b/tests/testthat/test-ui.R @@ -0,0 +1,26 @@ + +test_that("doc mounts are prepended", { + + assertions <- 0 + + root <- + test_path("files/static.R") %>% + pr() %>% + pr_set_docs(TRUE) %>% + pr_hook("exit", function(...) { + expect_length(root$mounts, 3) + assertions <<- assertions + 1 + expect_equal(names(root$mounts)[1], "/__docs__/") + assertions <<- assertions + 1 + }) + + expect_length(root$mounts, 2) + assertions <- assertions + 1 + + # no docs. do not find printed message + with_interrupt({ + root %>% pr_run() + }) + + expect_equal(assertions, 3) +}) From fd8bbb55f96b2b6adf64ef2ab12e1eda7826e06c Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 28 Sep 2022 13:08:41 -0400 Subject: [PATCH 04/18] Restore the wd last --- R/plumber.R | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/R/plumber.R b/R/plumber.R index b67b063f1..4d39af07e 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -255,6 +255,9 @@ Plumber <- R6Class( old_wd <- setwd(dirname(private$filename)) } + # Run user exit hooks given docs and wd are still set + on.exit(private$runHooks("exit"), add = TRUE) + if (isTRUE(docs_info$enabled)) { mount_docs( pr = self, @@ -265,12 +268,14 @@ Plumber <- R6Class( callback = swaggerCallback, quiet = quiet ) + # Unmount the docs before restoring the wd on.exit(unmount_docs(self, docs_info), add = TRUE) - on.exit({ - }, add = TRUE) } # Restore the prior working directory after exit hooks have run + if (!is.null(old_wd)) { + on.exit(setwd(old_wd), add = TRUE) + } httpuv::runServer(host, port, self) }, @@ -302,6 +307,8 @@ Plumber <- R6Class( path <- paste0(path, "/") } + # Mount order matters + # Append a mounted router private$mnts[[path]] <- router }, #' @description Unmount a Plumber router From 888c2a3bd76c86710f10158f7fbec46511f3497a Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 28 Sep 2022 13:09:58 -0400 Subject: [PATCH 05/18] comments --- R/plumber.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/plumber.R b/R/plumber.R index 4d39af07e..d4e4bbab3 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -269,6 +269,8 @@ Plumber <- R6Class( quiet = quiet ) # Unmount the docs before restoring the wd + # Unmount needs to happen after exit hooks + # No guarantee that new exit hooks are not added after running API on.exit(unmount_docs(self, docs_info), add = TRUE) } From b8ce6bcbfa195332e30256a37afbf1a4b9ca7706 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 28 Sep 2022 13:10:18 -0400 Subject: [PATCH 06/18] document --- man/Plumber.Rd | 2 +- man/PlumberEndpoint.Rd | 2 +- man/PlumberStatic.Rd | 2 +- man/PlumberStep.Rd | 2 +- man/deprecated_r6.Rd | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/man/Plumber.Rd b/man/Plumber.Rd index 576d47c8e..09c915926 100644 --- a/man/Plumber.Rd +++ b/man/Plumber.Rd @@ -114,7 +114,7 @@ pr$setErrorHandler(function(req, res, err) { \code{\link[=pr_set_docs_callback]{pr_set_docs_callback()}} } \section{Super class}{ -\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{Plumber} +\code{plumber::Hookable} -> \code{Plumber} } \section{Public fields}{ \if{html}{\out{
}} diff --git a/man/PlumberEndpoint.Rd b/man/PlumberEndpoint.Rd index e6695fa0d..e5dc949eb 100644 --- a/man/PlumberEndpoint.Rd +++ b/man/PlumberEndpoint.Rd @@ -15,7 +15,7 @@ Parameters values are obtained from parsing blocks of lines in a plumber file. They can also be provided manually for historical reasons. } \section{Super classes}{ -\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{\link[plumber:PlumberStep]{plumber::PlumberStep}} -> \code{PlumberEndpoint} +\code{plumber::Hookable} -> \code{plumber::PlumberStep} -> \code{PlumberEndpoint} } \section{Public fields}{ \if{html}{\out{
}} diff --git a/man/PlumberStatic.Rd b/man/PlumberStatic.Rd index 772e1c7a8..1a6311ea8 100644 --- a/man/PlumberStatic.Rd +++ b/man/PlumberStatic.Rd @@ -12,7 +12,7 @@ Static file router Creates a router that is backed by a directory of files on disk. } \section{Super classes}{ -\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{\link[plumber:Plumber]{plumber::Plumber}} -> \code{PlumberStatic} +\code{plumber::Hookable} -> \code{plumber::Plumber} -> \code{PlumberStatic} } \section{Methods}{ \subsection{Public methods}{ diff --git a/man/PlumberStep.Rd b/man/PlumberStep.Rd index a9ec1656b..e56416b40 100644 --- a/man/PlumberStep.Rd +++ b/man/PlumberStep.Rd @@ -8,7 +8,7 @@ an object representing a step in the lifecycle of the treatment of a request by a plumber router. } \section{Super class}{ -\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{PlumberStep} +\code{plumber::Hookable} -> \code{PlumberStep} } \section{Public fields}{ \if{html}{\out{
}} diff --git a/man/deprecated_r6.Rd b/man/deprecated_r6.Rd index 2778388c9..b3a763182 100644 --- a/man/deprecated_r6.Rd +++ b/man/deprecated_r6.Rd @@ -22,7 +22,7 @@ Deprecated R6 functions }} \keyword{internal} \section{Super class}{ -\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{hookable} +\code{plumber::Hookable} -> \code{hookable} } \section{Methods}{ \subsection{Public methods}{ @@ -68,7 +68,7 @@ The objects of this class are cloneable with this method. } } \section{Super classes}{ -\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{\link[plumber:Plumber]{plumber::Plumber}} -> \code{plumber} +\code{plumber::Hookable} -> \code{plumber::Plumber} -> \code{plumber} } \section{Methods}{ \subsection{Public methods}{ From 22d9983cff49165cdd205514e0a477a63702601c Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 28 Sep 2022 13:16:04 -0400 Subject: [PATCH 07/18] Update NEWS.md --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index cd188a2d2..dcd18d7ec 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # plumber (development version) +* Always mount OpenAPI documentation at `/__docs__/` as the first mount to have preference when using conflicting mount locations (e.g. a static mount at `/`). (#882) + # plumber 1.2.1 From 357b2995f0a39dee5b1d8c005313bf3dd65b7351 Mon Sep 17 00:00:00 2001 From: schloerke Date: Wed, 28 Sep 2022 17:19:19 +0000 Subject: [PATCH 08/18] `devtools::document()` (GitHub Actions) --- man/Plumber.Rd | 2 +- man/PlumberEndpoint.Rd | 2 +- man/PlumberStatic.Rd | 2 +- man/PlumberStep.Rd | 2 +- man/deprecated_r6.Rd | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/man/Plumber.Rd b/man/Plumber.Rd index 09c915926..576d47c8e 100644 --- a/man/Plumber.Rd +++ b/man/Plumber.Rd @@ -114,7 +114,7 @@ pr$setErrorHandler(function(req, res, err) { \code{\link[=pr_set_docs_callback]{pr_set_docs_callback()}} } \section{Super class}{ -\code{plumber::Hookable} -> \code{Plumber} +\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{Plumber} } \section{Public fields}{ \if{html}{\out{
}} diff --git a/man/PlumberEndpoint.Rd b/man/PlumberEndpoint.Rd index e5dc949eb..e6695fa0d 100644 --- a/man/PlumberEndpoint.Rd +++ b/man/PlumberEndpoint.Rd @@ -15,7 +15,7 @@ Parameters values are obtained from parsing blocks of lines in a plumber file. They can also be provided manually for historical reasons. } \section{Super classes}{ -\code{plumber::Hookable} -> \code{plumber::PlumberStep} -> \code{PlumberEndpoint} +\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{\link[plumber:PlumberStep]{plumber::PlumberStep}} -> \code{PlumberEndpoint} } \section{Public fields}{ \if{html}{\out{
}} diff --git a/man/PlumberStatic.Rd b/man/PlumberStatic.Rd index 1a6311ea8..772e1c7a8 100644 --- a/man/PlumberStatic.Rd +++ b/man/PlumberStatic.Rd @@ -12,7 +12,7 @@ Static file router Creates a router that is backed by a directory of files on disk. } \section{Super classes}{ -\code{plumber::Hookable} -> \code{plumber::Plumber} -> \code{PlumberStatic} +\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{\link[plumber:Plumber]{plumber::Plumber}} -> \code{PlumberStatic} } \section{Methods}{ \subsection{Public methods}{ diff --git a/man/PlumberStep.Rd b/man/PlumberStep.Rd index e56416b40..a9ec1656b 100644 --- a/man/PlumberStep.Rd +++ b/man/PlumberStep.Rd @@ -8,7 +8,7 @@ an object representing a step in the lifecycle of the treatment of a request by a plumber router. } \section{Super class}{ -\code{plumber::Hookable} -> \code{PlumberStep} +\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{PlumberStep} } \section{Public fields}{ \if{html}{\out{
}} diff --git a/man/deprecated_r6.Rd b/man/deprecated_r6.Rd index b3a763182..2778388c9 100644 --- a/man/deprecated_r6.Rd +++ b/man/deprecated_r6.Rd @@ -22,7 +22,7 @@ Deprecated R6 functions }} \keyword{internal} \section{Super class}{ -\code{plumber::Hookable} -> \code{hookable} +\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{hookable} } \section{Methods}{ \subsection{Public methods}{ @@ -68,7 +68,7 @@ The objects of this class are cloneable with this method. } } \section{Super classes}{ -\code{plumber::Hookable} -> \code{plumber::Plumber} -> \code{plumber} +\code{\link[plumber:Hookable]{plumber::Hookable}} -> \code{\link[plumber:Plumber]{plumber::Plumber}} -> \code{plumber} } \section{Methods}{ \subsection{Public methods}{ From 11110cb7903638194891514a126060eafce085ff Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 4 Oct 2022 13:49:25 -0400 Subject: [PATCH 09/18] Use `on.exit(after = FALSE)` to make logic cleaner --- DESCRIPTION | 2 +- R/plumber.R | 16 ++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index cb31b3419..5a4297aad 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -18,7 +18,7 @@ Description: Gives the ability to automatically generate and serve an HTTP API from R functions using the annotations in the R documentation around your functions. Depends: - R (>= 3.0.0) + R (>= 3.5.0) Imports: R6 (>= 2.0.0), stringi (>= 0.3.0), diff --git a/R/plumber.R b/R/plumber.R index d4e4bbab3..7cada9280 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -250,14 +250,11 @@ Plumber <- R6Class( } # Set and restore the wd to make it appear that the proc is running local to the file's definition. - old_wd <- NULL if (!is.null(private$filename)) { old_wd <- setwd(dirname(private$filename)) + on.exit(setwd(old_wd), add = TRUE, after = FALSE) } - # Run user exit hooks given docs and wd are still set - on.exit(private$runHooks("exit"), add = TRUE) - if (isTRUE(docs_info$enabled)) { mount_docs( pr = self, @@ -268,16 +265,11 @@ Plumber <- R6Class( callback = swaggerCallback, quiet = quiet ) - # Unmount the docs before restoring the wd - # Unmount needs to happen after exit hooks - # No guarantee that new exit hooks are not added after running API - on.exit(unmount_docs(self, docs_info), add = TRUE) + on.exit(unmount_docs(self, docs_info), add = TRUE, after = FALSE) } - # Restore the prior working directory after exit hooks have run - if (!is.null(old_wd)) { - on.exit(setwd(old_wd), add = TRUE) - } + # Run user exit hooks given docs and working directory + on.exit(private$runHooks("exit"), add = TRUE, after = FALSE) httpuv::runServer(host, port, self) }, From 6645ba234d4d0da74b4ade2d54d869a07a862330 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 4 Oct 2022 13:51:12 -0400 Subject: [PATCH 10/18] Add support for `Plumber$mount(..., after=)`; `...` is empty; `after` is like `append(after=)` for mount order --- R/plumber.R | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/R/plumber.R b/R/plumber.R index 7cada9280..ef1257558 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -280,8 +280,13 @@ Plumber <- R6Class( #' by paths which is a great technique for decomposing large APIs into smaller files. #' #' See also: [pr_mount()] - #' @param path a character string. Where to mount router. - #' @param router a Plumber router. Router to be mounted. + #' @param path a character string. Where to mount the sub router. + #' @param router a Plumber router. Sub router to be mounted. + #' @param ... Ignored. Used for possible parameter expansion. + #' @param after If `NULL` (default), the router will be appended to the end + #' of the mounts. If a number, the router will be inserted at the given + #' index. E.g. `after = 0` will prepend the sub router, giving it + #' preference over other mounted routers. #' @examples #' \dontrun{ #' root <- pr() @@ -292,7 +297,9 @@ Plumber <- R6Class( #' products <- Plumber$new("products.R") #' root$mount("/products", products) #' } - mount = function(path, router) { + mount = function(path, router, ..., after = NULL) { + ellipsis::check_dots_empty() + # Ensure that the path has both a leading and trailing slash. if (!grepl("^/", path)) { path <- paste0("/", path) @@ -302,8 +309,10 @@ Plumber <- R6Class( } # Mount order matters - # Append a mounted router - private$mnts[[path]] <- router + after <- after %||% length(private$mnts) + mntList <- list() + mntList[[path]] <- router + private$mnts <- append(private$mnts, mntList, after = after) }, #' @description Unmount a Plumber router #' @param path a character string. Where to unmount router. From a9ebbf98b566d9f0186c41bed84b969e38307164 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 4 Oct 2022 13:51:40 -0400 Subject: [PATCH 11/18] Remove `pr_private` logic and add utilize `pr$mount(after=0)` --- R/plumber.R | 1 - R/ui.R | 14 ++------------ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/R/plumber.R b/R/plumber.R index ef1257558..759361bc9 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -258,7 +258,6 @@ Plumber <- R6Class( if (isTRUE(docs_info$enabled)) { mount_docs( pr = self, - pr_private = private, host = host, port = port, docs_info = docs_info, diff --git a/R/ui.R b/R/ui.R index e8ff60e78..5708b51e4 100644 --- a/R/ui.R +++ b/R/ui.R @@ -3,7 +3,7 @@ docs_root <- paste0("/__docs__/") # Mount OpenAPI and Docs #' @noRd -mount_docs <- function(pr, pr_private, host, port, docs_info, callback, quiet = FALSE) { +mount_docs <- function(pr, host, port, docs_info, callback, quiet = FALSE) { # return early if not enabled if (!isTRUE(docs_info$enabled)) { @@ -35,17 +35,7 @@ mount_docs <- function(pr, pr_private, host, port, docs_info, callback, quiet = if (is_docs_available(docs_info$docs)) { docs_mount <- .globals$docs[[docs_info$docs]]$mount - current_mnt_names <- names(pr_private$mnts) docs_url <- do.call(docs_mount, c(list(pr, api_url), docs_info$args)) - # Mount order matters - # Move new & ordered docs mounts to the front to be processed first - post_mnt_names <- names(pr_private$mnts) - doc_paths <- setdiff(post_mnt_names, current_mnt_names) - pr_private$mnts <- c( - pr_private$mnts[doc_paths], - pr_private$mnts[setdiff(post_mnt_names, doc_paths)] - ) - if (!isTRUE(quiet)) { message("Running ", docs_info$docs, " Docs at ", docs_url, sep = "") } @@ -221,7 +211,7 @@ register_docs <- function(name, index, static = NULL) { message("") } - pr$mount(docs_root, docs_router) + pr$mount(docs_root, docs_router, after = 0) # add legacy swagger redirects (RStudio Connect) redirect_info <- swagger_redirects() From 5db42eb7bec0d8cceb1bc55a3fd567af2540a550 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 4 Oct 2022 13:59:33 -0400 Subject: [PATCH 12/18] Add support for `pr_mount(..., after=)` --- R/pr.R | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/R/pr.R b/R/pr.R index 40f1b4be0..57c96846b 100644 --- a/R/pr.R +++ b/R/pr.R @@ -210,8 +210,13 @@ pr_head <- function(pr, #' returns the updated router. #' #' @param pr The host Plumber router. -#' @param path A character string. Where to mount router. -#' @param router A Plumber router. Router to be mounted. +#' @param path a character string. Where to mount the sub router. +#' @param router a Plumber router. Sub router to be mounted. +#' @param ... Ignored. Used for possible parameter expansion. +#' @param after If `NULL` (default), the router will be appended to the end +#' of the mounts. If a number, the router will be inserted at the given +#' index. E.g. `after = 0` will prepend the sub router, giving it +#' preference over other mounted routers. #' #' @return A Plumber router with the supplied router mounted #' @@ -229,9 +234,11 @@ pr_head <- function(pr, #' @export pr_mount <- function(pr, path, - router) { + router, + ..., + after = NULL) { validate_pr(pr) - pr$mount(path = path, router = router) + pr$mount(path = path, router = router, ..., after = after) pr } From 39d94e6c7241d90bf3cd51905043d8e5adf53025 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Thu, 29 Sep 2022 09:28:30 -0400 Subject: [PATCH 13/18] Test: Mount a third router with `after=0` --- tests/testthat/files/router.R | 18 ++++++++++++++++++ tests/testthat/test-routing.R | 9 +++++++++ 2 files changed, 27 insertions(+) diff --git a/tests/testthat/files/router.R b/tests/testthat/files/router.R index a52a543f9..2f68ae0e0 100644 --- a/tests/testthat/files/router.R +++ b/tests/testthat/files/router.R @@ -61,3 +61,21 @@ function(res){ function(){ "dual path" } + +#* @plumber +function(pr) { + mnt_1 <- + pr() %>% + pr_get("/hello", function() "say hello") + mnt_2 <- + pr() %>% + pr_get("/world", function() "say hello world") + mnt_0 <- + pr() %>% + pr_get("/world", function() "say hello world") + + pr %>% + pr_mount("/say", mnt_1) %>% + pr_mount("/say/hello", mnt_2) %>% + pr_mount("/first", mnt_0, after = 0) +} diff --git a/tests/testthat/test-routing.R b/tests/testthat/test-routing.R index 224d08451..42daa29f2 100644 --- a/tests/testthat/test-routing.R +++ b/tests/testthat/test-routing.R @@ -20,6 +20,15 @@ test_that("Routing to errors and 404s works", { expect_equal(r$route(make_req("GET", "/path1"), res), "dual path") expect_equal(r$route(make_req("GET", "/path2"), res), "dual path") + ### Won't work yet + ## Mounts fall back to parent router when route is not found + # # Mount at `/say` with route `/hello` + # expect_equal(r$route(make_req("GET", "/say/hello"), res), "say hello") + # # Mount at `/say/hello` with route `/world` + # expect_equal(r$route(make_req("GET", "/say/hello/world"), res), "say hello world") + + expect_equal(names(r$mounts), c("/first/", "/say/", "/say/hello/")) + expect_equal(errors, 0) expect_equal(notFounds, 0) From 55878dd9627dd1e8c4492188712a814a145aa9a2 Mon Sep 17 00:00:00 2001 From: schloerke Date: Tue, 4 Oct 2022 18:06:24 +0000 Subject: [PATCH 14/18] `devtools::document()` (GitHub Actions) --- man/Plumber.Rd | 13 ++++++++++--- man/pr_mount.Rd | 13 ++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/man/Plumber.Rd b/man/Plumber.Rd index 576d47c8e..1c8b0cf2a 100644 --- a/man/Plumber.Rd +++ b/man/Plumber.Rd @@ -273,15 +273,22 @@ by paths which is a great technique for decomposing large APIs into smaller file See also: \code{\link[=pr_mount]{pr_mount()}} \subsection{Usage}{ -\if{html}{\out{
}}\preformatted{Plumber$mount(path, router)}\if{html}{\out{
}} +\if{html}{\out{
}}\preformatted{Plumber$mount(path, router, ..., after = NULL)}\if{html}{\out{
}} } \subsection{Arguments}{ \if{html}{\out{
}} \describe{ -\item{\code{path}}{a character string. Where to mount router.} +\item{\code{path}}{a character string. Where to mount the sub router.} -\item{\code{router}}{a Plumber router. Router to be mounted.} +\item{\code{router}}{a Plumber router. Sub router to be mounted.} + +\item{\code{...}}{Ignored. Used for possible parameter expansion.} + +\item{\code{after}}{If \code{NULL} (default), the router will be appended to the end +of the mounts. If a number, the router will be inserted at the given +index. E.g. \code{after = 0} will prepend the sub router, giving it +preference over other mounted routers.} } \if{html}{\out{
}} } diff --git a/man/pr_mount.Rd b/man/pr_mount.Rd index 7f08d1c13..e6a07e198 100644 --- a/man/pr_mount.Rd +++ b/man/pr_mount.Rd @@ -4,14 +4,21 @@ \alias{pr_mount} \title{Mount a Plumber router} \usage{ -pr_mount(pr, path, router) +pr_mount(pr, path, router, ..., after = NULL) } \arguments{ \item{pr}{The host Plumber router.} -\item{path}{A character string. Where to mount router.} +\item{path}{a character string. Where to mount the sub router.} -\item{router}{A Plumber router. Router to be mounted.} +\item{router}{a Plumber router. Sub router to be mounted.} + +\item{...}{Ignored. Used for possible parameter expansion.} + +\item{after}{If \code{NULL} (default), the router will be appended to the end +of the mounts. If a number, the router will be inserted at the given +index. E.g. \code{after = 0} will prepend the sub router, giving it +preference over other mounted routers.} } \value{ A Plumber router with the supplied router mounted From 48d98a7dfb2b36156f6e1043a848382a3bc282c5 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 5 Oct 2022 10:55:14 -0400 Subject: [PATCH 15/18] Add another news entry --- NEWS.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS.md b/NEWS.md index dcd18d7ec..4347e89f5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,15 @@ # plumber (development version) +## Breaking changes + +## New features + +* Added support for `pr_mount(after=)` / `Plumber$mount(after=)` which allows for mounts to be added in non-sequential order (#882) + * Always mount OpenAPI documentation at `/__docs__/` as the first mount to have preference when using conflicting mount locations (e.g. a static mount at `/`). (#882) +## Bug fixes + # plumber 1.2.1 From 4032882a0d0c11ff68703afc9332789e8fd939e8 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 5 Oct 2022 11:18:42 -0400 Subject: [PATCH 16/18] Remove prior mount if it exists --- R/plumber.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/R/plumber.R b/R/plumber.R index 759361bc9..2064f5cdb 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -307,6 +307,10 @@ Plumber <- R6Class( path <- paste0(path, "/") } + # Remove prior mount if it exists + self$unmount(path) + + # Add new mount # Mount order matters after <- after %||% length(private$mnts) mntList <- list() From 11ae0525c894a5932389d6bae87b361aa9d75c86 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 5 Oct 2022 11:19:32 -0400 Subject: [PATCH 17/18] Add breaking news entry about mounts being removed before added --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 4347e89f5..6fd6735d7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Breaking changes +* When mounting a router at an existing location, the old mounted router is removed and the router to be mounted is added to the end of the mount list (default; `after = NULL`). This makes it so prior mount calls do not affect current mount calls. (#882) + ## New features * Added support for `pr_mount(after=)` / `Plumber$mount(after=)` which allows for mounts to be added in non-sequential order (#882) From 777cf62c1c25a992c8a8666cf40790964669ec6a Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Wed, 5 Oct 2022 11:37:15 -0400 Subject: [PATCH 18/18] Use `rlang::is_interactive()` instead of `base::interactive()` --- NEWS.md | 2 ++ R/plumber.R | 12 +++++++----- R/pr.R | 2 +- R/pr_set.R | 4 ++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/NEWS.md b/NEWS.md index 6fd6735d7..a3b65a6c6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,8 @@ * When mounting a router at an existing location, the old mounted router is removed and the router to be mounted is added to the end of the mount list (default; `after = NULL`). This makes it so prior mount calls do not affect current mount calls. (#882) +* The default value of `debug` has been changed from `base::interactive()` to `rlang::is_interactive()`. This allows for tests to be consistent when run interactively or in batch mode. (#882) + ## New features * Added support for `pr_mount(after=)` / `Plumber$mount(after=)` which allows for mounts to be added in non-sequential order (#882) diff --git a/R/plumber.R b/R/plumber.R index 2064f5cdb..1620ef69a 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -143,7 +143,7 @@ Plumber <- R6Class( #' Mac OS X, port numbers smaller than 1025 require root privileges. #' #' This value does not need to be explicitly assigned. To explicitly set it, see [options_plumber()]. - #' @param debug If `TRUE`, it will provide more insight into your API errors. Using this value will only last for the duration of the run. If a `$setDebug()` has not been called, `debug` will default to `interactive()` at `$run()` time. See `$setDebug()` for more details. + #' @param debug If `TRUE`, it will provide more insight into your API errors. Using this value will only last for the duration of the run. If a `$setDebug()` has not been called, `debug` will default to [`rlang::is_interactive()`] at `$run()` time. See `$setDebug()` for more details. #' @param swagger Deprecated. Please use `docs` instead. See `$setDocs(docs)` or `$setApiSpec()` for more customization. #' @param swaggerCallback An optional single-argument function that is #' called back with the URL to an OpenAPI user interface when one becomes @@ -232,7 +232,7 @@ Plumber <- R6Class( }, add = TRUE) # Fix the debug value while running. self$setDebug( - # Order: Run method param, internally set value, is interactive() + # Order: Run method param, internally set value, `is_interactive()` # `$getDebug()` is dynamic given `setDebug()` has never been called. rlang::maybe_missing(debug, self$getDebug()) ) @@ -979,11 +979,11 @@ Plumber <- R6Class( #' #' See also: `$getDebug()` and [pr_set_debug()] #' @param debug `TRUE` provides more insight into your API errors. - setDebug = function(debug = interactive()) { + setDebug = function(debug = is_interactive()) { stopifnot(length(debug) == 1) private$debug <- isTRUE(debug) }, - #' @description Retrieve the `debug` value. If it has never been set, the result of `interactive()` will be used. + #' @description Retrieve the `debug` value. If it has never been set, the result of [`rlang::is_interactive()`] will be used. #' #' See also: `$getDebug()` and [pr_set_debug()] getDebug = function() { @@ -1351,8 +1351,10 @@ upgrade_docs_parameter <- function(docs, ...) { +#' @importFrom rlang is_interactive +# Method needed for testing mocking default_debug <- function() { - interactive() + is_interactive() } diff --git a/R/pr.R b/R/pr.R index 57c96846b..9237054dc 100644 --- a/R/pr.R +++ b/R/pr.R @@ -497,7 +497,7 @@ pr_filter <- function(pr, #' @param ... Should be empty. #' @param debug If `TRUE`, it will provide more insight into your API errors. #' Using this value will only last for the duration of the run. -#' If [pr_set_debug()] has not been called, `debug` will default to `interactive()` at [pr_run()] time +#' If [pr_set_debug()] has not been called, `debug` will default to [`rlang::is_interactive()`] at [pr_run()] time #' @param docs Visual documentation value to use while running the API. #' This value will only be used while running the router. #' If missing, defaults to information previously set with [pr_set_docs()]. diff --git a/R/pr_set.R b/R/pr_set.R index dd66561f9..375563fbd 100644 --- a/R/pr_set.R +++ b/R/pr_set.R @@ -98,7 +98,7 @@ pr_set_error <- function(pr, fun) { #' Set debug value to include error messages of routes cause an error #' #' To hide any error messages in production, set the debug value to `FALSE`. -#' The `debug` value is enabled by default for [interactive()] sessions. +#' The `debug` value is enabled by default for [`rlang::is_interactive()`] sessions. #' #' @template param_pr #' @param debug `TRUE` provides more insight into your API errors. @@ -118,7 +118,7 @@ pr_set_error <- function(pr, fun) { #' pr_get("/boom", function() stop("boom")) %>% #' pr_run() #' } -pr_set_debug <- function(pr, debug = interactive()) { +pr_set_debug <- function(pr, debug = is_interactive()) { validate_pr(pr) pr$setDebug(debug = debug) pr