padrino-framework
padrino-framework copied to clipboard
padrino
Major routing syntax cleanup
Josh has some ideas for some fairly major routing API changes to cleanup the whole controller situation. The core of this is that there are a number of ambiguous or confusing options in Padrino right now and more then that http_router has caused us some issues and has required a lot of ugly monkeypatching.
Josh argued what if instead we just go back to using (and augmenting) sinatra's router rather then requiring http_router. More then that we can simplify the routing syntax.
This approach changes the following things:
- Map syntax (deprecated)
- With syntax (deprecated)
- Parent syntax (deprecated)
- Priority (changed to controller level?)
Below is a list of breaking api changes:
Basic Named Routes
The new syntax for named routes would be:
# verb alias, url
get :index, "/" do
end
get :show, "/:id/show" do
end
with the named route first and the url path second. Standard sinatra syntax is also supported.
Map Keyword
The map would be deprecated:
# DEPRECATED
get :account, :map => "account/bar" do
and changed to:
get :account, "account/bar" do
The with keyword
# DEPRECATED
get :account, :with => :id do
would be deprecated in favor of the more explicit:
get :account, "/:id" do
Controllers
Controllers would still act as a natural grouping for routes and would be prefixed into the route paths:
SimpleApp.controllers :admin do
would still work (and append "/admin" to all contained routes), this would also work:
SimpleApp.controllers :admin, "/bar" do
which would change the prefix in a way consistent with the new mapping syntax.
SimpleApp.controllers "/admin" do
also still works.
The parent keyword
# DEPRECATED
SimpleApp.controllers :product, :parent => :user do
would be more explicit:
SimpleApp.controllers :product, "/user/:user_id/products" do
The priority keyword
We would change priority to be for controllers rather than routes, i.e
get :show, :map => '/*page', :priority => :low do
becomes
MyApp.controllers :products, :priority => :low
@achiu @DAddYE @skade I know this is a big change in some respects (favoring more explicitness) but I think this removes a lot of weird 'magic' and ambiguity in our routing. A new typical controller would look like:
MyApp.controller :posts, "/users/:user_id/posts" do
before :index, :create do
# ...
end
get :index, "/" do
# /posts/
end
get :show, ":id" do
# /posts/:id
end
get :new, "new" do
end
post :create, "/" do
# /posts
end
get :edit, ":id/edit" do
end
put :update, ":id" do
# /posts/:id
end
delete :destroy, ":id" do
# posts/:id
end
end
yes this looks very dry and also much more RESTful. I'd like the named route to be appended rather than prefixed with the given id. get :edit, :id do #/customer/:id/edit Not like today where it's /customer/edit/:id
Given the number of times I've tripped over the different keywords, I can only back this. The keywords where mostly inexpressive and with the new syntax I get the feeling that looking at a controller I might actually see my routes. :-)
+1
Yeah that's the way we saw it too. I think the number of keywords and the inconsistent behavior really only served to confuse
Love it! +1, I prefer that @joshbuddy make it inside padrino instead of making patch/adapters.
I agree with DAddYE on this one, as having as much as possibly tightly knitted to the other parts does indeed improve performance and readability.
Once (I don't know if it still is like that) you could define routes in Rails, where if the name of the route was :show and it wasn't mapped to a specific URI, it'd simply map it as /whatevers/:id, and not like Padrino does now, which is map it to /whatevers/:id/show. I would like to request this feature in padrino as well, unless of course it has been removed since, for a valid reason.
+1 for this. Every time I upgrade padrino I have problems with routes that start working in an inconsistent way.
https://github.com/joshbuddy/http_router/issues/22
I would enjoy if there was no special meaning to route keys, but a way to configure those. For example:
get :show do
end
And somewhere else:
default :show => ':controller/:id'
Adding wiki to document changes in next major point release: https://github.com/padrino/padrino-framework/wiki/Changes-for-Major-Release. Before releasing we need to document these breaking changes.
Maybe I can help and contibute these mayor changes in the documentation page of padrino.
Guys, why can't it be something as simple as:
get :index # => "/"
get :show, :id # => "/:id"
get :edit, :id # => "/edit/:id" (or as a special case "/:id/edit")
get :custom, :param1, :param2, :param3 # => "/custom/:param1/:param2/:param3"
Typing all those route strings for each action doesn't feel dry, especially for the routes like 'get :edit, ":id/edit"'. Ideally it can be something like
get :edit do |id|
end
get :edit, :id # => "/:id/edit" feels more RESTlike. You specify the resource you want to edit first. Specifiying /edit/ first is RPC style, imho and always bothers me in padrino (thats why I write all my routes by hand).
Yep, I agree that the /:id/edit looks better than /edit/:id, the point is that the get :edit, :id looks simpler than get :edit, ":id/edit", especially considering that there are a lot of actions with a single :id parameter.
Fully agree with you. Routes should be simple to define in the first place in a REST based app.
@mitya
So whats your idea for overloading?
As I said, I would prefer some preconfigured, but flexible way to attach default routes to labels. For example:
module RouteGenerator
#ensures that all :edit routes are generated as /:id/edit unless specified otherwise
def edit
"/:id/edit"
end
end
And in the end, edit is an oddball in REST anyways, as it is only needed for browsers, so you are pretty free. You could argue about that one for ages.
Correct, edit is a special representation of the resource. One could argue that not only browsers but other apps could use this representation as well. Say for instance a fat client app that builds its edit form by using the /:id/edit.xml representation.
@skade, about attaching routes to labels — that's definitely a good idea, but when you attach a route to a label than it will also make sense to attach a verb to the same label (eg attach :update, :put, ":id"), and then in all controllers action :update do ... end.
But the resulting API (action :update do ... end) will be one level of abstraction higher than the API used to define routes (the put :update, ":id" do end). So I think that attaching labels to routes can be built on top of the routing API (including the current one), but it should not be mixed with it.
@mitya Padrino already uses the first argument as a label for generation, so it would not have to be build on top of it in any case.
@nesquena +1 from me. I agree that the current routes are a bit too magical and doesn't feel right.
Aren't you changing the behavior of with?
get :account, "/:id" do
This is the same syntax as map, so wouldn't that URL map to /:id instead of /account/:id?
Butt howto include the "/" character in a parameter? like this:
get :tree, "/:branch/:folder(.*$)"
get :tree, :with => [:branch, :folder => /.*$/]
http://my.com/tree/master/lib/good/folder
params[:branch] = 'master'
params[:folder] = '/lib/good/folder'
It''s not supported yet. And this is not supported too:
get %r{/post/(\d\d)-(\d\d)-(\d\d\d\d)} do|month,day,year|
"Post requested from #{month}/#{day} in #{year}"
end
Current the only way is:
get %r{/post/(\d\d)-(\d\d)-(\d\d\d\d)} do
month= params[:captures][0]
day= params[:captures][1]
year= params[:captures][2]
end
btw, the Regular Expression named parameters can use too:
get %r{/post/(?<month>\d\d)-(?<day>\d\d)-(?<year>\d\d\d\d)}, :name => 'post', :generate_with =>'/post/' do
"Post requested from #{params[:month]}/#{params[:day]} in #{params[:year]}"
end
And raise the "padrino rake routes" error
ok, Got it in http_router:
get :index, '/tree/:branch/:dir', :dir => /.*/ do
Also see #820 for a common problem: should we allow routes with the same name, but different signatures?
If we're going to re-work routes, what do people think of making controllers classes instead of blocks? One of the big problems I have at the moment is that sharing functionality between routes is awkward. Right now I have a controller helper that has generic helper methods for controllers, but these are shared between all routes and controllers, so it turns into a big mess of seemingly unrelated methods.
With a controller class, you can define shared functionality as methods on the class. You also get inheritance, so like with my latest project, two controller that are very similar can share the majority of their code, with routes being overloaded only as needed. I just think class-based controllers give the user a lot more power without the awkwardness of the current controller block.
You'd still retain all of the current methods, e.g. you'd still have #get, #post, #put, etc, and access to the everything routes normally have access to. You're simply wrapping them in the construct of a class (which is instanced when a request comes in), instead of a block.
Thoughts?
@pepe I've actually since started writing my own sinatra/padrino-inspired framework as we speak. It's in it's early stages of implementation, but I've already spent a lot of time playing with and refining the API and core constructs. Class-based controllers which respond to #call are the main construct; every object in the stack is rack-compliant, even routes expect a rack environment hash. Everything is meant to plug and play. I'm really happy with how it's shaping up.
Sinatra itself is in need of a re-write which would suggest Padrino is probably equally so. I've been playing around with Sinatra, Padrino and Rack enough over the last 2 years to have a pretty good idea of what I want a ruby web framework to be, which is what Scorched will reflect. Thoughtful design is the projects #1 priority.
For a little more info, checkout the repo I only just created: https://github.com/Wardrop/scorched
what do you think?
##
# Funcional routing system...
#
# The method *desc* is not required, will be useful
# when you'll run
#
# rake routes
#
# or if you have an api interface to generate a
# documentation.
#
# Functional controllers are an alternative to the block syntax
#
Padrino::Application.new do
set :auto_run, true
# Here some serious stuff
desc 'Here the homepage'
get '/' do
# ...
link_to url.index
# ...
end
desc 'Get blog post with mandatory ...'
get '/post/(\d{2})/(\d{2})' do |month, year|
# ...
link_to url.post(10, 12)
# ...
end
desc 'Here an example of crud'
post blog: '/b-l-o-g/seo' do
# ...
# here we add explicitly add a custom alias
# ...
end
# New idea for controllers, other than previous blocky syntax
desc 'Start a functional controller'
controller blog: '/blog'
get do
# link_to url.blog.get, class: 'custom', id: 'mine'
# => http://example.com/blog
#
# link_to path.blog.get, class: 'custom', id: 'mine'
# => /blog
end
put do |id|
# => link_to url.blog.put(12)
# ...
end
del do |id|
end
desc 'Back to main, but now to provide api support'
controller '/'
provides :json
get '/fuck-this-seo-url' do
# => url.fuck_this_seo_url
end
get '/categories/' do
end
provides :xml
### ... and now in xml
end