Route dispatching needs to be smarter about parameters

[polotek]

First the problem. I've autogenerated routes for my controller with router.resource('posts'). Then I want to add some custom actions to the controller. But the dispatcher doesn't reach those routes because it gets stopped too soon by the show route. Like so.

// controller/posts.js

this.index = function() { ... }
this.show = function() { ... }
...
this.compose = function() { ... }

// routes.js
routes.resource('posts');
routes.get('/posts/compose').to('Posts.compose');

If you run the server and hit http://localhost:4000/posts/compose in your browser, you get an error.

TypeError: Cannot call method 'toObj' of undefined
    at /Users/polotek/src/writedown/app/controllers/posts.js:40:48
    at utils.mixin.load (/usr/local/lib/node_modules/geddy/node_modules/model/lib/adapters/memory/index.js:57:7)
    at Object.obj.all (/usr/local/lib/node_modules/geddy/node_modules/model/lib/index.js:407:25)
    at Function.obj.first (/usr/local/lib/node_modules/geddy/node_modules/model/lib/index.js:387:18)
    at show (/Users/polotek/src/writedown/app/controllers/posts.js:39:22)
    at callback (/usr/local/lib/node_modules/geddy/lib/controller/base_controller.js:347:22)
    at controller.BaseController._handleAction (/usr/local/lib/node_modules/geddy/lib/controller/base_controller.js:360:7)
    at finish (/usr/local/lib/node_modules/geddy/lib/app.js:376:34)
    at /usr/local/lib/node_modules/geddy/lib/app.js:559:19
    at localCallback (/usr/local/lib/node_modules/geddy/lib/sessions/index.js:

This is because the url was matched by the route /posts/:id.:format which was added by routes.resource. The problem is that it's not an id, it's a controller action. The easy fix is to put any custom routes before the call to router.resource. But I believe other frameworks like rails handle this issue. If the show controller tries to look up a post with the given id and fails, it should bubble back up to the route dispatcher and continue.

I could be wrong because I don't know a ton about rails. And our job would get more difficult because it would make route dispatching asynchronous. I'd rather avoid that. But I also feel like this problem is one that a lot of people are going to run into.

[techwraith]

Rails handles this the same way that Geddy does currently. Routes cascade, whatever matches first wins.

It would be interesting to to be able to throw a 404 from an action that would tell it to continue down the list, but this sounds pretty hellish from an implementation standpoint.

On Dec 11, 2012, at 9:03 PM, Marco Rogers notifications@github.com wrote:

First the problem. I've autogenerated routes for my controller with router.resource('posts'). Then I want to add some custom actions to the controller. But the dispatcher doesn't reach those routes because it gets stopped too soon by the show route. Like so.

// controller/posts.js

this.index = function() { ... } this.show = function() { ... } ... this.compose = function() { ... }

// routes.js routes.resource('posts'); routes.get('/posts/compose').to('Posts.compose'); If you run the server and hit http://localhost:4000/posts/compose in your browser, you get an error.

TypeError: Cannot call method 'toObj' of undefined at /Users/polotek/src/writedown/app/controllers/posts.js:40:48 at utils.mixin.load (/usr/local/lib/node_modules/geddy/node_modules/model/lib/adapters/memory/index.js:57:7) at Object.obj.all (/usr/local/lib/node_modules/geddy/node_modules/model/lib/index.js:407:25) at Function.obj.first (/usr/local/lib/node_modules/geddy/node_modules/model/lib/index.js:387:18) at show (/Users/polotek/src/writedown/app/controllers/posts.js:39:22) at callback (/usr/local/lib/node_modules/geddy/lib/controller/base_controller.js:347:22) at controller.BaseController._handleAction (/usr/local/lib/node_modules/geddy/lib/controller/base_controller.js:360:7) at finish (/usr/local/lib/node_modules/geddy/lib/app.js:376:34) at /usr/local/lib/node_modules/geddy/lib/app.js:559:19 at localCallback (/usr/local/lib/node_modules/geddy/lib/sessions/index.js: This is because the url was matched by the route /posts/:id.:format which was added by routes.resource. The problem is that it's not an id, it's a controller action. The easy fix is to put any custom routes before the call to router.resource. But I believe other frameworks like rails handle this issue. If the show controller tries to look up a post with the given id and fails, it should bubble back up to the route dispatcher and continue.

I could be wrong because I don't know a ton about rails. And our job would get more difficult because it would make route dispatching asynchronous. I'd rather avoid that. But I also feel like this problem is one that a lot of people are going to run into.

— Reply to this email directly or view it on GitHub.

[kieran]

We could accomplish this by calling something like:

// grab all the matching routes
var params_array = router.all( req.url, req.method )

var success = true
do {
  // pull the first match of the array
  var params = params_array.shift()
  // attempt dispatch
  success = fake_dispatch( params.controller, params.action, params )
} while (!success) // retry if the dispatch fails

This isn't very node-y, but you could pass in a next() method that does much the same thing. You get the idea.

Personally, I think this is a terrible idea. You can pre-vet the routes using match conditions so you never have to reject something in the controller.

For more complicated situations, there's also deferred routes, which I still haven't bothered writing tests or docs for. Sorry :-/

[kieran]

For the record, there's also (a probably undocumented) feature to nest routes under resources:

// only under the member (with :id)
router.resource('products').member(function(){
  this.get('/bob').to('Somewhere.else')
})

// only under the collection (index)
router.resource('products').collection(function(){
  this.get('/bob').to('Somewhere.else')
})

// or either (both?)
router.resource('products').nest(function(){
  this.get('/bob').to('Somewhere.else')
})

// routes work too (with nest)
router.get('/products/:id').to('Products.show').nest(function(){
  this.get('/bob').to('Somewhere.else')
})

(tests are here for examples)

My poor documentation practices, in a nutshell: