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

set default type: Integer and required: true for path params #367

Merged
merged 3 commits into from
Apr 2, 2016
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
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Metrics/AbcSize:
# Offense count: 1
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 233
Max: 250

# Offense count: 6
Metrics/CyclomaticComplexity:
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@

n.n.n / 2016-03-16
==================
[#367](https://github.com/ruby-grape/grape-swagger/pull/367)

- set default `type: Integer` and `required: true` for path params,
if they wasn't specified inside the `params` bloack as required

[#365](https://github.com/ruby-grape/grape-swagger/pull/365)

- fixes passing markdown with redcarpet even with nil description and detail
Expand Down
10 changes: 0 additions & 10 deletions lib/grape-swagger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,6 @@
require 'grape-swagger/endpoint'
require 'grape-swagger/errors'

require 'grape-swagger/doc_methods/produces_consumes'
require 'grape-swagger/doc_methods/data_type'
require 'grape-swagger/doc_methods/extensions'
require 'grape-swagger/doc_methods/operation_id'
require 'grape-swagger/doc_methods/optional_object'
require 'grape-swagger/doc_methods/path_string'
require 'grape-swagger/doc_methods/tag_name_description'
require 'grape-swagger/doc_methods/parse_params'
# require 'grape-swagger/doc_methods/move_params'

require 'grape-swagger/doc_methods'

require 'grape-swagger/markdown/kramdown_adapter'
Expand Down
10 changes: 10 additions & 0 deletions lib/grape-swagger/doc_methods.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
require 'grape-swagger/doc_methods/produces_consumes'
require 'grape-swagger/doc_methods/data_type'
require 'grape-swagger/doc_methods/extensions'
require 'grape-swagger/doc_methods/operation_id'
require 'grape-swagger/doc_methods/optional_object'
require 'grape-swagger/doc_methods/path_string'
require 'grape-swagger/doc_methods/tag_name_description'
require 'grape-swagger/doc_methods/parse_params'
# require 'grape-swagger/doc_methods/move_params'

module GrapeSwagger
module DocMethods
def name
Expand Down
4 changes: 2 additions & 2 deletions lib/grape-swagger/doc_methods/parse_params.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ def document_array_param(value_type)
def param_type(value_type)
param_type = value_type[:param_type] || value_type[:in]
case
when param_type
param_type
when value_type[:path].include?("{#{value_type[:param_name]}}")
'path'
when param_type
param_type
when %w(POST PUT PATCH).include?(value_type[:method])
GrapeSwagger::DocMethods::DataType.request_primitive?(value_type[:data_type]) ? 'formData' : 'body'
else
Expand Down
22 changes: 20 additions & 2 deletions lib/grape-swagger/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ def path_and_definition_objects(namespace_routes, options)

add_definitions_from options[:models]

# GrapeSwagger::DocMethods::MoveParams.to_definition(@paths, @definitions)
[@paths, @definitions]
end

Expand Down Expand Up @@ -144,6 +145,7 @@ def consumes_object(route, format)
def params_object(route)
partition_params(route).map do |param, value|
value = { required: false }.merge(value) if value.is_a?(Hash)
_, value = default_type([[param, value]]).first if value == ''
GrapeSwagger::DocMethods::ParseParams.call(param, value, route)
end
end
Expand All @@ -162,7 +164,10 @@ def response_object(route)
response_model = @item
response_model = expose_params_from_model(value[:model]) if value[:model]

memo[204] = memo.delete(200) if memo.key?(200) && route.route_method == 'DELETE' && value[:model].nil?
if memo.key?(200) && route.route_method == 'DELETE' && value[:model].nil?
memo[204] = memo.delete(200)
value[:code] = 204
end

next unless !response_model.start_with?('Swagger_doc') &&
((@definitions[response_model] && value[:code].to_s.start_with?('2')) || value[:model])
Expand All @@ -186,6 +191,9 @@ def partition_params(route)
declared_params = route.route_settings[:declared_params] if route.route_settings[:declared_params].present?
required, exposed = route.route_params.partition { |x| x.first.is_a? String }

default_type(required)
default_type(exposed)

unless declared_params.nil?
request_params = parse_request_params(required)
end
Expand All @@ -198,12 +206,22 @@ def partition_params(route)
end

key = model_name(@entity || @item)
@definitions[key] = { type: 'object', properties: properties } unless properties.empty? || @definitions.key?(key) || (route.route_method == 'DELETE' && !@entity)

unless properties.empty? || (route.route_method == 'DELETE' && !@entity)
@definitions[key] = { type: 'object', properties: properties } unless @definitions.key?(key)
@definitions[key][:properties].merge!(properties) if @definitions.key?(key)
end

return route.route_params if route.route_params && !route.route_settings[:declared_params].present?
request_params || {}
end

def default_type(params)
params.each do |param|
param[-1] = param.last == '' ? { required: true, type: 'Integer' } : param.last
end
end

def parse_request_params(required)
required.each_with_object({}) do |param, memo|
@array_key = param.first.to_s.gsub('[', '[][') if param.last[:type] == 'Array'
Expand Down
64 changes: 64 additions & 0 deletions spec/swagger_v2/api_swagger_v2_request_params_fix_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
require 'spec_helper'

describe 'additional parameter settings' do
before :all do
module TheApi
class RequestParamFix < Grape::API
resource :bookings do
desc 'Update booking'
params do
optional :name, type: String
end
put ':id' do
{ "declared_params" => declared(params) }
end

desc 'Get booking details'
get ':id' do
{ "declared_params" => declared(params) }
end

desc 'Get booking details by access_number'
get '/conf/:access_number' do
{ "declared_params" => declared(params) }
end

desc 'Remove booking'
delete ':id' do
{ "declared_params" => declared(params) }
end
end

add_swagger_documentation
end
end
end

def app
TheApi::RequestParamFix
end

subject do
get '/swagger_doc'
JSON.parse(last_response.body)
end

specify do
expect(subject['paths']['/bookings/{id}']['put']['parameters']).to eql([
{"in"=>"path", "name"=>"id", "description"=>nil, "type"=>"integer", "format"=>"int32", "required"=>true},
{"in"=>"formData", "name"=>"name", "description"=>nil, "type"=>"string", "required"=>false}
])
end

specify do
expect(subject['paths']['/bookings/{id}']['get']['parameters']).to eql([
{"in"=>"path", "name"=>"id", "description"=>nil, "type"=>"integer", "format"=>"int32", "required"=>true}
])
end

specify do
expect(subject['paths']['/bookings/{id}']['delete']['parameters']).to eql([
{"in"=>"path", "name"=>"id", "description"=>nil, "type"=>"integer", "format"=>"int32", "required"=>true}
])
end
end