Skip to content

Commit 315f6e8

Browse files
committed
Do not try to coerce "" for numeric & date params
When using a GET form for searching or filtering and a field is left blank, Rails converts '?foo=' to the params hash { 'foo' => '' }, and this raised an unexpected InvalidParameterError when we have defined `param! :foo, Integer` in the controller. Now we will treat that empty string as an un-supplied param and only reject it if the param's required option is enabled. - added shared examples for Float tests - nested shared examples in the appropriate scope
1 parent 15d3cc8 commit 315f6e8

File tree

9 files changed

+130
-22
lines changed

9 files changed

+130
-22
lines changed

lib/rails_param/coercion/big_decimal_param.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ class BigDecimalParam < VirtualParam
44
DEFAULT_PRECISION = 14
55

66
def coerce
7+
return nil if param == '' # e.g. from an empty field in a GET form
8+
79
stripped_param = if param.is_a?(String)
810
param.delete('$,').strip.to_f
911
else

lib/rails_param/coercion/float_param.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ module RailsParam
22
class Coercion
33
class FloatParam < VirtualParam
44
def coerce
5+
return nil if param == '' # e.g. from an empty field in a GET form
6+
57
Float(param)
68
end
79
end

lib/rails_param/coercion/integer_param.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ module RailsParam
22
class Coercion
33
class IntegerParam < VirtualParam
44
def coerce
5+
return nil if param == '' # e.g. from an empty field in a GET form
6+
57
Integer(param)
68
end
79
end

lib/rails_param/coercion/time_param.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ module RailsParam
22
class Coercion
33
class TimeParam < VirtualParam
44
def coerce
5+
return nil if param == '' # e.g. from an empty field in a GET form
6+
57
return type.strptime(param, options[:format]) if options[:format].present?
68

79
type.parse(param)

spec/rails_param/coercion/big_decimal_param_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@
1313
end
1414
end
1515

16+
shared_examples "returns nil" do
17+
it "returns the param as a nil value" do
18+
expect(subject.coerce).to be nil
19+
end
20+
end
21+
1622
it_behaves_like "returns BigDecimal with default precision"
1723

1824
context "given a precision option" do
@@ -35,6 +41,12 @@
3541
expect(subject.coerce).to eq 1.50
3642
end
3743
end
44+
45+
context "param is blank (e.g. empty field in a GET form)" do
46+
let(:param) { "" }
47+
48+
it_behaves_like "returns nil"
49+
end
3850
end
3951
end
4052
end

spec/rails_param/coercion/float_param_spec.rb

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,48 @@
66
let(:options) { {} }
77
subject { described_class.new(param: param, type: type, options: options) }
88

9-
context "value is valid" do
10-
let(:param) { "12.34" }
9+
shared_examples "does not raise an error" do
10+
it "does not raise an error" do
11+
expect { subject.coerce }.to_not raise_error
12+
end
13+
end
14+
15+
shared_examples "raises ArgumentError" do
16+
it "raises ArgumentError" do
17+
expect { subject.coerce }.to raise_error ArgumentError
18+
end
19+
end
1120

12-
it "returns a Float" do
21+
shared_examples "returns a Float" do
22+
it "returns the param as a Float" do
1323
expect(subject.coerce).to eq 12.34
1424
end
1525
end
1626

27+
shared_examples "returns nil" do
28+
it "returns the param as a nil value" do
29+
expect(subject.coerce).to be nil
30+
end
31+
end
32+
33+
context "value is valid" do
34+
let(:param) { "12.34" }
35+
36+
it_behaves_like "does not raise an error"
37+
it_behaves_like "returns a Float"
38+
end
39+
40+
context "param is blank (e.g. empty field in a GET form)" do
41+
let(:param) { "" }
42+
43+
it_behaves_like "does not raise an error"
44+
it_behaves_like "returns nil"
45+
end
46+
1747
context "value is invalid" do
1848
let(:param) { "foo" }
1949

20-
it "raises ArgumentError" do
21-
expect { subject.coerce }.to raise_error ArgumentError
22-
end
50+
it_behaves_like "raises ArgumentError"
2351
end
2452
end
2553
end

spec/rails_param/coercion/integer_param_spec.rb

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,34 @@
11
require 'spec_helper'
22

33
describe RailsParam::Coercion::IntegerParam do
4-
shared_examples "does not raise an error" do
5-
it "does not raise an error" do
6-
expect { subject.coerce }.to_not raise_error
4+
describe "#coerce" do
5+
let(:type) { Integer }
6+
let(:options) { {} }
7+
subject { described_class.new(param: param, type: type, options: options) }
8+
9+
shared_examples "does not raise an error" do
10+
it "does not raise an error" do
11+
expect { subject.coerce }.to_not raise_error
12+
end
713
end
8-
end
914

10-
shared_examples "raises ArgumentError" do
11-
it "raises ArgumentError" do
12-
expect { subject.coerce }.to raise_error ArgumentError
15+
shared_examples "raises ArgumentError" do
16+
it "raises ArgumentError" do
17+
expect { subject.coerce }.to raise_error ArgumentError
18+
end
1319
end
14-
end
1520

16-
shared_examples "returns an Integer" do
17-
it "returns the param as an Integer" do
18-
expect(subject.coerce).to eq 19
21+
shared_examples "returns an Integer" do
22+
it "returns the param as an Integer" do
23+
expect(subject.coerce).to eq 19
24+
end
1925
end
20-
end
2126

22-
describe "#coerce" do
23-
let(:type) { Integer }
24-
let(:options) { {} }
25-
subject { described_class.new(param: param, type: type, options: options) }
27+
shared_examples "returns nil" do
28+
it "returns the param as a nil value" do
29+
expect(subject.coerce).to be nil
30+
end
31+
end
2632

2733
context "param is a valid value" do
2834
let(:param) { "19" }
@@ -31,6 +37,13 @@
3137
it_behaves_like "returns an Integer"
3238
end
3339

40+
context "param is blank (e.g. empty field in a GET form)" do
41+
let(:param) { "" }
42+
43+
it_behaves_like "does not raise an error"
44+
it_behaves_like "returns nil"
45+
end
46+
3447
context "param is invalid value" do
3548
let(:param) { "notInteger" }
3649

spec/rails_param/coercion/time_param_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@
5656
end
5757
end
5858

59+
shared_examples "returns nil" do
60+
it "returns the param as a nil value" do
61+
expect(subject.coerce).to be nil
62+
end
63+
end
64+
5965
let(:options) { {} }
6066
subject { described_class.new(param: param, type: type, options: options) }
6167

@@ -195,5 +201,27 @@
195201
end
196202
end
197203
end
204+
205+
context "param is blank (e.g. empty field in a GET form)" do
206+
let(:param) { "" }
207+
208+
context "type is Date" do
209+
let(:type) { Date }
210+
211+
it_behaves_like "returns nil"
212+
end
213+
214+
context "type is Time" do
215+
let(:type) { Time }
216+
217+
it_behaves_like "returns nil"
218+
end
219+
220+
context "type is DateTime" do
221+
let(:type) { DateTime }
222+
223+
it_behaves_like "returns nil"
224+
end
225+
end
198226
end
199227
end

spec/rails_param/param_spec.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,12 @@ def params;
125125
expect(controller.params["foo"]).to eql(42)
126126
end
127127

128+
it "will allow a nil value (e.g. from an empty field in a GET form)" do
129+
allow(controller).to receive(:params).and_return({ "foo" => "" })
130+
controller.param! :foo, Integer
131+
expect(controller.params["foo"]).to be nil
132+
end
133+
128134
it "will raise InvalidParameterError if the value is not valid" do
129135
allow(controller).to receive(:params).and_return({ "foo" => "notInteger" })
130136
expect { controller.param! :foo, Integer }.to(
@@ -151,6 +157,12 @@ def params;
151157
expect(controller.params["foo"]).to eql(42.22)
152158
end
153159

160+
it "will allow a nil value (e.g. from an empty field in a GET form)" do
161+
allow(controller).to receive(:params).and_return({ "foo" => "" })
162+
controller.param! :foo, Float
163+
expect(controller.params["foo"]).to be nil
164+
end
165+
154166
it "will raise InvalidParameterError if the value is not valid" do
155167
allow(controller).to receive(:params).and_return({ "foo" => "notFloat" })
156168
expect { controller.param! :foo, Float }.to(
@@ -604,6 +616,13 @@ def params;
604616
)
605617
end
606618

619+
it "raises on a nil value (e.g. from an empty field in a GET form)" do
620+
allow(controller).to receive(:params).and_return({ "foo" => "" })
621+
expect { controller.param! :foo, BigDecimal, required: true }.to(
622+
raise_error(RailsParam::InvalidParameterError, "Parameter foo is required")
623+
)
624+
end
625+
607626
it "raises custom message" do
608627
allow(controller).to receive(:params).and_return({})
609628
expect { controller.param! :price, Integer, required: true, message: "No price specified" }.to(

0 commit comments

Comments
 (0)