Increase test coverage around nexted arrays/hashes with optional comp…

…onents

This adds a number of tests around edge cases and different structures
that could result in the previous validation incorrectly reporting
missing data as a result of an optional element not being present.

lib/grape/validations/single_attribute_iterator.rb

@@ -17,7 +17,6 @@ def yield_attributes(val, attrs)
  # are the parameter parsing stage as they are required to ensure
  # the correct indexing is maintained
  def skip?(val)
- # return false
  val == Grape::DSL::Parameters::EmptyOptionalValue
  end
 

spec/grape/validations_spec.rb

@@ -884,32 +884,188 @@ def validate_param!(attr_name, params)
  expect(declared_params).to eq([items: [:key, { optional_subitems: [:value] }, { required_subitems: [:value] }]])
  end
 
- it "does not report errors when required array inside missing optional array" do
- subject.params do
- requires :orders, type: Array do
- requires :id, type: Integer
- optional :drugs, type: Array do
- requires :batches, type: Array do
+ context <<~DESC do
+ Issue occurs whenever:
+ * param structure with at least three levels
+ * 1st level item is a required Array that has >1 entry with an optional item present and >1 entry with an optional item missing 
+ * 2nd level is an optional Array or Hash 
+ * 3rd level is a required item (can be any type)
+ * additional levels do not effect the issue from occuring
+ DESC
+
+ it "example based off actual real world use case" do
+ subject.params do
+ requires :orders, type: Array do
+ requires :id, type: Integer
+ optional :drugs, type: Array do
+ requires :batches, type: Array do
+ requires :batch_no, type: String
+ end
+ end
+ end
+ end
+
+ subject.get '/validate_required_arrays_under_optional_arrays' do
+ 'validate_required_arrays_under_optional_arrays works!'
+ end
+
+ data = {
+ orders: [
+ { id: 77, drugs: [{batches: [{batch_no: "A1234567"}]}]},
+ { id: 70 }
+ ]
+ }
+
+ get '/validate_required_arrays_under_optional_arrays', data
+ expect(last_response.body).to eq("validate_required_arrays_under_optional_arrays works!")
+ expect(last_response.status).to eq(200)
+ end
+
+ it "simplest example using Arry -> Array -> Hash -> String" do
+ subject.params do
+ requires :orders, type: Array do
+ requires :id, type: Integer
+ optional :drugs, type: Array do
+ requires :batch_no, type: String
+ end
+ end
+ end
+
+ subject.get '/validate_required_arrays_under_optional_arrays' do
+ 'validate_required_arrays_under_optional_arrays works!'
+ end
+
+ data = {
+ orders: [
+ { id: 77, drugs: [{batch_no: "A1234567"}]},
+ { id: 70 }
+ ]
+ }
+
+ get '/validate_required_arrays_under_optional_arrays', data
+ expect(last_response.body).to eq("validate_required_arrays_under_optional_arrays works!")
+ expect(last_response.status).to eq(200)
+ end
+
+ it "simplest example using Arry -> Hash -> String" do
+ subject.params do
+ requires :orders, type: Array do
+ requires :id, type: Integer
+ optional :drugs, type: Hash do
  requires :batch_no, type: String
  end
  end
  end
+
+ subject.get '/validate_required_arrays_under_optional_arrays' do
+ 'validate_required_arrays_under_optional_arrays works!'
+ end
+
+ data = {
+ orders: [
+ { id: 77, drugs: {batch_no: "A1234567"}},
+ { id: 70 }
+ ]
+ }
+
+ get '/validate_required_arrays_under_optional_arrays', data
+ expect(last_response.body).to eq("validate_required_arrays_under_optional_arrays works!")
+ expect(last_response.status).to eq(200)
  end
 
- subject.get '/validate_required_arrays_under_optional_arrays' do
- 'validate_required_arrays_under_optional_arrays works!'
+ it "correctly indexes invalida data" do
+ subject.params do
+ requires :orders, type: Array do
+ requires :id, type: Integer
+ optional :drugs, type: Array do
+ requires :batch_no, type: String
+ requires :quantity, type: Integer
+ end
+ end
+ end
+
+ subject.get '/correctly_indexes' do
+ 'correctly_indexes works!'
+ end
+
+ data = {
+ orders: [
+ { id: 70 },
+ { id: 77, drugs: [{batch_no: "A1234567", quantity: 12}, {batch_no: "B222222"}]}
+ ]
+ }
+
+ get '/correctly_indexes', data
+ expect(last_response.body).to eq("orders[1][drugs][1][quantity] is missing")
+ expect(last_response.status).to eq(400)
  end
 
- data = {
- orders: [
- { id: 77, drugs: [{batches: [{batch_no: "A1234567"}]}]},
- { id: 70 }
- ]
- }
+ context "multiple levels of optional and requires settings" do
+ before do
+ subject.params do
+ requires :top, type: Array do
+ requires :top_id, type: Integer, allow_blank: false
+ optional :middle_1, type: Array do
+ requires :middle_1_id, type: Integer, allow_blank: false
+ optional :middle_2, type: Array do
+ requires :middle_2_id, type: String, allow_blank: false
+ optional :bottom, type: Array do
+ requires :bottom_id, type: Integer, allow_blank: false
+ end
+ end
+ end
+ end
+ end
 
- get '/validate_required_arrays_under_optional_arrays', data
- expect(last_response.body).to eq("validate_required_arrays_under_optional_arrays works!")
- expect(last_response.status).to eq(200)
+ subject.get '/multi_level' do
+ 'multi_level works!'
+ end
+ end
+
+ it "with valid data" do
+ data_without_errors = {
+ top: [
+ { top_id: 1, middle_1: [
+ {middle_1_id: 11}, {middle_1_id: 12, middle_2: [
+ {middle_2_id: 121}, {middle_2_id: 122, bottom: [{bottom_id: 1221}]}]}]},
+ { top_id: 2, middle_1: [
+ {middle_1_id: 21}, {middle_1_id: 22, middle_2: [
+ {middle_2_id: 221}]}]},
+ { top_id: 3, middle_1: [
+ {middle_1_id: 31}, {middle_1_id: 32}]},
+ { top_id: 4 }
+ ]
+ }
+
+ get '/multi_level', data_without_errors
+ expect(last_response.body).to eq("multi_level works!")
+ expect(last_response.status).to eq(200)
+ end
+
+ it "with invalid data" do
+ data = {
+ top: [
+ { top_id: 1, middle_1: [
+ {middle_1_id: 11}, {middle_1_id: 12, middle_2: [
+ {middle_2_id: 121}, {middle_2_id: 122, bottom: [{bottom_id: nil}]}]}]},
+ { top_id: 2, middle_1: [
+ {middle_1_id: 21}, {middle_1_id: 22, middle_2: [{middle_2_id: nil}]}]},
+ { top_id: 3, middle_1: [
+ {middle_1_id: nil}, {middle_1_id: 32}]},
+ { top_id: nil, missing_top_id: 4 }
+ ]
+ }
+ # debugger
+ get '/multi_level', data
+ expect(last_response.body.split(", ")).to match_array([
+ "top[3][top_id] is empty",
+ "top[2][middle_1][0][middle_1_id] is empty",
+ "top[1][middle_1][1][middle_2][0][middle_2_id] is empty",
+ "top[0][middle_1][1][middle_2][1][bottom][0][bottom_id] is empty"
+ ])
+ expect(last_response.status).to eq(400)
+ end
+ end
  end
  end