fix: Handlers route edge cases

ixpantia · Oct 16, 2024 · 867c14d · 867c14d
1 parent 4f9df57
commit 867c14d
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 7 deletions.
diff --git a/R/http_helpers.R b/R/http_helpers.R
@@ -10,8 +10,9 @@ add_route <- function(tower, method = "GET", path, handler) {
   handler <- compiler::cmpfun(handler)
   route_handler <- compiler::cmpfun(function(req) {
     if (req$REQUEST_METHOD == method && req$PATH_INFO == path) {
-      handler(req)
+      return(handler(req))
     }
+    req$NEXT(req)
   })
   return(add_http_layer(tower, route_handler))
 }

diff --git a/R/service.R b/R/service.R
@@ -34,10 +34,9 @@ print.tower <- function(x, ...) {
 #'   will be called before the 'shiny' app's httpHandler.
 #' @param tower A tower
 #' @param layer A function that takes a request and returns either
-#'   a response or NULL. NULL indicates that the layer did not
-#'   short-circuit the request, therefore the next layer should be
-#'   called. If a response is returned, the request is short-circuited
-#'   and the response is returned to the client.
+#'   a response. A layer can short circuit by returning a response
+#'   directly or call the next layer will `req$NEXT(req)` which
+#'   will call the next layer in the middleware.
 #' @return The tower with the layer added
 #' @export
 add_http_layer <- function(tower, layer) {

diff --git a/tests/testthat/test-http_helpers.R b/tests/testthat/test-http_helpers.R
@@ -257,7 +257,7 @@ test_that("can extract req body as a json on multiple layers", {
     tower::add_http_layer(function(req) {
       body <- tower::req_body_json(req)
       testthat::expect_equal(body$secret, "Hello, world!")
-      return(NULL)
+      req$NEXT(req)
     }) |>
     tower::add_post_route("/hello", function(req) {
       shiny::httpResponse(
@@ -332,7 +332,7 @@ test_that("can extract req body form on many layers", {
     tower::add_http_layer(function(req) {
       body <- tower::req_body_form(req)
       testthat::expect_equal(body$secret, "Hello, world!")
-      return(NULL)
+      req$NEXT(req)
     }) |>
     tower::add_post_route("/hello", function(req) {
       shiny::httpResponse(

diff --git a/tests/testthat/test-test_shiny_middleware.R b/tests/testthat/test-test_shiny_middleware.R
@@ -60,6 +60,47 @@ test_that("http middleware can be forwarded", {
       if (req$PATH_INFO == "/about") {
         return(shiny::httpResponse(200, "text/plain", "About page"))
       }
+      req$NEXT(req)
+    }) |>
+    tower::build_tower()
+
+  request <- list(
+    PATH_INFO = "/",
+    REQUEST_METHOD = "GET"
+  )
+
+  parts <- app |>
+    tower::app_into_parts()
+
+  response <- parts$ui(request)
+
+  expect_equal(response$status, 200)
+  expect_equal(response$content_type, "text/html; charset=UTF-8")
+  expect_true(stringr::str_detect(response$content, "<!DOCTYPE html>"))
+
+  request <- list(
+    PATH_INFO = "/about",
+    REQUEST_METHOD = "GET"
+  )
+
+  response <- parts$ui(request)
+
+  expect_equal(response$status, 200)
+  expect_equal(response$content_type, "text/plain")
+  expect_equal(response$content, "About page")
+
+})
+
+test_that("http middleware can be forwarded with helpers", {
+  app <- shiny::shinyApp(
+    ui = shiny::fluidPage(),
+    server = function(input, output, session) {}
+  )
+
+  app <- app |>
+    tower::create_tower() |>
+    tower::add_route("GET", "/about", function(req) {
+      return(shiny::httpResponse(200, "text/plain", "About page"))
     }) |>
     tower::build_tower()