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

Only call value method if it has zero arity #496

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sikachu
Copy link

@sikachu sikachu commented Jun 25, 2021

This PR fixes a compatibility problem with Rails when the user specifies parameter with the same name as ActionDispatch::Integration::RequestHelpers methods.

This came up in our application because we are accepting a parameter name options, and when upgrading to Rails 6.1 we starting to get this error:

ArgumentError:
  wrong number of arguments (given 0, expected 1)
# [GEM_HOME]/actionpack-6.1.3.2/lib/action_dispatch/testing/integration.rb:51:in `options'
# [GEM_HOME]/actionpack-6.1.3.2/lib/action_dispatch/testing/integration.rb:429:in `public_send'
# [GEM_HOME]/actionpack-6.1.3.2/lib/action_dispatch/testing/integration.rb:429:in `method_missing'
# [BUNDLER_GEM_HOME]/rspec_api_documentation-d3892cc73884/lib/rspec_api_documentation/dsl/endpoint/set_param.rb:20:in `value'
# [BUNDLER_GEM_HOME]/rspec_api_documentation-d3892cc73884/lib/rspec_api_documentation/dsl/endpoint/params.rb:29:in `block in extended'
# [BUNDLER_GEM_HOME]/rspec_api_documentation-d3892cc73884/lib/rspec_api_documentation/dsl/endpoint/params.rb:27:in `map'
# [BUNDLER_GEM_HOME]/rspec_api_documentation-d3892cc73884/lib/rspec_api_documentation/dsl/endpoint/params.rb:27:in `extended'
# [BUNDLER_GEM_HOME]/rspec_api_documentation-d3892cc73884/lib/rspec_api_documentation/dsl/endpoint.rb:107:in `extended_parameters'
# [BUNDLER_GEM_HOME]/rspec_api_documentation-d3892cc73884/lib/rspec_api_documentation/dsl/endpoint.rb:41:in `do_request'

After some investigation, it seems like Rails 6.1 has added options method to ActionDispatch::Integration::RequestHelpers which get included to Request specs:

https://github.com/rails/rails/blob/v6.1.1/actionpack/lib/action_dispatch/testing/integration.rb#L49-L53

      # Performs an OPTIONS request with the given parameters. See ActionDispatch::Integration::Session#process
      # for more details.
      def options(path, **args)
        process(:options, path, **args)
      end

Therefore when rspec_api_documentation tries to call options method to get the value, it accidentally calling this helper method from Rails instead and resulting in ArgumentError.

Since it's a bit tricky to detect where the method was defined, I propose a solution in this PR in which we just check if the value method has an arity of zero (i.e. doesn't take any argument) before calling it. This way, rspec_api_documentation will still automatically call a value method if it's defined or overridden by the user.

@artofhuman
Copy link

@davidstosik @jakehow It looks good. Can it will be merged and released?

@ToriK17
Copy link

ToriK17 commented Mar 2, 2022

This fix was great! We needed it asap so we had to fork this and add it manually for the time being to regenerate docs quickly. If anyone else is using this in your application remember to add
config.response_body_formatter = Proc.new { |response_content_type, response_body| response_body }
to your application's configuration. Thanks @sikachu!

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

Successfully merging this pull request may close these issues.

None yet

3 participants