Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple mounts of the same API class #570

Closed
USvER opened this issue Feb 4, 2014 · 32 comments
Closed

Allow multiple mounts of the same API class #570

USvER opened this issue Feb 4, 2014 · 32 comments

Comments

@USvER
Copy link

USvER commented Feb 4, 2014

So i have Assets and have many other models that has many Assets.
How do i expose this in API using grape?
For now i'm duplicating Assest class that is exposing Assets resource... But it's not DRY...
I have many relations... and my current project starts to look like copy-paste shit.

I want to have one class that exposes Assets resource no matter on wich level it is (as top level resource or nested). But when i mount that class twice, only last mounted resource works.

Thank you.

@USvER
Copy link
Author

USvER commented Feb 4, 2014

Quick and dirty fix is to load and evaluate same code in the different places

eval(IO.read(Rails.root.join('app', 'api', 'v1', 'mixins', 'assets.rb')))

I'm new to ruby, so maybe my solution is ugly... best i could think of...

@dblock
Copy link
Member

dblock commented Feb 5, 2014

I'd like to be able to allow multiple mounts at different places and levels. So lets rename this into a feature request.

@USvER
Copy link
Author

USvER commented Feb 5, 2014

That would be nice!
Eval works... but debuging is hard...

@USvER USvER closed this as completed Feb 5, 2014
@USvER USvER reopened this Feb 5, 2014
@hobofan
Copy link

hobofan commented Jul 7, 2014

We added a small bounty for this issue on bountysource.

@codenamev
Copy link

How would this be different than Modules? Can you provide a working example?

@hobofan
Copy link

hobofan commented Jul 8, 2014

The Modules section describes how you can mount multiple APIs in the same/different paths.

The problem is that if you mount the same API (e.g. Twitter::APIv2) inside different APIs (e.g Twitter::API and AllAPIs::API) the paths etc. of the first time it is mounted will be overwritten which will result in the API being mounted twice at the same location.

@dblock
Copy link
Member

dblock commented Jul 9, 2014

@hobofan I think you could write a failing spec for this. Would be greatly appreciated.

@dblock dblock added the bug? label Jul 9, 2014
Aigeruth added a commit to Aigeruth/grape that referenced this issue Jul 21, 2014
@dspaeth-faber
Copy link
Contributor

The issue arises within Grape::API#mount. There is :mount_path set on class level for the API-Class. With this information route.route_compiled will be created later (some how) and this is always the same (because it uses the class level settings). I think that's no easy fix.

My workaround would be:

module APILogic
  def self.included(base)
    base.instance_eval do
      desc "Return a personal timeline."
      get :home_timeline do
        authenticate!
        current_user.statuses.limit(20)
      end
    end
  end
end

class API1 < Grape::API
  include APILogic
end

class API2 < Grape::API
  include APILogic
end


class Twitter::API < Grape::API
  mount API1 => '/mounted'
  mount API2 => '/double'
end

@lasseebert
Copy link
Contributor

The behavior achieved by @dspaeth-faber's example is the expected behavior, and should be default. I can't see an easy fix for this, but two possible fixes could be:

  • Don't have static config (class level config) on api classes. This is probably a major refactoring.
  • Somehow clone the class when mounting, so that only a copy of the class is mounted. This is not trivial, but my guess is that it is an easier fix.

@dspaeth-faber
Copy link
Contributor

@lasseebert cloning is not trivial. What count's in the end and what all the issues are about are the Endpoints. When you clone the API-Class you have to clone the Endpoints too. The Endpoints carry the routing informations. But when you clone the Endpoints and you change the original class after warts and add some routes then the cloned class will not have all the routes the original one has.

swagger-node-express walks around this. The "Endpoints" have no information about the route at all. They are only handler methods. The path information is hard coded for documentation purposes but you are free to mount an Endpoint where ever you want.

grape needs at the moment the routing information within the Endpoint to generate a good documentation and uses it directly so that Endoints can register them selfs within a Rack::Mount::RouteSet. In theory it can be handled differently.

But like you recognized major refactoring and major breaking change.

@lasseebert
Copy link
Contributor

I have made a neat superclass if this is something used a lot in a project:

class MountableAPI
  def self.anonymous_class
    Class.new(Grape::API).tap do |klass|
      klass.instance_eval(&@proc)
    end
  end

  def self.mounted(&block)
    @proc = block
  end
end

And use it like this:

class FooAPI < MountableAPI
  mounted do
    get '/foo' do
      # ...
    end
  end
end

And mount it like this:

# Inside another Grape app
mount FooAPI.anonymous_class => '/'

@dspaeth-faber
Copy link
Contributor

Yeah, and with this you bring up a painful subject. grape uses Classes where it should use objects.
It should look more like this with grape:

  mount Grape::API.new => '/'

@dblock dblock added the discuss! label Oct 2, 2014
@lasseebert
Copy link
Contributor

Agreed! 👍

@u2
Copy link
Contributor

u2 commented Apr 15, 2015

Intreasting!I want to make a PR,but it looks not easy for me.

@elado
Copy link
Contributor

elado commented Jun 10, 2015

Just encountered the same issue, made a spec to show the issue (can be used in a PR):

require 'spec_helper'

describe Grape::API do
  subject do
    class Stats < Grape::API
      get :stats do
        "stats"
      end
    end

    class Main < Grape::API
      resource :one do
        mount Stats
      end

      resource :two do
        mount Stats
      end
    end

    Main
  end

  def app
    subject
  end

  it 'can mount api in multiple resources' do
    get '/one/stats'
    puts one: last_response.body # => Not Found
    get '/two/stats'
    puts two: last_response.body # => stats

    get '/one/stats'
    expect(last_response.body).to eq('stats')

    get '/two/stats'
    expect(last_response.body).to eq('stats')
  end
end

@elado
Copy link
Contributor

elado commented Jun 10, 2015

@lasseebert's workaround works:

require 'spec_helper'

class MountableAPI
  def self.anonymous_class
    Class.new(Grape::API).tap do |klass|
      klass.instance_eval(&@proc)
    end
  end

  def self.mounted(&block)
    @proc = block
  end
end


describe Grape::API do
  subject do
    class Stats < MountableAPI
      mounted do
        get :stats do
          "stats"
        end
      end
    end

    class Main < Grape::API
      resource :one do
        mount Stats.anonymous_class
      end

      resource :two do
        mount Stats.anonymous_class
      end
    end

    Main
  end

  def app
    subject
  end

  it 'can mount api in multiple resources' do
    get '/one/stats'
    puts one: last_response.body
    get '/two/stats'
    puts two: last_response.body

    get '/one/stats'
    expect(last_response.body).to eq('stats')

    get '/two/stats'
    expect(last_response.body).to eq('stats')
  end
end

@dblock
Copy link
Member

dblock commented Jun 10, 2015

@elado This hack is interesting, do you think this is something we could roll into Grape in some way? Maybe we could be mounting an anonymous class that inherits from the API class instead of an eval-ed one?

@lasseebert
Copy link
Contributor

Another tip for anyone interested:

The way I use the MountableAPI class, I often need to set some variables on the new anonymous class. Variables that differ from mount point to mount point.

I have handled it by just setting an instance variable on the class and fetch it from a helper method:

class MountableAPI
  def self.anonymous_class(options = {})
    Class.new(BaseAPI).tap do |klass|
      klass.instance_eval(&@proc)
      klass.instance_variable_set(:@mount_options, options.dup)
      klass.helpers do
        def mount_options
          options[:for].instance_variable_get(:@mount_options)
        end
      end
    end
  end

  def self.mounted(&block)
    @proc = block
  end
end

Usage:

class FooAPI < MountableAPI
  mounted do
    get '/foo' do
      something = mount_options[:something]
      ...
    end
  end
end

# Inside a Grape app:
mount FooAPI.anonymous_class(something: "Hello world!")

# Inside another Grape app or in another namespace:
mount FooAPI.anonymous_class(something: "Waynes world!")

@elado
Copy link
Contributor

elado commented Jun 16, 2015

@dblock I'd love to see it in Grape. Yes, it'd be better to have a global solution that doesn't need eval and "just works" with mount. How big would a change like that be?

@dblock
Copy link
Member

dblock commented Jun 17, 2015

I am not sure @elado, if you want to try a PR we can discuss it over that.

rnubel pushed a commit to rnubel/grape that referenced this issue Jul 9, 2015
@rnubel
Copy link
Contributor

rnubel commented Jul 20, 2015

So, I spent a couple nights looking into this and found that (with much hackery) it is possible to have the mount method create an independent copy of the mounted class, scoped with the same settings it's mounted in (addressing #1050), and thus permit a single API to be mounted twice with predictable results. However, it breaks the current expectation for nested, post-definition mounting -- since mounting copies the class definition as-is, later modification to the old class doesn't affect the mounted copy.

But because the act of mounting an API requires modifying its class-level configuration, I don't see a way to preserve both behaviors. If we wanted to allow an API to be modified (e.g. by mounting another API inside of it) after it's mounted, it'd be hard to make it also be able to be mounted twice (a single API class can't maintain two sets of configuration, and a copied class would need some way to 'refresh' its configuration from the original class).

Personally, I think it makes sense to expect mounting to be nested inside-out (define your smallest units, mount them to create a bigger API). But that doesn't match existing behavior. To allow both, a re-architecture of how endpoints are defined and compiled into routes might be a better idea... though a significant amount of work.

@dblock
Copy link
Member

dblock commented Jul 20, 2015

Thanks @rnubel. I feel like we should be mounting instances of API classes, not static versions of those classes. Does this help in terms of direction?

@salimane
Copy link
Contributor

I've just met this problem. anyone still working on this ?
Thanks

@dblock
Copy link
Member

dblock commented Jan 29, 2016

@salimane I doubt it.

@toupeira
Copy link

toupeira commented Jul 20, 2017

Ran into this issue as well while working on GitLab, they have quite an extensive API and I saw two workarounds for this so far:

@kmhosny
Copy link

kmhosny commented Sep 21, 2017

the only way i found to allow for the same API path to be mounted under different header versions was
version 'v2', 'v3', using: :header, vendor: 'X'
but this means i had to rely on helpers a lot to differentiate between the headers specially that v3 enables oauth2 so i had to do it instead of have a separate class with separate helpers.

@dvandersluis
Copy link

I know this is a pretty old ticket that's been mostly dead for a while, but I'm wondering if there's any progress here or anyone knows how to make this work? I tried @lasseebert's Mountable class, but found that it only works one level deep (ie. V2 can "inherit" V1, but if I want V3 to inherit V2, it loses the endpoints from V1 unless I explicitly mount that in V3 too).

@arempe93
Copy link
Contributor

arempe93 commented Mar 19, 2018

@dvandersluis not sure if you found a solution but I'll add mine since I came across this ticket.

I just used a "meta-programming" type approach

module API
  module Client
    class Comments < Grape::API
      [Post, Profile, Story].each do |model|
        namespace model.table_name do
          route_param :ref, type: String do
            namespace :comments do
              desc "List comments for #{model.name.downcase}"
              params do
                optional :per_page, type: Integer, default: 10
                optional :page, type: Integer, default: 1
              end
              get do
                commentable = find(model, ref: params[:ref])
                present :comments, paginate(commentable.comments)
              end

              desc "Create comment in #{model.name.downcase}"
              params do
                requires :body, type: String
              end
              post do
                commentable = find(model, ref: params[:ref])

                comment = commentable.comments.create!(author_id: params[:_user_id], body: params[:body])
                MentionProcessingWorker.perform_async(comment.id, comment.base_class_name, :body)

                present :comment, comment
              end
            end
          end
        end
      end

      namespace :comments do
        route_param :id, type: Integer do
          desc 'Edit comment'
          params do
            requires :body, type: String
          end
          put do
            comment = find(Comment, id: params[:id])
            forbidden! unless comment.author_id == params[:_user_id]

            comment.update!(body: params[:body], edited: true)
            MentionProcessingWorker.perform_async(comment.id, comment.base_class_name, :body)

            present :comment, comment
          end
        end
      end
    end
  end
end

This creates the following routes when mounted

GET  /posts/:ref/comments
POST /posts/:ref/comments
PUT  /posts/:ref/comments/:id

GET  /profiles/:ref/comments
POST /profiles/:ref/comments
PUT  /profiles/:ref/comments/:id

GET  /stories/:ref/comments
POST /stories/:ref/comments
PUT  /stories/:ref/comments/:id

@dvandersluis
Copy link

I played around with a bunch of different metaprogramming but never got a solution that worked 100%. I really hope grape could support this properly and encourage code reuse.

@prawana-perera
Copy link

prawana-perera commented Feb 10, 2019

@dvandersluis not sure if you found a solution but I'll add mine since I came across this ticket.

I just used a "meta-programming" type approach

module API
  module Client
    class Comments < Grape::API
      [Post, Profile, Story].each do |model|
        namespace model.table_name do
          route_param :ref, type: String do
            namespace :comments do
              desc "List comments for #{model.name.downcase}"
              params do
                optional :per_page, type: Integer, default: 10
                optional :page, type: Integer, default: 1
              end
              get do
                commentable = find(model, ref: params[:ref])
                present :comments, paginate(commentable.comments)
              end

              desc "Create comment in #{model.name.downcase}"
              params do
                requires :body, type: String
              end
              post do
                commentable = find(model, ref: params[:ref])

                comment = commentable.comments.create!(author_id: params[:_user_id], body: params[:body])
                MentionProcessingWorker.perform_async(comment.id, comment.base_class_name, :body)

                present :comment, comment
              end
            end
          end
        end
      end

      namespace :comments do
        route_param :id, type: Integer do
          desc 'Edit comment'
          params do
            requires :body, type: String
          end
          put do
            comment = find(Comment, id: params[:id])
            forbidden! unless comment.author_id == params[:_user_id]

            comment.update!(body: params[:body], edited: true)
            MentionProcessingWorker.perform_async(comment.id, comment.base_class_name, :body)

            present :comment, comment
          end
        end
      end
    end
  end
end

This creates the following routes when mounted

GET  /posts/:ref/comments
POST /posts/:ref/comments
PUT  /posts/:ref/comments/:id

GET  /profiles/:ref/comments
POST /profiles/:ref/comments
PUT  /profiles/:ref/comments/:id

GET  /stories/:ref/comments
POST /stories/:ref/comments
PUT  /stories/:ref/comments/:id

This approach worked for me, if anyone is still looking how to do it.

namespace :namespace_1 do
       resource :my_resource do
       end
end

# some other place
namespace :namespace_2 do
        resource :my_resource do
       end
end

# or even, somewhere else
resource :my_resource do
end

And I get

/namespace_1/my_resource
/namespace_2/my_resource
/my_resource

@urkle
Copy link
Contributor

urkle commented Apr 19, 2019

So.. I'm very confused as to why this is still open.. From reading it you are simply wanting to re-use a set API endpoint beneath multiple other api endpoints which is supported in Grape already. I've been building new APIs recently and we did this to handle re-using endpoints (and so I can have each model in their endpoint class).
In this example we have Organizations Projects, and Tasks. And I want to expose the following endpoints

GET /organizations (Get all orgs I am in)
GET /organizations/# (get a specific org)
GET /organizations/#/projects (get all projects for org)
GET /organizations/#/tasks (get all tasks for all projects in the org)
GET /projects/# (get project)
GET /projects/#/tasks (get all tasks for a project)
class Resources::Organizations < Grape::API
  resource :organizations do
    # get all orgs
    get do
    end

    params do
      requires :organization_id, type: Integer
    end
    resource ':organization_id' do
      # get a single org
      get do
      end

      mount Resources::Projects, with: {nested: :organization}
      mount Resources::Tasks, with: {nested: :organization}
    end
  end
end

class Resources::Projects < Grape::API
  resource :projects do
    nested = configuration[:nested]

    if nested
      # get list of projects
      get do
      end
    end
    
    if nested.nil?
      # shallow routes
      params do
        requires :project_id, type: Integer
      end
      resource ':project_id' do
        # get a single project
        get do
        end

        mount Resources::Tasks, with: {nested: :project}
      end
    end
  end
end

class Resources::Tasks < Grape::API
  resource :tasks do
    nested = configuration[:nested]

    if nested
      # get list of tasks
      get do
      end
    end
    
    if nested.nil?
      # shallow routes
      params do
        requires :task_id, type: Integer
      end
      resource ':task_id' do
        # get a single task
        get do
        end
      end
    end
  end
end

Inside the task or project nested endpoints we can do this to determine what param we should fetch and how we should filter.

if options[:for].configuration[:nested] == :organization
  # filter tasks by org (params[:organization_id)
elsif options[:for].configuration[:nested] == :project
  # filter tasks by project (params[:project_id)
else
  raise ArgumentError, 'Endpoint mounted incorrectly'
end

@dblock
Copy link
Member

dblock commented Apr 19, 2019

Right on @urkle, thanks for resurfacing this. I believe #1803 resolved this. Closing. Please reopen a new issue if one of the scenarios above is still not handled.

@dblock dblock closed this as completed Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests