diff --git a/DESCRIPTION b/DESCRIPTION index 627a28b68..963846e3b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -2,7 +2,7 @@ Encoding: UTF-8 Package: plumber Type: Package Title: An API Generator for R -Version: 1.0.0.9999 +Version: 1.0.0.90001 Roxygen: list(markdown = TRUE) Authors@R: c( person("Barret", "Schloerke", role = c("cre", "aut"), email = "barret@rstudio.com"), @@ -31,7 +31,8 @@ Imports: swagger (>= 3.33.0), magrittr, mime, - lifecycle (>= 0.2.0) + lifecycle (>= 0.2.0), + ellipsis (>= 0.3.0) LazyData: TRUE ByteCompile: TRUE Suggests: @@ -67,11 +68,11 @@ Collate: 'new-rstudio-project.R' 'openapi-spec.R' 'openapi-types.R' + 'options_plumber.R' 'paths.R' 'plumb-block.R' 'plumb-globals.R' 'plumb.R' - 'plumber-options.R' 'plumber-response.R' 'plumber-static.R' 'plumber-step.R' diff --git a/NEWS.md b/NEWS.md index c7a64c04e..27135fdf8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,11 +7,18 @@ plumber 1.0.0.9999 Development version * When plumbing a Plumber file and using a Plumber router modifier (`#* @plumber`), an error will be thrown if the original router is not returned. (#738) +* `options_plumber()` now requires that all options are named. If no option name is provided, an error with be thrown. (#746) + ### New features +* Added option `plumber.trailingSlash`. This option (which is **disabled** by default) allows routes to be redirected to route definitions with a trailing slash. For example, if a `GET` request is submitted to `/test?a=1` with no `/test` route is defined, but a `GET` `/test/` route definition does exist, then the original request will respond with a `307` to reattempt against `GET` `/test/?a=1`. This option will be _enabled_ by default in a future release. This logic executed for before calling the `404` handler. (#746) + +* Added option `plumber.methodNotFound`. This option (which is enabled by default) allows for a status of `405` to be returned if an invalid method is used when requesting a valid route. This logic executed for before calling the default `404` handler. (#746) + * Guess OpenApi response content type from serializer. (@meztez #684) * Passing `edit = TRUE` to `plumb_api()` will open the API source file. (#699) + * Allow for spaces in `@apiTag` and `@tag` when tag is surrended by single or double quotes. (#685) * OpenAPI Specification can be set using a file path. (@meztez #696) diff --git a/R/default-handlers.R b/R/default-handlers.R index 8784c36c3..3ea03b21f 100644 --- a/R/default-handlers.R +++ b/R/default-handlers.R @@ -1,10 +1,4 @@ -default404Handler <- function(req, res){ - if (is_405(req$pr, req$PATH_INFO, req$REQUEST_METHOD)) { - res$status <- 405L - # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow - res$setHeader("Allow", paste(req$verbsAllowed, collapse = ", ")) - return(list(error = "405 - Method Not Allowed")) - } +default404Handler <- function(req, res) { res$status <- 404 res$serializer <- serializer_unboxed_json() list(error="404 - Resource Not Found") @@ -16,12 +10,14 @@ defaultErrorHandler <- function(){ li <- list() + # always serialize error with unboxed json + res$serializer <- serializer_unboxed_json() + if (res$status == 200L){ # The default is a 200. If that's still set, then we should probably override with a 500. # It's possible, however, than a handler set a 40x and then wants to use this function to # render an error, though. res$status <- 500 - res$serializer <- serializer_unboxed_json() li$error <- "500 - Internal server error" } else { li$error <- "Internal error" @@ -87,13 +83,15 @@ is_405 <- function(pr, path_to_find, verb_to_find) { # nothing found, not 405 if (length(verbs_allowed) == 0) return(FALSE) + # is verb excluded? !(verb_to_find %in% verbs_allowed) } router_has_route <- function(pr, path_to_find, verb_to_find) { verbs_allowed <- allowed_verbs(pr, path_to_find) - # nothing found, not 405 + # nothing found, not a route if (length(verbs_allowed) == 0) return(FALSE) + # is verb found? verb_to_find %in% verbs_allowed } diff --git a/R/plumber-options.R b/R/options_plumber.R similarity index 66% rename from R/plumber-options.R rename to R/options_plumber.R index aed685be2..f35159ce0 100644 --- a/R/plumber-options.R +++ b/R/options_plumber.R @@ -11,6 +11,14 @@ #' \item{`plumber.docs.callback`}{A function. Called with #' a single parameter corresponding to the visual documentation url after Plumber server is ready. This can be used #' by RStudio to open the docs when then API is ran from the editor. Defaults to option `NULL`.} +#' \item{`plumber.trailingSlash`}{Logical value which allows the router to redirect any request +#' that has a matching route with a trailing slash. For example, if set to `TRUE` and the +#' GET route `/test/` existed, then a GET request of `/test?a=1` would redirect to +#' `/test/?a=1`. Defaults to `FALSE`. This option will default to `TRUE` in a future release.} +#' \item{`plumber.methodNotAllowed`}{Logical value which allows the router to notify that an +#' unavailable method was requested, but a different request method is allowed. For example, +#' if set to `TRUE` and the GET route `/test` existed, then a POST request of `/test` would +#' receive a 405 status and the allowed methods. Defaults to `TRUE`.} #' \item{`plumber.apiURL`}{Server urls for OpenAPI Specification respecting #' pattern `scheme://host:port/path`. Other `api*` options will be ignored when set.} #' \item{`plumber.apiScheme`}{Scheme used to build OpenAPI url and server url for @@ -34,16 +42,20 @@ #' by settings this option to `FALSE`. Defaults to `TRUE`} #' } #' -#' @param port,docs,docs.callback,apiScheme,apiHost,apiPort,apiPath,apiURL,maxRequestSize,sharedSecret,legacyRedirects See details +#' @param ... Ignored. Should be empty +#' @param port,docs,docs.callback,trailingSlash,methodNotAllowed,apiScheme,apiHost,apiPort,apiPath,apiURL,maxRequestSize,sharedSecret,legacyRedirects See details #' @return #' The complete, prior set of [options()] values. #' If a particular parameter is not supplied, it will return the current value. #' If no parameters are supplied, all returned values will be the current [options()] values. #' @export options_plumber <- function( + ..., port = getOption("plumber.port"), docs = getOption("plumber.docs"), docs.callback = getOption("plumber.docs.callback"), + trailingSlash = getOption("plumber.trailingSlash"), + methodNotAllowed = getOption("plumber.methodNotAllowed"), apiURL = getOption("plumber.apiURL"), apiScheme = getOption("plumber.apiScheme"), apiHost = getOption("plumber.apiHost"), @@ -53,17 +65,21 @@ options_plumber <- function( sharedSecret = getOption("plumber.sharedSecret"), legacyRedirects = getOption("plumber.legacyRedirects") ) { + ellipsis::check_dots_empty() + options( - plumber.apiScheme = apiScheme, - plumber.apiHost = apiHost, - plumber.apiPort = apiPort, - plumber.apiPath = apiPath, - plumber.apiURL = apiURL, - plumber.maxRequestSize = maxRequestSize, - plumber.port = port, - plumber.sharedSecret = sharedSecret, - plumber.docs = docs, - plumber.docs.callback = docs.callback, - plumber.legacyRedirects = legacyRedirects + plumber.port = port, + plumber.docs = docs, + plumber.docs.callback = docs.callback, + plumber.trailingSlash = trailingSlash, + plumber.methodNotAllowed = methodNotAllowed, + plumber.apiURL = apiURL, + plumber.apiScheme = apiScheme, + plumber.apiHost = apiHost, + plumber.apiPort = apiPort, + plumber.apiPath = apiPath, + plumber.maxRequestSize = maxRequestSize, + plumber.sharedSecret = sharedSecret, + plumber.legacyRedirects = legacyRedirects ) } diff --git a/R/plumber.R b/R/plumber.R index e30cefedf..99bd63de3 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -726,6 +726,48 @@ Plumber <- R6Class( # No endpoint could handle this request. 404 notFoundStep <- function(...) { + + if (isTRUE(getOption("plumber.trailingSlash", FALSE))) { + # Redirect to the slash route, if it exists + path <- req$PATH_INFO + # If the path does not end in a slash, + if (!grepl("/$", path)) { + new_path <- paste0(path, "/") + # and a route with a slash exists... + if (router_has_route(req$pr, new_path, req$REQUEST_METHOD)) { + + # Temp redirect with same REQUEST_METHOD + # Add on the query string manually. They do not auto transfer + # The POST body will be reissued by caller + new_location <- paste0(new_path, req$QUERY_STRING) + res$status <- 307 + res$setHeader( + name = "Location", + value = new_location + ) + res$serializer <- serializer_unboxed_json() + return( + list(message = "307 - Redirecting with trailing slash") + ) + } + } + } + + # No trailing-slash route exists... + # Try allowed verbs + + if (isTRUE(getOption("plumber.methodNotAllowed", TRUE))) { + # Notify about allowed verbs + if (is_405(req$pr, req$PATH_INFO, req$REQUEST_METHOD)) { + res$status <- 405L + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow + res$setHeader("Allow", paste(req$verbsAllowed, collapse = ", ")) + res$serializer <- serializer_unboxed_json() + return(list(error = "405 - Method Not Allowed")) + } + } + + # Notify that there is no route found private$notFoundHandler(req = req, res = res) } steps <- append(steps, list(notFoundStep)) diff --git a/man/options_plumber.Rd b/man/options_plumber.Rd index 4c954d95c..756b071f2 100644 --- a/man/options_plumber.Rd +++ b/man/options_plumber.Rd @@ -1,13 +1,16 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/plumber-options.R +% Please edit documentation in R/options_plumber.R \name{options_plumber} \alias{options_plumber} \title{Plumber options} \usage{ options_plumber( + ..., port = getOption("plumber.port"), docs = getOption("plumber.docs"), docs.callback = getOption("plumber.docs.callback"), + trailingSlash = getOption("plumber.trailingSlash"), + methodNotAllowed = getOption("plumber.methodNotAllowed"), apiURL = getOption("plumber.apiURL"), apiScheme = getOption("plumber.apiScheme"), apiHost = getOption("plumber.apiHost"), @@ -19,7 +22,9 @@ options_plumber( ) } \arguments{ -\item{port, docs, docs.callback, apiScheme, apiHost, apiPort, apiPath, apiURL, maxRequestSize, sharedSecret, legacyRedirects}{See details} +\item{...}{Ignored. Should be empty} + +\item{port, docs, docs.callback, trailingSlash, methodNotAllowed, apiScheme, apiHost, apiPort, apiPath, apiURL, maxRequestSize, sharedSecret, legacyRedirects}{See details} } \value{ The complete, prior set of \code{\link[=options]{options()}} values. @@ -39,6 +44,14 @@ If the port is already in use, server will not be able to start. Defaults to \co \item{\code{plumber.docs.callback}}{A function. Called with a single parameter corresponding to the visual documentation url after Plumber server is ready. This can be used by RStudio to open the docs when then API is ran from the editor. Defaults to option \code{NULL}.} +\item{\code{plumber.trailingSlash}}{Logical value which allows the router to redirect any request +that has a matching route with a trailing slash. For example, if set to \code{TRUE} and the +GET route \verb{/test/} existed, then a GET request of \verb{/test?a=1} would redirect to +\verb{/test/?a=1}. Defaults to \code{FALSE}. This option will default to \code{TRUE} in a future release.} +\item{\code{plumber.methodNotAllowed}}{Logical value which allows the router to notify that an +unavailable method was requested, but a different request method is allowed. For example, +if set to \code{TRUE} and the GET route \verb{/test} existed, then a POST request of \verb{/test} would +receive a 405 status and the allowed methods. Defaults to \code{TRUE}.} \item{\code{plumber.apiURL}}{Server urls for OpenAPI Specification respecting pattern \verb{scheme://host:port/path}. Other \verb{api*} options will be ignored when set.} \item{\code{plumber.apiScheme}}{Scheme used to build OpenAPI url and server url for diff --git a/tests/testthat/helper-options.R b/tests/testthat/helper-options.R new file mode 100644 index 000000000..03586ddf8 --- /dev/null +++ b/tests/testthat/helper-options.R @@ -0,0 +1,8 @@ +with_options <- function(opts, code) { + old_opts <- options(opts) + on.exit({ + options(old_opts) + }, add = TRUE) + + force(code) +} diff --git a/tests/testthat/test-options.R b/tests/testthat/test-options.R index 7fe3ad1e5..6c7872780 100644 --- a/tests/testthat/test-options.R +++ b/tests/testthat/test-options.R @@ -1,14 +1,12 @@ context("Options") test_that("Options set and get", { - option_value <- getOption("plumber.port") - on.exit({ - options(plumber.port = option_value) - }, add = TRUE) - options_plumber(port = FALSE) - expect_false(getOption("plumber.port")) - options_plumber(port = NULL) - expect_null(getOption("plumber.port")) + with_options(list(plumber.port = NULL), { + options_plumber(port = FALSE) + expect_false(getOption("plumber.port")) + options_plumber(port = NULL) + expect_null(getOption("plumber.port")) + }) }) test_that("all options used are `options_plumber()` parameters", { @@ -39,7 +37,12 @@ test_that("all options used are `options_plumber()` parameters", { deprecated_options <- c("plumber.swagger.url") plumber_options_used <- plumber_options_used[!(plumber_options_used %in% deprecated_options)] ### code to match formals - options_plumber_formals <- paste0("plumber.", sort(names(formals(options_plumber)))) + formals_to_match <- + sort(setdiff( + names(formals(options_plumber)), + "..." + )) + options_plumber_formals <- paste0("plumber.", formals_to_match) expect_equal( plumber_options_used, @@ -49,16 +52,13 @@ test_that("all options used are `options_plumber()` parameters", { test_that("Legacy swagger redirect can be disabled", { - option_value <- getOption("plumber.legacyRedirects") - on.exit({ - options(plumber.legacyRedirects = option_value) - }, add = TRUE) + with_options(list(), { + options_plumber(legacyRedirects = TRUE) + redirects <- swagger_redirects() + expect_gt(length(redirects), 0) - options_plumber(legacyRedirects = TRUE) - redirects <- swagger_redirects() - expect_gt(length(redirects), 0) - - options_plumber(legacyRedirects = FALSE) - redirects <- swagger_redirects() - expect_equal(length(redirects), 0) + options_plumber(legacyRedirects = FALSE) + redirects <- swagger_redirects() + expect_equal(length(redirects), 0) + }) }) diff --git a/tests/testthat/test-plumber.R b/tests/testthat/test-plumber.R index 08ecce1d0..b141a531f 100644 --- a/tests/testthat/test-plumber.R +++ b/tests/testthat/test-plumber.R @@ -348,25 +348,67 @@ test_that("invalid hooks err", { }) test_that("handle invokes correctly", { - pr <- pr() - pr$handle("GET", "/trailslash", function(){ "getter" }) - pr$handle("POST", "/trailslashp/", function(){ "poster" }) + with_options( + list(plumber.trailingSlash = NULL), + { + pr <- pr() + pr$handle("GET", "/trailslash", function(){ "getter" }) + pr$handle("POST", "/trailslashp/", function(){ "poster" }) + + expect_equal(pr$call(make_req("GET", "/trailslash"))$body, jsonlite::toJSON("getter")) + res <- pr$call(make_req("GET", "/trailslash/")) # With trailing slash + expect_equal(res$status, 404) + res <- pr$call(make_req("POST", "/trailslash")) # Wrong verb + expect_equal(res$status, 405) + + expect_equal(pr$call(make_req("POST", "/trailslashp/"))$body, jsonlite::toJSON("poster")) + res <- pr$call(make_req("POST", "/trailslashp")) # w/o trailing slash + expect_equal(res$status, 404) + res <- pr$call(make_req("GET", "/trailslashp/")) # Wrong verb + expect_equal(res$status, 405) + } + ) - expect_equal(pr$call(make_req("GET", "/trailslash"))$body, jsonlite::toJSON("getter")) - res <- pr$call(make_req("GET", "/trailslash/")) # With trailing slash - expect_equal(res$status, 404) - res <- pr$call(make_req("POST", "/trailslash")) # Wrong verb - expect_equal(res$status, 405) +}) - expect_equal(pr$call(make_req("POST", "/trailslashp/"))$body, jsonlite::toJSON("poster")) - res <- pr$call(make_req("POST", "/trailslashp")) # w/o trailing slash - expect_equal(res$status, 404) - res <- pr$call(make_req("GET", "/trailslashp/")) # Wrong verb - expect_equal(res$status, 405) +test_that("trailing slashes are redirected", { + + pr <- pr() %>% + pr_get("/get/", function(a) a) %>% + pr_post("/post/", function(a) a) %>% + pr_mount( + "/mnt", + pr() %>% + pr_get("/", function(a) a) + ) + with_options(list(plumber.trailingSlash = FALSE), { + res <- pr$call(make_req("GET", "/get", "?a=1")) + expect_equal(res$status, 404) + res <- pr$call(make_req("POST", "/post", "?a=1")) + expect_equal(res$status, 404) + + res <- pr$call(make_req("GET", "/mnt", "?a=1")) + expect_equal(res$status, 404) + }) + + with_options(list(plumber.trailingSlash = TRUE), { + res <- pr$call(make_req("GET", "/get", "?a=1")) + expect_equal(res$status, 307) + expect_equal(res$headers$Location, "/get/?a=1") + + res <- pr$call(make_req("POST", "/post", "?a=1")) + expect_equal(res$status, 307) + expect_equal(res$headers$Location, "/post/?a=1") + + res <- pr$call(make_req("GET", "/mnt", "?a=1")) + expect_equal(res$status, 307) + expect_equal(res$headers$Location, "/mnt/?a=1") + }) }) + test_that("No 405 on same path, different verb", { pr <- pr() diff --git a/tests/testthat/test-zzz-plumb_api.R b/tests/testthat/test-zzz-plumb_api.R index b95be90a5..b26debc41 100644 --- a/tests/testthat/test-zzz-plumb_api.R +++ b/tests/testthat/test-zzz-plumb_api.R @@ -85,40 +85,45 @@ test_that("errors are thrown", { test_that("edit opens correct file", { # Redefine editor so that file.edit doesn't try to open a file - orig_opts <- options(editor = function(name, file, title) { - cat(file, " test file attempted to open\n", sep = "") - }) - on.exit(options(orig_opts), add = TRUE) - - apis <- available_apis() - - selected_api <- apis$package == "plumber" & apis$name == "01-append" - - expect_warning( - expect_output( - plumb_api("plumber", "01-append", edit = TRUE), - "plumber.R test file attempted to open", - fixed = TRUE + with_options( + list( + editor = function(name, file, title) { + cat(file, " test file attempted to open\n", sep = "") + } ), - "plumber.R has been opened in the editor" - ) - - selected_api <- apis$package == "plumber" & apis$name == "12-entrypoint" - - expect_warning( - expect_output( - plumb_api("plumber", "12-entrypoint", edit = TRUE), - "entrypoint.R test file attempted to open", - fixed = TRUE - ), - "entrypoint.R has been opened in the editor" + { + apis <- available_apis() + + selected_api <- apis$package == "plumber" & apis$name == "01-append" + + expect_warning( + expect_output( + plumb_api("plumber", "01-append", edit = TRUE), + "plumber.R test file attempted to open", + fixed = TRUE + ), + "plumber.R has been opened in the editor" + ) + + selected_api <- apis$package == "plumber" & apis$name == "12-entrypoint" + + expect_warning( + expect_output( + plumb_api("plumber", "12-entrypoint", edit = TRUE), + "entrypoint.R test file attempted to open", + fixed = TRUE + ), + "entrypoint.R has been opened in the editor" + ) + } ) }) test_that("edit throws a warning", { - orig_opts <- options(editor = function(name, file, title) NULL) - on.exit(options(orig_opts), add = TRUE) - expect_warning(plumb_api("plumber", "01-append", edit = TRUE)) + with_options( + list(editor = function(name, file, title) NULL), + expect_warning(plumb_api("plumber", "01-append", edit = TRUE)) + ) }) context("plumb() plumber APIs")