diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 04daa38a..dbbb35f8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -13,7 +13,7 @@ Metrics/AbcSize: # Offense count: 1 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 233 + Max: 250 # Offense count: 6 Metrics/CyclomaticComplexity: diff --git a/CHANGELOG.md b/CHANGELOG.md index 31793512..0633ae1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index c6b59a22..290c3e3f 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -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' diff --git a/lib/grape-swagger/doc_methods.rb b/lib/grape-swagger/doc_methods.rb index 8e8e801a..42688030 100644 --- a/lib/grape-swagger/doc_methods.rb +++ b/lib/grape-swagger/doc_methods.rb @@ -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 diff --git a/lib/grape-swagger/doc_methods/parse_params.rb b/lib/grape-swagger/doc_methods/parse_params.rb index f2a7ed38..f9d935f7 100644 --- a/lib/grape-swagger/doc_methods/parse_params.rb +++ b/lib/grape-swagger/doc_methods/parse_params.rb @@ -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 diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 4ea66b8c..6cd95fad 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -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 @@ -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 @@ -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]) @@ -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 @@ -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' diff --git a/spec/swagger_v2/api_swagger_v2_request_params_fix_spec.rb b/spec/swagger_v2/api_swagger_v2_request_params_fix_spec.rb new file mode 100644 index 00000000..8adf03a2 --- /dev/null +++ b/spec/swagger_v2/api_swagger_v2_request_params_fix_spec.rb @@ -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