Don't validate `nil` when setting the value to nil

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