Merge pull request #2753 from opscode/lcg/deep-merge-cache-fix

deep_merge_cache fixes for bugs in 12.0.0

3 files changed, 36 insertions, 4 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d39d0b7820..bf7c3d9dc3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,6 +29,7 @@
 * shell_out now sets `LANGUAGE` and `LANG` to the `Chef::Config[:internal_locale]` in addition to `LC_ALL` forcing
 * chef_gem supports a compile_time flag and will warn if it is not set (behavior will change in the future)
 * suppress 3694 warnings on the most trivial resource cloning
+* fixed bugs in the deep_merge_cache logic introduced in 12.0.0 around `node['foo']` vs `node[:foo]` vs. `node.foo`
 
 ## 12.0.3
 * [**Phil Dibowitz**](https://github.com/jaymzh):
diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 3c48f653eb..80f5ac4f8d 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -253,7 +253,7 @@ class Chef
          if path.nil?
            @deep_merge_cache = {}
          else
-           deep_merge_cache.delete(path)
+           deep_merge_cache.delete(path.to_s)
          end
        end
 
@@ -436,12 +436,12 @@ class Chef
        end
 
        def [](key)
-         if deep_merge_cache.has_key?(key)
+         if deep_merge_cache.has_key?(key.to_s)
            # return the cache of the deep merged values by top-level key
-           deep_merge_cache[key]
+           deep_merge_cache[key.to_s]
          else
            # save all the work of computing node[key]
-           deep_merge_cache[key] = merged_attributes(key)
+           deep_merge_cache[key.to_s] = merged_attributes(key)
          end
        end
 
diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb
index 0bc76db272..5939403ce6 100644
--- a/spec/unit/node_spec.rb
+++ b/spec/unit/node_spec.rb
@@ -611,6 +611,37 @@ describe Chef::Node do
       end
     end
 
+    # In Chef-12.0 there is a deep_merge cache on the top level attribute which had a bug
+    # where it cached node[:foo] separate from node['foo'].  These tests exercise those edge conditions.
+    #
+    # https://github.com/opscode/chef/issues/2700
+    # https://github.com/opscode/chef/issues/2712
+    # https://github.com/opscode/chef/issues/2745
+    #
+    describe "deep merge attribute cache edge conditions" do
+      it "does not error with complicated attribute substitution" do
+        node.default['chef_attribute_hell']['attr1'] = "attribute1"
+        node.default['chef_attribute_hell']['attr2'] = "#{node.chef_attribute_hell.attr1}/attr2"
+        expect { node.default['chef_attribute_hell']['attr3'] = "#{node.chef_attribute_hell.attr2}/attr3" }.not_to raise_error
+      end
+
+      it "caches both strings and symbols correctly" do
+        node.force_default[:solr][:version] = '4.10.2'
+        node.force_default[:solr][:data_dir] = "/opt/solr-#{node['solr'][:version]}/example/solr"
+        node.force_default[:solr][:xms] = "512M"
+        expect(node[:solr][:xms]).to eql("512M")
+        expect(node['solr'][:xms]).to eql("512M")
+      end
+
+      it "method interpolation syntax also works" do
+        node.default['passenger']['version']     = '4.0.57'
+        node.default['passenger']['root_path']   = "passenger-#{node['passenger']['version']}"
+        node.default['passenger']['root_path_2'] = "passenger-#{node.passenger['version']}"
+        expect(node['passenger']['root_path_2']).to eql("passenger-4.0.57")
+        expect(node[:passenger]['root_path_2']).to eql("passenger-4.0.57")
+      end
+    end
+
     it "should raise an ArgumentError if you ask for an attribute that doesn't exist via method_missing" do
       expect { node.sunshine }.to raise_error(NoMethodError)
     end