Merge pull request #7840 from chef/lcg/chef-15-attribute-array-fixes

Chef 15 node attribute array fixes

5 files changed, 186 insertions, 50 deletions

diff --git a/lib/chef/node/attribute_collections.rb b/lib/chef/node/attribute_collections.rb
index f62eceb646..382ecfba91 100644
--- a/lib/chef/node/attribute_collections.rb
+++ b/lib/chef/node/attribute_collections.rb
@@ -18,6 +18,9 @@
 
 require "chef/node/common_api"
 require "chef/node/mixin/state_tracking"
+require "chef/node/mixin/immutablize_array"
+require "chef/node/mixin/immutablize_hash"
+require "chef/node/mixin/mashy_array"
 
 class Chef
   class Node
@@ -26,36 +29,9 @@ class Chef
     # "root" (Chef::Node::Attribute) object, and will trigger a cache
     # invalidation on that object when mutated.
     class AttrArray < Array
-      MUTATOR_METHODS = [
-        :<<,
-        :[]=,
-        :clear,
-        :collect!,
-        :compact!,
-        :default=,
-        :default_proc=,
-        :delete_at,
-        :delete_if,
-        :fill,
-        :flatten!,
-        :insert,
-        :keep_if,
-        :map!,
-        :merge!,
-        :pop,
-        :push,
-        :update,
-        :reject!,
-        :reverse!,
-        :replace,
-        :select!,
-        :shift,
-        :slice!,
-        :sort!,
-        :sort_by!,
-        :uniq!,
-        :unshift,
-      ].freeze
+      include Chef::Node::Mixin::MashyArray
+
+      MUTATOR_METHODS = Chef::Node::Mixin::ImmutablizeArray::DISALLOWED_MUTATOR_METHODS
 
       # For all of the methods that may mutate an Array, we override them to
       # also invalidate the cached merged_attributes on the root
@@ -130,27 +106,11 @@ class Chef
       # Methods that mutate a VividMash. Each of them is overridden so that it
       # also invalidates the cached merged_attributes on the root Attribute
       # object.
-      MUTATOR_METHODS = [
-        :clear,
-        :delete_if,
-        :keep_if,
-        :merge!,
-        :update,
-        :reject!,
-        :replace,
-        :select!,
-        :shift,
-      ].freeze
+      MUTATOR_METHODS = Chef::Node::Mixin::ImmutablizeHash::DISALLOWED_MUTATOR_METHODS - [ :write, :write!, :unlink, :unlink! ]
 
       # For all of the mutating methods on Mash, override them so that they
       # also invalidate the cached `merged_attributes` on the root Attribute
       # object.
-
-      def delete(key, &block)
-        send_reset_cache(__path__, key)
-        super
-      end
-
       MUTATOR_METHODS.each do |mutator|
         define_method(mutator) do |*args, &block|
           send_reset_cache
@@ -158,6 +118,11 @@ class Chef
         end
       end
 
+      def delete(key, &block)
+        send_reset_cache(__path__, key)
+        super
+      end
+
       def initialize(data = {})
         super(data)
       end
diff --git a/lib/chef/node/mixin/immutablize_array.rb b/lib/chef/node/mixin/immutablize_array.rb
index 000a088410..b5e0993cfa 100644
--- a/lib/chef/node/mixin/immutablize_array.rb
+++ b/lib/chef/node/mixin/immutablize_array.rb
@@ -159,7 +159,6 @@ class Chef
           :sort_by!,
           :uniq!,
           :unshift,
-          :update,
         ].freeze
 
         # Redefine all of the methods that mutate a Hash to raise an error when called.
diff --git a/lib/chef/node/mixin/mashy_array.rb b/lib/chef/node/mixin/mashy_array.rb
new file mode 100644
index 0000000000..31506c5bbf
--- /dev/null
+++ b/lib/chef/node/mixin/mashy_array.rb
@@ -0,0 +1,68 @@
+#--
+# Copyright:: Copyright 2016-2018, Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+class Chef
+  class Node
+    module Mixin
+      # missing methods for Arrays similar to Chef::Mash methods that call
+      # convert_value correctly.
+      module MashyArray
+        def <<(obj)
+          super(convert_value(obj))
+        end
+
+        def []=(*keys, value)
+          super(*keys, convert_value(value))
+        end
+
+        def push(*objs)
+          objs = objs.map { |obj| convert_value(obj) }
+          super(*objs)
+        end
+
+        def unshift(*objs)
+          objs = objs.map { |obj| convert_value(obj) }
+          super(*objs)
+        end
+
+        def insert(index, *objs)
+          objs = objs.map { |obj| convert_value(obj) }
+          super(index, *objs)
+        end
+
+        def collect!(&block)
+          super
+          map! { |x| convert_value(x) }
+        end
+
+        def map!(&block)
+          super
+          super { |x| convert_value(x) }
+        end
+
+        def fill(*args, &block)
+          super
+          map! { |x| convert_value(x) }
+        end
+
+        def replace(obj)
+          super(convert_value(obj))
+        end
+      end
+    end
+  end
+end
diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb
index 273c3d2704..2208c45717 100644
--- a/spec/unit/node/immutable_collections_spec.rb
+++ b/spec/unit/node/immutable_collections_spec.rb
@@ -209,7 +209,6 @@ describe Chef::Node::ImmutableArray do
     :merge!,
     :pop,
     :push,
-    :update,
     :reject!,
     :reverse!,
     :replace,
diff --git a/spec/unit/node/vivid_mash_spec.rb b/spec/unit/node/vivid_mash_spec.rb
index e1021ba0c0..cfdc813b50 100644
--- a/spec/unit/node/vivid_mash_spec.rb
+++ b/spec/unit/node/vivid_mash_spec.rb
@@ -1,5 +1,5 @@
 #
-# Copyright:: Copyright 2016, Chef Software Inc.
+# Copyright:: Copyright 2016-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -351,3 +351,108 @@ describe Chef::Node::VividMash do
     end
   end
 end
+
+describe Chef::Node::AttrArray do
+  let(:root) { instance_double(Chef::Node::Attribute) }
+
+  let(:array) do
+    Chef::Node::AttrArray.new(
+      %w{zero one two},
+      root
+    )
+  end
+
+  context "#<<" do
+    it "converts a Hash appended with #<< to a VividMash" do
+      array << { "three" => "four" }
+      expect(array[3].class).to eql(Chef::Node::VividMash)
+    end
+
+    it "deeply converts objects appended with #<<" do
+      array << [ { "three" => [ 0, 1] } ]
+      expect(array[3].class).to eql(Chef::Node::AttrArray)
+      expect(array[3][0].class).to eql(Chef::Node::VividMash)
+      expect(array[3][0]["three"].class).to eql(Chef::Node::AttrArray)
+    end
+  end
+
+  context "#[]=" do
+    it "assigning a Hash into an array converts it to VividMash" do
+      array[0] = { "zero" => "zero2" }
+      expect(array[0].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#push" do
+    it "pushing a Hash into an array converts it to VividMash" do
+      array.push({ "three" => "four" })
+      expect(array[3].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#unshift" do
+    it "unshifting a Hash into an array converts it to VividMash" do
+      array.unshift({ "zero" => "zero2" })
+      expect(array[0].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#insert" do
+    it "inserting a Hash into an array converts it to VividMash" do
+      array.insert(1, { "zero" => "zero2" })
+      expect(array[1].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#collect!" do
+    it "converts Hashes" do
+      array.collect! { |x| { "zero" => "zero2" } }
+      expect(array[1].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#map!" do
+    it "converts Hashes" do
+      array.map! { |x| { "zero" => "zero2" } }
+      expect(array[1].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#compact!" do
+    it "VividMashes remain VividMashes" do
+      array = Chef::Node::AttrArray.new(
+          [ nil, { "one" => "two" }, nil ],
+          root
+      )
+      expect(array[1].class).to eql(Chef::Node::VividMash)
+      array.compact!
+      expect(array[0].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#fill" do
+    it "inserts VividMashes for Hashes" do
+      array.fill({ "one" => "two" })
+      expect(array[0].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#flatten!" do
+    it "flattens sub-arrays maintaining VividMashes in them" do
+      array = Chef::Node::AttrArray.new(
+          [ [ { "one" => "two" } ], [ { "one" => "two" } ] ],
+          root
+      )
+      expect(array[0][0].class).to eql(Chef::Node::VividMash)
+      array.flatten!
+      expect(array[0].class).to eql(Chef::Node::VividMash)
+    end
+  end
+
+  context "#replace" do
+    it "replaces the array converting hashes to mashes" do
+      array.replace([ { "foo" => "bar" } ])
+      expect(array[0].class).to eql(Chef::Node::VividMash)
+    end
+  end
+end