Merge pull request #6898 from chef/lcg/chef-13-revert-lazy-attributes

Chef-13 revert lazy attributes

9 files changed, 159 insertions, 372 deletions

diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 389968f543..2998866bb2 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -17,6 +17,7 @@
 # limitations under the License.
 #
 
+require "chef/node/mixin/deep_merge_cache"
 require "chef/node/mixin/immutablize_hash"
 require "chef/node/mixin/state_tracking"
 require "chef/node/immutable_collections"
@@ -44,6 +45,7 @@ class Chef
       # expects.  This include should probably be deleted?
       include Enumerable
 
+      include Chef::Node::Mixin::DeepMergeCache
       include Chef::Node::Mixin::StateTracking
       include Chef::Node::Mixin::ImmutablizeHash
 
@@ -185,9 +187,6 @@ 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)
@@ -203,8 +202,7 @@ class Chef
 
         @automatic      = VividMash.new(automatic, self, node, :automatic)
 
-        @deep_merge_cache = ImmutableMash.new({}, self, node, :merged)
-        @__node__ = node
+        super(nil, self, node, :merged)
       end
 
        # Debug what's going on with an attribute. +args+ is a path spec to the
@@ -228,22 +226,6 @@ 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
@@ -308,7 +290,7 @@ class Chef
 
        # clears attributes from all precedence levels
       def rm(*args)
-        with_deep_merged_return_value(combined_all, *args) do
+        with_deep_merged_return_value(self, *args) do
           rm_default(*args)
           rm_normal(*args)
           rm_override(*args)
@@ -355,9 +337,6 @@ 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
@@ -419,16 +398,16 @@ class Chef
        # all of node['foo'] even if the user only requires node['foo']['bar']['baz'].
        #
 
-      def combined_override(*path)
-        merge_overrides(path)
+      def merged_attributes(*path)
+        merge_all(path)
       end
 
-      def combined_all(*path)
-        path.empty? ? self : read(*path)
+      def combined_override(*path)
+        immutablize(merge_overrides(path))
       end
 
       def combined_default(*path)
-        merge_defaults(path)
+        immutablize(merge_defaults(path))
       end
 
       def normal_unless(*args)
@@ -497,14 +476,6 @@ 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}"
@@ -513,14 +484,7 @@ class Chef
 
       private
 
-      # For elements like Fixnums, true, nil...
-      def safe_dup(e)
-        e.dup
-      rescue TypeError
-        e
-      end
-
-       # Helper method for merge_defaults/merge_overrides.
+       # Helper method for merge_all/merge_defaults/merge_overrides.
        #
        # apply_path(thing, [ "foo", "bar", "baz" ]) = thing["foo"]["bar"]["baz"]
        #
@@ -550,6 +514,34 @@ 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.
@@ -621,6 +613,38 @@ 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 9e4a56b37d..a31b2d2b9b 100644
--- a/lib/chef/node/attribute_collections.rb
+++ b/lib/chef/node/attribute_collections.rb
@@ -63,13 +63,13 @@ class Chef
       MUTATOR_METHODS.each do |mutator|
         define_method(mutator) do |*args, &block|
           ret = super(*args, &block)
-          send_reset_cache(__path__)
+          send_reset_cache
           ret
         end
       end
 
       def delete(key, &block)
-        send_reset_cache(__path__)
+        send_reset_cache(__path__, key)
         super
       end
 
@@ -92,7 +92,6 @@ class Chef
       private
 
       def convert_value(value)
-        value.ensure_generated_cache! if value.respond_to?(:ensure_generated_cache!)
         case value
         when VividMash
           value
@@ -148,13 +147,13 @@ class Chef
       # object.
 
       def delete(key, &block)
-        send_reset_cache(__path__)
+        send_reset_cache(__path__, key)
         super
       end
 
       MUTATOR_METHODS.each do |mutator|
         define_method(mutator) do |*args, &block|
-          send_reset_cache(__path__)
+          send_reset_cache
           super(*args, &block)
         end
       end
@@ -175,7 +174,7 @@ class Chef
 
       def []=(key, value)
         ret = super
-        send_reset_cache(__path__)
+        send_reset_cache(__path__, key)
         ret
       end
 
@@ -190,7 +189,6 @@ class Chef
       # AttrArray for consistency and to ensure that the added parts of the
       # attribute tree will have the correct cache invalidation behavior.
       def convert_value(value)
-        value.ensure_generated_cache! if value.respond_to?(:ensure_generated_cache!)
         case value
         when VividMash
           value
diff --git a/lib/chef/node/common_api.rb b/lib/chef/node/common_api.rb
index fc0ffc57a5..a703c1ef54 100644
--- a/lib/chef/node/common_api.rb
+++ b/lib/chef/node/common_api.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016-2017, Chef Software Inc.
+# Copyright:: Copyright 2016, 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 6458db94e9..848e12d2df 100644
--- a/lib/chef/node/immutable_collections.rb
+++ b/lib/chef/node/immutable_collections.rb
@@ -30,16 +30,22 @@ class Chef
         e
       end
 
-      def convert_value(value, path = nil)
+      def convert_value(value)
         case value
         when Hash
-          ImmutableMash.new({}, __root__, __node__, __precedence__, path)
+          ImmutableMash.new(value, __root__, __node__, __precedence__)
         when Array
-          ImmutableArray.new([], __root__, __node__, __precedence__, path)
+          ImmutableArray.new(value, __root__, __node__, __precedence__)
+        when ImmutableMash, ImmutableArray
+          value
         else
           safe_dup(value).freeze
         end
       end
+
+      def immutablize(value)
+        convert_value(value)
+      end
     end
 
     # == ImmutableArray
@@ -53,49 +59,15 @@ 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
 
-      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 ]
-
-      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
-          array.to_a
-        end
-      end
-
-      def uniq
-        ensure_generated_cache!
-        return_normal_array(super)
-      end
+      alias :internal_push :<<
+      private :internal_push
 
       def initialize(array_data = [])
-        # Immutable collections no longer have initialized state
+        array_data.each do |value|
+          internal_push(immutablize(value))
+        end
       end
 
       # For elements like Fixnums, true, nil...
@@ -124,81 +96,8 @@ class Chef
 
       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
-        @short_circuit_attr_level = nil
-        internal_clear # redundant?
-      end
-
-      # @api private
-      def ensure_generated_cache!
-        generate_cache unless @generated_cache
-        @generated_cache = true
-      end
-
-      # This can be set to e.g. [ :@default ] by the parent container to cause this container
-      # to only use the default level and to bypass deep merging (the common case is either
-      # default-level or automatic-level and we aren't doing any deep merging).  Right now it
-      # "optimized" for the case where we're no longer merging anything and only tracking a
-      # single level, and setting this to anything other than a size=1 array would behave
-      # in a broken fashion.  That could be fixed, but the perf boost would likely not be
-      # that large in the typical case.
-      #
-      # @api private
-      attr_accessor :short_circuit_attr_levels
-
       private
 
-      # deep merging of array attribute within normal and override where they are merged together
-      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)
-          @tracked_components << component
-          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)
-          @tracked_components << component
-          array
-        end # else nil
-      end
-
-      def generate_cache
-        internal_clear
-        components = []
-        @tracked_components = []
-        if short_circuit_attr_levels
-          components << get_array(short_circuit_attr_levels.first)
-        else
-          components << combined_components(Attribute::DEFAULT_COMPONENTS)
-          components << get_array(:@normal)
-          components << combined_components(Attribute::OVERRIDE_COMPONENTS)
-          components << get_array(:@automatic)
-        end
-        highest = components.compact.last
-        if highest.is_a?(Array)
-          internal_replace( highest.each_with_index.map { |x, i| convert_value(x, __path__ + [ i ] ) } )
-        end
-        if @tracked_components.size == 1
-          # tracked_components is accurate enough to tell us if we're not really merging
-          internal_each do |key, value|
-            value.short_circuit_attr_levels = @tracked_components if value.respond_to?(:short_circuit_attr_levels)
-          end
-        end
-      end
-
       # needed for __path__
       def convert_key(key)
         key
@@ -221,31 +120,19 @@ 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
-      alias_method :internal_each, :each
-
       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, __path__ + [ key ]))
+        regular_writer(key, convert_value(value))
       end
 
       def initialize(mash_data = {})
-        # Immutable collections no longer have initialized state
+        mash_data.each do |key, value|
+          internal_set(key, value)
+        end
       end
 
       alias :attribute? :has_key?
@@ -281,55 +168,11 @@ class Chef
 
       alias_method :to_hash, :to_h
 
-      def [](key)
-        ensure_generated_cache!
-        super
-      end
-
-      alias_method :to_hash, :to_h
-
-      def reset
-        @generated_cache = false
-        @short_circuit_attr_level = nil
-        internal_clear # redundant?
-      end
-
-      # @api private
-      def ensure_generated_cache!
-        generate_cache unless @generated_cache
-        @generated_cache = true
-      end
-
-      # @api private
-      attr_accessor :short_circuit_attr_levels
-
-      private
-
-      def generate_cache
-        internal_clear
-        components = short_circuit_attr_levels ? short_circuit_attr_levels : Attribute::COMPONENTS.reverse
-        # tracked_components is not entirely accurate due to the short-circuit
-        tracked_components = []
-        components.each do |component|
-          subhash = __node__.attributes.instance_variable_get(component).read(*__path__)
-          unless subhash.nil? # FIXME: nil is used for not present
-            tracked_components << component
-            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
-        if tracked_components.size == 1
-          # tracked_components is accurate enough to tell us if we're not really merging
-          internal_each do |key, value|
-            value.short_circuit_attr_levels = tracked_components if value.respond_to?(:short_circuit_attr_levels)
-          end
-        end
+      # For elements like Fixnums, true, nil...
+      def safe_dup(e)
+        e.dup
+      rescue TypeError
+        e
       end
 
       prepend Chef::Node::Mixin::StateTracking
diff --git a/lib/chef/node/mixin/state_tracking.rb b/lib/chef/node/mixin/state_tracking.rb
index 0c3d4c33a4..5958973024 100644
--- a/lib/chef/node/mixin/state_tracking.rb
+++ b/lib/chef/node/mixin/state_tracking.rb
@@ -1,5 +1,5 @@
 #--
-# Copyright:: Copyright 2016-2018, Chef Software Inc.
+# Copyright:: Copyright 2016, Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -24,12 +24,11 @@ class Chef
         attr_reader :__node__
         attr_reader :__precedence__
 
-        def initialize(data = nil, root = self, node = nil, precedence = nil, path = nil)
+        def initialize(data = nil, root = self, node = nil, precedence = 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
@@ -77,8 +76,9 @@ class Chef
           end
         end
 
-        def send_reset_cache(path)
-          __root__.reset_cache(*path) if !__root__.nil? && __root__.respond_to?(:reset_cache) && !path.nil?
+        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?
         end
 
         def copy_state_to(ret, next_path)
diff --git a/spec/unit/mixin/powershell_type_coercions_spec.rb b/spec/unit/mixin/powershell_type_coercions_spec.rb
index 159a0a8d1d..6f52abccfb 100644
--- a/spec/unit/mixin/powershell_type_coercions_spec.rb
+++ b/spec/unit/mixin/powershell_type_coercions_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Jay Mundrawala (<jdm@chef.io>)
-# Copyright:: Copyright 2015-2017, Chef Software Inc.
+# Copyright:: Copyright 2015-2016, Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -64,15 +64,14 @@ describe Chef::Mixin::PowershellTypeCoercions do
     end
 
     it "translates a Chef::Node::ImmutableMash like a hash" do
-      node = Chef::Node.new
-      node.default[:test] = { "a" => 1, "b" => 1.2, "c" => false, "d" => true }
-      expect(test_class.translate_type(node[:test])).to eq("@{a=1;b=1.2;c=$false;d=$true}")
+      test_mash = Chef::Node::ImmutableMash.new({ "a" => 1, "b" => 1.2,
+                                                  "c" => false, "d" => true })
+      expect(test_class.translate_type(test_mash)).to eq("@{a=1;b=1.2;c=$false;d=$true}")
     end
 
     it "translates a Chef::Node::ImmutableArray like an array" do
-      node = Chef::Node.new
-      node.default[:test] = [ true, false ]
-      expect(test_class.translate_type(node[:test])).to eq("@($true,$false)")
+      test_array = Chef::Node::ImmutableArray.new([true, false])
+      expect(test_class.translate_type(test_array)).to eq("@($true,$false)")
     end
 
     it "falls back :to_psobject if we have not defined at explicit rule" do
diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb
index a9dab144ab..cf8d4d4a4f 100644
--- a/spec/unit/node/attribute_spec.rb
+++ b/spec/unit/node/attribute_spec.rb
@@ -171,7 +171,6 @@ 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
@@ -180,14 +179,13 @@ describe Chef::Node::Attribute do
     end
 
     it "should take an Automatioc, Normal, Default and Override hash" do
-      expect { Chef::Node::Attribute.new({}, {}, {}, {}, node) }.not_to raise_error
+      expect { Chef::Node::Attribute.new({}, {}, {}, {}) }.not_to raise_error
     end
 
     [ :normal, :default, :override, :automatic ].each do |accessor|
       it "should set #{accessor}" do
-        @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 })
+        na = Chef::Node::Attribute.new({ :normal => true }, { :default => true }, { :override => true }, { :automatic => true })
+        expect(na.send(accessor)).to eq({ accessor.to_s => true })
       end
     end
 
@@ -332,8 +330,7 @@ describe Chef::Node::Attribute do
     end
 
     it "merges nested hashes between precedence levels" do
-      @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node)
-      allow(node).to receive(:attributes).and_return(@attributes)
+      @attributes = Chef::Node::Attribute.new({}, {}, {}, {})
       @attributes.env_default = { "a" => { "b" => { "default" => "default" } } }
       @attributes.normal = { "a" => { "b" => { "normal" => "normal" } } }
       @attributes.override = { "a" => { "override" => "role" } }
@@ -587,10 +584,8 @@ 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
@@ -637,10 +632,8 @@ 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
@@ -677,10 +670,8 @@ 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
@@ -715,10 +706,8 @@ 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
@@ -753,10 +742,8 @@ 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
@@ -799,10 +786,9 @@ 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 empty?" do
@@ -810,9 +796,7 @@ describe Chef::Node::Attribute do
     end
 
     it "should return true when there are no keys" do
-      @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node)
-      allow(node).to receive(:attributes).and_return(@attributes)
-      expect(@attributes.empty?).to eq(true)
+      expect(@empty.empty?).to eq(true)
     end
 
     it "should return false when there are keys" do
@@ -836,10 +820,8 @@ 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
@@ -895,10 +877,8 @@ 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
@@ -942,10 +922,8 @@ 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
@@ -985,10 +963,8 @@ 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
@@ -1023,10 +999,8 @@ 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
@@ -1075,11 +1049,10 @@ 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
@@ -1091,9 +1064,7 @@ describe Chef::Node::Attribute do
     end
 
     it "should return 0 for an empty attribute" do
-      @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node)
-      allow(node).to receive(:attributes).and_return(@attributes)
-      expect(@attributes.size).to eq(0)
+      expect(@empty.size).to eq(0)
     end
 
     it "should return the number of pairs" do
@@ -1121,9 +1092,8 @@ describe Chef::Node::Attribute do
 
   describe "to_s" do
     it "should output simple attributes" do
-      @attributes = Chef::Node::Attribute.new(nil, nil, nil, nil, node)
-      allow(node).to receive(:attributes).and_return(@attributes)
-      expect(@attributes.to_s).to eq("{}")
+      attributes = Chef::Node::Attribute.new(nil, nil, nil, nil)
+      expect(attributes.to_s).to eq("{}")
     end
 
     it "should output merged attributes" do
@@ -1135,9 +1105,8 @@ describe Chef::Node::Attribute do
           "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}')
+      attributes = Chef::Node::Attribute.new(nil, default_hash, override_hash, nil)
+      expect(attributes.to_s).to eq('{"a"=>1, "b"=>3, "c"=>4}')
     end
   end
 
@@ -1304,19 +1273,4 @@ describe Chef::Node::Attribute do
       expect { @attributes["foo"]["bar"][0] << "buzz" }.to raise_error(RuntimeError, "can't modify frozen String")
     end
   end
-
-  describe "assigning lazy ungenerated caches to other attributes" do
-    it "works with arrays" do
-      @attributes.default["foo"]["baz"] = %w{one two}
-      @attributes.default["bar"]["baz"] = @attributes["foo"]["baz"]
-      expect(@attributes.default["bar"]["baz"]).to eql(%w{one two})
-    end
-
-    it "works with hashes" do
-      @attributes.default["foo"]["baz"] = { "one" => "two" }
-      @attributes.default["bar"]["baz"] = @attributes["foo"]["baz"]
-      expect(@attributes.default["bar"]["baz"]).to eql({ "one" => "two" })
-    end
-  end
-
 end
diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb
index 96adedddcb..2d3392041c 100644
--- a/spec/unit/node/immutable_collections_spec.rb
+++ b/spec/unit/node/immutable_collections_spec.rb
@@ -20,18 +20,13 @@ require "spec_helper"
 require "chef/node/immutable_collections"
 
 describe Chef::Node::ImmutableMash do
-
   before do
-    @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" } },
-                 },
+    @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" } },
     }
-    @node = Chef::Node.new()
-    @node.attributes.default = @data_in
-    @immutable_mash = @node["key"]
+    @immutable_mash = Chef::Node::ImmutableMash.new(@data_in)
   end
 
   it "element references like regular hash" do
@@ -62,9 +57,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
-    @node.attributes.default = { test: "foo", "test2" => "bar" }
-    expect(@node[:test]).to eql("foo")
-    expect(@node["test2"]).to eql("bar")
+    @mash = Chef::Node::ImmutableMash.new({ test: "foo", "test2" => "bar" })
+    expect(@mash[:test]).to eql("foo")
+    expect(@mash["test2"]).to eql("bar")
   end
 
   describe "to_hash" do
@@ -85,7 +80,7 @@ describe Chef::Node::ImmutableMash do
     end
 
     it "should create a mash with the same content" do
-      expect(@copy).to eql(@immutable_mash)
+      expect(@copy).to eq(@immutable_mash)
     end
 
     it "should allow mutation" do
@@ -180,11 +175,9 @@ end
 describe Chef::Node::ImmutableArray do
 
   before do
-    @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"]
+    @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])
   end
 
   ##
@@ -256,7 +249,7 @@ describe Chef::Node::ImmutableArray do
     end
 
     it "should create an array with the same content" do
-      expect(@immutable_nested_array).to eq(@copy)
+      expect(@copy).to eq(@immutable_nested_array)
     end
 
     it "should allow mutation" do
@@ -308,7 +301,7 @@ describe Chef::Node::ImmutableArray do
     end
 
     it "should create an array with the same content" do
-      expect(@immutable_nested_array).to eq(@copy)
+      expect(@copy).to eq(@immutable_nested_array)
     end
 
     it "should allow mutation" do
@@ -321,11 +314,4 @@ 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 651cfa0058..4898c22380 100644
--- a/spec/unit/node/vivid_mash_spec.rb
+++ b/spec/unit/node/vivid_mash_spec.rb
@@ -1,5 +1,5 @@
 #
-# Copyright:: Copyright 2016-2017, Chef Software Inc.
+# Copyright:: Copyright 2016, 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(no_args)
+      expect(root).to receive(:reset_cache).with("foo")
       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(no_args)
+      expect(root).to receive(:reset_cache).with("foo")
       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(no_args)
+      expect(root).to receive(:reset_cache).with("foo")
       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,55 +184,42 @@ 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", "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")
       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", "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")
+      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
-      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
@@ -253,10 +240,6 @@ 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
@@ -312,7 +295,7 @@ describe Chef::Node::VividMash do
     end
 
     it "should unlink hashes" do
-      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
+      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
@@ -324,7 +307,7 @@ describe Chef::Node::VividMash do
     end
 
     it "should unlink nil" 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(vivid.unlink("nil")).to eql(nil)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] })
     end
@@ -344,7 +327,7 @@ describe Chef::Node::VividMash do
     end
 
     it "should unlink! hashes" do
-      expect(root).to receive(:reset_cache).at_least(:once).with(no_args)
+      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
@@ -356,7 +339,7 @@ describe Chef::Node::VividMash do
     end
 
     it "should unlink! nil" 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(vivid.unlink!("nil")).to eql(nil)
       expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] })
     end