diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2017-03-13 19:54:38 -0700 |
---|---|---|
committer | Lamont Granquist <lamont@scriptkiddie.org> | 2017-03-13 19:54:38 -0700 |
commit | d28d5c65d0074e6a5e3786555b5f31893447b157 (patch) | |
tree | c8a5b9986bedee4c5b032f3f4ef35850cdcef9b5 | |
parent | 8cd2899fa40e21089d832b0a786b8fc4d335d738 (diff) | |
download | chef-d28d5c65d0074e6a5e3786555b5f31893447b157.tar.gz |
additional fix deep-duping as welllcg/deep-dup-attrs
similarly to fixing to_hash.
kind of feels like we should return a VividMash instead of a Mash
here, to get a writable thing that behaves like an attribute...
Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
-rw-r--r-- | lib/chef/node/immutable_collections.rb | 33 | ||||
-rw-r--r-- | spec/unit/node/attribute_spec.rb | 18 | ||||
-rw-r--r-- | spec/unit/node/immutable_collections_spec.rb | 52 |
3 files changed, 89 insertions, 14 deletions
diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb index 08bd9c9c77..13a8aefe97 100644 --- a/lib/chef/node/immutable_collections.rb +++ b/lib/chef/node/immutable_collections.rb @@ -70,19 +70,16 @@ class Chef end def to_a - a = Array.new - each do |v| - a << - case v - when ImmutableArray - v.to_a - when ImmutableMash - v.to_h - else - safe_dup(v) - end - end - a + Array.new(map do |v| + case v + when ImmutableArray + v.to_a + when ImmutableMash + v.to_h + else + safe_dup(v) + end + end) end alias_method :to_array, :to_a @@ -129,6 +126,10 @@ class Chef # Mash uses #convert_value to mashify values on input. # Since we're handling this ourselves, override it to be a no-op + # + # FIXME? this seems wrong to do and i think is responsible for + # #dup needing to be more complicated than Mash.new(self)? + # def convert_value(value) value end @@ -139,7 +140,11 @@ class Chef # Of course, 'default' has a specific meaning in Chef-land def dup - Mash.new(self) + h = Mash.new + each_pair do |k, v| + h[k] = safe_dup(v) + end + h end def to_h diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb index 25594ca20b..0869b83e43 100644 --- a/spec/unit/node/attribute_spec.rb +++ b/spec/unit/node/attribute_spec.rb @@ -483,6 +483,24 @@ describe Chef::Node::Attribute do @attributes.default[:foo] = %w{foo bar baz} + Array(1..3) + [nil, true, false, [ "el", 0, nil ] ] @attributes.default[:foo].dup end + + it "mutating strings should not mutate the attributes in a hash" do + @attributes.default["foo"]["bar"]["baz"] = "fizz" + hash = @attributes["foo"].dup + expect(hash).to eql({ "bar" => { "baz" => "fizz" } }) + hash["bar"]["baz"] << "buzz" + expect(hash).to eql({ "bar" => { "baz" => "fizzbuzz" } }) + expect(@attributes.default["foo"]).to eql({ "bar" => { "baz" => "fizz" } }) + end + + it "mutating array elements should not mutate the attributes" do + @attributes.default["foo"]["bar"] = [ "fizz" ] + hash = @attributes["foo"].dup + expect(hash).to eql({ "bar" => [ "fizz" ] }) + hash["bar"][0] << "buzz" + expect(hash).to eql({ "bar" => [ "fizzbuzz" ] }) + expect(@attributes.default["foo"]).to eql({ "bar" => [ "fizz" ] }) + end end describe "has_key?" do diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb index a8078f1093..520bc1ba42 100644 --- a/spec/unit/node/immutable_collections_spec.rb +++ b/spec/unit/node/immutable_collections_spec.rb @@ -80,6 +80,32 @@ describe Chef::Node::ImmutableMash do end end + describe "dup" do + before do + @copy = @immutable_mash.dup + end + + it "converts an immutable mash to a new mutable hash" do + expect(@copy).to be_instance_of(Mash) + end + + it "converts an immutable nested mash to a new mutable hash" do + expect(@copy["top_level_4"]["level2"]).to be_instance_of(Mash) + end + + it "converts an immutable nested array to a new mutable array" do + expect(@copy["top_level_2"]).to be_instance_of(Array) + end + + it "should create a mash with the same content" do + expect(@copy).to eq(@immutable_mash) + end + + it "should allow mutation" do + expect { @copy["m"] = "m" }.not_to raise_error + end + end + describe "to_h" do before do @copy = @immutable_mash.to_h @@ -223,6 +249,32 @@ describe Chef::Node::ImmutableArray do end end + describe "dup" do + before do + @copy = @immutable_nested_array.dup + end + + it "converts an immutable array to a new mutable array" do + expect(@copy).to be_instance_of(Array) + end + + it "converts an immutable nested array to a new mutable array" do + expect(@copy[1]).to be_instance_of(Array) + end + + it "converts an immutable nested mash to a new mutable hash" do + expect(@copy[2]).to be_instance_of(Mash) + end + + it "should create an array with the same content" do + expect(@copy).to eq(@immutable_nested_array) + end + + it "should allow mutation" do + expect { @copy << "m" }.not_to raise_error + end + end + describe "to_array" do before do @copy = @immutable_nested_array.to_array |