From 8b9825d0e9145fc3e88504ccbb20372505726d2e Mon Sep 17 00:00:00 2001 From: peter scholz Date: Wed, 16 Mar 2016 12:30:52 +0100 Subject: [PATCH] Update CHANGELOG.md --- .rubocop_todo.yml | 2 +- CHANGELOG.md | 4 + lib/grape-swagger.rb | 2 + lib/grape-swagger/doc_methods/parse_params.rb | 102 +++++++++++++++ lib/grape-swagger/endpoint.rb | 117 +++--------------- spec/lib/path_string_spec.rb | 1 - spec/support/api_swagger_v2_result.rb | 2 +- spec/support/the_api_entities.rb | 5 + .../api_swagger_v2_response_spec.rb | 64 ++++++++-- spec/swagger_v2/api_swagger_v2_spec.rb | 12 +- spec/swagger_v2/default_api_spec.rb | 2 +- 11 files changed, 193 insertions(+), 120 deletions(-) create mode 100644 lib/grape-swagger/doc_methods/parse_params.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8b41e880..d2c14bdb 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -13,7 +13,7 @@ Metrics/AbcSize: # Offense count: 1 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 300 + Max: 230 # Offense count: 6 Metrics/CyclomaticComplexity: diff --git a/CHANGELOG.md b/CHANGELOG.md index bc0abb6b..9db049b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ n.n.n / 2016-03-16 ================== +[#356](https://github.com/ruby-grape/grape-swagger/pull/356) + +- adds `consumes` setting +- refactoring [#354](https://github.com/ruby-grape/grape-swagger/pull/354) some improvements diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index 1f24bbc5..1c663c59 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -11,6 +11,8 @@ 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' require 'grape-swagger/markdown/kramdown_adapter' diff --git a/lib/grape-swagger/doc_methods/parse_params.rb b/lib/grape-swagger/doc_methods/parse_params.rb new file mode 100644 index 00000000..00dfd7c4 --- /dev/null +++ b/lib/grape-swagger/doc_methods/parse_params.rb @@ -0,0 +1,102 @@ +module GrapeSwagger + module DocMethods + class ParseParams + class << self + def call(param, value, route) + @array_items = {} + path = route.route_path + method = route.route_method + + additional_documentation = value.is_a?(Hash) ? value[:documentation] : nil + data_type = GrapeSwagger::DocMethods::DataType.call(value) + + if additional_documentation && value.is_a?(Hash) + value = additional_documentation.merge(value) + end + + description = value.is_a?(Hash) ? value[:desc] || value[:description] : nil + required = value.is_a?(Hash) ? value[:required] : false + default_value = value.is_a?(Hash) ? value[:default] : nil + example = value.is_a?(Hash) ? value[:example] : nil + is_array = value.is_a?(Hash) ? (value[:is_array] || false) : false + values = value.is_a?(Hash) ? value[:values] : nil + name = (value.is_a?(Hash) && value[:full_name]) || param + enum_or_range_values = parse_enum_or_range_values(values) + + value_type = { value: value, data_type: data_type, path: path } + + parsed_params = { + in: param_type(value_type, param, method, is_array), + name: name, + description: description, + type: data_type, + required: required, + allowMultiple: is_array + } + + if GrapeSwagger::DocMethods::DataType::PRIMITIVE_MAPPINGS.key?(data_type) + parsed_params[:type], parsed_params[:format] = GrapeSwagger::DocMethods::DataType::PRIMITIVE_MAPPINGS[data_type] + end + + parsed_params[:items] = @array_items if @array_items.present? + + parsed_params[:defaultValue] = example if example + parsed_params[:defaultValue] = default_value if default_value && example.blank? + + parsed_params.merge!(enum_or_range_values) if enum_or_range_values + parsed_params + end + + def primitive?(type) + %w(object integer long float double string byte boolean date datetime).include? type.to_s.downcase + end + + private + + def param_type(value_type, param, method, is_array) + # TODO: use `value_type.dig():value, :documentation, :param_type)` instead req ruby2.3 + # + if value_type[:value].is_a?(Hash) && + value_type[:value].key?(:documentation) && + value_type[:value][:documentation].key?(:param_type) + + if is_array + @array_items = { 'type' => value_type[:data_type] } + + 'array' + end + else + case + when value_type[:path].include?("{#{param}}") + 'path' + when %w(POST PUT PATCH).include?(method) + primitive?(value_type[:data_type]) ? 'formData' : 'body' + else + 'query' + end + end + end + + def parse_enum_or_range_values(values) + case values + when Range + parse_range_values(values) if values.first.is_a?(Integer) + when Proc + values_result = values.call + if values_result.is_a?(Range) && values_result.first.is_a?(Integer) + parse_range_values(values_result) + else + { enum: values_result } + end + else + { enum: values } if values + end + end + + def parse_range_values(values) + { minimum: values.first, maximum: values.last } + end + end + end + end +end diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 86b3415d..b573c767 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -65,9 +65,9 @@ def license_object(infos) # contact def contact_object(infos) { - contact_name: infos.delete(:contact_name), - contact_email: infos.delete(:contact_email), - contact_url: infos.delete(:contact_url) + name: infos.delete(:contact_name), + email: infos.delete(:contact_email), + url: infos.delete(:contact_url) }.delete_if { |_, value| value.blank? } end @@ -194,7 +194,7 @@ def default_staus_codes def params_object(route) partition_params(route).map do |param, value| value = { required: false }.merge(value) if value.is_a?(Hash) - parse_params(param, value, route.route_path, route.route_method) + GrapeSwagger::DocMethods::ParseParams.call(param, value, route) end end @@ -236,8 +236,12 @@ def parse_response_params(params) params.each_with_object({}) do |x, memo| x[0] = x.last[:as] if x.last[:as] - if x.last[:using].present? || could_it_be_a_model?(x.last) - name = expose_params_from_model(x.last[:using] || x.last[:type]) + + model = x.last[:using] if x.last[:using].present? + model ||= x.last[:documentation][:type] if x.last[:documentation] && could_it_be_a_model?(x.last[:documentation]) + + if model + name = expose_params_from_model(model) memo[x.first] = if x.last[:documentation] && x.last[:documentation][:is_array] { 'type' => 'array', 'items' => { '$ref' => "#/definitions/#{name}" } } else @@ -251,7 +255,7 @@ def parse_response_params(params) end def expose_params_from_model(model) - model_name = model.name.demodulize.camelize + model_name = model.respond_to?(:name) ? model.name.demodulize.camelize : model.split('::').last # DONE: has to be adept, to be ready for grape-entity >0.5.0 # TODO: this should only be a temporary hack ;) @@ -270,10 +274,14 @@ def expose_params_from_model(model) end def could_it_be_a_model?(value) - value[:type] && + ( + value[:type].to_s.include?('Entity') || value[:type].to_s.include?('Entities') + ) || ( + value[:type] && value[:type].is_a?(Class) && - !primitive?(value[:type].name.downcase) && + !GrapeSwagger::DocMethods::ParseParams.primitive?(value[:type].name.downcase) && !value[:type] == Array + ) end def hidden?(route) @@ -284,97 +292,6 @@ def hidden?(route) false end - # original methods - # - def parse_params(param, value, path, method) - @array_items = {} - - additional_documentation = value.is_a?(Hash) ? value[:documentation] : nil - data_type = GrapeSwagger::DocMethods::DataType.call(value) - - if additional_documentation && value.is_a?(Hash) - value = additional_documentation.merge(value) - end - - description = value.is_a?(Hash) ? value[:desc] || value[:description] : nil - required = value.is_a?(Hash) ? value[:required] : false - default_value = value.is_a?(Hash) ? value[:default] : nil - example = value.is_a?(Hash) ? value[:example] : nil - is_array = value.is_a?(Hash) ? (value[:is_array] || false) : false - values = value.is_a?(Hash) ? value[:values] : nil - name = (value.is_a?(Hash) && value[:full_name]) || param - enum_or_range_values = parse_enum_or_range_values(values) - - value_type = { value: value, data_type: data_type, path: path } - - parsed_params = { - in: param_type(value_type, param, method, is_array), - name: name, - description: description, - type: data_type, - required: required, - allowMultiple: is_array - } - - if GrapeSwagger::DocMethods::DataType::PRIMITIVE_MAPPINGS.key?(data_type) - parsed_params[:type], parsed_params[:format] = GrapeSwagger::DocMethods::DataType::PRIMITIVE_MAPPINGS[data_type] - end - - parsed_params[:items] = @array_items if @array_items.present? - - parsed_params[:defaultValue] = example if example - parsed_params[:defaultValue] = default_value if default_value && example.blank? - - parsed_params.merge!(enum_or_range_values) if enum_or_range_values - parsed_params - end - - def param_type(value_type, param, method, is_array) - if value_type[:value].is_a?(Hash) && - value_type[:value].key?(:documentation) && - value_type[:value][:documentation].key?(:param_type) - - if is_array - @array_items = { 'type' => value_type[:data_type] } - - 'array' - end - else - case - when value_type[:path].include?("{#{param}}") - 'path' - when %w(POST PUT PATCH).include?(method) - primitive?(value_type[:data_type]) ? 'formData' : 'body' - else - 'query' - end - end - end - - def parse_enum_or_range_values(values) - case values - when Range - parse_range_values(values) if values.first.is_a?(Integer) - when Proc - values_result = values.call - if values_result.is_a?(Range) && values_result.first.is_a?(Integer) - parse_range_values(values_result) - else - { enum: values_result } - end - else - { enum: values } if values - end - end - - def parse_range_values(values) - { minimum: values.first, maximum: values.last } - end - - def primitive?(type) - %w(object integer long float double string byte boolean date dateTime).include? type - end - def tag_object(route, version) Array(route.route_path.split('{')[0].split('/').reject(&:empty?).delete_if { |i| ((i == route.route_prefix.to_s) || (i == version)) }.first) end diff --git a/spec/lib/path_string_spec.rb b/spec/lib/path_string_spec.rb index cdfb4adf..d068dba0 100644 --- a/spec/lib/path_string_spec.rb +++ b/spec/lib/path_string_spec.rb @@ -32,7 +32,6 @@ expect(subject.build('/{version}/thing/:id', options)).to eql ['Thing', '/v1/thing/{id}'] expect(subject.build('/{version}/thing/foo/:id', options)).to eql ['Foo', '/v1/thing/foo/{id}'] end - end end end diff --git a/spec/support/api_swagger_v2_result.rb b/spec/support/api_swagger_v2_result.rb index 0f4888a7..8acedce6 100644 --- a/spec/support/api_swagger_v2_result.rb +++ b/spec/support/api_swagger_v2_result.rb @@ -65,7 +65,7 @@ class ApiError < Grape::Entity "title"=>"The API title to be displayed on the API homepage.", "description"=>"A description of the API.", "termsOfServiceUrl"=>"www.The-URL-of-the-terms-and-service.com", - "contact"=>{"contact_name"=>"Contact name", "contact_email"=>"Contact@email.com", "contact_url"=>"Contact URL"}, + "contact"=>{"name"=>"Contact name", "email"=>"Contact@email.com", "url"=>"Contact URL"}, "license"=>{"name"=>"The name of the license.", "url"=>"www.The-URL-of-the-license.org"}, "version"=>"v1" }, diff --git a/spec/support/the_api_entities.rb b/spec/support/the_api_entities.rb index a5246895..4d369b59 100644 --- a/spec/support/the_api_entities.rb +++ b/spec/support/the_api_entities.rb @@ -27,6 +27,11 @@ class UseResponse < Grape::Entity expose :description, documentation: { type: String } expose :items, as: '$responses', using: Entities::ResponseItem, documentation: { is_array: true } end + + class UseTemResponseAsType < Grape::Entity + expose :description, documentation: { type: String } + expose :responses, documentation: { type: Entities::ResponseItem, is_array: false } + end end end end diff --git a/spec/swagger_v2/api_swagger_v2_response_spec.rb b/spec/swagger_v2/api_swagger_v2_response_spec.rb index e79064c4..d765ffe6 100644 --- a/spec/swagger_v2/api_swagger_v2_response_spec.rb +++ b/spec/swagger_v2/api_swagger_v2_response_spec.rb @@ -22,6 +22,14 @@ class ResponseApi < Grape::API { "declared_params" => declared(params) } end + desc 'This returns something', + entity: Entities::UseTemResponseAsType, + failure: [{code: 400, message: 'NotFound', model: Entities::ApiError}] + get '/nested_type' do + { "declared_params" => declared(params) } + end + + add_swagger_documentation end end @@ -31,6 +39,42 @@ def app TheApi::ResponseApi end + describe "uses nested type as response object" do + subject do + get '/swagger_doc/nested_type' + JSON.parse(last_response.body) + end + specify do + expect(subject).to eql({ + "info"=>{"title"=>"API title", "version"=>"v1"}, + "swagger"=>"2.0", + "produces"=>["application/json"], + "host"=>"example.org", + "tags"=>[ + {"name"=>"params_response", "description"=>"Operations about params_responses"}, + {"name"=>"entity_response", "description"=>"Operations about entity_responses"}, + {"name"=>"nested_type", "description"=>"Operations about nested_types"} + ], + "schemes"=>["https", "http"], + "paths"=>{ + "/nested_type"=>{ + "get"=>{ + "produces"=>["application/json"], + "responses"=>{ + "200"=>{"description"=>"This returns something", "schema"=>{"$ref"=>"#/definitions/UseTemResponseAsType"}}, + "400"=>{"description"=>"NotFound", "schema"=>{"$ref"=>"#/definitions/ApiError"}} + }, + "tags"=>["nested_type"], + "operationId"=>"getNestedType" + }}}, + "definitions"=>{ + "ResponseItem"=>{"type"=>"object", "properties"=>{"id"=>{"type"=>"integer"}, "name"=>{"type"=>"string"}}}, + "UseTemResponseAsType"=>{"type"=>"object", "properties"=>{"description"=>{"type"=>"string"}, "responses"=>{"$ref"=>"#/definitions/ResponseItem"}}}, + "ApiError"=>{"type"=>"object", "properties"=>{"code"=>{"type"=>"integer"}, "message"=>{"type"=>"string"}}} + }}) + end + end + describe "uses entity as response object" do subject do get '/swagger_doc/entity_response' @@ -43,7 +87,11 @@ def app "swagger"=>"2.0", "produces"=>["application/json"], "host"=>"example.org", - "tags" => [{"name"=>"params_response", "description"=>"Operations about params_responses"}, {"name"=>"entity_response", "description"=>"Operations about entity_responses"}], + "tags" => [ + {"name"=>"params_response", "description"=>"Operations about params_responses"}, + {"name"=>"entity_response", "description"=>"Operations about entity_responses"}, + {"name"=>"nested_type", "description"=>"Operations about nested_types"} + ], "schemes"=>["https", "http"], "paths"=>{ "/entity_response"=>{ @@ -74,21 +122,17 @@ def app JSON.parse(last_response.body) end - - # usage of grape-entity 0.4.8 preduces a wrong definition for ParamsResponse, this one: - # "definitions" => { - # "ParamsResponse"=>{"properties"=>{"description"=>{"type"=>"string"}}}, - # "ApiError"=>{"type"=>"object", "properties"=>{"code"=>{"type"=>"integer"}, "message"=>{"type"=>"string"}}} - # } - # (`$response` property is missing) - specify do expect(subject).to eql({ "info"=>{"title"=>"API title", "version"=>"v1"}, "swagger"=>"2.0", "produces"=>["application/json"], "host"=>"example.org", - "tags" => [{"name"=>"params_response", "description"=>"Operations about params_responses"}, {"name"=>"entity_response", "description"=>"Operations about entity_responses"}], + "tags" => [ + {"name"=>"params_response", "description"=>"Operations about params_responses"}, + {"name"=>"entity_response", "description"=>"Operations about entity_responses"}, + {"name"=>"nested_type", "description"=>"Operations about nested_types"} + ], "schemes"=>["https", "http"], "paths"=>{ "/params_response"=>{ diff --git a/spec/swagger_v2/api_swagger_v2_spec.rb b/spec/swagger_v2/api_swagger_v2_spec.rb index 7d15cb74..4c0a5fd1 100644 --- a/spec/swagger_v2/api_swagger_v2_spec.rb +++ b/spec/swagger_v2/api_swagger_v2_spec.rb @@ -155,12 +155,12 @@ def app describe 'contact object' do let(:contact) { json['info']['contact'] } - it { expect(contact.keys).to include 'contact_name' } - it { expect(contact['contact_name']).to be_a String } - it { expect(contact.keys).to include 'contact_email' } - it { expect(contact['contact_email']).to be_a String } - it { expect(contact.keys).to include 'contact_url' } - it { expect(contact['contact_url']).to be_a String } + it { expect(contact.keys).to include 'name' } + it { expect(contact['name']).to be_a String } + it { expect(contact.keys).to include 'email' } + it { expect(contact['email']).to be_a String } + it { expect(contact.keys).to include 'url' } + it { expect(contact['url']).to be_a String } end end diff --git a/spec/swagger_v2/default_api_spec.rb b/spec/swagger_v2/default_api_spec.rb index a1faceb4..8194ee67 100644 --- a/spec/swagger_v2/default_api_spec.rb +++ b/spec/swagger_v2/default_api_spec.rb @@ -124,7 +124,7 @@ def app end it 'documents the contact email' do - expect(subject['contact']['contact_email']).to eql('support@test.com') + expect(subject['contact']['email']).to eql('support@test.com') end end end