grape icon indicating copy to clipboard operation
grape copied to clipboard

Still having issues with helper inheritance

Open dmmcgee opened this issue 6 years ago • 11 comments

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?

dmmcgee avatar Nov 12 '18 21:11 dmmcgee

Try to turn this into a spec?

dblock avatar Nov 13 '18 13:11 dblock

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.

bobbytables avatar Nov 19 '18 13:11 bobbytables

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

shanempope avatar Dec 12 '18 21:12 shanempope

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

myxoh avatar Dec 13 '18 00:12 myxoh

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

myxoh avatar Dec 13 '18 11:12 myxoh

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

tscholz avatar Feb 06 '20 16:02 tscholz

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
# }

lenon avatar Nov 21 '23 23:11 lenon

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

dblock avatar Nov 22 '23 15:11 dblock

I started writing a failing spec here: https://github.com/lenon/grape/commit/ceb27670dccc59a3b4943095b9ba6b08b9d515fc

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.

lenon avatar Nov 22 '23 18:11 lenon

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!

dblock avatar Nov 22 '23 19:11 dblock

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:

https://github.com/ruby-grape/grape/blob/3f01d03b7a28b088a52313d8d264f29aeacd41fa/lib/grape/util/inheritable_setting.rb#L48-L59

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:

https://github.com/ruby-grape/grape/blob/3f01d03b7a28b088a52313d8d264f29aeacd41fa/lib/grape/util/inheritable_setting.rb#L54

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.

lenon avatar Nov 23 '23 02:11 lenon