diff options
author | Thom May <thom@may.lt> | 2018-03-28 10:23:59 +0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-03-28 10:23:59 +0700 |
commit | 1775822934b6f42791cf46454027a3a17f14e726 (patch) | |
tree | 0581980aa9e0087de1ed1eeacce51deacf69edaf | |
parent | 7a6bf1531f91d121a21493924905cde37592a48b (diff) | |
parent | 2dc3fde7cde3779511108cff212f8f4bf32a33ce (diff) | |
download | chef-1775822934b6f42791cf46454027a3a17f14e726.tar.gz |
Merge pull request #7037 from chef/lcg/nil-sets-default
Setting nil to properties with implicit nil sets default value
-rw-r--r-- | lib/chef/mixin/params_validate.rb | 6 | ||||
-rw-r--r-- | lib/chef/property.rb | 21 | ||||
-rw-r--r-- | spec/unit/property/validation_spec.rb | 55 |
3 files changed, 64 insertions, 18 deletions
diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb index c955dd3b12..4ed4b3ab3b 100644 --- a/lib/chef/mixin/params_validate.rb +++ b/lib/chef/mixin/params_validate.rb @@ -131,10 +131,6 @@ class Chef private - def explicitly_allows_nil?(key, validation) - validation.has_key?(:is) && _pv_is({ key => nil }, key, validation[:is], raise_error: false) - end - def _validation_message(key, default) @validation_message.has_key?(key) ? @validation_message[key] : default end @@ -407,7 +403,7 @@ class Chef # x nil #=> invalid # ``` # - def _pv_is(opts, key, to_be, raise_error: true) + def _pv_is(opts, key, to_be) return true if !opts.has_key?(key.to_s) && !opts.has_key?(key.to_sym) value = _pv_opts_lookup(opts, key) to_be = [ to_be ].flatten(1) diff --git a/lib/chef/property.rb b/lib/chef/property.rb index b38ec24de6..3ad28fad10 100644 --- a/lib/chef/property.rb +++ b/lib/chef/property.rb @@ -367,7 +367,7 @@ class Chef # Otherwise, we have to validate it now. value = input_to_stored_value(resource, default, is_default: true) end - value = stored_value_to_output(resource, value, is_default: true) + value = stored_value_to_output(resource, value) # If the value is mutable (non-frozen), we set it on the instance # so that people can mutate it. (All constant default values are @@ -600,7 +600,9 @@ class Chef # @api private def explicitly_accepts_nil?(resource) options.has_key?(:coerce) || - (options.has_key?(:is) && resource.send(:_pv_is, { name => nil }, name, options[:is], raise_error: false)) + (options.has_key?(:is) && Chef::Mixin::ParamsValidate.send(:_pv_is, { name => nil }, name, options[:is])) + rescue Chef::Exceptions::ValidationFailed, Chef::Exceptions::CannotValidateStaticallyError + false end # @api private @@ -676,25 +678,26 @@ class Chef end def input_to_stored_value(resource, value, is_default: false) + if value.nil? && !is_default && !explicitly_accepts_nil?(resource) + value = default + end unless value.is_a?(DelayedEvaluator) - value = coerce_and_validate(resource, value, is_default: is_default) + value = coerce_and_validate(resource, value) end value end - def stored_value_to_output(resource, value, is_default: false) + def stored_value_to_output(resource, value) # Crack open lazy values before giving the result to the user if value.is_a?(DelayedEvaluator) value = exec_in_resource(resource, value) - value = coerce_and_validate(resource, value, is_default: is_default) + value = coerce_and_validate(resource, value) end value end - # Coerces and validates the value. If the value is a default, it will warn - # the user that invalid defaults are bad mmkay, and return it as if it were - # valid. - def coerce_and_validate(resource, value, is_default: false) + # Coerces and validates the value. + def coerce_and_validate(resource, value) result = coerce(resource, value) validate(resource, result) diff --git a/spec/unit/property/validation_spec.rb b/spec/unit/property/validation_spec.rb index 882ea3353b..b05d8c4e17 100644 --- a/spec/unit/property/validation_spec.rb +++ b/spec/unit/property/validation_spec.rb @@ -617,8 +617,9 @@ describe "Chef::Resource.property validation" do expect(resource.x 1).to eq 1 expect(resource.x).to eq 1 end - it "value nil is invalid" do - expect { resource.x nil }.to raise_error Chef::Exceptions::ValidationFailed + it "value nil sets to the default" do + # this mildly complicated because the default of a name property is a lazy evaluator to the actual resource.name + expect(resource.x nil).to be_a(Chef::DelayedEvaluator) end end @@ -630,8 +631,54 @@ describe "Chef::Resource.property validation" do expect(resource.x 1).to eq 1 expect(resource.x).to eq 1 end - it "value nil is invalid" do - expect { resource.x nil }.to raise_error Chef::Exceptions::ValidationFailed + it "value nil sets the default" do + expect(resource.x nil).to eq 10 + expect(resource.x).to eq 10 + end + end + end + + context "nil setting default" do + with_property ":x, String" do + it "if x is not specified, the default is returned" do + expect(resource.x).to eq nil + end + it "value '2' is valid" do + expect(resource.x "2").to eq "2" + expect(resource.x).to eq "2" + end + it "value nil sets the default" do + resource.x "2" + expect(resource.x nil).to eq nil + expect(resource.x).to eq nil + end + end + with_property ":x, String, default: '1'" do + it "if x is not specified, the default is returned" do + expect(resource.x).to eq "1" + end + it "value '2' is valid" do + expect(resource.x "2").to eq "2" + expect(resource.x).to eq "2" + end + it "value nil sets the default" do + resource.x "2" + expect(resource.x nil).to eq "1" + expect(resource.x).to eq "1" + end + end + with_property ":x, [ String, nil ] , default: '1'" do + it "if x is not specified, the default is returned" do + expect(resource.x).to eq "1" + end + it "value '2' is valid" do + expect(resource.x "2").to eq "2" + expect(resource.x).to eq "2" + end + it "value nil sets to nil" do + resource.x "2" + expect(resource.x nil).to eq nil + expect(resource.x).to eq nil end end end |