Add 405 http code handling

[meztez]

Fixes #493
Fixes #310

PR task list:

• Update NEWS
• Add tests
• Update documentation with devtools::document()

[schloerke]

(See comments)

[schloerke]

This is much better. Thank you.

It still feels weird overloading the $canServe() method by adding to the req object. Don't know if we need to just recursively reinspect at default404Handler() time. Will talk with @cpsievert later.

[schloerke]

Ok. I believe I have a solution that we can use inside default404handler(). It just needs access to the pr object which we can get from req$pr.

---

# this function works under the assumption that the regular route was not found.  
# Here we are only trying to find if there are other routes that can be 
allowed_verbs <- function(pr, path_to_find) {

  verbs_allowed <- c()

  # look at all possible endpoints
  for (endpoint_group in pr$endpoints) {
    for (endpoint in endpoint_group) {
      if (endpoint$matchesPath(path_to_find)) {
        verbs_allowed <- c(verbs_allowed, endpoint$verbs)
      }
    }
  }

  # look at all possible mounts
  for (i in seq_along(pr$mounts)) {
    mount <- pr$mounts[[i]]
    mount_path <- sub("/$", "", names(pr$mounts)[i]) # trim trailing `/`

    # if the front of the urls don't match, move on to next mount
    if (!identical(
      substr(path_to_find, 0, nchar(mount_path)),
      mount_path
    )) {
      next
    }

    # remove path and recurse
    mount_path_to_find <- substr(path_to_find, nchar(mount_path) + 1, nchar(path_to_find))
    m_verbs <- allowed_verbs(mount, mount_path_to_find)

    # add any verbs found
    if (length(m_verbs) > 0) {
      verbs_allowed <- c(verbs_allowed, m_verbs)
    }
  }

  # return verbs
  return(verbs_allowed)
}

pr <- plumber$new()
sub <- plumber$new()
sub$handle("GET", "/test", force)
pr$mount("/barret", sub)


allowed_verbs(pr, path_to_find = "/barret/test")        # "GET"
allowed_verbs(pr, path_to_find = "/subroute/not_found") # NULL
allowed_verbs(pr, path_to_find = "/barret/")            # NULL
allowed_verbs(pr, path_to_find = "/barret/wrong")       # NULL


####################


is_405 <- function(pr, path_to_find, verb_to_find) {
  verbs_allowed <- allowed_verbs(pr, path_to_find)
  
  # nothing found, not 405
  if (length(verbs_allowed) == 0) return(FALSE)

  !(verb_to_find %in% verbs_allowed)
}

is_405(pr, path_to_find = "/barret/test", "GET")  # FALSE
is_405(pr, path_to_find = "/barret/test", "POST") # TRUE
is_405(pr, path_to_find = "/subroute/not_found")  # FALSE

Will need a new method (as you said) to match the path.

# plumber-step.R
# PlumberEndpoint public defs
    #' @description ability to serve request
    #' @param req a request object
    #' @return a logical. `TRUE` when endpoint can serve request.
    canServe = function(req){
      req$REQUEST_METHOD %in% self$verbs && self$matchesRoute(req$PATH_INFO)
    },
    #' @description determines if route matches requested path
    #' @param path a url path
    #' @return a logical. `TRUE` when endpoint matches the requested path.
    matchesPath = function(path) {
      !is.na(stri_match_first_regex(path, private$regex$regex)[1,1])
    },

Untested 404 handler update...

  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"))
  }

---

...woot

[schloerke]

(done with updates in the above comment)

[meztez]

pr is not a value of req, did you expect it to be or should I add it?

[schloerke]

Sorry. It is currently not implemented.

See #310

Want to add it to this PR? Or can make separate PR

[schloerke]

The pr$req could prolly be added here:

plumber/R/plumber.R

Line 584 in 038e0bc

[meztez]

I added it here :

plumber/R/plumber.R

Line 814 in 038e0bc

req$.internal <- new.env()

, above route

[meztez]

Im finishing up, just need to fix test because we use a mock req with make_req that does not have a router

[schloerke]

@meztez Is this PR ready for review / merging?

[schloerke]

@meztez Is this PR ready for review / merging?
(resending again as it was close to the other PR. Sorry for the spam).

[meztez]

@schloerke yes

[cpsievert]

LGTM pending test