ambiorix-web / ambiorix

🖥️ Web framework for R
http://ambiorix.dev
GNU General Public License v3.0
211 stars 9 forks source link

feat: nested routers initial pass #73

Closed JohnCoene closed 1 day ago

JohnCoene commented 3 days ago

Allows nesting routers.

Instead of grabbing the routes, receivers (websocket handlers), and middlewares when the router is passed to use we delay that execution until start where we recursively go through all 3 items. Doing so recursively means we can nest as many routers as we want.

@kennedymwavu probably needs more testing, I haven't tested middlewares but this should work too. It's useful if, for instance, you have a specific middleware to check for authentication that you only want to apply to a subset of routes that are protected, e.g.: homepage will likely not require authentication while all routes in /app may require auth.

library(htmltools)
library(ambiorix)

app <- Ambiorix$new()

user_router <- Router$new("/user")
profile_router <- Router$new("/profile")

# /user/profile
user_router$use(profile_router)

app$use(user_router)

user_router$get("/", \(req, res) {
  res$send(h1("Users"))
})

profile_router$get("/", \(req, res) {
  res$send(h1("The user"))
})

profile_router$get("/me", \(req, res) {
  res$send(h1("Me me me"))
})

app$get("/", \(req, res) {
  page <- div(
    tags$a(href = "/user", "User"),
    tags$a(href = "/user/profile", "Profile"),
    tags$a(href = "/user/profile/me", "Me")
  )
  res$send(page)
})

app$start()
kennedymwavu commented 3 days ago

this is really great! but now the existing functionality of middleware is broken :/ here's a small reprex:

devtools::load_all()

mdd <- \(req, res) {
  print("inside a middleware")
  forward()
}

home_get <- \(req, res) {
  res$send("Ambiorix")
}

Ambiorix$
  new(port = 8000L)$
  use(mdd)$
  get("/", home_get)$
  start()

this is the error i get on the console:

ℹ Loading ambiorix
Error in super$get_middleware() : object 'router' not found
Calls: <Anonymous> -> <Anonymous>
Execution halted

hint:

looks like there are two small bugs...

      for(receiver in private$.receivers) {
        receivers <- router$get_receivers(receivers)
      }
      for(middleware in private$.middleware) {
        middlewares <- router$get_middleware(middlewares)
      }
JohnCoene commented 3 days ago

Right, now we're good.

library(htmltools)
library(ambiorix)

app <- Ambiorix$new()

# routers
first_router <- Router$new("/first")
second_router <- Router$new("/second")
third_router <- Router$new("/third")

# middleware
second_middleware <- \(req, res) {
  cat("Hello from /second...\n")
}

# nest
second_router$use(second_middleware)
second_router$use(third_router)
first_router$use(second_router)

app$use(first_router)

first_router$get("/", \(req, res) {
  res$send(h1("Users"))
})

second_router$get("/", \(req, res) {
  res$send(h1("Second!"))
})

third_router$get("/", \(req, res) {
  res$send(h1("Third!"))
})

app$get("/", \(req, res) {
  page <- div(
    tags$a(href = "/first", "1"),
    tags$a(href = "/first/second", "2"),
    tags$a(href = "/first/second/third", "3")
  )
  res$send(page)
})

app$start()
jrosell commented 3 days ago

That't cool. One question, should we add an issue for adding route helpers similarly like ruby and rails routers have with support for nested routes?

For example, in the case of resources :photos:

photos_path returns /photos

new_photo_path returns /photos/new

edit_photo_path(:id) returns /photos/:id/edit (for instance, edit_photo_path(10) returns /photos/10/edit)

photo_path(:id) returns /photos/:id (for instance, photo_path(10) returns /photos/10)

How could be for profile paths?

kennedymwavu commented 3 days ago

Right, now we're good.

@JohnCoene good progress, the nesting is now working well. but...

if we have a middleware set for the second router, it should not run when endpoints of the first router are accessed. using your reprex:

here's the console output:

ℹ 10-10-2024 10:17:24 GET on /
Hello from /second...
ℹ 10-10-2024 10:17:45 GET on /first
Hello from /second...
ℹ 10-10-2024 10:18:39 GET on /first/second
JohnCoene commented 3 days ago

@kennedymwavu I'll check it should indeed not work like that but I was sure I got this right, I don't remember seeing that, I probably did something wrong.

@jrosell we support parameters in ambiorix so you can totally do that, I'll share an example, unless I misunderstand your question.

JohnCoene commented 2 days ago

I fixed it, though it breaks all the tests... (it seems to be an issue with testthat to be honest, I'm getting some strange error).

And here's an example + for @jrosell using the parameters, try /first/second/third/fourth/something

app <- Ambiorix$new()

# routers
first_router <- Router$new("/first")
second_router <- Router$new("/second")
third_router <- Router$new("/third")
fourth_router <- Router$new("/fourth")

# middleware
second_middleware <- \(req, res) {
  cat("Hello from /second...\n")
}

# middleware
second_router$use(second_middleware)

# routers
third_router$use(fourth_router)
second_router$use(third_router)
first_router$use(second_router)
app$use(first_router)

first_router$get("/", \(req, res) {
  res$send(h1("First"))
})

second_router$get("/", \(req, res) {
  res$send(h1("Second!"))
})

third_router$get("/", \(req, res) {
  res$send(h1("Third!"))
})

fourth_router$get("/", \(req, res) {
  res$send(h1("fourth"))
})

fourth_router$get("/:this", \(req, res) {
  res$send(h1("path:", req$params$this))
})

app$get("/", \(req, res) {
  page <- div(
    tags$a(href = "/first", "1"),
    tags$a(href = "/first/second", "2"),
    tags$a(href = "/first/second/third", "3"),
    tags$a(href = "/first/second/third/fourth", "4")
  )
  res$send(page)
})

app$start()
jrosell commented 2 days ago

Sorry for not explaining it well. I'm talking about some path helper like:

For example some get_path(second_router) returning "/first/second" so it can be used in the links.

JohnCoene commented 2 days ago

Ah I get it!

I don't think you explained it poorly, I work on OSS in the wee hours of the night so I tend to misread.

We could totally consider that, I haven't explored rails but have only heard good things and DHH surely makes me want to try, I should take a look at rails to see if there are ideas/features we could port to Ambiorix.

I'm just wondering where you use those helpers in your code? Would you have an example or pseudo code for me to understand?

jrosell commented 2 days ago

For unnested routers, could be:

Then you could have glue('<a href="{path(photos)}">List all</a> <a href="{new_path(photos)}">New</a> <a href="{edit_path(photos, 1)">Edit it</a> <a href="{path(photos, 1)}">Show it</a>')

Why is that useful? When you nest the routers, you don't have to change the HTML with ne new routes, the helpers make it right for you.

Rails priorizes convention over configuration, and I think it's wonderful principle (model name = url path = db table)

Hope it makes sense.

JohnCoene commented 2 days ago

Ah, I see!

Not sure how we can make this work but we can certainly look into it. It pertains to the rendering engine which I was just talking about with @kennedymwavu needs a good revamp.

kennedymwavu commented 1 day ago

I fixed it, though it breaks all the tests... (it seems to be an issue with testthat to be honest, I'm getting some strange error).

@JohnCoene what error are you getting? all tests are passing on my end, albeit with some warnings (see attached).

image

JohnCoene commented 1 day ago

I had one failure which I just fixed, not sure what I did before