diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 220cb8ec..6e3322aa 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2016-05-10 15:59:21 -0400 using RuboCop version 0.40.0. +# on 2016-05-11 23:53:37 +0300 using RuboCop version 0.40.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -21,26 +21,26 @@ Lint/UselessAssignment: Exclude: - 'spec/lib/move_params_spec.rb' -# Offense count: 26 +# Offense count: 27 Metrics/AbcSize: Max: 56 # Offense count: 3 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 195 + Max: 201 # Offense count: 10 Metrics/CyclomaticComplexity: Max: 16 -# Offense count: 711 +# Offense count: 719 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes. # URISchemes: http, https Metrics/LineLength: Max: 454 -# Offense count: 30 +# Offense count: 31 # Configuration parameters: CountComments. Metrics/MethodLength: Max: 101 diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e19bfc7..e0e21d12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ #### Fixes * [#416](https://github.com/ruby-grape/grape-swagger/pull/416): Support recursive models - [@lest](https://github.com/lest). -* [#418](https://github.com/ruby-grape/grape-swagger/pull/418): Replaced github ref to rubygems for external gems - [@Bugagazavr](https://github.com/Bugagazavr). +* [#419](https://github.com/ruby-grape/grape-swagger/pull/419): Replaced github ref to rubygems for external gems - [@Bugagazavr](https://github.com/Bugagazavr). +* [#420](https://github.com/ruby-grape/grape-swagger/pull/420): Raise SwaggerSpec exception if swagger spec doesn't satified, when no parser for model registred or response model is empty - [@Bugagazavr](https://github.com/Bugagazavr). ### 0.20.3 (May 9, 2016) diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 01012787..77799d8f 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -232,17 +232,23 @@ def expose_params_from_model(model) return model_name if @definitions.key?(model_name) @definitions[model_name] = nil - properties = {} + properties = nil + parser = nil + GrapeSwagger.model_parsers.each do |klass, ancestor| next unless model.ancestors.map(&:to_s).include?(ancestor) - model_class = klass.new(model, self) - properties = model_class.call + parser = klass.new(model, self) break end - @definitions[model_name] = { type: 'object', properties: properties || {} } + properties = parser.call unless parser.nil? + + raise GrapeSwagger::Errors::UnregisteredParser, "No parser registred for #{model_name}." if parser.nil? + raise GrapeSwagger::Errors::SwaggerSpec, "Empty model #{model_name}, swagger 2.0 doesn't support empty definitions." if properties.nil? || properties.empty? + + @definitions[model_name] = { type: 'object', properties: properties } model_name end diff --git a/lib/grape-swagger/errors.rb b/lib/grape-swagger/errors.rb index 145d6429..243e47b0 100644 --- a/lib/grape-swagger/errors.rb +++ b/lib/grape-swagger/errors.rb @@ -5,5 +5,8 @@ def initialize(missing_gem) super("Missing required dependency: #{missing_gem}") end end + + class UnregisteredParser < StandardError; end + class SwaggerSpec < StandardError; end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 167919c3..b24c0e9b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,10 +1,10 @@ $LOAD_PATH.unshift File.expand_path('../../lib', __FILE__) -Dir[File.join(Dir.getwd, 'spec/support/*.rb')].each { |f| require f } MODEL_PARSER = ENV.key?('MODEL_PARSER') ? ENV['MODEL_PARSER'].to_s.downcase.sub('grape-swagger-', '') : 'mock' require 'grape' require 'grape-swagger' +Dir[File.join(Dir.getwd, 'spec/support/*.rb')].each { |f| require f } require "grape-swagger/#{MODEL_PARSER}" if MODEL_PARSER != 'mock' require File.join(Dir.getwd, "spec/support/model_parsers/#{MODEL_PARSER}_parser.rb") diff --git a/spec/support/empty_model_parser.rb b/spec/support/empty_model_parser.rb new file mode 100644 index 00000000..b79d3ff9 --- /dev/null +++ b/spec/support/empty_model_parser.rb @@ -0,0 +1,20 @@ +class EmptyClass +end + +module GrapeSwagger + class EmptyModelParser + attr_reader :model + attr_reader :endpoint + + def initialize(model, endpoint) + @model = model + @endpoint = endpoint + end + + def call + {} + end + end +end + +GrapeSwagger.model_parsers.register(GrapeSwagger::EmptyModelParser, EmptyClass) diff --git a/spec/support/mock_parser.rb b/spec/support/mock_parser.rb new file mode 100644 index 00000000..e39e7b8a --- /dev/null +++ b/spec/support/mock_parser.rb @@ -0,0 +1,22 @@ +module GrapeSwagger + class MockParser + attr_reader :model + attr_reader :endpoint + + def initialize(model, endpoint) + @model = model + @endpoint = endpoint + end + + def call + { + mock_data: { + type: :string, + description: "it's a mock" + } + } + end + end +end + +GrapeSwagger.model_parsers.register(GrapeSwagger::MockParser, OpenStruct) diff --git a/spec/support/model_parsers/mock_parser.rb b/spec/support/model_parsers/mock_parser.rb index 27750d19..ff66cf39 100644 --- a/spec/support/model_parsers/mock_parser.rb +++ b/spec/support/model_parsers/mock_parser.rb @@ -57,22 +57,56 @@ class RecursiveModel < OpenStruct; end let(:swagger_definitions_models) do { - 'ApiError' => { 'type' => 'object', 'properties' => {} }, - 'UseResponse' => { 'type' => 'object', 'properties' => {} }, - 'RecursiveModel' => { 'type' => 'object', 'properties' => {} } + 'ApiError' => { + 'type' => 'object', + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + } + }, + 'RecursiveModel' => { + 'type' => 'object', + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + } + }, + 'UseResponse' => { + 'type' => 'object', + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + } + } } end let(:swagger_nested_type) do { - 'UseItemResponseAsType' => { + 'ApiError' => { 'type' => 'object', - 'properties' => {}, + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + }, 'description' => 'This returns something' }, - 'ApiError' => { + 'UseItemResponseAsType' => { 'type' => 'object', - 'properties' => {}, + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + }, 'description' => 'This returns something' } } @@ -82,12 +116,22 @@ class RecursiveModel < OpenStruct; end { 'UseResponse' => { 'type' => 'object', - 'properties' => {}, + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + }, 'description' => 'This returns something' }, 'ApiError' => { 'type' => 'object', - 'properties' => {}, + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + }, 'description' => 'This returns something' } } @@ -97,14 +141,24 @@ class RecursiveModel < OpenStruct; end { 'ApiError' => { 'type' => 'object', - 'properties' => {}, + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + }, 'description' => 'This returns something' } } end let(:swagger_typed_defintion) do - {} + { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + } end let(:swagger_json) do @@ -221,17 +275,32 @@ class RecursiveModel < OpenStruct; end 'definitions' => { 'QueryInput' => { 'type' => 'object', - 'properties' => {}, + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + }, 'description' => 'nested route inside namespace' }, 'ApiError' => { 'type' => 'object', - 'properties' => {}, + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + }, 'description' => 'This gets Things.' }, 'Something' => { 'type' => 'object', - 'properties' => {}, + 'properties' => { + 'mock_data' => { + 'type' => 'string', + 'description' => "it's a mock" + } + }, 'description' => 'This gets Things.' } } diff --git a/spec/swagger_v2/errors_spec.rb b/spec/swagger_v2/errors_spec.rb new file mode 100644 index 00000000..f314d238 --- /dev/null +++ b/spec/swagger_v2/errors_spec.rb @@ -0,0 +1,75 @@ +require 'spec_helper' + +describe 'Errors' do + describe 'Empty model error' do + let!(:app) do + Class.new(Grape::API) do + format :json + + desc 'Empty model get.' do + http_codes [ + { code: 200, message: 'get Empty model', model: EmptyClass } + ] + end + get '/empty_model' do + something = OpenStruct.new text: 'something' + present something, with: EmptyClass + end + + version 'v3', using: :path + add_swagger_documentation api_version: 'v1', + base_path: '/api', + info: { + title: 'The API title to be displayed on the API homepage.', + description: 'A description of the API.', + contact_name: 'Contact name', + contact_email: 'Contact@email.com', + contact_url: 'Contact URL', + license: 'The name of the license.', + license_url: 'www.The-URL-of-the-license.org', + terms_of_service_url: 'www.The-URL-of-the-terms-and-service.com' + } + end + end + + it 'should raise SwaggerSpec exception' do + expect { get '/v3/swagger_doc' }.to raise_error(GrapeSwagger::Errors::SwaggerSpec, "Empty model EmptyClass, swagger 2.0 doesn't support empty definitions.") + end + end + + describe 'Parser not found error' do + let!(:app) do + Class.new(Grape::API) do + format :json + + desc 'Wrong model get.' do + http_codes [ + { code: 200, message: 'get Wrong model', model: Hash } + ] + end + get '/wrong_model' do + something = OpenStruct.new text: 'something' + present something, with: Hash + end + + version 'v3', using: :path + add_swagger_documentation api_version: 'v1', + base_path: '/api', + info: { + title: 'The API title to be displayed on the API homepage.', + description: 'A description of the API.', + contact_name: 'Contact name', + contact_email: 'Contact@email.com', + contact_url: 'Contact URL', + license: 'The name of the license.', + license_url: 'www.The-URL-of-the-license.org', + terms_of_service_url: 'www.The-URL-of-the-terms-and-service.com' + } + end + end + + it 'should raise UnregisteredParser exception' do + expect { get '/v3/swagger_doc' }.to raise_error(GrapeSwagger::Errors::UnregisteredParser, 'No parser registred for Hash.') + end + end +end