From e21ccb38ff12ba7e9a27fd2e8829242ebe13cce6 Mon Sep 17 00:00:00 2001 From: Braktar Date: Tue, 16 Jul 2019 16:18:53 +0200 Subject: [PATCH 01/18] Depply nested objects --- spec/issues/751_deeply_nested_objects_spec.rb | 196 ++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 spec/issues/751_deeply_nested_objects_spec.rb diff --git a/spec/issues/751_deeply_nested_objects_spec.rb b/spec/issues/751_deeply_nested_objects_spec.rb new file mode 100644 index 00000000..8f480287 --- /dev/null +++ b/spec/issues/751_deeply_nested_objects_spec.rb @@ -0,0 +1,196 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe '751 deeply nested objects' do + let(:app) do + Class.new(Grape::API) do + content_type :json, 'application/json; charset=UTF-8' + default_format :json + class Vrp < Grape::API + def self.vrp_request_timewindow(this) + this.optional(:start, types: [String, Float, Integer]) + this.optional(:end, types: [String, Float, Integer]) + end + + def self.vrp_request_point(this) + this.requires(:id, type: String, allow_blank: false) + this.optional(:matrix_index, type: Integer) + this.optional(:location, type: Hash) do + requires(:lat, type: Float, allow_blank: false) + requires(:lon, type: Float, allow_blank: false) + end + this.at_least_one_of :matrix_index, :location + end + + def self.vrp_request_activity(this) + this.optional(:duration, types: [String, Float, Integer]) + this.requires(:point_id, type: String, allow_blank: false) + this.optional(:timewindows, type: Array) do + Vrp.vrp_request_timewindow(self) + end + end + + def self.vrp_request_service(this) + this.requires(:id, type: String, allow_blank: false) + this.optional(:skills, type: Array[String]) + + this.optional(:activity, type: Hash) do + Vrp.vrp_request_activity(self) + end + this.optional(:activities, type: Array) do + Vrp.vrp_request_activity(self) + end + this.mutually_exclusive :activity, :activities + end + end + + namespace :vrp do + resource :submit do + desc 'Submit Problems', nickname: 'vrp' + params do + optional(:vrp, type: Hash, documentation: { param_type: 'body' }) do + optional(:points, type: Array) do + Vrp.vrp_request_point(self) + end + + optional(:services, type: Array) do + Vrp.vrp_request_service(self) + end + end + end + post do + { vrp: params[:vrp] }.to_json + end + end + end + + add_swagger_documentation format: :json + end + end + + subject do + get '/swagger_doc' + JSON.parse(last_response.body) + end + + describe 'Correctness of vrp Points' do + let(:get_points_response) { subject['definitions']['postVrpSubmit']['properties']['vrp']['items']['properties']['points'] } + specify do + expect(get_points_response).to include( + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'properties' => { + 'id' => { + 'type' => 'string' + }, + 'matrix_index' => { + 'type' => 'integer', + 'format' => 'int32' + }, + 'location' => { + 'type' => 'object', + 'items' => { + 'type' => 'object', + 'properties' => { + 'lat' => { + 'type' => 'number', + 'format' => 'float' + }, + 'lon' => { + 'type' => 'number', + 'format' => 'float' + } + }, + 'required' => %w[lat lon] + } + } + }, + 'required' => ['id'] + } + ) + end + end + + describe 'Correctness of vrp Services' do + let(:get_service_response) { subject['definitions']['postVrpSubmit']['properties']['vrp']['items']['properties']['services'] } + specify do + expect(get_service_response).to include( + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'properties' => { + 'id' => { + 'type' => 'string' + }, + 'skills' => { + 'type' => 'array', + 'items' => { + 'type' => 'string' + } + }, + 'activity' => { + 'type' => 'object', + 'items' => { + 'type' => 'object', + 'properties' => { + 'duration' => { + 'type' => 'string' + }, + 'point_id' => { + 'type' => 'string' + }, + 'timewindows' => { + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'properties' => { + 'start' => { + 'type' => 'string' + }, + 'end' => { + 'type' => 'string' + } + } + } + } + }, + 'required' => ['point_id'] + } + }, 'activities' => { + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'properties' => { + 'duration' => { + 'type' => 'string' + }, + 'point_id' => { + 'type' => 'string' + }, + 'timewindows' => { + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'properties' => { + 'start' => { + 'type' => 'string' + }, + 'end' => { + 'type' => 'string' + } + } + } + } + }, + 'required' => ['point_id'] + } + } + }, + 'required' => ['id'] + } + ) + end + end +end From 66ff646a6fc516bc77c2b1a5e09c291a941d8687 Mon Sep 17 00:00:00 2001 From: Braktar Date: Wed, 17 Jul 2019 10:44:25 +0200 Subject: [PATCH 02/18] Simple hash syntax edit --- lib/grape-swagger/endpoint.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 22631a36..04134398 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -78,8 +78,7 @@ def contact_object(infos) def path_and_definition_objects(namespace_routes, options) @paths = {} @definitions = {} - namespace_routes.each_key do |key| - routes = namespace_routes[key] + namespace_routes.each_value do |routes| path_item(routes, options) end From fb0090705ff58d4e3f2dd7d3bff7e501e6e0c10a Mon Sep 17 00:00:00 2001 From: Braktar Date: Thu, 18 Jul 2019 12:08:57 +0200 Subject: [PATCH 03/18] Nested objects are not necessary arrays --- lib/grape-swagger/endpoint/params_parser.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/grape-swagger/endpoint/params_parser.rb b/lib/grape-swagger/endpoint/params_parser.rb index 7cc7614e..20e177ef 100644 --- a/lib/grape-swagger/endpoint/params_parser.rb +++ b/lib/grape-swagger/endpoint/params_parser.rb @@ -26,16 +26,6 @@ def parse_request_params options[:is_array] = true name += '[]' if array_use_braces?(options) - else - keys = array_keys.find_all { |key| name.start_with? "#{key}[" } - if keys.any? - options[:is_array] = true - if array_use_braces?(options) - keys.sort.reverse_each do |key| - name = name.sub(key, "#{key}[]") - end - end - end end memo[name] = options unless %w[Hash Array].include?(param_type) && !options.key?(:documentation) From 73fd720d21b227f0cdaf5b1cfa493d055e5acecc Mon Sep 17 00:00:00 2001 From: Braktar Date: Thu, 18 Jul 2019 16:15:19 +0200 Subject: [PATCH 04/18] fix points test --- spec/issues/751_deeply_nested_objects_spec.rb | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/spec/issues/751_deeply_nested_objects_spec.rb b/spec/issues/751_deeply_nested_objects_spec.rb index 8f480287..9dbeddb8 100644 --- a/spec/issues/751_deeply_nested_objects_spec.rb +++ b/spec/issues/751_deeply_nested_objects_spec.rb @@ -75,9 +75,9 @@ def self.vrp_request_service(this) end describe 'Correctness of vrp Points' do - let(:get_points_response) { subject['definitions']['postVrpSubmit']['properties']['vrp']['items']['properties']['points'] } + let(:get_points_response) { subject['definitions']['postVrpSubmit']['properties']['vrp']['properties']['points'] } specify do - expect(get_points_response).to include( + expect(get_points_response).to eql( 'type' => 'array', 'items' => { 'type' => 'object', @@ -91,20 +91,17 @@ def self.vrp_request_service(this) }, 'location' => { 'type' => 'object', - 'items' => { - 'type' => 'object', - 'properties' => { - 'lat' => { - 'type' => 'number', - 'format' => 'float' - }, - 'lon' => { - 'type' => 'number', - 'format' => 'float' - } + 'properties' => { + 'lat' => { + 'type' => 'number', + 'format' => 'float' }, - 'required' => %w[lat lon] - } + 'lon' => { + 'type' => 'number', + 'format' => 'float' + } + }, + 'required' => %w[lat lon] } }, 'required' => ['id'] From 4d2f7ca631963e07879379159b6ac6e039c1dfd1 Mon Sep 17 00:00:00 2001 From: Braktar Date: Thu, 18 Jul 2019 16:21:34 +0200 Subject: [PATCH 05/18] Fix test services --- spec/issues/751_deeply_nested_objects_spec.rb | 45 +++++++++---------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/spec/issues/751_deeply_nested_objects_spec.rb b/spec/issues/751_deeply_nested_objects_spec.rb index 9dbeddb8..c674438f 100644 --- a/spec/issues/751_deeply_nested_objects_spec.rb +++ b/spec/issues/751_deeply_nested_objects_spec.rb @@ -111,7 +111,7 @@ def self.vrp_request_service(this) end describe 'Correctness of vrp Services' do - let(:get_service_response) { subject['definitions']['postVrpSubmit']['properties']['vrp']['items']['properties']['services'] } + let(:get_service_response) { subject['definitions']['postVrpSubmit']['properties']['vrp']['properties']['services'] } specify do expect(get_service_response).to include( 'type' => 'array', @@ -129,32 +129,29 @@ def self.vrp_request_service(this) }, 'activity' => { 'type' => 'object', - 'items' => { - 'type' => 'object', - 'properties' => { - 'duration' => { - 'type' => 'string' - }, - 'point_id' => { - 'type' => 'string' - }, - 'timewindows' => { - 'type' => 'array', - 'items' => { - 'type' => 'object', - 'properties' => { - 'start' => { - 'type' => 'string' - }, - 'end' => { - 'type' => 'string' - } + 'properties' => { + 'duration' => { + 'type' => 'string' + }, + 'point_id' => { + 'type' => 'string' + }, + 'timewindows' => { + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'properties' => { + 'start' => { + 'type' => 'string' + }, + 'end' => { + 'type' => 'string' } } } - }, - 'required' => ['point_id'] - } + } + }, + 'required' => ['point_id'] }, 'activities' => { 'type' => 'array', 'items' => { From fbcb69f8b10a770aba803e5696c9d729e6175640 Mon Sep 17 00:00:00 2001 From: Braktar Date: Thu, 18 Jul 2019 16:57:22 +0200 Subject: [PATCH 06/18] Doesn't require to delete hash or array properties --- lib/grape-swagger/endpoint/params_parser.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape-swagger/endpoint/params_parser.rb b/lib/grape-swagger/endpoint/params_parser.rb index 20e177ef..65c2e5a1 100644 --- a/lib/grape-swagger/endpoint/params_parser.rb +++ b/lib/grape-swagger/endpoint/params_parser.rb @@ -28,7 +28,7 @@ def parse_request_params name += '[]' if array_use_braces?(options) end - memo[name] = options unless %w[Hash Array].include?(param_type) && !options.key?(:documentation) + memo[name] = options #unless %w[Hash Array].include?(param_type) && !options.key?(:documentation) end end From fecda71ec7f7daeac45749502db12fe301bbb8a8 Mon Sep 17 00:00:00 2001 From: Braktar Date: Wed, 31 Jul 2019 10:08:44 +0200 Subject: [PATCH 07/18] Edit nested logic --- lib/grape-swagger/doc_methods/move_params.rb | 67 ++--- spec/lib/move_params_spec.rb | 262 ------------------- 2 files changed, 20 insertions(+), 309 deletions(-) diff --git a/lib/grape-swagger/doc_methods/move_params.rb b/lib/grape-swagger/doc_methods/move_params.rb index a887e192..da372a69 100644 --- a/lib/grape-swagger/doc_methods/move_params.rb +++ b/lib/grape-swagger/doc_methods/move_params.rb @@ -51,22 +51,33 @@ def parent_definition_of_params(params, path, route) def move_params_to_new(definition, params) params, nested_params = params.partition { |x| !x[:name].to_s.include?('[') } - - unless params.blank? - properties, required = build_properties(params) - add_properties_to_definition(definition, properties, required) + params.each do |param| + property = param[:name] + param_properties, param_required = build_properties([param]) + add_properties_to_definition(definition, param_properties, param_required) + related_nested_params, nested_params = nested_params.partition { |x| x[:name].start_with?("#{property}[") } + prepare_nested_names(property, related_nested_params) + + next if related_nested_params.blank? + + nested_definition = if should_expose_as_array?([param]) + move_params_to_new(array_type, related_nested_params) + else + move_params_to_new(object_type, related_nested_params) + end + if definition.key?(:items) + definition[:items][:properties][property.to_sym].deep_merge!(nested_definition) + else + definition[:properties][property.to_sym].deep_merge!(nested_definition) + end end - - nested_properties = build_nested_properties(nested_params) unless nested_params.blank? - add_properties_to_definition(definition, nested_properties, []) unless nested_params.blank? + definition end def build_properties(params) properties = {} required = [] - prepare_nested_types(params) if should_expose_as_array?(params) - params.each do |param| name = param[:name].to_sym @@ -103,28 +114,6 @@ def document_as_property(param) end end - def build_nested_properties(params, properties = {}) - property = params.bsearch { |x| x[:name].include?('[') }[:name].split('[').first - - nested_params, params = params.partition { |x| x[:name].start_with?("#{property}[") } - prepare_nested_names(property, nested_params) - - recursive_call(properties, property, nested_params) unless nested_params.empty? - build_nested_properties(params, properties) unless params.empty? - - properties - end - - def recursive_call(properties, property, nested_params) - if should_expose_as_array?(nested_params) - properties[property.to_sym] = array_type - move_params_to_new(properties[property.to_sym][:items], nested_params) - else - properties[property.to_sym] = object_type - move_params_to_new(properties[property.to_sym], nested_params) - end - end - def movable_params(params) to_delete = params.each_with_object([]) { |x, memo| memo << x if deletable?(x) } delete_from(params, to_delete) @@ -177,22 +166,6 @@ def object_type { type: 'object', properties: {} } end - def prepare_nested_types(params) - params.each do |param| - next unless param[:items] - - param[:type] = if param[:items][:type] == 'array' - 'string' - elsif param[:items].key?('$ref') - param[:type] = 'object' - else - param[:items][:type] - end - param[:format] = param[:items][:format] if param[:items][:format] - param.delete(:items) if param[:type] != 'object' - end - end - def prepare_nested_names(property, params) params.each { |x| x[:name] = x[:name].sub(property, '').sub('[', '').sub(']', '') } end diff --git a/spec/lib/move_params_spec.rb b/spec/lib/move_params_spec.rb index a8b9486e..3ad753f6 100644 --- a/spec/lib/move_params_spec.rb +++ b/spec/lib/move_params_spec.rb @@ -326,268 +326,6 @@ end end - describe 'prepare_nested_types' do - before :each do - subject.send(:prepare_nested_types, params) - end - - let(:params) do - [ - { - in: 'body', - name: 'address[street_lines]', - description: 'street lines', - type: 'array', - items: { - type: 'string' - }, - required: true - } - ] - end - - context 'when params contains nothing with :items key' do - let(:params) do - [ - { - in: 'body', - name: 'phone_number', - description: 'phone number', - type: 'string', - required: true - } - ] - end - - let(:expected_params) do - [ - { - in: 'body', - name: 'phone_number', - description: 'phone number', - type: 'string', - required: true - } - ] - end - - it 'does nothing' do - expect(params).to eq expected_params - end - end - - context 'when params contains :items key with array type' do - let(:params) do - [ - { - in: 'body', - name: 'address_street_lines', - description: 'street lines', - type: 'array', - items: { - type: 'array' - }, - required: true - } - ] - end - - let(:expected_params) do - [ - { - in: 'body', - name: 'address_street_lines', - description: 'street lines', - type: 'string', - required: true - } - ] - end - - it 'sets type to string and removes :items' do - expect(params).to eq expected_params - end - end - - context 'when params contains :items key with $ref' do - let(:params) do - [ - { - in: 'body', - name: 'address_street_lines', - description: 'street lines', - type: 'array', - items: { - '$ref' => '#/definitions/StreetLine' - }, - required: true - } - ] - end - - let(:expected_params) do - [ - { - in: 'body', - name: 'address_street_lines', - description: 'street lines', - type: 'object', - items: { - '$ref' => '#/definitions/StreetLine' - }, - required: true - } - ] - end - - it 'sets type to object and does not remove :items' do - expect(params).to eq expected_params - end - end - - context 'when params contains :items without $ref or array type' do - let(:params) do - [ - { - in: 'body', - name: 'address_street_lines', - description: 'street lines', - type: 'array', - items: { - type: 'string' - }, - required: true - } - ] - end - - let(:expected_params) do - [ - { - in: 'body', - name: 'address_street_lines', - description: 'street lines', - type: 'string', - required: true - } - ] - end - - it 'sets type to :items :type and removes :items' do - expect(params).to eq expected_params - end - end - - context 'when params contains :items key with :format' do - let(:params) do - [ - { - in: 'body', - name: 'street_number', - description: 'street number', - type: 'array', - items: { - type: 'integer', - format: 'int32' - }, - required: true - } - ] - end - - let(:expected_params) do - [ - { - in: 'body', - name: 'street_number', - description: 'street number', - type: 'integer', - format: 'int32', - required: true - } - ] - end - - it 'sets format and removes :items' do - expect(params).to eq expected_params - end - end - end - - describe 'recursive_call' do - before :each do - subject.send(:recursive_call, properties, 'test', nested_params) - end - - let(:properties) { {} } - - context 'when nested params is an array' do - let(:nested_params) do - [ - { - in: 'body', - name: 'aliases', - description: 'The aliases of test.', - type: 'array', - items: { type: 'string' }, - required: true - } - ] - end - - let(:expected_properties) do - { - type: 'array', - items: { - type: 'object', - properties: { - aliases: { - type: 'string', - description: 'The aliases of test.' - } - }, - required: [:aliases] - } - } - end - - it 'adds property as symbol with array type and items' do - expect(properties[:test]).to eq expected_properties - end - end - - context 'when nested params is not an array' do - let(:nested_params) do - [ - { - in: 'body', - name: 'id', - description: 'The unique ID of test.', - type: 'string', - required: true - } - ] - end - - let(:expected_properties) do - { - type: 'object', - required: [:id], - properties: { - id: { - type: 'string', - description: 'The unique ID of test.' - } - } - } - end - - it 'adds property as symbol with object type' do - expect(properties[:test]).to eq expected_properties - end - end - end - describe 'add_properties_to_definition' do before :each do subject.send(:add_properties_to_definition, definition, properties, []) From e5ba63ba26df6df72c0ef4ec07b2bd8499a9576d Mon Sep 17 00:00:00 2001 From: Braktar Date: Wed, 31 Jul 2019 10:46:07 +0200 Subject: [PATCH 08/18] Add vehicle test --- spec/issues/751_deeply_nested_objects_spec.rb | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/spec/issues/751_deeply_nested_objects_spec.rb b/spec/issues/751_deeply_nested_objects_spec.rb index c674438f..191acbbc 100644 --- a/spec/issues/751_deeply_nested_objects_spec.rb +++ b/spec/issues/751_deeply_nested_objects_spec.rb @@ -43,6 +43,28 @@ def self.vrp_request_service(this) end this.mutually_exclusive :activity, :activities end + + def self.vrp_request_vehicle(this) + this.requires(:id, type: String, allow_blank: false) + this.optional(:cost_time_multiplier, type: Float) + + this.optional(:matrix_id, type: String) + this.optional(:router_mode, type: String) + this.exactly_one_of :matrix_id, :router_mode + + this.optional(:skills, type: Array[Array[String]]) + + this.optional(:start_point_id, type: String) + this.optional(:end_point_id, type: String) + + this.optional(:sequence_timewindows, type: Array) do + Vrp.vrp_request_timewindow(self) + end + this.optional(:timewindow, type: Hash) do + Vrp.vrp_request_timewindow(self) + end + this.mutually_exclusive :sequence_timewindows, :timewindow + end end namespace :vrp do @@ -57,6 +79,10 @@ def self.vrp_request_service(this) optional(:services, type: Array) do Vrp.vrp_request_service(self) end + + optional(:vehicles, type: Array) do + Vrp.vrp_request_vehicle(self) + end end end post do @@ -187,4 +213,72 @@ def self.vrp_request_service(this) ) end end + + describe 'Correctness of vrp Vehicles' do + let(:get_points_response) { subject['definitions']['postVrpSubmit']['properties']['vrp']['properties']['vehicles'] } + specify do + expect(get_points_response).to eql( + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'properties' => { + 'id' => { + 'type' => 'string' + }, + 'cost_time_multiplier' => { + 'type' => 'float', + 'format' => 'number' + }, + 'matrix_id' => { + 'type' => 'string' + }, + 'router_mode' => { + 'type' => 'string' + }, + 'skills' => { + 'type' => 'array', + 'items' => { + 'type' => 'array', + 'items' => { + 'type' => 'string' + } + } + }, + 'start_point_id' => { + 'type' => 'string' + }, + 'end_point_id' => { + 'type' => 'string' + }, + 'timewindow' => { + 'type' => 'object', + 'properties' => { + 'start' => { + 'type' => 'string' + }, + 'end' => { + 'type' => 'string' + } + } + }, + 'sequence_timewindows' => { + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'properties' => { + 'start' => { + 'type' => 'string' + }, + 'end' => { + 'type' => 'string' + } + } + } + } + }, + 'required' => ['id'] + } + ) + end + end end From 7269989ef383f0e6e8926e73937d5989dd79b8c6 Mon Sep 17 00:00:00 2001 From: Braktar Date: Fri, 2 Aug 2019 17:30:41 +0200 Subject: [PATCH 09/18] Change parameter level is endpoint job --- lib/grape-swagger/endpoint.rb | 38 +++++++++++++++++++++ lib/grape-swagger/endpoint/params_parser.rb | 2 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 04134398..866e06bf 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -190,9 +190,47 @@ def params_object(route, options, path) parameters = GrapeSwagger::DocMethods::MoveParams.to_definition(path, parameters, route, @definitions) end + parameters.reject { |parameter| parameter[:in] == 'body' }.each do |b| + related_parameters = parameters.select do |p| + p[:name] != b[:name] && p[:name].to_s.include?(b[:name].to_s.gsub(/\[\]\z/, '') + '[') + end + parameters.reject! { |p| p[:name] == b[:name] } if format_data(b, related_parameters) + end + parameters.presence end + def format_data(parameter, related_parameters) + case parameter[:type] + when 'array' + add_array(parameter, related_parameters) + unless related_parameters.blank? + add_braces(parameter, related_parameters) if parameter[:name].match?(/\A.*\[\]\z/) + return true + end + when 'object' + return true + end + false + end + + def add_braces(parameter, related_parameters) + param_name = parameter[:name].gsub(/\A(.*)\[\]\z/, '\1') + related_parameters.each { |p| p[:name] = p[:name].gsub(param_name, param_name + '[]') } + end + + def add_array(parameter, related_parameters) + related_parameters.each do |p| + p[:items] = { type: p[:type], format: p[:format], enum: p[:enum], is_array: p[:is_array] } + p[:items].delete_if { |_k, v| v.nil? } + p[:type] = 'array' + p[:is_array] = parameter[:is_array] + p.delete(:format) + p.delete(:enum) + p.delete_if { |_k, v| v.nil? } + end + end + def response_object(route) codes = http_codes_from_route(route) codes.map! { |x| x.is_a?(Array) ? { code: x[0], message: x[1], model: x[2], examples: x[3], headers: x[4] } : x } diff --git a/lib/grape-swagger/endpoint/params_parser.rb b/lib/grape-swagger/endpoint/params_parser.rb index 65c2e5a1..925ee8ee 100644 --- a/lib/grape-swagger/endpoint/params_parser.rb +++ b/lib/grape-swagger/endpoint/params_parser.rb @@ -28,7 +28,7 @@ def parse_request_params name += '[]' if array_use_braces?(options) end - memo[name] = options #unless %w[Hash Array].include?(param_type) && !options.key?(:documentation) + memo[name] = options end end From 92ecd2b8447fa5c450eca0aaf506e6596c600019 Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 09:35:09 +0200 Subject: [PATCH 10/18] Array body shouldn't lost array type --- spec/swagger_v2/params_array_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/swagger_v2/params_array_spec.rb b/spec/swagger_v2/params_array_spec.rb index c3893ff5..42e3b4a6 100644 --- a/spec/swagger_v2/params_array_spec.rb +++ b/spec/swagger_v2/params_array_spec.rb @@ -123,10 +123,10 @@ 'type' => 'object', 'properties' => { 'array_of_string' => { - 'type' => 'string', 'description' => 'nested array of strings' + 'items' => { 'type' => 'string' }, 'type' => 'array', 'description' => 'nested array of strings' }, 'array_of_integer' => { - 'type' => 'integer', 'format' => 'int32', 'description' => 'nested array of integers' + 'items' => { 'type' => 'integer', 'format' => 'int32' }, 'type' => 'array', 'description' => 'nested array of integers' } }, 'required' => %w[array_of_string array_of_integer] From fc40e35d0c6d120161fe602b5ed4657bcd9a0974 Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 09:38:49 +0200 Subject: [PATCH 11/18] no more parser role --- spec/lib/endpoint/params_parser_spec.rb | 20 ++------------------ spec/lib/endpoint_spec.rb | 6 +++--- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/spec/lib/endpoint/params_parser_spec.rb b/spec/lib/endpoint/params_parser_spec.rb index 539c44ef..73a2eb4b 100644 --- a/spec/lib/endpoint/params_parser_spec.rb +++ b/spec/lib/endpoint/params_parser_spec.rb @@ -46,19 +46,11 @@ context 'when param is nested in a param of array type' do let(:params) { [['param_1', { type: 'Array' }], ['param_1[param_2]', { type: 'String' }]] } - it 'skips root parameter' do - is_expected.not_to have_key 'param_1' - end - - it 'adds is_array option to the nested param' do - expect(parse_request_params['param_1[param_2]']).to eq(type: 'String', is_array: true) - end - context 'and array_use_braces setting set to true' do let(:settings) { { array_use_braces: true } } it 'adds braces to the param key' do - expect(parse_request_params.keys.first).to eq 'param_1[][param_2]' + expect(parse_request_params.keys.last).to eq 'param_1[param_2]' end end end @@ -66,19 +58,11 @@ context 'when param is nested in a param of hash type' do let(:params) { [['param_1', { type: 'Hash' }], ['param_1[param_2]', { type: 'String' }]] } - it 'skips root parameter' do - is_expected.not_to have_key 'param_1' - end - - it 'does not change options to the nested param' do - expect(parse_request_params['param_1[param_2]']).to eq(type: 'String') - end - context 'and array_use_braces setting set to true' do let(:settings) { { array_use_braces: true } } it 'does not add braces to the param key' do - expect(parse_request_params.keys.first).to eq 'param_1[param_2]' + expect(parse_request_params.keys.last).to eq 'param_1[param_2]' end end end diff --git a/spec/lib/endpoint_spec.rb b/spec/lib/endpoint_spec.rb index 6347297c..689127f8 100644 --- a/spec/lib/endpoint_spec.rb +++ b/spec/lib/endpoint_spec.rb @@ -109,7 +109,7 @@ ['id', { required: true, type: 'String' }], ['description', { required: false, type: 'String' }], ['stuffs', { required: true, type: 'Array', is_array: true }], - ['stuffs[id]', { required: true, type: 'String', is_array: true }] + ['stuffs[id]', { required: true, type: 'String' }] ] end @@ -138,8 +138,8 @@ ['stuffs', { required: true, type: 'Array', is_array: true }], ['stuffs[owners]', { required: true, type: 'Array', is_array: true }], ['stuffs[creators]', { required: true, type: 'Array', is_array: true }], - ['stuffs[owners][id]', { required: true, type: 'String', is_array: true }], - ['stuffs[creators][id]', { required: true, type: 'String', is_array: true }], + ['stuffs[owners][id]', { required: true, type: 'String' }], + ['stuffs[creators][id]', { required: true, type: 'String' }], ['stuffs_and_things', { required: true, type: 'String' }] ] end From 963e36564a7387af8a07a525e5cfb63de352cff7 Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 09:57:41 +0200 Subject: [PATCH 12/18] array of array to array of string --- lib/grape-swagger/endpoint.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 866e06bf..ec270363 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -221,7 +221,8 @@ def add_braces(parameter, related_parameters) def add_array(parameter, related_parameters) related_parameters.each do |p| - p[:items] = { type: p[:type], format: p[:format], enum: p[:enum], is_array: p[:is_array] } + p_type = p[:type] == 'array' ? 'string' : p[:type] + p[:items] = { type: p_type, format: p[:format], enum: p[:enum], is_array: p[:is_array] } p[:items].delete_if { |_k, v| v.nil? } p[:type] = 'array' p[:is_array] = parameter[:is_array] From 1582de00d55545fde71aba28a60da2f18182d555 Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 10:06:32 +0200 Subject: [PATCH 13/18] move format data --- lib/grape-swagger/doc_methods.rb | 1 + lib/grape-swagger/doc_methods/format_data.rb | 50 ++++++++++++++++++++ lib/grape-swagger/endpoint.rb | 39 +-------------- 3 files changed, 52 insertions(+), 38 deletions(-) create mode 100644 lib/grape-swagger/doc_methods/format_data.rb diff --git a/lib/grape-swagger/doc_methods.rb b/lib/grape-swagger/doc_methods.rb index 326a1ea1..ac3c6138 100644 --- a/lib/grape-swagger/doc_methods.rb +++ b/lib/grape-swagger/doc_methods.rb @@ -4,6 +4,7 @@ require 'grape-swagger/doc_methods/produces_consumes' require 'grape-swagger/doc_methods/data_type' require 'grape-swagger/doc_methods/extensions' +require 'grape-swagger/doc_methods/format_data' require 'grape-swagger/doc_methods/operation_id' require 'grape-swagger/doc_methods/optional_object' require 'grape-swagger/doc_methods/path_string' diff --git a/lib/grape-swagger/doc_methods/format_data.rb b/lib/grape-swagger/doc_methods/format_data.rb new file mode 100644 index 00000000..84b8e71f --- /dev/null +++ b/lib/grape-swagger/doc_methods/format_data.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module GrapeSwagger + module DocMethods + class FormatData + class << self + def to_format(parameters) + parameters.reject { |parameter| parameter[:in] == 'body' }.each do |b| + related_parameters = parameters.select do |p| + p[:name] != b[:name] && p[:name].to_s.include?(b[:name].to_s.gsub(/\[\]\z/, '') + '[') + end + parameters.reject! { |p| p[:name] == b[:name] } if move_down(b, related_parameters) + end + end + + def move_down(parameter, related_parameters) + case parameter[:type] + when 'array' + add_array(parameter, related_parameters) + unless related_parameters.blank? + add_braces(parameter, related_parameters) if parameter[:name].match?(/\A.*\[\]\z/) + return true + end + when 'object' + return true + end + false + end + + def add_braces(parameter, related_parameters) + param_name = parameter[:name].gsub(/\A(.*)\[\]\z/, '\1') + related_parameters.each { |p| p[:name] = p[:name].gsub(param_name, param_name + '[]') } + end + + def add_array(parameter, related_parameters) + related_parameters.each do |p| + p_type = p[:type] == 'array' ? 'string' : p[:type] + p[:items] = { type: p_type, format: p[:format], enum: p[:enum], is_array: p[:is_array] } + p[:items].delete_if { |_k, v| v.nil? } + p[:type] = 'array' + p[:is_array] = parameter[:is_array] + p.delete(:format) + p.delete(:enum) + p.delete_if { |_k, v| v.nil? } + end + end + end + end + end +end diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index ec270363..be392419 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -190,48 +190,11 @@ def params_object(route, options, path) parameters = GrapeSwagger::DocMethods::MoveParams.to_definition(path, parameters, route, @definitions) end - parameters.reject { |parameter| parameter[:in] == 'body' }.each do |b| - related_parameters = parameters.select do |p| - p[:name] != b[:name] && p[:name].to_s.include?(b[:name].to_s.gsub(/\[\]\z/, '') + '[') - end - parameters.reject! { |p| p[:name] == b[:name] } if format_data(b, related_parameters) - end + GrapeSwagger::DocMethods::FormatData.to_format(parameters) parameters.presence end - def format_data(parameter, related_parameters) - case parameter[:type] - when 'array' - add_array(parameter, related_parameters) - unless related_parameters.blank? - add_braces(parameter, related_parameters) if parameter[:name].match?(/\A.*\[\]\z/) - return true - end - when 'object' - return true - end - false - end - - def add_braces(parameter, related_parameters) - param_name = parameter[:name].gsub(/\A(.*)\[\]\z/, '\1') - related_parameters.each { |p| p[:name] = p[:name].gsub(param_name, param_name + '[]') } - end - - def add_array(parameter, related_parameters) - related_parameters.each do |p| - p_type = p[:type] == 'array' ? 'string' : p[:type] - p[:items] = { type: p_type, format: p[:format], enum: p[:enum], is_array: p[:is_array] } - p[:items].delete_if { |_k, v| v.nil? } - p[:type] = 'array' - p[:is_array] = parameter[:is_array] - p.delete(:format) - p.delete(:enum) - p.delete_if { |_k, v| v.nil? } - end - end - def response_object(route) codes = http_codes_from_route(route) codes.map! { |x| x.is_a?(Array) ? { code: x[0], message: x[1], model: x[2], examples: x[3], headers: x[4] } : x } From 7a799b889cea14d32f5f3d1785fcc7db96a681b6 Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 11:21:31 +0200 Subject: [PATCH 14/18] fix formdata return --- lib/grape-swagger/doc_methods/format_data.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/grape-swagger/doc_methods/format_data.rb b/lib/grape-swagger/doc_methods/format_data.rb index 84b8e71f..3ad2980f 100644 --- a/lib/grape-swagger/doc_methods/format_data.rb +++ b/lib/grape-swagger/doc_methods/format_data.rb @@ -11,6 +11,7 @@ def to_format(parameters) end parameters.reject! { |p| p[:name] == b[:name] } if move_down(b, related_parameters) end + parameters end def move_down(parameter, related_parameters) From 0c3dcdcf383a29d49dd9f688fea37b3df329f5b3 Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 11:29:45 +0200 Subject: [PATCH 15/18] Moved params_parser role spec --- spec/lib/format_data_spec.rb | 39 ++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 spec/lib/format_data_spec.rb diff --git a/spec/lib/format_data_spec.rb b/spec/lib/format_data_spec.rb new file mode 100644 index 00000000..d196f5b8 --- /dev/null +++ b/spec/lib/format_data_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GrapeSwagger::DocMethods::FormatData do + let(:subject) { GrapeSwagger::DocMethods::FormatData } + + [true, false].each do |array_use_braces| + context 'when param is nested in a param of array type' do + let(:braces) { array_use_braces ? '[]' : '' } + let(:params) do + [ + { in: 'formData', name: "param1#{braces}", type: 'array', items: { type: 'string' } }, + { in: 'formData', name: 'param1[param2]', type: 'string' } + ] + end + + it 'skips root parameter' do + expect(subject.to_format(params).first).not_to have_key "param1#{braces}" + end + + it 'Move array type to param2' do + expect(subject.to_format(params).first).to include(name: "param1#{braces}[param2]", type: 'array') + end + end + end + + context 'when param is nested in a param of hash type' do + let(:params) { [{ in: 'formData', type: 'object', name: 'param1' }, { in: 'formData', name: 'param1[param2]', type: 'string' }] } + + it 'skips root parameter' do + expect(subject.to_format(params).first).not_to have_key 'param1' + end + + it 'Move array type to param2' do + expect(subject.to_format(params).first).to include(name: 'param1[param2]', type: 'string') + end + end +end From eb7654c5408f3a3a79b38e8f5a27b913981ec1ff Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 11:53:09 +0200 Subject: [PATCH 16/18] Move endpoint spec --- spec/lib/format_data_spec.rb | 52 ++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/spec/lib/format_data_spec.rb b/spec/lib/format_data_spec.rb index d196f5b8..d4779726 100644 --- a/spec/lib/format_data_spec.rb +++ b/spec/lib/format_data_spec.rb @@ -36,4 +36,56 @@ expect(subject.to_format(params).first).to include(name: 'param1[param2]', type: 'string') end end + + context 'when params contain a complex array' do + let(:params) do + [ + { name: 'id', required: true, type: 'string' }, + { name: 'description', required: false, type: 'string' }, + { name: 'stuffs', required: true, type: 'array' }, + { name: 'stuffs[id]', required: true, type: 'string' } + ] + end + + let(:expected_params) do + [ + { name: 'id', required: true, type: 'string' }, + { name: 'description', required: false, type: 'string' }, + { name: 'stuffs[id]', required: true, type: 'array', items: { type: 'string' } } + ] + end + + it 'parses params correctly and adds array type all concerned elements' do + expect(subject.to_format(params)).to eq expected_params + end + + context 'when array params are not contiguous with parent array' do + let(:params) do + [ + { name: 'id', required: true, type: 'string' }, + { name: 'description', required: false, type: 'string' }, + { name: 'stuffs', required: true, type: 'array' }, + { name: 'stuffs[owners]', required: true, type: 'array' }, + { name: 'stuffs[creators]', required: true, type: 'array' }, + { name: 'stuffs[owners][id]', required: true, type: 'string' }, + { name: 'stuffs[creators][id]', required: true, type: 'string' }, + { name: 'stuffs_and_things', required: true, type: 'string' } + ] + end + + let(:expected_params) do + [ + { name: 'id', required: true, type: 'string' }, + { name: 'description', required: false, type: 'string' }, + { name: 'stuffs[owners][id]', required: true, type: 'array', items: { type: 'string' } }, + { name: 'stuffs[creators][id]', required: true, type: 'array', items: { type: 'string' } }, + { name: 'stuffs_and_things', required: true, type: 'string' } + ] + end + + it 'parses params correctly and adds array type all concerned elements' do + expect(subject.to_format(params)).to eq expected_params + end + end + end end From 9ac60a36ba11dc5ab8c8161a2e1c7fac02ed6e2e Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 11:54:07 +0200 Subject: [PATCH 17/18] Revert "Add vehicle test" This reverts commit e5ba63ba26df6df72c0ef4ec07b2bd8499a9576d. --- spec/issues/751_deeply_nested_objects_spec.rb | 94 ------------------- 1 file changed, 94 deletions(-) diff --git a/spec/issues/751_deeply_nested_objects_spec.rb b/spec/issues/751_deeply_nested_objects_spec.rb index 191acbbc..c674438f 100644 --- a/spec/issues/751_deeply_nested_objects_spec.rb +++ b/spec/issues/751_deeply_nested_objects_spec.rb @@ -43,28 +43,6 @@ def self.vrp_request_service(this) end this.mutually_exclusive :activity, :activities end - - def self.vrp_request_vehicle(this) - this.requires(:id, type: String, allow_blank: false) - this.optional(:cost_time_multiplier, type: Float) - - this.optional(:matrix_id, type: String) - this.optional(:router_mode, type: String) - this.exactly_one_of :matrix_id, :router_mode - - this.optional(:skills, type: Array[Array[String]]) - - this.optional(:start_point_id, type: String) - this.optional(:end_point_id, type: String) - - this.optional(:sequence_timewindows, type: Array) do - Vrp.vrp_request_timewindow(self) - end - this.optional(:timewindow, type: Hash) do - Vrp.vrp_request_timewindow(self) - end - this.mutually_exclusive :sequence_timewindows, :timewindow - end end namespace :vrp do @@ -79,10 +57,6 @@ def self.vrp_request_vehicle(this) optional(:services, type: Array) do Vrp.vrp_request_service(self) end - - optional(:vehicles, type: Array) do - Vrp.vrp_request_vehicle(self) - end end end post do @@ -213,72 +187,4 @@ def self.vrp_request_vehicle(this) ) end end - - describe 'Correctness of vrp Vehicles' do - let(:get_points_response) { subject['definitions']['postVrpSubmit']['properties']['vrp']['properties']['vehicles'] } - specify do - expect(get_points_response).to eql( - 'type' => 'array', - 'items' => { - 'type' => 'object', - 'properties' => { - 'id' => { - 'type' => 'string' - }, - 'cost_time_multiplier' => { - 'type' => 'float', - 'format' => 'number' - }, - 'matrix_id' => { - 'type' => 'string' - }, - 'router_mode' => { - 'type' => 'string' - }, - 'skills' => { - 'type' => 'array', - 'items' => { - 'type' => 'array', - 'items' => { - 'type' => 'string' - } - } - }, - 'start_point_id' => { - 'type' => 'string' - }, - 'end_point_id' => { - 'type' => 'string' - }, - 'timewindow' => { - 'type' => 'object', - 'properties' => { - 'start' => { - 'type' => 'string' - }, - 'end' => { - 'type' => 'string' - } - } - }, - 'sequence_timewindows' => { - 'type' => 'array', - 'items' => { - 'type' => 'object', - 'properties' => { - 'start' => { - 'type' => 'string' - }, - 'end' => { - 'type' => 'string' - } - } - } - } - }, - 'required' => ['id'] - } - ) - end - end end From e8ccbc0f754b7b755e619202bbbad323216b0bb9 Mon Sep 17 00:00:00 2001 From: Braktar Date: Mon, 5 Aug 2019 12:07:36 +0200 Subject: [PATCH 18/18] remove redundant interpolation --- lib/grape-swagger.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index bb7d5e6a..c84c5062 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -26,7 +26,7 @@ module SwaggerRouting def combine_routes(app, doc_klass) app.routes.each do |route| route_path = route.path - route_match = route_path.split(/^.*?#{route.prefix.to_s}/).last + route_match = route_path.split(/^.*?#{route.prefix}/).last next unless route_match route_match = route_match.match('\/([\w|-]*?)[\.\/\(]') || route_match.match('\/([\w|-]*)$')