Attributes v1.1 changeslcg/attributes-v1.1

- fixes *_unless behavior and set_unless_value_present hack from Chef 12
- simplifies rm_* code
- introduces functional read/write/unlink/exist? API
- deprecates method_missing access to attributes for Chef 13
- deprecates set/set_unless aliases for Chef 14
- removes MultiMash mess that I wrote for Chef 13

https://github.com/chef/chef/pull/5029 for more details

4 files changed, 283 insertions, 202 deletions

diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb
index ab97cf99bf..09c78c6de6 100644
--- a/lib/chef/node/attribute.rb
+++ b/lib/chef/node/attribute.rb
@@ -19,6 +19,7 @@
 
 require "chef/node/immutable_collections"
 require "chef/node/attribute_collections"
+require "chef/decorator/unchain"
 require "chef/mixin/deep_merge"
 require "chef/log"
 
@@ -132,6 +133,7 @@ class Chef
        :take,
        :take_while,
        :to_a,
+       :to_h,
        :to_hash,
        :to_set,
        :value?,
@@ -187,8 +189,6 @@ class Chef
       attr_accessor :deep_merge_cache
 
       def initialize(normal, default, override, automatic)
-        @set_unless_present = false
-
         @default = VividMash.new(self, default)
         @env_default = VividMash.new(self, {})
         @role_default = VividMash.new(self, {})
@@ -214,15 +214,13 @@ class Chef
        # attribute you're interested in. For example, to debug where the value
        # of `node[:network][:default_interface]` is coming from, use:
        #   debug_value(:network, :default_interface).
-       # The return value is an Array of Arrays. The first element is
-       # `["set_unless_enabled?", Boolean]`, which describes whether the
-       # attribute collection is in "set_unless" mode. The rest of the Arrays
+       # The return value is an Array of Arrays.  The Arrays
        # are pairs of `["precedence_level", value]`, where precedence level is
        # the component, such as role default, normal, etc. and value is the
        # attribute value set at that precedence level. If there is no value at
        # that precedence level, +value+ will be the symbol +:not_present+.
       def debug_value(*args)
-        components = COMPONENTS.map do |component|
+        COMPONENTS.map do |component|
           ivar = instance_variable_get(component)
           value = args.inject(ivar) do |so_far, key|
             if so_far == :not_present
@@ -235,12 +233,6 @@ class Chef
           end
           [component.to_s.sub(/^@/, ""), value]
         end
-        [["set_unless_enabled?", @set_unless_present]] + components
-      end
-
-       # Enables or disables `||=`-like attribute setting. See, e.g., Node#set_unless
-      def set_unless_value_present=(setting)
-        @set_unless_present = setting
       end
 
        # Invalidate a key in the deep_merge_cache.  If called with nil, or no arg, this will invalidate
@@ -321,94 +313,134 @@ class Chef
 
        # clears attributes from all precedence levels
       def rm(*args)
-        reset(args[0])
-        # just easier to compute our retval, rather than collect+merge sub-retvals
-        ret = args.inject(merged_attributes) do |attr, arg|
-          if attr.nil? || !attr.respond_to?(:[])
-            nil
-          else
-            begin
-              attr[arg]
-            rescue TypeError
-              raise TypeError, "Wrong type in index of attribute (did you use a Hash index on an Array?)"
-            end
-          end
+        with_deep_merged_return_value(self, *args) do
+          rm_default(*args)
+          rm_normal(*args)
+          rm_override(*args)
         end
-        rm_default(*args)
-        rm_normal(*args)
-        rm_override(*args)
-        ret
       end
 
-       # does <level>['foo']['bar'].delete('baz')
-      def remove_from_precedence_level(level, *args, key)
-        multimash = level.element(*args)
-        multimash.nil? ? nil : multimash.delete(key)
-      end
-
-      private :remove_from_precedence_level
-
        # clears attributes from all default precedence levels
        #
-       # equivalent to: force_default!['foo']['bar'].delete('baz')
+       # similar to: force_default!['foo']['bar'].delete('baz')
+       # - does not autovivify
+       # - does not trainwreck if interior keys do not exist
       def rm_default(*args)
-        reset(args[0])
-        remove_from_precedence_level(force_default!(autovivify: false), *args)
+        with_deep_merged_return_value(combined_default, *args) do
+          default.unlink(*args)
+          role_default.unlink(*args)
+          env_default.unlink(*args)
+          force_default.unlink(*args)
+        end
       end
 
        # clears attributes from normal precedence
        #
        # equivalent to: normal!['foo']['bar'].delete('baz')
+       # - does not autovivify
+       # - does not trainwreck if interior keys do not exist
       def rm_normal(*args)
-        reset(args[0])
-        remove_from_precedence_level(normal!(autovivify: false), *args)
+        normal.unlink(*args)
       end
 
        # clears attributes from all override precedence levels
        #
        # equivalent to: force_override!['foo']['bar'].delete('baz')
+       # - does not autovivify
+       # - does not trainwreck if interior keys do not exist
       def rm_override(*args)
-        reset(args[0])
-        remove_from_precedence_level(force_override!(autovivify: false), *args)
+        with_deep_merged_return_value(combined_override, *args) do
+          override.unlink(*args)
+          role_override.unlink(*args)
+          env_override.unlink(*args)
+          force_override.unlink(*args)
+        end
+      end
+
+      def with_deep_merged_return_value(obj, *path, last)
+        hash = obj.read(*path)
+        return nil unless hash.is_a?(Hash)
+        ret = hash[last]
+        yield
+        ret
       end
 
+      private :with_deep_merged_return_value
+
        #
        # Replacing attributes without merging
        #
 
        # sets default attributes without merging
-      def default!(opts = {})
-        # FIXME: do not flush whole cache
-        reset
-        MultiMash.new(self, @default, [], opts)
+       #
+       # - this API autovivifies (and cannot trainwreck)
+      def default!(*args)
+        return Decorator::Unchain.new(self, :default!) unless args.length > 0
+        write(:default, *args)
       end
 
        # sets normal attributes without merging
-      def normal!(opts = {})
-        # FIXME: do not flush whole cache
-        reset
-        MultiMash.new(self, @normal, [], opts)
+       #
+       # - this API autovivifies (and cannot trainwreck)
+      def normal!(*args)
+        return Decorator::Unchain.new(self, :normal!) unless args.length > 0
+        write(:normal, *args)
       end
 
        # sets override attributes without merging
-      def override!(opts = {})
-        # FIXME: do not flush whole cache
-        reset
-        MultiMash.new(self, @override, [], opts)
+       #
+       # - this API autovivifies (and cannot trainwreck)
+      def override!(*args)
+        return Decorator::Unchain.new(self, :override!) unless args.length > 0
+        write(:override, *args)
       end
 
        # clears from all default precedence levels and then sets force_default
-      def force_default!(opts = {})
-        # FIXME: do not flush whole cache
-        reset
-        MultiMash.new(self, @force_default, [@default, @env_default, @role_default], opts)
+       #
+       # - this API autovivifies (and cannot trainwreck)
+      def force_default!(*args)
+        return Decorator::Unchain.new(self, :force_default!) unless args.length > 0
+        value = args.pop
+        rm_default(*args)
+        write(:force_default, *args, value)
       end
 
        # clears from all override precedence levels and then sets force_override
-      def force_override!(opts = {})
-        # FIXME: do not flush whole cache
-        reset
-        MultiMash.new(self, @force_override, [@override, @env_override, @role_override], opts)
+      def force_override!(*args)
+        return Decorator::Unchain.new(self, :force_override!) unless args.length > 0
+        value = args.pop
+        rm_override(*args)
+        write(:force_override, *args, value)
+      end
+
+      # method-style access to attributes
+
+      def read(*path)
+        merged_attributes.read(*path)
+      end
+
+      def read!(*path)
+        merged_attributes.read!(*path)
+      end
+
+      def exist?(*path)
+        merged_attributes.exist?(*path)
+      end
+
+      def write(level, *args, &block)
+        self.send(level).write(*args, &block)
+      end
+
+      def write!(level, *args, &block)
+        self.send(level).write!(*args, &block)
+      end
+
+      def unlink(level, *path)
+        self.send(level).unlink(*path)
+      end
+
+      def unlink!(level, *path)
+        self.send(level).unlink!(*path)
       end
 
        #
@@ -420,9 +452,9 @@ class Chef
        #
 
       def merged_attributes(*path)
-       # immutablize(
+        # immutablize(
         merge_all(path)
-       # )
+        # )
       end
 
       def combined_override(*path)
@@ -433,6 +465,27 @@ class Chef
         immutablize(merge_defaults(path))
       end
 
+      def normal_unless(*args)
+        return Decorator::Unchain.new(self, :normal_unless) unless args.length > 0
+        write(:normal, *args) if read(*args[0...-1]).nil?
+      end
+
+      def default_unless(*args)
+        return Decorator::Unchain.new(self, :default_unless) unless args.length > 0
+        write(:default, *args) if read(*args[0...-1]).nil?
+      end
+
+      def override_unless(*args)
+        return Decorator::Unchain.new(self, :override_unless) unless args.length > 0
+        write(:override, *args) if read(*args[0...-1]).nil?
+      end
+
+      def set_unless(*args)
+        Chef.log_deprecation("node.set_unless is deprecated and will be removed in Chef 14, please use node.default_unless/node.override_unless (or node.normal_unless if you really need persistence)")
+        return Decorator::Unchain.new(self, :default_unless) unless args.length > 0
+        write(:normal, *args) if read(*args[0...-1]).nil?
+      end
+
       def [](key)
         if deep_merge_cache.has_key?(key.to_s)
           # return the cache of the deep merged values by top-level key
@@ -461,13 +514,17 @@ class Chef
       alias :each_attribute :each
 
       def method_missing(symbol, *args)
-        if args.empty?
+        if symbol == :to_ary
+          merged_attributes.send(symbol, *args)
+        elsif args.empty?
+          Chef.log_deprecation %q{"method access to node attributes (node.foo.bar) is deprecated and will be removed in Chef 13, please use bracket syntax (node["foo"]["bar"])}
           if key?(symbol)
             self[symbol]
           else
             raise NoMethodError, "Undefined method or attribute `#{symbol}' on `node'"
           end
         elsif symbol.to_s =~ /=$/
+          Chef.log_deprecation %q{"method setting of node attributes (node.foo="bar") is deprecated and will be removed in Chef 13, please use bracket syntax (node["foo"]="bar")}
           key_to_set = symbol.to_s[/^(.+)=$/, 1]
           self[key_to_set] = (args.length == 1 ? args[0] : args)
         else
@@ -485,10 +542,6 @@ class Chef
         }.join(", ") << ">"
       end
 
-      def set_unless?
-        @set_unless_present
-      end
-
        private
 
        # Helper method for merge_all/merge_defaults/merge_overrides.
diff --git a/lib/chef/node/attribute_collections.rb b/lib/chef/node/attribute_collections.rb
index 68f3a69756..b739ea8490 100644
--- a/lib/chef/node/attribute_collections.rb
+++ b/lib/chef/node/attribute_collections.rb
@@ -16,15 +16,15 @@
 # limitations under the License.
 #
 
+require "chef/node/common_api"
+
 class Chef
   class Node
-
     # == AttrArray
     # AttrArray is identical to Array, except that it keeps a reference to the
     # "root" (Chef::Node::Attribute) object, and will trigger a cache
     # invalidation on that object when mutated.
     class AttrArray < Array
-
       MUTATOR_METHODS = [
         :<<,
         :[]=,
@@ -62,8 +62,9 @@ class Chef
       # Node::Attribute object.
       MUTATOR_METHODS.each do |mutator|
         define_method(mutator) do |*args, &block|
+          ret = super(*args, &block)
           root.reset_cache(root.top_level_breadcrumb)
-          super(*args, &block)
+          ret
         end
       end
 
@@ -96,14 +97,12 @@ class Chef
     #   in the creation of a new VividMash for that key. (This only works when
     #   using the element reference method, `[]` -- other methods, such as
     #   #fetch, work as normal).
-    # * It supports a set_unless flag (via the root Attribute object) which
-    #   allows `||=` style behavior (`||=` does not work with
-    #   auto-vivification). This is only implemented for #[]=; methods such as
-    #   #store work as normal.
     # * attr_accessor style element set and get are supported via method_missing
     class VividMash < Mash
       attr_reader :root
 
+      include CommonAPI
+
       # Methods that mutate a VividMash. Each of them is overridden so that it
       # also invalidates the cached merged_attributes on the root Attribute
       # object.
@@ -148,12 +147,9 @@ class Chef
 
       def []=(key, value)
         root.top_level_breadcrumb ||= key
-        if set_unless? && key?(key) && !self[key].nil?
-          self[key]
-        else
-          root.reset_cache(root.top_level_breadcrumb)
-          super
-        end
+        ret = super
+        root.reset_cache(root.top_level_breadcrumb)
+        ret
       end
 
       alias :attribute? :has_key?
@@ -176,10 +172,6 @@ class Chef
         end
       end
 
-      def set_unless?
-        @root.set_unless?
-      end
-
       def convert_key(key)
         super
       end
@@ -206,118 +198,5 @@ class Chef
       end
 
     end
-
-    # == MultiMash
-    # This is a Hash-like object that contains multiple VividMashes in it.  Its
-    # purpose is so that the user can descend into the mash and delete a subtree
-    # from all of the Mash objects (used to delete all values in a subtree from
-    # default, force_default, role_default and env_default at the same time).  The
-    # assignment operator strictly does assignment (does no merging) and works
-    # by deleting the subtree and then assigning to the last mash which passed in
-    # the initializer.
-    #
-    # A lot of the complexity of this class comes from the fact that at any key
-    # value some or all of the mashes may walk off their ends and become nil or
-    # true or something.  The schema may change so that one precidence leve may
-    # be 'true' object and another may be a VividMash.  It is also possible that
-    # one or many of them may transition from VividMashes to Hashes or Arrays.
-    #
-    # It also supports the case where you may be deleting a key using node.rm
-    # in which case if intermediate keys all walk off into nil then you don't want
-    # to be autovivifying keys as you go.  On the other hand you may be using
-    # node.force_default! in which case you'll wind up with a []= operator at the
-    # end and you want autovivification, so we conditionally have to support either
-    # operation.
-    #
-    # @todo: can we have an autovivify class that decorates a class that doesn't
-    # autovivify or something so that the code is less awful?
-    #
-    class MultiMash
-      attr_reader :root
-      attr_reader :mashes
-      attr_reader :opts
-      attr_reader :primary_mash
-
-      # Initialize with an array of mashes.  For the delete return value to work
-      # properly the mashes must come from the same attribute level (i.e. all
-      # override or all default, but not a mix of both).
-      def initialize(root, primary_mash, mashes, opts = {})
-        @root = root
-        @primary_mash = primary_mash
-        @mashes = mashes
-        @opts = opts
-        @opts[:autovivify] = true if @opts[:autovivify].nil?
-      end
-
-      def [](key)
-        # handle the secondary mashes
-        new_mashes = []
-        mashes.each do |mash|
-          new_mash = safe_evalute_key(mash, key)
-          # secondary mashes never autovivify so once they fall into nil, we just stop tracking them
-          new_mashes.push(new_mash) unless new_mash.nil?
-        end
-
-        new_primary_mash = safe_evalute_key(primary_mash, key)
-
-        if new_primary_mash.nil? && @opts[:autovivify]
-          primary_mash[key] = VividMash.new(root)
-          new_primary_mash = primary_mash[key]
-        end
-
-        MultiMash.new(root, new_primary_mash, new_mashes, opts)
-      end
-
-      def []=(key, value)
-        if primary_mash.nil?
-          # This theoretically should never happen since node#force_default! setter methods will autovivify and
-          # node#rm methods do not end in #[]= operators.
-          raise TypeError, "No autovivification was specified initially on a method chain ending in assignment"
-        end
-        ret = delete(key)
-        primary_mash[key] = value
-        ret
-      end
-
-      # mash.element('foo', 'bar') is the same as mash['foo']['bar']
-      def element(key = nil, *subkeys)
-        return self if key.nil?
-        submash = self[key]
-        subkeys.empty? ? submash : submash.element(*subkeys)
-      end
-
-      def delete(key)
-        # the return value is a deep merge which is correct semantics when
-        # merging between attributes on the same level (this would be incorrect
-        # if passed both override and default attributes which would need hash_only
-        # merging).
-        ret = mashes.inject(Mash.new) do |merged, mash|
-          Chef::Mixin::DeepMerge.merge(merged, mash)
-        end
-        ret = Chef::Mixin::DeepMerge.merge(ret, primary_mash)
-        mashes.each do |mash|
-          mash.delete(key) if mash.respond_to?(:delete)
-        end
-        primary_mash.delete(key) if primary_mash.respond_to?(:delete)
-        ret[key]
-      end
-
-      private
-
-      def safe_evalute_key(mash, key)
-        if mash.respond_to?(:[])
-          if mash.respond_to?(:has_key?)
-            if mash.has_key?(key)
-              return mash[key] if mash[key].respond_to?(:[])
-            end
-          elsif !mash[key].nil?
-            return mash[key] if mash[key].respond_to?(:[])
-          end
-        end
-        return nil
-      end
-
-    end
-
   end
 end
diff --git a/lib/chef/node/common_api.rb b/lib/chef/node/common_api.rb
new file mode 100644
index 0000000000..6f921e15aa
--- /dev/null
+++ b/lib/chef/node/common_api.rb
@@ -0,0 +1,124 @@
+#--
+# Copyright:: Copyright 2016, Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+class Chef
+  class Node
+    # shared API between VividMash and ImmutableMash, writer code can be
+    # 'shared' to keep it logically in this file by adding them to the
+    # block list in ImmutableMash.
+    module CommonAPI
+      # method-style access to attributes
+
+      def valid_container?(obj, key)
+        return obj.is_a?(Hash) || (obj.is_a?(Array) && key.is_a?(Fixnum))
+      end
+
+      private :valid_container?
+
+      # - autovivifying / autoreplacing writer
+      # - non-container-ey intermediate objects are replaced with hashes
+      def write(*args, &block)
+        value = block_given? ? yield : args.pop
+        last = args.pop
+        prev_memo = prev_key = nil
+        chain = args.inject(self) do |memo, key|
+          if !valid_container?(memo, key)
+            prev_memo[prev_key] = {}
+            memo = prev_memo[prev_key]
+          end
+          prev_memo = memo
+          prev_key = key
+          memo[key]
+        end
+        if !valid_container?(chain, last)
+          prev_memo[prev_key] = {}
+          chain = prev_memo[prev_key]
+        end
+        chain[last] = value
+      end
+
+      # this autovivifies, but can throw NoSuchAttribute when trying to access #[] on
+      # something that is not a container ("schema violation" issues).
+      #
+      def write!(*args, &block)
+        value = block_given? ? yield : args.pop
+        last = args.pop
+        obj = args.inject(self) do |memo, key|
+          raise Chef::Exceptions::AttributeTypeMismatch unless valid_container?(memo, key)
+          memo[key]
+        end
+        raise Chef::Exceptions::AttributeTypeMismatch unless valid_container?(obj, last)
+        obj[last] = value
+      end
+
+      # FIXME:(?) does anyone need a non-autovivifying writer for attributes that throws exceptions?
+
+      # return true or false based on if the attribute exists
+      def exist?(*path)
+        path.inject(self) do |memo, key|
+          return false unless valid_container?(memo, key)
+          if memo.is_a?(Hash)
+            if memo.key?(key)
+              memo[key]
+            else
+              return false
+            end
+          elsif memo.is_a?(Array)
+            if memo.length > key
+              memo[key]
+            else
+              return false
+            end
+          end
+        end
+        return true
+      end
+
+      # this is a safe non-autovivifying reader that returns nil if the attribute does not exist
+      def read(*path)
+        begin
+          read!(*path)
+        rescue Chef::Exceptions::NoSuchAttribute
+          nil
+        end
+      end
+
+      # non-autovivifying reader that throws an exception if the attribute does not exist
+      def read!(*path)
+        raise Chef::Exceptions::NoSuchAttribute unless exist?(*path)
+        path.inject(self) do |memo, key|
+          memo[key]
+        end
+      end
+
+      # FIXME:(?) does anyone really like the autovivifying reader that we have and wants the same behavior?  readers that write?  ugh...
+
+      def unlink(*path, last)
+        hash = path.empty? ? self : read(*path)
+        return nil unless hash.is_a?(Hash) || hash.is_a?(Array)
+        root.top_level_breadcrumb ||= last
+        hash.delete(last)
+      end
+
+      def unlink!(*path)
+        raise Chef::Exceptions::NoSuchAttribute unless exist?(*path)
+        unlink(*path)
+      end
+
+    end
+  end
+end
diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb
index b5fd86fa72..d4623ace2a 100644
--- a/lib/chef/node/immutable_collections.rb
+++ b/lib/chef/node/immutable_collections.rb
@@ -1,3 +1,21 @@
+#--
+# Copyright:: Copyright 2012-2016, Chef Software, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require "chef/node/common_api"
 
 class Chef
   class Node
@@ -124,6 +142,7 @@ class Chef
     class ImmutableMash < Mash
 
       include Immutablize
+      include CommonAPI
 
       alias :internal_set :[]=
       private :internal_set
@@ -144,6 +163,10 @@ class Chef
         :replace,
         :select!,
         :shift,
+        :write,
+        :write!,
+        :unlink,
+        :unlink!,
       ]
 
       def initialize(mash_data)
@@ -167,13 +190,15 @@ class Chef
       end
 
       def method_missing(symbol, *args)
-        if args.empty?
+        if symbol == :to_ary
+          super
+        elsif args.empty?
           if key?(symbol)
             self[symbol]
           else
             raise NoMethodError, "Undefined method or attribute `#{symbol}' on `node'"
           end
-        # This will raise a ImmutableAttributeModification error:
+          # This will raise a ImmutableAttributeModification error:
         elsif symbol.to_s =~ /=$/
           key_to_set = symbol.to_s[/^(.+)=$/, 1]
           self[key_to_set] = (args.length == 1 ? args[0] : args)