fixing more copy-on-write bugs

getting trolled by `@foo =` vs. `foo =` plus the indirection
in handling `#ffi-yajl` was really odd.

added pending test for `dup` returning a decorated object because
it feels like it should rather than like it is now where it
does not.

4 files changed, 37 insertions, 9 deletions

diff --git a/lib/chef/node/attribute_trait/copy_on_write.rb b/lib/chef/node/attribute_trait/copy_on_write.rb
index b8ed73a340..fb0c3b86ee 100644
--- a/lib/chef/node/attribute_trait/copy_on_write.rb
+++ b/lib/chef/node/attribute_trait/copy_on_write.rb
@@ -36,7 +36,7 @@ class Chef
 
         MUTATOR_METHODS.each do |method|
           define_method method do |*args, &block|
-            wrapped_object = safe_dup(wrapped_object)
+            @wrapped_object = dup
             super(*args, &block)
           end
         end
diff --git a/lib/chef/node/attribute_trait/decorator.rb b/lib/chef/node/attribute_trait/decorator.rb
index 59d71febfa..888d404bf4 100644
--- a/lib/chef/node/attribute_trait/decorator.rb
+++ b/lib/chef/node/attribute_trait/decorator.rb
@@ -44,7 +44,7 @@ class Chef
         #
 
         def ffi_yajl(*opts)
-          for_json.ffi_yajl(*opts)
+          wrapped_object.ffi_yajl(*opts)
         end
 
         def to_json(*opts)
@@ -52,13 +52,7 @@ class Chef
         end
 
         def for_json
-          if is_a?(Hash)
-            to_hash
-          elsif is_a?(Array)
-            to_ary
-          else
-            wrapped_object
-          end
+          wrapped_object
         end
 
         #
diff --git a/spec/unit/node/attribute_trait/decorator_spec.rb b/spec/unit/node/attribute_trait/decorator_spec.rb
index 3e261f28b5..f024dce4c1 100644
--- a/spec/unit/node/attribute_trait/decorator_spec.rb
+++ b/spec/unit/node/attribute_trait/decorator_spec.rb
@@ -77,6 +77,29 @@ describe Chef::Node::AttributeTrait::Decorator do
         test.each { |k, v| expect(v).not_to be_instance_of(Test) }
       end
     end
+
+    context "Hash#dup" do
+      it "yields decorated objects" do
+        pending "not fixed yet"
+        test["foo"] = { "bar" => "baz" }
+        test["bar"] = [ 1, 2 ]
+        testdup = test.dup
+        expect(testdup).to be_instance_of(Test)
+        expect(testdup["foo"]).to be_instance_of(Test)
+        expect(testdup["bar"]).to be_instance_of(Test)
+      end
+
+      it "deep dups the wrapped_object" do
+        test["foo"] = { "bar" => "baz" }
+        test["bar"] = [ 1, 2 ]
+        testdup = test.dup
+        testdup["foo"]["qux"] = "quux"
+        testdup["foo"]["bar"] << "buzz"
+        testdup["bar"] << 3
+        testdup["baz"] = false
+        expect(test).to eql({"foo" => { "bar" => "baz" }, "bar" => [ 1, 2 ]})
+      end
+    end
   end
 
   context "as an Array" do
diff --git a/spec/unit/node/cow_mash_spec.rb b/spec/unit/node/cow_mash_spec.rb
index cda31089fa..135d21341a 100644
--- a/spec/unit/node/cow_mash_spec.rb
+++ b/spec/unit/node/cow_mash_spec.rb
@@ -7,6 +7,7 @@ describe Chef::Node::COWMash do
     it "behaves correctly" do
       hash = { "foo" => true, "bar" => false }
       cow = Chef::Node::COWMash.new(wrapped_object: hash)
+      expect(hash).not_to receive(:keep_if)
       expect(cow.keep_if { |k, v| v }).to eql({ "foo" => true })
       expect(hash).to eql({ "foo" => true, "bar" => false })
       expect(cow).to eql({ "foo" => true })
@@ -19,4 +20,14 @@ describe Chef::Node::COWMash do
       expect(cow.keep_if { |k, v| v }).to equal(cow)
     end
   end
+
+  context "#[]=" do
+    it "behaves correctly" do
+      hash = { "foo" => true, "bar" => false }
+      cow = Chef::Node::COWMash.new(wrapped_object: hash)
+      expect(cow["baz"] = "qux").to eql("qux")
+      expect(cow).to eql({ "foo" => true, "bar" => false, "baz" => "qux" })
+      expect(hash).to eql({ "foo" => true, "bar" => false })
+    end
+  end
 end