Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mount docs as first mount #882

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
14 changes: 14 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
# plumber (development version)

## 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)

* 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)

* 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

Expand Down
42 changes: 30 additions & 12 deletions R/plumber.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
)
Expand All @@ -252,7 +252,7 @@ Plumber <- R6Class(
# Set and restore the wd to make it appear that the proc is running local to the file's definition.
if (!is.null(private$filename)) {
old_wd <- setwd(dirname(private$filename))
on.exit({setwd(old_wd)}, add = TRUE)
schloerke marked this conversation as resolved.
Show resolved Hide resolved
on.exit(setwd(old_wd), add = TRUE, after = FALSE)
}

if (isTRUE(docs_info$enabled)) {
Expand All @@ -264,10 +264,11 @@ Plumber <- R6Class(
callback = swaggerCallback,
quiet = quiet
)
on.exit(unmount_docs(self, docs_info), add = TRUE)
on.exit(unmount_docs(self, docs_info), add = TRUE, after = FALSE)
}

on.exit(private$runHooks("exit"), 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)
},
Expand All @@ -278,8 +279,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()
Expand All @@ -290,7 +296,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)
Expand All @@ -299,7 +307,15 @@ Plumber <- R6Class(
path <- paste0(path, "/")
}

private$mnts[[path]] <- router
# Remove prior mount if it exists
self$unmount(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you want to do this unmount...? With #883, you'll be able to mount two subrouters to one path and have one fall through to the other.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be sure to address that in the PR. There's a lot of decisions to be made in #883

For now, I'd like to keep existing behavior where it was overwritten.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if you still do want to unmount, I think you would then need to factor in how it affects the after index if it's a non-NULL, non-zero value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm. Let's keep this in draft to see if we're even going to merge #883


# Add new mount
# Mount order matters
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.
Expand Down Expand Up @@ -963,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() {
Expand Down Expand Up @@ -1335,8 +1351,10 @@ upgrade_docs_parameter <- function(docs, ...) {



#' @importFrom rlang is_interactive
# Method needed for testing mocking
default_debug <- function() {
interactive()
is_interactive()
}


Expand Down
17 changes: 12 additions & 5 deletions R/pr.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
#'
Expand All @@ -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
}

Expand Down Expand Up @@ -490,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()].
Expand Down
4 changes: 2 additions & 2 deletions R/pr_set.R
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions R/ui.R
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#' @include globals.R
docs_root <- paste0("/__docs__/")

# Mount OpenAPI and Docs
#' @noRd
Expand Down Expand Up @@ -184,7 +185,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
Expand All @@ -211,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()
Expand Down
13 changes: 10 additions & 3 deletions man/Plumber.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 10 additions & 3 deletions man/pr_mount.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions tests/testthat/files/router.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
6 changes: 6 additions & 0 deletions tests/testthat/helper-interrupt.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

with_interrupt <- function(expr, delay = 0) {
# Causes pr_run() to immediately exit
later::later(httpuv::interrupt, delay = delay)
force(expr)
}
6 changes: 0 additions & 6 deletions tests/testthat/test-plumber-run.R
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/test-routing.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
26 changes: 26 additions & 0 deletions tests/testthat/test-ui.R
Original file line number Diff line number Diff line change
@@ -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)
})