de-lazify the deep merge cache

that turns out to be too clever, we can have cache invalidation
at intermediate levels, but we can't have lazily created caches.

we may be able to use decorators--only around hashes and arrays--
in order to lazily the containers once again, but that will be
a deeper change.

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

7 files changed, 38 insertions, 25 deletions

diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index 7b530e1132..0a96787849 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -77,8 +77,6 @@ class Chef
       @policy_name = nil
       @policy_group = nil
 
-      @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, self)
-
       @run_state = {}
     end
 
@@ -182,7 +180,7 @@ class Chef
     end
 
     def attributes
-      @attributes
+      @attributes ||= Chef::Node::Attribute.new({}, {}, {}, {}, self)
     end
 
     alias :attribute :attributes
diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 142150aeef..1b92421175 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -185,9 +185,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,10 +200,13 @@ class Chef
 
         @automatic      = VividMash.new(automatic, self, node, :automatic)
 
-        @deep_merge_cache = ImmutableMash.new({}, self, node, :merged)
         @__node__ = node
       end
 
+      def deep_merge_cache
+        @deep_merge_cache ||= ImmutableMash.new({}, self, __node__, :merged)
+      end
+
        # Debug what's going on with an attribute. +args+ is a path spec to the
        # attribute you're interested in. For example, to debug where the value
        # of `node[:network][:default_interface]` is coming from, use:
@@ -229,7 +229,7 @@ class Chef
       end
 
       def reset
-        @deep_merge_cache = ImmutableMash.new({}, self, @__node__, :merged)
+        @deep_merge_cache = nil
       end
 
       def reset_cache(*path)
@@ -493,11 +493,11 @@ class Chef
       end
 
       def [](key)
-        @deep_merge_cache[key]
+        deep_merge_cache[key]
       end
 
       def merged_attributes
-        @deep_merge_cache
+        deep_merge_cache
       end
 
       def inspect
diff --git a/lib/chef/node/attribute_collections.rb b/lib/chef/node/attribute_collections.rb
index 0271febea5..28047ba22d 100644
--- a/lib/chef/node/attribute_collections.rb
+++ b/lib/chef/node/attribute_collections.rb
@@ -69,8 +69,9 @@ class Chef
       end
 
       def delete(key, &block)
+        ret = super
         send_reset_cache(__path__)
-        super
+        ret
       end
 
       def initialize(data = [])
@@ -148,14 +149,16 @@ class Chef
       # object.
 
       def delete(key, &block)
+        ret = super
         send_reset_cache(__path__)
-        super
+        ret
       end
 
       MUTATOR_METHODS.each do |mutator|
         define_method(mutator) do |*args, &block|
+          ret = super(*args, &block)
           send_reset_cache(__path__)
-          super(*args, &block)
+          ret
         end
       end
 
diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb
index dc8ead9176..8d1a97aacc 100644
--- a/lib/chef/node/immutable_collections.rb
+++ b/lib/chef/node/immutable_collections.rb
@@ -95,7 +95,7 @@ class Chef
       end
 
       def initialize(array_data = [])
-        # Immutable collections no longer have initialized state
+        ensure_generated_cache!
       end
 
       # For elements like Fixnums, true, nil...
@@ -130,9 +130,9 @@ class Chef
       end
 
       def reset
-        @generated_cache = false
         @short_circuit_attr_level = nil
-        internal_clear # redundant?
+        generate_cache
+        @generated_cache = true
       end
 
       # @api private
@@ -245,7 +245,7 @@ class Chef
       end
 
       def initialize(mash_data = {})
-        # Immutable collections no longer have initialized state
+        ensure_generated_cache!
       end
 
       alias :attribute? :has_key?
@@ -287,9 +287,8 @@ class Chef
       end
 
       def reset
-        @generated_cache = false
         @short_circuit_attr_level = nil
-        internal_clear # redundant?
+        generate_cache
       end
 
       # @api private
diff --git a/lib/chef/node/mixin/state_tracking.rb b/lib/chef/node/mixin/state_tracking.rb
index 0c3d4c33a4..442ed7a154 100644
--- a/lib/chef/node/mixin/state_tracking.rb
+++ b/lib/chef/node/mixin/state_tracking.rb
@@ -25,14 +25,12 @@ class Chef
         attr_reader :__precedence__
 
         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)
+          @__node__ = node
+          @__precedence__ = precedence
           @__path__ = path
           @__path__ ||= []
           @__root__ = root
-          @__node__ = node
-          @__precedence__ = precedence
+          data.nil? ? super() : super(data)
         end
 
         def [](*args)
diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb
index 42cdf2a9ce..97483524ae 100644
--- a/spec/unit/node/attribute_spec.rb
+++ b/spec/unit/node/attribute_spec.rb
@@ -1318,5 +1318,4 @@ describe Chef::Node::Attribute do
       expect(@attributes.default["bar"]["baz"]).to eql({ "one" => "two" })
     end
   end
-
 end
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index 2019f1ac42..4ab8e5d372 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -1809,4 +1809,20 @@ describe Chef::Node do
       expect(node["a"]["key"]).to eql(1)
     end
   end
+
+  describe "lazy ungenerated caches work with ruby internal APIs" do
+    it "works with hashes when first created" do
+      node.default["foo"] = { "one" => "two" }
+      result = {}.merge!(node["foo"])
+      expect(result).to eql({ "one" => "two" })
+    end
+
+    it "works with hashes that receive a reset" do
+      node.default["foo"] = { "one" => "two" }
+      node["foo"].to_h
+      node.default["foo"]["bar"] = "baz"
+      result = {}.merge!(node["foo"])
+      expect(result).to eql({ "one" => "two", "bar" => "baz" })
+    end
+  end
 end