diff options
-rw-r--r-- | lib/chef/node/attribute.rb | 116 | ||||
-rw-r--r-- | lib/chef/node/attribute_collections.rb | 10 | ||||
-rw-r--r-- | lib/chef/node/common_api.rb | 2 | ||||
-rw-r--r-- | lib/chef/node/immutable_collections.rb | 203 | ||||
-rw-r--r-- | lib/chef/node/mixin/state_tracking.rb | 12 | ||||
-rw-r--r-- | spec/unit/mixin/powershell_type_coercions_spec.rb | 13 | ||||
-rw-r--r-- | spec/unit/node/attribute_spec.rb | 79 | ||||
-rw-r--r-- | spec/unit/node/immutable_collections_spec.rb | 42 | ||||
-rw-r--r-- | spec/unit/node/vivid_mash_spec.rb | 37 |
9 files changed, 355 insertions, 159 deletions
diff --git a/lib/chef/node/attribute.rb b/lib/chef/node/attribute.rb index 2998866bb2..389968f543 100644 --- a/lib/chef/node/attribute.rb +++ b/lib/chef/node/attribute.rb @@ -17,7 +17,6 @@ # limitations under the License. # -require "chef/node/mixin/deep_merge_cache" require "chef/node/mixin/immutablize_hash" require "chef/node/mixin/state_tracking" require "chef/node/immutable_collections" @@ -45,7 +44,6 @@ class Chef # expects. This include should probably be deleted? include Enumerable - include Chef::Node::Mixin::DeepMergeCache include Chef::Node::Mixin::StateTracking include Chef::Node::Mixin::ImmutablizeHash @@ -187,6 +185,9 @@ class Chef # return the automatic level attribute component attr_reader :automatic + # return the immutablemash deep merge cache + attr_reader :deep_merge_cache + def initialize(normal, default, override, automatic, node = nil) @default = VividMash.new(default, self, node, :default) @env_default = VividMash.new({}, self, node, :env_default) @@ -202,7 +203,8 @@ class Chef @automatic = VividMash.new(automatic, self, node, :automatic) - super(nil, self, node, :merged) + @deep_merge_cache = ImmutableMash.new({}, self, node, :merged) + @__node__ = node end # Debug what's going on with an attribute. +args+ is a path spec to the @@ -226,6 +228,22 @@ class Chef end end + def reset + @deep_merge_cache = ImmutableMash.new({}, self, @__node__, :merged) + end + + def reset_cache(*path) + if path.empty? + reset + else + container = read(*path) + case container + when Hash, Array + container.reset + end + end + end + # Set the cookbook level default attribute component to +new_data+. def default=(new_data) reset @@ -290,7 +308,7 @@ class Chef # clears attributes from all precedence levels def rm(*args) - with_deep_merged_return_value(self, *args) do + with_deep_merged_return_value(combined_all, *args) do rm_default(*args) rm_normal(*args) rm_override(*args) @@ -337,6 +355,9 @@ class Chef def with_deep_merged_return_value(obj, *path, last) hash = obj.read(*path) return nil unless hash.is_a?(Hash) + # coerce from immutablemash/vividmash to plain-old Hash + # also de-immutablizes and dup's the return value correctly in chef-13 + hash = hash.to_hash ret = hash[last] yield ret @@ -398,16 +419,16 @@ class Chef # all of node['foo'] even if the user only requires node['foo']['bar']['baz']. # - def merged_attributes(*path) - merge_all(path) + def combined_override(*path) + merge_overrides(path) end - def combined_override(*path) - immutablize(merge_overrides(path)) + def combined_all(*path) + path.empty? ? self : read(*path) end def combined_default(*path) - immutablize(merge_defaults(path)) + merge_defaults(path) end def normal_unless(*args) @@ -476,6 +497,14 @@ class Chef merged_attributes.to_s end + def [](key) + @deep_merge_cache[key] + end + + def merged_attributes + @deep_merge_cache + end + def inspect "#<#{self.class} " << (COMPONENTS + [:@merged_attributes, :@properties]).map do |iv| "#{iv}=#{instance_variable_get(iv).inspect}" @@ -484,7 +513,14 @@ class Chef private - # Helper method for merge_all/merge_defaults/merge_overrides. + # For elements like Fixnums, true, nil... + def safe_dup(e) + e.dup + rescue TypeError + e + end + + # Helper method for merge_defaults/merge_overrides. # # apply_path(thing, [ "foo", "bar", "baz" ]) = thing["foo"]["bar"]["baz"] # @@ -514,34 +550,6 @@ class Chef end end - # For elements like Fixnums, true, nil... - def safe_dup(e) - e.dup - rescue TypeError - e - end - - # Deep merge all attribute levels using hash-only merging between different precidence - # levels (so override arrays completely replace arrays set at any default level). - # - # The path allows for selectively deep-merging a subtree of the node object. - # - # @param path [Array] Array of args to method chain to descend into the node object - # @return [attr] Deep Merged values (may be VividMash, Hash, Array, etc) from the node object - def merge_all(path) - components = [ - merge_defaults(path), - apply_path(@normal, path), - merge_overrides(path), - apply_path(@automatic, path), - ] - - ret = components.inject(NIL) do |merged, component| - hash_only_merge!(merged, component) - end - ret == NIL ? nil : ret - end - # Deep merge the default attribute levels with array merging. # # The path allows for selectively deep-merging a subtree of the node object. @@ -613,38 +621,6 @@ class Chef end end - # @api private - def hash_only_merge!(merge_onto, merge_with) - # If there are two Hashes, recursively merge. - if merge_onto.kind_of?(Hash) && merge_with.kind_of?(Hash) - merge_with.each do |key, merge_with_value| - value = - if merge_onto.has_key?(key) - hash_only_merge!(safe_dup(merge_onto[key]), merge_with_value) - else - merge_with_value - end - - # internal_set bypasses converting keys, does convert values and allows writing to immutable mashes - merge_onto.internal_set(key, value) - end - merge_onto - - # If merge_with is nil, don't replace merge_onto - elsif merge_with.nil? - merge_onto - - # In all other cases, replace merge_onto with merge_with - else - if merge_with.kind_of?(Hash) - Chef::Node::ImmutableMash.new(merge_with) - elsif merge_with.kind_of?(Array) - Chef::Node::ImmutableArray.new(merge_with) - else - merge_with - end - end - end end end end diff --git a/lib/chef/node/attribute_collections.rb b/lib/chef/node/attribute_collections.rb index a31b2d2b9b..bb32ae3b54 100644 --- a/lib/chef/node/attribute_collections.rb +++ b/lib/chef/node/attribute_collections.rb @@ -63,13 +63,13 @@ class Chef MUTATOR_METHODS.each do |mutator| define_method(mutator) do |*args, &block| ret = super(*args, &block) - send_reset_cache + send_reset_cache(__path__) ret end end def delete(key, &block) - send_reset_cache(__path__, key) + send_reset_cache(__path__) super end @@ -147,13 +147,13 @@ class Chef # object. def delete(key, &block) - send_reset_cache(__path__, key) + send_reset_cache(__path__) super end MUTATOR_METHODS.each do |mutator| define_method(mutator) do |*args, &block| - send_reset_cache + send_reset_cache(__path__) super(*args, &block) end end @@ -174,7 +174,7 @@ class Chef def []=(key, value) ret = super - send_reset_cache(__path__, key) + send_reset_cache(__path__) ret end diff --git a/lib/chef/node/common_api.rb b/lib/chef/node/common_api.rb index a703c1ef54..fc0ffc57a5 100644 --- a/lib/chef/node/common_api.rb +++ b/lib/chef/node/common_api.rb @@ -1,5 +1,5 @@ #-- -# Copyright:: Copyright 2016, Chef Software, Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/lib/chef/node/immutable_collections.rb b/lib/chef/node/immutable_collections.rb index 848e12d2df..6458db94e9 100644 --- a/lib/chef/node/immutable_collections.rb +++ b/lib/chef/node/immutable_collections.rb @@ -30,22 +30,16 @@ class Chef e end - def convert_value(value) + def convert_value(value, path = nil) case value when Hash - ImmutableMash.new(value, __root__, __node__, __precedence__) + ImmutableMash.new({}, __root__, __node__, __precedence__, path) when Array - ImmutableArray.new(value, __root__, __node__, __precedence__) - when ImmutableMash, ImmutableArray - value + ImmutableArray.new([], __root__, __node__, __precedence__, path) else safe_dup(value).freeze end end - - def immutablize(value) - convert_value(value) - end end # == ImmutableArray @@ -59,17 +53,51 @@ class Chef # Chef::Node::Attribute's values, it overrides all reader methods to # detect staleness and raise an error if accessed when stale. class ImmutableArray < Array + alias_method :internal_clear, :clear + alias_method :internal_replace, :replace + alias_method :internal_push, :<< + alias_method :internal_to_a, :to_a + alias_method :internal_each, :each + private :internal_push, :internal_replace, :internal_clear, :internal_each + protected :internal_to_a + include Immutablize - alias :internal_push :<< - private :internal_push + methods = Array.instance_methods - Object.instance_methods + + [ :!, :!=, :<=>, :==, :===, :eql?, :to_s, :hash, :key, :has_key?, :inspect, :pretty_print, :pretty_print_inspect, :pretty_print_cycle, :pretty_print_instance_variables ] - def initialize(array_data = []) - array_data.each do |value| - internal_push(immutablize(value)) + methods.each do |method| + define_method method do |*args, &block| + ensure_generated_cache! + super(*args, &block) end end + def each + ensure_generated_cache! + # aggressively pre generate the cache, works around ruby being too smart and fiddling with internals + internal_each { |i| i.ensure_generated_cache! if i.respond_to?(:ensure_generated_cache!) } + super + end + + # because sometimes ruby gives us back Arrays or ImmutableArrays out of objects from things like #uniq or array slices + def return_normal_array(array) + if array.respond_to?(:internal_to_a, true) + array.internal_to_a + else + array.to_a + end + end + + def uniq + ensure_generated_cache! + return_normal_array(super) + end + + def initialize(array_data = []) + # Immutable collections no longer have initialized state + end + # For elements like Fixnums, true, nil... def safe_dup(e) e.dup @@ -96,8 +124,81 @@ class Chef alias_method :to_array, :to_a + def [](*args) + ensure_generated_cache! + args.length > 1 ? return_normal_array(super) : super # correctly handle array slices + end + + def reset + @generated_cache = false + @short_circuit_attr_level = nil + internal_clear # redundant? + end + + # @api private + def ensure_generated_cache! + generate_cache unless @generated_cache + @generated_cache = true + end + + # This can be set to e.g. [ :@default ] by the parent container to cause this container + # to only use the default level and to bypass deep merging (the common case is either + # default-level or automatic-level and we aren't doing any deep merging). Right now it + # "optimized" for the case where we're no longer merging anything and only tracking a + # single level, and setting this to anything other than a size=1 array would behave + # in a broken fashion. That could be fixed, but the perf boost would likely not be + # that large in the typical case. + # + # @api private + attr_accessor :short_circuit_attr_levels + private + # deep merging of array attribute within normal and override where they are merged together + def combined_components(components) + combined_values = nil + components.each do |component| + values = __node__.attributes.instance_variable_get(component).read(*__path__) + next unless values.is_a?(Array) + @tracked_components << component + combined_values ||= [] + combined_values += values + end + combined_values + end + + def get_array(component) + array = __node__.attributes.instance_variable_get(component).read(*__path__) + if array.is_a?(Array) + @tracked_components << component + array + end # else nil + end + + def generate_cache + internal_clear + components = [] + @tracked_components = [] + if short_circuit_attr_levels + components << get_array(short_circuit_attr_levels.first) + else + components << combined_components(Attribute::DEFAULT_COMPONENTS) + components << get_array(:@normal) + components << combined_components(Attribute::OVERRIDE_COMPONENTS) + components << get_array(:@automatic) + end + highest = components.compact.last + if highest.is_a?(Array) + internal_replace( highest.each_with_index.map { |x, i| convert_value(x, __path__ + [ i ] ) } ) + end + if @tracked_components.size == 1 + # tracked_components is accurate enough to tell us if we're not really merging + internal_each do |key, value| + value.short_circuit_attr_levels = @tracked_components if value.respond_to?(:short_circuit_attr_levels) + end + end + end + # needed for __path__ def convert_key(key) key @@ -120,19 +221,31 @@ class Chef # it is stale. # * Values can be accessed in attr_reader-like fashion via method_missing. class ImmutableMash < Mash + alias_method :internal_clear, :clear + alias_method :internal_key?, :key? # FIXME: could bypass convert_key in Mash for perf + alias_method :internal_each, :each + include Immutablize include CommonAPI + methods = Hash.instance_methods - Object.instance_methods + + [ :!, :!=, :<=>, :==, :===, :eql?, :to_s, :hash, :key, :has_key?, :inspect, :pretty_print, :pretty_print_inspect, :pretty_print_cycle, :pretty_print_instance_variables ] + + methods.each do |method| + define_method method do |*args, &block| + ensure_generated_cache! + super(*args, &block) + end + end + # this is for deep_merge usage, chef users must never touch this API # @api private def internal_set(key, value) - regular_writer(key, convert_value(value)) + regular_writer(key, convert_value(value, __path__ + [ key ])) end def initialize(mash_data = {}) - mash_data.each do |key, value| - internal_set(key, value) - end + # Immutable collections no longer have initialized state end alias :attribute? :has_key? @@ -168,11 +281,55 @@ class Chef alias_method :to_hash, :to_h - # For elements like Fixnums, true, nil... - def safe_dup(e) - e.dup - rescue TypeError - e + def [](key) + ensure_generated_cache! + super + end + + alias_method :to_hash, :to_h + + def reset + @generated_cache = false + @short_circuit_attr_level = nil + internal_clear # redundant? + end + + # @api private + def ensure_generated_cache! + generate_cache unless @generated_cache + @generated_cache = true + end + + # @api private + attr_accessor :short_circuit_attr_levels + + private + + def generate_cache + internal_clear + components = short_circuit_attr_levels ? short_circuit_attr_levels : Attribute::COMPONENTS.reverse + # tracked_components is not entirely accurate due to the short-circuit + tracked_components = [] + components.each do |component| + subhash = __node__.attributes.instance_variable_get(component).read(*__path__) + unless subhash.nil? # FIXME: nil is used for not present + tracked_components << component + if subhash.kind_of?(Hash) + subhash.keys.each do |key| + next if internal_key?(key) + internal_set(key, subhash[key]) + end + else + break + end + end + end + if tracked_components.size == 1 + # tracked_components is accurate enough to tell us if we're not really merging + internal_each do |key, value| + value.short_circuit_attr_levels = tracked_components if value.respond_to?(:short_circuit_attr_levels) + end + end end prepend Chef::Node::Mixin::StateTracking diff --git a/lib/chef/node/mixin/state_tracking.rb b/lib/chef/node/mixin/state_tracking.rb index 5958973024..690d261df6 100644 --- a/lib/chef/node/mixin/state_tracking.rb +++ b/lib/chef/node/mixin/state_tracking.rb @@ -1,5 +1,5 @@ #-- -# Copyright:: Copyright 2016, Chef Software, Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -24,11 +24,12 @@ class Chef attr_reader :__node__ attr_reader :__precedence__ - def initialize(data = nil, root = self, node = nil, precedence = nil) + def initialize(data = nil, root = self, node = nil, precedence = nil, path = nil) # __path__ and __root__ must be nil when we call super so it knows # to avoid resetting the cache on construction data.nil? ? super() : super(data) - @__path__ = [] + @__path__ = path + @__path__ ||= [] @__root__ = root @__node__ = node @__precedence__ = precedence @@ -76,9 +77,8 @@ class Chef end end - def send_reset_cache(path = nil, key = nil) - next_path = [ path, key ].flatten.compact - __root__.reset_cache(next_path.first) if !__root__.nil? && __root__.respond_to?(:reset_cache) && !next_path.nil? + def send_reset_cache(path) + __root__.reset_cache(*path) if !__root__.nil? && __root__.respond_to?(:reset_cache) && !path.nil? end def copy_state_to(ret, next_path) diff --git a/spec/unit/mixin/powershell_type_coercions_spec.rb b/spec/unit/mixin/powershell_type_coercions_spec.rb index 6f52abccfb..159a0a8d1d 100644 --- a/spec/unit/mixin/powershell_type_coercions_spec.rb +++ b/spec/unit/mixin/powershell_type_coercions_spec.rb @@ -1,6 +1,6 @@ # # Author:: Jay Mundrawala (<jdm@chef.io>) -# Copyright:: Copyright 2015-2016, Chef Software, Inc. +# Copyright:: Copyright 2015-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -64,14 +64,15 @@ describe Chef::Mixin::PowershellTypeCoercions do end it "translates a Chef::Node::ImmutableMash like a hash" do - test_mash = Chef::Node::ImmutableMash.new({ "a" => 1, "b" => 1.2, - "c" => false, "d" => true }) - expect(test_class.translate_type(test_mash)).to eq("@{a=1;b=1.2;c=$false;d=$true}") + node = Chef::Node.new + node.default[:test] = { "a" => 1, "b" => 1.2, "c" => false, "d" => true } + expect(test_class.translate_type(node[:test])).to eq("@{a=1;b=1.2;c=$false;d=$true}") end it "translates a Chef::Node::ImmutableArray like an array" do - test_array = Chef::Node::ImmutableArray.new([true, false]) - expect(test_class.translate_type(test_array)).to eq("@($true,$false)") + node = Chef::Node.new + node.default[:test] = [ true, false ] + expect(test_class.translate_type(node[:test])).to eq("@($true,$false)") end it "falls back :to_psobject if we have not defined at explicit rule" do diff --git a/spec/unit/node/attribute_spec.rb b/spec/unit/node/attribute_spec.rb index cf8d4d4a4f..90b3f1fe51 100644 --- a/spec/unit/node/attribute_spec.rb +++ b/spec/unit/node/attribute_spec.rb @@ -171,6 +171,7 @@ describe Chef::Node::Attribute do } @automatic_hash = { "week" => "friday" } @attributes = Chef::Node::Attribute.new(@attribute_hash, @default_hash, @override_hash, @automatic_hash, node) + allow(node).to receive(:attributes).and_return(@attributes) end describe "initialize" do @@ -179,13 +180,14 @@ describe Chef::Node::Attribute do end it "should take an Automatioc, Normal, Default and Override hash" do - expect { Chef::Node::Attribute.new({}, {}, {}, {}) }.not_to raise_error + expect { Chef::Node::Attribute.new({}, {}, {}, {}, node) }.not_to raise_error end [ :normal, :default, :override, :automatic ].each do |accessor| it "should set #{accessor}" do - na = Chef::Node::Attribute.new({ :normal => true }, { :default => true }, { :override => true }, { :automatic => true }) - expect(na.send(accessor)).to eq({ accessor.to_s => true }) + @attributes = Chef::Node::Attribute.new({ :normal => true }, { :default => true }, { :override => true }, { :automatic => true }, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.send(accessor)).to eq({ accessor.to_s => true }) end end @@ -330,7 +332,8 @@ describe Chef::Node::Attribute do end it "merges nested hashes between precedence levels" do - @attributes = Chef::Node::Attribute.new({}, {}, {}, {}) + @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node) + allow(node).to receive(:attributes).and_return(@attributes) @attributes.env_default = { "a" => { "b" => { "default" => "default" } } } @attributes.normal = { "a" => { "b" => { "normal" => "normal" } } } @attributes.override = { "a" => { "override" => "role" } } @@ -584,8 +587,10 @@ describe Chef::Node::Attribute do "one" => { "six" => "seven" }, "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should yield each top level key" do @@ -632,8 +637,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should yield each top level key and value, post merge rules" do @@ -670,8 +677,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to each_key" do @@ -706,8 +715,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to each_pair" do @@ -742,8 +753,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to each_value" do @@ -786,9 +799,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) - @empty = Chef::Node::Attribute.new({}, {}, {}, {}) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to empty?" do @@ -796,7 +810,9 @@ describe Chef::Node::Attribute do end it "should return true when there are no keys" do - expect(@empty.empty?).to eq(true) + @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.empty?).to eq(true) end it "should return false when there are keys" do @@ -820,8 +836,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to fetch" do @@ -877,8 +895,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to has_value?" do @@ -922,8 +942,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to index" do @@ -963,8 +985,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to values" do @@ -999,8 +1023,10 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) end it "should respond to select" do @@ -1049,10 +1075,11 @@ describe Chef::Node::Attribute do "one" => "six", "snack" => "cookies", }, - {} + {}, + node ) + allow(node).to receive(:attributes).and_return(@attributes) - @empty = Chef::Node::Attribute.new({}, {}, {}, {}) end it "should respond to size" do @@ -1064,7 +1091,9 @@ describe Chef::Node::Attribute do end it "should return 0 for an empty attribute" do - expect(@empty.size).to eq(0) + @attributes = Chef::Node::Attribute.new({}, {}, {}, {}, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.size).to eq(0) end it "should return the number of pairs" do @@ -1092,8 +1121,9 @@ describe Chef::Node::Attribute do describe "to_s" do it "should output simple attributes" do - attributes = Chef::Node::Attribute.new(nil, nil, nil, nil) - expect(attributes.to_s).to eq("{}") + @attributes = Chef::Node::Attribute.new(nil, nil, nil, nil, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.to_s).to eq("{}") end it "should output merged attributes" do @@ -1105,8 +1135,9 @@ describe Chef::Node::Attribute do "b" => 3, "c" => 4, } - attributes = Chef::Node::Attribute.new(nil, default_hash, override_hash, nil) - expect(attributes.to_s).to eq('{"a"=>1, "b"=>3, "c"=>4}') + @attributes = Chef::Node::Attribute.new(nil, default_hash, override_hash, nil, node) + allow(node).to receive(:attributes).and_return(@attributes) + expect(@attributes.to_s).to eq('{"b"=>3, "c"=>4, "a"=>1}') end end diff --git a/spec/unit/node/immutable_collections_spec.rb b/spec/unit/node/immutable_collections_spec.rb index 2d3392041c..96adedddcb 100644 --- a/spec/unit/node/immutable_collections_spec.rb +++ b/spec/unit/node/immutable_collections_spec.rb @@ -20,13 +20,18 @@ require "spec_helper" require "chef/node/immutable_collections" describe Chef::Node::ImmutableMash do + before do - @data_in = { "top" => { "second_level" => "some value" }, - "top_level_2" => %w{array of values}, - "top_level_3" => [{ "hash_array" => 1, "hash_array_b" => 2 }], - "top_level_4" => { "level2" => { "key" => "value" } }, + @data_in = { "key" => + { "top" => { "second_level" => "some value" }, + "top_level_2" => %w{array of values}, + "top_level_3" => [{ "hash_array" => 1, "hash_array_b" => 2 }], + "top_level_4" => { "level2" => { "key" => "value" } }, + }, } - @immutable_mash = Chef::Node::ImmutableMash.new(@data_in) + @node = Chef::Node.new() + @node.attributes.default = @data_in + @immutable_mash = @node["key"] end it "element references like regular hash" do @@ -57,9 +62,9 @@ describe Chef::Node::ImmutableMash do # we only ever absorb VividMashes from other precedence levels, which already have # been coerced to only have string keys, so we do not need to do that work twice (performance). it "does not call convert_value like Mash/VividMash" do - @mash = Chef::Node::ImmutableMash.new({ test: "foo", "test2" => "bar" }) - expect(@mash[:test]).to eql("foo") - expect(@mash["test2"]).to eql("bar") + @node.attributes.default = { test: "foo", "test2" => "bar" } + expect(@node[:test]).to eql("foo") + expect(@node["test2"]).to eql("bar") end describe "to_hash" do @@ -80,7 +85,7 @@ describe Chef::Node::ImmutableMash do end it "should create a mash with the same content" do - expect(@copy).to eq(@immutable_mash) + expect(@copy).to eql(@immutable_mash) end it "should allow mutation" do @@ -175,9 +180,11 @@ end describe Chef::Node::ImmutableArray do before do - @immutable_array = Chef::Node::ImmutableArray.new(%w{foo bar baz} + Array(1..3) + [nil, true, false, [ "el", 0, nil ] ]) - immutable_mash = Chef::Node::ImmutableMash.new({ "m" => "m" }) - @immutable_nested_array = Chef::Node::ImmutableArray.new(["level1", @immutable_array, immutable_mash]) + @node = Chef::Node.new() + @node.attributes.default = { "key" => ["level1", %w{foo bar baz} + Array(1..3) + [nil, true, false, [ "el", 0, nil ] ], { "m" => "m" }] } + @immutable_array = @node["key"][1] + @immutable_mash = @node["key"][2] + @immutable_nested_array = @node["key"] end ## @@ -249,7 +256,7 @@ describe Chef::Node::ImmutableArray do end it "should create an array with the same content" do - expect(@copy).to eq(@immutable_nested_array) + expect(@immutable_nested_array).to eq(@copy) end it "should allow mutation" do @@ -301,7 +308,7 @@ describe Chef::Node::ImmutableArray do end it "should create an array with the same content" do - expect(@copy).to eq(@immutable_nested_array) + expect(@immutable_nested_array).to eq(@copy) end it "should allow mutation" do @@ -314,4 +321,11 @@ describe Chef::Node::ImmutableArray do expect(@immutable_array[1, 2]).to eql(%w{bar baz}) end end + + describe "uniq" do + it "works" do + @node.attributes.default = { "key" => %w{foo bar foo baz bar} } + expect(@node["key"].uniq).to eql(%w{foo bar baz}) + end + end end diff --git a/spec/unit/node/vivid_mash_spec.rb b/spec/unit/node/vivid_mash_spec.rb index 4898c22380..651cfa0058 100644 --- a/spec/unit/node/vivid_mash_spec.rb +++ b/spec/unit/node/vivid_mash_spec.rb @@ -1,5 +1,5 @@ # -# Copyright:: Copyright 2016, Chef Software Inc. +# Copyright:: Copyright 2016-2017, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -60,7 +60,7 @@ describe Chef::Node::VividMash do end it "deep converts values through arrays" do - expect(root).to receive(:reset_cache).with("foo") + expect(root).to receive(:reset_cache).with(no_args) vivid["foo"] = [ { :bar => true } ] expect(vivid["foo"].class).to eql(Chef::Node::AttrArray) expect(vivid["foo"][0].class).to eql(Chef::Node::VividMash) @@ -68,7 +68,7 @@ describe Chef::Node::VividMash do end it "deep converts values through nested arrays" do - expect(root).to receive(:reset_cache).with("foo") + expect(root).to receive(:reset_cache).with(no_args) vivid["foo"] = [ [ { :bar => true } ] ] expect(vivid["foo"].class).to eql(Chef::Node::AttrArray) expect(vivid["foo"][0].class).to eql(Chef::Node::AttrArray) @@ -77,7 +77,7 @@ describe Chef::Node::VividMash do end it "deep converts values through hashes" do - expect(root).to receive(:reset_cache).with("foo") + expect(root).to receive(:reset_cache).with(no_args) vivid["foo"] = { baz: { :bar => true } } expect(vivid["foo"]).to be_an_instance_of(Chef::Node::VividMash) expect(vivid["foo"]["baz"]).to be_an_instance_of(Chef::Node::VividMash) @@ -184,42 +184,55 @@ describe Chef::Node::VividMash do it "should deeply autovivify" do expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven", "eight") 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(no_args) 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(no_args) expect(root).to receive(:reset_cache).at_least(:once).with("array") + expect(root).to receive(:reset_cache).at_least(:once).with("array", "five") 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") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three") 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") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "two", "three", "four") 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(no_args) 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(no_args) expect(root).to receive(:reset_cache).at_least(:once).with("nil") + expect(root).to receive(:reset_cache).at_least(:once).with("nil", "one") vivid.write("nil", "one", "two", "three") expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ], "nil" => { "one" => { "two" => "three" } } }) end @@ -240,6 +253,10 @@ describe Chef::Node::VividMash do it "should deeply autovivify" do expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven") + expect(root).to receive(:reset_cache).at_least(:once).with("one", "five", "six", "seven", "eight") vivid.write!("one", "five", "six", "seven", "eight", "nine", "ten") expect(vivid["one"]["five"]["six"]["seven"]["eight"]["nine"]).to eql("ten") end @@ -295,7 +312,7 @@ describe Chef::Node::VividMash do end it "should unlink hashes" do - expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect( vivid.unlink("one") ).to eql({ "two" => { "three" => "four" } }) expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil }) end @@ -307,7 +324,7 @@ describe Chef::Node::VividMash do end it "should unlink nil" do - expect(root).to receive(:reset_cache).at_least(:once).with("nil") + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect(vivid.unlink("nil")).to eql(nil) expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] }) end @@ -327,7 +344,7 @@ describe Chef::Node::VividMash do end it "should unlink! hashes" do - expect(root).to receive(:reset_cache).at_least(:once).with("one") + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect( vivid.unlink!("one") ).to eql({ "two" => { "three" => "four" } }) expect(vivid).to eql({ "array" => [ 0, 1, 2 ], "nil" => nil }) end @@ -339,7 +356,7 @@ describe Chef::Node::VividMash do end it "should unlink! nil" do - expect(root).to receive(:reset_cache).at_least(:once).with("nil") + expect(root).to receive(:reset_cache).at_least(:once).with(no_args) expect(vivid.unlink!("nil")).to eql(nil) expect(vivid).to eql({ "one" => { "two" => { "three" => "four" } }, "array" => [ 0, 1, 2 ] }) end |