CHEF-3688 remove stale attribute read protection

This commit removes stale attribute read protection, as it does more
harm than good. Includes removal of passing "root" around to
immutablize.

4 files changed, 13 insertions, 423 deletions

diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index 6b7f6792ff..9da96886a5 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -66,8 +66,6 @@ class Chef
         :@force_override
       ]
 
-      attr_reader :serial_number
-
       [:all?,
        :any?,
        :assoc,
@@ -190,7 +188,6 @@ class Chef
        attr_reader :automatic
 
        def initialize(normal, default, override, automatic)
-         @serial_number = 0
          @set_unless_present = false
 
          @default = VividMash.new(self, default)
@@ -218,11 +215,8 @@ class Chef
        end
 
        # Clears merged_attributes, which will cause it to be recomputed on the
-       # next access. Additionally, increments the serial_number, which is used
-       # by the implementation of merged_attributes to detect reads from a
-       # stale merged attribute collection.
+       # next access. 
        def reset_cache
-         @serial_number += 1
          @merged_attributes = nil
          @combined_default  = nil
          @combined_override = nil
@@ -294,7 +288,7 @@ class Chef
                                     component_value = instance_variable_get(component_ivar)
                                     Chef::Mixin::DeepMerge.merge(merged, component_value)
                                   end
-                                  immutablize(self, resolved_attrs)
+                                  immutablize(resolved_attrs)
                                 end
        end
 
@@ -304,7 +298,7 @@ class Chef
                                     component_value = instance_variable_get(component_ivar)
                                     Chef::Mixin::DeepMerge.merge(merged, component_value)
                                   end
-                                  immutablize(self, resolved_attrs)
+                                  immutablize(resolved_attrs)
                                 end
        end
 
@@ -314,7 +308,7 @@ class Chef
                                     component_value = instance_variable_get(component_ivar)
                                     Chef::Mixin::DeepMerge.merge(merged, component_value)
                                   end
-                                  immutablize(self, resolved_attrs)
+                                  immutablize(resolved_attrs)
                                 end
        end
 
@@ -364,10 +358,6 @@ class Chef
          @set_unless_present
        end
 
-       def stale_subtree?(serial_number)
-         serial_number != @serial_number
-       end
-
     end
 
   end
diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb
index 857365d8d6..f5b3a5121d 100644
--- a/lib/chef/node/immutable_collections.rb
+++ b/lib/chef/node/immutable_collections.rb
@@ -3,12 +3,12 @@ class Chef
   class Node
 
     module Immutablize
-      def immutablize(root, value)
+      def immutablize(value)
         case value
         when Hash
-          ImmutableMash.new(root, value)
+          ImmutableMash.new(value)
         when Array
-          ImmutableArray.new(root, value)
+          ImmutableArray.new(value)
         else
           value
         end
@@ -28,8 +28,6 @@ class Chef
     class ImmutableArray < Array
       include Immutablize
 
-      attr_reader :root
-
       alias :internal_push :<<
       private :internal_push
 
@@ -68,104 +66,9 @@ class Chef
         :unshift
       ]
 
-      # A list of methods that read values from the Array. Each of these is
-      # overridden to verify that the Chef::Node::Attribute object that this
-      # object belongs to has not been modified since the value was computed.
-      READER_METHODS =
-      [
-        :&,
-        :*,
-        :+,
-        :-,
-        :[],
-        :all?,
-        :any?,
-        :assoc,
-        :at,
-        :chunk,
-        :collect,
-        :collect_concat,
-        :combination,
-        :compact,
-        :concat,
-        :count,
-        :cycle,
-        :detect,
-        :drop,
-        :drop_while,
-        :each,
-        :each_cons,
-        :each_entry,
-        :each_index,
-        :each_slice,
-        :each_with_index,
-        :each_with_object,
-        :empty?,
-        :entries,
-        :fetch,
-        :find,
-        :find_all,
-        :find_index,
-        :first,
-        :flat_map,
-        :flatten,
-        :grep,
-        :group_by,
-        :include?,
-        :index,
-        :inject,
-        :join,
-        :last,
-        :length,
-        :map,
-        :max,
-        :max_by,
-        :member?,
-        :min,
-        :min_by,
-        :minmax,
-        :minmax_by,
-        :none?,
-        :one?,
-        :pack,
-        :partition,
-        :permutation,
-        :product,
-        :rassoc,
-        :reduce,
-        :reject,
-        :repeated_combination,
-        :repeated_permutation,
-        :reverse,
-        :reverse_each,
-        :rindex,
-        :rotate,
-        :sample,
-        :select,
-        :shelljoin,
-        :shuffle,
-        :size,
-        :slice,
-        :slice_before,
-        :sort,
-        :sort_by,
-        :take,
-        :take_while,
-        :to_a,
-        :to_ary,
-        :to_set,
-        :transpose,
-        :uniq,
-        :values_at,
-        :zip,
-        :|
-      ]
-
-      def initialize(root, array_data)
-        @root = root
-        @serial_number = root.serial_number
+      def initialize(array_data)
         array_data.each do |value|
-          internal_push(immutablize(root, value))
+          internal_push(immutablize(value))
         end
       end
 
@@ -182,18 +85,6 @@ class Chef
         METHOD_DEFN
       end
 
-      READER_METHODS.each do |reader|
-        class_eval(<<-METHOD_DEFN, __FILE__, __LINE__)
-          def #{reader}(*args, &block)
-            if root.stale_subtree?(@serial_number)
-              raise Exceptions::StaleAttributeRead,
-                "Node attributes have been modified since this value was read. Get an updated value by reading from node, e.g., `node[:key]`"
-            end
-            super
-          end
-        METHOD_DEFN
-      end
-
       def dup
         Array.new(map {|e| e.dup })
       end
@@ -216,8 +107,6 @@ class Chef
 
       include Immutablize
 
-      attr_reader :root
-
       alias :internal_set :[]=
       private :internal_set
 
@@ -239,89 +128,9 @@ class Chef
         :shift
       ]
 
-      READER_METHODS = [
-        :[],
-        :all?,
-        :any?,
-        :assoc,
-        :chunk,
-        :collect,
-        :collect_concat,
-        :count,
-        :cycle,
-        :detect,
-        :drop,
-        :drop_while,
-        :each,
-        :each_cons,
-        :each_entry,
-        :each_key,
-        :each_pair,
-        :each_slice,
-        :each_value,
-        :each_with_index,
-        :each_with_object,
-        :empty?,
-        :entries,
-        :except,
-        :fetch,
-        :find,
-        :find_all,
-        :find_index,
-        :first,
-        :flat_map,
-        :flatten,
-        :grep,
-        :group_by,
-        :has_key?,
-        :has_value?,
-        :include?,
-        :index,
-        :inject,
-        :invert,
-        :key,
-        :key?,
-        :keys,
-        :length,
-        :map,
-        :max,
-        :max_by,
-        :member?,
-        :merge,
-        :min,
-        :min_by,
-        :minmax,
-        :minmax_by,
-        :none?,
-        :one?,
-        :partition,
-        :rassoc,
-        :reduce,
-        :reject,
-        :reverse_each,
-        :select,
-        :size,
-        :slice_before,
-        :sort,
-        :sort_by,
-        :store,
-        :symbolize_keys,
-        :take,
-        :take_while,
-        :to_a,
-        :to_hash,
-        :to_set,
-        :value?,
-        :values,
-        :values_at,
-        :zip
-      ]
-
-      def initialize(root, mash_data)
-        @serial_number = root.serial_number
-        @root = root
+      def initialize(mash_data)
         mash_data.each do |key, value|
-          internal_set(key, immutablize(root, value))
+          internal_set(key, immutablize(value))
         end
       end
 
@@ -340,18 +149,6 @@ class Chef
         METHOD_DEFN
       end
 
-      READER_METHODS.each do |reader_method|
-        class_eval(<<-METHOD_DEFN, __FILE__, __LINE__)
-          def #{reader_method}(*args, &block)
-            if root.stale_subtree?(@serial_number)
-              raise Exceptions::StaleAttributeRead,
-                "Node attributes have been modified since this value was read. Get an updated value by reading from node, e.g., `node[:key]`"
-            end
-            super
-          end
-        METHOD_DEFN
-      end
-
       def method_missing(symbol, *args)
         if args.empty?
           if key?(symbol)
diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb
index d568d6fc15..947de76b1d 100644
--- a/spec/unit/node/attribute_spec.rb
+++ b/spec/unit/node/attribute_spec.rb
@@ -1086,200 +1086,5 @@ describe Chef::Node::Attribute do
 
   end
 
-
-  describe "when reading from a stale sub tree" do
-    before do
-      @attributes.default[:sub_tree] = {:key => "old value", :ary => %w[foo bar]}
-      @sub_tree = @attributes[:sub_tree]
-      @sub_array = @attributes[:sub_tree][:ary]
-      @attributes.default[:sub_tree] = {:key => "new value"}
-    end
-
-    it "detects reads from a no-longer-valid merged attributes sub-tree" do
-      lambda { @sub_tree[:key] }.should raise_error(Chef::Exceptions::StaleAttributeRead)
-    end
-
-    it "detects reads from a no-longer-valid array value" do
-      lambda {@sub_array.first}.should raise_error(Chef::Exceptions::StaleAttributeRead)
-    end
-    [
-      :[],
-      :all?,
-      :any?,
-      :assoc,
-      :chunk,
-      :collect,
-      :collect_concat,
-      :count,
-      :cycle,
-      :detect,
-      :drop,
-      :drop_while,
-      :each,
-      :each_cons,
-      :each_entry,
-      :each_key,
-      :each_pair,
-      :each_slice,
-      :each_value,
-      :each_with_index,
-      :each_with_object,
-      :empty?,
-      :entries,
-      :except,
-      :fetch,
-      :find,
-      :find_all,
-      :find_index,
-      :first,
-      :flat_map,
-      :flatten,
-      :grep,
-      :group_by,
-      :has_key?,
-      :has_value?,
-      :include?,
-      :index,
-      :inject,
-      :invert,
-      :key,
-      :key?,
-      :keys,
-      :length,
-      :map,
-      :max,
-      :max_by,
-      :member?,
-      :merge,
-      :min,
-      :min_by,
-      :minmax,
-      :minmax_by,
-      :none?,
-      :one?,
-      :partition,
-      :rassoc,
-      :reduce,
-      :reject,
-      :reverse_each,
-      :select,
-      :size,
-      :slice_before,
-      :sort,
-      :sort_by,
-      :store,
-      :symbolize_keys,
-      :take,
-      :take_while,
-      :to_a,
-      :to_hash,
-      :to_set,
-      :value?,
-      :values,
-      :values_at,
-      :zip
-    ].each do |reader|
-      it "detects dirty reads from a no-longer-valid Mash via Mash##{reader}" do
-        lambda { @sub_tree.send(:reader) }.should raise_error(Chef::Exceptions::StaleAttributeRead)
-      end
-    end
-
-
-    [
-      :&,
-      :*,
-      :+,
-      :-,
-      :[],
-      :all?,
-      :any?,
-      :assoc,
-      :at,
-      :chunk,
-      :collect,
-      :collect_concat,
-      :combination,
-      :compact,
-      :concat,
-      :count,
-      :cycle,
-      :detect,
-      :drop,
-      :drop_while,
-      :each,
-      :each_cons,
-      :each_entry,
-      :each_index,
-      :each_slice,
-      :each_with_index,
-      :each_with_object,
-      :empty?,
-      :entries,
-      :fetch,
-      :find,
-      :find_all,
-      :find_index,
-      :first,
-      :flat_map,
-      :flatten,
-      :grep,
-      :group_by,
-      :include?,
-      :index,
-      :inject,
-      :join,
-      :last,
-      :length,
-      :map,
-      :max,
-      :max_by,
-      :member?,
-      :min,
-      :min_by,
-      :minmax,
-      :minmax_by,
-      :none?,
-      :one?,
-      :pack,
-      :partition,
-      :permutation,
-      :product,
-      :rassoc,
-      :reduce,
-      :reject,
-      :repeated_combination,
-      :repeated_permutation,
-      :reverse,
-      :reverse_each,
-      :rindex,
-      :rotate,
-      :sample,
-      :select,
-      :shelljoin,
-      :shuffle,
-      :size,
-      :slice,
-      :slice_before,
-      :sort,
-      :sort_by,
-      :take,
-      :take_while,
-      :to_a,
-      :to_ary,
-      :to_set,
-      :transpose,
-      :uniq,
-      :values_at,
-      :zip,
-      :|
-    ].each do |reader|
-
-        it "detects dirty reads via Array##{reader}" do
-          lambda {@sub_array.send(reader)}.should raise_error(Chef::Exceptions::StaleAttributeRead)
-        end
-      end
-
-  end
-
 end
 
diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb
index 4ad5313415..db4a79f21b 100644
--- a/spec/unit/node/immutable_collections_spec.rb
+++ b/spec/unit/node/immutable_collections_spec.rb
@@ -21,13 +21,12 @@ require "chef/node/immutable_collections"
 
 describe Chef::Node::ImmutableMash do
   before do
-    @root = Chef::Node::Attribute.new({}, {}, {}, {})
     @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"}}
     }
-    @immutable_mash = Chef::Node::ImmutableMash.new(@root, @data_in)
+    @immutable_mash = Chef::Node::ImmutableMash.new(@data_in)
   end
 
   it "element references like regular hash" do
@@ -87,8 +86,7 @@ end
 describe Chef::Node::ImmutableArray do
 
   before do
-    @root = Chef::Node::Attribute.new({}, {}, {}, {})
-    @immutable_array = Chef::Node::ImmutableArray.new(@root, %w[foo bar baz])
+    @immutable_array = Chef::Node::ImmutableArray.new(%w[foo bar baz])
   end
 
   ##