Revert "Per-container deep merge caching"lcg/revert-lazy-attributes

This reverts commit 0c29acc106ca774e230c8ef45694c8bffd166b69.

5 files changed, 105 insertions, 198 deletions

diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index debb3eeaef..cd6061e24f 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -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,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)
@@ -205,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
@@ -230,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
@@ -310,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)
@@ -357,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
@@ -421,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)
@@ -494,14 +471,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}"
@@ -510,14 +479,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"]
        #
@@ -547,6 +509,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.
@@ -618,6 +608,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 6922bcf200..2836473b1d 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
 
@@ -147,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
@@ -174,7 +174,7 @@ class Chef
 
       def []=(key, value)
         ret = super
-        send_reset_cache(__path__)
+        send_reset_cache(__path__, key)
         ret # rubocop:disable Lint/Void
       end
 
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 58ce58e61f..c12ee33268 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,50 +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
-          puts array.class
-          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...
@@ -125,55 +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
-        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
@@ -196,30 +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
-
       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?
@@ -255,39 +168,11 @@ class Chef
 
       alias_method :to_hash, :to_h
 
-      def [](key)
-        ensure_generated_cache!
-        super
-      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 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.each_key do |key|
-                next if internal_key?(key)
-                internal_set(key, subhash[key])
-              end
-            else
-              break
-            end
-          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 690d261df6..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-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");
@@ -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)