Raise exceptions on empty definitions
This commit is contained in:
		
							parent
							
								
									6f665dfc97
								
							
						
					
					
						commit
						4cac1a938c
					
				|  | @ -1,6 +1,6 @@ | ||||||
| # This configuration was generated by | # This configuration was generated by | ||||||
| # `rubocop --auto-gen-config` | # `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 | # The point is for the user to remove these configuration records | ||||||
| # one by one as the offenses are removed from the code base. | # one by one as the offenses are removed from the code base. | ||||||
| # Note that changes in the inspected code, or installation of new | # Note that changes in the inspected code, or installation of new | ||||||
|  | @ -21,26 +21,26 @@ Lint/UselessAssignment: | ||||||
|   Exclude: |   Exclude: | ||||||
|     - 'spec/lib/move_params_spec.rb' |     - 'spec/lib/move_params_spec.rb' | ||||||
| 
 | 
 | ||||||
| # Offense count: 26 | # Offense count: 27 | ||||||
| Metrics/AbcSize: | Metrics/AbcSize: | ||||||
|   Max: 56 |   Max: 56 | ||||||
| 
 | 
 | ||||||
| # Offense count: 3 | # Offense count: 3 | ||||||
| # Configuration parameters: CountComments. | # Configuration parameters: CountComments. | ||||||
| Metrics/ClassLength: | Metrics/ClassLength: | ||||||
|   Max: 195 |   Max: 201 | ||||||
| 
 | 
 | ||||||
| # Offense count: 10 | # Offense count: 10 | ||||||
| Metrics/CyclomaticComplexity: | Metrics/CyclomaticComplexity: | ||||||
|   Max: 16 |   Max: 16 | ||||||
| 
 | 
 | ||||||
| # Offense count: 711 | # Offense count: 719 | ||||||
| # Configuration parameters: AllowHeredoc, AllowURI, URISchemes. | # Configuration parameters: AllowHeredoc, AllowURI, URISchemes. | ||||||
| # URISchemes: http, https | # URISchemes: http, https | ||||||
| Metrics/LineLength: | Metrics/LineLength: | ||||||
|   Max: 454 |   Max: 454 | ||||||
| 
 | 
 | ||||||
| # Offense count: 30 | # Offense count: 31 | ||||||
| # Configuration parameters: CountComments. | # Configuration parameters: CountComments. | ||||||
| Metrics/MethodLength: | Metrics/MethodLength: | ||||||
|   Max: 101 |   Max: 101 | ||||||
|  |  | ||||||
|  | @ -8,7 +8,8 @@ | ||||||
| #### Fixes | #### Fixes | ||||||
| 
 | 
 | ||||||
| * [#416](https://github.com/ruby-grape/grape-swagger/pull/416): Support recursive models - [@lest](https://github.com/lest). | * [#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 isn't satisfied, when no parser for model registred or response model is empty - [@Bugagazavr](https://github.com/Bugagazavr). | ||||||
| 
 | 
 | ||||||
| ### 0.20.3 (May 9, 2016) | ### 0.20.3 (May 9, 2016) | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -232,17 +232,23 @@ module Grape | ||||||
|       return model_name if @definitions.key?(model_name) |       return model_name if @definitions.key?(model_name) | ||||||
|       @definitions[model_name] = nil |       @definitions[model_name] = nil | ||||||
| 
 | 
 | ||||||
|       properties = {} |       properties = nil | ||||||
|  |       parser = nil | ||||||
|  | 
 | ||||||
|       GrapeSwagger.model_parsers.each do |klass, ancestor| |       GrapeSwagger.model_parsers.each do |klass, ancestor| | ||||||
|         next unless model.ancestors.map(&:to_s).include?(ancestor) |         next unless model.ancestors.map(&:to_s).include?(ancestor) | ||||||
| 
 | 
 | ||||||
|         model_class = klass.new(model, self) |         parser = klass.new(model, self) | ||||||
|         properties = model_class.call |  | ||||||
| 
 | 
 | ||||||
|         break |         break | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       @definitions[model_name] = { type: 'object', properties: properties || {} } |       properties = parser.call unless parser.nil? | ||||||
|  | 
 | ||||||
|  |       raise GrapeSwagger::Errors::UnregisteredParser, "No parser registred for #{model_name}." unless parser | ||||||
|  |       raise GrapeSwagger::Errors::SwaggerSpec, "Empty model #{model_name}, swagger 2.0 doesn't support empty definitions." unless properties && properties.any? | ||||||
|  | 
 | ||||||
|  |       @definitions[model_name] = { type: 'object', properties: properties } | ||||||
| 
 | 
 | ||||||
|       model_name |       model_name | ||||||
|     end |     end | ||||||
|  |  | ||||||
|  | @ -5,5 +5,8 @@ module GrapeSwagger | ||||||
|         super("Missing required dependency: #{missing_gem}") |         super("Missing required dependency: #{missing_gem}") | ||||||
|       end |       end | ||||||
|     end |     end | ||||||
|  | 
 | ||||||
|  |     class UnregisteredParser < StandardError; end | ||||||
|  |     class SwaggerSpec < StandardError; end | ||||||
|   end |   end | ||||||
| end | end | ||||||
|  |  | ||||||
|  | @ -1,10 +1,10 @@ | ||||||
| $LOAD_PATH.unshift File.expand_path('../../lib', __FILE__) | $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' | MODEL_PARSER = ENV.key?('MODEL_PARSER') ? ENV['MODEL_PARSER'].to_s.downcase.sub('grape-swagger-', '') : 'mock' | ||||||
| 
 | 
 | ||||||
| require 'grape' | require 'grape' | ||||||
| require 'grape-swagger' | 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 "grape-swagger/#{MODEL_PARSER}" if MODEL_PARSER != 'mock' | ||||||
| require File.join(Dir.getwd, "spec/support/model_parsers/#{MODEL_PARSER}_parser.rb") | require File.join(Dir.getwd, "spec/support/model_parsers/#{MODEL_PARSER}_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) | ||||||
|  | @ -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) | ||||||
|  | @ -57,22 +57,56 @@ RSpec.shared_context 'mock swagger example' do | ||||||
| 
 | 
 | ||||||
|   let(:swagger_definitions_models) do |   let(:swagger_definitions_models) do | ||||||
|     { |     { | ||||||
|       'ApiError' => { 'type' => 'object', 'properties' => {} }, |       'ApiError' => { | ||||||
|       'UseResponse' => { 'type' => 'object', 'properties' => {} }, |         'type' => 'object', | ||||||
|       'RecursiveModel' => { 'type' => 'object', 'properties' => {} } |         '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 |   end | ||||||
| 
 | 
 | ||||||
|   let(:swagger_nested_type) do |   let(:swagger_nested_type) do | ||||||
|     { |     { | ||||||
|       'UseItemResponseAsType' => { |  | ||||||
|         'type' => 'object', |  | ||||||
|         'properties' => {}, |  | ||||||
|         'description' => 'This returns something' |  | ||||||
|       }, |  | ||||||
|       'ApiError' => { |       'ApiError' => { | ||||||
|         'type' => 'object', |         'type' => 'object', | ||||||
|         'properties' => {}, |         'properties' => { | ||||||
|  |           'mock_data' => { | ||||||
|  |             'type' => 'string', | ||||||
|  |             'description' => "it's a mock" | ||||||
|  |           } | ||||||
|  |         }, | ||||||
|  |         'description' => 'This returns something' | ||||||
|  |       }, | ||||||
|  |       'UseItemResponseAsType' => { | ||||||
|  |         'type' => 'object', | ||||||
|  |         'properties' => { | ||||||
|  |           'mock_data' => { | ||||||
|  |             'type' => 'string', | ||||||
|  |             'description' => "it's a mock" | ||||||
|  |           } | ||||||
|  |         }, | ||||||
|         'description' => 'This returns something' |         'description' => 'This returns something' | ||||||
|       } |       } | ||||||
|     } |     } | ||||||
|  | @ -82,12 +116,22 @@ RSpec.shared_context 'mock swagger example' do | ||||||
|     { |     { | ||||||
|       'UseResponse' => { |       'UseResponse' => { | ||||||
|         'type' => 'object', |         'type' => 'object', | ||||||
|         'properties' => {}, |         'properties' => { | ||||||
|  |           'mock_data' => { | ||||||
|  |             'type' => 'string', | ||||||
|  |             'description' => "it's a mock" | ||||||
|  |           } | ||||||
|  |         }, | ||||||
|         'description' => 'This returns something' |         'description' => 'This returns something' | ||||||
|       }, |       }, | ||||||
|       'ApiError' => { |       'ApiError' => { | ||||||
|         'type' => 'object', |         'type' => 'object', | ||||||
|         'properties' => {}, |         'properties' => { | ||||||
|  |           'mock_data' => { | ||||||
|  |             'type' => 'string', | ||||||
|  |             'description' => "it's a mock" | ||||||
|  |           } | ||||||
|  |         }, | ||||||
|         'description' => 'This returns something' |         'description' => 'This returns something' | ||||||
|       } |       } | ||||||
|     } |     } | ||||||
|  | @ -97,14 +141,24 @@ RSpec.shared_context 'mock swagger example' do | ||||||
|     { |     { | ||||||
|       'ApiError' => { |       'ApiError' => { | ||||||
|         'type' => 'object', |         'type' => 'object', | ||||||
|         'properties' => {}, |         'properties' => { | ||||||
|  |           'mock_data' => { | ||||||
|  |             'type' => 'string', | ||||||
|  |             'description' => "it's a mock" | ||||||
|  |           } | ||||||
|  |         }, | ||||||
|         'description' => 'This returns something' |         'description' => 'This returns something' | ||||||
|       } |       } | ||||||
|     } |     } | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   let(:swagger_typed_defintion) do |   let(:swagger_typed_defintion) do | ||||||
|     {} |     { | ||||||
|  |       'mock_data' => { | ||||||
|  |         'type' => 'string', | ||||||
|  |         'description' => "it's a mock" | ||||||
|  |       } | ||||||
|  |     } | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   let(:swagger_json) do |   let(:swagger_json) do | ||||||
|  | @ -221,17 +275,32 @@ RSpec.shared_context 'mock swagger example' do | ||||||
|       'definitions' => { |       'definitions' => { | ||||||
|         'QueryInput' => { |         'QueryInput' => { | ||||||
|           'type' => 'object', |           'type' => 'object', | ||||||
|           'properties' => {}, |           'properties' => { | ||||||
|  |             'mock_data' => { | ||||||
|  |               'type' => 'string', | ||||||
|  |               'description' => "it's a mock" | ||||||
|  |             } | ||||||
|  |           }, | ||||||
|           'description' => 'nested route inside namespace' |           'description' => 'nested route inside namespace' | ||||||
|         }, |         }, | ||||||
|         'ApiError' => { |         'ApiError' => { | ||||||
|           'type' => 'object', |           'type' => 'object', | ||||||
|           'properties' => {}, |           'properties' => { | ||||||
|  |             'mock_data' => { | ||||||
|  |               'type' => 'string', | ||||||
|  |               'description' => "it's a mock" | ||||||
|  |             } | ||||||
|  |           }, | ||||||
|           'description' => 'This gets Things.' |           'description' => 'This gets Things.' | ||||||
|         }, |         }, | ||||||
|         'Something' => { |         'Something' => { | ||||||
|           'type' => 'object', |           'type' => 'object', | ||||||
|           'properties' => {}, |           'properties' => { | ||||||
|  |             'mock_data' => { | ||||||
|  |               'type' => 'string', | ||||||
|  |               'description' => "it's a mock" | ||||||
|  |             } | ||||||
|  |           }, | ||||||
|           'description' => 'This gets Things.' |           'description' => 'This gets Things.' | ||||||
|         } |         } | ||||||
|       } |       } | ||||||
|  |  | ||||||
|  | @ -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 | ||||||
		Loading…
	
		Reference in New Issue