Add :only/:except options to map.resources

Having tried to get this functionality added before (http://dev.rubyonrails.org/ticke..., I'm definitely +1 on this idea. There was a discussion on the core mailing list as well so it might be worth your while to see why people objected to the idea before.

Thanks for the heads-up, Andrew. For the record, DHH said:

Rails already deals with resource routes that lead 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. (Thanks for taking the time and a shot at improving Rails, though. Please do keep that up!)

He's right that Rails already handles routes that lead to no action at all, although I'm surprised to hear that a 406 is considered "a lot better" than a 404 in this situation, since 406 is for when "the request is only capable of generating response entities which have content characteristics not acceptable according to the accept headers sent in the request", which isn't the case here at all -- 403-405 are all more appropriate.

HTTP semantics aside, the three pragmatic benefits I listed in the ticket (RAM consumption, granularity, documentation) still stand, and it would be appropriate for Rails to "get real" in this case. In particular, the amount of RAM used by all the unnecessary routes in a project I'm working on is really dealbreakingly prohibitive -- we've converted routes.rb to be written out "longhand" with named routes instead of map.resources, but that's much harder to maintain.

I'd like this functionality to be reconsidered. I'll see if I can merge the tests from the patch on Trac and then ask the core list to review.

Here's an updated patch, with tests adapted from those on http://dev.rubyonrails.org/ticke...>

This has missed the 2.2 boat sorry, we're due to cut a release any day now.

However can you show the memory stats and methodology that you're referring to with the routing changes here, it definitely seems interesting.

After we've branched for 2.2 I'd be happy to take something like this into edge, your tests are great :)

This looks relevant http://blog.hungrymachine.com/20...

And http://github.com/methodmissing/...

I remember supporting Andrew's Trac ticket for this. The syntax in the first post looks a bit iffy but this is definitely a feature I want.

Any syntax suggestions? The difficulty is in allowing the sets of both unformatted and formatted routes to be customised, and in making it easy to only include or only exclude a couple of actions from each.

The implementation assumes that you won't want a formatted route to an action that doesn't also have an unformatted route, which I think is sensible for almost all purposes. Don't know if there's a way to constrain the syntax to communicate that.

Might be nice to carry over the same terminology for ignoring/including actions from controller filters.

map.resources :things, :only => [:new, :create]

While this seems like a nice addition to routes, what do you guys think of having it be automatic based on action definitions? http://gist.github.com/23253 I'd prefer that if it were reasonable to achieve.

Also, I'm curious if a simpler routing implementation might be helpful. I haven't digged into how it's done now so I don't know if that would work, but just wondering if things could be done with fewer special objects or just simpler objects somehow.

Guys,

Here's another shot at this :

http://github.com/methodmissing/...

Refactoring of my initial plugin @ http://github.com/methodmissing/... with better test coverage and tighter framework integration.

Commits ( http://github.com/methodmissing/... ) >= 'Initialize routing after application classes loaded', 11/09/08.

Notes :

• Introduces AC::Base.formatted_routes = true | false

• AC::Base.formatted_routes true by default for backwards compat

• Supports a :format => true | false syntax for resource definition, with the sweet spot being AC::Base.formatted_routes turned off, and selectively enabled with the :format => true option

• Adjusts the Rails init order to setup routing AFTER preloading application classes ( default for production )

• This allows Routing to infer exactly which actions is defined, and only map routes && named routes for those.

• config.cache_classes = true required to enable route pruning

• There's off course also one more over the top optimization that can also yield smaller memory footprints ... the path && url named route variants, but that's pushing it too far.

All existing and new tests passing.I'll take it for a spin through a large app later today.

Also not clear on how to handle 404 and 406 responses, but it seems to be less important now that ActionController::RoutingError can be rescued by the app.

Thoughts ?

• Lourens

+1

I had similar results (heavy route memory usage) from running bleak as per

http://blog.hungrymachine.com/20...

A way to cut out these formatted routes like:

resources :users, :include_formatted_routes => false

would be very nice :)

Hi Lourens,

The plugin is good work, but not fine-grained enough for some common cases. In particular:

1. The choice between "no formatted routes" and "all formatted routes" per resource doesn't support the common case of only wanting formatted routes for one or two actions; it seems likely that most blog implementations fall into this category (i.e. only formatted_posts_path is required).

2. As per point 2 in the original ticket, deciding the set of available routes per-controller is too coarse-grained in the case where a single controller might be responsible for handling requests for multiple resources (e.g. in both nested and non-nested contexts) that naturally require different routes to be available (e.g. allow creation only when nested).

In addition to those granularity requirements, the controller introspection just strikes me as being too much magic, particularly since it requires changing the load order. It's also important that routes don't get hooked up by mistake, which IMHO justifies the "belt and braces" approach of explicitly specifying the routed actions in routes.rb, even if that does make things less DRY.

And I agree there'll never be any benefit in quibbling over _path/_url methods! :-)

Cheers, -Tom

Seth: that's almost how it works already, it's just that you need separate :only/:except options for both unformatted and formatted routes, so at the moment you'd need to say :actions => { :only => [:new, :create] } to distinguish it from :formatted => { :only => [:new, :create] }.

Tom,

Great points.Thanks for the feedback.

We have a x5 namespace project with a huge number of routes and my assumptions might have been biased towards not manually pruning and defining all of those, as well as the maintenance thereof.

@Tom: I agree on the granularity thing, it makes much more sense to allow some of the routes (posts) to be available formatted but others (users) not.

Beyond this issue though, has any work been done to see where the memory usage is within those segments?

Also, can you reveal a bit more information about the 100MB savings, what the routes looked like, how nested they were, etc etc.?

I definitely prefer the idea of :only and :except as we use those in other places. Using the definition of actions seems risky / tightly coupled. The lack of a method doesn't imply that there's no action with that name, as the presence of a template is sufficient.

A simpler routing implementation would certainly help some of this, but it's a significant undertaking. If we can find enough motivated hackers perhaps we can get it done in 2.3. I think the first step is to draw slightly thicker borders between routes and the rest of rails, and allow people to experiment in plugins.

After writing the original "100M" post, I believe I have a better solution to essentially make ".:format" optional, creating 50% of the routes than before.

See: http://blog.hungrymachine.com/20...

gist: http://gist.github.com/23712

This is not an original idea AFAIK. The Merb router has optional format's.

That said, I think the prune solutions are also awesome, and could be a secondary pass. I would point out, that a prune solution needs to account for "def foo" and the existence of a "foo.html.erb" file, which isn't accounted for in FooController#actions.

I like the idea of making formats optional, perhaps we can investigate that for 2.3 instead of adding an option like this?

Good point about implied actions via the presence of templates being a problem with the idea of making things automatic, Michael.

Tom, I see what you mean about the options interface now. I still don't like it, though. It got me to thinking..

What is the real value of the formatted named routes? Can you not just pass a :format param to any named route and get all the same behavior? What about just ditching formatted named routes altogether..?

I agree that making :format optional is a great idea, not least because it removes the confusion around how to specify options to map.resources in this patch (i.e. you could say :only => [:new, :create]) as desired, without worrying about how to turn formatted routes on/off (there won't be any).

For those following this thread, I've added a patch for the optional :format token here: http://rails.lighthouseapp.com/p...

Please take a look.

There's a small problem with this functionality, pointed out by Lourens: generating the index and show routes for a PostsController creates the posts and post named routes respectively, so that the posts_path and post_path helpers are made available to the application. The issue is that you might still need these helpers even if you don't want the index or show route: create also resides at posts_path, and update and destroy also reside at post_path.

Attached is a simple additional patch to fix this up; the idea is to ask for the appropriate named route when creating every route, not just index/show/new/edit, but only create the named route if it doesn't already exist. (This prevents the named route being reconnected by later actions, so that e.g. the post named route ends up being associated with destroy.)

The attempt to detect whether such a named route has already been defined is kind of clunky (@set.named_routes[route_name.to_sym].nil?) but it works fine. Extra tests included.

Good point, applied and pushed.

@Lourens: You should take a look and get involved in aaronbatalion's patch at #1359 Add optional :format argument to named routes

Before this ships, we should try to fix the unintuitive behaviour of inherited :only/:except as pointed out by Ryan Daigle on his blog. Currently the two options are inherited separately - when you supply one of them it doesn't knock out any inherited version of the other - but since :only takes priority you get unexpected behaviour when you inherit :only but supply :except (:except just gets ignored).

It's probably be better to have no inheritance at all than to have it work like that. It shouldn't be hard to fix, though. Just not from this iPhone.

Sure, take a crack at it and let us know. perhaps raise an error if both are present?

@Michael - 1359 updated

Okay, this patch should fix it. The new implementation uses :only and :except as two different ways to set the actual inherited option, :actions, which is kept private (much like :namespace). Since :only and :except aren't actually inherited any more, they can't interact in this weird way. Even more tests included.

Is there an actual use case for inheriting :only/:except ?

To me it seems to create more problems than it could ever solve. For example I can't limit the actions for a toplevel resource without setting explicit limits on all the child resources (trying to do the former is how I found out about the inheritance feature).

There also seems to be a bug with normal named routes under a resource with limiting actions, the named routes all get ":actions => [...]" requirement implicitly added to them which basically breaks all of them unless they explicitly set ":actions => nil".

  map.resources :posts, :except => [ :destroy ] do |posts|
    posts.random_named_route '/random'
  end

  # rake routes
  # post_random_named_route  /posts/:post_id/random   {:actions=>[:index, :create, :new, :edit, :show, :update], :controller=>"something", :action=>"random"}

I agree with Tarmo, inheriting doesn't really make sense.

Sounds reasonable guys. Open a new ticket with a patch and assign it straight to me.