Skip to content

Commit 15ee81c

Browse files
committed
Fix incorrect optional validation issues for MultipleParamsBase
Theese have previously been fixed for the Base class
1 parent d9d99ea commit 15ee81c

File tree

6 files changed

+98
-13
lines changed

6 files changed

+98
-13
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#### Fixes
88

99
* Your contribution here.
10+
* [#2129](https://github.com/ruby-grape/grape/pull/2129): Fix validation error when Required Array nested inside an optional array, for Multiparam validators - [@dwhenry](https://github.com/dwhenry).
1011
* [#2128](https://github.com/ruby-grape/grape/pull/2128): Fix validation error when Required Array nested inside an optional array - [@dwhenry](https://github.com/dwhenry).
1112
* [#2127](https://github.com/ruby-grape/grape/pull/2127): Fix a performance issue with dependent params - [@dnesteryuk](https://github.com/dnesteryuk).
1213
* [#2126](https://github.com/ruby-grape/grape/pull/2126): Fix warnings about redefined attribute accessors in `AttributeTranslator` - [@samsonjs](https://github.com/samsonjs).

lib/grape/validations/attributes_iterator.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,14 @@ def do_each(params_to_process, parent_indicies = [], &block)
4848
def yield_attributes(_resource_params, _attrs)
4949
raise NotImplementedError
5050
end
51+
52+
# This is a special case so that we can ignore tree's where option
53+
# values are missing lower down. Unfortunately we can remove this
54+
# are the parameter parsing stage as they are required to ensure
55+
# the correct indexing is maintained
56+
def skip?(val)
57+
val == Grape::DSL::Parameters::EmptyOptionalValue
58+
end
5159
end
5260
end
5361
end

lib/grape/validations/multiple_attributes_iterator.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ class MultipleAttributesIterator < AttributesIterator
66
private
77

88
def yield_attributes(resource_params, _attrs)
9-
yield resource_params
9+
yield resource_params, skip?(resource_params)
1010
end
1111
end
1212
end

lib/grape/validations/single_attribute_iterator.rb

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,6 @@ def yield_attributes(val, attrs)
1111
end
1212
end
1313

14-
15-
# This is a special case so that we can ignore tree's where option
16-
# values are missing lower down. Unfortunately we can remove this
17-
# are the parameter parsing stage as they are required to ensure
18-
# the correct indexing is maintained
19-
def skip?(val)
20-
val == Grape::DSL::Parameters::EmptyOptionalValue
21-
end
22-
2314
# Primitives like Integers and Booleans don't respond to +empty?+.
2415
# It could be possible to use +blank?+ instead, but
2516
#

lib/grape/validations/validators/multiple_params_base.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ def validate!(params)
77
attributes = MultipleAttributesIterator.new(self, @scope, params)
88
array_errors = []
99

10-
attributes.each do |resource_params|
10+
attributes.each do |resource_params, skip_value|
11+
next if skip_value
1112
begin
1213
validate_params!(resource_params)
1314
rescue Grape::Exceptions::Validation => e

spec/grape/validations_spec.rb

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,7 +1023,7 @@ def validate_param!(attr_name, params)
10231023
end
10241024

10251025
it "with valid data" do
1026-
data_without_errors = {
1026+
data = {
10271027
top: [
10281028
{ top_id: 1, middle_1: [
10291029
{middle_1_id: 11}, {middle_1_id: 12, middle_2: [
@@ -1037,7 +1037,7 @@ def validate_param!(attr_name, params)
10371037
]
10381038
}
10391039

1040-
get '/multi_level', data_without_errors
1040+
get '/multi_level', data
10411041
expect(last_response.body).to eq("multi_level works!")
10421042
expect(last_response.status).to eq(200)
10431043
end
@@ -1067,6 +1067,90 @@ def validate_param!(attr_name, params)
10671067
end
10681068
end
10691069
end
1070+
1071+
it "exactly_one_of" do
1072+
subject.params do
1073+
requires :orders, type: Array do
1074+
requires :id, type: Integer
1075+
optional :drugs, type: Hash do
1076+
optional :batch_no, type: String
1077+
optional :batch_id, type: String
1078+
exactly_one_of :batch_no, :batch_id
1079+
end
1080+
end
1081+
end
1082+
1083+
subject.get '/exactly_one_of' do
1084+
'exactly_one_of works!'
1085+
end
1086+
1087+
data = {
1088+
orders: [
1089+
{ id: 77, drugs: {batch_no: "A1234567"}},
1090+
{ id: 70 }
1091+
]
1092+
}
1093+
1094+
get '/exactly_one_of', data
1095+
expect(last_response.body).to eq("exactly_one_of works!")
1096+
expect(last_response.status).to eq(200)
1097+
end
1098+
1099+
it "at_least_one_of" do
1100+
subject.params do
1101+
requires :orders, type: Array do
1102+
requires :id, type: Integer
1103+
optional :drugs, type: Hash do
1104+
optional :batch_no, type: String
1105+
optional :batch_id, type: String
1106+
at_least_one_of :batch_no, :batch_id
1107+
end
1108+
end
1109+
end
1110+
1111+
subject.get '/at_least_one_of' do
1112+
'at_least_one_of works!'
1113+
end
1114+
1115+
data = {
1116+
orders: [
1117+
{ id: 77, drugs: {batch_no: "A1234567"}},
1118+
{ id: 70 }
1119+
]
1120+
}
1121+
1122+
get '/at_least_one_of', data
1123+
expect(last_response.body).to eq("at_least_one_of works!")
1124+
expect(last_response.status).to eq(200)
1125+
end
1126+
1127+
it "all_or_none_of" do
1128+
subject.params do
1129+
requires :orders, type: Array do
1130+
requires :id, type: Integer
1131+
optional :drugs, type: Hash do
1132+
optional :batch_no, type: String
1133+
optional :batch_id, type: String
1134+
all_or_none_of :batch_no, :batch_id
1135+
end
1136+
end
1137+
end
1138+
1139+
subject.get '/all_or_none_of' do
1140+
'all_or_none_of works!'
1141+
end
1142+
1143+
data = {
1144+
orders: [
1145+
{ id: 77, drugs: {batch_no: "A1234567", batch_id: "12"}},
1146+
{ id: 70 }
1147+
]
1148+
}
1149+
1150+
get '/all_or_none_of', data
1151+
expect(last_response.body).to eq("all_or_none_of works!")
1152+
expect(last_response.status).to eq(200)
1153+
end
10701154
end
10711155

10721156
context 'multiple validation errors' do

0 commit comments

Comments
 (0)