adrian-gomez / swaggard

Add Swagger documentation to your Rails REST endpoints.
MIT License
54 stars 39 forks source link

JSON doesn't include all the endpoints. #14

Open ankurmantri opened 8 years ago

ankurmantri commented 8 years ago

If you have more than one endpoints using same controller#action, the JSON object created contains only the last one.

adrian-gomez commented 8 years ago

Hi, can you provide an example? it seems strange that you have more than one endpoint for the same controller/action.

ankurmantri commented 8 years ago

For example, let's say I have two controllers (controller1 and controller 2) and have 5 endpoints in total

/api/employee(.:format) controller1#index /api/:members/employee(.:format) controller1#index

/api/company(.:format) controller2#index /api/:members/company(.:format) controller2#index /api/:group/company(.:format) controller2#index

It shows me only these in my output. (the last ones from both the controllers)

/api/:members/employee(.:format) controller1#index /api/:group/company(.:format) controller2#index

Does it overwrite the other endpoints? If yes, is there a way to avoid that?

adrian-gomez commented 8 years ago

I'll try to replicate that setup and find whats the issue. It should work but there might some issues when doing the controller <=> route matching.

ankurmantri commented 8 years ago

Awesome. I'll keep checking in. Rest everything works the way it should. Thanks!

adrian-gomez commented 8 years ago

is this the setup you have?:

class Api::OneController < ApplicationController
  def index
  end
end

class Api::TwoController < ApplicationController
  def index
  end
end

routes.rb

get 'api/employee', controller: :one, :action: index
get 'api/:members/employee', controller: :one, :action: index

get 'api/company', controller: :two, :action: index
get 'api/:members/company', controller: :two, :action: index
get 'api/:group/company', controller: :two, :action: index
ankurmantri commented 8 years ago

to be more specific

 class OneController < ApplicationController
    def index
    end
 end

 class TwoController < ApplicationController
    def index
    end
 end

routes.rb

Rails.application.routes.draw do
    mount Swaggard::Engine, at: '/swagger'

    scope :api do
      get '/employee' => 'one#index'
      get '/:members/employee'  => 'one#index'

      get '/company'  => 'two#index'
      get '/:members/company'  => 'two#index'
      get '/:group/company'  => 'two#index'
   end 
 end
adrian-gomez commented 8 years ago

I'm testing with that, I will let you know what I can find. You have a really strange setup for your routes/actions, I'm sure that if you create a different action per path all would work ok. Even more assuming you have this: routes.rb

  namespace :api do
    get 'employee', controller: :one, action: :index
    get ':members/employee', controller: :one, action: :index

    get 'company', controller: :two, action: :index
    get ':members/company', controller: :two, action: :index
    get ':group/company', controller: :two, action: :index
  end

this 2 routes are the same: get ':members/company', controller: :two, action: :index and get ':group/company', controller: :two, action: :index so you should keep only one of them. I cannot thing of a good reason why you would want to keep both

adrian-gomez commented 8 years ago

For the moment the routes generated per controller/action pair, that is only one one path can point to a single controller/action. I will try to address that asap

ankurmantri commented 8 years ago

Maybe my example is not a very good one but what I'm trying to achieve is the same type of data but with different parameters. Controller2 returns just returns company's information and the different endpoints are just using different parameters. I don't see the need to make separate actions for these as it would lead to code duplication.

adrian-gomez commented 8 years ago

Not necessarily code duplication:

class Api::OneController < ApplicationController
  def employee
    render_employee
  end

  def members_employee
    render_employee(param[:members])
  end

  private

  def render_employee(members = nil)
    employee = logic_to_get_employee_if_memebers_is_given_or_not

    render json: employee
  end
end

routes.rb:

  namespace :api do
    get 'employee', controller: :one, action: :employee
    get ':members/employee', controller: :one, action: :members_employee
  end

That being said I'm investigating to see if I can archive what you need, with the current status of the code its difficult because everything is done iterating over the controllers actions, so if multiple paths point to the same controller/action only the first matching will be set. Some rewrites would be needed to support this use case. I think swaggard not supporting this its a bug and I'll try to fix it for future versions

ankurmantri commented 8 years ago

It would be nice to have this fixed. I don't want to refactor my entire project for this.