Merge pull request ruby-grape#420 from Bugagazavr/avoid-empty-hashes

Avoid empty hashes

Author: dblock

.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

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 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)
 

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}." 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
     end

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

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")
 

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)

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)

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.'
         }
       }

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: '[email protected]',
+                                    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: '[email protected]',
+                                    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