Fix is_array check for non-contiguous sub and parent entities (#691)

CHANGELOG.md

@@ -7,6 +7,7 @@
 #### Fixes
 
 * [#688](https://github.com/ruby-grape/grape-swagger/pull/688): Use deep merge for nested parameter definitions - [@jdmurphy](https://github.com/jdmurphy).
+* [#691](https://github.com/ruby-grape/grape-swagger/pull/691): Disregard order when parsing request params for arrays - [@jdmurphy](https://github.com/jdmurphy).
 * Your contribution here.
 
 ###  0.30.1 (July 19, 2018)

lib/grape-swagger/endpoint.rb

@@ -306,13 +306,13 @@ def default_type(params)
     end
 
     def parse_request_params(params)
-      array_key = nil
+      array_keys = []
       params.select { |param| public_parameter?(param) }.each_with_object({}) do |param, memo|
         name, options = *param
         param_type = options[:type]
         param_type = param_type.to_s unless param_type.nil?
-        array_key = name.to_s if param_type_is_array?(param_type)
-        options[:is_array] = true if array_key && name.start_with?(array_key)
+        array_keys << name.to_s if param_type_is_array?(param_type)
+        options[:is_array] = true if array_keys.any? { |key| name == key || name.start_with?(key + '[') }
         memo[name] = options unless %w[Hash Array].include?(param_type) && !options.key?(:documentation)
       end
     end

spec/lib/endpoint_spec.rb

@@ -7,19 +7,6 @@
     described_class.new(Grape::Util::InheritableSetting.new, path: '/', method: :get)
   end
 
-  describe '#param_type_is_array?' do
-    it 'returns true if the value passed represents an array' do
-      expect(subject.send(:param_type_is_array?, 'Array')).to be_truthy
-      expect(subject.send(:param_type_is_array?, '[String]')).to be_truthy
-      expect(subject.send(:param_type_is_array?, 'Array[Integer]')).to be_truthy
-    end
-
-    it 'returns false if the value passed does not represent an array' do
-      expect(subject.send(:param_type_is_array?, 'String')).to be_falsey
-      expect(subject.send(:param_type_is_array?, '[String, Integer]')).to be_falsey
-    end
-  end
-
   describe '.content_types_for' do
     describe 'defined on target_class' do
       let(:own_json) { 'text/own-json' }
@@ -58,4 +45,121 @@
       end
     end
   end
+
+  describe '#param_type_is_array?' do
+    it 'returns true if the value passed represents an array' do
+      expect(subject.send(:param_type_is_array?, 'Array')).to be_truthy
+      expect(subject.send(:param_type_is_array?, '[String]')).to be_truthy
+      expect(subject.send(:param_type_is_array?, 'Array[Integer]')).to be_truthy
+    end
+
+    it 'returns false if the value passed does not represent an array' do
+      expect(subject.send(:param_type_is_array?, 'String')).to be_falsey
+      expect(subject.send(:param_type_is_array?, '[String, Integer]')).to be_falsey
+    end
+  end
+
+  describe 'parse_request_params' do
+    before do
+      subject.send(:parse_request_params, params)
+    end
+
+    context 'when params do not contain an array' do
+      let(:params) do
+        [
+          ['id', { required: true, type: 'String' }],
+          ['description', { required: false, type: 'String' }]
+        ]
+      end
+
+      let(:expected_params) do
+        [
+          ['id', { required: true, type: 'String' }],
+          ['description', { required: false, type: 'String' }]
+        ]
+      end
+
+      it 'parses params correctly' do
+        expect(params).to eq expected_params
+      end
+    end
+
+    context 'when params contain a simple array' do
+      let(:params) do
+        [
+          ['id', { required: true, type: 'String' }],
+          ['description', { required: false, type: 'String' }],
+          ['stuffs', { required: true, type: 'Array[String]' }]
+        ]
+      end
+
+      let(:expected_params) do
+        [
+          ['id', { required: true, type: 'String' }],
+          ['description', { required: false, type: 'String' }],
+          ['stuffs', { required: true, type: 'Array[String]', is_array: true }]
+        ]
+      end
+
+      it 'parses params correctly and adds is_array to the array' do
+        expect(params).to eq expected_params
+      end
+    end
+
+    context 'when params contain a complex array' do
+      let(:params) do
+        [
+          ['id', { required: true, type: 'String' }],
+          ['description', { required: false, type: 'String' }],
+          ['stuffs', { required: true, type: 'Array' }],
+          ['stuffs[id]', { required: true, type: 'String' }]
+        ]
+      end
+
+      let(:expected_params) do
+        [
+          ['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 }]
+        ]
+      end
+
+      it 'parses params correctly and adds is_array to the array and all elements' do
+        expect(params).to eq expected_params
+      end
+
+      context 'when array params are not contiguous with parent array' do
+        let(:params) do
+          [
+            ['id', { required: true, type: 'String' }],
+            ['description', { required: false, type: 'String' }],
+            ['stuffs', { required: true, type: 'Array' }],
+            ['stuffs[owners]', { required: true, type: 'Array' }],
+            ['stuffs[creators]', { required: true, type: 'Array' }],
+            ['stuffs[owners][id]', { required: true, type: 'String' }],
+            ['stuffs[creators][id]', { required: true, type: 'String' }],
+            ['stuffs_and_things', { required: true, type: 'String' }]
+          ]
+        end
+
+        let(:expected_params) do
+          [
+            ['id', { required: true, type: 'String' }],
+            ['description', { required: false, type: 'String' }],
+            ['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_and_things', { required: true, type: 'String' }]
+          ]
+        end
+
+        it 'parses params correctly and adds is_array to the array and all elements' do
+          expect(params).to eq expected_params
+        end
+      end
+    end
+  end
 end