Chef-15: add nillability to attribute deep merging

Changes to consistently uses the "NIL" object internally to mean "not present"
instead of abusing nil for that purpose.

So now "nil means nil" and we can deep merge that so it will effectively
override lower precedence levels if you write a nil.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 40 insertions, 8 deletions

diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 69cb327d70..d0808cfa34 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -406,11 +406,13 @@ class Chef
       end
 
       def combined_override(*path)
-        merge_overrides(path)
+        ret = merge_overrides(path)
+        ret == NIL ? nil : ret
       end
 
       def combined_default(*path)
-        merge_defaults(path)
+        ret = merge_defaults(path)
+        ret == NIL ? nil : ret
       end
 
       def normal_unless(*args)
@@ -505,10 +507,10 @@ class Chef
               # Hash-like thing (must check has_key? first to protect against Autovivification)
               val[path_arg]
             else
-              nil
+              NIL
             end
           else
-            nil
+            NIL
           end
         end
       end
@@ -552,7 +554,7 @@ class Chef
           component_value = apply_path(instance_variable_get(component_ivar), path)
           deep_merge!(merged, component_value)
         end
-        ret == NIL ? nil : ret
+        ret
       end
 
       # Deep merge the override attribute levels with array merging.
@@ -566,7 +568,7 @@ class Chef
           component_value = apply_path(instance_variable_get(component_ivar), path)
           deep_merge!(merged, component_value)
         end
-        ret == NIL ? nil : ret
+        ret
       end
 
       # needed for __path__
@@ -597,7 +599,7 @@ class Chef
           merge_onto |= merge_with
 
           # If merge_with is nil, don't replace merge_onto
-        elsif merge_with.nil?
+        elsif merge_with == NIL
           merge_onto
 
           # In all other cases, replace merge_onto with merge_with
@@ -630,7 +632,7 @@ class Chef
           merge_onto
 
           # If merge_with is nil, don't replace merge_onto
-        elsif merge_with.nil?
+        elsif merge_with == NIL
           merge_onto
 
           # In all other cases, replace merge_onto with merge_with
diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb
index 36827215f0..c6fdf1e1c2 100644
--- a/spec/unit/node/attribute_spec.rb
+++ b/spec/unit/node/attribute_spec.rb
@@ -1273,4 +1273,34 @@ describe Chef::Node::Attribute do
       expect { @attributes["foo"]["bar"][0] << "buzz" }.to raise_error(RuntimeError, "can't modify frozen String")
     end
   end
+
+  describe "deep merging with nils" do
+    it "nils when deep merging between default levels knocks out values" do
+      @attributes.default["foo"] = "bar"
+      expect(@attributes["foo"]).to eql("bar")
+      @attributes.force_default["foo"] = nil
+      expect(@attributes["foo"]).to be nil
+    end
+
+    it "nils when deep merging between override levels knocks out values" do
+      @attributes.override["foo"] = "bar"
+      expect(@attributes["foo"]).to eql("bar")
+      @attributes.force_override["foo"] = nil
+      expect(@attributes["foo"]).to be nil
+    end
+
+    it "nils when deep merging between default+override levels knocks out values" do
+      @attributes.default["foo"] = "bar"
+      expect(@attributes["foo"]).to eql("bar")
+      @attributes.override["foo"] = nil
+      expect(@attributes["foo"]).to be nil
+    end
+
+    it "nils when deep merging between normal+automatic levels knocks out values" do
+      @attributes.normal["foo"] = "bar"
+      expect(@attributes["foo"]).to eql("bar")
+      @attributes.automatic["foo"] = nil
+      expect(@attributes["foo"]).to be nil
+    end
+  end
 end