refoctors building definitions for body params

- adds base spec

Author: LeFnord

.rubocop_todo.yml

@@ -28,7 +28,7 @@ Metrics/AbcSize:
 # Offense count: 3
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 216
+  Max: 223
 
 # Offense count: 10
 Metrics/CyclomaticComplexity:

lib/grape-swagger/doc_methods/move_params.rb

@@ -2,54 +2,38 @@ module GrapeSwagger
   module DocMethods
     class MoveParams
       class << self
-        def to_definition(paths, definitions)
-          @definitions = definitions
+        attr_accessor :definitions
 
-          find_post_put(paths) do |method_definition|
-            verb = method_definition.keys.first
-            method_object = method_definition[verb]
+        def to_definition(params, route, definitions)
+          @definitions = definitions
 
-            find_definition_and_params(method_object, verb)
-          end
+          parent_definition_of_params(params, route)
         end
 
-        def find_post_put(paths)
-          paths.each do |x|
-            found = x.last.select { |y| move_methods.include?(y) }
-            yield found unless found.empty?
-          end
+        def can_be_moved?(params, http_verb)
+          move_methods.include?(http_verb) && includes_body_param?(params)
         end
 
-        def find_definition_and_params(path, verb)
-          params = path[:parameters]
-
-          return if params.nil?
-          return unless should_move?(params)
-
+        def parent_definition_of_params(params, route)
           unify!(params)
 
-          status_code = GrapeSwagger::DocMethods::StatusCodes.get[verb.to_sym][:code]
-          response = path[:responses][status_code]
+          definition_name = GrapeSwagger::DocMethods::OperationId.manipulate(parse_model(route.path))
+          referenced_definition = build_definition(definition_name, route.request_method.downcase)
+          definition = @definitions[referenced_definition]
 
-          if response[:schema] && response[:schema]['$ref']
-            referenced_definition = parse_model(response[:schema]['$ref'])
-            name = build_definition(referenced_definition, verb)
-          else
-            referenced_definition = path[:operationId]
-            name = build_definition(referenced_definition)
-          end
+          move_params_to_new(referenced_definition, definition, params)
+
+          definition[:description] = route.description if route.respond_to?(:description)
 
-          move_params_to_new(name, params)
+          params << build_body_parameter(referenced_definition, definition_name)
 
-          @definitions[name][:description] = path[:description] if path[:description]
-          path[:parameters] << build_body_parameter(response.dup, name)
+          params
         end
 
-        def move_params_to_new(name, params)
+        def move_params_to_new(definition_name, definition, params)
           properties = {}
-          definition = @definitions[name]
 
-          nested_definitions(name, params, properties)
+          nested_definitions(definition_name, params, properties)
 
           params.dup.each do |param|
             next unless movable?(param)
@@ -64,7 +48,6 @@ def move_params_to_new(name, params)
             end
 
             params.delete(param) if deletable?(param)
-
             definition[:required] << name if deletable?(param) && param[:required]
           end
 
@@ -90,29 +73,28 @@ def nested_definitions(name, params, properties)
             else
               properties[nested_name] = { '$ref' => "#/definitions/#{def_name}" }
             end
+
             prepare_nested_names(nested)
-            build_definition(def_name)
-            @definitions[def_name][:description] = "#{name} - #{nested_name}"
-            move_params_to_new(def_name, nested)
+            definition = build_definition(def_name)
+            @definitions[definition][:description] = "#{name} - #{nested_name}"
+            move_params_to_new(definition, @definitions[definition], nested)
           end
         end
 
         private
 
-        def build_body_parameter(response, name = false)
-          entity = response[:schema] ? parse_model(response[:schema]['$ref']) : name
+        def build_body_parameter(reference, name)
           body_param = {}
           body_param.tap do |x|
-            x[:name] = entity
+            x[:name] = name
             x[:in] = 'body'
             x[:required] = true
-            x[:schema] = { '$ref' => response[:schema]['$ref'] } unless name
-            x[:schema] = { '$ref' => "#/definitions/#{name}" } if name
+            x[:schema] = { '$ref' => "#/definitions/#{reference}" }
           end
         end
 
         def build_definition(name, verb = nil)
-          name = "#{verb}Request#{name}" if verb
+          name = "#{verb}#{name}" if verb
           @definitions[name] = { type: 'object', properties: {}, required: [] }
 
           name
@@ -136,18 +118,13 @@ def prepare_nested_names(params)
         end
 
         def unify!(params)
-          params.each do |param|
-            param[:in] = param.delete(:param_type) if param.key?(:param_type)
-            param[:in] = 'body' if param[:in] == 'formData'
-          end
+          params.each { |x| x[:in] = x.delete(:param_type) if x[:param_type] }
+          params.each { |x| x[:in] = 'body' if x[:in] == 'formData' } if includes_body_param?(params)
         end
 
         def parse_model(ref)
-          ref.split('/').last
-        end
-
-        def move_methods
-          [:post, :put, :patch]
+          parts = ref.split('/')
+          parts.last.include?('{') ? parts[0..-2].join('/') : parts[0..-1].join('/')
         end
 
         def property_keys
@@ -157,10 +134,16 @@ def property_keys
         def movable?(param)
           param[:in] == 'body'
         end
+
         alias deletable? movable?
 
-        def should_move?(params)
-          !params.select { |x| x[:in] == 'body' || x[:param_type] == 'body' }.empty?
+        def move_methods
+          [:post, :put, :patch, 'POST', 'PUT', 'PATCH']
+        end
+
+        def includes_body_param?(params)
+          params.map { |x| return true if x[:in] == 'body' }
+          false
         end
       end
     end

lib/grape-swagger/endpoint.rb

@@ -71,7 +71,6 @@ def path_and_definition_objects(namespace_routes, options)
       end
 
       add_definitions_from options[:models]
-      GrapeSwagger::DocMethods::MoveParams.to_definition(@paths, @definitions)
       [@paths, @definitions]
     end
 
@@ -154,11 +153,17 @@ def consumes_object(route, format)
     end
 
     def params_object(route)
-      partition_params(route).map do |param, value|
+      parameters = 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
+
+      if GrapeSwagger::DocMethods::MoveParams.can_be_moved?(parameters, route.request_method)
+        parameters = GrapeSwagger::DocMethods::MoveParams.to_definition(parameters, route, @definitions)
+      end
+
+      parameters
     end
 
     def response_object(route, markdown)

spec/lib/move_params_spec.rb

@@ -7,47 +7,22 @@
 
   it { expect(subject.to_s).to eql 'GrapeSwagger::DocMethods::MoveParams' }
 
-  describe 'find_post_put' do
-    let(:paths) { {} }
-
-    describe 'paths empty' do
-      specify { expect { |b| subject.find_post_put(paths, &b) }.not_to yield_control }
-    end
-
-    describe 'no post/put given' do
-      let(:paths) do
-        {
-          :'/foo' => { get: {}, delete: {} },
-          :'/bar/{key}' => { get: {}, delete: {} }
-        }
-      end
-      specify { expect { |b| subject.find_post_put(paths, &b) }.not_to yield_control }
+  describe 'parent_definition_of_params' do
+    let(:params) { paths['/in_body'][:post][:parameters] }
+    let(:options)  do
+      {
+        method: 'POST'
+      }
     end
+    let(:env) { Rack::MockRequest.env_for('/in_body', options) }
+    let(:request) { Grape::Request.new(env) }
 
-    describe 'no post/put given' do
-      let(:paths) do
-        {
-          :'/foo' => { get: {}, delete: {}, post: {}, put: {}, patch: {} },
-          :'/bar/{key}' => { get: {}, delete: {}, post: {}, put: {}, patch: {} }
-        }
-      end
-      let(:expected) do
-        [
-          { post: {}, put: {}, patch: {} },
-          { post: {}, put: {}, patch: {} }
-        ]
-      end
-      specify { expect { |b| subject.find_post_put(paths, &b) }.to yield_control.twice }
-      specify { expect { |b| subject.find_post_put(paths, &b) }.to yield_successive_args *expected }
-    end
-  end
-
-  describe 'find_definition_and_params' do
     specify do
       subject.instance_variable_set(:@definitions, definitions)
-      subject.find_definition_and_params(found_path[:post], :post)
-      expect(definitions.keys).to include 'InBody'
-      expect(definitions['postRequestInBody'].keys).to_not include :description
+      body_definition = subject.parent_definition_of_params(params, request).first
+
+      expect(body_definition[:schema]['$ref']).to eql '#/definitions/postInBody'
+      expect(subject.definitions['postInBody']).not_to include :description
     end
   end
 
@@ -61,10 +36,9 @@
 
       specify do
         subject.instance_variable_set(:@definitions, definitions)
-        name = subject.send(:build_definition, name, verb)
-        subject.move_params_to_new(name, params)
-
-        expect(definitions[name]).to eql expected_post_defs
+        def_name = subject.send(:build_definition, name, verb)
+        subject.move_params_to_new(def_name, definitions[def_name], params)
+        expect(definitions[def_name]).to eql expected_post_defs
         expect(params).to be_empty
       end
     end
@@ -75,10 +49,11 @@
 
       specify do
         subject.instance_variable_set(:@definitions, definitions)
-        name, definition = subject.send(:build_definition, name, verb)
-        subject.move_params_to_new(name, params)
 
-        expect(definitions[name]).to eql expected_put_defs
+        def_name = subject.send(:build_definition, name, verb)
+        subject.move_params_to_new(def_name, definitions[def_name], params)
+
+        expect(definitions[def_name]).to eql expected_put_defs
         expect(params.length).to be 1
       end
     end
@@ -130,7 +105,7 @@
 
         specify do
           definition = definitions.to_a.first
-          expect(definition.first).to eql 'postRequestFoo'
+          expect(definition.first).to eql 'postFoo'
           expect(definition.last).to eql(type: 'object', properties: {}, required: [])
         end
       end
@@ -149,36 +124,33 @@
     end
 
     describe 'build_body_parameter' do
-      let(:response) { { schema: { '$ref' => '#/definitions/Somewhere' } } }
-
-      describe 'no name given' do
-        let(:name) { nil }
-        let(:expected_param) do
-          { name: 'Somewhere', in: 'body', required: true, schema: { '$ref' => '#/definitions/Somewhere' } }
-        end
-        specify do
-          parameter = subject.send(:build_body_parameter, response)
-          expect(parameter).to eql expected_param
-        end
-      end
-
       describe 'name given' do
         let(:name) { 'Foo' }
+        let(:reference) { 'Bar' }
         let(:expected_param) do
-          { name: 'Somewhere', in: 'body', required: true, schema: { '$ref' => "#/definitions/#{name}" } }
+          { name: name, in: 'body', required: true, schema: { '$ref' => "#/definitions/#{reference}" } }
         end
         specify do
-          parameter = subject.send(:build_body_parameter, response, name)
+          parameter = subject.send(:build_body_parameter, reference, name)
           expect(parameter).to eql expected_param
         end
       end
     end
 
     describe 'parse_model' do
       let(:ref) { '#/definitions/InBody' }
-      subject(:object) { described_class.send(:parse_model, ref) }
+      describe 'post request' do
+        subject(:object) { described_class.send(:parse_model, ref) }
+
+        specify { expect(object).to eql ref }
+      end
+
+      describe 'post request' do
+        let(:put_ref) { '#/definitions/InBody/{id}' }
+        subject(:object) { described_class.send(:parse_model, put_ref) }
 
-      specify { expect(object).to eql 'InBody' }
+        specify { expect(object).to eql ref }
+      end
     end
 
     describe 'movable' do
@@ -225,32 +197,9 @@
       end
     end
 
-    describe 'should move' do
-      describe 'no move' do
-        let(:params) do
-          [
-            { in: 'path', name: 'key', description: nil, type: 'integer', format: 'int32', required: true },
-            { in: 'formData', name: 'in_form_data', description: 'in_form_data', type: 'integer', format: 'int32', required: true }
-          ]
-        end
-        it { expect(subject.send(:should_move?, params)).to be false }
-      end
-
-      describe 'move' do
-        let(:params) do
-          [
-            { in: 'path', name: 'key', description: nil, type: 'integer', format: 'int32', required: true },
-            { in: 'body', name: 'in_bosy', description: 'in_bosy', type: 'integer', format: 'int32', required: true },
-            { in: 'formData', name: 'in_form_data', description: 'in_form_data', type: 'integer', format: 'int32', required: true }
-          ]
-        end
-        it { expect(subject.send(:should_move?, params)).to be true }
-      end
-    end
-
     describe 'unify' do
-      before do
-        subject.send(:unify!, params) if subject.send(:should_move?, params)
+      before :each do
+        subject.send(:unify!, params)
       end
       describe 'param type with `:in` given' do
         let(:params) do