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

Fixes behaviour after grape upgrade to 1.4.0. #801

Merged
merged 1 commit into from
Jul 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ Layout/SpaceAroundMethodCallOperator:
Lint/DeprecatedOpenSSLConstant:
Enabled: true

Lint/DuplicateElsifCondition:
Enabled: true

Lint/MixedRegexpCaptureTypes:
Enabled: true

Expand Down Expand Up @@ -62,12 +65,29 @@ Naming:

# Style stuff
#
Style/AccessorGrouping:
Enabled: true

Style/ArrayCoercion:
Enabled: true

Style/BisectedAttrAccessor:
Enabled: true

Style/CaseLikeIf:
Enabled: true

Style/ExponentialNotation:
Enabled: true

Style/HashAsLastArrayItem:
Enabled: true
Style/HashEachMethods:
Enabled: true

Style/HashLikeCase:
Enabled: true

Style/HashTransformKeys:
Enabled: true

Expand All @@ -77,9 +97,15 @@ Style/HashTransformValues:
Style/RegexpLiteral:
Enabled: false

Style/RedundantAssignment:
Enabled: true

Style/RedundantFetchBlock:
Enabled: true

Style/RedundantFileExtensionInRequire:
Enabled: true

Style/RedundantRegexpCharacterClass:
Enabled: true

Expand All @@ -88,3 +114,5 @@ Style/RedundantRegexpEscape:

Style/SlicingWithRange:
Enabled: false


9 changes: 4 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ rvm:
- 2.6.6
- 2.7.1
env:
- GRAPE_VERSION=1.3.0 MODEL_PARSER=grape-swagger-entity
- GRAPE_VERSION=1.3.0 MODEL_PARSER=grape-swagger-representable
- GRAPE_VERSION=1.3.0
- GRAPE_VERSION=1.3.3
- GRAPE_VERSION=1.4.0 MODEL_PARSER=grape-swagger-entity
- GRAPE_VERSION=1.4.0 MODEL_PARSER=grape-swagger-representable
- GRAPE_VERSION=1.4.0
- GRAPE_VERSION=HEAD

jobs:
Expand All @@ -33,5 +34,3 @@ jobs:
- rvm: 2.4.10
- rvm: ruby-head
- rvm: jruby-head

- env: GRAPE_VERSION=HEAD
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Fixes

* Your contribution here.
* [#801](/~https://github.com/ruby-grape/grape-swagger/pull/801): Fixes behaviour after grape upgrade to 1.4.0 - [@LeFnord](/~https://github.com/LeFnord).


### 1.2.0 (July 1, 2020)
Expand Down
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ruby RUBY_VERSION

gemspec

gem 'grape', case version = ENV['GRAPE_VERSION'] || '>= 1.3.0'
gem 'grape', case version = ENV['GRAPE_VERSION'] || '>= 1.4.0'
when 'HEAD'
{ git: '/~https://github.com/ruby-grape/grape' }
else
Expand All @@ -27,7 +27,7 @@ group :development, :test do
gem 'rake'
gem 'rdoc'
gem 'rspec', '~> 3.9'
gem 'rubocop', '~> 0.85', require: false
gem 'rubocop', '~> 0.88', require: false
end

group :test do
Expand Down
3 changes: 1 addition & 2 deletions lib/grape-swagger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ def route_path_start_with?(route, name)
end

module SwaggerDocumentationAdder
attr_accessor :combined_namespaces, :combined_namespace_identifiers
attr_accessor :combined_routes, :combined_namespace_routes
attr_accessor :combined_namespaces, :combined_namespace_identifiers, :combined_routes, :combined_namespace_routes

include SwaggerRouting

Expand Down
26 changes: 18 additions & 8 deletions lib/grape-swagger/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,18 @@ def consumes_object(route, format)
end

def params_object(route, options, path)
parameters = partition_params(route, options).map do |param, value|
parameters = build_request_params(route, options).each_with_object([]) do |(param, value), memo|
next if hidden_parameter?(value)

value = { required: false }.merge(value) if value.is_a?(Hash)
_, value = default_type([[param, value]]).first if value == ''

if value.dig(:documentation, :type)
expose_params(value[:documentation][:type])
elsif value[:type]
expose_params(value[:type])
end
GrapeSwagger::DocMethods::ParseParams.call(param, value, path, route, @definitions)
memo << GrapeSwagger::DocMethods::ParseParams.call(param, value, path, route, @definitions)
end

if GrapeSwagger::DocMethods::MoveParams.can_be_moved?(route.request_method, parameters)
Expand Down Expand Up @@ -253,7 +256,7 @@ def success_codes_from_route(route)

def tag_object(route, path)
version = GrapeSwagger::DocMethods::Version.get(route)
version = [version] unless version.is_a?(Array)
version = Array(version)
prefix = route.prefix.to_s.split('/').reject(&:empty?)
Array(
path.split('{')[0].split('/').reject(&:empty?).delete_if do |i|
Expand Down Expand Up @@ -296,16 +299,13 @@ def build_file_response(memo)
memo['schema'] = { type: 'file' }
end

def partition_params(route, settings)
declared_params = route.settings[:declared_params] if route.settings[:declared_params].present?
def build_request_params(route, settings)
required = merge_params(route)
required = GrapeSwagger::DocMethods::Headers.parse(route) + required unless route.headers.nil?

default_type(required)

request_params = unless declared_params.nil? && route.headers.nil?
GrapeSwagger::Endpoint::ParamsParser.parse_request_params(required, settings, self)
end || {}
request_params = GrapeSwagger::Endpoint::ParamsParser.parse_request_params(required, settings, self)

request_params.empty? ? required : request_params
end
Expand Down Expand Up @@ -363,6 +363,16 @@ def hidden?(route, options)
options[:token_owner] ? route_hidden.call(send(options[:token_owner].to_sym)) : route_hidden.call
end

def hidden_parameter?(value)
return false if value.dig(:required)

if value.dig(:documentation, :hidden).is_a?(Proc)
value.dig(:documentation, :hidden).call
else
value.dig(:documentation, :hidden)
end
end

def success_code_from_entity(route, entity)
default_code = GrapeSwagger::DocMethods::StatusCodes.get[route.request_method.downcase.to_sym]
if entity.is_a?(Hash)
Expand Down
3 changes: 1 addition & 2 deletions spec/support/empty_model_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ class EmptyClass

module GrapeSwagger
class EmptyModelParser
attr_reader :model
attr_reader :endpoint
attr_reader :model, :endpoint

def initialize(model, endpoint)
@model = model
Expand Down
3 changes: 1 addition & 2 deletions spec/support/mock_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

module GrapeSwagger
class MockParser
attr_reader :model
attr_reader :endpoint
attr_reader :model, :endpoint

def initialize(model, endpoint)
@model = model
Expand Down
2 changes: 1 addition & 1 deletion spec/swagger_v2/api_swagger_v2_hide_param_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def resource_owner
requires :name, type: String, documentation: { desc: 'name' }
optional :favourite_color, type: String, documentation: { desc: 'I should not be anywhere', hidden: true }
optional :proc_param, type: String, documentation: { desc: 'I should not be anywhere', hidden: proc { true } }
optional :proc_with_token, type: String, documentation: { desc: 'I may be somewhere', hidden: proc { |token_owner = nil| token_owner.nil? } }
optional :proc_with_token, type: String, documentation: { desc: 'I may be somewhere', hidden: proc { false } }
end

post do
Expand Down
4 changes: 2 additions & 2 deletions spec/swagger_v2/security_requirement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
describe 'security requirement on endpoint method' do
def app
Class.new(Grape::API) do
desc 'Endpoint with security requirement', security: [oauth_pets: ['read:pets', 'write:pets']]
desc 'Endpoint with security requirement', security: [{ oauth_pets: ['read:pets', 'write:pets'] }]
get '/with_security' do
{ foo: 'bar' }
end
Expand Down Expand Up @@ -37,7 +37,7 @@ def app
end

it 'defines the security requirement on the endpoint method' do
expect(subject['paths']['/with_security']['get']['security']).to eql ['oauth_pets' => ['read:pets', 'write:pets']]
expect(subject['paths']['/with_security']['get']['security']).to eql [{ 'oauth_pets' => ['read:pets', 'write:pets'] }]
end

it 'defines an empty security requirement on the endpoint method' do
Expand Down