bploetz / versionist

A plugin for versioning Rails based RESTful APIs.
MIT License
972 stars 51 forks source link

Leading slashes on :path do not work with :resources #24

Closed abrisse closed 12 years ago

abrisse commented 12 years ago

Hi!

I would like to know if versionist was compatible with the Rails :shallow option ?

(:shallow - If true, paths for nested resources which reference a specific member (ie. those with an :id parameter) will not use the parent path prefix or name prefix.)

Here is my routes.rb :

api_version(:module => "V1", :path => "/v1") do
  resources :users, :shallow => true do
    resources :groups
  end
end

When running rake routes :

** Invoke routes (first_time)
** Invoke environment (first_time)
** Execute environment
rake aborted!
Invalid route name: 'new_/v1_user_group'
_/ruby-1.9.3-p194/gems/actionpack-3.2.1/lib/action_dispatch/routing/route_set.rb:353:in `add_route'
...
...
_/ruby-1.9.3-p194/gems/versionist-0.2.2/lib/versionist/routing.rb:41:in `configure_path'
_/ruby-1.9.3-p194/gems/versionist-0.2.2/lib/versionist/routing.rb:19:in `api_version'
bploetz commented 12 years ago

Thanks for the report. I don't think this has anything to do with the :shallow option, as when I take that out I still get the same error.

Looking into it.....

bploetz commented 12 years ago

It actually doesn't seem to have anything to do with the :shallow option after all. It's a bug in the handling of leading slashes in the path.

The following causes the error:

api_version(:module => "Api::V1", :path => "/v1") do
  resources :users, :shallow => true
end

The following works.

api_version(:module => "Api::V1", :path => "v1") do
  resources :users, :shallow => true
end
[bploetz:~/tmp/test-api-rails32]> rake routes
    v1_users GET    /v1/users(.:format)          Api::V1/users#index
             POST   /v1/users(.:format)          Api::V1/users#create
 new_v1_user GET    /v1/users/new(.:format)      Api::V1/users#new
edit_v1_user GET    /v1/users/:id/edit(.:format) Api::V1/users#edit
     v1_user GET    /v1/users/:id(.:format)      Api::V1/users#show
             PUT    /v1/users/:id(.:format)      Api::V1/users#update
             DELETE /v1/users/:id(.:format)      Api::V1/users#destroy

Note the leading slash is left off of the :path in the variant that works. This seems to be related to the gsub! that's happening here:

https://github.com/bploetz/versionist/blob/master/lib/versionist/routing.rb#L39

If you use something like match or get instead of resources....

api_version(:module => "Api::V1", :path => "/v1") do
  get "/test" => "test#test"
end

You'll see that it "works" but the route names are prefixed by a _ :

[bploetz:~/tmp/test-api-rails32]> rake routes
_v1_test GET /v1/test(.:format) Api::V1/test#test

Whereas this....

api_version(:module => "Api::V1", :path => "v1") do
  get "/test" => "test#test"
end

...results in this:

[bploetz:~/tmp/test-api-rails32]> rake routes
v1_test GET /v1/test(.:format) Api::V1/test#test

My suspicion is that the leading underscore that Versionist is adding is causing issues when using resources for some reason, regardless of whether :shallow is present or not.

So try leaving off the leading slash on the :path and see if that helps. I'll add some logic to remove the leading slash if it's present when using the Path strategy.

abrisse commented 12 years ago

Removing the leading slash indeed resolves the problem. Thanks Brian!

bploetz commented 12 years ago

@abrisse versionist 0.2.3 has been released with this fix. Can you update your Gemfile to depend on versionist 0.2.3, put back the leading slash on your :path and ensure that it works for you?

abrisse commented 12 years ago

Works perfectly!

bploetz commented 12 years ago

Great, thanks