diff options
author | John Keiser <john@johnkeiser.com> | 2015-06-04 10:11:25 -0700 |
---|---|---|
committer | John Keiser <john@johnkeiser.com> | 2015-06-23 15:23:02 -0700 |
commit | 6e4783f4e71681c7a6fefedde3e71fd60b1488a3 (patch) | |
tree | 22b8ce7fa912d9e9d3763af56351efe99e0bed2a | |
parent | 1d96b8c7bb96a476625a026bea58dd0f251af716 (diff) | |
download | chef-6e4783f4e71681c7a6fefedde3e71fd60b1488a3.tar.gz |
Don't validate `nil` when setting the value to nil
-rw-r--r-- | lib/chef/mixin/params_validate.rb | 39 | ||||
-rw-r--r-- | spec/unit/property/validation_spec.rb | 52 | ||||
-rw-r--r-- | spec/unit/property_spec.rb | 4 |
3 files changed, 59 insertions, 36 deletions
diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb index 2d382d8381..9cd70eede7 100644 --- a/lib/chef/mixin/params_validate.rb +++ b/lib/chef/mixin/params_validate.rb @@ -87,34 +87,39 @@ class Chef iv_symbol = "@#{symbol.to_s}".to_sym # If the user passed NOT_PASSED, or passed nil, then this is a get. - case value - when NOT_PASSED - is_get = true - value = nil - when nil - is_get = true unless explicitly_allows_nil?(symbol, validation) - end + if value == NOT_PASSED || (value.nil? && !explicitly_allows_nil?(symbol, validation)) - if self.instance_variable_defined?(iv_symbol) && is_get - value = self.instance_variable_get(iv_symbol) - if value.is_a?(DelayedEvaluator) - validate({ symbol => value.call }, { symbol => validation })[symbol] - else - value - end - else - if !value.is_a?(DelayedEvaluator) - value = validate({ symbol => value }, { symbol => validation })[symbol] + # Get the value if there is one + if self.instance_variable_defined?(iv_symbol) + value = self.instance_variable_get(iv_symbol) + if value.is_a?(DelayedEvaluator) + value = validate({ symbol => value.call }, { symbol => validation })[symbol] + end + # Get the default value + else + value = validate({}, { symbol => validation })[symbol] # Handle the case where the "default" was a DelayedEvaluator. In # this case, the block yields an optional parameter of +self+, # which is the equivalent of "new_resource" if value.is_a?(DelayedEvaluator) value = value.call(self) end + + # Defaults are presently "stickily" set on the instance + self.instance_variable_set(iv_symbol, value) end + + # Set the value + else + unless value.is_a?(DelayedEvaluator) + value = validate({ symbol => value }, { symbol => validation })[symbol] + end + self.instance_variable_set(iv_symbol, value) end + + value end private diff --git a/spec/unit/property/validation_spec.rb b/spec/unit/property/validation_spec.rb index b9e9e78d3d..d37d23b437 100644 --- a/spec/unit/property/validation_spec.rb +++ b/spec/unit/property/validation_spec.rb @@ -84,8 +84,8 @@ describe "Chef::Resource.property validation" do failure_values.each do |v| it "value #{v.inspect} is invalid" do expect { resource.x v }.to raise_error Chef::Exceptions::ValidationFailed - # resource.instance_eval { @x = 'default' } - # expect { resource.x v }.to raise_error Chef::Exceptions::ValidationFailed + resource.instance_eval { @x = 'default' } + expect { resource.x v }.to raise_error Chef::Exceptions::ValidationFailed end end getter_values.each do |v| @@ -180,32 +180,37 @@ describe "Chef::Resource.property validation" do context "bare types" do validation_test 'String', [ 'hi' ], - [ 10, nil ] + [ 10 ], + [ nil ] validation_test ':a', [ :a ], - [ :b, nil ] + [ :b ], + [ nil ] validation_test ':a, is: :b', [ :a, :b ], - [ :c, nil ] + [ :c ], + [ nil ] validation_test ':a, is: [ :b, :c ]', [ :a, :b, :c ], - [ :d, nil ] + [ :d ], + [ nil ] validation_test '[ :a, :b ], is: :c', [ :a, :b, :c ], - [ :d, nil ] + [ :d ], + [ nil ] validation_test '[ :a, :b ], is: [ :c, :d ]', [ :a, :b, :c, :d ], - [ :e, nil ] + [ :e ], + [ nil ] validation_test 'nil', [ nil ], - [ :a ], - [] + [ :a ] validation_test '[ nil ]', [ nil ], @@ -213,7 +218,8 @@ describe "Chef::Resource.property validation" do validation_test '[]', [], - [ :a, nil ] + [ :a ], + [ nil ] end # is @@ -221,25 +227,30 @@ describe "Chef::Resource.property validation" do # Class validation_test 'is: String', [ 'a', '' ], - [ nil, :a, 1 ] + [ :a, 1 ], + [ nil ] # Value validation_test 'is: :a', [ :a ], - [ :b, nil ] + [ :b ], + [ nil ] validation_test 'is: [ :a, :b ]', [ :a, :b ], - [ [ :a, :b ], nil ] + [ [ :a, :b ] ], + [ nil ] validation_test 'is: [ [ :a, :b ] ]', [ [ :a, :b ] ], - [ :a, :b, nil ] + [ :a, :b ], + [ nil ] # Regex validation_test 'is: /abc/', [ 'abc', 'wowabcwow' ], - [ '', 'abac', nil ] + [ '', 'abac' ], + [ nil ] # PropertyType # validation_test 'is: PropertyType.new(is: :a)', @@ -253,12 +264,14 @@ describe "Chef::Resource.property validation" do validation_test "is: Globalses.eq(10)", [ 10 ], - [ 1, nil ] + [ 1 ], + [ nil ] # Proc validation_test 'is: proc { |x| x }', [ true, 1 ], - [ false, nil ] + [ false ], + [ nil ] validation_test 'is: proc { |x| x > blah }', [ 10 ], @@ -274,7 +287,8 @@ describe "Chef::Resource.property validation" do validation_test 'is: []', [], - [ :a, nil ] + [ :a ], + [ nil ] end # Combination diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb index ff5e135c78..152bad9bc0 100644 --- a/spec/unit/property_spec.rb +++ b/spec/unit/property_spec.rb @@ -319,6 +319,10 @@ describe "Chef::Resource.property" do it "when x is not set, it is not included in state" do expect(resource.state).to eq({}) end + it "when x is set to nil, it returns nil" do + resource.instance_eval { @x = nil } + expect(resource.x).to be_nil + end context "With a subclass" do let(:subresource_class) do |