Per-container deep merge cachinglcg/deep-merge-cache-backport

Replaces the one-big top level deep merge cache with individual deep
merge caches in every container.  The Immutable container state becomes
the deep merge cache.

- Does not use a pure decorator style approach since that failed before
because of ruby internals breaking things like `===` and `=~` on
decorated objects, so we inherit (ultimately from Hash + Array).
- The state being the container state is useful in ruby since APIs on
Hash and Array poke around in internal state to make things fast.  If
we inherit from Hash/Array but don't have the correct internal state
things go wonky.
- Throwing away the internal state is equivalent to flushing the cache.
- Since we throw away all linked objects when we do that, we flush at
every level below the level being flushed (which is correct semantics).
- If a user has a pointer to an old immutable object from a sub-level,
that isn't mutated so the old object still contains the old view of
the data (which I think is correct, although I have some doubts that
its necessary, but it came along free for the ride).
- When we reset the cache we do mutate the cache being reset, which
might change data in held references.  If this becomes an issue the fix would
be to reset the cache at the level above by creating a new, "empty"
ImmutableHash/ImmutableArray object and inserting it into the
deep_merge_cache datastructure instead of clearing the internal state
of the child object.  I don't know practically how anyone would hit
this, though, so would prefer to wait on doing that work until we see
an actual bug report.
- Because of the way ruby pokes around internally there's some
weirdnesses like the pre-generation of the cache for all the values of
a subarray when #each is called, which is due to the way that ruby
walks through array-serialized hashes when called like:
`Array#each { key, value| ... }` (which is an undocumented(?) thing
in ruby).

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

10 files changed, 338 insertions, 157 deletions

diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 1ac9c92468..3265d44d92 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -1,7 +1,7 @@
 #--
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: AJ Christensen (<aj@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software, Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -17,7 +17,7 @@
 # limitations under the License.
 #
 
-require "chef/node/mixin/deep_merge_cache"
+#require "chef/node/mixin/deep_merge_cache"
 require "chef/node/mixin/immutablize_hash"
 require "chef/node/mixin/state_tracking"
 require "chef/node/immutable_collections"
@@ -45,7 +45,7 @@ class Chef
       # expects.  This include should probably be deleted?
       include Enumerable
 
-      include Chef::Node::Mixin::DeepMergeCache
+      #include Chef::Node::Mixin::DeepMergeCache
       include Chef::Node::Mixin::StateTracking
       include Chef::Node::Mixin::ImmutablizeHash
 
@@ -187,6 +187,9 @@ class Chef
        # return the automatic level attribute component
       attr_reader :automatic
 
+      # return the immutablemash deep merge cache
+      attr_reader :deep_merge_cache
+
       def initialize(normal, default, override, automatic, node = nil)
         @default        = VividMash.new(default, self, node, :default)
         @env_default    = VividMash.new({}, self, node, :env_default)
@@ -202,7 +205,8 @@ class Chef
 
         @automatic      = VividMash.new(automatic, self, node, :automatic)
 
-        super(nil, self, node, :merged)
+        @deep_merge_cache = ImmutableMash.new({}, self, node, :merged)
+        @__node__ = node
       end
 
        # Debug what's going on with an attribute. +args+ is a path spec to the
@@ -230,6 +234,22 @@ class Chef
         end
       end
 
+      def reset
+        @deep_merge_cache = ImmutableMash.new({}, self, @__node__, :merged)
+      end
+
+      def reset_cache(*path)
+        if path.empty?
+          reset
+        else
+          container = read(*path)
+          case container
+          when Hash, Array
+            container.reset
+          end
+        end
+      end
+
        # Set the cookbook level default attribute component to +new_data+.
       def default=(new_data)
         reset
@@ -294,7 +314,7 @@ class Chef
 
        # clears attributes from all precedence levels
       def rm(*args)
-        with_deep_merged_return_value(self, *args) do
+        with_deep_merged_return_value(combined_all, *args) do
           rm_default(*args)
           rm_normal(*args)
           rm_override(*args)
@@ -341,6 +361,9 @@ class Chef
       def with_deep_merged_return_value(obj, *path, last)
         hash = obj.read(*path)
         return nil unless hash.is_a?(Hash)
+        # coerce from immutablemash/vividmash to plain-old Hash
+        # also de-immutablizes and dup's the return value correctly in chef-13
+        hash = hash.to_hash
         ret = hash[last]
         yield
         ret
@@ -402,16 +425,16 @@ class Chef
        # all of node['foo'] even if the user only requires node['foo']['bar']['baz'].
        #
 
-      def merged_attributes(*path)
-        merge_all(path)
+      def combined_override(*path)
+        merge_overrides(path)
       end
 
-      def combined_override(*path)
-        immutablize(merge_overrides(path))
+      def combined_all(*path)
+        path.empty? ? self : read(*path)
       end
 
       def combined_default(*path)
-        immutablize(merge_defaults(path))
+        merge_defaults(path)
       end
 
       def normal_unless(*args)
@@ -499,6 +522,14 @@ class Chef
         merged_attributes.to_s
       end
 
+      def [](key)
+        @deep_merge_cache[key]
+      end
+
+      def merged_attributes
+        @deep_merge_cache
+      end
+
       def inspect
         "#<#{self.class} " << (COMPONENTS + [:@merged_attributes, :@properties]).map do |iv|
           "#{iv}=#{instance_variable_get(iv).inspect}"
@@ -507,7 +538,14 @@ class Chef
 
       private
 
-       # Helper method for merge_all/merge_defaults/merge_overrides.
+      # For elements like Fixnums, true, nil...
+      def safe_dup(e)
+        e.dup
+      rescue TypeError
+        e
+      end
+
+       # Helper method for merge_defaults/merge_overrides.
        #
        # apply_path(thing, [ "foo", "bar", "baz" ]) = thing["foo"]["bar"]["baz"]
        #
@@ -537,34 +575,6 @@ class Chef
         end
       end
 
-       # For elements like Fixnums, true, nil...
-      def safe_dup(e)
-        e.dup
-      rescue TypeError
-        e
-      end
-
-       # Deep merge all attribute levels using hash-only merging between different precidence
-       # levels (so override arrays completely replace arrays set at any default level).
-       #
-       # The path allows for selectively deep-merging a subtree of the node object.
-       #
-       # @param path [Array] Array of args to method chain to descend into the node object
-       # @return [attr] Deep Merged values (may be VividMash, Hash, Array, etc) from the node object
-      def merge_all(path)
-        components = [
-          merge_defaults(path),
-          apply_path(@normal, path),
-          merge_overrides(path),
-          apply_path(@automatic, path),
-        ]
-
-        ret = components.inject(NIL) do |merged, component|
-          hash_only_merge!(merged, component)
-        end
-        ret == NIL ? nil : ret
-      end
-
        # Deep merge the default attribute levels with array merging.
        #
        # The path allows for selectively deep-merging a subtree of the node object.
@@ -636,38 +646,6 @@ class Chef
         end
       end
 
-      # @api private
-      def hash_only_merge!(merge_onto, merge_with)
-        # If there are two Hashes, recursively merge.
-        if merge_onto.kind_of?(Hash) && merge_with.kind_of?(Hash)
-          merge_with.each do |key, merge_with_value|
-            value =
-              if merge_onto.has_key?(key)
-                hash_only_merge!(safe_dup(merge_onto[key]), merge_with_value)
-              else
-                merge_with_value
-              end
-
-            # internal_set bypasses converting keys, does convert values and allows writing to immutable mashes
-            merge_onto.internal_set(key, value)
-          end
-          merge_onto
-
-        # If merge_with is nil, don't replace merge_onto
-        elsif merge_with.nil?
-          merge_onto
-
-        # In all other cases, replace merge_onto with merge_with
-        else
-          if merge_with.kind_of?(Hash)
-            Chef::Node::ImmutableMash.new(merge_with)
-          elsif merge_with.kind_of?(Array)
-            Chef::Node::ImmutableArray.new(merge_with)
-          else
-            merge_with
-          end
-        end
-      end
     end
   end
 end
diff --git a/lib/chef/node/attribute_collections.rb b/lib/chef/node/attribute_collections.rb
index 694b5fbc3a..8f9b327b89 100644
--- a/lib/chef/node/attribute_collections.rb
+++ b/lib/chef/node/attribute_collections.rb
@@ -1,6 +1,6 @@
 #--
 # Author:: Daniel DeLeo (<dan@chef.io>)
-# Copyright:: Copyright 2012-2016, Chef Software, Inc.
+# Copyright:: Copyright 2012-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -63,13 +63,13 @@ class Chef
       MUTATOR_METHODS.each do |mutator|
         define_method(mutator) do |*args, &block|
           ret = super(*args, &block)
-          send_reset_cache
+          send_reset_cache(__path__)
           ret
         end
       end
 
       def delete(key, &block)
-        send_reset_cache(__path__, key)
+        send_reset_cache(__path__)
         super
       end
 
@@ -147,13 +147,13 @@ class Chef
       # object.
 
       def delete(key, &block)
-        send_reset_cache(__path__, key)
+        send_reset_cache(__path__)
         super
       end
 
       MUTATOR_METHODS.each do |mutator|
         define_method(mutator) do |*args, &block|
-          send_reset_cache
+          send_reset_cache(__path__)
           super(*args, &block)
         end
       end
@@ -174,7 +174,7 @@ class Chef
 
       def []=(key, value)
         ret = super
-        send_reset_cache(__path__, key)
+        send_reset_cache(__path__)
         ret
       end
 
diff --git a/lib/chef/node/common_api.rb b/lib/chef/node/common_api.rb
index a703c1ef54..fc0ffc57a5 100644
--- a/lib/chef/node/common_api.rb
+++ b/lib/chef/node/common_api.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016, Chef Software, Inc.
+# Copyright:: Copyright 2016-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb
index 57c2f8c021..0320e0fb8b 100644
--- a/lib/chef/node/immutable_collections.rb
+++ b/lib/chef/node/immutable_collections.rb
@@ -24,22 +24,16 @@ class Chef
   class Node
 
     module Immutablize
-      def convert_value(value)
+      def convert_value(value, path = nil)
         case value
         when Hash
-          ImmutableMash.new(value, __root__, __node__, __precedence__)
+          ImmutableMash.new({}, __root__, __node__, __precedence__, path)
         when Array
-          ImmutableArray.new(value, __root__, __node__, __precedence__)
-        when ImmutableMash, ImmutableArray
-          value
+          ImmutableArray.new([], __root__, __node__, __precedence__, path)
         else
           value
         end
       end
-
-      def immutablize(value)
-        convert_value(value)
-      end
     end
 
     # == ImmutableArray
@@ -53,17 +47,52 @@ class Chef
     #   Chef::Node::Attribute's values, it overrides all reader methods to
     #   detect staleness and raise an error if accessed when stale.
     class ImmutableArray < Array
+      alias_method :internal_clear, :clear
+      alias_method :internal_replace, :replace
+      alias_method :internal_push, :<<
+      alias_method :internal_to_a, :to_a
+      alias_method :internal_each, :each
+      private :internal_push, :internal_replace, :internal_clear, :internal_each
+      protected :internal_to_a
+
       include Immutablize
 
-      alias :internal_push :<<
-      private :internal_push
+      methods = Array.instance_methods - Object.instance_methods +
+        [ :!, :!=, :<=>, :==, :===, :eql?, :to_s, :hash, :key, :has_key?, :inspect, :pretty_print, :pretty_print_inspect, :pretty_print_cycle, :pretty_print_instance_variables ]
 
-      def initialize(array_data = [])
-        array_data.each do |value|
-          internal_push(immutablize(value))
+      methods.each do |method|
+        define_method method do |*args, &block|
+          ensure_generated_cache!
+          super(*args, &block)
+        end
+      end
+
+      def each
+        ensure_generated_cache!
+        # aggressively pre generate the cache, works around ruby being too smart and fiddling with internals
+        internal_each { |i| i.ensure_generated_cache! if i.respond_to?(:ensure_generated_cache!) }
+        super
+      end
+
+      # because sometimes ruby gives us back Arrays or ImmutableArrays out of objects from things like #uniq or array slices
+      def return_normal_array(array)
+        if array.respond_to?(:internal_to_a, true)
+          array.internal_to_a
+        else
+          puts array.class
+          array.to_a
         end
       end
 
+      def uniq
+        ensure_generated_cache!
+        return_normal_array(super)
+      end
+
+      def initialize(array_data = [])
+        # Immutable collections no longer have initialized state
+      end
+
       # For elements like Fixnums, true, nil...
       def safe_dup(e)
         e.dup
@@ -91,7 +120,58 @@ class Chef
         a
       end
 
-      # for consistency's sake -- integers 'converted' to integers
+      alias_method :to_array, :to_a
+
+      def [](*args)
+        ensure_generated_cache!
+        args.length > 1 ? return_normal_array(super) : super # correctly handle array slices
+      end
+
+      def reset
+        @generated_cache = false
+        internal_clear # redundant?
+      end
+
+      # @api private
+      def ensure_generated_cache!
+        generate_cache unless @generated_cache
+        @generated_cache = true
+      end
+
+      private
+
+      def combined_components(components)
+        combined_values = nil
+        components.each do |component|
+          values = __node__.attributes.instance_variable_get(component).read(*__path__)
+          next unless values.is_a?(Array)
+          combined_values ||= []
+          combined_values += values
+        end
+        combined_values
+      end
+
+      def get_array(component)
+        array = __node__.attributes.instance_variable_get(component).read(*__path__)
+        if array.is_a?(Array)
+          array
+        end # else nil
+      end
+
+      def generate_cache
+        internal_clear
+        components = []
+        components << combined_components(Attribute::DEFAULT_COMPONENTS)
+        components << get_array(:@normal)
+        components << combined_components(Attribute::OVERRIDE_COMPONENTS)
+        components << get_array(:@automatic)
+        highest = components.compact.last
+        if highest.is_a?(Array)
+          internal_replace( highest.each_with_index.map { |x, i| convert_value(x, __path__ + [ i ] ) } )
+        end
+      end
+
+      # needed for __path__
       def convert_key(key)
         key
       end
@@ -113,19 +193,30 @@ class Chef
     #   it is stale.
     # * Values can be accessed in attr_reader-like fashion via method_missing.
     class ImmutableMash < Mash
+      alias_method :internal_clear, :clear
+      alias_method :internal_key?, :key? # FIXME: could bypass convert_key in Mash for perf
+
       include Immutablize
       include CommonAPI
 
+      methods = Hash.instance_methods - Object.instance_methods +
+        [ :!, :!=, :<=>, :==, :===, :eql?, :to_s, :hash, :key, :has_key?, :inspect, :pretty_print, :pretty_print_inspect, :pretty_print_cycle, :pretty_print_instance_variables ]
+
+      methods.each do |method|
+        define_method method do |*args, &block|
+          ensure_generated_cache!
+          super(*args, &block)
+        end
+      end
+
       # this is for deep_merge usage, chef users must never touch this API
       # @api private
       def internal_set(key, value)
-        regular_writer(key, convert_value(value))
+        regular_writer(key, convert_value(value, __path__ + [ key ]))
       end
 
       def initialize(mash_data = {})
-        mash_data.each do |key, value|
-          internal_set(key, value)
-        end
+        # Immutable collections no longer have initialized state
       end
 
       alias :attribute? :has_key?
@@ -134,8 +225,8 @@ class Chef
         if symbol == :to_ary
           super
         elsif args.empty?
-          if key?(symbol)
-            self[symbol]
+          if key?(symbol.to_s)
+            self[symbol.to_s]
           else
             raise NoMethodError, "Undefined method or attribute `#{symbol}' on `node'"
           end
@@ -157,7 +248,7 @@ class Chef
         Mash.new(self)
       end
 
-      def to_hash
+      def to_h
         h = Hash.new
         each_pair do |k, v|
           h[k] =
@@ -173,6 +264,54 @@ class Chef
         h
       end
 
+      def [](key)
+        ensure_generated_cache!
+        super
+#        unless @merged_lazy_hash.nil?
+#          @merged_lazy_hash[key]
+#        else
+#          Attribute::COMPONENTS.reverse.each do |component|
+#            value = __node__.attributes.instance_variable_get(component).read(*__path__, key)
+#            unless value.nil?
+#              return convert_value(value, __path__ + [ key ])
+#            end
+#          end
+#          nil
+#        end
+      end
+
+      alias_method :to_hash, :to_h
+
+      def reset
+        @generated_cache = false
+        internal_clear # redundant?
+      end
+
+      # @api private
+      def ensure_generated_cache!
+        generate_cache unless @generated_cache
+        @generated_cache = true
+      end
+
+      private
+
+      def generate_cache
+        internal_clear
+        Attribute::COMPONENTS.reverse.each do |component|
+          subhash = __node__.attributes.instance_variable_get(component).read(*__path__)
+          unless subhash.nil? # FIXME: nil is used for not present
+            if subhash.kind_of?(Hash)
+              subhash.keys.each do |key|
+                next if internal_key?(key)
+                internal_set(key, subhash[key])
+              end
+            else
+              break
+            end
+          end
+        end
+      end
+
       prepend Chef::Node::Mixin::StateTracking
       prepend Chef::Node::Mixin::ImmutablizeHash
     end
diff --git a/lib/chef/node/mixin/immutablize_hash.rb b/lib/chef/node/mixin/immutablize_hash.rb
index f09e6944fc..f6b22ed7d7 100644
--- a/lib/chef/node/mixin/immutablize_hash.rb
+++ b/lib/chef/node/mixin/immutablize_hash.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016, Chef Software, Inc.
+# Copyright:: Copyright 2016-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/lib/chef/node/mixin/state_tracking.rb b/lib/chef/node/mixin/state_tracking.rb
index 5958973024..690d261df6 100644
--- a/lib/chef/node/mixin/state_tracking.rb
+++ b/lib/chef/node/mixin/state_tracking.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016, Chef Software, Inc.
+# Copyright:: Copyright 2016-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -24,11 +24,12 @@ class Chef
         attr_reader :__node__
         attr_reader :__precedence__
 
-        def initialize(data = nil, root = self, node = nil, precedence = nil)
+        def initialize(data = nil, root = self, node = nil, precedence = nil, path = nil)
           # __path__ and __root__ must be nil when we call super so it knows
           # to avoid resetting the cache on construction
           data.nil? ? super() : super(data)
-          @__path__ = []
+          @__path__ = path
+          @__path__ ||= []
           @__root__ = root
           @__node__ = node
           @__precedence__ = precedence
@@ -76,9 +77,8 @@ class Chef
           end
         end
 
-        def send_reset_cache(path = nil, key = nil)
-          next_path = [ path, key ].flatten.compact
-          __root__.reset_cache(next_path.first) if !__root__.nil? && __root__.respond_to?(:reset_cache) && !next_path.nil?
+        def send_reset_cache(path)
+          __root__.reset_cache(*path) if !__root__.nil? && __root__.respond_to?(:reset_cache) && !path.nil?
         end
 
         def copy_state_to(ret, next_path)
diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb
index a3e62ff939..e011c18c2c 100644
--- a/spec/unit/node/attribute_spec.rb
+++ b/spec/unit/node/attribute_spec.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
 # Author:: AJ Christensen (<aj@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -171,6 +171,7 @@ describe Chef::Node::Attribute do
     }
     @automatic_hash = { "week" => "friday" }
     @attributes = Chef::Node::Attribute.new(@attribute_hash, @default_hash, @override_hash, @automatic_hash, node)
+    allow(node).to receive(:attributes).and_return(@attributes)
   end
 
   describe "initialize" do
@@ -179,13 +180,14 @@ describe Chef::Node::Attribute do
     end
 
     it "should take an Automatioc, Normal, Default and Override hash" do
-      expect { Chef::Node::Attribute.new({}, {}, {}, {}) }.not_to raise_error
+      expect { Chef::Node::Attribute.new({}, {}, {}, {}, node) }.not_to raise_error
     end
 
     [ :normal, :default, :override, :automatic ].each do |accessor|
       it "should set #{accessor}" do
-        na = Chef::Node::Attribute.new({ :normal => true }, { :default => true }, { :override => true }, { :automatic => true })
-        expect(na.send(accessor)).to eq({ accessor.to_s => true })
+        @attributes = Chef::Node::Attribute.new({ :normal => true }, { :default => true }, { :override => true }, { :automatic => true }, node)
+        allow(node).to receive(:attributes).and_return(@attributes)
+        expect(@attributes.send(accessor)).to eq({ accessor.to_s => true })
       end
     end
 
@@ -313,7 +315,8 @@ describe Chef::Node::Attribute do
     end
 
     it "merges nested hashes between precedence levels" do
-      @attributes = Chef::Node::Attribute.new({}, {}, {}, {})
+      @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node)
+      allow(node).to receive(:attributes).and_return(@attributes)
       @attributes.env_default = { "a" => { "b" => { "default" => "default" } } }
       @attributes.normal = { "a" => { "b" => { "normal" => "normal" } } }
       @attributes.override = { "a" => { "override" => "role" } }
@@ -559,8 +562,10 @@ describe Chef::Node::Attribute do
           "one" => { "six" => "seven" },
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should yield each top level key" do
@@ -607,8 +612,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should yield each top level key and value, post merge rules" do
@@ -645,8 +652,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to each_key" do
@@ -681,8 +690,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to each_pair" do
@@ -717,8 +728,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to each_value" do
@@ -761,9 +774,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
-      @empty = Chef::Node::Attribute.new({}, {}, {}, {})
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to empty?" do
@@ -771,7 +785,9 @@ describe Chef::Node::Attribute do
     end
 
     it "should return true when there are no keys" do
-      expect(@empty.empty?).to eq(true)
+      @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node)
+      allow(node).to receive(:attributes).and_return(@attributes)
+      expect(@attributes.empty?).to eq(true)
     end
 
     it "should return false when there are keys" do
@@ -795,8 +811,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to fetch" do
@@ -852,8 +870,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to has_value?" do
@@ -897,8 +917,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to index" do
@@ -938,8 +960,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to values" do
@@ -974,8 +998,10 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
     end
 
     it "should respond to select" do
@@ -1024,10 +1050,11 @@ describe Chef::Node::Attribute do
           "one" => "six",
           "snack" => "cookies",
         },
-        {}
+        {},
+        node
       )
+      allow(node).to receive(:attributes).and_return(@attributes)
 
-      @empty = Chef::Node::Attribute.new({}, {}, {}, {})
     end
 
     it "should respond to size" do
@@ -1039,7 +1066,9 @@ describe Chef::Node::Attribute do
     end
 
     it "should return 0 for an empty attribute" do
-      expect(@empty.size).to eq(0)
+      @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node)
+      allow(node).to receive(:attributes).and_return(@attributes)
+      expect(@attributes.size).to eq(0)
     end
 
     it "should return the number of pairs" do
@@ -1067,8 +1096,9 @@ describe Chef::Node::Attribute do
 
   describe "to_s" do
     it "should output simple attributes" do
-      attributes = Chef::Node::Attribute.new(nil, nil, nil, nil)
-      expect(attributes.to_s).to eq("{}")
+      @attributes = Chef::Node::Attribute.new(nil, nil, nil, nil, node)
+      allow(node).to receive(:attributes).and_return(@attributes)
+      expect(@attributes.to_s).to eq("{}")
     end
 
     it "should output merged attributes" do
@@ -1080,8 +1110,9 @@ describe Chef::Node::Attribute do
           "b" => 3,
           "c" => 4,
       }
-      attributes = Chef::Node::Attribute.new(nil, default_hash, override_hash, nil)
-      expect(attributes.to_s).to eq('{"a"=>1, "b"=>3, "c"=>4}')
+      @attributes = Chef::Node::Attribute.new(nil, default_hash, override_hash, nil, node)
+      allow(node).to receive(:attributes).and_return(@attributes)
+      expect(@attributes.to_s).to eq('{"b"=>3, "c"=>4, "a"=>1}')
     end
   end
 
diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb
index 30c711d720..6fc511a47d 100644
--- a/spec/unit/node/immutable_collections_spec.rb
+++ b/spec/unit/node/immutable_collections_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Daniel DeLeo (<dan@chef.io>)
-# Copyright:: Copyright 2012-2016, Chef Software Inc.
+# Copyright:: Copyright 2012-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -20,13 +20,18 @@ require "spec_helper"
 require "chef/node/immutable_collections"
 
 describe Chef::Node::ImmutableMash do
+
   before do
-    @data_in = { "top" => { "second_level" => "some value" },
-                 "top_level_2" => %w{array of values},
-                 "top_level_3" => [{ "hash_array" => 1, "hash_array_b" => 2 }],
-                 "top_level_4" => { "level2" => { "key" => "value" } },
+    @data_in = { "key" =>
+                 { "top" => { "second_level" => "some value" },
+                   "top_level_2" => %w{array of values},
+                   "top_level_3" => [{ "hash_array" => 1, "hash_array_b" => 2 }],
+                   "top_level_4" => { "level2" => { "key" => "value" } },
+                 },
     }
-    @immutable_mash = Chef::Node::ImmutableMash.new(@data_in)
+    @node = Chef::Node.new()
+    @node.attributes.default = @data_in
+    @immutable_mash = @node["key"]
   end
 
   it "element references like regular hash" do
@@ -57,9 +62,9 @@ describe Chef::Node::ImmutableMash do
   # we only ever absorb VividMashes from other precedence levels, which already have
   # been coerced to only have string keys, so we do not need to do that work twice (performance).
   it "does not call convert_value like Mash/VividMash" do
-    @mash = Chef::Node::ImmutableMash.new({ test: "foo", "test2" => "bar" })
-    expect(@mash[:test]).to eql("foo")
-    expect(@mash["test2"]).to eql("bar")
+    @node.attributes.default = { test: "foo", "test2" => "bar" }
+    expect(@node[:test]).to eql("foo")
+    expect(@node["test2"]).to eql("bar")
   end
 
   describe "to_hash" do
@@ -80,7 +85,9 @@ describe Chef::Node::ImmutableMash do
     end
 
     it "should create a mash with the same content" do
-      expect(@copy).to eq(@immutable_mash)
+      puts @copy.class
+      puts @immutable_mash.class
+      expect(@immutable_mash).to eq(@copy)
     end
 
     it "should allow mutation" do
@@ -124,9 +131,11 @@ end
 describe Chef::Node::ImmutableArray do
 
   before do
-    @immutable_array = Chef::Node::ImmutableArray.new(%w{foo bar baz} + Array(1..3) + [nil, true, false, [ "el", 0, nil ] ])
-    immutable_mash = Chef::Node::ImmutableMash.new({ "m" => "m" })
-    @immutable_nested_array = Chef::Node::ImmutableArray.new(["level1", @immutable_array, immutable_mash])
+    @node = Chef::Node.new()
+    @node.attributes.default = { "key" => ["level1", %w{foo bar baz} + Array(1..3) + [nil, true, false, [ "el", 0, nil ] ], { "m" => "m" }] }
+    @immutable_array = @node["key"][1]
+    @immutable_mash = @node["key"][2]
+    @immutable_nested_array = @node["key"]
   end
 
   ##
@@ -198,7 +207,7 @@ describe Chef::Node::ImmutableArray do
     end
 
     it "should create an array with the same content" do
-      expect(@copy).to eq(@immutable_nested_array)
+      expect(@immutable_nested_array).to eq(@copy)
     end
 
     it "should allow mutation" do
@@ -211,4 +220,11 @@ describe Chef::Node::ImmutableArray do
       expect(@immutable_array[1, 2]).to eql(%w{bar baz})
     end
   end
+
+  describe "uniq" do
+    it "works" do
+      @node.attributes.default = { "key" => %w{foo bar foo baz bar} }
+      expect(@node["key"].uniq).to eql(%w{foo bar baz})
+    end
+  end
 end
diff --git a/spec/unit/node/vivid_mash_spec.rb b/spec/unit/node/vivid_mash_spec.rb
index 4898c22380..651cfa0058 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-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -60,7 +60,7 @@ describe Chef::Node::VividMash do
     end
 
     it "deep converts values through arrays" do
-      expect(root).to receive(:reset_cache).with("foo")
+      expect(root).to receive(:reset_cache).with(no_args)
       vivid["foo"] = [ { :bar => true } ]
       expect(vivid["foo"].class).to eql(Chef::Node::AttrArray)
       expect(vivid["foo"][0].class).to eql(Chef::Node::VividMash)
@@ -68,7 +68,7 @@ describe Chef::Node::VividMash do
     end
 
     it "deep converts values through nested arrays" do
-      expect(root).to receive(:reset_cache).with("foo")
+      expect(root).to receive(:reset_cache).with(no_args)
       vivid["foo"] = [ [ { :bar => true } ] ]
       expect(vivid["foo"].class).to eql(Chef::Node::AttrArray)
       expect(vivid["foo"][0].class).to eql(Chef::Node::AttrArray)
@@ -77,7 +77,7 @@ describe Chef::Node::VividMash do
     end
 
     it "deep converts values through hashes" do
-      expect(root).to receive(:reset_cache).with("foo")
+      expect(root).to receive(:reset_cache).with(no_args)
       vivid["foo"] = { baz: { :bar => true } }
       expect(vivid["foo"]).to be_an_instance_of(Chef::Node::VividMash)
       expect(vivid["foo"]["baz"]).to be_an_instance_of(Chef::Node::VividMash)
@@ -184,42 +184,55 @@ describe Chef::Node::VividMash do
 
     it "should deeply autovivify" do
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "five")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven", "eight")
       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
+      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
       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
+      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
       expect(root).to receive(:reset_cache).at_least(:once).with("array")
+      expect(root).to receive(:reset_cache).at_least(:once).with("array", "five")
       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
-      expect(root).to receive(:reset_cache).at_least(:once).with("one")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "two")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three")
       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
-      expect(root).to receive(:reset_cache).at_least(:once).with("one")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "two")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three", "four")
       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
+      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
       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
+      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
       expect(root).to receive(:reset_cache).at_least(:once).with("nil")
+      expect(root).to receive(:reset_cache).at_least(:once).with("nil", "one")
       vivid.write("nil", "one", "two", "three")
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => { "one" => { "two" => "three" } } })
     end
@@ -240,6 +253,10 @@ describe Chef::Node::VividMash do
 
     it "should deeply autovivify" do
       expect(root).to receive(:reset_cache).at_least(:once).with("one")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "five")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven")
+      expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven", "eight")
       vivid.write!("one", "five", "six", "seven", "eight", "nine", "ten")
       expect(vivid["one"]["five"]["six"]["seven"]["eight"]["nine"]).to eql("ten")
     end
@@ -295,7 +312,7 @@ describe Chef::Node::VividMash do
     end
 
     it "should unlink hashes" do
-      expect(root).to receive(:reset_cache).at_least(:once).with("one")
+      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
       expect( vivid.unlink("one") ).to eql({ "two" => { "three" => "four" } })
       expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil })
     end
@@ -307,7 +324,7 @@ describe Chef::Node::VividMash do
     end
 
     it "should unlink nil" do
-      expect(root).to receive(:reset_cache).at_least(:once).with("nil")
+      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
       expect(vivid.unlink("nil")).to eql(nil)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] })
     end
@@ -327,7 +344,7 @@ describe Chef::Node::VividMash do
     end
 
     it "should unlink! hashes" do
-      expect(root).to receive(:reset_cache).at_least(:once).with("one")
+      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
       expect( vivid.unlink!("one") ).to eql({ "two" => { "three" => "four" } })
       expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil })
     end
@@ -339,7 +356,7 @@ describe Chef::Node::VividMash do
     end
 
     it "should unlink! nil" do
-      expect(root).to receive(:reset_cache).at_least(:once).with("nil")
+      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
       expect(vivid.unlink!("nil")).to eql(nil)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] })
     end
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index ac227c5479..0a96f9adb3 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software, Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");