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

Still having issues with helper inheritance #1820

Open
dmmcgee opened this issue Nov 12, 2018 · 11 comments
Open

Still having issues with helper inheritance #1820

dmmcgee opened this issue Nov 12, 2018 · 11 comments
Labels

Comments

@dmmcgee
Copy link

dmmcgee commented Nov 12, 2018

I thought this was supposed to be fixed with #1665, but I am still having issues with it. I have a few classes inheriting from Grape::API and I want to avoid having to include the helpers in every endpoint class:

require "#{Rails.root}/app/api/v2/helpers/authentication_helper.rb"

module API
  module V2
    
    class Base < Grape::API
      prefix 'v2'
      format :json
    end
      
    class AuthenticatedBase < Base
       helpers ApiHelpers::AuthenticationHelper
       before { authenticate! }
    end
  
    class AdminBase < Base
      # other things in here for higher security routes
    end
    
  end
end

However, when I go to make my end point class inherit, I can't use the AuthenticationHelper methods like current_user, for example.

require "#{Rails.root}/app/api/v2/base_classes.rb"

module API
	module V2
		
		class Sensors < AuthenticatedBase

		  desc 'End-points for Sensors'
      
		  namespace :sensors do

...

Is this intended? What am I missing?

@dblock dblock added the bug? label Nov 13, 2018
@dblock
Copy link
Member

dblock commented Nov 13, 2018

Try to turn this into a spec?

@bobbytables
Copy link
Contributor

Just want to confirm I am seeing this as well. If I have a bit to spare I'll try to write a spec. But basically any helpers block I declare in a base class does not appear to be in my child class.

@shanempope
Copy link

I went to upgrade today and before (in 1.0~) I had

API < Grape::API
  helpers SomeHelper
  mount AnEndpoint
  mount AnotherEndpoint
end

And the helper would be picked up by all of the mounts, but now that doesn't seem to be the case. The sub-mounts can't seem to find the helpers. If this is just something I'm missing from the upgrade on my end that could help you. But maybe it just broke because of the change in how Grape mounts works with the new Grape::API::instance

@myxoh
Copy link
Member

myxoh commented Dec 13, 2018

Have you upgraded to 1.2.2 ? Or are you in a prior version? (Can we open a new issue?)

@myxoh
Copy link
Member

myxoh commented Dec 13, 2018

On my test - this seems to fail to load Params :requires_toggle_prm not found! on all versions.
It doesn't seem to be specific to the latest one.
Not really sure what am I doing wrong?
#helpers_spec.rb

        context 'when including a helper on a top-level api' do
          let(:api) do
            Class.new(Grape::API) do
              params { use :requires_toggle_prm }
              get('ping') { 'ok' }
            end
          end

          let(:top_level) do
            mounted_api = api
            Class.new(Grape::API) do
              helpers BooleanParam
              mount mounted_api
            end
          end

          def app
            top_level
          end

          it 'provides access to the top level helpers in the API' do
            expect((get 'ping').status).to eq 400
          end
        end

@tscholz
Copy link

tscholz commented Feb 6, 2020

Any news on that? I'm also running into that issue. :(

@lenon
Copy link

lenon commented Nov 21, 2023

I had this issue today. I thought that something like this would work, but it didn't:

class Base < Grape::API
  helpers do
    def helper_from_base = 'helper from base'
  end
end

class A < Base
  helpers do
    def helper_from_a = 'helper from A'
  end
end

class B < A
  helpers do
    def helper_from_b = 'helper from B'
  end

  get '/test' do
    {
      helper_from_base: respond_to?(:helper_from_base),
      helper_from_a:    respond_to?(:helper_from_a),
      helper_from_b:    respond_to?(:helper_from_b),
    }
  end
end

class Root < Grape::API
  format :json

  mount A
  mount B
end
# response from /test:
# {
#     "helper_from_base": false,
#     "helper_from_a": false,
#     "helper_from_b": true
# }

It is possible to workaround this problem by using the inherited method like the following, but would be nice to be able to use the helpers syntax:

class Base < Grape::API
  def self.inherited(subclass)
    super

    subclass.helpers do
      def helper_from_base = 'helper from base'
    end
  end
end

class A < Base
  def self.inherited(subclass)
    super

    subclass.helpers do
      def helper_from_a = 'helper from A'
    end
  end
end

class B < A
  helpers do
    def helper_from_b = 'helper from B'
  end

  get '/test' do
    {
      helper_from_base: respond_to?(:helper_from_base),
      helper_from_a:    respond_to?(:helper_from_a),
      helper_from_b:    respond_to?(:helper_from_b),
    }
  end
end

class Root < Grape::API
  format :json

  mount A
  mount B
end
# response from /test:
# {
#     "helper_from_base": true,
#     "helper_from_a": true,
#     "helper_from_b": true
# }

@dblock
Copy link
Member

dblock commented Nov 22, 2023

Still looking for someone to pickup this issue and write some (failing) specs to start!

lenon added a commit to lenon/grape that referenced this issue Nov 22, 2023
@lenon
Copy link

lenon commented Nov 22, 2023

I started writing a failing spec here: lenon@ceb2767

The spec that I added fails as expected but it also causes another test to fail (./spec/grape/api/inherited_helpers_spec.rb:61):

$ bundle exec rspec -fp spec/grape/api/inherited_helpers_spec.rb:61
Run options: include {:locations=>{"./spec/grape/api/inherited_helpers_spec.rb"=>[61]}}

Randomized with seed 2153
F

Failures:

  1) Grape::API::Helpers non overriding subclass given expected params inherits helpers from a superclass
     Failure/Error: raise NoMethodError.new("undefined method `#{name}' for #{self.class} in `#{route.origin}' endpoint")

     NoMethodError:
       undefined method `current_user' for #<Class:0x00000001063d2780> in `/resource' endpoint

mount seems to change the subclass somehow.

@dblock
Copy link
Member

dblock commented Nov 22, 2023

Thanks for the repro! I bet mount copies/wires up a lot of things and helpers aren't one of those things. Hope you/someone can narrow it down and fix it!

@lenon
Copy link

lenon commented Nov 23, 2023

I spent some time debugging this, here's my notes of what I found so far:

mount calls Grape::API::Instance#inherit_settings, which then calls Grape::Util::InheritableSetting#inherit_from:

def inherit_from(parent)
return if parent.nil?
self.parent = parent
namespace_inheritable.inherited_values = parent.namespace_inheritable
namespace_stackable.inherited_values = parent.namespace_stackable
namespace_reverse_stackable.inherited_values = parent.namespace_reverse_stackable
self.route = parent.route.merge(route)
point_in_time_copies.map { |cloned_one| cloned_one.inherit_from parent }
end

self at this point is an instance of Grape::Util::InheritableSetting that contains the helpers and is (somehow) related to the SubClass:

[1] pry(#<Grape::Util::InheritableSetting>)> namespace_stackable[:helpers]
=> [#<Module:0x0000000104fbfd38>]
[2] pry(#<Grape::Util::InheritableSetting>)> namespace_stackable[:helpers][0].instance_methods
=> [:current_user]

parent is just a fresh instance of Grape::Util::InheritableSetting:

[5] pry(#<Grape::Util::InheritableSetting>)> parent.namespace_stackable[:helpers]
=> []

In this line namespace_stackable.inherited_values is replaced by parent.namespace_stackable, which is just an empty array:

namespace_stackable.inherited_values = parent.namespace_stackable

This causes a side effect in the SubClass and is maybe the reason that the other test failed:

[1] pry(#<Grape::Util::InheritableSetting>)> InheritedHelpersSpec::SubClass.namespace_stackable(:helpers)
=> []

I'll get back to this when I have some free time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants