Merge pull request #5097 from chef/lcg/fix-5094

Fix #5094 node.default_unless issue in 12.12.13

3 files changed, 82 insertions, 20 deletions

diff --git a/lib/chef/node/common_api.rb b/lib/chef/node/common_api.rb
index 6f921e15aa..ce2c6b6878 100644
--- a/lib/chef/node/common_api.rb
+++ b/lib/chef/node/common_api.rb
@@ -32,6 +32,7 @@ class Chef
       # - autovivifying / autoreplacing writer
       # - non-container-ey intermediate objects are replaced with hashes
       def write(*args, &block)
+        root.top_level_breadcrumb = nil if respond_to?(:root)
         value = block_given? ? yield : args.pop
         last = args.pop
         prev_memo = prev_key = nil
@@ -55,6 +56,7 @@ class Chef
       # something that is not a container ("schema violation" issues).
       #
       def write!(*args, &block)
+        root.top_level_breadcrumb = nil if respond_to?(:root)
         value = block_given? ? yield : args.pop
         last = args.pop
         obj = args.inject(self) do |memo, key|
@@ -69,6 +71,7 @@ class Chef
 
       # return true or false based on if the attribute exists
       def exist?(*path)
+        root.top_level_breadcrumb = nil if respond_to?(:root)
         path.inject(self) do |memo, key|
           return false unless valid_container?(memo, key)
           if memo.is_a?(Hash)
@@ -100,6 +103,7 @@ class Chef
       # non-autovivifying reader that throws an exception if the attribute does not exist
       def read!(*path)
         raise Chef::Exceptions::NoSuchAttribute unless exist?(*path)
+        root.top_level_breadcrumb = nil if respond_to?(:root)
         path.inject(self) do |memo, key|
           memo[key]
         end
@@ -108,6 +112,7 @@ class Chef
       # FIXME:(?) does anyone really like the autovivifying reader that we have and wants the same behavior?  readers that write?  ugh...
 
       def unlink(*path, last)
+        root.top_level_breadcrumb = nil if respond_to?(:root)
         hash = path.empty? ? self : read(*path)
         return nil unless hash.is_a?(Hash) || hash.is_a?(Array)
         root.top_level_breadcrumb ||= last
diff --git a/spec/unit/node/vivid_mash_spec.rb b/spec/unit/node/vivid_mash_spec.rb
index ca5eddce57..5319ba4a35 100644
--- a/spec/unit/node/vivid_mash_spec.rb
+++ b/spec/unit/node/vivid_mash_spec.rb
@@ -32,6 +32,11 @@ describe Chef::Node::VividMash do
     )
   end
 
+  def with_breadcrumb(key)
+    expect(root).to receive(:top_level_breadcrumb=).with(nil).at_least(:once).and_call_original
+    expect(root).to receive(:top_level_breadcrumb=).with(key).at_least(:once).and_call_original
+  end
+
   context "#read" do
     before do
       # vivify the vividmash, then we're read-only so the cache should never be cleared afterwards
@@ -40,27 +45,27 @@ describe Chef::Node::VividMash do
     end
 
     it "reads hashes deeply" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect(vivid.read("one", "two", "three")).to eql("four")
     end
 
     it "does not trainwreck when hitting hash keys that do not exist" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect(vivid.read("one", "five", "six")).to eql(nil)
     end
 
     it "does not trainwreck when hitting an array with an out of bounds index" do
-      expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original
+      with_breadcrumb("array")
       expect(vivid.read("array", 5, "one")).to eql(nil)
     end
 
     it "does not trainwreck when hitting an array with a string key" do
-      expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original
+      with_breadcrumb("array")
       expect(vivid.read("array", "one", "two")).to eql(nil)
     end
 
     it "does not trainwreck when traversing a nil" do
-      expect(root).to receive(:top_level_breadcrumb=).with("nil").and_call_original
+      with_breadcrumb("nil")
       expect(vivid.read("nil", "one", "two")).to eql(nil)
     end
   end
@@ -73,42 +78,42 @@ describe Chef::Node::VividMash do
     end
 
     it "true if there's a hash key there" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect(vivid.exist?("one", "two", "three")).to be true
     end
 
     it "true for intermediate hashes" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect(vivid.exist?("one")).to be true
     end
 
     it "true for arrays that exist" do
-      expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original
+      with_breadcrumb("array")
       expect(vivid.exist?("array", 1)).to be true
     end
 
     it "true when the value of the key is nil" do
-      expect(root).to receive(:top_level_breadcrumb=).with("nil").and_call_original
+      with_breadcrumb("nil")
       expect(vivid.exist?("nil")).to be true
     end
 
     it "false when attributes don't exist" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect(vivid.exist?("one", "five", "six")).to be false
     end
 
     it "false when traversing a non-container" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect(vivid.exist?("one", "two", "three", "four")).to be false
     end
 
     it "false when an array index does not exist" do
-      expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original
+      with_breadcrumb("array")
       expect(vivid.exist?("array", 3)).to be false
     end
 
     it "false when traversing a nil" do
-      expect(root).to receive(:top_level_breadcrumb=).with("nil").and_call_original
+      with_breadcrumb("nil")
       expect(vivid.exist?("nil", "foo", "bar")).to be false
     end
   end
@@ -121,27 +126,27 @@ describe Chef::Node::VividMash do
     end
 
     it "reads hashes deeply" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect(vivid.read!("one", "two", "three")).to eql("four")
     end
 
     it "reads arrays deeply" do
-      expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original
+      with_breadcrumb("array")
       expect(vivid.read!("array", 1)).to eql(1)
     end
 
     it "throws an exception when attributes do not exist" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect { vivid.read!("one", "five", "six") }.to raise_error(Chef::Exceptions::NoSuchAttribute)
     end
 
     it "throws an exception when traversing a non-container" do
-      expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original
+      with_breadcrumb("one")
       expect { vivid.read!("one", "two", "three", "four") }.to raise_error(Chef::Exceptions::NoSuchAttribute)
     end
 
     it "throws an exception when an array element does not exist" do
-      expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original
+      with_breadcrumb("array")
       expect { vivid.read!("array", 3) }.to raise_error(Chef::Exceptions::NoSuchAttribute)
     end
   end
@@ -153,54 +158,63 @@ describe Chef::Node::VividMash do
     end
 
     it "should write into hashes" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       vivid.write("one", "five", "six")
       expect(vivid["one"]["five"]).to eql("six")
     end
 
     it "should deeply autovivify" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       vivid.write("one", "five", "six", "seven", "eight", "nine", "ten")
       expect(vivid["one"]["five"]["six"]["seven"]["eight"]["nine"]).to eql("ten")
     end
 
     it "should raise an exception if you overwrite an array with a hash" do
+      with_breadcrumb("array")
       expect(root).to receive(:reset_cache).at_least(:once).with("array")
       vivid.write("array", "five", "six")
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => { "five" => "six" }, "nil" => nil })
     end
 
     it "should raise an exception if you traverse through an array with a hash" do
+      with_breadcrumb("array")
       expect(root).to receive(:reset_cache).at_least(:once).with("array")
       vivid.write("array", "five", "six", "seven")
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => { "five" => { "six" => "seven" } }, "nil" => nil })
     end
 
     it "should raise an exception if you overwrite a string with a hash" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       vivid.write("one", "two", "three", "four", "five")
       expect(vivid).to eql({ "one" => { "two" => { "three" => { "four" => "five" } } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should raise an exception if you traverse through a string with a hash" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       vivid.write("one", "two", "three", "four", "five", "six")
       expect(vivid).to eql({ "one" => { "two" => { "three" => { "four" => { "five" => "six" } } } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should raise an exception if you overwrite a nil with a hash" do
+      with_breadcrumb("nil")
       expect(root).to receive(:reset_cache).at_least(:once).with("nil")
       vivid.write("nil", "one", "two")
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => { "one" => "two" } })
     end
 
     it "should raise an exception if you traverse through a nil with a hash" do
+      with_breadcrumb("nil")
       expect(root).to receive(:reset_cache).at_least(:once).with("nil")
       vivid.write("nil", "one", "two", "three")
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => { "one" => { "two" => "three" } } })
     end
 
     it "writes with a block" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       vivid.write("one", "five") { "six" }
       expect(vivid["one"]["five"]).to eql("six")
@@ -214,54 +228,63 @@ describe Chef::Node::VividMash do
     end
 
     it "should write into hashes" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       vivid.write!("one", "five", "six")
       expect(vivid["one"]["five"]).to eql("six")
     end
 
     it "should deeply autovivify" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       vivid.write!("one", "five", "six", "seven", "eight", "nine", "ten")
       expect(vivid["one"]["five"]["six"]["seven"]["eight"]["nine"]).to eql("ten")
     end
 
     it "should raise an exception if you overwrite an array with a hash" do
+      with_breadcrumb("array")
       expect(root).not_to receive(:reset_cache)
       expect { vivid.write!("array", "five", "six") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should raise an exception if you traverse through an array with a hash" do
+      with_breadcrumb("array")
       expect(root).not_to receive(:reset_cache)
       expect { vivid.write!("array", "five", "six", "seven") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should raise an exception if you overwrite a string with a hash" do
+      with_breadcrumb("one")
       expect(root).not_to receive(:reset_cache)
       expect { vivid.write!("one", "two", "three", "four", "five") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should raise an exception if you traverse through a string with a hash" do
+      with_breadcrumb("one")
       expect(root).not_to receive(:reset_cache)
       expect { vivid.write!("one", "two", "three", "four", "five", "six") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should raise an exception if you overwrite a nil with a hash" do
+      with_breadcrumb("nil")
       expect(root).not_to receive(:reset_cache)
       expect { vivid.write!("nil", "one", "two") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should raise an exception if you traverse through a nil with a hash" do
+      with_breadcrumb("nil")
       expect(root).not_to receive(:reset_cache)
       expect { vivid.write!("nil", "one", "two", "three") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "writes with a block" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       vivid.write!("one", "five") { "six" }
       expect(vivid["one"]["five"]).to eql("six")
@@ -274,31 +297,36 @@ describe Chef::Node::VividMash do
       expect(root).not_to receive(:reset_cache).with(nil)
     end
 
-    it "should return nil if the keys already don't exist" do
+    it "should return nil if the keys don't already exist" do
+      expect(root).to receive(:top_level_breadcrumb=).with(nil).at_least(:once).and_call_original
       expect(root).not_to receive(:reset_cache)
       expect(vivid.unlink("five", "six", "seven", "eight")).to eql(nil)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should unlink hashes" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       expect( vivid.unlink("one") ).to eql({ "two" => { "three" => "four" } })
       expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should unlink array elements" do
+      with_breadcrumb("array")
       expect(root).to receive(:reset_cache).at_least(:once).with("array")
       expect(vivid.unlink("array", 2)).to eql(2)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1 ], "nil" => nil })
     end
 
     it "should unlink nil" do
+      with_breadcrumb("nil")
       expect(root).to receive(:reset_cache).at_least(:once).with("nil")
       expect(vivid.unlink("nil")).to eql(nil)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] })
     end
 
     it "should traverse a nil and safely do nothing" do
+      with_breadcrumb("nil")
       expect(root).not_to receive(:reset_cache)
       expect(vivid.unlink("nil", "foo")).to eql(nil)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
@@ -311,31 +339,36 @@ describe Chef::Node::VividMash do
       expect(root).not_to receive(:reset_cache).with(nil)
     end
 
-    it "should raise an exception if the keys already don't exist" do
+    it "should raise an exception if the keys don't already exist" do
+      expect(root).to receive(:top_level_breadcrumb=).with(nil).at_least(:once).and_call_original
       expect(root).not_to receive(:reset_cache)
       expect { vivid.unlink!("five", "six", "seven", "eight") }.to raise_error(Chef::Exceptions::NoSuchAttribute)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should unlink! hashes" do
+      with_breadcrumb("one")
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
       expect( vivid.unlink!("one") ).to eql({ "two" => { "three" => "four" } })
       expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil })
     end
 
     it "should unlink! array elements" do
+      with_breadcrumb("array")
       expect(root).to receive(:reset_cache).at_least(:once).with("array")
       expect(vivid.unlink!("array", 2)).to eql(2)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1 ], "nil" => nil })
     end
 
     it "should unlink! nil" do
+      with_breadcrumb("nil")
       expect(root).to receive(:reset_cache).at_least(:once).with("nil")
       expect(vivid.unlink!("nil")).to eql(nil)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] })
     end
 
     it "should raise an exception if it traverses a nil" do
+      with_breadcrumb("nil")
       expect(root).not_to receive(:reset_cache)
       expect { vivid.unlink!("nil", "foo") }.to raise_error(Chef::Exceptions::NoSuchAttribute)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil })
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index af9a6e94fc..4af5b11174 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -357,6 +357,30 @@ describe Chef::Node do
         node.default.fuu.bahrr.baz = "qux"
         expect(node.fuu.bahrr.baz).to eq("qux")
       end
+
+      it "default_unless correctly resets the deep merge cache" do
+        node.normal["tags"] = []  # this sets our top-level breadcrumb
+        node.default_unless["foo"]["bar"] = "NK-19V"
+        expect(node["foo"]["bar"]).to eql("NK-19V")
+        node.default_unless["foo"]["baz"] = "NK-33"
+        expect(node["foo"]["baz"]).to eql("NK-33")
+      end
+
+      it "normal_unless correctly resets the deep merge cache" do
+        node.normal["tags"] = []  # this sets our top-level breadcrumb
+        node.normal_unless["foo"]["bar"] = "NK-19V"
+        expect(node["foo"]["bar"]).to eql("NK-19V")
+        node.normal_unless["foo"]["baz"] = "NK-33"
+        expect(node["foo"]["baz"]).to eql("NK-33")
+      end
+
+      it "override_unless correctly resets the deep merge cache" do
+        node.normal["tags"] = []  # this sets our top-level breadcrumb
+        node.override_unless["foo"]["bar"] = "NK-19V"
+        expect(node["foo"]["bar"]).to eql("NK-19V")
+        node.override_unless["foo"]["baz"] = "NK-33"
+        expect(node["foo"]["baz"]).to eql("NK-33")
+      end
     end
 
     describe "override attributes" do