diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2016-06-27 15:38:28 -0700 |
---|---|---|
committer | Lamont Granquist <lamont@scriptkiddie.org> | 2016-06-27 15:42:06 -0700 |
commit | 86fa507226b484a71a45ef6129840bc3928be6b7 (patch) | |
tree | b6f9efaedd634dbc61418ceb78817b91af1889df | |
parent | 945f23be7a43d90ae7ed402d05363b3ff0c11bff (diff) | |
download | chef-86fa507226b484a71a45ef6129840bc3928be6b7.tar.gz |
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
31 files changed, 852 insertions, 315 deletions
diff --git a/lib/chef/decorator/unchain.rb b/lib/chef/decorator/unchain.rb new file mode 100644 index 0000000000..30179d4e63 --- /dev/null +++ b/lib/chef/decorator/unchain.rb @@ -0,0 +1,43 @@ +class Chef + class Decorator < SimpleDelegator + # + # This decorator unchains method call chains and turns them into method calls + # with variable args. So this: + # + # node.set_unless["foo"]["bar"] = "baz" + # + # Can become: + # + # node.set_unless("foo", "bar", "baz") + # + # While this is a decorator it is not a Decorator and does not inherit because + # it deliberately does not need or want the method_missing magic. It is not legal + # to call anything on the intermediate values and only supports method chaining with + # #[] until the chain comes to an end with #[]=, so does not behave like a hash or + # array... e.g. + # + # node.default['foo'].keys is legal + # node.set_unless['foo'].keys is not legal now or ever + # + class Unchain + attr_accessor :__path__ + attr_accessor :__method__ + + def initialize(obj, method) + @__path__ = [] + @__method__ = method + @delegate_sd_obj = obj + end + + def [](key) + __path__.push(key) + self + end + + def []=(key, value) + __path__.push(key) + @delegate_sd_obj.public_send(__method__, *__path__, value) + end + end + end +end diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb index ea90d80cd8..43759568a7 100644 --- a/lib/chef/exceptions.rb +++ b/lib/chef/exceptions.rb @@ -106,7 +106,12 @@ class Chef # for back compat, need to raise an error that inherits from ArgumentError class CookbookNotFoundInRepo < ArgumentError; end class RecipeNotFound < ArgumentError; end + # AttributeNotFound really means the attribute file could not be found class AttributeNotFound < RuntimeError; end + # NoSuchAttribute is raised on access by node.read!("foo", "bar") when node["foo"]["bar"] does not exist. + class NoSuchAttribute < RuntimeError; end + # AttributeTypeMismatch is raised by node.write!("foo", "bar", "baz") when e.g. node["foo"] = "bar" (overwriting String with Hash) + class AttributeTypeMismatch < RuntimeError; end class MissingCookbookDependency < StandardError; end # CHEF-5120 class InvalidCommandOption < RuntimeError; end class CommandTimeout < RuntimeError; end diff --git a/lib/chef/node.rb b/lib/chef/node.rb index 8d77becbf0..54faab6d3e 100644 --- a/lib/chef/node.rb +++ b/lib/chef/node.rb @@ -43,6 +43,8 @@ class Chef def_delegators :attributes, :keys, :each_key, :each_value, :key?, :has_key? def_delegators :attributes, :rm, :rm_default, :rm_normal, :rm_override def_delegators :attributes, :default!, :normal!, :override!, :force_default!, :force_override! + def_delegators :attributes, :default_unless, :normal_unless, :override_unless, :set_unless + def_delegators :attributes, :read, :read!, :write, :write!, :unlink, :unlink! attr_accessor :recipe_list, :run_state, :override_runlist @@ -196,35 +198,18 @@ class Chef # might be missing def normal attributes.top_level_breadcrumb = nil - attributes.set_unless_value_present = false attributes.normal end - alias_method :set, :normal - - # Set a normal attribute of this node, auto-vivifying any mashes that are - # missing, but if the final value already exists, don't set it - def normal_unless - attributes.top_level_breadcrumb = nil - attributes.set_unless_value_present = true - attributes.normal + def set + Chef.log_deprecation("node.set is deprecated and will be removed in Chef 14, please use node.default/node.override (or node.normal only if you really need persistence)") + normal end - alias_method :set_unless, :normal_unless - # Set a default of this node, but auto-vivify any Mashes that might # be missing def default attributes.top_level_breadcrumb = nil - attributes.set_unless_value_present = false - attributes.default - end - - # Set a default attribute of this node, auto-vivifying any mashes that are - # missing, but if the final value already exists, don't set it - def default_unless - attributes.top_level_breadcrumb = nil - attributes.set_unless_value_present = true attributes.default end @@ -232,15 +217,6 @@ class Chef # might be missing def override attributes.top_level_breadcrumb = nil - attributes.set_unless_value_present = false - attributes.override - end - - # Set an override attribute of this node, auto-vivifying any mashes that - # are missing, but if the final value already exists, don't set it - def override_unless - attributes.top_level_breadcrumb = nil - attributes.set_unless_value_present = true attributes.override end @@ -262,7 +238,6 @@ class Chef def automatic_attrs attributes.top_level_breadcrumb = nil - attributes.set_unless_value_present = false attributes.automatic end @@ -290,8 +265,14 @@ class Chef end # Only works for attribute fetches, setting is no longer supported - def method_missing(symbol, *args) - attributes.send(symbol, *args) + # XXX: this should be deprecated + def method_missing(method, *args, &block) + attributes.public_send(method, *args, &block) + end + + # Fix respond_to + method so that it works with method_missing delegation + def respond_to_missing?(method, include_private = false) + attributes.respond_to?(method, false) end # Returns true if this Node expects a given recipe, false if not. 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) diff --git a/lib/chef/provider/batch.rb b/lib/chef/provider/batch.rb index bb294afd3f..0d857aaa79 100644 --- a/lib/chef/provider/batch.rb +++ b/lib/chef/provider/batch.rb @@ -29,7 +29,7 @@ class Chef end def command - basepath = is_forced_32bit ? wow64_directory : run_context.node.kernel.os_info.system_directory + basepath = is_forced_32bit ? wow64_directory : run_context.node["kernel"]["os_info"]["system_directory"] interpreter_path = Chef::Util::PathHelper.join(basepath, interpreter) diff --git a/lib/chef/provider/package/openbsd.rb b/lib/chef/provider/package/openbsd.rb index 2120b9aa48..8043c01693 100644 --- a/lib/chef/provider/package/openbsd.rb +++ b/lib/chef/provider/package/openbsd.rb @@ -127,7 +127,7 @@ class Chef end def pkg_path - ENV["PKG_PATH"] || "http://ftp.OpenBSD.org/pub/#{node.kernel.name}/#{node.kernel.release}/packages/#{node.kernel.machine}/" + ENV["PKG_PATH"] || "http://ftp.OpenBSD.org/pub/#{node["kernel"]["name"]}/#{node["kernel"]["release"]}/packages/#{node["kernel"]["machine"]}/" end end diff --git a/lib/chef/provider/powershell_script.rb b/lib/chef/provider/powershell_script.rb index 6365f6a171..ab85ec35ac 100644 --- a/lib/chef/provider/powershell_script.rb +++ b/lib/chef/provider/powershell_script.rb @@ -36,7 +36,7 @@ class Chef end def command - basepath = is_forced_32bit ? wow64_directory : run_context.node.kernel.os_info.system_directory + basepath = is_forced_32bit ? wow64_directory : run_context.node["kernel"]["os_info"]["system_directory"] # Powershell.exe is always in "v1.0" folder (for backwards compatibility) interpreter_path = Chef::Util::PathHelper.join(basepath, "WindowsPowerShell", "v1.0", interpreter) diff --git a/lib/chef/shell.rb b/lib/chef/shell.rb index aad5c49d00..26683cc25d 100644 --- a/lib/chef/shell.rb +++ b/lib/chef/shell.rb @@ -148,7 +148,7 @@ module Shell end def self.greeting - " #{Etc.getlogin}@#{Shell.session.node.fqdn}" + " #{Etc.getlogin}@#{Shell.session.node["fqdn"]}" rescue NameError, ArgumentError "" end diff --git a/spec/data/run_context/cookbooks/circular-dep1/attributes/default.rb b/spec/data/run_context/cookbooks/circular-dep1/attributes/default.rb index ef0967a4d2..e45e7d9f68 100644 --- a/spec/data/run_context/cookbooks/circular-dep1/attributes/default.rb +++ b/spec/data/run_context/cookbooks/circular-dep1/attributes/default.rb @@ -1,4 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "circular-dep1::default" - - +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "circular-dep1::default" diff --git a/spec/data/run_context/cookbooks/circular-dep2/attributes/default.rb b/spec/data/run_context/cookbooks/circular-dep2/attributes/default.rb index f2ef012aa1..37f396b1f9 100644 --- a/spec/data/run_context/cookbooks/circular-dep2/attributes/default.rb +++ b/spec/data/run_context/cookbooks/circular-dep2/attributes/default.rb @@ -1,3 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "circular-dep2::default" - +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "circular-dep2::default" diff --git a/spec/data/run_context/cookbooks/dependency1/attributes/aa_first.rb b/spec/data/run_context/cookbooks/dependency1/attributes/aa_first.rb index e818d36a9e..3059494198 100644 --- a/spec/data/run_context/cookbooks/dependency1/attributes/aa_first.rb +++ b/spec/data/run_context/cookbooks/dependency1/attributes/aa_first.rb @@ -1,2 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "dependency1::aa_first" +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "dependency1::aa_first" diff --git a/spec/data/run_context/cookbooks/dependency1/attributes/default.rb b/spec/data/run_context/cookbooks/dependency1/attributes/default.rb index 6875274e3f..a65a3345bc 100644 --- a/spec/data/run_context/cookbooks/dependency1/attributes/default.rb +++ b/spec/data/run_context/cookbooks/dependency1/attributes/default.rb @@ -1,2 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "dependency1::default" +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "dependency1::default" diff --git a/spec/data/run_context/cookbooks/dependency1/attributes/zz_last.rb b/spec/data/run_context/cookbooks/dependency1/attributes/zz_last.rb index 1a513b03d4..94ffb30133 100644 --- a/spec/data/run_context/cookbooks/dependency1/attributes/zz_last.rb +++ b/spec/data/run_context/cookbooks/dependency1/attributes/zz_last.rb @@ -1,3 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "dependency1::zz_last" - +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "dependency1::zz_last" diff --git a/spec/data/run_context/cookbooks/dependency2/attributes/default.rb b/spec/data/run_context/cookbooks/dependency2/attributes/default.rb index 526751f422..8917bf9730 100644 --- a/spec/data/run_context/cookbooks/dependency2/attributes/default.rb +++ b/spec/data/run_context/cookbooks/dependency2/attributes/default.rb @@ -1,3 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "dependency2::default" - +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "dependency2::default" diff --git a/spec/data/run_context/cookbooks/no-default-attr/attributes/server.rb b/spec/data/run_context/cookbooks/no-default-attr/attributes/server.rb index 3ad2b925aa..07294665b2 100644 --- a/spec/data/run_context/cookbooks/no-default-attr/attributes/server.rb +++ b/spec/data/run_context/cookbooks/no-default-attr/attributes/server.rb @@ -1,3 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "no-default-attr::server" - +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "no-default-attr::server" diff --git a/spec/data/run_context/cookbooks/test-with-circular-deps/attributes/default.rb b/spec/data/run_context/cookbooks/test-with-circular-deps/attributes/default.rb index cca56bc61f..77309462b1 100644 --- a/spec/data/run_context/cookbooks/test-with-circular-deps/attributes/default.rb +++ b/spec/data/run_context/cookbooks/test-with-circular-deps/attributes/default.rb @@ -1,3 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "test-with-circular-deps::default" - +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "test-with-circular-deps::default" diff --git a/spec/data/run_context/cookbooks/test-with-deps/attributes/default.rb b/spec/data/run_context/cookbooks/test-with-deps/attributes/default.rb index 4d71cc3cfe..c4cc8151a4 100644 --- a/spec/data/run_context/cookbooks/test-with-deps/attributes/default.rb +++ b/spec/data/run_context/cookbooks/test-with-deps/attributes/default.rb @@ -1,3 +1,2 @@ -set_unless[:attr_load_order] = [] -set[:attr_load_order] << "test-with-deps::default" - +normal_unless[:attr_load_order] = [] +normal[:attr_load_order] << "test-with-deps::default" diff --git a/spec/functional/resource/package_spec.rb b/spec/functional/resource/package_spec.rb index 6dc55f7f01..0f01a751ec 100644 --- a/spec/functional/resource/package_spec.rb +++ b/spec/functional/resource/package_spec.rb @@ -260,7 +260,7 @@ describe Chef::Resource::Package, metadata do end before do - node.set[:preseed_value] = "FROM TEMPLATE" + node.normal[:preseed_value] = "FROM TEMPLATE" end it "preseeds the package, then installs it" do diff --git a/spec/functional/resource/template_spec.rb b/spec/functional/resource/template_spec.rb index f270043f2c..32529fbb0c 100644 --- a/spec/functional/resource/template_spec.rb +++ b/spec/functional/resource/template_spec.rb @@ -110,7 +110,7 @@ describe Chef::Resource::Template do context "using single helper syntax referencing @node" do before do - node.set[:helper_test_attr] = "value from helper method" + node.normal[:helper_test_attr] = "value from helper method" resource.helper(:helper_method) { "#{@node[:helper_test_attr]}" } end @@ -131,7 +131,7 @@ describe Chef::Resource::Template do context "using an inline block referencing @node" do before do - node.set[:helper_test_attr] = "value from helper method" + node.normal[:helper_test_attr] = "value from helper method" resource.helpers do def helper_method @@ -168,7 +168,7 @@ describe Chef::Resource::Template do end before do - node.set[:helper_test_attr] = "value from helper method" + node.normal[:helper_test_attr] = "value from helper method" resource.helpers(ExampleModuleReferencingATNode) end diff --git a/spec/functional/shell_spec.rb b/spec/functional/shell_spec.rb index fe2abdb12a..636162fb16 100644 --- a/spec/functional/shell_spec.rb +++ b/spec/functional/shell_spec.rb @@ -137,7 +137,7 @@ describe Shell do it "sets the override_runlist from the command line" do output, exitstatus = run_chef_shell_with("-o 'override::foo,override::bar'") do |out, keyboard| - show_recipes_code = %q[puts "#{node.recipes.inspect}"] + show_recipes_code = %q[puts "#{node["recipes"].inspect}"] keyboard.puts(show_recipes_code) read_until(out, show_recipes_code) end diff --git a/spec/unit/cookbook_version_spec.rb b/spec/unit/cookbook_version_spec.rb index 7def2f5749..81ea161bfe 100644 --- a/spec/unit/cookbook_version_spec.rb +++ b/spec/unit/cookbook_version_spec.rb @@ -120,8 +120,8 @@ describe Chef::CookbookVersion do # Used to test file-specificity related file lookups let(:node) do Chef::Node.new.tap do |n| - n.set[:platform] = "ubuntu" - n.set[:platform_version] = "13.04" + n.normal[:platform] = "ubuntu" + n.normal[:platform_version] = "13.04" n.name("testing") end end @@ -203,8 +203,8 @@ describe Chef::CookbookVersion do # Used to test file-specificity related file lookups let(:node) do Chef::Node.new.tap do |n| - n.set[:platform] = "ubuntu" - n.set[:platform_version] = "13.04" + n.normal[:platform] = "ubuntu" + n.normal[:platform_version] = "13.04" n.name("testing") end end diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb index 57ad3c0c25..e40f454c0b 100644 --- a/spec/unit/node/attribute_spec.rb +++ b/spec/unit/node/attribute_spec.rb @@ -218,7 +218,7 @@ describe Chef::Node::Attribute do end it "gives the value at each level of precedence for a path spec" do - expected = [["set_unless_enabled?", false], + expected = [ %w{default default}, %w{env_default env_default}, %w{role_default role_default}, @@ -417,12 +417,6 @@ describe Chef::Node::Attribute do expect(@attributes.normal["foo"]["bar"]).to eq(:baz) end - it "should optionally skip setting the value if one already exists" do - @attributes.set_unless_value_present = true - @attributes.normal["hostname"] = "bar" - expect(@attributes["hostname"]).to eq("latte") - end - it "does not support ||= when setting" do # This is a limitation of auto-vivification. # Users who need this behavior can use set_unless and friends @@ -493,6 +487,7 @@ describe Chef::Node::Attribute do end it "should return true if an attribute exists but is set to nil using dot notation" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false expect(@attributes.music.deeper.has_key?("gates_of_ishtar")).to eq(true) end @@ -533,10 +528,12 @@ describe Chef::Node::Attribute do describe "method_missing" do it "should behave like a [] lookup" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false expect(@attributes.music.mastodon).to eq("rocks") end it "should allow the last method to set a value if it has an = sign on the end" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false @attributes.normal.music.mastodon = %w{dream still shining} expect(@attributes.normal.music.mastodon).to eq(%w{dream still shining}) end @@ -577,7 +574,7 @@ describe Chef::Node::Attribute do it "should yield lower if we go deeper" do collect = Array.new - @attributes.one.keys.each do |k| + @attributes["one"].keys.each do |k| collect << k end expect(collect.include?("two")).to eq(true) @@ -587,7 +584,7 @@ describe Chef::Node::Attribute do end it "should not raise an exception if one of the hashes has a nil value on a deep lookup" do - expect { @attributes.place.keys { |k| } }.not_to raise_error + expect { @attributes["place"].keys { |k| } }.not_to raise_error end end @@ -1171,6 +1168,7 @@ describe Chef::Node::Attribute do end it "raises an error when using `attr=value`" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false expect { @attributes.new_key = "new value" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) end diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb index f57ed459cd..fe4e50d1bd 100644 --- a/spec/unit/node/immutable_collections_spec.rb +++ b/spec/unit/node/immutable_collections_spec.rb @@ -95,6 +95,10 @@ describe Chef::Node::ImmutableMash do :replace, :select!, :shift, + :write, + :write!, + :unlink, + :unlink!, ].each do |mutator| it "doesn't allow mutation via `#{mutator}'" do expect { @immutable_mash.send(mutator) }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) diff --git a/spec/unit/node/vivid_mash_spec.rb b/spec/unit/node/vivid_mash_spec.rb new file mode 100644 index 0000000000..ca5eddce57 --- /dev/null +++ b/spec/unit/node/vivid_mash_spec.rb @@ -0,0 +1,344 @@ +# +# 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. +# + +require "spec_helper" +require "chef/node/attribute_collections" + +describe Chef::Node::VividMash do + class Root + attr_accessor :top_level_breadcrumb + end + + let(:root) { Root.new } + + let(:vivid) do + expect(root).to receive(:reset_cache).at_least(:once).with(nil) + Chef::Node::VividMash.new(root, + { "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil } + ) + end + + context "#read" do + before do + # vivify the vividmash, then we're read-only so the cache should never be cleared afterwards + vivid + expect(root).not_to receive(:reset_cache) + end + + it "reads hashes deeply" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect(vivid.read("one", "two", "three")).to eql("four") + end + + it "does not trainwreck when hitting hash keys that do not exist" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect(vivid.read("one", "five", "six")).to eql(nil) + end + + it "does not trainwreck when hitting an array with an out of bounds index" do + expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original + expect(vivid.read("array", 5, "one")).to eql(nil) + end + + it "does not trainwreck when hitting an array with a string key" do + expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original + expect(vivid.read("array", "one", "two")).to eql(nil) + end + + it "does not trainwreck when traversing a nil" do + expect(root).to receive(:top_level_breadcrumb=).with("nil").and_call_original + expect(vivid.read("nil", "one", "two")).to eql(nil) + end + end + + context "#exist?" do + before do + # vivify the vividmash, then we're read-only so the cache should never be cleared afterwards + vivid + expect(root).not_to receive(:reset_cache) + end + + it "true if there's a hash key there" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect(vivid.exist?("one", "two", "three")).to be true + end + + it "true for intermediate hashes" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect(vivid.exist?("one")).to be true + end + + it "true for arrays that exist" do + expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original + expect(vivid.exist?("array", 1)).to be true + end + + it "true when the value of the key is nil" do + expect(root).to receive(:top_level_breadcrumb=).with("nil").and_call_original + expect(vivid.exist?("nil")).to be true + end + + it "false when attributes don't exist" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect(vivid.exist?("one", "five", "six")).to be false + end + + it "false when traversing a non-container" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect(vivid.exist?("one", "two", "three", "four")).to be false + end + + it "false when an array index does not exist" do + expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original + expect(vivid.exist?("array", 3)).to be false + end + + it "false when traversing a nil" do + expect(root).to receive(:top_level_breadcrumb=).with("nil").and_call_original + expect(vivid.exist?("nil", "foo", "bar")).to be false + end + end + + context "#read!" do + before do + # vivify the vividmash, then we're read-only so the cache should never be cleared afterwards + vivid + expect(root).not_to receive(:reset_cache) + end + + it "reads hashes deeply" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect(vivid.read!("one", "two", "three")).to eql("four") + end + + it "reads arrays deeply" do + expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original + expect(vivid.read!("array", 1)).to eql(1) + end + + it "throws an exception when attributes do not exist" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect { vivid.read!("one", "five", "six") }.to raise_error(Chef::Exceptions::NoSuchAttribute) + end + + it "throws an exception when traversing a non-container" do + expect(root).to receive(:top_level_breadcrumb=).with("one").and_call_original + expect { vivid.read!("one", "two", "three", "four") }.to raise_error(Chef::Exceptions::NoSuchAttribute) + end + + it "throws an exception when an array element does not exist" do + expect(root).to receive(:top_level_breadcrumb=).with("array").and_call_original + expect { vivid.read!("array", 3) }.to raise_error(Chef::Exceptions::NoSuchAttribute) + end + end + + context "#write" do + before do + vivid + expect(root).not_to receive(:reset_cache).with(nil) + end + + it "should write into hashes" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + vivid.write("one", "five", "six") + expect(vivid["one"]["five"]).to eql("six") + end + + it "should deeply autovivify" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + vivid.write("one", "five", "six", "seven", "eight", "nine", "ten") + expect(vivid["one"]["five"]["six"]["seven"]["eight"]["nine"]).to eql("ten") + end + + it "should raise an exception if you overwrite an array with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with("array") + vivid.write("array", "five", "six") + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => { "five" => "six" }, "nil" => nil }) + end + + it "should raise an exception if you traverse through an array with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with("array") + vivid.write("array", "five", "six", "seven") + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => { "five" => { "six" => "seven" } }, "nil" => nil }) + end + + it "should raise an exception if you overwrite a string with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + vivid.write("one", "two", "three", "four", "five") + expect(vivid).to eql({ "one" => { "two" => { "three" => { "four" => "five" } } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should raise an exception if you traverse through a string with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + vivid.write("one", "two", "three", "four", "five", "six") + expect(vivid).to eql({ "one" => { "two" => { "three" => { "four" => { "five" => "six" } } } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should raise an exception if you overwrite a nil with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with("nil") + vivid.write("nil", "one", "two") + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => { "one" => "two" } }) + end + + it "should raise an exception if you traverse through a nil with a hash" do + expect(root).to receive(:reset_cache).at_least(:once).with("nil") + vivid.write("nil", "one", "two", "three") + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => { "one" => { "two" => "three" } } }) + end + + it "writes with a block" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + vivid.write("one", "five") { "six" } + expect(vivid["one"]["five"]).to eql("six") + end + end + + context "#write!" do + before do + vivid + expect(root).not_to receive(:reset_cache).with(nil) + end + + it "should write into hashes" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + vivid.write!("one", "five", "six") + expect(vivid["one"]["five"]).to eql("six") + end + + it "should deeply autovivify" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + vivid.write!("one", "five", "six", "seven", "eight", "nine", "ten") + expect(vivid["one"]["five"]["six"]["seven"]["eight"]["nine"]).to eql("ten") + end + + it "should raise an exception if you overwrite an array with a hash" do + expect(root).not_to receive(:reset_cache) + expect { vivid.write!("array", "five", "six") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should raise an exception if you traverse through an array with a hash" do + expect(root).not_to receive(:reset_cache) + expect { vivid.write!("array", "five", "six", "seven") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should raise an exception if you overwrite a string with a hash" do + expect(root).not_to receive(:reset_cache) + expect { vivid.write!("one", "two", "three", "four", "five") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should raise an exception if you traverse through a string with a hash" do + expect(root).not_to receive(:reset_cache) + expect { vivid.write!("one", "two", "three", "four", "five", "six") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should raise an exception if you overwrite a nil with a hash" do + expect(root).not_to receive(:reset_cache) + expect { vivid.write!("nil", "one", "two") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should raise an exception if you traverse through a nil with a hash" do + expect(root).not_to receive(:reset_cache) + expect { vivid.write!("nil", "one", "two", "three") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "writes with a block" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + vivid.write!("one", "five") { "six" } + expect(vivid["one"]["five"]).to eql("six") + end + end + + context "#unlink" do + before do + vivid + expect(root).not_to receive(:reset_cache).with(nil) + end + + it "should return nil if the keys already don't exist" do + expect(root).not_to receive(:reset_cache) + expect(vivid.unlink("five", "six", "seven", "eight")).to eql(nil) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should unlink hashes" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect( vivid.unlink("one") ).to eql({ "two" => { "three" => "four" } }) + expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should unlink array elements" do + expect(root).to receive(:reset_cache).at_least(:once).with("array") + expect(vivid.unlink("array", 2)).to eql(2) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1 ], "nil" => nil }) + end + + it "should unlink nil" do + expect(root).to receive(:reset_cache).at_least(:once).with("nil") + expect(vivid.unlink("nil")).to eql(nil) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] }) + end + + it "should traverse a nil and safely do nothing" do + expect(root).not_to receive(:reset_cache) + expect(vivid.unlink("nil", "foo")).to eql(nil) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + end + + context "#unlink!" do + before do + vivid + expect(root).not_to receive(:reset_cache).with(nil) + end + + it "should raise an exception if the keys already don't exist" do + expect(root).not_to receive(:reset_cache) + expect { vivid.unlink!("five", "six", "seven", "eight") }.to raise_error(Chef::Exceptions::NoSuchAttribute) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should unlink! hashes" do + expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect( vivid.unlink!("one") ).to eql({ "two" => { "three" => "four" } }) + expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil }) + end + + it "should unlink! array elements" do + expect(root).to receive(:reset_cache).at_least(:once).with("array") + expect(vivid.unlink!("array", 2)).to eql(2) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1 ], "nil" => nil }) + end + + it "should unlink! nil" do + expect(root).to receive(:reset_cache).at_least(:once).with("nil") + expect(vivid.unlink!("nil")).to eql(nil) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] }) + end + + it "should raise an exception if it traverses a nil" do + expect(root).not_to receive(:reset_cache) + expect { vivid.unlink!("nil", "foo") }.to raise_error(Chef::Exceptions::NoSuchAttribute) + expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => nil }) + end + end +end diff --git a/spec/unit/node_spec.rb b/spec/unit/node_spec.rb index 923b488f72..af9a6e94fc 100644 --- a/spec/unit/node_spec.rb +++ b/spec/unit/node_spec.rb @@ -233,60 +233,63 @@ describe Chef::Node do end it "should let you go deep with attribute?" do - node.set["battles"]["people"]["wonkey"] = true + node.normal["battles"]["people"]["wonkey"] = true expect(node["battles"]["people"].attribute?("wonkey")).to eq(true) expect(node["battles"]["people"].attribute?("snozzberry")).to eq(false) end it "does not allow you to set an attribute via method_missing" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false expect { node.sunshine = "is bright" }.to raise_error(Chef::Exceptions::ImmutableAttributeModification) end it "should allow you get get an attribute via method_missing" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false node.default.sunshine = "is bright" expect(node.sunshine).to eql("is bright") end describe "normal attributes" do it "should allow you to set an attribute with set, without pre-declaring a hash" do - node.set[:snoopy][:is_a_puppy] = true + node.normal[:snoopy][:is_a_puppy] = true expect(node[:snoopy][:is_a_puppy]).to eq(true) end it "should allow you to set an attribute with set_unless" do - node.set_unless[:snoopy][:is_a_puppy] = false + node.normal_unless[:snoopy][:is_a_puppy] = false expect(node[:snoopy][:is_a_puppy]).to eq(false) end it "should not allow you to set an attribute with set_unless if it already exists" do - node.set[:snoopy][:is_a_puppy] = true - node.set_unless[:snoopy][:is_a_puppy] = false + node.normal[:snoopy][:is_a_puppy] = true + node.normal_unless[:snoopy][:is_a_puppy] = false expect(node[:snoopy][:is_a_puppy]).to eq(true) end it "should allow you to set an attribute with set_unless if is a nil value" do node.attributes.normal = { snoopy: { is_a_puppy: nil } } - node.set_unless[:snoopy][:is_a_puppy] = false + node.normal_unless[:snoopy][:is_a_puppy] = false expect(node[:snoopy][:is_a_puppy]).to eq(false) end it "should allow you to set a value after a set_unless" do # this tests for set_unless_present state bleeding between statements CHEF-3806 - node.set_unless[:snoopy][:is_a_puppy] = false - node.set[:snoopy][:is_a_puppy] = true + node.normal_unless[:snoopy][:is_a_puppy] = false + node.normal[:snoopy][:is_a_puppy] = true expect(node[:snoopy][:is_a_puppy]).to eq(true) end it "should let you set a value after a 'dangling' set_unless" do # this tests for set_unless_present state bleeding between statements CHEF-3806 - node.set[:snoopy][:is_a_puppy] = "what" - node.set_unless[:snoopy][:is_a_puppy] - node.set[:snoopy][:is_a_puppy] = true + node.normal[:snoopy][:is_a_puppy] = "what" + node.normal_unless[:snoopy][:is_a_puppy] + node.normal[:snoopy][:is_a_puppy] = true expect(node[:snoopy][:is_a_puppy]).to eq(true) end it "auto-vivifies attributes created via method syntax" do - node.set.fuu.bahrr.baz = "qux" + Chef::Config[:treat_deprecation_warnings_as_errors] = false + node.normal.fuu.bahrr.baz = "qux" expect(node.fuu.bahrr.baz).to eq("qux") end @@ -295,6 +298,20 @@ describe Chef::Node do node.tag("three", "four") expect(node["tags"]).to eq(%w{one two three four}) end + + it "set is a deprecated alias for normal" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false + expect(Chef).to receive(:log_deprecation).with(/set is deprecated/) + node.set[:snoopy][:is_a_puppy] = true + expect(node[:snoopy][:is_a_puppy]).to eq(true) + end + + it "set_unless is a deprecated alias for normal_unless" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false + expect(Chef).to receive(:log_deprecation).with(/set_unless is deprecated/) + node.set_unless[:snoopy][:is_a_puppy] = false + expect(node[:snoopy][:is_a_puppy]).to eq(false) + end end describe "default attributes" do @@ -329,7 +346,14 @@ describe Chef::Node do expect(node[:snoopy][:is_a_puppy]).to eq(true) end + it "does not exhibit chef/chef/issues/5005 bug" do + node.env_default["a"]["r1"]["g"]["u"] = "u1" + node.default_unless["a"]["r1"]["g"]["r"] = "r" + expect(node["a"]["r1"]["g"]["u"]).to eql("u1") + end + it "auto-vivifies attributes created via method syntax" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false node.default.fuu.bahrr.baz = "qux" expect(node.fuu.bahrr.baz).to eq("qux") end @@ -368,6 +392,7 @@ describe Chef::Node do end it "auto-vivifies attributes created via method syntax" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false node.override.fuu.bahrr.baz = "qux" expect(node.fuu.bahrr.baz).to eq("qux") end @@ -453,8 +478,9 @@ describe Chef::Node do expect( node["mysql"]["server"][0]["port"] ).to be_nil end - it "does not have a horrible error message when mistaking arrays for hashes" do - expect { node.rm("mysql", "server", "port") }.to raise_error(TypeError, "Wrong type in index of attribute (did you use a Hash index on an Array?)") + it "when mistaking arrays for hashes, it considers the value removed and does nothing" do + node.rm("mysql", "server", "port") + expect(node["mysql"]["server"][0]["port"]).to eql(3456) end end end @@ -706,6 +732,7 @@ describe Chef::Node do # describe "deep merge attribute cache edge conditions" do it "does not error with complicated attribute substitution" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false 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 @@ -720,6 +747,7 @@ describe Chef::Node do end it "method interpolation syntax also works" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false 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']}" @@ -729,6 +757,7 @@ describe Chef::Node do end it "should raise an ArgumentError if you ask for an attribute that doesn't exist via method_missing" do + Chef::Config[:treat_deprecation_warnings_as_errors] = false expect { node.sunshine }.to raise_error(NoMethodError) end @@ -744,6 +773,51 @@ describe Chef::Node do expect(seen_attributes["sunshine"]).to eq("is bright") expect(seen_attributes["canada"]).to eq("is a nice place") end + + describe "functional attribute API" do + # deeper functional testing of this API is in the VividMash spec tests + it "should have an exist? function" do + node.default["foo"]["bar"] = "baz" + expect(node.exist?("foo", "bar")).to be true + expect(node.exist?("bar", "foo")).to be false + end + + it "should have a read function" do + node.override["foo"]["bar"] = "baz" + expect(node.read("foo", "bar")).to eql("baz") + expect(node.read("bar", "foo")).to eql(nil) + end + + it "should have a read! function" do + node.override["foo"]["bar"] = "baz" + expect(node.read!("foo", "bar")).to eql("baz") + expect { node.read!("bar", "foo") }.to raise_error(Chef::Exceptions::NoSuchAttribute) + end + + it "delegates write(:level) to node.level.write()" do + node.write(:default, "foo", "bar", "baz") + expect(node.default["foo"]["bar"]).to eql("baz") + end + + it "delegates write!(:level) to node.level.write!()" do + node.write!(:default, "foo", "bar", "baz") + expect(node.default["foo"]["bar"]).to eql("baz") + node.default["bar"] = true + expect { node.write!(:default, "bar", "foo", "baz") }.to raise_error(Chef::Exceptions::AttributeTypeMismatch) + end + + it "delegates unlink(:level) to node.level.unlink()" do + node.default["foo"]["bar"] = "baz" + expect(node.unlink(:default, "foo", "bar")).to eql("baz") + expect(node.unlink(:default, "bar", "foo")).to eql(nil) + end + + it "delegates unlink!(:level) to node.level.unlink!()" do + node.default["foo"]["bar"] = "baz" + expect(node.unlink!(:default, "foo", "bar")).to eql("baz") + expect { node.unlink!(:default, "bar", "foo") }.to raise_error(Chef::Exceptions::NoSuchAttribute) + end + end end describe "consuming json" do @@ -789,8 +863,8 @@ describe Chef::Node do it "should add json attributes to the node" do node.consume_external_attrs(@ohai_data, { "one" => "two", "three" => "four" }) - expect(node.one).to eql("two") - expect(node.three).to eql("four") + expect(node["one"]).to eql("two") + expect(node["three"]).to eql("four") end it "should set the tags attribute to an empty array if it is not already defined" do @@ -824,17 +898,17 @@ describe Chef::Node do it "deep merges attributes instead of overwriting them" do node.consume_external_attrs(@ohai_data, "one" => { "two" => { "three" => "four" } }) - expect(node.one.to_hash).to eq({ "two" => { "three" => "four" } }) + expect(node["one"].to_hash).to eq({ "two" => { "three" => "four" } }) node.consume_external_attrs(@ohai_data, "one" => { "abc" => "123" }) node.consume_external_attrs(@ohai_data, "one" => { "two" => { "foo" => "bar" } }) - expect(node.one.to_hash).to eq({ "two" => { "three" => "four", "foo" => "bar" }, "abc" => "123" }) + expect(node["one"].to_hash).to eq({ "two" => { "three" => "four", "foo" => "bar" }, "abc" => "123" }) end it "gives attributes from JSON priority when deep merging" do node.consume_external_attrs(@ohai_data, "one" => { "two" => { "three" => "four" } }) - expect(node.one.to_hash).to eq({ "two" => { "three" => "four" } }) + expect(node["one"].to_hash).to eq({ "two" => { "three" => "four" } }) node.consume_external_attrs(@ohai_data, "one" => { "two" => { "three" => "forty-two" } }) - expect(node.one.to_hash).to eq({ "two" => { "three" => "forty-two" } }) + expect(node["one"].to_hash).to eq({ "two" => { "three" => "forty-two" } }) end end @@ -1036,10 +1110,10 @@ describe Chef::Node do end it "sets attributes from the files" do - expect(node.ldap_server).to eql("ops1prod") - expect(node.ldap_basedn).to eql("dc=hjksolutions,dc=com") - expect(node.ldap_replication_password).to eql("forsure") - expect(node.smokey).to eql("robinson") + expect(node["ldap_server"]).to eql("ops1prod") + expect(node["ldap_basedn"]).to eql("dc=hjksolutions,dc=com") + expect(node["ldap_replication_password"]).to eql("forsure") + expect(node["smokey"]).to eql("robinson") end it "gives a sensible error when attempting to load a missing attributes file" do @@ -1083,8 +1157,8 @@ describe Chef::Node do it "should load a node from a ruby file" do node.from_file(File.expand_path(File.join(CHEF_SPEC_DATA, "nodes", "test.rb"))) expect(node.name).to eql("test.example.com-short") - expect(node.sunshine).to eql("in") - expect(node.something).to eql("else") + expect(node["sunshine"]).to eql("in") + expect(node["something"]).to eql("else") expect(node.run_list).to eq(["operations-master", "operations-monitoring"]) end @@ -1562,4 +1636,19 @@ describe Chef::Node do end end + describe "method_missing handling" do + it "should have an #empty? method via Chef::Node::Attribute" do + node.default["foo"] = "bar" + expect(node.empty?).to be false + end + + it "it should correctly implement #respond_to?" do + expect(node.respond_to?(:empty?)).to be true + end + + it "it should correctly retrieve the method with #method" do + expect(node.method(:empty?)).to be_kind_of(Method) + end + end + end diff --git a/spec/unit/provider/powershell_script_spec.rb b/spec/unit/provider/powershell_script_spec.rb index 5b5c783986..96869ff31c 100644 --- a/spec/unit/provider/powershell_script_spec.rb +++ b/spec/unit/provider/powershell_script_spec.rb @@ -43,8 +43,8 @@ describe Chef::Provider::PowershellScript, "action_run" do allow(Chef::Platform).to receive(:windows_nano_server?).and_return(true) allow(provider).to receive(:is_forced_32bit).and_return(false) os_info_double = double("os_info") - allow(provider.run_context.node.kernel).to receive(:os_info).and_return(os_info_double) - allow(os_info_double).to receive(:system_directory).and_return("C:\\Windows\\system32") + allow(provider.run_context.node["kernel"]).to receive(:[]).with("os_info").and_return(os_info_double) + allow(os_info_double).to receive(:[]).with("system_directory").and_return("C:\\Windows\\system32") end it "sets the -Command flag as the last flag" do @@ -58,8 +58,8 @@ describe Chef::Provider::PowershellScript, "action_run" do allow(Chef::Platform).to receive(:windows_nano_server?).and_return(false) allow(provider).to receive(:is_forced_32bit).and_return(false) os_info_double = double("os_info") - allow(provider.run_context.node.kernel).to receive(:os_info).and_return(os_info_double) - allow(os_info_double).to receive(:system_directory).and_return("C:\\Windows\\system32") + allow(provider.run_context.node["kernel"]).to receive(:[]).with("os_info").and_return(os_info_double) + allow(os_info_double).to receive(:[]).with("system_directory").and_return("C:\\Windows\\system32") end it "sets the -File flag as the last flag" do diff --git a/spec/unit/run_context_spec.rb b/spec/unit/run_context_spec.rb index 7b2a27e9f6..234cd3c9e1 100644 --- a/spec/unit/run_context_spec.rb +++ b/spec/unit/run_context_spec.rb @@ -153,8 +153,8 @@ describe Chef::RunContext do let(:chef_repo_path) { File.expand_path(File.join(CHEF_SPEC_DATA, "cookbooks")) } let(:node) { node = Chef::Node.new - node.set[:platform] = "ubuntu" - node.set[:platform_version] = "13.04" + node.normal[:platform] = "ubuntu" + node.normal[:platform_version] = "13.04" node.name("testing") node } diff --git a/spec/unit/shell/shell_session_spec.rb b/spec/unit/shell/shell_session_spec.rb index 22f3dc6f0c..259e6096a4 100644 --- a/spec/unit/shell/shell_session_spec.rb +++ b/spec/unit/shell/shell_session_spec.rb @@ -158,7 +158,7 @@ describe Shell::SoloSession do it "keeps json attribs and passes them to the node for consumption" do @session.node_attributes = { "besnard_lakes" => "are_the_dark_horse" } - expect(@session.node.besnard_lakes).to eq("are_the_dark_horse") + expect(@session.node["besnard_lakes"]).to eq("are_the_dark_horse") #pending "1) keep attribs in an ivar 2) pass them to the node 3) feed them to the node on reset" end |