A few days ago I posted a patch that adds :only and :except style conditions to route generation for resources. I've had a couple of +1s on Trac but someone suggested that the functionality should be discussed here.
I've just added a new patch that now works with r7416 so I'd like some feedback on whether this goes against the principles of RESTful routing or is it functionality people would like to see included.
http://dev.rubyonrails.org/ticket/9460
Andrew White
I don’t really believe in this kind of security. You implement a destructive controller method like “destroy”, and then you want to prevent access to it by cutting of the route? Why not solve it with proper access control, or simply declaring the method private? If the method is implemented in the controller then it must be because someone has to use it. And if not all people should be allowed to use it, implement authorization.
Imagine having dynamite in your house and then explaining the wife it is safe because you cut off the fuse.
Whilst it's true that you should remove the destroy action to be entirely safe, it's not beyond the realms of possibility that in a multi-person team someone might accidentally add a destroy action which shouldn't be there.
If you don't think it should be applied on security grounds what about efficiency? If your site consists of mainly read-only resources then you'll have 3-4 times as many routes as you really need.
Andrew White
I agree that this should not be considered a security feature, but the ability to only generate the routes that you are going to implement does seem somewhat useful.
It would reduce the chance of accidentally generating an url that leads nowhere. Yes, it adds a bit of overhead, but only if you actually want to use the feature.
So, is route matching that faster after this patch?
I am on the middle ground with this patch. It'd be good to remove the routes you're not using.
But at the same time, it'd make the complex routing code even more complex. Plus, the counter argument is, if you have read-only resources, why can't you just use named routes like :
map.with_options(:controller => 'whatever', :conditions => {:method => :get}) do |foo| ['all', 'your', 'readonly', 'actions'].each do |act| foo.send("#{something}_#{act}", "the/path/you/want", :action => act) end end
Untested. Should work I'd like to think.
So yeah, My vote is ±0 
So here's an excerpt from my application's routes.rb file with my patch:
map.namespace :shop do |shop| ... shop.resources 'products', :only => [:index, :show], :collection => { :search => :get } do |product| product.resources 'images', :only => :show do |image| image.resources 'thumbnails', :only => :show end end ... end
I don't think you can exactly replicate in named routes without basically writing it out longhand e.g:
map.with_options :controller => 'shop/products', :conditions => { :method => :get } do |products| products.shop_search_products 'shop/products/search', :action => 'search' products.shop_products 'shop/products', :action => 'index' products.shop_product 'shop/products/:id', :action => 'show' end
map.with_options :controller => 'shop/images', :conditions => { :method => :get } do |images| products.shop_product_image 'shop/products/:product_id/images/:id.:format', :action => 'show' end
map.with_options :controller => 'shop/thumbnails', :conditions => { :method => :get } do |thumbnails| thumbnails.shop_product_image_thumbnail 'shop/products/:product_id/images/:image_id/:id.:format', :action => 'show' end
Doing it this way would wear out my s, h, o & p keys I fear. 
Andrew White
Probably not, but it's not slower.
Andrew White
If your site consists of mainly read-only resources then you'll have 3-4 times as many routes as you really need.
Andrew White
Excellent. Entia non sunt multiplicanda sine necessitate!
I agree that this should not be considered a security feature, but the ability to only generate the routes that you are going to implement does seem somewhat useful.
It would reduce the chance of accidentally generating an url that leads nowhere. Yes, it adds a bit of overhead, but only if you actually want to use the feature.
Rails already deals with resource routes that leads to nowhere. We give you a 406 Not Supported back. Which is a lot better than the 404 Not Found that you'd get if this patch went into effect.
I'm -1 on this patch.
FYI, this appears to be incorrect - without the patch and no action defined you'll get a not found exception in every case. With the patch applied you'll at least get at method not allowed for update and delete.
Andrew White
> Rails already deals with resource routes that leads to nowhere. We > give you a 406 Not Supported back. Which is a lot better than the 404 > Not Found that you'd get if this patch went into effect.
FYI, this appears to be incorrect - without the patch and no action
defined you'll get a not found exception in every case. With the
patch applied you'll at least get at method not allowed for update
and delete.
Ah, you're right. This is me wishing that it'd return a 405. It's on missing respond_to's that we return a 406 if the requested content type is not available. We should figure out a way to automatically return 405 if the verb doesn't correspond to anything.
This could be as simple as ApplicationController having default methods for all the verbs that return 405, but of course will mostly be overwritten. Or there may be a cleaner way to do it.
In any case, I think 405 is what we should be doing and it should be happening automatically. If anyone wants to PDI that, it'd be great.
Rather than putting it directly in ApplicationController the patch I've uploaded in #9569 includes the seven basic actions into ActionController::Base and then subtracts those actions from the default list added to the hidden actions list.
The only issue I've yet to resolve in the patch is how to collect the full list of overridden actions. The current patch will list the ones implemented in the current subclass, but any overridden in the ancestor chain between ActionController::Base and subclasses parent will be missed out. Since this only affects the accuracy of the error message in development mode I'm not too worried but if someone who's Ruby fu is a bit stronger than mine can come up with a nice, simple way of getting all the overridden methods from an arbitrary position in the ancestor chain I'll amend the patch.
Andrew White